Re: [PATCH v13 05/10] arm_ffa: introduce armffa command
On Mon, 3 Jul 2023 at 16:53, Abdellatif El Khlifi
wrote:
>
> Hi Simon, Ilias,
>
> On Mon, Jul 03, 2023 at 02:30:51PM +0100, Simon Glass wrote:
> ...
> > > > > > > + log_info("device name %s, dev %p, driver name %s, ops
> > > > > > > %p\n",
> > > > > > > +dev->name,
> > > > > > > + (void *)map_to_sysmem(dev),
> > > > > > > +dev->driver->name,
> > > > > > > +(void *)map_to_sysmem(dev->driver->ops));
> > > > > >
> > > > > > Isn't it more useful to print the physical address map_to_sysmem()
> > > > > > retuns?
> > > > >
> > > > > That's what map_to_sysmem() does, it returns a physical address and
> > > > > it's shown in the log.
> > > >
> > > > I dont have access to u-boot source right, but why do you need all
> > > > these void * casts then?
> > >
> > > Because map_to_sysmem() returns an 'phys_addr_t' (aka 'long long unsigned
> > > int') . However, %p expects 'void *'.
> > >
> > > Compilation warning:
> > >
> > > cmd/armffa.c:181:18: warning: format ‘%p’ expects argument of type ‘void
> > > *’, but argument 3 has type ‘phys_addr_t’ {aka ‘long long unsigned int’}
> > > [8;
> > > ;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat=-Wformat=8;;]
> > > 181 | log_info("device name %s, dev %p, driver name %s, ops
> > > %p\n",
> > > |
> > > ^~
> > > 182 | dev->name,
> > > 183 | map_to_sysmem(dev),
> > > | ~~
> > > | |
> > > | phys_addr_t {aka long long unsigned int}
> >
> > You should be using %lx with map_to_sysmen() since it returns a . Use
> > %p for pointers.
> >
> > In general in U-Boot we use addresses (ulong) rather than pointers.
> > Unfortunately the phys_addr_t type can be larger than the word size. I
> > suggest creating a printf prefix for that type. See the top of blk.h
> > for an example of how to do it. Perhaps in a follow-up patch?
> >
>
> Sounds good. I'm gonna add it in a generic way under include/log.h if that
> makes sense:
>
> #ifdef CONFIG_PHYS_64BIT
> #define PhysAddrLength "ll"
> #else
> #define PhysAddrLength ""
> #endif
> #define PHYS_ADDR_LN "%" PhysAddrLength "x"
> #define PHYS_ADDR_LNU "%" PhysAddrLength "u"
> #endif
>
> Note: In a 32-bit platform phys_addr_t is "unsigned int", in a 64-bit
> platform it is "long long unsigned int"
>
> Then using it this way:
>
> log_info("device %s, addr " PHYS_ADDR_LN ", driver %s, ops "
> PHYS_ADDR_LN "\n",
> dev->name,
> map_to_sysmem(dev),
> dev->driver->name,
> map_to_sysmem(dev->driver->ops));
LGTM thanks
>
> Cheers
> Abdellatif
Re: [PATCH v2 4/5] dm: test: Add test for part_get_info_by_type
On Mon, 3 Jul 2023 at 14:40, Joshua Watt wrote: > > Adds a test suite to ensure that part_get_info_by_type works correctly > by creating a hybrid GPT/MBR partition table and reading both. > > Signed-off-by: Joshua Watt > --- > configs/sandbox_defconfig | 2 + > test/dm/part.c| 87 +++ > 2 files changed, 89 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH v2 5/5] cmd: mbr: Force DOS driver to be used for verify
On Mon, 3 Jul 2023 at 14:41, Joshua Watt wrote: > > Forces the DOS partition type driver to be used when verifying the MBR. > This is particularly useful when using a hybrid MBR & GPT layout as > otherwise MBR verification would mostly likely fail since the GPT > partitions will be returned, even if the MBR is actually valid. > > Signed-off-by: Joshua Watt > --- > cmd/mbr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH v2 3/5] disk: part: Add API to get partitions with specific driver
On Mon, 3 Jul 2023 at 14:40, Joshua Watt wrote: > > Adds part_driver_get_type() API which can be used to force a specific Nit: Add > driver to be used when getting partition information instead of relying > on auto detection. > > Signed-off-by: Joshua Watt > --- > disk/part.c| 38 +++--- > include/part.h | 19 ++- > 2 files changed, 49 insertions(+), 8 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 2/5] dm: test: Improve partition test error output
On Mon, 3 Jul 2023 at 14:40, Joshua Watt wrote: > > Improve the logging when the partition test fails so that it is clear > what went wrong, shown with actual values. > > Signed-off-by: Joshua Watt > --- > test/dm/part.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 1/5] dm: test: Fix partition test to use mmc2
On Mon, 3 Jul 2023 at 14:40, Joshua Watt wrote:
>
> d94d9844bc ("dm: part: Update test to use mmc2") attempted to make the
> test use mmc2, but the change was incomplete in that it didn't also
> change the strings that reference a specific partition. Fix these so
> that the test passes again
>
> Signed-off-by: Joshua Watt
> ---
> test/dm/part.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
Reviewed-by: Simon Glass
Fixes: d94d9844bc ("dm: part: Update test to use mmc2")
Thanks.
Re: [PATCH] tests: Fix exception when cleaning up skipped test
On Mon, 3 Jul 2023 at 14:35, Joshua Watt wrote: > > If test_cat and test_xxd cannot create the required file, the test will > be skipped, but this would result in an exception being raised in the > finally block because the file didn't exist to be cleaned up. This > caused the test to be marked as failed instead of skipped. > > Signed-off-by: Joshua Watt > --- > test/py/tests/test_cat/conftest.py | 3 ++- > test/py/tests/test_xxd/conftest.py | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 07/10] test: dm: add SCMI base protocol test
Hi Simon,
On Mon, Jul 03, 2023 at 02:30:57PM +0100, Simon Glass wrote:
> Hi,
>
> On Mon, 3 Jul 2023 at 01:57, AKASHI Takahiro
> wrote:
> >
> > On Thu, Jun 29, 2023 at 08:09:58PM +0100, Simon Glass wrote:
> > > Hi AKASHI,
> > >
> > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > wrote:
> > > >
> > > > Added is a new unit test for SCMI base protocol, which will exercise all
> > > > the commands provided by the protocol, except SCMI_BASE_NOTIFY_ERRORS.
> > > > $ ut dm scmi_base
> > > > It is assumed that test.dtb is used as sandbox's device tree.
> > > >
> > > > Signed-off-by: AKASHI Takahiro
> > > > ---
> > > > test/dm/scmi.c | 112 +
> > > > 1 file changed, 112 insertions(+)
> > > >
> > > > diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> > > > index 881be3171b7c..563017bb63e0 100644
> > > > --- a/test/dm/scmi.c
> > > > +++ b/test/dm/scmi.c
> > > > @@ -16,6 +16,9 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -95,6 +98,115 @@ static int dm_test_scmi_sandbox_agent(struct
> > > > unit_test_state *uts)
> > > > }
> > > > DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
> > > >
> > > > +static int dm_test_scmi_base(struct unit_test_state *uts)
> > > > +{
> > > > + struct udevice *agent_dev, *base;
> > > > + struct scmi_agent_priv *priv;
> > > > + const struct scmi_base_ops *ops;
> > > > + u32 version, num_agents, num_protocols, impl_version;
> > > > + u32 attributes, agent_id;
> > > > + char vendor[SCMI_BASE_NAME_LENGTH_MAX],
> > > > +agent_name[SCMI_BASE_NAME_LENGTH_MAX];
> > > > + u8 *protocols;
> > > > + int ret;
> > > > +
> > > > + /* preparation */
> > > > + ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, "scmi",
> > > > + _dev));
> > > > + ut_assertnonnull(agent_dev);
> > > > + ut_assertnonnull(priv = dev_get_uclass_plat(agent_dev));
> > > > + ut_assertnonnull(base = scmi_get_protocol(agent_dev,
> > > > +
> > > > SCMI_PROTOCOL_ID_BASE));
> > > > + ut_assertnonnull(ops = dev_get_driver_ops(base));
> > > > +
> > > > + /* version */
> > > > + ret = (*ops->protocol_version)(base, );
> > >
> > > Can you add uclass helpers to call each of the methods? That is how it
> > > is commonly done. You should not be calling ops->xxx directly here.
> >
> > Yes, I will add inline functions instead.
>
> I don't mean inline...see all the other uclasses which define a
Okay, I will *real* functions.
> function which is implemented in the uclass. It is confusing when one
> uclass does something different. People might copy this style and then
> the code base diverges. Did you not notice this when looking around
> the source tree?
But one concern came up in my mind.
Contrary to ordinary "device controllers", there exists only a single
implementation of driver for each of "udevice"'s associated with SCMI
protocols including the base protocol.
So if I follow your suggestion, the code (base.c) might look like:
===
static int __scmi_base_discover_vendor(struct udevice *dev, u8 *vendor)
{
...
}
struct scmi_base_ops scmi_base_ops = {
.base_discover_vendor = __scmi_base_discover_vendor,
}
int scmi_base_discover_vendor(struct udevice *dev, u8 *vendor)
{
struct scmi_base_ops *ops;
ops = scmi_base_dev_ops(dev);
return ops->base_discover_vendor(dev, vendor);
}
===
We will have to have similar definitions for every operation in ops.
It looks quite weird to me as there are always pairs of functions,
like __scmi_base_discover_vendor() and scmi_base_discover_vendor().
We can avoid this redundant code easily by eliminating "ops" abstraction.
But as far as I remember, you insist that every driver that complies
to U-Boot driver model should have a "ops".
What do you make of this?
Thanks
-Takahiro Akashi
> Regards,
> Simon
Re: [PATCH 09/10] doc: cmd: add documentation for scmi
Hi Simon, On Mon, Jul 03, 2023 at 02:30:55PM +0100, Simon Glass wrote: > Hi , > > On Mon, 3 Jul 2023 at 02:19, AKASHI Takahiro > wrote: > > > > On Thu, Jun 29, 2023 at 08:10:02PM +0100, Simon Glass wrote: > > > Hi AKASHI, > > > > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro > > > wrote: > > > > > > > > This is a help text for scmi command. > > > > > > > > Signed-off-by: AKASHI Takahiro > > > > --- > > > > doc/usage/cmd/scmi.rst | 98 ++ > > > > 1 file changed, 98 insertions(+) > > > > create mode 100644 doc/usage/cmd/scmi.rst > > > > > > > > diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst > > > > new file mode 100644 > > > > index ..20cdae4b877d > > > > --- /dev/null > > > > +++ b/doc/usage/cmd/scmi.rst > > > > @@ -0,0 +1,98 @@ > > > > +.. SPDX-License-Identifier: GPL-2.0+: > > > > + > > > > +scmi command > > > > + > > > > + > > > > +Synopsis > > > > + > > > > + > > > > +:: > > > > + > > > > +scmi base info > > > > +scmi base perm_dev > > > > +scmi base perm_proto > > > > +scmi base reset > > > > + > > > > +Description > > > > +--- > > > > + > > > > +The scmi command is used to access and operate on SCMI server. > > > > + > > > > +scmi base info > > > > +~~ > > > > +Show base information about SCMI server and supported protocols > > > > + > > > > +scmi base perm_dev > > > > +~~ > > > > +Allow or deny access permission to the device > > > > + > > > > +scmi base perm_proto > > > > + > > > > +Allow or deny access to the protocol on the device > > > > + > > > > +scmi base reset > > > > +~~~ > > > > +Reset the existing configurations > > > > + > > > > +Parameters are used as follows: > > > > + > > > > + > > > > +Agent ID > > > > > > what is this? > > > > I thought that the meaning was trivial in SCMI context. > > Will change it to "SCMI Agent ID". > > What is SCMI? Really you should explain and link to information about > things you mention in documentation. How else is anyone supposed to Generally yes, but please note that SCMI and related drivers are already there in U-Boot. For instance, see drivers/firmware/scmi/Kconfig. I don't think we need any more explanation about what is SCMI everywhere the word, "SCMI", appears. > figure out what you are talking about? Again, those who don't know about SCMI and if SCMI server is installed on their systems will never use this command. > > "SCMI Agent ID" doesn't tell us much. What form does it take? Is it a > string? How do you find it out? While I still believe that What SCMI agent ID means is trivial for those who read the SCMI specification, I will give a short description about possible values. > > > > > > > > + > > > > + > > > > +Device ID > > > > > > what is this? > > > > Again, will change it to "SCMI Device ID". > > That doesn't help much. The purpose of documentation is to explain > things for people who don't already know it. We need to try to be as > helpful as possible. The same above. Please note that the definition of "device (ID)", except its data type, is out of scope of SCMI specification. It doesn't describe any means by which values be identified/retrieved. > > > > > > + > > > > + > > > > +Protocol ID, should not be 0x10 (base protocol) > > > > > > what is this? Please add more detail > > > > Again, will change it to "SCMI Protocol ID". > > I think that users should be familiar with those terms > > if they want to use these interfaces. > > Maybe, but it still isn't clear what it is. A string? Will add a short description about its data type/format. > It is confusing > that the command ID is also a protocol ID. Yes, I know, but the confusion exists in SCMI specification. It sometimes seems to use both words almost interchangeably, even in a single context. For instance, the section 4.2.2.11 of [1] says, 4.2.2.11 BASE_SET_PROTOCOL_PERMISSIONS ... This command BASE_SET_PROTOCOL_PERMISSIONS is used to ... (This "command" has a yet another meaning.) Parameters uint32_t command_id Bits[31:8] Reserved, must be zero Bits[7:0] Protocol ID [1] https://developer.arm.com/documentation/den0056/e/?lang=en So using "command_id" for a parameter name and "Protocol ID" as a (part of) value is very much aligned with the specification. That said, I will change "command_id" to "protocol_id" for now. > > > > > > + > > > > + > > > > +Flags to control the action. See SCMI specification for > > > > +defined values. > > > > > > ? > > > > > > Please add the flags here, or at the very least provide a URL and page > > > number, etc. > > > > I intentionally avoid providing details here because a set of flags > > acceptable to a specific SCMI server may depend on the server > > and its implementation version. > > The interface on U-Boot is just a wrapper to make a call to SCMI server > > via a transport
[GIT PULL] Please pull u-boot-mmc master
Dear Tom, Please pull u-boot-mmc master into u-boot master branch. If there is any problem, let me know, plz. Sorry for too late. Those patches are the fixing reensas-sdhi tuning command error and improve the comment. (I have also tested with this patchset on my board.) Best Regards, Jaehoon Chung CI: https://source.denx.de/u-boot/custodians/u-boot-mmc/-/pipelines/16765 The following changes since commit e1bebc16e1d9aa0ddd56c53c0b781f7186dce557: Prepare v2023.07-rc6 (2023-07-03 13:48:58 -0400) are available in the Git repository at: g...@source.denx.de:u-boot/custodians/u-boot-mmc.git master for you to fetch changes up to 40825dbac96542e914df4baa3aa0252a503b6b88: mmc: Set clock when reverting to safe bus mode (2023-07-04 08:14:18 +0900) Hai Pham (2): mmc: Introduce mmc_send_stop_transmission() mmc: renesas-sdhi: Send stop when MMC tuning command fails Marek Vasut (1): mmc: Fix MMC_CMD_STOP_TRANSMISSION response type and add comment Pali Rohár (1): mmc: spl: Add comments for default_spl_mmc_emmc_boot_partition() Valentine Barshak (1): mmc: Set clock when reverting to safe bus mode common/spl/spl_mmc.c | 42 +++--- drivers/mmc/mmc.c | 26 ++ drivers/mmc/renesas-sdhi.c | 11 +++ include/mmc.h | 2 ++ 4 files changed, 74 insertions(+), 7 deletions(-)
Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware
On Mon, Jul 03, 2023 at 02:30:54PM +0100, Simon Glass wrote:
> Hi,
>
> On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro
> wrote:
> >
> > On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> > > Hi AKASHI,
> > >
> > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > wrote:
> > > >
> > > > This command, "scmi", provides a command line interface to various SCMI
> > > > protocols. It supports at least initially SCMI base protocol with the
> > > > sub
> > > > command, "base", and is intended mainly for debug purpose.
> > > >
> > > > Signed-off-by: AKASHI Takahiro
> > > > ---
> > > > cmd/Kconfig | 8 ++
> > > > cmd/Makefile | 1 +
> > > > cmd/scmi.c | 373 +++
> > > > 3 files changed, 382 insertions(+)
> > > > create mode 100644 cmd/scmi.c
> > > >
> > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > index 02e54f1e50fe..065470a76295 100644
> > > > --- a/cmd/Kconfig
> > > > +++ b/cmd/Kconfig
> > > > @@ -2504,6 +2504,14 @@ config CMD_CROS_EC
> > > > a number of sub-commands for performing EC tasks such as
> > > > updating its flash, accessing a small saved context area
> > > > and talking to the I2C bus behind the EC (if there is one).
> > > > +
> > > > +config CMD_SCMI
> > > > + bool "Enable scmi command"
> > > > + depends on SCMI_FIRMWARE
> > > > + default n
> > > > + help
> > > > + This command provides user interfaces to several SCMI
> > > > protocols,
> > > > + including Base protocol.
> > >
> > > Please mention what is SCMI
> >
> > I will give a full spelling.
> >
> > > > endmenu
> > > >
> > > > menu "Filesystem commands"
> > > > diff --git a/cmd/Makefile b/cmd/Makefile
> > > > index 6c37521b4e2b..826e0b74b587 100644
> > > > --- a/cmd/Makefile
> > > > +++ b/cmd/Makefile
> > > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> > > > obj-$(CONFIG_CMD_NVME) += nvme.o
> > > > obj-$(CONFIG_SANDBOX) += sb.o
> > > > obj-$(CONFIG_CMD_SF) += sf.o
> > > > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> > > > obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> > > > obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> > > > obj-$(CONFIG_CMD_SEAMA) += seama.o
> > > > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > > > new file mode 100644
> > > > index ..c97f77e97d95
> > > > --- /dev/null
> > > > +++ b/cmd/scmi.c
> > > > @@ -0,0 +1,373 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * SCMI utility command
> > > > + *
> > > > + * Copyright (c) 2023 Linaro Limited
> > > > + * Author: AKASHI Takahiro
> > > > + */
> > > > +
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include /* uclass_get_device */
> > >
> > > Goes before linux/bitfield.h
> >
> > Can you tell me why?
>
> It is just a convention:
> https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
Okay.
> >
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +static struct udevice *agent;
> > > > +static struct udevice *base_proto;
> > > > +static const struct scmi_base_ops *ops;
> > > > +
> > > > +struct {
> > > > + enum scmi_std_protocol id;
> > > > + const char *name;
> > > > +} protocol_name[] = {
> > > > + {SCMI_PROTOCOL_ID_BASE, "Base"},
> > > > + {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > > > + {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > > > + {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > > > + {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > > > + {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > > > + {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > > > + {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
> > > > +};
> > > > +
> > > > +/**
> > > > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > > > + *
> > > > + * @id:Protocol ID
> > > > + *
> > > > + * Get the printable name of the protocol, @id
> > > > + *
> > > > + * Return: Name string on success, NULL on failure
> > > > + */
> > > > +static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
> > >
> > > scmi_lookup_id?
> >
> > Not sure your intention.
> > The name above gives us exactly what this function does for pretty printing
> > instead of a number.
> > I think that 'lookup' implies we try to determine if the id exists or not.
>
> I am just trying to avoid the code turning into Java :-)
Java?
> How about scmi_get_name() ?
Well, more specifically scmi_get_protocol_name()?
-Takahiro Akashi
> Regards,
> Simon
[PATCH 1/1] RISC-V: CONFIG_SPL_OPENSBI_SCRATCH_OPTIONS description
Describe which numeric values can be used for as scratch options for OpenSBI. Signed-off-by: Heinrich Schuchardt --- common/spl/Kconfig | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 2c042ad306..7f99889ec3 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -1524,8 +1524,10 @@ config SPL_OPENSBI_SCRATCH_OPTIONS default 0x1 depends on SPL_OPENSBI help - Options passed to fw_dynamic, for example SBI_SCRATCH_NO_BOOT_PRINTS or - SBI_SCRATCH_DEBUG_PRINTS. + This bitmap of options is passed from U-Boot SPL to OpenSBI. + As of OpenSBI 1.3 the following bits are defined: + - SBI_SCRATCH_NO_BOOT_PRINTS = 0x1 (Disable prints during boot) + - SBI_SCRATCH_DEBUG_PRINTS = 0x2 (Enable runtime debug prints) config SPL_TARGET string "Addtional build targets for 'make'" -- 2.40.1
Re: [PATCH 2/3] mmc: sdhci-cadence: SD6 Controller support
Hi, On 6/5/23 22:58, Piyush Malgujar wrote: > From: Dhananjay Kangude > > Add support for SD6 controller along with: > - HS200, HS400 and HS400ES support > - Host side Slew and drive configuration > > These changes to support SD6 cadence IP are isolated from the > existing support of SD4 controller. > > Signed-off-by: Dhananjay Kangude > Co-developed-by: Jayanthi Annadurai > Signed-off-by: Jayanthi Annadurai > Signed-off-by: Piyush Malgujar > --- > drivers/mmc/Kconfig |1 + > drivers/mmc/sdhci-cadence.c | 1317 ++- > 2 files changed, 1301 insertions(+), 17 deletions(-) > > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig > index > de01b9687bad28f3b493208c145f5c5dd2f59f78..6b724f1d1ed050b49e31a6cbe5f898b4b636c330 > 100644 > --- a/drivers/mmc/Kconfig > +++ b/drivers/mmc/Kconfig > @@ -566,6 +566,7 @@ config MMC_SDHCI_CADENCE > depends on BLK && DM_MMC > depends on MMC_SDHCI > depends on OF_CONTROL > + select MMC_SDHCI_IO_ACCESSORS > help > This selects the Cadence SD/SDIO/eMMC driver. > > diff --git a/drivers/mmc/sdhci-cadence.c b/drivers/mmc/sdhci-cadence.c > index > 0bb258da63e442232310d9433b7b6882992bd45d..1d8e8e177eeeb6464ba9ade5fd63c1cca4554bb6 > 100644 > --- a/drivers/mmc/sdhci-cadence.c > +++ b/drivers/mmc/sdhci-cadence.c > @@ -6,6 +6,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -18,8 +19,19 @@ > #include > #include > > +#define SDHCI_CDNS_SD6_MAXCLK2 > + > +#define DEFAULT_CMD_DELAY16 > +#define SDHCI_CDNS_TUNE_START16 > +#define SDHCI_CDNS_TUNE_STEP 6 > +#define SDHCI_CDNS_TUNE_ITERATIONS 40 > + > +#define SDHCI_CDNS_HRS00 0x00 > +#define SDHCI_CDNS_HRS00_SWR BIT(0) > + > +#define SDHCI_CDNS_HRS02 0x08/* PHY access > port */ > +#define SDHCI_CDNS_HRS04 0x10/* PHY access > port */ > /* SD 4.0 Controller HRS - Host Register Set (specific to Cadence) */ > -#define SDHCI_CDNS_HRS04 0x10/* PHY access port */ > #define SDHCI_CDNS_SD4_HRS04_ACK BIT(26) > #define SDHCI_CDNS_SD4_HRS04_RD BIT(25) > #define SDHCI_CDNS_SD4_HRS04_WR BIT(24) > @@ -32,12 +44,93 @@ > #define SDHCI_CDNS_HRS06_TUNE GENMASK(13, 8) > #define SDHCI_CDNS_HRS06_MODE GENMASK(2, 0) > #define SDHCI_CDNS_HRS06_MODE_SD 0x0 > +#define SDHCI_CDNS_HRS06_MODE_LEGACY 0x1 > #define SDHCI_CDNS_HRS06_MODE_MMC_SDR 0x2 > #define SDHCI_CDNS_HRS06_MODE_MMC_DDR 0x3 > #define SDHCI_CDNS_HRS06_MODE_MMC_HS2000x4 > #define SDHCI_CDNS_HRS06_MODE_MMC_HS4000x5 > #define SDHCI_CDNS_HRS06_MODE_MMC_HS400ES 0x6 > > +/* SD 6.0 Controller HRS - Host Register Set (Specific to Cadence) */ > +#define SDHCI_CDNS_SD6_HRS04_ADDRGENMASK(15, 0) > + > +#define SDHCI_CDNS_HRS05 0x14 > + > +#define SDHCI_CDNS_HRS07 0x1C > +#define SDHCI_CDNS_HRS07_RW_COMPENSATE GENMASK(20, 16) > +#define SDHCI_CDNS_HRS07_IDELAY_VAL GENMASK(4, 0) > + > +#define SDHCI_CDNS_HRS09 0x24 > +#define SDHCI_CDNS_HRS09_RDDATA_EN BIT(16) > +#define SDHCI_CDNS_HRS09_RDCMD_EN BIT(15) > +#define SDHCI_CDNS_HRS09_EXTENDED_WR_MODE BIT(3) > +#define SDHCI_CDNS_HRS09_EXTENDED_RD_MODE BIT(2) > +#define SDHCI_CDNS_HRS09_PHY_INIT_COMPLETE BIT(1) > +#define SDHCI_CDNS_HRS09_PHY_SW_RESET BIT(0) > + > +#define SDHCI_CDNS_HRS10 0x28 > +#define SDHCI_CDNS_HRS10_HCSDCLKADJ GENMASK(19, 16) > + > +#define SDHCI_CDNS_SRS11 0x2c > +#define SDHCI_CDNS_SRS11_SW_RESET_ALLBIT(24) > +#define SDHCI_CDNS_SRS11_SW_RESET_CMDBIT(25) > +#define SDHCI_CDNS_SRS11_SW_RESET_DATBIT(26) > + > +#define SDHCI_CDNS_HRS16 0x40 > +#define SDHCI_CDNS_HRS16_WRDATA1_SDCLK_DLY GENMASK(31, 28) > +#define SDHCI_CDNS_HRS16_WRDATA0_SDCLK_DLY GENMASK(27, 24) > +#define SDHCI_CDNS_HRS16_WRCMD1_SDCLK_DLYGENMASK(23, 20) > +#define SDHCI_CDNS_HRS16_WRCMD0_SDCLK_DLYGENMASK(19, 16) > +#define SDHCI_CDNS_HRS16_WRDATA1_DLY GENMASK(15, 12) > +#define SDHCI_CDNS_HRS16_WRDATA0_DLY GENMASK(11, 8) > +#define SDHCI_CDNS_HRS16_WRCMD1_DLY GENMASK(7, 4) > +#define SDHCI_CDNS_HRS16_WRCMD0_DLY GENMASK(3, 0) > + > +/* PHY registers for SD6 controller */ > +#define SDHCI_CDNS_SD6_PHY_DQ_TIMING 0x2000 > +#define SDHCI_CDNS_SD6_PHY_DQ_TIMING_IO_MASK_ALWAYS_ON BIT(31) > +#define SDHCI_CDNS_SD6_PHY_DQ_TIMING_IO_MASK_END > GENMASK(29, 27) > +#define
Re: sipeed maix bit resets every 90 seconds with watchdog enabled
Hi Jaap, On 7/3/23 13:53, Jaap Aart wrote: Dear sean, and others, I encountered a small problem running linux on the sipeed maix bit from u-boot. Every 90 seconds (exactly) after boot it resets, if booted into linux, unless I disable the watchdog. This has occurred since this feature was enabled (e3282b1bbae4e8558c2b85bf27560d12358ed25f) Although I do not completely understand the watchdog features, it notes it is set to 60 seconds, and I am convinced that the point is not to reset every X seconds. Especially because this is enabled by default. I have not really found a good workaround besides disabling the WDT at runtime which is cumbersome. Is this expected behavior? And if so, how do I disable this feature at compile time? Kind regards, Jaap Aarts Do you have CONFIG_DW_WATCHDOG enabled in your kernel? --Sean
Re: [PATCH v2] efi_driver: fix duplicate efiblk#0 issue
On 03.07.23 15:30, Simon Glass wrote:
Hi Masahisa,
On Mon, 3 Jul 2023 at 07:09, Masahisa Kojima wrote:
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
Fixes: 05ef48a2484b ("efi_driver: EFI block driver")
Signed-off-by: Masahisa Kojima
---
Changes in v2:
- uses blk_next_free_devnum() instead of blk_find_max_devnum()
lib/efi_driver/efi_block_device.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebb..e3abd90275 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -124,10 +124,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
*interface)
struct efi_block_io *io = interface;
struct efi_blk_plat *plat;
- devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
Simon, this line was last changed by your patch
e33a5c6be55e ("blk: Switch over to using uclass IDs")
- if (devnum == -ENODEV)
- devnum = 0;
- else if (devnum < 0)
+ devnum = blk_next_free_devnum(UCLASS_EFI_LOADER);
This really should be an internal function but I see it was exported
as part of the virtio work.
How come the EFI and DM block devices are getting out of sync?
They never were in sync:
The bug dates back to Jan 2018:
05ef48a2484b ("efi_driver: EFI block driver")
Best regards
Heinrich
Anyway this function is munging around in the internals of the device
and should be fixed before it causes more problems.
For now, I suggest following what most other drivers so which is to
call blk_create_devicef() passing a devnum of -1.
+ if (devnum < 0)
return EFI_OUT_OF_RESOURCES;
name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
--
2.34.1
Regards,
Simon
Re: v2023.07-rc5 regression: Image overlaps SPL
On Mon, Jul 03, 2023 at 04:01:15PM -0400, Tom Rini wrote:
> On Mon, Jul 03, 2023 at 09:54:40PM +0200, Francesco Dolcini wrote:
> > On Mon, Jul 03, 2023 at 09:40:51PM +0200, Marek Vasut wrote:
> > > On 7/3/23 18:49, Francesco Dolcini wrote:
> > > > Short update on this regression.
> > > >
> > > > On Thu, Jun 29, 2023 at 04:19:22PM +0200, Francesco Dolcini wrote:
> > > > > I also noticed something weird on a colibri imx7s, this is not using
> > > > > SPL,
> > > > > likely something completly different, however given this is new also
> > > > > from rc5 I
> > > > > thought it's valuable to report:
> > > > >
> > > > > ```
> > > > > U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 -
> > > > > 13:39:58 +)
> > > > > CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> > > > > CPU: Extended Commercial temperature grade (-20C to 105C) at 30C
> > > > > Reset cause: POR
> > > > > DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3)
> > >
> > > If you have the u-boot (elf file, unstripped), then check which initcall
> > > it
> > > is that failed . Just run arm-...-objdump -lSD on it and search for the
> > > 8781b350: address ; remember to clear the LSbit of the address in case
> > > this
> > > is thumb mode of the CPU.
> >
> >
> > U-Boot 2023.07-rc6 (Jul 03 2023 - 21:50:58 +0200)
> >
> > CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> > CPU: Extended Commercial temperature grade (-20C to 105C) at 35C
> > Reset cause: POR
> > DRAM: initcall sequence 8786fb00 failed at call 8781b715 (err=-3)
> > ### ERROR ### Please RESET the board ###
> >
> >
> > u-boot/common/board_f.c:523
> > }
> > 8781b710: 2000movsr0, #0
> > 8781b712: bd08pop {r3, pc}
> >
> > 8781b714 :
> > fix_fdt():
> > u-boot/common/board_f.c:729
> > return board_fix_fdt((void *)gd->fdt_blob);
> > 8781b714: f8d9 0068 ldr.w r0, [r9, #104] ; 0x68
> > 8781b718: f7ea bafe b.w 87805d18
>
> So, that's in your board code then:
> board/toradex/colibri_imx7/colibri_imx7.c:int board_fix_fdt(void *rw_fdt_blob)
Yep, this is the problematic code:
board/toradex/colibri_imx7/colibri_imx7.c:
int board_fix_fdt(void *rw_fdt_blob)
{
/* i.MX 7Solo has only one single USB OTG1 but no USB host port */
if (is_cpu_type(MXC_CPU_MX7S)) {
int offset = fdt_path_offset(rw_fdt_blob,
"/soc/bus@3080/usb@30b2");
return fdt_status_disabled(rw_fdt_blob, offset);
}
return 0;
}
For what is worth in this node "/soc/bus@3080/usb@30b2" the
`status="ok"` property is already present.
Before 8c103c33fb14 ("dm: dts: Convert driver model tags to use new
schema") fdt_status_disabled() did return 0, so all good
no issue.
If I do this small partial revert
--- a/arch/arm/dts/imx7d-colibri-eval-v3-u-boot.dtsi
+++ b/arch/arm/dts/imx7d-colibri-eval-v3-u-boot.dtsi
@@ -15,7 +15,8 @@
pinctrl-0 = <_lcdif_dat
_lcdif_ctrl>;
display = <>;
- u-boot,dm-pre-reloc;
+ bootph-all;
fdt_status_disabled() returns 0 again.
With the current master, fdt_status_disabled() returns -3,
-FDT_ERR_NOSPACE, and I assume I could just change my code to call
fdt_increase_size() and call it done.
However, what the reason for this different behavior triggered by that
change in the *-u-boot.dtsi ? Is this expected?
>From a quick check multiple place in the code just disable/enable nodes
or set properties without taking care of this, are those going to be
affected by that commit that created the regression? Are those all
buggy?
$ git grep fdt_setprop |wc -l
461
We have some helper around, for example
arch/arm/mach-imx/imx8/fdt.c:disable_fdt_node(), but this is for example
just specific on that file.
Simon: any suggestion?
Thanks,
Francesco
Re: v2023.07-rc5 regression: Image overlaps SPL
On Mon, Jul 03, 2023 at 09:54:40PM +0200, Francesco Dolcini wrote:
> On Mon, Jul 03, 2023 at 09:40:51PM +0200, Marek Vasut wrote:
> > On 7/3/23 18:49, Francesco Dolcini wrote:
> > > Short update on this regression.
> > >
> > > On Thu, Jun 29, 2023 at 04:19:22PM +0200, Francesco Dolcini wrote:
> > > > I also noticed something weird on a colibri imx7s, this is not using
> > > > SPL,
> > > > likely something completly different, however given this is new also
> > > > from rc5 I
> > > > thought it's valuable to report:
> > > >
> > > > ```
> > > > U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58
> > > > +)
> > > > CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> > > > CPU: Extended Commercial temperature grade (-20C to 105C) at 30C
> > > > Reset cause: POR
> > > > DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3)
> >
> > If you have the u-boot (elf file, unstripped), then check which initcall it
> > is that failed . Just run arm-...-objdump -lSD on it and search for the
> > 8781b350: address ; remember to clear the LSbit of the address in case this
> > is thumb mode of the CPU.
>
>
> U-Boot 2023.07-rc6 (Jul 03 2023 - 21:50:58 +0200)
>
> CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> CPU: Extended Commercial temperature grade (-20C to 105C) at 35C
> Reset cause: POR
> DRAM: initcall sequence 8786fb00 failed at call 8781b715 (err=-3)
> ### ERROR ### Please RESET the board ###
>
>
> u-boot/common/board_f.c:523
> }
> 8781b710: 2000movsr0, #0
> 8781b712: bd08pop {r3, pc}
>
> 8781b714 :
> fix_fdt():
> u-boot/common/board_f.c:729
> return board_fix_fdt((void *)gd->fdt_blob);
> 8781b714: f8d9 0068 ldr.w r0, [r9, #104] ; 0x68
> 8781b718: f7ea bafe b.w 87805d18
So, that's in your board code then:
board/toradex/colibri_imx7/colibri_imx7.c:int board_fix_fdt(void *rw_fdt_blob)
--
Tom
signature.asc
Description: PGP signature
Re: v2023.07-rc5 regression: Image overlaps SPL
On Mon, Jul 03, 2023 at 09:54:40PM +0200, Francesco Dolcini wrote:
> On Mon, Jul 03, 2023 at 09:40:51PM +0200, Marek Vasut wrote:
> > On 7/3/23 18:49, Francesco Dolcini wrote:
> > > Short update on this regression.
> > >
> > > On Thu, Jun 29, 2023 at 04:19:22PM +0200, Francesco Dolcini wrote:
> > > > I also noticed something weird on a colibri imx7s, this is not using
> > > > SPL,
> > > > likely something completly different, however given this is new also
> > > > from rc5 I
> > > > thought it's valuable to report:
> > > >
> > > > ```
> > > > U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58
> > > > +)
> > > > CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> > > > CPU: Extended Commercial temperature grade (-20C to 105C) at 30C
> > > > Reset cause: POR
> > > > DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3)
> >
> > If you have the u-boot (elf file, unstripped), then check which initcall it
> > is that failed . Just run arm-...-objdump -lSD on it and search for the
> > 8781b350: address ; remember to clear the LSbit of the address in case this
> > is thumb mode of the CPU.
>
>
> U-Boot 2023.07-rc6 (Jul 03 2023 - 21:50:58 +0200)
>
> CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> CPU: Extended Commercial temperature grade (-20C to 105C) at 35C
> Reset cause: POR
> DRAM: initcall sequence 8786fb00 failed at call 8781b715 (err=-3)
> ### ERROR ### Please RESET the board ###
>
>
> u-boot/common/board_f.c:523
> }
> 8781b710: 2000movsr0, #0
> 8781b712: bd08pop {r3, pc}
>
> 8781b714 :
> fix_fdt():
> u-boot/common/board_f.c:729
> return board_fix_fdt((void *)gd->fdt_blob);
> 8781b714: f8d9 0068 ldr.w r0, [r9, #104] ; 0x68
> 8781b718: f7ea bafe b.w 87805d18
so, this seems to be the broken code:
/* i.MX 7Solo has only one single USB OTG1 but no USB host port */
if (is_cpu_type(MXC_CPU_MX7S)) {
int offset = fdt_path_offset(rw_fdt_blob,
"/soc/bus@3080/usb@30b2");
return fdt_status_disabled(rw_fdt_blob, offset);
}
now I wonder how is this related to that other change ...
Re: v2023.07-rc5 regression: Image overlaps SPL
On Mon, Jul 03, 2023 at 09:40:51PM +0200, Marek Vasut wrote:
> On 7/3/23 18:49, Francesco Dolcini wrote:
> > Short update on this regression.
> >
> > On Thu, Jun 29, 2023 at 04:19:22PM +0200, Francesco Dolcini wrote:
> > > I also noticed something weird on a colibri imx7s, this is not using SPL,
> > > likely something completly different, however given this is new also from
> > > rc5 I
> > > thought it's valuable to report:
> > >
> > > ```
> > > U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58
> > > +)
> > > CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> > > CPU: Extended Commercial temperature grade (-20C to 105C) at 30C
> > > Reset cause: POR
> > > DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3)
>
> If you have the u-boot (elf file, unstripped), then check which initcall it
> is that failed . Just run arm-...-objdump -lSD on it and search for the
> 8781b350: address ; remember to clear the LSbit of the address in case this
> is thumb mode of the CPU.
U-Boot 2023.07-rc6 (Jul 03 2023 - 21:50:58 +0200)
CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
CPU: Extended Commercial temperature grade (-20C to 105C) at 35C
Reset cause: POR
DRAM: initcall sequence 8786fb00 failed at call 8781b715 (err=-3)
### ERROR ### Please RESET the board ###
u-boot/common/board_f.c:523
}
8781b710: 2000movsr0, #0
8781b712: bd08pop {r3, pc}
8781b714 :
fix_fdt():
u-boot/common/board_f.c:729
return board_fix_fdt((void *)gd->fdt_blob);
8781b714: f8d9 0068 ldr.w r0, [r9, #104] ; 0x68
8781b718: f7ea bafe b.w 87805d18
Re: v2023.07-rc5 regression: Image overlaps SPL
On 7/3/23 18:49, Francesco Dolcini wrote: Short update on this regression. On Thu, Jun 29, 2023 at 04:19:22PM +0200, Francesco Dolcini wrote: I also noticed something weird on a colibri imx7s, this is not using SPL, likely something completly different, however given this is new also from rc5 I thought it's valuable to report: ``` U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58 +) CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 30C Reset cause: POR DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3) If you have the u-boot (elf file, unstripped), then check which initcall it is that failed . Just run arm-...-objdump -lSD on it and search for the 8781b350: address ; remember to clear the LSbit of the address in case this is thumb mode of the CPU.
Re: [PATCH 1/2] mxs: Fix VDDx brownout interrupt disable/enable
On 7/3/23 20:33, Cody Green wrote: On Mon, Jul 3, 2023 at 5:40 PM Marek Vasut wrote: On 7/3/23 18:33, Cody Green wrote: HW_POWER_CTRL register contains brownout interrupt enable bits ENIRQ_VDDIO_BO, ENIRQ_VDDA_BO and ENIRQ_VDDD_BO. So what does this patch do (that is missing in the commit message) ? There are incorrect registers HW_POWER_VDDIOCTRL, HW_POWER_VDDACTRL and HW_POWER_VDDDCTRL used in the current code to disable/enable brownout interrupts in 'mxs_power_set_vddx()'. This patch ensures the correct register HW_POWER_CTRL is used. Sorry for the unspecific commit message, I will amend the patch. Much better, thank you. Also CC Lukasz (on CC here) on V2.
Re: [PATCH 12/12] binman: Support simple templates
Hi Simon,
On 28.06.23 13:41, Simon Glass wrote:
> Collections can used to collect the contents of other entries into a
> single entry, but they result in a single entry, with the original entries
> 'left behind' in their old place.
>
> It is useful to be able to specific a set of entries ones and have it used
> in multiple images, or parts of an image.
>
> Implement this mechanism.
>
> Signed-off-by: Simon Glass
> ---
>
> tools/binman/binman.rst | 80
> tools/binman/control.py | 9 +++
> tools/binman/etype/section.py| 3 +-
> tools/binman/ftest.py| 8 +++
> tools/binman/test/286_entry_template.dts | 42 +
> 5 files changed, 141 insertions(+), 1 deletion(-)
> create mode 100644 tools/binman/test/286_entry_template.dts
>
> diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
> index a4b31fe5397b..9be979ae1c5b 100644
> --- a/tools/binman/binman.rst
> +++ b/tools/binman/binman.rst
> @@ -727,6 +727,13 @@ optional:
> Note that missing, optional blobs do not produce a non-zero exit code
> from
> binman, although it does show a warning about the missing external blob.
>
> +insert-template:
> +This is not strictly speaking an entry property, since it is processed
> early
> +in Binman before the entries are read. It is a list of phandles of nodes
> to
> +include in the current (target) node. For each node, its subnodes and
> their
> +properties are brought into the target node. See Templates_ below for
> +more information.
> +
> The attributes supported for images and sections are described below. Several
> are similar to those for entries.
>
> @@ -1172,6 +1179,79 @@ If you are having trouble figuring out what is going
> on, you can use
> arch/arm/dts/u-boot.dtsi ... found: "arch/arm/dts/juno-r2-u-boot.dtsi"
>
>
> +Templates
> +=
> +
> +Sometimes multiple images need to be created which have all have a common
> +part. For example, a board may generate SPI and eMMC images which both
> include
> +a FIT. Since the FIT includes many entries, it is tedious to repeat them
> twice
> +in the image description.
> +
> +Templates provide a simple way to handle this::
> +
> +binman {
> +multiple-images;
> +common_part: template-1 {
> +fit {
> +... lots of entries in here
> +};
> +
> +text {
> +text = "base image";
> +};
> +};
> +
> +spi-image {
> +filename = "image-spi.bin";
> +insert-template = <>;
> +
> +/* things specific to SPI follow */
> +header {
> +];
> +
> +text {
> +text = "SPI image";
> +};
> +};
> +
> +mmc-image {
> +filename = "image-mmc.bin";
> +insert-template = <>;
> +
> +/* things specific to MMC follow */
> +header {
> +];
> +
> +text {
> +text = "MMC image";
> +};
> +};
> +};
> +
> +The template node name must start with 'template', so it is not considered
> to be
> +an image itself.
> +
> +The mechanism is very simple. For each phandle in the 'insert-templates'
> +property, the source node is looked up. Then the subnodes of that source node
> +are copied into the target node, i.e. the one containing the
> `insert-template`
> +property.
> +
> +If the target node has a node with the same name as a template, its
> properties
> +override corresponding properties in the template. This allows the template
> to
> +be uses as a base, with the node providing updates to the properties as
> needed.
> +The overriding happens recursively.
> +
> +At present there is an unpleasant limitation on this feature: it works by
> +appending the template nodes after any existing subnodes to the target node.
> +This means that if the target node includes any subnodes, these appear in
> order
> +before the template node. In the above example, 'header' becomes the first
> +subnode of each image, followed by `fit` and `text`. If this is not what is
> +desired, there is no way to adjust it.
> +
> +Note: The above limitation will likely be removed in future, so that the
> +template subnodes appear before the target subnodes.
> +
> +
> Updating an ELF file
>
>
> diff --git a/tools/binman/control.py b/tools/binman/control.py
> index 68597c4e7792..e7faca78e9aa 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -22,6 +22,7 @@ from binman import bintool
> from binman import cbfs_util
> from binman import elf
> from binman import entry
> +from dtoc import fdt_util
> from u_boot_pylib import command
> from u_boot_pylib import tools
> from u_boot_pylib import tout
> @@ -478,6 +479,12 @@ def SignEntries(image_fname, input_fname,
> privatekey_fname, algo,
Re: [RFC] rockchip: rk3308: fix "same-as-spl" bug in boot devices order
On Mon, Jul 3, 2023 at 6:59 PM Pegorer Massimo wrote: > > Value "same-as-spl" in uboot,spl-boot-order attribute is not working for > boards based on rk3308 due to mismatch between definitions in rk3308.c > and those in rk3308.dtsi: in the first file boot devices are defined as > "/mcc@ff4", while in the DTSI they are "dwmmc@ff4...". > > Of course it could be fixed updating rk3308.c to match DTSI definitions. > Anyway, I've noticed that almost all rockchip DTS/DTSI files, in the past, > have been moved from using "dwmmc" to "mmc". And rk3308.dtsi in Linux > kernel sources too, it has been already updated to "mmc". > > Therefore, other two alternative ways to fix the problem are: replace > "dwmmc" with "mmc" in rk3308.dtsi; align rk3308.dtsi with Linux kernel > one. Which do you suggest? I would prefer the last, but there are several > other differences between u-boot and linux DTSI files. Sync the linux rk3308* dt files and update the drivers appropriately if needed to match. > A second point is: I need to add bootph-all to sdmmc because some models > of Rock Pi S are missing of SDNAND/eMMC and the only bootable device is the > uSD. Is it preferrable to add it in the common rk3308-u-boot.dsi or in the > board specific rk3308-rock-pi-s-u-boot.dtsi? A common rk3308-u-boot.dtsi is probably the most straight forward. Peter
U-Boot v2023.07-rc6 released
Hey all, I've put out v2023.07-rc6 today instead of the release itself. There's two reasons for this. First, the security fix that I merged for -rc5 had some other problems and while I think we have them resolved now, more boot testing cannot be a bad thing. The second one is that Francesco's regression report[1] is something I would really like to get narrowed down to either a specific board issue or a more general problem, and work from there. I'm going to hope that a week is enough time to get things sorted out one way or another. In terms of a changelog, git log --merges v2023.07-rc5..v2023.07-rc6 contains what I've pulled but as always, better PR messages and tags will provide better results here. I plan to do the final release on July 10th, 2023. Thanks all! [1]: https://lore.kernel.org/u-boot/zkl8hqkihrci7...@francesco-nb.int.toradex.com/ -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/3] spl: spl_legacy: Fix spl_end address
On Fri, Jun 30, 2023 at 11:30:53PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam
>
> Currently, spl_end points to the __bss_end address, which
> is an external RAM address instead of the end of the SPL text
> section in the internal RAM.
>
> This causes boot failures on imx6-colibri, for example:
>
> ```
> Trying to boot from MMC1
> SPL: Image overlaps SPL
> resetting ...
> ```
> Fix this problem by assigning spl_end to _image_binary_end, as this
> symbol properly represents the end of the SPL text section.
>
> From u-boot-spl.map:
>
> .end
> *(.__end)
> 0x009121a4_image_binary_end = .
>
> Fixes: 77aed22b48ab ("spl: spl_legacy: Add extra address checks")
> Reported-by: Francesco Dolcini
> Signed-off-by: Fabio Estevam
> Tested-by: Tom Rini
> Reviewed-by: Marek Vasut
> Tested-by: Marek Vasut # DH i.MX6Q DHCOM PDK2
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 2/3] microblaze: u-boot-spl.lds: Pass _image_binary_end
On Fri, Jun 30, 2023 at 11:30:52PM -0300, Fabio Estevam wrote: > From: Fabio Estevam > > Pass _image_binary_end to make a standard way to indicate the end > of the text section in SPL. > > The motivation for this is to have a uniform way to handle > the SPL boundary checks. > > Signed-off-by: Fabio Estevam > Reviewed-by: Marek Vasut Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/3] sunxi: u-boot-spl.lds: Pass _image_binary_end
On Fri, Jun 30, 2023 at 11:30:51PM -0300, Fabio Estevam wrote: > From: Fabio Estevam > > Pass _image_binary_end to make a standard way to indicate the end > of the text section in SPL. > > The motivation for this is to have a uniform way to handle > the SPL boundary checks. > > Signed-off-by: Fabio Estevam > Reviewed-by: Marek Vasut Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] doc: imx: habv4: Fix typo in 'signing'
On Thu, Jun 29, 2023 at 04:25:31PM -0300, Fabio Estevam wrote: > From: Fabio Estevam > > Fix two occurrences where 'signing' is misspelled. > > Signed-off-by: Fabio Estevam > Reviewed-by: Tim Harvey Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] rockchip: Restore support for boot scripts in legacy image format
On Mon, Jun 26, 2023 at 07:43:06PM +, Jonas Karlman wrote:
> Use of CONFIG_SPL_FIT_SIGNATURE=y cause CONFIG_LEGACY_IMAGE_FORMAT=n as
> default, this prevent boot scripts in legacy image format from working
> and was an unintended change in the listed fixes commits:
>
> Wrong image format for "source" command
>
> Add CONFIG_LEGACY_IMAGE_FORMAT=y to defconfig for affected boards to
> restore support for boot scripts in legacy image format.
>
> Fixes: 3bf8e4080763 ("board: rockchip: add Radxa ROCK5B Rk3588 board")
> Fixes: cf777572ca31 ("rockchip: rockpro64: Use SDMA to boost eMMC
> performance")
> Fixes: 6e2b8344d60c ("rockchip: rock-pi-4: Use SDMA to boost eMMC
> performance")
> Fixes: 1bf49d5a4a7c ("rockchip: rk3566-radxa-cm3-io: Update defconfig")
> Fixes: 703c170b40f2 ("rockchip: rk3568-evb: Update defconfig")
> Fixes: 68000f750acd ("rockchip: rk3568-rock-3a: Update defconfig")
> Fixes: 6fb02589a608 ("rockchip: rk3588-evb: Update defconfig")
> Signed-off-by: Jonas Karlman
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
[RFC] rockchip: rk3308: fix "same-as-spl" bug in boot devices order
Value "same-as-spl" in uboot,spl-boot-order attribute is not working for boards based on rk3308 due to mismatch between definitions in rk3308.c and those in rk3308.dtsi: in the first file boot devices are defined as "/mcc@ff4", while in the DTSI they are "dwmmc@ff4...". Of course it could be fixed updating rk3308.c to match DTSI definitions. Anyway, I've noticed that almost all rockchip DTS/DTSI files, in the past, have been moved from using "dwmmc" to "mmc". And rk3308.dtsi in Linux kernel sources too, it has been already updated to "mmc". Therefore, other two alternative ways to fix the problem are: replace "dwmmc" with "mmc" in rk3308.dtsi; align rk3308.dtsi with Linux kernel one. Which do you suggest? I would prefer the last, but there are several other differences between u-boot and linux DTSI files. A second point is: I need to add bootph-all to sdmmc because some models of Rock Pi S are missing of SDNAND/eMMC and the only bootable device is the uSD. Is it preferrable to add it in the common rk3308-u-boot.dsi or in the board specific rk3308-rock-pi-s-u-boot.dtsi? Thanks. Regards, Massimo
Re: v2023.07-rc5 regression: Image overlaps SPL
Short update on this regression.
On Thu, Jun 29, 2023 at 04:19:22PM +0200, Francesco Dolcini wrote:
> I also noticed something weird on a colibri imx7s, this is not using SPL,
> likely something completly different, however given this is new also from rc5
> I
> thought it's valuable to report:
>
> ```
> U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58 +)
> CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> CPU: Extended Commercial temperature grade (-20C to 105C) at 30C
> Reset cause: POR
> DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3)
> ### ERROR ### Please RESET the board ###
> ```
This is unrelated to v2023.07-rc5, the issue was introduced with
8c103c33fb14 ("dm: dts: Convert driver model tags to use new schema"),
that is in since v2023.07-rc1.
Sorry about the initial incomplete/wrong information.
For some reason this affects only imx7s, while imx7d is fine (single
core, only one USB host interface, while the dual variant seems just fine).
Not many ideas for the moment, we might as well be doing something plain
wrong in our board file.
I quickly tried removing the whole `` and it does not seems to
help. I do not think we even enable display output support at the
moment.
Francesco
Re: [PATCH 2/2] mxs: Don't enable 4P2 reg if MXS is powered only from DCDC_BATT
On 7/3/23 18:33, Cody Green wrote:
'mxs_power_enable_4p2()' function call was added to 'mxs_batt_boot()'
in 'commit a0f97610757d' to enable DCDC converter when board is powered
from 5V and has detected sufficient battery voltage.
This involves enabling 4P2 regulator and there is a code
in 'mxs_power_enable_4p2()' that disables VDDIO, VDDA, VDDD outputs of
the DCDC converter and enables BO for each power rail:
setbits_le32(_regs->hw_power_vddioctrl,
POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);
There is no issue if the MXS is powered from the 5V source and linear
regulators are supplying power to the VDDIO, VDDA, VDDD rails.
However, if the MXS is powered only from the DCDC_BATT without 5V,
disabling the DCDC converter outputs causes VDDIO, VDDA, VDDD rails to
lose power.
The proposed solution is not to call the 'mxs_power_enable_4p2()'
function if the MXS is powered only by the DCDC_BATT, because there is
no reason to enable 4P2 regulator in this case. Also 5V brownout should
not be enabled in 'mxs_power_init()' and linear regulator checks should
be disabled in 'mxs_power_set_vddx()'.
Signed-off-by: Cody Green
Cc: Stefano Babic
Cc: Marek Vasut
Cc: Fabio Estevam
---
arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
index 33b76533e4..72172705f2 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
@@ -752,7 +752,9 @@ static void mxs_batt_boot(void)
POWER_5VCTRL_CHARGE_4P2_ILIMIT_MASK,
0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET);
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE
mxs_power_enable_4p2();
+#endif
}
/**
@@ -1137,8 +1139,11 @@ static void mxs_power_set_vddx(const struct mxs_vddx_cfg
*cfg,
cur_target += cfg->lowest_mV;
adjust_up = new_target > cur_target;
+
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE
if (cfg->powered_by_linreg)
powered_by_linreg = cfg->powered_by_linreg();
+#endif
if (adjust_up && cfg->bo_irq) {
if (powered_by_linreg) {
@@ -1269,7 +1274,9 @@ void mxs_power_init(void)
POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ |
POWER_CTRL_DCDC4P2_BO_IRQ, _regs->hw_power_ctrl_clr);
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE
writel(POWER_5VCTRL_PWDN_5VBRNOUT, _regs->hw_power_5vctrl_set);
+#endif
early_delay(1000);
}
+CC Lukasz
Re: [PATCH 1/2] mxs: Fix VDDx brownout interrupt disable/enable
On 7/3/23 18:33, Cody Green wrote: HW_POWER_CTRL register contains brownout interrupt enable bits ENIRQ_VDDIO_BO, ENIRQ_VDDA_BO and ENIRQ_VDDD_BO. So what does this patch do (that is missing in the commit message) ?
[PATCH] board: stm32mp1: add splash screen on dk2
Display the STMicroelectronics logo.
Signed-off-by: Dario Binacchi
---
board/st/stm32mp1/stm32mp1.c | 12
1 file changed, 12 insertions(+)
diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 1a1b1844c8c0..c8c2a83b2acf 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -32,7 +32,10 @@
#include
#include
#include
+#include
+#include
#include
+#include
#include
#include
#include
@@ -684,6 +687,15 @@ int board_init(void)
fw_images[0].fw_name = u"STM32MP-FIP";
fw_images[0].image_index = 1;
#endif
+
+ if (IS_ENABLED(CONFIG_CMD_BMP)) {
+ if (board_is_stm32mp15x_dk2()) {
+ ulong logo =
+ (ulong)stmicroelectronics_uboot_logo_8bit_rle;
+ bmp_display(logo, BMP_ALIGN_CENTER, BMP_ALIGN_CENTER);
+ }
+ }
+
return 0;
}
--
2.32.0
Re: [PATCH v2 0/2] Fix 'no USB device found' error.
On Mon, Jul 03, 2023 at 07:12:47PM +0300, Roger Quadros wrote: > > > On 26/06/2023 10:32, Nishanth Menon wrote: > > On 13:59-20230623, Tom Rini wrote: > >> On Fri, Jun 23, 2023 at 09:42:01AM +0200, Julien Panis wrote: > >>> > >>> > >>> On 6/22/23 17:49, Tom Rini wrote: > On Thu, Jun 22, 2023 at 04:34:34PM +0200, Julien Panis wrote: > > This series fixes usb0 dr_mode for am335x-icev2 > > and am335x-evmsk. It must be set to 'peripheral' > > in order to avoid 'no USB device found' error, > > in usb_ether_init() function. > > > > Signed-off-by: Julien Panis > > --- > > Changes in v2: > > - Drop the modification made in arch/arm/mach-omap2/am33xx/board.c > > - Configure usb0 dr_mode as peripheral in 'am335x-icev2-u-boot.dtsi' > >and 'am335x-evmsk-u-boot.dtsi' device trees. > > - Link to v1: > > https://lore.kernel.org/r/20230621-fix_usb_ether_init-v1-1-215692399...@baylibre.com > > > > --- > > Julien Panis (2): > >arm: dts: am335x-icev2-u-boot: Configure peripheral mode for usb0 > >arm: dts: am335x-evmsk-u-boot: Configure peripheral mode for usb0 > > > > arch/arm/dts/am335x-evmsk-u-boot.dtsi | 4 > > arch/arm/dts/am335x-icev2-u-boot.dtsi | 4 > > 2 files changed, 8 insertions(+) > I'll ask the first question that Nishanth might also ask, which is why > don't these belong in the kernel dts files? Thanks! > > >>> > >>> That's a good question. :) Looping Nishanth... > >>> usb0 dr_mode property is already overlayed for am335x-evm, > >>> in 'am335x-evm-u-boot.dtsi'. So, it appeared more consistent > >>> to me to do the same thing for am335x-icev2 and am335x-evmsk. > >>> I guess that the goal is to configure usb0 as host by default at > >>> kernel boot, since we do not necessarily want to use this usb0 > >>> as peripheral from userspace. > >>> Is it the right explanation @Vignesh @Nishanth ? > >> > >> It's I think even more likely that the am335x_evm fragment needs to go > >> upstream too. > > > > Adding Tony to the thread, but I think it is better to send the changes > > to upstream kernel. > > > > Linux DT files are correct. USB0 is a dual-role port so it sets it to 'otg'. > u-boot doesn't support 'otg' so we need to override it to 'peripheral' in > -u-boot.dtsi Ah, thanks, that was the missing bit of background information. -- Tom signature.asc Description: PGP signature
Re: [PATCHv2 2/3] net/lwip: add README.lwip
On Fri, 30 Jun 2023 at 16:01, Ilias Apalodimas wrote: > Hi Maxim, > > On Thu, Jun 29, 2023 at 06:34:29PM +0600, Maxim Uvarov wrote: > > Just add inital README.lwip doc. > > We'll need a more accurate description for this. The first paragraph of > the documentation you are adding would do > > > > > Signed-off-by: Maxim Uvarov > > --- > > doc/README.lwip | 90 + > > 1 file changed, 90 insertions(+) > > create mode 100644 doc/README.lwip > > > > diff --git a/doc/README.lwip b/doc/README.lwip > > new file mode 100644 > > index 00..16f41049ab > > --- /dev/null > > +++ b/doc/README.lwip > > @@ -0,0 +1,90 @@ > > + RFC LWIP IP stack intergation for U-boot > > + > > + > > +Reliable and bug free IP stack is usually an issue when you are trying > to write it > > I am not sure about the bug free part :). Just move this entire paragraph > as > part of the commit message and remove the 'RFC' parts. > > > +from scratch. It looks like not, but when addinging new features it > will be chelledging. > > +This RFC work is a demo to enable lwIP (lightweight IP) which is a > widely used open-source > > +TCP/IP stack designed for embedded systems for U-boot. That will allow > using already > > +written network applications for microcontrollers. > > + > > +LwIP license: > > +lwIP is licensed under a BSD-style license: > http://lwip.wikia.com/wiki/License. > > + > > +Main features include: > > +- Protocols: IP, IPv6, ICMP, ND, MLD, UDP, TCP, IGMP, ARP, PPPoS, PPPoE > > +- DHCP client, DNS client (incl. mDNS hostname resolver), AutoIP/APIPA > (Zeroconf), SNMP agent (v1, v2c, v3, private MIB support & MIB compiler) > > +- APIs: specialized APIs for enhanced performance, optional > Berkeley-alike socket API > > +- Extended features: IP forwarding over multiple network interfaces, > TCP congestion control, RTT estimation and fast recovery/fast retransmit > > +- Addon applications: HTTP(S) server, SNTP client, SMTP(S) client, > ping, NetBIOS nameserver, mDNS responder, MQTT client, TFTP server > > + > > +U-boot implementation details: > > + > > +1. In general we can build lwIP as .a library and link it against > u-boot or compile it in > > +the U-boot tree in the same way as other U-boot files. There are few > reasons why I selected > > +the second variant: iwIP is very customizable with defines for > features, memory size, types of > > +allocation, some internal types and platform specific code. And it was > more easy to enable/disable > > + debug which is also done with defines, and is needed periodically. > > + > > +2. lwIP has 2 APIs - raw mode and sequential (as lwIP names it, or > socket API as we name it in Linux). > > + This RFC implements only raw API as the proof of work. > > + > > +Raw IP means that the call back function for RX path is registered and > will be called when packet > > +data passes the IP stack and is ready for the application. > > + > > +This RFC has an unmodified working ping example from lwip sources which > registeres this callback. > > + ping_pcb = raw_new(IP_PROTO_ICMP); > > + raw_recv(ping_pcb, ping_recv, NULL); <- ping_recv is app callback. > > + raw_bind(ping_pcb, IP_ADDR_ANY) > > + > > +Socket API also gives nice advantages due it will be easy to port linux > socket applications to u-boot. > > +I.e. lwIP sockets compatible with the linux ones. But that will require > RX thread running in the background. > > +So that means we need some kind of scheduler, locking and threading > support or find some other solution. > > + > > +3. Input and output > > + > > +RX packet path is injected to U-boot eth_rx() polling loop and TX patch > is in eth_send() accordingly. > > +So we do not touch any drivers code and just eat packets when they are > ready. > > + > > +4. Testing > > + > > +Unmodified ping example can be used. I did ping from qemu/u-boot tap > device on the host: > > + > > +=> lwip init > > +=> lwip ping 192.168.1.2 > > +ping: recv 3 ms > > +tcpdump on the host: > > +5:09:10.925951 ARP, Request who-has 192.168.1.200 tell 192.168.1.200, > length 28 15:09:12.114643 IP6 fe80::38e2:41ff:fec3:8bda > ip6-allrouters: > ICMP6, router solicitation, length 16 15:09:20.370725 ARP, Request who-has > 192.168.1.2 tell 192.168.1.200, length 28 15:09:20.370740 ARP, Reply > 192.168.1.2 is-at 3a:e2:41:c3:8b:da (oui Unknown), length 28 > 15:09:20.372789 IP 192.168.1.200 > 192.168.1.2: ICMP echo request, id > 44975, seq 1, length 40 15:09:20.372810 IP 192.168.1.2 > 192.168.1.200: > ICMP echo reply, id 44975, seq 1, length 40 15:09:25.426636 ARP, Request > who-has 192.168.1.200 tell 192.168.1.2, length 28 15:09:25.426870 ARP, > Reply 192.168.1.200 is-at f6:11:01:02:03:04 (oui Unknown), length 28 > > + > > + > > +5. Wget example > > + > > +Http server has 192.168.1.2 IP address. (I did not port DNS resolving > yet, > > +but it's also exist in lwip.) So example just downloads some file with > http > >
Re: [PATCH v2 0/2] Fix 'no USB device found' error.
On 26/06/2023 10:32, Nishanth Menon wrote: > On 13:59-20230623, Tom Rini wrote: >> On Fri, Jun 23, 2023 at 09:42:01AM +0200, Julien Panis wrote: >>> >>> >>> On 6/22/23 17:49, Tom Rini wrote: On Thu, Jun 22, 2023 at 04:34:34PM +0200, Julien Panis wrote: > This series fixes usb0 dr_mode for am335x-icev2 > and am335x-evmsk. It must be set to 'peripheral' > in order to avoid 'no USB device found' error, > in usb_ether_init() function. > > Signed-off-by: Julien Panis > --- > Changes in v2: > - Drop the modification made in arch/arm/mach-omap2/am33xx/board.c > - Configure usb0 dr_mode as peripheral in 'am335x-icev2-u-boot.dtsi' >and 'am335x-evmsk-u-boot.dtsi' device trees. > - Link to v1: > https://lore.kernel.org/r/20230621-fix_usb_ether_init-v1-1-215692399...@baylibre.com > > --- > Julien Panis (2): >arm: dts: am335x-icev2-u-boot: Configure peripheral mode for usb0 >arm: dts: am335x-evmsk-u-boot: Configure peripheral mode for usb0 > > arch/arm/dts/am335x-evmsk-u-boot.dtsi | 4 > arch/arm/dts/am335x-icev2-u-boot.dtsi | 4 > 2 files changed, 8 insertions(+) I'll ask the first question that Nishanth might also ask, which is why don't these belong in the kernel dts files? Thanks! >>> >>> That's a good question. :) Looping Nishanth... >>> usb0 dr_mode property is already overlayed for am335x-evm, >>> in 'am335x-evm-u-boot.dtsi'. So, it appeared more consistent >>> to me to do the same thing for am335x-icev2 and am335x-evmsk. >>> I guess that the goal is to configure usb0 as host by default at >>> kernel boot, since we do not necessarily want to use this usb0 >>> as peripheral from userspace. >>> Is it the right explanation @Vignesh @Nishanth ? >> >> It's I think even more likely that the am335x_evm fragment needs to go >> upstream too. > > Adding Tony to the thread, but I think it is better to send the changes > to upstream kernel. > Linux DT files are correct. USB0 is a dual-role port so it sets it to 'otg'. u-boot doesn't support 'otg' so we need to override it to 'peripheral' in -u-boot.dtsi -- cheers, -roger
Re: [PATCH v2 0/2] Fix 'no USB device found' error.
On 22/06/2023 17:34, Julien Panis wrote: > This series fixes usb0 dr_mode for am335x-icev2 > and am335x-evmsk. It must be set to 'peripheral' > in order to avoid 'no USB device found' error, > in usb_ether_init() function. > > Signed-off-by: Julien Panis > --- > Changes in v2: > - Drop the modification made in arch/arm/mach-omap2/am33xx/board.c > - Configure usb0 dr_mode as peripheral in 'am335x-icev2-u-boot.dtsi' > and 'am335x-evmsk-u-boot.dtsi' device trees. > - Link to v1: > https://lore.kernel.org/r/20230621-fix_usb_ether_init-v1-1-215692399...@baylibre.com > > --- > Julien Panis (2): > arm: dts: am335x-icev2-u-boot: Configure peripheral mode for usb0 > arm: dts: am335x-evmsk-u-boot: Configure peripheral mode for usb0 > > arch/arm/dts/am335x-evmsk-u-boot.dtsi | 4 > arch/arm/dts/am335x-icev2-u-boot.dtsi | 4 > 2 files changed, 8 insertions(+) For this series: Reviewed-by: Roger Quadros -- cheers, -roger
[RESEND PATCH] ARM: dts: stm32: fix display pinmux for stm32f746-disco
As reported by the datasheet (DocID027590 Rev 4) for PG12:
- AF9 -> LCD_B4
- AF14 -> LCD_B1
So replace AF14 with AF9 for PG12 in the dts.
Fixes: fe63d3cfb77ef ("ARM: dts: stm32: Sync DT with v4.20 kernel for stm32f7")
Signed-off-by: Dario Binacchi
---
arch/arm/dts/stm32f746-disco-u-boot.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/stm32f746-disco-u-boot.dtsi
b/arch/arm/dts/stm32f746-disco-u-boot.dtsi
index 19b5451db441..522cffb1ac9f 100644
--- a/arch/arm/dts/stm32f746-disco-u-boot.dtsi
+++ b/arch/arm/dts/stm32f746-disco-u-boot.dtsi
@@ -169,7 +169,7 @@
ltdc_pins: ltdc@0 {
pins {
pinmux = , /* B0 */
-, /* B4 */
+, /* B4 */
, /* VSYNC */
, /* HSYNC */
, /* CLK */
--
2.32.0
Re: [PATCH v13 05/10] arm_ffa: introduce armffa command
Hi Simon, Ilias,
On Mon, Jul 03, 2023 at 02:30:51PM +0100, Simon Glass wrote:
...
> > > > > > + log_info("device name %s, dev %p, driver name %s, ops %p\n",
> > > > > > +dev->name,
> > > > > > + (void *)map_to_sysmem(dev),
> > > > > > +dev->driver->name,
> > > > > > +(void *)map_to_sysmem(dev->driver->ops));
> > > > >
> > > > > Isn't it more useful to print the physical address map_to_sysmem()
> > > > > retuns?
> > > >
> > > > That's what map_to_sysmem() does, it returns a physical address and
> > > > it's shown in the log.
> > >
> > > I dont have access to u-boot source right, but why do you need all
> > > these void * casts then?
> >
> > Because map_to_sysmem() returns an 'phys_addr_t' (aka 'long long unsigned
> > int') . However, %p expects 'void *'.
> >
> > Compilation warning:
> >
> > cmd/armffa.c:181:18: warning: format ‘%p’ expects argument of type ‘void
> > *’, but argument 3 has type ‘phys_addr_t’ {aka ‘long long unsigned int’} [8;
> > ;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat=-Wformat=8;;]
> > 181 | log_info("device name %s, dev %p, driver name %s, ops %p\n",
> > | ^~
> > 182 | dev->name,
> > 183 | map_to_sysmem(dev),
> > | ~~
> > | |
> > | phys_addr_t {aka long long unsigned int}
>
> You should be using %lx with map_to_sysmen() since it returns a . Use
> %p for pointers.
>
> In general in U-Boot we use addresses (ulong) rather than pointers.
> Unfortunately the phys_addr_t type can be larger than the word size. I
> suggest creating a printf prefix for that type. See the top of blk.h
> for an example of how to do it. Perhaps in a follow-up patch?
>
Sounds good. I'm gonna add it in a generic way under include/log.h if that
makes sense:
#ifdef CONFIG_PHYS_64BIT
#define PhysAddrLength "ll"
#else
#define PhysAddrLength ""
#endif
#define PHYS_ADDR_LN "%" PhysAddrLength "x"
#define PHYS_ADDR_LNU "%" PhysAddrLength "u"
#endif
Note: In a 32-bit platform phys_addr_t is "unsigned int", in a 64-bit platform
it is "long long unsigned int"
Then using it this way:
log_info("device %s, addr " PHYS_ADDR_LN ", driver %s, ops "
PHYS_ADDR_LN "\n",
dev->name,
map_to_sysmem(dev),
dev->driver->name,
map_to_sysmem(dev->driver->ops));
Cheers
Abdellatif
Re: [PATCH 5/6] net: add fastboot TCP documentation and IP6-only mode
Reviewed-by: Ying-Chun Liu (PaulLiu)
On Tue, 9 May 2023 at 01:16, Dmitrii Merkurev wrote:
> Command to start IP6 only TCP fastboot:
> fastboot tcp -ipv6
>
> Signed-off-by: Dmitrii Merkurev
> Cc: Ying-Chun Liu (PaulLiu)
> Cc: Simon Glass
> Сс: Joe Hershberger
> Сс: Ramon Fried
> ---
> cmd/fastboot.c | 29 +
> doc/android/fastboot.rst | 8 +++-
> 2 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> index 3d5ff951eb..36f744ae01 100644
> --- a/cmd/fastboot.c
> +++ b/cmd/fastboot.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -121,10 +122,23 @@ static int do_fastboot(struct cmd_tbl *cmdtp, int
> flag, int argc,
> {
> uintptr_t buf_addr = (uintptr_t)NULL;
> size_t buf_size = 0;
> + bool is_ipv6_only = false;
> + bool is_usb = false;
> + bool is_udp = false;
> + bool is_tcp = false;
>
> if (argc < 2)
> return CMD_RET_USAGE;
>
> + if (IS_ENABLED(CONFIG_IPV6)) {
> + use_ip6 = false;
> + /* IPv6 parameter has to be always *last* */
> + if (!strcmp(argv[argc - 1], USE_IP6_CMD_PARAM)) {
> + is_ipv6_only = true;
> + --argc;
> + }
> + }
> +
> while (argc > 1 && **(argv + 1) == '-') {
> char *arg = *++argv;
>
> @@ -159,11 +173,18 @@ NXTARG:
>
> fastboot_init((void *)buf_addr, buf_size);
>
> - if (!strcmp(argv[1], "udp"))
> + is_usb = strcmp(argv[1], "usb") == 0;
> + is_udp = strcmp(argv[1], "udp") == 0;
> + is_tcp = strcmp(argv[1], "tcp") == 0;
> +
> + if (is_ipv6_only && is_tcp)
> + use_ip6 = true;
> +
> + if (is_udp)
> return do_fastboot_udp(argc, argv, buf_addr, buf_size);
> - if (!strcmp(argv[1], "tcp"))
> + if (is_tcp)
> return do_fastboot_tcp(argc, argv, buf_addr, buf_size);
> - if (!strcmp(argv[1], "usb")) {
> + if (is_usb) {
> argv++;
> argc--;
> }
> @@ -174,7 +195,7 @@ NXTARG:
> U_BOOT_CMD(
> fastboot, CONFIG_SYS_MAXARGS, 1, do_fastboot,
> "run as a fastboot usb or udp device",
> - "[-l addr] [-s size] usb | udp\n"
> + "[-l addr] [-s size] usb | udp [-ipv6] | tcp
> [-ipv6]\n"
> "\taddr - address of buffer used during data transfers ("
> __stringify(CONFIG_FASTBOOT_BUF_ADDR) ")\n"
> "\tsize - size of buffer used during data transfers ("
> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
> index 1ad8a897c8..aa6e9e5a9e 100644
> --- a/doc/android/fastboot.rst
> +++ b/doc/android/fastboot.rst
> @@ -181,13 +181,19 @@ Enter into fastboot by executing the fastboot
> command in U-Boot for either USB::
>
> => fastboot usb 0
>
> -or UDP::
> +UDP::
>
> => fastboot udp
> link up on port 0, speed 100, full duplex
> Using ethernet@4a10 device
> Listening for fastboot command on 192.168.0.102
>
> +or TCP::
> +
> + => fastboot tcp
> + Using ethernet@4a10 device
> + Listening for fastboot command on 192.168.0.102
> +
> On the client side you can fetch the bootloader version for instance::
>
> $ fastboot getvar version-bootloader
> --
> 2.40.1.521.gf1e218fcd8-goog
>
>
Re: [PATCH v2 4/6] net: add fastboot TCP6 support
Reviewed-by: Ying-Chun Liu (PaulLiu)
On Thu, 11 May 2023 at 01:00, Dmitrii Merkurev wrote:
> fastboot tcp command remains the same, but started
> listening IP6 in case it's enabled.
>
> Signed-off-by: Dmitrii Merkurev
> Cc: Ying-Chun Liu (PaulLiu)
> Cc: Simon Glass
> Сс: Joe Hershberger
> Сс: Ramon Fried
> ---
> include/net/fastboot_tcp.h | 2 +-
> net/fastboot_tcp.c | 72 --
> 2 files changed, 62 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/fastboot_tcp.h b/include/net/fastboot_tcp.h
> index 6cf29d52e9..98986fa10a 100644
> --- a/include/net/fastboot_tcp.h
> +++ b/include/net/fastboot_tcp.h
> @@ -7,7 +7,7 @@
> #define __NET_FASTBOOT_TCP_H__
>
> /**
> - * Wait for incoming tcp fastboot comands.
> + * Wait for incoming TCP fastboot comands.
> */
> void fastboot_tcp_start_server(void);
>
> diff --git a/net/fastboot_tcp.c b/net/fastboot_tcp.c
> index 2eb52ea256..d93b52ede5 100644
> --- a/net/fastboot_tcp.c
> +++ b/net/fastboot_tcp.c
> @@ -6,8 +6,10 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> +#include
>
> static char command[FASTBOOT_COMMAND_LEN] = {0};
> static char response[FASTBOOT_RESPONSE_LEN] = {0};
> @@ -20,18 +22,30 @@ static u16 curr_dport;
> static u32 curr_tcp_seq_num;
> static u32 curr_tcp_ack_num;
> static unsigned int curr_request_len;
> +static bool is_ipv6;
> +static size_t ip_header_size;
> static enum fastboot_tcp_state {
> FASTBOOT_CLOSED,
> FASTBOOT_CONNECTED,
> FASTBOOT_DISCONNECTING
> } state = FASTBOOT_CLOSED;
>
> +static int command_handled_id;
> +static bool command_handled_success;
> +
> static void fastboot_tcp_answer(u8 action, unsigned int len)
> {
> const u32 response_seq_num = curr_tcp_ack_num;
> const u32 response_ack_num = curr_tcp_seq_num +
> (curr_request_len > 0 ? curr_request_len : 1);
>
> +#if defined(CONFIG_IPV6)
> + if (is_ipv6) {
> + net_send_tcp_packet6(len, htons(curr_sport),
> htons(curr_dport),
> +action, response_seq_num,
> response_ack_num);
> + return;
> + }
> +#endif
> net_send_tcp_packet(len, htons(curr_sport), htons(curr_dport),
> action, response_seq_num, response_ack_num);
> }
> @@ -47,7 +61,7 @@ static void fastboot_tcp_send_packet(u8 action, const
> uchar *data, unsigned int
> uchar *pkt = net_get_async_tx_pkt_buf();
>
> memset(pkt, '\0', PKTSIZE);
> - pkt += net_eth_hdr_size() + IP_TCP_HDR_SIZE + TCP_TSOPT_SIZE + 2;
> + pkt += net_eth_hdr_size() + ip_header_size + TCP_HDR_SIZE +
> TCP_TSOPT_SIZE + 2;
> memcpy(pkt, data, len);
> fastboot_tcp_answer(action, len);
> memset(pkt, '\0', PKTSIZE);
> @@ -59,7 +73,7 @@ static void fastboot_tcp_send_message(const char
> *message, unsigned int len)
> uchar *pkt = net_get_async_tx_pkt_buf();
>
> memset(pkt, '\0', PKTSIZE);
> - pkt += net_eth_hdr_size() + IP_TCP_HDR_SIZE + TCP_TSOPT_SIZE + 2;
> + pkt += net_eth_hdr_size() + ip_header_size + TCP_HDR_SIZE +
> TCP_TSOPT_SIZE + 2;
> // Put first 8 bytes as a big endian message length
> memcpy(pkt, _be, 8);
> pkt += 8;
> @@ -68,10 +82,9 @@ static void fastboot_tcp_send_message(const char
> *message, unsigned int len)
> memset(pkt, '\0', PKTSIZE);
> }
>
> -static void fastboot_tcp_handler_ipv4(uchar *pkt, u16 dport,
> - struct in_addr sip, u16 sport,
> - u32 tcp_seq_num, u32 tcp_ack_num,
> - u8 action, unsigned int len)
> +static void fastboot_tcp_handler(uchar *pkt, u16 dport, u16 sport,
> +u32 tcp_seq_num, u32 tcp_ack_num,
> +u8 action, unsigned int len)
> {
> int fastboot_command_id;
> u64 command_size;
> @@ -88,7 +101,6 @@ static void fastboot_tcp_handler_ipv4(uchar *pkt, u16
> dport,
> case FASTBOOT_CLOSED:
> if (tcp_push) {
> if (len != handshake_length ||
> - strlen(pkt) != handshake_length ||
> memcmp(pkt, handshake, handshake_length) != 0)
> {
> fastboot_tcp_reset();
> break;
> @@ -111,18 +123,25 @@ static void fastboot_tcp_handler_ipv4(uchar *pkt,
> u16 dport,
> pkt += 8;
>
> // Only single packet messages are supported ATM
> - if (strlen(pkt) != command_size) {
> + if (len != command_size) {
> fastboot_tcp_reset();
> break;
> }
> strlcpy(command, pkt, len + 1);
> fastboot_command_id =
>
[PATCH] android_ab: Try backup booloader_message
Some devices keep 2 copies of the bootloader_message in the misc
partition and write each in sequence when updating. This ensures that
there is always one valid copy of the bootloader_message. Teach u-boot
to optionally try a backup bootloader_message from a specified offset if
the primary one fails its CRC check.
Signed-off-by: Joshua Watt
---
boot/android_ab.c | 77 ++
common/Kconfig | 9 ++
doc/android/ab.rst | 6
3 files changed, 80 insertions(+), 12 deletions(-)
diff --git a/boot/android_ab.c b/boot/android_ab.c
index 60ae002978..73b55c196c 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -85,11 +85,13 @@ static int ab_control_default(struct bootloader_control
*abc)
*/
static int ab_control_create_from_disk(struct blk_desc *dev_desc,
const struct disk_partition *part_info,
- struct bootloader_control **abc)
+ struct bootloader_control **abc,
+ ulong offset)
{
ulong abc_offset, abc_blocks, ret;
- abc_offset = offsetof(struct bootloader_message_ab, slot_suffix);
+ abc_offset = offset +
+offsetof(struct bootloader_message_ab, slot_suffix);
if (abc_offset % part_info->blksz) {
log_err("ANDROID: Boot control block not block aligned.\n");
return -EINVAL;
@@ -135,11 +137,12 @@ static int ab_control_create_from_disk(struct blk_desc
*dev_desc,
*/
static int ab_control_store(struct blk_desc *dev_desc,
const struct disk_partition *part_info,
- struct bootloader_control *abc)
+ struct bootloader_control *abc, ulong offset)
{
ulong abc_offset, abc_blocks, ret;
- abc_offset = offsetof(struct bootloader_message_ab, slot_suffix) /
+ abc_offset = offset +
+offsetof(struct bootloader_message_ab, slot_suffix) /
part_info->blksz;
abc_blocks = DIV_ROUND_UP(sizeof(struct bootloader_control),
part_info->blksz);
@@ -189,8 +192,11 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
disk_partition *part_info,
int slot, i, ret;
bool store_needed = false;
char slot_suffix[4];
+#if ANDROID_AB_BACKUP_OFFSET
+ struct bootloader_control *backup_abc = NULL;
+#endif
- ret = ab_control_create_from_disk(dev_desc, part_info, );
+ ret = ab_control_create_from_disk(dev_desc, part_info, , 0);
if (ret < 0) {
/*
* This condition represents an actual problem with the code or
@@ -200,22 +206,53 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
disk_partition *part_info,
return ret;
}
+#if ANDROID_AB_BACKUP_OFFSET
+ ret = ab_control_create_from_disk(dev_desc, part_info, _abc,
+ ANDROID_AB_BACKUP_OFFSET);
+ if (ret < 0) {
+ free(abc);
+ return ret;
+ }
+#endif
+
crc32_le = ab_control_compute_crc(abc);
if (abc->crc32_le != crc32_le) {
log_err("ANDROID: Invalid CRC-32 (expected %.8x, found %.8x),",
crc32_le, abc->crc32_le);
- log_err("re-initializing A/B metadata.\n");
-
- ret = ab_control_default(abc);
- if (ret < 0) {
- free(abc);
- return -ENODATA;
+#if ANDROID_AB_BACKUP_OFFSET
+ crc32_le = ab_control_compute_crc(backup_abc);
+ if (backup_abc->crc32_le != crc32_le) {
+ log_err("ANDROID: Invalid backup CRC-32 ")
+ log_err("expected %.8x, found %.8x),",
+ crc32_le, backup_abc->crc32_le);
+#endif
+
+ log_err("re-initializing A/B metadata.\n");
+
+ ret = ab_control_default(abc);
+ if (ret < 0) {
+#if ANDROID_AB_BACKUP_OFFSET
+ free(backup_abc);
+#endif
+ free(abc);
+ return -ENODATA;
+ }
+#if ANDROID_AB_BACKUP_OFFSET
+ } else {
+ /*
+* Backup is valid. Copy it to the primary
+*/
+ memcpy(abc, backup_abc, sizeof(*abc));
}
+#endif
store_needed = true;
}
if (abc->magic != BOOT_CTRL_MAGIC) {
log_err("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
+#if ANDROID_AB_BACKUP_OFFSET
+ free(backup_abc);
+#endif
free(abc);
return -ENODATA;
}
@@ -223,6 +260,9 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
Re: [PATCH v2] dt-bindings: riscv: deprecate riscv,isa
On 08/06/2023 17:54, Conor Dooley wrote: From: Conor Dooley intro = When the RISC-V dt-bindings were accepted upstream in Linux, the base ISA etc had yet to be ratified. By the ratification of the base ISA, incompatible changes had snuck into the specifications - for example the Zicsr and Zifencei extensions were spun out of the base ISA. Fast forward to today, and the reason for this patch. Currently the riscv,isa dt property permits only a specific subset of the ISA string - in particular it excludes version numbering. With the current constraints, it is not possible to discern whether "rv64i" means that the hart supports the fence.i instruction, for example. Future systems may choose to implement their own instruction fencing, perhaps using a vendor extension, or they may not implement the optional counter extensions. Software needs a way to determine this. versioning schemes == "Use the extension versions that are described in the ISA manual" you may say, and it's not like this has not been considered. Firstly, software that parses the riscv,isa property at runtime will need to contain a lookup table of some sort that maps arbitrary versions to versions it understands. There is not a consistent application of version number applied to extensions, with a higgledy-piggledy collection of tags, "bare" and version documents awaiting the reader on the "recently ratified extensions" page: https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions As an aside, this is reflected in the patch too, since many extensions have yet to appear in a release of the ISA specs, and are defined by commits in their respective "working draft" repositories. Secondly, there is an issue of backwards compatibility, whereby allowing numbers in the ISA string, some parsers may be broken. This would require an additional property to be created to even use the versions in this manner. boolean properties == If a new property is needed, the whole approach may as well be looked at from the bottom up. A string with limited character choices etc is hardly the best approach for communicating extension information to software. Switching to using boolean properties, one per extension, allows us to define explicit meanings for the DT representation of each extension - rather than the current situation where different operating systems or other bits of software may impart different meanings to characters in the string. Clearly the best source of meanings is the specifications themselves, this just provides us the ability to choose at what point in time the meaning is set. If an extension changes incompatibility in the future, a new property will be required. Off-list, some of the RVI folks have committed to shoring up the wording in either the ISA specifications, the riscv-isa-manual or so that in the future, modifications to and additions or removals of features will require a new extension. Codifying that assertion somewhere would make it quite unlikely that compatibility would be broken, but we have the tools required to deal with it, if & when it crops up. It is in our collective interest, as consumers of extension meanings, to define a scheme that enforces compatibility. The use of boolean properties, rather than elements in a string, will also permit validation that the strings have a meaning, as well as potentially reject mutually exclusive combinations, or enforce dependencies between instructions. That would not be possible with the current dt-schema infrastructure for arbitrary strings, as we would need to add a riscv,isa parser to dt-validate! That's not implemented in this patch, but rather left as future work (for the brave, or the foolish). acpi The current ACPI ECR is based on having a string unfortunately, but ideally ACPI will move to another method, perhaps GUIDs, that give explicit meaning to extensions. parser simplicity = Many systems that parse DT at runtime already implement an function that can check for the presence of boolean properties, rather than having to implement - although unfortunately for backwards compatibility with old dtbs, existing parsers may not be removable - which may greatly simplify dt parsing code. For example, in Linux, checking for an extension becomes as simple as: of_property_present(extension_node, "zicbom") vendor extensions = Compared to riscv,isa, this proposed scheme promotes vendor extensions, oft touted as the strength of RISC-V, to first-class citizens. At present, extensions are defined as meaning what the RISC-V ISA specifications say they do. There is no realistic way of using that interface to provide cross-platform definitions for what vendor extensions mean. Vendor extensions may also have even less consistency than RVI do in terms of versioning, or no care about backwards compatibility. A boolean property allows us to assign explicit
[PATCH v2 5/5] cmd: mbr: Force DOS driver to be used for verify
Forces the DOS partition type driver to be used when verifying the MBR.
This is particularly useful when using a hybrid MBR & GPT layout as
otherwise MBR verification would mostly likely fail since the GPT
partitions will be returned, even if the MBR is actually valid.
Signed-off-by: Joshua Watt
---
cmd/mbr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/mbr.c b/cmd/mbr.c
index c269833eb8..ec99b66283 100644
--- a/cmd/mbr.c
+++ b/cmd/mbr.c
@@ -244,7 +244,7 @@ static int do_verify_mbr(struct blk_desc *dev, const char
*str)
for (i = 0; i < count; i++) {
struct disk_partition p;
- if (part_get_info(dev, i + 1, ))
+ if (part_get_info_by_type(dev, i + 1, PART_TYPE_DOS, ))
goto fail;
if ((partitions[i].size && p.size != partitions[i].size) ||
--
2.33.0
[PATCH v2 4/5] dm: test: Add test for part_get_info_by_type
Adds a test suite to ensure that part_get_info_by_type works correctly
by creating a hybrid GPT/MBR partition table and reading both.
Signed-off-by: Joshua Watt
---
configs/sandbox_defconfig | 2 +
test/dm/part.c| 87 +++
2 files changed, 89 insertions(+)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 1ec44d5b33..cbc5d5b895 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -71,6 +71,7 @@ CONFIG_CMD_GPIO_READ=y
CONFIG_CMD_PWM=y
CONFIG_CMD_GPT=y
CONFIG_CMD_GPT_RENAME=y
+CONFIG_CMD_MBR=y
CONFIG_CMD_IDE=y
CONFIG_CMD_I2C=y
CONFIG_CMD_LOADM=y
@@ -131,6 +132,7 @@ CONFIG_CMD_MTDPARTS=y
CONFIG_CMD_STACKPROTECTOR_TEST=y
CONFIG_MAC_PARTITION=y
CONFIG_AMIGA_PARTITION=y
+CONFIG_DOS_PARTITION=y
CONFIG_OF_CONTROL=y
CONFIG_OF_LIVE=y
CONFIG_ENV_IS_NOWHERE=y
diff --git a/test/dm/part.c b/test/dm/part.c
index 7dd8bc7a3c..d6e4345812 100644
--- a/test/dm/part.c
+++ b/test/dm/part.c
@@ -108,3 +108,90 @@ static int dm_test_part_bootable(struct unit_test_state
*uts)
return 0;
}
DM_TEST(dm_test_part_bootable, UT_TESTF_SCAN_FDT);
+
+static int do_get_info_test(struct unit_test_state *uts,
+ struct blk_desc *dev_desc, int part, int part_type,
+ struct disk_partition const *reference)
+{
+ struct disk_partition p;
+ int ret;
+
+ memset(, 0, sizeof(p));
+
+ ret = part_get_info_by_type(dev_desc, part, part_type, );
+ printf("part_get_info_by_type(%d, 0x%x) = %d\n", part, part_type, ret);
+ if (ut_assertok(ret)) {
+ return 0;
+ }
+
+ ut_asserteq(reference->start, p.start);
+ ut_asserteq(reference->size, p.size);
+ ut_asserteq(reference->sys_ind, p.sys_ind);
+
+ return 0;
+}
+
+static int dm_test_part_get_info_by_type(struct unit_test_state *uts)
+{
+ char str_disk_guid[UUID_STR_LEN + 1];
+ struct blk_desc *mmc_dev_desc;
+ struct disk_partition gpt_parts[] = {
+ {
+ .start = 48, /* GPT data takes up the first 34 blocks
or so */
+ .size = 1,
+ .name = "test1",
+ .sys_ind = 0,
+ },
+ {
+ .start = 49,
+ .size = 1,
+ .name = "test2",
+ .sys_ind = 0,
+ },
+ };
+ struct disk_partition mbr_parts[] = {
+ {
+ .start = 1,
+ .size = 33,
+ .name = "gpt",
+ .sys_ind = EFI_PMBR_OSTYPE_EFI_GPT,
+ },
+ {
+ .start = 48,
+ .size = 1,
+ .name = "test1",
+ .sys_ind = 0x83,
+ },
+ };
+
+ ut_asserteq(2, blk_get_device_by_str("mmc", "2", _dev_desc));
+ if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
+ gen_rand_uuid_str(gpt_parts[0].uuid, UUID_STR_FORMAT_STD);
+ gen_rand_uuid_str(gpt_parts[1].uuid, UUID_STR_FORMAT_STD);
+ gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
+ }
+ ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, gpt_parts,
+ ARRAY_SIZE(gpt_parts)));
+
+ ut_assertok(write_mbr_partitions(mmc_dev_desc, mbr_parts,
+ARRAY_SIZE(mbr_parts), 0));
+
+#define get_info_test(_part, _part_type, _reference) \
+ ut_assertok(do_get_info_test(uts, mmc_dev_desc, _part, _part_type, \
+_reference))
+
+ for (int i = 0; i < ARRAY_SIZE(gpt_parts); i++) {
+ get_info_test(i + 1, PART_TYPE_UNKNOWN, _parts[i]);
+ }
+
+ for (int i = 0; i < ARRAY_SIZE(mbr_parts); i++) {
+ get_info_test(i + 1, PART_TYPE_DOS, _parts[i]);
+ }
+
+ for (int i = 0; i < ARRAY_SIZE(gpt_parts); i++) {
+ get_info_test(i + 1, PART_TYPE_EFI, _parts[i]);
+ }
+
+ return 0;
+}
+DM_TEST(dm_test_part_get_info_by_type, UT_TESTF_SCAN_PDATA |
UT_TESTF_SCAN_FDT);
--
2.33.0
[PATCH v2 3/5] disk: part: Add API to get partitions with specific driver
Adds part_driver_get_type() API which can be used to force a specific
driver to be used when getting partition information instead of relying
on auto detection.
Signed-off-by: Joshua Watt
---
disk/part.c| 38 +++---
include/part.h | 19 ++-
2 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 35300df590..1f8c786ca5 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -26,6 +26,22 @@
/* Check all partition types */
#define PART_TYPE_ALL -1
+static struct part_driver *part_driver_get_type(int part_type)
+{
+ struct part_driver *drv =
+ ll_entry_start(struct part_driver, part_driver);
+ const int n_ents = ll_entry_count(struct part_driver, part_driver);
+ struct part_driver *entry;
+
+ for (entry = drv; entry != drv + n_ents; entry++) {
+ if (part_type == entry->part_type)
+ return entry;
+ }
+
+ /* Not found */
+ return NULL;
+}
+
static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc)
{
struct part_driver *drv =
@@ -44,10 +60,7 @@ static struct part_driver *part_driver_lookup_type(struct
blk_desc *dev_desc)
}
}
} else {
- for (entry = drv; entry != drv + n_ents; entry++) {
- if (dev_desc->part_type == entry->part_type)
- return entry;
- }
+ return part_driver_get_type(dev_desc->part_type);
}
/* Not found */
@@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc)
drv->print(dev_desc);
}
-int part_get_info(struct blk_desc *dev_desc, int part,
- struct disk_partition *info)
+int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
+ struct disk_partition *info)
{
struct part_driver *drv;
@@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part,
info->type_guid[0] = 0;
#endif
- drv = part_driver_lookup_type(dev_desc);
+ if (part_type == PART_TYPE_UNKNOWN) {
+ drv = part_driver_lookup_type(dev_desc);
+ } else {
+ drv = part_driver_get_type(part_type);
+ }
+
if (!drv) {
debug("## Unknown partition table type %x\n",
dev_desc->part_type);
@@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part,
return -ENOENT;
}
+int part_get_info(struct blk_desc *dev_desc, int part,
+ struct disk_partition *info)
+{
+ return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, info);
+}
+
int part_get_info_whole_disk(struct blk_desc *dev_desc,
struct disk_partition *info)
{
diff --git a/include/part.h b/include/part.h
index be75c73549..8a0c8732a6 100644
--- a/include/part.h
+++ b/include/part.h
@@ -105,7 +105,24 @@ struct blk_desc *blk_get_dev(const char *ifname, int dev);
struct blk_desc *mg_disk_get_dev(int dev);
-/* disk/part.c */
+/**
+ * part_get_info_by_type() - Get partitions from a block device using a
specific
+ * partition driver
+ *
+ * Each interface allocates its own devices and typically struct blk_desc is
+ * contained with the interface's data structure. There is no global
+ * numbering for block devices, so the interface name must be provided.
+ *
+ * @dev_desc: Block device descriptor
+ * @part: Partition number to read
+ * @part_type: Partition driver to use, or PART_TYPE_UNKNOWN to automatically
+ * choose a driver
+ * @info: Returned partition information
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
+ struct disk_partition *info);
int part_get_info(struct blk_desc *dev_desc, int part,
struct disk_partition *info);
/**
--
2.33.0
[PATCH v2 2/5] dm: test: Improve partition test error output
Improve the logging when the partition test fails so that it is clear
what went wrong, shown with actual values.
Signed-off-by: Joshua Watt
---
test/dm/part.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/test/dm/part.c b/test/dm/part.c
index d5c58d6e3f..7dd8bc7a3c 100644
--- a/test/dm/part.c
+++ b/test/dm/part.c
@@ -17,10 +17,12 @@ static int do_test(struct unit_test_state *uts, int
expected,
struct blk_desc *mmc_dev_desc;
struct disk_partition part_info;
- ut_asserteq(expected,
- part_get_info_by_dev_and_name_or_num("mmc", part_str,
-_dev_desc,
-_info, whole));
+ int ret = part_get_info_by_dev_and_name_or_num("mmc", part_str,
+ _dev_desc,
+ _info, whole);
+
+ ut_assertf(expected == ret, "test(%d, \"%s\", %d) == %d", expected,
+ part_str, whole, ret);
return 0;
}
--
2.33.0
[PATCH v2 1/5] dm: test: Fix partition test to use mmc2
d94d9844bc ("dm: part: Update test to use mmc2") attempted to make the
test use mmc2, but the change was incomplete in that it didn't also
change the strings that reference a specific partition. Fix these so
that the test passes again
Signed-off-by: Joshua Watt
---
test/dm/part.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/test/dm/part.c b/test/dm/part.c
index 35e99eeb01..d5c58d6e3f 100644
--- a/test/dm/part.c
+++ b/test/dm/part.c
@@ -76,15 +76,15 @@ static int dm_test_part(struct unit_test_state *uts)
test(-EINVAL, "#test1", true);
test(1, "2", false);
test(1, "2", true);
- test(-ENOENT, "1:0", false);
- test(0, "1:0", true);
- test(1, "1:1", false);
- test(2, "1:2", false);
- test(1, "1.0", false);
- test(0, "1.0:0", true);
- test(1, "1.0:1", false);
- test(2, "1.0:2", false);
- test(-EINVAL, "1#bogus", false);
+ test(-ENOENT, "2:0", false);
+ test(0, "2:0", true);
+ test(1, "2:1", false);
+ test(2, "2:2", false);
+ test(1, "2.0", false);
+ test(0, "2.0:0", true);
+ test(1, "2.0:1", false);
+ test(2, "2.0:2", false);
+ test(-EINVAL, "2#bogus", false);
test(1, "2#test1", false);
test(2, "2#test2", false);
ret = 0;
--
2.33.0
[PATCH v2 0/5] Fix 'mbr' command with hybrid MBR/GPT
When dealing with a hybrid MBR/GPT partition table, the 'mbr' command would misbehave because it was reading the GPT partitions instead of reading from the MBR when verifying. Fix this by forcing 'mbr verify' to read MBR partitions. V2: Fixed up dm_test_part tests and added tests for new API Joshua Watt (5): dm: test: Fix partition test to use mmc2 dm: test: Improve partition test error output disk: part: Add API to get partitions with specific driver dm: test: Add test for part_get_info_by_type cmd: mbr: Force DOS driver to be used for verify cmd/mbr.c | 2 +- configs/sandbox_defconfig | 2 + disk/part.c | 38 ++--- include/part.h| 19 ++- test/dm/part.c| 115 +- 5 files changed, 154 insertions(+), 22 deletions(-) -- 2.33.0
[PATCH] tests: Fix exception when cleaning up skipped test
If test_cat and test_xxd cannot create the required file, the test will
be skipped, but this would result in an exception being raised in the
finally block because the file didn't exist to be cleaned up. This
caused the test to be marked as failed instead of skipped.
Signed-off-by: Joshua Watt
---
test/py/tests/test_cat/conftest.py | 3 ++-
test/py/tests/test_xxd/conftest.py | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_cat/conftest.py
b/test/py/tests/test_cat/conftest.py
index 058fe52352..ef1c54d513 100644
--- a/test/py/tests/test_cat/conftest.py
+++ b/test/py/tests/test_cat/conftest.py
@@ -32,4 +32,5 @@ def cat_data(u_boot_config):
pytest.skip('Setup failed')
finally:
shutil.rmtree(mnt_point)
-os.remove(image_path)
+if os.path.exists(image_path):
+os.remove(image_path)
diff --git a/test/py/tests/test_xxd/conftest.py
b/test/py/tests/test_xxd/conftest.py
index 59285aadf4..481a794961 100644
--- a/test/py/tests/test_xxd/conftest.py
+++ b/test/py/tests/test_xxd/conftest.py
@@ -32,4 +32,5 @@ def xxd_data(u_boot_config):
pytest.skip('Setup failed')
finally:
shutil.rmtree(mnt_point)
-os.remove(image_path)
+if os.path.exists(image_path):
+os.remove(image_path)
--
2.33.0
Re: [PATCH] video: backlight: pwm: avoid integer overflow in duty cycle calculation
On Fri, 30 Jun 2023 at 13:38, Matthias Schiffer wrote: > > The intermediate value could overflow for large periods and levels. > > Signed-off-by: Matthias Schiffer > --- > drivers/video/pwm_backlight.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 09/10] doc: cmd: add documentation for scmi
Hi , On Mon, 3 Jul 2023 at 02:19, AKASHI Takahiro wrote: > > On Thu, Jun 29, 2023 at 08:10:02PM +0100, Simon Glass wrote: > > Hi AKASHI, > > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro > > wrote: > > > > > > This is a help text for scmi command. > > > > > > Signed-off-by: AKASHI Takahiro > > > --- > > > doc/usage/cmd/scmi.rst | 98 ++ > > > 1 file changed, 98 insertions(+) > > > create mode 100644 doc/usage/cmd/scmi.rst > > > > > > diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst > > > new file mode 100644 > > > index ..20cdae4b877d > > > --- /dev/null > > > +++ b/doc/usage/cmd/scmi.rst > > > @@ -0,0 +1,98 @@ > > > +.. SPDX-License-Identifier: GPL-2.0+: > > > + > > > +scmi command > > > + > > > + > > > +Synopsis > > > + > > > + > > > +:: > > > + > > > +scmi base info > > > +scmi base perm_dev > > > +scmi base perm_proto > > > +scmi base reset > > > + > > > +Description > > > +--- > > > + > > > +The scmi command is used to access and operate on SCMI server. > > > + > > > +scmi base info > > > +~~ > > > +Show base information about SCMI server and supported protocols > > > + > > > +scmi base perm_dev > > > +~~ > > > +Allow or deny access permission to the device > > > + > > > +scmi base perm_proto > > > + > > > +Allow or deny access to the protocol on the device > > > + > > > +scmi base reset > > > +~~~ > > > +Reset the existing configurations > > > + > > > +Parameters are used as follows: > > > + > > > + > > > +Agent ID > > > > what is this? > > I thought that the meaning was trivial in SCMI context. > Will change it to "SCMI Agent ID". What is SCMI? Really you should explain and link to information about things you mention in documentation. How else is anyone supposed to figure out what you are talking about? "SCMI Agent ID" doesn't tell us much. What form does it take? Is it a string? How do you find it out? > > > > > + > > > + > > > +Device ID > > > > what is this? > > Again, will change it to "SCMI Device ID". That doesn't help much. The purpose of documentation is to explain things for people who don't already know it. We need to try to be as helpful as possible. > > > > + > > > + > > > +Protocol ID, should not be 0x10 (base protocol) > > > > what is this? Please add more detail > > Again, will change it to "SCMI Protocol ID". > I think that users should be familiar with those terms > if they want to use these interfaces. Maybe, but it still isn't clear what it is. A string? It is confusing that the command ID is also a protocol ID. > > > > + > > > + > > > +Flags to control the action. See SCMI specification for > > > +defined values. > > > > ? > > > > Please add the flags here, or at the very least provide a URL and page > > number, etc. > > I intentionally avoid providing details here because a set of flags > acceptable to a specific SCMI server may depend on the server > and its implementation version. > The interface on U-Boot is just a wrapper to make a call to SCMI server > via a transport layer and doesn't care what the parameters means. > That said, I agree to referring to a URL to SCMI specification somewhere > in this document. That will help. But also please add that this is a hex value (right?) > > Thanks, > -Takahiro Akashi > > > > + > > > +Example > > > +--- > > > + > > > +Obtain basic information about SCMI server: > > > + > > > +:: > > > + > > > +=> scmi base info > > > +SCMI device: scmi > > > + protocol version: 0x2 > > > + # of agents: 3 > > > + 0: platform > > > +> 1: OSPM > > > + 2: PSCI > > > + # of protocols: 4 > > > + Power domain management > > > + Performance domain management > > > + Clock management > > > + Sensor management > > > + vendor: Linaro > > > + sub vendor: PMWG > > > + impl version: 0x20b > > > + > > > +Ask for access permission to device#0: > > > + > > > +:: > > > + > > > +=> scmi base perm_dev 1 0 1 > > > + > > > +Reset configurations with all access permission settings retained: > > > + > > > +:: > > > + > > > +=> scmi base reset 1 0 > > > + > > > +Configuration > > > +- > > > + > > > +The scmi command is only available if CONFIG_CMD_SCMI=y. > > > +Default n because this command is mainly for debug purpose. > > > + > > > +Return value > > > + > > > + > > > +The return value ($?) is set to 0 if the operation succeeded, > > > +1 if the operation failed or -1 if the operation failed due to > > > +a syntax error. > > > -- > > > 2.41.0 > > > Regards, Simon
Re: [PATCH 07/10] test: dm: add SCMI base protocol test
Hi,
On Mon, 3 Jul 2023 at 01:57, AKASHI Takahiro wrote:
>
> On Thu, Jun 29, 2023 at 08:09:58PM +0100, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > wrote:
> > >
> > > Added is a new unit test for SCMI base protocol, which will exercise all
> > > the commands provided by the protocol, except SCMI_BASE_NOTIFY_ERRORS.
> > > $ ut dm scmi_base
> > > It is assumed that test.dtb is used as sandbox's device tree.
> > >
> > > Signed-off-by: AKASHI Takahiro
> > > ---
> > > test/dm/scmi.c | 112 +
> > > 1 file changed, 112 insertions(+)
> > >
> > > diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> > > index 881be3171b7c..563017bb63e0 100644
> > > --- a/test/dm/scmi.c
> > > +++ b/test/dm/scmi.c
> > > @@ -16,6 +16,9 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -95,6 +98,115 @@ static int dm_test_scmi_sandbox_agent(struct
> > > unit_test_state *uts)
> > > }
> > > DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
> > >
> > > +static int dm_test_scmi_base(struct unit_test_state *uts)
> > > +{
> > > + struct udevice *agent_dev, *base;
> > > + struct scmi_agent_priv *priv;
> > > + const struct scmi_base_ops *ops;
> > > + u32 version, num_agents, num_protocols, impl_version;
> > > + u32 attributes, agent_id;
> > > + char vendor[SCMI_BASE_NAME_LENGTH_MAX],
> > > +agent_name[SCMI_BASE_NAME_LENGTH_MAX];
> > > + u8 *protocols;
> > > + int ret;
> > > +
> > > + /* preparation */
> > > + ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, "scmi",
> > > + _dev));
> > > + ut_assertnonnull(agent_dev);
> > > + ut_assertnonnull(priv = dev_get_uclass_plat(agent_dev));
> > > + ut_assertnonnull(base = scmi_get_protocol(agent_dev,
> > > + SCMI_PROTOCOL_ID_BASE));
> > > + ut_assertnonnull(ops = dev_get_driver_ops(base));
> > > +
> > > + /* version */
> > > + ret = (*ops->protocol_version)(base, );
> >
> > Can you add uclass helpers to call each of the methods? That is how it
> > is commonly done. You should not be calling ops->xxx directly here.
>
> Yes, I will add inline functions instead.
I don't mean inline...see all the other uclasses which define a
function which is implemented in the uclass. It is confusing when one
uclass does something different. People might copy this style and then
the code base diverges. Did you not notice this when looking around
the source tree?
Regards,
Simon
Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware
Hi,
On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro wrote:
>
> On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > wrote:
> > >
> > > This command, "scmi", provides a command line interface to various SCMI
> > > protocols. It supports at least initially SCMI base protocol with the sub
> > > command, "base", and is intended mainly for debug purpose.
> > >
> > > Signed-off-by: AKASHI Takahiro
> > > ---
> > > cmd/Kconfig | 8 ++
> > > cmd/Makefile | 1 +
> > > cmd/scmi.c | 373 +++
> > > 3 files changed, 382 insertions(+)
> > > create mode 100644 cmd/scmi.c
> > >
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index 02e54f1e50fe..065470a76295 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -2504,6 +2504,14 @@ config CMD_CROS_EC
> > > a number of sub-commands for performing EC tasks such as
> > > updating its flash, accessing a small saved context area
> > > and talking to the I2C bus behind the EC (if there is one).
> > > +
> > > +config CMD_SCMI
> > > + bool "Enable scmi command"
> > > + depends on SCMI_FIRMWARE
> > > + default n
> > > + help
> > > + This command provides user interfaces to several SCMI protocols,
> > > + including Base protocol.
> >
> > Please mention what is SCMI
>
> I will give a full spelling.
>
> > > endmenu
> > >
> > > menu "Filesystem commands"
> > > diff --git a/cmd/Makefile b/cmd/Makefile
> > > index 6c37521b4e2b..826e0b74b587 100644
> > > --- a/cmd/Makefile
> > > +++ b/cmd/Makefile
> > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> > > obj-$(CONFIG_CMD_NVME) += nvme.o
> > > obj-$(CONFIG_SANDBOX) += sb.o
> > > obj-$(CONFIG_CMD_SF) += sf.o
> > > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> > > obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> > > obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> > > obj-$(CONFIG_CMD_SEAMA) += seama.o
> > > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > > new file mode 100644
> > > index ..c97f77e97d95
> > > --- /dev/null
> > > +++ b/cmd/scmi.c
> > > @@ -0,0 +1,373 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * SCMI utility command
> > > + *
> > > + * Copyright (c) 2023 Linaro Limited
> > > + * Author: AKASHI Takahiro
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include /* uclass_get_device */
> >
> > Goes before linux/bitfield.h
>
> Can you tell me why?
It is just a convention:
https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
>
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +static struct udevice *agent;
> > > +static struct udevice *base_proto;
> > > +static const struct scmi_base_ops *ops;
> > > +
> > > +struct {
> > > + enum scmi_std_protocol id;
> > > + const char *name;
> > > +} protocol_name[] = {
> > > + {SCMI_PROTOCOL_ID_BASE, "Base"},
> > > + {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > > + {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > > + {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > > + {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > > + {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > > + {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > > + {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
> > > +};
> > > +
> > > +/**
> > > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > > + *
> > > + * @id:Protocol ID
> > > + *
> > > + * Get the printable name of the protocol, @id
> > > + *
> > > + * Return: Name string on success, NULL on failure
> > > + */
> > > +static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
> >
> > scmi_lookup_id?
>
> Not sure your intention.
> The name above gives us exactly what this function does for pretty printing
> instead of a number.
> I think that 'lookup' implies we try to determine if the id exists or not.
I am just trying to avoid the code turning into Java :-)
How about scmi_get_name() ?
Regards,
Simon
Re: [PATCH] tpm: Add TPM2_GetTestResult command support
Hi Julia,
On Mon, 3 Jul 2023 at 14:03, Julia Daxenberger
wrote:
>
> Add TPM2_GetTestResult command support and change the command file and the
> help accordingly. Add Python tests and sandbox driver functionality.
>
> The TPM2_GetTestResult command is performed after the TPM2_SelfTest command
> and returns manufacturer-specific information regarding the results of the
> self-test and an indication of the test status.
>
> Signed-off-by: Julia Daxenberger
> ---
> cmd/tpm-v2.c | 60 +
> drivers/tpm/tpm2_tis_sandbox.c | 47 ++-
> include/tpm-v2.h | 23 ++
> lib/tpm-v2.c | 82 ++
> test/py/tests/test_tpm2.py | 50 +
> 5 files changed, 261 insertions(+), 1 deletion(-)
>
Reviewed-by: Simon Glass
See below
[..]
> diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
> index d2ad6f9e73..aad1d7a55b 100644
> --- a/test/py/tests/test_tpm2.py
> +++ b/test/py/tests/test_tpm2.py
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0+
> # Copyright (c) 2018, Bootlin
> # Author: Miquel Raynal
> +# Copyright (C) 2023 Infineon Technologies AG
>
> import os.path
> import pytest
> @@ -316,3 +317,52 @@ def test_tpm2_cleanup(u_boot_console):
> """Ensure the TPM is cleared from password or test related
> configuration."""
>
> force_init(u_boot_console, True)
> +
> +@pytest.mark.buildconfigspec('cmd_tpm_v2')
> +def test_tpm2_get_test_result(u_boot_console):
> +"""Execute a TPM_GetTestResult command.
> +
> +Ask the TPM to get the test result of the self test.
> +Display the Test Result and Test Result Data.
> +
> +Expected default return value of tpm2_get_test_result, if the TPM has
> not been initialized:
> +- TPM2_RC_INITIALIZE = TPM2_RC_VER1 + 0x = 0x0100.
> +
> +Expected default value for test_result:
> +- TPM_RC_NEEDS_TEST = 0x0153, if tpm2 self_test has not been
> executed.
> +- TPM_RC_SUCCESS = 0x, if testing is complete without functional
> failures.
> +
> +There is no expected default value for the test result data because it
> would depend on the chip
> +used. The test result data is therefore not tested.
> +"""
> +if is_sandbox(u_boot_console):
> +u_boot_console.restart_uboot()
We should get rid of this somehow. We don't want sandbox rebooting
inthe middle of a test. It makes debugging painful, apart from
anything else. What TPM state needs to be reset?
Looking at tpm2_tis_sandbox.c it is probably the s_state variable. The
TPM state can be preserved across runs and is stored in the state
file.
But if the state file is not being used (no -s argument) then the TPM
should be reset each time DM is brought back up, i.e. between every
test.
So, do we even need this reset?
Regards,
Simon
Re: [PATCH v2] efi_driver: fix duplicate efiblk#0 issue
Hi Masahisa,
On Mon, 3 Jul 2023 at 07:09, Masahisa Kojima wrote:
>
> The devnum value of the blk_desc structure starts from 0,
> current efi_bl_create_block_device() function creates
> two "efiblk#0" devices for the cases that blk_find_max_devnum()
> returns -ENODEV and blk_find_max_devnum() returns 0(one device
> found in this case).
>
> This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
>
> Signed-off-by: Masahisa Kojima
> ---
> Changes in v2:
> - uses blk_next_free_devnum() instead of blk_find_max_devnum()
>
> lib/efi_driver/efi_block_device.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/lib/efi_driver/efi_block_device.c
> b/lib/efi_driver/efi_block_device.c
> index add00eeebb..e3abd90275 100644
> --- a/lib/efi_driver/efi_block_device.c
> +++ b/lib/efi_driver/efi_block_device.c
> @@ -124,10 +124,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
> *interface)
> struct efi_block_io *io = interface;
> struct efi_blk_plat *plat;
>
> - devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
> - if (devnum == -ENODEV)
> - devnum = 0;
> - else if (devnum < 0)
> + devnum = blk_next_free_devnum(UCLASS_EFI_LOADER);
This really should be an internal function but I see it was exported
as part of the virtio work.
How come the EFI and DM block devices are getting out of sync?
Anyway this function is munging around in the internals of the device
and should be fixed before it causes more problems.
For now, I suggest following what most other drivers so which is to
call blk_create_devicef() passing a devnum of -1.
> + if (devnum < 0)
> return EFI_OUT_OF_RESOURCES;
>
> name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
> --
> 2.34.1
>
Regards,
Simon
Re: [PATCH v13 05/10] arm_ffa: introduce armffa command
Hi Abdellatif,
On Mon, 3 Jul 2023 at 13:09, Abdellatif El Khlifi
wrote:
>
> Hi Ilias,
>
> On Mon, Jul 03, 2023 at 12:59:58PM +0300, Ilias Apalodimas wrote:
> > > > [...]
> > > > > +int do_ffa_ping(struct cmd_tbl *cmdtp, int flag, int argc, char
> > > > > *const argv[])
> > > > > +{
> > > > > + struct ffa_send_direct_data msg = {
> > > > > + .data0 = 0x,
> > > > > + .data1 = 0x,
> > > > > + .data2 = 0x,
> > > > > + .data3 = 0x,
> > > > > + .data4 = 0x,
> > > > > + };
> > > > > + u16 part_id;
> > > > > + int ret;
> > > > > + struct udevice *dev;
> > > > > +
> > > > > + if (argc != 2) {
> > > > > + log_err("Missing argument\n");
> > > > > + return CMD_RET_USAGE;
> > > > > + }
> > > > > +
> > > > > + errno = 0;
> > > > > + part_id = strtoul(argv[1], NULL, 16);
> > > > > +
> > > > > + if (errno) {
> > > >
> > > > Is errno used in strtoul?
> > >
> > > Yes, please refer to [1].
> > >
> > > [1]: https://man7.org/linux/man-pages/man3/strtoul.3.html
> >
> >
> > that's what the libc version does. Can you check the u-boot version?
> >
>
> Short answer: errno not used in strtoul() an I'm gonna remove the errno check.
>
> More details:
>
> strtoul() is defined as simple_strtoul() in strto.c
> errno variable is not set by simple_strtoul() or its callees.
> errno.h is included by strto.c to access the error codes.
>
> > > >
> ...
> > > > > +int do_ffa_devlist(struct cmd_tbl *cmdtp, int flag, int argc, char
> > > > > *const argv[])
> > > > > +{
> > > > > + struct udevice *dev;
> > > > > + int ret;
> > > > > +
> > > > > + ret = ffa_get_dev();
> > > > > + if (ret)
> > > > > + return CMD_RET_FAILURE;
> > > > > +
> > > > > + log_info("device name %s, dev %p, driver name %s, ops %p\n",
> > > > > +dev->name,
> > > > > + (void *)map_to_sysmem(dev),
> > > > > +dev->driver->name,
> > > > > +(void *)map_to_sysmem(dev->driver->ops));
> > > >
> > > > Isn't it more useful to print the physical address map_to_sysmem()
> > > > retuns?
> > >
> > > That's what map_to_sysmem() does, it returns a physical address and it's
> > > shown in the log.
> >
> > I dont have access to u-boot source right, but why do you need all
> > these void * casts then?
>
> Because map_to_sysmem() returns an 'phys_addr_t' (aka 'long long unsigned
> int') . However, %p expects 'void *'.
>
> Compilation warning:
>
> cmd/armffa.c:181:18: warning: format ‘%p’ expects argument of type ‘void *’,
> but argument 3 has type ‘phys_addr_t’ {aka ‘long long unsigned int’} [8;
> ;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat=-Wformat=8;;]
> 181 | log_info("device name %s, dev %p, driver name %s, ops %p\n",
> | ^~
> 182 | dev->name,
> 183 | map_to_sysmem(dev),
> | ~~
> | |
> | phys_addr_t {aka long long unsigned int}
You should be using %lx with map_to_sysmen() since it returns a . Use
%p for pointers.
In general in U-Boot we use addresses (ulong) rather than pointers.
Unfortunately the phys_addr_t type can be larger than the word size. I
suggest creating a printf prefix for that type. See the top of blk.h
for an example of how to do it. Perhaps in a follow-up patch?
Regards,
Simon
[PATCH 2/2] imx93_evk: defconfig: add adc support
iMX93 ADC features: - 4 channels - 12 bit resolution Signed-off-by: Luca Ellero --- configs/imx93_11x11_evk_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/imx93_11x11_evk_defconfig b/configs/imx93_11x11_evk_defconfig index 89edebc4c6..30ef460c80 100644 --- a/configs/imx93_11x11_evk_defconfig +++ b/configs/imx93_11x11_evk_defconfig @@ -81,6 +81,7 @@ CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_DM=y CONFIG_REGMAP=y CONFIG_SYSCON=y +CONFIG_ADC_IMX93=y CONFIG_CPU=y CONFIG_CPU_IMX=y CONFIG_IMX_RGPIO2P=y -- 2.25.1
[PATCH 1/2] dm: adc: add iMX93 ADC support
This commit adds driver for iMX93 ADC.
The driver is implemented using driver model and provides
ADC uclass's methods for ADC single channel operations:
- adc_start_channel()
- adc_channel_data()
- adc_stop()
ADC features:
- channels: 4
- resolution: 12-bit
Signed-off-by: Luca Ellero
---
drivers/adc/Kconfig | 8 ++
drivers/adc/Makefile| 1 +
drivers/adc/imx93-adc.c | 290
3 files changed, 299 insertions(+)
create mode 100644 drivers/adc/imx93-adc.c
diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
index e719c38bb3..4336732dee 100644
--- a/drivers/adc/Kconfig
+++ b/drivers/adc/Kconfig
@@ -63,3 +63,11 @@ config STM32_ADC
- core driver to deal with common resources
- child driver to deal with individual ADC resources (declare ADC
device and associated channels, start/stop conversions)
+
+config ADC_IMX93
+ bool "Enable NXP IMX93 ADC driver"
+ help
+ This enables basic driver for NXP IMX93 ADC.
+ It provides:
+ - 4 analog input channels
+ - 12-bit resolution
diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile
index c1387f3a34..5336c82097 100644
--- a/drivers/adc/Makefile
+++ b/drivers/adc/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_ADC_SANDBOX) += sandbox.o
obj-$(CONFIG_SARADC_ROCKCHIP) += rockchip-saradc.o
obj-$(CONFIG_SARADC_MESON) += meson-saradc.o
obj-$(CONFIG_STM32_ADC) += stm32-adc.o stm32-adc-core.o
+obj-$(CONFIG_ADC_IMX93) += imx93-adc.o
diff --git a/drivers/adc/imx93-adc.c b/drivers/adc/imx93-adc.c
new file mode 100644
index 00..41d04e0426
--- /dev/null
+++ b/drivers/adc/imx93-adc.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Luca Ellero
+ *
+ * Originally based on NXP linux-imx kernel v5.15 drivers/iio/adc/imx93_adc.c
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define IMX93_ADC_MCR 0x00
+#define IMX93_ADC_MSR 0x04
+#define IMX93_ADC_ISR 0x10
+#define IMX93_ADC_IMR 0x20
+#define IMX93_ADC_CIMR00x24
+#define IMX93_ADC_CTR0 0x94
+#define IMX93_ADC_NCMR00xA4
+#define IMX93_ADC_PCDR00x100
+#define IMX93_ADC_PCDR10x104
+#define IMX93_ADC_PCDR20x108
+#define IMX93_ADC_PCDR30x10c
+#define IMX93_ADC_PCDR40x110
+#define IMX93_ADC_PCDR50x114
+#define IMX93_ADC_PCDR60x118
+#define IMX93_ADC_PCDR70x11c
+#define IMX93_ADC_CALSTAT 0x39C
+
+#define IMX93_ADC_MCR_MODE_MASKBIT(29)
+#define IMX93_ADC_MCR_NSTART_MASK BIT(24)
+#define IMX93_ADC_MCR_CALSTART_MASKBIT(14)
+#define IMX93_ADC_MCR_ADCLKSE_MASK BIT(8)
+#define IMX93_ADC_MCR_PWDN_MASKBIT(0)
+
+#define IMX93_ADC_MSR_CALFAIL_MASK BIT(30)
+#define IMX93_ADC_MSR_CALBUSY_MASK BIT(29)
+#define IMX93_ADC_MSR_ADCSTATUS_MASK GENMASK(2, 0)
+
+#define IMX93_ADC_ISR_EOC_MASK BIT(1)
+
+#define IMX93_ADC_IMR_EOC_MASK BIT(1)
+#define IMX93_ADC_IMR_ECH_MASK BIT(0)
+
+#define IMX93_ADC_PCDR_CDATA_MASK GENMASK(11, 0)
+
+#define IDLE 0
+#define POWER_DOWN 1
+#define WAIT_STATE 2
+#define BUSY_IN_CALIBRATION3
+#define SAMPLE 4
+#define CONVERSION 6
+
+#define IMX93_ADC_MAX_CHANNEL 3
+#define IMX93_ADC_DAT_MASK 0xfff
+#define IMX93_ADC_TIMEOUT 10
+
+struct imx93_adc_priv {
+ int active_channel;
+ void __iomem *regs;
+ struct clk ipg_clk;
+};
+
+static void imx93_adc_power_down(struct imx93_adc_priv *adc)
+{
+ u32 mcr, msr;
+ int ret;
+
+ mcr = readl(adc->regs + IMX93_ADC_MCR);
+ mcr |= FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+ writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+ ret = readl_poll_timeout(adc->regs + IMX93_ADC_MSR, msr,
+ ((msr & IMX93_ADC_MSR_ADCSTATUS_MASK) == POWER_DOWN), 50);
+ if (ret == -ETIMEDOUT)
+ pr_warn("ADC not in power down mode, current MSR: %x\n", msr);
+}
+
+static void imx93_adc_power_up(struct imx93_adc_priv *adc)
+{
+ u32 mcr;
+
+ /* bring ADC out of power down state, in idle state */
+ mcr = readl(adc->regs + IMX93_ADC_MCR);
+ mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+ writel(mcr, adc->regs + IMX93_ADC_MCR);
+}
+
+static void imx93_adc_config_ad_clk(struct imx93_adc_priv *adc)
+{
+ u32 mcr;
+
+ /* put adc in power down mode */
+ imx93_adc_power_down(adc);
+
+ /* config the AD_CLK equal to bus clock */
+ mcr = readl(adc->regs + IMX93_ADC_MCR);
+ mcr |=
[RESEND PATCH v5 0/2] imx93: add ADC support
Add ADC support for NXP iMX93 Changes for v2: - add "static" to functions - enable ADC in iMX93 EVK Changes for v3: - split in 3 commits - keep dts file in sync with Linux devicetree - add comments to commits Changes for v4: - add imx93_adc_power_down() in imx93_adc_stop() Changes for v5: - simplify code - remove redundant code - add clock handling Luca Ellero (2): dm: adc: add iMX93 ADC support imx93_evk: defconfig: add adc support configs/imx93_11x11_evk_defconfig | 1 + drivers/adc/Kconfig | 8 + drivers/adc/Makefile | 1 + drivers/adc/imx93-adc.c | 290 ++ 4 files changed, 300 insertions(+) create mode 100644 drivers/adc/imx93-adc.c -- 2.25.1
Re: [PATCH u-boot 0/4] mmc: Explain and cleanup partition selection
Hi, On 4/14/23 06:10, Pali Rohár wrote: > Some people do not want to read review comments in emails. So put > comments and explanation into the source code itself; make emmc > partition selection code more explicit and validate configuration in > bubt command. Sorry for too late. After applied this patch-set, some board are failed with spl's size overflowed The below is one of failed log. +arm-linux-gnueabi-ld.bfd: region `.sram' overflowed by 352 bytes +make[2]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1 +make[1]: *** [Makefile:2049: spl/u-boot-spl] Error 2 Best Regards, Jaehoon Chung > > Pali Rohár (4): > mmc: spl: Make partition choice in > default_spl_mmc_emmc_boot_partition() more explicit > cmd: mvebu/bubt: Validate EXT_CSD[179] eMMC register in > mmc_burn_image() > sunxi: eMMC: Add comments explaining mapping between bootpart and > mmc_switch_part() > board: purism: Use U-Boot mmc function for converting boot part to > part access > > arch/arm/mach-sunxi/board.c| 12 ++- > board/purism/librem5/librem5.c | 6 +- > cmd/mvebu/bubt.c | 24 +++-- > common/spl/spl_mmc.c | 38 +++--- > 4 files changed, 64 insertions(+), 16 deletions(-)
Re: [PATCH v13 05/10] arm_ffa: introduce armffa command
Hi Ilias,
On Mon, Jul 03, 2023 at 12:59:58PM +0300, Ilias Apalodimas wrote:
> > > [...]
> > > > +int do_ffa_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> > > > argv[])
> > > > +{
> > > > + struct ffa_send_direct_data msg = {
> > > > + .data0 = 0x,
> > > > + .data1 = 0x,
> > > > + .data2 = 0x,
> > > > + .data3 = 0x,
> > > > + .data4 = 0x,
> > > > + };
> > > > + u16 part_id;
> > > > + int ret;
> > > > + struct udevice *dev;
> > > > +
> > > > + if (argc != 2) {
> > > > + log_err("Missing argument\n");
> > > > + return CMD_RET_USAGE;
> > > > + }
> > > > +
> > > > + errno = 0;
> > > > + part_id = strtoul(argv[1], NULL, 16);
> > > > +
> > > > + if (errno) {
> > >
> > > Is errno used in strtoul?
> >
> > Yes, please refer to [1].
> >
> > [1]: https://man7.org/linux/man-pages/man3/strtoul.3.html
>
>
> that's what the libc version does. Can you check the u-boot version?
>
Short answer: errno not used in strtoul() an I'm gonna remove the errno check.
More details:
strtoul() is defined as simple_strtoul() in strto.c
errno variable is not set by simple_strtoul() or its callees.
errno.h is included by strto.c to access the error codes.
> > >
...
> > > > +int do_ffa_devlist(struct cmd_tbl *cmdtp, int flag, int argc, char
> > > > *const argv[])
> > > > +{
> > > > + struct udevice *dev;
> > > > + int ret;
> > > > +
> > > > + ret = ffa_get_dev();
> > > > + if (ret)
> > > > + return CMD_RET_FAILURE;
> > > > +
> > > > + log_info("device name %s, dev %p, driver name %s, ops %p\n",
> > > > +dev->name,
> > > > + (void *)map_to_sysmem(dev),
> > > > +dev->driver->name,
> > > > +(void *)map_to_sysmem(dev->driver->ops));
> > >
> > > Isn't it more useful to print the physical address map_to_sysmem() retuns?
> >
> > That's what map_to_sysmem() does, it returns a physical address and it's
> > shown in the log.
>
> I dont have access to u-boot source right, but why do you need all
> these void * casts then?
Because map_to_sysmem() returns an 'phys_addr_t' (aka 'long long unsigned int')
. However, %p expects 'void *'.
Compilation warning:
cmd/armffa.c:181:18: warning: format ‘%p’ expects argument of type ‘void *’,
but argument 3 has type ‘phys_addr_t’ {aka ‘long long unsigned int’} [8;
;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat=-Wformat=8;;]
181 | log_info("device name %s, dev %p, driver name %s, ops %p\n",
| ^~
182 | dev->name,
183 | map_to_sysmem(dev),
| ~~
| |
| phys_addr_t {aka long long unsigned int}
Cheers,
Abdellatif
[PULL] u-boot-usb/master
The following changes since commit ac29400f1f4ae5df2542bacfe4c142db7824bd6c: Merge tag 'efi-2023-07-rc6' of https://source.denx.de/u-boot/custodians/u-boot-efi (2023-07-01 16:11:51 -0400) are available in the Git repository at: git://source.denx.de/u-boot-usb.git master for you to fetch changes up to d266d4b63873c2176677a79d48faeb9db7d4acdd: usb: dwc3-generic: Ensure reset GPIO is configured as an output (2023-07-03 10:46:34 +0200) Peter Korsgaard (1): usb: dwc3-generic: Ensure reset GPIO is configured as an output drivers/usb/dwc3/dwc3-generic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v1] configs: rockchip: rock5b-rk3588: Enable CONFIG_PCI_INIT_R
Enable CONFIG_PCI_INIT_R for rock5b pci enumeration during boot in order to autodetect the PCI ethernet NIC during the boot process. Signed-off-by: Christopher Obbard --- configs/rock5b-rk3588_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig index c1155c20ef..ed2d0cc54a 100644 --- a/configs/rock5b-rk3588_defconfig +++ b/configs/rock5b-rk3588_defconfig @@ -33,6 +33,7 @@ CONFIG_OF_BOARD_SETUP=y CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-rock-5b.dtb" # CONFIG_DISPLAY_CPUINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_PCI_INIT_R=y CONFIG_SPL_MAX_SIZE=0x4 CONFIG_SPL_PAD_TO=0x7f8000 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y -- 2.40.1
Re: [PATCH v13 05/10] arm_ffa: introduce armffa command
On Mon, 3 Jul 2023 at 12:55, Abdellatif El Khlifi
wrote:
>
> Hi Ilias,
>
> On Tue, Jun 20, 2023 at 05:25:51PM +0300, Ilias Apalodimas wrote:
> > [...]
> > > +int do_ffa_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> > > argv[])
> > > +{
> > > + struct ffa_send_direct_data msg = {
> > > + .data0 = 0x,
> > > + .data1 = 0x,
> > > + .data2 = 0x,
> > > + .data3 = 0x,
> > > + .data4 = 0x,
> > > + };
> > > + u16 part_id;
> > > + int ret;
> > > + struct udevice *dev;
> > > +
> > > + if (argc != 2) {
> > > + log_err("Missing argument\n");
> > > + return CMD_RET_USAGE;
> > > + }
> > > +
> > > + errno = 0;
> > > + part_id = strtoul(argv[1], NULL, 16);
> > > +
> > > + if (errno) {
> >
> > Is errno used in strtoul?
>
> Yes, please refer to [1].
>
> [1]: https://man7.org/linux/man-pages/man3/strtoul.3.html
that's what the libc version does. Can you check the u-boot version?
>
> > Does FF-A have any limits regarding the partition id? If yes, it would
> > be saner to check against that.
>
> The only value that would be invalid is 0. I'll add a check for that in v14.
>
> >
> > > + log_err("Invalid partition ID\n");
> > > + return CMD_RET_USAGE;
> > > + }
> > > +
> > > + ret = ffa_get_dev();
> > > + if (ret)
> > > + return CMD_RET_FAILURE;
> > > +
> > > + ret = ffa_sync_send_receive(dev, part_id, , 1);
> > > + if (!ret) {
> > > + u8 cnt;
> > > +
> > > + log_info("SP response:\n[LSB]\n");
> > > + for (cnt = 0;
> > > +cnt < sizeof(struct ffa_send_direct_data) /
> > > sizeof(u64);
> > > +cnt++)
> > > + log_info("%llx\n", ((u64 *))[cnt]);
> >
> > I am not sure I understand why we print it like this.
>
> We would like to show the data received from secure world and in which order.
>
> example:
>
> corstone1000# armffa ping 0x8003
> SP response:
> [LSB]
> fffe
> 0
> 0
> 0
> 0
>
> >
> > > + return CMD_RET_SUCCESS;
> > > + }
> > > +
> > > + log_err("Sending direct request error (%d)\n", ret);
> > > + return CMD_RET_FAILURE;
> > > +}
> > > +
> > > +/**
> > > + *do_ffa_devlist() - implementation of the devlist subcommand
> > > + * @cmdtp: [in]Command Table
> > > + * @flag: flags
> > > + * @argc: number of arguments
> > > + * @argv: arguments
> > > + *
> > > + * Query the device belonging to the UCLASS_FFA
> > > + * class.
> > > + *
> > > + * Return:
> > > + *
> > > + * CMD_RET_SUCCESS: on success, otherwise failure
> > > + */
> > > +int do_ffa_devlist(struct cmd_tbl *cmdtp, int flag, int argc, char
> > > *const argv[])
> > > +{
> > > + struct udevice *dev;
> > > + int ret;
> > > +
> > > + ret = ffa_get_dev();
> > > + if (ret)
> > > + return CMD_RET_FAILURE;
> > > +
> > > + log_info("device name %s, dev %p, driver name %s, ops %p\n",
> > > +dev->name,
> > > + (void *)map_to_sysmem(dev),
> > > +dev->driver->name,
> > > +(void *)map_to_sysmem(dev->driver->ops));
> >
> > Isn't it more useful to print the physical address map_to_sysmem() retuns?
>
> That's what map_to_sysmem() does, it returns a physical address and it's
> shown in the log.
I dont have access to u-boot source right, but why do you need all
these void * casts then?
Thanks
/Ilias
>
> Cheers,
> Abdellatif
Re: [PATCH v13 05/10] arm_ffa: introduce armffa command
Hi Ilias,
On Tue, Jun 20, 2023 at 05:25:51PM +0300, Ilias Apalodimas wrote:
> [...]
> > +int do_ffa_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> > argv[])
> > +{
> > + struct ffa_send_direct_data msg = {
> > + .data0 = 0x,
> > + .data1 = 0x,
> > + .data2 = 0x,
> > + .data3 = 0x,
> > + .data4 = 0x,
> > + };
> > + u16 part_id;
> > + int ret;
> > + struct udevice *dev;
> > +
> > + if (argc != 2) {
> > + log_err("Missing argument\n");
> > + return CMD_RET_USAGE;
> > + }
> > +
> > + errno = 0;
> > + part_id = strtoul(argv[1], NULL, 16);
> > +
> > + if (errno) {
>
> Is errno used in strtoul?
Yes, please refer to [1].
[1]: https://man7.org/linux/man-pages/man3/strtoul.3.html
> Does FF-A have any limits regarding the partition id? If yes, it would
> be saner to check against that.
The only value that would be invalid is 0. I'll add a check for that in v14.
>
> > + log_err("Invalid partition ID\n");
> > + return CMD_RET_USAGE;
> > + }
> > +
> > + ret = ffa_get_dev();
> > + if (ret)
> > + return CMD_RET_FAILURE;
> > +
> > + ret = ffa_sync_send_receive(dev, part_id, , 1);
> > + if (!ret) {
> > + u8 cnt;
> > +
> > + log_info("SP response:\n[LSB]\n");
> > + for (cnt = 0;
> > +cnt < sizeof(struct ffa_send_direct_data) /
> > sizeof(u64);
> > +cnt++)
> > + log_info("%llx\n", ((u64 *))[cnt]);
>
> I am not sure I understand why we print it like this.
We would like to show the data received from secure world and in which order.
example:
corstone1000# armffa ping 0x8003
SP response:
[LSB]
fffe
0
0
0
0
>
> > + return CMD_RET_SUCCESS;
> > + }
> > +
> > + log_err("Sending direct request error (%d)\n", ret);
> > + return CMD_RET_FAILURE;
> > +}
> > +
> > +/**
> > + *do_ffa_devlist() - implementation of the devlist subcommand
> > + * @cmdtp: [in]Command Table
> > + * @flag: flags
> > + * @argc: number of arguments
> > + * @argv: arguments
> > + *
> > + * Query the device belonging to the UCLASS_FFA
> > + * class.
> > + *
> > + * Return:
> > + *
> > + * CMD_RET_SUCCESS: on success, otherwise failure
> > + */
> > +int do_ffa_devlist(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> > argv[])
> > +{
> > + struct udevice *dev;
> > + int ret;
> > +
> > + ret = ffa_get_dev();
> > + if (ret)
> > + return CMD_RET_FAILURE;
> > +
> > + log_info("device name %s, dev %p, driver name %s, ops %p\n",
> > +dev->name,
> > + (void *)map_to_sysmem(dev),
> > +dev->driver->name,
> > +(void *)map_to_sysmem(dev->driver->ops));
>
> Isn't it more useful to print the physical address map_to_sysmem() retuns?
That's what map_to_sysmem() does, it returns a physical address and it's shown
in the log.
Cheers,
Abdellatif
Re: [PATCH v13 01/10] arm64: smccc: add support for SMCCCv1.2 x0-x17 registers
Hi Ilias, On Tue, Jun 20, 2023 at 05:05:52PM +0300, Ilias Apalodimas wrote: > Hi Abdellatif, > > On Fri, 16 Jun 2023 at 18:28, Abdellatif El Khlifi > wrote: > > > > add support for x0-x17 registers used by the SMC calls > > > > In SMCCC v1.2 [1] arguments are passed in registers x1-x17. > > Results are returned in x0-x17. > > > > This work is inspired from the following kernel commit: > > > > arm64: smccc: Add support for SMCCCv1.2 extended input/output registers > > > > [1]: > > https://documentation-service.arm.com/static/5f8edaeff86e16515cdbe4c6?token= > > I did review this one in the past, any reason why that is missing? > Did the file change? Thanks, I was waiting for your confirmation. I'll add your Reviewed-by in v14. Cheers
Re: [PATCH v13 04/10] arm_ffa: introduce Arm FF-A support
Hi Simon, On Sun, Jul 02, 2023 at 04:44:41PM +0100, Simon Glass wrote: > Hi Abdellatif, > ... > > > > Changelog: > > > > === > > > > > > > > v13: > > > > > > > > * doc minor change: specify in the readme that the user > > > >should call ffa_rxtx_unmap() driver operation to unmap > > > >the RX/TX buffers on demand. > > > > Are you happy with this commit please ? May I add a Reviewed-by ? > > Sorry I think I did something wrong on the previous reply. > > Reviewed-by: Simon Glass > Thanks. Are you happy with the Sandbox support commit too [1] ? [1]: arm_ffa: introduce sandbox FF-A support Cheers Abdellatif
Re: [PATCH] mmc:Remove the legacy mode clock setting operation
On 6/21/23 12:11, xf_...@163.com wrote:
> From: xiefei
>
> Due to the need to read the register value before
> switching to hs mode, the standard protocol does
> not explicitly specify that the setting before
> switching to hs mode is in legacy mode. Therefore,
> the code at this point may cause communication
> abnormalities between the host and card
>
> Signed-off-by: xiefei
NACK.
> ---
> drivers/mmc/mmc.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 1af6af82e6..915f446973 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2138,7 +2138,6 @@ static int mmc_select_mode_and_width(struct mmc *mmc,
> uint card_caps)
> mmc_set_card_speed(mmc, MMC_HS, true);
> else
> #endif
> - mmc_set_clock(mmc, mmc->legacy_speed, MMC_CLK_ENABLE);
Even except your intention, your patch didn't consider #ifdef ~ #endif.
Best Regards,
Jaehoon Chung
>
> for_each_mmc_mode_by_pref(card_caps, mwt) {
> for_each_supported_width(card_caps & mwt->widths,
Re: [PATCHv2 3/3] net/lwip: add lwip library for the network stack
On Fri, 30 Jun 2023 at 22:38, Tom Rini wrote: > On Thu, Jun 29, 2023 at 06:34:30PM +0600, Maxim Uvarov wrote: > > > This commit adds lwip library for the U-boot network > > stack. Supported commands: ping, tftp, dhcp and wget. > > > > Signed-off-by: Maxim Uvarov > [snip] > > diff --git a/lib/lwip/Kconfig b/lib/lwip/Kconfig > > new file mode 100644 > > index 00..80d540ba54 > > --- /dev/null > > +++ b/lib/lwip/Kconfig > > @@ -0,0 +1,108 @@ > > +menu "LWIP" > > +config LWIP_LIB > > + bool "Support LWIP library" > > + help > > + Selecting this option will enable the shared LWIP library code. > > + > > +menu "LWIP options" > > +config LWIP_LIB_DEBUG > > + bool "enable debug" > > + default n > > +config LWIP_LIB_NOASSERT > > + bool "disable asserts" > > + default y > > + help > > + Disabling asserts reduces binary size on 16k. > > +config LWIP_LIB_TCP > > +bool "tcp" > > +default y > > +config LWIP_LIB_UDP > > +bool "udp" > > +default y > > +config LWIP_LIB_DNS > > +bool "dns" > > +default n > > +config LWIP_LIB_DHCP > > +bool "dhcp" > > +default y > > + > > +config LWIP_LIB_LOOPBACK > > +bool "loopback" > > +default n > > +help > > +Increases size on 1k. > > +config LWIP_LIB_SOCKET > > +bool "socket API" > > +default n > > +config LWIP_LIB_NETCONN > > +bool "netconn API" > > +default n > > +config LWIP_LIB_MEM_SIZE > > + int "mem size" > > + default 1600 > > + range 1 4096 > > + help > > + MEM_SIZE: the size of the heap memory. If the application will > send > > + a lot of data that needs to be copied, this should be set high. > > + > > +config LWIP_LIB_PBUF_LINK_HLEN > > +int "pbuf link hlen" > > +default 14 > > +range 4 1024 > > +help > > +PBUF_LINK_HLEN: the number of bytes that should be allocated > for a > > + link level header. The default is 14, the standard value for > Ethernet. > > +endmenu > > + > > +config CMD_LWIP > > +bool "lwip cmd" > > +default y > > + depends on LWIP_LIB > > +help > > + lwip networking command. > > + > > +config CMD_LWIP_PING > > +bool "ping" > > +default y > > + depends on CMD_LWIP > > +help > > + lwip ping command. > > + > > +config CMD_LWIP_REPLACE_PING > > +bool "replace original ping command" > > +default n > > + > > +config CMD_LWIP_WGET > > +bool "wget" > > +default y > > + depends on CMD_LWIP > > +help > > + lwip wget command. > > + > > +config CMD_LWIP_REPLACE_WGET > > + bool "replace original wget command" > > + default n > > + > > +config CMD_LWIP_TFTP > > +bool "tftp" > > +default y > > + depends on CMD_LWIP > > +help > > + lwip tftp command. > > + > > +config CMD_LWIP_REPLACE_TFTP > > + bool "replace original tftp command" > > + default n > > + > > +config CMD_LWIP_DHCP > > +bool "dhcp" > > +default y > > + depends on CMD_LWIP > > + depends on LWIP_LIB_DHCP > > +help > > + lwip dhcp command. > > + > > +config CMD_LWIP_REPLACE_DHCP > > + bool "replace original dhcp command" > > + default n > > +endmenu > > So,, the spacing needs to be fixed up, and "default n" is the default > when no "default" keyword is present. But here's the big change I want, > if we enable LWIP it should replace the other commands. For debug / > testing, it's OK for "lwip dhcp" to be how to run it, but since the goal > is replacement, it needs to replace. I want to be able to do drop > CONFIG_LWIP=y in my CI script and have all of my boards use LWIP for > the normal networking tests so I can report back that things work, or > that there's problems to sort out. Thanks! > > -- > Tom > Yea, it will be good to do some good testing for this. In this patch there are 4 options: CONFIG_CMD_LWIP_REPLACE_PING=y CONFIG_CMD_LWIP_REPLACE_WGET=y CONFIG_CMD_LWIP_REPLACE_TFTP=y CONFIG_CMD_LWIP_REPLACE_DHCP=y which actually replace original commands with lwip one. In the next version I will drop CONFIG_LWIP=y and make it compile when CONFIG_NET=y is enabled. BR, Maxim.
Re: [PATCH v2] efi_driver: fix duplicate efiblk#0 issue
On 7/3/23 08:08, Masahisa Kojima wrote:
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
Signed-off-by: Masahisa Kojima
---
Changes in v2:
- uses blk_next_free_devnum() instead of blk_find_max_devnum()
lib/efi_driver/efi_block_device.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebb..e3abd90275 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -124,10 +124,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
*interface)
struct efi_block_io *io = interface;
struct efi_blk_plat *plat;
- devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
- if (devnum == -ENODEV)
- devnum = 0;
- else if (devnum < 0)
+ devnum = blk_next_free_devnum(UCLASS_EFI_LOADER);
+ if (devnum < 0)
return EFI_OUT_OF_RESOURCES;
Reviewed-by: Heinrich Schuchardt
name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
Re: [PATCH] efi_driver: fix duplicate efiblk#0 issue
Hi Heinrich,
On Mon, 3 Jul 2023 at 15:10, Heinrich Schuchardt wrote:
>
> On 7/3/23 04:47, Masahisa Kojima wrote:
> > The devnum value of the blk_desc structure starts from 0,
> > current efi_bl_create_block_device() function creates
> > two "efiblk#0" devices for the cases that blk_find_max_devnum()
> > returns -ENODEV and blk_find_max_devnum() returns 0(one device
> > found in this case).
> >
> > The devnum value for the "efiblk" name needs to be incremented.
> >
> > Signed-off-by: Masahisa Kojima
> > ---
> > lib/efi_driver/efi_block_device.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/efi_driver/efi_block_device.c
> > b/lib/efi_driver/efi_block_device.c
> > index add00eeebb..e37bfe6e80 100644
> > --- a/lib/efi_driver/efi_block_device.c
> > +++ b/lib/efi_driver/efi_block_device.c
> > @@ -129,6 +129,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
> > *interface)
> > devnum = 0;
> > else if (devnum < 0)
> > return EFI_OUT_OF_RESOURCES;
> > + else
> > + devnum++; /* device found, note that devnum starts from 0 */
>
> Shouldn't we simply use blk_next_free_devnum() instead of duplicating
> the logic here?
Yes, Akashi-san also already pointed out, and I already sent v2 with
blk_next_free_devnum().
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> >
> > name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
> > if (!name)
>
Re: [PATCH] efi_driver: fix duplicate efiblk#0 issue
On 7/3/23 04:47, Masahisa Kojima wrote:
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
The devnum value for the "efiblk" name needs to be incremented.
Signed-off-by: Masahisa Kojima
---
lib/efi_driver/efi_block_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebb..e37bfe6e80 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -129,6 +129,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
*interface)
devnum = 0;
else if (devnum < 0)
return EFI_OUT_OF_RESOURCES;
+ else
+ devnum++; /* device found, note that devnum starts from 0 */
Shouldn't we simply use blk_next_free_devnum() instead of duplicating
the logic here?
Best regards
Heinrich
name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
if (!name)
[PATCH v2] efi_driver: fix duplicate efiblk#0 issue
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
Signed-off-by: Masahisa Kojima
---
Changes in v2:
- uses blk_next_free_devnum() instead of blk_find_max_devnum()
lib/efi_driver/efi_block_device.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebb..e3abd90275 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -124,10 +124,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
*interface)
struct efi_block_io *io = interface;
struct efi_blk_plat *plat;
- devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
- if (devnum == -ENODEV)
- devnum = 0;
- else if (devnum < 0)
+ devnum = blk_next_free_devnum(UCLASS_EFI_LOADER);
+ if (devnum < 0)
return EFI_OUT_OF_RESOURCES;
name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
--
2.34.1
