Re: [PATCH] iio: remove unnecessary condition judgment in am2315_trigger_handler
On 9/8/2018 7:17 AM, Jonathan Cameron wrote:
> On Sat, 8 Sep 2018 17:59:13 +0530
> Himanshu Jha wrote:
>
>> On Sat, Sep 08, 2018 at 06:57:36PM +0800, zhong jiang wrote:
>>> The iterator in for_each_set_bit is never null, therefore, remove
>>> the redundant conditional judgment.
>>>
>>> Signed-off-by: zhong jiang
>>> ---
>>> drivers/iio/humidity/am2315.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iio/humidity/am2315.c b/drivers/iio/humidity/am2315.c
>>> index 7d8669d..dc12e37 100644
>>> --- a/drivers/iio/humidity/am2315.c
>>> +++ b/drivers/iio/humidity/am2315.c
>>> @@ -176,8 +176,7 @@ static irqreturn_t am2315_trigger_handler(int irq, void
>>> *p)
>>> i = 0;
>>> for_each_set_bit(bit, indio_dev->active_scan_mask,
>>> indio_dev->masklength) {
>>> - data->buffer[i] = (bit ? sensor_data.temp_data :
>>> -sensor_data.hum_data);
>>> + data->buffer[i] = sensor_data.temp_data;
>>
>> No, this seems wrong!
>>
>> We have buffer support to either take both readings(temp & humid)
>> simultaneously, or only single channel using specified scan mask.
>
> Key think is that bit most definitely can be 0 if the 0th bit is set.
> This isn't a null check at all.
>
> I'm curious, was this a by inspection case or did some script throw
> this one up?
Firstly, +1 on the patch in this thread being an incorrect change.
While inspecting the surrounding code, I noticed that there's a bit of
questionable code in this area. I believe this whole chunk:
if (*(indio_dev->active_scan_mask) == AM2315_ALL_CHANNEL_MASK) {
data->buffer[0] = sensor_data.hum_data;
data->buffer[1] = sensor_data.temp_data;
} else {
i = 0;
for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength) {
data->buffer[i] = (bit ? sensor_data.temp_data :
sensor_data.hum_data);
i++;
}
}
could be reduced to this:
for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength)
data->buffer[bit] = (bit ? sensor_data.temp_data :
sensor_data.hum_data);
The if/else structure seems like an unnecessary optimization.
Thoughts?
Re: [PATCH] iio: remove unnecessary condition judgment in am2315_trigger_handler
On 9/8/2018 7:17 AM, Jonathan Cameron wrote:
> On Sat, 8 Sep 2018 17:59:13 +0530
> Himanshu Jha wrote:
>
>> On Sat, Sep 08, 2018 at 06:57:36PM +0800, zhong jiang wrote:
>>> The iterator in for_each_set_bit is never null, therefore, remove
>>> the redundant conditional judgment.
>>>
>>> Signed-off-by: zhong jiang
>>> ---
>>> drivers/iio/humidity/am2315.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iio/humidity/am2315.c b/drivers/iio/humidity/am2315.c
>>> index 7d8669d..dc12e37 100644
>>> --- a/drivers/iio/humidity/am2315.c
>>> +++ b/drivers/iio/humidity/am2315.c
>>> @@ -176,8 +176,7 @@ static irqreturn_t am2315_trigger_handler(int irq, void
>>> *p)
>>> i = 0;
>>> for_each_set_bit(bit, indio_dev->active_scan_mask,
>>> indio_dev->masklength) {
>>> - data->buffer[i] = (bit ? sensor_data.temp_data :
>>> -sensor_data.hum_data);
>>> + data->buffer[i] = sensor_data.temp_data;
>>
>> No, this seems wrong!
>>
>> We have buffer support to either take both readings(temp & humid)
>> simultaneously, or only single channel using specified scan mask.
>
> Key think is that bit most definitely can be 0 if the 0th bit is set.
> This isn't a null check at all.
>
> I'm curious, was this a by inspection case or did some script throw
> this one up?
Firstly, +1 on the patch in this thread being an incorrect change.
While inspecting the surrounding code, I noticed that there's a bit of
questionable code in this area. I believe this whole chunk:
if (*(indio_dev->active_scan_mask) == AM2315_ALL_CHANNEL_MASK) {
data->buffer[0] = sensor_data.hum_data;
data->buffer[1] = sensor_data.temp_data;
} else {
i = 0;
for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength) {
data->buffer[i] = (bit ? sensor_data.temp_data :
sensor_data.hum_data);
i++;
}
}
could be reduced to this:
for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength)
data->buffer[bit] = (bit ? sensor_data.temp_data :
sensor_data.hum_data);
The if/else structure seems like an unnecessary optimization.
Thoughts?
[PATCH v2 2/2] regmap: split up regmap_config.use_single_rw
Split regmap_config.use_single_rw into use_single_read and
use_single_write. This change enables drivers of devices which only
support bulk operations in one direction to use the regmap_bulk_*()
functions for both directions and have their bulk operation split into
single operations only when necessary.
Update all struct regmap_config instances where use_single_rw==true to
instead set both use_single_read and use_single_write. No attempt was
made to evaluate whether it is possible to set only one of
use_single_read or use_single_write.
Signed-off-by: David Frey
---
drivers/base/regmap/regmap.c | 4 ++--
drivers/edac/altera_edac.c | 3 ++-
drivers/hwmon/lm75.c | 3 ++-
drivers/hwmon/lm95245.c | 3 ++-
drivers/hwmon/tmp102.c | 3 ++-
drivers/hwmon/tmp108.c | 3 ++-
drivers/iio/light/apds9960.c | 3 ++-
drivers/iio/light/max44000.c | 23 ---
drivers/iio/temperature/mlx90632.c | 3 ++-
drivers/input/touchscreen/tsc200x-core.c | 3 ++-
drivers/mfd/altera-a10sr.c | 3 ++-
drivers/mfd/da9052-spi.c | 3 ++-
drivers/mfd/mc13xxx-spi.c| 3 ++-
drivers/mfd/twl6040.c| 3 ++-
drivers/regulator/ltc3589.c | 3 ++-
drivers/regulator/ltc3676.c | 3 ++-
include/linux/regmap.h | 12
sound/hda/hdac_regmap.c | 3 ++-
sound/soc/codecs/cs35l33.c | 3 ++-
sound/soc/codecs/cs35l35.c | 3 ++-
sound/soc/codecs/cs43130.c | 4 +++-
sound/soc/codecs/es8328.c| 3 ++-
sound/soc/codecs/rt1305.c| 3 ++-
sound/soc/codecs/rt5514.c| 3 ++-
sound/soc/codecs/rt5616.c| 3 ++-
sound/soc/codecs/rt5640.c| 3 ++-
sound/soc/codecs/rt5645.c| 9 ++---
sound/soc/codecs/rt5651.c| 3 ++-
sound/soc/codecs/rt5660.c| 3 ++-
sound/soc/codecs/rt5663.c| 9 ++---
sound/soc/codecs/rt5665.c| 3 ++-
sound/soc/codecs/rt5668.c| 3 ++-
sound/soc/codecs/rt5670.c| 3 ++-
sound/soc/codecs/rt5682.c| 3 ++-
34 files changed, 93 insertions(+), 52 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 0360a90ad6b6..78a778c08f92 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -762,8 +762,8 @@ struct regmap *__regmap_init(struct device *dev,
map->reg_stride_order = ilog2(map->reg_stride);
else
map->reg_stride_order = -1;
- map->use_single_read = config->use_single_rw || !bus || !bus->read;
- map->use_single_write = config->use_single_rw || !bus || !bus->write;
+ map->use_single_read = config->use_single_read || !bus || !bus->read;
+ map->use_single_write = config->use_single_write || !bus || !bus->write;
map->can_multi_write = config->can_multi_write && bus && bus->write;
if (bus) {
map->max_raw_read = bus->max_raw_read;
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 5762c3c383f2..ab7c5a937ab0 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -599,7 +599,8 @@ static const struct regmap_config s10_sdram_regmap_cfg = {
.volatile_reg = s10_sdram_volatile_reg,
.reg_read = s10_protected_reg_read,
.reg_write = s10_protected_reg_write,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int altr_s10_sdram_probe(struct platform_device *pdev)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 49f4b33a5685..542dc4058831 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -254,7 +254,8 @@ static const struct regmap_config lm75_regmap_config = {
.volatile_reg = lm75_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static void lm75_remove(void *data)
diff --git a/drivers/hwmon/lm95245.c b/drivers/hwmon/lm95245.c
index 27cb06d65594..996b50246175 100644
--- a/drivers/hwmon/lm95245.c
+++ b/drivers/hwmon/lm95245.c
@@ -541,7 +541,8 @@ static const struct regmap_config lm95245_regmap_config = {
.writeable_reg = lm95245_is_writeable_reg,
.volatile_reg = lm95245_is_volatile_reg,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static const u32 lm95245_chip_config[] = {
diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c
index dfc40c740d07..6
[PATCH v2 1/2] regmap: fix comment for regmap.use_single_write
Signed-off-by: David Frey
---
drivers/base/regmap/internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a6bf34d6394e..16414ccace96 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -149,7 +149,7 @@ struct regmap {
/* if set, converts bulk read to single read */
bool use_single_read;
- /* if set, converts bulk read to single read */
+ /* if set, converts bulk write to single write */
bool use_single_write;
/* if set, the device supports multi write mode */
bool can_multi_write;
--
2.11.0
regmap: split regmap_config.use_single_rw
This patch series splits regmap_config.use_single_rw into use_single_read and use_single_write. Motivation: When multiple sequential registers need to be read or written, the author can either choose to call regmap_bulk_read/write() once or call regmap_read/write() repeatedly. The _bulk option has the advantages of being more compact since only one function call is needed and more efficient since multiple registers are read/written at once instead of having to pay for transaction and addressing overhead of the underlying transport multiple times. Many chips don't support bulk access, but it's still convenient to be able to read/write multiple registers with a single function call. This is what use_single_rw=true is for. When this option is set, bulk reads/writes are automatically split into a series of single register accesses. Some other chips (for example the Bosch bmi160 IMU) support bulk reads, but not bulk writes. Currently for chips like this, the author must choose between convenience or efficiency by setting use_single_rw = true/false respectively. Proposal: In this patch series I split the use_single_rw member of struct regmap_config into use_single_read and use_single_write. This change is in line with what is already implemented inside of the internal struct regmap which is populated from struct regmap_config. Updates: v2 * Combined patch to split use_single_rw apart and the patch to update the clients into a single patch so that kbuild test robot will be happy. David Frey (2): regmap: fix comment for regmap.use_single_write regmap: split up regmap_config.use_single_rw drivers/base/regmap/internal.h | 2 +- drivers/base/regmap/regmap.c | 4 ++-- drivers/edac/altera_edac.c | 3 ++- drivers/hwmon/lm75.c | 3 ++- drivers/hwmon/lm95245.c | 3 ++- drivers/hwmon/tmp102.c | 3 ++- drivers/hwmon/tmp108.c | 3 ++- drivers/iio/light/apds9960.c | 3 ++- drivers/iio/light/max44000.c | 23 --- drivers/iio/temperature/mlx90632.c | 3 ++- drivers/input/touchscreen/tsc200x-core.c | 3 ++- drivers/mfd/altera-a10sr.c | 3 ++- drivers/mfd/da9052-spi.c | 3 ++- drivers/mfd/mc13xxx-spi.c| 3 ++- drivers/mfd/twl6040.c| 3 ++- drivers/regulator/ltc3589.c | 3 ++- drivers/regulator/ltc3676.c | 3 ++- include/linux/regmap.h | 12 sound/hda/hdac_regmap.c | 3 ++- sound/soc/codecs/cs35l33.c | 3 ++- sound/soc/codecs/cs35l35.c | 3 ++- sound/soc/codecs/cs43130.c | 4 +++- sound/soc/codecs/es8328.c| 3 ++- sound/soc/codecs/rt1305.c| 3 ++- sound/soc/codecs/rt5514.c| 3 ++- sound/soc/codecs/rt5616.c| 3 ++- sound/soc/codecs/rt5640.c| 3 ++- sound/soc/codecs/rt5645.c| 9 ++--- sound/soc/codecs/rt5651.c| 3 ++- sound/soc/codecs/rt5660.c| 3 ++- sound/soc/codecs/rt5663.c| 9 ++--- sound/soc/codecs/rt5665.c| 3 ++- sound/soc/codecs/rt5668.c| 3 ++- sound/soc/codecs/rt5670.c| 3 ++- sound/soc/codecs/rt5682.c| 3 ++- 35 files changed, 94 insertions(+), 53 deletions(-) -- 2.11.0
[PATCH v2 2/2] regmap: split up regmap_config.use_single_rw
Split regmap_config.use_single_rw into use_single_read and
use_single_write. This change enables drivers of devices which only
support bulk operations in one direction to use the regmap_bulk_*()
functions for both directions and have their bulk operation split into
single operations only when necessary.
Update all struct regmap_config instances where use_single_rw==true to
instead set both use_single_read and use_single_write. No attempt was
made to evaluate whether it is possible to set only one of
use_single_read or use_single_write.
Signed-off-by: David Frey
---
drivers/base/regmap/regmap.c | 4 ++--
drivers/edac/altera_edac.c | 3 ++-
drivers/hwmon/lm75.c | 3 ++-
drivers/hwmon/lm95245.c | 3 ++-
drivers/hwmon/tmp102.c | 3 ++-
drivers/hwmon/tmp108.c | 3 ++-
drivers/iio/light/apds9960.c | 3 ++-
drivers/iio/light/max44000.c | 23 ---
drivers/iio/temperature/mlx90632.c | 3 ++-
drivers/input/touchscreen/tsc200x-core.c | 3 ++-
drivers/mfd/altera-a10sr.c | 3 ++-
drivers/mfd/da9052-spi.c | 3 ++-
drivers/mfd/mc13xxx-spi.c| 3 ++-
drivers/mfd/twl6040.c| 3 ++-
drivers/regulator/ltc3589.c | 3 ++-
drivers/regulator/ltc3676.c | 3 ++-
include/linux/regmap.h | 12
sound/hda/hdac_regmap.c | 3 ++-
sound/soc/codecs/cs35l33.c | 3 ++-
sound/soc/codecs/cs35l35.c | 3 ++-
sound/soc/codecs/cs43130.c | 4 +++-
sound/soc/codecs/es8328.c| 3 ++-
sound/soc/codecs/rt1305.c| 3 ++-
sound/soc/codecs/rt5514.c| 3 ++-
sound/soc/codecs/rt5616.c| 3 ++-
sound/soc/codecs/rt5640.c| 3 ++-
sound/soc/codecs/rt5645.c| 9 ++---
sound/soc/codecs/rt5651.c| 3 ++-
sound/soc/codecs/rt5660.c| 3 ++-
sound/soc/codecs/rt5663.c| 9 ++---
sound/soc/codecs/rt5665.c| 3 ++-
sound/soc/codecs/rt5668.c| 3 ++-
sound/soc/codecs/rt5670.c| 3 ++-
sound/soc/codecs/rt5682.c| 3 ++-
34 files changed, 93 insertions(+), 52 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 0360a90ad6b6..78a778c08f92 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -762,8 +762,8 @@ struct regmap *__regmap_init(struct device *dev,
map->reg_stride_order = ilog2(map->reg_stride);
else
map->reg_stride_order = -1;
- map->use_single_read = config->use_single_rw || !bus || !bus->read;
- map->use_single_write = config->use_single_rw || !bus || !bus->write;
+ map->use_single_read = config->use_single_read || !bus || !bus->read;
+ map->use_single_write = config->use_single_write || !bus || !bus->write;
map->can_multi_write = config->can_multi_write && bus && bus->write;
if (bus) {
map->max_raw_read = bus->max_raw_read;
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 5762c3c383f2..ab7c5a937ab0 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -599,7 +599,8 @@ static const struct regmap_config s10_sdram_regmap_cfg = {
.volatile_reg = s10_sdram_volatile_reg,
.reg_read = s10_protected_reg_read,
.reg_write = s10_protected_reg_write,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int altr_s10_sdram_probe(struct platform_device *pdev)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 49f4b33a5685..542dc4058831 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -254,7 +254,8 @@ static const struct regmap_config lm75_regmap_config = {
.volatile_reg = lm75_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static void lm75_remove(void *data)
diff --git a/drivers/hwmon/lm95245.c b/drivers/hwmon/lm95245.c
index 27cb06d65594..996b50246175 100644
--- a/drivers/hwmon/lm95245.c
+++ b/drivers/hwmon/lm95245.c
@@ -541,7 +541,8 @@ static const struct regmap_config lm95245_regmap_config = {
.writeable_reg = lm95245_is_writeable_reg,
.volatile_reg = lm95245_is_volatile_reg,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static const u32 lm95245_chip_config[] = {
diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c
index dfc40c740d07..6
[PATCH v2 1/2] regmap: fix comment for regmap.use_single_write
Signed-off-by: David Frey
---
drivers/base/regmap/internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a6bf34d6394e..16414ccace96 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -149,7 +149,7 @@ struct regmap {
/* if set, converts bulk read to single read */
bool use_single_read;
- /* if set, converts bulk read to single read */
+ /* if set, converts bulk write to single write */
bool use_single_write;
/* if set, the device supports multi write mode */
bool can_multi_write;
--
2.11.0
regmap: split regmap_config.use_single_rw
This patch series splits regmap_config.use_single_rw into use_single_read and use_single_write. Motivation: When multiple sequential registers need to be read or written, the author can either choose to call regmap_bulk_read/write() once or call regmap_read/write() repeatedly. The _bulk option has the advantages of being more compact since only one function call is needed and more efficient since multiple registers are read/written at once instead of having to pay for transaction and addressing overhead of the underlying transport multiple times. Many chips don't support bulk access, but it's still convenient to be able to read/write multiple registers with a single function call. This is what use_single_rw=true is for. When this option is set, bulk reads/writes are automatically split into a series of single register accesses. Some other chips (for example the Bosch bmi160 IMU) support bulk reads, but not bulk writes. Currently for chips like this, the author must choose between convenience or efficiency by setting use_single_rw = true/false respectively. Proposal: In this patch series I split the use_single_rw member of struct regmap_config into use_single_read and use_single_write. This change is in line with what is already implemented inside of the internal struct regmap which is populated from struct regmap_config. Updates: v2 * Combined patch to split use_single_rw apart and the patch to update the clients into a single patch so that kbuild test robot will be happy. David Frey (2): regmap: fix comment for regmap.use_single_write regmap: split up regmap_config.use_single_rw drivers/base/regmap/internal.h | 2 +- drivers/base/regmap/regmap.c | 4 ++-- drivers/edac/altera_edac.c | 3 ++- drivers/hwmon/lm75.c | 3 ++- drivers/hwmon/lm95245.c | 3 ++- drivers/hwmon/tmp102.c | 3 ++- drivers/hwmon/tmp108.c | 3 ++- drivers/iio/light/apds9960.c | 3 ++- drivers/iio/light/max44000.c | 23 --- drivers/iio/temperature/mlx90632.c | 3 ++- drivers/input/touchscreen/tsc200x-core.c | 3 ++- drivers/mfd/altera-a10sr.c | 3 ++- drivers/mfd/da9052-spi.c | 3 ++- drivers/mfd/mc13xxx-spi.c| 3 ++- drivers/mfd/twl6040.c| 3 ++- drivers/regulator/ltc3589.c | 3 ++- drivers/regulator/ltc3676.c | 3 ++- include/linux/regmap.h | 12 sound/hda/hdac_regmap.c | 3 ++- sound/soc/codecs/cs35l33.c | 3 ++- sound/soc/codecs/cs35l35.c | 3 ++- sound/soc/codecs/cs43130.c | 4 +++- sound/soc/codecs/es8328.c| 3 ++- sound/soc/codecs/rt1305.c| 3 ++- sound/soc/codecs/rt5514.c| 3 ++- sound/soc/codecs/rt5616.c| 3 ++- sound/soc/codecs/rt5640.c| 3 ++- sound/soc/codecs/rt5645.c| 9 ++--- sound/soc/codecs/rt5651.c| 3 ++- sound/soc/codecs/rt5660.c| 3 ++- sound/soc/codecs/rt5663.c| 9 ++--- sound/soc/codecs/rt5665.c| 3 ++- sound/soc/codecs/rt5668.c| 3 ++- sound/soc/codecs/rt5670.c| 3 ++- sound/soc/codecs/rt5682.c| 3 ++- 35 files changed, 94 insertions(+), 53 deletions(-) -- 2.11.0
[PATCH 0/3] regmap: split regmap_config.use_single_rw
This patch series splits regmap_config.use_single_rw into use_single_read and use_single_write. Motivation: When multiple sequential registers need to be read or written, the author can either choose to call regmap_bulk_read/write() once or call regmap_read/write() repeatedly. The _bulk option has the advantages of being more compact since only one function call is needed and more efficient since multiple registers are read/written at once instead of having to pay for transaction and addressing overhead of the underlying transport multiple times. Many chips don't support bulk access, but it's still convenient to be able to read/write multiple registers with a single function call. This is what use_single_rw=true is for. When this option is set, bulk reads/writes are automatically split into a series of single register accesses. Some other chips (for example the Bosch bmi160 IMU) support bulk reads, but not bulk writes. Currently for chips like this, the author must choose between convenience or efficiency by setting use_single_rw = true/false respectively. Proposal: In this patch series I split the use_single_rw member of struct regmap_config into use_single_read and use_single_write. This change is in line with what is already implemented inside of the internal struct regmap which is populated from struct regmap_config. I submitted separate patches for the change that performs the split and the update to all of the existing clients. I wasn't sure if this is the preferred method or if it's better to combine those patches. David Frey (3): regmap: fix comment for regmap.use_single_write regmap: split up regmap_config.use_single_rw regmap: update users of regmap_config.use_single_rw drivers/base/regmap/internal.h | 2 +- drivers/base/regmap/regmap.c | 4 ++-- drivers/edac/altera_edac.c | 3 ++- drivers/hwmon/lm75.c | 3 ++- drivers/hwmon/lm95245.c | 3 ++- drivers/hwmon/tmp102.c | 3 ++- drivers/hwmon/tmp108.c | 3 ++- drivers/iio/light/apds9960.c | 3 ++- drivers/iio/light/max44000.c | 23 --- drivers/iio/temperature/mlx90632.c | 3 ++- drivers/input/touchscreen/tsc200x-core.c | 3 ++- drivers/mfd/altera-a10sr.c | 3 ++- drivers/mfd/da9052-spi.c | 3 ++- drivers/mfd/mc13xxx-spi.c| 3 ++- drivers/mfd/twl6040.c| 3 ++- drivers/regulator/ltc3589.c | 3 ++- drivers/regulator/ltc3676.c | 3 ++- include/linux/regmap.h | 12 sound/hda/hdac_regmap.c | 3 ++- sound/soc/codecs/cs35l33.c | 3 ++- sound/soc/codecs/cs35l35.c | 3 ++- sound/soc/codecs/cs43130.c | 4 +++- sound/soc/codecs/es8328.c| 3 ++- sound/soc/codecs/rt1305.c| 3 ++- sound/soc/codecs/rt5514.c| 3 ++- sound/soc/codecs/rt5616.c| 3 ++- sound/soc/codecs/rt5640.c| 3 ++- sound/soc/codecs/rt5645.c| 9 ++--- sound/soc/codecs/rt5651.c| 3 ++- sound/soc/codecs/rt5660.c| 3 ++- sound/soc/codecs/rt5663.c| 9 ++--- sound/soc/codecs/rt5665.c| 3 ++- sound/soc/codecs/rt5668.c| 3 ++- sound/soc/codecs/rt5670.c| 3 ++- sound/soc/codecs/rt5682.c| 3 ++- 35 files changed, 94 insertions(+), 53 deletions(-) -- 2.11.0
[PATCH 1/3] regmap: fix comment for regmap.use_single_write
Signed-off-by: David Frey
---
drivers/base/regmap/internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a6bf34d6394e..16414ccace96 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -149,7 +149,7 @@ struct regmap {
/* if set, converts bulk read to single read */
bool use_single_read;
- /* if set, converts bulk read to single read */
+ /* if set, converts bulk write to single write */
bool use_single_write;
/* if set, the device supports multi write mode */
bool can_multi_write;
--
2.11.0
[PATCH 2/3] regmap: split up regmap_config.use_single_rw
Split regmap_config.use_single_rw into use_single_read and
use_single_write. This change enables drivers of devices which only
support bulk operations in one direction to use the regmap_bulk_*()
functions for both directions and have their bulk operation split into
single operations only when necessary.
Signed-off-by: David Frey
---
drivers/base/regmap/regmap.c | 4 ++--
include/linux/regmap.h | 12
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 0360a90ad6b6..78a778c08f92 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -762,8 +762,8 @@ struct regmap *__regmap_init(struct device *dev,
map->reg_stride_order = ilog2(map->reg_stride);
else
map->reg_stride_order = -1;
- map->use_single_read = config->use_single_rw || !bus || !bus->read;
- map->use_single_write = config->use_single_rw || !bus || !bus->write;
+ map->use_single_read = config->use_single_read || !bus || !bus->read;
+ map->use_single_write = config->use_single_write || !bus || !bus->write;
map->can_multi_write = config->can_multi_write && bus && bus->write;
if (bus) {
map->max_raw_read = bus->max_raw_read;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 379505a53722..6ea9bf9377cb 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -315,9 +315,12 @@ typedef void (*regmap_unlock)(void *);
* masks are used.
* @zero_flag_mask: If set, read_flag_mask and write_flag_mask are used even
* if they are both empty.
- * @use_single_rw: If set, converts the bulk read and write operations into
- * a series of single read and write operations. This is useful
- * for device that does not support bulk read and write.
+ * @use_single_read: If set, converts the bulk read operation into a series of
+ * single read operations. This is useful for a device that
+ * does not support bulk read.
+ * @use_single_write: If set, converts the bulk write operation into a series
of
+ *single write operations. This is useful for a device that
+ *does not support bulk write.
* @can_multi_write: If set, the device supports the multi write mode of bulk
* write operations, if clear multi write requests will be
* split into individual write operations
@@ -380,7 +383,8 @@ struct regmap_config {
unsigned long write_flag_mask;
bool zero_flag_mask;
- bool use_single_rw;
+ bool use_single_read;
+ bool use_single_write;
bool can_multi_write;
enum regmap_endian reg_format_endian;
--
2.11.0
[PATCH 3/3] regmap: update users of regmap_config.use_single_rw
Update all struct regmap_config instances where the use_single_rw member
was set to instead set both use_single_read and use_single_write. No
attempt was made to evaluate whether it is possible to set only one of
use_single_read or use_single_write.
Signed-off-by: David Frey
---
drivers/edac/altera_edac.c | 3 ++-
drivers/hwmon/lm75.c | 3 ++-
drivers/hwmon/lm95245.c | 3 ++-
drivers/hwmon/tmp102.c | 3 ++-
drivers/hwmon/tmp108.c | 3 ++-
drivers/iio/light/apds9960.c | 3 ++-
drivers/iio/light/max44000.c | 23 ---
drivers/iio/temperature/mlx90632.c | 3 ++-
drivers/input/touchscreen/tsc200x-core.c | 3 ++-
drivers/mfd/altera-a10sr.c | 3 ++-
drivers/mfd/da9052-spi.c | 3 ++-
drivers/mfd/mc13xxx-spi.c| 3 ++-
drivers/mfd/twl6040.c| 3 ++-
drivers/regulator/ltc3589.c | 3 ++-
drivers/regulator/ltc3676.c | 3 ++-
sound/hda/hdac_regmap.c | 3 ++-
sound/soc/codecs/cs35l33.c | 3 ++-
sound/soc/codecs/cs35l35.c | 3 ++-
sound/soc/codecs/cs43130.c | 4 +++-
sound/soc/codecs/es8328.c| 3 ++-
sound/soc/codecs/rt1305.c| 3 ++-
sound/soc/codecs/rt5514.c| 3 ++-
sound/soc/codecs/rt5616.c| 3 ++-
sound/soc/codecs/rt5640.c| 3 ++-
sound/soc/codecs/rt5645.c| 9 ++---
sound/soc/codecs/rt5651.c| 3 ++-
sound/soc/codecs/rt5660.c| 3 ++-
sound/soc/codecs/rt5663.c| 9 ++---
sound/soc/codecs/rt5665.c| 3 ++-
sound/soc/codecs/rt5668.c| 3 ++-
sound/soc/codecs/rt5670.c| 3 ++-
sound/soc/codecs/rt5682.c| 3 ++-
32 files changed, 83 insertions(+), 46 deletions(-)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 5762c3c383f2..ab7c5a937ab0 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -599,7 +599,8 @@ static const struct regmap_config s10_sdram_regmap_cfg = {
.volatile_reg = s10_sdram_volatile_reg,
.reg_read = s10_protected_reg_read,
.reg_write = s10_protected_reg_write,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int altr_s10_sdram_probe(struct platform_device *pdev)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 49f4b33a5685..542dc4058831 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -254,7 +254,8 @@ static const struct regmap_config lm75_regmap_config = {
.volatile_reg = lm75_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static void lm75_remove(void *data)
diff --git a/drivers/hwmon/lm95245.c b/drivers/hwmon/lm95245.c
index 27cb06d65594..996b50246175 100644
--- a/drivers/hwmon/lm95245.c
+++ b/drivers/hwmon/lm95245.c
@@ -541,7 +541,8 @@ static const struct regmap_config lm95245_regmap_config = {
.writeable_reg = lm95245_is_writeable_reg,
.volatile_reg = lm95245_is_volatile_reg,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static const u32 lm95245_chip_config[] = {
diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c
index dfc40c740d07..6778283e36f9 100644
--- a/drivers/hwmon/tmp102.c
+++ b/drivers/hwmon/tmp102.c
@@ -212,7 +212,8 @@ static const struct regmap_config tmp102_regmap_config = {
.volatile_reg = tmp102_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int tmp102_probe(struct i2c_client *client,
diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
index 91bb94639286..429bfeae4ca8 100644
--- a/drivers/hwmon/tmp108.c
+++ b/drivers/hwmon/tmp108.c
@@ -345,7 +345,8 @@ static const struct regmap_config tmp108_regmap_config = {
.volatile_reg = tmp108_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int tmp108_probe(struct i2c_client *client,
diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c
index 1f112ae15f3c..b09b8b60bd83 100644
--- a/drivers/iio/light/apds9960.c
+++ b/drivers/iio/light/apds9960.c
@@ -206,7 +206,8 @@ static const struct regmap_config apds9960_regmap_config = {
.name = APDS9960_REGMAP_NAME,
.reg_bits = 8,
.val_bits = 8
[PATCH 0/3] regmap: split regmap_config.use_single_rw
This patch series splits regmap_config.use_single_rw into use_single_read and use_single_write. Motivation: When multiple sequential registers need to be read or written, the author can either choose to call regmap_bulk_read/write() once or call regmap_read/write() repeatedly. The _bulk option has the advantages of being more compact since only one function call is needed and more efficient since multiple registers are read/written at once instead of having to pay for transaction and addressing overhead of the underlying transport multiple times. Many chips don't support bulk access, but it's still convenient to be able to read/write multiple registers with a single function call. This is what use_single_rw=true is for. When this option is set, bulk reads/writes are automatically split into a series of single register accesses. Some other chips (for example the Bosch bmi160 IMU) support bulk reads, but not bulk writes. Currently for chips like this, the author must choose between convenience or efficiency by setting use_single_rw = true/false respectively. Proposal: In this patch series I split the use_single_rw member of struct regmap_config into use_single_read and use_single_write. This change is in line with what is already implemented inside of the internal struct regmap which is populated from struct regmap_config. I submitted separate patches for the change that performs the split and the update to all of the existing clients. I wasn't sure if this is the preferred method or if it's better to combine those patches. David Frey (3): regmap: fix comment for regmap.use_single_write regmap: split up regmap_config.use_single_rw regmap: update users of regmap_config.use_single_rw drivers/base/regmap/internal.h | 2 +- drivers/base/regmap/regmap.c | 4 ++-- drivers/edac/altera_edac.c | 3 ++- drivers/hwmon/lm75.c | 3 ++- drivers/hwmon/lm95245.c | 3 ++- drivers/hwmon/tmp102.c | 3 ++- drivers/hwmon/tmp108.c | 3 ++- drivers/iio/light/apds9960.c | 3 ++- drivers/iio/light/max44000.c | 23 --- drivers/iio/temperature/mlx90632.c | 3 ++- drivers/input/touchscreen/tsc200x-core.c | 3 ++- drivers/mfd/altera-a10sr.c | 3 ++- drivers/mfd/da9052-spi.c | 3 ++- drivers/mfd/mc13xxx-spi.c| 3 ++- drivers/mfd/twl6040.c| 3 ++- drivers/regulator/ltc3589.c | 3 ++- drivers/regulator/ltc3676.c | 3 ++- include/linux/regmap.h | 12 sound/hda/hdac_regmap.c | 3 ++- sound/soc/codecs/cs35l33.c | 3 ++- sound/soc/codecs/cs35l35.c | 3 ++- sound/soc/codecs/cs43130.c | 4 +++- sound/soc/codecs/es8328.c| 3 ++- sound/soc/codecs/rt1305.c| 3 ++- sound/soc/codecs/rt5514.c| 3 ++- sound/soc/codecs/rt5616.c| 3 ++- sound/soc/codecs/rt5640.c| 3 ++- sound/soc/codecs/rt5645.c| 9 ++--- sound/soc/codecs/rt5651.c| 3 ++- sound/soc/codecs/rt5660.c| 3 ++- sound/soc/codecs/rt5663.c| 9 ++--- sound/soc/codecs/rt5665.c| 3 ++- sound/soc/codecs/rt5668.c| 3 ++- sound/soc/codecs/rt5670.c| 3 ++- sound/soc/codecs/rt5682.c| 3 ++- 35 files changed, 94 insertions(+), 53 deletions(-) -- 2.11.0
[PATCH 1/3] regmap: fix comment for regmap.use_single_write
Signed-off-by: David Frey
---
drivers/base/regmap/internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a6bf34d6394e..16414ccace96 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -149,7 +149,7 @@ struct regmap {
/* if set, converts bulk read to single read */
bool use_single_read;
- /* if set, converts bulk read to single read */
+ /* if set, converts bulk write to single write */
bool use_single_write;
/* if set, the device supports multi write mode */
bool can_multi_write;
--
2.11.0
[PATCH 2/3] regmap: split up regmap_config.use_single_rw
Split regmap_config.use_single_rw into use_single_read and
use_single_write. This change enables drivers of devices which only
support bulk operations in one direction to use the regmap_bulk_*()
functions for both directions and have their bulk operation split into
single operations only when necessary.
Signed-off-by: David Frey
---
drivers/base/regmap/regmap.c | 4 ++--
include/linux/regmap.h | 12
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 0360a90ad6b6..78a778c08f92 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -762,8 +762,8 @@ struct regmap *__regmap_init(struct device *dev,
map->reg_stride_order = ilog2(map->reg_stride);
else
map->reg_stride_order = -1;
- map->use_single_read = config->use_single_rw || !bus || !bus->read;
- map->use_single_write = config->use_single_rw || !bus || !bus->write;
+ map->use_single_read = config->use_single_read || !bus || !bus->read;
+ map->use_single_write = config->use_single_write || !bus || !bus->write;
map->can_multi_write = config->can_multi_write && bus && bus->write;
if (bus) {
map->max_raw_read = bus->max_raw_read;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 379505a53722..6ea9bf9377cb 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -315,9 +315,12 @@ typedef void (*regmap_unlock)(void *);
* masks are used.
* @zero_flag_mask: If set, read_flag_mask and write_flag_mask are used even
* if they are both empty.
- * @use_single_rw: If set, converts the bulk read and write operations into
- * a series of single read and write operations. This is useful
- * for device that does not support bulk read and write.
+ * @use_single_read: If set, converts the bulk read operation into a series of
+ * single read operations. This is useful for a device that
+ * does not support bulk read.
+ * @use_single_write: If set, converts the bulk write operation into a series
of
+ *single write operations. This is useful for a device that
+ *does not support bulk write.
* @can_multi_write: If set, the device supports the multi write mode of bulk
* write operations, if clear multi write requests will be
* split into individual write operations
@@ -380,7 +383,8 @@ struct regmap_config {
unsigned long write_flag_mask;
bool zero_flag_mask;
- bool use_single_rw;
+ bool use_single_read;
+ bool use_single_write;
bool can_multi_write;
enum regmap_endian reg_format_endian;
--
2.11.0
[PATCH 3/3] regmap: update users of regmap_config.use_single_rw
Update all struct regmap_config instances where the use_single_rw member
was set to instead set both use_single_read and use_single_write. No
attempt was made to evaluate whether it is possible to set only one of
use_single_read or use_single_write.
Signed-off-by: David Frey
---
drivers/edac/altera_edac.c | 3 ++-
drivers/hwmon/lm75.c | 3 ++-
drivers/hwmon/lm95245.c | 3 ++-
drivers/hwmon/tmp102.c | 3 ++-
drivers/hwmon/tmp108.c | 3 ++-
drivers/iio/light/apds9960.c | 3 ++-
drivers/iio/light/max44000.c | 23 ---
drivers/iio/temperature/mlx90632.c | 3 ++-
drivers/input/touchscreen/tsc200x-core.c | 3 ++-
drivers/mfd/altera-a10sr.c | 3 ++-
drivers/mfd/da9052-spi.c | 3 ++-
drivers/mfd/mc13xxx-spi.c| 3 ++-
drivers/mfd/twl6040.c| 3 ++-
drivers/regulator/ltc3589.c | 3 ++-
drivers/regulator/ltc3676.c | 3 ++-
sound/hda/hdac_regmap.c | 3 ++-
sound/soc/codecs/cs35l33.c | 3 ++-
sound/soc/codecs/cs35l35.c | 3 ++-
sound/soc/codecs/cs43130.c | 4 +++-
sound/soc/codecs/es8328.c| 3 ++-
sound/soc/codecs/rt1305.c| 3 ++-
sound/soc/codecs/rt5514.c| 3 ++-
sound/soc/codecs/rt5616.c| 3 ++-
sound/soc/codecs/rt5640.c| 3 ++-
sound/soc/codecs/rt5645.c| 9 ++---
sound/soc/codecs/rt5651.c| 3 ++-
sound/soc/codecs/rt5660.c| 3 ++-
sound/soc/codecs/rt5663.c| 9 ++---
sound/soc/codecs/rt5665.c| 3 ++-
sound/soc/codecs/rt5668.c| 3 ++-
sound/soc/codecs/rt5670.c| 3 ++-
sound/soc/codecs/rt5682.c| 3 ++-
32 files changed, 83 insertions(+), 46 deletions(-)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 5762c3c383f2..ab7c5a937ab0 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -599,7 +599,8 @@ static const struct regmap_config s10_sdram_regmap_cfg = {
.volatile_reg = s10_sdram_volatile_reg,
.reg_read = s10_protected_reg_read,
.reg_write = s10_protected_reg_write,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int altr_s10_sdram_probe(struct platform_device *pdev)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 49f4b33a5685..542dc4058831 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -254,7 +254,8 @@ static const struct regmap_config lm75_regmap_config = {
.volatile_reg = lm75_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static void lm75_remove(void *data)
diff --git a/drivers/hwmon/lm95245.c b/drivers/hwmon/lm95245.c
index 27cb06d65594..996b50246175 100644
--- a/drivers/hwmon/lm95245.c
+++ b/drivers/hwmon/lm95245.c
@@ -541,7 +541,8 @@ static const struct regmap_config lm95245_regmap_config = {
.writeable_reg = lm95245_is_writeable_reg,
.volatile_reg = lm95245_is_volatile_reg,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static const u32 lm95245_chip_config[] = {
diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c
index dfc40c740d07..6778283e36f9 100644
--- a/drivers/hwmon/tmp102.c
+++ b/drivers/hwmon/tmp102.c
@@ -212,7 +212,8 @@ static const struct regmap_config tmp102_regmap_config = {
.volatile_reg = tmp102_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int tmp102_probe(struct i2c_client *client,
diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
index 91bb94639286..429bfeae4ca8 100644
--- a/drivers/hwmon/tmp108.c
+++ b/drivers/hwmon/tmp108.c
@@ -345,7 +345,8 @@ static const struct regmap_config tmp108_regmap_config = {
.volatile_reg = tmp108_is_volatile_reg,
.val_format_endian = REGMAP_ENDIAN_BIG,
.cache_type = REGCACHE_RBTREE,
- .use_single_rw = true,
+ .use_single_read = true,
+ .use_single_write = true,
};
static int tmp108_probe(struct i2c_client *client,
diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c
index 1f112ae15f3c..b09b8b60bd83 100644
--- a/drivers/iio/light/apds9960.c
+++ b/drivers/iio/light/apds9960.c
@@ -206,7 +206,8 @@ static const struct regmap_config apds9960_regmap_config = {
.name = APDS9960_REGMAP_NAME,
.reg_bits = 8,
.val_bits = 8
Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
On 7/22/2018 3:21 PM, Himanshu Jha wrote:
On Sat, Jul 21, 2018 at 08:45:34PM +0300, Daniel Baluta wrote:
On Sat, Jul 21, 2018 at 6:43 PM, Andy Shevchenko
wrote:
On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
wrote:
+ /* Look up table 1 for the possible gas range values */
+ u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
+ 2147483647u, 2147483647u, 2126008810u,
+ 2147483647u, 2130303777u, 2147483647u,
+ 2147483647u, 2143188679u, 2136746228u,
+ 2147483647u, 2126008810u, 2147483647u,
+ 2147483647u};
This one needs perhaps a bit of though, but...
+ /* Look up table 2 for the possible gas range values */
+ u32 lookupTable2[16] = {409600u, 204800u, 102400u,
+ 51200u, 255744255u, 127110228u, 6400u,
+ 32258064u, 16016016u, 800u, 400u,
+ 200u, 100u, 50u, 25u, 125000u};
...this one obviously just a not needed one. You may replace it with a
one constant and simple calculation to get either value (index from
value, or value from index).
Indeed this can be reduce to:
125.000 << (15 - idx).
The real question here is if we approximate 255.744.255u to 256.00.00u how
much different is the result. Being a gas sensor I think it is very
hard to appreciate.
We can go with this formula + adding a comment with the table with the
exact coefficients.
So, I have planned to use this 125000 << (15 - idx) equation with
approximating the array members.
About the difference in results we would get after approximating isn't
much of a problem IMHO because gas sensor is primarily used for IAQ, and
IAQ is relative to the resistance reading.
For eg:
Resistance(ohm) IAQ
value < 30K Very bad
30k < value < 50k worse
50k < value < 70k bad
...
..
so on..
So, what I simply imply is the scale will be adjusted and nothing else
changes, unlike if it had been pressure, temperature, humidity.
The IAQ implementation is userspace application suggesting
good/bad/ugly air quality.
And since we know David Frey is planning to use this sensor in his
product mangOH board.
So, David, how are you planning to use the gas sensing part in your
product ? RGB leds, buzzer, alarm ?
Thanks Andy for the suggestion :)
My understanding is that the Bosch BSEC (Bosch Sensortec Environmental
Cluster - https://www.bosch-sensortec.com/bst/products/all_products/bsec)
software calculates the indoor air quality (IAQ) which is presented in
the range of 0 to 500. BSEC is proprietary, pre-compiled static
library. I don't know how they derive the IAQ, but it seems that it
could be based on smoothing outlying gas resistance values and
integrating other values such as temperature, humidity and pressure.
Unless this driver can somehow produce IAQ values of equal or greater
reliability to the BSEC library, then I would prefer that it just
present the raw gas resistance value so that a user can write a program
to feed the sensor data into BSEC.
mangOH isn't really a traditional product. It's an open hardware board
designed around Sierra Wireless cellular modules that run Linux. So I
don't have any specific use case in mind, but I want to enable our users
(and thus future products) to make use of air quality measurements.
Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
On 7/22/2018 3:21 PM, Himanshu Jha wrote:
On Sat, Jul 21, 2018 at 08:45:34PM +0300, Daniel Baluta wrote:
On Sat, Jul 21, 2018 at 6:43 PM, Andy Shevchenko
wrote:
On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
wrote:
+ /* Look up table 1 for the possible gas range values */
+ u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
+ 2147483647u, 2147483647u, 2126008810u,
+ 2147483647u, 2130303777u, 2147483647u,
+ 2147483647u, 2143188679u, 2136746228u,
+ 2147483647u, 2126008810u, 2147483647u,
+ 2147483647u};
This one needs perhaps a bit of though, but...
+ /* Look up table 2 for the possible gas range values */
+ u32 lookupTable2[16] = {409600u, 204800u, 102400u,
+ 51200u, 255744255u, 127110228u, 6400u,
+ 32258064u, 16016016u, 800u, 400u,
+ 200u, 100u, 50u, 25u, 125000u};
...this one obviously just a not needed one. You may replace it with a
one constant and simple calculation to get either value (index from
value, or value from index).
Indeed this can be reduce to:
125.000 << (15 - idx).
The real question here is if we approximate 255.744.255u to 256.00.00u how
much different is the result. Being a gas sensor I think it is very
hard to appreciate.
We can go with this formula + adding a comment with the table with the
exact coefficients.
So, I have planned to use this 125000 << (15 - idx) equation with
approximating the array members.
About the difference in results we would get after approximating isn't
much of a problem IMHO because gas sensor is primarily used for IAQ, and
IAQ is relative to the resistance reading.
For eg:
Resistance(ohm) IAQ
value < 30K Very bad
30k < value < 50k worse
50k < value < 70k bad
...
..
so on..
So, what I simply imply is the scale will be adjusted and nothing else
changes, unlike if it had been pressure, temperature, humidity.
The IAQ implementation is userspace application suggesting
good/bad/ugly air quality.
And since we know David Frey is planning to use this sensor in his
product mangOH board.
So, David, how are you planning to use the gas sensing part in your
product ? RGB leds, buzzer, alarm ?
Thanks Andy for the suggestion :)
My understanding is that the Bosch BSEC (Bosch Sensortec Environmental
Cluster - https://www.bosch-sensortec.com/bst/products/all_products/bsec)
software calculates the indoor air quality (IAQ) which is presented in
the range of 0 to 500. BSEC is proprietary, pre-compiled static
library. I don't know how they derive the IAQ, but it seems that it
could be based on smoothing outlying gas resistance values and
integrating other values such as temperature, humidity and pressure.
Unless this driver can somehow produce IAQ values of equal or greater
reliability to the BSEC library, then I would prefer that it just
present the raw gas resistance value so that a user can write a program
to feed the sensor data into BSEC.
mangOH isn't really a traditional product. It's an open hardware board
designed around Sierra Wireless cellular modules that run Linux. So I
don't have any specific use case in mind, but I want to enable our users
(and thus future products) to make use of air quality measurements.
Re: [PATCH v3] iio: chemical: Add support for Bosch BME680 sensor
Hi Himanshu Jha, First a bit of background. I'm working on a device which will contain a bme680 sensor. A colleague of mine started work on a Linux kernel driver for the chip a little while ago. The (WIP) driver can be found here: https://github.com/mangOH/mangOH/tree/master/linux_kernel_modules/bme680 This driver is written targeting an older kernel (3.18.x) because that's the kernel we're stuck on for now. Rather than writing the driver from scratch, what we did was write the Linux kernel driver as a wrapper around the Bosch code. My theory at the time was that Bosch made the chip, so they probably know what they're doing when it comes to writing a driver library. After having looked at the code in more detail, I'm less confident that our approach was the best one. I'm not attempting to upstream the driver built by my colleague and I'm not trying to request review of this code either. I simply want to make you aware of it so that you can look at it to get some ideas. I have included a number of comments on your driver below. Keep up the good work! On 7/11/2018 5:13 AM, Himanshu Jha wrote: Bosch BME680 is a 4-in-1 sensor with temperature, pressure, humidity and gas sensing capability. It supports both I2C and SPI communication protocol for effective data communication. The device supports two modes: 1. Sleep mode 2. Forced mode The measurements only takes place when forced mode is triggered and a single TPHG cycle is performed by the sensor. The sensor automatically goes to sleep after afterwards. The device has various calibration constants/parameters programmed into devices' non-volatile memory(NVM) during production and can't be altered by the user. These constants are used in the compensation functions to get the required compensated readings along with the raw data. The compensation functions/algorithms are provided by Bosch Sensortec GmbH via their API[1]. As these don't change during the measurement cycle, therefore we read and store them at the probe. The default configs supplied by Bosch are also set at probe. 0-day tested with build success. GSoC-2018: https://summerofcode.withgoogle.com/projects/#6691473790074880 Mentor: Daniel Baluta [1] https://github.com/BoschSensortec/BME680_driver Datasheet: https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME680-DS001-00.pdf Cc: Daniel Baluta Signed-off-by: Himanshu Jha --- v3: -moved files to chemical directory instead of a dedicated directory. -read calibration parameters serially with endian conversions. -drop some return ret. -removed few unnecessary casts safely. -added 'u' suffix to explicitly specify unsigned for large values and thereby fixing comiler warning. -left aligned all comments. -added a comment explaining heater stability failure. v2: -Used devm_add_action() to add a generic remove method for both I2C & SPI driver. -Introduction of compensation functions. -chip initialisation routines moved to respective I2C and SPI driver. -Introduction of gas sensing rountines. -Simplified Kconfig to reduce various options. drivers/iio/chemical/Kconfig | 25 + drivers/iio/chemical/Makefile | 3 + drivers/iio/chemical/bme680.h | 99 drivers/iio/chemical/bme680_core.c | 946 + drivers/iio/chemical/bme680_i2c.c | 83 drivers/iio/chemical/bme680_spi.c | 123 + 6 files changed, 1279 insertions(+) create mode 100644 drivers/iio/chemical/bme680.h create mode 100644 drivers/iio/chemical/bme680_core.c create mode 100644 drivers/iio/chemical/bme680_i2c.c create mode 100644 drivers/iio/chemical/bme680_spi.c diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig index 5cb5be7..24790a8 100644 --- a/drivers/iio/chemical/Kconfig +++ b/drivers/iio/chemical/Kconfig @@ -21,6 +21,31 @@ config ATLAS_PH_SENSOR To compile this driver as module, choose M here: the module will be called atlas-ph-sensor. +config BME680 + tristate "Bosch Sensortec BME680 sensor driver" + depends on (I2C || SPI) + select REGMAP + select BME680_I2C if (I2C) + select BME680_SPI if (SPI) + help + Say yes here to build support for Bosch Sensortec BME680 sensor with + temperature, pressure, humidity and gas sensing capability. + + This driver can also be built as a module. If so, the module for I2C + would be called bme680_i2c and bme680_spi for SPI support. + +config BME680_I2C + tristate + depends on BME680 + depends on I2C + select REGMAP_I2C + +config BME680_SPI + tristate + depends on BME680 + depends on SPI + select REGMAP_SPI + config CCS811 tristate "AMS CCS811 VOC sensor" depends on I2C diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile index a629b29..2f4c4ba 100644 ---
Re: [PATCH v3] iio: chemical: Add support for Bosch BME680 sensor
Hi Himanshu Jha, First a bit of background. I'm working on a device which will contain a bme680 sensor. A colleague of mine started work on a Linux kernel driver for the chip a little while ago. The (WIP) driver can be found here: https://github.com/mangOH/mangOH/tree/master/linux_kernel_modules/bme680 This driver is written targeting an older kernel (3.18.x) because that's the kernel we're stuck on for now. Rather than writing the driver from scratch, what we did was write the Linux kernel driver as a wrapper around the Bosch code. My theory at the time was that Bosch made the chip, so they probably know what they're doing when it comes to writing a driver library. After having looked at the code in more detail, I'm less confident that our approach was the best one. I'm not attempting to upstream the driver built by my colleague and I'm not trying to request review of this code either. I simply want to make you aware of it so that you can look at it to get some ideas. I have included a number of comments on your driver below. Keep up the good work! On 7/11/2018 5:13 AM, Himanshu Jha wrote: Bosch BME680 is a 4-in-1 sensor with temperature, pressure, humidity and gas sensing capability. It supports both I2C and SPI communication protocol for effective data communication. The device supports two modes: 1. Sleep mode 2. Forced mode The measurements only takes place when forced mode is triggered and a single TPHG cycle is performed by the sensor. The sensor automatically goes to sleep after afterwards. The device has various calibration constants/parameters programmed into devices' non-volatile memory(NVM) during production and can't be altered by the user. These constants are used in the compensation functions to get the required compensated readings along with the raw data. The compensation functions/algorithms are provided by Bosch Sensortec GmbH via their API[1]. As these don't change during the measurement cycle, therefore we read and store them at the probe. The default configs supplied by Bosch are also set at probe. 0-day tested with build success. GSoC-2018: https://summerofcode.withgoogle.com/projects/#6691473790074880 Mentor: Daniel Baluta [1] https://github.com/BoschSensortec/BME680_driver Datasheet: https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME680-DS001-00.pdf Cc: Daniel Baluta Signed-off-by: Himanshu Jha --- v3: -moved files to chemical directory instead of a dedicated directory. -read calibration parameters serially with endian conversions. -drop some return ret. -removed few unnecessary casts safely. -added 'u' suffix to explicitly specify unsigned for large values and thereby fixing comiler warning. -left aligned all comments. -added a comment explaining heater stability failure. v2: -Used devm_add_action() to add a generic remove method for both I2C & SPI driver. -Introduction of compensation functions. -chip initialisation routines moved to respective I2C and SPI driver. -Introduction of gas sensing rountines. -Simplified Kconfig to reduce various options. drivers/iio/chemical/Kconfig | 25 + drivers/iio/chemical/Makefile | 3 + drivers/iio/chemical/bme680.h | 99 drivers/iio/chemical/bme680_core.c | 946 + drivers/iio/chemical/bme680_i2c.c | 83 drivers/iio/chemical/bme680_spi.c | 123 + 6 files changed, 1279 insertions(+) create mode 100644 drivers/iio/chemical/bme680.h create mode 100644 drivers/iio/chemical/bme680_core.c create mode 100644 drivers/iio/chemical/bme680_i2c.c create mode 100644 drivers/iio/chemical/bme680_spi.c diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig index 5cb5be7..24790a8 100644 --- a/drivers/iio/chemical/Kconfig +++ b/drivers/iio/chemical/Kconfig @@ -21,6 +21,31 @@ config ATLAS_PH_SENSOR To compile this driver as module, choose M here: the module will be called atlas-ph-sensor. +config BME680 + tristate "Bosch Sensortec BME680 sensor driver" + depends on (I2C || SPI) + select REGMAP + select BME680_I2C if (I2C) + select BME680_SPI if (SPI) + help + Say yes here to build support for Bosch Sensortec BME680 sensor with + temperature, pressure, humidity and gas sensing capability. + + This driver can also be built as a module. If so, the module for I2C + would be called bme680_i2c and bme680_spi for SPI support. + +config BME680_I2C + tristate + depends on BME680 + depends on I2C + select REGMAP_I2C + +config BME680_SPI + tristate + depends on BME680 + depends on SPI + select REGMAP_SPI + config CCS811 tristate "AMS CCS811 VOC sensor" depends on I2C diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile index a629b29..2f4c4ba 100644 ---
Seeking guidance on spelling fixes
I noticed a common spelling mistake in some Linux kernel code that I was reading the other day and it made me wonder how prevalent common spelling mistakes are in the kernel. I did some grepping and it seems that there are a large number of spelling mistakes. I did a bit of searching and I found (http://www.kegel.com/kerspell/) which includes some comments/tools relating to spelling and the Linux kernel. I have gone ahead and corrected about 100 total instances based on 25 common misspellings. There are many, many more common misspellings that I could check, but I don't want to sink more time into this if the changes won't be accepted. Is this type of change likely to be accepted? Right now I am making individual git commits for each correction. For example "aditional -> additional". I figured that it would be easier to review the list of replacements rather than a mega-patch which fixes many different errors. This way if any of the changes are controversial, I can easily rebase them out. I am trying to only correct true misspellings. I'm not trying to choose the "most correct" spelling for cases where there are acceptable alternate spellings. I've pushed the changes I have made so far to a "spelling_fixes" branch here: https://github.com/dpfrey/linux/commits/spelling_fixes just in case there is any question about what I am doing based on the above description. Just to be clear, I'm not trying to get this portion of the work merged at this point. I'm trying to get a feel for whether I should keep working on this. Thanks, David Frey
Seeking guidance on spelling fixes
I noticed a common spelling mistake in some Linux kernel code that I was reading the other day and it made me wonder how prevalent common spelling mistakes are in the kernel. I did some grepping and it seems that there are a large number of spelling mistakes. I did a bit of searching and I found (http://www.kegel.com/kerspell/) which includes some comments/tools relating to spelling and the Linux kernel. I have gone ahead and corrected about 100 total instances based on 25 common misspellings. There are many, many more common misspellings that I could check, but I don't want to sink more time into this if the changes won't be accepted. Is this type of change likely to be accepted? Right now I am making individual git commits for each correction. For example "aditional -> additional". I figured that it would be easier to review the list of replacements rather than a mega-patch which fixes many different errors. This way if any of the changes are controversial, I can easily rebase them out. I am trying to only correct true misspellings. I'm not trying to choose the "most correct" spelling for cases where there are acceptable alternate spellings. I've pushed the changes I have made so far to a "spelling_fixes" branch here: https://github.com/dpfrey/linux/commits/spelling_fixes just in case there is any question about what I am doing based on the above description. Just to be clear, I'm not trying to get this portion of the work merged at this point. I'm trying to get a feel for whether I should keep working on this. Thanks, David Frey
