| Message ID | 1595938400-13279-3-git-send-email-vikas.singh@puresoftware.com |
|---|---|
| State | Deferred |
| Delegated to: | David Miller |
| Headers | show |
| Series | Add fwnode helper functions to MDIO bus driver | expand |
On Tue, Jul 28, 2020 at 05:43:20PM +0530, Vikas Singh wrote: > This patch will extend "struct fixed_phy_status" by adding new > "struct device *dev" member entry in it. > This change will help to handle the fixed phy registration in > ACPI probe case for fwnodes. Hi Vikas Please could you tell us more about your use cases. It seems that using ACPI on ARM is limited to SBSA/SBSA system. It is not clear to me why you would need a fixed-link PHY on such a system. Andrew
On Wed, Jul 29, 2020 at 07:34:19PM +0530, Vikas Singh wrote: > Hi Andrew, > > I understand your concern But my use case is pretty simple and straightforward. > As you are already aware of the fact that operating systems running on standard > server hardware requires standard firmware interfaces to be present in order to > boot > and function correctly. > Here SBBR describes these firmware requirements which covers UEFI & ACPI. > Therefore I would like to provide dual boot support to such systems which means > supporting ACPI along with existing DT. > Now kernels "__fixed_phy_registe() " being a wonderful implementation catters > almost every thing required for both ACPI & DT > But fails to register fixed PHYs in case of ACPI because it has no clue of the > fwnode (of_node in case of DT) to attach with a PHY device. You failed to answer my question. Why do you need a fixed PHY? Please could you point me at your DT files so i can at least understand your hardware. Andrew
On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > Hi Andrew, > > Please refer to the "fman" node under > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > I have two 10G ethernet interfaces out of which one is of fixed-link. Please do not top post. How does XGMII (which is a 10G only interface) work at 1G speed? Is what is in DT itself a hack because fixed-phy doesn't support 10G modes?
On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux admin wrote: > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > Hi Andrew, > > > > Please refer to the "fman" node under > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > I have two 10G ethernet interfaces out of which one is of fixed-link. > > Please do not top post. > > How does XGMII (which is a 10G only interface) work at 1G speed? Is > what is in DT itself a hack because fixed-phy doesn't support 10G > modes? My gut feeling is there is some hack going on here, which is why i'm being persistent at trying to understand what is actually going on here. So Vikas, as Russell pointed out, fixed-link is limited to 1G. It seems odd you are running a 10G link at 1G. It is also unclear what you have on the other end of that fixed link? Is it an SFP and you are afraid of the work needed to get phylink working with ACPI? Is it an Ethernet switch, and you are afraid of the work needed to get DSA working with ACPI? Looking at https://www.nxp.com/docs/en/quick-reference-guide/LS1046AQRS.pdf I see a XFI/2-5G SGMII port connected to a PHY, which i guess is ethernet@f0000 { /* 10GEC1 */ phy-handle = <&aqr106_phy>; phy-connection-type = "xgmii"; }; and aqr106_phy: ethernet-phy@0 { compatible = "ethernet-phy-ieee802.3-c45"; interrupts = <0 131 4>; reg = <0x0>; }; Which leaves an XFI interface connected to a retimer and then to an SFP cage? Is this where you are using fixed-link? Andrew
> -----Original Message----- > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On > Behalf Of Andrew Lunn > Sent: 01 August 2020 18:11 > To: Russell King - ARM Linux admin <linux@armlinux.org.uk> > Cc: Vikas Singh <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > <calvin.johnson@oss.nxp.com>; kuldip dwivedi > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS) > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com> > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux admin > wrote: > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > > Hi Andrew, > > > > > > Please refer to the "fman" node under > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > > I have two 10G ethernet interfaces out of which one is of fixed-link. > > > > Please do not top post. > > > > How does XGMII (which is a 10G only interface) work at 1G speed? Is > > what is in DT itself a hack because fixed-phy doesn't support 10G > > modes? > > My gut feeling is there is some hack going on here, which is why i'm > being persistent at trying to understand what is actually going on > here. Hi Andrew, That platform used 1G fixed link there since there was no support for 10G fixed link at the time. PHYlib could have tolerated 10G speed there With a one-liner. I understand that PHYLink is working to describe this Better, but it was not there at that time. Adding the dependency on PHYLink was not desirable as most of the users for the DPAA 1 platforms were targeting kernels before the PHYLink introduction (and last I've looked, it's still under development, with unstable APIs so we'll take a look at this later, when it settles). > So Vikas, as Russell pointed out, fixed-link is limited to 1G. It > seems odd you are running a 10G link at 1G. It is also unclear what > you have on the other end of that fixed link? Is it an SFP and you are > afraid of the work needed to get phylink working with ACPI? Is it an > Ethernet switch, and you are afraid of the work needed to get DSA > working with ACPI? > > Looking at > https://www.nxp.com/docs/en/quick-reference-guide/LS1046AQRS.pdf > > I see a XFI/2-5G SGMII port connected to a PHY, which i guess is > > ethernet@f0000 { /* 10GEC1 */ > phy-handle = <&aqr106_phy>; > phy-connection-type = "xgmii"; > }; > > and > aqr106_phy: ethernet-phy@0 { > compatible = "ethernet-phy-ieee802.3-c45"; > interrupts = <0 131 4>; > reg = <0x0>; > }; > > Which leaves an XFI interface connected to a retimer and then to an > SFP cage? Is this where you are using fixed-link? > > Andrew
On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) wrote: > > -----Original Message----- > > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On > > Behalf Of Andrew Lunn > > Sent: 01 August 2020 18:11 > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > Cc: Vikas Singh <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > <calvin.johnson@oss.nxp.com>; kuldip dwivedi > > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS) > > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com> > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux admin > > wrote: > > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > > > Hi Andrew, > > > > > > > > Please refer to the "fman" node under > > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > > > I have two 10G ethernet interfaces out of which one is of fixed-link. > > > > > > Please do not top post. > > > > > > How does XGMII (which is a 10G only interface) work at 1G speed? Is > > > what is in DT itself a hack because fixed-phy doesn't support 10G > > > modes? > > > > My gut feeling is there is some hack going on here, which is why i'm > > being persistent at trying to understand what is actually going on > > here. > > Hi Andrew, > > That platform used 1G fixed link there since there was no support for > 10G fixed link at the time. PHYlib could have tolerated 10G speed there > With a one-liner. That statement is false. It is not a "one liner". fixed-phy exposes the settings to userspace as a Clause 22 PHY register set, and the Clause 22 register set does not support 10G. So, a "one liner" would just be yet another hack. Adding Clause 45 PHY emulation support would be a huge task. > I understand that PHYLink is working to describe this > Better, but it was not there at that time. Adding the dependency on > PHYLink was not desirable as most of the users for the DPAA 1 platforms > were targeting kernels before the PHYLink introduction (and last I've > looked, it's still under development, with unstable APIs so we'll > take a look at this later, when it settles). I think you need to read Documentation/process/stable-api-nonsense.rst particularly the section "Stable Kernel Source Interfaces". phylink is going to be under development for quite some time to come as requirements evolve. For example, when support for QSFP interfaces is eventually worked out, I suspect there will need to be some further changes to the driver interface. This is completely normal. Now, as to the stability of the phylink API to drivers, it has in fact been very stable - it has only changed over the course of this year to support split PCS, a necessary step for DPAA2 and a few others. It has been around in mainline for two years, and has been around much longer than that, and during that time it has been in mainline, the MAC facing interface has not changed until recently. So, I find your claim to be quite unreasonable.
> -----Original Message----- > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > Sent: 03 August 2020 12:07 > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > <vikas.singh@nxp.com> > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) wrote: > > > -----Original Message----- > > > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On > > > Behalf Of Andrew Lunn > > > Sent: 01 August 2020 18:11 > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > Cc: Vikas Singh <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > > > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > > <calvin.johnson@oss.nxp.com>; kuldip dwivedi > > > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS) > > > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com> > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed > PHY > > > > > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux > admin > > > wrote: > > > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > > > > Hi Andrew, > > > > > > > > > > Please refer to the "fman" node under > > > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > > > > I have two 10G ethernet interfaces out of which one is of fixed- > link. > > > > > > > > Please do not top post. > > > > > > > > How does XGMII (which is a 10G only interface) work at 1G speed? Is > > > > what is in DT itself a hack because fixed-phy doesn't support 10G > > > > modes? > > > > > > My gut feeling is there is some hack going on here, which is why i'm > > > being persistent at trying to understand what is actually going on > > > here. > > > > Hi Andrew, > > > > That platform used 1G fixed link there since there was no support for > > 10G fixed link at the time. PHYlib could have tolerated 10G speed there > > With a one-liner. > > That statement is false. It is not a "one liner". fixed-phy exposes > the settings to userspace as a Clause 22 PHY register set, and the > Clause 22 register set does not support 10G. So, a "one liner" would > just be yet another hack. Adding Clause 45 PHY emulation support > would be a huge task. > > > I understand that PHYLink is working to describe this > > Better, but it was not there at that time. Adding the dependency on > > PHYLink was not desirable as most of the users for the DPAA 1 platforms > > were targeting kernels before the PHYLink introduction (and last I've > > looked, it's still under development, with unstable APIs so we'll > > take a look at this later, when it settles). > > I think you need to read Documentation/process/stable-api-nonsense.rst > particularly the section "Stable Kernel Source Interfaces". > > phylink is going to be under development for quite some time to come > as requirements evolve. For example, when support for QSFP interfaces > is eventually worked out, I suspect there will need to be some further > changes to the driver interface. This is completely normal. > > Now, as to the stability of the phylink API to drivers, it has in fact > been very stable - it has only changed over the course of this year to > support split PCS, a necessary step for DPAA2 and a few others. It has > been around in mainline for two years, and has been around much longer > than that, and during that time it has been in mainline, the MAC facing > interface has not changed until recently. > > So, I find your claim to be quite unreasonable. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! I see you agree that there were and there will be many changes for a while, It's not a complaint, I know hot it works, it's just a decision based on required effort vs features offered vs user requirements. Lately it's been time consuming to try to fix things in this area. Regards Madalin
> I see you agree that there were and there will be many changes for a while, > It's not a complaint, I know hot it works, it's just a decision based on > required effort vs features offered vs user requirements. Lately it's been > time consuming to try to fix things in this area. So the conclusion to all this that you are unwilling to use the correct API for this, which would be phylink, and the SFP code. So: NACK Andrew
On Mon, Aug 03, 2020 at 11:45:55AM +0000, Madalin Bucur (OSS) wrote: > > -----Original Message----- > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > Sent: 03 August 2020 12:07 > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > <vikas.singh@nxp.com> > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > > > On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) wrote: > > > > -----Original Message----- > > > > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On > > > > Behalf Of Andrew Lunn > > > > Sent: 01 August 2020 18:11 > > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > > Cc: Vikas Singh <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > > > > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > > > <calvin.johnson@oss.nxp.com>; kuldip dwivedi > > > > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS) > > > > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com> > > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed > > PHY > > > > > > > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux > > admin > > > > wrote: > > > > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > > > > > Hi Andrew, > > > > > > > > > > > > Please refer to the "fman" node under > > > > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > > > > > I have two 10G ethernet interfaces out of which one is of fixed- > > link. > > > > > > > > > > Please do not top post. > > > > > > > > > > How does XGMII (which is a 10G only interface) work at 1G speed? Is > > > > > what is in DT itself a hack because fixed-phy doesn't support 10G > > > > > modes? > > > > > > > > My gut feeling is there is some hack going on here, which is why i'm > > > > being persistent at trying to understand what is actually going on > > > > here. > > > > > > Hi Andrew, > > > > > > That platform used 1G fixed link there since there was no support for > > > 10G fixed link at the time. PHYlib could have tolerated 10G speed there > > > With a one-liner. > > > > That statement is false. It is not a "one liner". fixed-phy exposes > > the settings to userspace as a Clause 22 PHY register set, and the > > Clause 22 register set does not support 10G. So, a "one liner" would > > just be yet another hack. Adding Clause 45 PHY emulation support > > would be a huge task. > > > > > I understand that PHYLink is working to describe this > > > Better, but it was not there at that time. Adding the dependency on > > > PHYLink was not desirable as most of the users for the DPAA 1 platforms > > > were targeting kernels before the PHYLink introduction (and last I've > > > looked, it's still under development, with unstable APIs so we'll > > > take a look at this later, when it settles). > > > > I think you need to read Documentation/process/stable-api-nonsense.rst > > particularly the section "Stable Kernel Source Interfaces". > > > > phylink is going to be under development for quite some time to come > > as requirements evolve. For example, when support for QSFP interfaces > > is eventually worked out, I suspect there will need to be some further > > changes to the driver interface. This is completely normal. > > > > Now, as to the stability of the phylink API to drivers, it has in fact > > been very stable - it has only changed over the course of this year to > > support split PCS, a necessary step for DPAA2 and a few others. It has > > been around in mainline for two years, and has been around much longer > > than that, and during that time it has been in mainline, the MAC facing > > interface has not changed until recently. > > > > So, I find your claim to be quite unreasonable. > > I see you agree that there were and there will be many changes for a while, > It's not a complaint, I know hot it works, it's just a decision based on > required effort vs features offered vs user requirements. Lately it's been > time consuming to try to fix things in this area. No, it hasn't been time consuming. The only API changes as far as drivers are concerned have been: 1. the change to the mac_link_up() prototype to move the setup of the final link parameters out of mac_config() - and almost all of the updates to users were done by me. 2. the addition of split PCS support, introducing new interfaces, has had minimal impact on those drivers that updated in step (1). There have been no other changes as far as users are concerned. Some of the difficulty with (1) has been that users of phylink appeared initially with no proper review, and consequently they got quite a lot wrong. The most common error has been using state->speed, state->duplex in mac_config() methods irrespective of the AN mode, which has _always_ since before phylink was merged into mainline, been totally unreliable. That leads me on to the other visible "changes" for users are concerned, which may be interpreted as interface changes, but are not; they have all been clarifications to the documentation, to strengthen things such as "do not use state->speed and state->duplex in mac_config() for various specific AN modes". Nothing has actually changed with any of those clarifications. For example, if in in-band mode, and mac_config() uses state->speed and state->duplex, then it doesn't matter which version of phylink you're using, if someone issues ethtool -s ethN ..., then state->speed and state->duplex will be their respective UNKNOWN values, and if you're using these in that situation, you will mis-program the MAC. Again, that is not something that has changed. Ever. But the documentation has because people just don't seem to get it, and I seemed to be constantly repeating myself in review after review on the same points. So, your assertion that the phylink API is not stable is false. It has been remarkably stable over the two years that it has been around. It is only natural that as the technology that a piece of code supports evolves, so the code evolves with it. That is exactly what has happened this year with the two changes I mention above. Now, if you've found it time consuming to "fix things" (unspecified what "things" are) then I assert that what has needed to be fixed are things that NXP have got wrong. Such as the rtnl cockups. Such as abusing state->speed and state->duplex. None of that is because the interface is unstable - they are down to buggy implementation on NXPs part. Essentially, what I'm saying is that your attempt to paint phylink as being painful on the basis of interface changes is totally and utterly wrong and is just an excuse to justify abusing the fixed-link code and specifying things that are clearly incorrect via DT. I will accept that the interface that had existed up until the mac_link_up() change was confusing - it clearly was due to the number of people getting mac_config() implementations wrong. That is actually another of the reasons why the mac_link_up() change was made. These problems are _only_ found by people making use of it. If people don't use stuff, then problems aren't found, and nothing changes. So, I think you can expect a NAK for the patch at the top of this thread on the basis that it is perpetuating an abuse not only the legacy fixed-link code, but also DT. However, I will leave Andrew to make that call.
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: 03 August 2020 15:58 > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>; Vikas Singh > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > <vikas.singh@nxp.com> > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > > I see you agree that there were and there will be many changes for a > while, > > It's not a complaint, I know hot it works, it's just a decision based on > > required effort vs features offered vs user requirements. Lately it's > been > > time consuming to try to fix things in this area. > > So the conclusion to all this that you are unwilling to use the > correct API for this, which would be phylink, and the SFP code. So: > > NACK > > Andrew You've rejected a generic change - ACPI support for fixed link. The discussion above is just an example of how it could have been (mis-)used. Are you rejecting the general case or just the particular one? Madalin
> -----Original Message----- > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > Sent: 03 August 2020 17:10 > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > <vikas.singh@nxp.com> > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > On Mon, Aug 03, 2020 at 11:45:55AM +0000, Madalin Bucur (OSS) wrote: > > > -----Original Message----- > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > Sent: 03 August 2020 12:07 > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > hkallweit1@gmail.com; > > > netdev@vger.kernel.org; Calvin Johnson (OSS) > <calvin.johnson@oss.nxp.com>; > > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > > <vikas.singh@nxp.com> > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed > PHY > > > > > > On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) wrote: > > > > > -----Original Message----- > > > > > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> > On > > > > > Behalf Of Andrew Lunn > > > > > Sent: 01 August 2020 18:11 > > > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > > > Cc: Vikas Singh <vikas.singh@puresoftware.com>; > f.fainelli@gmail.com; > > > > > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > > > > <calvin.johnson@oss.nxp.com>; kuldip dwivedi > > > > > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS) > > > > > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com> > > > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with > fixed > > > PHY > > > > > > > > > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux > > > admin > > > > > wrote: > > > > > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > > > > > > Hi Andrew, > > > > > > > > > > > > > > Please refer to the "fman" node under > > > > > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > > > > > > I have two 10G ethernet interfaces out of which one is of > fixed- > > > link. > > > > > > > > > > > > Please do not top post. > > > > > > > > > > > > How does XGMII (which is a 10G only interface) work at 1G speed? > Is > > > > > > what is in DT itself a hack because fixed-phy doesn't support > 10G > > > > > > modes? > > > > > > > > > > My gut feeling is there is some hack going on here, which is why > i'm > > > > > being persistent at trying to understand what is actually going on > > > > > here. > > > > > > > > Hi Andrew, > > > > > > > > That platform used 1G fixed link there since there was no support > for > > > > 10G fixed link at the time. PHYlib could have tolerated 10G speed > there > > > > With a one-liner. > > > > > > That statement is false. It is not a "one liner". fixed-phy exposes > > > the settings to userspace as a Clause 22 PHY register set, and the > > > Clause 22 register set does not support 10G. So, a "one liner" would > > > just be yet another hack. Adding Clause 45 PHY emulation support > > > would be a huge task. > > > > > > > I understand that PHYLink is working to describe this > > > > Better, but it was not there at that time. Adding the dependency on > > > > PHYLink was not desirable as most of the users for the DPAA 1 > platforms > > > > were targeting kernels before the PHYLink introduction (and last > I've > > > > looked, it's still under development, with unstable APIs so we'll > > > > take a look at this later, when it settles). > > > > > > I think you need to read Documentation/process/stable-api-nonsense.rst > > > particularly the section "Stable Kernel Source Interfaces". > > > > > > phylink is going to be under development for quite some time to come > > > as requirements evolve. For example, when support for QSFP interfaces > > > is eventually worked out, I suspect there will need to be some further > > > changes to the driver interface. This is completely normal. > > > > > > Now, as to the stability of the phylink API to drivers, it has in fact > > > been very stable - it has only changed over the course of this year to > > > support split PCS, a necessary step for DPAA2 and a few others. It > has > > > been around in mainline for two years, and has been around much longer > > > than that, and during that time it has been in mainline, the MAC > facing > > > interface has not changed until recently. > > > > > > So, I find your claim to be quite unreasonable. > > > > I see you agree that there were and there will be many changes for a > while, > > It's not a complaint, I know hot it works, it's just a decision based on > > required effort vs features offered vs user requirements. Lately it's > been > > time consuming to try to fix things in this area. > > No, it hasn't been time consuming. The only API changes as far as > drivers are concerned have been: > > 1. the change to the mac_link_up() prototype to move the setup of the > final link parameters out of mac_config() - and almost all of the > updates to users were done by me. > > 2. the addition of split PCS support, introducing new interfaces, has > had minimal impact on those drivers that updated in step (1). > > There have been no other changes as far as users are concerned. > > Some of the difficulty with (1) has been that users of phylink appeared > initially with no proper review, and consequently they got quite a lot > wrong. The most common error has been using state->speed, state->duplex > in mac_config() methods irrespective of the AN mode, which has _always_ > since before phylink was merged into mainline, been totally unreliable. > > That leads me on to the other visible "changes" for users are concerned, > which may be interpreted as interface changes, but are not; they have > all been clarifications to the documentation, to strengthen things such > as "do not use state->speed and state->duplex in mac_config() for > various specific AN modes". Nothing has actually changed with any of > those clarifications. > > For example, if in in-band mode, and mac_config() uses state->speed > and state->duplex, then it doesn't matter which version of phylink > you're using, if someone issues ethtool -s ethN ..., then state->speed > and state->duplex will be their respective UNKNOWN values, and if you're > using these in that situation, you will mis-program the MAC. > > Again, that is not something that has changed. Ever. But the > documentation has because people just don't seem to get it, and I seemed > to be constantly repeating myself in review after review on the same > points. > > So, your assertion that the phylink API is not stable is false. It > has been remarkably stable over the two years that it has been around. > It is only natural that as the technology that a piece of code supports > evolves, so the code evolves with it. That is exactly what has happened > this year with the two changes I mention above. > > Now, if you've found it time consuming to "fix things" (unspecified what > "things" are) then I assert that what has needed to be fixed are things > that NXP have got wrong. Such as the rtnl cockups. Such as abusing > state->speed and state->duplex. None of that is because the interface > is unstable - they are down to buggy implementation on NXPs part. > > Essentially, what I'm saying is that your attempt to paint phylink as > being painful on the basis of interface changes is totally and utterly > wrong and is just an excuse to justify abusing the fixed-link code and > specifying things that are clearly incorrect via DT. Thank you for the distilled phylink history, it may be easier to comprehend with these details. I was not referring to phylink, but PHY related issues on the DPAA 1 platforms. > I will accept that the interface that had existed up until the > mac_link_up() change was confusing - it clearly was due to the number > of people getting mac_config() implementations wrong. That is actually > another of the reasons why the mac_link_up() change was made. These > problems are _only_ found by people making use of it. If people don't > use stuff, then problems aren't found, and nothing changes. > > So, I think you can expect a NAK for the patch at the top of this > thread on the basis that it is perpetuating an abuse not only the > legacy fixed-link code, but also DT. However, I will leave Andrew to > make that call. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Aug 03, 2020 at 02:33:56PM +0000, Madalin Bucur (OSS) wrote: > > -----Original Message----- > > From: Andrew Lunn <andrew@lunn.ch> > > Sent: 03 August 2020 15:58 > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>; Vikas Singh > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > <vikas.singh@nxp.com> > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > > > > I see you agree that there were and there will be many changes for a > > while, > > > It's not a complaint, I know hot it works, it's just a decision based on > > > required effort vs features offered vs user requirements. Lately it's > > been > > > time consuming to try to fix things in this area. > > > > So the conclusion to all this that you are unwilling to use the > > correct API for this, which would be phylink, and the SFP code. So: > > > > NACK > > > > Andrew > > You've rejected a generic change - ACPI support for fixed link. > The discussion above is just an example of how it could have been (mis-)used. > Are you rejecting the general case or just the particular one? So far, nobody has corrected me that the MAC is not connected to an SFP socket. So i see two sorts of abuse going on here: 1) Using a fixed link with a hack to allow 10G. phylink allows 10G fixed links without an hacks. 2) Using a fixed link when not even appropriate since phylink should be used to control the SFP. Now, you can do whatever you want in your Vendor Crap tree. But there is no reason mainline should help you support your vendor crap tree. To make progress here, you need to add an in tree user of this generic change. And since that means an ACPI user, you need to follow what has been set out in the other thread. You need an ACPI maintainer to ACK it. And to get an ACPI maintainer to ACK it, you need a specification, and proof it is being used. And to get my ACK, it needs to be valid use of it as well. Andrew
On Mon, Aug 03, 2020 at 02:47:41PM +0000, Madalin Bucur (OSS) wrote: > > -----Original Message----- > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > Sent: 03 August 2020 17:10 > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > <vikas.singh@nxp.com> > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > > > On Mon, Aug 03, 2020 at 11:45:55AM +0000, Madalin Bucur (OSS) wrote: > > > > -----Original Message----- > > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > > Sent: 03 August 2020 12:07 > > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > > > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > > > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > > hkallweit1@gmail.com; > > > > netdev@vger.kernel.org; Calvin Johnson (OSS) > > <calvin.johnson@oss.nxp.com>; > > > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > > > <vikas.singh@nxp.com> > > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed > > PHY > > > > > > > > On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) wrote: > > > > > > -----Original Message----- > > > > > > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> > > On > > > > > > Behalf Of Andrew Lunn > > > > > > Sent: 01 August 2020 18:11 > > > > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > > > > Cc: Vikas Singh <vikas.singh@puresoftware.com>; > > f.fainelli@gmail.com; > > > > > > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > > > > > <calvin.johnson@oss.nxp.com>; kuldip dwivedi > > > > > > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS) > > > > > > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com> > > > > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with > > fixed > > > > PHY > > > > > > > > > > > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux > > > > admin > > > > > > wrote: > > > > > > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > > > > > > > Hi Andrew, > > > > > > > > > > > > > > > > Please refer to the "fman" node under > > > > > > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > > > > > > > I have two 10G ethernet interfaces out of which one is of > > fixed- > > > > link. > > > > > > > > > > > > > > Please do not top post. > > > > > > > > > > > > > > How does XGMII (which is a 10G only interface) work at 1G speed? > > Is > > > > > > > what is in DT itself a hack because fixed-phy doesn't support > > 10G > > > > > > > modes? > > > > > > > > > > > > My gut feeling is there is some hack going on here, which is why > > i'm > > > > > > being persistent at trying to understand what is actually going on > > > > > > here. > > > > > > > > > > Hi Andrew, > > > > > > > > > > That platform used 1G fixed link there since there was no support > > for > > > > > 10G fixed link at the time. PHYlib could have tolerated 10G speed > > there > > > > > With a one-liner. > > > > > > > > That statement is false. It is not a "one liner". fixed-phy exposes > > > > the settings to userspace as a Clause 22 PHY register set, and the > > > > Clause 22 register set does not support 10G. So, a "one liner" would > > > > just be yet another hack. Adding Clause 45 PHY emulation support > > > > would be a huge task. > > > > > > > > > I understand that PHYLink is working to describe this > > > > > Better, but it was not there at that time. Adding the dependency on > > > > > PHYLink was not desirable as most of the users for the DPAA 1 > > platforms > > > > > were targeting kernels before the PHYLink introduction (and last > > I've > > > > > looked, it's still under development, with unstable APIs so we'll > > > > > take a look at this later, when it settles). > > > > > > > > I think you need to read Documentation/process/stable-api-nonsense.rst > > > > particularly the section "Stable Kernel Source Interfaces". > > > > > > > > phylink is going to be under development for quite some time to come > > > > as requirements evolve. For example, when support for QSFP interfaces > > > > is eventually worked out, I suspect there will need to be some further > > > > changes to the driver interface. This is completely normal. > > > > > > > > Now, as to the stability of the phylink API to drivers, it has in fact > > > > been very stable - it has only changed over the course of this year to > > > > support split PCS, a necessary step for DPAA2 and a few others. It > > has > > > > been around in mainline for two years, and has been around much longer > > > > than that, and during that time it has been in mainline, the MAC > > facing > > > > interface has not changed until recently. > > > > > > > > So, I find your claim to be quite unreasonable. > > > > > > I see you agree that there were and there will be many changes for a > > while, > > > It's not a complaint, I know hot it works, it's just a decision based on > > > required effort vs features offered vs user requirements. Lately it's > > been > > > time consuming to try to fix things in this area. > > > > No, it hasn't been time consuming. The only API changes as far as > > drivers are concerned have been: > > > > 1. the change to the mac_link_up() prototype to move the setup of the > > final link parameters out of mac_config() - and almost all of the > > updates to users were done by me. > > > > 2. the addition of split PCS support, introducing new interfaces, has > > had minimal impact on those drivers that updated in step (1). > > > > There have been no other changes as far as users are concerned. > > > > Some of the difficulty with (1) has been that users of phylink appeared > > initially with no proper review, and consequently they got quite a lot > > wrong. The most common error has been using state->speed, state->duplex > > in mac_config() methods irrespective of the AN mode, which has _always_ > > since before phylink was merged into mainline, been totally unreliable. > > > > That leads me on to the other visible "changes" for users are concerned, > > which may be interpreted as interface changes, but are not; they have > > all been clarifications to the documentation, to strengthen things such > > as "do not use state->speed and state->duplex in mac_config() for > > various specific AN modes". Nothing has actually changed with any of > > those clarifications. > > > > For example, if in in-band mode, and mac_config() uses state->speed > > and state->duplex, then it doesn't matter which version of phylink > > you're using, if someone issues ethtool -s ethN ..., then state->speed > > and state->duplex will be their respective UNKNOWN values, and if you're > > using these in that situation, you will mis-program the MAC. > > > > Again, that is not something that has changed. Ever. But the > > documentation has because people just don't seem to get it, and I seemed > > to be constantly repeating myself in review after review on the same > > points. > > > > So, your assertion that the phylink API is not stable is false. It > > has been remarkably stable over the two years that it has been around. > > It is only natural that as the technology that a piece of code supports > > evolves, so the code evolves with it. That is exactly what has happened > > this year with the two changes I mention above. > > > > Now, if you've found it time consuming to "fix things" (unspecified what > > "things" are) then I assert that what has needed to be fixed are things > > that NXP have got wrong. Such as the rtnl cockups. Such as abusing > > state->speed and state->duplex. None of that is because the interface > > is unstable - they are down to buggy implementation on NXPs part. > > > > Essentially, what I'm saying is that your attempt to paint phylink as > > being painful on the basis of interface changes is totally and utterly > > wrong and is just an excuse to justify abusing the fixed-link code and > > specifying things that are clearly incorrect via DT. > > Thank you for the distilled phylink history, it may be easier to comprehend > with these details. I was not referring to phylink, but PHY related issues > on the DPAA 1 platforms. Sigh. No, you were referring to phylink. This is what you said: > I understand that PHYLink is working to describe this > Better, but it was not there at that time. Adding the dependency on > PHYLink was not desirable as most of the users for the DPAA 1 platforms > were targeting kernels before the PHYLink introduction (and last I've > looked, it's still under development, with unstable APIs so we'll > take a look at this later, when it settles). This discussion stems from your misconception and incorrect statements concerning phylink, which I am correcting in this discussion.
On Mon, Aug 03, 2020 at 05:00:51PM +0200, Andrew Lunn wrote: > On Mon, Aug 03, 2020 at 02:33:56PM +0000, Madalin Bucur (OSS) wrote: > > > -----Original Message----- > > > From: Andrew Lunn <andrew@lunn.ch> > > > Sent: 03 August 2020 15:58 > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>; Vikas Singh > > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > > > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > > <vikas.singh@nxp.com> > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > > > > > > I see you agree that there were and there will be many changes for a > > > while, > > > > It's not a complaint, I know hot it works, it's just a decision based on > > > > required effort vs features offered vs user requirements. Lately it's > > > been > > > > time consuming to try to fix things in this area. > > > > > > So the conclusion to all this that you are unwilling to use the > > > correct API for this, which would be phylink, and the SFP code. So: > > > > > > NACK > > > > > > Andrew > > > > You've rejected a generic change - ACPI support for fixed link. > > The discussion above is just an example of how it could have been (mis-)used. > > Are you rejecting the general case or just the particular one? > > So far, nobody has corrected me that the MAC is not connected to an > SFP socket. So i see two sorts of abuse going on here: > > 1) Using a fixed link with a hack to allow 10G. phylink allows 10G > fixed links without an hacks. > > 2) Using a fixed link when not even appropriate since phylink should > be used to control the SFP. > > Now, you can do whatever you want in your Vendor Crap tree. But there > is no reason mainline should help you support your vendor crap tree. +1 for everything above.
> -----Original Message----- > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > Sent: 03 August 2020 19:54 > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com; > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>; > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > <vikas.singh@nxp.com> > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY > > On Mon, Aug 03, 2020 at 02:47:41PM +0000, Madalin Bucur (OSS) wrote: > > > -----Original Message----- > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > Sent: 03 August 2020 17:10 > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > hkallweit1@gmail.com; > > > netdev@vger.kernel.org; Calvin Johnson (OSS) > <calvin.johnson@oss.nxp.com>; > > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > > <vikas.singh@nxp.com> > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed > PHY > > > > > > On Mon, Aug 03, 2020 at 11:45:55AM +0000, Madalin Bucur (OSS) wrote: > > > > > -----Original Message----- > > > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > > > Sent: 03 August 2020 12:07 > > > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com> > > > > > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh > > > > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; > > > hkallweit1@gmail.com; > > > > > netdev@vger.kernel.org; Calvin Johnson (OSS) > > > <calvin.johnson@oss.nxp.com>; > > > > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh > > > > > <vikas.singh@nxp.com> > > > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with > fixed > > > PHY > > > > > > > > > > On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) > wrote: > > > > > > > -----Original Message----- > > > > > > > From: netdev-owner@vger.kernel.org <netdev- > owner@vger.kernel.org> > > > On > > > > > > > Behalf Of Andrew Lunn > > > > > > > Sent: 01 August 2020 18:11 > > > > > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > > > > > > Cc: Vikas Singh <vikas.singh@puresoftware.com>; > > > f.fainelli@gmail.com; > > > > > > > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson > (OSS) > > > > > > > <calvin.johnson@oss.nxp.com>; kuldip dwivedi > > > > > > > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS) > > > > > > > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com> > > > > > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with > > > fixed > > > > > PHY > > > > > > > > > > > > > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM > Linux > > > > > admin > > > > > > > wrote: > > > > > > > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote: > > > > > > > > > Hi Andrew, > > > > > > > > > > > > > > > > > > Please refer to the "fman" node under > > > > > > > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > > > > > > > > > I have two 10G ethernet interfaces out of which one is of > > > fixed- > > > > > link. > > > > > > > > > > > > > > > > Please do not top post. > > > > > > > > > > > > > > > > How does XGMII (which is a 10G only interface) work at 1G > speed? > > > Is > > > > > > > > what is in DT itself a hack because fixed-phy doesn't > support > > > 10G > > > > > > > > modes? > > > > > > > > > > > > > > My gut feeling is there is some hack going on here, which is > why > > > i'm > > > > > > > being persistent at trying to understand what is actually > going on > > > > > > > here. > > > > > > > > > > > > Hi Andrew, > > > > > > > > > > > > That platform used 1G fixed link there since there was no > support > > > for > > > > > > 10G fixed link at the time. PHYlib could have tolerated 10G > speed > > > there > > > > > > With a one-liner. > > > > > > > > > > That statement is false. It is not a "one liner". fixed-phy > exposes > > > > > the settings to userspace as a Clause 22 PHY register set, and the > > > > > Clause 22 register set does not support 10G. So, a "one liner" > would > > > > > just be yet another hack. Adding Clause 45 PHY emulation support > > > > > would be a huge task. > > > > > > > > > > > I understand that PHYLink is working to describe this > > > > > > Better, but it was not there at that time. Adding the dependency > on > > > > > > PHYLink was not desirable as most of the users for the DPAA 1 > > > platforms > > > > > > were targeting kernels before the PHYLink introduction (and last > > > I've > > > > > > looked, it's still under development, with unstable APIs so > we'll > > > > > > take a look at this later, when it settles). > > > > > > > > > > I think you need to read Documentation/process/stable-api- > nonsense.rst > > > > > particularly the section "Stable Kernel Source Interfaces". > > > > > > > > > > phylink is going to be under development for quite some time to > come > > > > > as requirements evolve. For example, when support for QSFP > interfaces > > > > > is eventually worked out, I suspect there will need to be some > further > > > > > changes to the driver interface. This is completely normal. > > > > > > > > > > Now, as to the stability of the phylink API to drivers, it has in > fact > > > > > been very stable - it has only changed over the course of this > year to > > > > > support split PCS, a necessary step for DPAA2 and a few others. > It > > > has > > > > > been around in mainline for two years, and has been around much > longer > > > > > than that, and during that time it has been in mainline, the MAC > > > facing > > > > > interface has not changed until recently. > > > > > > > > > > So, I find your claim to be quite unreasonable. > > > > > > > > I see you agree that there were and there will be many changes for a > > > while, > > > > It's not a complaint, I know hot it works, it's just a decision > based on > > > > required effort vs features offered vs user requirements. Lately > it's > > > been > > > > time consuming to try to fix things in this area. > > > > > > No, it hasn't been time consuming. The only API changes as far as > > > drivers are concerned have been: > > > > > > 1. the change to the mac_link_up() prototype to move the setup of the > > > final link parameters out of mac_config() - and almost all of the > > > updates to users were done by me. > > > > > > 2. the addition of split PCS support, introducing new interfaces, has > > > had minimal impact on those drivers that updated in step (1). > > > > > > There have been no other changes as far as users are concerned. > > > > > > Some of the difficulty with (1) has been that users of phylink > appeared > > > initially with no proper review, and consequently they got quite a lot > > > wrong. The most common error has been using state->speed, state- > >duplex > > > in mac_config() methods irrespective of the AN mode, which has > _always_ > > > since before phylink was merged into mainline, been totally unreliable. > > > > > > That leads me on to the other visible "changes" for users are > concerned, > > > which may be interpreted as interface changes, but are not; they have > > > all been clarifications to the documentation, to strengthen things > such > > > as "do not use state->speed and state->duplex in mac_config() for > > > various specific AN modes". Nothing has actually changed with any of > > > those clarifications. > > > > > > For example, if in in-band mode, and mac_config() uses state->speed > > > and state->duplex, then it doesn't matter which version of phylink > > > you're using, if someone issues ethtool -s ethN ..., then state->speed > > > and state->duplex will be their respective UNKNOWN values, and if > you're > > > using these in that situation, you will mis-program the MAC. > > > > > > Again, that is not something that has changed. Ever. But the > > > documentation has because people just don't seem to get it, and I > seemed > > > to be constantly repeating myself in review after review on the same > > > points. > > > > > > So, your assertion that the phylink API is not stable is false. It > > > has been remarkably stable over the two years that it has been around. > > > It is only natural that as the technology that a piece of code > supports > > > evolves, so the code evolves with it. That is exactly what has > happened > > > this year with the two changes I mention above. > > > > > > Now, if you've found it time consuming to "fix things" (unspecified > what > > > "things" are) then I assert that what has needed to be fixed are > things > > > that NXP have got wrong. Such as the rtnl cockups. Such as abusing > > > state->speed and state->duplex. None of that is because the interface > > > is unstable - they are down to buggy implementation on NXPs part. > > > > > > Essentially, what I'm saying is that your attempt to paint phylink as > > > being painful on the basis of interface changes is totally and utterly > > > wrong and is just an excuse to justify abusing the fixed-link code and > > > specifying things that are clearly incorrect via DT. > > > > Thank you for the distilled phylink history, it may be easier to > comprehend > > with these details. I was not referring to phylink, but PHY related > issues > > on the DPAA 1 platforms. > > Sigh. No, you were referring to phylink. This is what you said: > > > I understand that PHYLink is working to describe this > > Better, but it was not there at that time. Adding the dependency on > > PHYLink was not desirable as most of the users for the DPAA 1 platforms > > were targeting kernels before the PHYLink introduction (and last I've > > looked, it's still under development, with unstable APIs so we'll > > take a look at this later, when it settles). > > This discussion stems from your misconception and incorrect statements > concerning phylink, which I am correcting in this discussion. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! "Lately it's been time consuming to try to fix things in this area." area != phylink Otherwise, yes, I may have some misconceptions in regards to phylink, not having paid the time to understand the details. Madalin
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index c4641b1..011c033 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -267,6 +267,8 @@ static struct phy_device *__fixed_phy_register(unsigned int irq, phy->duplex = status->duplex; phy->pause = status->pause; phy->asym_pause = status->asym_pause; + if (!np) + phy->mdio.dev.fwnode = status->dev->fwnode; } of_node_get(np); diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h index 52bc8e4..155fea6 100644 --- a/include/linux/phy_fixed.h +++ b/include/linux/phy_fixed.h @@ -8,6 +8,8 @@ struct fixed_phy_status { int duplex; int pause; int asym_pause; + /* Associated device node */ + struct device *dev; }; struct device_node;
This patch will extend "struct fixed_phy_status" by adding new "struct device *dev" member entry in it. This change will help to handle the fixed phy registration in ACPI probe case for fwnodes. Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com> --- drivers/net/phy/fixed_phy.c | 2 ++ include/linux/phy_fixed.h | 2 ++ 2 files changed, 4 insertions(+)