| Message ID | 20230717120738.165765-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | counter: rz-mtu3-cnt: Fix locking order | expand |
On Mon, Jul 17, 2023 at 01:07:38PM +0100, Biju Das wrote: > All functions except rz_mtu3_count_enable_write(), calls > pm_runtime_{get,put} inside the lock. For consistency do the same here. > > Reported-by: Pavel Machek <pavel@denx.de> > Closes: https://patchwork.kernel.org/project/cip-dev/patch/20230606075235.183132-6-biju.das.jz@bp.renesas.com/ > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Is this just a code cleanup for consistency with the rest of the driver, or does it fix an existing bug? If it's just cleanup, would you resend with a different title (e.g. "Reorder locking sequence for consistency") to make it clear this is not addressing a bug. Thanks, William Breathitt Gray > --- > drivers/counter/rz-mtu3-cnt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c > index 48c83933aa2f..ee821493b166 100644 > --- a/drivers/counter/rz-mtu3-cnt.c > +++ b/drivers/counter/rz-mtu3-cnt.c > @@ -500,8 +500,8 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter, > int ret = 0; > > if (enable) { > - pm_runtime_get_sync(ch->dev); > mutex_lock(&priv->lock); > + pm_runtime_get_sync(ch->dev); > ret = rz_mtu3_initialize_counter(counter, count->id); > if (ret == 0) > priv->count_is_enabled[count->id] = true; > @@ -510,8 +510,8 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter, > mutex_lock(&priv->lock); > rz_mtu3_terminate_counter(counter, count->id); > priv->count_is_enabled[count->id] = false; > - mutex_unlock(&priv->lock); > pm_runtime_put(ch->dev); > + mutex_unlock(&priv->lock); > } > > return ret; > -- > 2.25.1 >
Hi William Breathitt Gray, Thanks for the feedback. > Subject: Re: [PATCH] counter: rz-mtu3-cnt: Fix locking order > > On Mon, Jul 17, 2023 at 01:07:38PM +0100, Biju Das wrote: > > All functions except rz_mtu3_count_enable_write(), calls > > pm_runtime_{get,put} inside the lock. For consistency do the same > here. > > > > Reported-by: Pavel Machek <pavel@denx.de> > > Closes: > > https://patchwork.kernel.org/project/cip-dev/patch/20230606075235.1831 > > 32-6-biju.das.jz@bp.renesas.com/ > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Is this just a code cleanup for consistency with the rest of the driver, > or does it fix an existing bug? If it's just cleanup, would you resend > with a different title (e.g. "Reorder locking sequence for consistency") > to make it clear this is not addressing a bug. It is just cleanup. Will send v2 with title as you suggested. Cheers, Biju > > Thanks, > > William Breathitt Gray > > > --- > > drivers/counter/rz-mtu3-cnt.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/counter/rz-mtu3-cnt.c > > b/drivers/counter/rz-mtu3-cnt.c index 48c83933aa2f..ee821493b166 > > 100644 > > --- a/drivers/counter/rz-mtu3-cnt.c > > +++ b/drivers/counter/rz-mtu3-cnt.c > > @@ -500,8 +500,8 @@ static int rz_mtu3_count_enable_write(struct > counter_device *counter, > > int ret = 0; > > > > if (enable) { > > - pm_runtime_get_sync(ch->dev); > > mutex_lock(&priv->lock); > > + pm_runtime_get_sync(ch->dev); > > ret = rz_mtu3_initialize_counter(counter, count->id); > > if (ret == 0) > > priv->count_is_enabled[count->id] = true; @@ -510,8 > +510,8 @@ > > static int rz_mtu3_count_enable_write(struct counter_device *counter, > > mutex_lock(&priv->lock); > > rz_mtu3_terminate_counter(counter, count->id); > > priv->count_is_enabled[count->id] = false; > > - mutex_unlock(&priv->lock); > > pm_runtime_put(ch->dev); > > + mutex_unlock(&priv->lock); > > } > > > > return ret; > > -- > > 2.25.1 > >
diff --git a/drivers/counter/rz-mtu3-cnt.c b/drivers/counter/rz-mtu3-cnt.c index 48c83933aa2f..ee821493b166 100644 --- a/drivers/counter/rz-mtu3-cnt.c +++ b/drivers/counter/rz-mtu3-cnt.c @@ -500,8 +500,8 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter, int ret = 0; if (enable) { - pm_runtime_get_sync(ch->dev); mutex_lock(&priv->lock); + pm_runtime_get_sync(ch->dev); ret = rz_mtu3_initialize_counter(counter, count->id); if (ret == 0) priv->count_is_enabled[count->id] = true; @@ -510,8 +510,8 @@ static int rz_mtu3_count_enable_write(struct counter_device *counter, mutex_lock(&priv->lock); rz_mtu3_terminate_counter(counter, count->id); priv->count_is_enabled[count->id] = false; - mutex_unlock(&priv->lock); pm_runtime_put(ch->dev); + mutex_unlock(&priv->lock); } return ret;
All functions except rz_mtu3_count_enable_write(), calls pm_runtime_{get,put} inside the lock. For consistency do the same here. Reported-by: Pavel Machek <pavel@denx.de> Closes: https://patchwork.kernel.org/project/cip-dev/patch/20230606075235.183132-6-biju.das.jz@bp.renesas.com/ Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/counter/rz-mtu3-cnt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)