Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1447928 > unrolled thread

[PATCH 2/2] staging: iio: vcnl4000: Replace i2c api's with regmap

Started byPratik Prajapati <pratik.prajapati12@gmail.com>
First post2016-07-21 16:50 +0200
Last post2016-07-25 08:30 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] staging: iio: vcnl4000: Replace i2c api's with regmap Pratik Prajapati <pratik.prajapati12@gmail.com> - 2016-07-21 16:50 +0200
    Re: [PATCH 2/2] staging: iio: vcnl4000: Replace i2c api's with regmap Jonathan Cameron <jic23@kernel.org> - 2016-07-24 12:30 +0200
      Re: [PATCH 2/2] staging: iio: vcnl4000: Replace i2c api's with regmap Pratik Prajapati <pratik.prajapati12@gmail.com> - 2016-07-25 08:30 +0200

#1447928 — [PATCH 2/2] staging: iio: vcnl4000: Replace i2c api's with regmap

FromPratik Prajapati <pratik.prajapati12@gmail.com>
Date2016-07-21 16:50 +0200
Subject[PATCH 2/2] staging: iio: vcnl4000: Replace i2c api's with regmap
Message-ID<rXnst-gG-1@gated-at.bofh.it>
Signed-off-by: Pratik Prajapati <pratik.prajapati12@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 91 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 78 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 1378175..61e48f8 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -20,6 +20,7 @@
 #include <linux/i2c.h>
 #include <linux/err.h>
 #include <linux/delay.h>
+#include <linux/regmap.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -52,6 +53,7 @@
 struct vcnl4000_data {
 	struct i2c_client *client;
 	struct mutex lock;
+	struct regmap *regmap;
 };
 
 static const struct i2c_device_id vcnl4000_id[] = {
@@ -66,20 +68,20 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
 	int tries = 20;
 	__be16 buf;
 	int ret;
+	unsigned int regval;
 
 	mutex_lock(&data->lock);
 
-	ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND,
-					req_mask);
+	ret = regmap_write(data->regmap, VCNL4000_COMMAND, req_mask);
 	if (ret < 0)
 		goto fail;
 
 	/* wait for data to become ready */
 	while (tries--) {
-		ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
+		ret = regmap_read(data->regmap, VCNL4000_COMMAND, &regval);
 		if (ret < 0)
 			goto fail;
-		if (ret & rdy_mask)
+		if (regval & rdy_mask)
 			break;
 		msleep(20); /* measurement takes up to 100 ms */
 	}
@@ -91,8 +93,8 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
 		goto fail;
 	}
 
-	ret = i2c_smbus_read_i2c_block_data(data->client,
-		data_reg, sizeof(buf), (u8 *) &buf);
+	ret = regmap_bulk_read(data->regmap, data_reg, (u8 *) &buf,
+			sizeof(buf));
 	if (ret < 0)
 		goto fail;
 
@@ -131,23 +133,24 @@ static int vcnl4000_write_led_current_raw(struct vcnl4000_data *data, int val)
 	if (val < 0 || val > VCNL4000_LED_CURRENT_MAX)
 		return -ERANGE;
 	mutex_lock(&data->lock);
-	ret = i2c_smbus_write_byte_data(data->client, VCNL4000_LED_CURRENT,
-			val);
+	ret = regmap_write_bits(data->regmap, VCNL4000_LED_CURRENT,
+		VCNL4000_LED_CURRENT_MASK, val);
 	mutex_unlock(&data->lock);
 	return ret;
 }
 
 static int vcnl4000_read_led_current_raw(struct vcnl4000_data *data)
 {
+	unsigned int regval;
 	int ret;
 
 	mutex_lock(&data->lock);
-	ret = i2c_smbus_read_byte_data(data->client, VCNL4000_LED_CURRENT);
+	ret = regmap_read(data->regmap, VCNL4000_LED_CURRENT, &regval);
 	mutex_unlock(&data->lock);
 	if (ret < 0)
 		return ret;
-	ret &= VCNL4000_LED_CURRENT_MASK;
-	return ret;
+	regval &= VCNL4000_LED_CURRENT_MASK;
+	return regval;
 }
 
 static int vcnl4000_read_raw(struct iio_dev *indio_dev,
@@ -224,27 +227,89 @@ static const struct iio_info vcnl4000_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static bool vcnl4000_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case VCNL4000_COMMAND:
+	case VCNL4000_PROD_REV:
+	case VCNL4000_LED_CURRENT:
+	case VCNL4000_AL_PARAM:
+	case VCNL4000_AL_RESULT_HI:
+	case VCNL4000_AL_RESULT_LO:
+	case VCNL4000_PS_RESULT_HI:
+	case VCNL4000_PS_RESULT_LO:
+	case VCNL4000_PS_MEAS_FREQ:
+	case VCNL4000_PS_MOD_ADJ:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool vcnl4000_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case VCNL4000_COMMAND:
+	case VCNL4000_LED_CURRENT:
+	case VCNL4000_AL_PARAM:
+	case VCNL4000_PS_MEAS_FREQ:
+	case VCNL4000_PS_MOD_ADJ:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool vcnl4000_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case VCNL4000_COMMAND:
+	case VCNL4000_AL_RESULT_HI:
+	case VCNL4000_AL_RESULT_LO:
+	case VCNL4000_PS_RESULT_HI:
+	case VCNL4000_PS_RESULT_LO:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config vcnl4000_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.max_register	= VCNL4000_PS_MOD_ADJ,
+	.readable_reg	= vcnl4000_readable_reg,
+	.writeable_reg	= vcnl4000_writeable_reg,
+	.volatile_reg	= vcnl4000_volatile_reg,
+};
+
 static int vcnl4000_probe(struct i2c_client *client,
 			  const struct i2c_device_id *id)
 {
 	struct vcnl4000_data *data;
 	struct iio_dev *indio_dev;
 	int ret, prod_id;
+	unsigned int regval;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
 	data = iio_priv(indio_dev);
+	data->regmap = devm_regmap_init_i2c(client, &vcnl4000_regmap_config);
+	if (IS_ERR(data->regmap)) {
+		dev_err(&client->dev, "regmap_init failed!\n");
+		return PTR_ERR(data->regmap);
+	}
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 	mutex_init(&data->lock);
 
-	ret = i2c_smbus_read_byte_data(data->client, VCNL4000_PROD_REV);
+	ret = regmap_read(data->regmap, VCNL4000_PROD_REV, &regval);
 	if (ret < 0)
 		return ret;
 
-	prod_id = ret >> 4;
+	prod_id = regval >> 4;
 	if (prod_id != VCNL4010_ID && prod_id != VCNL4000_ID)
 		return -ENODEV;
 
-- 
2.6.2

[toc] | [next] | [standalone]

#1449105

FromJonathan Cameron <jic23@kernel.org>
Date2016-07-24 12:30 +0200
Message-ID<rYoPv-6IJ-7@gated-at.bofh.it>
In reply to#1447928
On 21/07/16 15:41, Pratik Prajapati wrote:
> Signed-off-by: Pratik Prajapati <pratik.prajapati12@gmail.com>
> ---
>  drivers/iio/light/vcnl4000.c | 91 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 78 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 1378175..61e48f8 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -20,6 +20,7 @@
>  #include <linux/i2c.h>
>  #include <linux/err.h>
>  #include <linux/delay.h>
> +#include <linux/regmap.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -52,6 +53,7 @@
>  struct vcnl4000_data {
>  	struct i2c_client *client;
>  	struct mutex lock;
> +	struct regmap *regmap;
You can use the regmap_get_device to avoid having to carry the client through the whole
driver. I haven't checked the result of applying this patch but I'm not entirely sure
you use client at all.
>  };
>  
>  static const struct i2c_device_id vcnl4000_id[] = {
> @@ -66,20 +68,20 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
>  	int tries = 20;
>  	__be16 buf;
>  	int ret;
> +	unsigned int regval;
>  
>  	mutex_lock(&data->lock);
>  
> -	ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND,
> -					req_mask);
> +	ret = regmap_write(data->regmap, VCNL4000_COMMAND, req_mask);
>  	if (ret < 0)
>  		goto fail;
>  
>  	/* wait for data to become ready */
>  	while (tries--) {
> -		ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
> +		ret = regmap_read(data->regmap, VCNL4000_COMMAND, &regval);
>  		if (ret < 0)
>  			goto fail;
> -		if (ret & rdy_mask)
> +		if (regval & rdy_mask)
>  			break;
>  		msleep(20); /* measurement takes up to 100 ms */
>  	}
> @@ -91,8 +93,8 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
>  		goto fail;
>  	}
>  
> -	ret = i2c_smbus_read_i2c_block_data(data->client,
> -		data_reg, sizeof(buf), (u8 *) &buf);
> +	ret = regmap_bulk_read(data->regmap, data_reg, (u8 *) &buf,
> +			sizeof(buf));
>  	if (ret < 0)
>  		goto fail;
>  
> @@ -131,23 +133,24 @@ static int vcnl4000_write_led_current_raw(struct vcnl4000_data *data, int val)
>  	if (val < 0 || val > VCNL4000_LED_CURRENT_MAX)
>  		return -ERANGE;
>  	mutex_lock(&data->lock);
> -	ret = i2c_smbus_write_byte_data(data->client, VCNL4000_LED_CURRENT,
> -			val);
Just noticed this.  You didn't do a masked write in the previous patch.
Are you sure that wasn't overwriting something it shouldn't have been?
> +	ret = regmap_write_bits(data->regmap, VCNL4000_LED_CURRENT,
> +		VCNL4000_LED_CURRENT_MASK, val);
>  	mutex_unlock(&data->lock);
>  	return ret;
>  }
>  
>  static int vcnl4000_read_led_current_raw(struct vcnl4000_data *data)
>  {
> +	unsigned int regval;
>  	int ret;
>  
>  	mutex_lock(&data->lock);
> -	ret = i2c_smbus_read_byte_data(data->client, VCNL4000_LED_CURRENT);
> +	ret = regmap_read(data->regmap, VCNL4000_LED_CURRENT, &regval);
>  	mutex_unlock(&data->lock);
>  	if (ret < 0)
>  		return ret;
> -	ret &= VCNL4000_LED_CURRENT_MASK;
> -	return ret;
> +	regval &= VCNL4000_LED_CURRENT_MASK;
Tempting to roll these two lines into one, but then it wouldn't be
quite so obvious that you did the conversion right.  Up to you :)
> +	return regval;
>  }
>  
>  static int vcnl4000_read_raw(struct iio_dev *indio_dev,
> @@ -224,27 +227,89 @@ static const struct iio_info vcnl4000_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static bool vcnl4000_readable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case VCNL4000_COMMAND:
> +	case VCNL4000_PROD_REV:
> +	case VCNL4000_LED_CURRENT:
> +	case VCNL4000_AL_PARAM:
> +	case VCNL4000_AL_RESULT_HI:
> +	case VCNL4000_AL_RESULT_LO:
> +	case VCNL4000_PS_RESULT_HI:
> +	case VCNL4000_PS_RESULT_LO:
> +	case VCNL4000_PS_MEAS_FREQ:
> +	case VCNL4000_PS_MOD_ADJ:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool vcnl4000_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case VCNL4000_COMMAND:
> +	case VCNL4000_LED_CURRENT:
> +	case VCNL4000_AL_PARAM:
> +	case VCNL4000_PS_MEAS_FREQ:
> +	case VCNL4000_PS_MOD_ADJ:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool vcnl4000_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case VCNL4000_COMMAND:
> +	case VCNL4000_AL_RESULT_HI:
> +	case VCNL4000_AL_RESULT_LO:
> +	case VCNL4000_PS_RESULT_HI:
> +	case VCNL4000_PS_RESULT_LO:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config vcnl4000_regmap_config = {
> +	.reg_bits	= 8,
> +	.val_bits	= 8,
> +	.max_register	= VCNL4000_PS_MOD_ADJ,
> +	.readable_reg	= vcnl4000_readable_reg,
> +	.writeable_reg	= vcnl4000_writeable_reg,
> +	.volatile_reg	= vcnl4000_volatile_reg,
> +};
> +
>  static int vcnl4000_probe(struct i2c_client *client,
>  			  const struct i2c_device_id *id)
>  {
>  	struct vcnl4000_data *data;
>  	struct iio_dev *indio_dev;
>  	int ret, prod_id;
> +	unsigned int regval;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
>  	data = iio_priv(indio_dev);
> +	data->regmap = devm_regmap_init_i2c(client, &vcnl4000_regmap_config);
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(&client->dev, "regmap_init failed!\n");
> +		return PTR_ERR(data->regmap);
> +	}
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
>  	mutex_init(&data->lock);
>  
> -	ret = i2c_smbus_read_byte_data(data->client, VCNL4000_PROD_REV);
> +	ret = regmap_read(data->regmap, VCNL4000_PROD_REV, &regval);
>  	if (ret < 0)
>  		return ret;
>  
> -	prod_id = ret >> 4;
> +	prod_id = regval >> 4;
>  	if (prod_id != VCNL4010_ID && prod_id != VCNL4000_ID)
>  		return -ENODEV;
>  
>

[toc] | [prev] | [next] | [standalone]

#1449300

FromPratik Prajapati <pratik.prajapati12@gmail.com>
Date2016-07-25 08:30 +0200
Message-ID<rYHyN-19w-3@gated-at.bofh.it>
In reply to#1449105
On Sun, Jul 24, 2016 at 3:56 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 21/07/16 15:41, Pratik Prajapati wrote:
>> Signed-off-by: Pratik Prajapati <pratik.prajapati12@gmail.com>
>> ---
>>  drivers/iio/light/vcnl4000.c | 91 +++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 78 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>> index 1378175..61e48f8 100644
>> --- a/drivers/iio/light/vcnl4000.c
>> +++ b/drivers/iio/light/vcnl4000.c
>> @@ -20,6 +20,7 @@
>>  #include <linux/i2c.h>
>>  #include <linux/err.h>
>>  #include <linux/delay.h>
>> +#include <linux/regmap.h>
>>
>>  #include <linux/iio/iio.h>
>>  #include <linux/iio/sysfs.h>
>> @@ -52,6 +53,7 @@
>>  struct vcnl4000_data {
>>       struct i2c_client *client;
>>       struct mutex lock;
>> +     struct regmap *regmap;
> You can use the regmap_get_device to avoid having to carry the client through the whole
> driver. I haven't checked the result of applying this patch but I'm not entirely sure
> you use client at all.
I have used client for devm_regmap_init_i2c() and i2c_set_clientdata()

>>  };
>>
>>  static const struct i2c_device_id vcnl4000_id[] = {
>> @@ -66,20 +68,20 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
>>       int tries = 20;
>>       __be16 buf;
>>       int ret;
>> +     unsigned int regval;
>>
>>       mutex_lock(&data->lock);
>>
>> -     ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND,
>> -                                     req_mask);
>> +     ret = regmap_write(data->regmap, VCNL4000_COMMAND, req_mask);
>>       if (ret < 0)
>>               goto fail;
>>
>>       /* wait for data to become ready */
>>       while (tries--) {
>> -             ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
>> +             ret = regmap_read(data->regmap, VCNL4000_COMMAND, &regval);
>>               if (ret < 0)
>>                       goto fail;
>> -             if (ret & rdy_mask)
>> +             if (regval & rdy_mask)
>>                       break;
>>               msleep(20); /* measurement takes up to 100 ms */
>>       }
>> @@ -91,8 +93,8 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
>>               goto fail;
>>       }
>>
>> -     ret = i2c_smbus_read_i2c_block_data(data->client,
>> -             data_reg, sizeof(buf), (u8 *) &buf);
>> +     ret = regmap_bulk_read(data->regmap, data_reg, (u8 *) &buf,
>> +                     sizeof(buf));
>>       if (ret < 0)
>>               goto fail;
>>
>> @@ -131,23 +133,24 @@ static int vcnl4000_write_led_current_raw(struct vcnl4000_data *data, int val)
>>       if (val < 0 || val > VCNL4000_LED_CURRENT_MAX)
>>               return -ERANGE;
>>       mutex_lock(&data->lock);
>> -     ret = i2c_smbus_write_byte_data(data->client, VCNL4000_LED_CURRENT,
>> -                     val);
> Just noticed this.  You didn't do a masked write in the previous patch.
> Are you sure that wasn't overwriting something it shouldn't have been?
No.
Its overwriting, I will correct that.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web