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/[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: Split fss node up
.../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 29 ++++++++++++-------
1 file changed, 19 insertions(+), 10 deletions(-)
--
2.40.0
unit-address should not use a 0x prefix.
Signed-off-by: Nishanth Menon <[email protected]>
---
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
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 <[email protected]>
---
.../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
Use ti,j721e-system-controller to be explicit about the syscon node we
are using.
Signed-off-by: Nishanth Menon <[email protected]>
---
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
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 <[email protected]>
> ---
> .../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";
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 <[email protected]>
> ---
> .../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/[email protected]/
[1] https://lore.kernel.org/all/[email protected]/
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 <[email protected]>
> > ---
> > .../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/[email protected]/
> [1] https://lore.kernel.org/all/[email protected]/
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
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/[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: 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/[email protected]/
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
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