2024-02-02 19:48:32

by Unnathi Chalicheemala

Subject: [PATCH] soc: qcom: llcc: Check return value on Broadcast_OR reg read

Commit a3134fb09e0b ("drivers: soc: Add LLCC driver") didn't
check return value after Broadcast_OR register read in
llcc_update_act_ctrl(), add it.

Signed-off-by: Unnathi Chalicheemala <[email protected]>
---
drivers/soc/qcom/llcc-qcom.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 4ca88eaebf06..cbef0dea1d5d 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -859,6 +859,8 @@ static int llcc_update_act_ctrl(u32 sid,
ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
slice_status, !(slice_status & status),
0, LLCC_STATUS_READ_DELAY);
+ if (ret)
+ return ret;

if (drv_data->version >= LLCC_VERSION_4_1_0_0)
ret = regmap_write(drv_data->bcast_regmap, act_clear_reg,

---
base-commit: 021533194476035883300d60fbb3136426ac8ea5
change-id: 20240202-fix_llcc_update_act_ctrl-64908aed9450

Best regards,
--
Unnathi Chalicheemala <[email protected]>

2024-02-02 20:02:49

by Unnathi Chalicheemala

Subject: Re: [PATCH] soc: qcom: llcc: Check return value on Broadcast_OR reg read

On 2/2/2024 11:56 AM, Elliot Berman wrote:
> On Fri, Feb 02, 2024 at 11:47:43AM -0800, Unnathi Chalicheemala wrote:
>> Commit a3134fb09e0b ("drivers: soc: Add LLCC driver") didn't
>> check return value after Broadcast_OR register read in
>> llcc_update_act_ctrl(), add it.
>>
>
> Reviewed-by: Elliot Berman <[email protected]>
>
> You'll probably want to add:
>
> Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")
>

Ack. Missed it, thanks Elliot!

>> Signed-off-by: Unnathi Chalicheemala <[email protected]>
>> ---
>> drivers/soc/qcom/llcc-qcom.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
>> index 4ca88eaebf06..cbef0dea1d5d 100644
>> --- a/drivers/soc/qcom/llcc-qcom.c
>> +++ b/drivers/soc/qcom/llcc-qcom.c
>> @@ -859,6 +859,8 @@ static int llcc_update_act_ctrl(u32 sid,
>> ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
>> slice_status, !(slice_status & status),
>> 0, LLCC_STATUS_READ_DELAY);
>> + if (ret)
>> + return ret;
>>
>> if (drv_data->version >= LLCC_VERSION_4_1_0_0)
>> ret = regmap_write(drv_data->bcast_regmap, act_clear_reg,
>>
>> ---
>> base-commit: 021533194476035883300d60fbb3136426ac8ea5
>> change-id: 20240202-fix_llcc_update_act_ctrl-64908aed9450
>>
>> Best regards,
>> --
>> Unnathi Chalicheemala <[email protected]>
>>

2024-02-02 20:21:10

by Elliot Berman

Subject: Re: [PATCH] soc: qcom: llcc: Check return value on Broadcast_OR reg read

On Fri, Feb 02, 2024 at 11:47:43AM -0800, Unnathi Chalicheemala wrote:
> Commit a3134fb09e0b ("drivers: soc: Add LLCC driver") didn't
> check return value after Broadcast_OR register read in
> llcc_update_act_ctrl(), add it.
>

Reviewed-by: Elliot Berman <[email protected]>

You'll probably want to add:

Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")

> Signed-off-by: Unnathi Chalicheemala <[email protected]>
> ---
> drivers/soc/qcom/llcc-qcom.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 4ca88eaebf06..cbef0dea1d5d 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -859,6 +859,8 @@ static int llcc_update_act_ctrl(u32 sid,
> ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
> slice_status, !(slice_status & status),
> 0, LLCC_STATUS_READ_DELAY);
> + if (ret)
> + return ret;
>
> if (drv_data->version >= LLCC_VERSION_4_1_0_0)
> ret = regmap_write(drv_data->bcast_regmap, act_clear_reg,
>
> ---
> base-commit: 021533194476035883300d60fbb3136426ac8ea5
> change-id: 20240202-fix_llcc_update_act_ctrl-64908aed9450
>
> Best regards,
> --
> Unnathi Chalicheemala <[email protected]>
>

2024-02-02 23:20:57

by Bjorn Andersson

Subject: Re: Re: [PATCH] soc: qcom: llcc: Check return value on Broadcast_OR reg read

On Fri, Feb 02, 2024 at 11:56:53AM -0800, Elliot Berman wrote:
> On Fri, Feb 02, 2024 at 11:47:43AM -0800, Unnathi Chalicheemala wrote:
> > Commit a3134fb09e0b ("drivers: soc: Add LLCC driver") didn't
> > check return value after Broadcast_OR register read in
> > llcc_update_act_ctrl(), add it.
> >
>
> Reviewed-by: Elliot Berman <[email protected]>
>
> You'll probably want to add:
>
> Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")

No, this was correct in a3134fb09e0b, ret was returned on the following
line. The problem was introduced when the new 4.1 if statement was
introduced without considering that ret might be overwritten.

Fixes: c72ca343f911 ("soc: qcom: llcc: Add v4.1 HW version support")

Regards,
Bjorn

>
> > Signed-off-by: Unnathi Chalicheemala <[email protected]>
> > ---
> > drivers/soc/qcom/llcc-qcom.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> > index 4ca88eaebf06..cbef0dea1d5d 100644
> > --- a/drivers/soc/qcom/llcc-qcom.c
> > +++ b/drivers/soc/qcom/llcc-qcom.c
> > @@ -859,6 +859,8 @@ static int llcc_update_act_ctrl(u32 sid,
> > ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
> > slice_status, !(slice_status & status),
> > 0, LLCC_STATUS_READ_DELAY);
> > + if (ret)
> > + return ret;
> >
> > if (drv_data->version >= LLCC_VERSION_4_1_0_0)
> > ret = regmap_write(drv_data->bcast_regmap, act_clear_reg,
> >
> > ---
> > base-commit: 021533194476035883300d60fbb3136426ac8ea5
> > change-id: 20240202-fix_llcc_update_act_ctrl-64908aed9450
> >
> > Best regards,
> > --
> > Unnathi Chalicheemala <[email protected]>
> >

2024-02-03 00:45:26

by Unnathi Chalicheemala

Subject: Re: [PATCH] soc: qcom: llcc: Check return value on Broadcast_OR reg read

On 2/2/2024 3:20 PM, Bjorn Andersson wrote:
> On Fri, Feb 02, 2024 at 11:56:53AM -0800, Elliot Berman wrote:
>> On Fri, Feb 02, 2024 at 11:47:43AM -0800, Unnathi Chalicheemala wrote:
>>> Commit a3134fb09e0b ("drivers: soc: Add LLCC driver") didn't
>>> check return value after Broadcast_OR register read in
>>> llcc_update_act_ctrl(), add it.
>>>
>>
>> Reviewed-by: Elliot Berman <[email protected]>
>>
>> You'll probably want to add:
>>
>> Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")
>
> No, this was correct in a3134fb09e0b, ret was returned on the following
> line. The problem was introduced when the new 4.1 if statement was
> introduced without considering that ret might be overwritten.
>
> Fixes: c72ca343f911 ("soc: qcom: llcc: Add v4.1 HW version support")
>
> Regards,
> Bjorn
>

Exactly right thank you for taking the time to review.
I will name the right fix in v2 patch.

>>
>>> Signed-off-by: Unnathi Chalicheemala <[email protected]>
>>> ---
>>> drivers/soc/qcom/llcc-qcom.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
>>> index 4ca88eaebf06..cbef0dea1d5d 100644
>>> --- a/drivers/soc/qcom/llcc-qcom.c
>>> +++ b/drivers/soc/qcom/llcc-qcom.c
>>> @@ -859,6 +859,8 @@ static int llcc_update_act_ctrl(u32 sid,
>>> ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
>>> slice_status, !(slice_status & status),
>>> 0, LLCC_STATUS_READ_DELAY);
>>> + if (ret)
>>> + return ret;
>>>
>>> if (drv_data->version >= LLCC_VERSION_4_1_0_0)
>>> ret = regmap_write(drv_data->bcast_regmap, act_clear_reg,
>>>
>>> ---
>>> base-commit: 021533194476035883300d60fbb3136426ac8ea5
>>> change-id: 20240202-fix_llcc_update_act_ctrl-64908aed9450
>>>
>>> Best regards,
>>> --
>>> Unnathi Chalicheemala <[email protected]>
>>>

2024-02-14 18:04:38

by Bjorn Andersson

Subject: Re: [PATCH] soc: qcom: llcc: Check return value on Broadcast_OR reg read

On Fri, 02 Feb 2024 11:47:43 -0800, Unnathi Chalicheemala wrote:
> Commit a3134fb09e0b ("drivers: soc: Add LLCC driver") didn't
> check return value after Broadcast_OR register read in
> llcc_update_act_ctrl(), add it.
>
>

Applied, thanks!

[1/1] soc: qcom: llcc: Check return value on Broadcast_OR reg read
commit: ceeaddc19a90039861564d8e1078b778a8f95101

Best regards,
--
Bjorn Andersson <[email protected]>