staging:iio:tsl2583 update interfaces

PATCH In review Jonathan Cameron 1 participant 5 messages Jan 12, 2013 – Jan 20, 2013

Subsystems (3)

Staging IIO Staging - industrial io

lore

Share & tools

lore

Jonathan Cameron

author maintainer

2013-01-12 10:42 UTC #0 lore raw

v2 cover staging:iio:tsl2583 update interfaces

Final patch fixed so it doesn't contain a typo that prevents it compiling. Sorry about that.

This driver is now fairly old and has lagged a bit as we have brought in newer cleaner ways of doing things. Anyhow, I was looking at it with a view to moving it out of staging and thought I would clean up a few bits whilst doing so. Note I don't have one of these.

There are some 'unusual' interfaces in here still such as integrationtime. We need to have a think about whether we want to add these to the info mask and handle them through read/write_raw or not.

I'm also decidely unkeen on the manual power enable / disable that is still in this driver. That should ideally be done on demand.

Hence i am not proposing moving this out of staging just yet!

Jonathan

Jonathan Cameron (3):
staging:iio:light:tsl2583 use read_raw, write_raw hooks. staging:iio:light:tsl2583 apply in_ prefix to attributes. staging:iio:light:tsl2583 switch to kstrto and strtobool

 drivers/staging/iio/light/tsl2583.c | 247 +++++++++++++++++-------------------
 1 file changed, 119 insertions(+), 128 deletions(-)

Signature · 1 line

1.8.1

Jonathan Cameron

author maintainer

2013-01-12 10:42 UTC #1 lore raw

PATCH v2 1/3 staging:iio:light:tsl2583 use read_raw, write_raw hooks.

This driver predates the introduction of these hooks that both reduce the amount of code needed to provide basic interfaces and allow for inkernel access to these values from other drivers.

This will change a few interfaces by applying an in_ prefix. The rest of the driver will be brought in line with this convention in a follow up patch.

Signed-off-by Jonathan Cameron <[email protected]>

---
 drivers/staging/iio/light/tsl2583.c | 187 ++++++++++++++++--------------------
 1 file changed, 85 insertions(+), 102 deletions(-)

drivers/staging/iio/light/tsl2583.c +85 ~~-102~~

index b377dd3..30c1a36 100644
@@ -506,62 +506,6 @@ static ssize_t taos_power_state_store(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_gain_show(struct device *dev,
-	struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-	char gain[4] = {0};
-
-	switch (chip->taos_settings.als_gain) {
-	case 0:
-		strcpy(gain, "001");
-		break;
-	case 1:
-		strcpy(gain, "008");
-		break;
-	case 2:
-		strcpy(gain, "016");
-		break;
-	case 3:
-		strcpy(gain, "111");
-		break;
-	}
-
-	return sprintf(buf, "%s\n", gain);
-}
-
-static ssize_t taos_gain_store(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-	unsigned long value;
-
-	if (strict_strtoul(buf, 0, &value))
-		return -EINVAL;
-
-	switch (value) {
-	case 1:
-		chip->taos_settings.als_gain = 0;
-		break;
-	case 8:
-		chip->taos_settings.als_gain = 1;
-		break;
-	case 16:
-		chip->taos_settings.als_gain = 2;
-		break;
-	case 111:
-		chip->taos_settings.als_gain = 3;
-		break;
-	default:
-		dev_err(dev, "Invalid Gain Index (must be 1,8,16,111)\n");
-		return -1;
-	}
-
-	return len;
-}
-
 static ssize_t taos_gain_available_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {

@@ -605,31 +549,6 @@ static ssize_t taos_als_time_available_show(struct device *dev,
 		"50 100 150 200 250 300 350 400 450 500 550 600 650");
 }
 
-static ssize_t taos_als_trim_show(struct device *dev,
-	struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-
-	return sprintf(buf, "%d\n", chip->taos_settings.als_gain_trim);
-}
-
-static ssize_t taos_als_trim_store(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-	unsigned long value;
-
-	if (strict_strtoul(buf, 0, &value))
-		return -EINVAL;
-
-	if (value)
-		chip->taos_settings.als_gain_trim = value;
-
-	return len;
-}
-
 static ssize_t taos_als_cal_target_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {

@@ -655,18 +574,6 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_lux_show(struct device *dev, struct device_attribute *attr,
-	char *buf)
-{
-	int ret;
-
-	ret = taos_get_lux(dev_to_iio_dev(dev));
-	if (ret < 0)
-		return ret;
-
-	return sprintf(buf, "%d\n", ret);
-}
-
 static ssize_t taos_do_calibrate(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t len)
 {

@@ -745,8 +652,6 @@ static ssize_t taos_luxtable_store(struct device *dev,
 static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
 		taos_power_state_show, taos_power_state_store);
 
-static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
-		taos_gain_show, taos_gain_store);
 static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
 		taos_gain_available_show, NULL);

@@ -755,26 +660,19 @@ static DEVICE_ATTR(illuminance0_integration_time, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
 		taos_als_time_available_show, NULL);
 
-static DEVICE_ATTR(illuminance0_calibbias, S_IRUGO | S_IWUSR,
-		taos_als_trim_show, taos_als_trim_store);
-
 static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
 		taos_als_cal_target_show, taos_als_cal_target_store);
 
-static DEVICE_ATTR(illuminance0_input, S_IRUGO, taos_lux_show, NULL);
 static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
 static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
 		taos_luxtable_show, taos_luxtable_store);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
 	&dev_attr_power_state.attr,
-	&dev_attr_illuminance0_calibscale.attr,			/* Gain  */
 	&dev_attr_illuminance0_calibscale_available.attr,
 	&dev_attr_illuminance0_integration_time.attr,	/* I time*/
 	&dev_attr_illuminance0_integration_time_available.attr,
-	&dev_attr_illuminance0_calibbias.attr,			/* trim  */
 	&dev_attr_illuminance0_input_target.attr,
-	&dev_attr_illuminance0_input.attr,
 	&dev_attr_illuminance0_calibrate.attr,
 	&dev_attr_illuminance0_lux_table.attr,
 	NULL

@@ -790,9 +688,92 @@ static int taos_tsl258x_device(unsigned char *bufp)
 	return ((bufp[TSL258X_CHIPID] & 0xf0) == 0x90);
 }
 
+static const int tsl2583_scales[] = {1, 8, 16, 111 };
+
+static int tsl2583_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val,
+			     int val2, 
+			     long m)
+{
+	struct tsl2583_chip *chip = iio_priv(indio_dev);
+	int ret = 0;
+
+	switch (m) {
+	case IIO_CHAN_INFO_CALIBBIAS:
+		chip->taos_settings.als_gain_trim = val;
+		break;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		switch (val) {
+		case 1:
+			chip->taos_settings.als_gain = 0;
+			break;
+		case 8:
+			chip->taos_settings.als_gain = 1;
+			break;
+		case 16:
+			chip->taos_settings.als_gain = 2;
+			break;
+		case 111:
+			chip->taos_settings.als_gain = 3;
+			break;
+		default:
+			ret = -EINVAL;
+		}
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+static int tsl2583_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val,
+			    int *val2,
+			    long m)
+{
+	struct tsl2583_chip *chip = iio_priv(indio_dev);
+	int ret;
+
+	switch (m) {
+	case IIO_CHAN_INFO_PROCESSED:
+		ret = taos_get_lux(indio_dev);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		*val = chip->taos_settings.als_gain_trim;
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		*val = tsl2583_scales[chip->taos_settings.als_gain];
+		ret = IIO_VAL_INT;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct iio_chan_spec tsl2583_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.indexed = 1,
+		.info_mask = IIO_CHAN_INFO_PROCESSED_SEPARATE_BIT |
+		IIO_CHAN_INFO_CALIBSCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_CALIBBIAS_SEPARATE_BIT,
+		.channel = 0,
+	},
+};
+
 static const struct iio_info tsl2583_info = {
 	.attrs = &tsl2583_attribute_group,
 	.driver_module = THIS_MODULE,
+	.read_raw = &tsl2583_read_raw,
+	.write_raw = &tsl2583_write_raw,
 };
 
 /*

@@ -863,6 +844,8 @@ static int taos_probe(struct i2c_client *clientp,
 	indio_dev->info = &tsl2583_info;
 	indio_dev->dev.parent = &clientp->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = tsl2583_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tsl2583_channels);
 	indio_dev->name = chip->client->name;
 	ret = iio_device_register(indio_dev);
 	if (ret) {

Signature · 1 line

1.8.1

Jonathan Cameron

author maintainer

2013-01-12 10:42 UTC #2 lore raw

PATCH v2 2/3 staging:iio:light:tsl2583 apply in_ prefix to attributes.

In reply to Jonathan Cameron

Bring the naming of remaining attributes in line with those created via the channel spec.

Signed-off-by Jonathan Cameron <[email protected]>

---
 drivers/staging/iio/light/tsl2583.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

drivers/staging/iio/light/tsl2583.c +12 ~~-12~~

index 30c1a36..944db26 100644
@@ -652,29 +652,29 @@ static ssize_t taos_luxtable_store(struct device *dev,
 static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
 		taos_power_state_show, taos_power_state_store);
 
-static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
+static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
 		taos_gain_available_show, NULL);
 
-static DEVICE_ATTR(illuminance0_integration_time, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
 		taos_als_time_show, taos_als_time_store);
-static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
+static DEVICE_ATTR(in_illuminance0_integration_time_available, S_IRUGO,
 		taos_als_time_available_show, NULL);
 
-static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_input_target, S_IRUGO | S_IWUSR,
 		taos_als_cal_target_show, taos_als_cal_target_store);
 
-static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
-static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
+static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
 		taos_luxtable_show, taos_luxtable_store);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
 	&dev_attr_power_state.attr,
-	&dev_attr_illuminance0_calibscale_available.attr,
-	&dev_attr_illuminance0_integration_time.attr,	/* I time*/
-	&dev_attr_illuminance0_integration_time_available.attr,
-	&dev_attr_illuminance0_input_target.attr,
-	&dev_attr_illuminance0_calibrate.attr,
-	&dev_attr_illuminance0_lux_table.attr,
+	&dev_attr_in_illuminance0_calibscale_available.attr,
+	&dev_attr_in_illuminance0_integration_time.attr,	/* I time*/
+	&dev_attr_in_illuminance0_integration_time_available.attr,
+	&dev_attr_in_illuminance0_input_target.attr,
+	&dev_attr_in_illuminance0_calibrate.attr,
+	&dev_attr_in_illuminance0_lux_table.attr,
 	NULL
 };

Signature · 1 line

1.8.1

Jonathan Cameron

author maintainer

2013-01-12 10:42 UTC #3 lore raw

PATCH v2 3/3 staging:iio:light:tsl2583 switch to kstrto and strtobool

In reply to Jonathan Cameron

Doing this results in cleaner code and better error handling.

Signed-off-by Jonathan Cameron <[email protected]>

---
 drivers/staging/iio/light/tsl2583.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

drivers/staging/iio/light/tsl2583.c +22 ~~-14~~

index 944db26..dfae0f7 100644
@@ -493,15 +493,17 @@ static ssize_t taos_power_state_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	unsigned long value;
+	bool state;
+	int ret;
 
-	if (strict_strtoul(buf, 0, &value))
-		return -EINVAL;
+	ret = strtobool(buf, &state);
+	if (ret < 0)
+		return ret;
 
-	if (value == 0)
-		taos_chip_off(indio_dev);
-	else
+	if (state)
 		taos_chip_on(indio_dev);
+	else
+		taos_chip_off(indio_dev);		
 
 	return len;
 }

@@ -527,9 +529,11 @@ static ssize_t taos_als_time_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
 	unsigned long value;
+	int ret;
 
-	if (strict_strtoul(buf, 0, &value))
-		return -EINVAL;
+	ret = kstrtoul(buf, 0, &value);
+	if (ret < 0)
+		return ret;
 
 	if ((value < 50) || (value > 650))
 		return -EINVAL;

@@ -564,9 +568,11 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
 	unsigned long value;
+	int ret;
 
-	if (strict_strtoul(buf, 0, &value))
-		return -EINVAL;
+	ret = kstrtoul(buf, 0, &value);
+	if (ret < 0)
+		return ret;
 
 	if (value)
 		chip->taos_settings.als_cal_target = value;

@@ -578,12 +584,14 @@ static ssize_t taos_do_calibrate(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	unsigned long value;
+	bool cal;
+	int ret;
 
-	if (strict_strtoul(buf, 0, &value))
-		return -EINVAL;
+	ret = strtobool(buf, &cal);
+	if (ret < 0)
+		return ret;
 
-	if (value == 1)
+	if (cal)
 		taos_als_calibrate(indio_dev);
 
 	return len;

Signature · 1 line

1.8.1

Jonathan Cameron

author maintainer

2013-01-20 12:10 UTC #4 lore raw

In reply to Jonathan Cameron

Jon,

Any chance you could have a look at these? No particular rush though...

Jonathan

Quoted 27 lines · Jonathan Cameron

Final patch fixed so it doesn't contain a typo that prevents it compiling. Sorry about that.
This driver is now fairly old and has lagged a bit as we have brought in newer cleaner ways of doing things. Anyhow, I was looking at it with a view to moving it out of staging and thought I would clean up a few bits whilst doing so. Note I don't have one of these.
There are some 'unusual' interfaces in here still such as integrationtime. We need to have a think about whether we want to add these to the info mask and handle them through read/write_raw or not.
I'm also decidely unkeen on the manual power enable / disable that is still in this driver. That should ideally be done on demand.
Hence i am not proposing moving this out of staging just yet!
Jonathan
Jonathan Cameron (3):
staging:iio:light:tsl2583 use read_raw, write_raw hooks. staging:iio:light:tsl2583 apply in_ prefix to attributes. staging:iio:light:tsl2583 switch to kstrto and strtobool
 drivers/staging/iio/light/tsl2583.c | 247 +++++++++++++++++-------------------
 1 file changed, 119 insertions(+), 128 deletions(-)