public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [RFC] clocksource/drivers/sh_cmt: Always leave device running after probe
Date: Mon, 13 Oct 2025 09:32:09 +0200	[thread overview]
Message-ID: <20251013073209.GA3585900@ragnatech.se> (raw)
In-Reply-To: <aOyoqfOQKRh81daL@shikoro>

Hi Wolfram,

On 2025-10-13 09:22:17 +0200, Wolfram Sang wrote:
> Hi Niklas,
> 
> thanks for your work on these timers!
> 
> > For non-PREEMPT_RT builds this is not really an issue, but for
> > PREEMPT_RT builds where normal spinlocks can sleep this might be an
> > issue. Be cautious and always leave the power and clock running after
> > probe.
> 
> I am not a super-duper PM expert but this sounds pretty reasonable to
> me. Only minor comments:
> 
> >  		ret = -ETIMEDOUT;
> > -		goto err1;
> > +		return ret;
> 
> return -ETIMEDOUT;

Wops, indeed. Thanks for spotting it. Will wait for feedback on the 
general idea and fix this in v2.

> 
> >  	if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) {
> > -		pm_runtime_get_sync(&ch->cmt->pdev->dev);
> >  		ret = sh_cmt_enable(ch);
> >  	}
> 
> Curly braces can go.
> 
> >  	if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) {
> >  		sh_cmt_disable(ch);
> > -		pm_runtime_put(&ch->cmt->pdev->dev);
> >  	}
> 
> ditto.
> 
> And I don't if this is a seperate patch or not, but we could simplify
> probe a little by using 'clk_prepare_enable()'...

This is a good idea, I would prefer to it in a separate patch. I'm 
always a bit scared touching this driver and making finding issues with 
bisect easier is always a good idea :-)

> 
> Happy hacking,
> 
>    Wolfram
> 



-- 
Kind Regards,
Niklas Söderlund

  reply	other threads:[~2025-10-13  7:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-12  9:10 [RFC] clocksource/drivers/sh_cmt: Always leave device running after probe Niklas Söderlund
2025-10-13  7:22 ` Wolfram Sang
2025-10-13  7:32   ` Niklas Söderlund [this message]
2025-10-15 12:43 ` Geert Uytterhoeven
2025-10-15 12:53   ` Niklas Söderlund

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251013073209.GA3585900@ragnatech.se \
    --to=niklas.soderlund+renesas@ragnatech.se \
    --cc=daniel.lezcano@linaro.org \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox