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

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)µchip_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)µchip_config }, { }
-- 2.31.0
-- Regards,
Dimitri.
participants (2)
-
Dimitri John Ledkov
-
Green Wan