Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 15 Oct 2021 17:40:47 -0700
From:   Jakub Kicinski <kuba@kernel.org>
To:     Alvin =?UTF-8?B?xaBpcHJhZ2E=?= <alvin@pqrs.dk>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Rob Herring <robh+dt@kernel.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Alvin =?UTF-8?B?xaBpcHJhZ2E=?= <alsi@bang-olufsen.dk>,
        Michael Rasmussen <mir@bang-olufsen.dk>,
        netdev@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 net-next 6/7] net: dsa: realtek-smi: add rtl8365mb
 subdriver for RTL8365MB-VC
Message-ID: <20211015174047.6552112a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <20211015171030.2713493-7-alvin@pqrs.dk>
References: <20211015171030.2713493-1-alvin@pqrs.dk>
        <20211015171030.2713493-7-alvin@pqrs.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, 15 Oct 2021 19:10:27 +0200 Alvin =C5=A0ipraga wrote:
> v2 -> v3:
>   - collect Florian's Reviewed-by
>   - move IRQ setup earlier in probe per Florian's suggestion
>   - follow Jakub's suggestion and use the standard ethtool stats API

Thanks a lot for doing this. The code LGTM, the only thing that stands
out is the use of spin_locks(). I couldn't quickly parse out what bus
this device hangs off, if it's MMIO and registers can be read without
sleeping you could potentially get rid of the delayed work to read
stats, but I think you need to switch to
regmap_read_poll_timeout_atomic() because regmap_read_poll_timeout()
itself can sleep.

If the register access sleeps (I2C, SPI, MDIO etc) you need to switch
from a spin lock to a mutex.

Either way CONFIG_DEBUG_ATOMIC_SLEEP is your friend.