greybus-dev

greybus-dev@lists.linaro.org

1 participants
549 discussions

[PATCH v2] staging: greybus: timesync: validate platform state callback

by Rui Miguel Silva

When tearingdown timesync, and not in arche platform, the state platform callback is not initialized. That will trigger the following NULL dereferencing. CallTrace: ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus] gb_timesync_teardown+0x85/0xc0 [greybus] gb_timesync_svc_remove+0xab/0x190 [greybus] gb_svc_del+0x29/0x110 [greybus] gb_hd_del+0x14/0x20 [greybus] ap_disconnect+0x24/0x60 [gb_es2] usb_unbind_interface+0x7a/0x2c0 __device_release_driver+0x96/0x150 device_release_driver+0x1e/0x30 bus_remove_device+0xe7/0x130 device_del+0x116/0x230 usb_disable_device+0x97/0x1f0 usb_disconnect+0x80/0x260 hub_event+0x5ca/0x10e0 process_one_work+0x126/0x3b0 worker_thread+0x55/0x4c0 ? process_one_work+0x3b0/0x3b0 kthread+0xc4/0xe0 ? kthread_park+0xb0/0xb0 ret_from_fork+0x22/0x30 So, fix that by adding checks before use the callback. Fixes: 970dc85bd95d ("greybus: timesync: Add timesync core driver") Cc: <stable(a)vger.kernel.org> # 4.9.x Signed-off-by: Rui Miguel Silva <rmfrfs(a)gmail.com> --- Greg, This patch does not contain the "commit <sha> upstream" snippet since you already removed the timesync code from the -next tree, even though it is still in 4.10-rc. However this will affect the greybus users of 4.9.x. If you meanwhile pull that remove to rc, this patch is only necessary in stable 4.9.x. v1->v2: - add Fixes tag to changelog Johan Hovold: - for symmetry with the lock, adjust the unlock_bus function accordantly. drivers/staging/greybus/timesync_platform.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/staging/greybus/timesync_platform.c b/drivers/staging/greybus/timesync_platform.c index 113f3d6c4b3a..27f75b17679b 100644 --- a/drivers/staging/greybus/timesync_platform.c +++ b/drivers/staging/greybus/timesync_platform.c @@ -45,12 +45,18 @@ u32 gb_timesync_platform_get_clock_rate(void) int gb_timesync_platform_lock_bus(struct gb_timesync_svc *pdata) { + if (!arche_platform_change_state_cb) + return 0; + return arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_TIME_SYNC, pdata); } void gb_timesync_platform_unlock_bus(void) { + if (!arche_platform_change_state_cb) + return; + arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL); } -- 2.11.0

8 years

[PATCH] staging: greybus: timesync: validate platform state callback

by Rui Miguel Silva

When tearingdown timesync, and not in arche platform, the state platform callback is not initialized. That will trigger the following NULL dereferencing. CallTrace: ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus] gb_timesync_teardown+0x85/0xc0 [greybus] gb_timesync_svc_remove+0xab/0x190 [greybus] gb_svc_del+0x29/0x110 [greybus] gb_hd_del+0x14/0x20 [greybus] ap_disconnect+0x24/0x60 [gb_es2] usb_unbind_interface+0x7a/0x2c0 __device_release_driver+0x96/0x150 device_release_driver+0x1e/0x30 bus_remove_device+0xe7/0x130 device_del+0x116/0x230 usb_disable_device+0x97/0x1f0 usb_disconnect+0x80/0x260 hub_event+0x5ca/0x10e0 process_one_work+0x126/0x3b0 worker_thread+0x55/0x4c0 ? process_one_work+0x3b0/0x3b0 kthread+0xc4/0xe0 ? kthread_park+0xb0/0xb0 ret_from_fork+0x22/0x30 So, fix that by adding checks before use the callback. Cc: <stable(a)vger.kernel.org> # 4.9.x Signed-off-by: Rui Miguel Silva <rmfrfs(a)gmail.com> --- Greg, This patch does not contain the "commit <sha> upstream" snippet since you already removed the timesync code from the -next tree. So, It would never get there. However this will affect the greybus users of 4.9.x. drivers/staging/greybus/timesync_platform.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/staging/greybus/timesync_platform.c b/drivers/staging/greybus/timesync_platform.c index 113f3d6c4b3a..7d3ccc2bc98c 100644 --- a/drivers/staging/greybus/timesync_platform.c +++ b/drivers/staging/greybus/timesync_platform.c @@ -45,13 +45,18 @@ u32 gb_timesync_platform_get_clock_rate(void) int gb_timesync_platform_lock_bus(struct gb_timesync_svc *pdata) { + if (!arche_platform_change_state_cb) + return 0; + return arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_TIME_SYNC, pdata); } void gb_timesync_platform_unlock_bus(void) { - arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL); + if (arche_platform_change_state_cb) + arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, + NULL); } static const struct of_device_id arch_timer_of_match[] = { -- 2.11.0

8 years

[PATCH v3 0/4] drivers/staging/greybus: add async operations

by Bryan O'Donoghue

V3: Picard: "Let's make sure history never forgets the name ... Enterprise." Drop gb_operation_cancel_async. The sync version works just as well - Johan V2: Scotty : "What are you standing around for - do you not know a jailbreak when you see one" ? Making sure to run format-patch against the right tree this time.. Updated meta-comments to highlight adding a generic async timeout - Johan Captured the result code of schedule_work/queue_work - Bryan Drop reference count on failure to submit schedule_work/queue_work - Bryan Added timeout suffix back to new gb_operation_request_send - Johan/Greg Updated Signed-off-by to my nexus-software.ie address - Bryan Iterated requisite Star Trek Quote - Bryan/Johan V4-private -> V1-public: McCOY: You've got him, Jim! You've got him where you want him. This patchset adds async operations to greybus-core. Rather than have different drivers do variations on the same theme of launching and timing out asynchronous operations, it makes sense instead to provide this functionality via greybus-core. This patchset makes it possible to launch asynchronous operations and have the completion callback for an operation indicate -ETIMEDOUT for timeouts without drivers having to manage that locally. This set doesn't convert the existing synchronous operations away from wait_for_completion_interruptible_timeout() since that would involve adding an extra work-queue item to each synchronous call, with no added benefit for synchronous operations. Synchronous operations can already detect -ETIMEDOUT by blocking on the synchronous send operations, asynchronous operations and the driver associated with those operations OTOH must implement a completion handler. The main improvement this patchset makes is to pass the -ETIMEDOUT completion status into that completion handler - hiding the setup/timeout of operations away from a driver-level and into a core-level set of logic. Loopback as an example can have lots and lots of redundant code removed and given we previously had some in-flight effort to add asynchronous operations to SDIO it makes sense to move the generic types of things we need to do on the asynchronous path into greybus-core so that SDIO and other bundle drivers can reuse instead of reimplement asynchronous operations. Note: we had approximately three internal versions of this on Project-Ara. Here's the log for completeness. V3-private -> V4-private: Fix bug in loopback conversion - Mitch Tasman V2-private -> V3-private: remotes/ara-main/sandbox/bodonoghue/ara-main-current+async_op3 Drop patch #6 converting sync operations to new t/o structure. Johan had a concern about doing an in-depth analysis on that change and considering our compressed timelines now, this analysis won't happen. OTOH the new async API is the right thing for loopback and for potential later greybus users/implementers to reuse. Although it wasn't part of the motive for this change - I observe slightly better performance than baseline with loopback tests with this change in place - which shouldn't be surprising since we have less aggregate context switching for operation timeouts. V1-private -> V2-private: Rename async_private => private - Greg gb_operation_request_send_async_timeout -> gb_operation_request_send_async() - Greg Using existing gb_operation_completion_wq - Viresh Split out gb_operation_cancel() - timeout need not wait synchronously on cancellation queue - Bryan Move timeout to work-queue - Greg/Viresh cancel timeout workqueue @ operation completion point - Viresh/Bryan Bryan O'Donoghue (4): staging: greybus: operation: add generic asynchronous timeout operation support staging: greybus: operation: add private data with get/set accessors staging: greybus: loopback: convert loopback to use generic async operations staging: greybus: loopback: convert to use msecs internally drivers/staging/greybus/loopback.c | 181 ++++++++---------------------------- drivers/staging/greybus/operation.c | 66 +++++++++++++ drivers/staging/greybus/operation.h | 18 ++++ 3 files changed, 123 insertions(+), 142 deletions(-) -- 2.7.4

8 years

[PATCH] Staging: greybus: gpio.c - style fix

by Derek Robson

Fixed bare use of 'unsigned', Found using checkpatch Signed-off-by: Derek Robson <robsonde(a)gmail.com> --- drivers/staging/greybus/gpio.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 250caa00de5e..e937666610e6 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -410,21 +410,21 @@ static int gb_gpio_request_handler(struct gb_operation *op) return 0; } -static int gb_gpio_request(struct gpio_chip *chip, unsigned offset) +static int gb_gpio_request(struct gpio_chip *chip, unsigned int offset) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); return gb_gpio_activate_operation(ggc, (u8)offset); } -static void gb_gpio_free(struct gpio_chip *chip, unsigned offset) +static void gb_gpio_free(struct gpio_chip *chip, unsigned int offset) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); gb_gpio_deactivate_operation(ggc, (u8)offset); } -static int gb_gpio_get_direction(struct gpio_chip *chip, unsigned offset) +static int gb_gpio_get_direction(struct gpio_chip *chip, unsigned int offset) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); u8 which; @@ -438,14 +438,14 @@ static int gb_gpio_get_direction(struct gpio_chip *chip, unsigned offset) return ggc->lines[which].direction ? 1 : 0; } -static int gb_gpio_direction_input(struct gpio_chip *chip, unsigned offset) +static int gb_gpio_direction_input(struct gpio_chip *chip, unsigned int offset) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); return gb_gpio_direction_in_operation(ggc, (u8)offset); } -static int gb_gpio_direction_output(struct gpio_chip *chip, unsigned offset, +static int gb_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int value) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); @@ -453,7 +453,7 @@ static int gb_gpio_direction_output(struct gpio_chip *chip, unsigned offset, return gb_gpio_direction_out_operation(ggc, (u8)offset, !!value); } -static int gb_gpio_get(struct gpio_chip *chip, unsigned offset) +static int gb_gpio_get(struct gpio_chip *chip, unsigned int offset) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); u8 which; @@ -467,15 +467,15 @@ static int gb_gpio_get(struct gpio_chip *chip, unsigned offset) return ggc->lines[which].value; } -static void gb_gpio_set(struct gpio_chip *chip, unsigned offset, int value) +static void gb_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); gb_gpio_set_value_operation(ggc, (u8)offset, !!value); } -static int gb_gpio_set_debounce(struct gpio_chip *chip, unsigned offset, - unsigned debounce) +static int gb_gpio_set_debounce(struct gpio_chip *chip, unsigned int offset, + unsigned int debounce) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); u16 usec; @@ -593,7 +593,7 @@ static int gb_gpio_irqchip_add(struct gpio_chip *chip, { struct gb_gpio_controller *ggc; unsigned int offset; - unsigned irq_base; + unsigned int irq_base; if (!chip || !irqchip) return -EINVAL; @@ -625,7 +625,7 @@ static int gb_gpio_irqchip_add(struct gpio_chip *chip, return 0; } -static int gb_gpio_to_irq(struct gpio_chip *chip, unsigned offset) +static int gb_gpio_to_irq(struct gpio_chip *chip, unsigned int offset) { struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); -- 2.11.0

8 years

[PATCH -next] staging: greybus: audio_gb.c: Change uint32_t to u32

by Marcos Paulo de Souza

Change uint32_t to u32, solved the issue reported by checkpatch.pl: CHECK: Prefer kernel type 'u32' over 'uint32_t' + uint32_t *format, uint32_t *rate, u8 *channels, CHECK: Prefer kernel type 'u32' over 'uint32_t' + uint32_t format, uint32_t rate, u8 channels, Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com> --- drivers/staging/greybus/audio_gb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c index 42f287d..7884d84 100644 --- a/drivers/staging/greybus/audio_gb.c +++ b/drivers/staging/greybus/audio_gb.c @@ -108,7 +108,7 @@ int gb_audio_gb_disable_widget(struct gb_connection *connection, EXPORT_SYMBOL_GPL(gb_audio_gb_disable_widget); int gb_audio_gb_get_pcm(struct gb_connection *connection, u16 data_cport, - uint32_t *format, uint32_t *rate, u8 *channels, + u32 *format, u32 *rate, u8 *channels, u8 *sig_bits) { struct gb_audio_get_pcm_request req; @@ -132,7 +132,7 @@ int gb_audio_gb_get_pcm(struct gb_connection *connection, u16 data_cport, EXPORT_SYMBOL_GPL(gb_audio_gb_get_pcm); int gb_audio_gb_set_pcm(struct gb_connection *connection, u16 data_cport, - uint32_t format, uint32_t rate, u8 channels, + u32 format, u32 rate, u8 channels, u8 sig_bits) { struct gb_audio_set_pcm_request req; -- 2.9.3

8 years

Re: [greybus-dev] [PATCH] staging: greybus: loopback_test: use octal permissions instead of symbolic

by Greg KH

On Wed, Jan 18, 2017 at 01:21:01AM -0800, Igor Pylypiv wrote: > checkpatch.pl warning: > Symbolic permissions are not preferred. Consider using octal permissions. > > Signed-off-by: Igor Pylypiv <igor.pylypiv(a)gmail.com> > --- > drivers/staging/greybus/tools/loopback_test.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Please always use the output of scripts/get_maintainer.pl to determine who to send patches to, you forgot a mailing list and a few other maintainers... Please try again. thanks, greg k-h

8 years, 1 month

[PATCH] staging: greybus: loopback_test: use octal permissions instead of symbolic

by Igor Pylypiv

checkpatch.pl warning: Symbolic permissions are not preferred. Consider using octal permissions. Signed-off-by: Igor Pylypiv <igor.pylypiv(a)gmail.com> --- drivers/staging/greybus/tools/loopback_test.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 5312b2a..18d7a3d 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -521,7 +521,6 @@ static int log_results(struct loopback_test *t) int fd, i, len, ret; struct tm tm; time_t local_time; - mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH; char file_name[MAX_SYSFS_PATH]; char data[CSV_MAX_LINE]; @@ -538,7 +537,7 @@ static int log_results(struct loopback_test *t) snprintf(file_name, sizeof(file_name), "%s_%d_%d.csv", t->test_name, t->size, t->iteration_max); - fd = open(file_name, O_WRONLY | O_CREAT | O_APPEND, mode); + fd = open(file_name, O_WRONLY | O_CREAT | O_APPEND, 0644); if (fd < 0) { fprintf(stderr, "unable to open %s for appendation\n", file_name); abort(); -- 2.7.4

8 years, 1 month

[PATCH v3 0/4] Cleanup greybus audio driver

by Vaibhav Agarwal

This patch series include following cleanup changes in GB Audio driver. - Avoid unnecessary checks for le32 variables - Initialize sig_bits before configuring hw_params - Remove junk codec register mapping - Ensure proper byte ordering Next task is to enable build for GB codec driver. However this requires pushing some changes in ASoC framework. Possibly in another two weeks (based on my freetime), I'll try to submit those changes to ASoC mailing list. And once the same are accepted there, I'll share relevant patches for GB Audio codec driver as well. Changes from v2: - patch 1/4 - Update commit message - Mark Greer - Move le32_to_cpu change to patch 4/4 - Mark Greer - patch 3/4 - Remove junk codec register related definitions in .h file - Mark Greer - patch 4/4 - Ensure byte order for the variable missed out in v1/v2 - Mark Greer Changes from v1: - Include review comments from Dan Vaibhav Agarwal (4): staging: greybus: audio: Avoid less than zero check for le32 variable staging: greybus: audio: Initialize sig_bits before configuring hwparams staging: greybus: audio: Cleanup junk codec registers staging: greybus: audio: Ensure proper byte order drivers/staging/greybus/audio_codec.c | 46 +++----------- drivers/staging/greybus/audio_codec.h | 46 -------------- drivers/staging/greybus/audio_module.c | 2 +- drivers/staging/greybus/audio_topology.c | 102 ++++++++++++++++--------------- 4 files changed, 62 insertions(+), 134 deletions(-) -- 2.7.4

8 years, 1 month

[PATCH v2 0/4] Cleanup greybus audio driver

by Vaibhav Agarwal

This patch series include following cleanup changes in GB Audio driver. - Avoid unnecessary checks for le32 variables - Initialize sig_bits before configuring hw_params - Remove junk codec register mapping - Ensure proper byte ordering Next task is to enable build for GB codec driver. However this requires pushing some changes in ASoC framework. Possibly in another two weeks (based on my freetime), I'll try to submit those changes to ASoC mailing list. And once the same are accepted there, I'll share relevant patches for GB Audio codec driver as well. Changes from v1: - Include review comments from Dan Vaibhav Agarwal (4): staging: greybus: audio: Avoid less than zero check for le32 variable staging: greybus: audio: Initialize sig_bits before configuring hwparams staging: greybus: audio: Cleanup junk codec registers staging: greybus: audio: Ensure proper byte order drivers/staging/greybus/audio_codec.c | 46 +++----------- drivers/staging/greybus/audio_module.c | 2 +- drivers/staging/greybus/audio_topology.c | 100 ++++++++++++++++--------------- 3 files changed, 61 insertions(+), 87 deletions(-) -- 2.7.4

8 years, 1 month

[PATCH 0/4] Cleanup greybus audio driver

by Vaibhav Agarwal

This patch series include following cleanup changes in GB Audio driver. - Avoid unnecessary checks for le32 variables - Initialize sig_bits before configuring hw_params - Remove junk codec register mapping - Ensure proper byte ordering Next task is to enable build for GB codec driver. However this requires pushing some changes in ASoC framework. Possibly in another two weeks (based on my freetime), I'll try to submit those changes to ASoC mailing list. And once the same are accepted there, I'll share relevant patches for GB Audio codec driver as well. Vaibhav Agarwal (4): staging: greybus: audio: Avoid less than zero check for le32 variable staging: greybus: audio: Initialize sig_bits before configuring hwparams staging: greybus: audio: Cleanup junk codec registers staging: greybus: audio: Ensure proper byte order drivers/staging/greybus/audio_codec.c | 46 +++------------- drivers/staging/greybus/audio_module.c | 2 +- drivers/staging/greybus/audio_topology.c | 94 ++++++++++++++++---------------- 3 files changed, 58 insertions(+), 84 deletions(-) -- 2.7.4

8 years, 1 month

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev