| Message ID | 20230717120333.165219-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | mfd: rz-mtu3: Improve critical sections | expand |
Hi! > Improve critical sections on rz_mtu3_start_stop_ch() and > rz_mtu3_is_enabled() by moving offset and bitpos computation > outside the critical section and drop the 'ret' variable on > rz_mtu3_is_enabled() and return 'tstr & BIT(bitpos)' directly. Thanks for the patch. Reviewed-by: Pavel Machek <pavel@denx.de> This is an improvement, but we'll need to move away from raw_spin_lock code. I asked tglx, raw_spin_lock is not okay to use without very clear reason. Best regards, Pavel
Hi Pavel, Thanks for the feedback. > -----Original Message----- > From: Pavel Machek <pavel@denx.de> > Sent: Wednesday, July 19, 2023 11:19 AM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Lee Jones <lee@kernel.org>; Geert Uytterhoeven > <geert+renesas@glider.be>; Pavel Machek <pavel@denx.de>; Prabhakar > Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; linux-renesas- > soc@vger.kernel.org > Subject: Re: [PATCH] mfd: rz-mtu3: Improve critical sections > > Hi! > > > Improve critical sections on rz_mtu3_start_stop_ch() and > > rz_mtu3_is_enabled() by moving offset and bitpos computation outside > > the critical section and drop the 'ret' variable on > > rz_mtu3_is_enabled() and return 'tstr & BIT(bitpos)' directly. > > Thanks for the patch. > > Reviewed-by: Pavel Machek <pavel@denx.de> > > This is an improvement, but we'll need to move away from raw_spin_lock > code. I asked tglx, raw_spin_lock is not okay to use without very clear > reason. OK, will replace raw_spin_lock->spin_lock(). Cheers, Biju
diff --git a/drivers/mfd/rz-mtu3.c b/drivers/mfd/rz-mtu3.c index 037956f0254b..e5cace963c7c 100644 --- a/drivers/mfd/rz-mtu3.c +++ b/drivers/mfd/rz-mtu3.c @@ -251,11 +251,12 @@ static void rz_mtu3_start_stop_ch(struct rz_mtu3_channel *ch, bool start) u16 offset; u8 bitpos; + offset = rz_mtu3_get_tstr_offset(ch); + bitpos = rz_mtu3_get_tstr_bit_pos(ch); + /* start stop register shared by multiple timer channels */ raw_spin_lock_irqsave(&priv->lock, flags); - offset = rz_mtu3_get_tstr_offset(ch); - bitpos = rz_mtu3_get_tstr_bit_pos(ch); tstr = rz_mtu3_shared_reg_read(ch, offset); __assign_bit(bitpos, &tstr, start); rz_mtu3_shared_reg_write(ch, offset, tstr); @@ -268,21 +269,18 @@ bool rz_mtu3_is_enabled(struct rz_mtu3_channel *ch) struct rz_mtu3 *mtu = dev_get_drvdata(ch->dev->parent); struct rz_mtu3_priv *priv = mtu->priv_data; unsigned long flags, tstr; - bool ret = false; u16 offset; u8 bitpos; - /* start stop register shared by multiple timer channels */ - raw_spin_lock_irqsave(&priv->lock, flags); - offset = rz_mtu3_get_tstr_offset(ch); bitpos = rz_mtu3_get_tstr_bit_pos(ch); - tstr = rz_mtu3_shared_reg_read(ch, offset); - ret = tstr & BIT(bitpos); + /* start stop register shared by multiple timer channels */ + raw_spin_lock_irqsave(&priv->lock, flags); + tstr = rz_mtu3_shared_reg_read(ch, offset); raw_spin_unlock_irqrestore(&priv->lock, flags); - return ret; + return tstr & BIT(bitpos); } EXPORT_SYMBOL_GPL(rz_mtu3_is_enabled);
Improve critical sections on rz_mtu3_start_stop_ch() and rz_mtu3_is_enabled() by moving offset and bitpos computation outside the critical section and drop the 'ret' variable on rz_mtu3_is_enabled() and return 'tstr & BIT(bitpos)' directly. Reported-by: Pavel Machek <pavel@denx.de> Closes: https://patchwork.kernel.org/project/cip-dev/patch/20230606075235.183132-4-biju.das.jz@bp.renesas.com/ Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/mfd/rz-mtu3.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)