> -----Original Message----- > From: Li Yang [mailto:leoyang...@nxp.com] > Sent: Thursday, 7 February, 2019 05:09 AM > To: Andrew Lunn <and...@lunn.ch> > Cc: Pankaj Bansal <pankaj.ban...@nxp.com>; Shawn Guo > <shawn...@kernel.org>; Florian Fainelli <f.faine...@gmail.com>; > netdev@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Rob Herring > <robh...@kernel.org> > Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes > > On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <and...@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
