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

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
[off-list ref] wrote:

quoted

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>

(...)

quoted

+#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