greybus-dev

greybus-dev@lists.linaro.org

13 participants
774 discussions

[PATCH] greybus: gb-beagleplay: bound bootloader receive buffering

by Pengpeng Hou

cc1352_bootloader_rx() appends each serdev chunk into the fixed rx_buffer before parsing bootloader packets. The helper can keep leftover bytes between callbacks and may receive multiple packets in one callback, so a single count value is not constrained by one packet length. Check that the incoming chunk fits in the remaining receive buffer space before memcpy(). If it does not, drop the staged data and consume the bytes instead of overflowing rx_buffer. Fixes: 0cf7befa3ea2 ("greybus: gb-beagleplay: Add firmware upload API") Cc: stable(a)vger.kernel.org Signed-off-by: Pengpeng Hou <pengpeng(a)iscas.ac.cn> --- drivers/greybus/gb-beagleplay.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 87186f891a6a..e70787146c4f 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -535,6 +535,13 @@ static size_t cc1352_bootloader_rx(struct gb_beagleplay *bg, const u8 *data, int ret; size_t off = 0; + if (count > sizeof(bg->rx_buffer) - bg->rx_buffer_len) { + dev_warn(&bg->sd->dev, + "dropping oversized bootloader receive chunk"); + bg->rx_buffer_len = 0; + return count; + } + memcpy(bg->rx_buffer + bg->rx_buffer_len, data, count); bg->rx_buffer_len += count; -- 2.50.1 (Apple Git-155)

1 month, 1 week

[PATCH v2 0/2] staging: use bounded formatting helpers in fbtft/greybus

by Yug Merabtene

This small cleanup series replaces open-coded sprintf() usage in a set of staging drivers with helpers intended for bounded and sysfs-safe formatting. Patch 1 updates fbtft logging strings to use scnprintf(). Patch 2 converts Greybus sysfs show paths to sysfs_emit(), including normalizing attributes that were missing a trailing newline. Changes in v2: - resend with corrected author/sender identity (yug.merabtene(a)gmail.com) - no code changes compared to v1 Yug Merabtene (2): staging: fbtft: use scnprintf() for log strings staging: greybus: switch sysfs show paths to sysfs_emit() drivers/staging/fbtft/fbtft-core.c | 8 +++++--- drivers/staging/greybus/arche-apb-ctrl.c | 12 ++++++------ drivers/staging/greybus/arche-platform.c | 10 +++++----- drivers/staging/greybus/audio_manager_module.c | 12 ++++++------ drivers/staging/greybus/gbphy.c | 2 +- drivers/staging/greybus/light.c | 4 ++-- drivers/staging/greybus/loopback.c | 14 +++++++------- 7 files changed, 32 insertions(+), 30 deletions(-) -- 2.34.1

1 month, 2 weeks

[PATCH v3] greybus: es2: drop redundant device reference

by Johan Hovold

Driver core holds a reference to the USB interface and its parent USB device while the interface is bound to a driver and there is no need to take additional references unless the structures are needed after disconnect. Drop the redundant device reference to reduce cargo culting, make it easier to spot drivers where an extra reference is needed, and reduce the risk of memory leaks when drivers fail to release it. Signed-off-by: Johan Hovold <johan(a)kernel.org> --- This one needs one more spin... Sorry about the mess up. Johan Changes in v3: - drop the leftover usb_dev_put() in early error paths Changes in v2: - drop temporary udev variable as reported by the kernel test robot (W=1 warning) drivers/greybus/es2.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/greybus/es2.c b/drivers/greybus/es2.c index 6ae0ac828afa..75b889fa21b4 100644 --- a/drivers/greybus/es2.c +++ b/drivers/greybus/es2.c @@ -772,7 +772,6 @@ static int check_urb_status(struct urb *urb) static void es2_destroy(struct es2_ap_dev *es2) { - struct usb_device *udev; struct urb *urb; int i; @@ -804,10 +803,7 @@ static void es2_destroy(struct es2_ap_dev *es2) gb_hd_cport_release_reserved(es2->hd, ES2_CPORT_CDSI1); gb_hd_cport_release_reserved(es2->hd, ES2_CPORT_CDSI0); - udev = es2->usb_dev; gb_hd_put(es2->hd); - - usb_put_dev(udev); } static void cport_in_callback(struct urb *urb) @@ -1257,11 +1253,10 @@ static int ap_probe(struct usb_interface *interface, bool bulk_in_found = false; bool arpc_in_found = false; - udev = usb_get_dev(interface_to_usbdev(interface)); + udev = interface_to_usbdev(interface); num_cports = apb_get_cport_count(udev); if (num_cports < 0) { - usb_put_dev(udev); dev_err(&udev->dev, "Cannot retrieve CPort count: %d\n", num_cports); return num_cports; @@ -1269,10 +1264,8 @@ static int ap_probe(struct usb_interface *interface, hd = gb_hd_create(&es2_driver, &udev->dev, ES2_GBUF_MSG_SIZE_MAX, num_cports); - if (IS_ERR(hd)) { - usb_put_dev(udev); + if (IS_ERR(hd)) return PTR_ERR(hd); - } es2 = hd_to_es2(hd); es2->hd = hd; -- 2.52.0

1 month, 2 weeks

[PATCH] staging: greybus: audio_manager: Add missing newline to sysfs_emit outputs

by Shivam Gupta

sysfs_emit outputs in audio_manager_module.c do not include a terminating newline, which is required for proper sysfs formatting. Add newline characters to all sysfs_emit format strings. Signed-off-by: Shivam Gupta <shivgupta751157(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.34.1

1 month, 2 weeks

[PATCH v2] staging: greybus: audio: fix error message for BTN_3 button

by Haoyu Lu

In gbaudio_init_jack(), when setting SND_JACK_BTN_3 key, the error message incorrectly says "Failed to set BTN_0". This should be "Failed to set BTN_3" to match the button being configured. Signed-off-by: Haoyu Lu <hechushiguitu666(a)gmail.com> Reviewed-by: Johan Hovold <johan(a)kernel.org> --- drivers/staging/greybus/audio_codec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index 444c53b4e08d..720aa752e17e 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -781,7 +781,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info *module, ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN); if (ret) { - dev_err(module->dev, "Failed to set BTN_0\n"); + dev_err(module->dev, "Failed to set BTN_3\n"); goto free_jacks; } } -- 2.17.1

1 month, 2 weeks

[PATCH] staging: greybus: audio: fix error message for BTN_3 button

by Haoyu Lu

In gbaudio_init_jack(), when setting SND_JACK_BTN_3 key, the error message incorrectly says "Failed to set BTN_0". This should be "Failed to set BTN_3" to match the button being configured. Signed-off-by: Haoyu Lu <hechushiguitu666(a)gmail.com> --- drivers/staging/greybus/audio_codec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c index 444c53b4e08d..720aa752e17e 100644 --- a/drivers/staging/greybus/audio_codec.c +++ b/drivers/staging/greybus/audio_codec.c @@ -781,7 +781,7 @@ static int gbaudio_init_jack(struct gbaudio_module_info *module, ret = snd_jack_set_key(module->button.jack.jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN); if (ret) { - dev_err(module->dev, "Failed to set BTN_0\n"); + dev_err(module->dev, "Failed to set BTN_3\n"); goto free_jacks; } } -- 2.17.1

1 month, 2 weeks

[PATCH] staging: greybus: camera: use scnprintf() for debugfs buffers

by Yug Merabtene

Signed-off-by: Yug Merabtene <yug.merabtene(a)gmail.com> --- drivers/staging/greybus/camera.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c index 62b55bb28408..2e4968d206d3 100644 --- a/drivers/staging/greybus/camera.c +++ b/drivers/staging/greybus/camera.c @@ -889,9 +889,15 @@ static ssize_t gb_camera_debugfs_capabilities(struct gb_camera *gcam, for (i = 0; i < size; i += 16) { unsigned int nbytes = min_t(unsigned int, size - i, 16); + size_t remain; - buffer->length += sprintf(buffer->data + buffer->length, - "%*ph\n", nbytes, caps + i); + remain = PAGE_SIZE - buffer->length; + if (!remain) + break; + + buffer->length += scnprintf(buffer->data + buffer->length, + remain, "%*ph\n", nbytes, + caps + i); } done: @@ -973,17 +979,19 @@ static ssize_t gb_camera_debugfs_configure_streams(struct gb_camera *gcam, if (ret < 0) goto done; - buffer->length = sprintf(buffer->data, "%u;%u;", nstreams, flags); + buffer->length = scnprintf(buffer->data, PAGE_SIZE, "%u;%u;", nstreams, flags); for (i = 0; i < nstreams; ++i) { struct gb_camera_stream_config *stream = &streams[i]; - buffer->length += sprintf(buffer->data + buffer->length, - "%u;%u;%u;%u;%u;%u;%u;", - stream->width, stream->height, - stream->format, stream->vc, - stream->dt[0], stream->dt[1], - stream->max_size); + if (buffer->length < PAGE_SIZE) + buffer->length += scnprintf(buffer->data + buffer->length, + PAGE_SIZE - buffer->length, + "%u;%u;%u;%u;%u;%u;%u;", + stream->width, stream->height, + stream->format, stream->vc, + stream->dt[0], stream->dt[1], + stream->max_size); } ret = len; @@ -1046,7 +1054,7 @@ static ssize_t gb_camera_debugfs_flush(struct gb_camera *gcam, if (ret < 0) return ret; - buffer->length = sprintf(buffer->data, "%u", req_id); + buffer->length = scnprintf(buffer->data, PAGE_SIZE, "%u", req_id); return len; } -- 2.34.1

1 month, 2 weeks

[PATCH] greybus/uart: fix misaligned wait_for_completion_timeout

by loren coomer

Align the second line of wait_for_completion_timeout() with the opening parenthesis. Signed-off-by: Loren Coomer <lorencoom(a)gmail.com>

1 month, 2 weeks

[RFC] greybus: support combined Host + SVC

by Ayush Singh

Hello everyone, I’ve recently been experimenting with chaining multiple Greybus nodes to a single host over I²C (QWIIC port [0]). This general idea is not new (see MicroMod Qwiic Pro Kit [1]), although those setups do not use Greybus. Since I²C does not allow a slave to initiate transfers, it is not well suited for node-initiated events (e.g. interrupts, SVC-initiated control). However, for my current use case I am primarily interested in polling-based functionality, so this limitation is acceptable. In a typical Greybus topology, an Application Processor (AP), an SVC, and one or more modules are connected via UniPro. In practice, because most application processors lack a native UniPro interface, they connect through a bridge device that also implements the SVC. For the I²C-based setup described above, I have considered the following topologies: 1. Separate co-processor (SVC/Bridge) This approach is reasonable on SoCs such as TI AM6254, which include an M4F core that can serve as the SVC/bridge and control the I²C bus. However, on devices like TI AM62L, which lack such a core, introducing an additional processor solely for Greybus does not seem justified. Also, this would make the above much less portable, since it expects a hardware component, not all BeagleBoard.org boards have. ``` +----+ +---------------+ +----------+ | AP | <--- bus ----> | SVC / Bridge | <--- I2C ----> | Module | +----+ +---------------+ +----------+ | | +----------+ `----------- I2C ----> | Module | +----------+ ``` 2. SVC per node Each node implements its own SVC. Since an I²C slave cannot initiate communication, the AP must already know the address of each SVC/module. This also seems inefficient when chaining multiple nodes. ``` +----+ +----------------+ | AP | <--- I2C ---> | SVC / Module | +----+ +----------------+ | | +----------------+ `---- I2C ----> | SVC / Module | +----------------+ ``` 3. SVC/Bridge functionality inside the AP For this use case, this seems to be the most practical option. To clarify, I am not proposing any new data paths in the Greybus pipeline. The idea is to have a reusable an SVC/bridge implementation similar to what exists in greybus-zephyr [2][3], but hosted within the AP. ``` +-------------+ +-----------+ | AP / SVC | <--- I2C ---> | Module | +-------------+ +-----------+ | | +----------+ `-- I2C ---> | Module | +----------+ ``` Before proceeding further, I would appreciate feedback on this approach—particularly whether there are concerns with option 3, or if there are alternative designs I should consider. Best regards, Ayush Singh [0]: https://www.sparkfun.com/qwiic [1]: https://www.digikey.in/en/maker/projects/micromod-qwiic-pro-kit-project-gui… [2]: https://github.com/beagleboard/greybus-zephyr/blob/main/subsys/greybus/svc.… [3]: https://github.com/beagleboard/greybus-zephyr/blob/main/subsys/greybus/apbr…

1 month, 2 weeks

[PATCH v4 1/2] greybus: raw: fix use-after-free on cdev close

by Damien Riégel

This addresses a use-after-free bug when a raw bundle is disconnected but its chardev is still opened by an application. When the application releases the cdev, it causes the following panic when init on free is enabled (CONFIG_INIT_ON_FREE_DEFAULT_ON=y): refcount_t: underflow; use-after-free. WARNING: CPU: 0 PID: 139 at lib/refcount.c:28 refcount_warn_saturate+0xd0/0x130 ... Call Trace: <TASK> cdev_put+0x18/0x30 __fput+0x255/0x2a0 __x64_sys_close+0x3d/0x80 do_syscall_64+0xa4/0x290 entry_SYSCALL_64_after_hwframe+0x77/0x7f The cdev is contained in the "gb_raw" structure, which is freed in the disconnect operation. When the cdev is released at a later time, cdev_put gets an address that points to freed memory. To fix this use-after-free, convert the struct device from a pointer to being embedded, that makes the lifetime of the cdev and of this device the same. Then, use cdev_device_add, which guarantees that the device won't be released until all references to the cdev have been released. Finally, delegate the freeing of the structure to the device release function, instead of freeing immediately in the disconnect callback. Fixes: e806c7fb8e9b ("greybus: raw: add raw greybus kernel driver") Reviewed-by: Johan Hovold <johan(a)kernel.org> Signed-off-by: Damien Riégel <damien.riegel(a)silabs.com> --- Changes in v4: - rebase on v7.0-rc4 -> change kzalloc into kzalloc_obj Changes in v3: - move assignment of raw->dev.parent - add Reviewed-By: Johan Hovold Changes in v2: - trim down trace in commit message to keep only the essential part - rework error paths in probe function to ensure device is always freed (set device release callback before any call to put_device) - move ida_free to release callback drivers/staging/greybus/raw.c | 69 +++++++++++++++++------------------ 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 3027a2c25bcd..47a984554681 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -21,9 +21,8 @@ struct gb_raw { struct list_head list; int list_data; struct mutex list_lock; - dev_t dev; struct cdev cdev; - struct device *device; + struct device dev; }; struct raw_data { @@ -148,6 +147,15 @@ static int gb_raw_send(struct gb_raw *raw, u32 len, const char __user *data) return retval; } +static void raw_dev_release(struct device *dev) +{ + struct gb_raw *raw = container_of(dev, struct gb_raw, dev); + + ida_free(&minors, MINOR(raw->dev.devt)); + + kfree(raw); +} + static int gb_raw_probe(struct gb_bundle *bundle, const struct greybus_bundle_id *id) { @@ -164,15 +172,30 @@ static int gb_raw_probe(struct gb_bundle *bundle, if (cport_desc->protocol_id != GREYBUS_PROTOCOL_RAW) return -ENODEV; - raw = kzalloc_obj(*raw); - if (!raw) + minor = ida_alloc(&minors, GFP_KERNEL); + if (minor < 0) + return minor; + + raw = kzalloc_obj(*raw, GFP_KERNEL); + if (!raw) { + ida_free(&minors, minor); return -ENOMEM; + } + + device_initialize(&raw->dev); + raw->dev.devt = MKDEV(raw_major, minor); + raw->dev.class = &raw_class; + raw->dev.parent = &bundle->dev; + raw->dev.release = raw_dev_release; + retval = dev_set_name(&raw->dev, "gb!raw%d", minor); + if (retval) + goto error_put_device; connection = gb_connection_create(bundle, le16_to_cpu(cport_desc->id), gb_raw_request_handler); if (IS_ERR(connection)) { retval = PTR_ERR(connection); - goto error_free; + goto error_put_device; } INIT_LIST_HEAD(&raw->list); @@ -181,46 +204,26 @@ static int gb_raw_probe(struct gb_bundle *bundle, raw->connection = connection; greybus_set_drvdata(bundle, raw); - minor = ida_alloc(&minors, GFP_KERNEL); - if (minor < 0) { - retval = minor; - goto error_connection_destroy; - } - - raw->dev = MKDEV(raw_major, minor); cdev_init(&raw->cdev, &raw_fops); retval = gb_connection_enable(connection); if (retval) - goto error_remove_ida; + goto error_connection_destroy; - retval = cdev_add(&raw->cdev, raw->dev, 1); + retval = cdev_device_add(&raw->cdev, &raw->dev); if (retval) goto error_connection_disable; - raw->device = device_create(&raw_class, &connection->bundle->dev, - raw->dev, raw, "gb!raw%d", minor); - if (IS_ERR(raw->device)) { - retval = PTR_ERR(raw->device); - goto error_del_cdev; - } - return 0; -error_del_cdev: - cdev_del(&raw->cdev); - error_connection_disable: gb_connection_disable(connection); -error_remove_ida: - ida_free(&minors, minor); - error_connection_destroy: gb_connection_destroy(connection); -error_free: - kfree(raw); +error_put_device: + put_device(&raw->dev); return retval; } @@ -231,11 +234,8 @@ static void gb_raw_disconnect(struct gb_bundle *bundle) struct raw_data *raw_data; struct raw_data *temp; - // FIXME - handle removing a connection when the char device node is open. - device_destroy(&raw_class, raw->dev); - cdev_del(&raw->cdev); + cdev_device_del(&raw->cdev, &raw->dev); gb_connection_disable(connection); - ida_free(&minors, MINOR(raw->dev)); gb_connection_destroy(connection); mutex_lock(&raw->list_lock); @@ -244,8 +244,7 @@ static void gb_raw_disconnect(struct gb_bundle *bundle) kfree(raw_data); } mutex_unlock(&raw->list_lock); - - kfree(raw); + put_device(&raw->dev); } /* -- 2.52.0

1 month, 2 weeks

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev