[PATCH v3] staging: iio: cdc: ad7746: add additional config defines
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
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()
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
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
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}
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
> -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
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
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.
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.
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.
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