Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 2 Nov 2020 05:52:28 -0500
From:   Peilin Ye <yepeilin.cs@gmail.com>
To:     Daniel Vetter <daniel@ffwll.ch>, Jiri Slaby <jirislaby@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Thomas Winischhofer <thomas@winischhofer.net>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Nicolas Pitre <nico@fluxnic.net>,
        "Gustavo A . R . Silva" <gustavoars@kernel.org>,
        Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
        George Kennedy <george.kennedy@oracle.com>,
        Nathan Chancellor <natechancellor@gmail.com>,
        Peter Rosin <peda@axentia.se>, linux-kernel@vger.kernel.org,
        linux-usb@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-fbdev@vger.kernel.org
Subject: Re: [PATCH v2 1/2] console: Remove dummy con_font_op() callback
 implementations
Message-ID: <20201102105228.GA1558395@PWN>
References: <c5563eeea36aae7bd72ea2e985bc610d585ece40.1604128639.git.yepeilin.cs@gmail.com>
 <c5563eeea36aae7bd72ea2e985bc610d585ece40.1604306433.git.yepeilin.cs@gmail.com>
 <a8157089-bc72-c409-c7cc-2fd7eb087ebc@kernel.org>
 <20201102101347.GN401619@phenom.ffwll.local>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201102101347.GN401619@phenom.ffwll.local>
Precedence: bulk

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