RE: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-02-25 Thread Manish Narani
HI Michael,

> -Original Message-
> From: Michael Grzeschik 
> Sent: Monday, February 22, 2021 9:01 PM
> To: Manish Narani 
> Cc: gre...@linuxfoundation.org; robh...@kernel.org; Michal Simek
> ; ba...@kernel.org; p.za...@pengutronix.de;
> devicet...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; git ; linux-arm-
> ker...@lists.infradead.org
> Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
> platforms
> 
> Hi Manish!
> 
> On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:
> >Add a new driver for supporting Xilinx platforms. This driver is used
> >for some sequence of operations required for Xilinx USB controllers.
> >This driver is also used to choose between PIPE clock coming from SerDes
> >and the Suspend Clock. Before the controller is out of reset, the clock
> >selection should be changed to PIPE clock in order to make the USB
> >controller work. There is a register added in Xilinx USB controller
> >register space for the same.
> >
> >Signed-off-by: Manish Narani 
> >---
> > drivers/usb/dwc3/Kconfig  |   9 +
> > drivers/usb/dwc3/Makefile |   1 +
> > drivers/usb/dwc3/dwc3-of-simple.c |   1 -
> > drivers/usb/dwc3/dwc3-xilinx.c| 334
> ++
> > 4 files changed, 344 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c
> >
> >diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> >index 7a2304565a73..0e00e6dfccd8 100644
> >--- a/drivers/usb/dwc3/Kconfig
> >+++ b/drivers/usb/dwc3/Kconfig
> >@@ -139,4 +139,13 @@ config USB_DWC3_QCOM
> >   for peripheral mode support.
> >   Say 'Y' or 'M' if you have one such device.
> >
> >+config USB_DWC3_XILINX
> >+tristate "Xilinx Platforms"
> >+depends on (ARCH_ZYNQMP || ARCH_VERSAL) && OF
> >+default USB_DWC3
> >+help
> >+  Support Xilinx SoCs with DesignWare Core USB3 IP.
> >+  This driver handles both ZynqMP and Versal SoC operations.
> >+  Say 'Y' or 'M' if you have one such device.
> >+
> > endif
> >diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> >index ae86da0dc5bd..add567578b1f 100644
> >--- a/drivers/usb/dwc3/Makefile
> >+++ b/drivers/usb/dwc3/Makefile
> >@@ -51,3 +51,4 @@ obj-$(CONFIG_USB_DWC3_MESON_G12A)  +=
> dwc3-meson-g12a.o
> > obj-$(CONFIG_USB_DWC3_OF_SIMPLE)+= dwc3-of-simple.o
> > obj-$(CONFIG_USB_DWC3_ST)   += dwc3-st.o
> > obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
> >+obj-$(CONFIG_USB_DWC3_XILINX)   += dwc3-xilinx.o
> >diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-
> of-simple.c
> >index e62ecd22b3ed..71fd620c5161 100644
> >--- a/drivers/usb/dwc3/dwc3-of-simple.c
> >+++ b/drivers/usb/dwc3/dwc3-of-simple.c
> >@@ -172,7 +172,6 @@ static const struct dev_pm_ops
> dwc3_of_simple_dev_pm_ops = {
> >
> > static const struct of_device_id of_dwc3_simple_match[] = {
> > { .compatible = "rockchip,rk3399-dwc3" },
> >-{ .compatible = "xlnx,zynqmp-dwc3" },
> > { .compatible = "cavium,octeon-7130-usb-uctl" },
> > { .compatible = "sprd,sc9860-dwc3" },
> > { .compatible = "allwinner,sun50i-h6-dwc3" },
> >diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-
> xilinx.c
> >new file mode 100644
> >index ..7e485951d2f7
> >--- /dev/null
> >+++ b/drivers/usb/dwc3/dwc3-xilinx.c
> >@@ -0,0 +1,334 @@
> >+// SPDX-License-Identifier: GPL-2.0
> >+/**
> >+ * dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver
> >+ *
> >+ * Authors: Manish Narani 
> >+ *  Anurag Kumar Vulisha 
> >+ */
> >+
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+
> >+#include 
> >+
> >+/* USB phy reset mask register */
> >+#define XLNX_USB_PHY_RST_EN 0x001C
> >+#define XLNX_PHY_RST_MASK   0x1
> >+
> >+/* Xilinx USB 3.0 IP Register */
> >+#define XLNX_USB_TRAFFIC_ROUTE_CONFIG   0x005C
> >+#define XLNX_USB_TRAFFIC_ROUTE_FPD  0x1
> >+
> >+/* Versal USB Reset ID */
> >+#define VERSAL_USB_RESET_ID 0xC104036
> >+
> >+#define XLNX_USB_FPD_PIPE

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-02-22 Thread Michael Grzeschik

Hi Manish!

On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:

Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from SerDes
and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.

Signed-off-by: Manish Narani 
---
drivers/usb/dwc3/Kconfig  |   9 +
drivers/usb/dwc3/Makefile |   1 +
drivers/usb/dwc3/dwc3-of-simple.c |   1 -
drivers/usb/dwc3/dwc3-xilinx.c| 334 ++
4 files changed, 344 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 7a2304565a73..0e00e6dfccd8 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -139,4 +139,13 @@ config USB_DWC3_QCOM
  for peripheral mode support.
  Say 'Y' or 'M' if you have one such device.

+config USB_DWC3_XILINX
+   tristate "Xilinx Platforms"
+   depends on (ARCH_ZYNQMP || ARCH_VERSAL) && OF
+   default USB_DWC3
+   help
+ Support Xilinx SoCs with DesignWare Core USB3 IP.
+ This driver handles both ZynqMP and Versal SoC operations.
+ Say 'Y' or 'M' if you have one such device.
+
endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index ae86da0dc5bd..add567578b1f 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -51,3 +51,4 @@ obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o
obj-$(CONFIG_USB_DWC3_OF_SIMPLE)+= dwc3-of-simple.o
obj-$(CONFIG_USB_DWC3_ST)   += dwc3-st.o
obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
+obj-$(CONFIG_USB_DWC3_XILINX)  += dwc3-xilinx.o
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index e62ecd22b3ed..71fd620c5161 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {

static const struct of_device_id of_dwc3_simple_match[] = {
{ .compatible = "rockchip,rk3399-dwc3" },
-   { .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
{ .compatible = "sprd,sc9860-dwc3" },
{ .compatible = "allwinner,sun50i-h6-dwc3" },
diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
new file mode 100644
index ..7e485951d2f7
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver
+ *
+ * Authors: Manish Narani 
+ *  Anurag Kumar Vulisha 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* USB phy reset mask register */
+#define XLNX_USB_PHY_RST_EN0x001C
+#define XLNX_PHY_RST_MASK  0x1
+
+/* Xilinx USB 3.0 IP Register */
+#define XLNX_USB_TRAFFIC_ROUTE_CONFIG  0x005C
+#define XLNX_USB_TRAFFIC_ROUTE_FPD 0x1
+
+/* Versal USB Reset ID */
+#define VERSAL_USB_RESET_ID0xC104036
+
+#define XLNX_USB_FPD_PIPE_CLK  0x7c
+#define PIPE_CLK_DESELECT  1
+#define PIPE_CLK_SELECT0
+#define XLNX_USB_FPD_POWER_PRSNT   0x80
+#define PIPE_POWER_ON  1
+#define PIPE_POWER_OFF 0
+
+struct dwc3_xlnx {
+   int num_clocks;
+   struct clk_bulk_data*clks;
+   struct device   *dev;
+   void __iomem*regs;
+   int (*pltfm_init)(struct dwc3_xlnx *data);
+};
+
+static void dwc3_xlnx_mask_phy_rst(struct dwc3_xlnx *priv_data, bool mask)
+{
+   u32 reg;
+
+   /*
+* Enable or disable ULPI PHY reset from USB Controller.
+* This does not actually reset the phy, but just controls
+* whether USB controller can or cannot reset ULPI PHY.
+*/
+   reg = readl(priv_data->regs + XLNX_USB_PHY_RST_EN);
+
+   if (mask)
+   reg &= ~XLNX_PHY_RST_MASK;
+   else
+   reg |= XLNX_PHY_RST_MASK;
+
+   writel(reg, priv_data->regs + XLNX_USB_PHY_RST_EN);
+}
+
+static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data)
+{
+   struct device   *dev = priv_data->dev;
+   int ret;
+
+   dwc3_xlnx_mask_phy_rst(priv_data, false);
+
+   /* Assert and De-assert reset */
+   ret = zynqmp_pm_reset_assert(VERSAL_USB_RESET_ID,
+ 

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-02-10 Thread Michael Grzeschik

Adding Tinh Nguyen to CC:

On Tue, Feb 09, 2021 at 09:02:00PM +0100, Michael Grzeschik wrote:

Hi Manish,

On Tue, Feb 09, 2021 at 06:01:58AM +, Manish Narani wrote:

Hi Michael,


-Original Message-
From: Michael Grzeschik 
Sent: Tuesday, February 9, 2021 5:26 AM
To: Manish Narani 
Cc: devicet...@vger.kernel.org; p.za...@pengutronix.de; ba...@kernel.org;
gre...@linuxfoundation.org; linux-...@vger.kernel.org; linux-
ker...@vger.kernel.org; robh...@kernel.org; Michal Simek
; git ; ker...@pengutronix.de; linux-
arm-ker...@lists.infradead.org
Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
platforms

Hi Manish!

On Thu, Jan 28, 2021 at 12:36:07AM +0100, Michael Grzeschik wrote:

On Fri, Jan 22, 2021 at 02:34:52PM +0100, Michael Grzeschik wrote:

On Fri, Jan 22, 2021 at 01:06:22PM +, Manish Narani wrote:

Hi Michael,


-Original Message-
From: Michael Grzeschik 
Sent: Friday, January 22, 2021 1:39 PM
To: Manish Narani 
Cc: devicet...@vger.kernel.org; ker...@pengutronix.de;

ba...@kernel.org;

gre...@linuxfoundation.org; linux-...@vger.kernel.org; Michal Simek
; linux-kernel@vger.kernel.org;

robh...@kernel.org;

git ; p.za...@pengutronix.de; linux-arm-
ker...@lists.infradead.org
Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
platforms

Hello!

On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:

Hi!

On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:

Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from

SerDes

and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.


I tried out this driver with the vanilla kernel on an zynqmp. Without
this patch the USB-Gadget is already acting buggy. In the gadget mode,
some iterations of plug/unplug results to an stalled gadget which will
never come back without a reboot.

With the corresponding code of this driver (reset assert, clk modify,
reset deassert) in the downstream kernels phy driver we found out it is
totaly stable. But using this exact glue driver which should do the same
as the downstream code, the gadget still was buggy the way described
above.

I suspect the difference lays in the different order of operations.
While the downstream code is runing the resets inside the phy driver
which is powered and initialized in the dwc3-core itself. With this glue
layser approach of this patch the whole phy init is done before even
touching dwc3-core in any way. It seems not to have the same effect,
though.

If really the order of operations is limiting us, we probably need
another solution than this glue layer. Any Ideas?


I found out what the difference between the Downstream and this
Glue is. When using vanilla with this Glue code we may not set
the following bit:

https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-
ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html


+   /* Set PIPE Power Present signal in FPD Power Present

Register*/

+   writel(PIPE_POWER_ON, priv_data->regs +

XLNX_USB_FPD_POWER_PRSNT);

When I comment this out, the link stays stable. This is different in
the Downstream Xilinx Kernel, where the bit is also set but has no
negativ effect.

Manish, can you give me a pointer what to look for?
So setting this will also work with mainline?

I am looking further on this but from what I see here is that,
In order to make USB function properly, there are some dt changes

needed in mainline for

USB node which include defining clocks coming from serdes.
The DT changes are pending to be sent to mainline.


Can you push that state somewhere, so I could test it?
Or is in the downstream kernel some things to copy?


Can you share the DT settings for USB node on your side?


Here is my current configuration for the device node at usb0:

zynqmp.dtsi

zynqmp_reset: reset-controller {
compatible = "xlnx,zynqmp-reset";
#reset-cells = <1>;
};

usb0: usb@ff9d {
#address-cells = <2>;
#size-cells = <2>;
status = "disabled";
compatible = "xlnx,zynqmp-dwc3";
reg = <0x0 0xff9d 0x0 0x100>;
clock-names = "bus_clk", "ref_clk";
power-domains = <_firmware PD_USB_0>;
ranges;
resets = <_reset ZYNQMP_RESET_USB0_CORERESET>,
<_reset ZYNQMP_RESET_USB0_HIBERRESET>,
<_reset ZYNQMP_RESET_USB0_APB>;
reset-names = "usb_crst", "usb_hibrst", "usb_apbrst";
phy-names = "usb3-phy";
phys = < 2 PHY_TYPE_USB3 0 2>;

usb0_dwc3: dwc3@fe20 {

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-02-09 Thread Michael Grzeschik

Hi Manish,

On Tue, Feb 09, 2021 at 06:01:58AM +, Manish Narani wrote:

Hi Michael,


-Original Message-
From: Michael Grzeschik 
Sent: Tuesday, February 9, 2021 5:26 AM
To: Manish Narani 
Cc: devicet...@vger.kernel.org; p.za...@pengutronix.de; ba...@kernel.org;
gre...@linuxfoundation.org; linux-...@vger.kernel.org; linux-
ker...@vger.kernel.org; robh...@kernel.org; Michal Simek
; git ; ker...@pengutronix.de; linux-
arm-ker...@lists.infradead.org
Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
platforms

Hi Manish!

On Thu, Jan 28, 2021 at 12:36:07AM +0100, Michael Grzeschik wrote:
>On Fri, Jan 22, 2021 at 02:34:52PM +0100, Michael Grzeschik wrote:
>>On Fri, Jan 22, 2021 at 01:06:22PM +, Manish Narani wrote:
>>>Hi Michael,
>>>
>>>>-Original Message-
>>>>From: Michael Grzeschik 
>>>>Sent: Friday, January 22, 2021 1:39 PM
>>>>To: Manish Narani 
>>>>Cc: devicet...@vger.kernel.org; ker...@pengutronix.de;
ba...@kernel.org;
>>>>gre...@linuxfoundation.org; linux-...@vger.kernel.org; Michal Simek
>>>>; linux-kernel@vger.kernel.org;
robh...@kernel.org;
>>>>git ; p.za...@pengutronix.de; linux-arm-
>>>>ker...@lists.infradead.org
>>>>Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
>>>>platforms
>>>>
>>>>Hello!
>>>>
>>>>On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:
>>>>>Hi!
>>>>>
>>>>>On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:
>>>>>>Add a new driver for supporting Xilinx platforms. This driver is used
>>>>>>for some sequence of operations required for Xilinx USB controllers.
>>>>>>This driver is also used to choose between PIPE clock coming from
SerDes
>>>>>>and the Suspend Clock. Before the controller is out of reset, the clock
>>>>>>selection should be changed to PIPE clock in order to make the USB
>>>>>>controller work. There is a register added in Xilinx USB controller
>>>>>>register space for the same.
>>>>>
>>>>>I tried out this driver with the vanilla kernel on an zynqmp. Without
>>>>>this patch the USB-Gadget is already acting buggy. In the gadget mode,
>>>>>some iterations of plug/unplug results to an stalled gadget which will
>>>>>never come back without a reboot.
>>>>>
>>>>>With the corresponding code of this driver (reset assert, clk modify,
>>>>>reset deassert) in the downstream kernels phy driver we found out it is
>>>>>totaly stable. But using this exact glue driver which should do the same
>>>>>as the downstream code, the gadget still was buggy the way described
>>>>>above.
>>>>>
>>>>>I suspect the difference lays in the different order of operations.
>>>>>While the downstream code is runing the resets inside the phy driver
>>>>>which is powered and initialized in the dwc3-core itself. With this glue
>>>>>layser approach of this patch the whole phy init is done before even
>>>>>touching dwc3-core in any way. It seems not to have the same effect,
>>>>>though.
>>>>>
>>>>>If really the order of operations is limiting us, we probably need
>>>>>another solution than this glue layer. Any Ideas?
>>>>
>>>>I found out what the difference between the Downstream and this
>>>>Glue is. When using vanilla with this Glue code we may not set
>>>>the following bit:
>>>>
>>>>https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-
>>>>ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html
>>>>
>>>>>>+   /* Set PIPE Power Present signal in FPD Power Present
Register*/
>>>>>>+   writel(PIPE_POWER_ON, priv_data->regs +
>>>>XLNX_USB_FPD_POWER_PRSNT);
>>>>
>>>>When I comment this out, the link stays stable. This is different in
>>>>the Downstream Xilinx Kernel, where the bit is also set but has no
>>>>negativ effect.
>>>>
>>>>Manish, can you give me a pointer what to look for?
>>>>So setting this will also work with mainline?
>>>I am looking further on this but from what I see here is that,
>>>In order to make USB function properly, there are some dt changes
needed in mainline for
>>>USB node which include defining clocks coming from serdes.
>>>The DT chan

RE: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-02-08 Thread Manish Narani
Hi Michael,

> -Original Message-
> From: Michael Grzeschik 
> Sent: Tuesday, February 9, 2021 5:26 AM
> To: Manish Narani 
> Cc: devicet...@vger.kernel.org; p.za...@pengutronix.de; ba...@kernel.org;
> gre...@linuxfoundation.org; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; robh...@kernel.org; Michal Simek
> ; git ; ker...@pengutronix.de; linux-
> arm-ker...@lists.infradead.org
> Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
> platforms
> 
> Hi Manish!
> 
> On Thu, Jan 28, 2021 at 12:36:07AM +0100, Michael Grzeschik wrote:
> >On Fri, Jan 22, 2021 at 02:34:52PM +0100, Michael Grzeschik wrote:
> >>On Fri, Jan 22, 2021 at 01:06:22PM +, Manish Narani wrote:
> >>>Hi Michael,
> >>>
> >>>>-Original Message-
> >>>>From: Michael Grzeschik 
> >>>>Sent: Friday, January 22, 2021 1:39 PM
> >>>>To: Manish Narani 
> >>>>Cc: devicet...@vger.kernel.org; ker...@pengutronix.de;
> ba...@kernel.org;
> >>>>gre...@linuxfoundation.org; linux-...@vger.kernel.org; Michal Simek
> >>>>; linux-kernel@vger.kernel.org;
> robh...@kernel.org;
> >>>>git ; p.za...@pengutronix.de; linux-arm-
> >>>>ker...@lists.infradead.org
> >>>>Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
> >>>>platforms
> >>>>
> >>>>Hello!
> >>>>
> >>>>On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:
> >>>>>Hi!
> >>>>>
> >>>>>On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:
> >>>>>>Add a new driver for supporting Xilinx platforms. This driver is used
> >>>>>>for some sequence of operations required for Xilinx USB controllers.
> >>>>>>This driver is also used to choose between PIPE clock coming from
> SerDes
> >>>>>>and the Suspend Clock. Before the controller is out of reset, the clock
> >>>>>>selection should be changed to PIPE clock in order to make the USB
> >>>>>>controller work. There is a register added in Xilinx USB controller
> >>>>>>register space for the same.
> >>>>>
> >>>>>I tried out this driver with the vanilla kernel on an zynqmp. Without
> >>>>>this patch the USB-Gadget is already acting buggy. In the gadget mode,
> >>>>>some iterations of plug/unplug results to an stalled gadget which will
> >>>>>never come back without a reboot.
> >>>>>
> >>>>>With the corresponding code of this driver (reset assert, clk modify,
> >>>>>reset deassert) in the downstream kernels phy driver we found out it is
> >>>>>totaly stable. But using this exact glue driver which should do the same
> >>>>>as the downstream code, the gadget still was buggy the way described
> >>>>>above.
> >>>>>
> >>>>>I suspect the difference lays in the different order of operations.
> >>>>>While the downstream code is runing the resets inside the phy driver
> >>>>>which is powered and initialized in the dwc3-core itself. With this glue
> >>>>>layser approach of this patch the whole phy init is done before even
> >>>>>touching dwc3-core in any way. It seems not to have the same effect,
> >>>>>though.
> >>>>>
> >>>>>If really the order of operations is limiting us, we probably need
> >>>>>another solution than this glue layer. Any Ideas?
> >>>>
> >>>>I found out what the difference between the Downstream and this
> >>>>Glue is. When using vanilla with this Glue code we may not set
> >>>>the following bit:
> >>>>
> >>>>https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-
> >>>>ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html
> >>>>
> >>>>>>+   /* Set PIPE Power Present signal in FPD Power Present
> Register*/
> >>>>>>+   writel(PIPE_POWER_ON, priv_data->regs +
> >>>>XLNX_USB_FPD_POWER_PRSNT);
> >>>>
> >>>>When I comment this out, the link stays stable. This is different in
> >>>>the Downstream Xilinx Kernel, where the bit is also set but has no
> >>>>negativ effect.
> >>>>
> >>>>Manish, can you give me a po

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-02-08 Thread Michael Grzeschik

Hi Manish!

On Thu, Jan 28, 2021 at 12:36:07AM +0100, Michael Grzeschik wrote:

On Fri, Jan 22, 2021 at 02:34:52PM +0100, Michael Grzeschik wrote:

On Fri, Jan 22, 2021 at 01:06:22PM +, Manish Narani wrote:

Hi Michael,


-Original Message-
From: Michael Grzeschik 
Sent: Friday, January 22, 2021 1:39 PM
To: Manish Narani 
Cc: devicet...@vger.kernel.org; ker...@pengutronix.de; ba...@kernel.org;
gre...@linuxfoundation.org; linux-...@vger.kernel.org; Michal Simek
; linux-kernel@vger.kernel.org; robh...@kernel.org;
git ; p.za...@pengutronix.de; linux-arm-
ker...@lists.infradead.org
Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
platforms

Hello!

On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:

Hi!

On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:

Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from SerDes
and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.


I tried out this driver with the vanilla kernel on an zynqmp. Without
this patch the USB-Gadget is already acting buggy. In the gadget mode,
some iterations of plug/unplug results to an stalled gadget which will
never come back without a reboot.

With the corresponding code of this driver (reset assert, clk modify,
reset deassert) in the downstream kernels phy driver we found out it is
totaly stable. But using this exact glue driver which should do the same
as the downstream code, the gadget still was buggy the way described
above.

I suspect the difference lays in the different order of operations.
While the downstream code is runing the resets inside the phy driver
which is powered and initialized in the dwc3-core itself. With this glue
layser approach of this patch the whole phy init is done before even
touching dwc3-core in any way. It seems not to have the same effect,
though.

If really the order of operations is limiting us, we probably need
another solution than this glue layer. Any Ideas?


I found out what the difference between the Downstream and this
Glue is. When using vanilla with this Glue code we may not set
the following bit:

https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-
ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html


+   /* Set PIPE Power Present signal in FPD Power Present Register*/
+   writel(PIPE_POWER_ON, priv_data->regs +

XLNX_USB_FPD_POWER_PRSNT);

When I comment this out, the link stays stable. This is different in
the Downstream Xilinx Kernel, where the bit is also set but has no
negativ effect.

Manish, can you give me a pointer what to look for?
So setting this will also work with mainline?

I am looking further on this but from what I see here is that,
In order to make USB function properly, there are some dt changes needed in 
mainline for
USB node which include defining clocks coming from serdes.
The DT changes are pending to be sent to mainline.


Can you push that state somewhere, so I could test it?
Or is in the downstream kernel some things to copy?


Can you share the DT settings for USB node on your side?


Here is my current configuration for the device node at usb0:

zynqmp.dtsi

zynqmp_reset: reset-controller {
compatible = "xlnx,zynqmp-reset";
#reset-cells = <1>;
};

usb0: usb@ff9d {
#address-cells = <2>;
#size-cells = <2>;
status = "disabled";
compatible = "xlnx,zynqmp-dwc3";
reg = <0x0 0xff9d 0x0 0x100>;
clock-names = "bus_clk", "ref_clk";
power-domains = <_firmware PD_USB_0>;
ranges;
resets = <_reset ZYNQMP_RESET_USB0_CORERESET>,
<_reset ZYNQMP_RESET_USB0_HIBERRESET>,
<_reset ZYNQMP_RESET_USB0_APB>;
reset-names = "usb_crst", "usb_hibrst", "usb_apbrst";
phy-names = "usb3-phy";
phys = < 2 PHY_TYPE_USB3 0 2>;

usb0_dwc3: dwc3@fe20 {
compatible = "snps,dwc3";
interrupt-parent = <>;
interrupts = <0 65 4>;
clock-names = "ref", "bus_early", "suspend";
reg = <0x0 0xfe20 0x0 0x4>;
};
};

platform.dts

 {
status = "okay";
phy-names = "usb3-phy";
phys = < 2 PHY_TYPE_USB3 0 2>;
};

_dwc3 {
dr_mode = "peripheral";

/* The following quirks are required, since the bInterval is 1 and we
 * handle steady ISOC streaming. See Usecase 3 in commit 729dcf

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-01-27 Thread Michael Grzeschik

On Fri, Jan 22, 2021 at 02:34:52PM +0100, Michael Grzeschik wrote:

On Fri, Jan 22, 2021 at 01:06:22PM +, Manish Narani wrote:

Hi Michael,


-Original Message-
From: Michael Grzeschik 
Sent: Friday, January 22, 2021 1:39 PM
To: Manish Narani 
Cc: devicet...@vger.kernel.org; ker...@pengutronix.de; ba...@kernel.org;
gre...@linuxfoundation.org; linux-...@vger.kernel.org; Michal Simek
; linux-kernel@vger.kernel.org; robh...@kernel.org;
git ; p.za...@pengutronix.de; linux-arm-
ker...@lists.infradead.org
Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
platforms

Hello!

On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:

Hi!

On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:

Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from SerDes
and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.


I tried out this driver with the vanilla kernel on an zynqmp. Without
this patch the USB-Gadget is already acting buggy. In the gadget mode,
some iterations of plug/unplug results to an stalled gadget which will
never come back without a reboot.

With the corresponding code of this driver (reset assert, clk modify,
reset deassert) in the downstream kernels phy driver we found out it is
totaly stable. But using this exact glue driver which should do the same
as the downstream code, the gadget still was buggy the way described
above.

I suspect the difference lays in the different order of operations.
While the downstream code is runing the resets inside the phy driver
which is powered and initialized in the dwc3-core itself. With this glue
layser approach of this patch the whole phy init is done before even
touching dwc3-core in any way. It seems not to have the same effect,
though.

If really the order of operations is limiting us, we probably need
another solution than this glue layer. Any Ideas?


I found out what the difference between the Downstream and this
Glue is. When using vanilla with this Glue code we may not set
the following bit:

https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-
ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html


+   /* Set PIPE Power Present signal in FPD Power Present Register*/
+   writel(PIPE_POWER_ON, priv_data->regs +

XLNX_USB_FPD_POWER_PRSNT);

When I comment this out, the link stays stable. This is different in
the Downstream Xilinx Kernel, where the bit is also set but has no
negativ effect.

Manish, can you give me a pointer what to look for?
So setting this will also work with mainline?

I am looking further on this but from what I see here is that,
In order to make USB function properly, there are some dt changes needed in 
mainline for
USB node which include defining clocks coming from serdes.
The DT changes are pending to be sent to mainline.


Can you push that state somewhere, so I could test it?
Or is in the downstream kernel some things to copy?


Can you share the DT settings for USB node on your side?


Here is my current configuration for the device node at usb0:

zynqmp.dtsi

zynqmp_reset: reset-controller {
compatible = "xlnx,zynqmp-reset";
#reset-cells = <1>;
};

usb0: usb@ff9d {
#address-cells = <2>;
#size-cells = <2>;
status = "disabled";
compatible = "xlnx,zynqmp-dwc3";
reg = <0x0 0xff9d 0x0 0x100>;
clock-names = "bus_clk", "ref_clk";
power-domains = <_firmware PD_USB_0>;
ranges;
resets = <_reset ZYNQMP_RESET_USB0_CORERESET>,
<_reset ZYNQMP_RESET_USB0_HIBERRESET>,
<_reset ZYNQMP_RESET_USB0_APB>;
reset-names = "usb_crst", "usb_hibrst", "usb_apbrst";
phy-names = "usb3-phy";
phys = < 2 PHY_TYPE_USB3 0 2>;

usb0_dwc3: dwc3@fe20 {
compatible = "snps,dwc3";
interrupt-parent = <>;
interrupts = <0 65 4>;
clock-names = "ref", "bus_early", "suspend";
reg = <0x0 0xfe20 0x0 0x4>;
};
};

platform.dts

 {
status = "okay";
phy-names = "usb3-phy";
phys = < 2 PHY_TYPE_USB3 0 2>;
};

_dwc3 {
dr_mode = "peripheral";

/* The following quirks are required, since the bInterval is 1 and we
 * handle steady ISOC streaming. See Usecase 3 in commit 729dcffd1ed3
 * ("usb: dwc3: gadget: Add support for disabling U1 and U

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-01-22 Thread Michael Grzeschik

On Fri, Jan 22, 2021 at 01:06:22PM +, Manish Narani wrote:

Hi Michael,


-Original Message-
From: Michael Grzeschik 
Sent: Friday, January 22, 2021 1:39 PM
To: Manish Narani 
Cc: devicet...@vger.kernel.org; ker...@pengutronix.de; ba...@kernel.org;
gre...@linuxfoundation.org; linux-...@vger.kernel.org; Michal Simek
; linux-kernel@vger.kernel.org; robh...@kernel.org;
git ; p.za...@pengutronix.de; linux-arm-
ker...@lists.infradead.org
Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
platforms

Hello!

On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:
>Hi!
>
>On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:
>>Add a new driver for supporting Xilinx platforms. This driver is used
>>for some sequence of operations required for Xilinx USB controllers.
>>This driver is also used to choose between PIPE clock coming from SerDes
>>and the Suspend Clock. Before the controller is out of reset, the clock
>>selection should be changed to PIPE clock in order to make the USB
>>controller work. There is a register added in Xilinx USB controller
>>register space for the same.
>
>I tried out this driver with the vanilla kernel on an zynqmp. Without
>this patch the USB-Gadget is already acting buggy. In the gadget mode,
>some iterations of plug/unplug results to an stalled gadget which will
>never come back without a reboot.
>
>With the corresponding code of this driver (reset assert, clk modify,
>reset deassert) in the downstream kernels phy driver we found out it is
>totaly stable. But using this exact glue driver which should do the same
>as the downstream code, the gadget still was buggy the way described
>above.
>
>I suspect the difference lays in the different order of operations.
>While the downstream code is runing the resets inside the phy driver
>which is powered and initialized in the dwc3-core itself. With this glue
>layser approach of this patch the whole phy init is done before even
>touching dwc3-core in any way. It seems not to have the same effect,
>though.
>
>If really the order of operations is limiting us, we probably need
>another solution than this glue layer. Any Ideas?

I found out what the difference between the Downstream and this
Glue is. When using vanilla with this Glue code we may not set
the following bit:

https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-
ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html

>>+   /* Set PIPE Power Present signal in FPD Power Present Register*/
>>+   writel(PIPE_POWER_ON, priv_data->regs +
XLNX_USB_FPD_POWER_PRSNT);

When I comment this out, the link stays stable. This is different in
the Downstream Xilinx Kernel, where the bit is also set but has no
negativ effect.

Manish, can you give me a pointer what to look for?
So setting this will also work with mainline?

I am looking further on this but from what I see here is that,
In order to make USB function properly, there are some dt changes needed in 
mainline for
USB node which include defining clocks coming from serdes.
The DT changes are pending to be sent to mainline.


Can you push that state somewhere, so I could test it?
Or is in the downstream kernel some things to copy?


Can you share the DT settings for USB node on your side?


Here is my current configuration for the device node at usb0:

zynqmp.dtsi

zynqmp_reset: reset-controller {
compatible = "xlnx,zynqmp-reset";
#reset-cells = <1>;
};

usb0: usb@ff9d {
#address-cells = <2>;
#size-cells = <2>;
status = "disabled";
compatible = "xlnx,zynqmp-dwc3";
reg = <0x0 0xff9d 0x0 0x100>;
clock-names = "bus_clk", "ref_clk";
power-domains = <_firmware PD_USB_0>;
ranges;
resets = <_reset ZYNQMP_RESET_USB0_CORERESET>,
<_reset ZYNQMP_RESET_USB0_HIBERRESET>,
<_reset ZYNQMP_RESET_USB0_APB>;
reset-names = "usb_crst", "usb_hibrst", "usb_apbrst";
phy-names = "usb3-phy";
phys = < 2 PHY_TYPE_USB3 0 2>;

usb0_dwc3: dwc3@fe20 {
compatible = "snps,dwc3";
interrupt-parent = <>;
interrupts = <0 65 4>;
clock-names = "ref", "bus_early", "suspend";
reg = <0x0 0xfe20 0x0 0x4>;
};
};

platform.dts

 {
status = "okay";
phy-names = "usb3-phy";
phys = < 2 PHY_TYPE_USB3 0 2>;
};

_dwc3 {
dr_mode = "peripheral";

/* The following quirks are required, since the bInterval is 1 and we
 * handle steady ISOC streaming. See Usecase 3 in 

RE: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-01-22 Thread Manish Narani
Hi Michael,

> -Original Message-
> From: Michael Grzeschik 
> Sent: Friday, January 22, 2021 1:39 PM
> To: Manish Narani 
> Cc: devicet...@vger.kernel.org; ker...@pengutronix.de; ba...@kernel.org;
> gre...@linuxfoundation.org; linux-...@vger.kernel.org; Michal Simek
> ; linux-kernel@vger.kernel.org; robh...@kernel.org;
> git ; p.za...@pengutronix.de; linux-arm-
> ker...@lists.infradead.org
> Subject: Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx
> platforms
> 
> Hello!
> 
> On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:
> >Hi!
> >
> >On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:
> >>Add a new driver for supporting Xilinx platforms. This driver is used
> >>for some sequence of operations required for Xilinx USB controllers.
> >>This driver is also used to choose between PIPE clock coming from SerDes
> >>and the Suspend Clock. Before the controller is out of reset, the clock
> >>selection should be changed to PIPE clock in order to make the USB
> >>controller work. There is a register added in Xilinx USB controller
> >>register space for the same.
> >
> >I tried out this driver with the vanilla kernel on an zynqmp. Without
> >this patch the USB-Gadget is already acting buggy. In the gadget mode,
> >some iterations of plug/unplug results to an stalled gadget which will
> >never come back without a reboot.
> >
> >With the corresponding code of this driver (reset assert, clk modify,
> >reset deassert) in the downstream kernels phy driver we found out it is
> >totaly stable. But using this exact glue driver which should do the same
> >as the downstream code, the gadget still was buggy the way described
> >above.
> >
> >I suspect the difference lays in the different order of operations.
> >While the downstream code is runing the resets inside the phy driver
> >which is powered and initialized in the dwc3-core itself. With this glue
> >layser approach of this patch the whole phy init is done before even
> >touching dwc3-core in any way. It seems not to have the same effect,
> >though.
> >
> >If really the order of operations is limiting us, we probably need
> >another solution than this glue layer. Any Ideas?
> 
> I found out what the difference between the Downstream and this
> Glue is. When using vanilla with this Glue code we may not set
> the following bit:
> 
> https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-
> ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html
> 
> >>+   /* Set PIPE Power Present signal in FPD Power Present Register*/
> >>+   writel(PIPE_POWER_ON, priv_data->regs +
> XLNX_USB_FPD_POWER_PRSNT);
> 
> When I comment this out, the link stays stable. This is different in
> the Downstream Xilinx Kernel, where the bit is also set but has no
> negativ effect.
> 
> Manish, can you give me a pointer what to look for?
> So setting this will also work with mainline?
I am looking further on this but from what I see here is that,
In order to make USB function properly, there are some dt changes needed in 
mainline for
USB node which include defining clocks coming from serdes.
The DT changes are pending to be sent to mainline.
Can you share the DT settings for USB node on your side?
Meanwhile I will keep updating on the same.

Thanks,
Manish

> 
> Regards,
> Michael
> 
> >>
> >>Signed-off-by: Manish Narani 
> >>---
> >>drivers/usb/dwc3/Kconfig  |   9 +
> >>drivers/usb/dwc3/Makefile |   1 +
> >>drivers/usb/dwc3/dwc3-of-simple.c |   1 -
> >>drivers/usb/dwc3/dwc3-xilinx.c| 334
> ++
> >>4 files changed, 344 insertions(+), 1 deletion(-)
> >>create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c
> >>
> >>diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> >>index 7a2304565a73..0e00e6dfccd8 100644
> >>--- a/drivers/usb/dwc3/Kconfig
> >>+++ b/drivers/usb/dwc3/Kconfig
> >>@@ -139,4 +139,13 @@ config USB_DWC3_QCOM
> >>  for peripheral mode support.
> >>  Say 'Y' or 'M' if you have one such device.
> >>
> >>+config USB_DWC3_XILINX
> >>+   tristate "Xilinx Platforms"
> >>+   depends on (ARCH_ZYNQMP || ARCH_VERSAL) && OF
> >>+   default USB_DWC3
> >>+   help
> >>+ Support Xilinx SoCs with DesignWare Core USB3 IP.
> >>+ This driver handles both ZynqMP and Versal SoC operations.
> >>+ Say 'Y' or 'M' if you have one such device.
> >>+
> >>endif
> >>diff --git a/

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-01-22 Thread Michael Grzeschik

Hello!

On Mon, Jan 18, 2021 at 02:42:24PM +0100, Michael Grzeschik wrote:

Hi!

On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:

Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from SerDes
and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.


I tried out this driver with the vanilla kernel on an zynqmp. Without
this patch the USB-Gadget is already acting buggy. In the gadget mode,
some iterations of plug/unplug results to an stalled gadget which will
never come back without a reboot.

With the corresponding code of this driver (reset assert, clk modify,
reset deassert) in the downstream kernels phy driver we found out it is
totaly stable. But using this exact glue driver which should do the same
as the downstream code, the gadget still was buggy the way described
above.

I suspect the difference lays in the different order of operations.
While the downstream code is runing the resets inside the phy driver
which is powered and initialized in the dwc3-core itself. With this glue
layser approach of this patch the whole phy init is done before even
touching dwc3-core in any way. It seems not to have the same effect,
though.

If really the order of operations is limiting us, we probably need
another solution than this glue layer. Any Ideas?


I found out what the difference between the Downstream and this
Glue is. When using vanilla with this Glue code we may not set
the following bit:

https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html#usb3_regs___fpd_power_prsnt.html


+   /* Set PIPE Power Present signal in FPD Power Present Register*/
+   writel(PIPE_POWER_ON, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);


When I comment this out, the link stays stable. This is different in
the Downstream Xilinx Kernel, where the bit is also set but has no
negativ effect.

Manish, can you give me a pointer what to look for?
So setting this will also work with mainline?

Regards,
Michael



Signed-off-by: Manish Narani 
---
drivers/usb/dwc3/Kconfig  |   9 +
drivers/usb/dwc3/Makefile |   1 +
drivers/usb/dwc3/dwc3-of-simple.c |   1 -
drivers/usb/dwc3/dwc3-xilinx.c| 334 ++
4 files changed, 344 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 7a2304565a73..0e00e6dfccd8 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -139,4 +139,13 @@ config USB_DWC3_QCOM
  for peripheral mode support.
  Say 'Y' or 'M' if you have one such device.

+config USB_DWC3_XILINX
+   tristate "Xilinx Platforms"
+   depends on (ARCH_ZYNQMP || ARCH_VERSAL) && OF
+   default USB_DWC3
+   help
+ Support Xilinx SoCs with DesignWare Core USB3 IP.
+ This driver handles both ZynqMP and Versal SoC operations.
+ Say 'Y' or 'M' if you have one such device.
+
endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index ae86da0dc5bd..add567578b1f 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -51,3 +51,4 @@ obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o
obj-$(CONFIG_USB_DWC3_OF_SIMPLE)+= dwc3-of-simple.o
obj-$(CONFIG_USB_DWC3_ST)   += dwc3-st.o
obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
+obj-$(CONFIG_USB_DWC3_XILINX)  += dwc3-xilinx.o
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index e62ecd22b3ed..71fd620c5161 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {

static const struct of_device_id of_dwc3_simple_match[] = {
{ .compatible = "rockchip,rk3399-dwc3" },
-   { .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
{ .compatible = "sprd,sc9860-dwc3" },
{ .compatible = "allwinner,sun50i-h6-dwc3" },
diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
new file mode 100644
index ..7e485951d2f7
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver
+ *
+ * Authors: Manish Narani 
+ *  Anurag Kumar Vulisha 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* USB phy reset mask register */
+#define XLNX_USB_PHY_RST_EN0x001C

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-01-19 Thread Michael Grzeschik

On Mon, Jan 18, 2021 at 05:24:38PM +0200, Felipe Balbi wrote:


Hi,

Michael Grzeschik  writes:

On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:

Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from SerDes
and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.


I tried out this driver with the vanilla kernel on an zynqmp. Without
this patch the USB-Gadget is already acting buggy. In the gadget mode,
some iterations of plug/unplug results to an stalled gadget which will
never come back without a reboot.

With the corresponding code of this driver (reset assert, clk modify,
reset deassert) in the downstream kernels phy driver we found out it is
totaly stable. But using this exact glue driver which should do the same
as the downstream code, the gadget still was buggy the way described
above.

I suspect the difference lays in the different order of operations.
While the downstream code is runing the resets inside the phy driver
which is powered and initialized in the dwc3-core itself. With this glue
layser approach of this patch the whole phy init is done before even
touching dwc3-core in any way. It seems not to have the same effect,
though.

If really the order of operations is limiting us, we probably need
another solution than this glue layer. Any Ideas?


might be a good idea to collect dwc3 trace events. Can you do that?


I already did that. In case the port is not working properly, the port
was producing several "Erratic Errors" between the plug/unplug events.

This was not the case until the reset_assert, pll configure,
reset_deassert sequence was applied like in the downstream kernels
phy driver on phy_init.

Regards,
Michael

--
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


signature.asc
Description: PGP signature


Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-01-18 Thread Felipe Balbi


Hi,

Michael Grzeschik  writes:
> On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:
>>Add a new driver for supporting Xilinx platforms. This driver is used
>>for some sequence of operations required for Xilinx USB controllers.
>>This driver is also used to choose between PIPE clock coming from SerDes
>>and the Suspend Clock. Before the controller is out of reset, the clock
>>selection should be changed to PIPE clock in order to make the USB
>>controller work. There is a register added in Xilinx USB controller
>>register space for the same.
>
> I tried out this driver with the vanilla kernel on an zynqmp. Without
> this patch the USB-Gadget is already acting buggy. In the gadget mode,
> some iterations of plug/unplug results to an stalled gadget which will
> never come back without a reboot.
>
> With the corresponding code of this driver (reset assert, clk modify,
> reset deassert) in the downstream kernels phy driver we found out it is
> totaly stable. But using this exact glue driver which should do the same
> as the downstream code, the gadget still was buggy the way described
> above.
>
> I suspect the difference lays in the different order of operations.
> While the downstream code is runing the resets inside the phy driver
> which is powered and initialized in the dwc3-core itself. With this glue
> layser approach of this patch the whole phy init is done before even
> touching dwc3-core in any way. It seems not to have the same effect,
> though.
>
> If really the order of operations is limiting us, we probably need
> another solution than this glue layer. Any Ideas?

might be a good idea to collect dwc3 trace events. Can you do that?

-- 
balbi


Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2021-01-18 Thread Michael Grzeschik

Hi!

On Tue, Dec 15, 2020 at 12:24:51PM +0530, Manish Narani wrote:

Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from SerDes
and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.


I tried out this driver with the vanilla kernel on an zynqmp. Without
this patch the USB-Gadget is already acting buggy. In the gadget mode,
some iterations of plug/unplug results to an stalled gadget which will
never come back without a reboot.

With the corresponding code of this driver (reset assert, clk modify,
reset deassert) in the downstream kernels phy driver we found out it is
totaly stable. But using this exact glue driver which should do the same
as the downstream code, the gadget still was buggy the way described
above.

I suspect the difference lays in the different order of operations.
While the downstream code is runing the resets inside the phy driver
which is powered and initialized in the dwc3-core itself. With this glue
layser approach of this patch the whole phy init is done before even
touching dwc3-core in any way. It seems not to have the same effect,
though.

If really the order of operations is limiting us, we probably need
another solution than this glue layer. Any Ideas?

Regards,
Michael



Signed-off-by: Manish Narani 
---
drivers/usb/dwc3/Kconfig  |   9 +
drivers/usb/dwc3/Makefile |   1 +
drivers/usb/dwc3/dwc3-of-simple.c |   1 -
drivers/usb/dwc3/dwc3-xilinx.c| 334 ++
4 files changed, 344 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 7a2304565a73..0e00e6dfccd8 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -139,4 +139,13 @@ config USB_DWC3_QCOM
  for peripheral mode support.
  Say 'Y' or 'M' if you have one such device.

+config USB_DWC3_XILINX
+   tristate "Xilinx Platforms"
+   depends on (ARCH_ZYNQMP || ARCH_VERSAL) && OF
+   default USB_DWC3
+   help
+ Support Xilinx SoCs with DesignWare Core USB3 IP.
+ This driver handles both ZynqMP and Versal SoC operations.
+ Say 'Y' or 'M' if you have one such device.
+
endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index ae86da0dc5bd..add567578b1f 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -51,3 +51,4 @@ obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o
obj-$(CONFIG_USB_DWC3_OF_SIMPLE)+= dwc3-of-simple.o
obj-$(CONFIG_USB_DWC3_ST)   += dwc3-st.o
obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
+obj-$(CONFIG_USB_DWC3_XILINX)  += dwc3-xilinx.o
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index e62ecd22b3ed..71fd620c5161 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {

static const struct of_device_id of_dwc3_simple_match[] = {
{ .compatible = "rockchip,rk3399-dwc3" },
-   { .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
{ .compatible = "sprd,sc9860-dwc3" },
{ .compatible = "allwinner,sun50i-h6-dwc3" },
diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
new file mode 100644
index ..7e485951d2f7
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver
+ *
+ * Authors: Manish Narani 
+ *  Anurag Kumar Vulisha 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* USB phy reset mask register */
+#define XLNX_USB_PHY_RST_EN0x001C
+#define XLNX_PHY_RST_MASK  0x1
+
+/* Xilinx USB 3.0 IP Register */
+#define XLNX_USB_TRAFFIC_ROUTE_CONFIG  0x005C
+#define XLNX_USB_TRAFFIC_ROUTE_FPD 0x1
+
+/* Versal USB Reset ID */
+#define VERSAL_USB_RESET_ID0xC104036
+
+#define XLNX_USB_FPD_PIPE_CLK  0x7c
+#define PIPE_CLK_DESELECT  1
+#define PIPE_CLK_SELECT0
+#define XLNX_USB_FPD_POWER_PRSNT   0x80
+#define PIPE_POWER_ON  1
+#define PIPE_POWER_OFF 0
+
+struct dwc3_xlnx {
+   int num_clocks;
+   struct clk_bulk_data*clks;
+   struct device

Re: [RESEND PATCH v3 2/2] usb: dwc3: Add driver for Xilinx platforms

2020-12-21 Thread Michael Tretter
On Tue, 15 Dec 2020 12:24:51 +0530, Manish Narani wrote:
> Add a new driver for supporting Xilinx platforms. This driver is used
> for some sequence of operations required for Xilinx USB controllers.
> This driver is also used to choose between PIPE clock coming from SerDes
> and the Suspend Clock. Before the controller is out of reset, the clock
> selection should be changed to PIPE clock in order to make the USB
> controller work. There is a register added in Xilinx USB controller
> register space for the same.
> 
> Signed-off-by: Manish Narani 
> ---
>  drivers/usb/dwc3/Kconfig  |   9 +
>  drivers/usb/dwc3/Makefile |   1 +
>  drivers/usb/dwc3/dwc3-of-simple.c |   1 -
>  drivers/usb/dwc3/dwc3-xilinx.c| 334 ++
>  4 files changed, 344 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 7a2304565a73..0e00e6dfccd8 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -139,4 +139,13 @@ config USB_DWC3_QCOM
> for peripheral mode support.
> Say 'Y' or 'M' if you have one such device.
>  
> +config USB_DWC3_XILINX
> + tristate "Xilinx Platforms"
> + depends on (ARCH_ZYNQMP || ARCH_VERSAL) && OF
> + default USB_DWC3
> + help
> +   Support Xilinx SoCs with DesignWare Core USB3 IP.
> +   This driver handles both ZynqMP and Versal SoC operations.
> +   Say 'Y' or 'M' if you have one such device.
> +
>  endif
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index ae86da0dc5bd..add567578b1f 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -51,3 +51,4 @@ obj-$(CONFIG_USB_DWC3_MESON_G12A)   += dwc3-meson-g12a.o
>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o
>  obj-$(CONFIG_USB_DWC3_ST)+= dwc3-st.o
>  obj-$(CONFIG_USB_DWC3_QCOM)  += dwc3-qcom.o
> +obj-$(CONFIG_USB_DWC3_XILINX)+= dwc3-xilinx.o
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index e62ecd22b3ed..71fd620c5161 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops 
> = {
>  
>  static const struct of_device_id of_dwc3_simple_match[] = {
>   { .compatible = "rockchip,rk3399-dwc3" },
> - { .compatible = "xlnx,zynqmp-dwc3" },
>   { .compatible = "cavium,octeon-7130-usb-uctl" },
>   { .compatible = "sprd,sc9860-dwc3" },
>   { .compatible = "allwinner,sun50i-h6-dwc3" },
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> new file mode 100644
> index ..7e485951d2f7
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -0,0 +1,334 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver
> + *
> + * Authors: Manish Narani 
> + *  Anurag Kumar Vulisha 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +/* USB phy reset mask register */
> +#define XLNX_USB_PHY_RST_EN  0x001C
> +#define XLNX_PHY_RST_MASK0x1
> +
> +/* Xilinx USB 3.0 IP Register */
> +#define XLNX_USB_TRAFFIC_ROUTE_CONFIG0x005C
> +#define XLNX_USB_TRAFFIC_ROUTE_FPD   0x1
> +
> +/* Versal USB Reset ID */
> +#define VERSAL_USB_RESET_ID  0xC104036
> +
> +#define XLNX_USB_FPD_PIPE_CLK0x7c
> +#define PIPE_CLK_DESELECT1
> +#define PIPE_CLK_SELECT  0
> +#define XLNX_USB_FPD_POWER_PRSNT 0x80
> +#define PIPE_POWER_ON1
> +#define PIPE_POWER_OFF   0

Don't use values for the defines, but rather define the bit. Its name in the
register reference is "option". Therefore, define it as

#define FPD_POWER_PRSNT_OPTION  BIT(0)

and set/unset the bit in the code. The same for the other registers/bits.

> +
> +struct dwc3_xlnx {
> + int num_clocks;
> + struct clk_bulk_data*clks;
> + struct device   *dev;
> + void __iomem*regs;
> + int (*pltfm_init)(struct dwc3_xlnx *data);
> +};
> +
> +static void dwc3_xlnx_mask_phy_rst(struct dwc3_xlnx *priv_data, bool mask)
> +{
> + u32 reg;
> +
> + /*
> +  * Enable or disable ULPI PHY reset from USB Controller.
> +  * This does not actually reset the phy, but just controls
> +  * whether USB controller can or cannot reset ULPI PHY.
> +  */
> + reg = readl(priv_data->regs + XLNX_USB_PHY_RST_EN);
> +
> + if (mask)
> +