netdev - Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Feb 2019 17:39:27 -0600
From:   Li Yang <leoyang.li@....com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Pankaj Bansal <pankaj.bansal@....com>,
        Shawn Guo <shawnguo@...nel.org>,
        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>,
        Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes

On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@...n.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