On Wed, Oct 19, 2016 at 01:34:44AM +0530, Shyam Saini wrote:
> On Tue, 2016-10-18 at 14:36 +0200, Greg KH wrote:
> > On Tue, Oct 18, 2016 at 05:52:21PM +0530, Shyam Saini wrote:
> > >
> > > On Mon, 2016-10-17 at 21:37 +0200, Greg KH wrote:
> > > >
> > > > On Mon, Oct 17, 2016 at 10:10:08PM +0530, Shyam Saini wrote:
> > > > >
> > > > >
> > > > > On Mon, 2016-10-17 at 18:07 +0200, Greg KH wrote:
> > > > > >
> > > > > >
> > > > > > On Mon, Oct 17, 2016 at 09:24:19PM +0530, Shyam Saini wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Fixed following checkpatch warning
> > > > > > > Symbolic permissions 'S_IRUGO' are not preferred. Consider
> > > > > > > using
> > > > > > > octal
> > > > > > > permissions '0444'
> > > > > > >
> > > > > > > Signed-off-by: Shyam Saini <mayhs11saini@xxxxxxxxx>
> > > > > > > ---
> > > > > > > drivers/staging/media/bcm2048/radio-bcm2048.c | 18
> > > > > > > +++++++++
> > > > > > > ----
> > > > > > > -----
> > > > > > > 1 file changed, 9 insertions(+), 9 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > > b/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > > index 5d2b702..9c286c3 100644
> > > > > > > --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > > +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > > @@ -2103,21 +2103,21 @@ static struct device_attribute
> > > > > > > attrs[]
> > > > > > > = {
> > > > > > > bcm2048_rds_pi_match_write),
> > > > > > > __ATTR(rds_wline, 0644, bcm2048_rds_wline_read,
> > > > > > > bcm2048_rds_wline_write),
> > > > > > > - __ATTR(rds_pi, S_IRUGO, bcm2048_rds_pi_read,
> > > > > > > NULL),
> > > > > > > - __ATTR(rds_rt, S_IRUGO, bcm2048_rds_rt_read,
> > > > > > > NULL),
> > > > > > > - __ATTR(rds_ps, S_IRUGO, bcm2048_rds_ps_read,
> > > > > > > NULL),
> > > > > > > - __ATTR(fm_rds_flags, S_IRUGO,
> > > > > > > bcm2048_fm_rds_flags_read,
> > > > > > > NULL),
> > > > > > > - __ATTR(region_bottom_frequency, S_IRUGO,
> > > > > > > + __ATTR(rds_pi, 0444, bcm2048_rds_pi_read, NULL),
> > > > > > > + __ATTR(rds_rt, 0444, bcm2048_rds_rt_read, NULL),
> > > > > > > + __ATTR(rds_ps, 0444, bcm2048_rds_ps_read, NULL),
> > > > > > > + __ATTR(fm_rds_flags, 0444,
> > > > > > > bcm2048_fm_rds_flags_read,
> > > > > > > NULL),
> > > > > > > + __ATTR(region_bottom_frequency, 0444,
> > > > > > > bcm2048_region_bottom_frequency_read,
> > > > > > > NULL),
> > > > > >
> > > > > >
> > > > > > Why not use __ATTR_RO() for all of these instead?
> > > > > I just used what checkpatch.pl suggested. Now I'll
> > > > > use __ATTR_RO().
> > > > >
> > > > > Also shouldn't we change checkpatch.pl accordingly ?
> > > > No, not all octal values should be converted that way.
> > > >
> > > > checkpatch is a "hint", not always the exact thing that needs to
> > > > be
> > > > done, and sometimes it should be ignored. Please always use your
> > > > judgement.
> > > I have used __ATTR_RO() __ATTR_RW(). But, when I build it, i got
> > > so
> > > many error messages and build fails. I'm attaching a file
> > > containing
> > > errors.
>
> > It's not always a drop-in replacement, as you have seen, you will
> > need
> > to rename some functions as well.
> >
>
> I checked <linux/sysfs.h>.
> What I can see is that __ATTR_RO append its argument with _show. I've
> no clue how to fix it.
That's not good. Did you look at how other users of this macro handle
this issue? Please work through how the macro works and what it
expects, that should show you what to do.
If not, I suggest working on some other project to get up to speed on
more advanced C topics (macros, preprocessor, etc.) before working on
the kernel.
best of luck!
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
