[PATCH v2] gpio: dwapb_gpio: Enable all clocks

[PATCH v2] gpio: dwapb_gpio: Enable all clocks

[Ralph Siemsen]

On some SoC (such as RZ/N1) it is necessary to enable clocks prior to
using the register interface.

Note that the "clocks" property is not required per devicetree binding.
Furthermore there are several devicetrees that lack "clocks" property.

Therefore ignore ENOENT from clk_get_bulk(), but report any other errors.

Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
---
Changes in v2:
- split out of the series "Renesas RZ/N1 additional drivers"
  https://lore.kernel.org/u-boot/20260731-rzn1-2026-07-v1-4-af2ce80db9d8@linaro.org/
- add error checking for clk_get_bulk() and clk_enable_bulk().
  Use a helper function to contain this logic.
---
 drivers/gpio/dwapb_gpio.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
index 7ab48780332..cdb26ff1ffa 100644
--- a/drivers/gpio/dwapb_gpio.c
+++ b/drivers/gpio/dwapb_gpio.c
@@ -7,6 +7,7 @@
 
 #include <asm/gpio.h>
 #include <asm/io.h>
+#include <clk.h>
 #include <dm/device.h>
 #include <dm/device-internal.h>
 #include <dm/device_compat.h>
@@ -28,6 +29,7 @@
 
 struct gpio_dwapb_priv {
 	struct reset_ctl_bulk	resets;
+	struct clk_bulk		clks;
 };
 
 struct gpio_dwapb_plat {
@@ -132,12 +134,34 @@ static int gpio_dwapb_reset(struct udevice *dev)
 	return 0;
 }
 
+static int gpio_dwapb_enable_optional_clocks(struct udevice *dev)
+{
+	struct gpio_dwapb_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = clk_get_bulk(dev, &priv->clks);
+	if (ret) {
+		/* Clocks are optional */
+		if (ret != -ENOENT)
+			dev_err(dev, "Failed to get clocks: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_enable_bulk(&priv->clks);
+	if (ret)
+		dev_err(dev, "Failed to enable clocks: %d\n", ret);
+
+	return ret;
+}
+
 static int gpio_dwapb_probe(struct udevice *dev)
 {
 	struct gpio_dev_priv *priv = dev_get_uclass_priv(dev);
 	struct gpio_dwapb_plat *plat = dev_get_plat(dev);
 
 	if (!plat) {
+		gpio_dwapb_enable_optional_clocks(dev);
+
 		/* Reset on parent device only */
 		return gpio_dwapb_reset(dev);
 	}
@@ -214,8 +238,10 @@ static int gpio_dwapb_remove(struct udevice *dev)
 	struct gpio_dwapb_plat *plat = dev_get_plat(dev);
 	struct gpio_dwapb_priv *priv = dev_get_priv(dev);
 
-	if (!plat && priv)
+	if (!plat && priv) {
+		clk_release_bulk(&priv->clks);
 		return reset_release_bulk(&priv->resets);
+	}
 
 	return 0;
 }

---
base-commit: 36c377b9859ffb53eb1e39ea31e8d96d1e0fe1e5
change-id: 20260811-rzn1-2026-10-gpio-5cb611f87674

Best regards,
--  
Ralph Siemsen <ralph.siemsen@linaro.org>

Re: [PATCH v2] gpio: dwapb_gpio: Enable all clocks

[Marek Vasut via U-Boot]

On 8/14/26 8:29 PM, Ralph Siemsen wrote:

[...]

> @@ -132,12 +134,34 @@ static int gpio_dwapb_reset(struct udevice *dev)
>   	return 0;
>   }
>   
> +static int gpio_dwapb_enable_optional_clocks(struct udevice *dev)
> +{
> +	struct gpio_dwapb_priv *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = clk_get_bulk(dev, &priv->clks);
> +	if (ret) {
> +		/* Clocks are optional */
> +		if (ret != -ENOENT)
> +			dev_err(dev, "Failed to get clocks: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_enable_bulk(&priv->clks);
> +	if (ret)
> +		dev_err(dev, "Failed to enable clocks: %d\n", ret);
> +
> +	return ret;
> +}
> +
>   static int gpio_dwapb_probe(struct udevice *dev)
>   {
>   	struct gpio_dev_priv *priv = dev_get_uclass_priv(dev);
>   	struct gpio_dwapb_plat *plat = dev_get_plat(dev);
>   
>   	if (!plat) {
> +		gpio_dwapb_enable_optional_clocks(dev);
Error is not propagated from this call.

Fail path that would disable the clock is missing in this function.

Re: [PATCH v2] gpio: dwapb_gpio: Enable all clocks

[Ralph Siemsen]

On Sat, Aug 15, 2026 at 12:05:55AM +0200, Marek Vasut wrote:
>On 8/14/26 8:29 PM, Ralph Siemsen wrote:
>
>[...]
>
>>@@ -132,12 +134,34 @@ static int gpio_dwapb_reset(struct udevice *dev)
>>  	return 0;
>>  }
>>+static int gpio_dwapb_enable_optional_clocks(struct udevice *dev)
>>+{
>>+	struct gpio_dwapb_priv *priv = dev_get_priv(dev);
>>+	int ret;
>>+
>>+	ret = clk_get_bulk(dev, &priv->clks);
>>+	if (ret) {
>>+		/* Clocks are optional */
>>+		if (ret != -ENOENT)
>>+			dev_err(dev, "Failed to get clocks: %d\n", ret);
>>+		return ret;
>>+	}
>>+
>>+	ret = clk_enable_bulk(&priv->clks);
>>+	if (ret)
>>+		dev_err(dev, "Failed to enable clocks: %d\n", ret);
>>+
>>+	return ret;
>>+}
>>+
>>  static int gpio_dwapb_probe(struct udevice *dev)
>>  {
>>  	struct gpio_dev_priv *priv = dev_get_uclass_priv(dev);
>>  	struct gpio_dwapb_plat *plat = dev_get_plat(dev);
>>  	if (!plat) {
>>+		gpio_dwapb_enable_optional_clocks(dev);

>Error is not propagated from this call.

This was intentional. All existing users work without explicitly 
enabling the clock. They probably do it in board specific code, or in a 
previous boot stage.

While adding clk_enable() should be harmless, what if it fails on some 
of the existing boards? I would rather not break them, this can be quite 
hard to debug.

>Fail path that would disable the clock is missing in this function.

Okay, I can add this.

Ralph