Re: [PATCH] iio: accel: mma8452: convert to regmap
On Wed, 3 Mar 2021 13:47:24 +0100
Sean Nyekjaer wrote:
> It will be easier to configure various device registers.
What do you mean by this? I'm not against regmap, but I think
a stronger rational for the change is needed.
A few specific comments inline, but nothing major.
Jonathan
>
> No functional change.
>
> Signed-off-by: Sean Nyekjaer
> ---
> drivers/iio/accel/mma8452.c | 291 +++-
> 1 file changed, 156 insertions(+), 135 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 33f0c419d8ff..fc29a6e16428 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
>
> #define MMA8452_STATUS 0x00
> #define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0))
> @@ -102,7 +103,8 @@
> #define MMA8452_AUTO_SUSPEND_DELAY_MS2000
>
> struct mma8452_data {
> - struct i2c_client *client;
> + struct regmap *regmap;
> + struct device *dev;
> struct mutex lock;
> u8 ctrl_reg1;
> u8 data_cfg;
> @@ -163,6 +165,11 @@ static const struct mma8452_event_regs trans_ev_regs = {
> .ev_count = MMA8452_TRANSIENT_COUNT,
> };
>
> +static const struct regmap_config mma8452_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> /**
> * struct mma_chip_info - chip specific data
> * @chip_id: WHO_AM_I register's value
> @@ -194,13 +201,14 @@ enum {
> static int mma8452_drdy(struct mma8452_data *data)
> {
> int tries = 150;
> + unsigned int reg;
>
> while (tries-- > 0) {
> - int ret = i2c_smbus_read_byte_data(data->client,
> - MMA8452_STATUS);
> + int ret = regmap_read(data->regmap, MMA8452_STATUS, );
> if (ret < 0)
> return ret;
> - if ((ret & MMA8452_STATUS_DRDY) == MMA8452_STATUS_DRDY)
> +
> + if ((reg & MMA8452_STATUS_DRDY) == MMA8452_STATUS_DRDY)
> return 0;
>
> if (data->sleep_val <= 20)
> @@ -210,28 +218,28 @@ static int mma8452_drdy(struct mma8452_data *data)
> msleep(20);
> }
>
> - dev_err(>client->dev, "data not ready\n");
> + dev_err(data->dev, "data not ready\n");
>
> return -EIO;
> }
>
> -static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
> +static int mma8452_set_runtime_pm_state(struct device *dev, bool on)
> {
> #ifdef CONFIG_PM
> int ret;
>
> if (on) {
> - ret = pm_runtime_get_sync(>dev);
> + ret = pm_runtime_get_sync(dev);
> } else {
> - pm_runtime_mark_last_busy(>dev);
> - ret = pm_runtime_put_autosuspend(>dev);
> + pm_runtime_mark_last_busy(dev);
> + ret = pm_runtime_put_autosuspend(dev);
> }
>
> if (ret < 0) {
> - dev_err(>dev,
> + dev_err(dev,
> "failed to change power state to %d\n", on);
> if (on)
> - pm_runtime_put_noidle(>dev);
> + pm_runtime_put_noidle(dev);
>
> return ret;
> }
> @@ -247,14 +255,17 @@ static int mma8452_read(struct mma8452_data *data,
> __be16 buf[3])
> if (ret < 0)
> return ret;
>
> - ret = mma8452_set_runtime_pm_state(data->client, true);
> + ret = mma8452_set_runtime_pm_state(data->dev, true);
> if (ret)
> return ret;
>
> - ret = i2c_smbus_read_i2c_block_data(data->client, MMA8452_OUT_X,
> - 3 * sizeof(__be16), (u8 *)buf);
> + ret = regmap_bulk_read(data->regmap, MMA8452_OUT_X, (u8 *)buf, 3 *
> sizeof(__be16));
> + if (ret < 0) {
> + dev_err(data->dev, "failed to read axes\n");
> + return ret;
> + }
>
> - ret = mma8452_set_runtime_pm_state(data->client, false);
> + ret = mma8452_set_runtime_pm_state(data->dev, false);
>
> return ret;
> }
> @@ -358,12 +369,12 @@ static const u16 mma8452_os_ratio[4][8] = {
>
> static int mma8452_get_power_mode(struct mma8452_data *data)
> {
> - int reg;
> + int ret;
> + unsigned int reg;
>
> - reg = i2c_smbus_read_byte_data(data->client,
> -MMA8452_CTRL_REG2);
> - if (reg < 0)
> - return reg;
> + ret = regmap_read(data->regmap, MMA8452_CTRL_REG2, );
> + if (ret < 0)
> + return ret;
>
> return ((reg & MMA8452_CTRL_REG2_MODS_MASK) >>
> MMA8452_CTRL_REG2_MODS_SHIFT);
> @@ -468,8 +479,9 @@ static int mma8452_get_hp_filter_index(struct
> mma8452_data *data,
> static int mma8452_read_hp_filter(struct mma8452_data *data, int *hz, int
> *uHz)
> {
> int j, i, ret;
> + unsigned int reg;
>
> - ret =
[PATCH] iio: accel: mma8452: convert to regmap
It will be easier to configure various device registers.
No functional change.
Signed-off-by: Sean Nyekjaer
---
drivers/iio/accel/mma8452.c | 291 +++-
1 file changed, 156 insertions(+), 135 deletions(-)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 33f0c419d8ff..fc29a6e16428 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#define MMA8452_STATUS 0x00
#define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0))
@@ -102,7 +103,8 @@
#define MMA8452_AUTO_SUSPEND_DELAY_MS 2000
struct mma8452_data {
- struct i2c_client *client;
+ struct regmap *regmap;
+ struct device *dev;
struct mutex lock;
u8 ctrl_reg1;
u8 data_cfg;
@@ -163,6 +165,11 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
};
+static const struct regmap_config mma8452_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
/**
* struct mma_chip_info - chip specific data
* @chip_id: WHO_AM_I register's value
@@ -194,13 +201,14 @@ enum {
static int mma8452_drdy(struct mma8452_data *data)
{
int tries = 150;
+ unsigned int reg;
while (tries-- > 0) {
- int ret = i2c_smbus_read_byte_data(data->client,
- MMA8452_STATUS);
+ int ret = regmap_read(data->regmap, MMA8452_STATUS, );
if (ret < 0)
return ret;
- if ((ret & MMA8452_STATUS_DRDY) == MMA8452_STATUS_DRDY)
+
+ if ((reg & MMA8452_STATUS_DRDY) == MMA8452_STATUS_DRDY)
return 0;
if (data->sleep_val <= 20)
@@ -210,28 +218,28 @@ static int mma8452_drdy(struct mma8452_data *data)
msleep(20);
}
- dev_err(>client->dev, "data not ready\n");
+ dev_err(data->dev, "data not ready\n");
return -EIO;
}
-static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
+static int mma8452_set_runtime_pm_state(struct device *dev, bool on)
{
#ifdef CONFIG_PM
int ret;
if (on) {
- ret = pm_runtime_get_sync(>dev);
+ ret = pm_runtime_get_sync(dev);
} else {
- pm_runtime_mark_last_busy(>dev);
- ret = pm_runtime_put_autosuspend(>dev);
+ pm_runtime_mark_last_busy(dev);
+ ret = pm_runtime_put_autosuspend(dev);
}
if (ret < 0) {
- dev_err(>dev,
+ dev_err(dev,
"failed to change power state to %d\n", on);
if (on)
- pm_runtime_put_noidle(>dev);
+ pm_runtime_put_noidle(dev);
return ret;
}
@@ -247,14 +255,17 @@ static int mma8452_read(struct mma8452_data *data, __be16
buf[3])
if (ret < 0)
return ret;
- ret = mma8452_set_runtime_pm_state(data->client, true);
+ ret = mma8452_set_runtime_pm_state(data->dev, true);
if (ret)
return ret;
- ret = i2c_smbus_read_i2c_block_data(data->client, MMA8452_OUT_X,
- 3 * sizeof(__be16), (u8 *)buf);
+ ret = regmap_bulk_read(data->regmap, MMA8452_OUT_X, (u8 *)buf, 3 *
sizeof(__be16));
+ if (ret < 0) {
+ dev_err(data->dev, "failed to read axes\n");
+ return ret;
+ }
- ret = mma8452_set_runtime_pm_state(data->client, false);
+ ret = mma8452_set_runtime_pm_state(data->dev, false);
return ret;
}
@@ -358,12 +369,12 @@ static const u16 mma8452_os_ratio[4][8] = {
static int mma8452_get_power_mode(struct mma8452_data *data)
{
- int reg;
+ int ret;
+ unsigned int reg;
- reg = i2c_smbus_read_byte_data(data->client,
- MMA8452_CTRL_REG2);
- if (reg < 0)
- return reg;
+ ret = regmap_read(data->regmap, MMA8452_CTRL_REG2, );
+ if (ret < 0)
+ return ret;
return ((reg & MMA8452_CTRL_REG2_MODS_MASK) >>
MMA8452_CTRL_REG2_MODS_SHIFT);
@@ -468,8 +479,9 @@ static int mma8452_get_hp_filter_index(struct mma8452_data
*data,
static int mma8452_read_hp_filter(struct mma8452_data *data, int *hz, int *uHz)
{
int j, i, ret;
+ unsigned int reg;
- ret = i2c_smbus_read_byte_data(data->client, MMA8452_HP_FILTER_CUTOFF);
+ ret = regmap_read(data->regmap, MMA8452_HP_FILTER_CUTOFF, );
if (ret < 0)
return ret;
@@ -479,8 +491,8 @@ static int mma8452_read_hp_filter(struct mma8452_data
*data, int *hz, int *uHz)
return j;
ret &= MMA8452_HP_FILTER_CUTOFF_SEL_MASK;
- *hz =
