Re: [PATCH v3 2/5] bus: stm32: Introduce firewall controller helpers

[Benjamin GAIGNARD]

On 5/5/20 4:40 PM, Greg KH wrote:
> On Tue, May 05, 2020 at 09:33:05AM +0200, Benjamin Gaignard wrote:
>> The goal of these helpers are to offer an interface for the
>> hardware blocks controlling bus accesses rights.
>>
>> Bus firewall controllers are typically used to control if a
>> hardware block can perform read or write operations on bus.
>>
>> Smarter firewall controllers could be able to define accesses
>> rights per hardware blocks to control where they can read
>> or write.
>>
>> Firewall controller configurations are provided in device node,
>> parsed by the helpers and send to the driver to apply them.
>> Each controller may need different number and type of inputs
>> to configure the firewall so device-tree properties size have to
>> be define by using "#firewall-cells".
>> Firewall configurations properties have to be named "firewall-X"
>> on device node.
>> "firewall-names" keyword can also be used to give a name to
>> a specific configuration.
>>
>> Example of device-tree:
>> ctrl0: firewall@0 {
>> 	#firewall-cells = <2>;
>>        };
>>
>> foo: foo@0 {
>> 	firewall-names = "default", "setting1";
>> 	firewall-0 = <&ctrl0 1 2>;
>> 	firewall-1 = <&ctrl0 3 4>;
>> };
>>
>> Configurations could be applied with functions like
>> firewall_set_config_by_index() or firewall_set_config_by_name().
>>
>> firewall_set_default_config() function will apply the
>> configuration named "default" (if existing) or the configuration
>> with index 0 (i.e. firewall-0).
>>
>> Drivers could register/unregister themselves be calling
>> firewall_register/firewall_unregister functions.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxx>
>> ---
>>   drivers/bus/Kconfig          |   2 +
>>   drivers/bus/Makefile         |   2 +
>>   drivers/bus/stm32/Kconfig    |   3 +
>>   drivers/bus/stm32/Makefile   |   1 +
>>   drivers/bus/stm32/firewall.c | 266 +++++++++++++++++++++++++++++++++++++++++++
>>   drivers/bus/stm32/firewall.h |  75 ++++++++++++
>>   6 files changed, 349 insertions(+)
>>   create mode 100644 drivers/bus/stm32/Kconfig
>>   create mode 100644 drivers/bus/stm32/Makefile
>>   create mode 100644 drivers/bus/stm32/firewall.c
>>   create mode 100644 drivers/bus/stm32/firewall.h
>>
>> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
>> index 6d4e4497b59b..843b356322d9 100644
>> --- a/drivers/bus/Kconfig
>> +++ b/drivers/bus/Kconfig
>> @@ -203,4 +203,6 @@ config DA8XX_MSTPRI
>>   source "drivers/bus/fsl-mc/Kconfig"
>>   source "drivers/bus/mhi/Kconfig"
>>   
>> +source "drivers/bus/stm32/Kconfig"
>> +
>>   endmenu
>> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
>> index 05f32cd694a4..5e0e34b10235 100644
>> --- a/drivers/bus/Makefile
>> +++ b/drivers/bus/Makefile
>> @@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
>>   
>>   # MHI
>>   obj-$(CONFIG_MHI_BUS)		+= mhi/
>> +
>> +obj-$(CONFIG_MACH_STM32MP157) 	+= stm32/
>> \ No newline at end of file
>> diff --git a/drivers/bus/stm32/Kconfig b/drivers/bus/stm32/Kconfig
>> new file mode 100644
>> index 000000000000..57221e833e2d
>> --- /dev/null
>> +++ b/drivers/bus/stm32/Kconfig
>> @@ -0,0 +1,3 @@
>> +config FIREWALL_CONTROLLERS
>> +	bool "Support of bus firewall controllers"
>> +	depends on OF
>> diff --git a/drivers/bus/stm32/Makefile b/drivers/bus/stm32/Makefile
>> new file mode 100644
>> index 000000000000..eb6b978d6450
>> --- /dev/null
>> +++ b/drivers/bus/stm32/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
>> diff --git a/drivers/bus/stm32/firewall.c b/drivers/bus/stm32/firewall.c
>> new file mode 100644
>> index 000000000000..95f716cf926f
>> --- /dev/null
>> +++ b/drivers/bus/stm32/firewall.c
>> @@ -0,0 +1,266 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
>> + * Author: Benjamin Gaignard <benjamin.gaignard@xxxxxx> for STMicroelectronics.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <linux/of.h>
>> +#include <linux/slab.h>
>> +
>> +#include "firewall.h"
>> +
>> +/* Mutex taken to protect firewall_list */
>> +static DEFINE_MUTEX(firewall_list_mutex);
>> +
>> +/* Global list of firewall control devices */
>> +static LIST_HEAD(firewall_list);
> Why is that needed?  Why can't you just walk the list of devices on this
> "bus/class" if you really wanted to?
>
> Along those lines, why is this going around the driver model and
> ignoring it?  Shouldn't this be a bus and you have devices attached to
> it of the specific type?
This part of the series is only a a set of common functions and bindings
that I plan to reuse for futur STM32 SoCs.
The 'real' bus implementation is in patch 4.

Benjamin
>
>
> greg k-h