2022-08-12 06:24:40

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v2 0/3] Fix crash when using Qcom LLCC/EDAC drivers

Hello,

This series fixes the crash seen on the Qualcomm SM8450 chipset with the
LLCC/EDAC drivers. The problem was due to the Qcom EDAC driver using the
fixed LLCC register offsets for detecting the LLCC errors.

This seems to have worked for SoCs till SM8450. But in SM8450, the LLCC
register offsets were changed. So accessing the fixed offsets causes the
crash on this platform.

So for fixing this issue, and also to make it work on future SoCs, let's
pass the LLCC offsets from the Qcom LLCC driver based on the individual
SoCs and let the EDAC driver make use of them.

This series has been tested on SM8450 based dev board.

Thanks,
Mani

Changes in v2:

* Volunteered myself as a maintainer for the EDAC driver since the current
maintainers have left Qualcomm and I couldn't get hold of them.

Manivannan Sadhasivam (3):
soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC
driver
EDAC/qcom: Get rid of hardcoded register offsets
MAINTAINERS: Add myself as the maintainer for qcom_edac driver

MAINTAINERS | 3 +-
drivers/edac/qcom_edac.c | 112 ++++++++++++++---------------
drivers/soc/qcom/llcc-qcom.c | 64 +++++++++++++++++
include/linux/soc/qcom/llcc-qcom.h | 35 +++++++--
4 files changed, 148 insertions(+), 66 deletions(-)

--
2.25.1


2022-08-12 06:25:55

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC driver

The LLCC EDAC register offsets varies between each SoCs. Until now, the
EDAC driver used the hardcoded register offsets. But this caused crash
on SM8450 SoC where the register offsets has been changed.

So to avoid this crash and also to make it easy to accomodate changes for
new SoCs, let's pass the SoC specific register offsets to the EDAC driver.

Currently, two set of offsets are used. One is SM8450 specific and another
one is common to all SoCs.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/soc/qcom/llcc-qcom.c | 64 ++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index eecafeded56f..1aedbbb8e96f 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -104,6 +104,7 @@ struct qcom_llcc_config {
int size;
bool need_llcc_cfg;
const u32 *reg_offset;
+ const struct llcc_edac_reg *edac_reg;
};

enum llcc_reg_offset {
@@ -252,6 +253,60 @@ static const struct llcc_slice_config sm8450_data[] = {
{LLCC_AENPU, 8, 2048, 1, 1, 0xFFFF, 0x0, 0, 0, 0, 0, 0, 0, 0 },
};

+static const struct llcc_edac_reg common_edac_reg = {
+ .trp_ecc_error_status0 = 0x20344,
+ .trp_ecc_error_status1 = 0x20348,
+ .trp_ecc_sb_err_syn0 = 0x2304c,
+ .trp_ecc_db_err_syn0 = 0x20370,
+ .trp_ecc_error_cntr_clear = 0x20440,
+ .trp_interrupt_0_status = 0x20480,
+ .trp_interrupt_0_clear = 0x20484,
+ .trp_interrupt_0_enable = 0x20488,
+
+ /* LLCC Common registers */
+ .cmn_status0 = 0x3000c,
+ .cmn_interrupt_0_enable = 0x3001c,
+ .cmn_interrupt_2_enable = 0x3003c,
+
+ /* LLCC DRP registers */
+ .drp_ecc_error_cfg = 0x40000,
+ .drp_ecc_error_cntr_clear = 0x40004,
+ .drp_interrupt_status = 0x41000,
+ .drp_interrupt_clear = 0x41008,
+ .drp_interrupt_enable = 0x4100c,
+ .drp_ecc_error_status0 = 0x42044,
+ .drp_ecc_error_status1 = 0x42048,
+ .drp_ecc_sb_err_syn0 = 0x4204c,
+ .drp_ecc_db_err_syn0 = 0x42070,
+};
+
+static const struct llcc_edac_reg sm8450_edac_reg = {
+ .trp_ecc_error_status0 = 0x20344,
+ .trp_ecc_error_status1 = 0x20348,
+ .trp_ecc_sb_err_syn0 = 0x2034c,
+ .trp_ecc_db_err_syn0 = 0x20370,
+ .trp_ecc_error_cntr_clear = 0x20440,
+ .trp_interrupt_0_status = 0x20480,
+ .trp_interrupt_0_clear = 0x20484,
+ .trp_interrupt_0_enable = 0x20488,
+
+ /* LLCC Common registers */
+ .cmn_status0 = 0x3400c,
+ .cmn_interrupt_0_enable = 0x3401c,
+ .cmn_interrupt_2_enable = 0x3403c,
+
+ /* LLCC DRP registers */
+ .drp_ecc_error_cfg = 0x50000,
+ .drp_ecc_error_cntr_clear = 0x50004,
+ .drp_interrupt_status = 0x50020,
+ .drp_interrupt_clear = 0x50028,
+ .drp_interrupt_enable = 0x5002c,
+ .drp_ecc_error_status0 = 0x520f4,
+ .drp_ecc_error_status1 = 0x520f8,
+ .drp_ecc_sb_err_syn0 = 0x520fc,
+ .drp_ecc_db_err_syn0 = 0x52120,
+};
+
static const u32 llcc_v1_2_reg_offset[] = {
[LLCC_COMMON_HW_INFO] = 0x00030000,
[LLCC_COMMON_STATUS0] = 0x0003000c,
@@ -267,6 +322,7 @@ static const struct qcom_llcc_config sc7180_cfg = {
.size = ARRAY_SIZE(sc7180_data),
.need_llcc_cfg = true,
.reg_offset = llcc_v1_2_reg_offset,
+ .edac_reg = &common_edac_reg,
};

static const struct qcom_llcc_config sc7280_cfg = {
@@ -274,6 +330,7 @@ static const struct qcom_llcc_config sc7280_cfg = {
.size = ARRAY_SIZE(sc7280_data),
.need_llcc_cfg = true,
.reg_offset = llcc_v1_2_reg_offset,
+ .edac_reg = &common_edac_reg,
};

static const struct qcom_llcc_config sdm845_cfg = {
@@ -281,6 +338,7 @@ static const struct qcom_llcc_config sdm845_cfg = {
.size = ARRAY_SIZE(sdm845_data),
.need_llcc_cfg = false,
.reg_offset = llcc_v1_2_reg_offset,
+ .edac_reg = &common_edac_reg,
};

static const struct qcom_llcc_config sm6350_cfg = {
@@ -288,6 +346,7 @@ static const struct qcom_llcc_config sm6350_cfg = {
.size = ARRAY_SIZE(sm6350_data),
.need_llcc_cfg = true,
.reg_offset = llcc_v1_2_reg_offset,
+ .edac_reg = &common_edac_reg,
};

static const struct qcom_llcc_config sm8150_cfg = {
@@ -295,6 +354,7 @@ static const struct qcom_llcc_config sm8150_cfg = {
.size = ARRAY_SIZE(sm8150_data),
.need_llcc_cfg = true,
.reg_offset = llcc_v1_2_reg_offset,
+ .edac_reg = &common_edac_reg,
};

static const struct qcom_llcc_config sm8250_cfg = {
@@ -302,6 +362,7 @@ static const struct qcom_llcc_config sm8250_cfg = {
.size = ARRAY_SIZE(sm8250_data),
.need_llcc_cfg = true,
.reg_offset = llcc_v1_2_reg_offset,
+ .edac_reg = &common_edac_reg,
};

static const struct qcom_llcc_config sm8350_cfg = {
@@ -309,6 +370,7 @@ static const struct qcom_llcc_config sm8350_cfg = {
.size = ARRAY_SIZE(sm8350_data),
.need_llcc_cfg = true,
.reg_offset = llcc_v1_2_reg_offset,
+ .edac_reg = &common_edac_reg,
};

static const struct qcom_llcc_config sm8450_cfg = {
@@ -316,6 +378,7 @@ static const struct qcom_llcc_config sm8450_cfg = {
.size = ARRAY_SIZE(sm8450_data),
.need_llcc_cfg = true,
.reg_offset = llcc_v21_reg_offset,
+ .edac_reg = &sm8450_edac_reg,
};

static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
@@ -716,6 +779,7 @@ static int qcom_llcc_probe(struct platform_device *pdev)

drv_data->cfg = llcc_cfg;
drv_data->cfg_size = sz;
+ drv_data->edac_reg = cfg->edac_reg;
mutex_init(&drv_data->lock);
platform_set_drvdata(pdev, drv_data);

--
2.25.1

2022-08-12 06:26:34

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v2 3/3] MAINTAINERS: Add myself as the maintainer for qcom_edac driver

The current maintainers have left Qualcomm and their email addresses were
bouncing. Since I couldn't get hold of them now, I'm volunteering myself
to maintain this driver.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
MAINTAINERS | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 491c7842419a..775f81644bfc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7143,8 +7143,7 @@ S: Maintained
F: drivers/edac/pnd2_edac.[ch]

EDAC-QCOM
-M: Channagoud Kadabi <[email protected]>
-M: Venkata Narendra Kumar Gutta <[email protected]>
+M: Manivannan Sadhasivam <[email protected]>
L: [email protected]
L: [email protected]
S: Maintained
--
2.25.1

2022-08-22 12:08:44

by Sai Prakash Ranjan

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC driver

Hi Mani,

On 8/12/2022 11:36 AM, Manivannan Sadhasivam wrote:
> The LLCC EDAC register offsets varies between each SoCs. Until now, the
> EDAC driver used the hardcoded register offsets. But this caused crash
> on SM8450 SoC where the register offsets has been changed.
>
> So to avoid this crash and also to make it easy to accomodate changes for
> new SoCs, let's pass the SoC specific register offsets to the EDAC driver.
>
> Currently, two set of offsets are used. One is SM8450 specific and another
> one is common to all SoCs.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

<snip> ...

> static const struct qcom_llcc_config sm8350_cfg = {
> @@ -309,6 +370,7 @@ static const struct qcom_llcc_config sm8350_cfg = {
> .size = ARRAY_SIZE(sm8350_data),
> .need_llcc_cfg = true,
> .reg_offset = llcc_v1_2_reg_offset,
> + .edac_reg = &common_edac_reg,
> };
>
> static const struct qcom_llcc_config sm8450_cfg = {
> @@ -316,6 +378,7 @@ static const struct qcom_llcc_config sm8450_cfg = {
> .size = ARRAY_SIZE(sm8450_data),
> .need_llcc_cfg = true,
> .reg_offset = llcc_v21_reg_offset,
> + .edac_reg = &sm8450_edac_reg,
> };
>

Can we have LLCC version specific register offsets instead of SoC specific similar to reg_offset callbacks?
For SM8450, it would be llcc_v21_edac_reg and for others llcc_v1_2_edac_reg instead of common_edac_reg.
common_edac_reg is very general and is not exactly common for all, its just common for SoCs with same LLCC.

Version based is more applicable as multiple SoCs might use same LLCC versions and would reduce SoC specific data
which would be needed for every SoC in case some newer LLCC comes out. I know you could just call sm8450_edac_reg
for lets say sm8550 or so on to reduce duplication but that won't look good.


Thanks,
Sai

2022-08-22 12:30:26

by Sai Prakash Ranjan

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] MAINTAINERS: Add myself as the maintainer for qcom_edac driver

On 8/12/2022 11:36 AM, Manivannan Sadhasivam wrote:
> The current maintainers have left Qualcomm and their email addresses were
> bouncing. Since I couldn't get hold of them now, I'm volunteering myself
> to maintain this driver.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> MAINTAINERS | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 491c7842419a..775f81644bfc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7143,8 +7143,7 @@ S: Maintained
> F: drivers/edac/pnd2_edac.[ch]
>
> EDAC-QCOM
> -M: Channagoud Kadabi <[email protected]>
> -M: Venkata Narendra Kumar Gutta <[email protected]>
> +M: Manivannan Sadhasivam <[email protected]>
> L: [email protected]
> L: [email protected]
> S: Maintained

Acked-by: Sai Prakash Ranjan <[email protected]>

2022-08-23 19:07:58

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC driver

On Mon, Aug 22, 2022 at 05:29:13PM +0530, Sai Prakash Ranjan wrote:
> Hi Mani,
>
> On 8/12/2022 11:36 AM, Manivannan Sadhasivam wrote:
> > The LLCC EDAC register offsets varies between each SoCs. Until now, the
> > EDAC driver used the hardcoded register offsets. But this caused crash
> > on SM8450 SoC where the register offsets has been changed.
> >
> > So to avoid this crash and also to make it easy to accomodate changes for
> > new SoCs, let's pass the SoC specific register offsets to the EDAC driver.
> >
> > Currently, two set of offsets are used. One is SM8450 specific and another
> > one is common to all SoCs.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
>
> <snip> ...
>
> > static const struct qcom_llcc_config sm8350_cfg = {
> > @@ -309,6 +370,7 @@ static const struct qcom_llcc_config sm8350_cfg = {
> > .size = ARRAY_SIZE(sm8350_data),
> > .need_llcc_cfg = true,
> > .reg_offset = llcc_v1_2_reg_offset,
> > + .edac_reg = &common_edac_reg,
> > };
> > static const struct qcom_llcc_config sm8450_cfg = {
> > @@ -316,6 +378,7 @@ static const struct qcom_llcc_config sm8450_cfg = {
> > .size = ARRAY_SIZE(sm8450_data),
> > .need_llcc_cfg = true,
> > .reg_offset = llcc_v21_reg_offset,
> > + .edac_reg = &sm8450_edac_reg,
> > };
> >
>
> Can we have LLCC version specific register offsets instead of SoC specific similar to reg_offset callbacks?
> For SM8450, it would be llcc_v21_edac_reg and for others llcc_v1_2_edac_reg instead of common_edac_reg.
> common_edac_reg is very general and is not exactly common for all, its just common for SoCs with same LLCC.
>

I thought about it but I was not sure if rest of the SoCs are using version
v1.2. I know that reg_offset uses v1.2 but I was skeptical and hence used the
SoC specific offsets.

Can you confirm if rest of the SoCs are using v1.2?

Thanks,
Mani

> Version based is more applicable as multiple SoCs might use same LLCC versions and would reduce SoC specific data
> which would be needed for every SoC in case some newer LLCC comes out. I know you could just call sm8450_edac_reg
> for lets say sm8550 or so on to reduce duplication but that won't look good.
>
>
> Thanks,
> Sai

--
மணிவண்ணன் சதாசிவம்

2022-08-24 05:50:13

by Sai Prakash Ranjan

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC driver

On 8/23/2022 9:01 PM, Manivannan Sadhasivam wrote:
> On Mon, Aug 22, 2022 at 05:29:13PM +0530, Sai Prakash Ranjan wrote:
>> Hi Mani,
>>
>> On 8/12/2022 11:36 AM, Manivannan Sadhasivam wrote:
>>> The LLCC EDAC register offsets varies between each SoCs. Until now, the
>>> EDAC driver used the hardcoded register offsets. But this caused crash
>>> on SM8450 SoC where the register offsets has been changed.
>>>
>>> So to avoid this crash and also to make it easy to accomodate changes for
>>> new SoCs, let's pass the SoC specific register offsets to the EDAC driver.
>>>
>>> Currently, two set of offsets are used. One is SM8450 specific and another
>>> one is common to all SoCs.
>>>
>>> Signed-off-by: Manivannan Sadhasivam <[email protected]>
>> <snip> ...
>>
>>> static const struct qcom_llcc_config sm8350_cfg = {
>>> @@ -309,6 +370,7 @@ static const struct qcom_llcc_config sm8350_cfg = {
>>> .size = ARRAY_SIZE(sm8350_data),
>>> .need_llcc_cfg = true,
>>> .reg_offset = llcc_v1_2_reg_offset,
>>> + .edac_reg = &common_edac_reg,
>>> };
>>> static const struct qcom_llcc_config sm8450_cfg = {
>>> @@ -316,6 +378,7 @@ static const struct qcom_llcc_config sm8450_cfg = {
>>> .size = ARRAY_SIZE(sm8450_data),
>>> .need_llcc_cfg = true,
>>> .reg_offset = llcc_v21_reg_offset,
>>> + .edac_reg = &sm8450_edac_reg,
>>> };
>>>
>> Can we have LLCC version specific register offsets instead of SoC specific similar to reg_offset callbacks?
>> For SM8450, it would be llcc_v21_edac_reg and for others llcc_v1_2_edac_reg instead of common_edac_reg.
>> common_edac_reg is very general and is not exactly common for all, its just common for SoCs with same LLCC.
>>
> I thought about it but I was not sure if rest of the SoCs are using version
> v1.2. I know that reg_offset uses v1.2 but I was skeptical and hence used the
> SoC specific offsets.
>
> Can you confirm if rest of the SoCs are using v1.2?

LLCC versioning follows w.x.y.z format and w and y are major and minor versions based
on which the naming for reg_offsets is chosen.

Now in above reg_offsets, llcc_v1_2 is not v1.2, it means v1.0 or v2.0 where 1, 2 is a major version
and 0 is a minor version. llcc_v21 is actually v2.1 where 2 is a major and 1 is a minor version.
I know the naming is pretty bad, should probably replace llcc_v1_2 with llcc_v1_0_v2_0 and
llcc_v21 with llcc_v2_1? Note here minor version is important because SM8350 is v2.0 and uses
old reg offsets.

So coming to your query now, all other SoCs except SM8450(which uses v2.1) are using LLCC v1.0
or v2.0, so it is valid to use the same logic as reg_offsets for edac_reg.

Thanks,
Sai

> Thanks,
> Mani
>
>> Version based is more applicable as multiple SoCs might use same LLCC versions and would reduce SoC specific data
>> which would be needed for every SoC in case some newer LLCC comes out. I know you could just call sm8450_edac_reg
>> for lets say sm8550 or so on to reduce duplication but that won't look good.
>>
>>
>> Thanks,
>> Sai

2022-08-24 13:03:30

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC driver

On Wed, Aug 24, 2022 at 10:43:51AM +0530, Sai Prakash Ranjan wrote:
> On 8/23/2022 9:01 PM, Manivannan Sadhasivam wrote:
> > On Mon, Aug 22, 2022 at 05:29:13PM +0530, Sai Prakash Ranjan wrote:
> > > Hi Mani,
> > >
> > > On 8/12/2022 11:36 AM, Manivannan Sadhasivam wrote:
> > > > The LLCC EDAC register offsets varies between each SoCs. Until now, the
> > > > EDAC driver used the hardcoded register offsets. But this caused crash
> > > > on SM8450 SoC where the register offsets has been changed.
> > > >
> > > > So to avoid this crash and also to make it easy to accomodate changes for
> > > > new SoCs, let's pass the SoC specific register offsets to the EDAC driver.
> > > >
> > > > Currently, two set of offsets are used. One is SM8450 specific and another
> > > > one is common to all SoCs.
> > > >
> > > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > <snip> ...
> > >
> > > > static const struct qcom_llcc_config sm8350_cfg = {
> > > > @@ -309,6 +370,7 @@ static const struct qcom_llcc_config sm8350_cfg = {
> > > > .size = ARRAY_SIZE(sm8350_data),
> > > > .need_llcc_cfg = true,
> > > > .reg_offset = llcc_v1_2_reg_offset,
> > > > + .edac_reg = &common_edac_reg,
> > > > };
> > > > static const struct qcom_llcc_config sm8450_cfg = {
> > > > @@ -316,6 +378,7 @@ static const struct qcom_llcc_config sm8450_cfg = {
> > > > .size = ARRAY_SIZE(sm8450_data),
> > > > .need_llcc_cfg = true,
> > > > .reg_offset = llcc_v21_reg_offset,
> > > > + .edac_reg = &sm8450_edac_reg,
> > > > };
> > > >
> > > Can we have LLCC version specific register offsets instead of SoC specific similar to reg_offset callbacks?
> > > For SM8450, it would be llcc_v21_edac_reg and for others llcc_v1_2_edac_reg instead of common_edac_reg.
> > > common_edac_reg is very general and is not exactly common for all, its just common for SoCs with same LLCC.
> > >
> > I thought about it but I was not sure if rest of the SoCs are using version
> > v1.2. I know that reg_offset uses v1.2 but I was skeptical and hence used the
> > SoC specific offsets.
> >
> > Can you confirm if rest of the SoCs are using v1.2?
>
> LLCC versioning follows w.x.y.z format and w and y are major and minor versions based
> on which the naming for reg_offsets is chosen.
>
> Now in above reg_offsets, llcc_v1_2 is not v1.2, it means v1.0 or v2.0 where 1, 2 is a major version
> and 0 is a minor version. llcc_v21 is actually v2.1 where 2 is a major and 1 is a minor version.
> I know the naming is pretty bad, should probably replace llcc_v1_2 with llcc_v1_0_v2_0 and
> llcc_v21 with llcc_v2_1? Note here minor version is important because SM8350 is v2.0 and uses
> old reg offsets.
>

Yeah it is confusing. I think we should just use the base LLCC version
that got changed with the previous one and add a comment on top of the
definition. For instance, all of the SoCs before SM8450 should use
llcc_v1_reg_offset since the LLCC version starts from v1.0.0 and SM8450 should
use llcc_v2_1_reg_offset since it supports the LLCC reg offset that got changed
since v2.1.0. Thoughts?

Thanks,
Mani

> So coming to your query now, all other SoCs except SM8450(which uses v2.1) are using LLCC v1.0
> or v2.0, so it is valid to use the same logic as reg_offsets for edac_reg.
>
> Thanks,
> Sai
>
> > Thanks,
> > Mani
> >
> > > Version based is more applicable as multiple SoCs might use same LLCC versions and would reduce SoC specific data
> > > which would be needed for every SoC in case some newer LLCC comes out. I know you could just call sm8450_edac_reg
> > > for lets say sm8550 or so on to reduce duplication but that won't look good.
> > >
> > >
> > > Thanks,
> > > Sai
>

--
மணிவண்ணன் சதாசிவம்

2022-08-24 13:29:08

by Sai Prakash Ranjan

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC driver

On 8/24/2022 6:27 PM, Manivannan Sadhasivam wrote:
> On Wed, Aug 24, 2022 at 10:43:51AM +0530, Sai Prakash Ranjan wrote:
>> On 8/23/2022 9:01 PM, Manivannan Sadhasivam wrote:
>>> On Mon, Aug 22, 2022 at 05:29:13PM +0530, Sai Prakash Ranjan wrote:
>>>> Hi Mani,
>>>>
>>>> On 8/12/2022 11:36 AM, Manivannan Sadhasivam wrote:
>>>>> The LLCC EDAC register offsets varies between each SoCs. Until now, the
>>>>> EDAC driver used the hardcoded register offsets. But this caused crash
>>>>> on SM8450 SoC where the register offsets has been changed.
>>>>>
>>>>> So to avoid this crash and also to make it easy to accomodate changes for
>>>>> new SoCs, let's pass the SoC specific register offsets to the EDAC driver.
>>>>>
>>>>> Currently, two set of offsets are used. One is SM8450 specific and another
>>>>> one is common to all SoCs.
>>>>>
>>>>> Signed-off-by: Manivannan Sadhasivam <[email protected]>
>>>> <snip> ...
>>>>
>>>>> static const struct qcom_llcc_config sm8350_cfg = {
>>>>> @@ -309,6 +370,7 @@ static const struct qcom_llcc_config sm8350_cfg = {
>>>>> .size = ARRAY_SIZE(sm8350_data),
>>>>> .need_llcc_cfg = true,
>>>>> .reg_offset = llcc_v1_2_reg_offset,
>>>>> + .edac_reg = &common_edac_reg,
>>>>> };
>>>>> static const struct qcom_llcc_config sm8450_cfg = {
>>>>> @@ -316,6 +378,7 @@ static const struct qcom_llcc_config sm8450_cfg = {
>>>>> .size = ARRAY_SIZE(sm8450_data),
>>>>> .need_llcc_cfg = true,
>>>>> .reg_offset = llcc_v21_reg_offset,
>>>>> + .edac_reg = &sm8450_edac_reg,
>>>>> };
>>>>>
>>>> Can we have LLCC version specific register offsets instead of SoC specific similar to reg_offset callbacks?
>>>> For SM8450, it would be llcc_v21_edac_reg and for others llcc_v1_2_edac_reg instead of common_edac_reg.
>>>> common_edac_reg is very general and is not exactly common for all, its just common for SoCs with same LLCC.
>>>>
>>> I thought about it but I was not sure if rest of the SoCs are using version
>>> v1.2. I know that reg_offset uses v1.2 but I was skeptical and hence used the
>>> SoC specific offsets.
>>>
>>> Can you confirm if rest of the SoCs are using v1.2?
>> LLCC versioning follows w.x.y.z format and w and y are major and minor versions based
>> on which the naming for reg_offsets is chosen.
>>
>> Now in above reg_offsets, llcc_v1_2 is not v1.2, it means v1.0 or v2.0 where 1, 2 is a major version
>> and 0 is a minor version. llcc_v21 is actually v2.1 where 2 is a major and 1 is a minor version.
>> I know the naming is pretty bad, should probably replace llcc_v1_2 with llcc_v1_0_v2_0 and
>> llcc_v21 with llcc_v2_1? Note here minor version is important because SM8350 is v2.0 and uses
>> old reg offsets.
>>
> Yeah it is confusing. I think we should just use the base LLCC version
> that got changed with the previous one and add a comment on top of the
> definition. For instance, all of the SoCs before SM8450 should use
> llcc_v1_reg_offset since the LLCC version starts from v1.0.0 and SM8450 should
> use llcc_v2_1_reg_offset since it supports the LLCC reg offset that got changed
> since v2.1.0. Thoughts?

Ya sounds good, only exception is SM8350 which is v2.0 but will be using v1 in naming but I guess its OK.

Thanks,
Sai

> Thanks,
> Mani
>
>> So coming to your query now, all other SoCs except SM8450(which uses v2.1) are using LLCC v1.0
>> or v2.0, so it is valid to use the same logic as reg_offsets for edac_reg.
>>
>> Thanks,
>> Sai
>>
>>> Thanks,
>>> Mani
>>>
>>>> Version based is more applicable as multiple SoCs might use same LLCC versions and would reduce SoC specific data
>>>> which would be needed for every SoC in case some newer LLCC comes out. I know you could just call sm8450_edac_reg
>>>> for lets say sm8550 or so on to reduce duplication but that won't look good.
>>>>
>>>>
>>>> Thanks,
>>>> Sai

2022-10-26 13:18:08

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] MAINTAINERS: Add myself as the maintainer for qcom_edac driver

On Fri, Aug 12, 2022 at 11:36:02AM +0530, Manivannan Sadhasivam wrote:
> The current maintainers have left Qualcomm and their email addresses were
> bouncing. Since I couldn't get hold of them now, I'm volunteering myself
> to maintain this driver.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> MAINTAINERS | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)

Applied, thanks.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette