[PATCH] x86: qemu: Fix broken multi-core boot
Unfortunately the multi-core boot for QEMU x86 has been broken since
commit 77a5e2d3bc61 ("x86: mp_init: Set up the CPU numbers at the start").
In order to support QEMU x86 multi-core boot, the /cpus node must be
bound before any actual fix up in qemu_cpu_fixup(). This adds the
uclass_get() call to ensure this, just like what was done before.
Fixes: 77a5e2d3bc61 ("x86: mp_init: Set up the CPU numbers at the start")
Signed-off-by: Bin Meng
---
arch/x86/cpu/qfw_cpu.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/x86/cpu/qfw_cpu.c b/arch/x86/cpu/qfw_cpu.c
index a35de87..b959ead 100644
--- a/arch/x86/cpu/qfw_cpu.c
+++ b/arch/x86/cpu/qfw_cpu.c
@@ -17,10 +17,16 @@ int qemu_cpu_fixup(void)
int ret;
int cpu_num;
int cpu_online;
+ struct uclass *uc;
struct udevice *dev, *pdev;
struct cpu_plat *plat;
char *cpu;
+ /* This will cause the CPUs devices to be bound */
+ ret = uclass_get(UCLASS_CPU, &uc);
+ if (ret)
+ return ret;
+
/* first we need to find '/cpus' */
for (device_find_first_child(dm_root(), &pdev);
pdev;
--
2.7.4
Re: [PATCH 4/4] fs: fat: remove trailing periods from long name
Am 2. Februar 2021 07:39:34 MEZ schrieb AKASHI Takahiro
:
>On Tue, Feb 02, 2021 at 07:05:53AM +0100, Heinrich Schuchardt wrote:
>> Am 2. Februar 2021 00:54:58 MEZ schrieb AKASHI Takahiro
>:
>> >On Mon, Feb 01, 2021 at 01:34:59PM +0100, Heinrich Schuchardt wrote:
>> >> On 01.02.21 09:18, AKASHI Takahiro wrote:
>> >> > On Sun, Jan 31, 2021 at 12:09:53AM +0100, Heinrich Schuchardt
>> >wrote:
>> >> >> The FAT32 File System Specification [1] requires leading and
>> >trailing
>> >> >> spaces as well as trailing periods of long names to be ignored.
>> >> >>
>> >> >> This renders a test for '.' and '..' as file name superfluous.
>> >> >>
>> >> >> But we must check that the resulting name has at least one
>> >character.
>> >> >>
>> >> >> [1]
>> >> >> Microsoft Extensible Firmware Initiative
>> >> >> FAT32 File System Specification
>> >> >> Version 1.03, December 6, 2000
>> >> >> Microsoft Corporation
>> >> >> https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf
>> >> >>
>> >> >> Signed-off-by: Heinrich Schuchardt
>> >> >> ---
>> >> >> fs/fat/fat_write.c | 29 +++--
>> >> >> 1 file changed, 23 insertions(+), 6 deletions(-)
>> >> >>
>> >> >> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
>> >> >> index 0f4786ef0f..1b0a0eda09 100644
>> >> >> --- a/fs/fat/fat_write.c
>> >> >> +++ b/fs/fat/fat_write.c
>> >> >> @@ -1237,12 +1237,32 @@ again:
>> >> >>}
>> >> >>
>> >> >>*last_slash_cont = '\0';
>> >> >> - *basename = last_slash_cont + 1;
>> >> >> + filename = last_slash_cont + 1;
>> >> >>} else {
>> >> >>*dirname = "/"; /* root by default */
>> >> >> - *basename = filename;
>> >> >>}
>> >> >>
>> >> >> + /*
>> >> >> + * The FAT32 File System Specification v1.03 requires leading
>> >and
>> >> >> + * trailing spaces as well as trailing periods to be ignored.
>> >> >> + */
>> >> >> + for (; *filename == ' '; ++filename)
>> >> >> + ;
>> >> >> + /* Remove trailing periods and spaces */
>> >> >> + for (p = filename + strlen(filename) - 1; p >= filename; --p)
>{
>> >> >> + switch (*p) {
>> >> >> + case ' ':
>> >> >> + case '.':
>> >> >> + *p = 0;
>> >> >> + break;
>> >> >> + default:
>> >> >> + goto done;
>> >> >> + }
>> >> >> + }
>> >> >
>> >> > Given the semantics of the functions, split_filename() and
>> >normalize_longname(),
>> >> > the code you added above should be moved to
>normalize_longname().
>> >>
>> >> normalize_longname(l_filename, filename) converts the argument
>> >filename
>> >> to a lowercase string l_filename. The parameter filename remains
>> >> unchanged. But it is the value of filename that is used to create
>the
>> >> new directory entry in file_fat_write_at() and fat_mkdir().
>> >
>> >That is why I also suggested, "I even think it would be best to move
>it
>> >to
>> >the caller, file_fat_write_at() or fat_mkdir()."
>> >
>> >> So moving the change to normalize_longname() will not lead to the
>> >> intended behavior.
>> >>
>> >> Removing leading and trailing blanks fits well into the task of
>> >> split_filename to identify the actual file name.
>> >
>> >Again, "." and ".." are legal directory names.
>> >To reject a request of creating such names is a caller's job,
>> >not split_filename()'s as its name suggests.
>> >
>>
>> These filenames are illegal for all callers.
>
>The purpose of split_filename() is to separate a file name from
>its directory path. As I said, excluding "." or ".." is out of this
>function's scope, but the caller's semantics.
>
>> We should not duplicate code in each caller.
>
>I don't think so.
>handling "." or ".." entry and removing leading/trailing spaces
>are totally different.
Trailing periods have to be removed here too.
Putting special handling for '.' and '..' here *and* in each caller will
increase code size without any gain in functionality.
Best regards
Heinrich
>
>-Takahiro Akashi
>
>> Best regards
>>
>> Heinrich
>>
>>
>> >-Takahiro Akashi
>> >
>> >> Best regards
>> >>
>> >> Heinrich
>> >>
>> >> >
>> >> >> +done:
>> >> >> + *basename = filename;
>> >> >> +
>> >> >>return 0;
>> >> >> }
>> >> >>
>> >> >> @@ -1260,10 +1280,7 @@ static int normalize_longname(char
>> >*l_filename, const char *filename)
>> >> >> {
>> >> >>const char *p, illegal[] = "<>:\"/\\|?*";
>> >> >>
>> >> >> - if (!strcmp(filename, ".") || !strcmp(filename, ".."))
>> >> >> - return -1;
>> >> >
>> >> > It would be better for the check above to remain here as "." and
>> >".." are
>> >> > legal directory names. (I even think it would be best to move it
>to
>> >> > the caller, file_fat_write_at() or fat_mkdir().)
>> >> >
>> >> > I think that the suggested sequence would be more intuitive for
>> >> > better understanding of
[PATCH] mmc: mv_sdhci: parse device-tree entry
Call mmc_of_parse() so that generic DT properties like 'non-removable' are taken into account. This fixes boot on Clearfog with eMMC on SOM that requires the non-removable property. Reported-by: Thorsten Spille Signed-off-by: Baruch Siach --- drivers/mmc/mv_sdhci.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c index 9b3dfa13e619..0a8987c1b5da 100644 --- a/drivers/mmc/mv_sdhci.c +++ b/drivers/mmc/mv_sdhci.c @@ -118,6 +118,10 @@ static int mv_sdhci_probe(struct udevice *dev) host->mmc->dev = dev; host->mmc->priv = host; + ret = mmc_of_parse(dev, &plat->cfg); + if (ret) + return ret; + ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0); if (ret) return ret; -- 2.30.0
Re: [PATCH 4/4] fs: fat: remove trailing periods from long name
On Tue, Feb 02, 2021 at 07:05:53AM +0100, Heinrich Schuchardt wrote:
> Am 2. Februar 2021 00:54:58 MEZ schrieb AKASHI Takahiro
> :
> >On Mon, Feb 01, 2021 at 01:34:59PM +0100, Heinrich Schuchardt wrote:
> >> On 01.02.21 09:18, AKASHI Takahiro wrote:
> >> > On Sun, Jan 31, 2021 at 12:09:53AM +0100, Heinrich Schuchardt
> >wrote:
> >> >> The FAT32 File System Specification [1] requires leading and
> >trailing
> >> >> spaces as well as trailing periods of long names to be ignored.
> >> >>
> >> >> This renders a test for '.' and '..' as file name superfluous.
> >> >>
> >> >> But we must check that the resulting name has at least one
> >character.
> >> >>
> >> >> [1]
> >> >> Microsoft Extensible Firmware Initiative
> >> >> FAT32 File System Specification
> >> >> Version 1.03, December 6, 2000
> >> >> Microsoft Corporation
> >> >> https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf
> >> >>
> >> >> Signed-off-by: Heinrich Schuchardt
> >> >> ---
> >> >> fs/fat/fat_write.c | 29 +++--
> >> >> 1 file changed, 23 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> >> >> index 0f4786ef0f..1b0a0eda09 100644
> >> >> --- a/fs/fat/fat_write.c
> >> >> +++ b/fs/fat/fat_write.c
> >> >> @@ -1237,12 +1237,32 @@ again:
> >> >> }
> >> >>
> >> >> *last_slash_cont = '\0';
> >> >> - *basename = last_slash_cont + 1;
> >> >> + filename = last_slash_cont + 1;
> >> >> } else {
> >> >> *dirname = "/"; /* root by default */
> >> >> - *basename = filename;
> >> >> }
> >> >>
> >> >> + /*
> >> >> +* The FAT32 File System Specification v1.03 requires leading
> >and
> >> >> +* trailing spaces as well as trailing periods to be ignored.
> >> >> +*/
> >> >> + for (; *filename == ' '; ++filename)
> >> >> + ;
> >> >> + /* Remove trailing periods and spaces */
> >> >> + for (p = filename + strlen(filename) - 1; p >= filename; --p) {
> >> >> + switch (*p) {
> >> >> + case ' ':
> >> >> + case '.':
> >> >> + *p = 0;
> >> >> + break;
> >> >> + default:
> >> >> + goto done;
> >> >> + }
> >> >> + }
> >> >
> >> > Given the semantics of the functions, split_filename() and
> >normalize_longname(),
> >> > the code you added above should be moved to normalize_longname().
> >>
> >> normalize_longname(l_filename, filename) converts the argument
> >filename
> >> to a lowercase string l_filename. The parameter filename remains
> >> unchanged. But it is the value of filename that is used to create the
> >> new directory entry in file_fat_write_at() and fat_mkdir().
> >
> >That is why I also suggested, "I even think it would be best to move it
> >to
> >the caller, file_fat_write_at() or fat_mkdir()."
> >
> >> So moving the change to normalize_longname() will not lead to the
> >> intended behavior.
> >>
> >> Removing leading and trailing blanks fits well into the task of
> >> split_filename to identify the actual file name.
> >
> >Again, "." and ".." are legal directory names.
> >To reject a request of creating such names is a caller's job,
> >not split_filename()'s as its name suggests.
> >
>
> These filenames are illegal for all callers.
The purpose of split_filename() is to separate a file name from
its directory path. As I said, excluding "." or ".." is out of this
function's scope, but the caller's semantics.
> We should not duplicate code in each caller.
I don't think so.
handling "." or ".." entry and removing leading/trailing spaces
are totally different.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
>
> >-Takahiro Akashi
> >
> >> Best regards
> >>
> >> Heinrich
> >>
> >> >
> >> >> +done:
> >> >> + *basename = filename;
> >> >> +
> >> >> return 0;
> >> >> }
> >> >>
> >> >> @@ -1260,10 +1280,7 @@ static int normalize_longname(char
> >*l_filename, const char *filename)
> >> >> {
> >> >> const char *p, illegal[] = "<>:\"/\\|?*";
> >> >>
> >> >> - if (!strcmp(filename, ".") || !strcmp(filename, ".."))
> >> >> - return -1;
> >> >
> >> > It would be better for the check above to remain here as "." and
> >".." are
> >> > legal directory names. (I even think it would be best to move it to
> >> > the caller, file_fat_write_at() or fat_mkdir().)
> >> >
> >> > I think that the suggested sequence would be more intuitive for
> >> > better understanding of what Windows requirements say.
> >> >
> >> > -Takahiro Akashi
> >> >
> >> >> - if (strlen(filename) >= VFAT_MAXLEN_BYTES)
> >> >> + if (!*filename || strlen(filename) >= VFAT_MAXLEN_BYTES)
> >> >> return -1;
> >> >>
> >> >> for (p = filename; *p; ++p) {
> >> >> --
> >> >> 2.29.2
> >> >>
> >>
>
RE: Subject: [PATCH v2 4/4] board/km: add support for seli8 design based on nxp ls102x
>-Original Message- >From: Aleksandar Gerasimovski powergrids.com> >Sent: Tuesday, January 19, 2021 4:11 PM >To: Priyanka Jain (OSS) ; u-boot@lists.denx.de >Cc: Valentin Longchamp ; Holger >Brunck ; Rainer Boschung >; Matteo Ghidoni > >Subject: Subject: [PATCH v2 4/4] board/km: add support for seli8 design based >on >nxp ls102x > >The SELI8 design is a new tdm service unit card for Hitachi-Powergrids XMC and >FOX product lines. > >It is based on NXP LS1021 SoC and it provides following interfaces: > - IFC interface for NOR, NAND and external FPGA's > - 1 x RGMII ETH for debug purposes > - 2 x SGMII ETH for management communication via back-plane > - 1 x uQE HDLC for management communication via back-plane > - 1 x I2C for peripheral devices > - 1 x SPI for peripheral devices > - 1 x UART for debug logging > >It is foreseen that the design will be later re-used for another XMC and FOX >service cards with similar SoC requirements. > >Signed-off-by: Rainer Boschung >Signed-off-by: Matteo Ghidoni >Signed-off-by: Aleksandar Gerasimovski powergrids.com> >--- Kindly fix below checkpatch errors /warnings WARNING: 'AYSNC' may be misspelled - perhaps 'ASYNC'? #799: FILE: include/configs/km/pg-wcom-ls102xa.h:64: + CSOR_NOR_NOR_MODE_AYSNC_NOR | \ ERROR: All commands are managed by Kconfig #953: FILE: include/configs/km/pg-wcom-ls102xa.h:218: +#define CONFIG_CMDLINE_TAG total: 1 errors, 2 warnings, 0 checks, 964 lines checked Regards Priyanka
Re: [PATCH 4/4] fs: fat: remove trailing periods from long name
Am 2. Februar 2021 00:54:58 MEZ schrieb AKASHI Takahiro
:
>On Mon, Feb 01, 2021 at 01:34:59PM +0100, Heinrich Schuchardt wrote:
>> On 01.02.21 09:18, AKASHI Takahiro wrote:
>> > On Sun, Jan 31, 2021 at 12:09:53AM +0100, Heinrich Schuchardt
>wrote:
>> >> The FAT32 File System Specification [1] requires leading and
>trailing
>> >> spaces as well as trailing periods of long names to be ignored.
>> >>
>> >> This renders a test for '.' and '..' as file name superfluous.
>> >>
>> >> But we must check that the resulting name has at least one
>character.
>> >>
>> >> [1]
>> >> Microsoft Extensible Firmware Initiative
>> >> FAT32 File System Specification
>> >> Version 1.03, December 6, 2000
>> >> Microsoft Corporation
>> >> https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf
>> >>
>> >> Signed-off-by: Heinrich Schuchardt
>> >> ---
>> >> fs/fat/fat_write.c | 29 +++--
>> >> 1 file changed, 23 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
>> >> index 0f4786ef0f..1b0a0eda09 100644
>> >> --- a/fs/fat/fat_write.c
>> >> +++ b/fs/fat/fat_write.c
>> >> @@ -1237,12 +1237,32 @@ again:
>> >> }
>> >>
>> >> *last_slash_cont = '\0';
>> >> - *basename = last_slash_cont + 1;
>> >> + filename = last_slash_cont + 1;
>> >> } else {
>> >> *dirname = "/"; /* root by default */
>> >> - *basename = filename;
>> >> }
>> >>
>> >> + /*
>> >> + * The FAT32 File System Specification v1.03 requires leading
>and
>> >> + * trailing spaces as well as trailing periods to be ignored.
>> >> + */
>> >> + for (; *filename == ' '; ++filename)
>> >> + ;
>> >> + /* Remove trailing periods and spaces */
>> >> + for (p = filename + strlen(filename) - 1; p >= filename; --p) {
>> >> + switch (*p) {
>> >> + case ' ':
>> >> + case '.':
>> >> + *p = 0;
>> >> + break;
>> >> + default:
>> >> + goto done;
>> >> + }
>> >> + }
>> >
>> > Given the semantics of the functions, split_filename() and
>normalize_longname(),
>> > the code you added above should be moved to normalize_longname().
>>
>> normalize_longname(l_filename, filename) converts the argument
>filename
>> to a lowercase string l_filename. The parameter filename remains
>> unchanged. But it is the value of filename that is used to create the
>> new directory entry in file_fat_write_at() and fat_mkdir().
>
>That is why I also suggested, "I even think it would be best to move it
>to
>the caller, file_fat_write_at() or fat_mkdir()."
>
>> So moving the change to normalize_longname() will not lead to the
>> intended behavior.
>>
>> Removing leading and trailing blanks fits well into the task of
>> split_filename to identify the actual file name.
>
>Again, "." and ".." are legal directory names.
>To reject a request of creating such names is a caller's job,
>not split_filename()'s as its name suggests.
>
These filenames are illegal for all callers.
We should not duplicate code in each caller.
Best regards
Heinrich
>-Takahiro Akashi
>
>> Best regards
>>
>> Heinrich
>>
>> >
>> >> +done:
>> >> + *basename = filename;
>> >> +
>> >> return 0;
>> >> }
>> >>
>> >> @@ -1260,10 +1280,7 @@ static int normalize_longname(char
>*l_filename, const char *filename)
>> >> {
>> >> const char *p, illegal[] = "<>:\"/\\|?*";
>> >>
>> >> - if (!strcmp(filename, ".") || !strcmp(filename, ".."))
>> >> - return -1;
>> >
>> > It would be better for the check above to remain here as "." and
>".." are
>> > legal directory names. (I even think it would be best to move it to
>> > the caller, file_fat_write_at() or fat_mkdir().)
>> >
>> > I think that the suggested sequence would be more intuitive for
>> > better understanding of what Windows requirements say.
>> >
>> > -Takahiro Akashi
>> >
>> >> - if (strlen(filename) >= VFAT_MAXLEN_BYTES)
>> >> + if (!*filename || strlen(filename) >= VFAT_MAXLEN_BYTES)
>> >> return -1;
>> >>
>> >> for (p = filename; *p; ++p) {
>> >> --
>> >> 2.29.2
>> >>
>>
RE: [v3 16/33] configs: ls1021atwr: enable CONFIG_MPC8XXX_GPIO
>-Original Message- >From: U-Boot On Behalf Of Biwen Li >Sent: Thursday, January 28, 2021 3:10 PM >To: Priyanka Jain >Cc: Jiafei Pan ; u-boot@lists.denx.de; Xiaobo Xie >; Biwen Li >Subject: [v3 16/33] configs: ls1021atwr: enable CONFIG_MPC8XXX_GPIO > >From: Biwen Li > >Enable CONFIG_MPC8XXX_GPIO for board ls1021atwr > >Signed-off-by: Biwen Li >--- Kindly fix below build error for ls1021atwr platforms +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../drivers/gpio/mpc8xxx_gpio.c: In function â mpc8xxx_gpio_get_valâ : +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../drivers/gpio/mpc8xxx_gpio.c:50:29: error: dereferencing pointer to incomplete type â struct ccsr_gpioâ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi)return in_le32(&data->base->gpdat) & mask; +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ^ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../include/linux/byteorder/little_endian.h:35:51: note: in definition of macro â __le32_to_cpuâ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi)^ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../arch/arm/include/asm/io.h:105:25: note: in expansion of macro â __arch_getlâ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) #define __raw_readl(a) __arch_getl(a) +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ^~~ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../arch/arm/include/asm/io.h:173:49: note: in expansion of macro â __raw_readlâ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) #define in_arch(type,endian,a) endian##_to_cpu(__raw_read##type(a)) +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ^~ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../arch/arm/include/asm/io.h:180:20: note: in expansion of macro â in_archâ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) #define in_le32(a) in_arch(l,le32,a) +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ^~~ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../drivers/gpio/mpc8xxx_gpio.c:50:10: note: in expansion of macro â in_le32â +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ^~~ +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../drivers/gpio/mpc8xxx_gpio.c: In function â mpc8xxx_gpio_of_to_platâ : +(ls1021atwr_nor ls1021atwr_nor_lpuart ls1021atwr_nor_SECURE_BOOT ls1021atwr_qspi ls1021atwr_sdcard_ifc ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021atwr_sdcard_qspi) ../drivers/gpio/mpc8xxx_gpio.c:184:6: error: dereferencing pointer to incomplete type â struct mpc8xxx_gpio_platâ +(ls1021atwr_nor ls1021atwr_nor_lp
Re: Intermittent failure with test_efi_selftest_text_input
Hi Simon, On Sun, Jan 31, 2021 at 11:45 AM Simon Glass wrote: > > Hi Heinrich and Bin, > > On Sat, 12 Sept 2020 at 14:15, Simon Glass wrote: > > > > Hi Heinrich, > > > > On Sat, 12 Sep 2020 at 14:11, Heinrich Schuchardt > > wrote: > > > > > > Am 12. September 2020 18:40:17 MESZ schrieb Simon Glass > > > : > > >> > > >> Hi Heinrich, > > >> > > >> I am seeing a failure in gitlab every now and then. Here is an example > > >> of a build that failed and then succeeded on retry: > > >> > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/jobs/149201 > > >> > > >> Here's the failing trace: > > >> > > >> https://pastebin.com/bdu6jmVy > > >> > > >> Do you have any ideas? > > >> > > >> Regards, > > >> Simon > > > > > > > > > I see no error in the EFI test itself. > > > > > > > > There seems to be some bug in QEMU: > > > > > > CPU: QEMU Virtual CPU version 2.5+ > > > DRAM: 128 MiB > > > unable to find cpus device > > > Warning: MP init failure > > > > +Bin Meng have you seen this before? Is it OK to do mp_init on qemu? > > > > > > > > > > I have no access to a computer this weekend. Please put the log into a > > > mail as I think Pastebin does not keep pastes for long. > > > > Just to note this is still happening, e.g.: > > https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/jobs/213892 I will take a look. Regards, Bin
Re: [PATCH] nvme: Fix cache alignment
On Sun, Jan 31, 2021 at 1:53 AM Marek Vasut wrote:
>
> The various structures in the driver are already correcty padded and
typo: correctly
> cache aligned in memory, however the cache operations are called on
> the structure sizes, which themselves might not be cache aligned. Add
> the necessary rounding to fix this, which permits the nvme to work on
> arm64.
+ARM guys
Which ARM64 SoC did you test this with?
The round down in this patch should be unnecessary. But it's better to
figure out which call to dcache_xxx() with an unaligned end address.
>
> Signed-off-by: Marek Vasut
> Cc: Bin Meng
> ---
> drivers/nvme/nvme.c | 50 +
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> index 5d6331ad34..758415a53b 100644
> --- a/drivers/nvme/nvme.c
> +++ b/drivers/nvme/nvme.c
> @@ -53,6 +53,27 @@ struct nvme_queue {
> unsigned long cmdid_data[];
> };
>
> +static void nvme_align_dcache_range(void *start, unsigned long size,
> + unsigned long *s, unsigned long *e)
> +{
> + *s = rounddown((uintptr_t)start, ARCH_DMA_MINALIGN);
> + *e = roundup((uintptr_t)start + size, ARCH_DMA_MINALIGN);
> +}
> +
> +static void nvme_flush_dcache_range(void *start, unsigned long size)
> +{
> + unsigned long s, e;
> + nvme_align_dcache_range(start, size, &s, &e);
> + flush_dcache_range(s, e);
> +}
> +
> +static void nvme_invalidate_dcache_range(void *start, unsigned long size)
> +{
> + unsigned long s, e;
> + nvme_align_dcache_range(start, size, &s, &e);
> + invalidate_dcache_range(s, e);
> +}
> +
> static int nvme_wait_ready(struct nvme_dev *dev, bool enabled)
> {
> u32 bit = enabled ? NVME_CSTS_RDY : 0;
> @@ -129,8 +150,7 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64
> *prp2,
> }
> *prp2 = (ulong)dev->prp_pool;
>
> - flush_dcache_range((ulong)dev->prp_pool, (ulong)dev->prp_pool +
> - dev->prp_entry_num * sizeof(u64));
> + nvme_flush_dcache_range(dev->prp_pool, dev->prp_entry_num *
> sizeof(u64));
>
> return 0;
> }
> @@ -144,10 +164,8 @@ static __le16 nvme_get_cmd_id(void)
>
> static u16 nvme_read_completion_status(struct nvme_queue *nvmeq, u16 index)
> {
> - u64 start = (ulong)&nvmeq->cqes[index];
> - u64 stop = start + sizeof(struct nvme_completion);
> -
> - invalidate_dcache_range(start, stop);
> + nvme_invalidate_dcache_range(&nvmeq->cqes[index],
> +sizeof(struct nvme_completion));
>
> return le16_to_cpu(readw(&(nvmeq->cqes[index].status)));
> }
> @@ -163,8 +181,7 @@ static void nvme_submit_cmd(struct nvme_queue *nvmeq,
> struct nvme_command *cmd)
> u16 tail = nvmeq->sq_tail;
>
> memcpy(&nvmeq->sq_cmds[tail], cmd, sizeof(*cmd));
> - flush_dcache_range((ulong)&nvmeq->sq_cmds[tail],
> - (ulong)&nvmeq->sq_cmds[tail] + sizeof(*cmd));
> + nvme_flush_dcache_range(&nvmeq->sq_cmds[tail], sizeof(*cmd));
>
> if (++tail == nvmeq->q_depth)
> tail = 0;
> @@ -338,8 +355,7 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16
> qid)
> nvmeq->cq_phase = 1;
> nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride];
> memset((void *)nvmeq->cqes, 0, NVME_CQ_SIZE(nvmeq->q_depth));
> - flush_dcache_range((ulong)nvmeq->cqes,
> - (ulong)nvmeq->cqes + NVME_CQ_SIZE(nvmeq->q_depth));
> + nvme_flush_dcache_range(nvmeq->cqes, NVME_CQ_SIZE(nvmeq->q_depth));
> dev->online_queues++;
> }
>
> @@ -466,13 +482,13 @@ int nvme_identify(struct nvme_dev *dev, unsigned nsid,
>
> c.identify.cns = cpu_to_le32(cns);
>
> - invalidate_dcache_range(dma_addr,
> - dma_addr + sizeof(struct nvme_id_ctrl));
> + nvme_invalidate_dcache_range((void *)dma_addr,
> +sizeof(struct nvme_id_ctrl));
>
> ret = nvme_submit_admin_cmd(dev, &c, NULL);
> if (!ret)
> - invalidate_dcache_range(dma_addr,
> - dma_addr + sizeof(struct
> nvme_id_ctrl));
> + nvme_invalidate_dcache_range((void *)dma_addr,
> +sizeof(struct nvme_id_ctrl));
>
> return ret;
> }
> @@ -729,8 +745,7 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t
> blknr,
> u16 lbas = 1 << (dev->max_transfer_shift - ns->lba_shift);
> u64 total_lbas = blkcnt;
>
> - flush_dcache_range((unsigned long)buffer,
> - (unsigned long)buffer + total_len);
> + nvme_flush_dcache_range(buffer, total_len);
>
> c.rw.opcode = read ? nvme_cmd_read : nvme_cmd_write;
> c.rw.flags = 0;
> @@ -767,8 +782,7 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t
> bl
Re: [PATCH] spi: imx: Implement set_speed
On Thu, Jan 28, 2021 at 12:01 AM Marek Vasut wrote:
>
> The set_speed() callback should configure the bus speed, make it so.
>
> Signed-off-by: Marek Vasut
> Cc: Jagan Teki
> Cc: Stefano Babic
> ---
> drivers/spi/mxc_spi.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index 553a0315df5..47100d89aef 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -661,7 +661,10 @@ static int mxc_spi_release_bus(struct udevice *dev)
>
> static int mxc_spi_set_speed(struct udevice *bus, uint speed)
> {
> - /* Nothing to do */
> + struct mxc_spi_slave *mxcs = dev_get_platdata(bus);
This is now renamed to dev_get_plat()
> +
> + mxcs->max_hz = speed;
> +
> return 0;
> }
>
Otherwise,
Reviewed-by: Bin Meng
[PATCH v2 3/3] mmc: mmc_spi: Document the 3 local functions
From: Bin Meng
mmc_spi_sendcmd(), mmc_spi_readdata() and mmc_spi_writedata() are
currently undocumented. Add comment blocks to explain the arguments
and the return value.
Signed-off-by: Bin Meng
Reviewed-by: Jaehoon Chung
---
(no changes since v1)
drivers/mmc/mmc_spi.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index fbdbcf7..e2d7879 100644
--- a/drivers/mmc/mmc_spi.c
+++ b/drivers/mmc/mmc_spi.c
@@ -37,7 +37,8 @@
#define SPI_RESPONSE_CRC_ERR ((5 << 1)|1)
#define SPI_RESPONSE_WRITE_ERR ((6 << 1)|1)
-/* Read and write blocks start with these tokens and end with crc;
+/*
+ * Read and write blocks start with these tokens and end with crc;
* on error, read tokens act like a subset of R2_SPI_* values.
*/
/* single block write multiblock read */
@@ -70,6 +71,20 @@ struct mmc_spi_priv {
struct spi_slave *spi;
};
+/**
+ * mmc_spi_sendcmd() - send a command to the SD card
+ *
+ * @dev: mmc_spi device
+ * @cmdidx:command index
+ * @cmdarg:command argument
+ * @resp_type: card response type
+ * @resp: buffer to store the card response
+ * @resp_size: size of the card response
+ * @resp_match:if true, compare each of received bytes with
@resp_match_value
+ * @resp_match_value: a value to be compared with each of received bytes
+ * @r1b: if true, receive additional bytes for busy signal token
+ * @return 0 if OK, -ETIMEDOUT if no card response is received, -ve on error
+ */
static int mmc_spi_sendcmd(struct udevice *dev,
ushort cmdidx, u32 cmdarg, u32 resp_type,
u8 *resp, u32 resp_size,
@@ -159,6 +174,15 @@ static int mmc_spi_sendcmd(struct udevice *dev,
return 0;
}
+/**
+ * mmc_spi_readdata() - read data block(s) from the SD card
+ *
+ * @dev: mmc_spi device
+ * @xbuf: buffer of the actual data (excluding token and crc) to read
+ * @bcnt: number of data blocks to transfer
+ * @bsize: size of the actual data (excluding token and crc) in bytes
+ * @return 0 if OK, -ECOMM if crc error, -ETIMEDOUT on other errors
+ */
static int mmc_spi_readdata(struct udevice *dev,
void *xbuf, u32 bcnt, u32 bsize)
{
@@ -207,6 +231,16 @@ static int mmc_spi_readdata(struct udevice *dev,
return ret;
}
+/**
+ * mmc_spi_writedata() - write data block(s) to the SD card
+ *
+ * @dev: mmc_spi device
+ * @xbuf: buffer of the actual data (excluding token and crc) to write
+ * @bcnt: number of data blocks to transfer
+ * @bsize: size of actual data (excluding token and crc) in bytes
+ * @multi: indicate a transfer by multiple block write command (CMD25)
+ * @return 0 if OK, -ECOMM if crc error, -ETIMEDOUT on other errors
+ */
static int mmc_spi_writedata(struct udevice *dev, const void *xbuf,
u32 bcnt, u32 bsize, int multi)
{
--
2.7.4
[PATCH v2 2/3] mmc: mmc_spi: Fix potential spec violation in receiving card response
From: Bin Meng
After command is sent and before card response shows up on the line,
there is a variable number of clock cycles in between called Ncr.
The spec [1] says the minimum is 1 byte and the maximum is 8 bytes.
Current logic in mmc_spi_sendcmd() has a flaw that it could only work
with certain SD cards with their Ncr being just 1 byte.
When resp_match is false, the codes try to receive only 1 byte from
the SD card. On the other hand when resp_match is true, the logic
happens to be no problem as it loops until timeout to receive as many
bytes as possible to see a match of the expected resp_match_value.
However not every call to mmc_spi_sendcmd() is made with resp_match
being true hence this exposes a potential issue with SD cards that
have a larger Ncr value.
Given no issue was reported as of today, we can reasonably conclude
that all cards being used on the supported boards happen to have a 1
byte Ncr timing requirement. But a broken case can be triggered by
utilizing QEMU to emulate a larger value of Ncr (by default 1 byte
Ncr is used on QEMU). This commit fixes such potential spec violation
to improve the card compatibility.
[1] "Physical Layer Specification Version 8.00"
chapter 7.5.1: Command / Response
chapter 7.5.4: Timing Values
Signed-off-by: Bin Meng
Reviewed-by: Jaehoon Chung
---
(no changes since v1)
drivers/mmc/mmc_spi.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index a06862a..fbdbcf7 100644
--- a/drivers/mmc/mmc_spi.c
+++ b/drivers/mmc/mmc_spi.c
@@ -103,29 +103,31 @@ static int mmc_spi_sendcmd(struct udevice *dev,
debug("%s: cmd%d", __func__, cmdidx);
- if (resp_match) {
+ if (resp_match)
r = ~resp_match_value;
- i = CMD_TIMEOUT;
- while (i) {
- ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
- if (ret)
- return ret;
- debug(" resp%d=0x%x", rpos, r);
- rpos++;
- i--;
+ i = CMD_TIMEOUT;
+ while (i) {
+ ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
+ if (ret)
+ return ret;
+ debug(" resp%d=0x%x", rpos, r);
+ rpos++;
+ i--;
+ if (resp_match) {
if (r == resp_match_value)
break;
+ } else {
+ if (!(r & 0x80))
+ break;
}
- if (!i && (r != resp_match_value))
+
+ if (!i)
return -ETIMEDOUT;
}
- for (i = 0; i < resp_size; i++) {
- if (i == 0 && resp_match) {
- resp[i] = resp_match_value;
- continue;
- }
+ resp[0] = r;
+ for (i = 1; i < resp_size; i++) {
ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
if (ret)
return ret;
--
2.7.4
[PATCH v2 1/3] mmc: mmc_spi: Move argument check to the beginning of mmc_spi_sendcmd()
From: Bin Meng
The argument check should happen before any transfer on the SPI lines.
Signed-off-by: Bin Meng
Reviewed-by: Jaehoon Chung
---
Changes in v2:
- move the check before the debug output
drivers/mmc/mmc_spi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index 23b9073..a06862a 100644
--- a/drivers/mmc/mmc_spi.c
+++ b/drivers/mmc/mmc_spi.c
@@ -78,6 +78,9 @@ static int mmc_spi_sendcmd(struct udevice *dev,
int i, rpos = 0, ret = 0;
u8 cmdo[7], r;
+ if (!resp || !resp_size)
+ return 0;
+
debug("%s: cmd%d cmdarg=0x%x resp_type=0x%x "
"resp_size=%d resp_match=%d resp_match_value=0x%x\n",
__func__, cmdidx, cmdarg, resp_type,
@@ -98,9 +101,6 @@ static int mmc_spi_sendcmd(struct udevice *dev,
if (ret)
return ret;
- if (!resp || !resp_size)
- return 0;
-
debug("%s: cmd%d", __func__, cmdidx);
if (resp_match) {
--
2.7.4
[PATCH v2 0/3] mmc: mmc_spi: Fix potential spec violation in receiving card response
After command is sent and before card response shows up on the line, there is a variable number of clock cycles in between called Ncr. The spec [1] says the minimum is 1 byte and the maximum is 8 bytes. Current logic in mmc_spi_sendcmd() has a flaw that it could only work with certain SD cards with their Ncr being just 1 byte. When resp_match is false, the codes try to receive only 1 byte from the SD card. On the other hand when resp_match is true, the logic happens to be no problem as it loops until timeout to receive as many bytes as possible to see a match of the expected resp_match_value. However not every call to mmc_spi_sendcmd() is made with resp_match being true hence this exposes a potential issue with SD cards that have a larger Ncr value. Given no issue was reported as of today, we can reasonably conclude that all cards being used on the supported boards happen to have a 1 byte Ncr timing requirement. But a broken case can be triggered by utilizing QEMU to emulate a larger value of Ncr (by default 1 byte Ncr is used on QEMU). This series fixes such potential spec violation to improve the card compatibility. [1] "Physical Layer Specification Version 8.00" chapter 7.5.1: Command / Response chapter 7.5.4: Timing Values Changes in v2: - move the check before the debug output Bin Meng (3): mmc: mmc_spi: Move argument check to the beginning of mmc_spi_sendcmd() mmc: mmc_spi: Fix potential spec violation in receiving card response mmc: mmc_spi: Document the 3 local functions drivers/mmc/mmc_spi.c | 74 ++- 1 file changed, 55 insertions(+), 19 deletions(-) -- 2.7.4
[PATCH v3] mmc: mmc_spi: Print verbose debug output when crc16 check fails
Add some verbose debug output when crc16 check fails.
Signed-off-by: Bin Meng
Reviewed-by: Jaehoon Chung
---
Changes in v3:
- use expected/got instead of expect/get
Changes in v2:
- do the crc_ok assignment at the the same line where it's defined
drivers/mmc/mmc_spi.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index 46800bb..23b9073 100644
--- a/drivers/mmc/mmc_spi.c
+++ b/drivers/mmc/mmc_spi.c
@@ -181,8 +181,10 @@ static int mmc_spi_readdata(struct udevice *dev,
if (ret)
return ret;
#ifdef CONFIG_MMC_SPI_CRC_ON
- if (be16_to_cpu(crc16_ccitt(0, buf, bsize)) != crc) {
- debug("%s: data crc error\n", __func__);
+ u16 crc_ok = be16_to_cpu(crc16_ccitt(0, buf, bsize));
+ if (crc_ok != crc) {
+ debug("%s: data crc error, expected %04x got
%04x\n",
+ __func__, crc_ok, crc);
r1 = R1_SPI_COM_CRC;
break;
}
--
2.7.4
Re: [PATCH 4/4] fs: fat: remove trailing periods from long name
On Mon, Feb 01, 2021 at 01:34:59PM +0100, Heinrich Schuchardt wrote:
> On 01.02.21 09:18, AKASHI Takahiro wrote:
> > On Sun, Jan 31, 2021 at 12:09:53AM +0100, Heinrich Schuchardt wrote:
> >> The FAT32 File System Specification [1] requires leading and trailing
> >> spaces as well as trailing periods of long names to be ignored.
> >>
> >> This renders a test for '.' and '..' as file name superfluous.
> >>
> >> But we must check that the resulting name has at least one character.
> >>
> >> [1]
> >> Microsoft Extensible Firmware Initiative
> >> FAT32 File System Specification
> >> Version 1.03, December 6, 2000
> >> Microsoft Corporation
> >> https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf
> >>
> >> Signed-off-by: Heinrich Schuchardt
> >> ---
> >> fs/fat/fat_write.c | 29 +++--
> >> 1 file changed, 23 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> >> index 0f4786ef0f..1b0a0eda09 100644
> >> --- a/fs/fat/fat_write.c
> >> +++ b/fs/fat/fat_write.c
> >> @@ -1237,12 +1237,32 @@ again:
> >>}
> >>
> >>*last_slash_cont = '\0';
> >> - *basename = last_slash_cont + 1;
> >> + filename = last_slash_cont + 1;
> >>} else {
> >>*dirname = "/"; /* root by default */
> >> - *basename = filename;
> >>}
> >>
> >> + /*
> >> + * The FAT32 File System Specification v1.03 requires leading and
> >> + * trailing spaces as well as trailing periods to be ignored.
> >> + */
> >> + for (; *filename == ' '; ++filename)
> >> + ;
> >> + /* Remove trailing periods and spaces */
> >> + for (p = filename + strlen(filename) - 1; p >= filename; --p) {
> >> + switch (*p) {
> >> + case ' ':
> >> + case '.':
> >> + *p = 0;
> >> + break;
> >> + default:
> >> + goto done;
> >> + }
> >> + }
> >
> > Given the semantics of the functions, split_filename() and
> > normalize_longname(),
> > the code you added above should be moved to normalize_longname().
>
> normalize_longname(l_filename, filename) converts the argument filename
> to a lowercase string l_filename. The parameter filename remains
> unchanged. But it is the value of filename that is used to create the
> new directory entry in file_fat_write_at() and fat_mkdir().
That is why I also suggested, "I even think it would be best to move it to
the caller, file_fat_write_at() or fat_mkdir()."
> So moving the change to normalize_longname() will not lead to the
> intended behavior.
>
> Removing leading and trailing blanks fits well into the task of
> split_filename to identify the actual file name.
Again, "." and ".." are legal directory names.
To reject a request of creating such names is a caller's job,
not split_filename()'s as its name suggests.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> >
> >> +done:
> >> + *basename = filename;
> >> +
> >>return 0;
> >> }
> >>
> >> @@ -1260,10 +1280,7 @@ static int normalize_longname(char *l_filename,
> >> const char *filename)
> >> {
> >>const char *p, illegal[] = "<>:\"/\\|?*";
> >>
> >> - if (!strcmp(filename, ".") || !strcmp(filename, ".."))
> >> - return -1;
> >
> > It would be better for the check above to remain here as "." and ".." are
> > legal directory names. (I even think it would be best to move it to
> > the caller, file_fat_write_at() or fat_mkdir().)
> >
> > I think that the suggested sequence would be more intuitive for
> > better understanding of what Windows requirements say.
> >
> > -Takahiro Akashi
> >
> >> - if (strlen(filename) >= VFAT_MAXLEN_BYTES)
> >> + if (!*filename || strlen(filename) >= VFAT_MAXLEN_BYTES)
> >>return -1;
> >>
> >>for (p = filename; *p; ++p) {
> >> --
> >> 2.29.2
> >>
>
Re: [PATCH v3 9/9] fastboot: Partition specification
On 2/1/21 5:30 PM, Heinrich Schuchardt wrote: On 01.02.21 17:43, Sean Anderson wrote: This documents the way U-Boot understands partitions specifications. This also updates the fastboot documentation for the changes in the previous commit. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- Changes in v3: - Rebase onto dfu/master Changes in v2: - Move partition documentation under doc/usage doc/android/fastboot.rst | 4 doc/usage/index.rst | 1 + doc/usage/part.rst | 33 + 3 files changed, 38 insertions(+) create mode 100644 doc/usage/part.rst diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst index 16b11399b3..ce513a2a0f 100644 --- a/doc/android/fastboot.rst +++ b/doc/android/fastboot.rst @@ -154,6 +154,10 @@ The device index starts from ``a`` and refers to the interface (e.g. USB controller, SD/MMC controller) or disk index. The partition index starts from ``1`` and describes the partition number on the particular device. +Alternatively, partition types may be specified using :ref:`U-Boot's partition +syntax `. This allows specifying partitions like ``0.1``, +``0#boot``, or ``:3``. The interface is always ``mmc``. + Writing Partition Table --- diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 83cfbafd90..9b64434cb2 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -6,6 +6,7 @@ Use U-Boot fdt_overlays netconsole + part Shell commands -- diff --git a/doc/usage/part.rst b/doc/usage/part.rst There is a part command. That command should be in doc/usage/part.rst. How about doc/usage/partitions.rst? Ok. Perhaps commands should be in a subdirectory of doc/usage then? This will help separate general usage documentation from man pages. new file mode 100644 index 00..e58b529147 --- /dev/null +++ b/doc/usage/part.rst @@ -0,0 +1,33 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. _partitions: + +Partitions +== + +Many U-Boot commands allow specifying partitions like:: + +some_command + +or like:: + +some_command From the above it is not clear what is optional and what can go together. Is this what you meant: :: [.][:|#] Yes. This will work. + +Where + + * ``interface`` is the device interface, like ``mmc`` or ``scsi``. For a full +list of supported interfaces, consult the ``if_typename_str`` array in +``drivers/block/blk-uclass.c`` + * ``devnum`` is the device number. This defaults to 0. + * ``hwpartnum`` is the hardware partition number. This defaults to 0 Could you, please, use a text form like: interface the device interface like mmc or scsi ... devnum the device number ... to match our existing man-pages. Here interface and devnum will automatically be rendered in bold without adding disturbing mark-up. Cf. https://u-boot.readthedocs.io/en/latest/usage/for.html (the user +partition on eMMC devices). + * ``partname`` is the partition name on GPT devices. Partitions do not have +names on MBR devices. + * ``partnum`` is the partition number, starting from 1. The partition number 0 +is special, and specifies that the whole device is to be used as one +"partition." + +If neither ``partname`` nor ``partnum`` is specified and there is a partition +table, then partition 1 is used. If there is no partition table, then the whole +device is used as one "partition." If none of ``devnum``, ``hwpartnum``, +``partnum``, or ``partname`` is specified, then ``devnum`` defaults to the value +of the ``bootdevice`` environmental variable. Please, add examples with their interpretation. Ok. --Sean A reference to the part command might be of interest. Best regards Heinrich
Re: [PATCH v3 9/9] fastboot: Partition specification
On 01.02.21 17:43, Sean Anderson wrote: > This documents the way U-Boot understands partitions specifications. > This also updates the fastboot documentation for the changes in the > previous commit. > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass > --- > > Changes in v3: > - Rebase onto dfu/master > > Changes in v2: > - Move partition documentation under doc/usage > > doc/android/fastboot.rst | 4 > doc/usage/index.rst | 1 + > doc/usage/part.rst | 33 + > 3 files changed, 38 insertions(+) > create mode 100644 doc/usage/part.rst > > diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst > index 16b11399b3..ce513a2a0f 100644 > --- a/doc/android/fastboot.rst > +++ b/doc/android/fastboot.rst > @@ -154,6 +154,10 @@ The device index starts from ``a`` and refers to > the interface (e.g. USB > controller, SD/MMC controller) or disk index. The partition index starts > from ``1`` and describes the partition number on the particular device. > > +Alternatively, partition types may be specified using :ref:`U-Boot's > partition > +syntax `. This allows specifying partitions like ``0.1``, > +``0#boot``, or ``:3``. The interface is always ``mmc``. > + > Writing Partition Table > --- > > diff --git a/doc/usage/index.rst b/doc/usage/index.rst > index 83cfbafd90..9b64434cb2 100644 > --- a/doc/usage/index.rst > +++ b/doc/usage/index.rst > @@ -6,6 +6,7 @@ Use U-Boot > > fdt_overlays > netconsole > + part > > Shell commands > -- > diff --git a/doc/usage/part.rst b/doc/usage/part.rst There is a part command. That command should be in doc/usage/part.rst. How about doc/usage/partitions.rst? > new file mode 100644 > index 00..e58b529147 > --- /dev/null > +++ b/doc/usage/part.rst > @@ -0,0 +1,33 @@ > +.. SPDX-License-Identifier: GPL-2.0+ > +.. _partitions: > + > +Partitions > +== > + > +Many U-Boot commands allow specifying partitions like:: > + > + some_command > + > +or like:: > + > + some_command[.][:|#] > + > +Where > + > + * ``interface`` is the device interface, like ``mmc`` or ``scsi``. > For a full > + list of supported interfaces, consult the ``if_typename_str`` array in > + ``drivers/block/blk-uclass.c`` > + * ``devnum`` is the device number. This defaults to 0. > + * ``hwpartnum`` is the hardware partition number. This defaults to 0 Could you, please, use a text form like: interface the device interface like mmc or scsi ... devnum the device number ... to match our existing man-pages. Here interface and devnum will automatically be rendered in bold without adding disturbing mark-up. Cf. https://u-boot.readthedocs.io/en/latest/usage/for.html > (the user > + partition on eMMC devices). > + * ``partname`` is the partition name on GPT devices. Partitions do > not have > + names on MBR devices. > + * ``partnum`` is the partition number, starting from 1. The partition > number 0 > + is special, and specifies that the whole device is to be used as one > + "partition." > + > +If neither ``partname`` nor ``partnum`` is specified and there is a > partition > +table, then partition 1 is used. If there is no partition table, then > the whole > +device is used as one "partition." If none of ``devnum``, ``hwpartnum``, > +``partnum``, or ``partname`` is specified, then ``devnum`` defaults to > the value > +of the ``bootdevice`` environmental variable. Please, add examples with their interpretation. A reference to the part command might be of interest. Best regards Heinrich
Re: [PATCH v5] net: tftp: Add client support for RFC 7440
On Sat, Jan 30, 2021 at 11:26 PM Ramon Fried wrote:
>
> On Sat, Jan 30, 2021 at 10:39 PM Suneel Garapati
> wrote:
> >
> > Hello Ramon,
> >
> > With TFTP window size as default 1 and enabling TFTPPUT config option
> > results in tftpboot command failure.
> >
> > Attached the pcap files for with and without TFTPPUT enabled.
> > Both captures are the same except that the TFTPPUT option enables ICMP
> > handler
> > and for the response from the server triggers a restart of netloop
> > operation and eventually fails as below.
> >
> > > tftpboot 0x3000 192.168.1.1:Image
> > Waiting for RPM0 LMAC0 link status... 10G_R [10G]
> > Using rvu_pf#0 device
> > TFTP from server 192.168.1.1; our IP address is 192.168.1.16
> > Filename 'Image'. Load address: 0x3000
> > Loading: ## 15.8 MiB
> > 787.1 KiB/s
> > done
> > TFTP server died; starting again
> > >
> >
> > I see the code issue as below on net/tftp.c [v2020.10] –
> > /*
> > * Acknowledge the block just received, which will
> > prompt
> > * the remote for the next one.
> > */
> > if (tftp_cur_block == tftp_next_ack) {
> > tftp_send();
> > tftp_next_ack += tftp_windowsize;
> > }
> >
> > if (len < tftp_block_size) {
> > //if (tftp_windowsize > 1) [Hack in use for
> > now to work around this issue]
> > tftp_send(); [ This
> > causes extra ACK packet send with same block number and causes server
> > to respond with ICMP error]
> > tftp_complete();
> > }
> >
> > I couldn’t try with tftp_windowsize > 1 as the test servers don’t support.
> > I tried with all latest commits on net/tftp.c on top of v2020.10 and
> > still see the issue.
> >
> > This change is introduced in
> > commit cc6b87ecaa96325577a8fafabc0d5972b816bc6c
> > Author: Ramon Fried
> > Date: Sat Jul 18 23:31:46 2020 +0300
> >
> > net: tftp: Add client support for RFC 7440
> >
> > Add support for RFC 7440: "TFTP Windowsize Option".
> >
> > Reverting this commit on v2020.10 also fixes the issue.
> >
> > I would like to know if this extra tftp_send is needed at all or only
> > for window size > 1
RFC7440 is not supported by most TFTP Servers, when adding support for
this feature I didn't even consider TFTPPUT.
I think that the best thing to do is to only support RFC7440 on TFTPGET.
In this case we should remove the tftp_send() when sending a file.
I will create a fix, would you mind testing to see if it works for you ?
Thanks,
Ramon.
> >
> > Regards,
> > Suneel
> Thanks for spotting this, I'll check it out.
Re: [PATCH v5 01/20] mmc: sdhci: Add helper functions for UHS modes
Hi Aswath,
On 1/29/21 11:47 PM, Aswath Govindraju wrote:
> From: Faiz Abbas
>
> Add a set_voltage() function which handles the switch from 3.3V to 1.8V
> for SD card UHS modes.
>
> Signed-off-by: Faiz Abbas
> Signed-off-by: Aswath Govindraju
> ---
> drivers/mmc/sdhci.c | 73 +
> include/sdhci.h | 10 +++
> 2 files changed, 83 insertions(+)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 06289343124e..276b6a08e571 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> static void sdhci_reset(struct sdhci_host *host, u8 mask)
> {
> @@ -509,6 +510,78 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
> sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> }
>
> +static void sdhci_set_voltage(struct sdhci_host *host)
> +{
> + if (IS_ENABLED(CONFIG_MMC_IO_VOLTAGE)) {
> + struct mmc *mmc = (struct mmc *)host->mmc;
> + u32 ctrl;
> +
> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> + switch (mmc->signal_voltage) {
> + case MMC_SIGNAL_VOLTAGE_330:
> +#if CONFIG_IS_ENABLED(DM_REGULATOR)
> + if (mmc->vqmmc_supply) {
> + if
> (regulator_set_enable_if_allowed(mmc->vqmmc_supply, false)) {
> + pr_err("failed to disable
> vqmmc-supply\n");
> + return;
> + }
> +
> + if (regulator_set_value(mmc->vqmmc_supply,
> 330)) {
> + pr_err("failed to set vqmmc-voltage to
> 3.3V\n");
> + return;
> + }
> +
> + if
> (regulator_set_enable_if_allowed(mmc->vqmmc_supply, true)) {
> + pr_err("failed to enable
> vqmmc-supply\n");
> + return;
> + }
> + }
> +#endif
> + /* 3.3V regulator output should be stable within 5 ms */
> + mdelay(5);
> + if (IS_SD(mmc)) {
> + ctrl &= ~SDHCI_CTRL_VDD_180;
> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> + }
According to Specification, after Signal bit was changed to 0 from 1, it needs
to wait for stable within 5ms.
Isn't it right that mdelay(5) locates at here?
Best Regards,
Jaehoon Chung
> + break;
> + case MMC_SIGNAL_VOLTAGE_180:
> +#if CONFIG_IS_ENABLED(DM_REGULATOR)
> + if (mmc->vqmmc_supply) {
> + if
> (regulator_set_enable_if_allowed(mmc->vqmmc_supply, false)) {
> + pr_err("failed to disable
> vqmmc-supply\n");
> + return;
> + }
> +
> + if (regulator_set_value(mmc->vqmmc_supply,
> 180)) {
> + pr_err("failed to set vqmmc-voltage to
> 1.8V\n");
> + return;
> + }
> +
> + if
> (regulator_set_enable_if_allowed(mmc->vqmmc_supply, true)) {
> + pr_err("failed to enable
> vqmmc-supply\n");
> + return;
> + }
> + }
> +#endif
> + if (IS_SD(mmc)) {
> + ctrl |= SDHCI_CTRL_VDD_180;
> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> + }
> + break;
> + default:
> + /* No signal voltage switch required */
> + return;
> + }
> + }
> +}
> +
> +void sdhci_set_control_reg(struct sdhci_host *host)
> +{
> + sdhci_set_voltage(host);
> + sdhci_set_uhs_timing(host);
> +}
> +
> #ifdef CONFIG_DM_MMC
> static int sdhci_set_ios(struct udevice *dev)
> {
> diff --git a/include/sdhci.h b/include/sdhci.h
> index 3e5a64981857..0ae9471ad749 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -491,6 +491,16 @@ void sdhci_set_uhs_timing(struct sdhci_host *host);
> /* Export the operations to drivers */
> int sdhci_probe(struct udevice *dev);
> int sdhci_set_clock(struct mmc *mmc, unsigned int clock);
> +
> +/**
> + * sdhci_set_control_reg - Set control registers
> + *
> + * This is used set up control registers for voltage level and UHS speed
> + * mode.
> + *
> + * @host: SDHCI host structure
> + */
> +void sdhci_set_control_reg(struct sdhci_host *host);
> extern const struct dm_mmc_ops sdhci_ops;
> #else
> #endif
>
Re: [PATCH 2/3] mmc: mmc_spi: Fix potential spec violation in receiving card response
On 2/1/21 1:20 PM, Bin Meng wrote:
> From: Bin Meng
>
> After command is sent and before card response shows up on the line,
> there is a variable number of clock cycles in between called Ncr.
> The spec [1] says the minimum is 1 byte and the maximum is 8 bytes.
>
> Current logic in mmc_spi_sendcmd() has a flaw that it could only work
> with certain SD cards with their Ncr being just 1 byte.
>
> When resp_match is false, the codes try to receive only 1 byte from
> the SD card. On the other hand when resp_match is true, the logic
> happens to be no problem as it loops until timeout to receive as many
> bytes as possible to see a match of the expected resp_match_value.
> However not every call to mmc_spi_sendcmd() is made with resp_match
> being true hence this exposes a potential issue with SD cards that
> have a larger Ncr value.
>
> Given no issue was reported as of today, we can reasonably conclude
> that all cards being used on the supported boards happen to have a 1
> byte Ncr timing requirement. But a broken case can be triggered by
> utilizing QEMU to emulate a larger value of Ncr (by default 1 byte
> Ncr is used on QEMU). This commit fixes such potential spec violation
> to improve the card compatibility.
>
> [1] "Physical Layer Specification Version 8.00"
> chapter 7.5.1: Command / Response
> chapter 7.5.4: Timing Values
>
> Signed-off-by: Bin Meng
Reviewed-by: Jaehoon Chung
Best Regards,
Jaehoon Chung
> ---
>
> drivers/mmc/mmc_spi.c | 32 +---
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
> index 85a2818..5ec6b0f 100644
> --- a/drivers/mmc/mmc_spi.c
> +++ b/drivers/mmc/mmc_spi.c
> @@ -103,29 +103,31 @@ static int mmc_spi_sendcmd(struct udevice *dev,
>
> debug("%s: cmd%d", __func__, cmdidx);
>
> - if (resp_match) {
> + if (resp_match)
> r = ~resp_match_value;
> - i = CMD_TIMEOUT;
> - while (i) {
> - ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
> - if (ret)
> - return ret;
> - debug(" resp%d=0x%x", rpos, r);
> - rpos++;
> - i--;
> + i = CMD_TIMEOUT;
> + while (i) {
> + ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
> + if (ret)
> + return ret;
> + debug(" resp%d=0x%x", rpos, r);
> + rpos++;
> + i--;
>
> + if (resp_match) {
> if (r == resp_match_value)
> break;
> + } else {
> + if (!(r & 0x80))
> + break;
> }
> - if (!i && (r != resp_match_value))
> +
> + if (!i)
> return -ETIMEDOUT;
> }
>
> - for (i = 0; i < resp_size; i++) {
> - if (i == 0 && resp_match) {
> - resp[i] = resp_match_value;
> - continue;
> - }
> + resp[0] = r;
> + for (i = 1; i < resp_size; i++) {
> ret = dm_spi_xfer(dev, 1 * 8, NULL, &r, 0);
> if (ret)
> return ret;
>
Re: [PATCH 1/1] sandbox: host bind must close file descriptor
Hi Heinrich,
On Mon, 1 Feb 2021 at 14:08, Heinrich Schuchardt wrote:
>
> On 2/1/21 9:44 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sun, 31 Jan 2021 at 03:39, Heinrich Schuchardt
> > wrote:
> >>
> >> Each invocation of the 'host bind' command with a file name argument opens
> >> a file descriptor. The next invocation of the 'host bind' command destroys
> >> the block device but the file descriptor remains open. The same holds true
> >> for the 'unbind blk' command.
> >>
> >> Close the file descriptor when unbinding the host block device.
> >>
> >> Signed-off-by: Heinrich Schuchardt
> >> ---
> >> drivers/block/sandbox.c | 20
> >> 1 file changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
> >> index f57f690d3c..02992ac34f 100644
> >> --- a/drivers/block/sandbox.c
> >> +++ b/drivers/block/sandbox.c
> >> @@ -230,6 +230,25 @@ int host_get_dev_err(int devnum, struct blk_desc
> >> **blk_devp)
> >> }
> >>
> >> #ifdef CONFIG_BLK
> >> +
> >> +int sandbox_host_unbind(struct udevice *dev)
> >> +{
> >> + struct host_block_dev *host_dev;
> >> +
> >> + host_dev = dev_get_plat(dev);
> >> + if (host_dev) {
> >> + if (host_dev->fd) {
> >> + os_close(host_dev->fd);
> >> + host_dev->fd = 0;
> >> + } else {
> >> + log_err("missing file descriptor\n");
> >
> > How does this happen?
> >
> >> + }
> >> + } else {
> >> + log_err("missing platform data\n");
> >
> > I don't think this case can happen. See the .plat_auto below.
>
> Both would only happen if there were a bug in the rest of the code.
>
> Do you prefer assert_noisy() or should the messages be removed?
Well a bug in the core driver model is the only way the platform data
can be missing. We have tests to protect against that. So I think that
should be removed.
For the file descriptor, I don't think we want a check against a bug
in host_dev_bind(). Let's just make them consistent as you have (one
sets it, one clears it), perhaps adding a comment if you like.
Regards,
Simon
Re: [PATCH] tools: Remove #include
On Wed, Jan 27, 2021 at 04:34:24PM +0100, Pali Rohár wrote:
> Header file version.h includes also autogenerated file timestamp.h which
> is recompiled on every time when SOURCE_DATE_EPOCH change.
>
> Tools do not use build time therefore they do not have to include
> timestamp.h file.
>
> This change prevents recompiling tools every time when SOURCE_DATE_EPOCH
> changes.
>
> Signed-off-by: Pali Rohár
> ---
> tools/dumpimage.c | 2 +-
> tools/mkenvimage.c | 2 +-
> tools/mkimage.c| 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/dumpimage.c b/tools/dumpimage.c
> index e5481435a7..54c2517c9e 100644
> --- a/tools/dumpimage.c
> +++ b/tools/dumpimage.c
> @@ -7,7 +7,7 @@
>
> #include "dumpimage.h"
> #include
> -#include
> +#include "generated/version_autogenerated.h"
>
> static void usage(void);
>
I don't know if I really like this approach. It also seems inconsistent
as we have for example tools/fit-image.c that has content from
generated/version_autogenerated.h but wasn't changed by this patch (nor
the rest of the series).
I think a bit more invasive approach is required here. I like the
approach you used elsewhere, and as best I can see,
the only place U_BOOT_VERSION_STRING, which is where we have the
timestamp itself, is used is cmd/version.c (and defined in version.h).
We should isolate the file that has the date such that it's only
in one file and referenced in one file. This means probably removing
the version_string global from arch/{powerpc,m68k} and dropping __weak
from cmd/version.c and seeing what happens, as well.
Do you see where I'm thinking with this, or do I need to try and explain
a bit more? Thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 1/1] sandbox: host bind must close file descriptor
On 2/1/21 9:44 PM, Simon Glass wrote:
Hi Heinrich,
On Sun, 31 Jan 2021 at 03:39, Heinrich Schuchardt wrote:
Each invocation of the 'host bind' command with a file name argument opens
a file descriptor. The next invocation of the 'host bind' command destroys
the block device but the file descriptor remains open. The same holds true
for the 'unbind blk' command.
Close the file descriptor when unbinding the host block device.
Signed-off-by: Heinrich Schuchardt
---
drivers/block/sandbox.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
index f57f690d3c..02992ac34f 100644
--- a/drivers/block/sandbox.c
+++ b/drivers/block/sandbox.c
@@ -230,6 +230,25 @@ int host_get_dev_err(int devnum, struct blk_desc
**blk_devp)
}
#ifdef CONFIG_BLK
+
+int sandbox_host_unbind(struct udevice *dev)
+{
+ struct host_block_dev *host_dev;
+
+ host_dev = dev_get_plat(dev);
+ if (host_dev) {
+ if (host_dev->fd) {
+ os_close(host_dev->fd);
+ host_dev->fd = 0;
+ } else {
+ log_err("missing file descriptor\n");
How does this happen?
+ }
+ } else {
+ log_err("missing platform data\n");
I don't think this case can happen. See the .plat_auto below.
Both would only happen if there were a bug in the rest of the code.
Do you prefer assert_noisy() or should the messages be removed?
Best regards
Heinrich
+ }
+ return 0;
+}
+
static const struct blk_ops sandbox_host_blk_ops = {
.read = host_block_read,
.write = host_block_write,
@@ -239,6 +258,7 @@ U_BOOT_DRIVER(sandbox_host_blk) = {
.name = "sandbox_host_blk",
.id = UCLASS_BLK,
.ops= &sandbox_host_blk_ops,
+ .unbind = sandbox_host_unbind,
.plat_auto = sizeof(struct host_block_dev),
};
#else
--
2.29.2
Regards,
Simon
Re: [PATCH] env: increment redund flag on read fail
On Thu, Dec 17, 2020 at 05:19:18PM -0600, Brandon Maier wrote: > If one of the reads fails when importing redundant environments (a > single read failure), the env_flags wouldn't get initialized in > env_import_redund(). If a user then calls saveenv, the new environment > will have the wrong flags value. So on the next load the new environment > will be ignored. > > While debugging this, I also noticed that env/sf.c was not correctly > handling a single read failure, as it would not check the crc before > assigning it to gd->env_addr. > > Having a special error path for when there is a single read failure > seems unnecessary and may lead to future bugs. Instead collapse the > 'single read failure' error to be the same as a 'single crc failure'. > That way env_check_redund() either passes or fails, and if it passes we > are guaranteed to have checked the CRC. > > Signed-off-by: Brandon Maier > CC: Joe Hershberger > CC: Wolfgang Denk > CC: Heiko Schocher Sorry for the delay. Looking this over, yes, I think this is correct so: Reviewed-by: Tom Rini And I'm going to give Joe or Wolfgang or Heiko a little more time to chime in before I apply this in time for v2021.04-rc2. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/5] arm: dts: r8a774a1: Import DTS queued for Linux 5.12-rc1
On 2/1/21 4:19 PM, Adam Ford wrote: On Mon, Jan 25, 2021 at 6:40 AM Marek Vasut wrote: On 1/25/21 12:22 AM, Adam Ford wrote: On Sun, Jan 24, 2021 at 11:10 AM Marek Vasut wrote: On 1/13/21 12:52 AM, Adam Ford wrote: Update the RZ/G2M dtsi and r8a774a1-beacon-rzg2m-kit kit from Renesas repo destined to become 5.12-rc1. I picked a DT sync for Linux 5.10 from Biju (on CC), does your board need something from 5.12-rc1 or can it be based on 5.10 too ? I honestly don't remember. I had to unexpectedly leave town for this week due to a funeral, and I won't be near a Linux development computer to run tests until Feb 1, so I don't know if the board will build using 5.10 or not. Sorry I don't have a better answer. Lets revisit this later then. I tested the Beacon boards, but they have a dependency on usb2_clksel that is not present in the 5.10 device tree. I also noticed that I forgot to include versaclock.h dt-binding file from the kernel, so the Beacon boards don't build for that too. I can submit a V2 with the binding file included, or I can submit the dt-binding file now and wait on the Beacon boards until that file has been merged, or I can redo the device tree in a custom way to work the 5.10 and not use the veraclock dt-bindings since there is no versaclock driver in U-Boot anyway. You may have an alternative as well. Can you try to figure out the best option with Biju , who is in charge of all the RZG stuff ? Either way works for me. You also likely want to roll V2 out soon, since the RC1 is out. I would still like to pick it for 2021.04 .
Re: [PATCH v4 0/6] Add support for ECDSA image signing (with test)
Hi Alex,
On Thu, 28 Jan 2021 at 10:54, Alex G. wrote:
>
> On 1/28/21 10:40 AM, Patrick DELAUNAY wrote:
> > Hi Alexandru,
> Hi Patrick
>
> > I found in doc/uImage.FIT/signature.txt the description
> >
> > - key-name-hint: Name of key to use for signing. The keys will
> > normally be in a single directory (parameter -k to mkimage).
> [snip]
>
> You are correct that the ECDSA path does not use the "key-name-hint".
> And this deviates from what RSA does. This is intentional.
>
We need to join these two up though. We can't add a new way of doing
things whenever we add a new algorithm.
>
> [snip]> so today the RSA features seens more compete based on openssl (with
> > ENGINE and pkcs11 support for exmaple)
> >
> > and keydir / keyname seens clear enought...
>
> The the common case with image signing is that one wants to sign an
> image with a key. keyname comes from the console, and keydir comes from
> the FIT image, which contradicts this simplicity.
>
> Another issue is incorporating this into a bigger build system like
> yocto. Now mkimage would impose a specific directory structure for
> signing keys. This would not be ideal.
It more likely requires a standard filename for keys...I wonder how
this works in yocto at present?
>
> > PS: I think the engine part could be shared between RSA and EDCSA part.
>
> I don't see the benefit of using the engine, and it seems highly
> libcrypto specific. It would be a lot more code, with no useful
> functionality -- we can ecdsa sign with the simpler code.
The hope was that the same API could be used...is that possible?
>
> [snip]
> > This new option -K with full path will be permanent added to mkimage
> >
> > or it is a temporarily solution (for initial ECDSA implementation).
> >
> >
> > If it is permanent it should be also supported in RSA mode ?
> >
> > => for example: -K allow to override the "key-name-hint" value
>
> Yes and no. It is temporary in that we'd like to update the RSA path to
> be consistent with the ECDSA path. It's permanent in that we want to
> have the -'k' option to specify the key filename instead of the key dir.
> At least that's my understanding given the previous discussion with Simon.
But we do need to either move RSA over (in the same patch series ) or
use a different flag. I'm leaning towards a different flag, since if
we are not changing RSA, it is just going to get confusing.
>
>
> [snip]
> > Sorry to open debate.
> > I propose to change the test with previous proposal.
> >
> [snip]
> > with test/py/tests/vboot/sign-images-sha256.its
> > fdt@1 {
> >
> > signature@1 {
> > algo = "sha1,rsa2048";
> > -key-name-hint = "dev";
> > + key-name-hint = "ecdsa-test-key.pem";
>
> This would go against us wanting to decouple the key filename from the
> key name. Consider haing several keys:
>
> dev-key-frobnozzle.pem
> prov-key-frobnozzle-r1.pem
> prov-key-frobnozzle-r2.pem
> prov-key-frobnozzle-r3-after-hack-mitigation.pem
>
> One might not want to expose those key names in the .its. The issue is,
> when the fit-image is created, the key filename must be known. But when
> the signing happens on a separate machine, the filename really isn't known.
>
> So we should really use the "key-name-hint" as a hint rather than a
> filename or part of a filename.
Yes it is just a hint. We must not rely on it. Its only purpose is to
hopefully speed up verification since fewer signatures might need to
be tried.
Regards,
Simon
Re: [PATCH v2 1/1] sandbox: mark os_abort() as noreturn
On Sun, 31 Jan 2021 at 17:24, Heinrich Schuchardt wrote: > > gcc -fanalyzer needs the information that a function does not return to > provide accurate information. > > os_abort() does not return. Mark it accordingly. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > use __attribute__((noreturn)) instead of __return > --- > include/os.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 1/1] firmware: smci: possible NULL dereference
On Sun, 31 Jan 2021 at 19:02, Heinrich Schuchardt wrote: > > sandbox_scmi_devices_ctx() may return NULL. We should not dereference this > value in sandbox_scmi_devices_remove(). > > The problem was indicated by 'gcc-11 -fanalyzer'. > > Signed-off-by: Heinrich Schuchardt > --- > drivers/firmware/scmi/sandbox-scmi_devices.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v1 1/7] command sf: help text format
On Thu, 28 Jan 2021 at 09:29, Bernhard Kirchen wrote: > > properly indent the help text and use single quotes consistently to mark > variable parameters. > > Signed-off-by: Bernhard Kirchen > --- > > cmd/sf.c | 48 ++-- > 1 file changed, 22 insertions(+), 26 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v3] sysboot: add zboot support to boot x86 Linux kernel image
Hi Kory, On Mon, 1 Feb 2021 at 08:31, Kory Maincent wrote: > > Add "zboot" command to the list of supported boot in the label_boot > function. > > Signed-off-by: Kory Maincent > --- > > Change since v1: > - Modify comment > > Change since v2: > - Update do_zboot to do_zboot_parent function to follow the patch: >5588e776b0 > > cmd/pxe_utils.c | 4 > include/command.h | 3 +++ > 2 files changed, 7 insertions(+) > > diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c > index 3526a651d7..06611262c1 100644 > --- a/cmd/pxe_utils.c > +++ b/cmd/pxe_utils.c > @@ -657,6 +657,10 @@ static int label_boot(struct cmd_tbl *cmdtp, struct > pxe_label *label) > /* Try booting a Image */ > else > do_bootz(cmdtp, 0, bootm_argc, bootm_argv); > +#elif defined(CONFIG_CMD_ZBOOT) Can this use IS_ENABLED() ? > + /* Try booting an x86_64 Linux kernel image */ > + else > + do_zboot_parent(cmdtp, 0, bootm_argc, bootm_argv, NULL); > #endif > unmap_sysmem(buf); > > diff --git a/include/command.h b/include/command.h > index e229bf2825..cb91ba81b5 100644 > --- a/include/command.h > +++ b/include/command.h > @@ -165,6 +165,9 @@ extern int do_bootz(struct cmd_tbl *cmdtp, int flag, int > argc, > extern int do_booti(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]); > > +extern int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, > + char *const argv[], int *repeatable); We don't normally use extern for function decls in header files. > + > extern int common_diskboot(struct cmd_tbl *cmdtp, const char *intf, int argc, >char *const argv[]); > > -- > 2.17.1 > Reviewed-by: Simon Glass
Re: [PATCH v1 7/7] provide "sf protect check" command
Hi Bernhard, On Thu, 28 Jan 2021 at 09:28, Bernhard Kirchen wrote: > > this change exposes checking the protection mechanism of SPI NOR flash > chips to the CLI. it expands what was already there to communicate not > only the protection state of a particular region, but also whether or > not the hardware write protection mechanism of the chip is enabled. > > the new command works for Micron (and compatible) SPI NOR flash chips as > well as the SST26* series of SPI NOR flash chips. > > the changes were tested on proprietary boards using a M25P16 and a > SST26VF016B. > > Signed-off-by: Bernhard Kirchen > --- > > cmd/sf.c | 23 +++-- > drivers/mtd/spi/spi-nor-core.c | 45 +- > include/linux/mtd/spi-nor.h| 17 - > include/spi_flash.h| 8 ++ > 4 files changed, 78 insertions(+), 15 deletions(-) > I seem to have got this twice, but the same comment re testing applies. Regards, Simon
Re: [PATCH v5 6/6] test/py: ecdsa: Add test for mkimage ECDSA signing
Hi Alexandru,
On Thu, 28 Jan 2021 at 08:52, Alexandru Gagniuc wrote:
>
> Add a test to make sure that the ECDSA signatures generated by
> mkimage can be verified successfully. pyCryptodomex was chosen as the
> crypto library because it integrates much better with python code.
> Using openssl would have been unnecessarily painful.
>
> Signed-off-by: Alexandru Gagniuc
> Reviewed-by: Simon Glass
> ---
> test/py/tests/test_fit_ecdsa.py | 111
> 1 file changed, 111 insertions(+)
> create mode 100644 test/py/tests/test_fit_ecdsa.py
>
> diff --git a/test/py/tests/test_fit_ecdsa.py b/test/py/tests/test_fit_ecdsa.py
> new file mode 100644
> index 00..6cadb0cbb5
> --- /dev/null
> +++ b/test/py/tests/test_fit_ecdsa.py
> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2020,2021 Alexandru Gagniuc
> +
> +"""
> +Test ECDSA signing of FIT images
> +
> +This test uses mkimage to sign an existing FIT image with an ECDSA key. The
> +signature is then extracted, and verified against pyCryptodome.
> +This test doesn't run the sandbox. It only checks the host tool 'mkimage'
> +"""
> +
> +import pytest
> +import u_boot_utils as util
> +from Cryptodome.Hash import SHA256
> +from Cryptodome.PublicKey import ECC
> +from Cryptodome.Signature import DSS
> +
> +class SignableFitImage(object):
> +""" Helper to manipulate a FIT image on disk """
> +def __init__(self, cons, file_name):
> +self.fit = file_name
> +self.cons = cons
> +self.signable_nodes = set()
> +
> +def __fdt_list(self, path):
> +return util.run_and_log(self.cons, f'fdtget -l {self.fit} {path}')
> +
> +def __fdt_set(self, node, **prop_value):
> +for prop, value in prop_value.items():
> +util.run_and_log(self.cons, f'fdtput -ts {self.fit} {node}
> {prop} {value}')
> +
> +def __fdt_get_binary(self, node, prop):
> +numbers = util.run_and_log(self.cons, f'fdtget -tbi {self.fit}
> {node} {prop}')
> +
> +bignum = bytearray()
> +for little_num in numbers.split():
> +bignum.append(int(little_num))
> +
> +return bignum
> +
> +def find_signable_image_nodes(self):
> +for node in self.__fdt_list('/images').split():
> +image = f'/images/{node}'
> +if 'signature' in self.__fdt_list(image):
> +self.signable_nodes.add(image)
> +
> +return self.signable_nodes
> +
> +def change_signature_algo_to_ecdsa(self):
> +for image in self.signable_nodes:
> +self.__fdt_set(f'{image}/signature', algo='sha256,ecdsa256')
> +
> +def sign(self, mkimage, key_file):
> +util.run_and_log(self.cons, [mkimage, '-F', self.fit,
> f'-k{key_file}'])
> +
> +def check_signatures(self, key):
> +for image in self.signable_nodes:
> +raw_sig = self.__fdt_get_binary(f'{image}/signature', 'value')
> +raw_bin = self.__fdt_get_binary(image, 'data')
> +
> +sha = SHA256.new(raw_bin)
> +verifier = DSS.new(key, 'fips-186-3')
> +verifier.verify(sha, bytes(raw_sig))
> +
> +
> +@pytest.mark.buildconfigspec('fit_signature')
> +@pytest.mark.requiredtool('dtc')
> +@pytest.mark.requiredtool('fdtget')
> +@pytest.mark.requiredtool('fdtput')
> +def test_fit_ecdsa(u_boot_console):
> +""" Test that signatures generated by mkimage are legible. """
> +def generate_ecdsa_key():
> +return ECC.generate(curve='prime256v1')
> +
> +def assemble_fit_image(dest_fit, its, destdir):
> +dtc_args = f'-I dts -O dtb -i {destdir}'
> +util.run_and_log(cons, [mkimage, '-D', dtc_args, '-f', its,
> dest_fit])
> +
> +def dtc(dts):
> +dtb = dts.replace('.dts', '.dtb')
> +util.run_and_log(cons, f'dtc {datadir}/{dts} -O dtb -o
> {tempdir}/{dtb}')
> +
> +cons = u_boot_console
> +mkimage = cons.config.build_dir + '/tools/mkimage'
> +datadir = cons.config.source_dir + '/test/py/tests/vboot/'
> +tempdir = cons.config.result_dir
> +key_file = f'{tempdir}/ecdsa-test-key.pem'
> +fit_file = f'{tempdir}/test.fit'
> +dtc('sandbox-kernel.dts')
> +
> +key = generate_ecdsa_key()
> +
> +# Create a fake kernel image -- zeroes will do just fine
> +with open(f'{tempdir}/test-kernel.bin', 'w') as fd:
> +fd.write(500 * chr(0))
> +
> +# invokations of mkimage expect to read the key from disk
> +with open(key_file, 'w') as f:
> +f.write(key.export_key(format='PEM'))
> +
> +assemble_fit_image(fit_file, f'{datadir}/sign-images-sha256.its',
> tempdir)
> +
> +fit = SignableFitImage(cons, fit_file)
> +nodes = fit.find_signable_image_nodes()
> +if len(nodes) == 0:
> +raise ValueError('FIT image has no "/image" nodes with "signature"')
> +
> +fit.change_signature_algo_to_ecdsa()
> +fit.sign(mkimage, key_file)
> +fit.check_signatures(key)
> --
> 2.26.2
>
As mentioned ear
Re: [PATCH v1] fix patman --limit-cc option
Hi Bernhard, On Fri, 29 Jan 2021 at 07:10, Bernhard Kirchen wrote: > > patman's --limit-cc option parses its argument to an integer and uses > that to trim the list of CC recipients to a particular maximum. but that > only works if the cc variable is a list, which it is not. > > Signed-off-by: Bernhard Kirchen > --- > > tools/patman/series.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass A test would be useful for this please as it is obviously missing...
Re: [PATCH 1/2] board: samsung: covert to driver model about power_key_pressed
On Thu, 28 Jan 2021 at 15:11, Jaehoon Chung wrote: > > Convert to driver model about power_key_pressed. > > Signed-off-by: Jaehoon Chung > --- > board/samsung/common/misc.c | 27 ++- > 1 file changed, 14 insertions(+), 13 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v1 1/7] command sf: help text format
On Thu, 28 Jan 2021 at 09:19, Bernhard Kirchen wrote: > > properly indent the help text and use single quotes consistently to mark > variable parameters. > > Signed-off-by: Bernhard Kirchen > --- > > cmd/sf.c | 48 ++-- > 1 file changed, 22 insertions(+), 26 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v1 2/7] sf protect: warn about failed (un)lock operation
On Thu, 28 Jan 2021 at 09:19, Bernhard Kirchen wrote: > > it is not guaranteed that there is a human readable message when the > lock or unlock operation failed. make sure there is a message emitted > by the "sf protect" implementation if the subcommand failed. > > Signed-off-by: Bernhard Kirchen > --- > > cmd/sf.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [ANN] U-Boot v2021.04-rc1 released
Hi Tom, On Mon, 1 Feb 2021 at 09:06, Tom Rini wrote: > > Hey all, > > It's release day, and here's v2021.04-rc1. As is often the case, > there's a few PRs still I would have hoped to see by now come in but I > think will be coming in soon. And there's a few more changes in my own > queue that I think I can / should pick up still and I'll try and get to > this week. As always, I leave what comes in at this point to the > discretion of the custodians making the PRs. > > In terms of a changelog, > git log --merges v2021.01..v2021.04-rc1 > contains what I've pulled but as always, better PR messages and tags > will provide better results here. > > I do have my reminders setup for doing -rc releases every other Monday > from here on out and final release on April 5th, 2021. Thanks all! I know this does not affect many people, but I have held off on applying the new of-platdata stuff. I now want that to go in with the test updates since it allows better tests on the SPL side...so soon. Regards, Simon
Re: [PATCH v2 10/10] bdinfo: Change to use bdinfo_print_num_ll() where the number could be 64-bit
On Sun, 31 Jan 2021 at 05:37, Bin Meng wrote: > > From: Bin Meng > > There are some calls to bdinfo_print_num_l() with parameters that > could be a 64-bit value on a 32-bit system. Change those calls to > use bdinfo_print_num_ll() instead. > > Signed-off-by: Bin Meng > --- > > (no changes since v1) > > arch/arm/lib/bdinfo.c | 8 > cmd/bdinfo.c | 4 ++-- > 2 files changed, 6 insertions(+), 6 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 3/4] .gitlab-ci: install doc/sphinx/requirements.txt
On Tue, 26 Jan 2021 at 11:36, Heinrich Schuchardt wrote: > > Install all requirements according to doc/sphinx/requirements.txt in the > virtual environment used for testing 'make htmldocs'. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > no change > --- > .azure-pipelines.yml | 6 +- > .gitlab-ci.yml | 3 +++ > 2 files changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 1/1] doc: dm: describe end of life of plat_auto
On Sun, 31 Jan 2021 at 03:07, Heinrich Schuchardt wrote: > > Describe when plat_auto is freed. > > Fix a typo. > > Signed-off-by: Heinrich Schuchardt > --- > doc/driver-model/design.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 2/4] doc: fix doc/develop/logging.rst
On Tue, 26 Jan 2021 at 11:36, Heinrich Schuchardt wrote: > > Sphinx 3 builds fail due to doc/develop/logging.rst producing duplicate > labels. > > Include logging.h only once in the API section and use cross-references for > the enums log_level_t and log_category_t. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > no change > --- > doc/api/index.rst | 1 + > doc/api/logging.rst | 6 ++ > doc/develop/logging.rst | 13 +++-- > 3 files changed, 10 insertions(+), 10 deletions(-) > create mode 100644 doc/api/logging.rst Reviewed-by: Simon Glass
Re: [PATCH v2 1/2] stdio: Introduce stdio_valid()
On Thu, 28 Jan 2021 at 06:12, Nicolas Saenz Julienne wrote: > > stdio_valid() will confirm that a struct stdio_dev pointer is indeed > valid. > > Signed-off-by: Nicolas Saenz Julienne > > --- > > Changes since v1: > - Properly document function > > common/stdio.c | 11 +++ > include/stdio_dev.h | 11 +++ > 2 files changed, 22 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 1/1] sandbox: host bind must close file descriptor
Hi Heinrich,
On Sun, 31 Jan 2021 at 03:39, Heinrich Schuchardt wrote:
>
> Each invocation of the 'host bind' command with a file name argument opens
> a file descriptor. The next invocation of the 'host bind' command destroys
> the block device but the file descriptor remains open. The same holds true
> for the 'unbind blk' command.
>
> Close the file descriptor when unbinding the host block device.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> drivers/block/sandbox.c | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
> index f57f690d3c..02992ac34f 100644
> --- a/drivers/block/sandbox.c
> +++ b/drivers/block/sandbox.c
> @@ -230,6 +230,25 @@ int host_get_dev_err(int devnum, struct blk_desc
> **blk_devp)
> }
>
> #ifdef CONFIG_BLK
> +
> +int sandbox_host_unbind(struct udevice *dev)
> +{
> + struct host_block_dev *host_dev;
> +
> + host_dev = dev_get_plat(dev);
> + if (host_dev) {
> + if (host_dev->fd) {
> + os_close(host_dev->fd);
> + host_dev->fd = 0;
> + } else {
> + log_err("missing file descriptor\n");
How does this happen?
> + }
> + } else {
> + log_err("missing platform data\n");
I don't think this case can happen. See the .plat_auto below.
> + }
> + return 0;
> +}
> +
> static const struct blk_ops sandbox_host_blk_ops = {
> .read = host_block_read,
> .write = host_block_write,
> @@ -239,6 +258,7 @@ U_BOOT_DRIVER(sandbox_host_blk) = {
> .name = "sandbox_host_blk",
> .id = UCLASS_BLK,
> .ops= &sandbox_host_blk_ops,
> + .unbind = sandbox_host_unbind,
> .plat_auto = sizeof(struct host_block_dev),
> };
> #else
> --
> 2.29.2
>
Regards,
Simon
Re: [PATCH 1/1] fs: fat: usage basename in file_fat_write_at, fat_mkdir
On Sat, 30 Jan 2021 at 02:22, Heinrich Schuchardt wrote: > > This patch involves no functional change. It is just about code > readability. > > Both in file_fat_write_at() and fat_mkdir() the incoming file or directory > path are split into two parts: the parent directory and the base name. > > In file_fat_write_at() the value of the variable basename is assigned to > the filename parameter and afterwards the variable filename is used instead > of basename. It is more readable to use the variable basename and leave > filename unchanged. > > In fat_mkdir() the base name variable is called directory. This is > confusing. Call it basename like in file_fat_write_at(). This allows to > rename parameter new_directory to directory in the implementation of > fat_mkdir() to match the function declaration. > > Signed-off-by: Heinrich Schuchardt > --- > fs/fat/fat_write.c | 27 +-- > 1 file changed, 13 insertions(+), 14 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH] azure: Add -E back for the world build script
On Sun, 31 Jan 2021 at 01:38, Bin Meng wrote:
>
> Commit dd5c954e917b ("travis/gitlab/azure: Use -W to avoid warnings check")
> added -W to avoid warnings check, but it mistakenly dropped -E for
> the world build script in the azure pipelines.
>
> This caused builds on the azure pipelines fail to report warnings. Let's
> add it back.
>
> Fixes: dd5c954e917b ("travis/gitlab/azure: Use -W to avoid warnings check")
> Signed-off-by: Bin Meng
> ---
>
> .azure-pipelines.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass
Re: [PATCH v2] test/py: fix runtest wrapper for pytest 6
On Sat, 30 Jan 2021 at 20:12, Stephen Warren wrote: > > The implementation of pytest_runtest_protocol() must call > pytest_runtest_logstart() and pytest_runtest_logfinish(). This appears to > be necessary even in pytest 5.2.1 judging by the default version of > pytest_runtest_protocol(), but evidently some form of code reorganization > in pytest only made this have a practical effect in the newer version. I'd > previously been under the impression that 100% of the required work of > pytest_runtest_protocol() was handled by the fact it called > runtestprotocol() as its implementation. However, it appears that custom > implementations do need to do a little more than this. > > Reported-by: Heinrich Schuchardt > Signed-off-by: Stephen Warren > --- > v2: Rebased on a marginally newer commit, hence removed the change to > test/py/test.py. Cleaned up return statement. > --- > test/py/conftest.py | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 08/12] pinctrl: single: add register access functions
On Thu, 28 Jan 2021 at 15:25, Dario Binacchi wrote: > > The configuration of pinmux registers was implemented with duplicate > code which can be removed by adding two functions for read/write access. > Access to 8-bit registers has also been added. > > Signed-off-by: Dario Binacchi > > --- > > Changes in v2: > - Updated commit message. > - Remove pointer to access functions. > > drivers/pinctrl/pinctrl-single.c | 71 +--- > 1 file changed, 46 insertions(+), 25 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 11/12] pinctrl: single: add get_pin_muxing operation
On Thu, 28 Jan 2021 at 15:25, Dario Binacchi wrote: > > It allows to display the muxing of a given pin. Inspired by more recent > versions of the Linux driver, in addition to the address and the value > of the configuration register I added the pin function retrieved from > the DT. In doing so, the information displayed does not depend on the > platform, being a generic type driver, and it can be useful for debug > purposes. > > Signed-off-by: Dario Binacchi > --- > > (no changes since v1) > > drivers/pinctrl/pinctrl-single.c | 222 +-- > 1 file changed, 213 insertions(+), 9 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 2/2] power: pmic: remove pmic_max8997/8 files
On Thu, 28 Jan 2021 at 15:11, Jaehoon Chung wrote: > > Remove pmic_max8997/8 files about no-DM. > There are already existed max8997/8 as driver-model. > > Signed-off-by: Jaehoon Chung > --- > drivers/power/pmic/Makefile | 2 - > drivers/power/pmic/pmic_max8997.c | 107 -- > drivers/power/pmic/pmic_max8998.c | 32 - > 3 files changed, 141 deletions(-) > delete mode 100644 drivers/power/pmic/pmic_max8997.c > delete mode 100644 drivers/power/pmic/pmic_max8998.c Reviewed-by: Simon Glass
Re: [PATCH v2 07/12] pinctrl: single: change function mask default value
On Thu, 28 Jan 2021 at 15:25, Dario Binacchi wrote: > > The patch is inspired by more recent versions of the Linux driver. > Replacing the default value 0x of the function mask with 0 is > certainly more conservative in case the "pinctrl-single,function-mask" > DT property is missing. > > Signed-off-by: Dario Binacchi > --- > > (no changes since v1) > > drivers/pinctrl/pinctrl-single.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v1 7/7] provide "sf protect check" command
Hi, On Thu, 28 Jan 2021 at 09:19, Bernhard Kirchen wrote: > > this change exposes checking the protection mechanism of SPI NOR flash > chips to the CLI. it expands what was already there to communicate not > only the protection state of a particular region, but also whether or > not the hardware write protection mechanism of the chip is enabled. > > the new command works for Micron (and compatible) SPI NOR flash chips as > well as the SST26* series of SPI NOR flash chips. > > the changes were tested on proprietary boards using a M25P16 and a > SST26VF016B. > > Signed-off-by: Bernhard Kirchen > --- > > cmd/sf.c | 23 +++-- > drivers/mtd/spi/spi-nor-core.c | 45 +- > include/linux/mtd/spi-nor.h| 17 - > include/spi_flash.h| 8 ++ > 4 files changed, 78 insertions(+), 15 deletions(-) Reviewed-by: Simon Glass Can we get a sandbox test for this in test/dm/sf.c ? Regards, Simon
Re: [PATCH] version: Move version_string[] from version.h to version_string.h
On Wed, 27 Jan 2021 at 08:57, Pali Rohár wrote: > > More C files do not use compile time timestamp macros and do not have to be > recompiled every time when SOURCE_DATE_EPOCH changes. > > This patch moves version_string[] from version.h to version_string.h and > updates other C files which only needs version_string[] string to include > version_string.h instead of version.h. After applying this patch these > files are not recompiled every time when SOURCE_DATE_EPOCH changes. > > Signed-off-by: Pali Rohár > --- > board/ge/b1x5v2/b1x5v2.c| 2 +- > board/ge/bx50v3/bx50v3.c| 2 +- > board/ge/mx53ppd/mx53ppd.c | 2 +- > cmd/version.c | 1 + > common/main.c | 2 +- > drivers/video/cfb_console.c | 3 +-- > include/version.h | 3 --- > include/version_string.h| 8 > lib/display_options.c | 2 +- > test/print_ut.c | 2 +- > 10 files changed, 16 insertions(+), 11 deletions(-) > create mode 100644 include/version_string.h > Reviewed-by: Simon Glass
Re: [PATCH] tools: Remove #include
On Wed, 27 Jan 2021 at 08:34, Pali Rohár wrote: > > Header file version.h includes also autogenerated file timestamp.h which > is recompiled on every time when SOURCE_DATE_EPOCH change. > > Tools do not use build time therefore they do not have to include > timestamp.h file. > > This change prevents recompiling tools every time when SOURCE_DATE_EPOCH > changes. > > Signed-off-by: Pali Rohár > --- > tools/dumpimage.c | 2 +- > tools/mkenvimage.c | 2 +- > tools/mkimage.c| 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH] arm: Remove #include from armv8/fwcall.c
On Wed, 27 Jan 2021 at 08:29, Pali Rohár wrote: > > No version information is used in armv8/fwcall.c therefore do not include > version.h header file. This change prevents recompiling fwcall.o when > SOURCE_DATE_EPOCH changes. > > Signed-off-by: Pali Rohár > --- > arch/arm/cpu/armv8/fwcall.c | 1 - > 1 file changed, 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 1/1] doc: update Kernel documentation build system
Hi Heinrich, On Tue, 26 Jan 2021 at 09:02, Tom Rini wrote: > > On Tue, Jan 26, 2021 at 04:28:24PM +0100, Heinrich Schuchardt wrote: > > On 25.01.21 02:56, Simon Glass wrote: > > > Hi, > > > > > > On Sun, 24 Jan 2021 at 18:37, Tom Rini wrote: > > >> > > >> On Mon, Jan 25, 2021 at 01:41:18AM +0100, Heinrich Schuchardt wrote: > > >>> On 1/25/21 12:59 AM, Tom Rini wrote: > > On Sun, Jan 24, 2021 at 10:05:27PM +0100, Heinrich Schuchardt wrote: > > > On 1/23/21 6:53 PM, Tom Rini wrote: > > >> On Sat, Jan 23, 2021 at 12:46:23PM -0500, Tom Rini wrote: > > >>> On Fri, Jan 01, 2021 at 01:21:11AM +0100, Heinrich Schuchardt wrote: > > >>> > > Update the docomentation build system according to Linux v5.11-rc1. > > > > With this patch we can build the HTML documentation using either of > > Sphinx 2 and Sphinx 3. > > > > Signed-off-by: Heinrich Schuchardt > > Reviewed-by: Simon Glass > > >>> > > >>> Applied to u-boot/master, thanks! > > >> > > >> I've had to revert this. While I caught and fixed up in a > > >> semi-logical > > >> way one duplicate label problem, there's another now that I see, and > > >> probably many more once I rework that one. It's unclear as well how > > >> best to handle these otherwise logical duplicate labels, such as > > >> "eMMC" > > >> in doc/board/microchip/mpfs_icicle.rst for example. > > > > > > Sphinx 2 is not available for current Linux distributions. Without > > > this > > > patch we cannot build with Sphinx 3. > > > > We need to be careful when saying "current". Ubuntu 18.04 is still > > quite current enough and will be until 2022 (as it doesn't go EOL until > > 2023). I'm not sure I can even get Sphinx 3. > > >>> > > >>> Developers will not be able to test the documentation if 'make htmldocs' > > >>> fails on their machines because their distribution does not provide > > >>> Sphinx 2. > > >>> > > >>> The current Ubuntu release is 20.10 and provides Sphinx 3.2. > > >>> https://packages.ubuntu.com/groovy/sphinx-common. > > >>> > > >>> Arch Linux is on Sphinx 3.4. > > >>> https://archlinux.org/packages/community/any/python-sphinx/ > > >> > > >> And 18.04 is an LTS that doesn't go EOL until April 2023. Developers > > >> will not be test their documentation if they don't have Sphinx 3 > > >> available. Either side can do as Sean notes and use venv to provide > > >> whatever, and we need to make the error in that case much clearer. I > > >> would assume that the "still works with gcc-4.9!" Linux kernel has a bit > > >> clearer of an error out in that case. If not perhaps they would take a > > >> patch :) > > >> > > >> But much more importantly: > > > All pages must be deduplicated. Instead of duplicate information > > > references have to be used. > > > > So long as it can be done in a way where documentation reads well > > still, > > yes. For example, how should we re-write the example I mentioned so > > that "eMMC" isn't duplicated? > > >> > > >> Is what really needs to be solved. Show me how the document in question > > >> gets updated to read well and not have the duplicated heading message. > > > > > > I agree we have to support Sphinx 2 for a while. So we need both! > > > > > > Minor niggle - is it possible to fix the need for the -w flag? Can the > > > Makefile check the version and pass the correct flags itself? > > > > Do you mean "-W Turn warnings into errors" which never changed its > > meaning in Sphinx since version 1.0? > > > > Yes, we want to this flag to reject any patch with broken reStructered text. > > So it is not like dtc/gcc/etc where we can do -Wno-some-specific-check, > OK. > Really what I am looking for is being able to build with version 2 or 3 without any problems, rather than requiring v3, or requiring removing -w . Is that what we will end up with? Regards, Simon
Re: [PATCH v2 4/4] doc: update Kernel documentation build system
On Tue, 26 Jan 2021 at 11:36, Heinrich Schuchardt wrote: > > Update the documentation build system according to Linux v5.11-rc1. > > Deactive the automarkup.py extension module which on Gitlab CI is > incompatible with Unicode. > > With this patch we can build the HTML documentation using either of > Sphinx 2 and Sphinx 3. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > Deactive the automarkup.py extension module > --- > doc/conf.py | 141 -- > doc/sphinx/automarkup.py | 290 +++ > doc/sphinx/cdomain.py | 93 +- > doc/sphinx/kernel_abi.py | 194 + > doc/sphinx/kernel_feat.py | 169 +++ > doc/sphinx/kerneldoc.py | 15 +- > doc/sphinx/kernellog.py | 6 +- > doc/sphinx/kfigure.py | 6 +- > doc/sphinx/load_config.py | 27 +- > doc/sphinx/maintainers_include.py | 197 + > doc/sphinx/parallel-wrapper.sh| 33 +++ > doc/sphinx/parse-headers.pl | 6 +- > doc/sphinx/requirements.txt | 5 +- > scripts/kernel-doc| 450 ++ > 14 files changed, 1476 insertions(+), 156 deletions(-) > create mode 100644 doc/sphinx/automarkup.py > create mode 100644 doc/sphinx/kernel_abi.py > create mode 100644 doc/sphinx/kernel_feat.py > create mode 100755 doc/sphinx/maintainers_include.py > create mode 100644 doc/sphinx/parallel-wrapper.sh Reviewed-by: Simon Glass
Re: [PATCH v2 1/4] doc: board: fix Microchip MPFS Icicle Kit doc
On Tue, 26 Jan 2021 at 11:36, Heinrich Schuchardt wrote:
>
> Two sibling headings (here eMMC) cannot have the same title.
>
> Warning, treated as error:
> doc/board/microchip/mpfs_icicle.rst:423:duplicate label
> board/microchip/mpfs_icicle:emmc, other instance in
> doc/board/microchip/mpfs_icicle.rst
> make[1]: *** [doc/Makefile:69: htmldocs] Error 2
>
> * Correct the heading levels.
> * Add missing empty lines after headings.
>
> Fixes: 9e550e18305f ("doc: board: Add Microchip MPFS Icicle Kit doc")
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> no change
> ---
> doc/board/microchip/mpfs_icicle.rst | 51 +++--
> 1 file changed, 33 insertions(+), 18 deletions(-)
Reviewed-by: Simon Glass
Re: [PATCH v5 4/4] configs: khadas-vim3(l): enable Function button support
On Tue, 26 Jan 2021 at 02:51, Marek Szyprowski wrote: > > Add options required to check the 'Function' button state. > > Signed-off-by: Marek Szyprowski > --- > configs/khadas-vim3_defconfig | 2 ++ > configs/khadas-vim3l_defconfig | 2 ++ > 2 files changed, 4 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver
Hi,
On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt wrote:
>
> On 26.01.21 12:25, Marek Szyprowski wrote:
> > Hi Heinrich,
> >
> > On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> >> On 1/26/21 10:50 AM, Marek Szyprowski wrote:
> >>> Add a simple Analog to Digital Converter device based button driver.
> >>> This
> >>> driver binds to the 'adc-keys' device tree node.
> >>>
> >>> Signed-off-by: Marek Szyprowski
> >>> ---
> >>> drivers/button/Kconfig | 8 ++
> >>> drivers/button/Makefile | 1 +
> >>> drivers/button/button-adc.c | 156
> >>> 3 files changed, 165 insertions(+)
> >>> create mode 100644 drivers/button/button-adc.c
> >>>
> >>> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> >>> index 6b3ec7e55d..6db3c5e93a 100644
> >>> --- a/drivers/button/Kconfig
> >>> +++ b/drivers/button/Kconfig
> >>> @@ -9,6 +9,14 @@ config BUTTON
> >>> can provide access to board-specific buttons. Use of the
> >>> device tree
> >>> for configuration is encouraged.
> >>>
> >>> +config BUTTON_ADC
> >>> +bool "Button adc"
> >>> +depends on BUTTON
> >>> +help
> >>> + Enable support for buttons which are connected to Analog to
> >>> Digital
> >>> + Converter device. The ADC driver must use driver model.
> >>> Buttons are
> >>> + configured using the device tree.
> >>> +
> >>> config BUTTON_GPIO
> >>> bool "Button gpio"
> >>> depends on BUTTON
> >>> diff --git a/drivers/button/Makefile b/drivers/button/Makefile
> >>> index fcc10ebe8d..bbd18af149 100644
> >>> --- a/drivers/button/Makefile
> >>> +++ b/drivers/button/Makefile
> >>> @@ -3,4 +3,5 @@
> >>> # Copyright (C) 2020 Philippe Reynes
> >>>
> >>> obj-$(CONFIG_BUTTON) += button-uclass.o
> >>> +obj-$(CONFIG_BUTTON_ADC) += button-adc.o
> >>> obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
> >>> diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
> >>> new file mode 100644
> >>> index 00..1901d59a0e
> >>> --- /dev/null
> >>> +++ b/drivers/button/button-adc.c
> >>> @@ -0,0 +1,156 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright (C) 2021 Samsung Electronics Co., Ltd.
> >>> + *http://www.samsung.com
> >>> + * Author: Marek Szyprowski
> >>> + */
> >>> +
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +
> >>> +/**
> >>> + * struct button_adc_priv - private data for button-adc driver.
> >>> + *
> >>> + * @adc: Analog to Digital Converter device to which button is
> >>> connected.
> >>> + * @channel: channel of the ADC device to probe the button state.
> >>> + * @min: minimal raw ADC sample value to consider button as pressed.
> >>> + * @max: maximal raw ADC sample value to consider button as pressed.
> >>> + */
> >>> +struct button_adc_priv {
> >>> +struct udevice *adc;
> >>> +int channel;
> >>> +int min;
> >>> +int max;
> >>> +};
> >>> +
> >>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> >>> +{
> >>> +struct button_adc_priv *priv = dev_get_priv(dev);
> >>> +unsigned int val;
> >>> +int ret;
> >>> +
> >>> +ret = adc_start_channel(priv->adc, priv->channel);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = adc_channel_data(priv->adc, priv->channel, &val);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +if (ret == 0)
> >>> +return (val >= priv->min && val < priv->max) ?
> >>> +BUTTON_ON : BUTTON_OFF;
> >>> +
> >>> +return ret;
> >>> +}
> >>> +
> >>> +static int button_adc_probe(struct udevice *dev)
> >>> +{
> >>> +struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> >>> +struct button_adc_priv *priv = dev_get_priv(dev);
> >>> +struct ofnode_phandle_args args;
> >>> +u32 treshold, up_treshold, t;
> >>> +unsigned int mask;
> >>> +ofnode node;
> >>> +int ret, vdd;
> >>> +
> >>> +/* Ignore the top-level button node */
> >>> +if (!uc_plat->label)
> >>> +return 0;
> >>> +
> >>> +ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> >>> + "#io-channel-cells", 0, 0, &args);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node,
> >>> &priv->adc);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = ofnode_read_u32(dev_ofnode(dev->parent),
> >>> + "keyup-threshold-microvolt", &up_treshold);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> >>> + &treshold);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +dev_for_each_subnode(node, dev->parent) {
> >>> +ret = ofnode_read_u32(dev_ofnode(dev),
> >>> + "press-threshold-microvolt", &t);
> >>> +if (ret)
> >
Re: [PATCH 10/11] pinctrl: single: add get_pin_muxing operation
On Tue, 26 Jan 2021 at 04:28, Dario Binacchi wrote: > > Hi Simon, > > > Il 24/01/2021 03:03 Simon Glass ha scritto: > > > > > > Hi Dario, > > > > On Sat, 23 Jan 2021 at 11:27, Dario Binacchi wrote: > > > > > > It allows to display the muxing of a given pin. Inspired by more recent > > > versions of the Linux driver, in addition to the address and the value > > > of the configuration register I added the pin function retrieved from > > > the DT. In doing so, the information displayed does not depend on the > > > platform, being a generic type driver, and it can be useful for debug > > > purposes. > > > > > > Signed-off-by: Dario Binacchi > > > --- > > > > > > drivers/pinctrl/pinctrl-single.c | 220 +-- > > > 1 file changed, 211 insertions(+), 9 deletions(-) > > > > Do we need an updated DT binding file? > > I would say no, the am335x DT has recently been resynced with > one of the latest versions of the Linux kernel. > Reviewed-by: Simon Glass
Re: [PATCH v5 3/4] adc: meson-saradc: add support for getting reference voltage value
On Tue, 26 Jan 2021 at 02:51, Marek Szyprowski wrote: > > Add support for getting the 'vref-supply' regulator and register it as > ADC's reference voltage regulator, so clients can translate sampled ADC > values to the voltage. > > Signed-off-by: Marek Szyprowski > --- > drivers/adc/meson-saradc.c | 21 + > 1 file changed, 21 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH v5 1/4] dt-bindings: input: adc-keys bindings documentation
On Tue, 26 Jan 2021 at 02:51, Marek Szyprowski wrote:
>
> Dump adc-keys bindings documentation from Linux kernel source tree from
> commit 698dc0cf9447 ("dt-bindings: input: adc-keys: clarify
> description").
>
> Signed-off-by: Marek Szyprowski
> ---
> doc/device-tree-bindings/input/adc-keys.txt | 67 +
> 1 file changed, 67 insertions(+)
> create mode 100644 doc/device-tree-bindings/input/adc-keys.txt
Reviewed-by: Simon Glass
Re: [PATCH 1/1] .gitlab-ci: install doc/sphinx/requirements.txt
On Mon, 25 Jan 2021 at 14:53, Heinrich Schuchardt wrote: > > Install all requirements according to doc/sphinx/requirements.txt in the > virtual environment used for testing 'make htmldocs'. > > Signed-off-by: Heinrich Schuchardt > --- > .azure-pipelines.yml | 6 +- > .gitlab-ci.yml | 3 +++ > 2 files changed, 8 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 2/2] console: Don't start/stop console if stdio device invalid
Hi Nicolas,
On Thu, 28 Jan 2021 at 06:12, Nicolas Saenz Julienne
wrote:
>
> Don't start/stop an stdio device that might have been already freed.
>
> Signed-off-by: Nicolas Saenz Julienne
> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
>
> ---
> Changes since v1:
> - Add comment stating this should be properly fixed
>
> common/console.c | 9 +
> 1 file changed, 9 insertions(+)
>
Reviewed-by: Simon Glass
Since this says it is a stopgap, when does the real fix come?
Regards,
Simon
[PATCH V2] ARM: imx: Include u-boot.img in u-boot-with-spl.imx if OF_SEPARATE=y
The u-boot-with-spl.imx is a concatenation of SPL and u-boot.uim. The u-boot.uim is u-boot.bin wrapped in uImage. In case OF_SEPARATE is enabled, the u-boot.bin does not contain control DT for U-Boot, and so u-boot.uim does not contain the DT, and so u-boot-with-spl.imx does not contain the DT, and a system where u-boot-with-spl.imx is written to offset 1024B to the start of storage no longer boots, as it is missing DT. In case OF_SEPARATE is enabled, u-boot.img contains both u-boot.bin and the necessary DTs. Therefore, use u-boot.img instead of u-boot.uim to generate u-boot-with-spl.imx when OF_SEPARATE is enabled. Tested-by: Fabio Estevam Signed-off-by: Marek Vasut Cc: Christoph Niedermaier Cc: Fabio Estevam Cc: Peng Fan Cc: Simon Glass Cc: Stefano Babic Cc: Ye Li Cc: uboot-imx --- V2: Swap the u-boot.uim <-> u-boot.img in the conditional so this would match the patch which Fabio originally tested --- arch/arm/mach-imx/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile index 1aa26a50ad8..e6b4654cd35 100644 --- a/arch/arm/mach-imx/Makefile +++ b/arch/arm/mach-imx/Makefile @@ -202,10 +202,10 @@ append = cat $(filter-out $< $(PHONY), $^) >> $@ quiet_cmd_pad_cat = CAT $@ cmd_pad_cat = $(cmd_objcopy) && $(append) || rm -f $@ -u-boot-with-spl.imx: SPL u-boot.uim FORCE +u-boot-with-spl.imx: SPL $(if $(CONFIG_OF_SEPARATE),u-boot.img,u-boot.uim) FORCE $(call if_changed,pad_cat) -u-boot-with-nand-spl.imx: spl/u-boot-nand-spl.imx u-boot.uim FORCE +u-boot-with-nand-spl.imx: spl/u-boot-nand-spl.imx $(if $(CONFIG_OF_SEPARATE),u-boot.img,u-boot.uim) FORCE $(call if_changed,pad_cat) quiet_cmd_u-boot-nand-spl_imx = GEN $@ -- 2.29.2
[scan-ad...@coverity.com: New Defects reported by Coverity Scan for Das U-Boot]
- Forwarded message from scan-ad...@coverity.com -
Date: Mon, 01 Feb 2021 16:18:03 + (UTC)
From: scan-ad...@coverity.com
To: tom.r...@gmail.com
Subject: New Defects reported by Coverity Scan for Das U-Boot
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found
with Coverity Scan.
1 new defect(s) introduced to Das U-Boot found with Coverity Scan.
3 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent
build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)
** CID 317953:(OVERRUN)
/drivers/misc/cros_ec_sandbox.c: 536 in process_cmd()
/drivers/misc/cros_ec_sandbox.c: 548 in process_cmd()
*** CID 317953:(OVERRUN)
/drivers/misc/cros_ec_sandbox.c: 536 in process_cmd()
530 const struct ec_params_vstore_write *req = req_data;
531 struct vstore_slot *slot;
532
533 if (req->slot >= EC_VSTORE_SLOT_MAX)
534 return -EINVAL;
535 slot = &ec->slot[req->slot];
>>> CID 317953:(OVERRUN)
>>> Overrunning array of 260 bytes at byte offset 2015 by dereferencing
>>> pointer "slot".
536 slot->locked = true;
537 memcpy(slot->data, req->data, EC_VSTORE_SLOT_SIZE);
538 len = 0;
539 break;
540 }
541 case EC_CMD_VSTORE_READ: {
/drivers/misc/cros_ec_sandbox.c: 548 in process_cmd()
542 const struct ec_params_vstore_read *req = req_data;
543 struct ec_response_vstore_read *resp = resp_data;
544 struct vstore_slot *slot;
545
546 if (req->slot >= EC_VSTORE_SLOT_MAX)
547 return -EINVAL;
>>> CID 317953:(OVERRUN)
>>> "&ec->slot[req->slot]" evaluates to an address that is at byte offset
>>> 2015 of an array of 260 bytes.
548 slot = &ec->slot[req->slot];
549 memcpy(resp->data, slot->data, EC_VSTORE_SLOT_SIZE);
550 len = sizeof(*resp);
551 break;
552 }
553 default:
To view the defects in Coverity Scan visit,
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3Djsgx_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTvw0i-2BZeaG2NwneHBLdclGj0dZxktyUtICgF-2Bw8qb-2FneqjEmvbgwhNvmXz70TzQRWpHGC1GPOtnJwuV-2FckrA-2BZiBdaNnl8UUpJ7kZhxZQ8SEHToTVO0UrgPu4MRukOIBHhlfE0M0ylVZGm578kgQu1oUY7oQY10WypcgJYSRFzSXsa60oObHMkzy4DPrA9sxlM-3D
To manage Coverity Scan email notifications for "tom.r...@gmail.com", click
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxWeIHzDeopm-2BEWQ6S6K-2FtUHv9ZTk8qZbuzkkz9sa-2BJFw4elYDyedRVZOC-2ButxjBZdouVmTGuWB6Aj6G7lm7t25-2Biv1B-2B9082pHzCCex2kqMs-3D-7XA_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTvw0i-2BZeaG2NwneHBLdclGj20f9M5rwX45j1npE5NazJWu81Awx8InXRPGu6jHKeg-2FiGihplqmlvrD2TJCzaX2RMUSTw1UsD73k4c-2BNmtoo4gnEa-2F9ofAHPE-2FZkYpp20hp5GosFa8Ui3NxsPSg45ev6lLxbCss-2FNUAnPCCwc-2BAHBiJS-2FlnTcurE6JsyCKtYop8-3D
- End forwarded message -
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v4 9/9] mtd: spi-nor-tiny: Add fixups for Cypress s25hl-t/s25hs-t
On 28/01/21 01:37PM, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> Fixes mode clocks for SPINOR_OP_READ_FAST_4B and volatile QE bit in tiny.
> The volatile QE bit function, spansion_quad_enable_volatile() supports
> dual/quad die package parts, by taking 'die_size' parameter that is used
> to iterate register update for all dies in the device.
I'm not so sure if this is a good idea. spi-nor-tiny should be the
minimal set of functionality to get the bootloader to the next stage.
1S-1S-1S mode is sufficient for that. Adding quad enable functions of
all the flashes will increase the size quite a bit. I know that some
flashes already have their quad enable hooks, and I don't think they
should be there either.
Of course, the maintainers have the final call, but from my side,
Nacked-by: Pratyush Yadav
Anyway, comments below in case the maintainers do plan on picking this
patch up.
> Signed-off-by: Takahiro Kuwano
> ---
> drivers/mtd/spi/spi-nor-tiny.c | 89 ++
> 1 file changed, 89 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c
> index 5cc2b7d996..66680df5a9 100644
> --- a/drivers/mtd/spi/spi-nor-tiny.c
> +++ b/drivers/mtd/spi/spi-nor-tiny.c
> @@ -555,6 +555,85 @@ static int spansion_read_cr_quad_enable(struct spi_nor
> *nor)
> }
> #endif /* CONFIG_SPI_FLASH_SPANSION */
>
> +#ifdef CONFIG_SPI_FLASH_SPANSION
> +/**
> + * spansion_quad_enable_volatile() - enable Quad I/O mode in volatile
> register.
> + * @nor: pointer to a 'struct spi_nor'
> + * @die_size:maximum number of bytes per die ('mtd.size' >
> 'die_size' in
> + * multi die package parts).
> + * @dummy: number of dummy cycles for register read
> + *
> + * It is recommended to update volatile registers in the field application
> due
> + * to a risk of the non-volatile registers corruption by power interrupt.
> This
> + * function sets Quad Enable bit in CFR1 volatile.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 die_size,
> + u8 dummy)
> +{
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(0, 1),
> +SPI_MEM_OP_ADDR(4, 0, 1),
> +SPI_MEM_OP_DUMMY(0, 1),
> +SPI_MEM_OP_DATA_IN(1, NULL, 1));
> + u32 addr;
> + u8 cr;
> + int ret;
> +
> + /* Use 4-byte address for RDAR/WRAR */
> + ret = spi_nor_write_reg(nor, SPINOR_OP_EN4B, NULL, 0);
> + if (ret < 0) {
> + dev_dbg(nor->dev,
> + "error while enabling 4-byte address\n");
> + return ret;
> + }
> +
> + for (addr = 0; addr < nor->mtd.size; addr += die_size) {
> + op.addr.val = addr + SPINOR_REG_ADDR_CFR1V;
So here you add the register offset to the base address, instead of
ORing it. Ok.
> +
> + /* Check current Quad Enable bit value. */
> + op.cmd.opcode = SPINOR_OP_RDAR;
> + op.dummy.nbytes = dummy / 8;
> + op.data.dir = SPI_MEM_DATA_IN;
> + ret = spi_nor_read_write_reg(nor, &op, &cr);
> + if (ret < 0) {
> + dev_dbg(nor->dev,
> + "error while reading configuration register\n");
> + return -EINVAL;
> + }
> +
> + if (cr & CR_QUAD_EN_SPAN)
> + return 0;
> +
> + /* Write new value. */
> + cr |= CR_QUAD_EN_SPAN;
> + op.cmd.opcode = SPINOR_OP_WRAR;
> + op.dummy.nbytes = 0;
> + op.data.dir = SPI_MEM_DATA_OUT;
> + write_enable(nor);
> + ret = spi_nor_read_write_reg(nor, &op, &cr);
> + if (ret < 0) {
> + dev_dbg(nor->dev,
> + "error while writing configuration register\n");
> + return -EINVAL;
> + }
> +
> + /* Read back and check it. */
> + op.data.dir = SPI_MEM_DATA_IN;
> + ret = spi_nor_read_write_reg(nor, &op, &cr);
> + if (ret || !(cr & CR_QUAD_EN_SPAN)) {
> + dev_dbg(nor->dev, "Spansion Quad bit not set\n");
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +#endif
> +
LGTM.
> static void
> spi_nor_set_read_settings(struct spi_nor_read_command *read,
> u8 num_mode_clocks,
> @@ -583,6 +662,11 @@ static int spi_nor_init_params(struct spi_nor *nor,
> spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_FAST],
> 0, 8, SPINOR_OP_READ_FAST,
> SNOR_PROTO_1_1_1);
> +#ifdef CONFIG_SPI_FLASH_SPANSION
> + if (JEDEC_MFR(info) == SNOR_MFR_CYPRESS &&
> +
Re: [PATCH] env: Fix warning when forcing environment without ENV_ACCESS_IGNORE_FORCE
On Fri, Jan 29, 2021 at 12:03:52AM +0100, Marek Vasut wrote:
> On 1/28/21 8:26 PM, Tom Rini wrote:
> > On Thu, Jan 28, 2021 at 08:07:54PM +0100, Marek Vasut wrote:
> > > On 1/11/21 11:27 AM, Martin Fuzzey wrote:
> > > > Since commit 0f036bf4b87e ("env: Warn on force access if
> > > > ENV_ACCESS_IGNORE_FORCE set")
> > > > a warning message is displayed when setenv -f is used WITHOUT
> > > > CONFIG_ENV_ACCESS_IGNORE_FORCE, but the variable is set anyway,
> > > > resulting
> > > > in lots of log pollution.
> > > >
> > > > env_flags_validate() returns 0 if the access is accepted, or non zero
> > > > if it is refused.
> > > >
> > > > So the original code
> > > > #ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
> > > > if (flag & H_FORCE)
> > > > return 0;
> > > > #endif
> > > >
> > > > was correct, it returns 0 (accepts the modification) if forced UNLESS
> > > > IGNORE_FORCE is set (in which case access checks in the following code
> > > > are applied). The broken patch just added a printf to the force accepted
> > > > case.
> > > >
> > > > To obtain the intent of the patch we need this:
> > > > if (flag & H_FORCE) {
> > > > #ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
> > > > printf("## Error: Can't force access to \"%s\"\n",
> > > > name);
> > > > #else
> > > > return 0;
> > > > #endif
> > > > }
> > > >
> > > > Fixes: 0f036bf4b87e ("env: Warn on force access if
> > > > ENV_ACCESS_IGNORE_FORCE set")
> > > >
> > > > Signed-off-by: Martin Fuzzey
> > > > ---
> > > >env/flags.c | 5 +++--
> > > >1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/env/flags.c b/env/flags.c
> > > > index df4aed2..e3e833c 100644
> > > > --- a/env/flags.c
> > > > +++ b/env/flags.c
> > > > @@ -563,12 +563,13 @@ int env_flags_validate(const struct env_entry
> > > > *item, const char *newval,
> > > > return 1;
> > > >#endif
> > > > -#ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
> > > > if (flag & H_FORCE) {
> > > > +#ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
> > > > printf("## Error: Can't force access to \"%s\"\n",
> > > > name);
> > > > +#else
> > > > return 0;
> > > > - }
> > > >#endif
> > >
> > > Based on env/Kconfig description of this option:
> > >
> > > config ENV_ACCESS_IGNORE_FORCE
> > > bool "Block forced environment operations"
> > > default n
> > > help
> > >If defined, don't allow the -f switch to env set override
> > > variable
> > >access flags.
> > >
> > > I would think the code should look like this:
> > >
> > > #ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
> > > if (flag & H_FORCE) {
> > > printf("## Error: Can't force access to \"%s\"\n", name);
> > > return 1;
> > > }
> > > #else
> > > if (flag & H_FORCE)
> > > return 0;
> > > #endif
> >
> > So, prior to 0f036bf4b87e we had what you're suggesting, and that lead
> > to 8a5cdf601f8d (which is the commit I was trying to think of) which
> > Heinrich did not like, but was what was needed to get things to function
> > again. Wouldn't what you're proposing break the use case you had in the
> > first place?
>
> No, the idea is to completely block the -f flag if
> CONFIG_ENV_ACCESS_IGNORE_FORCE is set from setting anything in the
> environment. That's how I understand the Kconfig entry help text.
So was this all a "by inspection" bug then and not something you ran in
to in use? I'm a bit worried that "how it's implemented" is relied upon
more than "how it's documented in the help", esp since the former is
probably older than the latter.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 2/2] console: Don't start/stop console if stdio device invalid
On Fri, Jan 29, 2021 at 09:50:50AM +0100, Matthias Brugger wrote:
>
>
> On 28/01/2021 16:52, Andy Shevchenko wrote:
> > On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
> >> Don't start/stop an stdio device that might have been already freed.
> >>
> >> Signed-off-by: Nicolas Saenz Julienne
> >> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
> >
> > ...
> >
> >> + /*
> >> + * TODO: This is a workaround to avoid accessing freed memory:
> >> + * console_stop() might be called on an stdio_dev that has already been
> >> + * de-registered, due to the fact that stdio_deregister_dev()
> >> + * doesn't update the global console_devices array.
> >> + */
> >
> > I see now. I think I have experienced this issue from time to time. I will
> > look
> > at it. Tom, Simon, please hold on applying these for a while.
> >
>
> Just for the notes, the failing tests hold back Nicolas series to support
> RPi400/CM4 [1] as it does not run the new tests added successfully. If it
> takes
> a long time to fix the test environment, I'd vote to take this series as a
> stop-gap so that we can support that HW in the next release.
Andy, since you're working on a better solution, do you want more time
for that or should I pick this series up for now and you can revert it
as part of your better fix? Thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v4 8/9] mtd: spi-nor-core: Add fixups for Cypress s25hl-t/s25hs-t
On 28/01/21 01:37PM, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> Add nor->setup() and fixup hooks to overwrite:
> - volatile QE bit
> - the ->ready() hook for dual/quad die package parts
> - overlaid erase
> - spi_nor_flash_parameter
> - mtd_info
>
> Signed-off-by: Takahiro Kuwano
> ---
> drivers/mtd/spi/spi-nor-core.c | 108 +
> 1 file changed, 108 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ef49328a28..3d8cb9c333 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -2648,8 +2648,116 @@ static int spi_nor_init(struct spi_nor *nor)
> return 0;
> }
>
> +#ifdef CONFIG_SPI_FLASH_SPANSION
> +static int s25hx_t_mdp_ready(struct spi_nor *nor)
> +{
> + u32 addr;
> + int ret;
> +
> + for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) {
> + ret = spansion_sr_ready(nor, addr, 0);
> + if (ret != 1)
> + return ret;
> + }
> +
> + return 1;
> +}
> +
> +static int s25hx_t_quad_enable(struct spi_nor *nor)
> +{
> + u32 addr;
> + int ret;
> +
> + for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) {
> + ret = spansion_quad_enable_volatile(nor, addr, 0);
So you need to set the QE bit on each die. Ok.
Out of curiosity, what will happen if you only set the QE bit on the
first die? Will reads from first die work in quad mode and rest in
single mode?
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int s25hx_t_setup(struct spi_nor *nor, const struct flash_info *info,
> + const struct spi_nor_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps)
> +{
> +#ifdef CONFIG_SPI_FLASH_BAR
> + return -ENOTSUPP; /* Bank Address Register is not supported */
> +#endif
> + /*
> + * The Cypress Semper family has transparent ECC. To preserve
> + * ECC enabled, multi-pass programming within the same 16-byte
> + * ECC data unit needs to be avoided. Set writesize to the page
> + * size and remove the MTD_BIT_WRITEABLE flag in mtd_info to
> + * prevent multi-pass programming.
> + */
> + nor->mtd.writesize = params->page_size;
The writesize should be set to 16. See [0][1][2].
> + nor->mtd.flags &= ~MTD_BIT_WRITEABLE;
Not needed. See discussions pointed to above.
> +
> + /* Emulate uniform sector architecure by this erase hook*/
> + nor->mtd._erase = spansion_overlaid_erase;
> +
> + /* For 2Gb (dual die) and 4Gb (quad die) parts */
> + if (nor->mtd.size > SZ_128M)
> + nor->ready = s25hx_t_mdp_ready;
> +
> + /* Enter 4-byte addressing mode for WRAR used in quad_enable */
> + set_4byte(nor, info, true);
> +
> + return spi_nor_default_setup(nor, info, params, hwcaps);
> +}
> +
> +static void s25hx_t_default_init(struct spi_nor *nor)
> +{
> + nor->setup = s25hx_t_setup;
> +}
> +
> +static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
> +const struct sfdp_parameter_header *header,
> +const struct sfdp_bfpt *bfpt,
> +struct spi_nor_flash_parameter *params)
> +{
> + /* Default page size is 256-byte, but BFPT reports 512-byte */
> + params->page_size = 256;
Read the page size from the register, like it is done on Linux for S28
flash family.
> + /* Reset erase size in case it is set to 4K from BFPT */
> + nor->mtd.erasesize = 0;
What does erasesize of 0 mean? I would take that to mean that the flash
does not support erases. I can't find any mention of 0 erase size in the
documentation of struct mtd_info.
> +
> + return 0;
> +}
> +
> +static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor,
> + struct spi_nor_flash_parameter *params)
> +{
> + /* READ_FAST_4B (0Ch) requires mode cycles*/
> + params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8;
> + /* PP_1_1_4 is not supported */
> + params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
> + /* Use volatile register to enable quad */
> + params->quad_enable = s25hx_t_quad_enable;
> +}
> +
> +static struct spi_nor_fixups s25hx_t_fixups = {
> + .default_init = s25hx_t_default_init,
> + .post_bfpt = s25hx_t_post_bfpt_fixup,
> + .post_sfdp = s25hx_t_post_sfdp_fixup,
Hmm, I don't think the fixups feature was ever applied to the u-boot
tree. I sent a patch for them a while ago [3] but they were never
applied due to some issues. I can't find any mentions of
"spi_nor_set_fixups" on my 4 day old checkout of Tom's master branch.
And that reminds me, the nor->setup() hook you are using is not there
either. Your patch series should not even build on upstream u-boot.
Please cherry pick the required patches from my series and send them
along with yours.
Please make su
Re: [PATCH v4 6/9] mtd: spi-nor-core: Add overlaid sector erase feature
Hi Takahiro,
On 28/01/21 01:36PM, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> Some of Spansion/Cypress chips have overlaid 4KB sectors at top and/or
> bottom, depending on the device configuration, while U-Boot supports
> uniform sector layout only. This patch adds an erase hook that emulates
> uniform sector layout.
>
> Signed-off-by: Takahiro Kuwano
> ---
> drivers/mtd/spi/spi-nor-core.c | 48 ++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 1c0ba5abf9..70da0081b6 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -788,6 +788,54 @@ erase_err:
> return ret;
> }
>
> +#ifdef CONFIG_SPI_FLASH_SPANSION
> +/*
> + * Erase for Spansion/Cypress Flash devices that has overlaid 4KB sectors at
> + * the top and/or bottom.
> + */
> +static int spansion_overlaid_erase(struct mtd_info *mtd,
> +struct erase_info *instr)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + struct erase_info instr_4k;
> + u8 opcode;
> + u32 erasesize;
> + int ret;
> +
> + /* Perform default erase operation (non-overlaid portion is erased) */
> + ret = spi_nor_erase(mtd, instr);
> + if (ret)
> + return ret;
> +
> + /* Backup default erase opcode and size */
> + opcode = nor->erase_opcode;
> + erasesize = mtd->erasesize;
> +
> + /*
> + * Erase 4KB sectors. Use the possible max length of 4KB sector region.
> + * The Flash just ignores the command if the address is not configured
> + * as 4KB sector and reports ready status immediately.
> + */
> + instr_4k.len = SZ_128K;
> + nor->erase_opcode = SPINOR_OP_BE_4K_4B;
> + mtd->erasesize = SZ_4K;
> + if (instr->addr == 0) {
> + instr_4k.addr = 0;
> + ret = spi_nor_erase(mtd, &instr_4k);
> + }
> + if (!ret && instr->addr + instr->len == mtd->size) {
> + instr_4k.addr = mtd->size - instr_4k.len;
> + ret = spi_nor_erase(mtd, &instr_4k);
> + }
This feels like a hack to me. Does the flash datasheet explicitly say
that erasing the overlaid area with the "normal" erase opcode is a
no-op?
I don't see a big reason to run this hack. You are already in a
flash-specific erase hook. Why not just directly issue the correct erase
commands to the sectors? That is, why not issue 4k erase to overlaid
sectors and normal erase to the rest? Why do you need to emulate uniform
erase?
> +
> + /* Restore erase opcode and size */
> + nor->erase_opcode = opcode;
> + mtd->erasesize = erasesize;
> +
> + return ret;
> +}
> +#endif
> +
> #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST)
> /* Write status register and ensure bits in mask match written values */
> static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)
> --
> 2.25.1
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
Re: [PATCH 10/14] spi: dw: Add DUAL/QUAD/OCTAL caps
On 1/31/21 7:34 PM, Sean Anderson wrote:
These capabilities correspond to SSIC_SPI_MODE of 1, 2, or 3, respectively.
This doesn't do much yet, but it does add support for detection and for
disallowing unsupported modes. Unfortunately, we cannot discriminate
between these modes (only that SSIC_SPI_MODE != 0), so we only assume DUAL
if something sticks to SPI_FRF.
Signed-off-by: Sean Anderson
---
drivers/spi/designware_spi.c | 41 +---
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 683394a5a4..1dd83e6ca6 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -129,6 +129,9 @@ struct dw_spi_priv {
#define DW_SPI_CAP_KEEMBAY_MSTBIT(1) /* Unimplemented */
#define DW_SPI_CAP_DWC_SSIBIT(2)
#define DW_SPI_CAP_DFS32 BIT(3)
+#define DW_SPI_CAP_DUALBIT(4)
+#define DW_SPI_CAP_QUADBIT(5)
+#define DW_SPI_CAP_OCTAL BIT(5)
Looks like these CAPs both use BIT(5). Will fix in next revision.
Actually, it might be better to just have a CAP_EXTENDED and let the SPI
slave determine the bus width...
--Sean
unsigned long caps;
unsigned long bus_clk_rate;
unsigned int freq; /* Default frequency */
@@ -141,6 +144,7 @@ struct dw_spi_priv {
u8 cs; /* chip select pin */
u8 tmode; /* TR/TO/RO/EEPROM */
u8 type;/* SPI/SSP/MicroWire */
+ u8 spi_frf; /* BYTE/DUAL/QUAD/OCTAL */
};
static inline u32 dw_read(struct dw_spi_priv *priv, u32 offset)
@@ -267,7 +271,6 @@ static void spi_hw_init(struct udevice *bus, struct
dw_spi_priv *priv)
priv->fifo_len = (fifo == 1) ? 0 : fifo;
dw_write(priv, DW_SPI_TXFTLR, 0);
}
- dev_dbg(bus, "fifo_len=%d\n", priv->fifo_len);
}
/*
@@ -351,22 +354,21 @@ static int dw_spi_probe(struct udevice *bus)
if (ret)
return ret;
- priv->caps = dev_get_driver_data(bus);
- dw_spi_detect_caps(bus, priv);
-
- version = dw_read(priv, DW_SPI_VERSION);
- dev_dbg(bus, "ssi_version_id=%c.%c%c%c ssi_max_xfer_size=%u\n",
- version >> 24, version >> 16, version >> 8, version,
- priv->caps & DW_SPI_CAP_DFS32 ? 32 : 16);
-
/* Currently only bits_per_word == 8 supported */
priv->bits_per_word = 8;
priv->tmode = 0; /* Tx & Rx */
/* Basic HW init */
+ priv->caps = dev_get_driver_data(bus);
spi_hw_init(bus, priv);
+ version = dw_read(priv, DW_SPI_VERSION);
+ dev_dbg(bus,
+ "ssi_version_id=%c.%c%c%c ssi_rx_fifo_depth=%u
ssi_max_xfer_size=%u\n",
+ version >> 24, version >> 16, version >> 8, version,
+ priv->fifo_len, priv->caps & DW_SPI_CAP_DFS32 ? 32 : 16);
+
return 0;
}
@@ -748,13 +750,25 @@ static int dw_spi_set_mode(struct udevice *bus, uint mode)
{
struct dw_spi_priv *priv = dev_get_priv(bus);
+ if (!(priv->caps & DW_SPI_CAP_DUAL) &&
+ (mode & (SPI_RX_DUAL | SPI_TX_DUAL)))
+ return -EINVAL;
+
+ if (!(priv->caps & DW_SPI_CAP_QUAD) &&
+ (mode & (SPI_RX_QUAD | SPI_TX_QUAD)))
+ return -EINVAL;
+
+ if (!(priv->caps & DW_SPI_CAP_OCTAL) &&
+ (mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
+ return -EINVAL;
+
/*
* Can't set mode yet. Since this depends on if rx, tx, or
* rx & tx is requested. So we have to defer this to the
* real transfer function.
*/
priv->mode = mode;
- dev_dbg(bus, "mode=%d\n", priv->mode);
+ dev_dbg(bus, "mode=%x\n", mode);
return 0;
}
@@ -813,10 +827,13 @@ static const struct udevice_id dw_spi_ids[] = {
*/
{ .compatible = "altr,socfpga-spi" },
{ .compatible = "altr,socfpga-arria10-spi" },
- { .compatible = "canaan,kendryte-k210-spi" },
+ {
+ .compatible = "canaan,kendryte-k210-spi",
+ .data = DW_SPI_CAP_QUAD | DW_SPI_CAP_OCTAL,
+ },
{
.compatible = "canaan,kendryte-k210-ssi",
- .data = DW_SPI_CAP_DWC_SSI,
+ .data = DW_SPI_CAP_DWC_SSI | DW_SPI_CAP_QUAD,
},
{ .compatible = "intel,stratix10-spi" },
{ .compatible = "intel,agilex-spi" },
Re: [PATCH v2 1/8] spl: Drop duplicate 'Jumping to U-Boot' message
On 23/01/21 10:36AM, Simon Glass wrote:
> This is printed twice but we only need one message, since there is very
> little processing in between them. Drop the first one.
>
> Signed-off-by: Simon Glass
> ---
>
> (no changes since v1)
>
> common/spl/spl.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 12b00e2a407..7fe0812799f 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -693,7 +693,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> #endif
> switch (spl_image.os) {
> case IH_OS_U_BOOT:
> - debug("Jumping to U-Boot\n");
IMO it would make more sense to drop the "jumping to u-boot" part from
the "loaded - jumping to u-boot" message below. This way you avoid
duplication in case of ATF, OPTEE, OpenSBI, etc. This would also avoid
saying "jumping to u-boot" right after saying "jumping to linux".
> break;
> #if CONFIG_IS_ENABLED(ATF)
> case IH_OS_ARM_TRUSTED_FIRMWARE:
> --
> 2.30.0.280.ga3ce27912f-goog
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
Re: MX6sabresd broken since v2019.04
On Mon, Feb 1, 2021 at 1:36 PM Marek Vasut wrote: > I don't think WB uses OF_SEPARATE with fitImages, so maybe there its not > applicable at all ? Wandboard does use OF_SEPARATE and FIT. I have just tested flashing u-boot-with-spl.imx into a SD card and booting a wandboard. It does behave the same as mx6sabresd: - Fails to boot without your patch - Boots fine with your patch So, yes, wandboard can be used to test u-boot-with-spl.imx and avoid future regressions in Heiko's boot farm.
[PATCH v2] sunxi: spl: Fix H616 clock initialization
It turns out that there is a magic bit in PRCM region which seemingly
makes PLLs work if it's enabled. Sadly, there is no documentation what
it does exactly, so we'll just mimick BSP boot0 behaviour and enable it
before any clock is set up.
Fixes: b18bd53d6cde ("sunxi: introduce support for H616 clocks")
Signed-off-by: Jernej Skrabec
---
Changes from v1:
- use if (IS_ENABLED()) instead of #ifdef #endif
arch/arm/mach-sunxi/clock_sun50i_h6.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/mach-sunxi/clock_sun50i_h6.c
b/arch/arm/mach-sunxi/clock_sun50i_h6.c
index 06d84eb158d7..492fc4a3fca8 100644
--- a/arch/arm/mach-sunxi/clock_sun50i_h6.c
+++ b/arch/arm/mach-sunxi/clock_sun50i_h6.c
@@ -9,6 +9,11 @@ void clock_init_safe(void)
{
struct sunxi_ccm_reg *const ccm =
(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+ /* this seems to enable PLLs on H616 */
+ if (IS_ENABLED(CONFIG_MACH_SUN50I_H616))
+ setbits_le32(SUNXI_PRCM_BASE + 0x250, 0x10);
+
clock_set_pll1(40800);
writel(CCM_PLL6_DEFAULT, &ccm->pll6_cfg);
--
2.30.0
Re: [PATCH] MAINTAINERS: Add maintainer to network subsystem
On Fri, Jan 29, 2021 at 10:18 AM Ramon Fried wrote: > > Add myself as co maintainer to network subsystem > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6d8c467410..dfc8cb7a45 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -893,6 +893,7 @@ F: arch/nds32/ > > NETWORK > M: Joe Hershberger > +M: Ramon Fried > S: Maintained > T: git https://gitlab.denx.de/u-boot/custodians/u-boot-net.git > F: drivers/net/ > -- > 2.29.2 > Acked-by: Joe Hershberger
Re: [PATCH v2 26/40] test: Use a local variable for test state
Hi Simon,
On 1/30/21 9:32 PM, Simon Glass wrote:
[snip]
+static struct unit_test_state *cur_test_state;
+
+struct unit_test_state *test_get_state(void)
+{
+ return cur_test_state;
+}
+
+void test_set_state(struct unit_test_state *uts)
+{
+ cur_test_state = uts;
+}
+
/**
* dm_test_pre_run() - Get ready to run a driver model test
*
@@ -180,6 +192,9 @@ static int ut_run_test(struct unit_test_state *uts, struct
unit_test *test,
note = " (flat tree)";
printf("Test: %s: %s%s\n", test_name, fname, note);
+ /* Allow access to test state from drivers */
+ cur_test_state = uts;
+
Is there a reason for setting 'cur_test_state' directly instead of
through test_set_state() ?
ret = test_pre_run(uts, test);
if (ret == -EAGAIN)
return -EAGAIN;
@@ -192,6 +207,8 @@ static int ut_run_test(struct unit_test_state *uts, struct
unit_test *test,
if (ret)
return ret;
+ cur_test_state = NULL;
+
ditto
return 0;
}
Re: [PATCH v2 1/9] mmc: sandbox: Add support for writing
On 2/1/21 2:59 AM, Lukasz Majewski wrote:
Hi Sean,
This adds support writing to the sandbox mmc backed by an in-memory
buffer. The unit test has been updated to test reading, writing, and
erasing. I'm not sure what MMCs erase to; I picked 0, but if it's 0xFF
then that can be easily changed.
Could you rebase this patch series on top of:
https://gitlab.denx.de/u-boot/custodians/u-boot-dfu/-/commits/master
I've encountered some conflicts with previous patches.
(And of course sorry for trouble - this patch series is the last one
with the pile I've got for USB/DFU).
Thanks in advance.
Ok, I've resent after rebasing. However, I would appreciate if you could
hold off on merging Patrick's first two patches. I think this series
better solves his problem.
--Sean
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
drivers/mmc/sandbox_mmc.c | 41
++- test/dm/mmc.c |
19 +- 2 files changed, 50 insertions(+), 10
deletions(-)
diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c
index e86ea8fe09..3daf708451 100644
--- a/drivers/mmc/sandbox_mmc.c
+++ b/drivers/mmc/sandbox_mmc.c
@@ -17,6 +17,17 @@ struct sandbox_mmc_plat {
struct mmc mmc;
};
+#define MMC_CSIZE 0
+#define MMC_CMULT 8 /* 8 because the card is high-capacity */
+#define MMC_BL_LEN_SHIFT 10
+#define MMC_BL_LEN BIT(MMC_BL_LEN_SHIFT)
+#define MMC_CAPACITY (((MMC_CSIZE + 1) << (MMC_CMULT + 2)) \
+ * MMC_BL_LEN) /* 1 MiB */
+
+struct sandbox_mmc_priv {
+ u8 buf[MMC_CAPACITY];
+};
+
/**
* sandbox_mmc_send_cmd() - Emulate SD commands
*
@@ -26,6 +37,10 @@ struct sandbox_mmc_plat {
static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd
*cmd, struct mmc_data *data)
{
+ struct sandbox_mmc_priv *priv = dev_get_priv(dev);
+ struct mmc *mmc = mmc_get_mmc_dev(dev);
+ static ulong erase_start, erase_end;
+
switch (cmd->cmdidx) {
case MMC_CMD_ALL_SEND_CID:
memset(cmd->response, '\0', sizeof(cmd->response));
@@ -44,8 +59,9 @@ static int sandbox_mmc_send_cmd(struct udevice
*dev, struct mmc_cmd *cmd, break;
case MMC_CMD_SEND_CSD:
cmd->response[0] = 0;
- cmd->response[1] = 10 << 16; /* 1 <<
block_len */
- cmd->response[2] = 0;
+ cmd->response[1] = (MMC_BL_LEN_SHIFT << 16) |
+ ((MMC_CSIZE >> 16) & 0x3f);
+ cmd->response[2] = (MMC_CSIZE & 0x) << 16;
cmd->response[3] = 0;
break;
case SD_CMD_SWITCH_FUNC: {
@@ -59,13 +75,27 @@ static int sandbox_mmc_send_cmd(struct udevice
*dev, struct mmc_cmd *cmd, break;
}
case MMC_CMD_READ_SINGLE_BLOCK:
- memset(data->dest, '\0', data->blocksize);
- break;
case MMC_CMD_READ_MULTIPLE_BLOCK:
- strcpy(data->dest, "this is a test");
+ memcpy(data->dest, &priv->buf[cmd->cmdarg *
data->blocksize],
+ data->blocks * data->blocksize);
+ break;
+ case MMC_CMD_WRITE_SINGLE_BLOCK:
+ case MMC_CMD_WRITE_MULTIPLE_BLOCK:
+ memcpy(&priv->buf[cmd->cmdarg * data->blocksize],
data->src,
+ data->blocks * data->blocksize);
break;
case MMC_CMD_STOP_TRANSMISSION:
break;
+ case SD_CMD_ERASE_WR_BLK_START:
+ erase_start = cmd->cmdarg;
+ break;
+ case SD_CMD_ERASE_WR_BLK_END:
+ erase_end = cmd->cmdarg;
+ break;
+ case MMC_CMD_ERASE:
+ memset(&priv->buf[erase_start * mmc->write_bl_len],
'\0',
+ (erase_end - erase_start + 1) *
mmc->write_bl_len);
+ break;
case SD_CMD_APP_SEND_OP_COND:
cmd->response[0] = OCR_BUSY | OCR_HCS;
cmd->response[1] = 0;
@@ -148,5 +178,6 @@ U_BOOT_DRIVER(mmc_sandbox) = {
.bind = sandbox_mmc_bind,
.unbind = sandbox_mmc_unbind,
.probe = sandbox_mmc_probe,
+ .priv_auto_alloc_size = sizeof(struct sandbox_mmc_priv),
.platdata_auto_alloc_size = sizeof(struct sandbox_mmc_plat),
};
diff --git a/test/dm/mmc.c b/test/dm/mmc.c
index 4e5136c850..f744452ff2 100644
--- a/test/dm/mmc.c
+++ b/test/dm/mmc.c
@@ -29,16 +29,25 @@ static int dm_test_mmc_blk(struct unit_test_state
*uts) {
struct udevice *dev;
struct blk_desc *dev_desc;
- char cmp[1024];
+ int i;
+ char write[1024], read[1024];
ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev));
ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc));
- /* Read a few blocks and look for the string we expect */
+ /* Write a few blocks and verify that we get the same data
back */ ut_asserteq(512, dev_desc->blksz);
- memset(cmp, '\0', sizeof(cmp))
[PATCH v3 9/9] fastboot: Partition specification
This documents the way U-Boot understands partitions specifications. This also updates the fastboot documentation for the changes in the previous commit. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- Changes in v3: - Rebase onto dfu/master Changes in v2: - Move partition documentation under doc/usage doc/android/fastboot.rst | 4 doc/usage/index.rst | 1 + doc/usage/part.rst | 33 + 3 files changed, 38 insertions(+) create mode 100644 doc/usage/part.rst diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst index 16b11399b3..ce513a2a0f 100644 --- a/doc/android/fastboot.rst +++ b/doc/android/fastboot.rst @@ -154,6 +154,10 @@ The device index starts from ``a`` and refers to the interface (e.g. USB controller, SD/MMC controller) or disk index. The partition index starts from ``1`` and describes the partition number on the particular device. +Alternatively, partition types may be specified using :ref:`U-Boot's partition +syntax `. This allows specifying partitions like ``0.1``, +``0#boot``, or ``:3``. The interface is always ``mmc``. + Writing Partition Table --- diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 83cfbafd90..9b64434cb2 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -6,6 +6,7 @@ Use U-Boot fdt_overlays netconsole + part Shell commands -- diff --git a/doc/usage/part.rst b/doc/usage/part.rst new file mode 100644 index 00..e58b529147 --- /dev/null +++ b/doc/usage/part.rst @@ -0,0 +1,33 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. _partitions: + +Partitions +== + +Many U-Boot commands allow specifying partitions like:: + +some_command + +or like:: + +some_command
[PATCH v3 8/9] fastboot: Allow u-boot-style partitions
This adds support for partitions of the form "dev.hwpart:part" and
"dev#partname". This allows one to flash to eMMC boot partitions without
having to use CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT. It also allows one to
flash to an entire device without needing CONFIG_FASTBOOT_MMC_USER_NAME.
Lastly, one can also flash MMC devices other than
CONFIG_FASTBOOT_FLASH_MMC_DEV.
Because devices can be specified explicitly, CONFIG_FASTBOOT_FLASH_MMC_DEV
is used only when necessary for existing functionality. For those cases,
fastboot_mmc_get_dev has been added as a helper function. This allows
There should be no conflicts with the existing system, but just in case, I
have ordered detection of these names after all existing names.
The fastboot_mmc_part test has been updated for these new names.
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
drivers/fastboot/fb_mmc.c | 158 +++---
test/dm/fastboot.c| 37 -
2 files changed, 132 insertions(+), 63 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index 71eeb02c8f..35e4a29145 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -76,12 +76,37 @@ static int raw_part_get_info_by_name(struct blk_desc
*dev_desc,
return 0;
}
-static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
- const char *name, struct disk_partition *info)
+static int do_get_part_info(struct blk_desc **dev_desc, const char *name,
+ struct disk_partition *info)
{
int ret;
- ret = part_get_info_by_name(dev_desc, name, info);
+ /* First try partition names on the default device */
+ *dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
+ if (*dev_desc) {
+ ret = part_get_info_by_name(*dev_desc, name, info);
+ if (ret >= 0)
+ return ret;
+
+ /* Then try raw partitions */
+ ret = raw_part_get_info_by_name(*dev_desc, name, info);
+ if (ret >= 0)
+ return ret;
+ }
+
+ /* Then try dev.hwpart:part */
+ ret = part_get_info_by_dev_and_name_or_num("mmc", name, dev_desc,
+ info, true);
+ return ret;
+}
+
+static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc,
+ const char *name,
+ struct disk_partition *info)
+{
+ int ret;
+
+ ret = do_get_part_info(dev_desc, name, info);
if (ret < 0) {
/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
char env_alias_name[25 + PART_NAME_LEN + 1];
@@ -92,8 +117,8 @@ static int part_get_info_by_name_or_alias(struct blk_desc
*dev_desc,
strncat(env_alias_name, name, PART_NAME_LEN);
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name != NULL)
- ret = part_get_info_by_name(dev_desc,
- aliased_part_name, info);
+ ret = do_get_part_info(dev_desc, aliased_part_name,
+ info);
}
return ret;
}
@@ -430,27 +455,49 @@ int fastboot_mmc_get_part_info(const char *part_name,
struct blk_desc **dev_desc,
struct disk_partition *part_info, char *response)
{
- int r = 0;
+ int ret;
- *dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
- if (!*dev_desc) {
- fastboot_fail("block device not found", response);
- return -ENOENT;
- }
if (!part_name || !strcmp(part_name, "")) {
fastboot_fail("partition not given", response);
return -ENOENT;
}
- if (raw_part_get_info_by_name(*dev_desc, part_name, part_info) < 0) {
- r = part_get_info_by_name_or_alias(*dev_desc, part_name,
part_info);
- if (r < 0) {
- fastboot_fail("partition not found", response);
- return r;
+ ret = part_get_info_by_name_or_alias(dev_desc, part_name, part_info);
+ if (ret < 0) {
+ switch (ret) {
+ case -ENOSYS:
+ case -EINVAL:
+ fastboot_fail("invalid partition or device", response);
+ break;
+ case -ENODEV:
+ fastboot_fail("no such device", response);
+ break;
+ case -ENOENT:
+ fastboot_fail("no such partition", response);
+ break;
+ case -EPROTONOSUPPORT:
+ fastboot_fail("unknown partition table type", response);
+ break;
+ default:
+
[PATCH v3 7/9] fastboot: Move part_get_info_by_name_or_alias after raw_part_get_info_by_name
This makes the next commit more readable by doing the move now.
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
drivers/fastboot/fb_mmc.c | 44 +++
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index 75347bb99b..71eeb02c8f 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -28,28 +28,6 @@ struct fb_mmc_sparse {
struct blk_desc *dev_desc;
};
-static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
- const char *name, struct disk_partition *info)
-{
- int ret;
-
- ret = part_get_info_by_name(dev_desc, name, info);
- if (ret < 0) {
- /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
- char env_alias_name[25 + PART_NAME_LEN + 1];
- char *aliased_part_name;
-
- /* check for alias */
- strcpy(env_alias_name, "fastboot_partition_alias_");
- strncat(env_alias_name, name, PART_NAME_LEN);
- aliased_part_name = env_get(env_alias_name);
- if (aliased_part_name != NULL)
- ret = part_get_info_by_name(dev_desc,
- aliased_part_name, info);
- }
- return ret;
-}
-
static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
const char *name,
struct disk_partition *info)
@@ -98,6 +76,28 @@ static int raw_part_get_info_by_name(struct blk_desc
*dev_desc,
return 0;
}
+static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
+ const char *name, struct disk_partition *info)
+{
+ int ret;
+
+ ret = part_get_info_by_name(dev_desc, name, info);
+ if (ret < 0) {
+ /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
+ char env_alias_name[25 + PART_NAME_LEN + 1];
+ char *aliased_part_name;
+
+ /* check for alias */
+ strcpy(env_alias_name, "fastboot_partition_alias_");
+ strncat(env_alias_name, name, PART_NAME_LEN);
+ aliased_part_name = env_get(env_alias_name);
+ if (aliased_part_name != NULL)
+ ret = part_get_info_by_name(dev_desc,
+ aliased_part_name, info);
+ }
+ return ret;
+}
+
/**
* fb_mmc_blk_write() - Write/erase MMC in chunks of FASTBOOT_MAX_BLK_WRITE
*
--
2.29.2
[PATCH v3 6/9] fastboot: Remove mmcpart argument from raw_part_get_info_by_name
The only thing mmcpart was used for was to pass to blk_dselect_hwpart.
This calls blk_dselect_hwpart directly from raw_part_get_info_by_name. The
error handling is dropped, but it is reintroduced in the next commit
(albeit less specificly).
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
drivers/fastboot/fb_mmc.c | 41 +--
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index 50532acb84..75347bb99b 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -51,7 +51,8 @@ static int part_get_info_by_name_or_alias(struct blk_desc
*dev_desc,
}
static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
- const char *name, struct disk_partition *info, int *mmcpart)
+const char *name,
+struct disk_partition *info)
{
/* strlen("fastboot_raw_partition_") + PART_NAME_LEN + 1 */
char env_desc_name[23 + PART_NAME_LEN + 1];
@@ -85,8 +86,13 @@ static int raw_part_get_info_by_name(struct blk_desc
*dev_desc,
strncpy((char *)info->name, name, PART_NAME_LEN);
if (raw_part_desc) {
- if (strcmp(strsep(&raw_part_desc, " "), "mmcpart") == 0)
- *mmcpart = simple_strtoul(raw_part_desc, NULL, 0);
+ if (strcmp(strsep(&raw_part_desc, " "), "mmcpart") == 0) {
+ ulong mmcpart = simple_strtoul(raw_part_desc, NULL, 0);
+ int ret = blk_dselect_hwpart(dev_desc, mmcpart);
+
+ if (ret)
+ return ret;
+ }
}
return 0;
@@ -425,7 +431,6 @@ int fastboot_mmc_get_part_info(const char *part_name,
struct disk_partition *part_info, char *response)
{
int r = 0;
- int mmcpart;
*dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
if (!*dev_desc) {
@@ -437,7 +442,7 @@ int fastboot_mmc_get_part_info(const char *part_name,
return -ENOENT;
}
- if (raw_part_get_info_by_name(*dev_desc, part_name, part_info, &mmcpart) < 0) {
+ if (raw_part_get_info_by_name(*dev_desc, part_name, part_info) < 0) {
r = part_get_info_by_name_or_alias(*dev_desc, part_name,
part_info);
if (r < 0) {
fastboot_fail("partition not found", response);
@@ -461,7 +466,6 @@ void fastboot_mmc_flash_write(const char *cmd, void
*download_buffer,
{
struct blk_desc *dev_desc;
struct disk_partition info;
- int mmcpart = 0;
dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
@@ -541,16 +545,12 @@ void fastboot_mmc_flash_write(const char *cmd, void
*download_buffer,
}
#endif
- if (raw_part_get_info_by_name(dev_desc, cmd, &info, &mmcpart) == 0) {
- if (blk_dselect_hwpart(dev_desc, mmcpart)) {
- pr_err("Failed to select hwpart\n");
- fastboot_fail("Failed to select hwpart", response);
+ if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
+ if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
+ pr_err("cannot find partition: '%s'\n", cmd);
+ fastboot_fail("cannot find partition", response);
return;
}
- } else if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
- pr_err("cannot find partition: '%s'\n", cmd);
- fastboot_fail("cannot find partition", response);
- return;
}
if (is_sparse_image(download_buffer)) {
@@ -593,7 +593,6 @@ void fastboot_mmc_erase(const char *cmd, char *response)
struct disk_partition info;
lbaint_t blks, blks_start, blks_size, grp_size;
struct mmc *mmc = find_mmc_device(CONFIG_FASTBOOT_FLASH_MMC_DEV);
- int mmcpart = 0;
if (mmc == NULL) {
pr_err("invalid mmc device\n");
@@ -632,16 +631,12 @@ void fastboot_mmc_erase(const char *cmd, char *response)
}
#endif
- if (raw_part_get_info_by_name(dev_desc, cmd, &info, &mmcpart) == 0) {
- if (blk_dselect_hwpart(dev_desc, mmcpart)) {
- pr_err("Failed to select hwpart\n");
- fastboot_fail("Failed to select hwpart", response);
+ if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
+ if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
+ pr_err("cannot find partition: '%s'\n", cmd);
+ fastboot_fail("cannot find partition", response);
return;
}
- } else if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
-
[PATCH v3 5/9] part: Support string block devices in part_get_info_by_dev_and_name
This adds support for things like "#partname" and "0.1#partname". The block
device parsing is done like in blk_get_device_part_str.
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v2)
Changes in v2:
- Update Documentation
disk/part.c | 41 ++---
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 39c6b00a59..aaeafbb0b2 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -692,12 +692,13 @@ int part_get_info_by_name(struct blk_desc *dev_desc,
const char *name,
* Get partition info from device number and partition name.
*
* Parse a device number and partition name string in the form of
- * "device_num#partition_name", for example "0#misc". If the partition
- * is found, sets dev_desc and part_info accordingly with the information
- * of the partition with the given partition_name.
+ * "devicenum.hwpartnum#partition_name", for example "0.1#misc". devicenum and
+ * hwpartnum are both optional, defaulting to 0. If the partition is found,
+ * sets dev_desc and part_info accordingly with the information of the
+ * partition with the given partition_name.
*
* @param[in] dev_iface Device interface
- * @param[in] dev_part_str Input string argument, like "0#misc"
+ * @param[in] dev_part_str Input string argument, like "0.1#misc"
* @param[out] dev_desc Place to store the device description pointer
* @param[out] part_info Place to store the partition information
* @return 0 on success, or a negative on error
@@ -707,29 +708,31 @@ static int part_get_info_by_dev_and_name(const char
*dev_iface,
struct blk_desc **dev_desc,
struct disk_partition *part_info)
{
- char *ep;
- const char *part_str;
- int dev_num, ret;
+ char *dup_str = NULL;
+ const char *dev_str, *part_str;
+ int ret;
+ /* Separate device and partition name specification */
part_str = strchr(dev_part_str, '#');
- if (!part_str || part_str == dev_part_str)
- return -EINVAL;
-
- dev_num = simple_strtoul(dev_part_str, &ep, 16);
- if (ep != part_str) {
- /* Not all the first part before the # was parsed. */
+ if (part_str) {
+ dup_str = strdup(dev_part_str);
+ dup_str[part_str - dev_part_str] = 0;
+ dev_str = dup_str;
+ part_str++;
+ } else {
return -EINVAL;
}
- part_str++;
- *dev_desc = blk_get_dev(dev_iface, dev_num);
- if (!*dev_desc) {
- printf("Could not find %s %d\n", dev_iface, dev_num);
- return -ENODEV;
- }
+ ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc);
+ if (ret)
+ goto cleanup;
+
ret = part_get_info_by_name(*dev_desc, part_str, part_info);
if (ret < 0)
printf("Could not find \"%s\" partition\n", part_str);
+
+cleanup:
+ free(dup_str);
return ret;
}
--
2.29.2
[PATCH v3 4/9] part: Support getting whole disk from part_get_info_by_dev_and_name_or_num
This adds an option to part_get_info_by_dev_and_name_or_num to allow
callers to specify whether whole-disk partitions are fine.
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
cmd/ab_select.c | 3 ++-
disk/part.c | 5 +++--
include/part.h | 6 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/cmd/ab_select.c b/cmd/ab_select.c
index 6298fcfb60..3e46663d6e 100644
--- a/cmd/ab_select.c
+++ b/cmd/ab_select.c
@@ -22,7 +22,8 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int flag, int
argc,
/* Lookup the "misc" partition from argv[2] and argv[3] */
if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3],
-&dev_desc, &part_info) < 0) {
+&dev_desc, &part_info,
+false) < 0) {
return CMD_RET_FAILURE;
}
diff --git a/disk/part.c b/disk/part.c
index 5e354e256f..39c6b00a59 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -736,7 +736,8 @@ static int part_get_info_by_dev_and_name(const char
*dev_iface,
int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
const char *dev_part_str,
struct blk_desc **dev_desc,
-struct disk_partition *part_info)
+struct disk_partition *part_info,
+int allow_whole_dev)
{
int ret;
@@ -750,7 +751,7 @@ int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
* directly.
*/
ret = blk_get_device_part_str(dev_iface, dev_part_str,
- dev_desc, part_info, 1);
+ dev_desc, part_info, allow_whole_dev);
if (ret < 0)
printf("Couldn't find partition %s %s\n",
dev_iface, dev_part_str);
diff --git a/include/part.h b/include/part.h
index 815515aa80..7f78271a98 100644
--- a/include/part.h
+++ b/include/part.h
@@ -227,12 +227,16 @@ int part_get_info_by_name(struct blk_desc *dev_desc,
* @param[in] dev_part_str Input partition description, like "0#misc" or "0:1"
* @param[out] dev_descPlace to store the device description pointer
* @param[out] part_info Place to store the partition information
+ * @param[in] allow_whole_dev true to allow the user to select partition 0
+ * (which means the whole device), false to require a valid
+ * partition number >= 1
* @return 0 on success, or a negative on error
*/
int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
const char *dev_part_str,
struct blk_desc **dev_desc,
-struct disk_partition *part_info);
+struct disk_partition *part_info,
+int allow_whole_dev);
/**
* part_set_generic_name() - create generic partition like hda1 or sdb2
--
2.29.2
[PATCH v3 3/9] part: Give several functions more useful return values
Several functions in disk/part.c just return -1 on error. This makes them
return different errnos for different failures. This helps callers
differentiate between failures, even if they cannot read stdout.
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
disk/part.c | 50 --
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index b69fd345f3..5e354e256f 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -354,7 +354,7 @@ int part_get_info(struct blk_desc *dev_desc, int part,
}
#endif /* CONFIG_HAVE_BLOCK_DEVICE */
- return -1;
+ return -ENOENT;
}
int part_get_info_whole_disk(struct blk_desc *dev_desc,
@@ -416,7 +416,7 @@ int blk_get_device_by_str(const char *ifname, const char
*dev_hwpart_str,
*dev_desc = get_dev_hwpart(ifname, dev, hwpart);
if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) {
debug("** Bad device %s %s **\n", ifname, dev_hwpart_str);
- dev = -ENOENT;
+ dev = -ENODEV;
goto cleanup;
}
@@ -440,7 +440,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
struct blk_desc **dev_desc,
struct disk_partition *info, int allow_whole_dev)
{
- int ret = -1;
+ int ret;
const char *part_str;
char *dup_str = NULL;
const char *dev_str;
@@ -482,7 +482,7 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
if (0 == strcmp(ifname, "ubi")) {
if (!ubifs_is_mounted()) {
printf("UBIFS not mounted, use ubifsmount to mount volume
first!\n");
- return -1;
+ return -EINVAL;
}
*dev_desc = NULL;
@@ -504,6 +504,7 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
/* If still no dev_part_str, it's an error */
if (!dev_part_str) {
printf("** No device specified **\n");
+ ret = -ENODEV;
goto cleanup;
}
@@ -520,8 +521,10 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
/* Look up the device */
dev = blk_get_device_by_str(ifname, dev_str, dev_desc);
- if (dev < 0)
+ if (dev < 0) {
+ ret = dev;
goto cleanup;
+ }
/* Convert partition ID string to number */
if (!part_str || !*part_str) {
@@ -538,6 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
if (*ep || (part == 0 && !allow_whole_dev)) {
printf("** Bad partition specification %s %s **\n",
ifname, dev_part_str);
+ ret = -ENOENT;
goto cleanup;
}
}
@@ -551,6 +555,7 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
if (!(*dev_desc)->lba) {
printf("** Bad device size - %s %s **\n", ifname,
dev_str);
+ ret = -EINVAL;
goto cleanup;
}
@@ -562,6 +567,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
if ((part > 0) || (!allow_whole_dev)) {
printf("** No partition table - %s %s **\n", ifname,
dev_str);
+ ret = -EPROTONOSUPPORT;
goto cleanup;
}
@@ -630,7 +636,6 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
*info = tmpinfo;
} else {
printf("** No valid partitions found **\n");
- ret = -1;
goto cleanup;
}
}
@@ -638,7 +643,7 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
printf("** Invalid partition type \"%.32s\""
" (expect \"" BOOT_PART_TYPE "\")\n",
info->type);
- ret = -1;
+ ret = -EINVAL;
goto cleanup;
}
@@ -674,7 +679,7 @@ int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name,
}
}
- return -1;
+ return -ENOENT;
}
int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
@@ -704,7 +709,7 @@ static int part_get_info_by_dev_and_name(const char
*dev_iface,
{
char *ep;
const char *part_str;
- int dev_num;
+ int dev_num, ret;
part_str = strchr(dev_part_str, '#');
if (!part_str || part_str == dev_part_str)
@@ -720,13 +725,12 @@ static int part_get_info_by_dev_and_name(const cha
[PATCH v3 2/9] test: dm: Add test for fastboot mmc partition naming
This test verifies the mapping between fastboot partitions and partitions
as understood by U-Boot. It also tests the creation of GPT partitions,
though that is not the primary goal.
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
configs/sandbox64_defconfig | 2 ++
configs/sandbox_defconfig | 2 ++
test/dm/Makefile| 3 ++
test/dm/fastboot.c | 64 +
4 files changed, 71 insertions(+)
create mode 100644 test/dm/fastboot.c
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 4e67819559..9fe07f0c38 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -110,6 +110,8 @@ CONFIG_DM_DEMO=y
CONFIG_DM_DEMO_SIMPLE=y
CONFIG_DM_DEMO_SHAPE=y
CONFIG_DFU_SF=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
CONFIG_GPIO_HOG=y
CONFIG_DM_GPIO_LOOKUP_LABEL=y
CONFIG_PM8916_GPIO=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 0c7674efc9..269288783f 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -135,6 +135,8 @@ CONFIG_DFU_SF=y
CONFIG_DMA=y
CONFIG_DMA_CHANNELS=y
CONFIG_SANDBOX_DMA=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
CONFIG_GPIO_HOG=y
CONFIG_DM_GPIO_LOOKUP_LABEL=y
CONFIG_PM8916_GPIO=y
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 46e076ed09..be4385c709 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -91,5 +91,8 @@ obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
ifneq ($(CONFIG_PINMUX),)
obj-$(CONFIG_PINCONF) += pinmux.o
endif
+ifneq ($(CONFIG_EFI_PARTITION)$(CONFIG_FASTBOOT_FLASH_MMC),)
+obj-y += fastboot.o
+endif
endif
endif # !SPL
diff --git a/test/dm/fastboot.c b/test/dm/fastboot.c
new file mode 100644
index 00..8f905d8fa8
--- /dev/null
+++ b/test/dm/fastboot.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2015 Google, Inc
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define FB_ALIAS_PREFIX "fastboot_partition_alias_"
+
+static int dm_test_fastboot_mmc_part(struct unit_test_state *uts)
+{
+ char response[FASTBOOT_RESPONSE_LEN] = {0};
+ char str_disk_guid[UUID_STR_LEN + 1];
+ struct blk_desc *mmc_dev_desc, *fb_dev_desc;
+ struct disk_partition part_info;
+ struct disk_partition parts[2] = {
+ {
+ .start = 48, /* GPT data takes up the first 34 blocks
or so */
+ .size = 1,
+ .name = "test1",
+ },
+ {
+ .start = 49,
+ .size = 1,
+ .name = "test2",
+ },
+ };
+
+ ut_assertok(blk_get_device_by_str("mmc",
+
__stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV),
+ &mmc_dev_desc));
+ if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
+ gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
+ gen_rand_uuid_str(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, parts,
+ ARRAY_SIZE(parts)));
+
+ /* "Classic" partition labels */
+ ut_asserteq(1, fastboot_mmc_get_part_info("test1", &fb_dev_desc,
+ &part_info, response));
+ ut_asserteq(2, fastboot_mmc_get_part_info("test2", &fb_dev_desc,
+ &part_info, response));
+
+ /* Test aliases */
+ ut_assertnull(env_get(FB_ALIAS_PREFIX "test3"));
+ ut_assertok(env_set(FB_ALIAS_PREFIX "test3", "test1"));
+ ut_asserteq(1, fastboot_mmc_get_part_info("test3", &fb_dev_desc,
+ &part_info, response));
+ ut_assertok(env_set(FB_ALIAS_PREFIX "test3", NULL));
+
+ return 0;
+}
+DM_TEST(dm_test_fastboot_mmc_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.29.2
Re: MX6sabresd broken since v2019.04
On 2/1/21 5:29 PM, Fabio Estevam wrote: Hi Marek, On Mon, Feb 1, 2021 at 12:01 PM Marek Vasut wrote: I posted a patch, you are on CC, thanks. Can you please add this u-boot-with-spl.imx into the NXP test matrix ? I am not aware of any NXP boot farm tests being done in U-Boot. Maybe Peng can comment. I know Heiko tests wandboard on his boot infrastructure. Maybe he could add u-boot-with-spl.imx there? I don't think WB uses OF_SEPARATE with fitImages, so maybe there its not applicable at all ?
[PATCH v3 1/9] mmc: sandbox: Add support for writing
This adds support writing to the sandbox mmc backed by an in-memory
buffer. The unit test has been updated to test reading, writing, and
erasing. I'm not sure what MMCs erase to; I picked 0, but if it's 0xFF
then that can be easily changed.
Signed-off-by: Sean Anderson
Reviewed-by: Simon Glass
---
(no changes since v1)
drivers/mmc/sandbox_mmc.c | 43 +--
test/dm/mmc.c | 19 -
2 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c
index 8a2391d651..d7b6e7f898 100644
--- a/drivers/mmc/sandbox_mmc.c
+++ b/drivers/mmc/sandbox_mmc.c
@@ -17,6 +17,17 @@ struct sandbox_mmc_plat {
struct mmc mmc;
};
+#define MMC_CSIZE 0
+#define MMC_CMULT 8 /* 8 because the card is high-capacity */
+#define MMC_BL_LEN_SHIFT 10
+#define MMC_BL_LEN BIT(MMC_BL_LEN_SHIFT)
+#define MMC_CAPACITY (((MMC_CSIZE + 1) << (MMC_CMULT + 2)) \
+ * MMC_BL_LEN) /* 1 MiB */
+
+struct sandbox_mmc_priv {
+ u8 buf[MMC_CAPACITY];
+};
+
/**
* sandbox_mmc_send_cmd() - Emulate SD commands
*
@@ -26,6 +37,10 @@ struct sandbox_mmc_plat {
static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
struct mmc_data *data)
{
+ struct sandbox_mmc_priv *priv = dev_get_priv(dev);
+ struct mmc *mmc = mmc_get_mmc_dev(dev);
+ static ulong erase_start, erase_end;
+
switch (cmd->cmdidx) {
case MMC_CMD_ALL_SEND_CID:
memset(cmd->response, '\0', sizeof(cmd->response));
@@ -44,8 +59,9 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct
mmc_cmd *cmd,
break;
case MMC_CMD_SEND_CSD:
cmd->response[0] = 0;
- cmd->response[1] = 10 << 16; /* 1 << block_len */
- cmd->response[2] = 0;
+ cmd->response[1] = (MMC_BL_LEN_SHIFT << 16) |
+ ((MMC_CSIZE >> 16) & 0x3f);
+ cmd->response[2] = (MMC_CSIZE & 0x) << 16;
cmd->response[3] = 0;
break;
case SD_CMD_SWITCH_FUNC: {
@@ -59,13 +75,27 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct
mmc_cmd *cmd,
break;
}
case MMC_CMD_READ_SINGLE_BLOCK:
- memset(data->dest, '\0', data->blocksize);
- break;
case MMC_CMD_READ_MULTIPLE_BLOCK:
- strcpy(data->dest, "this is a test");
+ memcpy(data->dest, &priv->buf[cmd->cmdarg * data->blocksize],
+ data->blocks * data->blocksize);
+ break;
+ case MMC_CMD_WRITE_SINGLE_BLOCK:
+ case MMC_CMD_WRITE_MULTIPLE_BLOCK:
+ memcpy(&priv->buf[cmd->cmdarg * data->blocksize], data->src,
+ data->blocks * data->blocksize);
break;
case MMC_CMD_STOP_TRANSMISSION:
break;
+ case SD_CMD_ERASE_WR_BLK_START:
+ erase_start = cmd->cmdarg;
+ break;
+ case SD_CMD_ERASE_WR_BLK_END:
+ erase_end = cmd->cmdarg;
+ break;
+ case MMC_CMD_ERASE:
+ memset(&priv->buf[erase_start * mmc->write_bl_len], '\0',
+ (erase_end - erase_start + 1) * mmc->write_bl_len);
+ break;
case SD_CMD_APP_SEND_OP_COND:
cmd->response[0] = OCR_BUSY | OCR_HCS;
cmd->response[1] = 0;
@@ -148,5 +178,6 @@ U_BOOT_DRIVER(mmc_sandbox) = {
.bind = sandbox_mmc_bind,
.unbind = sandbox_mmc_unbind,
.probe = sandbox_mmc_probe,
- .plat_auto = sizeof(struct sandbox_mmc_plat),
+ .priv_auto_alloc_size = sizeof(struct sandbox_mmc_priv),
+ .platdata_auto_alloc_size = sizeof(struct sandbox_mmc_plat),
};
diff --git a/test/dm/mmc.c b/test/dm/mmc.c
index 4e5136c850..f744452ff2 100644
--- a/test/dm/mmc.c
+++ b/test/dm/mmc.c
@@ -29,16 +29,25 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
{
struct udevice *dev;
struct blk_desc *dev_desc;
- char cmp[1024];
+ int i;
+ char write[1024], read[1024];
ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev));
ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc));
- /* Read a few blocks and look for the string we expect */
+ /* Write a few blocks and verify that we get the same data back */
ut_asserteq(512, dev_desc->blksz);
- memset(cmp, '\0', sizeof(cmp));
- ut_asserteq(2, blk_dread(dev_desc, 0, 2, cmp));
- ut_assertok(strcmp(cmp, "this is a test"));
+ for (i = 0; i < sizeof(write); i++)
+ write[i] = i;
+ ut_asserteq(2, blk_dwrite(dev_desc, 0, 2, write));
+ ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
+ ut_asserteq_mem(write, read, sizeof(write));
+
+ /* Now erase them */
+ memset(write, '\0', si
[PATCH v3 0/9] fastboot: Add better support for specifying partitions
This series adds support for flashing eMMC boot partitions, and for flashing whole partitions. Specifically, it does this by using the existing U-Boot naming scheme to specify partitions, and not by adding new KConfig options. I have added tests for partition naming, but not for the whole flash process (though that could be a future project). I have tested this on one board, but I have *not* tested the mt85-specific KConfigs. Hopefully this series can be a way to phase out those options. Changes in v3: - Rebase onto dfu/master Changes in v2: - Update documentation for part_get_info_by_dev_and_name - Move partition documentation under doc/usage Sean Anderson (9): mmc: sandbox: Add support for writing test: dm: Add test for fastboot mmc partition naming part: Give several functions more useful return values part: Support getting whole disk from part_get_info_by_dev_and_name_or_num part: Support string block devices in part_get_info_by_dev_and_name fastboot: Remove mmcpart argument from raw_part_get_info_by_name fastboot: Move part_get_info_by_name_or_alias after raw_part_get_info_by_name fastboot: Allow u-boot-style partitions fastboot: Partition specification cmd/ab_select.c | 3 +- configs/sandbox64_defconfig | 2 + configs/sandbox_defconfig | 2 + disk/part.c | 90 --- doc/android/fastboot.rst| 4 + doc/usage/index.rst | 1 + doc/usage/part.rst | 33 ++ drivers/fastboot/fb_mmc.c | 211 +--- drivers/mmc/sandbox_mmc.c | 43 +++- include/part.h | 6 +- test/dm/Makefile| 3 + test/dm/fastboot.c | 95 test/dm/mmc.c | 19 +++- 13 files changed, 370 insertions(+), 142 deletions(-) create mode 100644 doc/usage/part.rst create mode 100644 test/dm/fastboot.c -- 2.29.2
Re: [PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops
On 01/02/21 10:33AM, Sean Anderson wrote: > On 2/1/21 7:18 AM, Pratyush Yadav wrote: > > On 31/01/21 07:34PM, Sean Anderson wrote: > > > This prints some basic metadata about the SPI memory op. This information > > > may be used to debug SPI drivers (e.g. determining the expected SPI mode). > > > It is also helpful for verifying that the data on the wire matches the > > > data > > > intended to be transmitted (e.g. with a logic analyzer). The opcode is > > > printed with a format of %02Xh to match the notation commonly used in > > > flash > > > datasheets. > > > > > > Signed-off-by: Sean Anderson > > > --- > > > > > > drivers/spi/spi-mem.c | 6 ++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c > > > index c095ae9505..eb83c11910 100644 > > > --- a/drivers/spi/spi-mem.c > > > +++ b/drivers/spi/spi-mem.c > > > @@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const > > > struct spi_mem_op *op) > > > int ret; > > > int i; > > > + dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n", > > > + op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth, > > > + op->data.buswidth, op->addr.val, > > > + op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth > > > : 0, > > > > SPI MEM deals with dummy bytes not cycles [0]. Just print them directly > > instead of converting them to cycles first. > > I chose to do it this way, since as far as I can tell, most quad-spi > drivers (with the exception of mtk_snfi_spi.c, octeon_spi.c, spi-sifive.c, > and ti_qspi.c) deal with cycles and not bytes. The spi-nor system also > specifies dummies in cycles and only converts them to bytes when > creating a struct spi_mem_op. SPI flash datasheets also typically refer > to dummies in cycles; since there is no data being transmitted, they > cannot be truly measured in bytes. Lastly, it is much easier to only > work in cycles when looking at logic analyzer traces. When switching > between 1-1-4 and 1-4-4 instructions, the meaning of "cycles" remains > the same, while "bytes" has a variable amount of time. I agree that dummy cycles is a better unit than dummy bytes, but that is not what is currently being used. You are interpreting the dummy.nbytes field rather than simply printing it and letting the reader see what it means. We don't pass dummy cycles to the controller. We pass dummy bytes. So it makes sense to print that in the debug message. Anyway, I feel like we are getting into bikeshedding territory. So either is fine by me. Choose whichever makes more sense to you, keeping my arguments in mind. > > Also agree with Bin that "dummy cycles=" (or dummy bytes if you are > > printing bytes directly) is clearer. > > Sure. > > > > [0] For now. This might change later. See > > https://lore.kernel.org/linux-mtd/6396018a-485f-6eb4-7742-bdb5c4335...@microchip.com/ > > Would be nice :) > > --Sean > > > > > With these fixed, > > > > Reviewed-by: Pratyush Yadav > > > > > + op->data.nbytes); > > > + > > > if (!spi_mem_supports_op(slave, op)) > > > return -ENOTSUPP; > > > -- Regards, Pratyush Yadav Texas Instruments Inc.
Re: MX6sabresd broken since v2019.04
Hi Marek, On Mon, Feb 1, 2021 at 12:01 PM Marek Vasut wrote: > I posted a patch, you are on CC, thanks. > > Can you please add this u-boot-with-spl.imx into the NXP test matrix ? I am not aware of any NXP boot farm tests being done in U-Boot. Maybe Peng can comment. I know Heiko tests wandboard on his boot infrastructure. Maybe he could add u-boot-with-spl.imx there?
Re: [PATCH 04/11] dm: Remove uses of device_bind_offset()
On 01.02.2021 14:02, Simon Glass wrote:
> Hi Eugen,
>
> On Mon, 1 Feb 2021 at 01:13, wrote:
>>
>> On 31.01.2021 17:37, Simon Glass wrote:
>>> Hi Eugen,
>>>
>>> On Sun, 31 Jan 2021 at 02:18, wrote:
On 10.12.2020 02:26, Simon Glass wrote:
> This function is not needed since the standard device_bind() can be used
> instead.
>
> Signed-off-by: Simon Glass
> ---
>
> arch/x86/cpu/apollolake/spl.c | 2 +-
> drivers/clk/at91/compat.c | 20
> drivers/clk/clk.c | 2 +-
> drivers/gpio/mt7621_gpio.c | 4 ++--
> drivers/gpio/s5p_gpio.c | 4 ++--
> drivers/gpio/sunxi_gpio.c | 4 ++--
> drivers/gpio/tegra186_gpio.c| 4 ++--
> drivers/gpio/tegra_gpio.c | 6 +++---
> drivers/net/mvpp2.c | 4 ++--
> drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 5 ++---
> drivers/pinctrl/meson/pinctrl-meson.c | 4 +++-
> drivers/pinctrl/mscc/pinctrl-jr2.c | 4 ++--
> drivers/pinctrl/mscc/pinctrl-luton.c| 4 ++--
> drivers/pinctrl/mscc/pinctrl-ocelot.c | 4 ++--
> drivers/pinctrl/mscc/pinctrl-serval.c | 4 ++--
> drivers/pinctrl/mscc/pinctrl-servalt.c | 4 ++--
> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 8
> drivers/power/regulator/Kconfig | 2 +-
> include/dm/device-internal.h| 4 ++--
> include/power/regulator.h | 2 +-
> 20 files changed, 46 insertions(+), 49 deletions(-)
>
> Applied to u-boot-dm, thanks!
>
Hi Simon,
I bisected the tree and this commit looks to break
sama5d4_xplained_mmc_defconfig :
No serial driver found
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Could not initialize timer (err -19)
Booting u-boot fails when adding this commit.
Could you please help or let me know how I can fix it ?
>>>
>>> I suspect the problem could be in the changes to
>>> drivers/clk/at91/compat.c although I cannot see why
>>>
>>> You could try reverting that change, and just using offset_to_ofnode()
>>> in the device_bind_driver_to_node() call. I actually intended to do
>>> that at the time due to the risk, but somehow I missed this one.
>>>
>>> OTOH it would be good to move the code to livetree and stop using fdt
>>> offsets.
>>>
>>> Regards,
>>> Simon
>>>
>>
>> I reverted the changes in compat.c and indeed now it boots correctly.
>>
>> I tried to do the following change on top of your code as you suggested
>> but it does not help:
>>
>>
>> --- a/drivers/clk/at91/compat.c
>> +++ b/drivers/clk/at91/compat.c
>> @@ -67,7 +67,7 @@ int at91_clk_sub_device_bind(struct udevice *dev,
>> const char *drv_name)
>> bool pre_reloc_only = !(gd->flags & GD_FLG_RELOC);
>> const char *name;
>> int ret;
>> -
>> + int offset = dev_of_offset(dev);
>> ofnode_for_each_subnode(node, parent) {
>> if (pre_reloc_only && !ofnode_pre_reloc(node))
>> continue;
>> @@ -84,7 +84,7 @@ int at91_clk_sub_device_bind(struct udevice *dev,
>> const char *drv_name)
>> name = ofnode_get_name(node);
>> if (!name)
>> return -EINVAL;
>> - ret = device_bind_driver_to_node(dev, drv_name, name, node,
>> + ret = device_bind_driver_to_node(dev, drv_name, name,
>> offset_to_ofnode(offset),
>>NULL);
>> if (ret)
>> return ret;
>>
>>
>> I have a feeling the 'for loop' for the subnodes misses an essential
>> driver and thus it fails booting
>
> Then I think reverting all the changes is the best thing in this file.
> Can you send a patch?
>
> Ultimately this should be figured out, but I cannot see what is wrong
> and don't have that hardware to try. I do have an old SAM9260/9263 but
> I'm not sure if that uses the same driver.
>
> Regards,
> Simon
>
I can do that, but if you have any hunches, I can test patches for you.
Re: Please pull u-boot-x86
On Mon, Feb 01, 2021 at 05:37:17PM +0800, Bin Meng wrote: > Hi Tom, > > This PR includes the following x86 changes for v2021.04: > > - Fix CMD_ACPI dependency in Kconfig > - Correct overflow in __udelay() in TSC timer driver > - Add a devicetree node for eMMC for Coral > - Minor improvements on image loading > - Reduce size of Samus image > > Azure results: PASS > https://dev.azure.com/bmeng/GitHub/_build/results?buildId=316&view=results > > The following changes since commit b4804cdd5747d1d932bd338e0ca102ade51b8b6b: > > Merge https://gitlab.denx.de/u-boot/custodians/u-boot-usb > (2021-01-31 14:24:35 -0500) > > are available in the git repository at: > > https://gitlab.denx.de/u-boot/custodians/u-boot-x86 > > for you to fetch changes up to 77f898d04095cdccb69c476ba0aa19f257fca64d: > > x86: Reduce size of samus image (2021-02-01 15:33:26 +0800) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[ANN] U-Boot v2021.04-rc1 released
Hey all, It's release day, and here's v2021.04-rc1. As is often the case, there's a few PRs still I would have hoped to see by now come in but I think will be coming in soon. And there's a few more changes in my own queue that I think I can / should pick up still and I'll try and get to this week. As always, I leave what comes in at this point to the discretion of the custodians making the PRs. In terms of a changelog, git log --merges v2021.01..v2021.04-rc1 contains what I've pulled but as always, better PR messages and tags will provide better results here. I do have my reminders setup for doing -rc releases every other Monday from here on out and final release on April 5th, 2021. Thanks all! -- Tom signature.asc Description: PGP signature
Re: Pull request: u-boot-sunxi/master for v2021.04 (part 3)
On Mon, Feb 01, 2021 at 12:25:47AM +, Andre Przywara wrote: > Hi Tom, > > please pull the master branch from u-boot-sunxi, containing some late > sunxi patches for the 2021.04 merge window: Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 0/4] fastboot: mmc: Add CONFIG_FASTBOOT_MMC_USER_SUPPORT
Looks like I didn't CC people properly...
On 1/27/21 11:36 AM, Sean Anderson wrote:
Hi Patrick,
I believe that the first two patches in this series can be replicated
with [1]. For example, if you currently use FASTBOOT_MMC_BOOT_SUPPORT
with FASTBOOT_MMC_BOOT1_NAME set to "mmc0boot1", leading to commands
like
$ fastboot erase mmc0boot1
You could instead do
$ fastboot erase 0.1:0
And the first behavior could be emulated by setting the environmental
variable "fastboot_partition_alias_mmc0boot1" to "0.1:0".
I would like to work towards deprecating Kconfigs for achieving this
particular use case. This is because everything is set at compile-time,
but we have existing tools which make this easy to do at run-time.
Favoring run-time configuration makes it easier to use one U-Boot for
different boards, and also makes it easier for users to modify U-Boot.
For the latter two patches, I think there are two existing solutions.
First, there is the patch to add "ucmd" support to fastboot. This allows
running arbitrary commands on the U-Boot side. However, this may be
unsuitable for systems which need to maintain a chain of trust (since
allowing arbitrary commands would allow arbitrary software to run).
With this in mind, FIT images allow for script sections. For example,
one could create an image tree source file like
/dts-v1/;
/ {
description = "Configuration script";
#address-cells = <1>;
images {
default = "script-1";
script-1 {
data = /incbin/("mmc.scr");
type = "script";
compression = "none";
signature {
algo = "sha1,rsa2048";
key-name-hint = "dev";
};
};
};
};
(or something similar; I haven't tested this). This would create a fit
with containing "mmc.scr". On the U-Boot side, running
fastboot 0
source
Would source any script contained within the FIT image (if it was
downloaded e.g. with "fastboot boot mmc.itb". I think this process would
work well for "run once" scripts like setting the mmc boot partitions.
Please let me know if any of the above suggestions would achieve the
functionality you need.
--Sean
[1] https://patchwork.ozlabs.org/project/uboot/list/?series=223198
[2]
https://patchwork.ozlabs.org/project/uboot/patch/2021001919.228555-1...@denx.de/
On 1/27/21 8:46 AM, Patrick Delaunay wrote:
Hi,
It is a rebased V2 version of the serie [1].
This serie adds a lot of new #if and doesn't respect the last
U-Boot coding rules with 14 warnings detected by checkpatch:
warning: Use 'if (IS_ENABLED(CONFIG...))'
instead of '#if or #ifdef' where possible
But I chose to copy the existing code of the fastboot files
fb_command.c to a have an easier review.
So I prefer sent a patch (if it is required) to remove all the
#ifdef in this file when the serie will be accepted.
I check compilation of the added features on stm32mp1 platform
with the serie [2].
The compilation for modified boards (with already activated config
CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT) is verified with buildman:
tools/buildman/buildman mt8512_bm1_emmc mt8518_ap1_emmc pumpkin
Building current source for 3 boards (3 threads, 4 jobs per thread)
aarch64: w+ pumpkin
+= WARNING ==
+This board does not use CONFIG_DM_ETH (Driver Model
+for Ethernet drivers). Please update the board to use
+CONFIG_DM_ETH before the v2020.07 release. Failure to
+update by the deadline may result in board removal.
+See doc/driver-model/migration.rst for more info.
+
aarch64: w+ mt8518_ap1_emmc
+= WARNING ==
+This board does not use CONFIG_DM_ETH (Driver Model
+for Ethernet drivers). Please update the board to use
+CONFIG_DM_ETH before the v2020.07 release. Failure to
+update by the deadline may result in board removal.
+See doc/driver-model/migration.rst for more info.
+
aarch64: w+ mt8512_bm1_emmc
+= WARNING ==
+This board does not use CONFIG_DM_ETH (Driver Model
+for Ethernet drivers). Please update the board to use
+CONFIG_DM_ETH before the v2020.07 release. Failure to
+update by the deadline may result in board removal.
+See doc/driver-model/migration.rst for more info.
+
0 3 0 /3 0:00:07 : mt8512_bm1_emmc
Completed: 3 total built, duration 0:00:23, rate 0.13
[1] "fastboot: mmc: Add CONFIG_FASTBOOT_MMC_USER_SUPPORT"
http://patchwork.ozlabs.org/project/uboot/list/?series=200509&state=*
[2] "configs: stm32mp1: enable fastboot support of eMMC boot partition"
http://patchwork.ozlabs.org/project/uboot/list/?series=200510
Regards
Patrick
Changes in v2:
- rebase on master branch
- new impact on pumpkin_defconfig and mt8512_bm1_emmc_defconfig
- new impact on pumpki
Re: [PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops
On 2/1/21 7:18 AM, Pratyush Yadav wrote: On 31/01/21 07:34PM, Sean Anderson wrote: This prints some basic metadata about the SPI memory op. This information may be used to debug SPI drivers (e.g. determining the expected SPI mode). It is also helpful for verifying that the data on the wire matches the data intended to be transmitted (e.g. with a logic analyzer). The opcode is printed with a format of %02Xh to match the notation commonly used in flash datasheets. Signed-off-by: Sean Anderson --- drivers/spi/spi-mem.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index c095ae9505..eb83c11910 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) int ret; int i; + dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n", + op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth, + op->data.buswidth, op->addr.val, + op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth : 0, SPI MEM deals with dummy bytes not cycles [0]. Just print them directly instead of converting them to cycles first. I chose to do it this way, since as far as I can tell, most quad-spi drivers (with the exception of mtk_snfi_spi.c, octeon_spi.c, spi-sifive.c, and ti_qspi.c) deal with cycles and not bytes. The spi-nor system also specifies dummies in cycles and only converts them to bytes when creating a struct spi_mem_op. SPI flash datasheets also typically refer to dummies in cycles; since there is no data being transmitted, they cannot be truly measured in bytes. Lastly, it is much easier to only work in cycles when looking at logic analyzer traces. When switching between 1-1-4 and 1-4-4 instructions, the meaning of "cycles" remains the same, while "bytes" has a variable amount of time. Also agree with Bin that "dummy cycles=" (or dummy bytes if you are printing bytes directly) is clearer. Sure. [0] For now. This might change later. See https://lore.kernel.org/linux-mtd/6396018a-485f-6eb4-7742-bdb5c4335...@microchip.com/ Would be nice :) --Sean With these fixed, Reviewed-by: Pratyush Yadav + op->data.nbytes); + if (!spi_mem_supports_op(slave, op)) return -ENOTSUPP;
[PATCH v3] sysboot: add zboot support to boot x86 Linux kernel image
Add "zboot" command to the list of supported boot in the label_boot function. Signed-off-by: Kory Maincent --- Change since v1: - Modify comment Change since v2: - Update do_zboot to do_zboot_parent function to follow the patch: 5588e776b0 cmd/pxe_utils.c | 4 include/command.h | 3 +++ 2 files changed, 7 insertions(+) diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c index 3526a651d7..06611262c1 100644 --- a/cmd/pxe_utils.c +++ b/cmd/pxe_utils.c @@ -657,6 +657,10 @@ static int label_boot(struct cmd_tbl *cmdtp, struct pxe_label *label) /* Try booting a Image */ else do_bootz(cmdtp, 0, bootm_argc, bootm_argv); +#elif defined(CONFIG_CMD_ZBOOT) + /* Try booting an x86_64 Linux kernel image */ + else + do_zboot_parent(cmdtp, 0, bootm_argc, bootm_argv, NULL); #endif unmap_sysmem(buf); diff --git a/include/command.h b/include/command.h index e229bf2825..cb91ba81b5 100644 --- a/include/command.h +++ b/include/command.h @@ -165,6 +165,9 @@ extern int do_bootz(struct cmd_tbl *cmdtp, int flag, int argc, extern int do_booti(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); +extern int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[], int *repeatable); + extern int common_diskboot(struct cmd_tbl *cmdtp, const char *intf, int argc, char *const argv[]); -- 2.17.1
Re: [PATCH 00/13] Nokia RX-51: Fix USB TTY console and enable it
Hello! On Sunday 17 January 2021 16:07:30 Lokesh Vutla wrote: > Hi Lukasz, > > On 29/11/20 10:16 pm, Pali Rohár wrote: > > This patch series fix usbtty code (serial console via USB peripheral > > mode), fix underlying musb peripheral code, fix compilation of > > CONFIG_USB_DEVICE (used by usbtty), remove unused Nokia RX-51 code to > > decrease size of U-Boot binary and finally enable usbtty serial console > > for Nokia RX-51. > > > > With this patch series debugging of Nokia RX-51 can be done also via USB > > serial console. > > > > On computer this serial console is accessible via /dev/ttyACM0 device. > > > > With current implementation there is an issue in musb driver that it > > loose receiving bytes from USB bus when too many a characters are send > > over USB tty from computer. Typing on keyboard to kermit terminal > > connected to /dev/ttyACM0 is working fine. But pasting more more bytes > > to terminal cause data lost on receiving side. I do not know where is > > the issue or how to fix it (it looks like that data are lost at low > > level when reading them from msub FIFO hardware) but typing on keyboard > > is working fine. This is rather issue for sending files via x/y/z-modem > > or kermit protocol. Currently U-Boot is not able to receive any file > > via usbtty with musb driver due to this issue. > > Can you take a look at usb related patches and merge them if you are okay > with it? > > Thanks and regards, > Lokesh I would like to remind this patch series too! I have not received any negative feedback on it for 2 months and patches were already reviewed by Pavel. Could you please merge this patch series? > > > > Pali Rohár (13): > > serial: usbtty: Fix puts function > > usb: musb: Fix compilation of gadget code > > usb: musb: Always clear the data toggle bit when configuring ep > > usb: musb: Fix configuring FIFO for endpoints > > usb: musb: Read value of PERI_RXCSR to 16bit variable > > usb: musb: Fix transmission of bigger buffers > > usb: gadget: Do not export usbd_device_* arrays > > usb: gadget: Use dbg_ep0() macro instead of serial_printf() > > arm: omap3: Compile lowlevel_init() function only when it is used > > arm: omap3: Compile s_init() function only when it is used > > Nokia RX-51: Remove function set_muxconf_regs() > > Nokia RX-51: Move content of rx51.h to rx51.c > > Nokia RX-51: Enable usbtty serial console by default > > > > Makefile | 1 + > > arch/arm/mach-omap2/omap3/board.c | 3 + > > arch/arm/mach-omap2/omap3/lowlevel_init.S | 6 +- > > board/nokia/rx51/rx51.c | 28 +- > > board/nokia/rx51/rx51.h | 377 -- > > configs/nokia_rx51_defconfig | 6 +- > > doc/README.nokia_rx51 | 15 +- > > drivers/serial/usbtty.c | 4 +- > > drivers/usb/gadget/core.c | 38 +-- > > drivers/usb/gadget/ep0.c | 47 ++- > > drivers/usb/musb/musb_core.c | 10 +- > > drivers/usb/musb/musb_udc.c | 19 +- > > include/configs/nokia_rx51.h | 16 +- > > include/usbdevice.h | 15 - > > 14 files changed, 92 insertions(+), 493 deletions(-) > > delete mode 100644 board/nokia/rx51/rx51.h > >
