Re: [BUG] double free in dfu_free_entities()
Hi Heinrich,
> Hello Lukasz,
>
> Jose and I have observed segmentation violations when
> dfu_free_entities() is called.
>
Thanks for finding them.
> In our scenario we have:
>
> dfu_alt_info=
> sf 0:0=u-boot-bin raw 0x10 0x5;u-boot-env raw 0x15
> 0x20
>
> dfu_free_entities() is called with these entries in dfu:
>
> u-boot-env:
> dfu->data.sf.dev 15baf420 m25p16
> dfu->data.sf.dev->dev 15888410 spi.bin@0
> u-boot-bin:
> dfu->data.sf.dev 15baf420 m25p16
> dfu->data.sf.dev->dev 15888410 spi.bin@0
>
> The same device is released twice which eventually leads to the
> segmentation fault in device_chld_remove().
If I remember correctly that was fixed some time ago... Maybe something
has changed with recent patches.
Was it working before? Or is the above scenario a new one?
>
> Program received signal SIGSEGV, Segmentation fault.
> device_chld_remove (
> dev=dev@entry=0x557f6750 , drv=drv@entry=0x0,
> flags=flags@entry=1)
> at drivers/core/device-remove.c:55
> 55 list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) {
>
> We must ensure that dfu_free_entities() removes each device only once.
>
> Another bug in dfu_free_entities() is that only the first list member
> is freed. free() should be called for all list members.
Yes. Correct.
>
> Best regards
>
> Heinrich
>
>
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
pgpfC3IPrTr7n.pgp
Description: OpenPGP digital signature
Re: [BUG] double free in dfu_free_entities()
On 3/11/21 9:18 AM, Lukasz Majewski wrote:
Hi Heinrich,
Hello Lukasz,
Jose and I have observed segmentation violations when
dfu_free_entities() is called.
Thanks for finding them.
In our scenario we have:
dfu_alt_info=
sf 0:0=u-boot-bin raw 0x10 0x5;u-boot-env raw 0x15
0x20
dfu_free_entities() is called with these entries in dfu:
u-boot-env:
dfu->data.sf.dev 15baf420 m25p16
dfu->data.sf.dev->dev 15888410 spi.bin@0
u-boot-bin:
dfu->data.sf.dev 15baf420 m25p16
dfu->data.sf.dev->dev 15888410 spi.bin@0
The same device is released twice which eventually leads to the
segmentation fault in device_chld_remove().
If I remember correctly that was fixed some time ago... Maybe something
has changed with recent patches.
Was it working before? Or is the above scenario a new one?
Jose and Takahiro have provided patches that exercise DFU by repeatedly
calling dfu_init_env_entities() and dfu_free_entities(). See function
efi_get_dfu_info().
Furthermore for testing we use a dfu_alt_info with multiple images on
the same device.
No change was done in this part of DFU since 2014. So this is a
longstanding problem that we now uncovered.
[PATCH 1/1] mtd: spi_flash_free()
https://lists.denx.de/pipermail/u-boot/2021-March/443915.html
fixes the issue for the case of DM_SPI_FLASH.
But we should also address the DFU system itself.
Best regards
Heinrich
Program received signal SIGSEGV, Segmentation fault.
device_chld_remove (
dev=dev@entry=0x557f6750 , drv=drv@entry=0x0,
flags=flags@entry=1)
at drivers/core/device-remove.c:55
55 list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) {
We must ensure that dfu_free_entities() removes each device only once.
Another bug in dfu_free_entities() is that only the first list member
is freed. free() should be called for all list members.
Yes. Correct.
Best regards
Heinrich
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On Thu, Mar 11, 2021 at 08:50:22AM +0100, Heinrich Schuchardt wrote:
> On 3/5/21 11:22 PM, Ilias Apalodimas wrote:
> > On the following patches we allow for an initrd path to be stored in
> > Boot variables. Specifically we encode in the FIlePathList[] of
> > the EFI_LOAD_OPTIONS for each Boot variable.
> >
> > The FilePathList[] array looks like this:
> > kernel - 0xff - VenMedia(initrd GUID) - 0x01 - initrd1 - 0x01 initrd2 -0xff
> > So let's add the relevant functions to concatenate and retrieve a device
> > path based on a Vendor GUID.
> >
> > Signed-off-by: Ilias Apalodimas
> > ---
> > include/efi_loader.h | 4 ++
> > lib/efi_loader/efi_device_path.c | 72
> > 2 files changed, 76 insertions(+)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index f470bbd636f4..eb11a8c7d4b1 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -738,6 +738,10 @@ struct efi_load_option {
> > const u8 *optional_data;
> > };
> >
> > +struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> > + efi_uintn_t *size, efi_guid_t guid);
> > +struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
> > + const struct efi_device_path *dp2);
> > efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8
> > *data,
> > efi_uintn_t *size);
> > unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8
> > **data);
> > diff --git a/lib/efi_loader/efi_device_path.c
> > b/lib/efi_loader/efi_device_path.c
> > index c9315dd45857..cf1321828e87 100644
> > --- a/lib/efi_loader/efi_device_path.c
> > +++ b/lib/efi_loader/efi_device_path.c
> > @@ -310,6 +310,41 @@ struct efi_device_path *efi_dp_append(const struct
> > efi_device_path *dp1,
> > return ret;
> > }
> >
> > +/** efi_dp_concat() - Concatenate 2 device paths. The final device path
> > will contain
> > + *two device paths separated by and end node (0xff).
> > + *
> > + * @dp1: First device path
> > + * @size: Second device path
> > + *
> > + * Return: concatenated device path or NULL. Caller must free the returned
> > value
> > + */
> > +struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
> > + const struct efi_device_path *dp2)
> > +{
> > + struct efi_device_path *ret;
> > + efi_uintn_t sz1, sz2;
> > + void *p;
> > +
> > + if (!dp1 || !dp2)
> > + return NULL;
> > + sz1 = efi_dp_size(dp1);
> > + sz2 = efi_dp_size(dp2);
> > + p = dp_alloc(sz1 + sz2 + (2 * sizeof(END)));
> > + /* both dp1 and dp2 are non-null */
> > + if (!p)
> > + return NULL;
> > + ret = p;
> > + memcpy(p, dp1, sz1);
> > + p += sz1;
> > + memcpy(p, &END, sizeof(END));
> > + p += sizeof(END);
> > + memcpy(p, dp2, sz2);
> > + p += sz2;
> > + memcpy(p, &END, sizeof(END));
> > +
> > + return ret;
> > +}
> > +
> > struct efi_device_path *efi_dp_append_node(const struct efi_device_path
> > *dp,
> >const struct efi_device_path *node)
> > {
> > @@ -1160,3 +1195,40 @@ ssize_t efi_dp_check_length(const struct
> > efi_device_path *dp,
> > dp = (const struct efi_device_path *)((const u8 *)dp + len);
> > }
> > }
> > +
> > +/**
> > + * efi_dp_from_lo() - Get the instance of a Vendor Device Path
> > + * in a multi-instance device path that matches
> > + * a specific GUID
>
> The description does make it clear that you are looking for a VenMedia()
> node.
>
> Please, describe what the function is good for (find the device path for
> an initrd or dtb in a load option).
>
Ok
> > + *
> > + * @load_option: device paths to search
> > + * @size: size of the discovered device path
> > + * @guid: guid to search for
> > + *
> > + * Return: device path or NULL. Caller must free the returned value
>
> Please, keep the text aligned.
>
> Do we need a copy? Isn't a pointer good enough?
A pointer to what?
I think it's better to a copy here. This is a device path that might be used
out of a stack context were the load option might disappear.
Look at how the function is used in efi_get_dp_from_boot().
>
> > + */
> > +struct
> > +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> > + efi_uintn_t *size, efi_guid_t guid)
> > +{
> > + struct efi_device_path *fp = lo->file_path;
> > + struct efi_device_path_vendor *vendor;
> > + int lo_len = lo->file_path_length;
> > +
> > + while (lo_len) {
>
> lo_len must be at least sizeof(struct efi_device_path).
>
> > + if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
> > + fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH) {
> > + lo_len -= fp->length;
>
> Could the last device path in the array be followed by zero
Re: [PATCH 3/6] efi_loader: Introduce helper functions for EFI
On 05.03.21 23:22, Ilias Apalodimas wrote:
> A following patch introduces a different logic for loading initrd's
> based on the EFI_LOAD_FILE2_PROTOCOL.
> Since similar logic can be applied in the future for other system files
> (i.e DTBs), let's add some helper functions which will retrieve and
> parse file paths stored in EFI variables.
>
> Signed-off-by: Ilias Apalodimas
> ---
> include/efi_helper.h| 15 +
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_helper.c | 118
> 3 files changed, 134 insertions(+)
> create mode 100644 include/efi_helper.h
> create mode 100644 lib/efi_loader/efi_helper.c
>
> diff --git a/include/efi_helper.h b/include/efi_helper.h
> new file mode 100644
> index ..4980a1bb35d7
> --- /dev/null
> +++ b/include/efi_helper.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#if !defined _EFI_HELPER_H_
> +#define _EFI_HELPER_H
> +
> +#include
> +#include
> +
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
> +
> +#endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 10b42e8847bf..da2741adecfa 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -23,6 +23,7 @@ endif
> obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
> obj-y += efi_boottime.o
> +obj-y += efi_helper.o
> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> obj-y += efi_console.o
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> new file mode 100644
> index ..5437faa3ec49
> --- /dev/null
> +++ b/lib/efi_loader/efi_helper.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + * efi_get_var() - read value of an EFI variable
> + *
> + * @name:variable name
> + * @start: vendor GUID
> + * @size:size of allocated buffer
> + *
> + * Return: buffer with variable data or NULL
> + */
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
Please, move the function to lib/efi_loader/efi_var_common.c.
> +{
> + efi_status_t ret;
> + void *buf = NULL;
> +
> + *size = 0;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + buf = malloc(*size);
> + if (!buf)
> + return NULL;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + }
> +
> + if (ret != EFI_SUCCESS) {
> + free(buf);
> + *size = 0;
> + return NULL;
> + }
> +
> + return buf;
> +}
> +
> +/**
> + * create_boot_var_indexed() - Return Boot name were is replaced by
> + * the value of BootCurrent
> + *
> + * @var_name:variable name
> + * @var_name_size: size of var_name
> + *
> + * Return: Status code
> + */
> +static efi_status_t create_boot_var_indexed(u16 var_name[], size_t
> var_name_size)
The function name does not convey that it is related to BootCurrent.
How about efi_create_current_boot_var()?
> +{
> + efi_uintn_t boot_current_size;
> + efi_status_t ret;
> + u16 boot_current;
> + u16 *pos;
> +
> + boot_current_size = sizeof(boot_current);
> + ret = efi_get_variable_int(L"BootCurrent",
> +&efi_global_variable_guid, NULL,
> +&boot_current_size, &boot_current, NULL);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
> + boot_current);
> + if (!pos) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * efi_get_dp_from_boot() - Retrieve and return a device path from an EFI
> + * Boot### variable
> + *
> + * @guid:Vendor guid of the VenMedia DP
> + *
> + * Return: device path or NULL. Caller must free the returned value
> + */
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid)
> +{
> + struct efi_device_path *file_path;
> + struct efi_load_option lo;
> + void *var_value = NULL;
> + efi_uintn_t size;
> + efi_status_t ret;
> + u16 var_name[16];
> +
> + ret = create_boot_var_indexed(var_name, sizeof(var_name));
> + if (ret != EFI_SUCCESS)
> + return NULL;
> +
> + var_value = efi_get_var(var_name, &efi_global_variable_guid, &size);
> + if (!var_value)
> +
Re: [PATCH 2/4] net: phy: drop #ifdef CONFIG_DM_ETH around phy_connect_fixed
On Wed, Feb 17, 2021 at 6:48 AM Vladimir Oltean wrote: > > From: Vladimir Oltean > > In drivers/net/phy/Kconfig, CONFIG_PHY_FIXED already depends on > CONFIG_DM_ETH, so the function prototype definition when CONFIG_DM_ETH=n > does nothing, so it can be dropped. It is also never reachable, since > the whole function is already under #ifdef CONFIG_PHY_FIXED (which > again, as I said, depends on CONFIG_DM_ETH=y). > > Signed-off-by: Vladimir Oltean > --- > drivers/net/phy/phy.c | 6 -- > 1 file changed, 6 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH 02/20] net: phy: xilinx: Convert to use APIs which support live DT
Hi Vladimir, On Fri, Mar 5, 2021 at 7:05 AM Vladimir Oltean wrote: > > On Thu, Mar 04, 2021 at 01:36:45PM +0800, Bin Meng wrote: > > It looks that I need to rebase my series on top of yours? > > That would be nice, thank you. It would also create some sort of > incentive to finally get my patches accepted or at least looked at, they > are in the 'Needs Review/Ack' state, you seem to be quite the adequate > person for that :) I want to include your series as part of my v2 series, to remove dependencies for different maintainers. Are you okay with this? Regards, Bin
Re: [PATCH 02/20] net: phy: xilinx: Convert to use APIs which support live DT
On Thu, Mar 11, 2021 at 06:33:39PM +0800, Bin Meng wrote: > Hi Vladimir, > > On Fri, Mar 5, 2021 at 7:05 AM Vladimir Oltean > wrote: > > > > On Thu, Mar 04, 2021 at 01:36:45PM +0800, Bin Meng wrote: > > > It looks that I need to rebase my series on top of yours? > > > > That would be nice, thank you. It would also create some sort of > > incentive to finally get my patches accepted or at least looked at, they > > are in the 'Needs Review/Ack' state, you seem to be quite the adequate > > person for that :) > > I want to include your series as part of my v2 series, to remove > dependencies for different maintainers. > > Are you okay with this? > > Regards, > Bin Priyanka already merged your series and sent the pull request to Tom, frankly I don't think there's anything more for you to do.
[PATCH] mtd: spi-nor-ids: add dual and quad mode support to mx25l12805d
From: Stefan Herbrechtsmeier
The NOR flash MX25L12835F reuse the JEDEC ID of the MX25l12805D but
supports dual and quad mode.
Datasheet:
https://www.macronix.com/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf
Datasheet:
https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
Signed-off-by: Stefan Herbrechtsmeier
---
drivers/mtd/spi/spi-nor-ids.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 2b57797954..f6de2fb1b8 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -155,7 +155,7 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("mx25u1635e", 0xc22535, 0, 64 * 1024, 32, SECT_4K) },
{ INFO("mx25u3235f", 0xc22536, 0, 4 * 1024, 1024, SECT_4K) },
{ INFO("mx25u6435f", 0xc22537, 0, 64 * 1024, 128, SECT_4K) },
- { INFO("mx25l12805d", 0xc22018, 0, 64 * 1024, 256, SECT_4K) },
+ { INFO("mx25l12805d", 0xc22018, 0, 64 * 1024, 256, SECT_4K |
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ INFO("mx25u12835f", 0xc22538, 0, 64 * 1024, 256, SECT_4K) },
{ INFO("mx25l12855e", 0xc22618, 0, 64 * 1024, 256, 0) },
{ INFO("mx25l25635e", 0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ |
SPI_NOR_QUAD_READ) },
--
2.20.1
Re: [PATCH 02/20] net: phy: xilinx: Convert to use APIs which support live DT
Hi Vladimir, On Thu, Mar 11, 2021 at 6:37 PM Vladimir Oltean wrote: > > On Thu, Mar 11, 2021 at 06:33:39PM +0800, Bin Meng wrote: > > Hi Vladimir, > > > > On Fri, Mar 5, 2021 at 7:05 AM Vladimir Oltean > > wrote: > > > > > > On Thu, Mar 04, 2021 at 01:36:45PM +0800, Bin Meng wrote: > > > > It looks that I need to rebase my series on top of yours? > > > > > > That would be nice, thank you. It would also create some sort of > > > incentive to finally get my patches accepted or at least looked at, they > > > are in the 'Needs Review/Ack' state, you seem to be quite the adequate > > > person for that :) > > > > I want to include your series as part of my v2 series, to remove > > dependencies for different maintainers. > > > > Are you okay with this? > > > > Regards, > > Bin > > Priyanka already merged your series and sent the pull request to Tom, > frankly I don't think there's anything more for you to do. I did not see that. Did you mean the previous qemu ppc series? Regards, Bin
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On 11.03.21 10:10, Ilias Apalodimas wrote:
> On Thu, Mar 11, 2021 at 08:50:22AM +0100, Heinrich Schuchardt wrote:
>> On 3/5/21 11:22 PM, Ilias Apalodimas wrote:
>>> On the following patches we allow for an initrd path to be stored in
>>> Boot variables. Specifically we encode in the FIlePathList[] of
>>> the EFI_LOAD_OPTIONS for each Boot variable.
>>>
>>> The FilePathList[] array looks like this:
>>> kernel - 0xff - VenMedia(initrd GUID) - 0x01 - initrd1 - 0x01 initrd2 -0xff
>>> So let's add the relevant functions to concatenate and retrieve a device
>>> path based on a Vendor GUID.
>>>
>>> Signed-off-by: Ilias Apalodimas
>>> ---
>>> include/efi_loader.h | 4 ++
>>> lib/efi_loader/efi_device_path.c | 72
>>> 2 files changed, 76 insertions(+)
>>>
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index f470bbd636f4..eb11a8c7d4b1 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -738,6 +738,10 @@ struct efi_load_option {
>>> const u8 *optional_data;
>>> };
>>>
>>> +struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
>>> + efi_uintn_t *size, efi_guid_t guid);
>>> +struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
>>> + const struct efi_device_path *dp2);
>>> efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8
>>> *data,
>>> efi_uintn_t *size);
>>> unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8
>>> **data);
>>> diff --git a/lib/efi_loader/efi_device_path.c
>>> b/lib/efi_loader/efi_device_path.c
>>> index c9315dd45857..cf1321828e87 100644
>>> --- a/lib/efi_loader/efi_device_path.c
>>> +++ b/lib/efi_loader/efi_device_path.c
>>> @@ -310,6 +310,41 @@ struct efi_device_path *efi_dp_append(const struct
>>> efi_device_path *dp1,
>>> return ret;
>>> }
>>>
>>> +/** efi_dp_concat() - Concatenate 2 device paths. The final device path
>>> will contain
>>> + *two device paths separated by and end node (0xff).
>>> + *
>>> + * @dp1: First device path
>>> + * @size: Second device path
>>> + *
>>> + * Return: concatenated device path or NULL. Caller must free the returned
>>> value
>>> + */
>>> +struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
>>> + const struct efi_device_path *dp2)
>>> +{
>>> + struct efi_device_path *ret;
>>> + efi_uintn_t sz1, sz2;
>>> + void *p;
>>> +
>>> + if (!dp1 || !dp2)
>>> + return NULL;
>>> + sz1 = efi_dp_size(dp1);
>>> + sz2 = efi_dp_size(dp2);
>>> + p = dp_alloc(sz1 + sz2 + (2 * sizeof(END)));
>>> + /* both dp1 and dp2 are non-null */
>>> + if (!p)
>>> + return NULL;
>>> + ret = p;
>>> + memcpy(p, dp1, sz1);
>>> + p += sz1;
>>> + memcpy(p, &END, sizeof(END));
>>> + p += sizeof(END);
>>> + memcpy(p, dp2, sz2);
>>> + p += sz2;
>>> + memcpy(p, &END, sizeof(END));
>>> +
>>> + return ret;
>>> +}
>>> +
>>> struct efi_device_path *efi_dp_append_node(const struct efi_device_path
>>> *dp,
>>>const struct efi_device_path *node)
>>> {
>>> @@ -1160,3 +1195,40 @@ ssize_t efi_dp_check_length(const struct
>>> efi_device_path *dp,
>>> dp = (const struct efi_device_path *)((const u8 *)dp + len);
>>> }
>>> }
>>> +
>>> +/**
>>> + * efi_dp_from_lo() - Get the instance of a Vendor Device Path
>>> + * in a multi-instance device path that matches
>>> + * a specific GUID
>>
>> The description does make it clear that you are looking for a VenMedia()
>> node.
>>
>> Please, describe what the function is good for (find the device path for
>> an initrd or dtb in a load option).
>>
>
> Ok
>
>>> + *
>>> + * @load_option: device paths to search
>>> + * @size: size of the discovered device path
>>> + * @guid: guid to search for
>>> + *
>>> + * Return: device path or NULL. Caller must free the returned value
>>
>> Please, keep the text aligned.
>>
>> Do we need a copy? Isn't a pointer good enough?
>
> A pointer to what?
> I think it's better to a copy here. This is a device path that might be used
> out of a stack context were the load option might disappear.
> Look at how the function is used in efi_get_dp_from_boot().
You are duplicating in get_initrd_fp(). Why should we duplicate twice?
>
>>
>>> + */
>>> +struct
>>> +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
>>> + efi_uintn_t *size, efi_guid_t guid)
>>> +{
>>> + struct efi_device_path *fp = lo->file_path;
>>> + struct efi_device_path_vendor *vendor;
>>> + int lo_len = lo->file_path_length;
>>> +
>>> + while (lo_len) {
>>
>> lo_len must be at least sizeof(struct efi_device_path).
>>
>>> + if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
>>> + fp->sub_type != DEVICE_PATH_SUB
Re: [PATCH] arm: stm32mp1: Set soc_type, soc_pkg, soc_rev env variables
Hi Marek,
On 3/5/21 3:18 PM, Marek Vasut wrote:
Split up get_soc_name(), clean the decoding up a bit, and set up
environment variables which contain the SoC type, package, revision.
This is useful on SoMs, where multiple SoC options are populated.
Signed-off-by: Marek Vasut
Cc: Patrick Delaunay
Cc: Patrice Chotard
---
arch/arm/mach-stm32mp/cpu.c | 105 ++--
1 file changed, 53 insertions(+), 52 deletions(-)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index 3faa4ec18a4..887db9b5b0c 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -349,89 +349,78 @@ u32 get_cpu_package(void)
return get_otp(BSEC_OTP_PKG, PKG_SHIFT, PKG_MASK);
}
-void get_soc_name(char name[SOC_NAME_SIZE])
+static const char * const soc_type[] = {
+ "",
+ "151C", "151A", "151F", "151D",
+ "153C", "153A", "153F", "153D",
+ "157C", "157A", "157F", "157D"
+};
+
+static const char * const soc_pkg[] = { "??", "AD", "AC", "AB", "AA" };
+static const char * const soc_rev[] = { "?", "A", "B", "Z" };
+
+static void get_cpu_string_offsets(unsigned int *type, unsigned int *pkg,
+ unsigned int *rev)
{
- char *cpu_s, *cpu_r, *pkg;
+ u32 cpu_type = get_cpu_type();
+ u32 ct = cpu_type & ~(BIT(7) | BIT(0));
+ u32 cm = ((cpu_type & BIT(7)) >> 6) | (cpu_type & BIT(0));
+ u32 cp = get_cpu_package();
- /* MPUs Part Numbers */
- switch (get_cpu_type()) {
- case CPU_STM32MP157Fxx:
- cpu_s = "157F";
- break;
- case CPU_STM32MP157Dxx:
- cpu_s = "157D";
- break;
- case CPU_STM32MP157Cxx:
- cpu_s = "157C";
- break;
- case CPU_STM32MP157Axx:
- cpu_s = "157A";
- break;
- case CPU_STM32MP153Fxx:
- cpu_s = "153F";
- break;
- case CPU_STM32MP153Dxx:
- cpu_s = "153D";
+ /* Bits 0 and 7 are the ACDF, 00:C 01:A 10:F 11:D */
+ switch (ct) {
+ case CPU_STM32MP151Cxx:
+ *type = cm + 1;
break;
case CPU_STM32MP153Cxx:
- cpu_s = "153C";
- break;
- case CPU_STM32MP153Axx:
- cpu_s = "153A";
- break;
- case CPU_STM32MP151Fxx:
- cpu_s = "151F";
- break;
- case CPU_STM32MP151Dxx:
- cpu_s = "151D";
+ *type = cm + 5;
break;
- case CPU_STM32MP151Cxx:
- cpu_s = "151C";
- break;
- case CPU_STM32MP151Axx:
- cpu_s = "151A";
+ case CPU_STM32MP157Cxx:
+ *type = cm + 9;
break;
default:
- cpu_s = "";
+ *type = 0;
break;
}
/* Package */
- switch (get_cpu_package()) {
+ switch (cp) {
case PKG_AA_LBGA448:
- pkg = "AA";
- break;
case PKG_AB_LBGA354:
- pkg = "AB";
- break;
case PKG_AC_TFBGA361:
- pkg = "AC";
- break;
case PKG_AD_TFBGA257:
- pkg = "AD";
+ *pkg = cp;
break;
default:
./include/dt-bindings/pinctrl/stm32-pinfunc.h:37:
#define STM32MP_PKG_AA 0x1
#define STM32MP_PKG_AB 0x2
#define STM32MP_PKG_AC 0x4
#define STM32MP_PKG_AD 0x8
So it can't be used as index in array in the next part of the patch
=> soc_pkg[pkg]
need to parse a array ?
static const struct {
const u8 value;
char *;
} static const char * const soc_pkg[] =
{ { 0, "??"},
{ STM32MP_PKG_AA, "AA" },
{ STM32MP_PKG_AB, "AB" },
{ STM32MP_PKG_AC, "AC" },
{ STM32MP_PKG_AD, "AD" }};
- pkg = "??";
+ pkg = 0;
break;
}
- /* REVISION */
+ /* Revision */
switch (get_cpu_rev()) {
case CPU_REVA:
- cpu_r = "A";
+ *rev = 1;
break;
case CPU_REVB:
- cpu_r = "B";
+ *rev = 2;
break;
case CPU_REVZ:
- cpu_r = "Z";
+ *rev = 3;
For information rev = 2.1 => "Z" for STM32MP15 in STMicrolectonics
naming rules
normally
v1 => "A"
v2 => "B"
v3 => "C"
"Z" => first minor revision of the chip vX.1
"Y" => second minor revision of the chip => vX.2 or vY.1
It is a nightmare to have coherent naming rule, but I propose to use a
numericale value in "soc_rev"
/* Revision */
switch (get_cpu_rev()) {
case CPU_REVA:
*rev = 10;
break;
case CPU_REVB:
*rev = 20;
break;
case CPU_REVZ:
*rev = 21;
break;
Re: [PATCH 02/20] net: phy: xilinx: Convert to use APIs which support live DT
On Thu, Mar 11, 2021 at 06:58:33PM +0800, Bin Meng wrote: > Hi Vladimir, > > On Thu, Mar 11, 2021 at 6:37 PM Vladimir Oltean > wrote: > > > > On Thu, Mar 11, 2021 at 06:33:39PM +0800, Bin Meng wrote: > > > Hi Vladimir, > > > > > > On Fri, Mar 5, 2021 at 7:05 AM Vladimir Oltean > > > wrote: > > > > > > > > On Thu, Mar 04, 2021 at 01:36:45PM +0800, Bin Meng wrote: > > > > > It looks that I need to rebase my series on top of yours? > > > > > > > > That would be nice, thank you. It would also create some sort of > > > > incentive to finally get my patches accepted or at least looked at, they > > > > are in the 'Needs Review/Ack' state, you seem to be quite the adequate > > > > person for that :) > > > > > > I want to include your series as part of my v2 series, to remove > > > dependencies for different maintainers. > > > > > > Are you okay with this? > > > > > > Regards, > > > Bin > > > > Priyanka already merged your series and sent the pull request to Tom, > > frankly I don't think there's anything more for you to do. > > I did not see that. Did you mean the previous qemu ppc series? Ah, the series for the eTSEC in QEMU was different than the one for e500? Ok, in that case I'm wrong, your networking patches were not merged yet. If you could pick up my patches that would be nice. Let me know if there's anything I can help with.
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
Hi Heinrich
[...]
> >>> + * @load_option: device paths to search
> >>> + * @size: size of the discovered device path
> >>> + * @guid: guid to search for
> >>> + *
> >>> + * Return: device path or NULL. Caller must free the returned value
> >>
> >> Please, keep the text aligned.
> >>
> >> Do we need a copy? Isn't a pointer good enough?
> >
> > A pointer to what?
> > I think it's better to a copy here. This is a device path that might be used
> > out of a stack context were the load option might disappear.
> > Look at how the function is used in efi_get_dp_from_boot().
>
> You are duplicating in get_initrd_fp(). Why should we duplicate twice?
>
That's irrelevant though isn't it?
I did that in the efi initrd implementation. If someone else does the DTB in
the future and device to use efi_dp_from_lo return directly?
I'd much prefer an API (since that function goes into an API-related file for
device paths), that's safe and requires the user to free the memory, rather
than allowing him to accidentally shoot himself in the foot, keeping in mind
it's a single copy on a device path, which roughly adds anything on our boot
time.
> >
> >>
> >>> + */
> >>> +struct
> >>> +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> >>> + efi_uintn_t *size, efi_guid_t guid)
> >>> +{
> >>> + struct efi_device_path *fp = lo->file_path;
> >>> + struct efi_device_path_vendor *vendor;
> >>> + int lo_len = lo->file_path_length;
> >>> +
> >>> + while (lo_len) {
> >>
> >> lo_len must be at least sizeof(struct efi_device_path).
> >>
> >>> + if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
> >>> + fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH) {
> >>> + lo_len -= fp->length;
> >>
> >> Could the last device path in the array be followed by zero bytes for
> >> padding?
> >
> > How? Device paths are packed aren't they ?
> >
> >> Should we check that fp->length >= sizeof(struct efi_device_path)?
> >
> > Yea probably a good idea
>
> The content of the boot option comes from the user. Just assume that it
> can contain malicious content.
>
Yea the user doesn't add the device path directly though. The user adds
directories and a file, so the normalization is part of this function,
applied randomly and locally on a single input? or the device path creation
functions which this code uses? Since we use the pattern in a bunch of places
I assumed we did take care of that during the functions that create the device
paths. I haven't checked though ...
> We should also check that the identified device-path starting at
> VenMedia() ends within fp->length using efi_dp_check_length().
ok
>
> >
> >>
> >>> + fp = (void *)fp + fp->length;
> >>
> >> Please, avoid code duplication.
> >>
> >> E.g.
> >>
> >> for (; lo_len >= sizeof(struct efi_device_path);
> >> lo_len -= fp->length, fp = (void *)fp + fp->length) {
> >
> > I can an switch to that, but I really never liked this format.
> > It always seemed way less readable to me for some reason. Maybe because I
> > never got used to it ...
>
> Using "for" is only one option. You could use "goto next;" instead.
>
I really don't mind, I can just use what you propose.
> >
> >>
> >>> + continue;
> >>> + }
> >>> +
> >>> + vendor = (struct efi_device_path_vendor *)fp;
> >>> + if (!guidcmp(&vendor->guid, &guid))
> >>> + return efi_dp_dup(fp);
> >>
> >> Should we strip of the VenMedia() node here?
> >
> > Why? This is not supposed to get the file path. The function says "get
> > device
> > path from load option" and that device includes the VenMedia node.
> > It would make more sense for me to strip in efi_get_dp_from_boot() for
> > example, if you want a helper function to get the initrd path *only*.
>
> The VenMedia() node is not needed anymore once you have found the entry.
>
Yea it's not but as I said the name of the function says "get the *stored*
from a boot option. Not get the one that suits us.
There's another reason for that btw, the initrd related functions use that
(specifically get_initrd_fp()), to figure out if the Boot variable
contains an initrd path or not.
If the VenMedia is not present at all, the protocol is not installed allowing
the kernel to fallback in it's command line 'initrd=' option.
If the VenMedia is there though, we are checking the file path of the initrd
and if the file's not found we return an error allowing Bootmgr to fallback.
If we 'just' return the initrd path, we'll have to introduce another variable
in the function, indicating if the VenMedia is present or not so the rest
ofthe codepath can decide what to do.
> >
> > But really it's just one invocation of efi_dp_get_next_instance() after
> > whatever device path you get. Which also modifies the device path pointer,
> > so
> > I'd really prefer keeping that call in a local context.
>
> Why next instance? I thought next node.
>
>
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
Am 11. März 2021 12:36:04 MEZ schrieb Ilias Apalodimas
:
>Hi Heinrich
>
>[...]
>
>> >>> + * @load_option: device paths to search
>> >>> + * @size:size of the discovered device path
>> >>> + * @guid:guid to search for
>> >>> + *
>> >>> + * Return: device path or NULL. Caller must free the returned
>value
>> >>
>> >> Please, keep the text aligned.
>> >>
>> >> Do we need a copy? Isn't a pointer good enough?
>> >
>> > A pointer to what?
>> > I think it's better to a copy here. This is a device path that
>might be used
>> > out of a stack context were the load option might disappear.
>> > Look at how the function is used in efi_get_dp_from_boot().
>>
>> You are duplicating in get_initrd_fp(). Why should we duplicate
>twice?
>>
>
>That's irrelevant though isn't it?
>I did that in the efi initrd implementation. If someone else does the
>DTB in
>the future and device to use efi_dp_from_lo return directly?
>I'd much prefer an API (since that function goes into an API-related
>file for
>device paths), that's safe and requires the user to free the memory,
>rather
>than allowing him to accidentally shoot himself in the foot, keeping in
>mind
>it's a single copy on a device path, which roughly adds anything on our
>boot
>time.
>
>> >
>> >>
>> >>> + */
>> >>> +struct
>> >>> +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
>> >>> +efi_uintn_t *size, efi_guid_t guid)
>> >>> +{
>> >>> +struct efi_device_path *fp = lo->file_path;
>> >>> +struct efi_device_path_vendor *vendor;
>> >>> +int lo_len = lo->file_path_length;
>> >>> +
>> >>> +while (lo_len) {
>> >>
>> >> lo_len must be at least sizeof(struct efi_device_path).
>> >>
>> >>> +if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
>> >>> +fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH) {
>> >>> +lo_len -= fp->length;
>> >>
>> >> Could the last device path in the array be followed by zero bytes
>for
>> >> padding?
>> >
>> > How? Device paths are packed aren't they ?
>> >
>> >> Should we check that fp->length >= sizeof(struct efi_device_path)?
>> >
>> > Yea probably a good idea
>>
>> The content of the boot option comes from the user. Just assume that
>it
>> can contain malicious content.
>>
>
>Yea the user doesn't add the device path directly though. The user adds
>directories and a file, so the normalization is part of this function,
>applied randomly and locally on a single input? or the device path
>creation
>functions which this code uses? Since we use the pattern in a bunch of
>places
>I assumed we did take care of that during the functions that create the
>device
>paths. I haven't checked though ...
I am not referring to efidebug.
The user can update EFI variables with any binary content using an EFI binary
or OS functions.
E.g. copy a binary file to the efi variables file system in Linux.
>
>> We should also check that the identified device-path starting at
>> VenMedia() ends within fp->length using efi_dp_check_length().
>
>ok
>
>>
>> >
>> >>
>> >>> +fp = (void *)fp + fp->length;
>> >>
>> >> Please, avoid code duplication.
>> >>
>> >> E.g.
>> >>
>> >> for (; lo_len >= sizeof(struct efi_device_path);
>> >> lo_len -= fp->length, fp = (void *)fp + fp->length) {
>> >
>> > I can an switch to that, but I really never liked this format.
>> > It always seemed way less readable to me for some reason. Maybe
>because I
>> > never got used to it ...
>>
>> Using "for" is only one option. You could use "goto next;" instead.
>>
>
>I really don't mind, I can just use what you propose.
As long as you avoid code duplication I am fine.
Best regards
Heinrich
>
>> >
>> >>
>> >>> +continue;
>> >>> +}
>> >>> +
>> >>> +vendor = (struct efi_device_path_vendor *)fp;
>> >>> +if (!guidcmp(&vendor->guid, &guid))
>> >>> +return efi_dp_dup(fp);
>> >>
>> >> Should we strip of the VenMedia() node here?
>> >
>> > Why? This is not supposed to get the file path. The function says
>"get device
>> > path from load option" and that device includes the VenMedia node.
>> > It would make more sense for me to strip in efi_get_dp_from_boot()
>for
>> > example, if you want a helper function to get the initrd path
>*only*.
>>
>> The VenMedia() node is not needed anymore once you have found the
>entry.
>>
>
>Yea it's not but as I said the name of the function says "get the
>*stored*
>from a boot option. Not get the one that suits us.
>There's another reason for that btw, the initrd related functions use
>that
>(specifically get_initrd_fp()), to figure out if the Boot variable
>contains an initrd path or not.
>If the VenMedia is not present at all, the protocol is not installed
>allowing
>the kernel to fallback in it's command line 'initrd=' option.
>If the VenMedia is there though, we are checking the file
Re: [PATCH] mtd: spi-nor-ids: add dual and quad mode support to mx25l12805d
Hi Stefan,
On 11/03/21 10:55AM, Stefan Herbrechtsmeier wrote:
> From: Stefan Herbrechtsmeier
>
> The NOR flash MX25L12835F reuse the JEDEC ID of the MX25l12805D but
> supports dual and quad mode.
There is some discussion going on over at the Linux side [0] about these
two flashes and how to properly support them. I suggest this you hold on
to this patch until a proper solution is found and agreed upon for this.
Or even better, you can help drive the solution forward on Linux as
well.
[0]
https://lore.kernel.org/linux-mtd/caeymn7zep9f1sue6umrdwkr8bvt5hdri-4f3+g-gp9anugg...@mail.gmail.com/t/
>
> Datasheet:
> https://www.macronix.com/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf
> Datasheet:
> https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
>
> Signed-off-by: Stefan Herbrechtsmeier
>
> ---
>
> drivers/mtd/spi/spi-nor-ids.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index 2b57797954..f6de2fb1b8 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -155,7 +155,7 @@ const struct flash_info spi_nor_ids[] = {
> { INFO("mx25u1635e", 0xc22535, 0, 64 * 1024, 32, SECT_4K) },
> { INFO("mx25u3235f", 0xc22536, 0, 4 * 1024, 1024, SECT_4K) },
> { INFO("mx25u6435f", 0xc22537, 0, 64 * 1024, 128, SECT_4K) },
> - { INFO("mx25l12805d", 0xc22018, 0, 64 * 1024, 256, SECT_4K) },
> + { INFO("mx25l12805d", 0xc22018, 0, 64 * 1024, 256, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> { INFO("mx25u12835f", 0xc22538, 0, 64 * 1024, 256, SECT_4K) },
> { INFO("mx25l12855e", 0xc22618, 0, 64 * 1024, 256, 0) },
> { INFO("mx25l25635e", 0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ) },
> --
> 2.20.1
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
Re: [PATCH 4/6] efi_loader: Replace config option for initrd loading
On 05.03.21 23:23, Ilias Apalodimas wrote:
> Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd
> unconditionally. Although we correctly return various EFI exit codes
> depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the
> kernel loader, only falls back to the cmdline interpreted initrd if the
> protocol is not installed.
>
> This creates a problem for EFI installers, since they won't be able to
> load their own initrd and continue the installation. It also makes the
> feature hard to use, since we can either have a single initrd or we have
> to recompile u-boot if the filename changes.
>
> So let's introduce a different logic that will decouple the initrd
> path from the config option we currently have.
> When defining a UEFI Boot we can use the filepathlist and store
> a file path pointing to our initrd. Specifically the EFI spec describes:
>
> "The first element of the array is a device path that describes the device
> and location of the Image for this load option. Other device paths may
> optionally exist in the FilePathList, but their usage is OSV specific"
>
> When the EFI application is launched through the bootmgr, we'll try to
> interpret the extra device path. If that points to a file that exists on
> our disk, we'll now install the load_file2 and the efi-stub will be able
> to use it.
>
> This opens up another path using U-Boot and defines a new boot flow.
> A user will be able to control the kernel/initrd pairs without explicit
> cmdline args or GRUB.
>
> Signed-off-by: Ilias Apalodimas
> ---
> cmd/bootefi.c| 3 +
> include/efi_loader.h | 1 +
> lib/efi_loader/Kconfig | 15 +--
> lib/efi_loader/efi_bootmgr.c | 3 +
> lib/efi_loader/efi_load_initrd.c | 209 +--
> 5 files changed, 152 insertions(+), 79 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 271b385edea6..cba81ffe75e4 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle,
> void *load_options)
>
> free(load_options);
>
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
> + efi_initrd_deregister();
> +
> return ret;
> }
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index eb11a8c7d4b1..0d4f5d9acc9f 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -431,6 +431,7 @@ efi_status_t efi_net_register(void);
> /* Called by bootefi to make the watchdog available */
> efi_status_t efi_watchdog_register(void);
> efi_status_t efi_initrd_register(void);
> +void efi_initrd_deregister(void);
> /* Called by bootefi to make SMBIOS tables available */
> /**
> * efi_acpi_register() - write out ACPI tables
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e729f727df11..029f0e515f57 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD
> bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
> default n
> help
> - Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
> - use to load the initial ramdisk. Once this is enabled using
> - initrd= will stop working.
> -
> -config EFI_INITRD_FILESPEC
> - string "initramfs path"
> - default "host 0:1 initrd"
> - depends on EFI_LOAD_FILE2_INITRD
> - help
> - Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> + Linux v5.7 and later can make use of this option. If the boot
> option
> + selected by the UEFI boot manager specifies an existing file to
> be used
> + as initial RAM disk, a Linux specific Load File2 protocol will
> be
> + installed and Linux 5.7+ will ignore any initrd=
> command line
> + argument.
>
> config EFI_SECURE_BOOT
> bool "Enable EFI secure boot support"
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 25f5cebfdb67..d1baa8c71a4d 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -118,6 +118,9 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t
> *handle,
> ret = efi_set_variable_int(L"BootCurrent",
> &efi_global_variable_guid,
> attributes, sizeof(n), &n, false);
Why would you continue if BootCurrent is not set?
> + /* try to register load file2 for initrd's */
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
> + ret |= efi_initrd_register();
ret is not a boolean. So |= does not make sense to me here.
> if (ret != EFI_SUCCESS) {
> if (EFI_CALL(efi_unload_image(*handle))
> != EFI_SUCCESS)
> diff --git a/lib/efi_loader/efi_load_initrd.c
> b/lib/efi_loader/efi_load_initrd.c
> index b9ee8839054f..b11c5ee2
[PATCH] mtd: rawnand: fsl_ifc: fix FSL NAND driver to read all ONFI parameter pages
Linux commit a75bbe71a27 ("mtd: rawnand: fsl_ifc: fix FSL NAND driver to
read all ONFI parameter pages")
Per ONFI specification (Rev. 4.0), if the CRC of the first parameter page
read is not valid, the host should read redundant parameter page copies.
Fix FSL NAND driver to read the two redundant copies which are mandatory
in the specification.
Signed-off-by: Jane Wan
Signed-off-by: Boris Brezillon
Signed-off-by: Maxim Kochetkov
---
drivers/mtd/nand/raw/fsl_ifc_nand.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index cf20238782..e5ff937872 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -411,9 +411,16 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned
int command,
/* READID must read all possible bytes while CEB is active */
case NAND_CMD_READID:
case NAND_CMD_PARAM: {
+ /*
+* For READID, read 8 bytes that are currently used.
+* For PARAM, read all 3 copies of 256-bytes pages.
+*/
+ int len = 8;
int timing = IFC_FIR_OP_RB;
- if (command == NAND_CMD_PARAM)
+ if (command == NAND_CMD_PARAM) {
timing = IFC_FIR_OP_RBCD;
+ len = 256 * 3;
+ }
ifc_out32(&ifc->ifc_nand.nand_fir0,
(IFC_FIR_OP_CW0 << IFC_NAND_FIR0_OP0_SHIFT) |
@@ -423,12 +430,8 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned
int command,
command << IFC_NAND_FCR0_CMD0_SHIFT);
ifc_out32(&ifc->ifc_nand.row3, column);
- /*
-* although currently it's 8 bytes for READID, we always read
-* the maximum 256 bytes(for PARAM)
-*/
- ifc_out32(&ifc->ifc_nand.nand_fbcr, 256);
- ctrl->read_bytes = 256;
+ ifc_out32(&ifc->ifc_nand.nand_fbcr, len);
+ ctrl->read_bytes = len;
set_addr(mtd, 0, 0, 0);
fsl_ifc_run_command(mtd);
--
2.30.2
[PATCH 0/4] Add rt-thread art-pi board support
From: dillon min These patches aim to adds u-boot support on art-pi board. the board resources: - stm32h750xbh6 128k flash, 1024k sram - 32MiB sdram - 16MiB spi flash - 8MiB qspi flash - onboard wifi, bt, fm the detail board information can be found at: https://art-pi.gitee.io/website/ dillon min (4): ARM: dts: stm32: split sdram pin & timing parameter into specific board dts ARM: dts: stm32: Add RT-Thread art-pi board support board: Add rt-thread art-pi board support ram: stm32: fix strsep failed on read only memory arch/arm/dts/Makefile| 3 +- arch/arm/dts/stm32h7-u-boot.dtsi | 95 arch/arm/dts/stm32h743.dtsi | 8 + arch/arm/dts/stm32h743i-disco-u-boot.dtsi| 98 arch/arm/dts/stm32h743i-eval-u-boot.dtsi | 98 arch/arm/dts/stm32h750-pinctrl.dtsi | 319 +++ arch/arm/dts/stm32h750.dtsi | 5 + arch/arm/dts/stm32h750i-art-pi-u-boot.dtsi | 81 +++ arch/arm/dts/stm32h750i-art-pi.dts | 75 +++ arch/arm/mach-stm32/stm32h7/Kconfig | 4 + board/st/stm32h750-art-pi/Kconfig| 19 ++ board/st/stm32h750-art-pi/MAINTAINERS| 7 + board/st/stm32h750-art-pi/Makefile | 6 + board/st/stm32h750-art-pi/stm32h750-art-pi.c | 58 + configs/stm32h750-art-pi_defconfig | 51 + drivers/ram/stm32_sdram.c| 3 + include/configs/stm32h750-art-pi.h | 48 include/dt-bindings/memory/stm32-sdram.h | 2 + 18 files changed, 884 insertions(+), 96 deletions(-) create mode 100644 arch/arm/dts/stm32h750-pinctrl.dtsi create mode 100644 arch/arm/dts/stm32h750.dtsi create mode 100644 arch/arm/dts/stm32h750i-art-pi-u-boot.dtsi create mode 100644 arch/arm/dts/stm32h750i-art-pi.dts create mode 100644 board/st/stm32h750-art-pi/Kconfig create mode 100644 board/st/stm32h750-art-pi/MAINTAINERS create mode 100644 board/st/stm32h750-art-pi/Makefile create mode 100644 board/st/stm32h750-art-pi/stm32h750-art-pi.c create mode 100644 configs/stm32h750-art-pi_defconfig create mode 100644 include/configs/stm32h750-art-pi.h -- 2.7.4
[PATCH 1/4] ARM: dts: stm32: split sdram pin & timing parameter into specific board dts
From: dillon min
As different boards has their own sdram hw connection, mount different
sdram modules, so move sdram timing parameter and pin configuration
to their board device tree.
Signed-off-by: dillon min
---
arch/arm/dts/stm32h7-u-boot.dtsi | 95 --
arch/arm/dts/stm32h743i-disco-u-boot.dtsi | 98 +++
arch/arm/dts/stm32h743i-eval-u-boot.dtsi | 98 +++
3 files changed, 196 insertions(+), 95 deletions(-)
diff --git a/arch/arm/dts/stm32h7-u-boot.dtsi b/arch/arm/dts/stm32h7-u-boot.dtsi
index 54dd406..e34d066 100644
--- a/arch/arm/dts/stm32h7-u-boot.dtsi
+++ b/arch/arm/dts/stm32h7-u-boot.dtsi
@@ -36,30 +36,6 @@
pinctrl-0 = <&fmc_pins>;
pinctrl-names = "default";
status = "okay";
-
- /*
-* Memory configuration from sdram datasheet
IS42S32800G-6BLI
-* first bank is bank@0
-* second bank is bank@1
-*/
- bank1: bank@1 {
- st,sdram-control = /bits/ 8 ;
- st,sdram-timing = /bits/ 8 ;
- st,sdram-refcount = <1539>;
- };
};
};
};
@@ -136,77 +112,6 @@
compatible = "st,stm32-gpio";
};
-&pinctrl {
- fmc_pins: fmc@0 {
- pins {
- pinmux = ,
-,
-,
-,
-,
-,
-,
-
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-
-,
-,
-,
-,
-,
-,
-,
-
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-
-,
-,
-,
-,
-,
-,
-,
-,
-,
-;
-
- slew-rate = <3>;
- };
- };
-};
-
&pwrcfg {
u-boot,dm-pre-reloc;
};
diff --git a/arch/arm/dts/stm32h743i-disco-u-boot.dtsi
b/arch/arm/dts/stm32h743i-disco-u-boot.dtsi
index 5965afc..02e28c6 100644
--- a/arch/arm/dts/stm32h743i-disco-u-boot.dtsi
+++ b/arch/arm/dts/stm32h743i-disco-u-boot.dtsi
@@ -1,3 +1,101 @@
// SPDX-License-Identifier: GPL-2.0+
#include
+
+&fmc {
+
+ /*
+* Memory configuration from sdram datasheet IS42S32800G-6BLI
+* first bank is bank@0
+* second bank is bank@1
+*/
+ bank1: bank@1 {
+ st,sdram-control = /bits/ 8 ;
+ st,sdram-timing = /bits/ 8 ;
+ st,sdram-refcount = <1539>;
+ };
+};
+
+&pinctrl {
+ fmc_pins: fmc@0 {
+ pins {
+ pinmux = ,
+,
+,
+,
+,
+,
+,
+
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+
+
[PATCH 2/4] ARM: dts: stm32: Add RT-Thread art-pi board support
From: dillon min
All these files are add for support rt-thread art-pi board
the detail board information can be found at:
https://art-pi.gitee.io/website/
board resources:
- stm32h750xbh6 128k flash, 1024k sram
- 32MiB sdram
- 16MiB spi flash
- 8MiB qspi flash
- onboard wifi, bt, fm
Signed-off-by: dillon min
---
arch/arm/dts/Makefile | 3 +-
arch/arm/dts/stm32h743.dtsi| 8 +
arch/arm/dts/stm32h750-pinctrl.dtsi| 319 +
arch/arm/dts/stm32h750.dtsi| 5 +
arch/arm/dts/stm32h750i-art-pi-u-boot.dtsi | 81
arch/arm/dts/stm32h750i-art-pi.dts | 75 +++
include/dt-bindings/memory/stm32-sdram.h | 2 +
7 files changed, 492 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/dts/stm32h750-pinctrl.dtsi
create mode 100644 arch/arm/dts/stm32h750.dtsi
create mode 100644 arch/arm/dts/stm32h750i-art-pi-u-boot.dtsi
create mode 100644 arch/arm/dts/stm32h750i-art-pi.dts
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index c671082..0f54801 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -454,7 +454,8 @@ dtb-$(CONFIG_STM32F7) += stm32f746-disco.dtb \
stm32f769-disco.dtb \
stm32746g-eval.dtb
dtb-$(CONFIG_STM32H7) += stm32h743i-disco.dtb \
- stm32h743i-eval.dtb
+ stm32h743i-eval.dtb \
+ stm32h750i-art-pi.dtb
dtb-$(CONFIG_MACH_SUN4I) += \
sun4i-a10-a1000.dtb \
diff --git a/arch/arm/dts/stm32h743.dtsi b/arch/arm/dts/stm32h743.dtsi
index e4e4723..bb3e2bf 100644
--- a/arch/arm/dts/stm32h743.dtsi
+++ b/arch/arm/dts/stm32h743.dtsi
@@ -99,6 +99,14 @@
clocks = <&rcc USART2_CK>;
};
+ uart4: serial@40004c00 {
+ compatible = "st,stm32h7-uart";
+ reg = <0x40004c00 0x400>;
+ interrupts = <52>;
+ status = "disabled";
+ clocks = <&rcc UART4_CK>;
+ };
+
i2c1: i2c@40005400 {
compatible = "st,stm32f7-i2c";
#address-cells = <1>;
diff --git a/arch/arm/dts/stm32h750-pinctrl.dtsi
b/arch/arm/dts/stm32h750-pinctrl.dtsi
new file mode 100644
index 000..dd88ace
--- /dev/null
+++ b/arch/arm/dts/stm32h750-pinctrl.dtsi
@@ -0,0 +1,319 @@
+/*
+ * Copyright 2017 - Alexandre Torgue
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This file is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include
+
+/ {
+ soc {
+ pinctrl: pin-controller {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "st,stm32h743-pinctrl";
+ ranges = <0 0x5802 0x3000>;
+ interrupt-parent = <&exti>;
+ st,syscfg = <&syscfg 0x8>;
+ pins-are-numbered;
+
+ gpioa: gpio@5802 {
+ gpio-controller;
+ #gpio-cells = <2>;
+
[PATCH 3/4] board: Add rt-thread art-pi board support
From: dillon min
This patch adds support for rt-thread art-pi board.
for more information about art-pi, please goto:
https://art-pi.gitee.io/website/
Signed-off-by: dillon min
---
arch/arm/mach-stm32/stm32h7/Kconfig | 4 ++
board/st/stm32h750-art-pi/Kconfig| 19 +
board/st/stm32h750-art-pi/MAINTAINERS| 7
board/st/stm32h750-art-pi/Makefile | 6 +++
board/st/stm32h750-art-pi/stm32h750-art-pi.c | 58
configs/stm32h750-art-pi_defconfig | 51
include/configs/stm32h750-art-pi.h | 48 +++
7 files changed, 193 insertions(+)
create mode 100644 board/st/stm32h750-art-pi/Kconfig
create mode 100644 board/st/stm32h750-art-pi/MAINTAINERS
create mode 100644 board/st/stm32h750-art-pi/Makefile
create mode 100644 board/st/stm32h750-art-pi/stm32h750-art-pi.c
create mode 100644 configs/stm32h750-art-pi_defconfig
create mode 100644 include/configs/stm32h750-art-pi.h
diff --git a/arch/arm/mach-stm32/stm32h7/Kconfig
b/arch/arm/mach-stm32/stm32h7/Kconfig
index 55e6217..70233a4 100644
--- a/arch/arm/mach-stm32/stm32h7/Kconfig
+++ b/arch/arm/mach-stm32/stm32h7/Kconfig
@@ -6,7 +6,11 @@ config TARGET_STM32H743_DISCO
config TARGET_STM32H743_EVAL
bool "STM32H743 Evaluation board"
+config TARGET_STM32H750_ART_PI
+ bool "STM32H750 ART Pi board"
+
source "board/st/stm32h743-eval/Kconfig"
source "board/st/stm32h743-disco/Kconfig"
+source "board/st/stm32h750-art-pi/Kconfig"
endif
diff --git a/board/st/stm32h750-art-pi/Kconfig
b/board/st/stm32h750-art-pi/Kconfig
new file mode 100644
index 000..c31b984
--- /dev/null
+++ b/board/st/stm32h750-art-pi/Kconfig
@@ -0,0 +1,19 @@
+if TARGET_STM32H750_ART_PI
+
+config SYS_BOARD
+ string
+ default "stm32h750-art-pi"
+
+config SYS_VENDOR
+ string
+ default "st"
+
+config SYS_SOC
+ string
+ default "stm32h7"
+
+config SYS_CONFIG_NAME
+ string
+ default "stm32h750-art-pi"
+
+endif
diff --git a/board/st/stm32h750-art-pi/MAINTAINERS
b/board/st/stm32h750-art-pi/MAINTAINERS
new file mode 100644
index 000..9578833
--- /dev/null
+++ b/board/st/stm32h750-art-pi/MAINTAINERS
@@ -0,0 +1,7 @@
+STM32H750 ART PI BOARD
+M: Dillon Min
+S: Maintained
+F: board/st/stm32h750-art-pi
+F: include/configs/stm32h750-art-pi.h
+F: configs/stm32h750-art-pi_defconfig
+F: arch/arm/dts/stm32h7*
diff --git a/board/st/stm32h750-art-pi/Makefile
b/board/st/stm32h750-art-pi/Makefile
new file mode 100644
index 000..a06de87
--- /dev/null
+++ b/board/st/stm32h750-art-pi/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2021, RT-Thread - All Rights Reserved
+# Author(s): Dillon Min, for RT-Thread.
+
+obj-y := stm32h750-art-pi.o
diff --git a/board/st/stm32h750-art-pi/stm32h750-art-pi.c
b/board/st/stm32h750-art-pi/stm32h750-art-pi.c
new file mode 100644
index 000..405836a
--- /dev/null
+++ b/board/st/stm32h750-art-pi/stm32h750-art-pi.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
+ * Author(s): Patrice Chotard, for
STMicroelectronics.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int dram_init(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ ret = uclass_get_device(UCLASS_RAM, 0, &dev);
+ if (ret) {
+ debug("DRAM init failed: %d\n", ret);
+ return ret;
+ }
+
+ if (fdtdec_setup_mem_size_base() != 0)
+ ret = -EINVAL;
+
+ return ret;
+}
+
+int dram_init_banksize(void)
+{
+ fdtdec_setup_memory_banksize();
+
+ return 0;
+}
+
+int board_early_init_f(void)
+{
+ return 0;
+}
+
+u32 get_board_rev(void)
+{
+ return 0;
+}
+
+int board_late_init(void)
+{
+ return 0;
+}
+
+int board_init(void)
+{
+ gd->bd->bi_boot_params = gd->bd->bi_dram[0].start + 0x100;
+ return 0;
+}
diff --git a/configs/stm32h750-art-pi_defconfig
b/configs/stm32h750-art-pi_defconfig
new file mode 100644
index 000..a5c4cd7
--- /dev/null
+++ b/configs/stm32h750-art-pi_defconfig
@@ -0,0 +1,51 @@
+CONFIG_ARM=y
+CONFIG_ARCH_STM32=y
+CONFIG_SYS_TEXT_BASE=0x9000
+CONFIG_SYS_MALLOC_F_LEN=0xF00
+CONFIG_NR_DRAM_BANKS=1
+CONFIG_ENV_SIZE=0x2000
+CONFIG_STM32H7=y
+CONFIG_TARGET_STM32H750_ART_PI=y
+CONFIG_DEFAULT_DEVICE_TREE="stm32h750i-art-pi"
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_AUTOBOOT_KEYED=y
+CONFIG_AUTOBOOT_PROMPT="Hit SPACE in %d seconds to stop autoboot.\n"
+CONFIG_AUTOBOOT_STOP_STR=" "
+# CONFIG_USE_BOOTCOMMAND is not set
+CONFIG_DEFAULT_FDT_FILE="stm32h750i-art-pi"
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_BOARD_LATE_INIT=y
+CONFIG_SYS_PROMPT="U-Boot > "
+CONFIG_CMD_GPT=y
+CONFIG_CMD_MMC=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIMER=y
+CONFIG_CMD_EXT4_
[PATCH 4/4] ram: stm32: fix strsep failed on read only memory
From: dillon min
strsep will change data from original memory address,
in case the memory is in non-sdram/sram place, will
run into a bug(hang at SDRAM: )
just add a temporary array to store bank_name[] to fix this
bug.
Fixes: f303aaf ("ram: stm32: add second SDRAM bank management")
Signed-off-by: dillon min
---
drivers/ram/stm32_sdram.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/ram/stm32_sdram.c b/drivers/ram/stm32_sdram.c
index 540ad85..da27677 100644
--- a/drivers/ram/stm32_sdram.c
+++ b/drivers/ram/stm32_sdram.c
@@ -268,6 +268,7 @@ static int stm32_fmc_of_to_plat(struct udevice *dev)
u32 swp_fmc;
ofnode bank_node;
char *bank_name;
+ char _bank_name[128] = {0};
u8 bank = 0;
int ret;
@@ -300,6 +301,8 @@ static int stm32_fmc_of_to_plat(struct udevice *dev)
dev_for_each_subnode(bank_node, dev) {
/* extract the bank index from DT */
bank_name = (char *)ofnode_get_name(bank_node);
+ strcpy(_bank_name, bank_name);
+ bank_name = _bank_name;
strsep(&bank_name, "@");
if (!bank_name) {
pr_err("missing sdram bank index");
--
2.7.4
Re: [PATCH 4/6] efi_loader: Replace config option for initrd loading
On Thu, Mar 11, 2021 at 01:23:05PM +0100, Heinrich Schuchardt wrote: > On 05.03.21 23:23, Ilias Apalodimas wrote: > > Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd > > unconditionally. Although we correctly return various EFI exit codes > > depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the > > kernel loader, only falls back to the cmdline interpreted initrd if the > > protocol is not installed. > > > > This creates a problem for EFI installers, since they won't be able to > > load their own initrd and continue the installation. It also makes the > > feature hard to use, since we can either have a single initrd or we have > > to recompile u-boot if the filename changes. > > > > So let's introduce a different logic that will decouple the initrd > > path from the config option we currently have. > > When defining a UEFI Boot we can use the filepathlist and store > > a file path pointing to our initrd. Specifically the EFI spec describes: > > > > "The first element of the array is a device path that describes the device > > and location of the Image for this load option. Other device paths may > > optionally exist in the FilePathList, but their usage is OSV specific" > > > > When the EFI application is launched through the bootmgr, we'll try to > > interpret the extra device path. If that points to a file that exists on > > our disk, we'll now install the load_file2 and the efi-stub will be able > > to use it. > > > > This opens up another path using U-Boot and defines a new boot flow. > > A user will be able to control the kernel/initrd pairs without explicit > > cmdline args or GRUB. > > > > Signed-off-by: Ilias Apalodimas > > --- > > cmd/bootefi.c| 3 + > > include/efi_loader.h | 1 + > > lib/efi_loader/Kconfig | 15 +-- > > lib/efi_loader/efi_bootmgr.c | 3 + > > lib/efi_loader/efi_load_initrd.c | 209 +-- > > 5 files changed, 152 insertions(+), 79 deletions(-) > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > > index 271b385edea6..cba81ffe75e4 100644 > > --- a/cmd/bootefi.c > > +++ b/cmd/bootefi.c > > @@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t > > handle, void *load_options) > > > > free(load_options); > > > > + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) > > + efi_initrd_deregister(); > > + > > return ret; > > } > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index eb11a8c7d4b1..0d4f5d9acc9f 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -431,6 +431,7 @@ efi_status_t efi_net_register(void); > > /* Called by bootefi to make the watchdog available */ > > efi_status_t efi_watchdog_register(void); > > efi_status_t efi_initrd_register(void); > > +void efi_initrd_deregister(void); > > /* Called by bootefi to make SMBIOS tables available */ > > /** > > * efi_acpi_register() - write out ACPI tables > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index e729f727df11..029f0e515f57 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD > > bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk" > > default n > > help > > - Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can > > - use to load the initial ramdisk. Once this is enabled using > > - initrd= will stop working. > > - > > -config EFI_INITRD_FILESPEC > > - string "initramfs path" > > - default "host 0:1 initrd" > > - depends on EFI_LOAD_FILE2_INITRD > > - help > > - Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz. > > + Linux v5.7 and later can make use of this option. If the boot > > option > > + selected by the UEFI boot manager specifies an existing file to > > be used > > + as initial RAM disk, a Linux specific Load File2 protocol will > > be > > + installed and Linux 5.7+ will ignore any initrd= > > command line > > + argument. > > > > config EFI_SECURE_BOOT > > bool "Enable EFI secure boot support" > > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > > index 25f5cebfdb67..d1baa8c71a4d 100644 > > --- a/lib/efi_loader/efi_bootmgr.c > > +++ b/lib/efi_loader/efi_bootmgr.c > > @@ -118,6 +118,9 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t > > *handle, > > ret = efi_set_variable_int(L"BootCurrent", > >&efi_global_variable_guid, > >attributes, sizeof(n), &n, false); > > Why would you continue if BootCurrent is not set? Because the bitops below is just trying to simplify the code reading, since both must call efi_unload_image(). Calling efi_initrd_register() without a BootCurrent is guaranteed to fail as well. > > > + /* try to register load file2 for initrd's */ > > +
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On 11.03.21 12:44, Heinrich Schuchardt wrote:
> Am 11. März 2021 12:36:04 MEZ schrieb Ilias Apalodimas
> :
>> Hi Heinrich
>>
>> [...]
>>
>> + * @load_option: device paths to search
>> + * @size:size of the discovered device path
>> + * @guid:guid to search for
>> + *
>> + * Return: device path or NULL. Caller must free the returned
>> value
>
> Please, keep the text aligned.
>
> Do we need a copy? Isn't a pointer good enough?
A pointer to what?
I think it's better to a copy here. This is a device path that
>> might be used
out of a stack context were the load option might disappear.
Look at how the function is used in efi_get_dp_from_boot().
>>>
>>> You are duplicating in get_initrd_fp(). Why should we duplicate
>> twice?
>>>
>>
>> That's irrelevant though isn't it?
>> I did that in the efi initrd implementation. If someone else does the
>> DTB in
>> the future and device to use efi_dp_from_lo return directly?
>> I'd much prefer an API (since that function goes into an API-related
>> file for
>> device paths), that's safe and requires the user to free the memory,
>> rather
>> than allowing him to accidentally shoot himself in the foot, keeping in
>> mind
>> it's a single copy on a device path, which roughly adds anything on our
>> boot
>> time.
>>
>
>> + */
>> +struct
>> +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
>> +efi_uintn_t *size, efi_guid_t guid)
>> +{
>> +struct efi_device_path *fp = lo->file_path;
>> +struct efi_device_path_vendor *vendor;
>> +int lo_len = lo->file_path_length;
>> +
>> +while (lo_len) {
>
> lo_len must be at least sizeof(struct efi_device_path).
>
>> +if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
>> +fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH) {
>> +lo_len -= fp->length;
>
> Could the last device path in the array be followed by zero bytes
>> for
> padding?
How? Device paths are packed aren't they ?
> Should we check that fp->length >= sizeof(struct efi_device_path)?
Yea probably a good idea
>>>
>>> The content of the boot option comes from the user. Just assume that
>> it
>>> can contain malicious content.
>>>
>>
>> Yea the user doesn't add the device path directly though. The user adds
>> directories and a file, so the normalization is part of this function,
>> applied randomly and locally on a single input? or the device path
>> creation
>> functions which this code uses? Since we use the pattern in a bunch of
>> places
>> I assumed we did take care of that during the functions that create the
>> device
>> paths. I haven't checked though ...
>
> I am not referring to efidebug.
>
> The user can update EFI variables with any binary content using an EFI binary
> or OS functions.
>
> E.g. copy a binary file to the efi variables file system in Linux.
>
>>
>>> We should also check that the identified device-path starting at
>>> VenMedia() ends within fp->length using efi_dp_check_length().
>>
>> ok
>>
>>>
>
>> +fp = (void *)fp + fp->length;
>
> Please, avoid code duplication.
>
> E.g.
>
> for (; lo_len >= sizeof(struct efi_device_path);
> lo_len -= fp->length, fp = (void *)fp + fp->length) {
I can an switch to that, but I really never liked this format.
It always seemed way less readable to me for some reason. Maybe
>> because I
never got used to it ...
>>>
>>> Using "for" is only one option. You could use "goto next;" instead.
>>>
>>
>> I really don't mind, I can just use what you propose.
>
> As long as you avoid code duplication I am fine.
>
> Best regards
>
> Heinrich
>
>>
>
>> +continue;
>> +}
>> +
>> +vendor = (struct efi_device_path_vendor *)fp;
>> +if (!guidcmp(&vendor->guid, &guid))
>> +return efi_dp_dup(fp);
>
> Should we strip of the VenMedia() node here?
Why? This is not supposed to get the file path. The function says
>> "get device
path from load option" and that device includes the VenMedia node.
It would make more sense for me to strip in efi_get_dp_from_boot()
>> for
example, if you want a helper function to get the initrd path
>> *only*.
>>>
>>> The VenMedia() node is not needed anymore once you have found the
>> entry.
>>>
>>
>> Yea it's not but as I said the name of the function says "get the
>> *stored*
>>from a boot option. Not get the one that suits us.
>> There's another reason for that btw, the initrd related functions use
>> that
>> (specifically get_initrd_fp()), to figure out if the Boot variable
>> contains an initrd path or not.
>> If the VenMedia is not presen
Re: [PATCH 5/6] efidebug: add multiple device path instances on Boot####
On 05.03.21 23:23, Ilias Apalodimas wrote:
> The UEFI spec allow a packed array of UEFI device paths in the
> FilePathList[] of an EFI_LOAD_OPTION. The first file path must
> describe the loaded image but the rest are OS specific.
>
> Previous patches parse the device path and try to use the second
> member of the array as an initrd. So let's modify efidebug slightly
> and install the second file described in the command line as the
> initrd device path.
>
> Signed-off-by: Ilias Apalodimas
> ---
> cmd/efidebug.c| 193 ++
> doc/board/emulation/qemu_capsule_update.rst | 4 +-
> doc/uefi/uefi.rst | 2 +-
> .../test_efi_capsule/test_capsule_firmware.py | 6 +-
> test/py/tests/test_efi_secboot/test_signed.py | 16 +-
> .../test_efi_secboot/test_signed_intca.py | 8 +-
> .../tests/test_efi_secboot/test_unsigned.py | 8 +-
> 7 files changed, 179 insertions(+), 58 deletions(-)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index bbbcb0a54643..9a1c471a3eaa 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -9,6 +9,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> @@ -19,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> #define BS systab.boottime
> #define RT systab.runtime
> @@ -794,6 +797,65 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int
> flag,
> return CMD_RET_SUCCESS;
> }
>
> +/**
> + * add_initrd_instance() - Append a device path to load_options pointing to
> an
> + * inirtd
> + *
> + * @argc:Number of arguments
> + * @argv:Argument array
> + * @file_pathExisting device path, the new instance will be appended
> + * Return: Pointer to the device path or ERR_PTR
> + *
> + */
> +static
> +struct efi_device_path *add_initrd_instance(const char *dev, const char
> *part,
> + const char *file,
> + const struct efi_device_path *fp,
> + efi_uintn_t *fp_size)
> +{
> + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL;
> + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL;
> + efi_status_t ret;
> + const struct efi_initrd_dp id_dp = {
> + .vendor = {
> + {
> + DEVICE_PATH_TYPE_MEDIA_DEVICE,
> + DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> + sizeof(id_dp.vendor),
> + },
> + EFI_INITRD_MEDIA_GUID,
> + },
> + .end = {
> + DEVICE_PATH_TYPE_END,
> + DEVICE_PATH_SUB_TYPE_END,
> + sizeof(id_dp.end),
> + }
> + };
> +
> + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp);
> + if (ret != EFI_SUCCESS) {
> + printf("Cannot create device path for \"%s %s\"\n", part, file);
> + goto out;
> + }
> +
> + initrd_dp =
> + efi_dp_append_instance((const struct efi_device_path *)&id_dp,
Please, pass &id_dp.end here to avoid a superfluous end node.
Best regards
Heinrich
> +tmp_fp);
> + if (!initrd_dp) {
> + printf("Cannot append media vendor device path path\n");
> + goto out;
> + }
> + final_fp = efi_dp_concat(fp, initrd_dp);
> + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
> + (2 * sizeof(struct efi_device_path));
> +
> +out:
> + efi_free_pool(initrd_dp);
> + efi_free_pool(tmp_dp);
> + efi_free_pool(tmp_fp);
> + return final_fp ? final_fp : ERR_PTR(-EINVAL);
> +}
> +
> /**
> * do_efi_boot_add() - set UEFI load option
> *
> @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int
> flag,
> *
> * Implement efidebug "boot add" sub-command. Create or change UEFI load
> option.
> *
> - * efidebug boot add[:]
>
> + * efidebug boot add -b[:]
> + * -i [:]
> + * -s ''
> */
> static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int
> flag,
> size_t label_len, label_len16;
> u16 *label;
> struct efi_device_path *device_path = NULL, *file_path = NULL;
> + struct efi_device_path *final_fp = NULL;
> struct efi_load_option lo;
> void *data = NULL;
> efi_uintn_t size;
> + efi_uintn_t fp_size;
> efi_status_t ret;
> int r = CMD_RET_SUCCESS;
> -
> - if (argc < 6 || argc > 7)
> - return CMD_RET_USAGE;
> -
> - id = (int)simple_strtoul(argv[1], &endp, 16);
> - if (*endp != '\0' || id > 0x)
> - return CMD_RET_USAGE;
> -
> - sprintf(var_name, "Boot%04X", id)
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
Hi Heinrich, [...] > >> No, the structure is added in cmd/efidebug.c code. > >> It's created with efi_dp_append_instance() on > >> - const struct efi_initrd_dp id_dp > >> - file path of initrd > >> > >> which will create: > >> kernel path,end(0xff), > >> VenMedia(), end(0x01), > > This end node is superfluous. Why?i It's not defined in any standard (in fact these patches will be used for the USWG proposal), it's making our lives a lot easier during parsing, since you only need to get the next instance for the *entire* device path. On the other hand if you mix and match them, you'll have to skip the first entry with dp->len and then start using code to get the next instance. Similar logic applies to appending the instance obviously. Since you now how have to account for your 'special' first node, which isn't that special to begin with. So why exactly is it superflous? Because you can get the next initrd without having to parse an end of instace? Well that's the case for the initrd's as well then On the contrary I think it makes the entire device path look more generic, since each node is separated by an end of instance, no matter what the contents are. Thanks /Ilias > > Best regards > > Heinrich > > >> initrd1, end(0x01), > >> initrd2, end(0xff) > >> > >> I know I originally proposed the one you have, but it seemed cleaner > >> adding > >> an extra instance between VenMedia and the first initrd. > >> > >>> > >>> Please, document the structure. > >>> > >> > >> Sure > >> > >>> Best regards > >>> > >>> Heinrich > >> > >> Thanks > >> /Ilias > > >
Re: [PATCH 6/6] doc: Update uefi documentation for initrd loading options
On 05.03.21 23:23, Ilias Apalodimas wrote: > Document the command line options for efidebug and initrd loading > > Signed-off-by: Ilias Apalodimas > --- > doc/uefi/uefi.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst > index b3494c22e073..b3330f8d7273 100644 > --- a/doc/uefi/uefi.rst > +++ b/doc/uefi/uefi.rst > @@ -180,6 +180,12 @@ Set up boot parameters on your board:: > > efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > +Since kernel 5.7 there's an alternative way of loading an initrd using > +LoadFile2 protocol if CONFIG_EFI_LOAD_FILE2_INITRD is enabled. > +The initrd path can be specified with:: > + > +efidebug boot add -b ABE0 'kernel' mmc 0:1 Image -i mmc 0:1 initrd > + > Now your board can run the signed image via the boot manager (see below). > You can also try this sequence by running Pytest, test_efi_secboot, > on the sandbox > Please, add text to doc/uefi/uefi.rst describing how we use boot options for initrds. Best regards Heinrich
Re: [PATCH 5/6] efidebug: add multiple device path instances on Boot####
On Thu, Mar 11, 2021 at 01:38:33PM +0100, Heinrich Schuchardt wrote:
> > + }
> > +
> > + initrd_dp =
> > + efi_dp_append_instance((const struct efi_device_path *)&id_dp,
>
> Please, pass &id_dp.end here to avoid a superfluous end node.
Wont this make efi_dp_size() loop endlessly?
Regards
/Ilias
>
> Best regards
>
> Heinrich
>
> > + tmp_fp);
> > + if (!initrd_dp) {
> > + printf("Cannot append media vendor device path path\n");
> > + goto out;
> > + }
> > + final_fp = efi_dp_concat(fp, initrd_dp);
> > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
> > + (2 * sizeof(struct efi_device_path));
> > +
> > +out:
> > + efi_free_pool(initrd_dp);
> > + efi_free_pool(tmp_dp);
> > + efi_free_pool(tmp_fp);
> > + return final_fp ? final_fp : ERR_PTR(-EINVAL);
> > +}
> > +
> > /**
> > * do_efi_boot_add() - set UEFI load option
> > *
> > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp,
> > int flag,
> > *
> > * Implement efidebug "boot add" sub-command. Create or change UEFI load
> > option.
> > *
> > - * efidebug boot add[:]
> >
> > + * efidebug boot add -b[:]
> > + * -i [:]
> > + * -s ''
> > */
> > static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> >int argc, char *const argv[])
> > @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int
> > flag,
> > size_t label_len, label_len16;
> > u16 *label;
> > struct efi_device_path *device_path = NULL, *file_path = NULL;
> > + struct efi_device_path *final_fp = NULL;
> > struct efi_load_option lo;
> > void *data = NULL;
> > efi_uintn_t size;
> > + efi_uintn_t fp_size;
> > efi_status_t ret;
> > int r = CMD_RET_SUCCESS;
> > -
> > - if (argc < 6 || argc > 7)
> > - return CMD_RET_USAGE;
> > -
> > - id = (int)simple_strtoul(argv[1], &endp, 16);
> > - if (*endp != '\0' || id > 0x)
> > - return CMD_RET_USAGE;
> > -
> > - sprintf(var_name, "Boot%04X", id);
> > - p = var_name16;
> > - utf8_utf16_strncpy(&p, var_name, 9);
> > + int i;
> >
> > guid = efi_global_variable_guid;
> >
> > /* attributes */
> > lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */
> > + lo.optional_data = NULL;
> > + lo.label = NULL;
> >
> > - /* label */
> > - label_len = strlen(argv[2]);
> > - label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> > - label = malloc((label_len16 + 1) * sizeof(u16));
> > - if (!label)
> > - return CMD_RET_FAILURE;
> > - lo.label = label; /* label will be changed below */
> > - utf8_utf16_strncpy(&label, argv[2], label_len);
> > + /* search for -b first since the rest of the arguments depends on that
> > */
> > + for (i = 0; i < argc; i++) {
> > + if (!strcmp(argv[i], "-b")) {
> > + if (argc < i + 5 || lo.label) {
> > + r = CMD_RET_USAGE;
> > + goto out;
> > + }
> > + id = (int)simple_strtoul(argv[i + 1], &endp, 16);
> > + if (*endp != '\0' || id > 0x)
> > + return CMD_RET_USAGE;
> > +
> > + sprintf(var_name, "Boot%04X", id);
> > + p = var_name16;
> > + utf8_utf16_strncpy(&p, var_name, 9);
> > +
> > + /* label */
> > + label_len = strlen(argv[i + 2]);
> > + label_len16 = utf8_utf16_strnlen(argv[i + 2],
> > label_len);
> > + label = malloc((label_len16 + 1) * sizeof(u16));
> > + if (!label)
> > + return CMD_RET_FAILURE;
> > + lo.label = label; /* label will be changed below */
> > + utf8_utf16_strncpy(&label, argv[i + 2], label_len);
> > +
> > + /* file path */
> > + ret = efi_dp_from_name(argv[i + 3], argv[i + 4],
> > + argv[i + 5], &device_path,
> > + &file_path);
> > + if (ret != EFI_SUCCESS) {
> > + printf("Cannot create device path for \"%s
> > %s\"\n",
> > + argv[3], argv[4]);
> > + r = CMD_RET_FAILURE;
> > + goto out;
> > + break;
> > + }
> > + fp_size = efi_dp_size(file_path) +
> > + sizeof(struct efi_device_path);
> > + }
> > + }
> >
> > - /* file path */
> > - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> > - &file_path);
> > - if (ret != EFI_SUCCESS) {
> > - printf("Cannot create device path for \"%s %s\"\n",
> > - argv[3
Re: [PATCH u-boot 16/39] build: use thin archives instead of incremental linking
On Tue, 9 Mar 2021 21:00:00 +0800 Bin Meng wrote: > > --start-group is useless now. > > > $(u-boot-main) > > \ > > - --end-group > > \ > > + --no-whole-archive --end-group > > \ > > and --end-group > I will test this > > - rm -f $@; $(AR) rcs$(KBUILD_ARFLAGS) $@) > > + rm -f $@; $(AR) cDPrsT $@ $(filter $(obj-y), $^), \ > > + rm -f $@; $(AR) cPrsT$(KBUILD_ARFLAGS) $@) > > nits: should we use D for the empty one for consistency? OK > > > > $(builtin-target): $(obj-y) FORCE > > $(call if_changed,link_o_target) > > @@ -362,7 +361,7 @@ $(modorder-target): $(subdir-ym) FORCE > > # > > ifdef lib-target > > quiet_cmd_link_l_target = AR $@ > > -cmd_link_l_target = rm -f $@; $(AR) rcs$(KBUILD_ARFLAGS) $@ $(lib-y) > > +cmd_link_l_target = rm -f $@; $(AR) cPrs$(KBUILD_ARFLAGS) $@ $(lib-y) > > It looks this line change is not needed Hmm. I will look into this, maybe I added it just for consistency. > Otherwise LGTM: > Reviewed-by: Bin Meng THX
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On 11.03.21 13:39, Ilias Apalodimas wrote: > Hi Heinrich, > > [...] No, the structure is added in cmd/efidebug.c code. It's created with efi_dp_append_instance() on - const struct efi_initrd_dp id_dp - file path of initrd which will create: kernel path,end(0xff), VenMedia(), end(0x01), >> >> This end node is superfluous. > > Why?i It's not defined in any standard (in fact these patches will be used for > the USWG proposal), it's making our lives a lot easier during parsing, since > you only need to get the next instance for the *entire* device path. > On the other hand if you mix and match them, you'll have to skip the first > entry with dp->len and then start using code to get the next instance. Similar > logic applies to appending the instance obviously. Since you now how have to > account for your 'special' first node, which isn't that special to begin with. > > So why exactly is it superflous? Because you can get the next initrd without > having to parse an end of instace? Well that's the case for the initrd's as > well then > On the contrary I think it makes the entire device path look more generic, > since each node is separated by an end of instance, no matter what the > contents are. We use the VenMedia() node to identify a multi-part device tree containing one or multiple initrd. An end node does not convey any additional information. The first initrd device path can start directly behind the VenMedia() node. This saves space for non-volatile variables which is limited. Best regards Heinrich > > Thanks > /Ilias > >> >> Best regards >> >> Heinrich >> initrd1, end(0x01), initrd2, end(0xff) I know I originally proposed the one you have, but it seemed cleaner adding an extra instance between VenMedia and the first initrd. > > Please, document the structure. > Sure > Best regards > > Heinrich Thanks /Ilias >>> >>
Re: [PATCH u-boot 17/39] build: support building with Link Time Optimizations
On Tue, 9 Mar 2021 21:30:26 +0800 Bin Meng wrote: > > +config LTO > > + bool "Enable Link Time Optimizations" > > + depends on ARCH_SUPPORTS_LTO > > + default n > > nits: this line is not needed as the default value is n if omitted This is for consistency. The other options in this file also use "default n" > > +ifdef CONFIG_LTO > > + LTO_CFLAGS := -flto > > + LTO_FINAL_LDFLAGS := -fwhole-program > > This one should not be necessary per my read of the GCC doc. Also in > your patch, it is only used in the SPL build. I shall do some tests. > > +ifdef CONFIG_LTO > > +quiet_cmd_u-boot__ ?= LTO $@ > > + cmd_u-boot__ ?= > > \ > > + $(CC) -nostdlib -nostartfiles > > \ > > + $(LTO_FINAL_CFLAGS) $(c_flags) > > \ > > LTO_FINAL_CFLAGS is not defined anywhere. I guess you wanted to use > LTO_FINAL_LDFLAGS? THX, I forgot to change it here. We will see if this variable is needed (since it only contains -fwhole-program, which may be dropped) > > > + $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@ > > \ > > + -T u-boot.lds $(u-boot-init) > > \ > > + -Wl,--start-group -Wl,--whole-archive > > \ > > --start-group should be dropped Will test
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On Thu, Mar 11, 2021 at 01:44:05PM +0100, Heinrich Schuchardt wrote: > On 11.03.21 13:39, Ilias Apalodimas wrote: > > Hi Heinrich, > > > > [...] > No, the structure is added in cmd/efidebug.c code. > It's created with efi_dp_append_instance() on > - const struct efi_initrd_dp id_dp > - file path of initrd > > which will create: > kernel path,end(0xff), > VenMedia(), end(0x01), > >> > >> This end node is superfluous. > > > > Why?i It's not defined in any standard (in fact these patches will be used > > for > > the USWG proposal), it's making our lives a lot easier during parsing, since > > you only need to get the next instance for the *entire* device path. > > On the other hand if you mix and match them, you'll have to skip the first > > entry with dp->len and then start using code to get the next instance. > > Similar > > logic applies to appending the instance obviously. Since you now how have to > > account for your 'special' first node, which isn't that special to begin > > with. > > > > So why exactly is it superflous? Because you can get the next initrd without > > having to parse an end of instace? Well that's the case for the initrd's as > > well then > > On the contrary I think it makes the entire device path look more generic, > > since each node is separated by an end of instance, no matter what the > > contents are. > > We use the VenMedia() node to identify a multi-part device tree > containing one or multiple initrd. > > An end node does not convey any additional information. > > The first initrd device path can start directly behind the VenMedia() > node. This saves space for non-volatile variables which is limited. > Again, it can. But we can also remove the end node after each initrd no? Then we'd save even more space... My point is, I can't understand why we should complicate the adding code/parsing code, by adding a special use case. I don't think saving 4bytes is really worth the complexity. Thanks /Ilias > Best regards > > Heinrich > > > > > Thanks > > /Ilias > > > >> > >> Best regards > >> > >> Heinrich > >> > initrd1, end(0x01), > initrd2, end(0xff) > > I know I originally proposed the one you have, but it seemed cleaner > adding > an extra instance between VenMedia and the first initrd. > > > > > Please, document the structure. > > > > Sure > > > Best regards > > > > Heinrich > > Thanks > /Ilias > >>> > >> >
Re: [PATCH 4/6] efi_loader: Replace config option for initrd loading
On 11.03.21 13:30, Ilias Apalodimas wrote: > On Thu, Mar 11, 2021 at 01:23:05PM +0100, Heinrich Schuchardt wrote: >> On 05.03.21 23:23, Ilias Apalodimas wrote: >>> Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd >>> unconditionally. Although we correctly return various EFI exit codes >>> depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the >>> kernel loader, only falls back to the cmdline interpreted initrd if the >>> protocol is not installed. >>> >>> This creates a problem for EFI installers, since they won't be able to >>> load their own initrd and continue the installation. It also makes the >>> feature hard to use, since we can either have a single initrd or we have >>> to recompile u-boot if the filename changes. >>> >>> So let's introduce a different logic that will decouple the initrd >>> path from the config option we currently have. >>> When defining a UEFI Boot we can use the filepathlist and store >>> a file path pointing to our initrd. Specifically the EFI spec describes: >>> >>> "The first element of the array is a device path that describes the device >>> and location of the Image for this load option. Other device paths may >>> optionally exist in the FilePathList, but their usage is OSV specific" >>> >>> When the EFI application is launched through the bootmgr, we'll try to >>> interpret the extra device path. If that points to a file that exists on >>> our disk, we'll now install the load_file2 and the efi-stub will be able >>> to use it. >>> >>> This opens up another path using U-Boot and defines a new boot flow. >>> A user will be able to control the kernel/initrd pairs without explicit >>> cmdline args or GRUB. >>> >>> Signed-off-by: Ilias Apalodimas >>> --- >>> cmd/bootefi.c| 3 + >>> include/efi_loader.h | 1 + >>> lib/efi_loader/Kconfig | 15 +-- >>> lib/efi_loader/efi_bootmgr.c | 3 + >>> lib/efi_loader/efi_load_initrd.c | 209 +-- >>> 5 files changed, 152 insertions(+), 79 deletions(-) >>> >>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >>> index 271b385edea6..cba81ffe75e4 100644 >>> --- a/cmd/bootefi.c >>> +++ b/cmd/bootefi.c >>> @@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t >>> handle, void *load_options) >>> >>> free(load_options); >>> >>> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) >>> + efi_initrd_deregister(); >>> + >>> return ret; >>> } >>> >>> diff --git a/include/efi_loader.h b/include/efi_loader.h >>> index eb11a8c7d4b1..0d4f5d9acc9f 100644 >>> --- a/include/efi_loader.h >>> +++ b/include/efi_loader.h >>> @@ -431,6 +431,7 @@ efi_status_t efi_net_register(void); >>> /* Called by bootefi to make the watchdog available */ >>> efi_status_t efi_watchdog_register(void); >>> efi_status_t efi_initrd_register(void); >>> +void efi_initrd_deregister(void); >>> /* Called by bootefi to make SMBIOS tables available */ >>> /** >>> * efi_acpi_register() - write out ACPI tables >>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >>> index e729f727df11..029f0e515f57 100644 >>> --- a/lib/efi_loader/Kconfig >>> +++ b/lib/efi_loader/Kconfig >>> @@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD >>> bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk" >>> default n >>> help >>> - Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can >>> - use to load the initial ramdisk. Once this is enabled using >>> - initrd= will stop working. >>> - >>> -config EFI_INITRD_FILESPEC >>> - string "initramfs path" >>> - default "host 0:1 initrd" >>> - depends on EFI_LOAD_FILE2_INITRD >>> - help >>> - Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz. >>> + Linux v5.7 and later can make use of this option. If the boot >>> option >>> + selected by the UEFI boot manager specifies an existing file to >>> be used >>> + as initial RAM disk, a Linux specific Load File2 protocol will >>> be >>> + installed and Linux 5.7+ will ignore any initrd= >>> command line >>> + argument. >>> >>> config EFI_SECURE_BOOT >>> bool "Enable EFI secure boot support" >>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c >>> index 25f5cebfdb67..d1baa8c71a4d 100644 >>> --- a/lib/efi_loader/efi_bootmgr.c >>> +++ b/lib/efi_loader/efi_bootmgr.c >>> @@ -118,6 +118,9 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t >>> *handle, >>> ret = efi_set_variable_int(L"BootCurrent", >>>&efi_global_variable_guid, >>>attributes, sizeof(n), &n, false); >> >> Why would you continue if BootCurrent is not set? > > Because the bitops below is just trying to simplify the code > reading, since both must call efi_unload_image(). Calling > efi_initrd_register() without a BootCurrent is guaranteed to fail as well. > >> >>> + /*
[PATCH 0/4 v7] Add ESRT and test ESRT creation
The following 4 commits add the ESRT and provide tests for the functionality. The first commit adds the ESRT as defined in the UEFI 2.8 specification. An empty ESRT is created during the execution of the efi_init_obj_list(). The ESRT is updated when: 1) a FMP protocol is installed in the system: this will add the corresponding entries to the ESRT. 2) a capsule is installed via UpdateCapsule: this should update entries already present in the ESRT. This implementation of the ESRT creation takes input from FMP only. It is assumed that the FMP will maintain the following values across reboot: - LastAttemptVersion. - LastAttemptStatus. The second commit introduces a helper cmd function to print the ESRT in the u-boot shell. The third and forth commits contain two test for the ESRT creation in the sandbox platform. - Test 1 executes from the u-boot shell with "bootefi selftest". - Test 2 executes in the pytest environment. Patch v7: - Address v6 comments. Patch v6: - split the v5 3/3 commit into v6 3/4 (efi selftest) and v6 4/4 (addition to test_efi_capsule_fw3). - Address v5 comments. Patch v5: - Address v4 comments. - split the v4 2/2 commit into the v5 2/3 (debug print ESRT) and 3/3 (ESRT tests). Patch v4: - update stale [Patch 1/2 v3] commit message. Patch v3: - Address v2 comments. Patch v2: - The ESRT is now regenerated from scratch at every FMP EVT_NOTIFY_SIGNAL and whenever a capsule is updated. - Extended TestEfiCapsuleFirmwareFit::test_efi_capsule_fw3 to verify that the ESRT is correctly populated after an UpdateCapsule. - Addressed v1 comments. Patch v1: - reworked the ESRT creation code, allowing table to resize as FMPs are installed. - registered a callback for the FMP protocol install. - Created a unit test running on the sandbox platform. rfc: initial patch submission CC: Heinrich Schuchardt CC: Sughosh Ganu CC: AKASHI Takahiro CC: Ilias Apalodimas CC: Andre Przywara CC: Alexander Graf CC: n...@arm.com Jose Marinho (4): efi: Add ESRT to the EFI system table cmd: efi: ESRT table debug print efi: ESRT creation tests Test the ESRT creation within an existing pytest. cmd/efidebug.c| 88 +++ include/efi_api.h | 21 + include/efi_loader.h | 24 + lib/efi_loader/Kconfig| 7 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_boottime.c | 7 +- lib/efi_loader/efi_capsule.c | 8 + lib/efi_loader/efi_esrt.c | 510 ++ lib/efi_loader/efi_setup.c| 6 + lib/efi_selftest/Makefile | 2 + lib/efi_selftest/efi_selftest_esrt.c | 291 ++ .../test_efi_capsule/test_capsule_firmware.py | 8 + 12 files changed, 969 insertions(+), 4 deletions(-) create mode 100644 lib/efi_loader/efi_esrt.c create mode 100644 lib/efi_selftest/efi_selftest_esrt.c -- 2.17.1
[PATCH 1/4 v7] efi: Add ESRT to the EFI system table
The ESRT is initialised during efi_init_objlist after
efi_initialize_system_table().
The ESRT is recreated from scratch at the following events:
- successful UpdateCapsule;
- FMP instance install.
The code ensures that every ESRT entry has a unique fw_class value.
Limitations:
- The ESRT is not updated if an FMP instance is uninstalled;
- the fields image_type and flags are in the current implementation left
undefined. Setting these values will require a per-platform function
that returns the image_type/flags as a function of the image fw_class.
CC: Heinrich Schuchardt
CC: Sughosh Ganu
CC: AKASHI Takahiro
CC: Ilias Apalodimas
CC: Andre Przywara
CC: Alexander Graf
CC: n...@arm.com
Signed-off-by: Jose Marinho
Remove two EFI_CALL() indirections.
Move ESRT GUID in efidebug's list of GUIDs.
Reviewed-by: Heinrich Schuchardt
---
cmd/efidebug.c| 4 +
include/efi_api.h | 21 ++
include/efi_loader.h | 24 ++
lib/efi_loader/Kconfig| 7 +
lib/efi_loader/Makefile | 1 +
lib/efi_loader/efi_boottime.c | 7 +-
lib/efi_loader/efi_capsule.c | 8 +
lib/efi_loader/efi_esrt.c | 510 ++
lib/efi_loader/efi_setup.c| 6 +
9 files changed, 584 insertions(+), 4 deletions(-)
create mode 100644 lib/efi_loader/efi_esrt.c
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index e4030f514a..b3d7bd8897 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -516,6 +516,10 @@ static const struct {
"ACPI table",
EFI_ACPI_TABLE_GUID,
},
+ {
+ "EFI System Resource Table",
+ EFI_SYSTEM_RESOURCE_TABLE_GUID,
+ },
{
"device tree",
EFI_FDT_GUID,
diff --git a/include/efi_api.h b/include/efi_api.h
index 48e48a6263..fb53637419 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -1722,6 +1722,23 @@ struct efi_load_file_protocol {
void *buffer);
};
+struct efi_system_resource_entry {
+ efi_guid_t fw_class;
+ u32 fw_type;
+ u32 fw_version;
+ u32 lowest_supported_fw_version;
+ u32 capsule_flags;
+ u32 last_attempt_version;
+ u32 last_attempt_status;
+} __packed;
+
+struct efi_system_resource_table {
+ u32 fw_resource_count;
+ u32 fw_resource_count_max;
+ u64 fw_resource_version;
+ struct efi_system_resource_entry entries[];
+} __packed;
+
/* Boot manager load options */
#define LOAD_OPTION_ACTIVE 0x0001
#define LOAD_OPTION_FORCE_RECONNECT0x0002
@@ -1740,6 +1757,10 @@ struct efi_load_file_protocol {
#define ESRT_FW_TYPE_DEVICEFIRMWARE0x0002
#define ESRT_FW_TYPE_UEFIDRIVER0x0003
+#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
+ EFI_GUID(0xb122a263, 0x3661, 0x4f68,\
+ 0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80)
+
/* Last Attempt Status Values */
#define LAST_ATTEMPT_STATUS_SUCCESS0x
#define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL 0x0001
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 68daa1a4a9..719aa90490 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -214,6 +214,8 @@ extern const efi_guid_t efi_guid_rng_protocol;
extern const efi_guid_t efi_guid_capsule_report;
/* GUID of firmware management protocol */
extern const efi_guid_t efi_guid_firmware_management_protocol;
+/* GUID for the ESRT */
+extern const efi_guid_t efi_esrt_guid;
extern unsigned int __efi_runtime_start, __efi_runtime_stop;
extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
@@ -558,6 +560,10 @@ struct efi_simple_file_system_protocol
*efi_simple_file_system(
/* open file from device-path: */
struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);
+/* Registers a callback function for a notification event. */
+efi_status_t EFIAPI efi_register_protocol_notify(const efi_guid_t *protocol,
+struct efi_event *event,
+void **registration);
/**
* efi_size_in_pages() - convert size in bytes to size in pages
*
@@ -890,4 +896,22 @@ static inline efi_status_t efi_launch_capsules(void)
#endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
+/**
+ * Install the ESRT system table.
+ *
+ * @return status code
+ */
+efi_status_t efi_esrt_register(void);
+
+/**
+ * efi_esrt_populate() - Populates the ESRT entries from the FMP instances
+ * present in the system.
+ * If an ESRT already exists, the old ESRT is replaced in the system table.
+ * The memory of the old ESRT is deallocated.
+ *
+ * Return:
+ * - EFI_SUCCESS if the ESRT is correctly created
+ * - error code otherwise.
+ */
+efi_status_t efi_esrt_populate(void);
#endif /* _EFI_LOADER_H */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e729f727df..a96014ce18 100644
--- a/lib/efi_loader/Kconfig
+++ b/l
[PATCH 3/4 v7] efi: ESRT creation tests
This commmit exercises the ESRT creation in a EFI selftest.
A fake FMP, with TEST_ESRT_NUM_ENTRIES FW images, is installed in the
system leading to the corresponding ESRT entries being populated.
The ESRT entries are checked against the datastructure used to
initialize the FMP.
Invocation from the sandbox platform:
add to sandbox_defconfig:
+CONFIG_CMD_BOOTEFI_SELFTEST=y
make sandbox_capsule_defconfig all
./u-boot -d arch/sandbox/dts/test.dtb
bootefi selftest
CC: Heinrich Schuchardt
CC: Sughosh Ganu
CC: AKASHI Takahiro
CC: Ilias Apalodimas
CC: Andre Przywara
CC: Alexander Graf
CC: n...@arm.com
Signed-off-by: Jose Marinho
---
lib/efi_selftest/Makefile| 2 +
lib/efi_selftest/efi_selftest_esrt.c | 291 +++
2 files changed, 293 insertions(+)
create mode 100644 lib/efi_selftest/efi_selftest_esrt.c
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index b02fd56e0a..58b27d6da3 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -72,6 +72,8 @@ ifeq ($(CONFIG_BLK)$(CONFIG_DOS_PARTITION),yy)
obj-y += efi_selftest_block_device.o
endif
+obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
+
targets += \
efi_miniapp_file_image_exception.h \
efi_miniapp_file_image_exit.h \
diff --git a/lib/efi_selftest/efi_selftest_esrt.c
b/lib/efi_selftest/efi_selftest_esrt.c
new file mode 100644
index 00..99251f22a5
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_esrt.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test ESRT tables support
+ *
+ * Copyright (C) 2021 Arm Ltd.
+ */
+#include
+#include
+#include
+
+// This value must not exceed 255.
+// An FMP cannot contain more than 255 FW images.
+#define TEST_ESRT_NUM_ENTRIES 255
+
+static
+struct efi_firmware_image_descriptor static_img_info[TEST_ESRT_NUM_ENTRIES];
+
+static const struct efi_system_table *local_systable;
+
+static efi_handle_t fmp_handle;
+
+static const efi_guid_t efi_fmp_guid =
+ EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
+
+static void efi_test_esrt_init_info(void)
+{
+ for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++) {
+ static_img_info[idx].image_index = idx;
+
+ // Note: the 16 byte value present in
+ // static_img_info[idx].image_type_id is not strictly a GUID.
+ // The value is used for the sake of code testing.
+ static_img_info[idx].image_type_id.b[0] = idx;
+
+ static_img_info[idx].image_id = 0;
+ static_img_info[idx].image_id_name = NULL;
+ static_img_info[idx].version = 0;
+ static_img_info[idx].version_name = NULL;
+ static_img_info[idx].size = 0;
+ static_img_info[idx].lowest_supported_image_version = 1;
+ static_img_info[idx].last_attempt_version = 2;
+ static_img_info[idx].last_attempt_status = 3;
+ static_img_info[idx].hardware_instance = 1;
+ }
+}
+
+static efi_status_t
+EFIAPI efi_test_fmp_get_image_info(struct efi_firmware_management_protocol
*this,
+ efi_uintn_t *image_info_size,
+ struct efi_firmware_image_descriptor
*image_info,
+ u32 *descriptor_version,
+ u8 *descriptor_count,
+ efi_uintn_t *descriptor_size,
+ u32 *package_version,
+ u16 **package_version_name)
+{
+ efi_status_t ret = EFI_SUCCESS;
+
+ if (!image_info_size)
+ return EFI_INVALID_PARAMETER;
+
+ if (descriptor_version)
+ *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
+ if (descriptor_count)
+ *descriptor_count = TEST_ESRT_NUM_ENTRIES;
+ if (descriptor_size)
+ *descriptor_size = sizeof(*image_info);
+ if (package_version)
+ *package_version = 0x;
+ if (package_version_name)
+ *package_version_name = NULL;
+
+ if (*image_info_size < sizeof(*image_info)) {
+ *image_info_size = *descriptor_size * *descriptor_count;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
+ image_info[idx] = static_img_info[idx];
+
+ return ret;
+}
+
+static struct efi_firmware_management_protocol efi_test_fmp = {
+ .get_image_info = efi_test_fmp_get_image_info,
+ .get_image = NULL,
+ .set_image = NULL,
+ .check_image = NULL,
+ .get_package_info = NULL,
+ .set_package_info = NULL,
+};
+
+static void *lib_test_get_esrt(void)
+{
+ for (int idx = 0; idx < local_systable->nr_tables; idx++)
+ if (!guidcmp(&efi_esrt_guid, &local_systable->tables[idx].guid))
+ return local_systable->tables[idx].table;
[PATCH 2/4 v7] cmd: efi: ESRT table debug print
This commit enables the ESRT printing from the u-boot shell by invoking:
- efidebug capsule esrt
CC: Heinrich Schuchardt
CC: Sughosh Ganu
CC: AKASHI Takahiro
CC: Ilias Apalodimas
CC: Andre Przywara
CC: Alexander Graf
CC: n...@arm.com
Signed-off-by: Jose Marinho
---
cmd/efidebug.c | 84 ++
1 file changed, 84 insertions(+)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index b3d7bd8897..fffb347550 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -129,6 +129,82 @@ static int do_efi_capsule_show(struct cmd_tbl *cmdtp, int
flag,
return CMD_RET_SUCCESS;
}
+#ifdef CONFIG_EFI_ESRT
+
+#define EFI_ESRT_FW_TYPE_NUM 4
+char *efi_fw_type_str[EFI_ESRT_FW_TYPE_NUM] = {"unknown", "system FW", "device
FW",
+"UEFI driver"};
+
+#define EFI_ESRT_UPDATE_STATUS_NUM 9
+char *efi_update_status_str[EFI_ESRT_UPDATE_STATUS_NUM] = {"success",
"unsuccessful",
+ "insufficient resources", "incorrect version", "invalid format",
+ "auth error", "power event (AC)", "power event (batt)",
+ "unsatisfied dependencies"};
+
+#define EFI_FW_TYPE_STR_GET(idx) (\
+EFI_ESRT_FW_TYPE_NUM > (idx) ? efi_fw_type_str[(idx)] : "error"\
+)
+
+#define EFI_FW_STATUS_STR_GET(idx) (\
+EFI_ESRT_UPDATE_STATUS_NUM > (idx) ? efi_update_status_str[(idx)] : "error"\
+)
+
+/**
+ * do_efi_capsule_esrt() - manage UEFI capsules
+ *
+ * @cmdtp: Command table
+ * @flag: Command flag
+ * @argc: Number of arguments
+ * @argv: Argument array
+ * Return: CMD_RET_SUCCESS on success,
+ * CMD_RET_USAGE or CMD_RET_RET_FAILURE on failure
+ *
+ * Implement efidebug "capsule esrt" sub-command.
+ * The prints the current ESRT table.
+ *
+ * efidebug capsule esrt
+ */
+static int do_efi_capsule_esrt(struct cmd_tbl *cmdtp, int flag,
+ int argc, char * const argv[])
+{
+ struct efi_system_resource_table *esrt = NULL;
+
+ if (argc != 1)
+ return CMD_RET_USAGE;
+
+ for (int idx = 0; idx < systab.nr_tables; idx++)
+ if (!guidcmp(&efi_esrt_guid, &systab.tables[idx].guid))
+ esrt = (struct efi_system_resource_table
*)systab.tables[idx].table;
+
+ if (!esrt) {
+ log_info("ESRT: table not present\n");
+ return CMD_RET_SUCCESS;
+ }
+
+ printf("\n");
+ printf("ESRT: fw_resource_count=%d\n", esrt->fw_resource_count);
+ printf("ESRT: fw_resource_count_max=%d\n", esrt->fw_resource_count_max);
+ printf("ESRT: fw_resource_version=%lld\n", esrt->fw_resource_version);
+
+ for (int idx = 0; idx < esrt->fw_resource_count; idx++) {
+ printf("[entry %d]==\n", idx);
+ printf("ESRT: fw_class=%pUL\n", &esrt->entries[idx].fw_class);
+ printf("ESRT: fw_type=%s\n",
EFI_FW_TYPE_STR_GET(esrt->entries[idx].fw_type));
+ printf("ESRT: fw_version=%d\n", esrt->entries[idx].fw_version);
+ printf("ESRT: lowest_supported_fw_version=%d\n",
+ esrt->entries[idx].lowest_supported_fw_version);
+ printf("ESRT: capsule_flags=%d\n",
+ esrt->entries[idx].capsule_flags);
+ printf("ESRT: last_attempt_version=%d\n",
+ esrt->entries[idx].last_attempt_version);
+ printf("ESRT: last_attempt_status=%s\n",
+
EFI_FW_STATUS_STR_GET(esrt->entries[idx].last_attempt_status));
+ }
+ printf("\n");
+
+ return CMD_RET_SUCCESS;
+}
+#endif /* CONFIG_EFI_ESRT */
/**
* do_efi_capsule_res() - show a capsule update result
*
@@ -221,6 +297,10 @@ static struct cmd_tbl cmd_efidebug_capsule_sub[] = {
"", ""),
U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
"", ""),
+#ifdef CONFIG_EFI_ESRT
+ U_BOOT_CMD_MKENT(esrt, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_esrt,
+"", ""),
+#endif
U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
"", ""),
U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
@@ -1584,6 +1664,10 @@ static char efidebug_help_text[] =
" - show capsule information\n"
"efidebug capsule result []\n"
" - show a capsule update result\n"
+#ifdef CONFIG_EFI_ESRT
+ "efidebug capsule esrt\n"
+ " - print the ESRT\n"
+#endif
"\n"
#endif
"efidebug devices\n"
--
2.17.1
[PATCH 4/4 v7] Test the ESRT creation within an existing pytest.
This commit slightly extends test_efi_capsule_fw3. In order to run the test the following must be added to sandbox_defconfig: +CONFIG_CMD_SF=y +CONFIG_CMD_MEMORY=y +CONFIG_CMD_FAT=y +CONFIG_DFU=y The ESRT is printed in the u-boot shell by calling efidebug esrt. The test ensures that, after the capsule is installed, the ESRT contains entries with the GUIDs: - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; test invocation: sudo ./test/py/test.py --bd sandbox -k capsule_fw3 -l --build CC: Heinrich Schuchardt CC: Sughosh Ganu CC: AKASHI Takahiro CC: Ilias Apalodimas CC: Andre Przywara CC: Alexander Graf CC: n...@arm.com Signed-off-by: Jose Marinho Reviewed-by: Heinrich Schuchardt --- test/py/tests/test_efi_capsule/test_capsule_firmware.py | 8 1 file changed, 8 insertions(+) diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py index f006fa95d6..24101e8915 100644 --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py @@ -229,6 +229,14 @@ class TestEfiCapsuleFirmwareFit(object): output = u_boot_console.run_command( 'env print -e -all Capsule') +output = u_boot_console.run_command_list(['efidebug capsule esrt']) + +# ensure that EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID is in the ESRT. +assert 'AE13FF2D-9AD4-4E25-9AC8-6D80B3B22147' in ''.join(output) + +# ensure that EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID is in the ESRT. +assert 'E2BB9C06-70E9-4B14-97A3-5A7913176E3F' in ''.join(output) + output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, 'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR]) -- 2.17.1
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
> I am not referring to efidebug. > > The user can update EFI variables with any binary content using an EFI binary > or OS functions. > > E.g. copy a binary file to the efi variables file system in Linux. Right, I was mostly thinking SetVariable which still goes through u-boot. Copying an entire file is possiible though I guess Cheers /Ilias
[PATCH 0/7] Add FU740 chip and HiFive Unmatched board support
This patch set is to add SiFive fu740 chip and HiFive Unmatched board support. Patches are split into several parts: - [PATCH 1/7] dts for fu740 - [PATCH 2/7] support for fu740 cpu - [PATCH 3/7, 4/7] support for fu740 clk and ram - [PATCH 5/7] add pcie driver - [PATCH 6/7] add unmatched board - [PATCH 7/7] add fu740 support to macb driver Description - For fu740 cpu support, reuse most of fu540 cpu. - For prci driver, add one abstract layer to separate fu540 and fu740. Move orignal fu540 code to separate files. - For pcie driver, it depends on gpio, prci, clk and reset drivers to do init&reset works. - Align with Linux DT file. Tests and patch checks - Able to boot both unmatched and unleashed boards. - PCIe tests . M.2 NVMe SSD . e1000 compatibale ethernet adapter (ping&tftpboot) . pci-to-usb adapter(usb mass storage) - checkpatch is performed. To keep code derived from other boards the same, ignore some warnings/errors in [PATCH 6/7]. Green Wan (7): riscv: dts: add fu740 support riscv: cpu: fu740: Add support for cpu fu740 drivers: clk: add fu740 support drivers: ram: add fu740 support drivers: pci: add pcie support for fu740 board: sifive: add HiFive Unmatched board support drivers: net: macb: add fu740 support arch/riscv/Kconfig |8 + arch/riscv/cpu/fu740/Kconfig | 37 + arch/riscv/cpu/fu740/Makefile | 12 + arch/riscv/cpu/fu740/cache.c | 55 + arch/riscv/cpu/fu740/cpu.c | 22 + arch/riscv/cpu/fu740/dram.c| 38 + arch/riscv/cpu/fu740/spl.c | 23 + arch/riscv/dts/Makefile|1 + arch/riscv/dts/fu740-c000-u-boot.dtsi | 105 ++ arch/riscv/dts/fu740-c000.dtsi | 329 + arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi | 1489 arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi| 40 + arch/riscv/dts/hifive-unmatched-a00.dts| 263 arch/riscv/include/asm/arch-fu740/cache.h | 14 + arch/riscv/include/asm/arch-fu740/clk.h| 14 + arch/riscv/include/asm/arch-fu740/gpio.h | 38 + arch/riscv/include/asm/arch-fu740/reset.h | 13 + arch/riscv/include/asm/arch-fu740/spl.h| 14 + arch/riscv/lib/sifive_clint.c |1 - board/sifive/fu540/Kconfig |1 - board/sifive/fu540/fu540.c |2 +- board/sifive/hifive_unmatched_fu740/Kconfig| 50 + board/sifive/hifive_unmatched_fu740/MAINTAINERS|8 + board/sifive/hifive_unmatched_fu740/Makefile |9 + .../hifive-unmatched-fu740.c | 24 + board/sifive/hifive_unmatched_fu740/spl.c | 85 ++ common/spl/Kconfig |5 +- configs/sifive_hifive_unmatched_fu740_defconfig| 57 + doc/board/sifive/fu540.rst | 19 +- drivers/clk/sifive/Kconfig |8 +- drivers/clk/sifive/Makefile|4 +- drivers/clk/sifive/fu540-prci.c| 769 +- drivers/clk/sifive/fu540-prci.h| 22 + drivers/clk/sifive/fu740-prci.c| 158 +++ drivers/clk/sifive/fu740-prci.h| 22 + drivers/clk/sifive/sifive-prci.c | 733 ++ drivers/clk/sifive/sifive-prci.h | 323 + drivers/net/macb.c |8 +- drivers/pci/Kconfig|9 + drivers/pci/Makefile |1 + drivers/pci/pcie_sifive.c | 797 +++ drivers/pci/pcie_sifive.h | 374 + drivers/ram/sifive/Kconfig |8 +- drivers/ram/sifive/Makefile|2 +- drivers/ram/sifive/sifive_ddr.c| 410 ++ drivers/reset/Kconfig |2 +- include/configs/sifive-fu540.h |5 - include/configs/sifive-hifive-unmatched-fu740.h| 88 ++ include/dt-bindings/clock/sifive-fu740-prci.h | 25 + include/dt-bindings/reset/sifive-fu740-prci.h | 19 + 50 files changed, 5782 insertions(+), 781 deletions(-) create mode 100644 arch/riscv/cpu/fu740/Kconfig create mode 100644 arch/riscv/cpu/fu740/Makefile create mode 100644 arch/riscv/cpu/fu740/cache.c create mode 100644 arch/riscv/cpu/fu740/cpu.c create mode 100644 arch/riscv/cpu/fu740/dram.c create mode 100644 arch/riscv/cpu/fu740/spl.c create mode 100644 arch/riscv/dts/fu740-c000-u-boot.dtsi create mode 100644 arch/riscv/dts/fu740-c000.dtsi create mode 100644 arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi create mode 100644 arch/riscv/dts/hi
[PATCH 2/7] riscv: cpu: fu740: Add support for cpu fu740
Add SiFive fu740 cpu to support RISC-V arch
Signed-off-by: Green Wan
---
arch/riscv/Kconfig| 8 +
arch/riscv/cpu/fu740/Kconfig | 37 +
arch/riscv/cpu/fu740/Makefile | 12 +++
arch/riscv/cpu/fu740/cache.c | 55 +++
arch/riscv/cpu/fu740/cpu.c| 22 +
arch/riscv/cpu/fu740/dram.c | 38 +
arch/riscv/cpu/fu740/spl.c| 23 +
arch/riscv/include/asm/arch-fu740/cache.h | 14
arch/riscv/include/asm/arch-fu740/clk.h | 14
arch/riscv/include/asm/arch-fu740/gpio.h | 38 +
arch/riscv/include/asm/arch-fu740/reset.h | 13
arch/riscv/include/asm/arch-fu740/spl.h | 14
arch/riscv/lib/sifive_clint.c | 1 -
common/spl/Kconfig| 5 +--
14 files changed, 291 insertions(+), 3 deletions(-)
create mode 100644 arch/riscv/cpu/fu740/Kconfig
create mode 100644 arch/riscv/cpu/fu740/Makefile
create mode 100644 arch/riscv/cpu/fu740/cache.c
create mode 100644 arch/riscv/cpu/fu740/cpu.c
create mode 100644 arch/riscv/cpu/fu740/dram.c
create mode 100644 arch/riscv/cpu/fu740/spl.c
create mode 100644 arch/riscv/include/asm/arch-fu740/cache.h
create mode 100644 arch/riscv/include/asm/arch-fu740/clk.h
create mode 100644 arch/riscv/include/asm/arch-fu740/gpio.h
create mode 100644 arch/riscv/include/asm/arch-fu740/reset.h
create mode 100644 arch/riscv/include/asm/arch-fu740/spl.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 55eaee2..6bd30f0 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -20,6 +20,12 @@ config TARGET_QEMU_VIRT
config TARGET_SIFIVE_FU540
bool "Support SiFive FU540 Board"
+config TARGET_SIFIVE_UNMATCHED
+ bool "Support SiFive hifive-unmatched Board(FU740)"
+ help
+ Choose this option to select SiFive HiFive Unmatched board.
+ Include fu740 features such as prci, clk, reset, pcie, etc.
+
config TARGET_SIPEED_MAIX
bool "Support Sipeed Maix Board"
@@ -56,11 +62,13 @@ source "board/AndesTech/ax25-ae350/Kconfig"
source "board/emulation/qemu-riscv/Kconfig"
source "board/microchip/mpfs_icicle/Kconfig"
source "board/sifive/fu540/Kconfig"
+source "board/sifive/hifive_unmatched_fu740/Kconfig"
source "board/sipeed/maix/Kconfig"
# platform-specific options below
source "arch/riscv/cpu/ax25/Kconfig"
source "arch/riscv/cpu/fu540/Kconfig"
+source "arch/riscv/cpu/fu740/Kconfig"
source "arch/riscv/cpu/generic/Kconfig"
# architecture-specific options below
diff --git a/arch/riscv/cpu/fu740/Kconfig b/arch/riscv/cpu/fu740/Kconfig
new file mode 100644
index 000..b62fba6
--- /dev/null
+++ b/arch/riscv/cpu/fu740/Kconfig
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2020 SiFive, Inc
+# Pragnesh Patel
+
+config SIFIVE_FU740
+ bool
+ select ARCH_EARLY_INIT_R
+ select RAM
+ select SPL_RAM if SPL
+ imply CPU
+ imply CPU_RISCV
+ imply RISCV_TIMER if (RISCV_SMODE || SPL_RISCV_SMODE)
+ imply SIFIVE_CLINT if (RISCV_MMODE || SPL_RISCV_MMODE)
+ imply CMD_CPU
+ imply SPL_CPU_SUPPORT
+ imply SPL_OPENSBI
+ imply SPL_LOAD_FIT
+ imply SMP
+ imply CLK_SIFIVE
+ imply CLK_SIFIVE_PRCI
+ imply SIFIVE_SERIAL
+ imply MACB
+ imply MII
+ imply SPI
+ imply SPI_SIFIVE
+ imply MMC
+ imply MMC_SPI
+ imply MMC_BROKEN_CD
+ imply CMD_MMC
+ imply DM_GPIO
+ imply SIFIVE_GPIO
+ imply CMD_GPIO
+ imply MISC
+ imply SIFIVE_OTP
+ imply DM_PWM
+ imply PWM_SIFIVE
diff --git a/arch/riscv/cpu/fu740/Makefile b/arch/riscv/cpu/fu740/Makefile
new file mode 100644
index 000..088205e
--- /dev/null
+++ b/arch/riscv/cpu/fu740/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2020 SiFive, Inc
+# Pragnesh Patel
+
+ifeq ($(CONFIG_SPL_BUILD),y)
+obj-y += spl.o
+else
+obj-y += dram.o
+obj-y += cpu.o
+obj-y += cache.o
+endif
diff --git a/arch/riscv/cpu/fu740/cache.c b/arch/riscv/cpu/fu740/cache.c
new file mode 100644
index 000..680955c
--- /dev/null
+++ b/arch/riscv/cpu/fu740/cache.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020-2021 SiFive, Inc
+ *
+ * Authors:
+ * Pragnesh Patel
+ */
+
+#include
+#include
+#include
+#include
+
+/* Register offsets */
+#define L2_CACHE_CONFIG0x000
+#define L2_CACHE_ENABLE0x008
+
+#define MASK_NUM_WAYS GENMASK(15, 8)
+#define NUM_WAYS_SHIFT 8
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int cache_enable_ways(void)
+{
+ const void *blob = gd->fdt_blob;
+ int node;
+ fdt_addr_t base;
+ u32 config;
+ u32 ways;
+
+ volatile u32 *enable;
+
+ node = fdt_node_offset_by_compatible(blob, -1,
+"sifive,
[PATCH 1/7] riscv: dts: add fu740 support
Add dts support for fu740
Signed-off-by: Green Wan
---
arch/riscv/dts/Makefile | 1 +
arch/riscv/dts/fu740-c000-u-boot.dtsi | 105
arch/riscv/dts/fu740-c000.dtsi| 329 ++
include/dt-bindings/clock/sifive-fu740-prci.h | 25 ++
include/dt-bindings/reset/sifive-fu740-prci.h | 19 ++
5 files changed, 479 insertions(+)
create mode 100644 arch/riscv/dts/fu740-c000-u-boot.dtsi
create mode 100644 arch/riscv/dts/fu740-c000.dtsi
create mode 100644 include/dt-bindings/clock/sifive-fu740-prci.h
create mode 100644 include/dt-bindings/reset/sifive-fu740-prci.h
diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
index 01331b0..2254e6b 100644
--- a/arch/riscv/dts/Makefile
+++ b/arch/riscv/dts/Makefile
@@ -2,6 +2,7 @@
dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
dtb-$(CONFIG_TARGET_SIFIVE_FU540) += hifive-unleashed-a00.dtb
+dtb-$(CONFIG_TARGET_SIFIVE_UNMATCHED) += hifive-unmatched-a00.dtb
dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-mpfs-icicle-kit.dtb
diff --git a/arch/riscv/dts/fu740-c000-u-boot.dtsi
b/arch/riscv/dts/fu740-c000-u-boot.dtsi
new file mode 100644
index 000..0115a44
--- /dev/null
+++ b/arch/riscv/dts/fu740-c000-u-boot.dtsi
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * (C) Copyright 2020-2021 SiFive, Inc
+ */
+
+#include
+
+/ {
+ cpus {
+ assigned-clocks = <&prci PRCI_CLK_COREPLL>;
+ assigned-clock-rates = <100100>;
+ u-boot,dm-spl;
+ cpu0: cpu@0 {
+ clocks = <&prci PRCI_CLK_COREPLL>;
+ u-boot,dm-spl;
+ status = "okay";
+ cpu0_intc: interrupt-controller {
+ u-boot,dm-spl;
+ };
+ };
+ cpu1: cpu@1 {
+ clocks = <&prci PRCI_CLK_COREPLL>;
+ u-boot,dm-spl;
+ cpu1_intc: interrupt-controller {
+ u-boot,dm-spl;
+ };
+ };
+ cpu2: cpu@2 {
+ clocks = <&prci PRCI_CLK_COREPLL>;
+ u-boot,dm-spl;
+ cpu2_intc: interrupt-controller {
+u-boot,dm-spl;
+ };
+ };
+ cpu3: cpu@3 {
+ clocks = <&prci PRCI_CLK_COREPLL>;
+ u-boot,dm-spl;
+ cpu3_intc: interrupt-controller {
+ u-boot,dm-spl;
+ };
+ };
+ cpu4: cpu@4 {
+ clocks = <&prci PRCI_CLK_COREPLL>;
+ u-boot,dm-spl;
+ cpu4_intc: interrupt-controller {
+ u-boot,dm-spl;
+ };
+ };
+ };
+
+ soc {
+ u-boot,dm-spl;
+ clint: clint@200 {
+ compatible = "riscv,clint0";
+ interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
+ &cpu1_intc 3 &cpu1_intc 7
+ &cpu2_intc 3 &cpu2_intc 7
+ &cpu3_intc 3 &cpu3_intc 7
+ &cpu4_intc 3 &cpu4_intc 7>;
+ reg = <0x0 0x200 0x0 0x1>;
+ u-boot,dm-spl;
+ };
+ prci: clock-controller@1000 {
+ #reset-cells = <1>;
+ resets = <&prci PRCI_RST_DDR_CTRL_N>,
+<&prci PRCI_RST_DDR_AXI_N>,
+<&prci PRCI_RST_DDR_AHB_N>,
+<&prci PRCI_RST_DDR_PHY_N>,
+<&prci PRCI_RST_GEMGXL_N>,
+<&prci PRCI_RST_CLTX_N>;
+ reset-names = "ddr_ctrl", "ddr_axi", "ddr_ahb",
+ "ddr_phy", "gemgxl_reset", "cltx_reset";
+ };
+ dmc: dmc@100b {
+ compatible = "sifive,fu740-c000-ddr";
+ reg = <0x0 0x100b 0x0 0x0800
+ 0x0 0x100b2000 0x0 0x2000
+ 0x0 0x100b8000 0x0 0x1000>;
+ clocks = <&prci PRCI_CLK_DDRPLL>;
+ clock-frequency = <93324>;
+ u-boot,dm-spl;
+ };
+ };
+};
+
+&prci {
+ u-boot,dm-spl;
+};
+
+&uart0 {
+ u-boot,dm-spl;
+};
+
+&spi0 {
+ u-boot,dm-spl;
+};
+
+ð0 {
+ assigned-clocks = <&prci PRCI_CLK_GEMGXLPLL>;
+ assigned-clock-rates = <125125000>;
[PATCH 4/7] drivers: ram: add fu740 support
Add fu740 support for ram.
Signed-off-by: Green Wan
---
drivers/ram/sifive/Kconfig | 8 +-
drivers/ram/sifive/Makefile | 2 +-
drivers/ram/sifive/sifive_ddr.c | 410
3 files changed, 415 insertions(+), 5 deletions(-)
create mode 100644 drivers/ram/sifive/sifive_ddr.c
diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
index 6aca22a..7685184 100644
--- a/drivers/ram/sifive/Kconfig
+++ b/drivers/ram/sifive/Kconfig
@@ -5,9 +5,9 @@ config RAM_SIFIVE
help
This enables support for ram drivers of SiFive SoCs.
-config SIFIVE_FU540_DDR
- bool "SiFive FU540 DDR driver"
+config SIFIVE_DDR
+ bool "SiFive DDR driver"
depends on RAM_SIFIVE
- default y if TARGET_SIFIVE_FU540
+ default y if TARGET_SIFIVE_FU540 || TARGET_SIFIVE_UNMATCHED
help
- This enables DDR support for the platforms based on SiFive FU540 SoC.
+ This enables DDR support for the platforms based on SiFive SoCs.
diff --git a/drivers/ram/sifive/Makefile b/drivers/ram/sifive/Makefile
index d66efec..4ef89f8 100644
--- a/drivers/ram/sifive/Makefile
+++ b/drivers/ram/sifive/Makefile
@@ -3,4 +3,4 @@
# Copyright (c) 2020 SiFive, Inc
#
-obj-$(CONFIG_SIFIVE_FU540_DDR) += fu540_ddr.o
+obj-$(CONFIG_SIFIVE_DDR) += sifive_ddr.o
diff --git a/drivers/ram/sifive/sifive_ddr.c b/drivers/ram/sifive/sifive_ddr.c
new file mode 100644
index 000..d89a02b
--- /dev/null
+++ b/drivers/ram/sifive/sifive_ddr.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
+/*
+ * (C) Copyright 2020-2021 SiFive, Inc.
+ *
+ * Authors:
+ * Pragnesh Patel
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DENALI_CTL_0 0
+#define DENALI_CTL_21 21
+#define DENALI_CTL_120 120
+#define DENALI_CTL_132 132
+#define DENALI_CTL_136 136
+#define DENALI_CTL_170 170
+#define DENALI_CTL_181 181
+#define DENALI_CTL_182 182
+#define DENALI_CTL_184 184
+#define DENALI_CTL_208 208
+#define DENALI_CTL_209 209
+#define DENALI_CTL_210 210
+#define DENALI_CTL_212 212
+#define DENALI_CTL_214 214
+#define DENALI_CTL_216 216
+#define DENALI_CTL_224 224
+#define DENALI_CTL_225 225
+#define DENALI_CTL_260 260
+
+#define DENALI_PHY_11521152
+#define DENALI_PHY_12141214
+
+#define DRAM_CLASS_OFFSET 8
+#define DRAM_CLASS_DDR40xA
+#define OPTIMAL_RMODW_EN_OFFSET0
+#define DISABLE_RD_INTERLEAVE_OFFSET 16
+#define OUT_OF_RANGE_OFFSET1
+#define MULTIPLE_OUT_OF_RANGE_OFFSET 2
+#define PORT_COMMAND_CHANNEL_ERROR_OFFSET 7
+#define MC_INIT_COMPLETE_OFFSET8
+#define LEVELING_OPERATION_COMPLETED_OFFSET22
+#define DFI_PHY_WRLELV_MODE_OFFSET 24
+#define DFI_PHY_RDLVL_MODE_OFFSET 24
+#define DFI_PHY_RDLVL_GATE_MODE_OFFSET 0
+#define VREF_EN_OFFSET 24
+#define PORT_ADDR_PROTECTION_EN_OFFSET 0
+#define AXI0_ADDRESS_RANGE_ENABLE 8
+#define AXI0_RANGE_PROT_BITS_0_OFFSET 24
+#define RDLVL_EN_OFFSET16
+#define RDLVL_GATE_EN_OFFSET 24
+#define WRLVL_EN_OFFSET0
+
+#define PHY_RX_CAL_DQ0_0_OFFSET0
+#define PHY_RX_CAL_DQ1_0_OFFSET16
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct sifive_ddrctl {
+ volatile u32 denali_ctl[265];
+};
+
+struct sifive_ddrphy {
+ volatile u32 denali_phy[1215];
+};
+
+/**
+ * struct sifive_ddr_info
+ *
+ * @dev : pointer for the device
+ * @info: UCLASS RAM information
+ * @ctl : DDR controller base address
+ * @phy : DDR PHY base address
+ * @ctrl: DDR control base address
+ * @physical_filter_ctrl: DDR physical filter control base address
+ */
+struct sifive_ddr_info {
+ struct udevice *dev;
+ struct ram_info info;
+ struct sifive_ddrctl *ctl;
+ struct sifive_ddrphy *phy;
+ struct clk ddr_clk;
+ u32 *physical_filter_ctrl;
+};
+
+#if defined(CONFIG_SPL_BUILD)
+struct sifive_ddr_params {
+ struct sifive_ddrctl pctl_regs;
+ struct sifive_ddrphy phy_regs;
+};
+
+struct sifive_dmc_plat {
+ struct sifive_ddr_params ddr_params;
+};
+
+/*
+ * TODO : It can be possible to use common sdram_copy_to_reg() API
+ * n: Unit bytes
+ */
+static void sdram_copy_to_reg(volatile u32 *dest,
+ volatile u32 *src, u32 n)
+{
+ int i;
+
+ for (i = 0; i < n / sizeof(u32); i++) {
+ writel(*src, dest);
+ src++;
+ dest++;
+ }
+}
+
+static void sifive_ddr_setup_range_protection(volatile u32 *ctl, u64 end_addr)
+{
+ u32 end_addr_16kblocks = ((e
[PATCH 5/7] drivers: pci: add pcie support for fu740
Add pcie driver for SiFive fu740, the driver depends on
fu740 gpio, clk and reset driver to do init. Force running at Gen1
for better capatible enumeration.
Several devices are tested:
a) M.2 NVMe SSD
b) USB-to-PCI adapter
c) Ethernet adapter (E1000 compatible)
Signed-off-by: Green Wan
---
drivers/pci/Kconfig | 9 +
drivers/pci/Makefile | 1 +
drivers/pci/pcie_sifive.c | 797 ++
drivers/pci/pcie_sifive.h | 374 ++
4 files changed, 1181 insertions(+)
create mode 100644 drivers/pci/pcie_sifive.c
create mode 100644 drivers/pci/pcie_sifive.h
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index ba41787..b078e76 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -97,6 +97,15 @@ config PCIE_DW_MVEBU
Armada-8K SoCs. The PCIe controller on Armada-8K is based on
DesignWare hardware.
+config PCIE_SIFIVE_FU740
+ bool "Enable SiFive FU740 PCIe"
+ depends on CLK_SIFIVE_PRCI
+ depends on RESET_SIFIVE
+ depends on SIFIVE_GPIO
+ help
+ Say Y here if you want to enable PCIe controller support on
+ FU740.
+
config PCIE_FSL
bool "FSL PowerPC PCIe support"
depends on DM_PCI
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 5ed94bc..5400d59 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -51,3 +51,4 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o
obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o
obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o
obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o
+obj-$(CONFIG_PCIE_SIFIVE_FU740) += pcie_sifive.o
diff --git a/drivers/pci/pcie_sifive.c b/drivers/pci/pcie_sifive.c
new file mode 100644
index 000..ada6087
--- /dev/null
+++ b/drivers/pci/pcie_sifive.c
@@ -0,0 +1,797 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SiFive FU740 DesignWare PCIe Controller
+ *
+ * Copyright (C) 2020-2021 SiFive, Inc.
+ *
+ * Based in early part on the i.MX6 PCIe host controller shim which is:
+ *
+ * Copyright (C) 2013 Kosagi
+ * http://www.kosagi.com
+ *
+ * Based on driver from author: Alan Mikhak
+ */
+#include "pcie_sifive.h"
+#include
+#include
+
+/* Host Bridge Identification */
+#define DEVICE_NAME"SiFive FU740 PCIe Host Controller"
+#define VENDOR_ID 0x51fe
+#define DEVICE_ID 0x51fe
+
+static enum pcie_sifive_devtype pcie_sifive_get_devtype(struct pcie_sifive *sv)
+{
+ u32 val;
+
+ val = readl(sv->priv.iobase + MGMT_MISC_DEVICE_TYPE_OFFSET);
+ switch (val) {
+ case MGMT_MISC_DEVICE_TYPE_RC:
+ return SV_PCIE_HOST_TYPE;
+ case MGMT_MISC_DEVICE_TYPE_EP:
+ return SV_PCIE_ENDPOINT_TYPE;
+ default:
+ return SV_PCIE_UNKNOWN_TYPE;
+ }
+}
+
+static void pcie_sifive_priv_set_state(struct pcie_sifive *sv, u32 reg,
+ u32 bits, int state)
+{
+ u32 val;
+
+ val = readl(sv->priv.iobase + reg);
+ val = state ? (val | bits) : (val & !bits);
+ writel(val, sv->priv.iobase + reg);
+}
+
+static void pcie_sifive_assert_perstn(struct pcie_sifive *sv)
+{
+ dm_gpio_set_value(&sv->perstn_gpio, 0);
+ writel(0x0, sv->priv.iobase + PCIEX8MGMT_PERST_N);
+ mdelay(100);
+}
+
+static void pcie_sifive_power_on(struct pcie_sifive *sv)
+{
+ dm_gpio_set_value(&sv->pwren_gpio, 1);
+ mdelay(100);
+}
+
+static void pcie_sifive_deassert_perstn(struct pcie_sifive *sv)
+{
+ writel(0x1, sv->priv.iobase + PCIEX8MGMT_PERST_N);
+ dm_gpio_set_value(&sv->perstn_gpio, 1);
+ mdelay(100);
+}
+
+static int pcie_sifive_setphy(const u8 phy, const u8 write,
+ const u16 addr, const u16 wrdata,
+ u16 *rddata, struct pcie_sifive *sv)
+{
+ unsigned char ack = 0;
+
+ if (!(phy == 0 || phy == 1))
+ return -2;
+
+ /* setup phy para */
+ writel(addr, sv->priv.iobase +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_ADDR :
+ PCIEX8MGMT_PHY0_CR_PARA_ADDR));
+
+ if (write)
+ writel(wrdata, sv->priv.iobase +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_DATA :
+ PCIEX8MGMT_PHY0_CR_PARA_WR_DATA));
+
+ /* enable access if write */
+ if (write)
+ writel(1, sv->priv.iobase +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_EN :
+ PCIEX8MGMT_PHY0_CR_PARA_WR_EN));
+ else
+ writel(1, sv->priv.iobase +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_RD_EN :
+ PCIEX8MGMT_PHY0_CR_PARA_RD_EN));
+
+ /* wait for wait_idle */
+ do {
+ u32 val;
+
+ val = readl(sv->priv.iobase +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_ACK :
+PCIEX8MGMT_PHY0_CR_PARA_ACK));
+ if (val) {
+
[PATCH 3/7] drivers: clk: add fu740 support
Add fu740 support. One abstract layer is added for supporting multiple chips such as fu540 and fu740. Signed-off-by: Green Wan --- drivers/clk/sifive/Kconfig | 8 +- drivers/clk/sifive/Makefile | 4 +- drivers/clk/sifive/fu540-prci.c | 769 ++- drivers/clk/sifive/fu540-prci.h | 22 ++ drivers/clk/sifive/fu740-prci.c | 158 drivers/clk/sifive/fu740-prci.h | 22 ++ drivers/clk/sifive/sifive-prci.c | 733 + drivers/clk/sifive/sifive-prci.h | 323 8 files changed, 1286 insertions(+), 753 deletions(-) create mode 100644 drivers/clk/sifive/fu540-prci.h create mode 100644 drivers/clk/sifive/fu740-prci.c create mode 100644 drivers/clk/sifive/fu740-prci.h create mode 100644 drivers/clk/sifive/sifive-prci.c create mode 100644 drivers/clk/sifive/sifive-prci.h diff --git a/drivers/clk/sifive/Kconfig b/drivers/clk/sifive/Kconfig index c4d0a1f..20fc004 100644 --- a/drivers/clk/sifive/Kconfig +++ b/drivers/clk/sifive/Kconfig @@ -6,11 +6,11 @@ config CLK_SIFIVE help SoC drivers for SiFive Linux-capable SoCs. -config CLK_SIFIVE_FU540_PRCI - bool "PRCI driver for SiFive FU540 SoCs" +config CLK_SIFIVE_PRCI + bool "PRCI driver for SiFive SoCs" depends on CLK_SIFIVE select CLK_ANALOGBITS_WRPLL_CLN28HPC help Supports the Power Reset Clock interface (PRCI) IP block found in - FU540 SoCs. If this kernel is meant to run on a SiFive FU540 SoC, - enable this driver. + FU540/FU740 SoCs. If this kernel is meant to run on a SiFive FU540/ + FU740 SoCs, enable this driver. diff --git a/drivers/clk/sifive/Makefile b/drivers/clk/sifive/Makefile index b224279..51348b1 100644 --- a/drivers/clk/sifive/Makefile +++ b/drivers/clk/sifive/Makefile @@ -1,3 +1,5 @@ # SPDX-License-Identifier: GPL-2.0+ -obj-$(CONFIG_CLK_SIFIVE_FU540_PRCI)+= fu540-prci.o +obj-y += sifive-prci.o + +obj-$(CONFIG_CLK_SIFIVE_PRCI) += fu540-prci.o fu740-prci.o diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c index b3882d0..ceb2c6f 100644 --- a/drivers/clk/sifive/fu540-prci.c +++ b/drivers/clk/sifive/fu540-prci.c @@ -5,6 +5,8 @@ * Copyright (C) 2018 SiFive, Inc. * Wesley Terpstra * Paul Walmsley + * Zong Li + * Pragnesh Patel * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -15,632 +17,48 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * - * The FU540 PRCI implements clock and reset control for the SiFive - * FU540-C000 chip. This driver assumes that it has sole control - * over all PRCI resources. - * - * This driver is based on the PRCI driver written by Wesley Terpstra. - * - * Refer, commit 999529edf517ed75b56659d456d221b2ee56bb60 of: - * https://github.com/riscv/riscv-linux - * * References: * - SiFive FU540-C000 manual v1p0, Chapter 7 "Clocking and Reset" */ -#include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -/* - * EXPECTED_CLK_PARENT_COUNT: how many parent clocks this driver expects: - * hfclk and rtcclk - */ -#define EXPECTED_CLK_PARENT_COUNT 2 - -/* - * Register offsets and bitmasks - */ - -/* COREPLLCFG0 */ -#define PRCI_COREPLLCFG0_OFFSET0x4 -#define PRCI_COREPLLCFG0_DIVR_SHIFT0 -#define PRCI_COREPLLCFG0_DIVR_MASK (0x3f << PRCI_COREPLLCFG0_DIVR_SHIFT) -#define PRCI_COREPLLCFG0_DIVF_SHIFT6 -#define PRCI_COREPLLCFG0_DIVF_MASK (0x1ff << PRCI_COREPLLCFG0_DIVF_SHIFT) -#define PRCI_COREPLLCFG0_DIVQ_SHIFT15 -#define PRCI_COREPLLCFG0_DIVQ_MASK (0x7 << PRCI_COREPLLCFG0_DIVQ_SHIFT) -#define PRCI_COREPLLCFG0_RANGE_SHIFT 18 -#define PRCI_COREPLLCFG0_RANGE_MASK(0x7 << PRCI_COREPLLCFG0_RANGE_SHIFT) -#define PRCI_COREPLLCFG0_BYPASS_SHIFT 24 -#define PRCI_COREPLLCFG0_BYPASS_MASK (0x1 << PRCI_COREPLLCFG0_BYPASS_SHIFT) -#define PRCI_COREPLLCFG0_FSE_SHIFT 25 -#define PRCI_COREPLLCFG0_FSE_MASK (0x1 << PRCI_COREPLLCFG0_FSE_SHIFT) -#define PRCI_COREPLLCFG0_LOCK_SHIFT31 -#define PRCI_COREPLLCFG0_LOCK_MASK (0x1 << PRCI_COREPLLCFG0_LOCK_SHIFT) - -/* COREPLLCFG1 */ -#define PRCI_COREPLLCFG1_OFFSET0x8 -#define PRCI_COREPLLCFG1_CKE_SHIFT 31 -#define PRCI_COREPLLCFG1_CKE_MASK (0x1 << PRCI_COREPLLCFG1_CKE_SHIFT) - -/* DDRPLLCFG0 */ -#define PRCI_DDRPLLCFG0_OFFSET 0xc -#define PRCI_DDRPLLCFG0_DIVR_SHIFT 0 -#define PRCI_DDRPLLCFG0_DIVR_MASK (0x3f << PRCI_DDRPLLCFG0_DIVR_SHIFT) -#define PRCI_DDRPLLCFG0_DIVF_SHIFT 6 -#define PRCI_DDRPLLCFG0_DIVF_MASK (0x1ff << PRCI_DDRPLLCFG0_DIVF_SHIFT) -#define PRCI_DDRPLLCFG0_DIVQ_SHIFT 15 -#define PRCI_DDRPLLCFG0_DIVQ_MASK (0x7
[PATCH 6/7] board: sifive: add HiFive Unmatched board support
Add dts, defconfig and board support for HiFive Unmatched.
Signed-off-by: Green Wan
---
arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi | 1489
arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi| 40 +
arch/riscv/dts/hifive-unmatched-a00.dts| 263
board/sifive/fu540/Kconfig |1 -
board/sifive/fu540/fu540.c |2 +-
board/sifive/hifive_unmatched_fu740/Kconfig| 50 +
board/sifive/hifive_unmatched_fu740/MAINTAINERS|9 +
board/sifive/hifive_unmatched_fu740/Makefile |9 +
.../hifive-unmatched-fu740.c | 24 +
board/sifive/hifive_unmatched_fu740/spl.c | 85 ++
configs/sifive_hifive_unmatched_fu740_defconfig| 57 +
doc/board/sifive/fu540.rst | 19 +-
doc/board/sifive/hifive_unmatched_fu740.rst| 532 +++
drivers/reset/Kconfig |2 +-
include/configs/sifive-fu540.h |5 -
include/configs/sifive-hifive-unmatched-fu740.h| 88 ++
16 files changed, 2656 insertions(+), 19 deletions(-)
create mode 100644 arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
create mode 100644 arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi
create mode 100644 arch/riscv/dts/hifive-unmatched-a00.dts
create mode 100644 board/sifive/hifive_unmatched_fu740/Kconfig
create mode 100644 board/sifive/hifive_unmatched_fu740/MAINTAINERS
create mode 100644 board/sifive/hifive_unmatched_fu740/Makefile
create mode 100644 board/sifive/hifive_unmatched_fu740/hifive-unmatched-fu740.c
create mode 100644 board/sifive/hifive_unmatched_fu740/spl.c
create mode 100644 configs/sifive_hifive_unmatched_fu740_defconfig
create mode 100644 doc/board/sifive/hifive_unmatched_fu740.rst
create mode 100644 include/configs/sifive-hifive-unmatched-fu740.h
diff --git a/arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
b/arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
new file mode 100644
index 000..4209491
--- /dev/null
+++ b/arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
@@ -0,0 +1,1489 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * (C) Copyright 2020-2021 SiFive, Inc
+ */
+
+&dmc {
+ sifive,ddr-params = <
+ 0x0a00 /* DENALI_CTL_00_DATA */
+ 0x /* DENALI_CTL_01_DATA */
+ 0x /* DENALI_CTL_02_DATA */
+ 0x /* DENALI_CTL_03_DATA */
+ 0x /* DENALI_CTL_04_DATA */
+ 0x /* DENALI_CTL_05_DATA */
+ 0x000a /* DENALI_CTL_06_DATA */
+ 0x0002d362 /* DENALI_CTL_07_DATA */
+ 0x00071073 /* DENALI_CTL_08_DATA */
+ 0x0a1c0255 /* DENALI_CTL_09_DATA */
+ 0x1c1c0400 /* DENALI_CTL_10_DATA */
+ 0x0404c90b /* DENALI_CTL_11_DATA */
+ 0x2b050405 /* DENALI_CTL_12_DATA */
+ 0x0d0c081e /* DENALI_CTL_13_DATA */
+ 0x08090914 /* DENALI_CTL_14_DATA */
+ 0x00fde718 /* DENALI_CTL_15_DATA */
+ 0x00180a05 /* DENALI_CTL_16_DATA */
+ 0x008b130d /* DENALI_CTL_17_DATA */
+ 0x01000118 /* DENALI_CTL_18_DATA */
+ 0x0d032001 /* DENALI_CTL_19_DATA */
+ 0x /* DENALI_CTL_20_DATA */
+ 0x0101 /* DENALI_CTL_21_DATA */
+ 0x /* DENALI_CTL_22_DATA */
+ 0x0a00 /* DENALI_CTL_23_DATA */
+ 0x /* DENALI_CTL_24_DATA */
+ 0x01450100 /* DENALI_CTL_25_DATA */
+ 0x1c36 /* DENALI_CTL_26_DATA */
+ 0x0005 /* DENALI_CTL_27_DATA */
+ 0x00170006 /* DENALI_CTL_28_DATA */
+ 0x014e0400 /* DENALI_CTL_29_DATA */
+ 0x0301 /* DENALI_CTL_30_DATA */
+ 0x000a0e00 /* DENALI_CTL_31_DATA */
+ 0x04030200 /* DENALI_CTL_32_DATA */
+ 0x031f /* DENALI_CTL_33_DATA */
+ 0x00070004 /* DENALI_CTL_34_DATA */
+ 0x /* DENALI_CTL_35_DATA */
+ 0x /* DENALI_CTL_36_DATA */
+ 0x /* DENALI_CTL_37_DATA */
+ 0x /* DENALI_CTL_38_DATA */
+ 0x /* DENALI_CTL_39_DATA */
+ 0x /* DENALI_CTL_40_DATA */
+ 0x /* DENALI_CTL_41_DATA */
+ 0x /* DENALI_CTL_42_DATA */
+ 0x /* DENALI_CTL_43_DATA */
+ 0x /* DENALI_CTL_44_DATA */
+ 0x /* DENALI_CTL_45_DATA */
+ 0x /* DENALI_CTL_46_DATA */
+ 0x /* DENALI_CTL_47_DATA */
+ 0x /* DENALI_CTL_48_DATA */
+ 0x /* DENALI_CTL_49_DATA */
+ 0x /* DENALI_CTL_50_DATA */
+ 0x /* DENALI_CTL_51_DATA */
+
[PATCH 7/7] drivers: net: macb: add fu740 support
Add fu740 support to macb ethernet driver
Signed-off-by: Green Wan
---
drivers/net/macb.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 57ea45e..df65d82 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -592,7 +592,11 @@ static int macb_sifive_clk_init(struct udevice *dev, ulong
rate)
* and output clock on GMII output signal GTX_CLK
* 1 = MII mode. Use MII input signal TX_CLK in TX logic
*/
- writel(rate != 12500, gemgxl_regs);
+ if (device_is_compatible(dev, "sifive,fu540-c000-gem"))
+ writel(rate != 12500, gemgxl_regs);
+ else if (device_is_compatible(dev, "sifive,fu740-c000-gem"))
+ writel(rate != 125125000, gemgxl_regs);
+
return 0;
}
@@ -1507,6 +1511,8 @@ static const struct udevice_id macb_eth_ids[] = {
{ .compatible = "cdns,zynq-gem" },
{ .compatible = "sifive,fu540-c000-gem",
.data = (ulong)&sifive_config },
+ { .compatible = "sifive,fu740-c000-gem",
+ .data = (ulong)&sifive_config },
{ .compatible = "microchip,mpfs-mss-gem",
.data = (ulong)µchip_config },
{ }
--
2.7.4
Re: [PATCH 1/7] riscv: dts: add fu740 support
On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote:
>
> Add dts support for fu740
The commit summary should mention both fu740 and HiFive Unmatched
>
> Signed-off-by: Green Wan
> ---
> arch/riscv/dts/Makefile | 1 +
> arch/riscv/dts/fu740-c000-u-boot.dtsi | 105
> arch/riscv/dts/fu740-c000.dtsi| 329
> ++
> include/dt-bindings/clock/sifive-fu740-prci.h | 25 ++
> include/dt-bindings/reset/sifive-fu740-prci.h | 19 ++
> 5 files changed, 479 insertions(+)
> create mode 100644 arch/riscv/dts/fu740-c000-u-boot.dtsi
> create mode 100644 arch/riscv/dts/fu740-c000.dtsi
> create mode 100644 include/dt-bindings/clock/sifive-fu740-prci.h
> create mode 100644 include/dt-bindings/reset/sifive-fu740-prci.h
>
> diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
> index 01331b0..2254e6b 100644
> --- a/arch/riscv/dts/Makefile
> +++ b/arch/riscv/dts/Makefile
> @@ -2,6 +2,7 @@
>
> dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
> dtb-$(CONFIG_TARGET_SIFIVE_FU540) += hifive-unleashed-a00.dtb
> +dtb-$(CONFIG_TARGET_SIFIVE_UNMATCHED) += hifive-unmatched-a00.dtb
> dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
> dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-mpfs-icicle-kit.dtb
>
> diff --git a/arch/riscv/dts/fu740-c000-u-boot.dtsi
> b/arch/riscv/dts/fu740-c000-u-boot.dtsi
> new file mode 100644
> index 000..0115a44
> --- /dev/null
> +++ b/arch/riscv/dts/fu740-c000-u-boot.dtsi
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * (C) Copyright 2020-2021 SiFive, Inc
> + */
> +
> +#include
> +
> +/ {
> + cpus {
> + assigned-clocks = <&prci PRCI_CLK_COREPLL>;
> + assigned-clock-rates = <100100>;
> + u-boot,dm-spl;
> + cpu0: cpu@0 {
> + clocks = <&prci PRCI_CLK_COREPLL>;
> + u-boot,dm-spl;
> + status = "okay";
> + cpu0_intc: interrupt-controller {
> + u-boot,dm-spl;
> + };
> + };
> + cpu1: cpu@1 {
> + clocks = <&prci PRCI_CLK_COREPLL>;
> + u-boot,dm-spl;
> + cpu1_intc: interrupt-controller {
> + u-boot,dm-spl;
> + };
> + };
> + cpu2: cpu@2 {
> + clocks = <&prci PRCI_CLK_COREPLL>;
> + u-boot,dm-spl;
> + cpu2_intc: interrupt-controller {
> +u-boot,dm-spl;
> + };
> + };
> + cpu3: cpu@3 {
> + clocks = <&prci PRCI_CLK_COREPLL>;
> + u-boot,dm-spl;
> + cpu3_intc: interrupt-controller {
> + u-boot,dm-spl;
> + };
> + };
> + cpu4: cpu@4 {
> + clocks = <&prci PRCI_CLK_COREPLL>;
> + u-boot,dm-spl;
> + cpu4_intc: interrupt-controller {
> + u-boot,dm-spl;
> + };
> + };
> + };
> +
> + soc {
> + u-boot,dm-spl;
> + clint: clint@200 {
> + compatible = "riscv,clint0";
> + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> + &cpu1_intc 3 &cpu1_intc 7
> + &cpu2_intc 3 &cpu2_intc 7
> + &cpu3_intc 3 &cpu3_intc 7
> + &cpu4_intc 3 &cpu4_intc 7>;
> + reg = <0x0 0x200 0x0 0x1>;
> + u-boot,dm-spl;
> + };
> + prci: clock-controller@1000 {
> + #reset-cells = <1>;
> + resets = <&prci PRCI_RST_DDR_CTRL_N>,
> +<&prci PRCI_RST_DDR_AXI_N>,
> +<&prci PRCI_RST_DDR_AHB_N>,
> +<&prci PRCI_RST_DDR_PHY_N>,
> +<&prci PRCI_RST_GEMGXL_N>,
> +<&prci PRCI_RST_CLTX_N>;
> + reset-names = "ddr_ctrl", "ddr_axi", "ddr_ahb",
> + "ddr_phy", "gemgxl_reset",
> "cltx_reset";
> + };
> + dmc: dmc@100b {
> + compatible = "sifive,fu740-c000-ddr";
> + reg = <0x0 0x100b 0x0 0x0800
> + 0x0 0x100b2000 0x0 0x2000
> + 0x0 0x100b8000 0x0 0x1000>;
> + clocks = <&prci PRCI_CLK_DDRPLL>;
>
Re: [PATCH 2/7] riscv: cpu: fu740: Add support for cpu fu740
On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote: > > Add SiFive fu740 cpu to support RISC-V arch > > Signed-off-by: Green Wan > --- > arch/riscv/Kconfig| 8 + > arch/riscv/cpu/fu740/Kconfig | 37 + > arch/riscv/cpu/fu740/Makefile | 12 +++ > arch/riscv/cpu/fu740/cache.c | 55 > +++ > arch/riscv/cpu/fu740/cpu.c| 22 + > arch/riscv/cpu/fu740/dram.c | 38 + > arch/riscv/cpu/fu740/spl.c| 23 + > arch/riscv/include/asm/arch-fu740/cache.h | 14 > arch/riscv/include/asm/arch-fu740/clk.h | 14 > arch/riscv/include/asm/arch-fu740/gpio.h | 38 + > arch/riscv/include/asm/arch-fu740/reset.h | 13 > arch/riscv/include/asm/arch-fu740/spl.h | 14 > arch/riscv/lib/sifive_clint.c | 1 - > common/spl/Kconfig| 5 +-- > 14 files changed, 291 insertions(+), 3 deletions(-) > create mode 100644 arch/riscv/cpu/fu740/Kconfig > create mode 100644 arch/riscv/cpu/fu740/Makefile > create mode 100644 arch/riscv/cpu/fu740/cache.c > create mode 100644 arch/riscv/cpu/fu740/cpu.c > create mode 100644 arch/riscv/cpu/fu740/dram.c > create mode 100644 arch/riscv/cpu/fu740/spl.c > create mode 100644 arch/riscv/include/asm/arch-fu740/cache.h > create mode 100644 arch/riscv/include/asm/arch-fu740/clk.h > create mode 100644 arch/riscv/include/asm/arch-fu740/gpio.h > create mode 100644 arch/riscv/include/asm/arch-fu740/reset.h > create mode 100644 arch/riscv/include/asm/arch-fu740/spl.h > Reviewed-by: Bin Meng
Re: [PATCH 4/7] drivers: ram: add fu740 support
On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote: > > Add fu740 support for ram. > > Signed-off-by: Green Wan > --- > drivers/ram/sifive/Kconfig | 8 +- > drivers/ram/sifive/Makefile | 2 +- > drivers/ram/sifive/sifive_ddr.c | 410 > > 3 files changed, 415 insertions(+), 5 deletions(-) > create mode 100644 drivers/ram/sifive/sifive_ddr.c > > diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig > index 6aca22a..7685184 100644 > --- a/drivers/ram/sifive/Kconfig > +++ b/drivers/ram/sifive/Kconfig > @@ -5,9 +5,9 @@ config RAM_SIFIVE > help > This enables support for ram drivers of SiFive SoCs. > > -config SIFIVE_FU540_DDR > - bool "SiFive FU540 DDR driver" > +config SIFIVE_DDR > + bool "SiFive DDR driver" > depends on RAM_SIFIVE > - default y if TARGET_SIFIVE_FU540 > + default y if TARGET_SIFIVE_FU540 || TARGET_SIFIVE_UNMATCHED > help > - This enables DDR support for the platforms based on SiFive FU540 > SoC. > + This enables DDR support for the platforms based on SiFive SoCs. > diff --git a/drivers/ram/sifive/Makefile b/drivers/ram/sifive/Makefile > index d66efec..4ef89f8 100644 > --- a/drivers/ram/sifive/Makefile > +++ b/drivers/ram/sifive/Makefile > @@ -3,4 +3,4 @@ > # Copyright (c) 2020 SiFive, Inc > # > > -obj-$(CONFIG_SIFIVE_FU540_DDR) += fu540_ddr.o > +obj-$(CONFIG_SIFIVE_DDR) += sifive_ddr.o > diff --git a/drivers/ram/sifive/sifive_ddr.c b/drivers/ram/sifive/sifive_ddr.c > new file mode 100644 Can you do a "git mv" to the old fu540_ddr.c? > index 000..d89a02b > --- /dev/null > +++ b/drivers/ram/sifive/sifive_ddr.c [snip] Regards, Bin
Re: [PATCH 5/7] drivers: pci: add pcie support for fu740
On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote: > > Add pcie driver for SiFive fu740, the driver depends on > fu740 gpio, clk and reset driver to do init. Force running at Gen1 > for better capatible enumeration. > > Several devices are tested: > a) M.2 NVMe SSD > b) USB-to-PCI adapter > c) Ethernet adapter (E1000 compatible) > > Signed-off-by: Green Wan > --- > drivers/pci/Kconfig | 9 + > drivers/pci/Makefile | 1 + > drivers/pci/pcie_sifive.c | 797 > ++ > drivers/pci/pcie_sifive.h | 374 ++ > 4 files changed, 1181 insertions(+) > create mode 100644 drivers/pci/pcie_sifive.c > create mode 100644 drivers/pci/pcie_sifive.h > > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig > index ba41787..b078e76 100644 > --- a/drivers/pci/Kconfig > +++ b/drivers/pci/Kconfig > @@ -97,6 +97,15 @@ config PCIE_DW_MVEBU > Armada-8K SoCs. The PCIe controller on Armada-8K is based on > DesignWare hardware. > > +config PCIE_SIFIVE_FU740 > + bool "Enable SiFive FU740 PCIe" > + depends on CLK_SIFIVE_PRCI > + depends on RESET_SIFIVE > + depends on SIFIVE_GPIO > + help > + Say Y here if you want to enable PCIe controller support on > + FU740. > + > config PCIE_FSL > bool "FSL PowerPC PCIe support" > depends on DM_PCI > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile > index 5ed94bc..5400d59 100644 > --- a/drivers/pci/Makefile > +++ b/drivers/pci/Makefile > @@ -51,3 +51,4 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o > obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o > obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o > obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o > +obj-$(CONFIG_PCIE_SIFIVE_FU740) += pcie_sifive.o > diff --git a/drivers/pci/pcie_sifive.c b/drivers/pci/pcie_sifive.c > new file mode 100644 > index 000..ada6087 > --- /dev/null > +++ b/drivers/pci/pcie_sifive.c > @@ -0,0 +1,797 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * SiFive FU740 DesignWare PCIe Controller Should we call this driver a DesgnWare PCIe controller (e.g.: pcie_designware.c), instead of pcie_sifive.c? > + * > + * Copyright (C) 2020-2021 SiFive, Inc. > + * > + * Based in early part on the i.MX6 PCIe host controller shim which is: > + * > + * Copyright (C) 2013 Kosagi > + * http://www.kosagi.com > + * > + * Based on driver from author: Alan Mikhak > + */ > +#include "pcie_sifive.h" > +#include > +#include > + > +/* Host Bridge Identification */ > +#define DEVICE_NAME"SiFive FU740 PCIe Host Controller" > +#define VENDOR_ID 0x51fe > +#define DEVICE_ID 0x51fe > + [snip] Regards, Bin
Re: [PATCH 6/7] board: sifive: add HiFive Unmatched board support
On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote:
>
> Add dts, defconfig and board support for HiFive Unmatched.
>
> Signed-off-by: Green Wan
> ---
> arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi | 1489
>
> arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi| 40 +
> arch/riscv/dts/hifive-unmatched-a00.dts| 263
> board/sifive/fu540/Kconfig |1 -
> board/sifive/fu540/fu540.c |2 +-
It looks these fu540 changes are not needed?
> board/sifive/hifive_unmatched_fu740/Kconfig| 50 +
> board/sifive/hifive_unmatched_fu740/MAINTAINERS|9 +
> board/sifive/hifive_unmatched_fu740/Makefile |9 +
> .../hifive-unmatched-fu740.c | 24 +
> board/sifive/hifive_unmatched_fu740/spl.c | 85 ++
> configs/sifive_hifive_unmatched_fu740_defconfig| 57 +
> doc/board/sifive/fu540.rst | 19 +-
here?
> doc/board/sifive/hifive_unmatched_fu740.rst| 532 +++
> drivers/reset/Kconfig |2 +-
> include/configs/sifive-fu540.h |5 -
and here?
> include/configs/sifive-hifive-unmatched-fu740.h| 88 ++
> 16 files changed, 2656 insertions(+), 19 deletions(-)
> create mode 100644 arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
> create mode 100644 arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi
> create mode 100644 arch/riscv/dts/hifive-unmatched-a00.dts
> create mode 100644 board/sifive/hifive_unmatched_fu740/Kconfig
> create mode 100644 board/sifive/hifive_unmatched_fu740/MAINTAINERS
> create mode 100644 board/sifive/hifive_unmatched_fu740/Makefile
> create mode 100644
> board/sifive/hifive_unmatched_fu740/hifive-unmatched-fu740.c
> create mode 100644 board/sifive/hifive_unmatched_fu740/spl.c
> create mode 100644 configs/sifive_hifive_unmatched_fu740_defconfig
> create mode 100644 doc/board/sifive/hifive_unmatched_fu740.rst
> create mode 100644 include/configs/sifive-hifive-unmatched-fu740.h
>
> diff --git a/arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
> b/arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
> new file mode 100644
> index 000..4209491
> --- /dev/null
> +++ b/arch/riscv/dts/fu740-hifive-unmatched-a00-ddr.dtsi
[snip]
> diff --git a/arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi
> b/arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi
> new file mode 100644
> index 000..3dcd2b4
> --- /dev/null
> +++ b/arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2020-2021 SiFive, Inc
> + */
> +
> +#include "fu740-c000-u-boot.dtsi"
> +#include "fu740-hifive-unmatched-a00-ddr.dtsi"
> +
> +/ {
> + aliases {
> + spi0 = &spi0;
> + };
> +
> + memory@8000 {
> + u-boot,dm-spl;
> + };
> +
> + hfclk {
> + u-boot,dm-spl;
> + };
> +
> + rtcclk {
> + u-boot,dm-spl;
> + };
> +
> +};
> +
> +&clint {
> + clocks = <&rtcclk>;
> +};
> +
> +&spi0 {
> + mmc@0 {
> + u-boot,dm-spl;
> + };
> +};
> +
> +&gpio {
> + u-boot,dm-spl;
> +};
> diff --git a/arch/riscv/dts/hifive-unmatched-a00.dts
> b/arch/riscv/dts/hifive-unmatched-a00.dts
> new file mode 100644
> index 000..92410b4
> --- /dev/null
> +++ b/arch/riscv/dts/hifive-unmatched-a00.dts
> @@ -0,0 +1,263 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (c) 2019-2021 SiFive, Inc */
> +
> +#include "fu740-c000.dtsi"
> +#include
> +
> +/* Clock frequency (in Hz) of the PCB crystal for rtcclk */
> +#define RTCCLK_FREQ100
> +
> +/ {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + model = "SiFive HiFive Unmatched A00";
> + compatible = "sifive,hifive-unmatched-a00", "sifive,fu740-c000",
> +"sifive,fu740";
> +
> + chosen {
> + stdout-path = "serial0";
> + };
> +
> + cpus {
> + timebase-frequency = ;
> + };
> +
> + memory@8000 {
> + device_type = "memory";
> + reg = <0x0 0x8000 0x4 0x>;
> + };
> +
> + soc {
> + };
> +
> + hfclk: hfclk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <2600>;
> + clock-output-names = "hfclk";
> + };
> +
> + rtcclk: rtcclk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = ;
> + clock-output-names = "rtcclk";
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&uart1 {
> + status = "okay";
> +};
> +
> +
> +&qspi0 {
> + status = "okay";
> + flash@0 {
> + compatible = "issi,is25wp256", "jedec,spi-nor";
> +
Re: [PATCH 7/7] drivers: net: macb: add fu740 support
On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote:
>
> Add fu740 support to macb ethernet driver
>
> Signed-off-by: Green Wan
> ---
> drivers/net/macb.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 57ea45e..df65d82 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -592,7 +592,11 @@ static int macb_sifive_clk_init(struct udevice *dev,
> ulong rate)
> * and output clock on GMII output signal GTX_CLK
> * 1 = MII mode. Use MII input signal TX_CLK in TX logic
> */
> - writel(rate != 12500, gemgxl_regs);
> + if (device_is_compatible(dev, "sifive,fu540-c000-gem"))
> + writel(rate != 12500, gemgxl_regs);
> + else if (device_is_compatible(dev, "sifive,fu740-c000-gem"))
> + writel(rate != 125125000, gemgxl_regs);
125125000 seems to be an odd value for 1000 Mbps ethernet. Can you
please explain this a little bit?
> +
> return 0;
> }
>
> @@ -1507,6 +1511,8 @@ static const struct udevice_id macb_eth_ids[] = {
> { .compatible = "cdns,zynq-gem" },
> { .compatible = "sifive,fu540-c000-gem",
> .data = (ulong)&sifive_config },
> + { .compatible = "sifive,fu740-c000-gem",
> + .data = (ulong)&sifive_config },
> { .compatible = "microchip,mpfs-mss-gem",
> .data = (ulong)µchip_config },
> { }
>
Regards,
Bin
Re: [PATCH 5/7] drivers: pci: add pcie support for fu740
Bin Meng 於 2021年3月11日 週四，下午10:14寫道： > On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote: > > > > Add pcie driver for SiFive fu740, the driver depends on > > fu740 gpio, clk and reset driver to do init. Force running at Gen1 > > for better capatible enumeration. > > > > Several devices are tested: > > a) M.2 NVMe SSD > > b) USB-to-PCI adapter > > c) Ethernet adapter (E1000 compatible) > > > > Signed-off-by: Green Wan > > --- > > drivers/pci/Kconfig | 9 + > > drivers/pci/Makefile | 1 + > > drivers/pci/pcie_sifive.c | 797 > ++ > > drivers/pci/pcie_sifive.h | 374 ++ > > 4 files changed, 1181 insertions(+) > > create mode 100644 drivers/pci/pcie_sifive.c > > create mode 100644 drivers/pci/pcie_sifive.h > > > > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig > > index ba41787..b078e76 100644 > > --- a/drivers/pci/Kconfig > > +++ b/drivers/pci/Kconfig > > @@ -97,6 +97,15 @@ config PCIE_DW_MVEBU > > Armada-8K SoCs. The PCIe controller on Armada-8K is based on > > DesignWare hardware. > > > > +config PCIE_SIFIVE_FU740 > > + bool "Enable SiFive FU740 PCIe" > > + depends on CLK_SIFIVE_PRCI > > + depends on RESET_SIFIVE > > + depends on SIFIVE_GPIO > > + help > > + Say Y here if you want to enable PCIe controller support on > > + FU740. > > + > > config PCIE_FSL > > bool "FSL PowerPC PCIe support" > > depends on DM_PCI > > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile > > index 5ed94bc..5400d59 100644 > > --- a/drivers/pci/Makefile > > +++ b/drivers/pci/Makefile > > @@ -51,3 +51,4 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o > > obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o > > obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o > > obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o > > +obj-$(CONFIG_PCIE_SIFIVE_FU740) += pcie_sifive.o > > diff --git a/drivers/pci/pcie_sifive.c b/drivers/pci/pcie_sifive.c > > new file mode 100644 > > index 000..ada6087 > > --- /dev/null > > +++ b/drivers/pci/pcie_sifive.c > > @@ -0,0 +1,797 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * SiFive FU740 DesignWare PCIe Controller > > Should we call this driver a DesgnWare PCIe controller (e.g.: > pcie_designware.c), instead of pcie_sifive.c? > The driver currently depends on fu740 drivers such as prci, clk, gpio and reset. probably rename it to pcie_dw_sifive.c like other designware based driver. What do we think of it? > > + * > > + * Copyright (C) 2020-2021 SiFive, Inc. > > + * > > + * Based in early part on the i.MX6 PCIe host controller shim which is: > > + * > > + * Copyright (C) 2013 Kosagi > > + * http://www.kosagi.com > > + * > > + * Based on driver from author: Alan Mikhak > > > + */ > > +#include "pcie_sifive.h" > > +#include > > +#include > > + > > +/* Host Bridge Identification */ > > +#define DEVICE_NAME"SiFive FU740 PCIe Host Controller" > > +#define VENDOR_ID 0x51fe > > +#define DEVICE_ID 0x51fe > > + > > [snip] > > Regards, > Bin >
Re: [PULL] u-boot-usb/master
On Wed, Mar 10, 2021 at 12:58:47PM +0100, Marek Vasut wrote: > The following changes since commit b764bf21e60901133fd5eec6a003409c581dd519: > > Merge https://source.denx.de/u-boot/custodians/u-boot-x86 (2021-03-09 > 07:21:51 -0500) > > are available in the Git repository at: > > git://source.denx.de/u-boot-usb.git master > > for you to fetch changes up to 044d7003a7118ad4633e619a81d634cd4c35e670: > > usb: mtu3: flush cache for next GPD (2021-03-10 11:48:09 +0100) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 5/7] drivers: pci: add pcie support for fu740
On Thu, Mar 11, 2021 at 10:25 PM Green Wan wrote: > > > > Bin Meng 於 2021年3月11日 週四，下午10:14寫道： >> >> On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote: >> > >> > Add pcie driver for SiFive fu740, the driver depends on >> > fu740 gpio, clk and reset driver to do init. Force running at Gen1 >> > for better capatible enumeration. >> > >> > Several devices are tested: >> > a) M.2 NVMe SSD >> > b) USB-to-PCI adapter >> > c) Ethernet adapter (E1000 compatible) >> > >> > Signed-off-by: Green Wan >> > --- >> > drivers/pci/Kconfig | 9 + >> > drivers/pci/Makefile | 1 + >> > drivers/pci/pcie_sifive.c | 797 >> > ++ >> > drivers/pci/pcie_sifive.h | 374 ++ >> > 4 files changed, 1181 insertions(+) >> > create mode 100644 drivers/pci/pcie_sifive.c >> > create mode 100644 drivers/pci/pcie_sifive.h >> > >> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig >> > index ba41787..b078e76 100644 >> > --- a/drivers/pci/Kconfig >> > +++ b/drivers/pci/Kconfig >> > @@ -97,6 +97,15 @@ config PCIE_DW_MVEBU >> > Armada-8K SoCs. The PCIe controller on Armada-8K is based on >> > DesignWare hardware. >> > >> > +config PCIE_SIFIVE_FU740 >> > + bool "Enable SiFive FU740 PCIe" >> > + depends on CLK_SIFIVE_PRCI >> > + depends on RESET_SIFIVE >> > + depends on SIFIVE_GPIO >> > + help >> > + Say Y here if you want to enable PCIe controller support on >> > + FU740. >> > + >> > config PCIE_FSL >> > bool "FSL PowerPC PCIe support" >> > depends on DM_PCI >> > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile >> > index 5ed94bc..5400d59 100644 >> > --- a/drivers/pci/Makefile >> > +++ b/drivers/pci/Makefile >> > @@ -51,3 +51,4 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o >> > obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o >> > obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o >> > obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o >> > +obj-$(CONFIG_PCIE_SIFIVE_FU740) += pcie_sifive.o >> > diff --git a/drivers/pci/pcie_sifive.c b/drivers/pci/pcie_sifive.c >> > new file mode 100644 >> > index 000..ada6087 >> > --- /dev/null >> > +++ b/drivers/pci/pcie_sifive.c >> > @@ -0,0 +1,797 @@ >> > +// SPDX-License-Identifier: GPL-2.0+ >> > +/* >> > + * SiFive FU740 DesignWare PCIe Controller >> >> Should we call this driver a DesgnWare PCIe controller (e.g.: >> pcie_designware.c), instead of pcie_sifive.c? > > > The driver currently depends on fu740 drivers such as prci, clk, gpio and > reset. probably rename it to pcie_dw_sifive.c like other designware based > driver. What do we think of it? Is it possible to update the existing dw pcie driver to support fu740? Depending on prci, clk and reset is not a problem. We can use dm_ APIs to hide device specific (prci, sifive-gpio) stuff. Unless SiFive made significant changes to the designware PCIe IP, we should not create a new driver for it. > > >> >> > + * >> > + * Copyright (C) 2020-2021 SiFive, Inc. >> > + * >> > + * Based in early part on the i.MX6 PCIe host controller shim which is: >> > + * >> > + * Copyright (C) 2013 Kosagi >> > + * http://www.kosagi.com >> > + * >> > + * Based on driver from author: Alan Mikhak >> > + */ >> > +#include "pcie_sifive.h" >> > +#include >> > +#include >> > + >> > +/* Host Bridge Identification */ >> > +#define DEVICE_NAME"SiFive FU740 PCIe Host Controller" >> > +#define VENDOR_ID 0x51fe >> > +#define DEVICE_ID 0x51fe >> > + >> >> [snip] Regards, Bin
Re: [PATCH 7/7] drivers: net: macb: add fu740 support
Bin Meng 於 2021年3月11日 週四，下午10:22寫道：
> On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote:
> >
> > Add fu740 support to macb ethernet driver
> >
> > Signed-off-by: Green Wan
> > ---
> > drivers/net/macb.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> > index 57ea45e..df65d82 100644
> > --- a/drivers/net/macb.c
> > +++ b/drivers/net/macb.c
> > @@ -592,7 +592,11 @@ static int macb_sifive_clk_init(struct udevice
> *dev, ulong rate)
> > * and output clock on GMII output signal GTX_CLK
> > * 1 = MII mode. Use MII input signal TX_CLK in TX logic
> > */
> > - writel(rate != 12500, gemgxl_regs);
> > + if (device_is_compatible(dev, "sifive,fu540-c000-gem"))
> > + writel(rate != 12500, gemgxl_regs);
> > + else if (device_is_compatible(dev, "sifive,fu740-c000-gem"))
> > + writel(rate != 125125000, gemgxl_regs);
>
> 125125000 seems to be an odd value for 1000 Mbps ethernet. Can you
> please explain this a little bit?
It's simply due to the limitation in the PLL cannot generate 125 000 000 Hz
clock precisely. The closet one is 125 125 000 HZ.
>
> > +
> > return 0;
> > }
> >
> > @@ -1507,6 +1511,8 @@ static const struct udevice_id macb_eth_ids[] = {
> > { .compatible = "cdns,zynq-gem" },
> > { .compatible = "sifive,fu540-c000-gem",
> > .data = (ulong)&sifive_config },
> > + { .compatible = "sifive,fu740-c000-gem",
> > + .data = (ulong)&sifive_config },
> > { .compatible = "microchip,mpfs-mss-gem",
> > .data = (ulong)µchip_config },
> > { }
> >
>
> Regards,
> Bin
>
Re: [PATCH 7/7] drivers: net: macb: add fu740 support
On Thu, Mar 11, 2021 at 10:42 PM Green Wan wrote: > > > > Bin Meng 於 2021年3月11日 週四，下午10:22寫道： >> >> On Thu, Mar 11, 2021 at 9:50 PM Green Wan wrote: >> > >> > Add fu740 support to macb ethernet driver >> > >> > Signed-off-by: Green Wan >> > --- >> > drivers/net/macb.c | 8 +++- >> > 1 file changed, 7 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/net/macb.c b/drivers/net/macb.c >> > index 57ea45e..df65d82 100644 >> > --- a/drivers/net/macb.c >> > +++ b/drivers/net/macb.c >> > @@ -592,7 +592,11 @@ static int macb_sifive_clk_init(struct udevice *dev, >> > ulong rate) >> > * and output clock on GMII output signal GTX_CLK >> > * 1 = MII mode. Use MII input signal TX_CLK in TX logic >> > */ >> > - writel(rate != 12500, gemgxl_regs); >> > + if (device_is_compatible(dev, "sifive,fu540-c000-gem")) >> > + writel(rate != 12500, gemgxl_regs); >> > + else if (device_is_compatible(dev, "sifive,fu740-c000-gem")) >> > + writel(rate != 125125000, gemgxl_regs); >> >> 125125000 seems to be an odd value for 1000 Mbps ethernet. Can you >> please explain this a little bit? > > > It's simply due to the limitation in the PLL cannot generate 125 000 000 Hz > clock precisely. The closet one is 125 125 000 HZ. So FU540 can but FU740 cannot? I would consider this as a hardware regression ... Regards, Bin
Re: [PATCH] board: stm32mp1: use CONFIG_SYS_MMC_ENV_DEV when available
Hi Patrick
On 3/1/21 1:17 PM, Patrick Delaunay wrote:
> Check whether user has explicitly defined the mmc device to use
> in mmc_get_env_dev() with CONFIG_SYS_MMC_ENV_DEV.
>
> On STMicroelectronics boards the used mmc device for environment is
> the instance of boot device provided by the ROM code; the mmc instance
> is configured by alias in device tree. The used partition is defined in
> device tree with u-boot,mmc-env-partition = "ssbl".
>
> This patch allows to override this selection for the support of customer
> boards without alias; for example when SDMMC1 is not used and ENV in
> mmc0=SDMMC2, user can force the value: CONFIG_SYS_MMC_ENV_DEV = 0.
>
> On STMicroelectronics boards, the current behavior is kept with
> CONFIG_SYS_MMC_ENV_DEV = -1.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> board/st/stm32mp1/stm32mp1.c| 8 +++-
> configs/stm32mp15_basic_defconfig | 1 +
> configs/stm32mp15_trusted_defconfig | 1 +
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index ab85d8ba68..759181fb5d 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -862,8 +862,14 @@ const char *env_ext4_get_dev_part(void)
>
> int mmc_get_env_dev(void)
> {
> - u32 bootmode = get_bootmode();
> + u32 bootmode;
> +
> + if (CONFIG_SYS_MMC_ENV_DEV >= 0)
> + return CONFIG_SYS_MMC_ENV_DEV;
> +
> + bootmode = get_bootmode();
>
> + /* use boot instance to select the correct mmc device identifier */
> return (bootmode & TAMP_BOOT_INSTANCE_MASK) - 1;
> }
>
> diff --git a/configs/stm32mp15_basic_defconfig
> b/configs/stm32mp15_basic_defconfig
> index 1c680984b8..a2f745099e 100644
> --- a/configs/stm32mp15_basic_defconfig
> +++ b/configs/stm32mp15_basic_defconfig
> @@ -71,6 +71,7 @@ CONFIG_ENV_UBI_PART="UBI"
> CONFIG_ENV_UBI_VOLUME="uboot_config"
> CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r"
> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> +CONFIG_SYS_MMC_ENV_DEV=-1
> # CONFIG_SPL_ENV_IS_NOWHERE is not set
> # CONFIG_SPL_ENV_IS_IN_SPI_FLASH is not set
> CONFIG_STM32_ADC=y
> diff --git a/configs/stm32mp15_trusted_defconfig
> b/configs/stm32mp15_trusted_defconfig
> index 107041119f..f41aaa750f 100644
> --- a/configs/stm32mp15_trusted_defconfig
> +++ b/configs/stm32mp15_trusted_defconfig
> @@ -52,6 +52,7 @@ CONFIG_ENV_UBI_PART="UBI"
> CONFIG_ENV_UBI_VOLUME="uboot_config"
> CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r"
> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> +CONFIG_SYS_MMC_ENV_DEV=-1
> CONFIG_STM32_ADC=y
> CONFIG_SET_DFU_ALT_INFO=y
> CONFIG_USB_FUNCTION_FASTBOOT=y
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
[PATCH v1] imx: bootaux fix elf loading
This reverts the arch/arm/mach-imx/imx_bootaux.c changes of commit
805b3cac1e0c. The loader function name was changed so that it does
not clash with the generically available function in lib/elf.c.
imx-bootaux loads an elf file linked for an auxilary core. Thus the
loader function requires address translation from the auxilary core's
address space to where those are mapped into U-Boot's address space.
So the elf loader is specific and must not be replaced with a generic
loader which doesn't provide the address translation functionality.
Fixes commit 805b3cac1e0c ("lib: elf: Move the generic elf
loading/validating functions to lib")
Signed-off-by: Max Krummenacher
---
arch/arm/mach-imx/imx_bootaux.c | 67 -
1 file changed, 66 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/imx_bootaux.c b/arch/arm/mach-imx/imx_bootaux.c
index e1fc4b3e19a..30fb45d48c2 100644
--- a/arch/arm/mach-imx/imx_bootaux.c
+++ b/arch/arm/mach-imx/imx_bootaux.c
@@ -14,6 +14,71 @@
#include
#include
+#ifndef CONFIG_IMX8M
+const __weak struct rproc_att hostmap[] = { };
+
+static const struct rproc_att *get_host_mapping(unsigned long auxcore)
+{
+ const struct rproc_att *mmap = hostmap;
+
+ while (mmap && mmap->size) {
+ if (mmap->da <= auxcore &&
+ mmap->da + mmap->size > auxcore)
+ return mmap;
+ mmap++;
+ }
+
+ return NULL;
+}
+
+/*
+ * A very simple elf loader for the auxilary core, assumes the image
+ * is valid, returns the entry point address.
+ * Translates load addresses in the elf file to the U-Boot address space.
+ */
+static unsigned long load_elf_image_m_core_phdr(unsigned long addr)
+{
+ Elf32_Ehdr *ehdr; /* ELF header structure pointer */
+ Elf32_Phdr *phdr; /* Program header structure pointer */
+ int i;
+
+ ehdr = (Elf32_Ehdr *)addr;
+ phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
+
+ /* Load each program header */
+ for (i = 0; i < ehdr->e_phnum; ++i, ++phdr) {
+ const struct rproc_att *mmap = get_host_mapping(phdr->p_paddr);
+ void *dst, *src;
+
+ if (phdr->p_type != PT_LOAD)
+ continue;
+
+ if (!mmap) {
+ printf("Invalid aux core address: %08x",
+ phdr->p_paddr);
+ return 0;
+ }
+
+ dst = (void *)(phdr->p_paddr - mmap->da) + mmap->sa;
+ src = (void *)addr + phdr->p_offset;
+
+ debug("Loading phdr %i to 0x%p (%i bytes)\n",
+ i, dst, phdr->p_filesz);
+
+ if (phdr->p_filesz)
+ memcpy(dst, src, phdr->p_filesz);
+ if (phdr->p_filesz != phdr->p_memsz)
+ memset(dst + phdr->p_filesz, 0x00,
+ phdr->p_memsz - phdr->p_filesz);
+ flush_cache((unsigned long)dst &
+ ~(CONFIG_SYS_CACHELINE_SIZE - 1),
+ ALIGN(phdr->p_filesz, CONFIG_SYS_CACHELINE_SIZE));
+ }
+
+ return ehdr->e_entry;
+}
+#endif
+
int arch_auxiliary_core_up(u32 core_id, ulong addr)
{
ulong stack, pc;
@@ -31,7 +96,7 @@ int arch_auxiliary_core_up(u32 core_id, ulong addr)
*/
if (valid_elf_image(addr)) {
stack = 0x0;
- pc = load_elf_image_phdr(addr);
+ pc = load_elf_image_m_core_phdr(addr);
if (!pc)
return CMD_RET_FAILURE;
--
2.20.1
[PATCH u-boot-marvell RESEND] ddr: marvell: a38x: Add more space for additional info from SPD
From: Sujeet Baranwal
commit 258be123226f8f5cd516b7813fe201fb7d7416e9 upstream.
At this moment, only page 0 of SPD is being read but to support
smbios, we need to read page 1 also which has more info. In order
to do that, we need to allocate more space.
Signed-off-by: Sujeet Baranwal
Tested-by: sa_ip-sw-jenkins
Reviewed-by: Sujeet Kumar Baranwal
Reviewed-by: Nadav Haklai
Signed-off-by: Marek Behún
Tested-by: Chris Packham
---
Stefan, last time I overlooked this commit. This is also part of
upstream mv-ddr-marvell.
---
drivers/ddr/marvell/a38x/mv_ddr_spd.h | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/ddr/marvell/a38x/mv_ddr_spd.h
b/drivers/ddr/marvell/a38x/mv_ddr_spd.h
index b4bfef3103..6043f11b28 100644
--- a/drivers/ddr/marvell/a38x/mv_ddr_spd.h
+++ b/drivers/ddr/marvell/a38x/mv_ddr_spd.h
@@ -40,7 +40,10 @@
*/
union mv_ddr_spd_data {
unsigned char all_bytes[MV_DDR_SPD_DATA_BLOCK0_SIZE +
- MV_DDR_SPD_DATA_BLOCK1M_SIZE];
+ MV_DDR_SPD_DATA_BLOCK1M_SIZE +
+ MV_DDR_SPD_DATA_BLOCK1H_SIZE +
+ MV_DDR_SPD_DATA_BLOCK2E_SIZE +
+ MV_DDR_SPD_DATA_BLOCK2M_SIZE];
struct {
/* block 0 */
union { /* num of bytes used/num of bytes in spd device/crc
coverage */
@@ -271,6 +274,9 @@ union mv_ddr_spd_data {
} bit_fields;
} byte_131;
unsigned char bytes_132_191[60]; /* reserved; all 0s */
+ unsigned char bytes_192_255[MV_DDR_SPD_DATA_BLOCK1H_SIZE];
+ unsigned char bytes_256_319[MV_DDR_SPD_DATA_BLOCK2E_SIZE];
+ unsigned char bytes_320_383[MV_DDR_SPD_DATA_BLOCK2M_SIZE];
} byte_fields;
};
--
2.26.2
Re: [PATCH u-boot 18/39] build: LTO: move platform libs into --start-group list
On Tue, 9 Mar 2021 23:24:01 +0800 Bin Meng wrote: > On Sun, Mar 7, 2021 at 12:26 PM Marek Behún wrote: > > > > When building with LTO, move $(PLATFORM_LIBS) into the --start-group / > > --end-group list. > > This should be --whole-archive > > > Otherwise some functions declared in assembly may not be resolved and > > linking may fail. > > I am not sure if this is caused by we build these lib functions with -flto? It is caused by using thin archives, not LTO
[PATCH 0/4] Updates for Toradex boards
This patchset includes fixes and improvements for Toradex boards. Igor Opaniuk (1): colibri_imx6: adjust boot order Oleksandr Suvorov (2): apalis/colibri_imx6: remove video= settings board: toradex: apalis-imx8x: fix build instructions Philippe Schenker (1): colibri_imx6ull/imx7: add missing tdxargs variable doc/board/toradex/apalix-imx8x.rst | 33 -- include/configs/apalis_imx6.h | 5 + include/configs/colibri-imx6ull.h | 2 +- include/configs/colibri_imx6.h | 4 +--- include/configs/colibri_imx7.h | 2 +- 5 files changed, 22 insertions(+), 24 deletions(-) -- 2.29.2
[PATCH 1/4] colibri_imx6: adjust boot order
From: Igor Opaniuk
Remove duplicate of mmc0, set this boot order:
1) SD
2) eMMC
3) USB
4) DHCP boot
Fixes: 0e15165bc4e0 ("colibri_imx6: boot env configuration updates")
Signed-off-by: Igor Opaniuk
Signed-off-by: Oleksandr Suvorov
---
include/configs/colibri_imx6.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/configs/colibri_imx6.h b/include/configs/colibri_imx6.h
index c014d6b2d5..5f5e201b4c 100644
--- a/include/configs/colibri_imx6.h
+++ b/include/configs/colibri_imx6.h
@@ -72,7 +72,6 @@
#ifndef CONFIG_SPL_BUILD
#define BOOT_TARGET_DEVICES(func) \
- func(MMC, mmc, 0) \
func(MMC, mmc, 1) \
func(MMC, mmc, 0) \
func(USB, usb, 0) \
--
2.29.2
[PATCH 2/4] colibri_imx6ull/imx7: add missing tdxargs variable
From: Philippe Schenker
All the other boards have tdxargs specified for setting manual kernel
command-line arguments. Add them also to NAND-based boards.
Signed-off-by: Philippe Schenker
Signed-off-by: Oleksandr Suvorov
---
include/configs/colibri-imx6ull.h | 2 +-
include/configs/colibri_imx7.h| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/colibri-imx6ull.h
b/include/configs/colibri-imx6ull.h
index 2827c171c9..22ee2ba03e 100644
--- a/include/configs/colibri-imx6ull.h
+++ b/include/configs/colibri-imx6ull.h
@@ -63,7 +63,7 @@
"ubi.fm_autoconvert=1\0" \
"ubiboot=run setup; " \
"setenv bootargs ${defargs} ${ubiargs} " \
- "${setupargs} ${vidargs}; echo Booting from NAND...; " \
+ "${setupargs} ${vidargs} ${tdxargs}; echo Booting from NAND...;
" \
"ubi part ubi &&" \
"ubi read ${kernel_addr_r} kernel && " \
"ubi read ${fdt_addr_r} dtb && " \
diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx7.h
index 85dd891055..2fffaa39c0 100644
--- a/include/configs/colibri_imx7.h
+++ b/include/configs/colibri_imx7.h
@@ -110,7 +110,7 @@
"ubi.fm_autoconvert=1\0" \
"ubiboot=run setup; " \
"setenv bootargs ${defargs} ${ubiargs} " \
- "${setupargs} ${vidargs}; echo Booting from NAND...; " \
+ "${setupargs} ${vidargs} ${tdxargs}; echo Booting from NAND...;
" \
"ubi part ubi && run m4boot && " \
"ubi read ${kernel_addr_r} kernel && " \
"ubi read ${fdt_addr_r} dtb && " \
--
2.29.2
[PATCH 3/4] apalis/colibri_imx6: remove video= settings
Since Toradex provides the full set of overlays for Linux kernel
for display interfaces for both Apalis iMX6Q and Colibri iMX6DL
modules, the video= settings are obsolete. Remove them.
Signed-off-by: Oleksandr Suvorov
---
include/configs/apalis_imx6.h | 5 +
include/configs/colibri_imx6.h | 3 +--
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/include/configs/apalis_imx6.h b/include/configs/apalis_imx6.h
index 0e81ef94d3..12de0105c6 100644
--- a/include/configs/apalis_imx6.h
+++ b/include/configs/apalis_imx6.h
@@ -163,10 +163,7 @@
"source ${loadaddr}\0" \
"splashpos=m,m\0" \
"splashimage=" __stringify(CONFIG_LOADADDR) "\0" \
- "vidargs=mxc_hdmi.only_cea=1 " \
- "video=mxcfb0:dev=hdmi,1920x1080M@60,if=RGB24 " \
- "video=mxcfb1:off video=mxcfb2:off video=mxcfb3:off " \
- "fbmem=32M\0 "
+ "vidargs=mxc_hdmi.only_cea=1 fbmem=32M\0"
/* Miscellaneous configurable options */
#undef CONFIG_SYS_CBSIZE
diff --git a/include/configs/colibri_imx6.h b/include/configs/colibri_imx6.h
index 5f5e201b4c..804a144a03 100644
--- a/include/configs/colibri_imx6.h
+++ b/include/configs/colibri_imx6.h
@@ -143,8 +143,7 @@
"source ${loadaddr}\0" \
"splashpos=m,m\0" \
"splashimage=" __stringify(CONFIG_LOADADDR) "\0" \
- "vidargs=video=mxcfb0:dev=lcd,640x480M@60,if=RGB666 " \
- "video=mxcfb1:off fbmem=8M\0 "
+ "vidargs=fbmem=8M\0"
/* Miscellaneous configurable options */
#undef CONFIG_SYS_CBSIZE
--
2.29.2
[PATCH 4/4] board: toradex: apalis-imx8x: fix build instructions
Fix an URL for downloading the SCFW binary for an Apalis iMX8X and improve u-boot image build instructions. Signed-off-by: Oleksandr Suvorov --- doc/board/toradex/apalix-imx8x.rst | 33 -- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/doc/board/toradex/apalix-imx8x.rst b/doc/board/toradex/apalix-imx8x.rst index ce7dde8d00..efa7e0c72e 100644 --- a/doc/board/toradex/apalix-imx8x.rst +++ b/doc/board/toradex/apalix-imx8x.rst @@ -1,43 +1,46 @@ .. SPDX-License-Identifier: GPL-2.0+ -Apalis iMX8X V1.1A Module +Apalis iMX8X V1.1A Module (SoC NXP i.MX8QXP RevB) == Quick Start --- -- Build the ARM trusted firmware binary -- Get scfw_tcm.bin and ahab-container.img +- Get and Build the ARM trusted firmware +- Get System Controller firmware +- Get SECO container - Build U-Boot - Load U-Boot binary using uuu - Flash U-Boot binary into the eMMC - Boot +Note: builddir is U-Boot build directory (source directory for in-tree builds) + Get and Build the ARM Trusted Firmware -- .. code-block:: bash -$ git clone -b toradex_imx_5.4.24_2.1.0 http://git.toradex.com/cgit/imx-atf.git -$ cd imx-atf/ -$ make PLAT=imx8qx bl31 +$ cd $(builddir) +$ git clone -b toradex_imx_5.4.70_2.3.0 http://git.toradex.com/cgit/imx-atf.git +$ make PLAT=imx8qx bl31 -C imx-atf +$ cp imx-atf/build/imx8qx/release/bl31.bin $(builddir) -Get scfw_tcm.bin and ahab-container.img +Get System Controller firmware --- .. code-block:: bash -$ wget https://github.com/toradex/i.MX-System-Controller-Firmware/blob/master/src/scfw_export_mx8qx_b0/build_mx8qx_b0/mx8qx-apalis-scfw-tcm.bin -$ wget https://www.nxp.com/lgfiles/NMG/MAD/YOCTO/imx-seco-3.6.3.bin -$ chmod +x imx-seco-3.6.3.bin -$ ./imx-seco-3.6.3.bin +$ wget https://github.com/toradex/i.MX-System-Controller-Firmware/raw/master/src/scfw_export_mx8qx_b0/build_mx8qx_b0/mx8qx-apalis-scfw-tcm.bin -Copy the following binaries to the U-Boot folder: +Get SECO container +--- .. code-block:: bash -$ cp imx-atf/build/imx8qx/release/bl31.bin . -$ cp imx-seco-3.6.3/firmware/seco/mx8qxb0-ahab-container.img mx8qx-ahab-container.imx8_defconfig +$ wget https://www.nxp.com/lgfiles/NMG/MAD/YOCTO/imx-seco-3.7.4.bin +$ sh imx-seco-3.7.4.bin +$ cp imx-seco-3.7.4/firmware/seco/mx8qxb0-ahab-container.img $(builddir)/mx8qx-ahab-container.img Build U-Boot @@ -58,7 +61,7 @@ to your host and execute uuu: .. code-block:: bash -sudo ./uuu u-boot/u-boot-dtb.imx +sudo ./uuu $(builddir)/u-boot-dtb.imx Flash the U-Boot Binary into the eMMC - -- 2.29.2
Re: [PATCH v1] imx: bootaux fix elf loading
On Thu, Mar 11, 2021 at 7:18 PM Max Krummenacher wrote:
>
> This reverts the arch/arm/mach-imx/imx_bootaux.c changes of commit
> 805b3cac1e0c. The loader function name was changed so that it does
> not clash with the generically available function in lib/elf.c.
>
> imx-bootaux loads an elf file linked for an auxilary core. Thus the
> loader function requires address translation from the auxilary core's
> address space to where those are mapped into U-Boot's address space.
>
> So the elf loader is specific and must not be replaced with a generic
> loader which doesn't provide the address translation functionality.
>
> Fixes commit 805b3cac1e0c ("lib: elf: Move the generic elf
> loading/validating functions to lib")
>
> Signed-off-by: Max Krummenacher
Acked-by: Oleksandr Suvorov
>
> ---
>
> arch/arm/mach-imx/imx_bootaux.c | 67 -
> 1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-imx/imx_bootaux.c b/arch/arm/mach-imx/imx_bootaux.c
> index e1fc4b3e19a..30fb45d48c2 100644
> --- a/arch/arm/mach-imx/imx_bootaux.c
> +++ b/arch/arm/mach-imx/imx_bootaux.c
> @@ -14,6 +14,71 @@
> #include
> #include
>
> +#ifndef CONFIG_IMX8M
> +const __weak struct rproc_att hostmap[] = { };
> +
> +static const struct rproc_att *get_host_mapping(unsigned long auxcore)
> +{
> + const struct rproc_att *mmap = hostmap;
> +
> + while (mmap && mmap->size) {
> + if (mmap->da <= auxcore &&
> + mmap->da + mmap->size > auxcore)
> + return mmap;
> + mmap++;
> + }
> +
> + return NULL;
> +}
> +
> +/*
> + * A very simple elf loader for the auxilary core, assumes the image
> + * is valid, returns the entry point address.
> + * Translates load addresses in the elf file to the U-Boot address space.
> + */
> +static unsigned long load_elf_image_m_core_phdr(unsigned long addr)
> +{
> + Elf32_Ehdr *ehdr; /* ELF header structure pointer */
> + Elf32_Phdr *phdr; /* Program header structure pointer */
> + int i;
> +
> + ehdr = (Elf32_Ehdr *)addr;
> + phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
> +
> + /* Load each program header */
> + for (i = 0; i < ehdr->e_phnum; ++i, ++phdr) {
> + const struct rproc_att *mmap =
> get_host_mapping(phdr->p_paddr);
> + void *dst, *src;
> +
> + if (phdr->p_type != PT_LOAD)
> + continue;
> +
> + if (!mmap) {
> + printf("Invalid aux core address: %08x",
> + phdr->p_paddr);
> + return 0;
> + }
> +
> + dst = (void *)(phdr->p_paddr - mmap->da) + mmap->sa;
> + src = (void *)addr + phdr->p_offset;
> +
> + debug("Loading phdr %i to 0x%p (%i bytes)\n",
> + i, dst, phdr->p_filesz);
> +
> + if (phdr->p_filesz)
> + memcpy(dst, src, phdr->p_filesz);
> + if (phdr->p_filesz != phdr->p_memsz)
> + memset(dst + phdr->p_filesz, 0x00,
> + phdr->p_memsz - phdr->p_filesz);
> + flush_cache((unsigned long)dst &
> + ~(CONFIG_SYS_CACHELINE_SIZE - 1),
> + ALIGN(phdr->p_filesz, CONFIG_SYS_CACHELINE_SIZE));
> + }
> +
> + return ehdr->e_entry;
> +}
> +#endif
> +
> int arch_auxiliary_core_up(u32 core_id, ulong addr)
> {
> ulong stack, pc;
> @@ -31,7 +96,7 @@ int arch_auxiliary_core_up(u32 core_id, ulong addr)
> */
> if (valid_elf_image(addr)) {
> stack = 0x0;
> - pc = load_elf_image_phdr(addr);
> + pc = load_elf_image_m_core_phdr(addr);
> if (!pc)
> return CMD_RET_FAILURE;
>
> --
> 2.20.1
>
--
Best regards
Oleksandr Suvorov
cryo...@gmail.com
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On 3/11/21 2:31 PM, Ilias Apalodimas wrote: I am not referring to efidebug. The user can update EFI variables with any binary content using an EFI binary or OS functions. E.g. copy a binary file to the efi variables file system in Linux. Right, I was mostly thinking SetVariable which still goes through u-boot. Copying an entire file is possiible though I guess dd if=foo of=/sys/firmware/efi/efivars/Boot0123-8be4df61-93ca-11d2-aa0d-00e098032b8c But Linux makes some consistency checks. Best regards Heinrich
[PATCH 1/6] spl: LOAD_FIT_FULL: Fix selection of the "fdt" node
The correct FDT to use is described by the "fdt" property of the
configuration node. When the fit_unamep argument to fit_image_load()
is "fdt", we get the "/images/fdt" node. This is incorrect, as it
ignores the "fdt" property of the config node, and in most cases,
the "/images/fdt" node doesn't exist.
When the 'fit_unamep' argument is NULL, fit_image_load() uses the
IH_TYPE_FLATDT value to read the config property "fdt", which points
to the correct FDT node(s).
fit_image_load() should probably be split into a function that reads
an image by name, and one that reads an image by config reference. I
don't make those decisions, I just point out the craziness.
Signed-off-by: Alexandru Gagniuc
---
common/spl/spl.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c
index e3d84082f4..986cfbc6fd 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -201,7 +201,6 @@ static int spl_load_fit_image(struct spl_image_info
*spl_image,
{
bootm_headers_t images;
const char *fit_uname_config = NULL;
- const char *fit_uname_fdt = FIT_FDT_PROP;
const char *uname;
ulong fw_data = 0, dt_data = 0, img_data = 0;
ulong fw_len = 0, dt_len = 0, img_len = 0;
@@ -230,8 +229,7 @@ static int spl_load_fit_image(struct spl_image_info
*spl_image,
#ifdef CONFIG_SPL_FIT_SIGNATURE
images.verify = 1;
#endif
- ret = fit_image_load(&images, (ulong)header,
- &fit_uname_fdt, &fit_uname_config,
+ ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config,
IH_ARCH_DEFAULT, IH_TYPE_FLATDT, -1,
FIT_LOAD_OPTIONAL, &dt_data, &dt_len);
if (ret >= 0)
--
2.26.2
[PATCH 0/6] SPL: FIT: Fix some omissions of SPL_LOAD_FIT_FULL and bootm
Although this series focuses on SPL_LOAD_FIT_FULL, some of the fixes will also apply to bootm, as they are sharing significant amounts of code. Originally SPL_LOAD_FIT_FULL could not start either a linux FIT or a u-boot image. It didn't even take FIT images generated automatically by mkimage, as part of the u-boot build!!! The goal is to at the very least, be able to boot autogenerated mkimage FITs. This brings us much more in line with SPL_LOAD_FIT, and the documentation. It's not perfect, and the fpga 'compatible =' support is still not implemented. That's all I currently have time for before someone notices I'm working on u-boot again. Alexandru Gagniuc (6): spl: LOAD_FIT_FULL: Fix selection of the "fdt" node spl: LOAD_FIT_FULL: Do not hard-code os to IH_OS_U_BOOT spl: LOAD_FIT_FULL: Relocate FDT for u-boot payloads spl: LOAD_FIT_FULL: Support 'kernel' and 'firmware' properties image-fit: Accept IH_TYPE_FIRMWARE in fit_image_load() as valid image-fit: Accept OP-TEE images when booting a FIT common/image-fit.c | 4 common/spl/spl.c | 40 +--- 2 files changed, 37 insertions(+), 7 deletions(-) -- 2.26.2
[PATCH 2/6] spl: LOAD_FIT_FULL: Do not hard-code os to IH_OS_U_BOOT
The information on the OS should be contained in the FIT, as the self-explanatory "os" property of a node under /images. Hard-coding this to U_BOOT might send us down the wrong path later in the boot process. Signed-off-by: Alexandru Gagniuc --- common/spl/spl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/spl/spl.c b/common/spl/spl.c index 986cfbc6fd..8f6c8dba6f 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -220,8 +220,9 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, spl_image->size = fw_len; spl_image->entry_point = fw_data; spl_image->load_addr = fw_data; - spl_image->os = IH_OS_U_BOOT; - spl_image->name = "U-Boot"; + if (!fit_image_get_os(header, ret, &spl_image->os)) + spl_image->os = IH_OS_INVALID; + spl_image->name = genimg_get_os_name(spl_image->os); debug(SPL_TPL_PROMPT "payload image: %32s load addr: 0x%lx size: %d\n", spl_image->name, spl_image->load_addr, spl_image->size); -- 2.26.2
[PATCH 6/6] image-fit: Accept OP-TEE images when booting a FIT
OP-TEE images are normally packaged with type = "tee; os = "tee"; However, fit_image_load() thinks that is somehow invalid. however if they were declared as type = "kernel", os = "linux", fit_image_load() would happily accept them and allow the boot to continue. There is no technical limitation to excluding "tee". Allowing "tee" images is useful in a boot flow where OP-TEE is executed before linux. In fact, I think it's unintuitive for a "load"ing function to also do parsing and contain a bunch ad-hoc heuristics that only its caller might know. But I don't make the rules, I just write fixes. In more polite terms: refactoring the fit_image API is beyond the scope of this change. Signed-off-by: Alexandru Gagniuc --- common/image-fit.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/image-fit.c b/common/image-fit.c index 8cd1621a18..31cb73a870 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -2089,6 +2089,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL); type_ok = fit_image_check_type(fit, noffset, image_type) || fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) || + fit_image_check_type(fit, noffset, IH_TYPE_TEE) || (image_type == IH_TYPE_KERNEL && fit_image_check_type(fit, noffset, IH_TYPE_KERNEL_NOLOAD)); @@ -2096,6 +2097,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, image_type == IH_TYPE_FPGA || fit_image_check_os(fit, noffset, IH_OS_LINUX) || fit_image_check_os(fit, noffset, IH_OS_U_BOOT) || + fit_image_check_os(fit, noffset, IH_OS_TEE) || fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) || fit_image_check_os(fit, noffset, IH_OS_EFI) || fit_image_check_os(fit, noffset, IH_OS_VXWORKS); -- 2.26.2
[PATCH 4/6] spl: LOAD_FIT_FULL: Support 'kernel' and 'firmware' properties
The 'firmware' property of a config node takes precedence over the
'kernel' property. 'standalone' is deprecated. However, give users a
couple of releases where 'standalone' still works, but warns loudly.
Signed-off-by: Alexandru Gagniuc
---
common/spl/spl.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c
index e63f05bb33..da4751b4ac 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -214,7 +214,24 @@ static int spl_load_fit_image(struct spl_image_info
*spl_image,
ret = fit_image_load(&images, (ulong)header,
NULL, &fit_uname_config,
IH_ARCH_DEFAULT, IH_TYPE_STANDALONE, -1,
-FIT_LOAD_REQUIRED, &fw_data, &fw_len);
+FIT_LOAD_OPTIONAL, &fw_data, &fw_len);
+ if (ret >= 0) {
+ printf("DEPRECATED: 'standalone = ' property.");
+ printf("Please use either 'firmware =' or 'kernel ='\n");
+ } else {
+ ret = fit_image_load(&images, (ulong)header, NULL,
+&fit_uname_config, IH_ARCH_DEFAULT,
+IH_TYPE_FIRMWARE, -1, FIT_LOAD_OPTIONAL,
+&fw_data, &fw_len);
+ }
+
+ if (ret < 0) {
+ ret = fit_image_load(&images, (ulong)header, NULL,
+&fit_uname_config, IH_ARCH_DEFAULT,
+IH_TYPE_KERNEL, -1, FIT_LOAD_OPTIONAL,
+&fw_data, &fw_len);
+ }
+
if (ret < 0)
return ret;
--
2.26.2
[PATCH 3/6] spl: LOAD_FIT_FULL: Relocate FDT for u-boot payloads
U-Boot expects the FDT to be located right after the _end
linker symbol (see fdtdec.c: board_fdt_blob_setup())
The "basic" LOAD_FIT path is aware of this limitation, and relocates
the FDT at the expected location. Guessing the expected location
probably only works reliably on 32-bit arm, and it feels like a hack.
One proposal would be to pass the FDT address to u-boot
(e.g. using 'r2' on arm platforms).
The variable is named "fdt_hack" to remind future contributors that,
"hey! we should fix the underlying problem". However, that is beyond
the scope of this patch.
Signed-off-by: Alexandru Gagniuc
---
common/spl/spl.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 8f6c8dba6f..e63f05bb33 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -201,6 +201,7 @@ static int spl_load_fit_image(struct spl_image_info
*spl_image,
{
bootm_headers_t images;
const char *fit_uname_config = NULL;
+ uintptr_t fdt_hack;
const char *uname;
ulong fw_data = 0, dt_data = 0, img_data = 0;
ulong fw_len = 0, dt_len = 0, img_len = 0;
@@ -233,9 +234,18 @@ static int spl_load_fit_image(struct spl_image_info
*spl_image,
ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config,
IH_ARCH_DEFAULT, IH_TYPE_FLATDT, -1,
FIT_LOAD_OPTIONAL, &dt_data, &dt_len);
- if (ret >= 0)
+ if (ret >= 0) {
spl_image->fdt_addr = (void *)dt_data;
+ if (spl_image->os == IH_OS_U_BOOT) {
+ /* HACK: U-boot expects FDT at a specific address */
+ fdt_hack = spl_image->load_addr + spl_image->size;
+ fdt_hack = (fdt_hack + 3) & ~3;
+ debug("Relocating FDT to %p\n", spl_image->fdt_addr);
+ memcpy((void *)fdt_hack, spl_image->fdt_addr, dt_len);
+ }
+ }
+
conf_noffset = fit_conf_get_node((const void *)header,
fit_uname_config);
if (conf_noffset <= 0)
--
2.26.2
[PATCH 5/6] image-fit: Accept IH_TYPE_FIRMWARE in fit_image_load() as valid
Consider the following FIT:
images {
whipple {};
};
configurations {
conf-1 {
firmware = "whipple";
};
};
Getting the 'firmware' image with fit_image_load() is not possible, as
it doesn't understand 'firmware =' properties. Although one could pass
IH_TYPE_FIRMWARE for 'image_type', this needs to be converted to a
"firmware" string for FDT lookup -- exactly what this change does.
Signed-off-by: Alexandru Gagniuc
---
common/image-fit.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/common/image-fit.c b/common/image-fit.c
index 28b3d2b191..8cd1621a18 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1955,6 +1955,8 @@ static const char *fit_get_image_type_property(int type)
return FIT_FDT_PROP;
case IH_TYPE_KERNEL:
return FIT_KERNEL_PROP;
+ case IH_TYPE_FIRMWARE:
+ return FIT_FIRMWARE_PROP;
case IH_TYPE_RAMDISK:
return FIT_RAMDISK_PROP;
case IH_TYPE_X86_SETUP:
--
2.26.2
[PATCH] net: gem: Fix setting PCS auto-negotiation state
The code was trying to disable PCS auto-negotiation when a fixed-link node
is present and enable it otherwise. However, the PCS registers were being
written before the PCSSEL bit was set in the network configuration
register, and it appears that in this state, PCS register writes are
ignored. The result is that the intended change only took effect on the
second network operation that was performed, since at that time PCSSEL is
already enabled.
Fix the order of register writes so that PCS registers are only written to
after the PCS is enabled.
Fixes: 26e62cc971 ("net: gem: Disable PCS autonegotiation in case of
fixed-link")
Signed-off-by: Robert Hancock
---
drivers/net/zynq_gem.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index baf06a2ad8..ff59982267 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -454,14 +454,6 @@ static int zynq_gem_init(struct udevice *dev)
priv->int_pcs) {
nwconfig |= ZYNQ_GEM_NWCFG_SGMII_ENBL |
ZYNQ_GEM_NWCFG_PCS_SEL;
-#ifdef CONFIG_ARM64
- if (priv->phydev->phy_id != PHY_FIXED_ID)
- writel(readl(®s->pcscntrl) | ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
- ®s->pcscntrl);
- else
- writel(readl(®s->pcscntrl) & ~ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
- ®s->pcscntrl);
-#endif
}
switch (priv->phydev->speed) {
@@ -480,6 +472,23 @@ static int zynq_gem_init(struct udevice *dev)
break;
}
+#ifdef CONFIG_ARM64
+ if (priv->interface == PHY_INTERFACE_MODE_SGMII &&
+ priv->int_pcs) {
+ /*
+* Disable AN for fixed link configuration, enable otherwise.
+* Must be written after PCS_SEL is set in nwconfig,
+* otherwise writes will not take effect.
+*/
+ if (priv->phydev->phy_id != PHY_FIXED_ID)
+ writel(readl(®s->pcscntrl) |
ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
+ ®s->pcscntrl);
+ else
+ writel(readl(®s->pcscntrl) &
~ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
+ ®s->pcscntrl);
+ }
+#endif
+
ret = clk_set_rate(&priv->tx_clk, clk_rate);
if (IS_ERR_VALUE(ret)) {
dev_err(dev, "failed to set tx clock rate\n");
--
2.27.0
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
Ilias,
I may have missed your past discussions, but any way,
On Thu, Mar 11, 2021 at 01:36:04PM +0200, Ilias Apalodimas wrote:
> Hi Heinrich
>
> [...]
>
> > >>> + * @load_option: device paths to search
> > >>> + * @size: size of the discovered device path
> > >>> + * @guid: guid to search for
> > >>> + *
> > >>> + * Return: device path or NULL. Caller must free the returned value
> > >>
> > >> Please, keep the text aligned.
> > >>
> > >> Do we need a copy? Isn't a pointer good enough?
> > >
> > > A pointer to what?
> > > I think it's better to a copy here. This is a device path that might be
> > > used
> > > out of a stack context were the load option might disappear.
> > > Look at how the function is used in efi_get_dp_from_boot().
> >
> > You are duplicating in get_initrd_fp(). Why should we duplicate twice?
> >
>
> That's irrelevant though isn't it?
> I did that in the efi initrd implementation. If someone else does the DTB in
> the future and device to use efi_dp_from_lo return directly?
> I'd much prefer an API (since that function goes into an API-related file for
> device paths), that's safe and requires the user to free the memory, rather
> than allowing him to accidentally shoot himself in the foot, keeping in mind
> it's a single copy on a device path, which roughly adds anything on our boot
> time.
>
> > >
> > >>
> > >>> + */
> > >>> +struct
> > >>> +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> > >>> + efi_uintn_t *size, efi_guid_t guid)
> > >>> +{
> > >>> + struct efi_device_path *fp = lo->file_path;
> > >>> + struct efi_device_path_vendor *vendor;
> > >>> + int lo_len = lo->file_path_length;
> > >>> +
> > >>> + while (lo_len) {
> > >>
> > >> lo_len must be at least sizeof(struct efi_device_path).
> > >>
> > >>> + if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
> > >>> + fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH) {
> > >>> + lo_len -= fp->length;
> > >>
> > >> Could the last device path in the array be followed by zero bytes for
> > >> padding?
> > >
> > > How? Device paths are packed aren't they ?
> > >
> > >> Should we check that fp->length >= sizeof(struct efi_device_path)?
> > >
> > > Yea probably a good idea
> >
> > The content of the boot option comes from the user. Just assume that it
> > can contain malicious content.
> >
>
> Yea the user doesn't add the device path directly though. The user adds
> directories and a file, so the normalization is part of this function,
> applied randomly and locally on a single input? or the device path creation
> functions which this code uses? Since we use the pattern in a bunch of places
> I assumed we did take care of that during the functions that create the device
> paths. I haven't checked though ...
>
> > We should also check that the identified device-path starting at
> > VenMedia() ends within fp->length using efi_dp_check_length().
>
> ok
>
> >
> > >
> > >>
> > >>> + fp = (void *)fp + fp->length;
> > >>
> > >> Please, avoid code duplication.
> > >>
> > >> E.g.
> > >>
> > >> for (; lo_len >= sizeof(struct efi_device_path);
> > >> lo_len -= fp->length, fp = (void *)fp + fp->length) {
> > >
> > > I can an switch to that, but I really never liked this format.
> > > It always seemed way less readable to me for some reason. Maybe because I
> > > never got used to it ...
> >
> > Using "for" is only one option. You could use "goto next;" instead.
> >
>
> I really don't mind, I can just use what you propose.
>
> > >
> > >>
> > >>> + continue;
> > >>> + }
> > >>> +
> > >>> + vendor = (struct efi_device_path_vendor *)fp;
> > >>> + if (!guidcmp(&vendor->guid, &guid))
> > >>> + return efi_dp_dup(fp);
> > >>
> > >> Should we strip of the VenMedia() node here?
> > >
> > > Why? This is not supposed to get the file path. The function says "get
> > > device
> > > path from load option" and that device includes the VenMedia node.
> > > It would make more sense for me to strip in efi_get_dp_from_boot() for
> > > example, if you want a helper function to get the initrd path *only*.
> >
> > The VenMedia() node is not needed anymore once you have found the entry.
> >
>
> Yea it's not but as I said the name of the function says "get the *stored*
> from a boot option. Not get the one that suits us.
> There's another reason for that btw, the initrd related functions use that
> (specifically get_initrd_fp()), to figure out if the Boot variable
> contains an initrd path or not.
> If the VenMedia is not present at all, the protocol is not installed allowing
> the kernel to fallback in it's command line 'initrd=' option.
> If the VenMedia is there though, we are checking the file path of the initrd
> and if the file's not found we return an error allowing Bootmgr to fallback.
>
>
Re: [PATCH 3/4] net: mdio: teach dm_eth_phy_connect to connect to fixed PHY
Hi Vladimir, On Wed, Feb 17, 2021 at 6:48 AM Vladimir Oltean wrote: > > From: Vladimir Oltean > > It would be desirable for top-level callers of PHYLIB to deal with as > little complexity as possible, and when they call dm_eth_phy_connect, > they get a struct phy_device that "just works". > > There is a phy_connect_fixed interception put in phy_connect, however > dm_eth_phy_connect will not reach there: if will search for a phy-handle > all by itself, and error out if there isn't one. So we can make callers > of dm_eth_phy_connect suffer by having them call: > > err = dm_eth_phy_connect(); > if (err) > err = dm_eth_phy_connect_fixed(); > > or we can just add the logic in dm_eth_phy_connect() that searches for a > fixed-link before searching for a phy-handle. This logic already exists since: commit bdf319273b4a752664c089fbffee5bb2024c8586 Author: Rasmus Villemoes Date: Mon Oct 5 15:15:16 2020 +0200 mdio-uclass.c: support fixed-link subnodes > In fact we already have an in-tree driver that can make use of this > refactoring: the Freescale TSEC driver. I will drop the dm_eth_phy_connect() changes and only keep the TSEC changes in v2. > > Signed-off-by: Vladimir Oltean > --- > drivers/net/tsec.c | 6 +- > net/mdio-uclass.c | 12 ++-- > 2 files changed, 11 insertions(+), 7 deletions(-) > Regards, Bin
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
Akashi-san, On Fri, Mar 12, 2021 at 11:50:32AM +0900, AKASHI Takahiro wrote: > Ilias, > > I may have missed your past discussions, but any way, It's on the boot-architecture mailing list [1] > > On Thu, Mar 11, 2021 at 01:36:04PM +0200, Ilias Apalodimas wrote: > > > [...] > > > My understanding is that we have: > > > > > > kernel path,end(0xff), > > > VenMedia(), /* no end node here */ > > > initrd1, end(0x01), > > > initrd2, end(0xff) > > > > No, the structure is added in cmd/efidebug.c code. > > It's created with efi_dp_append_instance() on > > - const struct efi_initrd_dp id_dp > > - file path of initrd > > > > which will create: > > kernel path,end(0xff), > > VenMedia(), end(0x01), > > initrd1, end(0x01), > > initrd2, end(0xff) > > What is the difference between end(0xff) and end(0x01)? > 0xff is a subtype of 'end the entire device path', while 0x01 is an 'end of instance of a device path and start a new device path' > If the first argument of a load option is a list of device paths, > I would expect the format would look like: > kernel path,end(0xff), > VenMedia(INITRD),initrd1 path,end(0xff), > VenMedia(INITRD),initrd2 path,end(0xff), > > so that VenMedia can work as an identify of the succeeding path. > Is it simple enough, isn't it? It's essentially the same thing. It has an effect on the EFI spec and how you interpret it, but honestly it feels as an implementation detail to me, since none of those are standardized anyway. In fact what you are saying was part of my proposal in the original mail (check proposal 1.) Anyway the difference between the two is that what I coded looks like this: FilePathList[0] -> kernel FilePathList[1] -> initrd1 - initrdn while whe other is FilePathList[0] -> kernel FilePathList[1] -> initrd1 FilePathList[2] -> initrd2 FilePathList[n] -> initrdn If we ever manage to wire in the DTBs in there as well it may look like: FilePathList[0] -> kernel FilePathList[1] -> initrd1 - initrdn FilePathList[2] -> dtb1 - dtbn Vs FilePathList[0] -> kernel FilePathList[1] -> initrd1 FilePathList[2] -> initrd2 FilePathList[3] -> dtb1 FilePathList[n] -> initrdn FilePathList[n+1] -> dtb2 > > -Takahiro Akashi > > > I know I originally proposed the one you have, but it seemed cleaner adding > > an extra instance between VenMedia and the first initrd. > > > > > > > > Please, document the structure. > > > > > > > Sure > > > > > Best regards > > > > > > Heinrich > > > > Thanks > > /Ilias [1] https://lists.linaro.org/pipermail/boot-architecture/2021-February/001686.html Cheers /Ilias
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On Fri, Mar 12, 2021 at 06:10:29AM +0200, Ilias Apalodimas wrote: > Akashi-san, > > On Fri, Mar 12, 2021 at 11:50:32AM +0900, AKASHI Takahiro wrote: > > Ilias, > > > > I may have missed your past discussions, but any way, > > It's on the boot-architecture mailing list [1] > > > > > On Thu, Mar 11, 2021 at 01:36:04PM +0200, Ilias Apalodimas wrote: > > > > > > [...] > > > > > My understanding is that we have: > > > > > > > > kernel path,end(0xff), > > > > VenMedia(), /* no end node here */ > > > > initrd1, end(0x01), > > > > initrd2, end(0xff) > > > > > > No, the structure is added in cmd/efidebug.c code. > > > It's created with efi_dp_append_instance() on > > > - const struct efi_initrd_dp id_dp > > > - file path of initrd > > > > > > which will create: > > > kernel path,end(0xff), > > > VenMedia(), end(0x01), > > > initrd1, end(0x01), > > > initrd2, end(0xff) > > > > What is the difference between end(0xff) and end(0x01)? > > > > 0xff is a subtype of 'end the entire device path', while 0x01 is an 'end of > instance of a device path and start a new device path' > > > If the first argument of a load option is a list of device paths, > > I would expect the format would look like: > > kernel path,end(0xff), > > VenMedia(INITRD),initrd1 path,end(0xff), > > VenMedia(INITRD),initrd2 path,end(0xff), > > > > so that VenMedia can work as an identify of the succeeding path. > > Is it simple enough, isn't it? > > It's essentially the same thing. It has an effect on the EFI spec and how you > interpret it, but honestly it feels as an implementation detail to me, since > none of those are standardized anyway. > > In fact what you are saying was part of my proposal in the original mail > (check proposal 1.) > > Anyway the difference between the two is that what I coded looks like this: > FilePathList[0] -> kernel > FilePathList[1] -> initrd1 - initrdn > > while whe other is > FilePathList[0] -> kernel > FilePathList[1] -> initrd1 > FilePathList[2] -> initrd2 > FilePathList[n] -> initrdn > > If we ever manage to wire in the DTBs in there as well it may look like: > > FilePathList[0] -> kernel > FilePathList[1] -> initrd1 - initrdn > FilePathList[2] -> dtb1 - dtbn > > Vs > > FilePathList[0] -> kernel > FilePathList[1] -> initrd1 > FilePathList[2] -> initrd2 > FilePathList[3] -> dtb1 > FilePathList[n] -> initrdn > FilePathList[n+1] -> dtb2 What is the semantics? Which do you want to do? a) boot one of combinations: 1.kernel+initrd1+dtb1, or 2.kernel+initrd2+dtb2 b) boot kernel + (initrd1 + initrd2) + (dtb1 + dtb2) I assume you meant (a). In that case, how can you specify (a-1) or (a-2) at boot time? Is there any clear description about that? (I"m simply asking here.) -Takahiro Akashi > > > > > > -Takahiro Akashi > > > > > I know I originally proposed the one you have, but it seemed cleaner > > > adding > > > an extra instance between VenMedia and the first initrd. > > > > > > > > > > > Please, document the structure. > > > > > > > > > > Sure > > > > > > > Best regards > > > > > > > > Heinrich > > > > > > Thanks > > > /Ilias > > [1] > https://lists.linaro.org/pipermail/boot-architecture/2021-February/001686.html > > Cheers > /Ilias
Re: [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
On Fri, Mar 12, 2021 at 01:32:50PM +0900, AKASHI Takahiro wrote: [...] > > > > > My understanding is that we have: > > > > > > > > > > kernel path,end(0xff), > > > > > VenMedia(), /* no end node here */ > > > > > initrd1, end(0x01), > > > > > initrd2, end(0xff) > > > > > > > > No, the structure is added in cmd/efidebug.c code. > > > > It's created with efi_dp_append_instance() on > > > > - const struct efi_initrd_dp id_dp > > > > - file path of initrd > > > > > > > > which will create: > > > > kernel path,end(0xff), > > > > VenMedia(), end(0x01), > > > > initrd1, end(0x01), > > > > initrd2, end(0xff) > > > > > > What is the difference between end(0xff) and end(0x01)? > > > > > > > 0xff is a subtype of 'end the entire device path', while 0x01 is an 'end of > > instance of a device path and start a new device path' > > > > > If the first argument of a load option is a list of device paths, > > > I would expect the format would look like: > > > kernel path,end(0xff), > > > VenMedia(INITRD),initrd1 path,end(0xff), > > > VenMedia(INITRD),initrd2 path,end(0xff), > > > > > > so that VenMedia can work as an identify of the succeeding path. > > > Is it simple enough, isn't it? > > > > It's essentially the same thing. It has an effect on the EFI spec and how > > you > > interpret it, but honestly it feels as an implementation detail to me, since > > none of those are standardized anyway. > > > > In fact what you are saying was part of my proposal in the original mail > > (check proposal 1.) > > > > Anyway the difference between the two is that what I coded looks like this: > > FilePathList[0] -> kernel > > FilePathList[1] -> initrd1 - initrdn > > > > while whe other is > > FilePathList[0] -> kernel > > FilePathList[1] -> initrd1 > > FilePathList[2] -> initrd2 > > FilePathList[n] -> initrdn > > > > If we ever manage to wire in the DTBs in there as well it may look like: > > > > FilePathList[0] -> kernel > > FilePathList[1] -> initrd1 - initrdn > > FilePathList[2] -> dtb1 - dtbn > > > > Vs > > > > FilePathList[0] -> kernel > > FilePathList[1] -> initrd1 > > FilePathList[2] -> initrd2 > > FilePathList[3] -> dtb1 > > FilePathList[n] -> initrdn > > FilePathList[n+1] -> dtb2 > > What is the semantics? > Which do you want to do? > a) boot one of combinations: > 1.kernel+initrd1+dtb1, or > 2.kernel+initrd2+dtb2 > b) boot > kernel + (initrd1 + initrd2) + (dtb1 + dtb2) > > I assume you meant (a). > In that case, how can you specify (a-1) or (a-2) at boot time? > > Is there any clear description about that? > (I"m simply asking here.) it's b) if you want different combinations of kernel/initrds (as described in a) you can add another Boot variable. So let's assume you got three boot options Boot, Boot0001 and Boot0002 Boot efi_load_options: kernel1 + (initrd1 + initrd2) + (dtb1 + dtb2). The bootloader will concat initrd1+initrd2 when the kernel requests it. Similar behavior can be coded for dtb before installing the table. Boot0001: kernel1 + initrd1 + dtb1 Load a kernel with a single initrd and dtb. Boot0002: kernel2 + initrd2 + dtb2 ditto. Hope that's clear now Cheers /Ilias > > -Takahiro Akashi > > > > > > > > > > > -Takahiro Akashi > > > > > > > I know I originally proposed the one you have, but it seemed cleaner > > > > adding > > > > an extra instance between VenMedia and the first initrd. > > > > > > > > > > > > > > Please, document the structure. > > > > > > > > > > > > > Sure > > > > > > > > > Best regards > > > > > > > > > > Heinrich > > > > > > > > Thanks > > > > /Ilias > > > > [1] > > https://lists.linaro.org/pipermail/boot-architecture/2021-February/001686.html > > > > Cheers > > /Ilias
Re: [PATCH 5/6] efidebug: add multiple device path instances on Boot####
Ilias,
Sorry, but I will have to repeat my question for better understandings.
On Sat, Mar 06, 2021 at 12:23:01AM +0200, Ilias Apalodimas wrote:
> The UEFI spec allow a packed array of UEFI device paths in the
> FilePathList[] of an EFI_LOAD_OPTION. The first file path must
> describe the loaded image but the rest are OS specific.
>
> Previous patches parse the device path and try to use the second
> member of the array as an initrd. So let's modify efidebug slightly
> and install the second file described in the command line as the
> initrd device path.
>
> Signed-off-by: Ilias Apalodimas
> ---
> cmd/efidebug.c| 193 ++
> doc/board/emulation/qemu_capsule_update.rst | 4 +-
> doc/uefi/uefi.rst | 2 +-
> .../test_efi_capsule/test_capsule_firmware.py | 6 +-
> test/py/tests/test_efi_secboot/test_signed.py | 16 +-
> .../test_efi_secboot/test_signed_intca.py | 8 +-
> .../tests/test_efi_secboot/test_unsigned.py | 8 +-
> 7 files changed, 179 insertions(+), 58 deletions(-)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index bbbcb0a54643..9a1c471a3eaa 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -9,6 +9,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> @@ -19,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> #define BS systab.boottime
> #define RT systab.runtime
> @@ -794,6 +797,65 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int
> flag,
> return CMD_RET_SUCCESS;
> }
>
> +/**
> + * add_initrd_instance() - Append a device path to load_options pointing to
> an
> + * inirtd
> + *
> + * @argc:Number of arguments
> + * @argv:Argument array
> + * @file_pathExisting device path, the new instance will be appended
> + * Return: Pointer to the device path or ERR_PTR
> + *
> + */
> +static
> +struct efi_device_path *add_initrd_instance(const char *dev, const char
> *part,
> + const char *file,
> + const struct efi_device_path *fp,
> + efi_uintn_t *fp_size)
> +{
> + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL;
> + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL;
> + efi_status_t ret;
> + const struct efi_initrd_dp id_dp = {
> + .vendor = {
> + {
> + DEVICE_PATH_TYPE_MEDIA_DEVICE,
> + DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> + sizeof(id_dp.vendor),
> + },
> + EFI_INITRD_MEDIA_GUID,
> + },
> + .end = {
> + DEVICE_PATH_TYPE_END,
> + DEVICE_PATH_SUB_TYPE_END,
> + sizeof(id_dp.end),
> + }
> + };
> +
> + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp);
> + if (ret != EFI_SUCCESS) {
> + printf("Cannot create device path for \"%s %s\"\n", part, file);
> + goto out;
> + }
> +
> + initrd_dp =
> + efi_dp_append_instance((const struct efi_device_path *)&id_dp,
> +tmp_fp);
> + if (!initrd_dp) {
> + printf("Cannot append media vendor device path path\n");
> + goto out;
> + }
> + final_fp = efi_dp_concat(fp, initrd_dp);
> + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
> + (2 * sizeof(struct efi_device_path));
> +
> +out:
> + efi_free_pool(initrd_dp);
> + efi_free_pool(tmp_dp);
> + efi_free_pool(tmp_fp);
> + return final_fp ? final_fp : ERR_PTR(-EINVAL);
> +}
> +
> /**
> * do_efi_boot_add() - set UEFI load option
> *
> @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int
> flag,
> *
> * Implement efidebug "boot add" sub-command. Create or change UEFI load
> option.
> *
> - * efidebug boot add[:]
>
> + * efidebug boot add -b[:]
> + * -i [:]
> + * -s ''
We discussed another syntax:
efidebug boot add ...
efidebug boot add-initrd
(Please don't care detailed syntax for now.)
What is the difficulty that you have had to implement this type of
interface?
Even if we follow your new syntax,
Why do we need '-b' option?
" [:] " are all mandatory arguments,
aren't they?
-Takahiro Akashi
> */
> static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int
> flag,
> size_t label_len, label_len16;
> u16 *label;
> struct efi_device_path *device_path = NULL, *file_path = NULL;
> + struct efi_device_path *final_fp = NULL;
> struct efi_load_option lo;
> void *data = NULL;
> e
Re: [PATCH v2 2/5] sysinfo: Provide some global/default IDs
On Fri, 5 Mar 2021 at 13:12, Sean Anderson wrote: > > This adds an ID for a board revision. Existing IDs have been moved above > SYSINFO_ID_END to allow for future expansion. > > Signed-off-by: Sean Anderson > --- > > Changes in v2: > - Set BOARD_HWVERSION to SYSINFO_ID_REVISION, as they represent the same > content. > - Use enums instead of defines for sysinfo ids > > drivers/sysinfo/gazerbeam.h | 8 +--- > drivers/sysinfo/sandbox.h | 2 +- > include/sysinfo.h | 13 + > 3 files changed, 19 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 3/5] sysinfo: Require that sysinfo_detect be called before other methods
On Fri, 5 Mar 2021 at 13:12, Sean Anderson wrote:
>
> This has the uclass enforce calling detect() before other methods. This
> allows drivers to cache information in detect() and perform (cheaper)
> retrieval in the other accessors. This also modifies the only instance
> where this sequencing was not followed.
>
> Signed-off-by: Sean Anderson
> ---
>
> Changes in v2:
> - Enforce sysinfo detect ordering in uclass. Users must still call
> sysinfo_detect beforehand.
> - Modify sysinfo test to check for detect() ordering.
>
> common/spl/spl_fit.c | 4
> drivers/sysinfo/sysinfo-uclass.c | 25 -
> include/sysinfo.h| 26 +-
> test/dm/sysinfo.c| 23 ++-
> 4 files changed, 59 insertions(+), 19 deletions(-)
Reviewed-by: Simon Glass
But please see below.
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index a6ad094e91..4d17582af5 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -116,6 +116,10 @@ static int spl_fit_get_image_name(const void *fit, int
> images,
> * no string in the property for this index. Check if the
> * sysinfo-level code can supply one.
> */
> + rc = sysinfo_detect(sysinfo);
> + if (rc)
> + return rc;
> +
> rc = sysinfo_get_fit_loadable(sysinfo, index - i - 1, type,
> &str);
> if (rc && rc != -ENOENT)
> diff --git a/drivers/sysinfo/sysinfo-uclass.c
> b/drivers/sysinfo/sysinfo-uclass.c
> index 6df58fe160..f034ee0870 100644
> --- a/drivers/sysinfo/sysinfo-uclass.c
> +++ b/drivers/sysinfo/sysinfo-uclass.c
> @@ -15,19 +15,29 @@ int sysinfo_get(struct udevice **devp)
>
> int sysinfo_detect(struct udevice *dev)
> {
> + int ret;
> + bool *detected = dev_get_uclass_priv(dev);
I would argue for a struct with a bool in it here, instead of a raw bool.
Regards,
Simon
Re: [PATCH v6 1/4] x86: qemu: move QFW to its own uclass
On Sat, 6 Mar 2021 at 21:01, Asherah Connor wrote: > > We move qfw into its own uclass and split the PIO functions into a > specific driver for that uclass. The PIO driver is selected in the > qemu-x86 board config (this covers x86 and x86_64). > > include/qfw.h is cleaned up and documentation added. > > Signed-off-by: Asherah Connor > > --- > > Changes in v6: > * Sandbox driver and tests are split into own commit. > > Changes in v5: > * Split conversion of existing x86-only QFW to DM into its own patch. > * Fix qfw_get_dev() so it actually returns -ENODEV when device is > missing. > * Add CONFIG_QFW_PIO. > * Choose CONFIG_QFW_PIO in board/emulation/qemu-x86/Kconfig. > * QFW sandbox driver no longer inserts itself from device tree; wasn't > relevant to implementation and the ``compatible'' string was too > made-up. Use U_BOOT_DRVINFO like the other QFW driver in this patch. > * Relevant documentation and sandbox/qemu tests are now here. > > arch/x86/cpu/qemu/cpu.c | 9 +- > arch/x86/cpu/qemu/qemu.c | 49 +-- > arch/x86/cpu/qfw_cpu.c | 11 +- > board/emulation/qemu-x86/Kconfig | 1 + > cmd/qfw.c| 56 --- > common/Makefile | 2 + > common/qfw.c | 105 + > drivers/misc/Kconfig | 11 +- > drivers/misc/Makefile| 5 +- > drivers/misc/qfw.c | 243 +++ > drivers/misc/qfw_pio.c | 69 + > include/dm/uclass-id.h | 1 + > include/qfw.h| 200 + > 13 files changed, 490 insertions(+), 272 deletions(-) > create mode 100644 common/qfw.c > create mode 100644 drivers/misc/qfw_pio.c Reviewed-by: Simon Glass
Re: [PATCH v6 4/4] qemu: arm: select QFW, MMIO on qemu-arm
On Sat, 6 Mar 2021 at 21:01, Asherah Connor wrote: > > Select CMD_QFW and QFW_MMIO in the qemu-arm board (covers arm and > arm64). > > Signed-off-by: Asherah Connor > > --- > > (no changes since v5) > > Changes in v5: > * Split adding MMIO driver to QEMU arm/64 into own commit. > * Use the generic qemu-arm board config rather than adding to multiple > defconfigs. > > board/emulation/qemu-arm/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH] cmd: part: number: remove inconsistent 0x from returned value
Hi Stefan, On Mon, 8 Mar 2021 at 03:45, Stefan Herbrechtsmeier wrote: > > Hi, > > Am 06.03.2021 um 21:12 schrieb Tom Rini: > > On Fri, Mar 05, 2021 at 07:35:24AM -0700, Simon Glass wrote: > >> Hi, > >> > >> On Fri, 5 Mar 2021 at 07:33, Stefan Herbrechtsmeier > >> wrote: > >>> > >>> Hi Eugeniu, > >>> > >>> Am 05.03.2021 um 12:52 schrieb Eugeniu Rosca: > Hello Stefan, > > On Fri, Mar 05, 2021 at 07:39:04AM +, Stefan Herbrechtsmeier wrote: > > From: Stefan Herbrechtsmeier > > > > The part number sub-command returns the hexadecimal value with a leading > > 0x. > > That's to make sure that: > - users have clear and unequivocal feedback that '10' > returned by the command is really HEX 10, not DEC 10. > - other U-Boot commands which need to take '0x10' as input > will interpret it correctly, regardless of the way these > other commands implement ascii-to-integer conversion. > >>> > >>> 'Almost all U-Boot commands expect numbers to be entered in hexadecimal > >>> input format.' [1] > >>> > >>> The filesystem commands use `simple_strtoul(.., 16)` and interpret the > >>> value as hexadecimal value. > >>> > >>> The 0x suggests that a 10 will be interpreted as decimal value and this > >>> isn't true. > >>> > > This is inconsistent with other values in the command > > It could be, but it is then better to fix the inconsistency in those > commands/sub-commands which add the ambiguity. > >>> > >>> Normally you are right but U-Boot by design use hexadecimal values > >>> without 0x. The env_set_hex functions doesn't use 0x. > >>> > > and U-Boot uses hexadecimal values generally. > > The key word is "generally", but not always. Some U-Boot commands will > process '10' as HEX 10 and some will process 10 as DEC 10. So, in order > to avoid these games, I vote for leaving the 0x in place. > >> > >> I would be very surprised if 10 means 0d10 in a partition number. I > >> agree that putting a 0x in these values is a dangerous precedent and > >> will just cause confusion. U-Boot uses hex for addresses and most > >> arguments > >> > >>> > >>> You can avoid it only if you could mark decimal numbers and that is > >>> impossible. > >> > >> 0d10 is available. People are not used to it though. > >> > >> Reviewed-by: Simon Glass > >> > >>> > >>> @Tom: Does U-Boot still expect numbers to be hexadecimal values? > >>> > >>> [1] https://www.denx.de/wiki/DULG/UBootCommandLineInterface > >>> > > > > Signed-off-by: Stefan Herbrechtsmeier > > > > > > --- > > > >cmd/part.c | 2 +- > >1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/cmd/part.c b/cmd/part.c > > index 3395c17b89..56e1852c66 100644 > > --- a/cmd/part.c > > +++ b/cmd/part.c > > @@ -152,7 +152,7 @@ static int do_part_info(int argc, char *const > > argv[], enum cmd_part_info param) > > snprintf(buf, sizeof(buf), LBAF, info.size); > > break; > > case CMD_PART_INFO_NUMBER: > > -snprintf(buf, sizeof(buf), "0x%x", part); > > +snprintf(buf, sizeof(buf), "%x", part); > > > > > I am not a fan of this change as well, especially having spent time on > > some platforms that have literally 20+ partition entries. Being clear > > here that this is a hex value is important. > > But isn't it confusing to use a 0x for a value which is treated as > hexadecimal value by the commands independent of the 0x. The 0x results > in the assumption that the partition is a decimal value without the 0x. > > What is the correct way to convert a hexadecimal value into a decimal > value on the shell? I need a value without prefix (decimal value) to > pass it to the root parameter of the bootargs. > > At the moment the different default numeral systems of common functions > like load and test are very irritating. The load command uses > hexadecimal value and doesn't support decimal value. The test command > uses decimal value by default but supports hexadecimal values with a > prefix. This means that the returned variable filesize of the command > load couldn't be checked by test and the user have to use the itest command. Let's use hex where possible. I am not sure what to do with the test command, nor how often we use 'test' with addresses, but I think adding an option to 'test' to make it use hex or dec would be good. Then one day we could change the default to hex. Regards, Simon
Re: [PATCH u-boot] api: fix a potential serious bug caused by undef CONFIG_SYS_64BIT_LBA
On Sat, 6 Mar 2021 at 15:43, Marek Behún wrote: > > The api_public.h header file undefined macro CONFIG_SYS_64BIT_LBA. > > But api/api_storage.c includes this header before including part.h, > causing the type of lbaint_t and subsequently the type signature of > blk_dread() and blk_dwrite() functions to change from the rest of U-Boot > (if CONFIG_SYS_64BIT_LBA is defined for the board). > > This is of course wrong, because the call to blk_dread() / blk_dwrite() > will recieve mangled arguments. > > Fix this by removing the undef of macro CONFIG_SYS_64BIT_LBA and instead > make the immediate code do what it would do as if the macro was not > defined. > > Add a FIXME to whoever is maintaining this code. > > CI managed to trigger this bug when compiling for lsxhl_defconfig, which > has CONFIG_API selected. The compiler complained about blk_dwrite() and > blk_dread() not matching original declarations: > > include/blk.h:280:15: warning: type of ‘blk_dwrite’ does not match > original declaration > [-Wlto-type-mismatch] > 280 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st > | ^ > drivers/block/blk-uclass.c:456:15: note: type mismatch in parameter 2 > 456 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st > | ^ > > Signed-off-by: Marek Behún > --- > include/api_public.h | 23 ++- > 1 file changed, 18 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass
Re: [RFC PATCH u-boot 02/12] sandbox: errno: avoid conflict with libc's errno
On Tue, 2 Mar 2021 at 21:12, Marek Behún wrote: > > When building with LTO, the system libc's `errno` variable used in > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > lib/errno.c) with the following error: > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > section .tbss mismatches non-TLS reference in > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > To avoid this conflict use different asm label for this variable when > CONFIG_SANDBOX is enabled. > > Signed-off-by: Marek Behún > --- > include/errno.h | 8 +++- > lib/errno.c | 4 +++- > 2 files changed, 10 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v4 4/5] test: scmi: add local variables for scmi agent reference
On Mon, 8 Mar 2021 at 14:38, Etienne Carriere wrote: > > Add local variables agent0/agent1 to refer to SCMI sandbox context > agent and ease readability of the test. > > For consistency, rename regul_dev to regul0_dev and remove sandbox_voltd > in dm_test_scmi_voltage_domains(). > > Signed-off-by: Etienne Carriere > --- > Changes in v4 > - no change > > Changes in v3 > - new change added to the series, suggested from patch v2 review. > --- > test/dm/scmi.c | 100 +++-- > 1 file changed, 55 insertions(+), 45 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v4 5/5] regulator: scmi: fix header file inclusion order
On Mon, 8 Mar 2021 at 14:38, Etienne Carriere wrote: > > Fix header file inclusion order in SCMI regulators driver. > > Signed-off-by: Etienne Carriere > --- > Changes in v4 > - new change added to the series. > --- > drivers/firmware/scmi/scmi_agent-uclass.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 1/2] terminal: correct stdio_dev invocations
On Wed, 10 Mar 2021 at 04:39, Asherah Connor wrote: > > stdio_dev methods have taken a pointer to themselves since 709ea543 > (nearly 7 years ago). > > Signed-off-by: Asherah Connor > --- > > cmd/terminal.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 01/16] dm: core: Add helper to compare node names
Hi Kishon,
On Tue, 9 Mar 2021 at 05:27, Kishon Vijay Abraham I wrote:
>
> Add helper to compare node names.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/core/ofnode.c | 13 +
> include/dm/ofnode.h | 9 +
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index fa0bd2a9c4..4e196d680e 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -18,6 +18,19 @@
> #include
> #include
>
> +bool ofnode_name_eq(ofnode node, const char *name)
> +{
> + const char *node_name;
> + size_t len;
> +
> + assert(ofnode_valid(node));
> +
> + node_name = ofnode_get_name(node);
> + len = strchrnul(node_name, '@') - node_name;
> +
> + return (strlen(name) == len) && (strncmp(node_name, name, len) == 0);
Can you use !strncmp() instead of == 0 ?
> +}
> +
> int ofnode_read_u32(ofnode node, const char *propname, u32 *outp)
> {
> return ofnode_read_u32_index(node, propname, 0, outp);
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 2c0597c407..86a139276b 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -231,6 +231,15 @@ static inline ofnode ofnode_root(void)
> return node;
> }
>
> +/**
> + * ofnode_name_eq() - Check if the node name is equivalent to a given name
> + *
How about a comment indicating this ignores the unit address?
> + * @node: valid node reference that has to be compared
> + * @name: name that has to be compared with the node name
> + * @return 1 if matches, 0 if it doesn't match
> + */
> +bool ofnode_name_eq(ofnode node, const char *name);
> +
> /**
> * ofnode_read_u32() - Read a 32-bit integer from a property
> *
> --
> 2.17.1
>
Please add a test for this in test/dm
Regards,
SImon
Re: [PATCH 02/16] drivers: reset: Handle gracefully NULL pointers
Hi Kishon, On Tue, 9 Mar 2021 at 05:27, Kishon Vijay Abraham I wrote: > > From: Jean-Jacques Hiblot > > Prepare the way for a managed reset API by handling NULL pointers without > crashing nor failing. > > Signed-off-by: Jean-Jacques Hiblot > Signed-off-by: Vignesh Raghavendra > Signed-off-by: Kishon Vijay Abraham I > --- > drivers/reset/reset-uclass.c | 30 +- > 1 file changed, 17 insertions(+), 13 deletions(-) Why do you want this? This patch is missing the motivation which should be at the start of the commit message. Regards, Simon
Re: [PATCH 2/2] terminal: only serial_reinit_all if available
On Wed, 10 Mar 2021 at 04:39, Asherah Connor wrote: > > serial_reinit_all() is only available if CONFIG_SERIAL is defined (i.e. > !CONFIG_DM_SERIAL). > > Signed-off-by: Asherah Connor > > --- > > cmd/terminal.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCHv5 0/6] SCP03 control, documentation and tests.
Hi Foundries, On Wed, 10 Mar 2021 at 09:59, Jorge Ramirez-Ortiz, Foundries wrote: > > On 14/02/21, Jorge Ramirez-Ortiz wrote: > > Simply adding the reviewed-by tags to the v5 series. > > why has this not been merged yet? You can check in patchwork to see whose queue it is in. Regards, Simon
Re: [PATCH v2] tee: optee: Change printing during optee_probe
Hi Ilias, On Wed, 10 Mar 2021 at 06:35, Ilias Apalodimas wrote: > > Right now the error messages when optee has a version mismatch or shared > memory is not configured are done with a debug(). > That's not very convenient since you have to enable debugging to figure > out what's going on, although this is an actual error. The code that probes the device should report the error. If you put errors in a device driver it bloats the code and also it makes it impossible for the caller to suppress the error, e.g. if it is OK for the device to not probe. > > So let's switch the debug() -> dev_err() and report those explicitly. > > Signed-off-by: Ilias Apalodimas > Reviewed-by: Patrick Delaunay > --- > drivers/tee/optee/core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-)
Re: [PATCH v6 3/4] qemu: add MMIO driver for QFW
On Sat, 6 Mar 2021 at 21:01, Asherah Connor wrote: > > Add MMIO driver for QFW. > > Note that there is no consumer as of this patch. > > Signed-off-by: Asherah Connor > --- > > (no changes since v5) > > Changes in v5: > * Split MMIO driver into its own commit. > * Add CONFIG_QFW_MMIO for selection by arch/board. > > drivers/misc/Kconfig| 7 +++ > drivers/misc/Makefile | 1 + > drivers/misc/qfw_mmio.c | 119 > 3 files changed, 127 insertions(+) > create mode 100644 drivers/misc/qfw_mmio.c Reviewed-by: Simon Glass
Re: [PATCH 5/6] efidebug: add multiple device path instances on Boot####
Akashi-san
> >
[...]
> > +/**
> > + * add_initrd_instance() - Append a device path to load_options pointing
> > to an
> > + *inirtd
> > + if (!initrd_dp) {
> > + printf("Cannot append media vendor device path path\n");
> > + goto out;
> > + }
> > + final_fp = efi_dp_concat(fp, initrd_dp);
> > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
> > + (2 * sizeof(struct efi_device_path));
> > +
> > +out:
> > + efi_free_pool(initrd_dp);
> > + efi_free_pool(tmp_dp);
> > + efi_free_pool(tmp_fp);
> > + return final_fp ? final_fp : ERR_PTR(-EINVAL);
> > +}
> > +
> > /**
> > * do_efi_boot_add() - set UEFI load option
> > *
> > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp,
> > int flag,
> > *
> > * Implement efidebug "boot add" sub-command. Create or change UEFI load
> > option.
> > *
> > - * efidebug boot add[:]
> >
> > + * efidebug boot add -b[:]
> > + * -i [:]
> > + * -s ''
>
> We discussed another syntax:
> efidebug boot add ...
> efidebug boot add-initrd
> (Please don't care detailed syntax for now.)
Yep and I completely agree this is a better format but ...
>
> What is the difficulty that you have had to implement this type of
> interface?
The problem is that the initrd and kernel eventually go into *one* Boot
variable. So it's much easier to treat them in a single command as a bundle.
Otherwise you'll have to start adding checks on the initrd code to make
sure the Boot### variable exists before you append an initrd.
Then you have to deserialize the existing stored device path in Boot,
append the initrd and serialize it again (and last time I checked this is not
as easy as it sounds).
Also what happens if you edit a Boot option and that option has an initrd?
You have to pick up the existing initrd and move it over? Or do we silently
delete it?
Something like:
efidebug boot add 0001
efidebug boot add-initrd 0001
efidebug boot add 0001
or even
efidebug boot add 0001
efidebug boot add-initrd 0001
efidebug boot add-initrd 0001
Again it looks better. It's probably easier to use, but this is an efidebug
command. efibootmgr etc is supposed to set this correctly, so does the extra
effort and code worth it?
>
> Even if we follow your new syntax,
> Why do we need '-b' option?
> " [:] " are all mandatory
> arguments,
> aren't they?
Yes, it's just felt a bit more natural to add args on everything. If other
feel the same way I can obviously remove it.
Regards
/Ilias
>
> -Takahiro Akashi
>
>
>
> > */
> > static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> >int argc, char *const argv[])
> > @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int
> > flag,
> > size_t label_len, label_len16;
> > u16 *label;
> > struct efi_device_path *device_path = NULL, *file_path = NULL;
> > + struct efi_device_path *final_fp = NULL;
> > struct efi_load_option lo;
> > void *data = NULL;
> > efi_uintn_t size;
> > + efi_uintn_t fp_size;
> > efi_status_t ret;
> > int r = CMD_RET_SUCCESS;
> > -
> > - if (argc < 6 || argc > 7)
> > - return CMD_RET_USAGE;
> > -
> > - id = (int)simple_strtoul(argv[1], &endp, 16);
> > - if (*endp != '\0' || id > 0x)
> > - return CMD_RET_USAGE;
> > -
> > - sprintf(var_name, "Boot%04X", id);
> > - p = var_name16;
> > - utf8_utf16_strncpy(&p, var_name, 9);
> > + int i;
> >
> > guid = efi_global_variable_guid;
> >
> > /* attributes */
> > lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */
> > + lo.optional_data = NULL;
> > + lo.label = NULL;
> >
> > - /* label */
> > - label_len = strlen(argv[2]);
> > - label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> > - label = malloc((label_len16 + 1) * sizeof(u16));
> > - if (!label)
> > - return CMD_RET_FAILURE;
> > - lo.label = label; /* label will be changed below */
> > - utf8_utf16_strncpy(&label, argv[2], label_len);
> > + /* search for -b first since the rest of the arguments depends on that
> > */
> > + for (i = 0; i < argc; i++) {
> > + if (!strcmp(argv[i], "-b")) {
> > + if (argc < i + 5 || lo.label) {
> > + r = CMD_RET_USAGE;
> > + goto out;
> > + }
> > + id = (int)simple_strtoul(argv[i + 1], &endp, 16);
> > + if (*endp != '\0' || id > 0x)
> > + return CMD_RET_USAGE;
> > +
> > + sprintf(var_name, "Boot%04X", id);
> > + p = var_name16;
> > + utf8_utf16_strncpy(&p, var_name, 9);
> > +
> > + /* label */
> > + label_len = strlen(argv[i + 2]);
> > + label_len16 = utf8_utf16_strnlen(argv[i + 2],
> > label_le
