On Mon, Oct 28, 2019 at 09:27:33AM +0000, wang.yi59@xxxxxxxxxx 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@xxxxxxxxxx> > > > Reviewed-by: Steven Price <steven.price@xxxxxxx> > > > --- > > > > > > 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. 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). 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
