linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM
@ 2020-09-15 11:20 Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines Roger Quadros
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Roger Quadros @ 2020-09-15 11:20 UTC (permalink / raw)
  To: t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt,
	linux-arm-kernel, Roger Quadros

Hi Tero/Nishanth,

This series adds USB2.0 support for the J7200 EVM.

Series is based on top of:

    Faiz's MMC/SD support series
    https://lore.kernel.org/lkml/20200907090520.25313-1-faiz_abbas@ti.com/
    Lokesh's initial support series
    https://patchwork.kernel.org/cover/11740039/
    Vignesh's I2C support series
    https://lore.kernel.org/patchwork/cover/1282152/
    Vignesh's Hyperflash series
    https://lore.kernel.org/patchwork/cover/1285326/

cheers,
-roger

Changelog:
v3:
- use 0x00 instead of 0x0 in device tree for consistency.
- update commit log for USB support patch.

v2:
- fixed warnings when built with W=2. Still one warning is present
as property name "dr_mode" by USB core contains underscore.

Kishon Vijay Abraham I (1):
  arm64: dts: ti: k3-j7200-common-proc-board: Configure the SERDES lane
    function

Roger Quadros (5):
  dt-bindings: mux-j7200-wiz: Add lane function defines
  arm64: dts: ti: k3-j7200-main: Add SERDES lane control mux
  arm64: dts: ti: k3-j7200-main.dtsi: Add USB to SERDES lane MUX
  arm64: dts: ti: k3-j7200-main: Add USB controller
  arm64: dts: ti: k3-j7200-common-proc-board: Add USB support

 .../dts/ti/k3-j7200-common-proc-board.dts     | 28 ++++++++++
 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi     | 51 +++++++++++++++++++
 include/dt-bindings/mux/mux-j7200-wiz.h       | 29 +++++++++++
 3 files changed, 108 insertions(+)
 create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-15 11:20 [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM Roger Quadros
@ 2020-09-15 11:20 ` Roger Quadros
  2020-09-16  4:52   ` Peter Rosin
  2020-09-15 11:20 ` [PATCH v3 2/6] arm64: dts: ti: k3-j7200-main: Add SERDES lane control mux Roger Quadros
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Roger Quadros @ 2020-09-15 11:20 UTC (permalink / raw)
  To: t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt, Peter Rosin,
	linux-arm-kernel, Roger Quadros

Each SERDES lane mux can select upto 4 different IPs.
There are 4 lanes in each J7200 SERDES. Define all
the possible functions in this file.

Cc: Peter Rosin <peda@axentia.se>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h

diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
new file mode 100644
index 000000000000..b091b1185a36
--- /dev/null
+++ b/include/dt-bindings/mux/mux-j7200-wiz.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides constants for J7200 WIZ.
+ */
+
+#ifndef _DT_BINDINGS_J7200_WIZ
+#define _DT_BINDINGS_J7200_WIZ
+
+#define SERDES0_LANE0_QSGMII_LANE3	0x0
+#define SERDES0_LANE0_PCIE1_LANE0	0x1
+#define SERDES0_LANE0_IP3_UNUSED	0x2
+#define SERDES0_LANE0_IP4_UNUSED	0x3
+
+#define SERDES0_LANE1_QSGMII_LANE4	0x0
+#define SERDES0_LANE1_PCIE1_LANE1	0x1
+#define SERDES0_LANE1_IP3_UNUSED	0x2
+#define SERDES0_LANE1_IP4_UNUSED	0x3
+
+#define SERDES0_LANE2_QSGMII_LANE1	0x0
+#define SERDES0_LANE2_PCIE1_LANE2	0x1
+#define SERDES0_LANE2_IP3_UNUSED	0x2
+#define SERDES0_LANE2_IP4_UNUSED	0x3
+
+#define SERDES0_LANE3_QSGMII_LANE2	0x0
+#define SERDES0_LANE3_PCIE1_LANE3	0x1
+#define SERDES0_LANE3_USB		0x2
+#define SERDES0_LANE3_IP4_UNUSED	0x3
+
+#endif /* _DT_BINDINGS_J7200_WIZ */
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 2/6] arm64: dts: ti: k3-j7200-main: Add SERDES lane control mux
  2020-09-15 11:20 [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines Roger Quadros
@ 2020-09-15 11:20 ` Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 3/6] arm64: dts: ti: k3-j7200-main.dtsi: Add USB to SERDES lane MUX Roger Quadros
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Roger Quadros @ 2020-09-15 11:20 UTC (permalink / raw)
  To: t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt,
	linux-arm-kernel, Roger Quadros

The SERDES lane control mux registers are present in the
CTRLMMR space.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
index 1702ac0bbf40..d6d688efc32a 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
@@ -18,6 +18,21 @@
 		};
 	};
 
+	scm_conf: scm-conf@100000 {
+		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
+		reg = <0x00 0x00100000 0x00 0x1c000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x00 0x00 0x00100000 0x1c000>;
+
+		serdes_ln_ctrl: serdes-ln-ctrl@4080 {
+			compatible = "mmio-mux";
+			#mux-control-cells = <1>;
+			mux-reg-masks = <0x4080 0x3>, <0x4084 0x3>, /* SERDES0 lane0/1 select */
+					<0x4088 0x3>, <0x408c 0x3>; /* SERDES0 lane2/3 select */
+		};
+	};
+
 	gic500: interrupt-controller@1800000 {
 		compatible = "arm,gic-v3";
 		#address-cells = <2>;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 3/6] arm64: dts: ti: k3-j7200-main.dtsi: Add USB to SERDES lane MUX
  2020-09-15 11:20 [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 2/6] arm64: dts: ti: k3-j7200-main: Add SERDES lane control mux Roger Quadros
@ 2020-09-15 11:20 ` Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 4/6] arm64: dts: ti: k3-j7200-main: Add USB controller Roger Quadros
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Roger Quadros @ 2020-09-15 11:20 UTC (permalink / raw)
  To: t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt,
	linux-arm-kernel, Roger Quadros

The USB controller can be connected to one of the 2 lanes
of SERDES0 using a MUX. Add a MUX controller node for that.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
index d6d688efc32a..42f66b8dffa7 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
@@ -31,6 +31,12 @@
 			mux-reg-masks = <0x4080 0x3>, <0x4084 0x3>, /* SERDES0 lane0/1 select */
 					<0x4088 0x3>, <0x408c 0x3>; /* SERDES0 lane2/3 select */
 		};
+
+		usb_serdes_mux: mux-controller@4000 {
+			compatible = "mmio-mux";
+			#mux-control-cells = <1>;
+			mux-reg-masks = <0x4000 0x8000000>; /* USB0 to SERDES0 lane 1/3 mux */
+		};
 	};
 
 	gic500: interrupt-controller@1800000 {
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 4/6] arm64: dts: ti: k3-j7200-main: Add USB controller
  2020-09-15 11:20 [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM Roger Quadros
                   ` (2 preceding siblings ...)
  2020-09-15 11:20 ` [PATCH v3 3/6] arm64: dts: ti: k3-j7200-main.dtsi: Add USB to SERDES lane MUX Roger Quadros
@ 2020-09-15 11:20 ` Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 5/6] arm64: dts: ti: k3-j7200-common-proc-board: Configure the SERDES lane function Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 6/6] arm64: dts: ti: k3-j7200-common-proc-board: Add USB support Roger Quadros
  5 siblings, 0 replies; 19+ messages in thread
From: Roger Quadros @ 2020-09-15 11:20 UTC (permalink / raw)
  To: t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt,
	linux-arm-kernel, Roger Quadros

j7200 has on USB controller instance. Add that.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 30 +++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
index 42f66b8dffa7..519e6f718363 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
@@ -331,4 +331,34 @@
 		no-1-8-v;
 		dma-coherent;
 	};
+
+	usbss0: cdns-usb@4104000 {
+		compatible = "ti,j721e-usb";
+		reg = <0x00 0x4104000 0x00 0x100>;
+		dma-coherent;
+		power-domains = <&k3_pds 288 TI_SCI_PD_EXCLUSIVE>;
+		clocks = <&k3_clks 288 12>, <&k3_clks 288 3>;
+		clock-names = "ref", "lpm";
+		assigned-clocks = <&k3_clks 288 12>;	/* USB2_REFCLK */
+		assigned-clock-parents = <&k3_clks 288 13>; /* HFOSC0 */
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		usb0: 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>,	/* irq.0 */
+				     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,	/* irq.6 */
+				     <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;	/* otgirq.0 */
+			interrupt-names = "host",
+					  "peripheral",
+					  "otg";
+			maximum-speed = "super-speed";
+			dr_mode = "otg";
+		};
+	};
 };
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 5/6] arm64: dts: ti: k3-j7200-common-proc-board: Configure the SERDES lane function
  2020-09-15 11:20 [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM Roger Quadros
                   ` (3 preceding siblings ...)
  2020-09-15 11:20 ` [PATCH v3 4/6] arm64: dts: ti: k3-j7200-main: Add USB controller Roger Quadros
@ 2020-09-15 11:20 ` Roger Quadros
  2020-09-15 11:20 ` [PATCH v3 6/6] arm64: dts: ti: k3-j7200-common-proc-board: Add USB support Roger Quadros
  5 siblings, 0 replies; 19+ messages in thread
From: Roger Quadros @ 2020-09-15 11:20 UTC (permalink / raw)
  To: t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt,
	linux-arm-kernel, Roger Quadros

From: Kishon Vijay Abraham I <kishon@ti.com>

First two lanes of SERDES is connected to PCIe, third lane is
connected to QSGMII and the last lane is connected to USB. However,
Cadence torrent SERDES doesn't support more than 2 protocols
at the same time. Configure it only for PCIe and QSGMII.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
index 8e534ef8a3f5..0ecaba600704 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
@@ -6,6 +6,7 @@
 /dts-v1/;
 
 #include "k3-j7200-som-p0.dtsi"
+#include <dt-bindings/mux/mux-j7200-wiz.h>
 
 / {
 	chosen {
@@ -139,3 +140,8 @@
 	ti,driver-strength-ohm = <50>;
 	disable-wp;
 };
+
+&serdes_ln_ctrl {
+	idle-states = <SERDES0_LANE0_PCIE1_LANE0>, <SERDES0_LANE1_PCIE1_LANE1>,
+		      <SERDES0_LANE2_QSGMII_LANE1>, <SERDES0_LANE3_IP4_UNUSED>;
+};
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 6/6] arm64: dts: ti: k3-j7200-common-proc-board: Add USB support
  2020-09-15 11:20 [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM Roger Quadros
                   ` (4 preceding siblings ...)
  2020-09-15 11:20 ` [PATCH v3 5/6] arm64: dts: ti: k3-j7200-common-proc-board: Configure the SERDES lane function Roger Quadros
@ 2020-09-15 11:20 ` Roger Quadros
  5 siblings, 0 replies; 19+ messages in thread
From: Roger Quadros @ 2020-09-15 11:20 UTC (permalink / raw)
  To: t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt,
	linux-arm-kernel, Roger Quadros

The board uses lane 3 of SERDES for USB. Set the mux
accordingly.

The USB controller and EVM supports super-speed for USB0
on the Type-C port. However, the SERDES has a limitation
that upto 2 protocols can be used at a time. The SERDES is
wired for PCIe, QSGMII and USB super-speed. It has been
chosen to use PCI2 and QSGMII as default. So restrict
USB0 to high-speed mode.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../dts/ti/k3-j7200-common-proc-board.dts     | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
index 0ecaba600704..5ce3fddbd617 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
@@ -42,6 +42,12 @@
 			J721E_IOPAD(0xe4, PIN_INPUT, 8) /* (V1) TIMER_IO0.MMC1_SDCD */
 		>;
 	};
+
+	main_usbss0_pins_default: main-usbss0-pins-default {
+		pinctrl-single,pins = <
+			J721E_IOPAD(0x120, PIN_OUTPUT, 0) /* (T4) USB0_DRVVBUS */
+		>;
+	};
 };
 
 &wkup_uart0 {
@@ -145,3 +151,19 @@
 	idle-states = <SERDES0_LANE0_PCIE1_LANE0>, <SERDES0_LANE1_PCIE1_LANE1>,
 		      <SERDES0_LANE2_QSGMII_LANE1>, <SERDES0_LANE3_IP4_UNUSED>;
 };
+
+&usb_serdes_mux {
+	idle-states = <1>; /* USB0 to SERDES lane 3 */
+};
+
+&usbss0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&main_usbss0_pins_default>;
+	ti,vbus-divider;
+	ti,usb2-only;
+};
+
+&usb0 {
+	dr_mode = "otg";
+	maximum-speed = "high-speed";
+};
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-15 11:20 ` [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines Roger Quadros
@ 2020-09-16  4:52   ` Peter Rosin
  2020-09-16 15:45     ` Nishanth Menon
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2020-09-16  4:52 UTC (permalink / raw)
  To: Roger Quadros, t-kristo, nm
  Cc: devicetree, nsekhar, linux-kernel, kishon, robh+dt, linux-arm-kernel

Hi,

Sorry for the delay.

On 2020-09-15 13:20, Roger Quadros wrote:
> Each SERDES lane mux can select upto 4 different IPs.
> There are 4 lanes in each J7200 SERDES. Define all
> the possible functions in this file.
> 
> Cc: Peter Rosin <peda@axentia.se>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
> 
> diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
> new file mode 100644
> index 000000000000..b091b1185a36
> --- /dev/null
> +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides constants for J7200 WIZ.
> + */
> +
> +#ifndef _DT_BINDINGS_J7200_WIZ
> +#define _DT_BINDINGS_J7200_WIZ
> +
> +#define SERDES0_LANE0_QSGMII_LANE3	0x0
> +#define SERDES0_LANE0_PCIE1_LANE0	0x1
> +#define SERDES0_LANE0_IP3_UNUSED	0x2
> +#define SERDES0_LANE0_IP4_UNUSED	0x3
> +
> +#define SERDES0_LANE1_QSGMII_LANE4	0x0
> +#define SERDES0_LANE1_PCIE1_LANE1	0x1
> +#define SERDES0_LANE1_IP3_UNUSED	0x2
> +#define SERDES0_LANE1_IP4_UNUSED	0x3
> +
> +#define SERDES0_LANE2_QSGMII_LANE1	0x0
> +#define SERDES0_LANE2_PCIE1_LANE2	0x1
> +#define SERDES0_LANE2_IP3_UNUSED	0x2
> +#define SERDES0_LANE2_IP4_UNUSED	0x3
> +
> +#define SERDES0_LANE3_QSGMII_LANE2	0x0
> +#define SERDES0_LANE3_PCIE1_LANE3	0x1
> +#define SERDES0_LANE3_USB		0x2
> +#define SERDES0_LANE3_IP4_UNUSED	0x3
> +
> +#endif /* _DT_BINDINGS_J7200_WIZ */

Should not the defines start with J7200_WIZ? SERDES0 seems like a too
generic prefix, at least to me.

Cheers,
Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-16  4:52   ` Peter Rosin
@ 2020-09-16 15:45     ` Nishanth Menon
  2020-09-17  9:45       ` Peter Rosin
                         ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Nishanth Menon @ 2020-09-16 15:45 UTC (permalink / raw)
  To: Peter Rosin
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel, Roger Quadros

On 06:52-20200916, Peter Rosin wrote:
> Hi,
> 
> Sorry for the delay.
> 
> On 2020-09-15 13:20, Roger Quadros wrote:
> > Each SERDES lane mux can select upto 4 different IPs.
> > There are 4 lanes in each J7200 SERDES. Define all
> > the possible functions in this file.
> > 
> > Cc: Peter Rosin <peda@axentia.se>
> > Signed-off-by: Roger Quadros <rogerq@ti.com>
> > ---
> >  include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
> > 
> > diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
> > new file mode 100644
> > index 000000000000..b091b1185a36
> > --- /dev/null
> > +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
> > @@ -0,0 +1,29 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * This header provides constants for J7200 WIZ.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_J7200_WIZ
> > +#define _DT_BINDINGS_J7200_WIZ
> > +
> > +#define SERDES0_LANE0_QSGMII_LANE3	0x0
> > +#define SERDES0_LANE0_PCIE1_LANE0	0x1
> > +#define SERDES0_LANE0_IP3_UNUSED	0x2
> > +#define SERDES0_LANE0_IP4_UNUSED	0x3
> > +
> > +#define SERDES0_LANE1_QSGMII_LANE4	0x0
> > +#define SERDES0_LANE1_PCIE1_LANE1	0x1
> > +#define SERDES0_LANE1_IP3_UNUSED	0x2
> > +#define SERDES0_LANE1_IP4_UNUSED	0x3
> > +
> > +#define SERDES0_LANE2_QSGMII_LANE1	0x0
> > +#define SERDES0_LANE2_PCIE1_LANE2	0x1
> > +#define SERDES0_LANE2_IP3_UNUSED	0x2
> > +#define SERDES0_LANE2_IP4_UNUSED	0x3
> > +
> > +#define SERDES0_LANE3_QSGMII_LANE2	0x0
> > +#define SERDES0_LANE3_PCIE1_LANE3	0x1
> > +#define SERDES0_LANE3_USB		0x2
> > +#define SERDES0_LANE3_IP4_UNUSED	0x3
> > +
> > +#endif /* _DT_BINDINGS_J7200_WIZ */
> 
> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
> generic prefix, at least to me.

Thanks, good point. I am not sure if WIZ should even be used.. It is
a TI internal prefix for various serdes solutions, but I agree that
SERDES0 is too generic a terminology. That said, we should cleanup
include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
j7200 changes.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-16 15:45     ` Nishanth Menon
@ 2020-09-17  9:45       ` Peter Rosin
  2020-09-17 12:27         ` Nishanth Menon
  2020-09-17 10:17       ` Kishon Vijay Abraham I
  2020-09-17 12:00       ` Roger Quadros
  2 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2020-09-17  9:45 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel, Roger Quadros

Hi!

On 2020-09-16 17:45, Nishanth Menon wrote:
> On 06:52-20200916, Peter Rosin wrote:
>> Hi,
>>
>> Sorry for the delay.
>>
>> On 2020-09-15 13:20, Roger Quadros wrote:
>>> Each SERDES lane mux can select upto 4 different IPs.
>>> There are 4 lanes in each J7200 SERDES. Define all
>>> the possible functions in this file.
>>>
>>> Cc: Peter Rosin <peda@axentia.se>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---

*snip*

>> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
>> generic prefix, at least to me.
> 
> Thanks, good point. I am not sure if WIZ should even be used.. It is
> a TI internal prefix for various serdes solutions, but I agree that
> SERDES0 is too generic a terminology. That said, we should cleanup
> include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> j7200 changes.

Right. As maintainer for the directory in question, I should have
been on Cc for that series too, but it appears I wasn't. Hence, I
didn't notice that file until now when I went looking for it. Why
wasn't I on Cc?

Cheers,
Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-16 15:45     ` Nishanth Menon
  2020-09-17  9:45       ` Peter Rosin
@ 2020-09-17 10:17       ` Kishon Vijay Abraham I
  2020-09-17 12:36         ` Nishanth Menon
  2020-09-17 12:00       ` Roger Quadros
  2 siblings, 1 reply; 19+ messages in thread
From: Kishon Vijay Abraham I @ 2020-09-17 10:17 UTC (permalink / raw)
  To: Nishanth Menon, Peter Rosin
  Cc: devicetree, nsekhar, linux-kernel, t-kristo, robh+dt,
	linux-arm-kernel, Roger Quadros

Nishanth,

On 16/09/20 9:15 pm, Nishanth Menon wrote:
> On 06:52-20200916, Peter Rosin wrote:
>> Hi,
>>
>> Sorry for the delay.
>>
>> On 2020-09-15 13:20, Roger Quadros wrote:
>>> Each SERDES lane mux can select upto 4 different IPs.
>>> There are 4 lanes in each J7200 SERDES. Define all
>>> the possible functions in this file.
>>>
>>> Cc: Peter Rosin <peda@axentia.se>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>  include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
>>>  1 file changed, 29 insertions(+)
>>>  create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
>>>
>>> diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
>>> new file mode 100644
>>> index 000000000000..b091b1185a36
>>> --- /dev/null
>>> +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
>>> @@ -0,0 +1,29 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * This header provides constants for J7200 WIZ.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_J7200_WIZ
>>> +#define _DT_BINDINGS_J7200_WIZ
>>> +
>>> +#define SERDES0_LANE0_QSGMII_LANE3	0x0
>>> +#define SERDES0_LANE0_PCIE1_LANE0	0x1
>>> +#define SERDES0_LANE0_IP3_UNUSED	0x2
>>> +#define SERDES0_LANE0_IP4_UNUSED	0x3
>>> +
>>> +#define SERDES0_LANE1_QSGMII_LANE4	0x0
>>> +#define SERDES0_LANE1_PCIE1_LANE1	0x1
>>> +#define SERDES0_LANE1_IP3_UNUSED	0x2
>>> +#define SERDES0_LANE1_IP4_UNUSED	0x3
>>> +
>>> +#define SERDES0_LANE2_QSGMII_LANE1	0x0
>>> +#define SERDES0_LANE2_PCIE1_LANE2	0x1
>>> +#define SERDES0_LANE2_IP3_UNUSED	0x2
>>> +#define SERDES0_LANE2_IP4_UNUSED	0x3
>>> +
>>> +#define SERDES0_LANE3_QSGMII_LANE2	0x0
>>> +#define SERDES0_LANE3_PCIE1_LANE3	0x1
>>> +#define SERDES0_LANE3_USB		0x2
>>> +#define SERDES0_LANE3_IP4_UNUSED	0x3
>>> +
>>> +#endif /* _DT_BINDINGS_J7200_WIZ */
>>
>> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
>> generic prefix, at least to me.
> 
> Thanks, good point. I am not sure if WIZ should even be used.. It is
> a TI internal prefix for various serdes solutions, but I agree that
> SERDES0 is too generic a terminology. That said, we should cleanup
> include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> j7200 changes.

WIZ is defined in public TRM (https://www.ti.com/lit/pdf/spruiu1).
"
The SERDES mux (WIZ) module supports the following features:
*) Multiplexes device interfaces onto a single SERDES lane (Tx and Rx)
*) Provides registers to implement SERDES control and status functions
and alignment delays
.
.
"

Thanks
Kishon

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-16 15:45     ` Nishanth Menon
  2020-09-17  9:45       ` Peter Rosin
  2020-09-17 10:17       ` Kishon Vijay Abraham I
@ 2020-09-17 12:00       ` Roger Quadros
  2020-09-17 12:14         ` Nishanth Menon
  2020-09-17 12:37         ` Peter Rosin
  2 siblings, 2 replies; 19+ messages in thread
From: Roger Quadros @ 2020-09-17 12:00 UTC (permalink / raw)
  To: Nishanth Menon, Peter Rosin
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel

Hi Peter & Nishanth,

On 16/09/2020 18:45, Nishanth Menon wrote:
> On 06:52-20200916, Peter Rosin wrote:
>> Hi,
>>
>> Sorry for the delay.
>>
>> On 2020-09-15 13:20, Roger Quadros wrote:
>>> Each SERDES lane mux can select upto 4 different IPs.
>>> There are 4 lanes in each J7200 SERDES. Define all
>>> the possible functions in this file.
>>>
>>> Cc: Peter Rosin <peda@axentia.se>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>   include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
>>>   1 file changed, 29 insertions(+)
>>>   create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
>>>
>>> diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
>>> new file mode 100644
>>> index 000000000000..b091b1185a36
>>> --- /dev/null
>>> +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
>>> @@ -0,0 +1,29 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * This header provides constants for J7200 WIZ.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_J7200_WIZ
>>> +#define _DT_BINDINGS_J7200_WIZ
>>> +
>>> +#define SERDES0_LANE0_QSGMII_LANE3	0x0
>>> +#define SERDES0_LANE0_PCIE1_LANE0	0x1
>>> +#define SERDES0_LANE0_IP3_UNUSED	0x2
>>> +#define SERDES0_LANE0_IP4_UNUSED	0x3
>>> +
>>> +#define SERDES0_LANE1_QSGMII_LANE4	0x0
>>> +#define SERDES0_LANE1_PCIE1_LANE1	0x1
>>> +#define SERDES0_LANE1_IP3_UNUSED	0x2
>>> +#define SERDES0_LANE1_IP4_UNUSED	0x3
>>> +
>>> +#define SERDES0_LANE2_QSGMII_LANE1	0x0
>>> +#define SERDES0_LANE2_PCIE1_LANE2	0x1
>>> +#define SERDES0_LANE2_IP3_UNUSED	0x2
>>> +#define SERDES0_LANE2_IP4_UNUSED	0x3
>>> +
>>> +#define SERDES0_LANE3_QSGMII_LANE2	0x0
>>> +#define SERDES0_LANE3_PCIE1_LANE3	0x1
>>> +#define SERDES0_LANE3_USB		0x2
>>> +#define SERDES0_LANE3_IP4_UNUSED	0x3
>>> +
>>> +#endif /* _DT_BINDINGS_J7200_WIZ */
>>
>> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
>> generic prefix, at least to me.
> 
> Thanks, good point. I am not sure if WIZ should even be used.. It is
> a TI internal prefix for various serdes solutions, but I agree that
> SERDES0 is too generic a terminology. That said, we should cleanup
> include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> j7200 changes.
> 

I'm planning to put all TI SERDES definitions in one header file "ti-serdes-mux.h"
and add SOC specific prefixes to the macros.

This will mean some churn in the existing DT files. (only 2 so far)

Are you guys OK if I do the change in one patch to avoid a broken build in between.
You guys can then decide whose tree it goes through.

The new SoC addition will be separate of course.

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-17 12:00       ` Roger Quadros
@ 2020-09-17 12:14         ` Nishanth Menon
  2020-09-17 12:37         ` Peter Rosin
  1 sibling, 0 replies; 19+ messages in thread
From: Nishanth Menon @ 2020-09-17 12:14 UTC (permalink / raw)
  To: Roger Quadros
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	Peter Rosin, linux-arm-kernel

On 15:00-20200917, Roger Quadros wrote:
> Hi Peter & Nishanth,
> 
> On 16/09/2020 18:45, Nishanth Menon wrote:
> > On 06:52-20200916, Peter Rosin wrote:
> > > Hi,
> > > 
> > > Sorry for the delay.
> > > 
> > > On 2020-09-15 13:20, Roger Quadros wrote:
> > > > Each SERDES lane mux can select upto 4 different IPs.
> > > > There are 4 lanes in each J7200 SERDES. Define all
> > > > the possible functions in this file.
> > > > 
> > > > Cc: Peter Rosin <peda@axentia.se>
> > > > Signed-off-by: Roger Quadros <rogerq@ti.com>
> > > > ---
> > > >   include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
> > > >   1 file changed, 29 insertions(+)
> > > >   create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
> > > > 
> > > > diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
> > > > new file mode 100644
> > > > index 000000000000..b091b1185a36
> > > > --- /dev/null
> > > > +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
> > > > @@ -0,0 +1,29 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * This header provides constants for J7200 WIZ.
> > > > + */
> > > > +
> > > > +#ifndef _DT_BINDINGS_J7200_WIZ
> > > > +#define _DT_BINDINGS_J7200_WIZ
> > > > +
> > > > +#define SERDES0_LANE0_QSGMII_LANE3	0x0
> > > > +#define SERDES0_LANE0_PCIE1_LANE0	0x1
> > > > +#define SERDES0_LANE0_IP3_UNUSED	0x2
> > > > +#define SERDES0_LANE0_IP4_UNUSED	0x3
> > > > +
> > > > +#define SERDES0_LANE1_QSGMII_LANE4	0x0
> > > > +#define SERDES0_LANE1_PCIE1_LANE1	0x1
> > > > +#define SERDES0_LANE1_IP3_UNUSED	0x2
> > > > +#define SERDES0_LANE1_IP4_UNUSED	0x3
> > > > +
> > > > +#define SERDES0_LANE2_QSGMII_LANE1	0x0
> > > > +#define SERDES0_LANE2_PCIE1_LANE2	0x1
> > > > +#define SERDES0_LANE2_IP3_UNUSED	0x2
> > > > +#define SERDES0_LANE2_IP4_UNUSED	0x3
> > > > +
> > > > +#define SERDES0_LANE3_QSGMII_LANE2	0x0
> > > > +#define SERDES0_LANE3_PCIE1_LANE3	0x1
> > > > +#define SERDES0_LANE3_USB		0x2
> > > > +#define SERDES0_LANE3_IP4_UNUSED	0x3
> > > > +
> > > > +#endif /* _DT_BINDINGS_J7200_WIZ */
> > > 
> > > Should not the defines start with J7200_WIZ? SERDES0 seems like a too
> > > generic prefix, at least to me.
> > 
> > Thanks, good point. I am not sure if WIZ should even be used.. It is
> > a TI internal prefix for various serdes solutions, but I agree that
> > SERDES0 is too generic a terminology. That said, we should cleanup
> > include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> > j7200 changes.
> > 
> 
> I'm planning to put all TI SERDES definitions in one header file "ti-serdes-mux.h"
> and add SOC specific prefixes to the macros.
> 
> This will mean some churn in the existing DT files. (only 2 so far)

Please check bindings and examples if any reference as well. Those
changes will need to be considered as well.

> 
> Are you guys OK if I do the change in one patch to avoid a broken build in between.
> You guys can then decide whose tree it goes through.
> 
> The new SoC addition will be separate of course.

If Peter acks and is OK with the changes, then based on Peter's opinion,
I'd rather take the changes via SoC tree for 5.10+ for maintaining
bisectability.

I prefer we name it ti-serdes-mux or something that Peter is OK with as
well. reasons:

i) "wiz" is yet another TLA deal even if documented in public TI TRM in some
   remote chapter, other non-TI folks are going to go scratching their
   heads..
ii) There is no way this can scale with one header per SoC!

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-17  9:45       ` Peter Rosin
@ 2020-09-17 12:27         ` Nishanth Menon
  2020-09-17 12:53           ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Nishanth Menon @ 2020-09-17 12:27 UTC (permalink / raw)
  To: Peter Rosin
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel, Roger Quadros

On 11:45-20200917, Peter Rosin wrote:
[...]
> 
> >> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
> >> generic prefix, at least to me.
> > 
> > Thanks, good point. I am not sure if WIZ should even be used.. It is
> > a TI internal prefix for various serdes solutions, but I agree that
> > SERDES0 is too generic a terminology. That said, we should cleanup
> > include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> > j7200 changes.
> 
> Right. As maintainer for the directory in question, I should have
> been on Cc for that series too, but it appears I wasn't. Hence, I

yes, you should have been. The following commit introduced it.

commit b766e3b0d5f6 ("arm64: dts: ti: k3-j721e-main: Add system controller
node and SERDES lane mux")

> didn't notice that file until now when I went looking for it. Why
> wasn't I on Cc?

Got through the SoC tree - an oversight on our part[1] and should'nt have,
Apologies on the bad call.

I would like to propose the following:
a) The header should be renamed to be something more human friendly.
b) The header should be renamed to be something TI specific and NOT per
TI SoC.
c) The macros need renaming to be less generic as it stands right now.


If you ack the changes, I am guessing that the changes will impact dts
a lot and would rather take the cleanups through SoC tree to maintain
bisectability? OR I can pick on an immutable tag from you with just the
header file change and pick on the dts - but I doubt that would be
bisectable. Just worried that I have picked a bunch of cleanups already
on the dts for 5.10, and would like to avoid a merge conflict.


your thoughts?

[1] https://lore.kernel.org/linux-devicetree/20200709231933.GA1083562@bogus/

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-17 10:17       ` Kishon Vijay Abraham I
@ 2020-09-17 12:36         ` Nishanth Menon
  0 siblings, 0 replies; 19+ messages in thread
From: Nishanth Menon @ 2020-09-17 12:36 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: devicetree, nsekhar, linux-kernel, t-kristo, robh+dt,
	Peter Rosin, linux-arm-kernel, Roger Quadros

On 15:47-20200917, Kishon Vijay Abraham I wrote:

[..]
> > Thanks, good point. I am not sure if WIZ should even be used.. It is
> > a TI internal prefix for various serdes solutions, but I agree that
> > SERDES0 is too generic a terminology. That said, we should cleanup
> > include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> > j7200 changes.
> 
> WIZ is defined in public TRM (https://www.ti.com/lit/pdf/spruiu1).
> "

Maybe give the TRM team a feedback to use a little more human readable
naming? if I just ctrl+f "wiz" in the 10,000 page trm the first match
is: "CA bits can be swizzled between any bit positions via the
DDRSS_PHY_1053[23-0] PHY_ADR_ADDR_SEL_0 field" (it is not even in the
table of contents?)

However, if I search for serdes, and then go down to page 7773, "WIZ:
The WIZ acts as a wrapper for the SerDes, and can both send control
signals to and report status signals from the SerDes, and muxes SerDes
to peripherals"

just call it ti-k3-serdes-mux (since there are other TI serdes muxes..)?

You dont want to be on the brunt of something like [1] caused by, your's
truely..


Also this is never going to scale with the number of devices we are
spinning out.. one header per SoC? makes no sense to me.

[1]
https://groups.google.com/forum/#!msg/linux.kernel/fcntv48yoOc/35bAdI1eaiUJ
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-17 12:00       ` Roger Quadros
  2020-09-17 12:14         ` Nishanth Menon
@ 2020-09-17 12:37         ` Peter Rosin
  2020-09-17 12:51           ` Nishanth Menon
  1 sibling, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2020-09-17 12:37 UTC (permalink / raw)
  To: Roger Quadros, Nishanth Menon
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel

Hi!

On 2020-09-17 14:00, Roger Quadros wrote:
> Hi Peter & Nishanth,
> 
> On 16/09/2020 18:45, Nishanth Menon wrote:
>> On 06:52-20200916, Peter Rosin wrote:
>>> Hi,
>>>
>>> Sorry for the delay.
>>>
>>> On 2020-09-15 13:20, Roger Quadros wrote:
>>>> Each SERDES lane mux can select upto 4 different IPs.
>>>> There are 4 lanes in each J7200 SERDES. Define all
>>>> the possible functions in this file.
>>>>
>>>> Cc: Peter Rosin <peda@axentia.se>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>>   include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
>>>>   1 file changed, 29 insertions(+)
>>>>   create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
>>>>
>>>> diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
>>>> new file mode 100644
>>>> index 000000000000..b091b1185a36
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
>>>> @@ -0,0 +1,29 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +/*
>>>> + * This header provides constants for J7200 WIZ.
>>>> + */
>>>> +
>>>> +#ifndef _DT_BINDINGS_J7200_WIZ
>>>> +#define _DT_BINDINGS_J7200_WIZ
>>>> +
>>>> +#define SERDES0_LANE0_QSGMII_LANE3	0x0
>>>> +#define SERDES0_LANE0_PCIE1_LANE0	0x1
>>>> +#define SERDES0_LANE0_IP3_UNUSED	0x2
>>>> +#define SERDES0_LANE0_IP4_UNUSED	0x3
>>>> +
>>>> +#define SERDES0_LANE1_QSGMII_LANE4	0x0
>>>> +#define SERDES0_LANE1_PCIE1_LANE1	0x1
>>>> +#define SERDES0_LANE1_IP3_UNUSED	0x2
>>>> +#define SERDES0_LANE1_IP4_UNUSED	0x3
>>>> +
>>>> +#define SERDES0_LANE2_QSGMII_LANE1	0x0
>>>> +#define SERDES0_LANE2_PCIE1_LANE2	0x1
>>>> +#define SERDES0_LANE2_IP3_UNUSED	0x2
>>>> +#define SERDES0_LANE2_IP4_UNUSED	0x3
>>>> +
>>>> +#define SERDES0_LANE3_QSGMII_LANE2	0x0
>>>> +#define SERDES0_LANE3_PCIE1_LANE3	0x1
>>>> +#define SERDES0_LANE3_USB		0x2
>>>> +#define SERDES0_LANE3_IP4_UNUSED	0x3
>>>> +
>>>> +#endif /* _DT_BINDINGS_J7200_WIZ */
>>>
>>> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
>>> generic prefix, at least to me.
>>
>> Thanks, good point. I am not sure if WIZ should even be used.. It is
>> a TI internal prefix for various serdes solutions, but I agree that
>> SERDES0 is too generic a terminology. That said, we should cleanup
>> include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
>> j7200 changes.
>>
> 
> I'm planning to put all TI SERDES definitions in one header file "ti-serdes-mux.h"
> and add SOC specific prefixes to the macros.
> 
> This will mean some churn in the existing DT files. (only 2 so far)
> 
> Are you guys OK if I do the change in one patch to avoid a broken build in between.
> You guys can then decide whose tree it goes through.
> 
> The new SoC addition will be separate of course.

We should get these changes done before 5.9 is released.

Not breaking the build for each intermediate step is always a priority.
Also, renaming mux-j721e-wiz.h to ti-serdes-mux.h and renaming the macros
could be seen as orthogonal, and it is certainly possible to do that
as two patches without breaking the build in between. It would just need
changes on both sides of the interface in both patches. But I wouldn't
worry about separating this into two patches, just do a rename patch and
be done with it. Then follow up with additions for j7200.

However, now that we are renaming things anyway, do we really need "mux"
in the name of the file itself?
I personally find .../dt-dbindings/mux/ti-serdes.h descriptive enough.

Cheers,
Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-17 12:37         ` Peter Rosin
@ 2020-09-17 12:51           ` Nishanth Menon
  0 siblings, 0 replies; 19+ messages in thread
From: Nishanth Menon @ 2020-09-17 12:51 UTC (permalink / raw)
  To: Peter Rosin
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel, Roger Quadros

On 14:37-20200917, Peter Rosin wrote:
[...]

> >>> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
> >>> generic prefix, at least to me.
> >>
> >> Thanks, good point. I am not sure if WIZ should even be used.. It is
> >> a TI internal prefix for various serdes solutions, but I agree that
> >> SERDES0 is too generic a terminology. That said, we should cleanup
> >> include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> >> j7200 changes.
> >>
> > 
> > I'm planning to put all TI SERDES definitions in one header file "ti-serdes-mux.h"
> > and add SOC specific prefixes to the macros.
> > 
> > This will mean some churn in the existing DT files. (only 2 so far)
> > 
> > Are you guys OK if I do the change in one patch to avoid a broken build in between.
> > You guys can then decide whose tree it goes through.
> > 
> > The new SoC addition will be separate of course.
> 
> We should get these changes done before 5.9 is released.

OK.

> Not breaking the build for each intermediate step is always a priority.
> Also, renaming mux-j721e-wiz.h to ti-serdes-mux.h and renaming the macros
> could be seen as orthogonal, and it is certainly possible to do that
> as two patches without breaking the build in between. It would just need
> changes on both sides of the interface in both patches. But I wouldn't
> worry about separating this into two patches, just do a rename patch and
> be done with it. Then follow up with additions for j7200.
> 
> However, now that we are renaming things anyway, do we really need "mux"
> in the name of the file itself?
> I personally find .../dt-dbindings/mux/ti-serdes.h descriptive enough.

yep, OK with me.


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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-17 12:27         ` Nishanth Menon
@ 2020-09-17 12:53           ` Peter Rosin
  2020-09-17 13:00             ` Nishanth Menon
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2020-09-17 12:53 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel, Roger Quadros



On 2020-09-17 14:27, Nishanth Menon wrote:
> On 11:45-20200917, Peter Rosin wrote:
> [...]
>>
>>>> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
>>>> generic prefix, at least to me.
>>>
>>> Thanks, good point. I am not sure if WIZ should even be used.. It is
>>> a TI internal prefix for various serdes solutions, but I agree that
>>> SERDES0 is too generic a terminology. That said, we should cleanup
>>> include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
>>> j7200 changes.
>>
>> Right. As maintainer for the directory in question, I should have
>> been on Cc for that series too, but it appears I wasn't. Hence, I
> 
> yes, you should have been. The following commit introduced it.
> 
> commit b766e3b0d5f6 ("arm64: dts: ti: k3-j721e-main: Add system controller
> node and SERDES lane mux")
> 
>> didn't notice that file until now when I went looking for it. Why
>> wasn't I on Cc?
> 
> Got through the SoC tree - an oversight on our part[1] and should'nt have,
> Apologies on the bad call.
> 
> I would like to propose the following:
> a) The header should be renamed to be something more human friendly.
> b) The header should be renamed to be something TI specific and NOT per
> TI SoC.
> c) The macros need renaming to be less generic as it stands right now.
> 
> 
> If you ack the changes, I am guessing that the changes will impact dts
> a lot and would rather take the cleanups through SoC tree to maintain
> bisectability? OR I can pick on an immutable tag from you with just the
> header file change and pick on the dts - but I doubt that would be
> bisectable. Just worried that I have picked a bunch of cleanups already
> on the dts for 5.10, and would like to avoid a merge conflict.

[Our mails crossed.]

I do not have a tree and dt-patches should normally not go *through* me.
But I'd still like to see what's happening.

I did not realize this was going to cause a *lot* of churn in the dt
files. How bad can it be when the file is new in this cycle? And is it
worth it? But it seems you all see problems with the current naming and
in that case it must surely be better to fix it early?

And if it's a lot, maybe it needs to be more than one patch? I get the
feeling this will need to be taken care of by someone other than me,
because I'm just the maintainer of a very small subsystem and I don't
normally have to deal with "big" issues involving several trees. I would
be a bottleneck.

Cheers,
Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines
  2020-09-17 12:53           ` Peter Rosin
@ 2020-09-17 13:00             ` Nishanth Menon
  0 siblings, 0 replies; 19+ messages in thread
From: Nishanth Menon @ 2020-09-17 13:00 UTC (permalink / raw)
  To: Peter Rosin
  Cc: devicetree, nsekhar, linux-kernel, kishon, t-kristo, robh+dt,
	linux-arm-kernel, Roger Quadros

On 14:53-20200917, Peter Rosin wrote:
[..]
> I do not have a tree and dt-patches should normally not go *through* me.
yep. 
> But I'd still like to see what's happening.

> I did not realize this was going to cause a *lot* of churn in the dt
> files. How bad can it be when the file is new in this cycle? And is it
> worth it? But it seems you all see problems with the current naming and
> in that case it must surely be better to fix it early?

yes - I think I can figure it out.
> 
> And if it's a lot, maybe it needs to be more than one patch? I get the
> feeling this will need to be taken care of by someone other than me,
> because I'm just the maintainer of a very small subsystem and I don't
> normally have to deal with "big" issues involving several trees. I would
> be a bottleneck.

No a pain, I think I can help get it through, may be, I worry too much..
Lets see how the cleanup series looks like.

Roger: I am not touching the j7200 series till we cleanup j721e as
discussed in this thread. please use 5.9-rc1 as your baseline for
cleanup. Once available, we can figure out how to get 5.10-rc1 staged
items later. Window is narrowing rather soon, so appreciate a quick
cleaup :).

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2020-09-17 13:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15 11:20 [PATCH v3 0/6] arm64: dts: ti: Add USB support for J7200 EVM Roger Quadros
2020-09-15 11:20 ` [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines Roger Quadros
2020-09-16  4:52   ` Peter Rosin
2020-09-16 15:45     ` Nishanth Menon
2020-09-17  9:45       ` Peter Rosin
2020-09-17 12:27         ` Nishanth Menon
2020-09-17 12:53           ` Peter Rosin
2020-09-17 13:00             ` Nishanth Menon
2020-09-17 10:17       ` Kishon Vijay Abraham I
2020-09-17 12:36         ` Nishanth Menon
2020-09-17 12:00       ` Roger Quadros
2020-09-17 12:14         ` Nishanth Menon
2020-09-17 12:37         ` Peter Rosin
2020-09-17 12:51           ` Nishanth Menon
2020-09-15 11:20 ` [PATCH v3 2/6] arm64: dts: ti: k3-j7200-main: Add SERDES lane control mux Roger Quadros
2020-09-15 11:20 ` [PATCH v3 3/6] arm64: dts: ti: k3-j7200-main.dtsi: Add USB to SERDES lane MUX Roger Quadros
2020-09-15 11:20 ` [PATCH v3 4/6] arm64: dts: ti: k3-j7200-main: Add USB controller Roger Quadros
2020-09-15 11:20 ` [PATCH v3 5/6] arm64: dts: ti: k3-j7200-common-proc-board: Configure the SERDES lane function Roger Quadros
2020-09-15 11:20 ` [PATCH v3 6/6] arm64: dts: ti: k3-j7200-common-proc-board: Add USB support Roger Quadros

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).