[PATCH v4 0/2] staging: greybus: fixing checkpatch issues

[PATCH v4 0/2] staging: greybus: fixing checkpatch issues

[Rachit Dhar]

This series fixes some of the checkpatch.pl checks
highlighted in greybus.

Changes in v4:
- As per the suggestions of Greg K.H., I am removing two patches
that added comments to mutex declarations, since it is hard to
verify them, without extensive analysis and documentation.
- The patches were sent out-of-order previously, so will
try sending them all together this time.

Changes in v3:
- Added cover letter detailing changes made in subsequent versions of
patch-sets.
- v2 patch-set was only emailed to maintainers. The email was forwarded
to the lists, but this rendered the patch-set corrupted. v3 hopes to
correct this.

Changes in v2:
- Split patch 1 into two individual patches 1/4 and 2/4 to keep logical
changes separate.

Rachit Dhar (2):
  staging: greybus: fixed styling issue in fw-management.c
  staging: greybus: resolved checkpatch checks for light.c

 drivers/staging/greybus/fw-management.c | 3 ++-
 drivers/staging/greybus/light.c         | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v4 1/2] staging: greybus: fixed styling issue in fw-management.c

[Rachit Dhar]

Fixed a styling issue highlighted by checkpatch.pl, that was occurring due to improper alignment of function parameters.

CHECK: Alignment should match open parenthesis
+               ret = fw_mgmt_load_and_validate_operation(fw_mgmt,
+                               intf_load.load_method, intf_load.firmware_tag);

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

diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index 5d01da6b6eb9..e20e9136c106 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -434,7 +434,8 @@ static int fw_mgmt_ioctl(struct fw_mgmt *fw_mgmt, unsigned int cmd,
 			return -EFAULT;
 
 		ret = fw_mgmt_load_and_validate_operation(fw_mgmt,
-				intf_load.load_method, intf_load.firmware_tag);
+							  intf_load.load_method,
+							  intf_load.firmware_tag);
 		if (ret)
 			return ret;
 
-- 
2.43.0

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

[Rachit Dhar]

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

[Dan Carpenter]

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

[Dan Carpenter]

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

Re: [PATCH v4 0/2] staging: greybus: fixing checkpatch issues

[Greg Kroah-Hartman]

On Sat, Mar 07, 2026 at 02:09:23PM +0000, Rachit Dhar wrote:
> This series fixes some of the checkpatch.pl checks
> highlighted in greybus.
> 
> Changes in v4:
> - As per the suggestions of Greg K.H., I am removing two patches
> that added comments to mutex declarations, since it is hard to
> verify them, without extensive analysis and documentation.

You still sent a patch that did this, as patch 2/2 :(