On Wed, Jul 31, 2013 at 03:32:14PM +0800, Peter Chen wrote:
> On Wed, Jul 31, 2013 at 09:23:56AM +0200, Michael Grzeschik wrote:
> > Hi Peter,
> >
> > On Wed, Jul 31, 2013 at 09:39:50AM +0800, Peter Chen wrote:
> > > On Tue, Jul 30, 2013 at 02:41:23PM +0200, Michael Grzeschik wrote:
> > > > From: Michael Grzeschik <mgr@xxxxxxxxxxxxxx>
> > > >
> > > > This patch moves the regulator code from ci_hdrc_imx gluecode to the
> > > > core layer. It also checks the errorpathes in case the platformglue
> > > > didn't prepare an regulator for this driver.
> > > >
> > > > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/usb/chipidea/ci_hdrc_imx.c | 26 ++------------------------
> > > > drivers/usb/chipidea/core.c | 16 ++++++++++++++++
> > > > include/linux/usb/chipidea.h | 1 +
> > > > 3 files changed, 19 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > > index 06bc775..d2937e1 100644
> > > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > > @@ -19,7 +19,6 @@
> > > > #include <linux/dma-mapping.h>
> > > > #include <linux/usb/chipidea.h>
> > > > #include <linux/clk.h>
> > > > -#include <linux/regulator/consumer.h>
> > > >
> > > > #include "ci.h"
> > > > #include "ci_hdrc_imx.h"
> > > > @@ -31,7 +30,6 @@ struct ci_hdrc_imx_data {
> > > > struct usb_phy *phy;
> > > > struct platform_device *ci_pdev;
> > > > struct clk *clk;
> > > > - struct regulator *reg_vbus;
> > > > };
> > > >
> > > > static const struct usbmisc_ops *usbmisc_ops;
> > > > @@ -134,20 +132,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > > > goto err_clk;
> > > > }
> > > >
> > > > - /* we only support host now, so enable vbus here */
> > > > - data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > > > - if (!IS_ERR(data->reg_vbus)) {
> > > > - ret = regulator_enable(data->reg_vbus);
> > > > - if (ret) {
> > > > - dev_err(&pdev->dev,
> > > > - "Failed to enable vbus regulator, err=%d\n",
> > > > - ret);
> > > > - goto err_clk;
> > > > - }
> > > > - } else {
> > > > - data->reg_vbus = NULL;
> > > > - }
> > > > -
> > > > pdata.phy = data->phy;
> > > >
> > > > if (!pdev->dev.dma_mask)
> > > > @@ -160,7 +144,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > > > if (ret) {
> > > > dev_err(&pdev->dev,
> > > > "usbmisc init failed, ret=%d\n", ret);
> > > > - goto err;
> > > > + goto err_clk;
> > > > }
> > > > }
> > > >
> > > > @@ -172,7 +156,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > > > dev_err(&pdev->dev,
> > > > "Can't register ci_hdrc platform device, err=%d\n",
> > > > ret);
> > > > - goto err;
> > > > + goto err_clk;
> > > > }
> > > >
> > > > if (usbmisc_ops && usbmisc_ops->post) {
> > > > @@ -193,9 +177,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > > >
> > > > disable_device:
> > > > ci_hdrc_remove_device(data->ci_pdev);
> > > > -err:
> > > > - if (data->reg_vbus)
> > > > - regulator_disable(data->reg_vbus);
> > > > err_clk:
> > > > clk_disable_unprepare(data->clk);
> > > > return ret;
> > > > @@ -208,9 +189,6 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
> > > > pm_runtime_disable(&pdev->dev);
> > > > ci_hdrc_remove_device(data->ci_pdev);
> > > >
> > > > - if (data->reg_vbus)
> > > > - regulator_disable(data->reg_vbus);
> > > > -
> > > > if (data->phy) {
> > > > usb_phy_shutdown(data->phy);
> > > > module_put(data->phy->dev->driver->owner);
> > > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > > index a5df24c..178b61d 100644
> > > > --- a/drivers/usb/chipidea/core.c
> > > > +++ b/drivers/usb/chipidea/core.c
> > > > @@ -57,6 +57,7 @@
> > > > #include <linux/interrupt.h>
> > > > #include <linux/io.h>
> > > > #include <linux/kernel.h>
> > > > +#include <linux/regulator/consumer.h>
> > > > #include <linux/slab.h>
> > > > #include <linux/pm_runtime.h>
> > > > #include <linux/usb/ch9.h>
> > > > @@ -363,6 +364,21 @@ struct platform_device *ci_hdrc_add_device(struct device *dev,
> > > > goto put_id;
> > > > }
> > > >
> > > > + /* Get the vbus regulator */
> > > > + platdata->reg_vbus = devm_regulator_get(dev, "vbus");
> > > > + if (PTR_ERR(platdata->reg_vbus) == -EPROBE_DEFER) {
> > > > + ret = -EPROBE_DEFER;
> > > > + goto err;
> > > > + } else if (PTR_ERR(platdata->reg_vbus) == -ENODEV) {
> > > > + platdata->reg_vbus = NULL; /* no vbus regualator is needed */
> > > > + } else if (IS_ERR(platdata->reg_vbus)) {
> > > > + dev_err(&pdev->dev,
> > > > + "Getting regulator error: %ld\n",
> > > > + PTR_ERR(platdata->reg_vbus));
> > > > + ret = PTR_ERR(platdata->reg_vbus);
> > > > + goto err;
> > > > + }
> > > > +
> > > > pdev->dev.parent = dev;
> > >
> > > Michael, thanks to do this for me.
> > >
> > > One small suggestion is: it is better add one function like:
> > > ci_get_platdata, and move the vbus stuff to that function.
> > > This function is used to get common thing from glue layer.
> > > And it is better at the beginning of ci_hdrc_add_device,
> > > since the other code at ci_hdrc_add_device are all related to
> > > core device.
> > >
> > > I can help to do refine this patch (with some typo which Felipe mentioned)
> > > if you consider it is necessary. I know you may be busy with other things.
> >
> > Feel free to take this patch over! It's mainly your code anyway.
> > And consider splitting your last series into main topics like
> > vbus, otg, and so forth.
> >
>
> You mean do not put my last patch [14/14] at this vbus/otg patchset?
> Yes, it is a bug fix, and found by adding vbus/otg patchset.
Yes, if the fixup has no dependency to hunks your own series
introduce, then there is no need to have it in that series.
In the second case, you would repair code which another
patch of your series wrongly introduce. In that case
that patch has no need to exist anyway.
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
