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

[Dimitri John Ledkov]

On Wed, May 5, 2021 at 4:15 AM Green Wan <green.wan at sifive.com> wrote:
>
> 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.

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 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.



-- 
Regards,

Dimitri.