Re: [PATCH v3 04/13] firmware: scmi: framework for installing additional protocols
On Fri, Sep 08, 2023 at 02:01:52PM +, Etienne CARRIERE wrote:
>
> > From: AKASHI Takahiro
> > Sent: Friday, September 8, 2023 04:51
> >
> > This framework allows SCMI protocols to be installed and bound to the agent
> > so that the agent can manage and utilize them later.
> >
> > Signed-off-by: AKASHI Takahiro
> > Reviewed-by: Simon Glass
> > Reviewed-by: Etienne Carriere
> > ---
> > v3
> > * move "per_device_plat_auto" from a earlier patch
> > * fix comments in "scmi_agent_priv"
> > * modify an order of include files in scmi_agent.h
> > v2
> > * check for availability of protocols
> > ---
> > drivers/firmware/scmi/scmi_agent-uclass.c | 101 +-
> > include/scmi_agent-uclass.h | 15 +++-
> > include/scmi_agent.h | 14 +++
> > 3 files changed, 126 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
> > b/drivers/firmware/scmi/scmi_agent-uclass.c
> > index 94e54eeb066b..e823d105a3eb 100644
> > --- a/drivers/firmware/scmi/scmi_agent-uclass.c
> > +++ b/drivers/firmware/scmi/scmi_agent-uclass.c
> > @@ -38,6 +38,86 @@ static const struct error_code scmi_linux_errmap[] = {
> > { .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, },
> > };
> >
> > +/*
> > + * NOTE: The only one instance should exist according to
> > + * the current specification and device tree bindings.
> > + */
> > +struct udevice *scmi_agent;
> > +
> > +struct udevice *scmi_get_protocol(struct udevice *dev,
> > + enum scmi_std_protocol id)
> > +{
> > + struct scmi_agent_priv *priv;
> > + struct udevice *proto;
> > +
> > + priv = dev_get_uclass_plat(dev);
> > + if (!priv) {
> > + dev_err(dev, "No priv data found\n");
> > + return NULL;
> > + }
> > +
> > + switch (id) {
> > + case SCMI_PROTOCOL_ID_CLOCK:
> > + proto = priv->clock_dev;
> > + break;
> > + case SCMI_PROTOCOL_ID_RESET_DOMAIN:
> > + proto = priv->resetdom_dev;
> > + break;
> > + case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
> > + proto = priv->voltagedom_dev;
> > + break;
> > + default:
> > + dev_err(dev, "Protocol not supported\n");
> > + proto = NULL;
> > + break;
> > + }
> > + if (proto && device_probe(proto))
> > + dev_err(dev, "Probe failed\n");
> > +
> > + return proto;
> > +}
> > +
> > +/**
> > + * scmi_add_protocol - add protocol to agent
> > + * @dev: SCMI agent device
> > + * @proto_id: SCMI protocol ID
> > + * @proto: SCMI protocol device
> > + *
> > + * Associate the protocol instance, @proto, to the agent, @dev,
> > + * for later use.
> > + *
> > + * Return: 0 on success, error code otherwise
> > + */
> > +static int scmi_add_protocol(struct udevice *dev,
> > + enum scmi_std_protocol proto_id,
> > + struct udevice *proto)
> > +{
> > + struct scmi_agent_priv *priv;
> > +
> > + priv = dev_get_uclass_plat(dev);
> > + if (!priv) {
> > + dev_err(dev, "No priv data found\n");
> > + return -ENODEV;
> > + }
> > +
> > + switch (proto_id) {
> > + case SCMI_PROTOCOL_ID_CLOCK:
> > + priv->clock_dev = proto;
> > + break;
> > + case SCMI_PROTOCOL_ID_RESET_DOMAIN:
> > + priv->resetdom_dev = proto;
> > + break;
> > + case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
> > + priv->voltagedom_dev = proto;
> > + break;
> > + default:
> > + dev_err(dev, "Protocol not supported\n");
> > + return -EPROTO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int scmi_to_linux_errno(s32 scmi_code)
> > {
> > int n;
> > @@ -164,12 +244,14 @@ int devm_scmi_process_msg(struct udevice *dev, struct
> > scmi_msg *msg)
> > */
> > static int scmi_bind_protocols(struct udevice *dev)
> > {
> > + struct udevice *agent;
>
> nit: my build reported 'agent' here is not used in the scope of this patch.
> Strictly speaking, it should be brought by patch v3 05/13.
> ("firmware: scmi: install base protocol to SCMI agent")
You're right.
I will move this variable declaration to patch#5.
-Takahiro Akashi
> BR,
> etienne
>
> > (snip)
Re: [PATCH v3 01/13] scmi: refactor the code to hide a channel from devices
Hi Etienne,
Thank you gain for your review.
On Fri, Sep 08, 2023 at 02:01:51PM +, Etienne CARRIERE wrote:
> Hello Akashi-san,
>
> >
> > From: AKASHI Takahiro
> > Sent: Friday, September 8, 2023 04:51
> >
> > The commit 85dc58289238 ("firmware: scmi: prepare uclass to pass channel
> > reference") added an explicit parameter, channel, but it seems to make
> > the code complex.
> >
> > Hiding this parameter will allow for adding a generic (protocol-agnostic)
> > helper function, i.e. for PROTOCOL_VERSION, in a later patch.
> >
> > Signed-off-by: AKASHI Takahiro
> > Reviewed-by: Simon Glass
> > ---
> > v3
> > * fix an issue on ST board (reported by Etienne)
> > by taking care of cases where probed devices are children of
> > SCMI protocol device (i.e. clock devices under CCF)
> > See find_scmi_protocol_device().
> > * move "per_device_plato_auto" to a succeeding right patch
> > v2
> > * new patch
> > ---
> > drivers/clk/clk_scmi.c | 27 +---
> > drivers/firmware/scmi/scmi_agent-uclass.c | 160 +++---
> > drivers/power/regulator/scmi_regulator.c | 27 +---
> > drivers/reset/reset-scmi.c | 19 +--
> > include/scmi_agent.h | 15 +-
> > 5 files changed, 103 insertions(+), 145 deletions(-)
> >
> > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> > index d172fed24c9d..34a49363a51a 100644
> > --- a/drivers/clk/clk_scmi.c
> > +++ b/drivers/clk/clk_scmi.c
> > @@ -13,17 +13,8 @@
> > #include
> > #include
> >
> > -/**
> > - * struct scmi_clk_priv - Private data for SCMI clocks
> > - * @channel: Reference to the SCMI channel to use
> > - */
> > -struct scmi_clk_priv {
> > - struct scmi_channel *channel;
> > -};
> > -
> > static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(dev);
> > struct scmi_clk_protocol_attr_out out;
> > struct scmi_msg msg = {
> > .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > @@ -33,7 +24,7 @@ static int scmi_clk_get_num_clock(struct udevice *dev,
> > size_t *num_clocks)
> > };
> > int ret;
> >
> > - ret = devm_scmi_process_msg(dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(dev, &msg);
> > if (ret)
> > return ret;
> >
> > @@ -44,7 +35,6 @@ static int scmi_clk_get_num_clock(struct udevice *dev,
> > size_t *num_clocks)
> >
> > static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char
> > **name)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(dev);
> > struct scmi_clk_attribute_in in = {
> > .clock_id = clkid,
> > };
> > @@ -59,7 +49,7 @@ static int scmi_clk_get_attibute(struct udevice *dev, int
> > clkid, char **name)
> > };
> > int ret;
> >
> > - ret = devm_scmi_process_msg(dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(dev, &msg);
> > if (ret)
> > return ret;
> >
> > @@ -70,7 +60,6 @@ static int scmi_clk_get_attibute(struct udevice *dev, int
> > clkid, char **name)
> >
> > static int scmi_clk_gate(struct clk *clk, int enable)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> > struct scmi_clk_state_in in = {
> > .clock_id = clk->id,
> > .attributes = enable,
> > @@ -81,7 +70,7 @@ static int scmi_clk_gate(struct clk *clk, int enable)
> > in, out);
> > int ret;
> >
> > - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(clk->dev, &msg);
> > if (ret)
> > return ret;
> >
> > @@ -100,7 +89,6 @@ static int scmi_clk_disable(struct clk *clk)
> >
> > static ulong scmi_clk_get_rate(struct clk *clk)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> > struct scmi_clk_rate_get_in in = {
> > .clock_id = clk->id,
> > };
> > @@ -110,7 +98,7 @@ static ulong scmi_clk_get_rate(struct clk *clk)
> > in, out);
> > int ret;
> >
> > - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(clk->dev, &msg);
> > if (ret < 0)
> > return ret;
> >
> > @@ -123,7 +111,6 @@ static ulong scmi_clk_get_rate(struct clk *clk)
> >
> > static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> > struct scmi_clk_rate_set_in in = {
> > .clock_id = clk->id,
> > .flags = SCMI_CLK_RATE_ROUND_CLOSEST,
> > @@ -136,7 +123,7 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong
> > rate)
> > in, out);
> > int ret;
> >
> > -
Re: [PATCH 30/32] fdt: Allow the devicetree to come from a bloblist
On 9/11/23 08:17, Ilias Apalodimas wrote: Hi Simon, On Sun, 10 Sept 2023 at 22:14, Simon Glass wrote: Hi Ilias, On Mon, 4 Sept 2023 at 03:31, Ilias Apalodimas wrote: Hi Simon, On Fri, 1 Sept 2023 at 18:51, Simon Glass wrote: Hi Ilias, On Fri, 1 Sept 2023 at 01:50, Ilias Apalodimas wrote: [...] +config OF_BLOBLIST + bool "DTB is provided by a bloblist" + help + Select this to read the devicetree from the bloblist. This allows + using a bloblist to transfer the devicetree between U-Boot phases. + The devicetree is stored in the bloblist by an early phase so that + U-Boot can read it. + I dont think this is a good idea. We used to have 4-5 different options here, which we tried to clean up and ended up with two very discrete options. Why do we have to reintroduce a new one? Doesn't that bloblist have a header of some sort? The bloblist literally comes from a previous stage bootloader which is what OF_BOARD is here for. So why can't we just read the header and figure out if the magic of the bloblist matches? No, OF_BOARD is a hack to allow boards to do what they like with the FDT. This patch is a standard mechanism to pass the DT from one firmware phase to the next. We have spent quite a bit of time creating a spec for it, and we should use it. Where exactly am I objecting using the spec? Can you please re-read my email? I am actually pointing out we should use the spec *properly*. So instead of having a Kconfig option for the DT, which is pretty pointless, we should parse the bloblist. If the header defined by the *spec* is found, we should just search for the DT in there. What you are doing here, is take the spec, pick a very specific item that the list contains, and create a Kconfig option out of it. Which basically ignores the discoverable options of the bloblist. For example, that bloblist can also contain an entry to a TPM eventlog. Should we start creating Kconfig options for all the firmware handoff entries that are defined on that spec? OK so that is a different thing. What should it do if it expects to find a bloblist but cannot? I want it to throw an error, because I am trying to make the boot deterministic. What do you think? That's fine by me. You can even put that under IS_ENABLED for the bloblist inside the existing OF_BOARD check. So I was thinking - If no bloblist is required in Kconfig options we do the hacks we used to - if bloblist is selected and the config option is OF_BOARD, throw an error and mention that the previous stage loader should hand over a DT Is that what you had in mind? Yes, that sounds good to me. But I still think we need an OF_BLOBLIST option to control whether the devicetree comes from there. Otherwise we will end up with people using OF_BOARD to work around it. We also have the SPL case which does not pass the DT in a bloblist...in fact SPL typically doesn't even have the full DT. Wouldn't IS_ENABLED(BLOBLIST) || IS_ENABLED(SPL_BLOBLIST) be enough? Inside the OF_BOARD portion of the function, search for a bloblist if the option is enabled. If you can't find a bloblist and the DT within that bloblist, then error out Just my 2c here. I think all options should be possible to disable. It means I can imagine to disable u-boot not to take care about DT provided from previous stage. The same is for TPM event log. IMHO every stage should have an option to simply ignore data pass from previous stage. I don't really mind how it is done but there should be an option to by purpose say to ignore the part of pass data. Regarding DT part. We are experimenting with TF-A injecting for example DDR memory reservation to DT. Injection is working properly if you are using single DT but in SOM case we are using FIT image with a lot of DTBs inside (issue with checksums calculation). I see couple of platforms (IIRC renesas/imx) which are using DT overlays and passing it via specific registers. For these using bloblist might be right way to go. I wasn't aware about the whole fdt bloblist discussion but based on my experiments you can create multiple FDT entries that's why I expect that there could be DT overlays from different stages. And I even think that all of them can be overlays without base DT which can be select later on. Thanks, Michal
Re: [PATCH 30/32] fdt: Allow the devicetree to come from a bloblist
Hi Simon, On Sun, 10 Sept 2023 at 22:14, Simon Glass wrote: > > Hi Ilias, > > On Mon, 4 Sept 2023 at 03:31, Ilias Apalodimas > wrote: > > > > Hi Simon, > > > > On Fri, 1 Sept 2023 at 18:51, Simon Glass wrote: > > > > > > Hi Ilias, > > > > > > On Fri, 1 Sept 2023 at 01:50, Ilias Apalodimas > > > wrote: > > > > > > > > [...] > > > > > > > > > > > > > > > > > +config OF_BLOBLIST > > > > > > > + bool "DTB is provided by a bloblist" > > > > > > > + help > > > > > > > + Select this to read the devicetree from the bloblist. > > > > > > > This allows > > > > > > > + using a bloblist to transfer the devicetree between > > > > > > > U-Boot phases. > > > > > > > + The devicetree is stored in the bloblist by an early > > > > > > > phase so that > > > > > > > + U-Boot can read it. > > > > > > > + > > > > > > > > > > > > I dont think this is a good idea. We used to have 4-5 different > > > > > > options > > > > > > here, which we tried to clean up and ended up with two very discrete > > > > > > options. Why do we have to reintroduce a new one? Doesn't that > > > > > > bloblist > > > > > > have a header of some sort? The bloblist literally comes from a > > > > > > previous > > > > > > stage bootloader which is what OF_BOARD is here for. So why can't > > > > > > we just > > > > > > read the header and figure out if the magic of the bloblist matches? > > > > > > > > > > No, OF_BOARD is a hack to allow boards to do what they like with the > > > > > FDT. > > > > > > > > > > This patch is a standard mechanism to pass the DT from one firmware > > > > > phase to the next. We have spent quite a bit of time creating a spec > > > > > for it, and we should use it. > > > > > > > > Where exactly am I objecting using the spec? Can you please re-read > > > > my email? > > > > I am actually pointing out we should use the spec *properly*. So > > > > instead of having a Kconfig option for the DT, which is pretty > > > > pointless, we should parse the bloblist. If the header defined by > > > > the *spec* is found, we should just search for the DT in there. > > > > What you are doing here, is take the spec, pick a very specific item > > > > that the list contains, and create a Kconfig option out of it. Which > > > > basically ignores the discoverable options of the bloblist. For > > > > example, that bloblist can also contain an entry to a TPM eventlog. > > > > Should we start creating Kconfig options for all the firmware handoff > > > > entries that are defined on that spec? > > > > > > OK so that is a different thing. What should it do if it expects to find > > > a bloblist but cannot? I want it to throw an error, because I am trying > > > to make the boot deterministic. What do you think? > > > > That's fine by me. You can even put that under IS_ENABLED for the > > bloblist inside the existing OF_BOARD check. So I was thinking > > - If no bloblist is required in Kconfig options we do the hacks we used to > > - if bloblist is selected and the config option is OF_BOARD, throw an > > error and mention that the previous stage loader should hand over a DT > > > > Is that what you had in mind? > > Yes, that sounds good to me. > > But I still think we need an OF_BLOBLIST option to control whether the > devicetree comes from there. > Otherwise we will end up with people > using OF_BOARD to work around it. We also have the SPL case which does > not pass the DT in a bloblist...in fact SPL typically doesn't even > have the full DT. Wouldn't IS_ENABLED(BLOBLIST) || IS_ENABLED(SPL_BLOBLIST) be enough? Inside the OF_BOARD portion of the function, search for a bloblist if the option is enabled. If you can't find a bloblist and the DT within that bloblist, then error out Thanks /Ilias > > [..] > > Regards, > Simon
Re: [RFC 6/6] test: dm: add SCMI pinctrl test
On Sun, Sep 10, 2023 at 01:13:30PM -0600, Simon Glass wrote:
> Hi AKASHI,
>
> On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> wrote:
> >
> > In this test, SCMI-based pinmux feature is exercised.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > test/dm/scmi.c | 62 ++
> > 1 file changed, 62 insertions(+)
> >
> > diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> > index 423b6ef70b29..ca5a2e9c781e 100644
> > --- a/test/dm/scmi.c
> > +++ b/test/dm/scmi.c
> > @@ -21,6 +21,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -395,3 +396,64 @@ static int dm_test_scmi_voltage_domains(struct
> > unit_test_state *uts)
> > return release_sandbox_scmi_test_devices(uts, dev);
> > }
> > DM_TEST(dm_test_scmi_voltage_domains, UT_TESTF_SCAN_FDT);
> > +
> > +/*
> > + * This part is derived from test/dm/pinmux.c, So
> > + *
> > + * Copyright (C) 2020 Sean Anderson
> > + */
> > +
> > +static char buf[64];
> > +#define test_muxing(selector, expected) do { \
> > + ut_assertok(pinctrl_get_pin_muxing(dev, selector, buf,
> > sizeof(buf))); \
> > + ut_asserteq_str(expected, (char *)&buf); \
>
> Can you instead make this a function?
Sure, but please note that the whole scheme was borrowed
from test/dm/pinmux.c.
> > +} while (0)
> > +
> > +#define test_name(selector, expected) do { \
> > + ut_assertok(pinctrl_get_pin_name(dev, selector, buf, sizeof(buf)));
> > \
> > + ut_asserteq_str(expected, (char *)&buf); \
> > +} while (0)
> > +
> > +static int dm_test_scmi_pinmux(struct unit_test_state *uts)
> > +{
> > + struct udevice *dev;
> > +
> > + ut_assertok(uclass_get_device_by_name(UCLASS_PINCTRL, "protocol@19",
> > + &dev));
> > + ut_assertok(pinctrl_select_state(dev, "default"));
> > + test_muxing(0, "");
>
> ut_assertok(check_muxing(uts, ...));
Assertion for
> > + test_muxing(1, "");
> > + test_muxing(2, "I2S.");
> > + test_muxing(3, "I2S.");
> > + test_muxing(4, "I2S.");
> > + test_muxing(5, "GPIO bias-pull-up.");
> > + test_muxing(6, "GPIO drive-open-drain output-mode output-value.");
> > + test_muxing(7, "GPIO bias-pull-down input-mode.");
> > + test_muxing(8, "GPIO bias-disable.");
this chunk of code?
It is not clear to me what is the advantage, though.
-Takahiro Akashi
> > +
> > + ut_assertok(pinctrl_select_state(dev, "alternate"));
> > + test_muxing(0, "I2C drive-open-drain.");
> > + test_muxing(1, "I2C drive-open-drain.");
> > + test_muxing(2, "SPI.");
> > + test_muxing(3, "SPI.");
> > + test_muxing(4, "SPI.");
> > + test_muxing(5, "CS bias-pull-up.");
> > + test_muxing(6, "CS drive-open-drain output-mode output-value.");
> > + test_muxing(7, "GPIO bias-pull-down input-mode.");
> > + test_muxing(8, "GPIO bias-disable.");
> > +
> > + ut_assertok(pinctrl_select_state(dev, "0"));
> > + test_muxing(0, "I2C drive-open-drain.");
> > + test_muxing(1, "I2C drive-open-drain.");
> > + test_muxing(2, "I2S.");
> > + test_muxing(3, "I2S.");
> > + test_muxing(4, "I2S.");
> > + test_muxing(5, "GPIO bias-pull-up.");
> > + test_muxing(6, "GPIO drive-open-drain output-mode output-value.");
> > + test_muxing(7, "GPIO bias-pull-down input-mode.");
> > + test_muxing(8, "GPIO bias-disable.");
> > +
> > + return 0;
> > +}
> > +
> > +DM_TEST(dm_test_scmi_pinmux, UT_TESTF_SCAN_FDT);
> > --
> > 2.34.1
> >
>
> Regards,
> Simon
Re: [RFC 4/6] gpio: add scmi driver based on pinctrl
Hi Simon,
On Sun, Sep 10, 2023 at 01:13:24PM -0600, Simon Glass wrote:
> Hi,
>
> On Thu, 7 Sept 2023 at 22:32, AKASHI Takahiro
> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
> > > Hi AKASHI,
> > >
> > > On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> > > wrote:
> > > >
> > > > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > > > gpio devices exposed by SCMI firmware (server).
> > > >
> > > > Signed-off-by: AKASHI Takahiro
> > > > ---
> > > > drivers/pinctrl/pinctrl-scmi.c | 544 -
> > > > 1 file changed, 539 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/pinctrl/pinctrl-scmi.c
> > > > b/drivers/pinctrl/pinctrl-scmi.c
> > > > index 3ebdad57b86c..73d385bdbfcc 100644
> > > > --- a/drivers/pinctrl/pinctrl-scmi.c
> > > > +++ b/drivers/pinctrl/pinctrl-scmi.c
> > > > @@ -11,21 +11,20 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > +#include
> > > > +#include
> > > > #include
> > > >
> > > [..]
> > >
> > > > +
> > > > +U_BOOT_DRIVER(scmi_gpio) = {
> > > > + .name = "scmi_gpio",
> > > > + .id = UCLASS_GPIO,
> > > > + .of_match = scmi_gpio_ids,
> > > > + .of_to_plat = scmi_gpio_probe,
> > > > + .ops = &scmi_gpio_ops,
> > > > + .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
> > > > +};
> > > > +
> > > > +/**
> > > > + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
> > > > + * @parent:SCMI pinctrl device
> > > > + *
> > > > + * Create a pinctrl-controlled gpio device
> > > > + *
> > > > + * Return: 0 on success, error code on failure
> > > > + */
> > > > +static int scmi_gpiochip_register(struct udevice *parent)
> > > > +{
> > > > + ofnode node;
> > > > + struct driver *drv;
> > > > + struct udevice *gpio_dev;
> > > > + int ret;
> > > > +
> > > > + /* TODO: recovery if failed */
> > > > + dev_for_each_subnode(node, parent) {
> > > > + if (!ofnode_is_enabled(node))
> > > > + continue;
> > >
> > > Can we not rely on the normal driver model binding to bind these
> > > devices? Why does it need to be done manually here?
> >
> > First, please take a look at the cover letter.
> > In this RFC, I basically assume two patterns of DT bindings,
> > (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
> >
> > In (B), a DT node as a gpio device, which is essentially a child
> > of pinctrl device, is located *under* a pinctrl device. It need
> > to be probed manually as there is no implicit method to enumerate
> > it as a DM device automatically.
>
> You could add a post_bind() method to the pinctrl uclass to call
> dm_scan_fdt_dev() as we do with TPM.
Okay, but in that case, defining "compatible" property will become
mandatory, while I invented (B) so that we don't need extra bindings.
Please note that no discussion started about the binding of gpio devices.
-Takahiro Akashi
> >
> > On the other hand, in (A), the same node can be put anywhere in a DT
> > as it contains a "compatible" property to identify itself.
> > So if we want to only support (A), scmi_gpiochip_register() and
> > scmi_pinctrl_bind() are not necessary.
> >
> > Since there is no discussion about bindings for GPIO managed by SCMI
> > pinctrl device yet on the kernel side, I have left two solutions
> > in this RFC.
>
>
> OK.
>
> [..]
>
> Regards,
> Simon
Re: [PATCH] ufs: ufs-renesas: Drop include common.h
On 9/9/23 8:24 AM, Marek Vasut wrote: The "#include " is being phased out in favor of more fine grained header management, i.e. ideally include a subset of headers that are really needed. Remove it from this driver. Signed-off-by: Marek Vasut --- Cc: Bhupesh Sharma Cc: Neha Malcom Francis Cc: Tom Rini --- drivers/ufs/ufs-renesas.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/ufs/ufs-renesas.c b/drivers/ufs/ufs-renesas.c index f6086050cde..ae05bdc8102 100644 --- a/drivers/ufs/ufs-renesas.c +++ b/drivers/ufs/ufs-renesas.c @@ -6,7 +6,6 @@ */ #include -#include #include #include #include Reviewed-by: Bhupesh Sharma Thanks.
Re: [RFC 5/6] firmware: scmi: add pseudo pinctrl protocol support on sandbox
On Sun, Sep 10, 2023 at 01:13:28PM -0600, Simon Glass wrote:
> Hi AKASHI,
>
> On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> wrote:
> >
> > With this patch, sandbox SCMI agent can handle pinctrl protocol.
> > This feature is used in an unit test for SCMI pinctrl.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > arch/sandbox/dts/test.dts | 115
> > drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +
> > 2 files changed, 837 insertions(+)
> >
> > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > index dc0bfdfb6e4b..d2ddea801995 100644
> > --- a/arch/sandbox/dts/test.dts
> > +++ b/arch/sandbox/dts/test.dts
> > @@ -723,9 +723,124 @@
> > };
> > };
> > };
> > +
> > + pinctrl_scmi: protocol@19 {
> > + reg = <0x19>;
> > +
> > + pinctrl-names = "default","alternate";
> > + pinctrl-0 = <&scmi_pinctrl_gpios>,
> > <&scmi_pinctrl_i2s>;
> > + pinctrl-1 = <&scmi_pinctrl_spi>,
> > <&scmi_pinctrl_i2c>;
> > +
> > +#if 0
>
> Are these alternatives that you are testing?
I should have had more explanation.
Yes, as I mentioned in the cover letter (and patch#4), there
are two alternatives for defining SCMI pinctrl based gpio devices.
Actually, this "#if" corresponds to the case (B) where gpio node
is located under scmi node.
Since I didn't come up with any good way to switch two cases dynamically
in the test, I had modified this option manually.
(I left two "#if" here as the patch was an RFC, any way.)
Thanks,
-Takahiro Akashi
>
> The bindings look OK to me - is this how it is done in Linux?
>
> Regards,
> Simon
Re: [RFC 3/6] pinctrl: add scmi driver
Hi Simon,
On Sun, Sep 10, 2023 at 01:13:27PM -0600, Simon Glass wrote:
> Hi AKASHI,
>
> On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> wrote:
> >
> > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > pinctrl devices exposed by SCMI firmware (server).
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > drivers/pinctrl/Kconfig| 11 +
> > drivers/pinctrl/Makefile | 1 +
> > drivers/pinctrl/pinctrl-scmi.c | 537 +
> > 3 files changed, 549 insertions(+)
> > create mode 100644 drivers/pinctrl/pinctrl-scmi.c
>
>
> [..]
>
> +
> > +U_BOOT_DRIVER(scmi_pinctrl) = {
> > + .name = "scmi_pinctrl",
> > + .id = UCLASS_PINCTRL,
> > + .ops = &scmi_pinctrl_ops,
> > + .probe = scmi_pinctrl_probe,
> > + .priv_auto = sizeof(struct scmi_pinctrl_priv),
> > +};
> > --
> > 2.34.1
> >
>
> Where is the compatible string for this driver?
Nothing defined.
As I mentioned in the cover letter, pinctrl driver consists of
two layers:
- helper functions which directly manipulate protocol interfaces.
- DM-compliant (that you doubt about :) pinctrl driver.
All the SCMI-based drivers (existing ones, clock, reset and voltage domain,
except base) follow this scheme.
According to the DT bindings for SCMI protocols, they are sub-nodes
of scmi node and identified by "reg" properties. When the *bind* takes
place, scmi_bind_protocols() will enumerate all the sub-nodes and
try to bind associated protocol drivers (pinctrl in my patch) based
on "reg" value.
So there is no need to have a "compatible" property for each protocol.
Do you think it is ugly?
I suppose the corresponding kernel code, scmi_probe() in
drivers/firmware/arm_scmi/driver.c, has a similar logic although
it seems a bit more complicated.
-Takahiro Akashi
>
> Regards,
> Simon
Re: [RFC 1/6] firmware: scmi: fix protocol enumeration logic
Hi Simon,
On Sun, Sep 10, 2023 at 01:13:26PM -0600, Simon Glass wrote:
> Hi AKASHI,
>
> On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> wrote:
> >
> > The original logic in enumerating all the protocols accidentally
> > modifies a *loop* variable, node, at Voltage domain protocol.
> > So subsequent protocol nodes in a device tree won't be detected.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > drivers/firmware/scmi/scmi_agent-uclass.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
> > b/drivers/firmware/scmi/scmi_agent-uclass.c
> > index 46a2933d51a4..79584c00a066 100644
> > --- a/drivers/firmware/scmi/scmi_agent-uclass.c
> > +++ b/drivers/firmware/scmi/scmi_agent-uclass.c
> > @@ -422,8 +422,11 @@ static int scmi_bind_protocols(struct udevice *dev)
> > case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
> > if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) &&
> > scmi_protocol_is_supported(dev, protocol_id)) {
> > - node = ofnode_find_subnode(node,
> > "regulators");
> > - if (!ofnode_valid(node)) {
> > + ofnode sub_node;
> > +
> > + sub_node = ofnode_find_subnode(node,
> > +
> > "regulators");
> > + if (!ofnode_valid(sub_node)) {
> > dev_err(dev, "no regulators
> > node\n");
> > return -ENXIO;
> > }
> > --
> > 2.34.1
> >
>
> This function is very ugly...could we instead rely on driver model to
I think you are talking about scmi_bind_protocols() itself,
not the hunk above in my patch, which is merely a bug fix
in the existing code.
If so,
> bind the devices? It seems unfortunate to have to write code for
> something which could be done with no extra code.
We will discuss in the threads on your comment [1].
[1] https://lists.denx.de/pipermail/u-boot/2023-September/530200.html
-Takahiro Akashi
> Regards,
> Simon
Re: [PATCH v2] binman: Fix SyntaxWarning: invalid escape sequence '\('
On 9/11/23 6:36 AM, Simon Glass wrote:
Hi,
On Tue, 5 Sept 2023 at 19:34, 荣涛 wrote:
Thanks, Simon
The following lines is my systeme ENV and make log
$ cat /etc/os-release
NAME="Fedora Linux"
VERSION="40 (Workstation Edition Prerelease)"
ID=fedora
VERSION_ID=40
VERSION_CODENAME=""
PLATFORM_ID="platform:f40"
PRETTY_NAME="Fedora Linux 40 (Workstation Edition Prerelease)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:40"
DEFAULT_HOSTNAME="fedora"
HOME_URL="https://fedoraproject.org/";
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/rawhide/system-administrators-guide/";
SUPPORT_URL="https://ask.fedoraproject.org/";
BUG_REPORT_URL="https://bugzilla.redhat.com/";
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=rawhide
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=rawhide
SUPPORT_END=2024-05-14
VARIANT="Workstation Edition"
VARIANT_ID=workstation
$ uname -r
6.6.0-0.rc0.20230901git99d99825fc07.3.fc40.x86_64
$ gcc --version
gcc (GCC) 13.2.1 20230728 (Red Hat 13.2.1-1)
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ make qemu-x86_64_defconfig
HOSTCC scripts/basic/fixdep
HOSTCC scripts/kconfig/conf.o
YACCscripts/kconfig/zconf.tab.c
LEX scripts/kconfig/zconf.lex.c
HOSTCC scripts/kconfig/zconf.tab.o
HOSTLD scripts/kconfig/conf
#
# configuration written to .config
#
$ make -j8
... [too much logs]
SYM spl/u-boot-spl.sym
OBJCOPY spl/u-boot-x86-start16-spl.bin
OBJCOPY spl/u-boot-x86-reset16-spl.bin
CAT spl/u-boot-spl-dtb.bin
COPYspl/u-boot-spl.bin
BINMAN .binman_stamp
/home/sda/git-repos/u-boot/tools/binman/etype/section.py:25: SyntaxWarning:
invalid escape sequence '\('
"""Entry that contains other entries
OFCHK .config
Thanks for that. What is your Python version?
Thanks Simon
$ python --version
Python 3.12.0rc1
Regards,
Simon
[PATCH] buildman: Disable CONFIG_CMD_CONFIG for reproduceable build
When the ordering of CONFIG options changes, the gzipped CONFIG list can
change in size and contents. This makes it hard to compare commits which
only different in CONFIG ordering.
Disable this when -r is given, to make this easier. Update the
documentation as well.
Signed-off-by: Simon Glass
---
tools/buildman/buildman.rst | 3 ++-
tools/buildman/control.py | 4
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst
index aae2477b5c3d..4cca97dbda8c 100644
--- a/tools/buildman/buildman.rst
+++ b/tools/buildman/buildman.rst
@@ -1021,7 +1021,8 @@ various files to be rebuilt even if no source changes are
made, which in turn
requires that the final U-Boot binary be re-linked. This unnecessary work can
be avoided by turning off the timestamp feature. This can be achieved using
the `-r` flag, which enables reproducible builds by setting
-`SOURCE_DATE_EPOCH=0` when building.
+`SOURCE_DATE_EPOCH=0` when building, as well as disabling `LOCALVERSION_AUTO`
+and `CONFIG_CMD_CONFIG`.
Combining all of these options together yields the command-line shown below.
This will provide the quickest possible feedback regarding the current content
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index f2ffb7f5b4aa..0ce4f2e2f3b5 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -575,6 +575,10 @@ def calc_adjust_cfg(adjust_cfg, reproducible_builds):
print('Not dropping LOCALVERSION_AUTO for reproducible build')
else:
adjust_cfg['LOCALVERSION_AUTO'] = '~'
+if 'CMD_CONFIG' in adjust_cfg:
+print('Not dropping CMD_CONFIG for reproducible build')
+else:
+adjust_cfg['CMD_CONFIG'] = '~'
return adjust_cfg
--
2.42.0.283.g2d96d420d3-goog
Re: [PATCH v3 5/9] board: qualcomm: Add support for dragonboard845c
Hi Sumit,
On Tue, 29 Aug 2023 at 04:24, Sumit Garg wrote:
>
> Hi Simon,
>
> On Tue, 29 Aug 2023 at 03:39, Simon Glass wrote:
> >
> > Hi Peter,
> >
> > On Mon, 28 Aug 2023 at 14:24, Peter Robinson wrote:
> > >
> > > On Mon, Aug 28, 2023 at 6:55 PM Simon Glass wrote:
> > > >
> > > > Hi Sumit,
> > > >
> > > > On Thu, 24 Aug 2023 at 04:44, Sumit Garg wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Thu, 24 Aug 2023 at 05:29, Simon Glass wrote:
> > > > > >
> > > > > > Hi Sumit,
> > > > > >
> > > > > > On Tue, 12 Jul 2022 at 01:12, Sumit Garg
> > > > > > wrote:
> > > > > > >
> > > > > > > Add support for 96Boards Dragonboard 845C aka Robotics RB3
> > > > > > > development
> > > > > > > platform. This board complies with 96Boards Open Platform
> > > > > > > Specifications.
> > > > > > >
> > > > > > > Features:
> > > > > > > - Qualcomm Snapdragon SDA845 SoC
> > > > > > > - 4GiB RAM
> > > > > > > - 64GiB UFS drive
> > > > > > >
> > > > > > > U-boot is chain loaded by ABL in 64-bit mode as part of boot.img.
> > > > > > > For detailed build and boot instructions, refer to
> > > > > > > doc/board/qualcomm/sdm845.rst, board: dragonboard845c.
> > > > > > >
> > > > > > > Signed-off-by: Sumit Garg
> > > > > > > Reviewed-by: Ramon Fried
> > > > > > > ---
> > > > > > > arch/arm/dts/dragonboard845c-uboot.dtsi | 37 +++
> > > > > > > arch/arm/dts/dragonboard845c.dts | 44
> > > > > > > arch/arm/mach-snapdragon/Kconfig | 14 +++
> > > > > > > board/qualcomm/dragonboard845c/Kconfig| 12 +++
> > > > > > > board/qualcomm/dragonboard845c/MAINTAINERS| 6 ++
> > > > > > > board/qualcomm/dragonboard845c/Makefile | 9 ++
> > > > > > > board/qualcomm/dragonboard845c/db845c.its | 63 +++
> > > > > > > .../dragonboard845c/dragonboard845c.c | 9 ++
> > > > > > > configs/dragonboard845c_defconfig | 28 +
> > > > > > > doc/board/qualcomm/sdm845.rst | 100
> > > > > > > +++---
> > > > > > > include/configs/dragonboard845c.h | 28 +
> > > > > > > 11 files changed, 337 insertions(+), 13 deletions(-)
> > > > > > > create mode 100644 arch/arm/dts/dragonboard845c-uboot.dtsi
> > > > > > > create mode 100644 arch/arm/dts/dragonboard845c.dts
> > > > > > > create mode 100644 board/qualcomm/dragonboard845c/Kconfig
> > > > > > > create mode 100644 board/qualcomm/dragonboard845c/MAINTAINERS
> > > > > > > create mode 100644 board/qualcomm/dragonboard845c/Makefile
> > > > > > > create mode 100644 board/qualcomm/dragonboard845c/db845c.its
> > > > > > > create mode 100644
> > > > > > > board/qualcomm/dragonboard845c/dragonboard845c.c
> > > > > > > create mode 100644 configs/dragonboard845c_defconfig
> > > > > > > create mode 100644 include/configs/dragonboard845c.h
> > > > > > >
> > > > > > [..]
> > > > > >
> > > > > > > diff --git a/doc/board/qualcomm/sdm845.rst
> > > > > > > b/doc/board/qualcomm/sdm845.rst
> > > > > > > index b6642c9579..8ef4749287 100644
> > > > > > > --- a/doc/board/qualcomm/sdm845.rst
> > > > > > > +++ b/doc/board/qualcomm/sdm845.rst
> > > > > > > @@ -35,9 +35,25 @@ Pack android boot image
> > > > > > > ^^^
> > > > > > > We'll assemble android boot image with ``u-boot.bin`` instead of
> > > > > > > linux kernel,
> > > > > > > and FIT image instead of ``initramfs``. Android bootloader
> > > > > > > expect gzipped kernel
> > > > > > > -with appended dtb, so let's mimic linux to satisfy stock
> > > > > > > bootloader:
> > > > > > > +with appended dtb, so let's mimic linux to satisfy stock
> > > > > > > bootloader.
> > > > > > >
> > > > > >
> > > > > > [..]
> > > > > >
> > > > > > > +The dragonboard845c is a Qualcomm Robotics RB3 Development
> > > > > > > Platform, based on
> > > > > > > +the Qualcomm SDM845 SoC.
> > > > > > > +
> > > > > > > +Steps:
> > > > > > > +
> > > > > > > +- Build u-boot::
> > > > > > > +
> > > > > > > + $ export CROSS_COMPILE=
> > > > > > > + $ make dragonboard845c_defconfig
> > > > > > > + $ make
> > > > > > > +
> > > > > > > +- Create dummy dtb::
> > > > > > > +
> > > > > > > + workdir=/tmp/prepare_payload
> > > > > > > + mkdir -p "$workdir"
> > > > > > > + mock_dtb="$workdir"/payload_mock.dtb
> > > > > > > +
> > > > > > > + dtc -I dts -O dtb -o "$mock_dtb" << EOF
> > > > > > > + /dts-v1/;
> > > > > > > + / {
> > > > > > > + #address-cells = <2>;
> > > > > > > + #size-cells = <2>;
> > > > > > > +
> > > > > > > + memory@8000 {
> > > > > > > + device_type = "memory";
> > > > > > > + /* We expect the bootloader to fill in
> > > > > > > the size */
> > > > > > > + reg = <0 0x8000 0 0>;
> > > > > > > + };
> > > > > > > +
> > > > > > > + chosen { };
> > > > > > > + };
> > > > > > > + EOF
> > >
Re: [PATCH v3 5/9] board_f: Fix corruption of relocaddr
Hi Devarsh,
On Thu, 17 Aug 2023 at 09:10, Tom Rini wrote:
>
> On Wed, Aug 16, 2023 at 09:16:05PM +0530, Devarsh Thakkar wrote:
> > Hi Simon,
> >
> > On 15/08/23 20:14, Simon Glass wrote:
> > > Hi Devarsh,
> > >
> > > On Tue, 15 Aug 2023 at 03:23, Devarsh Thakkar wrote:
> > >>
> > >> Hi Simon, Tom,
> > >>
> > >> On 15/08/23 04:13, Simon Glass wrote:
> > >>> Hi Devarsh, Nikhil, Tom,
> > >>>
> > >>> On Wed, 9 Aug 2023 at 09:29, Bin Meng wrote:
> >
> > On Thu, Aug 3, 2023 at 7:03 PM Bin Meng wrote:
> > >
> > > On Thu, Aug 3, 2023 at 6:37 PM Bin Meng wrote:
> > >>
> > >> On Tue, Aug 1, 2023 at 12:00 AM Simon Glass
> > >> wrote:
> > >>>
> > >>> When the video framebuffer comes from the bloblist, we should not
> > >>> change
> > >>> relocaddr to this address, since it interfers with the normal memory
> > >>
> > >> typo: interferes
> > >>
> > >>> allocation.
> > >>>
> > >>> This fixes a boot loop in qemu-x86_64
> > >>>
> > >>> Signed-off-by: Simon Glass
> > >>> Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from
> > >>> SPL to u-boot")
> > >>> Suggested-by: Nikhil M Jain
> > >>> ---
> > >>>
> > >>> Changes in v3:
> > >>> - Reword the Kconfig help as suggested
> > >>>
> > >>> Changes in v2:
> > >>> - Add a Kconfig as the suggested conditional did not work
> > >>>
> > >>> common/board_f.c | 3 ++-
> > >>> drivers/video/Kconfig | 9 +
> > >>> 2 files changed, 11 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/common/board_f.c b/common/board_f.c
> > >>> index 7d2c380e91e..5173d0a0c2d 100644
> > >>> --- a/common/board_f.c
> > >>> +++ b/common/board_f.c
> > >>> @@ -419,7 +419,8 @@ static int reserve_video(void)
> > >>> if (!ho)
> > >>> return log_msg_ret("blf", -ENOENT);
> > >>> video_reserve_from_bloblist(ho);
> > >>> - gd->relocaddr = ho->fb;
> > >>> + if (IS_ENABLED(CONFIG_VIDEO_RESERVE_SPL))
> > >>> + gd->relocaddr = ho->fb;
> > >>> } else if (CONFIG_IS_ENABLED(VIDEO)) {
> > >>> ulong addr;
> > >>> int ret;
> > >>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > >>> index b41dc60cec5..f2e56204d52 100644
> > >>> --- a/drivers/video/Kconfig
> > >>> +++ b/drivers/video/Kconfig
> > >>> @@ -1106,6 +1106,15 @@ config SPL_VIDEO_REMOVE
> > >>>if this option is enabled video driver will be removed
> > >>> at the end of
> > >>>SPL stage, beforeloading the next stage.
> > >>>
> > >>> +config VIDEO_RESERVE_SPL
> > >>> + bool
> > >>> + help
> > >>> + This adjusts reserve_video() to redirect memory
> > >>> reservation when it
> > >>> + sees a video handoff blob (BLOBLISTT_U_BOOT_VIDEO). This
> > >>> avoids the
> > >>> + memory used for framebuffer from being allocated by
> > >>> U-Boot proper,
> > >>> + thus preventing any further memory reservations done by
> > >>> U-Boot proper
> > >>> + from overwriting the framebuffer.
> > >>> +
> > >>> if SPL_SPLASH_SCREEN
> > >>>
> > >>> config SPL_SPLASH_SCREEN_ALIGN
> > >>
> > >> Reviewed-by: Bin Meng
> > >
> > > applied to u-boot-x86, thanks!
> >
> > Dropped this one from the x86 queue per the discussion.
> > >>>
> > >>> I just wanted to come back to this discussion.
> > >>>
> > >>> Do we have an agreed way forward? Who is waiting for who?
> > >>>
> > >>
> > >> I was waiting on feedback on
> > >> https://lore.kernel.org/all/3b1e8005-f161-8058-13e7-3de2316aa...@ti.com/
> > >> but per my opinion, I would prefer to go with "Approach 2" with a
> > >> Kconfig as it looks simpler to me. It would look something like below :
> > >>
> > >> if (gd->relocaddr > (unsigned long)ho->fb) {
> > >> ulong fb_reloc_gap = gd->relocaddr - gd->ho->fb;
> > >>
> > >> /* Relocate after framebuffer area if nearing too close to it */
> > >> if (fb_reloc_gap < CONFIG_BLOBLIST_FB_RELOC_MIN_GAP)
> > >> gd->relocaddr = ho->fb;
> > >> }
> > >>
> > >> Regarding CONFIG_BLOBLIST_FB_RELOC_MIN_GAP
> > >> -> This describes minimum gap to keep between framebuffer address and
> > >> relocation address to avoid overlap when framebuffer address used by
> > >> blob is below the current relocation address
> > >>
> > >> -> It would be selected as default when CONFIG_BLOBLIST is selected with
> > >> default value set to 100Mb
> > >>
> > >> -> SoC specific Vendors can override this in their defconfigs to a
> > >> custom value if they feel 100Mb is not enough
> > >>
> > >> Also probably we can have some debug/error prints in the code to show
> > >> overlap happened (or is going happen) s
Re: [PATCH v2] binman: Fix SyntaxWarning: invalid escape sequence '\('
Hi,
On Tue, 5 Sept 2023 at 19:34, 荣涛 wrote:
>
> Thanks, Simon
>
> The following lines is my systeme ENV and make log
>
> $ cat /etc/os-release
> NAME="Fedora Linux"
> VERSION="40 (Workstation Edition Prerelease)"
> ID=fedora
> VERSION_ID=40
> VERSION_CODENAME=""
> PLATFORM_ID="platform:f40"
> PRETTY_NAME="Fedora Linux 40 (Workstation Edition Prerelease)"
> ANSI_COLOR="0;38;2;60;110;180"
> LOGO=fedora-logo-icon
> CPE_NAME="cpe:/o:fedoraproject:fedora:40"
> DEFAULT_HOSTNAME="fedora"
> HOME_URL="https://fedoraproject.org/";
> DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/rawhide/system-administrators-guide/";
> SUPPORT_URL="https://ask.fedoraproject.org/";
> BUG_REPORT_URL="https://bugzilla.redhat.com/";
> REDHAT_BUGZILLA_PRODUCT="Fedora"
> REDHAT_BUGZILLA_PRODUCT_VERSION=rawhide
> REDHAT_SUPPORT_PRODUCT="Fedora"
> REDHAT_SUPPORT_PRODUCT_VERSION=rawhide
> SUPPORT_END=2024-05-14
> VARIANT="Workstation Edition"
> VARIANT_ID=workstation
>
> $ uname -r
> 6.6.0-0.rc0.20230901git99d99825fc07.3.fc40.x86_64
>
> $ gcc --version
> gcc (GCC) 13.2.1 20230728 (Red Hat 13.2.1-1)
> Copyright (C) 2023 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> $ make qemu-x86_64_defconfig
> HOSTCC scripts/basic/fixdep
> HOSTCC scripts/kconfig/conf.o
> YACCscripts/kconfig/zconf.tab.c
> LEX scripts/kconfig/zconf.lex.c
> HOSTCC scripts/kconfig/zconf.tab.o
> HOSTLD scripts/kconfig/conf
> #
> # configuration written to .config
> #
>
> $ make -j8
> ... [too much logs]
> SYM spl/u-boot-spl.sym
> OBJCOPY spl/u-boot-x86-start16-spl.bin
> OBJCOPY spl/u-boot-x86-reset16-spl.bin
> CAT spl/u-boot-spl-dtb.bin
> COPYspl/u-boot-spl.bin
> BINMAN .binman_stamp
> /home/sda/git-repos/u-boot/tools/binman/etype/section.py:25: SyntaxWarning:
> invalid escape sequence '\('
> """Entry that contains other entries
> OFCHK .config
>
Thanks for that. What is your Python version?
Regards,
Simon
Re: [PATCH] Makefile: Force regeneration of env.txt
Hi Andrew,
On Tue, 5 Sept 2023 at 12:15, Andrew Davis wrote:
>
> On 9/5/23 1:09 PM, Andrew Davis wrote:
> > If the source .env file changes to one that is also older than
> > the generated env.txt file then it is not regenerated. This means
> > when switching board configs we do not regenerate the env. This
> > can be tested easily with:
> >
> > $ make j721e_evm_a72_defconfig
> > $ make # this may fail to complete but that is okay for this test
> > $ make am64x_evm_a53_defconfig
> > $ make
> > $ vim include/generated/env.txt
> >
> > Note this is still the J721e env not the AM64 config as expected.
> >
> > There is probably a better way to detect if the dependency name changed,
> > but that may involve extra files and hashing contents, so let's just
> > force it for now.
> >
> > Signed-off-by: Andrew Davis
> > ---
>
> This is basically a revert of:
> 36fc832927eb ("Makefile: Fix incorrect FORCE deps on env rules")
>
> But without changing the `include/generated/env.in` rule. The more
> I think about it, that should also be changed to forced as right
> now any changes to include/config.h or other kconfig options are
> not reflected in the env.in file after updates.
>
> That rule should depend on the generated config file so that any
> change in kconfig variables will cause it to regenerate, that
> might be what we need for this target instead of FORCE.
>
> Andrew
>
> > Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 9be24c4ec61..d195590d4b0 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1830,7 +1830,7 @@ quiet_cmd_envc = ENVC$@
> > touch $@ ; \
> > fi
> >
> > -include/generated/env.txt: $(wildcard $(ENV_FILE))
> > +include/generated/env.txt: $(wildcard $(ENV_FILE)) FORCE
> > $(call cmd,envc)
I thought that FORCE was only for if_changed - see makefiles.rst.txt
Could it depend on autoconf.h instead?
> >
> > # Write out the resulting environment, converted to a C string
Regards,
Simon
Re: sandbox trace errors in CI loop
Hi Michal, On Fri, 8 Sept 2023 at 09:00, Michal Simek wrote: > > Hi Tom, > > pá 8. 9. 2023 v 15:42 odesílatel Michal Simek napsal: > > > > > > > > On 9/8/23 15:34, Tom Rini wrote: > > > On Fri, Sep 08, 2023 at 12:24:10PM +0200, Michal Simek wrote: > > >> > > >> > > >> On 9/7/23 20:14, Tom Rini wrote: > > >>> On Thu, Sep 07, 2023 at 05:30:03PM +0200, Michal Simek wrote: > > Hi Simon and Tom, > > > > I am preparing pull request and I see that CI loop is reporting issue > > with > > sandbox trace and I have no idea what's going wrong here. > > > > This is the first problematic commit but have no clue what it has to > > do with trace. > > > > https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/commit/8c5f80d11b29536979ac41a6087fb8938f45bbf2 > > > > If you have an access you can take a look at my queue to see that only > > this > > job is failing. > > https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/pipelines > > > > Thanks, > > Michal > > > > collected 1152 items / 1151 deselected / 1 selected > > test/py/tests/test_trace.py F > > [100%] > > === FAILURES > > === > > __ test_trace > > __ > > test/py/tests/test_trace.py:292: in test_trace > > check_function(cons, fname, proftool, map_fname, trace_dat) > > test/py/tests/test_trace.py:117: in check_function > > out = util.run_and_log(cons, ['sh', '-c', cmd]) > > test/py/u_boot_utils.py:181: in run_and_log > > output = runner.run(cmd, ignore_errors=ignore_errors, > > stdin=stdin, env=env) > > test/py/multiplexed_log.py:183: in run > > raise exception > > E ValueError: Exit code: 1 > > Captured stdout setup > > - > > /u-boot > > trace: early enable at 0010 > > sandbox_serial serial: pinctrl_select_state_full: > > uclass_get_device_by_phandle_id: err=-19 > > => > > >>> > > >>> I don't get it either since I see this on master with trace options > > >>> enabled per CI: > > >>> $ ./u-boot -T arch/sandbox/dts/test.dtb > > >>> trace: early enable at 0010 > > >>> sandbox_serial serial: pinctrl_select_state_full: > > >>> uclass_get_device_by_phandle_id: err=-19 > > >>> ... rest of boot proceeds ... > > >>> > > >>> So the test should be failing already if that was the problem. > > >>> > > >> > > >> I don't thin it is the problematic part. > > >> > > >> When I look at > > >> https://source.denx.de/u-boot/u-boot/-/blob/master/tools/docker/Dockerfile?ref_type=heads#L225 > > >> > > >> clone is done from > > >> git clone https://github.com/rostedt/trace-cmd.git /tmp/trace/trace-cmd > > >> && \ > > >> > > >> https://github.com/rostedt/trace-cmd/blob/master/README#L5 > > >> > > >> we should be using > > >> git://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git instead. > > >> > > >> I installed the latest version on my PC and run it via pytest and this is > > >> what I am getting. > > >> You see that trace-cmd report is returning "failed to init data". > > >> > > >> Are we wired to any specific trace-cmd version? > > > > > > Wait, no, what's going on exactly? The github repository is a mirror of > > > the kernel.org one. So yes, we should correct to point to the main one, > > > but the top of tree in both cases is commit > > > 354dccca52e805ce1b22e2b62cbae8c265886c27. > > > > > > > I dig into it more. > > What it is happening is that when sandbox u-boot is bigger trace.dat file is > > placing information to different offset then it is recorded somewhere in > > header. > > > > Wrong placement > > > > 0005b000 20 70 65 72 66 20 6d 6f 6e 6f 20 6d 6f 6e 6f 5f | perf mono > > mono_| > > 0005b010 72 61 77 20 62 6f 6f 74 20 78 38 36 2d 74 73 63 |raw boot > > x86-tsc| > > 0005b020 0a 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > || > > 0005b030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > || > > * > > 0005c000 89 df f6 1d 00 00 00 00 dc 0f 00 00 00 00 00 00 > > || > > 0005c010 06 00 00 00 01 00 00 00 01 00 00 00 af e5 0a 00 > > || > > 0005c020 00 00 00 00 ef dc 09 00 00 00 00 00 06 00 00 00 > > || > > 0005c030 01 00 00 00 01 00 00 00 af e5 0a 00 00 00 00 00 > > || > > > > Correct placement for trace-cmd > > > > 0005aee0 75 70 74 69 6d 65 20 70 65 72 66 20 6d 6f 6e 6f |uptime perf > > mono| > > 0005aef0 20 6d 6f 6e 6f 5f 72 61 77 20 62 6f 6f 74 20 78 | mono_raw boot > > x| > > 0005af00 38 36 2d 74 73 63 0a 00 00 00 00 00 00 00 00 00 > > |86-tsc..| > > 0005af10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > |.
Re: [PATCH v3] bootstd: sata: Add bootstd support for ahci sata
Hi Tony,
On Fri, 8 Sept 2023 at 13:10, Tony Dinh wrote:
>
> Add ahci sata bootdev and corresponding hunting function.
>
> Signed-off-by: Tony Dinh
> ---
>
> Changes in v3:
> - Correct drivers/ata/Makefile to compile sata_bootdev only if
> ahci sata is enabled.
>
> Changes in v2:
> - set devtype to sata in bootmeth_script for non-scsi SATA device.
>
> boot/bootmeth_script.c | 12 ++--
> drivers/ata/Makefile | 2 +-
> drivers/ata/sata.c | 25 +++
> drivers/ata/sata_bootdev.c | 62 ++
> include/sata.h | 1 +
> 5 files changed, 99 insertions(+), 3 deletions(-)
> create mode 100644 drivers/ata/sata_bootdev.c
>
> diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
> index 58c57a2d4b..0269e0f9b0 100644
> --- a/boot/bootmeth_script.c
> +++ b/boot/bootmeth_script.c
> @@ -190,10 +190,18 @@ static int script_boot(struct udevice *dev, struct
> bootflow *bflow)
> ulong addr;
> int ret;
>
> - if (desc->uclass_id == UCLASS_USB)
> + if (desc->uclass_id == UCLASS_USB) {
> ret = env_set("devtype", "usb");
> - else
> + } else {
> ret = env_set("devtype", blk_get_devtype(bflow->blk));
> +
> + /* If the parent uclass is AHCI, but the driver is ATA
> +* (not scsi), set devtype to sata
> +*/
> + if (!ret && IS_ENABLED(CONFIG_SATA) &&
> + !strcmp("ahci", env_get("devtype")))
Can you not use desc->uclass_id to do the right thing here? It seems
odd to compare with the value of the env var you just set up above.
> + ret = env_set("devtype", "sata");
> + }
> if (!ret)
> ret = env_set_hex("devnum", desc->devnum);
> if (!ret)
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 6e30180b8b..0b6f91098a 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -10,7 +10,7 @@ obj-$(CONFIG_SCSI_AHCI) += ahci.o
> obj-$(CONFIG_DWC_AHSATA) += dwc_ahsata.o
> obj-$(CONFIG_FSL_SATA) += fsl_sata.o
> obj-$(CONFIG_LIBATA) += libata.o
> -obj-$(CONFIG_SATA) += sata.o
> +obj-$(CONFIG_SATA) += sata.o sata_bootdev.o
> obj-$(CONFIG_SATA_CEVA) += sata_ceva.o
> obj-$(CONFIG_SATA_MV) += sata_mv.o
> obj-$(CONFIG_SATA_SIL) += sata_sil.o
> diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c
> index ce3e9b5a40..9da7218564 100644
> --- a/drivers/ata/sata.c
> +++ b/drivers/ata/sata.c
> @@ -15,6 +15,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #ifndef CONFIG_AHCI
> struct blk_desc sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> @@ -50,6 +52,29 @@ int sata_scan(struct udevice *dev)
> return ops->scan(dev);
> }
>
> +int sata_rescan(bool verbose)
> +{
> + struct udevice *dev;
> + int devnum = 0;
> + int ret;
> +
> + /* Find and probing the SATA controller */
> + ret = uclass_get_device(UCLASS_AHCI, devnum, &dev);
This does not actually rescan...if the device has already been probed,
it won't be probed again. You will need to use the logic like in
sata_remove().
Also, please avoid using devnum. You should probably just use
uclass_probe_all(UCLASS_AHCI)
> +
> + /* Sanity check */
> + if (ret)
> + ret = uclass_find_first_device(UCLASS_AHCI, &dev);
That just finds it, but does not probe it. See uclass_first_device_err()
> + if (ret) {
> + printf("Cannot probe SATA device %d (err=%d)\n", devnum, ret);
> + return -ENOSYS;
return ret
> + }
> + if (!dev) {
> + printf("No SATA device found!\n");
> + return -ENOSYS;
> + }
> + return 0;
> +}
> +
> #ifndef CONFIG_AHCI
> #ifdef CONFIG_PARTITIONS
> struct blk_desc *sata_get_dev(int dev)
> diff --git a/drivers/ata/sata_bootdev.c b/drivers/ata/sata_bootdev.c
> new file mode 100644
> index 00..f638493ce0
> --- /dev/null
> +++ b/drivers/ata/sata_bootdev.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Bootdev for sata
> + *
> + * Copyright 2023 Tony Dinh
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static int sata_bootdev_bind(struct udevice *dev)
> +{
> + struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev);
> +
> + ucp->prio = BOOTDEVP_4_SCAN_FAST;
> +
> + return 0;
> +}
> +
> +static int sata_bootdev_hunt(struct bootdev_hunter *info, bool show)
> +{
> + int ret;
> +
> + if (IS_ENABLED(CONFIG_PCI)) {
> + ret = pci_init();
> + if (ret)
> + return ret;
> + }
> +
> + ret = sata_rescan(true);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +struct bootdev_ops sata_bootdev_ops = {
> +};
> +
> +static const struct udevice_id sata_bootdev_ids[] = {
> + { .compatible = "u-boot,bootdev-sata" },
> +
Re: [PATCH] arm: apple: Add initial Apple M2 Ultra support
Hi,
On Wed, 6 Sept 2023 at 15:50, Janne Grunau wrote:
>
> Apple's M2 Ultra SoC are somewhat similar to the M1 Ultra but needs
> a tweaked memory map as the M2 Pro/Max SoCs. USB, NVMe, UART, WDT
> and PCIe are working with the existing drivers.
>
> Signed-off-by: Janne Grunau
> ---
> arch/arm/mach-apple/board.c | 183
>
> 1 file changed, 183 insertions(+)
>
> diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
> index d50194811843..47393babbc62 100644
> --- a/arch/arm/mach-apple/board.c
> +++ b/arch/arm/mach-apple/board.c
> @@ -444,6 +444,187 @@ static struct mm_region t6020_mem_map[] = {
> }
> };
>
> +/* Apple M2 Ultra */
> +
> +static struct mm_region t6022_mem_map[] = {
> + {
> + /* I/O */
> + .virt = 0x28000,
> + .phys = 0x28000,
> + .size = SZ_1G,
> + .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
> +PTE_BLOCK_NON_SHARE |
> +PTE_BLOCK_PXN | PTE_BLOCK_UXN
> + }, {
Is there no devicetree binding for this information?
[..]
Regards,
Simon
Re: NVMe support on RPi CM4 board
Hi Luis, On Tue, 5 Sept 2023 at 17:17, Luis Alfredo da Silva wrote: > > Thanks for your answer Simon, > >> Can you check if the device has PCI bus master enabled? > > > I think this one will take me a bit more time, should this be in raspberry > pi’s eeprom firmware? > > Will the disabled PCI bus master also be a problem for the Linux kernel? As > this is working as expected (BTW the quoting seems broken in your email; also the text came out white so at first I thought you had sent an empty reply) Actually I can see that nvme_probe() turns on bus mastering. So no, I don't know what is wrong, sorry. Perhaps there are some quirks in the Linux driver which need to be added to U-Boot? Perhaps there is a power GPIO to set? Regards, Simon
Re: [RFC 4/6] gpio: add scmi driver based on pinctrl
Hi,
On Thu, 7 Sept 2023 at 22:32, AKASHI Takahiro
wrote:
>
> Hi Simon,
>
> On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> > wrote:
> > >
> > > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > > gpio devices exposed by SCMI firmware (server).
> > >
> > > Signed-off-by: AKASHI Takahiro
> > > ---
> > > drivers/pinctrl/pinctrl-scmi.c | 544 -
> > > 1 file changed, 539 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/pinctrl-scmi.c
> > > b/drivers/pinctrl/pinctrl-scmi.c
> > > index 3ebdad57b86c..73d385bdbfcc 100644
> > > --- a/drivers/pinctrl/pinctrl-scmi.c
> > > +++ b/drivers/pinctrl/pinctrl-scmi.c
> > > @@ -11,21 +11,20 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > +#include
> > > +#include
> > > #include
> > >
> > [..]
> >
> > > +
> > > +U_BOOT_DRIVER(scmi_gpio) = {
> > > + .name = "scmi_gpio",
> > > + .id = UCLASS_GPIO,
> > > + .of_match = scmi_gpio_ids,
> > > + .of_to_plat = scmi_gpio_probe,
> > > + .ops = &scmi_gpio_ops,
> > > + .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
> > > +};
> > > +
> > > +/**
> > > + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
> > > + * @parent:SCMI pinctrl device
> > > + *
> > > + * Create a pinctrl-controlled gpio device
> > > + *
> > > + * Return: 0 on success, error code on failure
> > > + */
> > > +static int scmi_gpiochip_register(struct udevice *parent)
> > > +{
> > > + ofnode node;
> > > + struct driver *drv;
> > > + struct udevice *gpio_dev;
> > > + int ret;
> > > +
> > > + /* TODO: recovery if failed */
> > > + dev_for_each_subnode(node, parent) {
> > > + if (!ofnode_is_enabled(node))
> > > + continue;
> >
> > Can we not rely on the normal driver model binding to bind these
> > devices? Why does it need to be done manually here?
>
> First, please take a look at the cover letter.
> In this RFC, I basically assume two patterns of DT bindings,
> (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
>
> In (B), a DT node as a gpio device, which is essentially a child
> of pinctrl device, is located *under* a pinctrl device. It need
> to be probed manually as there is no implicit method to enumerate
> it as a DM device automatically.
You could add a post_bind() method to the pinctrl uclass to call
dm_scan_fdt_dev() as we do with TPM.
>
> On the other hand, in (A), the same node can be put anywhere in a DT
> as it contains a "compatible" property to identify itself.
> So if we want to only support (A), scmi_gpiochip_register() and
> scmi_pinctrl_bind() are not necessary.
>
> Since there is no discussion about bindings for GPIO managed by SCMI
> pinctrl device yet on the kernel side, I have left two solutions
> in this RFC.
OK.
[..]
Regards,
Simon
Re: [PATCH v2] bootstd: Make efi_mgr bootmeth work for non-sandbox setups
Hi Mark,
On Sun, 10 Sept 2023 at 08:57, Mark Kettenis wrote:
>
> > From: Simon Glass
> > Date: Thu, 7 Sep 2023 09:57:34 -0600
> >
> > Hi Mark,
> >
> > On Thu, 7 Sept 2023 at 07:08, Mark Kettenis wrote:
> > >
> > > > From: Simon Glass
> > > > Date: Thu, 7 Sep 2023 06:23:10 -0600
> > > >
> > > > Hi Mark,
> > > >
> > > > On Sun, 3 Sept 2023 at 14:40, Mark Kettenis
> > > > wrote:
> > > > >
> > > > > Enable the bootflow based on this bootmeth if the BootOrder EFI
> > > > > variable is set.
> > > > >
> > > > > Signed-off-by: Mark Kettenis
> > > > > ---
> > > > >
> > > > > ChangeLog:
> > > > >
> > > > > v2: - Initialize EFI subsystem to populate EFI variables
> > > > >
> > > > >
> > > > > boot/bootmeth_efi_mgr.c | 17 -
> > > > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > > index e9d973429f..e6c42d41fb 100644
> > > > > --- a/boot/bootmeth_efi_mgr.c
> > > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > > @@ -14,6 +14,8 @@
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > +#include
> > > > > +#include
> > > > >
> > > > > /**
> > > > > * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > > @@ -46,13 +48,26 @@ static int efi_mgr_check(struct udevice *dev,
> > > > > struct bootflow_iter *iter)
> > > > > static int efi_mgr_read_bootflow(struct udevice *dev, struct
> > > > > bootflow *bflow)
> > > > > {
> > > > > struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > > + efi_status_t ret;
> > > > > + efi_uintn_t size;
> > > > > + u16 *bootorder;
> > > > >
> > > > > if (priv->fake_dev) {
> > > > > bflow->state = BOOTFLOWST_READY;
> > > > > return 0;
> > > > > }
> > > > >
> > > > > - /* To be implemented */
> > > > > + ret = efi_init_obj_list();
> > > >
> > > > Doesn't this start up the whole EFI system? Is there a cheaper way to
> > > > see if the variable subsystem exists, or can work?
> > >
> > > A fair bit of it at least. With the possible exception of launching
> > > EFI capsules (which shouldn't happen for a "normal" boot), I don't
> > > think this is very expensive though.
> > >
> > > There currently isn't a way to only initialize the variable subsystem;
> > > we can't just call efi_init_variables() here since we have to also
> > > call efi_disks_register() and a later efi_init_obj_list() call would
> > > call efi_init_variables() again, which would leak memory.
> > >
> > > Alternatively we could just unconditionally declare the efi_mgr
> > > bootmeth as "ready" (like you already do for sandbox). That would
> > > postpone initialization of the EFI subsystem until we actually execute
> > > this bootmeth.
> > >
> > > But we need to do something before we switch more targets to standard
> > > boot since otherwise booting through the EFI boot manager is just
> > > plain broken.
> >
> > Yes...
> >
> > I don't have any great ideas on this. If we could get the EFI code
> > better integrated into U-Boot then perhaps we would not need this
> > 'monolithm' init?
>
> You'll have to take that up with the maintainers of the EFI loader
> subsystem. I just want to repair the breakage that was introduced
> with bootstd.
Yes, understood. Do you know how long it takes to do this step?
The control for this feature is CMD_BOOTEFI_BOOTMGR...I think it would
be better to have BOOTEFI_BOOTMGR as well as CMD_BOOTEFI_BOOTMGR,
since it is strange to have a CMD Kconfig in lib/efi_loader
I am OK with this patch as I don't see any alternative. We need to do
something and we can always improve it if it causes problems.
Heinrich?
Regards,
Simon
Re: [PATCH 30/32] fdt: Allow the devicetree to come from a bloblist
Hi Ilias, On Mon, 4 Sept 2023 at 03:31, Ilias Apalodimas wrote: > > Hi Simon, > > On Fri, 1 Sept 2023 at 18:51, Simon Glass wrote: > > > > Hi Ilias, > > > > On Fri, 1 Sept 2023 at 01:50, Ilias Apalodimas > > wrote: > > > > > > [...] > > > > > > > > > > > > > > +config OF_BLOBLIST > > > > > > + bool "DTB is provided by a bloblist" > > > > > > + help > > > > > > + Select this to read the devicetree from the bloblist. This > > > > > > allows > > > > > > + using a bloblist to transfer the devicetree between U-Boot > > > > > > phases. > > > > > > + The devicetree is stored in the bloblist by an early phase > > > > > > so that > > > > > > + U-Boot can read it. > > > > > > + > > > > > > > > > > I dont think this is a good idea. We used to have 4-5 different > > > > > options > > > > > here, which we tried to clean up and ended up with two very discrete > > > > > options. Why do we have to reintroduce a new one? Doesn't that > > > > > bloblist > > > > > have a header of some sort? The bloblist literally comes from a > > > > > previous > > > > > stage bootloader which is what OF_BOARD is here for. So why can't we > > > > > just > > > > > read the header and figure out if the magic of the bloblist matches? > > > > > > > > No, OF_BOARD is a hack to allow boards to do what they like with the > > > > FDT. > > > > > > > > This patch is a standard mechanism to pass the DT from one firmware > > > > phase to the next. We have spent quite a bit of time creating a spec > > > > for it, and we should use it. > > > > > > Where exactly am I objecting using the spec? Can you please re-read my > > > email? > > > I am actually pointing out we should use the spec *properly*. So > > > instead of having a Kconfig option for the DT, which is pretty > > > pointless, we should parse the bloblist. If the header defined by > > > the *spec* is found, we should just search for the DT in there. > > > What you are doing here, is take the spec, pick a very specific item > > > that the list contains, and create a Kconfig option out of it. Which > > > basically ignores the discoverable options of the bloblist. For > > > example, that bloblist can also contain an entry to a TPM eventlog. > > > Should we start creating Kconfig options for all the firmware handoff > > > entries that are defined on that spec? > > > > OK so that is a different thing. What should it do if it expects to find a > > bloblist but cannot? I want it to throw an error, because I am trying to > > make the boot deterministic. What do you think? > > That's fine by me. You can even put that under IS_ENABLED for the > bloblist inside the existing OF_BOARD check. So I was thinking > - If no bloblist is required in Kconfig options we do the hacks we used to > - if bloblist is selected and the config option is OF_BOARD, throw an > error and mention that the previous stage loader should hand over a DT > > Is that what you had in mind? Yes, that sounds good to me. But I still think we need an OF_BLOBLIST option to control whether the devicetree comes from there. Otherwise we will end up with people using OF_BOARD to work around it. We also have the SPL case which does not pass the DT in a bloblist...in fact SPL typically doesn't even have the full DT. [..] Regards, Simon
Re: [RFC 6/6] test: dm: add SCMI pinctrl test
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
wrote:
>
> In this test, SCMI-based pinmux feature is exercised.
>
> Signed-off-by: AKASHI Takahiro
> ---
> test/dm/scmi.c | 62 ++
> 1 file changed, 62 insertions(+)
>
> diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> index 423b6ef70b29..ca5a2e9c781e 100644
> --- a/test/dm/scmi.c
> +++ b/test/dm/scmi.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -395,3 +396,64 @@ static int dm_test_scmi_voltage_domains(struct
> unit_test_state *uts)
> return release_sandbox_scmi_test_devices(uts, dev);
> }
> DM_TEST(dm_test_scmi_voltage_domains, UT_TESTF_SCAN_FDT);
> +
> +/*
> + * This part is derived from test/dm/pinmux.c, So
> + *
> + * Copyright (C) 2020 Sean Anderson
> + */
> +
> +static char buf[64];
> +#define test_muxing(selector, expected) do { \
> + ut_assertok(pinctrl_get_pin_muxing(dev, selector, buf, sizeof(buf)));
> \
> + ut_asserteq_str(expected, (char *)&buf); \
Can you instead make this a function?
> +} while (0)
> +
> +#define test_name(selector, expected) do { \
> + ut_assertok(pinctrl_get_pin_name(dev, selector, buf, sizeof(buf))); \
> + ut_asserteq_str(expected, (char *)&buf); \
> +} while (0)
> +
> +static int dm_test_scmi_pinmux(struct unit_test_state *uts)
> +{
> + struct udevice *dev;
> +
> + ut_assertok(uclass_get_device_by_name(UCLASS_PINCTRL, "protocol@19",
> + &dev));
> + ut_assertok(pinctrl_select_state(dev, "default"));
> + test_muxing(0, "");
ut_assertok(check_muxing(uts, ...));
> + test_muxing(1, "");
> + test_muxing(2, "I2S.");
> + test_muxing(3, "I2S.");
> + test_muxing(4, "I2S.");
> + test_muxing(5, "GPIO bias-pull-up.");
> + test_muxing(6, "GPIO drive-open-drain output-mode output-value.");
> + test_muxing(7, "GPIO bias-pull-down input-mode.");
> + test_muxing(8, "GPIO bias-disable.");
> +
> + ut_assertok(pinctrl_select_state(dev, "alternate"));
> + test_muxing(0, "I2C drive-open-drain.");
> + test_muxing(1, "I2C drive-open-drain.");
> + test_muxing(2, "SPI.");
> + test_muxing(3, "SPI.");
> + test_muxing(4, "SPI.");
> + test_muxing(5, "CS bias-pull-up.");
> + test_muxing(6, "CS drive-open-drain output-mode output-value.");
> + test_muxing(7, "GPIO bias-pull-down input-mode.");
> + test_muxing(8, "GPIO bias-disable.");
> +
> + ut_assertok(pinctrl_select_state(dev, "0"));
> + test_muxing(0, "I2C drive-open-drain.");
> + test_muxing(1, "I2C drive-open-drain.");
> + test_muxing(2, "I2S.");
> + test_muxing(3, "I2S.");
> + test_muxing(4, "I2S.");
> + test_muxing(5, "GPIO bias-pull-up.");
> + test_muxing(6, "GPIO drive-open-drain output-mode output-value.");
> + test_muxing(7, "GPIO bias-pull-down input-mode.");
> + test_muxing(8, "GPIO bias-disable.");
> +
> + return 0;
> +}
> +
> +DM_TEST(dm_test_scmi_pinmux, UT_TESTF_SCAN_FDT);
> --
> 2.34.1
>
Regards,
Simon
Re: [PATCH v2] global: Use proper project name U-Boot (next2)
On Fri, 8 Sept 2023 at 01:11, Michal Simek wrote:
>
> Use proper project name in README, rst and comment.
> Done in connection to commit bb922ca3eb4b ("global: Use proper project name
> U-Boot (next)").
>
> Signed-off-by: Michal Simek
> ---
>
> Changes in v2:
> - Revert change in tools/binman/entries.rst reported by Simon
>
> board/cobra5272/README | 18 +-
> board/emulation/qemu-ppce500/qemu-ppce500.c | 2 +-
> doc/board/xilinx/zynq.rst | 2 +-
> doc/board/xilinx/zynqmp-r5.rst | 4 ++--
> doc/imx/mkimage/imximage.txt| 2 +-
> doc/usage/environment.rst | 2 +-
> doc/usage/semihosting.rst | 2 +-
> 7 files changed, 16 insertions(+), 16 deletions(-)
Reviewed-by: Simon Glass
Re: [PATCH] tpm: Fix autostart for TPM1.2 devices
On Fri, 8 Sept 2023 at 22:07, Heinrich Schuchardt wrote:
>
> On 9/8/23 22:37, Ilias Apalodimas wrote:
> > On commit e663b2ff4ba2("tpm: Add 'tpm autostart' shell command") an
> > autostart function was added for both TPM1.2 and 2.0 devices. Instead
> > of correctly wiring the autostart command for TPM1.2 devices that patch
> > mistakenly added it on 'tpm init'
> >
> > Fixes: commit e663b2ff4ba2("tpm: Add 'tpm autostart' shell command")
> > Signed-off-by: Ilias Apalodimas
>
> Reviewed-by: Heinrich Schuchardt
>
> > ---
> > cmd/tpm-v1.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass
Re: [RFC 1/6] firmware: scmi: fix protocol enumeration logic
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
wrote:
>
> The original logic in enumerating all the protocols accidentally
> modifies a *loop* variable, node, at Voltage domain protocol.
> So subsequent protocol nodes in a device tree won't be detected.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/firmware/scmi/scmi_agent-uclass.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
> b/drivers/firmware/scmi/scmi_agent-uclass.c
> index 46a2933d51a4..79584c00a066 100644
> --- a/drivers/firmware/scmi/scmi_agent-uclass.c
> +++ b/drivers/firmware/scmi/scmi_agent-uclass.c
> @@ -422,8 +422,11 @@ static int scmi_bind_protocols(struct udevice *dev)
> case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
> if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) &&
> scmi_protocol_is_supported(dev, protocol_id)) {
> - node = ofnode_find_subnode(node,
> "regulators");
> - if (!ofnode_valid(node)) {
> + ofnode sub_node;
> +
> + sub_node = ofnode_find_subnode(node,
> + "regulators");
> + if (!ofnode_valid(sub_node)) {
> dev_err(dev, "no regulators node\n");
> return -ENXIO;
> }
> --
> 2.34.1
>
This function is very ugly...could we instead rely on driver model to
bind the devices? It seems unfortunate to have to write code for
something which could be done with no extra code.
Regards,
Simon
Re: [RFC 3/6] pinctrl: add scmi driver
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
wrote:
>
> This DM-compliant driver deals with SCMI pinctrl protocol and presents
> pinctrl devices exposed by SCMI firmware (server).
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/pinctrl/Kconfig| 11 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-scmi.c | 537 +
> 3 files changed, 549 insertions(+)
> create mode 100644 drivers/pinctrl/pinctrl-scmi.c
[..]
+
> +U_BOOT_DRIVER(scmi_pinctrl) = {
> + .name = "scmi_pinctrl",
> + .id = UCLASS_PINCTRL,
> + .ops = &scmi_pinctrl_ops,
> + .probe = scmi_pinctrl_probe,
> + .priv_auto = sizeof(struct scmi_pinctrl_priv),
> +};
> --
> 2.34.1
>
Where is the compatible string for this driver?
Regards,
Simon
Re: [RFC 5/6] firmware: scmi: add pseudo pinctrl protocol support on sandbox
Hi AKASHI,
On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
wrote:
>
> With this patch, sandbox SCMI agent can handle pinctrl protocol.
> This feature is used in an unit test for SCMI pinctrl.
>
> Signed-off-by: AKASHI Takahiro
> ---
> arch/sandbox/dts/test.dts | 115
> drivers/firmware/scmi/sandbox-scmi_agent.c | 722 +
> 2 files changed, 837 insertions(+)
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index dc0bfdfb6e4b..d2ddea801995 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -723,9 +723,124 @@
> };
> };
> };
> +
> + pinctrl_scmi: protocol@19 {
> + reg = <0x19>;
> +
> + pinctrl-names = "default","alternate";
> + pinctrl-0 = <&scmi_pinctrl_gpios>,
> <&scmi_pinctrl_i2s>;
> + pinctrl-1 = <&scmi_pinctrl_spi>,
> <&scmi_pinctrl_i2c>;
> +
> +#if 0
Are these alternatives that you are testing?
The bindings look OK to me - is this how it is done in Linux?
Regards,
Simon
Re: [PATCH 3/8] binman: capsule: Generate capsules through config file
Hi Sughosh, On Fri, 8 Sept 2023 at 06:00, Sughosh Ganu wrote: > > Add support in binman for generating capsules by reading the capsule > parameters through a config file. Also add a test case in binman for > this mode of capsule generation. > > Signed-off-by: Sughosh Ganu > --- > > tools/binman/entries.rst | 35 > tools/binman/etype/efi_capsule_cfg_file.py | 66 ++ > tools/binman/ftest.py | 29 ++ > tools/binman/test/319_capsule_cfg.dts | 15 + > 4 files changed, 145 insertions(+) > create mode 100644 tools/binman/etype/efi_capsule_cfg_file.py > create mode 100644 tools/binman/test/319_capsule_cfg.dts I think we discussed this some time ago. The problem I have with this is that it adds another way of specifying the image description, separate from the binman definition. This introduces all sorts of problems: - the question of where the filenames are located, something which binman tries to handle cohesively - binman has no knowledge of what is being put in the capsules - the resulting fdtmap lacks any information about what is in the capsules - it seems to create two ways of doing the same thing Binman can presumably already create multiple capsule files, just by adding to the description. What need does this feature meet? Regards, Simon
Re: [PATCH] binman: doc: fix reference tag placement for Logging section
Hi Massimo,
On Sun, 10 Sept 2023 at 09:35, Massimo Pegorer
wrote:
>
> Hi Simon, Heinrich,
>
> Il giorno sab 9 set 2023 alle ore 15:52 Massimo Pegorer
> ha scritto:
> >
> > Move BinmanLogging reference tag after section "Signing FIT container
> > with private key in an image" and just before section "Logging".
> >
> > Fixes: 0f40e23fd22 ("binman: add documentation for binman sign option")
> > Signed-off-by: Massimo Pegorer
> >
> > ---
> > Note that at the end of section "Replacing files in an image" and just
> > before section "Signing FIT container with private key in an image"
> > there is a line which seems a typo or an incomplete sentence:
> >
> > Repacking an image involves
>
> Any comment about this note? Should I remove this line in a v2?
Yes you can remove it. I suspect I was starting to explain what binman
does to repack...
Regards,
Simon
Re: [PATCH 1/1] test: build dependency for event unit tests
On Thu, 7 Sept 2023 at 10:45, Heinrich Schuchardt
wrote:
>
> The test_event_base and test_event_probe unit tests use function
> event_register() which depends on CONFIG_EVENT_DYNAMIC=y.
>
> Fixes: 7d02645fe4c0 ("event: Add a simple test")
> Signed-off-by: Heinrich Schuchardt
> ---
> test/common/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Simon Glass
Re: [PATCH v1] doc: Update path to source_file_format.rst
On Fri, 8 Sept 2023 at 13:28, Heinrich Schuchardt wrote: > > On 9/7/23 22:50, Joao Marcos Costa wrote: > > Previously, the file extension was .txt, and it referenced the uImage.FIT > > directory, which no longer exists. This commit updates the path accordingly. > > > > Signed-off-by: Joao Marcos Costa > > --- > > doc/usage/fit/howto.rst | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/doc/usage/fit/howto.rst b/doc/usage/fit/howto.rst > > index c933703d1d..8b38cd534d 100644 > > --- a/doc/usage/fit/howto.rst > > +++ b/doc/usage/fit/howto.rst > > @@ -22,7 +22,7 @@ for its latest version. mkimage (together with dtc) takes > > as input > > an image source file, which describes the contents of the image and > > defines > > its various properties used during booting. By convention, image source > > file > > has the ".its" extension, also, the details of its format are given in > > -doc/uImage.FIT/source_file_format.txt. The actual data that is to be > > included in > > +doc/usage/fit/source_file_format.rst. The actual data that is to be > > included in > > Thank you for updating the reference. > > In the HTML documentation we want to generate a link here using: > > :doc:`source_file_format` > > Best regards > > Heinrich > > > the uImage (kernel, ramdisk, etc.) is specified in the image source file > > in the > > form of paths to appropriate data files. The outcome of the image creation > > process is a binary file (by convention with the ".itb" extension) that > Reviewed-by: Simon Glass
Re: UEFI update files missing on sandbox?
Hi Sughosh, On Sun, 10 Sept 2023 at 04:54, Sughosh Ganu wrote: > > hi Simon, > > On Sun, 10 Sept 2023 at 04:10, Simon Glass wrote: > > > > Hi Sughosh, > > > > I thought that all your patches were merged but now when I build > > sandbox I don't see the capsule-update files generated. Is there > > something missing? > > The building of the EFI capsules was moved to the corresponding test > setup. This was based on feedback from Tom Rini [1], where he had > suggested that instead of having the binman nodes for the capsules as > part of the sandbox dtb, it should rather be highlighted in the > document. Hmmm that was not how I was hoping it would work. Is there a way to make the build create the files? ...I just found efi_capsule_data() I really don't like this at all...it is running a part of the build system in the test. We have to do this in some cases, e.g. for tracing we must run with a special build of sandbox, but it should not be necessary for EFI capsules. I would really like the sandbox build to produce the capsules needed for tests, without any extra effort. As it is, there is no way to try this out. > > -sughosh > > [1] - https://lists.denx.de/pipermail/u-boot/2023-August/526987.html Regards, Simon
Re: [PATCH] patman: Respect include directive on Git config lookup
On Fri, 8 Sept 2023 at 08:17, Fei Shao wrote: > > People may put their user name and email in a local config file and > reference it by the include.* directives, however `git config --global` > doesn't look up the included configs by default. > > Enable the --includes option explicitly to support such use cases. > > Signed-off-by: Fei Shao > --- > > tools/patman/gitutil.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass
[PATCH v4 2/2] x86: Update cbmem driver
This driver is not actually built since a Kconfig was never created for
it.
Add a Kconfig (which is already implied by COREBOOT) and update the
implementation to avoid using unnecessary memory. Drop the #ifdef at the
top since we can rely on Kconfig to get that right.
To enable it (in addition to serial and video), use:
setenv stdout serial,vidconsole,cbmem
Signed-off-by: Simon Glass
---
Changes in v4:
- Add some comments to help understand the overflow mechanism
Changes in v2:
- Update to support the new overflow mechanism
drivers/misc/Kconfig | 8 +++
drivers/misc/cbmem_console.c | 43 ++--
2 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a6e3f62ecb09..c930e4a361bf 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -122,6 +122,14 @@ config VEXPRESS_CONFIG
configuration bus on the Arm Versatile Express boards via
a sysreg driver.
+config CBMEM_CONSOLE
+ bool "Write console output to coreboot cbmem"
+ depends on X86
+ help
+ Enables console output to the cbmem console, which is a memory
+ region set up by coreboot to hold a record of all console output.
+ Enable this only if booting from coreboot.
+
config CMD_CROS_EC
bool "Enable crosec command"
depends on CROS_EC
diff --git a/drivers/misc/cbmem_console.c b/drivers/misc/cbmem_console.c
index 8bbe33d414da..3cbe9fb1a46a 100644
--- a/drivers/misc/cbmem_console.c
+++ b/drivers/misc/cbmem_console.c
@@ -5,27 +5,37 @@
#include
#include
-#ifndef CONFIG_SYS_COREBOOT
-#error This driver requires coreboot
-#endif
-
#include
-struct cbmem_console {
- u32 buffer_size;
- u32 buffer_cursor;
- u8 buffer_body[0];
-} __attribute__ ((__packed__));
-
-static struct cbmem_console *cbmem_console_p;
-
void cbmemc_putc(struct stdio_dev *dev, char data)
{
- int cursor;
+ const struct sysinfo_t *sysinfo = cb_get_sysinfo();
+ struct cbmem_console *cons;
+ uint pos, flags;
+
+ if (!sysinfo)
+ return;
+ cons = sysinfo->cbmem_cons;
+ if (!cons)
+ return;
+
+ pos = cons->cursor & CBMC_CURSOR_MASK;
+
+ /* preserve the overflow flag if present */
+ flags = cons->cursor & ~CBMC_CURSOR_MASK;
+
+ cons->body[pos++] = data;
+
+ /*
+* Deal with overflow - the flag may be cleared by another program which
+* reads the buffer out, e.g. Linux
+*/
+ if (pos >= cons->size) {
+ pos = 0;
+ flags |= CBMC_OVERFLOW;
+ }
- cursor = cbmem_console_p->buffer_cursor++;
- if (cursor < cbmem_console_p->buffer_size)
- cbmem_console_p->buffer_body[cursor] = data;
+ cons->cursor = flags | pos;
}
void cbmemc_puts(struct stdio_dev *dev, const char *str)
@@ -40,7 +50,6 @@ int cbmemc_init(void)
{
int rc;
struct stdio_dev cons_dev;
- cbmem_console_p = lib_sysinfo.cbmem_cons;
memset(&cons_dev, 0, sizeof(cons_dev));
--
2.42.0.283.g2d96d420d3-goog
[PATCH v4 1/2] x86: coreboot: Document cbmem console struct
Coreboot changed a few years ago to include an overflow flag. Update the
structure to match this.
This comes from coreboot commit:
6f5ead14b4 ("mb/google/nissa/var/joxer: Update eMMC DLL settings")
Note: There are several implementations of this in coreboot. I have chosen
to follow the one in src/lib/cbmem_console.c
Signed-off-by: Simon Glass
---
Changes in v4:
- Reword commit and change title
Changes in v3:
- Drop __packed as it does nothing useful
arch/x86/include/asm/coreboot_tables.h | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/coreboot_tables.h
b/arch/x86/include/asm/coreboot_tables.h
index 4de137fbab9d..0dfb64babb96 100644
--- a/arch/x86/include/asm/coreboot_tables.h
+++ b/arch/x86/include/asm/coreboot_tables.h
@@ -299,11 +299,24 @@ struct cb_vdat {
#define CB_TAG_TIMESTAMPS 0x0016
#define CB_TAG_CBMEM_CONSOLE 0x0017
+#define CBMC_CURSOR_MASK ((1 << 28) - 1)
+#define CBMC_OVERFLOW BIT(31)
+
+/*
+ * struct cbmem_console - In-memory console buffer for coreboot
+ *
+ * Structure describing console buffer. It is overlaid on a flat memory area,
+ * with body covering the extent of the memory. Once the buffer is full,
+ * output will wrap back around to the start of the buffer. The high bit of the
+ * cursor field gets set to indicate that this happened. If the underlying
+ * storage allows this, the buffer will persist across multiple boots and
append
+ * to the previous log.
+ */
struct cbmem_console {
u32 size;
u32 cursor;
- char body[0];
-} __packed;
+ u8 body[0];
+};
#define CB_TAG_MRC_CACHE 0x0018
--
2.42.0.283.g2d96d420d3-goog
Re: [PATCH v3 0/4] support for booting the compressed U-boot binary on Rockchip based ARM64 SOC's
Hi, On 2023-09-10 20:24, Manoj Sai wrote: > This patchset adds the support on Rockchip based ARM64 SOC's that compress > the U-BOOT proper along with dtb > and ATF in FIT image format.Second stage bootloader(SPL) loads the compressed > binaries, uncompress > them and handover control to the next stage. > > Changes for V3 :- > > 1. Replaced spl_decompression_enabled() function instead of checking > IS_ENABLED(CONFIG_SPL_GZIP) >and IS_ENABLED(CONFIG_SPL_LZMA) in spl_fit.c > > 2. Removed extra wrapping parentheses in spl_decompression_enabled(). > > Size Comparision between compressed and uncompressed binaries :- > >size of uncompressed binary:- 9.0M (94,21,824 bytes) > manoj:u-boot$ ls -lb u-boot-rockchip.bin > -rw-rw-r-- 1 manoj manoj 9421824 Sep 10 22:22 u-boot-rockchip.bin > >size of GZIP compressed binary :- 8.6M (89,85,088 bytes) > manoj:u-boot$ ls -lb u-boot-rockchip.bin > -rw-rw-r-- 1 manoj manoj 8985088 Jul 25 07:42 u-boot-rockchip.bin > >size of LZMA compressed binary :- 8.6 M (90,06,592 bytes) > manoj:u-boot$ ls -lb u-boot-rockchip.bin > -rw-rw-r-- 1 manoj manoj 9006592 Jul 25 07:47 u-boot-rockchip.bin > > Test results of Booting time using bootstage command in Uboot command prompt > on roc-rk3399-pc board :- > > 1) Uncompressed U-BOOT : Total boot time ≈ 12.063971 seconds > => bootstage report > Timer summary in microseconds (10 records): >MarkElapsed Stage > 0 0 reset > 1,833,884 1,833,884 board_init_f > 2,959,528 1,125,644 board_init_r > 5,224,521 2,264,993 eth_common_init > 5,523,428298,907 eth_initialize > 5,523,606178 main_loop > 5,523,764158 usb_start > 12,063,971 6,540,207 cli_loop > > 2) GZIP Compressed U-BOOT : Total time ≈ 12.824968 seconds > > => bootstage report > Timer summary in microseconds (10 records): >MarkElapsed Stage > 0 0 reset > 2,594,709 2,594,709 board_init_f > 3,719,969 1,125,260 board_init_r > 5,985,450 2,265,481 eth_common_init > 6,284,371298,921 eth_initialize > 6,284,549178 main_loop > 6,284,708159 usb_start > 12,824,968 6,540,260 cli_loop > > 3) LZMA Compressed U-BOOT : Total time ≈ 17.025004 seconds > > => bootstage report > Timer summary in microseconds (10 records): >MarkElapsed Stage > 0 0 reset > 6,852,254 6,852,254 board_init_f > 7,940,143 1,087,889 board_init_r > 10,190,458 2,250,315 eth_common_init > 10,487,254296,796 eth_initialize > 10,487,432178 main_loop > 10,487,590158 usb_start > 17,025,004 6,537,414 cli_loop > > > As per suggestion from Mr.Jonas Karlman (jo...@kwiboo.se) through Patchset > V2,that check boot time > with enabling CONFIG_SPL_FIT_SIGNATURE that might impact boot time. > > Tried to check the boot time with CONFIG_FIT_SIGNATURE enabled, I didnt find > any significant > boot time improvement with enabling CONFIG_SPL_FIT_SIGNATURE. I may not have been that clear in my last mail, it is the following rfc/patch that may improve performance. That prfc/patch does improve performance for sha256 validation when CONFIG_SPL_FIT_SIGNATURE is enabled. [RFC] rockchip: spl: Enable caches to speed up checksum validation https://patchwork.ozlabs.org/project/uboot/patch/20230702110055.3686457-1-jo...@kwiboo.se/ Would be great to get confirmation if D-cache enabled in SPL also benefit this series, and not just checksum validation. (that rfc/patch unfortunately did not get much feedback) Regards, Jonas > > Comparision of GZIP and LZMA compressed U-boot Boot time with and without > Enable of CONFIG_FIT_SIGNATURE :- > > - GZIP Compressed U-BOOT and CONFIG_FIT_SIGNATURE enabled :- Total time ≈ > 13.283998 seconds > > => bootstage report > Timer summary in microseconds (10 records): >MarkElapsed Stage > 0 0 reset > 3,052,571 3,052,571 board_init_f > 4,179,787 1,127,216 board_init_r > 6,445,472 2,265,685 eth_common_init > 6,744,416298,944 eth_initialize > 6,744,593177 main_loop > 6,744,751158 usb_start > 13,283,998 6,539,247 cli_loop > > - GZIP Compressed U-BOOT and CONFIG_FIT_SIGNATURE disabled :- Total time ≈ > 12.824968 seconds > > > - LZMA Compressed U-BOOT and CONFIG_FIT_SIGNATURE enabled :- Total time ≈ > 17.005996 seconds > >=> bootstage report > Timer summary in microseconds (10 records): >MarkElapsed Stage > 0 0 reset > 6,775,071 6,775,071 board_init_f > 7,902,443 1,127,372 board_init_r > 10,167,546 2,265,103 eth_common_init > 10,466,418298,872 eth_initialize > 10,466,595177 main_loop > 10,466,753158 usb_start > 17,005,996 6,539,243 cli_loop > > - LZMA Compressed U-BOOT and CONFIG_FIT_SIGNATURE disabled :- Total time ≈ > 17.025004 seconds > > Ma
[PATCH v3 4/4] rockchip: Add support to generate LZMA compressed U-boot binary
Add support for generating a LZMA-compressed U-boot binary with the
help of binman, if CONFIG_SPL_LZMA is selected.
Signed-off-by: Manoj Sai
Reviewed-by: Simon Glass
Reviewed-by: Kever Yang
---
Changes in v3:
- None
Changes in v2:
- New patch for v2
arch/arm/dts/rockchip-u-boot.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/rockchip-u-boot.dtsi
b/arch/arm/dts/rockchip-u-boot.dtsi
index 8f248f941f..c8c928c7e5 100644
--- a/arch/arm/dts/rockchip-u-boot.dtsi
+++ b/arch/arm/dts/rockchip-u-boot.dtsi
@@ -58,6 +58,8 @@
#endif
#if defined(CONFIG_SPL_GZIP)
compression = "gzip";
+#elif defined(CONFIG_SPL_LZMA)
+ compression = "lzma";
#else
compression = "none";
#endif
@@ -66,6 +68,8 @@
u-boot-nodtb {
#if defined(CONFIG_SPL_GZIP)
compress = "gzip";
+#elif defined(CONFIG_SPL_LZMA)
+ compress = "lzma";
#endif
};
#ifdef CONFIG_SPL_FIT_SIGNATURE
--
2.25.1
[PATCH v3 3/4] rockchip: Add support to generate GZIP compressed U-boot binary
Add support for generating a GZIP-compressed U-boot binary with the
help of binman, if CONFIG_SPL_GZIP is selected.
Signed-off-by: Manoj Sai
Reviewed-by: Simon Glass
Reviewed-by: Kever Yang
---
Changes in v3:
- None
Changes in v2:
- New patch for v2
arch/arm/dts/rockchip-u-boot.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/dts/rockchip-u-boot.dtsi
b/arch/arm/dts/rockchip-u-boot.dtsi
index be2658e8ef..8f248f941f 100644
--- a/arch/arm/dts/rockchip-u-boot.dtsi
+++ b/arch/arm/dts/rockchip-u-boot.dtsi
@@ -56,10 +56,17 @@
#else
arch = "arm";
#endif
+#if defined(CONFIG_SPL_GZIP)
+ compression = "gzip";
+#else
compression = "none";
+#endif
load = ;
entry = ;
u-boot-nodtb {
+#if defined(CONFIG_SPL_GZIP)
+ compress = "gzip";
+#endif
};
#ifdef CONFIG_SPL_FIT_SIGNATURE
hash {
--
2.25.1
[PATCH v3 2/4] spl: fit: support for booting a LZMA-compressed U-boot binary
If LZMA Compression support is enabled, LZMA compressed U-Boot
binary will be placed at a specified RAM location which is
defined at CONFIG_SYS_LOAD_ADDR and will be assigned as the
source address.
image_decomp() function, will decompress the LZMA compressed
U-Boot binary which is placed at source address(CONFIG_SYS_LOAD_ADDR)
to the default CONFIG_SYS_TEXT_BASE location.
spl_load_fit_image function will load the decompressed U-Boot
binary, which is placed at the CONFIG_SYS_TEXT_BASE location.
Signed-off-by: Manoj Sai
Signed-off-by: Suniel Mahesh
Reviewed-by: Simon Glass
Reviewed-by: Kever Yang
---
Changes in v3:
- added IS_ENABLED(CONFIG_SPL_LZMA) to spl_decompression_enabled() function.
- Removed extra parentheses.
Changes in v2:
- New patch for v2
common/spl/spl_fit.c | 13 -
include/spl.h| 2 +-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index eb97259f57..75895ef15c 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -281,7 +281,8 @@ static int spl_load_fit_image(struct spl_load_info *info,
ulong sector,
return 0;
}
- if (spl_decompression_enabled() && image_comp == IH_COMP_GZIP)
+ if (spl_decompression_enabled() &&
+ (image_comp == IH_COMP_GZIP || image_comp == IH_COMP_LZMA))
src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR,
ARCH_DMA_MINALIGN), len);
else
src_ptr = map_sysmem(ALIGN(load_addr,
ARCH_DMA_MINALIGN), len);
@@ -329,6 +330,16 @@ static int spl_load_fit_image(struct spl_load_info *info,
ulong sector,
return -EIO;
}
length = size;
+ } else if (IS_ENABLED(CONFIG_SPL_LZMA) && image_comp == IH_COMP_LZMA) {
+ size = CONFIG_SYS_BOOTM_LEN;
+ ulong loadEnd;
+
+ if (image_decomp(IH_COMP_LZMA, CONFIG_SYS_LOAD_ADDR, 0, 0,
+load_ptr, src, length, size, &loadEnd)) {
+ puts("Uncompressing error\n");
+ return -EIO;
+ }
+ length = loadEnd - CONFIG_SYS_LOAD_ADDR;
} else {
memcpy(load_ptr, src, length);
}
diff --git a/include/spl.h b/include/spl.h
index 3a7e448cc7..9de93a34cd 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -905,6 +905,6 @@ void spl_save_restore_data(void);
*/
static inline bool spl_decompression_enabled(void)
{
- return IS_ENABLED(CONFIG_SPL_GZIP);
+ return IS_ENABLED(CONFIG_SPL_GZIP) || IS_ENABLED(CONFIG_SPL_LZMA);
}
#endif
--
2.25.1
[PATCH v3 1/4] spl: fit: support for booting a GZIP-compressed U-boot binary
If GZIP Compression support is enabled, GZIP compressed U-Boot binary
will be at a specified RAM location which is defined at
CONFIG_SYS_LOAD_ADDR and will be assign it as the source address.
gunzip function in spl_load_fit_image ,will decompress the GZIP
compressed U-Boot binary which is placed at
source address(CONFIG_SYS_LOAD_ADDR) to the default
CONFIG_SYS_TEXT_BASE location.
spl_load_fit_image function will load the decompressed U-Boot
binary, which is placed at the CONFIG_SYS_TEXT_BASE location.
Signed-off-by: Manoj Sai
Signed-off-by: Suniel Mahesh
Reviewed-by: Kever Yang
Reviewed-by: Simon Glass
---
Changes in v3:
- Replaced spl_decompression_enabled() function instead of
checking IS_ENABLED(CONFIG_SPL_GZIP).
- Removed checking IS_ENABLED(CONFIG_SPL_LZMA) in spl_decompression_enabled()
function.
Changes in v2:
- New patch for v2
common/spl/spl_fit.c | 9 ++---
include/spl.h| 10 ++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 730639f756..eb97259f57 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -239,14 +239,14 @@ static int spl_load_fit_image(struct spl_load_info *info,
ulong sector,
bool external_data = false;
if (IS_ENABLED(CONFIG_SPL_FPGA) ||
- (IS_ENABLED(CONFIG_SPL_OS_BOOT) && IS_ENABLED(CONFIG_SPL_GZIP))) {
+ (IS_ENABLED(CONFIG_SPL_OS_BOOT) && spl_decompression_enabled())) {
if (fit_image_get_type(fit, node, &type))
puts("Cannot get image type.\n");
else
debug("%s ", genimg_get_type_name(type));
}
- if (IS_ENABLED(CONFIG_SPL_GZIP)) {
+ if (spl_decompression_enabled()) {
fit_image_get_comp(fit, node, &image_comp);
debug("%s ", genimg_get_comp_name(image_comp));
}
@@ -281,7 +281,10 @@ static int spl_load_fit_image(struct spl_load_info *info,
ulong sector,
return 0;
}
- src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len);
+ if (spl_decompression_enabled() && image_comp == IH_COMP_GZIP)
+ src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR,
ARCH_DMA_MINALIGN), len);
+ else
+ src_ptr = map_sysmem(ALIGN(load_addr,
ARCH_DMA_MINALIGN), len);
length = len;
overhead = get_aligned_image_overhead(info, offset);
diff --git a/include/spl.h b/include/spl.h
index 93e906431e..3a7e448cc7 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -897,4 +897,14 @@ struct legacy_img_hdr *spl_get_load_buffer(ssize_t offset,
size_t size);
void board_boot_order(u32 *spl_boot_list);
void spl_save_restore_data(void);
+
+/*
+ * spl_decompression_enabled() - check decompression support is enabled for
SPL build
+ *
+ * Returns true if decompression support is enabled, else False
+ */
+static inline bool spl_decompression_enabled(void)
+{
+ return IS_ENABLED(CONFIG_SPL_GZIP);
+}
#endif
--
2.25.1
[PATCH v3 0/4] support for booting the compressed U-boot binary on Rockchip based ARM64 SOC's
This patchset adds the support on Rockchip based ARM64 SOC's that compress the U-BOOT proper along with dtb and ATF in FIT image format.Second stage bootloader(SPL) loads the compressed binaries, uncompress them and handover control to the next stage. Changes for V3 :- 1. Replaced spl_decompression_enabled() function instead of checking IS_ENABLED(CONFIG_SPL_GZIP) and IS_ENABLED(CONFIG_SPL_LZMA) in spl_fit.c 2. Removed extra wrapping parentheses in spl_decompression_enabled(). Size Comparision between compressed and uncompressed binaries :- size of uncompressed binary:- 9.0M (94,21,824 bytes) manoj:u-boot$ ls -lb u-boot-rockchip.bin -rw-rw-r-- 1 manoj manoj 9421824 Sep 10 22:22 u-boot-rockchip.bin size of GZIP compressed binary :- 8.6M (89,85,088 bytes) manoj:u-boot$ ls -lb u-boot-rockchip.bin -rw-rw-r-- 1 manoj manoj 8985088 Jul 25 07:42 u-boot-rockchip.bin size of LZMA compressed binary :- 8.6 M (90,06,592 bytes) manoj:u-boot$ ls -lb u-boot-rockchip.bin -rw-rw-r-- 1 manoj manoj 9006592 Jul 25 07:47 u-boot-rockchip.bin Test results of Booting time using bootstage command in Uboot command prompt on roc-rk3399-pc board :- 1) Uncompressed U-BOOT : Total boot time ??? 12.063971 seconds => bootstage report Timer summary in microseconds (10 records): MarkElapsed Stage 0 0 reset 1,833,884 1,833,884 board_init_f 2,959,528 1,125,644 board_init_r 5,224,521 2,264,993 eth_common_init 5,523,428298,907 eth_initialize 5,523,606178 main_loop 5,523,764158 usb_start 12,063,971 6,540,207 cli_loop 2) GZIP Compressed U-BOOT : Total time ??? 12.824968 seconds => bootstage report Timer summary in microseconds (10 records): MarkElapsed Stage 0 0 reset 2,594,709 2,594,709 board_init_f 3,719,969 1,125,260 board_init_r 5,985,450 2,265,481 eth_common_init 6,284,371298,921 eth_initialize 6,284,549178 main_loop 6,284,708159 usb_start 12,824,968 6,540,260 cli_loop 3) LZMA Compressed U-BOOT : Total time ??? 17.025004 seconds => bootstage report Timer summary in microseconds (10 records): MarkElapsed Stage 0 0 reset 6,852,254 6,852,254 board_init_f 7,940,143 1,087,889 board_init_r 10,190,458 2,250,315 eth_common_init 10,487,254296,796 eth_initialize 10,487,432178 main_loop 10,487,590158 usb_start 17,025,004 6,537,414 cli_loop As per suggestion from Mr.Jonas Karlman (jo...@kwiboo.se) through Patchset V2,that check boot time with enabling CONFIG_SPL_FIT_SIGNATURE that might impact boot time. Tried to check the boot time with CONFIG_FIT_SIGNATURE enabled, I didnt find any significant boot time improvement with enabling CONFIG_SPL_FIT_SIGNATURE. Comparision of GZIP and LZMA compressed U-boot Boot time with and without Enable of CONFIG_FIT_SIGNATURE :- - GZIP Compressed U-BOOT and CONFIG_FIT_SIGNATURE enabled :- Total time ??? 13.283998 seconds => bootstage report Timer summary in microseconds (10 records): MarkElapsed Stage 0 0 reset 3,052,571 3,052,571 board_init_f 4,179,787 1,127,216 board_init_r 6,445,472 2,265,685 eth_common_init 6,744,416298,944 eth_initialize 6,744,593177 main_loop 6,744,751158 usb_start 13,283,998 6,539,247 cli_loop - GZIP Compressed U-BOOT and CONFIG_FIT_SIGNATURE disabled :- Total time ??? 12.824968 seconds - LZMA Compressed U-BOOT and CONFIG_FIT_SIGNATURE enabled :- Total time ??? 17.005996 seconds => bootstage report Timer summary in microseconds (10 records): MarkElapsed Stage 0 0 reset 6,775,071 6,775,071 board_init_f 7,902,443 1,127,372 board_init_r 10,167,546 2,265,103 eth_common_init 10,466,418298,872 eth_initialize 10,466,595177 main_loop 10,466,753158 usb_start 17,005,996 6,539,243 cli_loop - LZMA Compressed U-BOOT and CONFIG_FIT_SIGNATURE disabled :- Total time ??? 17.025004 seconds Manoj Sai (4): spl: fit: support for booting a GZIP-compressed U-boot binary spl: fit: support for booting a LZMA-compressed U-boot binary rockchip: Add support to generate GZIP compressed U-boot binary rockchip: Add support to generate LZMA compressed U-boot binary arch/arm/dts/rockchip-u-boot.dtsi | 11 +++ common/spl/spl_fit.c | 20 +--- include/spl.h | 10 ++ 3 files changed, 38 insertions(+), 3 deletions(-) -- 2.25.1
Re: [PATCH] binman: doc: fix reference tag placement for Logging section
Hi Simon, Heinrich,
Il giorno sab 9 set 2023 alle ore 15:52 Massimo Pegorer
ha scritto:
>
> Move BinmanLogging reference tag after section "Signing FIT container
> with private key in an image" and just before section "Logging".
>
> Fixes: 0f40e23fd22 ("binman: add documentation for binman sign option")
> Signed-off-by: Massimo Pegorer
>
> ---
> Note that at the end of section "Replacing files in an image" and just
> before section "Signing FIT container with private key in an image"
> there is a line which seems a typo or an incomplete sentence:
>
> Repacking an image involves
Any comment about this note? Should I remove this line in a v2?
Regards,
Massimo
> ---
> tools/binman/binman.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
> index aeea33fddb..8945b19446 100644
> --- a/tools/binman/binman.rst
> +++ b/tools/binman/binman.rst
> @@ -1482,7 +1482,6 @@ when it was created.
>
> Repacking an image involves
>
> -.. _`BinmanLogging`:
>
> Signing FIT container with private key in an image
> --
> @@ -1501,6 +1500,7 @@ If you want to sign and replace FIT container in place::
> which will sign FIT container with private key and replace it immediately
> inside your image.
>
> +.. _`BinmanLogging`:
>
> Logging
> ---
> --
> 2.34.1
>
Re: [PATCH v2] bootstd: Make efi_mgr bootmeth work for non-sandbox setups
> From: Simon Glass
> Date: Thu, 7 Sep 2023 09:57:34 -0600
>
> Hi Mark,
>
> On Thu, 7 Sept 2023 at 07:08, Mark Kettenis wrote:
> >
> > > From: Simon Glass
> > > Date: Thu, 7 Sep 2023 06:23:10 -0600
> > >
> > > Hi Mark,
> > >
> > > On Sun, 3 Sept 2023 at 14:40, Mark Kettenis wrote:
> > > >
> > > > Enable the bootflow based on this bootmeth if the BootOrder EFI
> > > > variable is set.
> > > >
> > > > Signed-off-by: Mark Kettenis
> > > > ---
> > > >
> > > > ChangeLog:
> > > >
> > > > v2: - Initialize EFI subsystem to populate EFI variables
> > > >
> > > >
> > > > boot/bootmeth_efi_mgr.c | 17 -
> > > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > index e9d973429f..e6c42d41fb 100644
> > > > --- a/boot/bootmeth_efi_mgr.c
> > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > @@ -14,6 +14,8 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > +#include
> > > >
> > > > /**
> > > > * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > @@ -46,13 +48,26 @@ static int efi_mgr_check(struct udevice *dev,
> > > > struct bootflow_iter *iter)
> > > > static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow
> > > > *bflow)
> > > > {
> > > > struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > + efi_status_t ret;
> > > > + efi_uintn_t size;
> > > > + u16 *bootorder;
> > > >
> > > > if (priv->fake_dev) {
> > > > bflow->state = BOOTFLOWST_READY;
> > > > return 0;
> > > > }
> > > >
> > > > - /* To be implemented */
> > > > + ret = efi_init_obj_list();
> > >
> > > Doesn't this start up the whole EFI system? Is there a cheaper way to
> > > see if the variable subsystem exists, or can work?
> >
> > A fair bit of it at least. With the possible exception of launching
> > EFI capsules (which shouldn't happen for a "normal" boot), I don't
> > think this is very expensive though.
> >
> > There currently isn't a way to only initialize the variable subsystem;
> > we can't just call efi_init_variables() here since we have to also
> > call efi_disks_register() and a later efi_init_obj_list() call would
> > call efi_init_variables() again, which would leak memory.
> >
> > Alternatively we could just unconditionally declare the efi_mgr
> > bootmeth as "ready" (like you already do for sandbox). That would
> > postpone initialization of the EFI subsystem until we actually execute
> > this bootmeth.
> >
> > But we need to do something before we switch more targets to standard
> > boot since otherwise booting through the EFI boot manager is just
> > plain broken.
>
> Yes...
>
> I don't have any great ideas on this. If we could get the EFI code
> better integrated into U-Boot then perhaps we would not need this
> 'monolithm' init?
You'll have to take that up with the maintainers of the EFI loader
subsystem. I just want to repair the breakage that was introduced
with bootstd.
Re: UEFI update files missing on sandbox?
hi Simon, On Sun, 10 Sept 2023 at 04:10, Simon Glass wrote: > > Hi Sughosh, > > I thought that all your patches were merged but now when I build > sandbox I don't see the capsule-update files generated. Is there > something missing? The building of the EFI capsules was moved to the corresponding test setup. This was based on feedback from Tom Rini [1], where he had suggested that instead of having the binman nodes for the capsules as part of the sandbox dtb, it should rather be highlighted in the document. -sughosh [1] - https://lists.denx.de/pipermail/u-boot/2023-August/526987.html
