greybus-dev August 2026

greybus-dev@lists.linaro.org

20 participants
25 discussions

[PATCH 0/7] Add Greybus Sotfsvc and UART Node drivers

by Ayush Singh

A Greybus network needs an SVC (Supervisory Controller) to bring interfaces up, assign device IDs and connect CPorts to the AP. On a UniPro network the SVC is a real entity on the bus. On transports that merely carry Greybus messages - a UART, an I2C bus, a network link - nothing on the wire plays that role, so the SVC has to be emulated somewhere: in a user-space bridge (gbridge), in coprocessor firmware (cc1352p7 in gb-beagleplay), or open-coded inside the host driver itself. This series moves the emulation into the kernel and makes it shared. This was discussed in a prior RFC as well [2], but to keep things short, removing the need for an external SVC greatly simplifies the greybus network setup when nodes are directly connected over common transports such as UART, I2C etc. gb-uart-node gb-softsvc greybus core (transport, serdev) <-> (SVC + host device) <-> (bundles, protocols) NodeOps module_insert() submit_message() module_remove() gb-softsvc registers itself as a Greybus host device and answers the SVC-side operations the core expects during interface bring-up and teardown. A host driver implements the NodeOps trait to push data towards its node, calls module_insert() to announce a new node, submit_message() to hand incoming Greybus messages back to the core, and module_remove() on disconnect. The first user is gb-uart-node, a driver for Greybus nodes attached over a plain serial port. Framing is HDLC, with a one-byte address (0x01 for Greybus) and control byte, followed by the 16-bit CPort ID and the Greybus message. No SVC firmware is required on the far end, so the node can be a bare microcontroller speaking Greybus - a BeagleConnect Freedom over its serial link, in this case. The testing is performed with greybus-zephyr [0] implementation. Both drivers are written in Rust, which is why the middle of the series is abstractions rather than drivers. Only the APIs these two drivers need are covered: protocols.rs abstracts the SVC-facing parts of greybus_protocols.h, and types such as Greybus Interface do not implement AlwaysRefCounted yet. The intent is to grow this as more Rust host drivers appear rather than to abstract the whole subsystem up front. Patches 1 and 2 are small C-side preparations to the Greybus core: exporting gb_connection_get()/gb_connection_put() and adding gb_connection_hd_find_by_intf(), a lookup by remote interface and CPort id for callers that only know the far end of a connection. Patch 3 adds a CRC-CCITT abstraction, needed for HDLC frame checks. Patch 4 adds the Greybus abstractions, patches 5 and 6 the two drivers, and patch 7 the device tree binding for BeagleConnect Freedom. Open questions *************** - gb-uart-node imports types from gb-softsvc, so the series carries the Rust-to-Rust cross-module calling setup from nova-core [1]: gb_softsvc_exports.c plus the Makefile plumbing that emits crate metadata and generates the export list. This is a workaround for the build system not supporting Rust cross-module dependencies natively, and it should go away once that lands. - Connection create/destroy and interface activate/resume in gb-softsvc currently just acknowledge the request. Callbacks into NodeOps can be added when a transport actually needs to act on them; I did not want to invent an interface without a user. - Zerocopy is currently not being used in grebeybus/protocols.rs. They cannot be derived yet since types generated by bindgen do not have them, and it seems explicitly forbindden to manually impl the traits. So using old traits from transmute. - The bindings are supposed to be created for actual device, but any MCU that supports Zephyr, can run the greybus-zephyr firmware with UART transport. So not sure if adding a beagle,beagleconnect-freedom compatible is the correct choice here. - The individual patches can be spun off into independent patch series if required. The reason for this single patch series is to provide a complete picture of usage. - Since gb-softsvc currently is not being used from a C driver, no C API is provided. However, if required, it can be added. - Writing to UART from gb-uart-node is currently a bit broken. I am not quite sure what the safe way is to go from a non-bound device to a bound device. Any suggestions on this front are welcome. - I am not sure if Rust abstractions should have a seperate entry in MAINTAINERS with me as the maintainer, or if they should just be added to the respective subsystem entries. [0]: https://github.com/beagleboard/greybus-zephyr [1]: https://lore.kernel.org/all/20260622-nova-exports-v5-0-6191773fc977@nvidia.… [2]: https://lore.kernel.org/all/ecca8eb2-8e5a-4770-bcf6-3fb49773088b@beagleboar… Signed-off-by: Ayush Singh <ayush(a)beagleboard.org> --- Ayush Singh (7): greybus: connection: Export gb_connection_get() and gb_connection_put() greybus: connection: Add gb_connection_hd_find_by_intf() rust: crc_ccitt: add CRC-CCITT abstraction rust: kernel: Add greybus abstractions drivers: greybus: Add software SVC implementation greybus: Add Rust UART node driver dt-bindings: beagle: Add BeagleConnect Freedom .../beagle/beagle,beagleconnect-freedom.yaml | 30 ++ MAINTAINERS | 15 + drivers/greybus/.gitignore | 1 + drivers/greybus/Kconfig | 27 ++ drivers/greybus/Makefile | 50 +++ drivers/greybus/connection.c | 28 +- drivers/greybus/gb_softsvc.rs | 472 +++++++++++++++++++++ drivers/greybus/gb_softsvc_exports.c | 15 + drivers/greybus/gb_uart_node.rs | 231 ++++++++++ include/linux/greybus/connection.h | 6 + lib/crc/Kconfig | 7 + rust/bindings/bindings_helper.h | 2 + rust/kernel/crc_ccitt.rs | 26 ++ rust/kernel/greybus/hd.rs | 315 ++++++++++++++ rust/kernel/greybus/mod.rs | 230 ++++++++++ rust/kernel/greybus/protocols.rs | 392 +++++++++++++++++ rust/kernel/lib.rs | 4 + 17 files changed, 1849 insertions(+), 2 deletions(-) --- base-commit: 6b8c8af514d739d0335f5579b585e02babe8a727 change-id: 20260810-gb-uart-transport-9255d6557c4c Best regards, -- Ayush Singh <ayush(a)beagleboard.org>

2 hours, 18 minutes

Re: [PATCH] staging: greybus: spilib: Remove the local unused

by Johan Hovold

On Thu, Aug 20, 2026 at 04:02:35PM +0530, singh.supreet14(a)gmail.com wrote: > From: Supreet Singh <singh.supreet14(a)gmail.com> > > The return value of spi_new_device() is only used to check whether > device creation succeeded. Remove the unnecessary local variable and > check the return value directly. > > Signed-off-by: Supreet Singh <singh.supreet14(a)gmail.com> > --- > drivers/staging/greybus/spilib.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c > index 39d5bf234c61..0e8711fdc429 100644 > --- a/drivers/staging/greybus/spilib.c > +++ b/drivers/staging/greybus/spilib.c > @@ -444,7 +444,6 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs) > struct gb_spi_device_config_request request; > struct gb_spi_device_config_response response; > struct spi_board_info spi_board = { {0} }; > - struct spi_device *spidev; > int ret; > u8 dev_type; > > @@ -480,8 +479,7 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs) > spi_board.chip_select = cs; > spi_board.max_speed_hz = le32_to_cpu(response.max_speed_hz); > > - spidev = spi_new_device(ctlr, &spi_board); > - if (!spidev) > + if (!spi_new_device(ctlr, &spi_board)) > return -EINVAL; No, this change just makes the code harder to read. Johan

18 hours, 31 minutes

Re: [PATCH] staging: greybus: light: initialize channel lock before registration

by Greg Kroah-Hartman

On Wed, Aug 19, 2026 at 09:21:08PM +0800, Runyu Xiao wrote: > You're right, and "tested" was the wrong word in my reply. Sorry, I have no context :( Remember, some of us get thousands of emails a day. > The finding here is from code inspection, not from a runtime test. The > path I checked is: > > gb_lights_channel_register() > -> led_classdev_register() > publishes cdev->brightness_set_blocking > (= gb_brightness_set() > -> __gb_lights_brightness_set() > -> __gb_lights_led_brightness_set() > -> mutex_lock(&channel->lock)) > > but mutex_init(&channel->lock) only runs after > gb_lights_channel_register() returns, and that is the only place where > channel->lock is initialized. So the lock is published before it is > initialized. > > The QEMU module I mentioned does not exercise this driver. It only shows > that taking an uninitialized mutex triggers the expected > DEBUG_LOCKS_WARN_ON(lock->magic != lock), so I should not have described > it as driver testing or as a reproducer for this driver. > > And yes, 6.1.66 was only the kernel version in that local QEMU setup. The > patch itself was made and compile-checked against current mainline, and > I'll keep validation against mainline for future submissions. Just build testing against a many-year-old kernel is a sure way to ensure that nothing will actually work at all :( > The initial report came from PatchProof, a static-analysis pipeline I am > building for lock-API misuse. It flagged this publish-before-init ordering, > and I then manually verified the call path above. I did use an LLM as an > assistant during the workflow, but the finding itself was from the checker > and the patch I sent was based on my manual review of the code. Please read the archives for why we don't take LLM patches for drivers/staging/ unless you can test the change on real hardware. thanks, greg k-h

1 day, 15 hours

Re: [PATCH] staging: greybus: light: initialize channel lock before registration

by Greg Kroah-Hartman

On Wed, Aug 19, 2026 at 08:32:45PM +0800, Runyu Xiao wrote: > On Tue, Aug 18, 2026 at 20:23:10 +0200, Greg Kroah-Hartman wrote: > > How was this found and tested? > > > Found by code inspection: gb_lights_channel_register() publishes the > LED class device and its brightness callback before > mutex_init(&light->channels[i].lock) runs, so a concurrent brightness > update can take an uninitialized mutex. What tool did this inspection? A LLM? > Tested by reproducing that code shape in a minimal out-of-tree module > under QEMU (kernel 6.1.66 with CONFIG_DEBUG_MUTEXES=y and > CONFIG_DEBUG_LOCK_ALLOC=y), since the real driver needs greybus/LED > hardware I don't have. The module kzalloc's a channel, registers > (publishes) it, and runs the brightness callback, which takes the > embedded mutex: > > > - before the fix, mutex_init() runs after register, so the callback > locks the still-zeroed mutex and trips > > > DEBUG_LOCKS_WARN_ON(lock->magic != lock) > WARNING: CPU: 1 PID: 188 at kernel/locking/mutex.c:582 __mutex_lock+0x712/0xd20 > > > - after the fix, mutex_init() runs before register and the same > trigger path produces no warning. But that isn't testing anything at all. And why 6.1.66? That's not what new patches need to be made against, as it's years old. > The real driver hits this as a race (a concurrent brightness update), > whereas the reproducer runs the callback synchronously during registration > to make it deterministic; both leave the callback observing an > uninitialized lock. What "reproducer"? Again, was this done by a LLM? Where is the "reproducer"? thanks, greg k-h

1 day, 16 hours

Re: [PATCH] staging: greybus: light: initialize channel lock before registration

by Greg Kroah-Hartman

1 day, 16 hours

Re: [PATCH] staging: greybus: light: initialize channel lock before registration

by Greg Kroah-Hartman

On Tue, Aug 18, 2026 at 10:20:20PM +0800, Runyu Xiao wrote: > gb_lights_light_register() registers each channel before initializing > light->channels[i].lock. Channel registration exposes the LED class > device and its brightness callback, which later takes this mutex. A > concurrent brightness update can therefore hit an uninitialized lock. > > Initialize the channel mutex before registering the channel so the > callback always sees a valid lock. > > Fixes: cc43368a3cde ("greybus: lights: Control runtime pm suspend/resume on AP side") > Cc: stable(a)vger.kernel.org > Signed-off-by: Runyu Xiao <runyu.xiao(a)seu.edu.cn> > --- > 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 38c233a706c4..056ef6b0276e 100644 > --- a/drivers/staging/greybus/light.c > +++ b/drivers/staging/greybus/light.c > @@ -1043,11 +1043,11 @@ static int gb_lights_light_register(struct gb_light *light) > * found. > */ > for (i = 0; i < light->channels_count; i++) { > + mutex_init(&light->channels[i].lock); > + > ret = gb_lights_channel_register(&light->channels[i]); > if (ret < 0) > return ret; > - > - mutex_init(&light->channels[i].lock); > } > > light->ready = true; How was this found and tested? thanks, greg k-h

2 days, 10 hours

[PATCH] staging: greybus: use sysfs_emit() in show functions

by Malathi

Documentation/filesystems/sysfs.rst recommends that show() callbacks use sysfs_emit() instead of sprintf(). sysfs_emit() knows the sysfs output buffer starts at the beginning of a page and is bounded by PAGE_SIZE, so it guards against accidental buffer overruns. Convert the sprintf() calls in the protocol_id, fade_in, fade_out and color attribute show functions accordingly. No functional change. Signed-off-by: Malathi <malathi.a2000(a)gmail.com> --- drivers/staging/greybus/gbphy.c | 2 +- drivers/staging/greybus/light.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/greybus/gbphy.c b/drivers/staging/greybus/gbphy.c index bdb0f5164a6f..bb9a5b538e6e 100644 --- a/drivers/staging/greybus/gbphy.c +++ b/drivers/staging/greybus/gbphy.c @@ -31,7 +31,7 @@ static ssize_t protocol_id_show(struct device *dev, { struct gbphy_device *gbphy_dev = to_gbphy_dev(dev); - return sprintf(buf, "0x%02x\n", gbphy_dev->cport_desc->protocol_id); + return sysfs_emit(buf, "0x%02x\n", gbphy_dev->cport_desc->protocol_id); } static DEVICE_ATTR_RO(protocol_id); diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c index cab02b5da867..2689f9a7524a 100644 --- a/drivers/staging/greybus/light.c +++ b/drivers/staging/greybus/light.c @@ -173,7 +173,7 @@ static ssize_t fade_##__dir##_show(struct device *dev, \ struct led_classdev *cdev = dev_get_drvdata(dev); \ struct gb_channel *channel = get_channel_from_cdev(cdev); \ \ - return sprintf(buf, "%u\n", channel->fade_##__dir); \ + return sysfs_emit(buf, "%u\n", channel->fade_##__dir); \ } \ \ static ssize_t fade_##__dir##_store(struct device *dev, \ @@ -220,7 +220,7 @@ static ssize_t color_show(struct device *dev, struct device_attribute *attr, struct led_classdev *cdev = dev_get_drvdata(dev); struct gb_channel *channel = get_channel_from_cdev(cdev); - return sprintf(buf, "0x%08x\n", channel->color); + return sysfs_emit(buf, "0x%08x\n", channel->color); } static ssize_t color_store(struct device *dev, struct device_attribute *attr, -- 2.43.0

6 days, 19 hours

[PATCH] staging: greybus: loopback: remove unused includes

by Batu Ada Tutkun

linux/random.h, linux/sizes.h, linux/cdev.h, linux/fs.h, and linux/workqueue.h are not used anywhere in this file; none of their symbols appear in the source, and none are needed transitively either (debugfs helpers come from linux/debugfs.h, the async worker uses kthread_run()/kthread_stop() rather than the workqueue API). Verified by removing the five includes and rebuilding drivers/staging/greybus out-of-tree with W=1: no new warnings or errors, and checkpatch.pl reports the same warning/check count before and after. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Batu Ada Tutkun <batuadatutkun(a)gmail.com> --- drivers/staging/greybus/loopback.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index ea57b1f5d156..1169cfcd28c6 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -14,15 +14,10 @@ #include <linux/slab.h> #include <linux/kthread.h> #include <linux/delay.h> -#include <linux/random.h> -#include <linux/sizes.h> -#include <linux/cdev.h> -#include <linux/fs.h> #include <linux/kfifo.h> #include <linux/debugfs.h> #include <linux/list_sort.h> #include <linux/spinlock.h> -#include <linux/workqueue.h> #include <linux/atomic.h> #include <linux/pm_runtime.h> #include <linux/greybus.h> -- 2.53.0

1 week, 1 day

[PATCH] staging: greybus: remove unused loopback macro

by André Moreira

The gb_loopback_attr macro is no longer used in the Greybus loopback driver. Remove the unused macro to avoid keeping dead code in the driver. Signed-off-by: André Moreira <andrem.33333(a)gmail.com> --- drivers/staging/greybus/loopback.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index ea57b1f5d156..2a623fc2e99b 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -167,32 +167,6 @@ static DEVICE_ATTR_RO(name##_avg) gb_loopback_ro_stats_attr(field, max, u); \ gb_loopback_ro_avg_attr(field) -#define gb_loopback_attr(field, type) \ -static ssize_t field##_show(struct device *dev, \ - struct device_attribute *attr, \ - char *buf) \ -{ \ - struct gb_loopback *gb = dev_get_drvdata(dev); \ - return sysfs_emit(buf, "%" #type "\n", gb->field); \ -} \ -static ssize_t field##_store(struct device *dev, \ - struct device_attribute *attr, \ - const char *buf, \ - size_t len) \ -{ \ - int ret; \ - struct gb_loopback *gb = dev_get_drvdata(dev); \ - mutex_lock(&gb->mutex); \ - ret = sscanf(buf, "%"#type, &gb->field); \ - if (ret != 1) \ - len = -EINVAL; \ - else \ - gb_loopback_check_attr(gb, bundle); \ - mutex_unlock(&gb->mutex); \ - return len; \ -} \ -static DEVICE_ATTR_RW(field) - #define gb_dev_loopback_ro_attr(field) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ -- 2.43.0

1 week, 1 day

[PATCH] staging: greybus: add missing newline to sysfs_emit() calls in audio_manager_module.c

by Raushan Kumar

sysfs attribute show functions are expected to terminate their output with a newline. Add the missing \n to the format strings in gb_audio_module_name_show(), gb_audio_module_vid_show(), gb_audio_module_pid_show(), gb_audio_module_intf_id_show(), gb_audio_module_ip_devices_show(), and gb_audio_module_op_devices_show(), as flagged by checkpatch.pl. Signed-off-by: Raushan Kumar <mitramaurya80(a)gmail.com> --- drivers/staging/greybus/audio_manager_module.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c index dc90cc2d2308..5737f2a32f5a 100644 --- a/drivers/staging/greybus/audio_manager_module.c +++ b/drivers/staging/greybus/audio_manager_module.c @@ -75,7 +75,7 @@ static void gb_audio_module_release(struct kobject *kobj) static ssize_t gb_audio_module_name_show(struct gb_audio_manager_module *module, struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%s", module->desc.name); + return sysfs_emit(buf, "%s\n", module->desc.name); } static struct gb_audio_manager_module_attribute gb_audio_module_name_attribute = @@ -84,7 +84,7 @@ static struct gb_audio_manager_module_attribute gb_audio_module_name_attribute = static ssize_t gb_audio_module_vid_show(struct gb_audio_manager_module *module, struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%d", module->desc.vid); + return sysfs_emit(buf, "%d\n", module->desc.vid); } static struct gb_audio_manager_module_attribute gb_audio_module_vid_attribute = @@ -93,7 +93,7 @@ static struct gb_audio_manager_module_attribute gb_audio_module_vid_attribute = static ssize_t gb_audio_module_pid_show(struct gb_audio_manager_module *module, struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%d", module->desc.pid); + return sysfs_emit(buf, "%d\n", module->desc.pid); } static struct gb_audio_manager_module_attribute gb_audio_module_pid_attribute = @@ -103,7 +103,7 @@ static ssize_t gb_audio_module_intf_id_show(struct gb_audio_manager_module *modu struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "%d", module->desc.intf_id); + return sysfs_emit(buf, "%d\n", module->desc.intf_id); } static struct gb_audio_manager_module_attribute @@ -114,7 +114,7 @@ static ssize_t gb_audio_module_ip_devices_show(struct gb_audio_manager_module *m struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "0x%X", module->desc.ip_devices); + return sysfs_emit(buf, "0x%X\n", module->desc.ip_devices); } static struct gb_audio_manager_module_attribute @@ -125,7 +125,7 @@ static ssize_t gb_audio_module_op_devices_show(struct gb_audio_manager_module *m struct gb_audio_manager_module_attribute *attr, char *buf) { - return sysfs_emit(buf, "0x%X", module->desc.op_devices); + return sysfs_emit(buf, "0x%X\n", module->desc.op_devices); } static struct gb_audio_manager_module_attribute -- 2.55.0

1 week, 1 day

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev August 2026