[PATCH] staging: iio: light: Replace symbolic permissions as per coding style

[PATCH] staging: iio: light: Replace symbolic permissions as per coding style

[surenderpolsani]

From: Surender Polsani <surenderpolsani@gmail.com>

Fixed the following checkpatch.pl warnings:
octal permissions are more preferable than symbolic permissions

Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
---
 drivers/staging/iio/light/tsl2x7x_core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
index af3910b..c63fe6a 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -1498,34 +1498,34 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(power_state, 0644,
 		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
 
-static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO,
+static DEVICE_ATTR(in_proximity0_calibscale_available, 0444,
 		tsl2x7x_prox_gain_available_show, NULL);
 
-static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
+static DEVICE_ATTR(in_illuminance0_calibscale_available, 0444,
 		tsl2x7x_gain_available_show, NULL);
 
-static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_integration_time, 0644,
 		tsl2x7x_als_time_show, tsl2x7x_als_time_store);
 
-static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_target_input, 0644,
 		tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store);
 
-static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL,
+static DEVICE_ATTR(in_illuminance0_calibrate, 0200, NULL,
 		tsl2x7x_do_calibrate);
 
-static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL,
+static DEVICE_ATTR(in_proximity0_calibrate, 0200, NULL,
 		tsl2x7x_do_prox_calibrate);
 
-static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_lux_table, 0644,
 		tsl2x7x_luxtable_show, tsl2x7x_luxtable_store);
 
-static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_intensity0_thresh_period, 0644,
 		tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store);
 
-static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_proximity0_thresh_period, 0644,
 		tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store);
 
 /* Use the default register values to identify the Taos device */
-- 
1.9.1

Re: [PATCH] staging: iio: light: Replace symbolic permissions as per coding style

[Greg KH]

On Thu, May 18, 2017 at 02:06:29PM +0530, surenderpolsani@gmail.com wrote:
> From: Surender Polsani <surenderpolsani@gmail.com>
> 
> Fixed the following checkpatch.pl warnings:
> octal permissions are more preferable than symbolic permissions
> 
> Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
> ---
>  drivers/staging/iio/light/tsl2x7x_core.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
> index af3910b..c63fe6a 100644
> --- a/drivers/staging/iio/light/tsl2x7x_core.c
> +++ b/drivers/staging/iio/light/tsl2x7x_core.c
> @@ -1498,34 +1498,34 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(power_state, 0644,
>  		tsl2x7x_power_state_show, tsl2x7x_power_state_store);

How about using DEVICE_ATTR_RW() and DEVICE_ATTR_RO() instead?

thanks,

greg k-h

[PATCH v2] staging: iio: light: Replace symbolic permissions as per coding style

[surenderpolsani]

From: Surender Polsani <surenderpolsani@gmail.com>

Fixed the following checkpatch.pl warnings:
octal permissions are more preferable than symbolic permissions

Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
as suggested by Greg K-H. Changed attributes and function
names where ever required to satisfy internal macro definitions
like __ATTR__RW().

Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
---
Changes for v2:

- Made few changes as suggested by Greg K-H and modified
  description accordingly.
---
 drivers/staging/iio/light/tsl2x7x_core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
index af3910b..c63fe6a 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -1498,34 +1498,34 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(power_state, 0644,
 		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
 
-static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO,
+static DEVICE_ATTR(in_proximity0_calibscale_available, 0444,
 		tsl2x7x_prox_gain_available_show, NULL);
 
-static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
+static DEVICE_ATTR(in_illuminance0_calibscale_available, 0444,
 		tsl2x7x_gain_available_show, NULL);
 
-static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_integration_time, 0644,
 		tsl2x7x_als_time_show, tsl2x7x_als_time_store);
 
-static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_target_input, 0644,
 		tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store);
 
-static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL,
+static DEVICE_ATTR(in_illuminance0_calibrate, 0200, NULL,
 		tsl2x7x_do_calibrate);
 
-static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL,
+static DEVICE_ATTR(in_proximity0_calibrate, 0200, NULL,
 		tsl2x7x_do_prox_calibrate);
 
-static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_illuminance0_lux_table, 0644,
 		tsl2x7x_luxtable_show, tsl2x7x_luxtable_store);
 
-static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_intensity0_thresh_period, 0644,
 		tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store);
 
-static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(in_proximity0_thresh_period, 0644,
 		tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store);
 
 /* Use the default register values to identify the Taos device */
-- 
1.9.1

Re: [PATCH v2] staging: iio: light: Replace symbolic permissions as per coding style

[Greg KH]

On Fri, May 19, 2017 at 12:33:11PM +0530, surenderpolsani@gmail.com wrote:
> From: Surender Polsani <surenderpolsani@gmail.com>
> 
> Fixed the following checkpatch.pl warnings:
> octal permissions are more preferable than symbolic permissions
> 
> Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
> as suggested by Greg K-H. Changed attributes and function
> names where ever required to satisfy internal macro definitions
> like __ATTR__RW().

No you did not:

> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> +static DEVICE_ATTR(power_state, 0644,
>  		tsl2x7x_power_state_show, tsl2x7x_power_state_store);

???

Wrong patch?

Re: [PATCH v2] staging: iio: light: Replace symbolic permissions as per coding style

[suri]

On Friday 19 May 2017 12:45 PM, Greg KH wrote:
> On Fri, May 19, 2017 at 12:33:11PM +0530, surenderpolsani@gmail.com wrote:
>> From: Surender Polsani <surenderpolsani@gmail.com>
>>
>> Fixed the following checkpatch.pl warnings:
>> octal permissions are more preferable than symbolic permissions
>>
>> Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
>> as suggested by Greg K-H. Changed attributes and function
>> names where ever required to satisfy internal macro definitions
>> like __ATTR__RW().
>
> No you did not:
>
>> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
>> +static DEVICE_ATTR(power_state, 0644,
>>  		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
>
> ???
>
> Wrong patch?
>

Sorry i sent wrong patch by mistake. I will send correct patch shortly.

[PATCH v2] staging: iio: light: Replace symbolic permissions as per coding style

[surenderpolsani]

From: Surender Polsani <surenderpolsani@gmail.com>

Fixed the following checkpatch.pl warnings:
octal permissions are more preferable than symbolic permissions

Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
as suggested by Greg K-H. Changed attributes and function
names where ever required to satisfy internal macro definitions
like __ATTR__RW().

Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
---
Changes for v2:

- Made few changes as suggested by Greg K-H and modified
  description accordingly.
---
 drivers/staging/iio/light/tsl2x7x_core.c | 94 ++++++++++++++------------------
 1 file changed, 42 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
index af3910b..48768bd 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -1140,7 +1140,7 @@ static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
 	return IIO_VAL_INT_PLUS_MICRO;
 }
 
-static ssize_t tsl2x7x_do_calibrate(struct device *dev,
+static ssize_t tsl2x7x_do_calibrate_store(struct device *dev,
 				    struct device_attribute *attr,
 				    const char *buf, size_t len)
 {
@@ -1226,7 +1226,7 @@ static ssize_t tsl2x7x_luxtable_store(struct device *dev,
 	return len;
 }
 
-static ssize_t tsl2x7x_do_prox_calibrate(struct device *dev,
+static ssize_t tsl2x7x_do_prox_calibrate_store(struct device *dev,
 					 struct device_attribute *attr,
 					 const char *buf, size_t len)
 {
@@ -1498,35 +1498,25 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
-		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
+static DEVICE_ATTR_RW(tsl2x7x_power_state);
 
-static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO,
-		tsl2x7x_prox_gain_available_show, NULL);
+static DEVICE_ATTR_RO(tsl2x7x_prox_gain_available);
 
-static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
-		tsl2x7x_gain_available_show, NULL);
+static DEVICE_ATTR_RO(tsl2x7x_gain_available);
 
-static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
-		tsl2x7x_als_time_show, tsl2x7x_als_time_store);
+static DEVICE_ATTR_RW(tsl2x7x_als_time);
 
-static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR,
-		tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store);
+static DEVICE_ATTR_RW(tsl2x7x_als_cal_target);
 
-static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL,
-		tsl2x7x_do_calibrate);
+static DEVICE_ATTR_WO(tsl2x7x_do_calibrate);
 
-static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL,
-		tsl2x7x_do_prox_calibrate);
+static DEVICE_ATTR_WO(tsl2x7x_do_prox_calibrate);
 
-static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
-		tsl2x7x_luxtable_show, tsl2x7x_luxtable_store);
+static DEVICE_ATTR_RW(tsl2x7x_luxtable);
 
-static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR,
-		tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store);
+static DEVICE_ATTR_RW(tsl2x7x_als_persistence);
 
-static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR,
-		tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store);
+static DEVICE_ATTR_RW(tsl2x7x_prox_persistence);
 
 /* Use the default register values to identify the Taos device */
 static int tsl2x7x_device_id(unsigned char *id, int target)
@@ -1594,67 +1584,67 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 }
 
 static struct attribute *tsl2x7x_ALS_device_attrs[] = {
-	&dev_attr_power_state.attr,
-	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&dev_attr_in_illuminance0_integration_time.attr,
+	&dev_attr_tsl2x7x_power_state.attr,
+	&dev_attr_tsl2x7x_gain_available.attr,
+	&dev_attr_tsl2x7x_als_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
-	&dev_attr_in_illuminance0_target_input.attr,
-	&dev_attr_in_illuminance0_calibrate.attr,
-	&dev_attr_in_illuminance0_lux_table.attr,
+	&dev_attr_tsl2x7x_als_cal_target.attr,
+	&dev_attr_tsl2x7x_do_calibrate.attr,
+	&dev_attr_tsl2x7x_luxtable.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_PRX_device_attrs[] = {
-	&dev_attr_power_state.attr,
-	&dev_attr_in_proximity0_calibrate.attr,
+	&dev_attr_tsl2x7x_power_state.attr,
+	&dev_attr_tsl2x7x_do_prox_calibrate.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
-	&dev_attr_power_state.attr,
-	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&dev_attr_in_illuminance0_integration_time.attr,
+	&dev_attr_tsl2x7x_power_state.attr,
+	&dev_attr_tsl2x7x_gain_available.attr,
+	&dev_attr_tsl2x7x_als_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
-	&dev_attr_in_illuminance0_target_input.attr,
-	&dev_attr_in_illuminance0_calibrate.attr,
-	&dev_attr_in_illuminance0_lux_table.attr,
-	&dev_attr_in_proximity0_calibrate.attr,
+	&dev_attr_tsl2x7x_als_cal_target.attr,
+	&dev_attr_tsl2x7x_do_calibrate.attr,
+	&dev_attr_tsl2x7x_luxtable.attr,
+	&dev_attr_tsl2x7x_do_prox_calibrate.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
-	&dev_attr_power_state.attr,
-	&dev_attr_in_proximity0_calibrate.attr,
-	&dev_attr_in_proximity0_calibscale_available.attr,
+	&dev_attr_tsl2x7x_power_state.attr,
+	&dev_attr_tsl2x7x_do_prox_calibrate.attr,
+	&dev_attr_tsl2x7x_prox_gain_available.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
-	&dev_attr_power_state.attr,
-	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&dev_attr_in_illuminance0_integration_time.attr,
+	&dev_attr_tsl2x7x_power_state.attr,
+	&dev_attr_tsl2x7x_gain_available.attr,
+	&dev_attr_tsl2x7x_als_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
-	&dev_attr_in_illuminance0_target_input.attr,
-	&dev_attr_in_illuminance0_calibrate.attr,
-	&dev_attr_in_illuminance0_lux_table.attr,
-	&dev_attr_in_proximity0_calibrate.attr,
-	&dev_attr_in_proximity0_calibscale_available.attr,
+	&dev_attr_tsl2x7x_als_cal_target.attr,
+	&dev_attr_tsl2x7x_do_calibrate.attr,
+	&dev_attr_tsl2x7x_luxtable.attr,
+	&dev_attr_tsl2x7x_do_prox_calibrate.attr,
+	&dev_attr_tsl2x7x_prox_gain_available.attr,
 	NULL
 };
 
 static struct attribute *tsl2X7X_ALS_event_attrs[] = {
-	&dev_attr_in_intensity0_thresh_period.attr,
+	&dev_attr_tsl2x7x_als_persistence.attr,
 	NULL,
 };
 
 static struct attribute *tsl2X7X_PRX_event_attrs[] = {
-	&dev_attr_in_proximity0_thresh_period.attr,
+	&dev_attr_tsl2x7x_prox_persistence.attr,
 	NULL,
 };
 
 static struct attribute *tsl2X7X_ALSPRX_event_attrs[] = {
-	&dev_attr_in_intensity0_thresh_period.attr,
-	&dev_attr_in_proximity0_thresh_period.attr,
+	&dev_attr_tsl2x7x_als_persistence.attr,
+	&dev_attr_tsl2x7x_prox_persistence.attr,
 	NULL,
 };
 
-- 
1.9.1

Re: [PATCH v2] staging: iio: light: Replace symbolic permissions as per coding style

[Greg KH]

On Fri, May 19, 2017 at 01:42:55PM +0530, surenderpolsani@gmail.com wrote:
> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> -		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
> +static DEVICE_ATTR_RW(tsl2x7x_power_state);

You do realize you just renamed the sysfs file, changing the user/kernel
api?  That's generally not a good thing to ever do :(

thanks,

greg k-h

[PATCH v3] staging: iio: light: Replace symbolic permissions as per coding style

[surenderpolsani]

From: Surender Polsani <surenderpolsani@gmail.com>

Fixed the following checkpatch.pl warnings:
octal permissions are more preferable than symbolic permissions

Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
as suggested by Greg K-H. Changed attributes and function
names where ever required to satisfy internal macro definitions
like __ATTR__RW().

Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
---
Changes for v3:

- Reverted some changes from previous version and made
  changes as suggested by Greg K-H.

Changes for v2:

- Made few changes as suggested by Greg K-H and modified
  description accordingly.
---
 drivers/staging/iio/light/tsl2x7x_core.c | 62 ++++++++++++++------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
index af3910b..4de1408 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -919,7 +919,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 		tsl2x7x_chip_on(indio_dev);
 }
 
-static ssize_t tsl2x7x_power_state_show(struct device *dev,
+static ssize_t power_state_show(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
 {
@@ -928,7 +928,7 @@ static ssize_t tsl2x7x_power_state_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", chip->tsl2x7x_chip_status);
 }
 
-static ssize_t tsl2x7x_power_state_store(struct device *dev,
+static ssize_t power_state_store(struct device *dev,
 					 struct device_attribute *attr,
 					 const char *buf, size_t len)
 {
@@ -946,7 +946,7 @@ static ssize_t tsl2x7x_power_state_store(struct device *dev,
 	return len;
 }
 
-static ssize_t tsl2x7x_gain_available_show(struct device *dev,
+static ssize_t in_illuminance0_calibscale_available_show(struct device *dev,
 					   struct device_attribute *attr,
 					   char *buf)
 {
@@ -964,14 +964,14 @@ static ssize_t tsl2x7x_gain_available_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%s\n", "1 8 16 120");
 }
 
-static ssize_t tsl2x7x_prox_gain_available_show(struct device *dev,
+static ssize_t in_proximity0_calibscale_available_show(struct device *dev,
 						struct device_attribute *attr,
 						char *buf)
 {
 		return snprintf(buf, PAGE_SIZE, "%s\n", "1 2 4 8");
 }
 
-static ssize_t tsl2x7x_als_time_show(struct device *dev,
+static ssize_t in_illuminance0_integration_time_show(struct device *dev,
 				     struct device_attribute *attr,
 				     char *buf)
 {
@@ -986,7 +986,7 @@ static ssize_t tsl2x7x_als_time_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
 }
 
-static ssize_t tsl2x7x_als_time_store(struct device *dev,
+static ssize_t in_illuminance0_integration_time_store(struct device *dev,
 				      struct device_attribute *attr,
 				      const char *buf, size_t len)
 {
@@ -1014,7 +1014,7 @@ static ssize_t tsl2x7x_als_time_store(struct device *dev,
 static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
 		".00272 - .696");
 
-static ssize_t tsl2x7x_als_cal_target_show(struct device *dev,
+static ssize_t in_illuminance0_target_input_show(struct device *dev,
 					   struct device_attribute *attr,
 					   char *buf)
 {
@@ -1024,7 +1024,7 @@ static ssize_t tsl2x7x_als_cal_target_show(struct device *dev,
 			chip->tsl2x7x_settings.als_cal_target);
 }
 
-static ssize_t tsl2x7x_als_cal_target_store(struct device *dev,
+static ssize_t in_illuminance0_target_input_store(struct device *dev,
 					    struct device_attribute *attr,
 					    const char *buf, size_t len)
 {
@@ -1044,7 +1044,7 @@ static ssize_t tsl2x7x_als_cal_target_store(struct device *dev,
 }
 
 /* persistence settings */
-static ssize_t tsl2x7x_als_persistence_show(struct device *dev,
+static ssize_t in_intensity0_thresh_period_show(struct device *dev,
 					    struct device_attribute *attr,
 					    char *buf)
 {
@@ -1061,7 +1061,7 @@ static ssize_t tsl2x7x_als_persistence_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
 }
 
-static ssize_t tsl2x7x_als_persistence_store(struct device *dev,
+static ssize_t in_intensity0_thresh_period_store(struct device *dev,
 					     struct device_attribute *attr,
 					     const char *buf, size_t len)
 {
@@ -1092,7 +1092,7 @@ static ssize_t tsl2x7x_als_persistence_store(struct device *dev,
 	return IIO_VAL_INT_PLUS_MICRO;
 }
 
-static ssize_t tsl2x7x_prox_persistence_show(struct device *dev,
+static ssize_t in_proximity0_thresh_period_show(struct device *dev,
 					     struct device_attribute *attr,
 					     char *buf)
 {
@@ -1109,7 +1109,7 @@ static ssize_t tsl2x7x_prox_persistence_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
 }
 
-static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
+static ssize_t in_proximity0_thresh_period_store(struct device *dev,
 					      struct device_attribute *attr,
 					      const char *buf, size_t len)
 {
@@ -1140,7 +1140,7 @@ static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
 	return IIO_VAL_INT_PLUS_MICRO;
 }
 
-static ssize_t tsl2x7x_do_calibrate(struct device *dev,
+static ssize_t in_illuminance0_calibrate_store(struct device *dev,
 				    struct device_attribute *attr,
 				    const char *buf, size_t len)
 {
@@ -1158,7 +1158,7 @@ static ssize_t tsl2x7x_do_calibrate(struct device *dev,
 	return len;
 }
 
-static ssize_t tsl2x7x_luxtable_show(struct device *dev,
+static ssize_t in_illuminance0_lux_table_show(struct device *dev,
 				     struct device_attribute *attr,
 				     char *buf)
 {
@@ -1186,7 +1186,7 @@ static ssize_t tsl2x7x_luxtable_show(struct device *dev,
 	return offset;
 }
 
-static ssize_t tsl2x7x_luxtable_store(struct device *dev,
+static ssize_t in_illuminance0_lux_table_store(struct device *dev,
 				      struct device_attribute *attr,
 				      const char *buf, size_t len)
 {
@@ -1226,7 +1226,7 @@ static ssize_t tsl2x7x_luxtable_store(struct device *dev,
 	return len;
 }
 
-static ssize_t tsl2x7x_do_prox_calibrate(struct device *dev,
+static ssize_t in_proximity0_calibrate_store(struct device *dev,
 					 struct device_attribute *attr,
 					 const char *buf, size_t len)
 {
@@ -1498,35 +1498,25 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
-		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
+static DEVICE_ATTR_RW(power_state);
 
-static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO,
-		tsl2x7x_prox_gain_available_show, NULL);
+static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
 
-static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
-		tsl2x7x_gain_available_show, NULL);
+static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
 
-static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
-		tsl2x7x_als_time_show, tsl2x7x_als_time_store);
+static DEVICE_ATTR_RW(in_illuminance0_integration_time);
 
-static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR,
-		tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store);
+static DEVICE_ATTR_RW(in_illuminance0_target_input);
 
-static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL,
-		tsl2x7x_do_calibrate);
+static DEVICE_ATTR_WO(in_illuminance0_calibrate);
 
-static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL,
-		tsl2x7x_do_prox_calibrate);
+static DEVICE_ATTR_WO(in_proximity0_calibrate);
 
-static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
-		tsl2x7x_luxtable_show, tsl2x7x_luxtable_store);
+static DEVICE_ATTR_RW(in_illuminance0_lux_table);
 
-static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR,
-		tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store);
+static DEVICE_ATTR_RW(in_intensity0_thresh_period);
 
-static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR,
-		tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store);
+static DEVICE_ATTR_RW(in_proximity0_thresh_period);
 
 /* Use the default register values to identify the Taos device */
 static int tsl2x7x_device_id(unsigned char *id, int target)
-- 
1.9.1

Re: [PATCH v3] staging: iio: light: Replace symbolic permissions as per coding style

[Jonathan Cameron]

On 19/05/17 10:37, surenderpolsani@gmail.com wrote:
> From: Surender Polsani <surenderpolsani@gmail.com>
> 
> Fixed the following checkpatch.pl warnings:
> octal permissions are more preferable than symbolic permissions
> 
> Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
> as suggested by Greg K-H. Changed attributes and function
> names where ever required to satisfy internal macro definitions
> like __ATTR__RW().
> 
> Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
Nicely presented patch, but it runs into the fact that some of these
shouldn't exist as hand rolled attrs in the first place.

Some of theses should be handled through the various info_mask and
event_info_mask bitmaps + read_raw etc.

This would be a much less mechanical change however...

See inline and I'll try and pick out which ones.

Brian is working on this driver as well at the moment so there may
well be some clashes.

Perhaps you two could confer on who will target what?  Saves
everyone time to work together!

Jonathan

p.s. Brian if you'd prefer I just apply this and you rebase as
appropriate that's fine with me as well...
> ---
> Changes for v3:
> 
> - Reverted some changes from previous version and made
>    changes as suggested by Greg K-H.
> 
> Changes for v2:
> 
> - Made few changes as suggested by Greg K-H and modified
>    description accordingly.
> ---
>   drivers/staging/iio/light/tsl2x7x_core.c | 62 ++++++++++++++------------------
>   1 file changed, 26 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x_core.c b/drivers/staging/iio/light/tsl2x7x_core.c
> index af3910b..4de1408 100644
> --- a/drivers/staging/iio/light/tsl2x7x_core.c
> +++ b/drivers/staging/iio/light/tsl2x7x_core.c
> @@ -919,7 +919,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>   		tsl2x7x_chip_on(indio_dev);
>   }
>   
> -static ssize_t tsl2x7x_power_state_show(struct device *dev,
> +static ssize_t power_state_show(struct device *dev,
>   					struct device_attribute *attr,
>   					char *buf)
>   {
> @@ -928,7 +928,7 @@ static ssize_t tsl2x7x_power_state_show(struct device *dev,
>   	return snprintf(buf, PAGE_SIZE, "%d\n", chip->tsl2x7x_chip_status);
>   }
>   
> -static ssize_t tsl2x7x_power_state_store(struct device *dev,
> +static ssize_t power_state_store(struct device *dev,
>   					 struct device_attribute *attr,
>   					 const char *buf, size_t len)
>   {
> @@ -946,7 +946,7 @@ static ssize_t tsl2x7x_power_state_store(struct device *dev,
>   	return len;
>   }
>   
> -static ssize_t tsl2x7x_gain_available_show(struct device *dev,
> +static ssize_t in_illuminance0_calibscale_available_show(struct device *dev,
>   					   struct device_attribute *attr,
>   					   char *buf)
>   {
> @@ -964,14 +964,14 @@ static ssize_t tsl2x7x_gain_available_show(struct device *dev,
>   	return snprintf(buf, PAGE_SIZE, "%s\n", "1 8 16 120");
>   }
>   
> -static ssize_t tsl2x7x_prox_gain_available_show(struct device *dev,
> +static ssize_t in_proximity0_calibscale_available_show(struct device *dev,
>   						struct device_attribute *attr,
>   						char *buf)
>   {
>   		return snprintf(buf, PAGE_SIZE, "%s\n", "1 2 4 8");
>   }
>   
> -static ssize_t tsl2x7x_als_time_show(struct device *dev,
> +static ssize_t in_illuminance0_integration_time_show(struct device *dev,
>   				     struct device_attribute *attr,
>   				     char *buf)
>   {
> @@ -986,7 +986,7 @@ static ssize_t tsl2x7x_als_time_show(struct device *dev,
>   	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
>   }
>   
> -static ssize_t tsl2x7x_als_time_store(struct device *dev,
> +static ssize_t in_illuminance0_integration_time_store(struct device *dev,
>   				      struct device_attribute *attr,
>   				      const char *buf, size_t len)
>   {
> @@ -1014,7 +1014,7 @@ static ssize_t tsl2x7x_als_time_store(struct device *dev,
>   static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
>   		".00272 - .696");
>   
> -static ssize_t tsl2x7x_als_cal_target_show(struct device *dev,
> +static ssize_t in_illuminance0_target_input_show(struct device *dev,
>   					   struct device_attribute *attr,
>   					   char *buf)
>   {
> @@ -1024,7 +1024,7 @@ static ssize_t tsl2x7x_als_cal_target_show(struct device *dev,
>   			chip->tsl2x7x_settings.als_cal_target);
>   }
>   
> -static ssize_t tsl2x7x_als_cal_target_store(struct device *dev,
> +static ssize_t in_illuminance0_target_input_store(struct device *dev,
>   					    struct device_attribute *attr,
>   					    const char *buf, size_t len)
>   {
> @@ -1044,7 +1044,7 @@ static ssize_t tsl2x7x_als_cal_target_store(struct device *dev,
>   }
>   
>   /* persistence settings */
> -static ssize_t tsl2x7x_als_persistence_show(struct device *dev,
> +static ssize_t in_intensity0_thresh_period_show(struct device *dev,
>   					    struct device_attribute *attr,
>   					    char *buf)
>   {
> @@ -1061,7 +1061,7 @@ static ssize_t tsl2x7x_als_persistence_show(struct device *dev,
>   	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
>   }
>   
> -static ssize_t tsl2x7x_als_persistence_store(struct device *dev,
> +static ssize_t in_intensity0_thresh_period_store(struct device *dev,
>   					     struct device_attribute *attr,
>   					     const char *buf, size_t len)
>   {
> @@ -1092,7 +1092,7 @@ static ssize_t tsl2x7x_als_persistence_store(struct device *dev,
>   	return IIO_VAL_INT_PLUS_MICRO;
>   }
>   
> -static ssize_t tsl2x7x_prox_persistence_show(struct device *dev,
> +static ssize_t in_proximity0_thresh_period_show(struct device *dev,
>   					     struct device_attribute *attr,
>   					     char *buf)
>   {
> @@ -1109,7 +1109,7 @@ static ssize_t tsl2x7x_prox_persistence_show(struct device *dev,
>   	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
>   }
>   
> -static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
> +static ssize_t in_proximity0_thresh_period_store(struct device *dev,
>   					      struct device_attribute *attr,
>   					      const char *buf, size_t len)
>   {
> @@ -1140,7 +1140,7 @@ static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
>   	return IIO_VAL_INT_PLUS_MICRO;
>   }
>   
> -static ssize_t tsl2x7x_do_calibrate(struct device *dev,
> +static ssize_t in_illuminance0_calibrate_store(struct device *dev,
>   				    struct device_attribute *attr,
>   				    const char *buf, size_t len)
>   {
> @@ -1158,7 +1158,7 @@ static ssize_t tsl2x7x_do_calibrate(struct device *dev,
>   	return len;
>   }
>   
> -static ssize_t tsl2x7x_luxtable_show(struct device *dev,
> +static ssize_t in_illuminance0_lux_table_show(struct device *dev,
>   				     struct device_attribute *attr,
>   				     char *buf)
>   {
> @@ -1186,7 +1186,7 @@ static ssize_t tsl2x7x_luxtable_show(struct device *dev,
>   	return offset;
>   }
>   
> -static ssize_t tsl2x7x_luxtable_store(struct device *dev,
> +static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>   				      struct device_attribute *attr,
>   				      const char *buf, size_t len)
>   {
> @@ -1226,7 +1226,7 @@ static ssize_t tsl2x7x_luxtable_store(struct device *dev,
>   	return len;
>   }
>   
> -static ssize_t tsl2x7x_do_prox_calibrate(struct device *dev,
> +static ssize_t in_proximity0_calibrate_store(struct device *dev,
>   					 struct device_attribute *attr,
>   					 const char *buf, size_t len)
>   {
> @@ -1498,35 +1498,25 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>   	return 0;
>   }
>   
> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> -		tsl2x7x_power_state_show, tsl2x7x_power_state_store);
> +static DEVICE_ATTR_RW(power_state);
>   
> -static DEVICE_ATTR(in_proximity0_calibscale_available, S_IRUGO,
> -		tsl2x7x_prox_gain_available_show, NULL);
> +static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
>   
> -static DEVICE_ATTR(in_illuminance0_calibscale_available, S_IRUGO,
> -		tsl2x7x_gain_available_show, NULL);
> +static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
There is core support for these, but as it's not well documented now
fair enough to do it like this for now...
>   
> -static DEVICE_ATTR(in_illuminance0_integration_time, S_IRUGO | S_IWUSR,
> -		tsl2x7x_als_time_show, tsl2x7x_als_time_store);
> +static DEVICE_ATTR_RW(in_illuminance0_integration_time);
Should be iio_chan_spec mask_separate as  BIT(IIO_CHAN_INFO_INT_TIME)
and support added to read_raw and write_raw callbacks of iio_info
structure.
>   
> -static DEVICE_ATTR(in_illuminance0_target_input, S_IRUGO | S_IWUSR,
> -		tsl2x7x_als_cal_target_show, tsl2x7x_als_cal_target_store);
> +static DEVICE_ATTR_RW(in_illuminance0_target_input);
>   
> -static DEVICE_ATTR(in_illuminance0_calibrate, S_IWUSR, NULL,
> -		tsl2x7x_do_calibrate);
> +static DEVICE_ATTR_WO(in_illuminance0_calibrate);
>   
> -static DEVICE_ATTR(in_proximity0_calibrate, S_IWUSR, NULL,
> -		tsl2x7x_do_prox_calibrate);
> +static DEVICE_ATTR_WO(in_proximity0_calibrate);
>   
> -static DEVICE_ATTR(in_illuminance0_lux_table, S_IRUGO | S_IWUSR,
> -		tsl2x7x_luxtable_show, tsl2x7x_luxtable_store);
> +static DEVICE_ATTR_RW(in_illuminance0_lux_table);
>   
> -static DEVICE_ATTR(in_intensity0_thresh_period, S_IRUGO | S_IWUSR,
> -		tsl2x7x_als_persistence_show, tsl2x7x_als_persistence_store);
> +static DEVICE_ATTR_RW(in_intensity0_thresh_period);
As below.
>   
> -static DEVICE_ATTR(in_proximity0_thresh_period, S_IRUGO | S_IWUSR,
> -		tsl2x7x_prox_persistence_show, tsl2x7x_prox_persistence_store);
> +static DEVICE_ATTR_RW(in_proximity0_thresh_period);
Should be in the iio_event_spec mask_separate as BIT(IIO_EV_INFO_PERIOD) and
add the relevant reading and writing code to tsl2x7x_read_thresh
(which would then be obviously misnamed and need renaming)
>   
>   /* Use the default register values to identify the Taos device */
>   static int tsl2x7x_device_id(unsigned char *id, int target)
> 

Re: [PATCH v3] staging: iio: light: Replace symbolic permissions as per coding style

[Brian Masney]

On Sat, May 20, 2017 at 06:55:02PM +0100, Jonathan Cameron wrote:
> On 19/05/17 10:37, surenderpolsani@gmail.com wrote:
> >From: Surender Polsani <surenderpolsani@gmail.com>
> >
> >Fixed the following checkpatch.pl warnings:
> >octal permissions are more preferable than symbolic permissions
> >
> >Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
> >as suggested by Greg K-H. Changed attributes and function
> >names where ever required to satisfy internal macro definitions
> >like __ATTR__RW().
> >
> >Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
> Nicely presented patch, but it runs into the fact that some of these
> shouldn't exist as hand rolled attrs in the first place.
> 
> Some of theses should be handled through the various info_mask and
> event_info_mask bitmaps + read_raw etc.
> 
> This would be a much less mechanical change however...
> 
> See inline and I'll try and pick out which ones.
> 
> Brian is working on this driver as well at the moment so there may
> well be some clashes.
> 
> Perhaps you two could confer on who will target what?  Saves
> everyone time to work together!

You can apply this if you'd like. I just got 7 different hardware
samples from Jon this week that are supported by this driver. I haven't
gotten far yet with coding since I'm currently working on getting
them all setup so that it will be easy for me to test my upcoming
driver changes.

For my first patch series, I'm planning to migrate the driver to use
IIO channels, cleaning up the I2C calls, and runtime power management
support. Hopefully I'll have this to you for next weekend.

Brian

Re: [PATCH v3] staging: iio: light: Replace symbolic permissions as per coding style

[Jonathan Cameron]

On 20/05/17 19:36, Brian Masney wrote:
> On Sat, May 20, 2017 at 06:55:02PM +0100, Jonathan Cameron wrote:
>> On 19/05/17 10:37, surenderpolsani@gmail.com wrote:
>>> From: Surender Polsani <surenderpolsani@gmail.com>
>>>
>>> Fixed the following checkpatch.pl warnings:
>>> octal permissions are more preferable than symbolic permissions
>>>
>>> Replaced DEVICE_ATTR family macros with DEVICE_ATTR_RW family
>>> as suggested by Greg K-H. Changed attributes and function
>>> names where ever required to satisfy internal macro definitions
>>> like __ATTR__RW().
>>>
>>> Signed-off-by: Surender Polsani <surenderpolsani@gmail.com>
>> Nicely presented patch, but it runs into the fact that some of these
>> shouldn't exist as hand rolled attrs in the first place.
>>
>> Some of theses should be handled through the various info_mask and
>> event_info_mask bitmaps + read_raw etc.
>>
>> This would be a much less mechanical change however...
>>
>> See inline and I'll try and pick out which ones.
>>
>> Brian is working on this driver as well at the moment so there may
>> well be some clashes.
>>
>> Perhaps you two could confer on who will target what?  Saves
>> everyone time to work together!
> 
> You can apply this if you'd like. I just got 7 different hardware
> samples from Jon this week that are supported by this driver. I haven't
> gotten far yet with coding since I'm currently working on getting
> them all setup so that it will be easy for me to test my upcoming
> driver changes.
> 
> For my first patch series, I'm planning to migrate the driver to use
> IIO channels, cleaning up the I2C calls, and runtime power management
> support. Hopefully I'll have this to you for next weekend.
> 
I've applied this patch (with a lot of fuzz as the driver had changed
under it).

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to see what we messed up.

Jonathan
> Brian
> --
> 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
>