On 22/11/2023 17:38, Detlev Casanova wrote: > On Wednesday, November 22, 2023 10:56:20 A.M. EST Hans Verkuil wrote: >> On 24/10/2023 21:09, Detlev Casanova wrote: >>> `false` was used as the keep_bitstream_buffers parameter permissions. >>> This looks more like a default value for the parameter, so change it to >>> 0 to avoid confusion. >>> >>> Reviewed-by: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx> >>> Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx> >>> --- >>> >>> drivers/media/test-drivers/visl/visl-core.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/media/test-drivers/visl/visl-core.c >>> b/drivers/media/test-drivers/visl/visl-core.c index >>> 9970dc739ca5..df6515530fbf 100644 >>> --- a/drivers/media/test-drivers/visl/visl-core.c >>> +++ b/drivers/media/test-drivers/visl/visl-core.c >>> @@ -74,7 +74,7 @@ MODULE_PARM_DESC(visl_dprintk_nframes, >>> >>> " the number of frames to trace with dprintk"); >>> >>> bool keep_bitstream_buffers; >>> >>> -module_param(keep_bitstream_buffers, bool, false); >>> +module_param(keep_bitstream_buffers, bool, 0); >> >> ??? >> >> This last parameter is the permission, it makes no sense that this >> is either 0 or false: then nobody can see it in /sys/modules/. > > It makes sense if we want it set when the module is loaded only. This way, we > don't have to manage the parameters values changing while work is being done > and keep it simple. > It could be made readable if that looks better, but there is no real need for > it to be read either. Why not? It makes it easy to read what this module option's value is. I now see that both visl and vidtv uses 0 a lot, so I'm OK with this patch for consistency. But I think especially these test-drivers should use 0444 instead of 0 so you can see how the test driver is configured. That might actually be relevant when writing tests using these drivers. Perhaps separate patches for visl and vidtv that change 0 to 0444 for all the module parameters? Regards, Hans > >> Typically this is 0444 if it is readable only, or 0644 if it can be >> written by root. >> >> Regards, >> >> Hans >> >>> MODULE_PARM_DESC(keep_bitstream_buffers, >>> >>> " keep bitstream buffers in debugfs after streaming is > stopped"); >
