Re: [PATCH 08/17] arch/sh: Do not assign FBINFO_FLAG_DEFAULT to fb_videomode.flag

Thomas Zimmermann <[email protected]>
Newsgroups gmane.linux.ports.sh.devel,gmane.comp.video.dri.devel,gmane.linux.kernel,gmane.comp.freedesktop.amd-gfx,gmane.linux.kernel.input,gmane.linux.drivers.video-input-infrastructure,gmane.linux.ports.arm.kernel,gmane.linux.ports.geode,gmane.linux.ports.arm.omap,gmane.linux.ports.ppc64.devel,gmane.comp.emulators.kvm.devel
Message-ID <[email protected]>
Hi

Am 10.07.23 um 15:42 schrieb John Paul Adrian Glaubitz:
> Hi Thomas!
> 
> On Mon, 2023-07-10 at 14:50 +0200, Thomas Zimmermann wrote:
>> FBINFO_FLAG_DEFAULT is a flag for a framebuffer in struct fb_info.
>> Flags for videomodes are prefixed with FB_MODE_. FBINFO_FLAG_DEFAULT
>> is 0 and the static declaration already clears the memory area of
>> sh7763fb_videomode. So remove the assignment.
>>
>> Signed-off-by: Thomas Zimmermann <[email protected]>
>> Cc: Yoshinori Sato <[email protected]>
>> Cc: Rich Felker <[email protected]>
>> Cc: John Paul Adrian Glaubitz <[email protected]>
>> ---
>>   arch/sh/boards/mach-sh7763rdp/setup.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/sh/boards/mach-sh7763rdp/setup.c b/arch/sh/boards/mach-sh7763rdp/setup.c
>> index 97e715e4e9b3..345f2b76c85a 100644
>> --- a/arch/sh/boards/mach-sh7763rdp/setup.c
>> +++ b/arch/sh/boards/mach-sh7763rdp/setup.c
>> @@ -119,7 +119,6 @@ static struct fb_videomode sh7763fb_videomode = {
>>   	.vsync_len = 1,
>>   	.sync = 0,
>>   	.vmode = FB_VMODE_NONINTERLACED,
>> -	.flag = FBINFO_FLAG_DEFAULT,
>>   };
>>   
>>   static struct sh7760fb_platdata sh7763fb_def_pdata = {
> 
> I would argue that the current code is more readable that your proposed change.
> 
> I agree that it's a no-op, but code is not just about functionality but also
> readability, isn't it?

I won't argue with that, but the flag itself is wrong. 
FBINFO_FLAG_DEFAULT is/was for struct fb_info.flags. You have struct 
fb_videomode.flag. The valid flags for this field are at [1]. If 
anything, the field could be initialized to FB_MODE_IS_UNKNOWN, which 
has the same value.

[1] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L681

> 
> Also, I prefer "sh:" as the architecture prefix, not "arch/sh:".

Ok.

Best regards
Thomas
> 
> Thanks,
> Adrian
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature (application/pgp-signature, 840 B) - not displayed
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.