Re: [RFC PATCH v2 11/11] riscv: soc: Allwinner D1 GMAC driver only for temp use

Guo Ren
2021-06-06T16:53:31Z

On Mon, Jun 7, 2021 at 12:32 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Dne nedelja, 06. junij 2021 ob 18:16:44 CEST je Arnd Bergmann napisal(a):
> > On Sun, Jun 6, 2021 at 11:04 AM <guoren@kernel.org> wrote:
> > > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > > b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts index
> > > cd9f7c9..31b681d 100644
> > > --- a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > > @@ -11,7 +11,7 @@
> > >
> > > compatible = "allwinner,d1-nezha-kit";
> > >
> > > chosen {
> > >
> > > - bootargs = "console=ttyS0,115200";
> > > + bootargs = "console=ttyS0,115200 rootwait init=/sbin/init
> > > root=/dev/nfs rw nfsroot=192.168.101.200:/tmp/rootfs_nfs,v3,tcp,nolock
> > > ip=192.168.101.23";
> > These are not board specific options, they should be set by the bootloader
> > according to the network environment. It clearly doens't belong
> > into this patch .
> >
> > > stdout-path = &serial0;
> > >
> > > };
> > >
> > > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > > b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi index 11cd938..d317e19
> > > 100644
> > > --- a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > > @@ -80,5 +80,21 @@
> > >
> > > clocks = <&dummy_apb>;
> > > status = "disabled";
> > >
> > > };
> > >
> > > +
> > > + eth@4500000 {
> > > + compatible = "allwinner,sunxi-gmac";
> > > + reg = <0x00 0x4500000 0x00 0x10000 0x00 0x3000030
> > > 0x00 0x04>; + interrupts-extended = <&plic 0x3e
> > > 0x04>;
> > > + interrupt-names = "gmacirq";
> > > + device_type = "gmac0";
> > > + phy-mode = "rgmii";
> > > + use_ephy25m = <0x01>;
> > > + tx-delay = <0x03>;
> > > + rx-delay = <0x03>;
> > > + gmac-power0;
> > > + gmac-power1;
> > > + gmac-power2;
> > > + status = "okay";
> > > + };
> >
> > Before you add this in the dts file, the properties need to be documented in
> > the binding file. The "allwinner,sunxi-gmac" identifier does not appear to
> > be specific enough here, and the properties don't match what dwmac uses,
> > which would make it unnecessarily hard to change to the other driver later
> > on without breaking compatibility to old dtb files.
> >
> > > +++ b/drivers/net/ethernet/allwinnertmp/sunxi-gmac-ops.c
> > > @@ -0,0 +1,690 @@
> > > +/*
> > > + * linux/drivers/net/ethernet/allwinner/sunxi_gmac_ops.c
> > > + *
> > > + * Copyright © 2016-2018, fuzhaoke
> > > + * Author: fuzhaoke <fuzhaoke@allwinnertech.com>
> > > + *
> > > + * This file is provided under a dual BSD/GPL license. When using or
> > > + * redistributing this file, you may do so under either license.
> >
> > Are you sure this is the correct copyright information and "fuzhaoke" is
> > the copyright holder for this file? If this is derived from either the
> > designware
> > code or the Linux stmmac driver, the authors should be mentioned,
> > and the license be compatible with the original license terms.
> >
> > Andre already commented on the driver quality and code duplication, those
> > are also show-stoppers, but the unclear license terms and dt binding
> > compatibility are even stronger reasons to not get anywhere close to this
> > driver.
>
> I got impression that this patch is not meant to be merged and it's forward
> ported from vendor kernel as a stop gap measure for developers until proper
> mainline ethernet driver is developed.
Yes

>
> Best regards,
> Jernej
>
>
>
>

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/