[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZrJa6IahC6Edlitb@embed-PC.myguest.virtualbox.org>
Date: Tue, 6 Aug 2024 22:48:32 +0530
From: Abhishek Tamboli <abhishektamboli9@...il.com>
To: Takashi Iwai <tiwai@...e.de>
Cc: perex@...ex.cz, oswald.buddenhagen@....de, skhan@...uxfoundation.org,
rbmarliere@...il.com,
linux-kernel-mentees@...ts.linuxfoundation.org,
linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ALSA: emu10k1: Fix coding style issues and warnings
On Tue, Aug 06, 2024 at 06:29:17PM +0200, Takashi Iwai wrote:
> On Mon, 05 Aug 2024 17:52:06 +0200,
> Abhishek Tamboli wrote:
> >
> > Fix coding style errors and warnings in sound/pci/emu10k1/memory.c
> > reported by checkpatch.pl. Changes includes:
> >
> > - Add spaces after commas in macro definitions.
> > - Insert blank lines after variable declarations.
> > - Consistent spacing around operators.
> > - Remove trailing whitespace.
> > - Move EXPORT_SYMBOL declarations to immediately follow their
> > respective functions.
> > - Correct spacing in conditional statements and type casts.
> >
> > Fix the following errors and warnings:
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define __set_ptb_entry(emu,page,addr) \
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define __set_ptb_entry(emu,page,addr) \
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define set_ptb_entry(emu,page,addr) __set_ptb_entry(emu,page,addr)
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define set_ptb_entry(emu,page,addr) __set_ptb_entry(emu,page,addr)
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define set_ptb_entry(emu,page,addr) __set_ptb_entry(emu,page,addr)
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define set_ptb_entry(emu,page,addr) __set_ptb_entry(emu,page,addr)
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define set_silent_ptb(emu,page)
> > __set_ptb_entry(emu,page,emu->silent_page.addr)
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define set_silent_ptb(emu,page)
> > __set_ptb_entry(emu,page,emu->silent_page.addr)
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define set_silent_ptb(emu,page)
> > __set_ptb_entry(emu,page,emu->silent_page.addr)
> >
> > WARNING: Missing a blank line after declarations
> > + int i;
> > + page *= UNIT_PAGES;
> >
> > WARNING: Missing a blank line after declarations
> > + int i;
> > + page *= UNIT_PAGES;
> >
> > ERROR: space required after that ',' (ctx:VxV)
> > +#define get_emu10k1_memblk(l,member)
> > list_entry(l, struct snd_emu10k1_memblk, member)
> >
> > WARNING: space prohibited between function name and open parenthesis '('
> > + list_for_each (pos, &emu->mapped_link_head) {
> >
> > WARNING: Missing a blank line after declarations
> > + struct snd_emu10k1_memblk *blk =
> > get_emu10k1_memblk(pos, mapped_link);
> > + if (blk->mapped_page < 0)
> >
> > ERROR: else should follow close brace '}'
> > + }
> > + else if (size > max_size) {
> >
> > ERROR: need consistent spacing around '-' (ctx:WxV)
> > + psize = get_aligned_page(size + PAGE_SIZE -1);
> > ^
> >
> > WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> > +EXPORT_SYMBOL(snd_emu10k1_memblk_map);
> >
> > WARNING: Missing a blank line after declarations
> > + dma_addr_t addr;
> > + if (ofs >= runtime->dma_bytes)
> >
> > ERROR: space prohibited after that '!' (ctx:BxW)
> > + if (! is_valid_page(emu, addr)) {
> > ^
> >
> > ERROR: trailing whitespace
> > +^Istruct snd_util_memhdr *hdr = hw->memhdr; $
> >
> > WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> > +EXPORT_SYMBOL(snd_emu10k1_synth_alloc);
> >
> > ERROR: trailing whitespace
> > +^Istruct snd_util_memhdr *hdr = emu->memhdr; $
> >
> > WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> > +EXPORT_SYMBOL(snd_emu10k1_synth_free);
> >
> > WARNING: Missing a blank line after declarations
> > + int first_page, last_page;
> > + first_page = blk->first_page;
> >
> > WARNING: Missing a blank line after declarations
> > + char *ptr;
> > + if (snd_BUG_ON(page < 0 || page >= emu->max_cache_pages))
> >
> > ERROR: space prohibited after that '!' (ctx:BxW)
> > + if (! ptr) {
> > ^
> >
> > ERROR: "(foo*)" should be "(foo *)"
> > + return (void*)ptr;
> >
> > WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> > +EXPORT_SYMBOL(snd_emu10k1_synth_memset);
> >
> > WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> > +EXPORT_SYMBOL(snd_emu10k1_synth_copy_from_user);
> >
> > Signed-off-by: Abhishek Tamboli <abhishektamboli9@...il.com>
>
> Thanks for the patch. But in general, I take this kind of
> white-space-only patches as a preliminary only when coupled with some
> other real fixes or improvement changes -- especially for old driver
> code. Otherwise it'd just scratch the git history.
>
> So, if you have other actual fixes for this driver based on this clean
> up, let me know.
Hi Takashi,
Thank you for the feedback. I understand your point regarding
white-space-only patches and I'll keep this in mind for future
contributions. Unfortunately, I don't have any additional
fixes or improvements for this driver at the moment.
Regards,
Abhishek
Powered by blists - more mailing lists