* [PATCH 0/2] add regmap API support to hts221 iio driver
@ 2017-12-23 17:16 Lorenzo Bianconi
2017-12-23 17:16 ` [PATCH 1/2] iio: humidity: hts221: move common code in hts221_core Lorenzo Bianconi
2017-12-23 17:16 ` [PATCH 2/2] iio: humidity: hts221: add regmap API support Lorenzo Bianconi
0 siblings, 2 replies; 7+ messages in thread
From: Lorenzo Bianconi @ 2017-12-23 17:16 UTC (permalink / raw)
To: jic23; +Cc: linux-iio
Lorenzo Bianconi (2):
iio: humidity: hts221: move common code in hts221_core
iio: humidity: hts221: add regmap API support
drivers/iio/humidity/Kconfig | 2 +
drivers/iio/humidity/hts221.h | 28 ++++-------
drivers/iio/humidity/hts221_buffer.c | 20 +++-----
drivers/iio/humidity/hts221_core.c | 96 ++++++++++++++----------------------
drivers/iio/humidity/hts221_i2c.c | 76 +++++++---------------------
drivers/iio/humidity/hts221_spi.c | 95 +++++++----------------------------
6 files changed, 90 insertions(+), 227 deletions(-)
--
2.13.6
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] iio: humidity: hts221: move common code in hts221_core
2017-12-23 17:16 [PATCH 0/2] add regmap API support to hts221 iio driver Lorenzo Bianconi
@ 2017-12-23 17:16 ` Lorenzo Bianconi
2017-12-29 13:24 ` Jonathan Cameron
2017-12-23 17:16 ` [PATCH 2/2] iio: humidity: hts221: add regmap API support Lorenzo Bianconi
1 sibling, 1 reply; 7+ messages in thread
From: Lorenzo Bianconi @ 2017-12-23 17:16 UTC (permalink / raw)
To: jic23; +Cc: linux-iio
Move duplicated i2c/spi probe code in hts221_probe()
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
drivers/iio/humidity/hts221.h | 3 ++-
drivers/iio/humidity/hts221_core.c | 18 ++++++++++++++++--
drivers/iio/humidity/hts221_i2c.c | 18 ++----------------
drivers/iio/humidity/hts221_spi.c | 18 ++----------------
4 files changed, 22 insertions(+), 35 deletions(-)
diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
index 51d021966222..c581af8c0f5d 100644
--- a/drivers/iio/humidity/hts221.h
+++ b/drivers/iio/humidity/hts221.h
@@ -61,7 +61,8 @@ struct hts221_hw {
extern const struct dev_pm_ops hts221_pm_ops;
int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
-int hts221_probe(struct iio_dev *iio_dev);
+int hts221_probe(struct device *dev, int irq, const char *name,
+ const struct hts221_transfer_function *tf_ops);
int hts221_set_enable(struct hts221_hw *hw, bool enable);
int hts221_allocate_buffers(struct hts221_hw *hw);
int hts221_allocate_trigger(struct hts221_hw *hw);
diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index daef177219b6..d3f7904766bd 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -581,12 +581,26 @@ static const struct iio_info hts221_info = {
static const unsigned long hts221_scan_masks[] = {0x3, 0x0};
-int hts221_probe(struct iio_dev *iio_dev)
+int hts221_probe(struct device *dev, int irq, const char *name,
+ const struct hts221_transfer_function *tf_ops)
{
- struct hts221_hw *hw = iio_priv(iio_dev);
+ struct iio_dev *iio_dev;
+ struct hts221_hw *hw;
int err;
u8 data;
+ iio_dev = devm_iio_device_alloc(dev, sizeof(*hw));
+ if (!iio_dev)
+ return -ENOMEM;
+
+ dev_set_drvdata(dev, (void *)iio_dev);
+
+ hw = iio_priv(iio_dev);
+ hw->name = name;
+ hw->dev = dev;
+ hw->irq = irq;
+ hw->tf = tf_ops;
+
mutex_init(&hw->lock);
err = hts221_check_whoami(hw);
diff --git a/drivers/iio/humidity/hts221_i2c.c b/drivers/iio/humidity/hts221_i2c.c
index f38e4b7e0160..2c97350a0f76 100644
--- a/drivers/iio/humidity/hts221_i2c.c
+++ b/drivers/iio/humidity/hts221_i2c.c
@@ -66,22 +66,8 @@ static const struct hts221_transfer_function hts221_transfer_fn = {
static int hts221_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- struct hts221_hw *hw;
- struct iio_dev *iio_dev;
-
- iio_dev = devm_iio_device_alloc(&client->dev, sizeof(*hw));
- if (!iio_dev)
- return -ENOMEM;
-
- i2c_set_clientdata(client, iio_dev);
-
- hw = iio_priv(iio_dev);
- hw->name = client->name;
- hw->dev = &client->dev;
- hw->irq = client->irq;
- hw->tf = &hts221_transfer_fn;
-
- return hts221_probe(iio_dev);
+ return hts221_probe(&client->dev, client->irq,
+ client->name, &hts221_transfer_fn);
}
static const struct acpi_device_id hts221_acpi_match[] = {
diff --git a/drivers/iio/humidity/hts221_spi.c b/drivers/iio/humidity/hts221_spi.c
index 57cbc256771b..55b29b53b9d1 100644
--- a/drivers/iio/humidity/hts221_spi.c
+++ b/drivers/iio/humidity/hts221_spi.c
@@ -80,22 +80,8 @@ static const struct hts221_transfer_function hts221_transfer_fn = {
static int hts221_spi_probe(struct spi_device *spi)
{
- struct hts221_hw *hw;
- struct iio_dev *iio_dev;
-
- iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*hw));
- if (!iio_dev)
- return -ENOMEM;
-
- spi_set_drvdata(spi, iio_dev);
-
- hw = iio_priv(iio_dev);
- hw->name = spi->modalias;
- hw->dev = &spi->dev;
- hw->irq = spi->irq;
- hw->tf = &hts221_transfer_fn;
-
- return hts221_probe(iio_dev);
+ return hts221_probe(&spi->dev, spi->irq,
+ spi->modalias, &hts221_transfer_fn);
}
static const struct of_device_id hts221_spi_of_match[] = {
--
2.13.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] iio: humidity: hts221: add regmap API support
2017-12-23 17:16 [PATCH 0/2] add regmap API support to hts221 iio driver Lorenzo Bianconi
2017-12-23 17:16 ` [PATCH 1/2] iio: humidity: hts221: move common code in hts221_core Lorenzo Bianconi
@ 2017-12-23 17:16 ` Lorenzo Bianconi
2017-12-29 13:24 ` Jonathan Cameron
1 sibling, 1 reply; 7+ messages in thread
From: Lorenzo Bianconi @ 2017-12-23 17:16 UTC (permalink / raw)
To: jic23; +Cc: linux-iio
Introduce regmap API support to access to i2c/spi bus instead of
using a custom support.
Remove lock mutex since concurrency is already managed by regmap API
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
drivers/iio/humidity/Kconfig | 2 +
drivers/iio/humidity/hts221.h | 27 ++++--------
drivers/iio/humidity/hts221_buffer.c | 20 ++++-----
drivers/iio/humidity/hts221_core.c | 84 ++++++++++--------------------------
drivers/iio/humidity/hts221_i2c.c | 64 ++++++++-------------------
drivers/iio/humidity/hts221_spi.c | 79 ++++++++-------------------------
6 files changed, 76 insertions(+), 200 deletions(-)
diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
index 2c0fc9a400b8..1a0d458e4f4e 100644
--- a/drivers/iio/humidity/Kconfig
+++ b/drivers/iio/humidity/Kconfig
@@ -68,10 +68,12 @@ config HTS221
config HTS221_I2C
tristate
depends on HTS221
+ select REGMAP_I2C
config HTS221_SPI
tristate
depends on HTS221
+ select REGMAP_SPI
config HTU21
tristate "Measurement Specialties HTU21 humidity & temperature sensor"
diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
index c581af8c0f5d..00596879010e 100644
--- a/drivers/iio/humidity/hts221.h
+++ b/drivers/iio/humidity/hts221.h
@@ -15,21 +15,8 @@
#include <linux/iio/iio.h>
-#define HTS221_RX_MAX_LENGTH 8
-#define HTS221_TX_MAX_LENGTH 8
-
#define HTS221_DATA_SIZE 2
-struct hts221_transfer_buffer {
- u8 rx_buf[HTS221_RX_MAX_LENGTH];
- u8 tx_buf[HTS221_TX_MAX_LENGTH] ____cacheline_aligned;
-};
-
-struct hts221_transfer_function {
- int (*read)(struct device *dev, u8 addr, int len, u8 *data);
- int (*write)(struct device *dev, u8 addr, int len, u8 *data);
-};
-
enum hts221_sensor_type {
HTS221_SENSOR_H,
HTS221_SENSOR_T,
@@ -44,8 +31,8 @@ struct hts221_sensor {
struct hts221_hw {
const char *name;
struct device *dev;
+ struct regmap *regmap;
- struct mutex lock;
struct iio_trigger *trig;
int irq;
@@ -53,16 +40,18 @@ struct hts221_hw {
bool enabled;
u8 odr;
-
- const struct hts221_transfer_function *tf;
- struct hts221_transfer_buffer tb;
};
extern const struct dev_pm_ops hts221_pm_ops;
-int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
+static inline int hts221_write_with_mask(struct hts221_hw *hw, u8 addr,
+ u8 mask, u8 val)
+{
+ return regmap_update_bits(hw->regmap, addr, mask, val << __ffs(mask));
+}
+
int hts221_probe(struct device *dev, int irq, const char *name,
- const struct hts221_transfer_function *tf_ops);
+ struct regmap *regmap);
int hts221_set_enable(struct hts221_hw *hw, bool enable);
int hts221_allocate_buffers(struct hts221_hw *hw);
int hts221_allocate_trigger(struct hts221_hw *hw);
diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index e971ea425268..b1141acfd100 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -12,6 +12,7 @@
#include <linux/device.h>
#include <linux/interrupt.h>
#include <linux/irqreturn.h>
+#include <linux/regmap.h>
#include <linux/iio/iio.h>
#include <linux/iio/trigger.h>
@@ -38,12 +39,9 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
{
struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
struct hts221_hw *hw = iio_priv(iio_dev);
- int err;
- err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
+ return hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
HTS221_REG_DRDY_EN_MASK, state);
-
- return err < 0 ? err : 0;
}
static const struct iio_trigger_ops hts221_trigger_ops = {
@@ -53,11 +51,9 @@ static const struct iio_trigger_ops hts221_trigger_ops = {
static irqreturn_t hts221_trigger_handler_thread(int irq, void *private)
{
struct hts221_hw *hw = private;
- u8 status;
- int err;
+ int err, status;
- err = hw->tf->read(hw->dev, HTS221_REG_STATUS_ADDR, sizeof(status),
- &status);
+ err = regmap_read(hw->regmap, HTS221_REG_STATUS_ADDR, &status);
if (err < 0)
return IRQ_HANDLED;
@@ -171,15 +167,15 @@ static irqreturn_t hts221_buffer_handler_thread(int irq, void *p)
/* humidity data */
ch = &iio_dev->channels[HTS221_SENSOR_H];
- err = hw->tf->read(hw->dev, ch->address, HTS221_DATA_SIZE,
- buffer);
+ err = regmap_bulk_read(hw->regmap, ch->address,
+ buffer, HTS221_DATA_SIZE);
if (err < 0)
goto out;
/* temperature data */
ch = &iio_dev->channels[HTS221_SENSOR_T];
- err = hw->tf->read(hw->dev, ch->address, HTS221_DATA_SIZE,
- buffer + HTS221_DATA_SIZE);
+ err = regmap_bulk_read(hw->regmap, ch->address,
+ buffer + HTS221_DATA_SIZE, HTS221_DATA_SIZE);
if (err < 0)
goto out;
diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index d3f7904766bd..b9665846b26f 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -15,6 +15,7 @@
#include <linux/delay.h>
#include <linux/pm.h>
#include <asm/unaligned.h>
+#include <linux/regmap.h>
#include "hts221.h"
@@ -131,38 +132,11 @@ static const struct iio_chan_spec hts221_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(2),
};
-int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val)
-{
- u8 data;
- int err;
-
- mutex_lock(&hw->lock);
-
- err = hw->tf->read(hw->dev, addr, sizeof(data), &data);
- if (err < 0) {
- dev_err(hw->dev, "failed to read %02x register\n", addr);
- goto unlock;
- }
-
- data = (data & ~mask) | ((val << __ffs(mask)) & mask);
-
- err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
- if (err < 0)
- dev_err(hw->dev, "failed to write %02x register\n", addr);
-
-unlock:
- mutex_unlock(&hw->lock);
-
- return err;
-}
-
static int hts221_check_whoami(struct hts221_hw *hw)
{
- u8 data;
- int err;
+ int err, data;
- err = hw->tf->read(hw->dev, HTS221_REG_WHOAMI_ADDR, sizeof(data),
- &data);
+ err = regmap_read(hw->regmap, HTS221_REG_WHOAMI_ADDR, &data);
if (err < 0) {
dev_err(hw->dev, "failed to read whoami register\n");
return err;
@@ -286,35 +260,31 @@ int hts221_set_enable(struct hts221_hw *hw, bool enable)
static int hts221_parse_temp_caldata(struct hts221_hw *hw)
{
- int err, *slope, *b_gen;
+ int err, *slope, *b_gen, cal0, cal1;
s16 cal_x0, cal_x1, cal_y0, cal_y1;
- u8 cal0, cal1;
- err = hw->tf->read(hw->dev, HTS221_REG_0T_CAL_Y_H,
- sizeof(cal0), &cal0);
+ err = regmap_read(hw->regmap, HTS221_REG_0T_CAL_Y_H, &cal0);
if (err < 0)
return err;
- err = hw->tf->read(hw->dev, HTS221_REG_T1_T0_CAL_Y_H,
- sizeof(cal1), &cal1);
+ err = regmap_read(hw->regmap, HTS221_REG_T1_T0_CAL_Y_H, &cal1);
if (err < 0)
return err;
cal_y0 = (le16_to_cpu(cal1 & 0x3) << 8) | cal0;
- err = hw->tf->read(hw->dev, HTS221_REG_1T_CAL_Y_H,
- sizeof(cal0), &cal0);
+ err = regmap_read(hw->regmap, HTS221_REG_1T_CAL_Y_H, &cal0);
if (err < 0)
return err;
cal_y1 = (((cal1 & 0xc) >> 2) << 8) | cal0;
- err = hw->tf->read(hw->dev, HTS221_REG_0T_CAL_X_L, sizeof(cal_x0),
- (u8 *)&cal_x0);
+ err = regmap_bulk_read(hw->regmap, HTS221_REG_0T_CAL_X_L,
+ &cal_x0, sizeof(cal_x0));
if (err < 0)
return err;
cal_x0 = le16_to_cpu(cal_x0);
- err = hw->tf->read(hw->dev, HTS221_REG_1T_CAL_X_L, sizeof(cal_x1),
- (u8 *)&cal_x1);
+ err = regmap_bulk_read(hw->regmap, HTS221_REG_1T_CAL_X_L,
+ &cal_x1, sizeof(cal_x1));
if (err < 0)
return err;
cal_x1 = le16_to_cpu(cal_x1);
@@ -332,30 +302,27 @@ static int hts221_parse_temp_caldata(struct hts221_hw *hw)
static int hts221_parse_rh_caldata(struct hts221_hw *hw)
{
- int err, *slope, *b_gen;
+ int err, *slope, *b_gen, data;
s16 cal_x0, cal_x1, cal_y0, cal_y1;
- u8 data;
- err = hw->tf->read(hw->dev, HTS221_REG_0RH_CAL_Y_H, sizeof(data),
- &data);
+ err = regmap_read(hw->regmap, HTS221_REG_0RH_CAL_Y_H, &data);
if (err < 0)
return err;
cal_y0 = data;
- err = hw->tf->read(hw->dev, HTS221_REG_1RH_CAL_Y_H, sizeof(data),
- &data);
+ err = regmap_read(hw->regmap, HTS221_REG_1RH_CAL_Y_H, &data);
if (err < 0)
return err;
cal_y1 = data;
- err = hw->tf->read(hw->dev, HTS221_REG_0RH_CAL_X_H, sizeof(cal_x0),
- (u8 *)&cal_x0);
+ err = regmap_bulk_read(hw->regmap, HTS221_REG_0RH_CAL_X_H,
+ &cal_x0, sizeof(cal_x0));
if (err < 0)
return err;
cal_x0 = le16_to_cpu(cal_x0);
- err = hw->tf->read(hw->dev, HTS221_REG_1RH_CAL_X_H, sizeof(cal_x1),
- (u8 *)&cal_x1);
+ err = regmap_bulk_read(hw->regmap, HTS221_REG_1RH_CAL_X_H,
+ &cal_x1, sizeof(cal_x1));
if (err < 0)
return err;
cal_x1 = le16_to_cpu(cal_x1);
@@ -440,7 +407,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
msleep(50);
- err = hw->tf->read(hw->dev, addr, sizeof(data), data);
+ err = regmap_bulk_read(hw->regmap, addr, data, sizeof(data));
if (err < 0)
return err;
@@ -582,7 +549,7 @@ static const struct iio_info hts221_info = {
static const unsigned long hts221_scan_masks[] = {0x3, 0x0};
int hts221_probe(struct device *dev, int irq, const char *name,
- const struct hts221_transfer_function *tf_ops)
+ struct regmap *regmap)
{
struct iio_dev *iio_dev;
struct hts221_hw *hw;
@@ -599,9 +566,7 @@ int hts221_probe(struct device *dev, int irq, const char *name,
hw->name = name;
hw->dev = dev;
hw->irq = irq;
- hw->tf = tf_ops;
-
- mutex_init(&hw->lock);
+ hw->regmap = regmap;
err = hts221_check_whoami(hw);
if (err < 0)
@@ -673,12 +638,9 @@ static int __maybe_unused hts221_suspend(struct device *dev)
{
struct iio_dev *iio_dev = dev_get_drvdata(dev);
struct hts221_hw *hw = iio_priv(iio_dev);
- int err;
-
- err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
- HTS221_ENABLE_MASK, false);
- return err < 0 ? err : 0;
+ return hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
+ HTS221_ENABLE_MASK, false);
}
static int __maybe_unused hts221_resume(struct device *dev)
diff --git a/drivers/iio/humidity/hts221_i2c.c b/drivers/iio/humidity/hts221_i2c.c
index 2c97350a0f76..b5b3f408a658 100644
--- a/drivers/iio/humidity/hts221_i2c.c
+++ b/drivers/iio/humidity/hts221_i2c.c
@@ -13,61 +13,33 @@
#include <linux/acpi.h>
#include <linux/i2c.h>
#include <linux/slab.h>
-#include "hts221.h"
-
-#define I2C_AUTO_INCREMENT 0x80
-
-static int hts221_i2c_read(struct device *dev, u8 addr, int len, u8 *data)
-{
- struct i2c_msg msg[2];
- struct i2c_client *client = to_i2c_client(dev);
-
- if (len > 1)
- addr |= I2C_AUTO_INCREMENT;
-
- msg[0].addr = client->addr;
- msg[0].flags = client->flags;
- msg[0].len = 1;
- msg[0].buf = &addr;
-
- msg[1].addr = client->addr;
- msg[1].flags = client->flags | I2C_M_RD;
- msg[1].len = len;
- msg[1].buf = data;
-
- return i2c_transfer(client->adapter, msg, 2);
-}
+#include <linux/regmap.h>
-static int hts221_i2c_write(struct device *dev, u8 addr, int len, u8 *data)
-{
- u8 send[len + 1];
- struct i2c_msg msg;
- struct i2c_client *client = to_i2c_client(dev);
-
- if (len > 1)
- addr |= I2C_AUTO_INCREMENT;
-
- send[0] = addr;
- memcpy(&send[1], data, len * sizeof(u8));
-
- msg.addr = client->addr;
- msg.flags = client->flags;
- msg.len = len + 1;
- msg.buf = send;
+#include "hts221.h"
- return i2c_transfer(client->adapter, &msg, 1);
-}
+#define HTS221_I2C_AUTO_INCREMENT BIT(7)
-static const struct hts221_transfer_function hts221_transfer_fn = {
- .read = hts221_i2c_read,
- .write = hts221_i2c_write,
+static const struct regmap_config hts221_i2c_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .write_flag_mask = HTS221_I2C_AUTO_INCREMENT,
+ .read_flag_mask = HTS221_I2C_AUTO_INCREMENT,
};
static int hts221_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_i2c(client, &hts221_i2c_regmap_config);
+ if (IS_ERR(regmap)) {
+ dev_err(&client->dev, "Failed to register i2c regmap %d\n",
+ (int)PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
return hts221_probe(&client->dev, client->irq,
- client->name, &hts221_transfer_fn);
+ client->name, regmap);
}
static const struct acpi_device_id hts221_acpi_match[] = {
diff --git a/drivers/iio/humidity/hts221_spi.c b/drivers/iio/humidity/hts221_spi.c
index 55b29b53b9d1..ea3aaec54035 100644
--- a/drivers/iio/humidity/hts221_spi.c
+++ b/drivers/iio/humidity/hts221_spi.c
@@ -12,76 +12,31 @@
#include <linux/module.h>
#include <linux/spi/spi.h>
#include <linux/slab.h>
-#include "hts221.h"
-
-#define SENSORS_SPI_READ 0x80
-#define SPI_AUTO_INCREMENT 0x40
-
-static int hts221_spi_read(struct device *dev, u8 addr, int len, u8 *data)
-{
- int err;
- struct spi_device *spi = to_spi_device(dev);
- struct iio_dev *iio_dev = spi_get_drvdata(spi);
- struct hts221_hw *hw = iio_priv(iio_dev);
-
- struct spi_transfer xfers[] = {
- {
- .tx_buf = hw->tb.tx_buf,
- .bits_per_word = 8,
- .len = 1,
- },
- {
- .rx_buf = hw->tb.rx_buf,
- .bits_per_word = 8,
- .len = len,
- }
- };
-
- if (len > 1)
- addr |= SPI_AUTO_INCREMENT;
- hw->tb.tx_buf[0] = addr | SENSORS_SPI_READ;
-
- err = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
- if (err < 0)
- return err;
-
- memcpy(data, hw->tb.rx_buf, len * sizeof(u8));
+#include <linux/regmap.h>
- return len;
-}
-
-static int hts221_spi_write(struct device *dev, u8 addr, int len, u8 *data)
-{
- struct spi_device *spi = to_spi_device(dev);
- struct iio_dev *iio_dev = spi_get_drvdata(spi);
- struct hts221_hw *hw = iio_priv(iio_dev);
-
- struct spi_transfer xfers = {
- .tx_buf = hw->tb.tx_buf,
- .bits_per_word = 8,
- .len = len + 1,
- };
-
- if (len >= HTS221_TX_MAX_LENGTH)
- return -ENOMEM;
-
- if (len > 1)
- addr |= SPI_AUTO_INCREMENT;
- hw->tb.tx_buf[0] = addr;
- memcpy(&hw->tb.tx_buf[1], data, len);
+#include "hts221.h"
- return spi_sync_transfer(spi, &xfers, 1);
-}
+#define HTS221_SPI_AUTO_INCREMENT BIT(6)
-static const struct hts221_transfer_function hts221_transfer_fn = {
- .read = hts221_spi_read,
- .write = hts221_spi_write,
+static const struct regmap_config hts221_spi_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .write_flag_mask = HTS221_SPI_AUTO_INCREMENT,
+ .read_flag_mask = HTS221_SPI_AUTO_INCREMENT,
};
static int hts221_spi_probe(struct spi_device *spi)
{
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_spi(spi, &hts221_spi_regmap_config);
+ if (IS_ERR(regmap)) {
+ dev_err(&spi->dev, "Failed to register spi regmap %d\n",
+ (int)PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
return hts221_probe(&spi->dev, spi->irq,
- spi->modalias, &hts221_transfer_fn);
+ spi->modalias, regmap);
}
static const struct of_device_id hts221_spi_of_match[] = {
--
2.13.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] iio: humidity: hts221: add regmap API support
2017-12-23 17:16 ` [PATCH 2/2] iio: humidity: hts221: add regmap API support Lorenzo Bianconi
@ 2017-12-29 13:24 ` Jonathan Cameron
2017-12-29 13:33 ` Lorenzo Bianconi
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2017-12-29 13:24 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio
On Sat, 23 Dec 2017 18:16:21 +0100
Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
> Introduce regmap API support to access to i2c/spi bus instead of
> using a custom support.
> Remove lock mutex since concurrency is already managed by regmap API
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Looks good. A few minor comments and a question inline.
Thanks,
Jonathan
> ---
> drivers/iio/humidity/Kconfig | 2 +
> drivers/iio/humidity/hts221.h | 27 ++++--------
> drivers/iio/humidity/hts221_buffer.c | 20 ++++-----
> drivers/iio/humidity/hts221_core.c | 84 ++++++++++--------------------------
> drivers/iio/humidity/hts221_i2c.c | 64 ++++++++-------------------
> drivers/iio/humidity/hts221_spi.c | 79 ++++++++-------------------------
> 6 files changed, 76 insertions(+), 200 deletions(-)
>
> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
> index 2c0fc9a400b8..1a0d458e4f4e 100644
> --- a/drivers/iio/humidity/Kconfig
> +++ b/drivers/iio/humidity/Kconfig
> @@ -68,10 +68,12 @@ config HTS221
> config HTS221_I2C
> tristate
> depends on HTS221
> + select REGMAP_I2C
>
> config HTS221_SPI
> tristate
> depends on HTS221
> + select REGMAP_SPI
>
> config HTU21
> tristate "Measurement Specialties HTU21 humidity & temperature sensor"
> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
> index c581af8c0f5d..00596879010e 100644
> --- a/drivers/iio/humidity/hts221.h
> +++ b/drivers/iio/humidity/hts221.h
> @@ -15,21 +15,8 @@
>
> #include <linux/iio/iio.h>
>
> -#define HTS221_RX_MAX_LENGTH 8
> -#define HTS221_TX_MAX_LENGTH 8
> -
> #define HTS221_DATA_SIZE 2
>
> -struct hts221_transfer_buffer {
> - u8 rx_buf[HTS221_RX_MAX_LENGTH];
> - u8 tx_buf[HTS221_TX_MAX_LENGTH] ____cacheline_aligned;
> -};
> -
> -struct hts221_transfer_function {
> - int (*read)(struct device *dev, u8 addr, int len, u8 *data);
> - int (*write)(struct device *dev, u8 addr, int len, u8 *data);
> -};
> -
> enum hts221_sensor_type {
> HTS221_SENSOR_H,
> HTS221_SENSOR_T,
> @@ -44,8 +31,8 @@ struct hts221_sensor {
> struct hts221_hw {
> const char *name;
> struct device *dev;
> + struct regmap *regmap;
>
> - struct mutex lock;
> struct iio_trigger *trig;
> int irq;
>
> @@ -53,16 +40,18 @@ struct hts221_hw {
>
> bool enabled;
> u8 odr;
> -
> - const struct hts221_transfer_function *tf;
> - struct hts221_transfer_buffer tb;
> };
>
> extern const struct dev_pm_ops hts221_pm_ops;
>
> -int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
> +static inline int hts221_write_with_mask(struct hts221_hw *hw, u8 addr,
> + u8 mask, u8 val)
> +{
> + return regmap_update_bits(hw->regmap, addr, mask, val << __ffs(mask));
> +}
> +
> int hts221_probe(struct device *dev, int irq, const char *name,
> - const struct hts221_transfer_function *tf_ops);
> + struct regmap *regmap);
> int hts221_set_enable(struct hts221_hw *hw, bool enable);
> int hts221_allocate_buffers(struct hts221_hw *hw);
> int hts221_allocate_trigger(struct hts221_hw *hw);
> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
> index e971ea425268..b1141acfd100 100644
> --- a/drivers/iio/humidity/hts221_buffer.c
> +++ b/drivers/iio/humidity/hts221_buffer.c
> @@ -12,6 +12,7 @@
> #include <linux/device.h>
> #include <linux/interrupt.h>
> #include <linux/irqreturn.h>
> +#include <linux/regmap.h>
>
> #include <linux/iio/iio.h>
> #include <linux/iio/trigger.h>
> @@ -38,12 +39,9 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
> {
> struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
> struct hts221_hw *hw = iio_priv(iio_dev);
> - int err;
>
> - err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
> + return hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
> HTS221_REG_DRDY_EN_MASK, state);
You could use the regmap call directly inconjunction with FIELD_PREP
return regmap_update_bits(hw->regmap, HTS221_REG_DRDY_EN_ADDR,
HTS221_REG_DRDY_EN_MASK,
FIELD_PREP(HTS221_REG_DRDY_EN_MASK, val));
> -
> - return err < 0 ? err : 0;
> }
>
> static const struct iio_trigger_ops hts221_trigger_ops = {
> @@ -53,11 +51,9 @@ static const struct iio_trigger_ops hts221_trigger_ops = {
> static irqreturn_t hts221_trigger_handler_thread(int irq, void *private)
> {
> struct hts221_hw *hw = private;
> - u8 status;
> - int err;
> + int err, status;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_STATUS_ADDR, sizeof(status),
> - &status);
> + err = regmap_read(hw->regmap, HTS221_REG_STATUS_ADDR, &status);
> if (err < 0)
> return IRQ_HANDLED;
>
> @@ -171,15 +167,15 @@ static irqreturn_t hts221_buffer_handler_thread(int irq, void *p)
>
> /* humidity data */
> ch = &iio_dev->channels[HTS221_SENSOR_H];
> - err = hw->tf->read(hw->dev, ch->address, HTS221_DATA_SIZE,
> - buffer);
> + err = regmap_bulk_read(hw->regmap, ch->address,
> + buffer, HTS221_DATA_SIZE);
> if (err < 0)
> goto out;
>
> /* temperature data */
> ch = &iio_dev->channels[HTS221_SENSOR_T];
> - err = hw->tf->read(hw->dev, ch->address, HTS221_DATA_SIZE,
> - buffer + HTS221_DATA_SIZE);
> + err = regmap_bulk_read(hw->regmap, ch->address,
> + buffer + HTS221_DATA_SIZE, HTS221_DATA_SIZE);
> if (err < 0)
> goto out;
>
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index d3f7904766bd..b9665846b26f 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -15,6 +15,7 @@
> #include <linux/delay.h>
> #include <linux/pm.h>
> #include <asm/unaligned.h>
> +#include <linux/regmap.h>
>
> #include "hts221.h"
>
> @@ -131,38 +132,11 @@ static const struct iio_chan_spec hts221_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(2),
> };
>
> -int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val)
> -{
> - u8 data;
> - int err;
> -
> - mutex_lock(&hw->lock);
> -
> - err = hw->tf->read(hw->dev, addr, sizeof(data), &data);
> - if (err < 0) {
> - dev_err(hw->dev, "failed to read %02x register\n", addr);
> - goto unlock;
> - }
> -
> - data = (data & ~mask) | ((val << __ffs(mask)) & mask);
> -
> - err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
> - if (err < 0)
> - dev_err(hw->dev, "failed to write %02x register\n", addr);
> -
> -unlock:
> - mutex_unlock(&hw->lock);
> -
> - return err;
> -}
> -
> static int hts221_check_whoami(struct hts221_hw *hw)
> {
> - u8 data;
> - int err;
> + int err, data;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_WHOAMI_ADDR, sizeof(data),
> - &data);
> + err = regmap_read(hw->regmap, HTS221_REG_WHOAMI_ADDR, &data);
> if (err < 0) {
> dev_err(hw->dev, "failed to read whoami register\n");
> return err;
> @@ -286,35 +260,31 @@ int hts221_set_enable(struct hts221_hw *hw, bool enable)
>
> static int hts221_parse_temp_caldata(struct hts221_hw *hw)
> {
> - int err, *slope, *b_gen;
> + int err, *slope, *b_gen, cal0, cal1;
> s16 cal_x0, cal_x1, cal_y0, cal_y1;
> - u8 cal0, cal1;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_0T_CAL_Y_H,
> - sizeof(cal0), &cal0);
> + err = regmap_read(hw->regmap, HTS221_REG_0T_CAL_Y_H, &cal0);
> if (err < 0)
> return err;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_T1_T0_CAL_Y_H,
> - sizeof(cal1), &cal1);
> + err = regmap_read(hw->regmap, HTS221_REG_T1_T0_CAL_Y_H, &cal1);
> if (err < 0)
> return err;
> cal_y0 = (le16_to_cpu(cal1 & 0x3) << 8) | cal0;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_1T_CAL_Y_H,
> - sizeof(cal0), &cal0);
> + err = regmap_read(hw->regmap, HTS221_REG_1T_CAL_Y_H, &cal0);
> if (err < 0)
> return err;
> cal_y1 = (((cal1 & 0xc) >> 2) << 8) | cal0;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_0T_CAL_X_L, sizeof(cal_x0),
> - (u8 *)&cal_x0);
> + err = regmap_bulk_read(hw->regmap, HTS221_REG_0T_CAL_X_L,
> + &cal_x0, sizeof(cal_x0));
> if (err < 0)
> return err;
> cal_x0 = le16_to_cpu(cal_x0);
>
> - err = hw->tf->read(hw->dev, HTS221_REG_1T_CAL_X_L, sizeof(cal_x1),
> - (u8 *)&cal_x1);
> + err = regmap_bulk_read(hw->regmap, HTS221_REG_1T_CAL_X_L,
> + &cal_x1, sizeof(cal_x1));
> if (err < 0)
> return err;
> cal_x1 = le16_to_cpu(cal_x1);
> @@ -332,30 +302,27 @@ static int hts221_parse_temp_caldata(struct hts221_hw *hw)
>
> static int hts221_parse_rh_caldata(struct hts221_hw *hw)
> {
> - int err, *slope, *b_gen;
> + int err, *slope, *b_gen, data;
> s16 cal_x0, cal_x1, cal_y0, cal_y1;
> - u8 data;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_0RH_CAL_Y_H, sizeof(data),
> - &data);
> + err = regmap_read(hw->regmap, HTS221_REG_0RH_CAL_Y_H, &data);
> if (err < 0)
> return err;
> cal_y0 = data;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_1RH_CAL_Y_H, sizeof(data),
> - &data);
> + err = regmap_read(hw->regmap, HTS221_REG_1RH_CAL_Y_H, &data);
> if (err < 0)
> return err;
> cal_y1 = data;
>
> - err = hw->tf->read(hw->dev, HTS221_REG_0RH_CAL_X_H, sizeof(cal_x0),
> - (u8 *)&cal_x0);
> + err = regmap_bulk_read(hw->regmap, HTS221_REG_0RH_CAL_X_H,
> + &cal_x0, sizeof(cal_x0));
> if (err < 0)
> return err;
> cal_x0 = le16_to_cpu(cal_x0);
>
> - err = hw->tf->read(hw->dev, HTS221_REG_1RH_CAL_X_H, sizeof(cal_x1),
> - (u8 *)&cal_x1);
> + err = regmap_bulk_read(hw->regmap, HTS221_REG_1RH_CAL_X_H,
> + &cal_x1, sizeof(cal_x1));
> if (err < 0)
> return err;
> cal_x1 = le16_to_cpu(cal_x1);
> @@ -440,7 +407,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
>
> msleep(50);
>
> - err = hw->tf->read(hw->dev, addr, sizeof(data), data);
> + err = regmap_bulk_read(hw->regmap, addr, data, sizeof(data));
> if (err < 0)
> return err;
>
> @@ -582,7 +549,7 @@ static const struct iio_info hts221_info = {
> static const unsigned long hts221_scan_masks[] = {0x3, 0x0};
>
> int hts221_probe(struct device *dev, int irq, const char *name,
> - const struct hts221_transfer_function *tf_ops)
> + struct regmap *regmap)
> {
> struct iio_dev *iio_dev;
> struct hts221_hw *hw;
> @@ -599,9 +566,7 @@ int hts221_probe(struct device *dev, int irq, const char *name,
> hw->name = name;
> hw->dev = dev;
> hw->irq = irq;
> - hw->tf = tf_ops;
> -
> - mutex_init(&hw->lock);
> + hw->regmap = regmap;
>
> err = hts221_check_whoami(hw);
> if (err < 0)
> @@ -673,12 +638,9 @@ static int __maybe_unused hts221_suspend(struct device *dev)
> {
> struct iio_dev *iio_dev = dev_get_drvdata(dev);
> struct hts221_hw *hw = iio_priv(iio_dev);
> - int err;
> -
> - err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> - HTS221_ENABLE_MASK, false);
>
> - return err < 0 ? err : 0;
> + return hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> + HTS221_ENABLE_MASK, false);
> }
>
> static int __maybe_unused hts221_resume(struct device *dev)
> diff --git a/drivers/iio/humidity/hts221_i2c.c b/drivers/iio/humidity/hts221_i2c.c
> index 2c97350a0f76..b5b3f408a658 100644
> --- a/drivers/iio/humidity/hts221_i2c.c
> +++ b/drivers/iio/humidity/hts221_i2c.c
> @@ -13,61 +13,33 @@
> #include <linux/acpi.h>
> #include <linux/i2c.h>
> #include <linux/slab.h>
> -#include "hts221.h"
> -
> -#define I2C_AUTO_INCREMENT 0x80
> -
> -static int hts221_i2c_read(struct device *dev, u8 addr, int len, u8 *data)
> -{
> - struct i2c_msg msg[2];
> - struct i2c_client *client = to_i2c_client(dev);
> -
> - if (len > 1)
> - addr |= I2C_AUTO_INCREMENT;
> -
> - msg[0].addr = client->addr;
> - msg[0].flags = client->flags;
> - msg[0].len = 1;
> - msg[0].buf = &addr;
> -
> - msg[1].addr = client->addr;
> - msg[1].flags = client->flags | I2C_M_RD;
> - msg[1].len = len;
> - msg[1].buf = data;
> -
> - return i2c_transfer(client->adapter, msg, 2);
> -}
> +#include <linux/regmap.h>
>
> -static int hts221_i2c_write(struct device *dev, u8 addr, int len, u8 *data)
> -{
> - u8 send[len + 1];
> - struct i2c_msg msg;
> - struct i2c_client *client = to_i2c_client(dev);
> -
> - if (len > 1)
> - addr |= I2C_AUTO_INCREMENT;
> -
> - send[0] = addr;
> - memcpy(&send[1], data, len * sizeof(u8));
> -
> - msg.addr = client->addr;
> - msg.flags = client->flags;
> - msg.len = len + 1;
> - msg.buf = send;
> +#include "hts221.h"
>
> - return i2c_transfer(client->adapter, &msg, 1);
> -}
> +#define HTS221_I2C_AUTO_INCREMENT BIT(7)
>
> -static const struct hts221_transfer_function hts221_transfer_fn = {
> - .read = hts221_i2c_read,
> - .write = hts221_i2c_write,
> +static const struct regmap_config hts221_i2c_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .write_flag_mask = HTS221_I2C_AUTO_INCREMENT,
> + .read_flag_mask = HTS221_I2C_AUTO_INCREMENT,
> };
>
> static int hts221_i2c_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init_i2c(client, &hts221_i2c_regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(&client->dev, "Failed to register i2c regmap %d\n",
> + (int)PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> return hts221_probe(&client->dev, client->irq,
> - client->name, &hts221_transfer_fn);
> + client->name, regmap);
> }
>
> static const struct acpi_device_id hts221_acpi_match[] = {
> diff --git a/drivers/iio/humidity/hts221_spi.c b/drivers/iio/humidity/hts221_spi.c
> index 55b29b53b9d1..ea3aaec54035 100644
> --- a/drivers/iio/humidity/hts221_spi.c
> +++ b/drivers/iio/humidity/hts221_spi.c
> @@ -12,76 +12,31 @@
> #include <linux/module.h>
> #include <linux/spi/spi.h>
> #include <linux/slab.h>
> -#include "hts221.h"
> -
> -#define SENSORS_SPI_READ 0x80
I'm somewhat surprised to see this one go - not needed in the read mask
for regmap?
> -#define SPI_AUTO_INCREMENT 0x40
> -
> -static int hts221_spi_read(struct device *dev, u8 addr, int len, u8 *data)
> -{
> - int err;
> - struct spi_device *spi = to_spi_device(dev);
> - struct iio_dev *iio_dev = spi_get_drvdata(spi);
> - struct hts221_hw *hw = iio_priv(iio_dev);
> -
> - struct spi_transfer xfers[] = {
> - {
> - .tx_buf = hw->tb.tx_buf,
> - .bits_per_word = 8,
> - .len = 1,
> - },
> - {
> - .rx_buf = hw->tb.rx_buf,
> - .bits_per_word = 8,
> - .len = len,
> - }
> - };
> -
> - if (len > 1)
> - addr |= SPI_AUTO_INCREMENT;
> - hw->tb.tx_buf[0] = addr | SENSORS_SPI_READ;
> -
> - err = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> - if (err < 0)
> - return err;
> -
> - memcpy(data, hw->tb.rx_buf, len * sizeof(u8));
> +#include <linux/regmap.h>
>
> - return len;
> -}
> -
> -static int hts221_spi_write(struct device *dev, u8 addr, int len, u8 *data)
> -{
> - struct spi_device *spi = to_spi_device(dev);
> - struct iio_dev *iio_dev = spi_get_drvdata(spi);
> - struct hts221_hw *hw = iio_priv(iio_dev);
> -
> - struct spi_transfer xfers = {
> - .tx_buf = hw->tb.tx_buf,
> - .bits_per_word = 8,
> - .len = len + 1,
> - };
> -
> - if (len >= HTS221_TX_MAX_LENGTH)
> - return -ENOMEM;
> -
> - if (len > 1)
> - addr |= SPI_AUTO_INCREMENT;
> - hw->tb.tx_buf[0] = addr;
> - memcpy(&hw->tb.tx_buf[1], data, len);
> +#include "hts221.h"
>
> - return spi_sync_transfer(spi, &xfers, 1);
> -}
> +#define HTS221_SPI_AUTO_INCREMENT BIT(6)
>
> -static const struct hts221_transfer_function hts221_transfer_fn = {
> - .read = hts221_spi_read,
> - .write = hts221_spi_write,
> +static const struct regmap_config hts221_spi_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .write_flag_mask = HTS221_SPI_AUTO_INCREMENT,
> + .read_flag_mask = HTS221_SPI_AUTO_INCREMENT,
> };
>
> static int hts221_spi_probe(struct spi_device *spi)
> {
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init_spi(spi, &hts221_spi_regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(&spi->dev, "Failed to register spi regmap %d\n",
> + (int)PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> return hts221_probe(&spi->dev, spi->irq,
> - spi->modalias, &hts221_transfer_fn);
> + spi->modalias, regmap);
> }
>
> static const struct of_device_id hts221_spi_of_match[] = {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iio: humidity: hts221: move common code in hts221_core
2017-12-23 17:16 ` [PATCH 1/2] iio: humidity: hts221: move common code in hts221_core Lorenzo Bianconi
@ 2017-12-29 13:24 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2017-12-29 13:24 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio
On Sat, 23 Dec 2017 18:16:20 +0100
Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
> Move duplicated i2c/spi probe code in hts221_probe()
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
This is fine. Will pick up when patch 2 is ready.
Thanks,
Jonathan
> ---
> drivers/iio/humidity/hts221.h | 3 ++-
> drivers/iio/humidity/hts221_core.c | 18 ++++++++++++++++--
> drivers/iio/humidity/hts221_i2c.c | 18 ++----------------
> drivers/iio/humidity/hts221_spi.c | 18 ++----------------
> 4 files changed, 22 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
> index 51d021966222..c581af8c0f5d 100644
> --- a/drivers/iio/humidity/hts221.h
> +++ b/drivers/iio/humidity/hts221.h
> @@ -61,7 +61,8 @@ struct hts221_hw {
> extern const struct dev_pm_ops hts221_pm_ops;
>
> int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
> -int hts221_probe(struct iio_dev *iio_dev);
> +int hts221_probe(struct device *dev, int irq, const char *name,
> + const struct hts221_transfer_function *tf_ops);
> int hts221_set_enable(struct hts221_hw *hw, bool enable);
> int hts221_allocate_buffers(struct hts221_hw *hw);
> int hts221_allocate_trigger(struct hts221_hw *hw);
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index daef177219b6..d3f7904766bd 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -581,12 +581,26 @@ static const struct iio_info hts221_info = {
>
> static const unsigned long hts221_scan_masks[] = {0x3, 0x0};
>
> -int hts221_probe(struct iio_dev *iio_dev)
> +int hts221_probe(struct device *dev, int irq, const char *name,
> + const struct hts221_transfer_function *tf_ops)
> {
> - struct hts221_hw *hw = iio_priv(iio_dev);
> + struct iio_dev *iio_dev;
> + struct hts221_hw *hw;
> int err;
> u8 data;
>
> + iio_dev = devm_iio_device_alloc(dev, sizeof(*hw));
> + if (!iio_dev)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, (void *)iio_dev);
> +
> + hw = iio_priv(iio_dev);
> + hw->name = name;
> + hw->dev = dev;
> + hw->irq = irq;
> + hw->tf = tf_ops;
> +
> mutex_init(&hw->lock);
>
> err = hts221_check_whoami(hw);
> diff --git a/drivers/iio/humidity/hts221_i2c.c b/drivers/iio/humidity/hts221_i2c.c
> index f38e4b7e0160..2c97350a0f76 100644
> --- a/drivers/iio/humidity/hts221_i2c.c
> +++ b/drivers/iio/humidity/hts221_i2c.c
> @@ -66,22 +66,8 @@ static const struct hts221_transfer_function hts221_transfer_fn = {
> static int hts221_i2c_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - struct hts221_hw *hw;
> - struct iio_dev *iio_dev;
> -
> - iio_dev = devm_iio_device_alloc(&client->dev, sizeof(*hw));
> - if (!iio_dev)
> - return -ENOMEM;
> -
> - i2c_set_clientdata(client, iio_dev);
> -
> - hw = iio_priv(iio_dev);
> - hw->name = client->name;
> - hw->dev = &client->dev;
> - hw->irq = client->irq;
> - hw->tf = &hts221_transfer_fn;
> -
> - return hts221_probe(iio_dev);
> + return hts221_probe(&client->dev, client->irq,
> + client->name, &hts221_transfer_fn);
> }
>
> static const struct acpi_device_id hts221_acpi_match[] = {
> diff --git a/drivers/iio/humidity/hts221_spi.c b/drivers/iio/humidity/hts221_spi.c
> index 57cbc256771b..55b29b53b9d1 100644
> --- a/drivers/iio/humidity/hts221_spi.c
> +++ b/drivers/iio/humidity/hts221_spi.c
> @@ -80,22 +80,8 @@ static const struct hts221_transfer_function hts221_transfer_fn = {
>
> static int hts221_spi_probe(struct spi_device *spi)
> {
> - struct hts221_hw *hw;
> - struct iio_dev *iio_dev;
> -
> - iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*hw));
> - if (!iio_dev)
> - return -ENOMEM;
> -
> - spi_set_drvdata(spi, iio_dev);
> -
> - hw = iio_priv(iio_dev);
> - hw->name = spi->modalias;
> - hw->dev = &spi->dev;
> - hw->irq = spi->irq;
> - hw->tf = &hts221_transfer_fn;
> -
> - return hts221_probe(iio_dev);
> + return hts221_probe(&spi->dev, spi->irq,
> + spi->modalias, &hts221_transfer_fn);
> }
>
> static const struct of_device_id hts221_spi_of_match[] = {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] iio: humidity: hts221: add regmap API support
2017-12-29 13:24 ` Jonathan Cameron
@ 2017-12-29 13:33 ` Lorenzo Bianconi
2017-12-29 17:25 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Lorenzo Bianconi @ 2017-12-29 13:33 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: Lorenzo Bianconi, linux-iio
> On Sat, 23 Dec 2017 18:16:21 +0100
> Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
>
>> Introduce regmap API support to access to i2c/spi bus instead of
>> using a custom support.
>> Remove lock mutex since concurrency is already managed by regmap API
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>
> Looks good. A few minor comments and a question inline.
Hi Jonathan,
Thanks for the review.
>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/humidity/Kconfig | 2 +
>> drivers/iio/humidity/hts221.h | 27 ++++--------
>> drivers/iio/humidity/hts221_buffer.c | 20 ++++-----
>> drivers/iio/humidity/hts221_core.c | 84 ++++++++++--------------------------
>> drivers/iio/humidity/hts221_i2c.c | 64 ++++++++-------------------
>> drivers/iio/humidity/hts221_spi.c | 79 ++++++++-------------------------
>> 6 files changed, 76 insertions(+), 200 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>> index 2c0fc9a400b8..1a0d458e4f4e 100644
>> --- a/drivers/iio/humidity/Kconfig
>> +++ b/drivers/iio/humidity/Kconfig
>> @@ -68,10 +68,12 @@ config HTS221
>> config HTS221_I2C
>> tristate
>> depends on HTS221
>> + select REGMAP_I2C
>>
>> config HTS221_SPI
>> tristate
>> depends on HTS221
>> + select REGMAP_SPI
>>
>> config HTU21
>> tristate "Measurement Specialties HTU21 humidity & temperature sensor"
>> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
>> index c581af8c0f5d..00596879010e 100644
>> --- a/drivers/iio/humidity/hts221.h
>> +++ b/drivers/iio/humidity/hts221.h
>> @@ -15,21 +15,8 @@
>>
>> #include <linux/iio/iio.h>
>>
>> -#define HTS221_RX_MAX_LENGTH 8
>> -#define HTS221_TX_MAX_LENGTH 8
>> -
>> #define HTS221_DATA_SIZE 2
>>
>> -struct hts221_transfer_buffer {
>> - u8 rx_buf[HTS221_RX_MAX_LENGTH];
>> - u8 tx_buf[HTS221_TX_MAX_LENGTH] ____cacheline_aligned;
>> -};
>> -
>> -struct hts221_transfer_function {
>> - int (*read)(struct device *dev, u8 addr, int len, u8 *data);
>> - int (*write)(struct device *dev, u8 addr, int len, u8 *data);
>> -};
>> -
>> enum hts221_sensor_type {
>> HTS221_SENSOR_H,
>> HTS221_SENSOR_T,
>> @@ -44,8 +31,8 @@ struct hts221_sensor {
>> struct hts221_hw {
>> const char *name;
>> struct device *dev;
>> + struct regmap *regmap;
>>
>> - struct mutex lock;
>> struct iio_trigger *trig;
>> int irq;
>>
>> @@ -53,16 +40,18 @@ struct hts221_hw {
>>
>> bool enabled;
>> u8 odr;
>> -
>> - const struct hts221_transfer_function *tf;
>> - struct hts221_transfer_buffer tb;
>> };
>>
>> extern const struct dev_pm_ops hts221_pm_ops;
>>
>> -int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
>> +static inline int hts221_write_with_mask(struct hts221_hw *hw, u8 addr,
>> + u8 mask, u8 val)
>> +{
>> + return regmap_update_bits(hw->regmap, addr, mask, val << __ffs(mask));
>> +}
>> +
>> int hts221_probe(struct device *dev, int irq, const char *name,
>> - const struct hts221_transfer_function *tf_ops);
>> + struct regmap *regmap);
>> int hts221_set_enable(struct hts221_hw *hw, bool enable);
>> int hts221_allocate_buffers(struct hts221_hw *hw);
>> int hts221_allocate_trigger(struct hts221_hw *hw);
>> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
>> index e971ea425268..b1141acfd100 100644
>> --- a/drivers/iio/humidity/hts221_buffer.c
>> +++ b/drivers/iio/humidity/hts221_buffer.c
>> @@ -12,6 +12,7 @@
>> #include <linux/device.h>
>> #include <linux/interrupt.h>
>> #include <linux/irqreturn.h>
>> +#include <linux/regmap.h>
>>
>> #include <linux/iio/iio.h>
>> #include <linux/iio/trigger.h>
>> @@ -38,12 +39,9 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
>> {
>> struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
>> struct hts221_hw *hw = iio_priv(iio_dev);
>> - int err;
>>
>> - err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
>> + return hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
>> HTS221_REG_DRDY_EN_MASK, state);
>
> You could use the regmap call directly inconjunction with FIELD_PREP
>
> return regmap_update_bits(hw->regmap, HTS221_REG_DRDY_EN_ADDR,
> HTS221_REG_DRDY_EN_MASK,
> FIELD_PREP(HTS221_REG_DRDY_EN_MASK, val));
>
Do you prefer to always drop hts221_write_with_mask() and use just
regmap_update_bits() with FIELD_PREP()?
>> -
>> - return err < 0 ? err : 0;
>> }
>>
>> static const struct iio_trigger_ops hts221_trigger_ops = {
>> @@ -53,11 +51,9 @@ static const struct iio_trigger_ops hts221_trigger_ops = {
>> static irqreturn_t hts221_trigger_handler_thread(int irq, void *private)
>> {
>> struct hts221_hw *hw = private;
>> - u8 status;
>> - int err;
>> + int err, status;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_STATUS_ADDR, sizeof(status),
>> - &status);
>> + err = regmap_read(hw->regmap, HTS221_REG_STATUS_ADDR, &status);
>> if (err < 0)
>> return IRQ_HANDLED;
>>
>> @@ -171,15 +167,15 @@ static irqreturn_t hts221_buffer_handler_thread(int irq, void *p)
>>
>> /* humidity data */
>> ch = &iio_dev->channels[HTS221_SENSOR_H];
>> - err = hw->tf->read(hw->dev, ch->address, HTS221_DATA_SIZE,
>> - buffer);
>> + err = regmap_bulk_read(hw->regmap, ch->address,
>> + buffer, HTS221_DATA_SIZE);
>> if (err < 0)
>> goto out;
>>
>> /* temperature data */
>> ch = &iio_dev->channels[HTS221_SENSOR_T];
>> - err = hw->tf->read(hw->dev, ch->address, HTS221_DATA_SIZE,
>> - buffer + HTS221_DATA_SIZE);
>> + err = regmap_bulk_read(hw->regmap, ch->address,
>> + buffer + HTS221_DATA_SIZE, HTS221_DATA_SIZE);
>> if (err < 0)
>> goto out;
>>
>> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>> index d3f7904766bd..b9665846b26f 100644
>> --- a/drivers/iio/humidity/hts221_core.c
>> +++ b/drivers/iio/humidity/hts221_core.c
>> @@ -15,6 +15,7 @@
>> #include <linux/delay.h>
>> #include <linux/pm.h>
>> #include <asm/unaligned.h>
>> +#include <linux/regmap.h>
>>
>> #include "hts221.h"
>>
>> @@ -131,38 +132,11 @@ static const struct iio_chan_spec hts221_channels[] = {
>> IIO_CHAN_SOFT_TIMESTAMP(2),
>> };
>>
>> -int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val)
>> -{
>> - u8 data;
>> - int err;
>> -
>> - mutex_lock(&hw->lock);
>> -
>> - err = hw->tf->read(hw->dev, addr, sizeof(data), &data);
>> - if (err < 0) {
>> - dev_err(hw->dev, "failed to read %02x register\n", addr);
>> - goto unlock;
>> - }
>> -
>> - data = (data & ~mask) | ((val << __ffs(mask)) & mask);
>> -
>> - err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
>> - if (err < 0)
>> - dev_err(hw->dev, "failed to write %02x register\n", addr);
>> -
>> -unlock:
>> - mutex_unlock(&hw->lock);
>> -
>> - return err;
>> -}
>> -
>> static int hts221_check_whoami(struct hts221_hw *hw)
>> {
>> - u8 data;
>> - int err;
>> + int err, data;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_WHOAMI_ADDR, sizeof(data),
>> - &data);
>> + err = regmap_read(hw->regmap, HTS221_REG_WHOAMI_ADDR, &data);
>> if (err < 0) {
>> dev_err(hw->dev, "failed to read whoami register\n");
>> return err;
>> @@ -286,35 +260,31 @@ int hts221_set_enable(struct hts221_hw *hw, bool enable)
>>
>> static int hts221_parse_temp_caldata(struct hts221_hw *hw)
>> {
>> - int err, *slope, *b_gen;
>> + int err, *slope, *b_gen, cal0, cal1;
>> s16 cal_x0, cal_x1, cal_y0, cal_y1;
>> - u8 cal0, cal1;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_0T_CAL_Y_H,
>> - sizeof(cal0), &cal0);
>> + err = regmap_read(hw->regmap, HTS221_REG_0T_CAL_Y_H, &cal0);
>> if (err < 0)
>> return err;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_T1_T0_CAL_Y_H,
>> - sizeof(cal1), &cal1);
>> + err = regmap_read(hw->regmap, HTS221_REG_T1_T0_CAL_Y_H, &cal1);
>> if (err < 0)
>> return err;
>> cal_y0 = (le16_to_cpu(cal1 & 0x3) << 8) | cal0;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_1T_CAL_Y_H,
>> - sizeof(cal0), &cal0);
>> + err = regmap_read(hw->regmap, HTS221_REG_1T_CAL_Y_H, &cal0);
>> if (err < 0)
>> return err;
>> cal_y1 = (((cal1 & 0xc) >> 2) << 8) | cal0;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_0T_CAL_X_L, sizeof(cal_x0),
>> - (u8 *)&cal_x0);
>> + err = regmap_bulk_read(hw->regmap, HTS221_REG_0T_CAL_X_L,
>> + &cal_x0, sizeof(cal_x0));
>> if (err < 0)
>> return err;
>> cal_x0 = le16_to_cpu(cal_x0);
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_1T_CAL_X_L, sizeof(cal_x1),
>> - (u8 *)&cal_x1);
>> + err = regmap_bulk_read(hw->regmap, HTS221_REG_1T_CAL_X_L,
>> + &cal_x1, sizeof(cal_x1));
>> if (err < 0)
>> return err;
>> cal_x1 = le16_to_cpu(cal_x1);
>> @@ -332,30 +302,27 @@ static int hts221_parse_temp_caldata(struct hts221_hw *hw)
>>
>> static int hts221_parse_rh_caldata(struct hts221_hw *hw)
>> {
>> - int err, *slope, *b_gen;
>> + int err, *slope, *b_gen, data;
>> s16 cal_x0, cal_x1, cal_y0, cal_y1;
>> - u8 data;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_0RH_CAL_Y_H, sizeof(data),
>> - &data);
>> + err = regmap_read(hw->regmap, HTS221_REG_0RH_CAL_Y_H, &data);
>> if (err < 0)
>> return err;
>> cal_y0 = data;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_1RH_CAL_Y_H, sizeof(data),
>> - &data);
>> + err = regmap_read(hw->regmap, HTS221_REG_1RH_CAL_Y_H, &data);
>> if (err < 0)
>> return err;
>> cal_y1 = data;
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_0RH_CAL_X_H, sizeof(cal_x0),
>> - (u8 *)&cal_x0);
>> + err = regmap_bulk_read(hw->regmap, HTS221_REG_0RH_CAL_X_H,
>> + &cal_x0, sizeof(cal_x0));
>> if (err < 0)
>> return err;
>> cal_x0 = le16_to_cpu(cal_x0);
>>
>> - err = hw->tf->read(hw->dev, HTS221_REG_1RH_CAL_X_H, sizeof(cal_x1),
>> - (u8 *)&cal_x1);
>> + err = regmap_bulk_read(hw->regmap, HTS221_REG_1RH_CAL_X_H,
>> + &cal_x1, sizeof(cal_x1));
>> if (err < 0)
>> return err;
>> cal_x1 = le16_to_cpu(cal_x1);
>> @@ -440,7 +407,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
>>
>> msleep(50);
>>
>> - err = hw->tf->read(hw->dev, addr, sizeof(data), data);
>> + err = regmap_bulk_read(hw->regmap, addr, data, sizeof(data));
>> if (err < 0)
>> return err;
>>
>> @@ -582,7 +549,7 @@ static const struct iio_info hts221_info = {
>> static const unsigned long hts221_scan_masks[] = {0x3, 0x0};
>>
>> int hts221_probe(struct device *dev, int irq, const char *name,
>> - const struct hts221_transfer_function *tf_ops)
>> + struct regmap *regmap)
>> {
>> struct iio_dev *iio_dev;
>> struct hts221_hw *hw;
>> @@ -599,9 +566,7 @@ int hts221_probe(struct device *dev, int irq, const char *name,
>> hw->name = name;
>> hw->dev = dev;
>> hw->irq = irq;
>> - hw->tf = tf_ops;
>> -
>> - mutex_init(&hw->lock);
>> + hw->regmap = regmap;
>>
>> err = hts221_check_whoami(hw);
>> if (err < 0)
>> @@ -673,12 +638,9 @@ static int __maybe_unused hts221_suspend(struct device *dev)
>> {
>> struct iio_dev *iio_dev = dev_get_drvdata(dev);
>> struct hts221_hw *hw = iio_priv(iio_dev);
>> - int err;
>> -
>> - err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> - HTS221_ENABLE_MASK, false);
>>
>> - return err < 0 ? err : 0;
>> + return hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> + HTS221_ENABLE_MASK, false);
>> }
>>
>> static int __maybe_unused hts221_resume(struct device *dev)
>> diff --git a/drivers/iio/humidity/hts221_i2c.c b/drivers/iio/humidity/hts221_i2c.c
>> index 2c97350a0f76..b5b3f408a658 100644
>> --- a/drivers/iio/humidity/hts221_i2c.c
>> +++ b/drivers/iio/humidity/hts221_i2c.c
>> @@ -13,61 +13,33 @@
>> #include <linux/acpi.h>
>> #include <linux/i2c.h>
>> #include <linux/slab.h>
>> -#include "hts221.h"
>> -
>> -#define I2C_AUTO_INCREMENT 0x80
>> -
>> -static int hts221_i2c_read(struct device *dev, u8 addr, int len, u8 *data)
>> -{
>> - struct i2c_msg msg[2];
>> - struct i2c_client *client = to_i2c_client(dev);
>> -
>> - if (len > 1)
>> - addr |= I2C_AUTO_INCREMENT;
>> -
>> - msg[0].addr = client->addr;
>> - msg[0].flags = client->flags;
>> - msg[0].len = 1;
>> - msg[0].buf = &addr;
>> -
>> - msg[1].addr = client->addr;
>> - msg[1].flags = client->flags | I2C_M_RD;
>> - msg[1].len = len;
>> - msg[1].buf = data;
>> -
>> - return i2c_transfer(client->adapter, msg, 2);
>> -}
>> +#include <linux/regmap.h>
>>
>> -static int hts221_i2c_write(struct device *dev, u8 addr, int len, u8 *data)
>> -{
>> - u8 send[len + 1];
>> - struct i2c_msg msg;
>> - struct i2c_client *client = to_i2c_client(dev);
>> -
>> - if (len > 1)
>> - addr |= I2C_AUTO_INCREMENT;
>> -
>> - send[0] = addr;
>> - memcpy(&send[1], data, len * sizeof(u8));
>> -
>> - msg.addr = client->addr;
>> - msg.flags = client->flags;
>> - msg.len = len + 1;
>> - msg.buf = send;
>> +#include "hts221.h"
>>
>> - return i2c_transfer(client->adapter, &msg, 1);
>> -}
>> +#define HTS221_I2C_AUTO_INCREMENT BIT(7)
>>
>> -static const struct hts221_transfer_function hts221_transfer_fn = {
>> - .read = hts221_i2c_read,
>> - .write = hts221_i2c_write,
>> +static const struct regmap_config hts221_i2c_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .write_flag_mask = HTS221_I2C_AUTO_INCREMENT,
>> + .read_flag_mask = HTS221_I2C_AUTO_INCREMENT,
>> };
>>
>> static int hts221_i2c_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> + struct regmap *regmap;
>> +
>> + regmap = devm_regmap_init_i2c(client, &hts221_i2c_regmap_config);
>> + if (IS_ERR(regmap)) {
>> + dev_err(&client->dev, "Failed to register i2c regmap %d\n",
>> + (int)PTR_ERR(regmap));
>> + return PTR_ERR(regmap);
>> + }
>> +
>> return hts221_probe(&client->dev, client->irq,
>> - client->name, &hts221_transfer_fn);
>> + client->name, regmap);
>> }
>>
>> static const struct acpi_device_id hts221_acpi_match[] = {
>> diff --git a/drivers/iio/humidity/hts221_spi.c b/drivers/iio/humidity/hts221_spi.c
>> index 55b29b53b9d1..ea3aaec54035 100644
>> --- a/drivers/iio/humidity/hts221_spi.c
>> +++ b/drivers/iio/humidity/hts221_spi.c
>> @@ -12,76 +12,31 @@
>> #include <linux/module.h>
>> #include <linux/spi/spi.h>
>> #include <linux/slab.h>
>> -#include "hts221.h"
>> -
>> -#define SENSORS_SPI_READ 0x80
>
> I'm somewhat surprised to see this one go - not needed in the read mask
> for regmap?
Ack. Will do in v2, thx
>
>> -#define SPI_AUTO_INCREMENT 0x40
>> -
>> -static int hts221_spi_read(struct device *dev, u8 addr, int len, u8 *data)
>> -{
>> - int err;
>> - struct spi_device *spi = to_spi_device(dev);
>> - struct iio_dev *iio_dev = spi_get_drvdata(spi);
>> - struct hts221_hw *hw = iio_priv(iio_dev);
>> -
>> - struct spi_transfer xfers[] = {
>> - {
>> - .tx_buf = hw->tb.tx_buf,
>> - .bits_per_word = 8,
>> - .len = 1,
>> - },
>> - {
>> - .rx_buf = hw->tb.rx_buf,
>> - .bits_per_word = 8,
>> - .len = len,
>> - }
>> - };
>> -
>> - if (len > 1)
>> - addr |= SPI_AUTO_INCREMENT;
>> - hw->tb.tx_buf[0] = addr | SENSORS_SPI_READ;
>> -
>> - err = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
>> - if (err < 0)
>> - return err;
>> -
>> - memcpy(data, hw->tb.rx_buf, len * sizeof(u8));
>> +#include <linux/regmap.h>
>>
>> - return len;
>> -}
>> -
>> -static int hts221_spi_write(struct device *dev, u8 addr, int len, u8 *data)
>> -{
>> - struct spi_device *spi = to_spi_device(dev);
>> - struct iio_dev *iio_dev = spi_get_drvdata(spi);
>> - struct hts221_hw *hw = iio_priv(iio_dev);
>> -
>> - struct spi_transfer xfers = {
>> - .tx_buf = hw->tb.tx_buf,
>> - .bits_per_word = 8,
>> - .len = len + 1,
>> - };
>> -
>> - if (len >= HTS221_TX_MAX_LENGTH)
>> - return -ENOMEM;
>> -
>> - if (len > 1)
>> - addr |= SPI_AUTO_INCREMENT;
>> - hw->tb.tx_buf[0] = addr;
>> - memcpy(&hw->tb.tx_buf[1], data, len);
>> +#include "hts221.h"
>>
>> - return spi_sync_transfer(spi, &xfers, 1);
>> -}
>> +#define HTS221_SPI_AUTO_INCREMENT BIT(6)
>>
>> -static const struct hts221_transfer_function hts221_transfer_fn = {
>> - .read = hts221_spi_read,
>> - .write = hts221_spi_write,
>> +static const struct regmap_config hts221_spi_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .write_flag_mask = HTS221_SPI_AUTO_INCREMENT,
>> + .read_flag_mask = HTS221_SPI_AUTO_INCREMENT,
>> };
>>
>> static int hts221_spi_probe(struct spi_device *spi)
>> {
>> + struct regmap *regmap;
>> +
>> + regmap = devm_regmap_init_spi(spi, &hts221_spi_regmap_config);
>> + if (IS_ERR(regmap)) {
>> + dev_err(&spi->dev, "Failed to register spi regmap %d\n",
>> + (int)PTR_ERR(regmap));
>> + return PTR_ERR(regmap);
>> + }
>> return hts221_probe(&spi->dev, spi->irq,
>> - spi->modalias, &hts221_transfer_fn);
>> + spi->modalias, regmap);
>> }
>>
>> static const struct of_device_id hts221_spi_of_match[] = {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Lorenzo
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] iio: humidity: hts221: add regmap API support
2017-12-29 13:33 ` Lorenzo Bianconi
@ 2017-12-29 17:25 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2017-12-29 17:25 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, linux-iio
On Fri, 29 Dec 2017 14:33:39 +0100
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
> > On Sat, 23 Dec 2017 18:16:21 +0100
> > Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
> >
> >> Introduce regmap API support to access to i2c/spi bus instead of
> >> using a custom support.
> >> Remove lock mutex since concurrency is already managed by regmap API
> >>
> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> >
> > Looks good. A few minor comments and a question inline.
>
> Hi Jonathan,
>
> Thanks for the review.
>
> >
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/humidity/Kconfig | 2 +
> >> drivers/iio/humidity/hts221.h | 27 ++++--------
> >> drivers/iio/humidity/hts221_buffer.c | 20 ++++-----
> >> drivers/iio/humidity/hts221_core.c | 84 ++++++++++--------------------------
> >> drivers/iio/humidity/hts221_i2c.c | 64 ++++++++-------------------
> >> drivers/iio/humidity/hts221_spi.c | 79 ++++++++-------------------------
> >> 6 files changed, 76 insertions(+), 200 deletions(-)
> >>
> >> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
> >> index 2c0fc9a400b8..1a0d458e4f4e 100644
> >> --- a/drivers/iio/humidity/Kconfig
> >> +++ b/drivers/iio/humidity/Kconfig
> >> @@ -68,10 +68,12 @@ config HTS221
> >> config HTS221_I2C
> >> tristate
> >> depends on HTS221
> >> + select REGMAP_I2C
> >>
> >> config HTS221_SPI
> >> tristate
> >> depends on HTS221
> >> + select REGMAP_SPI
> >>
> >> config HTU21
> >> tristate "Measurement Specialties HTU21 humidity & temperature sensor"
> >> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
> >> index c581af8c0f5d..00596879010e 100644
> >> --- a/drivers/iio/humidity/hts221.h
> >> +++ b/drivers/iio/humidity/hts221.h
> >> @@ -15,21 +15,8 @@
> >>
> >> #include <linux/iio/iio.h>
> >>
> >> -#define HTS221_RX_MAX_LENGTH 8
> >> -#define HTS221_TX_MAX_LENGTH 8
> >> -
> >> #define HTS221_DATA_SIZE 2
> >>
> >> -struct hts221_transfer_buffer {
> >> - u8 rx_buf[HTS221_RX_MAX_LENGTH];
> >> - u8 tx_buf[HTS221_TX_MAX_LENGTH] ____cacheline_aligned;
> >> -};
> >> -
> >> -struct hts221_transfer_function {
> >> - int (*read)(struct device *dev, u8 addr, int len, u8 *data);
> >> - int (*write)(struct device *dev, u8 addr, int len, u8 *data);
> >> -};
> >> -
> >> enum hts221_sensor_type {
> >> HTS221_SENSOR_H,
> >> HTS221_SENSOR_T,
> >> @@ -44,8 +31,8 @@ struct hts221_sensor {
> >> struct hts221_hw {
> >> const char *name;
> >> struct device *dev;
> >> + struct regmap *regmap;
> >>
> >> - struct mutex lock;
> >> struct iio_trigger *trig;
> >> int irq;
> >>
> >> @@ -53,16 +40,18 @@ struct hts221_hw {
> >>
> >> bool enabled;
> >> u8 odr;
> >> -
> >> - const struct hts221_transfer_function *tf;
> >> - struct hts221_transfer_buffer tb;
> >> };
> >>
> >> extern const struct dev_pm_ops hts221_pm_ops;
> >>
> >> -int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
> >> +static inline int hts221_write_with_mask(struct hts221_hw *hw, u8 addr,
> >> + u8 mask, u8 val)
> >> +{
> >> + return regmap_update_bits(hw->regmap, addr, mask, val << __ffs(mask));
> >> +}
> >> +
> >> int hts221_probe(struct device *dev, int irq, const char *name,
> >> - const struct hts221_transfer_function *tf_ops);
> >> + struct regmap *regmap);
> >> int hts221_set_enable(struct hts221_hw *hw, bool enable);
> >> int hts221_allocate_buffers(struct hts221_hw *hw);
> >> int hts221_allocate_trigger(struct hts221_hw *hw);
> >> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
> >> index e971ea425268..b1141acfd100 100644
> >> --- a/drivers/iio/humidity/hts221_buffer.c
> >> +++ b/drivers/iio/humidity/hts221_buffer.c
> >> @@ -12,6 +12,7 @@
> >> #include <linux/device.h>
> >> #include <linux/interrupt.h>
> >> #include <linux/irqreturn.h>
> >> +#include <linux/regmap.h>
> >>
> >> #include <linux/iio/iio.h>
> >> #include <linux/iio/trigger.h>
> >> @@ -38,12 +39,9 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
> >> {
> >> struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
> >> struct hts221_hw *hw = iio_priv(iio_dev);
> >> - int err;
> >>
> >> - err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
> >> + return hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
> >> HTS221_REG_DRDY_EN_MASK, state);
> >
> > You could use the regmap call directly inconjunction with FIELD_PREP
> >
> > return regmap_update_bits(hw->regmap, HTS221_REG_DRDY_EN_ADDR,
> > HTS221_REG_DRDY_EN_MASK,
> > FIELD_PREP(HTS221_REG_DRDY_EN_MASK, val));
> >
>
> Do you prefer to always drop hts221_write_with_mask() and use just
> regmap_update_bits() with FIELD_PREP()?
Hmm. On balance yes. It slightly wins I think.
<snip>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-12-29 17:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-23 17:16 [PATCH 0/2] add regmap API support to hts221 iio driver Lorenzo Bianconi
2017-12-23 17:16 ` [PATCH 1/2] iio: humidity: hts221: move common code in hts221_core Lorenzo Bianconi
2017-12-29 13:24 ` Jonathan Cameron
2017-12-23 17:16 ` [PATCH 2/2] iio: humidity: hts221: add regmap API support Lorenzo Bianconi
2017-12-29 13:24 ` Jonathan Cameron
2017-12-29 13:33 ` Lorenzo Bianconi
2017-12-29 17:25 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox