From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergei Shtylyov Subject: Re: [PATCH 4/5] USB: OTG: msm: Configure PHY Analog and Digital voltage domains Date: Fri, 29 Apr 2011 14:14:05 +0400 Message-ID: <4DBA8F6D.7000508@ru.mvista.com> References: <1303977693-18389-1-git-send-email-pkondeti@codeaurora.org> <1303977693-18389-4-git-send-email-pkondeti@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wy0-f174.google.com ([74.125.82.174]:45245 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752060Ab1D2KPp (ORCPT ); Fri, 29 Apr 2011 06:15:45 -0400 In-Reply-To: <1303977693-18389-4-git-send-email-pkondeti@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Pavankumar Kondeti Cc: greg@kroah.com, linux-usb@vger.kernel.org, linux-arm-msm@vger.kernel.org, Anji jonnala On 28.04.2011 12:01, Pavankumar Kondeti wrote: > From: Anji jonnala > Signed-off-by: Anji jonnala > Signed-off-by: Pavankumar Kondeti [...] > diff --git a/drivers/usb/otg/msm_otg.c b/drivers/usb/otg/msm_otg.c > index 526a650..425418d 100644 > --- a/drivers/usb/otg/msm_otg.c > +++ b/drivers/usb/otg/msm_otg.c > @@ -38,6 +38,7 @@ > #include > #include > #include > +#include > > #include > > @@ -45,6 +46,175 @@ > #define DRIVER_NAME "msm_otg" > > #define ULPI_IO_TIMEOUT_USEC (10 * 1000) > + > +#define USB_PHY_3P3_VOL_MIN 3050000 /* uV */ > +#define USB_PHY_3P3_VOL_MAX 3300000 /* uV */ > +#define USB_PHY_3P3_HPM_LOAD 50000 /* uA */ > +#define USB_PHY_3P3_LPM_LOAD 4000 /* uA */ > + > +#define USB_PHY_1P8_VOL_MIN 1800000 /* uV */ > +#define USB_PHY_1P8_VOL_MAX 1800000 /* uV */ > +#define USB_PHY_1P8_HPM_LOAD 50000 /* uA */ > +#define USB_PHY_1P8_LPM_LOAD 4000 /* uA */ > + > +#define USB_PHY_VDD_DIG_VOL_MIN 1000000 /* uV */ > +#define USB_PHY_VDD_DIG_VOL_MAX 1320000 /* uV */ > + > +static struct regulator *hsusb_3p3; > +static struct regulator *hsusb_1p8; > +static struct regulator *hsusb_vddcx; > + > +static int msm_hsusb_init_vddcx(struct msm_otg *motg, int init) > +{ > + int ret = 0; > + > + if (init) { > + hsusb_vddcx = regulator_get(motg->otg.dev, "HSUSB_VDDCX"); > + if (IS_ERR(hsusb_vddcx)) { > + dev_err(motg->otg.dev, "unable to get hsusb vddcx\n"); > + return PTR_ERR(hsusb_vddcx); > + } > + > + ret = regulator_set_voltage(hsusb_vddcx, > + USB_PHY_VDD_DIG_VOL_MIN, > + USB_PHY_VDD_DIG_VOL_MAX); > + if (ret) { > + dev_err(motg->otg.dev, "unable to set the voltage" > + "for hsusb vddcx\n"); > + regulator_put(hsusb_vddcx); > + return ret; > + } > + > + ret = regulator_enable(hsusb_vddcx); > + if (ret) { > + dev_err(motg->otg.dev, "unable to enable hsusb vddcx\n"); > + regulator_put(hsusb_vddcx); > + } > + } else { > + ret = regulator_set_voltage(hsusb_vddcx, 0, > + USB_PHY_VDD_DIG_VOL_MIN); > + if (ret) { > + dev_err(motg->otg.dev, "unable to set the voltage" > + "for hsusb vddcx\n"); > + return ret; > + } > + ret = regulator_disable(hsusb_vddcx); > + if (ret) { > + dev_err(motg->otg.dev, "unable to disable hsusb vddcx\n"); > + return ret; I suspect you should call regulator_put() on error cases as well... > + } > + > + regulator_put(hsusb_vddcx); > + } > + > + return ret; > +} > + > +static int msm_hsusb_ldo_init(struct msm_otg *motg, int init) > +{ > + int rc = 0; > + > + if (init) { > + hsusb_3p3 = regulator_get(motg->otg.dev, "HSUSB_3p3"); > + if (IS_ERR(hsusb_3p3)) { > + dev_err(motg->otg.dev, "unable to get hsusb 3p3\n"); > + return PTR_ERR(hsusb_3p3); > + } > + > + rc = regulator_set_voltage(hsusb_3p3, USB_PHY_3P3_VOL_MIN, > + USB_PHY_3P3_VOL_MAX); > + if (rc) { > + dev_err(motg->otg.dev, "unable to set voltage level for" > + "hsusb 3p3\n"); > + goto put_3p3; > + } > + rc = regulator_enable(hsusb_3p3); > + if (rc) { > + dev_err(motg->otg.dev, "unable to enable the hsusb 3p3\n"); > + goto put_3p3_lpm; > + } > + hsusb_1p8 = regulator_get(motg->otg.dev, "HSUSB_1p8"); > + if (IS_ERR(hsusb_1p8)) { > + dev_err(motg->otg.dev, "unable to get hsusb 1p8\n"); > + rc = PTR_ERR(hsusb_1p8); > + goto put_3p3_lpm; > + } > + rc = regulator_set_voltage(hsusb_1p8, USB_PHY_1P8_VOL_MIN, > + USB_PHY_1P8_VOL_MAX); > + if (rc) { > + dev_err(motg->otg.dev, "unable to set voltage level for" > + "hsusb 1p8\n"); > + goto put_1p8; > + } > + rc = regulator_enable(hsusb_1p8); > + if (rc) { > + dev_err(motg->otg.dev, "unable to enable the hsusb 1p8\n"); > + goto disable_1p8; > + } > + > + return 0; > + } > + > +disable_1p8: > + regulator_set_voltage(hsusb_1p8, 0, USB_PHY_1P8_VOL_MAX); > + regulator_disable(hsusb_1p8); Why call regualator_disable() if regulator_enable() has just failed? > +put_1p8: > + regulator_put(hsusb_1p8); > +put_3p3_lpm: > + regulator_set_voltage(hsusb_3p3, 0, USB_PHY_3P3_VOL_MAX); > +put_3p3: > + regulator_put(hsusb_3p3); > + return rc; > +} [...] > @@ -1345,6 +1532,10 @@ free_irq: > disable_clks: > clk_disable(motg->pclk); > clk_disable(motg->clk); > +free_ldo_init: 'ldo_exit' perhaps? > + msm_hsusb_ldo_init(motg, 0); > +free_config_vddcx: 'vddcx_exit' perhaps? > + msm_hsusb_init_vddcx(motg, 0); > free_regs: > iounmap(motg->regs); > put_core_clk: WBR, Sergei