Re: [Outreachy kernel] Re: [PATCH v4 ] staging: iio: cdc: ad7150: add comment to mutex

[Julia Lawall <julia.lawall@lip6.fr>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2851 bytes --]



On Sun, 2 Oct 2016, Anchal Jain wrote:

> >I think I need to just not take any of these types of patches, as it's
> >not exactly a good thing for someone who does not understand the driver
> >to be able to fix up.
> >
> >Just ignore these warnings for now please
>
> I am justing trying to understand your drivers. It will helpful me learn
> how it works
> So, please I am requesting to you. Let me try. If it is not correct then
> don't accept it.

If you want to submit such patches, you need to look carefully at how the
lock is used in the code.  In this case, the code between the lock and the
unlock is rather complex.  In general, it is rather difficult to
understand why a lock is needed in a certain place, which is why the
comment is suggested in the first place.  But it is more targeting the
person who originally wrote the driver than somone who is doing code
cleanups.  Then you need also to look at other drivers that already have
such a comment to see what kind of information should be provided.

julia

>
> Thank you,
> On Sun, Oct 2, 2016 at 9:10 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>       On Sun, Oct 02, 2016 at 09:04:43PM +0530, Anchal Jain wrote:
>       > Add a comment in mutex to explain what the mutex is work
>       here.
>       >
>       > Signed-off-by: Anchal Jain <anchalj109@gmail.com>
>       > ---
>       >  drivers/staging/iio/cdc/ad7150.c | 2 +-
>       >  1 file changed, 1 insertion(+), 1 deletion(-)
>       >
>       > diff --git a/drivers/staging/iio/cdc/ad7150.c
>       b/drivers/staging/iio/cdc/ad7150.c
>       > index f6b9a10..13709bf 100644
>       > --- a/drivers/staging/iio/cdc/ad7150.c
>       > +++ b/drivers/staging/iio/cdc/ad7150.c
>       > @@ -78,7 +78,7 @@ struct ad7150_chip_info {
>       >       u8 mag_timeout[2][2];
>       >       int old_state;
>       >       char *conversion_mode;
>       > -     struct mutex state_lock;
>       > +     struct mutex state_lock; /* protect hardware state */
>
>       I think I need to just not take any of these types of patches,
>       as it's
>       not exactly a good thing for someone who does not understand
>       the driver
>       to be able to fix up.
>
>       Just ignore these warnings for now please.
>
>       thanks,
>
>       greg k-h
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CAAbeOSuPu599ySH5Mm%2BV
> B3xx-j0UX25%2BZtb2qVbqh8qnErFCKg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>