[v7,8/8] drivers: net: macb: add fu740 support

From: David Abdurachmanov <david.abdurachmanov@sifive.com>

Add fu740 support to macb ethernet driver

There is a PLL HW quirk in FU740. The VSC8541XMV-02 specification
requires 125 +/-0.0125 Mhz. But the most close value can be output
by PLL is 125.125 MHz and out of VSC8541XMV-02 spec.

Signed-off-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
Signed-off-by: Green Wan <green.wan@sifive.com>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 drivers/net/macb.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

(resending to list after subscribing)

Hi,

On Thu, Apr 22, 2021 at 10:15 AM Green Wan <green.wan@sifive.com> wrote:
>
> From: David Abdurachmanov <david.abdurachmanov@sifive.com>
>
> Add fu740 support to macb ethernet driver
>
> There is a PLL HW quirk in FU740. The VSC8541XMV-02 specification
> requires 125 +/-0.0125 Mhz. But the most close value can be output
> by PLL is 125.125 MHz and out of VSC8541XMV-02 spec.
>

In the Linux kernel driver for this
drivers/net/ethernet/cadence/macb_main.c it is not marked as
compatible with "sifive,fu740-c000-gem" and it does not have a similar
fix (and appears to use 125.0 MHz).
Should a similar fix be contributed to the Linux kernel?

As otherwise at the moment, one cannot pass the dtb from u-boot to
linux, as that leads to loss of network since the kernel doesn't know
about "sifive,fu740-c000-gem". If linux kernel contribution is not
forthcoming, would it be possible to have u-boot dtb to be compatible
with _both_  "sifive,fu540-c000-gem" and "sifive,fu740-c000-gem" on
unmatched boards, such that if one (mistakenly) uses u-boots dtb with
vanilla linux kernel networking still works? And then adjust the test
to check for "sifive,fu740-c000-gem" compatible string first.

> Signed-off-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
> Signed-off-by: Green Wan <green.wan@sifive.com>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>  drivers/net/macb.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 57ea45e2dc..bf70525c54 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -591,8 +591,17 @@ static int macb_sifive_clk_init(struct udevice *dev, ulong rate)
>          * 0 = GMII mode. Use 125 MHz gemgxlclk from PRCI in TX logic
>          *     and output clock on GMII output signal GTX_CLK
>          * 1 = MII mode. Use MII input signal TX_CLK in TX logic
> +        *
> +        * FU740 have a PLL HW quirk. The 125.125 Mhz is actually out of
> +        * VSC8541XMV-02 specification. The tolerance level is +/-100ppm.
> +        * Which means the range should be in between 125MHz +/-0.0125.
> +        * But the most close value can be output by PLL is 125.125 MHz.
>          */
> -       writel(rate != 125000000, gemgxl_regs);
> +       if (device_is_compatible(dev, "sifive,fu540-c000-gem"))
> +               writel(rate != 125000000, gemgxl_regs);
> +       else if (device_is_compatible(dev, "sifive,fu740-c000-gem"))
> +               writel(rate != 125125000, gemgxl_regs);
> +
>         return 0;
>  }
>
> @@ -1507,6 +1516,8 @@ static const struct udevice_id macb_eth_ids[] = {
>         { .compatible = "cdns,zynq-gem" },
>         { .compatible = "sifive,fu540-c000-gem",
>           .data = (ulong)&sifive_config },
> +       { .compatible = "sifive,fu740-c000-gem",
> +         .data = (ulong)&sifive_config },
>         { .compatible = "microchip,mpfs-mss-gem",
>           .data = (ulong)&microchip_config },
>         { }
> --
> 2.31.0
>


--
Regards,

Dimitri.

Hi Dimitri,

Thanks for looking into this.

On Tue, May 4, 2021 at 5:33 PM Dimitri John Ledkov
<dimitri.ledkov@canonical.com> wrote:
>
> Hi,
>
> On Thu, Apr 22, 2021 at 10:15 AM Green Wan <green.wan@sifive.com> wrote:
> >
> > From: David Abdurachmanov <david.abdurachmanov@sifive.com>
> >
> > Add fu740 support to macb ethernet driver
> >
> > There is a PLL HW quirk in FU740. The VSC8541XMV-02 specification
> > requires 125 +/-0.0125 Mhz. But the most close value can be output
> > by PLL is 125.125 MHz and out of VSC8541XMV-02 spec.
> >
>
> In the Linux kernel driver for this
> drivers/net/ethernet/cadence/macb_main.c it is not marked as
> compatible with "sifive,fu740-c000-gem" and it does not have a similar
> fix (and appears to use 125.0 MHz).
> Should a similar fix be contributed to the Linux kernel?

You're right. We also notice some refinement should be made here.
We're working on the way to solve it.

>
> As otherwise at the moment, one cannot pass the dtb from u-boot to
> linux, as that leads to loss of network since the kernel doesn't know
> about "sifive,fu740-c000-gem". If linux kernel contribution is not
> forthcoming, would it be possible to have u-boot dtb to be compatible
> with _both_  "sifive,fu540-c000-gem" and "sifive,fu740-c000-gem" on
> unmatched boards, such that if one (mistakenly) uses u-boots dtb with
> vanilla linux kernel networking still works? And then adjust the test
> to check for "sifive,fu740-c000-gem" compatible string first.
>

Not sure whether I get it correct. I think the best case is to have
Linux driver support both compatible string. And I'm a bit reluctant
to handle incorrect DTB passed situations. It might end up with
propagating issues and harder to trace back the root cause. I'll check
with colleagues and see if we can resolve that concern.

Thanks,

> > Signed-off-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
> > Signed-off-by: Green Wan <green.wan@sifive.com>
> > Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >  drivers/net/macb.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> > index 57ea45e2dc..bf70525c54 100644
> > --- a/drivers/net/macb.c
> > +++ b/drivers/net/macb.c
> > @@ -591,8 +591,17 @@ static int macb_sifive_clk_init(struct udevice *dev, ulong rate)
> >          * 0 = GMII mode. Use 125 MHz gemgxlclk from PRCI in TX logic
> >          *     and output clock on GMII output signal GTX_CLK
> >          * 1 = MII mode. Use MII input signal TX_CLK in TX logic
> > +        *
> > +        * FU740 have a PLL HW quirk. The 125.125 Mhz is actually out of
> > +        * VSC8541XMV-02 specification. The tolerance level is +/-100ppm.
> > +        * Which means the range should be in between 125MHz +/-0.0125.
> > +        * But the most close value can be output by PLL is 125.125 MHz.
> >          */
> > -       writel(rate != 125000000, gemgxl_regs);
> > +       if (device_is_compatible(dev, "sifive,fu540-c000-gem"))
> > +               writel(rate != 125000000, gemgxl_regs);
> > +       else if (device_is_compatible(dev, "sifive,fu740-c000-gem"))
> > +               writel(rate != 125125000, gemgxl_regs);
> > +
> >         return 0;
> >  }
> >
> > @@ -1507,6 +1516,8 @@ static const struct udevice_id macb_eth_ids[] = {
> >         { .compatible = "cdns,zynq-gem" },
> >         { .compatible = "sifive,fu540-c000-gem",
> >           .data = (ulong)&sifive_config },
> > +       { .compatible = "sifive,fu740-c000-gem",
> > +         .data = (ulong)&sifive_config },
> >         { .compatible = "microchip,mpfs-mss-gem",
> >           .data = (ulong)&microchip_config },
> >         { }
> > --
> > 2.31.0
> >
>
>
> --
> Regards,
>
> Dimitri.

On Wed, May 5, 2021 at 4:15 AM Green Wan <green.wan@sifive.com> wrote:
>
> Hi Dimitri,
>
> Thanks for looking into this.
>
> On Tue, May 4, 2021 at 5:33 PM Dimitri John Ledkov
> <dimitri.ledkov@canonical.com> wrote:
> >
> > Hi,
> >
> > On Thu, Apr 22, 2021 at 10:15 AM Green Wan <green.wan@sifive.com> wrote:
> > >
> > > From: David Abdurachmanov <david.abdurachmanov@sifive.com>
> > >
> > > Add fu740 support to macb ethernet driver
> > >
> > > There is a PLL HW quirk in FU740. The VSC8541XMV-02 specification
> > > requires 125 +/-0.0125 Mhz. But the most close value can be output
> > > by PLL is 125.125 MHz and out of VSC8541XMV-02 spec.
> > >
> >
> > In the Linux kernel driver for this
> > drivers/net/ethernet/cadence/macb_main.c it is not marked as
> > compatible with "sifive,fu740-c000-gem" and it does not have a similar
> > fix (and appears to use 125.0 MHz).
> > Should a similar fix be contributed to the Linux kernel?
>
> You're right. We also notice some refinement should be made here.
> We're working on the way to solve it.
>
> >
> > As otherwise at the moment, one cannot pass the dtb from u-boot to
> > linux, as that leads to loss of network since the kernel doesn't know
> > about "sifive,fu740-c000-gem". If linux kernel contribution is not
> > forthcoming, would it be possible to have u-boot dtb to be compatible
> > with _both_  "sifive,fu540-c000-gem" and "sifive,fu740-c000-gem" on
> > unmatched boards, such that if one (mistakenly) uses u-boots dtb with
> > vanilla linux kernel networking still works? And then adjust the test
> > to check for "sifive,fu740-c000-gem" compatible string first.
> >
>
> Not sure whether I get it correct. I think the best case is to have
> Linux driver support both compatible string.

That is my desired outcome too. Please just do that.

> And I'm a bit reluctant
> to handle incorrect DTB passed situations. It might end up with
> propagating issues and harder to trace back the root cause. I'll check
> with colleagues and see if we can resolve that concern.
>

Writing / handling / handing-over incorrect DTB is not nice.

> Thanks,
>
> > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
> > > Signed-off-by: Green Wan <green.wan@sifive.com>
> > > Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > ---
> > >  drivers/net/macb.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> > > index 57ea45e2dc..bf70525c54 100644
> > > --- a/drivers/net/macb.c
> > > +++ b/drivers/net/macb.c
> > > @@ -591,8 +591,17 @@ static int macb_sifive_clk_init(struct udevice *dev, ulong rate)
> > >          * 0 = GMII mode. Use 125 MHz gemgxlclk from PRCI in TX logic
> > >          *     and output clock on GMII output signal GTX_CLK
> > >          * 1 = MII mode. Use MII input signal TX_CLK in TX logic
> > > +        *
> > > +        * FU740 have a PLL HW quirk. The 125.125 Mhz is actually out of
> > > +        * VSC8541XMV-02 specification. The tolerance level is +/-100ppm.
> > > +        * Which means the range should be in between 125MHz +/-0.0125.
> > > +        * But the most close value can be output by PLL is 125.125 MHz.
> > >          */
> > > -       writel(rate != 125000000, gemgxl_regs);
> > > +       if (device_is_compatible(dev, "sifive,fu540-c000-gem"))
> > > +               writel(rate != 125000000, gemgxl_regs);
> > > +       else if (device_is_compatible(dev, "sifive,fu740-c000-gem"))
> > > +               writel(rate != 125125000, gemgxl_regs);
> > > +
> > >         return 0;
> > >  }
> > >
> > > @@ -1507,6 +1516,8 @@ static const struct udevice_id macb_eth_ids[] = {
> > >         { .compatible = "cdns,zynq-gem" },
> > >         { .compatible = "sifive,fu540-c000-gem",
> > >           .data = (ulong)&sifive_config },
> > > +       { .compatible = "sifive,fu740-c000-gem",
> > > +         .data = (ulong)&sifive_config },
> > >         { .compatible = "microchip,mpfs-mss-gem",
> > >           .data = (ulong)&microchip_config },
> > >         { }
> > > --
> > > 2.31.0
> > >
> >
> >
> > --
> > Regards,
> >
> > Dimitri.

Hi,

On Thu, Apr 22, 2021 at 5:14 PM Green Wan <green.wan@sifive.com> wrote:
>
> From: David Abdurachmanov <david.abdurachmanov@sifive.com>
>
> Add fu740 support to macb ethernet driver
>
> There is a PLL HW quirk in FU740. The VSC8541XMV-02 specification
> requires 125 +/-0.0125 Mhz. But the most close value can be output
> by PLL is 125.125 MHz and out of VSC8541XMV-02 spec.
>
> Signed-off-by: David Abdurachmanov <david.abdurachmanov@sifive.com>
> Signed-off-by: Green Wan <green.wan@sifive.com>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>  drivers/net/macb.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>

It looks this patch was never applied. Both U-Boot and Linux drivers
do not have the "sifive,fu740-c000-gem" codes.

Not sure what the current upstream status of this?

Regards,
Bin