From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 80A84C3DA7D for ; Tue, 3 Jan 2023 19:27:57 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B26B685309; Tue, 3 Jan 2023 20:27:54 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="oKLhCYrH"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id EBEC685478; Tue, 3 Jan 2023 20:27:52 +0100 (CET) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9D7CF8521D for ; Tue, 3 Jan 2023 20:27:49 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=robh@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D9A4B614EC for ; Tue, 3 Jan 2023 19:27:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 49E45C43392 for ; Tue, 3 Jan 2023 19:27:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1672774067; bh=9oGPBD+B5OPgKmnzmX3VqlgQcD38XSYyXed2izG3Mfc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=oKLhCYrHR6wVVdCpM1WbBVP6nwLuzQ9DkzCoOSgK0NKrteokRldELWD/rSjlDWDJH EKsuj/UXBml93iD4O5slrQRiKd0qJsjl90Iezd0rNjKWC4s1OHuGLErZ5fzR/QhOWl X5XYSiQ8om5I4HzDYay4kHYdxoJe0P+dwTRtMDUIbBBjkVaIVzYJLOfYrShriNr8Zq RldyIQbTtxdN75mHUY/JOyvmcrQgxBAo39iTbFACu3sGYFwQ1rItvS3ytE2TfZ05+s eF4GKD5EYavtug+5IT2tAhyLmqpIBBrSd0TYjRAM9bfH4sNEuvkf/KAAwz80ZCUACe HutOkmvQXqGJQ== Received: by mail-ua1-f49.google.com with SMTP id f15so3667163uaq.8 for ; Tue, 03 Jan 2023 11:27:47 -0800 (PST) X-Gm-Message-State: AFqh2koeLV8ucZHKmHkVzmwbR8Tu9oAhxDHlSx47ir5p7H5owUNLkIS8 MSVinPVxBoBy8hoyY+CD1kZJAAX71ohozd7Ocw== X-Google-Smtp-Source: AMrXdXs25ghNGWboznk7KSdBE1Ug0evX0QS8yyq7BkynizoiR9L+WRTRffpn4kTehRbhke0b8I4iEiIF+tHxHj/4GJg= X-Received: by 2002:ab0:3a8d:0:b0:419:678:cf31 with SMTP id r13-20020ab03a8d000000b004190678cf31mr4110167uaw.63.1672774066191; Tue, 03 Jan 2023 11:27:46 -0800 (PST) MIME-Version: 1.0 References: <20221123175006.4080122-1-paul.barker@sancloud.com> <20221123175006.4080122-4-paul.barker@sancloud.com> <20221220155518.GA583248-robh@kernel.org> In-Reply-To: From: Rob Herring Date: Tue, 3 Jan 2023 13:27:34 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v5 3/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export To: Paul Barker Cc: u-boot@lists.denx.de, Simon Glass , Tom Rini , Heinrich Schuchardt , Ilias Apalodimas , Jagan Teki Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Sat, Dec 24, 2022 at 6:04 AM Paul Barker 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 > >> --- > >> 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