Re: [PATCH v2] boot: android: fix booting without a ramdisk
Hi,
> > > - if (ret)
> > > + if (ret == -ENOENT)
> > > + return -ENOPKG;
>
> We normally use -ENOENT for this sort of thing.
That's the way select_ramdisk() is documented. It was actually
introduced by yourself in commit e4c928792e93 ("image: Split up
boot_get_ramdisk()") :)
-michael
signature.asc
Description: PGP signature
[PATCH v2] boot: android: fix booting without a ramdisk
android_image_get_ramdisk() will return an error if there is no ramdisk.
Using the android image without a ramdisk worked until commit
1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
the return code wasn't checked until then. Return -ENOENT in case
there is no ramdisk and translate that into -ENOPKG in the calling
code, which will then indicate "no ramdisk" to its caller
(boot_get_ramdisk()).
This way, we can get rid of the "*rd_data = *rd_len = 0;" in the error
path, too.
With this, I'm able to boot a linux kernel using fastboot again:
fastboot --base 0x4100 --header-version 2 --dtb /path/to/dtb \
--cmdline "root=/dev/mmcblk0p1 rootwait" boot path/to/Image
Signed-off-by: Michael Walle
---
boot/image-android.c | 7 +++
boot/image-board.c | 4 +++-
include/image.h | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/boot/image-android.c b/boot/image-android.c
index 09c7a44e058..774565fd1fe 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -393,10 +393,9 @@ int android_image_get_ramdisk(const void *hdr, const void
*vendor_boot_img,
if (!android_image_get_data(hdr, vendor_boot_img, &img_data))
return -EINVAL;
- if (!img_data.ramdisk_size) {
- *rd_data = *rd_len = 0;
- return -1;
- }
+ if (!img_data.ramdisk_size)
+ return -ENOENT;
+
if (img_data.header_version > 2) {
ramdisk_ptr = img_data.ramdisk_addr;
memcpy((void *)(ramdisk_ptr), (void
*)img_data.vendor_ramdisk_ptr,
diff --git a/boot/image-board.c b/boot/image-board.c
index f2124013046..eca1b1d2bff 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -427,7 +427,9 @@ static int select_ramdisk(struct bootm_headers *images,
const char *select, u8 a
unmap_sysmem(ptr);
}
- if (ret)
+ if (ret == -ENOENT)
+ return -ENOPKG;
+ else if (ret)
return ret;
done = true;
}
diff --git a/include/image.h b/include/image.h
index dd4042d1bd9..2ab17075287 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1858,7 +1858,7 @@ int android_image_get_kernel(const void *hdr,
* @vendor_boot_img : Pointer to vendor boot image header
* @rd_data: Pointer to a ulong variable, will hold ramdisk address
* @rd_len:Pointer to a ulong variable, will hold ramdisk length
- * Return: 0 if succeeded, -1 if ramdisk size is 0
+ * Return: 0 if OK, -ENOPKG if no ramdisk, -EINVAL if invalid image
*/
int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
ulong *rd_data, ulong *rd_len);
--
2.39.2
[PATCH] boot: android: fix booting without a ramdisk
android_image_get_ramdisk() will return an error if there is no ramdisk.
Using the android image without a ramdisk worked until commit
1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
that return code wasn't checked. Now that it is checked, don't return an
error in the (valid) case that there is no ramdisk in the image.
With this, I'm able to boot a linux kernel using fastboot again:
fastboot --base 0x4100 --header-version 2 --dtb /path/to/dtb \
--cmdline "root=/dev/mmcblk0p1 rootwait" boot path/to/Image
Signed-off-by: Michael Walle
---
boot/image-android.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot/image-android.c b/boot/image-android.c
index 09c7a44e058..0900579ee8c 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -395,7 +395,7 @@ int android_image_get_ramdisk(const void *hdr, const void
*vendor_boot_img,
if (!img_data.ramdisk_size) {
*rd_data = *rd_len = 0;
- return -1;
+ return 0;
}
if (img_data.header_version > 2) {
ramdisk_ptr = img_data.ramdisk_addr;
--
2.39.2
[PATCH v2 2/2] spi: sunxi: fix clock divider calculation for max frequency setting
If the maximum frequency is requested, we still fall into the CDR2
handling. But there the minimal divider is 2. For the sun6i and sun8i we
can do better with the CDR1 setting where the minimal divider is 1:
SPI_CLK = MOD_CLK / 2 ^ cdr with cdr = 0
Thus, handle the div = 1 case specially.
While at it, correct the comment above the calculation.
Signed-off-by: Michael Walle
---
drivers/spi/spi-sunxi.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index f110a8b7658..81f1298adea 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -249,6 +249,8 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
* We have two choices there. Either we can use the clock
* divide rate 1, which is calculated thanks to this formula:
* SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
+* Or for sun6i/sun8i variants:
+* SPI_CLK = MOD_CLK / (2 ^ cdr)
* Or we can use CDR2, which is calculated with the formula:
* SPI_CLK = MOD_CLK / (2 * (cdr + 1))
* Whether we use the former or the latter is set through the
@@ -256,13 +258,16 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
*
* First try CDR2, and if we can't reach the expected
* frequency, fall back to CDR1.
+* There is one exception if the requested clock is the input
+* clock. In that case we always use CDR1 because we'll get a
+* 1:1 ration for sun6i/sun8i variants.
*/
div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
div_cdr2 = DIV_ROUND_UP(div, 2);
reg = readl(SPI_REG(priv, SPI_CCR));
- if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+ if (div != 1 && (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1))) {
reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
reg |= SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
} else {
--
2.39.2
[PATCH v2 1/2] spi: sunxi: fix CDR2 calculation
The CDR2 divider calculation always yield a frequency greater than the
requested one. Use DIV_ROUND_UP() to keep the frequency equal or below
the requested one. This way, we can also drop the "if div > 0" check
because we know for a fact that div cannot be zero.
FWIW, this aligns the CDR2 calculation with the linux driver.
Suggested-by: Andre Przywara
Signed-off-by: Michael Walle
---
drivers/spi/spi-sunxi.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index bfb402902b8..f110a8b7658 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -233,7 +233,7 @@ err_ahb:
static void sun4i_spi_set_speed_mode(struct udevice *dev)
{
struct sun4i_spi_priv *priv = dev_get_priv(dev);
- unsigned int div;
+ unsigned int div, div_cdr2;
u32 reg;
/*
@@ -259,15 +259,12 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
*/
div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
+ div_cdr2 = DIV_ROUND_UP(div, 2);
reg = readl(SPI_REG(priv, SPI_CCR));
- if ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
- div /= 2;
- if (div > 0)
- div--;
-
+ if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
- reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
+ reg |= SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
} else {
div = fls(div - 1);
/* The F1C100s encodes the divider as 2^(n+1) */
--
2.39.2
Re: [PATCH 2/2] spi: sunxi: fix clock divider calculation for max frequency setting
> > - if ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> > + if (div != 1 && ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1))) {
> > div /= 2;
>
> This is still not fully correct, is it? If I ask for 10 MHz, the
> algorithm should select 8 MHz (24/3) or actually 6 MHz (24/4), but it
> chooses 12 MHz (24/2), which is too much.
> So I think this division here should be either:
> div = (div + 1) / 2;
> or:
> div = DIV_ROUND_UP(div, 2);
>
> Can someone confirm this?
When I've written this patch, I've looked at how linux does it (and
it's history) and I'm sure you know that linux has two drivers, for
sun4i and sun6i/sun8i. Somehow u-boot conflates them into one with
just one being correct with the SPI_CLK in the CDR2 case (?).
But anyway, this is about CDR1 and it seems you're right. But OTOH
I've tested this briefly with "sf probe 0:0 " and looked at the
SCK frequency of the readid command with a scope and it was always
less than my requested frequency. At least after the second probe
(there must be another bug which will still keep the frequency of
the probe at the former speed). Soo.. I'm not sure. Mh.
While this might be a bug, it doesn't affect this patch which will
just make sure we can get a 1:1 ratio on SoCs where this is
possible, i.e. not on the sun4i variant.
-michael
Re: [PATCH 1/2] spi: sunxi: drop max_hz handling
Hi, > > The driver is trying to read the "spi-max-frequency" property of the > > *controller* driver node. There is no such property. The > > "spi-max-frequency" property belongs to the SPI devices on the bus. > > Ah, indeed, good catch! Many thanks for sending this! > > > Right now, the driver will always fall back to the default value of 1MHz > > and thus flash reads are very slow with just about 215kb/s. > > That's even slower, right? I guess around 125 KB/s? Yes of course :) 1Mhz/8 at most. I was fooled by the "sf update" command which will skip the same sectors and then the overall speed will be faster. > > In fact, the SPI uclass will already take care of everything and we just > > have to clamp the frequency to the values the driver/hardware supports. > > Thus, drop the whole max_hz handling. > > Looks good to me, I verified this by timing the read, this patch indeed > significantly increases the performance. Also changing the limit in the > DT gets reflected in the driver and in the read speed. Also verified > that the values read from the SPI flash are the same in all cases. > > > Signed-off-by: Michael Walle > > Reviewed-by: Andre Przywara > Tested-by: Andre Przywara > > I will make this part of the first 2024.10 PR. This means just 1/2 or both? Because there was no Rb on the second patch. -michael
Re: [PATCH 0/2] spi: sunxi: Improve the loading speed
Right now, the maximal transfer speed from an SPI flash on a V3s is about 240kb/s. That is pretty slow. It turns out, that due to an error u-boot is setting the maximum frequency to 1MHz. By fixing that another bug is unearthed: one cannot set a clock divider of 1:1 due to the handling between CDR1 and CDR2 handling. By fixing that I achieved loading speeds of about 1.5MB/s. Minor nit, should the clock fix go first so there's not a regression if someone needs to do a bisect on the first commit? Sure can do for the next version. -michael
[PATCH 2/2] spi: sunxi: fix clock divider calculation for max frequency setting
If the maximum frequency is requested, we still fall into the CDR2
handling. But there the minimal divider is 2. For the sun6i and sun8i we
can do better with the CDR1 setting where the minimal divider is 1:
SPI_CLK = MOD_CLK / 2 ^ cdr with cdr = 0
Thus, handle the div = 1 case specially.
While at it, correct the comment above the calculation.
Signed-off-by: Michael Walle
---
drivers/spi/spi-sunxi.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index bfb402902b8..3048ab0ecf7 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -249,6 +249,8 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
* We have two choices there. Either we can use the clock
* divide rate 1, which is calculated thanks to this formula:
* SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
+* Or for sun6i/sun8i variants:
+* SPI_CLK = MOD_CLK / (2 ^ cdr)
* Or we can use CDR2, which is calculated with the formula:
* SPI_CLK = MOD_CLK / (2 * (cdr + 1))
* Whether we use the former or the latter is set through the
@@ -256,12 +258,15 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
*
* First try CDR2, and if we can't reach the expected
* frequency, fall back to CDR1.
+* There is one exception if the requested clock is the input
+* clock. In that case we always use CDR1 because we'll get a
+* 1:1: ration for sun6i/sun8i variants.
*/
div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
reg = readl(SPI_REG(priv, SPI_CCR));
- if ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+ if (div != 1 && ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1))) {
div /= 2;
if (div > 0)
div--;
--
2.39.2
[PATCH 1/2] spi: sunxi: drop max_hz handling
The driver is trying to read the "spi-max-frequency" property of the
*controller* driver node. There is no such property. The
"spi-max-frequency" property belongs to the SPI devices on the bus.
Right now, the driver will always fall back to the default value of 1MHz
and thus flash reads are very slow with just about 215kb/s.
In fact, the SPI uclass will already take care of everything and we just
have to clamp the frequency to the values the driver/hardware supports.
Thus, drop the whole max_hz handling.
Signed-off-by: Michael Walle
---
drivers/spi/spi-sunxi.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index 13725ee7a2d..bfb402902b8 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -135,7 +135,6 @@ struct sun4i_spi_variant {
struct sun4i_spi_plat {
struct sun4i_spi_variant *variant;
u32 base;
- u32 max_hz;
};
struct sun4i_spi_priv {
@@ -237,6 +236,13 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
unsigned int div;
u32 reg;
+ /*
+* The uclass should take care that this won't happen. But anyway,
+* avoid a div-by-zero exception.
+*/
+ if (!priv->freq)
+ return;
+
/*
* Setup clock divider.
*
@@ -401,11 +407,10 @@ static int sun4i_spi_xfer(struct udevice *dev, unsigned
int bitlen,
static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
{
- struct sun4i_spi_plat *plat = dev_get_plat(dev);
struct sun4i_spi_priv *priv = dev_get_priv(dev);
- if (speed > plat->max_hz)
- speed = plat->max_hz;
+ if (speed > SUN4I_SPI_MAX_RATE)
+ speed = SUN4I_SPI_MAX_RATE;
if (speed < SUN4I_SPI_MIN_RATE)
speed = SUN4I_SPI_MIN_RATE;
@@ -458,7 +463,6 @@ static int sun4i_spi_probe(struct udevice *bus)
priv->variant = plat->variant;
priv->base = plat->base;
- priv->freq = plat->max_hz;
return 0;
}
@@ -466,16 +470,9 @@ static int sun4i_spi_probe(struct udevice *bus)
static int sun4i_spi_of_to_plat(struct udevice *bus)
{
struct sun4i_spi_plat *plat = dev_get_plat(bus);
- int node = dev_of_offset(bus);
plat->base = dev_read_addr(bus);
plat->variant = (struct sun4i_spi_variant *)dev_get_driver_data(bus);
- plat->max_hz = fdtdec_get_int(gd->fdt_blob, node,
- "spi-max-frequency",
- SUN4I_SPI_DEFAULT_RATE);
-
- if (plat->max_hz > SUN4I_SPI_MAX_RATE)
- plat->max_hz = SUN4I_SPI_MAX_RATE;
return 0;
}
--
2.39.2
[PATCH 0/2] spi: sunxi: Improve the loading speed
Right now, the maximal transfer speed from an SPI flash on a V3s is about 240kb/s. That is pretty slow. It turns out, that due to an error u-boot is setting the maximum frequency to 1MHz. By fixing that another bug is unearthed: one cannot set a clock divider of 1:1 due to the handling between CDR1 and CDR2 handling. By fixing that I achieved loading speeds of about 1.5MB/s. Michael Walle (2): spi: sunxi: drop max_hz handling spi: sunxi: fix clock divider calculation for max frequency setting drivers/spi/spi-sunxi.c | 28 +++- 1 file changed, 15 insertions(+), 13 deletions(-) -- 2.39.2
[PATCH] sunxi: board: probe USB gadget
Due to the lazy probing, the gadget driver might not be probed when the
u-boot cli is active. In this case the "ums" command won't work, for
example. If enabled, probe the USB gadget during board_init().
Signed-off-by: Michael Walle
---
board/sunxi/board.c | 4
1 file changed, 4 insertions(+)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index ed86f1df5dc..7f93aba9ee7 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -189,6 +189,7 @@ enum env_location env_get_location(enum env_operation op,
int prio)
int board_init(void)
{
__maybe_unused int id_pfr1, ret;
+ struct udevice *dev;
gd->bd->bi_boot_params = (PHYS_SDRAM_0 + 0x100);
@@ -225,6 +226,9 @@ int board_init(void)
if (ret)
return ret;
+ if (CONFIG_IS_ENABLED(USB_GADGET))
+ uclass_get_device(UCLASS_USB_GADGET_GENERIC, 0, &dev);
+
eth_init_board();
return 0;
--
2.39.2
Re: [PATCH] sunxi: SPL SPI: add support for the V3s SoC
On Tue May 14, 2024 at 1:43 AM CEST, Michael Walle wrote: > The V3s is identical regarding register layout, clocks and resets to > the sun6i variants. Therefore, we can just add the MACH_SUN8I_V3S to > the sun6i compatible ones. > > SPI boot was tested on a custom board with a Gigadevice GD25Q64 8MiB > SPI flash. Also here.. any news? -michael
Re: [PATCH 0/2] sunxi: v3s: add network support
Hi, On Mon May 13, 2024 at 10:56 PM CEST, Michael Walle wrote: > Add network support for the V3s which only supports the internal > PHY. Adding support was straight forward. The emac driver just needs > the compatible string and some platform data and the clock driver > needs to know the bits for the clock gating as well as the reset > bits. > > This was tested on a custom board. Any news here? -michael > > Michael Walle (2): > clk: sunxi: add EMAC and EPHY clocks and resets for the V3s SoC > net: sun8i_emac: add support for the V3s > > drivers/clk/sunxi/clk_v3s.c | 6 ++ > drivers/net/sun8i_emac.c| 7 +++ > 2 files changed, 13 insertions(+)
Re: obscure microsd detection issue between U-Boot and kernel
On Tue Jun 4, 2024 at 9:47 AM CEST, Christian Loehle wrote: > On 6/3/24 22:28, Tim Harvey wrote: > > On Mon, Jun 3, 2024 at 1:18 AM Christian Loehle > > wrote: > >> > >> On 5/31/24 21:47, Tim Harvey wrote: > >>> Greetings, > >>> > >>> I'm seeing an issue on an imx8mm board (imx8mm-venice-gw73xx) where > >>> for a specific set of microsd cards if I have accessed the microsd in > >>> U-Boot with UHS/1.8V the kernel will not recognize that microsd when > >>> scanning. > >>> > >>> The issue does not occur with all microsd cards but seems to appear > >>> with a large sample size of a specific card/model (Kingston SDC32 32GB > >>> SDR104 card). I do not see a signal integrity issue on the scope. > >>> > >>> Instrumenting the kernel the issue is that the host reports a CRC > >>> error as soon as the first mmc_send_if_cond call which occurs in > >>> mmc_rescan_try_freq. > >>> > >>> I can avoid the issue by either not accessing the microsd in U-Boot or > >>> by disabling UHS/1.8V mode in U-Boot therefore what I think is > >>> happening is that U-Boot leaves the card in UHS/1.8V signalling mode > >>> and when the kernel scans it sets the voltage back to 3.3V > >>> standard/default and default timings then issues its clock cycles to > >>> 'reset' the card and the card does not recognize the reset. I'm > >>> wondering if this is because the reset is done via clock cycles after > >>> the kernel has set the I/O voltage back to 3.3V when perhaps the card > >>> is still in 1.8V mode (although I don't see how that would cause an > >>> issue)? > >> > >> It will cause an issue for many cards and might break some cards. > >> > >>> > >>> Is there some sort of MMC 'reset' I can/should do in U-Boot before > >>> booting the kernel? Has anyone encountered anything like this before? > >> > >> There is no 'switching back' to 3.3V signalling from UHS 1.8V. > >> The only way this can be done is therefore a full power-off. > >> Is that done correctly for your system? > >> AFAIR spec dictates 500ms of <0.5V on VCC. Note that driving CLK/signal > >> lines can also sustain the card somewhat, as leakage is only limited > >> within operating voltage. > > > > Hi Christian, > > > > Are you saying the only way to properly reset from 1.8V is to have a > > VDD supply on the microSD card that can be turned off before booting > > to Linux? We have never had that before and never encountered > > something like this. > > Yes, the only safe way to use UHS-I really anyway. I can second that. And also keep in mind that the actual supply voltage must be below that threshold. Thus, the power-off time also depends on the capacitance on that supply line after the power load switch. There are switches with dedicated output discharge circuits built-in. That being said, from my experience there are cards which will work when switching back from 1V8 to 3V3 signalling and some don't. I haven't digged deeper into that topic, though. -michael > You could disable UHS for u-boot but that still leaves (potentially) > problematic warm-reboots of the board. > Having a (software-controlled) switchable regulator on SD VCC is pretty > common for this reason and you should be able to find it in most dts > for host controllers with sd-uhs-* property. > I'm afraid that the relevant spec section isn't available in the > simplified version. > > Kind Regards, > Christian signature.asc Description: PGP signature
[PATCH] sunxi: SPL SPI: add support for the V3s SoC
The V3s is identical regarding register layout, clocks and resets to
the sun6i variants. Therefore, we can just add the MACH_SUN8I_V3S to
the sun6i compatible ones.
SPI boot was tested on a custom board with a Gigadevice GD25Q64 8MiB
SPI flash.
Signed-off-by: Michael Walle
---
arch/arm/mach-sunxi/Kconfig | 2 +-
arch/arm/mach-sunxi/spl_spi_sunxi.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index ddf9414b08e..17666814c52 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1078,7 +1078,7 @@ config SPL_STACK_R_ADDR
config SPL_SPI_SUNXI
bool "Support for SPI Flash on Allwinner SoCs in SPL"
- depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5
|| MACH_SUN50I || MACH_SUN8I_R40 || SUN50I_GEN_H6 || MACH_SUNIV ||
SUNXI_GEN_NCAT2
+ depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5
|| MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN8I_V3S || SUN50I_GEN_H6 ||
MACH_SUNIV || SUNXI_GEN_NCAT2
help
Enable support for SPI Flash. This option allows SPL to read from
sunxi SPI Flash. It uses the same method as the boot ROM, so does
diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index 7acb44f52ae..d7abdc2e401 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -140,7 +140,8 @@ static bool is_sun6i_gen_spi(void)
{
return IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I) ||
IS_ENABLED(CONFIG_SUN50I_GEN_H6) ||
- IS_ENABLED(CONFIG_SUNXI_GEN_NCAT2);
+ IS_ENABLED(CONFIG_SUNXI_GEN_NCAT2) ||
+ IS_ENABLED(CONFIG_MACH_SUN8I_V3S);
}
static uintptr_t spi0_base_address(void)
--
2.39.2
[PATCH 2/2] net: sun8i_emac: add support for the V3s
Add the compatible string for the emac found on the V3s SoC. The SoC
only supports the internal PHY. There are no (R)MII signals on any pins.
Signed-off-by: Michael Walle
---
drivers/net/sun8i_emac.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 8bff4fe9a9e..94bcd40acb8 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -893,6 +893,11 @@ static const struct emac_variant emac_variant_r40 = {
.syscon_offset = 0x164,
};
+static const struct emac_variant emac_variant_v3s = {
+ .syscon_offset = 0x30,
+ .soc_has_internal_phy = true,
+};
+
static const struct emac_variant emac_variant_a64 = {
.syscon_offset = 0x30,
.support_rmii = true,
@@ -910,6 +915,8 @@ static const struct udevice_id sun8i_emac_eth_ids[] = {
.data = (ulong)&emac_variant_h3 },
{ .compatible = "allwinner,sun8i-r40-gmac",
.data = (ulong)&emac_variant_r40 },
+ { .compatible = "allwinner,sun8i-v3s-emac",
+ .data = (ulong)&emac_variant_v3s },
{ .compatible = "allwinner,sun50i-a64-emac",
.data = (ulong)&emac_variant_a64 },
{ .compatible = "allwinner,sun50i-h6-emac",
--
2.39.2
[PATCH 1/2] clk: sunxi: add EMAC and EPHY clocks and resets for the V3s SoC
Add the clock gate registers as well as the reset register bits for the
EMAC and EPHY for the V3s. These are needed by the sun8i network driver.
Signed-off-by: Michael Walle
---
drivers/clk/sunxi/clk_v3s.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c
index 6524c13540e..0402d5ed190 100644
--- a/drivers/clk/sunxi/clk_v3s.c
+++ b/drivers/clk/sunxi/clk_v3s.c
@@ -17,6 +17,7 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_BUS_MMC0] = GATE(0x060, BIT(8)),
[CLK_BUS_MMC1] = GATE(0x060, BIT(9)),
[CLK_BUS_MMC2] = GATE(0x060, BIT(10)),
+ [CLK_BUS_EMAC] = GATE(0x060, BIT(17)),
[CLK_BUS_SPI0] = GATE(0x060, BIT(20)),
[CLK_BUS_OTG] = GATE(0x060, BIT(24)),
@@ -31,6 +32,8 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_BUS_UART1] = GATE(0x06c, BIT(17)),
[CLK_BUS_UART2] = GATE(0x06c, BIT(18)),
+ [CLK_BUS_EPHY] = GATE(0x070, BIT(0)),
+
[CLK_SPI0] = GATE(0x0a0, BIT(31)),
[CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),
@@ -45,12 +48,15 @@ static struct ccu_reset v3s_resets[] = {
[RST_BUS_MMC0] = RESET(0x2c0, BIT(8)),
[RST_BUS_MMC1] = RESET(0x2c0, BIT(9)),
[RST_BUS_MMC2] = RESET(0x2c0, BIT(10)),
+ [RST_BUS_EMAC] = RESET(0x2c0, BIT(17)),
[RST_BUS_SPI0] = RESET(0x2c0, BIT(20)),
[RST_BUS_OTG] = RESET(0x2c0, BIT(24)),
[RST_BUS_TCON0] = RESET(0x2c4, BIT(4)),
[RST_BUS_DE]= RESET(0x2c4, BIT(12)),
+ [RST_BUS_EPHY] = RESET(0x2c8, BIT(2)),
+
[RST_BUS_I2C0] = RESET(0x2d8, BIT(0)),
[RST_BUS_I2C1] = RESET(0x2d8, BIT(1)),
[RST_BUS_UART0] = RESET(0x2d8, BIT(16)),
--
2.39.2
[PATCH 0/2] sunxi: v3s: add network support
Add network support for the V3s which only supports the internal PHY. Adding support was straight forward. The emac driver just needs the compatible string and some platform data and the clock driver needs to know the bits for the clock gating as well as the reset bits. This was tested on a custom board. Michael Walle (2): clk: sunxi: add EMAC and EPHY clocks and resets for the V3s SoC net: sun8i_emac: add support for the V3s drivers/clk/sunxi/clk_v3s.c | 6 ++ drivers/net/sun8i_emac.c| 7 +++ 2 files changed, 13 insertions(+) -- 2.39.2
Re: [PATCH 032/149] board: buffalo: Remove and add needed includes
On Wed May 1, 2024 at 4:41 AM CEST, Tom Rini wrote: > Remove from this board vendor directory and when needed > add missing include files directly. > > Signed-off-by: Tom Rini Acked-by: Michael Walle
Re: [PATCH 080/149] board: kontron: Remove and add needed includes
On Wed May 1, 2024 at 4:42 AM CEST, Tom Rini wrote: > Remove from this board vendor directory and when needed > add missing include files directly. > > Signed-off-by: Tom Rini Acked-by: Michael Walle
Re: Capsule GUIDs and LVFS
Hi, On Thu Apr 25, 2024 at 8:19 AM CEST, Ilias Apalodimas wrote: > I've cc'ed all the people I could find in board specific MAINTAINER files. > Can you respond to Richard with the proper company name & board name > so we can bind the following GUIDs to a vendor properly? > Richard any guidance on how this should be done properly is > appreciated, since I am not too aware of LVFS internals. Thanks for taking care! > Kontron KONTRON_PITX_IMX8M_FIT_IMAGE_GUID c898e959-5b1f-4e6d-88e0-40d45cca1399 This (board) should likely be dropped to lower maintenance burden as it is EOL and "not recommended for new designs". > Kontron KONTRON_SL_MX8MM_FIT_IMAGE_GUID d488e45a-4929-4b55-8c14-86cea2cd6629 This is probably (at least) the wrong name. SL is a System-on-Module, a small PCB that will be soldered on a carrier board and cannot run on it's own and a customer using that SoM will likely have their own bootloader. There is, though the "BL" which is the in-house board for this SoM. Company name is "Kontron Electronics GmbH". > Kontron KONTRON_SL28_FIT_IMAGE_GUID 86ebd44f-feb8-466f-8bb8-890618456d8b Company name is "Kontron Europe GmbH". Both entities belong to the Kontron Group, not sure how this is handled in LVFS though. -michael signature.asc Description: PGP signature
Re: [PATCH] board: sl28: move to OF_UPSTREAM
On Wed Mar 6, 2024 at 5:19 PM CET, Michael Walle wrote: > Use the new device devicetree files in dts/upstream/ and delete the old > ones. Still keep the -u-boot.dtsi with all u-boot specifics around. > > There is one catch and that is fsl-ls1028a-kontron-sl28-var3.dts which > is not available upstream (yet!). For now, the base dts is used for this > variant as this only differ in the compatible and the (human readable) > model name. > > Signed-off-by: Michael Walle Ping. Any news here? -michael signature.asc Description: PGP signature
Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries
Hi, On Fri Apr 12, 2024 at 5:03 AM CEST, Neha Malcom Francis wrote: > On 05/04/24 13:12, Michael Walle wrote: > > On Thu Apr 4, 2024 at 11:10 AM CEST, Neha Malcom Francis wrote: > >> But again in the interest of time... this would mean this cleaning up > >> effort be > >> kept on hold. If we can agree to move to using the generator later as the > >> final > >> solution, can we pick up this series for now? > > > > Agreed. I just saw the new RFC for the j722s support. It should also > > make use of this cleanup then, btw. > > > > Right, I'll sync with J722S efforts as well. > > So is this series good to go? If the ultimate goal is to support the generator, sure. -michael
Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries
Hi, On Thu Apr 4, 2024 at 11:10 AM CEST, Neha Malcom Francis wrote: > But again in the interest of time... this would mean this cleaning up effort > be > kept on hold. If we can agree to move to using the generator later as the > final > solution, can we pick up this series for now? Agreed. I just saw the new RFC for the j722s support. It should also make use of this cleanup then, btw. -michael
Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries
Hi,
> > > > > And on top of that, it will just be a base board and there will
> > > > > likely be some carrier device trees (overlay? I'm not sure yet).
> > > > >
> > > > > As far as I can tell, you've put the memory configuration into the
> > > > > device tree, so I'll probably need to switch between them somehow.
> > > >
> > > > The "k3--ddr.dtsi" file will be present in your k3-r5.dts
> > > > which makes sense, the memory configuration depends on the board.
> > >
> >
> > k3--ddr.dtsi* (e.g J721E EVM vs. SK boards consume different memory
> > configurations.
Right.
> > > And one board might have multiple configuration depending on the
> > > variant of the board. Typically, one board is available with
> > > different memory options. i.e. 1GiB, 4GiB and so on. The actual RAM
> > > chips can come from different manufacturers. So all all, I presume
> > > there will be different RAM settings, i.e. different
> > > k3--ddr.dtsi. But I have to switch between the setting during
> > > runtime because there will be only one boot image for that board.
> >
> > This is a runtime dynamic DDR configuration support you are describing
> > correct? This means you would be including all the supported memory option
> > DTSIs in your k3--r5.dts correct and probably do some board magic
> > code in the SPL DDR driver to choose the DTB. How is this affecting the
> > packing of the final bootloader which will anyways pack the whole R5 DTB?
Correct, the DDR configuration should be chosen at runtime after
reading some board strappings. Unless, it will work with with the
same configuration which seems unlikely to me. But it is not an
unusual configuration I'd say.
I haven't looked into this in detail, but to me it seems not that
obvious how to do that in a generic/upstreamable way. Multiple
device nodes sounds wrong. Thus, I'd likely need different device
trees for the different memory configurations for the R5 SPL. Not
sure that is yet possible with u-boot, though. If you have any
better idea, I'm all ears.
> > > > > Also, regarding the board variants, I'll probably need to choose
> > > > > between multiple device trees. That is invisible to the user,
> > > > > because u-boot will choose the correct DTB according a board
> > > > > strapping, which btw. works really fine, see for example
> > > > > (boards/kontron/sl28/spl.c:board_fit_config_name_match).
> > > >
> > > > Again, this is assuming that there is some HW blown register available
> > > > for the board to use (or in our earlier K3 case, the EEPROM) but that is
> > > > not necessarily true every time.
> > >
> > > No, that is of course board dependent. It is just an example that
> > > there are boards with more than one DTB.
> > >
> > > Let's step back a bit. Right now, there is
> > >k3---binman.dtsi
> > > which is fine. But it seems, that TI is heading towards a common
> > >k3--binman.dtsi
> > > which is intended to be used by all the boards that are using that
> > > particular SoC, correct me if I'm wrong here. Now the problem with
> > > that is that you hardcode the FIT configuations which are really
> > > board dependent and assume that there will be exactly one DTB per
> > > board, i.e. your "#define SPL_BOARD_DTB" etc.
> > >
> >
> > Correct, but as I mentioned in the earlier message, if your board supports
> > more than 1 FIT configuration, you can easily extend the image and add more
> > configurations.
> >
> > > Thus, what I was trying to say is that you should split all the
> > > board independent configuration (dt fragments) from the board
> > > specific configuration.
> > >
> > > And again, of course I could just ignore the k3--binman.dtsi
> > > and just use a suitable copy "k3---binman.dtsi" for my
> > > board. But as I said, I'm not sure, this is the way to go and I have
> > > a slight feeling I will be asked to reuse the "k3--binman.dtsi"
> > > when I submit my board support.
> > >
> > > > >
> > > > > I don't think it makes much sense to hardcode your generic
> > > > > *-binman.dtsi to just one FIT configuration. I'd rather see a split
> > > > > between generic things which are shared across all boards and board
> > > > > specifics, like the FIT configuration. I mean I could just copy all
> > > >
> > > > Correct me if I'm wrong, but my understanding is that you would want to
> > > > add more FDT blobs in the *-binman.dtsi correct? That is still possible,
> > > > adding another "fdt-1" and "conf-1" in the
> > > >
> > > > Something like this in your -u-boot.dtsi,
> > > >
> > > > tispl {
> > > > insert-template = <&ti_spl>;
> > > > fit {
> > > > images {
> > > > fdt-1 {
> > > > ...
> > > > };
> > > > };
> > > > configurations {
> > > > conf-1 {
> > > > ...
> > > > };
> > > > };
> > > > };
> >
[PATCH] net: ti: am65-cpsw: Fix buffer overflow
The device name is a concatenation of the device node name of the cpsw
device and of the device node name of the port. In my case that is
ethernet@800
port@1
First the buffer is really too small, but more importantly, there is no
boundary check. Use snprintf() and increase the buffer size.
Fixes: 38922b1f4acc ("net: ti: am65-cpsw: Add support for multi port
independent MAC mode")
Signed-off-by: Michael Walle
---
drivers/net/ti/am65-cpsw-nuss.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
index d68ed671836..b151e25d6a4 100644
--- a/drivers/net/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ti/am65-cpsw-nuss.c
@@ -664,7 +664,7 @@ static int am65_cpsw_port_probe(struct udevice *dev)
struct am65_cpsw_priv *priv = dev_get_priv(dev);
struct eth_pdata *pdata = dev_get_plat(dev);
struct am65_cpsw_common *cpsw_common;
- char portname[15];
+ char portname[32];
int ret;
priv->dev = dev;
@@ -672,7 +672,7 @@ static int am65_cpsw_port_probe(struct udevice *dev)
cpsw_common = dev_get_priv(dev->parent);
priv->cpsw_common = cpsw_common;
- sprintf(portname, "%s%s", dev->parent->name, dev->name);
+ snprintf(portname, sizeof(portname), "%s%s", dev->parent->name,
dev->name);
device_set_name(dev, portname);
ret = am65_cpsw_ofdata_parse_phy(dev);
--
2.39.2
Re: [PATCH] net: phy: broadcom: Configure LEDs on BCM54210E
On Thu Mar 28, 2024 at 4:09 PM CET, Tom Rini wrote: > On Mon, Jan 01, 2024 at 10:07:47PM +0100, Marek Vasut wrote: > > > Configure LEDs on BCM54210E so they would blink on activity > > and indicate link speed. Without this the LEDs are always on > > if cable is plugged in. > > > > Signed-off-by: Marek Vasut > > Applied to u-boot/next, thanks! Pretty late and I'm not implying this should be reverted. I just want to point out, that this is really board dependent and might even break boards which have this PHY and are using its default configuration for the attached LEDs. FWIW, linux now have LED PHY DT bindings. -michael signature.asc Description: PGP signature
Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries
Hi, On Thu Mar 28, 2024 at 12:18 PM CET, Neha Malcom Francis wrote: > On 27-Mar-24 8:03 PM, Michael Walle wrote: > > On Wed Mar 27, 2024 at 8:01 AM CET, Neha Malcom Francis wrote: > >> On 26/03/24 19:18, Michael Walle wrote: > >>> On Fri Mar 22, 2024 at 2:10 PM CET, Neha Malcom Francis wrote: > >>>> Clean up templatized boot binaries for all K3 boards. This includes > >>>> modifying the k3-binman.dtsi to use SPL_BOARD_DTB, BOARD_DESCRIPTION and > >>>> UBOOT_BOARD_DESCRIPTION from the files that include it to further reuse > >>>> code. > >>>> > >>>> All k3--binman.dtsi will contain only templates. Only required boot > >>>> binaries can be built from the templates in the boards' respective > >>>> -u-boot.dtsi file (or k3--binman.dtsi if it exists). This allows > >>>> clear distinction between the SoC common stuff vs. what is additionally > >>>> needed to boot up a specific board. > >>> > >>> I appreciate the cleanup. But as far as I can see, a board might > >>> only have one device tree. How would that work if the uboot proper > >>> must support multiple device trees? > >>> > >> > >> From the discussions that took place in the mailing list [1] the > >> consensus > >> seems to be to not focus on multiple devicetree support as it leads to > >> confusion > >> for downstream users. > > > > What are users in this regard? I don't think you'd confuse > > developers. > > > > Anyway, I'm planning on upstreaming a TI board which will have > > different memory configurations and different variants of the board. > > I am assuming you are reusing an existing TI SoC? Not really yet. It's the j722s. > > And on top of that, it will just be a base board and there will > > likely be some carrier device trees (overlay? I'm not sure yet). > > > > As far as I can tell, you've put the memory configuration into the > > device tree, so I'll probably need to switch between them somehow. > > The "k3--ddr.dtsi" file will be present in your k3-r5.dts > which makes sense, the memory configuration depends on the board. And one board might have multiple configuration depending on the variant of the board. Typically, one board is available with different memory options. i.e. 1GiB, 4GiB and so on. The actual RAM chips can come from different manufacturers. So all all, I presume there will be different RAM settings, i.e. different k3--ddr.dtsi. But I have to switch between the setting during runtime because there will be only one boot image for that board. > > Also, regarding the board variants, I'll probably need to choose > > between multiple device trees. That is invisible to the user, > > because u-boot will choose the correct DTB according a board > > strapping, which btw. works really fine, see for example > > (boards/kontron/sl28/spl.c:board_fit_config_name_match). > > Again, this is assuming that there is some HW blown register available > for the board to use (or in our earlier K3 case, the EEPROM) but that is > not necessarily true every time. No, that is of course board dependent. It is just an example that there are boards with more than one DTB. Let's step back a bit. Right now, there is k3---binman.dtsi which is fine. But it seems, that TI is heading towards a common k3--binman.dtsi which is intended to be used by all the boards that are using that particular SoC, correct me if I'm wrong here. Now the problem with that is that you hardcode the FIT configuations which are really board dependent and assume that there will be exactly one DTB per board, i.e. your "#define SPL_BOARD_DTB" etc. Thus, what I was trying to say is that you should split all the board independent configuration (dt fragments) from the board specific configuration. And again, of course I could just ignore the k3--binman.dtsi and just use a suitable copy "k3---binman.dtsi" for my board. But as I said, I'm not sure, this is the way to go and I have a slight feeling I will be asked to reuse the "k3--binman.dtsi" when I submit my board support. > > > > I don't think it makes much sense to hardcode your generic > > *-binman.dtsi to just one FIT configuration. I'd rather see a split > > between generic things which are shared across all boards and board > > specifics, like the FIT configuration. I mean I could just copy all > > Correct me if I'm wrong, but my understanding is that you would want to > add more FDT blobs in the *-binman.dtsi correct? That is still possible, &g
Re: [PATCH v2] arm: dts: kirkwood: Enable upstream DT on Kirkwood boards
Hi, On Thu Mar 28, 2024 at 3:18 AM CET, Tony Dinh wrote: > Enable OF_UPSTREAM to use upstream DT and add marvell/ prefix to the > DEFAULT_DEVICE_TREE for Kirkwood boards. And so we can directly build > DTBs from dts/upstream/src/arm/marvell, and including *-u-boot.dtsi > files from arch/arm/dts/ directory. > > Background: > > Hi Stefan, > Hi Michael, > > I did a survey and we currently have 28 Kirkwood boards. Using some > commands and filters, here are the finding. > > git grep -li arch_kirkwood configs | xargs grep DEVICE_TREE | cut -d '"' -f2 > | xargs -n1 sh -c 'diff -qs arch/arm/dts/$1.dts > dts/upstream/src/arm/marvell/$1.dts' sh | grep differ > > diff: dts/upstream/src/arm/marvell/kirkwood-atl-sbx81lifkw.dts: No such file > or directory > diff: dts/upstream/src/arm/marvell/kirkwood-atl-sbx81lifxcat.dts: No such > file or directory ... Are you sure you want to have all this text in the commit log? You seem to have forgotten my tag: Tested-by: Michael Walle # on lschv2
Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries
Hi, On Wed Mar 27, 2024 at 8:01 AM CET, Neha Malcom Francis wrote: > On 26/03/24 19:18, Michael Walle wrote: > > On Fri Mar 22, 2024 at 2:10 PM CET, Neha Malcom Francis wrote: > >> Clean up templatized boot binaries for all K3 boards. This includes > >> modifying the k3-binman.dtsi to use SPL_BOARD_DTB, BOARD_DESCRIPTION and > >> UBOOT_BOARD_DESCRIPTION from the files that include it to further reuse > >> code. > >> > >> All k3--binman.dtsi will contain only templates. Only required boot > >> binaries can be built from the templates in the boards' respective > >> -u-boot.dtsi file (or k3--binman.dtsi if it exists). This allows > >> clear distinction between the SoC common stuff vs. what is additionally > >> needed to boot up a specific board. > > > > I appreciate the cleanup. But as far as I can see, a board might > > only have one device tree. How would that work if the uboot proper > > must support multiple device trees? > > > > From the discussions that took place in the mailing list [1] the consensus > seems to be to not focus on multiple devicetree support as it leads to > confusion > for downstream users. What are users in this regard? I don't think you'd confuse developers. Anyway, I'm planning on upstreaming a TI board which will have different memory configurations and different variants of the board. And on top of that, it will just be a base board and there will likely be some carrier device trees (overlay? I'm not sure yet). As far as I can tell, you've put the memory configuration into the device tree, so I'll probably need to switch between them somehow. Also, regarding the board variants, I'll probably need to choose between multiple device trees. That is invisible to the user, because u-boot will choose the correct DTB according a board strapping, which btw. works really fine, see for example (boards/kontron/sl28/spl.c:board_fit_config_name_match). I don't think it makes much sense to hardcode your generic *-binman.dtsi to just one FIT configuration. I'd rather see a split between generic things which are shared across all boards and board specifics, like the FIT configuration. I mean I could just copy all the binman and tiboot3.bin and tispl.bin magic and put it into my own "-u-boot.dtsi". But I'm not sure that will make things any better. -michael
Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries
Hi, On Fri Mar 22, 2024 at 2:10 PM CET, Neha Malcom Francis wrote: > Clean up templatized boot binaries for all K3 boards. This includes > modifying the k3-binman.dtsi to use SPL_BOARD_DTB, BOARD_DESCRIPTION and > UBOOT_BOARD_DESCRIPTION from the files that include it to further reuse > code. > > All k3--binman.dtsi will contain only templates. Only required boot > binaries can be built from the templates in the boards' respective > -u-boot.dtsi file (or k3--binman.dtsi if it exists). This allows > clear distinction between the SoC common stuff vs. what is additionally > needed to boot up a specific board. I appreciate the cleanup. But as far as I can see, a board might only have one device tree. How would that work if the uboot proper must support multiple device trees? Thanks, -michael signature.asc Description: PGP signature
[PATCH] tools: binman: ti_board_cfg: improve error message
When there is a lint error the user gets the following cryptic message:
binman: Node '/path/to/some/node': Yamllint error: 18: comments
This isn't very helpful. Improve the message to tell the user that the
number is actually a line number and also tell the user in which file
they have to look.
Signed-off-by: Michael Walle
---
tools/binman/etype/ti_board_config.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/ti_board_config.py
b/tools/binman/etype/ti_board_config.py
index 2c3bb8f7b56..c10d66edcb1 100644
--- a/tools/binman/etype/ti_board_config.py
+++ b/tools/binman/etype/ti_board_config.py
@@ -248,7 +248,7 @@ class Entry_ti_board_config(Entry_section):
yaml_config = config.YamlLintConfig("extends: default")
for p in yamllint.linter.run(open(self._config_file, "r"),
yaml_config):
-self.Raise(f"Yamllint error: {p.line}: {p.rule}")
+self.Raise(f"Yamllint error: Line {p.line} in
{self._config_file}: {p.rule}")
try:
validate(self.file_yaml, self.schema_yaml)
except Exception as e:
--
2.39.2
Re: [PATCH] arm: dts: kirkwood: Enable upstream DT on Kirkwood boards
Hi Tony, On Fri Mar 22, 2024 at 3:17 AM CET, Tony Dinh wrote: > Enable OF_UPSTREAM to use upstream DT and add marvell/ prefix to the > DEFAULT_DEVICE_TREE for Kirkwood boards. And so we can directly build > DTBs from dts/upstream/src/arm/marvell, and including *-u-boot.dtsi > files from arch/arm/dts/ directory. Thanks for taking care. > Signed-off-by: Tony Dinh Tested-by: Michael Walle # on lschlv2 lsxl should work too as it is just different in memory and cpu frequency settings. > --- > > arch/arm/dts/kirkwood-lschlv2-u-boot.dtsi | 6 -- > arch/arm/dts/kirkwood-lsxhl-u-boot.dtsi | 6 -- arch/arm/dts/kirkwood-lschlv2.dts arch/arm/dts/kirkwood-lsxhl.dts Should be then be removed. -michael
[PATCH] board: sl28: move to OF_UPSTREAM
Use the new device devicetree files in dts/upstream/ and delete the old
ones. Still keep the -u-boot.dtsi with all u-boot specifics around.
There is one catch and that is fsl-ls1028a-kontron-sl28-var3.dts which
is not available upstream (yet!). For now, the base dts is used for this
variant as this only differ in the compatible and the (human readable)
model name.
Signed-off-by: Michael Walle
---
I'll send a patch to linux to add the var3.dts, then I'll add the
correct var3 dts again.
---
.../arm/dts/fsl-ls1028a-kontron-sl28-var1.dts | 59
.../arm/dts/fsl-ls1028a-kontron-sl28-var2.dts | 65
.../arm/dts/fsl-ls1028a-kontron-sl28-var3.dts | 15 -
.../arm/dts/fsl-ls1028a-kontron-sl28-var4.dts | 47 ---
arch/arm/dts/fsl-ls1028a-kontron-sl28.dts | 308 --
board/kontron/sl28/spl.c | 11 +-
configs/kontron_sl28_defconfig| 5 +-
7 files changed, 8 insertions(+), 502 deletions(-)
delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts
delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts
delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var4.dts
delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28.dts
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts
b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts
deleted file mode 100644
index 7cd29ab970..00
--- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts
+++ /dev/null
@@ -1,59 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Device Tree file for the Kontron SMARC-sAL28 board.
- *
- * This is for the network variant 1 which has one ethernet port. It is
- * different than the base variant, which also has one port, but here the
- * port is connected via RGMII. This port is not TSN aware.
- * None of the four SerDes lanes are used by the module, instead they are
- * all led out to the carrier for customer use.
- *
- * Copyright (C) 2021 Michael Walle
- *
- */
-
-/dts-v1/;
-#include "fsl-ls1028a-kontron-sl28.dts"
-#include
-
-/ {
- model = "Kontron SMARC-sAL28 (4 Lanes)";
- compatible = "kontron,sl28-var1", "kontron,sl28", "fsl,ls1028a";
-};
-
-&enetc_mdio_pf3 {
- /* Delete unused phy node */
- /delete-node/ ethernet-phy@5;
-
- phy0: ethernet-phy@4 {
- reg = <0x4>;
- eee-broken-1000t;
- eee-broken-100tx;
- qca,clk-out-frequency = <12500>;
- qca,clk-out-strength = ;
- qca,keep-pll-enabled;
- vddio-supply = <&vddio>;
-
- vddio: vddio-regulator {
- regulator-name = "VDDIO";
- regulator-min-microvolt = <180>;
- regulator-max-microvolt = <180>;
- };
-
- vddh: vddh-regulator {
- regulator-name = "VDDH";
- };
- };
-};
-
-&enetc_port0 {
- status = "disabled";
- /* Delete the phy-handle to the old phy0 label */
- /delete-property/ phy-handle;
-};
-
-&enetc_port1 {
- phy-handle = <&phy0>;
- phy-mode = "rgmii-id";
- status = "okay";
-};
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts
b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts
deleted file mode 100644
index 330e34f933..00
--- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts
+++ /dev/null
@@ -1,65 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Device Tree file for the Kontron SMARC-sAL28 board.
- *
- * This is for the network variant 2 which has two ethernet ports. These
- * ports are connected to the internal switch.
- *
- * Copyright (C) 2021 Michael Walle
- *
- */
-
-/dts-v1/;
-#include "fsl-ls1028a-kontron-sl28.dts"
-
-/ {
- model = "Kontron SMARC-sAL28 (TSN-on-module)";
- compatible = "kontron,sl28-var2", "kontron,sl28", "fsl,ls1028a";
-};
-
-&enetc_mdio_pf3 {
- phy1: ethernet-phy@4 {
- reg = <0x4>;
- eee-broken-1000t;
- eee-broken-100tx;
- };
-};
-
-&enetc_port0 {
- status = "disabled";
- /*
-* In the base device tree the PHY at address 5 was assigned for
-* this port. On this module this PHY is connected to a switch
-* port instead. Therefore, delete the phy-handle property here.
-*/
- /delete-property/ phy-handle;
-};
-
-&enetc_port2 {
- status = "okay";
-};
-
-&mscc_felix {
- status = "okay";
-};
-
-&mscc_felix_port0 {
- label = "swp0";
- managed = "in-band-status";
- phy-handle = <&phy0>;
- phy-mode = "sgmii";
-
Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot
Hi, On Wed Mar 6, 2024 at 3:56 AM CET, Marek Vasut wrote: > > I'd argue if one wants to use the locking at all, you have to set > > UNLOCK_ALL=n. Otherwise, the bootloader might come alone and just > > clear your locking bits again. Clearing the WPS bit there is just > > one more thing which IMHO doesn't make much sense. > > On the other hand, if UNLOCK_ALL=y is supposed to work and reset > locking, then the SR3 WPS bit has to be configured to select the > standard SPI NOR locking scheme, so the locking can actually be reset > using that scheme. Yes, that's what I've meant with "the unlock all works as advertised" and your patch is fine. -michael signature.asc Description: PGP signature
Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot
On Tue Mar 5, 2024 at 7:54 PM CET, Marek Vasut wrote:
> On 3/5/24 5:55 PM, Michael Walle wrote:
>
> [...]
>
> >>>>>> Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in
> >>>>>> Linux, since Linux that is booted afterward then gets a device that has
> >>>>>> locking scheme configured in a way that Linux expects and can operate.
> >>>>>>
> >>>>>> Better yet, if some old LTS version of the Linux kernel is in use, it
> >>>>>> will also work correctly, because this patch will configure the SPI NOR
> >>>>>> locking scheme to what Linux expects it to be, before the SPI NOR is
> >>>>>> handed over to that old kernel.
> >>>>>
> >>>>> Agreed, but it should *not* be done automatically and nor
> >>>>> unconditionally. Please don't just overwrite any locking bits just
> >>>>> because there is one flash which have it set.
> >>>>
> >>>> The unlock code is not triggered unconditionally, it is protected by
> >>>>
> >>>> if (IS_ENABLED(CONFIG_SPI_FLASH_UNLOCK_ALL)...
> >>>>
> >>>> Kconfig option, so this can be turned on/off in board config already.
> >>>
> >>> Ahh, OK then :)
> >>>
> >>> But keep in mind that enabling this option is foobar and I've gone
> >>> lengths to eliminate it in linux. And actually made that option in
> >>> u-boot.
> >>>
> >>> See linux commit 31ad3eff093c ("mtd: spi-nor: keep lock bits if they
> >>> are non-volatile").
> >>
> >> So, how shall we proceed here?
> >
> > Regarding this patch, I think it's fine. It will unlock the whole
> > flash as advertised.
>
> This change won't unlock the flash, it will switch to the supported
> locking scheme, one which can then be used to unlock the flash.
I can't follow you here. The code is added right below the
write_sr(0), which will clear all the BP bits, thus unlocking
everything if WPS=0. Therefore, no locking will be enabled anymore
afterwards. What did I miss?
> > But u-boot should really consider making that a "default n" option.
> > And most likely adding =y to every existing defconfig out there so
> > that at least new boards will benefit from it.
>
> Yes, I agree with that.
>
> >> The way I see this, if Linux ever implements this scheme, Linux can set
> >> the SR3 WPS bit as needed, it does not matter which way bootloader sets
> >> the bit as the protection bits are not cleared when the bit is cleared,
> >> they seem to be stored elsewhere.
> >
> > On each reboot you'd wear out that cell with two erases/writes.
> > That's another reason why that whole unlocking thing is foobar for
> > non-volatile bits. For me, non-volatile bits are for provisioning,
> > so during a normal boot they should not be touched at all. Just
> > during board manufacturing or because the user actively want to
> > protect something.
>
> That is what happens here, the write to clear the bit is triggered only
> if the bit is set , so OK .
>
> And if Linux wants to use the new locking scheme, then the bootloader
> should ideally be updated to match, i.e. SPI_FLASH_UNLOCK_ALL=n etc, so
> that is also OK .
I'd argue if one wants to use the locking at all, you have to set
UNLOCK_ALL=n. Otherwise, the bootloader might come alone and just
clear your locking bits again. Clearing the WPS bit there is just
one more thing which IMHO doesn't make much sense.
> In other words, there should be no writes into the non-volatile bits,
> unless U-Boot and Linux disagree on the locking scheme,
Agreed.
> in which case
> writes are unavoidable (if you want unlock to work in both projects).
But this should only happen if the user want to change the locking
at all. u-boot should not just do "oh this bit is set, I'm clearing
it" during SPI flash probe. Again, I'm not caring much if this is
guarded by the UNLOCK_ALL=y, because u-boot is already doing "oh the
BP bits are set, lets clear em".
> > If you clear this bit during the unlock all command, all the locking
> > bits are cleared anyway. Or do you mean, the individual bits are
> > still retained?
>
> The lock bits themselves are retained, this SR3 WPS only selects which
> of those bits take effect, whether the SR ones (standard locking scheme)
> or the per-page ones (winbond custom locking scheme).
Ok. So switching back to WPS=1 might come with some suprises :)
-mich
Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot
On Tue Mar 5, 2024 at 5:28 PM CET, Marek Vasut wrote:
> On 3/5/24 4:53 PM, Michael Walle wrote:
> > On Tue Mar 5, 2024 at 4:37 PM CET, Marek Vasut wrote:
> >> On 3/5/24 1:50 PM, Michael Walle wrote:
> >>> On Tue Mar 5, 2024 at 1:31 PM CET, Marek Vasut wrote:
> >>>> On 3/5/24 9:55 AM, Michael Walle wrote:
> >>>>> On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> >>>>>> Some Winbond SPI NORs have special SR3 register which is
> >>>>>> used among other things to control whether non-standard
> >>>>>> "Individual Block/Sector Write Protection" (WPS bit)
> >>>>>> locking scheme is activated. This non-standard locking
> >>>>>> scheme is not supported by either U-Boot or Linux SPI
> >>>>>> NOR stack so make sure it is disabled, otherwise the
> >>>>>> SPI NOR may appear locked for no obvious reason.
> >>>>>
> >>>>> I don't think it is a good idea to just disable the WPS bit.
> >>>>> Usually, that bit is non-volatile and the default is not set.
> >>>>
> >>>> Yes, that's right, the bit is non-volatile and should not be set unless
> >>>> the MTD stack can handle it correctly. Currently, neither U-Boot nor
> >>>> Linux does handle the bit at all, instead both attempt to use the
> >>>> standard SPI NOR locking scheme which is also implemented by this SPI
> >>>> NOR model and they both fail to unlock the SPI NOR that way.
> >>>>
> >>>> Note that the SR3 WPS bit only switches between two different locking
> >>>> schemes (unset = standard SPI NOR locking scheme, set = custom winbond
> >>>> locking scheme), setting SR3 WPS does not immediately imply the SPI NOR
> >>>> is locked, rather the opposite. Out of manufacturing, the SPI NOR is
> >>>> unlocked in either locking scheme. Depending on the SR3 WPS bit state,
> >>>> different commands are used to lock and unlock the SPI NOR.
> >>>>
> >>>> I recently ran across a device which had the SR3 WPS bit incorrectly set
> >>>> out of manufacturing of that device (i.e. before it was populated on a
> >>>> board at board manufacturer) and the device was locked using the custom
> >>>> locking scheme. I was not able to unlock or use that device because both
> >>>> U-Boot and Linux tried to use the standard scheme for that purpose.
> >>>
> >>> Still, I don't think it makes any sense, to disable that bit for all
> >>> winbond flashes just because there was one vendor which set it the
> >>> wrong way - or the board manufacturer didn't test it and programmed
> >>> the flash correctly.
> >>
> >> OK
> >>
> >>>> Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in
> >>>> Linux, since Linux that is booted afterward then gets a device that has
> >>>> locking scheme configured in a way that Linux expects and can operate.
> >>>>
> >>>> Better yet, if some old LTS version of the Linux kernel is in use, it
> >>>> will also work correctly, because this patch will configure the SPI NOR
> >>>> locking scheme to what Linux expects it to be, before the SPI NOR is
> >>>> handed over to that old kernel.
> >>>
> >>> Agreed, but it should *not* be done automatically and nor
> >>> unconditionally. Please don't just overwrite any locking bits just
> >>> because there is one flash which have it set.
> >>
> >> The unlock code is not triggered unconditionally, it is protected by
> >>
> >> if (IS_ENABLED(CONFIG_SPI_FLASH_UNLOCK_ALL)...
> >>
> >> Kconfig option, so this can be turned on/off in board config already.
> >
> > Ahh, OK then :)
> >
> > But keep in mind that enabling this option is foobar and I've gone
> > lengths to eliminate it in linux. And actually made that option in
> > u-boot.
> >
> > See linux commit 31ad3eff093c ("mtd: spi-nor: keep lock bits if they
> > are non-volatile").
>
> So, how shall we proceed here?
Regarding this patch, I think it's fine. It will unlock the whole
flash as advertised.
But u-boot should really consider making that a "default n" option.
And most likely adding =y to every existing defconfig out there so
that at least new boards will benefit from it.
> The way I see this, if Lin
Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot
On Tue Mar 5, 2024 at 4:37 PM CET, Marek Vasut wrote:
> On 3/5/24 1:50 PM, Michael Walle wrote:
> > Hi Marek,
>
> Hi,
>
> > On Tue Mar 5, 2024 at 1:31 PM CET, Marek Vasut wrote:
> >> On 3/5/24 9:55 AM, Michael Walle wrote:
> >>> On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> >>>> Some Winbond SPI NORs have special SR3 register which is
> >>>> used among other things to control whether non-standard
> >>>> "Individual Block/Sector Write Protection" (WPS bit)
> >>>> locking scheme is activated. This non-standard locking
> >>>> scheme is not supported by either U-Boot or Linux SPI
> >>>> NOR stack so make sure it is disabled, otherwise the
> >>>> SPI NOR may appear locked for no obvious reason.
> >>>
> >>> I don't think it is a good idea to just disable the WPS bit.
> >>> Usually, that bit is non-volatile and the default is not set.
> >>
> >> Yes, that's right, the bit is non-volatile and should not be set unless
> >> the MTD stack can handle it correctly. Currently, neither U-Boot nor
> >> Linux does handle the bit at all, instead both attempt to use the
> >> standard SPI NOR locking scheme which is also implemented by this SPI
> >> NOR model and they both fail to unlock the SPI NOR that way.
> >>
> >> Note that the SR3 WPS bit only switches between two different locking
> >> schemes (unset = standard SPI NOR locking scheme, set = custom winbond
> >> locking scheme), setting SR3 WPS does not immediately imply the SPI NOR
> >> is locked, rather the opposite. Out of manufacturing, the SPI NOR is
> >> unlocked in either locking scheme. Depending on the SR3 WPS bit state,
> >> different commands are used to lock and unlock the SPI NOR.
> >>
> >> I recently ran across a device which had the SR3 WPS bit incorrectly set
> >> out of manufacturing of that device (i.e. before it was populated on a
> >> board at board manufacturer) and the device was locked using the custom
> >> locking scheme. I was not able to unlock or use that device because both
> >> U-Boot and Linux tried to use the standard scheme for that purpose.
> >
> > Still, I don't think it makes any sense, to disable that bit for all
> > winbond flashes just because there was one vendor which set it the
> > wrong way - or the board manufacturer didn't test it and programmed
> > the flash correctly.
>
> OK
>
> >> Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in
> >> Linux, since Linux that is booted afterward then gets a device that has
> >> locking scheme configured in a way that Linux expects and can operate.
> >>
> >> Better yet, if some old LTS version of the Linux kernel is in use, it
> >> will also work correctly, because this patch will configure the SPI NOR
> >> locking scheme to what Linux expects it to be, before the SPI NOR is
> >> handed over to that old kernel.
> >
> > Agreed, but it should *not* be done automatically and nor
> > unconditionally. Please don't just overwrite any locking bits just
> > because there is one flash which have it set.
>
> The unlock code is not triggered unconditionally, it is protected by
>
> if (IS_ENABLED(CONFIG_SPI_FLASH_UNLOCK_ALL)...
>
> Kconfig option, so this can be turned on/off in board config already.
Ahh, OK then :)
But keep in mind that enabling this option is foobar and I've gone
lengths to eliminate it in linux. And actually made that option in
u-boot.
See linux commit 31ad3eff093c ("mtd: spi-nor: keep lock bits if they
are non-volatile").
-michael
> > This should be either be a board level option or maybe expose some
> > command line interface to let the user change the settings.
> >
> >>> Thus,
> >>> there is likely someone, probably the manufacturer of the board,
> >>> who intentionally set this bit. Now, with this patch it will get
> >>> disabled *unconditionally*, forever.
> >>
> >> In my case, it is exactly the opposite, the SR3 WPS shouldn't have been
> >> set and the device should not have been locked, but it was and that
> >> confused both U-Boot and Linux.
> >>
> >> I would argue that if the board manufacturer intention was to lock the
> >> SPI NOR, they would have had multiple better options at their disposal,
> >> and those options should have been aligned with the software support:
> >> - nWP pin of the SPI NOR could be tied low to enable WRITE PROTECT
> >> - OTP bits could have been programmed to enable permanent WRITE PROTECT
> >> - standard SPI NOR locking scheme could have been used too
> >>
> >> If they did set SR3 WPS and hoped that U-Boot or Linux would fail to
> >> unlock the SPI NOR using standard locking scheme commands, that is I
> >> think broken design.
> >
> > There might be software/OS which could handle that correctly. Also,
> > if linux will ever learn to use the new locking scheme
> > unconditionally, u-boot will always mess it up then.
>
> See CONFIG_SPI_FLASH_UNLOCK_ALL above.
signature.asc
Description: PGP signature
Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot
Hi Marek, On Tue Mar 5, 2024 at 1:31 PM CET, Marek Vasut wrote: > On 3/5/24 9:55 AM, Michael Walle wrote: > > On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote: > >> Some Winbond SPI NORs have special SR3 register which is > >> used among other things to control whether non-standard > >> "Individual Block/Sector Write Protection" (WPS bit) > >> locking scheme is activated. This non-standard locking > >> scheme is not supported by either U-Boot or Linux SPI > >> NOR stack so make sure it is disabled, otherwise the > >> SPI NOR may appear locked for no obvious reason. > > > > I don't think it is a good idea to just disable the WPS bit. > > Usually, that bit is non-volatile and the default is not set. > > Yes, that's right, the bit is non-volatile and should not be set unless > the MTD stack can handle it correctly. Currently, neither U-Boot nor > Linux does handle the bit at all, instead both attempt to use the > standard SPI NOR locking scheme which is also implemented by this SPI > NOR model and they both fail to unlock the SPI NOR that way. > > Note that the SR3 WPS bit only switches between two different locking > schemes (unset = standard SPI NOR locking scheme, set = custom winbond > locking scheme), setting SR3 WPS does not immediately imply the SPI NOR > is locked, rather the opposite. Out of manufacturing, the SPI NOR is > unlocked in either locking scheme. Depending on the SR3 WPS bit state, > different commands are used to lock and unlock the SPI NOR. > > I recently ran across a device which had the SR3 WPS bit incorrectly set > out of manufacturing of that device (i.e. before it was populated on a > board at board manufacturer) and the device was locked using the custom > locking scheme. I was not able to unlock or use that device because both > U-Boot and Linux tried to use the standard scheme for that purpose. Still, I don't think it makes any sense, to disable that bit for all winbond flashes just because there was one vendor which set it the wrong way - or the board manufacturer didn't test it and programmed the flash correctly. > Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in > Linux, since Linux that is booted afterward then gets a device that has > locking scheme configured in a way that Linux expects and can operate. > > Better yet, if some old LTS version of the Linux kernel is in use, it > will also work correctly, because this patch will configure the SPI NOR > locking scheme to what Linux expects it to be, before the SPI NOR is > handed over to that old kernel. Agreed, but it should *not* be done automatically and nor unconditionally. Please don't just overwrite any locking bits just because there is one flash which have it set. This should be either be a board level option or maybe expose some command line interface to let the user change the settings. > > Thus, > > there is likely someone, probably the manufacturer of the board, > > who intentionally set this bit. Now, with this patch it will get > > disabled *unconditionally*, forever. > > In my case, it is exactly the opposite, the SR3 WPS shouldn't have been > set and the device should not have been locked, but it was and that > confused both U-Boot and Linux. > > I would argue that if the board manufacturer intention was to lock the > SPI NOR, they would have had multiple better options at their disposal, > and those options should have been aligned with the software support: > - nWP pin of the SPI NOR could be tied low to enable WRITE PROTECT > - OTP bits could have been programmed to enable permanent WRITE PROTECT > - standard SPI NOR locking scheme could have been used too > > If they did set SR3 WPS and hoped that U-Boot or Linux would fail to > unlock the SPI NOR using standard locking scheme commands, that is I > think broken design. There might be software/OS which could handle that correctly. Also, if linux will ever learn to use the new locking scheme unconditionally, u-boot will always mess it up then. -michael signature.asc Description: PGP signature
Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot
[+ linux-mtd ]
Hi Marek,
On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> Some Winbond SPI NORs have special SR3 register which is
> used among other things to control whether non-standard
> "Individual Block/Sector Write Protection" (WPS bit)
> locking scheme is activated. This non-standard locking
> scheme is not supported by either U-Boot or Linux SPI
> NOR stack so make sure it is disabled, otherwise the
> SPI NOR may appear locked for no obvious reason.
I don't think it is a good idea to just disable the WPS bit.
Usually, that bit is non-volatile and the default is not set. Thus,
there is likely someone, probably the manufacturer of the board,
who intentionally set this bit. Now, with this patch it will get
disabled *unconditionally*, forever.
-michael
> W25Q16DW, but the W25Q16DW does not implement the SR3 WPS bit.
>
> Signed-off-by: Marek Vasut
> ---
> Cc: Hai Pham
> Cc: Heinrich Schuchardt
> Cc: Jagan Teki
> Cc: Johann Neuhauser
> Cc: Simon Glass
> Cc: Takahiro Kuwano
> Cc: Venkatesh Yadav Abbarapu
> Cc: Vignesh R
> ---
> drivers/mtd/spi/spi-nor-core.c | 47 ++
> include/linux/mtd/spi-nor.h| 5
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 9a1801ba93d..68dee57258d 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -544,6 +544,24 @@ static int read_cr(struct spi_nor *nor)
> }
> #endif
>
> +/**
> + * read_sr3() - Read status register 3 unique to newer Winbond flashes
> + * @nor: pointer to a 'struct spi_nor'
> + */
> +static int read_sr3(struct spi_nor *nor)
> +{
> + int ret;
> + u8 val;
> +
> + ret = nor->read_reg(nor, SPINOR_OP_RDSR3, &val, 1);
> + if (ret < 0) {
> + dev_dbg(nor->dev, "error %d reading SR3\n", ret);
> + return ret;
> + }
> +
> + return val;
> +}
> +
> /*
> * Write status register 1 byte
> * Returns negative if error occurred.
> @@ -554,6 +572,17 @@ static int write_sr(struct spi_nor *nor, u8 val)
> return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 1);
> }
>
> +/**
> + * write_sr3() - Write status register 3 unique to newer Winbond flashes
> + * @nor: pointer to a 'struct spi_nor'
> + * @val: value to be written into SR3
> + */
> +static int write_sr3(struct spi_nor *nor, u8 val)
> +{
> + nor->cmd_buf[0] = val;
> + return nor->write_reg(nor, SPINOR_OP_WRSR3, nor->cmd_buf, 1);
> +}
> +
> /*
> * Set write enable latch with Write Enable command.
> * Returns negative if error occurred.
> @@ -3890,6 +3919,24 @@ static int spi_nor_init(struct spi_nor *nor)
> write_enable(nor);
> write_sr(nor, 0);
> spi_nor_wait_till_ready(nor);
> +
> + /*
> + * Some Winbond SPI NORs have special SR3 register which is
> + * used among other things to control whether non-standard
> + * "Individual Block/Sector Write Protection" (WPS bit)
> + * locking scheme is activated. This non-standard locking
> + * scheme is not supported by either U-Boot or Linux SPI
> + * NOR stack so make sure it is disabled, otherwise the
> + * SPI NOR may appear locked for no obvious reason.
> + */
> + if (JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
> + err = read_sr3(nor);
> + if (err > 0 && err & SR3_WPS) {
> + write_enable(nor);
> + write_sr3(nor, err & ~SR3_WPS);
> + write_disable(nor);
> + }
> + }
> }
>
> if (nor->quad_enable) {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 2861b73edbc..ceb32e3906f 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -45,6 +45,8 @@
> #define SPINOR_OP_WRSR 0x01/* Write status register 1 byte
> */
> #define SPINOR_OP_RDSR2 0x3f/* Read status register 2 */
> #define SPINOR_OP_WRSR2 0x3e/* Write status register 2 */
> +#define SPINOR_OP_RDSR3 0x15/* Read status register 3 */
> +#define SPINOR_OP_WRSR3 0x11/* Write status register 3 */
> #define SPINOR_OP_READ 0x03/* Read data bytes (low
> frequency) */
> #define SPINOR_OP_READ_FAST 0x0b/* Read data bytes (high frequency) */
> #define SPINOR_OP_READ_1_1_2 0x3b/* Read data bytes (Dual Output SPI) */
> @@ -185,6 +187,9 @@
> /* Status Register 2 bits. */
> #define SR2_QUAD_EN_BIT7 BIT(7)
>
> +/* Status Register 3 bits. */
> +#define SR3_WPS BIT(2)
> +
> /* For Cypress flash. */
> #define SPINOR_OP_RD_ANY_REG 0x65/* Read any register */
> #define SPINOR_OP_WR_ANY_REG 0x71/* Write any regi
Re: [PATCH v2] boot: add support for button commands
Using CONFIG_EXTRA_ENV_SETTINGS should be good enough to provide the fallback defaults. However, the users can still mess the things up, but again, they can do that already in many places. I disagree. In my case that is a last resort recovery. And it should work in any case. Even if the user has messed up anything (except from erasing the bootloader in the SPI flash ;)). Maybe the solution could be another compile-time option to "lock down" the built-in defaults provided through CONFIG_EXTRA_ENV_SETTINGS? If that new option is selected, changes to the environment would make no changes to the built-in defaults, i.e. those parts of the environment would actually be ignored. Not sure locking down the whole environment is a good idea. In summary, the registered (compiled-in) command should always take precedence. If one wants to supply a default command which can be changed later, that can go via the (compiled-in) default environment. Sorry, this is a bit confusing to me. Didn't you write above that the users should be able to change the associated commands through the environment variables? I had two kinds of button commands in mind: immutable ones and mutable ones. The first can be achieved with compiled-in commands, the second with a default environment and environment variables. Also, whether a command is a mutable one or not is the decision of the developer (or the one who's compiling/configuring u-boot), not the user. I believe that the additional compile-time option, which I proposed above, could be extended to specify which of the built-in default button-command associations are immutable, and which are allowed to be modified through the environment variables. IIRC there is already a mechanism for that. Environment hooks or something like that. But I'm not sure that has other implications and qualify as simple and lightweight for this use-case. Anyway, we digress. I just wanted to make you aware of another use-case, which btw. is already done today in the lsxl board for example. -michael
Re: [PATCH v2] boot: add support for button commands
Hi, Using CONFIG_EXTRA_ENV_SETTINGS should be good enough to provide the fallback defaults. However, the users can still mess the things up, but again, they can do that already in many places. I disagree. In my case that is a last resort recovery. And it should work in any case. Even if the user has messed up anything (except from erasing the bootloader in the SPI flash ;)). In summary, the registered (compiled-in) command should always take precedence. If one wants to supply a default command which can be changed later, that can go via the (compiled-in) default environment. Sorry, this is a bit confusing to me. Didn't you write above that the users should be able to change the associated commands through the environment variables? I had two kinds of button commands in mind: immutable ones and mutable ones. The first can be achieved with compiled-in commands, the second with a default environment and environment variables. Also, whether a command is a mutable one or not is the decision of the developer (or the one who's compiling/configuring u-boot), not the user. -michael
Re: [PATCH v2] boot: add support for button commands
>> This is simply awesome, but I see one possible issue -- the need to have >> proper environment variables defined for a particular board or device, >> to make the buttons work as expected. Obviously, those environment >> variables can be absent or can become missing for numerous reasons. > > Is CFG_EXTRA_ENV_SETTINGS not persistent enough? IMHO no. Because a user might accidentially mess up the environment variables. >> I think that we should have an additional mechanism in place that >> defines the buttons and the associated commands even if no environment >> variables are found. Like a set of fallback defaults for a particular >> board or device, built into the U-Boot image. For example, Rockchip >> boards have those defaults pretty well defined. > > A programmatic API for register button/cmd mapping from > board_late_init() (for example) sounds sensible to me. I don't know if it has to be that complex, or if it will be enough to just have some compile-time constants like CONFIG_BUTTON_CMD_N. > Is this really an issue that invalidates the implementation proposed > here though? It feels much more like a nice-to-have addition that maybe > we could leave out for now? Agreed. Looks like one can add it to get_button_cmd() later. > It also has a MUCH wider scope imo - should board override env or vice > versa? What about triggering default AND custom actions for one button > press? What if a board wants to register a callback function instead of > running a command?) - these are questions I don't want to answer with > this patch. One use case I have is restoring the default environment *in any case*; regardless what the state of the board is. In this case, the environment must not override the button settings. This can be a compiled-in command, which always takes precedence. If you want to be able to overwrite a button command, then I guess you can already do that with the environment setting. Provide sane defaults via CFG_EXTRA_ENV_SETTINGS and a user can then overwrite it. In summary, the registered (compiled-in) command should always take precedence. If one wants to supply a default command which can be changed later, that can go via the (compiled-in) default environment. -michael
Re: Adding EFI runtime support to the Arm's FF-A bus
Hi Mark, > Any runtime device drivers for variable storage should not be in the > U-Boot runtime but live in the secure world (e.g. OP-TEE) FF-A is the > new ARM protocol for talking to the secure world and hence fits into > the picture. What if I just want a simple embedded boot stack where I don't want any secure world and just want to be able to boot a COTS linux distribution via EFI? That already works for many Linux distros. As long as the distro installs the appropriate BOOTxxx.EFI file you don't actually need to set any EFI variables for the OS to boot. It can't get any simpler than that. Of the main Linux distros it seems that only Debian doesn't do this. Someone should probably lobby Debian to do this as well as it would mean that Debian would just work on an EBBR compliant system. I know. Last time I checked CentOS (or was it Ubuntu?) tried to set EFI variables and the installer just failed. Might be fixed now, though. Things get more complicated if you want to install multiple OSes. Then having EFI variable support makes things a lot more straightforward. And of course EFI secure boot needs EFI variable support as well (with proper support) for authenticated EFI variables. But IMHO that no longer falls into "simple embedded boot stack" territory. Thats clear. Assuming, that there might be a simple dedicated EEPROM to store the variables which is not exposed to linux, is that something which would be rejected by u-boot mainline now? Not necessarily. But such an approach will have limitations: * Completely hiding the EEPROM from the OS may be hard. Even if you have a dedicated SPI controller for the EEPROM things like the SPI bus clock or power domains may still be under OS control. Fair point, but I was thinking about the ls1028a for example, where - if I remember correctly - there was one dedicated i2c controller in a sense of isolation, probably to use with a secure OS. Also there is no dynamic clocking. So, technically it should be possible, even with a low overhead, like no device model etc, which could reside in the efi os services. Just testing the waters here, not that I'm interested in adding support for that in u-boot. Just a bit concerned that it (EFI variables) will only work with a full stack (tf-a, optee) in the future. * It is not possible to properly implement authenticated variables for secure boot if the EEPROM and associated hardware is just removed from the device tree but still accessable to the OS. An implementation that pretends the variables are "secure" will probably be rejected. Sure. I excluded any secure stuff. But, with that i2c controller i was talking about earlier, it should be possible to mark it as EL3 access only. Thanks, -michael
Re: Adding EFI runtime support to the Arm's FF-A bus
Hi Heinrich, > Any runtime device drivers for variable storage should not be in the > U-Boot runtime but live in the secure world (e.g. OP-TEE) FF-A is the > new ARM protocol for talking to the secure world and hence fits into > the picture. What if I just want a simple embedded boot stack where I don't want any secure world and just want to be able to boot a COTS linux distribution via EFI? Assuming, that there might be a simple dedicated EEPROM to store the variables which is not exposed to linux, is that something which would be rejected by u-boot mainline now? -michael
Re: [PATCH v1] cmd: mtd: OTP access support
Hi,
+static int do_mtd_otp_write(struct cmd_tbl *cmdtp, int flag, int
argc,
+ char *const argv[])
+{
..
+ printf("Caution! OTP data bits can't be erased! Continue
(y/n)?\n");
Please note, that with current SPI-NOR flashes this is not true and
there is usually some kind of erase command for the OTP bits. Only
the region lock is permanent and with that set, no more write or erase
is possible.
I see, so may be just rephrase this message, like just "Continue?
(y/n)?"
I don't have any strong opinion here. The code may or may not be used
to write to the OTP of an SPI-NOR flash. Also, this patchset doesn't
support the erase op (I guess u-boot's MTD won't support it either). So
from an user's perspective these OTP data can't be erased ;)
-michael
Re: [PATCH v1] cmd: mtd: OTP access support
+static int do_mtd_otp_write(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
..
+ printf("Caution! OTP data bits can't be erased! Continue (y/n)?\n");
Please note, that with current SPI-NOR flashes this is not true and
there is usually some kind of erase command for the OTP bits. Only
the region lock is permanent and with that set, no more write or erase
is possible.
-michael
Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible
Hi, I'm still not sure why that compatible is needed. Also I'd need to change the label which might break user space apps looking for that specific name. Also, our board might have u-boot/spl or u-boot/spl/bl31/bl32, right now that's something which depends on an u-boot configuration variable, which then enables or disables binman nodes in the -u-boot.dtsi. So in linux we only have that "bootloader" partition, but there might be either u-boot+spl or u-boot+spl+bl31+bl32. Honestly, I'm really not sure this should go into a device tree. I think we might be getting a bit ahead of ourselves here. I thought that the decision was that the label should indicate the contents. If you have multiple things in a partition then it would become a 'section' in Binman's terminology. Either the label programmatically describes what is inside or it doesn't. We can't have it both ways. What do you suggest? As Rob pointed out earlier, it's just a user-facing string. I'm a bit reluctant to use it programatically. Taking my example again, the string "bootloader" is sufficient for a user. He doesn't care if it's u-boot with spl or u-boot with tfa, or even coreboot. It just says, "in this partition is the bootloader". If you have an "bootloader" image you can flash it there. If it has a label "u-boot" and I want to switch to coreboot, will it have to change to "coreboot"? I really don't think this is practical, you are really putting software configuration into the device tree. At present it seems you have the image described in two places - one is the binman node and the other is the partitions node. I would like to unify these. And I'm not sure that will work for all the corner cases :/ If you keep the binman section seperate from the flash partition definition you don't have any of these problems, although there is some redundancy: - you only have compatible = "binman", "fixed-partition", no further compatibles are required - you don't have any conflicts with the current partition descriptions - you could even use the labels, because binman is the (only?) user But of course you need to find a place where to put your node. What does user space do with the partition labels? I'm not sure. Also I'm not sure if it really matters, I just wanted to point out, that you'll force users to change it. -michael >> What if a board uses eMMC to store the firmware binaries? Will that >> then >> be a subnode to the eMMC device? > > I thought there was a way to link the partition nodes and the device > using a property, without having the partition info as a subnode of > the device. But I may have imagined it as I cannot find it now. So > yes, it will be a subnode of the eMMC device. Not sure if that will fly. I can't find it anyway. There is somelike like that in simple-framebuffer with the 'display' property. Regards, SImon
Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible
Hi,
>> >> Add a compatible string for binman, so we can extend fixed-partitions
>> >> in various ways.
>> >
>> > I've been thinking at the proper way to describe the binman partitions.
>> > I am wondering if we should really extend the fixed-partitions
>> > schema. This description is really basic and kind of supposed to remain
>> > like that. Instead, I wonder if we should not just keep the binman
>> > compatible alone, like many others already. This way it would be very clear
>> > what is expected and allowed in both cases. I am thinking about
>> > something like that:
>> >
>> >
Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml
>> >
>> > this file is also referenced there (but this patch does the same, which
>> > is what I'd expect):
>> >
>> > Documentation/devicetree/bindings/mtd/partitions/partitions.yaml
>> >
>> > I'll let the binding maintainers judge whether they think it's
>> > relevant, it's not a strong opposition.
>>
>> What is the overall goal here? To replace the current binman node
>> which is
>> usually contained in the -u-boot.dtsi files? If one is using binman to
>> create an image, is it expected that one needs to adapt the DT in
>> linux?
>> Or will it still be a seperate -u-boot.dtsi? > Because in the latter
>> case
>> I see that there will be conflicts because you have to overwrite the
>> flash node. Or will it be a seperate node with all the information
>> duplicated?
>
> The goal is simply to have a full binding for firmware layout, such
> that firmware images can be created, examined and updated. The
> -u-boot.dtsi files are a stopgap while we sort out a real binding.
> They should eventually go away.
You haven't answered whether this node should be a seperate binman
node - or if you'll reuse the existing flash (partitions) node(s) and
add any missing property there. If it's the latter, I don't think
compatible = "binman", "fixed-partitions"; is correct.
My intent is to make it compatible, so wouldn't it make sense to have
binman as the first compatible, then falling back to fixed-partitions
as the second?
As far as I know, the compatibles should get more specific with each
string. But "binman" seems to be used as a kind of tag which could be
added to any compatible under the flash node. What if one wants to build
an image which isn't compatible = "fixed-partitions"? E.g.
"linksys,ns-partitions", will it then have
compatible = "binman", "linksys,ns-partitions"?
>> Maybe (a more complete) example would be helpful.
>
> Can you please be a bit more specific? What is missing from the
> example?
Like a complete (stripped) DTS. Right now I just see how the
individual
node looks like. But with a complete example DTS, my question from
above
would have been answered.
So to give an example myself, please correct it if it's wrong. From
our board (kontron-sl28):
&fspi {
status = "okay";
flash@0 {
compatible = "jedec,spi-nor";
m25p,fast-read;
spi-max-frequency = <13300>;
reg = <0>;
/* The following setting enables 1-1-2 (CMD-ADDR-DATA)
mode */
spi-rx-bus-width = <2>; /* 2 SPI Rx lines */
spi-tx-bus-width = <1>; /* 1 SPI Tx line */
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
reg = <0x00 0x01>;
label = "rcw";
read-only;
};
partition@1 {
reg = <0x01 0x1d>;
label = "failsafe bootloader";
read-only;
};
partition@20 {
reg = <0x20 0x01>;
label = "configuration store";
};
partition@21 {
reg = <0x21 0x1d>;
label = "bootloader";
};
partition@3e {
reg = <0x3e 0x02>;
label = "bootloader environment";
};
};
};
};
In u-boot we use binman, see
arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi
in the u-boot repository.
Now to use the new method, am I expected to adapt the dts in the
linux kernel? As far as I understand that is the case. So that node
from above would look something like the following:
&fspi {
status = "okay";
flash@0 {
compatible = "jedec,spi-nor";
m25p,fast-read;
spi-max-frequency = <13300>;
Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible
Hi, >> Add a compatible string for binman, so we can extend fixed-partitions >> in various ways. > > I've been thinking at the proper way to describe the binman partitions. > I am wondering if we should really extend the fixed-partitions > schema. This description is really basic and kind of supposed to remain > like that. Instead, I wonder if we should not just keep the binman > compatible alone, like many others already. This way it would be very clear > what is expected and allowed in both cases. I am thinking about > something like that: > > Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml > > this file is also referenced there (but this patch does the same, which > is what I'd expect): > > Documentation/devicetree/bindings/mtd/partitions/partitions.yaml > > I'll let the binding maintainers judge whether they think it's > relevant, it's not a strong opposition. What is the overall goal here? To replace the current binman node which is usually contained in the -u-boot.dtsi files? If one is using binman to create an image, is it expected that one needs to adapt the DT in linux? Or will it still be a seperate -u-boot.dtsi? > Because in the latter case I see that there will be conflicts because you have to overwrite the flash node. Or will it be a seperate node with all the information duplicated? The goal is simply to have a full binding for firmware layout, such that firmware images can be created, examined and updated. The -u-boot.dtsi files are a stopgap while we sort out a real binding. They should eventually go away. You haven't answered whether this node should be a seperate binman node - or if you'll reuse the existing flash (partitions) node(s) and add any missing property there. If it's the latter, I don't think compatible = "binman", "fixed-partitions"; is correct. Maybe (a more complete) example would be helpful. Can you please be a bit more specific? What is missing from the example? Like a complete (stripped) DTS. Right now I just see how the individual node looks like. But with a complete example DTS, my question from above would have been answered. What if a board uses eMMC to store the firmware binaries? Will that then be a subnode to the eMMC device? -michael
Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible
Hi, >> Add a compatible string for binman, so we can extend fixed-partitions >> in various ways. > > I've been thinking at the proper way to describe the binman partitions. > I am wondering if we should really extend the fixed-partitions > schema. This description is really basic and kind of supposed to remain > like that. Instead, I wonder if we should not just keep the binman > compatible alone, like many others already. This way it would be very clear > what is expected and allowed in both cases. I am thinking about > something like that: > > > Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml > > this file is also referenced there (but this patch does the same, which > is what I'd expect): > > Documentation/devicetree/bindings/mtd/partitions/partitions.yaml > > I'll let the binding maintainers judge whether they think it's > relevant, it's not a strong opposition. What is the overall goal here? To replace the current binman node which is usually contained in the -u-boot.dtsi files? If one is using binman to create an image, is it expected that one needs to adapt the DT in linux? Or will it still be a seperate -u-boot.dtsi? Because in the latter case I see that there will be conflicts because you have to overwrite the flash node. Or will it be a seperate node with all the information duplicated? Maybe (a more complete) example would be helpful. -michael
Re: [PATCH] MAINTAINERS: remove Vikas Manocha
Hi, The mails are bouncing with 550 5.1.1 User Unknown (in reply to RCPT TO command) Remove the entry and mark the ARM STM STV0991 arch as Orphan. Signed-off-by: Michael Walle Cc: Patrick Delaunay Cc: Patrice Chotard --- CCing other ST people, maybe someone want to take over instead. --- MAINTAINERS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 84de9de531..57ede4bea9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -641,8 +641,7 @@ N: stm32 ARM STM STV0991 -M: Vikas Manocha -S: Maintained +S: Orphan F: arch/arm/cpu/armv7/stv0991/ F: arch/arm/include/asm/arch-stv0991/ This misses board/st/stv0991/MAINTAINERS where he's also listed. So, someone needs to step up to maintain it all, or we should look at dropping it. Thanks! Ah, dang. I forgot about u-boots special per board MAINTAINERS. If there's no reply (maybe in like a week or so). I'll respin and put the board on Orphan. -michael
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
Hi, > This series adjusts binman to enforce just 4 extensions for output > images: > >.bin >.rom >.itb >.img > > Other extensions will produce an error. With this rule observed, > buildman > can keep the required files. How does this work? I didn't get all the patches from this series, which makes it really hard to review or ack the individual patches. Are we just whitelisting any files with these extension? Honestly, this sounds like an arbitrary restriction to me. But maybe I'm missing some context. You can see the full series here: https://patchwork.ozlabs.org/project/uboot/list/?series=370121 I know the archives/patchwork. All I was saying is that sending just parts of a patch series to specific ppl doesn't make any sense, because you always miss the context and for that you have to look at the archives. -michael
[PATCH] MAINTAINERS: remove Vikas Manocha
The mails are bouncing with 550 5.1.1 User Unknown (in reply to RCPT TO command) Remove the entry and mark the ARM STM STV0991 arch as Orphan. Signed-off-by: Michael Walle Cc: Patrick Delaunay Cc: Patrice Chotard --- CCing other ST people, maybe someone want to take over instead. --- MAINTAINERS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 84de9de531..57ede4bea9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -641,8 +641,7 @@ N: stm32 ARM STM STV0991 -M: Vikas Manocha -S: Maintained +S: Orphan F: arch/arm/cpu/armv7/stv0991/ F: arch/arm/include/asm/arch-stv0991/ -- 2.39.2
Re: [PATCH 5/6] kontron_sl28: Use u-boot-update.bin instead of u-boot.update
Am 2023-08-24 05:02, schrieb Simon Glass: A '.update' extension is not allowed anymore, so change it. Signed-off-by: Simon Glass Looks good to me, as it is just an intermediate binary. Acked-by: Michael Walle
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
Hi, This series adjusts binman to enforce just 4 extensions for output images: .bin .rom .itb .img Other extensions will produce an error. With this rule observed, buildman can keep the required files. How does this work? I didn't get all the patches from this series, which makes it really hard to review or ack the individual patches. Are we just whitelisting any files with these extension? Honestly, this sounds like an arbitrary restriction to me. But maybe I'm missing some context. -michael
Re: [PATCH v5 2/2] board: mediatek: add mt8195 demo board
Hi,
> + printf("Disabling WDT\n");
> + writel(0, 0x10007000);
Please don't use magic numbers. Also, I guess this should be a
real watchdog driver and u-boot will take care of disabling it
if the user wants to.
> +
> + printf("Enabling SCP SRAM\n");
> + for (unsigned int val = 0x; val != 0U;) {
> + val = val >> 1;
> + writel(val, 0x1072102C);
Even more magic numbers.
> + }
Thanks,
-michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
If the use of MTD is restricted to passive serial, this is OK with me.
Yeah, but that is not how upstream things work. You need to also think
of any other use cases.
These are the things I want to achieve.
* transfer data using the SPI driver and not use board files.
For that, the FPGA should be under the SPI in the device tree.
So to me it sounds like you actually want a "altera passive spi"
driver which uses the spi subsytem (and not the MTD layer).
* The size of the FPGA bitstream should be specified in the device
tree.
I'm not experienced with the fpga bindings, but maybe they have
something like that. Or you can deduce it from the compatible string.
What's your usecase here? It sounds like a u-boot limitation to me (if
you don't use files).
* The FPGA should be accessed as a device, not an address.
* I want a list of the FPGAs connected in the system
* Minimize code.
* Simplify user interface.
agreed.
From the CPU point of view, the FPGA is just a RAM location on the SPI
bus. It cannot be read, but it can be written.
This is not surprising, because it is simply a RAM.
It happens to have side effects, but that is not important.
Can't follow you here. How can the FPGA be a location in RAM if it's
behind an SPI bus? Thats not how SPI works.
The MTD subsystem supplies everything I need, and adding the driver
there is much less work than anything else.
Yeah but just because it has everything you need, doesn't mean
it is a good fit for your use case.
Again. I think you want to have a passive serial driver which
represents your fpga as a device which then can be configured.
There is already an empty (sigh) FPGA_UCLASS. That should probably
be extended. Ideally, this should be probed by the linux fpga
bindings.
The additional features of MTD simplifies the user interaction by
exposing a device and
providing info on the device.
Which would that be? What are the features of MTD which you need
here? You've mentioned MTD partitions, but I'm not sure these really
apply to partial reconfigurations. I know that Linux has a different
device tree binding for it. So making u-boot use mtd partitions (if
thats possible at all) and not the linux bindings means they'll
diverge, which is something we want to avoid.
You cannot do partial reconfiguration on passive serial,
so it is only of interest if someone feels a need to expand
the MTD to other configuration methods.
So.. what MTD features are then used by your approach?
Here is how an FPGA region looks in Linux.
If I understand things correctly, you have one memory region per
partial configuration. This looks quite a lot like partitions.
&fpga_region0 {
#address-cells = <1>;
#size-cells = <1>;
firmware-name = "soc_system.rbf";
fpga-bridges = <&fpga_bridge1>;
ranges = <0x2 0xff20 0x10>,
<0x0 0xc000 0x2000>;
gpio@10040 {
compatible = "altr,pio-1.0";
reg = <0x10040 0x20>;
altr,ngpio = <4>;
#gpio-cells = <2>;
clocks = <2>;
gpio-controller;
};
onchip-memory {
device_type = "memory";
compatible = "altr,onchipmem-15.1";
reg = <0x0 0x1>;
};
};
The only difference the MTD subsystem would see is
that there is a new subdirectory "fpga" with drivers
and the Kconfig + Makefile changes to support the directory.
Otherwise it plugs right in.
From a user point of view this is really confusing. Why would
you sometimes configure an fpga with the mtd command and sometimes
with the fpga command depening whether it is using this passive
programming thingy.
I think that most board designers would select one method of
configuring
the FPGA and they would only have either the FPGA or the MTD command
set. I would be surprised if an engineer would be confused by this.
As a user I'd expect one command for any similar devices. As a board
integrator, the fpga would be loaded by the board file anyway if needed
during u-boot/bootup.
Just my two cents as a spi-nor reviewer in linux. You'd have to
convince the u-boot maintainers. There is already an FPGA subsystem
in linux so I doubt your approach would make it into linux.
-michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Sorry, I didn't follow this too closely. Do you have some pointers? I just saw your latest mail. Thanks. -michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Am 2023-02-21 11:42, schrieb Ulf Samuelsson: Den 2023-02-21 kl. 10:08, skrev Michael Walle: If it is right or wrong to use that as an MTD is a matter of opinion. I am still hoping the MTD maintainer would provide input here. I might be missing something, but what is the reasoning here, to add this to the mtd subsystem? One is saving space, but I agree with Marek, this isn't a valid argument to just put any (unrelated) stuff into the MTD subsystem. Also, as Marek pointed out, there are many different 'programming' solutions for CPLDs/FPGAs and most of them don't share anything with MTD. You seem to be just focusing on the "passive serial" one. Yes, the passive serial is very different from the other ways of configuring an FPGA. It is write only. You cannot read back the configuration and no partial reconfiguration. You do not have to go through AXI/PCIe busses. You access through an SPI device, but todays solution does not support using the SPI driver. I can't follow you here. Now, I saw you mentioned that | the current passive serial solutions does not use the existing SPI | drivers. What do you mean with SPI drivers? SPI flash drivers or SPI controller drivers? Does the "passive serial solution" expose an SPI bus and you can access the SPI flash on it in a generic way? Then the solution should be to write a SPI (controller) driver, the flash should then be automatically be detected. The proposed solution is similar to an SPI flash driver. The FPGA bitstream is typically stored in the CPU boot flash. The CPU reads the bitstream and does an SPI transfer to the FPGA. The FPGA is not connected to any flash memory. Ah I missed that. So you don't want to program the SPI flash behind the FPGA, but the FPGA directly. You are loading a bitstream from somewhere and configure the FPGA. Using a SPI like interface, i.e. it is write only as it lacks the DO. The FPGA chip exposes (in SPI terms) SCK, nCS and MOSI, but not MISO so you cannot read back anything. it should read "This FPGA chip". I expect there are much more variants how to configure an FPGA. Would these also use your proposed solution? The driver will return an empty block on read. The SPI transfer to configure the FPGA is doing * toggle a GPIO signal (nCONFIG) * Do an SPI write (using the SPI controller driver) * After the SPI transfer complete, you check the status of some GPIO (nSTATUS etc.). This is all hidden from the MTD. What the MTD subsystem sees is a "write-only memory" that has to be written with exactly 'n' bytes. MTD is mainly around erasable blocks, although there are some exotic things like mtdram. I fail to see how this would apply to your FPGA. The additional features of MTD simplifies the user interaction by exposing a device and providing info on the device. Which would that be? What are the features of MTD which you need here? You've mentioned MTD partitions, but I'm not sure these really apply to partial reconfigurations. I know that Linux has a different device tree binding for it. So making u-boot use mtd partitions (if thats possible at all) and not the linux bindings means they'll diverge, which is something we want to avoid. The only difference the MTD subsystem would see is that there is a new subdirectory "fpga" with drivers and the Kconfig + Makefile changes to support the directory. Otherwise it plugs right in. From a user point of view this is really confusing. Why would you sometimes configure an fpga with the mtd command and sometimes with the fpga command depening whether it is using this passive programming thingy. The current solutions for passive serial cannot use the SPI controller driver so each board implements SPI controller inside their 'board' files. You cannot reuse this code in practice, so every one that wants to have passive serial has to write their own SPI access routines. Sorry, I didn't follow this too closely. Do you have some pointers? -michael The FPGA manager does not support device tree, and I will not be able to spend time on introducing this, as Marek advices. Best Regards Ulf Samuelsson -michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
>> If it is right or wrong to use that as an MTD is a matter of opinion. > > I am still hoping the MTD maintainer would provide input here. I might be missing something, but what is the reasoning here, to add this to the mtd subsystem? One is saving space, but I agree with Marek, this isn't a valid argument to just put any (unrelated) stuff into the MTD subsystem. Also, as Marek pointed out, there are many different 'programming' solutions for CPLDs/FPGAs and most of them don't share anything with MTD. You seem to be just focusing on the "passive serial" one. Now, I saw you mentioned that | the current passive serial solutions does not use the existing SPI | drivers. What do you mean with SPI drivers? SPI flash drivers or SPI controller drivers? Does the "passive serial solution" expose an SPI bus and you can access the SPI flash on it in a generic way? Then the solution should be to write a SPI (controller) driver, the flash should then be automatically be detected. -michael
Re: [PATCH] arm: kirkwood: Enable uart0 dm-pre-reloc for Pogoplug V4 board
Am 2023-02-13 09:43, schrieb Stefan Roese: On 2/10/23 22:08, Tony Dinh wrote: When DM_SERIAL is enabled, the device-tree tag u-boot,dm-pre-reloc is required for this board to boot over UART with kwboot. Enable this in kirkwood-pogoplug-series-4-u-boot.dtsi. Signed-off-by: Tony Dinh Reviewed-by: Stefan Roese If I understood it correctly, this is a workaround, right? Maybe we can include a link to the original discussion here for the case when someone stumbles over this commit: Link: https://lore.kernel.org/r/20230201080210.ypz4nrj4y2igwxz3@pali/ -michael
Re: [PATCH v1] RFC: rtc: rv8803: avoid bootloop on low voltage of button cell
Am 2023-02-08 12:29, schrieb Oliver Graute: Am 08.02.2023 um 09:32 schrieb Michael Walle : Am 2023-02-08 08:38, schrieb Oliver Graute: if the rtc button cell is on low voltage this can result in a permanent bootloop in u-boot because V2F Register is permanent set. ### Warning: temperature compensation has stopped ### Warning: Voltage low, data is invalid resetting ... With this patch the bootloop is prevented Signed-off-by: Oliver Graute I'm curious, how is the call tree here? I don't see the dm_rtc_get() being called during boot. Basically, you are now just ignoring the error. I use dm_rtc_get() in a modified boot/bootm.c to check for a End-Off Live booting date. Just to prevent that testers uses devices with out-dated images. So if this only affects me so this patch can be ignored. In this case you should just ignore the return value (and live with bogus values). Or.. handle it gracefully. -michael
Re: [PATCH v1] RFC: rtc: rv8803: avoid bootloop on low voltage of button cell
Am 2023-02-08 08:38, schrieb Oliver Graute: if the rtc button cell is on low voltage this can result in a permanent bootloop in u-boot because V2F Register is permanent set. ### Warning: temperature compensation has stopped ### Warning: Voltage low, data is invalid resetting ... With this patch the bootloop is prevented Signed-off-by: Oliver Graute I'm curious, how is the call tree here? I don't see the dm_rtc_get() being called during boot. Basically, you are now just ignoring the error. -michael
Re: [PATCH 1/1] efi_loader: stop watchdogs in ExitBootServices()
Basically I want the following: (1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled) The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board. For SUNXI boards disabling CONFIG_WATCHDOG_AUTOSTART solved the problem with the very short maximum expiration time of the watchdog. I can't follow you here. What "very short maximum expiration time"? With CONFIG_WATCHDOG_AUTOSTART disabled, the watchdog won't be kicked by u-boot, right? wdt->running will never be set to true and wdt_cyclic() will be a noop. The sunxi boards failed to boot with CONFIG_WATCHDOG_AUTOSTART because 16s is too short for Linux to install a watchdog driver. With CONFIG_WATCHDOG_AUTOSTART=n the watchdog is not running and the boards boot. But how does that help in my case? -michael
Re: [PATCH 1/1] efi_loader: stop watchdogs in ExitBootServices()
Honestly, not really? Some good number of SoCs will start the watchdog in ROM and these are also the ones that don't allow you to turn it off. I hope not, that sounds really risky. How would you debug such a platform? _Every single_ custom piece of industrial (as opposed to consumer-grade) hardware I've worked on as a consultant has had an external, always-running, gpio-petted watchdog. It's simply just something that the hardware designers include, and in some cases that's even due to certification requirements. So an always-running, cannot-be-turned-off, watchdog is a real thing, in real hardware, and if specs don't account for that, well, the spec is just paper, and we can ignore it. I agree. But on the other hand, you cannot assume or force the OS to have a watchdog driver in the general case - which is as I understand it - one goal of EFI. Obviously, there are watchdogs that can be disabled and some which cannot. I don't want to argue about the advantages and disadvantages. For watchdogs which cannot be turned off, we can't really do anything anyway after the handoff to the OS - except increasing its timeout if thats possible. For watchdogs that can be disabled (and are enabled in u-boot of course), there seems to be two use-cases: (1) embedded EFI boot, that is you know exactly what you are booting, i.e. self compiled kernel with a watchdog driver (2) booting a general OS via EFI, think of a debian boot CD for example. I agree, that for (1) the watchdog shouldn't be disabled. For (2) you cannot assume the booting OS has a driver for the watchdog, let it be an older version of a distribution which just haven't the SoC watchdog driver enabled or maybe because there is no driver for it at all (yet). Is there a way, to have the watchdog disabled for case (2) while also having the possibity to use bootm/booti/bootz and keep the watchdog enabled? Basically I want the following: (1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled) The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board. -michael For SUNXI boards disabling CONFIG_WATCHDOG_AUTOSTART solved the problem with the very short maximum expiration time of the watchdog. I can't follow you here. What "very short maximum expiration time"? With CONFIG_WATCHDOG_AUTOSTART disabled, the watchdog won't be kicked by u-boot, right? wdt->running will never be set to true and wdt_cyclic() will be a noop. -michael
Re: [PATCH 1/1] efi_loader: stop watchdogs in ExitBootServices()
>>> Honestly, not really? Some good number of SoCs will start the watchdog >>> in ROM and these are also the ones that don't allow you to turn it off. >> >> I hope not, that sounds really risky. How would you debug such a platform? > > _Every single_ custom piece of industrial (as opposed to consumer-grade) > hardware I've worked on as a consultant has had an external, > always-running, gpio-petted watchdog. It's simply just something that > the hardware designers include, and in some cases that's even due to > certification requirements. So an always-running, cannot-be-turned-off, > watchdog is a real thing, in real hardware, and if specs don't account > for that, well, the spec is just paper, and we can ignore it. I agree. But on the other hand, you cannot assume or force the OS to have a watchdog driver in the general case - which is as I understand it - one goal of EFI. Obviously, there are watchdogs that can be disabled and some which cannot. I don't want to argue about the advantages and disadvantages. For watchdogs which cannot be turned off, we can't really do anything anyway after the handoff to the OS - except increasing its timeout if thats possible. For watchdogs that can be disabled (and are enabled in u-boot of course), there seems to be two use-cases: (1) embedded EFI boot, that is you know exactly what you are booting, i.e. self compiled kernel with a watchdog driver (2) booting a general OS via EFI, think of a debian boot CD for example. I agree, that for (1) the watchdog shouldn't be disabled. For (2) you cannot assume the booting OS has a driver for the watchdog, let it be an older version of a distribution which just haven't the SoC watchdog driver enabled or maybe because there is no driver for it at all (yet). Is there a way, to have the watchdog disabled for case (2) while also having the possibity to use bootm/booti/bootz and keep the watchdog enabled? Basically I want the following: (1) board boots with watchdog enabled (2) u-boot services watchdog (3a) booting embedded linux with booti (watchdog enabled) or (3b) booting generic OS with bootefi (watchdog disabled) The missing case is booting an embedded linux with bootefi, which would be nice to have. But I don't really see it as a use-case for our board. -michael
Re: [PATCH] arm: kirkwood: Enable uart0 dm-pre-reloc for Kirkwood boards
> When DM_SERIAL is enabled, the device-tree property dm-pre-reloc is > required to boot over UART with kwboot. Enable this in a Kirkwood > common u-boot dtsi. My (dev) board unfortunately, have a bootloader which can't boot over serial. This is feature of Marvell BootROM and does not require any special from Bootloader. So you should be able to boot over UART (if you have accessible pins). I know, but there are known versions ob the bootrom where uart boot isn't supported (correctly). I also have another board which can boot over uart. But thats in daily use ;) Could you elaborate that a bit more? Why is this required for uart boot? kwboot will talk with the bootrom why does u-boot need anything? Or will there just be no output until the uart is initialized? On mvebu/armada boards this dm-pre-reloc is required to ensure that DT nodes are present in SPL DTB file. Otherwise build process drop all non-pre-realoc nodes from SPL version of DTB file. And because SPL use DM serial, it is required to have uart DT nodes in DTB file. Btw, same problem is with SPI in SPL. But... kirkwood does not use SPL, so I do not know what is reason for this here. Yes thats what puzzled me, too. > > Signed-off-by: Tony Dinh > --- > > arch/arm/dts/kirkwood-u-boot.dtsi | 7 +++ > 1 file changed, 7 insertions(+) > create mode 100644 arch/arm/dts/kirkwood-u-boot.dtsi Is this new? AFAIK only -u-boot.dtsi is included automatically. It is not new, Tom wrote about it quite ago: https://lore.kernel.org/u-boot/20220802121113.GG1146598@bill-the-cat/ Thats relatively new for someone not following the u-boot development that closely ;) Thanks for the pointer. -michael
Re: [PATCH] arm: kirkwood: Enable uart0 dm-pre-reloc for Kirkwood boards
Hi Tony, Am 2023-02-01 02:11, schrieb Tony Dinh: When DM_SERIAL is enabled, the device-tree property dm-pre-reloc is required to boot over UART with kwboot. Enable this in a Kirkwood common u-boot dtsi. My (dev) board unfortunately, have a bootloader which can't boot over serial. Could you elaborate that a bit more? Why is this required for uart boot? kwboot will talk with the bootrom why does u-boot need anything? Or will there just be no output until the uart is initialized? Signed-off-by: Tony Dinh --- arch/arm/dts/kirkwood-u-boot.dtsi | 7 +++ 1 file changed, 7 insertions(+) create mode 100644 arch/arm/dts/kirkwood-u-boot.dtsi Is this new? AFAIK only -u-boot.dtsi is included automatically. -michael
Re: [PATCH 2/3] pinctrl: get rid of some ifdeffery
Am 2023-01-18 14:08, schrieb Marek Vasut: On 1/18/23 13:43, Michael Walle wrote: Am 2023-01-18 13:18, schrieb Marek Vasut: On 1/18/23 13:12, Michael Walle wrote: [...] @@ -411,12 +405,11 @@ static int __maybe_unused pinctrl_post_bind(struct udevice *dev) } /* - * If set_state callback is set, we assume this pinctrl driver is the - * full implementation. In this case, its child nodes should be bound - * so that peripheral devices can easily search in parent devices - * during later DT-parsing. + * If the pinctrl driver has the full implementation, its child nodes + * should be bound so that peripheral devices can easily search in + * parent devices during later DT-parsing. */ - if (ops->set_state) + if (CONFIG_IS_ENABLED(PINCTRL_FULL)) return pinconfig_post_bind(dev); Is it correct to drop the ops->set_state non-NULL check ? See include/dm/pinctrl.h. set_state() is mandatory. Although, that seems to be wrong for the simple implementation. Could that be fixed for the simple case ? I think that should be easy to do, right ? You mean a documentation fix? -michael
Re: [PATCH 2/3] pinctrl: get rid of some ifdeffery
Am 2023-01-18 13:18, schrieb Marek Vasut: On 1/18/23 13:12, Michael Walle wrote: [...] @@ -411,12 +405,11 @@ static int __maybe_unused pinctrl_post_bind(struct udevice *dev) } /* - * If set_state callback is set, we assume this pinctrl driver is the - * full implementation. In this case, its child nodes should be bound -* so that peripheral devices can easily search in parent devices -* during later DT-parsing. + * If the pinctrl driver has the full implementation, its child nodes +* should be bound so that peripheral devices can easily search in +* parent devices during later DT-parsing. */ - if (ops->set_state) + if (CONFIG_IS_ENABLED(PINCTRL_FULL)) return pinconfig_post_bind(dev); Is it correct to drop the ops->set_state non-NULL check ? See include/dm/pinctrl.h. set_state() is mandatory. Although, that seems to be wrong for the simple implementation. -michael
[PATCH 3/3] pinctrl: fix docstring
Fix the copy and paste error. Signed-off-by: Michael Walle --- drivers/pinctrl/pinctrl-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index e6cd0889b0..23a1504716 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -382,7 +382,7 @@ int pinctrl_get_pin_muxing(struct udevice *dev, int selector, char *buf, } /** - * pinconfig_post_bind() - post binding for PINCTRL uclass + * pinctrl_post_bind() - post binding for PINCTRL uclass * Recursively bind child nodes as pinconfig devices in case of full pinctrl. * * @dev: pinctrl device -- 2.30.2
[PATCH 2/3] pinctrl: get rid of some ifdeffery
Don't define an empty version for pinconfig_post_bind(). Just guard the
call and let the linker garbage collection do the rest. This way, we
also don't have to do any guesswork.
Signed-off-by: Michael Walle
---
drivers/pinctrl/pinctrl-uclass.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c
index 419ef5f52f..e6cd0889b0 100644
--- a/drivers/pinctrl/pinctrl-uclass.c
+++ b/drivers/pinctrl/pinctrl-uclass.c
@@ -20,7 +20,6 @@
DECLARE_GLOBAL_DATA_PTR;
-#if CONFIG_IS_ENABLED(PINCTRL_FULL)
/**
* pinctrl_config_one() - apply pinctrl settings for a single node
*
@@ -148,6 +147,7 @@ static int pinconfig_post_bind(struct udevice *dev)
return 0;
}
+#if CONFIG_IS_ENABLED(PINCTRL_FULL)
UCLASS_DRIVER(pinconfig) = {
.id = UCLASS_PINCONFIG,
#if CONFIG_IS_ENABLED(PINCONF_RECURSIVE)
@@ -160,12 +160,6 @@ U_BOOT_DRIVER(pinconfig_generic) = {
.name = "pinconfig",
.id = UCLASS_PINCONFIG,
};
-
-#else
-static int pinconfig_post_bind(struct udevice *dev)
-{
- return 0;
-}
#endif
static int
@@ -411,12 +405,11 @@ static int __maybe_unused pinctrl_post_bind(struct
udevice *dev)
}
/*
-* If set_state callback is set, we assume this pinctrl driver is the
-* full implementation. In this case, its child nodes should be bound
-* so that peripheral devices can easily search in parent devices
-* during later DT-parsing.
+* If the pinctrl driver has the full implementation, its child nodes
+* should be bound so that peripheral devices can easily search in
+* parent devices during later DT-parsing.
*/
- if (ops->set_state)
+ if (CONFIG_IS_ENABLED(PINCTRL_FULL))
return pinconfig_post_bind(dev);
return 0;
--
2.30.2
[PATCH 1/3] pinctrl: don't fall back to pinctrl_select_state_simple()
If CONFIG_PINCTRL_FULL is enabled, never fall back to the simple
implementation. pinctrl_select_state() is called for each device and it
is expected to fail. A fallback to the simple imeplementation doesn't
make much sense.
To keep the return code consistent, we need to change the -EINVAL (which
was ignored before) to -ENOSYS.
Signed-off-by: Michael Walle
---
The real underlying problem is that the pinctrl_select_state_simple()
doesn't really work before relocation. This implementation calls
uclass_get_device() which will try to probe the pinctrl, but that device
might not be ready yet, i.e. in my case, it is an MFD and it needs its
parent. For each device in the devicetree, there will be a
device_probe(pinctrl) attempt and if that device has some kind of
private or platform data, it will eat away the early memory, because
there is no free.
I'm not really sure how that could be fixed. But still, if there is a
full implementation, it shouldn't fall back to the simple one.
---
drivers/pinctrl/pinctrl-uclass.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c
index ce2d5ddf6d..419ef5f52f 100644
--- a/drivers/pinctrl/pinctrl-uclass.c
+++ b/drivers/pinctrl/pinctrl-uclass.c
@@ -71,13 +71,13 @@ static int pinctrl_select_state_full(struct udevice *dev,
const char *statename)
*/
state = dectoul(statename, &end);
if (*end)
- return -EINVAL;
+ return -ENOSYS;
}
snprintf(propname, sizeof(propname), "pinctrl-%d", state);
list = dev_read_prop(dev, propname, &size);
if (!list)
- return -EINVAL;
+ return -ENOSYS;
size /= sizeof(*list);
for (i = 0; i < size; i++) {
@@ -162,11 +162,6 @@ U_BOOT_DRIVER(pinconfig_generic) = {
};
#else
-static int pinctrl_select_state_full(struct udevice *dev, const char
*statename)
-{
- return -ENODEV;
-}
-
static int pinconfig_post_bind(struct udevice *dev)
{
return 0;
@@ -317,10 +312,10 @@ int pinctrl_select_state(struct udevice *dev, const char
*statename)
* Try full-implemented pinctrl first.
* If it fails or is not implemented, try simple one.
*/
- if (pinctrl_select_state_full(dev, statename))
- return pinctrl_select_state_simple(dev);
+ if (CONFIG_IS_ENABLED(PINCTRL_FULL))
+ return pinctrl_select_state_full(dev, statename);
- return 0;
+ return pinctrl_select_state_simple(dev);
}
int pinctrl_request(struct udevice *dev, int func, int flags)
--
2.30.2
Re: [PATCH 6/6] nvmem: u-boot-env: post process "ethaddr" env variable
Hi,
Am 2023-01-10 11:54, schrieb Rafał Miłecki:
From: Rafał Miłecki
U-Boot environment variables are stored in ASCII format so "ethaddr"
requires parsing into binary to make it work with Ethernet interfaces.
This includes support for indexes to support #nvmem-cell-cells = <1>.
Signed-off-by: Rafał Miłecki
---
drivers/nvmem/layouts/Kconfig | 1 +
drivers/nvmem/layouts/u-boot-env.c | 24
2 files changed, 25 insertions(+)
diff --git a/drivers/nvmem/layouts/Kconfig
b/drivers/nvmem/layouts/Kconfig
index 8a38c514943a..566b4f25630d 100644
--- a/drivers/nvmem/layouts/Kconfig
+++ b/drivers/nvmem/layouts/Kconfig
@@ -23,6 +23,7 @@ config NVMEM_LAYOUT_ONIE_TLV
config NVMEM_LAYOUT_U_BOOT_ENV
bool "U-Boot environment variables support"
select CRC32
+ select GENERIC_NET_UTILS
help
U-Boot stores its setup as environment variables. This driver adds
support for verifying & exporting such data. It also exposes
variables
diff --git a/drivers/nvmem/layouts/u-boot-env.c
b/drivers/nvmem/layouts/u-boot-env.c
index 95c314553952..63baeb18bd56 100644
--- a/drivers/nvmem/layouts/u-boot-env.c
+++ b/drivers/nvmem/layouts/u-boot-env.c
@@ -4,6 +4,8 @@
*/
#include
+#include
+#include
#include
#include
#include
@@ -36,6 +38,26 @@ struct u_boot_env_image_broadcom {
uint8_t data[];
} __packed;
+static int u_boot_env_read_post_process_ethaddr(void *context, const
char *id, int index,
+ unsigned int offset, void
*data, size_t *bytes)
+{
+ u8 mac[ETH_ALEN];
+
+ if (*bytes != 3 * ETH_ALEN - 1)
+ return -EINVAL;
+
+ if (!mac_pton(data, mac))
+ return -EINVAL;
+
+ if (index)
+ eth_addr_add(mac, index);
+
+ ether_addr_copy(data, mac);
+ *bytes = ETH_ALEN;
I don't know how to feel about this. This will only work if the new
size is smaller than the old one. Can't we have a correct size in
the first place? I.e. while adding the cells.
-michael
+
+ return 0;
+}
+
static int u_boot_env_parse_data(struct device *dev, struct
nvmem_device *nvmem, uint8_t *buf,
size_t data_offset, size_t data_len)
{
@@ -67,6 +89,8 @@ static int u_boot_env_parse_data(struct device *dev,
struct nvmem_device *nvmem,
info.offset = data_offset + value - data;
info.bytes = strlen(value);
info.np = of_get_child_by_name(np, info.name);
+ if (!strcmp(var, "ethaddr"))
+ info.read_post_process =
u_boot_env_read_post_process_ethaddr;
err = nvmem_add_one_cell(nvmem, &info);
if (err) {
Re: [PATCH 2/3] nvmem: core: allow .read_post_process() callbacks to adjust data length
Hi, Am 2023-01-05 18:10, schrieb Rafał Miłecki: From: Rafał Miłecki Sometimes reading NVMEM cell value involves some data reformatting. It requires passing updated size value to the caller. Support that. Wouldn't it make more sense to convert that driver to proper nvmem layouts, where (1) you get that for free, (2) support others storages than just mtd (3) don't duplicate the mtd read code -michael
Re: Rockchip RK3328 4-byte addressing problem in SPI
So leaving 4-byte switched by UBoot SPI chip made it unusable to RockChip Bootrom. I found this by dumping Bootrom and decompiling it. >> >> Sync with engineer working on these area, and get below: >> >> Yes, this "4-byte addressing problem in SPI" issue is in SoCs including >> rk3328, this only happen > > Can we have an explicit list of SoCs affected by this errata please? I'm not sure this is a problem of the SoC (bootrom). The issue is either board design or flash usage pattern. If you enable 4byte mode and don't have a hardware reset pin (or don't assert it during board reset) you are likely in trouble. Have a look at JESD216D. The 16th DWORD lists 6 different software reset methods, there is even a "no software reset instruction is supported". That's also true for the "exit 4byte mode": there is no common way to exit it. You could try to read the SFDP tables. But that will only work for flash devices with (valid) SFDP. So in the general case, you are probably f** when you enable the 4 byte mode, boot from that flash and don't have a reset. As mentioned here, you should instead use the 4byte opcodes where possible. -michael
[PATCH 2/2] cmd: net: wget: fix Kconfig dependency
The wget command uses TCP, but fails to select PROT_TCP in Kconfig. Instead it selects the non-existing symbol TCP. Fix the typo. Signed-off-by: Michael Walle --- cmd/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index d93731f2af..b2d7598717 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1814,7 +1814,7 @@ config SYS_DISABLE_AUTOLOAD config CMD_WGET bool "wget" - select TCP + select PROT_TCP help wget is a simple command to download kernel, or other files, from a http server over TCP. -- 2.30.2
[PATCH 1/2] net: wget: fix implicit declaration
The compiler complains about the missing declaration of print_size(): net/wget.c:415:3: warning: implicit declaration of function ‘print_size’ [-Wimplicit-function-declaration] Fix it. Signed-off-by: Michael Walle --- net/wget.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/wget.c b/net/wget.c index 3826c4b364..eebdf80eb5 100644 --- a/net/wget.c +++ b/net/wget.c @@ -6,6 +6,7 @@ #include #include +#include #include #include #include -- 2.30.2
Re: Converting to DM SERIAL for Kirkwood boards
>> On 12/9/22 04:55, Tony Dinh wrote:
>> > Hi Simon et al,
>> >
>> > (Resend to include u-boot mailing list)
>> >
>> > I'm in the process of converting Kirkwood boards to use DM SERIAL. I
>> > could not seem to get it to work, having tried adding
>> > CONFIG_DM_SERIAL, and also playing with various related CONFIG
>> > options (CONFIG_SPECIFY_CONSOLE_INDEX and CONFIG_CONS_INDEX ). From
>> > my reading various board configurations that were already converted to
>> > DM_SERIAL, I'm under the impression that just turning on
>> > CONFIG_DM_SERIAL would work without any other addition.
>> >
>> > The board I'm testing is Zyxel NSA310S Kirkwood 6702 (6192) SoC.
>> >
>> > diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
>> > index afa0cad041..e81d1495bd 100644
>> > --- a/configs/nsa310s_defconfig
>> > +++ b/configs/nsa310s_defconfig
>> > @@ -41,7 +41,6 @@ CONFIG_ENV_OVERWRITE=y
>> > CONFIG_ENV_IS_IN_NAND=y
>> > CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>> > CONFIG_NET_RANDOM_ETHADDR=y
>> > -CONFIG_NETCONSOLE=y
>> > CONFIG_SYS_FAULT_ECHO_LINK_DOWN=y
>> > CONFIG_SATA_MV=y
>> > CONFIG_SYS_SATA_MAX_DEVICE=1
>> > @@ -53,6 +52,7 @@ CONFIG_MTD_RAW_NAND=y
>> > CONFIG_PHY_MARVELL=y
>> > CONFIG_MVGBE=y
>> > CONFIG_MII=y
>> > +CONFIG_DM_SERIAL=y
>> > CONFIG_SYS_NS16550=y
>> > CONFIG_USB=y
>> > CONFIG_USB_EHCI_HCD=y
>> >
>> > I also added kirkwood-nsa310s-u-boot.dtsi to help in running kwboot.
>> >
>> > &uart0 {
>> > u-boot,dm-pre-reloc;
>> > };
>> >
>> > I've tried kwboot the new u-boot image, and also flashed this image to
>> > NAND, and in both cases, I got a silent serial console. It seems to
>> > hang up right off the bat. Hope you can help by giving me some
>> > pointers on how to debug this.
>>
>> Might be that the alias is missing and / or that the uart DT node is
>> not enabled. Please give this test-only patch a try:
>>
>> diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
>> b/arch/arm/dts/kirkwood-nsa310s.dts
>> index 09ee76c2a2e0..ca7a49af9ba4 100644
>> --- a/arch/arm/dts/kirkwood-nsa310s.dts
>> +++ b/arch/arm/dts/kirkwood-nsa310s.dts
>> @@ -17,6 +17,10 @@
>> model = "Zyxel NSA310S";
>> compatible = "zyxel,nsa320s", "marvell,kirkwood-88f6702",
>> "marvell,kirkwood";
>>
>> + aliases {
>> + serial0 = &uart0;
>> + };
>> +
>> memory {
>> device_type = "memory";
>> reg = <0x 0x1000>;
>> @@ -317,3 +321,8 @@
>> &pcie0 {
>> status = "okay";
>> };
>> +
>> +&uart0 {
>> + status = "okay";
>> + u-boot,dm-pre-reloc;
>> +};
>
> Thanks for the patch! but the behavior is still the same (silent
> serial console and hung the board).
>
> Thanks,
> Tony
Maybe this will help:
https://lore.kernel.org/u-boot/20220817193809.1059688-20-mich...@walle.cc/
The lsxl is also a kirkwood based board.
-michael
Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices
Hi, >> I'm not really happy with this approach. It's not that upstream doesn't >> have aliases now, it's that it has different aliases, right? That's why >> they won't accept these? > > imx6q.dtsi does have the default mmc aliases: > > mmc0 = &usdhc1; > mmc1 = &usdhc2; > mmc2 = &usdhc3; > mmc3 = &usdhc4; > > Upstream does not want to change mmc alias because users may rely on > this mmc aliases. This thread seems to be another reason *not* to put any aliases into a common dtsi file. They are board specific and should really go into the board dts. -michael
Re: [PATCH V2 1/2] nvmem: core: refactor .cell_post_process() CB arguments
Am 2022-11-28 07:59, schrieb Rafał Miłecki: From: Rafał Miłecki Pass whole NVMEM cell struct and length pointer as arguments to callback functions. This allows: 1. Cells content to be modified based on more info Some cells (identified by their names) contain specific data that needs further processing. This can be e.g. MAC address stored in an ASCII format. NVMEM consumers expect MAC to be read in a binary form. More complex cells may be additionally described in DT. This change allows also accessing relevant DT nodes and reading extra info. 2. Adjusting data length If cell processing results in reformatting it, it's required to adjust length. This again applies e.g. to the MAC format change from ASCII to the byte-based. Later on we may consider more cleanups & features like: 1. Dropping "const char *id" and just using NVMEM cell name 2. Adding extra argument for cells providing multiple values Signed-off-by: Rafał Miłecki --- This solution conflicts with 1 part of Michael's work: [PATCH v2 00/20] nvmem: core: introduce NVMEM layouts https://lore.kernel.org/linux-arm-kernel/20220901221857.2600340-1-mich...@walle.cc/ Instead of: 1. Adding NVMEM cell-level post_process callback 2. Adding callback (.fixup_cell_info()) for setting callbacks 3. Dropping NVMEM device-level post_process callback I decided to refactor existing callback. Michael's work on adding #nvmem-cell-cells should be possible to easily rebase on top of those changes. As yours should be easily added on top of my series. I've showed that providing a global post process hook is bad because that way you need to have *all* cells of your device read-only. -michael
Re: Setting MAC address from I2C EEPROM - debug / commands? (Xilinx)
>> ethernet {
>> nvmem-cells = <&mac_address>;
>> nvmem-cell-names = "mac-address";
>> };
>>
>> You'll need 2022.07 for this I think. This is the same method which
>> Linux uses. I added this specificly to be able to load MAC addresses
>> from EEPROMs without needing to hard code stuff into Kconfig.
>
> This looks good and I see Sean wired it in the U-Boot already. It should work
> fine with all Xilinx formats but on boards just for MAC address. (FRU format
> is
> also designed in a way that the same boards have MAC address at the same
> location).
> The code I described above is also checking in FRU format that checksums are
> correct and also reading more information from it for other use cases.
You might also be interested in the new NVMEM layout patch series:
https://lore.kernel.org/lkml/20221118185118.1190044-1-mich...@walle.cc/
-michael
Re: [PATCH v2 3/8] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support
Am 2022-09-04 02:02, schrieb Tony Dinh: Hi Stefan, Sorry, that message was prematurely sent (fat finger). Please see the continuation below. On Sat, Sep 3, 2022 at 4:43 PM Tony Dinh wrote: Hi Stefan, On Sat, Sep 3, 2022 at 3:44 AM Stefan Roese wrote: > > Hi Tony, > > On 03.09.22 11:44, Tony Dinh wrote: > > Hi Stefan, > > > > On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese wrote: > >> > >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE > >> enabled, like pogo_v4. > >> > >> Signed-off-by: Stefan Roese > >> --- > >> v2: > >> - Change timer_get_boot_us() to use the timer_early functions > >> - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) > >> > >> Simon, I'm currently looking into this timer_get_boot_us() to using > >> timer_early_get_count() etc consolidation. > > > > Indeed, as you've mentioned above, I think timer_early_get_count() and > > timer_early_get_rate() do need to take into consideration what the > > input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as > > the Pogo V4. > > > > I'm seeing on the Pogo V4 test, the timer command reports time about 6 > > times slower than it should. It does seem to jive with the fact that > > the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. > > Ah, I've missing updating the early functions to also differentiate > between fixed clocks and TCLK timer. > > Please give the attached patch a try - should be applied on top of this > latest patchset. That looks promising, but I think we are still missing something. After applying the attached patch, I ran the test again and it behaved the same way (clock rate 6 times slower). So I did another test. --- Test 1 Pogo_V4> timer start; sleep 60; timer get; sleep 30; timer get 60.000 90.000 So apparently the sleep cmd has reset the correct clock rate. --- Test 2 Pogo_V4> timer start; sleep 30; timer get; sleep 30; timer get 30.000 60.000 And then wait for 30 seconds, do another "timer get" (I expected to see about 90 to 95 seconds). Pogo_V4> timer get 66.237 I've did the same test and can confirm it. But I think that is a drawback from the timer command. With a 32bit timer and a TCLK of 166MHz (on my board), the timer will wrap around about every 26s. So if you don't do a get_timer() call for that whole period, the overflow won't be counted in timer_conv_64(). For the sleep command it works correctly, because it will poll get_timer() every 100us. In summary, you cannot expect a "timer get" on the console to work if you cannot make sure, the you call it at least every "2^32 / TCLK" seconds. -michael
Re: [PATCH v2 1/8] timer: orion-timer: Use timer_conv_64() to fix timer wrap around
Am 2022-09-02 08:25, schrieb Stefan Roese:
While testing on some Kirkwood platforms it was noticed that the timer
did not function correctly all the time. The driver did not correctly
handle 32bit timer value wrap arounds. Using the timer_conv_64()
conversion function fixes this issue.
Fixes: e9e73d78a8fb ("timer: add orion-timer support")
Ahh. Sorry about that. I've missed that conversation which is also
in lib/time.c in a different call tree.
Maybe this can be picked to master (if this series is going into
next)?
-michael
Re: [PATCH] arm: orion5: Completely remove Orion5 platform support
Am 2022-09-01 10:53, schrieb Stefan Roese: As there is no Orion5 based target in mainline U-Boot any more, let's completely remove the support for this pretty old Marvell platform. Signed-off-by: Stefan Roese Cc: Tony Dinh Cc: Pali Rohár Cc: Michael Walle --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -440,7 +440,7 @@ config KSZ9477 config MVGBE bool "Marvell Orion5x/Kirkwood network interface support" Remove Orion5x here, too? - depends on ARCH_KIRKWOOD || ARCH_ORION5X + depends on ARCH_KIRKWOOD select PHYLIB help This driver supports the network interface units in the
Re: [PATCH 3/6] arm: mvebu: Use CONFIG_TIMER on all MVEBU & KIRKWOOD platforms
Am 2022-08-30 13:53, schrieb Stefan Roese: Now that the new timer support is available for these platforms, let's select this IF for all these platforms. This way it's not necessary that each board changes it's config header. Signed-off-by: Stefan Roese --- arch/arm/Kconfig | 4 arch/arm/mach-mvebu/include/mach/config.h | 5 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 0b72e4f6503e..60f524a2d118 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -618,6 +618,7 @@ config ARCH_KIRKWOOD select BOARD_EARLY_INIT_F select CPU_ARM926EJS select GPIO_EXTRA_HEADER + select TIMER If selected by the arch now (and the timer driver defaulting to y on this arch), could you clean the lschvl2 and lsxhl_defconfigs up and remove the two symbols there? config ARCH_MVEBU bool "Marvell MVEBU family (Armada XP/375/38x/3700/7K/8K)" @@ -629,6 +630,8 @@ config ARCH_MVEBU select GPIO_EXTRA_HEADER select SPL_DM_SPI if SPL select SPL_DM_SPI_FLASH if SPL + select SPL_TIMER if SPL + select TIMER select OF_CONTROL select OF_SEPARATE select SPI @@ -639,6 +642,7 @@ config ARCH_ORION5X select CPU_ARM926EJS select GPIO_EXTRA_HEADER select SPL_SEPARATE_BSS if SPL + select TIMER config TARGET_STV0991 bool "Support stv0991" diff --git a/arch/arm/mach-mvebu/include/mach/config.h b/arch/arm/mach-mvebu/include/mach/config.h index 4add0d9e1030..9b5036c31dd3 100644 --- a/arch/arm/mach-mvebu/include/mach/config.h +++ b/arch/arm/mach-mvebu/include/mach/config.h @@ -41,9 +41,4 @@ #endif #endif -/* Use common timer */ -#define CONFIG_SYS_TIMER_COUNTS_DOWN -#define CONFIG_SYS_TIMER_COUNTER (MVEBU_TIMER_BASE + 0x14) -#define CONFIG_SYS_TIMER_RATE 2500 - #endif /* __MVEBU_CONFIG_H */
Re: [PATCH 2/6] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support
Am 2022-08-30 13:53, schrieb Stefan Roese:
Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE
enabled, like pogo_v4.
Signed-off-by: Stefan Roese
---
drivers/timer/orion-timer.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index 02ed138642b8..7e920eaeaa40 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -41,6 +41,28 @@ u64 notrace timer_early_get_count(void)
}
#endif
+#if CONFIG_IS_ENABLED(BOOTSTAGE)
+ulong timer_get_boot_us(void)
+{
+ u64 ticks = 0;
+ u32 rate = 1;
+ u64 us;
+ int ret;
+
+ ret = dm_timer_init();
+ if (!ret) {
+ /* The timer is available */
+ rate = timer_get_rate(gd->timer);
+ timer_get_count(gd->timer, &ticks);
+ } else {
+ return 0;
+ }
+
+ us = (ticks * 1000) / rate;
+ return us;
+}
+#endif
This is duplicate code in almost all the timer drivers, shouldn't
this be a (weak) default implementation in timer-uclass.c? Also,
do you need to guard it with CONFIG_IS_ENABLED(BOOTSTAGE), aren't
unused functions discarded anyway?
-michael
Re: mvebu - switch to orion-timer
Hi, >On 24.08.22 00:33, Pali Rohár wrote: >> Hello Stefan! Now when U-Boot contains new orion-timer.c driver, which >> Michael wrote, I think that it mvebu platform should switch to use it. >> Because build process for armada boards prints deprecation warning that >> new timer is not being used. Could you look at it, if it is possible to >> do global switch for mach-mvebu and mach-kirkwood? I think it does not >> make sense to do per-board switching as it is de-facto platform related >> code. > > Yes, I also though about this. > > But... > > In Linux we have 2 different timer clocksource drivers for Orion / > Kirkwood and Armada XP / 370 etc: > > drivers/clocksource/timer-orion.c > drivers/clocksource/timer-armada-370-xp.c > > I did not check, if and why this is necessary to handle those chips > via different drivers yet. Perhaps later this week. Or someone of > you beats me with this. ;) One thing which is definitely missing is https://elixir.bootlin.com/u-boot/v2022.10-rc3/source/arch/arm/mach-mvebu/timer.c#L33 I didn't bother to handle that one. And of course all the compatibles are missing. -michael
Re: [PATCH 00/22] board: lsxl: major update and DM conversion
Am 2022-08-23 17:01, schrieb Stefan Roese: Applied to u-boot-marvell/master, with my small fix for the ds109 board Great! Thanks, Stefan. -michael
[PATCH v3 3/5] board: sl28: add user friendly names for the boot sources
During startup the SPL will print where the u-boot proper is read from.
Instead of using the default names, provide more user friendly names.
Signed-off-by: Michael Walle
---
board/kontron/sl28/spl.c | 16
1 file changed, 16 insertions(+)
diff --git a/board/kontron/sl28/spl.c b/board/kontron/sl28/spl.c
index b1fefc22b2..ffaf517a8b 100644
--- a/board/kontron/sl28/spl.c
+++ b/board/kontron/sl28/spl.c
@@ -95,6 +95,22 @@ unsigned int spl_spi_get_uboot_offs(struct spi_flash *flash)
}
}
+const char *spl_board_loader_name(u32 boot_device)
+{
+ enum boot_source src = sl28_boot_source();
+
+ switch (src) {
+ case BOOT_SOURCE_SDHC:
+ return "SD card (Test mode)";
+ case BOOT_SOURCE_SPI:
+ return "Failsafe SPI flash";
+ case BOOT_SOURCE_I2C:
+ return "SPI flash";
+ case BOOT_SOURCE_MMC:
+ return "eMMC";
+ default:
+ return "(unknown)";
+ }
}
int board_early_init_f(void)
--
2.30.2
[PATCH v3 5/5] board: sl28: remove COUNTER_FREQUENCY_REAL
The frequency of the system counter is static which is given by the COUNTER_FREQUENCY option. Remove COUNTER_FREQUENCY_REAL. Signed-off-by: Michael Walle --- include/configs/kontron_sl28.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/configs/kontron_sl28.h b/include/configs/kontron_sl28.h index 38063ba484..df46e586f3 100644 --- a/include/configs/kontron_sl28.h +++ b/include/configs/kontron_sl28.h @@ -37,8 +37,6 @@ /* serial port */ #define CONFIG_SYS_NS16550_CLK (get_bus_freq(0) / 2) -#define COUNTER_FREQUENCY_REAL (get_board_sys_clk() / 4) - /* SPL */ #define CONFIG_SYS_MONITOR_LEN (1024 * 1024) -- 2.30.2
[PATCH v3 4/5] board: sl28: support dynamic prompts
Depending on the boot source, set different CLI prompts. This will help
the user to figure out in which mode the bootloader was started. There
are two special modes: failsafe and SDHC boot.
Signed-off-by: Michael Walle
---
board/kontron/sl28/sl28.c | 20
configs/kontron_sl28_defconfig | 1 +
2 files changed, 21 insertions(+)
diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
index 54719cc01c..0576b3eae4 100644
--- a/board/kontron/sl28/sl28.c
+++ b/board/kontron/sl28/sl28.c
@@ -126,8 +126,28 @@ static void stop_recovery_watchdog(void)
wdt_stop(dev);
}
+static void sl28_set_prompt(void)
+{
+ enum boot_source src = sl28_boot_source();
+
+ switch (src) {
+ case BOOT_SOURCE_SPI:
+ env_set("PS1", "[FAILSAFE] => ");
+ break;
+ case BOOT_SOURCE_SDHC:
+ env_set("PS1", "[SDHC] => ");
+ break;
+ default:
+ env_set("PS1", NULL);
+ break;
+ }
+}
+
int fsl_board_late_init(void)
{
+ if (IS_ENABLED(CONFIG_CMDLINE_PS_SUPPORT))
+ sl28_set_prompt();
+
/*
* Usually, the after a board reset, the watchdog is enabled by
* default. This is to supervise the bootloader boot-up. Therefore,
diff --git a/configs/kontron_sl28_defconfig b/configs/kontron_sl28_defconfig
index 4d50c681f9..fc1c607927 100644
--- a/configs/kontron_sl28_defconfig
+++ b/configs/kontron_sl28_defconfig
@@ -55,6 +55,7 @@ CONFIG_SPL_SPI_LOAD=y
CONFIG_SYS_CBSIZE=256
CONFIG_SYS_PBSIZE=276
CONFIG_SYS_BOOTM_LEN=0x80
+CONFIG_CMDLINE_PS_SUPPORT=y
CONFIG_CMD_ASKENV=y
CONFIG_CMD_GREPENV=y
CONFIG_CMD_NVEDIT_EFI=y
--
2.30.2
[PATCH v3 2/5] board: sl28: implement additional bootsources
The board is able to boot from the following source:
- user-updateble SPI flash
- write-protected part of the same SPI flash
- eMMC
- SD card
Implement the needed function hooks to support all of these boot
sources.
Signed-off-by: Michael Walle
---
board/kontron/sl28/common.c| 22
board/kontron/sl28/sl28.c | 23
board/kontron/sl28/sl28.h | 16 ++
board/kontron/sl28/spl.c | 38 +-
configs/kontron_sl28_defconfig | 5 -
5 files changed, 102 insertions(+), 2 deletions(-)
create mode 100644 board/kontron/sl28/sl28.h
diff --git a/board/kontron/sl28/common.c b/board/kontron/sl28/common.c
index 33c6843c3f..331de29bae 100644
--- a/board/kontron/sl28/common.c
+++ b/board/kontron/sl28/common.c
@@ -2,6 +2,9 @@
#include
#include
+#include
+
+#include "sl28.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -9,3 +12,22 @@ u32 get_lpuart_clk(void)
{
return gd->bus_clk / CONFIG_SYS_FSL_LPUART_CLK_DIV;
}
+
+enum boot_source sl28_boot_source(void)
+{
+ u32 rcw_src = in_le32(DCFG_BASE + DCFG_PORSR1) & DCFG_PORSR1_RCW_SRC;
+
+ switch (rcw_src) {
+ case DCFG_PORSR1_RCW_SRC_SDHC1:
+ return BOOT_SOURCE_SDHC;
+ case DCFG_PORSR1_RCW_SRC_SDHC2:
+ return BOOT_SOURCE_MMC;
+ case DCFG_PORSR1_RCW_SRC_I2C:
+ return BOOT_SOURCE_I2C;
+ case DCFG_PORSR1_RCW_SRC_FSPI_NOR:
+ return BOOT_SOURCE_SPI;
+ default:
+ debug("unknown bootsource (%08x)\n", rcw_src);
+ return BOOT_SOURCE_UNKNOWN;
+ }
+}
diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
index 32e9694b77..54719cc01c 100644
--- a/board/kontron/sl28/sl28.c
+++ b/board/kontron/sl28/sl28.c
@@ -24,6 +24,8 @@
#include
#include
+#include "sl28.h"
+
DECLARE_GLOBAL_DATA_PTR;
#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
@@ -60,6 +62,27 @@ int board_eth_init(struct bd_info *bis)
return pci_eth_init(bis);
}
+enum env_location env_get_location(enum env_operation op, int prio)
+{
+ enum boot_source src = sl28_boot_source();
+
+ if (prio)
+ return ENVL_UNKNOWN;
+
+ if (!CONFIG_IS_ENABLED(ENV_IS_IN_SPI_FLASH))
+ return ENVL_NOWHERE;
+
+ /* write and erase always operate on the environment */
+ if (op == ENVOP_SAVE || op == ENVOP_ERASE)
+ return ENVL_SPI_FLASH;
+
+ /* failsafe boot will always use the compiled-in default environment */
+ if (src == BOOT_SOURCE_SPI)
+ return ENVL_NOWHERE;
+
+ return ENVL_SPI_FLASH;
+}
+
static int __sl28cpld_read(uint reg)
{
struct udevice *dev;
diff --git a/board/kontron/sl28/sl28.h b/board/kontron/sl28/sl28.h
new file mode 100644
index 00..7f0105049c
--- /dev/null
+++ b/board/kontron/sl28/sl28.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __SL28_H
+#define __SL28_H
+
+enum boot_source {
+ BOOT_SOURCE_UNKNOWN,
+ BOOT_SOURCE_SDHC,
+ BOOT_SOURCE_MMC,
+ BOOT_SOURCE_I2C,
+ BOOT_SOURCE_SPI,
+};
+
+enum boot_source sl28_boot_source(void);
+
+#endif
diff --git a/board/kontron/sl28/spl.c b/board/kontron/sl28/spl.c
index 0e6ad5f37e..b1fefc22b2 100644
--- a/board/kontron/sl28/spl.c
+++ b/board/kontron/sl28/spl.c
@@ -5,6 +5,9 @@
#include
#include
#include
+#include
+
+#include "sl28.h"
#define DCFG_RCWSR25 0x160
#define GPINFO_HW_VARIANT_MASK 0xff
@@ -58,7 +61,40 @@ int board_fit_config_name_match(const char *name)
void board_boot_order(u32 *spl_boot_list)
{
- spl_boot_list[0] = BOOT_DEVICE_SPI;
+ enum boot_source src = sl28_boot_source();
+
+ switch (src) {
+ case BOOT_SOURCE_SDHC:
+ spl_boot_list[0] = BOOT_DEVICE_MMC2;
+ break;
+ case BOOT_SOURCE_SPI:
+ case BOOT_SOURCE_I2C:
+ spl_boot_list[0] = BOOT_DEVICE_SPI;
+ break;
+ case BOOT_SOURCE_MMC:
+ spl_boot_list[0] = BOOT_DEVICE_MMC1;
+ break;
+ default:
+ panic("unexpected bootsource (%d)\n", src);
+ break;
+ }
+}
+
+unsigned int spl_spi_get_uboot_offs(struct spi_flash *flash)
+{
+ enum boot_source src = sl28_boot_source();
+
+ switch (src) {
+ case BOOT_SOURCE_SPI:
+ return 0x00;
+ case BOOT_SOURCE_I2C:
+ return 0x23;
+ default:
+ panic("unexpected bootsource (%d)\n", src);
+ break;
+ }
+}
+
}
int board_early_init_f(void)
diff --git a/configs/kontron_sl28_defconfig b/configs/kontron_sl28_defconfig
index b0b5fb11ca..4d50c681f9 100644
--- a/configs/kontron_sl28_defconfig
+++ b/configs/kontron_sl28_defconfig
@@ -12,6 +12,7 @@ CONFIG_ENV_SECT_SIZE=0x1
CONFIG_DEFAULT_DEVICE_TREE="fsl-ls1028a-kontron-sl28"
C
[PATCH v3 1/5] armv8: layerscape: spl: mark OCRAM as non-secure
By default the OCRAM is marked as secure. While the SPL runs in EL3 and
thus can access it, DMA devices cannot. Mark the whole OCRAM as
non-secure.
This will fix MMC and SD card boot on LS1028A when using SPL instead of
TF-A.
Signed-off-by: Michael Walle
---
arch/arm/cpu/armv8/fsl-layerscape/spl.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
index 5f09ef0a4a..3a4b665f24 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/spl.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/spl.c
@@ -67,11 +67,24 @@ void spl_board_init(void)
#endif
}
+void tzpc_init(void)
+{
+ /*
+* Mark the whole OCRAM as non-secure, otherwise DMA devices cannot
+* access it. This is for example necessary for MMC boot.
+*/
+#ifdef TZPCR0SIZE_BASE
+ out_le32(TZPCR0SIZE_BASE, 0);
+#endif
+}
+
void board_init_f(ulong dummy)
{
int ret;
icache_enable();
+ tzpc_init();
+
/* Clear global data */
memset((void *)gd, 0, sizeof(gd_t));
if (IS_ENABLED(CONFIG_DEBUG_UART))
--
2.30.2
[PATCH v3 0/5] board: sl28: various updates
This is a resend of the original v1 series. It was just rebased. Apparently not all patches made it into u-boot. Unfortunately, there was no feedback at all. This is an update for the sl28 board which adds support for - 8 GiB memory variant - different boot sources, like eMMC, SD-card - dynamic prompts - various cleanups changes since v2: - only mark secure ram on layerscape SoCs which actually have it. changes since v1: - rebased onto the latest master Michael Walle (5): armv8: layerscape: spl: mark OCRAM as non-secure board: sl28: implement additional bootsources board: sl28: add user friendly names for the boot sources board: sl28: support dynamic prompts board: sl28: remove COUNTER_FREQUENCY_REAL arch/arm/cpu/armv8/fsl-layerscape/spl.c | 13 ++ board/kontron/sl28/common.c | 22 ++ board/kontron/sl28/sl28.c | 43 board/kontron/sl28/sl28.h | 16 board/kontron/sl28/spl.c| 54 - configs/kontron_sl28_defconfig | 6 ++- include/configs/kontron_sl28.h | 2 - 7 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 board/kontron/sl28/sl28.h -- 2.30.2
Re: [GIT PULL] please pull fsl-qoriq-2022-8-17
>On 8/18/2022 6:40 AM, Tom Rini wrote: >> On Wed, Aug 17, 2022 at 02:26:32AM +, Peng Fan (OSS) wrote: >> >>> Hi Tom, >>> >>> Please pull fsl-qoriq-2022-8-17 >>> >>> CI: >>> https://source.denx.de/u-boot/custodians/u-boot-fsl-qoriq/-/pipelines/13155 >> >> First, applied to u-boot/master, thanks! Second, do you have any comment >> on the various layerscape platforms patches that have been posted of >> late? I plan to for example apply the ones to disable ethernet on some >> layerscape platforms soon. > > I am ok. Please directly apply those patches for now. I'll find someone > to enable DM_ETH for LS platform later and enable ethernet then. Speaking of, I'd be glad if someone could finally pull in these patches: https://patchwork.ozlabs.org/project/uboot/list/?series=309627 They idle on the list for over a month now (again!). The original series was https://patchwork.ozlabs.org/project/uboot/list/?series=302570&state=%2A&archive=both Which was posted in May, some patches made it through, some don't, not sure why though because there was never any feedback. Thanks, -michael
[PATCH 14/22] board: lsxl: use proper *_r variables
Use the common kernel_addr_r, ramdisk_addr_r and fdt_addr_r variable
names.
Signed-off-by: Michael Walle
---
include/configs/lsxl.h | 42 +-
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/include/configs/lsxl.h b/include/configs/lsxl.h
index 7c2c0e22ad..162f07790f 100644
--- a/include/configs/lsxl.h
+++ b/include/configs/lsxl.h
@@ -25,31 +25,31 @@
#define CONFIG_EXTRA_ENV_SETTINGS \
"bootsource=legacy\0" \
"hdpart=0:1\0" \
- "kernel_addr=0x0080\0" \
- "ramdisk_addr=0x0100\0" \
- "fdt_addr=0x00ff\0" \
+ "kernel_addr_r=0x0080\0"\
+ "ramdisk_addr_r=0x0100\0" \
+ "fdt_addr_r=0x00ff\0" \
"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
"bootcmd_legacy=sata init " \
- "&& load sata ${hdpart} ${kernel_addr} /uImage.buffalo "\
- "&& load sata ${hdpart} ${ramdisk_addr} /initrd.buffalo "\
- "&& bootm ${kernel_addr} ${ramdisk_addr}\0" \
- "bootcmd_net=bootp ${kernel_addr} vmlinuz " \
- "&& tftpboot ${fdt_addr} ${fdtfile} " \
- "&& tftpboot ${ramdisk_addr} initrd.img " \
- "&& bootz ${kernel_addr} " \
- "${ramdisk_addr}:${filesize} ${fdt_addr}\0" \
+ "&& load sata ${hdpart} ${kernel_addr_r} /uImage.buffalo "\
+ "&& load sata ${hdpart} ${ramdisk_addr_r} /initrd.buffalo "\
+ "&& bootm ${kernel_addr_r} ${ramdisk_addr_r}\0" \
+ "bootcmd_net=bootp ${kernel_addr_r} vmlinuz " \
+ "&& tftpboot ${fdt_addr_r} ${fdtfile} " \
+ "&& tftpboot ${ramdisk_addr_r} initrd.img " \
+ "&& bootz ${kernel_addr_r} "\
+ "${ramdisk_addr_r}:${filesize} ${fdt_addr_r}\0" \
"bootcmd_hdd=sata init "\
- "&& load sata ${hdpart} ${kernel_addr} /vmlinuz " \
- "&& load sata ${hdpart} ${fdt_addr} /dtb " \
- "&& load sata ${hdpart} ${ramdisk_addr} /initrd.img " \
- "&& bootz ${kernel_addr} " \
- "${ramdisk_addr}:${filesize} ${fdt_addr}\0" \
+ "&& load sata ${hdpart} ${kernel_addr_r} /vmlinuz " \
+ "&& load sata ${hdpart} ${fdt_addr_r} /dtb "\
+ "&& load sata ${hdpart} ${ramdisk_addr_r} /initrd.img " \
+ "&& bootz ${kernel_addr_r} "\
+ "${ramdisk_addr_r}:${filesize} ${fdt_addr_r}\0" \
"bootcmd_usb=usb start "\
- "&& load usb 0:1 ${kernel_addr} /vmlinuz " \
- "&& load usb 0:1 ${fdt_addr} ${fdtfile} " \
- "&& load usb 0:1 ${ramdisk_addr} /initrd.img " \
- "&& bootz ${kernel_addr} " \
- "${ramdisk_addr}:${filesize} ${fdt_addr}\0" \
+ "&& load usb 0:1 ${kernel_addr_r} /vmlinuz "\
+ "&& load usb 0:1 ${fdt_addr_r} ${fdtfile} " \
+ "&& load usb 0:1 ${ramdisk_addr_r} /initrd.img "\
+ "&& bootz ${kernel_addr_r} "\
+ "${ramdisk_addr_r}:${filesize} ${fdt_addr_r}\0" \
"bootcmd_rescue=run config_nc_dhcp; run nc\0" \
"config_nc_dhcp=setenv autoload_old ${autoload}; " \
"setenv autoload no " \
--
2.30.2
[PATCH 18/22] board: lsxl: convert to DM_ETH
Just enabling the Kconfig option for DM_ETH and DM_MDIO is enough.
Additionally, we can remove the old hardcoded config.
Signed-off-by: Michael Walle
---
configs/lschlv2_defconfig | 2 ++
configs/lsxhl_defconfig | 2 ++
include/configs/lsxl.h| 8
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/configs/lschlv2_defconfig b/configs/lschlv2_defconfig
index 2146a33276..4e356fb150 100644
--- a/configs/lschlv2_defconfig
+++ b/configs/lschlv2_defconfig
@@ -61,6 +61,8 @@ CONFIG_MTD=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=2500
CONFIG_SPI_FLASH_STMICRO=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
CONFIG_MVGBE=y
CONFIG_MII=y
CONFIG_DM_REGULATOR=y
diff --git a/configs/lsxhl_defconfig b/configs/lsxhl_defconfig
index d8953128e0..6de0322bfa 100644
--- a/configs/lsxhl_defconfig
+++ b/configs/lsxhl_defconfig
@@ -62,6 +62,8 @@ CONFIG_MTD=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=2500
CONFIG_SPI_FLASH_STMICRO=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
CONFIG_MVGBE=y
CONFIG_MII=y
CONFIG_DM_REGULATOR=y
diff --git a/include/configs/lsxl.h b/include/configs/lsxl.h
index 4d5908d236..c82eb8b04b 100644
--- a/include/configs/lsxl.h
+++ b/include/configs/lsxl.h
@@ -59,12 +59,4 @@
"setenv autoload_old\0" \
"nc=setenv stdin nc; setenv stdout nc; setenv stderr nc\0" \
-/*
- * Ethernet Driver configuration
- */
-#ifdef CONFIG_CMD_NET
-#define CONFIG_MVGBE_PORTS {0, 1} /* enable port 1 only */
-#define CONFIG_PHY_BASE_ADR7
-#endif /* CONFIG_CMD_NET */
-
#endif /* _CONFIG_LSXL_H */
--
2.30.2
