Hi Sam,
Thank you for the review.
On Sun, Dec 15, 2019 at 01:03:31PM +0100, Sam Ravnborg wrote:
> Hi Laurent.
>
> One nit below.
>
> > +
> > +struct display_connector {
> > + struct drm_bridge bridge;
> > +
> > + const char *label;
>
> label is defined here.
>
> > + struct gpio_desc *hpd_gpio;
> > + int hpd_irq;
> > +};
> > +
> ...
> > +
> > + /* Get the optional connector label. */
> > + of_property_read_string(pdev->dev.of_node, "label", &conn->label);
>
> Assinged here.
>
> > +
> > + /*
> > + * Get the HPD GPIO for DVI and HDMI connectors. If the GPIO can provide
> > + * edge interrupts, register an interrupt handler.
> > + */
> ...
> > +
> > + dev_info(&pdev->dev,
> > + "Found %s display connector '%s' %s DDC bus and %s HPD GPIO (ops 0x%x)\n",
> > + drm_get_connector_type_name(conn->bridge.type),
> > + conn->label ? conn->label : "<unlabelled>",
> > + conn->bridge.ddc ? "with" : "without",
> > + conn->hpd_gpio ? "with" : "without",
> > + conn->bridge.ops);
>
> And this is the only user - within the same function where label is
> assigned.
> We could use a loacal variable, no need to have it in struct display_connector
> unless futher use is planned.
Agreed, I'll move the field to a local variable. We can always move it
back to the display_connector structure later if needed.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
