Define new regmap structure for Broadcast_AND region and initialize
this regmap when HW block version is greater than 4.1, otherwise
initialize as a NULL pointer for backwards compatibility.
Switch from broadcast_OR to broadcast_AND region (when defined in DT)
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 | 14 +++++++++++++-
include/linux/soc/qcom/llcc-qcom.h | 4 +++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index cbef0dea1d5d..727d00c527f1 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -821,6 +821,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_putd);
static int llcc_update_act_ctrl(u32 sid,
u32 act_ctrl_reg_val, u32 status)
{
+ struct regmap *regmap;
u32 act_ctrl_reg;
u32 act_clear_reg;
u32 status_reg;
@@ -849,7 +850,8 @@ 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,
+ regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
+ ret = regmap_read_poll_timeout(regmap, status_reg,
slice_status, (slice_status & ACT_COMPLETE),
0, LLCC_STATUS_READ_DELAY);
if (ret)
@@ -1284,6 +1286,16 @@ static int qcom_llcc_probe(struct platform_device *pdev)
drv_data->version = version;
+ /* Applicable only when drv_data->version >= 4.1 */
+ 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;
--
2.25.1On 29/03/2024 22:53, Unnathi Chalicheemala wrote:
> Define new regmap structure for Broadcast_AND region and initialize
> this regmap when HW block version is greater than 4.1, otherwise
> initialize as a NULL pointer for backwards compatibility.
>
> + struct regmap *regmap;
> u32 act_ctrl_reg;
> u32 act_clear_reg;
> u32 status_reg;
> @@ -849,7 +850,8 @@ 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,
> + regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
> + ret = regmap_read_poll_timeout(regmap, status_reg,
> slice_status, (slice_status & ACT_COMPLETE),
> 0, LLCC_STATUS_READ_DELAY);
> if (ret)
> @@ -1284,6 +1286,16 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>
> drv_data->version = version;
>
> + /* Applicable only when drv_data->version >= 4.1 */
> + drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
> + if (IS_ERR(drv_data->bcast_and_regmap)) {
I am pretty sure this breaks all users. Can you please explain how do
you maintain ABI and that IS_ERR() is applied only for version >= 4.1?
Best regards,
Krzysztof
On 3/30/2024 4:46 AM, Krzysztof Kozlowski wrote:
> On 29/03/2024 22:53, Unnathi Chalicheemala wrote:
>> Define new regmap structure for Broadcast_AND region and initialize
>> this regmap when HW block version is greater than 4.1, otherwise
>> initialize as a NULL pointer for backwards compatibility.
>>
>
>> + struct regmap *regmap;
>> u32 act_ctrl_reg;
>> u32 act_clear_reg;
>> u32 status_reg;
>> @@ -849,7 +850,8 @@ 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,
>> + regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
>> + ret = regmap_read_poll_timeout(regmap, status_reg,
>> slice_status, (slice_status & ACT_COMPLETE),
>> 0, LLCC_STATUS_READ_DELAY);
>> if (ret)
>> @@ -1284,6 +1286,16 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>>
>> drv_data->version = version;
>>
>> + /* Applicable only when drv_data->version >= 4.1 */
>> + drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
>> + if (IS_ERR(drv_data->bcast_and_regmap)) {
>
> I am pretty sure this breaks all users. Can you please explain how do
> you maintain ABI and that IS_ERR() is applied only for version >= 4.1?
>
> Best regards,
> Krzysztof
>
IS_ERR() check is done for all versions.
If new register isn't defined in DT(for version < 4.1) it simply sets bcast_and_regmap to NULL.
Otherwise, for version >= 4.1, it goes to err(in the case bcast_and_regmap isn't set properly).
Thank you for reviewing Krzysztof!
On 4/2/24 21:34, Unnathi Chalicheemala wrote:
> On 3/30/2024 4:46 AM, Krzysztof Kozlowski wrote:
>> On 29/03/2024 22:53, Unnathi Chalicheemala wrote:
>>> Define new regmap structure for Broadcast_AND region and initialize
>>> this regmap when HW block version is greater than 4.1, otherwise
>>> initialize as a NULL pointer for backwards compatibility.
>>>
>>
>>> + struct regmap *regmap;
>>> u32 act_ctrl_reg;
>>> u32 act_clear_reg;
>>> u32 status_reg;
>>> @@ -849,7 +850,8 @@ 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,
>>> + regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
>>> + ret = regmap_read_poll_timeout(regmap, status_reg,
>>> slice_status, (slice_status & ACT_COMPLETE),
>>> 0, LLCC_STATUS_READ_DELAY);
>>> if (ret)
>>> @@ -1284,6 +1286,16 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>>>
>>> drv_data->version = version;
>>>
>>> + /* Applicable only when drv_data->version >= 4.1 */
>>> + drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
>>> + if (IS_ERR(drv_data->bcast_and_regmap)) {
>>
>> I am pretty sure this breaks all users. Can you please explain how do
>> you maintain ABI and that IS_ERR() is applied only for version >= 4.1?
>>
>> Best regards,
>> Krzysztof
>>
> IS_ERR() check is done for all versions.
> If new register isn't defined in DT(for version < 4.1) it simply sets bcast_and_regmap to NULL.
> Otherwise, for version >= 4.1, it goes to err(in the case bcast_and_regmap isn't set properly).
b4 shazam <this series>
booting on 8250, I get:
[ 2.794850] qcom-llcc 9200000.system-cache-controller: invalid resource (null)
which comes from lib/devres.c : __devm_ioremap_resource()
Now, this is gonna get you an angry Johan(+CC) response when he sees this land in
the next release. Perhaps, this warning could either be removed from libdevres,
or some sort of an _optional variant that doesn't print it could be introduced.
Konrad
On 4/10/2024 11:24 AM, Konrad Dybcio wrote:
>
>
> On 4/2/24 21:34, Unnathi Chalicheemala wrote:
>> On 3/30/2024 4:46 AM, Krzysztof Kozlowski wrote:
>>> On 29/03/2024 22:53, Unnathi Chalicheemala wrote:
>>>> Define new regmap structure for Broadcast_AND region and initialize
>>>> this regmap when HW block version is greater than 4.1, otherwise
>>>> initialize as a NULL pointer for backwards compatibility.
>>>>
>>>
>>>> + struct regmap *regmap;
>>>> u32 act_ctrl_reg;
>>>> u32 act_clear_reg;
>>>> u32 status_reg;
>>>> @@ -849,7 +850,8 @@ 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,
>>>> + regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
>>>> + ret = regmap_read_poll_timeout(regmap, status_reg,
>>>> slice_status, (slice_status & ACT_COMPLETE),
>>>> 0, LLCC_STATUS_READ_DELAY);
>>>> if (ret)
>>>> @@ -1284,6 +1286,16 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>>>> drv_data->version = version;
>>>> + /* Applicable only when drv_data->version >= 4.1 */
>>>> + drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
>>>> + if (IS_ERR(drv_data->bcast_and_regmap)) {
>>>
>>> I am pretty sure this breaks all users. Can you please explain how do
>>> you maintain ABI and that IS_ERR() is applied only for version >= 4.1?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> IS_ERR() check is done for all versions.
>> If new register isn't defined in DT(for version < 4.1) it simply sets bcast_and_regmap to NULL.
>> Otherwise, for version >= 4.1, it goes to err(in the case bcast_and_regmap isn't set properly).
>
> b4 shazam <this series>
>
> booting on 8250, I get:
>
> [ 2.794850] qcom-llcc 9200000.system-cache-controller: invalid resource (null)
>
> which comes from lib/devres.c : __devm_ioremap_resource()
>
> Now, this is gonna get you an angry Johan(+CC) response when he sees this land in
> the next release. Perhaps, this warning could either be removed from libdevres,
> or some sort of an _optional variant that doesn't print it could be introduced.
>
> Konrad
Apologies for extremely late reply Konrad. Let me try to recap quickly.
The part you pointed out initializes a new regmap LLCC Boradcast AND region
which is available only SM8450 onward. This patch set is updating respective DTs
and driver code.
Regarding the resource error on booting, I had added this check in previous version
of patch set (https://lore.kernel.org/all/1ca4d384-9df4-4c00-a4c9-0c5ff491616e@linaro.org/)
@@ -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;
+ }
+ }
+
This check will make sure we call devm_ioremap_resource() only when LLCC Boradcast AND region
is defined in the devicetree and error is not shown in log.
On Fri, Mar 29, 2024 at 02:53:41PM -0700, Unnathi Chalicheemala wrote: > Define new regmap structure for Broadcast_AND region and initialize > this regmap when HW block version is greater than 4.1, otherwise > initialize as a NULL pointer for backwards compatibility. > > Switch from broadcast_OR to broadcast_AND region (when defined in DT) > for checking status bit 1 as Broadcast_OR region checks only for bit 0. > Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> Regards, Bjorn
© 2016 - 2026 Red Hat, Inc.