| Message ID | 169277f53affed98ef41e5a7cbf2401fe62716bd.1707202761.git.quic_uchalich@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | LLCC: Support for Broadcast_AND region | expand |
On 06/02/2024 08:15, Unnathi Chalicheemala wrote: > Define new regmap structure for Broadcast_AND region and initialize > regmap for Broadcast_AND region when HW block version > is greater than 4.1 for backwards compatibility. > > Switch from broadcast_OR to broadcast_AND region for checking > status bit 1 as Broadcast_OR region checks only for bit 0. > Driver changes cannot be after DTS, because there should be no dependency between hardware description and drivers. Hardware did not change, did it? Your patchset might have wrong ordering or might break bisectability. Best regards, Krzysztof
On 6.02.2024 08:15, Unnathi Chalicheemala wrote: > Define new regmap structure for Broadcast_AND region and initialize > regmap for Broadcast_AND region when HW block version > is greater than 4.1 for backwards compatibility. Are they actually separate regions and not a single contiguous one? > > Switch from broadcast_OR to broadcast_AND region for checking > status bit 1 as Broadcast_OR region checks only for bit 0. > > Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com> > --- > drivers/soc/qcom/llcc-qcom.c | 22 +++++++++++++++++++--- > include/linux/soc/qcom/llcc-qcom.h | 4 +++- > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c > index 4ca88eaebf06..fbd2542cd4c5 100644 > --- a/drivers/soc/qcom/llcc-qcom.c > +++ b/drivers/soc/qcom/llcc-qcom.c > @@ -849,9 +849,14 @@ static int llcc_update_act_ctrl(u32 sid, > return ret; > > if (drv_data->version >= LLCC_VERSION_4_1_0_0) { > - ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg, > - slice_status, (slice_status & ACT_COMPLETE), > - 0, LLCC_STATUS_READ_DELAY); > + if (!drv_data->bcast_and_regmap) > + ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg, > + slice_status, (slice_status & ACT_COMPLETE), > + 0, LLCC_STATUS_READ_DELAY); > + else > + ret = regmap_read_poll_timeout(drv_data->bcast_and_regmap, status_reg, > + slice_status, (slice_status & ACT_COMPLETE), > + 0, LLCC_STATUS_READ_DELAY); struct regmap *regmap = drv_data->bcast_and_regmap ?: bcast_regmap; ? > + if (drv_data->version >= LLCC_VERSION_4_1_0_0) { This check is rather redundant.. If there's no such region in hardware, it won't be described, and as such the _get()s will return some sort of an error. Might as well make it a comment that it's intended for >=v4.1 and definitely leave a comment for the next guy that there's a backwards compatibility quirk involved.. Konrad
On 2/6/2024 10:42 AM, Konrad Dybcio wrote: > On 6.02.2024 08:15, Unnathi Chalicheemala wrote: >> Define new regmap structure for Broadcast_AND region and initialize >> regmap for Broadcast_AND region when HW block version >> is greater than 4.1 for backwards compatibility. > > Are they actually separate regions and not a single contiguous one? > Yes, they are separate regions. >> >> Switch from broadcast_OR to broadcast_AND region for checking >> status bit 1 as Broadcast_OR region checks only for bit 0. >> >> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com> >> --- >> drivers/soc/qcom/llcc-qcom.c | 22 +++++++++++++++++++--- >> include/linux/soc/qcom/llcc-qcom.h | 4 +++- >> 2 files changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c >> index 4ca88eaebf06..fbd2542cd4c5 100644 >> --- a/drivers/soc/qcom/llcc-qcom.c >> +++ b/drivers/soc/qcom/llcc-qcom.c >> @@ -849,9 +849,14 @@ static int llcc_update_act_ctrl(u32 sid, >> return ret; >> >> if (drv_data->version >= LLCC_VERSION_4_1_0_0) { >> - ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg, >> - slice_status, (slice_status & ACT_COMPLETE), >> - 0, LLCC_STATUS_READ_DELAY); >> + if (!drv_data->bcast_and_regmap) >> + ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg, >> + slice_status, (slice_status & ACT_COMPLETE), >> + 0, LLCC_STATUS_READ_DELAY); >> + else >> + ret = regmap_read_poll_timeout(drv_data->bcast_and_regmap, status_reg, >> + slice_status, (slice_status & ACT_COMPLETE), >> + 0, LLCC_STATUS_READ_DELAY); > > struct regmap *regmap = drv_data->bcast_and_regmap ?: bcast_regmap; > > ? Ack. Will minimize the redundancy. > >> + if (drv_data->version >= LLCC_VERSION_4_1_0_0) { > > This check is rather redundant.. If there's no such region in hardware, > it won't be described, and as such the _get()s will return some sort > of an error. > I see what you're saying. > Might as well make it a comment that it's intended for >=v4.1 and > definitely leave a comment for the next guy that there's a backwards > compatibility quirk involved.. Ack. Thanks for the review Konrad! > > Konrad >
diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c index 4ca88eaebf06..fbd2542cd4c5 100644 --- a/drivers/soc/qcom/llcc-qcom.c +++ b/drivers/soc/qcom/llcc-qcom.c @@ -849,9 +849,14 @@ static int llcc_update_act_ctrl(u32 sid, return ret; if (drv_data->version >= LLCC_VERSION_4_1_0_0) { - ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg, - slice_status, (slice_status & ACT_COMPLETE), - 0, LLCC_STATUS_READ_DELAY); + if (!drv_data->bcast_and_regmap) + ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg, + slice_status, (slice_status & ACT_COMPLETE), + 0, LLCC_STATUS_READ_DELAY); + else + ret = regmap_read_poll_timeout(drv_data->bcast_and_regmap, status_reg, + slice_status, (slice_status & ACT_COMPLETE), + 0, LLCC_STATUS_READ_DELAY); if (ret) return ret; } @@ -1282,6 +1287,17 @@ static int qcom_llcc_probe(struct platform_device *pdev) drv_data->version = version; + if (drv_data->version >= LLCC_VERSION_4_1_0_0) { + drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base"); + if (IS_ERR(drv_data->bcast_and_regmap)) { + ret = PTR_ERR(drv_data->bcast_and_regmap); + if (ret == -EINVAL) + drv_data->bcast_and_regmap = NULL; + else + goto err; + } + } + llcc_cfg = cfg->sct_data; sz = cfg->size; diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h index 1a886666bbb6..9e9f528b1370 100644 --- a/include/linux/soc/qcom/llcc-qcom.h +++ b/include/linux/soc/qcom/llcc-qcom.h @@ -115,7 +115,8 @@ struct llcc_edac_reg_offset { /** * struct llcc_drv_data - Data associated with the llcc driver * @regmaps: regmaps associated with the llcc device - * @bcast_regmap: regmap associated with llcc broadcast offset + * @bcast_regmap: regmap associated with llcc broadcast OR offset + * @bcast_and_regmap: regmap associated with llcc broadcast AND offset * @cfg: pointer to the data structure for slice configuration * @edac_reg_offset: Offset of the LLCC EDAC registers * @lock: mutex associated with each slice @@ -129,6 +130,7 @@ struct llcc_edac_reg_offset { struct llcc_drv_data { struct regmap **regmaps; struct regmap *bcast_regmap; + struct regmap *bcast_and_regmap; const struct llcc_slice_config *cfg; const struct llcc_edac_reg_offset *edac_reg_offset; struct mutex lock;
Define new regmap structure for Broadcast_AND region and initialize regmap for Broadcast_AND region when HW block version is greater than 4.1 for backwards compatibility. Switch from broadcast_OR to broadcast_AND region for checking status bit 1 as Broadcast_OR region checks only for bit 0. Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com> --- drivers/soc/qcom/llcc-qcom.c | 22 +++++++++++++++++++--- include/linux/soc/qcom/llcc-qcom.h | 4 +++- 2 files changed, 22 insertions(+), 4 deletions(-)