On Sat, Dec 24, 2022 at 6:04 AM Paul Barker <paul.bar...@sancloud.com> wrote: > > On 20/12/2022 15:55, Rob Herring wrote: > > On Wed, Nov 23, 2022 at 05:50:06PM +0000, Paul Barker wrote: > >> Add properties to the Authenta SPI flash device node to enable access by > >> a UEFI application using a fixed GUID. > >> > >> Signed-off-by: Paul Barker <paul.bar...@sancloud.com> > >> --- > >> arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi | 13 ++++++++++--- > >> arch/arm/dts/am335x-sancloud-bbe-lite.dts | 2 +- > >> 2 files changed, 11 insertions(+), 4 deletions(-) > >> > >> diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi > >> b/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi > >> index 01c105ebb383..6c4ff67f9a4b 100644 > >> --- a/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi > >> +++ b/arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi > >> @@ -38,7 +38,14 @@ > >> > >> &spi0 { > >> u-boot,dm-pre-reloc; > >> - channel@0 { > >> - u-boot,dm-pre-reloc; > >> - }; > >> +}; > >> + > >> +&authenta_flash { > >> + u-boot,dm-pre-reloc; > >> + > >> + u-boot,uefi-spi-vendor = "micron"; > >> + u-boot,uefi-spi-part-number = "mt25ql128abb"; > > > > Looks like a compatible string. Yet, the flash node compatible string, > > micron,spi-authenta, is not documented (though in use for spidev). So > > use a compatible string for the flash that is specific to the flash > > model. I assume there is some reason the specific model is needed? > > For context, the UEFI Platform Initialization (PI) spec defines > EFI_SPI_PART, EFI_SPI_PERIPHERAL and EFI_SPI_IO_PROTOCOL structures. > I'm referencing v1.7 Errata A. See https://uefi.org/specifications for > downloads. > > The EFI_SPI_PART structure has "Vendor" and "PartNumber" fields. We need > something to put in those fields and the device tree is the best place > to store the data. These properties are in the `-u-boot.dtsi` file so > they won't be submitted to the Linux kernel.
Can't you just split the `micron,mt25ql128abb` compatible string to populate those? > The `micron,spi-authenta` compatible string is unfortunate in my > opinion. I plan to replace this with two entries, `micron,mt25ql128abb` > and `jedec,spi-nor`, once we've got MTD support for the Authenta parts > merged into the mainline kernel. That work is currently on hold, I'll > be working with Micron in the new year to unblock it. We could submit > a change to the compatible property in the meantime if you think it's > worth getting a temporary improvement in before the MTD changes are > done. No need to wait on driver changes to add compatible strings. Anyone that says otherwise is wrong. Looking at the datasheet, maybe `micron,mt25ql128abb` is too specific. Perhaps 'abb' or just the last 'b' can be dropped? It's a trade off of lots of compatible strings vs. whether you might need to distinguish the parts (only before you do jedec discovery). > > >> + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */ > >> + u-boot,uefi-spi-io-guid = [30 67 12 77 ca a4 86 43 > >> + b3 41 88 1f e1 8e 7f 7d]; > > > > We need to define first in the DT spec the format for GUIDs. I don't > > think there are any existing cases though some have been proposed. IMO, > > I would make this a string instead. The byte array is not that > > readable with its little endian order. A GUID as a string is readily > > identifiable as a GUID. > > I see there is a `uuid_str_to_bin()` function in u-boot so I could make > this a string property and convert it to a binary GUID at runtime. This > would make the dts more readable so I'll make the change for v6 of this > series. > > > Why is this u-boot specific? Another UEFI implementation doesn't need > > the GUID? > > Each EFI_SPI_IO_PROTOCOL instance needs its own GUID so that it can > be located at runtime. These GUIDs are not pre-defined by the spec, > instead an arbitrary per-device GUID is used and is stored in the > SpiPeripheralDriverGuid field of the relevant EFI_SPI_PERIPHERAL > instance. We could randomly generate these GUIDs at runtime since a UEFI > application can walk the tree of SPI busses and peripherals, looking > for a match in the Vendor and PartNumber fields defined above, to get > the appropriate GUID. However, storing a known value in the device tree > allows a UEFI application to just lookup the GUID without needing to > walk the SPI tree. Okay, then I guess my next question is why is it SPI IO protocol specific? I'd assume the UEFI spec would use this for any h/w protocol. Generating GUIDs at runtime seems like the right way though. We simply can't populate DT with every client's method of identifying a given instance. That simply doesn't scale. Rob