added comments on spinlocks for producer-consumer model, rearranged the lines on function calls where it should not end with "(" this bracket, also removed white-spaces and aligned the arguments of function calls.
Signed-off-by: Rujra Bhatt braker.noob.kernel@gmail.com
8------------------------------------------------------8<
drivers/greybus/gb-beagleplay.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 473ac3f2d382..fa1c3a40dd0b 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -73,7 +73,9 @@ struct gb_beagleplay { struct gb_host_device *gb_hd;
struct work_struct tx_work; + //used to ensure that only one producer can access the shared resource at a time. spinlock_t tx_producer_lock; + //used to ensure that only one consumer can access the shared resource at a time. spinlock_t tx_consumer_lock; struct circ_buf tx_circ_buf; u16 tx_crc; @@ -642,8 +644,8 @@ static int cc1352_bootloader_wait_for_ack(struct gb_beagleplay *bg) { int ret;
- ret = wait_for_completion_timeout( - &bg->fwl_ack_com, msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); + ret = wait_for_completion_timeout(&bg->fwl_ack_com, + msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire ack semaphore"); @@ -680,9 +682,8 @@ static int cc1352_bootloader_get_status(struct gb_beagleplay *bg) if (ret < 0) return ret;
- ret = wait_for_completion_timeout( - &bg->fwl_cmd_response_com, - msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); + ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com, + msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); @@ -765,9 +766,8 @@ static int cc1352_bootloader_crc32(struct gb_beagleplay *bg, u32 *crc32) if (ret < 0) return ret;
- ret = wait_for_completion_timeout( - &bg->fwl_cmd_response_com, - msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); + ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com, + msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); -- 2.43.0
On 4/16/25 17:47, rujra wrote:
added comments on spinlocks for producer-consumer model, rearranged the lines on function calls where it should not end with "(" this bracket, also removed white-spaces and aligned the arguments of function calls.
Are these manual adjustments, or using clang-format?
I do not care about formatting being "readable". As long as it can be done by a tool like clang-format, that's fine with me.
Of course if you are fixing some checkpatch error, that is okay, but if now, please avoid formatting changes.
The comments are fine. Although you probably want to add a space between `//` and the sentence start.
Signed-off-by: Rujra Bhatt braker.noob.kernel@gmail.com
8------------------------------------------------------8<
drivers/greybus/gb-beagleplay.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 473ac3f2d382..fa1c3a40dd0b 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -73,7 +73,9 @@ struct gb_beagleplay { struct gb_host_device *gb_hd;
struct work_struct tx_work;
//used to ensure that only one producer can access the sharedresource at a time. spinlock_t tx_producer_lock;
//used to ensure that only one consumer can access the sharedresource at a time. spinlock_t tx_consumer_lock; struct circ_buf tx_circ_buf; u16 tx_crc; @@ -642,8 +644,8 @@ static int cc1352_bootloader_wait_for_ack(struct gb_beagleplay *bg) { int ret;
ret = wait_for_completion_timeout(&bg->fwl_ack_com, msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_ack_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire ack semaphore"); @@ -680,9 +682,8 @@ static int cc1352_bootloader_get_status(struct gb_beagleplay *bg) if (ret < 0) return ret;
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); @@ -765,9 +766,8 @@ static int cc1352_bootloader_crc32(struct gb_beagleplay *bg, u32 *crc32) if (ret < 0) return ret;
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); -- 2.43.0
Best Regards,
Ayush Singh
On Wed, 16 Apr 2025, Ayush Singh wrote:
On 4/16/25 17:47, rujra wrote:
added comments on spinlocks for producer-consumer model, rearranged the lines on function calls where it should not end with "(" this bracket, also removed white-spaces and aligned the arguments of function calls.
Are these manual adjustments, or using clang-format?
I do not care about formatting being "readable". As long as it can be done by a tool like clang-format, that's fine with me.
Of course if you are fixing some checkpatch error, that is okay, but if now, please avoid formatting changes.
The comments are fine. Although you probably want to add a space between `//` and the sentence start.
I don't think the kernel commonly uses // for comments.
julia
Signed-off-by: Rujra Bhatt braker.noob.kernel@gmail.com
8------------------------------------------------------8<
drivers/greybus/gb-beagleplay.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 473ac3f2d382..fa1c3a40dd0b 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -73,7 +73,9 @@ struct gb_beagleplay { struct gb_host_device *gb_hd;
struct work_struct tx_work;
//used to ensure that only one producer can access the sharedresource at a time. spinlock_t tx_producer_lock;
//used to ensure that only one consumer can access the sharedresource at a time. spinlock_t tx_consumer_lock; struct circ_buf tx_circ_buf; u16 tx_crc; @@ -642,8 +644,8 @@ static int cc1352_bootloader_wait_for_ack(struct gb_beagleplay *bg) { int ret;
ret = wait_for_completion_timeout(&bg->fwl_ack_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_ack_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire ack semaphore"); @@ -680,9 +682,8 @@ static int cc1352_bootloader_get_status(struct gb_beagleplay *bg) if (ret < 0) return ret;
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); @@ -765,9 +766,8 @@ static int cc1352_bootloader_crc32(struct gb_beagleplay *bg, u32 *crc32) if (ret < 0) return ret;
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); -- 2.43.0
Best Regards,
Ayush Singh
On 4/16/25 19:15, Julia Lawall wrote:
On Wed, 16 Apr 2025, Ayush Singh wrote:
On 4/16/25 17:47, rujra wrote:
added comments on spinlocks for producer-consumer model, rearranged the lines on function calls where it should not end with "(" this bracket, also removed white-spaces and aligned the arguments of function calls.
Are these manual adjustments, or using clang-format?
I do not care about formatting being "readable". As long as it can be done by a tool like clang-format, that's fine with me.
Of course if you are fixing some checkpatch error, that is okay, but if now, please avoid formatting changes.
The comments are fine. Although you probably want to add a space between `//` and the sentence start.
I don't think the kernel commonly uses // for comments.
julia
Ahh, right. `/* */` is for C comments. So that needs to be fixed as well.
Signed-off-by: Rujra Bhatt braker.noob.kernel@gmail.com
8------------------------------------------------------8<
drivers/greybus/gb-beagleplay.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 473ac3f2d382..fa1c3a40dd0b 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -73,7 +73,9 @@ struct gb_beagleplay { struct gb_host_device *gb_hd;
struct work_struct tx_work;
//used to ensure that only one producer can access the sharedresource at a time. spinlock_t tx_producer_lock;
//used to ensure that only one consumer can access the sharedresource at a time. spinlock_t tx_consumer_lock; struct circ_buf tx_circ_buf; u16 tx_crc; @@ -642,8 +644,8 @@ static int cc1352_bootloader_wait_for_ack(struct gb_beagleplay *bg) { int ret;
ret = wait_for_completion_timeout(&bg->fwl_ack_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_ack_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire ack semaphore"); @@ -680,9 +682,8 @@ static int cc1352_bootloader_get_status(struct gb_beagleplay *bg) if (ret < 0) return ret;
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); @@ -765,9 +766,8 @@ static int cc1352_bootloader_crc32(struct gb_beagleplay *bg, u32 *crc32) if (ret < 0) return ret;
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT));
ret = wait_for_completion_timeout(&bg->fwl_cmd_response_com,msecs_to_jiffies(CC1352_BOOTLOADER_TIMEOUT)); if (ret < 0) return dev_err_probe(&bg->sd->dev, ret, "Failed to acquire last status semaphore"); -- 2.43.0
Best Regards,
Ayush Singh
Best Regards,
Ayush Singh
greybus-dev mailing list -- greybus-dev@lists.linaro.org To unsubscribe send an email to greybus-dev-leave@lists.linaro.org
On Wed, Apr 16, 2025 at 05:47:41PM +0530, rujra wrote:
added comments on spinlocks for producer-consumer model, rearranged the lines on function calls where it should not end with "(" this bracket, also removed white-spaces and aligned the arguments of function calls.
Signed-off-by: Rujra Bhatt braker.noob.kernel@gmail.com
You're doing too many things in one patch, the patch is white space damaged, and the patch prefix is wrong since this driver does not live in staging.
If you want to practise creating patches, please make sure to work in drivers/staging where changes like these may be accepted.
Johan
hi team, I have acknowledge the feedback and sure will try to change the file inside drivers/staging directory. thanking you, regards, rujra bhatt
On Wed, 16 Apr 2025, 7:21 pm Johan Hovold, johan@kernel.org wrote:
On Wed, Apr 16, 2025 at 05:47:41PM +0530, rujra wrote:
added comments on spinlocks for producer-consumer model, rearranged the lines on function calls where it should not end with "(" this bracket, also removed white-spaces and aligned the arguments of function calls.
Signed-off-by: Rujra Bhatt braker.noob.kernel@gmail.com
You're doing too many things in one patch, the patch is white space damaged, and the patch prefix is wrong since this driver does not live in staging.
If you want to practise creating patches, please make sure to work in drivers/staging where changes like these may be accepted.
Johan
On 4/16/2025 7:00 AM, rujra wrote:
hi team, I have acknowledge the feedback and sure will try to change the file inside drivers/staging directory. thanking you, regards, rujra bhatt
There is a lot to learn!
Part of the learning is guidance on responding to review comments. Please read: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#no-mi... https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-t...
/jeff
-
Ayush Singh -
Ayush Singh -
Jeff Johnson -
Johan Hovold -
Julia Lawall -
rujra