diff mbox series

soc: qcom: llcc: Check return value on Broadcast_OR reg read

Message ID 20240202-fix_llcc_update_act_ctrl-v1-1-d36df95c8bd5@quicinc.com
State New
Headers
Series soc: qcom: llcc: Check return value on Broadcast_OR reg read |

Commit Message

Unnathi Chalicheemala Feb. 2, 2024, 7:47 p.m. UTC 
  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 <quic_uchalich@quicinc.com>
---
 drivers/soc/qcom/llcc-qcom.c | 2 ++
 1 file changed, 2 insertions(+)


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

Best regards,

Comments

Elliot Berman Feb. 2, 2024, 7:56 p.m. UTC | #1 
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 <quic_eberman@quicinc.com>

You'll probably want to add:

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

> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
> ---
>  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 <quic_uchalich@quicinc.com>
>
Unnathi Chalicheemala Feb. 2, 2024, 7:59 p.m. UTC | #2 
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 <quic_eberman@quicinc.com>
> 
> You'll probably want to add:
> 
> Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")
> 

Ack. Missed it, thanks Elliot!

>> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
>> ---
>>  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 <quic_uchalich@quicinc.com>
>>
Bjorn Andersson Feb. 2, 2024, 11:20 p.m. UTC | #3 
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 <quic_eberman@quicinc.com>
> 
> 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 <quic_uchalich@quicinc.com>
> > ---
> >  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 <quic_uchalich@quicinc.com>
> >
Unnathi Chalicheemala Feb. 3, 2024, 12:45 a.m. UTC | #4 
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 <quic_eberman@quicinc.com>
>>
>> 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 <quic_uchalich@quicinc.com>
>>> ---
>>>  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 <quic_uchalich@quicinc.com>
>>>
Bjorn Andersson Feb. 14, 2024, 5:57 p.m. UTC | #5 
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,
diff mbox series

Patch

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,