From: Pankaj Bansal <pankaj.bansal@nxp.com> To: Leo Li <leoyang.li@nxp.com>, Andrew Lunn <andrew@lunn.ch> Cc: Shawn Guo <shawnguo@kernel.org>, Florian Fainelli <f.fainelli@gmail.com>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Rob Herring <robh+dt@kernel.org> Subject: RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes Date: Thu, 7 Feb 2019 04:42:37 +0000 [thread overview] Message-ID: <VI1PR0401MB24966218B333880762D931B5F1680@VI1PR0401MB2496.eurprd04.prod.outlook.com> (raw) In-Reply-To: <CADRPPNRFfP1UHSF+99LJ_6QMCFeXi=QhmNJAhAtBPZ_pJ-jmgg@mail.gmail.com> > -----Original Message----- > From: Li Yang [mailto:leoyang.li@nxp.com] > Sent: Thursday, 7 February, 2019 05:09 AM > To: Andrew Lunn <andrew@lunn.ch> > Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>; > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Rob Herring > <robh+dt@kernel.org> > Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes > > On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@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
WARNING: multiple messages have this Message-ID (diff)
From: Pankaj Bansal <pankaj.bansal@nxp.com> To: Leo Li <leoyang.li@nxp.com>, Andrew Lunn <andrew@lunn.ch> Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>, Florian Fainelli <f.fainelli@gmail.com>, Shawn Guo <shawnguo@kernel.org>, Rob Herring <robh+dt@kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: RE: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes Date: Thu, 7 Feb 2019 04:42:37 +0000 [thread overview] Message-ID: <VI1PR0401MB24966218B333880762D931B5F1680@VI1PR0401MB2496.eurprd04.prod.outlook.com> (raw) In-Reply-To: <CADRPPNRFfP1UHSF+99LJ_6QMCFeXi=QhmNJAhAtBPZ_pJ-jmgg@mail.gmail.com> > -----Original Message----- > From: Li Yang [mailto:leoyang.li@nxp.com] > Sent: Thursday, 7 February, 2019 05:09 AM > To: Andrew Lunn <andrew@lunn.ch> > Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Shawn Guo > <shawnguo@kernel.org>; Florian Fainelli <f.fainelli@gmail.com>; > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Rob Herring > <robh+dt@kernel.org> > Subject: Re: [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes > > On Wed, Feb 6, 2019 at 3:46 PM Andrew Lunn <andrew@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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-02-07 4:43 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-04 8:51 [PATCH v2] arm64: dts: lx2160aqds: Add mdio mux nodes Pankaj Bansal 2019-02-04 8:51 ` Pankaj Bansal 2019-02-04 16:50 ` David Miller 2019-02-04 16:50 ` David Miller 2019-02-05 12:23 ` Pankaj Bansal 2019-02-05 12:23 ` Pankaj Bansal 2019-02-05 18:38 ` Li Yang 2019-02-05 18:38 ` Li Yang 2019-02-05 18:37 ` Li Yang 2019-02-05 18:37 ` Li Yang 2019-02-06 4:01 ` Pankaj Bansal 2019-02-06 4:01 ` Pankaj Bansal 2019-02-06 21:17 ` Leo Li 2019-02-06 21:17 ` Leo Li 2019-02-06 21:44 ` Andrew Lunn 2019-02-06 21:44 ` Andrew Lunn 2019-02-06 23:39 ` Li Yang 2019-02-06 23:39 ` Li Yang 2019-02-07 4:42 ` Pankaj Bansal [this message] 2019-02-07 4:42 ` Pankaj Bansal 2019-02-07 23:24 ` Li Yang 2019-02-07 23:24 ` Li Yang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=VI1PR0401MB24966218B333880762D931B5F1680@VI1PR0401MB2496.eurprd04.prod.outlook.com \ --to=pankaj.bansal@nxp.com \ --cc=andrew@lunn.ch \ --cc=f.fainelli@gmail.com \ --cc=leoyang.li@nxp.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=netdev@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=shawnguo@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.