FEC unbind/bind feature

FEC unbind/bind feature

[Joakim Zhang]

Hi Andrew, Florian, Heiner

You are all Ethernet MDIO bus and PHY experts, I have some questions may need your help, thanks a lot in advance.

For many board designs, if it has dual MAC instances, they always share one MDIO bus to save PINs. Such as, i.MX6UL EVK board:

&fec1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet1>;
	phy-mode = "rmii";
	phy-handle = <&ethphy0>;
	phy-supply = <&reg_peri_3v3>;
	status = "okay";
};

&fec2 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet2>;
	phy-mode = "rmii";
	phy-handle = <&ethphy1>;
	phy-supply = <&reg_peri_3v3>;
	status = "okay";

	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		ethphy0: ethernet-phy@2 {
			compatible = "ethernet-phy-id0022.1560";
			reg = <2>;
			micrel,led-mode = <1>;
			clocks = <&clks IMX6UL_CLK_ENET_REF>;
			clock-names = "rmii-ref";

		};

		ethphy1: ethernet-phy@1 {
			compatible = "ethernet-phy-id0022.1560";
			reg = <1>;
			micrel,led-mode = <1>;
			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
			clock-names = "rmii-ref";
		};
	};
};

For FEC driver now, there is a patch from Fabio to prevent unbind/bind feature since dual FEC controllers share one MDIO bus. (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/net/ethernet/freescale/fec_main.c?h=next-20210324&id=272bb0e9e8cdc76e04baeefa0cd43019daa0841b)
If we unbind fec2 and then fec1 can't work since MDIO bus is controlled by FEC1, FEC2 can't use it independently.

My question is that if we want to implement unbind/bind feature, what need we do? It seems to abstract an independent MDIO bus for dual FEC instances. I look at the MDIO dt bindings, it seems support such case as it has "reg" property. (Documentation/devicetree/bindings/net/mdio.yaml)

Is there any implements existing in the Linux kernel for a reference? From your opinions, do you think it is necessary to improve it?

Best Regards,
Joakim Zhang

Re: FEC unbind/bind feature

[Andrew Lunn]

On Thu, Mar 25, 2021 at 08:04:58AM +0000, Joakim Zhang wrote:
> 
> Hi Andrew, Florian, Heiner
> 
> You are all Ethernet MDIO bus and PHY experts, I have some questions may need your help, thanks a lot in advance.
> 
> For many board designs, if it has dual MAC instances, they always share one MDIO bus to save PINs. Such as, i.MX6UL EVK board:

Please wrap your lines at around 75 characters. Standard netiquette
rules for emails apply to all Linux lists.

> 
> &fec1 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&pinctrl_enet1>;
> 	phy-mode = "rmii";
> 	phy-handle = <&ethphy0>;
> 	phy-supply = <&reg_peri_3v3>;
> 	status = "okay";
> };
> 
> &fec2 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&pinctrl_enet2>;
> 	phy-mode = "rmii";
> 	phy-handle = <&ethphy1>;
> 	phy-supply = <&reg_peri_3v3>;
> 	status = "okay";
> 
> 	mdio {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		ethphy0: ethernet-phy@2 {
> 			compatible = "ethernet-phy-id0022.1560";
> 			reg = <2>;
> 			micrel,led-mode = <1>;
> 			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> 			clock-names = "rmii-ref";
> 
> 		};
> 
> 		ethphy1: ethernet-phy@1 {
> 			compatible = "ethernet-phy-id0022.1560";
> 			reg = <1>;
> 			micrel,led-mode = <1>;
> 			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
> 			clock-names = "rmii-ref";
> 		};
> 	};
> };
> 
> For FEC driver now, there is a patch from Fabio to prevent unbind/bind feature since dual FEC controllers share one MDIO bus. (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/net/ethernet/freescale/fec_main.c?h=next-20210324&id=272bb0e9e8cdc76e04baeefa0cd43019daa0841b)
> If we unbind fec2 and then fec1 can't work since MDIO bus is controlled by FEC1, FEC2 can't use it independently.
> 
> My question is that if we want to implement unbind/bind feature, what need we do?

One option is you unbind FEC1 first, and then FEC2.

> It seems to abstract an independent MDIO bus for dual FEC
> instances. I look at the MDIO dt bindings, it seems support such
> case as it has "reg"
> property. (Documentation/devicetree/bindings/net/mdio.yaml)

You can have fully standalone MDIO bus drivers. You generally do this
when the MDIO bus registers are in their own address space, which you
can ioremap() separately from the MAC registers. Take a look in
drivers/net/mdio/.

> From your opinions, do you think it is necessary to improve it?

What is you use case for unbinding/binding the FEC?

     Andrew

RE: FEC unbind/bind feature

[Joakim Zhang]

Hi Andrew,

Thanks for your kindly reply!

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 2021年3月25日 20:45
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; hkallweit1@gmail.com;
> netdev@vger.kernel.org
> Subject: Re: FEC unbind/bind feature
> 
> On Thu, Mar 25, 2021 at 08:04:58AM +0000, Joakim Zhang wrote:
> >
> > Hi Andrew, Florian, Heiner
> >
> > You are all Ethernet MDIO bus and PHY experts, I have some questions may
> need your help, thanks a lot in advance.
> >
> > For many board designs, if it has dual MAC instances, they always share one
> MDIO bus to save PINs. Such as, i.MX6UL EVK board:
> 
> Please wrap your lines at around 75 characters. Standard netiquette rules for
> emails apply to all Linux lists.

Ok, thanks.

> >
> > &fec1 {
> > 	pinctrl-names = "default";
> > 	pinctrl-0 = <&pinctrl_enet1>;
> > 	phy-mode = "rmii";
> > 	phy-handle = <&ethphy0>;
> > 	phy-supply = <&reg_peri_3v3>;
> > 	status = "okay";
> > };
> >
> > &fec2 {
> > 	pinctrl-names = "default";
> > 	pinctrl-0 = <&pinctrl_enet2>;
> > 	phy-mode = "rmii";
> > 	phy-handle = <&ethphy1>;
> > 	phy-supply = <&reg_peri_3v3>;
> > 	status = "okay";
> >
> > 	mdio {
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> >
> > 		ethphy0: ethernet-phy@2 {
> > 			compatible = "ethernet-phy-id0022.1560";
> > 			reg = <2>;
> > 			micrel,led-mode = <1>;
> > 			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> > 			clock-names = "rmii-ref";
> >
> > 		};
> >
> > 		ethphy1: ethernet-phy@1 {
> > 			compatible = "ethernet-phy-id0022.1560";
> > 			reg = <1>;
> > 			micrel,led-mode = <1>;
> > 			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
> > 			clock-names = "rmii-ref";
> > 		};
> > 	};
> > };
> >
> > For FEC driver now, there is a patch from Fabio to prevent unbind/bind
> > feature since dual FEC controllers share one MDIO bus.
> > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-next.gi
> t
> > %2Fcommit%2Fdrivers%2Fnet%2Fethernet%2Ffreescale%2Ffec_main.c%3Fh
> %3Dne
> >
> xt-20210324%26id%3D272bb0e9e8cdc76e04baeefa0cd43019daa0841b&amp;
> data=0
> >
> 4%7C01%7Cqiangqing.zhang%40nxp.com%7C4ac266f1ef514bd09e9c08d8ef8b
> ee33%
> >
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637522731561505308
> %7CUnkn
> >
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwi
> >
> LCJXVCI6Mn0%3D%7C1000&amp;sdata=dstGAIhwHtLt3YW9D8p8L5cNpxIWL
> h3wKzQmLp
> > oGGgE%3D&amp;reserved=0) If we unbind fec2 and then fec1 can't work
> > since MDIO bus is controlled by FEC1, FEC2 can't use it independently.
> >
> > My question is that if we want to implement unbind/bind feature, what need
> we do?
> 
> One option is you unbind FEC1 first, and then FEC2.

Yes, you are right. It should be always fine for single FEC controller, and unbind/bind one by one should also be fine for dual FEC controllers which share one MDIO bus. I test on i.MX6UL, i.MX8MM/MP.

> > It seems to abstract an independent MDIO bus for dual FEC instances. I
> > look at the MDIO dt bindings, it seems support such case as it has
> > "reg"
> > property. (Documentation/devicetree/bindings/net/mdio.yaml)
> 
> You can have fully standalone MDIO bus drivers. You generally do this when the
> MDIO bus registers are in their own address space, which you can ioremap()
> separately from the MAC registers. Take a look in drivers/net/mdio/.
> 
> > From your opinions, do you think it is necessary to improve it?
> 
> What is you use case for unbinding/binding the FEC?

Users may want to unbind FEC driver, and then bind to FEC UIO driver, such as for DPDK use case to improve the throughput.

Best Regards,
Joakim Zhang
>      Andrew

RE: FEC unbind/bind feature

[Joakim Zhang]

> -----Original Message-----
> From: Joakim Zhang <qiangqing.zhang@nxp.com>
> Sent: 2021年3月26日 16:03
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; hkallweit1@gmail.com;
> netdev@vger.kernel.org
> Subject: RE: FEC unbind/bind feature
> 
> 
> Hi Andrew,
> 
> Thanks for your kindly reply!
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: 2021年3月25日 20:45
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Cc: Florian Fainelli <f.fainelli@gmail.com>; hkallweit1@gmail.com;
> > netdev@vger.kernel.org
> > Subject: Re: FEC unbind/bind feature
> >
> > On Thu, Mar 25, 2021 at 08:04:58AM +0000, Joakim Zhang wrote:
> > >
> > > Hi Andrew, Florian, Heiner
> > >
> > > You are all Ethernet MDIO bus and PHY experts, I have some questions
> > > may
> > need your help, thanks a lot in advance.
> > >
> > > For many board designs, if it has dual MAC instances, they always
> > > share one
> > MDIO bus to save PINs. Such as, i.MX6UL EVK board:
> >
> > Please wrap your lines at around 75 characters. Standard netiquette
> > rules for emails apply to all Linux lists.
> 
> Ok, thanks.
> 
> > >
> > > &fec1 {
> > > 	pinctrl-names = "default";
> > > 	pinctrl-0 = <&pinctrl_enet1>;
> > > 	phy-mode = "rmii";
> > > 	phy-handle = <&ethphy0>;
> > > 	phy-supply = <&reg_peri_3v3>;
> > > 	status = "okay";
> > > };
> > >
> > > &fec2 {
> > > 	pinctrl-names = "default";
> > > 	pinctrl-0 = <&pinctrl_enet2>;
> > > 	phy-mode = "rmii";
> > > 	phy-handle = <&ethphy1>;
> > > 	phy-supply = <&reg_peri_3v3>;
> > > 	status = "okay";
> > >
> > > 	mdio {
> > > 		#address-cells = <1>;
> > > 		#size-cells = <0>;
> > >
> > > 		ethphy0: ethernet-phy@2 {
> > > 			compatible = "ethernet-phy-id0022.1560";
> > > 			reg = <2>;
> > > 			micrel,led-mode = <1>;
> > > 			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> > > 			clock-names = "rmii-ref";
> > >
> > > 		};
> > >
> > > 		ethphy1: ethernet-phy@1 {
> > > 			compatible = "ethernet-phy-id0022.1560";
> > > 			reg = <1>;
> > > 			micrel,led-mode = <1>;
> > > 			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
> > > 			clock-names = "rmii-ref";
> > > 		};
> > > 	};
> > > };
> > >
> > > For FEC driver now, there is a patch from Fabio to prevent
> > > unbind/bind feature since dual FEC controllers share one MDIO bus.
> > > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> > > it
> > > .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-next.
> g
> > > i
> > t
> > > %2Fcommit%2Fdrivers%2Fnet%2Fethernet%2Ffreescale%2Ffec_main.c%3F
> h
> > %3Dne
> > >
> >
> xt-20210324%26id%3D272bb0e9e8cdc76e04baeefa0cd43019daa0841b&amp;
> > data=0
> > >
> >
> 4%7C01%7Cqiangqing.zhang%40nxp.com%7C4ac266f1ef514bd09e9c08d8ef8b
> > ee33%
> > >
> >
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637522731561505308
> > %7CUnkn
> > >
> >
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> > aWwi
> > >
> >
> LCJXVCI6Mn0%3D%7C1000&amp;sdata=dstGAIhwHtLt3YW9D8p8L5cNpxIWL
> > h3wKzQmLp
> > > oGGgE%3D&amp;reserved=0) If we unbind fec2 and then fec1 can't work
> > > since MDIO bus is controlled by FEC1, FEC2 can't use it independently.
> > >
> > > My question is that if we want to implement unbind/bind feature,
> > > what need
> > we do?
> >
> > One option is you unbind FEC1 first, and then FEC2.
> 
> Yes, you are right. It should be always fine for single FEC controller, and
> unbind/bind one by one should also be fine for dual FEC controllers which share
> one MDIO bus. I test on i.MX6UL, i.MX8MM/MP.
> 
> > > It seems to abstract an independent MDIO bus for dual FEC instances.
> > > I look at the MDIO dt bindings, it seems support such case as it has
> > > "reg"
> > > property. (Documentation/devicetree/bindings/net/mdio.yaml)
> >
> > You can have fully standalone MDIO bus drivers. You generally do this
> > when the MDIO bus registers are in their own address space, which you
> > can ioremap() separately from the MAC registers. Take a look in
> drivers/net/mdio/.

One more add, yes, I am looking the drivers/net/mdio, it is better to implement standalone MDIO driver when writing the MAC driver at the beginning.
Now if I abstract MDIO driver from FEC driver, dt bindings would change, it will break all existing implementations in the kernel based on FEC driver, let them can't work.
How to compatible the legacy dt bindings? I have no idea now. At the same time, I also feel that it seems not necessary to rewrite it.

Best Regards,
Joakim Zhang
> > > From your opinions, do you think it is necessary to improve it?
> >
> > What is you use case for unbinding/binding the FEC?
> 
> Users may want to unbind FEC driver, and then bind to FEC UIO driver, such as
> for DPDK use case to improve the throughput.
> 
> Best Regards,
> Joakim Zhang
> >      Andrew

Re: FEC unbind/bind feature

[Andrew Lunn]

> > Please wrap your lines at around 75 characters. Standard netiquette rules for
> > emails apply to all Linux lists.
> 
> Ok, thanks.

Please keep fighting with your email client, 

> Yes, you are right. It should be always fine for single FEC controller, and unbind/bind one by one should also be fine for dual FEC controllers which share one MDIO bus. I test on i.MX6UL, i.MX8MM/MP.
 
> > > It seems to abstract an independent MDIO bus for dual FEC instances. I
> > > look at the MDIO dt bindings, it seems support such case as it has
> > > "reg"
> > > property. (Documentation/devicetree/bindings/net/mdio.yaml)
> > 
> > You can have fully standalone MDIO bus drivers. You generally do this when the
> > MDIO bus registers are in their own address space, which you can ioremap()
> > separately from the MAC registers. Take a look in drivers/net/mdio/.
> > 
> > > From your opinions, do you think it is necessary to improve it?
> > 
> > What is you use case for unbinding/binding the FEC?
> 
> Users may want to unbind FEC driver, and then bind to FEC UIO driver, such as for DPDK use case to improve the throughput.

You could blacklist the FEC driver so it never loads. Or change the
compatible string so it only matches the DPDK driver. Or use XDP :-)

	   Andrew

Re: FEC unbind/bind feature

[Andrew Lunn]

> One more add, yes, I am looking the drivers/net/mdio, it is better to implement standalone MDIO driver when writing the MAC driver at the beginning.
> Now if I abstract MDIO driver from FEC driver, dt bindings would change, it will break all existing implementations in the kernel based on FEC driver, let them can't work.
> How to compatible the legacy dt bindings? I have no idea now. At the same time, I also feel that it seems not necessary to rewrite it.

I have a reasonable understanding of the FEC MDIO driver. I have
broken it a few times :-)

It is going to be hard to make it an independent driver, because it
needs access to the interrupt flags and the clocks for power
saving. From a hardware perspective, it is not an independent hardware
block, it is integrated into the MAC.

XDP probably is your easier path.

    Andrew

RE: FEC unbind/bind feature

[Joakim Zhang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 2021年3月26日 20:34
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; hkallweit1@gmail.com;
> netdev@vger.kernel.org
> Subject: Re: FEC unbind/bind feature
> 
> > One more add, yes, I am looking the drivers/net/mdio, it is better to
> implement standalone MDIO driver when writing the MAC driver at the
> beginning.
> > Now if I abstract MDIO driver from FEC driver, dt bindings would change, it
> will break all existing implementations in the kernel based on FEC driver, let
> them can't work.
> > How to compatible the legacy dt bindings? I have no idea now. At the same
> time, I also feel that it seems not necessary to rewrite it.
> 
> I have a reasonable understanding of the FEC MDIO driver. I have broken it a
> few times :-)
> 
> It is going to be hard to make it an independent driver, because it needs access
> to the interrupt flags and the clocks for power saving. From a hardware
> perspective, it is not an independent hardware block, it is integrated into the
> MAC.
> 
> XDP probably is your easier path.

Thanks Andrew for your share, you mean use XDP instead DPDK, right?

Best Regards,
Joakim Zhang
>     Andrew

Re: FEC unbind/bind feature

[Andrew Lunn]

On Fri, Mar 26, 2021 at 01:18:52PM +0000, Joakim Zhang wrote:
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: 2021年3月26日 20:34
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Cc: Florian Fainelli <f.fainelli@gmail.com>; hkallweit1@gmail.com;
> > netdev@vger.kernel.org
> > Subject: Re: FEC unbind/bind feature
> > 
> > > One more add, yes, I am looking the drivers/net/mdio, it is better to
> > implement standalone MDIO driver when writing the MAC driver at the
> > beginning.
> > > Now if I abstract MDIO driver from FEC driver, dt bindings would change, it
> > will break all existing implementations in the kernel based on FEC driver, let
> > them can't work.
> > > How to compatible the legacy dt bindings? I have no idea now. At the same
> > time, I also feel that it seems not necessary to rewrite it.
> > 
> > I have a reasonable understanding of the FEC MDIO driver. I have broken it a
> > few times :-)
> > 
> > It is going to be hard to make it an independent driver, because it needs access
> > to the interrupt flags and the clocks for power saving. From a hardware
> > perspective, it is not an independent hardware block, it is integrated into the
> > MAC.
> > 
> > XDP probably is your easier path.
> 
> Thanks Andrew for your share, you mean use XDP instead DPDK, right?

Yes. We have much more control over XDP since it is in kernel, and
uses kernel drivers. No need to unbind the FEC, since the FEC is the
driver used by XDP. The down side is, the FEC has not been optimised
for XDP, so you will be using the generic support. Its performance
won't be optimal. To get that, you need to add native XDP support to
the FEC driver.

    Andrew

RE: FEC unbind/bind feature

[Joakim Zhang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 2021年3月26日 20:34
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; hkallweit1@gmail.com;
> netdev@vger.kernel.org
> Subject: Re: FEC unbind/bind feature
> 
> > One more add, yes, I am looking the drivers/net/mdio, it is better to
> implement standalone MDIO driver when writing the MAC driver at the
> beginning.
> > Now if I abstract MDIO driver from FEC driver, dt bindings would change, it
> will break all existing implementations in the kernel based on FEC driver, let
> them can't work.
> > How to compatible the legacy dt bindings? I have no idea now. At the same
> time, I also feel that it seems not necessary to rewrite it.
> 
> I have a reasonable understanding of the FEC MDIO driver. I have broken it a
> few times :-)
> 
> It is going to be hard to make it an independent driver, because it needs access
> to the interrupt flags and the clocks for power saving. From a hardware
> perspective, it is not an independent hardware block, it is integrated into the
> MAC.

Agree 😊

For another curiosity, dual FEC instances share one MDIO bus, we can successfully unbind them one by one. But if users first unbind FEC which attached MDIO bus, kernel would have a dump or crash, it seems not good.
So I look at the code, want to find a way to reject unbind this FEC first, then print a log, something like "other FEC instances depend on MDIO bus of this FEC, so can't unbind it now". It seems no way to do this at FEC driver remove path (fec_drv_remove). If you have some idea, happy share with me. Thanks.

Best Regards,
Joakim Zhang
> XDP probably is your easier path.
> 
>     Andrew