Re: [PATCH v2 11/11] pinctrl: stm32: add firewall checks before probing the HDP driver

[Linus Walleij]

Hi Gatien,

thanks for your patch!

On Wed, Jan 14, 2026 at 11:31 AM Gatien Chevallier
<gatien.chevallier@xxxxxxxxxxx> 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@xxxxxxxxxxx>
(...)
> +#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