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 08:18:49AM +0100, Mieczyslaw Nalewaj wrote:
> Subject: Re: [PATCH net-next v7 08/10] net: dsa: realtek: rtl8365mb:
> re-latch the family D SerDes
> 
> On Wed, Nov 04, 2026 at 01:57:24PM +0100, Alvin Šipraga wrote:
> > On Mon, Nov 02, 2026 at 03:43:45PM +0100, Mieczyslaw Nalewaj wrote:
> > > +    struct rtl8365mb *mb = priv->chip_data;
> > >      int ret;
> > >
> > > +    mutex_lock(&mb->sds_lock);
> >
> > OK, I don't think I can give a particularly good review of this patch
> > since I didn't follow the previous work for SerDes support... But at
> > least, can you use guard(mutex)() here? ;-) And elsewhere it might
> > apply.
> 
> Sure, done. 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://lore.kernel.org/all/5ef1e25b-2d6d-4cb1-8030-756d60c37c47@xxxxxxx/

Andrew here gives some input on netdev maintainters' view of guard().

So perhaps you can ignore my comment to this patch after all.

> 
> > > +    mutex_lock(&mb->sds_lock);
> >
> > Like here, etc.
> 
> These two are the only users of sds_lock. The other locking in the patch
> (sds_cfg_lock in pcs_config(), pcs_disable(), conduit_state_change() and
> teardown()) already uses guard(mutex)().
> 
> As you suggested, I will not post v9 yet and will wait until we have
> settled the remaining points in this thread.
> 
> Best regards,
> Mieczyslaw