From: "李佑鸿 " <dayou5941@163.com>
To: "Biju Das" <biju.das.jz@bp.renesas.com>
Cc: "wbg@kernel.org" <wbg@kernel.org>,
"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>,
"Li Youhong" <liyouhong@kylinos.cn>
Subject: Re:RE: [PATCH v2 2/3] counter: rz-mtu3-cnt: check pm_runtime_get_sync() return values
Date: Thu, 3 Sep 2026 15:06:29 +0800 (CST) [thread overview]
Message-ID: <25c84af9.59e7.1a066172a8e.Coremail.dayou5941@163.com> (raw)
In-Reply-To: <TY3PR01MB1134649222A8DFA98891DCF1086B62@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Hi Biju,
At 2026-09-03 14:53:24, "Biju Das" <biju.das.jz@bp.renesas.com> wrote:
>Hi Li Youhong,
>
>> -----Original Message-----
>> From: Li Youhong <dayou5941@163.com>
>> Sent: 31 August 2026 06:44
>> Subject: [PATCH v2 2/3] counter: rz-mtu3-cnt: check pm_runtime_get_sync() return values
>>
>> From: Li Youhong <liyouhong@kylinos.cn>
>>
>> The driver ignored pm_runtime_get_sync() return values. When runtime resume fails, pm_runtime_get_sync()
>> returns a negative error code, but the driver continued with register accesses anyway. Check all call
>> sites and propagate the error to the caller.
>>
>> Fixes: 0be8907359df ("counter: Add Renesas RZ/G2L MTU3a counter driver")
>> Signed-off-by: Li Youhong <liyouhong@kylinos.cn>
>> ---
>> drivers/counter/rz-mtu3-cnt.c | 98 ++++++++++++++++++++++++++++-------
>> 1 file changed, 80 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c index
>> 3654e0fdab20..17912491e522 100644
>> --- a/drivers/counter/rz-mtu3-cnt.c
>> +++ b/drivers/counter/rz-mtu3-cnt.c
>> @@ -102,22 +102,27 @@ static inline struct rz_mtu3_channel *rz_mtu3_get_ch(struct counter_device *coun
>> return &priv->ch[ch_id];
>> }
>>
>> -static bool rz_mtu3_is_counter_invalid(struct counter_device *counter, int id)
>> +static int rz_mtu3_validate_counter(struct counter_device *counter, int
>> +id)
>> {
>> struct rz_mtu3_cnt *const priv = counter_priv(counter);
>> unsigned long tmdr;
>> + int ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>
>There is a better API for error check, pm_runtime_resume_and_get()
>Which will avoid code duplication below.
>
>Can you please switch to this API?
>
>Cheers,
>Biju
Thanks for the suggestion. Will switch to pm_runtime_resume_and_get ()
and send a v3.
Regards,
Li Youhong
>
>
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + return ret;
>> + }
>> tmdr = rz_mtu3_shared_reg_read(priv->ch, RZ_MTU3_TMDR3);
>> pm_runtime_put(counter->parent);
>>
>> if (id == RZ_MTU3_32_BIT_CH && test_bit(RZ_MTU3_TMDR3_LWA, &tmdr))
>> - return false;
>> + return 0;
>>
>> if (id != RZ_MTU3_32_BIT_CH && !test_bit(RZ_MTU3_TMDR3_LWA, &tmdr))
>> - return false;
>> + return 0;
>>
>> - return true;
>> + return -EBUSY;
>> }
>>
>> static int rz_mtu3_lock_if_counter_is_valid(struct counter_device *counter, @@ -125,6 +130,8 @@ static
>> int rz_mtu3_lock_if_counter_is_valid(struct counter_device *counter,
>> struct rz_mtu3_cnt *const priv,
>> int id)
>> {
>> + int ret;
>> +
>> mutex_lock(&priv->lock);
>>
>> if (ch->is_busy && !priv->count_is_enabled[id]) { @@ -132,9 +139,10 @@ static int
>> rz_mtu3_lock_if_counter_is_valid(struct counter_device *counter,
>> return -EINVAL;
>> }
>>
>> - if (rz_mtu3_is_counter_invalid(counter, id)) {
>> + ret = rz_mtu3_validate_counter(counter, id);
>> + if (ret) {
>> mutex_unlock(&priv->lock);
>> - return -EBUSY;
>> + return ret;
>> }
>>
>> return 0;
>> @@ -165,7 +173,12 @@ static int rz_mtu3_count_read(struct counter_device *counter,
>> if (ret)
>> return ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> if (count->id == RZ_MTU3_32_BIT_CH)
>> *val = rz_mtu3_32bit_ch_read(ch, RZ_MTU3_TCNTLW);
>> else
>> @@ -187,7 +200,12 @@ static int rz_mtu3_count_write(struct counter_device *counter,
>> if (ret)
>> return ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> if (count->id == RZ_MTU3_32_BIT_CH)
>> rz_mtu3_32bit_ch_write(ch, RZ_MTU3_TCNTLW, val);
>> else
>> @@ -203,8 +221,13 @@ static int rz_mtu3_count_function_read_helper(struct rz_mtu3_channel *const ch,
>> enum counter_function *function) {
>> u8 timer_mode;
>> + int ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + return ret;
>> + }
>> timer_mode = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TMDR1);
>> pm_runtime_put(counter->parent);
>>
>> @@ -279,7 +302,12 @@ static int rz_mtu3_count_function_write(struct counter_device *counter,
>> return -EINVAL;
>> }
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, timer_mode);
>> pm_runtime_put(counter->parent);
>> mutex_unlock(&priv->lock);
>> @@ -300,7 +328,12 @@ static int rz_mtu3_count_direction_read(struct counter_device *counter,
>> if (ret)
>> return ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> tsr = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TSR);
>> pm_runtime_put(counter->parent);
>>
>> @@ -377,7 +410,12 @@ static int rz_mtu3_count_ceiling_write(struct counter_device *counter,
>> return -EINVAL;
>> }
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> if (count->id == RZ_MTU3_32_BIT_CH)
>> rz_mtu3_32bit_ch_write(ch, RZ_MTU3_TGRALW, ceiling);
>> else
>> @@ -504,7 +542,11 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter,
>> goto exit;
>>
>> if (enable) {
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + goto exit;
>> + }
>> ret = rz_mtu3_initialize_counter(counter, count->id);
>> if (ret == 0)
>> priv->count_is_enabled[count->id] = true; @@ -543,7 +585,12 @@ static int
>> rz_mtu3_cascade_counts_enable_get(struct counter_device *counter,
>> if (ret)
>> return ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> tmdr = rz_mtu3_shared_reg_read(priv->ch, RZ_MTU3_TMDR3);
>> pm_runtime_put(counter->parent);
>> *cascade_enable = test_bit(RZ_MTU3_TMDR3_LWA, &tmdr); @@ -562,7 +609,12 @@ static int
>> rz_mtu3_cascade_counts_enable_set(struct counter_device *counter,
>> if (ret)
>> return ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> rz_mtu3_shared_reg_update_bit(priv->ch, RZ_MTU3_TMDR3,
>> RZ_MTU3_TMDR3_LWA, cascade_enable);
>> pm_runtime_put(counter->parent);
>> @@ -582,7 +634,12 @@ static int rz_mtu3_ext_input_phase_clock_select_get(struct counter_device *count
>> if (ret)
>> return ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> tmdr = rz_mtu3_shared_reg_read(priv->ch, RZ_MTU3_TMDR3);
>> pm_runtime_put(counter->parent);
>> *ext_input_phase_clock_select = test_bit(RZ_MTU3_TMDR3_PHCKSEL, &tmdr); @@ -601,7 +658,12 @@
>> static int rz_mtu3_ext_input_phase_clock_select_set(struct counter_device *count
>> if (ret)
>> return ret;
>>
>> - pm_runtime_get_sync(counter->parent);
>> + ret = pm_runtime_get_sync(counter->parent);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(counter->parent);
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> + }
>> rz_mtu3_shared_reg_update_bit(priv->ch, RZ_MTU3_TMDR3,
>> RZ_MTU3_TMDR3_PHCKSEL,
>> ext_input_phase_clock_select);
>> --
>> 2.25.1
next prev parent reply other threads:[~2026-09-03 7:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-31 5:44 [PATCH v2 0/3] counter: rz-mtu3-cnt: check clock and runtime PM errors Li Youhong
2026-08-31 5:44 ` [PATCH v2 1/3] counter: rz-mtu3-cnt: check clk_prepare_enable() and pm_runtime_set_active() Li Youhong
2026-08-31 5:44 ` [PATCH v2 2/3] counter: rz-mtu3-cnt: check pm_runtime_get_sync() return values Li Youhong
2026-09-03 6:53 ` Biju Das
2026-09-03 7:06 ` 李佑鸿 [this message]
2026-08-31 5:44 ` [PATCH v2 3/3] counter: rz-mtu3-cnt: put runtime PM ref if counter init fails Li Youhong
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=25c84af9.59e7.1a066172a8e.Coremail.dayou5941@163.com \
--to=dayou5941@163.com \
--cc=biju.das.jz@bp.renesas.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=liyouhong@kylinos.cn \
--cc=wbg@kernel.org \
/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