Re: [PATCH net-next v7 08/10] net: dsa: realtek: rtl8365mb: re-latch the family D SerDes

[Alvin Šipraga]

On Thu, Nov 05, 2026 at 11:24:08AM +0100, Mieczyslaw Nalewaj wrote:
> On Thu, Nov 05, 2026 at 09:52:53AM +0100, Alvin Šipraga wrote:
> > On Thu, Nov 05, 2026 at 08:18:49AM +0100, Mieczyslaw Nalewaj wrote:
> > > rtl8365mb_sds_write() and rtl8365mb_sds_read() now take
> > > sds_lock with guard(mutex)(&mb->sds_lock), so the goto out_unlock paths
> > > and the explicit mutex_unlock() are gone and every error path is a plain
> > > return.
> >
> > I've spent too much time out of netdev recently:
> >
> > https://kernel.org
> >
> > Andrew here gives some input on netdev maintainters' view of guard().
> >
> > So perhaps you can ignore my comment to this patch after all.
> 
> Thanks for the heads-up and for sharing Andrew's input.
> 
> However, please note that guard(mutex) is already used for sds_cfg_lock in
> several other functions within this patch, namely pcs_config(),
> pcs_disable(), conduit_state_change(), and teardown().
> 
> To maintain a consistent coding style across the whole patch, should I keep
> guard(mutex) as it is, or would you prefer that I convert all of them back
> to the classic mutex_lock()/mutex_unlock() pattern for v9?
> 
> Both rtl8365mb_sds_write() and rtl8365mb_sds_read() are very short and the
> lock always spans until the final return, so they shouldn't suffer from the
> over-protection issues Andrew mentioned, but I am happy to change the whole
> file back to explicit locking if that is preferred for netdev.

After considering what Andrew said, and the fact that none of the
current driver code uses guard()s, I would suggest being consistent and
not using them at all.

Kind regards,
Alvin