On Thu, Dec 23, 2021 at 10:56:15AM +1300, Paulo Miguel Almeida wrote: > Checkpatch reports: CHECK: struct mutex definition without comment. > Fix this by documenting what rx_mutex struct is used for in pi433 driver. > > Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@xxxxxxxxx> > --- > v2: ellaborate on reasons why the mutex lock is used in the driver (Req: Greg k-h) > v1: https://lore.kernel.org/lkml/20211222093626.GA13332@localhost.localdomain/ > --- > drivers/staging/pi433/pi433_if.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c > index 29bd37669059..1cd3d5f2df2a 100644 > --- a/drivers/staging/pi433/pi433_if.c > +++ b/drivers/staging/pi433/pi433_if.c > @@ -92,6 +92,17 @@ struct pi433_device { > u32 rx_bytes_to_drop; > u32 rx_bytes_dropped; > unsigned int rx_position; > + /* > + * rx_lock is used to avoid race-conditions that can be triggered from userspace. > + * > + * For instance, if a program in userspace is reading the char device > + * allocated in this module then another program won't be able to change RX > + * configuration of the RF69 hardware module via ioctl and vice versa. > + * > + * utilization summary: > + * - pi433_read: blocks are read until rx read something (up to the buffer size) > + * - pi433_ioctl: during pending read request, change of config not allowed > + */ This is nice, but way too specific, and will quickly get out-of-date. How about something simple like: /* Protects all rx_* variable accesses */ thanks, greg k-h
