[PATCH v3 1/2] net: fec: Add possibility to enable TXC delay

[PATCH v3 1/2] net: fec: Add possibility to enable TXC delay

[Philippe Schenker]

This patch enables the possibility to set FEC_ENET_ENABLE_TXC_DELAY or
FEC_ENET_ENABLE_RXC_DELAY so one can via a define enable the RXC or TXC
delay in the MAC.

Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

---

Changes in v3:
- Add Olek's reviewed-by tag

Changes in v2: None

 drivers/net/fec_mxc.c | 10 ++++++++++
 drivers/net/fec_mxc.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index bc5b63d7881..345d37be4e8 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -503,6 +503,16 @@ static int fec_open(struct eth_device *edev)
 	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_ETHER_EN,
 	       &fec->eth->ecntrl);
 
+#ifdef FEC_ENET_ENABLE_TXC_DELAY
+	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_TXC_DLY,
+	       &fec->eth->ecntrl);
+#endif
+
+#ifdef FEC_ENET_ENABLE_RXC_DELAY
+	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_RXC_DLY,
+	       &fec->eth->ecntrl);
+#endif
+
 #if defined(CONFIG_MX25) || defined(CONFIG_MX53) || defined(CONFIG_MX6SL)
 	udelay(100);
 
diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
index 159aec89679..3c8fdda2638 100644
--- a/drivers/net/fec_mxc.h
+++ b/drivers/net/fec_mxc.h
@@ -188,6 +188,8 @@ struct ethernet_regs {
 #define FEC_ECNTRL_ETHER_EN		0x00000002	/* enable the FEC */
 #define FEC_ECNTRL_SPEED		0x00000020
 #define FEC_ECNTRL_DBSWAP		0x00000100
+#define FEC_ECNTRL_TXC_DLY		0x00010000	/* TXC delayed */
+#define FEC_ECNTRL_RXC_DLY		0x00020000	/* RXC delayed */
 
 #define FEC_X_WMRK_STRFWD		0x00000100
 
-- 
2.25.1

[PATCH v3 2/2] config: apalis-imx8: Enable FEC TXC delay

[Philippe Schenker]

define FEC_ENET_ENABLE_TXC_DELAY to enable the delay on TXC line on
the MAC. This has to be done in order to meet RGMII specs.
According to RGMII specs the clock should get delayed so the edges of
the clock are preferrably in the middle of the edges of data-lines so
they can be sampled properly.
Our PHY expects the MAC to delay TXC line, as it is also written
in the spec. This patch makes sure the TXC delay on the FEC is enabled

Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

---

Changes in v3:
- Add Olek's revied-by tag

Changes in v2:
- Entered a 'why' description in patch 'config: apalis-imx8: Enable FEC
  TXC delay'

 include/configs/apalis-imx8.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
index bea035c3e23..40037157339 100644
--- a/include/configs/apalis-imx8.h
+++ b/include/configs/apalis-imx8.h
@@ -24,6 +24,7 @@
 
 /* Networking */
 #define FEC_QUIRK_ENET_MAC
+#define FEC_ENET_ENABLE_TXC_DELAY
 
 #define CONFIG_TFTP_TSIZE
 
-- 
2.25.1

[PATCH v3 1/2] net: fec: Add possibility to enable TXC delay

[Stefano Babic]

Hi Philippe,

On 11/03/20 11:52, Philippe Schenker wrote:
> This patch enables the possibility to set FEC_ENET_ENABLE_TXC_DELAY or
> FEC_ENET_ENABLE_RXC_DELAY so one can via a define enable the RXC or TXC
> delay in the MAC.
> 
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> 
> ---
> 
> Changes in v3:
> - Add Olek's reviewed-by tag
> 
> Changes in v2: None
> 
>  drivers/net/fec_mxc.c | 10 ++++++++++
>  drivers/net/fec_mxc.h |  2 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index bc5b63d7881..345d37be4e8 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -503,6 +503,16 @@ static int fec_open(struct eth_device *edev)
>  	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_ETHER_EN,
>  	       &fec->eth->ecntrl);
>  
> +#ifdef FEC_ENET_ENABLE_TXC_DELAY
> +	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_TXC_DLY,
> +	       &fec->eth->ecntrl);
> +#endif
> +
> +#ifdef FEC_ENET_ENABLE_RXC_DELAY
> +	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_RXC_DLY,
> +	       &fec->eth->ecntrl);
> +#endif
> +
>  #if defined(CONFIG_MX25) || defined(CONFIG_MX53) || defined(CONFIG_MX6SL)
>  	udelay(100);
>  
> diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
> index 159aec89679..3c8fdda2638 100644
> --- a/drivers/net/fec_mxc.h
> +++ b/drivers/net/fec_mxc.h
> @@ -188,6 +188,8 @@ struct ethernet_regs {
>  #define FEC_ECNTRL_ETHER_EN		0x00000002	/* enable the FEC */
>  #define FEC_ECNTRL_SPEED		0x00000020
>  #define FEC_ECNTRL_DBSWAP		0x00000100
> +#define FEC_ECNTRL_TXC_DLY		0x00010000	/* TXC delayed */
> +#define FEC_ECNTRL_RXC_DLY		0x00020000	/* RXC delayed */
>  
>  #define FEC_X_WMRK_STRFWD		0x00000100
>  
> 

But these are not exactly the properties removed by verdin-imx8mm in
DTS, that is for example rgmii_rxc_dly ? And then, should not be better
to support it in DT instead of a CONFIG_ ?

Regards,
Stefano

-- 
=====================================================================
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: sbabic at denx.de
=====================================================================

[PATCH v3 1/2] net: fec: Add possibility to enable TXC delay

[Philippe Schenker]

On Fri, 2020-04-17 at 18:35 +0200, Stefano Babic wrote:
> Hi Philippe,
> 
> On 11/03/20 11:52, Philippe Schenker wrote:
> > This patch enables the possibility to set FEC_ENET_ENABLE_TXC_DELAY
> > or
> > FEC_ENET_ENABLE_RXC_DELAY so one can via a define enable the RXC or
> > TXC
> > delay in the MAC.
> > 
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> > Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > 
> > ---
> > 
> > Changes in v3:
> > - Add Olek's reviewed-by tag
> > 
> > Changes in v2: None
> > 
> >  drivers/net/fec_mxc.c | 10 ++++++++++
> >  drivers/net/fec_mxc.h |  2 ++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> > index bc5b63d7881..345d37be4e8 100644
> > --- a/drivers/net/fec_mxc.c
> > +++ b/drivers/net/fec_mxc.c
> > @@ -503,6 +503,16 @@ static int fec_open(struct eth_device *edev)
> >  	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_ETHER_EN,
> >  	       &fec->eth->ecntrl);
> >  
> > +#ifdef FEC_ENET_ENABLE_TXC_DELAY
> > +	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_TXC_DLY,
> > +	       &fec->eth->ecntrl);
> > +#endif
> > +
> > +#ifdef FEC_ENET_ENABLE_RXC_DELAY
> > +	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_RXC_DLY,
> > +	       &fec->eth->ecntrl);
> > +#endif
> > +
> >  #if defined(CONFIG_MX25) || defined(CONFIG_MX53) ||
> > defined(CONFIG_MX6SL)
> >  	udelay(100);
> >  
> > diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
> > index 159aec89679..3c8fdda2638 100644
> > --- a/drivers/net/fec_mxc.h
> > +++ b/drivers/net/fec_mxc.h
> > @@ -188,6 +188,8 @@ struct ethernet_regs {
> >  #define FEC_ECNTRL_ETHER_EN		0x00000002	/* enable
> > the FEC */
> >  #define FEC_ECNTRL_SPEED		0x00000020
> >  #define FEC_ECNTRL_DBSWAP		0x00000100
> > +#define FEC_ECNTRL_TXC_DLY		0x00010000	/* TXC delayed */
> > +#define FEC_ECNTRL_RXC_DLY		0x00020000	/* RXC delayed */
> >  
> >  #define FEC_X_WMRK_STRFWD		0x00000100
> >  
> > 
> 
> But these are not exactly the properties removed by verdin-imx8mm in
> DTS, that is for example rgmii_rxc_dly ? And then, should not be
> better
> to support it in DT instead of a CONFIG_ ?

Hi Stefano,

I'll add Igor in cc as I'm not that familiar with most recent
development on u-boot. Yes you are right. Those tags (rgmii_rxc_dly) are
exactly the same. However they came in from our downstream branch and
were never meant for mainline u-boot. We dropped them in favor of the
proper phy-mode 'rgmii-id' in linux. Those originally probably also
originated from our downstream linux branch...

But U-Boot does not support phy-modes on KSZ PHYs so I felt this was a
straight forward and easy solution for u-boot that we can enable as soon
as we have our new PHY KSZ9131 on our board. At the moment there is
KSZ9031 placed which does indeed not support this.

Regards,
Philippe
> 
> Regards,
> Stefano
> 

[PATCH v3 1/2] net: fec: Add possibility to enable TXC delay

[Stefano Babic]

On 17.04.20 18:56, Philippe Schenker wrote:
> On Fri, 2020-04-17 at 18:35 +0200, Stefano Babic wrote:
>> Hi Philippe,
>>
>> On 11/03/20 11:52, Philippe Schenker wrote:
>>> This patch enables the possibility to set FEC_ENET_ENABLE_TXC_DELAY
>>> or
>>> FEC_ENET_ENABLE_RXC_DELAY so one can via a define enable the RXC or
>>> TXC
>>> delay in the MAC.
>>>
>>> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>>> Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
>>>
>>> ---
>>>
>>> Changes in v3:
>>> - Add Olek's reviewed-by tag
>>>
>>> Changes in v2: None
>>>
>>>  drivers/net/fec_mxc.c | 10 ++++++++++
>>>  drivers/net/fec_mxc.h |  2 ++
>>>  2 files changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
>>> index bc5b63d7881..345d37be4e8 100644
>>> --- a/drivers/net/fec_mxc.c
>>> +++ b/drivers/net/fec_mxc.c
>>> @@ -503,6 +503,16 @@ static int fec_open(struct eth_device *edev)
>>>  	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_ETHER_EN,
>>>  	       &fec->eth->ecntrl);
>>>  
>>> +#ifdef FEC_ENET_ENABLE_TXC_DELAY
>>> +	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_TXC_DLY,
>>> +	       &fec->eth->ecntrl);
>>> +#endif
>>> +
>>> +#ifdef FEC_ENET_ENABLE_RXC_DELAY
>>> +	writel(readl(&fec->eth->ecntrl) | FEC_ECNTRL_RXC_DLY,
>>> +	       &fec->eth->ecntrl);
>>> +#endif
>>> +
>>>  #if defined(CONFIG_MX25) || defined(CONFIG_MX53) ||
>>> defined(CONFIG_MX6SL)
>>>  	udelay(100);
>>>  
>>> diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
>>> index 159aec89679..3c8fdda2638 100644
>>> --- a/drivers/net/fec_mxc.h
>>> +++ b/drivers/net/fec_mxc.h
>>> @@ -188,6 +188,8 @@ struct ethernet_regs {
>>>  #define FEC_ECNTRL_ETHER_EN		0x00000002	/* enable
>>> the FEC */
>>>  #define FEC_ECNTRL_SPEED		0x00000020
>>>  #define FEC_ECNTRL_DBSWAP		0x00000100
>>> +#define FEC_ECNTRL_TXC_DLY		0x00010000	/* TXC delayed */
>>> +#define FEC_ECNTRL_RXC_DLY		0x00020000	/* RXC delayed */
>>>  
>>>  #define FEC_X_WMRK_STRFWD		0x00000100
>>>  
>>>
>>
>> But these are not exactly the properties removed by verdin-imx8mm in
>> DTS, that is for example rgmii_rxc_dly ? And then, should not be
>> better
>> to support it in DT instead of a CONFIG_ ?
> 
> Hi Stefano,
> 
> I'll add Igor in cc as I'm not that familiar with most recent
> development on u-boot. Yes you are right. Those tags (rgmii_rxc_dly) are
> exactly the same. However they came in from our downstream branch and
> were never meant for mainline u-boot. We dropped them in favor of the
> proper phy-mode 'rgmii-id' in linux. Those originally probably also
> originated from our downstream linux branch...
> 
> But U-Boot does not support phy-modes on KSZ PHYs so I felt this was a
> straight forward and easy solution

This is an easy and straighforward solution, sure. Strange enough, your
solution in DT is exactly as Oliver's did here:

http://patchwork.ozlabs.org/project/uboot/patch/20200203135619.10180-1-oliver.graute at kococonnector.com/

Both of you tried to get in in DT.

> for u-boot that we can enable as soon
> as we have our new PHY KSZ9131 on our board. At the moment there is
> KSZ9031 placed which does indeed not support this.

Anyway, until the property is not mainlined to Linux, I am fine to merge
your series.

Regards,
Stefano

-- 
=====================================================================
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: sbabic at denx.de
=====================================================================

[PATCH v3 1/2] net: fec: Add possibility to enable TXC delay

[sbabic at denx.de]

> This patch enables the possibility to set FEC_ENET_ENABLE_TXC_DELAY or
> FEC_ENET_ENABLE_RXC_DELAY so one can via a define enable the RXC or TXC
> delay in the MAC.
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
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: sbabic at denx.de
=====================================================================

[PATCH v3 2/2] config: apalis-imx8: Enable FEC TXC delay

[sbabic at denx.de]

> define FEC_ENET_ENABLE_TXC_DELAY to enable the delay on TXC line on
> the MAC. This has to be done in order to meet RGMII specs.
> According to RGMII specs the clock should get delayed so the edges of
> the clock are preferrably in the middle of the edges of data-lines so
> they can be sampled properly.
> Our PHY expects the MAC to delay TXC line, as it is also written
> in the spec. This patch makes sure the TXC delay on the FEC is enabled
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
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: sbabic at denx.de
=====================================================================