diff mbox series

[v3,4/6] bus: stm32_sys_bus: add support for STM32MP15 and STM32MP13 system bus

Message ID 20230127164040.1047583-5-gatien.chevallier@foss.st.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series Introduce STM32 system bus | expand

Commit Message

Gatien CHEVALLIER Jan. 27, 2023, 4:40 p.m. UTC 
This driver is checking the access rights of the different
peripherals connected to the system bus. If access is denied,
the associated device tree node is skipped so the platform bus
does not probe it.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Loic PALLARDY <loic.pallardy@st.com>
---

No changes in V2.

Changes in V3:
	- Remove useless sys_bus_get_access and unused map_table fields in
	stm32_sys_bus_match_data structure
	- Use devm_platform_ioremap_resource() helper
	- Clean check on of_device_get_match_data() and remove unused local
	variables in stm32_sys_bus_probe().

 MAINTAINERS                 |   6 ++
 drivers/bus/Kconfig         |   9 ++
 drivers/bus/Makefile        |   1 +
 drivers/bus/stm32_sys_bus.c | 168 ++++++++++++++++++++++++++++++++++++
 4 files changed, 184 insertions(+)
 create mode 100644 drivers/bus/stm32_sys_bus.c

Comments

Jonathan Cameron Jan. 28, 2023, 4:12 p.m. UTC | #1 
On Fri, 27 Jan 2023 17:40:38 +0100
Gatien Chevallier <gatien.chevallier@foss.st.com> wrote:

> This driver is checking the access rights of the different
> peripherals connected to the system bus. If access is denied,
> the associated device tree node is skipped so the platform bus
> does not probe it.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> Signed-off-by: Loic PALLARDY <loic.pallardy@st.com>

Hi Gatien,

A few comments inline,

Thanks,

Jonathan

> diff --git a/drivers/bus/stm32_sys_bus.c b/drivers/bus/stm32_sys_bus.c
> new file mode 100644
> index 000000000000..c12926466bae
> --- /dev/null
> +++ b/drivers/bus/stm32_sys_bus.c
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +/* ETZPC peripheral as firewall bus */
> +/* ETZPC registers */
> +#define ETZPC_DECPROT			0x10
> +
> +/* ETZPC miscellaneous */
> +#define ETZPC_PROT_MASK			GENMASK(1, 0)
> +#define ETZPC_PROT_A7NS			0x3
> +#define ETZPC_DECPROT_SHIFT		1

This define makes the code harder to read.  What we care about is
the number of bits in the register divided by number of entries.
(which is 2) hence the shift by 1. See below for more on this.


> +
> +#define IDS_PER_DECPROT_REGS		16

> +#define STM32MP15_ETZPC_ENTRIES		96
> +#define STM32MP13_ETZPC_ENTRIES		64

These defines just make the code harder to check.
They aren't magic numbers, but rather just telling us how many
entries there are, so I would just put them in the structures directly.
Their use make it clear what they are without needing to give them a name.


> +struct stm32_sys_bus_match_data {

Comment on naming of this below.

> +	unsigned int max_entries;
> +};
> +

+static int stm32_etzpc_get_access(struct sys_bus_data *pdata, struct device_node *np)
+{
+	int err;
+	u32 offset, reg_offset, sec_val, id;
+
+	err = stm32_sys_bus_get_periph_id(pdata, np, &id);
+	if (err)
+		return err;
+
+	/* Check access configuration, 16 peripherals per register */
+	reg_offset = ETZPC_DECPROT + 0x4 * (id / IDS_PER_DECPROT_REGS);
+	offset = (id % IDS_PER_DECPROT_REGS) << ETZPC_DECPROT_SHIFT;

Use of defines in here is actively unhelpful when it comes to review. I would suggest letting
the maths be self explanatory (even if it's more code).

	offset = (id % IDS_PER_DECPROT_REGS) * (sizeof(u32) * BITS_PER_BYTE / IDS_PER_DECPROT_REGS);

Or if you prefer have a define of

#define DECPROT_BITS_PER_ID (sizeof(u32) * BITS_PER_BYTE / IDS_PER_DECPROT_REGS)

and
	offset = (id % IDS_PER_DECPROT_REGS) * DECPROT_BITS_PER_ID;

+
+	/* Verify peripheral is non-secure and attributed to cortex A7 */
+	sec_val = (readl(pdata->sys_bus_base + reg_offset) >> offset) & ETZPC_PROT_MASK;
+	if (sec_val != ETZPC_PROT_A7NS) {
+		dev_dbg(pdata->dev, "Invalid bus configuration: reg_offset %#x, value %d\n",
+			reg_offset, sec_val);
+		return -EACCES;
+	}
+
+	return 0;
+}
+
...

> +static int stm32_sys_bus_probe(struct platform_device *pdev)
> +{
> +	struct sys_bus_data *pdata;
> +	void __iomem *mmio;
> +	struct device_node *np = pdev->dev.of_node;

I'd be consistent. You use dev_of_node() accessor elsewhere, so should
use it here as well.

> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	mmio = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(mmio))
> +		return PTR_ERR(mmio);
> +
> +	pdata->sys_bus_base = mmio;
> +	pdata->pconf = of_device_get_match_data(&pdev->dev);
> +	pdata->dev = &pdev->dev;
> +
> +	platform_set_drvdata(pdev, pdata);

Does this get used? I can't immediately spot where but maybe I just
missed it.

> +
> +	stm32_sys_bus_populate(pdata);
> +
> +	/* Populate all available nodes */
> +	return of_platform_populate(np, NULL, NULL, &pdev->dev);

As np only used here, I'd not bother with the local variable in this function.

> +}
> +
> +static const struct stm32_sys_bus_match_data stm32mp15_sys_bus_data = {

Naming a structure after where it comes from is a little unusual and
confusion when a given call gets it from somewhere else.

I'd expect it to be named after what sort of thing it contains.
stm32_sys_bus_info or something like that.

> +	.max_entries = STM32MP15_ETZPC_ENTRIES,
> +};
> +
> +static const struct stm32_sys_bus_match_data stm32mp13_sys_bus_data = {
> +	.max_entries = STM32MP13_ETZPC_ENTRIES,
> +};
> +
> +static const struct of_device_id stm32_sys_bus_of_match[] = {
> +	{ .compatible = "st,stm32mp15-sys-bus", .data = &stm32mp15_sys_bus_data },
> +	{ .compatible = "st,stm32mp13-sys-bus", .data = &stm32mp13_sys_bus_data },

Alphabetical order usually preferred when there isn't a strong reason for
another choice.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, stm32_sys_bus_of_match);
> +
> +static struct platform_driver stm32_sys_bus_driver = {
> +	.probe  = stm32_sys_bus_probe,
> +	.driver = {
> +		.name = "stm32-sys-bus",
> +		.of_match_table = stm32_sys_bus_of_match,
> +	},
> +};
> +
> +static int __init stm32_sys_bus_init(void)
> +{
> +	return platform_driver_register(&stm32_sys_bus_driver);
> +}
> +arch_initcall(stm32_sys_bus_init);
> +

Unwanted trailing blank line.
Gatien CHEVALLIER Feb. 7, 2023, 2:12 p.m. UTC | #2 
Hi Jonathan,

On 1/28/23 17:12, Jonathan Cameron wrote:
> On Fri, 27 Jan 2023 17:40:38 +0100
> Gatien Chevallier <gatien.chevallier@foss.st.com> wrote:
> 
>> This driver is checking the access rights of the different
>> peripherals connected to the system bus. If access is denied,
>> the associated device tree node is skipped so the platform bus
>> does not probe it.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
>> Signed-off-by: Loic PALLARDY <loic.pallardy@st.com>
> 
> Hi Gatien,
> 
> A few comments inline,
> 
> Thanks,
> 
> Jonathan
> 
>> diff --git a/drivers/bus/stm32_sys_bus.c b/drivers/bus/stm32_sys_bus.c
>> new file mode 100644
>> index 000000000000..c12926466bae
>> --- /dev/null
>> +++ b/drivers/bus/stm32_sys_bus.c
>> @@ -0,0 +1,168 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/bits.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +/* ETZPC peripheral as firewall bus */
>> +/* ETZPC registers */
>> +#define ETZPC_DECPROT			0x10
>> +
>> +/* ETZPC miscellaneous */
>> +#define ETZPC_PROT_MASK			GENMASK(1, 0)
>> +#define ETZPC_PROT_A7NS			0x3
>> +#define ETZPC_DECPROT_SHIFT		1
> 
> This define makes the code harder to read.  What we care about is
> the number of bits in the register divided by number of entries.
> (which is 2) hence the shift by 1. See below for more on this.
> 
> 
>> +
>> +#define IDS_PER_DECPROT_REGS		16
> 
>> +#define STM32MP15_ETZPC_ENTRIES		96
>> +#define STM32MP13_ETZPC_ENTRIES		64
> 
> These defines just make the code harder to check.
> They aren't magic numbers, but rather just telling us how many
> entries there are, so I would just put them in the structures directly.
> Their use make it clear what they are without needing to give them a name.
> 

Honestly, I'd rather read the hardware configuration registers to get 
this information instead of differentiating MP13/15. Would you agree on 
that?

> 
>> +struct stm32_sys_bus_match_data {
> 
> Comment on naming of this below.
> 
>> +	unsigned int max_entries;
>> +};
>> +
> 
> +static int stm32_etzpc_get_access(struct sys_bus_data *pdata, struct device_node *np)
> +{
> +	int err;
> +	u32 offset, reg_offset, sec_val, id;
> +
> +	err = stm32_sys_bus_get_periph_id(pdata, np, &id);
> +	if (err)
> +		return err;
> +
> +	/* Check access configuration, 16 peripherals per register */
> +	reg_offset = ETZPC_DECPROT + 0x4 * (id / IDS_PER_DECPROT_REGS);
> +	offset = (id % IDS_PER_DECPROT_REGS) << ETZPC_DECPROT_SHIFT;
> 
> Use of defines in here is actively unhelpful when it comes to review. I would suggest letting
> the maths be self explanatory (even if it's more code).
> 
> 	offset = (id % IDS_PER_DECPROT_REGS) * (sizeof(u32) * BITS_PER_BYTE / IDS_PER_DECPROT_REGS);
> 
> Or if you prefer have a define of
> 
> #define DECPROT_BITS_PER_ID (sizeof(u32) * BITS_PER_BYTE / IDS_PER_DECPROT_REGS)
> 
> and
> 	offset = (id % IDS_PER_DECPROT_REGS) * DECPROT_BITS_PER_ID;
> 

Ok I'll rework this for better understanding. Your suggestion seems fine

> +
> +	/* Verify peripheral is non-secure and attributed to cortex A7 */
> +	sec_val = (readl(pdata->sys_bus_base + reg_offset) >> offset) & ETZPC_PROT_MASK;
> +	if (sec_val != ETZPC_PROT_A7NS) {
> +		dev_dbg(pdata->dev, "Invalid bus configuration: reg_offset %#x, value %d\n",
> +			reg_offset, sec_val);
> +		return -EACCES;
> +	}
> +
> +	return 0;
> +}
> +
> ...
> 
>> +static int stm32_sys_bus_probe(struct platform_device *pdev)
>> +{
>> +	struct sys_bus_data *pdata;
>> +	void __iomem *mmio;
>> +	struct device_node *np = pdev->dev.of_node;
> 
> I'd be consistent. You use dev_of_node() accessor elsewhere, so should
> use it here as well >> +
>> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> +	if (!pdata)
>> +		return -ENOMEM;
>> +
>> +	mmio = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(mmio))
>> +		return PTR_ERR(mmio);
>> +
>> +	pdata->sys_bus_base = mmio;
>> +	pdata->pconf = of_device_get_match_data(&pdev->dev);
>> +	pdata->dev = &pdev->dev;
>> +
>> +	platform_set_drvdata(pdev, pdata);
> 
> Does this get used? I can't immediately spot where but maybe I just
> missed it.
> 

Not for now :)

>> +
>> +	stm32_sys_bus_populate(pdata);
>> +
>> +	/* Populate all available nodes */
>> +	return of_platform_populate(np, NULL, NULL, &pdev->dev);
> 
> As np only used here, I'd not bother with the local variable in this function.
> 

Agreed

>> +}
>> +
>> +static const struct stm32_sys_bus_match_data stm32mp15_sys_bus_data = {
> 
> Naming a structure after where it comes from is a little unusual and
> confusion when a given call gets it from somewhere else.
> 
> I'd expect it to be named after what sort of thing it contains.
> stm32_sys_bus_info or something like that.
> 

Then, this shall be removed thanks to the read to hardware registers.

>> +	.max_entries = STM32MP15_ETZPC_ENTRIES,
>> +};
>> +
>> +static const struct stm32_sys_bus_match_data stm32mp13_sys_bus_data = {
>> +	.max_entries = STM32MP13_ETZPC_ENTRIES,
>> +};
>> +
>> +static const struct of_device_id stm32_sys_bus_of_match[] = {
>> +	{ .compatible = "st,stm32mp15-sys-bus", .data = &stm32mp15_sys_bus_data },
>> +	{ .compatible = "st,stm32mp13-sys-bus", .data = &stm32mp13_sys_bus_data },
> 
> Alphabetical order usually preferred when there isn't a strong reason for
> another choice.
> 

I second that

>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_sys_bus_of_match);
>> +
>> +static struct platform_driver stm32_sys_bus_driver = {
>> +	.probe  = stm32_sys_bus_probe,
>> +	.driver = {
>> +		.name = "stm32-sys-bus",
>> +		.of_match_table = stm32_sys_bus_of_match,
>> +	},
>> +};
>> +
>> +static int __init stm32_sys_bus_init(void)
>> +{
>> +	return platform_driver_register(&stm32_sys_bus_driver);
>> +}
>> +arch_initcall(stm32_sys_bus_init);
>> +
> 
> Unwanted trailing blank line.
> 

Good spot, thanks

> 

Best regards,
Gatien
Jonathan Cameron Feb. 8, 2023, 7:08 p.m. UTC | #3 
On Tue, 7 Feb 2023 15:12:23 +0100
Gatien CHEVALLIER <gatien.chevallier@foss.st.com> wrote:

> Hi Jonathan,
> 
> On 1/28/23 17:12, Jonathan Cameron wrote:
> > On Fri, 27 Jan 2023 17:40:38 +0100
> > Gatien Chevallier <gatien.chevallier@foss.st.com> wrote:
> >   
> >> This driver is checking the access rights of the different
> >> peripherals connected to the system bus. If access is denied,
> >> the associated device tree node is skipped so the platform bus
> >> does not probe it.
> >>
> >> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> >> Signed-off-by: Loic PALLARDY <loic.pallardy@st.com>  
> > 
> > Hi Gatien,
> > 
> > A few comments inline,
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> >> diff --git a/drivers/bus/stm32_sys_bus.c b/drivers/bus/stm32_sys_bus.c
> >> new file mode 100644
> >> index 000000000000..c12926466bae
> >> --- /dev/null
> >> +++ b/drivers/bus/stm32_sys_bus.c
> >> @@ -0,0 +1,168 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
> >> + */
> >> +
> >> +#include <linux/bitfield.h>
> >> +#include <linux/bits.h>
> >> +#include <linux/device.h>
> >> +#include <linux/err.h>
> >> +#include <linux/io.h>
> >> +#include <linux/init.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_platform.h>
> >> +#include <linux/platform_device.h>
> >> +
> >> +/* ETZPC peripheral as firewall bus */
> >> +/* ETZPC registers */
> >> +#define ETZPC_DECPROT			0x10
> >> +
> >> +/* ETZPC miscellaneous */
> >> +#define ETZPC_PROT_MASK			GENMASK(1, 0)
> >> +#define ETZPC_PROT_A7NS			0x3
> >> +#define ETZPC_DECPROT_SHIFT		1  
> > 
> > This define makes the code harder to read.  What we care about is
> > the number of bits in the register divided by number of entries.
> > (which is 2) hence the shift by 1. See below for more on this.
> > 
> >   
> >> +
> >> +#define IDS_PER_DECPROT_REGS		16  
> >   
> >> +#define STM32MP15_ETZPC_ENTRIES		96
> >> +#define STM32MP13_ETZPC_ENTRIES		64  
> > 
> > These defines just make the code harder to check.
> > They aren't magic numbers, but rather just telling us how many
> > entries there are, so I would just put them in the structures directly.
> > Their use make it clear what they are without needing to give them a name.
> >   
> 
> Honestly, I'd rather read the hardware configuration registers to get 
> this information instead of differentiating MP13/15. Would you agree on 
> that?

Sure, if they are discoverable even better.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 5b74014994f5..aafe32aa1925 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19782,6 +19782,12 @@  L:	linux-spi@vger.kernel.org
 S:	Maintained
 F:	drivers/spi/spi-stm32.c
 
+ST STM32 SYS BUS DRIVER
+M:	Gatien Chevallier <gatien.chevallier@foss.st.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/bus/st,sys-bus.yaml
+F:	drivers/bus/stm32_sys_bus.c
+
 ST STPDDC60 DRIVER
 M:	Daniel Nilsson <daniel.nilsson@flex.com>
 L:	linux-hwmon@vger.kernel.org
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 7bfe998f3514..3f7bc1f67916 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -163,6 +163,15 @@  config QCOM_SSC_BLOCK_BUS
 	  i2c/spi/uart controllers, a hexagon core, and a clock controller
 	  which provides clocks for the above.
 
+config STM32_SYS_BUS
+	bool "STM32 System bus controller"
+	depends on ARCH_STM32 || COMPILE_TEST
+	default MACH_STM32MP157 || MACH_STM32MP13
+	help
+	  Say y to enable device access right verification before device probing.
+	  If access not granted, device won't be probed and an error message will
+	  provide the reason.
+
 config SUN50I_DE2_BUS
 	bool "Allwinner A64 DE2 Bus Driver"
 	  default ARM64
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index d90eed189a65..b15fdc42d0be 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -26,6 +26,7 @@  obj-$(CONFIG_OMAP_INTERCONNECT)	+= omap_l3_smx.o omap_l3_noc.o
 obj-$(CONFIG_OMAP_OCP2SCP)	+= omap-ocp2scp.o
 obj-$(CONFIG_QCOM_EBI2)		+= qcom-ebi2.o
 obj-$(CONFIG_QCOM_SSC_BLOCK_BUS)	+= qcom-ssc-block-bus.o
+obj-$(CONFIG_STM32_SYS_BUS)	+= stm32_sys_bus.o
 obj-$(CONFIG_SUN50I_DE2_BUS)	+= sun50i-de2.o
 obj-$(CONFIG_SUNXI_RSB)		+= sunxi-rsb.o
 obj-$(CONFIG_OF)		+= simple-pm-bus.o
diff --git a/drivers/bus/stm32_sys_bus.c b/drivers/bus/stm32_sys_bus.c
new file mode 100644
index 000000000000..c12926466bae
--- /dev/null
+++ b/drivers/bus/stm32_sys_bus.c
@@ -0,0 +1,168 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+/* ETZPC peripheral as firewall bus */
+/* ETZPC registers */
+#define ETZPC_DECPROT			0x10
+
+/* ETZPC miscellaneous */
+#define ETZPC_PROT_MASK			GENMASK(1, 0)
+#define ETZPC_PROT_A7NS			0x3
+#define ETZPC_DECPROT_SHIFT		1
+
+#define IDS_PER_DECPROT_REGS		16
+#define STM32MP15_ETZPC_ENTRIES		96
+#define STM32MP13_ETZPC_ENTRIES		64
+
+struct sys_bus_data;
+
+struct stm32_sys_bus_match_data {
+	unsigned int max_entries;
+};
+
+struct sys_bus_data {
+	const struct stm32_sys_bus_match_data *pconf;
+	void __iomem *sys_bus_base;
+	struct device *dev;
+};
+
+static int stm32_sys_bus_get_periph_id(struct sys_bus_data *pdata, struct device_node *np, u32 *id)
+{
+	int err;
+	u32 feature_domain_cell[2];
+	u32 id_bus;
+
+	/* Get reg from device node */
+	err = of_property_read_u32_array(np, "feature-domains", feature_domain_cell, 2);
+	if (err) {
+		dev_err(pdata->dev, "Unable to find feature-domains property\n");
+		return -ENODEV;
+	}
+
+	id_bus = feature_domain_cell[1];
+
+	if (id_bus >= pdata->pconf->max_entries) {
+		dev_err(pdata->dev, "Invalid sys bus ID for %s\n", np->full_name);
+		return -EINVAL;
+	}
+
+	*id = id_bus;
+
+	return 0;
+}
+
+static int stm32_etzpc_get_access(struct sys_bus_data *pdata, struct device_node *np)
+{
+	int err;
+	u32 offset, reg_offset, sec_val, id;
+
+	err = stm32_sys_bus_get_periph_id(pdata, np, &id);
+	if (err)
+		return err;
+
+	/* Check access configuration, 16 peripherals per register */
+	reg_offset = ETZPC_DECPROT + 0x4 * (id / IDS_PER_DECPROT_REGS);
+	offset = (id % IDS_PER_DECPROT_REGS) << ETZPC_DECPROT_SHIFT;
+
+	/* Verify peripheral is non-secure and attributed to cortex A7 */
+	sec_val = (readl(pdata->sys_bus_base + reg_offset) >> offset) & ETZPC_PROT_MASK;
+	if (sec_val != ETZPC_PROT_A7NS) {
+		dev_dbg(pdata->dev, "Invalid bus configuration: reg_offset %#x, value %d\n",
+			reg_offset, sec_val);
+		return -EACCES;
+	}
+
+	return 0;
+}
+
+static void stm32_sys_bus_populate(struct sys_bus_data *pdata)
+{
+	struct device *parent;
+	struct device_node *child;
+
+	parent = pdata->dev;
+
+	dev_dbg(parent, "Populating %s system bus\n", pdata->dev->driver->name);
+
+	for_each_available_child_of_node(dev_of_node(parent), child) {
+		if (stm32_etzpc_get_access(pdata, child)) {
+			/*
+			 * Peripheral access not allowed.
+			 * Mark the node as populated so platform bus won't probe it
+			 */
+			of_node_set_flag(child, OF_POPULATED);
+			dev_dbg(parent, "%s: Peripheral will not be probed\n",
+				child->full_name);
+		}
+	}
+}
+
+static int stm32_sys_bus_probe(struct platform_device *pdev)
+{
+	struct sys_bus_data *pdata;
+	void __iomem *mmio;
+	struct device_node *np = pdev->dev.of_node;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	mmio = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mmio))
+		return PTR_ERR(mmio);
+
+	pdata->sys_bus_base = mmio;
+	pdata->pconf = of_device_get_match_data(&pdev->dev);
+	pdata->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, pdata);
+
+	stm32_sys_bus_populate(pdata);
+
+	/* Populate all available nodes */
+	return of_platform_populate(np, NULL, NULL, &pdev->dev);
+}
+
+static const struct stm32_sys_bus_match_data stm32mp15_sys_bus_data = {
+	.max_entries = STM32MP15_ETZPC_ENTRIES,
+};
+
+static const struct stm32_sys_bus_match_data stm32mp13_sys_bus_data = {
+	.max_entries = STM32MP13_ETZPC_ENTRIES,
+};
+
+static const struct of_device_id stm32_sys_bus_of_match[] = {
+	{ .compatible = "st,stm32mp15-sys-bus", .data = &stm32mp15_sys_bus_data },
+	{ .compatible = "st,stm32mp13-sys-bus", .data = &stm32mp13_sys_bus_data },
+	{}
+};
+MODULE_DEVICE_TABLE(of, stm32_sys_bus_of_match);
+
+static struct platform_driver stm32_sys_bus_driver = {
+	.probe  = stm32_sys_bus_probe,
+	.driver = {
+		.name = "stm32-sys-bus",
+		.of_match_table = stm32_sys_bus_of_match,
+	},
+};
+
+static int __init stm32_sys_bus_init(void)
+{
+	return platform_driver_register(&stm32_sys_bus_driver);
+}
+arch_initcall(stm32_sys_bus_init);
+