* [PATCH 1/3] dt-bindings: usb: Convert cdns-usb3.txt to YAML schema
2020-08-25 12:00 [PATCH 0/3] usb: cdns,usb3: Convert DT binding to YALM Roger Quadros
@ 2020-08-25 12:00 ` Roger Quadros
2020-08-25 12:00 ` [PATCH 2/3] dt-bindings: usb: cdns,usb3: Add cdns,phyrst-a-enable property Roger Quadros
2020-08-25 12:00 ` [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup Roger Quadros
2 siblings, 0 replies; 17+ messages in thread
From: Roger Quadros @ 2020-08-25 12:00 UTC (permalink / raw)
To: balbi
Cc: pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree, Roger Quadros
Converts cdns-usb3.txt to YAML schema cdns,usb3.yaml
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
.../devicetree/bindings/usb/cdns,usb3.yaml | 89 +++++++++++++++++++
.../devicetree/bindings/usb/cdns-usb3.txt | 45 ----------
2 files changed, 89 insertions(+), 45 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/cdns,usb3.yaml
delete mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3.txt
diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
new file mode 100644
index 000000000000..7bc0263accee
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/cdns,usb3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cadence USBSS-DRD controller bindings
+
+maintainers:
+ - Pawel Laszczak <pawell@cadence.com>
+
+properties:
+ compatible:
+ oneOf:
+ - const: cdns,usb3
+
+ reg:
+ items:
+ - description: OTG controller registers
+ - description: XHCI Host controller registers
+ - description: DEVICE controller registers
+
+ reg-names:
+ items:
+ - const: otg
+ - const: xhci
+ - const: dev
+
+ interrupts:
+ items:
+ - description: OTG/DRD controller interrupt
+ - description: XHCI host controller interrupt
+ - description: Device controller interrupt
+
+ interrupt-names:
+ items:
+ - const: host
+ - const: peripheral
+ - const: otg
+
+ dr_mode:
+ enum: [host, otg, peripheral]
+
+ maximum-speed:
+ enum: [super-speed, high-speed, full-speed]
+
+ phys:
+ minItems: 1
+ maxItems: 2
+
+ phy-names:
+ minItems: 1
+ maxItems: 2
+
+ cdns,on-chip-buff-size:
+ description:
+ size of memory intended as internal memory for endpoints
+ buffers expressed in KB
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ usb@6000000 {
+ compatible = "cdns,usb3";
+ reg = <0x00 0x6000000 0x00 0x10000>,
+ <0x00 0x6010000 0x00 0x10000>,
+ <0x00 0x6020000 0x00 0x10000>;
+ reg-names = "otg", "xhci", "dev";
+ interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "host", "peripheral", "otg";
+ maximum-speed = "super-speed";
+ dr_mode = "otg";
+ };
+ };
diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3.txt b/Documentation/devicetree/bindings/usb/cdns-usb3.txt
deleted file mode 100644
index b7dc606d37b5..000000000000
--- a/Documentation/devicetree/bindings/usb/cdns-usb3.txt
+++ /dev/null
@@ -1,45 +0,0 @@
-Binding for the Cadence USBSS-DRD controller
-
-Required properties:
- - reg: Physical base address and size of the controller's register areas.
- Controller has 3 different regions:
- - HOST registers area
- - DEVICE registers area
- - OTG/DRD registers area
- - reg-names - register memory area names:
- "xhci" - for HOST registers space
- "dev" - for DEVICE registers space
- "otg" - for OTG/DRD registers space
- - compatible: Should contain: "cdns,usb3"
- - interrupts: Interrupts used by cdns3 controller:
- "host" - interrupt used by XHCI driver.
- "peripheral" - interrupt used by device driver
- "otg" - interrupt used by DRD/OTG part of driver
-
-Optional properties:
- - maximum-speed : valid arguments are "super-speed", "high-speed" and
- "full-speed"; refer to usb/generic.txt
- - dr_mode: Should be one of "host", "peripheral" or "otg".
- - phys: reference to the USB PHY
- - phy-names: from the *Generic PHY* bindings;
- Supported names are:
- - cdns3,usb2-phy
- - cdns3,usb3-phy
-
- - cdns,on-chip-buff-size : size of memory intended as internal memory for endpoints
- buffers expressed in KB
-
-Example:
- usb@f3000000 {
- compatible = "cdns,usb3";
- interrupts = <GIC_USB_IRQ 7 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_USB_IRQ 7 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_USB_IRQ 8 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "host", "peripheral", "otg";
- reg = <0xf3000000 0x10000>, /* memory area for HOST registers */
- <0xf3010000 0x10000>, /* memory area for DEVICE registers */
- <0xf3020000 0x10000>; /* memory area for OTG/DRD registers */
- reg-names = "xhci", "dev", "otg";
- phys = <&usb2_phy>, <&usb3_phy>;
- phy-names = "cdns3,usb2-phy", "cnds3,usb3-phy";
- };
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/3] dt-bindings: usb: cdns,usb3: Add cdns,phyrst-a-enable property
2020-08-25 12:00 [PATCH 0/3] usb: cdns,usb3: Convert DT binding to YALM Roger Quadros
2020-08-25 12:00 ` [PATCH 1/3] dt-bindings: usb: Convert cdns-usb3.txt to YAML schema Roger Quadros
@ 2020-08-25 12:00 ` Roger Quadros
2020-08-27 11:14 ` Peter Chen
2020-08-25 12:00 ` [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup Roger Quadros
2 siblings, 1 reply; 17+ messages in thread
From: Roger Quadros @ 2020-08-25 12:00 UTC (permalink / raw)
To: balbi
Cc: pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree, Roger Quadros
Controller version 0x0002450D has USB2 PHY RX sensitivity issues
that needs to be worked around by enabling phyrst-a-enable bit
in PHYRST_CFG register.
There is no way to distinguish between the controller version
before the device controller is started so we need to add this
DT property.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
Documentation/devicetree/bindings/usb/cdns,usb3.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
index 7bc0263accee..e670adc955e1 100644
--- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
+++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
@@ -58,6 +58,10 @@ properties:
buffers expressed in KB
$ref: /schemas/types.yaml#/definitions/uint32
+ cdns,phyrst-a-enable:
+ description: Enable resetting of PHY if Rx fail is detected
+ type: boolean
+
required:
- compatible
- reg
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] dt-bindings: usb: cdns,usb3: Add cdns,phyrst-a-enable property
2020-08-25 12:00 ` [PATCH 2/3] dt-bindings: usb: cdns,usb3: Add cdns,phyrst-a-enable property Roger Quadros
@ 2020-08-27 11:14 ` Peter Chen
2020-09-02 13:28 ` Roger Quadros
0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2020-08-27 11:14 UTC (permalink / raw)
To: Roger Quadros
Cc: balbi, pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree
On 20-08-25 15:00:58, Roger Quadros wrote:
> Controller version 0x0002450D has USB2 PHY RX sensitivity issues
> that needs to be worked around by enabling phyrst-a-enable bit
> in PHYRST_CFG register.
>
> There is no way to distinguish between the controller version
> before the device controller is started so we need to add this
> DT property.
Maybe you should say "There is no way to know controller version
at host mode, but this workaround needs for both host and device
mode, so we have to add this DT property", the same for the comments
of your driver code patch.
Peter
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> Documentation/devicetree/bindings/usb/cdns,usb3.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> index 7bc0263accee..e670adc955e1 100644
> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> @@ -58,6 +58,10 @@ properties:
> buffers expressed in KB
> $ref: /schemas/types.yaml#/definitions/uint32
>
> + cdns,phyrst-a-enable:
> + description: Enable resetting of PHY if Rx fail is detected
> + type: boolean
> +
> required:
> - compatible
> - reg
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] dt-bindings: usb: cdns,usb3: Add cdns,phyrst-a-enable property
2020-08-27 11:14 ` Peter Chen
@ 2020-09-02 13:28 ` Roger Quadros
2020-09-02 22:48 ` Peter Chen
0 siblings, 1 reply; 17+ messages in thread
From: Roger Quadros @ 2020-09-02 13:28 UTC (permalink / raw)
To: Peter Chen
Cc: balbi, pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree
Peter,
On 27/08/2020 14:14, Peter Chen wrote:
> On 20-08-25 15:00:58, Roger Quadros wrote:
>> Controller version 0x0002450D has USB2 PHY RX sensitivity issues
>> that needs to be worked around by enabling phyrst-a-enable bit
>> in PHYRST_CFG register.
>>
>> There is no way to distinguish between the controller version
>> before the device controller is started so we need to add this
>> DT property.
>
> Maybe you should say "There is no way to know controller version
> at host mode, but this workaround needs for both host and device
> mode, so we have to add this DT property", the same for the comments
> of your driver code patch.
I will reword it to
"There is no way to know controller version before device controller
is started and the workaround needs to be applied for both host and
device modes, so we add this DT property"
cheers,
-roger
>
> Peter
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> Documentation/devicetree/bindings/usb/cdns,usb3.yaml | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
>> index 7bc0263accee..e670adc955e1 100644
>> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
>> @@ -58,6 +58,10 @@ properties:
>> buffers expressed in KB
>> $ref: /schemas/types.yaml#/definitions/uint32
>>
>> + cdns,phyrst-a-enable:
>> + description: Enable resetting of PHY if Rx fail is detected
>> + type: boolean
>> +
>> required:
>> - compatible
>> - reg
>> --
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] dt-bindings: usb: cdns,usb3: Add cdns,phyrst-a-enable property
2020-09-02 13:28 ` Roger Quadros
@ 2020-09-02 22:48 ` Peter Chen
0 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2020-09-02 22:48 UTC (permalink / raw)
To: Roger Quadros
Cc: balbi, pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree
On 20-09-02 16:28:31, Roger Quadros wrote:
> Peter,
>
> On 27/08/2020 14:14, Peter Chen wrote:
> > On 20-08-25 15:00:58, Roger Quadros wrote:
> > > Controller version 0x0002450D has USB2 PHY RX sensitivity issues
> > > that needs to be worked around by enabling phyrst-a-enable bit
> > > in PHYRST_CFG register.
> > >
> > > There is no way to distinguish between the controller version
> > > before the device controller is started so we need to add this
> > > DT property.
> >
> > Maybe you should say "There is no way to know controller version
> > at host mode, but this workaround needs for both host and device
> > mode, so we have to add this DT property", the same for the comments
> > of your driver code patch.
>
> I will reword it to
>
> "There is no way to know controller version before device controller
> is started and the workaround needs to be applied for both host and
> device modes, so we add this DT property"
>
After apply it, feel free add:
Reviewed-by: Peter Chen <peter.chen@nxp.com>
Please apply above for device party commit log as well.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-25 12:00 [PATCH 0/3] usb: cdns,usb3: Convert DT binding to YALM Roger Quadros
2020-08-25 12:00 ` [PATCH 1/3] dt-bindings: usb: Convert cdns-usb3.txt to YAML schema Roger Quadros
2020-08-25 12:00 ` [PATCH 2/3] dt-bindings: usb: cdns,usb3: Add cdns,phyrst-a-enable property Roger Quadros
@ 2020-08-25 12:00 ` Roger Quadros
2020-08-26 3:21 ` Peter Chen
2020-08-27 11:09 ` Peter Chen
2 siblings, 2 replies; 17+ messages in thread
From: Roger Quadros @ 2020-08-25 12:00 UTC (permalink / raw)
To: balbi
Cc: pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree, Roger Quadros
From: Pawel Laszczak <pawell@cadence.com>
USB2.0 PHY hangs in Rx Compliance test when the incoming packet
amplitude is varied below and above the Squelch Level of
Receiver during the active packet multiple times.
Version 1 of the controller allows PHY to be reset when RX fail condition
is detected to work around the above issue. This feature is
disabled by default and needs to be enabled using a bit from
the newly added PHYRST_CFG register. This patch enables the workaround.
As there is no way to distinguish between the controller version
before the device controller is started we need to rely on a
DT property to decide when to apply the workaround.
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/usb/cdns3/core.c | 2 ++
drivers/usb/cdns3/core.h | 1 +
drivers/usb/cdns3/drd.c | 12 ++++++++++++
drivers/usb/cdns3/drd.h | 5 ++++-
4 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 5c1586ec7824..34b36487682b 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
return -ENXIO;
}
+ cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
+
cdns->otg_res = *res;
mutex_init(&cdns->mutex);
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index 1ad1f1fe61e9..24cf0f1b5726 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -76,6 +76,7 @@ struct cdns3 {
#define CDNS3_CONTROLLER_V0 0
#define CDNS3_CONTROLLER_V1 1
u32 version;
+ bool phyrst_a_enable;
int otg_irq;
int dev_irq;
diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
index 6234bcd6158a..b74803e9703d 100644
--- a/drivers/usb/cdns3/drd.c
+++ b/drivers/usb/cdns3/drd.c
@@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
reg = readl(&cdns->otg_v1_regs->override);
reg |= OVERRIDE_IDPULLUP;
writel(reg, &cdns->otg_v1_regs->override);
+
+ /*
+ * Enable work around feature built into the
+ * controller to address issue with RX Sensitivity
+ * est (EL_17) for USB2 PHY. The issue only occures
+ * for 0x0002450D controller version.
+ */
+ if (cdns->phyrst_a_enable) {
+ reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
+ reg |= PHYRST_CFG_PHYRST_A_ENABLE;
+ writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
+ }
} else {
reg = readl(&cdns->otg_v0_regs->ctrl1);
reg |= OVERRIDE_IDPULLUP_V0;
diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
index 7e7cf7fa2dd3..f1ccae285a16 100644
--- a/drivers/usb/cdns3/drd.h
+++ b/drivers/usb/cdns3/drd.h
@@ -31,7 +31,7 @@ struct cdns3_otg_regs {
__le32 simulate;
__le32 override;
__le32 susp_ctrl;
- __le32 reserved4;
+ __le32 phyrst_cfg;
__le32 anasts;
__le32 adp_ramp_time;
__le32 ctrl1;
@@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
/* Only for CDNS3_CONTROLLER_V0 version */
#define OVERRIDE_IDPULLUP_V0 BIT(24)
+/* PHYRST_CFG - bitmasks */
+#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
+
#define CDNS3_ID_PERIPHERAL 1
#define CDNS3_ID_HOST 0
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-25 12:00 ` [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup Roger Quadros
@ 2020-08-26 3:21 ` Peter Chen
2020-08-26 4:04 ` Pawel Laszczak
2020-08-27 11:09 ` Peter Chen
1 sibling, 1 reply; 17+ messages in thread
From: Peter Chen @ 2020-08-26 3:21 UTC (permalink / raw)
To: Roger Quadros
Cc: balbi, pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree
On 20-08-25 15:00:59, Roger Quadros wrote:
> From: Pawel Laszczak <pawell@cadence.com>
>
> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
> amplitude is varied below and above the Squelch Level of
> Receiver during the active packet multiple times.
>
> Version 1 of the controller allows PHY to be reset when RX fail condition
> is detected to work around the above issue. This feature is
> disabled by default and needs to be enabled using a bit from
> the newly added PHYRST_CFG register. This patch enables the workaround.
>
> As there is no way to distinguish between the controller version
> before the device controller is started we need to rely on a
> DT property to decide when to apply the workaround.
Pawel, it could know the controller version at cdns3_gadget_start,
but the controller starts when it tries to bind gadget driver, at that
time, it has already known the controller version.
For me, the device controller starts is using USB_CONF.DEVEN (Device
Enable) through usb_gadget_connect, I am not sure if it is the same
with yours.
Peter
>
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/cdns3/core.c | 2 ++
> drivers/usb/cdns3/core.h | 1 +
> drivers/usb/cdns3/drd.c | 12 ++++++++++++
> drivers/usb/cdns3/drd.h | 5 ++++-
> 4 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 5c1586ec7824..34b36487682b 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
> return -ENXIO;
> }
>
> + cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
> +
> cdns->otg_res = *res;
>
> mutex_init(&cdns->mutex);
> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> index 1ad1f1fe61e9..24cf0f1b5726 100644
> --- a/drivers/usb/cdns3/core.h
> +++ b/drivers/usb/cdns3/core.h
> @@ -76,6 +76,7 @@ struct cdns3 {
> #define CDNS3_CONTROLLER_V0 0
> #define CDNS3_CONTROLLER_V1 1
> u32 version;
> + bool phyrst_a_enable;
>
> int otg_irq;
> int dev_irq;
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 6234bcd6158a..b74803e9703d 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
> reg = readl(&cdns->otg_v1_regs->override);
> reg |= OVERRIDE_IDPULLUP;
> writel(reg, &cdns->otg_v1_regs->override);
> +
> + /*
> + * Enable work around feature built into the
> + * controller to address issue with RX Sensitivity
> + * est (EL_17) for USB2 PHY. The issue only occures
> + * for 0x0002450D controller version.
> + */
> + if (cdns->phyrst_a_enable) {
> + reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
> + reg |= PHYRST_CFG_PHYRST_A_ENABLE;
> + writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
> + }
> } else {
> reg = readl(&cdns->otg_v0_regs->ctrl1);
> reg |= OVERRIDE_IDPULLUP_V0;
> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> index 7e7cf7fa2dd3..f1ccae285a16 100644
> --- a/drivers/usb/cdns3/drd.h
> +++ b/drivers/usb/cdns3/drd.h
> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
> __le32 simulate;
> __le32 override;
> __le32 susp_ctrl;
> - __le32 reserved4;
> + __le32 phyrst_cfg;
> __le32 anasts;
> __le32 adp_ramp_time;
> __le32 ctrl1;
> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
> /* Only for CDNS3_CONTROLLER_V0 version */
> #define OVERRIDE_IDPULLUP_V0 BIT(24)
>
> +/* PHYRST_CFG - bitmasks */
> +#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
> +
> #define CDNS3_ID_PERIPHERAL 1
> #define CDNS3_ID_HOST 0
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-26 3:21 ` Peter Chen
@ 2020-08-26 4:04 ` Pawel Laszczak
2020-08-26 7:16 ` Peter Chen
0 siblings, 1 reply; 17+ messages in thread
From: Pawel Laszczak @ 2020-08-26 4:04 UTC (permalink / raw)
To: Peter Chen, Roger Quadros
Cc: balbi, Rahul Kumar, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree
>On 20-08-25 15:00:59, Roger Quadros wrote:
>> From: Pawel Laszczak <pawell@cadence.com>
>>
>> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
>> amplitude is varied below and above the Squelch Level of
>> Receiver during the active packet multiple times.
>>
>> Version 1 of the controller allows PHY to be reset when RX fail condition
>> is detected to work around the above issue. This feature is
>> disabled by default and needs to be enabled using a bit from
>> the newly added PHYRST_CFG register. This patch enables the workaround.
>>
>> As there is no way to distinguish between the controller version
>> before the device controller is started we need to rely on a
>> DT property to decide when to apply the workaround.
>
>Pawel, it could know the controller version at cdns3_gadget_start,
>but the controller starts when it tries to bind gadget driver, at that
>time, it has already known the controller version.
>
>For me, the device controller starts is using USB_CONF.DEVEN (Device
>Enable) through usb_gadget_connect, I am not sure if it is the same
>with yours.
>
Yes in device mode driver knows controller version but this workaround
Must be enabled also in host mode. In host mode the controller
doesn't have access to device registers. The controller version is
placed in device register.
Pawel
>Peter
>
>
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> drivers/usb/cdns3/core.c | 2 ++
>> drivers/usb/cdns3/core.h | 1 +
>> drivers/usb/cdns3/drd.c | 12 ++++++++++++
>> drivers/usb/cdns3/drd.h | 5 ++++-
>> 4 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index 5c1586ec7824..34b36487682b 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
>> return -ENXIO;
>> }
>>
>> + cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
>> +
>> cdns->otg_res = *res;
>>
>> mutex_init(&cdns->mutex);
>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>> index 1ad1f1fe61e9..24cf0f1b5726 100644
>> --- a/drivers/usb/cdns3/core.h
>> +++ b/drivers/usb/cdns3/core.h
>> @@ -76,6 +76,7 @@ struct cdns3 {
>> #define CDNS3_CONTROLLER_V0 0
>> #define CDNS3_CONTROLLER_V1 1
>> u32 version;
>> + bool phyrst_a_enable;
>>
>> int otg_irq;
>> int dev_irq;
>> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
>> index 6234bcd6158a..b74803e9703d 100644
>> --- a/drivers/usb/cdns3/drd.c
>> +++ b/drivers/usb/cdns3/drd.c
>> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
>> reg = readl(&cdns->otg_v1_regs->override);
>> reg |= OVERRIDE_IDPULLUP;
>> writel(reg, &cdns->otg_v1_regs->override);
>> +
>> + /*
>> + * Enable work around feature built into the
>> + * controller to address issue with RX Sensitivity
>> + * est (EL_17) for USB2 PHY. The issue only occures
>> + * for 0x0002450D controller version.
>> + */
>> + if (cdns->phyrst_a_enable) {
>> + reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
>> + reg |= PHYRST_CFG_PHYRST_A_ENABLE;
>> + writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
>> + }
>> } else {
>> reg = readl(&cdns->otg_v0_regs->ctrl1);
>> reg |= OVERRIDE_IDPULLUP_V0;
>> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
>> index 7e7cf7fa2dd3..f1ccae285a16 100644
>> --- a/drivers/usb/cdns3/drd.h
>> +++ b/drivers/usb/cdns3/drd.h
>> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
>> __le32 simulate;
>> __le32 override;
>> __le32 susp_ctrl;
>> - __le32 reserved4;
>> + __le32 phyrst_cfg;
>> __le32 anasts;
>> __le32 adp_ramp_time;
>> __le32 ctrl1;
>> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
>> /* Only for CDNS3_CONTROLLER_V0 version */
>> #define OVERRIDE_IDPULLUP_V0 BIT(24)
>>
>> +/* PHYRST_CFG - bitmasks */
>> +#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
>> +
>> #define CDNS3_ID_PERIPHERAL 1
>> #define CDNS3_ID_HOST 0
>>
>> --
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>
>
>--
>
>Thanks,
>Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-26 4:04 ` Pawel Laszczak
@ 2020-08-26 7:16 ` Peter Chen
2020-08-26 7:44 ` Pawel Laszczak
0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2020-08-26 7:16 UTC (permalink / raw)
To: Pawel Laszczak
Cc: Roger Quadros, balbi, Rahul Kumar, nsekhar, vigneshr, robh+dt,
linux-usb, linux-kernel, devicetree
On 20-08-26 04:04:01, Pawel Laszczak wrote:
> >On 20-08-25 15:00:59, Roger Quadros wrote:
> >> From: Pawel Laszczak <pawell@cadence.com>
> >>
> >> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
> >> amplitude is varied below and above the Squelch Level of
> >> Receiver during the active packet multiple times.
> >>
> >> Version 1 of the controller allows PHY to be reset when RX fail condition
> >> is detected to work around the above issue. This feature is
> >> disabled by default and needs to be enabled using a bit from
> >> the newly added PHYRST_CFG register. This patch enables the workaround.
> >>
> >> As there is no way to distinguish between the controller version
> >> before the device controller is started we need to rely on a
> >> DT property to decide when to apply the workaround.
> >
> >Pawel, it could know the controller version at cdns3_gadget_start,
> >but the controller starts when it tries to bind gadget driver, at that
> >time, it has already known the controller version.
> >
> >For me, the device controller starts is using USB_CONF.DEVEN (Device
> >Enable) through usb_gadget_connect, I am not sure if it is the same
> >with yours.
> >
>
> Yes in device mode driver knows controller version but this workaround
> Must be enabled also in host mode. In host mode the controller
> doesn't have access to device registers. The controller version is
> placed in device register.
>
You may suggest your design team adding CHIP_VER register at global
register region, it will easy the software engineer life.
From what I read, this register is only enabling USB2 PHY reset software
control, it needs for all chips with rev 0x0002450D, and the place you
current change is only for 0x0002450D, right?
Peter
> Pawel
>
> >Peter
> >
> >
> >>
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >> ---
> >> drivers/usb/cdns3/core.c | 2 ++
> >> drivers/usb/cdns3/core.h | 1 +
> >> drivers/usb/cdns3/drd.c | 12 ++++++++++++
> >> drivers/usb/cdns3/drd.h | 5 ++++-
> >> 4 files changed, 19 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> >> index 5c1586ec7824..34b36487682b 100644
> >> --- a/drivers/usb/cdns3/core.c
> >> +++ b/drivers/usb/cdns3/core.c
> >> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
> >> return -ENXIO;
> >> }
> >>
> >> + cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
> >> +
> >> cdns->otg_res = *res;
> >>
> >> mutex_init(&cdns->mutex);
> >> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> >> index 1ad1f1fe61e9..24cf0f1b5726 100644
> >> --- a/drivers/usb/cdns3/core.h
> >> +++ b/drivers/usb/cdns3/core.h
> >> @@ -76,6 +76,7 @@ struct cdns3 {
> >> #define CDNS3_CONTROLLER_V0 0
> >> #define CDNS3_CONTROLLER_V1 1
> >> u32 version;
> >> + bool phyrst_a_enable;
> >>
> >> int otg_irq;
> >> int dev_irq;
> >> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> >> index 6234bcd6158a..b74803e9703d 100644
> >> --- a/drivers/usb/cdns3/drd.c
> >> +++ b/drivers/usb/cdns3/drd.c
> >> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
> >> reg = readl(&cdns->otg_v1_regs->override);
> >> reg |= OVERRIDE_IDPULLUP;
> >> writel(reg, &cdns->otg_v1_regs->override);
> >> +
> >> + /*
> >> + * Enable work around feature built into the
> >> + * controller to address issue with RX Sensitivity
> >> + * est (EL_17) for USB2 PHY. The issue only occures
> >> + * for 0x0002450D controller version.
> >> + */
> >> + if (cdns->phyrst_a_enable) {
> >> + reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
> >> + reg |= PHYRST_CFG_PHYRST_A_ENABLE;
> >> + writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
> >> + }
> >> } else {
> >> reg = readl(&cdns->otg_v0_regs->ctrl1);
> >> reg |= OVERRIDE_IDPULLUP_V0;
> >> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> >> index 7e7cf7fa2dd3..f1ccae285a16 100644
> >> --- a/drivers/usb/cdns3/drd.h
> >> +++ b/drivers/usb/cdns3/drd.h
> >> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
> >> __le32 simulate;
> >> __le32 override;
> >> __le32 susp_ctrl;
> >> - __le32 reserved4;
> >> + __le32 phyrst_cfg;
> >> __le32 anasts;
> >> __le32 adp_ramp_time;
> >> __le32 ctrl1;
> >> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
> >> /* Only for CDNS3_CONTROLLER_V0 version */
> >> #define OVERRIDE_IDPULLUP_V0 BIT(24)
> >>
> >> +/* PHYRST_CFG - bitmasks */
> >> +#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
> >> +
> >> #define CDNS3_ID_PERIPHERAL 1
> >> #define CDNS3_ID_HOST 0
> >>
> >> --
> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> >>
> >
> >--
> >
> >Thanks,
> >Peter Chen
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-26 7:16 ` Peter Chen
@ 2020-08-26 7:44 ` Pawel Laszczak
2020-08-26 8:07 ` Peter Chen
0 siblings, 1 reply; 17+ messages in thread
From: Pawel Laszczak @ 2020-08-26 7:44 UTC (permalink / raw)
To: Peter Chen
Cc: Roger Quadros, balbi, Rahul Kumar, nsekhar, vigneshr, robh+dt,
linux-usb, linux-kernel, devicetree
>
>On 20-08-26 04:04:01, Pawel Laszczak wrote:
>> >On 20-08-25 15:00:59, Roger Quadros wrote:
>> >> From: Pawel Laszczak <pawell@cadence.com>
>> >>
>> >> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
>> >> amplitude is varied below and above the Squelch Level of
>> >> Receiver during the active packet multiple times.
>> >>
>> >> Version 1 of the controller allows PHY to be reset when RX fail condition
>> >> is detected to work around the above issue. This feature is
>> >> disabled by default and needs to be enabled using a bit from
>> >> the newly added PHYRST_CFG register. This patch enables the workaround.
>> >>
>> >> As there is no way to distinguish between the controller version
>> >> before the device controller is started we need to rely on a
>> >> DT property to decide when to apply the workaround.
>> >
>> >Pawel, it could know the controller version at cdns3_gadget_start,
>> >but the controller starts when it tries to bind gadget driver, at that
>> >time, it has already known the controller version.
>> >
>> >For me, the device controller starts is using USB_CONF.DEVEN (Device
>> >Enable) through usb_gadget_connect, I am not sure if it is the same
>> >with yours.
>> >
>>
>> Yes in device mode driver knows controller version but this workaround
>> Must be enabled also in host mode. In host mode the controller
>> doesn't have access to device registers. The controller version is
>> placed in device register.
>>
>
>You may suggest your design team adding CHIP_VER register at global
>register region, it will easy the software engineer life.
>
>From what I read, this register is only enabling USB2 PHY reset software
>control, it needs for all chips with rev 0x0002450D, and the place you
>current change is only for 0x0002450D, right?
Even I could say that this workaround should be enabled only for
Specific USB2 PHY (only 0x0002450D)
This bit should not have any impact for Cadence PHY but it can has
Impact for third party PHYs.
Pawel
>
>Peter
>
>> Pawel
>>
>> >Peter
>> >
>> >
>> >>
>> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> >> ---
>> >> drivers/usb/cdns3/core.c | 2 ++
>> >> drivers/usb/cdns3/core.h | 1 +
>> >> drivers/usb/cdns3/drd.c | 12 ++++++++++++
>> >> drivers/usb/cdns3/drd.h | 5 ++++-
>> >> 4 files changed, 19 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> >> index 5c1586ec7824..34b36487682b 100644
>> >> --- a/drivers/usb/cdns3/core.c
>> >> +++ b/drivers/usb/cdns3/core.c
>> >> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
>> >> return -ENXIO;
>> >> }
>> >>
>> >> + cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
>> >> +
>> >> cdns->otg_res = *res;
>> >>
>> >> mutex_init(&cdns->mutex);
>> >> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>> >> index 1ad1f1fe61e9..24cf0f1b5726 100644
>> >> --- a/drivers/usb/cdns3/core.h
>> >> +++ b/drivers/usb/cdns3/core.h
>> >> @@ -76,6 +76,7 @@ struct cdns3 {
>> >> #define CDNS3_CONTROLLER_V0 0
>> >> #define CDNS3_CONTROLLER_V1 1
>> >> u32 version;
>> >> + bool phyrst_a_enable;
>> >>
>> >> int otg_irq;
>> >> int dev_irq;
>> >> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
>> >> index 6234bcd6158a..b74803e9703d 100644
>> >> --- a/drivers/usb/cdns3/drd.c
>> >> +++ b/drivers/usb/cdns3/drd.c
>> >> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
>> >> reg = readl(&cdns->otg_v1_regs->override);
>> >> reg |= OVERRIDE_IDPULLUP;
>> >> writel(reg, &cdns->otg_v1_regs->override);
>> >> +
>> >> + /*
>> >> + * Enable work around feature built into the
>> >> + * controller to address issue with RX Sensitivity
>> >> + * est (EL_17) for USB2 PHY. The issue only occures
>> >> + * for 0x0002450D controller version.
>> >> + */
>> >> + if (cdns->phyrst_a_enable) {
>> >> + reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
>> >> + reg |= PHYRST_CFG_PHYRST_A_ENABLE;
>> >> + writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
>> >> + }
>> >> } else {
>> >> reg = readl(&cdns->otg_v0_regs->ctrl1);
>> >> reg |= OVERRIDE_IDPULLUP_V0;
>> >> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
>> >> index 7e7cf7fa2dd3..f1ccae285a16 100644
>> >> --- a/drivers/usb/cdns3/drd.h
>> >> +++ b/drivers/usb/cdns3/drd.h
>> >> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
>> >> __le32 simulate;
>> >> __le32 override;
>> >> __le32 susp_ctrl;
>> >> - __le32 reserved4;
>> >> + __le32 phyrst_cfg;
>> >> __le32 anasts;
>> >> __le32 adp_ramp_time;
>> >> __le32 ctrl1;
>> >> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
>> >> /* Only for CDNS3_CONTROLLER_V0 version */
>> >> #define OVERRIDE_IDPULLUP_V0 BIT(24)
>> >>
>> >> +/* PHYRST_CFG - bitmasks */
>> >> +#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
>> >> +
>> >> #define CDNS3_ID_PERIPHERAL 1
>> >> #define CDNS3_ID_HOST 0
>> >>
>> >> --
>> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>> >>
>> >
>> >--
>> >
>> >Thanks,
>> >Peter Chen
>
>--
>
>Thanks,
>Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-26 7:44 ` Pawel Laszczak
@ 2020-08-26 8:07 ` Peter Chen
2020-08-26 12:49 ` Roger Quadros
0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2020-08-26 8:07 UTC (permalink / raw)
To: Pawel Laszczak
Cc: Roger Quadros, balbi, Rahul Kumar, nsekhar, vigneshr, robh+dt,
linux-usb, linux-kernel, devicetree
>
>
> >
> >On 20-08-26 04:04:01, Pawel Laszczak wrote:
> >> >On 20-08-25 15:00:59, Roger Quadros wrote:
> >> >> From: Pawel Laszczak <pawell@cadence.com>
> >> >>
> >> >> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
> >> >> amplitude is varied below and above the Squelch Level of Receiver
> >> >> during the active packet multiple times.
> >> >>
> >> >> Version 1 of the controller allows PHY to be reset when RX fail
> >> >> condition is detected to work around the above issue. This feature
> >> >> is disabled by default and needs to be enabled using a bit from
> >> >> the newly added PHYRST_CFG register. This patch enables the workaround.
> >> >>
> >> >> As there is no way to distinguish between the controller version
> >> >> before the device controller is started we need to rely on a DT
> >> >> property to decide when to apply the workaround.
> >> >
> >> >Pawel, it could know the controller version at cdns3_gadget_start,
> >> >but the controller starts when it tries to bind gadget driver, at
> >> >that time, it has already known the controller version.
> >> >
> >> >For me, the device controller starts is using USB_CONF.DEVEN (Device
> >> >Enable) through usb_gadget_connect, I am not sure if it is the same
> >> >with yours.
> >> >
> >>
> >> Yes in device mode driver knows controller version but this
> >> workaround Must be enabled also in host mode. In host mode the
> >> controller doesn't have access to device registers. The controller
> >> version is placed in device register.
> >>
> >
> >You may suggest your design team adding CHIP_VER register at global
> >register region, it will easy the software engineer life.
> >
> >From what I read, this register is only enabling USB2 PHY reset
> >software control, it needs for all chips with rev 0x0002450D, and the
> >place you current change is only for 0x0002450D, right?
>
> Even I could say that this workaround should be enabled only for Specific USB2
> PHY (only 0x0002450D)
>
> This bit should not have any impact for Cadence PHY but it can has Impact for third
> party PHYs.
>
So, it is related to specific PHY, but enable this specific PHY reset bit is at controller region, why don't
put this enable bit at PHY region?
So, you use controller's device property to know this specific PHY, can controller know this specific
PHY dynamically?
Peter
> Pawel
>
> >
> >Peter
> >
> >> Pawel
> >>
> >> >Peter
> >> >
> >> >
> >> >>
> >> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >> >> ---
> >> >> drivers/usb/cdns3/core.c | 2 ++
> >> >> drivers/usb/cdns3/core.h | 1 +
> >> >> drivers/usb/cdns3/drd.c | 12 ++++++++++++
> >> >> drivers/usb/cdns3/drd.h | 5 ++++-
> >> >> 4 files changed, 19 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> >> >> index 5c1586ec7824..34b36487682b 100644
> >> >> --- a/drivers/usb/cdns3/core.c
> >> >> +++ b/drivers/usb/cdns3/core.c
> >> >> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
> >> >> return -ENXIO;
> >> >> }
> >> >>
> >> >> + cdns->phyrst_a_enable = device_property_read_bool(dev,
> >> >> +"cdns,phyrst-a-enable");
> >> >> +
> >> >> cdns->otg_res = *res;
> >> >>
> >> >> mutex_init(&cdns->mutex);
> >> >> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> >> >> index 1ad1f1fe61e9..24cf0f1b5726 100644
> >> >> --- a/drivers/usb/cdns3/core.h
> >> >> +++ b/drivers/usb/cdns3/core.h
> >> >> @@ -76,6 +76,7 @@ struct cdns3 {
> >> >> #define CDNS3_CONTROLLER_V0 0
> >> >> #define CDNS3_CONTROLLER_V1 1
> >> >> u32 version;
> >> >> + bool phyrst_a_enable;
> >> >>
> >> >> int otg_irq;
> >> >> int dev_irq;
> >> >> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> >> >> index 6234bcd6158a..b74803e9703d 100644
> >> >> --- a/drivers/usb/cdns3/drd.c
> >> >> +++ b/drivers/usb/cdns3/drd.c
> >> >> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum
> usb_dr_mode mode)
> >> >> reg = readl(&cdns->otg_v1_regs->override);
> >> >> reg |= OVERRIDE_IDPULLUP;
> >> >> writel(reg, &cdns->otg_v1_regs->override);
> >> >> +
> >> >> + /*
> >> >> + * Enable work around feature built into the
> >> >> + * controller to address issue with RX Sensitivity
> >> >> + * est (EL_17) for USB2 PHY. The issue only occures
> >> >> + * for 0x0002450D controller version.
> >> >> + */
> >> >> + if (cdns->phyrst_a_enable) {
> >> >> + reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
> >> >> + reg |= PHYRST_CFG_PHYRST_A_ENABLE;
> >> >> + writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
> >> >> + }
> >> >> } else {
> >> >> reg = readl(&cdns->otg_v0_regs->ctrl1);
> >> >> reg |= OVERRIDE_IDPULLUP_V0;
> >> >> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> >> >> index 7e7cf7fa2dd3..f1ccae285a16 100644
> >> >> --- a/drivers/usb/cdns3/drd.h
> >> >> +++ b/drivers/usb/cdns3/drd.h
> >> >> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
> >> >> __le32 simulate;
> >> >> __le32 override;
> >> >> __le32 susp_ctrl;
> >> >> - __le32 reserved4;
> >> >> + __le32 phyrst_cfg;
> >> >> __le32 anasts;
> >> >> __le32 adp_ramp_time;
> >> >> __le32 ctrl1;
> >> >> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
> >> >> /* Only for CDNS3_CONTROLLER_V0 version */
> >> >> #define OVERRIDE_IDPULLUP_V0 BIT(24)
> >> >>
> >> >> +/* PHYRST_CFG - bitmasks */
> >> >> +#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
> >> >> +
> >> >> #define CDNS3_ID_PERIPHERAL 1
> >> >> #define CDNS3_ID_HOST 0
> >> >>
> >> >> --
> >> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> >> >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> >> >>
> >> >
> >> >--
> >> >
> >> >Thanks,
> >> >Peter Chen
> >
> >--
> >
> >Thanks,
> >Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-26 8:07 ` Peter Chen
@ 2020-08-26 12:49 ` Roger Quadros
2020-08-27 0:24 ` Peter Chen
0 siblings, 1 reply; 17+ messages in thread
From: Roger Quadros @ 2020-08-26 12:49 UTC (permalink / raw)
To: Peter Chen, Pawel Laszczak
Cc: balbi, Rahul Kumar, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree
Peter,
On 26/08/2020 11:07, Peter Chen wrote:
>
>>
>>
>>>
>>> On 20-08-26 04:04:01, Pawel Laszczak wrote:
>>>>> On 20-08-25 15:00:59, Roger Quadros wrote:
>>>>>> From: Pawel Laszczak <pawell@cadence.com>
>>>>>>
>>>>>> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
>>>>>> amplitude is varied below and above the Squelch Level of Receiver
>>>>>> during the active packet multiple times.
>>>>>>
>>>>>> Version 1 of the controller allows PHY to be reset when RX fail
>>>>>> condition is detected to work around the above issue. This feature
>>>>>> is disabled by default and needs to be enabled using a bit from
>>>>>> the newly added PHYRST_CFG register. This patch enables the workaround.
>>>>>>
>>>>>> As there is no way to distinguish between the controller version
>>>>>> before the device controller is started we need to rely on a DT
>>>>>> property to decide when to apply the workaround.
>>>>>
>>>>> Pawel, it could know the controller version at cdns3_gadget_start,
>>>>> but the controller starts when it tries to bind gadget driver, at
>>>>> that time, it has already known the controller version.
>>>>>
>>>>> For me, the device controller starts is using USB_CONF.DEVEN (Device
>>>>> Enable) through usb_gadget_connect, I am not sure if it is the same
>>>>> with yours.
>>>>>
>>>>
>>>> Yes in device mode driver knows controller version but this
>>>> workaround Must be enabled also in host mode. In host mode the
>>>> controller doesn't have access to device registers. The controller
>>>> version is placed in device register.
>>>>
>>>
>>> You may suggest your design team adding CHIP_VER register at global
>>> register region, it will easy the software engineer life.
>>>
>> >From what I read, this register is only enabling USB2 PHY reset
>>> software control, it needs for all chips with rev 0x0002450D, and the
>>> place you current change is only for 0x0002450D, right?
>>
>> Even I could say that this workaround should be enabled only for Specific USB2
>> PHY (only 0x0002450D)
>>
>> This bit should not have any impact for Cadence PHY but it can has Impact for third
>> party PHYs.
>>
>
> So, it is related to specific PHY, but enable this specific PHY reset bit is at controller region, why don't
> put this enable bit at PHY region?
I think this is related to Controller + PHY combination.
The fix for the issue is via a bit in the controller, so it needs to be managed by the
controller driver.
>
> So, you use controller's device property to know this specific PHY, can controller know this specific
> PHY dynamically?
Still the PHY will have to tell the controller the enable that bit. How to do that?
Adding a dt-property that vendors can used was the simplest option.
cheers,
-roger
>
> Peter
>
>> Pawel
>>
>>>
>>> Peter
>>>
>>>> Pawel
>>>>
>>>>> Peter
>>>>>
>>>>>
>>>>>>
>>>>>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>> ---
>>>>>> drivers/usb/cdns3/core.c | 2 ++
>>>>>> drivers/usb/cdns3/core.h | 1 +
>>>>>> drivers/usb/cdns3/drd.c | 12 ++++++++++++
>>>>>> drivers/usb/cdns3/drd.h | 5 ++++-
>>>>>> 4 files changed, 19 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>>>> index 5c1586ec7824..34b36487682b 100644
>>>>>> --- a/drivers/usb/cdns3/core.c
>>>>>> +++ b/drivers/usb/cdns3/core.c
>>>>>> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
>>>>>> return -ENXIO;
>>>>>> }
>>>>>>
>>>>>> + cdns->phyrst_a_enable = device_property_read_bool(dev,
>>>>>> +"cdns,phyrst-a-enable");
>>>>>> +
>>>>>> cdns->otg_res = *res;
>>>>>>
>>>>>> mutex_init(&cdns->mutex);
>>>>>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>>>>>> index 1ad1f1fe61e9..24cf0f1b5726 100644
>>>>>> --- a/drivers/usb/cdns3/core.h
>>>>>> +++ b/drivers/usb/cdns3/core.h
>>>>>> @@ -76,6 +76,7 @@ struct cdns3 {
>>>>>> #define CDNS3_CONTROLLER_V0 0
>>>>>> #define CDNS3_CONTROLLER_V1 1
>>>>>> u32 version;
>>>>>> + bool phyrst_a_enable;
>>>>>>
>>>>>> int otg_irq;
>>>>>> int dev_irq;
>>>>>> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
>>>>>> index 6234bcd6158a..b74803e9703d 100644
>>>>>> --- a/drivers/usb/cdns3/drd.c
>>>>>> +++ b/drivers/usb/cdns3/drd.c
>>>>>> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum
>> usb_dr_mode mode)
>>>>>> reg = readl(&cdns->otg_v1_regs->override);
>>>>>> reg |= OVERRIDE_IDPULLUP;
>>>>>> writel(reg, &cdns->otg_v1_regs->override);
>>>>>> +
>>>>>> + /*
>>>>>> + * Enable work around feature built into the
>>>>>> + * controller to address issue with RX Sensitivity
>>>>>> + * est (EL_17) for USB2 PHY. The issue only occures
>>>>>> + * for 0x0002450D controller version.
>>>>>> + */
>>>>>> + if (cdns->phyrst_a_enable) {
>>>>>> + reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
>>>>>> + reg |= PHYRST_CFG_PHYRST_A_ENABLE;
>>>>>> + writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
>>>>>> + }
>>>>>> } else {
>>>>>> reg = readl(&cdns->otg_v0_regs->ctrl1);
>>>>>> reg |= OVERRIDE_IDPULLUP_V0;
>>>>>> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
>>>>>> index 7e7cf7fa2dd3..f1ccae285a16 100644
>>>>>> --- a/drivers/usb/cdns3/drd.h
>>>>>> +++ b/drivers/usb/cdns3/drd.h
>>>>>> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
>>>>>> __le32 simulate;
>>>>>> __le32 override;
>>>>>> __le32 susp_ctrl;
>>>>>> - __le32 reserved4;
>>>>>> + __le32 phyrst_cfg;
>>>>>> __le32 anasts;
>>>>>> __le32 adp_ramp_time;
>>>>>> __le32 ctrl1;
>>>>>> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
>>>>>> /* Only for CDNS3_CONTROLLER_V0 version */
>>>>>> #define OVERRIDE_IDPULLUP_V0 BIT(24)
>>>>>>
>>>>>> +/* PHYRST_CFG - bitmasks */
>>>>>> +#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
>>>>>> +
>>>>>> #define CDNS3_ID_PERIPHERAL 1
>>>>>> #define CDNS3_ID_HOST 0
>>>>>>
>>>>>> --
>>>>>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>>>>>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>>>>>
>>>>>
>>>>> --
>>>>>
>>>>> Thanks,
>>>>> Peter Chen
>>>
>>> --
>>>
>>> Thanks,
>>> Peter Chen
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-26 12:49 ` Roger Quadros
@ 2020-08-27 0:24 ` Peter Chen
2020-08-27 9:36 ` Roger Quadros
0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2020-08-27 0:24 UTC (permalink / raw)
To: Roger Quadros
Cc: Pawel Laszczak, balbi, Rahul Kumar, nsekhar, vigneshr, robh+dt,
linux-usb, linux-kernel, devicetree
On 20-08-26 15:49:57, Roger Quadros wrote:
> Peter,
>
> On 26/08/2020 11:07, Peter Chen wrote:
> > >
> > >
> > > >
> > > > On 20-08-26 04:04:01, Pawel Laszczak wrote:
> > > > > > On 20-08-25 15:00:59, Roger Quadros wrote:
> > > > > > > From: Pawel Laszczak <pawell@cadence.com>
> > > > > > >
> > > > > > > USB2.0 PHY hangs in Rx Compliance test when the incoming packet
> > > > > > > amplitude is varied below and above the Squelch Level of Receiver
> > > > > > > during the active packet multiple times.
> > > > > > >
> > > > > > > Version 1 of the controller allows PHY to be reset when RX fail
> > > > > > > condition is detected to work around the above issue. This feature
> > > > > > > is disabled by default and needs to be enabled using a bit from
> > > > > > > the newly added PHYRST_CFG register. This patch enables the workaround.
> > > > > > >
> > > > > > > As there is no way to distinguish between the controller version
> > > > > > > before the device controller is started we need to rely on a DT
> > > > > > > property to decide when to apply the workaround.
> > > > > >
> > > > > > Pawel, it could know the controller version at cdns3_gadget_start,
> > > > > > but the controller starts when it tries to bind gadget driver, at
> > > > > > that time, it has already known the controller version.
> > > > > >
> > > > > > For me, the device controller starts is using USB_CONF.DEVEN (Device
> > > > > > Enable) through usb_gadget_connect, I am not sure if it is the same
> > > > > > with yours.
> > > > > >
> > > > >
> > > > > Yes in device mode driver knows controller version but this
> > > > > workaround Must be enabled also in host mode. In host mode the
> > > > > controller doesn't have access to device registers. The controller
> > > > > version is placed in device register.
> > > > >
> > > >
> > > > You may suggest your design team adding CHIP_VER register at global
> > > > register region, it will easy the software engineer life.
> > > >
> > > >From what I read, this register is only enabling USB2 PHY reset
> > > > software control, it needs for all chips with rev 0x0002450D, and the
> > > > place you current change is only for 0x0002450D, right?
> > >
> > > Even I could say that this workaround should be enabled only for Specific USB2
> > > PHY (only 0x0002450D)
> > >
> > > This bit should not have any impact for Cadence PHY but it can has Impact for third
> > > party PHYs.
> > >
> >
> > So, it is related to specific PHY, but enable this specific PHY reset bit is at controller region, why don't
> > put this enable bit at PHY region?
>
> I think this is related to Controller + PHY combination.
> The fix for the issue is via a bit in the controller, so it needs to be managed by the
> controller driver.
>
> >
> > So, you use controller's device property to know this specific PHY, can controller know this specific
> > PHY dynamically?
>
> Still the PHY will have to tell the controller the enable that bit. How to do that?
>
> Adding a dt-property that vendors can used was the simplest option.
>
Ok, does all controllers with ver 0x0002450D need this fix? I just think
if we introduce a flag stands for ver 0x0002450D in case this ver has
other issues in future or just using phy reset enable property?
Pawel & Roger, what's your opinion?
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-27 0:24 ` Peter Chen
@ 2020-08-27 9:36 ` Roger Quadros
2020-08-27 13:02 ` Felipe Balbi
0 siblings, 1 reply; 17+ messages in thread
From: Roger Quadros @ 2020-08-27 9:36 UTC (permalink / raw)
To: Peter Chen
Cc: Pawel Laszczak, balbi, Rahul Kumar, nsekhar, vigneshr, robh+dt,
linux-usb, linux-kernel, devicetree
On 27/08/2020 03:24, Peter Chen wrote:
> On 20-08-26 15:49:57, Roger Quadros wrote:
>> Peter,
>>
>> On 26/08/2020 11:07, Peter Chen wrote:
>>>>
>>>>
>>>>>
>>>>> On 20-08-26 04:04:01, Pawel Laszczak wrote:
>>>>>>> On 20-08-25 15:00:59, Roger Quadros wrote:
>>>>>>>> From: Pawel Laszczak <pawell@cadence.com>
>>>>>>>>
>>>>>>>> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
>>>>>>>> amplitude is varied below and above the Squelch Level of Receiver
>>>>>>>> during the active packet multiple times.
>>>>>>>>
>>>>>>>> Version 1 of the controller allows PHY to be reset when RX fail
>>>>>>>> condition is detected to work around the above issue. This feature
>>>>>>>> is disabled by default and needs to be enabled using a bit from
>>>>>>>> the newly added PHYRST_CFG register. This patch enables the workaround.
>>>>>>>>
>>>>>>>> As there is no way to distinguish between the controller version
>>>>>>>> before the device controller is started we need to rely on a DT
>>>>>>>> property to decide when to apply the workaround.
>>>>>>>
>>>>>>> Pawel, it could know the controller version at cdns3_gadget_start,
>>>>>>> but the controller starts when it tries to bind gadget driver, at
>>>>>>> that time, it has already known the controller version.
>>>>>>>
>>>>>>> For me, the device controller starts is using USB_CONF.DEVEN (Device
>>>>>>> Enable) through usb_gadget_connect, I am not sure if it is the same
>>>>>>> with yours.
>>>>>>>
>>>>>>
>>>>>> Yes in device mode driver knows controller version but this
>>>>>> workaround Must be enabled also in host mode. In host mode the
>>>>>> controller doesn't have access to device registers. The controller
>>>>>> version is placed in device register.
>>>>>>
>>>>>
>>>>> You may suggest your design team adding CHIP_VER register at global
>>>>> register region, it will easy the software engineer life.
>>>>>
>>>> >From what I read, this register is only enabling USB2 PHY reset
>>>>> software control, it needs for all chips with rev 0x0002450D, and the
>>>>> place you current change is only for 0x0002450D, right?
>>>>
>>>> Even I could say that this workaround should be enabled only for Specific USB2
>>>> PHY (only 0x0002450D)
>>>>
>>>> This bit should not have any impact for Cadence PHY but it can has Impact for third
>>>> party PHYs.
>>>>
>>>
>>> So, it is related to specific PHY, but enable this specific PHY reset bit is at controller region, why don't
>>> put this enable bit at PHY region?
>>
>> I think this is related to Controller + PHY combination.
>> The fix for the issue is via a bit in the controller, so it needs to be managed by the
>> controller driver.
>>
>>>
>>> So, you use controller's device property to know this specific PHY, can controller know this specific
>>> PHY dynamically?
>>
>> Still the PHY will have to tell the controller the enable that bit. How to do that?
>>
>> Adding a dt-property that vendors can used was the simplest option.
>>
>
> Ok, does all controllers with ver 0x0002450D need this fix? I just think
> if we introduce a flag stands for ver 0x0002450D in case this ver has
> other issues in future or just using phy reset enable property?
>
> Pawel & Roger, what's your opinion?
>
I think it is best to keep the flags specific to the issue rather than a one flag for
all issues with a specific version. This way you can re-use the flag irrespective
of IP version.
But best case is that Cadence put a IP revision register in common area as you
have previously suggested so driver can automatically apply quirks to specific
versions.
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-27 9:36 ` Roger Quadros
@ 2020-08-27 13:02 ` Felipe Balbi
0 siblings, 0 replies; 17+ messages in thread
From: Felipe Balbi @ 2020-08-27 13:02 UTC (permalink / raw)
To: Roger Quadros, Peter Chen
Cc: Pawel Laszczak, Rahul Kumar, nsekhar, vigneshr, robh+dt,
linux-usb, linux-kernel, devicetree
[-- Attachment #1: Type: text/plain, Size: 3653 bytes --]
Hi,
Roger Quadros <rogerq@ti.com> writes:
>>>>>>>>> From: Pawel Laszczak <pawell@cadence.com>
>>>>>>>>>
>>>>>>>>> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
>>>>>>>>> amplitude is varied below and above the Squelch Level of Receiver
>>>>>>>>> during the active packet multiple times.
>>>>>>>>>
>>>>>>>>> Version 1 of the controller allows PHY to be reset when RX fail
>>>>>>>>> condition is detected to work around the above issue. This feature
>>>>>>>>> is disabled by default and needs to be enabled using a bit from
>>>>>>>>> the newly added PHYRST_CFG register. This patch enables the workaround.
>>>>>>>>>
>>>>>>>>> As there is no way to distinguish between the controller version
>>>>>>>>> before the device controller is started we need to rely on a DT
>>>>>>>>> property to decide when to apply the workaround.
>>>>>>>>
>>>>>>>> Pawel, it could know the controller version at cdns3_gadget_start,
>>>>>>>> but the controller starts when it tries to bind gadget driver, at
>>>>>>>> that time, it has already known the controller version.
>>>>>>>>
>>>>>>>> For me, the device controller starts is using USB_CONF.DEVEN (Device
>>>>>>>> Enable) through usb_gadget_connect, I am not sure if it is the same
>>>>>>>> with yours.
>>>>>>>>
>>>>>>>
>>>>>>> Yes in device mode driver knows controller version but this
>>>>>>> workaround Must be enabled also in host mode. In host mode the
>>>>>>> controller doesn't have access to device registers. The controller
>>>>>>> version is placed in device register.
>>>>>>>
>>>>>>
>>>>>> You may suggest your design team adding CHIP_VER register at global
>>>>>> register region, it will easy the software engineer life.
>>>>>>
>>>>> >From what I read, this register is only enabling USB2 PHY reset
>>>>>> software control, it needs for all chips with rev 0x0002450D, and the
>>>>>> place you current change is only for 0x0002450D, right?
>>>>>
>>>>> Even I could say that this workaround should be enabled only for Specific USB2
>>>>> PHY (only 0x0002450D)
>>>>>
>>>>> This bit should not have any impact for Cadence PHY but it can has Impact for third
>>>>> party PHYs.
>>>>>
>>>>
>>>> So, it is related to specific PHY, but enable this specific PHY reset bit is at controller region, why don't
>>>> put this enable bit at PHY region?
>>>
>>> I think this is related to Controller + PHY combination.
>>> The fix for the issue is via a bit in the controller, so it needs to be managed by the
>>> controller driver.
>>>
>>>>
>>>> So, you use controller's device property to know this specific PHY, can controller know this specific
>>>> PHY dynamically?
>>>
>>> Still the PHY will have to tell the controller the enable that bit. How to do that?
>>>
>>> Adding a dt-property that vendors can used was the simplest option.
>>>
>>
>> Ok, does all controllers with ver 0x0002450D need this fix? I just think
>> if we introduce a flag stands for ver 0x0002450D in case this ver has
>> other issues in future or just using phy reset enable property?
>>
>> Pawel & Roger, what's your opinion?
>>
> I think it is best to keep the flags specific to the issue rather than
> a one flag for all issues with a specific version. This way you can
> re-use the flag irrespective of IP version.
I second that. Specially when some SoC-manufacturers may implement ECO
fixes and not change IP revision.
> But best case is that Cadence put a IP revision register in common area as you
> have previously suggested so driver can automatically apply quirks to specific
> versions.
little too late for that :-)
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup
2020-08-25 12:00 ` [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup Roger Quadros
2020-08-26 3:21 ` Peter Chen
@ 2020-08-27 11:09 ` Peter Chen
1 sibling, 0 replies; 17+ messages in thread
From: Peter Chen @ 2020-08-27 11:09 UTC (permalink / raw)
To: Roger Quadros
Cc: balbi, pawell, kurahul, nsekhar, vigneshr, robh+dt, linux-usb,
linux-kernel, devicetree
On 20-08-25 15:00:59, Roger Quadros wrote:
> From: Pawel Laszczak <pawell@cadence.com>
>
> USB2.0 PHY hangs in Rx Compliance test when the incoming packet
> amplitude is varied below and above the Squelch Level of
> Receiver during the active packet multiple times.
>
> Version 1 of the controller allows PHY to be reset when RX fail condition
> is detected to work around the above issue. This feature is
> disabled by default and needs to be enabled using a bit from
> the newly added PHYRST_CFG register. This patch enables the workaround.
>
> As there is no way to distinguish between the controller version
> before the device controller is started we need to rely on a
> DT property to decide when to apply the workaround.
>
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
Reviewed-by: Peter Chen <peter.chen@nxp.com>
Peter
> ---
> drivers/usb/cdns3/core.c | 2 ++
> drivers/usb/cdns3/core.h | 1 +
> drivers/usb/cdns3/drd.c | 12 ++++++++++++
> drivers/usb/cdns3/drd.h | 5 ++++-
> 4 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 5c1586ec7824..34b36487682b 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
> return -ENXIO;
> }
>
> + cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
> +
> cdns->otg_res = *res;
>
> mutex_init(&cdns->mutex);
> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> index 1ad1f1fe61e9..24cf0f1b5726 100644
> --- a/drivers/usb/cdns3/core.h
> +++ b/drivers/usb/cdns3/core.h
> @@ -76,6 +76,7 @@ struct cdns3 {
> #define CDNS3_CONTROLLER_V0 0
> #define CDNS3_CONTROLLER_V1 1
> u32 version;
> + bool phyrst_a_enable;
>
> int otg_irq;
> int dev_irq;
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 6234bcd6158a..b74803e9703d 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
> reg = readl(&cdns->otg_v1_regs->override);
> reg |= OVERRIDE_IDPULLUP;
> writel(reg, &cdns->otg_v1_regs->override);
> +
> + /*
> + * Enable work around feature built into the
> + * controller to address issue with RX Sensitivity
> + * est (EL_17) for USB2 PHY. The issue only occures
> + * for 0x0002450D controller version.
> + */
> + if (cdns->phyrst_a_enable) {
> + reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
> + reg |= PHYRST_CFG_PHYRST_A_ENABLE;
> + writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
> + }
> } else {
> reg = readl(&cdns->otg_v0_regs->ctrl1);
> reg |= OVERRIDE_IDPULLUP_V0;
> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> index 7e7cf7fa2dd3..f1ccae285a16 100644
> --- a/drivers/usb/cdns3/drd.h
> +++ b/drivers/usb/cdns3/drd.h
> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
> __le32 simulate;
> __le32 override;
> __le32 susp_ctrl;
> - __le32 reserved4;
> + __le32 phyrst_cfg;
> __le32 anasts;
> __le32 adp_ramp_time;
> __le32 ctrl1;
> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
> /* Only for CDNS3_CONTROLLER_V0 version */
> #define OVERRIDE_IDPULLUP_V0 BIT(24)
>
> +/* PHYRST_CFG - bitmasks */
> +#define PHYRST_CFG_PHYRST_A_ENABLE BIT(0)
> +
> #define CDNS3_ID_PERIPHERAL 1
> #define CDNS3_ID_HOST 0
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread