Re: [PATCH] include/linux/kernel.h: fix mult_frac() multiple argument evaluation bug

2024-05-15 Thread Sascha Hauer


On Wed, 15 May 2024 13:27:03 +0200, Ahmad Fatoum wrote:
> This is a port of Linux commit 048a9883267f9b8f8e05dca9e9e8e6f991eea61e:
> 
> | Author: Alexey Dobriyan 
> | AuthorDate: Sat May 20 21:25:19 2023 +0300
> |
> | include/linux/math.h: fix mult_frac() multiple argument evaluation bug
> |
> | mult_frac() evaluates _all_ arguments multiple times in the body.
> |
> | Clarify comment while I'm at it.
> |
> | Link: https://lkml.kernel.org/r/f9f9fdbb-ec8e-4f5e-a998-2a58627a1a43@p183
> | Signed-off-by: Alexey Dobriyan 
> | Signed-off-by: Andrew Morton 
> 
> [...]

Applied, thanks!

[1/1] include/linux/kernel.h: fix mult_frac() multiple argument evaluation bug
  https://git.pengutronix.de/cgit/barebox/commit/?id=3934f477dc82 (link may 
not be stable)

Best regards,
-- 
Sascha Hauer 




Re: [PATCH] ARM: dts: GoMe e143_01: fix default state priorities

2024-05-15 Thread Sascha Hauer


On Wed, 15 May 2024 15:16:08 +0200, Roland Hieber wrote:
> During the initial installation we can forgo writing an initial state
> variable set to the eMMC if it is zeroed, and if the default values
> priorities are set correctly. Since only the first rootfs partition is
> populated during the install process, it should be preferred over the
> second rootfs.
> 
> 
> [...]

Applied, thanks!

[1/1] ARM: dts: GoMe e143_01: fix default state priorities
  https://git.pengutronix.de/cgit/barebox/commit/?id=a657866d377b (link may 
not be stable)

Best regards,
-- 
Sascha Hauer 




Re: [PATCH] fs: ramfs: allocate once instead of twice per ramfs chunk

2024-05-15 Thread Sascha Hauer


On Wed, 15 May 2024 17:24:10 +0200, Ahmad Fatoum wrote:
> There's no reason to maintain two allocations per chunk, so just collect
> them both into the same calloc call.
> 
> No functional change.
> 
> 

Applied, thanks!

[1/1] fs: ramfs: allocate once instead of twice per ramfs chunk
  https://git.pengutronix.de/cgit/barebox/commit/?id=6e502a0ca8d9 (link may 
not be stable)

Best regards,
-- 
Sascha Hauer 




[PATCH] fs: ramfs: allocate once instead of twice per ramfs chunk

2024-05-15 Thread Ahmad Fatoum
There's no reason to maintain two allocations per chunk, so just collect
them both into the same calloc call.

No functional change.

Signed-off-by: Ahmad Fatoum 
---
I didn't test thoroughly what performance improvement this might bring,
but it looks like a sensible thing to do.
---
 fs/ramfs.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/ramfs.c b/fs/ramfs.c
index 117e69b70c0c..3223beba7212 100644
--- a/fs/ramfs.c
+++ b/fs/ramfs.c
@@ -28,10 +28,10 @@
 #define CHUNK_SIZE (4096 * 2)
 
 struct ramfs_chunk {
-   char *data;
unsigned long ofs;
int size;
struct list_head list;
+   char data[];
 };
 
 struct ramfs_inode {
@@ -98,19 +98,14 @@ static struct inode *ramfs_get_inode(struct super_block 
*sb, const struct inode
 
 static struct ramfs_chunk *ramfs_get_chunk(unsigned long size)
 {
-   struct ramfs_chunk *data = malloc(sizeof(struct ramfs_chunk));
-
-   if (!data)
-   return NULL;
+   struct ramfs_chunk *data;
 
if (size < MIN_SIZE)
size = MIN_SIZE;
 
-   data->data = calloc(size, 1);
-   if (!data->data) {
-   free(data);
+   data = calloc(struct_size(data, data, size), 1);
+   if (!data)
return NULL;
-   }
 
data->size = size;
 
@@ -119,7 +114,6 @@ static struct ramfs_chunk *ramfs_get_chunk(unsigned long 
size)
 
 static void ramfs_put_chunk(struct ramfs_chunk *data)
 {
-   free(data->data);
free(data);
 }
 
-- 
2.39.2




[PATCH] ARM: dts: GoMe e143_01: fix default state priorities

2024-05-15 Thread Roland Hieber
During the initial installation we can forgo writing an initial state
variable set to the eMMC if it is zeroed, and if the default values
priorities are set correctly. Since only the first rootfs partition is
populated during the install process, it should be preferred over the
second rootfs.

Signed-off-by: Roland Hieber 
---
 arch/arm/dts/imx7d-gome-e143_01.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/imx7d-gome-e143_01.dts 
b/arch/arm/dts/imx7d-gome-e143_01.dts
index ea118ddc76f7..ed7f3cfa216b 100644
--- a/arch/arm/dts/imx7d-gome-e143_01.dts
+++ b/arch/arm/dts/imx7d-gome-e143_01.dts
@@ -50,7 +50,7 @@ remaining_attempts@0 {
priority@4 {
reg = <0x4 0x4>;
type = "uint32";
-   default = <10>;
+   default = <20>;
};
};
 
@@ -67,7 +67,7 @@ remaining_attempts@8 {
priority@c {
reg = <0xc 0x4>;
type = "uint32";
-   default = <20>;
+   default = <10>;
};
};
 
-- 
2.39.2




Re: [PATCH v2] ARM64: reloc: fix relocation error for big fat bareboxes

2024-05-15 Thread Ahmad Fatoum
Hi,

On 15.05.24 07:55, Sascha Hauer wrote:
> 
> On Mon, 13 May 2024 16:01:21 +0200, Ahmad Fatoum wrote:
>> A multi_v8 barebox with KASAN enabled is 2051804 bytes even after
>> compression and this breaks linking for me:
>>
>>   arch/arm/cpu/common.o: in function `global_variable_offset':
>>   arch/arm/include/asm/reloc.h:20:(.text.relocate_to_current_adr+0x1c):
>> relocation truncated to fit: R_AARCH64_ADR_PREL_LO21 against symbol
>> `_text' defined in .text section in .tmp_barebox1
>>   arch/arm/include/asm/reloc.h:20:(.text.relocate_to_current_adr+0x40):
>> relocation truncated to fit: R_AARCH64_ADR_PREL_LO21 against symbol
>> `_text' defined in .text section in .tmp_barebox1
>>
>> [...]
> 
> Applied, thanks!

Thanks for applying. I thought some more about whether we should instead
have:

#ifdef __PBL__
"adr%0, _text\n"
#else
/* (Decompressed) barebox proper should always be 4K aligned
 * so adrp here should be fine. PBL may also have adrp
 * references
 */
"adrp   %0, _text\n"
"add%0, %0, :lo12:_text\n"
#endif

Otherwise, we require PBL to be placed 4K-aligned. Looking at GCC 13.2.1
output for an i.MX8M board, there are quite a lot of adrp references already,
so I think this shouldn't break anything that's not broken already anyway.

Just writing my thoughts for future reference.

Cheers,
Ahmad

> 
> [1/1] ARM64: reloc: fix relocation error for big fat bareboxes
>   https://git.pengutronix.de/cgit/barebox/commit/?id=9246c916a25a (link 
> may not be stable)
> 
> Best regards,

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |




[PATCH] include/linux/kernel.h: fix mult_frac() multiple argument evaluation bug

2024-05-15 Thread Ahmad Fatoum
This is a port of Linux commit 048a9883267f9b8f8e05dca9e9e8e6f991eea61e:

| Author: Alexey Dobriyan 
| AuthorDate: Sat May 20 21:25:19 2023 +0300
|
| include/linux/math.h: fix mult_frac() multiple argument evaluation bug
|
| mult_frac() evaluates _all_ arguments multiple times in the body.
|
| Clarify comment while I'm at it.
|
| Link: https://lkml.kernel.org/r/f9f9fdbb-ec8e-4f5e-a998-2a58627a1a43@p183
| Signed-off-by: Alexey Dobriyan 
| Signed-off-by: Andrew Morton 

Signed-off-by: Ahmad Fatoum 
---
 include/linux/kernel.h | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index c411ac0860dd..cd7dac73f93a 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -107,17 +107,17 @@ extern long long simple_strtoll(const char *,char 
**,unsigned int);
 }  \
 )
 
-/*
- * Multiplies an integer by a fraction, while avoiding unnecessary
- * overflow or loss of precision.
- */
-#define mult_frac(x, numer, denom)(\
-{  \
-   typeof(x) quot = (x) / (denom); \
-   typeof(x) rem  = (x) % (denom); \
-   (quot * (numer)) + ((rem * (numer)) / (denom)); \
-}  \
-)
+/* Calculate "x * n / d" without unnecessary overflow or loss of precision. */
+#define mult_frac(x, n, d) \
+({ \
+   typeof(x) x_ = (x); \
+   typeof(n) n_ = (n); \
+   typeof(d) d_ = (d); \
+   \
+   typeof(x_) q = x_ / d_; \
+   typeof(x_) r = x_ % d_; \
+   q * n_ + r * n_ / d_;   \
+})
 
 extern const char hex_asc[];
 #define hex_asc_lo(x)  hex_asc[((x) & 0x0f)]
-- 
2.39.2




[PATCH] of: move ramoops device creation into common code

2024-05-15 Thread Ahmad Fatoum
We already have a generic place where we check for reserved memory
matches, so add ramoops there as well.

Signed-off-by: Ahmad Fatoum 
---
 drivers/of/base.c |  1 +
 fs/pstore/ram.c   | 15 ---
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 3b8878f34be3..2213165fd72d 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2735,6 +2735,7 @@ static void of_platform_device_create_root(struct 
device_node *np)
 }
 
 static const struct of_device_id reserved_mem_matches[] = {
+   { .compatible = "ramoops" },
{ .compatible = "nvmem-rmem" },
{}
 };
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 9ecf7ef5e901..4cdeca904fad 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -685,21 +685,6 @@ static struct driver ramoops_driver = {
 
 static int __init ramoops_init(void)
 {
-   if (IS_ENABLED(CONFIG_OFTREE)) {
-   struct device_node *node;
-
-   node = of_get_root_node();
-   if (!node)
-   return 0;
-
-   node = of_get_child_by_name(node, "reserved-memory");
-   if (!node)
-   return 0;
-
-   for_each_matching_node(node, ramoops_dt_ids)
-   of_platform_device_create(node, NULL);
-   }
-
ramoops_register_dummy();
return platform_driver_register(_driver);
 }
-- 
2.39.2




[PATCH 2/2] ARM: i.MX8MP: skov: update timing parameters for Samsung RAM

2024-05-15 Thread Ahmad Fatoum
From: Soren Andersen 

The RAM is to be operated at a slightly higher data rate of 3200 MT/s,
hence the timings have to be adjusted.

Signed-off-by: Soren Andersen 
Signed-off-by: Ulrich Ölmann 
Signed-off-by: Ahmad Fatoum 
---
 arch/arm/boards/skov-imx8mp/lpddr4-timing.c | 46 ++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm/boards/skov-imx8mp/lpddr4-timing.c 
b/arch/arm/boards/skov-imx8mp/lpddr4-timing.c
index a93506b0bdeb..96882910a2df 100644
--- a/arch/arm/boards/skov-imx8mp/lpddr4-timing.c
+++ b/arch/arm/boards/skov-imx8mp/lpddr4-timing.c
@@ -15,29 +15,29 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d400030, 0x1 },
{ 0x3d40, 0xa1080020 },
{ 0x3d400020, 0x1223 },
-   { 0x3d400024, 0x16e3600 },
-   { 0x3d400064, 0x5b00d2 },
+   { 0x3d400024, 0x186a000 },
+   { 0x3d400064, 0x6100e0 },
{ 0x3d400070, 0x7027f90 },
{ 0x3d400074, 0x790 },
-   { 0x3d4000d0, 0xc00305ba },
-   { 0x3d4000d4, 0x94 },
+   { 0x3d4000d0, 0xc003061c },
+   { 0x3d4000d4, 0x9e },
{ 0x3d4000dc, 0xd4002d },
{ 0x3d4000e0, 0x31 },
{ 0x3d4000e8, 0x660048 },
{ 0x3d4000ec, 0x160048 },
-   { 0x3d400100, 0x191e1920 },
-   { 0x3d400104, 0x60630 },
-   { 0x3d40010c, 0xb0b000 },
-   { 0x3d400110, 0xe04080e },
+   { 0x3d400100, 0x1a201b22 },
+   { 0x3d400104, 0x60633 },
+   { 0x3d40010c, 0xc0c000 },
+   { 0x3d400110, 0xf04080f },
{ 0x3d400114, 0x2040c0c },
{ 0x3d400118, 0x1010007 },
{ 0x3d40011c, 0x402 },
{ 0x3d400130, 0x20600 },
{ 0x3d400134, 0xc12 },
-   { 0x3d400138, 0xd8 },
-   { 0x3d400144, 0x96004b },
-   { 0x3d400180, 0x2ee0017 },
-   { 0x3d400184, 0x2605b8e },
+   { 0x3d400138, 0xe6 },
+   { 0x3d400144, 0xa00050 },
+   { 0x3d400180, 0x3200018 },
+   { 0x3d400184, 0x28061a8 },
{ 0x3d400188, 0x0 },
{ 0x3d400190, 0x497820a },
{ 0x3d400194, 0x80303 },
@@ -270,7 +270,7 @@ static struct dram_cfg_param ddr_ddrphy_cfg[] = {
{ 0x20018, 0x3 },
{ 0x20075, 0x4 },
{ 0x20050, 0x0 },
-   { 0x20008, 0x2ee },
+   { 0x20008, 0x320 },
{ 0x120008, 0x64 },
{ 0x220008, 0x19 },
{ 0x20088, 0x9 },
@@ -335,7 +335,7 @@ static struct dram_cfg_param ddr_ddrphy_cfg[] = {
 /* P0 message block paremeter for training firmware */
 static struct dram_cfg_param ddr_fsp0_cfg[] = {
{ 0xd, 0x0 },
-   { 0x54003, 0xbb8 },
+   { 0x54003, 0xc80 },
{ 0x54004, 0x2 },
{ 0x54005, 0x2228 },
{ 0x54006, 0x14 },
@@ -457,7 +457,7 @@ static struct dram_cfg_param ddr_fsp2_cfg[] = {
 /* P0 2D message block paremeter for training firmware */
 static struct dram_cfg_param ddr_fsp0_2d_cfg[] = {
{ 0xd, 0x0 },
-   { 0x54003, 0xbb8 },
+   { 0x54003, 0xc80 },
{ 0x54004, 0x2 },
{ 0x54005, 0x2228 },
{ 0x54006, 0x14 },
@@ -976,9 +976,9 @@ static struct dram_cfg_param ddr_phy_pie[] = {
{ 0x400d7, 0x20b },
{ 0x2003a, 0x2 },
{ 0x200be, 0x3 },
-   { 0x2000b, 0x34b },
-   { 0x2000c, 0xbb },
-   { 0x2000d, 0x753 },
+   { 0x2000b, 0x384 },
+   { 0x2000c, 0xc8 },
+   { 0x2000d, 0x7d0 },
{ 0x2000e, 0x2c },
{ 0x12000b, 0x70 },
{ 0x12000c, 0x19 },
@@ -1081,8 +1081,8 @@ static struct dram_cfg_param ddr_phy_pie[] = {
 
 static struct dram_fsp_msg ddr_dram_fsp_msg[] = {
{
-   /* P0 3000mts 1D */
-   .drate = 3000,
+   /* P0 3200mts 1D */
+   .drate = 3200,
.fw_type = FW_1D_IMAGE,
.fsp_cfg = ddr_fsp0_cfg,
.fsp_cfg_num = ARRAY_SIZE(ddr_fsp0_cfg),
@@ -1102,8 +1102,8 @@ static struct dram_fsp_msg ddr_dram_fsp_msg[] = {
.fsp_cfg_num = ARRAY_SIZE(ddr_fsp2_cfg),
},
{
-   /* P0 3000mts 2D */
-   .drate = 3000,
+   /* P0 3200mts 2D */
+   .drate = 3200,
.fw_type = FW_2D_IMAGE,
.fsp_cfg = ddr_fsp0_2d_cfg,
.fsp_cfg_num = ARRAY_SIZE(ddr_fsp0_2d_cfg),
@@ -1120,6 +1120,6 @@ struct dram_timing_info imx8mp_skov_dram_timing = {
.fsp_msg_num = ARRAY_SIZE(ddr_dram_fsp_msg),
.ddrphy_pie = ddr_phy_pie,
.ddrphy_pie_num = ARRAY_SIZE(ddr_phy_pie),
-   .fsp_table = { 3000, 400, 100, },
+   .fsp_table = { 3200, 400, 100, },
 };
 
-- 
2.39.2




[PATCH 1/2] ARM: i.MX8MP: skov: fix variant detection on boards without state

2024-05-15 Thread Ahmad Fatoum
From: Soren Andersen 

If barebox booted a board without state partition on the eMMC it did not set a
refined compatible at all resulting in a failure to boot into Linux userspace.

Instead just default to the LVDS flavor of the board variant which at least
leads to a working Linux userspace.

Signed-off-by: Soren Andersen 
Signed-off-by: Ulrich Ölmann 
Reviewed-by: Oleksij Rempel 
Signed-off-by: Ahmad Fatoum 
---
 arch/arm/boards/skov-imx8mp/board.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm/boards/skov-imx8mp/board.c 
b/arch/arm/boards/skov-imx8mp/board.c
index 3cb7a8752a27..ce8d154a7355 100644
--- a/arch/arm/boards/skov-imx8mp/board.c
+++ b/arch/arm/boards/skov-imx8mp/board.c
@@ -186,10 +186,7 @@ static int skov_imx8mp_init_variant(struct 
skov_imx8mp_priv *priv)
 
if (variant->flags & SKOV_IMX8MP_HAS_HDMI) {
ret = skov_imx8mp_get_hdmi(dev);
-   if (ret < 0)
-   return ret;
-
-   if (ret)
+   if (ret == 1)
compatible = variant->dts_compatible_hdmi;
else
compatible = variant->dts_compatible;
-- 
2.39.2




[PATCH] Documentation: pstore: add some information about interfacing with Linux

2024-05-15 Thread Ahmad Fatoum
The help text erroneously states that CONFIG_FS_PSTORE_RAMOOPS_RO
disables writing completely, but this is not correct: It merely stops
barebox from zapping log content previously written by Linux.

The Kconfig help text is correct, so adjust the documentation to reflect
that and while at it, add some more information about how to access
pstore in Linux.

Signed-off-by: Ahmad Fatoum 
---
 Documentation/filesystems/pstore.rst | 29 +++-
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/filesystems/pstore.rst 
b/Documentation/filesystems/pstore.rst
index b8c2cb825ced..e16c4f35e704 100644
--- a/Documentation/filesystems/pstore.rst
+++ b/Documentation/filesystems/pstore.rst
@@ -73,11 +73,30 @@ in the Barebox shell. The RAMOOPS area is listed as 
'persistent ram':
 0x2fee4000 - 0x2fee7fff (size 0x4000) ttb
 0x2fee8000 - 0x2fee (size 0x8000) stack
 
-All pstore files that could be found are added to the /pstore directory. This 
is
-a read-only filesystem. If you disable the Kconfig option FS_PSTORE_RAMOOPS_RO,
-the RAMOOPS area is reset and its ECC recalculated. But that does not allow any
-writes from Barebox into that area.
-
 If the menu entry ``FS_PSTORE_CONSOLE`` is enabled, Barebox itself will add all
 its own console output to the *ramoops:console* part, which enables the regular
 userland later on to have access to the bootloaders output.
+
+All pstore files that could be found are added to the /pstore directory. This 
is
+a read-only filesystem with the only supported operation being unlinking:
+This resets (zaps) the RAMOOPS area and recalculates the ECC.
+
+Zapping is done automatically, if the menu entry ``FS_PSTORE_RAMOOPS_RO`` is
+disabled. In this case, only the barebox log will be available to the kernel
+and ramoops from previous boots will not survive.
+
+The usual setup is to not zap any buffers, i.e. 
``CONFIG_FS_PSTORE_RAMOOPS_RO=y``
+and no manual unlinking of files in ``/pstore``.
+
+In Linux, the pstore file system is often mounted at ``/sys/fs/pstore``.
+This will often not contain older output as Linux frequently zaps ramoops
+buffers to conserve the limited space.
+
+To counteract this, userland needs to archive the pstore contents
+after booting into Linux and then clear pstore. Systemd has built-in support
+for this when compiled with ``-Dpstore=true``.
+
+Logs (including barebox log messages if enabled) will then be written to
+journal by default and are accessible via::
+
+  journalctl -b -o verbose -a -t systemd-pstore
-- 
2.39.2




[PATCH] watchdog: imxwd: Do not suspend in lpm on i.MX27

2024-05-15 Thread Sascha Hauer
The i.MX watchdog has two different configurable behaviours for low
power modes.  The watchdog can either be suspended during low power
modes or kept running.
The useful behaviour is normally to disable it during low power modes to
be able to suspend the system without having the watchdog resetting the
system. This setting was introduced in the Kernel back in 2014 [1] and
in barebox back in 2021 [2].
On i.MX27 however this setting has the effect that the watchdog is
suspended during normal cpu_do_idle(), so on an idle system it takes
very long time until the watchdog triggers. This renders this setting
useless, so disable it on i.MX27. This seems to be fixed on SoCs newer
than i.MX35.

[1] 1a9c5efa576ec ("watchdog: imx2_wdt: disable watchdog timer during low power 
mode")
[2] c3787a81f26bb ("watchdog: imxwd: suspend watchdog timer in low power mode")

Signed-off-by: Sascha Hauer 
---
 drivers/watchdog/imxwd.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/imxwd.c b/drivers/watchdog/imxwd.c
index 8f4de5a994..616248af38 100644
--- a/drivers/watchdog/imxwd.c
+++ b/drivers/watchdog/imxwd.c
@@ -33,6 +33,7 @@ struct imx_wd {
struct restart_handler restart_warm;
bool ext_reset;
bool bigendian;
+   bool suspend_in_lpm;
 };
 
 #define to_imx_wd(h) container_of(h, struct imx_wd, wd)
@@ -127,7 +128,8 @@ static int imx21_watchdog_set_timeout(struct imx_wd *priv, 
unsigned timeout)
val |= IMX21_WDOG_WCR_WDT;
 
/* Suspend timer in low power mode */
-   val |= IMX21_WDOG_WCR_WDZST;
+   if (priv->suspend_in_lpm)
+   val |= IMX21_WDOG_WCR_WDZST;
 
/*
 * set time and some write once bits first prior enabling the
@@ -273,6 +275,7 @@ static int imx_wd_probe(struct device *dev)
 
priv->ext_reset = of_property_read_bool(dev->of_node,
"fsl,ext-reset-output");
+   priv->suspend_in_lpm = !of_machine_is_compatible("fsl,imx27");
 
if (IS_ENABLED(CONFIG_WATCHDOG_IMX)) {
if (priv->ops->is_running) {
-- 
2.39.2




[PATCH] ddr: imx: rename ddr_get_firmware_ddr to ddr_get_firmware_ddr4

2024-05-15 Thread Ahmad Fatoum
The omission of the 4 was likely a typo. Adding it makes it clearer what
the function does and serves symmetry with ddr_get_firmware_lpddr4().

Signed-off-by: Ahmad Fatoum 
---
 drivers/ddr/imx/ddrphy_train.c | 2 +-
 include/soc/imx/ddr.h  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ddr/imx/ddrphy_train.c b/drivers/ddr/imx/ddrphy_train.c
index d599445e6f57..24997a6f3667 100644
--- a/drivers/ddr/imx/ddrphy_train.c
+++ b/drivers/ddr/imx/ddrphy_train.c
@@ -40,7 +40,7 @@ static size_t ddr4_imem_2d_size;
 static const u16 *ddr4_dmem_2d;
 static size_t ddr4_dmem_2d_size;
 
-void ddr_get_firmware_ddr(void)
+void ddr_get_firmware_ddr4(void)
 {
get_builtin_firmware(ddr4_imem_1d_bin, _imem_1d,
 _imem_1d_size);
diff --git a/include/soc/imx/ddr.h b/include/soc/imx/ddr.h
index caf33d3e34b9..1e64613895d1 100644
--- a/include/soc/imx/ddr.h
+++ b/include/soc/imx/ddr.h
@@ -114,14 +114,14 @@ struct dram_controller {
 };
 
 void ddr_get_firmware_lpddr4(void);
-void ddr_get_firmware_ddr(void);
+void ddr_get_firmware_ddr4(void);
 
 static inline void ddr_get_firmware(enum dram_type dram_type)
 {
if (dram_type == DRAM_TYPE_LPDDR4)
ddr_get_firmware_lpddr4();
else
-   ddr_get_firmware_ddr();
+   ddr_get_firmware_ddr4();
 }
 
 int ddr_cfg_phy(struct dram_controller *dram, struct dram_timing_info 
*timing_info);
-- 
2.39.2




[PATCH master v2 0/3] Fixes for WolfVision board code library and PF5 mainboard code

2024-05-15 Thread Michael Riesch
Habidere,

This series is a follow-up to 
https://lore.barebox.org/barebox/20240412-feature-wolfvision-pf5-v2-0-7e277cc88...@wolfvision.net/
and fixes a few things. I tried to reply to said thread and announce a
v3, but I just found that this reply never hit the list for ... reasons.

Anyway, here are the fixes. Do you think they can be applied directly
to master?

Looking forward to your comments!

Signed-off-by: Michael Riesch 
---
Changes in v2:
- replace basprintf() with xstrdup() to avoid compiler warning
- Link to v1: 
https://lore.kernel.org/r/20240515-b4-pf5-fixup-v1-0-a58c8ec94...@wolfvision.net

---
Michael Riesch (3):
  arm: boards: add pr_fmt() prefix to wolfvision pf5 board code
  common: boards: move dependencies to wolfvision board code library
  common: boards: wolfvision: fix handling of overlays parameter

 arch/arm/boards/wolfvision-pf5/board.c | 3 +++
 arch/arm/mach-rockchip/Kconfig | 2 --
 common/boards/Kconfig  | 2 ++
 common/boards/wolfvision/common.c  | 5 -
 4 files changed, 9 insertions(+), 3 deletions(-)
---
base-commit: 593248cde35ddedcb27c0791c621e6a4403d7068
change-id: 20240515-b4-pf5-fixup-1c0f8b592313

Best regards,
-- 
Michael Riesch 




[PATCH master v2 2/3] common: boards: move dependencies to wolfvision board code library

2024-05-15 Thread Michael Riesch
Since hardware ID detection has been moved to the WolfVision board code
library, move the Kconfig dependencies as well.

Signed-off-by: Michael Riesch 
---
 arch/arm/mach-rockchip/Kconfig | 2 --
 common/boards/Kconfig  | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index f373624f5c..e7c0c634aa 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -117,9 +117,7 @@ config MACH_RADXA_CM3
 
 config MACH_WOLFVISION_PF5
select ARCH_RK3568
-   select AIODEV
select BOARD_WOLFVISION
-   select ROCKCHIP_SARADC
bool "WolfVision PF5 mainboard"
help
  Say Y here if you are using a WolfVision PF5 mainboard
diff --git a/common/boards/Kconfig b/common/boards/Kconfig
index a2a51155ea..586a54d7ca 100644
--- a/common/boards/Kconfig
+++ b/common/boards/Kconfig
@@ -17,3 +17,5 @@ config BOARD_TQ
 
 config BOARD_WOLFVISION
bool
+   select AIODEV
+   select ROCKCHIP_SARADC

-- 
2.34.1




[PATCH master v2 1/3] arm: boards: add pr_fmt() prefix to wolfvision pf5 board code

2024-05-15 Thread Michael Riesch
Add pr_fmt() message prefix to WolfVision PF5 board code.

Signed-off-by: Michael Riesch 
---
 arch/arm/boards/wolfvision-pf5/board.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boards/wolfvision-pf5/board.c 
b/arch/arm/boards/wolfvision-pf5/board.c
index 797f51bc2e..5a2f4201ba 100644
--- a/arch/arm/boards/wolfvision-pf5/board.c
+++ b/arch/arm/boards/wolfvision-pf5/board.c
@@ -4,6 +4,9 @@
  *
  * Copyright (C) 2024 WolfVision GmbH.
  */
+
+#define pr_fmt(fmt) "WolfVision PF5: " fmt
+
 #include 
 #include 
 #include 

-- 
2.34.1




[PATCH master v2 3/3] common: boards: wolfvision: fix handling of overlays parameter

2024-05-15 Thread Michael Riesch
If the char **overlays parameter to wolfvision_rk3568_detect_hw is NULL,
the overlay file names are not collected.

If overlays points to a NULL pointer, it is initialized properly with
an empty string. This is convenient as the call to
globalvar_set("of.overlay.filepattern", my_resulting_string);
that usually follows actually resets the global filepattern variable
from its default "*" to "". Thereby, a paradoxical situation in which no
extensions are detected but all available overlays are applied (due to "*")
is avoided.

Nevertheless, it is still possible to pass an existing string to this
method and let the method append overlay file names.

Signed-off-by: Michael Riesch 
---
 common/boards/wolfvision/common.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/boards/wolfvision/common.c 
b/common/boards/wolfvision/common.c
index f483918cec..08836096b9 100644
--- a/common/boards/wolfvision/common.c
+++ b/common/boards/wolfvision/common.c
@@ -20,7 +20,7 @@ int wolfvision_apply_overlay(const struct wv_overlay 
*overlay, char **files)
 {
int ret;
 
-   if (overlay->filename) {
+   if (overlay->filename && files) {
if (*files) {
char *old = *files;
*files = basprintf("%s %s", old, overlay->filename);
@@ -115,6 +115,9 @@ int wolfvision_rk3568_detect_hw(const struct 
wv_rk3568_extension *extensions,
if (ret)
return ret;
 
+   if (overlays && !*overlays)
+   *overlays = xstrdup("");
+
for (i = 0; i < num_extensions; i++) {
const struct wv_rk3568_extension *extension = [i];
const struct wv_overlay *overlay;

-- 
2.34.1




Re: [PATCH master 3/3] common: boards: wolfvision: fix handling of overlays parameter

2024-05-15 Thread Michael Riesch
Hi Ahmad,

On 5/15/24 11:14, Ahmad Fatoum wrote:
> On 15.05.24 10:32, Michael Riesch wrote:
>> Hi all,
>>
>> On 5/15/24 10:00, Michael Riesch wrote:
>>> If the char **overlays parameter to wolfvision_rk3568_detect_hw is NULL,
>>> the overlay file names are not collected.
>>>
>>> If overlays points to a NULL pointer, it is initialized properly with
>>> an empty string. This is convenient as the call to
>>> globalvar_set("of.overlay.filepattern", my_resulting_string);
>>> that usually follows actually resets the global filepattern variable
>>> from its default "*" to "". Thereby, a paradoxical situation in which no
>>> extensions are detected but all available overlays are applied (due to "*")
>>> is avoided.
>>>
>>> Nevertheless, it is still possible to pass an existing string to this
>>> method and let the method append overlay file names.
>>>
>>> Signed-off-by: Michael Riesch 
>>> ---
>>>  common/boards/wolfvision/common.c | 5 -
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/boards/wolfvision/common.c 
>>> b/common/boards/wolfvision/common.c
>>> index f483918cec..6cb76250dd 100644
>>> --- a/common/boards/wolfvision/common.c
>>> +++ b/common/boards/wolfvision/common.c
>>> @@ -20,7 +20,7 @@ int wolfvision_apply_overlay(const struct wv_overlay 
>>> *overlay, char **files)
>>>  {
>>> int ret;
>>>  
>>> -   if (overlay->filename) {
>>> +   if (overlay->filename && files) {
>>> if (*files) {
>>> char *old = *files;
>>> *files = basprintf("%s %s", old, overlay->filename);
>>> @@ -115,6 +115,9 @@ int wolfvision_rk3568_detect_hw(const struct 
>>> wv_rk3568_extension *extensions,
>>> if (ret)
>>> return ret;
>>>  
>>> +   if (overlays && !*overlays)
>>> +   *overlays = basprintf("");
>>
>> Hm, apparently this gives a compiler warning "warning: zero-length
>> gnu_printf format string [-Wformat-zero-length]".
>>
>> ... = basprintf("%s", ""); ???
> 
> xstrdup("")

Well if you insist on a straight-forward elegant solution... :-)

v2 coming up soon!

Thanks and regards,
Michael

> 
>>
>> Best regards,
>> Michael
>>
>>> +
>>> for (i = 0; i < num_extensions; i++) {
>>> const struct wv_rk3568_extension *extension = [i];
>>> const struct wv_overlay *overlay;
>>>
>>
>>
> 



[PATCH] fixup! partition: reparse tables only if CONFIG_PARTITION_MANIPULATION=y

2024-05-15 Thread Ahmad Fatoum
partitions: add prompt and help text for CONFIG_PARTITION_MANIPULATION

So far, CONFIG_PARTITION_MANIPULATION was only selected by
CONFIG_CMD_PARTED and influenced repartitioning support.

Now that it's used to guard the feature of reparsing partition tables,
it makes sense to give it a help text and a prompt for devlopment usage.

Signed-off-by: Ahmad Fatoum 
---
 common/Kconfig | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/Kconfig b/common/Kconfig
index 97a03217eac9..67cbbf5197da 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -888,7 +888,15 @@ config PARTITION
prompt "Enable Partitions"
 
 config PARTITION_MANIPULATION
-   bool
+   bool "Runtime reparsing of partition table" if COMPILE_TEST
+   help
+ Say y here to have barebox reparse the partition table automatically
+ when it's rewritten. This is useful when using the parted command
+ or when writing full disk images that change the existing partitions.
+
+ Reparsing the partition table will delete existing partitions and thus
+ may break users that don't do proper reference counting. For this
+ reason, this option is currently disabled by default.
 
 source "common/partitions/Kconfig"
 
-- 
2.39.2




Re: [PATCH master 3/3] common: boards: wolfvision: fix handling of overlays parameter

2024-05-15 Thread Ahmad Fatoum
On 15.05.24 10:32, Michael Riesch wrote:
> Hi all,
> 
> On 5/15/24 10:00, Michael Riesch wrote:
>> If the char **overlays parameter to wolfvision_rk3568_detect_hw is NULL,
>> the overlay file names are not collected.
>>
>> If overlays points to a NULL pointer, it is initialized properly with
>> an empty string. This is convenient as the call to
>> globalvar_set("of.overlay.filepattern", my_resulting_string);
>> that usually follows actually resets the global filepattern variable
>> from its default "*" to "". Thereby, a paradoxical situation in which no
>> extensions are detected but all available overlays are applied (due to "*")
>> is avoided.
>>
>> Nevertheless, it is still possible to pass an existing string to this
>> method and let the method append overlay file names.
>>
>> Signed-off-by: Michael Riesch 
>> ---
>>  common/boards/wolfvision/common.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/boards/wolfvision/common.c 
>> b/common/boards/wolfvision/common.c
>> index f483918cec..6cb76250dd 100644
>> --- a/common/boards/wolfvision/common.c
>> +++ b/common/boards/wolfvision/common.c
>> @@ -20,7 +20,7 @@ int wolfvision_apply_overlay(const struct wv_overlay 
>> *overlay, char **files)
>>  {
>>  int ret;
>>  
>> -if (overlay->filename) {
>> +if (overlay->filename && files) {
>>  if (*files) {
>>  char *old = *files;
>>  *files = basprintf("%s %s", old, overlay->filename);
>> @@ -115,6 +115,9 @@ int wolfvision_rk3568_detect_hw(const struct 
>> wv_rk3568_extension *extensions,
>>  if (ret)
>>  return ret;
>>  
>> +if (overlays && !*overlays)
>> +*overlays = basprintf("");
> 
> Hm, apparently this gives a compiler warning "warning: zero-length
> gnu_printf format string [-Wformat-zero-length]".
> 
> ... = basprintf("%s", ""); ???

xstrdup("")

> 
> Best regards,
> Michael
> 
>> +
>>  for (i = 0; i < num_extensions; i++) {
>>  const struct wv_rk3568_extension *extension = [i];
>>  const struct wv_overlay *overlay;
>>
> 
> 

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |




Re: [PATCH master 3/3] common: boards: wolfvision: fix handling of overlays parameter

2024-05-15 Thread Michael Riesch
Hi all,

On 5/15/24 10:00, Michael Riesch wrote:
> If the char **overlays parameter to wolfvision_rk3568_detect_hw is NULL,
> the overlay file names are not collected.
> 
> If overlays points to a NULL pointer, it is initialized properly with
> an empty string. This is convenient as the call to
> globalvar_set("of.overlay.filepattern", my_resulting_string);
> that usually follows actually resets the global filepattern variable
> from its default "*" to "". Thereby, a paradoxical situation in which no
> extensions are detected but all available overlays are applied (due to "*")
> is avoided.
> 
> Nevertheless, it is still possible to pass an existing string to this
> method and let the method append overlay file names.
> 
> Signed-off-by: Michael Riesch 
> ---
>  common/boards/wolfvision/common.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/common/boards/wolfvision/common.c 
> b/common/boards/wolfvision/common.c
> index f483918cec..6cb76250dd 100644
> --- a/common/boards/wolfvision/common.c
> +++ b/common/boards/wolfvision/common.c
> @@ -20,7 +20,7 @@ int wolfvision_apply_overlay(const struct wv_overlay 
> *overlay, char **files)
>  {
>   int ret;
>  
> - if (overlay->filename) {
> + if (overlay->filename && files) {
>   if (*files) {
>   char *old = *files;
>   *files = basprintf("%s %s", old, overlay->filename);
> @@ -115,6 +115,9 @@ int wolfvision_rk3568_detect_hw(const struct 
> wv_rk3568_extension *extensions,
>   if (ret)
>   return ret;
>  
> + if (overlays && !*overlays)
> + *overlays = basprintf("");

Hm, apparently this gives a compiler warning "warning: zero-length
gnu_printf format string [-Wformat-zero-length]".

... = basprintf("%s", ""); ???

Best regards,
Michael

> +
>   for (i = 0; i < num_extensions; i++) {
>   const struct wv_rk3568_extension *extension = [i];
>   const struct wv_overlay *overlay;
> 



[PATCH master 3/3] partition: reparse tables only if CONFIG_PARTITION_MANIPULATION=y

2024-05-15 Thread Ahmad Fatoum
reparse_partition_table() will delete a device's existing partition
cdevs and allocate new ones according to the new partition table.

Holding a reference to a cdev prevents it from bring removed to avoid
dangling pointers and use-after-frees.

Unfortunately, not all users call cdev_open on the cdev and increment
the usage count, instead using functions like cdev_by_name, which don't
call cdev_open.

The proper solution for this is probably to change all functions that
return a cdev to actually take a reference to the cdev.

This will involve quite a bit of work though, so for now, restore old
behavior by making reparsing of the partition tables dependent on
CONFIG_PARTITION_MANIPULATION=y.

The option isn't fully appropriate as partition manipulation can happen
by writing to the parent block device, but not noticing changes of
partitions is the behavior we had before.

Signed-off-by: Ahmad Fatoum 
---
 common/partitions.c |  2 ++
 include/disks.h | 11 +++
 2 files changed, 13 insertions(+)

diff --git a/common/partitions.c b/common/partitions.c
index 17c2f1eb281a..0b10377e7327 100644
--- a/common/partitions.c
+++ b/common/partitions.c
@@ -266,6 +266,7 @@ int parse_partition_table(struct block_device *blk)
return rc;
 }
 
+#ifdef CONFIG_PARTITION_MANIPULATION
 int reparse_partition_table(struct block_device *blk)
 {
struct cdev *cdev = >cdev;
@@ -285,6 +286,7 @@ int reparse_partition_table(struct block_device *blk)
 
return parse_partition_table(blk);
 }
+#endif
 
 int partition_parser_register(struct partition_parser *p)
 {
diff --git a/include/disks.h b/include/disks.h
index ccb50d3ce94f..a3673d1e276f 100644
--- a/include/disks.h
+++ b/include/disks.h
@@ -3,6 +3,9 @@
 #ifndef DISKS_H
 #define DISKS_H
 
+#include 
+#include 
+
 struct block_device;
 
 /** Size of one sector in bytes */
@@ -24,6 +27,14 @@ struct partition_entry {
 } __attribute__ ((packed));
 
 extern int parse_partition_table(struct block_device*);
+
+#ifdef CONFIG_PARTITION_MANIPULATION
 int reparse_partition_table(struct block_device *blk);
+#else
+static inline int reparse_partition_table(struct block_device *blk)
+{
+   return -ENOSYS;
+}
+#endif
 
 #endif /* DISKS_H */
-- 
2.39.2




[PATCH master 1/3] cdev: return error code from cdev_close

2024-05-15 Thread Ahmad Fatoum
cdev_operations::close can fail and thus cdev_close should pass along
the error code to the users instead of discarding it.

Do that, so users like devfs close() can start propagating this error.

Signed-off-by: Ahmad Fatoum 
---
 fs/devfs-core.c  | 11 ---
 include/driver.h |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 376c62be9eab..21e5c2dc969a 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -224,12 +224,17 @@ struct cdev *cdev_open_by_name(const char *name, unsigned 
long flags)
return cdev;
 }
 
-void cdev_close(struct cdev *cdev)
+int cdev_close(struct cdev *cdev)
 {
-   if (cdev->ops->close)
-   cdev->ops->close(cdev);
+   if (cdev->ops->close) {
+   int ret = cdev->ops->close(cdev);
+   if (ret)
+   return ret;
+   }
 
cdev->open--;
+
+   return 0;
 }
 
 ssize_t cdev_read(struct cdev *cdev, void *buf, size_t count, loff_t offset, 
ulong flags)
diff --git a/include/driver.h b/include/driver.h
index 5a3e46e99e14..3401ab4f07f4 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -529,7 +529,7 @@ struct cdev *cdev_create_loop(const char *path, ulong 
flags, loff_t offset);
 void cdev_remove_loop(struct cdev *cdev);
 int cdev_open(struct cdev *, unsigned long flags);
 int cdev_fdopen(struct cdev *cdev, unsigned long flags);
-void cdev_close(struct cdev *cdev);
+int cdev_close(struct cdev *cdev);
 int cdev_flush(struct cdev *cdev);
 ssize_t cdev_read(struct cdev *cdev, void *buf, size_t count, loff_t offset, 
ulong flags);
 ssize_t cdev_write(struct cdev *cdev, const void *buf, size_t count, loff_t 
offset, ulong flags);
-- 
2.39.2




[PATCH master 2/3] fs: devfs: restore cdev reference count maintenance

2024-05-15 Thread Ahmad Fatoum
barebox v2024.03.0 moves the reference count maintenance into
cdev_open/cdev_close, but missed to switch devfs_open and devfs_close to
actually use cdev_open/cdev_close. The result was that the cdev could be
deleted, e.g. due to partition reparsing, and further use of the file
descriptor would result in a use-after-free.

Fix this by doing what the commit introducing the breakage was purported
to do.

Fixes: d84c3daf1314 ("fs: move cdev open count to cdev_open()/cdev_close()")
Signed-off-by: Ahmad Fatoum 
---
 fs/devfs.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/fs/devfs.c b/fs/devfs.c
index c8ddbbdab04d..f5bad5aa9bf2 100644
--- a/fs/devfs.c
+++ b/fs/devfs.c
@@ -102,33 +102,19 @@ static int devfs_open(struct device *_dev, FILE *f, const 
char *filename)
struct inode *inode = f->f_inode;
struct devfs_inode *node = container_of(inode, struct devfs_inode, 
inode);
struct cdev *cdev = node->cdev;
-   int ret;
 
f->size = cdev->flags & DEVFS_IS_CHARACTER_DEV ?
FILE_SIZE_STREAM : cdev->size;
f->priv = cdev;
 
-   if (cdev->ops->open) {
-   ret = cdev->ops->open(cdev, f->flags);
-   if (ret)
-   return ret;
-   }
-
-   return 0;
+   return cdev_open(cdev, f->flags);
 }
 
 static int devfs_close(struct device *_dev, FILE *f)
 {
struct cdev *cdev = f->priv;
-   int ret;
 
-   if (cdev->ops->close) {
-   ret = cdev->ops->close(cdev);
-   if (ret)
-   return ret;
-   }
-
-   return 0;
+   return cdev_close(cdev);
 }
 
 static int devfs_flush(struct device *_dev, FILE *f)
-- 
2.39.2




[PATCH master 3/3] common: boards: wolfvision: fix handling of overlays parameter

2024-05-15 Thread Michael Riesch
If the char **overlays parameter to wolfvision_rk3568_detect_hw is NULL,
the overlay file names are not collected.

If overlays points to a NULL pointer, it is initialized properly with
an empty string. This is convenient as the call to
globalvar_set("of.overlay.filepattern", my_resulting_string);
that usually follows actually resets the global filepattern variable
from its default "*" to "". Thereby, a paradoxical situation in which no
extensions are detected but all available overlays are applied (due to "*")
is avoided.

Nevertheless, it is still possible to pass an existing string to this
method and let the method append overlay file names.

Signed-off-by: Michael Riesch 
---
 common/boards/wolfvision/common.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/boards/wolfvision/common.c 
b/common/boards/wolfvision/common.c
index f483918cec..6cb76250dd 100644
--- a/common/boards/wolfvision/common.c
+++ b/common/boards/wolfvision/common.c
@@ -20,7 +20,7 @@ int wolfvision_apply_overlay(const struct wv_overlay 
*overlay, char **files)
 {
int ret;
 
-   if (overlay->filename) {
+   if (overlay->filename && files) {
if (*files) {
char *old = *files;
*files = basprintf("%s %s", old, overlay->filename);
@@ -115,6 +115,9 @@ int wolfvision_rk3568_detect_hw(const struct 
wv_rk3568_extension *extensions,
if (ret)
return ret;
 
+   if (overlays && !*overlays)
+   *overlays = basprintf("");
+
for (i = 0; i < num_extensions; i++) {
const struct wv_rk3568_extension *extension = [i];
const struct wv_overlay *overlay;

-- 
2.34.1




[PATCH master 2/3] common: boards: move dependencies to wolfvision board code library

2024-05-15 Thread Michael Riesch
Since hardware ID detection has been moved to the WolfVision board code
library, move the Kconfig dependencies as well.

Signed-off-by: Michael Riesch 
---
 arch/arm/mach-rockchip/Kconfig | 2 --
 common/boards/Kconfig  | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index f373624f5c..e7c0c634aa 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -117,9 +117,7 @@ config MACH_RADXA_CM3
 
 config MACH_WOLFVISION_PF5
select ARCH_RK3568
-   select AIODEV
select BOARD_WOLFVISION
-   select ROCKCHIP_SARADC
bool "WolfVision PF5 mainboard"
help
  Say Y here if you are using a WolfVision PF5 mainboard
diff --git a/common/boards/Kconfig b/common/boards/Kconfig
index a2a51155ea..586a54d7ca 100644
--- a/common/boards/Kconfig
+++ b/common/boards/Kconfig
@@ -17,3 +17,5 @@ config BOARD_TQ
 
 config BOARD_WOLFVISION
bool
+   select AIODEV
+   select ROCKCHIP_SARADC

-- 
2.34.1




[PATCH master 1/3] arm: boards: add pr_fmt() prefix to wolfvision pf5 board code

2024-05-15 Thread Michael Riesch
Add pr_fmt() message prefix to WolfVision PF5 board code.

Signed-off-by: Michael Riesch 
---
 arch/arm/boards/wolfvision-pf5/board.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boards/wolfvision-pf5/board.c 
b/arch/arm/boards/wolfvision-pf5/board.c
index 797f51bc2e..5a2f4201ba 100644
--- a/arch/arm/boards/wolfvision-pf5/board.c
+++ b/arch/arm/boards/wolfvision-pf5/board.c
@@ -4,6 +4,9 @@
  *
  * Copyright (C) 2024 WolfVision GmbH.
  */
+
+#define pr_fmt(fmt) "WolfVision PF5: " fmt
+
 #include 
 #include 
 #include 

-- 
2.34.1




[PATCH master 0/3] Fixes for WolfVision board code library and PF5 mainboard code

2024-05-15 Thread Michael Riesch
Habidere,

This series is a follow-up to 
https://lore.barebox.org/barebox/20240412-feature-wolfvision-pf5-v2-0-7e277cc88...@wolfvision.net/
and fixes a few things. I tried to reply to said thread and announce a
v3, but I just found that this reply never hit the list for ... reasons.

Anyway, here are the fixes. Do you think they can be applied directly
to master?

Looking forward to your comments!

Signed-off-by: Michael Riesch 
---
Michael Riesch (3):
  arm: boards: add pr_fmt() prefix to wolfvision pf5 board code
  common: boards: move dependencies to wolfvision board code library
  common: boards: wolfvision: fix handling of overlays parameter

 arch/arm/boards/wolfvision-pf5/board.c | 3 +++
 arch/arm/mach-rockchip/Kconfig | 2 --
 common/boards/Kconfig  | 2 ++
 common/boards/wolfvision/common.c  | 5 -
 4 files changed, 9 insertions(+), 3 deletions(-)
---
base-commit: 593248cde35ddedcb27c0791c621e6a4403d7068
change-id: 20240515-b4-pf5-fixup-1c0f8b592313

Best regards,
-- 
Michael Riesch 




Re: [PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

2024-05-15 Thread Michael Riesch
Hi Ahmad,

On 5/15/24 08:35, Ahmad Fatoum wrote:
> Hello Michael,
> 
> On 15.05.24 08:29, Michael Riesch wrote:
>> Hi Ahmad,
>>
>> Thanks a lot for your patch!
>>
>> On 5/15/24 08:07, Ahmad Fatoum wrote:
>>> This introduces no functional change, but makes code a bit more compact.
>>>
>>> Cc: Michael Riesch 
>>> Signed-off-by: Ahmad Fatoum 
>>> ---
>>>  common/boards/wolfvision/common.c | 6 +-
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/common/boards/wolfvision/common.c 
>>> b/common/boards/wolfvision/common.c
>>> index f483918cecfe..5484a8ac6b06 100644
>>> --- a/common/boards/wolfvision/common.c
>>> +++ b/common/boards/wolfvision/common.c
>>> @@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
>>> char mac[ETH_ALEN];
>>> int ret;
>>>  
>>> -   ret = of_device_ensure_probed_by_alias("state");
>>
>> Just to be on the safe side: of_device_ensure_probed_by_alias makes sure
>> that the underlying drivers are probed, right?
> 
> Yes.
> 
>>
>>> -   if (ret)
>>> -   return ret;
>>> -
>>> -   state = state_by_name("state");
>>> +   state = state_by_alias("state");
>>
>> state_by_alias, on the other hand, calls only of_find_node_by_alias,
>> which (as I presume) does not ensure that.
> 
> Yes, but afterwards it calls state_by_node(), which calls 
> of_device_ensure_probed().

Ah, nice!

>> IIRC the of_device_ensure_... magic was necessary in our setup, but I
>> can give your patch a test during the next round of barebox board code
>> cleanups.
> 
> Yes, I ran into these problems before too on a deep probe system, which is
> why state_by_alias was added.

OK, this sounds good.

Reviewed-by: Michael Riesch 

Thanks and regards,
Michael

> 
> Cheers,
> Ahmad
> 
>>
>> Best regards,
>> Michael
>>
>>> if (!state)
>>> return -ENOENT;
>>>  
>>
> 



Re: [PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

2024-05-15 Thread Ahmad Fatoum
Hello Michael,

On 15.05.24 08:29, Michael Riesch wrote:
> Hi Ahmad,
> 
> Thanks a lot for your patch!
> 
> On 5/15/24 08:07, Ahmad Fatoum wrote:
>> This introduces no functional change, but makes code a bit more compact.
>>
>> Cc: Michael Riesch 
>> Signed-off-by: Ahmad Fatoum 
>> ---
>>  common/boards/wolfvision/common.c | 6 +-
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/common/boards/wolfvision/common.c 
>> b/common/boards/wolfvision/common.c
>> index f483918cecfe..5484a8ac6b06 100644
>> --- a/common/boards/wolfvision/common.c
>> +++ b/common/boards/wolfvision/common.c
>> @@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
>>  char mac[ETH_ALEN];
>>  int ret;
>>  
>> -ret = of_device_ensure_probed_by_alias("state");
> 
> Just to be on the safe side: of_device_ensure_probed_by_alias makes sure
> that the underlying drivers are probed, right?

Yes.

> 
>> -if (ret)
>> -return ret;
>> -
>> -state = state_by_name("state");
>> +state = state_by_alias("state");
> 
> state_by_alias, on the other hand, calls only of_find_node_by_alias,
> which (as I presume) does not ensure that.

Yes, but afterwards it calls state_by_node(), which calls 
of_device_ensure_probed().

> IIRC the of_device_ensure_... magic was necessary in our setup, but I
> can give your patch a test during the next round of barebox board code
> cleanups.

Yes, I ran into these problems before too on a deep probe system, which is
why state_by_alias was added.

Cheers,
Ahmad

> 
> Best regards,
> Michael
> 
>>  if (!state)
>>  return -ENOENT;
>>  
> 

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |




Re: [PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

2024-05-15 Thread Michael Riesch
Hi Ahmad,

Thanks a lot for your patch!

On 5/15/24 08:07, Ahmad Fatoum wrote:
> This introduces no functional change, but makes code a bit more compact.
> 
> Cc: Michael Riesch 
> Signed-off-by: Ahmad Fatoum 
> ---
>  common/boards/wolfvision/common.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/common/boards/wolfvision/common.c 
> b/common/boards/wolfvision/common.c
> index f483918cecfe..5484a8ac6b06 100644
> --- a/common/boards/wolfvision/common.c
> +++ b/common/boards/wolfvision/common.c
> @@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
>   char mac[ETH_ALEN];
>   int ret;
>  
> - ret = of_device_ensure_probed_by_alias("state");

Just to be on the safe side: of_device_ensure_probed_by_alias makes sure
that the underlying drivers are probed, right?

> - if (ret)
> - return ret;
> -
> - state = state_by_name("state");
> + state = state_by_alias("state");

state_by_alias, on the other hand, calls only of_find_node_by_alias,
which (as I presume) does not ensure that.

IIRC the of_device_ensure_... magic was necessary in our setup, but I
can give your patch a test during the next round of barebox board code
cleanups.

Best regards,
Michael

>   if (!state)
>   return -ENOENT;
>  



[PATCH master 2/2] pblimage: ls1028a: fix handling of short reads on

2024-05-15 Thread Ahmad Fatoum
Newer toolchains rightly complain:

  scripts/pblimage.c:259:17: error: ignoring return value of ‘read’ declared
   with attribute ‘warn_unused_result’ [-Werror=unused-result]
259 | read(in_fd, mem_buf + 0x1000, image_size);

In the ls1046a case, this doesn't happen, because pbl_fget is used,
which does a number of things:

  - report warnings and exit on read errors
  - fill buffer with 0xff on short reads
  - increment pmem_buf and pbl_size by the number of bytes emitted

All of which, sound like a good idea for the ls1028a as well. Therefore,
initialize pmem_buf and pbl_size to the correct value and use pbl_fget.

This makes the code easier to follow through by making it explicit that
the image's payload is always given a 4K alignment, independent of the
size of the image header.

This change has only been build-tested.

Fixes: d5e5a65c2b64 ("pblimage: Add LS1028a support")
Signed-off-by: Ahmad Fatoum 
---
 scripts/pblimage.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/scripts/pblimage.c b/scripts/pblimage.c
index 610d93b36800..a3ed74836883 100644
--- a/scripts/pblimage.c
+++ b/scripts/pblimage.c
@@ -254,10 +254,16 @@ static void pbl_load_image(void)
buf32[3] = image_size;
buf32[4] = 0x80ff;
pbl_size += 20;
-   pmem_buf += 20;
 
-   read(in_fd, mem_buf + 0x1000, image_size);
-   pbl_size = 0x1000 + image_size;
+   if (pbl_size > 0x1000) {
+   fprintf(stderr, "Header exceeded maximum size of 4K\n");
+   exit(EXIT_FAILURE);
+   }
+
+   pmem_buf = mem_buf + 0x1000;
+   pbl_size = 0x1000;
+
+   pbl_fget(image_size, in_fd);
} else {
exit(EXIT_FAILURE);
}
-- 
2.39.2




[PATCH master 1/2] list: fix CONFIG_DEBUG_LIST link failure in PBL

2024-05-15 Thread Ahmad Fatoum
With the addition of PBL handoff data, we now use  in PBL.
This works fine with CONFIG_DEBUG_LIST disabled, because all functions are
inlined, but when building with the option enabled, references to the
out-of-line sanity checking functions breaks the build.

Fix this by omitting these references when building for PBL.

Signed-off-by: Ahmad Fatoum 
---
 include/linux/list.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/list.h b/include/linux/list.h
index 2b3a39ea81e8..e47a8188e807 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -36,7 +36,7 @@ static inline void INIT_LIST_HEAD(struct list_head *list)
list->prev = list;
 }
 
-#ifdef CONFIG_DEBUG_LIST
+#if defined(CONFIG_DEBUG_LIST) && !defined(__PBL__)
 extern bool __list_add_valid_or_report(struct list_head *new,
   struct list_head *prev,
   struct list_head *next);
-- 
2.39.2




Re: [PATCH 1/2] usb:gadget:composite: add public functions for managing setup requests

2024-05-15 Thread Sascha Hauer


On Tue, 14 May 2024 15:03:26 +0200, Enrico Scholz wrote:
> The composite driver does some bookkeeping about pending requests and
> decides in its cleanup function whether requests must be dequeued.
> 
> There are some function drivers (dfu, acm) which queue the requests
> directly which causes e.g.
> 
> | :/ dfu /tmp/img(img)c
> | ...
> | g_multi gadget0: high-speed config #1: Multifunction Composite Gadget
> | fsl_free_request: Freeing queued request
> | [<2fd8d8e5>] (unwind_backtrace+0x1/0x78) from [<2fd34b1f>] 
> (fsl_free_request+0x1f/0x34)
> | [<2fd34b1f>] (fsl_free_request+0x1f/0x34) from [<2fd337cf>] 
> (composite_dev_cleanup+0x77/0xc0)
> | [<2fd337cf>] (composite_dev_cleanup+0x77/0xc0) from [<2fd33867>] 
> (__composite_unbind+0x4f/0x94)
> | [<2fd33867>] (__composite_unbind+0x4f/0x94) from [<2fd3432b>] 
> (gadget_unbind_driver+0x37/0x70)
> | [<2fd3432b>] (gadget_unbind_driver+0x37/0x70) from [<2fd1275f>] 
> (device_remove+0xf/0x20)
> | [<2fd1275f>] (device_remove+0xf/0x20) from [<2fd1289b>] 
> (unregister_driver+0x47/0x60)
> | [<2fd1289b>] (unregister_driver+0x47/0x60) from [<2fd34663>] 
> (usb_gadget_unregister_driver+0xf/0x18)
> | [<2fd34663>] (usb_gadget_unregister_driver+0xf/0x18) from [<2fd37c5b>] 
> (usb_multi_unregister+0x13/0x30)
> | [<2fd37c5b>] (usb_multi_unregister+0x13/0x30) from [<2fd59f67>] 
> (do_dfu+0x47/0x68)
> | [<2fd59f67>] (do_dfu+0x47/0x68) from [<2fd04fdf>] 
> (execute_command+0x23/0x4c)
> | [<2fd04fdf>] (execute_command+0x23/0x4c) from [<2fd0a737>] 
> (run_list_real+0x5ef/0x690)
> | [<2fd0a737>] (run_list_real+0x5ef/0x690) from [<2fd0a00b>] 
> (parse_stream_outer+0xc7/0x154)
> | [<2fd0a00b>] (parse_stream_outer+0xc7/0x154) from [<2fd0a927>] 
> (run_shell+0x3f/0x6c)
> | [<2fd0a927>] (run_shell+0x3f/0x6c) from [<2fd01103>] (run_init+0xeb/0x210)
> | [<2fd01103>] (run_init+0xeb/0x210) from [<2fd01253>] 
> (start_barebox+0x2b/0x6c)
> | [<2fd01253>] (start_barebox+0x2b/0x6c) from [<2fd89b37>] 
> (barebox_non_pbl_start+0xc3/0x108)
> | [<2fd89b37>] (barebox_non_pbl_start+0xc3/0x108) from [<2fd5>] 
> (__bare_init_start+0x1/0xc)
> 
> [...]

Applied, thanks!

[1/2] usb:gadget:composite: add public functions for managing setup requests
  https://git.pengutronix.de/cgit/barebox/commit/?id=bf91067ef12c (link may 
not be stable)
[2/2] usb:gadget: use helper function to queue composite requests
  https://git.pengutronix.de/cgit/barebox/commit/?id=593248cde35d (link may 
not be stable)

Best regards,
-- 
Sascha Hauer 




[PATCH] common: boards: wolfvision: use state_by_alias instead of opencoding

2024-05-15 Thread Ahmad Fatoum
This introduces no functional change, but makes code a bit more compact.

Cc: Michael Riesch 
Signed-off-by: Ahmad Fatoum 
---
 common/boards/wolfvision/common.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/common/boards/wolfvision/common.c 
b/common/boards/wolfvision/common.c
index f483918cecfe..5484a8ac6b06 100644
--- a/common/boards/wolfvision/common.c
+++ b/common/boards/wolfvision/common.c
@@ -62,11 +62,7 @@ int wolfvision_register_ethaddr(void)
char mac[ETH_ALEN];
int ret;
 
-   ret = of_device_ensure_probed_by_alias("state");
-   if (ret)
-   return ret;
-
-   state = state_by_name("state");
+   state = state_by_alias("state");
if (!state)
return -ENOENT;
 
-- 
2.39.2