* [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-04 8:51 ` Pankaj Bansal
0 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-04 8:51 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:
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 | 115 +++++++++++++++++
.../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
2 files changed, 133 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..2c3020a72d41 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
@@ -46,6 +46,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..a74045ad22ad 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -762,5 +762,23 @@
<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>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ little-endian; /* force the driver in LE mode */
+ };
+
+ /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
+ emdio2: mdio@8b97000 {
+ compatible = "fsl,fman-memac-mdio";
+ reg = <0x0 0x8b97000 0x0 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ little-endian; /* force the driver in LE mode */
+ };
};
};
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-04 8:51 ` Pankaj Bansal
0 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-04 8:51 UTC (permalink / raw)
To: Shawn Guo, Leo Li, Andrew Lunn, Florian Fainelli
Cc: netdev, Pankaj Bansal, linux-arm-kernel
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:
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 | 115 +++++++++++++++++
.../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
2 files changed, 133 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..2c3020a72d41 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
@@ -46,6 +46,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..a74045ad22ad 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -762,5 +762,23 @@
<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>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ little-endian; /* force the driver in LE mode */
+ };
+
+ /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
+ emdio2: mdio@8b97000 {
+ compatible = "fsl,fman-memac-mdio";
+ reg = <0x0 0x8b97000 0x0 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ little-endian; /* force the driver in LE mode */
+ };
};
};
--
2.17.1
_______________________________________________
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] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-04 8:51 ` Pankaj Bansal
@ 2019-02-04 16:50 ` David Miller
-1 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2019-02-04 16:50 UTC (permalink / raw)
To: pankaj.bansal
Cc: shawnguo, leoyang.li, andrew, f.fainelli, netdev, linux-arm-kernel
From: Pankaj Bansal <pankaj.bansal@nxp.com>
Date: Mon, 4 Feb 2019 08:51:57 +0000
> 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>
Am I applying this to my networking tree or are the ARM folks taking this?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-04 16:50 ` David Miller
0 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2019-02-04 16:50 UTC (permalink / raw)
To: pankaj.bansal
Cc: andrew, f.fainelli, netdev, leoyang.li, shawnguo, linux-arm-kernel
From: Pankaj Bansal <pankaj.bansal@nxp.com>
Date: Mon, 4 Feb 2019 08:51:57 +0000
> 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>
Am I applying this to my networking tree or are the ARM folks taking this?
_______________________________________________
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] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-04 16:50 ` David Miller
@ 2019-02-05 12:23 ` Pankaj Bansal
-1 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-05 12:23 UTC (permalink / raw)
To: Leo Li, shawnguo
Cc: andrew, f.fainelli, netdev, linux-arm-kernel, David Miller
Hi Shawn/Leo,
If you have no more comments, can you please merge this path in your branch?
in same branch in which you have accepted LX2160AQDS board patches.
Regards,
Pankaj Bansal
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, 4 February, 2019 10:21 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; andrew@lunn.ch;
> f.fainelli@gmail.com; netdev@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
> Date: Mon, 4 Feb 2019 08:51:57 +0000
>
> > 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>
>
> Am I applying this to my networking tree or are the ARM folks taking this?
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-05 12:23 ` Pankaj Bansal
0 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-05 12:23 UTC (permalink / raw)
To: Leo Li, shawnguo
Cc: andrew, f.fainelli, David Miller, linux-arm-kernel, netdev
Hi Shawn/Leo,
If you have no more comments, can you please merge this path in your branch?
in same branch in which you have accepted LX2160AQDS board patches.
Regards,
Pankaj Bansal
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, 4 February, 2019 10:21 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; andrew@lunn.ch;
> f.fainelli@gmail.com; netdev@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
> Date: Mon, 4 Feb 2019 08:51:57 +0000
>
> > 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>
>
> Am I applying this to my networking tree or are the ARM folks taking this?
_______________________________________________
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] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-04 8:51 ` Pankaj Bansal
@ 2019-02-05 18:37 ` Li Yang
-1 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-05 18:37 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Shawn Guo, Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
On Mon, Feb 4, 2019 at 2:53 AM Pankaj Bansal <pankaj.bansal@nxp.com> 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:
> 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
There are still some comments from V1 haven't been addressed.
>
> .../boot/dts/freescale/fsl-lx2160a-qds.dts | 115 +++++++++++++++++
> .../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
> 2 files changed, 133 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..2c3020a72d41 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> @@ -46,6 +46,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 {
Still no compatible string defined for the node. Probably should be
"mdio-mux-mmioreg", "mdio-mux"
> + 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..a74045ad22ad 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -762,5 +762,23 @@
> <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>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + little-endian; /* force the driver in LE mode */
Still doesn't fully align with the binding at
"Documentation/devicetree/bindings/powerpc/fsl/fman.txt".
It should either have the "interrupts" property for external MDIO or
"fsl,fman-internal-mdio" for internal MDIO.
> + };
> +
> + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> + emdio2: mdio@8b97000 {
> + compatible = "fsl,fman-memac-mdio";
> + reg = <0x0 0x8b97000 0x0 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + little-endian; /* force the driver in LE mode */
> + };
> };
> };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-05 18:37 ` Li Yang
0 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-05 18:37 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Andrew Lunn, Florian Fainelli, Shawn Guo, linux-arm-kernel, netdev
On Mon, Feb 4, 2019 at 2:53 AM Pankaj Bansal <pankaj.bansal@nxp.com> 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:
> 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
There are still some comments from V1 haven't been addressed.
>
> .../boot/dts/freescale/fsl-lx2160a-qds.dts | 115 +++++++++++++++++
> .../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
> 2 files changed, 133 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..2c3020a72d41 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> @@ -46,6 +46,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 {
Still no compatible string defined for the node. Probably should be
"mdio-mux-mmioreg", "mdio-mux"
> + 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..a74045ad22ad 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -762,5 +762,23 @@
> <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>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + little-endian; /* force the driver in LE mode */
Still doesn't fully align with the binding at
"Documentation/devicetree/bindings/powerpc/fsl/fman.txt".
It should either have the "interrupts" property for external MDIO or
"fsl,fman-internal-mdio" for internal MDIO.
> + };
> +
> + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> + emdio2: mdio@8b97000 {
> + compatible = "fsl,fman-memac-mdio";
> + reg = <0x0 0x8b97000 0x0 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + little-endian; /* force the driver in LE mode */
> + };
> };
> };
> --
> 2.17.1
>
_______________________________________________
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] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-05 12:23 ` Pankaj Bansal
@ 2019-02-05 18:38 ` Li Yang
-1 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-05 18:38 UTC (permalink / raw)
To: Pankaj Bansal
Cc: shawnguo, andrew, f.fainelli, netdev, linux-arm-kernel, David Miller
On Tue, Feb 5, 2019 at 6:26 AM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
> Hi Shawn/Leo,
>
> If you have no more comments, can you please merge this path in your branch?
> in same branch in which you have accepted LX2160AQDS board patches.
It can be merged through the ARM SoC tree. But there are still
comments remain to be addressed.
>
> Regards,
> Pankaj Bansal
>
> > -----Original Message-----
> > From: David Miller [mailto:davem@davemloft.net]
> > Sent: Monday, 4 February, 2019 10:21 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; andrew@lunn.ch;
> > f.fainelli@gmail.com; netdev@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org
> > Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> > From: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Date: Mon, 4 Feb 2019 08:51:57 +0000
> >
> > > 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>
> >
> > Am I applying this to my networking tree or are the ARM folks taking this?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-05 18:38 ` Li Yang
0 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-05 18:38 UTC (permalink / raw)
To: Pankaj Bansal
Cc: andrew, f.fainelli, netdev, shawnguo, David Miller, linux-arm-kernel
On Tue, Feb 5, 2019 at 6:26 AM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
> Hi Shawn/Leo,
>
> If you have no more comments, can you please merge this path in your branch?
> in same branch in which you have accepted LX2160AQDS board patches.
It can be merged through the ARM SoC tree. But there are still
comments remain to be addressed.
>
> Regards,
> Pankaj Bansal
>
> > -----Original Message-----
> > From: David Miller [mailto:davem@davemloft.net]
> > Sent: Monday, 4 February, 2019 10:21 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; andrew@lunn.ch;
> > f.fainelli@gmail.com; netdev@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org
> > Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> > From: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Date: Mon, 4 Feb 2019 08:51:57 +0000
> >
> > > 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>
> >
> > Am I applying this to my networking tree or are the ARM folks taking this?
_______________________________________________
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] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-05 18:37 ` Li Yang
@ 2019-02-06 4:01 ` Pankaj Bansal
-1 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-06 4:01 UTC (permalink / raw)
To: Leo Li; +Cc: Shawn Guo, Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@nxp.com]
> Sent: Wednesday, 6 February, 2019 12:07 AM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> 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 v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> On Mon, Feb 4, 2019 at 2:53 AM Pankaj Bansal <pankaj.bansal@nxp.com>
> 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:
> > 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
>
> There are still some comments from V1 haven't been addressed.
>
> >
> > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 115 +++++++++++++++++
> > .../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
> > 2 files changed, 133 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..2c3020a72d41 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > @@ -46,6 +46,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 {
>
> Still no compatible string defined for the node. Probably should be "mdio-mux-
> mmioreg", "mdio-mux"
it is not a specific device. MDIO mux is meant to be controlled by some registers of parent device (FPGA).
Therefore, IMO this should not be a device and there should not be any "compatible" property for it.
>
> > + 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..a74045ad22ad 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > @@ -762,5 +762,23 @@
> > <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>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + little-endian; /* force the driver in LE mode
> > + */
>
> Still doesn't fully align with the binding at
> "Documentation/devicetree/bindings/powerpc/fsl/fman.txt".
>
> It should either have the "interrupts" property for external MDIO or "fsl,fman-
> internal-mdio" for internal MDIO.
I see that for DPAA1 based SOCs, there was definitely an interrupt property in external MDIO node.
BUT, for DPAA2 based devices none of the SOC has this property. I am looking further into this.
>
> > + };
> > +
> > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > + emdio2: mdio@8b97000 {
> > + compatible = "fsl,fman-memac-mdio";
> > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + little-endian; /* force the driver in LE mode */
> > + };
> > };
> > };
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-06 4:01 ` Pankaj Bansal
0 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-06 4:01 UTC (permalink / raw)
To: Leo Li; +Cc: Andrew Lunn, Florian Fainelli, Shawn Guo, linux-arm-kernel, netdev
> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@nxp.com]
> Sent: Wednesday, 6 February, 2019 12:07 AM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> 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 v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> On Mon, Feb 4, 2019 at 2:53 AM Pankaj Bansal <pankaj.bansal@nxp.com>
> 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:
> > 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
>
> There are still some comments from V1 haven't been addressed.
>
> >
> > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 115 +++++++++++++++++
> > .../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
> > 2 files changed, 133 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..2c3020a72d41 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > @@ -46,6 +46,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 {
>
> Still no compatible string defined for the node. Probably should be "mdio-mux-
> mmioreg", "mdio-mux"
it is not a specific device. MDIO mux is meant to be controlled by some registers of parent device (FPGA).
Therefore, IMO this should not be a device and there should not be any "compatible" property for it.
>
> > + 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..a74045ad22ad 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > @@ -762,5 +762,23 @@
> > <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>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + little-endian; /* force the driver in LE mode
> > + */
>
> Still doesn't fully align with the binding at
> "Documentation/devicetree/bindings/powerpc/fsl/fman.txt".
>
> It should either have the "interrupts" property for external MDIO or "fsl,fman-
> internal-mdio" for internal MDIO.
I see that for DPAA1 based SOCs, there was definitely an interrupt property in external MDIO node.
BUT, for DPAA2 based devices none of the SOC has this property. I am looking further into this.
>
> > + };
> > +
> > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > + emdio2: mdio@8b97000 {
> > + compatible = "fsl,fman-memac-mdio";
> > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + little-endian; /* force the driver in LE mode */
> > + };
> > };
> > };
> > --
> > 2.17.1
> >
_______________________________________________
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] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-06 4:01 ` Pankaj Bansal
@ 2019-02-06 21:17 ` Leo Li
-1 siblings, 0 replies; 22+ messages in thread
From: Leo Li @ 2019-02-06 21:17 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Shawn Guo, Andrew Lunn, Florian Fainelli, netdev, linux-arm-kernel
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Tuesday, February 5, 2019 10:02 PM
> To: Leo Li <leoyang.li@nxp.com>
> 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 v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
>
>
> > -----Original Message-----
> > From: Li Yang [mailto:leoyang.li@nxp.com]
> > Sent: Wednesday, 6 February, 2019 12:07 AM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > 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 v2] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> > On Mon, Feb 4, 2019 at 2:53 AM Pankaj Bansal <pankaj.bansal@nxp.com>
> > 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:
> > > 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
> >
> > There are still some comments from V1 haven't been addressed.
> >
> > >
> > > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 115 +++++++++++++++++
> > > .../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
> > > 2 files changed, 133 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..2c3020a72d41 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > @@ -46,6 +46,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 {
> >
> > Still no compatible string defined for the node. Probably should be
> > "mdio-mux- mmioreg", "mdio-mux"
>
> it is not a specific device. MDIO mux is meant to be controlled by some
> registers of parent device (FPGA).
> Therefore, IMO this should not be a device and there should not be any
> "compatible" property for it.
If it is not a device why we are defining a device node for it? It is probably not a physical device per se, but it can be considered a virtual device provided by FPGA.
This also bring up another question that why this device cannot reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver? If we think regmap is a better solution, shall we replace the mmioreg driver with the regmap driver?
>
> >
> > > + 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..a74045ad22ad 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > @@ -762,5 +762,23 @@
> > > <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>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + little-endian; /* force the driver in LE
> > > + mode */
> >
> > Still doesn't fully align with the binding at
> > "Documentation/devicetree/bindings/powerpc/fsl/fman.txt".
> >
> > It should either have the "interrupts" property for external MDIO or
> > "fsl,fman- internal-mdio" for internal MDIO.
>
> I see that for DPAA1 based SOCs, there was definitely an interrupt property
> in external MDIO node.
> BUT, for DPAA2 based devices none of the SOC has this property. I am
> looking further into this.
>
> >
> > > + };
> > > +
> > > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > > + emdio2: mdio@8b97000 {
> > > + compatible = "fsl,fman-memac-mdio";
> > > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + little-endian; /* force the driver in LE mode */
> > > + };
> > > };
> > > };
> > > --
> > > 2.17.1
> > >
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-06 21:17 ` Leo Li
0 siblings, 0 replies; 22+ messages in thread
From: Leo Li @ 2019-02-06 21:17 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Andrew Lunn, Florian Fainelli, Shawn Guo, linux-arm-kernel, netdev
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Tuesday, February 5, 2019 10:02 PM
> To: Leo Li <leoyang.li@nxp.com>
> 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 v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
>
>
> > -----Original Message-----
> > From: Li Yang [mailto:leoyang.li@nxp.com]
> > Sent: Wednesday, 6 February, 2019 12:07 AM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > 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 v2] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> > On Mon, Feb 4, 2019 at 2:53 AM Pankaj Bansal <pankaj.bansal@nxp.com>
> > 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:
> > > 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
> >
> > There are still some comments from V1 haven't been addressed.
> >
> > >
> > > .../boot/dts/freescale/fsl-lx2160a-qds.dts | 115 +++++++++++++++++
> > > .../boot/dts/freescale/fsl-lx2160a.dtsi | 18 +++
> > > 2 files changed, 133 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..2c3020a72d41 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > @@ -46,6 +46,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 {
> >
> > Still no compatible string defined for the node. Probably should be
> > "mdio-mux- mmioreg", "mdio-mux"
>
> it is not a specific device. MDIO mux is meant to be controlled by some
> registers of parent device (FPGA).
> Therefore, IMO this should not be a device and there should not be any
> "compatible" property for it.
If it is not a device why we are defining a device node for it? It is probably not a physical device per se, but it can be considered a virtual device provided by FPGA.
This also bring up another question that why this device cannot reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver? If we think regmap is a better solution, shall we replace the mmioreg driver with the regmap driver?
>
> >
> > > + 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..a74045ad22ad 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > @@ -762,5 +762,23 @@
> > > <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>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + little-endian; /* force the driver in LE
> > > + mode */
> >
> > Still doesn't fully align with the binding at
> > "Documentation/devicetree/bindings/powerpc/fsl/fman.txt".
> >
> > It should either have the "interrupts" property for external MDIO or
> > "fsl,fman- internal-mdio" for internal MDIO.
>
> I see that for DPAA1 based SOCs, there was definitely an interrupt property
> in external MDIO node.
> BUT, for DPAA2 based devices none of the SOC has this property. I am
> looking further into this.
>
> >
> > > + };
> > > +
> > > + /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > > + emdio2: mdio@8b97000 {
> > > + compatible = "fsl,fman-memac-mdio";
> > > + reg = <0x0 0x8b97000 0x0 0x1000>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + little-endian; /* force the driver in LE mode */
> > > + };
> > > };
> > > };
> > > --
> > > 2.17.1
> > >
_______________________________________________
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] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-06 21:17 ` Leo Li
@ 2019-02-06 21:44 ` Andrew Lunn
-1 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2019-02-06 21:44 UTC (permalink / raw)
To: Leo Li
Cc: Pankaj Bansal, Shawn Guo, Florian Fainelli, netdev, linux-arm-kernel
> > > > &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 {
> > >
> > > Still no compatible string defined for the node. Probably should be
> > > "mdio-mux- mmioreg", "mdio-mux"
> >
> > it is not a specific device. MDIO mux is meant to be controlled by some
> > registers of parent device (FPGA).
> > Therefore, IMO this should not be a device and there should not be any
> > "compatible" property for it.
> If it is not a device why we are defining a device node for it? It
> is probably not a physical device per se, but it can be considered a
> virtual device provided by FPGA.
It is a physical device. But it happens to be embedded inside another
device. And that embedded is not performed as a bus with devices on
it, so the device tree concepts don't fit directly.
> This also bring up another question that why this device cannot
> reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
Because it is on an i2c bus, not an mmio bus.
> If we think regmap is a better solution, shall we replace the
> mmioreg driver with the regmap driver?
regmap can be used with mmio. But for a single MMIO register it is a
huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
If however the device is already using regmap, adding one more
register is very little overhead. And it might be possible to use this
new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
covering the best of both worlds.
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-06 21:44 ` Andrew Lunn
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2019-02-06 21:44 UTC (permalink / raw)
To: Leo Li
Cc: netdev, Pankaj Bansal, Florian Fainelli, Shawn Guo, linux-arm-kernel
> > > > &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 {
> > >
> > > Still no compatible string defined for the node. Probably should be
> > > "mdio-mux- mmioreg", "mdio-mux"
> >
> > it is not a specific device. MDIO mux is meant to be controlled by some
> > registers of parent device (FPGA).
> > Therefore, IMO this should not be a device and there should not be any
> > "compatible" property for it.
> If it is not a device why we are defining a device node for it? It
> is probably not a physical device per se, but it can be considered a
> virtual device provided by FPGA.
It is a physical device. But it happens to be embedded inside another
device. And that embedded is not performed as a bus with devices on
it, so the device tree concepts don't fit directly.
> This also bring up another question that why this device cannot
> reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
Because it is on an i2c bus, not an mmio bus.
> If we think regmap is a better solution, shall we replace the
> mmioreg driver with the regmap driver?
regmap can be used with mmio. But for a single MMIO register it is a
huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
If however the device is already using regmap, adding one more
register is very little overhead. And it might be possible to use this
new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
covering the best of both worlds.
Andrew
_______________________________________________
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] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-06 21:44 ` Andrew Lunn
@ 2019-02-06 23:39 ` Li Yang
-1 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-06 23:39 UTC (permalink / raw)
To: Andrew Lunn
Cc: Pankaj Bansal, Shawn Guo, Florian Fainelli, netdev,
linux-arm-kernel, Rob Herring
On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > > &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 {
> > > >
> > > > Still no compatible string defined for the node. Probably should be
> > > > "mdio-mux- mmioreg", "mdio-mux"
> > >
> > > it is not a specific device. MDIO mux is meant to be controlled by some
> > > registers of parent device (FPGA).
> > > Therefore, IMO this should not be a device and there should not be any
> > > "compatible" property for it.
>
> > If it is not a device why we are defining a device node for it? It
> > is probably not a physical device per se, but it can be considered a
> > virtual device provided by FPGA.
>
> It is a physical device. But it happens to be embedded inside another
> device. And that embedded is not performed as a bus with devices on
> it, so the device tree concepts don't fit directly.
Whether or not it is populated as a bus(which probably should as the
FPGA does contain many different functions and these functions like
the mdio-mux we are discussing about could have separate drivers), the
node should have a new binding documentation similar to the
mdio_mux_mmioreg binding or even covers the mmioreg too. And the best
way to match the node with the binding is through compatible strings
IMO. This is why I'm asking the node to have a compatible string.
>
> > This also bring up another question that why this device cannot
> > reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
>
> Because it is on an i2c bus, not an mmio bus.
Oops, I missed that.
>
> > If we think regmap is a better solution, shall we replace the
> > mmioreg driver with the regmap driver?
>
> regmap can be used with mmio. But for a single MMIO register it is a
> huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
>
> If however the device is already using regmap, adding one more
> register is very little overhead. And it might be possible to use this
> new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
> covering the best of both worlds.
Ya. It would be ideal if the new driver can cover the legacy
mdio-mux-mmioreg case too.
Regards,
Leo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-06 23:39 ` Li Yang
0 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-06 23:39 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, netdev, Pankaj Bansal, Rob Herring, Shawn Guo,
linux-arm-kernel
On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > > &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 {
> > > >
> > > > Still no compatible string defined for the node. Probably should be
> > > > "mdio-mux- mmioreg", "mdio-mux"
> > >
> > > it is not a specific device. MDIO mux is meant to be controlled by some
> > > registers of parent device (FPGA).
> > > Therefore, IMO this should not be a device and there should not be any
> > > "compatible" property for it.
>
> > If it is not a device why we are defining a device node for it? It
> > is probably not a physical device per se, but it can be considered a
> > virtual device provided by FPGA.
>
> It is a physical device. But it happens to be embedded inside another
> device. And that embedded is not performed as a bus with devices on
> it, so the device tree concepts don't fit directly.
Whether or not it is populated as a bus(which probably should as the
FPGA does contain many different functions and these functions like
the mdio-mux we are discussing about could have separate drivers), the
node should have a new binding documentation similar to the
mdio_mux_mmioreg binding or even covers the mmioreg too. And the best
way to match the node with the binding is through compatible strings
IMO. This is why I'm asking the node to have a compatible string.
>
> > This also bring up another question that why this device cannot
> > reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
>
> Because it is on an i2c bus, not an mmio bus.
Oops, I missed that.
>
> > If we think regmap is a better solution, shall we replace the
> > mmioreg driver with the regmap driver?
>
> regmap can be used with mmio. But for a single MMIO register it is a
> huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
>
> If however the device is already using regmap, adding one more
> register is very little overhead. And it might be possible to use this
> new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
> covering the best of both worlds.
Ya. It would be ideal if the new driver can cover the legacy
mdio-mux-mmioreg case too.
Regards,
Leo
_______________________________________________
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] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-06 23:39 ` Li Yang
@ 2019-02-07 4:42 ` Pankaj Bansal
-1 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-07 4:42 UTC (permalink / raw)
To: Leo Li, Andrew Lunn
Cc: Shawn Guo, Florian Fainelli, netdev, linux-arm-kernel, Rob Herring
> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@nxp.com]
> Sent: Thursday, 7 February, 2019 05:09 AM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Shawn Guo
> <shawnguo@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Rob Herring
> <robh+dt@kernel.org>
> Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > > > &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 {
> > > > >
> > > > > Still no compatible string defined for the node. Probably
> > > > > should be
> > > > > "mdio-mux- mmioreg", "mdio-mux"
> > > >
> > > > it is not a specific device. MDIO mux is meant to be controlled by
> > > > some registers of parent device (FPGA).
> > > > Therefore, IMO this should not be a device and there should not be
> > > > any "compatible" property for it.
> >
> > > If it is not a device why we are defining a device node for it? It
> > > is probably not a physical device per se, but it can be considered a
> > > virtual device provided by FPGA.
> >
> > It is a physical device. But it happens to be embedded inside another
> > device. And that embedded is not performed as a bus with devices on
> > it, so the device tree concepts don't fit directly.
>
> Whether or not it is populated as a bus(which probably should as the FPGA does
> contain many different functions and these functions like the mdio-mux we are
> discussing about could have separate drivers), the node should have a new
> binding documentation similar to the mdio_mux_mmioreg binding or even
> covers the mmioreg too. And the best way to match the node with the binding
> is through compatible strings IMO. This is why I'm asking the node to have a
> compatible string.
The mdio_mux is NOT a device. FPGA is a device that provides the mdio mux functionality
(among other functions). The mdio mux is controlled via some bits In one of the FPGA register.
In my previous approach, I also used a compatible field for mdio_mux node in FPGA.
https://www.mail-archive.com/netdev@vger.kernel.org/msg252744.html
The FPGA driver would create as many platform devices for each subnode, and those devices
Would attach to mdio_mux_regmap driver based on compatible field.
BUT, this platform device creation is the problem. Since mdio_mux is not an actual device, how can
We create a platform device for it?
Which is why I removed the compatible field from mdio_mux nodes. The FPGA driver detects the mdio_mux
Using their name i.e. " mdio-mux-1@54". Like this:
for_each_child_of_node(dev->of_node, child) {
if (!of_node_name_prefix(child, "mdio-mux"))
Refer : https://patchwork.kernel.org/patch/10797227/
>
> >
> > > This also bring up another question that why this device cannot
> > > reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
> >
> > Because it is on an i2c bus, not an mmio bus.
>
> Oops, I missed that.
>
> >
> > > If we think regmap is a better solution, shall we replace the
> > > mmioreg driver with the regmap driver?
> >
> > regmap can be used with mmio. But for a single MMIO register it is a
> > huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
> >
> > If however the device is already using regmap, adding one more
> > register is very little overhead. And it might be possible to use this
> > new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
> > covering the best of both worlds.
>
> Ya. It would be ideal if the new driver can cover the legacy mdio-mux-mmioreg
> case too.
Refer : https://patchwork.kernel.org/patch/10797227/
The mdio_mux_regmap can be used by any FPGA be it i2c connected or MMIO based.
>
> Regards,
> Leo
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-07 4:42 ` Pankaj Bansal
0 siblings, 0 replies; 22+ messages in thread
From: Pankaj Bansal @ 2019-02-07 4:42 UTC (permalink / raw)
To: Leo Li, Andrew Lunn
Cc: netdev, Florian Fainelli, Shawn Guo, Rob Herring, linux-arm-kernel
> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@nxp.com]
> Sent: Thursday, 7 February, 2019 05:09 AM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Shawn Guo
> <shawnguo@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Rob Herring
> <robh+dt@kernel.org>
> Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
>
> On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > > > &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 {
> > > > >
> > > > > Still no compatible string defined for the node. Probably
> > > > > should be
> > > > > "mdio-mux- mmioreg", "mdio-mux"
> > > >
> > > > it is not a specific device. MDIO mux is meant to be controlled by
> > > > some registers of parent device (FPGA).
> > > > Therefore, IMO this should not be a device and there should not be
> > > > any "compatible" property for it.
> >
> > > If it is not a device why we are defining a device node for it? It
> > > is probably not a physical device per se, but it can be considered a
> > > virtual device provided by FPGA.
> >
> > It is a physical device. But it happens to be embedded inside another
> > device. And that embedded is not performed as a bus with devices on
> > it, so the device tree concepts don't fit directly.
>
> Whether or not it is populated as a bus(which probably should as the FPGA does
> contain many different functions and these functions like the mdio-mux we are
> discussing about could have separate drivers), the node should have a new
> binding documentation similar to the mdio_mux_mmioreg binding or even
> covers the mmioreg too. And the best way to match the node with the binding
> is through compatible strings IMO. This is why I'm asking the node to have a
> compatible string.
The mdio_mux is NOT a device. FPGA is a device that provides the mdio mux functionality
(among other functions). The mdio mux is controlled via some bits In one of the FPGA register.
In my previous approach, I also used a compatible field for mdio_mux node in FPGA.
https://www.mail-archive.com/netdev@vger.kernel.org/msg252744.html
The FPGA driver would create as many platform devices for each subnode, and those devices
Would attach to mdio_mux_regmap driver based on compatible field.
BUT, this platform device creation is the problem. Since mdio_mux is not an actual device, how can
We create a platform device for it?
Which is why I removed the compatible field from mdio_mux nodes. The FPGA driver detects the mdio_mux
Using their name i.e. " mdio-mux-1@54". Like this:
for_each_child_of_node(dev->of_node, child) {
if (!of_node_name_prefix(child, "mdio-mux"))
Refer : https://patchwork.kernel.org/patch/10797227/
>
> >
> > > This also bring up another question that why this device cannot
> > > reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
> >
> > Because it is on an i2c bus, not an mmio bus.
>
> Oops, I missed that.
>
> >
> > > If we think regmap is a better solution, shall we replace the
> > > mmioreg driver with the regmap driver?
> >
> > regmap can be used with mmio. But for a single MMIO register it is a
> > huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
> >
> > If however the device is already using regmap, adding one more
> > register is very little overhead. And it might be possible to use this
> > new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
> > covering the best of both worlds.
>
> Ya. It would be ideal if the new driver can cover the legacy mdio-mux-mmioreg
> case too.
Refer : https://patchwork.kernel.org/patch/10797227/
The mdio_mux_regmap can be used by any FPGA be it i2c connected or MMIO based.
>
> Regards,
> Leo
_______________________________________________
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] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
2019-02-07 4:42 ` Pankaj Bansal
@ 2019-02-07 23:24 ` Li Yang
-1 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-07 23:24 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Andrew Lunn, Shawn Guo, Florian Fainelli, netdev,
linux-arm-kernel, Rob Herring
On Wed, Feb 6, 2019 at 10:44 PM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Li Yang [mailto:leoyang.li@nxp.com]
> > Sent: Thursday, 7 February, 2019 05:09 AM
> > To: Andrew Lunn <andrew@lunn.ch>
> > Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Shawn Guo
> > <shawnguo@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>;
> > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Rob Herring
> > <robh+dt@kernel.org>
> > Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> > On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > > > > &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 {
> > > > > >
> > > > > > Still no compatible string defined for the node. Probably
> > > > > > should be
> > > > > > "mdio-mux- mmioreg", "mdio-mux"
> > > > >
> > > > > it is not a specific device. MDIO mux is meant to be controlled by
> > > > > some registers of parent device (FPGA).
> > > > > Therefore, IMO this should not be a device and there should not be
> > > > > any "compatible" property for it.
> > >
> > > > If it is not a device why we are defining a device node for it? It
> > > > is probably not a physical device per se, but it can be considered a
> > > > virtual device provided by FPGA.
> > >
> > > It is a physical device. But it happens to be embedded inside another
> > > device. And that embedded is not performed as a bus with devices on
> > > it, so the device tree concepts don't fit directly.
> >
> > Whether or not it is populated as a bus(which probably should as the FPGA does
> > contain many different functions and these functions like the mdio-mux we are
> > discussing about could have separate drivers), the node should have a new
> > binding documentation similar to the mdio_mux_mmioreg binding or even
> > covers the mmioreg too. And the best way to match the node with the binding
> > is through compatible strings IMO. This is why I'm asking the node to have a
> > compatible string.
>
> The mdio_mux is NOT a device. FPGA is a device that provides the mdio mux functionality
> (among other functions). The mdio mux is controlled via some bits In one of the FPGA register.
With modern chips, it is likely to have multiple functions in a single
physical device that are covered by multiple subsystems. We shouldn't
limit the concept of device to only physical devices.
>
> In my previous approach, I also used a compatible field for mdio_mux node in FPGA.
> https://www.mail-archive.com/netdev@vger.kernel.org/msg252744.html
> The FPGA driver would create as many platform devices for each subnode, and those devices
> Would attach to mdio_mux_regmap driver based on compatible field.
>
> BUT, this platform device creation is the problem. Since mdio_mux is not an actual device, how can
> We create a platform device for it?
Like I said platform device doesn't have to be a physically separate
device. In this specific case, I think a multi-function device(MFD)
will be the best fit for this FPGA device. The framework will help to
create sub-devices and help to share the regmap to all the
sub-function drivers. Please check existing MFD drivers for more
details.
>
> Which is why I removed the compatible field from mdio_mux nodes. The FPGA driver detects the mdio_mux
> Using their name i.e. " mdio-mux-1@54". Like this:
> for_each_child_of_node(dev->of_node, child) {
> if (!of_node_name_prefix(child, "mdio-mux"))
>
> Refer : https://patchwork.kernel.org/patch/10797227/
>
> >
> > >
> > > > This also bring up another question that why this device cannot
> > > > reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
> > >
> > > Because it is on an i2c bus, not an mmio bus.
> >
> > Oops, I missed that.
> >
> > >
> > > > If we think regmap is a better solution, shall we replace the
> > > > mmioreg driver with the regmap driver?
> > >
> > > regmap can be used with mmio. But for a single MMIO register it is a
> > > huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
> > >
> > > If however the device is already using regmap, adding one more
> > > register is very little overhead. And it might be possible to use this
> > > new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
> > > covering the best of both worlds.
> >
> > Ya. It would be ideal if the new driver can cover the legacy mdio-mux-mmioreg
> > case too.
>
> Refer : https://patchwork.kernel.org/patch/10797227/
> The mdio_mux_regmap can be used by any FPGA be it i2c connected or MMIO based.
>
> >
> > Regards,
> > Leo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
@ 2019-02-07 23:24 ` Li Yang
0 siblings, 0 replies; 22+ messages in thread
From: Li Yang @ 2019-02-07 23:24 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Andrew Lunn, Florian Fainelli, netdev, Rob Herring, Shawn Guo,
linux-arm-kernel
On Wed, Feb 6, 2019 at 10:44 PM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Li Yang [mailto:leoyang.li@nxp.com]
> > Sent: Thursday, 7 February, 2019 05:09 AM
> > To: Andrew Lunn <andrew@lunn.ch>
> > Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Shawn Guo
> > <shawnguo@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>;
> > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Rob Herring
> > <robh+dt@kernel.org>
> > Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes
> >
> > On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > > > > &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 {
> > > > > >
> > > > > > Still no compatible string defined for the node. Probably
> > > > > > should be
> > > > > > "mdio-mux- mmioreg", "mdio-mux"
> > > > >
> > > > > it is not a specific device. MDIO mux is meant to be controlled by
> > > > > some registers of parent device (FPGA).
> > > > > Therefore, IMO this should not be a device and there should not be
> > > > > any "compatible" property for it.
> > >
> > > > If it is not a device why we are defining a device node for it? It
> > > > is probably not a physical device per se, but it can be considered a
> > > > virtual device provided by FPGA.
> > >
> > > It is a physical device. But it happens to be embedded inside another
> > > device. And that embedded is not performed as a bus with devices on
> > > it, so the device tree concepts don't fit directly.
> >
> > Whether or not it is populated as a bus(which probably should as the FPGA does
> > contain many different functions and these functions like the mdio-mux we are
> > discussing about could have separate drivers), the node should have a new
> > binding documentation similar to the mdio_mux_mmioreg binding or even
> > covers the mmioreg too. And the best way to match the node with the binding
> > is through compatible strings IMO. This is why I'm asking the node to have a
> > compatible string.
>
> The mdio_mux is NOT a device. FPGA is a device that provides the mdio mux functionality
> (among other functions). The mdio mux is controlled via some bits In one of the FPGA register.
With modern chips, it is likely to have multiple functions in a single
physical device that are covered by multiple subsystems. We shouldn't
limit the concept of device to only physical devices.
>
> In my previous approach, I also used a compatible field for mdio_mux node in FPGA.
> https://www.mail-archive.com/netdev@vger.kernel.org/msg252744.html
> The FPGA driver would create as many platform devices for each subnode, and those devices
> Would attach to mdio_mux_regmap driver based on compatible field.
>
> BUT, this platform device creation is the problem. Since mdio_mux is not an actual device, how can
> We create a platform device for it?
Like I said platform device doesn't have to be a physically separate
device. In this specific case, I think a multi-function device(MFD)
will be the best fit for this FPGA device. The framework will help to
create sub-devices and help to share the regmap to all the
sub-function drivers. Please check existing MFD drivers for more
details.
>
> Which is why I removed the compatible field from mdio_mux nodes. The FPGA driver detects the mdio_mux
> Using their name i.e. " mdio-mux-1@54". Like this:
> for_each_child_of_node(dev->of_node, child) {
> if (!of_node_name_prefix(child, "mdio-mux"))
>
> Refer : https://patchwork.kernel.org/patch/10797227/
>
> >
> > >
> > > > This also bring up another question that why this device cannot
> > > > reuse the existing drivers/net/phy/mdio-mux-mmioreg.c driver?
> > >
> > > Because it is on an i2c bus, not an mmio bus.
> >
> > Oops, I missed that.
> >
> > >
> > > > If we think regmap is a better solution, shall we replace the
> > > > mmioreg driver with the regmap driver?
> > >
> > > regmap can be used with mmio. But for a single MMIO register it is a
> > > huge framework. So it makes sense to keep mdio-mux-mmioreg simple.
> > >
> > > If however the device is already using regmap, adding one more
> > > register is very little overhead. And it might be possible to use this
> > > new mux with an mmio regmap, or an spi regmap, etc. So we seem to be
> > > covering the best of both worlds.
> >
> > Ya. It would be ideal if the new driver can cover the legacy mdio-mux-mmioreg
> > case too.
>
> Refer : https://patchwork.kernel.org/patch/10797227/
> The mdio_mux_regmap can be used by any FPGA be it i2c connected or MMIO based.
>
> >
> > Regards,
> > Leo
_______________________________________________
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] 22+ messages in thread
end of thread, other threads:[~2019-02-07 23:24 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-04 8:51 [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes Pankaj Bansal
2019-02-04 8:51 ` Pankaj Bansal
2019-02-04 16:50 ` David Miller
2019-02-04 16:50 ` David Miller
2019-02-05 12:23 ` Pankaj Bansal
2019-02-05 12:23 ` Pankaj Bansal
2019-02-05 18:38 ` Li Yang
2019-02-05 18:38 ` Li Yang
2019-02-05 18:37 ` Li Yang
2019-02-05 18:37 ` Li Yang
2019-02-06 4:01 ` Pankaj Bansal
2019-02-06 4:01 ` Pankaj Bansal
2019-02-06 21:17 ` Leo Li
2019-02-06 21:17 ` Leo Li
2019-02-06 21:44 ` Andrew Lunn
2019-02-06 21:44 ` Andrew Lunn
2019-02-06 23:39 ` Li Yang
2019-02-06 23:39 ` Li Yang
2019-02-07 4:42 ` Pankaj Bansal
2019-02-07 4:42 ` Pankaj Bansal
2019-02-07 23:24 ` Li Yang
2019-02-07 23:24 ` Li Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.