Re: [Linaro-mm-sig] [RFC][PATCH 0/6] ion: improved ABI

2016-06-07 Thread Chen Feng
The idea is good, define the heap ids in header file is inconvenient.

But if we query the heaps information from user-space.

It need to maintain this ids and name userspace one by one. The code may
be complicated in different module user-space.

In android, the gralloc and other lib will all use ion to alloc memory.

This will make it more difficult to maintain user-space code.


But beyond this, The new alloc2 with not-handle flag is good.
And the pull out of ioctl interface is also a good cleanup.

On 2016/6/7 2:23, Laura Abbott wrote:
> 
> The ABI for Ion's ioctl interface are a pain to work with. The heap IDs
> are a 32-bit non-discoverable namespace that form part of the ABI. There's
> no way to determine what ABI version is in use which leads to problems
> if the ABI changes or needs to be updated.
> 
> This series is a first approach to give a better ABI for Ion. This includes:
> 
> - Following the advice in botching-up-ioctls.txt
> - Ioctl for ABI version
> - Dynamic assignment of heap ids
> - queryable heap ids
> - Runtime mapping of heap ids, including fallbacks. This avoids the need to
>   encode the fallbacks as an ABI.
> 
> I'm most interested in feedback if this ABI is actually an improvement and
> usable. The heap id map/query interface seems error prone but I didn't have
> a cleaner solution. There aren't any kernel APIs for the new features as the
> focus was on a userspace API but I anticipate that following easily once
> the userspace API is established.
> 
> 
> Thanks,
> Laura
> 
> P.S. Not to turn this into a bike shedding session but if you have suggestions
> for a name for this framework other than Ion I would be interested to hear
> them. Too many other things are already named Ion.
> 
> Laura Abbott (6):
>   staging: android: ion: return error value for ion_device_add_heap
>   staging: android: ion: Switch to using an idr to manage heaps
>   staging: android: ion: Drop heap type masks
>   staging: android: ion: Pull out ion ioctls to a separate file
>   staging: android: ion: Add an ioctl for ABI checking
>   staging: android: ion: Introduce new ioctls for dynamic heaps
> 
>  drivers/staging/android/ion/Makefile|   3 +-
>  drivers/staging/android/ion/ion-ioctl.c | 243 ++
>  drivers/staging/android/ion/ion.c   | 438 
> 
>  drivers/staging/android/ion/ion_priv.h  | 109 +++-
>  drivers/staging/android/uapi/ion.h  | 164 +++-
>  5 files changed, 728 insertions(+), 229 deletions(-)
>  create mode 100644 drivers/staging/android/ion/ion-ioctl.c
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/4] Staging: comedi: Block comment issue fixed for das16.c

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This is a patch to the das16.c file that fixes up a
WARNING: 'Block comments use a trailing */ on a separate line'
found by the checkpatch.pl tool.

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das16.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index 4d6e581..ef345dc 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -429,8 +429,10 @@ static const struct das16_board das16_boards[] = {
},
 };
 
-/* Period for timer interrupt in jiffies.  It's a function
- * to deal with possibility of dynamic HZ patches  */
+/*
+ * Period for timer interrupt in jiffies.  It's a function
+ * to deal with possibility of dynamic HZ patches
+ */
 static inline int timer_period(void)
 {
return HZ / 20;
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 4/4] Staging: comedi: Prefer unsigned int instead of unsigned in das800.c

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This is a patch to the das800.c file that fixes up a
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
found by the checkpatch.pl tool.

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das800.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das800.c 
b/drivers/staging/comedi/drivers/das800.c
index 1f1b7ad..fd4cb49 100644
--- a/drivers/staging/comedi/drivers/das800.c
+++ b/drivers/staging/comedi/drivers/das800.c
@@ -218,7 +218,7 @@ struct das800_private {
 };
 
 static void das800_ind_write(struct comedi_device *dev,
-unsigned val, unsigned reg)
+unsigned int val, unsigned int reg)
 {
/*
 * Select dev->iobase + 2 to be desired register
@@ -228,7 +228,7 @@ static void das800_ind_write(struct comedi_device *dev,
outb(val, dev->iobase + 2);
 }
 
-static unsigned das800_ind_read(struct comedi_device *dev, unsigned reg)
+static unsigned int das800_ind_read(struct comedi_device *dev, unsigned int 
reg)
 {
/*
 * Select dev->iobase + 7 to be desired register
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/4] Staging: comedi: fix blank line issue in das16.c

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This is a patch to the das16.c file that fixes up a blank line after
function/struct/union/enum  check found by the checkpatch.pl tool

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das16.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index fd8e0b7..4d6e581 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -198,6 +198,7 @@ enum {
das16_pg_1601,
das16_pg_1602,
 };
+
 static const int *const das16_gainlists[] = {
NULL,
das16jr_gainlist,
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 3/4] Staging: comedi: fix comment issue fixed for das800.c

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This is a patch to the das800.c file that fixes up a
WARNING: 'Block comments use a trailing */ on a separate line'
found by the checkpatch.pl tool

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das800.c | 102 
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das800.c 
b/drivers/staging/comedi/drivers/das800.c
index b02f122..1f1b7ad 100644
--- a/drivers/staging/comedi/drivers/das800.c
+++ b/drivers/staging/comedi/drivers/das800.c
@@ -1,56 +1,56 @@
 /*
-comedi/drivers/das800.c
-Driver for Keitley das800 series boards and compatibles
-Copyright (C) 2000 Frank Mori Hess 
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 2000 David A. Schleef 
-
-This program is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 2 of the License, or
-(at your option) any later version.
-
-This program is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-*/
+ * comedi/drivers/das800.c
+ * Driver for Keitley das800 series boards and compatibles
+ * Copyright (C) 2000 Frank Mori Hess 
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 2000 David A. Schleef 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 /*
-Driver: das800
-Description: Keithley Metrabyte DAS800 (& compatibles)
-Author: Frank Mori Hess 
-Devices: [Keithley Metrabyte] DAS-800 (das-800), DAS-801 (das-801),
-  DAS-802 (das-802),
-  [Measurement Computing] CIO-DAS800 (cio-das800),
-  CIO-DAS801 (cio-das801), CIO-DAS802 (cio-das802),
-  CIO-DAS802/16 (cio-das802/16)
-Status: works, cio-das802/16 untested - email me if you have tested it
-
-Configuration options:
-  [0] - I/O port base address
-  [1] - IRQ (optional, required for timed or externally triggered conversions)
-
-Notes:
-   IRQ can be omitted, although the cmd interface will not work without it.
-
-   All entries in the channel/gain list must use the same gain and be
-   consecutive channels counting upwards in channel number (these are
-   hardware limitations.)
-
-   I've never tested the gain setting stuff since I only have a
-   DAS-800 board with fixed gain.
-
-   The cio-das802/16 does not have a fifo-empty status bit!  Therefore
-   only fifo-half-full transfers are possible with this card.
-
-cmd triggers supported:
-   start_src:  TRIG_NOW | TRIG_EXT
-   scan_begin_src: TRIG_FOLLOW
-   scan_end_src:   TRIG_COUNT
-   convert_src:TRIG_TIMER | TRIG_EXT
-   stop_src:   TRIG_NONE | TRIG_COUNT
-*/
+ * Driver: das800
+ * Description: Keithley Metrabyte DAS800 (& compatibles)
+ * Author: Frank Mori Hess 
+ * Devices: [Keithley Metrabyte] DAS-800 (das-800), DAS-801 (das-801),
+ * DAS-802 (das-802),
+ * [Measurement Computing] CIO-DAS800 (cio-das800),
+ * CIO-DAS801 (cio-das801), CIO-DAS802 (cio-das802),
+ * CIO-DAS802/16 (cio-das802/16)
+ * Status: works, cio-das802/16 untested - email me if you have tested it
+ *
+ * Configuration options:
+ * [0] - I/O port base address
+ * [1] - IRQ (optional, required for timed or externally triggered conversions)
+ *
+ * Notes:
+ * IRQ can be omitted, although the cmd interface will not work without it.
+ *
+ * All entries in the channel/gain list must use the same gain and be
+ * consecutive channels counting upwards in channel number (these are
+ * hardware limitations.)
+ *
+ * I've never tested the gain setting stuff since I only have a
+ * DAS-800 board with fixed gain.
+ *
+ * The cio-das802/16 does not have a fifo-empty status bit!  Therefore
+ * only fifo-half-full transfers are possible with this card.
+ *
+ * cmd triggers supported:
+ * start_src:  TRIG_NOW | TRIG_EXT
+ * scan_begin_src: TRIG_FOLLOW
+ * scan_end_src:   TRIG_COUNT
+ * convert_src:TRIG_TIMER | TRIG_EXT
+ * stop_src:   TRIG_NONE | TRIG_COUNT
+ */
 
 #include 
 #include 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 5/5] Staging: comedi: Prefer using the BIT macro issue in dmm32at.c

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This patch Replace all occurences of (1<
---
 drivers/staging/comedi/drivers/dmm32at.c | 86 
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dmm32at.c 
b/drivers/staging/comedi/drivers/dmm32at.c
index 958c0d4..3c4722a 100644
--- a/drivers/staging/comedi/drivers/dmm32at.c
+++ b/drivers/staging/comedi/drivers/dmm32at.c
@@ -46,73 +46,73 @@
 #define DMM32AT_AI_START_CONV_REG  0x00
 #define DMM32AT_AI_LSB_REG 0x00
 #define DMM32AT_AUX_DOUT_REG   0x01
-#define DMM32AT_AUX_DOUT2  (1 << 2)  /* J3.42 - OUT2 (OUT2EN) */
-#define DMM32AT_AUX_DOUT1  (1 << 1)  /* J3.43 */
-#define DMM32AT_AUX_DOUT0  (1 << 0)  /* J3.44 - OUT0 (OUT0EN) */
+#define DMM32AT_AUX_DOUT2  BIT(2)  /* J3.42 - OUT2 (OUT2EN) */
+#define DMM32AT_AUX_DOUT1  BIT(1)  /* J3.43 */
+#define DMM32AT_AUX_DOUT0  BIT(0)  /* J3.44 - OUT0 (OUT0EN) */
 #define DMM32AT_AI_MSB_REG 0x01
 #define DMM32AT_AI_LO_CHAN_REG 0x02
 #define DMM32AT_AI_HI_CHAN_REG 0x03
 #define DMM32AT_AUX_DI_REG 0x04
-#define DMM32AT_AUX_DI_DACBUSY (1 << 7)
-#define DMM32AT_AUX_DI_CALBUSY (1 << 6)
-#define DMM32AT_AUX_DI3(1 << 3)  /* J3.45 - ADCLK 
(CLKSEL) */
-#define DMM32AT_AUX_DI2(1 << 2)  /* J3.46 - GATE12 
(GT12EN) */
-#define DMM32AT_AUX_DI1(1 << 1)  /* J3.47 - GATE0 
(GT0EN) */
-#define DMM32AT_AUX_DI0(1 << 0)  /* J3.48 - CLK0 
(SRC0) */
+#define DMM32AT_AUX_DI_DACBUSY BIT(7)
+#define DMM32AT_AUX_DI_CALBUSY BIT(6)
+#define DMM32AT_AUX_DI3BIT(3)  /* J3.45 - ADCLK 
(CLKSEL) */
+#define DMM32AT_AUX_DI2BTI(2)  /* J3.46 - GATE12 
(GT12EN) */
+#define DMM32AT_AUX_DI1BIT(1)  /* J3.47 - GATE0 
(GT0EN) */
+#define DMM32AT_AUX_DI0BIT(0)  /* J3.48 - CLK0 (SRC0) 
*/
 #define DMM32AT_AO_LSB_REG 0x04
 #define DMM32AT_AO_MSB_REG 0x05
 #define DMM32AT_AO_MSB_DACH(x) ((x) << 6)
 #define DMM32AT_FIFO_DEPTH_REG 0x06
 #define DMM32AT_FIFO_CTRL_REG  0x07
-#define DMM32AT_FIFO_CTRL_FIFOEN   (1 << 3)
-#define DMM32AT_FIFO_CTRL_SCANEN   (1 << 2)
-#define DMM32AT_FIFO_CTRL_FIFORST  (1 << 1)
+#define DMM32AT_FIFO_CTRL_FIFOEN   BIT(3)
+#define DMM32AT_FIFO_CTRL_SCANEN   BIT(2)
+#define DMM32AT_FIFO_CTRL_FIFORST  BIT(1)
 #define DMM32AT_FIFO_STATUS_REG0x07
-#define DMM32AT_FIFO_STATUS_EF (1 << 7)
-#define DMM32AT_FIFO_STATUS_HF (1 << 6)
-#define DMM32AT_FIFO_STATUS_FF (1 << 5)
-#define DMM32AT_FIFO_STATUS_OVF(1 << 4)
-#define DMM32AT_FIFO_STATUS_FIFOEN (1 << 3)
-#define DMM32AT_FIFO_STATUS_SCANEN (1 << 2)
+#define DMM32AT_FIFO_STATUS_EF BIT(7)
+#define DMM32AT_FIFO_STATUS_HF BIT(6)
+#define DMM32AT_FIFO_STATUS_FF BIT(5)
+#define DMM32AT_FIFO_STATUS_OVFBIT(4)
+#define DMM32AT_FIFO_STATUS_FIFOEN BIT(3)
+#define DMM32AT_FIFO_STATUS_SCANEN BIT(2)
 #define DMM32AT_FIFO_STATUS_PAGE_MASK  (3 << 0)
 #define DMM32AT_CTRL_REG   0x08
-#define DMM32AT_CTRL_RESETA(1 << 5)
-#define DMM32AT_CTRL_RESETD(1 << 4)
-#define DMM32AT_CTRL_INTRST(1 << 3)
+#define DMM32AT_CTRL_RESETABIT(5)
+#define DMM32AT_CTRL_RESETDBIT(4)
+#define DMM32AT_CTRL_INTRSTBIT(3)
 #define DMM32AT_CTRL_PAGE_8254 (0 << 0)
-#define DMM32AT_CTRL_PAGE_8255 (1 << 0)
+#define DMM32AT_CTRL_PAGE_8255 BIT(0)
 #define DMM32AT_CTRL_PAGE_CALIB(3 << 0)
 #define DMM32AT_AI_STATUS_REG  0x08
-#define DMM32AT_AI_STATUS_STS  (1 << 7)
-#define DMM32AT_AI_STATUS_SD1  (1 << 6)
-#define DMM32AT_AI_STATUS_SD0  (1 << 5)
+#define DMM32AT_AI_STATUS_STS  BIT(7)
+#define DMM32AT_AI_STATUS_SD1  BIT(6)
+#define DMM32AT_AI_STATUS_SD0  BIT(5)
 #define DMM32AT_AI_STATUS_ADCH_MASK(0x1f << 0)
 #define DMM32AT_INTCLK_REG 0x09
-#define DMM32AT_INTCLK_ADINT   (1 << 7)
-#define DMM32AT_INTCLK_DINT(1 << 6)
-#define DMM32AT_INTCLK_TINT(1 << 5)
-#define DMM32AT_INTCLK_CLKEN   (1 << 1)  /* 1=see below  0=software */
-#define DMM32AT_INTCLK_CLKSEL  (1 << 0)  /* 1=OUT2  0=EXTCLK */
+#define DMM32AT_INTCLK_ADINT   BIT(7)
+#define DMM32AT_INTCLK_DINTBIT(6)
+#define DMM32AT_INTCLK_TINTBIT(5)
+#define DMM32AT_INTCLK_CLKEN   BIT(1)  /* 1=see below  0=software */
+#define DMM32AT_INTCLK_CLKSEL  BIT(0)  /* 1=OUT2  0=EXTCLK */
 #define DMM32AT_CTRDIO_CFG_REG 0x0a
-#define DMM32AT_CTRDIO_CFG_FREQ12  (1 << 7)  /* CLK12 1=100KHz 0=10MHz */
-#define DMM32AT_CTRDIO_CFG_FREQ0   (1 << 6)  /* CLK0  1=

Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters

2016-06-07 Thread Neil Horman
On Sat, Jun 04, 2016 at 01:27:04PM -0400, David Kershner wrote:
> From: David Binder 
> 
> Removes unused module parameters from visorbus_main.c, in response to
> findings by SonarQube.
> 
> Signed-off-by: David Binder 
> Signed-off-by: David Kershner 
> Reviewed-by: Tim Sell 
> ---
>  drivers/staging/unisys/visorbus/visorbus_main.c | 9 +
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> index 2ed9628..71bff07 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -27,10 +27,9 @@
>  #define MYDRVNAME "visorbus"
>  
>  /* module parameters */
> -static int visorbus_debug;
>  static int visorbus_forcematch;
>  static int visorbus_forcenomatch;
> -static int visorbus_debugref;
> +
>  #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024)
>  
>  /* Display string that is guaranteed to be no longer the 99 characters*/
> @@ -1332,9 +1331,6 @@ visorbus_exit(void)
>   remove_bus_type();
>  }
>  
> -module_param_named(debug, visorbus_debug, int, S_IRUGO);
> -MODULE_PARM_DESC(visorbus_debug, "1 to debug");
> -
>  module_param_named(forcematch, visorbus_forcematch, int, S_IRUGO);
>  MODULE_PARM_DESC(visorbus_forcematch,
>"1 to force a successful dev <--> drv match");
> @@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch,
>  module_param_named(forcenomatch, visorbus_forcenomatch, int, S_IRUGO);
>  MODULE_PARM_DESC(visorbus_forcenomatch,
>"1 to force an UNsuccessful dev <--> drv match");
> -
> -module_param_named(debugref, visorbus_debugref, int, S_IRUGO);
> -MODULE_PARM_DESC(visorbus_debugref, "1 to debug reference counting");

visorbus_forcematch and visorbus_forcenomatch appear to be referenced in
visorbus_match (at least in the HEAD of linus' tree).  If you're going to remove
the module parameters, why not also remove those references and the
force[no]match variables themselves?

Neil

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 11/30] staging: unisys: visorbus: use kernel timer instead of workqueue

2016-06-07 Thread Neil Horman
On Sat, Jun 04, 2016 at 01:27:11PM -0400, David Kershner wrote:
> From: Tim Sell 
> 
> A kernel timer is now used as the vehicle to periodically call the
> channel_interrupt function of registered visor drivers, instead of a
> workqueue.
> 
> This simplifies a lot of things by making periodic_work.c and
> periodic_work.h no longer necessary.  This change also means that the
> channel_interrupt() callbacks registered by visor drivers (via
> visorbus_register_visor_driver()) will now be called in atomic context
> (i.e., canNOT sleep) rather than kernel thread context (CAN sleep).
> Fortunately this did NOT necessitate any change to the existing
> channel_interrupt() callbacks, because none of them ever perform any
> operations that would be invalid in atomic context.
> 
> Signed-off-by: Tim Sell 
> Signed-off-by: David Kershner 
> ---
>  drivers/staging/unisys/include/visorbus.h   | 10 +++--
>  drivers/staging/unisys/visorbus/visorbus_main.c | 54 
> +++--
>  2 files changed, 21 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/staging/unisys/include/visorbus.h 
> b/drivers/staging/unisys/include/visorbus.h
> index 9baf1ec..9bb88bb 100644
> --- a/drivers/staging/unisys/include/visorbus.h
> +++ b/drivers/staging/unisys/include/visorbus.h
> @@ -34,8 +34,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
> -#include "periodic_work.h"
>  #include "channel.h"
>  
>  struct visor_driver;
> @@ -126,8 +127,8 @@ struct visor_driver {
>   * device:   Device struct meant for use by the bus driver
>   *   only.
>   * list_all: Used by the bus driver to enumerate devices.
> - * periodic_work:Device work queue. Private use by bus driver
> - *   only.
> + * timer:Timer fired periodically to do interrupt-type
> + *   activity.
>   * being_removed:Indicates that the device is being removed from
>   *   the bus. Private bus driver use only.
>   * visordriver_callback_lock:Used by the bus driver to lock when 
> handling
> @@ -157,7 +158,8 @@ struct visor_device {
>   /* These fields are for private use by the bus driver only. */
>   struct device device;
>   struct list_head list_all;
> - struct periodic_work *periodic_work;
> + struct timer_list timer;
> + bool timer_active;
>   bool being_removed;
>   struct semaphore visordriver_callback_lock;
>   bool pausing;
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> index ebdd5de..e98e720 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -19,7 +19,6 @@
>  #include "visorbus.h"
>  #include "visorbus_private.h"
>  #include "version.h"
> -#include "periodic_work.h"
>  #include "vbuschannel.h"
>  #include "guestlinuxdebug.h"
>  #include "vmcallinterface.h"
> @@ -116,7 +115,6 @@ struct bus_type visorbus_type = {
>   .bus_groups = visorbus_bus_groups,
>  };
>  
> -static struct workqueue_struct *periodic_dev_workqueue;
>  static long long bus_count;  /** number of bus instances */
>   /** ever-increasing */
>  
> @@ -222,10 +220,6 @@ visorbus_release_device(struct device *xdev)
>  {
>   struct visor_device *dev = to_visor_device(xdev);
>  
> - if (dev->periodic_work) {
> - visor_periodic_work_destroy(dev->periodic_work);
> - dev->periodic_work = NULL;
> - }
>   if (dev->visorchannel) {
>   visorchannel_destroy(dev->visorchannel);
>   dev->visorchannel = NULL;
> @@ -530,35 +524,36 @@ unregister_driver_attributes(struct visor_driver *drv)
>  }
>  
>  static void
> -dev_periodic_work(void *xdev)
> +dev_periodic_work(unsigned long __opaque)
>  {
> - struct visor_device *dev = xdev;
> + struct visor_device *dev = (struct visor_device *)__opaque;
>   struct visor_driver *drv = to_visor_driver(dev->device.driver);
>  
> - down(&dev->visordriver_callback_lock);
>   if (drv->channel_interrupt)
>   drv->channel_interrupt(dev);
visorinput_channel_interrupt uses down_write (and other channel_interrupt
methods may do the same).  While its legal to use semaphores in interrupt
context, you need to convert that/those calls to the trylock variants and handle
a failed lock to avoid blocking when in interrupt context, which moving to a
timer handler puts you in.

> - up(&dev->visordriver_callback_lock);
> - if (!visor_periodic_work_nextperiod(dev->periodic_work))
> - put_device(&dev->device);
> + mod_timer(&dev->timer, jiffies + POLLJIFFIES_NORMALCHANNEL);
>  }
>  
>  static void
>  dev_start_periodic_work(struct visor_device *dev)
>  {
> - if (dev->being_removed)
> + if (dev->being_removed || dev->timer_active)
>   return;

Re: [PATCH v3 11/30] staging: unisys: visorbus: use kernel timer instead of workqueue

2016-06-07 Thread Neil Horman
On Tue, Jun 07, 2016 at 01:47:13PM +, Sell, Timothy C wrote:
> > -Original Message-
> > From: Neil Horman [mailto:nhor...@redhat.com]
> > Sent: Tuesday, June 07, 2016 9:40 AM
> > To: Kershner, David A
> > Cc: cor...@lwn.net; t...@linutronix.de; mi...@redhat.com;
> > h...@zytor.com; gre...@linuxfoundation.org; Arfvidson, Erik; Sell, Timothy
> > C; hof...@osadl.org; dzic...@redhat.com; jes.soren...@redhat.com;
> > Curtin, Alexander Paul; janani.rvchn...@gmail.com;
> > sudipm.mukher...@gmail.com; pra...@redhat.com; Binder, David Anthony;
> > dan.j.willi...@intel.com; linux-ker...@vger.kernel.org; linux-
> > d...@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> > Maintainer
> > Subject: Re: [PATCH v3 11/30] staging: unisys: visorbus: use kernel timer
> > instead of workqueue
> > 
> > On Sat, Jun 04, 2016 at 01:27:11PM -0400, David Kershner wrote:
> > > From: Tim Sell 
> > >
> > > A kernel timer is now used as the vehicle to periodically call the
> > > channel_interrupt function of registered visor drivers, instead of a
> > > workqueue.
> > >
> > > This simplifies a lot of things by making periodic_work.c and
> > > periodic_work.h no longer necessary.  This change also means that the
> > > channel_interrupt() callbacks registered by visor drivers (via
> > > visorbus_register_visor_driver()) will now be called in atomic context
> > > (i.e., canNOT sleep) rather than kernel thread context (CAN sleep).
> > > Fortunately this did NOT necessitate any change to the existing
> > > channel_interrupt() callbacks, because none of them ever perform any
> > > operations that would be invalid in atomic context.
> > >
> > > Signed-off-by: Tim Sell 
> > > Signed-off-by: David Kershner 
> > > ---
> > >  drivers/staging/unisys/include/visorbus.h   | 10 +++--
> > >  drivers/staging/unisys/visorbus/visorbus_main.c | 54 
> > > +++---
> > ---
> > >  2 files changed, 21 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/drivers/staging/unisys/include/visorbus.h
> > b/drivers/staging/unisys/include/visorbus.h
> > > index 9baf1ec..9bb88bb 100644
> > > --- a/drivers/staging/unisys/include/visorbus.h
> > > +++ b/drivers/staging/unisys/include/visorbus.h
> > > @@ -34,8 +34,9 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > > +#include 
> > >
> > > -#include "periodic_work.h"
> > >  #include "channel.h"
> > >
> > >  struct visor_driver;
> > > @@ -126,8 +127,8 @@ struct visor_driver {
> > >   * device:   Device struct meant for use by the bus 
> > > driver
> > >   *   only.
> > >   * list_all: Used by the bus driver to enumerate
> > devices.
> > > - * periodic_work:Device work queue. Private use by bus 
> > > driver
> > > - *   only.
> > > + * timer:Timer fired periodically to do 
> > > interrupt-type
> > > + *   activity.
> > >   * being_removed:Indicates that the device is being 
> > > removed
> > from
> > >   *   the bus. Private bus driver use only.
> > >   * visordriver_callback_lock:Used by the bus driver to lock when 
> > > handling
> > > @@ -157,7 +158,8 @@ struct visor_device {
> > >   /* These fields are for private use by the bus driver only. */
> > >   struct device device;
> > >   struct list_head list_all;
> > > - struct periodic_work *periodic_work;
> > > + struct timer_list timer;
> > > + bool timer_active;
> > >   bool being_removed;
> > >   struct semaphore visordriver_callback_lock;
> > >   bool pausing;
> > > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c
> > b/drivers/staging/unisys/visorbus/visorbus_main.c
> > > index ebdd5de..e98e720 100644
> > > --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> > > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> > > @@ -19,7 +19,6 @@
> > >  #include "visorbus.h"
> > >  #include "visorbus_private.h"
> > >  #include "version.h"
> > > -#include "periodic_work.h"
> > >  #include "vbuschannel.h"
> > >  #include "guestlinuxdebug.h"
> > >  #include "vmcallinterface.h"
> > > @@ -116,7 +115,6 @@ struct bus_type visorbus_type = {
> > >   .bus_groups = visorbus_bus_groups,
> > >  };
> > >
> > > -static struct workqueue_struct *periodic_dev_workqueue;
> > >  static long long bus_count;  /** number of bus instances */
> > >   /** ever-increasing */
> > >
> > > @@ -222,10 +220,6 @@ visorbus_release_device(struct device *xdev)
> > >  {
> > >   struct visor_device *dev = to_visor_device(xdev);
> > >
> > > - if (dev->periodic_work) {
> > > - visor_periodic_work_destroy(dev->periodic_work);
> > > - dev->periodic_work = NULL;
> > > - }
> > >   if (dev->visorchannel) {
> > >   visorchannel_destroy(dev->visorchannel);
> > >   dev->visorchannel = NULL;
> > > @@ -530,35 +524,36 @@ unregister_driver_attributes(struct visor_d

RE: [PATCH v3 11/30] staging: unisys: visorbus: use kernel timer instead of workqueue

2016-06-07 Thread Sell, Timothy C
> -Original Message-
> From: Neil Horman [mailto:nhor...@redhat.com]
> Sent: Tuesday, June 07, 2016 9:40 AM
> To: Kershner, David A
> Cc: cor...@lwn.net; t...@linutronix.de; mi...@redhat.com;
> h...@zytor.com; gre...@linuxfoundation.org; Arfvidson, Erik; Sell, Timothy
> C; hof...@osadl.org; dzic...@redhat.com; jes.soren...@redhat.com;
> Curtin, Alexander Paul; janani.rvchn...@gmail.com;
> sudipm.mukher...@gmail.com; pra...@redhat.com; Binder, David Anthony;
> dan.j.willi...@intel.com; linux-ker...@vger.kernel.org; linux-
> d...@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> Maintainer
> Subject: Re: [PATCH v3 11/30] staging: unisys: visorbus: use kernel timer
> instead of workqueue
> 
> On Sat, Jun 04, 2016 at 01:27:11PM -0400, David Kershner wrote:
> > From: Tim Sell 
> >
> > A kernel timer is now used as the vehicle to periodically call the
> > channel_interrupt function of registered visor drivers, instead of a
> > workqueue.
> >
> > This simplifies a lot of things by making periodic_work.c and
> > periodic_work.h no longer necessary.  This change also means that the
> > channel_interrupt() callbacks registered by visor drivers (via
> > visorbus_register_visor_driver()) will now be called in atomic context
> > (i.e., canNOT sleep) rather than kernel thread context (CAN sleep).
> > Fortunately this did NOT necessitate any change to the existing
> > channel_interrupt() callbacks, because none of them ever perform any
> > operations that would be invalid in atomic context.
> >
> > Signed-off-by: Tim Sell 
> > Signed-off-by: David Kershner 
> > ---
> >  drivers/staging/unisys/include/visorbus.h   | 10 +++--
> >  drivers/staging/unisys/visorbus/visorbus_main.c | 54 +++---
> ---
> >  2 files changed, 21 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/staging/unisys/include/visorbus.h
> b/drivers/staging/unisys/include/visorbus.h
> > index 9baf1ec..9bb88bb 100644
> > --- a/drivers/staging/unisys/include/visorbus.h
> > +++ b/drivers/staging/unisys/include/visorbus.h
> > @@ -34,8 +34,9 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >
> > -#include "periodic_work.h"
> >  #include "channel.h"
> >
> >  struct visor_driver;
> > @@ -126,8 +127,8 @@ struct visor_driver {
> >   * device: Device struct meant for use by the bus driver
> >   * only.
> >   * list_all:   Used by the bus driver to enumerate
> devices.
> > - * periodic_work:  Device work queue. Private use by bus driver
> > - * only.
> > + * timer:  Timer fired periodically to do interrupt-type
> > + * activity.
> >   * being_removed:  Indicates that the device is being removed
> from
> >   * the bus. Private bus driver use only.
> >   * visordriver_callback_lock:  Used by the bus driver to lock when 
> > handling
> > @@ -157,7 +158,8 @@ struct visor_device {
> > /* These fields are for private use by the bus driver only. */
> > struct device device;
> > struct list_head list_all;
> > -   struct periodic_work *periodic_work;
> > +   struct timer_list timer;
> > +   bool timer_active;
> > bool being_removed;
> > struct semaphore visordriver_callback_lock;
> > bool pausing;
> > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> > index ebdd5de..e98e720 100644
> > --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> > @@ -19,7 +19,6 @@
> >  #include "visorbus.h"
> >  #include "visorbus_private.h"
> >  #include "version.h"
> > -#include "periodic_work.h"
> >  #include "vbuschannel.h"
> >  #include "guestlinuxdebug.h"
> >  #include "vmcallinterface.h"
> > @@ -116,7 +115,6 @@ struct bus_type visorbus_type = {
> > .bus_groups = visorbus_bus_groups,
> >  };
> >
> > -static struct workqueue_struct *periodic_dev_workqueue;
> >  static long long bus_count;/** number of bus instances */
> > /** ever-increasing */
> >
> > @@ -222,10 +220,6 @@ visorbus_release_device(struct device *xdev)
> >  {
> > struct visor_device *dev = to_visor_device(xdev);
> >
> > -   if (dev->periodic_work) {
> > -   visor_periodic_work_destroy(dev->periodic_work);
> > -   dev->periodic_work = NULL;
> > -   }
> > if (dev->visorchannel) {
> > visorchannel_destroy(dev->visorchannel);
> > dev->visorchannel = NULL;
> > @@ -530,35 +524,36 @@ unregister_driver_attributes(struct visor_driver
> *drv)
> >  }
> >
> >  static void
> > -dev_periodic_work(void *xdev)
> > +dev_periodic_work(unsigned long __opaque)
> >  {
> > -   struct visor_device *dev = xdev;
> > +   struct visor_device *dev = (struct visor_device *)__opaque;
> > struct visor_driver *drv = to_visor_driver(dev->device.driver);
> >
> > -   down

[PATCH] Drivers: hv: utils: fix a race on userspace daemons registration

2016-06-07 Thread Vitaly Kuznetsov
Background: userspace daemons registration protocol for Hyper-V utilities
drivers has two steps:
1) daemon writes its own version to kernel
2) kernel reads it and replies with module version
at this point we consider the handshake procedure being completed and we
do hv_poll_channel() transitioning the utility device to HVUTIL_READY
state. At this point we're ready to handle messages from kernel.

When hvutil_transport is in HVUTIL_TRANSPORT_CHARDEV mode we have a
single buffer for outgoing message. hvutil_transport_send() puts to this
buffer and till the buffer is cleared with hvt_op_read() returns -EFAULT
to all consequent calls. Host<->guest protocol guarantees there is no more
than one request at a time and we will not get new requests till we reply
to the previous one so this single message buffer is enough.

Now to the race. When we finish negotiation procedure and send kernel
module version to userspace with hvutil_transport_send() it goes into the
above mentioned buffer and if the daemon is slow enough to read it from
there we can get a collision when a request from the host comes, we won't
be able to put anything to the buffer so the request will be lost. To
solve the issue we need to know when the negotiation is really done (when
the version message is read by the daemon) and transition to HVUTIL_READY
state after this happens. Implement a callback on read to support this.
Old style netlink communication is not affected by the change, we don't
really know when these messages are delivered but we don't have a single
message buffer there.

Reported-by: Barry Davis 
Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/hv_fcopy.c   | 14 ++
 drivers/hv/hv_kvp.c | 27 ---
 drivers/hv/hv_snapshot.c| 16 +++-
 drivers/hv/hv_utils_transport.c | 15 ++-
 drivers/hv/hv_utils_transport.h |  4 +++-
 5 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 23c7079..8b2ba98 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -83,6 +83,12 @@ static void fcopy_timeout_func(struct work_struct *dummy)
hv_poll_channel(fcopy_transaction.recv_channel, fcopy_poll_wrapper);
 }
 
+static void fcopy_register_done(void)
+{
+   pr_debug("FCP: userspace daemon registered\n");
+   hv_poll_channel(fcopy_transaction.recv_channel, fcopy_poll_wrapper);
+}
+
 static int fcopy_handle_handshake(u32 version)
 {
u32 our_ver = FCOPY_CURRENT_VERSION;
@@ -94,7 +100,8 @@ static int fcopy_handle_handshake(u32 version)
break;
case FCOPY_VERSION_1:
/* Daemon expects us to reply with our own version */
-   if (hvutil_transport_send(hvt, &our_ver, sizeof(our_ver)))
+   if (hvutil_transport_send(hvt, &our_ver, sizeof(our_ver),
+   fcopy_register_done))
return -EFAULT;
dm_reg_value = version;
break;
@@ -107,8 +114,7 @@ static int fcopy_handle_handshake(u32 version)
 */
return -EINVAL;
}
-   pr_debug("FCP: userspace daemon ver. %d registered\n", version);
-   hv_poll_channel(fcopy_transaction.recv_channel, fcopy_poll_wrapper);
+   pr_debug("FCP: userspace daemon ver. %d connected\n", version);
return 0;
 }
 
@@ -161,7 +167,7 @@ static void fcopy_send_data(struct work_struct *dummy)
}
 
fcopy_transaction.state = HVUTIL_USERSPACE_REQ;
-   rc = hvutil_transport_send(hvt, out_src, out_len);
+   rc = hvutil_transport_send(hvt, out_src, out_len, NULL);
if (rc) {
pr_debug("FCP: failed to communicate to the daemon: %d\n", rc);
if (cancel_delayed_work_sync(&fcopy_timeout_work)) {
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index cb1a916..5e1fdc8 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -102,6 +102,17 @@ static void kvp_poll_wrapper(void *channel)
hv_kvp_onchannelcallback(channel);
 }
 
+static void kvp_register_done(void)
+{
+   /*
+* If we're still negotiating with the host cancel the timeout
+* work to not poll the channel twice.
+*/
+   pr_debug("KVP: userspace daemon registered\n");
+   cancel_delayed_work_sync(&kvp_host_handshake_work);
+   hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper);
+}
+
 static void
 kvp_register(int reg_value)
 {
@@ -116,7 +127,8 @@ kvp_register(int reg_value)
kvp_msg->kvp_hdr.operation = reg_value;
strcpy(version, HV_DRV_VERSION);
 
-   hvutil_transport_send(hvt, kvp_msg, sizeof(*kvp_msg));
+   hvutil_transport_send(hvt, kvp_msg, sizeof(*kvp_msg),
+ kvp_register_done);
kfree(kvp_msg);
}
 }
@@ -158,17 +170,10 @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg)
/*
 *

RE: [PATCH 5/5] Staging: comedi: Prefer using the BIT macro issue in dmm32at.c

2016-06-07 Thread Hartley Sweeten
On Tuesday, June 07, 2016 4:51 AM, Ravishankar Karkala Mallikarjunayya wrote:
> Subject: [PATCH 5/5] Staging: comedi: Prefer using the BIT macro issue in 
> dmm32at.c

Please don't add random patches to a series. You started this series with 1/4, 
2/4,
3/4 and 4/4 then added a 5/5. 

Also, please fix the subject lines to include the driver name. This makes thing
clearer when multiple patches do similar things. The subject lines should look
something like this:

staging: comedi: : 

> This patch Replace all occurences of (1< to get rid of checkpatch.pl "CHECK" output "Prefer using the BIT macro"

There is no need to state that this is a patch. That is already known.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Warnings in comedi_fops.c

2016-06-07 Thread Rithvik Patibandla
**Intro**
The following patch fixes warnings thrown by checkpatch.pl script. The
warnings are: 
"It is preferable to use WRITE_ONCE(, ) instead of
ACCESS_ONCE() = " 

"It is preferable to use READ_ONCE() instead of ACCESS_ONCE()"

Signed-off-by: Rithvik Patibandla 
---
 drivers/staging/comedi/comedi_fops.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 629080f..4d87596 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -312,8 +312,8 @@ static void comedi_file_reset(struct file *file)
}
cfp->last_attached = dev->attached;
cfp->last_detach_count = dev->detach_count;
-   ACCESS_ONCE(cfp->read_subdev) = read_s;
-   ACCESS_ONCE(cfp->write_subdev) = write_s;
+   WRITE_ONCE(cfp->read_subdev, read_s);
+   WRITE_ONCE(cfp->write_subdev, write_s);
 }
 
 static void comedi_file_check(struct file *file)
@@ -331,7 +331,7 @@ static struct comedi_subdevice 
*comedi_file_read_subdevice(struct file *file)
struct comedi_file *cfp = file->private_data;
 
comedi_file_check(file);
-   return ACCESS_ONCE(cfp->read_subdev);
+   return READ_ONCE(cfp->read_subdev);
 }
 
 static struct comedi_subdevice *comedi_file_write_subdevice(struct file *file)
@@ -339,7 +339,7 @@ static struct comedi_subdevice 
*comedi_file_write_subdevice(struct file *file)
struct comedi_file *cfp = file->private_data;
 
comedi_file_check(file);
-   return ACCESS_ONCE(cfp->write_subdev);
+   return READ_ONCE(cfp->write_subdev);
 }
 
 static int resize_async_buffer(struct comedi_device *dev,
@@ -1992,7 +1992,7 @@ static int do_setrsubd_ioctl(struct comedi_device *dev, 
unsigned long arg,
!(s_old->async->cmd.flags & CMDF_WRITE))
return -EBUSY;
 
-   ACCESS_ONCE(cfp->read_subdev) = s_new;
+   WRITE_ONCE(cfp->read_subdev, s_new);
return 0;
 }
 
@@ -2034,7 +2034,7 @@ static int do_setwsubd_ioctl(struct comedi_device *dev, 
unsigned long arg,
(s_old->async->cmd.flags & CMDF_WRITE))
return -EBUSY;
 
-   ACCESS_ONCE(cfp->write_subdev) = s_new;
+   WRITE_ONCE(cfp->write_subdev, s_new);
return 0;
 }
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Warnings in comedi_fops.c

2016-06-07 Thread Greg KH
On Wed, Jun 08, 2016 at 01:30:01AM +0530, Rithvik Patibandla wrote:
> **Intro**

Why this line?

> The following patch fixes warnings thrown by checkpatch.pl script. The
> warnings are: 
> "It is preferable to use WRITE_ONCE(, ) instead of
> ACCESS_ONCE() = " 
> 
> "It is preferable to use READ_ONCE() instead of ACCESS_ONCE()"

And your subject is a bit "odd", look at how other patches are written
and accepted for what you should do instead.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


how to assign issue which require patch to my to do list

2016-06-07 Thread rspawnwolf
Dear sir,
What the best place to assign issues which require patch to my to do list. 
Best
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters

2016-06-07 Thread Binder, David Anthony
> -Original Message-
> From: Neil Horman [mailto:nhor...@redhat.com]
> Sent: Tuesday, June 07, 2016 9:23 AM
> To: Kershner, David A 
> Cc: cor...@lwn.net; t...@linutronix.de; mi...@redhat.com;
> h...@zytor.com; gre...@linuxfoundation.org; Arfvidson, Erik
> ; Sell, Timothy C ;
> hof...@osadl.org; dzic...@redhat.com; jes.soren...@redhat.com; Curtin,
> Alexander Paul ;
> janani.rvchn...@gmail.com; sudipm.mukher...@gmail.com;
> pra...@redhat.com; Binder, David Anthony ;
> dan.j.willi...@intel.com; linux-ker...@vger.kernel.org; linux-
> d...@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> Maintainer 
> Subject: Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused
> module parameters
> 
> On Sat, Jun 04, 2016 at 01:27:04PM -0400, David Kershner wrote:
> > From: David Binder 
> >
> > Removes unused module parameters from visorbus_main.c, in response to
> > findings by SonarQube.
> >
> > Signed-off-by: David Binder 
> > Signed-off-by: David Kershner 
> > Reviewed-by: Tim Sell 
> > ---
> >  drivers/staging/unisys/visorbus/visorbus_main.c | 9 +
> >  1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> > index 2ed9628..71bff07 100644
> > --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> > @@ -27,10 +27,9 @@
> >  #define MYDRVNAME "visorbus"
> >
> >  /* module parameters */
> > -static int visorbus_debug;
> >  static int visorbus_forcematch;
> >  static int visorbus_forcenomatch;
> > -static int visorbus_debugref;
> > +
> >  #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024)
> >
> >  /* Display string that is guaranteed to be no longer the 99 characters*/
> > @@ -1332,9 +1331,6 @@ visorbus_exit(void)
> > remove_bus_type();
> >  }
> >
> > -module_param_named(debug, visorbus_debug, int, S_IRUGO);
> > -MODULE_PARM_DESC(visorbus_debug, "1 to debug");
> > -
> >  module_param_named(forcematch, visorbus_forcematch, int, S_IRUGO);
> >  MODULE_PARM_DESC(visorbus_forcematch,
> >  "1 to force a successful dev <--> drv match");
> > @@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch,
> >  module_param_named(forcenomatch, visorbus_forcenomatch, int,
> S_IRUGO);
> >  MODULE_PARM_DESC(visorbus_forcenomatch,
> >  "1 to force an UNsuccessful dev <--> drv match");
> > -
> > -module_param_named(debugref, visorbus_debugref, int, S_IRUGO);
> > -MODULE_PARM_DESC(visorbus_debugref, "1 to debug reference
> counting");
> 
> visorbus_forcematch and visorbus_forcenomatch appear to be referenced in
> visorbus_match (at least in the HEAD of linus' tree).  If you're going to
> remove
> the module parameters, why not also remove those references and the
> force[no]match variables themselves?
> 
> Neil

We presently use visorbus_forcematch and visorbus_forcenomatch for
debugging purposes, and therefore decided to keep the variables in
the driver.

David Binder
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/2] staging: comedi: das16: fixed coding style issue

2016-06-07 Thread Akshay Shipurkar
Fixed a multi-line comment coding style issue.

Signed-off-by: Akshay Shipurkar 
---
 drivers/staging/comedi/drivers/das16.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index fd8e0b7..9bf1f29 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -429,7 +429,8 @@ static const struct das16_board das16_boards[] = {
 };
 
 /* Period for timer interrupt in jiffies.  It's a function
- * to deal with possibility of dynamic HZ patches  */
+ * to deal with possibility of dynamic HZ patches
+ */
 static inline int timer_period(void)
 {
return HZ / 20;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/5] Staging: comedi: das16: fix blank line

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This fixes up a blank line after function/struct/union/enum check found
by the checkpatch.pl tool

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das16.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index fd8e0b7..4d6e581 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -198,6 +198,7 @@ enum {
das16_pg_1601,
das16_pg_1602,
 };
+
 static const int *const das16_gainlists[] = {
NULL,
das16jr_gainlist,
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 4/5] Staging: comedi: das800: Prefer unsigned int instead of unsigned

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This fixes up a WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
found by the checkpatch.pl tool.

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das800.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das800.c 
b/drivers/staging/comedi/drivers/das800.c
index 0680d87..ef48c48 100644
--- a/drivers/staging/comedi/drivers/das800.c
+++ b/drivers/staging/comedi/drivers/das800.c
@@ -218,7 +218,7 @@ struct das800_private {
 };
 
 static void das800_ind_write(struct comedi_device *dev,
-unsigned val, unsigned reg)
+unsigned int val, unsigned int reg)
 {
/*
 * Select dev->iobase + 2 to be desired register
@@ -228,7 +228,7 @@ static void das800_ind_write(struct comedi_device *dev,
outb(val, dev->iobase + 2);
 }
 
-static unsigned das800_ind_read(struct comedi_device *dev, unsigned reg)
+static unsigned int das800_ind_read(struct comedi_device *dev, unsigned int 
reg)
 {
/*
 * Select dev->iobase + 7 to be desired register
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 5/5] Staging: comedi: dmm32at: Prefer using the BIT macro

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This fixes all occurences of (1<
---
 drivers/staging/comedi/drivers/dmm32at.c | 86 
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dmm32at.c 
b/drivers/staging/comedi/drivers/dmm32at.c
index 958c0d4..4ca6104 100644
--- a/drivers/staging/comedi/drivers/dmm32at.c
+++ b/drivers/staging/comedi/drivers/dmm32at.c
@@ -46,73 +46,73 @@
 #define DMM32AT_AI_START_CONV_REG  0x00
 #define DMM32AT_AI_LSB_REG 0x00
 #define DMM32AT_AUX_DOUT_REG   0x01
-#define DMM32AT_AUX_DOUT2  (1 << 2)  /* J3.42 - OUT2 (OUT2EN) */
-#define DMM32AT_AUX_DOUT1  (1 << 1)  /* J3.43 */
-#define DMM32AT_AUX_DOUT0  (1 << 0)  /* J3.44 - OUT0 (OUT0EN) */
+#define DMM32AT_AUX_DOUT2  BIT(2)  /* J3.42 - OUT2 (OUT2EN) */
+#define DMM32AT_AUX_DOUT1  BIT(1)  /* J3.43 */
+#define DMM32AT_AUX_DOUT0  BIT(0)  /* J3.44 - OUT0 (OUT0EN) */
 #define DMM32AT_AI_MSB_REG 0x01
 #define DMM32AT_AI_LO_CHAN_REG 0x02
 #define DMM32AT_AI_HI_CHAN_REG 0x03
 #define DMM32AT_AUX_DI_REG 0x04
-#define DMM32AT_AUX_DI_DACBUSY (1 << 7)
-#define DMM32AT_AUX_DI_CALBUSY (1 << 6)
-#define DMM32AT_AUX_DI3(1 << 3)  /* J3.45 - ADCLK 
(CLKSEL) */
-#define DMM32AT_AUX_DI2(1 << 2)  /* J3.46 - GATE12 
(GT12EN) */
-#define DMM32AT_AUX_DI1(1 << 1)  /* J3.47 - GATE0 
(GT0EN) */
-#define DMM32AT_AUX_DI0(1 << 0)  /* J3.48 - CLK0 
(SRC0) */
+#define DMM32AT_AUX_DI_DACBUSY BIT(7)
+#define DMM32AT_AUX_DI_CALBUSY BIT(6)
+#define DMM32AT_AUX_DI3BIT(3)  /* J3.45 - ADCLK 
(CLKSEL) */
+#define DMM32AT_AUX_DI2BIT(2)  /* J3.46 - GATE12 
(GT12EN) */
+#define DMM32AT_AUX_DI1BIT(1)  /* J3.47 - GATE0 
(GT0EN) */
+#define DMM32AT_AUX_DI0BIT(0)  /* J3.48 - CLK0 (SRC0) 
*/
 #define DMM32AT_AO_LSB_REG 0x04
 #define DMM32AT_AO_MSB_REG 0x05
 #define DMM32AT_AO_MSB_DACH(x) ((x) << 6)
 #define DMM32AT_FIFO_DEPTH_REG 0x06
 #define DMM32AT_FIFO_CTRL_REG  0x07
-#define DMM32AT_FIFO_CTRL_FIFOEN   (1 << 3)
-#define DMM32AT_FIFO_CTRL_SCANEN   (1 << 2)
-#define DMM32AT_FIFO_CTRL_FIFORST  (1 << 1)
+#define DMM32AT_FIFO_CTRL_FIFOEN   BIT(3)
+#define DMM32AT_FIFO_CTRL_SCANEN   BIT(2)
+#define DMM32AT_FIFO_CTRL_FIFORST  BIT(1)
 #define DMM32AT_FIFO_STATUS_REG0x07
-#define DMM32AT_FIFO_STATUS_EF (1 << 7)
-#define DMM32AT_FIFO_STATUS_HF (1 << 6)
-#define DMM32AT_FIFO_STATUS_FF (1 << 5)
-#define DMM32AT_FIFO_STATUS_OVF(1 << 4)
-#define DMM32AT_FIFO_STATUS_FIFOEN (1 << 3)
-#define DMM32AT_FIFO_STATUS_SCANEN (1 << 2)
+#define DMM32AT_FIFO_STATUS_EF BIT(7)
+#define DMM32AT_FIFO_STATUS_HF BIT(6)
+#define DMM32AT_FIFO_STATUS_FF BIT(5)
+#define DMM32AT_FIFO_STATUS_OVFBIT(4)
+#define DMM32AT_FIFO_STATUS_FIFOEN BIT(3)
+#define DMM32AT_FIFO_STATUS_SCANEN BIT(2)
 #define DMM32AT_FIFO_STATUS_PAGE_MASK  (3 << 0)
 #define DMM32AT_CTRL_REG   0x08
-#define DMM32AT_CTRL_RESETA(1 << 5)
-#define DMM32AT_CTRL_RESETD(1 << 4)
-#define DMM32AT_CTRL_INTRST(1 << 3)
+#define DMM32AT_CTRL_RESETABIT(5)
+#define DMM32AT_CTRL_RESETDBIT(4)
+#define DMM32AT_CTRL_INTRSTBIT(3)
 #define DMM32AT_CTRL_PAGE_8254 (0 << 0)
-#define DMM32AT_CTRL_PAGE_8255 (1 << 0)
+#define DMM32AT_CTRL_PAGE_8255 BIT(0)
 #define DMM32AT_CTRL_PAGE_CALIB(3 << 0)
 #define DMM32AT_AI_STATUS_REG  0x08
-#define DMM32AT_AI_STATUS_STS  (1 << 7)
-#define DMM32AT_AI_STATUS_SD1  (1 << 6)
-#define DMM32AT_AI_STATUS_SD0  (1 << 5)
+#define DMM32AT_AI_STATUS_STS  BIT(7)
+#define DMM32AT_AI_STATUS_SD1  BIT(6)
+#define DMM32AT_AI_STATUS_SD0  BIT(5)
 #define DMM32AT_AI_STATUS_ADCH_MASK(0x1f << 0)
 #define DMM32AT_INTCLK_REG 0x09
-#define DMM32AT_INTCLK_ADINT   (1 << 7)
-#define DMM32AT_INTCLK_DINT(1 << 6)
-#define DMM32AT_INTCLK_TINT(1 << 5)
-#define DMM32AT_INTCLK_CLKEN   (1 << 1)  /* 1=see below  0=software */
-#define DMM32AT_INTCLK_CLKSEL  (1 << 0)  /* 1=OUT2  0=EXTCLK */
+#define DMM32AT_INTCLK_ADINT   BIT(7)
+#define DMM32AT_INTCLK_DINTBIT(6)
+#define DMM32AT_INTCLK_TINTBIT(5)
+#define DMM32AT_INTCLK_CLKEN   BIT(1)  /* 1=see below  0=software */
+#define DMM32AT_INTCLK_CLKSEL  BIT(0)  /* 1=OUT2  0=EXTCLK */
 #define DMM32AT_CTRDIO_CFG_REG 0x0a
-#define DMM32AT_CTRDIO_CFG_FREQ12  (1 << 7)  /* CLK12 1=100KHz 0=10MHz */
-#define DMM32AT_CTRDIO_CFG_FREQ0   (1 << 6)  /* CLK0  1=10KHz  0

[PATCH 2/5] Staging: comedi: das16: fix Block comment

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This fixes up a WARNING: 'Block comments use a trailing */ on a
separate line'found by the checkpatch.pl tool.

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das16.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index 4d6e581..ef345dc 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -429,8 +429,10 @@ static const struct das16_board das16_boards[] = {
},
 };
 
-/* Period for timer interrupt in jiffies.  It's a function
- * to deal with possibility of dynamic HZ patches  */
+/*
+ * Period for timer interrupt in jiffies.  It's a function
+ * to deal with possibility of dynamic HZ patches
+ */
 static inline int timer_period(void)
 {
return HZ / 20;
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 3/5] Staging: comedi: das800: fix comment issue

2016-06-07 Thread Ravishankar Karkala Mallikarjunayya
This fixes up a WARNING: 'Block comments use a trailing */ on a
separate line'found by the checkpatch.pl tool

Signed-off-by: Ravishankar Karkala Mallikarjunayya 
---
 drivers/staging/comedi/drivers/das800.c | 102 
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das800.c 
b/drivers/staging/comedi/drivers/das800.c
index b02f122..0680d87 100644
--- a/drivers/staging/comedi/drivers/das800.c
+++ b/drivers/staging/comedi/drivers/das800.c
@@ -1,56 +1,56 @@
 /*
-comedi/drivers/das800.c
-Driver for Keitley das800 series boards and compatibles
-Copyright (C) 2000 Frank Mori Hess 
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 2000 David A. Schleef 
-
-This program is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 2 of the License, or
-(at your option) any later version.
-
-This program is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-*/
+ * comedi/drivers/das800.c
+ * Driver for Keitley das800 series boards and compatibles
+ * Copyright (C) 2000 Frank Mori Hess 
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 2000 David A. Schleef 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 /*
-Driver: das800
-Description: Keithley Metrabyte DAS800 (& compatibles)
-Author: Frank Mori Hess 
-Devices: [Keithley Metrabyte] DAS-800 (das-800), DAS-801 (das-801),
-  DAS-802 (das-802),
-  [Measurement Computing] CIO-DAS800 (cio-das800),
-  CIO-DAS801 (cio-das801), CIO-DAS802 (cio-das802),
-  CIO-DAS802/16 (cio-das802/16)
-Status: works, cio-das802/16 untested - email me if you have tested it
-
-Configuration options:
-  [0] - I/O port base address
-  [1] - IRQ (optional, required for timed or externally triggered conversions)
-
-Notes:
-   IRQ can be omitted, although the cmd interface will not work without it.
-
-   All entries in the channel/gain list must use the same gain and be
-   consecutive channels counting upwards in channel number (these are
-   hardware limitations.)
-
-   I've never tested the gain setting stuff since I only have a
-   DAS-800 board with fixed gain.
-
-   The cio-das802/16 does not have a fifo-empty status bit!  Therefore
-   only fifo-half-full transfers are possible with this card.
-
-cmd triggers supported:
-   start_src:  TRIG_NOW | TRIG_EXT
-   scan_begin_src: TRIG_FOLLOW
-   scan_end_src:   TRIG_COUNT
-   convert_src:TRIG_TIMER | TRIG_EXT
-   stop_src:   TRIG_NONE | TRIG_COUNT
-*/
+ * Driver: das800
+ * Description: Keithley Metrabyte DAS800 (& compatibles)
+ * Author: Frank Mori Hess 
+ * Devices: [Keithley Metrabyte] DAS-800 (das-800), DAS-801 (das-801),
+ * DAS-802 (das-802),
+ * [Measurement Computing] CIO-DAS800 (cio-das800),
+ * CIO-DAS801 (cio-das801), CIO-DAS802 (cio-das802),
+ * CIO-DAS802/16 (cio-das802/16)
+ * Status: works, cio-das802/16 untested - email me if you have tested it
+ *
+ * Configuration options:
+ * [0] - I/O port base address
+ *  [1] - IRQ (optional, required for timed or externally triggered 
conversions)
+ *
+ * Notes:
+ * IRQ can be omitted, although the cmd interface will not work without it.
+ *
+ * All entries in the channel/gain list must use the same gain and be
+ * consecutive channels counting upwards in channel number (these are
+ * hardware limitations.)
+ *
+ * I've never tested the gain setting stuff since I only have a
+ * DAS-800 board with fixed gain.
+ *
+ * The cio-das802/16 does not have a fifo-empty status bit!  Therefore
+ * only fifo-half-full transfers are possible with this card.
+ *
+ * cmd triggers supported:
+ * start_src:  TRIG_NOW | TRIG_EXT
+ * scan_begin_src: TRIG_FOLLOW
+ * scan_end_src:   TRIG_COUNT
+ * convert_src:TRIG_TIMER | TRIG_EXT
+ * stop_src:   TRIG_NONE | TRIG_COUNT
+ */
 
 #include 
 #include 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 4/9] staging: unisys: visorhba: simplify and enhance debugfs interface

2016-06-07 Thread Greg KH
On Thu, May 12, 2016 at 09:14:43AM -0400, David Kershner wrote:
> From: Tim Sell 
> 
> debugfs info for each visorhba device is now presented by a file named of
> the following form within the debugfs tree:
> 
> visorhba/vbus:dev/info
> 
> where  is the vbus number, and  is the relative device number.
> 
> Also, the debugfs presentation function was converted to use the seq_file
> interface, so that it could access the device context without resorting to
> a global array.  This also simplified the function.

When you say "also" in a patch changelog, that usually means this should
be split into multiple patches.  I'll take this one as no one cares
about debugfs files, but be careful in the future...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Staging: unisys: visorhba: visorhba_main: fixed a coding style issue

2016-06-07 Thread Greg KH
On Sat, May 21, 2016 at 12:30:46AM +0530, Rumesh Hapuarachchi wrote:
> fixed checkpatch.pl warning about 'Prefer 'unsigned int' to bare use of 
> 'unsigned'
> 
> Signed-off-by: Rumesh Hapuarahcchi 
> ---
>  drivers/staging/unisys/visorhba/visorhba_main.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c 
> b/drivers/staging/unisys/visorhba/visorhba_main.c
> index 6a4570d..3b69b33 100644
> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
> @@ -1122,9 +1122,9 @@ static int visorhba_probe(struct visor_device *dev)
>   if (err < 0)
>   goto err_scsi_host_put;
>  
> - scsihost->max_id = (unsigned)max.max_id;
> - scsihost->max_lun = (unsigned)max.max_lun;
> - scsihost->cmd_per_lun = (unsigned)max.cmd_per_lun;
> + scsihost->max_id = (unsigned int)max.max_id;
> + scsihost->max_lun = (unsigned int)max.max_lun;
> + scsihost->cmd_per_lun = (unsigned int)max.cmd_per_lun;
>   scsihost->max_sectors =
>   (unsigned short)(max.max_io_size >> 9);
>   scsihost->sg_tablesize =

Someone else sent this patch for this issue before you did, sorry :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel