| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <VI1PR0401MB2496C24647AB1DF2EC7BA266F1630@VI1PR0401MB2496.eurprd04.prod.outlook.com>
Date: Mon, 18 Feb 2019 05:49:18 +0000
From: Pankaj Bansal <pankaj.bansal@....com>
To: Leo Li <leoyang.li@....com>, Rob Herring <robh+dt@...nel.org>
CC: Shawn Guo <shawnguo@...nel.org>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@....com]
> Sent: Friday, 15 February, 2019 04:03 AM
> To: Pankaj Bansal <pankaj.bansal@....com>; Rob Herring <robh+dt@...nel.org>
> Cc: Shawn Guo <shawnguo@...nel.org>; Andrew Lunn <andrew@...n.ch>;
> Florian Fainelli <f.fainelli@...il.com>; netdev@...r.kernel.org; linux-arm-
> kernel@...ts.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@....com> wrote:
> >
> > On Mon, Feb 11, 2019 at 9:28 PM Pankaj Bansal <pankaj.bansal@....com>
> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Leo Li
> > > > Sent: Tuesday, 12 February, 2019 02:14 AM
> > > > To: Shawn Guo <shawnguo@...nel.org>; Pankaj Bansal
> > > > <pankaj.bansal@....com>
> > > > Cc: Andrew Lunn <andrew@...n.ch>; Florian Fainelli
> > > > <f.fainelli@...il.com>; netdev@...r.kernel.org;
> > > > linux-arm-kernel@...ts.infradead.org
> > > > Subject: RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Shawn Guo <shawnguo@...nel.org>
> > > > > Sent: Sunday, February 10, 2019 9:00 PM
> > > > > To: Pankaj Bansal <pankaj.bansal@....com>
> > > > > Cc: Leo Li <leoyang.li@....com>; Andrew Lunn <andrew@...n.ch>;
> > > > > Florian Fainelli <f.fainelli@...il.com>; netdev@...r.kernel.org;
> > > > > linux-arm- kernel@...ts.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@....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@...6000 {
> > > > > > + 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@...7000 {
> > > > > > + 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
> > > > > >
Powered by blists - more mailing lists