Messages in this thread | | | Date | Wed, 24 Aug 2022 18:37:36 +0530 | Subject | Re: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register offsets to EDAC driver | From | Sai Prakash Ranjan <> |
| |
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 <manivannan.sadhasivam@linaro.org> >>>> <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
| |