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

Groups > linux.kernel > #1546978 > unrolled thread

[RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup

Started byAlexander Koch <mail@alexanderkoch.net>
First post2016-12-23 23:20 +0100
Last post2016-12-30 02:00 +0100
Articles 6 — 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

  [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup Alexander Koch <mail@alexanderkoch.net> - 2016-12-23 23:20 +0100
    Re: [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup Guenter Roeck <linux@roeck-us.net> - 2016-12-26 11:50 +0100
      Re: [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup Alexander Koch <mail@alexanderkoch.net> - 2016-12-29 19:30 +0100
        Re: [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup Guenter Roeck <linux@roeck-us.net> - 2016-12-29 20:50 +0100
          Re: [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup Alexander Koch <mail@alexanderkoch.net> - 2016-12-29 21:40 +0100
            Re: [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup Guenter Roeck <linux@roeck-us.net> - 2016-12-30 02:00 +0100

#1546978 — [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup

FromAlexander Koch <mail@alexanderkoch.net>
Date2016-12-23 23:20 +0100
Subject[RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup
Message-ID<sRGlX-4lT-5@gated-at.bofh.it>
Replace sysfs symbolic file permissions, e.g. 'S_IRUGO', by octal
permissions. This fixes checkpatch.pl warnings.

Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
---
 drivers/hwmon/adc128d818.c | 99 ++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 60 deletions(-)

diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c
index 8667f454ea11..cbb3bc5e5229 100644
--- a/drivers/hwmon/adc128d818.c
+++ b/drivers/hwmon/adc128d818.c
@@ -242,69 +242,48 @@ static ssize_t adc128_show_alarm(struct device *dev,
 	return sprintf(buf, "%u\n", !!(alarms & mask));
 }
 
-static SENSOR_DEVICE_ATTR_2(in0_input, S_IRUGO,
-			    adc128_show_in, NULL, 0, 0);
-static SENSOR_DEVICE_ATTR_2(in0_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 0, 1);
-static SENSOR_DEVICE_ATTR_2(in0_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 0, 2);
-
-static SENSOR_DEVICE_ATTR_2(in1_input, S_IRUGO,
-			    adc128_show_in, NULL, 1, 0);
-static SENSOR_DEVICE_ATTR_2(in1_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 1, 1);
-static SENSOR_DEVICE_ATTR_2(in1_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 1, 2);
-
-static SENSOR_DEVICE_ATTR_2(in2_input, S_IRUGO,
-			    adc128_show_in, NULL, 2, 0);
-static SENSOR_DEVICE_ATTR_2(in2_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 2, 1);
-static SENSOR_DEVICE_ATTR_2(in2_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 2, 2);
-
-static SENSOR_DEVICE_ATTR_2(in3_input, S_IRUGO,
-			    adc128_show_in, NULL, 3, 0);
-static SENSOR_DEVICE_ATTR_2(in3_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 3, 1);
-static SENSOR_DEVICE_ATTR_2(in3_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 3, 2);
-
-static SENSOR_DEVICE_ATTR_2(in4_input, S_IRUGO,
-			    adc128_show_in, NULL, 4, 0);
-static SENSOR_DEVICE_ATTR_2(in4_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 4, 1);
-static SENSOR_DEVICE_ATTR_2(in4_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 4, 2);
-
-static SENSOR_DEVICE_ATTR_2(in5_input, S_IRUGO,
-			    adc128_show_in, NULL, 5, 0);
-static SENSOR_DEVICE_ATTR_2(in5_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 5, 1);
-static SENSOR_DEVICE_ATTR_2(in5_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 5, 2);
-
-static SENSOR_DEVICE_ATTR_2(in6_input, S_IRUGO,
-			    adc128_show_in, NULL, 6, 0);
-static SENSOR_DEVICE_ATTR_2(in6_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 6, 1);
-static SENSOR_DEVICE_ATTR_2(in6_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 6, 2);
-
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, adc128_show_temp, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
+static SENSOR_DEVICE_ATTR_2(in0_input, 0444, adc128_show_in, NULL, 0, 0);
+static SENSOR_DEVICE_ATTR_2(in0_min, 0644, adc128_show_in, adc128_set_in, 0, 1);
+static SENSOR_DEVICE_ATTR_2(in0_max, 0644, adc128_show_in, adc128_set_in, 0, 2);
+
+static SENSOR_DEVICE_ATTR_2(in1_input, 0444, adc128_show_in, NULL, 1, 0);
+static SENSOR_DEVICE_ATTR_2(in1_min, 0644, adc128_show_in, adc128_set_in, 1, 1);
+static SENSOR_DEVICE_ATTR_2(in1_max, 0644, adc128_show_in, adc128_set_in, 1, 2);
+
+static SENSOR_DEVICE_ATTR_2(in2_input, 0444, adc128_show_in, NULL, 2, 0);
+static SENSOR_DEVICE_ATTR_2(in2_min, 0644, adc128_show_in, adc128_set_in, 2, 1);
+static SENSOR_DEVICE_ATTR_2(in2_max, 0644, adc128_show_in, adc128_set_in, 2, 2);
+
+static SENSOR_DEVICE_ATTR_2(in3_input, 0444, adc128_show_in, NULL, 3, 0);
+static SENSOR_DEVICE_ATTR_2(in3_min, 0644, adc128_show_in, adc128_set_in, 3, 1);
+static SENSOR_DEVICE_ATTR_2(in3_max, 0644, adc128_show_in, adc128_set_in, 3, 2);
+
+static SENSOR_DEVICE_ATTR_2(in4_input, 0444, adc128_show_in, NULL, 4, 0);
+static SENSOR_DEVICE_ATTR_2(in4_min, 0644, adc128_show_in, adc128_set_in, 4, 1);
+static SENSOR_DEVICE_ATTR_2(in4_max, 0644, adc128_show_in, adc128_set_in, 4, 2);
+
+static SENSOR_DEVICE_ATTR_2(in5_input, 0444, adc128_show_in, NULL, 5, 0);
+static SENSOR_DEVICE_ATTR_2(in5_min, 0644, adc128_show_in, adc128_set_in, 5, 1);
+static SENSOR_DEVICE_ATTR_2(in5_max, 0644, adc128_show_in, adc128_set_in, 5, 2);
+
+static SENSOR_DEVICE_ATTR_2(in6_input, 0444, adc128_show_in, NULL, 6, 0);
+static SENSOR_DEVICE_ATTR_2(in6_min, 0644, adc128_show_in, adc128_set_in, 6, 1);
+static SENSOR_DEVICE_ATTR_2(in6_max, 0644, adc128_show_in, adc128_set_in, 6, 2);
+
+static SENSOR_DEVICE_ATTR(temp1_input, 0444, adc128_show_temp, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_max, 0644,
 			  adc128_show_temp, adc128_set_temp, 1);
-static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp1_max_hyst, 0644,
 			  adc128_show_temp, adc128_set_temp, 2);
 
-static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, adc128_show_alarm, NULL, 0);
-static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, adc128_show_alarm, NULL, 1);
-static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, adc128_show_alarm, NULL, 2);
-static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, adc128_show_alarm, NULL, 3);
-static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, adc128_show_alarm, NULL, 4);
-static SENSOR_DEVICE_ATTR(in5_alarm, S_IRUGO, adc128_show_alarm, NULL, 5);
-static SENSOR_DEVICE_ATTR(in6_alarm, S_IRUGO, adc128_show_alarm, NULL, 6);
-static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adc128_show_alarm, NULL, 7);
+static SENSOR_DEVICE_ATTR(in0_alarm, 0444, adc128_show_alarm, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_alarm, 0444, adc128_show_alarm, NULL, 1);
+static SENSOR_DEVICE_ATTR(in2_alarm, 0444, adc128_show_alarm, NULL, 2);
+static SENSOR_DEVICE_ATTR(in3_alarm, 0444, adc128_show_alarm, NULL, 3);
+static SENSOR_DEVICE_ATTR(in4_alarm, 0444, adc128_show_alarm, NULL, 4);
+static SENSOR_DEVICE_ATTR(in5_alarm, 0444, adc128_show_alarm, NULL, 5);
+static SENSOR_DEVICE_ATTR(in6_alarm, 0444, adc128_show_alarm, NULL, 6);
+static SENSOR_DEVICE_ATTR(temp1_max_alarm, 0444, adc128_show_alarm, NULL, 7);
 
 static struct attribute *adc128_attrs[] = {
 	&sensor_dev_attr_in0_min.dev_attr.attr,
-- 
2.11.0

[toc] | [next] | [standalone]

#1547301

FromGuenter Roeck <linux@roeck-us.net>
Date2016-12-26 11:50 +0100
Message-ID<sSB0S-3Fv-7@gated-at.bofh.it>
In reply to#1546978
On 12/23/2016 02:12 PM, Alexander Koch wrote:
> Replace sysfs symbolic file permissions, e.g. 'S_IRUGO', by octal
> permissions. This fixes checkpatch.pl warnings.
>
> Signed-off-by: Alexander Koch <mail@alexanderkoch.net>

Please do not bother with those warnings and ignore checkpatch.
We are in the process of doing an automated conversion.

Thanks,
Guenter

> ---
>  drivers/hwmon/adc128d818.c | 99 ++++++++++++++++++----------------------------
>  1 file changed, 39 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c
> index 8667f454ea11..cbb3bc5e5229 100644
> --- a/drivers/hwmon/adc128d818.c
> +++ b/drivers/hwmon/adc128d818.c
> @@ -242,69 +242,48 @@ static ssize_t adc128_show_alarm(struct device *dev,
>  	return sprintf(buf, "%u\n", !!(alarms & mask));
>  }
>
> -static SENSOR_DEVICE_ATTR_2(in0_input, S_IRUGO,
> -			    adc128_show_in, NULL, 0, 0);
> -static SENSOR_DEVICE_ATTR_2(in0_min, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 0, 1);
> -static SENSOR_DEVICE_ATTR_2(in0_max, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 0, 2);
> -
> -static SENSOR_DEVICE_ATTR_2(in1_input, S_IRUGO,
> -			    adc128_show_in, NULL, 1, 0);
> -static SENSOR_DEVICE_ATTR_2(in1_min, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 1, 1);
> -static SENSOR_DEVICE_ATTR_2(in1_max, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 1, 2);
> -
> -static SENSOR_DEVICE_ATTR_2(in2_input, S_IRUGO,
> -			    adc128_show_in, NULL, 2, 0);
> -static SENSOR_DEVICE_ATTR_2(in2_min, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 2, 1);
> -static SENSOR_DEVICE_ATTR_2(in2_max, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 2, 2);
> -
> -static SENSOR_DEVICE_ATTR_2(in3_input, S_IRUGO,
> -			    adc128_show_in, NULL, 3, 0);
> -static SENSOR_DEVICE_ATTR_2(in3_min, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 3, 1);
> -static SENSOR_DEVICE_ATTR_2(in3_max, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 3, 2);
> -
> -static SENSOR_DEVICE_ATTR_2(in4_input, S_IRUGO,
> -			    adc128_show_in, NULL, 4, 0);
> -static SENSOR_DEVICE_ATTR_2(in4_min, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 4, 1);
> -static SENSOR_DEVICE_ATTR_2(in4_max, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 4, 2);
> -
> -static SENSOR_DEVICE_ATTR_2(in5_input, S_IRUGO,
> -			    adc128_show_in, NULL, 5, 0);
> -static SENSOR_DEVICE_ATTR_2(in5_min, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 5, 1);
> -static SENSOR_DEVICE_ATTR_2(in5_max, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 5, 2);
> -
> -static SENSOR_DEVICE_ATTR_2(in6_input, S_IRUGO,
> -			    adc128_show_in, NULL, 6, 0);
> -static SENSOR_DEVICE_ATTR_2(in6_min, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 6, 1);
> -static SENSOR_DEVICE_ATTR_2(in6_max, S_IWUSR | S_IRUGO,
> -			    adc128_show_in, adc128_set_in, 6, 2);
> -
> -static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, adc128_show_temp, NULL, 0);
> -static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
> +static SENSOR_DEVICE_ATTR_2(in0_input, 0444, adc128_show_in, NULL, 0, 0);
> +static SENSOR_DEVICE_ATTR_2(in0_min, 0644, adc128_show_in, adc128_set_in, 0, 1);
> +static SENSOR_DEVICE_ATTR_2(in0_max, 0644, adc128_show_in, adc128_set_in, 0, 2);
> +
> +static SENSOR_DEVICE_ATTR_2(in1_input, 0444, adc128_show_in, NULL, 1, 0);
> +static SENSOR_DEVICE_ATTR_2(in1_min, 0644, adc128_show_in, adc128_set_in, 1, 1);
> +static SENSOR_DEVICE_ATTR_2(in1_max, 0644, adc128_show_in, adc128_set_in, 1, 2);
> +
> +static SENSOR_DEVICE_ATTR_2(in2_input, 0444, adc128_show_in, NULL, 2, 0);
> +static SENSOR_DEVICE_ATTR_2(in2_min, 0644, adc128_show_in, adc128_set_in, 2, 1);
> +static SENSOR_DEVICE_ATTR_2(in2_max, 0644, adc128_show_in, adc128_set_in, 2, 2);
> +
> +static SENSOR_DEVICE_ATTR_2(in3_input, 0444, adc128_show_in, NULL, 3, 0);
> +static SENSOR_DEVICE_ATTR_2(in3_min, 0644, adc128_show_in, adc128_set_in, 3, 1);
> +static SENSOR_DEVICE_ATTR_2(in3_max, 0644, adc128_show_in, adc128_set_in, 3, 2);
> +
> +static SENSOR_DEVICE_ATTR_2(in4_input, 0444, adc128_show_in, NULL, 4, 0);
> +static SENSOR_DEVICE_ATTR_2(in4_min, 0644, adc128_show_in, adc128_set_in, 4, 1);
> +static SENSOR_DEVICE_ATTR_2(in4_max, 0644, adc128_show_in, adc128_set_in, 4, 2);
> +
> +static SENSOR_DEVICE_ATTR_2(in5_input, 0444, adc128_show_in, NULL, 5, 0);
> +static SENSOR_DEVICE_ATTR_2(in5_min, 0644, adc128_show_in, adc128_set_in, 5, 1);
> +static SENSOR_DEVICE_ATTR_2(in5_max, 0644, adc128_show_in, adc128_set_in, 5, 2);
> +
> +static SENSOR_DEVICE_ATTR_2(in6_input, 0444, adc128_show_in, NULL, 6, 0);
> +static SENSOR_DEVICE_ATTR_2(in6_min, 0644, adc128_show_in, adc128_set_in, 6, 1);
> +static SENSOR_DEVICE_ATTR_2(in6_max, 0644, adc128_show_in, adc128_set_in, 6, 2);
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, 0444, adc128_show_temp, NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp1_max, 0644,
>  			  adc128_show_temp, adc128_set_temp, 1);
> -static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,
> +static SENSOR_DEVICE_ATTR(temp1_max_hyst, 0644,
>  			  adc128_show_temp, adc128_set_temp, 2);
>
> -static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, adc128_show_alarm, NULL, 0);
> -static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, adc128_show_alarm, NULL, 1);
> -static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, adc128_show_alarm, NULL, 2);
> -static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, adc128_show_alarm, NULL, 3);
> -static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, adc128_show_alarm, NULL, 4);
> -static SENSOR_DEVICE_ATTR(in5_alarm, S_IRUGO, adc128_show_alarm, NULL, 5);
> -static SENSOR_DEVICE_ATTR(in6_alarm, S_IRUGO, adc128_show_alarm, NULL, 6);
> -static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adc128_show_alarm, NULL, 7);
> +static SENSOR_DEVICE_ATTR(in0_alarm, 0444, adc128_show_alarm, NULL, 0);
> +static SENSOR_DEVICE_ATTR(in1_alarm, 0444, adc128_show_alarm, NULL, 1);
> +static SENSOR_DEVICE_ATTR(in2_alarm, 0444, adc128_show_alarm, NULL, 2);
> +static SENSOR_DEVICE_ATTR(in3_alarm, 0444, adc128_show_alarm, NULL, 3);
> +static SENSOR_DEVICE_ATTR(in4_alarm, 0444, adc128_show_alarm, NULL, 4);
> +static SENSOR_DEVICE_ATTR(in5_alarm, 0444, adc128_show_alarm, NULL, 5);
> +static SENSOR_DEVICE_ATTR(in6_alarm, 0444, adc128_show_alarm, NULL, 6);
> +static SENSOR_DEVICE_ATTR(temp1_max_alarm, 0444, adc128_show_alarm, NULL, 7);
>
>  static struct attribute *adc128_attrs[] = {
>  	&sensor_dev_attr_in0_min.dev_attr.attr,
>

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

#1548340

FromAlexander Koch <mail@alexanderkoch.net>
Date2016-12-29 19:30 +0100
Message-ID<sTNCF-1FX-5@gated-at.bofh.it>
In reply to#1547301
On 12/26/2016 11:47 AM, Guenter Roeck wrote:
> On 12/23/2016 02:12 PM, Alexander Koch wrote:
>> Replace sysfs symbolic file permissions, e.g. 'S_IRUGO', by octal
>> permissions. This fixes checkpatch.pl warnings.
>>
>> Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
>
> Please do not bother with those warnings and ignore checkpatch.
> We are in the process of doing an automated conversion.

Okay, so I shall create v3 of the patchset, without these changes. I've
found a typo in the 4th patch so I thought about going v3 anyways.

I've tested operation modes 1-3 on real hardware today and found no
issues so far, so I hope v3 will have good chances of getting accepted.


Cheers,

Alex

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

#1548352

FromGuenter Roeck <linux@roeck-us.net>
Date2016-12-29 20:50 +0100
Message-ID<sTOS5-2nW-1@gated-at.bofh.it>
In reply to#1548340
On Thu, Dec 29, 2016 at 07:22:12PM +0100, Alexander Koch wrote:
> On 12/26/2016 11:47 AM, Guenter Roeck wrote:
> > On 12/23/2016 02:12 PM, Alexander Koch wrote:
> >> Replace sysfs symbolic file permissions, e.g. 'S_IRUGO', by octal
> >> permissions. This fixes checkpatch.pl warnings.
> >>
> >> Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
> >
> > Please do not bother with those warnings and ignore checkpatch.
> > We are in the process of doing an automated conversion.
> 
> Okay, so I shall create v3 of the patchset, without these changes. I've
> found a typo in the 4th patch so I thought about going v3 anyways.
> 
> I've tested operation modes 1-3 on real hardware today and found no
> issues so far, so I hope v3 will have good chances of getting accepted.
> 
Pretty much. One request, though: If there is no configuration data
from DT, I would like the driver to read the mode from the chip - if for
nothing else, this will let me test all modes, but it also supports the
case where the chip is configured by BIOS/ROMMON.

Thanks,
Guenter

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

#1548361

FromAlexander Koch <mail@alexanderkoch.net>
Date2016-12-29 21:40 +0100
Message-ID<sTPEt-2W1-3@gated-at.bofh.it>
In reply to#1548352
On 12/29/2016 08:46 PM, Guenter Roeck wrote:
> On Thu, Dec 29, 2016 at 07:22:12PM +0100, Alexander Koch wrote:
>> On 12/26/2016 11:47 AM, Guenter Roeck wrote:
>>> On 12/23/2016 02:12 PM, Alexander Koch wrote:
>>>> Replace sysfs symbolic file permissions, e.g. 'S_IRUGO', by octal
>>>> permissions. This fixes checkpatch.pl warnings.
>>>>
>>>> Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
>>> Please do not bother with those warnings and ignore checkpatch.
>>> We are in the process of doing an automated conversion.
>> Okay, so I shall create v3 of the patchset, without these changes. I've
>> found a typo in the 4th patch so I thought about going v3 anyways.
>>
>> I've tested operation modes 1-3 on real hardware today and found no
>> issues so far, so I hope v3 will have good chances of getting accepted.
>>
> Pretty much. One request, though: If there is no configuration data
> from DT, I would like the driver to read the mode from the chip - if for
> nothing else, this will let me test all modes, but it also supports the
> case where the chip is configured by BIOS/ROMMON.

Aye, will add this as new fourth patch then. I assume the chip reset in
adc128_init_client() clears the chip mode as well, so I will read it in
the operation mode block in adc128_probe().

Just out of interest: how does this help you test the modes? Do you
configure the chip externally and test it on a platform without
devicetree support?


Cheers,

Alex

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

#1548419

FromGuenter Roeck <linux@roeck-us.net>
Date2016-12-30 02:00 +0100
Message-ID<sTTI5-5BR-3@gated-at.bofh.it>
In reply to#1548361
On 12/29/2016 12:30 PM, Alexander Koch wrote:
> On 12/29/2016 08:46 PM, Guenter Roeck wrote:
>> On Thu, Dec 29, 2016 at 07:22:12PM +0100, Alexander Koch wrote:
>>> On 12/26/2016 11:47 AM, Guenter Roeck wrote:
>>>> On 12/23/2016 02:12 PM, Alexander Koch wrote:
>>>>> Replace sysfs symbolic file permissions, e.g. 'S_IRUGO', by octal
>>>>> permissions. This fixes checkpatch.pl warnings.
>>>>>
>>>>> Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
>>>> Please do not bother with those warnings and ignore checkpatch.
>>>> We are in the process of doing an automated conversion.
>>> Okay, so I shall create v3 of the patchset, without these changes. I've
>>> found a typo in the 4th patch so I thought about going v3 anyways.
>>>
>>> I've tested operation modes 1-3 on real hardware today and found no
>>> issues so far, so I hope v3 will have good chances of getting accepted.
>>>
>> Pretty much. One request, though: If there is no configuration data
>> from DT, I would like the driver to read the mode from the chip - if for
>> nothing else, this will let me test all modes, but it also supports the
>> case where the chip is configured by BIOS/ROMMON.
>
> Aye, will add this as new fourth patch then. I assume the chip reset in
> adc128_init_client() clears the chip mode as well, so I will read it in
> the operation mode block in adc128_probe().
>
> Just out of interest: how does this help you test the modes? Do you
> configure the chip externally and test it on a platform without
> devicetree support?
>

I use the i2c test driver in the kernel and a module test script, on x86.
See https://github.com/groeck/module-tests. That only works if the driver
does not overwrite the register values on probe.

Not really sure if resetting the chip during probe is such a good idea
in the first place. It is quite unusual.

Guenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web