From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Chen Subject: Re: [PATCH 14/21] usb: chipidea: msm: Add proper clk and reset support Date: Wed, 29 Jun 2016 15:02:12 +0800 Message-ID: <20160629070212.GI25236@shlinux2> References: <20160626072838.28082-1-stephen.boyd@linaro.org> <20160626072838.28082-15-stephen.boyd@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-pf0-f196.google.com ([209.85.192.196]:32928 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751020AbcF2HJM (ORCPT ); Wed, 29 Jun 2016 03:09:12 -0400 Content-Disposition: inline In-Reply-To: <20160626072838.28082-15-stephen.boyd@linaro.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Stephen Boyd Cc: linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Andy Gross , Bjorn Andersson , Neil Armstrong , Arnd Bergmann , Felipe Balbi , Peter Chen , Greg Kroah-Hartman On Sun, Jun 26, 2016 at 12:28:31AM -0700, Stephen Boyd wrote: > The msm chipidea controller uses two main clks, an AHB clk to > read/write the MMIO registers and a core clk called the system > clk that drives the controller itself. Add support for these clks > as they're required in all designs. > > Also add support for an optional third clk that we need to turn > on to read/write the ULPI phy registers. Some ULPI phys drive > this clk themselves and so it isn't necessary to turn on to probe > a ULPI device, but the HSIC phy doesn't provide one itself, so we > must turn it on here. > > Cc: Peter Chen > Cc: Greg Kroah-Hartman > Signed-off-by: Stephen Boyd > --- > drivers/usb/chipidea/ci_hdrc_msm.c | 58 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 54 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c > index 07cccd24a87f..40249b0e3e93 100644 > --- a/drivers/usb/chipidea/ci_hdrc_msm.c > +++ b/drivers/usb/chipidea/ci_hdrc_msm.c > @@ -10,11 +10,19 @@ > #include > #include > #include > +#include > +#include > > #include "ci.h" > > #define HS_PHY_AHB_MODE 0x0098 > > +struct ci_hdrc_msm { > + struct platform_device *ci; > + struct clk *core_clk; > + struct clk *iface_clk; > +}; > + > static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event) > { > struct device *dev = ci->gadget.dev.parent; > @@ -43,34 +51,76 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = { > > static int ci_hdrc_msm_probe(struct platform_device *pdev) > { > + struct ci_hdrc_msm *ci; > struct platform_device *plat_ci; > + struct clk *clk; > + struct reset_control *reset; > + int ret; > > dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n"); > > + ci = devm_kzalloc(&pdev->dev, sizeof(*ci), GFP_KERNEL); > + if (!ci) > + return -ENOMEM; > + platform_set_drvdata(pdev, ci); > + > + reset = devm_reset_control_get(&pdev->dev, "core"); > + if (IS_ERR(reset)) > + return PTR_ERR(reset); > + > + ci->core_clk = clk = devm_clk_get(&pdev->dev, "core"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + ci->iface_clk = clk = devm_clk_get(&pdev->dev, "iface"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); You say it has three clocks in commit log, why it is only two in the code? > + > + reset_control_assert(reset); > + usleep_range(10000, 12000); > + reset_control_deassert(reset); > + > + ret = clk_prepare_enable(ci->core_clk); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(ci->iface_clk); > + if (ret) > + goto err_iface; > + > plat_ci = ci_hdrc_add_device(&pdev->dev, > pdev->resource, pdev->num_resources, > &ci_hdrc_msm_platdata); ci->plat_ci(or ci) = ... Delete the local variable plat_ci > if (IS_ERR(plat_ci)) { > dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n"); > - return PTR_ERR(plat_ci); > + ret = PTR_ERR(plat_ci); > + goto err_mux; > } > > - platform_set_drvdata(pdev, plat_ci); > + ci->ci = plat_ci; [...] -- Best Regards, Peter Chen From mboxrd@z Thu Jan 1 00:00:00 1970 From: hzpeterchen@gmail.com (Peter Chen) Date: Wed, 29 Jun 2016 15:02:12 +0800 Subject: [PATCH 14/21] usb: chipidea: msm: Add proper clk and reset support In-Reply-To: <20160626072838.28082-15-stephen.boyd@linaro.org> References: <20160626072838.28082-1-stephen.boyd@linaro.org> <20160626072838.28082-15-stephen.boyd@linaro.org> Message-ID: <20160629070212.GI25236@shlinux2> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Sun, Jun 26, 2016 at 12:28:31AM -0700, Stephen Boyd wrote: > The msm chipidea controller uses two main clks, an AHB clk to > read/write the MMIO registers and a core clk called the system > clk that drives the controller itself. Add support for these clks > as they're required in all designs. > > Also add support for an optional third clk that we need to turn > on to read/write the ULPI phy registers. Some ULPI phys drive > this clk themselves and so it isn't necessary to turn on to probe > a ULPI device, but the HSIC phy doesn't provide one itself, so we > must turn it on here. > > Cc: Peter Chen > Cc: Greg Kroah-Hartman > Signed-off-by: Stephen Boyd > --- > drivers/usb/chipidea/ci_hdrc_msm.c | 58 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 54 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c > index 07cccd24a87f..40249b0e3e93 100644 > --- a/drivers/usb/chipidea/ci_hdrc_msm.c > +++ b/drivers/usb/chipidea/ci_hdrc_msm.c > @@ -10,11 +10,19 @@ > #include > #include > #include > +#include > +#include > > #include "ci.h" > > #define HS_PHY_AHB_MODE 0x0098 > > +struct ci_hdrc_msm { > + struct platform_device *ci; > + struct clk *core_clk; > + struct clk *iface_clk; > +}; > + > static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event) > { > struct device *dev = ci->gadget.dev.parent; > @@ -43,34 +51,76 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = { > > static int ci_hdrc_msm_probe(struct platform_device *pdev) > { > + struct ci_hdrc_msm *ci; > struct platform_device *plat_ci; > + struct clk *clk; > + struct reset_control *reset; > + int ret; > > dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n"); > > + ci = devm_kzalloc(&pdev->dev, sizeof(*ci), GFP_KERNEL); > + if (!ci) > + return -ENOMEM; > + platform_set_drvdata(pdev, ci); > + > + reset = devm_reset_control_get(&pdev->dev, "core"); > + if (IS_ERR(reset)) > + return PTR_ERR(reset); > + > + ci->core_clk = clk = devm_clk_get(&pdev->dev, "core"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + ci->iface_clk = clk = devm_clk_get(&pdev->dev, "iface"); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); You say it has three clocks in commit log, why it is only two in the code? > + > + reset_control_assert(reset); > + usleep_range(10000, 12000); > + reset_control_deassert(reset); > + > + ret = clk_prepare_enable(ci->core_clk); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(ci->iface_clk); > + if (ret) > + goto err_iface; > + > plat_ci = ci_hdrc_add_device(&pdev->dev, > pdev->resource, pdev->num_resources, > &ci_hdrc_msm_platdata); ci->plat_ci(or ci) = ... Delete the local variable plat_ci > if (IS_ERR(plat_ci)) { > dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n"); > - return PTR_ERR(plat_ci); > + ret = PTR_ERR(plat_ci); > + goto err_mux; > } > > - platform_set_drvdata(pdev, plat_ci); > + ci->ci = plat_ci; [...] -- Best Regards, Peter Chen