diff mbox series

[v3] arm64: dts: lx2160aqds: Add mdio mux nodes

Message ID 20190206150520.9683-1-pankaj.bansal@nxp.com (mailing list archive)
State New, archived
Headers show
Series [v3] arm64: dts: lx2160aqds: Add mdio mux nodes | expand

Commit Message

Pankaj Bansal Feb. 6, 2019, 9:40 a.m. UTC
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(+)

Comments

Shawn Guo Feb. 11, 2019, 3 a.m. UTC | #1
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
>
Leo Li Feb. 11, 2019, 8:43 p.m. UTC | #2
> -----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
> >
Pankaj Bansal Feb. 12, 2019, 3:26 a.m. UTC | #3
> -----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
> > >
Leo Li Feb. 12, 2019, 6:01 p.m. UTC | #4
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
> > > >
Leo Li Feb. 14, 2019, 10:32 p.m. UTC | #5
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
> > > > >
Pankaj Bansal Feb. 18, 2019, 5:49 a.m. UTC | #6
> -----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
> > > > > >
diff mbox series

Patch

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";
+		};
 	};
 };