[PATCH v4 2/2] staging: greybus: resolved checkpatch checks for light.c

Rachit Dhar posted 2 patches 3 weeks, 4 days ago
[PATCH v4 2/2] staging: greybus: resolved checkpatch checks for light.c
Posted by Rachit Dhar 3 weeks, 4 days ago
Added comments to mutex declarations, to resolve the associated checkpatch.pl checks:

CHECK: struct mutex definition without comment
+       struct mutex                    lock;

CHECK: struct mutex definition without comment
+       struct mutex            lights_lock;

Signed-off-by: Rachit Dhar <rchtdhr@gmail.com>
---
 drivers/staging/greybus/light.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index cab02b5da867..1391c2b5d5f4 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -37,7 +37,7 @@ struct gb_channel {
 	bool				releasing;
 	bool				strobe_state;
 	bool				active;
-	struct mutex			lock;
+	struct mutex			lock; /* protects gb_channel->active */
 };
 
 struct gb_light {
@@ -59,7 +59,7 @@ struct gb_lights {
 	struct gb_connection	*connection;
 	u8			lights_count;
 	struct gb_light		*lights;
-	struct mutex		lights_lock;
+	struct mutex		lights_lock; /* protects gb_lights->lights */
 };
 
 static void gb_lights_channel_free(struct gb_channel *channel);
-- 
2.43.0
Re: [PATCH v4 2/2] staging: greybus: resolved checkpatch checks for light.c
Posted by Dan Carpenter 3 weeks, 4 days ago
On Sat, Mar 07, 2026 at 02:09:25PM +0000, Rachit Dhar wrote:
> Added comments to mutex declarations, to resolve the associated checkpatch.pl checks:
> 
> CHECK: struct mutex definition without comment
> +       struct mutex                    lock;
> 
> CHECK: struct mutex definition without comment
> +       struct mutex            lights_lock;
> 
> Signed-off-by: Rachit Dhar <rchtdhr@gmail.com>

These kinds of things require more than a two word explanation.  It
should probably be a paragraph.  But first do a proper review of the
locking.  When do we start needing to worry about concurrent accesses?
How is it accessed?  What would happen if the locking were not there?
Is the unregister sequence correct?

regards,
dan carpenter
Re: [PATCH v4 2/2] staging: greybus: resolved checkpatch checks for light.c
Posted by Dan Carpenter 3 weeks, 4 days ago
On Sat, Mar 07, 2026 at 02:09:25PM +0000, Rachit Dhar wrote:
> Added comments to mutex declarations, to resolve the associated checkpatch.pl checks:
> 
> CHECK: struct mutex definition without comment
> +       struct mutex                    lock;
> 
> CHECK: struct mutex definition without comment
> +       struct mutex            lights_lock;
> 
> Signed-off-by: Rachit Dhar <rchtdhr@gmail.com>
> ---

This is a v4 so there should be a explanation of what changed since v3.

https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/

regards,
dan carpenter