* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
@ 2019-05-06 12:18 ` Andrew F. Davis
0 siblings, 0 replies; 19+ messages in thread
From: Andrew F. Davis @ 2019-05-06 12:18 UTC (permalink / raw)
To: Yash Shah, linux-riscv, devicetree, palmer
Cc: mark.rutland, aou, linux-kernel, sachin.ghadi, robh+dt, paul.walmsley
On 5/6/19 6:48 AM, Yash Shah wrote:
> The driver currently supports only SiFive FU540-C000 platform.
>
> The initial version of L2 cache controller driver includes:
> - Initial configuration reporting at boot up.
> - Support for ECC related functionality.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
> arch/riscv/mm/Makefile | 1 +
> arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
> 3 files changed, 192 insertions(+)
> create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
> create mode 100644 arch/riscv/mm/sifive_l2_cache.c
>
> diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
> new file mode 100644
> index 0000000..04f6748
> --- /dev/null
> +++ b/arch/riscv/include/asm/sifive_l2_cache.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * SiFive L2 Cache Controller header file
> + *
> + */
> +
> +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
> +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
> +
> +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
> +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
> +
> +#define SIFIVE_L2_ERR_TYPE_CE 0
> +#define SIFIVE_L2_ERR_TYPE_UE 1
> +
> +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index eb22ab4..1523ee5 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -3,3 +3,4 @@ obj-y += fault.o
> obj-y += extable.o
> obj-y += ioremap.o
> obj-y += cacheflush.o
> +obj-y += sifive_l2_cache.o
> diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
> new file mode 100644
> index 0000000..4eb6461
> --- /dev/null
> +++ b/arch/riscv/mm/sifive_l2_cache.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SiFive L2 cache controller Driver
> + *
> + * Copyright (C) 2018-2019 SiFive, Inc.
> + *
> + */
> +#include <linux/debugfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <asm/sifive_l2_cache.h>
> +
> +#define SIFIVE_L2_DIRECCFIX_LOW 0x100
> +#define SIFIVE_L2_DIRECCFIX_HIGH 0x104
> +#define SIFIVE_L2_DIRECCFIX_COUNT 0x108
> +
> +#define SIFIVE_L2_DATECCFIX_LOW 0x140
> +#define SIFIVE_L2_DATECCFIX_HIGH 0x144
> +#define SIFIVE_L2_DATECCFIX_COUNT 0x148
> +
> +#define SIFIVE_L2_DATECCFAIL_LOW 0x160
> +#define SIFIVE_L2_DATECCFAIL_HIGH 0x164
> +#define SIFIVE_L2_DATECCFAIL_COUNT 0x168
> +
> +#define SIFIVE_L2_CONFIG 0x00
> +#define SIFIVE_L2_WAYENABLE 0x08
> +#define SIFIVE_L2_ECCINJECTERR 0x40
> +
> +#define SIFIVE_L2_MAX_ECCINTR 3
> +
> +static void __iomem *l2_base;
> +static int g_irq[SIFIVE_L2_MAX_ECCINTR];
> +
> +enum {
> + DIR_CORR = 0,
> + DATA_CORR,
> + DATA_UNCORR,
> +};
> +
> +#ifdef CONFIG_DEBUG_FS
> +static struct dentry *sifive_test;
> +
> +static ssize_t l2_write(struct file *file, const char __user *data,
> + size_t count, loff_t *ppos)
> +{
> + unsigned int val;
> +
> + if (kstrtouint_from_user(data, count, 0, &val))
> + return -EINVAL;
> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
I'm guessing bit 16 is the enable and the lower 8 are some kind of
region to enable the error? This is probably a bad interface, it looks
useful for testing but doesn't provide any debugging info useful for
running systems. Do you really want userspace to be able to do this?
Andrew
> + writel(val, l2_base + SIFIVE_L2_ECCINJECTERR);
> + else
> + return -EINVAL;
> + return count;
> +}
> +
> +static const struct file_operations l2_fops = {
> + .owner = THIS_MODULE,
> + .open = simple_open,
> + .write = l2_write
> +};
> +
> +static void setup_sifive_debug(void)
> +{
> + sifive_test = debugfs_create_dir("sifive_l2_cache", NULL);
> +
> + debugfs_create_file("sifive_debug_inject_error", 0200,
> + sifive_test, NULL, &l2_fops);
> +}
> +#endif
> +
> +static void l2_config_read(void)
> +{
> + u32 regval, val;
> +
> + regval = readl(l2_base + SIFIVE_L2_CONFIG);
> + val = regval & 0xFF;
> + pr_info("L2CACHE: No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + pr_info("L2CACHE: No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + pr_info("L2CACHE: Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + pr_info("L2CACHE: Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +
> + regval = readl(l2_base + SIFIVE_L2_WAYENABLE);
> + pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval);
> +}
> +
> +static const struct of_device_id sifive_l2_ids[] = {
> + { .compatible = "sifive,fu540-c000-ccache" },
> + { /* end of table */ },
> +};
> +
> +static ATOMIC_NOTIFIER_HEAD(l2_err_chain);
> +
> +int register_sifive_l2_error_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_register(&l2_err_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_sifive_l2_error_notifier);
> +
> +int unregister_sifive_l2_error_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_unregister(&l2_err_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_sifive_l2_error_notifier);
> +
> +static irqreturn_t l2_int_handler(int irq, void *device)
> +{
> + unsigned int regval, add_h, add_l;
> +
> + if (irq == g_irq[DIR_CORR]) {
> + add_h = readl(l2_base + SIFIVE_L2_DIRECCFIX_HIGH);
> + add_l = readl(l2_base + SIFIVE_L2_DIRECCFIX_LOW);
> + pr_err("L2CACHE: DirError @ 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(l2_base + SIFIVE_L2_DIRECCFIX_COUNT);
> + atomic_notifier_call_chain(&l2_err_chain, SIFIVE_L2_ERR_TYPE_CE,
> + "DirECCFix");
> + }
> + if (irq == g_irq[DATA_CORR]) {
> + add_h = readl(l2_base + SIFIVE_L2_DATECCFIX_HIGH);
> + add_l = readl(l2_base + SIFIVE_L2_DATECCFIX_LOW);
> + pr_err("L2CACHE: DataError @ 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(l2_base + SIFIVE_L2_DATECCFIX_COUNT);
> + atomic_notifier_call_chain(&l2_err_chain, SIFIVE_L2_ERR_TYPE_CE,
> + "DatECCFix");
> + }
> + if (irq == g_irq[DATA_UNCORR]) {
> + add_h = readl(l2_base + SIFIVE_L2_DATECCFAIL_HIGH);
> + add_l = readl(l2_base + SIFIVE_L2_DATECCFAIL_LOW);
> + pr_err("L2CACHE: DataFail @ 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(l2_base + SIFIVE_L2_DATECCFAIL_COUNT);
> + atomic_notifier_call_chain(&l2_err_chain, SIFIVE_L2_ERR_TYPE_UE,
> + "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +int __init sifive_l2_init(void)
> +{
> + struct device_node *np;
> + struct resource res;
> + int i, rc;
> +
> + np = of_find_matching_node(NULL, sifive_l2_ids);
> + if (!np)
> + return -ENODEV;
> +
> + if (of_address_to_resource(np, 0, &res))
> + return -ENODEV;
> +
> + l2_base = ioremap(res.start, resource_size(&res));
> + if (!l2_base)
> + return -ENOMEM;
> +
> + for (i = 0; i < SIFIVE_L2_MAX_ECCINTR; i++) {
> + g_irq[i] = irq_of_parse_and_map(np, i);
> + rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL);
> + if (rc) {
> + pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]);
> + return rc;
> + }
> + }
> +
> + l2_config_read();
> +
> +#ifdef CONFIG_DEBUG_FS
> + setup_sifive_debug();
> +#endif
> + return 0;
> +}
> +device_initcall(sifive_l2_init);
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
@ 2019-05-06 12:18 ` Andrew F. Davis
0 siblings, 0 replies; 19+ messages in thread
From: Andrew F. Davis @ 2019-05-06 12:18 UTC (permalink / raw)
To: Yash Shah, linux-riscv, devicetree, palmer
Cc: paul.walmsley, linux-kernel, aou, mark.rutland, robh+dt, sachin.ghadi
On 5/6/19 6:48 AM, Yash Shah wrote:
> The driver currently supports only SiFive FU540-C000 platform.
>
> The initial version of L2 cache controller driver includes:
> - Initial configuration reporting at boot up.
> - Support for ECC related functionality.
>
> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> ---
> arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
> arch/riscv/mm/Makefile | 1 +
> arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
> 3 files changed, 192 insertions(+)
> create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
> create mode 100644 arch/riscv/mm/sifive_l2_cache.c
>
> diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
> new file mode 100644
> index 0000000..04f6748
> --- /dev/null
> +++ b/arch/riscv/include/asm/sifive_l2_cache.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * SiFive L2 Cache Controller header file
> + *
> + */
> +
> +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
> +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
> +
> +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
> +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
> +
> +#define SIFIVE_L2_ERR_TYPE_CE 0
> +#define SIFIVE_L2_ERR_TYPE_UE 1
> +
> +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index eb22ab4..1523ee5 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -3,3 +3,4 @@ obj-y += fault.o
> obj-y += extable.o
> obj-y += ioremap.o
> obj-y += cacheflush.o
> +obj-y += sifive_l2_cache.o
> diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
> new file mode 100644
> index 0000000..4eb6461
> --- /dev/null
> +++ b/arch/riscv/mm/sifive_l2_cache.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SiFive L2 cache controller Driver
> + *
> + * Copyright (C) 2018-2019 SiFive, Inc.
> + *
> + */
> +#include <linux/debugfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <asm/sifive_l2_cache.h>
> +
> +#define SIFIVE_L2_DIRECCFIX_LOW 0x100
> +#define SIFIVE_L2_DIRECCFIX_HIGH 0x104
> +#define SIFIVE_L2_DIRECCFIX_COUNT 0x108
> +
> +#define SIFIVE_L2_DATECCFIX_LOW 0x140
> +#define SIFIVE_L2_DATECCFIX_HIGH 0x144
> +#define SIFIVE_L2_DATECCFIX_COUNT 0x148
> +
> +#define SIFIVE_L2_DATECCFAIL_LOW 0x160
> +#define SIFIVE_L2_DATECCFAIL_HIGH 0x164
> +#define SIFIVE_L2_DATECCFAIL_COUNT 0x168
> +
> +#define SIFIVE_L2_CONFIG 0x00
> +#define SIFIVE_L2_WAYENABLE 0x08
> +#define SIFIVE_L2_ECCINJECTERR 0x40
> +
> +#define SIFIVE_L2_MAX_ECCINTR 3
> +
> +static void __iomem *l2_base;
> +static int g_irq[SIFIVE_L2_MAX_ECCINTR];
> +
> +enum {
> + DIR_CORR = 0,
> + DATA_CORR,
> + DATA_UNCORR,
> +};
> +
> +#ifdef CONFIG_DEBUG_FS
> +static struct dentry *sifive_test;
> +
> +static ssize_t l2_write(struct file *file, const char __user *data,
> + size_t count, loff_t *ppos)
> +{
> + unsigned int val;
> +
> + if (kstrtouint_from_user(data, count, 0, &val))
> + return -EINVAL;
> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
I'm guessing bit 16 is the enable and the lower 8 are some kind of
region to enable the error? This is probably a bad interface, it looks
useful for testing but doesn't provide any debugging info useful for
running systems. Do you really want userspace to be able to do this?
Andrew
> + writel(val, l2_base + SIFIVE_L2_ECCINJECTERR);
> + else
> + return -EINVAL;
> + return count;
> +}
> +
> +static const struct file_operations l2_fops = {
> + .owner = THIS_MODULE,
> + .open = simple_open,
> + .write = l2_write
> +};
> +
> +static void setup_sifive_debug(void)
> +{
> + sifive_test = debugfs_create_dir("sifive_l2_cache", NULL);
> +
> + debugfs_create_file("sifive_debug_inject_error", 0200,
> + sifive_test, NULL, &l2_fops);
> +}
> +#endif
> +
> +static void l2_config_read(void)
> +{
> + u32 regval, val;
> +
> + regval = readl(l2_base + SIFIVE_L2_CONFIG);
> + val = regval & 0xFF;
> + pr_info("L2CACHE: No. of Banks in the cache: %d\n", val);
> + val = (regval & 0xFF00) >> 8;
> + pr_info("L2CACHE: No. of ways per bank: %d\n", val);
> + val = (regval & 0xFF0000) >> 16;
> + pr_info("L2CACHE: Sets per bank: %llu\n", (uint64_t)1 << val);
> + val = (regval & 0xFF000000) >> 24;
> + pr_info("L2CACHE: Bytes per cache block: %llu\n", (uint64_t)1 << val);
> +
> + regval = readl(l2_base + SIFIVE_L2_WAYENABLE);
> + pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval);
> +}
> +
> +static const struct of_device_id sifive_l2_ids[] = {
> + { .compatible = "sifive,fu540-c000-ccache" },
> + { /* end of table */ },
> +};
> +
> +static ATOMIC_NOTIFIER_HEAD(l2_err_chain);
> +
> +int register_sifive_l2_error_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_register(&l2_err_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_sifive_l2_error_notifier);
> +
> +int unregister_sifive_l2_error_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_unregister(&l2_err_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_sifive_l2_error_notifier);
> +
> +static irqreturn_t l2_int_handler(int irq, void *device)
> +{
> + unsigned int regval, add_h, add_l;
> +
> + if (irq == g_irq[DIR_CORR]) {
> + add_h = readl(l2_base + SIFIVE_L2_DIRECCFIX_HIGH);
> + add_l = readl(l2_base + SIFIVE_L2_DIRECCFIX_LOW);
> + pr_err("L2CACHE: DirError @ 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(l2_base + SIFIVE_L2_DIRECCFIX_COUNT);
> + atomic_notifier_call_chain(&l2_err_chain, SIFIVE_L2_ERR_TYPE_CE,
> + "DirECCFix");
> + }
> + if (irq == g_irq[DATA_CORR]) {
> + add_h = readl(l2_base + SIFIVE_L2_DATECCFIX_HIGH);
> + add_l = readl(l2_base + SIFIVE_L2_DATECCFIX_LOW);
> + pr_err("L2CACHE: DataError @ 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(l2_base + SIFIVE_L2_DATECCFIX_COUNT);
> + atomic_notifier_call_chain(&l2_err_chain, SIFIVE_L2_ERR_TYPE_CE,
> + "DatECCFix");
> + }
> + if (irq == g_irq[DATA_UNCORR]) {
> + add_h = readl(l2_base + SIFIVE_L2_DATECCFAIL_HIGH);
> + add_l = readl(l2_base + SIFIVE_L2_DATECCFAIL_LOW);
> + pr_err("L2CACHE: DataFail @ 0x%08X.%08X\n", add_h, add_l);
> + regval = readl(l2_base + SIFIVE_L2_DATECCFAIL_COUNT);
> + atomic_notifier_call_chain(&l2_err_chain, SIFIVE_L2_ERR_TYPE_UE,
> + "DatECCFail");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +int __init sifive_l2_init(void)
> +{
> + struct device_node *np;
> + struct resource res;
> + int i, rc;
> +
> + np = of_find_matching_node(NULL, sifive_l2_ids);
> + if (!np)
> + return -ENODEV;
> +
> + if (of_address_to_resource(np, 0, &res))
> + return -ENODEV;
> +
> + l2_base = ioremap(res.start, resource_size(&res));
> + if (!l2_base)
> + return -ENOMEM;
> +
> + for (i = 0; i < SIFIVE_L2_MAX_ECCINTR; i++) {
> + g_irq[i] = irq_of_parse_and_map(np, i);
> + rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL);
> + if (rc) {
> + pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]);
> + return rc;
> + }
> + }
> +
> + l2_config_read();
> +
> +#ifdef CONFIG_DEBUG_FS
> + setup_sifive_debug();
> +#endif
> + return 0;
> +}
> +device_initcall(sifive_l2_init);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
2019-05-06 12:18 ` Andrew F. Davis
@ 2019-05-07 6:48 ` Yash Shah
-1 siblings, 0 replies; 19+ messages in thread
From: Yash Shah @ 2019-05-07 6:48 UTC (permalink / raw)
To: Andrew F. Davis
Cc: linux-riscv, devicetree, Palmer Dabbelt, Paul Walmsley,
linux-kernel, aou, mark.rutland, robh+dt, Sachin Ghadi
On Mon, May 6, 2019 at 5:48 PM Andrew F. Davis <afd@ti.com> wrote:
>
> On 5/6/19 6:48 AM, Yash Shah wrote:
> > The driver currently supports only SiFive FU540-C000 platform.
> >
> > The initial version of L2 cache controller driver includes:
> > - Initial configuration reporting at boot up.
> > - Support for ECC related functionality.
> >
> > Signed-off-by: Yash Shah <yash.shah@sifive.com>
> > ---
> > arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
> > arch/riscv/mm/Makefile | 1 +
> > arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
> > 3 files changed, 192 insertions(+)
> > create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
> > create mode 100644 arch/riscv/mm/sifive_l2_cache.c
> >
> > diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
> > new file mode 100644
> > index 0000000..04f6748
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/sifive_l2_cache.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * SiFive L2 Cache Controller header file
> > + *
> > + */
> > +
> > +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
> > +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
> > +
> > +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
> > +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
> > +
> > +#define SIFIVE_L2_ERR_TYPE_CE 0
> > +#define SIFIVE_L2_ERR_TYPE_UE 1
> > +
> > +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
> > diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> > index eb22ab4..1523ee5 100644
> > --- a/arch/riscv/mm/Makefile
> > +++ b/arch/riscv/mm/Makefile
> > @@ -3,3 +3,4 @@ obj-y += fault.o
> > obj-y += extable.o
> > obj-y += ioremap.o
> > obj-y += cacheflush.o
> > +obj-y += sifive_l2_cache.o
> > diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
> > new file mode 100644
> > index 0000000..4eb6461
> > --- /dev/null
> > +++ b/arch/riscv/mm/sifive_l2_cache.c
> > @@ -0,0 +1,175 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * SiFive L2 cache controller Driver
> > + *
> > + * Copyright (C) 2018-2019 SiFive, Inc.
> > + *
> > + */
[...]
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +static struct dentry *sifive_test;
> > +
> > +static ssize_t l2_write(struct file *file, const char __user *data,
> > + size_t count, loff_t *ppos)
> > +{
> > + unsigned int val;
> > +
> > + if (kstrtouint_from_user(data, count, 0, &val))
> > + return -EINVAL;
> > + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
>
> I'm guessing bit 16 is the enable and the lower 8 are some kind of
> region to enable the error? This is probably a bad interface, it looks
> useful for testing but doesn't provide any debugging info useful for
> running systems. Do you really want userspace to be able to do this?
Bit 16 selects the type of ECC error (0=data or 1=directory error).
The lower 8 bits toggles (corrupt) that bit index.
Are you suggesting to remove this debug interface altogether or you
want me to improve the current interface?
Something like providing 2 separate debugfs files for data and
directory errors. And create a separate 8-bit debugfs variable to
select the bit index to toggle.
- Yash
>
> Andrew
>
--
The information transmitted is intended only for the person or entity to
which it is addressed and may contain confidential and/or privileged
material. If you are not the intended recipient of this message please do
not read, copy, use or disclose this communication and notify the sender
immediately. It should be noted that any review, retransmission,
dissemination or other use of, or taking action or reliance upon, this
information by persons or entities other than the intended recipient is
prohibited.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
@ 2019-05-07 6:48 ` Yash Shah
0 siblings, 0 replies; 19+ messages in thread
From: Yash Shah @ 2019-05-07 6:48 UTC (permalink / raw)
To: Andrew F. Davis
Cc: mark.rutland, devicetree, aou, Palmer Dabbelt, linux-kernel,
Sachin Ghadi, robh+dt, Paul Walmsley, linux-riscv
On Mon, May 6, 2019 at 5:48 PM Andrew F. Davis <afd@ti.com> wrote:
>
> On 5/6/19 6:48 AM, Yash Shah wrote:
> > The driver currently supports only SiFive FU540-C000 platform.
> >
> > The initial version of L2 cache controller driver includes:
> > - Initial configuration reporting at boot up.
> > - Support for ECC related functionality.
> >
> > Signed-off-by: Yash Shah <yash.shah@sifive.com>
> > ---
> > arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
> > arch/riscv/mm/Makefile | 1 +
> > arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
> > 3 files changed, 192 insertions(+)
> > create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
> > create mode 100644 arch/riscv/mm/sifive_l2_cache.c
> >
> > diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
> > new file mode 100644
> > index 0000000..04f6748
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/sifive_l2_cache.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * SiFive L2 Cache Controller header file
> > + *
> > + */
> > +
> > +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
> > +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
> > +
> > +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
> > +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
> > +
> > +#define SIFIVE_L2_ERR_TYPE_CE 0
> > +#define SIFIVE_L2_ERR_TYPE_UE 1
> > +
> > +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
> > diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> > index eb22ab4..1523ee5 100644
> > --- a/arch/riscv/mm/Makefile
> > +++ b/arch/riscv/mm/Makefile
> > @@ -3,3 +3,4 @@ obj-y += fault.o
> > obj-y += extable.o
> > obj-y += ioremap.o
> > obj-y += cacheflush.o
> > +obj-y += sifive_l2_cache.o
> > diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
> > new file mode 100644
> > index 0000000..4eb6461
> > --- /dev/null
> > +++ b/arch/riscv/mm/sifive_l2_cache.c
> > @@ -0,0 +1,175 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * SiFive L2 cache controller Driver
> > + *
> > + * Copyright (C) 2018-2019 SiFive, Inc.
> > + *
> > + */
[...]
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +static struct dentry *sifive_test;
> > +
> > +static ssize_t l2_write(struct file *file, const char __user *data,
> > + size_t count, loff_t *ppos)
> > +{
> > + unsigned int val;
> > +
> > + if (kstrtouint_from_user(data, count, 0, &val))
> > + return -EINVAL;
> > + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
>
> I'm guessing bit 16 is the enable and the lower 8 are some kind of
> region to enable the error? This is probably a bad interface, it looks
> useful for testing but doesn't provide any debugging info useful for
> running systems. Do you really want userspace to be able to do this?
Bit 16 selects the type of ECC error (0=data or 1=directory error).
The lower 8 bits toggles (corrupt) that bit index.
Are you suggesting to remove this debug interface altogether or you
want me to improve the current interface?
Something like providing 2 separate debugfs files for data and
directory errors. And create a separate 8-bit debugfs variable to
select the bit index to toggle.
- Yash
>
> Andrew
>
--
The information transmitted is intended only for the person or entity to
which it is addressed and may contain confidential and/or privileged
material. If you are not the intended recipient of this message please do
not read, copy, use or disclose this communication and notify the sender
immediately. It should be noted that any review, retransmission,
dissemination or other use of, or taking action or reliance upon, this
information by persons or entities other than the intended recipient is
prohibited.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
2019-05-07 6:48 ` Yash Shah
(?)
@ 2019-05-07 13:44 ` Andrew F. Davis
-1 siblings, 0 replies; 19+ messages in thread
From: Andrew F. Davis @ 2019-05-07 13:44 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, devicetree, Palmer Dabbelt, Paul Walmsley,
linux-kernel, aou, mark.rutland, robh+dt, Sachin Ghadi
On 5/7/19 2:48 AM, Yash Shah wrote:
> On Mon, May 6, 2019 at 5:48 PM Andrew F. Davis <afd@ti.com> wrote:
>>
>> On 5/6/19 6:48 AM, Yash Shah wrote:
>>> The driver currently supports only SiFive FU540-C000 platform.
>>>
>>> The initial version of L2 cache controller driver includes:
>>> - Initial configuration reporting at boot up.
>>> - Support for ECC related functionality.
>>>
>>> Signed-off-by: Yash Shah <yash.shah@sifive.com>
>>> ---
>>> arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
>>> arch/riscv/mm/Makefile | 1 +
>>> arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
>>> 3 files changed, 192 insertions(+)
>>> create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
>>> create mode 100644 arch/riscv/mm/sifive_l2_cache.c
>>>
>>> diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
>>> new file mode 100644
>>> index 0000000..04f6748
>>> --- /dev/null
>>> +++ b/arch/riscv/include/asm/sifive_l2_cache.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * SiFive L2 Cache Controller header file
>>> + *
>>> + */
>>> +
>>> +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
>>> +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
>>> +
>>> +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
>>> +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
>>> +
>>> +#define SIFIVE_L2_ERR_TYPE_CE 0
>>> +#define SIFIVE_L2_ERR_TYPE_UE 1
>>> +
>>> +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
>>> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
>>> index eb22ab4..1523ee5 100644
>>> --- a/arch/riscv/mm/Makefile
>>> +++ b/arch/riscv/mm/Makefile
>>> @@ -3,3 +3,4 @@ obj-y += fault.o
>>> obj-y += extable.o
>>> obj-y += ioremap.o
>>> obj-y += cacheflush.o
>>> +obj-y += sifive_l2_cache.o
>>> diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
>>> new file mode 100644
>>> index 0000000..4eb6461
>>> --- /dev/null
>>> +++ b/arch/riscv/mm/sifive_l2_cache.c
>>> @@ -0,0 +1,175 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * SiFive L2 cache controller Driver
>>> + *
>>> + * Copyright (C) 2018-2019 SiFive, Inc.
>>> + *
>>> + */
> [...]
>>> +
>>> +#ifdef CONFIG_DEBUG_FS
>>> +static struct dentry *sifive_test;
>>> +
>>> +static ssize_t l2_write(struct file *file, const char __user *data,
>>> + size_t count, loff_t *ppos)
>>> +{
>>> + unsigned int val;
>>> +
>>> + if (kstrtouint_from_user(data, count, 0, &val))
>>> + return -EINVAL;
>>> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
>>
>> I'm guessing bit 16 is the enable and the lower 8 are some kind of
>> region to enable the error? This is probably a bad interface, it looks
>> useful for testing but doesn't provide any debugging info useful for
>> running systems. Do you really want userspace to be able to do this?
>
> Bit 16 selects the type of ECC error (0=data or 1=directory error).
> The lower 8 bits toggles (corrupt) that bit index.
> Are you suggesting to remove this debug interface altogether or you
> want me to improve the current interface?
> Something like providing 2 separate debugfs files for data and
> directory errors. And create a separate 8-bit debugfs variable to
> select the bit index to toggle.
>
I was suggesting to remove the whole thing. I don't see it being all
that useful, but it is up to you.
Andrew
> - Yash
>
>>
>> Andrew
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
@ 2019-05-07 13:44 ` Andrew F. Davis
0 siblings, 0 replies; 19+ messages in thread
From: Andrew F. Davis @ 2019-05-07 13:44 UTC (permalink / raw)
To: Yash Shah
Cc: mark.rutland, devicetree, aou, Palmer Dabbelt, linux-kernel,
Sachin Ghadi, robh+dt, Paul Walmsley, linux-riscv
On 5/7/19 2:48 AM, Yash Shah wrote:
> On Mon, May 6, 2019 at 5:48 PM Andrew F. Davis <afd@ti.com> wrote:
>>
>> On 5/6/19 6:48 AM, Yash Shah wrote:
>>> The driver currently supports only SiFive FU540-C000 platform.
>>>
>>> The initial version of L2 cache controller driver includes:
>>> - Initial configuration reporting at boot up.
>>> - Support for ECC related functionality.
>>>
>>> Signed-off-by: Yash Shah <yash.shah@sifive.com>
>>> ---
>>> arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
>>> arch/riscv/mm/Makefile | 1 +
>>> arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
>>> 3 files changed, 192 insertions(+)
>>> create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
>>> create mode 100644 arch/riscv/mm/sifive_l2_cache.c
>>>
>>> diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
>>> new file mode 100644
>>> index 0000000..04f6748
>>> --- /dev/null
>>> +++ b/arch/riscv/include/asm/sifive_l2_cache.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * SiFive L2 Cache Controller header file
>>> + *
>>> + */
>>> +
>>> +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
>>> +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
>>> +
>>> +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
>>> +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
>>> +
>>> +#define SIFIVE_L2_ERR_TYPE_CE 0
>>> +#define SIFIVE_L2_ERR_TYPE_UE 1
>>> +
>>> +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
>>> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
>>> index eb22ab4..1523ee5 100644
>>> --- a/arch/riscv/mm/Makefile
>>> +++ b/arch/riscv/mm/Makefile
>>> @@ -3,3 +3,4 @@ obj-y += fault.o
>>> obj-y += extable.o
>>> obj-y += ioremap.o
>>> obj-y += cacheflush.o
>>> +obj-y += sifive_l2_cache.o
>>> diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
>>> new file mode 100644
>>> index 0000000..4eb6461
>>> --- /dev/null
>>> +++ b/arch/riscv/mm/sifive_l2_cache.c
>>> @@ -0,0 +1,175 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * SiFive L2 cache controller Driver
>>> + *
>>> + * Copyright (C) 2018-2019 SiFive, Inc.
>>> + *
>>> + */
> [...]
>>> +
>>> +#ifdef CONFIG_DEBUG_FS
>>> +static struct dentry *sifive_test;
>>> +
>>> +static ssize_t l2_write(struct file *file, const char __user *data,
>>> + size_t count, loff_t *ppos)
>>> +{
>>> + unsigned int val;
>>> +
>>> + if (kstrtouint_from_user(data, count, 0, &val))
>>> + return -EINVAL;
>>> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
>>
>> I'm guessing bit 16 is the enable and the lower 8 are some kind of
>> region to enable the error? This is probably a bad interface, it looks
>> useful for testing but doesn't provide any debugging info useful for
>> running systems. Do you really want userspace to be able to do this?
>
> Bit 16 selects the type of ECC error (0=data or 1=directory error).
> The lower 8 bits toggles (corrupt) that bit index.
> Are you suggesting to remove this debug interface altogether or you
> want me to improve the current interface?
> Something like providing 2 separate debugfs files for data and
> directory errors. And create a separate 8-bit debugfs variable to
> select the bit index to toggle.
>
I was suggesting to remove the whole thing. I don't see it being all
that useful, but it is up to you.
Andrew
> - Yash
>
>>
>> Andrew
>>
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
@ 2019-05-07 13:44 ` Andrew F. Davis
0 siblings, 0 replies; 19+ messages in thread
From: Andrew F. Davis @ 2019-05-07 13:44 UTC (permalink / raw)
To: Yash Shah
Cc: linux-riscv, devicetree, Palmer Dabbelt, Paul Walmsley,
linux-kernel, aou, mark.rutland, robh+dt, Sachin Ghadi
On 5/7/19 2:48 AM, Yash Shah wrote:
> On Mon, May 6, 2019 at 5:48 PM Andrew F. Davis <afd@ti.com> wrote:
>>
>> On 5/6/19 6:48 AM, Yash Shah wrote:
>>> The driver currently supports only SiFive FU540-C000 platform.
>>>
>>> The initial version of L2 cache controller driver includes:
>>> - Initial configuration reporting at boot up.
>>> - Support for ECC related functionality.
>>>
>>> Signed-off-by: Yash Shah <yash.shah@sifive.com>
>>> ---
>>> arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
>>> arch/riscv/mm/Makefile | 1 +
>>> arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
>>> 3 files changed, 192 insertions(+)
>>> create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
>>> create mode 100644 arch/riscv/mm/sifive_l2_cache.c
>>>
>>> diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
>>> new file mode 100644
>>> index 0000000..04f6748
>>> --- /dev/null
>>> +++ b/arch/riscv/include/asm/sifive_l2_cache.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * SiFive L2 Cache Controller header file
>>> + *
>>> + */
>>> +
>>> +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
>>> +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
>>> +
>>> +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
>>> +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
>>> +
>>> +#define SIFIVE_L2_ERR_TYPE_CE 0
>>> +#define SIFIVE_L2_ERR_TYPE_UE 1
>>> +
>>> +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
>>> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
>>> index eb22ab4..1523ee5 100644
>>> --- a/arch/riscv/mm/Makefile
>>> +++ b/arch/riscv/mm/Makefile
>>> @@ -3,3 +3,4 @@ obj-y += fault.o
>>> obj-y += extable.o
>>> obj-y += ioremap.o
>>> obj-y += cacheflush.o
>>> +obj-y += sifive_l2_cache.o
>>> diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
>>> new file mode 100644
>>> index 0000000..4eb6461
>>> --- /dev/null
>>> +++ b/arch/riscv/mm/sifive_l2_cache.c
>>> @@ -0,0 +1,175 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * SiFive L2 cache controller Driver
>>> + *
>>> + * Copyright (C) 2018-2019 SiFive, Inc.
>>> + *
>>> + */
> [...]
>>> +
>>> +#ifdef CONFIG_DEBUG_FS
>>> +static struct dentry *sifive_test;
>>> +
>>> +static ssize_t l2_write(struct file *file, const char __user *data,
>>> + size_t count, loff_t *ppos)
>>> +{
>>> + unsigned int val;
>>> +
>>> + if (kstrtouint_from_user(data, count, 0, &val))
>>> + return -EINVAL;
>>> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
>>
>> I'm guessing bit 16 is the enable and the lower 8 are some kind of
>> region to enable the error? This is probably a bad interface, it looks
>> useful for testing but doesn't provide any debugging info useful for
>> running systems. Do you really want userspace to be able to do this?
>
> Bit 16 selects the type of ECC error (0=data or 1=directory error).
> The lower 8 bits toggles (corrupt) that bit index.
> Are you suggesting to remove this debug interface altogether or you
> want me to improve the current interface?
> Something like providing 2 separate debugfs files for data and
> directory errors. And create a separate 8-bit debugfs variable to
> select the bit index to toggle.
>
I was suggesting to remove the whole thing. I don't see it being all
that useful, but it is up to you.
Andrew
> - Yash
>
>>
>> Andrew
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
2019-05-07 13:44 ` Andrew F. Davis
@ 2019-05-08 5:49 ` Yash Shah
-1 siblings, 0 replies; 19+ messages in thread
From: Yash Shah @ 2019-05-08 5:49 UTC (permalink / raw)
To: Andrew F. Davis
Cc: linux-riscv, devicetree, Palmer Dabbelt, Paul Walmsley,
linux-kernel, aou, mark.rutland, robh+dt, Sachin Ghadi
On Tue, May 7, 2019 at 7:15 PM Andrew F. Davis <afd@ti.com> wrote:
>
> On 5/7/19 2:48 AM, Yash Shah wrote:
> > On Mon, May 6, 2019 at 5:48 PM Andrew F. Davis <afd@ti.com> wrote:
> >>
> >> On 5/6/19 6:48 AM, Yash Shah wrote:
> >>> The driver currently supports only SiFive FU540-C000 platform.
> >>>
> >>> The initial version of L2 cache controller driver includes:
> >>> - Initial configuration reporting at boot up.
> >>> - Support for ECC related functionality.
> >>>
> >>> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> >>> ---
> >>> arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
> >>> arch/riscv/mm/Makefile | 1 +
> >>> arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
> >>> 3 files changed, 192 insertions(+)
> >>> create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
> >>> create mode 100644 arch/riscv/mm/sifive_l2_cache.c
> >>>
> >>> diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
> >>> new file mode 100644
> >>> index 0000000..04f6748
> >>> --- /dev/null
> >>> +++ b/arch/riscv/include/asm/sifive_l2_cache.h
> >>> @@ -0,0 +1,16 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +/*
> >>> + * SiFive L2 Cache Controller header file
> >>> + *
> >>> + */
> >>> +
> >>> +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
> >>> +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
> >>> +
> >>> +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
> >>> +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
> >>> +
> >>> +#define SIFIVE_L2_ERR_TYPE_CE 0
> >>> +#define SIFIVE_L2_ERR_TYPE_UE 1
> >>> +
> >>> +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
> >>> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> >>> index eb22ab4..1523ee5 100644
> >>> --- a/arch/riscv/mm/Makefile
> >>> +++ b/arch/riscv/mm/Makefile
> >>> @@ -3,3 +3,4 @@ obj-y += fault.o
> >>> obj-y += extable.o
> >>> obj-y += ioremap.o
> >>> obj-y += cacheflush.o
> >>> +obj-y += sifive_l2_cache.o
> >>> diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
> >>> new file mode 100644
> >>> index 0000000..4eb6461
> >>> --- /dev/null
> >>> +++ b/arch/riscv/mm/sifive_l2_cache.c
> >>> @@ -0,0 +1,175 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * SiFive L2 cache controller Driver
> >>> + *
> >>> + * Copyright (C) 2018-2019 SiFive, Inc.
> >>> + *
> >>> + */
> > [...]
> >>> +
> >>> +#ifdef CONFIG_DEBUG_FS
> >>> +static struct dentry *sifive_test;
> >>> +
> >>> +static ssize_t l2_write(struct file *file, const char __user *data,
> >>> + size_t count, loff_t *ppos)
> >>> +{
> >>> + unsigned int val;
> >>> +
> >>> + if (kstrtouint_from_user(data, count, 0, &val))
> >>> + return -EINVAL;
> >>> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
> >>
> >> I'm guessing bit 16 is the enable and the lower 8 are some kind of
> >> region to enable the error? This is probably a bad interface, it looks
> >> useful for testing but doesn't provide any debugging info useful for
> >> running systems. Do you really want userspace to be able to do this?
> >
> > Bit 16 selects the type of ECC error (0=data or 1=directory error).
> > The lower 8 bits toggles (corrupt) that bit index.
> > Are you suggesting to remove this debug interface altogether or you
> > want me to improve the current interface?
> > Something like providing 2 separate debugfs files for data and
> > directory errors. And create a separate 8-bit debugfs variable to
> > select the bit index to toggle.
> >
>
> I was suggesting to remove the whole thing. I don't see it being all
> that useful, but it is up to you.
Thanks for the suggestion, but I will keep it as we do need it for our testing.
- Yash
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/2] RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs
@ 2019-05-08 5:49 ` Yash Shah
0 siblings, 0 replies; 19+ messages in thread
From: Yash Shah @ 2019-05-08 5:49 UTC (permalink / raw)
To: Andrew F. Davis
Cc: mark.rutland, devicetree, aou, Palmer Dabbelt, linux-kernel,
Sachin Ghadi, robh+dt, Paul Walmsley, linux-riscv
On Tue, May 7, 2019 at 7:15 PM Andrew F. Davis <afd@ti.com> wrote:
>
> On 5/7/19 2:48 AM, Yash Shah wrote:
> > On Mon, May 6, 2019 at 5:48 PM Andrew F. Davis <afd@ti.com> wrote:
> >>
> >> On 5/6/19 6:48 AM, Yash Shah wrote:
> >>> The driver currently supports only SiFive FU540-C000 platform.
> >>>
> >>> The initial version of L2 cache controller driver includes:
> >>> - Initial configuration reporting at boot up.
> >>> - Support for ECC related functionality.
> >>>
> >>> Signed-off-by: Yash Shah <yash.shah@sifive.com>
> >>> ---
> >>> arch/riscv/include/asm/sifive_l2_cache.h | 16 +++
> >>> arch/riscv/mm/Makefile | 1 +
> >>> arch/riscv/mm/sifive_l2_cache.c | 175 +++++++++++++++++++++++++++++++
> >>> 3 files changed, 192 insertions(+)
> >>> create mode 100644 arch/riscv/include/asm/sifive_l2_cache.h
> >>> create mode 100644 arch/riscv/mm/sifive_l2_cache.c
> >>>
> >>> diff --git a/arch/riscv/include/asm/sifive_l2_cache.h b/arch/riscv/include/asm/sifive_l2_cache.h
> >>> new file mode 100644
> >>> index 0000000..04f6748
> >>> --- /dev/null
> >>> +++ b/arch/riscv/include/asm/sifive_l2_cache.h
> >>> @@ -0,0 +1,16 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +/*
> >>> + * SiFive L2 Cache Controller header file
> >>> + *
> >>> + */
> >>> +
> >>> +#ifndef _ASM_RISCV_SIFIVE_L2_CACHE_H
> >>> +#define _ASM_RISCV_SIFIVE_L2_CACHE_H
> >>> +
> >>> +extern int register_sifive_l2_error_notifier(struct notifier_block *nb);
> >>> +extern int unregister_sifive_l2_error_notifier(struct notifier_block *nb);
> >>> +
> >>> +#define SIFIVE_L2_ERR_TYPE_CE 0
> >>> +#define SIFIVE_L2_ERR_TYPE_UE 1
> >>> +
> >>> +#endif /* _ASM_RISCV_SIFIVE_L2_CACHE_H */
> >>> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> >>> index eb22ab4..1523ee5 100644
> >>> --- a/arch/riscv/mm/Makefile
> >>> +++ b/arch/riscv/mm/Makefile
> >>> @@ -3,3 +3,4 @@ obj-y += fault.o
> >>> obj-y += extable.o
> >>> obj-y += ioremap.o
> >>> obj-y += cacheflush.o
> >>> +obj-y += sifive_l2_cache.o
> >>> diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
> >>> new file mode 100644
> >>> index 0000000..4eb6461
> >>> --- /dev/null
> >>> +++ b/arch/riscv/mm/sifive_l2_cache.c
> >>> @@ -0,0 +1,175 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * SiFive L2 cache controller Driver
> >>> + *
> >>> + * Copyright (C) 2018-2019 SiFive, Inc.
> >>> + *
> >>> + */
> > [...]
> >>> +
> >>> +#ifdef CONFIG_DEBUG_FS
> >>> +static struct dentry *sifive_test;
> >>> +
> >>> +static ssize_t l2_write(struct file *file, const char __user *data,
> >>> + size_t count, loff_t *ppos)
> >>> +{
> >>> + unsigned int val;
> >>> +
> >>> + if (kstrtouint_from_user(data, count, 0, &val))
> >>> + return -EINVAL;
> >>> + if ((val >= 0 && val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
> >>
> >> I'm guessing bit 16 is the enable and the lower 8 are some kind of
> >> region to enable the error? This is probably a bad interface, it looks
> >> useful for testing but doesn't provide any debugging info useful for
> >> running systems. Do you really want userspace to be able to do this?
> >
> > Bit 16 selects the type of ECC error (0=data or 1=directory error).
> > The lower 8 bits toggles (corrupt) that bit index.
> > Are you suggesting to remove this debug interface altogether or you
> > want me to improve the current interface?
> > Something like providing 2 separate debugfs files for data and
> > directory errors. And create a separate 8-bit debugfs variable to
> > select the bit index to toggle.
> >
>
> I was suggesting to remove the whole thing. I don't see it being all
> that useful, but it is up to you.
Thanks for the suggestion, but I will keep it as we do need it for our testing.
- Yash
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 19+ messages in thread