netdev - Re: [PATCH v2 net-next 0/3] net: stmmac: RK3566

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <08e97e0e291aecb218b9ebb8aaa37afead504e1c.camel@collabora.com>
Date:   Fri, 23 Apr 2021 10:57:24 -0300
From:   Ezequiel Garcia <ezequiel@...labora.com>
To:     David Wu <david.wu@...k-chips.com>, netdev@...r.kernel.org,
        linux-rockchip@...ts.infradead.org
Cc:     Jose Abreu <joabreu@...opsys.com>,
        Heiko Stuebner <heiko@...ech.de>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Peter Geis <pgwipeout@...il.com>,
        Kever Yang <kever.yang@...k-chips.com>, kernel@...labora.com
Subject: Re: [PATCH v2 net-next 0/3] net: stmmac: RK3566

Hi David,

Thanks a lot for reviewing.

On Fri, 2021-04-23 at 17:24 +0800, David Wu wrote:
> Hi Ezequiel,
> 
> 在 2021/4/22 上午4:34, Ezequiel Garcia 写道:
> > Now that RK3568 SoC devicetree upstreaming is happening [1],
> > here's another round for the RK3566 dwmac. There wasn't any clear
> > consensus on how to implement the two interfaces present
> > on RK3568, so I decided to drop that and just submit RK3566 for now.
> > 
> > This has been tested on a Pine64 RK3566 Quartz64 Model B board,
> > DHCP and iperf are looking good.
> > 
> > For all the people testing, here's Quartz 64 Model B device tree
> > snippet:
> > 
> >          gmac1: ethernet@...10000 {
> >                  compatible = "rockchip,rk3566-gmac", "snps,dwmac-4.20a";
> 
> It is better to use "rockchip,rk3568-gmac" here, "rockchip,rk3566-gmac" 
> is not compatible, 3568 has two gmacs, which are compatible with 3566.
> 
> If there is no better way, using bus_id from alias is good, it is a 
> fixed id, and U-Boot also use the id to write MAC address into kernel DTB.
> 
> plat->bus_id = of_alias_get_id(np, "ethernet");
> 

This was discussed, see ChenYu's reply. I think the idea
is considered fragile:

https://lore.kernel.org/netdev/CAGb2v67ZBR=XDFPeXQc429HNu_dbY__-KN50tvBW44fXMs78_w@mail.gmail.com/

However, I agree with you about going back to just "rockchip,rk3568-gmac".
Adding rockchip,rk3566-gmac maybe wasn't a great idea :-)

The most accepted way forward seems Heiko's proposal of hardcoding the mmio
addresses to identify each block, as it's done in the DSI driver:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170

Would you agree with it?

Thanks!
Ezequiel
 
> >                  reg = <0x0 0xfe010000 0x0 0x10000>;
> >                  interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
> >                               <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> >                  interrupt-names = "macirq", "eth_wake_irq";
> >                  rockchip,grf = <&grf>;
> >                  clocks = <&cru SCLK_GMAC1>, <&cru SCLK_GMAC1_RX_TX>,
> >                           <&cru SCLK_GMAC1_RX_TX>, <&cru CLK_MAC1_REFOUT>,
> >                           <&cru ACLK_GMAC1>, <&cru PCLK_GMAC1>,
> >                           <&cru SCLK_GMAC1_RX_TX>, <&cru CLK_GMAC1_PTP_REF>;
> >                  clock-names = "stmmaceth", "mac_clk_rx",
> >                                "mac_clk_tx", "clk_mac_refout",
> >                                "aclk_mac", "pclk_mac",
> >                                "clk_mac_speed", "ptp_ref";
> >                  resets = <&cru SRST_A_GMAC1>;
> >                  reset-names = "stmmaceth";
> > 
> >                  snps,mixed-burst;
> >                  snps,tso;
> > 
> >                  snps,axi-config = <&gmac1_stmmac_axi_setup>;
> >                  snps,mtl-rx-config = <&gmac1_mtl_rx_setup>;
> >                  snps,mtl-tx-config = <&gmac1_mtl_tx_setup>;
> >                  status = "disabled";
> > 
> >                  mdio1: mdio {
> >                          compatible = "snps,dwmac-mdio";
> >                          #address-cells = <0x1>;
> >                          #size-cells = <0x0>;
> >                  };
> > 
> >                  gmac1_stmmac_axi_setup: stmmac-axi-config {
> >                          snps,wr_osr_lmt = <4>;
> >                          snps,rd_osr_lmt = <8>;
> >                          snps,blen = <0 0 0 0 16 8 4>;
> >                  };
> > 
> >                  gmac1_mtl_rx_setup: rx-queues-config {
> >                          snps,rx-queues-to-use = <1>;
> >                          queue0 {};
> >                  };
> > 
> >                  gmac1_mtl_tx_setup: tx-queues-config {
> >                          snps,tx-queues-to-use = <1>;
> >                          queue0 {};
> >                  };
> >          };
> > 
> > While here, I'm adding a small patch from David Wu, for some
> > sanity checks for dwmac-rockchip-specific non-NULL ops.
> > 
> > Thanks!
> > 
> > [1] http://lore.kernel.org/r/20210421065921.23917-1-cl@rock-chips.com
> > 
> > David Wu (2):
> >    net: stmmac: dwmac-rk: Check platform-specific ops
> >    net: stmmac: Add RK3566 SoC support
> > 
> > Ezequiel Garcia (1):
> >    net: stmmac: Don't set has_gmac if has_gmac4 is set
> > 
> >   .../bindings/net/rockchip-dwmac.txt           |   1 +
> >   .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 126 +++++++++++++++++-
> >   2 files changed, 124 insertions(+), 3 deletions(-)
> > 
> 
>