Re: [PATCH v7 08/13] usb: chipidea: add a usb2 driver for ci13xxx

2014-11-17 Thread Antoine Tenart
Felipe,

On Fri, Nov 14, 2014 at 03:13:34PM -0600, Felipe Balbi wrote:
> On Fri, Nov 14, 2014 at 10:10:52PM +0100, Antoine Tenart wrote:
> > On Fri, Nov 14, 2014 at 03:08:32PM -0600, Felipe Balbi wrote:
> > > On Fri, Nov 14, 2014 at 04:25:58PM +0100, 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 
> > > > ---
> > > >  drivers/usb/chipidea/Makefile   |   1 +
> > > >  drivers/usb/chipidea/ci_hdrc_usb2.c | 117 
> > > > 
> > > >  2 files changed, 118 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
> > > 
> > > usb2 is too generic, you should call it ci_hdcr_marvell, or something
> > > like that.
> > 
> > This driver is generic, and not only for Marvell Berlin SoCs. It is part
> > of the Berlin USB series as it is the first SoC to use it but Xilinx can
> > also use it for example.
> 
> then how about ci_hdrc_generic.c ?

We had this discussion during in the v2 thread of this series[1]. I tend
to prefer not using the 'generic' keyword. I think using 'usb2' is
generic enough here.


Antoine

[1] https://lkml.org/lkml/2014/6/24/197

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 08/13] usb: chipidea: add a usb2 driver for ci13xxx

2014-11-16 Thread Peter Chen
On Fri, Nov 14, 2014 at 04:25:58PM +0100, 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 

I remember someone tested it before, you can add his tested-by if you
can find it.

> ---
>  drivers/usb/chipidea/Makefile   |   1 +
>  drivers/usb/chipidea/ci_hdrc_usb2.c | 117 
> 
>  2 files changed, 118 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 ..15a0947483b8
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Tenart 
> + *
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "ci.h"
> +
> +struct ci_hdrc_usb2_priv {
> + struct platform_device  *ci_pdev;
> + struct clk  *clk;
> +};
> +
> +static struct ci_hdrc_platform_data ci_default_pdata = {
> + .capoffset  = DEF_CAPOFFSET,
> + .flags  = CI_HDRC_REQUIRE_TRANSCEIVER |
> +   CI_HDRC_DISABLE_STREAMING,
> +};
> +

"CI_HDRC_REQUIRE_TRANSCEIVER" is deleted, have you rebased my next tree?

> +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);

You can dev instead of &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;
> + }
> + }
> +
> + 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 ");
> +MODULE_LICENSE("GPL");
> -- 
> 2.1.0
> 

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-u

Re: [PATCH v7 08/13] usb: chipidea: add a usb2 driver for ci13xxx

2014-11-14 Thread Felipe Balbi
On Fri, Nov 14, 2014 at 10:10:52PM +0100, Antoine Tenart wrote:
> Felipe,
> 
> On Fri, Nov 14, 2014 at 03:08:32PM -0600, Felipe Balbi wrote:
> > On Fri, Nov 14, 2014 at 04:25:58PM +0100, 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 
> > > ---
> > >  drivers/usb/chipidea/Makefile   |   1 +
> > >  drivers/usb/chipidea/ci_hdrc_usb2.c | 117 
> > > 
> > >  2 files changed, 118 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
> > 
> > usb2 is too generic, you should call it ci_hdcr_marvell, or something
> > like that.
> 
> This driver is generic, and not only for Marvell Berlin SoCs. It is part
> of the Berlin USB series as it is the first SoC to use it but Xilinx can
> also use it for example.

then how about ci_hdrc_generic.c ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v7 08/13] usb: chipidea: add a usb2 driver for ci13xxx

2014-11-14 Thread Antoine Tenart
Felipe,

On Fri, Nov 14, 2014 at 03:08:32PM -0600, Felipe Balbi wrote:
> On Fri, Nov 14, 2014 at 04:25:58PM +0100, 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 
> > ---
> >  drivers/usb/chipidea/Makefile   |   1 +
> >  drivers/usb/chipidea/ci_hdrc_usb2.c | 117 
> > 
> >  2 files changed, 118 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
> 
> usb2 is too generic, you should call it ci_hdcr_marvell, or something
> like that.

This driver is generic, and not only for Marvell Berlin SoCs. It is part
of the Berlin USB series as it is the first SoC to use it but Xilinx can
also use it for example.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 08/13] usb: chipidea: add a usb2 driver for ci13xxx

2014-11-14 Thread Felipe Balbi
On Fri, Nov 14, 2014 at 04:25:58PM +0100, 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 
> ---
>  drivers/usb/chipidea/Makefile   |   1 +
>  drivers/usb/chipidea/ci_hdrc_usb2.c | 117 
> 
>  2 files changed, 118 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

usb2 is too generic, you should call it ci_hdcr_marvell, or something
like that.

-- 
balbi


signature.asc
Description: Digital signature