Re: [PATCH v5 3/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export

Paul Barker
2023-01-11T12:20:00Z

On 03/01/2023 19:27, Rob Herring wrote:
> On Sat, Dec 24, 2022 at 6:04 AM Paul Barker <paul.barker@sancloud.com> wrote:
>> 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?

Yes, we can go that route. I'll prepare a patch to add
`micron,mt25ql128` to the compatibile strings for this node and submit
that to both the kernel & u-boot. Then I'll add code to split that into
vendor & part number at runtime.

>>> 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.

Sadly, common sense assumptions and the UEFI spec do not go together.
There at least 3 different ways this is handled in UEFI:

* For SD/MMC, the UEFI spec defines a single GUID for the I/O protocol
and the API takes a slot number to determine which actual device to
operate on. The `This` argument is assumed to point to a global
instance of EFI_SD_MMC_PASS_THRU_PROTOCOL.

* For NVMe, it's the same as above but with a namespace ID instead of a
slot number.

* For ATA, PCI, SCSI, USB, etc, it looks to be a similar story.

* For I2C, there is a fixed GUID for the I/O protocol but the API takes
an additional GUID to identify the device to operate on. So a unique
GUID must be allocated for each device, but only one instance of
EFI_I2C_IO_PROTOCOL is needed.

* For SPI, the I/O protocol has no fixed GUID and it's expected that
there is a separate EFI_SPI_IO_PROTOCOL instance with a unique GUID
for each device a the SPI bus. The API does not take any arguments to
identify the SPI bus or chip select, instead the `This` argument
points to the specific instance of EFI_SPI_IO_PROTOCOL for the device
we want to operate on. There is a separate protocol,
EFI_SPI_CONFIGURATION_PROTOCOL, with a fixed GUID, that allows the
SPI bus to be walked.

> 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.

Ok. We can re-write our application code to walk the SPI bus nodes
looking for the appropriate Vendor & PartNumber pair, then lookup the
GUID for that device. That will allow us to generate the GUID
dynamically at runtime. We will then be relying solely on the compatible
property in the device tree to fill in the Vendor & PartNumber fields.

> Rob

Thanks for your feedback Rob. It's been good to see another perspective
on this.

--
Paul Barker
Principal Software Engineer
SanCloud Ltd

e: paul.barker@sancloud.com
w: https://sancloud.com/