Re: [PATCH v3 net-next 6/7] net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC

[Alvin Šipraga]

On 10/16/21 2:40 AM, Jakub Kicinski wrote:
> On Fri, 15 Oct 2021 19:10:27 +0200 Alvin Šipraga 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.

It's an "SMI" bus bit-banged with GPIO - details are in 
realtek-smi-core.c - a Realtek peculiarity. Initially I thought this was 
implementation was sleeping, but actually it's using ndelay() which is 
OK in atomic context if I'm not mistaken? So I guess you're right, I can 
get rid of the delayed work. :)

> 
> 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.

Thanks, I'll test with this and follow up. I see a kernel test robot 
warning so I will be sending v4 anyway.