diff mbox series

[v1,2/3] soc: qcom: llcc: Update configuration data for IPQ5424

Message ID 20241104073840.3686674-3-quic_varada@quicinc.com (mailing list archive)
State Superseded
Headers show
Series qcom: ipq5424: Add LLCC/system-cache-controller | expand

Commit Message

Varadarajan Narayanan Nov. 4, 2024, 7:38 a.m. UTC 
The 'broadcast' register space is present only in chipsets that
have multiple instances of LLCC IP. Since IPQ5424 has only one
instance, both the LLCC and LLCC_BROADCAST points to the same
register space.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/soc/qcom/llcc-qcom.c | 60 +++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

Comments

Konrad Dybcio Nov. 4, 2024, 11:04 a.m. UTC | #1 
On 4.11.2024 8:38 AM, Varadarajan Narayanan wrote:
> The 'broadcast' register space is present only in chipsets that
> have multiple instances of LLCC IP. Since IPQ5424 has only one
> instance, both the LLCC and LLCC_BROADCAST points to the same
> register space.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---

[...]

> +static const struct llcc_slice_config ipq5424_data[] =  {
> +	{
> +		.usecase_id = LLCC_CPUSS,
> +		.slice_id = 1,
> +		.max_cap = 768,
> +		.priority = 1,
> +		.bonus_ways = 0xFFFF,
> +		.retain_on_pc = 1,
> +		.activate_on_init = 1,
> +		.write_scid_cacheable_en = 1,
> +		.stale_en = 1,
> +		.stale_cap_en = 1,
> +		.alloc_oneway_en = 1,
> +		.ovcap_en = 1,
> +		.ovcap_prio = 1,
> +		.vict_prio = 1,

Many of these are booleans, please use true/false values

[...]

>  
> -	drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
> -	if (IS_ERR(drv_data->bcast_regmap)) {
> -		ret = PTR_ERR(drv_data->bcast_regmap);
> -		goto err;
> +	if (num_banks == 1) {
> +		drv_data->bcast_regmap = regmap;
> +	} else {
> +		drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
> +		if (IS_ERR(drv_data->bcast_regmap)) {
> +			ret = PTR_ERR(drv_data->bcast_regmap);
> +			goto err;
> +		}
>  	}

This won't work. See for example

https://lore.kernel.org/linux-arm-msm/20241031-add_llcc_dts_node_for_qcs615-v2-1-205766a607ca@quicinc.com/

Which has both just one bank and llcc_broadcase_base.

You probably want to introduce a quirk like:

if (IS_ERR(drv_data->bcast_regmap)) {
	if (cfg->no_broadcast_register)
		bcast_regmap = drv_data->regmaps[0];
	else {
		ret = PTR_ERR(drv_data->bcast_regmap);
		goto err;
	}
}

Konrad
Varadarajan Narayanan Nov. 5, 2024, 10:24 a.m. UTC | #2 
On Mon, Nov 04, 2024 at 12:04:52PM +0100, Konrad Dybcio wrote:
> On 4.11.2024 8:38 AM, Varadarajan Narayanan wrote:
> > The 'broadcast' register space is present only in chipsets that
> > have multiple instances of LLCC IP. Since IPQ5424 has only one
> > instance, both the LLCC and LLCC_BROADCAST points to the same
> > register space.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
>
> [...]
>
> > +static const struct llcc_slice_config ipq5424_data[] =  {
> > +	{
> > +		.usecase_id = LLCC_CPUSS,
> > +		.slice_id = 1,
> > +		.max_cap = 768,
> > +		.priority = 1,
> > +		.bonus_ways = 0xFFFF,
> > +		.retain_on_pc = 1,
> > +		.activate_on_init = 1,
> > +		.write_scid_cacheable_en = 1,
> > +		.stale_en = 1,
> > +		.stale_cap_en = 1,
> > +		.alloc_oneway_en = 1,
> > +		.ovcap_en = 1,
> > +		.ovcap_prio = 1,
> > +		.vict_prio = 1,
>
> Many of these are booleans, please use true/false values
>
> [...]
>
> >
> > -	drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
> > -	if (IS_ERR(drv_data->bcast_regmap)) {
> > -		ret = PTR_ERR(drv_data->bcast_regmap);
> > -		goto err;
> > +	if (num_banks == 1) {
> > +		drv_data->bcast_regmap = regmap;
> > +	} else {
> > +		drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
> > +		if (IS_ERR(drv_data->bcast_regmap)) {
> > +			ret = PTR_ERR(drv_data->bcast_regmap);
> > +			goto err;
> > +		}
> >  	}
>
> This won't work. See for example
>
> https://lore.kernel.org/linux-arm-msm/20241031-add_llcc_dts_node_for_qcs615-v2-1-205766a607ca@quicinc.com/
>
> Which has both just one bank and llcc_broadcase_base.
>
> You probably want to introduce a quirk like:
>
> if (IS_ERR(drv_data->bcast_regmap)) {
> 	if (cfg->no_broadcast_register)
> 		bcast_regmap = drv_data->regmaps[0];
> 	else {
> 		ret = PTR_ERR(drv_data->bcast_regmap);
> 		goto err;
> 	}
> }

Konrad,

Have posted v2 incorporating the above comments, please review.

Thanks
Varada
diff mbox series

Patch

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index a470285f54a8..51dba8ec2183 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -152,6 +152,38 @@  enum llcc_reg_offset {
 	LLCC_COMMON_STATUS0,
 };
 
+static const struct llcc_slice_config ipq5424_data[] =  {
+	{
+		.usecase_id = LLCC_CPUSS,
+		.slice_id = 1,
+		.max_cap = 768,
+		.priority = 1,
+		.bonus_ways = 0xFFFF,
+		.retain_on_pc = 1,
+		.activate_on_init = 1,
+		.write_scid_cacheable_en = 1,
+		.stale_en = 1,
+		.stale_cap_en = 1,
+		.alloc_oneway_en = 1,
+		.ovcap_en = 1,
+		.ovcap_prio = 1,
+		.vict_prio = 1,
+	},
+	{
+		.usecase_id = LLCC_VIDSC0,
+		.slice_id = 2,
+		.max_cap = 256,
+		.priority = 2,
+		.fixed_size = 1,
+		.bonus_ways = 0xF000,
+		.retain_on_pc = 1,
+		.activate_on_init = 1,
+		.write_scid_cacheable_en = 1,
+		.stale_en = 1,
+		.stale_cap_en = 1,
+	},
+};
+
 static const struct llcc_slice_config sa8775p_data[] =  {
 	{
 		.usecase_id = LLCC_CPUSS,
@@ -2677,6 +2709,16 @@  static const struct qcom_llcc_config qdu1000_cfg[] = {
 	},
 };
 
+static const struct qcom_llcc_config ipq5424_cfg[] = {
+	{
+		.sct_data       = ipq5424_data,
+		.size           = ARRAY_SIZE(ipq5424_data),
+		.need_llcc_cfg  = true,
+		.reg_offset     = llcc_v2_1_reg_offset,
+		.edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+	},
+};
+
 static const struct qcom_llcc_config sa8775p_cfg[] = {
 	{
 		.sct_data	= sa8775p_data,
@@ -2834,6 +2876,11 @@  static const struct qcom_sct_config qdu1000_cfgs = {
 	.num_config	= ARRAY_SIZE(qdu1000_cfg),
 };
 
+static const struct qcom_sct_config ipq5424_cfgs = {
+	.llcc_config	= ipq5424_cfg,
+	.num_config	= ARRAY_SIZE(ipq5424_cfg),
+};
+
 static const struct qcom_sct_config sa8775p_cfgs = {
 	.llcc_config	= sa8775p_cfg,
 	.num_config	= ARRAY_SIZE(sa8775p_cfg),
@@ -3410,10 +3457,14 @@  static int qcom_llcc_probe(struct platform_device *pdev)
 		}
 	}
 
-	drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
-	if (IS_ERR(drv_data->bcast_regmap)) {
-		ret = PTR_ERR(drv_data->bcast_regmap);
-		goto err;
+	if (num_banks == 1) {
+		drv_data->bcast_regmap = regmap;
+	} else {
+		drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
+		if (IS_ERR(drv_data->bcast_regmap)) {
+			ret = PTR_ERR(drv_data->bcast_regmap);
+			goto err;
+		}
 	}
 
 	/* Extract version of the IP */
@@ -3485,6 +3536,7 @@  static int qcom_llcc_probe(struct platform_device *pdev)
 
 static const struct of_device_id qcom_llcc_of_match[] = {
 	{ .compatible = "qcom,qdu1000-llcc", .data = &qdu1000_cfgs},
+	{ .compatible = "qcom,ipq5424-llcc", .data = &ipq5424_cfgs},
 	{ .compatible = "qcom,sa8775p-llcc", .data = &sa8775p_cfgs },
 	{ .compatible = "qcom,sc7180-llcc", .data = &sc7180_cfgs },
 	{ .compatible = "qcom,sc7280-llcc", .data = &sc7280_cfgs },