On Tue, 7 Feb 2023 15:12:23 +0100 Gatien CHEVALLIER <gatien.chevallier@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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@xxxxxxxxxxx> > >> Signed-off-by: Loic PALLARDY <loic.pallardy@xxxxxx> > > > > 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.
