On Thu, Feb 27, 2025 at 11:57:54AM +0100, Marc Kleine-Budde wrote:

On 25.02.2025 16:14:34, Mihalcea Laurentiu wrote:

quoted

On 21.02.2025 21:56, Frank Li wrote:

quoted

On Fri, Feb 21, 2025 at 02:19:08PM -0500, Laurentiu Mihalcea wrote:

quoted

From: Laurentiu Mihalcea <redacted>

AIPS5 is actually AIPSTZ5 as it offers some security-related
configurations. Since these configurations need to be applied before
accessing any of the peripherals on the bus, it's better to make AIPSTZ5
be their parent instead of keeping AIPS5 and adding a child node for
AIPSTZ5. Also, because of the security configurations, the address space
of the bus has to be changed to that of the configuration registers.

The orginal 0x30c0_0000..0x31200000 include 0x30df0000, why not map only
config address part in your drivers.

Frank

Any concerns/anything wrong with current approach?


I find it a bit awkward to have the whole bus address space
in the DT given that we're only interested in using the access
controller register space.


I'm fine with the approach you suggested but I don't see a
reason for using it?

Looking at the "AIPS5 Memory Map" (page 34/35 in i.MX 8M Plus
Applications Processor Reference Manual, Rev. 3, 08/2024), the
AIPS5_Configuration is part of the AIPS5 bus. IMHO the bus is something
different than the bus configuration. Why not model it as part of the
bus?

quoted

quoted

quoted

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index e0d3b8cba221..a1d9b834d2da 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi

@@ -1399,11 +1399,13 @@ eqos: ethernet@30bf0000 {
 			};
 		};

-		aips5: bus@30c00000 {
-			compatible = "fsl,aips-bus", "simple-bus";
-			reg = <0x30c00000 0x400000>;
+		aips5: bus@30df0000 {

                       ^^^^^^^^^^^^

quoted

quoted

quoted

+			compatible = "fsl,imx8mp-aipstz", "simple-bus";
+			reg = <0x30df0000 0x10000>;
+			power-domains = <&pgc_audio>;
 			#address-cells = <1>;
 			#size-cells = <1>;
+			#access-controller-cells = <0>;
 			ranges;

 			spba-bus@30c00000 {

                        ^^^^^^^^^^^^^^^^^

This looks very strange: The aips5 bus starts at 0x30df0000 and has a
child bus starting at 0x30c00000?

@30df0000 should match controller reg's address.

subnode address 0x30c00000,  should be descript in "ranges", which 1:1 map.

So it should be reasonable. another example:
i2c@1000 {

	device@1c <- which use difference address space.
}

The similar case also happen at pcie.

Frank

regards,
Marc

--
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |