* [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-06 9:40 Pankaj Bansal
2019-02-11 3:00 ` Shawn Guo
0 siblings, 1 reply; 7+ messages in thread
From: Pankaj Bansal @ 2019-02-06 9:40 UTC (permalink / raw)
To: Shawn Guo, Leo Li, Andrew Lunn, Florian Fainelli
Cc: netdev, linux-arm-kernel, Pankaj Bansal
The two external MDIO buses used to communicate with phy devices that are
external to SOC are muxed in LX2160AQDS board.
These buses can be routed to any one of the eight IO slots on LX2160AQDS
board depending on value in fpga register 0x54.
Additionally the external MDIO1 is used to communicate to the onboard
RGMII phy devices.
The mdio1 is controlled by bits 4-7 of fpga register and mdio2 is
controlled by bits 0-3 of fpga register.
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---
Notes:
V3:
- Add status = disabled in soc file and status = okay in board file
for external MDIO nodes
- Add interrupts property in external mdio nodes in soc file
V2:
- removed unnecassary TODO statements
- removed device_type from mdio nodes
- change the case of hex number to lowercase
- removed board specific comments from soc file
.../boot/dts/freescale/fsl-lx2160a-qds.dts | 123 +++++++++++++++++
.../boot/dts/freescale/fsl-lx2160a.dtsi | 22 +++
2 files changed, 145 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
index 99a22abbe725..079264b391a2 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
@@ -35,6 +35,14 @@
status = "okay";
};
+&emdio1 {
+ status = "okay";
+};
+
+&emdio2 {
+ status = "okay";
+};
+
&esdhc0 {
status = "okay";
};
@@ -46,6 +54,121 @@
&i2c0 {
status = "okay";
+ fpga@66 {
+ compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
+ reg = <0x66>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mdio-mux-1@54 {
+ mdio-parent-bus = <&emdio1>;
+ reg = <0x54>; /* BRDCFG4 */
+ mux-mask = <0xf8>; /* EMI1_MDIO */
+ #address-cells=<1>;
+ #size-cells = <0>;
+
+ mdio@0 {
+ reg = <0x00>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@40 {
+ reg = <0x40>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@c0 {
+ reg = <0xc0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@c8 {
+ reg = <0xc8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@d0 {
+ reg = <0xd0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@d8 {
+ reg = <0xd8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@e0 {
+ reg = <0xe0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@e8 {
+ reg = <0xe8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@f0 {
+ reg = <0xf0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@f8 {
+ reg = <0xf8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ mdio-mux-2@54 {
+ mdio-parent-bus = <&emdio2>;
+ reg = <0x54>; /* BRDCFG4 */
+ mux-mask = <0x07>; /* EMI2_MDIO */
+ #address-cells=<1>;
+ #size-cells = <0>;
+
+ mdio@0 {
+ reg = <0x00>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@1 {
+ reg = <0x01>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@2 {
+ reg = <0x02>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@3 {
+ reg = <0x03>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@4 {
+ reg = <0x04>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@5 {
+ reg = <0x05>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@6 {
+ reg = <0x06>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio@7 {
+ reg = <0x07>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+ };
+
i2c-mux@77 {
compatible = "nxp,pca9547";
reg = <0x77>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
index a79f5c1ea56d..7def5252ac1a 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -762,5 +762,27 @@
<GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
dma-coherent;
};
+
+ /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
+ emdio1: mdio@8b96000 {
+ compatible = "fsl,fman-memac-mdio";
+ reg = <0x0 0x8b96000 0x0 0x1000>;
+ interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ little-endian; /* force the driver in LE mode */
+ status = "disabled";
+ };
+
+ /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
+ emdio2: mdio@8b97000 {
+ compatible = "fsl,fman-memac-mdio";
+ reg = <0x0 0x8b97000 0x0 0x1000>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ little-endian; /* force the driver in LE mode */
+ status = "disabled";
+ };
};
};
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-06 9:40 [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes Pankaj Bansal
@ 2019-02-11 3:00 ` Shawn Guo
2019-02-11 20:43 ` Leo Li
0 siblings, 1 reply; 7+ messages in thread
From: Shawn Guo @ 2019-02-11 3:00 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Leo Li, Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
On Wed, Feb 06, 2019 at 09:40:33AM +0000, Pankaj Bansal wrote:
> The two external MDIO buses used to communicate with phy devices that are
> external to SOC are muxed in LX2160AQDS board.
>
> These buses can be routed to any one of the eight IO slots on LX2160AQDS
> board depending on value in fpga register 0x54.
>
> Additionally the external MDIO1 is used to communicate to the onboard
> RGMII phy devices.
>
> The mdio1 is controlled by bits 4-7 of fpga register and mdio2 is
> controlled by bits 0-3 of fpga register.
>
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>
> Notes:
> V3:
> - Add status = disabled in soc file and status = okay in board file
> for external MDIO nodes
> - Add interrupts property in external mdio nodes in soc file
> V2:
> - removed unnecassary TODO statements
> - removed device_type from mdio nodes
> - change the case of hex number to lowercase
> - removed board specific comments from soc file
>
> .../boot/dts/freescale/fsl-lx2160a-qds.dts | 123 +++++++++++++++++
> .../boot/dts/freescale/fsl-lx2160a.dtsi | 22 +++
> 2 files changed, 145 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> index 99a22abbe725..079264b391a2 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> @@ -35,6 +35,14 @@
> status = "okay";
> };
>
> +&emdio1 {
> + status = "okay";
> +};
> +
> +&emdio2 {
> + status = "okay";
> +};
> +
> &esdhc0 {
> status = "okay";
> };
> @@ -46,6 +54,121 @@
> &i2c0 {
> status = "okay";
>
> + fpga@66 {
> + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> + reg = <0x66>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mdio-mux-1@54 {
> + mdio-parent-bus = <&emdio1>;
> + reg = <0x54>; /* BRDCFG4 */
> + mux-mask = <0xf8>; /* EMI1_MDIO */
> + #address-cells=<1>;
> + #size-cells = <0>;
> +
> + mdio@0 {
> + reg = <0x00>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
Please have a newline between nodes. It doesn't deserve a respin
though. I can fix them up when applying if Leo is fine with this
version.
Shawn
> + mdio@40 {
> + reg = <0x40>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@c0 {
> + reg = <0xc0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@c8 {
> + reg = <0xc8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@d0 {
> + reg = <0xd0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@d8 {
> + reg = <0xd8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@e0 {
> + reg = <0xe0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@e8 {
> + reg = <0xe8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@f0 {
> + reg = <0xf0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@f8 {
> + reg = <0xf8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> + mdio-mux-2@54 {
> + mdio-parent-bus = <&emdio2>;
> + reg = <0x54>; /* BRDCFG4 */
> + mux-mask = <0x07>; /* EMI2_MDIO */
> + #address-cells=<1>;
> + #size-cells = <0>;
> +
> + mdio@0 {
> + reg = <0x00>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@1 {
> + reg = <0x01>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@2 {
> + reg = <0x02>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@3 {
> + reg = <0x03>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@4 {
> + reg = <0x04>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@5 {
> + reg = <0x05>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@6 {
> + reg = <0x06>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio@7 {
> + reg = <0x07>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> + };
> +
> i2c-mux@77 {
> compatible = "nxp,pca9547";
> reg = <0x77>;
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> index a79f5c1ea56d..7def5252ac1a 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -762,5 +762,27 @@
> <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> dma-coherent;
> };
> +
> + /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
> + emdio1: mdio@8b96000 {
> + compatible = "fsl,fman-memac-mdio";
> + reg = <0x0 0x8b96000 0x0 0x1000>;
> + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + little-endian; /* force the driver in LE mode */
> + status = "disabled";
> + };
> +
> + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> + emdio2: mdio@8b97000 {
> + compatible = "fsl,fman-memac-mdio";
> + reg = <0x0 0x8b97000 0x0 0x1000>;
> + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + little-endian; /* force the driver in LE mode */
> + status = "disabled";
> + };
> };
> };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-11 3:00 ` Shawn Guo
@ 2019-02-11 20:43 ` Leo Li
2019-02-12 3:26 ` Pankaj Bansal
0 siblings, 1 reply; 7+ messages in thread
From: Leo Li @ 2019-02-11 20:43 UTC (permalink / raw)
To: Shawn Guo, Pankaj Bansal
Cc: Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: Sunday, February 10, 2019 9:00 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: Leo Li <leoyang.li@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Florian
> Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> On Wed, Feb 06, 2019 at 09:40:33AM +0000, Pankaj Bansal wrote:
> > The two external MDIO buses used to communicate with phy devices that
> > are external to SOC are muxed in LX2160AQDS board.
> >
> > These buses can be routed to any one of the eight IO slots on
> > LX2160AQDS board depending on value in fpga register 0x54.
> >
> > Additionally the external MDIO1 is used to communicate to the onboard
> > RGMII phy devices.
> >
> > The mdio1 is controlled by bits 4-7 of fpga register and mdio2 is
> > controlled by bits 0-3 of fpga register.
> >
> > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > ---
> >
> > Notes:
> > V3:
> > - Add status = disabled in soc file and status = okay in board file
> > for external MDIO nodes
> > - Add interrupts property in external mdio nodes in soc file
> > V2:
> > - removed unnecassary TODO statements
> > - removed device_type from mdio nodes
> > - change the case of hex number to lowercase
> > - removed board specific comments from soc file
> >
> > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 123 +++++++++++++++++
> > .../boot/dts/freescale/fsl-lx2160a.dtsi | 22 +++
> > 2 files changed, 145 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > index 99a22abbe725..079264b391a2 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > @@ -35,6 +35,14 @@
> > status = "okay";
> > };
> >
> > +&emdio1 {
> > + status = "okay";
> > +};
> > +
> > +&emdio2 {
> > + status = "okay";
> > +};
> > +
> > &esdhc0 {
> > status = "okay";
> > };
> > @@ -46,6 +54,121 @@
> > &i2c0 {
> > status = "okay";
> >
> > + fpga@66 {
> > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > + reg = <0x66>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + mdio-mux-1@54 {
> > + mdio-parent-bus = <&emdio1>;
> > + reg = <0x54>; /* BRDCFG4 */
> > + mux-mask = <0xf8>; /* EMI1_MDIO */
> > + #address-cells=<1>;
> > + #size-cells = <0>;
> > +
> > + mdio@0 {
> > + reg = <0x00>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
>
> Please have a newline between nodes. It doesn't deserve a respin though. I
> can fix them up when applying if Leo is fine with this version.
I think there should be a compatible string defined for the binding of parent node mdio-mux, probably "mdio-mux-regmap", and be used here in the device tree.
>
> Shawn
>
> > + mdio@40 {
> > + reg = <0x40>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@c0 {
> > + reg = <0xc0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@c8 {
> > + reg = <0xc8>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@d0 {
> > + reg = <0xd0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@d8 {
> > + reg = <0xd8>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@e0 {
> > + reg = <0xe0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@e8 {
> > + reg = <0xe8>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@f0 {
> > + reg = <0xf0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@f8 {
> > + reg = <0xf8>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + };
> > +
> > + mdio-mux-2@54 {
> > + mdio-parent-bus = <&emdio2>;
> > + reg = <0x54>; /* BRDCFG4 */
> > + mux-mask = <0x07>; /* EMI2_MDIO */
> > + #address-cells=<1>;
> > + #size-cells = <0>;
> > +
> > + mdio@0 {
> > + reg = <0x00>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@1 {
> > + reg = <0x01>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@2 {
> > + reg = <0x02>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@3 {
> > + reg = <0x03>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@4 {
> > + reg = <0x04>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@5 {
> > + reg = <0x05>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@6 {
> > + reg = <0x06>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + mdio@7 {
> > + reg = <0x07>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > + };
> > + };
> > +
> > i2c-mux@77 {
> > compatible = "nxp,pca9547";
> > reg = <0x77>;
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > index a79f5c1ea56d..7def5252ac1a 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > @@ -762,5 +762,27 @@
> > <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> > dma-coherent;
> > };
> > +
> > + /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
> > + emdio1: mdio@8b96000 {
> > + compatible = "fsl,fman-memac-mdio";
> > + reg = <0x0 0x8b96000 0x0 0x1000>;
> > + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + little-endian; /* force the driver in LE mode */
> > + status = "disabled";
> > + };
> > +
> > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > + emdio2: mdio@8b97000 {
> > + compatible = "fsl,fman-memac-mdio";
> > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + little-endian; /* force the driver in LE mode */
> > + status = "disabled";
> > + };
> > };
> > };
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-11 20:43 ` Leo Li
@ 2019-02-12 3:26 ` Pankaj Bansal
2019-02-12 18:01 ` Li Yang
0 siblings, 1 reply; 7+ messages in thread
From: Pankaj Bansal @ 2019-02-12 3:26 UTC (permalink / raw)
To: Leo Li, Shawn Guo; +Cc: Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
> -----Original Message-----
> From: Leo Li
> Sent: Tuesday, 12 February, 2019 02:14 AM
> To: Shawn Guo <shawnguo@kernel.org>; Pankaj Bansal
> <pankaj.bansal@nxp.com>
> Cc: Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
>
>
>
> > -----Original Message-----
> > From: Shawn Guo <shawnguo@kernel.org>
> > Sent: Sunday, February 10, 2019 9:00 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Leo Li <leoyang.li@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Florian
> > Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org
> > Subject: Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> > On Wed, Feb 06, 2019 at 09:40:33AM +0000, Pankaj Bansal wrote:
> > > The two external MDIO buses used to communicate with phy devices
> > > that are external to SOC are muxed in LX2160AQDS board.
> > >
> > > These buses can be routed to any one of the eight IO slots on
> > > LX2160AQDS board depending on value in fpga register 0x54.
> > >
> > > Additionally the external MDIO1 is used to communicate to the
> > > onboard RGMII phy devices.
> > >
> > > The mdio1 is controlled by bits 4-7 of fpga register and mdio2 is
> > > controlled by bits 0-3 of fpga register.
> > >
> > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > ---
> > >
> > > Notes:
> > > V3:
> > > - Add status = disabled in soc file and status = okay in board file
> > > for external MDIO nodes
> > > - Add interrupts property in external mdio nodes in soc file
> > > V2:
> > > - removed unnecassary TODO statements
> > > - removed device_type from mdio nodes
> > > - change the case of hex number to lowercase
> > > - removed board specific comments from soc file
> > >
> > > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 123 +++++++++++++++++
> > > .../boot/dts/freescale/fsl-lx2160a.dtsi | 22 +++
> > > 2 files changed, 145 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > index 99a22abbe725..079264b391a2 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > @@ -35,6 +35,14 @@
> > > status = "okay";
> > > };
> > >
> > > +&emdio1 {
> > > + status = "okay";
> > > +};
> > > +
> > > +&emdio2 {
> > > + status = "okay";
> > > +};
> > > +
> > > &esdhc0 {
> > > status = "okay";
> > > };
> > > @@ -46,6 +54,121 @@
> > > &i2c0 {
> > > status = "okay";
> > >
> > > + fpga@66 {
> > > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > > + reg = <0x66>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + mdio-mux-1@54 {
> > > + mdio-parent-bus = <&emdio1>;
> > > + reg = <0x54>; /* BRDCFG4 */
> > > + mux-mask = <0xf8>; /* EMI1_MDIO */
> > > + #address-cells=<1>;
> > > + #size-cells = <0>;
> > > +
> > > + mdio@0 {
> > > + reg = <0x00>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> >
> > Please have a newline between nodes. It doesn't deserve a respin
> > though. I can fix them up when applying if Leo is fine with this version.
>
> I think there should be a compatible string defined for the binding of parent
> node mdio-mux, probably "mdio-mux-regmap", and be used here in the device
> tree.
I have two concerns :
1. The regmap is linux s/w construct, while device tree is h/w representation and is s/w agnostic. can we use regmap in device tree?
2. By convention the device tree compatible binding is defined as "<manufacturer>,<model>" e.g. "fsl,mpc8349-uart". The mdio-mux node and it's sub nodes are a generic representation of mdio mux and it is not dependent on a particular manufacturer device. How to define the compatible in this case?
>
> >
> > Shawn
> >
> > > + mdio@40 {
> > > + reg = <0x40>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@c0 {
> > > + reg = <0xc0>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@c8 {
> > > + reg = <0xc8>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@d0 {
> > > + reg = <0xd0>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@d8 {
> > > + reg = <0xd8>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@e0 {
> > > + reg = <0xe0>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@e8 {
> > > + reg = <0xe8>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@f0 {
> > > + reg = <0xf0>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@f8 {
> > > + reg = <0xf8>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + };
> > > +
> > > + mdio-mux-2@54 {
> > > + mdio-parent-bus = <&emdio2>;
> > > + reg = <0x54>; /* BRDCFG4 */
> > > + mux-mask = <0x07>; /* EMI2_MDIO */
> > > + #address-cells=<1>;
> > > + #size-cells = <0>;
> > > +
> > > + mdio@0 {
> > > + reg = <0x00>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@1 {
> > > + reg = <0x01>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@2 {
> > > + reg = <0x02>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@3 {
> > > + reg = <0x03>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@4 {
> > > + reg = <0x04>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@5 {
> > > + reg = <0x05>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@6 {
> > > + reg = <0x06>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + mdio@7 {
> > > + reg = <0x07>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + };
> > > + };
> > > + };
> > > +
> > > i2c-mux@77 {
> > > compatible = "nxp,pca9547";
> > > reg = <0x77>;
> > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > index a79f5c1ea56d..7def5252ac1a 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > @@ -762,5 +762,27 @@
> > > <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> > > dma-coherent;
> > > };
> > > +
> > > + /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
> > > + emdio1: mdio@8b96000 {
> > > + compatible = "fsl,fman-memac-mdio";
> > > + reg = <0x0 0x8b96000 0x0 0x1000>;
> > > + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + little-endian; /* force the driver in LE mode */
> > > + status = "disabled";
> > > + };
> > > +
> > > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > > + emdio2: mdio@8b97000 {
> > > + compatible = "fsl,fman-memac-mdio";
> > > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > > + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + little-endian; /* force the driver in LE mode */
> > > + status = "disabled";
> > > + };
> > > };
> > > };
> > > --
> > > 2.17.1
> > >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-12 3:26 ` Pankaj Bansal
@ 2019-02-12 18:01 ` Li Yang
2019-02-14 22:32 ` Li Yang
0 siblings, 1 reply; 7+ messages in thread
From: Li Yang @ 2019-02-12 18:01 UTC (permalink / raw)
To: Pankaj Bansal, Rob Herring
Cc: Shawn Guo, Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
On Mon, Feb 11, 2019 at 9:28 PM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Leo Li
> > Sent: Tuesday, 12 February, 2019 02:14 AM
> > To: Shawn Guo <shawnguo@kernel.org>; Pankaj Bansal
> > <pankaj.bansal@nxp.com>
> > Cc: Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>;
> > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > Subject: RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> >
> >
> > > -----Original Message-----
> > > From: Shawn Guo <shawnguo@kernel.org>
> > > Sent: Sunday, February 10, 2019 9:00 PM
> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > Cc: Leo Li <leoyang.li@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Florian
> > > Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; linux-arm-
> > > kernel@lists.infradead.org
> > > Subject: Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> > >
> > > On Wed, Feb 06, 2019 at 09:40:33AM +0000, Pankaj Bansal wrote:
> > > > The two external MDIO buses used to communicate with phy devices
> > > > that are external to SOC are muxed in LX2160AQDS board.
> > > >
> > > > These buses can be routed to any one of the eight IO slots on
> > > > LX2160AQDS board depending on value in fpga register 0x54.
> > > >
> > > > Additionally the external MDIO1 is used to communicate to the
> > > > onboard RGMII phy devices.
> > > >
> > > > The mdio1 is controlled by bits 4-7 of fpga register and mdio2 is
> > > > controlled by bits 0-3 of fpga register.
> > > >
> > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > ---
> > > >
> > > > Notes:
> > > > V3:
> > > > - Add status = disabled in soc file and status = okay in board file
> > > > for external MDIO nodes
> > > > - Add interrupts property in external mdio nodes in soc file
> > > > V2:
> > > > - removed unnecassary TODO statements
> > > > - removed device_type from mdio nodes
> > > > - change the case of hex number to lowercase
> > > > - removed board specific comments from soc file
> > > >
> > > > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 123 +++++++++++++++++
> > > > .../boot/dts/freescale/fsl-lx2160a.dtsi | 22 +++
> > > > 2 files changed, 145 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > index 99a22abbe725..079264b391a2 100644
> > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > @@ -35,6 +35,14 @@
> > > > status = "okay";
> > > > };
> > > >
> > > > +&emdio1 {
> > > > + status = "okay";
> > > > +};
> > > > +
> > > > +&emdio2 {
> > > > + status = "okay";
> > > > +};
> > > > +
> > > > &esdhc0 {
> > > > status = "okay";
> > > > };
> > > > @@ -46,6 +54,121 @@
> > > > &i2c0 {
> > > > status = "okay";
> > > >
> > > > + fpga@66 {
> > > > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > > > + reg = <0x66>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + mdio-mux-1@54 {
> > > > + mdio-parent-bus = <&emdio1>;
> > > > + reg = <0x54>; /* BRDCFG4 */
> > > > + mux-mask = <0xf8>; /* EMI1_MDIO */
> > > > + #address-cells=<1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + mdio@0 {
> > > > + reg = <0x00>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > >
> > > Please have a newline between nodes. It doesn't deserve a respin
> > > though. I can fix them up when applying if Leo is fine with this version.
> >
> > I think there should be a compatible string defined for the binding of parent
> > node mdio-mux, probably "mdio-mux-regmap", and be used here in the device
> > tree.
>
> I have two concerns :
> 1. The regmap is linux s/w construct, while device tree is h/w representation and is s/w agnostic. can we use regmap in device tree?
Well, if we want to avoid using the regmap name, we probably can try
"mdio-mux-reg" or "mdio-mux-syscon"? With further search I also found
a more generic mux binding at Documentation/devicetree/bindings/mux/,
would be even better if we can use that to describe the mux.
> 2. By convention the device tree compatible binding is defined as "<manufacturer>,<model>" e.g. "fsl,mpc8349-uart". The mdio-mux node and it's sub nodes are a generic representation of mdio mux and it is not dependent on a particular manufacturer device. How to define the compatible in this case?
The manufacturer prefix is for vendor specific bindings. If the
binding a suppose to be generic, we don't need the vendor prefix.
>
> >
> > >
> > > Shawn
> > >
> > > > + mdio@40 {
> > > > + reg = <0x40>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@c0 {
> > > > + reg = <0xc0>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@c8 {
> > > > + reg = <0xc8>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@d0 {
> > > > + reg = <0xd0>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@d8 {
> > > > + reg = <0xd8>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@e0 {
> > > > + reg = <0xe0>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@e8 {
> > > > + reg = <0xe8>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@f0 {
> > > > + reg = <0xf0>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@f8 {
> > > > + reg = <0xf8>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + };
> > > > +
> > > > + mdio-mux-2@54 {
> > > > + mdio-parent-bus = <&emdio2>;
> > > > + reg = <0x54>; /* BRDCFG4 */
> > > > + mux-mask = <0x07>; /* EMI2_MDIO */
> > > > + #address-cells=<1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + mdio@0 {
> > > > + reg = <0x00>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@1 {
> > > > + reg = <0x01>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@2 {
> > > > + reg = <0x02>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@3 {
> > > > + reg = <0x03>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@4 {
> > > > + reg = <0x04>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@5 {
> > > > + reg = <0x05>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@6 {
> > > > + reg = <0x06>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + mdio@7 {
> > > > + reg = <0x07>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > + };
> > > > + };
> > > > +
> > > > i2c-mux@77 {
> > > > compatible = "nxp,pca9547";
> > > > reg = <0x77>;
> > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > index a79f5c1ea56d..7def5252ac1a 100644
> > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > @@ -762,5 +762,27 @@
> > > > <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> > > > dma-coherent;
> > > > };
> > > > +
> > > > + /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
> > > > + emdio1: mdio@8b96000 {
> > > > + compatible = "fsl,fman-memac-mdio";
> > > > + reg = <0x0 0x8b96000 0x0 0x1000>;
> > > > + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + little-endian; /* force the driver in LE mode */
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > > > + emdio2: mdio@8b97000 {
> > > > + compatible = "fsl,fman-memac-mdio";
> > > > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > > > + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + little-endian; /* force the driver in LE mode */
> > > > + status = "disabled";
> > > > + };
> > > > };
> > > > };
> > > > --
> > > > 2.17.1
> > > >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-12 18:01 ` Li Yang
@ 2019-02-14 22:32 ` Li Yang
2019-02-18 5:49 ` Pankaj Bansal
0 siblings, 1 reply; 7+ messages in thread
From: Li Yang @ 2019-02-14 22:32 UTC (permalink / raw)
To: Pankaj Bansal, Rob Herring
Cc: Shawn Guo, Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
On Tue, Feb 12, 2019 at 12:01 PM Li Yang <leoyang.li@nxp.com> wrote:
>
> On Mon, Feb 11, 2019 at 9:28 PM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Leo Li
> > > Sent: Tuesday, 12 February, 2019 02:14 AM
> > > To: Shawn Guo <shawnguo@kernel.org>; Pankaj Bansal
> > > <pankaj.bansal@nxp.com>
> > > Cc: Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>;
> > > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > > Subject: RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Shawn Guo <shawnguo@kernel.org>
> > > > Sent: Sunday, February 10, 2019 9:00 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Cc: Leo Li <leoyang.li@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Florian
> > > > Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; linux-arm-
> > > > kernel@lists.infradead.org
> > > > Subject: Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> > > >
> > > > On Wed, Feb 06, 2019 at 09:40:33AM +0000, Pankaj Bansal wrote:
> > > > > The two external MDIO buses used to communicate with phy devices
> > > > > that are external to SOC are muxed in LX2160AQDS board.
> > > > >
> > > > > These buses can be routed to any one of the eight IO slots on
> > > > > LX2160AQDS board depending on value in fpga register 0x54.
> > > > >
> > > > > Additionally the external MDIO1 is used to communicate to the
> > > > > onboard RGMII phy devices.
> > > > >
> > > > > The mdio1 is controlled by bits 4-7 of fpga register and mdio2 is
> > > > > controlled by bits 0-3 of fpga register.
> > > > >
> > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > ---
> > > > >
> > > > > Notes:
> > > > > V3:
> > > > > - Add status = disabled in soc file and status = okay in board file
> > > > > for external MDIO nodes
> > > > > - Add interrupts property in external mdio nodes in soc file
> > > > > V2:
> > > > > - removed unnecassary TODO statements
> > > > > - removed device_type from mdio nodes
> > > > > - change the case of hex number to lowercase
> > > > > - removed board specific comments from soc file
> > > > >
> > > > > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 123 +++++++++++++++++
> > > > > .../boot/dts/freescale/fsl-lx2160a.dtsi | 22 +++
> > > > > 2 files changed, 145 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > index 99a22abbe725..079264b391a2 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > @@ -35,6 +35,14 @@
> > > > > status = "okay";
> > > > > };
> > > > >
> > > > > +&emdio1 {
> > > > > + status = "okay";
> > > > > +};
> > > > > +
> > > > > +&emdio2 {
> > > > > + status = "okay";
> > > > > +};
> > > > > +
> > > > > &esdhc0 {
> > > > > status = "okay";
> > > > > };
> > > > > @@ -46,6 +54,121 @@
> > > > > &i2c0 {
> > > > > status = "okay";
> > > > >
> > > > > + fpga@66 {
> > > > > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > > > > + reg = <0x66>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + mdio-mux-1@54 {
> > > > > + mdio-parent-bus = <&emdio1>;
> > > > > + reg = <0x54>; /* BRDCFG4 */
> > > > > + mux-mask = <0xf8>; /* EMI1_MDIO */
> > > > > + #address-cells=<1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + mdio@0 {
> > > > > + reg = <0x00>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > >
> > > > Please have a newline between nodes. It doesn't deserve a respin
> > > > though. I can fix them up when applying if Leo is fine with this version.
> > >
> > > I think there should be a compatible string defined for the binding of parent
> > > node mdio-mux, probably "mdio-mux-regmap", and be used here in the device
> > > tree.
> >
> > I have two concerns :
> > 1. The regmap is linux s/w construct, while device tree is h/w representation and is s/w agnostic. can we use regmap in device tree?
>
> Well, if we want to avoid using the regmap name, we probably can try
> "mdio-mux-reg" or "mdio-mux-syscon"? With further search I also found
> a more generic mux binding at Documentation/devicetree/bindings/mux/,
> would be even better if we can use that to describe the mux.
To make it more clear, with the use of
Documentation/devicetree/bindings/mux/ binding, I think it would end
up with something like this:
i2c {
fpga {
compatible = "fsl,lx2160aqds-fpga", "syscon";
....
mux: mux-controller {
compatible = "mmio-mux"
...
}
}
...
}
mdio-mux {
compatible = "mdio-mux"
mux-controls = <&mux 0>;
....
mdio {
phy {
}
...
}
mdio {
...
}
...
}
}"
>
> > 2. By convention the device tree compatible binding is defined as "<manufacturer>,<model>" e.g. "fsl,mpc8349-uart". The mdio-mux node and it's sub nodes are a generic representation of mdio mux and it is not dependent on a particular manufacturer device. How to define the compatible in this case?
>
> The manufacturer prefix is for vendor specific bindings. If the
> binding a suppose to be generic, we don't need the vendor prefix.
>
> >
> > >
> > > >
> > > > Shawn
> > > >
> > > > > + mdio@40 {
> > > > > + reg = <0x40>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@c0 {
> > > > > + reg = <0xc0>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@c8 {
> > > > > + reg = <0xc8>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@d0 {
> > > > > + reg = <0xd0>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@d8 {
> > > > > + reg = <0xd8>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@e0 {
> > > > > + reg = <0xe0>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@e8 {
> > > > > + reg = <0xe8>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@f0 {
> > > > > + reg = <0xf0>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@f8 {
> > > > > + reg = <0xf8>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + };
> > > > > +
> > > > > + mdio-mux-2@54 {
> > > > > + mdio-parent-bus = <&emdio2>;
> > > > > + reg = <0x54>; /* BRDCFG4 */
> > > > > + mux-mask = <0x07>; /* EMI2_MDIO */
> > > > > + #address-cells=<1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + mdio@0 {
> > > > > + reg = <0x00>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@1 {
> > > > > + reg = <0x01>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@2 {
> > > > > + reg = <0x02>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@3 {
> > > > > + reg = <0x03>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@4 {
> > > > > + reg = <0x04>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@5 {
> > > > > + reg = <0x05>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@6 {
> > > > > + reg = <0x06>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + mdio@7 {
> > > > > + reg = <0x07>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > + };
> > > > > + };
> > > > > +
> > > > > i2c-mux@77 {
> > > > > compatible = "nxp,pca9547";
> > > > > reg = <0x77>;
> > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > index a79f5c1ea56d..7def5252ac1a 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > @@ -762,5 +762,27 @@
> > > > > <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> > > > > dma-coherent;
> > > > > };
> > > > > +
> > > > > + /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
> > > > > + emdio1: mdio@8b96000 {
> > > > > + compatible = "fsl,fman-memac-mdio";
> > > > > + reg = <0x0 0x8b96000 0x0 0x1000>;
> > > > > + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + little-endian; /* force the driver in LE mode */
> > > > > + status = "disabled";
> > > > > + };
> > > > > +
> > > > > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > > > > + emdio2: mdio@8b97000 {
> > > > > + compatible = "fsl,fman-memac-mdio";
> > > > > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > > > > + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + little-endian; /* force the driver in LE mode */
> > > > > + status = "disabled";
> > > > > + };
> > > > > };
> > > > > };
> > > > > --
> > > > > 2.17.1
> > > > >
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-14 22:32 ` Li Yang
@ 2019-02-18 5:49 ` Pankaj Bansal
0 siblings, 0 replies; 7+ messages in thread
From: Pankaj Bansal @ 2019-02-18 5:49 UTC (permalink / raw)
To: Leo Li, Rob Herring
Cc: Shawn Guo, Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@nxp.com]
> Sent: Friday, 15 February, 2019 04:03 AM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>; Rob Herring <robh+dt@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>; Andrew Lunn <andrew@lunn.ch>;
> Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> On Tue, Feb 12, 2019 at 12:01 PM Li Yang <leoyang.li@nxp.com> wrote:
> >
> > On Mon, Feb 11, 2019 at 9:28 PM Pankaj Bansal <pankaj.bansal@nxp.com>
> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Leo Li
> > > > Sent: Tuesday, 12 February, 2019 02:14 AM
> > > > To: Shawn Guo <shawnguo@kernel.org>; Pankaj Bansal
> > > > <pankaj.bansal@nxp.com>
> > > > Cc: Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
> > > > <f.fainelli@gmail.com>; netdev@vger.kernel.org;
> > > > linux-arm-kernel@lists.infradead.org
> > > > Subject: RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Shawn Guo <shawnguo@kernel.org>
> > > > > Sent: Sunday, February 10, 2019 9:00 PM
> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > Cc: Leo Li <leoyang.li@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> > > > > Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org;
> > > > > linux-arm- kernel@lists.infradead.org
> > > > > Subject: Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux
> > > > > nodes
> > > > >
> > > > > On Wed, Feb 06, 2019 at 09:40:33AM +0000, Pankaj Bansal wrote:
> > > > > > The two external MDIO buses used to communicate with phy
> > > > > > devices that are external to SOC are muxed in LX2160AQDS board.
> > > > > >
> > > > > > These buses can be routed to any one of the eight IO slots on
> > > > > > LX2160AQDS board depending on value in fpga register 0x54.
> > > > > >
> > > > > > Additionally the external MDIO1 is used to communicate to the
> > > > > > onboard RGMII phy devices.
> > > > > >
> > > > > > The mdio1 is controlled by bits 4-7 of fpga register and mdio2
> > > > > > is controlled by bits 0-3 of fpga register.
> > > > > >
> > > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > ---
> > > > > >
> > > > > > Notes:
> > > > > > V3:
> > > > > > - Add status = disabled in soc file and status = okay in board file
> > > > > > for external MDIO nodes
> > > > > > - Add interrupts property in external mdio nodes in soc file
> > > > > > V2:
> > > > > > - removed unnecassary TODO statements
> > > > > > - removed device_type from mdio nodes
> > > > > > - change the case of hex number to lowercase
> > > > > > - removed board specific comments from soc file
> > > > > >
> > > > > > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 123 +++++++++++++++++
> > > > > > .../boot/dts/freescale/fsl-lx2160a.dtsi | 22 +++
> > > > > > 2 files changed, 145 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > > index 99a22abbe725..079264b391a2 100644
> > > > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > > @@ -35,6 +35,14 @@
> > > > > > status = "okay";
> > > > > > };
> > > > > >
> > > > > > +&emdio1 {
> > > > > > + status = "okay";
> > > > > > +};
> > > > > > +
> > > > > > +&emdio2 {
> > > > > > + status = "okay";
> > > > > > +};
> > > > > > +
> > > > > > &esdhc0 {
> > > > > > status = "okay";
> > > > > > };
> > > > > > @@ -46,6 +54,121 @@
> > > > > > &i2c0 {
> > > > > > status = "okay";
> > > > > >
> > > > > > + fpga@66 {
> > > > > > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > > > > > + reg = <0x66>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > +
> > > > > > + mdio-mux-1@54 {
> > > > > > + mdio-parent-bus = <&emdio1>;
> > > > > > + reg = <0x54>; /* BRDCFG4 */
> > > > > > + mux-mask = <0xf8>; /* EMI1_MDIO */
> > > > > > + #address-cells=<1>;
> > > > > > + #size-cells = <0>;
> > > > > > +
> > > > > > + mdio@0 {
> > > > > > + reg = <0x00>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > >
> > > > > Please have a newline between nodes. It doesn't deserve a
> > > > > respin though. I can fix them up when applying if Leo is fine with this
> version.
> > > >
> > > > I think there should be a compatible string defined for the
> > > > binding of parent node mdio-mux, probably "mdio-mux-regmap", and
> > > > be used here in the device tree.
> > >
> > > I have two concerns :
> > > 1. The regmap is linux s/w construct, while device tree is h/w representation
> and is s/w agnostic. can we use regmap in device tree?
> >
> > Well, if we want to avoid using the regmap name, we probably can try
> > "mdio-mux-reg" or "mdio-mux-syscon"? With further search I also found
> > a more generic mux binding at Documentation/devicetree/bindings/mux/,
> > would be even better if we can use that to describe the mux.
>
>
> To make it more clear, with the use of
> Documentation/devicetree/bindings/mux/ binding, I think it would end up with
> something like this:
>
> i2c {
> fpga {
> compatible = "fsl,lx2160aqds-fpga", "syscon";
> ....
> mux: mux-controller {
> compatible = "mmio-mux"
The FPGA in LX2160AQDS is not mmio controlled but an I2c controlled FPGA.
As such there is no binding yet for mux nodes controlled by SPI or I2C devices.
Even the syscon binding is for MMIO controlled devices.
I have added such binding at https://patchwork.ozlabs.org/patch/1043790/
Let's see if this gets accepted or not? After that I will resend this patch.
> ...
> }
> }
> ...
> }
>
> mdio-mux {
> compatible = "mdio-mux"
> mux-controls = <&mux 0>;
> ....
> mdio {
> phy {
> }
> ...
> }
> mdio {
> ...
> }
> ...
> }
>
>
> }"
>
> >
> > > 2. By convention the device tree compatible binding is defined as
> "<manufacturer>,<model>" e.g. "fsl,mpc8349-uart". The mdio-mux node and it's
> sub nodes are a generic representation of mdio mux and it is not dependent on a
> particular manufacturer device. How to define the compatible in this case?
> >
> > The manufacturer prefix is for vendor specific bindings. If the
> > binding a suppose to be generic, we don't need the vendor prefix.
> >
> > >
> > > >
> > > > >
> > > > > Shawn
> > > > >
> > > > > > + mdio@40 {
> > > > > > + reg = <0x40>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@c0 {
> > > > > > + reg = <0xc0>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@c8 {
> > > > > > + reg = <0xc8>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@d0 {
> > > > > > + reg = <0xd0>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@d8 {
> > > > > > + reg = <0xd8>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@e0 {
> > > > > > + reg = <0xe0>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@e8 {
> > > > > > + reg = <0xe8>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@f0 {
> > > > > > + reg = <0xf0>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@f8 {
> > > > > > + reg = <0xf8>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + };
> > > > > > +
> > > > > > + mdio-mux-2@54 {
> > > > > > + mdio-parent-bus = <&emdio2>;
> > > > > > + reg = <0x54>; /* BRDCFG4 */
> > > > > > + mux-mask = <0x07>; /* EMI2_MDIO */
> > > > > > + #address-cells=<1>;
> > > > > > + #size-cells = <0>;
> > > > > > +
> > > > > > + mdio@0 {
> > > > > > + reg = <0x00>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@1 {
> > > > > > + reg = <0x01>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@2 {
> > > > > > + reg = <0x02>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@3 {
> > > > > > + reg = <0x03>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@4 {
> > > > > > + reg = <0x04>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@5 {
> > > > > > + reg = <0x05>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@6 {
> > > > > > + reg = <0x06>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + mdio@7 {
> > > > > > + reg = <0x07>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + };
> > > > > > + };
> > > > > > + };
> > > > > > +
> > > > > > i2c-mux@77 {
> > > > > > compatible = "nxp,pca9547";
> > > > > > reg = <0x77>;
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > > index a79f5c1ea56d..7def5252ac1a 100644
> > > > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > > @@ -762,5 +762,27 @@
> > > > > > <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > dma-coherent;
> > > > > > };
> > > > > > +
> > > > > > + /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
> > > > > > + emdio1: mdio@8b96000 {
> > > > > > + compatible = "fsl,fman-memac-mdio";
> > > > > > + reg = <0x0 0x8b96000 0x0 0x1000>;
> > > > > > + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + little-endian; /* force the driver in LE mode */
> > > > > > + status = "disabled";
> > > > > > + };
> > > > > > +
> > > > > > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > > > > > + emdio2: mdio@8b97000 {
> > > > > > + compatible = "fsl,fman-memac-mdio";
> > > > > > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > > > > > + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > + little-endian; /* force the driver in LE mode */
> > > > > > + status = "disabled";
> > > > > > + };
> > > > > > };
> > > > > > };
> > > > > > --
> > > > > > 2.17.1
> > > > > >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-18 5:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06 9:40 [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes Pankaj Bansal
2019-02-11 3:00 ` Shawn Guo
2019-02-11 20:43 ` Leo Li
2019-02-12 3:26 ` Pankaj Bansal
2019-02-12 18:01 ` Li Yang
2019-02-14 22:32 ` Li Yang
2019-02-18 5:49 ` Pankaj Bansal
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).