On Tue, 7 Feb 2023 15:12:23 +0100
Gatien CHEVALLIER [off-list ref] wrote:

Hi Jonathan,

On 1/28/23 17:12, Jonathan Cameron wrote:

quoted

On Fri, 27 Jan 2023 17:40:38 +0100
Gatien Chevallier [off-list ref] wrote:
  

quoted

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 <redacted>  

Hi Gatien,

A few comments inline,

Thanks,

Jonathan
  

quoted

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.

  

quoted

+
+#define IDS_PER_DECPROT_REGS		16  

quoted

+#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.