On Mon, Nov 02, 2020 at 11:13:47AM +0100, Daniel Vetter wrote: > 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? I see. I will try to elaborate in future patches, thank you! > > 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. Sure, I will try to include sufficient information for one to easily understand what I'm trying to do with a patch. Thank you, Peilin _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
