| Message ID | 20240202-fix_llcc_update_act_ctrl-v1-1-d36df95c8bd5@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | soc: qcom: llcc: Check return value on Broadcast_OR reg read | expand |
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> >
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> >>
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> > >
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> >>>
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 --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,
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,