[PATCH] mci: bcm2835: Set mci host for sdhci

2024-05-22 Thread Jonas Richardsen
The pointer `sdhci.mci` is currently not being set for the bcm2835. This
leads to a null pointer dereference for example in `sdhci_wait_idle()`
if the `sdhci_read` function fails or times out.

Set the pointer within the `bcm2835_mci_probe` function. This is
analogous to the behaviour seen in `arasan_sdhci_probe`,
`fsl_esdhc_probe`, `rk_sdhci_probe` and other, similar functions.

Signed-off-by: Jonas Richardsen 
---
 drivers/mci/mci-bcm2835.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mci/mci-bcm2835.c b/drivers/mci/mci-bcm2835.c
index 3546cc3a32..7fcf4f905b 100644
--- a/drivers/mci/mci-bcm2835.c
+++ b/drivers/mci/mci-bcm2835.c
@@ -382,6 +382,7 @@ static int bcm2835_mci_probe(struct device *hw_dev)
host->hw_dev = hw_dev;
host->max_clock = clk_get_rate(clk);
 
+   host->sdhci.mci = >mci;
host->sdhci.read32 = bcm2835_sdhci_read32;
host->sdhci.write32 = bcm2835_sdhci_write32;
 
-- 
2.42.0




[PATCH 3/3] raspi: add a fixup for the `dma-ranges` property of the `/emmc2bus` dt node

2024-04-22 Thread Jonas Richardsen
As mentioned in `bcm2711.dtsi`, line 412, the `dma-ranges` property
of the `/emmc2bus` node is overridden by the video core firmware:
> /*
>  * emmc2 has different DMA constraints based on SoC revisions. It was
>  * moved into its own bus, so as for RPi4's firmware to update them.
>  * The firmware will find whether the emmc2bus alias is defined, and if
>  * so, it'll edit the dma-ranges property below accordingly.
>  */
This change adds a fixup that copies the correct value of this property
from the video core device tree.

Signed-off-by: Jonas Richardsen 
---
 arch/arm/boards/raspberry-pi/rpi-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boards/raspberry-pi/rpi-common.c 
b/arch/arm/boards/raspberry-pi/rpi-common.c
index 3c685852d3..04e1a899f1 100644
--- a/arch/arm/boards/raspberry-pi/rpi-common.c
+++ b/arch/arm/boards/raspberry-pi/rpi-common.c
@@ -375,6 +375,7 @@ static void rpi_vc_fdt_parse(struct device_node *root)
register_vc_property_fixup(root, "/reserved-memory/nvram@0", "reg");
register_vc_property_fixup(root, "/reserved-memory/nvram@0", "status");
register_vc_fixup(root, "/hat");
+   register_vc_property_fixup(root, "/emmc2bus", "dma-ranges");
register_vc_fixup(root, "/chosen/bootloader");
chosen = register_vc_fixup(root, "/chosen");
if (!chosen) {
-- 
2.42.0




[PATCH 2/3] raspi: override properties in /reserved-memory node of device tree

2024-04-22 Thread Jonas Richardsen
Previously, the fixups in rpi-common.c only tries to copy the
`/reserved-memory` node from the video core. As explained in
`bcm2711-rpi.dtsi`, line 58, the video core does only update the
placement information of the `/reserved-memory/nvram@0` subnode:
> /*
>  * RPi4's co-processor will copy the board's bootloader configuration
>  * into memory for the OS to consume. It'll also update this node with
>  * its placement information.
>  */
This behaviour is not achieved by the previous fixup, as this fixup
operates on the wrong node and, even if applied to
"/reserved-memory/nvram@0" would not override the already existing
properties of this node. Instead, we can use the previously added
`register_vc_property_fixup` method to achieve the desired behaviour.

Signed-off-by: Jonas Richardsen 
---
 arch/arm/boards/raspberry-pi/rpi-common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boards/raspberry-pi/rpi-common.c 
b/arch/arm/boards/raspberry-pi/rpi-common.c
index 2245c36cbe..3c685852d3 100644
--- a/arch/arm/boards/raspberry-pi/rpi-common.c
+++ b/arch/arm/boards/raspberry-pi/rpi-common.c
@@ -372,7 +372,8 @@ static void rpi_vc_fdt_parse(struct device_node *root)
 
register_vc_fixup(root, "/system");
register_vc_fixup(root, "/axi");
-   register_vc_fixup(root, "/reserved-memory");
+   register_vc_property_fixup(root, "/reserved-memory/nvram@0", "reg");
+   register_vc_property_fixup(root, "/reserved-memory/nvram@0", "status");
register_vc_fixup(root, "/hat");
register_vc_fixup(root, "/chosen/bootloader");
chosen = register_vc_fixup(root, "/chosen");
-- 
2.42.0




[PATCH 1/3] raspi: add fixup method for specific properties

2024-04-22 Thread Jonas Richardsen
Add `rpi_vc_property_fixup` method to allow for fixups of specific
properties, i.e., copy a property of some node from the video core
device tree. Notably, this does override the property if it already
existed (as opposed to `rpi_vc_fixup`, which copies an entire node, but
does not override existing properties).

Signed-off-by: Jonas Richardsen 
---
 arch/arm/boards/raspberry-pi/rpi-common.c | 46 +++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/boards/raspberry-pi/rpi-common.c 
b/arch/arm/boards/raspberry-pi/rpi-common.c
index 7c82c740e2..2245c36cbe 100644
--- a/arch/arm/boards/raspberry-pi/rpi-common.c
+++ b/arch/arm/boards/raspberry-pi/rpi-common.c
@@ -60,6 +60,11 @@ struct rpi_priv {
const char *name;
 };
 
+struct rpi_property_fixup_data {
+const struct device_node* vc_node;
+const char *propname;
+};
+
 static void rpi_set_usbethaddr(void)
 {
u8 mac[ETH_ALEN];
@@ -301,6 +306,47 @@ static struct device_node *register_vc_fixup(struct 
device_node *root,
return ret;
 }
 
+static int rpi_vc_fdt_fixup_property(struct device_node *root, void *data)
+{
+   const struct rpi_property_fixup_data *fixup = data;
+   struct device_node *node;
+   struct property *prop;
+
+   node = of_create_node(root, fixup->vc_node->full_name);
+   if (!node)
+   return -ENOMEM;
+
+   prop = of_find_property(fixup->vc_node, fixup->propname, NULL);
+   if (!prop)
+   return -ENOENT;
+
+   return of_set_property(node, prop->name,
+  of_property_get_value(prop), 
prop->length, 1);
+}
+
+static int register_vc_property_fixup(struct device_node *root,
+const char *path, const char 
*propname)
+{
+   struct device_node *node, *tmp;
+   struct rpi_property_fixup_data* fixup_data;
+
+   node = of_find_node_by_path_from(root, path);
+   if (node) {
+   tmp = of_dup(node);
+   tmp->full_name = xstrdup(node->full_name);
+   fixup_data = xzalloc(sizeof(*fixup_data));
+   fixup_data->vc_node = tmp;
+   fixup_data->propname = xstrdup(propname);
+
+   of_register_fixup(rpi_vc_fdt_fixup_property, fixup_data);
+   } else {
+   pr_info("no '%s' node found in vc fdt\n", path);
+   return -ENOENT;
+   }
+
+   return 0;
+}
+
 static u32 rpi_boot_mode, rpi_boot_part;
 /* Extract useful information from the VideoCore FDT we got.
  * Some parameters are defined here:
-- 
2.42.0




[PATCH 0/3] raspi: cleanup of vc fixups

2024-04-22 Thread Jonas Richardsen
This patch series contains a few changes to the video core fixups where
the desired behaviour is obvious from raspberry device trees. These were
tested on a Raspberry Pi 4B. There's a few more differences between the
video core device tree and the one provided by barebox for which a
discussion is welcome:

- After a [recent change][link1] the `/chosen` node of the device tree
  is now fully copied from the video core device tree. This could
  possibly be restricted to only copy relevant properties.
- The properties `memreserve` and `serial-number` of the root node are
  added by the video core and could also be copied. 
- The property `model` of the root node is updated with the specific
  hardware revision of the pi.
- The video core adds the two aliases `i2c_arm` and `i2c_vc` (as
  properties to the `/aliases` node). As the [raspberrypi
  documentation][link2] suggests to use the former for writing overlays,
  it should maybe also be copied as a fixup.

[link1]: 
https://git.pengutronix.de/cgit/barebox/commit/?id=30754085bdba91853acbb3973ce348bf5aa2c9e7
[link2]: 
https://www.raspberrypi.com/documentation/computers/configuration.html#part3.3

Jonas Richardsen (3):
  raspi: add fixup method for specific properties
  raspi: override properties in /reserved-memory node of device tree
  raspi: add a fixup for the `dma-ranges` property of the `/emmc2bus` dt
node

 arch/arm/boards/raspberry-pi/rpi-common.c | 50 ++-
 1 file changed, 49 insertions(+), 1 deletion(-)

-- 
2.42.0




[PATCH v2] of: do not copy properties if they already exist in the destination

2024-04-15 Thread Jonas Richardsen
Currently `of_copy_property` copies the given property even if a property 
with the same name already exists on the destination node. 
This leads to kernel warnings about duplicate properties:
```
[0.014063] Duplicate name in chosen, renamed to "stdout-path#1"
[0.014093] Duplicate name in chosen, renamed to "bootargs#1"
[0.014119] Duplicate name in chosen, renamed to "phandle#1"
[0.014197] Duplicate name in reserved-memory, renamed to "#address-cells#1"
[0.014226] Duplicate name in reserved-memory, renamed to "#size-cells#1"
[0.014252] Duplicate name in reserved-memory, renamed to "ranges#1"
[0.014278] Duplicate name in reserved-memory, renamed to "phandle#1"
```
Therefore, the function was changed to return an error if the property
already exists in the destination.
The change does not cause any regressions, because the only usage of
this function occurs within `arch/arm/boards/raspberry-pi/rpi-common.c`
where the original behaviour of the function is obviously unintended.

Signed-off-by: Jonas Richardsen 
---
In the process of creating this patch, we realized that the fixups 
for the Raspberry Pi are extensively copying properties from the video
core device tree that are not strictly necessary. We will do some
work on making the fixups more precise (i.e., only copy specific
properties, not entire nodes) soon.

 drivers/of/base.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index b22959dabe..3192a74ab9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2346,6 +2346,9 @@ struct property *of_copy_property(const struct 
device_node *src,
if (!prop)
return NULL;
 
+   if (of_property_present(dst, propname))
+   return ERR_PTR(-EEXIST);
+
return of_new_property(dst, propname,
   of_property_get_value(prop), prop->length);
 }
-- 
2.42.0




[PATCH] of: do not copy properties if they already exist in the destination

2024-04-15 Thread Jonas Richardsen
---
 drivers/of/base.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index b22959dabe..9bb4ae13db 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2346,6 +2346,9 @@ struct property *of_copy_property(const struct 
device_node *src,
if (!prop)
return NULL;
 
+   if (of_find_property(dst, propname, NULL))
+   return ERR_PTR(-EEXIST);
+
return of_new_property(dst, propname,
   of_property_get_value(prop), prop->length);
 }
-- 
2.42.0




raspi: should vc fixups update properties of existing nodes?

2024-04-10 Thread Jonas Richardsen

Hi,

the function `rpi_vc_fdt_parse` in 
arch/arm/boards/raspberry-pi/rpi-common.c registers multiple fixups that 
take over nodes from the video core device tree. These fixups make use 
of the `of_copy_property` function to copy the properties of the 
respective node. In the case of already existing nodes (and properties), 
this function duplicates the properties instead of updating them.


If the intention is to not override existing properties, one should 
probably check for the existence of each property before copying to 
avoid kernel warnings and misconfiguration due to duplicate properties.


If existing properties are supposed to be updated, this could be 
achieved by switching to `of_set_property` (or something similar). Note 
that this notion of overriding properties also exists in video core, see 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/broadcom/bcm2711.dtsi?h=v6.8#n412 
for an example.


I would be glad to submit a patch for either case.

Regards,

Jonas