devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] usb: dwc3: Add USB3 Gen2 De-emphasis setting from DT
@ 2024-10-07 13:55 Joy Chakraborty
  2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis Joy Chakraborty
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-07 13:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi
  Cc: linux-usb, devicetree, linux-kernel, Joy Chakraborty

This Patch series adds support for setting 18 bit de-emphasis setting in
PIPE4 spec from dwc3 Link registers based on quirks passed from device
tree.

Joy Chakraborty (2):
  dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  usb: dwc3: Program USB Gen2 de-emphasis defined in PIPE4 spec

 .../devicetree/bindings/usb/snps,dwc3.yaml          | 12 ++++++++++++
 drivers/usb/dwc3/core.c                             | 13 +++++++++++++
 drivers/usb/dwc3/core.h                             |  6 ++++++
 3 files changed, 31 insertions(+)

-- 
2.47.0.rc0.187.ge670bccf7e-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-07 13:55 [PATCH 0/2] usb: dwc3: Add USB3 Gen2 De-emphasis setting from DT Joy Chakraborty
@ 2024-10-07 13:55 ` Joy Chakraborty
  2024-10-07 14:56   ` Krzysztof Kozlowski
  2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 De-emphasis Joy Chakraborty
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-07 13:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi
  Cc: linux-usb, devicetree, linux-kernel, Joy Chakraborty

PIPE4 spec defines an 18bit de-emphasis setting to be passed from
controller to the PHY.
TxDeemph[17:0] is split as [5:0] C-1, [11:6] C0, [17:12] C+1 for 3 tap
filter used for USB Gen2(10GT/s).

Signed-off-by: Joy Chakraborty <joychakr@google.com>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 1cd0ca90127d..a1f1bbcf1467 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -190,6 +190,18 @@ properties:
       - 1 # -3.5dB de-emphasis
       - 2 # No de-emphasis
 
+  snps,tx_gen2_de_emphasis_quirk:
+    description: When set core will set Tx de-emphasis for USB Gen2
+    type: boolean
+
+  snps,tx_gen2_de_emphasis:
+    description:
+      The 18bit value of Tx deemphasis defined in PIPE4 spec driven to PHY
+      for normal operation.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 0x3ffff
+
   snps,dis_u3_susphy_quirk:
     description: When set core will disable USB3 suspend phy
     type: boolean
-- 
2.47.0.rc0.187.ge670bccf7e-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 De-emphasis
  2024-10-07 13:55 [PATCH 0/2] usb: dwc3: Add USB3 Gen2 De-emphasis setting from DT Joy Chakraborty
  2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis Joy Chakraborty
@ 2024-10-07 13:55 ` Joy Chakraborty
  2024-10-07 13:55 ` [PATCH 2/2] usb: dwc3: Program USB Gen2 de-emphasis defined in PIPE4 spec Joy Chakraborty
  2024-10-07 13:55 ` [PATCH 2/2] usb: dwc3: Program USB Gen2 De-emphasis " Joy Chakraborty
  3 siblings, 0 replies; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-07 13:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi
  Cc: linux-usb, devicetree, linux-kernel, Joy Chakraborty

PIPE4 spec defines an 18bit de-emphasis setting to be passed from
controller to the PHY.
TxDeemph[17:0] is split as [5:0] C-1, [11:6] C0, [17:12] C+1 for 3 tap
filter used for USB Gen2(10GT/s).

Signed-off-by: Joy Chakraborty <joychakr@google.com>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 1cd0ca90127d..a1f1bbcf1467 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -190,6 +190,18 @@ properties:
       - 1 # -3.5dB de-emphasis
       - 2 # No de-emphasis
 
+  snps,tx_gen2_de_emphasis_quirk:
+    description: When set core will set Tx de-emphasis for USB Gen2
+    type: boolean
+
+  snps,tx_gen2_de_emphasis:
+    description:
+      The 18bit value of Tx deemphasis defined in PIPE4 spec driven to PHY
+      for normal operation.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 0x3ffff
+
   snps,dis_u3_susphy_quirk:
     description: When set core will disable USB3 suspend phy
     type: boolean
-- 
2.47.0.rc0.187.ge670bccf7e-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] usb: dwc3: Program USB Gen2 de-emphasis defined in PIPE4 spec
  2024-10-07 13:55 [PATCH 0/2] usb: dwc3: Add USB3 Gen2 De-emphasis setting from DT Joy Chakraborty
  2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis Joy Chakraborty
  2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 De-emphasis Joy Chakraborty
@ 2024-10-07 13:55 ` Joy Chakraborty
  2024-10-07 13:55 ` [PATCH 2/2] usb: dwc3: Program USB Gen2 De-emphasis " Joy Chakraborty
  3 siblings, 0 replies; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-07 13:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi
  Cc: linux-usb, devicetree, linux-kernel, Joy Chakraborty

Set 18bit TxDeemph[17:0] in LCSR_TX_DEEMPH(n) register for USB3 Gen2
Normal Operation as defined in PIPE4 spec based on dt quirk
"snps,tx_gen2_de_emphasis_quirk" and "snps,tx_gen2_de_emphasis".

Signed-off-by: Joy Chakraborty <joychakr@google.com>
---
 drivers/usb/dwc3/core.c | 13 +++++++++++++
 drivers/usb/dwc3/core.h |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9eb085f359ce..25e19aea364a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -693,6 +693,11 @@ static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
 
 	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
 
+	if (dwc->tx_gen2_de_emphasis_quirk) {
+		reg = dwc->tx_gen2_de_emphasis;
+		dwc3_writel(dwc->regs, DWC3_LCSR_TX_DEEMPH(index), reg);
+	}
+
 	return 0;
 }
 
@@ -1654,6 +1659,7 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 	u8			tx_thr_num_pkt_prd = 0;
 	u8			tx_max_burst_prd = 0;
 	u8			tx_fifo_resize_max_num;
+	u32			tx_gen2_de_emphasis = 0;
 	const char		*usb_psy_name;
 	int			ret;
 
@@ -1797,8 +1803,15 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 	dwc->dis_split_quirk = device_property_read_bool(dev,
 				"snps,dis-split-quirk");
 
+	dwc->tx_gen2_de_emphasis_quirk = device_property_read_bool(dev,
+								   "snps,tx_gen2_de_emphasis_quirk");
+	if (dwc->tx_gen2_de_emphasis_quirk)
+		device_property_read_u32(dev, "snps,tx_gen2_de_emphasis",
+					 &tx_gen2_de_emphasis);
+
 	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
 	dwc->tx_de_emphasis = tx_de_emphasis;
+	dwc->tx_gen2_de_emphasis = tx_gen2_de_emphasis;
 
 	dwc->hird_threshold = hird_threshold;
 
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index c71240e8f7c7..fa9db38b7e15 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -179,7 +179,9 @@
 #define DWC3_OEVTEN		0xcc0C
 #define DWC3_OSTS		0xcc10
 
+/* Link Registers */
 #define DWC3_LLUCTL		0xd024
+#define DWC3_LCSR_TX_DEEMPH(n)	(0xd060 + ((n) * 0x80))
 
 /* Bit fields */
 
@@ -1145,6 +1147,8 @@ struct dwc3_scratchpad_array {
  *	1	- -3.5dB de-emphasis
  *	2	- No de-emphasis
  *	3	- Reserved
+ * @tx_gen2_de_emphasis_quirk: set if we enable USB Gen2 Tx de-emphasis quirk
+ * @tx_gen2_de_emphasis: Tx de-emphasis value used in USB Gen2 with PIPE4
  * @dis_metastability_quirk: set to disable metastability quirk.
  * @dis_split_quirk: set to disable split boundary.
  * @sys_wakeup: set if the device may do system wakeup.
@@ -1374,6 +1378,8 @@ struct dwc3 {
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
+	unsigned		tx_gen2_de_emphasis_quirk:1;
+	unsigned		tx_gen2_de_emphasis:18;
 
 	unsigned		dis_metastability_quirk:1;
 
-- 
2.47.0.rc0.187.ge670bccf7e-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] usb: dwc3: Program USB Gen2 De-emphasis defined in PIPE4 spec
  2024-10-07 13:55 [PATCH 0/2] usb: dwc3: Add USB3 Gen2 De-emphasis setting from DT Joy Chakraborty
                   ` (2 preceding siblings ...)
  2024-10-07 13:55 ` [PATCH 2/2] usb: dwc3: Program USB Gen2 de-emphasis defined in PIPE4 spec Joy Chakraborty
@ 2024-10-07 13:55 ` Joy Chakraborty
  2024-10-16  8:16   ` Greg Kroah-Hartman
  3 siblings, 1 reply; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-07 13:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi
  Cc: linux-usb, devicetree, linux-kernel, Joy Chakraborty

Set 18bit TxDeemph[17:0] in LCSR_TX_DEEMPH(n) register for USB3 Gen2
Normal Operation as defined in PIPE4 spec based on dt quirk
"snps,tx_gen2_de_emphasis_quirk" and "snps,tx_gen2_de_emphasis".

Signed-off-by: Joy Chakraborty <joychakr@google.com>
---
 drivers/usb/dwc3/core.c | 13 +++++++++++++
 drivers/usb/dwc3/core.h |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9eb085f359ce..25e19aea364a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -693,6 +693,11 @@ static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
 
 	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
 
+	if (dwc->tx_gen2_de_emphasis_quirk) {
+		reg = dwc->tx_gen2_de_emphasis;
+		dwc3_writel(dwc->regs, DWC3_LCSR_TX_DEEMPH(index), reg);
+	}
+
 	return 0;
 }
 
@@ -1654,6 +1659,7 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 	u8			tx_thr_num_pkt_prd = 0;
 	u8			tx_max_burst_prd = 0;
 	u8			tx_fifo_resize_max_num;
+	u32			tx_gen2_de_emphasis = 0;
 	const char		*usb_psy_name;
 	int			ret;
 
@@ -1797,8 +1803,15 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 	dwc->dis_split_quirk = device_property_read_bool(dev,
 				"snps,dis-split-quirk");
 
+	dwc->tx_gen2_de_emphasis_quirk = device_property_read_bool(dev,
+								   "snps,tx_gen2_de_emphasis_quirk");
+	if (dwc->tx_gen2_de_emphasis_quirk)
+		device_property_read_u32(dev, "snps,tx_gen2_de_emphasis",
+					 &tx_gen2_de_emphasis);
+
 	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
 	dwc->tx_de_emphasis = tx_de_emphasis;
+	dwc->tx_gen2_de_emphasis = tx_gen2_de_emphasis;
 
 	dwc->hird_threshold = hird_threshold;
 
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index c71240e8f7c7..fa9db38b7e15 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -179,7 +179,9 @@
 #define DWC3_OEVTEN		0xcc0C
 #define DWC3_OSTS		0xcc10
 
+/* Link Registers */
 #define DWC3_LLUCTL		0xd024
+#define DWC3_LCSR_TX_DEEMPH(n)	(0xd060 + ((n) * 0x80))
 
 /* Bit fields */
 
@@ -1145,6 +1147,8 @@ struct dwc3_scratchpad_array {
  *	1	- -3.5dB de-emphasis
  *	2	- No de-emphasis
  *	3	- Reserved
+ * @tx_gen2_de_emphasis_quirk: set if we enable USB Gen2 Tx de-emphasis quirk
+ * @tx_gen2_de_emphasis: Tx de-emphasis value used in USB Gen2 with PIPE4
  * @dis_metastability_quirk: set to disable metastability quirk.
  * @dis_split_quirk: set to disable split boundary.
  * @sys_wakeup: set if the device may do system wakeup.
@@ -1374,6 +1378,8 @@ struct dwc3 {
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
+	unsigned		tx_gen2_de_emphasis_quirk:1;
+	unsigned		tx_gen2_de_emphasis:18;
 
 	unsigned		dis_metastability_quirk:1;
 
-- 
2.47.0.rc0.187.ge670bccf7e-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis Joy Chakraborty
@ 2024-10-07 14:56   ` Krzysztof Kozlowski
  2024-10-08 10:23     ` Joy Chakraborty
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-07 14:56 UTC (permalink / raw)
  To: Joy Chakraborty, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen, Felipe Balbi
  Cc: linux-usb, devicetree, linux-kernel

On 07/10/2024 15:55, Joy Chakraborty wrote:
> PIPE4 spec defines an 18bit de-emphasis setting to be passed from
> controller to the PHY.
> TxDeemph[17:0] is split as [5:0] C-1, [11:6] C0, [17:12] C+1 for 3 tap
> filter used for USB Gen2(10GT/s).
> 
> Signed-off-by: Joy Chakraborty <joychakr@google.com>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 1cd0ca90127d..a1f1bbcf1467 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -190,6 +190,18 @@ properties:
>        - 1 # -3.5dB de-emphasis
>        - 2 # No de-emphasis
>  
> +  snps,tx_gen2_de_emphasis_quirk:

No underscores.

> +    description: When set core will set Tx de-emphasis for USB Gen2

And why it cannot be implied by compatible?

> +    type: boolean
> +
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-07 14:56   ` Krzysztof Kozlowski
@ 2024-10-08 10:23     ` Joy Chakraborty
  2024-10-08 11:23       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-08 10:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi, linux-usb, devicetree,
	linux-kernel

On Mon, Oct 7, 2024 at 8:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 07/10/2024 15:55, Joy Chakraborty wrote:
> > PIPE4 spec defines an 18bit de-emphasis setting to be passed from
> > controller to the PHY.
> > TxDeemph[17:0] is split as [5:0] C-1, [11:6] C0, [17:12] C+1 for 3 tap
> > filter used for USB Gen2(10GT/s).
> >
> > Signed-off-by: Joy Chakraborty <joychakr@google.com>
> > ---
> >  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > index 1cd0ca90127d..a1f1bbcf1467 100644
> > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > @@ -190,6 +190,18 @@ properties:
> >        - 1 # -3.5dB de-emphasis
> >        - 2 # No de-emphasis
> >
> > +  snps,tx_gen2_de_emphasis_quirk:
>
> No underscores.

Ack, will fix it with a follow up patch.

>
> > +    description: When set core will set Tx de-emphasis for USB Gen2
>
> And why it cannot be implied by compatible?

As per my understanding these are tuning coefficients for de-emphasis
particular to a platform and not the dwc3 controller, hence should not
be a controller compatible.
Similar to the property defined right above this definition which is
from PIPE3 spec for USB Gen1.

Thanks
Joy
>
> > +    type: boolean
> > +
> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-08 10:23     ` Joy Chakraborty
@ 2024-10-08 11:23       ` Krzysztof Kozlowski
  2024-10-08 11:59         ` Joy Chakraborty
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-08 11:23 UTC (permalink / raw)
  To: Joy Chakraborty
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi, linux-usb, devicetree,
	linux-kernel

On 08/10/2024 12:23, Joy Chakraborty wrote:
> On Mon, Oct 7, 2024 at 8:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 07/10/2024 15:55, Joy Chakraborty wrote:
>>> PIPE4 spec defines an 18bit de-emphasis setting to be passed from
>>> controller to the PHY.
>>> TxDeemph[17:0] is split as [5:0] C-1, [11:6] C0, [17:12] C+1 for 3 tap
>>> filter used for USB Gen2(10GT/s).
>>>
>>> Signed-off-by: Joy Chakraborty <joychakr@google.com>
>>> ---
>>>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> index 1cd0ca90127d..a1f1bbcf1467 100644
>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> @@ -190,6 +190,18 @@ properties:
>>>        - 1 # -3.5dB de-emphasis
>>>        - 2 # No de-emphasis
>>>
>>> +  snps,tx_gen2_de_emphasis_quirk:
>>
>> No underscores.
> 
> Ack, will fix it with a follow up patch.
> 
>>
>>> +    description: When set core will set Tx de-emphasis for USB Gen2
>>
>> And why it cannot be implied by compatible?
> 
> As per my understanding these are tuning coefficients for de-emphasis
> particular to a platform and not the dwc3 controller, hence should not
> be a controller compatible.

Platforms must have specific compatible, so this should be implied by
compatible.

> Similar to the property defined right above this definition which is
> from PIPE3 spec for USB Gen1.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-08 11:23       ` Krzysztof Kozlowski
@ 2024-10-08 11:59         ` Joy Chakraborty
  2024-10-08 12:10           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-08 11:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi, linux-usb, devicetree,
	linux-kernel

On Tue, Oct 8, 2024 at 4:53 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 08/10/2024 12:23, Joy Chakraborty wrote:
> > On Mon, Oct 7, 2024 at 8:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 07/10/2024 15:55, Joy Chakraborty wrote:
> >>> PIPE4 spec defines an 18bit de-emphasis setting to be passed from
> >>> controller to the PHY.
> >>> TxDeemph[17:0] is split as [5:0] C-1, [11:6] C0, [17:12] C+1 for 3 tap
> >>> filter used for USB Gen2(10GT/s).
> >>>
> >>> Signed-off-by: Joy Chakraborty <joychakr@google.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 12 ++++++++++++
> >>>  1 file changed, 12 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >>> index 1cd0ca90127d..a1f1bbcf1467 100644
> >>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >>> @@ -190,6 +190,18 @@ properties:
> >>>        - 1 # -3.5dB de-emphasis
> >>>        - 2 # No de-emphasis
> >>>
> >>> +  snps,tx_gen2_de_emphasis_quirk:
> >>
> >> No underscores.
> >
> > Ack, will fix it with a follow up patch.
> >
> >>
> >>> +    description: When set core will set Tx de-emphasis for USB Gen2
> >>
> >> And why it cannot be implied by compatible?
> >
> > As per my understanding these are tuning coefficients for de-emphasis
> > particular to a platform and not the dwc3 controller, hence should not
> > be a controller compatible.
>
> Platforms must have specific compatible, so this should be implied by
> compatible.

Maybe I am using the word "platform" incorrectly here, what I
understand is that the same controller(in a chip) when used on 2
different physical form factors might need different deemphasis
coefficient values to be passed to its Phy. Someone could correct me
from the USB link stand point if I am mistaken here.

Thanks
Joy

>
> > Similar to the property defined right above this definition which is
> > from PIPE3 spec for USB Gen1.
>
>
> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-08 11:59         ` Joy Chakraborty
@ 2024-10-08 12:10           ` Krzysztof Kozlowski
  2024-10-08 12:40             ` Joy Chakraborty
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-08 12:10 UTC (permalink / raw)
  To: Joy Chakraborty
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi, linux-usb, devicetree,
	linux-kernel

On 08/10/2024 13:59, Joy Chakraborty wrote:
>>>
>>>>
>>>>> +    description: When set core will set Tx de-emphasis for USB Gen2
>>>>
>>>> And why it cannot be implied by compatible?
>>>
>>> As per my understanding these are tuning coefficients for de-emphasis
>>> particular to a platform and not the dwc3 controller, hence should not
>>> be a controller compatible.
>>
>> Platforms must have specific compatible, so this should be implied by
>> compatible.
> 
> Maybe I am using the word "platform" incorrectly here, what I
> understand is that the same controller(in a chip) when used on 2
> different physical form factors might need different deemphasis

You mean boards? This is board-level property?

> coefficient values to be passed to its Phy. Someone could correct me
> from the USB link stand point if I am mistaken here.
> 

Then please point me to the upstream DTS boards using these properties.
Lore link is enough, if board or DTS change is being upstreamed.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-08 12:10           ` Krzysztof Kozlowski
@ 2024-10-08 12:40             ` Joy Chakraborty
  2024-10-08 12:59               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Joy Chakraborty @ 2024-10-08 12:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi, linux-usb, devicetree,
	linux-kernel

On Tue, Oct 8, 2024 at 5:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 08/10/2024 13:59, Joy Chakraborty wrote:
> >>>
> >>>>
> >>>>> +    description: When set core will set Tx de-emphasis for USB Gen2
> >>>>
> >>>> And why it cannot be implied by compatible?
> >>>
> >>> As per my understanding these are tuning coefficients for de-emphasis
> >>> particular to a platform and not the dwc3 controller, hence should not
> >>> be a controller compatible.
> >>
> >> Platforms must have specific compatible, so this should be implied by
> >> compatible.
> >
> > Maybe I am using the word "platform" incorrectly here, what I
> > understand is that the same controller(in a chip) when used on 2
> > different physical form factors might need different deemphasis
>
> You mean boards? This is board-level property?

Yes, the USB controller can be paired with different phys in a SoC and
used on different board layouts where we should be able to drive
different de-emphasis coefficients to the phy as per the link
equalization requirements is my understanding.

>
> > coefficient values to be passed to its Phy. Someone could correct me
> > from the USB link stand point if I am mistaken here.
> >
>
> Then please point me to the upstream DTS boards using these properties.
> Lore link is enough, if board or DTS change is being upstreamed.
>

The DTS change is not being upstreamed currently.
But this change would help bring up a new or development board where
USB compliance is being run and this parameter needs tuning,  hence
being able to upstream this would help.

> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis
  2024-10-08 12:40             ` Joy Chakraborty
@ 2024-10-08 12:59               ` Krzysztof Kozlowski
  0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-08 12:59 UTC (permalink / raw)
  To: Joy Chakraborty
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Thinh Nguyen, Felipe Balbi, linux-usb, devicetree,
	linux-kernel

On 08/10/2024 14:40, Joy Chakraborty wrote:
> On Tue, Oct 8, 2024 at 5:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 08/10/2024 13:59, Joy Chakraborty wrote:
>>>>>
>>>>>>
>>>>>>> +    description: When set core will set Tx de-emphasis for USB Gen2
>>>>>>
>>>>>> And why it cannot be implied by compatible?
>>>>>
>>>>> As per my understanding these are tuning coefficients for de-emphasis
>>>>> particular to a platform and not the dwc3 controller, hence should not
>>>>> be a controller compatible.
>>>>
>>>> Platforms must have specific compatible, so this should be implied by
>>>> compatible.
>>>
>>> Maybe I am using the word "platform" incorrectly here, what I
>>> understand is that the same controller(in a chip) when used on 2
>>> different physical form factors might need different deemphasis
>>
>> You mean boards? This is board-level property?
> 
> Yes, the USB controller can be paired with different phys in a SoC and

That's not board specific, but SoC.

> used on different board layouts where we should be able to drive
> different de-emphasis coefficients to the phy as per the link
> equalization requirements is my understanding.

You keep mixing different stories, so I am not convinced.

> 
>>
>>> coefficient values to be passed to its Phy. Someone could correct me
>>> from the USB link stand point if I am mistaken here.
>>>
>>
>> Then please point me to the upstream DTS boards using these properties.
>> Lore link is enough, if board or DTS change is being upstreamed.
>>
> 
> The DTS change is not being upstreamed currently.

Why do we want code without any user?

> But this change would help bring up a new or development board where
> USB compliance is being run and this parameter needs tuning,  hence
> being able to upstream this would help.

To me whatever Google or any other vendor is doing downstream does not
matter, just "does not exist".

Upstream the DTS so we can verify how this is exactly used.

To me it looks so far as SoC specific and your earlier comment about
pairing USB controller with phy is confirming this.

That's a common practice from Google (but not Chromium folks, they are
awesome!) and few other vendors to upstream whatever they have in their
GKI downstream, regardless whether it is right or not, whether it
follows rules or not, whether there is any user or not (and again: users
are upstream). Rationale for all this is the same - "we have downstream
some crap thus we want it".

Nah, upstream your stuff to be considered as a user.

That's a NAK, sorry.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] usb: dwc3: Program USB Gen2 De-emphasis defined in PIPE4 spec
  2024-10-07 13:55 ` [PATCH 2/2] usb: dwc3: Program USB Gen2 De-emphasis " Joy Chakraborty
@ 2024-10-16  8:16   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2024-10-16  8:16 UTC (permalink / raw)
  To: Joy Chakraborty
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, linux-usb, devicetree, linux-kernel

On Mon, Oct 07, 2024 at 01:55:08PM +0000, Joy Chakraborty wrote:
> Set 18bit TxDeemph[17:0] in LCSR_TX_DEEMPH(n) register for USB3 Gen2
> Normal Operation as defined in PIPE4 spec based on dt quirk
> "snps,tx_gen2_de_emphasis_quirk" and "snps,tx_gen2_de_emphasis".
> 
> Signed-off-by: Joy Chakraborty <joychakr@google.com>
> ---
>  drivers/usb/dwc3/core.c | 13 +++++++++++++
>  drivers/usb/dwc3/core.h |  6 ++++++
>  2 files changed, 19 insertions(+)

Any reason why we got 2 copies of all of these in the same series?
Please fix that up for when you send a v2 of them.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-10-16  8:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-07 13:55 [PATCH 0/2] usb: dwc3: Add USB3 Gen2 De-emphasis setting from DT Joy Chakraborty
2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 de-emphasis Joy Chakraborty
2024-10-07 14:56   ` Krzysztof Kozlowski
2024-10-08 10:23     ` Joy Chakraborty
2024-10-08 11:23       ` Krzysztof Kozlowski
2024-10-08 11:59         ` Joy Chakraborty
2024-10-08 12:10           ` Krzysztof Kozlowski
2024-10-08 12:40             ` Joy Chakraborty
2024-10-08 12:59               ` Krzysztof Kozlowski
2024-10-07 13:55 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add binding for USB Gen2 De-emphasis Joy Chakraborty
2024-10-07 13:55 ` [PATCH 2/2] usb: dwc3: Program USB Gen2 de-emphasis defined in PIPE4 spec Joy Chakraborty
2024-10-07 13:55 ` [PATCH 2/2] usb: dwc3: Program USB Gen2 De-emphasis " Joy Chakraborty
2024-10-16  8:16   ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).