| Message ID | 1411468088-5702-8-git-send-email-antoine.tenart@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday 23 September 2014 12:28:03 Antoine Tenart wrote: > + if (dev->of_node) { > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > + if (ret) > + goto clk_err; > + } else { > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > + if (ret) > + goto clk_err; > + } > Why do you care about the non-DT case here? I think it would be nicer to open-code the ci_hdrc_usb2_dt_probe() function in here and remove the dma_set_mask_and_coherent(), which should not even be necessary for the case where you have a hardwired platform device. Arnd
Arnd, On Tue, Sep 23, 2014 at 12:39:04PM +0200, Arnd Bergmann wrote: > On Tuesday 23 September 2014 12:28:03 Antoine Tenart wrote: > > + if (dev->of_node) { > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > + if (ret) > > + goto clk_err; > > + } else { > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > + if (ret) > > + goto clk_err; > > + } > > > > Why do you care about the non-DT case here? I think it would be nicer to > open-code the ci_hdrc_usb2_dt_probe() function in here and remove > the dma_set_mask_and_coherent(), which should not even be necessary for > the case where you have a hardwired platform device. > I thought we agreed to call dma_set_mask_and_coherent(): http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273335.html I do not have a strong opinion on this as I only use the dt case for my usage. Antoine
On Tuesday 23 September 2014 15:36:45 Antoine Tenart wrote: > On Tue, Sep 23, 2014 at 12:39:04PM +0200, Arnd Bergmann wrote: > > On Tuesday 23 September 2014 12:28:03 Antoine Tenart wrote: > > > + if (dev->of_node) { > > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > > + if (ret) > > > + goto clk_err; > > > + } else { > > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > > + if (ret) > > > + goto clk_err; > > > + } > > > > > > > Why do you care about the non-DT case here? I think it would be nicer to > > open-code the ci_hdrc_usb2_dt_probe() function in here and remove > > the dma_set_mask_and_coherent(), which should not even be necessary for > > the case where you have a hardwired platform device. > > > > I thought we agreed to call dma_set_mask_and_coherent(): > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273335.html > > I do not have a strong opinion on this as I only use the dt case for my > usage. The question is more about who actually wants the non-DT case. Since this is a new driver, I suspect that the answer is "nobody", as the existing board files are all for legacy platforms that we are not going to adapt for this driver. I see in the thread that at least Peter Chen was assuming the non-DT case was still needed, but I can't find a reason for this in the code. If we no longer care about that, the call to dev_get_platdata() can also get removed. Looking through the code some more, I also notice that it's using a strange way of doing the abstraction: ci_hdrc_add_device() actually creates a child device node, while the preferred way would be to just call into ci_hdrc_probe(), or a generalized version of that. That should probably be changed, but can be done as a later cleanup. Arnd
HI, On Tue, Sep 23, 2014 at 06:44:40PM +0200, Arnd Bergmann wrote: > On Tuesday 23 September 2014 15:36:45 Antoine Tenart wrote: > > On Tue, Sep 23, 2014 at 12:39:04PM +0200, Arnd Bergmann wrote: > > > On Tuesday 23 September 2014 12:28:03 Antoine Tenart wrote: > > > > + if (dev->of_node) { > > > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > > > + if (ret) > > > > + goto clk_err; > > > > + } else { > > > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > > > + if (ret) > > > > + goto clk_err; > > > > + } > > > > > > > > > > Why do you care about the non-DT case here? I think it would be nicer to > > > open-code the ci_hdrc_usb2_dt_probe() function in here and remove > > > the dma_set_mask_and_coherent(), which should not even be necessary for > > > the case where you have a hardwired platform device. > > > > > > > I thought we agreed to call dma_set_mask_and_coherent(): > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/273335.html > > > > I do not have a strong opinion on this as I only use the dt case for my > > usage. > > The question is more about who actually wants the non-DT case. > > Since this is a new driver, I suspect that the answer is "nobody", > as the existing board files are all for legacy platforms that we > are not going to adapt for this driver. wait a minute... will the legacy platforms be adapted to DT and, thus, to this driver in the future ? I really don't want to keep several copies of chipidea driver just because there are still some legacy platforms still using them. I have said in the past and will say again, everybody should move to the generic chipidea driver at the earliest opportunity so we avoid duplication of work.
On Tue, 2014-09-23 at 12:28PM +0200, Antoine Tenart wrote: > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock > and DMA mask, to support USB2 ChipIdea controllers that don't need > specific functions. > > Tested on the Marvell Berlin SoCs USB controllers. > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> This driver also works for Zynq. I didn't do extensive testing, but I could access a flash drive successfully. Tested-by: Soren Brinkmann <soren.brinkmann@xilinx.com> Thanks, Sören
On Tue, Sep 23, 2014 at 12:28:03PM +0200, Antoine Tenart wrote: > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock > and DMA mask, to support USB2 ChipIdea controllers that don't need > specific functions. > > Tested on the Marvell Berlin SoCs USB controllers. > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > --- > drivers/usb/chipidea/Makefile | 1 + > drivers/usb/chipidea/ci_hdrc_usb2.c | 138 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 139 insertions(+) > create mode 100644 drivers/usb/chipidea/ci_hdrc_usb2.c > > diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile > index 2f099c7df7b5..1fc86a2ca22d 100644 > --- a/drivers/usb/chipidea/Makefile > +++ b/drivers/usb/chipidea/Makefile > @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_OTG_FSM) += otg_fsm.o > > # Glue/Bridge layers go here > > +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_usb2.o > obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_msm.o > obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o > > diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c > new file mode 100644 > index 000000000000..6eae1de587f2 > --- /dev/null > +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c > @@ -0,0 +1,138 @@ > +/* > + * Copyright (C) 2014 Marvell Technology Group Ltd. > + * > + * Antoine Tenart <antoine.tenart@free-electrons.com> > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include <linux/clk.h> > +#include <linux/dma-mapping.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > +#include <linux/usb/chipidea.h> > +#include <linux/usb/hcd.h> > +#include <linux/usb/ulpi.h> > + > +#include "ci.h" > + > +struct ci_hdrc_usb2_priv { > + struct platform_device *ci_pdev; > + struct clk *clk; > +}; > + > +static int ci_hdrc_usb2_dt_probe(struct device *dev, > + struct ci_hdrc_platform_data *ci_pdata) > +{ > + ci_pdata->phy = of_phy_get(dev->of_node, 0); > + if (IS_ERR(ci_pdata->phy)) { > + if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + /* PHY is optional */ > + ci_pdata->phy = NULL; > + } > + > + return 0; > +} > + > +static struct ci_hdrc_platform_data ci_default_pdata = { > + .capoffset = DEF_CAPOFFSET, > + .flags = CI_HDRC_REQUIRE_TRANSCEIVER | > + CI_HDRC_DISABLE_STREAMING, > +}; > + > +static int ci_hdrc_usb2_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct ci_hdrc_usb2_priv *priv; > + struct ci_hdrc_platform_data *ci_pdata = dev_get_platdata(&pdev->dev); > + int ret; > + > + if (!ci_pdata) > + ci_pdata = &ci_default_pdata; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->clk = devm_clk_get(dev, NULL); > + if (!IS_ERR(priv->clk)) { > + ret = clk_prepare_enable(priv->clk); > + if (ret) { > + dev_err(dev, "failed to enable the clock: %d\n", ret); > + return ret; > + } > + } > + > + if (dev->of_node) { > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > + if (ret) > + goto clk_err; > + } else { > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > + if (ret) > + goto clk_err; > + } Hi Antoine, the above code may not be needed, since get phy and set dma mask are common operation, we can do it at core code, the only thing you need to do is something like: dev->dma_mask = DMA_BIT_MASK(32). I will check your generic phy support for chipidea driver later, if it is ok to apply, you can have a patchset to move get phy operation to common code prior to this one (If you have no time, I can do it). Peter > + > + ci_pdata->name = dev_name(&pdev->dev); > + > + priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource, > + pdev->num_resources, ci_pdata); > + if (IS_ERR(priv->ci_pdev)) { > + ret = PTR_ERR(priv->ci_pdev); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, > + "failed to register ci_hdrc platform device: %d\n", > + ret); > + goto clk_err; > + } > + > + platform_set_drvdata(pdev, priv); > + > + pm_runtime_no_callbacks(dev); > + pm_runtime_enable(dev); > + > + return 0; > + > +clk_err: > + if (!IS_ERR(priv->clk)) > + clk_disable_unprepare(priv->clk); > + return ret; > +} > + > +static int ci_hdrc_usb2_remove(struct platform_device *pdev) > +{ > + struct ci_hdrc_usb2_priv *priv = platform_get_drvdata(pdev); > + > + pm_runtime_disable(&pdev->dev); > + ci_hdrc_remove_device(priv->ci_pdev); > + clk_disable_unprepare(priv->clk); > + > + return 0; > +} > + > +static const struct of_device_id ci_hdrc_usb2_of_match[] = { > + { .compatible = "chipidea,usb2" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match); > + > +static struct platform_driver ci_hdrc_usb2_driver = { > + .probe = ci_hdrc_usb2_probe, > + .remove = ci_hdrc_usb2_remove, > + .driver = { > + .name = "chipidea-usb2", > + .owner = THIS_MODULE, > + .of_match_table = of_match_ptr(ci_hdrc_usb2_of_match), > + }, > +}; > +module_platform_driver(ci_hdrc_usb2_driver); > + > +MODULE_DESCRIPTION("ChipIdea HDRC USB2 binding for ci13xxx"); > +MODULE_AUTHOR("Antoine Tenart <antoine.tenart@free-electrons.com>"); > +MODULE_LICENSE("GPL"); > -- > 1.9.1 >
On Thursday 25 September 2014 09:16:48 Peter Chen wrote: > > + } > > + > > + if (dev->of_node) { > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > + if (ret) > > + goto clk_err; > > + } else { > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > + if (ret) > > + goto clk_err; > > + } > > Hi Antoine, the above code may not be needed, since get phy and set dma > mask are common operation, we can do it at core code, the only thing you > need to do is something like: dev->dma_mask = DMA_BIT_MASK(32). > Certainly not, doing that would be broken for a number of reasons: - dev->dma_mask is a pointer, a driver should not reassign that - the device may have been set up for a bus that requires a smaller mask that is already set, changing the mask would cause data corruption - setting just the dma mask but not coherent mask is wrong - setting the dma mask can fail, e.g. if the mask is smaller than the smallest memory zone, so you have to check the return value. Arnd
On Tuesday 23 September 2014, Antoine Tenart wrote: > +static int ci_hdrc_usb2_dt_probe(struct device *dev, > + struct ci_hdrc_platform_data *ci_pdata) > +{ > + ci_pdata->phy = of_phy_get(dev->of_node, 0); FWIW, I accidentally built a kernel with this driver enabled and got a warning for this code. The problem is that ci_pdata->phy is a 'struct usb_phy' pointer, while of_phy_get() returns a generic 'struct phy'. While the two have similar behavior, they are not the same thing and this can't work. Arnd
Arnd, On Thu, Sep 25, 2014 at 09:12:07PM +0200, Arnd Bergmann wrote: > On Tuesday 23 September 2014, Antoine Tenart wrote: > > +static int ci_hdrc_usb2_dt_probe(struct device *dev, > > + struct ci_hdrc_platform_data *ci_pdata) > > +{ > > + ci_pdata->phy = of_phy_get(dev->of_node, 0); > > FWIW, I accidentally built a kernel with this driver enabled and got a warning > for this code. The problem is that ci_pdata->phy is a 'struct usb_phy' pointer, > while of_phy_get() returns a generic 'struct phy'. While the two have similar > behavior, they are not the same thing and this can't work. That's because this series applies on top of: https://lkml.org/lkml/2014/9/15/141 Antoine
On Thu, Sep 25, 2014 at 09:11:35AM +0200, Arnd Bergmann wrote: > On Thursday 25 September 2014 09:16:48 Peter Chen wrote: > > > + } > > > + > > > + if (dev->of_node) { > > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > > + if (ret) > > > + goto clk_err; > > > + } else { > > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > > + if (ret) > > > + goto clk_err; > > > + } > > > > Hi Antoine, the above code may not be needed, since get phy and set dma > > mask are common operation, we can do it at core code, the only thing you > > need to do is something like: dev->dma_mask = DMA_BIT_MASK(32). > > > > Certainly not, doing that would be broken for a number of reasons: > > - dev->dma_mask is a pointer, a driver should not reassign that > - the device may have been set up for a bus that requires a smaller mask > that is already set, changing the mask would cause data corruption > - setting just the dma mask but not coherent mask is wrong > - setting the dma mask can fail, e.g. if the mask is smaller than the > smallest memory zone, so you have to check the return value. > In current chipidea structure, the parent (glue layer) driver will not be used for dma, udc/host driver uses dma mask from child (core layer), at core layer we will do: pdev->dev.dma_mask = dev->dma_mask; /* this device is parent */ dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask); Would you suggestion us a suitable way? Or it is ok we use just Antoine's way that call dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) at parent driver no matter dt or non-dt? Thanks.
On Friday 26 September 2014 08:23:40 Peter Chen wrote: > In current chipidea structure, the parent (glue layer) driver will not be > used for dma, udc/host driver uses dma mask from child (core layer), at core > layer we will do: > > > pdev->dev.dma_mask = dev->dma_mask; /* this device is parent */ > dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask); > > Would you suggestion us a suitable way? Or it is ok we use just Antoine's way that > call dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) at parent > driver no matter dt or non-dt? Thanks. dma_coerce_mask_and_coherent is not ok, it will force a dma mask that is unrelated to the actual requirements of the hardware. I think the best way would be to never use the child device pointer for DMA operations, just use a pointer to the parent device and make the child dev->dma_mask pointer NULL to ensure all DMA operations fail. Arnd
On Wed, Sep 24, 2014 at 04:58:14PM -0700, Sören Brinkmann wrote: > On Tue, 2014-09-23 at 12:28PM +0200, Antoine Tenart wrote: > > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock > > and DMA mask, to support USB2 ChipIdea controllers that don't need > > specific functions. > > > > Tested on the Marvell Berlin SoCs USB controllers. > > > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > > This driver also works for Zynq. I didn't do extensive testing, but I > could access a flash drive successfully. > > Tested-by: Soren Brinkmann <soren.brinkmann@xilinx.com> Thanks! Antoine
Peter, Arnd, Felipe, On Tue, Sep 23, 2014 at 12:28:03PM +0200, Antoine Tenart wrote: > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock > and DMA mask, to support USB2 ChipIdea controllers that don't need > specific functions. Did we agree on the modifications needed to get this accepted? If so, can one of you sum up what's need to be done, I got a bit lost reading all the discussions on this thread. Anyway, that would be good to get this series accepted as the Berlin USB support depends on it. This series has been out since the beginning of June and non-critical changes can be added by future series. Thanks, Antoine
On Tue, Sep 23, 2014 at 12:28:03PM +0200, Antoine Tenart wrote: > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock > and DMA mask, to support USB2 ChipIdea controllers that don't need > specific functions. > > Tested on the Marvell Berlin SoCs USB controllers. > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > --- > drivers/usb/chipidea/Makefile | 1 + > drivers/usb/chipidea/ci_hdrc_usb2.c | 138 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 139 insertions(+) > create mode 100644 drivers/usb/chipidea/ci_hdrc_usb2.c > > diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile > index 2f099c7df7b5..1fc86a2ca22d 100644 > --- a/drivers/usb/chipidea/Makefile > +++ b/drivers/usb/chipidea/Makefile > @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_OTG_FSM) += otg_fsm.o > > # Glue/Bridge layers go here > > +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_usb2.o > obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_msm.o > obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o > > diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c > new file mode 100644 > index 000000000000..6eae1de587f2 > --- /dev/null > +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c > @@ -0,0 +1,138 @@ > +/* > + * Copyright (C) 2014 Marvell Technology Group Ltd. > + * > + * Antoine Tenart <antoine.tenart@free-electrons.com> > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include <linux/clk.h> > +#include <linux/dma-mapping.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > +#include <linux/usb/chipidea.h> > +#include <linux/usb/hcd.h> > +#include <linux/usb/ulpi.h> > + > +#include "ci.h" > + > +struct ci_hdrc_usb2_priv { > + struct platform_device *ci_pdev; > + struct clk *clk; > +}; > + > +static int ci_hdrc_usb2_dt_probe(struct device *dev, > + struct ci_hdrc_platform_data *ci_pdata) > +{ > + ci_pdata->phy = of_phy_get(dev->of_node, 0); > + if (IS_ERR(ci_pdata->phy)) { > + if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + /* PHY is optional */ > + ci_pdata->phy = NULL; > + } > + > + return 0; > +} > + > +static struct ci_hdrc_platform_data ci_default_pdata = { > + .capoffset = DEF_CAPOFFSET, > + .flags = CI_HDRC_REQUIRE_TRANSCEIVER | > + CI_HDRC_DISABLE_STREAMING, > +}; > + > +static int ci_hdrc_usb2_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct ci_hdrc_usb2_priv *priv; > + struct ci_hdrc_platform_data *ci_pdata = dev_get_platdata(&pdev->dev); > + int ret; > + > + if (!ci_pdata) > + ci_pdata = &ci_default_pdata; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->clk = devm_clk_get(dev, NULL); > + if (!IS_ERR(priv->clk)) { > + ret = clk_prepare_enable(priv->clk); > + if (ret) { > + dev_err(dev, "failed to enable the clock: %d\n", ret); > + return ret; > + } > + } > + > + if (dev->of_node) { > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > + if (ret) > + goto clk_err; > + } else { > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > + if (ret) > + goto clk_err; > + } My suggestion: - call dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32)) for both dt and non-dt - Do not need function ci_hdrc_usb2_dt_probe, the phy handle should be moved to core driver, since your generic phy driver are still not accepted, I can't do it by myself. Either you or I can do it after your generic phy support series has been accepted. - Others are ok. Peter > + > + ci_pdata->name = dev_name(&pdev->dev); > + > + priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource, > + pdev->num_resources, ci_pdata); > + if (IS_ERR(priv->ci_pdev)) { > + ret = PTR_ERR(priv->ci_pdev); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, > + "failed to register ci_hdrc platform device: %d\n", > + ret); > + goto clk_err; > + } > + > + platform_set_drvdata(pdev, priv); > + > + pm_runtime_no_callbacks(dev); > + pm_runtime_enable(dev); > + > + return 0; > + > +clk_err: > + if (!IS_ERR(priv->clk)) > + clk_disable_unprepare(priv->clk); > + return ret; > +} > + > +static int ci_hdrc_usb2_remove(struct platform_device *pdev) > +{ > + struct ci_hdrc_usb2_priv *priv = platform_get_drvdata(pdev); > + > + pm_runtime_disable(&pdev->dev); > + ci_hdrc_remove_device(priv->ci_pdev); > + clk_disable_unprepare(priv->clk); > + > + return 0; > +} > + > +static const struct of_device_id ci_hdrc_usb2_of_match[] = { > + { .compatible = "chipidea,usb2" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match); > + > +static struct platform_driver ci_hdrc_usb2_driver = { > + .probe = ci_hdrc_usb2_probe, > + .remove = ci_hdrc_usb2_remove, > + .driver = { > + .name = "chipidea-usb2", > + .owner = THIS_MODULE, > + .of_match_table = of_match_ptr(ci_hdrc_usb2_of_match), > + }, > +}; > +module_platform_driver(ci_hdrc_usb2_driver); > + > +MODULE_DESCRIPTION("ChipIdea HDRC USB2 binding for ci13xxx"); > +MODULE_AUTHOR("Antoine Tenart <antoine.tenart@free-electrons.com>"); > +MODULE_LICENSE("GPL"); > -- > 1.9.1 >
On Tuesday 30 September 2014 08:12:07 Peter Chen wrote: > > + > > + if (dev->of_node) { > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > + if (ret) > > + goto clk_err; > > + } else { > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > + if (ret) > > + goto clk_err; > > + } > > My suggestion: > > - call dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32)) for both > dt and non-dt No, as I explained before, hardcoding the dma mask is always wrong, don't do that. Call dma_set_mask_and_coherent and check the return value. It's not wrong to do that for both DT and ATAGS. Arnd
On Tue, Sep 30, 2014 at 12:03:42PM +0200, Arnd Bergmann wrote: > On Tuesday 30 September 2014 08:12:07 Peter Chen wrote: > > > + > > > + if (dev->of_node) { > > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > > + if (ret) > > > + goto clk_err; > > > + } else { > > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > > + if (ret) > > > + goto clk_err; > > > + } > > > > My suggestion: > > > > - call dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32)) for both > > dt and non-dt > > No, as I explained before, hardcoding the dma mask is always wrong, don't > do that. Call dma_set_mask_and_coherent and check the return value. > It's not wrong to do that for both DT and ATAGS. > Thanks, Arnd. I had not thought setting dma mask is so complicated, yes, it should check the return value, two things to confirm: - dma_coerce_mask_and_coherent or dma_set_mask_and_coherent, the only difference of these two API is the first one do "dev->dma_mask = &dev->coherent_dma_mask;" The reason you suggest choosing dma_set_mask_and_coherent is you do not want assign dev->dma_mask? - The second parameter for dma_set_mask_and_coherent is DMA_BIT_MASK(32), is it ok? I just a little confused of what's the operation is "hardcoding the dma mask"?
On Tuesday 30 September 2014 20:39:34 Peter Chen wrote: > Thanks, Arnd. I had not thought setting dma mask is so complicated, yes, it > should check the return value, two things to confirm: > > - dma_coerce_mask_and_coherent or dma_set_mask_and_coherent, the only difference > of these two API is the first one do "dev->dma_mask = &dev->coherent_dma_mask;" > The reason you suggest choosing dma_set_mask_and_coherent is you do not want > assign dev->dma_mask? No, that is just the current definition on ARM32 with CONFIG_ARCH_MULTIPLATFORM, and that is going to change soon to be DT aware. dma_set_mask_and_coherent() is supposed to check whether the platform can support the respective mask and return an error if it cannot. > - The second parameter for dma_set_mask_and_coherent is DMA_BIT_MASK(32), is it > ok? > > I just a little confused of what's the operation is "hardcoding the dma mask"? dma_coerce_mask_and_coherent() will hardcode the dma mask and override whatever the platform says is necessary. Arnd
> Subject: Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx > > On Tuesday 30 September 2014 20:39:34 Peter Chen wrote: > > Thanks, Arnd. I had not thought setting dma mask is so complicated, > > yes, it should check the return value, two things to confirm: > > > > - dma_coerce_mask_and_coherent or dma_set_mask_and_coherent, the > only > > difference of these two API is the first one do "dev->dma_mask = &dev- > >coherent_dma_mask;" > > The reason you suggest choosing dma_set_mask_and_coherent is you do > > not want assign dev->dma_mask? > > No, that is just the current definition on ARM32 with > CONFIG_ARCH_MULTIPLATFORM, and that is going to change soon to be DT > aware. > dma_set_mask_and_coherent() is supposed to check whether the platform > can support the respective mask and return an error if it cannot. > > > - The second parameter for dma_set_mask_and_coherent is > > DMA_BIT_MASK(32), is it ok? > > > > I just a little confused of what's the operation is "hardcoding the dma mask"? > > dma_coerce_mask_and_coherent() will hardcode the dma mask and override > whatever the platform says is necessary. > So, we should use dma_set_mask_and_coherent() in most of cases in device driver, and use dma_coerce_mask_and_coherent() only when the device's dma_mask is wrong? Peter
On Wednesday 01 October 2014 06:35:58 Peter Chen wrote: > > Subject: Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx > > > > On Tuesday 30 September 2014 20:39:34 Peter Chen wrote: > > > Thanks, Arnd. I had not thought setting dma mask is so complicated, > > > yes, it should check the return value, two things to confirm: > > > > > > - dma_coerce_mask_and_coherent or dma_set_mask_and_coherent, the > > only > > > difference of these two API is the first one do "dev->dma_mask = &dev- > > >coherent_dma_mask;" > > > The reason you suggest choosing dma_set_mask_and_coherent is you do > > > not want assign dev->dma_mask? > > > > No, that is just the current definition on ARM32 with > > CONFIG_ARCH_MULTIPLATFORM, and that is going to change soon to be DT > > aware. > > dma_set_mask_and_coherent() is supposed to check whether the platform > > can support the respective mask and return an error if it cannot. > > > > > - The second parameter for dma_set_mask_and_coherent is > > > DMA_BIT_MASK(32), is it ok? > > > > > > I just a little confused of what's the operation is "hardcoding the dma mask"? > > > > dma_coerce_mask_and_coherent() will hardcode the dma mask and override > > whatever the platform says is necessary. > > > > So, we should use dma_set_mask_and_coherent() in most of cases in device driver, > and use dma_coerce_mask_and_coherent() only when the device's dma_mask is wrong? > > dma_coerce_mask_and_coherent() should really not be used at all. Russell introduced it to convert drivers that were incorrectly setting the dma_mask pointer themselves to something slightly more palatable. The initial dma_mask is supposed to be set by the platform for each DMA capable device, according to the address width of its upstream bus. For DT based probing, we now set it to 32-bit mask but should really set it to something smaller if the bus is more limited than that. For devices created by platform code (board files), the platform should call platform_device_register_full() and specify the dma mask in the platform_device_info. Older platforms sometimes define a static platform device structure that has the dma mask set. This works as well but is discouraged for other reasons. Drivers that require a dma mask that is either smaller than 32-bit (because of device specific quirks) or that know that the device is capable of larger DMA space and want to use that need to call dma_set_mask or dma_set_mask_and_coherent and check the return value. Ideally all device drivers that want to do DMA should call set_dma_mask or set_dma_mask_and_coherent, even if the device has exactly a 32-bit connection to the upstream bus. One more complication: if the device is not a DMA master itself but uses a dma engine, you should not use set_dma_mask_and_coherent() but instead use dma_get_mask() on the dmaengine device to query its mask, and also use that same pointer to pass into dma_alloc_coherent and dma_map_*. Arnd
> On Wednesday 01 October 2014 06:35:58 Peter Chen wrote: > > > Subject: Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for > > > ci13xxx > > > > > > On Tuesday 30 September 2014 20:39:34 Peter Chen wrote: > > > > Thanks, Arnd. I had not thought setting dma mask is so > > > > complicated, yes, it should check the return value, two things to confirm: > > > > > > > > - dma_coerce_mask_and_coherent or dma_set_mask_and_coherent, > the > > > only > > > > difference of these two API is the first one do "dev->dma_mask = > > > >&dev- coherent_dma_mask;" > > > > The reason you suggest choosing dma_set_mask_and_coherent is you > > > >do not want assign dev->dma_mask? > > > > > > No, that is just the current definition on ARM32 with > > > CONFIG_ARCH_MULTIPLATFORM, and that is going to change soon to be > DT > > > aware. > > > dma_set_mask_and_coherent() is supposed to check whether the > > > platform can support the respective mask and return an error if it cannot. > > > > > > > - The second parameter for dma_set_mask_and_coherent is > > > > DMA_BIT_MASK(32), is it ok? > > > > > > > > I just a little confused of what's the operation is "hardcoding the dma > mask"? > > > > > > dma_coerce_mask_and_coherent() will hardcode the dma mask and > > > override whatever the platform says is necessary. > > > > > > > So, we should use dma_set_mask_and_coherent() in most of cases in > > device driver, and use dma_coerce_mask_and_coherent() only when the > device's dma_mask is wrong? > > > > > > dma_coerce_mask_and_coherent() should really not be used at all. Russell > introduced it to convert drivers that were incorrectly setting the dma_mask > pointer themselves to something slightly more palatable. > > The initial dma_mask is supposed to be set by the platform for each DMA > capable device, according to the address width of its upstream bus. > For DT based probing, we now set it to 32-bit mask but should really set it to > something smaller if the bus is more limited than that. > > For devices created by platform code (board files), the platform should call > platform_device_register_full() and specify the dma mask in the > platform_device_info. Older platforms sometimes define a static platform > device structure that has the dma mask set. This works as well but is > discouraged for other reasons. > > Drivers that require a dma mask that is either smaller than 32-bit (because of > device specific quirks) or that know that the device is capable of larger DMA > space and want to use that need to call dma_set_mask or > dma_set_mask_and_coherent and check the return value. > > Ideally all device drivers that want to do DMA should call set_dma_mask or > set_dma_mask_and_coherent, even if the device has exactly a 32-bit > connection to the upstream bus. > > One more complication: if the device is not a DMA master itself but uses a dma > engine, you should not use set_dma_mask_and_coherent() but instead use > dma_get_mask() on the dmaengine device to query its mask, and also use that > same pointer to pass into dma_alloc_coherent and dma_map_*. > Thank you very much, Arnd. It makes things clear. Peter
Peter, Arnd, Felipe, On Mon, Sep 29, 2014 at 05:08:37PM +0200, Antoine Tenart wrote: > On Tue, Sep 23, 2014 at 12:28:03PM +0200, Antoine Tenart wrote: > > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock > > and DMA mask, to support USB2 ChipIdea controllers that don't need > > specific functions. > > Did we agree on the modifications needed to get this accepted? If so, > can one of you sum up what's need to be done, I got a bit lost reading > all the discussions on this thread. > > Anyway, that would be good to get this series accepted as the Berlin USB > support depends on it. This series has been out since the beginning of > June and non-critical changes can be added by future series. I'm seriously considering sending a new version with dt-only support as I don't need the other case. There are still many discussions between the three of you regarding DMA support, which is not a problem for my use case. As this is not a modification of an existing driver, future additions can be done to support more cases later and we do not need this driver to handle all possible cases right now as this won't be a regression. Please, can one of you sum up the modifications needed to get this accepted, if any. If the dt part is fine for you, I'll resend this series with dt-only support. The platform compatibility can surely be added later, when needed by someone, and when an agreement on how to do it is reached. I'm afraid this series is currently blocked by implementation related discussions not linked with the USB Berlin support, while this could be done as a follow up series. Thanks, Antoine
On Tue, Sep 30, 2014 at 08:12:07AM +0800, Peter Chen wrote: > On Tue, Sep 23, 2014 at 12:28:03PM +0200, Antoine Tenart wrote: > > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock > > and DMA mask, to support USB2 ChipIdea controllers that don't need > > specific functions. > > > > Tested on the Marvell Berlin SoCs USB controllers. > > > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > > --- > > drivers/usb/chipidea/Makefile | 1 + > > drivers/usb/chipidea/ci_hdrc_usb2.c | 138 ++++++++++++++++++++++++++++++++++++ > > 2 files changed, 139 insertions(+) > > create mode 100644 drivers/usb/chipidea/ci_hdrc_usb2.c > > > > diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile > > index 2f099c7df7b5..1fc86a2ca22d 100644 > > --- a/drivers/usb/chipidea/Makefile > > +++ b/drivers/usb/chipidea/Makefile > > @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_OTG_FSM) += otg_fsm.o > > > > # Glue/Bridge layers go here > > > > +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_usb2.o > > obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_msm.o > > obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c > > new file mode 100644 > > index 000000000000..6eae1de587f2 > > --- /dev/null > > +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c > > @@ -0,0 +1,138 @@ > > +/* > > + * Copyright (C) 2014 Marvell Technology Group Ltd. > > + * > > + * Antoine Tenart <antoine.tenart@free-electrons.com> > > + * > > + * This file is licensed under the terms of the GNU General Public > > + * License version 2. This program is licensed "as is" without any > > + * warranty of any kind, whether express or implied. > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/dma-mapping.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/phy/phy.h> > > +#include <linux/platform_device.h> > > +#include <linux/usb/chipidea.h> > > +#include <linux/usb/hcd.h> > > +#include <linux/usb/ulpi.h> > > + > > +#include "ci.h" > > + > > +struct ci_hdrc_usb2_priv { > > + struct platform_device *ci_pdev; > > + struct clk *clk; > > +}; > > + > > +static int ci_hdrc_usb2_dt_probe(struct device *dev, > > + struct ci_hdrc_platform_data *ci_pdata) > > +{ > > + ci_pdata->phy = of_phy_get(dev->of_node, 0); > > + if (IS_ERR(ci_pdata->phy)) { > > + if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > + > > + /* PHY is optional */ > > + ci_pdata->phy = NULL; > > + } > > + > > + return 0; > > +} > > + > > +static struct ci_hdrc_platform_data ci_default_pdata = { > > + .capoffset = DEF_CAPOFFSET, > > + .flags = CI_HDRC_REQUIRE_TRANSCEIVER | > > + CI_HDRC_DISABLE_STREAMING, > > +}; > > + > > +static int ci_hdrc_usb2_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct ci_hdrc_usb2_priv *priv; > > + struct ci_hdrc_platform_data *ci_pdata = dev_get_platdata(&pdev->dev); > > + int ret; > > + > > + if (!ci_pdata) > > + ci_pdata = &ci_default_pdata; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->clk = devm_clk_get(dev, NULL); > > + if (!IS_ERR(priv->clk)) { > > + ret = clk_prepare_enable(priv->clk); > > + if (ret) { > > + dev_err(dev, "failed to enable the clock: %d\n", ret); > > + return ret; > > + } > > + } > > + > > + if (dev->of_node) { > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); > > + if (ret) > > + goto clk_err; > > + } else { > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > + if (ret) > > + goto clk_err; > > + } > Hi Antoine, if you pay attention the discussion for this patch, you should know to do change for your next revision - Call below unconditionally: > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > + if (ret) > > + goto clk_err; -Do not need function ci_hdrc_usb2_dt_probe, we need to cover both dt and non-dt, but there is nothing we can to differentiate now, The phy handle should be moved to core driver, since your generic phy driver are still not accepted, I can't do it by myself. Either you or I can do it after your generic phy support series has been accepted. > > + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); - Others are ok - The sequence of patch sets: 1. The generic PHY support for chipidea idea 2. Move getting PHY from individual glue layer to core driver 3. The generic chipidea driver Peter > > + > > + ci_pdata->name = dev_name(&pdev->dev); > > + > > + priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource, > > + pdev->num_resources, ci_pdata); > > + if (IS_ERR(priv->ci_pdev)) { > > + ret = PTR_ERR(priv->ci_pdev); > > + if (ret != -EPROBE_DEFER) > > + dev_err(dev, > > + "failed to register ci_hdrc platform device: %d\n", > > + ret); > > + goto clk_err; > > + } > > + > > + platform_set_drvdata(pdev, priv); > > + > > + pm_runtime_no_callbacks(dev); > > + pm_runtime_enable(dev); > > + > > + return 0; > > + > > +clk_err: > > + if (!IS_ERR(priv->clk)) > > + clk_disable_unprepare(priv->clk); > > + return ret; > > +} > > + > > +static int ci_hdrc_usb2_remove(struct platform_device *pdev) > > +{ > > + struct ci_hdrc_usb2_priv *priv = platform_get_drvdata(pdev); > > + > > + pm_runtime_disable(&pdev->dev); > > + ci_hdrc_remove_device(priv->ci_pdev); > > + clk_disable_unprepare(priv->clk); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id ci_hdrc_usb2_of_match[] = { > > + { .compatible = "chipidea,usb2" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match); > > + > > +static struct platform_driver ci_hdrc_usb2_driver = { > > + .probe = ci_hdrc_usb2_probe, > > + .remove = ci_hdrc_usb2_remove, > > + .driver = { > > + .name = "chipidea-usb2", > > + .owner = THIS_MODULE, > > + .of_match_table = of_match_ptr(ci_hdrc_usb2_of_match), > > + }, > > +}; > > +module_platform_driver(ci_hdrc_usb2_driver); > > + > > +MODULE_DESCRIPTION("ChipIdea HDRC USB2 binding for ci13xxx"); > > +MODULE_AUTHOR("Antoine Tenart <antoine.tenart@free-electrons.com>"); > > +MODULE_LICENSE("GPL"); > > -- > > 1.9.1 > > > > -- > Best Regards, > Peter Chen > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile index 2f099c7df7b5..1fc86a2ca22d 100644 --- a/drivers/usb/chipidea/Makefile +++ b/drivers/usb/chipidea/Makefile @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_OTG_FSM) += otg_fsm.o # Glue/Bridge layers go here +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_usb2.o obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_msm.o obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c new file mode 100644 index 000000000000..6eae1de587f2 --- /dev/null +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c @@ -0,0 +1,138 @@ +/* + * Copyright (C) 2014 Marvell Technology Group Ltd. + * + * Antoine Tenart <antoine.tenart@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/clk.h> +#include <linux/dma-mapping.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/usb/chipidea.h> +#include <linux/usb/hcd.h> +#include <linux/usb/ulpi.h> + +#include "ci.h" + +struct ci_hdrc_usb2_priv { + struct platform_device *ci_pdev; + struct clk *clk; +}; + +static int ci_hdrc_usb2_dt_probe(struct device *dev, + struct ci_hdrc_platform_data *ci_pdata) +{ + ci_pdata->phy = of_phy_get(dev->of_node, 0); + if (IS_ERR(ci_pdata->phy)) { + if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + /* PHY is optional */ + ci_pdata->phy = NULL; + } + + return 0; +} + +static struct ci_hdrc_platform_data ci_default_pdata = { + .capoffset = DEF_CAPOFFSET, + .flags = CI_HDRC_REQUIRE_TRANSCEIVER | + CI_HDRC_DISABLE_STREAMING, +}; + +static int ci_hdrc_usb2_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct ci_hdrc_usb2_priv *priv; + struct ci_hdrc_platform_data *ci_pdata = dev_get_platdata(&pdev->dev); + int ret; + + if (!ci_pdata) + ci_pdata = &ci_default_pdata; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->clk = devm_clk_get(dev, NULL); + if (!IS_ERR(priv->clk)) { + ret = clk_prepare_enable(priv->clk); + if (ret) { + dev_err(dev, "failed to enable the clock: %d\n", ret); + return ret; + } + } + + if (dev->of_node) { + ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata); + if (ret) + goto clk_err; + } else { + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); + if (ret) + goto clk_err; + } + + ci_pdata->name = dev_name(&pdev->dev); + + priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource, + pdev->num_resources, ci_pdata); + if (IS_ERR(priv->ci_pdev)) { + ret = PTR_ERR(priv->ci_pdev); + if (ret != -EPROBE_DEFER) + dev_err(dev, + "failed to register ci_hdrc platform device: %d\n", + ret); + goto clk_err; + } + + platform_set_drvdata(pdev, priv); + + pm_runtime_no_callbacks(dev); + pm_runtime_enable(dev); + + return 0; + +clk_err: + if (!IS_ERR(priv->clk)) + clk_disable_unprepare(priv->clk); + return ret; +} + +static int ci_hdrc_usb2_remove(struct platform_device *pdev) +{ + struct ci_hdrc_usb2_priv *priv = platform_get_drvdata(pdev); + + pm_runtime_disable(&pdev->dev); + ci_hdrc_remove_device(priv->ci_pdev); + clk_disable_unprepare(priv->clk); + + return 0; +} + +static const struct of_device_id ci_hdrc_usb2_of_match[] = { + { .compatible = "chipidea,usb2" }, + { } +}; +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match); + +static struct platform_driver ci_hdrc_usb2_driver = { + .probe = ci_hdrc_usb2_probe, + .remove = ci_hdrc_usb2_remove, + .driver = { + .name = "chipidea-usb2", + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(ci_hdrc_usb2_of_match), + }, +}; +module_platform_driver(ci_hdrc_usb2_driver); + +MODULE_DESCRIPTION("ChipIdea HDRC USB2 binding for ci13xxx"); +MODULE_AUTHOR("Antoine Tenart <antoine.tenart@free-electrons.com>"); +MODULE_LICENSE("GPL");
Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock and DMA mask, to support USB2 ChipIdea controllers that don't need specific functions. Tested on the Marvell Berlin SoCs USB controllers. Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> --- drivers/usb/chipidea/Makefile | 1 + drivers/usb/chipidea/ci_hdrc_usb2.c | 138 ++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 drivers/usb/chipidea/ci_hdrc_usb2.c