| Message ID | 20230710130113.14563-9-tzimmermann@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags | expand |
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 <tzimmermann@suse.de> > Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > Cc: Rich Felker <dalias@libc.org> > Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> > --- > 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? Also, I prefer "sh:" as the architecture prefix, not "arch/sh:". Thanks, Adrian
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 <tzimmermann@suse.de> >> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> >> Cc: Rich Felker <dalias@libc.org> >> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> >> --- >> 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 >
Hi Thomas! On Mon, 2023-07-10 at 15:52 +0200, Thomas Zimmermann wrote: > > 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 FB_MODE_IS_UNKNOWN sounds very reasonable to me. Would you agree using that instead? > > > > Also, I prefer "sh:" as the architecture prefix, not "arch/sh:". > > Ok. Thanks. Adrian
Hi Am 10.07.23 um 15:59 schrieb John Paul Adrian Glaubitz: > Hi Thomas! > > On Mon, 2023-07-10 at 15:52 +0200, Thomas Zimmermann wrote: >>> 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 > > FB_MODE_IS_UNKNOWN sounds very reasonable to me. Would you agree using that instead? Sure, I'll update the patch accordingly. Best regards Thomas > >>> >>> Also, I prefer "sh:" as the architecture prefix, not "arch/sh:". >> >> Ok. > > Thanks. > > Adrian >
Hi! On Mon, 2023-07-10 at 16:04 +0200, Thomas Zimmermann wrote: > > > 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 > > > > FB_MODE_IS_UNKNOWN sounds very reasonable to me. Would you agree using that instead? > > Sure, I'll update the patch accordingly. Thanks! I'll ack the updated patch. Adrian
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 = {
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 <tzimmermann@suse.de> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> Cc: Rich Felker <dalias@libc.org> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> --- arch/sh/boards/mach-sh7763rdp/setup.c | 1 - 1 file changed, 1 deletion(-)