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

[Gatien CHEVALLIER]

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