2023-05-05 12:05:59

by Vaishnav Achath

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

Hi,

Few fixups for j7200 dtbs_check warnings.

This is V2 for the following series with the feedback addressed,

V1 : https://lore.kernel.org/all/[email protected]/

Bootlog with basic hyperflash testing:
https://gist.github.com/vaishnavachath/f7265e932725fd992dbc4e48b993e9c0

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

Patch 3/3 depends on the following patch under review which enables reg-mux
to be used when parent node is not syscon :
https://lore.kernel.org/all/[email protected]/

Changelog:

V1->V2:
* Address feedback as recommended in [2].
* Address feedback from Udit to limit the FSS register region size as
per TRM.
* Use reg-mux changes in [3] to simplify the hbmc-mux modelling

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/20230503115130.c7m4a7crub7kmfjw@gluten/
[3] https://lore.kernel.org/all/[email protected]/

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: Update fss node and hbmc_mux

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

--
2.17.1


2023-05-05 12:08:40

by Vaishnav Achath

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

From: Nishanth Menon <[email protected]>

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

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Vaishnav Achath <[email protected]>
---

V1->V2 : No changes.

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 @@
};

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

2023-05-05 12:09:10

by Vaishnav Achath

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

From: Nishanth Menon <[email protected]>

unit-address should not use a 0x prefix.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Vaishnav Achath <[email protected]>
---

V1->V2 : No changes.

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 @@
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 @@
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 @@
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.17.1

2023-05-05 12:10:50

by Vaishnav Achath

[permalink] [raw]
Subject: [PATCH v2 3/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Update fss node and hbmc_mux

From: Nishanth Menon <[email protected]>

fss node claims to be a syscon node, while it actually is a simple bus
where ospi, hbmc peripherals are located and a mux for path select
between OSPI and Hyperbus which can be modelled as a reg-mux. So model
it accordingly and use reg-mux to describe the hbmc-mux. Also update
the region size to the correct values as per the TRM.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Vaishnav Achath <[email protected]>
---

V1->V2:
* Address feedback from Udit to limit the FSS register region size as
per TRM.
* Use reg-mux changes to simplify the hbmc-mux modelling.
* Update commit message to reflect changes.

Depends on:
https://lore.kernel.org/all/[email protected]/

arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 13 +++++++------
1 file changed, 7 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..333564ca9c91 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -338,22 +338,23 @@
status = "disabled";
};

- fss: syscon@47000000 {
- compatible = "syscon", "simple-mfd";
- reg = <0x00 0x47000000 0x00 0x100>;
+ fss: bus@47000000 {
+ compatible = "simple-bus";
+ reg = <0x00 0x47000000 0x0 0x7c>;
#address-cells = <2>;
#size-cells = <2>;
ranges;

- hbmc_mux: hbmc-mux {
- compatible = "mmio-mux";
+ hbmc_mux: mux-controller@47000004 {
+ compatible = "reg-mux";
+ reg = <0x00 0x47000004 0x00 0x2>;
#mux-control-cells = <1>;
mux-reg-masks = <0x4 0x2>; /* HBMC select */
};

hbmc: hyperbus@47034000 {
compatible = "ti,am654-hbmc";
- reg = <0x00 0x47034000 0x00 0x100>,
+ reg = <0x00 0x47034000 0x00 0x0c>,
<0x05 0x00000000 0x01 0x0000000>;
power-domains = <&k3_pds 102 TI_SCI_PD_EXCLUSIVE>;
clocks = <&k3_clks 102 0>;
--
2.17.1

2023-05-09 11:19:35

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Update fss node and hbmc_mux



On 05/05/23 17:28, Vaishnav Achath wrote:
> From: Nishanth Menon <[email protected]>
>
> fss node claims to be a syscon node, while it actually is a simple bus

FSS

> where ospi, hbmc peripherals are located and a mux for path select

OSPI, HBMC

> between OSPI and Hyperbus which can be modelled as a reg-mux. So model
> it accordingly and use reg-mux to describe the hbmc-mux. Also update
> the region size to the correct values as per the TRM.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> Signed-off-by: Vaishnav Achath <[email protected]>
> ---
>
> V1->V2:
> * Address feedback from Udit to limit the FSS register region size as
> per TRM.
> * Use reg-mux changes to simplify the hbmc-mux modelling.
> * Update commit message to reflect changes.
>
> Depends on:
> https://lore.kernel.org/all/[email protected]/
>
> arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 13 +++++++------
> 1 file changed, 7 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..333564ca9c91 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> @@ -338,22 +338,23 @@
> status = "disabled";
> };
>
> - fss: syscon@47000000 {
> - compatible = "syscon", "simple-mfd";
> - reg = <0x00 0x47000000 0x00 0x100>;
> + fss: bus@47000000 {
> + compatible = "simple-bus";
> + reg = <0x00 0x47000000 0x0 0x7c>;

^^^^ 0x00

I know the registers only go up to 0x7c, but its convention to map
entire region that is reserved for the IP irrespective of how many
registers are actually valid (I see this across arm64 SoC Vendors).
Eg as per TRM, Table 203 MCU Domain map:

MCU_FSS0_CFG 0x0047000000 - 0x00470000FF (256B)




> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
>
> - hbmc_mux: hbmc-mux {
> - compatible = "mmio-mux";
> + hbmc_mux: mux-controller@47000004 {
> + compatible = "reg-mux";
> + reg = <0x00 0x47000004 0x00 0x2>;
> #mux-control-cells = <1>;
> mux-reg-masks = <0x4 0x2>; /* HBMC select */
> };
>
> hbmc: hyperbus@47034000 {
> compatible = "ti,am654-hbmc";
> - reg = <0x00 0x47034000 0x00 0x100>,
> + reg = <0x00 0x47034000 0x00 0x0c>,

Hmm, doesn't look correct? I see register addresses up to 0x47034048h in
TRM?

I prefer to map entire region reserved in the SoC memory map:
MCU_FSS0_HPB_CTRL 0x0047034000 - 0x00470340FF (256B)


> <0x05 0x00000000 0x01 0x0000000>;
> power-domains = <&k3_pds 102 TI_SCI_PD_EXCLUSIVE>;
> clocks = <&k3_clks 102 0>;

--
Regards
Vignesh

2023-05-10 16:06:25

by Andrew Davis

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] arm64: dts: ti: k3-j7200-mcu-wakeup: Update fss node and hbmc_mux

On 5/9/23 5:41 AM, Vignesh Raghavendra wrote:
>
>
> On 05/05/23 17:28, Vaishnav Achath wrote:
>> From: Nishanth Menon <[email protected]>
>>
>> fss node claims to be a syscon node, while it actually is a simple bus
>
> FSS
>
>> where ospi, hbmc peripherals are located and a mux for path select
>
> OSPI, HBMC
>
>> between OSPI and Hyperbus which can be modelled as a reg-mux. So model
>> it accordingly and use reg-mux to describe the hbmc-mux. Also update
>> the region size to the correct values as per the TRM.
>>
>> Signed-off-by: Nishanth Menon <[email protected]>
>> Signed-off-by: Vaishnav Achath <[email protected]>
>> ---
>>
>> V1->V2:
>> * Address feedback from Udit to limit the FSS register region size as
>> per TRM.
>> * Use reg-mux changes to simplify the hbmc-mux modelling.
>> * Update commit message to reflect changes.
>>
>> Depends on:
>> https://lore.kernel.org/all/[email protected]/
>>
>> arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 13 +++++++------
>> 1 file changed, 7 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..333564ca9c91 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
>> @@ -338,22 +338,23 @@
>> status = "disabled";
>> };
>>
>> - fss: syscon@47000000 {
>> - compatible = "syscon", "simple-mfd";
>> - reg = <0x00 0x47000000 0x00 0x100>;
>> + fss: bus@47000000 {
>> + compatible = "simple-bus";
>> + reg = <0x00 0x47000000 0x0 0x7c>;
>
> ^^^^ 0x00
>
> I know the registers only go up to 0x7c, but its convention to map
> entire region that is reserved for the IP irrespective of how many
> registers are actually valid (I see this across arm64 SoC Vendors).
> Eg as per TRM, Table 203 MCU Domain map:
>
> MCU_FSS0_CFG 0x0047000000 - 0x00470000FF (256B)
>
>
>
>
>> #address-cells = <2>;
>> #size-cells = <2>;
>> ranges;
>>
>> - hbmc_mux: hbmc-mux {
>> - compatible = "mmio-mux";
>> + hbmc_mux: mux-controller@47000004 {
>> + compatible = "reg-mux";
>> + reg = <0x00 0x47000004 0x00 0x2>;
>> #mux-control-cells = <1>;
>> mux-reg-masks = <0x4 0x2>; /* HBMC select */
>> };
>>
>> hbmc: hyperbus@47034000 {
>> compatible = "ti,am654-hbmc";
>> - reg = <0x00 0x47034000 0x00 0x100>,
>> + reg = <0x00 0x47034000 0x00 0x0c>,
>
> Hmm, doesn't look correct? I see register addresses up to 0x47034048h in
> TRM?
>
> I prefer to map entire region reserved in the SoC memory map:
> MCU_FSS0_HPB_CTRL 0x0047034000 - 0x00470340FF (256B)
>

I do agree here 0x100 is more clean, but we do have to watch for the
holes we have in memory right after some register spaces which cause
SErrors on access..

Either way this reg change should have been its own patch, not squashed
into this otherwise correct s/mmio-mux/reg-mux patch.

Andrew

>
>> <0x05 0x00000000 0x01 0x0000000>;
>> power-domains = <&k3_pds 102 TI_SCI_PD_EXCLUSIVE>;
>> clocks = <&k3_clks 102 0>;
>