Re: [PATCH] staging: ks7010: fix spinlock_t definition without comment

On Tue, 12 Mar 2024, Tahera Fahimi wrote:

> On Tue, Mar 12, 2024 at 10:49:58PM +0100, Julia Lawall wrote:
> >
> >
> > On Tue, 12 Mar 2024, TaheraFahimi wrote:
> >
> > > This change ensures adherence to coding style guidelines.
> > >
> > > Signed-off-by: TaheraFahimi <[email protected]>
> > > ---
> > >  drivers/staging/ks7010/ks_wlan.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
> > > index 3e9a91b5131c..94322e2ea304 100644
> > > --- a/drivers/staging/ks7010/ks_wlan.h
> > > +++ b/drivers/staging/ks7010/ks_wlan.h
> > > @@ -468,7 +468,7 @@ struct ks_wlan_private {
> > >  	char nick[IW_ESSID_MAX_SIZE + 1];
> > >
> > >  	spinlock_t multicast_spin;
> > > -
> > > +	/* lock for synchronize access to device read operation */
> >
> > How did you come up with this comment?  I'm not sure that it reflects very
> > well the function in which the lock is used.
> >
> > julia
> >
> > >  	spinlock_t dev_read_lock;
> > >  	wait_queue_head_t devread_wait;
> > >
> > > --
> > > 2.34.1
> > >
> > >
> > >
> I came up with this comment since this lock is used in function "devio_rec_ind" in ks_hostif.c to ensure safe access to the device's receive buffer.

OK.  That doesn't seem to be conveyed by the comment.

Since you are replying to my message, it would be better to put your text
directly under my message, and not at the end.

julia