Re:[PATCH v3] drm/panfrost: fix -Wmissing-prototypes warnings

[wang.yi59 at zte.com.cn]

> On Mon, Oct 28, 2019 at 09:27:33AM +0000, wang.yi59 at zte.com.cn wrote:
> > Hi Steve,
> >
> > Thanks a lot for your time and patience :)
> >
> > > On 25/10/2019 02:30, Yi Wang wrote:
> > > > We get these warnings when build kernel W=1:
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:35:6: warning: no previous prototype for ‘panfrost_perfcnt_clean_cache_done’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:40:6: warning: no previous prototype for ‘panfrost_perfcnt_sample_done’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:190:5: warning: no previous prototype for ‘panfrost_ioctl_perfcnt_enable’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:218:5: warning: no previous prototype for ‘panfrost_ioctl_perfcnt_dump’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:250:6: warning: no previous prototype for ‘panfrost_perfcnt_close’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:264:5: warning: no previous prototype for ‘panfrost_perfcnt_init’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:320:6: warning: no previous prototype for ‘panfrost_perfcnt_fini’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_mmu.c:227:6: warning: no previous prototype for ‘panfrost_mmu_flush_range’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_mmu.c:435:5: warning: no previous prototype for ‘panfrost_mmu_map_fault_addr’ [-Wmissing-prototypes]
> > > >
> > > > For file panfrost_mmu.c, make functions static to fix this.
> > > > For file panfrost_perfcnt.c, include header file can fix this.
> > > >
> > > > Signed-off-by: Yi Wang <wang.yi59 at zte.com.cn>
> > > > Reviewed-by: Steven Price <steven.price at arm.com>
> > > > ---
> > > >
> > > > v3: using tab size of 8 other than 4.
> > > >
> > > > v2: align parameter line and modify comment. Thanks to Steve.
> > > > ---
> > > >  drivers/gpu/drm/panfrost/panfrost_mmu.c     | 9 +++++----
> > > >  drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 1 +
> > > >  2 files changed, 6 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > > > index bdd9905..871574c 100644
> > > > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > > > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > > > @@ -224,9 +224,9 @@ static size_t get_pgsize(u64 addr, size_t size)
> > > >      return SZ_2M;
> > > >  }
> > > >
> > > > -void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > > > -                  struct panfrost_mmu *mmu,
> > > > -                  u64 iova, size_t size)
> > > > +static void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > > > +                    struct panfrost_mmu *mmu,
> > > > +                    u64 iova, size_t size)
> > >
> > > Ok, I'll admit I wouldn't have spotted this unless I'd double checked by
> > > applying the patch, but you still seem to have something misconfigured
> > > in your editor. This is out by one character:
> > >
> > > static void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > > >------->------->------->-------    struct panfrost_mmu *mmu,
> > > >------->------->------->-------    u64 iova, size_t size)
> > >
> > > There should be an extra space to align correctly.
> >
> > According to [Linux kernel coding style](https://www.kernel.org/doc/html/v4.10/process/coding-style.html):
> > > Outside of comments, documentation and except in Kconfig, spaces are
> > > never used for indentation, and the above example is deliberately broken.
> >
> > If I understand corretly, the tab is enough for indentation :-)
>
> There's a (subtle) difference between indentation and alignment.
> Indentation is where the code is moved over because it is contained
> within an outer statement (e.g. an 'if' or 'while'). The Linux coding
> style is that this should be done with only tabs as you quote.

Ok, I got it.

>
> However when the line is a continuation of the previous line, like in
> the example here, then we often need a combination of tabs/spaces to
> align correctly. The desire here is that the continuation lines all
> start straight after the "(" on the first line. Since that is 37
> characters from the start we need 4 tabs + 5 spaces (4*8+5=37), whereas
> you had 4 tabs + 4 spaces. My guess is this is because you've got the
> following vim configuration:
>
>  tabstop = 8
>  softtabstop = 4
>  noexpandtab
>
> This means that pressing <tab> moves you along only 4 columns (i.e.
> inserts 4 spaces), a second <tab> will then replace the spaces with a
> real tab character (i.e. move another 4 columns for a total of 8).
>
> It's probably best to set "softtabstop = 0" so that <tab> always inserts
> a real tab character. You might also find it useful to make tabs
> visible:
>
>  set list
>  set listchars=tab:>-
>
> That way you can easily see where you have tab characters and spaces
> (like in the example I quoted above).

That's really useful. Thank you, Steve.
I will send a new patch soon.

>
> Steve
>
> > >
> > > >  {
> > > >      if (mmu->as < 0)
> > > >          return;
> > > > @@ -432,7 +432,8 @@ void panfrost_mmu_pgtable_free(struct panfrost_file_priv *priv)
> > > >
> > > >  #define NUM_FAULT_PAGES (SZ_2M / PAGE_SIZE)
> > > >
> > > > -int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, u64 addr)
> > > > +static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> > > > +                    u64 addr)
> > >
> > > Here you're off-by-one in the other direction - you need to replace the
> > > final tab with 7 spaces:
> > >
> > > static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> > > >------->------->------->------->-------u64 addr)
> > >
> > > Sorry to nit-pick over this, but it's good to get your editor setup
> > > correctly to ensure your formatting is correct.
> >
> > Yeah, it worth time on the editor setup, and thanks again.
> >
> > >
> > > Thanks,
> > >
> > > Steve
> >
> >
> > ---
> > Best wishes
> > Yi Wang



---
Best wishes
Yi Wang