On Mon, Nov 02, 2020 at 10:47:55AM +0100, Jiri Slaby wrote: > On 02. 11. 20, 10:36, Peilin Ye wrote: > > `struct console_font` is a UAPI structure, thus ideally should not be > > used for kernel internal abstraction. Remove some dummy .con_font_set, > > .con_font_default and .con_font_copy `struct consw` callback > > implementations, to make it cleaner. > > ESEMANTIC_ERROR. > > 1) What do you refer to with the last "it"? > > 2) What's the purpose of mentioning struct console_font at all? > > 3) Could you clarify whether you checked it is safe to remove the hooks? > > 4) All the hooks now return ENOSYS for both consoles (and not 0). Is this > intentional? > > I know answers to the first 3 questions, but you need to elaborate a bit in > the commit log to connect those sentences. Esp. for people not dealing with > the code on a daily basis. Ad 4) I am not sure. Yup the behaviour change from 4) needs to be called out. I think this should then also be done as part of the large patch series to remove the dummy functions from all console drivers. I don't expect the errno change to cause trouble, and it's the more honest errno - changing fonts not supported is the truth. But if it is, we can patch that up appropriately when we get a regression report. That's kinda unavoidable with old crufty uapi like this one here. Also a bikeshed: Additional information like the patch changelog or reasons why you do something is imo best to include in the commit message itself. It ends up looking a bit less tidy sometimes, but often there's crucial information in these parts that was accidentally left out from the commit message. Thanks, Daniel > > > Suggested-by: Daniel Vetter <daniel@xxxxxxxx> > > Signed-off-by: Peilin Ye <yepeilin.cs@xxxxxxxxx> > > --- > > Change in v2: > > - [v2 2/2] no longer Cc: stable, so do not Cc: stable > > > > Context: https://lore.kernel.org/lkml/CAKMK7uFY2zv0adjKJ_ORVFT7Zzwn075MaU0rEU7_FuqENLR=UA@xxxxxxxxxxxxxx/ > > > > drivers/usb/misc/sisusbvga/sisusb_con.c | 21 --------------------- > > drivers/video/console/dummycon.c | 20 -------------------- > > 2 files changed, 41 deletions(-) > > > > diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c b/drivers/usb/misc/sisusbvga/sisusb_con.c > > index c63e545fb105..dfa0d5ce6012 100644 > > --- a/drivers/usb/misc/sisusbvga/sisusb_con.c > > +++ b/drivers/usb/misc/sisusbvga/sisusb_con.c > > @@ -1345,24 +1345,6 @@ static int sisusbdummycon_blank(struct vc_data *vc, int blank, int mode_switch) > > return 0; > > } > > -static int sisusbdummycon_font_set(struct vc_data *vc, > > - struct console_font *font, > > - unsigned int flags) > > -{ > > - return 0; > > -} > > - > > -static int sisusbdummycon_font_default(struct vc_data *vc, > > - struct console_font *font, char *name) > > -{ > > - return 0; > > -} > > - > > -static int sisusbdummycon_font_copy(struct vc_data *vc, int con) > > -{ > > - return 0; > > -} > > - > > static const struct consw sisusb_dummy_con = { > > .owner = THIS_MODULE, > > .con_startup = sisusbdummycon_startup, > > @@ -1375,9 +1357,6 @@ static const struct consw sisusb_dummy_con = { > > .con_scroll = sisusbdummycon_scroll, > > .con_switch = sisusbdummycon_switch, > > .con_blank = sisusbdummycon_blank, > > - .con_font_set = sisusbdummycon_font_set, > > - .con_font_default = sisusbdummycon_font_default, > > - .con_font_copy = sisusbdummycon_font_copy, > > }; > > int > > diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c > > index 2a0d0bda7faa..f1711b2f9ff0 100644 > > --- a/drivers/video/console/dummycon.c > > +++ b/drivers/video/console/dummycon.c > > @@ -124,23 +124,6 @@ static int dummycon_switch(struct vc_data *vc) > > return 0; > > } > > -static int dummycon_font_set(struct vc_data *vc, struct console_font *font, > > - unsigned int flags) > > -{ > > - return 0; > > -} > > - > > -static int dummycon_font_default(struct vc_data *vc, > > - struct console_font *font, char *name) > > -{ > > - return 0; > > -} > > - > > -static int dummycon_font_copy(struct vc_data *vc, int con) > > -{ > > - return 0; > > -} > > - > > /* > > * The console `switch' structure for the dummy console > > * > > @@ -159,8 +142,5 @@ const struct consw dummy_con = { > > .con_scroll = dummycon_scroll, > > .con_switch = dummycon_switch, > > .con_blank = dummycon_blank, > > - .con_font_set = dummycon_font_set, > > - .con_font_default = dummycon_font_default, > > - .con_font_copy = dummycon_font_copy, > > }; > > EXPORT_SYMBOL_GPL(dummy_con); > > > > > -- > js > suse labs -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
