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. Best Regards, Peter Chen -- 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