[PATCH v3] staging: bcm2835-audio: add terminating new line to Kconfig

[PATCH v3] staging: bcm2835-audio: add terminating new line to Kconfig

[Prasad Pandit]

From: Prasad Pandit <pjp@fedoraproject.org>

Add terminating new line to the Kconfig file. It helps
Kconfig parsers to read file without error.

Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
---
 drivers/staging/vc04_services/bcm2835-audio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

v3: fix typo and note about parsing errors.
  -> https://lore.kernel.org/lkml/CAE8KmOzcD+__7xdC7tegbHO9HEP48s7=reA4j-tvqVDwzHr+8Q@mail.gmail.com/T/#t

diff --git a/drivers/staging/vc04_services/bcm2835-audio/Kconfig b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
index 7f22f6c85067..7fbb29d3c34d 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/Kconfig
+++ b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
@@ -8,4 +8,4 @@ config SND_BCM2835
 	  Say Y or M if you want to support BCM2835 built in audio.
 	  This driver handles both 3.5mm and HDMI audio, by leveraging
 	  the VCHIQ messaging interface between the kernel and the firmware
-	  running on VideoCore.
\ No newline at end of file
+	  running on VideoCore.
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] staging: bcm2835-audio: add terminating new line to Kconfig

[Dan Carpenter]

On Wed, Mar 13, 2024 at 05:11:26PM +0530, Prasad Pandit wrote:
> From: Prasad Pandit <pjp@fedoraproject.org>
> 
> Add terminating new line to the Kconfig file. It helps
> Kconfig parsers to read file without error.
> 
> Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
> ---
>  drivers/staging/vc04_services/bcm2835-audio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> v3: fix typo and note about parsing errors.
>   -> https://lore.kernel.org/lkml/CAE8KmOzcD+__7xdC7tegbHO9HEP48s7=reA4j-tvqVDwzHr+8Q@mail.gmail.com/T/#t

Normally this would go about the diffstat but no stress...

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] staging: bcm2835-audio: add terminating new line to Kconfig

[Greg KH]

On Wed, Mar 13, 2024 at 05:11:26PM +0530, Prasad Pandit wrote:
> From: Prasad Pandit <pjp@fedoraproject.org>
> 
> Add terminating new line to the Kconfig file. It helps
> Kconfig parsers to read file without error.

What in-tree parser has a problem with this file as-is?  If it's an
out-of-tree parser, that's different, and the tool should be fixed, no
need to touch the kernel files for no good reason.

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] staging: bcm2835-audio: add terminating new line to Kconfig

[Dan Carpenter]

On Mon, Mar 25, 2024 at 07:04:15PM +0100, Greg KH wrote:
> On Wed, Mar 13, 2024 at 05:11:26PM +0530, Prasad Pandit wrote:
> > From: Prasad Pandit <pjp@fedoraproject.org>
> > 
> > Add terminating new line to the Kconfig file. It helps
> > Kconfig parsers to read file without error.
> 
> What in-tree parser has a problem with this file as-is?  If it's an
> out-of-tree parser, that's different, and the tool should be fixed, no
> need to touch the kernel files for no good reason.

It's annoying to cat a file when it doesn't have a newline on the end...

dcarpenter@moroto:~/progs/kernel/trees$ cat -n drivers/staging/vc04_services/bcm2835-audio/Kconfig
     1  # SPDX-License-Identifier: GPL-2.0
     2  config SND_BCM2835
     3          tristate "BCM2835 Audio"
     4          depends on (ARCH_BCM2835 || COMPILE_TEST) && SND
     5          select SND_PCM
     6          select BCM2835_VCHIQ if HAS_DMA
     7          help
     8            Say Y or M if you want to support BCM2835 built in audio.
     9            This driver handles both 3.5mm and HDMI audio, by leveraging
    10            the VCHIQ messaging interface between the kernel and the firmware
    11            running on VideoCore.dcarpenter@moroto:~/progs/kernel/trees$
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

So you could resend with that as a justification.  But, yeah, it's a
good idea to fix the tool as well.

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] staging: bcm2835-audio: add terminating new line to Kconfig

[Prasad Pandit]

Hello Greg, Dan

On Tue, 26 Mar 2024 at 01:12, Dan Carpenter <dan.carpenter@linaro.org> wrote:
> On Mon, Mar 25, 2024 at 07:04:15PM +0100, Greg KH wrote:
>>  If it's an out-of-tree parser, that's different, and the tool should be fixed, no
>> need to touch the kernel files for no good reason.
> It's annoying to cat a file when it doesn't have a newline on the end...
>
> dcarpenter@moroto:~/progs/kernel/trees$ cat -n drivers/staging/vc04_services/bcm2835-audio/Kconfig
>     11            running on VideoCore.dcarpenter@moroto:~/progs/kernel/trees$
>                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> So you could resend with that as a justification.  But, yeah, it's a
> good idea to fix the tool as well.

* I'm trying to fix errors reported by the config-kernel[1] tool. It
is an out-of-tree parser.

* IMHO, this patch should be evaluated on whether not having the
terminating new-line character is right or wrong? Saying that patch is
not required because we don't know if it breaks an in-tree parser OR
that patch is acceptable for the annoyance while using cat(1) command
does not seem right.

* As for fixing the parser, I did try to do that in the past but did
not find a good fix for it, I'll try again. Meanwhile, I tried to find
out how many such files are there?
===
$ tf=0 df=0; for f in $(find . -path "./[a-zA-Z0-9]*" -name Kconfig*);
do tf=$(($tf+1)); eof=$(tail -c1 $f); if [ -n "$eof" -a "$eof" != "\n"
]; then echo $f; df=$(($df+1)); fi done 2> /dev/null; echo
"No-NL-Files/Total-Files: $df/$tf";
./drivers/staging/vc04_services/bcm2835-audio/Kconfig
./drivers/media/dvb-frontends/cxd2880/Kconfig
No-NL-Files/Total-Files: 2/1698

$ tf=0 df=0; for f in $(find . -path "./[a-zA-Z0-9]*" -type f); do
tf=$(($tf+1)); eof=$(tail -c1 $f); if [ -n "$eof" -a "$eof" != "\n" ];
then echo $f; df=$(($df+1)); fi done 2> /dev/null; echo
"No-NL-Files/Total-Files: $df/$tf";
./drivers/gpu/drm/i915/gt/shaders/README
./drivers/gpu/drm/xe/display/xe_fb_pin.c
./drivers/gpu/drm/sprd/Makefile
...
No-NL-Files/Total-Files: 65/84319
===

* There are 2 Kconfig files which don't terminate with a new-line
character. I have sent fix patches for both of them. Rest of all
~99.90+% regular files follow the norm of terminating with a new-line
character. Clearly files not terminating with a new-line are erroneous
and should be fixed. Suggesting to fix the parsers to handle erroneous
input files, which can be easily fixed in the source tree itself, is
not reasonable. Maybe as Dan suggested, it'll help to add a check to
'checkpatch.pl' script to flag patches which try to add files without
terminating new-lines. I tried to look into checkpatch.pl script, but
it's quite big to understand in a day or two.

* I'm okay to resend the patch saying annoyance while using cat(1)
command as justification if that works. If in-tree parser is the only
criteria, what would be the process to make config-kernel[1] tool an
in-tree parser? I can try to follow that process.

Thank you.
---
  - Prasad
[1] https://github.com/pjps/config-kernel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel