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

[Green Wan]

Hi Dimitri,

Thanks for looking into this.

On Tue, May 4, 2021 at 5:33 PM Dimitri John Ledkov
<dimitri.ledkov at canonical.com> wrote:
>
> Hi,
>
> On Thu, Apr 22, 2021 at 10:15 AM Green Wan <green.wan at sifive.com> wrote:
> >
> > From: David Abdurachmanov <david.abdurachmanov at 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 at sifive.com>
> > Signed-off-by: Green Wan <green.wan at sifive.com>
> > Reviewed-by: Ramon Fried <rfried.dev at gmail.com>
> > Reviewed-by: Bin Meng <bmeng.cn at 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.