Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 24 Aug 2023 12:46:56 +0000
From:   "Dr. David Alan Gilbert" <linux@treblig.org>
To:     Michael Ellerman <mpe@ellerman.id.au>
Cc:     npiggin@gmail.com, christophe.leroy@csgroup.eu, sam@ravnborg.org,
        benh@kernel.crashing.org, linuxppc-dev@lists.ozlabs.org,
        linux-kernel@vger.kernel.org, rdunlap@infradead.org
Subject: Re: [PATCH v3] powerpc: Use shared font data
Message-ID: <ZOdRQFKaxnKQ+s/U@gallifrey>
References: <20230812010241.529173-1-linux@treblig.org>
 <ZOY0dANP4s3L3VyX@gallifrey>
 <87msygvjvy.fsf@mail.lhotse>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <87msygvjvy.fsf@mail.lhotse>
User-Agent: Mutt/2.0.5 (2021-01-21)
Precedence: bulk

* Michael Ellerman (mpe@ellerman.id.au) wrote:
> "Dr. David Alan Gilbert" <linux@treblig.org> writes:
> > * linux@treblig.org (linux@treblig.org) wrote:
> >> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> >> 
> >> PowerPC has a 'btext' font used for the console which is almost identical
> >> to the shared font_sun8x16, so use it rather than duplicating the data.
> >> 
> >> They were actually identical until about a decade ago when
> >>    commit bcfbeecea11c ("drivers: console: font_: Change a glyph from
> >>                         "broken bar" to "vertical line"")
> >> 
> >> which changed the | in the shared font to be a solid
> >> bar rather than a broken bar.  That's the only difference.
> >> 
> >> This was originally spotted by the PMF source code analyser, which
> >> noticed that sparc does the same thing with the same data, and they
> >> also share a bunch of functions to manipulate the data.  I've previously
> >> posted a near identical patch for sparc.
> >> 
> >> Tested very lightly with a boot without FS in qemu.
> >> 
> >> Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> >
> > Hi Michael,
> >   Just checking, is there anything else I need to do here?
>    
> I felt like the lib/fonts/Kconfig change could be cleaner, but the
> existing Kconfig is not well structured for what we want to do.

Yeh that line has caused quite a bit of pain in this otherwise simple
set.

> I think at least it could be simplified to:
> 
> config FONT_SUN8x16
> 	bool "Sparc console 8x16 font"
> 	depends on (FRAMEBUFFER_CONSOLE && (FONTS || SPARC)) || BOOTX_TEXT

OK, I'll give it a go - but I wonder what else we need to test?
My version was careful not to change other arch's behaviour
on this config, I worry this might. (Although IMHO I actually think
we could just remove this entire depends, and the only reason I haven't
is because I worry what other knock on there might be).

Dave

> 
> cheers
> 
> 
> >> diff --git a/lib/fonts/Kconfig b/lib/fonts/Kconfig
> >> index c035fde66aebe..1804abe08ffe4 100644
> >> --- a/lib/fonts/Kconfig
> >> +++ b/lib/fonts/Kconfig
> >> @@ -98,7 +98,8 @@ config FONT_10x18
> >>  
> >>  config FONT_SUN8x16
> >>  	bool "Sparc console 8x16 font"
> >> -	depends on FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC)
> >> +	depends on (FRAMEBUFFER_CONSOLE || BOOTX_TEXT) &&\
> >> +	           (!SPARC && FONTS || SPARC || PPC)
> >>  	help
> >>  	  This is the high resolution console font for Sun machines. Say Y.
> >>  
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/