Re: [PATCH v2 08/10] ARM: mmu: use reserve mem entries to modify maps

2022-09-12 Thread Sascha Hauer
On Mon, Sep 12, 2022 at 05:15:45PM +0200, Ahmad Fatoum wrote:
> Hi,
> 
> On 12.09.22 14:01, Sascha Hauer wrote:
> > This patch breaks NAND support on my Phytec i.MX6 board. There are some
> > problems with this patch, so I ended up reverting it for now.
> 
> I wonder why. I see no memory reserves in imx6q-phytec-phycore-som-nand.dts
> and the files it includes.
> 
> > 
> > On Wed, Aug 17, 2022 at 01:42:42PM +0200, Ahmad Fatoum wrote:
> >> @@ -468,11 +469,28 @@ void __mmu_init(bool mmu_on)
> >>vectors_init();
> >>  
> >>for_each_memory_bank(bank) {
> >> +  struct resource *rsv;
> >> +
> >>create_sections(ttb, bank->start, bank->start + bank->size - 1,
> >>PMD_SECT_DEF_CACHED);
> >> -  __mmu_cache_flush();
> >> +
> >> +  for_each_reserved_region(bank, rsv) {
> >> +  create_sections(ttb, resource_first_page(rsv),
> >> +  resource_count_pages(rsv),
> >> +  attrs_uncached_mem());
> >> +  }
> > 
> > This operates on 1MiB sections, so everything requiring a finer
> > granularity will fail here. Not sure if we currently need that, but not
> > even issuing a warning is not good.
> 
> At worst it'd needlessly mark some memory uncached/XN. If users are
> affected, they will notice and we can revisit this. I could add a debug
> print here.
> 
> I need to rework this though, because I see now create_sections differs
> between ARM64 and ARM32. On ARM64, it accepts the last address as argument,
> while on ARM64, it's the size.. resource_count_pages() is not a nice
> name either, because it returns bytes (aligned up to PAGE_SIZE).
> 
> > 
> >>}
> >>  
> >> +  /*
> >> +   * We could set_ttbr(ttb) here instead and save on the copy, but
> >> +   * for now we play it safe, so we don't mess with the older ARMs.
> >> +   */
> >> +  if (oldttb) {
> >> +  memcpy(oldttb, ttb, ARM_TTB_SIZE);
> >> +  free(ttb);
> >> +  }
> > 
> > in the early MMU case the MMU still uses 'oldttb' as ttb whereas 'ttb'
> > now points to invalid memory. Still functions like arm_create_pte()
> > still operate on 'ttb'. It looks like a ttb = oldttb is missing here.
> 
> Why would ttb point at invalid memory? It's allocated unconditionally
> with xmemalign and freed here.

It becomes clearer when you look at the scope of the variable.

> 
> > Also I wonder when we have to map the reserved regions as execute never,
> > then the early MMU setup seems broken as well, as that creates a flat
> > mapping without honoring the reserved regions. Shouldn't that be fixed?
> 
> Yes, see excerpt from cover letter:
> 
> "Note that this doesn't yet solve all problems. For example, PPA secure
>  monitor installation on Layerscape may happen with CONFIG_MMU_EARLY=y,
>  in which case barebox in EL2 may speculate into the secure memory
>  before any device tree reserved-memory settings are considered. For this
>  reason, both early MMU and normal MMU setup must be aware of the
>  reserved memory regions. The original patch set by Rouven used FDT
>  parsing in PBL to achieve this, but this is omitted here to limit
>  scope of the patch series. Instead we only handle the CONFIG_OPTEE_SIZE
>  case out-of-the-box."

Ok.

Sascha

-- 
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 v2] of: implement of_prepend_property

2022-09-12 Thread Ahmad Fatoum
Like of_append_property for adding at the end of properties, implement
of_prepend_property for placing data into the front.

This is especially useful to fixup most-specific compatibles into
existing nodes.

Signed-off-by: Ahmad Fatoum 
---
v1 -> v2:
  - simplify with malloc/memcpy instead of realloc/memmove (Sascha)
---
 drivers/of/base.c   | 30 ++
 include/of.h|  8 
 test/self/of_manipulation.c |  2 +-
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 212b226eb55c..ea2a88764bb2 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2378,6 +2378,36 @@ int of_append_property(struct device_node *np, const 
char *name, const void *val
return 0;
 }
 
+int of_prepend_property(struct device_node *np, const char *name, const void 
*val, int len)
+{
+   struct property *pp;
+   const void *oldval;
+   void *buf;
+   int oldlen;
+
+   pp = of_find_property(np, name, );
+   if (!pp) {
+   of_new_property(np, name, val, len);
+   return 0;
+   }
+
+   oldval = of_property_get_value(pp);
+
+   buf = malloc(len + oldlen);
+   if (!buf)
+   return -ENOMEM;
+
+   memcpy(buf, val, len);
+   memcpy(buf + len, oldval, oldlen);
+
+   free(pp->value);
+   pp->value = buf;
+   pp->length = len + oldlen;
+   pp->value_const = NULL;
+
+   return 0;
+}
+
 int of_property_sprintf(struct device_node *np,
const char *propname, const char *fmt, ...)
 {
diff --git a/include/of.h b/include/of.h
index 153e75d3e51d..052d5fcad84c 100644
--- a/include/of.h
+++ b/include/of.h
@@ -135,6 +135,8 @@ extern int of_set_property(struct device_node *node, const 
char *p,
const void *val, int len, int create);
 extern int of_append_property(struct device_node *np, const char *p,
  const void *val, int len);
+extern int of_prepend_property(struct device_node *np, const char *name,
+  const void *val, int len);
 extern struct property *of_new_property(struct device_node *node,
const char *name, const void *data, int len);
 extern struct property *of_new_property_const(struct device_node *node,
@@ -536,6 +538,12 @@ static inline int of_append_property(struct device_node 
*np, const char *p,
return -ENOSYS;
 }
 
+static inline int of_prepend_property(struct device_node *np, const char *name,
+ const void *val, int len)
+{
+   return -ENOSYS;
+}
+
 static inline struct property *of_new_property(struct device_node *node,
const char *name, const void *data, int len)
 {
diff --git a/test/self/of_manipulation.c b/test/self/of_manipulation.c
index 7e30a60ca687..f7f95fa269af 100644
--- a/test/self/of_manipulation.c
+++ b/test/self/of_manipulation.c
@@ -103,9 +103,9 @@ static void test_of_property_strings(struct device_node 
*root)
 
of_append_property(np4, "property-single", "ayy", 4);
 
-   of_append_property(np4, "property-multi", "ayy", 4);
of_append_property(np4, "property-multi", "bee", 4);
of_append_property(np4, "property-multi", "sea", 4);
+   of_prepend_property(np4, "property-multi", "ayy", 4);
 
assert_equal(np3, np4);
 }
-- 
2.30.2




Re: [PATCH v2 08/10] ARM: mmu: use reserve mem entries to modify maps

2022-09-12 Thread Ahmad Fatoum
Hi,

On 12.09.22 14:01, Sascha Hauer wrote:
> This patch breaks NAND support on my Phytec i.MX6 board. There are some
> problems with this patch, so I ended up reverting it for now.

I wonder why. I see no memory reserves in imx6q-phytec-phycore-som-nand.dts
and the files it includes.

> 
> On Wed, Aug 17, 2022 at 01:42:42PM +0200, Ahmad Fatoum wrote:
>> @@ -468,11 +469,28 @@ void __mmu_init(bool mmu_on)
>>  vectors_init();
>>  
>>  for_each_memory_bank(bank) {
>> +struct resource *rsv;
>> +
>>  create_sections(ttb, bank->start, bank->start + bank->size - 1,
>>  PMD_SECT_DEF_CACHED);
>> -__mmu_cache_flush();
>> +
>> +for_each_reserved_region(bank, rsv) {
>> +create_sections(ttb, resource_first_page(rsv),
>> +resource_count_pages(rsv),
>> +attrs_uncached_mem());
>> +}
> 
> This operates on 1MiB sections, so everything requiring a finer
> granularity will fail here. Not sure if we currently need that, but not
> even issuing a warning is not good.

At worst it'd needlessly mark some memory uncached/XN. If users are
affected, they will notice and we can revisit this. I could add a debug
print here.

I need to rework this though, because I see now create_sections differs
between ARM64 and ARM32. On ARM64, it accepts the last address as argument,
while on ARM64, it's the size.. resource_count_pages() is not a nice
name either, because it returns bytes (aligned up to PAGE_SIZE).

> 
>>  }
>>  
>> +/*
>> + * We could set_ttbr(ttb) here instead and save on the copy, but
>> + * for now we play it safe, so we don't mess with the older ARMs.
>> + */
>> +if (oldttb) {
>> +memcpy(oldttb, ttb, ARM_TTB_SIZE);
>> +free(ttb);
>> +}
> 
> in the early MMU case the MMU still uses 'oldttb' as ttb whereas 'ttb'
> now points to invalid memory. Still functions like arm_create_pte()
> still operate on 'ttb'. It looks like a ttb = oldttb is missing here.

Why would ttb point at invalid memory? It's allocated unconditionally
with xmemalign and freed here.

> Also I wonder when we have to map the reserved regions as execute never,
> then the early MMU setup seems broken as well, as that creates a flat
> mapping without honoring the reserved regions. Shouldn't that be fixed?

Yes, see excerpt from cover letter:

"Note that this doesn't yet solve all problems. For example, PPA secure
 monitor installation on Layerscape may happen with CONFIG_MMU_EARLY=y,
 in which case barebox in EL2 may speculate into the secure memory
 before any device tree reserved-memory settings are considered. For this
 reason, both early MMU and normal MMU setup must be aware of the
 reserved memory regions. The original patch set by Rouven used FDT
 parsing in PBL to achieve this, but this is omitted here to limit
 scope of the patch series. Instead we only handle the CONFIG_OPTEE_SIZE
 case out-of-the-box."

My use case is the CONFIG_OPTEE_SIZE one and at least for ARM64, that looks
resolved now IMO.

Cheers,
Ahmad

> 
> Sascha
> 


-- 
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 v2 08/10] ARM: mmu: use reserve mem entries to modify maps

2022-09-12 Thread Sascha Hauer
This patch breaks NAND support on my Phytec i.MX6 board. There are some
problems with this patch, so I ended up reverting it for now.

On Wed, Aug 17, 2022 at 01:42:42PM +0200, Ahmad Fatoum wrote:
> @@ -468,11 +469,28 @@ void __mmu_init(bool mmu_on)
>   vectors_init();
>  
>   for_each_memory_bank(bank) {
> + struct resource *rsv;
> +
>   create_sections(ttb, bank->start, bank->start + bank->size - 1,
>   PMD_SECT_DEF_CACHED);
> - __mmu_cache_flush();
> +
> + for_each_reserved_region(bank, rsv) {
> + create_sections(ttb, resource_first_page(rsv),
> + resource_count_pages(rsv),
> + attrs_uncached_mem());
> + }

This operates on 1MiB sections, so everything requiring a finer
granularity will fail here. Not sure if we currently need that, but not
even issuing a warning is not good.

>   }
>  
> + /*
> +  * We could set_ttbr(ttb) here instead and save on the copy, but
> +  * for now we play it safe, so we don't mess with the older ARMs.
> +  */
> + if (oldttb) {
> + memcpy(oldttb, ttb, ARM_TTB_SIZE);
> + free(ttb);
> + }

in the early MMU case the MMU still uses 'oldttb' as ttb whereas 'ttb'
now points to invalid memory. Still functions like arm_create_pte()
still operate on 'ttb'. It looks like a ttb = oldttb is missing here.

Also I wonder when we have to map the reserved regions as execute never,
then the early MMU setup seems broken as well, as that creates a flat
mapping without honoring the reserved regions. Shouldn't that be fixed?

Sascha

-- 
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] commands: mm: don't ignore pread() errors for larger access sizes

2022-09-12 Thread Sascha Hauer
On Fri, Sep 02, 2022 at 12:04:58PM +0200, Ahmad Fatoum wrote:
> Error check was before pread was called and because ret was initialized,
> compiler didn't warn about it. Fix this.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  commands/mm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/commands/mm.c b/commands/mm.c
> index 9ce883964485..8fe87a80a18e 100644
> --- a/commands/mm.c
> +++ b/commands/mm.c
> @@ -16,7 +16,7 @@
>  
>  static int do_mem_mm(int argc, char *argv[])
>  {
> - int ret = 0;
> + int ret;
>   int fd;
>   char *filename = "/dev/mem";
>   int mode = O_RWSIZE_4;
> @@ -65,9 +65,9 @@ static int do_mem_mm(int argc, char *argv[])
>   goto out_write;
>   break;
>   case O_RWSIZE_4:
> + ret = pread(fd, , 4, adr);
>   if (ret < 0)
>   goto out_read;
> - ret = pread(fd, , 4, adr);
>   val32 &= ~mask;
>   val32 |= (value & mask);
>   ret = pwrite(fd, , 4, adr);
> @@ -75,9 +75,9 @@ static int do_mem_mm(int argc, char *argv[])
>   goto out_write;
>   break;
>   case O_RWSIZE_8:
> + ret = pread(fd, , 8, adr);
>   if (ret < 0)
>   goto out_read;
> - ret = pread(fd, , 8, adr);
>   val64 &= ~mask;
>   val64 |= (value & mask);
>   ret = pwrite(fd, , 8, adr);
> -- 
> 2.30.2
> 
> 
> 

-- 
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] commands: add pm_domain for listing power domains

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 09:04:48AM +0200, Ahmad Fatoum wrote:
> Like the regulator command, this new pm_domain command gives an easy
> way for listing registered power domains and whether barebox had them
> activated.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  commands/Kconfig |  8 
>  commands/Makefile|  1 +
>  commands/pm_domain.c | 18 ++
>  drivers/base/power.c | 10 ++
>  include/pm_domain.h  |  2 ++
>  5 files changed, 39 insertions(+)
>  create mode 100644 commands/pm_domain.c

Applied, thanks

Sascha

> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 3e21dc4c0500..9894ecb9aa31 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -240,6 +240,14 @@ config CMD_REGULATOR
> the regulator command lists the currently registered regulators and
> their current state.
>  
> +config CMD_PM_DOMAIN
> + bool
> + depends on PM_GENERIC_DOMAINS
> + prompt "pm_domain command"
> + help
> +   The pm_domain command lists the currently registered power domains and
> +   their current state.
> +
>  config CMD_NVMEM
>   bool
>   depends on NVMEM
> diff --git a/commands/Makefile b/commands/Makefile
> index 0aae8893d696..068fbb32dad7 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -116,6 +116,7 @@ obj-$(CONFIG_CMD_READF)   += readf.o
>  obj-$(CONFIG_CMD_MENUTREE)   += menutree.o
>  obj-$(CONFIG_CMD_2048)   += 2048.o
>  obj-$(CONFIG_CMD_REGULATOR)  += regulator.o
> +obj-$(CONFIG_CMD_PM_DOMAIN)  += pm_domain.o
>  obj-$(CONFIG_CMD_LSPCI)  += lspci.o
>  obj-$(CONFIG_CMD_IMD)+= imd.o
>  obj-$(CONFIG_CMD_HWCLOCK)+= hwclock.o
> diff --git a/commands/pm_domain.c b/commands/pm_domain.c
> new file mode 100644
> index ..ec8b769df193
> --- /dev/null
> +++ b/commands/pm_domain.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include 
> +#include 
> +#include 
> +
> +static int do_pm_domain(int argc, char *argv[])
> +{
> + pm_genpd_print();
> +
> + return 0;
> +}
> +
> +BAREBOX_CMD_START(pm_domain)
> + .cmd= do_pm_domain,
> + BAREBOX_CMD_DESC("list power domains")
> + BAREBOX_CMD_GROUP(CMD_GRP_INFO)
> +BAREBOX_CMD_END
> diff --git a/drivers/base/power.c b/drivers/base/power.c
> index 4a206051b137..3eabf3c897b2 100644
> --- a/drivers/base/power.c
> +++ b/drivers/base/power.c
> @@ -266,3 +266,13 @@ int genpd_dev_pm_attach(struct device_d *dev)
>   return __genpd_dev_pm_attach(dev, dev->device_node, 0, true);
>  }
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
> +
> +void pm_genpd_print(void)
> +{
> + struct generic_pm_domain *genpd;
> +
> + printf("%-20s %6s\n", "name", "active");
> + list_for_each_entry(genpd, _list, gpd_list_node)
> + printf("%-20s %6s\n", genpd->name,
> +genpd->status == GPD_STATE_ACTIVE ? "on" : "off");
> +}
> diff --git a/include/pm_domain.h b/include/pm_domain.h
> index 48fd170007fd..ff1aa3751165 100644
> --- a/include/pm_domain.h
> +++ b/include/pm_domain.h
> @@ -54,6 +54,8 @@ int pm_genpd_init(struct generic_pm_domain *genpd, void 
> *gov, bool is_off);
>  int of_genpd_add_provider_simple(struct device_node *np,
>struct generic_pm_domain *genpd);
>  
> +void pm_genpd_print(void);
> +
>  #else
>  
>  static inline int pm_genpd_init(struct generic_pm_domain *genpd,
> -- 
> 2.30.2
> 
> 
> 

-- 
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 0/3] tftp fixups

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 10:56:55AM +0200, Enrico Scholz wrote:
> Fixups for the "tftp" branch in next.
> 
> Enrico Scholz (3):
>   fixup! tftp: implement 'windowsize' (RFC 7440) support
>   fixup! tftp: detect out-of-memory situations
>   tftp: make read() fail in error case
> 
>  fs/tftp.c | 29 ++---
>  1 file changed, 22 insertions(+), 7 deletions(-)

Applied, thanks

-- 
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 00/32] treewide: fix some clang-analyze static analyzer warnings

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 11:55:25AM +0200, Ahmad Fatoum wrote:
> I ran scan-build while building barebox for sandbox with clang and
> patched some of the reported issues.
> 
> Ahmad Fatoum (32):
>   clk: define stub implementation for clk_get_parent
>   clk: have SCMI and SiFive clock controllers depend on COMMON_CLK
>   meminfo: support SANDBOX build with DEBUG log level
>   net: phy: micrel: drop useless assignment of dummy read
>   mci: core: drop useless assignment
>   nvmem: core: propagate read failure
>   fs: remove never-read initializer in mount_all()
>   usb: otg: always propagate failure to register parameters
>   usb: dwc2: gracefully handle unknown hs_phy_type
>   state: propagate failure to fixup enum32 into DT
>   of: silence warning about never-read error assignment
>   commands: trigger: drop unused variable
>   commands: tutorial: fix memory leak
>   bthread: fix null pointer dereference in error path
>   common: env: drop never-read initialization
>   of: refactor for of_fixup_reserved_memory() for clarity
>   password: avoid static analyzer false positive
>   regmap-mmio: regmap_mmio_get_min_stride: unify branches for
> readability
>   crypto: caam - delete unused variable
>   misc: ubootvar: always initialize struct ubootvar_data::flag
>   nvmem: core: drop always true condition
>   of: fdt: gracefully handle out-of-place properties
>   of: overlay: drop unused variable of_overlay_apply_dir()
>   of: partition: drop unused variable
>   serial: ns16550_pci: drop useless assignment
>   phy: core: drop useless else clause
>   fs: ext4: ext_barebox: handle ext_get_inode() errors
>   fs: fat: propagate f_lseek failure
>   fs: drop duplicate follow_managed() call
>   lib: parse_area_spec: guard against NULL pointer dereference
>   net: ping: propagate failure
>   net: fastboot: keep error message initialized at all times

Applied 2-10 and 12-32, thanks

Sascha

-- 
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 13/32] commands: tutorial: fix memory leak

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 11:55:38AM +0200, Ahmad Fatoum wrote:
> We don't need to duplicate argv[i], because it persists for the lieftime

s/lieftime/lifetime/

> of print_tutorial_step. We can thus drop the strdup and while at it,
> just drop the redundant step variable altogether.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  commands/tutorial.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/commands/tutorial.c b/commands/tutorial.c
> index 444177764311..afe5b66f0b0a 100644
> --- a/commands/tutorial.c
> +++ b/commands/tutorial.c
> @@ -84,7 +84,6 @@ out:
>  static int do_tutorial_next(int argc, char *argv[])
>  {
>   int opt, i;
> - char *step = NULL;
>   char *oldcwd;
>   ssize_t ret = 0;
>   bool is_prev = *argv[0] == 'p';
> @@ -121,14 +120,12 @@ static int do_tutorial_next(int argc, char *argv[])
>   next_step = next_step > 0 ? next_step - 1 : 0;
>  
>   if (optind == argc) {
> - step = steps.gl_pathv[next_step];
> - ret = print_tutorial_step(step);
> + ret = print_tutorial_step(steps.gl_pathv[next_step]);
>   if (ret == 0 && !is_prev)
>   next_step = (next_step + 1) % steps.gl_pathc;
>   } else {
>   for (i = optind; i < argc; i++) {
> - step = strdup(argv[i]);
> - ret = print_tutorial_step(step);
> + ret = print_tutorial_step(argv[i]);
>   if (ret)
>   break;
>   }
> -- 
> 2.30.2
> 
> 
> 

-- 
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 11/32] of: silence warning about never-read error assignment

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 11:55:36AM +0200, Ahmad Fatoum wrote:
> err is assigned specific error codes, but they are not propagated and
> instead NULL is returned for error. Make it explicit that we handle all
> errors the same by typecasting to (void).

This warning likely goes back to assigning the variable 'err' right
before jumping to the label 'err':

err = -EINVAL;
goto err;

The assignment is indeed unused. We should remove this assignment
rather than suppressing the resulting warning.

Sascha

> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  drivers/of/resolver.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
> index 2457ae96a412..510d36f95192 100644
> --- a/drivers/of/resolver.c
> +++ b/drivers/of/resolver.c
> @@ -272,6 +272,7 @@ out:
>  err:
>   of_delete_node(result);
>  
> + (void)err;
>   return NULL;
>  
>  }
> -- 
> 2.30.2
> 
> 
> 

-- 
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 01/32] clk: define stub implementation for clk_get_parent

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 11:55:26AM +0200, Ahmad Fatoum wrote:
> To make it easier to build drivers utilizing clk_get_parent on sandbox
> for static analysis, provide a stub implementation of the function.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  include/linux/clk.h | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)

This will break architectures which do not use the common clk framework
and implement clk_get_parent themselves.

Fortunately there's only one architecture left doing this: AT91

Sascha

> 
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 42c64d650d1f..b18fc733843d 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -128,14 +128,6 @@ int clk_hw_set_rate(struct clk_hw *hw, unsigned long 
> rate);
>  int clk_set_parent(struct clk *clk, struct clk *parent);
>  int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *hwp);
>  
> -/**
> - * clk_get_parent - get the parent clock source for this clock
> - * @clk: clock source
> - *
> - * Returns struct clk corresponding to parent clock source, or
> - * valid IS_ERR() condition containing errno.
> - */
> -struct clk *clk_get_parent(struct clk *clk);
>  struct clk_hw *clk_hw_get_parent(struct clk_hw *hw);
>  
>  int clk_set_phase(struct clk *clk, int degrees);
> @@ -658,6 +650,14 @@ struct clk_hw *of_clk_hw_simple_get(struct 
> of_phandle_args *clkspec, void *data)
>  struct clk *of_clk_get(struct device_node *np, int index);
>  struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
>  struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
> +/**
> + * clk_get_parent - get the parent clock source for this clock
> + * @clk: clock source
> + *
> + * Returns struct clk corresponding to parent clock source, or
> + * valid IS_ERR() condition containing errno.
> + */
> +struct clk *clk_get_parent(struct clk *clk);
>  unsigned int of_clk_get_parent_count(struct device_node *np);
>  int of_clk_parent_fill(struct device_node *np, const char **parents,
>  unsigned int size);
> @@ -717,6 +717,10 @@ static inline unsigned int 
> of_clk_get_parent_count(struct device_node *np)
>  {
>   return 0;
>  }
> +static inline struct clk *clk_get_parent(struct clk *clk)
> +{
> + return ERR_PTR(-ENOENT);
> +}
>  static inline int of_clk_init(struct device_node *root,
> const struct of_device_id *matches)
>  {
> -- 
> 2.30.2
> 
> 
> 

-- 
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 0/2] of: overlay: avoid potential null pointer exception

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 12:07:15PM +0200, Michael Riesch wrote:
> Hi all,
> 
> The function of_overlay_fix_path returns NULL in certain error cases but
> of_overlay_apply_symbols (which is the only caller) does not check the
> return value. For broken overlays this may result in a null pointer
> exception. Fix this by checking the return value and inform the user
> what exactly went wrong. To this end, improve the error handling in
> of_overlay_apply_tree.
> 
> The thread [0] gives a bit more context.
> 
> Best regards,
> Michael
> 
> [0] 
> https://lore.barebox.org/barebox/95ff064f-aa11-c1ce-9d41-e38f2040c...@wolfvision.net/T/#u
> 
> Michael Riesch (2):
>   of: overlay: improve error handling in of_overlay_apply_tree
>   of: overlay: avoid potential null pointer exception

Applied, thanks

Sascha

> 
>  drivers/of/overlay.c | 18 +-
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 

-- 
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: Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name

2022-09-12 Thread Sascha Hauer
On Wed, Sep 07, 2022 at 11:21:18AM +0300, Antony Pavlov wrote:
> Hi Everyone!
> 
> During MIPS ath79_defconfig build I have a 'missing whitespace after the 
> macro name' warning:
> 
>   AS [P]  arch/mips/dts/ar9331_tl_mr3020.dtb.pbl.o
>   DTC arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb
> :1:9: warning: missing whitespace after the macro name
>   XZKERN  arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb.z
> 
> E.g. see https://gitlab.com/frantony/barebox/-/jobs/2969826747#L47
> 
> The reason is the scripts/Makefile.lib generates a C macro with the point 
> symbol in the macro name because the arch/mips/dts/ar9344-tl-wdr4300-v1.7.dts 
> file name contains the point symbol before '.dts', as a result we have:
> 
>   #define ar9344_tl_wdr4300_v1.7_dts 1
> 
> e.g.
> 
>   barebox$ grep -RHn -o "define ar9344_tl_wdr4300_v1.* 1" . 2>/dev/null
>   ./arch/mips/dts/.ar9344-tl-wdr4300-v1.7.dtb.cmd:1:define 
> ar9344_tl_wdr4300_v1.7_dts 1
> 
> cmd_dtc in scripts/Makefile.lib substitutes the '-' symbols with the '_' 
> symbols but do nothing with other unwanted C preprocessor macro name symbols.
> 
> It looks like the linux kernel has no problems with extra point symbols in 
> dts file names, there are several files with extra dot in dts:
> 
>   barebox$ find dts/ -iname '*.*.dts' | wc -l
>   33
> 
> So we have to fix Makefile.lib.
> 
> This simple patch fixes the warning problem:
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 16308497b84..2f79656c1e9 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -363,7 +363,7 @@ $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
>  dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
>  quiet_cmd_dtc = DTC $@
>  # For compatibility between make 4.2 and 4.3
> -cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 
> 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
> +cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(subst 
> .,_,$(*F)))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | 
> \
>   $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
>   $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
>   -i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
> 
> I suppose that this simple patch may lead to some undesirable side effects.

One side effect is that this gets even less readable.

Another one would be that two dts filenames which only differ in the
usage of '.' and '_' would result in the same define, but I think that
case is negligible as this define is unused in barebox itself. It could
be used by external dts fragments passed in via CONFIG_EXTERNAL_DTS_FRAGMENTS.

Other than that, what side effects are you afraid of?

Sascha

-- 
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 0/5] commands: gpio: add controller as optional argument

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 12:35:41PM +0200, Ahmad Fatoum wrote:
> gpioinfo currently displays all GPIOs and the rest of the GPIO commands
> like gpio_direction_output expects either a GPIO label or a global GPIO
> number to act upon.
> 
> With this series, the gpio commands now accept an optional argument,
> e.g.:
> 
>   gpioinfo gpio4
>   gpio_direction_output -d gpio4 20 1
> 
> Controllers are identified by either device name, full path to the
> device tree node backing it or an alias pointing at it.
> 
> Ahmad Fatoum (5):
>   gpiolib: implement gpio_get_chip_by_dev()
>   of: platform: optimize of_find_device_by_node when deep probing
>   driver: implement find_device() helper
>   commands: gpio: add -d argument to set/get commands
>   gpiolib: gpioinfo: add optional CONTROLLER command line argument

Applied, thanks

Sascha

> 
>  commands/gpio.c| 51 +---
>  drivers/base/driver.c  | 16 
>  drivers/gpio/gpiolib.c | 59 --
>  drivers/of/platform.c  |  3 +++
>  include/driver.h   |  5 
>  include/gpio.h |  1 +
>  6 files changed, 111 insertions(+), 24 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 

-- 
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: Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name

2022-09-12 Thread Antony Pavlov
On Mon, 12 Sep 2022 10:01:52 +0200
Sascha Hauer  wrote:

Hi Sascha!

> On Wed, Sep 07, 2022 at 11:21:18AM +0300, Antony Pavlov wrote:
> > Hi Everyone!
> > 
> > During MIPS ath79_defconfig build I have a 'missing whitespace after the 
> > macro name' warning:
> > 
> >   AS [P]  arch/mips/dts/ar9331_tl_mr3020.dtb.pbl.o
> >   DTC arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb
> > :1:9: warning: missing whitespace after the macro name
> >   XZKERN  arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb.z
> > 
> > E.g. see https://gitlab.com/frantony/barebox/-/jobs/2969826747#L47
> > 
> > The reason is the scripts/Makefile.lib generates a C macro with the point 
> > symbol in the macro name because the 
> > arch/mips/dts/ar9344-tl-wdr4300-v1.7.dts file name contains the point 
> > symbol before '.dts', as a result we have:
> > 
> >   #define ar9344_tl_wdr4300_v1.7_dts 1
> > 
> > e.g.
> > 
> >   barebox$ grep -RHn -o "define ar9344_tl_wdr4300_v1.* 1" . 2>/dev/null
> >   ./arch/mips/dts/.ar9344-tl-wdr4300-v1.7.dtb.cmd:1:define 
> > ar9344_tl_wdr4300_v1.7_dts 1
> > 
> > cmd_dtc in scripts/Makefile.lib substitutes the '-' symbols with the '_' 
> > symbols but do nothing with other unwanted C preprocessor macro name 
> > symbols.
> > 
> > It looks like the linux kernel has no problems with extra point symbols in 
> > dts file names, there are several files with extra dot in dts:
> > 
> >   barebox$ find dts/ -iname '*.*.dts' | wc -l
> >   33
> > 
> > So we have to fix Makefile.lib.
> > 
> > This simple patch fixes the warning problem:
> > 
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 16308497b84..2f79656c1e9 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -363,7 +363,7 @@ $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
> >  dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
> >  quiet_cmd_dtc = DTC $@
> >  # For compatibility between make 4.2 and 4.3
> > -cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 
> > 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
> > +cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(subst 
> > .,_,$(*F)))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') 
> > | \
> > $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
> > $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
> > -i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
> > 
> > I suppose that this simple patch may lead to some undesirable side effects.
> 
> One side effect is that this gets even less readable.

Yes, you are right :)

> Another one would be that two dts filenames which only differ in the
> usage of '.' and '_' would result in the same define, but I think that
> case is negligible as this define is unused in barebox itself. It could
   
> be used by external dts fragments passed in via CONFIG_EXTERNAL_DTS_FRAGMENTS.
> 
> Other than that, what side effects are you afraid of?

I have an idea that someone relies on these *_dts macros.

Now I see (as you have noted above) that there is no macro users in mainline 
source tree:

  $ git grep -w .*_dts
  common/Kconfig:   #ifdef foo_board_dts
  scripts/Makefile.lib:cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst 
-,_,$(*F))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \

-- 
Best regards,
  Antony Pavlov



Re: [PATCH 1/2] i2c: implement of_i2c_register_devices_by_node()

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 12:36:12PM +0200, Ahmad Fatoum wrote:
> i2c controllers probe the devices on their bus themselves, so in the
> case the i2c controller was already probed, we currently have no way to
> have it reread the device tree after fixing it up or applying an
> overlay. Add a new helper that retriggers the device creation.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  drivers/i2c/i2c.c | 20 
>  include/i2c/i2c.h |  1 +
>  2 files changed, 21 insertions(+)

Applied, thanks

Sascha

> 
> diff --git a/drivers/i2c/i2c.c b/drivers/i2c/i2c.c
> index b24fd88f5b39..e70e51618850 100644
> --- a/drivers/i2c/i2c.c
> +++ b/drivers/i2c/i2c.c
> @@ -427,6 +427,12 @@ static void of_i2c_register_devices(struct i2c_adapter 
> *adap)
>   const __be32 *addr;
>   int len;
>  
> + if (n->dev) {
> + dev_dbg(>dev, "of_i2c: skipping already 
> registered %s\n",
> + dev_name(n->dev));
> + continue;
> + }
> +
>   of_modalias_node(n, info.type, I2C_NAME_SIZE);
>  
>   info.of_node = n;
> @@ -452,6 +458,20 @@ static void of_i2c_register_devices(struct i2c_adapter 
> *adap)
>   }
>  }
>  
> +int of_i2c_register_devices_by_node(struct device_node *node)
> +{
> + struct i2c_adapter *adap;
> +
> + adap = of_find_i2c_adapter_by_node(node);
> + if (!adap)
> + return -ENODEV;
> + if (IS_ERR(adap))
> + return PTR_ERR(adap);
> +
> + of_i2c_register_devices(adap);
> + return 0;
> +}
> +
>  /**
>   * i2c_new_dummy - return a new i2c device bound to a dummy driver
>   * @adapter: the adapter managing the device
> diff --git a/include/i2c/i2c.h b/include/i2c/i2c.h
> index 7207b1180e1d..f5e2dc511ed1 100644
> --- a/include/i2c/i2c.h
> +++ b/include/i2c/i2c.h
> @@ -295,6 +295,7 @@ static inline int i2c_register_board_info(int busnum,
>  extern int i2c_add_numbered_adapter(struct i2c_adapter *adapter);
>  struct i2c_adapter *i2c_get_adapter(int busnum);
>  struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
> +int of_i2c_register_devices_by_node(struct device_node *node);
>  struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
>  
>  void i2c_parse_fw_timings(struct device_d *dev, struct i2c_timings *t, bool 
> use_defaults);
> -- 
> 2.30.2
> 
> 
> 

-- 
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] of: implement of_rename_property()

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 12:36:39PM +0200, Ahmad Fatoum wrote:
> It's often desirable to have the same barebox binary for multiple
> variants. DT-level differences between the variants are often handled
> by having extra device tree nodes that are disabled by default and
> patched as appropriate by board code. It can be useful to have a finer
> granularity for patching though, that covers only a select property, e.g.:
> 
>usbotg1 {
>   vbus-supply = <_vbus_common>;
>   vendor,specialvariant-vbus-supply = <_vbus_specialvariant>;
>};
> 
> The new of_rename_property allows board code to easily activate the
> alternative vbus supply on the specialvariant:
> 
>   of_rename_property(np, "vendor,specialvariant-vbus-supply",
>"vbus-supply");
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  drivers/of/base.c | 16 
>  include/of.h  |  8 
>  test/self/of_manipulation.c   | 10 ++
>  test/self/of_manipulation.dts |  6 --
>  4 files changed, 38 insertions(+), 2 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6a51dd71793d..212b226eb55c 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2301,6 +2301,22 @@ void of_delete_property(struct property *pp)
>   free(pp);
>  }
>  
> +struct property *of_rename_property(struct device_node *np,
> + const char *old_name, const char *new_name)
> +{
> + struct property *pp;
> +
> + pp = of_find_property(np, old_name, NULL);
> + if (!pp)
> + return NULL;
> +
> + of_property_write_bool(np, new_name, false);
> +
> + free(pp->name);
> + pp->name = xstrdup(new_name);
> + return pp;
> +}
> +
>  /**
>   * of_set_property - create a property for a given node
>   * @node - the node
> diff --git a/include/of.h b/include/of.h
> index 29df7d912b33..153e75d3e51d 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -143,6 +143,8 @@ extern struct property *of_new_property_const(struct 
> device_node *node,
>  extern struct property *__of_new_property(struct device_node *node,
> const char *name, void *data, int 
> len);
>  extern void of_delete_property(struct property *pp);
> +extern struct property *of_rename_property(struct device_node *np,
> +const char *old_name, const char 
> *new_name);
>  
>  extern struct device_node *of_find_node_by_name(struct device_node *from,
>   const char *name);
> @@ -550,6 +552,12 @@ static inline void of_delete_property(struct property 
> *pp)
>  {
>  }
>  
> +static inline struct property *of_rename_property(struct device_node *np,
> +   const char *old_name, const 
> char *new_name)
> +{
> + return NULL;
> +}
> +
>  static inline int of_property_read_u32_index(const struct device_node *np,
>   const char *propname, u32 index, u32 *out_value)
>  {
> diff --git a/test/self/of_manipulation.c b/test/self/of_manipulation.c
> index 6eb6062e126b..7e30a60ca687 100644
> --- a/test/self/of_manipulation.c
> +++ b/test/self/of_manipulation.c
> @@ -57,6 +57,16 @@ static void test_of_basics(struct device_node *root)
>   of_property_write_bool(node1, "property1", true);
>  
>   assert_equal(node1, node2);
> +
> + of_property_write_bool(node2, "property1", false);
> + of_property_write_u32(node2, "property1", 1);
> + of_property_write_u32(node2, "property2", 2);
> +
> + of_property_write_u32(node1, "property3", 1);
> + of_property_write_u32(node1, "property2", 2);
> + of_rename_property(node1, "property3", "property1");
> +
> + assert_equal(node1, node2);
>  }
>  
>  static void test_of_property_strings(struct device_node *root)
> diff --git a/test/self/of_manipulation.dts b/test/self/of_manipulation.dts
> index a69d944c1eb5..2cc6773fa98e 100644
> --- a/test/self/of_manipulation.dts
> +++ b/test/self/of_manipulation.dts
> @@ -4,12 +4,14 @@
>  
>  / {
>   node1 {
> - property1;
> + property1 = <1>;
> + property2 = <2>;
>   node21 { };
>   };
>  
>   node2 {
> - property1;
> + property1 = <1>;
> + property2 = <2>;
>   node21 { };
>   };
>  
> -- 
> 2.30.2
> 
> 
> 

-- 
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 next] Documentation: filesystems: tftp: fix code blocks

2022-09-12 Thread Antony Pavlov
The patch fixes these sphinx errors:

  Documentation/filesystems/tftp.rst:44: ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 3 supplied.
  Documentation/filesystems/tftp.rst:53: ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 4 supplied.

Signed-off-by: Antony Pavlov 
---
 Documentation/filesystems/tftp.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/filesystems/tftp.rst 
b/Documentation/filesystems/tftp.rst
index 8929213d3c4..e88ad3dd4c8 100644
--- a/Documentation/filesystems/tftp.rst
+++ b/Documentation/filesystems/tftp.rst
@@ -42,6 +42,7 @@ the opposite effect.  Performance depends on:
100 Mb/s) it had to be reduced to
 
.. code-block:: console
+
  global tftp.windowsize=26
 
for example.
@@ -51,6 +52,7 @@ the opposite effect.  Performance depends on:
example, the `fec-imx` driver reserves place for
 
.. code-block:: c
+
  #define FEC_RBD_NUM   64
 
packets before they are dropped
-- 
2.37.2




Re: [PATCH] of: platform: propagate of_devices_ensure_probed_by(name|property) errors

2022-09-12 Thread Sascha Hauer
On Mon, Sep 05, 2022 at 01:59:44PM +0200, Ahmad Fatoum wrote:
> The of_devices_ensure_probed_by functions are expected to return an
> error code after iterating over all matching devices should any device
> have failed its of_device_ensure_probed. Doing this unearths one common
> failure: a matching node has status = "disabled". These will have
> of_device_ensure_probed return -ENODEV, which makes sense for users
> wanting to ensure a specific device is probed, but doesn't when
> iterating over multiple nodes.
> 
> We already have of_devices_ensure_probed_by_dev_id, which does an early
> of_device_is_available check, so do likewise for the other to
> ensure_probed_by_* functions.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  drivers/of/platform.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index dc784ea8e550..a9a5d4c2daf2 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -525,12 +525,15 @@ int of_devices_ensure_probed_by_property(const char 
> *property_name)
>   return 0;
>  
>   for_each_node_with_property(node, property_name) {
> - ret = of_device_ensure_probed(node);
> + if (!of_device_is_available(node))
> + continue;
> +
> + err = of_device_ensure_probed(node);
>   if (err)
>   ret = err;
>   }
>  
> - return 0;
> + return ret;
>  }
>  EXPORT_SYMBOL_GPL(of_devices_ensure_probed_by_property);
>  
> @@ -543,12 +546,15 @@ int of_devices_ensure_probed_by_name(const char *name)
>   return 0;
>  
>   for_each_node_by_name(node, name) {
> - ret = of_device_ensure_probed(node);
> + if (!of_device_is_available(node))
> + continue;
> +
> + err = of_device_ensure_probed(node);
>   if (err)
>   ret = err;
>   }
>  
> - return 0;
> + return ret;
>  }
>  EXPORT_SYMBOL_GPL(of_devices_ensure_probed_by_name);
>  
> -- 
> 2.30.2
> 
> 
> 

-- 
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- |



[RFC] Documentation/conf.py: fix copyright years

2022-09-12 Thread Antony Pavlov
N.B. N-dash is used!

Signed-off-by: Antony Pavlov 
---
 Documentation/conf.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index bcd8633c919..5fb8b07c380 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -44,7 +44,7 @@ master_doc = 'index'
 
 # General information about the project.
 project = u'barebox'
-copyright = u'2014, The barebox project'
+copyright = u'2014–2022, The barebox project'
 
 # The version info for the project you're documenting, acts as replacement for
 # |version| and |release|, also used in various other places throughout the
-- 
2.37.2




[PATCH 5/7] Documentation: barebox environment: fix list rendering

2022-09-12 Thread Antony Pavlov
Signed-off-by: Antony Pavlov 
---
 .../devicetree/bindings/barebox/barebox,environment.rst  | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/barebox/barebox,environment.rst 
b/Documentation/devicetree/bindings/barebox/barebox,environment.rst
index 918efd15f55..8a57bf1add8 100644
--- a/Documentation/devicetree/bindings/barebox/barebox,environment.rst
+++ b/Documentation/devicetree/bindings/barebox/barebox,environment.rst
@@ -9,6 +9,7 @@ Required properties:
 * ``device-path``: path to the device environment is on
 
 Optional properties:
+
 * ``file-path``: path to a file in the device named by device-path
 
 The device-path is a multistring property. The first string should contain
-- 
2.37.2




[PATCH 7/7] Documentation: Voltage/Current Regulators: fix sphinx error

2022-09-12 Thread Antony Pavlov
sphinx error:

Documentation/devicetree/bindings/regulator/regulator.rst:8:
  ERROR: Unexpected indentation.

Signed-off-by: Antony Pavlov 
---
 Documentation/devicetree/bindings/regulator/regulator.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.rst 
b/Documentation/devicetree/bindings/regulator/regulator.rst
index 9afc020acd3..754c4743286 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.rst
+++ b/Documentation/devicetree/bindings/regulator/regulator.rst
@@ -4,6 +4,7 @@ Voltage/Current Regulators
 In addition to the upstream bindings, another property is added:
 
 Optional properties:
+
 - ``barebox,allow-dummy-supply`` : A property to allow usage of dummy power
   regulator. This can be added to regulator nodes, whose drivers are not yet
   supported. It will rely on regulator reset defaults and use of dummy 
regulator
-- 
2.37.2




[PATCH 1/7] Documentation: OKUD Max9331: fix sphinx warning

2022-09-12 Thread Antony Pavlov
sphinx warning:

  Documentation/boards/mips/max9331.rst:129: WARNING:
Pygments lexer name 'assembly' is not known

Use pygments lexer for Gas (AT) assembly code, see
https://pygments.org/docs/lexers/#lexers-for-assembly-languages
for details.

Signed-off-by: Antony Pavlov 
---
 Documentation/boards/mips/max9331.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/boards/mips/max9331.rst 
b/Documentation/boards/mips/max9331.rst
index f09dabc2da1..f7529f9874d 100644
--- a/Documentation/boards/mips/max9331.rst
+++ b/Documentation/boards/mips/max9331.rst
@@ -126,7 +126,7 @@ it jump to 0x9f02 where the first instruction of 
barebox.
 This is usefull when debug with jtag or choosing different bootloaders.
 or even boot kernel without bootloader.
 
-.. code-block:: assembly
+.. code-block:: asm
 
  lui  ra, 0x9f02
  jr   ra
-- 
2.37.2




[PATCH 6/7] Documentation: Common leds properties: fix 'panic-indicator' list entry rendering

2022-09-12 Thread Antony Pavlov
Signed-off-by: Antony Pavlov 
---
 Documentation/devicetree/bindings/leds/common.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/common.rst 
b/Documentation/devicetree/bindings/leds/common.rst
index 911a55f4f66..09b4e401bc2 100644
--- a/Documentation/devicetree/bindings/leds/common.rst
+++ b/Documentation/devicetree/bindings/leds/common.rst
@@ -14,4 +14,4 @@ Common leds properties
   from the node name (excluding the unit address).
 
 * ``panic-indicator`` - This property specifies that the LED should be used as 
a
- panic indicator.
+  panic indicator.
-- 
2.37.2




[PATCH 4/7] Documentation: barebox DT aliases: fix sphinx issues

2022-09-12 Thread Antony Pavlov
sphinx issues:

  Documentation/devicetree/bindings/barebox/aliases.rst:8:
ERROR: Unexpected indentation.
  Documentation/devicetree/bindings/barebox/aliases.rst:12:
WARNING: Definition list ends without a blank line; unexpected unindent.
  Documentation/devicetree/bindings/barebox/aliases.rst:13:
WARNING: Definition list ends without a blank line; unexpected unindent.

Signed-off-by: Antony Pavlov 
---
 Documentation/devicetree/bindings/barebox/aliases.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/barebox/aliases.rst 
b/Documentation/devicetree/bindings/barebox/aliases.rst
index 527cc85ef69..e6face2c331 100644
--- a/Documentation/devicetree/bindings/barebox/aliases.rst
+++ b/Documentation/devicetree/bindings/barebox/aliases.rst
@@ -2,9 +2,10 @@ barebox DT aliases
 ==
 
 barebox can use the properties in the ``/aliases`` node to arrive
-at deterministic names for devices, e.g.::
+at deterministic names for devices, e.g.:
 
 .. code-block:: none
+
/ {
aliases {
mmc0 = 
-- 
2.37.2




[PATCH 3/7] Documentation: Altera FPGAs in passive-serial mode: fix sphinx issues

2022-09-12 Thread Antony Pavlov
sphinx issues:

  Documentation/devicetree/bindings/firmware/altr,passive-serial.rst:10:
ERROR: Unexpected indentation.
  Documentation/devicetree/bindings/firmware/altr,passive-serial.rst:11:
WARNING: Block quote ends without a blank line; unexpected unindent.

Insert necessary empty line to fix the problem.

Signed-off-by: Antony Pavlov 
---
 .../devicetree/bindings/firmware/altr,passive-serial.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/firmware/altr,passive-serial.rst 
b/Documentation/devicetree/bindings/firmware/altr,passive-serial.rst
index 1012137bc93..cbddd700ce9 100644
--- a/Documentation/devicetree/bindings/firmware/altr,passive-serial.rst
+++ b/Documentation/devicetree/bindings/firmware/altr,passive-serial.rst
@@ -6,6 +6,7 @@ passive serial mode. This is used to upload the firmware and
 to start the FPGA.
 
 Required properties:
+
 - ``compatible``: shall be ``"altr,fpga-passive-serial"`` or
   ``"altr,fpga-arria10-passive-serial"`` for Arria 10
 - ``reg``: SPI chip select
-- 
2.37.2




[PATCH 2/7] Documentation: Altera SOCFPGA FPGA Manager: fix sphinx error

2022-09-12 Thread Antony Pavlov
sphinx error:

  Documentation/devicetree/bindings/firmware/altr,socfpga-fpga-mgr.rst:10:
   ERROR: Unexpected indentation.

Insert necessary whitespaces and empty lines to form correct
nested lists structure.

Signed-off-by: Antony Pavlov 
---
 .../devicetree/bindings/firmware/altr,socfpga-fpga-mgr.rst  | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/firmware/altr,socfpga-fpga-mgr.rst 
b/Documentation/devicetree/bindings/firmware/altr,socfpga-fpga-mgr.rst
index 9f7de6b9858..478e8e6fc44 100644
--- a/Documentation/devicetree/bindings/firmware/altr,socfpga-fpga-mgr.rst
+++ b/Documentation/devicetree/bindings/firmware/altr,socfpga-fpga-mgr.rst
@@ -5,10 +5,12 @@ This binding defines the FPGA Manager on Altera SOCFPGAs. 
This is used to upload
 the firmware to the FPGA part of the SoC.
 
 Required properties:
+
 - ``compatible``: shall be ``"altr,socfpga-fpga-mgr"``
 - ``reg``: Must contain 2 register ranges:
-   1. The control address space of the FPGA manager.
-   2. The configuration data address space where the firmware data is 
written to.
+
+  1. The control address space of the FPGA manager.
+  2. The configuration data address space where the firmware data is written 
to.
 
 Example:
 
-- 
2.37.2




[PATCH 0/7] Documentation misc fixes

2022-09-12 Thread Antony Pavlov
Antony Pavlov (7):
  Documentation: OKUD Max9331: fix sphinx warning
  Documentation: Altera SOCFPGA FPGA Manager: fix sphinx error
  Documentation: Altera FPGAs in passive-serial mode: fix sphinx issues
  Documentation: barebox DT aliases: fix sphinx issues
  Documentation: barebox environment: fix list rendering
  Documentation: Common leds properties: fix 'panic-indicator' list
entry rendering
  Documentation: Voltage/Current Regulators: fix sphinx error

 Documentation/boards/mips/max9331.rst   | 2 +-
 Documentation/devicetree/bindings/barebox/aliases.rst   | 3 ++-
 .../devicetree/bindings/barebox/barebox,environment.rst | 1 +
 .../devicetree/bindings/firmware/altr,passive-serial.rst| 1 +
 .../devicetree/bindings/firmware/altr,socfpga-fpga-mgr.rst  | 6 --
 Documentation/devicetree/bindings/leds/common.rst   | 2 +-
 Documentation/devicetree/bindings/regulator/regulator.rst   | 1 +
 7 files changed, 11 insertions(+), 5 deletions(-)

-- 
2.37.2




Re: [PATCH] of: implement of_prepend_property

2022-09-12 Thread Sascha Hauer
On Fri, Sep 09, 2022 at 09:40:50AM +0200, Ahmad Fatoum wrote:
> Like of_append_property for adding at the end of properties, implement
> of_prepend_property for placing data into the front.
> 
> This is especially useful to fixup most-specific compatibles into
> existing nodes.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  drivers/of/base.c   | 35 +++
>  include/of.h|  8 
>  test/self/of_manipulation.c |  2 +-
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 212b226eb55c..e208949b08bd 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2378,6 +2378,41 @@ int of_append_property(struct device_node *np, const 
> char *name, const void *val
>   return 0;
>  }
>  
> +int of_prepend_property(struct device_node *np, const char *name, const void 
> *val, int len)
> +{
> + struct property *pp;
> + int orig_len;
> + const void *orig_buf;
> + void *buf;
> +
> + if (!np)
> + return -ENOENT;
> +
> + pp = of_find_property(np, name, NULL);
> + if (!pp) {
> + of_new_property(np, name, val, len);
> + return 0;
> + }
> +
> + orig_len = pp->length;
> + buf = realloc(pp->value, orig_len + len);
> + if (!buf)
> + return -ENOMEM;
> +
> + orig_buf = pp->value_const ?: buf;
> + if (orig_buf) {

orig_buf is always non NULL here.

> + memmove(buf + len, orig_buf, orig_len);
> + pp->value_const = NULL;
> + }
> +
> + memcpy(buf, val, len);
> +
> + pp->value = buf;
> + pp->length += len;
> +
> + return 0;
> +}

How about a bit more straight forward variant like this:

int of_prepend_property(struct device_node *np, const char *name, const void 
*val, int len)
{
struct property *pp;
int oldlen = 0;

pp = of_find_property(np, name, );
if (!pp) {
of_new_property(np, name, val, len);
return 0;
}

oldval = of_property_get_value(pp);

buf = malloc(len + oldlen);
if (!buf)
return -ENOMEM;

memcpy(buf, val, len);
memcpy(buf + len, oldval, oldlen);

free(pp->value);
pp->value = buf;
pp->length = len + oldlen;
pp->value_const = NULL;

return 0;
}

Sascha

-- 
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] ARM: i.MX23: fix memory size calulcation

2022-09-12 Thread Sascha Hauer
On Fri, Sep 09, 2022 at 04:39:15PM +0200, Bastian Krause wrote:
> According to the i.MX233 Reference Manual (i.MX233RM, Rev. 4, 03 April
> 2009) section "14.1.1 AHB Address Ranges", the formula to calculate the
> DRAM memory size is:
> 
>   dram_memory_available = 2 * 2^col * 2^row * (# dram_devices) * (# 
> banks_per_device)
> 
> The calulcation in barebox misses the first factor, so the result is
> only half as big as it should be.
> 
> On an iMX233 OLinuXino board the following errors can be observed:
> 
>   mmu: Critical Error: Can't request SDRAM region for ttb at 43fe4000
>   Error: Cannot request SDRAM region for stack
> 
> The faulty calulcation leads to 32 MB being detected (as seen in the
> output of `iomem`), although 64 MB should be available (as passed to
> barebox_arm_entry() in lowlevel code).
> 
> Fix the memory calculation by adding the missing factor 2.
> 
> Fixes: 7158c5987e6 ("ARM: i.MX23: Add memory size detection")
> Signed-off-by: Bastian Krause 
> Signed-off-by: Jürgen Borleis 
> ---
>  arch/arm/mach-mxs/include/mach/imx23.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to master, thanks

Sascha

> 
> diff --git a/arch/arm/mach-mxs/include/mach/imx23.h 
> b/arch/arm/mach-mxs/include/mach/imx23.h
> index bdd3ae4407a..03eddabed0a 100644
> --- a/arch/arm/mach-mxs/include/mach/imx23.h
> +++ b/arch/arm/mach-mxs/include/mach/imx23.h
> @@ -25,7 +25,7 @@ static inline u32 imx23_get_memsize(void)
>   cs0 = FIELD_GET(DRAM_CTL14_CS0_EN, ctl14);
>   cs1 = FIELD_GET(DRAM_CTL14_CS1_EN, ctl14);
>  
> - return (1 << columns) * (1 << rows) * banks * (cs0 + cs1);
> + return 2 * (1 << columns) * (1 << rows) * banks * (cs0 + cs1);
>  }
>  
>  #endif /* __MACH_IMX23_H */
> -- 
> 2.30.2
> 
> 
> 

-- 
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- |