linux-kernel - Re: [PATCH v2 net-next 5/6] net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC

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 PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 13 Oct 2021 19:25:30 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Alvin Šipraga <alvin@...s.dk>,
        Linus Walleij <linus.walleij@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>
Cc:     Alvin Šipraga <alsi@...g-olufsen.dk>,
        Michael Rasmussen <mir@...g-olufsen.dk>,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 net-next 5/6] net: dsa: realtek-smi: add rtl8365mb
 subdriver for RTL8365MB-VC



On 10/13/2021 7:50 AM, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@...g-olufsen.dk>
> 
> This patch adds a realtek-smi subdriver for the RTL8365MB-VC 4+1 port
> 10/100/1000M switch controller. The driver has been developed based on a
> GPL-licensed OS-agnostic Realtek vendor driver known as rtl8367c found
> in the OpenWrt source tree.
> 
> Despite the name, the RTL8365MB-VC has an entirely different register
> layout to the already-supported RTL8366RB ASIC. Notwithstanding this,
> the structure of the rtl8365mb subdriver is based on the rtl8366rb
> subdriver and makes use of the rtl8366 helper library for setup of the
> SMI interface and handling of MIB counters. Like the 'rb, it establishes
> its own irqchip to handle cascaded PHY link status interrupts.
> 
> The RTL8365MB-VC switch is capable of offloading a large number of
> features from the software, but this patch introduces only the most
> basic DSA driver functionality. The ports always function as standalone
> ports, with bridging handled in software.
> 
> One more thing. Realtek's nomenclature for switches makes it hard to
> know exactly what other ASICs might be supported by this driver. The
> vendor driver goes by the name rtl8367c, but as far as I can tell, no
> chip actually exists under this name. As such, the subdriver is named
> rtl8365mb to emphasize the potentially limited support. But it is clear
> from the vendor sources that a number of other more advanced switches
> share a similar register layout, and further support should not be too
> hard to add given access to the relevant hardware. With this in mind,
> the subdriver has been written with as few assumptions about the
> particular chip as is reasonable. But the RTL8365MB-VC is the only
> hardware I have available, so some further work is surely needed.
> 
> Co-developed-by: Michael Rasmussen <mir@...g-olufsen.dk>
> Signed-off-by: Michael Rasmussen <mir@...g-olufsen.dk>
> Signed-off-by: Alvin Šipraga <alsi@...g-olufsen.dk>
> Reviewed-by: Vladimir Oltean <olteanv@...il.com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>

Nice work! If you happen to spin a v3, maybe consider moving the IRQ 
chip setup to an earlier point in the driver probe since it is a path 
that can return -EPROBE_DEFER?
-- 
Florian