On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
> Hi,
>
>
> This patchset brings initial support for Silicon Labs CPC protocol,
> standing for Co-Processor Communication. This protocol is used by the
> EFR32 Series [1]. These devices offer a variety for radio protocols,
> such as Bluetooth, Z-Wave, Zigbee [2].
Before we get too deep into the details of the patches, please could
you do a compare/contrast to Greybus.
The core of Greybus is already in the kernel, with some more bits
being in staging. I don't know it too well, but at first glance it
looks very similar. We should not duplicate that.
Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
has support for these, although the code is current in staging. But
for staging code, it is actually pretty good.
Why should we add a vendor implementation when we already appear to
have something which does most of what is needed?
Andrew
Added a detailed comment explaining the role of the mutex
in serializing firmware management ioctl() operations.
The mutex prevents concurrent access to firmware operations
and protects the 'disabled' state flag during disconnection.
Signed-off-by: Sridhar Arra <sridhar.arra(a)yahoo.com>
---
drivers/staging/greybus/fw-management.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index a47385175582..56725b711a17 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -28,6 +28,19 @@ struct fw_mgmt {
/* Common id-map for interface and backend firmware requests */
struct ida id_map;
+ /*
+ * Mutex to serialize firmware management ioctl() operations and
+ * protect against concurrent access.
+ *
+ * Ensures that user-space cannot perform multiple firmware
+ * operations in parallel (e.g., updating interface firmware)
+ * for the same Interface, avoiding race conditions and reducing
+ * code complexity.
+ *
+ * Also protects the 'disabled' state flag, preventing new
+ * operations from starting when the firmware management
+ * connection is being disconnected.
+ */
struct mutex mutex;
struct completion completion;
struct cdev cdev;
--
2.43.0
Correct the alignment of the parameters to match the open parenthesis.
Reported by checkpatch:
CHECK: Alignment should match open parenthesis
Signed-off-by: Andreas Kleinbichler <andi.kleinbichler(a)gmail.com>
---
drivers/staging/greybus/camera.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
index ec9fddfc0b14..a1b76df39a8f 100644
--- a/drivers/staging/greybus/camera.c
+++ b/drivers/staging/greybus/camera.c
@@ -263,9 +263,10 @@ static int gb_camera_get_max_pkt_size(struct gb_camera *gcam,
* Validate the stream configuration response verifying padding is correctly
* set and the returned number of streams is supported
*/
-static const int gb_camera_configure_streams_validate_response(struct gb_camera *gcam,
- struct gb_camera_configure_streams_response *resp,
- unsigned int nstreams)
+static const int gb_camera_configure_streams_validate_response
+ (struct gb_camera *gcam,
+ struct gb_camera_configure_streams_response *resp,
+ unsigned int nstreams)
{
unsigned int i;
--
2.43.0
This patch series splits and improves the previously submitted single patch by
breaking it into four independent, logical changes following kernel submission
etiquette.
Link: https://lore.kernel.org/r/d683962c-e8b7-4adc-9902-483976197637@riscstar.com
Link: https://lore.kernel.org/r/5773d200-1d9d-4d6e-b01e-10d962ee9e8e@quicinc.com
Link: https://lore.kernel.org/r/4f47df18-e98c-4f23-afde-3fa8e9fd0f86@quicinc.com
Link: https://lore.kernel.org/r/202504010829.vIzweYue-lkp@intel.com
Link: https://lore.kernel.org/r/202504011217.iRb2Bbls-lkp@intel.com
All changes are isolated, reviewed, and tested.
Patches included:
1. Replace deprecated strncpy() with strscpy() in firmware.c
2. Replace sprintf() with sysfs_emit() in sysfs show functions
3. Refactor gb_loopback_fn() into smaller helpers
4. Fulfill TODO by splitting get_topology() logic in audio_gb.c
v1 feedback from maintainers highlighted the need to split changes and avoid
unrelated whitespace or formatting edits. These recommendations have been
carefully addressed in this version.
Signed-off-by: Ganesh Kumar Pittala <ganeshkpittala(a)gmail.com>
Ganesh Kumar Pittala (4):
staging: greybus: replace deprecated strncpy with strscpy in
firmware.c
staging: greybus: replace sprintf with sysfs_emit in sysfs show
functions
staging: greybus: refactor gb_loopback_fn into smaller helper
functions
staging: greybus: split gb_audio_gb_get_topology into helper functions
.../greybus/Documentation/firmware/firmware.c | 6 +-
drivers/staging/greybus/arche-apb-ctrl.c | 11 +-
drivers/staging/greybus/arche-platform.c | 11 +-
drivers/staging/greybus/audio_gb.c | 36 +++-
.../staging/greybus/audio_manager_module.c | 13 +-
drivers/staging/greybus/gbphy.c | 3 +-
drivers/staging/greybus/light.c | 5 +-
drivers/staging/greybus/loopback.c | 167 ++++++++++--------
8 files changed, 145 insertions(+), 107 deletions(-)
--
2.43.0
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(a)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