[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230703-patrol-paced-76cd5e24db6b@wendy>
Date: Mon, 3 Jul 2023 14:02:15 +0100
From: Conor Dooley <conor.dooley@...rochip.com>
To: 运辉崔 <cuiyunhui@...edance.com>
CC: Conor Dooley <conor@...nel.org>, <ardb@...nel.org>,
<palmer@...belt.com>, <paul.walmsley@...ive.com>,
<aou@...s.berkeley.edu>, <linux-riscv@...ts.infradead.org>,
<rminnich@...il.com>, <mark.rutland@....com>,
<lpieralisi@...nel.org>, <rafael@...nel.org>, <lenb@...nel.org>,
<jdelvare@...e.com>, <yc.hung@...iatek.com>,
<angelogioacchino.delregno@...labora.com>,
<allen-kh.cheng@...iatek.com>,
<pierre-louis.bossart@...ux.intel.com>,
<tinghan.shen@...iatek.com>, <linux-kernel@...r.kernel.org>,
<linux-acpi@...r.kernel.org>, <geshijian@...edance.com>,
<weidong.wd@...edance.com>
Subject: Re: [External] Re: [PATCH v2 2/3] firmware: introduce FFI for SMBIOS
entry.
On Mon, Jul 03, 2023 at 08:41:30PM +0800, 运辉崔 wrote:
> On Mon, Jul 3, 2023 at 4:36 PM Conor Dooley <conor.dooley@...rochip.com> wrote:
> > On Mon, Jul 03, 2023 at 04:23:53PM +0800, 运辉崔 wrote:
> > > > > +FDT FIRMWARE INTERFACE (FFI)
> > > > > +M: Yunhui Cui cuiyunhui@...edance.com
> > > > > +S: Maintained
> > > > > +F: drivers/firmware/ffi.c
> > > > > +F: include/linux/ffi.h
> > > >
> > > > Are you going to apply patches for this, or is someone else?
> > > Yes, it will be used by patch 3/3.
> >
> > That's not what I asked :(
>
> Sorry, ok, what do you want to ask?
Who is going to apply patches for drivers/firmware/ffi*?
> > > > > static void __init dmi_scan_machine(void)
> > > > > @@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
> > > > > char __iomem *p, *q;
> > > > > char buf[32];
> > > > >
> > > > > +#ifdef CONFIG_FDT_FW_INTERFACE
> > > > > + if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> > > >
> > > > "dmi_sacn_smbios"
> > > >
> > > > > + goto error;
> > > > > +#endif
> > > >
> > > > Does this not mean that if FDT_FW_INTERFACE is enabled, but the platform
> > > > wants to use EFI, it won't be able to? The `goto error;` makes this look
> > > > mutually exclusive to my efi-unaware eyes.
> > >
> > > If you have enabled FFI, then if something goes wrong, you should goto error.
> > > Just like the origin code:
> > > if (efi_enabled(EFI_CONFIG_TABLES)) {
> > > if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> > > goto error;
> > > } else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
> > > p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
> > > if (p == NULL)
> > > goto error;
> >
> > Does this not make FFI and EFI mutually exclusive Kconfig options?
> > Suppose you are on a system that does not implement FFI, but does
> > implement EFI - what's going to happen then?
> > AFAICT, dmi_sacn_smbios(ffi.smbios3, ffi.smbios) will fail & you'll do a
> > `goto error` & skip the EFI code. What am I missing?
>
> Code is not intended to be mutually exclusive, get the correct value and return,
> The code is going to be changed to this:
>
> #ifdef CONFIG_FDT_FW_INTERFACE
Ideally, these would be IS_ENABLED() instead of #ifdef - but if you copy
what EFI does, then you don't need either, as there will always be an
ffi_enabled() defined.
> if (ffi_enabled(FFI_CONFIG_TABLES)) {
I don't know what this function is, but this code seems like a step in
the right direction.
> if (!dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> return;
> }
> #endif
Thanks,
Conor.
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists