staging: pi433: add comment to rx_lock mutex definition
PATCH v2 1/1 staging: pi433: add comment to rx_lock mutex definition
Checkpatch reports: CHECK: struct mutex definition without comment. Fix this by documenting what rx_mutex struct is used for in pi433 driver.
---
v2: ellaborate on reasons why the mutex lock is used in the driver (Req: Greg k-h)
v1: https://lore.kernel.org/lkml/[email protected]/
--- drivers/staging/pi433/pi433_if.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/staging/pi433/pi433_if.c +11 -0
@@ -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 +*/ struct mutex rx_lock;wait_queue_head_t rx_wait_queue;
Signature · 1 line
2.25.4
Greg KH
maintainerQuoted 20 lines · Paulo Miguel Almeida
Checkpatch reports: CHECK: struct mutex definition without comment. Fix this by documenting what
rx_mutexstruct is used for in pi433 driver.Signed-off-by Paulo Miguel Almeida <[email protected]>---
v2: ellaborate on reasons why the mutex lock is used in the driver (Req: Greg k-h)
v1: https://lore.kernel.org/lkml/[email protected]/--- drivers/staging/pi433/pi433_if.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/staging/pi433/pi433_if.c +1
-0@@ -92,6 +92,17 @@ struct pi433_device {u32 rx_bytes_to_drop;u32 rx_bytes_dropped;unsigned int rx_position; +/*
drivers/staging/pi433/pi433_if.c struct pi433_device { · line 105
Quoted 10 lines
drivers/staging/pi433/pi433_if.c +10
-0+* 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
Quoted 15 lines · Greg KH
drivers/staging/pi433/pi433_if.c struct pi433_device { · line 105
Quoted 11 lines · Paulo Miguel Almeida
drivers/staging/pi433/pi433_if.c +11
-0+/* +* 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.
Quoted 3 lines
How about something simple like:
/* Protects all rx_* variable accesses */
I see your point. I will send a new version of this patch with your sugestion. Thanks for guidance.
thanks,
Paulo A.