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@xxxxxxxxxx/) @@ -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.