Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <396e6b2e-11d1-a11d-206a-cfd69f6cd358@quicinc.com>
Date:   Mon, 22 Aug 2022 17:29:13 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.1
Subject: Re: [PATCH v2 1/3] soc: qcom: llcc: Pass SoC specific EDAC register
 offsets to EDAC driver
Content-Language: en-US
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        <bjorn.andersson@linaro.org>, <bp@alien8.de>, <mchehab@kernel.org>
CC:     <james.morse@arm.com>, <rric@kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-edac@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_tsoni@quicinc.com>
References: <20220812060602.7672-1-manivannan.sadhasivam@linaro.org>
 <20220812060602.7672-2-manivannan.sadhasivam@linaro.org>
From:   Sai Prakash Ranjan <quic_saipraka@quicinc.com>
In-Reply-To: <20220812060602.7672-2-manivannan.sadhasivam@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.

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