Re: [PATCH RFC 04/11] staging: vchiq_arm: Fix platform device unregistration
On Thu, Oct 25, 2018 at 05:29:28PM +0200, Stefan Wahren wrote: > In error case platform_device_register_data would return an ERR_PTR > instead of NULL. So we better check this before unregistration. > > Fixes: 37b7b3087a2f ("staging/vc04_services: Register a platform device for > the camera driver.") > Signed-off-by: Stefan Wahren > --- > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > index ea78937..d7d7c2f0 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > @@ -3672,7 +3672,8 @@ static int vchiq_probe(struct platform_device *pdev) > > static int vchiq_remove(struct platform_device *pdev) > { > - platform_device_unregister(bcm2835_camera); > + if (!IS_ERR(bcm2835_camera)) > + platform_device_unregister(bcm2835_camera); This wouldn't be needed if we checked for platform_device_register_data() errors in probe. That would be a better fix. This is obviously a bug, but is it a real life bug, btw? I would be surprised if platform_device_register_data() actually failed. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 06/11] staging: vchiq_arm: Register a platform device for audio
On Thu, Oct 25, 2018 at 05:29:30PM +0200, Stefan Wahren wrote: > Following Eric's commit 37b7b3087a2f ("staging/vc04_services: Register a > platform device for the camera driver.") this register the audio driver as > a platform device, too. > > Signed-off-by: Stefan Wahren > --- > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > index 778a252..fc6388b 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > @@ -170,6 +170,7 @@ static struct class *vchiq_class; > static struct device *vchiq_dev; > static DEFINE_SPINLOCK(msg_queue_spinlock); > static struct platform_device *bcm2835_camera; > +static struct platform_device *bcm2835_audio; > > static struct vchiq_drvdata bcm2835_drvdata = { > .cache_line_size = 32, > @@ -3670,6 +3671,7 @@ static int vchiq_probe(struct platform_device *pdev) > MAJOR(vchiq_devid), MINOR(vchiq_devid)); > > bcm2835_camera = vchiq_register_child(pdev, "bcm2835-camera"); > + bcm2835_audio = vchiq_register_child(pdev, "bcm2835_audio"); Same thing. Check here and not in the remove function. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 06/11] staging: vchiq_arm: Register a platform device for audio
On Fri, Oct 26, 2018 at 11:09:31AM +0300, Dan Carpenter wrote: > On Thu, Oct 25, 2018 at 05:29:30PM +0200, Stefan Wahren wrote: > > Following Eric's commit 37b7b3087a2f ("staging/vc04_services: Register a > > platform device for the camera driver.") this register the audio driver as > > a platform device, too. > > > > Signed-off-by: Stefan Wahren > > --- > > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > index 778a252..fc6388b 100644 > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > @@ -170,6 +170,7 @@ static struct class *vchiq_class; > > static struct device *vchiq_dev; > > static DEFINE_SPINLOCK(msg_queue_spinlock); > > static struct platform_device *bcm2835_camera; > > +static struct platform_device *bcm2835_audio; > > > > static struct vchiq_drvdata bcm2835_drvdata = { > > .cache_line_size = 32, > > @@ -3670,6 +3671,7 @@ static int vchiq_probe(struct platform_device *pdev) > > MAJOR(vchiq_devid), MINOR(vchiq_devid)); > > > > bcm2835_camera = vchiq_register_child(pdev, "bcm2835-camera"); > > + bcm2835_audio = vchiq_register_child(pdev, "bcm2835_audio"); > > Same thing. Check here and not in the remove function. > Never mind. I see what you're doing now that you have both camera and audio. You want it to still load even if one is not present. That's fine. I am slightly uncomfortable just leaving error pointers lying around. It might be nicer to just do: bcm2835_camera = vchiq_register_child(pdev, "bcm2835-camera"); if (IS_ERR(bcm2835_camera)) { dev_err(pdev, "bcm2835-camera not registered.\n"); bcm2835_camera = NULL; } In that case NULL becomes a special case of success. The unregister functions accept NULL pointers so they wouldn't need to be changed. Btw, if these had been normal patch instead of RFC we would have just applied them and I wouldn't have complained. But since you're deliberately requesting comments anyway, then ... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] binder: ipc namespace support for android binder
Hi We are working for running android in container, but we found that binder is not isolated by ipc namespace. Since binder is a form of IPC and therefore should be tied to ipc namespace. With this patch, we can run more than one android container on one host. This patch move "binder_procs" and "binder_context" into ipc_namespace, driver will find the context from it when opening. Althought statistics in debugfs remain global. Signed-off-by: choury zhou --- drivers/android/Kconfig | 2 +- drivers/android/binder.c | 126 +- include/linux/ipc_namespace.h | 14 ipc/namespace.c | 4 ++ 4 files changed, 111 insertions(+), 35 deletions(-) diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig index 432e9ad77070..09883443b2da 100644 --- a/drivers/android/Kconfig +++ b/drivers/android/Kconfig @@ -10,7 +10,7 @@ if ANDROID config ANDROID_BINDER_IPC bool "Android Binder IPC Driver" - depends on MMU + depends on MMU && SYSVIPC default n ---help--- Binder is used in Android for both communication between processes, diff --git a/drivers/android/binder.c b/drivers/android/binder.c index d58763b6b009..e061dba9b8b3 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -68,6 +68,7 @@ #include #include #include +#include #include #include #include @@ -79,13 +80,12 @@ #include "binder_alloc.h" #include "binder_trace.h" +#define ipcns (current->nsproxy->ipc_ns) + static HLIST_HEAD(binder_deferred_list); static DEFINE_MUTEX(binder_deferred_lock); static HLIST_HEAD(binder_devices); -static HLIST_HEAD(binder_procs); -static DEFINE_MUTEX(binder_procs_lock); - static HLIST_HEAD(binder_dead_nodes); static DEFINE_SPINLOCK(binder_dead_nodes_lock); @@ -231,7 +231,7 @@ struct binder_transaction_log_entry { int return_error_line; uint32_t return_error; uint32_t return_error_param; - const char *context_name; + int context_device; }; struct binder_transaction_log { atomic_t cur; @@ -262,19 +262,66 @@ static struct binder_transaction_log_entry *binder_transaction_log_add( } struct binder_context { + struct hlist_node hlist; struct binder_node *binder_context_mgr_node; struct mutex context_mgr_node_lock; kuid_t binder_context_mgr_uid; - const char *name; + intdevice; }; struct binder_device { struct hlist_node hlist; struct miscdevice miscdev; - struct binder_context context; }; +void binder_exit_ns(struct ipc_namespace *ns) +{ + struct binder_context *context; + struct hlist_node *tmp; + + mutex_destroy(&ns->binder_procs_lock); + mutex_destroy(&ns->binder_contexts_lock); + hlist_for_each_entry_safe(context, tmp, &ns->binder_contexts, hlist) { + mutex_destroy(&context->context_mgr_node_lock); + hlist_del(&context->hlist); + kfree(context); + } +} + +int binder_init_ns(struct ipc_namespace *ns) +{ + int ret; + struct binder_device *device; + + mutex_init(&ns->binder_procs_lock); + INIT_HLIST_HEAD(&ns->binder_procs); + mutex_init(&ns->binder_contexts_lock); + INIT_HLIST_HEAD(&ns->binder_contexts); + + hlist_for_each_entry(device, &binder_devices, hlist) { + struct binder_context *context; + + context = kzalloc(sizeof(*context), GFP_KERNEL); + if (!context) { + ret = -ENOMEM; + goto err; + } + + context->device = device->miscdev.minor; + context->binder_context_mgr_uid = INVALID_UID; + mutex_init(&context->context_mgr_node_lock); + + hlist_add_head(&context->hlist, &ns->binder_contexts); + } + + return 0; +err: + binder_exit_ns(ns); + return ret; +} + + /** * struct binder_work - work enqueued on a worklist * @entry: node enqueued on list @@ -2748,7 +2795,7 @@ static void binder_transaction(struct binder_proc *proc, e->target_handle = tr->target.handle; e->data_size = tr->data_size; e->offsets_size = tr->offsets_size; - e->context_name = proc->context->name; + e->context_device = proc->context->device; if (reply) { binder_inner_proc_lock(proc); @@ -4754,6 +4801,7 @@ static int binder_open(struct inode *nodp, struct file *filp) { struct binder_proc *proc; struct binder_device *binder_dev; + struct binder_context *context; binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__, current->group_leader->pid, current->pid); @@ -4770,7 +4818,17 @@ static int binder_open(struct inode *nodp, struct file *filp) proc->default_priority = task_nice(current); binder_dev = container_of(filp->private_data, s
Re: [PATCH 2/4] stagign: wilc1000: validate cfg parameters before scheduling the work
On Thu, Oct 25, 2018 at 09:11:00PM +, adham.aboza...@microchip.com wrote: You'll need to resend these because your email from header is in the wrong format. Also there is a typo in the subject. s/stagign/staging/. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
On Thu, Oct 25, 2018 at 09:45:11PM -0300, Matheus Tavares wrote: > From: Victor Colombo > > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and > implements the relative read behavior at ad2s90_read_raw. > > Signed-off-by: Victor Colombo > --- You should be adding your S-o-B here as well because the patch is passing through your hands. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] staging: vboxvideo: Remove chekpatch issues
Hi, On 25-10-18 21:40, Shayenne da Luz Moura wrote: This series cleans the following checkpatch.pl issues: CHECK: Prefer kernel type 'u32' over 'uint32_t' CHECK: Avoid using bool structure members because of possible alignment issues Shayenne da Luz Moura (2): staging: vboxvideo: Change uint32_t to u32 staging: vboxvideo: Use unsigned int instead bool Why am I receiving only the cover letter of this series? Please resend so that I get all the patches. Also at least the first patch is wrong, the drm headers use uint32_t in the prototype definition of the callback functions we are defining, so the vboxvideo code should use the same even if the compiler does not warn about the callback implementation having different parameter types in this case. Regards, Hans drivers/staging/vboxvideo/vbox_drv.h| 14 +++--- drivers/staging/vboxvideo/vbox_mode.c | 2 +- drivers/staging/vboxvideo/vboxvideo_guest.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 11/11] staging: bcm2835-camera: Add hint about possible faulty config
On Thu, 2018-10-25 at 17:29 +0200, Stefan Wahren wrote: > As per default the GPU memory config of the Raspberry Pi isn't > sufficient > for the camera usage. Even worse the bcm2835 camera driver doesn't > provide a > helpful error message in this case. So let's add a hint to point the > user > to the likely cause. > > Signed-off-by: Stefan Wahren > --- > drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal- > vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c > index cc2d993..bffd75d 100644 > --- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c > +++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c > @@ -1623,8 +1623,11 @@ int vchiq_mmal_component_init(struct > vchiq_mmal_instance *instance, > component = &instance->component[instance->component_idx]; > > ret = create_component(instance, component, name); > - if (ret < 0) > + if (ret < 0) { > + pr_err("%s: failed to create component %d (Not enough > GPU mem?)\n", > +__func__, ret); > goto unlock; > + } > > /* ports info needs gathering */ > component->control.type = MMAL_PORT_TYPE_CONTROL; Nice, had to deal with this the hard way, would have saved me some time. signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 00/11] staging: vc04_services: Improve driver load/unload
Hi Stefan, On Thu, 2018-10-25 at 17:29 +0200, Stefan Wahren wrote: > This patch series improves the load/unload of bcm2835 camera and > audio > drivers. It has been tested with Raspberry Pi 3 B and a camera module > V1. > > This series based on current linux-next and Phil Elwell's series > ("Improve VCHIQ > cache line size handling"). After Nicolas' series ("staging: > vc04_services: > Some dead code removal") has been applied, i will rebase my series. > > Stefan Wahren (11): > staging: bcm2835-camera: Abort probe if there is no camera > staging: bcm2835-camera: fix module autoloading > staging: bcm2835-camera: Move module info to the end > staging: vchiq_arm: Fix platform device unregistration > staging: vchiq_arm: Fix camera device registration > staging: vchiq_arm: Register a platform device for audio > staging: bcm2835-audio: Enable compile test > staging: bcm2835-audio: use module_platform_driver() macro > staging: bcm2835-audio: Drop DT dependency > staging: bcm2835-camera: Provide more specific probe error messages > staging: bcm2835-camera: Add hint about possible faulty config > > .../staging/vc04_services/bcm2835-audio/Kconfig| 2 +- > .../staging/vc04_services/bcm2835-audio/bcm2835.c | 61 ++ > --- > .../vc04_services/bcm2835-camera/bcm2835-camera.c | 78 > +++--- > .../vc04_services/bcm2835-camera/mmal-vchiq.c | 5 +- > .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 27 ++-- > 5 files changed, 102 insertions(+), 71 deletions(-) > I prefer Dan's approach to error checking in vchiq_probe(). Apart from that seems good to me. Reviewed-by: Nicolas Saenz Julienne Regards, Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vboxvideo: Remove unnecessary parentheses
Hi, On 23-10-18 19:43, Shayenne da Luz Moura wrote: Remove unneeded parentheses around the arguments of ||. This reduces clutter and code behave in the same way. Change suggested by checkpatch.pl. vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 < crtc->x' Signed-off-by: Shayenne da Luz Moura Patch looks good to me: Reviewed-by: Hans de Goede Regards, Hans --- Changes in v2: - Make the commit message more clearer. drivers/staging/vboxvideo/vbox_main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 429f6a453619..10a862674789 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -116,10 +116,10 @@ void vbox_framebuffer_dirty_rectangles(struct drm_framebuffer *fb, struct vbva_cmd_hdr cmd_hdr; unsigned int crtc_id = to_vbox_crtc(crtc)->crtc_id; - if ((rects[i].x1 > crtc->x + crtc->hwmode.hdisplay) || - (rects[i].y1 > crtc->y + crtc->hwmode.vdisplay) || - (rects[i].x2 < crtc->x) || - (rects[i].y2 < crtc->y)) + if (rects[i].x1 > crtc->x + crtc->hwmode.hdisplay || + rects[i].y1 > crtc->y + crtc->hwmode.vdisplay || + rects[i].x2 < crtc->x || + rects[i].y2 < crtc->y) continue; cmd_hdr.x = (s16)rects[i].x1; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 1/2] staging: iio: ad2s1210: Switch to the gpio descriptor interface
Use the gpiod interface instead of the deprecated old non-descriptor interface. Signed-off-by: Nishad Kamdar --- Changes in v4: - Add spaces after { and before } in gpios[] initialization. - Check the correct pointer for error. - Align the dev_err msg to existing format in the code. Changes in v3: - Use a pointer to pointer for gpio_desc in struct ad2s1210_gpio as it will be used to modify a pointer. - Use dot notation to initialize the structure. - Use a pointer variable to avoid writing gpios[i]. Changes in v2: - Use the spi_device struct embedded in st instead of passing it as an argument to ad2s1210_setup_gpios(). - Use an array of structs to reduce redundant code in in ad2s1210_setup_gpios(). - Remove ad2s1210_free_gpios() as devm API is being used. --- drivers/staging/iio/resolver/ad2s1210.c | 92 ++--- drivers/staging/iio/resolver/ad2s1210.h | 3 - 2 files changed, 50 insertions(+), 45 deletions(-) diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c index ac13b99bd9cb..93c3c70ce62e 100644 --- a/drivers/staging/iio/resolver/ad2s1210.c +++ b/drivers/staging/iio/resolver/ad2s1210.c @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include @@ -69,10 +69,21 @@ enum ad2s1210_mode { static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 }; +struct ad2s1210_gpio { + struct gpio_desc **ptr; + const char *name; + unsigned long flags; +}; + struct ad2s1210_state { const struct ad2s1210_platform_data *pdata; struct mutex lock; struct spi_device *sdev; + struct gpio_desc *sample; + struct gpio_desc *a0; + struct gpio_desc *a1; + struct gpio_desc *res0; + struct gpio_desc *res1; unsigned int fclkin; unsigned int fexcit; bool hysteresis; @@ -91,8 +102,8 @@ static const int ad2s1210_mode_vals[4][2] = { static inline void ad2s1210_set_mode(enum ad2s1210_mode mode, struct ad2s1210_state *st) { - gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]); - gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]); + gpiod_set_value(st->a0, ad2s1210_mode_vals[mode][0]); + gpiod_set_value(st->a1, ad2s1210_mode_vals[mode][1]); st->mode = mode; } @@ -152,8 +163,8 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st) static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st) { - int resolution = (gpio_get_value(st->pdata->res[0]) << 1) | - gpio_get_value(st->pdata->res[1]); + int resolution = (gpiod_get_value(st->res0) << 1) | + gpiod_get_value(st->res1); return ad2s1210_resolution_value[resolution]; } @@ -164,10 +175,10 @@ static const int ad2s1210_res_pins[4][2] = { static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st) { - gpio_set_value(st->pdata->res[0], - ad2s1210_res_pins[(st->resolution - 10) / 2][0]); - gpio_set_value(st->pdata->res[1], - ad2s1210_res_pins[(st->resolution - 10) / 2][1]); + gpiod_set_value(st->res0, + ad2s1210_res_pins[(st->resolution - 10) / 2][0]); + gpiod_set_value(st->res1, + ad2s1210_res_pins[(st->resolution - 10) / 2][1]); } static inline int ad2s1210_soft_reset(struct ad2s1210_state *st) @@ -401,15 +412,15 @@ static ssize_t ad2s1210_clear_fault(struct device *dev, int ret; mutex_lock(&st->lock); - gpio_set_value(st->pdata->sample, 0); + gpiod_set_value(st->sample, 0); /* delay (2 * tck + 20) nano seconds */ udelay(1); - gpio_set_value(st->pdata->sample, 1); + gpiod_set_value(st->sample, 1); ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT); if (ret < 0) goto error_ret; - gpio_set_value(st->pdata->sample, 0); - gpio_set_value(st->pdata->sample, 1); + gpiod_set_value(st->sample, 0); + gpiod_set_value(st->sample, 1); error_ret: mutex_unlock(&st->lock); @@ -466,7 +477,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev, s16 vel; mutex_lock(&st->lock); - gpio_set_value(st->pdata->sample, 0); + gpiod_set_value(st->sample, 0); /* delay (6 * tck + 20) nano seconds */ udelay(1); @@ -512,7 +523,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev, } error_ret: - gpio_set_value(st->pdata->sample, 1); + gpiod_set_value(st->sample, 1); /* delay (2 * tck + 20) nano seconds */ udelay(1); mutex_unlock(&st->lock); @@ -630,30 +641,32 @@ static const struct iio_info ad2s1210_info = { static int ad2s1210_setup_gpios(struct ad2s1210_state *st) { - unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_I
[PATCH RFC 05/18] staging: vchiq_arm: get rid of vchi_mh.h
The concept of VCHI_MEM_HANDLE_T is introduced by this header file and was meant to be used with bulk transfers. After a quick look in vchiq_core.c it is pretty clear that it actually accomplishes nothing nor alters the bulk transfers in any way. Signed-off-by: Nicolas Saenz Julienne --- .../vc04_services/interface/vchi/vchi.h | 3 -- .../vc04_services/interface/vchi/vchi_mh.h| 42 --- .../interface/vchiq_arm/vchiq_2835_arm.c | 6 +-- .../interface/vchiq_arm/vchiq_arm.c | 27 ++-- .../interface/vchiq_arm/vchiq_core.c | 17 .../interface/vchiq_arm/vchiq_core.h | 10 ++--- .../interface/vchiq_arm/vchiq_if.h| 8 ++-- 7 files changed, 27 insertions(+), 86 deletions(-) delete mode 100644 drivers/staging/vc04_services/interface/vchi/vchi_mh.h diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h b/drivers/staging/vc04_services/interface/vchi/vchi.h index 379a16ebfd5b..e326926eac31 100644 --- a/drivers/staging/vc04_services/interface/vchi/vchi.h +++ b/drivers/staging/vc04_services/interface/vchi/vchi.h @@ -36,7 +36,6 @@ #include "interface/vchi/vchi_cfg.h" #include "interface/vchi/vchi_common.h" -#include "vchi_mh.h" /** Global defs @@ -239,7 +238,6 @@ extern int32_t vchi_bulk_queue_receive(VCHI_SERVICE_HANDLE_T handle, // Prepare interface for a transfer from the other side into relocatable memory. int32_t vchi_bulk_queue_receive_reloc(const VCHI_SERVICE_HANDLE_T handle, - VCHI_MEM_HANDLE_T h_dst, uint32_t offset, uint32_t data_size, const VCHI_FLAGS_T flags, @@ -261,7 +259,6 @@ extern int32_t vchi_bulk_queue_transmit(VCHI_SERVICE_HANDLE_T handle, #endif extern int32_t vchi_bulk_queue_transmit_reloc(VCHI_SERVICE_HANDLE_T handle, - VCHI_MEM_HANDLE_T h_src, uint32_t offset, uint32_t data_size, VCHI_FLAGS_T flags, diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h deleted file mode 100644 index 198bd076b666.. --- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h +++ /dev/null @@ -1,42 +0,0 @@ -/** - * Copyright (c) 2010-2012 Broadcom. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - *notice, this list of conditions, and the following disclaimer, - *without modification. - * 2. Redistributions in binary form must reproduce the above copyright - *notice, this list of conditions and the following disclaimer in the - *documentation and/or other materials provided with the distribution. - * 3. The names of the above-listed copyright holders may not be used - *to endorse or promote products derived from this software without - *specific prior written permission. - * - * ALTERNATIVELY, this software may be distributed under the terms of the - * GNU General Public License ("GPL") version 2, as published by the Free - * Software Foundation. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS - * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, - * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR - * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, - * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, - * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR - * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF - * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING - * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#ifndef VCHI_MH_H_ -#define VCHI_MH_H_ - -#include - -typedef int32_t VCHI_MEM_HANDLE_T; -#define VCHI_MEM_HANDLE_INVALID 0 - -#endif diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c index 83d740feab96..014583cdf367 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c @@ -247,13 +247,10 @@ remote_event_signal(REMOTE_EVENT_T *event) } VCHIQ_STATUS_T -vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, VCHI_MEM_HANDLE_T memhandle, - void *offset, int siz
[PATCH RFC 12/18] staging: vchiq_util: use completions instead of semaphores
It is preferred in the kernel to avoid using semaphores to wait for events, as they are optimised for the opposite situation; where the common case is that they are available and may block only occasionally. FYI see this thread: https://lkml.org/lkml/2008/4/11/323. Also completions are semantically more explicit in this case. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_util.c | 16 .../interface/vchiq_arm/vchiq_util.h | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c index 2e52f07bbaa9..44b954daa74a 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c @@ -48,8 +48,8 @@ int vchiu_queue_init(VCHIU_QUEUE_T *queue, int size) queue->write = 0; queue->initialized = 1; - sema_init(&queue->pop, 0); - sema_init(&queue->push, 0); + init_completion(&queue->pop); + init_completion(&queue->push); queue->storage = kcalloc(size, sizeof(VCHIQ_HEADER_T *), GFP_KERNEL); if (!queue->storage) { @@ -80,7 +80,7 @@ void vchiu_queue_push(VCHIU_QUEUE_T *queue, VCHIQ_HEADER_T *header) return; while (queue->write == queue->read + queue->size) { - if (down_interruptible(&queue->pop) != 0) + if (wait_for_completion_interruptible(&queue->pop)) flush_signals(current); } @@ -100,17 +100,17 @@ void vchiu_queue_push(VCHIU_QUEUE_T *queue, VCHIQ_HEADER_T *header) queue->write++; - up(&queue->push); + complete(&queue->push); } VCHIQ_HEADER_T *vchiu_queue_peek(VCHIU_QUEUE_T *queue) { while (queue->write == queue->read) { - if (down_interruptible(&queue->push) != 0) + if (wait_for_completion_interruptible(&queue->push)) flush_signals(current); } - up(&queue->push); // We haven't removed anything from the queue. + complete(&queue->push); // We haven't removed anything from the queue. /* * Read from queue->storage must be visible after read from @@ -126,7 +126,7 @@ VCHIQ_HEADER_T *vchiu_queue_pop(VCHIU_QUEUE_T *queue) VCHIQ_HEADER_T *header; while (queue->write == queue->read) { - if (down_interruptible(&queue->push) != 0) + if (wait_for_completion_interruptible(&queue->push)) flush_signals(current); } @@ -146,7 +146,7 @@ VCHIQ_HEADER_T *vchiu_queue_pop(VCHIU_QUEUE_T *queue) queue->read++; - up(&queue->pop); + complete(&queue->pop); return header; } diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h index 5a1540d349d3..b226227fe5bc 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h @@ -35,7 +35,7 @@ #define VCHIQ_UTIL_H #include -#include +#include #include #include #include @@ -60,8 +60,8 @@ typedef struct { int write; int initialized; - struct semaphore pop; - struct semaphore push; + struct completion pop; + struct completion push; VCHIQ_HEADER_T **storage; } VCHIU_QUEUE_T; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 14/18] staging: vchiq_util: get rid of unneeded memory barriers
All the memory operations featured in this file modify/access memory that is only accessed by the CPU. So we can assume that all the memory barrier handling done by the completion routines is good enough for us. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_util.c | 32 --- 1 file changed, 32 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c index 44b954daa74a..4b8554bc647e 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c @@ -84,20 +84,7 @@ void vchiu_queue_push(VCHIU_QUEUE_T *queue, VCHIQ_HEADER_T *header) flush_signals(current); } - /* -* Write to queue->storage must be visible after read from -* queue->read -*/ - smp_mb(); - queue->storage[queue->write & (queue->size - 1)] = header; - - /* -* Write to queue->storage must be visible before write to -* queue->write -*/ - smp_wmb(); - queue->write++; complete(&queue->push); @@ -112,12 +99,6 @@ VCHIQ_HEADER_T *vchiu_queue_peek(VCHIU_QUEUE_T *queue) complete(&queue->push); // We haven't removed anything from the queue. - /* -* Read from queue->storage must be visible after read from -* queue->write -*/ - smp_rmb(); - return queue->storage[queue->read & (queue->size - 1)]; } @@ -130,20 +111,7 @@ VCHIQ_HEADER_T *vchiu_queue_pop(VCHIU_QUEUE_T *queue) flush_signals(current); } - /* -* Read from queue->storage must be visible after read from -* queue->write -*/ - smp_rmb(); - header = queue->storage[queue->read & (queue->size - 1)]; - - /* -* Read from queue->storage must be visible before write to -* queue->read -*/ - smp_mb(); - queue->read++; complete(&queue->pop); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 09/18] staging: vchiq_core: do not initialize semaphores twice
vchiq_init_state() initialises a series of semaphores to then call remote_event_create() on the same semaphores, which initializes them again. Signed-off-by: Nicolas Saenz Julienne --- .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index dee5ea7bfe4f..6242357735b9 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -2200,11 +2200,6 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero) sema_init(&state->connect, 0); mutex_init(&state->mutex); - sema_init(&state->trigger_event, 0); - sema_init(&state->recycle_event, 0); - sema_init(&state->sync_trigger_event, 0); - sema_init(&state->sync_release_event, 0); - mutex_init(&state->slot_mutex); mutex_init(&state->recycle_mutex); mutex_init(&state->sync_mutex); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 02/18] staging: vchiq_arm: rework close/remove_service IOCTLS
The implementation of both IOCTLS was the same except for one function call. This joins both implementations and updates the code to avoid unneeded indentations. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 66 +++ 1 file changed, 24 insertions(+), 42 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 6d503392341e..d88dd7415e1e 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -1019,55 +1019,37 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) } } break; - case VCHIQ_IOC_CLOSE_SERVICE: { + case VCHIQ_IOC_CLOSE_SERVICE: + case VCHIQ_IOC_REMOVE_SERVICE: { VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg; + USER_SERVICE_T *user_service; service = find_service_for_instance(instance, handle); - if (service != NULL) { - USER_SERVICE_T *user_service = - (USER_SERVICE_T *)service->base.userdata; - /* close_pending is false on first entry, and when the - wait in vchiq_close_service has been interrupted. */ - if (!user_service->close_pending) { - status = vchiq_close_service(service->handle); - if (status != VCHIQ_SUCCESS) - break; - } - - /* close_pending is true once the underlying service - has been closed until the client library calls the - CLOSE_DELIVERED ioctl, signalling close_event. */ - if (user_service->close_pending && - down_interruptible(&user_service->close_event)) - status = VCHIQ_RETRY; - } else + if (!service) { ret = -EINVAL; - } break; + break; + } - case VCHIQ_IOC_REMOVE_SERVICE: { - VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg; + user_service = service->base.userdata; - service = find_service_for_instance(instance, handle); - if (service != NULL) { - USER_SERVICE_T *user_service = - (USER_SERVICE_T *)service->base.userdata; - /* close_pending is false on first entry, and when the - wait in vchiq_close_service has been interrupted. */ - if (!user_service->close_pending) { - status = vchiq_remove_service(service->handle); - if (status != VCHIQ_SUCCESS) - break; - } + /* close_pending is false on first entry, and when the + wait in vchiq_close_service has been interrupted. */ + if (!user_service->close_pending) { + status = (cmd == VCHIQ_IOC_CLOSE_SERVICE) ? +vchiq_close_service(service->handle) : +vchiq_remove_service(service->handle); + if (status != VCHIQ_SUCCESS) + break; + } - /* close_pending is true once the underlying service - has been closed until the client library calls the - CLOSE_DELIVERED ioctl, signalling close_event. */ - if (user_service->close_pending && - down_interruptible(&user_service->close_event)) - status = VCHIQ_RETRY; - } else - ret = -EINVAL; - } break; + /* close_pending is true once the underlying service + has been closed until the client library calls the + CLOSE_DELIVERED ioctl, signalling close_event. */ + if (user_service->close_pending && + down_interruptible(&user_service->close_event)) + status = VCHIQ_RETRY; + break; + } case VCHIQ_IOC_USE_SERVICE: case VCHIQ_IOC_RELEASE_SERVICE: { -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 03/18] staging: vchiq_shim: delete vchi_service_create
No one is using the API neither in the actual staging tree nor in the downstream tree (https://github.com/raspberrypi/linux). Signed-off-by: Nicolas Saenz Julienne --- .../vc04_services/interface/vchi/vchi.h | 5 --- .../interface/vchiq_arm/vchiq_shim.c | 32 --- 2 files changed, 37 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h b/drivers/staging/vc04_services/interface/vchi/vchi.h index 01381904775d..379a16ebfd5b 100644 --- a/drivers/staging/vc04_services/interface/vchi/vchi.h +++ b/drivers/staging/vc04_services/interface/vchi/vchi.h @@ -113,11 +113,6 @@ extern uint32_t vchi_current_time(VCHI_INSTANCE_T instance_handle); /** Global service API */ -// Routine to create a named service -extern int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle, - SERVICE_CREATION_T *setup, - VCHI_SERVICE_HANDLE_T *handle); - // Routine to destroy a service extern int32_t vchi_service_destroy(const VCHI_SERVICE_HANDLE_T handle); diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c index c3223fcdaf87..81cac68f4b78 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c @@ -660,38 +660,6 @@ int32_t vchi_service_open(VCHI_INSTANCE_T instance_handle, } EXPORT_SYMBOL(vchi_service_open); -int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle, - SERVICE_CREATION_T *setup, - VCHI_SERVICE_HANDLE_T *handle) -{ - VCHIQ_INSTANCE_T instance = (VCHIQ_INSTANCE_T)instance_handle; - struct shim_service *service = service_alloc(instance, setup); - - *handle = (VCHI_SERVICE_HANDLE_T)service; - - if (service) { - VCHIQ_SERVICE_PARAMS_T params; - VCHIQ_STATUS_T status; - - memset(¶ms, 0, sizeof(params)); - params.fourcc = setup->service_id; - params.callback = shim_callback; - params.userdata = service; - params.version = setup->version.version; - params.version_min = setup->version.version_min; - status = vchiq_add_service(instance, ¶ms, &service->handle); - - if (status != VCHIQ_SUCCESS) { - service_free(service); - service = NULL; - *handle = NULL; - } - } - - return (service != NULL) ? 0 : -1; -} -EXPORT_SYMBOL(vchi_service_create); - int32_t vchi_service_close(const VCHI_SERVICE_HANDLE_T handle) { int32_t ret = -1; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 06/18] staging: vchiq_arm: rework vchiq_ioc_copy_element_data
The function is passed to vchiq_core.c for it to go trough all the transfer elements (an array of pointers to data) and copy them into the actual transfer memory (contiguous memory). The logic in the function was "copy an element and return, except when the element is empty, in which case look for the next non-empty element and copy it. The function will be called as many times as necessary until all the elements are copied". Now, this approach already forces the function to loop around elements and felt convoluted, so it was changed to a more straightforward "Copy all the elements into memory as long as they fit". The resulting function is shorter and simpler. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 89 +++ 1 file changed, 31 insertions(+), 58 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 34160cc3b8bd..1cdfdb714abc 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -752,74 +752,48 @@ static void close_delivered(USER_SERVICE_T *user_service) } struct vchiq_io_copy_callback_context { - struct vchiq_element *current_element; - size_t current_element_offset; + struct vchiq_element *element; + size_t element_offset; unsigned long elements_to_go; - size_t current_offset; }; -static ssize_t -vchiq_ioc_copy_element_data( - void *context, - void *dest, - size_t offset, - size_t maxsize) +static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest, + size_t offset, size_t maxsize) { - long res; + struct vchiq_io_copy_callback_context *cc = context; + size_t total_bytes_copied = 0; size_t bytes_this_round; - struct vchiq_io_copy_callback_context *copy_context = - (struct vchiq_io_copy_callback_context *)context; - - if (offset != copy_context->current_offset) - return 0; - - if (!copy_context->elements_to_go) - return 0; - - /* -* Complex logic here to handle the case of 0 size elements -* in the middle of the array of elements. -* -* Need to skip over these 0 size elements. -*/ - while (1) { - bytes_this_round = min(copy_context->current_element->size - - copy_context->current_element_offset, - maxsize); - - if (bytes_this_round) - break; - copy_context->elements_to_go--; - copy_context->current_element++; - copy_context->current_element_offset = 0; + while (total_bytes_copied < maxsize) { + if (!cc->elements_to_go) + return total_bytes_copied; - if (!copy_context->elements_to_go) - return 0; - } + if (!cc->element->size) { + cc->elements_to_go--; + cc->element++; + cc->element_offset = 0; + continue; + } - res = copy_from_user(dest, -copy_context->current_element->data + -copy_context->current_element_offset, -bytes_this_round); + bytes_this_round = min(cc->element->size - cc->element_offset, + maxsize - total_bytes_copied); - if (res != 0) - return -EFAULT; + if (copy_from_user(dest + total_bytes_copied, + cc->element->data + cc->element_offset, + bytes_this_round)) + return -EFAULT; - copy_context->current_element_offset += bytes_this_round; - copy_context->current_offset += bytes_this_round; + cc->element_offset += bytes_this_round; + total_bytes_copied += bytes_this_round; - /* -* Check if done with current element, and if so advance to the next. -*/ - if (copy_context->current_element_offset == - copy_context->current_element->size) { - copy_context->elements_to_go--; - copy_context->current_element++; - copy_context->current_element_offset = 0; + if (cc->element_offset == cc->element->size) { + cc->elements_to_go--; + cc->element++; + cc->element_offset = 0; + } } - return bytes_this_round; + return maxsize; } /** @@ -836,10 +810,9 @@ vchiq_ioc_qu
[PATCH RFC 08/18] staging: vchiq_core: remove unnecessary safety checks in vchiq_init_state
vchiq_init_state() checks the initial contents of slot_zero are correct. These are set in vchiq_init_slots(), using the same hard-coded defaults as the checks. Both functions are called sequentially and Video Core isn't yet aware of the slot's address. There is no way the contents of slot_zero changed in between functions, making the checks useless. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_core.c | 59 --- 1 file changed, 59 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 34a892011296..dee5ea7bfe4f 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -2170,65 +2170,6 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero) return VCHIQ_ERROR; } - /* Check the input configuration */ - - if (slot_zero->magic != VCHIQ_MAGIC) { - vchiq_loud_error_header(); - vchiq_loud_error("Invalid VCHIQ magic value found."); - vchiq_loud_error("slot_zero=%pK: magic=%x (expected %x)", - slot_zero, slot_zero->magic, VCHIQ_MAGIC); - vchiq_loud_error_footer(); - return VCHIQ_ERROR; - } - - if (slot_zero->version < VCHIQ_VERSION_MIN) { - vchiq_loud_error_header(); - vchiq_loud_error("Incompatible VCHIQ versions found."); - vchiq_loud_error("slot_zero=%pK: VideoCore version=%d (minimum %d)", - slot_zero, slot_zero->version, VCHIQ_VERSION_MIN); - vchiq_loud_error("Restart with a newer VideoCore image."); - vchiq_loud_error_footer(); - return VCHIQ_ERROR; - } - - if (VCHIQ_VERSION < slot_zero->version_min) { - vchiq_loud_error_header(); - vchiq_loud_error("Incompatible VCHIQ versions found."); - vchiq_loud_error("slot_zero=%pK: version=%d (VideoCore minimum %d)", - slot_zero, VCHIQ_VERSION, slot_zero->version_min); - vchiq_loud_error("Restart with a newer kernel."); - vchiq_loud_error_footer(); - return VCHIQ_ERROR; - } - - if ((slot_zero->slot_zero_size != sizeof(VCHIQ_SLOT_ZERO_T)) || -(slot_zero->slot_size != VCHIQ_SLOT_SIZE) || -(slot_zero->max_slots != VCHIQ_MAX_SLOTS) || -(slot_zero->max_slots_per_side != VCHIQ_MAX_SLOTS_PER_SIDE)) { - vchiq_loud_error_header(); - if (slot_zero->slot_zero_size != sizeof(VCHIQ_SLOT_ZERO_T)) - vchiq_loud_error("slot_zero=%pK: slot_zero_size=%d (expected %d)", - slot_zero, slot_zero->slot_zero_size, - (int)sizeof(VCHIQ_SLOT_ZERO_T)); - if (slot_zero->slot_size != VCHIQ_SLOT_SIZE) - vchiq_loud_error("slot_zero=%pK: slot_size=%d (expected %d)", - slot_zero, slot_zero->slot_size, - VCHIQ_SLOT_SIZE); - if (slot_zero->max_slots != VCHIQ_MAX_SLOTS) - vchiq_loud_error("slot_zero=%pK: max_slots=%d (expected %d)", - slot_zero, slot_zero->max_slots, - VCHIQ_MAX_SLOTS); - if (slot_zero->max_slots_per_side != VCHIQ_MAX_SLOTS_PER_SIDE) - vchiq_loud_error("slot_zero=%pK: max_slots_per_side=%d (expected %d)", - slot_zero, slot_zero->max_slots_per_side, - VCHIQ_MAX_SLOTS_PER_SIDE); - vchiq_loud_error_footer(); - return VCHIQ_ERROR; - } - - if (VCHIQ_VERSION < slot_zero->version) - slot_zero->version = VCHIQ_VERSION; - local = &slot_zero->slave; remote = &slot_zero->master; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 01/18] staging: vchiq_core: rework vchiq_get_config
The function is overly complicated for what it's ultimately achieving. It's simply filling up a structure. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 12 .../interface/vchiq_arm/vchiq_core.c | 30 +-- .../interface/vchiq_arm/vchiq_if.h| 3 +- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index ea789376de0f..6d503392341e 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -1480,13 +1480,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) ret = -EINVAL; break; } - status = vchiq_get_config(instance, args.config_size, &config); - if (status == VCHIQ_SUCCESS) { - if (copy_to_user((void __user *)args.pconfig, - &config, args.config_size) != 0) { - ret = -EFAULT; - break; - } + + vchiq_get_config(&config); + if (copy_to_user(args.pconfig, &config, args.config_size)) { + ret = -EFAULT; + break; } } break; diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 7642ced31436..89f1ccdc3b98 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -3583,28 +3583,14 @@ vchiq_get_peer_version(VCHIQ_SERVICE_HANDLE_T handle, short *peer_version) return status; } -VCHIQ_STATUS_T -vchiq_get_config(VCHIQ_INSTANCE_T instance, - int config_size, VCHIQ_CONFIG_T *pconfig) -{ - VCHIQ_CONFIG_T config; - - (void)instance; - - config.max_msg_size = VCHIQ_MAX_MSG_SIZE; - config.bulk_threshold = VCHIQ_MAX_MSG_SIZE; - config.max_outstanding_bulks = VCHIQ_NUM_SERVICE_BULKS; - config.max_services = VCHIQ_MAX_SERVICES; - config.version= VCHIQ_VERSION; - config.version_min= VCHIQ_VERSION_MIN; - - if (config_size > sizeof(VCHIQ_CONFIG_T)) - return VCHIQ_ERROR; - - memcpy(pconfig, &config, - min(config_size, (int)(sizeof(VCHIQ_CONFIG_T; - - return VCHIQ_SUCCESS; +void vchiq_get_config(VCHIQ_CONFIG_T *config) +{ + config->max_msg_size = VCHIQ_MAX_MSG_SIZE; + config->bulk_threshold = VCHIQ_MAX_MSG_SIZE; + config->max_outstanding_bulks = VCHIQ_NUM_SERVICE_BULKS; + config->max_services = VCHIQ_MAX_SERVICES; + config->version= VCHIQ_VERSION; + config->version_min= VCHIQ_VERSION_MIN; } VCHIQ_STATUS_T diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h index e4109a83e628..87829a244465 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h @@ -164,8 +164,7 @@ extern VCHIQ_STATUS_T vchiq_bulk_receive_handle(VCHIQ_SERVICE_HANDLE_T service, extern int vchiq_get_client_id(VCHIQ_SERVICE_HANDLE_T service); extern void *vchiq_get_service_userdata(VCHIQ_SERVICE_HANDLE_T service); extern int vchiq_get_service_fourcc(VCHIQ_SERVICE_HANDLE_T service); -extern VCHIQ_STATUS_T vchiq_get_config(VCHIQ_INSTANCE_T instance, - int config_size, VCHIQ_CONFIG_T *pconfig); +extern void vchiq_get_config(VCHIQ_CONFIG_T *config); extern VCHIQ_STATUS_T vchiq_set_service_option(VCHIQ_SERVICE_HANDLE_T service, VCHIQ_SERVICE_OPTION_T option, int value); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 10/18] staging: vchiq_core: don't add a wmb() before remote_event_signal()
It's the first thing remote_event_signal() does. Signed-off-by: Nicolas Saenz Julienne --- .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c| 4 1 file changed, 4 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 6242357735b9..e8b7ccabd9cb 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -1138,9 +1138,6 @@ queue_message_sync(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service, size); } - /* Make sure the new header is visible to the peer. */ - wmb(); - remote_event_signal(&state->remote->sync_trigger); if (VCHIQ_MSG_TYPE(msgid) != VCHIQ_MSG_PAUSE) @@ -3265,7 +3262,6 @@ static void release_message_sync(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header) { header->msgid = VCHIQ_MSGID_PADDING; - wmb(); remote_event_signal(&state->remote->sync_release); } -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 00/18] staging: vchiq: remove dead code & misc fixes
Hi All, This series was written in parallel with reading and understanding the vchiq code. So excuse me for the lack of logic in the sequence of patches. It's an RFC for various reasons, first I think it's going to clash with the last Stefan's series. Also I'm not used to doing big series. The main focus was to delete as much code as possible, I've counted around 550 lines, which is not bad. Apart from that there are some patches enforcing proper kernel APIs usage. The only patch that really changes code is the vchiq_ioc_copy_element_data() rewrite, I was having doubts whether to add it into the series, but since it's a RFC, I don't think it hurts. The last commit updates the TODO list with some of my observations, I realise some of the might be a little opinionated. If anything it's going to force a discussion on the topic, which is nice. The series was developed on top of linux-next, and was tested on a RPIv3B+ with audio, video and running vchiq_test. Regards, Nicolas === Nicolas Saenz Julienne (18): staging: vchiq_core: rework vchiq_get_config staging: vchiq_arm: rework close/remove_service IOCTLS staging: vchiq_shim: delete vchi_service_create stagning: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list staging: vchiq_arm: get rid of vchi_mh.h staging: vchiq_arm: rework vchiq_ioc_copy_element_data staging: vchiq-core: get rid of is_master distinction staging: vchiq_core: remove unnecessary safety checks in vchiq_init_state staging: vchiq_core: do not initialize semaphores twice staging: vchiq_core: don't add a wmb() before remote_event_signal() staging: vchiq_arm: use completions instead of semaphores staging: vchiq_util: use completions instead of semaphores staging: vchiq_core: use completions instead of semaphores staging: vchiq_util: get rid of unneeded memory barriers stagning: vchiq_core: fix logic redundancy in parse_open staging: vchiq_arm: rework probe and init functions staging: vchiq_arm: fix open/release cdev functions staging: vchiq: add more tasks to the TODO list .../staging/vc04_services/interface/vchi/TODO | 46 +- .../vc04_services/interface/vchi/vchi.h | 8 - .../vc04_services/interface/vchi/vchi_mh.h| 42 -- .../interface/vchiq_arm/vchiq_2835_arm.c | 18 +- .../interface/vchiq_arm/vchiq_arm.c | 598 -- .../interface/vchiq_arm/vchiq_core.c | 511 +++ .../interface/vchiq_arm/vchiq_core.h | 47 +- .../interface/vchiq_arm/vchiq_if.h| 11 +- .../interface/vchiq_arm/vchiq_shim.c | 32 - .../interface/vchiq_arm/vchiq_util.c | 48 +- .../interface/vchiq_arm/vchiq_util.h | 6 +- 11 files changed, 429 insertions(+), 938 deletions(-) delete mode 100644 drivers/staging/vc04_services/interface/vchi/vchi_mh.h -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 11/18] staging: vchiq_arm: use completions instead of semaphores
It is preferred in the kernel to avoid using semaphores to wait for events, as they are optimised for the opposite situation; where the common case is that they are available and may block only occasionally. FYI see this thread: https://lkml.org/lkml/2008/4/11/323. Also completions are semantically more explicit in this case. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 58 ++- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 1cdfdb714abc..463d0eec2e96 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -44,7 +44,7 @@ #include #include #include -#include +#include #include #include #include @@ -121,9 +121,9 @@ typedef struct user_service_struct { int message_available_pos; int msg_insert; int msg_remove; - struct semaphore insert_event; - struct semaphore remove_event; - struct semaphore close_event; + struct completion insert_event; + struct completion remove_event; + struct completion close_event; VCHIQ_HEADER_T * msg_queue[MSG_QUEUE_SIZE]; } USER_SERVICE_T; @@ -138,8 +138,8 @@ struct vchiq_instance_struct { VCHIQ_COMPLETION_DATA_T completions[MAX_COMPLETIONS]; int completion_insert; int completion_remove; - struct semaphore insert_event; - struct semaphore remove_event; + struct completion insert_event; + struct completion remove_event; struct mutex completion_mutex; int connected; @@ -562,7 +562,8 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason, vchiq_log_trace(vchiq_arm_log_level, "%s - completion queue full", __func__); DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT); - if (down_interruptible(&instance->remove_event) != 0) { + if (wait_for_completion_interruptible( + &instance->remove_event)) { vchiq_log_info(vchiq_arm_log_level, "service_callback interrupted"); return VCHIQ_RETRY; @@ -600,7 +601,7 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason, insert++; instance->completion_insert = insert; - up(&instance->insert_event); + complete(&instance->insert_event); return VCHIQ_SUCCESS; } @@ -673,7 +674,8 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T *header, } DEBUG_TRACE(SERVICE_CALLBACK_LINE); - if (down_interruptible(&user_service->remove_event) + if (wait_for_completion_interruptible( + &user_service->remove_event) != 0) { vchiq_log_info(vchiq_arm_log_level, "%s interrupted", __func__); @@ -705,7 +707,7 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T *header, } spin_unlock(&msg_queue_spinlock); - up(&user_service->insert_event); + complete(&user_service->insert_event); header = NULL; } @@ -745,7 +747,7 @@ static void close_delivered(USER_SERVICE_T *user_service) unlock_service(user_service->service); /* Wake the user-thread blocked in close_ or remove_service */ - up(&user_service->close_event); + complete(&user_service->close_event); user_service->close_pending = 0; } @@ -867,7 +869,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if (status == VCHIQ_SUCCESS) { /* Wake the completion thread and ask it to exit */ instance->closing = 1; - up(&instance->insert_event); + complete(&instance->insert_event); } break; @@ -948,9 +950,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) instance->completion_remove - 1; user_service->msg_insert = 0; user_service->msg_remove = 0; - sema_init(&user_service->insert_event, 0); - sema_init(&user_service->remove_event, 0); - sema_init(&user_service->close_event, 0); + init_completion(&user_service->insert_event); + init_completion(&user_service->remove_event); + init_completion(&user_service->close_event);
[PATCH RFC 07/18] staging: vchiq-core: get rid of is_master distinction
VCHIQ bulk transfers are what most people call DMA transfers. The CPU sends a list of physical addresses to the VideoCore which then access the memory directly without the need for CPU interaction. With this setup we call the CPU the "slave" and the VideoCore the "master". There seems to be an option to switch roles in vchiq. Which nobody is using nor is properly implemented. So we get rid of the "is_master == 1" option, and all the related code. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_2835_arm.c | 12 +- .../interface/vchiq_arm/vchiq_core.c | 300 +++--- .../interface/vchiq_arm/vchiq_core.h | 11 +- 3 files changed, 38 insertions(+), 285 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c index 014583cdf367..ecee54a31f8d 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c @@ -163,7 +163,7 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state) *(char **)&g_fragments_base[i * g_fragments_size] = NULL; sema_init(&g_free_fragments_sema, MAX_FRAGMENTS); - if (vchiq_init_state(state, vchiq_slot_zero, 0) != VCHIQ_SUCCESS) + if (vchiq_init_state(state, vchiq_slot_zero) != VCHIQ_SUCCESS) return -EINVAL; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -278,16 +278,6 @@ vchiq_complete_bulk(VCHIQ_BULK_T *bulk) bulk->actual); } -void -vchiq_transfer_bulk(VCHIQ_BULK_T *bulk) -{ - /* -* This should only be called on the master (VideoCore) side, but -* provide an implementation to avoid the need for ifdefery. -*/ - BUG(); -} - void vchiq_dump_platform_state(void *dump_context) { diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 8c7bda2e7cb6..34a892011296 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -85,8 +85,6 @@ int vchiq_core_log_level = VCHIQ_LOG_DEFAULT; int vchiq_core_msg_log_level = VCHIQ_LOG_DEFAULT; int vchiq_sync_log_level = VCHIQ_LOG_DEFAULT; -static atomic_t pause_bulks_count = ATOMIC_INIT(0); - static DEFINE_SPINLOCK(service_spinlock); DEFINE_SPINLOCK(bulk_waiter_spinlock); static DEFINE_SPINLOCK(quota_spinlock); @@ -1222,32 +1220,7 @@ notify_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue, (queue == &service->bulk_tx) ? 't' : 'r', queue->process, queue->remote_notify, queue->remove); - if (service->state->is_master) { - while (queue->remote_notify != queue->process) { - VCHIQ_BULK_T *bulk = - &queue->bulks[BULK_INDEX(queue->remote_notify)]; - int msgtype = (bulk->dir == VCHIQ_BULK_TRANSMIT) ? - VCHIQ_MSG_BULK_RX_DONE : VCHIQ_MSG_BULK_TX_DONE; - int msgid = VCHIQ_MAKE_MSG(msgtype, service->localport, - service->remoteport); - /* Only reply to non-dummy bulk requests */ - if (bulk->remote_data) { - status = queue_message( - service->state, - NULL, - msgid, - memcpy_copy_callback, - &bulk->actual, - 4, - 0); - if (status != VCHIQ_SUCCESS) - break; - } - queue->remote_notify++; - } - } else { - queue->remote_notify = queue->process; - } + queue->remote_notify = queue->process; if (status == VCHIQ_SUCCESS) { while (queue->remove != queue->remote_notify) { @@ -1385,63 +1358,6 @@ poll_services(VCHIQ_STATE_T *state) } } -/* Called by the slot handler or application threads, holding the bulk mutex. */ -static int -resolve_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue) -{ - VCHIQ_STATE_T *state = service->state; - int resolved = 0; - - while ((queue->process != queue->local_insert) && - (queue->process != queue->remote_insert)) { - VCHIQ_BULK_T *bulk = &queue->bulks[BULK_INDEX(queue->process)]; - - vchiq_log_trace(vchiq_core_log_level, - "%d: rb:%d %cx - li=%x ri=%x p=%x", -
[PATCH RFC 17/18] staging: vchiq_arm: fix open/release cdev functions
Both functions checked the minor number of the cdev prior running the code. This was useless since the number of devices is already limited by alloc_chrdev_region. This removes the check and reindents the code where relevant. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 247 +++--- 1 file changed, 100 insertions(+), 147 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index a7dcced79980..153a396d21bd 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -63,8 +63,6 @@ #undef MODULE_PARAM_PREFIX #define MODULE_PARAM_PREFIX DEVICE_NAME "." -#define VCHIQ_MINOR 0 - /* Some per-instance constants */ #define MAX_COMPLETIONS 128 #define MAX_SERVICES 64 @@ -1950,195 +1948,150 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) #endif -/ -* -* vchiq_open -* -***/ - -static int -vchiq_open(struct inode *inode, struct file *file) +static int vchiq_open(struct inode *inode, struct file *file) { - int dev = iminor(inode) & 0x0f; + VCHIQ_STATE_T *state = vchiq_get_state(); + VCHIQ_INSTANCE_T instance; vchiq_log_info(vchiq_arm_log_level, "vchiq_open"); - switch (dev) { - case VCHIQ_MINOR: { - VCHIQ_STATE_T *state = vchiq_get_state(); - VCHIQ_INSTANCE_T instance; - if (!state) { - vchiq_log_error(vchiq_arm_log_level, + if (!state) { + vchiq_log_error(vchiq_arm_log_level, "vchiq has no connection to VideoCore"); - return -ENOTCONN; - } - - instance = kzalloc(sizeof(*instance), GFP_KERNEL); - if (!instance) - return -ENOMEM; + return -ENOTCONN; + } - instance->state = state; - instance->pid = current->tgid; + instance = kzalloc(sizeof(*instance), GFP_KERNEL); + if (!instance) + return -ENOMEM; - vchiq_debugfs_add_instance(instance); + instance->state = state; + instance->pid = current->tgid; - init_completion(&instance->insert_event); - init_completion(&instance->remove_event); - mutex_init(&instance->completion_mutex); - mutex_init(&instance->bulk_waiter_list_mutex); - INIT_LIST_HEAD(&instance->bulk_waiter_list); + vchiq_debugfs_add_instance(instance); - file->private_data = instance; - } break; + init_completion(&instance->insert_event); + init_completion(&instance->remove_event); + mutex_init(&instance->completion_mutex); + mutex_init(&instance->bulk_waiter_list_mutex); + INIT_LIST_HEAD(&instance->bulk_waiter_list); - default: - vchiq_log_error(vchiq_arm_log_level, - "Unknown minor device: %d", dev); - return -ENXIO; - } + file->private_data = instance; return 0; } -/ -* -* vchiq_release -* -***/ - -static int -vchiq_release(struct inode *inode, struct file *file) +static int vchiq_release(struct inode *inode, struct file *file) { - int dev = iminor(inode) & 0x0f; + VCHIQ_INSTANCE_T instance = file->private_data; + VCHIQ_STATE_T *state = vchiq_get_state(); + VCHIQ_SERVICE_T *service; int ret = 0; + int i; - switch (dev) { - case VCHIQ_MINOR: { - VCHIQ_INSTANCE_T instance = file->private_data; - VCHIQ_STATE_T *state = vchiq_get_state(); - VCHIQ_SERVICE_T *service; - int i; + vchiq_log_info(vchiq_arm_log_level, "%s: instance=%lx", __func__, + (unsigned long)instance); - vchiq_log_info(vchiq_arm_log_level, - "%s: instance=%lx", - __func__, (unsigned long)instance); + if (!state) { + ret = -EPERM; + goto out; + } - if (!state) { - ret = -EPERM; - goto out; - } + /* Ensure videocore is awake to allow termination. */ + vchiq_use_internal(instance->state, NULL, USE_TYPE_VCHIQ); - /* Ensure videocore is awake to allow termination. */ - vchiq_use_internal(instance->state, NULL, - USE_TYPE_VCHIQ); + mutex_lock(&i
[PATCH RFC 18/18] staging: vchiq: add more tasks to the TODO list
The more the better. Signed-off-by: Nicolas Saenz Julienne --- .../staging/vc04_services/interface/vchi/TODO | 46 ++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchi/TODO b/drivers/staging/vc04_services/interface/vchi/TODO index 0b3ec75ff627..bf2135826431 100644 --- a/drivers/staging/vc04_services/interface/vchi/TODO +++ b/drivers/staging/vc04_services/interface/vchi/TODO @@ -27,8 +27,8 @@ unused. 3) Make driver more portable Building this driver with arm/multi_v7_defconfig or arm64/defconfig -leads to data corruption during the following command: - +leads to data corruption during the following command: + vchiq_test -f 1 This should be fixed. @@ -49,3 +49,45 @@ such as dev_info, dev_dbg, and friends. A short top-down description of this driver's architecture (function of kthreads, userspace, limitations) could be very helpful for reviewers. + +7) Review and comment memory barriers + +There is a heavy use of memory barriers in this driver, it would be very +beneficial to go over all of them and, if correct, comment on their merits. +Extra points to whomever confidently reviews the remote_event_*() family of +functions. + +8) Get rid of custom function return values + +Most functions use a custom set of return values, we should force proper Linux +error numbers. Special care is needed for VCHIQ_RETRY. + +9) Reformat core code with more sane indentations + +The code follows the 80 characters limitation yet tends to go 3 or 4 levels of +indentation deep making it very unpleasant to read. This is specially relevant +in the character driver ioctl code and in the core thread functions. + +10) Reorganize file structure: Move char driver to it's own file and join both +platform files + +The cdev is defined alongside with the platform code in vchiq_arm.c. It would +be nice to completely decouple it from the actual core code. For instance to be +able to use bcm2835-audio without having /dev/vchiq created. One could argue +it's better for security reasons or general cleanliness. It could even be +interesting to create two different kernel modules, something the likes of +vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process. + +The code in vchiq_bcm2835_arm.c should fit in the generic platform file. + +12) Get rid of all the struct typedefs + +Most structs are typedefd, it's not encouraged in the kernel. + +13) Get rid of all non essential global structures and create a proper per +device structure + +The first thing one generally sees in a probe function is a memory allocation +for all the device specific data. This structure is then passed all over the +driver. This is good practice since it makes the driver work regardless of the +number of devices probed. -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 04/18] stagning: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list
The resulting code is way more readeable and intuitive compared to plain list_for_each. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 52 ++- 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index d88dd7415e1e..5f55c708ade8 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -280,16 +280,11 @@ VCHIQ_STATUS_T vchiq_shutdown(VCHIQ_INSTANCE_T instance) "%s(%p): returning %d", __func__, instance, status); if (status == VCHIQ_SUCCESS) { - struct list_head *pos, *next; + struct bulk_waiter_node *waiter, *next; - list_for_each_safe(pos, next, - &instance->bulk_waiter_list) { - struct bulk_waiter_node *waiter; - - waiter = list_entry(pos, - struct bulk_waiter_node, - list); - list_del(pos); + list_for_each_entry_safe(waiter, next, +&instance->bulk_waiter_list, list) { + list_del(&waiter->list); vchiq_log_info(vchiq_arm_log_level, "bulk_waiter - cleaned up %pK for pid %d", waiter, waiter->pid); @@ -473,7 +468,6 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, void *data, VCHIQ_SERVICE_T *service; VCHIQ_STATUS_T status; struct bulk_waiter_node *waiter = NULL; - struct list_head *pos; service = find_service_by_handle(handle); if (!service) @@ -484,13 +478,9 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, void *data, unlock_service(service); mutex_lock(&instance->bulk_waiter_list_mutex); - list_for_each(pos, &instance->bulk_waiter_list) { - if (list_entry(pos, struct bulk_waiter_node, - list)->pid == current->pid) { - waiter = list_entry(pos, - struct bulk_waiter_node, - list); - list_del(pos); + list_for_each_entry(waiter, &instance->bulk_waiter_list, list) { + if (waiter->pid == current->pid) { + list_del(&waiter->list); break; } } @@ -1135,21 +1125,16 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) ret = -ENOMEM; break; } + args.userdata = &waiter->bulk_waiter; } else if (args.mode == VCHIQ_BULK_MODE_WAITING) { - struct list_head *pos; - mutex_lock(&instance->bulk_waiter_list_mutex); - list_for_each(pos, &instance->bulk_waiter_list) { - if (list_entry(pos, struct bulk_waiter_node, - list)->pid == current->pid) { - waiter = list_entry(pos, - struct bulk_waiter_node, - list); - list_del(pos); + list_for_each_entry(waiter, &instance->bulk_waiter_list, + list) { + if (waiter->pid == current->pid) { + list_del(&waiter->list); break; } - } mutex_unlock(&instance->bulk_waiter_list_mutex); if (!waiter) { @@ -2158,16 +2143,11 @@ vchiq_release(struct inode *inode, struct file *file) vchiq_release_internal(instance->state, NULL); { - struct list_head *pos, *next; - - list_for_each_safe(pos, next, - &instance->bulk_waiter_list) { - struct bulk_waiter_node *waiter; + struct bulk_waiter_node *waiter, *next; - waiter = list_entry(pos, - struct bulk_waiter_node, - list); - list_del(pos); + list_for_each_entry_safe(waiter, next, + &instance->bulk_waiter_list, list) { + list_del(&waiter->list
[PATCH RFC 13/18] staging: vchiq_core: use completions instead of semaphores
It is preferred in the kernel to avoid using semaphores to wait for events as, they are optimised for the opposite situation; where the common case is that they are available and may block only occasionally. FYI see this thread: https://lkml.org/lkml/2008/4/11/323. Also completions are semantically more explicit in this case. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 2 +- .../interface/vchiq_arm/vchiq_core.c | 94 +-- .../interface/vchiq_arm/vchiq_core.h | 26 ++--- 3 files changed, 61 insertions(+), 61 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 463d0eec2e96..383013a92939 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2063,7 +2063,7 @@ vchiq_release(struct inode *inode, struct file *file) != NULL) { USER_SERVICE_T *user_service = service->base.userdata; - down(&service->remove_event); + wait_for_completion(&service->remove_event); BUG_ON(service->srvstate != VCHIQ_SRVSTATE_FREE); diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index e8b7ccabd9cb..0c37b7032857 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -376,7 +376,7 @@ mark_service_closing_internal(VCHIQ_SERVICE_T *service, int sh_thread) /* Unblock any sending thread. */ service_quota = &state->service_quotas[service->localport]; - up(&service_quota->quota_event); + complete(&service_quota->quota_event); } static void @@ -423,7 +423,7 @@ remote_event_create(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event) event->armed = 0; /* Don't clear the 'fired' flag because it may already have been set ** by the other side. */ - sema_init((struct semaphore *)((char *)state + event->event), 0); + init_completion((struct completion *)((char *)state + event->event)); } static inline int @@ -433,9 +433,9 @@ remote_event_wait(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event) event->armed = 1; dsb(sy); if (!event->fired) { - if (down_interruptible( - (struct semaphore *) - ((char *)state + event->event)) != 0) { + if (wait_for_completion_interruptible( + (struct completion *) + ((char *)state + event->event))) { event->armed = 0; return 0; } @@ -452,7 +452,7 @@ static inline void remote_event_signal_local(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event) { event->armed = 0; - up((struct semaphore *)((char *)state + event->event)); + complete((struct completion *)((char *)state + event->event)); } static inline void @@ -582,7 +582,7 @@ reserve_space(VCHIQ_STATE_T *state, size_t space, int is_blocking) /* If there is no free slot... */ - if (down_trylock(&state->slot_available_event) != 0) { + if (!try_wait_for_completion(&state->slot_available_event)) { /* ...wait for one. */ VCHIQ_STATS_INC(state, slot_stalls); @@ -593,13 +593,13 @@ reserve_space(VCHIQ_STATE_T *state, size_t space, int is_blocking) remote_event_signal(&state->remote->trigger); if (!is_blocking || - (down_interruptible( - &state->slot_available_event) != 0)) + (wait_for_completion_interruptible( + &state->slot_available_event))) return NULL; /* No space available */ } if (tx_pos == (state->slot_queue_available * VCHIQ_SLOT_SIZE)) { - up(&state->slot_available_event); + complete(&state->slot_available_event); pr_warn("%s: invalid tx_pos: %d\n", __func__, tx_pos); return NULL; } @@ -679,7 +679,7 @@ process_free_queue(VCHIQ_STATE_T *state, BITSET_T *service_found, size_t length) /* Signal the service that it ** has dropped below its quota */ - up(&service_quota->quota_event); +
[PATCH RFC 16/18] staging: vchiq_arm: rework probe and init functions
Some operations performed in the probe function should have been implemented in the init function. Namely class and dev region creations. Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 71 --- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 383013a92939..a7dcced79980 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -166,7 +166,6 @@ static struct cdevvchiq_cdev; static dev_t vchiq_devid; static VCHIQ_STATE_T g_state; static struct class *vchiq_class; -static struct device *vchiq_dev; static DEFINE_SPINLOCK(msg_queue_spinlock); static struct platform_device *bcm2835_camera; @@ -3552,34 +3551,19 @@ static int vchiq_probe(struct platform_device *pdev) if (err != 0) goto failed_platform_init; - err = alloc_chrdev_region(&vchiq_devid, VCHIQ_MINOR, 1, DEVICE_NAME); - if (err != 0) { - vchiq_log_error(vchiq_arm_log_level, - "Unable to allocate device number"); - goto failed_platform_init; - } cdev_init(&vchiq_cdev, &vchiq_fops); vchiq_cdev.owner = THIS_MODULE; err = cdev_add(&vchiq_cdev, vchiq_devid, 1); if (err != 0) { vchiq_log_error(vchiq_arm_log_level, "Unable to register device"); - goto failed_cdev_add; + goto failed_platform_init; } - /* create sysfs entries */ - vchiq_class = class_create(THIS_MODULE, DEVICE_NAME); - err = PTR_ERR(vchiq_class); - if (IS_ERR(vchiq_class)) - goto failed_class_create; - - vchiq_dev = device_create(vchiq_class, NULL, - vchiq_devid, NULL, "vchiq"); - err = PTR_ERR(vchiq_dev); - if (IS_ERR(vchiq_dev)) + if (IS_ERR(device_create(vchiq_class, &pdev->dev, vchiq_devid, +NULL, "vchiq"))) goto failed_device_create; - /* create debugfs entries */ vchiq_debugfs_init(); vchiq_log_info(vchiq_arm_log_level, @@ -3594,11 +3578,7 @@ static int vchiq_probe(struct platform_device *pdev) return 0; failed_device_create: - class_destroy(vchiq_class); -failed_class_create: cdev_del(&vchiq_cdev); -failed_cdev_add: - unregister_chrdev_region(vchiq_devid, 1); failed_platform_init: vchiq_log_warning(vchiq_arm_log_level, "could not load vchiq"); return err; @@ -3609,9 +3589,7 @@ static int vchiq_remove(struct platform_device *pdev) platform_device_unregister(bcm2835_camera); vchiq_debugfs_deinit(); device_destroy(vchiq_class, vchiq_devid); - class_destroy(vchiq_class); cdev_del(&vchiq_cdev); - unregister_chrdev_region(vchiq_devid, 1); return 0; } @@ -3624,7 +3602,48 @@ static struct platform_driver vchiq_driver = { .probe = vchiq_probe, .remove = vchiq_remove, }; -module_platform_driver(vchiq_driver); + +static int __init vchiq_driver_init(void) +{ + int ret; + + vchiq_class = class_create(THIS_MODULE, DEVICE_NAME); + if (IS_ERR(vchiq_class)) { + pr_err("Failed to create vchiq class\n"); + return PTR_ERR(vchiq_class); + } + + ret = alloc_chrdev_region(&vchiq_devid, VCHIQ_MINOR, 1, DEVICE_NAME); + if (ret) { + pr_err("Failed to allocate vchiq's chrdev region\n"); + goto class_destroy; + } + + ret = platform_driver_register(&vchiq_driver); + if (ret) { + pr_err("Failed to register vchiq driver\n"); + goto region_unregister; + } + + return 0; + +region_unregister: + platform_driver_unregister(&vchiq_driver); + +class_destroy: + class_destroy(vchiq_class); + + return ret; +} +module_init(vchiq_driver_init); + +static void __exit vchiq_driver_exit(void) +{ + platform_driver_unregister(&vchiq_driver); + unregister_chrdev_region(vchiq_devid, 1); + class_destroy(vchiq_class); +} +module_exit(vchiq_driver_exit); MODULE_LICENSE("Dual BSD/GPL"); MODULE_DESCRIPTION("Videocore VCHIQ driver"); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 15/18] stagning: vchiq_core: fix logic redundancy in parse_open
We update sync to reflect that the firmware version is compatible with that option. We don't need to check both of them again further down the code. Signed-off-by: Nicolas Saenz Julienne --- .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c| 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 0c37b7032857..a5664a44258e 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -1462,9 +1462,7 @@ parse_open(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header) service->sync = 0; /* Acknowledge the OPEN */ - if (service->sync && - (state->version_common >= -VCHIQ_VERSION_SYNCHRONOUS_MODE)) { + if (service->sync) { if (queue_message_sync( state, NULL, -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 0/2] staging: iio: ad2s1210: Switch to the gpio descriptor interface.
Use the gpiod interface instead of the deprecated old non-descriptor Changes in v5: - Add device tree support. - Add device tree table for matching vendor ID. - Add Support for retrieving platform data from device tree. Changes in v4: - Add spaces after { and before } in gpios[] initialization. - Check the correct pointer for error. - Align the dev_err msg to existing format in the code. Changes in v3: - Use a pointer to pointer for gpio_desc in struct ad2s1210_gpio as it will be used to modify a pointer. - Use dot notation to initialize the structure. - Use a pointer variable to avoid writing gpios[i]. Changes in v2: - Use the spi_device struct embedded in st instead of passing it as an argument to ad2s1210_setup_gpios(). - Use an array of structs to reduce redundant code in in ad2s1210_setup_gpios(). - Remove ad2s1210_free_gpios() as devm API is being used. - Nishad Kamdar (2): staging: iio: ad2s1210: Switch to the gpio descriptor interface staging: iio: ad2s1210: Add device tree support. drivers/staging/iio/resolver/ad2s1210.c | 129 drivers/staging/iio/resolver/ad2s1210.h | 3 - 2 files changed, 89 insertions(+), 43 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 2/2] staging: iio: ad2s1210: Add device tree support.
Add device tree table for matching vendor ID and support for retrieving platform data from device tree. Signed-off-by: Nishad Kamdar --- drivers/staging/iio/resolver/ad2s1210.c | 43 - 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c index 93c3c70ce62e..9fd5461c4ed0 100644 --- a/drivers/staging/iio/resolver/ad2s1210.c +++ b/drivers/staging/iio/resolver/ad2s1210.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st) return 0; } +#ifdef CONFIG_OF +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev) +{ + struct device_node *np = dev->of_node; + struct ad2s1210_platform_data *pdata; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return NULL; + + pdata->gpioin = of_property_read_bool(np, "adi,gpioin"); + + return pdata; +} +#else +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev) +{ + return NULL; +} +#endif + static int ad2s1210_probe(struct spi_device *spi) { struct iio_dev *indio_dev; @@ -682,7 +704,19 @@ static int ad2s1210_probe(struct spi_device *spi) if (!indio_dev) return -ENOMEM; st = iio_priv(indio_dev); - st->pdata = spi->dev.platform_data; + if (spi->dev.of_node) { + st->pdata = ad2s1210_parse_dt(&spi->dev); + if (!st->pdata) + return -EINVAL; + } else { + st->pdata = spi->dev.platform_data; + } + + if (!st->pdata) { + dev_err(&spi->dev, "ad2s1210: no platform data supplied\n"); + return -EINVAL; + } + ret = ad2s1210_setup_gpios(st); if (ret < 0) return ret; @@ -724,6 +758,12 @@ static int ad2s1210_remove(struct spi_device *spi) return 0; } +static const struct of_device_id ad2s1210_of_match[] = { + { .compatible = "adi,ad2s1210", }, + { } +}; +MODULE_DEVICE_TABLE(of, ad2s1210_of_match); + static const struct spi_device_id ad2s1210_id[] = { { "ad2s1210" }, {} @@ -733,6 +773,7 @@ MODULE_DEVICE_TABLE(spi, ad2s1210_id); static struct spi_driver ad2s1210_driver = { .driver = { .name = DRV_NAME, + .of_match_table = of_match_ptr(ad2s1210_of_match), }, .probe = ad2s1210_probe, .remove = ad2s1210_remove, -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RESEND PATCH] staging: mt7621-pci: dt-bindings: add dt bindings for mt7621 pcie controller
This commit adds pci device tree bindings for the Mt7621 pci controller. This is a temporal file included in staging driver directory and will be moved to its correct location when this driver gets out of staging. Cc: Rob Herring Signed-off-by: Sergio Paracuellos --- I resend this because I don't get any feedback in more than two weeks. Rob, I was told by Greg that for getting this added into the staging tree your Ack is needed to in order to get bindings correct from the beginning. I forgot also to send this patch to the device tree mail list. Do it now. Thanks in advance for your time. Best regards, Sergio Paracuellos drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt | 100 + 1 file changed, 100 insertions(+) create mode 100644 drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt diff --git a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt new file mode 100644 index 000..4f3b082 --- /dev/null +++ b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt @@ -0,0 +1,100 @@ +MediaTek MT7621 PCIe controller + +Required properties: +- compatible: "mediatek,mt7621-pci" +- device_type: Must be "pci" +- reg: Base addresses and lengths of the PCIe subsys and root ports. +- bus-range: Range of bus numbers associated with this controller. +- #address-cells: Address representation for root ports (must be 3) +- pinctrl-names : The pin control state names. +- pinctrl-0: The "default" pinctrl state. +- #size-cells: Size representation for root ports (must be 2) +- ranges: Ranges for the PCI memory and I/O regions. +- #interrupt-cells: Must be 1 +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties. + Please refer to the standard PCI bus binding document for a more detailed + explanation. +- status: either "disabled" or "okay". +- resets: Must contain an entry for each entry in reset-names. + See ../reset/reset.txt for details. +- reset-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of + root ports. +- clocks: Must contain an entry for each entry in clock-names. + See ../clocks/clock-bindings.txt for details. +- clock-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of + root ports. + +In addition, the device tree node must have sub-nodes describing each PCIe port +interface, having the following mandatory properties: + +Required properties: +- reg: Only the first four bytes are used to refer to the correct bus number + and device number. +- #address-cells: Must be 3 +- #size-cells: Must be 2 +- ranges: Sub-ranges distributed from the PCIe controller node. An empty + property is sufficient. +- bus-range: Range of bus numbers associated with this port. + +Example for MT7621: + + pcie: pcie@1e14 { + compatible = "mediatek,mt7621-pci"; +reg = <0x1e14 0x100/* host-pci bridge registers */ + 0x1e142000 0x100/* pcie port 0 RC control registers */ + 0x1e143000 0x100/* pcie port 1 RC control registers */ + 0x1e144000 0x100/* pcie port 2 RC control registers */ + 0x1e00 0x100>; /* sysctl */ + + #address-cells = <3>; + #size-cells = <2>; + + pinctrl-names = "default"; + pinctrl-0 = <&pcie_pins>; + + device_type = "pci"; + + bus-range = <0 255>; + ranges = < + 0x0200 0 0x 0x6000 0 0x1000 /* pci memory */ + 0x0100 0 0x 0x1e16 0 0x0001 /* io space */ + >; + + #interrupt-cells = <1>; + interrupt-map-mask = <0xF 0 0 1>; + interrupt-map = <0x1 0 0 1 &gic GIC_SHARED 4 IRQ_TYPE_LEVEL_HIGH>, + <0x2 0 0 1 &gic GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>, + <0x3 0 0 1 &gic GIC_SHARED 25 IRQ_TYPE_LEVEL_HIGH>; + + status = "disabled"; + + resets = <&rstctrl 24 &rstctrl 25 &rstctrl 26>; + reset-names = "pcie0", "pcie1", "pcie2"; + clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>; + clock-names = "pcie0", "pcie1", "pcie2"; + + pcie@0,0 { + reg = <0x 0 0 0 0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + bus-range = <0x00 0xff>; + }; + + pcie@1,0 { + reg = <0x0800 0 0 0 0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + bus-range = <0x00 0xff>; + }; + + pcie@2,0 { + reg = <0x1000 0 0 0 0>; + #address-cells = <3>; + #size-cells = <2>; +
Re: [PATCH] binder: ipc namespace support for android binder
On Fri, Oct 26, 2018 at 2:20 AM chouryzhou(周威) wrote: > > Hi > We are working for running android in container, but we found that binder is > not isolated by ipc namespace. Since binder is a form of IPC and therefore > should > be tied to ipc namespace. With this patch, we can run more than one android > container on one host. > This patch move "binder_procs" and "binder_context" into ipc_namespace, > driver will find the context from it when opening. Althought statistics in > debugfs > remain global. > > Signed-off-by: choury zhou > --- > drivers/android/Kconfig | 2 +- > drivers/android/binder.c | 126 +- > include/linux/ipc_namespace.h | 14 > ipc/namespace.c | 4 ++ > 4 files changed, 111 insertions(+), 35 deletions(-) > > diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig > index 432e9ad77070..09883443b2da 100644 > --- a/drivers/android/Kconfig > +++ b/drivers/android/Kconfig > @@ -10,7 +10,7 @@ if ANDROID > > config ANDROID_BINDER_IPC > bool "Android Binder IPC Driver" > - depends on MMU > + depends on MMU && SYSVIPC NAK. We can't force SYSVIPC on for Android. The notion of running binder in a container is reasonable, but needs to be done without explicit requirement for SYSVIPC. binder-in-container is a topic in the android microconf at Linux plumbers -- are you going to be at LPC? It's not obvious from this patch where this dependency comes from...why is SYSVIPC required? I'd like to not have to require IPC_NS either for devices. > default n > ---help--- > Binder is used in Android for both communication between processes, > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index d58763b6b009..e061dba9b8b3 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -68,6 +68,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -79,13 +80,12 @@ > #include "binder_alloc.h" > #include "binder_trace.h" > > +#define ipcns (current->nsproxy->ipc_ns) > + > static HLIST_HEAD(binder_deferred_list); > static DEFINE_MUTEX(binder_deferred_lock); > > static HLIST_HEAD(binder_devices); > -static HLIST_HEAD(binder_procs); > -static DEFINE_MUTEX(binder_procs_lock); > - > static HLIST_HEAD(binder_dead_nodes); > static DEFINE_SPINLOCK(binder_dead_nodes_lock); > > @@ -231,7 +231,7 @@ struct binder_transaction_log_entry { > int return_error_line; > uint32_t return_error; > uint32_t return_error_param; > - const char *context_name; > + int context_device; > }; > struct binder_transaction_log { > atomic_t cur; > @@ -262,19 +262,66 @@ static struct binder_transaction_log_entry > *binder_transaction_log_add( > } > > struct binder_context { > + struct hlist_node hlist; > struct binder_node *binder_context_mgr_node; > struct mutex context_mgr_node_lock; > > kuid_t binder_context_mgr_uid; > - const char *name; > + intdevice; > }; > > struct binder_device { > struct hlist_node hlist; > struct miscdevice miscdev; > - struct binder_context context; > }; > > +void binder_exit_ns(struct ipc_namespace *ns) > +{ > + struct binder_context *context; > + struct hlist_node *tmp; > + > + mutex_destroy(&ns->binder_procs_lock); > + mutex_destroy(&ns->binder_contexts_lock); > + hlist_for_each_entry_safe(context, tmp, &ns->binder_contexts, hlist) { > + mutex_destroy(&context->context_mgr_node_lock); > + hlist_del(&context->hlist); > + kfree(context); > + } > +} > + > +int binder_init_ns(struct ipc_namespace *ns) > +{ > + int ret; > + struct binder_device *device; > + > + mutex_init(&ns->binder_procs_lock); > + INIT_HLIST_HEAD(&ns->binder_procs); > + mutex_init(&ns->binder_contexts_lock); > + INIT_HLIST_HEAD(&ns->binder_contexts); > + > + hlist_for_each_entry(device, &binder_devices, hlist) { > + struct binder_context *context; > + > + context = kzalloc(sizeof(*context), GFP_KERNEL); > + if (!context) { > + ret = -ENOMEM; > + goto err; > + } > + > + context->device = device->miscdev.minor; > + context->binder_context_mgr_uid = INVALID_UID; > + mutex_init(&context->context_mgr_node_lock); > + > + hlist_add_head(&context->hlist, &ns->binder_contexts); > + } > + > + return 0; > +err: > + binder_exit_ns(ns); > + return ret; > +} > + > + > /** > * struct binder_work - work enqueued on a worklist > * @entry: node enqueued on list > @@ -2748,7 +2795,7 @@ static void binder_transaction(struct binder_proc *proc, > e->target_handle = tr->target.handle; > e->data_size = tr->data_size
Re: [PATCH 0/7] staging: vc04_services: Some dead code removal
On Thu, 18 Oct 2018 at 10:38, Stefan Wahren wrote: > > Am 18.10.2018 um 11:22 schrieb Dave Stevenson: > > On Wed, 17 Oct 2018 at 17:51, Peter Robinson wrote: > >> Drop various pieces of dead code from here and there to get rid of > >> the remaining users of VCHI_CONNECTION_T. After that we get to drop > >> entire header files worth of unused code. > >> > >> I've tested on a Raspberry Pi Model B (bcm2835_defconfig) that > >> snd-bcm2835 can still play analog audio just fine. > >> > > thanks and i'm fine with your patch series: > > > > Acked-by: Stefan Wahren > > > > Unfortunately this would break compilation of the downstream vchi > > drivers like vcsm [1]. Personally i don't want to maintain another > > one, because i cannot see the gain of the resulting effort. > > > > [1] - > > https://github.com/raspberrypi/linux/tree/rpi-4.14.y/drivers/char/broadcom/vc_sm > > I feel like everyone else already knows the answer but why don't we just > merge that code into staging? > >>> Dave's been working on a new VCSM service where the firmware can call > >>> back into Linux to allocate (instead of just having a permanent carveout > >>> of system memory that the firmware allocates from), and lets us make > >>> dma-bufs out of those buffers. That driver makes a no-copies v4l2 media > >>> decode driver possible, which would then let Kodi and similar projects > >>> switch from downstream kernels with closed graphics to upstream kernels > >>> with open graphics. > >>> > >>> Given that the new VCSM service is a rewrite, it's not clear to me that > >>> importing the old VCSM driver is a win. But maybe we should go raid > >>> https://github.com/6by9/linux/tree/rpi-4.14.y-codecs-push-pt2a and grab > >>> the new drivers. Upstreaming the VCHI audio driver to staging has > >>> clearly been a win for it, so maybe other eyes on the new v4l2 codec > >>> could help Dave along in stabilizing it. > >> I think that makes sense as long as the firmware side changes are in > >> place so it can actually be used. > > The firmware has supported the necessary for dmabuf import since Sept 2017. > > > > The new vcsm driver currently only supports importing from other > > kernel modules as I cut it back to the bare minimum to ease > > upstreaming. To be a complete replacement of the existing then it > > needs to support userspace alloc/free/import/mmap. I did have most of > > that working, but will add it in stages. > > The codec code is working for decode but something is off for setting > > formats on encode. > > Both drivers are loading through DT at the moment as I couldn't get > > Eric's platform driver stuff working. IIRC A combination of modules > > not getting loaded and getting the appropriate coherent DMA mask set > > (being under soc in DT gives the correct mappings, but being a > > platform driver didn't). > > I'm working on these issues and i will post a proper solution soon. > > In case you need a hack in order to test your stuff, i can prepare a > branch for you. Thanks Stefan. I've picked up your latest patches which mean I can get the driver loaded via the (almost) approved method. I do seem to still have issues with not getting the expected address ranges, so the driver/VPU was trying to map cached alias memory. As your patches only came through yesterday I haven't had a chance to dig through why yet. I've done a temporary hack to ensure we always map the uncached alias, but that can't persist. > > > > I'm fire-fighting a networking issue at the moment, but hope to be > > back on codecs next week. > > Could you hold off raiding my trees until say Fri 26th Oct so I can > > ensure they are fully up to date? If I get a chance then I'll start > > the work of porting into staging before then. > > The merge window will open soon, so i don't see the need to hurry. The networking issue has been resolved :-) I've pushed where I've got to to https://github.com/6by9/linux/tree/rpi-4.14.y-codecs-push-pt2b It's a touch messy due to integrating in your patches in the last 24 hours. It needs a full rebase so that my changes are on top of yours rather than haphazard. As we're moving to 4.19 fairly soon I may well abandon my 4.14 tree and jump to either that or directly on staging. I'll see where I get to early next week. Dave ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
On Fri, Oct 26, 2018 at 7:04 AM Dan Carpenter wrote: > > On Thu, Oct 25, 2018 at 09:45:11PM -0300, Matheus Tavares wrote: > > From: Victor Colombo > > > > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and > > implements the relative read behavior at ad2s90_read_raw. > > > > Signed-off-by: Victor Colombo > > --- > > You should be adding your S-o-B here as well because the patch is > passing through your hands. Thanks for the review! I'll be sending a v2 with my S-o-B there. > regards, > dan carpenter > > -- > You received this message because you are subscribed to the Google Groups > "Kernel USP" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to kernel-usp+unsubscr...@googlegroups.com. > To post to this group, send email to kernel-...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/kernel-usp/20181026100422.lvz2avowd6ddix54%40mwanda. > For more options, visit https://groups.google.com/d/optout. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RESEND PATCH 0/2] staging: vboxvideo: Remove chekpatch issues
This series cleans the following checkpatch.pl issues: CHECK: Prefer kernel type 'u32' over 'uint32_t' CHECK: Avoid using bool structure members because of possible alignment issues Shayenne da Luz Moura (2): staging: vboxvideo: Change uint32_t to u32 staging: vboxvideo: Use unsigned int instead bool drivers/staging/vboxvideo/vbox_drv.h| 14 +++--- drivers/staging/vboxvideo/vbox_mode.c | 2 +- drivers/staging/vboxvideo/vboxvideo_guest.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RESEND PATCH 1/2] staging: vboxvideo: Change uint32_t to u32
This change was suggested by checkpath.pl. CHECK: Prefer kernel type 'u32' over 'uint32_t' Signed-off-by: Shayenne da Luz Moura --- drivers/staging/vboxvideo/vbox_mode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c index 79836c8fb909..8a1b117990b8 100644 --- a/drivers/staging/vboxvideo/vbox_mode.c +++ b/drivers/staging/vboxvideo/vbox_mode.c @@ -311,7 +311,7 @@ static int vbox_crtc_mode_set(struct drm_crtc *crtc, static int vbox_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, - uint32_t page_flip_flags, + u32 page_flip_flags, struct drm_modeset_acquire_ctx *ctx) { struct vbox_private *vbox = crtc->dev->dev_private; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool
This change was suggested by checkpath.pl. Use unsigned int with bitfield allocate only one bit to the boolean variable. CHECK: Avoid using bool structure members because of possible alignment issues Signed-off-by: Shayenne da Luz Moura --- drivers/staging/vboxvideo/vbox_drv.h| 14 +++--- drivers/staging/vboxvideo/vboxvideo_guest.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h index 594f84272957..7d3e329a6b1c 100644 --- a/drivers/staging/vboxvideo/vbox_drv.h +++ b/drivers/staging/vboxvideo/vbox_drv.h @@ -81,7 +81,7 @@ struct vbox_private { u8 __iomem *vbva_buffers; struct gen_pool *guest_pool; struct vbva_buf_ctx *vbva_info; - bool any_pitch; + unsigned int any_pitch:1; u32 num_crtcs; /** Amount of available VRAM, including space used for buffers. */ u32 full_vram_size; @@ -106,7 +106,7 @@ struct vbox_private { * depending on whether they react to a hot-plug event after the initial * mode query. */ - bool initial_mode_queried; + unsigned int initial_mode_queried:1; struct work_struct hotplug_work; u32 input_mapping_width; u32 input_mapping_height; @@ -114,7 +114,7 @@ struct vbox_private { * Is user-space using an X.Org-style layout of one large frame-buffer * encompassing all screen ones or is the fbdev console active? */ - bool single_framebuffer; + unsigned int single_framebuffer:1; u32 cursor_width; u32 cursor_height; u32 cursor_hot_x; @@ -139,17 +139,17 @@ struct vbox_connector { struct { u32 width; u32 height; - bool disconnected; + unsigned int disconnected:1; } mode_hint; }; struct vbox_crtc { struct drm_crtc base; - bool blanked; - bool disconnected; + unsigned int blanked:1; + unsigned int disconnected:1; unsigned int crtc_id; u32 fb_offset; - bool cursor_enabled; + unsigned int cursor_enabled:1; u32 x_hint; u32 y_hint; }; diff --git a/drivers/staging/vboxvideo/vboxvideo_guest.h b/drivers/staging/vboxvideo/vboxvideo_guest.h index d09da841711a..7a98fb4fb108 100644 --- a/drivers/staging/vboxvideo/vboxvideo_guest.h +++ b/drivers/staging/vboxvideo/vboxvideo_guest.h @@ -36,7 +36,7 @@ struct vbva_buf_ctx { /** Length of the buffer in bytes */ u32 buffer_length; /** Set if we wrote to the buffer faster than the host could read it */ - bool buffer_overflow; + unsigned int buffer_overflow:1; /** VBVA record that we are currently preparing for the host, or NULL */ struct vbva_record *record; /** -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool
On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote: This change was suggested by checkpath.pl. Use unsigned int with bitfield allocate only one bit to the boolean variable. CHECK: Avoid using bool structure members because of possible alignment issues Signed-off-by: Shayenne da Luz Moura --- drivers/staging/vboxvideo/vbox_drv.h| 14 +++--- drivers/staging/vboxvideo/vboxvideo_guest.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/vboxvideo/vbox_drv.h b/drivers/staging/vboxvideo/vbox_drv.h index 594f84272957..7d3e329a6b1c 100644 --- a/drivers/staging/vboxvideo/vbox_drv.h +++ b/drivers/staging/vboxvideo/vbox_drv.h @@ -81,7 +81,7 @@ struct vbox_private { u8 __iomem *vbva_buffers; struct gen_pool *guest_pool; struct vbva_buf_ctx *vbva_info; - bool any_pitch; + unsigned int any_pitch:1; u32 num_crtcs; /** Amount of available VRAM, including space used for buffers. */ u32 full_vram_size; Using bitfields for booleans in these cases is less efficient than just using "regular" booleans for two reasons: 1. It will use the same amount of space. Due to alignment requirements, the compiler can't squeeze in anything into the 7 bits that are now "free". Each member, unless it's another bitfield, must start at a whole byte. 2. This is actually less efficient (slower) for the compiler to work with. The smallest granularity we have to access memory is 1 byte; we can't set individual bits directly in memory. For the original code, the assembly for 'vbox_private.any_pitch = true' would look something like this: movl $0x1,-0x10(%rsp) As you can see, the compiler can directly write into the variable. However, when we switch to using bitfields, the compiler must preserve the original value of the other 7 bits, so it must first read them from memory, manipulate the value and write it back. The assembly would look something like this: movzbl -0x10(%rsp),%eax or $0x1,%eax mov%al,-0x10(%rsp) Which is less efficient than what was previously happening. -- Thanks, Sasha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool
[Adding Joe Perches] On Fri, 26 Oct 2018, Sasha Levin wrote: > On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote: > > This change was suggested by checkpath.pl. Use unsigned int with bitfield > > allocate only one bit to the boolean variable. > > > > CHECK: Avoid using bool structure members because of possible alignment > > issues > > > > Signed-off-by: Shayenne da Luz Moura > > --- > > drivers/staging/vboxvideo/vbox_drv.h| 14 +++--- > > drivers/staging/vboxvideo/vboxvideo_guest.h | 2 +- > > 2 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h > > b/drivers/staging/vboxvideo/vbox_drv.h > > index 594f84272957..7d3e329a6b1c 100644 > > --- a/drivers/staging/vboxvideo/vbox_drv.h > > +++ b/drivers/staging/vboxvideo/vbox_drv.h > > @@ -81,7 +81,7 @@ struct vbox_private { > > u8 __iomem *vbva_buffers; > > struct gen_pool *guest_pool; > > struct vbva_buf_ctx *vbva_info; > > - bool any_pitch; > > + unsigned int any_pitch:1; > > u32 num_crtcs; > > /** Amount of available VRAM, including space used for buffers. */ > > u32 full_vram_size; > > Using bitfields for booleans in these cases is less efficient than just > using "regular" booleans for two reasons: > > 1. It will use the same amount of space. Due to alignment requirements, > the compiler can't squeeze in anything into the 7 bits that are now > "free". Each member, unless it's another bitfield, must start at a whole > byte. > > 2. This is actually less efficient (slower) for the compiler to work > with. The smallest granularity we have to access memory is 1 byte; we > can't set individual bits directly in memory. For the original code, the > assembly for 'vbox_private.any_pitch = true' would look something like > this: > > movl $0x1,-0x10(%rsp) > > As you can see, the compiler can directly write into the variable. > However, when we switch to using bitfields, the compiler must preserve > the original value of the other 7 bits, so it must first read them from > memory, manipulate the value and write it back. The assembly would > look something like this: > > movzbl -0x10(%rsp),%eax > or $0x1,%eax > mov%al,-0x10(%rsp) > > Which is less efficient than what was previously happening. Maybe checkpatch could be more precise about what kind of bools should be changed? julia ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] stagign: wilc1000: validate cfg parameters before scheduling the work
On 10/26/18 2:36 AM, Dan Carpenter wrote: > On Thu, Oct 25, 2018 at 09:11:00PM +, adham.aboza...@microchip.com wrote: > > You'll need to resend these because your email from header is in the > wrong format. Also there is a typo in the subject. s/stagign/staging/. > Apologies Dan for that! Will correct the subject and email from format and send v2. Regards, Adham ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Changes in v2: - Added my S-o-B in patch 5. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 55 ++- 1 file changed, 37 insertions(+), 18 deletions(-) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not handling its possible failure. This patch makes ad2s90_probe check if the code is an error code and, if so, do the following: - Call dev_err with an appropriate error message. - Return the spi_setup's error code, aborting the execution of the rest of the function. Signed-off-by: Matheus Tavares --- drivers/staging/iio/resolver/ad2s90.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index 11fac9f90148..d6a42e3f1bd8 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi) /* need 600ns between CS and the first falling edge of SCLK */ spi->max_speed_hz = 83; spi->mode = SPI_MODE_3; - spi_setup(spi); + ret = spi_setup(spi); + + if (ret < 0) { + dev_err(&spi->dev, "spi_setup failed!\n"); + return ret; + } return 0; } -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw, the code was ignored and IIO_VAL_INT was returned. This patch makes the function return the error code returned by spi_read when it fails. Signed-off-by: Matheus Tavares --- drivers/staging/iio/resolver/ad2s90.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index 59586947a936..11fac9f90148 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev, struct ad2s90_state *st = iio_priv(indio_dev); mutex_lock(&st->lock); + ret = spi_read(st->sdev, st->rx, 2); - if (ret) - goto error_ret; + if (ret < 0) { + mutex_unlock(&st->lock); + return ret; + } + *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4); -error_ret: mutex_unlock(&st->lock); return IIO_VAL_INT; -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the spi_setup call and the spi_device's max_speed_hz and mode assignments. This could lead to a race condition since the driver was still being set up after it was already made ready to use. To fix it, this patch moves the device registration to the end of ad2s90_probe. Signed-off-by: Matheus Tavares --- drivers/staging/iio/resolver/ad2s90.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index c20d37dc065a..b4a6a89c11b0 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -81,10 +81,6 @@ static int ad2s90_probe(struct spi_device *spi) indio_dev->num_channels = 1; indio_dev->name = spi_get_device_id(spi)->name; - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev); - if (ret) - return ret; - /* need 600ns between CS and the first falling edge of SCLK */ spi->max_speed_hz = 83; spi->mode = SPI_MODE_3; @@ -95,7 +91,7 @@ static int ad2s90_probe(struct spi_device *spi) return ret; } - return 0; + return devm_iio_device_register(indio_dev->dev.parent, indio_dev); } static const struct spi_device_id ad2s90_id[] = { -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and implements the relative read behavior at ad2s90_read_raw. Signed-off-by: Victor Colombo Signed-off-by: Matheus Tavares --- drivers/staging/iio/resolver/ad2s90.c | 32 ++- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index b4a6a89c11b0..52b656875ed1 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev, int ret; struct ad2s90_state *st = iio_priv(indio_dev); - mutex_lock(&st->lock); + switch (m) { + case IIO_CHAN_INFO_SCALE: + /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */ + *val = 0; + *val2 = 1534355; + return IIO_VAL_INT_PLUS_NANO; + case IIO_CHAN_INFO_RAW: + mutex_lock(&st->lock); + + ret = spi_read(st->sdev, st->rx, 2); + if (ret < 0) { + mutex_unlock(&st->lock); + return ret; + } + + *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4); - ret = spi_read(st->sdev, st->rx, 2); - if (ret < 0) { mutex_unlock(&st->lock); - return ret; - } - *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4); - - mutex_unlock(&st->lock); + return IIO_VAL_INT; + default: + break; + } - return IIO_VAL_INT; + return -EINVAL; } static const struct iio_info ad2s90_info = { @@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = { .type = IIO_ANGL, .indexed = 1, .channel = 0, - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), }; static int ad2s90_probe(struct spi_device *spi) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe, that was always overwritten. Signed-off-by: Matheus Tavares --- drivers/staging/iio/resolver/ad2s90.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index d6a42e3f1bd8..c20d37dc065a 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -64,7 +64,7 @@ static int ad2s90_probe(struct spi_device *spi) { struct iio_dev *indio_dev; struct ad2s90_state *st; - int ret = 0; + int ret; indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); if (!indio_dev) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the ad2s90_read_raw function. Since ad2s90 has only one channel, it just checks if the given channel is the expected one and if not, return -EINVAL. Signed-off-by: Matheus Tavares --- drivers/staging/iio/resolver/ad2s90.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index 52b656875ed1..24002042a5c5 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev, int ret; struct ad2s90_state *st = iio_priv(indio_dev); + if (chan->type != IIO_ANGL) + return -EINVAL; + switch (m) { case IIO_CHAN_INFO_SCALE: /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */ -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel