Because the HDP peripheral both depends on debug and firewall
configuration, when CONFIG_STM32_FIREWALL is present, use the
stm32 firewall framework to be able to check these configuration against
the relevant controllers.
Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
drivers/pinctrl/stm32/pinctrl-stm32-hdp.c | 41 +++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32-hdp.c b/drivers/pinctrl/stm32/pinctrl-stm32-hdp.c
index 0b1dff01e04c..7e4aa0465c06 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32-hdp.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32-hdp.c
@@ -4,6 +4,7 @@
* Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
*/
#include <linux/bits.h>
+#include <linux/bus/stm32_firewall_device.h>
#include <linux/clk.h>
#include <linux/gpio/driver.h>
#include <linux/gpio/generic.h>
@@ -605,10 +606,50 @@ MODULE_DEVICE_TABLE(of, stm32_hdp_of_match);
static int stm32_hdp_probe(struct platform_device *pdev)
{
struct gpio_generic_chip_config config;
+ struct stm32_firewall *firewall = NULL;
struct device *dev = &pdev->dev;
struct stm32_hdp *hdp;
+ int nb_firewall;
u8 version;
int err;
+ int i;
+
+ nb_firewall = of_count_phandle_with_args(pdev->dev.of_node, "access-controllers",
+ "#access-controller-cells");
+ if (IS_ENABLED(CONFIG_STM32_FIREWALL) && nb_firewall != -ENOENT) {
+ if (nb_firewall <= 0)
+ return -EINVAL;
+
+ firewall = devm_kcalloc(dev, nb_firewall, sizeof(*firewall), GFP_KERNEL);
+ if (!firewall)
+ return -ENOMEM;
+
+ /* Get stm32 firewall information */
+ err = stm32_firewall_get_firewall(dev->of_node, firewall, nb_firewall);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to get firewall controller\n");
+
+ for (i = 0; i < nb_firewall; i++) {
+ err = stm32_firewall_grant_access_by_id(firewall + i,
+ firewall[i].firewall_id);
+ if (err) {
+ while (i) {
+ u32 id;
+
+ i--;
+ id = firewall[i].firewall_id;
+ stm32_firewall_release_access_by_id(firewall + i, id);
+ }
+ if (err == -EACCES) {
+ dev_info(dev, "No firewall access\n");
+ return -ENODEV;
+ }
+
+ return dev_err_probe(dev, err, "Error checking firewall access\n");
+ }
+ }
+ }
+
hdp = devm_kzalloc(dev, sizeof(*hdp), GFP_KERNEL);
if (!hdp)
--
2.43.0
Hi Gatien,
thanks for your patch!
On Wed, Jan 14, 2026 at 11:31 AM Gatien Chevallier
<gatien.chevallier@foss.st.com> wrote:
> Because the HDP peripheral both depends on debug and firewall
> configuration, when CONFIG_STM32_FIREWALL is present, use the
> stm32 firewall framework to be able to check these configuration against
> the relevant controllers.
>
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
(...)
> +#include <linux/bus/stm32_firewall_device.h>
> #include <linux/clk.h>
> #include <linux/gpio/driver.h>
> #include <linux/gpio/generic.h>
> @@ -605,10 +606,50 @@ MODULE_DEVICE_TABLE(of, stm32_hdp_of_match);
> static int stm32_hdp_probe(struct platform_device *pdev)
> {
> struct gpio_generic_chip_config config;
> + struct stm32_firewall *firewall = NULL;
> struct device *dev = &pdev->dev;
> struct stm32_hdp *hdp;
> + int nb_firewall;
> u8 version;
> int err;
> + int i;
> +
> + nb_firewall = of_count_phandle_with_args(pdev->dev.of_node, "access-controllers",
> + "#access-controller-cells");
> + if (IS_ENABLED(CONFIG_STM32_FIREWALL) && nb_firewall != -ENOENT) {
> + if (nb_firewall <= 0)
> + return -EINVAL;
> +
> + firewall = devm_kcalloc(dev, nb_firewall, sizeof(*firewall), GFP_KERNEL);
> + if (!firewall)
> + return -ENOMEM;
> +
> + /* Get stm32 firewall information */
> + err = stm32_firewall_get_firewall(dev->of_node, firewall, nb_firewall);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to get firewall controller\n");
> +
> + for (i = 0; i < nb_firewall; i++) {
> + err = stm32_firewall_grant_access_by_id(firewall + i,
> + firewall[i].firewall_id);
> + if (err) {
> + while (i) {
> + u32 id;
> +
> + i--;
> + id = firewall[i].firewall_id;
> + stm32_firewall_release_access_by_id(firewall + i, id);
> + }
> + if (err == -EACCES) {
> + dev_info(dev, "No firewall access\n");
> + return -ENODEV;
> + }
> +
> + return dev_err_probe(dev, err, "Error checking firewall access\n");
> + }
> + }
> + }
Doesn't this whole piece of code look very generic?
Point out to me if something is pin control-specific about it?
Can't we just add a helper function such as
stm32_firewall_of_check_access(struct device *dev)
{
struct stm32_firewall *firewall = NULL;
int nb_firewall;
nb_firewall = of_count_phandle_with_args(pdev->dev.of_node,
"access-controllers",
"#access-controller-cells");
(...)
}
Then place the prototype for this in <linux/bus/stm32_firewall_device.h>.
I think this will be helpful for the next driver that needs to check
firewall access
before continuing.
Yours,
Linus Walleij
On 1/18/26 23:19, Linus Walleij wrote:
> Hi Gatien,
>
> thanks for your patch!
>
> On Wed, Jan 14, 2026 at 11:31 AM Gatien Chevallier
> <gatien.chevallier@foss.st.com> wrote:
>
>> Because the HDP peripheral both depends on debug and firewall
>> configuration, when CONFIG_STM32_FIREWALL is present, use the
>> stm32 firewall framework to be able to check these configuration against
>> the relevant controllers.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> (...)
>> +#include <linux/bus/stm32_firewall_device.h>
>> #include <linux/clk.h>
>> #include <linux/gpio/driver.h>
>> #include <linux/gpio/generic.h>
>> @@ -605,10 +606,50 @@ MODULE_DEVICE_TABLE(of, stm32_hdp_of_match);
>> static int stm32_hdp_probe(struct platform_device *pdev)
>> {
>> struct gpio_generic_chip_config config;
>> + struct stm32_firewall *firewall = NULL;
>> struct device *dev = &pdev->dev;
>> struct stm32_hdp *hdp;
>> + int nb_firewall;
>> u8 version;
>> int err;
>> + int i;
>> +
>> + nb_firewall = of_count_phandle_with_args(pdev->dev.of_node, "access-controllers",
>> + "#access-controller-cells");
>> + if (IS_ENABLED(CONFIG_STM32_FIREWALL) && nb_firewall != -ENOENT) {
>> + if (nb_firewall <= 0)
>> + return -EINVAL;
>> +
>> + firewall = devm_kcalloc(dev, nb_firewall, sizeof(*firewall), GFP_KERNEL);
>> + if (!firewall)
>> + return -ENOMEM;
>> +
>> + /* Get stm32 firewall information */
>> + err = stm32_firewall_get_firewall(dev->of_node, firewall, nb_firewall);
>> + if (err)
>> + return dev_err_probe(dev, err, "Failed to get firewall controller\n");
>> +
>> + for (i = 0; i < nb_firewall; i++) {
>> + err = stm32_firewall_grant_access_by_id(firewall + i,
>> + firewall[i].firewall_id);
>> + if (err) {
>> + while (i) {
>> + u32 id;
>> +
>> + i--;
>> + id = firewall[i].firewall_id;
>> + stm32_firewall_release_access_by_id(firewall + i, id);
>> + }
>> + if (err == -EACCES) {
>> + dev_info(dev, "No firewall access\n");
>> + return -ENODEV;
>> + }
>> +
>> + return dev_err_probe(dev, err, "Error checking firewall access\n");
>> + }
>> + }
>> + }
>
> Doesn't this whole piece of code look very generic?
>
> Point out to me if something is pin control-specific about it?
>
> Can't we just add a helper function such as
>
> stm32_firewall_of_check_access(struct device *dev)
> {
> struct stm32_firewall *firewall = NULL;
> int nb_firewall;
>
> nb_firewall = of_count_phandle_with_args(pdev->dev.of_node,
> "access-controllers",
> "#access-controller-cells");
> (...)
> }
>
> Then place the prototype for this in <linux/bus/stm32_firewall_device.h>.
>
> I think this will be helpful for the next driver that needs to check
> firewall access
> before continuing.
>
> Yours,
> Linus Walleij
Hello Linus,
Thanks for your feedback. There is already a function to check the
firewall access that is stm32_firewall_grant_access().
However, a helper could clearly implemented to wrap it with the
get when all elements should be assessed, as you're suggesting.
I'll submit V3 with a proposition, let's see.
Best regards,
Gatien
© 2016 - 2026 Red Hat, Inc.