Re: [PATCH v3 11/50] drm/bridge: Add bridge driver for display connectors

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel