Re: [PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
Hi Marek, On Sun, Jan 24, 2021 at 4:52 PM Marek Vasut wrote: > > On 1/24/21 3:35 PM, Oleksandr Suvorov wrote: > > On Sun, Jan 24, 2021 at 12:04 AM Stefano Babic wrote: > >> > >> Hi Marek, > >> > >> On 23.01.21 20:57, Marek Vasut wrote: > >>> On 1/23/21 8:54 PM, sba...@denx.de wrote: > > The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which > > supports OF control, correctly set the rgmii-id PHY mode in the DT and > > then determine whether or not to enable the FEC RXC/TXC internal delay > > based on the PHY mode. > >>> > >>> [...] > >>> > Applied to u-boot-imx, master, thanks ! > >>> > >>> Didn't this fail to build on mx25 ? > >> > >> Yes, sorry, this is a false positive - patch has not been applied. > > > > Anyway, as there will be another solution that won't break mx25/mx35, > > I should retest the new patch version on apalis, so I'm calling off my > > review so far. > > The only change required to that patch is an extra ifdef DM_ETH, so for > mx8 apalis nothing changed, since you have DM_ETH enabled. Thanks for clarifying! So you have my review for apalis mx8. -- Best regards Oleksandr Suvorov cryo...@gmail.com
Re: [PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
On 1/24/21 3:35 PM, Oleksandr Suvorov wrote: On Sun, Jan 24, 2021 at 12:04 AM Stefano Babic wrote: Hi Marek, On 23.01.21 20:57, Marek Vasut wrote: On 1/23/21 8:54 PM, sba...@denx.de wrote: The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which supports OF control, correctly set the rgmii-id PHY mode in the DT and then determine whether or not to enable the FEC RXC/TXC internal delay based on the PHY mode. [...] Applied to u-boot-imx, master, thanks ! Didn't this fail to build on mx25 ? Yes, sorry, this is a false positive - patch has not been applied. Anyway, as there will be another solution that won't break mx25/mx35, I should retest the new patch version on apalis, so I'm calling off my review so far. The only change required to that patch is an extra ifdef DM_ETH, so for mx8 apalis nothing changed, since you have DM_ETH enabled.
Re: [PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
On Sun, Jan 24, 2021 at 12:04 AM Stefano Babic wrote: > > Hi Marek, > > On 23.01.21 20:57, Marek Vasut wrote: > > On 1/23/21 8:54 PM, sba...@denx.de wrote: > >>> The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which > >>> supports OF control, correctly set the rgmii-id PHY mode in the DT and > >>> then determine whether or not to enable the FEC RXC/TXC internal delay > >>> based on the PHY mode. > > > > [...] > > > >> Applied to u-boot-imx, master, thanks ! > > > > Didn't this fail to build on mx25 ? > > Yes, sorry, this is a false positive - patch has not been applied. Anyway, as there will be another solution that won't break mx25/mx35, I should retest the new patch version on apalis, so I'm calling off my review so far. > Regards, > Stefano -- Best regards Oleksandr Suvorov cryo...@gmail.com
Re: [PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
Hi Marek, On 23.01.21 20:57, Marek Vasut wrote: On 1/23/21 8:54 PM, sba...@denx.de wrote: The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which supports OF control, correctly set the rgmii-id PHY mode in the DT and then determine whether or not to enable the FEC RXC/TXC internal delay based on the PHY mode. [...] Applied to u-boot-imx, master, thanks ! Didn't this fail to build on mx25 ? Yes, sorry, this is a false positive - patch has not been applied. Regards, Stefano
Re: [PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
On 1/23/21 8:54 PM, sba...@denx.de wrote: The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which supports OF control, correctly set the rgmii-id PHY mode in the DT and then determine whether or not to enable the FEC RXC/TXC internal delay based on the PHY mode. [...] Applied to u-boot-imx, master, thanks ! Didn't this fail to build on mx25 ?
[PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
> The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which > supports OF control, correctly set the rgmii-id PHY mode in the DT and > then determine whether or not to enable the FEC RXC/TXC internal delay > based on the PHY mode. > Signed-off-by: Marek Vasut > Cc: Oleksandr Suvorov > Cc: Stefano Babic > Cc: Ramon Fried > Reviewed-by: Oleksandr Suvorov Applied to u-boot-imx, master, thanks ! Best regards, Stefano Babic -- = DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de =
Re: [PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
Hi Marek, this rather breaks old platforms - I check with mx25pdk, but also mx35 is broken. Could you take a look ? Thanks ! +drivers/net/fec_mxc.c:510:9: error: 'struct fec_priv' has no member named 'interface' + 510 | if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || + | ^~ +drivers/net/fec_mxc.c:511:9: error: 'struct fec_priv' has no member named 'interface' + 511 | fec->interface == PHY_INTERFACE_MODE_RGMII_TXID) +drivers/net/fec_mxc.c:514:9: error: 'struct fec_priv' has no member named 'interface' + 514 | if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || +drivers/net/fec_mxc.c:515:9: error: 'struct fec_priv' has no member named 'interface' + 515 | fec->interface == PHY_INTERFACE_MODE_RGMII_RXID) +make[2]: *** [scripts/Makefile.build:266: drivers/net/fec_mxc.o] Error 1 +make[1]: *** [Makefile:1787: drivers/net] Error 2 +make: *** [Makefile:167: sub-make] Error 2 arm: + mx25pdk Regards, Stefano On 20.01.21 09:17, Oleksandr Suvorov wrote: > Hi Marek, > > On Tue, Jan 19, 2021 at 1:57 AM Marek Vasut wrote: >> >> The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which >> supports OF control, correctly set the rgmii-id PHY mode in the DT and >> then determine whether or not to enable the FEC RXC/TXC internal delay >> based on the PHY mode. >> >> Signed-off-by: Marek Vasut > > Reviewed-by: Oleksandr Suvorov > > Thanks & Regards > >> Cc: Oleksandr Suvorov >> Cc: Stefano Babic >> Cc: Ramon Fried >> --- >> arch/arm/dts/fsl-imx8qm-apalis.dts | 2 +- >> arch/arm/dts/fsl-imx8qxp-apalis.dts | 2 +- >> drivers/net/fec_mxc.c | 14 ++ >> include/configs/apalis-imx8.h | 3 --- >> include/configs/apalis-imx8x.h | 3 --- >> 5 files changed, 8 insertions(+), 16 deletions(-) >> >> diff --git a/arch/arm/dts/fsl-imx8qm-apalis.dts >> b/arch/arm/dts/fsl-imx8qm-apalis.dts >> index 5187b794527..0d8d3b3e8e8 100644 >> --- a/arch/arm/dts/fsl-imx8qm-apalis.dts >> +++ b/arch/arm/dts/fsl-imx8qm-apalis.dts >> @@ -503,7 +503,7 @@ >> pinctrl-0 = <_fec1>; >> fsl,magic-packet; >> phy-handle = <>; >> - phy-mode = "rgmii"; >> + phy-mode = "rgmii-id"; >> phy-reset-duration = <10>; >> phy-reset-gpios = < 11 1>; >> status = "okay"; >> diff --git a/arch/arm/dts/fsl-imx8qxp-apalis.dts >> b/arch/arm/dts/fsl-imx8qxp-apalis.dts >> index 6bd231b2834..9cb3d3a809b 100644 >> --- a/arch/arm/dts/fsl-imx8qxp-apalis.dts >> +++ b/arch/arm/dts/fsl-imx8qxp-apalis.dts >> @@ -229,7 +229,7 @@ >> pinctrl-0 = <_fec1>; >> fsl,magic-packet; >> phy-handle = <>; >> - phy-mode = "rgmii"; >> + phy-mode = "rgmii-id"; >> phy-reset-duration = <10>; >> phy-reset-post-delay = <150>; >> phy-reset-gpios = < 4 GPIO_ACTIVE_LOW>; >> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c >> index bb55be9a267..73fc5f4c903 100644 >> --- a/drivers/net/fec_mxc.c >> +++ b/drivers/net/fec_mxc.c >> @@ -507,15 +507,13 @@ static int fec_open(struct eth_device *edev) >> writel(readl(>eth->ecntrl) | FEC_ECNTRL_ETHER_EN, >>>eth->ecntrl); >> >> -#ifdef FEC_ENET_ENABLE_TXC_DELAY >> - writel(readl(>eth->ecntrl) | FEC_ECNTRL_TXC_DLY, >> - >eth->ecntrl); >> -#endif >> + if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || >> + fec->interface == PHY_INTERFACE_MODE_RGMII_TXID) >> + setbits_le32(>eth->ecntrl, FEC_ECNTRL_TXC_DLY); >> >> -#ifdef FEC_ENET_ENABLE_RXC_DELAY >> - writel(readl(>eth->ecntrl) | FEC_ECNTRL_RXC_DLY, >> - >eth->ecntrl); >> -#endif >> + if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || >> + fec->interface == PHY_INTERFACE_MODE_RGMII_RXID) >> + setbits_le32(>eth->ecntrl, FEC_ECNTRL_RXC_DLY); >> >> #if defined(CONFIG_MX25) || defined(CONFIG_MX53) || defined(CONFIG_MX6SL) >> udelay(100); >> diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h >> index db4e9011c0b..81441d7770f 100644 >> --- a/include/configs/apalis-imx8.h >> +++ b/include/configs/apalis-imx8.h >> @@ -21,9 +21,6 @@ >> #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG >> >> /* Networking */ >> -#define FEC_QUIRK_ENET_MAC >> -#define FEC_ENET_ENABLE_TXC_DELAY >> - >> #define CONFIG_TFTP_TSIZE >> >> #define CONFIG_IPADDR 192.168.10.2 >> diff --git a/include/configs/apalis-imx8x.h b/include/configs/apalis-imx8x.h >> index db31c210f50..90bbeb926d3 100644 >> --- a/include/configs/apalis-imx8x.h >> +++ b/include/configs/apalis-imx8x.h >> @@ -25,9 +25,6 @@ >> #define CONFIG_NETMASK 255.255.255.0 >> #define CONFIG_SERVERIP192.168.10.1 >> >> -#define FEC_ENET_ENABLE_TXC_DELAY >> -#define FEC_ENET_ENABLE_RXC_DELAY >> - >> #define MEM_LAYOUT_ENV_SETTINGS \ >> "kernel_addr_r=0x8028\0" \ >> "fdt_addr_r=0x8310\0" \ >> -- >> 2.29.2 >> > > --
Re: [PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
Hi Marek, On Tue, Jan 19, 2021 at 1:57 AM Marek Vasut wrote: > > The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which > supports OF control, correctly set the rgmii-id PHY mode in the DT and > then determine whether or not to enable the FEC RXC/TXC internal delay > based on the PHY mode. > > Signed-off-by: Marek Vasut Reviewed-by: Oleksandr Suvorov Thanks & Regards > Cc: Oleksandr Suvorov > Cc: Stefano Babic > Cc: Ramon Fried > --- > arch/arm/dts/fsl-imx8qm-apalis.dts | 2 +- > arch/arm/dts/fsl-imx8qxp-apalis.dts | 2 +- > drivers/net/fec_mxc.c | 14 ++ > include/configs/apalis-imx8.h | 3 --- > include/configs/apalis-imx8x.h | 3 --- > 5 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/arch/arm/dts/fsl-imx8qm-apalis.dts > b/arch/arm/dts/fsl-imx8qm-apalis.dts > index 5187b794527..0d8d3b3e8e8 100644 > --- a/arch/arm/dts/fsl-imx8qm-apalis.dts > +++ b/arch/arm/dts/fsl-imx8qm-apalis.dts > @@ -503,7 +503,7 @@ > pinctrl-0 = <_fec1>; > fsl,magic-packet; > phy-handle = <>; > - phy-mode = "rgmii"; > + phy-mode = "rgmii-id"; > phy-reset-duration = <10>; > phy-reset-gpios = < 11 1>; > status = "okay"; > diff --git a/arch/arm/dts/fsl-imx8qxp-apalis.dts > b/arch/arm/dts/fsl-imx8qxp-apalis.dts > index 6bd231b2834..9cb3d3a809b 100644 > --- a/arch/arm/dts/fsl-imx8qxp-apalis.dts > +++ b/arch/arm/dts/fsl-imx8qxp-apalis.dts > @@ -229,7 +229,7 @@ > pinctrl-0 = <_fec1>; > fsl,magic-packet; > phy-handle = <>; > - phy-mode = "rgmii"; > + phy-mode = "rgmii-id"; > phy-reset-duration = <10>; > phy-reset-post-delay = <150>; > phy-reset-gpios = < 4 GPIO_ACTIVE_LOW>; > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c > index bb55be9a267..73fc5f4c903 100644 > --- a/drivers/net/fec_mxc.c > +++ b/drivers/net/fec_mxc.c > @@ -507,15 +507,13 @@ static int fec_open(struct eth_device *edev) > writel(readl(>eth->ecntrl) | FEC_ECNTRL_ETHER_EN, >>eth->ecntrl); > > -#ifdef FEC_ENET_ENABLE_TXC_DELAY > - writel(readl(>eth->ecntrl) | FEC_ECNTRL_TXC_DLY, > - >eth->ecntrl); > -#endif > + if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || > + fec->interface == PHY_INTERFACE_MODE_RGMII_TXID) > + setbits_le32(>eth->ecntrl, FEC_ECNTRL_TXC_DLY); > > -#ifdef FEC_ENET_ENABLE_RXC_DELAY > - writel(readl(>eth->ecntrl) | FEC_ECNTRL_RXC_DLY, > - >eth->ecntrl); > -#endif > + if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || > + fec->interface == PHY_INTERFACE_MODE_RGMII_RXID) > + setbits_le32(>eth->ecntrl, FEC_ECNTRL_RXC_DLY); > > #if defined(CONFIG_MX25) || defined(CONFIG_MX53) || defined(CONFIG_MX6SL) > udelay(100); > diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h > index db4e9011c0b..81441d7770f 100644 > --- a/include/configs/apalis-imx8.h > +++ b/include/configs/apalis-imx8.h > @@ -21,9 +21,6 @@ > #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG > > /* Networking */ > -#define FEC_QUIRK_ENET_MAC > -#define FEC_ENET_ENABLE_TXC_DELAY > - > #define CONFIG_TFTP_TSIZE > > #define CONFIG_IPADDR 192.168.10.2 > diff --git a/include/configs/apalis-imx8x.h b/include/configs/apalis-imx8x.h > index db31c210f50..90bbeb926d3 100644 > --- a/include/configs/apalis-imx8x.h > +++ b/include/configs/apalis-imx8x.h > @@ -25,9 +25,6 @@ > #define CONFIG_NETMASK 255.255.255.0 > #define CONFIG_SERVERIP192.168.10.1 > > -#define FEC_ENET_ENABLE_TXC_DELAY > -#define FEC_ENET_ENABLE_RXC_DELAY > - > #define MEM_LAYOUT_ENV_SETTINGS \ > "kernel_addr_r=0x8028\0" \ > "fdt_addr_r=0x8310\0" \ > -- > 2.29.2 > -- Best regards Oleksandr Suvorov Toradex AG Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00
[PATCH] net: fec: Get rid of FEC_ENET_ENABLE_[TR]XC_DELAY
The FEC_ENET_ENABLE_.XC_DELAY setting is only used by one system which supports OF control, correctly set the rgmii-id PHY mode in the DT and then determine whether or not to enable the FEC RXC/TXC internal delay based on the PHY mode. Signed-off-by: Marek Vasut Cc: Oleksandr Suvorov Cc: Stefano Babic Cc: Ramon Fried --- arch/arm/dts/fsl-imx8qm-apalis.dts | 2 +- arch/arm/dts/fsl-imx8qxp-apalis.dts | 2 +- drivers/net/fec_mxc.c | 14 ++ include/configs/apalis-imx8.h | 3 --- include/configs/apalis-imx8x.h | 3 --- 5 files changed, 8 insertions(+), 16 deletions(-) diff --git a/arch/arm/dts/fsl-imx8qm-apalis.dts b/arch/arm/dts/fsl-imx8qm-apalis.dts index 5187b794527..0d8d3b3e8e8 100644 --- a/arch/arm/dts/fsl-imx8qm-apalis.dts +++ b/arch/arm/dts/fsl-imx8qm-apalis.dts @@ -503,7 +503,7 @@ pinctrl-0 = <_fec1>; fsl,magic-packet; phy-handle = <>; - phy-mode = "rgmii"; + phy-mode = "rgmii-id"; phy-reset-duration = <10>; phy-reset-gpios = < 11 1>; status = "okay"; diff --git a/arch/arm/dts/fsl-imx8qxp-apalis.dts b/arch/arm/dts/fsl-imx8qxp-apalis.dts index 6bd231b2834..9cb3d3a809b 100644 --- a/arch/arm/dts/fsl-imx8qxp-apalis.dts +++ b/arch/arm/dts/fsl-imx8qxp-apalis.dts @@ -229,7 +229,7 @@ pinctrl-0 = <_fec1>; fsl,magic-packet; phy-handle = <>; - phy-mode = "rgmii"; + phy-mode = "rgmii-id"; phy-reset-duration = <10>; phy-reset-post-delay = <150>; phy-reset-gpios = < 4 GPIO_ACTIVE_LOW>; diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index bb55be9a267..73fc5f4c903 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -507,15 +507,13 @@ static int fec_open(struct eth_device *edev) writel(readl(>eth->ecntrl) | FEC_ECNTRL_ETHER_EN, >eth->ecntrl); -#ifdef FEC_ENET_ENABLE_TXC_DELAY - writel(readl(>eth->ecntrl) | FEC_ECNTRL_TXC_DLY, - >eth->ecntrl); -#endif + if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || + fec->interface == PHY_INTERFACE_MODE_RGMII_TXID) + setbits_le32(>eth->ecntrl, FEC_ECNTRL_TXC_DLY); -#ifdef FEC_ENET_ENABLE_RXC_DELAY - writel(readl(>eth->ecntrl) | FEC_ECNTRL_RXC_DLY, - >eth->ecntrl); -#endif + if (fec->interface == PHY_INTERFACE_MODE_RGMII_ID || + fec->interface == PHY_INTERFACE_MODE_RGMII_RXID) + setbits_le32(>eth->ecntrl, FEC_ECNTRL_RXC_DLY); #if defined(CONFIG_MX25) || defined(CONFIG_MX53) || defined(CONFIG_MX6SL) udelay(100); diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h index db4e9011c0b..81441d7770f 100644 --- a/include/configs/apalis-imx8.h +++ b/include/configs/apalis-imx8.h @@ -21,9 +21,6 @@ #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG /* Networking */ -#define FEC_QUIRK_ENET_MAC -#define FEC_ENET_ENABLE_TXC_DELAY - #define CONFIG_TFTP_TSIZE #define CONFIG_IPADDR 192.168.10.2 diff --git a/include/configs/apalis-imx8x.h b/include/configs/apalis-imx8x.h index db31c210f50..90bbeb926d3 100644 --- a/include/configs/apalis-imx8x.h +++ b/include/configs/apalis-imx8x.h @@ -25,9 +25,6 @@ #define CONFIG_NETMASK 255.255.255.0 #define CONFIG_SERVERIP192.168.10.1 -#define FEC_ENET_ENABLE_TXC_DELAY -#define FEC_ENET_ENABLE_RXC_DELAY - #define MEM_LAYOUT_ENV_SETTINGS \ "kernel_addr_r=0x8028\0" \ "fdt_addr_r=0x8310\0" \ -- 2.29.2
