[PATCH 0/3] arm64: dts: ti: k3-j7200: Fixes for various dtbs_checks warnings

[PATCH 0/3] arm64: dts: ti: k3-j7200: Fixes for various dtbs_checks warnings

[Nishanth Menon]

Hi,

Few fixups for j7200 dtbs_check warnings.

Bootlog: https://gist.github.com/nmenon/6a37fca2f05633b7153e661d2516deab

NOTE: lets see the discussion summary of [1] to see where to take this
series, but, I will put it out here in the list for discussion anyways.

[1] https://lore.kernel.org/all/76da0b98-3274-b047-db11-ecabc117ae11@ti.com/

Nishanth Menon (3):
  arm64: dts: ti: k3-j7200-mcu-wakeup: Remove 0x unit address prefix
    from nodename
  arm64: dts: ti: k3-j7200-mcu-wakeup: Switch mcu_syscon to
    ti,j721e-system-controller
  arm64: dts: ti: k3-j7200-mcu-wakeup: Split fss node up

 .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 29 ++++++++++++-------
 1 file changed, 19 insertions(+), 10 deletions(-)

-- 
2.40.0

[PATCH 1/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Remove 0x unit address prefix from nodename

[Nishanth Menon]

unit-address should not use a 0x prefix.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index 331b4e482e41..674e695ef844 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -62,7 +62,7 @@ wkup_pmx0: pinctrl@4301c000 {
 		pinctrl-single,function-mask = <0xffffffff>;
 	};
 
-	wkup_pmx1: pinctrl@0x4301c038 {
+	wkup_pmx1: pinctrl@4301c038 {
 		compatible = "pinctrl-single";
 		/* Proxy 0 addressing */
 		reg = <0x00 0x4301c038 0x00 0x8>;
@@ -71,7 +71,7 @@ wkup_pmx1: pinctrl@0x4301c038 {
 		pinctrl-single,function-mask = <0xffffffff>;
 	};
 
-	wkup_pmx2: pinctrl@0x4301c068 {
+	wkup_pmx2: pinctrl@4301c068 {
 		compatible = "pinctrl-single";
 		/* Proxy 0 addressing */
 		reg = <0x00 0x4301c068 0x00 0xec>;
@@ -80,7 +80,7 @@ wkup_pmx2: pinctrl@0x4301c068 {
 		pinctrl-single,function-mask = <0xffffffff>;
 	};
 
-	wkup_pmx3: pinctrl@0x4301c174 {
+	wkup_pmx3: pinctrl@4301c174 {
 		compatible = "pinctrl-single";
 		/* Proxy 0 addressing */
 		reg = <0x00 0x4301c174 0x00 0x20>;
-- 
2.40.0

[PATCH 2/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Switch mcu_syscon to ti,j721e-system-controller

[Nishanth Menon]

Use ti,j721e-system-controller to be explicit about the syscon node we
are using.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index 674e695ef844..b58a31371bf3 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -35,7 +35,7 @@ k3_reset: reset-controller {
 	};
 
 	mcu_conf: syscon@40f00000 {
-		compatible = "syscon", "simple-mfd";
+		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
 		reg = <0x00 0x40f00000 0x00 0x20000>;
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
2.40.0

[PATCH 3/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Split fss node up

[Nishanth Menon]

fss node claims to be entirely a syscon node, but it is really two
parts of it - one a syscon that controls the hbmc mux and a simple bus
where ospi, hbmc peripherals are located. So model it accordingly by
splitting the node up and using ti,j721e-system-controller to describe
the syscon

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 21 +++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index b58a31371bf3..7653cb191be1 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -338,18 +338,27 @@ mcu_spi2: spi@40320000 {
 		status = "disabled";
 	};
 
-	fss: syscon@47000000 {
-		compatible = "syscon", "simple-mfd";
+	hbmc_syscon: syscon@47000000 {
+		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
 		reg = <0x00 0x47000000 0x00 0x100>;
-		#address-cells = <2>;
-		#size-cells = <2>;
-		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x00 0x00 0x47000000 0x100>;
 
-		hbmc_mux: hbmc-mux {
+		hbmc_mux: mux-controller@4 {
 			compatible = "mmio-mux";
+			reg = <0x4 0x2>;
 			#mux-control-cells = <1>;
 			mux-reg-masks = <0x4 0x2>; /* HBMC select */
 		};
+	};
+
+	fss: bus@47030000 {
+		compatible = "simple-bus";
+		reg = <0x0 0x47030000 0x0 0x100>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
 
 		hbmc: hyperbus@47034000 {
 			compatible = "ti,am654-hbmc";
-- 
2.40.0

Re: [PATCH 3/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Split fss node up

[Kumar, Udit]

Hi Nishanth,

On 4/24/2023 11:06 PM, Nishanth Menon wrote:
> fss node claims to be entirely a syscon node, but it is really two
> parts of it - one a syscon that controls the hbmc mux and a simple bus
> where ospi, hbmc peripherals are located. So model it accordingly by
> splitting the node up and using ti,j721e-system-controller to describe
> the syscon
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>   .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 21 +++++++++++++------
>   1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> index b58a31371bf3..7653cb191be1 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> @@ -338,18 +338,27 @@ mcu_spi2: spi@40320000 {
>   		status = "disabled";
>   	};
>   
> -	fss: syscon@47000000 {
> -		compatible = "syscon", "simple-mfd";
> +	hbmc_syscon: syscon@47000000 {
> +		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
>   		reg = <0x00 0x47000000 0x00 0x100>;

Description is given upto 0x78 register in TRM (Section 12.3.1.6 FSS 
Registers)

Should we limit length to 0x78 ?

> -		#address-cells = <2>;
> -		#size-cells = <2>;
> -		ranges;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0x00 0x00 0x47000000 0x100>;
>   
> -		hbmc_mux: hbmc-mux {
> +		hbmc_mux: mux-controller@4 {
>   			compatible = "mmio-mux";
> +			reg = <0x4 0x2>;
>   			#mux-control-cells = <1>;
>   			mux-reg-masks = <0x4 0x2>; /* HBMC select */
>   		};
> +	};
> +
> +	fss: bus@47030000 {
> +		compatible = "simple-bus";
> +		reg = <0x0 0x47030000 0x0 0x100>;

Only registers upto address  0x47030008 has valid description in TRM

Section, 13.3.3.6.3 HyperBus Subsystem Registers

Please see, if we need to limit length to 8 instead of 256

> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
>   
>   		hbmc: hyperbus@47034000 {
>   			compatible = "ti,am654-hbmc";

Re: [PATCH 3/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Split fss node up

[Andrew Davis]

On 4/24/23 12:36 PM, Nishanth Menon wrote:
> fss node claims to be entirely a syscon node, but it is really two
> parts of it - one a syscon that controls the hbmc mux and a simple bus
> where ospi, hbmc peripherals are located. So model it accordingly by
> splitting the node up and using ti,j721e-system-controller to describe
> the syscon
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>   .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 21 +++++++++++++------
>   1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> index b58a31371bf3..7653cb191be1 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> @@ -338,18 +338,27 @@ mcu_spi2: spi@40320000 {
>   		status = "disabled";
>   	};
>   
> -	fss: syscon@47000000 {
> -		compatible = "syscon", "simple-mfd";
> +	hbmc_syscon: syscon@47000000 {
> +		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
>   		reg = <0x00 0x47000000 0x00 0x100>;
> -		#address-cells = <2>;
> -		#size-cells = <2>;
> -		ranges;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0x00 0x00 0x47000000 0x100>;
>   
> -		hbmc_mux: hbmc-mux {
> +		hbmc_mux: mux-controller@4 {
>   			compatible = "mmio-mux";
> +			reg = <0x4 0x2>;
>   			#mux-control-cells = <1>;
>   			mux-reg-masks = <0x4 0x2>; /* HBMC select */
>   		};
> +	};
> +
> +	fss: bus@47030000 {
> +		compatible = "simple-bus";
> +		reg = <0x0 0x47030000 0x0 0x100>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
>   
>   		hbmc: hyperbus@47034000 {
>   			compatible = "ti,am654-hbmc";


I hope all the things you had to do here show you what I mean in my
comments on [0] :)

I've posted a first step patch that allows "reg-mux" node to work with
regular reg properties[1]. Which means this patch could have been just this:

--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -305,15 +305,16 @@ wkup_i2c0: i2c@42120000 {
                 status = "disabled";
         };
  
-       fss: syscon@47000000 {
-               compatible = "syscon", "simple-mfd";
+       fss: bus@47000000 {
+               compatible = "simple-bus";
                 reg = <0x00 0x47000000 0x00 0x100>;
                 #address-cells = <2>;
                 #size-cells = <2>;
                 ranges;
  
                 hbmc_mux: hbmc-mux {
-                       compatible = "mmio-mux";
+                       compatible = "reg-mux";
+                       reg = <0x00 0x47000004 0x00 0x2>;
                         #mux-control-cells = <1>;
                         mux-reg-masks = <0x4 0x2>; /* HBMC select */
                 };

Andrew

[0] https://lore.kernel.org/all/76da0b98-3274-b047-db11-ecabc117ae11@ti.com/
[1] https://lore.kernel.org/all/20230424184810.29453-1-afd@ti.com/

Re: [PATCH 3/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Split fss node up

[Nishanth Menon]

On 13:55-20230424, Andrew Davis wrote:
> On 4/24/23 12:36 PM, Nishanth Menon wrote:
> > fss node claims to be entirely a syscon node, but it is really two
> > parts of it - one a syscon that controls the hbmc mux and a simple bus
> > where ospi, hbmc peripherals are located. So model it accordingly by
> > splitting the node up and using ti,j721e-system-controller to describe
> > the syscon
> > 
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> >   .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 21 +++++++++++++------
> >   1 file changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> > index b58a31371bf3..7653cb191be1 100644
> > --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> > @@ -338,18 +338,27 @@ mcu_spi2: spi@40320000 {
> >   		status = "disabled";
> >   	};
> > -	fss: syscon@47000000 {
> > -		compatible = "syscon", "simple-mfd";
> > +	hbmc_syscon: syscon@47000000 {
> > +		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
> >   		reg = <0x00 0x47000000 0x00 0x100>;
> > -		#address-cells = <2>;
> > -		#size-cells = <2>;
> > -		ranges;
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		ranges = <0x00 0x00 0x47000000 0x100>;
> > -		hbmc_mux: hbmc-mux {
> > +		hbmc_mux: mux-controller@4 {
> >   			compatible = "mmio-mux";
> > +			reg = <0x4 0x2>;
> >   			#mux-control-cells = <1>;
> >   			mux-reg-masks = <0x4 0x2>; /* HBMC select */
> >   		};
> > +	};
> > +
> > +	fss: bus@47030000 {
> > +		compatible = "simple-bus";
> > +		reg = <0x0 0x47030000 0x0 0x100>;
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> >   		hbmc: hyperbus@47034000 {
> >   			compatible = "ti,am654-hbmc";
> 
> 
> I hope all the things you had to do here show you what I mean in my
> comments on [0] :)

yup.

> 
> I've posted a first step patch that allows "reg-mux" node to work with
> regular reg properties[1]. Which means this patch could have been just this:
> 
> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> @@ -305,15 +305,16 @@ wkup_i2c0: i2c@42120000 {
>                 status = "disabled";
>         };
> -       fss: syscon@47000000 {
> -               compatible = "syscon", "simple-mfd";
> +       fss: bus@47000000 {
> +               compatible = "simple-bus";
>                 reg = <0x00 0x47000000 0x00 0x100>;
>                 #address-cells = <2>;
>                 #size-cells = <2>;
>                 ranges;
>                 hbmc_mux: hbmc-mux {
> -                       compatible = "mmio-mux";
> +                       compatible = "reg-mux";
> +                       reg = <0x00 0x47000004 0x00 0x2>;
>                         #mux-control-cells = <1>;
>                         mux-reg-masks = <0x4 0x2>; /* HBMC select */
>                 };
> 
> Andrew
> 
> [0] https://lore.kernel.org/all/76da0b98-3274-b047-db11-ecabc117ae11@ti.com/
> [1] https://lore.kernel.org/all/20230424184810.29453-1-afd@ti.com/

yes, this is a better approach. Lets see the conclusion of the
discussion.


-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 0/3] arm64: dts: ti: k3-j7200: Fixes for various dtbs_checks warnings

[Nishanth Menon]

On 12:36-20230424, Nishanth Menon wrote:
> Hi,
> 
> Few fixups for j7200 dtbs_check warnings.
> 
> Bootlog: https://gist.github.com/nmenon/6a37fca2f05633b7153e661d2516deab
> 
> NOTE: lets see the discussion summary of [1] to see where to take this
> series, but, I will put it out here in the list for discussion anyways.
> 
> [1] https://lore.kernel.org/all/76da0b98-3274-b047-db11-ecabc117ae11@ti.com/
> 
> Nishanth Menon (3):
>   arm64: dts: ti: k3-j7200-mcu-wakeup: Remove 0x unit address prefix
>     from nodename
>   arm64: dts: ti: k3-j7200-mcu-wakeup: Switch mcu_syscon to
>     ti,j721e-system-controller
>   arm64: dts: ti: k3-j7200-mcu-wakeup: Split fss node up
> 
>  .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 29 ++++++++++++-------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 

After looking a bit closer at this, lets drop this series.
For the pinctrl fixup: Please pick up
https://lore.kernel.org/all/20230510091850.28881-1-tony@atomide.com/
instead

As discussed in
https://lore.kernel.org/all/20230605205220.rjmcsi5tjn4auqa7@arose/ we
can look at fss and syscon cleanup in the next kernel rev once the
dependencies are sorted out.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: (subset) [PATCH 0/3] arm64: dts: ti: k3-j7200: Fixes for various dtbs_checks warnings

[Vignesh Raghavendra]

Hi Nishanth Menon,

On Mon, 24 Apr 2023 12:36:20 -0500, Nishanth Menon wrote:
> Few fixups for j7200 dtbs_check warnings.
> 
> Bootlog: https://gist.github.com/nmenon/6a37fca2f05633b7153e661d2516deab
> 
> NOTE: lets see the discussion summary of [1] to see where to take this
> series, but, I will put it out here in the list for discussion anyways.
> 
> [...]

I have applied the following to branch ti-k3-dts-next on [1].
Thank you!

[1/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Remove 0x unit address prefix from nodename
      commit: 4c3cdac1955a5274a42d915e98827ba0f136c286

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
--
Vignesh