[PATCH 1/2] of: base: add new barebox,status property

[PATCH 1/2] of: base: add new barebox,status property

[Ahmad Fatoum]

I am confronted with an occasional hang on the Radxa Rock 3A during
PCI probe. The board has no PCI devices connected.

This hang doesn't happen in Linux and doesn't happen on the QNAP
TS433-eU, which is also RK3568, but actually has PCIe devices, which are
probed normally.

For such purposes, add a barebox,status property as setting the status
to disabled, would impact Linux as well if barebox were to pass along
its own device tree, e.g. when installing the UEFI device tree
configuration table.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - rework to make clearer when to use the new property. (Marco)
  - add example with UEFI device tree configuration table into commit
    message (Marco)
---
 .../devicetree/bindings/barebox/barebox,status.rst | 14 ++++++++++++++
 drivers/of/base.c                                  |  4 +++-
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/barebox/barebox,status.rst

diff --git a/Documentation/devicetree/bindings/barebox/barebox,status.rst b/Documentation/devicetree/bindings/barebox/barebox,status.rst
new file mode 100644
index 000000000000..03f484cdcfe1
--- /dev/null
+++ b/Documentation/devicetree/bindings/barebox/barebox,status.rst
@@ -0,0 +1,14 @@
+barebox,status property
+=======================
+
+barebox interprets ``barebox,status`` the same as it does ``status``,
+but gives the barebox-specific property precedence if both exist.
+
+The purpose of this property is to keep the ``status`` property of the
+upstream DT, imported from Linux, untouched.
+
+Using ``barebox,status`` may be necessary to temporarily workaround
+barebox drivers that misbehave on a given board; Disabling the driver
+may be undesirable if it can handle other instances of the same device
+on the board or if barebox is being built to support other boards
+at the same time, where the driver functions correctly.
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 54fd458bd9a1..4e83a757a4e9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2149,7 +2149,9 @@ int of_device_is_available(const struct device_node *device)
 	const char *status;
 	int statlen;
 
-	status = of_get_property(device, "status", &statlen);
+	status = of_get_property(device, "barebox,status", &statlen);
+	if (status == NULL)
+		status = of_get_property(device, "status", &statlen);
 	if (status == NULL)
 		return 1;
 
-- 
2.47.3

[PATCH 2/2] arm: dts: rk356x: rock3a: disable pcie3x2

[Ahmad Fatoum]

The board hangs occasionally on PCI probe after:

  phy7: lane number 0, val 1

On other boots, it continues to:

  rockchip-dw-pcie 3c0800000.pcie@fe280000.of: Phy link never came up

In both cases, the hang affects the second PCIe host controller
to probe and the first probe never hangs:

  rockchip-dw-pcie 3c0000000.pcie@fe260000.of: Phy link never came up

This hang happens on the very first read access to the PCI controller
at register PCIE_ATU_VIEWPORT. Reading Linux code, the first access
seems to be to PCIE_VERSION_NUMBER (0x8F8), but accessing that in
barebox equally hangs from time to time.

My board doesn't have any PCIe devices connected and this hang
doesn't happen in Linux and doesn't happen on the QNAP
TS433-eU, which is also RK3568, but actually has PCIe devices, which are
probed normally.

Disable the device in barebox, so the board is usable with
rockchip_v8_defconfig, which enables the PCI driver.
barebox,status is used, so the Linux device tree is not affected.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - add short comment explaining why we disable this device in particular
---
 arch/arm/dts/rk3568-rock-3a.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/dts/rk3568-rock-3a.dts b/arch/arm/dts/rk3568-rock-3a.dts
index bcbfab14a580..4834a85b367a 100644
--- a/arch/arm/dts/rk3568-rock-3a.dts
+++ b/arch/arm/dts/rk3568-rock-3a.dts
@@ -55,3 +55,8 @@ environment_sd: partition@408000 {
 		};
 	};
 };
+
+/* Device driver probe occasionally hangs, but pcie2x1 is fine... */
+&pcie3x2 {
+	barebox,status = "disabled";
+};
-- 
2.47.3

Re: [PATCH 1/2] of: base: add new barebox,status property

[Ahmad Fatoum]

Hi,

sorry missed adding the v2..

On 1/21/26 12:31 PM, Ahmad Fatoum wrote:
> I am confronted with an occasional hang on the Radxa Rock 3A during
> PCI probe. The board has no PCI devices connected.
> 
> This hang doesn't happen in Linux and doesn't happen on the QNAP
> TS433-eU, which is also RK3568, but actually has PCIe devices, which are
> probed normally.
> 
> For such purposes, add a barebox,status property as setting the status
> to disabled, would impact Linux as well if barebox were to pass along
> its own device tree, e.g. when installing the UEFI device tree
> configuration table.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2:
>   - rework to make clearer when to use the new property. (Marco)
>   - add example with UEFI device tree configuration table into commit
>     message (Marco)
> ---
>  .../devicetree/bindings/barebox/barebox,status.rst | 14 ++++++++++++++
>  drivers/of/base.c                                  |  4 +++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/barebox/barebox,status.rst
> 
> diff --git a/Documentation/devicetree/bindings/barebox/barebox,status.rst b/Documentation/devicetree/bindings/barebox/barebox,status.rst
> new file mode 100644
> index 000000000000..03f484cdcfe1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/barebox/barebox,status.rst
> @@ -0,0 +1,14 @@
> +barebox,status property
> +=======================
> +
> +barebox interprets ``barebox,status`` the same as it does ``status``,
> +but gives the barebox-specific property precedence if both exist.
> +
> +The purpose of this property is to keep the ``status`` property of the
> +upstream DT, imported from Linux, untouched.
> +
> +Using ``barebox,status`` may be necessary to temporarily workaround
> +barebox drivers that misbehave on a given board; Disabling the driver
> +may be undesirable if it can handle other instances of the same device
> +on the board or if barebox is being built to support other boards
> +at the same time, where the driver functions correctly.
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 54fd458bd9a1..4e83a757a4e9 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2149,7 +2149,9 @@ int of_device_is_available(const struct device_node *device)
>  	const char *status;
>  	int statlen;
>  
> -	status = of_get_property(device, "status", &statlen);
> +	status = of_get_property(device, "barebox,status", &statlen);
> +	if (status == NULL)
> +		status = of_get_property(device, "status", &statlen);
>  	if (status == NULL)
>  		return 1;
>  

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

Re: [PATCH 1/2] of: base: add new barebox,status property

[Marco Felsch]

On 26-01-21, Ahmad Fatoum wrote:
> I am confronted with an occasional hang on the Radxa Rock 3A during
> PCI probe. The board has no PCI devices connected.
> 
> This hang doesn't happen in Linux and doesn't happen on the QNAP
> TS433-eU, which is also RK3568, but actually has PCIe devices, which are
> probed normally.
> 
> For such purposes, add a barebox,status property as setting the status
> to disabled, would impact Linux as well if barebox were to pass along
> its own device tree, e.g. when installing the UEFI device tree
> configuration table.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
> v1 -> v2:
>   - rework to make clearer when to use the new property. (Marco)
>   - add example with UEFI device tree configuration table into commit
>     message (Marco)
> ---
>  .../devicetree/bindings/barebox/barebox,status.rst | 14 ++++++++++++++
>  drivers/of/base.c                                  |  4 +++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/barebox/barebox,status.rst
> 
> diff --git a/Documentation/devicetree/bindings/barebox/barebox,status.rst b/Documentation/devicetree/bindings/barebox/barebox,status.rst
> new file mode 100644
> index 000000000000..03f484cdcfe1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/barebox/barebox,status.rst
> @@ -0,0 +1,14 @@
> +barebox,status property
> +=======================
> +
> +barebox interprets ``barebox,status`` the same as it does ``status``,
> +but gives the barebox-specific property precedence if both exist.
> +
> +The purpose of this property is to keep the ``status`` property of the
> +upstream DT, imported from Linux, untouched.
> +
> +Using ``barebox,status`` may be necessary to temporarily workaround
> +barebox drivers that misbehave on a given board; Disabling the driver
> +may be undesirable if it can handle other instances of the same device
> +on the board or if barebox is being built to support other boards
> +at the same time, where the driver functions correctly.
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 54fd458bd9a1..4e83a757a4e9 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2149,7 +2149,9 @@ int of_device_is_available(const struct device_node *device)
>  	const char *status;
>  	int statlen;
>  
> -	status = of_get_property(device, "status", &statlen);
> +	status = of_get_property(device, "barebox,status", &statlen);
> +	if (status == NULL)
> +		status = of_get_property(device, "status", &statlen);
>  	if (status == NULL)
>  		return 1;
>  
> -- 
> 2.47.3
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

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

Re: [PATCH 2/2] arm: dts: rk356x: rock3a: disable pcie3x2

[Marco Felsch]

On 26-01-21, Ahmad Fatoum wrote:
> The board hangs occasionally on PCI probe after:
> 
>   phy7: lane number 0, val 1
> 
> On other boots, it continues to:
> 
>   rockchip-dw-pcie 3c0800000.pcie@fe280000.of: Phy link never came up
> 
> In both cases, the hang affects the second PCIe host controller
> to probe and the first probe never hangs:
> 
>   rockchip-dw-pcie 3c0000000.pcie@fe260000.of: Phy link never came up
> 
> This hang happens on the very first read access to the PCI controller
> at register PCIE_ATU_VIEWPORT. Reading Linux code, the first access
> seems to be to PCIE_VERSION_NUMBER (0x8F8), but accessing that in
> barebox equally hangs from time to time.
> 
> My board doesn't have any PCIe devices connected and this hang
> doesn't happen in Linux and doesn't happen on the QNAP
> TS433-eU, which is also RK3568, but actually has PCIe devices, which are
> probed normally.
> 
> Disable the device in barebox, so the board is usable with
> rockchip_v8_defconfig, which enables the PCI driver.
> barebox,status is used, so the Linux device tree is not affected.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

Re: [PATCH 1/2] of: base: add new barebox,status property

[Marco Felsch]

On 26-01-20, Ahmad Fatoum wrote:
> I am confronted with an occasional hang on the Radxa Rock 3A during
> PCI probe. The board has no PCI devices connected.
> 
> This hang doesn't happen in Linux and doesn't happen on the QNAP
> TS433-eU, which is also RK3568, but actually has PCIe devices, which are
> probed normally.
> 
> For such purposes, add a barebox,status property as setting the status
> to disabled, would impact Linux as well if barebox were to pass along
> its own device tree.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  .../devicetree/bindings/barebox/barebox,status.rst        | 8 ++++++++
>  drivers/of/base.c                                         | 4 +++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/barebox/barebox,status.rst
> 
> diff --git a/Documentation/devicetree/bindings/barebox/barebox,status.rst b/Documentation/devicetree/bindings/barebox/barebox,status.rst
> new file mode 100644
> index 000000000000..06524b218fb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/barebox/barebox,status.rst
> @@ -0,0 +1,8 @@
> +barebox,status property
> +=======================
> +
> +barebox interprets ``barebox,status`` the same as it does ``status``,
> +but gives the former precedence if both exist.
> +
> +Use this property only for devices for which barebox has a driver, but
> +that barebox should not be probing on a per-board basis.

I would rephrase the 2nd sentence a bit:

"""
The purpose of this property is to keep the status property of the
upstream DT, provided by Linux, untouched.

This may be necessary to temporary workaround broken barebox drivers on
a per-board basis in case you can't disable the driver due to a
multi-config build.
"""

Regards,
  Marco


> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 54fd458bd9a1..4e83a757a4e9 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2149,7 +2149,9 @@ int of_device_is_available(const struct device_node *device)
>  	const char *status;
>  	int statlen;
>  
> -	status = of_get_property(device, "status", &statlen);
> +	status = of_get_property(device, "barebox,status", &statlen);
> +	if (status == NULL)
> +		status = of_get_property(device, "status", &statlen);
>  	if (status == NULL)
>  		return 1;
>  
> -- 
> 2.47.3
> 
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

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

[PATCH 1/2] of: base: add new barebox,status property

[Ahmad Fatoum]

I am confronted with an occasional hang on the Radxa Rock 3A during
PCI probe. The board has no PCI devices connected.

This hang doesn't happen in Linux and doesn't happen on the QNAP
TS433-eU, which is also RK3568, but actually has PCIe devices, which are
probed normally.

For such purposes, add a barebox,status property as setting the status
to disabled, would impact Linux as well if barebox were to pass along
its own device tree.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .../devicetree/bindings/barebox/barebox,status.rst        | 8 ++++++++
 drivers/of/base.c                                         | 4 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/barebox/barebox,status.rst

diff --git a/Documentation/devicetree/bindings/barebox/barebox,status.rst b/Documentation/devicetree/bindings/barebox/barebox,status.rst
new file mode 100644
index 000000000000..06524b218fb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/barebox/barebox,status.rst
@@ -0,0 +1,8 @@
+barebox,status property
+=======================
+
+barebox interprets ``barebox,status`` the same as it does ``status``,
+but gives the former precedence if both exist.
+
+Use this property only for devices for which barebox has a driver, but
+that barebox should not be probing on a per-board basis.
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 54fd458bd9a1..4e83a757a4e9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2149,7 +2149,9 @@ int of_device_is_available(const struct device_node *device)
 	const char *status;
 	int statlen;
 
-	status = of_get_property(device, "status", &statlen);
+	status = of_get_property(device, "barebox,status", &statlen);
+	if (status == NULL)
+		status = of_get_property(device, "status", &statlen);
 	if (status == NULL)
 		return 1;
 
-- 
2.47.3