[PATCH] Staging: iio: frequency: use octal permissions instead of symbolic

[PATCH] Staging: iio: frequency: use octal permissions instead of symbolic

[Miguel Robles]

Fix checkpatch warnings:
Symbolic permissions are not preferred. Consider using octal permissions.

Signed-off-by: Miguel Robles <miguel.robles@farole.net>
---
 drivers/staging/iio/frequency/ad9832.c | 20 ++++++++++----------
 drivers/staging/iio/frequency/ad9834.c | 22 +++++++++++-----------
 drivers/staging/iio/frequency/dds.h    |  2 +-
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 8d40c8e..163a4ef 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -248,22 +248,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
  * see dds.h for further information
  */
 
-static IIO_DEV_ATTR_FREQ(0, 0, S_IWUSR, NULL, ad9832_write, AD9832_FREQ0HM);
-static IIO_DEV_ATTR_FREQ(0, 1, S_IWUSR, NULL, ad9832_write, AD9832_FREQ1HM);
-static IIO_DEV_ATTR_FREQSYMBOL(0, S_IWUSR, NULL, ad9832_write, AD9832_FREQ_SYM);
+static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
+static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
+static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
 static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
 
-static IIO_DEV_ATTR_PHASE(0, 0, S_IWUSR, NULL, ad9832_write, AD9832_PHASE0H);
-static IIO_DEV_ATTR_PHASE(0, 1, S_IWUSR, NULL, ad9832_write, AD9832_PHASE1H);
-static IIO_DEV_ATTR_PHASE(0, 2, S_IWUSR, NULL, ad9832_write, AD9832_PHASE2H);
-static IIO_DEV_ATTR_PHASE(0, 3, S_IWUSR, NULL, ad9832_write, AD9832_PHASE3H);
-static IIO_DEV_ATTR_PHASESYMBOL(0, S_IWUSR, NULL,
+static IIO_DEV_ATTR_PHASE(0, 0, 0200, NULL, ad9832_write, AD9832_PHASE0H);
+static IIO_DEV_ATTR_PHASE(0, 1, 0200, NULL, ad9832_write, AD9832_PHASE1H);
+static IIO_DEV_ATTR_PHASE(0, 2, 0200, NULL, ad9832_write, AD9832_PHASE2H);
+static IIO_DEV_ATTR_PHASE(0, 3, 0200, NULL, ad9832_write, AD9832_PHASE3H);
+static IIO_DEV_ATTR_PHASESYMBOL(0, 0200, NULL,
 				ad9832_write, AD9832_PHASE_SYM);
 static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12 rad*/
 
-static IIO_DEV_ATTR_PINCONTROL_EN(0, S_IWUSR, NULL,
+static IIO_DEV_ATTR_PINCONTROL_EN(0, 0200, NULL,
 				ad9832_write, AD9832_PINCTRL_EN);
-static IIO_DEV_ATTR_OUT_ENABLE(0, S_IWUSR, NULL,
+static IIO_DEV_ATTR_OUT_ENABLE(0, 0200, NULL,
 				ad9832_write, AD9832_OUTPUT_EN);
 
 static struct attribute *ad9832_attributes[] = {
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index f92ff7f..a56d17d 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -291,7 +291,7 @@ ssize_t ad9834_show_out0_wavetype_available(struct device *dev,
 	return sprintf(buf, "%s\n", str);
 }
 
-static IIO_DEVICE_ATTR(out_altvoltage0_out0_wavetype_available, S_IRUGO,
+static IIO_DEVICE_ATTR(out_altvoltage0_out0_wavetype_available, 0444,
 		       ad9834_show_out0_wavetype_available, NULL, 0);
 
 static
@@ -311,27 +311,27 @@ ssize_t ad9834_show_out1_wavetype_available(struct device *dev,
 	return sprintf(buf, "%s\n", str);
 }
 
-static IIO_DEVICE_ATTR(out_altvoltage0_out1_wavetype_available, S_IRUGO,
+static IIO_DEVICE_ATTR(out_altvoltage0_out1_wavetype_available, 0444,
 		       ad9834_show_out1_wavetype_available, NULL, 0);
 
 /**
  * see dds.h for further information
  */
 
-static IIO_DEV_ATTR_FREQ(0, 0, S_IWUSR, NULL, ad9834_write, AD9834_REG_FREQ0);
-static IIO_DEV_ATTR_FREQ(0, 1, S_IWUSR, NULL, ad9834_write, AD9834_REG_FREQ1);
-static IIO_DEV_ATTR_FREQSYMBOL(0, S_IWUSR, NULL, ad9834_write, AD9834_FSEL);
+static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9834_write, AD9834_REG_FREQ0);
+static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9834_write, AD9834_REG_FREQ1);
+static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9834_write, AD9834_FSEL);
 static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
 
-static IIO_DEV_ATTR_PHASE(0, 0, S_IWUSR, NULL, ad9834_write, AD9834_REG_PHASE0);
-static IIO_DEV_ATTR_PHASE(0, 1, S_IWUSR, NULL, ad9834_write, AD9834_REG_PHASE1);
-static IIO_DEV_ATTR_PHASESYMBOL(0, S_IWUSR, NULL, ad9834_write, AD9834_PSEL);
+static IIO_DEV_ATTR_PHASE(0, 0, 0200, NULL, ad9834_write, AD9834_REG_PHASE0);
+static IIO_DEV_ATTR_PHASE(0, 1, 0200, NULL, ad9834_write, AD9834_REG_PHASE1);
+static IIO_DEV_ATTR_PHASESYMBOL(0, 0200, NULL, ad9834_write, AD9834_PSEL);
 static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12 rad*/
 
-static IIO_DEV_ATTR_PINCONTROL_EN(0, S_IWUSR, NULL,
+static IIO_DEV_ATTR_PINCONTROL_EN(0, 0200, NULL,
 	ad9834_write, AD9834_PIN_SW);
-static IIO_DEV_ATTR_OUT_ENABLE(0, S_IWUSR, NULL, ad9834_write, AD9834_RESET);
-static IIO_DEV_ATTR_OUTY_ENABLE(0, 1, S_IWUSR, NULL,
+static IIO_DEV_ATTR_OUT_ENABLE(0, 0200, NULL, ad9834_write, AD9834_RESET);
+static IIO_DEV_ATTR_OUTY_ENABLE(0, 1, 0200, NULL,
 	ad9834_write, AD9834_OPBITEN);
 static IIO_DEV_ATTR_OUT_WAVETYPE(0, 0, ad9834_store_wavetype, 0);
 static IIO_DEV_ATTR_OUT_WAVETYPE(0, 1, ad9834_store_wavetype, 1);
diff --git a/drivers/staging/iio/frequency/dds.h b/drivers/staging/iio/frequency/dds.h
index fe53e732..d6ccd99 100644
--- a/drivers/staging/iio/frequency/dds.h
+++ b/drivers/staging/iio/frequency/dds.h
@@ -101,7 +101,7 @@
 
 #define IIO_DEV_ATTR_OUT_WAVETYPE(_channel, _output, _store, _addr)	\
 	IIO_DEVICE_ATTR(out_altvoltage##_channel##_out##_output##_wavetype,\
-			S_IWUSR, NULL, _store, _addr)
+			0200, NULL, _store, _addr)
 
 /**
  * /sys/bus/iio/devices/.../out_altvoltageX_outY_wavetype_available
-- 
2.7.4

Re: [PATCH] Staging: iio: frequency: use octal permissions instead of symbolic

[Greg KH]

On Thu, Mar 16, 2017 at 02:47:36PM +0100, Miguel Robles wrote:
> Fix checkpatch warnings:
> Symbolic permissions are not preferred. Consider using octal permissions.
> 
> Signed-off-by: Miguel Robles <miguel.robles@farole.net>
> ---
>  drivers/staging/iio/frequency/ad9832.c | 20 ++++++++++----------
>  drivers/staging/iio/frequency/ad9834.c | 22 +++++++++++-----------
>  drivers/staging/iio/frequency/dds.h    |  2 +-
>  3 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 8d40c8e..163a4ef 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -248,22 +248,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
>   * see dds.h for further information
>   */
>  
> -static IIO_DEV_ATTR_FREQ(0, 0, S_IWUSR, NULL, ad9832_write, AD9832_FREQ0HM);
> -static IIO_DEV_ATTR_FREQ(0, 1, S_IWUSR, NULL, ad9832_write, AD9832_FREQ1HM);
> -static IIO_DEV_ATTR_FREQSYMBOL(0, S_IWUSR, NULL, ad9832_write, AD9832_FREQ_SYM);
> +static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
> +static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
> +static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);

Meta-comment, would a IIO_DEV_ATTR_WO_FREQ() and friends help out here?
That way it's impossible to get the permission values incorrect.

thanks,

greg k-h

Re: [PATCH] Staging: iio: frequency: use octal permissions instead of symbolic

[Miguel Robles]

On Thu, Mar 16, 2017 at 11:05:55PM +0900, Greg KH wrote:
> On Thu, Mar 16, 2017 at 02:47:36PM +0100, Miguel Robles wrote:
> > Fix checkpatch warnings:
> > Symbolic permissions are not preferred. Consider using octal permissions.
> > 
> > Signed-off-by: Miguel Robles <miguel.robles@farole.net>
> > ---
> >  drivers/staging/iio/frequency/ad9832.c | 20 ++++++++++----------
> >  drivers/staging/iio/frequency/ad9834.c | 22 +++++++++++-----------
> >  drivers/staging/iio/frequency/dds.h    |  2 +-
> >  3 files changed, 22 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> > index 8d40c8e..163a4ef 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -248,22 +248,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
> >   * see dds.h for further information
> >   */
> >  
> > -static IIO_DEV_ATTR_FREQ(0, 0, S_IWUSR, NULL, ad9832_write, AD9832_FREQ0HM);
> > -static IIO_DEV_ATTR_FREQ(0, 1, S_IWUSR, NULL, ad9832_write, AD9832_FREQ1HM);
> > -static IIO_DEV_ATTR_FREQSYMBOL(0, S_IWUSR, NULL, ad9832_write, AD9832_FREQ_SYM);
> > +static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
> > +static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
> > +static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
> 
> Meta-comment, would a IIO_DEV_ATTR_WO_FREQ() and friends help out here?
> That way it's impossible to get the permission values incorrect.
>
Hi Greg

I Agree with it and regarding that the macros IIO_DEV_ATTR_[FREQ, FREQSYMBOL,
PHASE, PHASESYMBOL, PINCONTROL_EN, OUT_ENABLE, OUTY_ENABLE] are only used
with "0200" permission in ad9832.c and ad9834.c files.
So, if everyone is fine with it, I could rename these macros this way
IIO_DEV_ATTR_WO_***.

Miguel
> thanks,
> 
> greg k-h
> --
> 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

Re: [PATCH] Staging: iio: frequency: use octal permissions instead of symbolic

[Jonathan Cameron]

On 16 March 2017 14:05:55 GMT+00:00, Greg KH <gregkh@linuxfoundation=2Eorg=
> wrote:
>On Thu, Mar 16, 2017 at 02:47:36PM +0100, Miguel Robles wrote:
>> Fix checkpatch warnings:
>> Symbolic permissions are not preferred=2E Consider using octal
>permissions=2E
>>=20
>> Signed-off-by: Miguel Robles <miguel=2Erobles@farole=2Enet>
>> ---
>>  drivers/staging/iio/frequency/ad9832=2Ec | 20 ++++++++++----------
>>  drivers/staging/iio/frequency/ad9834=2Ec | 22 +++++++++++-----------
>>  drivers/staging/iio/frequency/dds=2Eh    |  2 +-
>>  3 files changed, 22 insertions(+), 22 deletions(-)
>>=20
>> diff --git a/drivers/staging/iio/frequency/ad9832=2Ec
>b/drivers/staging/iio/frequency/ad9832=2Ec
>> index 8d40c8e=2E=2E163a4ef 100644
>> --- a/drivers/staging/iio/frequency/ad9832=2Ec
>> +++ b/drivers/staging/iio/frequency/ad9832=2Ec
>> @@ -248,22 +248,22 @@ static ssize_t ad9832_write(struct device *dev,
>struct device_attribute *attr,
>>   * see dds=2Eh for further information
>>   */
>> =20
>> -static IIO_DEV_ATTR_FREQ(0, 0, S_IWUSR, NULL, ad9832_write,
>AD9832_FREQ0HM);
>> -static IIO_DEV_ATTR_FREQ(0, 1, S_IWUSR, NULL, ad9832_write,
>AD9832_FREQ1HM);
>> -static IIO_DEV_ATTR_FREQSYMBOL(0, S_IWUSR, NULL, ad9832_write,
>AD9832_FREQ_SYM);
>> +static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9832_write,
>AD9832_FREQ0HM);
>> +static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9832_write,
>AD9832_FREQ1HM);
>> +static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9832_write,
>AD9832_FREQ_SYM);
>
>Meta-comment, would a IIO_DEV_ATTR_WO_FREQ() and friends help out here?
>That way it's impossible to get the permission values incorrect=2E
>
Keep it local to driver perhaps as this should be done with properly descr=
ibed channels=2E Snag is=20
no one has pinned down a final abi for these devices yet!

Lars is cleaning this up on the to-do list?

J
>thanks,
>
>greg k-h

--=20
Sent from my Android device with K-9 Mail=2E Please excuse my brevity=2E

Re: [PATCH] Staging: iio: frequency: use octal permissions instead of symbolic

[Lars-Peter Clausen]

On 03/16/2017 06:19 PM, Jonathan Cameron wrote:
> 
> 
> On 16 March 2017 14:05:55 GMT+00:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Thu, Mar 16, 2017 at 02:47:36PM +0100, Miguel Robles wrote:
>>> Fix checkpatch warnings:
>>> Symbolic permissions are not preferred. Consider using octal
>> permissions.
>>>
>>> Signed-off-by: Miguel Robles <miguel.robles@farole.net>
>>> ---
>>>  drivers/staging/iio/frequency/ad9832.c | 20 ++++++++++----------
>>>  drivers/staging/iio/frequency/ad9834.c | 22 +++++++++++-----------
>>>  drivers/staging/iio/frequency/dds.h    |  2 +-
>>>  3 files changed, 22 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/frequency/ad9832.c
>> b/drivers/staging/iio/frequency/ad9832.c
>>> index 8d40c8e..163a4ef 100644
>>> --- a/drivers/staging/iio/frequency/ad9832.c
>>> +++ b/drivers/staging/iio/frequency/ad9832.c
>>> @@ -248,22 +248,22 @@ static ssize_t ad9832_write(struct device *dev,
>> struct device_attribute *attr,
>>>   * see dds.h for further information
>>>   */
>>>  
>>> -static IIO_DEV_ATTR_FREQ(0, 0, S_IWUSR, NULL, ad9832_write,
>> AD9832_FREQ0HM);
>>> -static IIO_DEV_ATTR_FREQ(0, 1, S_IWUSR, NULL, ad9832_write,
>> AD9832_FREQ1HM);
>>> -static IIO_DEV_ATTR_FREQSYMBOL(0, S_IWUSR, NULL, ad9832_write,
>> AD9832_FREQ_SYM);
>>> +static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9832_write,
>> AD9832_FREQ0HM);
>>> +static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9832_write,
>> AD9832_FREQ1HM);
>>> +static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9832_write,
>> AD9832_FREQ_SYM);
>>
>> Meta-comment, would a IIO_DEV_ATTR_WO_FREQ() and friends help out here?
>> That way it's impossible to get the permission values incorrect.
>>
> Keep it local to driver perhaps as this should be done with properly described channels. Snag is 
> no one has pinned down a final abi for these devices yet!
> 
> Lars is cleaning this up on the to-do list?

Ideally those would be made proper IIO channel properties and then just let
the IIO core and the sysfs file generation. No need to handle permissions in
the driver at all. As far as I know nobody is working on it though, but if
somebody wants to pick it up, go ahead.