Re: [PATCH 1/8] OMAPDSS: HDMI: Move GPIO handling to HDMI driver

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

On Fri, 2012-08-24 at 11:30 +0530, Archit Taneja wrote:
> On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:
> > We currently manage HDMI GPIOs in the board files via
> > platform_enable/disable calls. This won't work with device tree, and in
> > any case the correct place to manage the GPIOs is in the HDMI driver.
> >
> > This patch moves the handling of the GPIOs to the HDMI driver. The GPIO
> > handling is moved to the common hdmi.c file, and this probably needs to
> > be revisited when adding OMAP5 HDMI support to see if the GPIO handling
> > needs to be moved to IP specific files.

<snip>

> >   static int __init hdmi_init_display(struct omap_dss_device *dssdev)
> >   {
> > +	int r;
> > +
> > +	struct gpio gpios[] = {
> > +		{ hdmi.ct_cp_hpd_gpio, GPIOF_OUT_INIT_LOW, "hdmi_ct_cp_hpd" },
> > +		{ hdmi.ls_oe_gpio, GPIOF_OUT_INIT_LOW, "hdmi_ls_oe" },
> > +		{ hdmi.hpd_gpio, GPIOF_DIR_IN, "hdmi_hpd" },
> > +	};
> > +
> >   	DSSDBG("init_display\n");
> >
> >   	dss_init_hdmi_ip_ops(&hdmi.ip_data);
> > +
> > +	r = gpio_request_array(gpios, ARRAY_SIZE(gpios));
> > +	if (r)
> > +		return r;
> > +
> 
> Is there a reason to request these gpios in hdmi_init_display()? Why 
> can't these be requested simply in the probe of the hdmi platform 
> device. These gpios are sort of specific to the hdmi output on OMAP, 
> aren't they?

Well, it is a bit ugly, agreed. But I'm not sure it can be helped
easily.

First, the struct omap_dss_hdmi_data where the gpios are passed from
board file, is really "panel" platform data. I.e. it's attached to the
hdmi dssdev, and thus we can't handle it in hdmi output driver's probe,
as we don't have dssdevs there.

Second, the gpios are actually not OMAP HDMI stuff. They belong to the
tpd12s015 chip, sitting between OMAP HDMI output and the HDMI connector,
which is the ESD/level shifter/whatnot (I don't remember exactly what it
does =). So making the GPIOs a property of the OMAP HDMI device wouldn't
be right either.

tpd12s015 is rather simple chip, but it still has the gpios and require
special handling. I think the only way to properly handle this would be
to have a chain of external display devices, and the tpd12s015 would be
handled by a separate driver. Until then, the gpio handling is quite
hacky.

We could move the gpios to omapdss's platform data, but it would still
be wrong, and I'm not sure if it'd be any cleaner hack.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]