RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path

[Haiyang Zhang]

> -Original Message-
> From: Michael Kelley (EOSG)
> Sent: Saturday, March 24, 2018 12:48 PM
> To: Haiyang Zhang ; da...@davemloft.net;
> net...@vger.kernel.org
> Cc: KY Srinivasan ; Stephen Hemminger
> ; o...@aepfle.de; vkuzn...@redhat.com;
> de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
> Subject: RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path
> 
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org
> >  On Behalf Of Haiyang Zhang
> > Sent: Thursday, March 22, 2018 12:01 PM
> > To: da...@davemloft.net; net...@vger.kernel.org
> > Cc: Haiyang Zhang ; KY Srinivasan
> > ; Stephen Hemminger ;
> > o...@aepfle.de; vkuzn...@redhat.com; de...@linuxdriverproject.org;
> > linux-ker...@vger.kernel.org
> > Subject: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX
> > path
> >
> > From: Haiyang Zhang 
> >
> > As defined in hyperv_net.h, the NVSP_STAT_SUCCESS is one not zero.
> > Some functions returns 0 when it actually means NVSP_STAT_SUCCESS.
> > This patch fixes them.
> >
> > In netvsc_receive(), it puts the last RNDIS packet's receive status
> > for all packets in a vmxferpage which may contain multiple RNDIS
> > packets.
> > This patch puts NVSP_STAT_FAIL in the receive completion if one of the
> > packets in a vmxferpage fails.
> 
> This patch changes the status field that is being reported back to the Hyper-V
> host in the receive completion message in
> enq_receive_complete().   The current code reports 0 on success,
> and with the patch, it will report 1 on success.  So does this change affect
> anything on the Hyper-V side?  Or is Hyper-V just ignoring
> the value?   If this change doesn't have any impact on the
> interactions with Hyper-V, perhaps it would be good to explain why in the
> commit message.

Here is the definition of each status code for NetVSP. 
enum {
NVSP_STAT_NONE = 0,
NVSP_STAT_SUCCESS,
NVSP_STAT_FAIL,
NVSP_STAT_PROTOCOL_TOO_NEW,
NVSP_STAT_PROTOCOL_TOO_OLD,
NVSP_STAT_INVALID_RNDIS_PKT,
NVSP_STAT_BUSY,
NVSP_STAT_PROTOCOL_UNSUPPORTED,
NVSP_STAT_MAX,
};

Existing code returns NVSP_STAT_NONE = 0, and with this patch
we return NVSP_STAT_SUCCESS = 1. 
Based on testing, either way works for now. But for correctness
and future stability (e.g. host side becomes more stringent), we
should follow the protocol.

Thanks,
- Haiyang

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

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 24, 2018, at 8:02 AM, Jonathan Cameron  wrote:
> 
> On Mon, 19 Mar 2018 22:57:16 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
>> 
>> Thank you for all your hard work. Your feedback is really helpful. I’m 
>> surprised that no one from Analog Device has offered any suggestions.
>> 
> 
> Sadly those active in the mainline linux kernel from ADI are focused in
> other areas currently :(
Good point. I will reach out to Analog Devices and get a name of someone 
who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion. 
>> Anyway, please see my comments inline below
>> 
>> Regards,
>> John
>> 
>> 
>> 
>> 
>> 
>>> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
>>> 
>>> On Sat, 17 Mar 2018 23:11:45 -0700
>>> John Syne  wrote:
>>> 
 Hi Jonathan,  
>>> Hi John and All,
>>> 
>>> I'd love to get some additional input on this from anyone interested.
>>> There are a lot of weird and wonderful derived quantities in an energy
>>> meter and it seems we need to make some fundamental changes to support
>>> them - including potentially a userspace breaking change to the event
>>> code description.
>>> 
 
 Here is the complete list of registers for the ADE7878 which I copied from 
 the data sheet. I added a column “IIO Attribute” which I hope follows your 
 IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
 several registers that cannot be generalized and are used purely for chip 
 configuration. I think we should add a new naming convention, namely 
 {register}_{}. Also, I see in the sys_bus_iio doc
> 
> 
> 
> 
 in_accel_x_peak_raw
 
 so shouldn’t the phase be represented as follows:
 
 in_current_A_scale  
>>> I'm still confused.  What does A represent here?  I assumed that was a wild
>>> card for the channel number before but clearly not.
>>> 
>>> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
>>> I guess they sort of look like axis, and sort of like independent channels.
>>> So could be indexed or done via modifiers depending on how you look at it.  
>> In metering terminology, the three phases are commonly referred to as RED, 
>> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
>> ABCN nomenclature so anyone using this driver will probably have referenced 
>> the Analog Devices datasheet so this terminology should be familiar. 
> Which is more commonly used in general?  We are designing what we hope
> will be a general interface and last thing we want is to have everyone
> not using ADI parts confused!  Personally not assigning 'colours' makes
> more sense to me.
I did a little research and here is the naming used by vendor:

ST Microelectronics: P1, P2, P3, N
http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf

Teridian: ABCN
https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf

Maxim Integrated: ABCN
https://datasheets.maximintegrated.com/en/ds/78M6631.pdf

Microchip: ABCN
http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf

NXP: L1, L2, L3, N
https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING

Renesas: ABCN
https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html

So the most consistent would be ABCN


All vendor providing three phase energy metering ICs use the ABCN terminology. 
> 
> (btw please wrap any lines that don't need to be long to around 80 
> characters).
> 
>>> 
>>> Hmm. With neutral in there as well I guess we need to make them
>>> modifiers (but might change my mind later ;)
>>> 
>>> Particularly as we are using the the modifier for RMS under the previous
>>> plan... It appears we should treat that instead like we did for peak
>>> and do it as an additional info mask element.  The problem with doing that
>>> on a continuous measurement is that we can't treat it as a channel to
>>> be output through the buffered interface.  
>> I’ve changed the layout of the spreadsheet to breakout the Direction, Type, 
>> Index, Modifier and Info Mask to make sure there is no miss-understanding 
>> and will help identify all the items we need to add.
>> 
>> The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, 
>> ICWV, VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. 
>> So I guess we have to add an index
> Probably a good idea anyway, we are sort of slowly moving away
> from index less channels.  The ABI always allowed index assignment
> and it makes for easier userspace code.
> 
>> 
>> Address Register IIO Attribute   
>> Dir TypeIndex   ModifierInfo Mask   R/W  
>>Bit Bit Length  TypeDefault 

[PATCH] staging: pi433: cleanup tx_fifo locking

[Valentin Vidic]

pi433_write requires locking due to multiple kfifo writers.  After
acquiring the lock check if enough free space is available in the kfifo
to write the whole message. This check should prevent partial writes to
kfifo so kfifo_reset is not needed anymore.

pi433_tx_thread is the only kfifo reader so it does not require locking
after kfifo_reset is also removed.

Signed-off-by: Valentin Vidic 
---
 drivers/staging/pi433/pi433_if.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index d1e0ddbc79ce..97239b0eb322 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -87,7 +87,7 @@ struct pi433_device {
 
/* tx related values */
STRUCT_KFIFO_REC_1(MSG_FIFO_SIZE) tx_fifo;
-   struct mutextx_fifo_lock; // TODO: check, whether necessary 
or obsolete
+   struct mutextx_fifo_lock; /* serialize multiple writers */
struct task_struct  *tx_task_struct;
wait_queue_head_t   tx_wait_queue;
u8  free_in_fifo;
@@ -589,19 +589,15 @@ pi433_tx_thread(void *data)
 * - size of message
 * - message
 */
-   mutex_lock(>tx_fifo_lock);
-
retval = kfifo_out(>tx_fifo, _cfg, sizeof(tx_cfg));
if (retval != sizeof(tx_cfg)) {
dev_dbg(device->dev, "reading tx_cfg from fifo failed: 
got %d byte(s), expected %d", retval, (unsigned int)sizeof(tx_cfg));
-   mutex_unlock(>tx_fifo_lock);
continue;
}
 
retval = kfifo_out(>tx_fifo, , sizeof(size_t));
if (retval != sizeof(size_t)) {
dev_dbg(device->dev, "reading msg size from fifo 
failed: got %d, expected %d", retval, (unsigned int)sizeof(size_t));
-   mutex_unlock(>tx_fifo_lock);
continue;
}
 
@@ -634,7 +630,6 @@ pi433_tx_thread(void *data)
   sizeof(device->buffer) - position);
dev_dbg(device->dev,
"read %d message byte(s) from fifo queue.", retval);
-   mutex_unlock(>tx_fifo_lock);
 
/* if rx is active, we need to interrupt the waiting for
 * incoming telegrams, to be able to send something.
@@ -818,7 +813,7 @@ pi433_write(struct file *filp, const char __user *buf,
struct pi433_instance   *instance;
struct pi433_device *device;
int retval;
-   unsigned intcopied;
+   unsigned intrequired, available, copied;
 
instance = filp->private_data;
device = instance->device;
@@ -833,6 +828,16 @@ pi433_write(struct file *filp, const char __user *buf,
 * - message
 */
mutex_lock(>tx_fifo_lock);
+
+   required = sizeof(instance->tx_cfg) + sizeof(size_t) + count;
+   available = kfifo_avail(>tx_fifo);
+   if (required > available) {
+   dev_dbg(device->dev, "write to fifo failed: %d bytes required 
but %d available",
+   required, available);
+   mutex_unlock(>tx_fifo_lock);
+   return -EAGAIN;
+   }
+
retval = kfifo_in(>tx_fifo, >tx_cfg,
  sizeof(instance->tx_cfg));
if (retval != sizeof(instance->tx_cfg))
@@ -856,7 +861,6 @@ pi433_write(struct file *filp, const char __user *buf,
 
 abort:
dev_dbg(device->dev, "write to fifo failed: 0x%x", retval);
-   kfifo_reset(>tx_fifo); // TODO: maybe find a solution, not to 
discard already stored, valid entries
mutex_unlock(>tx_fifo_lock);
return -EAGAIN;
 }
-- 
2.16.2

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

Re: [PATCH][next][V2] staging: r8822be: fix typos in header guard macros

[Larry Finger]

On 03/23/2018 01:00 PM, Colin King wrote:

From: Colin Ian King 

The macros for __PHYDMKFREE_H__ and __PHYDM_FEATURES_H__ contain
typos and don't match the #if guard check. Defined them correctly.

Cleans up clang warnings:
warning: '__PHYDMKFREE_H__' is used as a header guard here, followed
by #define of a different macro [-Wheader-guard]
warning: '__PHYDM_FEATURES_H__' is used as a header guard here, followed
by #define of a different macro [-Wheader-guard]

Fixes: 9ce99b04b5b8 ("staging: r8822be: Add phydm mini driver")
Signed-off-by: Colin Ian King 


Acked-by: Larry Finger 


---
  drivers/staging/rtlwifi/phydm/phydm_features.h | 2 +-
  drivers/staging/rtlwifi/phydm/phydm_kfree.h| 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtlwifi/phydm/phydm_features.h 
b/drivers/staging/rtlwifi/phydm/phydm_features.h
index 37f6f0cd7235..a12361c6a1a0 100644
--- a/drivers/staging/rtlwifi/phydm/phydm_features.h
+++ b/drivers/staging/rtlwifi/phydm/phydm_features.h
@@ -24,7 +24,7 @@
   
*/
  
  #ifndef __PHYDM_FEATURES_H__

-#define __PHYDM_FEATURES
+#define __PHYDM_FEATURES_H__
  
  /*phydm debyg report & tools*/
  
diff --git a/drivers/staging/rtlwifi/phydm/phydm_kfree.h b/drivers/staging/rtlwifi/phydm/phydm_kfree.h

index 1ee60059afc1..2c6b0a48e76e 100644
--- a/drivers/staging/rtlwifi/phydm/phydm_kfree.h
+++ b/drivers/staging/rtlwifi/phydm/phydm_kfree.h
@@ -24,7 +24,7 @@
   
*/
  
  #ifndef __PHYDMKFREE_H__

-#define __PHYDKFREE_H__
+#define __PHYDKKFREE_H__
  
  #define KFREE_VERSION "1.0"
  



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

[PATCH 3/4] staging: iio: tsl2x7x: use either direction for IIO_EV_INFO_{ENABLE, PERIOD}

[Brian Masney]

The events IIO_EV_INFO_VALUE and IIO_EV_INFO_ENABLE currently have a
falling and rising direction configured. There does not need to be a
separate distinction so this patch changes these to use the
either direction. Directory listing of event sysfs attributes for a
TSL2772 with this patch applied:

in_intensity0_thresh_either_en
in_intensity0_thresh_either_period
in_intensity0_thresh_falling_value
in_intensity0_thresh_rising_value
in_proximity0_thresh_either_en
in_proximity0_thresh_either_period
in_proximity0_thresh_falling_value
in_proximity0_thresh_rising_value

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2x7x.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c 
b/drivers/staging/iio/light/tsl2x7x.c
index d5a237fb0a0b..940bea2378a9 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1469,17 +1469,16 @@ static const struct iio_event_spec tsl2x7x_events[] = {
{
.type = IIO_EV_TYPE_THRESH,
.dir = IIO_EV_DIR_RISING,
-   .mask_separate = BIT(IIO_EV_INFO_VALUE) |
-   BIT(IIO_EV_INFO_ENABLE),
+   .mask_separate = BIT(IIO_EV_INFO_VALUE),
}, {
.type = IIO_EV_TYPE_THRESH,
.dir = IIO_EV_DIR_FALLING,
-   .mask_separate = BIT(IIO_EV_INFO_VALUE) |
-   BIT(IIO_EV_INFO_ENABLE),
+   .mask_separate = BIT(IIO_EV_INFO_VALUE),
}, {
.type = IIO_EV_TYPE_THRESH,
.dir = IIO_EV_DIR_EITHER,
-   .mask_separate = BIT(IIO_EV_INFO_PERIOD),
+   .mask_separate = BIT(IIO_EV_INFO_PERIOD) |
+   BIT(IIO_EV_INFO_ENABLE),
},
 };
 
-- 
2.14.3

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

[PATCH 1/4] staging: iio: tsl2x7x: use auto increment I2C protocol

[Brian Masney]

The hardware supports 16-bit ALS and proximity readings, however the
datasheet recommends using the I2C auto increment protocol so that the
correct high and low bytes are read even if the integration cycle ends
between reading the lower and upper registers. More information about
this protocol can be found at https://www.i2c-bus.org/auto-increment/.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2x7x.c | 100 
 1 file changed, 67 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c 
b/drivers/staging/iio/light/tsl2x7x.c
index 77a81d75af4f..8530bccdb317 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -80,6 +80,8 @@
 /* tsl2X7X cmd reg masks */
 #define TSL2X7X_CMD_REG0x80
 #define TSL2X7X_CMD_SPL_FN 0x60
+#define TSL2X7X_CMD_REPEAT_PROTO   0x00
+#define TSL2X7X_CMD_AUTOINC_PROTO  0x20
 
 #define TSL2X7X_CMD_PROX_INT_CLR   0X05
 #define TSL2X7X_CMD_ALS_INT_CLR0x06
@@ -320,6 +322,55 @@ static int tsl2x7x_write_control_reg(struct tsl2X7X_chip 
*chip, u8 data)
return ret;
 }
 
+static int tsl2x7x_read_autoinc_regs(struct tsl2X7X_chip *chip, int lower_reg,
+int upper_reg)
+{
+   u8 buf[2];
+   int ret;
+
+   ret = i2c_smbus_write_byte(chip->client,
+  TSL2X7X_CMD_REG | TSL2X7X_CMD_AUTOINC_PROTO |
+  lower_reg);
+   if (ret < 0) {
+   dev_err(>client->dev,
+   "%s: failed to enable auto increment protocol: %d\n",
+   __func__, ret);
+   return ret;
+   }
+
+   ret = i2c_smbus_read_byte_data(chip->client,
+  TSL2X7X_CMD_REG | lower_reg);
+   if (ret < 0) {
+   dev_err(>client->dev,
+   "%s: failed to read from register %x: %d\n", __func__,
+   lower_reg, ret);
+   return ret;
+   }
+   buf[0] = ret;
+
+   ret = i2c_smbus_read_byte_data(chip->client,
+  TSL2X7X_CMD_REG | upper_reg);
+   if (ret < 0) {
+   dev_err(>client->dev,
+   "%s: failed to read from register %x: %d\n", __func__,
+   upper_reg, ret);
+   return ret;
+   }
+   buf[1] = ret;
+
+   ret = i2c_smbus_write_byte(chip->client,
+  TSL2X7X_CMD_REG | TSL2X7X_CMD_REPEAT_PROTO |
+  lower_reg);
+   if (ret < 0) {
+   dev_err(>client->dev,
+   "%s: failed to enable repeated byte protocol: %d\n",
+   __func__, ret);
+   return ret;
+   }
+
+   return le16_to_cpup((const __le16 *)[0]);
+}
+
 /**
  * tsl2x7x_get_lux() - Reads and calculates current lux value.
  * @indio_dev: pointer to IIO device
@@ -340,9 +391,8 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
struct tsl2X7X_chip *chip = iio_priv(indio_dev);
struct tsl2x7x_lux *p;
u32 lux, ratio;
-   int i, ret;
u64 lux64;
-   u8 buf[4];
+   int ret;
 
mutex_lock(>als_mutex);
 
@@ -366,23 +416,17 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
goto out_unlock;
}
 
-   for (i = 0; i < 4; i++) {
-   int reg = TSL2X7X_CMD_REG | (TSL2X7X_ALS_CHAN0LO + i);
-
-   ret = i2c_smbus_read_byte_data(chip->client, reg);
-   if (ret < 0) {
-   dev_err(>client->dev,
-   "%s: failed to read from register %x: %d\n",
-   __func__, reg, ret);
-   goto out_unlock;
-   }
-
-   buf[i] = ret;
-   }
+   ret = tsl2x7x_read_autoinc_regs(chip, TSL2X7X_ALS_CHAN0LO,
+   TSL2X7X_ALS_CHAN0HI);
+   if (ret < 0)
+   goto out_unlock;
+   chip->als_cur_info.als_ch0 = ret;
 
-   /* extract ALS/lux data */
-   chip->als_cur_info.als_ch0 = le16_to_cpup((const __le16 *)[0]);
-   chip->als_cur_info.als_ch1 = le16_to_cpup((const __le16 *)[2]);
+   ret = tsl2x7x_read_autoinc_regs(chip, TSL2X7X_ALS_CHAN1LO,
+   TSL2X7X_ALS_CHAN1HI);
+   if (ret < 0)
+   goto out_unlock;
+   chip->als_cur_info.als_ch1 = ret;
 
if (chip->als_cur_info.als_ch0 >= chip->als_saturation ||
chip->als_cur_info.als_ch1 >= chip->als_saturation) {
@@ -456,10 +500,8 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
  */
 static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 {
-   int i;
-   int ret;
-   u8 chdata[2];
struct tsl2X7X_chip *chip = iio_priv(indio_dev);
+   int ret;
 
  

[PATCH 2/4] staging: iio: tsl2x7x: move IIO_CHAN_INFO_CALIB{SCALE, BIAS} to IIO_LIGHT channel

[Brian Masney]

The IIO_CHAN_INFO_CALIBSCALE and IIO_CHAN_INFO_CALIBBIAS masks are
currently associated with the IIO_INTENSITY channel but should be
associated with the IIO_LIGHT channel since these values are used to
calculate the lux. Directory listing of the sysfs attributes for a
TSL2772 with this patch applied:

dev
events
in_illuminance0_calibbias
in_illuminance0_calibrate
in_illuminance0_calibscale
in_illuminance0_calibscale_available
in_illuminance0_input
in_illuminance0_integration_time
in_illuminance0_integration_time_available
in_illuminance0_lux_table
in_illuminance0_target_input
in_intensity0_raw
in_intensity1_raw
in_proximity0_calibrate
in_proximity0_calibscale
in_proximity0_calibscale_available
in_proximity0_raw
name
of_node
power
subsystem
uevent

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2x7x.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c 
b/drivers/staging/iio/light/tsl2x7x.c
index 8530bccdb317..d5a237fb0a0b 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1491,14 +1491,14 @@ static const struct tsl2x7x_chip_info 
tsl2x7x_chip_info_tbl[] = {
.indexed = 1,
.channel = 0,
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
- BIT(IIO_CHAN_INFO_INT_TIME),
+   BIT(IIO_CHAN_INFO_INT_TIME) |
+   BIT(IIO_CHAN_INFO_CALIBSCALE) |
+   BIT(IIO_CHAN_INFO_CALIBBIAS),
}, {
.type = IIO_INTENSITY,
.indexed = 1,
.channel = 0,
-   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-   BIT(IIO_CHAN_INFO_CALIBSCALE) |
-   BIT(IIO_CHAN_INFO_CALIBBIAS),
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
.event_spec = tsl2x7x_events,
.num_event_specs = ARRAY_SIZE(tsl2x7x_events),
}, {
@@ -1531,14 +1531,14 @@ static const struct tsl2x7x_chip_info 
tsl2x7x_chip_info_tbl[] = {
.indexed = 1,
.channel = 0,
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
- BIT(IIO_CHAN_INFO_INT_TIME),
+   BIT(IIO_CHAN_INFO_INT_TIME) |
+   BIT(IIO_CHAN_INFO_CALIBSCALE) |
+   BIT(IIO_CHAN_INFO_CALIBBIAS),
}, {
.type = IIO_INTENSITY,
.indexed = 1,
.channel = 0,
-   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-   BIT(IIO_CHAN_INFO_CALIBSCALE) |
-   BIT(IIO_CHAN_INFO_CALIBBIAS),
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
.event_spec = tsl2x7x_events,
.num_event_specs = ARRAY_SIZE(tsl2x7x_events),
}, {
@@ -1580,14 +1580,14 @@ static const struct tsl2x7x_chip_info 
tsl2x7x_chip_info_tbl[] = {
.indexed = 1,
.channel = 0,
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
- BIT(IIO_CHAN_INFO_INT_TIME),
+   BIT(IIO_CHAN_INFO_INT_TIME) |
+   BIT(IIO_CHAN_INFO_CALIBSCALE) |
+   BIT(IIO_CHAN_INFO_CALIBBIAS),
}, {
.type = IIO_INTENSITY,
.indexed = 1,
.channel = 0,
-   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-   BIT(IIO_CHAN_INFO_CALIBSCALE) |
-   BIT(IIO_CHAN_INFO_CALIBBIAS),
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
.event_spec = tsl2x7x_events,
.num_event_specs = ARRAY_SIZE(tsl2x7x_events),
}, {
-- 
2.14.3

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

[PATCH 4/4] staging: iio: tsl2x7x: move out of staging

[Brian Masney]

Move the tsl2x7x driver out of staging and into mainline.

Signed-off-by: Brian Masney 
---
Note: I intentionally ran git format-patch with --no-renames since
Jonathan likes to see the whole files in the email body for staging
graduation patches.

The #include "tsl2x7x.h" was changed to #include
 in tsl2x7x.c during the rename.

 drivers/iio/light/Kconfig |8 +
 drivers/iio/light/Makefile|1 +
 drivers/iio/light/tsl2x7x.c   | 1784 +
 drivers/staging/iio/Kconfig   |1 -
 drivers/staging/iio/Makefile  |1 -
 drivers/staging/iio/light/Kconfig |   14 -
 drivers/staging/iio/light/Makefile|5 -
 drivers/staging/iio/light/tsl2x7x.c   | 1784 -
 drivers/staging/iio/light/tsl2x7x.h   |  103 --
 include/linux/platform_data/tsl2x7x.h |  103 ++
 10 files changed, 1896 insertions(+), 1908 deletions(-)
 create mode 100644 drivers/iio/light/tsl2x7x.c
 delete mode 100644 drivers/staging/iio/light/Kconfig
 delete mode 100644 drivers/staging/iio/light/Makefile
 delete mode 100644 drivers/staging/iio/light/tsl2x7x.c
 delete mode 100644 drivers/staging/iio/light/tsl2x7x.h
 create mode 100644 include/linux/platform_data/tsl2x7x.h

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 074e50657366..8161e3e1ba1b 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -409,6 +409,14 @@ config TSL2583
 Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
 Access ALS data via iio, sysfs.
 
+config TSL2x7x
+   tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
+   depends on I2C
+   help
+Support for: tsl2571, tsl2671, tmd2671, tsl2771, tmd2771, tsl2572, 
tsl2672,
+tmd2672, tsl2772, tmd2772 devices.
+Provides iio_events and direct access via sysfs.
+
 config TSL4531
tristate "TAOS TSL4531 ambient light sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index f1777036d4f8..a20a3662950b 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2x7x)  += tsl2x7x.o
 obj-$(CONFIG_TSL4531)  += tsl4531.o
 obj-$(CONFIG_US5182D)  += us5182d.o
 obj-$(CONFIG_VCNL4000) += vcnl4000.o
diff --git a/drivers/iio/light/tsl2x7x.c b/drivers/iio/light/tsl2x7x.c
new file mode 100644
index ..fffd23ae5c72
--- /dev/null
+++ b/drivers/iio/light/tsl2x7x.c
@@ -0,0 +1,1784 @@
+/*
+ * Device driver for monitoring ambient light intensity in (lux)
+ * and proximity detection (prox) within the TAOS TSL2X7X family of devices.
+ *
+ * Copyright (c) 2012, TAOS Corporation.
+ * Copyright (c) 2017-2018 Brian Masney 
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Cal defs*/
+#define PROX_STAT_CAL  0
+#define PROX_STAT_SAMP 1
+#define MAX_SAMPLES_CAL200
+
+/* TSL2X7X Device ID */
+#define TRITON_ID  0x00
+#define SWORDFISH_ID   0x30
+#define HALIBUT_ID 0x20
+
+/* Lux calculation constants */
+#define TSL2X7X_LUX_CALC_OVER_FLOW 65535
+
+/* TAOS Register definitions - note:
+ * depending on device, some of these register are not used and the
+ * register address is benign.
+ */
+/* 2X7X register offsets */
+#define TSL2X7X_MAX_CONFIG_REG 16
+
+/* Device Registers and Masks */
+#define TSL2X7X_CNTRL  0x00
+#define TSL2X7X_ALS_TIME   0X01
+#define TSL2X7X_PRX_TIME   0x02
+#define TSL2X7X_WAIT_TIME  0x03
+#define TSL2X7X_ALS_MINTHRESHLO0X04
+#define TSL2X7X_ALS_MINTHRESHHI0X05
+#define TSL2X7X_ALS_MAXTHRESHLO0X06
+#define TSL2X7X_ALS_MAXTHRESHHI0X07
+#define TSL2X7X_PRX_MINTHRESHLO0X08
+#define TSL2X7X_PRX_MINTHRESHHI0X09
+#define TSL2X7X_PRX_MAXTHRESHLO0X0A
+#define TSL2X7X_PRX_MAXTHRESHHI0X0B
+#define TSL2X7X_PERSISTENCE0x0C

[PATCH 0/4] staging: iio: tsl2x7x: move out of staging

[Brian Masney]

Here is a patch series to move the tsl2x7x driver out of staging and
into mainline. Driver was tested using various TSL2X7X devices on a
Raspberry Pi 2.

Datasheet for the TSl2772:
https://ams.com/eng/content/download/291503/1066377/file/TSL2772_DS000181_2-00.pdf

Brian Masney (4):
  staging: iio: tsl2x7x: use auto increment I2C protocol
  staging: iio: tsl2x7x: move IIO_CHAN_INFO_CALIB{SCALE,BIAS} to
IIO_LIGHT channel
  staging: iio: tsl2x7x: use either direction for
IIO_EV_INFO_{ENABLE,PERIOD}
  staging: iio: tsl2x7x: move out of staging

 drivers/iio/light/Kconfig  |   8 ++
 drivers/iio/light/Makefile |   1 +
 drivers/{staging => }/iio/light/tsl2x7x.c  | 135 +
 drivers/staging/iio/Kconfig|   1 -
 drivers/staging/iio/Makefile   |   1 -
 drivers/staging/iio/light/Kconfig  |  14 ---
 drivers/staging/iio/light/Makefile |   5 -
 .../linux/platform_data}/tsl2x7x.h |   0
 8 files changed, 93 insertions(+), 72 deletions(-)
 rename drivers/{staging => }/iio/light/tsl2x7x.c (95%)
 delete mode 100644 drivers/staging/iio/light/Kconfig
 delete mode 100644 drivers/staging/iio/light/Makefile
 rename {drivers/staging/iio/light => include/linux/platform_data}/tsl2x7x.h 
(100%)

-- 
2.14.3

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

Re: [PATCH 10/11] staging: iio: ad2s1200: Replace angle channel with inclination channel

[Jonathan Cameron]

On Sat, 24 Mar 2018 15:57:19 +0100
David Julian Veenstra  wrote:

> On 24, March 2018 14:12, Jonathan Cameron wrote:
> 
> > On Sat, 24 Mar 2018 13:36:44 +0100
> > David Julian Veenstra  wrote:
> >  
> >> On 23, March 2018 14:27, Jonathan Cameron wrote:
> >>   
> >> > On Sun, 18 Mar 2018 14:37:04 +0100
> >> > David Veenstra  wrote:
> >> >
> >> >> The angle channel is not defined in sysfs iio ABI. So it is replaced
> >> >> with an inclination channel, because it is defined in the ABI, and has 
> >> >> the
> >> >> semantics of an angle.
> >> >> 
> >> >> In addition, a fractional scaling factor of 360 / (2^12 -1) is added,
> >> >> to scale the 12-bits angular position to degrees, conform the ABI.
> >> >> 
> >> >> Signed-off-by: David Veenstra 
> >> > No, please do not blugeon a device into the existing documented ABI.
> >> >
> >> > A resolver does not measure something that can be remotely
> >> > sensibly mapped to inclination.  Inclination is relative to 'down'.
> >> > A resolver doesn't care about down.
> >> >
> >> > Add an angle type to the ABI docs. It simply hasn't been documented
> >> > before because there were no drivers outside staging that use it.
> >> 
> >> I'm sorry, I misunderstood our previous discussing on this topic
> >> (https://marc.info/?l=linux-iio=152087432322446=2) as saying:
> >> "there already exists another channel type that is a good enough match".  
> > Ah, no I was making the point we need to match with the 'units' not
> > use the channel type. 
> >
> > Hmm. We have an an unfortunate inconsistency between use of radians/sec
> > and degrees for different types.
> >
> > Radians feels perfectly sensible for a rotary device, but not so much
> > for an angle of tilt.
> >
> > I'm not sure how we resolve this cleanly or if we can.
> > My gut feeling is we need to go with radians for angle measures on
> > a rotating devices (to match against anglvel) and leave inclination
> > in degrees.  
> 
> I agree that radians doesn't make sense for inclination, and that it
> makes sense to have consistency between the unit of angle and that of
> angular velocity. 
> 
> However, if ABI consistency is desired, another option would be to
> change the unit of angular velocity to degrees per seconds. Then
> everything would be the same. But this sounds like a very painful
> change...
It's effectively impossible without ABI breakage and getting shouted
at by users (and potentially Linus forcing a revert).

The only way we could do it would be to introduce a new 'similarly'
named type and deprecate the old one, removing it some years in the
future.

Unfortunately we (where I meant I :() mess up sometimes and have
to live with the result.

Jonathan
> 
> For now, I'll use radians for the angle. 
> 
> Best regards,
> David Veenstra
> 
> >
> > Sorry for the confusion, I'd missed that the units were inconsistent
> > which would have made that comment harder to interpret.
> >
> > Jonathan
> >  
> >> 
> >> The in_incli and in_rot_from_* family all use degrees as their unit.
> >> For V2 I'll change it back to an angle channel and use angle as its
> >> unit.
> >> 
> >> Best regards,
> >> David Veenstra
> >>   
> >> >
> >> > Jonathan
> >> >
> >> >> ---
> >> >>  drivers/staging/iio/resolver/ad2s1200.c | 11 ---
> >> >>  1 file changed, 8 insertions(+), 3 deletions(-)
> >> >> 
> >> >> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> >> >> b/drivers/staging/iio/resolver/ad2s1200.c
> >> >> index 4b97a106012c..937676bcc0d4 100644
> >> >> --- a/drivers/staging/iio/resolver/ad2s1200.c
> >> >> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> >> >> @@ -64,6 +64,10 @@ static int ad2s1200_read_raw(struct iio_dev 
> >> >> *indio_dev,
> >> >> switch (m) {
> >> >> case IIO_CHAN_INFO_SCALE:
> >> >> switch (chan->type) {
> >> >> +   case IIO_INCLI:
> >> >> +   *val = 360;
> >> >> +   *val2 = 0xFFF;
> >> >> +   return IIO_VAL_FRACTIONAL;
> >> >> case IIO_ANGL_VEL:
> >> >> /*
> >> >>  * 2 * Pi is almost equal to
> >> >> @@ -91,7 +95,7 @@ static int ad2s1200_read_raw(struct iio_dev 
> >> >> *indio_dev,
> >> >> /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> >> >> udelay(1);
> >> >> gpiod_set_value(st->sample, 1);
> >> >> -   gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> >> >> +   gpiod_set_value(st->rdvel, !!(chan->type == IIO_INCLI));
> >> >>  
> >> >> ret = spi_read(st->sdev, st->rx, 2);
> >> >> if (ret < 0) {
> >> >> @@ -101,7 +105,7 @@ static int ad2s1200_read_raw(struct iio_dev 
> >> >> *indio_dev,
> >> >>  
> >> >> vel = be16_to_cpup((__be16 *)st->rx);
> >> >> 

RE: [PATCH v2] storvsc: Set up correct queue depth values for IDE devices

[Michael Kelley (EOSG)]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  
> On Behalf
> Of Long Li
> Sent: Thursday, March 22, 2018 2:47 PM
> To: KY Srinivasan ; Haiyang Zhang 
> ; Stephen
> Hemminger ; James E . J . Bottomley 
> ;
> Martin K . Petersen ; 
> de...@linuxdriverproject.org; linux-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Long Li 
> Subject: [PATCH v2] storvsc: Set up correct queue depth values for IDE devices
> 
> From: Long Li 
> 
> Unlike SCSI and FC, we don't use multiple channels for IDE.
> Also fix the calculation for sub-channels.
> 
> Change log:
> v2: Addressed comment on incorrect number of sub-channels.
> (Michael Kelley )
> 
> Signed-off-by: Long Li 

Reviewed-by: Michael Kelley 

> ---
>  drivers/scsi/storvsc_drv.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 8c51d628b52e..a2ec0bc9e9fa 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1722,11 +1722,14 @@ static int storvsc_probe(struct hv_device *device,
>   max_targets = STORVSC_MAX_TARGETS;
>   max_channels = STORVSC_MAX_CHANNELS;
>   /*
> -  * On Windows8 and above, we support sub-channels for storage.
> +  * On Windows8 and above, we support sub-channels for storage
> +  * on SCSI and FC controllers.
>* The number of sub-channels offerred is based on the number of
>* VCPUs in the guest.
>*/
> - max_sub_channels = (num_cpus / storvsc_vcpus_per_sub_channel);
> + if (!dev_is_ide)
> + max_sub_channels =
> + (num_cpus - 1) / storvsc_vcpus_per_sub_channel;
>   }
> 
>   scsi_driver.can_queue = (max_outstanding_req_per_channel *
> --
> 2.14.1

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

RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path

[Michael Kelley (EOSG)]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  
> On Behalf
> Of Haiyang Zhang
> Sent: Thursday, March 22, 2018 12:01 PM
> To: da...@davemloft.net; net...@vger.kernel.org
> Cc: Haiyang Zhang ; KY Srinivasan 
> ; Stephen
> Hemminger ; o...@aepfle.de; vkuzn...@redhat.com;
> de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
> Subject: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path
> 
> From: Haiyang Zhang 
> 
> As defined in hyperv_net.h, the NVSP_STAT_SUCCESS is one not zero.
> Some functions returns 0 when it actually means NVSP_STAT_SUCCESS.
> This patch fixes them.
> 
> In netvsc_receive(), it puts the last RNDIS packet's receive status
> for all packets in a vmxferpage which may contain multiple RNDIS
> packets.
> This patch puts NVSP_STAT_FAIL in the receive completion if one of
> the packets in a vmxferpage fails.

This patch changes the status field that is being reported back to
the Hyper-V host in the receive completion message in
enq_receive_complete().   The current code reports 0 on success,
and with the patch, it will report 1 on success.  So does this change
affect anything on the Hyper-V side?  Or is Hyper-V just ignoring
the value?   If this change doesn't have any impact on the
interactions with Hyper-V, perhaps it would be good to explain
why in the commit message.

Michael

> 
> Signed-off-by: Haiyang Zhang 
> ---
>  drivers/net/hyperv/netvsc.c   | 8 ++--
>  drivers/net/hyperv/netvsc_drv.c   | 2 +-
>  drivers/net/hyperv/rndis_filter.c | 4 ++--
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index aa95e81af6e5..1ddb2c39b6e4 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -1098,12 +1098,16 @@ static int netvsc_receive(struct net_device *ndev,
>   void *data = recv_buf
>   + vmxferpage_packet->ranges[i].byte_offset;
>   u32 buflen = vmxferpage_packet->ranges[i].byte_count;
> + int ret;
> 
>   trace_rndis_recv(ndev, q_idx, data);
> 
>   /* Pass it to the upper layer */
> - status = rndis_filter_receive(ndev, net_device,
> -   channel, data, buflen);
> + ret = rndis_filter_receive(ndev, net_device,
> +channel, data, buflen);
> +
> + if (unlikely(ret != NVSP_STAT_SUCCESS))
> + status = NVSP_STAT_FAIL;
>   }
> 
>   enq_receive_complete(ndev, net_device, q_idx,
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index cdb78eefab67..33607995be62 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -818,7 +818,7 @@ int netvsc_recv_callback(struct net_device *net,
>   u64_stats_update_end(_stats->syncp);
> 
>   napi_gro_receive(>napi, skb);
> - return 0;
> + return NVSP_STAT_SUCCESS;
>  }
> 
>  static void netvsc_get_drvinfo(struct net_device *net,
> diff --git a/drivers/net/hyperv/rndis_filter.c 
> b/drivers/net/hyperv/rndis_filter.c
> index 2dc00f714482..591fb8080f11 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -443,10 +443,10 @@ int rndis_filter_receive(struct net_device *ndev,
>   "unhandled rndis message (type %u len %u)\n",
>  rndis_msg->ndis_msg_type,
>  rndis_msg->msg_len);
> - break;
> + return NVSP_STAT_FAIL;
>   }
> 
> - return 0;
> + return NVSP_STAT_SUCCESS;
>  }
> 
>  static int rndis_filter_query_device(struct rndis_device *dev,
> --
> 2.15.1

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

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Mon, 19 Mar 2018 23:28:45 -0700
John Syne  wrote:

> Hi Jonathan,
> 
> I broke out the {Direction}_{Type}_{Index}_{Modifier}_{Info_Mask} into 
> separate columns to make sure I understand your instructions. Good way to 
> check the results. 
> 
> Probably easier to copy and paste this table into a spreadsheet. Let me know 
> if there is anything I got wrong. Thank you again for all your help.
Yeah, we need to shrink this if we do it again.

Can drop anything not related to ABI (so RW mask address etc and just have the
register name and the bits around ABI.

Note I mentioned the altvoltage stuff in the previous email, so we need
to think about whether that is useful to distinguish the instantaneous
measurements from the multi cycle AC ones.
Actually I think we are fine here as we explicitly describe all 
alt measurements as mav or rms which going to be handled in our
new computedvalue description.


> 
> Address   RegisterIIO Attribute   Device Tree or Code 
> Direction   TypeIndex   ModifierInfo Mask   R/W Bit 
> Length  Bit Length During CommunicationsTypeDefault Value   
> Description
> 0x4380AIGAIN  in_current0_phaseA_scalein  current 
> 0   phaseA  scale   R/W 24  32 ZPSE S   0x00Phase 
> A current gain adjust.
> 0x4381AVGAIN  in_voltage0_phaseA_scalein  voltage 
> 0   phaseA  scale   R/W 24  32 ZPSE S   0x00Phase 
> A voltage gain adjust.
> 0x4382BIGAIN  in_current0_phaseB_scalein  current 
> 0   phaseB  scale   R/W 24  32 ZPSE S   0x00Phase 
> B current gain adjust.
> 0x4383BVGAIN  in_voltage0_phaseB_scalein  voltage 
> 0   phaseB  scale   R/W 24  32 ZPSE S   0x00Phase 
> B voltage gain adjust.
> 0x4384CIGAIN  in_current0_phaseC_scalein  current 
> 0   phaseC  scale   R/W 24  32 ZPSE S   0x00Phase 
> C current gain adjust.
> 0x4385CVGAIN  in_voltage0_phaseC_scalein  voltage 
> 0   phaseC  scale   R/W 24  32 ZPSE S   0x00Phase 
> C voltage gain adjust.
> 0x4386NIGAIN  in_current0_neutral_scale   in  current 
> 0   neutral scale   R/W 24  32 ZPSE S   0x00
> Neutral current gain adjust (ADE7868 and ADE7878 only).
> 0x4387AIRMSOS in_current0_phaseA_rms_offset   in  current 
> 0   phaseA  offset  R/W 24  32 ZPSE S   0x00Phase 
> A current rms offset.
> 0x4388AVRMSOS in_voltage0_phaseA_rms_offset   in  voltage 
> 0   phaseA  offset  R/W 24  32 ZPSE S   0x00Phase 
> A voltage rms offset.
> 0x4389BIRMSOS in_current0_phaseB_rms_offset   in  current 
> 0   phaseB  offset  R/W 24  32 ZPSE S   0x00Phase 
> B current rms offset.
> 0x438ABVRMSOS in_voltage0_phaseB_rms_offset   in  voltage 
> 0   phaseB  offset  R/W 24  32 ZPSE S   0x00Phase 
> B voltage rms offset.
> 0x438BCIRMSOS in_current0_phaseC_rms_offset   in  current 
> 0   phaseC  offset  R/W 24  32 ZPSE S   0x00Phase 
> C current rms offset.
> 0x438CCVRMSOS in_voltage0_phaseC_rms_offset   in  voltage 
> 0   phaseC  offset  R/W 24  32 ZPSE S   0x00Phase 
> C voltage rms offset.
> 0x438DNIRMSOS in_current0_neutral_rms_offset  in  current 
> 0   neutral offset  R/W 24  32 ZPSE S   0x00
> Neutral current rms offset (ADE7868 and ADE7878 only).
> 0x438EAVAGAIN in_powerapparent0_phaseA_scale  in  
> powerapparent   0   phaseA  scale   R/W 24  32 ZPSE S   
> 0x00Phase A apparent power gain adjust.
> 0x438FBVAGAIN in_powerapparent0_phaseB_scale  in  
> powerapparent   0   phaseB  scale   R/W 24  32 ZPSE S   
> 0x00Phase B apparent power gain adjust.
> 0x4390CVAGAIN in_powerapparent0_phaseC_scale  in  
> powerapparent   0   phaseC  scale   R/W 24  32 ZPSE S   
> 0x00Phase C apparent power gain adjust.
> 0x4391AWGAIN  in_power0_phaseA_scale  in  power   0   
> phaseA  scale   R/W 24  32 ZPSE S   0x00Phase A total 
> active power gain adjust.
> 0x4392AWATTOS in_power0_phaseA_offset in  power   0   
> phaseA  offset  R/W 24  32 ZPSE S   0x00Phase A total 
> active power offset adjust.
> 0x4393BWGAIN  in_power0_phaseB_scale  in  power   0   
> phaseB  scale   R/W 24  32 ZPSE S   0x00Phase B total 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Mon, 19 Mar 2018 22:57:16 -0700
John Syne  wrote:

> Hi Jonathan,
> 
> Thank you for all your hard work. Your feedback is really helpful. I’m 
> surprised that no one from Analog Device has offered any suggestions.
> 

Sadly those active in the mainline linux kernel from ADI are focused in
other areas currently :(
> Anyway, please see my comments inline below
> 
> Regards,
> John
> 
> 
> 
> 
> 
> > On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
> > 
> > On Sat, 17 Mar 2018 23:11:45 -0700
> > John Syne  wrote:
> >   
> >> Hi Jonathan,  
> > Hi John and All,
> > 
> > I'd love to get some additional input on this from anyone interested.
> > There are a lot of weird and wonderful derived quantities in an energy
> > meter and it seems we need to make some fundamental changes to support
> > them - including potentially a userspace breaking change to the event
> > code description.
> >   
> >> 
> >> Here is the complete list of registers for the ADE7878 which I copied from 
> >> the data sheet. I added a column “IIO Attribute” which I hope follows your 
> >> IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
> >> several registers that cannot be generalized and are used purely for chip 
> >> configuration. I think we should add a new naming convention, namely 
> >> {register}_{}. Also, I see in the sys_bus_iio doc




> >> in_accel_x_peak_raw
> >> 
> >> so shouldn’t the phase be represented as follows:
> >> 
> >> in_current_A_scale  
> > I'm still confused.  What does A represent here?  I assumed that was a wild
> > card for the channel number before but clearly not.
> > 
> > Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> > I guess they sort of look like axis, and sort of like independent channels.
> > So could be indexed or done via modifiers depending on how you look at it.  
> In metering terminology, the three phases are commonly referred to as RED, 
> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
> ABCN nomenclature so anyone using this driver will probably have referenced 
> the Analog Devices datasheet so this terminology should be familiar. 
Which is more commonly used in general?  We are designing what we hope
will be a general interface and last thing we want is to have everyone
not using ADI parts confused!  Personally not assigning 'colours' makes
more sense to me.

(btw please wrap any lines that don't need to be long to around 80 characters).

> > 
> > Hmm. With neutral in there as well I guess we need to make them
> > modifiers (but might change my mind later ;)
> > 
> > Particularly as we are using the the modifier for RMS under the previous
> > plan... It appears we should treat that instead like we did for peak
> > and do it as an additional info mask element.  The problem with doing that
> > on a continuous measurement is that we can't treat it as a channel to
> > be output through the buffered interface.  
> I’ve changed the layout of the spreadsheet to breakout the Direction, Type, 
> Index, Modifier and Info Mask to make sure there is no miss-understanding and 
> will help identify all the items we need to add.
> 
> The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, 
> ICWV, VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. 
> So I guess we have to add an index
Probably a good idea anyway, we are sort of slowly moving away
from index less channels.  The ABI always allowed index assignment
and it makes for easier userspace code.

> 
> Address Register  IIO Attribute   
> Dir TypeIndex   ModifierInfo Mask   R/W   
>   Bit Bit Length  TypeDefault Description 
>   
>   
>   Length  During Comm 
> Value   
> 0xE50CIAWVin_current0_phaseA_instantaneousin  
> current 0   phaseA  instantaneous   R   24
>   32 SE   S   N/A Instantaneous value of 
> Phase A current.

I'm unconvinced by "instantaneous".  That is the default assumption that you are
measuring current at this point in time.  I'm still confused on what this 
actually
is. Is it effectively the DC current? I.e. the wave form value for current? 
You answer this below.  They are DC measurements..

Which actually raises a point I'd forgotten.  We have an explicit type 
for altvoltage (defined for DDS devices as the waveform amplitude IIRC).
We should be consistent with that if possible.

So I think this should be.
in_current0_phaseA_raw
etc
A channel can be uniquely identified using index and modifier as as pair (if we 
add
the new field for computation applied 

Re: [PATCH 10/11] staging: iio: ad2s1200: Replace angle channel with inclination channel

[David Julian Veenstra]

On 24, March 2018 14:12, Jonathan Cameron wrote:

> On Sat, 24 Mar 2018 13:36:44 +0100
> David Julian Veenstra  wrote:
>
>> On 23, March 2018 14:27, Jonathan Cameron wrote:
>> 
>> > On Sun, 18 Mar 2018 14:37:04 +0100
>> > David Veenstra  wrote:
>> >  
>> >> The angle channel is not defined in sysfs iio ABI. So it is replaced
>> >> with an inclination channel, because it is defined in the ABI, and has the
>> >> semantics of an angle.
>> >> 
>> >> In addition, a fractional scaling factor of 360 / (2^12 -1) is added,
>> >> to scale the 12-bits angular position to degrees, conform the ABI.
>> >> 
>> >> Signed-off-by: David Veenstra   
>> > No, please do not blugeon a device into the existing documented ABI.
>> >
>> > A resolver does not measure something that can be remotely
>> > sensibly mapped to inclination.  Inclination is relative to 'down'.
>> > A resolver doesn't care about down.
>> >
>> > Add an angle type to the ABI docs. It simply hasn't been documented
>> > before because there were no drivers outside staging that use it.  
>> 
>> I'm sorry, I misunderstood our previous discussing on this topic
>> (https://marc.info/?l=linux-iio=152087432322446=2) as saying:
>> "there already exists another channel type that is a good enough match".
> Ah, no I was making the point we need to match with the 'units' not
> use the channel type. 
>
> Hmm. We have an an unfortunate inconsistency between use of radians/sec
> and degrees for different types.
>
> Radians feels perfectly sensible for a rotary device, but not so much
> for an angle of tilt.
>
> I'm not sure how we resolve this cleanly or if we can.
> My gut feeling is we need to go with radians for angle measures on
> a rotating devices (to match against anglvel) and leave inclination
> in degrees.

I agree that radians doesn't make sense for inclination, and that it
makes sense to have consistency between the unit of angle and that of
angular velocity. 

However, if ABI consistency is desired, another option would be to
change the unit of angular velocity to degrees per seconds. Then
everything would be the same. But this sounds like a very painful
change...

For now, I'll use radians for the angle. 

Best regards,
David Veenstra

>
> Sorry for the confusion, I'd missed that the units were inconsistent
> which would have made that comment harder to interpret.
>
> Jonathan
>
>> 
>> The in_incli and in_rot_from_* family all use degrees as their unit.
>> For V2 I'll change it back to an angle channel and use angle as its
>> unit.
>> 
>> Best regards,
>> David Veenstra
>> 
>> >
>> > Jonathan
>> >  
>> >> ---
>> >>  drivers/staging/iio/resolver/ad2s1200.c | 11 ---
>> >>  1 file changed, 8 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
>> >> b/drivers/staging/iio/resolver/ad2s1200.c
>> >> index 4b97a106012c..937676bcc0d4 100644
>> >> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> >> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> >> @@ -64,6 +64,10 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>> >>   switch (m) {
>> >>   case IIO_CHAN_INFO_SCALE:
>> >>   switch (chan->type) {
>> >> + case IIO_INCLI:
>> >> + *val = 360;
>> >> + *val2 = 0xFFF;
>> >> + return IIO_VAL_FRACTIONAL;
>> >>   case IIO_ANGL_VEL:
>> >>   /*
>> >>* 2 * Pi is almost equal to
>> >> @@ -91,7 +95,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>> >>   /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> >>   udelay(1);
>> >>   gpiod_set_value(st->sample, 1);
>> >> - gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> >> + gpiod_set_value(st->rdvel, !!(chan->type == IIO_INCLI));
>> >>  
>> >>   ret = spi_read(st->sdev, st->rx, 2);
>> >>   if (ret < 0) {
>> >> @@ -101,7 +105,7 @@ static int ad2s1200_read_raw(struct iio_dev 
>> >> *indio_dev,
>> >>  
>> >>   vel = be16_to_cpup((__be16 *)st->rx);
>> >>   switch (chan->type) {
>> >> - case IIO_ANGL:
>> >> + case IIO_INCLI:
>> >>   *val = vel >> 4;
>> >>   ret = IIO_VAL_INT;
>> >>   break;
>> >> @@ -128,10 +132,11 @@ static int ad2s1200_read_raw(struct iio_dev 
>> >> *indio_dev,
>> >>  
>> >>  static const struct iio_chan_spec ad2s1200_channels[] = {
>> >>   {
>> >> - .type = IIO_ANGL,
>> >> + .type = IIO_INCLI,
>> >>   .indexed = 1,
>> >>   .channel = 0,
>> >>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> >> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>> >>   }, {
>> >>   .type = IIO_ANGL_VEL,
>> >>   .indexed = 1,  
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to 

Re: [PATCH 4/4] Staging: iio: accel: adis16201: Move adis16201 driver out of staging

[Jonathan Cameron]

On Fri, 23 Mar 2018 00:42:45 +0530
Himanshu Jha  wrote:

> Move adis16201 driver out of staging and merge into mainline
> IIO subsystem.
> 
> Signed-off-by: Himanshu Jha 

There are a few really minor points inline.  However,
non prevent this moving out of staging (and I'll fix one up anyway).

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Note this won't go upstream now until after the merge window so
will take a little time.  Just unfortunate timing against the
normal kernel cycle I'm afraid!

Thanks,

Jonathan


> ---
>  drivers/iio/accel/Kconfig |  12 ++
>  drivers/iio/accel/Makefile|   1 +
>  drivers/iio/accel/adis16201.c | 321 
> ++
>  drivers/staging/iio/accel/Kconfig |  12 --
>  drivers/staging/iio/accel/Makefile|   1 -
>  drivers/staging/iio/accel/adis16201.c | 321 
> --
>  6 files changed, 334 insertions(+), 334 deletions(-)
>  create mode 100644 drivers/iio/accel/adis16201.c
>  delete mode 100644 drivers/staging/iio/accel/adis16201.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index c6d9517..9416c6f 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -5,6 +5,18 @@
>  
>  menu "Accelerometers"
>  
> +config ADIS16201
> +tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer 
> and Accelerometer"
> +depends on SPI
> +select IIO_ADIS_LIB
> +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> +help
> +  Say Y here to build support for Analog Devices adis16201 dual-axis
> +  digital inclinometer and accelerometer.
> +
> +  To compile this driver as a module, say M here: the module will
> +  be called adis16201.
> +
>  config ADXL345
>   tristate
>  
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 368aedb..7832ec9 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ADIS16201) += adis16201.o
>  obj-$(CONFIG_ADXL345) += adxl345_core.o
>  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
>  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> new file mode 100644
> index 000..51e1afb
> --- /dev/null
> +++ b/drivers/iio/accel/adis16201.c
> @@ -0,0 +1,321 @@
> +/*
> + * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
> + *
> + * Copyright 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define ADIS16201_STARTUP_DELAY_MS   220
> +#define ADIS16201_FLASH_CNT  0x00
> +
> +/* Data Output Register Information */
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG  0x04
> +#define ADIS16201_YACCL_OUT_REG  0x06
> +#define ADIS16201_AUX_ADC_REG0x08
> +#define ADIS16201_TEMP_OUT_REG   0x0A
> +#define ADIS16201_XINCL_OUT_REG  0x0C
> +#define ADIS16201_YINCL_OUT_REG  0x0E
> +
> +/* Calibration Register Definition */
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG0x14
> +#define ADIS16201_YACCL_SCALE_REG0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG0x1C
> +#define ADIS16201_YINCL_SCALE_REG0x1E
> +
> +/* Alarm Register Definition */
> +#define ADIS16201_ALM_MAG1_REG   0x20
> +#define ADIS16201_ALM_MAG2_REG   0x22
> +#define ADIS16201_ALM_SMPL1_REG  0x24
> +#define ADIS16201_ALM_SMPL2_REG  0x26
> +#define ADIS16201_ALM_CTRL_REG   0x28
> +
> +#define ADIS16201_AUX_DAC_REG0x30
> +#define ADIS16201_GPIO_CTRL_REG  0x32
> +#define ADIS16201_SMPL_PRD_REG   0x36
> +/* Operation, filter configuration */
> +#define ADIS16201_AVG_CNT_REG0x38
> +#define ADIS16201_SLP_CNT_REG0x3A
> +
> +/* Miscellaneous Control Register Definition */
> +#define ADIS16201_MSC_CTRL_REG   0x34
> +#define  ADIS16201_MSC_CTRL_SELF_TEST_EN  

Re: [PATCH 3/4] Staging: iio: accel: adis16201: Fix 80 character line limit

[Jonathan Cameron]

On Fri, 23 Mar 2018 00:42:44 +0530
Himanshu Jha  wrote:

> Split the line over 80 characters limit to fix checkpatch
> warning.
> 
> Signed-off-by: Himanshu Jha 
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/accel/adis16201.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index e7593fa..51e1afb 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -206,7 +206,8 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
>  }
>  
>  static const struct iio_chan_spec adis16201_channels[] = {
> - ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 
> 12),
> + ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0,
> +  12),
>   ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
>   ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
>   BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),

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

Re: [PATCH 2/4] Staging: iio: accel: adis16201: Use GENMASK

[Jonathan Cameron]

On Fri, 23 Mar 2018 00:42:43 +0530
Himanshu Jha  wrote:

> Use GENMASK to improve readability and remove the local variables used to
> store intermediate data.
> 
> Signed-off-by: Himanshu Jha 
Applied to the togreg branch of iio.git.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/accel/adis16201.c | 34 +++---
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index b04dbb3..e7593fa 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -185,28 +185,24 @@ static int adis16201_write_raw(struct iio_dev 
> *indio_dev,
>  long mask)
>  {
>   struct adis *st = iio_priv(indio_dev);
> - int bits;
> - s16 val16;
> - u8 addr;
> + int m;
>  
> - switch (mask) {
> - case IIO_CHAN_INFO_CALIBBIAS:
> - switch (chan->type) {
> - case IIO_ACCEL:
> - bits = 12;
> - break;
> - case IIO_INCLI:
> - bits = 9;
> - break;
> - default:
> - return -EINVAL;
> + if (mask != IIO_CHAN_INFO_CALIBBIAS)
> + return -EINVAL;
> +
> + switch (chan->type) {
> + case IIO_ACCEL:
> + m = GENMASK(11, 0);
> + break;
> + case IIO_INCLI:
> + m = GENMASK(8, 0);
> + break;
> + default:
> + return -EINVAL;
>   }
> - val16 = val & ((1 << bits) - 1);
> - addr = adis16201_addresses[chan->scan_index];
> - return adis_write_reg_16(st, addr, val16);
> - }
>  
> - return -EINVAL;
> + return adis_write_reg_16(st, adis16201_addresses[chan->scan_index],
> +  val & m);
>  }
>  
>  static const struct iio_chan_spec adis16201_channels[] = {

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

Re: [PATCH 1/4] Staging: iio: accel: adis16201: Remove unused headers

[Jonathan Cameron]

On Fri, 23 Mar 2018 00:42:42 +0530
Himanshu Jha  wrote:

> Remove few unused headers files since the adis core handles the buffer and
> sysfs support.
> 
> Signed-off-by: Himanshu Jha 
Applied to the togreg branch of iio.git and pushed out as testing.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/accel/adis16201.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 97e25a3..b04dbb3 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -6,7 +6,6 @@
>   * Licensed under the GPL-2 or later.
>   */
>  
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -16,8 +15,6 @@
>  #include 
>  
>  #include 
> -#include 
> -#include 
>  #include 
>  
>  #define ADIS16201_STARTUP_DELAY_MS   220

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

Re: [PATCH 10/11] staging: iio: ad2s1200: Replace angle channel with inclination channel

[Jonathan Cameron]

On Sat, 24 Mar 2018 13:36:44 +0100
David Julian Veenstra  wrote:

> On 23, March 2018 14:27, Jonathan Cameron wrote:
> 
> > On Sun, 18 Mar 2018 14:37:04 +0100
> > David Veenstra  wrote:
> >  
> >> The angle channel is not defined in sysfs iio ABI. So it is replaced
> >> with an inclination channel, because it is defined in the ABI, and has the
> >> semantics of an angle.
> >> 
> >> In addition, a fractional scaling factor of 360 / (2^12 -1) is added,
> >> to scale the 12-bits angular position to degrees, conform the ABI.
> >> 
> >> Signed-off-by: David Veenstra   
> > No, please do not blugeon a device into the existing documented ABI.
> >
> > A resolver does not measure something that can be remotely
> > sensibly mapped to inclination.  Inclination is relative to 'down'.
> > A resolver doesn't care about down.
> >
> > Add an angle type to the ABI docs. It simply hasn't been documented
> > before because there were no drivers outside staging that use it.  
> 
> I'm sorry, I misunderstood our previous discussing on this topic
> (https://marc.info/?l=linux-iio=152087432322446=2) as saying:
> "there already exists another channel type that is a good enough match".
Ah, no I was making the point we need to match with the 'units' not
use the channel type. 

Hmm. We have an an unfortunate inconsistency between use of radians/sec
and degrees for different types.

Radians feels perfectly sensible for a rotary device, but not so much
for an angle of tilt.

I'm not sure how we resolve this cleanly or if we can.
My gut feeling is we need to go with radians for angle measures on
a rotating devices (to match against anglvel) and leave inclination
in degrees.

Sorry for the confusion, I'd missed that the units were inconsistent
which would have made that comment harder to interpret.

Jonathan

> 
> The in_incli and in_rot_from_* family all use degrees as their unit.
> For V2 I'll change it back to an angle channel and use angle as its
> unit.
> 
> Best regards,
> David Veenstra
> 
> >
> > Jonathan
> >  
> >> ---
> >>  drivers/staging/iio/resolver/ad2s1200.c | 11 ---
> >>  1 file changed, 8 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> >> b/drivers/staging/iio/resolver/ad2s1200.c
> >> index 4b97a106012c..937676bcc0d4 100644
> >> --- a/drivers/staging/iio/resolver/ad2s1200.c
> >> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> >> @@ -64,6 +64,10 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> >>switch (m) {
> >>case IIO_CHAN_INFO_SCALE:
> >>switch (chan->type) {
> >> +  case IIO_INCLI:
> >> +  *val = 360;
> >> +  *val2 = 0xFFF;
> >> +  return IIO_VAL_FRACTIONAL;
> >>case IIO_ANGL_VEL:
> >>/*
> >> * 2 * Pi is almost equal to
> >> @@ -91,7 +95,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> >>/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> >>udelay(1);
> >>gpiod_set_value(st->sample, 1);
> >> -  gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> >> +  gpiod_set_value(st->rdvel, !!(chan->type == IIO_INCLI));
> >>  
> >>ret = spi_read(st->sdev, st->rx, 2);
> >>if (ret < 0) {
> >> @@ -101,7 +105,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> >>  
> >>vel = be16_to_cpup((__be16 *)st->rx);
> >>switch (chan->type) {
> >> -  case IIO_ANGL:
> >> +  case IIO_INCLI:
> >>*val = vel >> 4;
> >>ret = IIO_VAL_INT;
> >>break;
> >> @@ -128,10 +132,11 @@ static int ad2s1200_read_raw(struct iio_dev 
> >> *indio_dev,
> >>  
> >>  static const struct iio_chan_spec ad2s1200_channels[] = {
> >>{
> >> -  .type = IIO_ANGL,
> >> +  .type = IIO_INCLI,
> >>.indexed = 1,
> >>.channel = 0,
> >>.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> >> +  .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> >>}, {
> >>.type = IIO_ANGL_VEL,
> >>.indexed = 1,  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 05/11] staging: iio: ad2s1200: Introduce variable for repeated value

[Jonathan Cameron]

On Sat, 24 Mar 2018 13:22:22 +0100
David Julian Veenstra  wrote:

> On 23, March 2018 14:17, Jonathan Cameron wrote:
> 
> > On Sun, 18 Mar 2018 14:35:46 +0100
> > David Veenstra  wrote:
> >  
> >> Add variable to hold >dev in ad2s1200_probe. This value is repeatedly
> >> used in ad2s1200_probe.
> >> 
> >> Signed-off-by: David Veenstra   
> > I'm a little unconvinced by this one. It adds code to save
> > a really very trivial bit of repetition.  If it had been
> > via another pointer or this would have made a big difference
> > in number of lines by bringing a lot of entries below 80 chars
> > that weren't previous, then there would be a good argument.
> >
> > Here, not so much.  
> 
> The 9th patch adds another 4 occurrences. So in total >dev appears
> 9 times in ad2s1200_probe. To me it seemed worth it to
> introduce a variable for it. But I agree that it doesn't add much.
> 
> Best regards,
> David Veenstra
> 
OK, up to you then, I don't care enough to resist the change.

Jonathan


> >
> > Jonathan  
> >> ---
> >>  drivers/staging/iio/resolver/ad2s1200.c | 13 -
> >>  1 file changed, 8 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
> >> b/drivers/staging/iio/resolver/ad2s1200.c
> >> index 00efba598671..eceb86e952de 100644
> >> --- a/drivers/staging/iio/resolver/ad2s1200.c
> >> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> >> @@ -118,19 +118,22 @@ static int ad2s1200_probe(struct spi_device *spi)
> >>unsigned short *pins = spi->dev.platform_data;
> >>struct ad2s1200_state *st;
> >>struct iio_dev *indio_dev;
> >> +  struct device *dev;
> >>int pn, ret = 0;
> >>  
> >> +  dev = >dev;
> >> +
> >>for (pn = 0; pn < AD2S1200_PN; pn++) {
> >> -  ret = devm_gpio_request_one(>dev, pins[pn], GPIOF_DIR_OUT,
> >> +  ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
> >>DRV_NAME);
> >>if (ret) {
> >> -  dev_err(>dev, "request gpio pin %d failed\n",
> >> +  dev_err(dev, "request gpio pin %d failed\n",
> >>pins[pn]);
> >>return ret;
> >>}
> >>}
> >>  
> >> -  indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
> >> +  indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> >>if (!indio_dev)
> >>return -ENOMEM;
> >>  
> >> @@ -141,14 +144,14 @@ static int ad2s1200_probe(struct spi_device *spi)
> >>st->sample = pins[0];
> >>st->rdvel = pins[1];
> >>  
> >> -  indio_dev->dev.parent = >dev;
> >> +  indio_dev->dev.parent = dev;
> >>indio_dev->info = _info;
> >>indio_dev->modes = INDIO_DIRECT_MODE;
> >>indio_dev->channels = ad2s1200_channels;
> >>indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
> >>indio_dev->name = spi_get_device_id(spi)->name;
> >>  
> >> -  ret = devm_iio_device_register(>dev, indio_dev);
> >> +  ret = devm_iio_device_register(dev, indio_dev);
> >>if (ret)
> >>return ret;
> >>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 11/11] staging: iio: tsl2x7x: add copyright

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:12 -0400
Brian Masney  wrote:

> Add Brian Masney's copyright and to the list of module authors for all
> of the staging cleanups. This patch also update's Jon Brenner's current
> work email address since AMS now owns TAOS.
> 
> Signed-off-by: Brian Masney 
Applied to the togreg branch of iio.git and pushed out as testing.
Likely these will be going upstream after the coming merge window.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 5f78664d2b0e..77a81d75af4f 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -3,6 +3,7 @@
>   * and proximity detection (prox) within the TAOS TSL2X7X family of devices.
>   *
>   * Copyright (c) 2012, TAOS Corporation.
> + * Copyright (c) 2017-2018 Brian Masney 
>   *
>   * 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
> @@ -1744,6 +1745,7 @@ static struct i2c_driver tsl2x7x_driver = {
>  
>  module_i2c_driver(tsl2x7x_driver);
>  
> -MODULE_AUTHOR("J. August Brenner");
> +MODULE_AUTHOR("J. August Brenner ");
> +MODULE_AUTHOR("Brian Masney ");
>  MODULE_DESCRIPTION("TAOS tsl2x7x ambient and proximity light sensor driver");
>  MODULE_LICENSE("GPL");

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

Re: [PATCH 10/11] staging: iio: tsl2x7x: put local variables in reverse Christmas tree order

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:11 -0400
Brian Masney  wrote:

> This patch ensures that all of the local variable declarations are in
> reverse Christmas tree order where possible to increase code
> readability.
> 
> Signed-off-by: Brian Masney 
Applied. As ever it's a minor improvement but I suppose worth doing
whilst we are there!

Thanks,

Jonathan.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 21 -
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 65b7fbca8c5d..5f78664d2b0e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -336,13 +336,12 @@ static int tsl2x7x_write_control_reg(struct 
> tsl2X7X_chip *chip, u8 data)
>   */
>  static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  {
> - u32 lux; /* raw lux calculated from device data */
> - u64 lux64;
> - u32 ratio;
> - u8 buf[4];
> - struct tsl2x7x_lux *p;
>   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> + struct tsl2x7x_lux *p;
> + u32 lux, ratio;
>   int i, ret;
> + u64 lux64;
> + u8 buf[4];
>  
>   mutex_lock(>als_mutex);
>  
> @@ -589,13 +588,9 @@ static int tsl2x7x_als_calibrate(struct iio_dev 
> *indio_dev)
>  
>  static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  {
> - int i;
> - int ret = 0;
> - u8 *dev_reg;
> - int als_count;
> - int als_time;
>   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> - u8 reg_val = 0;
> + int ret, i, als_count, als_time;
> + u8 *dev_reg, reg_val;
>  
>   /* Non calculated parameters */
>   chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
> @@ -1121,8 +1116,8 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
>   int *val2,
>   long mask)
>  {
> - int ret = -EINVAL;
>   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> + int ret = -EINVAL;
>  
>   switch (mask) {
>   case IIO_CHAN_INFO_PROCESSED:
> @@ -1583,9 +1578,9 @@ static const struct tsl2x7x_chip_info 
> tsl2x7x_chip_info_tbl[] = {
>  static int tsl2x7x_probe(struct i2c_client *clientp,
>const struct i2c_device_id *id)
>  {
> - int ret;
>   struct iio_dev *indio_dev;
>   struct tsl2X7X_chip *chip;
> + int ret;
>  
>   indio_dev = devm_iio_device_alloc(>dev, sizeof(*chip));
>   if (!indio_dev)

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

Re: [PATCH 09/11] staging: iio: tsl2x7x: remove ch0 and ch1 variables from tsl2x7x_get_lux()

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:10 -0400
Brian Masney  wrote:

> Remove the ch0 and ch1 variables from tsl2x7x_get_lux() and
> write those values directly into the chip->als_cur_info.als_ch0
> and chip->als_cur_info.als_ch01 variables.
> 
> Signed-off-by: Brian Masney 
Hmm. A marginal improvement in readability I suppose, but not a big one.
This I wouldn't have bothered changing but fair enough.

Applied to the togreg branch of iio.git and pushed out as testing.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 1e38e8449f9e..65b7fbca8c5d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -336,7 +336,6 @@ static int tsl2x7x_write_control_reg(struct tsl2X7X_chip 
> *chip, u8 data)
>   */
>  static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  {
> - u16 ch0, ch1; /* separated ch0/ch1 data from device */
>   u32 lux; /* raw lux calculated from device data */
>   u64 lux64;
>   u32 ratio;
> @@ -382,24 +381,24 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   }
>  
>   /* extract ALS/lux data */
> - ch0 = le16_to_cpup((const __le16 *)[0]);
> - ch1 = le16_to_cpup((const __le16 *)[2]);
> + chip->als_cur_info.als_ch0 = le16_to_cpup((const __le16 *)[0]);
> + chip->als_cur_info.als_ch1 = le16_to_cpup((const __le16 *)[2]);
>  
> - chip->als_cur_info.als_ch0 = ch0;
> - chip->als_cur_info.als_ch1 = ch1;
> -
> - if (ch0 >= chip->als_saturation || ch1 >= chip->als_saturation) {
> + if (chip->als_cur_info.als_ch0 >= chip->als_saturation ||
> + chip->als_cur_info.als_ch1 >= chip->als_saturation) {
>   lux = TSL2X7X_LUX_CALC_OVER_FLOW;
>   goto return_max;
>   }
>  
> - if (!ch0) {
> + if (!chip->als_cur_info.als_ch0) {
>   /* have no data, so return LAST VALUE */
>   ret = chip->als_cur_info.lux;
>   goto out_unlock;
>   }
> +
>   /* calculate ratio */
> - ratio = (ch1 << 15) / ch0;
> + ratio = (chip->als_cur_info.als_ch1 << 15) / chip->als_cur_info.als_ch0;
> +
>   /* convert to unscaled lux using the pointer to the table */
>   p = (struct tsl2x7x_lux *)chip->tsl2x7x_device_lux;
>   while (p->ratio != 0 && p->ratio < ratio)
> @@ -408,9 +407,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   if (p->ratio == 0) {
>   lux = 0;
>   } else {
> - lux = DIV_ROUND_UP(ch0 * p->ch0,
> + lux = DIV_ROUND_UP(chip->als_cur_info.als_ch0 * p->ch0,
>  tsl2x7x_als_gain[chip->settings.als_gain]) -
> -   DIV_ROUND_UP(ch1 * p->ch1,
> +   DIV_ROUND_UP(chip->als_cur_info.als_ch1 * p->ch1,
>  tsl2x7x_als_gain[chip->settings.als_gain]);
>   }
>  

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

Re: [PATCH 07/11] staging: iio: tsl2x7x: split out als and prox persistence settings

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:08 -0400
Brian Masney  wrote:

> The struct tsl2x7x_settings contained a persistence member that
> contained both the ALS and proximity persistence fields. This patch
> splits this out into two separate fields so that the bitmasks in
> several parts of the code are no longer necessary.
> 
> The default persistence settings are also changed by this patch from:
> 
> - Proximity: 0 (Every proximity cycle generates an interrupt)
> - ALS: 255 (60 consecutive values out of range)
> 
> to something a little more reasonable based on my testing:
> 
> - Proximity: 1 (1 proximity value out of range)
> - ALS: 1 (1 value outside of threshold range)
> 
> Signed-off-by: Brian Masney 
Applied, thanks.

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 24 +++-
>  drivers/staging/iio/light/tsl2x7x.h |  9 ++---
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 07ce3076a05d..c1e441857226 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -226,10 +226,11 @@ static const struct tsl2x7x_settings 
> tsl2x7x_default_settings = {
>   .prox_config = 0,
>   .als_gain_trim = 1000,
>   .als_cal_target = 150,
> + .als_persistence = 1,
>   .als_interrupt_en = false,
>   .als_thresh_low = 200,
>   .als_thresh_high = 256,
> - .persistence = 255,
> + .prox_persistence = 1,
>   .prox_interrupt_en = false,
>   .prox_thres_low  = 0,
>   .prox_thres_high = 512,
> @@ -621,7 +622,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>   (chip->settings.als_thresh_high) & 0xFF;
>   chip->tsl2x7x_config[TSL2X7X_ALS_MAXTHRESHHI] =
>   (chip->settings.als_thresh_high >> 8) & 0xFF;
> - chip->tsl2x7x_config[TSL2X7X_PERSISTENCE] = chip->settings.persistence;
> + chip->tsl2x7x_config[TSL2X7X_PERSISTENCE] =
> + (chip->settings.prox_persistence & 0xFF) << 4 |
> + (chip->settings.als_persistence & 0xFF);
>  
>   chip->tsl2x7x_config[TSL2X7X_PRX_COUNT] =
>   chip->settings.prox_pulse_count;
> @@ -1043,15 +1046,10 @@ static int tsl2x7x_write_event_value(struct iio_dev 
> *indio_dev,
>  
>   filter_delay = DIV_ROUND_UP((val * 1000) + val2, z);
>  
> - if (chan->type == IIO_INTENSITY) {
> - chip->settings.persistence &= 0xF0;
> - chip->settings.persistence |=
> - (filter_delay & 0x0F);
> - } else {
> - chip->settings.persistence &= 0x0F;
> - chip->settings.persistence |=
> - ((filter_delay << 4) & 0xF0);
> - }
> + if (chan->type == IIO_INTENSITY)
> + chip->settings.als_persistence = filter_delay;
> + else
> + chip->settings.prox_persistence = filter_delay;
>   ret = 0;
>   break;
>   default:
> @@ -1108,10 +1106,10 @@ static int tsl2x7x_read_event_value(struct iio_dev 
> *indio_dev,
>   case IIO_EV_INFO_PERIOD:
>   if (chan->type == IIO_INTENSITY) {
>   time = chip->settings.als_time;
> - mult = chip->settings.persistence & 0x0F;
> + mult = chip->settings.als_persistence;
>   } else {
>   time = chip->settings.prx_time;
> - mult = (chip->settings.persistence & 0xF0) >> 4;
> + mult = chip->settings.prox_persistence;
>   }
>  
>   /* Determine integration time */
> diff --git a/drivers/staging/iio/light/tsl2x7x.h 
> b/drivers/staging/iio/light/tsl2x7x.h
> index b2aa642299b3..d382cdbb976e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -50,11 +50,13 @@ struct tsl2x7x_lux {
>   *  @prox_config:   Prox configuration filters.
>   *  @als_cal_target:Known external ALS reading for
>   *  calibration.
> - *  @persistence:   H/W Filters, Number of 'out of limits'
> - *  ADC readings PRX/ALS.
> + *  @als_persistence:   H/W Filters, Number of 'out of limits'
> + *  ALS readings.
>   *  @als_interrupt_en:  Enable/Disable ALS interrupts
>   *  @als_thresh_low:CH0 'low' count to trigger interrupt.
>   *  @als_thresh_high:   CH0 'high' count to trigger interrupt.
> + *  @prox_persistence:  H/W Filters, Number of 'out of limits'
> + *  proximity readings.
>   *  @prox_interrupt_en: Enable/Disable proximity interrupts
>   *  @prox_thres_low:Low threshold proximity detection.
>   *  @prox_thres_high:   High threshold proximity detection
> @@ 

Re: [PATCH 08/11] staging: iio: tsl2x7x: remove unused variables from tsl2x7x_get_lux()

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:09 -0400
Brian Masney  wrote:

> tsl2x7x_get_lux() has a ch0lux and ch1lux variables that are not used
> so this patch removes them.
> 
> Signed-off-by: Brian Masney 
Had to argue with this one ;)

Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index c1e441857226..1e38e8449f9e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -344,8 +344,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   struct tsl2x7x_lux *p;
>   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>   int i, ret;
> - u32 ch0lux = 0;
> - u32 ch1lux = 0;
>  
>   mutex_lock(>als_mutex);
>  
> @@ -416,15 +414,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  tsl2x7x_als_gain[chip->settings.als_gain]);
>   }
>  
> - /* note: lux is 31 bit max at this point */
> - if (ch1lux > ch0lux) {
> - dev_dbg(>client->dev,
> - "%s: ch1lux > ch0lux; returning last value\n",
> - __func__);
> - ret = chip->als_cur_info.lux;
> - goto out_unlock;
> - }
> -
>   /* adjust for active time scale */
>   if (chip->als_time_scale == 0)
>   lux = 0;

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

Re: [PATCH 06/11] staging: iio: tsl2x7x: make logging consistent and correct newlines

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:07 -0400
Brian Masney  wrote:

> This patch updates all of the logging commands so that they are
> consistent with the other messages, includes __func__ in the message,
> and all of the messages include newlines. This patch also removes some
> debug log messages.
> 
> Signed-off-by: Brian Masney 
Hmm. This is definitely an improvement, but at somepoint we may want
to drop all of the __func__ usage as the dynamic debug system can
provide back traces which obviously include this info.

So might need a follow up, though perhaps not pre moving out of staging
as this really isn't important.

Hence, applied.

Thanks,

Jonathan

> ---
> Changes since v1:
> - Remove some debug messages and every log message doesn't have to have
>   the function name.
> - Earlier patches in this current series dropped some of the debug
>   messages that were in v1.
> 
>  drivers/staging/iio/light/tsl2x7x.c | 42 
> ++---
>  1 file changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index f7e7fcc17059..07ce3076a05d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -374,7 +374,8 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   ret = i2c_smbus_read_byte_data(chip->client, reg);
>   if (ret < 0) {
>   dev_err(>client->dev,
> - "failed to read. err=%x\n", ret);
> + "%s: failed to read from register %x: %d\n",
> + __func__, reg, ret);
>   goto out_unlock;
>   }
>  
> @@ -416,7 +417,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  
>   /* note: lux is 31 bit max at this point */
>   if (ch1lux > ch0lux) {
> - dev_dbg(>client->dev, "ch1lux > ch0lux-return last 
> value\n");
> + dev_dbg(>client->dev,
> + "%s: ch1lux > ch0lux; returning last value\n",
> + __func__);
>   ret = chip->als_cur_info.lux;
>   goto out_unlock;
>   }
> @@ -591,8 +594,6 @@ static int tsl2x7x_als_calibrate(struct iio_dev 
> *indio_dev)
>   return -ERANGE;
>  
>   chip->settings.als_gain_trim = ret;
> - dev_info(>client->dev,
> -  "%s als_calibrate completed\n", chip->client->name);
>  
>   return ret;
>  }
> @@ -674,12 +675,14 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>*/
>   for (i = 0, dev_reg = chip->tsl2x7x_config;
>   i < TSL2X7X_MAX_CONFIG_REG; i++) {
> - ret = i2c_smbus_write_byte_data(chip->client,
> - TSL2X7X_CMD_REG + i,
> + int reg = TSL2X7X_CMD_REG + i;
> +
> + ret = i2c_smbus_write_byte_data(chip->client, reg,
>   *dev_reg++);
>   if (ret < 0) {
>   dev_err(>client->dev,
> - "failed on write to reg %d.\n", i);
> + "%s: failed to write to register %x: %d\n",
> + __func__, reg, ret);
>   return ret;
>   }
>   }
> @@ -907,15 +910,11 @@ static ssize_t in_illuminance0_lux_table_store(struct 
> device *dev,
>*/
>   n = value[0];
>   if ((n % 3) || n < 6 ||
> - n > ((ARRAY_SIZE(chip->tsl2x7x_device_lux) - 1) * 3)) {
> - dev_info(dev, "LUX TABLE INPUT ERROR 1 Value[0]=%d\n", n);
> + n > ((ARRAY_SIZE(chip->tsl2x7x_device_lux) - 1) * 3))
>   return -EINVAL;
> - }
>  
> - if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0) {
> - dev_info(dev, "LUX TABLE INPUT ERROR 2 Value[0]=%d\n", n);
> + if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0)
>   return -EINVAL;
> - }
>  
>   if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
>   ret = tsl2x7x_chip_off(indio_dev);
> @@ -1048,15 +1047,10 @@ static int tsl2x7x_write_event_value(struct iio_dev 
> *indio_dev,
>   chip->settings.persistence &= 0xF0;
>   chip->settings.persistence |=
>   (filter_delay & 0x0F);
> - dev_info(>client->dev, "%s: ALS persistence = %d",
> -  __func__, filter_delay);
>   } else {
>   chip->settings.persistence &= 0x0F;
>   chip->settings.persistence |=
>   ((filter_delay << 4) & 0xF0);
> - dev_info(>client->dev,
> -  "%s: Proximity persistence = %d",
> -  __func__, filter_delay);
>   }
>   ret = 0;
>   

Re: [PATCH 05/11] staging: iio: tsl2x7x: split out als and prox interrupt settings

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:06 -0400
Brian Masney  wrote:

> The struct tsl2x7x_settings contained an interrupts_en member that was
> a bitmask for which interrupts are enabled. This required having
> bitmasks in several parts of the code. This patch splits this field
> out into two booleans to remove most of the bitmasks in the code.
> 
> This patch also fixes a bug where if an interrupt pin was configured,
> but proximity interrupts were disabled, then the proximity value could
> not be polled.
> 
> This patch also removes an unnecessary second call to writing the
> control register in tsl2x7x_chip_on().
> 
> Driver tested using a TSL2772 hooked up to a Raspberry Pi 2.
> 
> Signed-off-by: Brian Masney 
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 64 
> -
>  drivers/staging/iio/light/tsl2x7x.h |  7 ++--
>  2 files changed, 24 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 99230d9313e1..f7e7fcc17059 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -226,10 +226,11 @@ static const struct tsl2x7x_settings 
> tsl2x7x_default_settings = {
>   .prox_config = 0,
>   .als_gain_trim = 1000,
>   .als_cal_target = 150,
> + .als_interrupt_en = false,
>   .als_thresh_low = 200,
>   .als_thresh_high = 256,
>   .persistence = 255,
> - .interrupts_en = 0,
> + .prox_interrupt_en = false,
>   .prox_thres_low  = 0,
>   .prox_thres_high = 512,
>   .prox_max_samples_cal = 30,
> @@ -686,37 +687,22 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>   /* Power-on settling time */
>   usleep_range(3000, 3500);
>  
> - /*
> -  * NOW enable the ADC
> -  * initialize the desired mode of operation
> -  */
> - ret = tsl2x7x_write_control_reg(chip,
> - TSL2X7X_CNTL_PWR_ON |
> - TSL2X7X_CNTL_ADC_ENBL |
> - TSL2X7X_CNTL_PROX_DET_ENBL);
> + reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL |
> +   TSL2X7X_CNTL_PROX_DET_ENBL;
> + if (chip->settings.als_interrupt_en)
> + reg_val |= TSL2X7X_CNTL_ALS_INT_ENBL;
> + if (chip->settings.prox_interrupt_en)
> + reg_val |= TSL2X7X_CNTL_PROX_INT_ENBL;
> +
> + ret = tsl2x7x_write_control_reg(chip, reg_val);
>   if (ret < 0)
>   return ret;
>  
> - chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
> -
> - if (chip->settings.interrupts_en != 0) {
> - dev_info(>client->dev, "Setting Up Interrupt(s)\n");
> -
> - reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
> - if (chip->settings.interrupts_en == 0x20 ||
> - chip->settings.interrupts_en == 0x30)
> - reg_val |= TSL2X7X_CNTL_PROX_DET_ENBL;
> -
> - reg_val |= chip->settings.interrupts_en;
> - ret = tsl2x7x_write_control_reg(chip, reg_val);
> - if (ret < 0)
> - return ret;
> + ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
> + if (ret < 0)
> + return ret;
>  
> - ret = tsl2x7x_clear_interrupts(chip,
> -TSL2X7X_CMD_PROXALS_INT_CLR);
> - if (ret < 0)
> - return ret;
> - }
> + chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
>  
>   return ret;
>  }
> @@ -978,14 +964,11 @@ static int tsl2x7x_read_interrupt_config(struct iio_dev 
> *indio_dev,
>enum iio_event_direction dir)
>  {
>   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> - int ret;
>  
>   if (chan->type == IIO_INTENSITY)
> - ret = !!(chip->settings.interrupts_en & 0x10);
> + return chip->settings.als_interrupt_en;
>   else
> - ret = !!(chip->settings.interrupts_en & 0x20);
> -
> - return ret;
> + return chip->settings.prox_interrupt_en;
>  }
>  
>  static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
> @@ -997,17 +980,10 @@ static int tsl2x7x_write_interrupt_config(struct 
> iio_dev *indio_dev,
>   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>   int ret;
>  
> - if (chan->type == IIO_INTENSITY) {
> - if (val)
> - chip->settings.interrupts_en |= 0x10;
> - else
> - chip->settings.interrupts_en &= 0x20;
> - } else {
> - if (val)
> - chip->settings.interrupts_en |= 0x20;
> - else
> - chip->settings.interrupts_en &= 0x10;
> - }
> + if (chan->type == IIO_INTENSITY)
> + chip->settings.als_interrupt_en = val ? true : false;
> + else
> + 

Re: [PATCH 04/11] staging: iio: tsl2x7x: simplify tsl2x7x_prox_cal()

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:05 -0400
Brian Masney  wrote:

> tsl2x7x_prox_cal() would set the interrupt flag, and reset the device to
> start doing the calibration routine. However, this did not actually
> affect the readings since they are polled. This patch drops the interrupt
> code.
> 
> This patch also drops the function tsl2x7x_prox_calculate() and removes
> support for the standard deviation and min sample since those values
> were not used.
> 
> Driver was tested using a TSL2772 hooked up to a Raspberry Pi 2. I
> performed the following testing at various distances:
> 
> - Put hand in front of sensor and keep the sensor and hand stationary.
> - Perform calibration routine.
> - Run iio_event_monitor.
> - Verify that a proximity event is triggered when my hand comes
>   anywhere between the sensor and where I performed the calibration
>   routine.
> 
> Signed-off-by: Brian Masney 
Applied.  Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 107 
> +---
>  1 file changed, 15 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 9c929e273135..99230d9313e1 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -149,13 +149,6 @@ struct tsl2x7x_als_info {
>   u16 lux;
>  };
>  
> -struct tsl2x7x_prox_stat {
> - int min;
> - int max;
> - int mean;
> - unsigned long stddev;
> -};
> -
>  struct tsl2x7x_chip_info {
>   int chan_table_elements;
>   struct iio_chan_specchannel[4];
> @@ -771,106 +764,36 @@ static int tsl2x7x_invoke_change(struct iio_dev 
> *indio_dev)
>   return ret;
>  }
>  
> -static void tsl2x7x_prox_calculate(int *data, int length,
> -struct tsl2x7x_prox_stat *stat)
> -{
> - int i;
> - int sample_sum;
> - int tmp;
> -
> - if (!length)
> - length = 1;
> -
> - sample_sum = 0;
> - stat->min = INT_MAX;
> - stat->max = INT_MIN;
> - for (i = 0; i < length; i++) {
> - sample_sum += data[i];
> - stat->min = min(stat->min, data[i]);
> - stat->max = max(stat->max, data[i]);
> - }
> -
> - stat->mean = sample_sum / length;
> - sample_sum = 0;
> - for (i = 0; i < length; i++) {
> - tmp = data[i] - stat->mean;
> - sample_sum += tmp * tmp;
> - }
> - stat->stddev = int_sqrt((long)sample_sum / length);
> -}
> -
> -/**
> - * tsl2x7x_prox_cal() - Calculates std. and sets thresholds.
> - * @indio_dev:   pointer to IIO device
> - *
> - * Calculates a standard deviation based on the samples,
> - * and sets the threshold accordingly.
> - */
>  static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  {
> - int prox_history[MAX_SAMPLES_CAL + 1];
> - int i, ret;
> - struct tsl2x7x_prox_stat prox_stat_data[2];
> - struct tsl2x7x_prox_stat *cal;
>   struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> - u8 tmp_irq_settings;
> - u8 current_state = chip->tsl2x7x_chip_status;
> -
> - if (chip->settings.prox_max_samples_cal > MAX_SAMPLES_CAL) {
> - dev_err(>client->dev,
> - "max prox samples cal is too big: %d\n",
> - chip->settings.prox_max_samples_cal);
> - chip->settings.prox_max_samples_cal = MAX_SAMPLES_CAL;
> - }
> -
> - /* have to stop to change settings */
> - ret = tsl2x7x_chip_off(indio_dev);
> - if (ret < 0)
> - return ret;
> -
> - /* Enable proximity detection save just in case prox not wanted yet*/
> - tmp_irq_settings = chip->settings.interrupts_en;
> - chip->settings.interrupts_en |= TSL2X7X_CNTL_PROX_INT_ENBL;
> + int prox_history[MAX_SAMPLES_CAL + 1];
> + int i, ret, mean, max, sample_sum;
>  
> - /*turn on device if not already on*/
> - ret = tsl2x7x_chip_on(indio_dev);
> - if (ret < 0)
> - return ret;
> + if (chip->settings.prox_max_samples_cal < 1 ||
> + chip->settings.prox_max_samples_cal > MAX_SAMPLES_CAL)
> + return -EINVAL;
>  
> - /*gather the samples*/
>   for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
>   usleep_range(15000, 17500);
>   ret = tsl2x7x_get_prox(indio_dev);
>   if (ret < 0)
>   return ret;
> +
>   prox_history[i] = chip->prox_data;
> - dev_info(>client->dev, "2 i=%d prox data= %d\n",
> -  i, chip->prox_data);
>   }
>  
> - ret = tsl2x7x_chip_off(indio_dev);
> - if (ret < 0)
> - return ret;
> - cal = _stat_data[PROX_STAT_CAL];
> - tsl2x7x_prox_calculate(prox_history,
> -chip->settings.prox_max_samples_cal, cal);
> - chip->settings.prox_thres_high = (cal->max << 1) - cal->mean;
> -
> - 

Re: [PATCH 03/11] staging: iio: tsl2x7x: no need to clear interrupt flag when getting lux

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:04 -0400
Brian Masney  wrote:

> tsl2x7x_get_lux() does not need to clear the interrupt flag when
> querying the ALS. The interrupt flag is cleared in
> tsl2x7x_event_handler(). This patches removes the unnecessary code.
> 
> Signed-off-by: Brian Masney 
Applied.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 59921850a226..9c929e273135 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -387,10 +387,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>   buf[i] = ret;
>   }
>  
> - ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_ALS_INT_CLR);
> - if (ret < 0)
> - goto out_unlock;
> -
>   /* extract ALS/lux data */
>   ch0 = le16_to_cpup((const __le16 *)[0]);
>   ch1 = le16_to_cpup((const __le16 *)[2]);

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

Re: [PATCH 02/11] staging: iio: tsl2x7x: correct interrupt handler trigger

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:03 -0400
Brian Masney  wrote:

> tsl2x7x_event_handler() was not called as expected when the device was
> asserting a hardware interrupt. This patch changes the interrupt line
> trigger from rising to falling.
I guess the original test board used for driver development must have
inverted this for some reason and hence was miss configured.

Anyhow, good catch.
Applied to the togreg branch of iio.git.

Thanks,

Jonathan

> 
> The driver was tested on a TSL2772 hooked up to a Raspberry Pi 2. The
> interrupt pin also had a 10K pull-up resistor per the requirements from
> the datasheet. The relevant device tree binding:
> 
>  {
>   tsl2772@39 {
>   compatible = "amstaos,tsl2772";
>   reg = <0x39>;
>   interrupt-parent = <>;
>   interrupts = <22 0x2>;
>   };
> };
> 
> With this patch, iio_event_monitor now shows the events when the
> channels are outside the defined interrupt thresholds.
> 
> $ sudo ./iio_event_monitor tsl2772
> Found IIO device with name tsl2772 with device number 0
> Event: time: 1478193460053760446, type: proximity, channel: 0, evtype:
> thresh, direction: either
> ...
> Event: time: 1478193463020270185, type: illuminance, channel: 0, evtype:
> thresh, direction: either
> ...
> 
> Signed-off-by: Brian Masney 
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 82cf9d853b18..59921850a226 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1763,7 +1763,7 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>   ret = devm_request_threaded_irq(>dev, clientp->irq,
>   NULL,
>   _event_handler,
> - IRQF_TRIGGER_RISING |
> + IRQF_TRIGGER_FALLING |
>   IRQF_ONESHOT,
>   "TSL2X7X_event",
>   indio_dev);

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

Re: [PATCH 01/11] staging: iio: tsl2x7x: remove unnecessary code

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:02 -0400
Brian Masney  wrote:

> As a follow up to the work in commit a0722d05a195 ("staging: iio:
> tsl2x7x: convert mutex_trylock() to mutex_lock()"), this patch removes
> the unnecessary calls to tsl2x7x_get_prox() and tsl2x7x_get_lux() in
> tsl2x7x_event_handler(). Previously, these functions were locked with
> mutex_trylock(), but that is no longer the case. This patch also removes
> a comment that is no longer relevant about returning the last sample.
> 
> Signed-off-by: Brian Masney 
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c 
> b/drivers/staging/iio/light/tsl2x7x.c
> index 82681300e106..82cf9d853b18 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1430,7 +1430,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void 
> *private)
>  
>   /* What type of interrupt do we need to process */
>   if (ret & TSL2X7X_STA_PRX_INTR) {
> - tsl2x7x_get_prox(indio_dev); /* freshen data for ABI */
>   iio_push_event(indio_dev,
>  IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY,
>   0,
> @@ -1440,7 +1439,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void 
> *private)
>   }
>  
>   if (ret & TSL2X7X_STA_ALS_INTR) {
> - tsl2x7x_get_lux(indio_dev); /* freshen data for ABI */
>   iio_push_event(indio_dev,
>  IIO_UNMOD_EVENT_CODE(IIO_LIGHT,
>   0,
> @@ -1745,10 +1743,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>   return ret;
>   }
>  
> - /*
> -  * ALS and PROX functions can be invoked via user space poll
> -  * or H/W interrupt. If busy return last sample.
> -  */
>   mutex_init(>als_mutex);
>   mutex_init(>prox_mutex);
>  

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

Re: [PATCH v3 8/8] staging:iio:ade7854: Remove read_reg_* duplications

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:27:27 -0300
Rodrigo Siqueira  wrote:

> The original code had a read function per data size; after updates, all
> read functions tasks were centralized in a single function, but the old
> signature was kept to maintain the module working without problems. This
> patch removes a set of duplications associated with read_reg_*, and
> update the areas that calling the old interface by the new one.
> 
> Signed-off-by: Rodrigo Siqueira 
Applied.  Note these probably won't go upstream until after the merge
window closes.  It's just possible Linus will decide to do an rc8, in which
case I might do another pull request to Greg, but probably not.

Jonathan

> ---
> Changes in v3:
>  - Update commit message
> 
>  drivers/staging/iio/meter/ade7854-i2c.c | 33 +--
>  drivers/staging/iio/meter/ade7854-spi.c | 35 
> ++---
>  drivers/staging/iio/meter/ade7854.c | 18 -
>  drivers/staging/iio/meter/ade7854.h |  7 +++
>  4 files changed, 15 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 63793f9664c7..c3aa6ea9d036 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -110,34 +110,6 @@ static int ade7854_i2c_read_reg(struct device *dev,
>   return ret;
>  }
>  
> -static int ade7854_i2c_read_reg_8(struct device *dev,
> -   u16 reg_address,
> -   u8 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val, 8);
> -}
> -
> -static int ade7854_i2c_read_reg_16(struct device *dev,
> -u16 reg_address,
> -u16 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val, 16);
> -}
> -
> -static int ade7854_i2c_read_reg_24(struct device *dev,
> -u16 reg_address,
> -u32 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val, 24);
> -}
> -
> -static int ade7854_i2c_read_reg_32(struct device *dev,
> -u16 reg_address,
> -u32 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val, 32);
> -}
> -
>  static int ade7854_i2c_probe(struct i2c_client *client,
>const struct i2c_device_id *id)
>  {
> @@ -149,10 +121,7 @@ static int ade7854_i2c_probe(struct i2c_client *client,
>   return -ENOMEM;
>   st = iio_priv(indio_dev);
>   i2c_set_clientdata(client, indio_dev);
> - st->read_reg_8 = ade7854_i2c_read_reg_8;
> - st->read_reg_16 = ade7854_i2c_read_reg_16;
> - st->read_reg_24 = ade7854_i2c_read_reg_24;
> - st->read_reg_32 = ade7854_i2c_read_reg_32;
> + st->read_reg = ade7854_i2c_read_reg;
>   st->write_reg = ade7854_i2c_write_reg;
>   st->i2c = client;
>   st->irq = client->irq;
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index ee6e4d166ece..fc9146757283 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -94,7 +94,7 @@ static int ade7854_spi_read_reg(struct device *dev,
>   st->tx[2] = reg_address & 0xFF;
>  
>   ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> - if (ret) {
> + if (ret < 0) {
>   dev_err(>spi->dev, "problem when reading register 0x%02X",
>   reg_address);
>   goto unlock;
> @@ -120,34 +120,6 @@ static int ade7854_spi_read_reg(struct device *dev,
>   return ret;
>  }
>  
> -static int ade7854_spi_read_reg_8(struct device *dev,
> -   u16 reg_address,
> -   u8 *val)
> -{
> - return ade7854_spi_read_reg(dev, reg_address, (u32 *)val, 8);
> -}
> -
> -static int ade7854_spi_read_reg_16(struct device *dev,
> -u16 reg_address,
> -u16 *val)
> -{
> - return ade7854_spi_read_reg(dev, reg_address, (u32 *)val, 16);
> -}
> -
> -static int ade7854_spi_read_reg_24(struct device *dev,
> -u16 reg_address,
> -u32 *val)
> -{
> - return ade7854_spi_read_reg(dev, reg_address, (u32 *)val, 24);
> -}
> -
> -static int ade7854_spi_read_reg_32(struct device *dev,
> -u16 reg_address,
> -u32 *val)
> -{
> - return ade7854_spi_read_reg(dev, reg_address, (u32 *)val, 32);
> -}
> -
>  static int ade7854_spi_probe(struct spi_device *spi)
>  {
>   struct ade7854_state *st;
> @@ -158,10 +130,7 @@ static int ade7854_spi_probe(struct spi_device *spi)
>   return -ENOMEM;
>   st = 

Re: [PATCH v3 7/8] staging:iio:ade7854: Rework SPI read function

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:27:12 -0300
Rodrigo Siqueira  wrote:

> Rework read SPI function to reduce the code duplication and centralizes
> all the task in a single function.
> 
> Signed-off-by: Rodrigo Siqueira 
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v3:
>  - Remove the use of defines that not improve the readability
>  - Replace variable name "bytes" for "bits"
>  - Update commit message
> 
>  drivers/staging/iio/meter/ade7854-spi.c | 136 
> 
>  1 file changed, 33 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index be7397042850..ee6e4d166ece 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -67,9 +67,10 @@ static int ade7854_spi_write_reg(struct device *dev,
>   return ret;
>  }
>  
> -static int ade7854_spi_read_reg_8(struct device *dev,
> -   u16 reg_address,
> -   u8 *val)
> +static int ade7854_spi_read_reg(struct device *dev,
> + u16 reg_address,
> + u32 *val,
> + int bits)
>  {
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   struct ade7854_state *st = iio_priv(indio_dev);
> @@ -82,7 +83,7 @@ static int ade7854_spi_read_reg_8(struct device *dev,
>   }, {
>   .rx_buf = st->rx,
>   .bits_per_word = 8,
> - .len = 1,
> + .len = bits,
>   }
>   };
>  
> @@ -94,128 +95,57 @@ static int ade7854_spi_read_reg_8(struct device *dev,
>  
>   ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
>   if (ret) {
> - dev_err(>spi->dev, "problem when reading 8 bit register 
> 0x%02X",
> + dev_err(>spi->dev, "problem when reading register 0x%02X",
>   reg_address);
> - goto error_ret;
> + goto unlock;
> + }
> +
> + switch (bits) {
> + case 8:
> + *val = st->rx[0];
> + break;
> + case 16:
> + *val = be16_to_cpup((const __be16 *)st->rx);
> + break;
> + case 24:
> + *val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
> + break;
> + case 32:
> + *val = be32_to_cpup((const __be32 *)st->rx);
> + break;
>   }
> - *val = st->rx[0];
>  
> -error_ret:
> +unlock:
>   mutex_unlock(>buf_lock);
>   return ret;
>  }
>  
> +static int ade7854_spi_read_reg_8(struct device *dev,
> +   u16 reg_address,
> +   u8 *val)
> +{
> + return ade7854_spi_read_reg(dev, reg_address, (u32 *)val, 8);
> +}
> +
>  static int ade7854_spi_read_reg_16(struct device *dev,
>  u16 reg_address,
>  u16 *val)
>  {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - int ret;
> - struct spi_transfer xfers[] = {
> - {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len = 3,
> - }, {
> - .rx_buf = st->rx,
> - .bits_per_word = 8,
> - .len = 2,
> - }
> - };
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = ADE7854_READ_REG;
> - st->tx[1] = (reg_address >> 8) & 0xFF;
> - st->tx[2] = reg_address & 0xFF;
> -
> - ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> - if (ret) {
> - dev_err(>spi->dev, "problem when reading 16 bit register 
> 0x%02X",
> - reg_address);
> - goto error_ret;
> - }
> - *val = be16_to_cpup((const __be16 *)st->rx);
> -
> -error_ret:
> - mutex_unlock(>buf_lock);
> - return ret;
> + return ade7854_spi_read_reg(dev, reg_address, (u32 *)val, 16);
>  }
>  
>  static int ade7854_spi_read_reg_24(struct device *dev,
>  u16 reg_address,
>  u32 *val)
>  {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - int ret;
> - struct spi_transfer xfers[] = {
> - {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len = 3,
> - }, {
> - .rx_buf = st->rx,
> - .bits_per_word = 8,
> - .len = 3,
> - }
> - };
> -
> - mutex_lock(>buf_lock);
> -
> - st->tx[0] = ADE7854_READ_REG;
> - st->tx[1] = 

Re: [PATCH v3 6/8] staging:iio:ade7854: Rework I2C read function

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:26:57 -0300
Rodrigo Siqueira  wrote:

> The read operation for the I2C function has many duplications that can
> be generalized into a single function. This patch reworks the read
> operation for I2C to centralizes all similar code in a single function.
> 
> It is possible to remove all the old interface to use the new one,
> however, for keeping the things simple and working this patch maintain
> legacy interface.
> 
> Signed-off-by: Rodrigo Siqueira 
Applied, thanks.

Jonathan

> ---
> Changes in v3:
>  - Remove the use of defines that not improve the readability
>  - Replace variable name "bytes" for "bits"
>  - Update commit message
> 
>  drivers/staging/iio/meter/ade7854-i2c.c | 106 
> +++-
>  1 file changed, 37 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 29e959fdb932..63793f9664c7 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -65,9 +65,10 @@ static int ade7854_i2c_write_reg(struct device *dev,
>   return ret < 0 ? ret : 0;
>  }
>  
> -static int ade7854_i2c_read_reg_8(struct device *dev,
> -   u16 reg_address,
> -   u8 *val)
> +static int ade7854_i2c_read_reg(struct device *dev,
> + u16 reg_address,
> + u32 *val,
> + int bits)
>  {
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   struct ade7854_state *st = iio_priv(indio_dev);
> @@ -79,95 +80,62 @@ static int ade7854_i2c_read_reg_8(struct device *dev,
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
>   if (ret < 0)
> - goto out;
> + goto unlock;
>  
> - ret = i2c_master_recv(st->i2c, st->rx, 1);
> + ret = i2c_master_recv(st->i2c, st->rx, bits);
>   if (ret < 0)
> - goto out;
> + goto unlock;
> +
> + switch (bits) {
> + case 8:
> + *val = st->rx[0];
> + break;
> + case 16:
> + *val = (st->rx[0] << 8) | st->rx[1];
> + break;
> + case 24:
> + *val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
> + break;
> + case 32:
> + *val = (st->rx[0] << 24) | (st->rx[1] << 16) |
> + (st->rx[2] << 8) | st->rx[3];
> + break;
> + default:
> + ret = -EINVAL;
> + goto unlock;
> + }
>  
> - *val = st->rx[0];
> -out:
> +unlock:
>   mutex_unlock(>buf_lock);
>   return ret;
>  }
>  
> +static int ade7854_i2c_read_reg_8(struct device *dev,
> +   u16 reg_address,
> +   u8 *val)
> +{
> + return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val, 8);
> +}
> +
>  static int ade7854_i2c_read_reg_16(struct device *dev,
>  u16 reg_address,
>  u16 *val)
>  {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret < 0)
> - goto out;
> -
> - ret = i2c_master_recv(st->i2c, st->rx, 2);
> - if (ret < 0)
> - goto out;
> -
> - *val = (st->rx[0] << 8) | st->rx[1];
> -out:
> - mutex_unlock(>buf_lock);
> - return ret;
> + return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val, 16);
>  }
>  
>  static int ade7854_i2c_read_reg_24(struct device *dev,
>  u16 reg_address,
>  u32 *val)
>  {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret < 0)
> - goto out;
> -
> - ret = i2c_master_recv(st->i2c, st->rx, 3);
> - if (ret < 0)
> - goto out;
> -
> - *val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
> -out:
> - mutex_unlock(>buf_lock);
> - return ret;
> + return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val, 24);
>  }
>  
>  static int ade7854_i2c_read_reg_32(struct device *dev,
>  u16 reg_address,
>  u32 *val)
>  {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = 

Re: [PATCH v3 5/8] staging:iio:ade7854: Remove write_reg_* duplications

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:26:41 -0300
Rodrigo Siqueira  wrote:

> This patch removes code duplications related to the write_reg_*
> functions and centralizes them in a single function. Also, it eliminates
> the legacy functions and replaces them by a unique signature that is
> used by SPI and I2C.
> 
> Signed-off-by: Rodrigo Siqueira 
Applied, thanks.

Jonathan

> ---
> Changes in v3:
>  - Update commit message
> 
>  drivers/staging/iio/meter/ade7854-i2c.c | 33 
> +
>  drivers/staging/iio/meter/ade7854-spi.c | 33 
> +
>  drivers/staging/iio/meter/ade7854.c | 12 ++--
>  drivers/staging/iio/meter/ade7854.h |  9 -
>  4 files changed, 12 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index c9f46d26b752..29e959fdb932 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -65,34 +65,6 @@ static int ade7854_i2c_write_reg(struct device *dev,
>   return ret < 0 ? ret : 0;
>  }
>  
> -static int ade7854_i2c_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, 8);
> -}
> -
> -static int ade7854_i2c_write_reg_16(struct device *dev,
> - u16 reg_address,
> - u16 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, 16);
> -}
> -
> -static int ade7854_i2c_write_reg_24(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, 24);
> -}
> -
> -static int ade7854_i2c_write_reg_32(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, 32);
> -}
> -
>  static int ade7854_i2c_read_reg_8(struct device *dev,
> u16 reg_address,
> u8 *val)
> @@ -213,10 +185,7 @@ static int ade7854_i2c_probe(struct i2c_client *client,
>   st->read_reg_16 = ade7854_i2c_read_reg_16;
>   st->read_reg_24 = ade7854_i2c_read_reg_24;
>   st->read_reg_32 = ade7854_i2c_read_reg_32;
> - st->write_reg_8 = ade7854_i2c_write_reg_8;
> - st->write_reg_16 = ade7854_i2c_write_reg_16;
> - st->write_reg_24 = ade7854_i2c_write_reg_24;
> - st->write_reg_32 = ade7854_i2c_write_reg_32;
> + st->write_reg = ade7854_i2c_write_reg;
>   st->i2c = client;
>   st->irq = client->irq;
>  
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index 9c5c16c4d6e0..be7397042850 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -67,34 +67,6 @@ static int ade7854_spi_write_reg(struct device *dev,
>   return ret;
>  }
>  
> -static int ade7854_spi_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, 8);
> -}
> -
> -static int ade7854_spi_write_reg_16(struct device *dev,
> - u16 reg_address,
> - u16 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, 16);
> -}
> -
> -static int ade7854_spi_write_reg_24(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, 24);
> -}
> -
> -static int ade7854_spi_write_reg_32(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, 32);
> -}
> -
>  static int ade7854_spi_read_reg_8(struct device *dev,
> u16 reg_address,
> u8 *val)
> @@ -260,10 +232,7 @@ static int ade7854_spi_probe(struct spi_device *spi)
>   st->read_reg_16 = ade7854_spi_read_reg_16;
>   st->read_reg_24 = ade7854_spi_read_reg_24;
>   st->read_reg_32 = ade7854_spi_read_reg_32;
> - st->write_reg_8 = ade7854_spi_write_reg_8;
> - st->write_reg_16 = ade7854_spi_write_reg_16;
> - st->write_reg_24 = ade7854_spi_write_reg_24;
> - st->write_reg_32 = ade7854_spi_write_reg_32;
> + st->write_reg = ade7854_spi_write_reg;
>   st->irq = spi->irq;
>   st->spi = spi;
>  
> diff --git a/drivers/staging/iio/meter/ade7854.c 
> b/drivers/staging/iio/meter/ade7854.c
> index 0193ae3aae29..45984b9a2bee 100644
> --- a/drivers/staging/iio/meter/ade7854.c
> +++ 

Re: [PATCH v3 4/8] staging:iio:ade7854: Rework SPI write function

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:26:25 -0300
Rodrigo Siqueira  wrote:

> The write operation using SPI has a many code duplications (similar to
> I2C) and four different interfaces per data size. This patch introduces
> a single function that centralizes the main task related to SPI.
> 
> Signed-off-by: Rodrigo Siqueira 
Applied.

Thanks,

Jonathan

> ---
> Changes in v3:
>  - Remove the use of defines that not improve the readability
>  - Replace variable name "bytes" for "bits"
> 
>  drivers/staging/iio/meter/ade7854-spi.c | 108 
> 
>  1 file changed, 41 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index 4419b8f06197..9c5c16c4d6e0 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -15,9 +15,10 @@
>  #include 
>  #include "ade7854.h"
>  
> -static int ade7854_spi_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> +static int ade7854_spi_write_reg(struct device *dev,
> +  u16 reg_address,
> +  u32 val,
> +  int bits)
>  {
>   int ret;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -32,93 +33,66 @@ static int ade7854_spi_write_reg_8(struct device *dev,
>   st->tx[0] = ADE7854_WRITE_REG;
>   st->tx[1] = (reg_address >> 8) & 0xFF;
>   st->tx[2] = reg_address & 0xFF;
> - st->tx[3] = val & 0xFF;
> + switch (bits) {
> + case 8:
> + st->tx[3] = val & 0xFF;
> + break;
> + case 16:
> + xfer.len = 5;
> + st->tx[3] = (val >> 8) & 0xFF;
> + st->tx[4] = val & 0xFF;
> + break;
> + case 24:
> + xfer.len = 6;
> + st->tx[3] = (val >> 16) & 0xFF;
> + st->tx[4] = (val >> 8) & 0xFF;
> + st->tx[5] = val & 0xFF;
> + break;
> + case 32:
> + xfer.len = 7;
> + st->tx[3] = (val >> 24) & 0xFF;
> + st->tx[4] = (val >> 16) & 0xFF;
> + st->tx[5] = (val >> 8) & 0xFF;
> + st->tx[6] = val & 0xFF;
> + break;
> + default:
> + ret = -EINVAL;
> + goto unlock;
> + }
>  
>   ret = spi_sync_transfer(st->spi, , 1);
> +unlock:
>   mutex_unlock(>buf_lock);
>  
>   return ret;
>  }
>  
> +static int ade7854_spi_write_reg_8(struct device *dev,
> +u16 reg_address,
> +u8 val)
> +{
> + return ade7854_spi_write_reg(dev, reg_address, val, 8);
> +}
> +
>  static int ade7854_spi_write_reg_16(struct device *dev,
>   u16 reg_address,
>   u16 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - struct spi_transfer xfer = {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len = 5,
> - };
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = ADE7854_WRITE_REG;
> - st->tx[1] = (reg_address >> 8) & 0xFF;
> - st->tx[2] = reg_address & 0xFF;
> - st->tx[3] = (val >> 8) & 0xFF;
> - st->tx[4] = val & 0xFF;
> -
> - ret = spi_sync_transfer(st->spi, , 1);
> - mutex_unlock(>buf_lock);
> -
> - return ret;
> + return ade7854_spi_write_reg(dev, reg_address, val, 16);
>  }
>  
>  static int ade7854_spi_write_reg_24(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - struct spi_transfer xfer = {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len = 6,
> - };
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = ADE7854_WRITE_REG;
> - st->tx[1] = (reg_address >> 8) & 0xFF;
> - st->tx[2] = reg_address & 0xFF;
> - st->tx[3] = (val >> 16) & 0xFF;
> - st->tx[4] = (val >> 8) & 0xFF;
> - st->tx[5] = val & 0xFF;
> -
> - ret = spi_sync_transfer(st->spi, , 1);
> - mutex_unlock(>buf_lock);
> -
> - return ret;
> + return ade7854_spi_write_reg(dev, reg_address, val, 24);
>  }
>  
>  static int ade7854_spi_write_reg_32(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - struct spi_transfer xfer = {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len = 7,
> - };
> -
> - 

Re: [PATCH v3 3/8] staging:iio:ade7854: Rework I2C write function

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:26:06 -0300
Rodrigo Siqueira  wrote:

> The write operation using I2C has many code duplications and four
> different interfaces per data size. This patch introduces a single
> function that centralizes the main tasks.
> 
> The central function inserted by this patch can easily replace all the
> four functions related to the data size. However, this patch does not
> remove any code signature for keeping the meter module work and make
> easier to review this patch.
> 
> Signed-off-by: Rodrigo Siqueira 
Applied.

Thanks,

Jonathan

> ---
> Changes in v3:
>  - Remove the use of defines that not improve the readability
>  - Replace variable name "bytes" for "bits"
> 
>  drivers/staging/iio/meter/ade7854-i2c.c | 96 
> -
>  1 file changed, 46 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 37c957482493..c9f46d26b752 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -15,86 +15,82 @@
>  #include 
>  #include "ade7854.h"
>  
> -static int ade7854_i2c_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> +static int ade7854_i2c_write_reg(struct device *dev,
> +  u16 reg_address,
> +  u32 val,
> +  int bits)
>  {
>   int ret;
> + int count;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   struct ade7854_state *st = iio_priv(indio_dev);
>  
>   mutex_lock(>buf_lock);
>   st->tx[0] = (reg_address >> 8) & 0xFF;
>   st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = val;
>  
> - ret = i2c_master_send(st->i2c, st->tx, 3);
> + switch (bits) {
> + case 8:
> + st->tx[2] = val & 0xFF;
> + count = 3;
> + break;
> + case 16:
> + st->tx[2] = (val >> 8) & 0xFF;
> + st->tx[3] = val & 0xFF;
> + count = 4;
> + break;
> + case 24:
> + st->tx[2] = (val >> 16) & 0xFF;
> + st->tx[3] = (val >> 8) & 0xFF;
> + st->tx[4] = val & 0xFF;
> + count = 5;
> + break;
> + case 32:
> + st->tx[2] = (val >> 24) & 0xFF;
> + st->tx[3] = (val >> 16) & 0xFF;
> + st->tx[4] = (val >> 8) & 0xFF;
> + st->tx[5] = val & 0xFF;
> + count = 6;
> + break;
> + default:
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + ret = i2c_master_send(st->i2c, st->tx, count);
> +
> +unlock:
>   mutex_unlock(>buf_lock);
>  
>   return ret < 0 ? ret : 0;
>  }
>  
> +static int ade7854_i2c_write_reg_8(struct device *dev,
> +u16 reg_address,
> +u8 val)
> +{
> + return ade7854_i2c_write_reg(dev, reg_address, val, 8);
> +}
> +
>  static int ade7854_i2c_write_reg_16(struct device *dev,
>   u16 reg_address,
>   u16 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = (val >> 8) & 0xFF;
> - st->tx[3] = val & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 4);
> - mutex_unlock(>buf_lock);
> -
> - return ret < 0 ? ret : 0;
> + return ade7854_i2c_write_reg(dev, reg_address, val, 16);
>  }
>  
>  static int ade7854_i2c_write_reg_24(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = (val >> 16) & 0xFF;
> - st->tx[3] = (val >> 8) & 0xFF;
> - st->tx[4] = val & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 5);
> - mutex_unlock(>buf_lock);
> -
> - return ret < 0 ? ret : 0;
> + return ade7854_i2c_write_reg(dev, reg_address, val, 24);
>  }
>  
>  static int ade7854_i2c_write_reg_32(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = (val >> 24) & 0xFF;
> - st->tx[3] = (val >> 

Re: [PATCH v3 2/8] staging:iio:ade7854: Fix the wrong number of bits to read

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:25:48 -0300
John Syne  wrote:

> Fixes: correctly handle the data size in the read operation for I2C
> 
> The function ade7854_i2c_read_reg_32() have to invoke the
> i2c_master_recv() for read 32 bits values, however, the counter is set
> to 3 which means 24 bits. This patch fixes the wrong size of 24 bits, to
> 32 bits.
> 
> Signed-off-by: John Syne 
> Signed-off-by: Rodrigo Siqueira 
Applied with fixed 'fixes' tag.

Jonathan

> ---
> Changes in v3:
>  - Clarify authorship
>  - Add Fixes tag in the commit message
> 
>  drivers/staging/iio/meter/ade7854-i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 4437f1e33261..37c957482493 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -191,7 +191,7 @@ static int ade7854_i2c_read_reg_32(struct device *dev,
>   if (ret < 0)
>   goto out;
>  
> - ret = i2c_master_recv(st->i2c, st->rx, 3);
> + ret = i2c_master_recv(st->i2c, st->rx, 4);
>   if (ret < 0)
>   goto out;
>  

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

Re: [PATCH v3 1/8] staging:iio:ade7854: Fix error handling on read/write

[Jonathan Cameron]

On Fri, 23 Mar 2018 11:22:10 -0300
John Syne  wrote:

> Fixes: correctly handle errors on the read and write operation for I2C
Please look at the Submitting patches documentation.  This is not
what a fixes tag is about!  I'll fix it up this time but please
look at it.
> 
> The original code does not correctly handle the error related to I2C
> read and write. This patch fixes the error handling related to all
> read/write functions for I2C.
> 
> Signed-off-by: John Syne 
> Signed-off-by: Rodrigo Siqueira 
Given this has been broken a long time I'm not going to push
this for stable.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v3:
>  - Clarify authorship
>  - Add Fixes tag in the commit message
> 
>  drivers/staging/iio/meter/ade7854-i2c.c | 24 
>  drivers/staging/iio/meter/ade7854.c | 10 +-
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 317e4f0d8176..4437f1e33261 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -31,7 +31,7 @@ static int ade7854_i2c_write_reg_8(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 3);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
>  
>  static int ade7854_i2c_write_reg_16(struct device *dev,
> @@ -51,7 +51,7 @@ static int ade7854_i2c_write_reg_16(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 4);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
>  
>  static int ade7854_i2c_write_reg_24(struct device *dev,
> @@ -72,7 +72,7 @@ static int ade7854_i2c_write_reg_24(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 5);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
>  
>  static int ade7854_i2c_write_reg_32(struct device *dev,
> @@ -94,7 +94,7 @@ static int ade7854_i2c_write_reg_32(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 6);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
>  
>  static int ade7854_i2c_read_reg_8(struct device *dev,
> @@ -110,11 +110,11 @@ static int ade7854_i2c_read_reg_8(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 1);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = st->rx[0];
> @@ -136,11 +136,11 @@ static int ade7854_i2c_read_reg_16(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = (st->rx[0] << 8) | st->rx[1];
> @@ -162,11 +162,11 @@ static int ade7854_i2c_read_reg_24(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 3);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
> @@ -188,11 +188,11 @@ static int ade7854_i2c_read_reg_32(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 3);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = (st->rx[0] << 24) | (st->rx[1] << 16) |
> diff --git a/drivers/staging/iio/meter/ade7854.c 
> b/drivers/staging/iio/meter/ade7854.c
> index 90d07cdca4b8..0193ae3aae29 100644
> --- a/drivers/staging/iio/meter/ade7854.c
> +++ b/drivers/staging/iio/meter/ade7854.c
> @@ -33,7 +33,7 @@ static ssize_t ade7854_read_8bit(struct device *dev,
>   struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>   ret = st->read_reg_8(dev, this_attr->address, );
> - if (ret)
> + if (ret < 0)
>   return ret;
>  
>   return sprintf(buf, "%u\n", val);
> @@ -50,7 +50,7 @@ static ssize_t ade7854_read_16bit(struct device *dev,
>   struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>   ret = st->read_reg_16(dev, this_attr->address, );
> - if (ret)
> + if (ret < 0)
>   return ret;
>  
>   return sprintf(buf, "%u\n", val);
> @@ -67,7 +67,7 @@ static ssize_t ade7854_read_24bit(struct 

Re: [PATCH 10/11] staging: iio: ad2s1200: Replace angle channel with inclination channel

[David Julian Veenstra]

On 23, March 2018 14:27, Jonathan Cameron wrote:

> On Sun, 18 Mar 2018 14:37:04 +0100
> David Veenstra  wrote:
>
>> The angle channel is not defined in sysfs iio ABI. So it is replaced
>> with an inclination channel, because it is defined in the ABI, and has the
>> semantics of an angle.
>> 
>> In addition, a fractional scaling factor of 360 / (2^12 -1) is added,
>> to scale the 12-bits angular position to degrees, conform the ABI.
>> 
>> Signed-off-by: David Veenstra 
> No, please do not blugeon a device into the existing documented ABI.
>
> A resolver does not measure something that can be remotely
> sensibly mapped to inclination.  Inclination is relative to 'down'.
> A resolver doesn't care about down.
>
> Add an angle type to the ABI docs. It simply hasn't been documented
> before because there were no drivers outside staging that use it.

I'm sorry, I misunderstood our previous discussing on this topic
(https://marc.info/?l=linux-iio=152087432322446=2) as saying:
"there already exists another channel type that is a good enough match".

The in_incli and in_rot_from_* family all use degrees as their unit.
For V2 I'll change it back to an angle channel and use angle as its
unit.

Best regards,
David Veenstra

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 11 ---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
>> b/drivers/staging/iio/resolver/ad2s1200.c
>> index 4b97a106012c..937676bcc0d4 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -64,6 +64,10 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  switch (m) {
>>  case IIO_CHAN_INFO_SCALE:
>>  switch (chan->type) {
>> +case IIO_INCLI:
>> +*val = 360;
>> +*val2 = 0xFFF;
>> +return IIO_VAL_FRACTIONAL;
>>  case IIO_ANGL_VEL:
>>  /*
>>   * 2 * Pi is almost equal to
>> @@ -91,7 +95,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>>  udelay(1);
>>  gpiod_set_value(st->sample, 1);
>> -gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +gpiod_set_value(st->rdvel, !!(chan->type == IIO_INCLI));
>>  
>>  ret = spi_read(st->sdev, st->rx, 2);
>>  if (ret < 0) {
>> @@ -101,7 +105,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  
>>  vel = be16_to_cpup((__be16 *)st->rx);
>>  switch (chan->type) {
>> -case IIO_ANGL:
>> +case IIO_INCLI:
>>  *val = vel >> 4;
>>  ret = IIO_VAL_INT;
>>  break;
>> @@ -128,10 +132,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  
>>  static const struct iio_chan_spec ad2s1200_channels[] = {
>>  {
>> -.type = IIO_ANGL,
>> +.type = IIO_INCLI,
>>  .indexed = 1,
>>  .channel = 0,
>>  .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>>  }, {
>>  .type = IIO_ANGL_VEL,
>>  .indexed = 1,

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

Re: [PATCH 07/11] staging: iio: ad2s1200: Ensure udelay(1) in all necessary code paths

[David Julian Veenstra]

On 23, March 2018 14:20, Jonathan Cameron wrote:

> On Sun, 18 Mar 2018 14:36:15 +0100
> David Veenstra  wrote:
>
>> After a successful spi transaction, a udelay(1) is needed.
>> This doesn't happen for the default case of the switch statement
>> in ad2s1200_read_raw. This patch makes sure that it does.
>> 
>> Signed-off-by: David Veenstra 
> Given this one can't actually happen as the chan->type
> is always one of the two options, I can't see the point
> in making sure we sleep.
>
> The default is really just there to catch bugs and to suppress
> the build warning we would otherwise get.
>
> So I am unconvinced on this patch.

I wasn't sure about this patch, but tried to be as critical as possible.
I'll leave it out for V2.

Best regards,
David Veenstra

>
> Jonathan
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 8 +---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
>> b/drivers/staging/iio/resolver/ad2s1200.c
>> index e0e7c88368ed..6ce9ca13094a 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -78,20 +78,22 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  switch (chan->type) {
>>  case IIO_ANGL:
>>  *val = vel >> 4;
>> +ret = IIO_VAL_INT;
>>  break;
>>  case IIO_ANGL_VEL:
>>  *val = sign_extend32((s16)vel >> 4, 11);
>> +ret = IIO_VAL_INT;
>>  break;
>>  default:
>> -mutex_unlock(>lock);
>> -return -EINVAL;
>> +ret = -EINVAL;
>> +break;
>>  }
>>  
>>  /* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>>  udelay(1);
>>  mutex_unlock(>lock);
>>  
>> -return IIO_VAL_INT;
>> +return ret;
>>  }
>>  
>>  static const struct iio_chan_spec ad2s1200_channels[] = {

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

Re: [PATCH 05/11] staging: iio: ad2s1200: Introduce variable for repeated value

[David Julian Veenstra]

On 23, March 2018 14:17, Jonathan Cameron wrote:

> On Sun, 18 Mar 2018 14:35:46 +0100
> David Veenstra  wrote:
>
>> Add variable to hold >dev in ad2s1200_probe. This value is repeatedly
>> used in ad2s1200_probe.
>> 
>> Signed-off-by: David Veenstra 
> I'm a little unconvinced by this one. It adds code to save
> a really very trivial bit of repetition.  If it had been
> via another pointer or this would have made a big difference
> in number of lines by bringing a lot of entries below 80 chars
> that weren't previous, then there would be a good argument.
>
> Here, not so much.

The 9th patch adds another 4 occurrences. So in total >dev appears
9 times in ad2s1200_probe. To me it seemed worth it to
introduce a variable for it. But I agree that it doesn't add much.

Best regards,
David Veenstra

>
> Jonathan
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 13 -
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
>> b/drivers/staging/iio/resolver/ad2s1200.c
>> index 00efba598671..eceb86e952de 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -118,19 +118,22 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  unsigned short *pins = spi->dev.platform_data;
>>  struct ad2s1200_state *st;
>>  struct iio_dev *indio_dev;
>> +struct device *dev;
>>  int pn, ret = 0;
>>  
>> +dev = >dev;
>> +
>>  for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -ret = devm_gpio_request_one(>dev, pins[pn], GPIOF_DIR_OUT,
>> +ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>>  DRV_NAME);
>>  if (ret) {
>> -dev_err(>dev, "request gpio pin %d failed\n",
>> +dev_err(dev, "request gpio pin %d failed\n",
>>  pins[pn]);
>>  return ret;
>>  }
>>  }
>>  
>> -indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
>> +indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>>  if (!indio_dev)
>>  return -ENOMEM;
>>  
>> @@ -141,14 +144,14 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  st->sample = pins[0];
>>  st->rdvel = pins[1];
>>  
>> -indio_dev->dev.parent = >dev;
>> +indio_dev->dev.parent = dev;
>>  indio_dev->info = _info;
>>  indio_dev->modes = INDIO_DIRECT_MODE;
>>  indio_dev->channels = ad2s1200_channels;
>>  indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
>>  indio_dev->name = spi_get_device_id(spi)->name;
>>  
>> -ret = devm_iio_device_register(>dev, indio_dev);
>> +ret = devm_iio_device_register(dev, indio_dev);
>>  if (ret)
>>  return ret;
>>  

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

[PATCH 1/2] staging: ks7010: Remove trailing "_t" from all structure names.

[Quytelda Kahja]

The "_t" suffix is not needed for structure names in this driver,
and is a reflection of an older typedef system that is no longer
in place.  Remove the "_t" suffix from every structure defined in this
driver.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/ks7010/ks7010_sdio.c |   2 +-
 drivers/staging/ks7010/ks_hostif.c   |  80 ++--
 drivers/staging/ks7010/ks_hostif.h   | 142 +--
 drivers/staging/ks7010/ks_wlan.h |  66 
 drivers/staging/ks7010/ks_wlan_net.c |  12 +--
 drivers/staging/ks7010/michael_mic.c |   8 +-
 drivers/staging/ks7010/michael_mic.h |   4 +-
 7 files changed, 157 insertions(+), 157 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index b8f55a11ee1c..d083bf8d238e 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -950,7 +950,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 /* send stop request to MAC */
 static int send_stop_request(struct sdio_func *func)
 {
-   struct hostif_stop_request_t *pp;
+   struct hostif_stop_request *pp;
struct ks_sdio_card *card;
size_t size;
 
diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 143413c3cae2..d558fdf48e28 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -110,16 +110,16 @@ int ks_wlan_do_power_save(struct ks_wlan_private *priv)
 }
 
 static
-int get_current_ap(struct ks_wlan_private *priv, struct link_ap_info_t 
*ap_info)
+int get_current_ap(struct ks_wlan_private *priv, struct link_ap_info *ap_info)
 {
-   struct local_ap_t *ap;
+   struct local_ap *ap;
union iwreq_data wrqu;
struct net_device *netdev = priv->net_dev;
 
ap = >current_ap;
 
if (is_disconnect_status(priv->connect_status)) {
-   memset(ap, 0, sizeof(struct local_ap_t));
+   memset(ap, 0, sizeof(struct local_ap));
return -EPERM;
}
 
@@ -226,13 +226,13 @@ static u8 read_ie(unsigned char *bp, u8 max, u8 *body)
 
 
 static
-int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info,
-  struct local_ap_t *ap)
+int get_ap_information(struct ks_wlan_private *priv, struct ap_info *ap_info,
+  struct local_ap *ap)
 {
unsigned char *bp;
int bsize, offset;
 
-   memset(ap, 0, sizeof(struct local_ap_t));
+   memset(ap, 0, sizeof(struct local_ap));
 
/* bssid */
memcpy(ap->bssid, ap_info->bssid, ETH_ALEN);
@@ -317,11 +317,11 @@ int hostif_data_indication_wpa(struct ks_wlan_private 
*priv,
unsigned char recv_mic[8];
char buf[128];
unsigned long now;
-   struct mic_failure_t *mic_failure;
-   struct michael_mic_t michael_mic;
+   struct mic_failure *mic_failure;
+   struct michael_mic michael_mic;
union iwreq_data wrqu;
unsigned int key_index = auth_type - 1;
-   struct wpa_key_t *key = >wpa.key[key_index];
+   struct wpa_key *key = >wpa.key[key_index];
 
eth_hdr = (struct ether_hdr *)(priv->rxp);
eth_proto = ntohs(eth_hdr->h_proto);
@@ -748,7 +748,7 @@ void hostif_connect_indication(struct ks_wlan_private *priv)
break;
}
 
-   get_current_ap(priv, (struct link_ap_info_t *)priv->rxp);
+   get_current_ap(priv, (struct link_ap_info *)priv->rxp);
if (is_connect_status(priv->connect_status) &&
is_disconnect_status(old_status)) {
/* for power save */
@@ -774,10 +774,10 @@ static
 void hostif_scan_indication(struct ks_wlan_private *priv)
 {
int i;
-   struct ap_info_t *ap_info;
+   struct ap_info *ap_info;
 
netdev_dbg(priv->net_dev, "scan_ind_count = %d\n", 
priv->scan_ind_count);
-   ap_info = (struct ap_info_t *)(priv->rxp);
+   ap_info = (struct ap_info *)(priv->rxp);
 
if (priv->scan_ind_count) {
/* bssid check */
@@ -797,7 +797,7 @@ void hostif_scan_indication(struct ks_wlan_private *priv)
if (priv->scan_ind_count < LOCAL_APLIST_MAX + 1) {
netdev_dbg(priv->net_dev, " scan_ind_count=%d :: 
aplist.size=%d\n",
priv->scan_ind_count, priv->aplist.size);
-   get_ap_information(priv, (struct ap_info_t *)(priv->rxp),
+   get_ap_information(priv, (struct ap_info *)(priv->rxp),
   &(priv->aplist.ap[priv->scan_ind_count - 
1]));
priv->aplist.size = priv->scan_ind_count;
} else {
@@ -866,8 +866,8 @@ void hostif_adhoc_set_confirm(struct ks_wlan_private *priv)
 static
 void hostif_associate_indication(struct ks_wlan_private *priv)
 {
-   struct association_request_t *assoc_req;
-   struct association_response_t *assoc_resp;
+   struct association_request *assoc_req;
+   

[PATCH 2/2] staging: ks7010: Fix spelling mistakes.

[Quytelda Kahja]

Fix two spelling mistakes in comments.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/ks7010/ks_hostif.h | 2 +-
 drivers/staging/ks7010/ks_wlan.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index 7f6647f3b337..f42a5921528f 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -59,7 +59,7 @@
 
 /*
  * HOST-MAC I/F data structure
- * Byte alignmet Little Endian
+ * Byte alignment Little Endian
  */
 
 struct hostif_hdr {
diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index c94a88028710..a76c5bef5605 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -54,7 +54,7 @@ struct ks_wlan_parameter {
 #define BEACON_LOST_COUNT_MAX 65535
u32 beacon_lost_count;  /*  Beacon Lost Count */
u32 rts;/*  RTS Threashold */
-   u32 fragment;   /*  Fragmentation Threashold */
+   u32 fragment;   /*  Fragmentation Threshold */
u32 privacy_invoked;
u32 wep_index;
struct {
-- 
2.16.2

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

[PATCH 0/4] net: drivers/net: Use octal permissions

[Joe Perches]

Using octal and not symbolic permissions is preferred by many as
more readable.

https://lkml.org/lkml/2016/8/2/1945

Rather than getting these piecemeal, just do them all.
Done with checkpatch and some typing.

Joe Perches (4):
  ethernet: Use octal not symbolic permissions
  wireless: Use octal not symbolic permissions
  net: Use octal not symbolic permissions
  drivers/net: Use octal not symbolic permissions

 drivers/net/bonding/bond_procfs.c  |   2 +-
 drivers/net/bonding/bond_sysfs.c   |  73 +++---
 drivers/net/bonding/bond_sysfs_slave.c |   4 +-
 drivers/net/caif/caif_serial.c |  32 +++---
 drivers/net/caif/caif_spi.c|  16 +--
 drivers/net/caif/caif_virtio.c |  16 +--
 drivers/net/can/at91_can.c |   3 +-
 drivers/net/can/cc770/cc770.c  |   4 +-
 drivers/net/can/cc770/cc770_isa.c  |  16 +--
 drivers/net/can/grcan.c|   4 +-
 drivers/net/can/janz-ican3.c   |   6 +-
 drivers/net/can/sja1000/sja1000_isa.c  |  14 +--
 drivers/net/can/softing/softing_main.c |   4 +-
 drivers/net/can/spi/mcp251x.c  |   2 +-
 drivers/net/can/usb/esd_usb2.c |   6 +-
 drivers/net/can/vcan.c |   2 +-
 drivers/net/ethernet/8390/apne.c   |   2 +-
 drivers/net/ethernet/8390/lib8390.c|   2 +-
 drivers/net/ethernet/8390/ne.c |   2 +-
 drivers/net/ethernet/8390/ne2k-pci.c   |   2 +-
 drivers/net/ethernet/8390/smc-ultra.c  |   2 +-
 drivers/net/ethernet/8390/stnic.c  |   2 +-
 drivers/net/ethernet/8390/wd.c |   2 +-
 drivers/net/ethernet/altera/altera_tse_main.c  |   6 +-
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c   |  10 +-
 drivers/net/ethernet/amd/xgbe/xgbe-main.c  |   2 +-
 drivers/net/ethernet/broadcom/bnx2.c   |   2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   |  12 +--
 drivers/net/ethernet/broadcom/sb1250-mac.c |  10 +-
 drivers/net/ethernet/broadcom/tg3.c|   6 +-
 drivers/net/ethernet/brocade/bna/bnad.c|   2 +-
 drivers/net/ethernet/brocade/bna/bnad_debugfs.c|  10 +-
 drivers/net/ethernet/cavium/thunder/nicvf_main.c   |   2 +-
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c|   6 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 112 ++---
 .../net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c|  10 +-
 drivers/net/ethernet/ec_bhf.c  |   2 +-
 drivers/net/ethernet/emulex/benet/be_main.c|   6 +-
 drivers/net/ethernet/ibm/ehea/ehea_main.c  |   7 +-
 drivers/net/ethernet/ibm/ibmveth.c |   2 +-
 drivers/net/ethernet/intel/igb/igb_hwmon.c |   2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c |   2 +-
 drivers/net/ethernet/marvell/mvneta.c  |   8 +-
 drivers/net/ethernet/marvell/skge.c|   2 +-
 drivers/net/ethernet/marvell/sky2.c|   2 +-
 drivers/net/ethernet/mellanox/mlx4/main.c  |  16 +--
 drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c   |  10 +-
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c   |  32 +++---
 .../net/ethernet/netronome/nfp/nfp_net_debugfs.c   |   6 +-
 .../net/ethernet/qlogic/netxen/netxen_nic_main.c   |  14 +--
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c  |  30 +++---
 drivers/net/ethernet/qualcomm/qca_debug.c  |   2 +-
 drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c|   4 +-
 drivers/net/ethernet/sfc/mcdi_mon.c|   2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  26 ++---
 drivers/net/ethernet/sun/niu.c |  10 +-
 drivers/net/hamradio/bpqether.c|   3 +-
 drivers/net/hamradio/yam.c |   2 +-
 drivers/net/hyperv/netvsc_drv.c|   4 +-
 drivers/net/ieee802154/at86rf230.c |   2 +-
 drivers/net/phy/spi_ks8995.c   |   2 +-
 drivers/net/ppp/ppp_generic.c  |   2 +-
 drivers/net/ppp/pppoe.c|   2 +-
 drivers/net/usb/cdc_ncm.c  |  12 +--
 drivers/net/usb/hso.c  |   8 +-
 drivers/net/wireless/ath/ath5k/base.c  |   6 +-
 drivers/net/wireless/ath/ath5k/debug.c |  37 ++-
 drivers/net/wireless/ath/ath5k/sysfs.c |   8 +-
 drivers/net/wireless/ath/ath6kl/debug.c|  43 
 drivers/net/wireless/ath/ath9k/common-debug.c  |   9 +-
 drivers/net/wireless/ath/ath9k/common-spectral.c   |  10 +-
 drivers/net/wireless/ath/ath9k/debug.c |  40 
 drivers/net/wireless/ath/ath9k/debug_sta.c |   6 +-
 drivers/net/wireless/ath/ath9k/dfs_debug.c |   4