| Message ID | 1345729514-2441-5-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote: > The HDMI driver requires vdda_hdmi_dac power for operation, but does not > enable it. This has worked because the regulator has been always > enabled. > > But this may not always be the case, as I encountered when implementing > HDMI device tree support. > > This patch changes the HDMI driver to use the vdda_hdmi_dac. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/video/omap2/dss/hdmi.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c > index 96a6e29..ccfc677 100644 > --- a/drivers/video/omap2/dss/hdmi.c > +++ b/drivers/video/omap2/dss/hdmi.c > @@ -33,6 +33,7 @@ > #include <linux/pm_runtime.h> > #include <linux/clk.h> > #include <linux/gpio.h> > +#include <linux/regulator/consumer.h> > #include <video/omapdss.h> > > #include "ti_hdmi.h" > @@ -62,6 +63,7 @@ static struct { > struct hdmi_ip_data ip_data; > > struct clk *sys_clk; > + struct regulator *vdda_hdmi_dac_reg; > > int ct_cp_hpd_gpio; > int ls_oe_gpio; > @@ -331,6 +333,19 @@ static int __init hdmi_init_display(struct omap_dss_device *dssdev) > > dss_init_hdmi_ip_ops(&hdmi.ip_data); > > + if (hdmi.vdda_hdmi_dac_reg == NULL) { > + struct regulator *reg; > + > + reg = devm_regulator_get(&hdmi.pdev->dev, "vdda_hdmi_dac"); There is no corresponding devm_regulator_put() call here, I guess that's what devm_* calls are supposed to help us with. But the only place I saw the usage of dev_regulator_get() is here: sound/soc/ux500/ux500_msp_dai.c And here, they are doing devm_regulator_put() calls to, so I was wondering what the deal is. Archit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2012-08-24 at 11:50 +0530, Archit Taneja wrote: > On Thursday 23 August 2012 07:15 PM, Tomi Valkeinen wrote: > > The HDMI driver requires vdda_hdmi_dac power for operation, but does not > > enable it. This has worked because the regulator has been always > > enabled. > > > > But this may not always be the case, as I encountered when implementing > > HDMI device tree support. > > > > This patch changes the HDMI driver to use the vdda_hdmi_dac. > > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > > --- > > drivers/video/omap2/dss/hdmi.c | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c > > index 96a6e29..ccfc677 100644 > > --- a/drivers/video/omap2/dss/hdmi.c > > +++ b/drivers/video/omap2/dss/hdmi.c > > @@ -33,6 +33,7 @@ > > #include <linux/pm_runtime.h> > > #include <linux/clk.h> > > #include <linux/gpio.h> > > +#include <linux/regulator/consumer.h> > > #include <video/omapdss.h> > > > > #include "ti_hdmi.h" > > @@ -62,6 +63,7 @@ static struct { > > struct hdmi_ip_data ip_data; > > > > struct clk *sys_clk; > > + struct regulator *vdda_hdmi_dac_reg; > > > > int ct_cp_hpd_gpio; > > int ls_oe_gpio; > > @@ -331,6 +333,19 @@ static int __init hdmi_init_display(struct omap_dss_device *dssdev) > > > > dss_init_hdmi_ip_ops(&hdmi.ip_data); > > > > + if (hdmi.vdda_hdmi_dac_reg == NULL) { > > + struct regulator *reg; > > + > > + reg = devm_regulator_get(&hdmi.pdev->dev, "vdda_hdmi_dac"); > > There is no corresponding devm_regulator_put() call here, I guess that's > what devm_* calls are supposed to help us with. But the only place I saw > the usage of dev_regulator_get() is here: > > sound/soc/ux500/ux500_msp_dai.c > > And here, they are doing devm_regulator_put() calls to, so I was > wondering what the deal is. No idea. But there are other places also: sound/soc/codecs/wm5100.c, sound/soc/codecs/wm8996.c, sound/soc/soc-dapm.c, and those don't use put(). Anyway, I think it's quite clear how devm_* functions are used, so the put call in ux500_msp_dai.c is probably just a mistake. I also want to mention that doing the regulator_get in hdmi_init_display() is somewhat strange, but the point is to do the regulator_get only if a hdmi display has been defined in the board file. If we did it unconditionally in hdmi's probe, we'd try to get the regulator always, and it could be that there's no regulator if the hdmi is not used on a particular board. This is again something that feels hackish, and I'd like to find a cleaner solution to it. Tomi
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c index 96a6e29..ccfc677 100644 --- a/drivers/video/omap2/dss/hdmi.c +++ b/drivers/video/omap2/dss/hdmi.c @@ -33,6 +33,7 @@ #include <linux/pm_runtime.h> #include <linux/clk.h> #include <linux/gpio.h> +#include <linux/regulator/consumer.h> #include <video/omapdss.h> #include "ti_hdmi.h" @@ -62,6 +63,7 @@ static struct { struct hdmi_ip_data ip_data; struct clk *sys_clk; + struct regulator *vdda_hdmi_dac_reg; int ct_cp_hpd_gpio; int ls_oe_gpio; @@ -331,6 +333,19 @@ static int __init hdmi_init_display(struct omap_dss_device *dssdev) dss_init_hdmi_ip_ops(&hdmi.ip_data); + if (hdmi.vdda_hdmi_dac_reg == NULL) { + struct regulator *reg; + + reg = devm_regulator_get(&hdmi.pdev->dev, "vdda_hdmi_dac"); + + if (IS_ERR(reg)) { + DSSERR("can't get VDDA_HDMI_DAC regulator\n"); + return PTR_ERR(reg); + } + + hdmi.vdda_hdmi_dac_reg = reg; + } + r = gpio_request_array(gpios, ARRAY_SIZE(gpios)); if (r) return r; @@ -495,6 +510,10 @@ static int hdmi_power_on(struct omap_dss_device *dssdev) /* wait 300us after CT_CP_HPD for the 5V power output to reach 90% */ udelay(300); + r = regulator_enable(hdmi.vdda_hdmi_dac_reg); + if (r) + goto err_vdac_enable; + r = hdmi_runtime_get(); if (r) goto err_runtime_get; @@ -562,6 +581,8 @@ err_phy_enable: err_pll_enable: hdmi_runtime_put(); err_runtime_get: + regulator_disable(hdmi.vdda_hdmi_dac_reg); +err_vdac_enable: gpio_set_value(hdmi.ct_cp_hpd_gpio, 0); gpio_set_value(hdmi.ls_oe_gpio, 0); return -EIO; @@ -576,6 +597,8 @@ static void hdmi_power_off(struct omap_dss_device *dssdev) hdmi.ip_data.ops->pll_disable(&hdmi.ip_data); hdmi_runtime_put(); + regulator_disable(hdmi.vdda_hdmi_dac_reg); + gpio_set_value(hdmi.ct_cp_hpd_gpio, 0); gpio_set_value(hdmi.ls_oe_gpio, 0); }
The HDMI driver requires vdda_hdmi_dac power for operation, but does not enable it. This has worked because the regulator has been always enabled. But this may not always be the case, as I encountered when implementing HDMI device tree support. This patch changes the HDMI driver to use the vdda_hdmi_dac. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/video/omap2/dss/hdmi.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)