2024-02-01 12:04:45

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v3 0/5] arm64: dts: ti: am62: Add USB support for k3-am62p

Hi,

This series first adds device nodes for USB0_PHY_CTRL and USB1_PHY_CTRL
in the wkup_conf node and fixus up the USB nodes to use the newly
added nodes.

Then it adds USB support for AM62P SoC and AM62P5-SK board.

In v3, 2 new patches were added to add the missing PHY2 register
space to the USB controller wrapper node.

Changelog in each patch.

cheers,
-roger

Roger Quadros (5):
dt-bindings: mfd: syscon: Add ti,am62-usb-phy-ctrl compatible
arm64: dts: ti: k3-am62/a: use sub-node for USB_PHY_CTRL registers
arm64: dts: ti: k3-am62p: add the USB sub-system
dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space
arm64: dts: ti: k3-am62*: Add PHY2 region to USB wrapper node

.../devicetree/bindings/mfd/syscon.yaml | 1 +
.../devicetree/bindings/usb/ti,am62-usb.yaml | 7 +-
arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 10 +--
arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 10 +++
arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 10 +--
arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi | 10 +++
arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 48 +++++++++++++
arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 10 +++
arch/arm64/boot/dts/ti/k3-am62p5-sk.dts | 67 +++++++++++++++++++
9 files changed, 163 insertions(+), 10 deletions(-)


base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
--
2.34.1



2024-02-01 12:04:59

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v3 1/5] dt-bindings: mfd: syscon: Add ti,am62-usb-phy-ctrl compatible

Add the compatible for TI AM62 USB PHY Control register. This
register is found in the TI AM62 WKUP_CTRL_MMR0 space [1]. It
is used to indicate the USB PHY PLL reference clock rate and
core voltage level to the USB controller.

[1] - https://www.ti.com/lit/pdf/spruiv7

Signed-off-by: Roger Quadros <[email protected]>
---

Notes:
Changelog:

v3 - add compatibles in alphabetical order
v2 - New patch

Changelog:

v2 - New patch

Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 084b5c2a2a3c..9437705af92f 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -72,6 +72,7 @@ properties:
- rockchip,rk3588-qos
- rockchip,rv1126-qos
- starfive,jh7100-sysmain
+ - ti,am62-usb-phy-ctrl
- ti,am654-dss-oldi-io-ctrl

- const: syscon
--
2.34.1


2024-02-01 12:05:25

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v3 2/5] arm64: dts: ti: k3-am62/a: use sub-node for USB_PHY_CTRL registers

Exposing the entire CTRL_MMR space to syscon is not a good idea.
Add sub-nodes for USB0_PHY_CTRL and USB1_PHY_CTRL and use them
in the USB0/USB1 nodes.

Signed-off-by: Roger Quadros <[email protected]>
---

Notes:
Changelog:

v3 - no change

v2:
- moved am62p changes to next patch
- use new compatible for USB PHY CTRL node

arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 4 ++--
arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 10 ++++++++++
arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 4 ++--
arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi | 10 ++++++++++
4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index 464b7565d085..9432ed344d52 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -625,7 +625,7 @@ usbss0: dwc3-usb@f900000 {
reg = <0x00 0x0f900000 0x00 0x800>;
clocks = <&k3_clks 161 3>;
clock-names = "ref";
- ti,syscon-phy-pll-refclk = <&wkup_conf 0x4008>;
+ ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
#address-cells = <2>;
#size-cells = <2>;
power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>;
@@ -648,7 +648,7 @@ usbss1: dwc3-usb@f910000 {
reg = <0x00 0x0f910000 0x00 0x800>;
clocks = <&k3_clks 162 3>;
clock-names = "ref";
- ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
+ ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
#address-cells = <2>;
#size-cells = <2>;
power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>;
diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
index fef76f52a52e..817700b2eacf 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
@@ -19,6 +19,16 @@ chipid: chipid@14 {
compatible = "ti,am654-chipid";
reg = <0x14 0x4>;
};
+
+ usb0_phy_ctrl: syscon@4008 {
+ compatible = "ti,am62-usb-phy-ctrl", "syscon";
+ reg = <0x4008 0x4>;
+ };
+
+ usb1_phy_ctrl: syscon@4018 {
+ compatible = "ti,am62-usb-phy-ctrl", "syscon";
+ reg = <0x4018 0x4>;
+ };
};

wkup_uart0: serial@2b300000 {
diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index f0b8c9ab1459..8311c7c44cd3 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -566,7 +566,7 @@ usbss0: dwc3-usb@f900000 {
reg = <0x00 0x0f900000 0x00 0x800>;
clocks = <&k3_clks 161 3>;
clock-names = "ref";
- ti,syscon-phy-pll-refclk = <&wkup_conf 0x4008>;
+ ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
#address-cells = <2>;
#size-cells = <2>;
power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>;
@@ -589,7 +589,7 @@ usbss1: dwc3-usb@f910000 {
reg = <0x00 0x0f910000 0x00 0x800>;
clocks = <&k3_clks 162 3>;
clock-names = "ref";
- ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
+ ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
#address-cells = <2>;
#size-cells = <2>;
power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>;
diff --git a/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
index 4e8279fa01e1..4a375f5e0c19 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
@@ -17,6 +17,16 @@ chipid: chipid@14 {
compatible = "ti,am654-chipid";
reg = <0x14 0x4>;
};
+
+ usb0_phy_ctrl: syscon@4008 {
+ compatible = "ti,am62-usb-phy-ctrl", "syscon";
+ reg = <0x4008 0x4>;
+ };
+
+ usb1_phy_ctrl: syscon@4018 {
+ compatible = "ti,am62-usb-phy-ctrl", "syscon";
+ reg = <0x4018 0x4>;
+ };
};

wkup_uart0: serial@2b300000 {
--
2.34.1


2024-02-01 12:06:01

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

So far this was not required but due to the newly identified
Errata i2409 [1] we need to poke this register space.

[1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf

Signed-off-by: Roger Quadros <[email protected]>
---

Notes:
Changelog:

v3 - new patch

Documentation/devicetree/bindings/usb/ti,am62-usb.yaml | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
index fec5651f5602..c02d9d467d9c 100644
--- a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
@@ -14,7 +14,9 @@ properties:
const: ti,am62-usb

reg:
- maxItems: 1
+ items:
+ - description: USB CFG register space
+ - description: USB PHY2 register space

ranges: true

@@ -82,7 +84,8 @@ examples:

usbss1: usb@f910000 {
compatible = "ti,am62-usb";
- reg = <0x00 0x0f910000 0x00 0x800>;
+ reg = <0x00 0x0f910000 0x00 0x800>,
+ <0x00 0x0f918000 0x00 0x400>;
clocks = <&k3_clks 162 3>;
clock-names = "ref";
ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
--
2.34.1


2024-02-01 12:06:04

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v3 3/5] arm64: dts: ti: k3-am62p: add the USB sub-system

There are two USB instances available on the am62p5 starter kit. Include
and enable them for use on the board.

Signed-off-by: Vignesh Raghavendra <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---

Notes:
Changelog:

v3 - no change

v2:
- added USB PHY CTRL node changes here
- changed USB wrapper node names to usb@
- changed Type-C chip node name to usb-power-control@

arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 46 ++++++++++++++
arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 10 +++
arch/arm64/boot/dts/ti/k3-am62p5-sk.dts | 67 +++++++++++++++++++++
3 files changed, 123 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
index 4c51bae06b57..17d28390d587 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
@@ -560,6 +560,52 @@ sdhci2: mmc@fa20000 {
status = "disabled";
};

+ usbss0: usb@f900000 {
+ compatible = "ti,am62-usb";
+ reg = <0x00 0x0f900000 0x00 0x800>;
+ clocks = <&k3_clks 161 3>;
+ clock-names = "ref";
+ ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>;
+ ranges;
+ status = "disabled";
+
+ usb0: usb@31000000 {
+ compatible = "snps,dwc3";
+ reg = <0x00 0x31000000 0x00 0x50000>;
+ interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */
+ <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>; /* irq.0 */
+ interrupt-names = "host", "peripheral";
+ maximum-speed = "high-speed";
+ dr_mode = "otg";
+ };
+ };
+
+ usbss1: usb@f910000 {
+ compatible = "ti,am62-usb";
+ reg = <0x00 0x0f910000 0x00 0x800>;
+ clocks = <&k3_clks 162 3>;
+ clock-names = "ref";
+ ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>;
+ ranges;
+ status = "disabled";
+
+ usb1: usb@31100000 {
+ compatible = "snps,dwc3";
+ reg = <0x00 0x31100000 0x00 0x50000>;
+ interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */
+ <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>; /* irq.0 */
+ interrupt-names = "host", "peripheral";
+ maximum-speed = "high-speed";
+ dr_mode = "otg";
+ };
+ };
+
fss: bus@fc00000 {
compatible = "simple-bus";
reg = <0x00 0x0fc00000 0x00 0x70000>;
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
index 19f42b39394e..00dd38b02a52 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
@@ -18,6 +18,16 @@ chipid: chipid@14 {
reg = <0x14 0x4>;
bootph-all;
};
+
+ usb0_phy_ctrl: syscon@4008 {
+ compatible = "ti,am62-usb-phy-ctrl", "syscon";
+ reg = <0x4008 0x4>;
+ };
+
+ usb1_phy_ctrl: syscon@4018 {
+ compatible = "ti,am62-usb-phy-ctrl", "syscon";
+ reg = <0x4018 0x4>;
+ };
};

wkup_uart0: serial@2b300000 {
diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
index 1773c05f752c..80be56c0a4e0 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
@@ -27,6 +27,8 @@ aliases {
spi0 = &ospi0;
ethernet0 = &cpsw_port1;
ethernet1 = &cpsw_port2;
+ usb0 = &usb0;
+ usb1 = &usb1;
};

chosen {
@@ -297,6 +299,12 @@ AM62PX_IOPAD(0x01b0, PIN_OUTPUT, 2) /* (G20) MCASP0_ACLKR.UART1_TXD */
bootph-all;
};

+ main_usb1_pins_default: main-usb1-default-pins {
+ pinctrl-single,pins = <
+ AM62PX_IOPAD(0x0258, PIN_INPUT, 0) /* (G21) USB1_DRVVBUS */
+ >;
+ };
+
main_wlirq_pins_default: main-wlirq-default-pins {
pinctrl-single,pins = <
AM62PX_IOPAD(0x0128, PIN_INPUT, 7) /* (K25) MMC2_SDWP.GPIO0_72 */
@@ -340,6 +348,36 @@ AM62PX_IOPAD(0x0124, PIN_INPUT, 7) /* (J25) MMC2_SDCD.GPIO0_71 */
};
};

+&main_i2c0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c0_pins_default>;
+ clock-frequency = <400000>;
+
+ typec_pd0: usb-power-controller@3f {
+ compatible = "ti,tps6598x";
+ reg = <0x3f>;
+
+ connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ self-powered;
+ data-role = "dual";
+ power-role = "sink";
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ usb_con_hs: endpoint {
+ remote-endpoint = <&usb0_hs_ep>;
+ };
+ };
+ };
+ };
+ };
+};
+
&main_i2c1 {
status = "okay";
pinctrl-names = "default";
@@ -460,6 +498,35 @@ cpsw3g_phy1: ethernet-phy@1 {
};
};

+&usbss0 {
+ status = "okay";
+ ti,vbus-divider;
+};
+
+&usbss1 {
+ status = "okay";
+ ti,vbus-divider;
+};
+
+&usb0 {
+ usb-role-switch;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ usb0_hs_ep: endpoint {
+ remote-endpoint = <&usb_con_hs>;
+ };
+ };
+};
+
+&usb1 {
+ dr_mode = "host";
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_usb1_pins_default>;
+};
+
&mcasp1 {
status = "okay";
#sound-dai-cells = <0>;
--
2.34.1


2024-02-01 12:06:23

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v3 5/5] arm64: dts: ti: k3-am62*: Add PHY2 region to USB wrapper node

Add PHY2 register space to USB wrapper node. This is required
to deal with Errata i2409.

Signed-off-by: Roger Quadros <[email protected]>
---

Notes:
Changelog:

v3 - new patch

arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 6 ++++--
arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 6 ++++--
arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 6 ++++--
3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index 9432ed344d52..da50fbfcce56 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -622,7 +622,8 @@ sdhci2: mmc@fa20000 {

usbss0: dwc3-usb@f900000 {
compatible = "ti,am62-usb";
- reg = <0x00 0x0f900000 0x00 0x800>;
+ reg = <0x00 0x0f900000 0x00 0x800>,
+ <0x00 0x0f908000 0x00 0x400>;
clocks = <&k3_clks 161 3>;
clock-names = "ref";
ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
@@ -645,7 +646,8 @@ usb0: usb@31000000 {

usbss1: dwc3-usb@f910000 {
compatible = "ti,am62-usb";
- reg = <0x00 0x0f910000 0x00 0x800>;
+ reg = <0x00 0x0f910000 0x00 0x800>,
+ <0x00 0x0f918000 0x00 0x400>;
clocks = <&k3_clks 162 3>;
clock-names = "ref";
ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index 8311c7c44cd3..523bab94eace 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -563,7 +563,8 @@ sdhci1: mmc@fa00000 {

usbss0: dwc3-usb@f900000 {
compatible = "ti,am62-usb";
- reg = <0x00 0x0f900000 0x00 0x800>;
+ reg = <0x00 0x0f900000 0x00 0x800>,
+ <0x00 0x0f908000 0x00 0x400>;
clocks = <&k3_clks 161 3>;
clock-names = "ref";
ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
@@ -586,7 +587,8 @@ usb0: usb@31000000 {

usbss1: dwc3-usb@f910000 {
compatible = "ti,am62-usb";
- reg = <0x00 0x0f910000 0x00 0x800>;
+ reg = <0x00 0x0f910000 0x00 0x800>,
+ <0x00 0x0f918000 0x00 0x400>;
clocks = <&k3_clks 162 3>;
clock-names = "ref";
ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
index 17d28390d587..ae0ab67460b4 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
@@ -562,7 +562,8 @@ sdhci2: mmc@fa20000 {

usbss0: usb@f900000 {
compatible = "ti,am62-usb";
- reg = <0x00 0x0f900000 0x00 0x800>;
+ reg = <0x00 0x0f900000 0x00 0x800>,
+ <0x00 0x0f908000 0x00 0x400>;
clocks = <&k3_clks 161 3>;
clock-names = "ref";
ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
@@ -585,7 +586,8 @@ usb0: usb@31000000 {

usbss1: usb@f910000 {
compatible = "ti,am62-usb";
- reg = <0x00 0x0f910000 0x00 0x800>;
+ reg = <0x00 0x0f910000 0x00 0x800>,
+ <0x00 0x0f918000 0x00 0x400>;
clocks = <&k3_clks 162 3>;
clock-names = "ref";
ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
--
2.34.1


2024-02-01 15:25:00

by Andrew Davis

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] arm64: dts: ti: k3-am62/a: use sub-node for USB_PHY_CTRL registers

On 2/1/24 6:03 AM, Roger Quadros wrote:
> Exposing the entire CTRL_MMR space to syscon is not a good idea.
> Add sub-nodes for USB0_PHY_CTRL and USB1_PHY_CTRL and use them
> in the USB0/USB1 nodes.
>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
>

Reviewed-by: Andrew Davis <[email protected]>

> Notes:
> Changelog:
>
> v3 - no change
>
> v2:
> - moved am62p changes to next patch
> - use new compatible for USB PHY CTRL node
>
> arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 4 ++--
> arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 10 ++++++++++
> arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 4 ++--
> arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi | 10 ++++++++++
> 4 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> index 464b7565d085..9432ed344d52 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> @@ -625,7 +625,7 @@ usbss0: dwc3-usb@f900000 {
> reg = <0x00 0x0f900000 0x00 0x800>;
> clocks = <&k3_clks 161 3>;
> clock-names = "ref";
> - ti,syscon-phy-pll-refclk = <&wkup_conf 0x4008>;
> + ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
> #address-cells = <2>;
> #size-cells = <2>;
> power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>;
> @@ -648,7 +648,7 @@ usbss1: dwc3-usb@f910000 {
> reg = <0x00 0x0f910000 0x00 0x800>;
> clocks = <&k3_clks 162 3>;
> clock-names = "ref";
> - ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
> + ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
> #address-cells = <2>;
> #size-cells = <2>;
> power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>;
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> index fef76f52a52e..817700b2eacf 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> @@ -19,6 +19,16 @@ chipid: chipid@14 {
> compatible = "ti,am654-chipid";
> reg = <0x14 0x4>;
> };
> +
> + usb0_phy_ctrl: syscon@4008 {
> + compatible = "ti,am62-usb-phy-ctrl", "syscon";
> + reg = <0x4008 0x4>;
> + };
> +
> + usb1_phy_ctrl: syscon@4018 {
> + compatible = "ti,am62-usb-phy-ctrl", "syscon";
> + reg = <0x4018 0x4>;
> + };
> };
>
> wkup_uart0: serial@2b300000 {
> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> index f0b8c9ab1459..8311c7c44cd3 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
> @@ -566,7 +566,7 @@ usbss0: dwc3-usb@f900000 {
> reg = <0x00 0x0f900000 0x00 0x800>;
> clocks = <&k3_clks 161 3>;
> clock-names = "ref";
> - ti,syscon-phy-pll-refclk = <&wkup_conf 0x4008>;
> + ti,syscon-phy-pll-refclk = <&usb0_phy_ctrl 0x0>;
> #address-cells = <2>;
> #size-cells = <2>;
> power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>;
> @@ -589,7 +589,7 @@ usbss1: dwc3-usb@f910000 {
> reg = <0x00 0x0f910000 0x00 0x800>;
> clocks = <&k3_clks 162 3>;
> clock-names = "ref";
> - ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
> + ti,syscon-phy-pll-refclk = <&usb1_phy_ctrl 0x0>;
> #address-cells = <2>;
> #size-cells = <2>;
> power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>;
> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
> index 4e8279fa01e1..4a375f5e0c19 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
> @@ -17,6 +17,16 @@ chipid: chipid@14 {
> compatible = "ti,am654-chipid";
> reg = <0x14 0x4>;
> };
> +
> + usb0_phy_ctrl: syscon@4008 {
> + compatible = "ti,am62-usb-phy-ctrl", "syscon";
> + reg = <0x4008 0x4>;
> + };
> +
> + usb1_phy_ctrl: syscon@4018 {
> + compatible = "ti,am62-usb-phy-ctrl", "syscon";
> + reg = <0x4018 0x4>;
> + };
> };
>
> wkup_uart0: serial@2b300000 {

2024-02-01 18:17:00

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
> So far this was not required but due to the newly identified
> Errata i2409 [1] we need to poke this register space.
>
> [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
>
> Signed-off-by: Roger Quadros <[email protected]>

Acked-by: Conor Dooley <[email protected]>

> ---
>
> Notes:
> Changelog:
>
> v3 - new patch
>
> Documentation/devicetree/bindings/usb/ti,am62-usb.yaml | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> index fec5651f5602..c02d9d467d9c 100644
> --- a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> @@ -14,7 +14,9 @@ properties:
> const: ti,am62-usb
>
> reg:
> - maxItems: 1
> + items:
> + - description: USB CFG register space
> + - description: USB PHY2 register space
>
> ranges: true
>
> @@ -82,7 +84,8 @@ examples:
>
> usbss1: usb@f910000 {
> compatible = "ti,am62-usb";
> - reg = <0x00 0x0f910000 0x00 0x800>;
> + reg = <0x00 0x0f910000 0x00 0x800>,
> + <0x00 0x0f918000 0x00 0x400>;

Why the double zeros btw?

Cheers,
Conor.

> clocks = <&k3_clks 162 3>;
> clock-names = "ref";
> ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
> --
> 2.34.1
>


Attachments:
(No filename) (1.51 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-01 18:18:19

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On Thu, Feb 01, 2024 at 06:15:20PM +0000, Conor Dooley wrote:
> On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
> > So far this was not required but due to the newly identified
> > Errata i2409 [1] we need to poke this register space.
> >
> > [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
> >
> > Signed-off-by: Roger Quadros <[email protected]>
>
> Acked-by: Conor Dooley <[email protected]>

Actually, where is the user for this that actually pokes the register
space?
You're adding another register region, so I went to check how you were
handling that in drivers, but there's no driver patch.


>
> > ---
> >
> > Notes:
> > Changelog:
> >
> > v3 - new patch
> >
> > Documentation/devicetree/bindings/usb/ti,am62-usb.yaml | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> > index fec5651f5602..c02d9d467d9c 100644
> > --- a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> > @@ -14,7 +14,9 @@ properties:
> > const: ti,am62-usb
> >
> > reg:
> > - maxItems: 1
> > + items:
> > + - description: USB CFG register space
> > + - description: USB PHY2 register space
> >
> > ranges: true
> >
> > @@ -82,7 +84,8 @@ examples:
> >
> > usbss1: usb@f910000 {
> > compatible = "ti,am62-usb";
> > - reg = <0x00 0x0f910000 0x00 0x800>;
> > + reg = <0x00 0x0f910000 0x00 0x800>,
> > + <0x00 0x0f918000 0x00 0x400>;
>
> Why the double zeros btw?
>
> Cheers,
> Conor.
>
> > clocks = <&k3_clks 162 3>;
> > clock-names = "ref";
> > ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
> > --
> > 2.34.1
> >



Attachments:
(No filename) (1.89 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-01 19:05:04

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On Thu, Feb 01, 2024 at 06:18:05PM +0000, Conor Dooley wrote:
> On Thu, Feb 01, 2024 at 06:15:20PM +0000, Conor Dooley wrote:
> > On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
> > > So far this was not required but due to the newly identified
> > > Errata i2409 [1] we need to poke this register space.
> > >
> > > [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
> > >
> > > Signed-off-by: Roger Quadros <[email protected]>
> >
> > Acked-by: Conor Dooley <[email protected]>
>
> Actually, where is the user for this that actually pokes the register
> space?
> You're adding another register region, so I went to check how you were
> handling that in drivers, but there's no driver patch.

See Roger's another patch set 'Add workaround for Errata i2409' posted
on 16th.

-Bin.

>
>
> >
> > > ---
> > >
> > > Notes:
> > > Changelog:
> > >
> > > v3 - new patch
> > >
> > > Documentation/devicetree/bindings/usb/ti,am62-usb.yaml | 7 +++++--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> > > index fec5651f5602..c02d9d467d9c 100644
> > > --- a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
> > > @@ -14,7 +14,9 @@ properties:
> > > const: ti,am62-usb
> > >
> > > reg:
> > > - maxItems: 1
> > > + items:
> > > + - description: USB CFG register space
> > > + - description: USB PHY2 register space
> > >
> > > ranges: true
> > >
> > > @@ -82,7 +84,8 @@ examples:
> > >
> > > usbss1: usb@f910000 {
> > > compatible = "ti,am62-usb";
> > > - reg = <0x00 0x0f910000 0x00 0x800>;
> > > + reg = <0x00 0x0f910000 0x00 0x800>,
> > > + <0x00 0x0f918000 0x00 0x400>;
> >
> > Why the double zeros btw?
> >
> > Cheers,
> > Conor.
> >
> > > clocks = <&k3_clks 162 3>;
> > > clock-names = "ref";
> > > ti,syscon-phy-pll-refclk = <&wkup_conf 0x4018>;
> > > --
> > > 2.34.1
> > >
>
>



2024-02-01 19:40:59

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On Thu, Feb 01, 2024 at 12:35:22PM -0600, Bin Liu wrote:
> On Thu, Feb 01, 2024 at 06:18:05PM +0000, Conor Dooley wrote:
> > On Thu, Feb 01, 2024 at 06:15:20PM +0000, Conor Dooley wrote:
> > > On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
> > > > So far this was not required but due to the newly identified
> > > > Errata i2409 [1] we need to poke this register space.
> > > >
> > > > [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
> > > >
> > > > Signed-off-by: Roger Quadros <[email protected]>
> > >
> > > Acked-by: Conor Dooley <[email protected]>
> >
> > Actually, where is the user for this that actually pokes the register
> > space?
> > You're adding another register region, so I went to check how you were
> > handling that in drivers, but there's no driver patch.
>
> See Roger's another patch set 'Add workaround for Errata i2409' posted
> on 16th.

This patch should be with that series, not with these dts patches.

Thanks,
Conor.


Attachments:
(No filename) (0.98 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-02 09:43:01

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space



On 01/02/2024 21:13, Conor Dooley wrote:
> On Thu, Feb 01, 2024 at 12:35:22PM -0600, Bin Liu wrote:
>> On Thu, Feb 01, 2024 at 06:18:05PM +0000, Conor Dooley wrote:
>>> On Thu, Feb 01, 2024 at 06:15:20PM +0000, Conor Dooley wrote:
>>>> On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
>>>>> So far this was not required but due to the newly identified
>>>>> Errata i2409 [1] we need to poke this register space.
>>>>>
>>>>> [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
>>>>>
>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>
>>>> Acked-by: Conor Dooley <[email protected]>
>>>
>>> Actually, where is the user for this that actually pokes the register
>>> space?

https://lore.kernel.org/all/[email protected]/

>>> You're adding another register region, so I went to check how you were
>>> handling that in drivers, but there's no driver patch.
>>
>> See Roger's another patch set 'Add workaround for Errata i2409' posted
>> on 16th.
>
> This patch should be with that series, not with these dts patches.
>

Why not? There should be no dependency between DTS and driver implementation.

As DTS and driver will be merged by separate maintainers I thought it
would be easier for maintainers this way.

--
cheers,
-roger

2024-02-02 09:54:52

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On Fri, Feb 02, 2024 at 11:36:55AM +0200, Roger Quadros wrote:
>
>
> On 01/02/2024 21:13, Conor Dooley wrote:
> > On Thu, Feb 01, 2024 at 12:35:22PM -0600, Bin Liu wrote:
> >> On Thu, Feb 01, 2024 at 06:18:05PM +0000, Conor Dooley wrote:
> >>> On Thu, Feb 01, 2024 at 06:15:20PM +0000, Conor Dooley wrote:
> >>>> On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
> >>>>> So far this was not required but due to the newly identified
> >>>>> Errata i2409 [1] we need to poke this register space.
> >>>>>
> >>>>> [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
> >>>>>
> >>>>> Signed-off-by: Roger Quadros <[email protected]>
> >>>>
> >>>> Acked-by: Conor Dooley <[email protected]>
> >>>
> >>> Actually, where is the user for this that actually pokes the register
> >>> space?
>
> https://lore.kernel.org/all/[email protected]/
>
> >>> You're adding another register region, so I went to check how you were
> >>> handling that in drivers, but there's no driver patch.
> >>
> >> See Roger's another patch set 'Add workaround for Errata i2409' posted
> >> on 16th.
> >
> > This patch should be with that series, not with these dts patches.
> >
>
> Why not? There should be no dependency between DTS and driver implementation.
>
> As DTS and driver will be merged by separate maintainers I thought it
> would be easier for maintainers this way.

dts and driver might be merged by different people, but dt-bindings and
drivers are merged by the same people. This is a bindings patch, not a
dts patch. Look at what get_maintainer says for this file:
Greg Kroah-Hartman <[email protected]> (supporter:USB SUBSYSTEM)
Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
Krzysztof Kozlowski <[email protected]> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
Conor Dooley <[email protected]> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
Aswath Govindraju <[email protected]> (in file)
[email protected] (open list:USB SUBSYSTEM)
[email protected] (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
[email protected] (open list)
Greg and linux-usb are on there, but you have not CCed them.

Being with the driver also allows bindings maintainers to check that you
don't break backwards compatibility. It also prevents me having to ask
for the driver patch, then be given just a subject line that I have to
go and look up myself!

Thanks,
Conor.


Attachments:
(No filename) (2.54 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-02 10:38:45

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space



On 02/02/2024 11:53, Conor Dooley wrote:
> On Fri, Feb 02, 2024 at 11:36:55AM +0200, Roger Quadros wrote:
>>
>>
>> On 01/02/2024 21:13, Conor Dooley wrote:
>>> On Thu, Feb 01, 2024 at 12:35:22PM -0600, Bin Liu wrote:
>>>> On Thu, Feb 01, 2024 at 06:18:05PM +0000, Conor Dooley wrote:
>>>>> On Thu, Feb 01, 2024 at 06:15:20PM +0000, Conor Dooley wrote:
>>>>>> On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
>>>>>>> So far this was not required but due to the newly identified
>>>>>>> Errata i2409 [1] we need to poke this register space.
>>>>>>>
>>>>>>> [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
>>>>>>>
>>>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>>>
>>>>>> Acked-by: Conor Dooley <[email protected]>
>>>>>
>>>>> Actually, where is the user for this that actually pokes the register
>>>>> space?
>>
>> https://lore.kernel.org/all/[email protected]/
>>
>>>>> You're adding another register region, so I went to check how you were
>>>>> handling that in drivers, but there's no driver patch.
>>>>
>>>> See Roger's another patch set 'Add workaround for Errata i2409' posted
>>>> on 16th.
>>>
>>> This patch should be with that series, not with these dts patches.
>>>
>>
>> Why not? There should be no dependency between DTS and driver implementation.
>>
>> As DTS and driver will be merged by separate maintainers I thought it
>> would be easier for maintainers this way.
>
> dts and driver might be merged by different people, but dt-bindings and
> drivers are merged by the same people. This is a bindings patch, not a

If we do that then I get a bunch of dtbs_check warnings

dwc3-usb@f900000: reg: [[0, 261095424, 0, 2048], [0, 261128192, 0, 1024]] is too long

> dts patch. Look at what get_maintainer says for this file:
> Greg Kroah-Hartman <[email protected]> (supporter:USB SUBSYSTEM)
> Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> Krzysztof Kozlowski <[email protected]> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> Conor Dooley <[email protected]> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> Aswath Govindraju <[email protected]> (in file)
> [email protected] (open list:USB SUBSYSTEM)
> [email protected] (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> [email protected] (open list)
> Greg and linux-usb are on there, but you have not CCed them.

My bad. Will be more careful next time.

>
> Being with the driver also allows bindings maintainers to check that you
> don't break backwards compatibility. It also prevents me having to ask
> for the driver patch, then be given just a subject line that I have to
> go and look up myself!
>

Sorry about that. It took a bit longer but I did point you directly to the
patch on lore.kernel.org.

--
cheers,
-roger

2024-02-02 12:23:55

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On 12:13-20240202, Roger Quadros wrote:
[..]
> >>
> >> As DTS and driver will be merged by separate maintainers I thought it
> >> would be easier for maintainers this way.
> >
> > dts and driver might be merged by different people, but dt-bindings and
> > drivers are merged by the same people. This is a bindings patch, not a
>
> If we do that then I get a bunch of dtbs_check warnings
>
> dwc3-usb@f900000: reg: [[0, 261095424, 0, 2048], [0, 261128192, 0, 1024]] is too long

Just my 2 cents: If the binding (and driver) change was truly backward
compatible (which it should be - for example: errata can only be
applied if the second property is described), then you want to control
that reg property to add minItems? - thatm I think will allow the dts
change to come in at the next cycle once the binding has been merged.

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2024-02-02 14:20:25

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space



On 02/02/2024 14:18, Nishanth Menon wrote:
> On 12:13-20240202, Roger Quadros wrote:
> [..]
>>>>
>>>> As DTS and driver will be merged by separate maintainers I thought it
>>>> would be easier for maintainers this way.
>>>
>>> dts and driver might be merged by different people, but dt-bindings and
>>> drivers are merged by the same people. This is a bindings patch, not a
>>
>> If we do that then I get a bunch of dtbs_check warnings
>>
>> dwc3-usb@f900000: reg: [[0, 261095424, 0, 2048], [0, 261128192, 0, 1024]] is too long
>
> Just my 2 cents: If the binding (and driver) change was truly backward
> compatible (which it should be - for example: errata can only be
> applied if the second property is described), then you want to control
> that reg property to add minItems? - thatm I think will allow the dts
> change to come in at the next cycle once the binding has been merged.
>

Thanks for the hint.
Please drop patches 4 and 5 in case you pick this series.

I'll send patch 4 along with the driver series v2.
Patch 5, I'll send after the DT binding has been merged.

--
cheers,
-roger

2024-02-02 14:59:33

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On 15:59-20240202, Roger Quadros wrote:
>
>
> On 02/02/2024 14:18, Nishanth Menon wrote:
> > On 12:13-20240202, Roger Quadros wrote:
> > [..]
> >>>>
> >>>> As DTS and driver will be merged by separate maintainers I thought it
> >>>> would be easier for maintainers this way.
> >>>
> >>> dts and driver might be merged by different people, but dt-bindings and
> >>> drivers are merged by the same people. This is a bindings patch, not a
> >>
> >> If we do that then I get a bunch of dtbs_check warnings
> >>
> >> dwc3-usb@f900000: reg: [[0, 261095424, 0, 2048], [0, 261128192, 0, 1024]] is too long
> >
> > Just my 2 cents: If the binding (and driver) change was truly backward
> > compatible (which it should be - for example: errata can only be
> > applied if the second property is described), then you want to control
> > that reg property to add minItems? - thatm I think will allow the dts
> > change to come in at the next cycle once the binding has been merged.
> >
>
> Thanks for the hint.
> Please drop patches 4 and 5 in case you pick this series.
>
> I'll send patch 4 along with the driver series v2.
> Patch 5, I'll send after the DT binding has been merged.

I suggest to resubmit requisite series (with patches +- or what ever)
specific to appropriate maintainers (I don't typically like folks
sending driver change along with dts change in a single series without
indicating to driver maintainer that dts is something they shouldn't
be picking) and avoid any confusion ;)

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2024-02-02 17:09:41

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: usb/ti,am62-usb.yaml: Add PHY2 register space

On Fri, Feb 02, 2024 at 12:13:22PM +0200, Roger Quadros wrote:
>
>
> On 02/02/2024 11:53, Conor Dooley wrote:
> > On Fri, Feb 02, 2024 at 11:36:55AM +0200, Roger Quadros wrote:
> >>
> >>
> >> On 01/02/2024 21:13, Conor Dooley wrote:
> >>> On Thu, Feb 01, 2024 at 12:35:22PM -0600, Bin Liu wrote:
> >>>> On Thu, Feb 01, 2024 at 06:18:05PM +0000, Conor Dooley wrote:
> >>>>> On Thu, Feb 01, 2024 at 06:15:20PM +0000, Conor Dooley wrote:
> >>>>>> On Thu, Feb 01, 2024 at 02:03:31PM +0200, Roger Quadros wrote:
> >>>>>>> So far this was not required but due to the newly identified
> >>>>>>> Errata i2409 [1] we need to poke this register space.
> >>>>>>>
> >>>>>>> [1] https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
> >>>>>>>
> >>>>>>> Signed-off-by: Roger Quadros <[email protected]>
> >>>>>>
> >>>>>> Acked-by: Conor Dooley <[email protected]>
> >>>>>
> >>>>> Actually, where is the user for this that actually pokes the register
> >>>>> space?
> >>
> >> https://lore.kernel.org/all/[email protected]/
> >>
> >>>>> You're adding another register region, so I went to check how you were
> >>>>> handling that in drivers, but there's no driver patch.
> >>>>
> >>>> See Roger's another patch set 'Add workaround for Errata i2409' posted
> >>>> on 16th.
> >>>
> >>> This patch should be with that series, not with these dts patches.
> >>>
> >>
> >> Why not? There should be no dependency between DTS and driver implementation.
> >>
> >> As DTS and driver will be merged by separate maintainers I thought it
> >> would be easier for maintainers this way.
> >
> > dts and driver might be merged by different people, but dt-bindings and
> > drivers are merged by the same people. This is a bindings patch, not a
>
> If we do that then I get a bunch of dtbs_check warnings
>
> dwc3-usb@f900000: reg: [[0, 261095424, 0, 2048], [0, 261128192, 0, 1024]] is too long

I don't know what your platform maintainers view is, but to me it is fine
as long as linux-next is clean.


Attachments:
(No filename) (2.00 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-02 20:26:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] dt-bindings: mfd: syscon: Add ti,am62-usb-phy-ctrl compatible


On Thu, 01 Feb 2024 14:03:28 +0200, Roger Quadros wrote:
> Add the compatible for TI AM62 USB PHY Control register. This
> register is found in the TI AM62 WKUP_CTRL_MMR0 space [1]. It
> is used to indicate the USB PHY PLL reference clock rate and
> core voltage level to the USB controller.
>
> [1] - https://www.ti.com/lit/pdf/spruiv7
>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
>
> Notes:
> Changelog:
>
> v3 - add compatibles in alphabetical order
> v2 - New patch
>
> Changelog:
>
> v2 - New patch
>
> Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
> 1 file changed, 1 insertion(+)
>

Acked-by: Rob Herring <[email protected]>


2024-02-14 11:27:26

by Andrejs Cainikovs

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] usb: dwc3-am62: add workaround for Errata i2409

> All AM62 devices have Errata i2409 [1] due to which
> USB2 PHY may lock up due to short suspend.
>
> Workaround involves setting bit 5 and 4 PLL_REG12
> in PHY2 register space after USB controller is brought
> out of LPSC reset but before controller initialization.
>
> Handle this workaround.

Tested-by: Andrejs Cainikovs <[email protected]>

You have requested [1] to check whether this patch
fixes issues I observed, and I can confirm it does.

[1]: https://lore.kernel.org/all/[email protected]/

2024-04-08 09:53:01

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] arm64: dts: ti: k3-am62*: Add PHY2 region to USB wrapper node

Hello Roger,

On Thu, Feb 01, 2024 at 02:03:32PM +0200, Roger Quadros wrote:
> Add PHY2 register space to USB wrapper node. This is required
> to deal with Errata i2409.
>
> Signed-off-by: Roger Quadros <[email protected]>

What's the status/plan for this? v6.9-rc misses it and therefore we have
this error in the logs (and of course, we miss the workaround).

[ 0.583305] dwc3-am62 f910000.dwc3-usb: invalid resource (null)
[ 0.589304] dwc3-am62 f910000.dwc3-usb: can't map PHY IOMEM resource. Won't apply i2409 fix.

Apart for the error message, the change here seems required for the
hardware to properly function (IOW IMHO it should be back-ported to stable).

Francesco


2024-04-12 11:40:11

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] arm64: dts: ti: k3-am62*: Add PHY2 region to USB wrapper node

Hi Francesco,

On 08/04/2024 12:52, Francesco Dolcini wrote:
> Hello Roger,
>
> On Thu, Feb 01, 2024 at 02:03:32PM +0200, Roger Quadros wrote:
>> Add PHY2 register space to USB wrapper node. This is required
>> to deal with Errata i2409.
>>
>> Signed-off-by: Roger Quadros <[email protected]>
>
> What's the status/plan for this? v6.9-rc misses it and therefore we have
> this error in the logs (and of course, we miss the workaround).

I will send the fix for this for v6.9+.

>
> [ 0.583305] dwc3-am62 f910000.dwc3-usb: invalid resource (null)
> [ 0.589304] dwc3-am62 f910000.dwc3-usb: can't map PHY IOMEM resource. Won't apply i2409 fix.
>
> Apart for the error message, the change here seems required for the
> hardware to properly function (IOW IMHO it should be back-ported to stable).
>
> Francesco
>

--
cheers,
-roger