Re: [PATCH RFC 04/11] staging: vchiq_arm: Fix platform device unregistration

2018-10-26 Thread Dan Carpenter
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

2018-10-26 Thread Dan Carpenter
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

2018-10-26 Thread Dan Carpenter
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

2018-10-26 Thread 周威
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

2018-10-26 Thread Dan Carpenter
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

2018-10-26 Thread Dan Carpenter
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

2018-10-26 Thread Hans de Goede

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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Hans de Goede

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

2018-10-26 Thread Nishad Kamdar
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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()

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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

2018-10-26 Thread Nicolas Saenz Julienne
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.

2018-10-26 Thread Nishad Kamdar
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.

2018-10-26 Thread Nishad Kamdar
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

2018-10-26 Thread Sergio Paracuellos
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

2018-10-26 Thread Todd Kjos
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

2018-10-26 Thread Dave Stevenson
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

2018-10-26 Thread Matheus Tavares Bernardino
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

2018-10-26 Thread Shayenne da Luz Moura
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

2018-10-26 Thread Shayenne da Luz Moura
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

2018-10-26 Thread Shayenne da Luz Moura
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

2018-10-26 Thread Sasha Levin

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

2018-10-26 Thread Julia Lawall
[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

2018-10-26 Thread Adham.Abozaeid
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

2018-10-26 Thread Matheus Tavares
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

2018-10-26 Thread Matheus Tavares
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

2018-10-26 Thread Matheus Tavares
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

2018-10-26 Thread Matheus Tavares
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

2018-10-26 Thread Matheus Tavares
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

2018-10-26 Thread Matheus Tavares
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

2018-10-26 Thread Matheus Tavares
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