[PATCH] arm: dts: db410c/db820c: Fix SPMI addresses

[PATCH] arm: dts: db410c/db820c: Fix SPMI addresses

[Stephan Gerhold]

The Qualcomm device trees in U-Boot are currently not consistent with
the upstream DTs used in the Linux kernel. While some bindings are
similar to the official specification in the Linux kernel, several
nodes have subtle differences, e.g. the "compatible"s or the exact
specification of memory registers.

This means that some of the Qualcomm-related U-Boot drivers are not
compatible with the Linux DT (and vice versa).

The SPMI node is one such example: the "core" region starts at
0x0200f000 in the upstream Linux MSM8916 DT, but in U-Boot it starts at
0x0200f800. The end result is normally the same, since the Linux SPMI
driver simply adds the 0x800 internally.

However, commit f5a2d6b4b03a ("spmi: msm: add arbiter version 5
support") imported this behavior into the U-Boot driver, without
adjusting the DB410c/DB820c device trees. This means that the 0x800
offset is now added twice, breaking all SPMI read/write operations:

  Failed to find PMIC pon node. Check device tree
  Failed to find pm8916_gpios@c000 node.
  USB init failed: -6
  starting USB...
  Bus ehci@78d9000: Failed to find pm8916_gpios@c000 node.
  probe failed, error -6
  No working controllers found

While the mistake is strictly speaking in the spmi-msm driver, fix the
issue by making the SPMI nodes in the DB410c/DB820c consistent with the
upstream Linux DT instead.

Ideally we should even go a step further by fixing the remaining uses
of custom bindings in the U-Boot drivers and moving to using the Linux
DTs as-is. This would likely avoid such mistakes in the future and
would also make the porting process much easier.

Cc: Dzmitry Sankouski <dsankouski@gmail.com>
Fixes: f5a2d6b4b03a ("spmi: msm: add arbiter version 5 support")
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Sorry for sending this literally a few days after the U-Boot release.
This change has been sitting in my local U-Boot clone for a couple of
months already (it's broken at least since v2022.01) but I got caught
up in other work and completely forgot about it. I guess no one else
is actively testing U-Boot on DB410c/DB820c. :/
---
 arch/arm/dts/dragonboard410c.dts |  9 +++++++--
 arch/arm/dts/dragonboard820c.dts | 11 +++++++----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/arm/dts/dragonboard410c.dts b/arch/arm/dts/dragonboard410c.dts
index 7e56140df2..50523712cb 100644
--- a/arch/arm/dts/dragonboard410c.dts
+++ b/arch/arm/dts/dragonboard410c.dts
@@ -137,9 +137,14 @@
 			};
 		};
 
-		spmi@200f000 {
+		spmi_bus: spmi@200f000 {
 			compatible = "qcom,spmi-pmic-arb";
-			reg = <0x200f800 0x200 0x2400000 0x400000 0x2c00000 0x400000>;
+			reg = <0x0200f000 0x001000>,
+			      <0x02400000 0x400000>,
+			      <0x02c00000 0x400000>,
+			      <0x03800000 0x200000>,
+			      <0x0200a000 0x002100>;
+			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
 			#address-cells = <0x1>;
 			#size-cells = <0x1>;
 			pmic0: pm8916@0 {
diff --git a/arch/arm/dts/dragonboard820c.dts b/arch/arm/dts/dragonboard820c.dts
index 1114ddd7d3..b72a2471cf 100644
--- a/arch/arm/dts/dragonboard820c.dts
+++ b/arch/arm/dts/dragonboard820c.dts
@@ -93,11 +93,14 @@
 			clock-frequency = <200000000>;
 		 };
 
-		spmi@400f000 {
+		spmi_bus: spmi@400f000 {
 			compatible = "qcom,spmi-pmic-arb";
-			reg = <0x400f800 0x200>,
-			      <0x4400000 0x400000>,
-			      <0x4c00000 0x400000>;
+			reg = <0x0400f000 0x1000>,
+			      <0x04400000 0x800000>,
+			      <0x04c00000 0x800000>,
+			      <0x05800000 0x200000>,
+			      <0x0400a000 0x002100>;
+			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
 			#address-cells = <0x1>;
 			#size-cells = <0x1>;
 
-- 
2.37.0

Re: [PATCH] arm: dts: db410c/db820c: Fix SPMI addresses

[Sumit Garg]

Hi Stephan,

On Thu, 14 Jul 2022 at 00:47, Stephan Gerhold <stephan@gerhold.net> wrote:
>
> The Qualcomm device trees in U-Boot are currently not consistent with
> the upstream DTs used in the Linux kernel. While some bindings are
> similar to the official specification in the Linux kernel, several
> nodes have subtle differences, e.g. the "compatible"s or the exact
> specification of memory registers.
>
> This means that some of the Qualcomm-related U-Boot drivers are not
> compatible with the Linux DT (and vice versa).
>
> The SPMI node is one such example: the "core" region starts at
> 0x0200f000 in the upstream Linux MSM8916 DT, but in U-Boot it starts at
> 0x0200f800. The end result is normally the same, since the Linux SPMI
> driver simply adds the 0x800 internally.
>
> However, commit f5a2d6b4b03a ("spmi: msm: add arbiter version 5
> support") imported this behavior into the U-Boot driver, without
> adjusting the DB410c/DB820c device trees. This means that the 0x800
> offset is now added twice, breaking all SPMI read/write operations:
>
>   Failed to find PMIC pon node. Check device tree
>   Failed to find pm8916_gpios@c000 node.
>   USB init failed: -6
>   starting USB...
>   Bus ehci@78d9000: Failed to find pm8916_gpios@c000 node.
>   probe failed, error -6
>   No working controllers found
>
> While the mistake is strictly speaking in the spmi-msm driver, fix the
> issue by making the SPMI nodes in the DB410c/DB820c consistent with the
> upstream Linux DT instead.
>
> Ideally we should even go a step further by fixing the remaining uses
> of custom bindings in the U-Boot drivers and moving to using the Linux
> DTs as-is. This would likely avoid such mistakes in the future and
> would also make the porting process much easier.

+1, another patch [1] in this direction.

[1] https://patchwork.ozlabs.org/project/uboot/patch/20220714073337.2298978-1-sumit.garg@linaro.org/

>
> Cc: Dzmitry Sankouski <dsankouski@gmail.com>
> Fixes: f5a2d6b4b03a ("spmi: msm: add arbiter version 5 support")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> Sorry for sending this literally a few days after the U-Boot release.
> This change has been sitting in my local U-Boot clone for a couple of
> months already (it's broken at least since v2022.01) but I got caught
> up in other work and completely forgot about it. I guess no one else
> is actively testing U-Boot on DB410c/DB820c. :/
> ---
>  arch/arm/dts/dragonboard410c.dts |  9 +++++++--
>  arch/arm/dts/dragonboard820c.dts | 11 +++++++----
>  2 files changed, 14 insertions(+), 6 deletions(-)
>

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

> diff --git a/arch/arm/dts/dragonboard410c.dts b/arch/arm/dts/dragonboard410c.dts
> index 7e56140df2..50523712cb 100644
> --- a/arch/arm/dts/dragonboard410c.dts
> +++ b/arch/arm/dts/dragonboard410c.dts
> @@ -137,9 +137,14 @@
>                         };
>                 };
>
> -               spmi@200f000 {
> +               spmi_bus: spmi@200f000 {
>                         compatible = "qcom,spmi-pmic-arb";
> -                       reg = <0x200f800 0x200 0x2400000 0x400000 0x2c00000 0x400000>;
> +                       reg = <0x0200f000 0x001000>,
> +                             <0x02400000 0x400000>,
> +                             <0x02c00000 0x400000>,
> +                             <0x03800000 0x200000>,
> +                             <0x0200a000 0x002100>;
> +                       reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
>                         #address-cells = <0x1>;
>                         #size-cells = <0x1>;
>                         pmic0: pm8916@0 {
> diff --git a/arch/arm/dts/dragonboard820c.dts b/arch/arm/dts/dragonboard820c.dts
> index 1114ddd7d3..b72a2471cf 100644
> --- a/arch/arm/dts/dragonboard820c.dts
> +++ b/arch/arm/dts/dragonboard820c.dts
> @@ -93,11 +93,14 @@
>                         clock-frequency = <200000000>;
>                  };
>
> -               spmi@400f000 {
> +               spmi_bus: spmi@400f000 {
>                         compatible = "qcom,spmi-pmic-arb";
> -                       reg = <0x400f800 0x200>,
> -                             <0x4400000 0x400000>,
> -                             <0x4c00000 0x400000>;
> +                       reg = <0x0400f000 0x1000>,
> +                             <0x04400000 0x800000>,
> +                             <0x04c00000 0x800000>,
> +                             <0x05800000 0x200000>,
> +                             <0x0400a000 0x002100>;
> +                       reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
>                         #address-cells = <0x1>;
>                         #size-cells = <0x1>;
>
> --
> 2.37.0
>

Re: [PATCH] arm: dts: db410c/db820c: Fix SPMI addresses

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]

On Wed, Jul 13, 2022 at 09:17:11PM +0200, Stephan Gerhold wrote:

> The Qualcomm device trees in U-Boot are currently not consistent with
> the upstream DTs used in the Linux kernel. While some bindings are
> similar to the official specification in the Linux kernel, several
> nodes have subtle differences, e.g. the "compatible"s or the exact
> specification of memory registers.
> 
> This means that some of the Qualcomm-related U-Boot drivers are not
> compatible with the Linux DT (and vice versa).
> 
> The SPMI node is one such example: the "core" region starts at
> 0x0200f000 in the upstream Linux MSM8916 DT, but in U-Boot it starts at
> 0x0200f800. The end result is normally the same, since the Linux SPMI
> driver simply adds the 0x800 internally.
> 
> However, commit f5a2d6b4b03a ("spmi: msm: add arbiter version 5
> support") imported this behavior into the U-Boot driver, without
> adjusting the DB410c/DB820c device trees. This means that the 0x800
> offset is now added twice, breaking all SPMI read/write operations:
> 
>   Failed to find PMIC pon node. Check device tree
>   Failed to find pm8916_gpios@c000 node.
>   USB init failed: -6
>   starting USB...
>   Bus ehci@78d9000: Failed to find pm8916_gpios@c000 node.
>   probe failed, error -6
>   No working controllers found
> 
> While the mistake is strictly speaking in the spmi-msm driver, fix the
> issue by making the SPMI nodes in the DB410c/DB820c consistent with the
> upstream Linux DT instead.
> 
> Ideally we should even go a step further by fixing the remaining uses
> of custom bindings in the U-Boot drivers and moving to using the Linux
> DTs as-is. This would likely avoid such mistakes in the future and
> would also make the porting process much easier.
> 
> Cc: Dzmitry Sankouski <dsankouski@gmail.com>
> Fixes: f5a2d6b4b03a ("spmi: msm: add arbiter version 5 support")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]