[PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Nathan Sullivan]

The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
unlike the default platform data.  Add platform data specific to the
Zynq udc.

Based on a patch by the same name from the Xilinx vendor tree.

Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
---
 drivers/usb/chipidea/ci_hdrc_usb2.c |   25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
index 9eae1a1..4456d2c 100644
--- a/drivers/usb/chipidea/ci_hdrc_usb2.c
+++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
@@ -12,6 +12,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/usb/chipidea.h>
@@ -30,18 +31,36 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
 	.flags		= CI_HDRC_DISABLE_STREAMING,
 };
 
+static struct ci_hdrc_platform_data ci_zynq_pdata = {
+	.capoffset	= DEF_CAPOFFSET,
+};
+
+static const struct of_device_id ci_hdrc_usb2_of_match[] = {
+	{ .compatible = "chipidea,usb2"},
+	{ .compatible = "xlnx,zynq-usb-2.20a", .data = &ci_zynq_pdata},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
+
 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(dev);
 	int ret;
+	const struct of_device_id *match;
 
 	if (!ci_pdata) {
 		ci_pdata = devm_kmalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
 		*ci_pdata = ci_default_pdata;	/* struct copy */
 	}
 
+	match = of_match_device(ci_hdrc_usb2_of_match, &pdev->dev);
+	if (match && match->data) {
+		/* struct copy */
+		*ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
+	}
+
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -96,12 +115,6 @@ static int ci_hdrc_usb2_remove(struct platform_device *pdev)
 	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,
-- 
1.7.10.4

[PATCH 2/2] Documentation: bindings: add doc for zynq USB

[Nathan Sullivan]

Document the binding for the zynq specific chipidea UDC binding.

Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 553e2fa..29ec09e 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -6,6 +6,7 @@ Required properties:
 	"lsi,zevio-usb"
 	"qcom,ci-hdrc"
 	"chipidea,usb2"
+	"xlnx,zynq-usb-2.20a"
 - reg: base address and length of the registers
 - interrupts: interrupt for the USB controller
 
-- 
1.7.10.4

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> Hi,
> 
> 
> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > unlike the default platform data.  Add platform data specific to the
> > Zynq udc.
> >
> > Based on a patch by the same name from the Xilinx vendor tree.
> 
> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> temporary fix and
> I did not debug further why UDC works only when streaming is enabled.
> Probably this is right time to post my question here.
> I was expecting like:
> Streaming disabled - both low bandwidth and high bandwidth systems
> should work fine
> Streaming enabled - only for high bandwidth systems
> but this is not the case, Zynq UDC works only when Streaming is enabled.
> Please correct me if I am wrong.

You are right, stream mode disabled should work at anytime.
It is so strange why zynq UDC only works when stream mode is enabled.

Peter
> 
> Thanks,
> Sundeep
> 
> >
> > Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> > ---
> >  drivers/usb/chipidea/ci_hdrc_usb2.c |   25 +++++++++++++++++++------
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> > index 9eae1a1..4456d2c 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/dma-mapping.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > +#include <linux/of_platform.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/usb/chipidea.h>
> > @@ -30,18 +31,36 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
> >         .flags          = CI_HDRC_DISABLE_STREAMING,
> >  };
> >
> > +static struct ci_hdrc_platform_data ci_zynq_pdata = {
> > +       .capoffset      = DEF_CAPOFFSET,
> > +};
> > +
> > +static const struct of_device_id ci_hdrc_usb2_of_match[] = {
> > +       { .compatible = "chipidea,usb2"},
> > +       { .compatible = "xlnx,zynq-usb-2.20a", .data = &ci_zynq_pdata},
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
> > +
> >  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(dev);
> >         int ret;
> > +       const struct of_device_id *match;
> >
> >         if (!ci_pdata) {
> >                 ci_pdata = devm_kmalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
> >                 *ci_pdata = ci_default_pdata;   /* struct copy */
> >         }
> >
> > +       match = of_match_device(ci_hdrc_usb2_of_match, &pdev->dev);
> > +       if (match && match->data) {
> > +               /* struct copy */
> > +               *ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
> > +       }
> > +
> >         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >         if (!priv)
> >                 return -ENOMEM;
> > @@ -96,12 +115,6 @@ static int ci_hdrc_usb2_remove(struct platform_device *pdev)
> >         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,
> > --
> > 1.7.10.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

-- 

Best Regards,
Peter Chen

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> Hi,
> 
> 
> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > unlike the default platform data.  Add platform data specific to the
> > Zynq udc.
> >
> > Based on a patch by the same name from the Xilinx vendor tree.
> 
> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> temporary fix and
> I did not debug further why UDC works only when streaming is enabled.
> Probably this is right time to post my question here.
> I was expecting like:
> Streaming disabled - both low bandwidth and high bandwidth systems
> should work fine
> Streaming enabled - only for high bandwidth systems
> but this is not the case, Zynq UDC works only when Streaming is enabled.
> Please correct me if I am wrong.

You are right, stream mode disabled should work at anytime.
It is so strange why zynq UDC only works when stream mode is enabled.

Peter
> 
> Thanks,
> Sundeep
> 
> >
> > Signed-off-by: Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org>
> > ---
> >  drivers/usb/chipidea/ci_hdrc_usb2.c |   25 +++++++++++++++++++------
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> > index 9eae1a1..4456d2c 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/dma-mapping.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > +#include <linux/of_platform.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/usb/chipidea.h>
> > @@ -30,18 +31,36 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
> >         .flags          = CI_HDRC_DISABLE_STREAMING,
> >  };
> >
> > +static struct ci_hdrc_platform_data ci_zynq_pdata = {
> > +       .capoffset      = DEF_CAPOFFSET,
> > +};
> > +
> > +static const struct of_device_id ci_hdrc_usb2_of_match[] = {
> > +       { .compatible = "chipidea,usb2"},
> > +       { .compatible = "xlnx,zynq-usb-2.20a", .data = &ci_zynq_pdata},
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
> > +
> >  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(dev);
> >         int ret;
> > +       const struct of_device_id *match;
> >
> >         if (!ci_pdata) {
> >                 ci_pdata = devm_kmalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
> >                 *ci_pdata = ci_default_pdata;   /* struct copy */
> >         }
> >
> > +       match = of_match_device(ci_hdrc_usb2_of_match, &pdev->dev);
> > +       if (match && match->data) {
> > +               /* struct copy */
> > +               *ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
> > +       }
> > +
> >         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >         if (!priv)
> >                 return -ENOMEM;
> > @@ -96,12 +115,6 @@ static int ci_hdrc_usb2_remove(struct platform_device *pdev)
> >         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,
> > --
> > 1.7.10.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[sundeep subbaraya]

Hi,


On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
> The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> unlike the default platform data.  Add platform data specific to the
> Zynq udc.
>
> Based on a patch by the same name from the Xilinx vendor tree.

I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
temporary fix and
I did not debug further why UDC works only when streaming is enabled.
Probably this is right time to post my question here.
I was expecting like:
Streaming disabled - both low bandwidth and high bandwidth systems
should work fine
Streaming enabled - only for high bandwidth systems
but this is not the case, Zynq UDC works only when Streaming is enabled.
Please correct me if I am wrong.

Thanks,
Sundeep

>
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> ---
>  drivers/usb/chipidea/ci_hdrc_usb2.c |   25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 9eae1a1..4456d2c 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -12,6 +12,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/usb/chipidea.h>
> @@ -30,18 +31,36 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
>         .flags          = CI_HDRC_DISABLE_STREAMING,
>  };
>
> +static struct ci_hdrc_platform_data ci_zynq_pdata = {
> +       .capoffset      = DEF_CAPOFFSET,
> +};
> +
> +static const struct of_device_id ci_hdrc_usb2_of_match[] = {
> +       { .compatible = "chipidea,usb2"},
> +       { .compatible = "xlnx,zynq-usb-2.20a", .data = &ci_zynq_pdata},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
> +
>  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(dev);
>         int ret;
> +       const struct of_device_id *match;
>
>         if (!ci_pdata) {
>                 ci_pdata = devm_kmalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
>                 *ci_pdata = ci_default_pdata;   /* struct copy */
>         }
>
> +       match = of_match_device(ci_hdrc_usb2_of_match, &pdev->dev);
> +       if (match && match->data) {
> +               /* struct copy */
> +               *ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
> +       }
> +
>         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
>                 return -ENOMEM;
> @@ -96,12 +115,6 @@ static int ci_hdrc_usb2_remove(struct platform_device *pdev)
>         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,
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[sundeep subbaraya]

Hi,


On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
> The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> unlike the default platform data.  Add platform data specific to the
> Zynq udc.
>
> Based on a patch by the same name from the Xilinx vendor tree.

I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
temporary fix and
I did not debug further why UDC works only when streaming is enabled.
Probably this is right time to post my question here.
I was expecting like:
Streaming disabled - both low bandwidth and high bandwidth systems
should work fine
Streaming enabled - only for high bandwidth systems
but this is not the case, Zynq UDC works only when Streaming is enabled.
Please correct me if I am wrong.

Thanks,
Sundeep

>
> Signed-off-by: Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org>
> ---
>  drivers/usb/chipidea/ci_hdrc_usb2.c |   25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 9eae1a1..4456d2c 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -12,6 +12,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/usb/chipidea.h>
> @@ -30,18 +31,36 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
>         .flags          = CI_HDRC_DISABLE_STREAMING,
>  };
>
> +static struct ci_hdrc_platform_data ci_zynq_pdata = {
> +       .capoffset      = DEF_CAPOFFSET,
> +};
> +
> +static const struct of_device_id ci_hdrc_usb2_of_match[] = {
> +       { .compatible = "chipidea,usb2"},
> +       { .compatible = "xlnx,zynq-usb-2.20a", .data = &ci_zynq_pdata},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
> +
>  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(dev);
>         int ret;
> +       const struct of_device_id *match;
>
>         if (!ci_pdata) {
>                 ci_pdata = devm_kmalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
>                 *ci_pdata = ci_default_pdata;   /* struct copy */
>         }
>
> +       match = of_match_device(ci_hdrc_usb2_of_match, &pdev->dev);
> +       if (match && match->data) {
> +               /* struct copy */
> +               *ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
> +       }
> +
>         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
>                 return -ENOMEM;
> @@ -96,12 +115,6 @@ static int ci_hdrc_usb2_remove(struct platform_device *pdev)
>         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,
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[punnaiah choudary kalluri]

Hi,

On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen@freescale.com> wrote:
> On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
>> Hi,
>>
>>
>> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
>> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
>> > unlike the default platform data.  Add platform data specific to the
>> > Zynq udc.
>> >
>> > Based on a patch by the same name from the Xilinx vendor tree.
>>
>> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
>> temporary fix and
>> I did not debug further why UDC works only when streaming is enabled.
>> Probably this is right time to post my question here.
>> I was expecting like:
>> Streaming disabled - both low bandwidth and high bandwidth systems
>> should work fine
>> Streaming enabled - only for high bandwidth systems
>> but this is not the case, Zynq UDC works only when Streaming is enabled.
>> Please correct me if I am wrong.
>
> You are right, stream mode disabled should work at anytime.
> It is so strange why zynq UDC only works when stream mode is enabled.

I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
 this is what it says about SDIS (streaming mode disable option)

Before activating this mode, the user must check if the TX latency
buffers per endpoint are able to
accommodate at least one entire maximum size packet. The RX buffer
size must, at least, double the TX
buffer size per endpoint. To optimize the stream disable performance,
system bus burst must be set as high
as possible.
When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
and VUSB_HS_TX_BURST)
must be a integer sub-multiple of the latency buffer size
(VUSB_HS_RX_DEPTH for RX buffer and
VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
controller will not work properly in stream
disable mode.
The stream disable mode should just be used in situations where the
available system bandwidth is low or the
system bus access latency is high, in order to avoid underruns and
overruns in the latency buffers. This works
for all types of endpoints, except for ISO endpoints.
Such a system can't ensure the real time support that the ISO
endpoints require, so the ISO endpoints are not
supported when the SDIS bit is set.

Definitely we need to root cause why disable streaming mode is not
working for zynq but from controller spec
point of view it is possible that controller not work properly in
stream disable mode.

Regards,
Punnaiah

>
> Peter
>>

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Nathan Sullivan]

On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> Hi,
> 
> On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen@freescale.com> wrote:
> > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> >> Hi,
> >>
> >>
> >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
> >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> >> > unlike the default platform data.  Add platform data specific to the
> >> > Zynq udc.
> >> >
> >> > Based on a patch by the same name from the Xilinx vendor tree.
> >>
> >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> >> temporary fix and
> >> I did not debug further why UDC works only when streaming is enabled.
> >> Probably this is right time to post my question here.
> >> I was expecting like:
> >> Streaming disabled - both low bandwidth and high bandwidth systems
> >> should work fine
> >> Streaming enabled - only for high bandwidth systems
> >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> >> Please correct me if I am wrong.
> >
> > You are right, stream mode disabled should work at anytime.
> > It is so strange why zynq UDC only works when stream mode is enabled.
> 
> I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
>  this is what it says about SDIS (streaming mode disable option)
> 
> Before activating this mode, the user must check if the TX latency
> buffers per endpoint are able to
> accommodate at least one entire maximum size packet. The RX buffer
> size must, at least, double the TX
> buffer size per endpoint. To optimize the stream disable performance,
> system bus burst must be set as high
> as possible.
> When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> and VUSB_HS_TX_BURST)
> must be a integer sub-multiple of the latency buffer size
> (VUSB_HS_RX_DEPTH for RX buffer and
> VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> controller will not work properly in stream
> disable mode.
> The stream disable mode should just be used in situations where the
> available system bandwidth is low or the
> system bus access latency is high, in order to avoid underruns and
> overruns in the latency buffers. This works
> for all types of endpoints, except for ISO endpoints.
> Such a system can't ensure the real time support that the ISO
> endpoints require, so the ISO endpoints are not
> supported when the SDIS bit is set.
> 
> Definitely we need to root cause why disable streaming mode is not
> working for zynq but from controller spec
> point of view it is possible that controller not work properly in
> stream disable mode.
> 
> Regards,
> Punnaiah
> 

Maybe the burst size isn't set correctly by default?  It does say the controller
won't work correctly with stream disable set and an invalid burst size.  Looks
like TX and RX burst both default to 16, per the Zynq manual.

With the stream disable bit set, the behvior we see on our hardware is
that priming just stops, with an outstanding transfer in memory marked
active in the status field by the controller.  This happens at random, even 
when doing single transfers at a time like with g_ether set to have a queue
size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
bandwidth constrained, it seems like SDIS clear should be the default.

> >
> > Peter
> >>

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Nathan Sullivan]

On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> Hi,
> 
> On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> >> Hi,
> >>
> >>
> >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
> >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> >> > unlike the default platform data.  Add platform data specific to the
> >> > Zynq udc.
> >> >
> >> > Based on a patch by the same name from the Xilinx vendor tree.
> >>
> >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> >> temporary fix and
> >> I did not debug further why UDC works only when streaming is enabled.
> >> Probably this is right time to post my question here.
> >> I was expecting like:
> >> Streaming disabled - both low bandwidth and high bandwidth systems
> >> should work fine
> >> Streaming enabled - only for high bandwidth systems
> >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> >> Please correct me if I am wrong.
> >
> > You are right, stream mode disabled should work at anytime.
> > It is so strange why zynq UDC only works when stream mode is enabled.
> 
> I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
>  this is what it says about SDIS (streaming mode disable option)
> 
> Before activating this mode, the user must check if the TX latency
> buffers per endpoint are able to
> accommodate at least one entire maximum size packet. The RX buffer
> size must, at least, double the TX
> buffer size per endpoint. To optimize the stream disable performance,
> system bus burst must be set as high
> as possible.
> When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> and VUSB_HS_TX_BURST)
> must be a integer sub-multiple of the latency buffer size
> (VUSB_HS_RX_DEPTH for RX buffer and
> VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> controller will not work properly in stream
> disable mode.
> The stream disable mode should just be used in situations where the
> available system bandwidth is low or the
> system bus access latency is high, in order to avoid underruns and
> overruns in the latency buffers. This works
> for all types of endpoints, except for ISO endpoints.
> Such a system can't ensure the real time support that the ISO
> endpoints require, so the ISO endpoints are not
> supported when the SDIS bit is set.
> 
> Definitely we need to root cause why disable streaming mode is not
> working for zynq but from controller spec
> point of view it is possible that controller not work properly in
> stream disable mode.
> 
> Regards,
> Punnaiah
> 

Maybe the burst size isn't set correctly by default?  It does say the controller
won't work correctly with stream disable set and an invalid burst size.  Looks
like TX and RX burst both default to 16, per the Zynq manual.

With the stream disable bit set, the behvior we see on our hardware is
that priming just stops, with an outstanding transfer in memory marked
active in the status field by the controller.  This happens at random, even 
when doing single transfers at a time like with g_ether set to have a queue
size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
bandwidth constrained, it seems like SDIS clear should be the default.

> >
> > Peter
> >>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> > Hi,
> > 
> > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen@freescale.com> wrote:
> > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> > >> Hi,
> > >>
> > >>
> > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
> > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > >> > unlike the default platform data.  Add platform data specific to the
> > >> > Zynq udc.
> > >> >
> > >> > Based on a patch by the same name from the Xilinx vendor tree.
> > >>
> > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> > >> temporary fix and
> > >> I did not debug further why UDC works only when streaming is enabled.
> > >> Probably this is right time to post my question here.
> > >> I was expecting like:
> > >> Streaming disabled - both low bandwidth and high bandwidth systems
> > >> should work fine
> > >> Streaming enabled - only for high bandwidth systems
> > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> > >> Please correct me if I am wrong.
> > >
> > > You are right, stream mode disabled should work at anytime.
> > > It is so strange why zynq UDC only works when stream mode is enabled.
> > 
> > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
> >  this is what it says about SDIS (streaming mode disable option)
> > 
> > Before activating this mode, the user must check if the TX latency
> > buffers per endpoint are able to
> > accommodate at least one entire maximum size packet. The RX buffer
> > size must, at least, double the TX
> > buffer size per endpoint. To optimize the stream disable performance,
> > system bus burst must be set as high
> > as possible.
> > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> > and VUSB_HS_TX_BURST)
> > must be a integer sub-multiple of the latency buffer size
> > (VUSB_HS_RX_DEPTH for RX buffer and
> > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> > controller will not work properly in stream
> > disable mode.
> > The stream disable mode should just be used in situations where the
> > available system bandwidth is low or the
> > system bus access latency is high, in order to avoid underruns and
> > overruns in the latency buffers. This works
> > for all types of endpoints, except for ISO endpoints.
> > Such a system can't ensure the real time support that the ISO
> > endpoints require, so the ISO endpoints are not
> > supported when the SDIS bit is set.
> > 
> > Definitely we need to root cause why disable streaming mode is not
> > working for zynq but from controller spec
> > point of view it is possible that controller not work properly in
> > stream disable mode.
> > 
> > Regards,
> > Punnaiah
> > 
> 
> Maybe the burst size isn't set correctly by default?  It does say the controller
> won't work correctly with stream disable set and an invalid burst size.  Looks
> like TX and RX burst both default to 16, per the Zynq manual.
> 
> With the stream disable bit set, the behvior we see on our hardware is
> that priming just stops, with an outstanding transfer in memory marked
> active in the status field by the controller.  This happens at random, even 
> when doing single transfers at a time like with g_ether set to have a queue
> size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
> bandwidth constrained, it seems like SDIS clear should be the default.
> 

I suspect the possible reason is the tx buffer for each endpoint is
small (<=512 bytes), so it can't copy one packet (assume max packet size
for bulk) to tx buffer, then the prime can't be finished.

Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
($BASE + 124)?

tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
(DWORD_PER_BYTES)

DWORD_PER_BYTES is 4

-- 

Best Regards,
Peter Chen

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> > Hi,
> > 
> > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> > >> Hi,
> > >>
> > >>
> > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
> > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > >> > unlike the default platform data.  Add platform data specific to the
> > >> > Zynq udc.
> > >> >
> > >> > Based on a patch by the same name from the Xilinx vendor tree.
> > >>
> > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> > >> temporary fix and
> > >> I did not debug further why UDC works only when streaming is enabled.
> > >> Probably this is right time to post my question here.
> > >> I was expecting like:
> > >> Streaming disabled - both low bandwidth and high bandwidth systems
> > >> should work fine
> > >> Streaming enabled - only for high bandwidth systems
> > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> > >> Please correct me if I am wrong.
> > >
> > > You are right, stream mode disabled should work at anytime.
> > > It is so strange why zynq UDC only works when stream mode is enabled.
> > 
> > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
> >  this is what it says about SDIS (streaming mode disable option)
> > 
> > Before activating this mode, the user must check if the TX latency
> > buffers per endpoint are able to
> > accommodate at least one entire maximum size packet. The RX buffer
> > size must, at least, double the TX
> > buffer size per endpoint. To optimize the stream disable performance,
> > system bus burst must be set as high
> > as possible.
> > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> > and VUSB_HS_TX_BURST)
> > must be a integer sub-multiple of the latency buffer size
> > (VUSB_HS_RX_DEPTH for RX buffer and
> > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> > controller will not work properly in stream
> > disable mode.
> > The stream disable mode should just be used in situations where the
> > available system bandwidth is low or the
> > system bus access latency is high, in order to avoid underruns and
> > overruns in the latency buffers. This works
> > for all types of endpoints, except for ISO endpoints.
> > Such a system can't ensure the real time support that the ISO
> > endpoints require, so the ISO endpoints are not
> > supported when the SDIS bit is set.
> > 
> > Definitely we need to root cause why disable streaming mode is not
> > working for zynq but from controller spec
> > point of view it is possible that controller not work properly in
> > stream disable mode.
> > 
> > Regards,
> > Punnaiah
> > 
> 
> Maybe the burst size isn't set correctly by default?  It does say the controller
> won't work correctly with stream disable set and an invalid burst size.  Looks
> like TX and RX burst both default to 16, per the Zynq manual.
> 
> With the stream disable bit set, the behvior we see on our hardware is
> that priming just stops, with an outstanding transfer in memory marked
> active in the status field by the controller.  This happens at random, even 
> when doing single transfers at a time like with g_ether set to have a queue
> size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
> bandwidth constrained, it seems like SDIS clear should be the default.
> 

I suspect the possible reason is the tx buffer for each endpoint is
small (<=512 bytes), so it can't copy one packet (assume max packet size
for bulk) to tx buffer, then the prime can't be finished.

Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
($BASE + 124)?

tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
(DWORD_PER_BYTES)

DWORD_PER_BYTES is 4

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Nathan Sullivan]

On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> > > Hi,
> > > 
> > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen@freescale.com> wrote:
> > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> > > >> Hi,
> > > >>
> > > >>
> > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
> > > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > > >> > unlike the default platform data.  Add platform data specific to the
> > > >> > Zynq udc.
> > > >> >
> > > >> > Based on a patch by the same name from the Xilinx vendor tree.
> > > >>
> > > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> > > >> temporary fix and
> > > >> I did not debug further why UDC works only when streaming is enabled.
> > > >> Probably this is right time to post my question here.
> > > >> I was expecting like:
> > > >> Streaming disabled - both low bandwidth and high bandwidth systems
> > > >> should work fine
> > > >> Streaming enabled - only for high bandwidth systems
> > > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> > > >> Please correct me if I am wrong.
> > > >
> > > > You are right, stream mode disabled should work at anytime.
> > > > It is so strange why zynq UDC only works when stream mode is enabled.
> > > 
> > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
> > >  this is what it says about SDIS (streaming mode disable option)
> > > 
> > > Before activating this mode, the user must check if the TX latency
> > > buffers per endpoint are able to
> > > accommodate at least one entire maximum size packet. The RX buffer
> > > size must, at least, double the TX
> > > buffer size per endpoint. To optimize the stream disable performance,
> > > system bus burst must be set as high
> > > as possible.
> > > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> > > and VUSB_HS_TX_BURST)
> > > must be a integer sub-multiple of the latency buffer size
> > > (VUSB_HS_RX_DEPTH for RX buffer and
> > > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> > > controller will not work properly in stream
> > > disable mode.
> > > The stream disable mode should just be used in situations where the
> > > available system bandwidth is low or the
> > > system bus access latency is high, in order to avoid underruns and
> > > overruns in the latency buffers. This works
> > > for all types of endpoints, except for ISO endpoints.
> > > Such a system can't ensure the real time support that the ISO
> > > endpoints require, so the ISO endpoints are not
> > > supported when the SDIS bit is set.
> > > 
> > > Definitely we need to root cause why disable streaming mode is not
> > > working for zynq but from controller spec
> > > point of view it is possible that controller not work properly in
> > > stream disable mode.
> > > 
> > > Regards,
> > > Punnaiah
> > > 
> > 
> > Maybe the burst size isn't set correctly by default?  It does say the controller
> > won't work correctly with stream disable set and an invalid burst size.  Looks
> > like TX and RX burst both default to 16, per the Zynq manual.
> > 
> > With the stream disable bit set, the behvior we see on our hardware is
> > that priming just stops, with an outstanding transfer in memory marked
> > active in the status field by the controller.  This happens at random, even 
> > when doing single transfers at a time like with g_ether set to have a queue
> > size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
> > bandwidth constrained, it seems like SDIS clear should be the default.
> > 
> 
> I suspect the possible reason is the tx buffer for each endpoint is
> small (<=512 bytes), so it can't copy one packet (assume max packet size
> for bulk) to tx buffer, then the prime can't be finished.
> 
> Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
> ($BASE + 124)?
> 
> tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
> (DWORD_PER_BYTES)
> 
> DWORD_PER_BYTES is 4
> 
> -- 
> 
> Best Regards,
> Peter Chen

HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Nathan Sullivan]

On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> > > Hi,
> > > 
> > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> > > >> Hi,
> > > >>
> > > >>
> > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
> > > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > > >> > unlike the default platform data.  Add platform data specific to the
> > > >> > Zynq udc.
> > > >> >
> > > >> > Based on a patch by the same name from the Xilinx vendor tree.
> > > >>
> > > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> > > >> temporary fix and
> > > >> I did not debug further why UDC works only when streaming is enabled.
> > > >> Probably this is right time to post my question here.
> > > >> I was expecting like:
> > > >> Streaming disabled - both low bandwidth and high bandwidth systems
> > > >> should work fine
> > > >> Streaming enabled - only for high bandwidth systems
> > > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> > > >> Please correct me if I am wrong.
> > > >
> > > > You are right, stream mode disabled should work at anytime.
> > > > It is so strange why zynq UDC only works when stream mode is enabled.
> > > 
> > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
> > >  this is what it says about SDIS (streaming mode disable option)
> > > 
> > > Before activating this mode, the user must check if the TX latency
> > > buffers per endpoint are able to
> > > accommodate at least one entire maximum size packet. The RX buffer
> > > size must, at least, double the TX
> > > buffer size per endpoint. To optimize the stream disable performance,
> > > system bus burst must be set as high
> > > as possible.
> > > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> > > and VUSB_HS_TX_BURST)
> > > must be a integer sub-multiple of the latency buffer size
> > > (VUSB_HS_RX_DEPTH for RX buffer and
> > > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> > > controller will not work properly in stream
> > > disable mode.
> > > The stream disable mode should just be used in situations where the
> > > available system bandwidth is low or the
> > > system bus access latency is high, in order to avoid underruns and
> > > overruns in the latency buffers. This works
> > > for all types of endpoints, except for ISO endpoints.
> > > Such a system can't ensure the real time support that the ISO
> > > endpoints require, so the ISO endpoints are not
> > > supported when the SDIS bit is set.
> > > 
> > > Definitely we need to root cause why disable streaming mode is not
> > > working for zynq but from controller spec
> > > point of view it is possible that controller not work properly in
> > > stream disable mode.
> > > 
> > > Regards,
> > > Punnaiah
> > > 
> > 
> > Maybe the burst size isn't set correctly by default?  It does say the controller
> > won't work correctly with stream disable set and an invalid burst size.  Looks
> > like TX and RX burst both default to 16, per the Zynq manual.
> > 
> > With the stream disable bit set, the behvior we see on our hardware is
> > that priming just stops, with an outstanding transfer in memory marked
> > active in the status field by the controller.  This happens at random, even 
> > when doing single transfers at a time like with g_ether set to have a queue
> > size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
> > bandwidth constrained, it seems like SDIS clear should be the default.
> > 
> 
> I suspect the possible reason is the tx buffer for each endpoint is
> small (<=512 bytes), so it can't copy one packet (assume max packet size
> for bulk) to tx buffer, then the prime can't be finished.
> 
> Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
> ($BASE + 124)?
> 
> tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
> (DWORD_PER_BYTES)
> 
> DWORD_PER_BYTES is 4
> 
> -- 
> 
> Best Regards,
> Peter Chen

HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> > > > Hi,
> > > > 
> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen@freescale.com> wrote:
> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> > > > >> Hi,
> > > > >>
> > > > >>
> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
> > > > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > > > >> > unlike the default platform data.  Add platform data specific to the
> > > > >> > Zynq udc.
> > > > >> >
> > > > >> > Based on a patch by the same name from the Xilinx vendor tree.
> > > > >>
> > > > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> > > > >> temporary fix and
> > > > >> I did not debug further why UDC works only when streaming is enabled.
> > > > >> Probably this is right time to post my question here.
> > > > >> I was expecting like:
> > > > >> Streaming disabled - both low bandwidth and high bandwidth systems
> > > > >> should work fine
> > > > >> Streaming enabled - only for high bandwidth systems
> > > > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> > > > >> Please correct me if I am wrong.
> > > > >
> > > > > You are right, stream mode disabled should work at anytime.
> > > > > It is so strange why zynq UDC only works when stream mode is enabled.
> > > > 
> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
> > > >  this is what it says about SDIS (streaming mode disable option)
> > > > 
> > > > Before activating this mode, the user must check if the TX latency
> > > > buffers per endpoint are able to
> > > > accommodate at least one entire maximum size packet. The RX buffer
> > > > size must, at least, double the TX
> > > > buffer size per endpoint. To optimize the stream disable performance,
> > > > system bus burst must be set as high
> > > > as possible.
> > > > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> > > > and VUSB_HS_TX_BURST)
> > > > must be a integer sub-multiple of the latency buffer size
> > > > (VUSB_HS_RX_DEPTH for RX buffer and
> > > > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> > > > controller will not work properly in stream
> > > > disable mode.
> > > > The stream disable mode should just be used in situations where the
> > > > available system bandwidth is low or the
> > > > system bus access latency is high, in order to avoid underruns and
> > > > overruns in the latency buffers. This works
> > > > for all types of endpoints, except for ISO endpoints.
> > > > Such a system can't ensure the real time support that the ISO
> > > > endpoints require, so the ISO endpoints are not
> > > > supported when the SDIS bit is set.
> > > > 
> > > > Definitely we need to root cause why disable streaming mode is not
> > > > working for zynq but from controller spec
> > > > point of view it is possible that controller not work properly in
> > > > stream disable mode.
> > > > 
> > > > Regards,
> > > > Punnaiah
> > > > 
> > > 
> > > Maybe the burst size isn't set correctly by default?  It does say the controller
> > > won't work correctly with stream disable set and an invalid burst size.  Looks
> > > like TX and RX burst both default to 16, per the Zynq manual.
> > > 
> > > With the stream disable bit set, the behvior we see on our hardware is
> > > that priming just stops, with an outstanding transfer in memory marked
> > > active in the status field by the controller.  This happens at random, even 
> > > when doing single transfers at a time like with g_ether set to have a queue
> > > size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
> > > bandwidth constrained, it seems like SDIS clear should be the default.
> > > 
> > 
> > I suspect the possible reason is the tx buffer for each endpoint is
> > small (<=512 bytes), so it can't copy one packet (assume max packet size
> > for bulk) to tx buffer, then the prime can't be finished.
> > 
> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
> > ($BASE + 124)?
> > 
> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
> > (DWORD_PER_BYTES)
> > 
> > DWORD_PER_BYTES is 4
> > 
> > -- 
> > 
> > Best Regards,
> > Peter Chen
> 
> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.

Are you sure you read correct address? Your DCCPARAMS means
it is host capable, but not device capable.

-- 

Best Regards,
Peter Chen

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> > > > Hi,
> > > > 
> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> > > > >> Hi,
> > > > >>
> > > > >>
> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
> > > > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> > > > >> > unlike the default platform data.  Add platform data specific to the
> > > > >> > Zynq udc.
> > > > >> >
> > > > >> > Based on a patch by the same name from the Xilinx vendor tree.
> > > > >>
> > > > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> > > > >> temporary fix and
> > > > >> I did not debug further why UDC works only when streaming is enabled.
> > > > >> Probably this is right time to post my question here.
> > > > >> I was expecting like:
> > > > >> Streaming disabled - both low bandwidth and high bandwidth systems
> > > > >> should work fine
> > > > >> Streaming enabled - only for high bandwidth systems
> > > > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> > > > >> Please correct me if I am wrong.
> > > > >
> > > > > You are right, stream mode disabled should work at anytime.
> > > > > It is so strange why zynq UDC only works when stream mode is enabled.
> > > > 
> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
> > > >  this is what it says about SDIS (streaming mode disable option)
> > > > 
> > > > Before activating this mode, the user must check if the TX latency
> > > > buffers per endpoint are able to
> > > > accommodate at least one entire maximum size packet. The RX buffer
> > > > size must, at least, double the TX
> > > > buffer size per endpoint. To optimize the stream disable performance,
> > > > system bus burst must be set as high
> > > > as possible.
> > > > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> > > > and VUSB_HS_TX_BURST)
> > > > must be a integer sub-multiple of the latency buffer size
> > > > (VUSB_HS_RX_DEPTH for RX buffer and
> > > > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> > > > controller will not work properly in stream
> > > > disable mode.
> > > > The stream disable mode should just be used in situations where the
> > > > available system bandwidth is low or the
> > > > system bus access latency is high, in order to avoid underruns and
> > > > overruns in the latency buffers. This works
> > > > for all types of endpoints, except for ISO endpoints.
> > > > Such a system can't ensure the real time support that the ISO
> > > > endpoints require, so the ISO endpoints are not
> > > > supported when the SDIS bit is set.
> > > > 
> > > > Definitely we need to root cause why disable streaming mode is not
> > > > working for zynq but from controller spec
> > > > point of view it is possible that controller not work properly in
> > > > stream disable mode.
> > > > 
> > > > Regards,
> > > > Punnaiah
> > > > 
> > > 
> > > Maybe the burst size isn't set correctly by default?  It does say the controller
> > > won't work correctly with stream disable set and an invalid burst size.  Looks
> > > like TX and RX burst both default to 16, per the Zynq manual.
> > > 
> > > With the stream disable bit set, the behvior we see on our hardware is
> > > that priming just stops, with an outstanding transfer in memory marked
> > > active in the status field by the controller.  This happens at random, even 
> > > when doing single transfers at a time like with g_ether set to have a queue
> > > size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
> > > bandwidth constrained, it seems like SDIS clear should be the default.
> > > 
> > 
> > I suspect the possible reason is the tx buffer for each endpoint is
> > small (<=512 bytes), so it can't copy one packet (assume max packet size
> > for bulk) to tx buffer, then the prime can't be finished.
> > 
> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
> > ($BASE + 124)?
> > 
> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
> > (DWORD_PER_BYTES)
> > 
> > DWORD_PER_BYTES is 4
> > 
> > -- 
> > 
> > Best Regards,
> > Peter Chen
> 
> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.

Are you sure you read correct address? Your DCCPARAMS means
it is host capable, but not device capable.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Mon, Aug 31, 2015 at 09:01:51AM +0530, punnaiah choudary kalluri wrote:
> On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen <peter.chen@freescale.com> wrote:
> > On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> >> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> >> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> >> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
> >> > > > Hi,
> >> > > >
> >> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen@freescale.com> wrote:
> >> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
> >> > > > >> Hi,
> >> > > > >>
> >> > > > >>
> >> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
> >> > > > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> >> > > > >> > unlike the default platform data.  Add platform data specific to the
> >> > > > >> > Zynq udc.
> >> > > > >> >
> >> > > > >> > Based on a patch by the same name from the Xilinx vendor tree.
> >> > > > >>
> >> > > > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
> >> > > > >> temporary fix and
> >> > > > >> I did not debug further why UDC works only when streaming is enabled.
> >> > > > >> Probably this is right time to post my question here.
> >> > > > >> I was expecting like:
> >> > > > >> Streaming disabled - both low bandwidth and high bandwidth systems
> >> > > > >> should work fine
> >> > > > >> Streaming enabled - only for high bandwidth systems
> >> > > > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
> >> > > > >> Please correct me if I am wrong.
> >> > > > >
> >> > > > > You are right, stream mode disabled should work at anytime.
> >> > > > > It is so strange why zynq UDC only works when stream mode is enabled.
> >> > > >
> >> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
> >> > > >  this is what it says about SDIS (streaming mode disable option)
> >> > > >
> >> > > > Before activating this mode, the user must check if the TX latency
> >> > > > buffers per endpoint are able to
> >> > > > accommodate at least one entire maximum size packet. The RX buffer
> >> > > > size must, at least, double the TX
> >> > > > buffer size per endpoint. To optimize the stream disable performance,
> >> > > > system bus burst must be set as high
> >> > > > as possible.
> >> > > > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
> >> > > > and VUSB_HS_TX_BURST)
> >> > > > must be a integer sub-multiple of the latency buffer size
> >> > > > (VUSB_HS_RX_DEPTH for RX buffer and
> >> > > > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
> >> > > > controller will not work properly in stream
> >> > > > disable mode.
> >> > > > The stream disable mode should just be used in situations where the
> >> > > > available system bandwidth is low or the
> >> > > > system bus access latency is high, in order to avoid underruns and
> >> > > > overruns in the latency buffers. This works
> >> > > > for all types of endpoints, except for ISO endpoints.
> >> > > > Such a system can't ensure the real time support that the ISO
> >> > > > endpoints require, so the ISO endpoints are not
> >> > > > supported when the SDIS bit is set.
> >> > > >
> >> > > > Definitely we need to root cause why disable streaming mode is not
> >> > > > working for zynq but from controller spec
> >> > > > point of view it is possible that controller not work properly in
> >> > > > stream disable mode.
> >> > > >
> >> > > > Regards,
> >> > > > Punnaiah
> >> > > >
> >> > >
> >> > > Maybe the burst size isn't set correctly by default?  It does say the controller
> >> > > won't work correctly with stream disable set and an invalid burst size.  Looks
> >> > > like TX and RX burst both default to 16, per the Zynq manual.
> >> > >
> >> > > With the stream disable bit set, the behvior we see on our hardware is
> >> > > that priming just stops, with an outstanding transfer in memory marked
> >> > > active in the status field by the controller.  This happens at random, even
> >> > > when doing single transfers at a time like with g_ether set to have a queue
> >> > > size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
> >> > > bandwidth constrained, it seems like SDIS clear should be the default.
> >> > >
> >> >
> >> > I suspect the possible reason is the tx buffer for each endpoint is
> >> > small (<=512 bytes), so it can't copy one packet (assume max packet size
> >> > for bulk) to tx buffer, then the prime can't be finished.
> >> >
> >> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
> >> > ($BASE + 124)?
> >> >
> >> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
> >> > (DWORD_PER_BYTES)
> >> >
> >> > DWORD_PER_BYTES is 4
> >> >
> >> > --
> >> >
> >> > Best Regards,
> >> > Peter Chen
> >>
> >> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
> >
> > Are you sure you read correct address? Your DCCPARAMS means
> > it is host capable, but not device capable.
> 
> HWTXBUF is 0x8060A10
> DCCPARAMS is 0x0000018C
> 
> VUSB_HS_TX_ADD - 0xA
> DEN - 0xC
> 
> From the above formula tx buffer size is 341.33 bytes.
> 
> 

ok, so your platform can't use stream mode disabled.

-- 

Best Regards,
Peter Chen

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Wed, Aug 26, 2015 at 10:27:44AM -0500, Nathan Sullivan wrote:
> The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> unlike the default platform data.  Add platform data specific to the
> Zynq udc.
> 

You may add the possible reason, eg, tx buffer is not enough.

Others are ok.

> Based on a patch by the same name from the Xilinx vendor tree.
> 
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> ---
>  drivers/usb/chipidea/ci_hdrc_usb2.c |   25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 9eae1a1..4456d2c 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -12,6 +12,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/usb/chipidea.h>
> @@ -30,18 +31,36 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
>  	.flags		= CI_HDRC_DISABLE_STREAMING,
>  };
>  
> +static struct ci_hdrc_platform_data ci_zynq_pdata = {
> +	.capoffset	= DEF_CAPOFFSET,
> +};
> +
> +static const struct of_device_id ci_hdrc_usb2_of_match[] = {
> +	{ .compatible = "chipidea,usb2"},
> +	{ .compatible = "xlnx,zynq-usb-2.20a", .data = &ci_zynq_pdata},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
> +
>  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(dev);
>  	int ret;
> +	const struct of_device_id *match;
>  
>  	if (!ci_pdata) {
>  		ci_pdata = devm_kmalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
>  		*ci_pdata = ci_default_pdata;	/* struct copy */
>  	}
>  
> +	match = of_match_device(ci_hdrc_usb2_of_match, &pdev->dev);
> +	if (match && match->data) {
> +		/* struct copy */
> +		*ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
> +	}
> +
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
> @@ -96,12 +115,6 @@ static int ci_hdrc_usb2_remove(struct platform_device *pdev)
>  	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,
> -- 
> 1.7.10.4
> 

-- 

Best Regards,
Peter Chen

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Peter Chen]

On Wed, Aug 26, 2015 at 10:27:44AM -0500, Nathan Sullivan wrote:
> The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
> unlike the default platform data.  Add platform data specific to the
> Zynq udc.
> 

You may add the possible reason, eg, tx buffer is not enough.

Others are ok.

> Based on a patch by the same name from the Xilinx vendor tree.
> 
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> ---
>  drivers/usb/chipidea/ci_hdrc_usb2.c |   25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 9eae1a1..4456d2c 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -12,6 +12,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/usb/chipidea.h>
> @@ -30,18 +31,36 @@ static const struct ci_hdrc_platform_data ci_default_pdata = {
>  	.flags		= CI_HDRC_DISABLE_STREAMING,
>  };
>  
> +static struct ci_hdrc_platform_data ci_zynq_pdata = {
> +	.capoffset	= DEF_CAPOFFSET,
> +};
> +
> +static const struct of_device_id ci_hdrc_usb2_of_match[] = {
> +	{ .compatible = "chipidea,usb2"},
> +	{ .compatible = "xlnx,zynq-usb-2.20a", .data = &ci_zynq_pdata},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
> +
>  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(dev);
>  	int ret;
> +	const struct of_device_id *match;
>  
>  	if (!ci_pdata) {
>  		ci_pdata = devm_kmalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
>  		*ci_pdata = ci_default_pdata;	/* struct copy */
>  	}
>  
> +	match = of_match_device(ci_hdrc_usb2_of_match, &pdev->dev);
> +	if (match && match->data) {
> +		/* struct copy */
> +		*ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
> +	}
> +
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
> @@ -96,12 +115,6 @@ static int ci_hdrc_usb2_remove(struct platform_device *pdev)
>  	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,
> -- 
> 1.7.10.4
> 

-- 

Best Regards,
Peter Chen

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[punnaiah choudary kalluri]

On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen <peter.chen@freescale.com> wrote:
> On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
>> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
>> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
>> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
>> > > > Hi,
>> > > >
>> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen@freescale.com> wrote:
>> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
>> > > > >> Hi,
>> > > > >>
>> > > > >>
>> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan@ni.com> wrote:
>> > > > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
>> > > > >> > unlike the default platform data.  Add platform data specific to the
>> > > > >> > Zynq udc.
>> > > > >> >
>> > > > >> > Based on a patch by the same name from the Xilinx vendor tree.
>> > > > >>
>> > > > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
>> > > > >> temporary fix and
>> > > > >> I did not debug further why UDC works only when streaming is enabled.
>> > > > >> Probably this is right time to post my question here.
>> > > > >> I was expecting like:
>> > > > >> Streaming disabled - both low bandwidth and high bandwidth systems
>> > > > >> should work fine
>> > > > >> Streaming enabled - only for high bandwidth systems
>> > > > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
>> > > > >> Please correct me if I am wrong.
>> > > > >
>> > > > > You are right, stream mode disabled should work at anytime.
>> > > > > It is so strange why zynq UDC only works when stream mode is enabled.
>> > > >
>> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
>> > > >  this is what it says about SDIS (streaming mode disable option)
>> > > >
>> > > > Before activating this mode, the user must check if the TX latency
>> > > > buffers per endpoint are able to
>> > > > accommodate at least one entire maximum size packet. The RX buffer
>> > > > size must, at least, double the TX
>> > > > buffer size per endpoint. To optimize the stream disable performance,
>> > > > system bus burst must be set as high
>> > > > as possible.
>> > > > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
>> > > > and VUSB_HS_TX_BURST)
>> > > > must be a integer sub-multiple of the latency buffer size
>> > > > (VUSB_HS_RX_DEPTH for RX buffer and
>> > > > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
>> > > > controller will not work properly in stream
>> > > > disable mode.
>> > > > The stream disable mode should just be used in situations where the
>> > > > available system bandwidth is low or the
>> > > > system bus access latency is high, in order to avoid underruns and
>> > > > overruns in the latency buffers. This works
>> > > > for all types of endpoints, except for ISO endpoints.
>> > > > Such a system can't ensure the real time support that the ISO
>> > > > endpoints require, so the ISO endpoints are not
>> > > > supported when the SDIS bit is set.
>> > > >
>> > > > Definitely we need to root cause why disable streaming mode is not
>> > > > working for zynq but from controller spec
>> > > > point of view it is possible that controller not work properly in
>> > > > stream disable mode.
>> > > >
>> > > > Regards,
>> > > > Punnaiah
>> > > >
>> > >
>> > > Maybe the burst size isn't set correctly by default?  It does say the controller
>> > > won't work correctly with stream disable set and an invalid burst size.  Looks
>> > > like TX and RX burst both default to 16, per the Zynq manual.
>> > >
>> > > With the stream disable bit set, the behvior we see on our hardware is
>> > > that priming just stops, with an outstanding transfer in memory marked
>> > > active in the status field by the controller.  This happens at random, even
>> > > when doing single transfers at a time like with g_ether set to have a queue
>> > > size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
>> > > bandwidth constrained, it seems like SDIS clear should be the default.
>> > >
>> >
>> > I suspect the possible reason is the tx buffer for each endpoint is
>> > small (<=512 bytes), so it can't copy one packet (assume max packet size
>> > for bulk) to tx buffer, then the prime can't be finished.
>> >
>> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
>> > ($BASE + 124)?
>> >
>> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
>> > (DWORD_PER_BYTES)
>> >
>> > DWORD_PER_BYTES is 4
>> >
>> > --
>> >
>> > Best Regards,
>> > Peter Chen
>>
>> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
>
> Are you sure you read correct address? Your DCCPARAMS means
> it is host capable, but not device capable.

HWTXBUF is 0x8060A10
DCCPARAMS is 0x0000018C

VUSB_HS_TX_ADD - 0xA
DEN - 0xC

>From the above formula tx buffer size is 341.33 bytes.


Regards,
Punnaiah

>
> --
>
> Best Regards,
> Peter Chen
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[punnaiah choudary kalluri]

On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
>> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
>> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
>> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary kalluri wrote:
>> > > > Hi,
>> > > >
>> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep subbaraya wrote:
>> > > > >> Hi,
>> > > > >>
>> > > > >>
>> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
>> > > > >> > The Xilinx Zynq udc does not need the CI_HDRC_DISABLE_STREAMING flag,
>> > > > >> > unlike the default platform data.  Add platform data specific to the
>> > > > >> > Zynq udc.
>> > > > >> >
>> > > > >> > Based on a patch by the same name from the Xilinx vendor tree.
>> > > > >>
>> > > > >> I am that Xilinx guy who sent this patch :). It is in Xilinx tree as
>> > > > >> temporary fix and
>> > > > >> I did not debug further why UDC works only when streaming is enabled.
>> > > > >> Probably this is right time to post my question here.
>> > > > >> I was expecting like:
>> > > > >> Streaming disabled - both low bandwidth and high bandwidth systems
>> > > > >> should work fine
>> > > > >> Streaming enabled - only for high bandwidth systems
>> > > > >> but this is not the case, Zynq UDC works only when Streaming is enabled.
>> > > > >> Please correct me if I am wrong.
>> > > > >
>> > > > > You are right, stream mode disabled should work at anytime.
>> > > > > It is so strange why zynq UDC only works when stream mode is enabled.
>> > > >
>> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS controllervDoc 2.20a,
>> > > >  this is what it says about SDIS (streaming mode disable option)
>> > > >
>> > > > Before activating this mode, the user must check if the TX latency
>> > > > buffers per endpoint are able to
>> > > > accommodate at least one entire maximum size packet. The RX buffer
>> > > > size must, at least, double the TX
>> > > > buffer size per endpoint. To optimize the stream disable performance,
>> > > > system bus burst must be set as high
>> > > > as possible.
>> > > > When the stream disable mode is used, the burst size (VUSB_HS_RX_BURST
>> > > > and VUSB_HS_TX_BURST)
>> > > > must be a integer sub-multiple of the latency buffer size
>> > > > (VUSB_HS_RX_DEPTH for RX buffer and
>> > > > VUSB_HS_TX_CHAN for the TX buffer). If this is not respected the
>> > > > controller will not work properly in stream
>> > > > disable mode.
>> > > > The stream disable mode should just be used in situations where the
>> > > > available system bandwidth is low or the
>> > > > system bus access latency is high, in order to avoid underruns and
>> > > > overruns in the latency buffers. This works
>> > > > for all types of endpoints, except for ISO endpoints.
>> > > > Such a system can't ensure the real time support that the ISO
>> > > > endpoints require, so the ISO endpoints are not
>> > > > supported when the SDIS bit is set.
>> > > >
>> > > > Definitely we need to root cause why disable streaming mode is not
>> > > > working for zynq but from controller spec
>> > > > point of view it is possible that controller not work properly in
>> > > > stream disable mode.
>> > > >
>> > > > Regards,
>> > > > Punnaiah
>> > > >
>> > >
>> > > Maybe the burst size isn't set correctly by default?  It does say the controller
>> > > won't work correctly with stream disable set and an invalid burst size.  Looks
>> > > like TX and RX burst both default to 16, per the Zynq manual.
>> > >
>> > > With the stream disable bit set, the behvior we see on our hardware is
>> > > that priming just stops, with an outstanding transfer in memory marked
>> > > active in the status field by the controller.  This happens at random, even
>> > > when doing single transfers at a time like with g_ether set to have a queue
>> > > size of 1.  With SDIS clear everything works great.  Given that the Zynq is not
>> > > bandwidth constrained, it seems like SDIS clear should be the default.
>> > >
>> >
>> > I suspect the possible reason is the tx buffer for each endpoint is
>> > small (<=512 bytes), so it can't copy one packet (assume max packet size
>> > for bulk) to tx buffer, then the prime can't be finished.
>> >
>> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and DCCPARAMS
>> > ($BASE + 124)?
>> >
>> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) / DCCPARAMS.DEN) *
>> > (DWORD_PER_BYTES)
>> >
>> > DWORD_PER_BYTES is 4
>> >
>> > --
>> >
>> > Best Regards,
>> > Peter Chen
>>
>> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
>
> Are you sure you read correct address? Your DCCPARAMS means
> it is host capable, but not device capable.

HWTXBUF is 0x8060A10
DCCPARAMS is 0x0000018C

VUSB_HS_TX_ADD - 0xA
DEN - 0xC

>From the above formula tx buffer size is 341.33 bytes.


Regards,
Punnaiah

>
> --
>
> Best Regards,
> Peter Chen
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Subbaraya Sundeep Bhatta]

Hi,

> -----Original Message-----
> From: Peter Chen [mailto:peter.chen@freescale.com]
> Sent: Monday, August 31, 2015 7:59 AM
> To: Punnaiah Choudary Kalluri
> Cc: Nathan Sullivan; sundeep subbaraya; Subbaraya Sundeep Bhatta;
> robh+dt@kernel.org; pawel.moll@arm.com; Mark Rutland;
> ijc+devicetree@hellion.org.uk; Kumar Gala; Greg Kroah-Hartman;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> usb@vger.kernel.org
> Subject: Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> 
> On Mon, Aug 31, 2015 at 09:01:51AM +0530, punnaiah choudary kalluri
> wrote:
> > On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen
> <peter.chen@freescale.com> wrote:
> > > On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> > >> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> > >> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > >> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary
> kalluri wrote:
> > >> > > > Hi,
> > >> > > >
> > >> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen
> <peter.chen@freescale.com> wrote:
> > >> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep
> subbaraya wrote:
> > >> > > > >> Hi,
> > >> > > > >>
> > >> > > > >>
> > >> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan
> <nathan.sullivan@ni.com> wrote:
> > >> > > > >> > The Xilinx Zynq udc does not need the
> > >> > > > >> > CI_HDRC_DISABLE_STREAMING flag, unlike the default
> > >> > > > >> > platform data.  Add platform data specific to the Zynq udc.
> > >> > > > >> >
> > >> > > > >> > Based on a patch by the same name from the Xilinx vendor
> tree.
> > >> > > > >>
> > >> > > > >> I am that Xilinx guy who sent this patch :). It is in
> > >> > > > >> Xilinx tree as temporary fix and I did not debug further
> > >> > > > >> why UDC works only when streaming is enabled.
> > >> > > > >> Probably this is right time to post my question here.
> > >> > > > >> I was expecting like:
> > >> > > > >> Streaming disabled - both low bandwidth and high bandwidth
> > >> > > > >> systems should work fine Streaming enabled - only for high
> > >> > > > >> bandwidth systems but this is not the case, Zynq UDC works
> > >> > > > >> only when Streaming is enabled.
> > >> > > > >> Please correct me if I am wrong.
> > >> > > > >
> > >> > > > > You are right, stream mode disabled should work at anytime.
> > >> > > > > It is so strange why zynq UDC only works when stream mode is
> enabled.
> > >> > > >
> > >> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS
> > >> > > > controllervDoc 2.20a,  this is what it says about SDIS
> > >> > > > (streaming mode disable option)
> > >> > > >
> > >> > > > Before activating this mode, the user must check if the TX
> > >> > > > latency buffers per endpoint are able to accommodate at least
> > >> > > > one entire maximum size packet. The RX buffer size must, at
> > >> > > > least, double the TX buffer size per endpoint. To optimize
> > >> > > > the stream disable performance, system bus burst must be set
> > >> > > > as high as possible.
> > >> > > > When the stream disable mode is used, the burst size
> > >> > > > (VUSB_HS_RX_BURST and VUSB_HS_TX_BURST) must be a
> integer
> > >> > > > sub-multiple of the latency buffer size (VUSB_HS_RX_DEPTH for
> > >> > > > RX buffer and VUSB_HS_TX_CHAN for the TX buffer). If this is
> > >> > > > not respected the controller will not work properly in stream
> > >> > > > disable mode.
> > >> > > > The stream disable mode should just be used in situations
> > >> > > > where the available system bandwidth is low or the system bus
> > >> > > > access latency is high, in order to avoid underruns and
> > >> > > > overruns in the latency buffers. This works for all types of
> > >> > > > endpoints, except for ISO endpoints.
> > >> > > > Such a system can't ensure the real time support that the ISO
> > >> > > > endpoints require, so the ISO endpoints are not supported
> > >> > > > when the SDIS bit is set.
> > >> > > >
> > >> > > > Definitely we need to root cause why disable streaming mode
> > >> > > > is not working for zynq but from controller spec point of
> > >> > > > view it is possible that controller not work properly in
> > >> > > > stream disable mode.
> > >> > > >
> > >> > > > Regards,
> > >> > > > Punnaiah
> > >> > > >
> > >> > >
> > >> > > Maybe the burst size isn't set correctly by default?  It does
> > >> > > say the controller won't work correctly with stream disable set
> > >> > > and an invalid burst size.  Looks like TX and RX burst both default
> to 16, per the Zynq manual.
> > >> > >
> > >> > > With the stream disable bit set, the behvior we see on our
> > >> > > hardware is that priming just stops, with an outstanding
> > >> > > transfer in memory marked active in the status field by the
> > >> > > controller.  This happens at random, even when doing single
> > >> > > transfers at a time like with g_ether set to have a queue size
> > >> > > of 1.  With SDIS clear everything works great.  Given that the Zynq
> is not bandwidth constrained, it seems like SDIS clear should be the default.
> > >> > >
> > >> >
> > >> > I suspect the possible reason is the tx buffer for each endpoint
> > >> > is small (<=512 bytes), so it can't copy one packet (assume max
> > >> > packet size for bulk) to tx buffer, then the prime can't be finished.
> > >> >
> > >> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and
> > >> > DCCPARAMS ($BASE + 124)?
> > >> >
> > >> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) /
> DCCPARAMS.DEN) *
> > >> > (DWORD_PER_BYTES)
> > >> >
> > >> > DWORD_PER_BYTES is 4
> > >> >
> > >> > --
> > >> >
> > >> > Best Regards,
> > >> > Peter Chen
> > >>
> > >> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
> > >
> > > Are you sure you read correct address? Your DCCPARAMS means it is
> > > host capable, but not device capable.
> >
> > HWTXBUF is 0x8060A10
> > DCCPARAMS is 0x0000018C
> >
> > VUSB_HS_TX_ADD - 0xA
> > DEN - 0xC
> >
> > From the above formula tx buffer size is 341.33 bytes.
> >
> >
> 
> ok, so your platform can't use stream mode disabled.

This is patch can be applied then. Thank you all.

Sundeep 
> 
> --
> 
> Best Regards,
> Peter Chen

RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Subbaraya Sundeep Bhatta]

Hi Nathan and Peter,

Is this patch applied? Please let me know I have some other patches to send on top of this.

Thanks,
Sundeep

> -----Original Message-----
> From: Subbaraya Sundeep Bhatta
> Sent: Tuesday, September 01, 2015 12:22 PM
> To: 'Peter Chen'; Punnaiah Choudary Kalluri
> Cc: Nathan Sullivan; sundeep subbaraya; robh+dt@kernel.org;
> pawel.moll@arm.com; Mark Rutland; ijc+devicetree@hellion.org.uk; Kumar
> Gala; Greg Kroah-Hartman; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> 
> Hi,
> 
> > -----Original Message-----
> > From: Peter Chen [mailto:peter.chen@freescale.com]
> > Sent: Monday, August 31, 2015 7:59 AM
> > To: Punnaiah Choudary Kalluri
> > Cc: Nathan Sullivan; sundeep subbaraya; Subbaraya Sundeep Bhatta;
> > robh+dt@kernel.org; pawel.moll@arm.com; Mark Rutland;
> > ijc+devicetree@hellion.org.uk; Kumar Gala; Greg Kroah-Hartman;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> > usb@vger.kernel.org
> > Subject: Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> >
> > On Mon, Aug 31, 2015 at 09:01:51AM +0530, punnaiah choudary kalluri
> > wrote:
> > > On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen
> > <peter.chen@freescale.com> wrote:
> > > > On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> > > >> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> > > >> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > > >> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary
> > kalluri wrote:
> > > >> > > > Hi,
> > > >> > > >
> > > >> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen
> > <peter.chen@freescale.com> wrote:
> > > >> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep
> > subbaraya wrote:
> > > >> > > > >> Hi,
> > > >> > > > >>
> > > >> > > > >>
> > > >> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan
> > <nathan.sullivan@ni.com> wrote:
> > > >> > > > >> > The Xilinx Zynq udc does not need the
> > > >> > > > >> > CI_HDRC_DISABLE_STREAMING flag, unlike the default
> > > >> > > > >> > platform data.  Add platform data specific to the Zynq udc.
> > > >> > > > >> >
> > > >> > > > >> > Based on a patch by the same name from the Xilinx
> > > >> > > > >> > vendor
> > tree.
> > > >> > > > >>
> > > >> > > > >> I am that Xilinx guy who sent this patch :). It is in
> > > >> > > > >> Xilinx tree as temporary fix and I did not debug further
> > > >> > > > >> why UDC works only when streaming is enabled.
> > > >> > > > >> Probably this is right time to post my question here.
> > > >> > > > >> I was expecting like:
> > > >> > > > >> Streaming disabled - both low bandwidth and high
> > > >> > > > >> bandwidth systems should work fine Streaming enabled -
> > > >> > > > >> only for high bandwidth systems but this is not the
> > > >> > > > >> case, Zynq UDC works only when Streaming is enabled.
> > > >> > > > >> Please correct me if I am wrong.
> > > >> > > > >
> > > >> > > > > You are right, stream mode disabled should work at anytime.
> > > >> > > > > It is so strange why zynq UDC only works when stream mode
> > > >> > > > > is
> > enabled.
> > > >> > > >
> > > >> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS
> > > >> > > > controllervDoc 2.20a,  this is what it says about SDIS
> > > >> > > > (streaming mode disable option)
> > > >> > > >
> > > >> > > > Before activating this mode, the user must check if the TX
> > > >> > > > latency buffers per endpoint are able to accommodate at
> > > >> > > > least one entire maximum size packet. The RX buffer size
> > > >> > > > must, at least, double the TX buffer size per endpoint. To
> > > >> > > > optimize the stream disable performance, system bus burst
> > > >> > > > must be set as high as possible.
> > > >> > > > When the stream disable mode is used, the burst size
> > > >> > > > (VUSB_HS_RX_BURST and VUSB_HS_TX_BURST) must be a
> > integer
> > > >> > > > sub-multiple of the latency buffer size (VUSB_HS_RX_DEPTH
> > > >> > > > for RX buffer and VUSB_HS_TX_CHAN for the TX buffer). If
> > > >> > > > this is not respected the controller will not work properly
> > > >> > > > in stream disable mode.
> > > >> > > > The stream disable mode should just be used in situations
> > > >> > > > where the available system bandwidth is low or the system
> > > >> > > > bus access latency is high, in order to avoid underruns and
> > > >> > > > overruns in the latency buffers. This works for all types
> > > >> > > > of endpoints, except for ISO endpoints.
> > > >> > > > Such a system can't ensure the real time support that the
> > > >> > > > ISO endpoints require, so the ISO endpoints are not
> > > >> > > > supported when the SDIS bit is set.
> > > >> > > >
> > > >> > > > Definitely we need to root cause why disable streaming mode
> > > >> > > > is not working for zynq but from controller spec point of
> > > >> > > > view it is possible that controller not work properly in
> > > >> > > > stream disable mode.
> > > >> > > >
> > > >> > > > Regards,
> > > >> > > > Punnaiah
> > > >> > > >
> > > >> > >
> > > >> > > Maybe the burst size isn't set correctly by default?  It does
> > > >> > > say the controller won't work correctly with stream disable
> > > >> > > set and an invalid burst size.  Looks like TX and RX burst
> > > >> > > both default
> > to 16, per the Zynq manual.
> > > >> > >
> > > >> > > With the stream disable bit set, the behvior we see on our
> > > >> > > hardware is that priming just stops, with an outstanding
> > > >> > > transfer in memory marked active in the status field by the
> > > >> > > controller.  This happens at random, even when doing single
> > > >> > > transfers at a time like with g_ether set to have a queue
> > > >> > > size of 1.  With SDIS clear everything works great.  Given
> > > >> > > that the Zynq
> > is not bandwidth constrained, it seems like SDIS clear should be the default.
> > > >> > >
> > > >> >
> > > >> > I suspect the possible reason is the tx buffer for each
> > > >> > endpoint is small (<=512 bytes), so it can't copy one packet
> > > >> > (assume max packet size for bulk) to tx buffer, then the prime can't be
> finished.
> > > >> >
> > > >> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and
> > > >> > DCCPARAMS ($BASE + 124)?
> > > >> >
> > > >> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) /
> > DCCPARAMS.DEN) *
> > > >> > (DWORD_PER_BYTES)
> > > >> >
> > > >> > DWORD_PER_BYTES is 4
> > > >> >
> > > >> > --
> > > >> >
> > > >> > Best Regards,
> > > >> > Peter Chen
> > > >>
> > > >> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
> > > >
> > > > Are you sure you read correct address? Your DCCPARAMS means it is
> > > > host capable, but not device capable.
> > >
> > > HWTXBUF is 0x8060A10
> > > DCCPARAMS is 0x0000018C
> > >
> > > VUSB_HS_TX_ADD - 0xA
> > > DEN - 0xC
> > >
> > > From the above formula tx buffer size is 341.33 bytes.
> > >
> > >
> >
> > ok, so your platform can't use stream mode disabled.
> 
> This is patch can be applied then. Thank you all.
> 
> Sundeep
> >
> > --
> >
> > Best Regards,
> > Peter Chen

RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Subbaraya Sundeep Bhatta]

Hi Nathan and Peter,

Is this patch applied? Please let me know I have some other patches to send on top of this.

Thanks,
Sundeep

> -----Original Message-----
> From: Subbaraya Sundeep Bhatta
> Sent: Tuesday, September 01, 2015 12:22 PM
> To: 'Peter Chen'; Punnaiah Choudary Kalluri
> Cc: Nathan Sullivan; sundeep subbaraya; robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org;
> pawel.moll-5wv7dgnIgG8@public.gmane.org; Mark Rutland; ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org; Kumar
> Gala; Greg Kroah-Hartman; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> 
> Hi,
> 
> > -----Original Message-----
> > From: Peter Chen [mailto:peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org]
> > Sent: Monday, August 31, 2015 7:59 AM
> > To: Punnaiah Choudary Kalluri
> > Cc: Nathan Sullivan; sundeep subbaraya; Subbaraya Sundeep Bhatta;
> > robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; pawel.moll-5wv7dgnIgG8@public.gmane.org; Mark Rutland;
> > ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org; Kumar Gala; Greg Kroah-Hartman;
> > devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> > usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Subject: Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> >
> > On Mon, Aug 31, 2015 at 09:01:51AM +0530, punnaiah choudary kalluri
> > wrote:
> > > On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen
> > <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > > > On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> > > >> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> > > >> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > > >> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary
> > kalluri wrote:
> > > >> > > > Hi,
> > > >> > > >
> > > >> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen
> > <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > > >> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep
> > subbaraya wrote:
> > > >> > > > >> Hi,
> > > >> > > > >>
> > > >> > > > >>
> > > >> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan
> > <nathan.sullivan-acOepvfBmUk@public.gmane.org> wrote:
> > > >> > > > >> > The Xilinx Zynq udc does not need the
> > > >> > > > >> > CI_HDRC_DISABLE_STREAMING flag, unlike the default
> > > >> > > > >> > platform data.  Add platform data specific to the Zynq udc.
> > > >> > > > >> >
> > > >> > > > >> > Based on a patch by the same name from the Xilinx
> > > >> > > > >> > vendor
> > tree.
> > > >> > > > >>
> > > >> > > > >> I am that Xilinx guy who sent this patch :). It is in
> > > >> > > > >> Xilinx tree as temporary fix and I did not debug further
> > > >> > > > >> why UDC works only when streaming is enabled.
> > > >> > > > >> Probably this is right time to post my question here.
> > > >> > > > >> I was expecting like:
> > > >> > > > >> Streaming disabled - both low bandwidth and high
> > > >> > > > >> bandwidth systems should work fine Streaming enabled -
> > > >> > > > >> only for high bandwidth systems but this is not the
> > > >> > > > >> case, Zynq UDC works only when Streaming is enabled.
> > > >> > > > >> Please correct me if I am wrong.
> > > >> > > > >
> > > >> > > > > You are right, stream mode disabled should work at anytime.
> > > >> > > > > It is so strange why zynq UDC only works when stream mode
> > > >> > > > > is
> > enabled.
> > > >> > > >
> > > >> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS
> > > >> > > > controllervDoc 2.20a,  this is what it says about SDIS
> > > >> > > > (streaming mode disable option)
> > > >> > > >
> > > >> > > > Before activating this mode, the user must check if the TX
> > > >> > > > latency buffers per endpoint are able to accommodate at
> > > >> > > > least one entire maximum size packet. The RX buffer size
> > > >> > > > must, at least, double the TX buffer size per endpoint. To
> > > >> > > > optimize the stream disable performance, system bus burst
> > > >> > > > must be set as high as possible.
> > > >> > > > When the stream disable mode is used, the burst size
> > > >> > > > (VUSB_HS_RX_BURST and VUSB_HS_TX_BURST) must be a
> > integer
> > > >> > > > sub-multiple of the latency buffer size (VUSB_HS_RX_DEPTH
> > > >> > > > for RX buffer and VUSB_HS_TX_CHAN for the TX buffer). If
> > > >> > > > this is not respected the controller will not work properly
> > > >> > > > in stream disable mode.
> > > >> > > > The stream disable mode should just be used in situations
> > > >> > > > where the available system bandwidth is low or the system
> > > >> > > > bus access latency is high, in order to avoid underruns and
> > > >> > > > overruns in the latency buffers. This works for all types
> > > >> > > > of endpoints, except for ISO endpoints.
> > > >> > > > Such a system can't ensure the real time support that the
> > > >> > > > ISO endpoints require, so the ISO endpoints are not
> > > >> > > > supported when the SDIS bit is set.
> > > >> > > >
> > > >> > > > Definitely we need to root cause why disable streaming mode
> > > >> > > > is not working for zynq but from controller spec point of
> > > >> > > > view it is possible that controller not work properly in
> > > >> > > > stream disable mode.
> > > >> > > >
> > > >> > > > Regards,
> > > >> > > > Punnaiah
> > > >> > > >
> > > >> > >
> > > >> > > Maybe the burst size isn't set correctly by default?  It does
> > > >> > > say the controller won't work correctly with stream disable
> > > >> > > set and an invalid burst size.  Looks like TX and RX burst
> > > >> > > both default
> > to 16, per the Zynq manual.
> > > >> > >
> > > >> > > With the stream disable bit set, the behvior we see on our
> > > >> > > hardware is that priming just stops, with an outstanding
> > > >> > > transfer in memory marked active in the status field by the
> > > >> > > controller.  This happens at random, even when doing single
> > > >> > > transfers at a time like with g_ether set to have a queue
> > > >> > > size of 1.  With SDIS clear everything works great.  Given
> > > >> > > that the Zynq
> > is not bandwidth constrained, it seems like SDIS clear should be the default.
> > > >> > >
> > > >> >
> > > >> > I suspect the possible reason is the tx buffer for each
> > > >> > endpoint is small (<=512 bytes), so it can't copy one packet
> > > >> > (assume max packet size for bulk) to tx buffer, then the prime can't be
> finished.
> > > >> >
> > > >> > Would you help to dump the registers HWTXBUF ($BASE + 0x10) and
> > > >> > DCCPARAMS ($BASE + 124)?
> > > >> >
> > > >> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) /
> > DCCPARAMS.DEN) *
> > > >> > (DWORD_PER_BYTES)
> > > >> >
> > > >> > DWORD_PER_BYTES is 4
> > > >> >
> > > >> > --
> > > >> >
> > > >> > Best Regards,
> > > >> > Peter Chen
> > > >>
> > > >> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
> > > >
> > > > Are you sure you read correct address? Your DCCPARAMS means it is
> > > > host capable, but not device capable.
> > >
> > > HWTXBUF is 0x8060A10
> > > DCCPARAMS is 0x0000018C
> > >
> > > VUSB_HS_TX_ADD - 0xA
> > > DEN - 0xC
> > >
> > > From the above formula tx buffer size is 341.33 bytes.
> > >
> > >
> >
> > ok, so your platform can't use stream mode disabled.
> 
> This is patch can be applied then. Thank you all.
> 
> Sundeep
> >
> > --
> >
> > Best Regards,
> > Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data

[Subbaraya Sundeep Bhatta]

Please ignore got the answer.

> -----Original Message-----
> From: Subbaraya Sundeep Bhatta
> Sent: Wednesday, September 23, 2015 12:29 PM
> To: 'Peter Chen'; Punnaiah Choudary Kalluri
> Cc: 'Nathan Sullivan'; 'sundeep subbaraya'; 'robh+dt@kernel.org';
> 'pawel.moll@arm.com'; 'Mark Rutland'; 'ijc+devicetree@hellion.org.uk'; 'Kumar
> Gala'; 'Greg Kroah-Hartman'; 'devicetree@vger.kernel.org'; 'linux-
> kernel@vger.kernel.org'; 'linux-usb@vger.kernel.org'
> Subject: RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> 
> Hi Nathan and Peter,
> 
> Is this patch applied? Please let me know I have some other patches to send on
> top of this.
> 
> Thanks,
> Sundeep
> 
> > -----Original Message-----
> > From: Subbaraya Sundeep Bhatta
> > Sent: Tuesday, September 01, 2015 12:22 PM
> > To: 'Peter Chen'; Punnaiah Choudary Kalluri
> > Cc: Nathan Sullivan; sundeep subbaraya; robh+dt@kernel.org;
> > pawel.moll@arm.com; Mark Rutland; ijc+devicetree@hellion.org.uk; Kumar
> > Gala; Greg Kroah-Hartman; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-usb@vger.kernel.org
> > Subject: RE: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Peter Chen [mailto:peter.chen@freescale.com]
> > > Sent: Monday, August 31, 2015 7:59 AM
> > > To: Punnaiah Choudary Kalluri
> > > Cc: Nathan Sullivan; sundeep subbaraya; Subbaraya Sundeep Bhatta;
> > > robh+dt@kernel.org; pawel.moll@arm.com; Mark Rutland;
> > > ijc+devicetree@hellion.org.uk; Kumar Gala; Greg Kroah-Hartman;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> > > usb@vger.kernel.org
> > > Subject: Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform
> > > data
> > >
> > > On Mon, Aug 31, 2015 at 09:01:51AM +0530, punnaiah choudary kalluri
> > > wrote:
> > > > On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen
> > > <peter.chen@freescale.com> wrote:
> > > > > On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> > > > >> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> > > > >> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > > > >> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary
> > > kalluri wrote:
> > > > >> > > > Hi,
> > > > >> > > >
> > > > >> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen
> > > <peter.chen@freescale.com> wrote:
> > > > >> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep
> > > subbaraya wrote:
> > > > >> > > > >> Hi,
> > > > >> > > > >>
> > > > >> > > > >>
> > > > >> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan
> > > <nathan.sullivan@ni.com> wrote:
> > > > >> > > > >> > The Xilinx Zynq udc does not need the
> > > > >> > > > >> > CI_HDRC_DISABLE_STREAMING flag, unlike the default
> > > > >> > > > >> > platform data.  Add platform data specific to the Zynq udc.
> > > > >> > > > >> >
> > > > >> > > > >> > Based on a patch by the same name from the Xilinx
> > > > >> > > > >> > vendor
> > > tree.
> > > > >> > > > >>
> > > > >> > > > >> I am that Xilinx guy who sent this patch :). It is in
> > > > >> > > > >> Xilinx tree as temporary fix and I did not debug
> > > > >> > > > >> further why UDC works only when streaming is enabled.
> > > > >> > > > >> Probably this is right time to post my question here.
> > > > >> > > > >> I was expecting like:
> > > > >> > > > >> Streaming disabled - both low bandwidth and high
> > > > >> > > > >> bandwidth systems should work fine Streaming enabled -
> > > > >> > > > >> only for high bandwidth systems but this is not the
> > > > >> > > > >> case, Zynq UDC works only when Streaming is enabled.
> > > > >> > > > >> Please correct me if I am wrong.
> > > > >> > > > >
> > > > >> > > > > You are right, stream mode disabled should work at anytime.
> > > > >> > > > > It is so strange why zynq UDC only works when stream
> > > > >> > > > > mode is
> > > enabled.
> > > > >> > > >
> > > > >> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS
> > > > >> > > > controllervDoc 2.20a,  this is what it says about SDIS
> > > > >> > > > (streaming mode disable option)
> > > > >> > > >
> > > > >> > > > Before activating this mode, the user must check if the
> > > > >> > > > TX latency buffers per endpoint are able to accommodate
> > > > >> > > > at least one entire maximum size packet. The RX buffer
> > > > >> > > > size must, at least, double the TX buffer size per
> > > > >> > > > endpoint. To optimize the stream disable performance,
> > > > >> > > > system bus burst must be set as high as possible.
> > > > >> > > > When the stream disable mode is used, the burst size
> > > > >> > > > (VUSB_HS_RX_BURST and VUSB_HS_TX_BURST) must be a
> > > integer
> > > > >> > > > sub-multiple of the latency buffer size (VUSB_HS_RX_DEPTH
> > > > >> > > > for RX buffer and VUSB_HS_TX_CHAN for the TX buffer). If
> > > > >> > > > this is not respected the controller will not work
> > > > >> > > > properly in stream disable mode.
> > > > >> > > > The stream disable mode should just be used in situations
> > > > >> > > > where the available system bandwidth is low or the system
> > > > >> > > > bus access latency is high, in order to avoid underruns
> > > > >> > > > and overruns in the latency buffers. This works for all
> > > > >> > > > types of endpoints, except for ISO endpoints.
> > > > >> > > > Such a system can't ensure the real time support that the
> > > > >> > > > ISO endpoints require, so the ISO endpoints are not
> > > > >> > > > supported when the SDIS bit is set.
> > > > >> > > >
> > > > >> > > > Definitely we need to root cause why disable streaming
> > > > >> > > > mode is not working for zynq but from controller spec
> > > > >> > > > point of view it is possible that controller not work
> > > > >> > > > properly in stream disable mode.
> > > > >> > > >
> > > > >> > > > Regards,
> > > > >> > > > Punnaiah
> > > > >> > > >
> > > > >> > >
> > > > >> > > Maybe the burst size isn't set correctly by default?  It
> > > > >> > > does say the controller won't work correctly with stream
> > > > >> > > disable set and an invalid burst size.  Looks like TX and
> > > > >> > > RX burst both default
> > > to 16, per the Zynq manual.
> > > > >> > >
> > > > >> > > With the stream disable bit set, the behvior we see on our
> > > > >> > > hardware is that priming just stops, with an outstanding
> > > > >> > > transfer in memory marked active in the status field by the
> > > > >> > > controller.  This happens at random, even when doing single
> > > > >> > > transfers at a time like with g_ether set to have a queue
> > > > >> > > size of 1.  With SDIS clear everything works great.  Given
> > > > >> > > that the Zynq
> > > is not bandwidth constrained, it seems like SDIS clear should be the default.
> > > > >> > >
> > > > >> >
> > > > >> > I suspect the possible reason is the tx buffer for each
> > > > >> > endpoint is small (<=512 bytes), so it can't copy one packet
> > > > >> > (assume max packet size for bulk) to tx buffer, then the
> > > > >> > prime can't be
> > finished.
> > > > >> >
> > > > >> > Would you help to dump the registers HWTXBUF ($BASE + 0x10)
> > > > >> > and DCCPARAMS ($BASE + 124)?
> > > > >> >
> > > > >> > tx buffer size = ((2 ^ HWTXBUF.VUSB_HS_TX_ADD) /
> > > DCCPARAMS.DEN) *
> > > > >> > (DWORD_PER_BYTES)
> > > > >> >
> > > > >> > DWORD_PER_BYTES is 4
> > > > >> >
> > > > >> > --
> > > > >> >
> > > > >> > Best Regards,
> > > > >> > Peter Chen
> > > > >>
> > > > >> HWTXBUF is 0x80060A10, DCCPARAMS is 0xE0003124.
> > > > >
> > > > > Are you sure you read correct address? Your DCCPARAMS means it
> > > > > is host capable, but not device capable.
> > > >
> > > > HWTXBUF is 0x8060A10
> > > > DCCPARAMS is 0x0000018C
> > > >
> > > > VUSB_HS_TX_ADD - 0xA
> > > > DEN - 0xC
> > > >
> > > > From the above formula tx buffer size is 341.33 bytes.
> > > >
> > > >
> > >
> > > ok, so your platform can't use stream mode disabled.
> >
> > This is patch can be applied then. Thank you all.
> >
> > Sundeep
> > >
> > > --
> > >
> > > Best Regards,
> > > Peter Chen