diff mbox series

[v4,15/16] qcom: llcc/edac: Fix the base address used for accessing LLCC banks

Message ID 20221222131656.49584-16-manivannan.sadhasivam@linaro.org (mailing list archive)
State Superseded
Headers show
Series Qcom: LLCC/EDAC: Fix base address used for LLCC banks | expand

Commit Message

Manivannan Sadhasivam Dec. 22, 2022, 1:16 p.m. UTC 
The Qualcomm LLCC/EDAC drivers were using a fixed register stride for
accessing the (Control and Status Registers) CSRs of each LLCC bank.
This stride only works for some SoCs like SDM845 for which driver
support was initially added.

But the later SoCs use different register stride that vary between the
banks with holes in-between. So it is not possible to use a single register
stride for accessing the CSRs of each bank. By doing so could result in a
crash.

For fixing this issue, let's obtain the base address of each LLCC bank from
devicetree and get rid of the fixed stride. This also means, we no longer
need to rely on reg-names property and get the base addresses using index.

First index is LLCC bank 0 and last index is LLCC broadcast. If the SoC
supports more than one bank, then those needs to be defined in devicetree
for index from 1..N-1.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/edac/qcom_edac.c           | 14 +++---
 drivers/soc/qcom/llcc-qcom.c       | 72 +++++++++++++++++-------------
 include/linux/soc/qcom/llcc-qcom.h |  6 +--
 3 files changed, 48 insertions(+), 44 deletions(-)

Comments

Bjorn Andersson Dec. 28, 2022, 4:29 a.m. UTC | #1 
On Thu, Dec 22, 2022 at 06:46:55PM +0530, Manivannan Sadhasivam wrote:
> First index is LLCC bank 0 and last index is LLCC broadcast. If the SoC
> supports more than one bank, then those needs to be defined in devicetree
> for index from 1..N-1.
> 

What happened to my request for dropping the reliance on reg-names and
pick reg entries per the logic you describe here?

Was it request just lost or was there a reason why you stuck with the
reg-names?

Regards,
Bjorn

> Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
> Tested-by: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/edac/qcom_edac.c           | 14 +++---
>  drivers/soc/qcom/llcc-qcom.c       | 72 +++++++++++++++++-------------
>  include/linux/soc/qcom/llcc-qcom.h |  6 +--
>  3 files changed, 48 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> index 3256254c3722..1d3cc1930a74 100644
> --- a/drivers/edac/qcom_edac.c
> +++ b/drivers/edac/qcom_edac.c
> @@ -213,7 +213,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
>  
>  	for (i = 0; i < reg_data.reg_cnt; i++) {
>  		synd_reg = reg_data.synd_reg + (i * 4);
> -		ret = regmap_read(drv->regmap, drv->offsets[bank] + synd_reg,
> +		ret = regmap_read(drv->regmaps[bank], synd_reg,
>  				  &synd_val);
>  		if (ret)
>  			goto clear;
> @@ -222,8 +222,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
>  			    reg_data.name, i, synd_val);
>  	}
>  
> -	ret = regmap_read(drv->regmap,
> -			  drv->offsets[bank] + reg_data.count_status_reg,
> +	ret = regmap_read(drv->regmaps[bank], reg_data.count_status_reg,
>  			  &err_cnt);
>  	if (ret)
>  		goto clear;
> @@ -233,8 +232,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
>  	edac_printk(KERN_CRIT, EDAC_LLCC, "%s: Error count: 0x%4x\n",
>  		    reg_data.name, err_cnt);
>  
> -	ret = regmap_read(drv->regmap,
> -			  drv->offsets[bank] + reg_data.ways_status_reg,
> +	ret = regmap_read(drv->regmaps[bank], reg_data.ways_status_reg,
>  			  &err_ways);
>  	if (ret)
>  		goto clear;
> @@ -296,8 +294,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
>  
>  	/* Iterate over the banks and look for Tag RAM or Data RAM errors */
>  	for (i = 0; i < drv->num_banks; i++) {
> -		ret = regmap_read(drv->regmap,
> -				  drv->offsets[i] + DRP_INTERRUPT_STATUS,
> +		ret = regmap_read(drv->regmaps[i], DRP_INTERRUPT_STATUS,
>  				  &drp_error);
>  
>  		if (!ret && (drp_error & SB_ECC_ERROR)) {
> @@ -312,8 +309,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
>  		if (!ret)
>  			irq_rc = IRQ_HANDLED;
>  
> -		ret = regmap_read(drv->regmap,
> -				  drv->offsets[i] + TRP_INTERRUPT_0_STATUS,
> +		ret = regmap_read(drv->regmaps[i], TRP_INTERRUPT_0_STATUS,
>  				  &trp_error);
>  
>  		if (!ret && (trp_error & SB_ECC_ERROR)) {
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 23ce2f78c4ed..72f3f2a9aaa0 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -62,8 +62,6 @@
>  #define LLCC_TRP_WRSC_CACHEABLE_EN    0x21f2c
>  #define LLCC_TRP_ALGO_CFG8	      0x21f30
>  
> -#define BANK_OFFSET_STRIDE	      0x80000
> -
>  #define LLCC_VERSION_2_0_0_0          0x02000000
>  #define LLCC_VERSION_2_1_0_0          0x02010000
>  #define LLCC_VERSION_4_1_0_0          0x04010000
> @@ -898,8 +896,8 @@ static int qcom_llcc_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
> -		const char *name)
> +static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev, u8 index,
> +					  const char *name)
>  {
>  	void __iomem *base;
>  	struct regmap_config llcc_regmap_config = {
> @@ -909,7 +907,7 @@ static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
>  		.fast_io = true,
>  	};
>  
> -	base = devm_platform_ioremap_resource_byname(pdev, name);
> +	base = devm_platform_ioremap_resource(pdev, index);
>  	if (IS_ERR(base))
>  		return ERR_CAST(base);
>  
> @@ -927,6 +925,7 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>  	const struct llcc_slice_config *llcc_cfg;
>  	u32 sz;
>  	u32 version;
> +	struct regmap *regmap;
>  
>  	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
>  	if (!drv_data) {
> @@ -934,21 +933,51 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	drv_data->regmap = qcom_llcc_init_mmio(pdev, "llcc_base");
> -	if (IS_ERR(drv_data->regmap)) {
> -		ret = PTR_ERR(drv_data->regmap);
> +	/* Initialize the first LLCC bank regmap */
> +	regmap = qcom_llcc_init_mmio(pdev, 0, "llcc0_base");
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
>  		goto err;
>  	}
>  
> -	drv_data->bcast_regmap =
> -		qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
> +	cfg = of_device_get_match_data(&pdev->dev);
> +
> +	ret = regmap_read(regmap, cfg->reg_offset[LLCC_COMMON_STATUS0], &num_banks);
> +	if (ret)
> +		goto err;
> +
> +	num_banks &= LLCC_LB_CNT_MASK;
> +	num_banks >>= LLCC_LB_CNT_SHIFT;
> +	drv_data->num_banks = num_banks;
> +
> +	drv_data->regmaps = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmaps), GFP_KERNEL);
> +	if (!drv_data->regmaps) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	drv_data->regmaps[0] = regmap;
> +
> +	/* Initialize rest of LLCC bank regmaps */
> +	for (i = 1; i < num_banks; i++) {
> +		char *base = kasprintf(GFP_KERNEL, "llcc%d_base", i);
> +
> +		drv_data->regmaps[i] = qcom_llcc_init_mmio(pdev, i, base);
> +		if (IS_ERR(drv_data->regmaps[i])) {
> +			ret = PTR_ERR(drv_data->regmaps[i]);
> +			kfree(base);
> +			goto err;
> +		}
> +
> +		kfree(base);
> +	}
> +
> +	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;
>  	}
>  
> -	cfg = of_device_get_match_data(&pdev->dev);
> -
>  	/* Extract version of the IP */
>  	ret = regmap_read(drv_data->bcast_regmap, cfg->reg_offset[LLCC_COMMON_HW_INFO],
>  			  &version);
> @@ -957,15 +986,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>  
>  	drv_data->version = version;
>  
> -	ret = regmap_read(drv_data->regmap, cfg->reg_offset[LLCC_COMMON_STATUS0],
> -			  &num_banks);
> -	if (ret)
> -		goto err;
> -
> -	num_banks &= LLCC_LB_CNT_MASK;
> -	num_banks >>= LLCC_LB_CNT_SHIFT;
> -	drv_data->num_banks = num_banks;
> -
>  	llcc_cfg = cfg->sct_data;
>  	sz = cfg->size;
>  
> @@ -973,16 +993,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>  		if (llcc_cfg[i].slice_id > drv_data->max_slices)
>  			drv_data->max_slices = llcc_cfg[i].slice_id;
>  
> -	drv_data->offsets = devm_kcalloc(dev, num_banks, sizeof(u32),
> -							GFP_KERNEL);
> -	if (!drv_data->offsets) {
> -		ret = -ENOMEM;
> -		goto err;
> -	}
> -
> -	for (i = 0; i < num_banks; i++)
> -		drv_data->offsets[i] = i * BANK_OFFSET_STRIDE;
> -
>  	drv_data->bitmap = devm_bitmap_zalloc(dev, drv_data->max_slices,
>  					      GFP_KERNEL);
>  	if (!drv_data->bitmap) {
> diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
> index ad1fd718169d..423220e66026 100644
> --- a/include/linux/soc/qcom/llcc-qcom.h
> +++ b/include/linux/soc/qcom/llcc-qcom.h
> @@ -120,7 +120,7 @@ struct llcc_edac_reg_offset {
>  
>  /**
>   * struct llcc_drv_data - Data associated with the llcc driver
> - * @regmap: regmap associated with the llcc device
> + * @regmaps: regmaps associated with the llcc device
>   * @bcast_regmap: regmap associated with llcc broadcast offset
>   * @cfg: pointer to the data structure for slice configuration
>   * @edac_reg_offset: Offset of the LLCC EDAC registers
> @@ -129,12 +129,11 @@ struct llcc_edac_reg_offset {
>   * @max_slices: max slices as read from device tree
>   * @num_banks: Number of llcc banks
>   * @bitmap: Bit map to track the active slice ids
> - * @offsets: Pointer to the bank offsets array
>   * @ecc_irq: interrupt for llcc cache error detection and reporting
>   * @version: Indicates the LLCC version
>   */
>  struct llcc_drv_data {
> -	struct regmap *regmap;
> +	struct regmap **regmaps;
>  	struct regmap *bcast_regmap;
>  	const struct llcc_slice_config *cfg;
>  	const struct llcc_edac_reg_offset *edac_reg_offset;
> @@ -143,7 +142,6 @@ struct llcc_drv_data {
>  	u32 max_slices;
>  	u32 num_banks;
>  	unsigned long *bitmap;
> -	u32 *offsets;
>  	int ecc_irq;
>  	u32 version;
>  };
> -- 
> 2.25.1
>
Manivannan Sadhasivam Dec. 28, 2022, 6:53 a.m. UTC | #2 
On Tue, Dec 27, 2022 at 10:29:44PM -0600, Bjorn Andersson wrote:
> On Thu, Dec 22, 2022 at 06:46:55PM +0530, Manivannan Sadhasivam wrote:
> > First index is LLCC bank 0 and last index is LLCC broadcast. If the SoC
> > supports more than one bank, then those needs to be defined in devicetree
> > for index from 1..N-1.
> > 
> 
> What happened to my request for dropping the reliance on reg-names and
> pick reg entries per the logic you describe here?
> 
> Was it request just lost or was there a reason why you stuck with the
> reg-names?
> 

The driver uses index to map the resources from DT and not using reg-names.
See qcom_llcc_init_mmio().

But the reg-names property is kept in the devicetree as per Krzysztof's
request. I've recorded these in the changelog.

Thanks,
Mani

> Regards,
> Bjorn
> 
> > Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
> > Tested-by: Luca Weiss <luca.weiss@fairphone.com>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/edac/qcom_edac.c           | 14 +++---
> >  drivers/soc/qcom/llcc-qcom.c       | 72 +++++++++++++++++-------------
> >  include/linux/soc/qcom/llcc-qcom.h |  6 +--
> >  3 files changed, 48 insertions(+), 44 deletions(-)
> > 
> > diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> > index 3256254c3722..1d3cc1930a74 100644
> > --- a/drivers/edac/qcom_edac.c
> > +++ b/drivers/edac/qcom_edac.c
> > @@ -213,7 +213,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
> >  
> >  	for (i = 0; i < reg_data.reg_cnt; i++) {
> >  		synd_reg = reg_data.synd_reg + (i * 4);
> > -		ret = regmap_read(drv->regmap, drv->offsets[bank] + synd_reg,
> > +		ret = regmap_read(drv->regmaps[bank], synd_reg,
> >  				  &synd_val);
> >  		if (ret)
> >  			goto clear;
> > @@ -222,8 +222,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
> >  			    reg_data.name, i, synd_val);
> >  	}
> >  
> > -	ret = regmap_read(drv->regmap,
> > -			  drv->offsets[bank] + reg_data.count_status_reg,
> > +	ret = regmap_read(drv->regmaps[bank], reg_data.count_status_reg,
> >  			  &err_cnt);
> >  	if (ret)
> >  		goto clear;
> > @@ -233,8 +232,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
> >  	edac_printk(KERN_CRIT, EDAC_LLCC, "%s: Error count: 0x%4x\n",
> >  		    reg_data.name, err_cnt);
> >  
> > -	ret = regmap_read(drv->regmap,
> > -			  drv->offsets[bank] + reg_data.ways_status_reg,
> > +	ret = regmap_read(drv->regmaps[bank], reg_data.ways_status_reg,
> >  			  &err_ways);
> >  	if (ret)
> >  		goto clear;
> > @@ -296,8 +294,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
> >  
> >  	/* Iterate over the banks and look for Tag RAM or Data RAM errors */
> >  	for (i = 0; i < drv->num_banks; i++) {
> > -		ret = regmap_read(drv->regmap,
> > -				  drv->offsets[i] + DRP_INTERRUPT_STATUS,
> > +		ret = regmap_read(drv->regmaps[i], DRP_INTERRUPT_STATUS,
> >  				  &drp_error);
> >  
> >  		if (!ret && (drp_error & SB_ECC_ERROR)) {
> > @@ -312,8 +309,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
> >  		if (!ret)
> >  			irq_rc = IRQ_HANDLED;
> >  
> > -		ret = regmap_read(drv->regmap,
> > -				  drv->offsets[i] + TRP_INTERRUPT_0_STATUS,
> > +		ret = regmap_read(drv->regmaps[i], TRP_INTERRUPT_0_STATUS,
> >  				  &trp_error);
> >  
> >  		if (!ret && (trp_error & SB_ECC_ERROR)) {
> > diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> > index 23ce2f78c4ed..72f3f2a9aaa0 100644
> > --- a/drivers/soc/qcom/llcc-qcom.c
> > +++ b/drivers/soc/qcom/llcc-qcom.c
> > @@ -62,8 +62,6 @@
> >  #define LLCC_TRP_WRSC_CACHEABLE_EN    0x21f2c
> >  #define LLCC_TRP_ALGO_CFG8	      0x21f30
> >  
> > -#define BANK_OFFSET_STRIDE	      0x80000
> > -
> >  #define LLCC_VERSION_2_0_0_0          0x02000000
> >  #define LLCC_VERSION_2_1_0_0          0x02010000
> >  #define LLCC_VERSION_4_1_0_0          0x04010000
> > @@ -898,8 +896,8 @@ static int qcom_llcc_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > -static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
> > -		const char *name)
> > +static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev, u8 index,
> > +					  const char *name)
> >  {
> >  	void __iomem *base;
> >  	struct regmap_config llcc_regmap_config = {
> > @@ -909,7 +907,7 @@ static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
> >  		.fast_io = true,
> >  	};
> >  
> > -	base = devm_platform_ioremap_resource_byname(pdev, name);
> > +	base = devm_platform_ioremap_resource(pdev, index);
> >  	if (IS_ERR(base))
> >  		return ERR_CAST(base);
> >  
> > @@ -927,6 +925,7 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> >  	const struct llcc_slice_config *llcc_cfg;
> >  	u32 sz;
> >  	u32 version;
> > +	struct regmap *regmap;
> >  
> >  	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
> >  	if (!drv_data) {
> > @@ -934,21 +933,51 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> >  		goto err;
> >  	}
> >  
> > -	drv_data->regmap = qcom_llcc_init_mmio(pdev, "llcc_base");
> > -	if (IS_ERR(drv_data->regmap)) {
> > -		ret = PTR_ERR(drv_data->regmap);
> > +	/* Initialize the first LLCC bank regmap */
> > +	regmap = qcom_llcc_init_mmio(pdev, 0, "llcc0_base");
> > +	if (IS_ERR(regmap)) {
> > +		ret = PTR_ERR(regmap);
> >  		goto err;
> >  	}
> >  
> > -	drv_data->bcast_regmap =
> > -		qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
> > +	cfg = of_device_get_match_data(&pdev->dev);
> > +
> > +	ret = regmap_read(regmap, cfg->reg_offset[LLCC_COMMON_STATUS0], &num_banks);
> > +	if (ret)
> > +		goto err;
> > +
> > +	num_banks &= LLCC_LB_CNT_MASK;
> > +	num_banks >>= LLCC_LB_CNT_SHIFT;
> > +	drv_data->num_banks = num_banks;
> > +
> > +	drv_data->regmaps = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmaps), GFP_KERNEL);
> > +	if (!drv_data->regmaps) {
> > +		ret = -ENOMEM;
> > +		goto err;
> > +	}
> > +
> > +	drv_data->regmaps[0] = regmap;
> > +
> > +	/* Initialize rest of LLCC bank regmaps */
> > +	for (i = 1; i < num_banks; i++) {
> > +		char *base = kasprintf(GFP_KERNEL, "llcc%d_base", i);
> > +
> > +		drv_data->regmaps[i] = qcom_llcc_init_mmio(pdev, i, base);
> > +		if (IS_ERR(drv_data->regmaps[i])) {
> > +			ret = PTR_ERR(drv_data->regmaps[i]);
> > +			kfree(base);
> > +			goto err;
> > +		}
> > +
> > +		kfree(base);
> > +	}
> > +
> > +	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;
> >  	}
> >  
> > -	cfg = of_device_get_match_data(&pdev->dev);
> > -
> >  	/* Extract version of the IP */
> >  	ret = regmap_read(drv_data->bcast_regmap, cfg->reg_offset[LLCC_COMMON_HW_INFO],
> >  			  &version);
> > @@ -957,15 +986,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> >  
> >  	drv_data->version = version;
> >  
> > -	ret = regmap_read(drv_data->regmap, cfg->reg_offset[LLCC_COMMON_STATUS0],
> > -			  &num_banks);
> > -	if (ret)
> > -		goto err;
> > -
> > -	num_banks &= LLCC_LB_CNT_MASK;
> > -	num_banks >>= LLCC_LB_CNT_SHIFT;
> > -	drv_data->num_banks = num_banks;
> > -
> >  	llcc_cfg = cfg->sct_data;
> >  	sz = cfg->size;
> >  
> > @@ -973,16 +993,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> >  		if (llcc_cfg[i].slice_id > drv_data->max_slices)
> >  			drv_data->max_slices = llcc_cfg[i].slice_id;
> >  
> > -	drv_data->offsets = devm_kcalloc(dev, num_banks, sizeof(u32),
> > -							GFP_KERNEL);
> > -	if (!drv_data->offsets) {
> > -		ret = -ENOMEM;
> > -		goto err;
> > -	}
> > -
> > -	for (i = 0; i < num_banks; i++)
> > -		drv_data->offsets[i] = i * BANK_OFFSET_STRIDE;
> > -
> >  	drv_data->bitmap = devm_bitmap_zalloc(dev, drv_data->max_slices,
> >  					      GFP_KERNEL);
> >  	if (!drv_data->bitmap) {
> > diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
> > index ad1fd718169d..423220e66026 100644
> > --- a/include/linux/soc/qcom/llcc-qcom.h
> > +++ b/include/linux/soc/qcom/llcc-qcom.h
> > @@ -120,7 +120,7 @@ struct llcc_edac_reg_offset {
> >  
> >  /**
> >   * struct llcc_drv_data - Data associated with the llcc driver
> > - * @regmap: regmap associated with the llcc device
> > + * @regmaps: regmaps associated with the llcc device
> >   * @bcast_regmap: regmap associated with llcc broadcast offset
> >   * @cfg: pointer to the data structure for slice configuration
> >   * @edac_reg_offset: Offset of the LLCC EDAC registers
> > @@ -129,12 +129,11 @@ struct llcc_edac_reg_offset {
> >   * @max_slices: max slices as read from device tree
> >   * @num_banks: Number of llcc banks
> >   * @bitmap: Bit map to track the active slice ids
> > - * @offsets: Pointer to the bank offsets array
> >   * @ecc_irq: interrupt for llcc cache error detection and reporting
> >   * @version: Indicates the LLCC version
> >   */
> >  struct llcc_drv_data {
> > -	struct regmap *regmap;
> > +	struct regmap **regmaps;
> >  	struct regmap *bcast_regmap;
> >  	const struct llcc_slice_config *cfg;
> >  	const struct llcc_edac_reg_offset *edac_reg_offset;
> > @@ -143,7 +142,6 @@ struct llcc_drv_data {
> >  	u32 max_slices;
> >  	u32 num_banks;
> >  	unsigned long *bitmap;
> > -	u32 *offsets;
> >  	int ecc_irq;
> >  	u32 version;
> >  };
> > -- 
> > 2.25.1
> >
Krzysztof Kozlowski Dec. 28, 2022, 9:43 a.m. UTC | #3 
On 28/12/2022 07:53, Manivannan Sadhasivam wrote:
> On Tue, Dec 27, 2022 at 10:29:44PM -0600, Bjorn Andersson wrote:
>> On Thu, Dec 22, 2022 at 06:46:55PM +0530, Manivannan Sadhasivam wrote:
>>> First index is LLCC bank 0 and last index is LLCC broadcast. If the SoC
>>> supports more than one bank, then those needs to be defined in devicetree
>>> for index from 1..N-1.
>>>
>>
>> What happened to my request for dropping the reliance on reg-names and
>> pick reg entries per the logic you describe here?
>>
>> Was it request just lost or was there a reason why you stuck with the
>> reg-names?
>>
> 
> The driver uses index to map the resources from DT and not using reg-names.
> See qcom_llcc_init_mmio().
> 
> But the reg-names property is kept in the devicetree as per Krzysztof's
> request. I've recorded these in the changelog.

For the record, my comment was about keeping the reg-names in the DTS
and bindings (as non-required, although that part I maybe did not
express explicitly).

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 3256254c3722..1d3cc1930a74 100644
--- a/drivers/edac/qcom_edac.c
+++ b/drivers/edac/qcom_edac.c
@@ -213,7 +213,7 @@  dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
 
 	for (i = 0; i < reg_data.reg_cnt; i++) {
 		synd_reg = reg_data.synd_reg + (i * 4);
-		ret = regmap_read(drv->regmap, drv->offsets[bank] + synd_reg,
+		ret = regmap_read(drv->regmaps[bank], synd_reg,
 				  &synd_val);
 		if (ret)
 			goto clear;
@@ -222,8 +222,7 @@  dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
 			    reg_data.name, i, synd_val);
 	}
 
-	ret = regmap_read(drv->regmap,
-			  drv->offsets[bank] + reg_data.count_status_reg,
+	ret = regmap_read(drv->regmaps[bank], reg_data.count_status_reg,
 			  &err_cnt);
 	if (ret)
 		goto clear;
@@ -233,8 +232,7 @@  dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
 	edac_printk(KERN_CRIT, EDAC_LLCC, "%s: Error count: 0x%4x\n",
 		    reg_data.name, err_cnt);
 
-	ret = regmap_read(drv->regmap,
-			  drv->offsets[bank] + reg_data.ways_status_reg,
+	ret = regmap_read(drv->regmaps[bank], reg_data.ways_status_reg,
 			  &err_ways);
 	if (ret)
 		goto clear;
@@ -296,8 +294,7 @@  llcc_ecc_irq_handler(int irq, void *edev_ctl)
 
 	/* Iterate over the banks and look for Tag RAM or Data RAM errors */
 	for (i = 0; i < drv->num_banks; i++) {
-		ret = regmap_read(drv->regmap,
-				  drv->offsets[i] + DRP_INTERRUPT_STATUS,
+		ret = regmap_read(drv->regmaps[i], DRP_INTERRUPT_STATUS,
 				  &drp_error);
 
 		if (!ret && (drp_error & SB_ECC_ERROR)) {
@@ -312,8 +309,7 @@  llcc_ecc_irq_handler(int irq, void *edev_ctl)
 		if (!ret)
 			irq_rc = IRQ_HANDLED;
 
-		ret = regmap_read(drv->regmap,
-				  drv->offsets[i] + TRP_INTERRUPT_0_STATUS,
+		ret = regmap_read(drv->regmaps[i], TRP_INTERRUPT_0_STATUS,
 				  &trp_error);
 
 		if (!ret && (trp_error & SB_ECC_ERROR)) {
diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 23ce2f78c4ed..72f3f2a9aaa0 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -62,8 +62,6 @@ 
 #define LLCC_TRP_WRSC_CACHEABLE_EN    0x21f2c
 #define LLCC_TRP_ALGO_CFG8	      0x21f30
 
-#define BANK_OFFSET_STRIDE	      0x80000
-
 #define LLCC_VERSION_2_0_0_0          0x02000000
 #define LLCC_VERSION_2_1_0_0          0x02010000
 #define LLCC_VERSION_4_1_0_0          0x04010000
@@ -898,8 +896,8 @@  static int qcom_llcc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
-		const char *name)
+static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev, u8 index,
+					  const char *name)
 {
 	void __iomem *base;
 	struct regmap_config llcc_regmap_config = {
@@ -909,7 +907,7 @@  static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
 		.fast_io = true,
 	};
 
-	base = devm_platform_ioremap_resource_byname(pdev, name);
+	base = devm_platform_ioremap_resource(pdev, index);
 	if (IS_ERR(base))
 		return ERR_CAST(base);
 
@@ -927,6 +925,7 @@  static int qcom_llcc_probe(struct platform_device *pdev)
 	const struct llcc_slice_config *llcc_cfg;
 	u32 sz;
 	u32 version;
+	struct regmap *regmap;
 
 	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
 	if (!drv_data) {
@@ -934,21 +933,51 @@  static int qcom_llcc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	drv_data->regmap = qcom_llcc_init_mmio(pdev, "llcc_base");
-	if (IS_ERR(drv_data->regmap)) {
-		ret = PTR_ERR(drv_data->regmap);
+	/* Initialize the first LLCC bank regmap */
+	regmap = qcom_llcc_init_mmio(pdev, 0, "llcc0_base");
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
 		goto err;
 	}
 
-	drv_data->bcast_regmap =
-		qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
+	cfg = of_device_get_match_data(&pdev->dev);
+
+	ret = regmap_read(regmap, cfg->reg_offset[LLCC_COMMON_STATUS0], &num_banks);
+	if (ret)
+		goto err;
+
+	num_banks &= LLCC_LB_CNT_MASK;
+	num_banks >>= LLCC_LB_CNT_SHIFT;
+	drv_data->num_banks = num_banks;
+
+	drv_data->regmaps = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmaps), GFP_KERNEL);
+	if (!drv_data->regmaps) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	drv_data->regmaps[0] = regmap;
+
+	/* Initialize rest of LLCC bank regmaps */
+	for (i = 1; i < num_banks; i++) {
+		char *base = kasprintf(GFP_KERNEL, "llcc%d_base", i);
+
+		drv_data->regmaps[i] = qcom_llcc_init_mmio(pdev, i, base);
+		if (IS_ERR(drv_data->regmaps[i])) {
+			ret = PTR_ERR(drv_data->regmaps[i]);
+			kfree(base);
+			goto err;
+		}
+
+		kfree(base);
+	}
+
+	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;
 	}
 
-	cfg = of_device_get_match_data(&pdev->dev);
-
 	/* Extract version of the IP */
 	ret = regmap_read(drv_data->bcast_regmap, cfg->reg_offset[LLCC_COMMON_HW_INFO],
 			  &version);
@@ -957,15 +986,6 @@  static int qcom_llcc_probe(struct platform_device *pdev)
 
 	drv_data->version = version;
 
-	ret = regmap_read(drv_data->regmap, cfg->reg_offset[LLCC_COMMON_STATUS0],
-			  &num_banks);
-	if (ret)
-		goto err;
-
-	num_banks &= LLCC_LB_CNT_MASK;
-	num_banks >>= LLCC_LB_CNT_SHIFT;
-	drv_data->num_banks = num_banks;
-
 	llcc_cfg = cfg->sct_data;
 	sz = cfg->size;
 
@@ -973,16 +993,6 @@  static int qcom_llcc_probe(struct platform_device *pdev)
 		if (llcc_cfg[i].slice_id > drv_data->max_slices)
 			drv_data->max_slices = llcc_cfg[i].slice_id;
 
-	drv_data->offsets = devm_kcalloc(dev, num_banks, sizeof(u32),
-							GFP_KERNEL);
-	if (!drv_data->offsets) {
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	for (i = 0; i < num_banks; i++)
-		drv_data->offsets[i] = i * BANK_OFFSET_STRIDE;
-
 	drv_data->bitmap = devm_bitmap_zalloc(dev, drv_data->max_slices,
 					      GFP_KERNEL);
 	if (!drv_data->bitmap) {
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index ad1fd718169d..423220e66026 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -120,7 +120,7 @@  struct llcc_edac_reg_offset {
 
 /**
  * struct llcc_drv_data - Data associated with the llcc driver
- * @regmap: regmap associated with the llcc device
+ * @regmaps: regmaps associated with the llcc device
  * @bcast_regmap: regmap associated with llcc broadcast offset
  * @cfg: pointer to the data structure for slice configuration
  * @edac_reg_offset: Offset of the LLCC EDAC registers
@@ -129,12 +129,11 @@  struct llcc_edac_reg_offset {
  * @max_slices: max slices as read from device tree
  * @num_banks: Number of llcc banks
  * @bitmap: Bit map to track the active slice ids
- * @offsets: Pointer to the bank offsets array
  * @ecc_irq: interrupt for llcc cache error detection and reporting
  * @version: Indicates the LLCC version
  */
 struct llcc_drv_data {
-	struct regmap *regmap;
+	struct regmap **regmaps;
 	struct regmap *bcast_regmap;
 	const struct llcc_slice_config *cfg;
 	const struct llcc_edac_reg_offset *edac_reg_offset;
@@ -143,7 +142,6 @@  struct llcc_drv_data {
 	u32 max_slices;
 	u32 num_banks;
 	unsigned long *bitmap;
-	u32 *offsets;
 	int ecc_irq;
 	u32 version;
 };