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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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