[PATCH v3] staging: iio: cdc: ad7746: add additional config defines

2016-10-28 Thread Eva Rachel Retuya
Introduce defines for shifting and mask under the config register for
better readability. Also, introduce helper variables for index
calculation.

Signed-off-by: Eva Rachel Retuya 
---
This patch might cause a conflict with this patch:
staging: iio: cdc/ad7746: fix missing return value
https://marc.info/?l=linux-driver-devel&m=147741283722806&w=2
I am waiting to rebase to the branch where this commit is present but
I was told it was not yet pushed to kernel.org

Changes in v3:
* Make commit message more detailed.
* Fix incorrect use of GENMASK.
* Drop the AD7746_CONF_*FS(x) macros and use the mask and direct shifting where
  needed.
* With the proper masks set, invert the AND'ng and shifting on the index
  calculations.

Changes in v2:
* Use GENMASK to generate both VTFS and CAPFS masks including offset,
  respectively.
* Introduce helper variables for index calculation.

 drivers/staging/iio/cdc/ad7746.c | 38 ++
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index f41251c..e97658a 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -70,8 +70,10 @@
 #define AD7746_EXCSETUP_EXCLVL(x)  (((x) & 0x3) << 0)
 
 /* Config Register Bit Designations (AD7746_REG_CFG) */
-#define AD7746_CONF_VTFS(x)((x) << 6)
-#define AD7746_CONF_CAPFS(x)   ((x) << 3)
+#define AD7746_CONF_VTFS_SHIFT 6
+#define AD7746_CONF_CAPFS_SHIFT3
+#define AD7746_CONF_VTFS_MASK  GENMASK(7, 6)
+#define AD7746_CONF_CAPFS_MASK GENMASK(5, 3)
 #define AD7746_CONF_MODE_IDLE  (0 << 0)
 #define AD7746_CONF_MODE_CONT_CONV (1 << 0)
 #define AD7746_CONF_MODE_SINGLE_CONV   (2 << 0)
@@ -217,15 +219,16 @@ static int ad7746_select_channel(struct iio_dev 
*indio_dev,
struct iio_chan_spec const *chan)
 {
struct ad7746_chip_info *chip = iio_priv(indio_dev);
-   int ret, delay;
+   int ret, delay, idx;
u8 vt_setup, cap_setup;
 
switch (chan->type) {
case IIO_CAPACITANCE:
cap_setup = (chan->address & 0xFF) | AD7746_CAPSETUP_CAPEN;
vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
-   delay = ad7746_cap_filter_rate_table[(chip->config >> 3) &
-   0x7][1];
+   idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
+   AD7746_CONF_CAPFS_SHIFT;
+   delay = ad7746_cap_filter_rate_table[idx][1];
 
if (chip->capdac_set != chan->channel) {
ret = i2c_smbus_write_byte_data(chip->client,
@@ -246,8 +249,9 @@ static int ad7746_select_channel(struct iio_dev *indio_dev,
case IIO_TEMP:
vt_setup = (chan->address & 0xFF) | AD7746_VTSETUP_VTEN;
cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
-   delay = ad7746_cap_filter_rate_table[(chip->config >> 6) &
-   0x3][1];
+   idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
+   AD7746_CONF_VTFS_SHIFT;
+   delay = ad7746_cap_filter_rate_table[idx][1];
break;
default:
return -EINVAL;
@@ -369,8 +373,8 @@ static int ad7746_store_cap_filter_rate_setup(struct 
ad7746_chip_info *chip,
if (i >= ARRAY_SIZE(ad7746_cap_filter_rate_table))
i = ARRAY_SIZE(ad7746_cap_filter_rate_table) - 1;
 
-   chip->config &= ~AD7746_CONF_CAPFS(0x7);
-   chip->config |= AD7746_CONF_CAPFS(i);
+   chip->config &= ~AD7746_CONF_CAPFS_MASK;
+   chip->config |= i << AD7746_CONF_CAPFS_SHIFT;
 
return 0;
 }
@@ -387,8 +391,8 @@ static int ad7746_store_vt_filter_rate_setup(struct 
ad7746_chip_info *chip,
if (i >= ARRAY_SIZE(ad7746_vt_filter_rate_table))
i = ARRAY_SIZE(ad7746_vt_filter_rate_table) - 1;
 
-   chip->config &= ~AD7746_CONF_VTFS(0x3);
-   chip->config |= AD7746_CONF_VTFS(i);
+   chip->config &= ~AD7746_CONF_VTFS_MASK;
+   chip->config |= i << AD7746_CONF_VTFS_SHIFT;
 
return 0;
 }
@@ -527,7 +531,7 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
   long mask)
 {
struct ad7746_chip_info *chip = iio_priv(indio_dev);
-   int ret, delay;
+   int ret, delay, idx;
u8 regval, reg;
 
mutex_lock(&indio_dev->mlock);
@@ -635,13 +639,15 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SAMP_FREQ:
switch (chan->type) {
case IIO_CAPACITANCE:
-   *val = ad7746_cap_filter_rate_table[
-   (chip->config >> 3) & 0x7][0];
+   idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
+   AD7746_CONF_CAPFS_SHIFT;
+   *val = ad7746_cap_filter_rate_table[idx][0]

[PATCH 01/10] staging: iio: tsl2583: add of_match table for device tree support

2016-10-28 Thread Brian Masney
Add device tree support for the tsl2583 IIO driver with no custom
properties.

Signed-off-by: Brian Masney 
---
 .../devicetree/bindings/iio/light/tsl2583.txt  | 26 ++
 drivers/staging/iio/light/tsl2583.c| 13 +++
 2 files changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt

diff --git a/Documentation/devicetree/bindings/iio/light/tsl2583.txt 
b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
new file mode 100644
index 000..8e2066c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
@@ -0,0 +1,26 @@
+* TAOS TSL 2580/2581/2583 ALS sensor
+
+Required properties:
+
+  - compatible: Should be one of
+   "amstaos,tsl2580"
+   "amstaos,tsl2581"
+   "amstaos,tsl2583"
+  - reg: the I2C address of the device
+
+Optional properties:
+
+  - interrupt-parent: should be the phandle for the interrupt controller
+  - interrupts: the sole interrupt generated by the device
+
+  Refer to interrupt-controller/interrupts.txt for generic interrupt client
+  node bindings.
+
+  - vcc-supply: phandle to the regulator that provides power to the sensor.
+
+Example:
+
+tsl2581@29 {
+   compatible = "amstaos,tsl2581";
+   reg = <0x29>;
+};
diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index 08f1583..fd4b6ef 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -947,11 +947,24 @@ static struct i2c_device_id taos_idtable[] = {
 };
 MODULE_DEVICE_TABLE(i2c, taos_idtable);
 
+#ifdef CONFIG_OF
+static const struct of_device_id taos2583_of_match[] = {
+   { .compatible = "amstaos,tsl2580", },
+   { .compatible = "amstaos,tsl2581", },
+   { .compatible = "amstaos,tsl2583", },
+   { },
+};
+MODULE_DEVICE_TABLE(of, taos2583_of_match);
+#else
+#define taos2583_of_match NULL
+#endif
+
 /* Driver definition */
 static struct i2c_driver taos_driver = {
.driver = {
.name = "tsl2583",
.pm = TAOS_PM_OPS,
+   .of_match_table = taos2583_of_match,
},
.id_table = taos_idtable,
.probe = taos_probe,
-- 
2.7.4

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


[PATCH 02/10] staging: iio: tsl2583: check for error code from i2c_smbus_read_byte()

2016-10-28 Thread Brian Masney
taos_i2c_read() and taos_als_calibrate() does not check to see if the
value returned by i2c_smbus_read_byte() was an error code. This patch
adds the appropriate error handling.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index fd4b6ef..35c1696 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -171,7 +171,14 @@ taos_i2c_read(struct i2c_client *client, u8 reg, u8 *val, 
unsigned int len)
return ret;
}
/* read the data */
-   *val = i2c_smbus_read_byte(client);
+   ret = i2c_smbus_read_byte(client);
+   if (ret < 0) {
+   dev_err(&client->dev,
+   "%s failed to read byte after writing to 
register %x\n",
+   __func__, reg);
+   return ret;
+   }
+   *val = ret;
val++;
reg++;
}
@@ -355,6 +362,13 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
}
 
reg_val = i2c_smbus_read_byte(chip->client);
+   if (reg_val < 0) {
+   dev_err(&chip->client->dev,
+   "%s failed to read after writing to the CNTRL 
register\n",
+   __func__);
+   return ret;
+   }
+
if ((reg_val & (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON))
!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
dev_err(&chip->client->dev,
@@ -371,6 +385,12 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
return ret;
}
reg_val = i2c_smbus_read_byte(chip->client);
+   if (reg_val < 0) {
+   dev_err(&chip->client->dev,
+   "%s failed to read after writing to the STATUS 
register\n",
+   __func__);
+   return ret;
+   }
 
if ((reg_val & TSL258X_STA_ADC_VALID) != TSL258X_STA_ADC_VALID) {
dev_err(&chip->client->dev,
-- 
2.7.4

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


[PATCH 03/10] staging: iio: tsl2583: return proper error code instead of -1

2016-10-28 Thread Brian Masney
taos_als_calibrate() has a code path where -1 is returned. This patch
changes the code so that a proper error code is returned.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index 35c1696..47fd373 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -373,7 +373,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
dev_err(&chip->client->dev,
"taos_als_calibrate failed: device not powered on with 
ADC enabled\n");
-   return -1;
+   return -EINVAL;
}
 
ret = i2c_smbus_write_byte(chip->client,
-- 
2.7.4

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


[PATCH 07/10] staging: iio: tsl2583: convert illuminance0_calibscale sysfs attr to use iio_chan_spec

2016-10-28 Thread Brian Masney
The illuminance0_calibscale sysfs attribute is not currently created by
the IIO core. This patch adds the appropriate mask to iio_chan_spec,
along with the appropriate data handling in the read_raw() and
write_raw() functions, so that the sysfs attribute is created by the IIO
core. With this change, this sysfs entry will have its prefix changed
from illuminance0_ to in_illuminance_.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 89 +++--
 1 file changed, 25 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index 6a61a86..bfff6ca 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -124,14 +124,15 @@ static struct taos_lux taos_device_lux[11] = {
 struct gainadj {
s16 ch0;
s16 ch1;
+   s16 mean;
 };
 
 /* Index = (0 - 3) Used to validate the gain selection index */
 static const struct gainadj gainadj[] = {
-   { 1, 1 },
-   { 8, 8 },
-   { 16, 16 },
-   { 107, 115 }
+   { 1, 1, 1 },
+   { 8, 8, 8 },
+   { 16, 16, 16 },
+   { 107, 115, 111 }
 };
 
 /*
@@ -505,63 +506,6 @@ static int taos_chip_off(struct iio_dev *indio_dev)
 
 /* Sysfs Interface Functions */
 
-static ssize_t taos_gain_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2583_chip *chip = iio_priv(indio_dev);
-   char gain[4] = {0};
-
-   switch (chip->taos_settings.als_gain) {
-   case 0:
-   strcpy(gain, "001");
-   break;
-   case 1:
-   strcpy(gain, "008");
-   break;
-   case 2:
-   strcpy(gain, "016");
-   break;
-   case 3:
-   strcpy(gain, "111");
-   break;
-   }
-
-   return sprintf(buf, "%s\n", gain);
-}
-
-static ssize_t taos_gain_store(struct device *dev,
-  struct device_attribute *attr,
-  const char *buf, size_t len)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2583_chip *chip = iio_priv(indio_dev);
-   int value;
-
-   if (kstrtoint(buf, 0, &value))
-   return -EINVAL;
-
-   switch (value) {
-   case 1:
-   chip->taos_settings.als_gain = 0;
-   break;
-   case 8:
-   chip->taos_settings.als_gain = 1;
-   break;
-   case 16:
-   chip->taos_settings.als_gain = 2;
-   break;
-   case 111:
-   chip->taos_settings.als_gain = 3;
-   break;
-   default:
-   dev_err(dev, "Invalid Gain Index (must be 1,8,16,111)\n");
-   return -1;
-   }
-
-   return len;
-}
-
 static ssize_t taos_gain_available_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -691,8 +635,6 @@ static ssize_t taos_luxtable_store(struct device *dev,
return ret;
 }
 
-static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
-   taos_gain_show, taos_gain_store);
 static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
taos_gain_available_show, NULL);
 
@@ -707,7 +649,6 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | 
S_IWUSR,
taos_luxtable_show, taos_luxtable_store);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
-   &dev_attr_illuminance0_calibscale.attr, /* Gain  */
&dev_attr_illuminance0_calibscale_available.attr,
&dev_attr_illuminance0_integration_time_available.attr,
&dev_attr_illuminance0_input_target.attr,
@@ -743,6 +684,7 @@ static const struct iio_chan_spec tsl2583_channels[] = {
.type = IIO_LIGHT,
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
  BIT(IIO_CHAN_INFO_CALIBBIAS) |
+ BIT(IIO_CHAN_INFO_CALIBSCALE) |
  BIT(IIO_CHAN_INFO_INT_TIME),
},
 };
@@ -801,6 +743,12 @@ static int tsl2583_read_raw(struct iio_dev *indio_dev,
ret = IIO_VAL_INT;
}
break;
+   case IIO_CHAN_INFO_CALIBSCALE:
+   if (chan->type == IIO_LIGHT) {
+   *val = gainadj[chip->taos_settings.als_gain].mean;
+   ret = IIO_VAL_INT;
+   }
+   break;
case IIO_CHAN_INFO_INT_TIME:
if (chan->type == IIO_LIGHT) {
*val = 0;
@@ -839,6 +787,19 @@ static int tsl2583_write_raw(struct iio_dev *indio_dev,
ret = 0;
}
break;
+   case IIO_CHAN_INFO_CALIBSCALE:
+   if (chan->typ

[PATCH 05/10] staging: iio: tsl2583: check return values from taos_chip_{on, off}

2016-10-28 Thread Brian Masney
The return value from taos_chip_on() and taos_chip_off() was not
checked in taos_luxtable_store() and taos_probe(). This patch adds
proper error checking to these function calls.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index f8ccb4d..e975bba 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -727,7 +727,7 @@ static ssize_t taos_luxtable_store(struct device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct tsl2583_chip *chip = iio_priv(indio_dev);
int value[ARRAY_SIZE(taos_device_lux) * 3 + 1];
-   int n;
+   int n, ret = -EINVAL;
 
get_options(buf, ARRAY_SIZE(value), value);
 
@@ -739,23 +739,31 @@ static ssize_t taos_luxtable_store(struct device *dev,
n = value[0];
if ((n % 3) || n < 6 || n > ((ARRAY_SIZE(taos_device_lux) - 1) * 3)) {
dev_info(dev, "LUX TABLE INPUT ERROR 1 Value[0]=%d\n", n);
-   return -EINVAL;
+   goto done;
}
if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0) {
dev_info(dev, "LUX TABLE INPUT ERROR 2 Value[0]=%d\n", n);
-   return -EINVAL;
+   goto done;
}
 
-   if (chip->taos_chip_status == TSL258X_CHIP_WORKING)
-   taos_chip_off(indio_dev);
+   if (chip->taos_chip_status == TSL258X_CHIP_WORKING) {
+   ret = taos_chip_off(indio_dev);
+   if (ret < 0)
+   goto done;
+   }
 
/* Zero out the table */
memset(taos_device_lux, 0, sizeof(taos_device_lux));
memcpy(taos_device_lux, &value[1], (value[0] * 4));
 
-   taos_chip_on(indio_dev);
+   ret = taos_chip_on(indio_dev);
+   if (ret < 0)
+   goto done;
 
-   return len;
+   ret = len;
+
+done:
+   return ret;
 }
 
 static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
@@ -883,7 +891,9 @@ static int taos_probe(struct i2c_client *clientp,
taos_defaults(chip);
 
/* Make sure the chip is on */
-   taos_chip_on(indio_dev);
+   ret = taos_chip_on(indio_dev);
+   if (ret < 0)
+   return ret;
 
dev_info(&clientp->dev, "Light sensor found.\n");
return 0;
-- 
2.7.4

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


[PATCH 04/10] staging: iio: tsl2583: remove redundant power_state sysfs attribute

2016-10-28 Thread Brian Masney
IIO devices have a /sys/bus/iio/devices/iio:deviceX/power/ directory
that allows viewing and controling various power parameters. The tsl2583
driver also has an additional custom sysfs attribute named power_state
that is not needed. This patch removes the redundant power_state sysfs
attribute.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 31 ---
 1 file changed, 31 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index 47fd373..f8ccb4d 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -511,33 +511,6 @@ static int taos_chip_off(struct iio_dev *indio_dev)
 
 /* Sysfs Interface Functions */
 
-static ssize_t taos_power_state_show(struct device *dev,
-struct device_attribute *attr, char *buf)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2583_chip *chip = iio_priv(indio_dev);
-
-   return sprintf(buf, "%d\n", chip->taos_chip_status);
-}
-
-static ssize_t taos_power_state_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t len)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   int value;
-
-   if (kstrtoint(buf, 0, &value))
-   return -EINVAL;
-
-   if (!value)
-   taos_chip_off(indio_dev);
-   else
-   taos_chip_on(indio_dev);
-
-   return len;
-}
-
 static ssize_t taos_gain_show(struct device *dev,
  struct device_attribute *attr, char *buf)
 {
@@ -785,9 +758,6 @@ static ssize_t taos_luxtable_store(struct device *dev,
return len;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
-   taos_power_state_show, taos_power_state_store);
-
 static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
taos_gain_show, taos_gain_store);
 static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
@@ -810,7 +780,6 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | 
S_IWUSR,
taos_luxtable_show, taos_luxtable_store);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
-   &dev_attr_power_state.attr,
&dev_attr_illuminance0_calibscale.attr, /* Gain  */
&dev_attr_illuminance0_calibscale_available.attr,
&dev_attr_illuminance0_integration_time.attr,   /* I time*/
-- 
2.7.4

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


[PATCH 06/10] staging: iio: tsl2583: convert to use iio_chan_spec and {read, write}_raw

2016-10-28 Thread Brian Masney
The tsl2583 driver directly creates sysfs attributes that should instead
be created by the IIO core on behalf of the driver. This patch adds the
iio_chan_spec array, the relevant info_mask elements and the read_raw()
and write_raw() functions to take advantage of features provided by the
IIO core. These sysfs attributes were migrated with this patch:
illuminance0_input, illuminance0_calibbias,
illuminance0_integration_time. This also exposes the raw values read
from the two channels on the sensor.

With this change, these four sysfs entries have their prefix changed
from illuminance0_ to in_illuminance_. This is deemed to be acceptable
since none of the IIO light drivers in mainline use the illuminance0_
prefix, however 8 of the IIO light drivers in mainline use the
in_illuminance_ prefix.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 236 ++--
 1 file changed, 143 insertions(+), 93 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index e975bba..6a61a86 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -211,28 +211,23 @@ static int taos_get_lux(struct iio_dev *indio_dev)
u32 ch0lux = 0;
u32 ch1lux = 0;
 
-   if (mutex_trylock(&chip->als_mutex) == 0) {
-   dev_info(&chip->client->dev, "taos_get_lux device is busy\n");
-   return chip->als_cur_info.lux; /* busy, so return LAST VALUE */
-   }
-
if (chip->taos_chip_status != TSL258X_CHIP_WORKING) {
/* device is not enabled */
dev_err(&chip->client->dev, "taos_get_lux device is not 
enabled\n");
ret = -EBUSY;
-   goto out_unlock;
+   goto done;
}
 
ret = taos_i2c_read(chip->client, (TSL258X_CMD_REG), &buf[0], 1);
if (ret < 0) {
dev_err(&chip->client->dev, "taos_get_lux failed to read 
CMD_REG\n");
-   goto out_unlock;
+   goto done;
}
/* is data new & valid */
if (!(buf[0] & TSL258X_STA_ADC_INTR)) {
dev_err(&chip->client->dev, "taos_get_lux data not valid\n");
ret = chip->als_cur_info.lux; /* return LAST VALUE */
-   goto out_unlock;
+   goto done;
}
 
for (i = 0; i < 4; i++) {
@@ -243,7 +238,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
dev_err(&chip->client->dev,
"taos_get_lux failed to read register %x\n",
reg);
-   goto out_unlock;
+   goto done;
}
}
 
@@ -259,7 +254,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
dev_err(&chip->client->dev,
"taos_i2c_write_command failed in taos_get_lux, err = 
%d\n",
ret);
-   goto out_unlock; /* have no data, so return failure */
+   goto done; /* have no data, so return failure */
}
 
/* extract ALS/lux data */
@@ -276,7 +271,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
/* have no data, so return LAST VALUE */
ret = 0;
chip->als_cur_info.lux = 0;
-   goto out_unlock;
+   goto done;
}
/* calculate ratio */
ratio = (ch1 << 15) / ch0;
@@ -302,7 +297,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
dev_dbg(&chip->client->dev, "No Data - Return last value\n");
ret = 0;
chip->als_cur_info.lux = 0;
-   goto out_unlock;
+   goto done;
}
 
/* adjust for active time scale */
@@ -334,8 +329,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
chip->als_cur_info.lux = lux;
ret = lux;
 
-out_unlock:
-   mutex_unlock(&chip->als_mutex);
+done:
return ret;
 }
 
@@ -575,69 +569,12 @@ static ssize_t taos_gain_available_show(struct device 
*dev,
return sprintf(buf, "%s\n", "1 8 16 111");
 }
 
-static ssize_t taos_als_time_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2583_chip *chip = iio_priv(indio_dev);
-
-   return sprintf(buf, "%d\n", chip->taos_settings.als_time);
-}
-
-static ssize_t taos_als_time_store(struct device *dev,
-  struct device_attribute *attr,
-  const char *buf, size_t len)
-{
-   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-   struct tsl2583_chip *chip = iio_priv(indio_dev);
-   int value;
-
-   if (kstrtoint(buf, 0, &value))
-   return -EINVAL;
-
-   if ((value < 50) || (value > 650))
-   return -EINVAL;
-
-   if (value % 50)
- 

[PATCH 00/10] staging: iio: tsl2583: staging cleanups

2016-10-28 Thread Brian Masney
This patch set begins cleaning up some of the major items that is
keeping the tsl2583 driver out of mainline. Highlights include device
tree support, converts the driver over to use the iio_chan_spec,
improved error handling, and fixes for some concurrency issues. There
is more work required to get this driver out of staging that I will
send later as a separate patch set.

Driver was tested using a TSL2581 hooked up to a Raspberry Pi 2.

The sysfs attributes were previously prefixed with illuminance0_ however
they are now prefixed with in_illuminance_. None of the IIO light
drivers in mainline have their sysfs attributes prefixed with
illuminance0_, however 8 of the IIO light drivers in mainline use the
in_illuminance_ prefix so I assume that this is the naming convention
that should be used for this driver as well.

sysfs attribute names before this patch set:

raspberrypi:/sys/bus/iio/devices/iio:device0$ ls -l
total 0
-r--r--r-- 1 root root 4096 Oct 27 20:27 dev
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_calibbias
--w--- 1 root root 4096 Oct 27 20:27 illuminance0_calibrate
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_calibscale
-r--r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_calibscale_available
-r--r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_input
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_input_target
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_integration_time
-r--r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_integration_time_available
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_lux_table
-r--r--r-- 1 root root 4096 Oct 27 20:27 name
lrwxrwxrwx 1 root root0 Oct 27 20:27 of_node ->
 ../../../../../../../firmware/devicetree/base/soc/i2c@7e804000/tsl2581@29/
drwxr-xr-x 2 root root0 Oct 27 20:27 power/
-rw-r--r-- 1 root root 4096 Oct 27 20:27 power_state
lrwxrwxrwx 1 root root0 Oct 27 20:27 subsystem -> 
../../../../../../../bus/iio/
-rw-r--r-- 1 root root 4096 Oct 27 20:27 uevent

sysfs attribute names after this patch set:

raspberrypi:/sys/bus/iio/devices/iio:device0$ ls -l
total 0
-r--r--r-- 1 root root 4096 Oct 27 22:29 dev
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_both_raw
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_calibbias
--w--- 1 root root 4096 Oct 27 22:29 in_illuminance_calibrate
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_calibscale
-r--r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_calibscale_available
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_input
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_input_target
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_integration_time
-r--r--r-- 1 root root 4096 Oct 27 22:29 
in_illuminance_integration_time_available
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_ir_raw
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_lux_table
-r--r--r-- 1 root root 4096 Oct 27 22:29 name
lrwxrwxrwx 1 root root0 Oct 27 22:29 of_node ->
 ../../../../../../../firmware/devicetree/base/soc/i2c@7e804000/tsl2581@29/
drwxr-xr-x 2 root root0 Oct 27 22:29 power/
lrwxrwxrwx 1 root root0 Oct 27 22:29 subsystem ->
 ../../../../../../../bus/iio/
-rw-r--r-- 1 root root 4096 Oct 27 22:23 uevent

Brian Masney (10):
  staging: iio: tsl2583: add of_match table for device tree support
  staging: iio: tsl2583: check for error code from i2c_smbus_read_byte()
  staging: iio: tsl2583: return proper error code instead of -1
  staging: iio: tsl2583: remove redundant power_state sysfs attribute
  staging: iio: tsl2583: check return values from taos_chip_{on,off}
  staging: iio: tsl2583: convert to use iio_chan_spec and
{read,write}_raw
  staging: iio: tsl2583: convert illuminance0_calibscale sysfs attr to
use iio_chan_spec
  staging: iio: tsl2583: use IIO_*_ATTR* macros to create sysfs entries
  staging: iio: tsl2583: add error code to sysfs store functions
  staging: iio: tsl2583: add locking to sysfs attributes

 .../devicetree/bindings/iio/light/tsl2583.txt  |  26 ++
 drivers/staging/iio/light/tsl2583.c| 509 +++--
 2 files changed, 288 insertions(+), 247 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt

-- 
2.7.4

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


[PATCH 09/10] staging: iio: tsl2583: add error code to sysfs store functions

2016-10-28 Thread Brian Masney
in_illuminance_input_target_store() and in_illuminance_calibrate_store()
validated the data from userspace, however it would not return an
error code to userspace if an invalid value was passed in. This patch
changes these functions so that they return -EINVAL if invalid data is
passed in.

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

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index 1462374..98afa5b 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -525,11 +525,10 @@ static ssize_t in_illuminance_input_target_store(struct 
device *dev,
struct tsl2583_chip *chip = iio_priv(indio_dev);
int value;
 
-   if (kstrtoint(buf, 0, &value))
+   if (kstrtoint(buf, 0, &value) || !value)
return -EINVAL;
 
-   if (value)
-   chip->taos_settings.als_cal_target = value;
+   chip->taos_settings.als_cal_target = value;
 
return len;
 }
@@ -541,11 +540,10 @@ static ssize_t in_illuminance_calibrate_store(struct 
device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
int value;
 
-   if (kstrtoint(buf, 0, &value))
+   if (kstrtoint(buf, 0, &value) || value != 1)
return -EINVAL;
 
-   if (value == 1)
-   taos_als_calibrate(indio_dev);
+   taos_als_calibrate(indio_dev);
 
return len;
 }
-- 
2.7.4

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


[PATCH 10/10] staging: iio: tsl2583: add locking to sysfs attributes

2016-10-28 Thread Brian Masney
in_illuminance_input_target_show(), in_illuminance_input_target_store(),
in_illuminance_calibrate_store(), and in_illuminance_lux_table_store()
accesses data from the tsl2583_chip struct. Some of these fields can be
modified by other parts of the driver concurrently. This patch adds the
mutex locking to these sysfs attributes.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index 98afa5b..49b19f5 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -513,8 +513,13 @@ static ssize_t in_illuminance_input_target_show(struct 
device *dev,
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct tsl2583_chip *chip = iio_priv(indio_dev);
+   int ret;
+
+   mutex_lock(&chip->als_mutex);
+   ret = sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
+   mutex_unlock(&chip->als_mutex);
 
-   return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
+   return ret;
 }
 
 static ssize_t in_illuminance_input_target_store(struct device *dev,
@@ -528,7 +533,9 @@ static ssize_t in_illuminance_input_target_store(struct 
device *dev,
if (kstrtoint(buf, 0, &value) || !value)
return -EINVAL;
 
+   mutex_lock(&chip->als_mutex);
chip->taos_settings.als_cal_target = value;
+   mutex_unlock(&chip->als_mutex);
 
return len;
 }
@@ -538,12 +545,15 @@ static ssize_t in_illuminance_calibrate_store(struct 
device *dev,
  const char *buf, size_t len)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+   struct tsl2583_chip *chip = iio_priv(indio_dev);
int value;
 
if (kstrtoint(buf, 0, &value) || value != 1)
return -EINVAL;
 
+   mutex_lock(&chip->als_mutex);
taos_als_calibrate(indio_dev);
+   mutex_unlock(&chip->als_mutex);
 
return len;
 }
@@ -583,6 +593,8 @@ static ssize_t in_illuminance_lux_table_store(struct device 
*dev,
int value[ARRAY_SIZE(taos_device_lux) * 3 + 1];
int n, ret = -EINVAL;
 
+   mutex_lock(&chip->als_mutex);
+
get_options(buf, ARRAY_SIZE(value), value);
 
/* We now have an array of ints starting at value[1], and
@@ -617,6 +629,8 @@ static ssize_t in_illuminance_lux_table_store(struct device 
*dev,
ret = len;
 
 done:
+   mutex_unlock(&chip->als_mutex);
+
return ret;
 }
 
-- 
2.7.4

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


[PATCH 08/10] staging: iio: tsl2583: use IIO_*_ATTR* macros to create sysfs entries

2016-10-28 Thread Brian Masney
Use the IIO_CONST_ATTR, IIO_DEVICE_ATTR_RW, and IIO_DEVICE_ATTR_WO
macros for creating the in_illuminance_calibscale_available,
in_illuminance_integration_time_available, in_illuminance_input_target,
in_illuminance_calibrate, and in_illuminance_lux_table sysfs entries.
Previously these sysfs entries were prefixed with illuminance0_, however
they are now prefixed with in_illuminance_ to make these sysfs entries
consistent with how the IIO core is creating the other sysfs entries.

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/tsl2583.c | 73 ++---
 1 file changed, 27 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2583.c 
b/drivers/staging/iio/light/tsl2583.c
index bfff6ca..1462374 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define TSL258X_MAX_DEVICE_REGS32
 
@@ -506,24 +507,9 @@ static int taos_chip_off(struct iio_dev *indio_dev)
 
 /* Sysfs Interface Functions */
 
-static ssize_t taos_gain_available_show(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   return sprintf(buf, "%s\n", "1 8 16 111");
-}
-
-static ssize_t taos_als_time_available_show(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
-{
-   return sprintf(buf, "%s\n",
-   "0.50 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 
0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
-}
-
-static ssize_t taos_als_cal_target_show(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
+static ssize_t in_illuminance_input_target_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -531,9 +517,9 @@ static ssize_t taos_als_cal_target_show(struct device *dev,
return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
 }
 
-static ssize_t taos_als_cal_target_store(struct device *dev,
-struct device_attribute *attr,
-const char *buf, size_t len)
+static ssize_t in_illuminance_input_target_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t len)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -548,9 +534,9 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
return len;
 }
 
-static ssize_t taos_do_calibrate(struct device *dev,
-struct device_attribute *attr,
-const char *buf, size_t len)
+static ssize_t in_illuminance_calibrate_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
int value;
@@ -564,8 +550,9 @@ static ssize_t taos_do_calibrate(struct device *dev,
return len;
 }
 
-static ssize_t taos_luxtable_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t in_illuminance_lux_table_show(struct device *dev,
+struct device_attribute *attr,
+char *buf)
 {
int i;
int offset = 0;
@@ -589,9 +576,9 @@ static ssize_t taos_luxtable_show(struct device *dev,
return offset;
 }
 
-static ssize_t taos_luxtable_store(struct device *dev,
-  struct device_attribute *attr,
-  const char *buf, size_t len)
+static ssize_t in_illuminance_lux_table_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -635,25 +622,19 @@ static ssize_t taos_luxtable_store(struct device *dev,
return ret;
 }
 
-static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
-   taos_gain_available_show, NULL);
-
-static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
-   taos_als_time_available_show, NULL);
-
-static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
- 

[PATCH] staging: most: hdm-usb: add comment

2016-10-28 Thread Christian Gromm
This patch adds a comment to function hdm_configure_channel() to clarify
its execution paths as requested by Dan Carpenter.

Signed-off-by: Christian Gromm 
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c 
b/drivers/staging/most/hdm-usb/hdm_usb.c
index 3433646..d6db0bd 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -634,6 +634,15 @@ static int hdm_enqueue(struct most_interface *iface, int 
channel,
  * @iface: interface
  * @channel: channel ID
  * @conf: structure that holds the configuration information
+ *
+ * The attached network interface controller (NIC) supports a padding mode
+ * to avoid short packets on USB, hence increasing the performance due to a
+ * lower interrupt load. This mode is default for synchronous data and can
+ * be switched on for isochronous data. In case padding is active the
+ * driver needs to know the frame size of the payload in order to calculate
+ * the number of bytes it needs to pad when transmitting or to cut off when
+ * receiving data.
+ *
  */
 static int hdm_configure_channel(struct most_interface *iface, int channel,
 struct most_channel_config *conf)
@@ -667,6 +676,11 @@ static int hdm_configure_channel(struct most_interface 
*iface, int channel,
!(conf->data_type == MOST_CH_ISOC &&
  conf->packets_per_xact != 0xFF)) {
mdev->padding_active[channel] = false;
+   /*
+* Since the NIC's padding mode is not going to be
+* used, we can skip the frame size calculations and
+* move directly on to exit.
+*/
goto exit;
}
 
-- 
1.9.1

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


Re: [PATCH] staging: most: hdm-usb: add comment

2016-10-28 Thread Greg KH
On Fri, Oct 28, 2016 at 01:11:15PM +0200, Christian Gromm wrote:
> This patch adds a comment to function hdm_configure_channel() to clarify
> its execution paths as requested by Dan Carpenter.
> 
> Signed-off-by: Christian Gromm 

"reported-by:" tag please?

thanks,

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


[PATCH v2] staging: most: hdm-usb: add comment

2016-10-28 Thread Christian Gromm
This patch adds a comment to function hdm_configure_channel() to clarify
its execution paths.

Signed-off-by: Christian Gromm 
Reported-by: Dan Carpenter 
---
v2: added Reported-by tag

 drivers/staging/most/hdm-usb/hdm_usb.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c 
b/drivers/staging/most/hdm-usb/hdm_usb.c
index 3433646..d6db0bd 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -634,6 +634,15 @@ static int hdm_enqueue(struct most_interface *iface, int 
channel,
  * @iface: interface
  * @channel: channel ID
  * @conf: structure that holds the configuration information
+ *
+ * The attached network interface controller (NIC) supports a padding mode
+ * to avoid short packets on USB, hence increasing the performance due to a
+ * lower interrupt load. This mode is default for synchronous data and can
+ * be switched on for isochronous data. In case padding is active the
+ * driver needs to know the frame size of the payload in order to calculate
+ * the number of bytes it needs to pad when transmitting or to cut off when
+ * receiving data.
+ *
  */
 static int hdm_configure_channel(struct most_interface *iface, int channel,
 struct most_channel_config *conf)
@@ -667,6 +676,11 @@ static int hdm_configure_channel(struct most_interface 
*iface, int channel,
!(conf->data_type == MOST_CH_ISOC &&
  conf->packets_per_xact != 0xFF)) {
mdev->padding_active[channel] = false;
+   /*
+* Since the NIC's padding mode is not going to be
+* used, we can skip the frame size calculations and
+* move directly on to exit.
+*/
goto exit;
}
 
-- 
1.9.1

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


[PATCH] staging: vc04_services: tie up loose ends with dma_map_sg conversion

2016-10-28 Thread Michael Zoran
The conversion to dma_map_sg left a few loose ends.  This change
ties up those loose ends.

1. Settings the DMA mask is mandatory on 64 bit even though it
is optional on 32 bit.  Set the mask so that DMA space is always
in the lower 32 bit range of data on both platforms.

2. The scatterlist was not completely initialized correctly.
Initialize the list with sg_init_table so that DMA works correctly
if scatterlist debugging is enabled in the build configuration.

3. The error paths in create_pagelist were not consistent.  Make
them all consistent by calling a helper function called
cleanup_pagelistinfo to cleanup regardless of what state the pagelist
is in.

4. create_pagelist and free_pagelist had a very large amount of
duplication in computing offsets into a single allocation of memory
in the DMA area.  Introduce a new structure called the pagelistinfo
that is appened to the end of the allocation to store necessary
information to prevent duplication of code and make cleanup on errors
easier.

When combined with a fix for vchiq_copy_from_user which is not
included at this time, both functional and pings tests of vchiq_test
now pass in both 32 bit and 64 bit modes.

Even though this cleanup could have been broken down to chunks,
all the changes are to a single file and submitting it as a single
related change should make reviewing the diff much easier then if it
were submitted piecemeal.

Signed-off-by: Michael Zoran 
---
 .../interface/vchiq_arm/vchiq_2835_arm.c   | 239 +++--
 1 file changed, 129 insertions(+), 110 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index a5afcc5..06df77a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -62,6 +62,18 @@ typedef struct vchiq_2835_state_struct {
VCHIQ_ARM_STATE_T arm_state;
 } VCHIQ_2835_ARM_STATE_T;
 
+struct vchiq_pagelist_info {
+   PAGELIST_T *pagelist;
+   size_t pagelist_buffer_size;
+   dma_addr_t dma_addr;
+   enum dma_data_direction dma_dir;
+   unsigned int num_pages;
+   unsigned int pages_need_release;
+   struct page **pages;
+   struct scatterlist *scatterlist;
+   unsigned int scatterlist_mapped;
+};
+
 static void __iomem *g_regs;
 static unsigned int g_cache_line_size = sizeof(CACHE_LINE_SIZE);
 static unsigned int g_fragments_size;
@@ -77,13 +89,13 @@ static DEFINE_SEMAPHORE(g_free_fragments_mutex);
 static irqreturn_t
 vchiq_doorbell_irq(int irq, void *dev_id);
 
-static int
+static struct vchiq_pagelist_info *
 create_pagelist(char __user *buf, size_t count, unsigned short type,
-   struct task_struct *task, PAGELIST_T **ppagelist,
-   dma_addr_t *dma_addr);
+   struct task_struct *task);
 
 static void
-free_pagelist(dma_addr_t dma_addr, PAGELIST_T *pagelist, int actual);
+free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
+ int actual);
 
 int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
 {
@@ -97,6 +109,16 @@ int vchiq_platform_init(struct platform_device *pdev, 
VCHIQ_STATE_T *state)
int slot_mem_size, frag_mem_size;
int err, irq, i;
 
+   /*
+* Setting the DMA mask is necessary in the 64 bit environment.
+* It isn't necessary in a 32 bit environment, but is considered
+* a good practice.
+*/
+   err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+   if (err < 0)
+   return err;
+
(void)of_property_read_u32(dev->of_node, "cache-line-size",
   &g_cache_line_size);
g_fragments_size = 2 * g_cache_line_size;
@@ -226,29 +248,27 @@ VCHIQ_STATUS_T
 vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, VCHI_MEM_HANDLE_T memhandle,
void *offset, int size, int dir)
 {
-   PAGELIST_T *pagelist;
-   int ret;
-   dma_addr_t dma_addr;
+   struct vchiq_pagelist_info *pagelistinfo;
 
WARN_ON(memhandle != VCHI_MEM_HANDLE_INVALID);
 
-   ret = create_pagelist((char __user *)offset, size,
-   (dir == VCHIQ_BULK_RECEIVE)
-   ? PAGELIST_READ
-   : PAGELIST_WRITE,
-   current,
-   &pagelist,
-   &dma_addr);
+   pagelistinfo = create_pagelist((char __user *)offset, size,
+  (dir == VCHIQ_BULK_RECEIVE)
+  ? PAGELIST_READ
+  : PAGELIST_WRITE,
+  current);
 
-   if (ret != 0)
+   if (!pagelistinfo)
return VCHIQ_ERROR;
 
bulk->handle = memhandle;
-   bulk->data = (void *)(unsigned long)dma_addr;
+   bulk->data = (void *)(unsigned long)pagelistinfo->dma_a

Re: [PATCH] staging: vc04_services: tie up loose ends with dma_map_sg conversion

2016-10-28 Thread Greg KH
On Fri, Oct 28, 2016 at 08:16:51AM -0700, Michael Zoran wrote:
> The conversion to dma_map_sg left a few loose ends.  This change
> ties up those loose ends.
> 
> 1. Settings the DMA mask is mandatory on 64 bit even though it
> is optional on 32 bit.  Set the mask so that DMA space is always
> in the lower 32 bit range of data on both platforms.
> 
> 2. The scatterlist was not completely initialized correctly.
> Initialize the list with sg_init_table so that DMA works correctly
> if scatterlist debugging is enabled in the build configuration.
> 
> 3. The error paths in create_pagelist were not consistent.  Make
> them all consistent by calling a helper function called
> cleanup_pagelistinfo to cleanup regardless of what state the pagelist
> is in.
> 
> 4. create_pagelist and free_pagelist had a very large amount of
> duplication in computing offsets into a single allocation of memory
> in the DMA area.  Introduce a new structure called the pagelistinfo
> that is appened to the end of the allocation to store necessary
> information to prevent duplication of code and make cleanup on errors
> easier.
> 
> When combined with a fix for vchiq_copy_from_user which is not
> included at this time, both functional and pings tests of vchiq_test
> now pass in both 32 bit and 64 bit modes.
> 
> Even though this cleanup could have been broken down to chunks,
> all the changes are to a single file and submitting it as a single
> related change should make reviewing the diff much easier then if it
> were submitted piecemeal.

No, it's harder.  A patch should only do one type of thing, this patch
has to be reviewed thinking of 4 different things all at once, making it
much more difficult to do so.

We write patches to be read easily, and make them easy to review.  We
don't write them in a way to be easy for the developer to create :)

Can you please break this up into a patch series?

thanks,

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


Re: [PATCH] staging: vc04_services: tie up loose ends with dma_map_sg conversion

2016-10-28 Thread Michael Zoran
On Fri, 2016-10-28 at 11:31 -0400, Greg KH wrote:
> On Fri, Oct 28, 2016 at 08:16:51AM -0700, Michael Zoran wrote:
> > The conversion to dma_map_sg left a few loose ends.  This change
> > ties up those loose ends.
> > 
> > 1. Settings the DMA mask is mandatory on 64 bit even though it
> > is optional on 32 bit.  Set the mask so that DMA space is always
> > in the lower 32 bit range of data on both platforms.
> > 
> > 2. The scatterlist was not completely initialized correctly.
> > Initialize the list with sg_init_table so that DMA works correctly
> > if scatterlist debugging is enabled in the build configuration.
> > 
> > 3. The error paths in create_pagelist were not consistent.  Make
> > them all consistent by calling a helper function called
> > cleanup_pagelistinfo to cleanup regardless of what state the
> > pagelist
> > is in.
> > 
> > 4. create_pagelist and free_pagelist had a very large amount of
> > duplication in computing offsets into a single allocation of memory
> > in the DMA area.  Introduce a new structure called the pagelistinfo
> > that is appened to the end of the allocation to store necessary
> > information to prevent duplication of code and make cleanup on
> > errors
> > easier.
> > 
> > When combined with a fix for vchiq_copy_from_user which is not
> > included at this time, both functional and pings tests of
> > vchiq_test
> > now pass in both 32 bit and 64 bit modes.
> > 
> > Even though this cleanup could have been broken down to chunks,
> > all the changes are to a single file and submitting it as a single
> > related change should make reviewing the diff much easier then if
> > it
> > were submitted piecemeal.
> 
> No, it's harder.  A patch should only do one type of thing, this
> patch
> has to be reviewed thinking of 4 different things all at once, making
> it
> much more difficult to do so.
> 
> We write patches to be read easily, and make them easy to review.  We
> don't write them in a way to be easy for the developer to create :)
> 
> Can you please break this up into a patch series?
> 
> thanks,
> 
> greg k-h

Point #1 and #2 would be very easy to seperate.   Point #3 and #4 are
essentually a redo of two major functions and are where most of the
changes are.

Would making #1 and #2 independent but combining #3 and #4 sufficient?


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


Re: [PATCH] staging: vc04_services: tie up loose ends with dma_map_sg conversion

2016-10-28 Thread Greg KH
On Fri, Oct 28, 2016 at 08:36:34AM -0700, Michael Zoran wrote:
> On Fri, 2016-10-28 at 11:31 -0400, Greg KH wrote:
> > On Fri, Oct 28, 2016 at 08:16:51AM -0700, Michael Zoran wrote:
> > > The conversion to dma_map_sg left a few loose ends.  This change
> > > ties up those loose ends.
> > > 
> > > 1. Settings the DMA mask is mandatory on 64 bit even though it
> > > is optional on 32 bit.  Set the mask so that DMA space is always
> > > in the lower 32 bit range of data on both platforms.
> > > 
> > > 2. The scatterlist was not completely initialized correctly.
> > > Initialize the list with sg_init_table so that DMA works correctly
> > > if scatterlist debugging is enabled in the build configuration.
> > > 
> > > 3. The error paths in create_pagelist were not consistent.  Make
> > > them all consistent by calling a helper function called
> > > cleanup_pagelistinfo to cleanup regardless of what state the
> > > pagelist
> > > is in.
> > > 
> > > 4. create_pagelist and free_pagelist had a very large amount of
> > > duplication in computing offsets into a single allocation of memory
> > > in the DMA area.  Introduce a new structure called the pagelistinfo
> > > that is appened to the end of the allocation to store necessary
> > > information to prevent duplication of code and make cleanup on
> > > errors
> > > easier.
> > > 
> > > When combined with a fix for vchiq_copy_from_user which is not
> > > included at this time, both functional and pings tests of
> > > vchiq_test
> > > now pass in both 32 bit and 64 bit modes.
> > > 
> > > Even though this cleanup could have been broken down to chunks,
> > > all the changes are to a single file and submitting it as a single
> > > related change should make reviewing the diff much easier then if
> > > it
> > > were submitted piecemeal.
> > 
> > No, it's harder.  A patch should only do one type of thing, this
> > patch
> > has to be reviewed thinking of 4 different things all at once, making
> > it
> > much more difficult to do so.
> > 
> > We write patches to be read easily, and make them easy to review.  We
> > don't write them in a way to be easy for the developer to create :)
> > 
> > Can you please break this up into a patch series?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Point #1 and #2 would be very easy to seperate.   Point #3 and #4 are
> essentually a redo of two major functions and are where most of the
> changes are.
> 
> Would making #1 and #2 independent but combining #3 and #4 sufficient?

I don't know, try it and see what the patches look like.

Think about it from my point of view, which would be easier to review?

thanks,

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


Re: [PATCH] staging: vc04_services: tie up loose ends with dma_map_sg conversion

2016-10-28 Thread Michael Zoran
On Fri, 2016-10-28 at 11:42 -0400, Greg KH wrote:
> On Fri, Oct 28, 2016 at 08:36:34AM -0700, Michael Zoran wrote:
> > On Fri, 2016-10-28 at 11:31 -0400, Greg KH wrote:
> > > On Fri, Oct 28, 2016 at 08:16:51AM -0700, Michael Zoran wrote:
> > > > The conversion to dma_map_sg left a few loose ends.  This
> > > > change
> > > > ties up those loose ends.
> > > > 
> > > > 1. Settings the DMA mask is mandatory on 64 bit even though it
> > > > is optional on 32 bit.  Set the mask so that DMA space is
> > > > always
> > > > in the lower 32 bit range of data on both platforms.
> > > > 
> > > > 2. The scatterlist was not completely initialized correctly.
> > > > Initialize the list with sg_init_table so that DMA works
> > > > correctly
> > > > if scatterlist debugging is enabled in the build configuration.
> > > > 
> > > > 3. The error paths in create_pagelist were not
> > > > consistent.  Make
> > > > them all consistent by calling a helper function called
> > > > cleanup_pagelistinfo to cleanup regardless of what state the
> > > > pagelist
> > > > is in.
> > > > 
> > > > 4. create_pagelist and free_pagelist had a very large amount of
> > > > duplication in computing offsets into a single allocation of
> > > > memory
> > > > in the DMA area.  Introduce a new structure called the
> > > > pagelistinfo
> > > > that is appened to the end of the allocation to store necessary
> > > > information to prevent duplication of code and make cleanup on
> > > > errors
> > > > easier.
> > > > 
> > > > When combined with a fix for vchiq_copy_from_user which is not
> > > > included at this time, both functional and pings tests of
> > > > vchiq_test
> > > > now pass in both 32 bit and 64 bit modes.
> > > > 
> > > > Even though this cleanup could have been broken down to chunks,
> > > > all the changes are to a single file and submitting it as a
> > > > single
> > > > related change should make reviewing the diff much easier then
> > > > if
> > > > it
> > > > were submitted piecemeal.
> > > 
> > > No, it's harder.  A patch should only do one type of thing, this
> > > patch
> > > has to be reviewed thinking of 4 different things all at once,
> > > making
> > > it
> > > much more difficult to do so.
> > > 
> > > We write patches to be read easily, and make them easy to
> > > review.  We
> > > don't write them in a way to be easy for the developer to create
> > > :)
> > > 
> > > Can you please break this up into a patch series?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > Point #1 and #2 would be very easy to seperate.   Point #3 and #4
> > are
> > essentually a redo of two major functions and are where most of the
> > changes are.
> > 
> > Would making #1 and #2 independent but combining #3 and #4
> > sufficient?
> 
> I don't know, try it and see what the patches look like.
> 
> Think about it from my point of view, which would be easier to
> review?
> 
> thanks,
> 
> greg k-h

Greg, I totally agree with you here and I understand your point of
view.

I'm wondering if it would be best to have me reword the description to
say that I completely rewrote a section of the file.  And essentially
consider it a ground up rewrite rather then a change.

Eric had some complaints about the way that specific section of the
code is structured, so maybe a rewrite is best.

 

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


[PATCH] staging: dgnc: Fix lines longer than 80 chars.

2016-10-28 Thread Fernando Apesteguia
Done by either unindenting some comments or converting them to
multi line comments.

This fixes some checkpatch warnings.

Signed-off-by: Fernando Apesteguia 
---
 drivers/staging/dgnc/dgnc_neo.h | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.h b/drivers/staging/dgnc/dgnc_neo.h
index abddd48..97f0386 100644
--- a/drivers/staging/dgnc/dgnc_neo.h
+++ b/drivers/staging/dgnc/dgnc_neo.h
@@ -28,18 +28,20 @@
  /
 
 struct neo_uart_struct {
-   u8 txrx;/* WR  RHR/THR - Holding Reg */
+   u8 txrx;/* WR  RHR/THR - Holding Reg */
u8 ier; /* WR  IER - Interrupt Enable Reg */
-   u8 isr_fcr; /* WR  ISR/FCR - Interrupt Status Reg/Fifo 
Control Reg */
+   u8 isr_fcr; /* WR  ISR/FCR - Interrupt Status Reg/Fifo
+* Control Reg
+*/
u8 lcr; /* WR  LCR - Line Control Reg */
u8 mcr; /* WR  MCR - Modem Control Reg */
u8 lsr; /* WR  LSR - Line Status Reg */
u8 msr; /* WR  MSR - Modem Status Reg */
u8 spr; /* WR  SPR - Scratch Pad Reg */
-   u8 fctr;/* WR  FCTR - Feature Control Reg */
+   u8 fctr;/* WR  FCTR - Feature Control Reg */
u8 efr; /* WR  EFR - Enhanced Function Reg */
-   u8 tfifo;   /* WR  TXCNT/TXTRG - Transmit FIFO Reg */
-   u8 rfifo;   /* WR  RXCNT/RXTRG - Receive  FIFO Reg */
+   u8 tfifo;   /* WR  TXCNT/TXTRG - Transmit FIFO Reg */
+   u8 rfifo;   /* WR  RXCNT/RXTRG - Receive  FIFO Reg */
u8 xoffchar1;   /* WR  XOFF 1 - XOff Character 1 Reg */
u8 xoffchar2;   /* WR  XOFF 2 - XOff Character 2 Reg */
u8 xonchar1;/* WR  XON 1 - Xon Character 1 Reg */
@@ -108,7 +110,9 @@ struct neo_uart_struct {
 /* 17158 Extended IIR's */
 #define UART_17158_IIR_RDI_TIMEOUT 0x0C/* Receiver data TIMEOUT */
 #define UART_17158_IIR_XONXOFF 0x10/* Received an XON/XOFF char */
-#define UART_17158_IIR_HWFLOW_STATE_CHANGE 0x20/* CTS/DSR or RTS/DTR 
state change */
+#define UART_17158_IIR_HWFLOW_STATE_CHANGE 0x20/* CTS/DSR or RTS/DTR
+* state change
+*/
 #define UART_17158_IIR_FIFO_ENABLED0xC0/* 16550 FIFOs are Enabled */
 
 /*
@@ -119,8 +123,12 @@ struct neo_uart_struct {
 #define UART_17158_RXRDY_TIMEOUT   0x2 /* RX Ready Timeout */
 #define UART_17158_TXRDY   0x3 /* TX Ready */
 #define UART_17158_MSR 0x4 /* Modem State Change */
-#define UART_17158_TX_AND_FIFO_CLR 0x40/* Transmitter Holding Reg 
Empty */
-#define UART_17158_RX_FIFO_DATA_ERROR  0x80/* UART detected an RX FIFO 
Data error */
+#define UART_17158_TX_AND_FIFO_CLR 0x40/* Transmitter Holding
+* Reg Empty
+*/
+#define UART_17158_RX_FIFO_DATA_ERROR  0x80/* UART detected an RX FIFO
+* Data error
+*/
 
 /*
  * These are the EXTENDED definitions for the 17C158's Interrupt
@@ -132,8 +140,12 @@ struct neo_uart_struct {
 #define UART_17158_EFR_RTSDTR  0x40/* Auto RTS/DTR Flow Control Enable */
 #define UART_17158_EFR_CTSDSR  0x80/* Auto CTS/DSR Flow COntrol Enable */
 
-#define UART_17158_XOFF_DETECT 0x1 /* Indicates whether chip saw an 
incoming XOFF char  */
-#define UART_17158_XON_DETECT  0x2 /* Indicates whether chip saw an 
incoming XON char */
+#define UART_17158_XOFF_DETECT 0x1 /* Indicates whether chip saw an
+* incoming XOFF char
+*/
+#define UART_17158_XON_DETECT  0x2 /* Indicates whether chip saw an
+* incoming XON char
+*/
 
 #define UART_17158_IER_RSVD1   0x10/* Reserved by Exar */
 #define UART_17158_IER_XOFF0x20/* Xoff Interrupt Enable */
-- 
2.7.4

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


[PATCH] staging: dgnc: Fix multi-line comment alignment

2016-10-28 Thread Fernando Apesteguia
This fixes a checkpatch warning.

Also, change the line above so it is aligned to the others in the
same block.

Signed-off-by: Fernando Apesteguia 
---
 drivers/staging/dgnc/digi.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/dgnc/digi.h b/drivers/staging/dgnc/digi.h
index 5b983e6..4e36573 100644
--- a/drivers/staging/dgnc/digi.h
+++ b/drivers/staging/dgnc/digi.h
@@ -44,11 +44,11 @@
 #define DIGI_SETA  (('e' << 8) | 95)   /* Set params   */
 #define DIGI_SETAW (('e' << 8) | 96)   /* Drain & set params   */
 #define DIGI_SETAF (('e' << 8) | 97)   /* Drain, flush & set params */
-#define DIGI_GET_NI_INFO (('d' << 8) | 250) /* Non-intelligent state info */
-#define DIGI_LOOPBACK (('d' << 8) | 252) /*
-   * Enable/disable UART
-   * internal loopback
-   */
+#define DIGI_GET_NI_INFO (('d' << 8) | 250)/* Non-intelligent state info */
+#define DIGI_LOOPBACK (('d' << 8) | 252)   /*
+* Enable/disable UART
+* internal loopback
+*/
 #define DIGI_FAST  0x0002  /* Fast baud rates  */
 #define RTSPACE0x0004  /* RTS input flow control   
*/
 #define CTSPACE0x0008  /* CTS output flow control  
*/
-- 
2.7.4

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


[PATCH] staging: vc04_services: setup DMA and coherent mask

2016-10-28 Thread Michael Zoran
Setting the DMA mask is optional on 32 bit but
is mandatory on 64 bit.  Set the DMA mask and coherent
to force all DMA to be in the 32 bit address space.

This is considered a "good practice" and most drivers
already do this.

Signed-off-by: Michael Zoran 
---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index a5afcc5..6fa2b5a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -97,6 +97,16 @@ int vchiq_platform_init(struct platform_device *pdev, 
VCHIQ_STATE_T *state)
int slot_mem_size, frag_mem_size;
int err, irq, i;
 
+   /*
+* Setting the DMA mask is necessary in the 64 bit environment.
+* It isn't necessary in a 32 bit environment but is considered
+* a good practice.
+*/
+   err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+   if (err < 0)
+   return err;
+
(void)of_property_read_u32(dev->of_node, "cache-line-size",
   &g_cache_line_size);
g_fragments_size = 2 * g_cache_line_size;
-- 
2.10.1

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


[PATCH] staging: vc04_services: call sg_init_table to init scatterlist

2016-10-28 Thread Michael Zoran
Call the sg_init_table function to correctly initialze
the DMA scatterlist.  This function is required to completely
initialize the list and is mandatory if DMA debugging is
enabled in the build configuration.

One of the purposes of sg_init_table is to set
the magic "cookie" on each list element and ensure
the chain end is marked.

Signed-off-by: Michael Zoran 
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 6fa2b5a..21b26e5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -464,6 +464,12 @@ create_pagelist(char __user *buf, size_t count, unsigned 
short type,
pagelist->type = type;
pagelist->offset = offset;
 
+   /*
+* Initialize the scatterlist so that the magic cookie
+*  is filled if debugging is enabled
+*/
+   sg_init_table(scatterlist, num_pages);
+   /* Now set the pages for each scatterlist */
for (i = 0; i < num_pages; i++)
sg_set_page(scatterlist + i, pages[i], PAGE_SIZE, 0);
 
-- 
2.10.1

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


RE: [PATCH] Tools: hv: recover after hv_vss_daemon freeze times out

2016-10-28 Thread KY Srinivasan


> -Original Message-
> From: Michael Gissing [mailto:m...@faulpeltz.net]
> Sent: Thursday, October 13, 2016 2:27 PM
> To: Alex Ng (LIS) 
> Cc: KY Srinivasan ; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> vkuzn...@redhat.com; gre...@linuxfoundation.org
> Subject: [PATCH] Tools: hv: recover after hv_vss_daemon freeze times out
> 
> 
> If a FIFREEZE operation run by the hv_vss_daemon takes longer than the
> VSS_USERSPACE_TIMEOUT set in the hv_snapshot module, instead of exiting
> after a write failure, try to recover by reopening the hv_vss device and
> performing the initial handshake again. Exiting causes all subsequent VSS
> operations sent by the Hyper-V host to fail until the daemon is restarted.
> 
> Signed-off-by: Michael Gissing 
> 
> ---
>   tools/hv/hv_vss_daemon.c | 11 ++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
> index 5d51d6f..0ecbdab 100644
> --- a/tools/hv/hv_vss_daemon.c
> +++ b/tools/hv/hv_vss_daemon.c
> @@ -176,6 +176,7 @@ int main(int argc, char *argv[])
>   openlog("Hyper-V VSS", 0, LOG_USER);
>   syslog(LOG_INFO, "VSS starting; pid is:%d", getpid());
> 
> +recover:
>   vss_fd = open("/dev/vmbus/hv_vss", O_RDWR);
>   if (vss_fd < 0) {
>   syslog(LOG_ERR, "open /dev/vmbus/hv_vss failed; error: %d %s",
> @@ -196,6 +197,7 @@ int main(int argc, char *argv[])
>   }
> 
>   pfd.fd = vss_fd;
> +in_handshake = 1;
> 
>   while (1) {
>   pfd.events = POLLIN;
> @@ -258,7 +260,14 @@ int main(int argc, char *argv[])
>   if (len != sizeof(struct hv_vss_msg)) {
>   syslog(LOG_ERR, "write failed; error: %d %s", errno,
>  strerror(errno));
> -exit(EXIT_FAILURE);
> +/*
> + * try to recover from possible timeout by THAWing
> + * and restarting the message loop
> +*/
> +vss_operate(VSS_OP_THAW);
> +close(vss_fd);
> +syslog(LOG_INFO, "trying to recover VSS connection");
> +goto recover;
>   }
>   }

I agree with issuing a THAW command when we timeout in the kernel as this would 
leave
the file system in a sane state. That said, I am not sure why we need to close 
the fd and reinitialize
everything in the daemon. What if we just ignored the write error and go back 
to wait for new commands
from the host.

Regards,

K. Y  
> 
> --
> 2.7.4
> 

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


Re: [PATCH] Staging: ks7010: ks7010_stio: Fixed several coding style issues

2016-10-28 Thread Greg KH
On Sat, Oct 29, 2016 at 03:48:17AM +0530, Manoj Sawai wrote:
> Fixed all the "errors" reported by checkpath.pl in ks7010_stio.c

"all"?  Please break this up into one-patch-per-thing series of patches.
And no, as my patchbot told you earlier today, "all coding style issues"
is not one thing.

thanks,

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


[PATCH] Staging: ks7010: ks7010_stio: Fixed several coding style issues

2016-10-28 Thread Manoj Sawai
Fixed all the "errors" reported by checkpath.pl in ks7010_stio.c
The "TODO" file ask to take the 80 character limit lightly, so
the file still has some warnings about character limit. All the
errors have been removed.

Signed-off-by: Manoj Sawai 
---
 drivers/staging/ks7010/ks7010_sdio.c | 60 
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 81c46f4..6bbfda4 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -35,18 +35,17 @@ MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
 /* macro */
 
 #define inc_txqhead(priv) \
-(priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE)
+   (priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE)
 #define inc_txqtail(priv) \
-(priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE)
+   (priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE)
 #define cnt_txqbody(priv) \
-(((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % 
TX_DEVICE_BUFF_SIZE)
-
+   (((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % 
TX_DEVICE_BUFF_SIZE)
 #define inc_rxqhead(priv) \
-(priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE)
+   (priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE)
 #define inc_rxqtail(priv) \
-(priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE)
+   (priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE)
 #define cnt_rxqbody(priv) \
-(((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % 
RX_DEVICE_BUFF_SIZE)
+   (((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % 
RX_DEVICE_BUFF_SIZE)
 
 static int ks7010_sdio_read(struct ks_wlan_private *priv, unsigned int address,
unsigned char *buffer, int length)
@@ -190,9 +189,9 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private 
*priv)

atomic_read(&priv->psstatus.snooze_guard),
cnt_txqbody(priv));
 
-   if (!atomic_read(&priv->psstatus.confirm_wait)
-   && 
!atomic_read(&priv->psstatus.snooze_guard)
-   && !cnt_txqbody(priv)) {
+   if (!atomic_read(&priv->psstatus.confirm_wait) 
&&
+   !atomic_read(&priv->psstatus.snooze_guard) 
&&
+   !cnt_txqbody(priv)) {
retval =
ks7010_sdio_read(priv, INT_PENDING,
 &rw_data,
@@ -255,7 +254,7 @@ int ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 
 static int enqueue_txdev(struct ks_wlan_private *priv, unsigned char *p,
 unsigned long size,
-void (*complete_handler) (void *arg1, void *arg2),
+void (*complete_handler)(void *arg1, void *arg2),
 void *arg1, void *arg2)
 {
struct tx_device_buffer *sp;
@@ -294,6 +293,7 @@ static int write_to_device(struct ks_wlan_private *priv, 
unsigned char *buffer,
int retval;
unsigned char rw_data;
struct hostif_hdr *hdr;
+
hdr = (struct hostif_hdr *)buffer;
 
DPRINTK(4, "size=%d\n", hdr->size);
@@ -326,8 +326,8 @@ static void tx_device_task(void *dev)
int rc = 0;
 
DPRINTK(4, "\n");
-   if (cnt_txqbody(priv) > 0
-   && atomic_read(&priv->psstatus.status) != PS_SNOOZE) {
+   if (cnt_txqbody(priv) > 0 &&
+   atomic_read(&priv->psstatus.status) != PS_SNOOZE) {
sp = &priv->tx_dev.tx_dev_buff[priv->tx_dev.qhead];
if (priv->dev_state >= DEVICE_STATE_BOOT) {
rc = write_to_device(priv, sp->sendp, sp->size);
@@ -353,11 +353,12 @@ static void tx_device_task(void *dev)
 }
 
 int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size,
- void (*complete_handler) (void *arg1, void *arg2),
+ void (*complete_handler)(void *arg1, void *arg2),
  void *arg1, void *arg2)
 {
int result = 0;
struct hostif_hdr *hdr;
+
hdr = (struct hostif_hdr *)p;
 
if (hdr->event < HIF_DATA_REQ || HIF_REQ_MAX < hdr->event) {
@@ -412,7 +413,7 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
/* receive data */
if (cnt_rxqbody(priv) >= (RX_DEVICE_BUFF_SIZE - 1)) {
/* in case of buffer overflow */
-   DPRINTK(1, "rx buffer overflow \n");
+   DPRINTK(1, "rx buffer overflow\n");
goto error_out;
}
rx_buffer = &priv->

[PATCH 1/3] staging: rtl8192e: Standardize test for NULL.

2016-10-28 Thread Elise Lennion
The test for NULL of the return variable of functions was changed from
(ret == NULL) to !ret to match the standard.

Coccinelle was used with semantic patch:
@@
expression e;
identifier id, f;
statement S;
@@

f(...) { <+...

id =
\(kmalloc\|devm_kzalloc\|kmalloc_array\|devm_ioremap
\|usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\) (...)

... when any
when != id = e

+ if (!id)
- if (\(NULL == id\|id == NULL\))
S

...+> }

Signed-off-by: Elise Lennion 
---
 drivers/staging/rtl8192e/rtl8192e/r8192E_cmdpkt.c | 2 +-
 drivers/staging/rtl8192e/rtllib_rx.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_cmdpkt.c 
b/drivers/staging/rtl8192e/rtl8192e/r8192E_cmdpkt.c
index f9003a2..757ffd4 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_cmdpkt.c
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_cmdpkt.c
@@ -49,7 +49,7 @@ bool rtl92e_send_cmd_pkt(struct net_device *dev, u32 type, 
const void *data,
else
skb = dev_alloc_skb(frag_length + 4);
 
-   if (skb == NULL) {
+   if (!skb) {
rt_status = false;
goto Failed;
}
diff --git a/drivers/staging/rtl8192e/rtllib_rx.c 
b/drivers/staging/rtl8192e/rtllib_rx.c
index d6777ec..d67e3f3 100644
--- a/drivers/staging/rtl8192e/rtllib_rx.c
+++ b/drivers/staging/rtl8192e/rtllib_rx.c
@@ -130,7 +130,7 @@ rtllib_frag_cache_get(struct rtllib_device *ieee,
ETH_ALEN /* WDS */ +
/* QOS Control */
(RTLLIB_QOS_HAS_SEQ(fc) ? 2 : 0));
-   if (skb == NULL)
+   if (!skb)
return NULL;
 
entry = &ieee->frag_cache[tid][ieee->frag_next_idx[tid]];
@@ -1430,7 +1430,7 @@ static int rtllib_rx_InfraAdhoc(struct rtllib_device 
*ieee, struct sk_buff *skb,
/* skb: hdr + (possible reassembled) full plaintext payload */
payload = skb->data + hdrlen;
rxb = kmalloc(sizeof(struct rtllib_rxb), GFP_ATOMIC);
-   if (rxb == NULL)
+   if (!rxb)
goto rx_dropped;
 
/* to parse amsdu packets */
-- 
2.7.4

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


[PATCH 2/3] staging: rtl8192u: Standardize test for NULL.

2016-10-28 Thread Elise Lennion
The test for NULL of the return variable of functions was changed from
(ret == NULL) to !ret to match the standard.

Coccinelle was used with semantic patch:
@@
expression e;
identifier id, f;
statement S;
@@

f(...) { <+...

id =
\(kmalloc\|devm_kzalloc\|kmalloc_array\|devm_ioremap
\|usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\) (...)

... when any
when != id = e

+ if (!id)
- if (\(NULL == id\|id == NULL\))
S

...+> }

Signed-off-by: Elise Lennion 
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 2e4d2d7..82f6543 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -129,7 +129,7 @@ ieee80211_frag_cache_get(struct ieee80211_device *ieee,
8 /* WEP */ +
ETH_ALEN /* WDS */ +
(IEEE80211_QOS_HAS_SEQ(fc)?2:0) /* QOS 
Control */);
-   if (skb == NULL)
+   if (!skb)
return NULL;
 
entry = &ieee->frag_cache[tid][ieee->frag_next_idx[tid]];
-- 
2.7.4

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


[PATCH 3/3] staging: rtl8188eu: Standardize test for NULL.

2016-10-28 Thread Elise Lennion
The test for NULL of the return variable of functions was changed from
(ret == NULL) to !ret to match the standard.

Coccinelle was used with semantic patch:
@@
expression e;
identifier id, f;
statement S;
@@

f(...) { <+...

id =
\(kmalloc\|devm_kzalloc\|kmalloc_array\|devm_ioremap
\|usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\) (...)

... when any
when != id = e

+ if (!id)
- if (\(NULL == id\|id == NULL\))
S

...+> }

Signed-off-by: Elise Lennion 
---
 drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c 
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index 34198fe..e2dbe1b 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -252,7 +252,7 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8 
request, u16 value, u16 i
/*  Acquire IO memory for vendorreq */
pIo_buf = kmalloc(MAX_USB_IO_CTL_SIZE, GFP_ATOMIC);
 
-   if (pIo_buf == NULL) {
+   if (!pIo_buf) {
DBG_88E("[%s] pIo_buf == NULL\n", __func__);
status = -ENOMEM;
goto release_mutex;
-- 
2.7.4

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