Message ID | 20161223221205.8825-4-mail@alexanderkoch.net (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
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, > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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(-)