From: Rabara Niravkumar L <[email protected]>
A bug in legacy U-Boot causes a crash during SDRAM boot if ECC
is not enabled in the bitstream but enabled in the Linux config.
Memory mapped read of the ECC Enabled bit was only enabled
if U-Boot determined ECC was enabled in the bitstream.
The Linux driver checks the ECC enable bit using a memory map
read. In the ECC disabled bitstream case, U-Boot didn't
enable ECC register memory map reads and since they are not
allowed this results in a crash.
This patch always reads the ECC Enable register through a
SMC call which is always allowed and it works with legacy
and current U-Boot.
Signed-off-by: Rabara Niravkumar L <[email protected]>
---
drivers/edac/altera_edac.c | 40 +++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 5dd29789f97d..e7e8e624a436 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1083,8 +1083,46 @@ static int __init __maybe_unused altr_init_a10_ecc_device_type(char *compat)
#ifdef CONFIG_EDAC_ALTERA_SDRAM
+/*
+ * A legacy U-Boot bug only enabled memory mapped access to the ECC Enable
+ * register if ECC is enabled. Linux checks the ECC Enable register to
+ * determine ECC status.
+ * Use an SMC call (which always works) to determine ECC enablement.
+ */
+static int altr_s10_sdram_check_ecc_deps(struct altr_edac_device_dev *device)
+{
+ const struct edac_device_prv_data *prv = device->data;
+ unsigned long sdram_ecc_addr;
+ struct arm_smccc_res result;
+ struct device_node *np;
+ phys_addr_t sdram_addr;
+ u32 read_reg;
+ int ret;
+
+ np = of_find_compatible_node(NULL, NULL, "altr,sdr-ctl");
+ if (!np)
+ goto sdram_err;
+
+ sdram_addr = of_translate_address(np, of_get_address(np, 0,
+ NULL, NULL));
+ of_node_put(np);
+ sdram_ecc_addr = (unsigned long)sdram_addr + prv->ecc_en_ofst;
+ arm_smccc_smc(INTEL_SIP_SMC_REG_READ, sdram_ecc_addr,
+ 0, 0, 0, 0, 0, 0, &result);
+ read_reg = (unsigned int)result.a1;
+ ret = (int)result.a0;
+ if (!ret && (read_reg & prv->ecc_enable_mask))
+ return 0;
+
+sdram_err:
+ edac_printk(KERN_ERR, EDAC_DEVICE,
+ "%s: No ECC present or ECC disabled.\n",
+ device->edac_dev_name);
+ return -ENODEV;
+}
+
static const struct edac_device_prv_data s10_sdramecc_data = {
- .setup = altr_check_ecc_deps,
+ .setup = altr_s10_sdram_check_ecc_deps,
.ce_clear_mask = ALTR_S10_ECC_SERRPENA,
.ue_clear_mask = ALTR_S10_ECC_DERRPENA,
.ecc_enable_mask = ALTR_S10_ECC_EN,
--
2.19.0
On 3/4/22 19:41, [email protected] wrote:
> From: Rabara Niravkumar L <[email protected]>
>
> A bug in legacy U-Boot causes a crash during SDRAM boot if ECC
> is not enabled in the bitstream but enabled in the Linux config.
>
> Memory mapped read of the ECC Enabled bit was only enabled
> if U-Boot determined ECC was enabled in the bitstream.
>
> The Linux driver checks the ECC enable bit using a memory map
> read. In the ECC disabled bitstream case, U-Boot didn't
> enable ECC register memory map reads and since they are not
> allowed this results in a crash.
>
> This patch always reads the ECC Enable register through a
> SMC call which is always allowed and it works with legacy
> and current U-Boot.
>
> Signed-off-by: Rabara Niravkumar L <[email protected]>
> ---
> drivers/edac/altera_edac.c | 40 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 5dd29789f97d..e7e8e624a436 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -1083,8 +1083,46 @@ static int __init __maybe_unused altr_init_a10_ecc_device_type(char *compat)
>
> #ifdef CONFIG_EDAC_ALTERA_SDRAM
>
> +/*
> + * A legacy U-Boot bug only enabled memory mapped access to the ECC Enable
> + * register if ECC is enabled. Linux checks the ECC Enable register to
> + * determine ECC status.
> + * Use an SMC call (which always works) to determine ECC enablement.
> + */
> +static int altr_s10_sdram_check_ecc_deps(struct altr_edac_device_dev *device)
> +{
> + const struct edac_device_prv_data *prv = device->data;
> + unsigned long sdram_ecc_addr;
> + struct arm_smccc_res result;
> + struct device_node *np;
> + phys_addr_t sdram_addr;
> + u32 read_reg;
> + int ret;
> +
> + np = of_find_compatible_node(NULL, NULL, "altr,sdr-ctl");
> + if (!np)
> + goto sdram_err;
> +
> + sdram_addr = of_translate_address(np, of_get_address(np, 0,
> + NULL, NULL));
> + of_node_put(np);
> + sdram_ecc_addr = (unsigned long)sdram_addr + prv->ecc_en_ofst;
> + arm_smccc_smc(INTEL_SIP_SMC_REG_READ, sdram_ecc_addr,
> + 0, 0, 0, 0, 0, 0, &result);
> + read_reg = (unsigned int)result.a1;
> + ret = (int)result.a0;
> + if (!ret && (read_reg & prv->ecc_enable_mask))
> + return 0;
> +
> +sdram_err:
> + edac_printk(KERN_ERR, EDAC_DEVICE,
> + "%s: No ECC present or ECC disabled.\n",
> + device->edac_dev_name);
> + return -ENODEV;
> +}
> +
> static const struct edac_device_prv_data s10_sdramecc_data = {
> - .setup = altr_check_ecc_deps,
> + .setup = altr_s10_sdram_check_ecc_deps,
> .ce_clear_mask = ALTR_S10_ECC_SERRPENA,
> .ue_clear_mask = ALTR_S10_ECC_DERRPENA,
> .ecc_enable_mask = ALTR_S10_ECC_EN,
Acked-by: Dinh Nguyen <[email protected]>
On Sat, Mar 05, 2022 at 09:41:18AM +0800, [email protected] wrote:
> From: Rabara Niravkumar L <[email protected]>
>
> A bug in legacy U-Boot causes a crash during SDRAM boot if ECC
> is not enabled in the bitstream but enabled in the Linux config.
>
> Memory mapped read of the ECC Enabled bit was only enabled
> if U-Boot determined ECC was enabled in the bitstream.
>
> The Linux driver checks the ECC enable bit using a memory map
> read. In the ECC disabled bitstream case, U-Boot didn't
> enable ECC register memory map reads and since they are not
> allowed this results in a crash.
>
> This patch always reads the ECC Enable register through a
Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.
Also, do
$ git grep 'This patch' Documentation/process
for more details.
I've removed it now but pls pay attention for the future.
> SMC call which is always allowed and it works with legacy
> and current U-Boot.
>
> Signed-off-by: Rabara Niravkumar L <[email protected]>
> ---
> drivers/edac/altera_edac.c | 40 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
Applied, thanks.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette