[PATCH] clk: rk3288: add stub for ops->enable

[PATCH] clk: rk3288: add stub for ops->enable

[Marius Dinu]

This fixes a bug where the watchdog can't enable the clock.
AFAIK, this clock is always enabled.

Tested on Asus TinkerBoard. Test results:
wdt dev         => works
wdt start 200   => works, but timeout is 43s
wdt start 10000 => works, but timeout is 1m27s
wdt reset       => works
wdt stop        => doesn't work

Signed-off-by: Marius Dinu <m95d+git@psihoexpert.ro>
---
 drivers/clk/rockchip/clk_rk3288.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/rockchip/clk_rk3288.c b/drivers/clk/rockchip/clk_rk3288.c
index a4ff1c41abb..9cc883662ff 100644
--- a/drivers/clk/rockchip/clk_rk3288.c
+++ b/drivers/clk/rockchip/clk_rk3288.c
@@ -745,6 +745,10 @@ static ulong rockchip_saradc_set_clk(struct rockchip_cru *cru, uint hz)
 	return rockchip_saradc_get_clk(cru);
 }
 
+static int rk3288_clk_enable(struct clk *clk){
+	return 0;
+}
+
 static ulong rk3288_clk_get_rate(struct clk *clk)
 {
 	struct rk3288_clk_priv *priv = dev_get_priv(clk->dev);
@@ -947,6 +951,7 @@ static int __maybe_unused rk3288_clk_set_parent(struct clk *clk, struct clk *par
 }
 
 static struct clk_ops rk3288_clk_ops = {
+	.enable		= rk3288_clk_enable,
 	.get_rate	= rk3288_clk_get_rate,
 	.set_rate	= rk3288_clk_set_rate,
 #if CONFIG_IS_ENABLED(OF_REAL)
-- 
2.52.0

Re: [PATCH] clk: rk3288: add stub for ops->enable

[Jonas Karlman]

Hi Marius,

On 3/16/2026 10:57 AM, Marius Dinu wrote:
> This fixes a bug where the watchdog can't enable the clock.
> AFAIK, this clock is always enabled.
> 
> Tested on Asus TinkerBoard. Test results:
> wdt dev         => works
> wdt start 200   => works, but timeout is 43s
> wdt start 10000 => works, but timeout is 1m27s
> wdt reset       => works
> wdt stop        => doesn't work

I have a different patch incoming that should fix this issue at watchdog
driver level, something like following should make this work and
properly handle platforms not implementing clk_enable ops.

diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c
index bd9d7105366..91228de5e8e 100644
--- a/drivers/watchdog/designware_wdt.c
+++ b/drivers/watchdog/designware_wdt.c
@@ -122,7 +122,7 @@ static int designware_wdt_probe(struct udevice *dev)
                return ret;

 	ret = clk_enable(&clk);
-	if (ret)
+	if (ret && ret != -ENOSYS)
 		return ret;

 	priv->clk_khz = clk_get_rate(&clk) / 1000;

> 
> Signed-off-by: Marius Dinu <m95d+git@psihoexpert.ro>
> ---
>  drivers/clk/rockchip/clk_rk3288.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/clk/rockchip/clk_rk3288.c b/drivers/clk/rockchip/clk_rk3288.c
> index a4ff1c41abb..9cc883662ff 100644
> --- a/drivers/clk/rockchip/clk_rk3288.c
> +++ b/drivers/clk/rockchip/clk_rk3288.c
> @@ -745,6 +745,10 @@ static ulong rockchip_saradc_set_clk(struct rockchip_cru *cru, uint hz)
>  	return rockchip_saradc_get_clk(cru);
>  }
>  
> +static int rk3288_clk_enable(struct clk *clk){
> +	return 0;

This should properly be implemented for the clocks you need, and return
-ENOSYS for the clocks not implemented.

> +}
> +
>  static ulong rk3288_clk_get_rate(struct clk *clk)
>  {
>  	struct rk3288_clk_priv *priv = dev_get_priv(clk->dev);
> @@ -947,6 +951,7 @@ static int __maybe_unused rk3288_clk_set_parent(struct clk *clk, struct clk *par
>  }
>  
>  static struct clk_ops rk3288_clk_ops = {
> +	.enable		= rk3288_clk_enable,

Also include proper disable ops for the clocks you add enable ops for.

Regards,
Jonas

>  	.get_rate	= rk3288_clk_get_rate,
>  	.set_rate	= rk3288_clk_set_rate,
>  #if CONFIG_IS_ENABLED(OF_REAL)

Re: [PATCH] clk: rk3288: add stub for ops->enable

[Marius Dinu]

On Mon, 2026-03-16 11.10.38 ++0100, Jonas Karlman wrote:
> Hi Marius,
> 
> On 3/16/2026 10:57 AM, Marius Dinu wrote:
> > This fixes a bug where the watchdog can't enable the clock.
> > AFAIK, this clock is always enabled.
> > 
> > Tested on Asus TinkerBoard. Test results:
> > wdt dev         => works
> > wdt start 200   => works, but timeout is 43s
> > wdt start 10000 => works, but timeout is 1m27s
> > wdt reset       => works
> > wdt stop        => doesn't work
> 
> I have a different patch incoming that should fix this issue at watchdog
> driver level, something like following should make this work and
> properly handle platforms not implementing clk_enable ops.
> 
> diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c
> index bd9d7105366..91228de5e8e 100644
> --- a/drivers/watchdog/designware_wdt.c
> +++ b/drivers/watchdog/designware_wdt.c
> @@ -122,7 +122,7 @@ static int designware_wdt_probe(struct udevice *dev)
>                 return ret;
> 
>  	ret = clk_enable(&clk);
> -	if (ret)
> +	if (ret && ret != -ENOSYS)
>  		return ret;
> 
>  	priv->clk_khz = clk_get_rate(&clk) / 1000;
> 

OK. I'm moving on from this arch and I just wanted to send all remaining
fixes that I use.

About your patch: aren't there commands or drivers other than wdt that may
try to enable the clock and fail?

> > 
> > Signed-off-by: Marius Dinu <m95d+git@psihoexpert.ro>
> > ---
> >  drivers/clk/rockchip/clk_rk3288.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/clk/rockchip/clk_rk3288.c b/drivers/clk/rockchip/clk_rk3288.c
> > index a4ff1c41abb..9cc883662ff 100644
> > --- a/drivers/clk/rockchip/clk_rk3288.c
> > +++ b/drivers/clk/rockchip/clk_rk3288.c
> > @@ -745,6 +745,10 @@ static ulong rockchip_saradc_set_clk(struct rockchip_cru *cru, uint hz)
> >  	return rockchip_saradc_get_clk(cru);
> >  }
> >  
> > +static int rk3288_clk_enable(struct clk *clk){
> > +	return 0;
> 
> This should properly be implemented for the clocks you need, and return
> -ENOSYS for the clocks not implemented.
> 
> > +}
> > +
> >  static ulong rk3288_clk_get_rate(struct clk *clk)
> >  {
> >  	struct rk3288_clk_priv *priv = dev_get_priv(clk->dev);
> > @@ -947,6 +951,7 @@ static int __maybe_unused rk3288_clk_set_parent(struct clk *clk, struct clk *par
> >  }
> >  
> >  static struct clk_ops rk3288_clk_ops = {
> > +	.enable		= rk3288_clk_enable,
> 
> Also include proper disable ops for the clocks you add enable ops for.
> 
> Regards,
> Jonas
> 
> >  	.get_rate	= rk3288_clk_get_rate,
> >  	.set_rate	= rk3288_clk_set_rate,
> >  #if CONFIG_IS_ENABLED(OF_REAL)
> 

Right! That's probably why "wdt stop" doesn't work.

Marius

Re: [PATCH] clk: rk3288: add stub for ops->enable

[Marius Dinu]

On Mon, 2026-03-16 11.10.38 ++0100, Jonas Karlman wrote:
> Hi Marius,
> 
> On 3/16/2026 10:57 AM, Marius Dinu wrote:
> > This fixes a bug where the watchdog can't enable the clock.
> > AFAIK, this clock is always enabled.
> > 
> > Tested on Asus TinkerBoard. Test results:
> > wdt dev         => works
> > wdt start 200   => works, but timeout is 43s
> > wdt start 10000 => works, but timeout is 1m27s
> > wdt reset       => works
> > wdt stop        => doesn't work
> 
> I have a different patch incoming that should fix this issue at watchdog
> driver level, something like following should make this work and
> properly handle platforms not implementing clk_enable ops.
> 
> diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c
> index bd9d7105366..91228de5e8e 100644
> --- a/drivers/watchdog/designware_wdt.c
> +++ b/drivers/watchdog/designware_wdt.c
> @@ -122,7 +122,7 @@ static int designware_wdt_probe(struct udevice *dev)
>                 return ret;
> 
>  	ret = clk_enable(&clk);
> -	if (ret)
> +	if (ret && ret != -ENOSYS)
>  		return ret;
> 
>  	priv->clk_khz = clk_get_rate(&clk) / 1000;
> 

OK. I'm moving on from this arch and I just wanted to send all remaining
fixes that I use.

About your patch: aren't there commands or drivers other than wdt that may
try to enable the clock and fail?

> > 
> > Signed-off-by: Marius Dinu <m95d+git@psihoexpert.ro>
> > ---
> >  drivers/clk/rockchip/clk_rk3288.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/clk/rockchip/clk_rk3288.c b/drivers/clk/rockchip/clk_rk3288.c
> > index a4ff1c41abb..9cc883662ff 100644
> > --- a/drivers/clk/rockchip/clk_rk3288.c
> > +++ b/drivers/clk/rockchip/clk_rk3288.c
> > @@ -745,6 +745,10 @@ static ulong rockchip_saradc_set_clk(struct rockchip_cru *cru, uint hz)
> >  	return rockchip_saradc_get_clk(cru);
> >  }
> >  
> > +static int rk3288_clk_enable(struct clk *clk){
> > +	return 0;
> 
> This should properly be implemented for the clocks you need, and return
> -ENOSYS for the clocks not implemented.
> 
> > +}
> > +
> >  static ulong rk3288_clk_get_rate(struct clk *clk)
> >  {
> >  	struct rk3288_clk_priv *priv = dev_get_priv(clk->dev);
> > @@ -947,6 +951,7 @@ static int __maybe_unused rk3288_clk_set_parent(struct clk *clk, struct clk *par
> >  }
> >  
> >  static struct clk_ops rk3288_clk_ops = {
> > +	.enable		= rk3288_clk_enable,
> 
> Also include proper disable ops for the clocks you add enable ops for.
> 
> Regards,
> Jonas
> 
> >  	.get_rate	= rk3288_clk_get_rate,
> >  	.set_rate	= rk3288_clk_set_rate,
> >  #if CONFIG_IS_ENABLED(OF_REAL)
> 

Right! That's probably why "wdt stop" doesn't work.

Marius

PS: There is a duplicate of this email sent from the wrong address. Sorry.