On Wed, May 5, 2021 at 4:15 AM Green Wan <green....@sifive.com> wrote:
>
> Hi Dimitri,
>
> Thanks for looking into this.
>
> On Tue, May 4, 2021 at 5:33 PM Dimitri John Ledkov
> <dimitri.led...@canonical.com> wrote:
> >
> > Hi,
> >
> > On Thu, Apr 22, 2021 at 10:15 AM Green Wan <green....@sifive.com> wrote:
> > >
> > > From: David Abdurachmanov <david.abdurachma...@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.abdurachma...@sifive.com>
> > > Signed-off-by: Green Wan <green....@sifive.com>
> > > Reviewed-by: Ramon Fried <rfried....@gmail.com>
> > > Reviewed-by: Bin Meng <bmeng...@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)µchip_config },
> > > { }
> > > --
> > > 2.31.0
> > >
> >
> >
> > --
> > Regards,
> >
> > Dimitri.
--
Regards,
Dimitri.
