Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1205901 > unrolled thread
| Started by | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| First post | 2015-08-12 12:20 +0200 |
| Last post | 2015-08-15 04:20 +0200 |
| Articles | 20 on this page of 25 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 00/20] Regmap max_raw_io and bmc150 SPI support Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:20 +0200
[PATCH 17/20] iio: bmc150: Use i2c regmap Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:20 +0200
Re: [PATCH 17/20] iio: bmc150: Use i2c regmap Mark Brown <broonie@kernel.org> - 2015-08-12 14:10 +0200
Re: [PATCH 17/20] iio: bmc150: Use i2c regmap Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 15:00 +0200
Re: [PATCH 17/20] iio: bmc150: Use i2c regmap Jonathan Cameron <jic23@kernel.org> - 2015-08-15 15:30 +0200
Re: [PATCH 17/20] iio: bmc150: Use i2c regmap Markus Pargmann <mpa@pengutronix.de> - 2015-08-17 09:50 +0200
[PATCH 15/20] regmap-i2c: Add smbus i2c block support Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:20 +0200
Re: [PATCH 15/20] regmap-i2c: Add smbus i2c block support Mark Brown <broonie@kernel.org> - 2015-08-12 14:00 +0200
Re: [PATCH 15/20] regmap-i2c: Add smbus i2c block support Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 15:00 +0200
[PATCH 01/20] regmap: Add missing comments about struct regmap_bus Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:30 +0200
Re: [PATCH 01/20] regmap: Add missing comments about struct regmap_bus Hartmut Knaack <knaack.h@gmx.de> - 2015-08-12 22:20 +0200
Re: [PATCH 01/20] regmap: Add missing comments about struct regmap_bus Markus Pargmann <mpa@pengutronix.de> - 2015-08-17 09:20 +0200
[PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:30 +0200
Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() Mark Brown <broonie@kernel.org> - 2015-08-12 13:30 +0200
Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 14:30 +0200
Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() Mark Brown <broonie@kernel.org> - 2015-08-12 14:40 +0200
Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 15:10 +0200
Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() Mark Brown <broonie@kernel.org> - 2015-08-15 04:20 +0200
[PATCH 10/20] regmap: _regmap_raw_multi_reg_write: Add reg_write() support Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:30 +0200
Re: [PATCH 10/20] regmap: _regmap_raw_multi_reg_write: Add reg_write() support Mark Brown <broonie@kernel.org> - 2015-08-12 14:50 +0200
Re: [PATCH 10/20] regmap: _regmap_raw_multi_reg_write: Add reg_write() support Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 15:20 +0200
[PATCH 08/20] regmap: Fix regmap_can_raw_write check Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:30 +0200
Re: [PATCH 00/20] Regmap max_raw_io and bmc150 SPI support Mark Brown <broonie@kernel.org> - 2015-08-12 12:40 +0200
Re: [PATCH 00/20] Regmap max_raw_io and bmc150 SPI support Markus Pargmann <mpa@pengutronix.de> - 2015-08-12 12:50 +0200
Re: [PATCH 00/20] Regmap max_raw_io and bmc150 SPI support Mark Brown <broonie@kernel.org> - 2015-08-15 04:20 +0200
Page 1 of 2 [1] 2 Next page →
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 12:20 +0200 |
| Subject | [PATCH 00/20] Regmap max_raw_io and bmc150 SPI support |
| Message-ID | <pWBix-1oV-9@gated-at.bofh.it> |
Hi,
this series was created to add SPI support to the bmc150 accelerometer driver.
To not add any regressions, I had to add some infrastructure that allows to use
regmap with busses that do limit the size of transfers (block smbus). I hope
this is sufficient to not break anything.
The series has several different parts for regmap and the iio driver:
Patches 1-4 fix some minor things in the regmap core code.
Patches 5-11 fix some issues with regmap busses that do not have an
implementation of read() or write(). Currently a lot of the regmap API
functions simply fail for those busses.
Patches 12-14 introduce 'max_raw_io' for busses which defines the max number of
bytes that may be send or received by this bus. This includes handling in
regmap_bulk_read/write and reporting an error for raw_read/writes that are
above this limit (E2BIG).
Patch 15 makes use of max_raw_io by adding smbus i2c block bus driver. This
patch is created to avoid regressions in the bmc150 driver.
Patches 16-20 move bmc150 to use regmap and add SPI support.
As I don't have a bmc150 connected via smbus I am not able to test Patch 14 and
the resulting I2C part of the bmc150 driver. It would be great if someone with
the hardware could test it. Also it would be great to have some test coverage
for all other regmap setups.
Best Regards,
Markus
Markus Pargmann (20):
regmap: Add missing comments about struct regmap_bus
regmap: Remove regmap_bulk_write 64bit support
regmap: Fix integertypes for register address and value
regmap: Do not skip format initialization
regmap: Restructure writes in _regmap_raw_write()
regmap: Fix regmap_bulk_write for bus writes
regmap: Without bus read() or write(), force use_single_rw
regmap: Fix regmap_can_raw_write check
regmap: _regmap_raw_write fix for busses without write()
regmap: _regmap_raw_multi_reg_write: Add reg_write() support
regmap: _regmap_raw_read: Add handling of busses without bus->read()
regmap: Introduce max_raw_io for regmap_bulk_read/write
regmap: regmap max_raw_io getter function
regmap: Add raw_write/read checks for max_raw_write/read sizes
regmap-i2c: Add smbus i2c block support
iio: bmc150: Fix irq checks
iio: bmc150: Use i2c regmap
iio: bcm150: Remove i2c_client from private data
iio: bmc150: Split the driver into core and i2c
iio: bmc150: Add SPI driver
drivers/base/regmap/internal.h | 3 +
drivers/base/regmap/regmap-i2c.c | 46 +++
drivers/base/regmap/regmap.c | 221 ++++++++---
drivers/iio/accel/Kconfig | 32 +-
drivers/iio/accel/Makefile | 4 +-
.../accel/{bmc150-accel.c => bmc150-accel-core.c} | 404 ++++++++-------------
drivers/iio/accel/bmc150-accel-i2c.c | 101 ++++++
drivers/iio/accel/bmc150-accel-spi.c | 86 +++++
drivers/iio/accel/bmc150-accel.h | 21 ++
include/linux/regmap.h | 8 +-
10 files changed, 622 insertions(+), 304 deletions(-)
rename drivers/iio/accel/{bmc150-accel.c => bmc150-accel-core.c} (82%)
create mode 100644 drivers/iio/accel/bmc150-accel-i2c.c
create mode 100644 drivers/iio/accel/bmc150-accel-spi.c
create mode 100644 drivers/iio/accel/bmc150-accel.h
--
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 12:20 +0200 |
| Subject | [PATCH 17/20] iio: bmc150: Use i2c regmap |
| Message-ID | <pWBiz-1oV-61@gated-at.bofh.it> |
| In reply to | #1205901 |
This replaces all usage of direct i2c accesses with regmap accesses.
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
drivers/iio/accel/Kconfig | 2 +
drivers/iio/accel/bmc150-accel.c | 225 +++++++++++++++++----------------------
2 files changed, 101 insertions(+), 126 deletions(-)
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 00e7bcbdbe24..01dd03d194d1 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -22,6 +22,8 @@ config BMC150_ACCEL
depends on I2C
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
+ select REGMAP
+ select REGMAP_I2C
help
Say yes here to build support for the following Bosch accelerometers:
BMC150, BMI055, BMA250E, BMA222E, BMA255, BMA280.
diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
index fe2d2316158f..1484e956482e 100644
--- a/drivers/iio/accel/bmc150-accel.c
+++ b/drivers/iio/accel/bmc150-accel.c
@@ -35,6 +35,7 @@
#include <linux/iio/trigger.h>
#include <linux/iio/trigger_consumer.h>
#include <linux/iio/triggered_buffer.h>
+#include <linux/regmap.h>
#define BMC150_ACCEL_DRV_NAME "bmc150_accel"
#define BMC150_ACCEL_IRQ_NAME "bmc150_accel_event"
@@ -185,6 +186,8 @@ enum bmc150_accel_trigger_id {
struct bmc150_accel_data {
struct i2c_client *client;
+ struct regmap *regmap;
+ struct device *dev;
struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
atomic_t active_intr;
struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
@@ -241,6 +244,14 @@ static const struct {
{500000, BMC150_ACCEL_SLEEP_500_MS},
{1000000, BMC150_ACCEL_SLEEP_1_SEC} };
+static const struct regmap_config bmc150_i2c_regmap_conf = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x3f,
+
+ .use_single_rw = false,
+ .cache_type = REGCACHE_NONE,
+};
static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
enum bmc150_power_modes mode,
@@ -270,8 +281,7 @@ static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
dev_dbg(&data->client->dev, "Set Mode bits %x\n", lpw_bits);
- ret = i2c_smbus_write_byte_data(data->client,
- BMC150_ACCEL_REG_PMU_LPW, lpw_bits);
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_LPW, lpw_bits);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_pmu_lpw\n");
return ret;
@@ -289,8 +299,7 @@ static int bmc150_accel_set_bw(struct bmc150_accel_data *data, int val,
for (i = 0; i < ARRAY_SIZE(bmc150_accel_samp_freq_table); ++i) {
if (bmc150_accel_samp_freq_table[i].val == val &&
bmc150_accel_samp_freq_table[i].val2 == val2) {
- ret = i2c_smbus_write_byte_data(
- data->client,
+ ret = regmap_write(data->regmap,
BMC150_ACCEL_REG_PMU_BW,
bmc150_accel_samp_freq_table[i].bw_bits);
if (ret < 0)
@@ -307,26 +316,19 @@ static int bmc150_accel_set_bw(struct bmc150_accel_data *data, int val,
static int bmc150_accel_update_slope(struct bmc150_accel_data *data)
{
- int ret, val;
+ int ret;
- ret = i2c_smbus_write_byte_data(data->client, BMC150_ACCEL_REG_INT_6,
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_6,
data->slope_thres);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_int_6\n");
return ret;
}
- ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_INT_5);
+ ret = regmap_update_bits(data->regmap, BMC150_ACCEL_REG_INT_5,
+ BMC150_ACCEL_SLOPE_DUR_MASK, data->slope_dur);
if (ret < 0) {
- dev_err(&data->client->dev, "Error reading reg_int_5\n");
- return ret;
- }
-
- val = (ret & ~BMC150_ACCEL_SLOPE_DUR_MASK) | data->slope_dur;
- ret = i2c_smbus_write_byte_data(data->client, BMC150_ACCEL_REG_INT_5,
- val);
- if (ret < 0) {
- dev_err(&data->client->dev, "Error write reg_int_5\n");
+ dev_err(&data->client->dev, "Error updating reg_int_5\n");
return ret;
}
@@ -348,17 +350,18 @@ static int bmc150_accel_any_motion_setup(struct bmc150_accel_trigger *t,
static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
{
int ret;
+ unsigned int val;
- ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_CHIP_ID);
+ ret = regmap_read(data->regmap, BMC150_ACCEL_REG_CHIP_ID, &val);
if (ret < 0) {
dev_err(&data->client->dev,
"Error: Reading chip id\n");
return ret;
}
- dev_dbg(&data->client->dev, "Chip Id %x\n", ret);
- if (ret != data->chip_info->chip_id) {
- dev_err(&data->client->dev, "Invalid chip %x\n", ret);
+ dev_dbg(&data->client->dev, "Chip Id %x\n", val);
+ if (val != data->chip_info->chip_id) {
+ dev_err(&data->client->dev, "Invalid chip %x\n", val);
return -ENODEV;
}
@@ -372,9 +375,8 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
return ret;
/* Set Default Range */
- ret = i2c_smbus_write_byte_data(data->client,
- BMC150_ACCEL_REG_PMU_RANGE,
- BMC150_ACCEL_DEF_RANGE_4G);
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_RANGE,
+ BMC150_ACCEL_DEF_RANGE_4G);
if (ret < 0) {
dev_err(&data->client->dev,
"Error writing reg_pmu_range\n");
@@ -391,10 +393,9 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
return ret;
/* Set default as latched interrupts */
- ret = i2c_smbus_write_byte_data(data->client,
- BMC150_ACCEL_REG_INT_RST_LATCH,
- BMC150_ACCEL_INT_MODE_LATCH_INT |
- BMC150_ACCEL_INT_MODE_LATCH_RESET);
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
+ BMC150_ACCEL_INT_MODE_LATCH_INT |
+ BMC150_ACCEL_INT_MODE_LATCH_RESET);
if (ret < 0) {
dev_err(&data->client->dev,
"Error writing reg_int_rst_latch\n");
@@ -527,38 +528,18 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
return ret;
/* map the interrupt to the appropriate pins */
- ret = i2c_smbus_read_byte_data(data->client, info->map_reg);
- if (ret < 0) {
- dev_err(&data->client->dev, "Error reading reg_int_map\n");
- goto out_fix_power_state;
- }
- if (state)
- ret |= info->map_bitmask;
- else
- ret &= ~info->map_bitmask;
-
- ret = i2c_smbus_write_byte_data(data->client, info->map_reg,
- ret);
+ ret = regmap_update_bits(data->regmap, info->map_reg, info->map_bitmask,
+ (state ? info->map_bitmask : 0));
if (ret < 0) {
- dev_err(&data->client->dev, "Error writing reg_int_map\n");
+ dev_err(&data->client->dev, "Error updating reg_int_map\n");
goto out_fix_power_state;
}
/* enable/disable the interrupt */
- ret = i2c_smbus_read_byte_data(data->client, info->en_reg);
- if (ret < 0) {
- dev_err(&data->client->dev, "Error reading reg_int_en\n");
- goto out_fix_power_state;
- }
-
- if (state)
- ret |= info->en_bitmask;
- else
- ret &= ~info->en_bitmask;
-
- ret = i2c_smbus_write_byte_data(data->client, info->en_reg, ret);
+ ret = regmap_update_bits(data->regmap, info->en_reg, info->en_bitmask,
+ (state ? info->en_bitmask : 0));
if (ret < 0) {
- dev_err(&data->client->dev, "Error writing reg_int_en\n");
+ dev_err(&data->client->dev, "Error updating reg_int_en\n");
goto out_fix_power_state;
}
@@ -581,8 +562,7 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
for (i = 0; i < ARRAY_SIZE(data->chip_info->scale_table); ++i) {
if (data->chip_info->scale_table[i].scale == val) {
- ret = i2c_smbus_write_byte_data(
- data->client,
+ ret = regmap_write(data->regmap,
BMC150_ACCEL_REG_PMU_RANGE,
data->chip_info->scale_table[i].reg_range);
if (ret < 0) {
@@ -602,16 +582,17 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
static int bmc150_accel_get_temp(struct bmc150_accel_data *data, int *val)
{
int ret;
+ unsigned int value;
mutex_lock(&data->mutex);
- ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_TEMP);
+ ret = regmap_read(data->regmap, BMC150_ACCEL_REG_TEMP, &value);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_temp\n");
mutex_unlock(&data->mutex);
return ret;
}
- *val = sign_extend32(ret, 7);
+ *val = sign_extend32(value, 7);
mutex_unlock(&data->mutex);
@@ -624,6 +605,7 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
{
int ret;
int axis = chan->scan_index;
+ unsigned int raw_val;
mutex_lock(&data->mutex);
ret = bmc150_accel_set_power_state(data, true);
@@ -632,15 +614,15 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
return ret;
}
- ret = i2c_smbus_read_word_data(data->client,
- BMC150_ACCEL_AXIS_TO_REG(axis));
+ ret = regmap_bulk_read(data->regmap, BMC150_ACCEL_AXIS_TO_REG(axis),
+ &raw_val, 2);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading axis %d\n", axis);
bmc150_accel_set_power_state(data, false);
mutex_unlock(&data->mutex);
return ret;
}
- *val = sign_extend32(ret >> chan->scan_type.shift,
+ *val = sign_extend32(raw_val >> chan->scan_type.shift,
chan->scan_type.realbits - 1);
ret = bmc150_accel_set_power_state(data, false);
mutex_unlock(&data->mutex);
@@ -904,52 +886,37 @@ static int bmc150_accel_set_watermark(struct iio_dev *indio_dev, unsigned val)
* We must read at least one full frame in one burst, otherwise the rest of the
* frame data is discarded.
*/
-static int bmc150_accel_fifo_transfer(const struct i2c_client *client,
+static int bmc150_accel_fifo_transfer(struct bmc150_accel_data *data,
char *buffer, int samples)
{
int sample_length = 3 * 2;
- u8 reg_fifo_data = BMC150_ACCEL_REG_FIFO_DATA;
- int ret = -EIO;
-
- if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
- struct i2c_msg msg[2] = {
- {
- .addr = client->addr,
- .flags = 0,
- .buf = ®_fifo_data,
- .len = sizeof(reg_fifo_data),
- },
- {
- .addr = client->addr,
- .flags = I2C_M_RD,
- .buf = (u8 *)buffer,
- .len = samples * sample_length,
- }
- };
+ int ret;
+ int total_length = samples * sample_length;
+ int i, step;
- ret = i2c_transfer(client->adapter, msg, 2);
- if (ret != 2)
- ret = -EIO;
- else
- ret = 0;
- } else {
- int i, step = I2C_SMBUS_BLOCK_MAX / sample_length;
-
- for (i = 0; i < samples * sample_length; i += step) {
- ret = i2c_smbus_read_i2c_block_data(client,
- reg_fifo_data, step,
- &buffer[i]);
- if (ret != step) {
- ret = -EIO;
- break;
- }
+ ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA, buffer,
+ total_length);
+ if (ret != -E2BIG) {
+ if (ret)
+ dev_err(data->dev, "Error transferring data from fifo\n");
+ return ret;
+ }
- ret = 0;
- }
+ /*
+ * Seems we have a bus with size limitation so we have to execute
+ * multiple reads
+ */
+ step = regmap_get_raw_io_max(data->regmap) / sample_length;
+ for (i = -1; i < samples * sample_length; i += step) {
+ ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA,
+ &buffer[i], step);
+ if (ret)
+ break;
}
if (ret)
- dev_err(&client->dev, "Error transferring data from fifo\n");
+ dev_err(data->dev, "Error transferring data from fifo in single steps of %zu\n",
+ step);
return ret;
}
@@ -963,14 +930,15 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,
u16 buffer[BMC150_ACCEL_FIFO_LENGTH * 3];
int64_t tstamp;
uint64_t sample_period;
- ret = i2c_smbus_read_byte_data(data->client,
- BMC150_ACCEL_REG_FIFO_STATUS);
+ unsigned int val;
+
+ ret = regmap_read(data->regmap, BMC150_ACCEL_REG_FIFO_STATUS, &val);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_fifo_status\n");
return ret;
}
- count = ret & 0x7F;
+ count = val & 0x7F;
if (!count)
return 0;
@@ -1009,7 +977,7 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,
if (samples && count > samples)
count = samples;
- ret = bmc150_accel_fifo_transfer(data->client, (u8 *)buffer, count);
+ ret = bmc150_accel_fifo_transfer(data, (u8 *)buffer, count);
if (ret)
return ret;
@@ -1206,17 +1174,19 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
struct iio_dev *indio_dev = pf->indio_dev;
struct bmc150_accel_data *data = iio_priv(indio_dev);
int bit, ret, i = 0;
+ unsigned int raw_val;
mutex_lock(&data->mutex);
for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength) {
- ret = i2c_smbus_read_word_data(data->client,
- BMC150_ACCEL_AXIS_TO_REG(bit));
+ ret = regmap_bulk_read(data->regmap,
+ BMC150_ACCEL_AXIS_TO_REG(bit), &raw_val,
+ 2);
if (ret < 0) {
mutex_unlock(&data->mutex);
goto err_read;
}
- data->buffer[i++] = ret;
+ data->buffer[i++] = raw_val;
}
mutex_unlock(&data->mutex);
@@ -1240,10 +1210,9 @@ static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
mutex_lock(&data->mutex);
/* clear any latched interrupt */
- ret = i2c_smbus_write_byte_data(data->client,
- BMC150_ACCEL_REG_INT_RST_LATCH,
- BMC150_ACCEL_INT_MODE_LATCH_INT |
- BMC150_ACCEL_INT_MODE_LATCH_RESET);
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
+ BMC150_ACCEL_INT_MODE_LATCH_INT |
+ BMC150_ACCEL_INT_MODE_LATCH_RESET);
mutex_unlock(&data->mutex);
if (ret < 0) {
dev_err(&data->client->dev,
@@ -1300,34 +1269,34 @@ static int bmc150_accel_handle_roc_event(struct iio_dev *indio_dev)
struct bmc150_accel_data *data = iio_priv(indio_dev);
int dir;
int ret;
+ unsigned int val;
- ret = i2c_smbus_read_byte_data(data->client,
- BMC150_ACCEL_REG_INT_STATUS_2);
+ ret = regmap_read(data->regmap, BMC150_ACCEL_REG_INT_STATUS_2, &val);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_int_status_2\n");
return ret;
}
- if (ret & BMC150_ACCEL_ANY_MOTION_BIT_SIGN)
+ if (val & BMC150_ACCEL_ANY_MOTION_BIT_SIGN)
dir = IIO_EV_DIR_FALLING;
else
dir = IIO_EV_DIR_RISING;
- if (ret & BMC150_ACCEL_ANY_MOTION_BIT_X)
+ if (val & BMC150_ACCEL_ANY_MOTION_BIT_X)
iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
0,
IIO_MOD_X,
IIO_EV_TYPE_ROC,
dir),
data->timestamp);
- if (ret & BMC150_ACCEL_ANY_MOTION_BIT_Y)
+ if (val & BMC150_ACCEL_ANY_MOTION_BIT_Y)
iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
0,
IIO_MOD_Y,
IIO_EV_TYPE_ROC,
dir),
data->timestamp);
- if (ret & BMC150_ACCEL_ANY_MOTION_BIT_Z)
+ if (val & BMC150_ACCEL_ANY_MOTION_BIT_Z)
iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
0,
IIO_MOD_Z,
@@ -1360,10 +1329,9 @@ static irqreturn_t bmc150_accel_irq_thread_handler(int irq, void *private)
}
if (ack) {
- ret = i2c_smbus_write_byte_data(data->client,
- BMC150_ACCEL_REG_INT_RST_LATCH,
- BMC150_ACCEL_INT_MODE_LATCH_INT |
- BMC150_ACCEL_INT_MODE_LATCH_RESET);
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
+ BMC150_ACCEL_INT_MODE_LATCH_INT |
+ BMC150_ACCEL_INT_MODE_LATCH_RESET);
if (ret)
dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
ret = IRQ_HANDLED;
@@ -1516,7 +1484,7 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
u8 reg = BMC150_ACCEL_REG_FIFO_CONFIG1;
int ret;
- ret = i2c_smbus_write_byte_data(data->client, reg, data->fifo_mode);
+ ret = regmap_write(data->regmap, reg, data->fifo_mode);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_fifo_config1\n");
return ret;
@@ -1525,9 +1493,8 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
if (!data->fifo_mode)
return 0;
- ret = i2c_smbus_write_byte_data(data->client,
- BMC150_ACCEL_REG_FIFO_CONFIG0,
- data->watermark);
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_FIFO_CONFIG0,
+ data->watermark);
if (ret < 0)
dev_err(&data->client->dev, "Error writing reg_fifo_config0\n");
@@ -1627,6 +1594,13 @@ static int bmc150_accel_probe(struct i2c_client *client,
data = iio_priv(indio_dev);
i2c_set_clientdata(client, indio_dev);
data->client = client;
+ data->dev = &client->dev;
+
+ data->regmap = devm_regmap_init_i2c(client, &bmc150_i2c_regmap_conf);
+ if (IS_ERR(data->regmap)) {
+ dev_err(&client->dev, "Failed to initialize i2c regmap\n");
+ return PTR_ERR(data->regmap);
+ }
if (id) {
name = id->name;
@@ -1680,9 +1654,8 @@ static int bmc150_accel_probe(struct i2c_client *client,
* want to use latch mode when we can to prevent interrupt
* flooding.
*/
- ret = i2c_smbus_write_byte_data(data->client,
- BMC150_ACCEL_REG_INT_RST_LATCH,
- BMC150_ACCEL_INT_MODE_LATCH_RESET);
+ ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
+ BMC150_ACCEL_INT_MODE_LATCH_RESET);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
goto err_buffer_cleanup;
--
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-08-12 14:10 +0200 |
| Subject | Re: [PATCH 17/20] iio: bmc150: Use i2c regmap |
| Message-ID | <pWD12-3Uo-53@gated-at.bofh.it> |
| In reply to | #1205902 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 12:12:42PM +0200, Markus Pargmann wrote: > + select REGMAP > + select REGMAP_I2C You don't need to select regmap, only REGMAP_I2C.
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 15:00 +0200 |
| Subject | Re: [PATCH 17/20] iio: bmc150: Use i2c regmap |
| Message-ID | <pWDNn-4Q9-5@gated-at.bofh.it> |
| In reply to | #1205999 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 01:01:46PM +0100, Mark Brown wrote: > On Wed, Aug 12, 2015 at 12:12:42PM +0200, Markus Pargmann wrote: > > > + select REGMAP > > + select REGMAP_I2C > > You don't need to select regmap, only REGMAP_I2C. Ok, thanks, will fix. Best Regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2015-08-15 15:30 +0200 |
| Subject | Re: [PATCH 17/20] iio: bmc150: Use i2c regmap |
| Message-ID | <pXJH4-2GT-17@gated-at.bofh.it> |
| In reply to | #1205902 |
On 12/08/15 11:12, Markus Pargmann wrote:
> This replaces all usage of direct i2c accesses with regmap accesses.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Clearly there is some work needed on the earlier patches and this
might change as a result (particularly the fifo read). I'll review
as is however..
Few bits on top of what other reviews have highlighted...
Jonathan
> ---
> drivers/iio/accel/Kconfig | 2 +
> drivers/iio/accel/bmc150-accel.c | 225 +++++++++++++++++----------------------
> 2 files changed, 101 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 00e7bcbdbe24..01dd03d194d1 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -22,6 +22,8 @@ config BMC150_ACCEL
> depends on I2C
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> + select REGMAP
> + select REGMAP_I2C
> help
> Say yes here to build support for the following Bosch accelerometers:
> BMC150, BMI055, BMA250E, BMA222E, BMA255, BMA280.
> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> index fe2d2316158f..1484e956482e 100644
> --- a/drivers/iio/accel/bmc150-accel.c
> +++ b/drivers/iio/accel/bmc150-accel.c
> @@ -35,6 +35,7 @@
> #include <linux/iio/trigger.h>
> #include <linux/iio/trigger_consumer.h>
> #include <linux/iio/triggered_buffer.h>
> +#include <linux/regmap.h>
>
> #define BMC150_ACCEL_DRV_NAME "bmc150_accel"
> #define BMC150_ACCEL_IRQ_NAME "bmc150_accel_event"
> @@ -185,6 +186,8 @@ enum bmc150_accel_trigger_id {
>
> struct bmc150_accel_data {
> struct i2c_client *client;
> + struct regmap *regmap;
> + struct device *dev;
> struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
> atomic_t active_intr;
> struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
> @@ -241,6 +244,14 @@ static const struct {
> {500000, BMC150_ACCEL_SLEEP_500_MS},
> {1000000, BMC150_ACCEL_SLEEP_1_SEC} };
>
> +static const struct regmap_config bmc150_i2c_regmap_conf = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x3f,
> +
> + .use_single_rw = false,
> + .cache_type = REGCACHE_NONE,
> +};
>
> static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
> enum bmc150_power_modes mode,
> @@ -270,8 +281,7 @@ static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
>
> dev_dbg(&data->client->dev, "Set Mode bits %x\n", lpw_bits);
>
> - ret = i2c_smbus_write_byte_data(data->client,
> - BMC150_ACCEL_REG_PMU_LPW, lpw_bits);
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_LPW, lpw_bits);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_pmu_lpw\n");
> return ret;
> @@ -289,8 +299,7 @@ static int bmc150_accel_set_bw(struct bmc150_accel_data *data, int val,
> for (i = 0; i < ARRAY_SIZE(bmc150_accel_samp_freq_table); ++i) {
> if (bmc150_accel_samp_freq_table[i].val == val &&
> bmc150_accel_samp_freq_table[i].val2 == val2) {
> - ret = i2c_smbus_write_byte_data(
> - data->client,
> + ret = regmap_write(data->regmap,
> BMC150_ACCEL_REG_PMU_BW,
> bmc150_accel_samp_freq_table[i].bw_bits);
> if (ret < 0)
> @@ -307,26 +316,19 @@ static int bmc150_accel_set_bw(struct bmc150_accel_data *data, int val,
>
> static int bmc150_accel_update_slope(struct bmc150_accel_data *data)
> {
> - int ret, val;
> + int ret;
>
> - ret = i2c_smbus_write_byte_data(data->client, BMC150_ACCEL_REG_INT_6,
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_6,
> data->slope_thres);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_int_6\n");
> return ret;
> }
>
> - ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_INT_5);
> + ret = regmap_update_bits(data->regmap, BMC150_ACCEL_REG_INT_5,
> + BMC150_ACCEL_SLOPE_DUR_MASK, data->slope_dur);
> if (ret < 0) {
> - dev_err(&data->client->dev, "Error reading reg_int_5\n");
> - return ret;
> - }
> -
> - val = (ret & ~BMC150_ACCEL_SLOPE_DUR_MASK) | data->slope_dur;
> - ret = i2c_smbus_write_byte_data(data->client, BMC150_ACCEL_REG_INT_5,
> - val);
> - if (ret < 0) {
> - dev_err(&data->client->dev, "Error write reg_int_5\n");
> + dev_err(&data->client->dev, "Error updating reg_int_5\n");
> return ret;
> }
>
> @@ -348,17 +350,18 @@ static int bmc150_accel_any_motion_setup(struct bmc150_accel_trigger *t,
> static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
> {
> int ret;
> + unsigned int val;
>
> - ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_CHIP_ID);
> + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_CHIP_ID, &val);
> if (ret < 0) {
> dev_err(&data->client->dev,
> "Error: Reading chip id\n");
> return ret;
> }
>
> - dev_dbg(&data->client->dev, "Chip Id %x\n", ret);
> - if (ret != data->chip_info->chip_id) {
> - dev_err(&data->client->dev, "Invalid chip %x\n", ret);
> + dev_dbg(&data->client->dev, "Chip Id %x\n", val);
> + if (val != data->chip_info->chip_id) {
> + dev_err(&data->client->dev, "Invalid chip %x\n", val);
> return -ENODEV;
> }
>
> @@ -372,9 +375,8 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
> return ret;
>
> /* Set Default Range */
> - ret = i2c_smbus_write_byte_data(data->client,
> - BMC150_ACCEL_REG_PMU_RANGE,
> - BMC150_ACCEL_DEF_RANGE_4G);
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_RANGE,
> + BMC150_ACCEL_DEF_RANGE_4G);
> if (ret < 0) {
> dev_err(&data->client->dev,
> "Error writing reg_pmu_range\n");
> @@ -391,10 +393,9 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
> return ret;
>
> /* Set default as latched interrupts */
> - ret = i2c_smbus_write_byte_data(data->client,
> - BMC150_ACCEL_REG_INT_RST_LATCH,
> - BMC150_ACCEL_INT_MODE_LATCH_INT |
> - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> + BMC150_ACCEL_INT_MODE_LATCH_INT |
> + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> if (ret < 0) {
> dev_err(&data->client->dev,
> "Error writing reg_int_rst_latch\n");
> @@ -527,38 +528,18 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
> return ret;
>
> /* map the interrupt to the appropriate pins */
> - ret = i2c_smbus_read_byte_data(data->client, info->map_reg);
> - if (ret < 0) {
> - dev_err(&data->client->dev, "Error reading reg_int_map\n");
> - goto out_fix_power_state;
> - }
> - if (state)
> - ret |= info->map_bitmask;
> - else
> - ret &= ~info->map_bitmask;
> -
> - ret = i2c_smbus_write_byte_data(data->client, info->map_reg,
> - ret);
> + ret = regmap_update_bits(data->regmap, info->map_reg, info->map_bitmask,
> + (state ? info->map_bitmask : 0));
> if (ret < 0) {
> - dev_err(&data->client->dev, "Error writing reg_int_map\n");
> + dev_err(&data->client->dev, "Error updating reg_int_map\n");
> goto out_fix_power_state;
> }
>
> /* enable/disable the interrupt */
> - ret = i2c_smbus_read_byte_data(data->client, info->en_reg);
> - if (ret < 0) {
> - dev_err(&data->client->dev, "Error reading reg_int_en\n");
> - goto out_fix_power_state;
> - }
> -
> - if (state)
> - ret |= info->en_bitmask;
> - else
> - ret &= ~info->en_bitmask;
> -
> - ret = i2c_smbus_write_byte_data(data->client, info->en_reg, ret);
> + ret = regmap_update_bits(data->regmap, info->en_reg, info->en_bitmask,
> + (state ? info->en_bitmask : 0));
> if (ret < 0) {
> - dev_err(&data->client->dev, "Error writing reg_int_en\n");
> + dev_err(&data->client->dev, "Error updating reg_int_en\n");
> goto out_fix_power_state;
> }
>
> @@ -581,8 +562,7 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
>
> for (i = 0; i < ARRAY_SIZE(data->chip_info->scale_table); ++i) {
> if (data->chip_info->scale_table[i].scale == val) {
> - ret = i2c_smbus_write_byte_data(
> - data->client,
> + ret = regmap_write(data->regmap,
> BMC150_ACCEL_REG_PMU_RANGE,
> data->chip_info->scale_table[i].reg_range);
> if (ret < 0) {
> @@ -602,16 +582,17 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
> static int bmc150_accel_get_temp(struct bmc150_accel_data *data, int *val)
> {
> int ret;
> + unsigned int value;
>
> mutex_lock(&data->mutex);
>
> - ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_TEMP);
> + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_TEMP, &value);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_temp\n");
> mutex_unlock(&data->mutex);
> return ret;
> }
> - *val = sign_extend32(ret, 7);
> + *val = sign_extend32(value, 7);
>
> mutex_unlock(&data->mutex);
>
> @@ -624,6 +605,7 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
> {
> int ret;
> int axis = chan->scan_index;
> + unsigned int raw_val;
>
> mutex_lock(&data->mutex);
> ret = bmc150_accel_set_power_state(data, true);
> @@ -632,15 +614,15 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
> return ret;
> }
>
> - ret = i2c_smbus_read_word_data(data->client,
> - BMC150_ACCEL_AXIS_TO_REG(axis));
> + ret = regmap_bulk_read(data->regmap, BMC150_ACCEL_AXIS_TO_REG(axis),
> + &raw_val, 2);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading axis %d\n", axis);
> bmc150_accel_set_power_state(data, false);
> mutex_unlock(&data->mutex);
> return ret;
> }
> - *val = sign_extend32(ret >> chan->scan_type.shift,
> + *val = sign_extend32(raw_val >> chan->scan_type.shift,
> chan->scan_type.realbits - 1);
> ret = bmc150_accel_set_power_state(data, false);
> mutex_unlock(&data->mutex);
> @@ -904,52 +886,37 @@ static int bmc150_accel_set_watermark(struct iio_dev *indio_dev, unsigned val)
> * We must read at least one full frame in one burst, otherwise the rest of the
> * frame data is discarded.
> */
> -static int bmc150_accel_fifo_transfer(const struct i2c_client *client,
> +static int bmc150_accel_fifo_transfer(struct bmc150_accel_data *data,
> char *buffer, int samples)
> {
> int sample_length = 3 * 2;
> - u8 reg_fifo_data = BMC150_ACCEL_REG_FIFO_DATA;
> - int ret = -EIO;
> -
> - if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> - struct i2c_msg msg[2] = {
> - {
> - .addr = client->addr,
> - .flags = 0,
> - .buf = ®_fifo_data,
> - .len = sizeof(reg_fifo_data),
> - },
> - {
> - .addr = client->addr,
> - .flags = I2C_M_RD,
> - .buf = (u8 *)buffer,
> - .len = samples * sample_length,
> - }
> - };
> + int ret;
> + int total_length = samples * sample_length;
> + int i, step;
>
> - ret = i2c_transfer(client->adapter, msg, 2);
> - if (ret != 2)
> - ret = -EIO;
> - else
> - ret = 0;
> - } else {
> - int i, step = I2C_SMBUS_BLOCK_MAX / sample_length;
> -
> - for (i = 0; i < samples * sample_length; i += step) {
> - ret = i2c_smbus_read_i2c_block_data(client,
> - reg_fifo_data, step,
> - &buffer[i]);
> - if (ret != step) {
> - ret = -EIO;
> - break;
> - }
> + ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA, buffer,
> + total_length);
> + if (ret != -E2BIG) {
> + if (ret)
I'd invert the logic for more readability.
if (ret == -E2BIT) {
...
} else if (ret) {
...
} else {
return ret;
}
> + dev_err(data->dev, "Error transferring data from fifo\n");
> + return ret;
> + }
>
> - ret = 0;
> - }
> + /*
> + * Seems we have a bus with size limitation so we have to execute
> + * multiple reads
> + */
Can we not just query this in advance before going through the previous
failed call? THat would be cleaner to my mind.
> + step = regmap_get_raw_io_max(data->regmap) / sample_length;
> + for (i = -1; i < samples * sample_length; i += step) {
> + ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA,
> + &buffer[i], step);
umm. Can't say I like the negative index into buffer. Why is it
necessary?
> + if (ret)
> + break;
> }
>
> if (ret)
> - dev_err(&client->dev, "Error transferring data from fifo\n");
> + dev_err(data->dev, "Error transferring data from fifo in single steps of %zu\n",
multiple steps of %zu perhaps?
> + step);
>
> return ret;
> }
> @@ -963,14 +930,15 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,
> u16 buffer[BMC150_ACCEL_FIFO_LENGTH * 3];
> int64_t tstamp;
> uint64_t sample_period;
> - ret = i2c_smbus_read_byte_data(data->client,
> - BMC150_ACCEL_REG_FIFO_STATUS);
> + unsigned int val;
> +
> + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_FIFO_STATUS, &val);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_fifo_status\n");
> return ret;
> }
>
> - count = ret & 0x7F;
> + count = val & 0x7F;
>
> if (!count)
> return 0;
> @@ -1009,7 +977,7 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,
> if (samples && count > samples)
> count = samples;
>
> - ret = bmc150_accel_fifo_transfer(data->client, (u8 *)buffer, count);
> + ret = bmc150_accel_fifo_transfer(data, (u8 *)buffer, count);
> if (ret)
> return ret;
>
> @@ -1206,17 +1174,19 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
> struct iio_dev *indio_dev = pf->indio_dev;
> struct bmc150_accel_data *data = iio_priv(indio_dev);
> int bit, ret, i = 0;
> + unsigned int raw_val;
>
> mutex_lock(&data->mutex);
> for_each_set_bit(bit, indio_dev->active_scan_mask,
> indio_dev->masklength) {
> - ret = i2c_smbus_read_word_data(data->client,
> - BMC150_ACCEL_AXIS_TO_REG(bit));
> + ret = regmap_bulk_read(data->regmap,
> + BMC150_ACCEL_AXIS_TO_REG(bit), &raw_val,
> + 2);
Is using a variable on the stack not going to cause issues when we add
SPI? (cacheline dma requirements).
> if (ret < 0) {
> mutex_unlock(&data->mutex);
> goto err_read;
> }
> - data->buffer[i++] = ret;
> + data->buffer[i++] = raw_val;
> }
> mutex_unlock(&data->mutex);
>
> @@ -1240,10 +1210,9 @@ static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
>
> mutex_lock(&data->mutex);
> /* clear any latched interrupt */
> - ret = i2c_smbus_write_byte_data(data->client,
> - BMC150_ACCEL_REG_INT_RST_LATCH,
> - BMC150_ACCEL_INT_MODE_LATCH_INT |
> - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> + BMC150_ACCEL_INT_MODE_LATCH_INT |
> + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> mutex_unlock(&data->mutex);
> if (ret < 0) {
> dev_err(&data->client->dev,
> @@ -1300,34 +1269,34 @@ static int bmc150_accel_handle_roc_event(struct iio_dev *indio_dev)
> struct bmc150_accel_data *data = iio_priv(indio_dev);
> int dir;
> int ret;
> + unsigned int val;
>
> - ret = i2c_smbus_read_byte_data(data->client,
> - BMC150_ACCEL_REG_INT_STATUS_2);
> + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_INT_STATUS_2, &val);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_int_status_2\n");
> return ret;
> }
>
> - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_SIGN)
> + if (val & BMC150_ACCEL_ANY_MOTION_BIT_SIGN)
> dir = IIO_EV_DIR_FALLING;
> else
> dir = IIO_EV_DIR_RISING;
>
> - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_X)
> + if (val & BMC150_ACCEL_ANY_MOTION_BIT_X)
> iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
> 0,
> IIO_MOD_X,
> IIO_EV_TYPE_ROC,
> dir),
> data->timestamp);
> - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_Y)
> + if (val & BMC150_ACCEL_ANY_MOTION_BIT_Y)
> iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
> 0,
> IIO_MOD_Y,
> IIO_EV_TYPE_ROC,
> dir),
> data->timestamp);
> - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_Z)
> + if (val & BMC150_ACCEL_ANY_MOTION_BIT_Z)
> iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
> 0,
> IIO_MOD_Z,
> @@ -1360,10 +1329,9 @@ static irqreturn_t bmc150_accel_irq_thread_handler(int irq, void *private)
> }
>
> if (ack) {
> - ret = i2c_smbus_write_byte_data(data->client,
> - BMC150_ACCEL_REG_INT_RST_LATCH,
> - BMC150_ACCEL_INT_MODE_LATCH_INT |
> - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> + BMC150_ACCEL_INT_MODE_LATCH_INT |
> + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> if (ret)
> dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
> ret = IRQ_HANDLED;
> @@ -1516,7 +1484,7 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
> u8 reg = BMC150_ACCEL_REG_FIFO_CONFIG1;
> int ret;
>
> - ret = i2c_smbus_write_byte_data(data->client, reg, data->fifo_mode);
> + ret = regmap_write(data->regmap, reg, data->fifo_mode);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_fifo_config1\n");
> return ret;
> @@ -1525,9 +1493,8 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
> if (!data->fifo_mode)
> return 0;
>
> - ret = i2c_smbus_write_byte_data(data->client,
> - BMC150_ACCEL_REG_FIFO_CONFIG0,
> - data->watermark);
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_FIFO_CONFIG0,
> + data->watermark);
> if (ret < 0)
> dev_err(&data->client->dev, "Error writing reg_fifo_config0\n");
>
> @@ -1627,6 +1594,13 @@ static int bmc150_accel_probe(struct i2c_client *client,
> data = iio_priv(indio_dev);
> i2c_set_clientdata(client, indio_dev);
> data->client = client;
> + data->dev = &client->dev;
> +
> + data->regmap = devm_regmap_init_i2c(client, &bmc150_i2c_regmap_conf);
> + if (IS_ERR(data->regmap)) {
> + dev_err(&client->dev, "Failed to initialize i2c regmap\n");
> + return PTR_ERR(data->regmap);
> + }
>
> if (id) {
> name = id->name;
> @@ -1680,9 +1654,8 @@ static int bmc150_accel_probe(struct i2c_client *client,
> * want to use latch mode when we can to prevent interrupt
> * flooding.
> */
> - ret = i2c_smbus_write_byte_data(data->client,
> - BMC150_ACCEL_REG_INT_RST_LATCH,
> - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
> goto err_buffer_cleanup;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-17 09:50 +0200 |
| Subject | Re: [PATCH 17/20] iio: bmc150: Use i2c regmap |
| Message-ID | <pYnl8-Qf-27@gated-at.bofh.it> |
| In reply to | #1208081 |
[Multipart message — attachments visible in raw view] — view raw
On Sat, Aug 15, 2015 at 02:27:47PM +0100, Jonathan Cameron wrote:
> On 12/08/15 11:12, Markus Pargmann wrote:
> > This replaces all usage of direct i2c accesses with regmap accesses.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> Clearly there is some work needed on the earlier patches and this
> might change as a result (particularly the fifo read). I'll review
> as is however..
>
> Few bits on top of what other reviews have highlighted...
>
> Jonathan
> > ---
> > drivers/iio/accel/Kconfig | 2 +
> > drivers/iio/accel/bmc150-accel.c | 225 +++++++++++++++++----------------------
> > 2 files changed, 101 insertions(+), 126 deletions(-)
> >
> > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> > index 00e7bcbdbe24..01dd03d194d1 100644
> > --- a/drivers/iio/accel/Kconfig
> > +++ b/drivers/iio/accel/Kconfig
> > @@ -22,6 +22,8 @@ config BMC150_ACCEL
> > depends on I2C
> > select IIO_BUFFER
> > select IIO_TRIGGERED_BUFFER
> > + select REGMAP
> > + select REGMAP_I2C
> > help
> > Say yes here to build support for the following Bosch accelerometers:
> > BMC150, BMI055, BMA250E, BMA222E, BMA255, BMA280.
> > diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> > index fe2d2316158f..1484e956482e 100644
> > --- a/drivers/iio/accel/bmc150-accel.c
> > +++ b/drivers/iio/accel/bmc150-accel.c
> > @@ -35,6 +35,7 @@
> > #include <linux/iio/trigger.h>
> > #include <linux/iio/trigger_consumer.h>
> > #include <linux/iio/triggered_buffer.h>
> > +#include <linux/regmap.h>
> >
> > #define BMC150_ACCEL_DRV_NAME "bmc150_accel"
> > #define BMC150_ACCEL_IRQ_NAME "bmc150_accel_event"
> > @@ -185,6 +186,8 @@ enum bmc150_accel_trigger_id {
> >
> > struct bmc150_accel_data {
> > struct i2c_client *client;
> > + struct regmap *regmap;
> > + struct device *dev;
> > struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
> > atomic_t active_intr;
> > struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
> > @@ -241,6 +244,14 @@ static const struct {
> > {500000, BMC150_ACCEL_SLEEP_500_MS},
> > {1000000, BMC150_ACCEL_SLEEP_1_SEC} };
> >
> > +static const struct regmap_config bmc150_i2c_regmap_conf = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > + .max_register = 0x3f,
> > +
> > + .use_single_rw = false,
> > + .cache_type = REGCACHE_NONE,
> > +};
> >
> > static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
> > enum bmc150_power_modes mode,
> > @@ -270,8 +281,7 @@ static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
> >
> > dev_dbg(&data->client->dev, "Set Mode bits %x\n", lpw_bits);
> >
> > - ret = i2c_smbus_write_byte_data(data->client,
> > - BMC150_ACCEL_REG_PMU_LPW, lpw_bits);
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_LPW, lpw_bits);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error writing reg_pmu_lpw\n");
> > return ret;
> > @@ -289,8 +299,7 @@ static int bmc150_accel_set_bw(struct bmc150_accel_data *data, int val,
> > for (i = 0; i < ARRAY_SIZE(bmc150_accel_samp_freq_table); ++i) {
> > if (bmc150_accel_samp_freq_table[i].val == val &&
> > bmc150_accel_samp_freq_table[i].val2 == val2) {
> > - ret = i2c_smbus_write_byte_data(
> > - data->client,
> > + ret = regmap_write(data->regmap,
> > BMC150_ACCEL_REG_PMU_BW,
> > bmc150_accel_samp_freq_table[i].bw_bits);
> > if (ret < 0)
> > @@ -307,26 +316,19 @@ static int bmc150_accel_set_bw(struct bmc150_accel_data *data, int val,
> >
> > static int bmc150_accel_update_slope(struct bmc150_accel_data *data)
> > {
> > - int ret, val;
> > + int ret;
> >
> > - ret = i2c_smbus_write_byte_data(data->client, BMC150_ACCEL_REG_INT_6,
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_6,
> > data->slope_thres);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error writing reg_int_6\n");
> > return ret;
> > }
> >
> > - ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_INT_5);
> > + ret = regmap_update_bits(data->regmap, BMC150_ACCEL_REG_INT_5,
> > + BMC150_ACCEL_SLOPE_DUR_MASK, data->slope_dur);
> > if (ret < 0) {
> > - dev_err(&data->client->dev, "Error reading reg_int_5\n");
> > - return ret;
> > - }
> > -
> > - val = (ret & ~BMC150_ACCEL_SLOPE_DUR_MASK) | data->slope_dur;
> > - ret = i2c_smbus_write_byte_data(data->client, BMC150_ACCEL_REG_INT_5,
> > - val);
> > - if (ret < 0) {
> > - dev_err(&data->client->dev, "Error write reg_int_5\n");
> > + dev_err(&data->client->dev, "Error updating reg_int_5\n");
> > return ret;
> > }
> >
> > @@ -348,17 +350,18 @@ static int bmc150_accel_any_motion_setup(struct bmc150_accel_trigger *t,
> > static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
> > {
> > int ret;
> > + unsigned int val;
> >
> > - ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_CHIP_ID);
> > + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_CHIP_ID, &val);
> > if (ret < 0) {
> > dev_err(&data->client->dev,
> > "Error: Reading chip id\n");
> > return ret;
> > }
> >
> > - dev_dbg(&data->client->dev, "Chip Id %x\n", ret);
> > - if (ret != data->chip_info->chip_id) {
> > - dev_err(&data->client->dev, "Invalid chip %x\n", ret);
> > + dev_dbg(&data->client->dev, "Chip Id %x\n", val);
> > + if (val != data->chip_info->chip_id) {
> > + dev_err(&data->client->dev, "Invalid chip %x\n", val);
> > return -ENODEV;
> > }
> >
> > @@ -372,9 +375,8 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
> > return ret;
> >
> > /* Set Default Range */
> > - ret = i2c_smbus_write_byte_data(data->client,
> > - BMC150_ACCEL_REG_PMU_RANGE,
> > - BMC150_ACCEL_DEF_RANGE_4G);
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_RANGE,
> > + BMC150_ACCEL_DEF_RANGE_4G);
> > if (ret < 0) {
> > dev_err(&data->client->dev,
> > "Error writing reg_pmu_range\n");
> > @@ -391,10 +393,9 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
> > return ret;
> >
> > /* Set default as latched interrupts */
> > - ret = i2c_smbus_write_byte_data(data->client,
> > - BMC150_ACCEL_REG_INT_RST_LATCH,
> > - BMC150_ACCEL_INT_MODE_LATCH_INT |
> > - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> > + BMC150_ACCEL_INT_MODE_LATCH_INT |
> > + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > if (ret < 0) {
> > dev_err(&data->client->dev,
> > "Error writing reg_int_rst_latch\n");
> > @@ -527,38 +528,18 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
> > return ret;
> >
> > /* map the interrupt to the appropriate pins */
> > - ret = i2c_smbus_read_byte_data(data->client, info->map_reg);
> > - if (ret < 0) {
> > - dev_err(&data->client->dev, "Error reading reg_int_map\n");
> > - goto out_fix_power_state;
> > - }
> > - if (state)
> > - ret |= info->map_bitmask;
> > - else
> > - ret &= ~info->map_bitmask;
> > -
> > - ret = i2c_smbus_write_byte_data(data->client, info->map_reg,
> > - ret);
> > + ret = regmap_update_bits(data->regmap, info->map_reg, info->map_bitmask,
> > + (state ? info->map_bitmask : 0));
> > if (ret < 0) {
> > - dev_err(&data->client->dev, "Error writing reg_int_map\n");
> > + dev_err(&data->client->dev, "Error updating reg_int_map\n");
> > goto out_fix_power_state;
> > }
> >
> > /* enable/disable the interrupt */
> > - ret = i2c_smbus_read_byte_data(data->client, info->en_reg);
> > - if (ret < 0) {
> > - dev_err(&data->client->dev, "Error reading reg_int_en\n");
> > - goto out_fix_power_state;
> > - }
> > -
> > - if (state)
> > - ret |= info->en_bitmask;
> > - else
> > - ret &= ~info->en_bitmask;
> > -
> > - ret = i2c_smbus_write_byte_data(data->client, info->en_reg, ret);
> > + ret = regmap_update_bits(data->regmap, info->en_reg, info->en_bitmask,
> > + (state ? info->en_bitmask : 0));
> > if (ret < 0) {
> > - dev_err(&data->client->dev, "Error writing reg_int_en\n");
> > + dev_err(&data->client->dev, "Error updating reg_int_en\n");
> > goto out_fix_power_state;
> > }
> >
> > @@ -581,8 +562,7 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
> >
> > for (i = 0; i < ARRAY_SIZE(data->chip_info->scale_table); ++i) {
> > if (data->chip_info->scale_table[i].scale == val) {
> > - ret = i2c_smbus_write_byte_data(
> > - data->client,
> > + ret = regmap_write(data->regmap,
> > BMC150_ACCEL_REG_PMU_RANGE,
> > data->chip_info->scale_table[i].reg_range);
> > if (ret < 0) {
> > @@ -602,16 +582,17 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
> > static int bmc150_accel_get_temp(struct bmc150_accel_data *data, int *val)
> > {
> > int ret;
> > + unsigned int value;
> >
> > mutex_lock(&data->mutex);
> >
> > - ret = i2c_smbus_read_byte_data(data->client, BMC150_ACCEL_REG_TEMP);
> > + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_TEMP, &value);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error reading reg_temp\n");
> > mutex_unlock(&data->mutex);
> > return ret;
> > }
> > - *val = sign_extend32(ret, 7);
> > + *val = sign_extend32(value, 7);
> >
> > mutex_unlock(&data->mutex);
> >
> > @@ -624,6 +605,7 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
> > {
> > int ret;
> > int axis = chan->scan_index;
> > + unsigned int raw_val;
> >
> > mutex_lock(&data->mutex);
> > ret = bmc150_accel_set_power_state(data, true);
> > @@ -632,15 +614,15 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
> > return ret;
> > }
> >
> > - ret = i2c_smbus_read_word_data(data->client,
> > - BMC150_ACCEL_AXIS_TO_REG(axis));
> > + ret = regmap_bulk_read(data->regmap, BMC150_ACCEL_AXIS_TO_REG(axis),
> > + &raw_val, 2);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error reading axis %d\n", axis);
> > bmc150_accel_set_power_state(data, false);
> > mutex_unlock(&data->mutex);
> > return ret;
> > }
> > - *val = sign_extend32(ret >> chan->scan_type.shift,
> > + *val = sign_extend32(raw_val >> chan->scan_type.shift,
> > chan->scan_type.realbits - 1);
> > ret = bmc150_accel_set_power_state(data, false);
> > mutex_unlock(&data->mutex);
> > @@ -904,52 +886,37 @@ static int bmc150_accel_set_watermark(struct iio_dev *indio_dev, unsigned val)
> > * We must read at least one full frame in one burst, otherwise the rest of the
> > * frame data is discarded.
> > */
> > -static int bmc150_accel_fifo_transfer(const struct i2c_client *client,
> > +static int bmc150_accel_fifo_transfer(struct bmc150_accel_data *data,
> > char *buffer, int samples)
> > {
> > int sample_length = 3 * 2;
> > - u8 reg_fifo_data = BMC150_ACCEL_REG_FIFO_DATA;
> > - int ret = -EIO;
> > -
> > - if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> > - struct i2c_msg msg[2] = {
> > - {
> > - .addr = client->addr,
> > - .flags = 0,
> > - .buf = ®_fifo_data,
> > - .len = sizeof(reg_fifo_data),
> > - },
> > - {
> > - .addr = client->addr,
> > - .flags = I2C_M_RD,
> > - .buf = (u8 *)buffer,
> > - .len = samples * sample_length,
> > - }
> > - };
> > + int ret;
> > + int total_length = samples * sample_length;
> > + int i, step;
> >
> > - ret = i2c_transfer(client->adapter, msg, 2);
> > - if (ret != 2)
> > - ret = -EIO;
> > - else
> > - ret = 0;
> > - } else {
> > - int i, step = I2C_SMBUS_BLOCK_MAX / sample_length;
> > -
> > - for (i = 0; i < samples * sample_length; i += step) {
> > - ret = i2c_smbus_read_i2c_block_data(client,
> > - reg_fifo_data, step,
> > - &buffer[i]);
> > - if (ret != step) {
> > - ret = -EIO;
> > - break;
> > - }
> > + ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA, buffer,
> > + total_length);
> > + if (ret != -E2BIG) {
> > + if (ret)
> I'd invert the logic for more readability.
>
> if (ret == -E2BIT) {
> ...
> } else if (ret) {
> ...
> } else {
> return ret;
> }
Okay.
> > + dev_err(data->dev, "Error transferring data from fifo\n");
> > + return ret;
> > + }
> >
> > - ret = 0;
> > - }
> > + /*
> > + * Seems we have a bus with size limitation so we have to execute
> > + * multiple reads
> > + */
> Can we not just query this in advance before going through the previous
> failed call? THat would be cleaner to my mind.
Yes we can check that in advance and make the reads accordingly.
>
> > + step = regmap_get_raw_io_max(data->regmap) / sample_length;
> > + for (i = -1; i < samples * sample_length; i += step) {
> > + ret = regmap_raw_read(data->regmap, BMC150_ACCEL_REG_FIFO_DATA,
> > + &buffer[i], step);
> umm. Can't say I like the negative index into buffer. Why is it
> necessary?
No this is an error, thanks.
> > + if (ret)
> > + break;
> > }
> >
> > if (ret)
> > - dev_err(&client->dev, "Error transferring data from fifo\n");
> > + dev_err(data->dev, "Error transferring data from fifo in single steps of %zu\n",
>
> multiple steps of %zu perhaps?
Yes.
> > + step);
> >
> > return ret;
> > }
> > @@ -963,14 +930,15 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,
> > u16 buffer[BMC150_ACCEL_FIFO_LENGTH * 3];
> > int64_t tstamp;
> > uint64_t sample_period;
> > - ret = i2c_smbus_read_byte_data(data->client,
> > - BMC150_ACCEL_REG_FIFO_STATUS);
> > + unsigned int val;
> > +
> > + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_FIFO_STATUS, &val);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error reading reg_fifo_status\n");
> > return ret;
> > }
> >
> > - count = ret & 0x7F;
> > + count = val & 0x7F;
> >
> > if (!count)
> > return 0;
> > @@ -1009,7 +977,7 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,
> > if (samples && count > samples)
> > count = samples;
> >
> > - ret = bmc150_accel_fifo_transfer(data->client, (u8 *)buffer, count);
> > + ret = bmc150_accel_fifo_transfer(data, (u8 *)buffer, count);
> > if (ret)
> > return ret;
> >
> > @@ -1206,17 +1174,19 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
> > struct iio_dev *indio_dev = pf->indio_dev;
> > struct bmc150_accel_data *data = iio_priv(indio_dev);
> > int bit, ret, i = 0;
> > + unsigned int raw_val;
> >
> > mutex_lock(&data->mutex);
> > for_each_set_bit(bit, indio_dev->active_scan_mask,
> > indio_dev->masklength) {
> > - ret = i2c_smbus_read_word_data(data->client,
> > - BMC150_ACCEL_AXIS_TO_REG(bit));
> > + ret = regmap_bulk_read(data->regmap,
> > + BMC150_ACCEL_AXIS_TO_REG(bit), &raw_val,
> > + 2);
> Is using a variable on the stack not going to cause issues when we add
> SPI? (cacheline dma requirements).
Good question. Just looked it up. regmap uses spi_write_then_read which
uses its own buffer and copies the result back to the receive buffer at
the end. So this shouldn't be a problem.
Thanks,
Markus
> > if (ret < 0) {
> > mutex_unlock(&data->mutex);
> > goto err_read;
> > }
> > - data->buffer[i++] = ret;
> > + data->buffer[i++] = raw_val;
> > }
> > mutex_unlock(&data->mutex);
> >
> > @@ -1240,10 +1210,9 @@ static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
> >
> > mutex_lock(&data->mutex);
> > /* clear any latched interrupt */
> > - ret = i2c_smbus_write_byte_data(data->client,
> > - BMC150_ACCEL_REG_INT_RST_LATCH,
> > - BMC150_ACCEL_INT_MODE_LATCH_INT |
> > - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> > + BMC150_ACCEL_INT_MODE_LATCH_INT |
> > + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > mutex_unlock(&data->mutex);
> > if (ret < 0) {
> > dev_err(&data->client->dev,
> > @@ -1300,34 +1269,34 @@ static int bmc150_accel_handle_roc_event(struct iio_dev *indio_dev)
> > struct bmc150_accel_data *data = iio_priv(indio_dev);
> > int dir;
> > int ret;
> > + unsigned int val;
> >
> > - ret = i2c_smbus_read_byte_data(data->client,
> > - BMC150_ACCEL_REG_INT_STATUS_2);
> > + ret = regmap_read(data->regmap, BMC150_ACCEL_REG_INT_STATUS_2, &val);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error reading reg_int_status_2\n");
> > return ret;
> > }
> >
> > - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_SIGN)
> > + if (val & BMC150_ACCEL_ANY_MOTION_BIT_SIGN)
> > dir = IIO_EV_DIR_FALLING;
> > else
> > dir = IIO_EV_DIR_RISING;
> >
> > - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_X)
> > + if (val & BMC150_ACCEL_ANY_MOTION_BIT_X)
> > iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
> > 0,
> > IIO_MOD_X,
> > IIO_EV_TYPE_ROC,
> > dir),
> > data->timestamp);
> > - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_Y)
> > + if (val & BMC150_ACCEL_ANY_MOTION_BIT_Y)
> > iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
> > 0,
> > IIO_MOD_Y,
> > IIO_EV_TYPE_ROC,
> > dir),
> > data->timestamp);
> > - if (ret & BMC150_ACCEL_ANY_MOTION_BIT_Z)
> > + if (val & BMC150_ACCEL_ANY_MOTION_BIT_Z)
> > iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL,
> > 0,
> > IIO_MOD_Z,
> > @@ -1360,10 +1329,9 @@ static irqreturn_t bmc150_accel_irq_thread_handler(int irq, void *private)
> > }
> >
> > if (ack) {
> > - ret = i2c_smbus_write_byte_data(data->client,
> > - BMC150_ACCEL_REG_INT_RST_LATCH,
> > - BMC150_ACCEL_INT_MODE_LATCH_INT |
> > - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> > + BMC150_ACCEL_INT_MODE_LATCH_INT |
> > + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > if (ret)
> > dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
> > ret = IRQ_HANDLED;
> > @@ -1516,7 +1484,7 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
> > u8 reg = BMC150_ACCEL_REG_FIFO_CONFIG1;
> > int ret;
> >
> > - ret = i2c_smbus_write_byte_data(data->client, reg, data->fifo_mode);
> > + ret = regmap_write(data->regmap, reg, data->fifo_mode);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error writing reg_fifo_config1\n");
> > return ret;
> > @@ -1525,9 +1493,8 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
> > if (!data->fifo_mode)
> > return 0;
> >
> > - ret = i2c_smbus_write_byte_data(data->client,
> > - BMC150_ACCEL_REG_FIFO_CONFIG0,
> > - data->watermark);
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_FIFO_CONFIG0,
> > + data->watermark);
> > if (ret < 0)
> > dev_err(&data->client->dev, "Error writing reg_fifo_config0\n");
> >
> > @@ -1627,6 +1594,13 @@ static int bmc150_accel_probe(struct i2c_client *client,
> > data = iio_priv(indio_dev);
> > i2c_set_clientdata(client, indio_dev);
> > data->client = client;
> > + data->dev = &client->dev;
> > +
> > + data->regmap = devm_regmap_init_i2c(client, &bmc150_i2c_regmap_conf);
> > + if (IS_ERR(data->regmap)) {
> > + dev_err(&client->dev, "Failed to initialize i2c regmap\n");
> > + return PTR_ERR(data->regmap);
> > + }
> >
> > if (id) {
> > name = id->name;
> > @@ -1680,9 +1654,8 @@ static int bmc150_accel_probe(struct i2c_client *client,
> > * want to use latch mode when we can to prevent interrupt
> > * flooding.
> > */
> > - ret = i2c_smbus_write_byte_data(data->client,
> > - BMC150_ACCEL_REG_INT_RST_LATCH,
> > - BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > + ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
> > + BMC150_ACCEL_INT_MODE_LATCH_RESET);
> > if (ret < 0) {
> > dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
> > goto err_buffer_cleanup;
> >
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 12:20 +0200 |
| Subject | [PATCH 15/20] regmap-i2c: Add smbus i2c block support |
| Message-ID | <pWBiA-1oV-65@gated-at.bofh.it> |
| In reply to | #1205901 |
This allows to read/write up to 32 bytes of data and is to be prefered
if supported before the register read/write smbus support.
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
drivers/base/regmap/regmap-i2c.c | 46 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 4b76e33110a2..2de6eb4e6ec3 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -209,11 +209,57 @@ static struct regmap_bus regmap_i2c = {
.val_format_endian_default = REGMAP_ENDIAN_BIG,
};
+static int regmap_i2c_smbus_i2c_write(void *context, const void *data,
+ size_t count)
+{
+ struct device *dev = context;
+ struct i2c_client *i2c = to_i2c_client(dev);
+
+ if (count < 1 || count >= I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ --count;
+ return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count,
+ ((u8 *)data + 1));
+}
+
+static int regmap_i2c_smbus_i2c_read(void *context, const void *reg,
+ size_t reg_size, void *val,
+ size_t val_size)
+{
+ struct device *dev = context;
+ struct i2c_client *i2c = to_i2c_client(dev);
+ int ret;
+
+ if (reg_size != 1)
+ return -EINVAL;
+ if (val_size < 1 || val_size >= I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ ret = i2c_smbus_read_i2c_block_data(i2c, ((u8 *)reg)[0], val_size, val);
+ if (ret == val_size)
+ return 0;
+ else if (ret < 0)
+ return ret;
+ else
+ return -EIO;
+}
+
+static struct regmap_bus regmap_i2c_smbus_i2c_block = {
+ .write = regmap_i2c_smbus_i2c_write,
+ .read = regmap_i2c_smbus_i2c_read,
+ .max_raw_io = I2C_SMBUS_BLOCK_MAX,
+};
+
static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
const struct regmap_config *config)
{
if (i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C))
return ®map_i2c;
+ else if (config->reg_bits == 8 &&
+ i2c_check_functionality(i2c->adapter,
+ I2C_FUNC_SMBUS_I2C_BLOCK))
+ return ®map_i2c_smbus_i2c_block;
else if (config->val_bits == 16 && config->reg_bits == 8 &&
i2c_check_functionality(i2c->adapter,
I2C_FUNC_SMBUS_WORD_DATA))
--
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-08-12 14:00 +0200 |
| Subject | Re: [PATCH 15/20] regmap-i2c: Add smbus i2c block support |
| Message-ID | <pWCRl-3tG-47@gated-at.bofh.it> |
| In reply to | #1205903 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 12:12:40PM +0200, Markus Pargmann wrote: > + if (count < 1 || count >= I2C_SMBUS_BLOCK_MAX) > + return -EINVAL; Elsewhere you added returns of -E2BIG if the transfer was too big, why not do that here as well?
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 15:00 +0200 |
| Subject | Re: [PATCH 15/20] regmap-i2c: Add smbus i2c block support |
| Message-ID | <pWDNo-4Q9-9@gated-at.bofh.it> |
| In reply to | #1205996 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 12:59:37PM +0100, Mark Brown wrote: > On Wed, Aug 12, 2015 at 12:12:40PM +0200, Markus Pargmann wrote: > > > + if (count < 1 || count >= I2C_SMBUS_BLOCK_MAX) > > + return -EINVAL; > > Elsewhere you added returns of -E2BIG if the transfer was too big, why > not do that here as well? Sounds good to use E2BIG here as well. Best Regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 12:30 +0200 |
| Subject | [PATCH 01/20] regmap: Add missing comments about struct regmap_bus |
| Message-ID | <pWBsd-1Ag-3@gated-at.bofh.it> |
| In reply to | #1205901 |
There are some fields of this struct undocumented or old. This patch
updates the missing comments.
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
include/linux/regmap.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 59c55ea0f0b5..6ff83c9ddb45 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -296,8 +296,11 @@ typedef void (*regmap_hw_free_context)(void *context);
* if not implemented on a given device.
* @async_write: Write operation which completes asynchronously, optional and
* must serialise with respect to non-async I/O.
+ * @reg_write: Write operation for a register. Writes value to register.
* @read: Read operation. Data is returned in the buffer used to transmit
* data.
+ * @reg_read: Read operation for a register. Reads a value from a register.
+ * @free_conetext: Free context.
* @async_alloc: Allocate a regmap_async() structure.
* @read_flag_mask: Mask to be set in the top byte of the register when doing
* a read.
@@ -307,7 +310,6 @@ typedef void (*regmap_hw_free_context)(void *context);
* @val_format_endian_default: Default endianness for formatted register
* values. Used when the regmap_config specifies DEFAULT. If this is
* DEFAULT, BIG is assumed.
- * @async_size: Size of struct used for async work.
*/
struct regmap_bus {
bool fast_io;
--
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Hartmut Knaack <knaack.h@gmx.de> |
|---|---|
| Date | 2015-08-12 22:20 +0200 |
| Subject | Re: [PATCH 01/20] regmap: Add missing comments about struct regmap_bus |
| Message-ID | <pWKFc-6tA-15@gated-at.bofh.it> |
| In reply to | #1205904 |
Markus Pargmann schrieb am 12.08.2015 um 12:12:
> There are some fields of this struct undocumented or old. This patch
> updates the missing comments.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
> include/linux/regmap.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index 59c55ea0f0b5..6ff83c9ddb45 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -296,8 +296,11 @@ typedef void (*regmap_hw_free_context)(void *context);
> * if not implemented on a given device.
> * @async_write: Write operation which completes asynchronously, optional and
> * must serialise with respect to non-async I/O.
> + * @reg_write: Write operation for a register. Writes value to register.
> * @read: Read operation. Data is returned in the buffer used to transmit
> * data.
> + * @reg_read: Read operation for a register. Reads a value from a register.
> + * @free_conetext: Free context.
Typo: free_context
> * @async_alloc: Allocate a regmap_async() structure.
> * @read_flag_mask: Mask to be set in the top byte of the register when doing
> * a read.
> @@ -307,7 +310,6 @@ typedef void (*regmap_hw_free_context)(void *context);
> * @val_format_endian_default: Default endianness for formatted register
> * values. Used when the regmap_config specifies DEFAULT. If this is
> * DEFAULT, BIG is assumed.
> - * @async_size: Size of struct used for async work.
> */
> struct regmap_bus {
> bool fast_io;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-17 09:20 +0200 |
| Subject | Re: [PATCH 01/20] regmap: Add missing comments about struct regmap_bus |
| Message-ID | <pYmS6-i1-15@gated-at.bofh.it> |
| In reply to | #1206360 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 10:12:22PM +0200, Hartmut Knaack wrote: > Markus Pargmann schrieb am 12.08.2015 um 12:12: > > There are some fields of this struct undocumented or old. This patch > > updates the missing comments. > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > > --- > > include/linux/regmap.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/regmap.h b/include/linux/regmap.h > > index 59c55ea0f0b5..6ff83c9ddb45 100644 > > --- a/include/linux/regmap.h > > +++ b/include/linux/regmap.h > > @@ -296,8 +296,11 @@ typedef void (*regmap_hw_free_context)(void *context); > > * if not implemented on a given device. > > * @async_write: Write operation which completes asynchronously, optional and > > * must serialise with respect to non-async I/O. > > + * @reg_write: Write operation for a register. Writes value to register. > > * @read: Read operation. Data is returned in the buffer used to transmit > > * data. > > + * @reg_read: Read operation for a register. Reads a value from a register. > > + * @free_conetext: Free context. > > Typo: free_context Thanks, fixed for next version. Best regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 12:30 +0200 |
| Subject | [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() |
| Message-ID | <pWBse-1Ag-25@gated-at.bofh.it> |
| In reply to | #1205901 |
Some busses don't have a write() function defined. However we can use
reg_write() in special cases.
This patch adds support for reg_write() and throws errors if it was
unsuccessful.
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
drivers/base/regmap/regmap.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 510dab052a95..78eb96288a68 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1229,6 +1229,11 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
}
}
+ if (!map->bus->write && val_len == map->format.val_bytes) {
+ ret = _regmap_bus_reg_write(map, reg, *(unsigned int *)val);
+ return ret;
+ }
+
range = _regmap_range_lookup(map, reg);
if (range) {
int val_num = val_len / map->format.val_bytes;
@@ -1340,7 +1345,7 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
* send the work_buf directly, otherwise try to do a gather
* write.
*/
- if (val == work_val) {
+ if (val == work_val && map->bus->write) {
ret = map->bus->write(map->bus_context, map->work_buf,
map->format.reg_bytes +
map->format.pad_bytes +
@@ -1354,7 +1359,7 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
val, val_len);
/* If that didn't work fall back on linearising by hand. */
- if (ret == -ENOTSUPP) {
+ if (ret == -ENOTSUPP && map->bus->write) {
len = map->format.reg_bytes + map->format.pad_bytes +
val_len;
buf = kzalloc(len, GFP_KERNEL);
--
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-08-12 13:30 +0200 |
| Subject | Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() |
| Message-ID | <pWCoi-2VG-17@gated-at.bofh.it> |
| In reply to | #1205910 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 12:12:34PM +0200, Markus Pargmann wrote:
> @@ -1229,6 +1229,11 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
> }
> }
>
> + if (!map->bus->write && val_len == map->format.val_bytes) {
> + ret = _regmap_bus_reg_write(map, reg, *(unsigned int *)val);
> + return ret;
> + }
This is broken - you can't use a raw value as a register value. The
endianness of the device may not be the same as the endianness of the
system and you can't cast a value to unsigned int, the value may be of
any size.
> @@ -1340,7 +1345,7 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
> * send the work_buf directly, otherwise try to do a gather
> * write.
> */
> - if (val == work_val) {
> + if (val == work_val && map->bus->write) {
> ret = map->bus->write(map->bus_context, map->work_buf,
> map->format.reg_bytes +
> map->format.pad_bytes +
This appears to be another case of merging an unrelated change :(
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 14:30 +0200 |
| Subject | Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() |
| Message-ID | <pWDko-4hj-49@gated-at.bofh.it> |
| In reply to | #1205973 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 12:20:35PM +0100, Mark Brown wrote:
> On Wed, Aug 12, 2015 at 12:12:34PM +0200, Markus Pargmann wrote:
>
> > @@ -1229,6 +1229,11 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
> > }
> > }
> >
> > + if (!map->bus->write && val_len == map->format.val_bytes) {
> > + ret = _regmap_bus_reg_write(map, reg, *(unsigned int *)val);
> > + return ret;
> > + }
>
> This is broken - you can't use a raw value as a register value. The
I am not sure what you mean here?
The register value given to _regmap_raw_write is the real register
value, not formatted differenty. This is given directly towards
bus->reg_write() which should handle the rest.
At least that's how I understood the code. For example regmap_read()
directly calls _regmap_read() which in turn calls directly
bus->reg_read() without any formating.
> endianness of the device may not be the same as the endianness of the
> system and you can't cast a value to unsigned int, the value may be of
> any size.
Yes right. On the other hand if bus->read() and bus->write() was not set
in the init method (before this patch series) no formatting functions at
all were assigned. So it was always ignored for bus->reg_read() and
bus->reg_write()?!
>
> > @@ -1340,7 +1345,7 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
> > * send the work_buf directly, otherwise try to do a gather
> > * write.
> > */
> > - if (val == work_val) {
> > + if (val == work_val && map->bus->write) {
> > ret = map->bus->write(map->bus_context, map->work_buf,
> > map->format.reg_bytes +
> > map->format.pad_bytes +
>
> This appears to be another case of merging an unrelated change :(
Yes, will fix.
Thanks,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-08-12 14:40 +0200 |
| Subject | Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() |
| Message-ID | <pWDu1-4ta-3@gated-at.bofh.it> |
| In reply to | #1206013 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 02:20:11PM +0200, Markus Pargmann wrote:
> On Wed, Aug 12, 2015 at 12:20:35PM +0100, Mark Brown wrote:
> > On Wed, Aug 12, 2015 at 12:12:34PM +0200, Markus Pargmann wrote:
> >
> > > @@ -1229,6 +1229,11 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
> > > }
> > > }
> > >
> > > + if (!map->bus->write && val_len == map->format.val_bytes) {
> > > + ret = _regmap_bus_reg_write(map, reg, *(unsigned int *)val);
> > > + return ret;
> > > + }
> > This is broken - you can't use a raw value as a register value. The
> I am not sure what you mean here?
> The register value given to _regmap_raw_write is the real register
> value, not formatted differenty. This is given directly towards
> bus->reg_write() which should handle the rest.
I mean the value for the register, not the register address.
> At least that's how I understood the code. For example regmap_read()
> directly calls _regmap_read() which in turn calls directly
> bus->reg_read() without any formating.
You're adding this code to regmap_raw_write() which takes raw register
values for the device, not unsigned integers.
> > endianness of the device may not be the same as the endianness of the
> > system and you can't cast a value to unsigned int, the value may be of
> > any size.
> Yes right. On the other hand if bus->read() and bus->write() was not set
> in the init method (before this patch series) no formatting functions at
> all were assigned. So it was always ignored for bus->reg_read() and
> bus->reg_write()?!
I'm not sure what the "it" you're talking about here is, sorry. There
are unsupported features in the API especially for cases that don't make
a huge amount of sense, the error handling isn't always complete. It
sounds like you might be trying to support one of these nonsensical
cases - it's not obvious what raw I/O on a device where we don't know
the raw format of the device should mean or how anything could sensibly
use that.
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 15:10 +0200 |
| Subject | Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() |
| Message-ID | <pWDX3-5h7-1@gated-at.bofh.it> |
| In reply to | #1206015 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 01:34:06PM +0100, Mark Brown wrote:
> On Wed, Aug 12, 2015 at 02:20:11PM +0200, Markus Pargmann wrote:
> > On Wed, Aug 12, 2015 at 12:20:35PM +0100, Mark Brown wrote:
> > > On Wed, Aug 12, 2015 at 12:12:34PM +0200, Markus Pargmann wrote:
> > >
> > > > @@ -1229,6 +1229,11 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
> > > > }
> > > > }
> > > >
> > > > + if (!map->bus->write && val_len == map->format.val_bytes) {
> > > > + ret = _regmap_bus_reg_write(map, reg, *(unsigned int *)val);
> > > > + return ret;
> > > > + }
>
> > > This is broken - you can't use a raw value as a register value. The
>
> > I am not sure what you mean here?
>
> > The register value given to _regmap_raw_write is the real register
> > value, not formatted differenty. This is given directly towards
> > bus->reg_write() which should handle the rest.
>
> I mean the value for the register, not the register address.
>
> > At least that's how I understood the code. For example regmap_read()
> > directly calls _regmap_read() which in turn calls directly
> > bus->reg_read() without any formating.
>
> You're adding this code to regmap_raw_write() which takes raw register
> values for the device, not unsigned integers.
Ah yes, I see.
>
> > > endianness of the device may not be the same as the endianness of the
> > > system and you can't cast a value to unsigned int, the value may be of
> > > any size.
>
> > Yes right. On the other hand if bus->read() and bus->write() was not set
> > in the init method (before this patch series) no formatting functions at
> > all were assigned. So it was always ignored for bus->reg_read() and
> > bus->reg_write()?!
>
> I'm not sure what the "it" you're talking about here is, sorry. There
> are unsupported features in the API especially for cases that don't make
> a huge amount of sense, the error handling isn't always complete. It
> sounds like you might be trying to support one of these nonsensical
> cases - it's not obvious what raw I/O on a device where we don't know
> the raw format of the device should mean or how anything could sensibly
> use that.
The bus and the regmap user are separate. So as a regmap user, I am not
able to know if the bus the device is connected to actually supports raw
reads/writes. At least it should not fail with a null pointer when using
these functions anyway so yes error handling is missing a bit here.
Also the real use of this function is regmap_bulk_write() which always
uses _regmap_raw_write() regardless of a missing bus->write() function.
So regmap_bulk_write will fail for those as well and this should be
supported.
But it is probably better to handle this in regmap_bulk_write() and
add a simple check for bus->write() here.
Best Regards,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-08-15 04:20 +0200 |
| Subject | Re: [PATCH 09/20] regmap: _regmap_raw_write fix for busses without write() |
| Message-ID | <pXzeG-3OQ-39@gated-at.bofh.it> |
| In reply to | #1206065 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 03:05:18PM +0200, Markus Pargmann wrote: > On Wed, Aug 12, 2015 at 01:34:06PM +0100, Mark Brown wrote: > > > Yes right. On the other hand if bus->read() and bus->write() was not set > > > in the init method (before this patch series) no formatting functions at > > > all were assigned. So it was always ignored for bus->reg_read() and > > > bus->reg_write()?! > > I'm not sure what the "it" you're talking about here is, sorry. There > > are unsupported features in the API especially for cases that don't make > > a huge amount of sense, the error handling isn't always complete. It > > sounds like you might be trying to support one of these nonsensical > > cases - it's not obvious what raw I/O on a device where we don't know > > the raw format of the device should mean or how anything could sensibly > > use that. > The bus and the regmap user are separate. So as a regmap user, I am not > able to know if the bus the device is connected to actually supports raw > reads/writes. At least it should not fail with a null pointer when using You should generally have a pretty good idea simply by knowing which device you're working with - unless you're writing generic code you know which device you're working with and what it's capabilities are. A driver that doesn't know these things should never be trying to do raw I/O, and a driver that is doing raw I/O clearly depends on having the ability to get a bytestream to and from the device since that's what raw I/O does.
[toc] | [prev] | [next] | [standalone]
| From | Markus Pargmann <mpa@pengutronix.de> |
|---|---|
| Date | 2015-08-12 12:30 +0200 |
| Subject | [PATCH 10/20] regmap: _regmap_raw_multi_reg_write: Add reg_write() support |
| Message-ID | <pWBse-1Ag-27@gated-at.bofh.it> |
| In reply to | #1205901 |
Define a fallback for busses which do not define a write() function.
Instead we write one register at a time using reg_write().
Without this patch, _regmap_raw_multi_reg_write would break as it tries
to call bus->write() without checking if it exists before.
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
drivers/base/regmap/regmap.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 78eb96288a68..87f15fb60bc5 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1778,6 +1778,20 @@ static int _regmap_raw_multi_reg_write(struct regmap *map,
if (!len)
return -EINVAL;
+ /*
+ * If bus->write is not supported we have to use reg_write for each
+ * register value.
+ */
+ if (!map->bus->write) {
+ for (i = 0; i < num_regs; i++) {
+ ret = map->reg_write(map, regs[i].reg, regs[i].def);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+ }
+
buf = kzalloc(len, GFP_KERNEL);
if (!buf)
return -ENOMEM;
--
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-08-12 14:50 +0200 |
| Subject | Re: [PATCH 10/20] regmap: _regmap_raw_multi_reg_write: Add reg_write() support |
| Message-ID | <pWDDJ-4EJ-25@gated-at.bofh.it> |
| In reply to | #1205911 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Aug 12, 2015 at 12:12:35PM +0200, Markus Pargmann wrote: > Define a fallback for busses which do not define a write() function. > Instead we write one register at a time using reg_write(). > > Without this patch, _regmap_raw_multi_reg_write would break as it tries > to call bus->write() without checking if it exists before. Why are we trying to use multi write APIs in the first place if we can't do raw I/O?
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web