Re: [PATCH 5/6] gpio: Add new gpio-macsmc driver for Apple Macs

["Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx>]

On Tue, Sep 06, 2022 at 12:58:48AM +0900, Hector Martin wrote:
> On 06/09/2022 00.44, Martin Povišer wrote:
> > 
> >> On 5. 9. 2022, at 17:32, Russell King (Oracle) <linux@xxxxxxxxxxxxxxx> wrote:
> > 
> >> I still need a resolution between you and Hector over the smc_key
> >> issue - specifically, do I pick up the patch that adds support for
> >> %p4ch, or do we re-architect the smc_key thing and also in doing so
> >> get rid of the need for your "endian conversion" thing.
> > 
> > Idea about the %p4ch thing: We will leave the keys true
> > to their nature (i.e. 32-bit integer), and at least initially
> > for the prints we will employ macros
> > 
> > #define SMC_KEYFMT “%c%c%c%c”
> > #define SMC_KEYFMT_VAL(val) (val)>>24,(val)>>16,(val)>>8,(val)
> > 
> > used like
> > 
> > printk(“blah blah” SMC_KEYFMT “ blah\n”, SMC_KEYFMT_VAL(key));
> > 
> > This has the nice property that it is pretty much like the specifier,
> > and later can be easily replaced with the real thing.
> 
> Not the prettiest, but I'll take this over trying to mess around with
> string buffer conversions or anything involving non-native endianness if
> the printk specifier patch is going to be controversial.
> 
> I'd prefer shorter macro names though, like SMC_KFMT/SMC_KVAL(), to
> avoid further lengthening already-long printk lines.

I suggest that I try resubmitting the series with IRQ support dropped,
and with the %p4ch support in it and we'll see what happens. If %p4ch
gets accepted, then changing it would be adding extra work. In any
case, these %p... format extensions are supposed to avoid yucky stuff
such as the above.

Andy's objection to %p4ch was predicated on using the illegal C of
&cpu_to_be32(key) which has been shown to have been a waste of time.

For reference for those reading this, %p4ch doesn't print only print
the key as characters, it prints the hex value as well. For example:

macsmc-rtkit 23e400000.smc: Initialized (922 keys #KEY (0x234b4559)..zETM (0x7a45544d))
                                                  ^^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^
macsmc-gpio macsmc-gpio: First GPIO key: gP01 (0x67503031)
                                         ^^^^^^^^^^^^^^^^^

The underlined strings is the output from %p4ch.

So, even if Andy's cpu_to_be32() idea was legal C, it wouldn't be
functionally the same without adding extra code to every place that
one of these keys is printed.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!