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

On Fri, 2012-08-24 at 11:30 +0530, Archit Taneja wrote:

On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote:

quoted

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>

quoted

  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