<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, Mar 28, 2025 at 4:34 AM Luca Ceresoli <<a href="mailto:luca.ceresoli@bootlin.com">luca.ceresoli@bootlin.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Anusha,<br>
<br>
Thanks for your continued effort.<br>
<br>
I have a few minor comments. Nothing big, but since Maxime requested a<br>
change you'll have to send a new iteration, so find my comments below.<br>
<br>
On Thu, 27 Mar 2025 10:55:39 -0400<br>
Anusha Srivatsa <<a href="mailto:asrivats@redhat.com" target="_blank">asrivats@redhat.com</a>> wrote:<br>
<br>
[...]<br>
<br>
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h<br>
> index a9c042c8dea1a82ef979c7a68204e0b55483fc28..53251c6b11d78149ede3dad41ffa6a88f3c3c58b 100644<br>
> --- a/include/drm/drm_panel.h<br>
> +++ b/include/drm/drm_panel.h<br>
> @@ -28,6 +28,7 @@<br>
>  #include <linux/errno.h><br>
>  #include <linux/list.h><br>
>  #include <linux/mutex.h><br>
> +#include <linux/kref.h><br>
<br>
Minor nit: you don't need this include in patch 1. You should move it<br>
to patch 2 where it is actually used.<br>
<br>
> @@ -268,6 +269,28 @@ struct drm_panel {<br>
>       bool enabled;<br>
>  };<br>
>  <br>
> +void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,<br>
> +                          const struct drm_panel_funcs *funcs,<br>
> +                          int connector_type);<br>
> +<br>
> +/**<br>
> + * devm_drm_panel_alloc - Allocate and initialize an refcounted panel<br>
                                                     ^^<br>
A typo here is certainly not a huge problem, but I think I had already<br>
reported this should be "a refcounted panel".<br>
<br></blockquote><div>Yeah you had and I thought I had taken care of it. WIll change this.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + * @dev: struct device of the panel device<br>
> + * @type: the type of the struct which contains struct &drm_panel<br>
> + * @member: the name of the &drm_panel within @type<br>
> + * @funcs: callbacks for this panel<br>
> + * @connector_type: the connector type (DRM_MODE_CONNECTOR_*) corresponding to<br>
> + * the panel interface<br>
> + * Returns:<br>
> + * Pointer to container structure embedding the panel, ERR_PTR on failure.<br>
> + * The reference count is initialised to 1 and is automatically  given back<br>
> + * by devm action.<br>
> + */<br>
<br>
In addition to Maxime's comment: I think it's a common practice to have<br>
an empty line after the last @argument and also before the "Returns:"<br>
line, to improve readability<br>
<br></blockquote><div>On it!</div><div>Thanks,</div><div>Anusha <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Luca<br>
<br>
-- <br>
Luca Ceresoli, Bootlin<br>
Embedded Linux and Kernel engineering<br>
<a href="https://bootlin.com" rel="noreferrer" target="_blank">https://bootlin.com</a><br>
<br>
</blockquote></div></div>