2024-04-10 23:22:50

by Samuel Holland

[permalink] [raw]
Subject: [PATCH 0/4] cache: sifive_ccache: Auxiliary device support

As of commit c90847bcbfb6 ("cache: sifive_ccache: Partially convert to a
platform driver"), the cache subsystem binds a platform driver to the
Composable Cache's DT node. This prevents the perf subsystem from doing
the same for the new PMU driver[1]. To allow using both drivers at the
same time without conflicts or module linkage dependencies, attach the
PMU driver to the auxiliary device bus. While at it, prepare to use the
auxiliary device bus for the EDAC driver as well, which significantly
simplifies that driver. The actual EDAC driver conversion has to wait
another development cycle to avoid dependencies between git trees.

[1]: https://lore.kernel.org/linux-riscv/[email protected]/


Samuel Holland (4):
cache: sifive_ccache: Silence unused variable warning
cache: sifive_ccache: Use of_iomap() helper
cache: sifive_ccache: Export base address for child drivers
cache: sifive_ccache: Add EDAC and PMU as auxiliary devices

drivers/cache/Kconfig | 1 +
drivers/cache/sifive_ccache.c | 94 +++++++++++++++++++++---------
include/soc/sifive/sifive_ccache.h | 8 +++
3 files changed, 74 insertions(+), 29 deletions(-)

--
2.44.0



2024-04-10 23:23:02

by Samuel Holland

[permalink] [raw]
Subject: [PATCH 2/4] cache: sifive_ccache: Use of_iomap() helper

This simplifies the code with no functional change.

Signed-off-by: Samuel Holland <[email protected]>
---

drivers/cache/sifive_ccache.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/cache/sifive_ccache.c b/drivers/cache/sifive_ccache.c
index 6874b72ec59d..9833e9f6f36c 100644
--- a/drivers/cache/sifive_ccache.c
+++ b/drivers/cache/sifive_ccache.c
@@ -288,7 +288,6 @@ static struct platform_driver sifive_ccache_driver = {
static int __init sifive_ccache_init(void)
{
struct device_node *np;
- struct resource res;
const struct of_device_id *match;
unsigned long quirks __maybe_unused;
int rc;
@@ -299,12 +298,7 @@ static int __init sifive_ccache_init(void)

quirks = (uintptr_t)match->data;

- if (of_address_to_resource(np, 0, &res)) {
- rc = -ENODEV;
- goto err_node_put;
- }
-
- ccache_base = ioremap(res.start, resource_size(&res));
+ ccache_base = of_iomap(np, 0);
if (!ccache_base) {
rc = -ENOMEM;
goto err_node_put;
--
2.44.0


2024-04-10 23:23:12

by Samuel Holland

[permalink] [raw]
Subject: [PATCH 3/4] cache: sifive_ccache: Export base address for child drivers

The SiFive Composable Cache functionality is split across multiple
drivers, including the base cache driver, the EDAC driver, and a
PMU driver. Export the MMIO base address from the main driver so the
child drivers can access the MMIO space. Since each driver uses a
different subset of registers, there are no synchronization concerns.

Signed-off-by: Samuel Holland <[email protected]>
---

drivers/cache/sifive_ccache.c | 44 +++++++++++++++---------------
include/soc/sifive/sifive_ccache.h | 4 +++
2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/cache/sifive_ccache.c b/drivers/cache/sifive_ccache.c
index 9833e9f6f36c..42dac39c41cf 100644
--- a/drivers/cache/sifive_ccache.c
+++ b/drivers/cache/sifive_ccache.c
@@ -53,7 +53,7 @@
#define SIFIVE_CCACHE_MAX_ECCINTR 4
#define SIFIVE_CCACHE_LINE_SIZE 64

-static void __iomem *ccache_base;
+static struct sifive_ccache ccache;
static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
static struct riscv_cacheinfo_ops ccache_cache_ops;
static int level;
@@ -81,7 +81,7 @@ static ssize_t ccache_write(struct file *file, const char __user *data,
if (kstrtouint_from_user(data, count, 0, &val))
return -EINVAL;
if ((val < 0xFF) || (val >= 0x10000 && val < 0x100FF))
- writel(val, ccache_base + SIFIVE_CCACHE_ECCINJECTERR);
+ writel(val, ccache.base + SIFIVE_CCACHE_ECCINJECTERR);
else
return -EINVAL;
return count;
@@ -106,14 +106,14 @@ static void ccache_config_read(void)
{
u32 cfg;

- cfg = readl(ccache_base + SIFIVE_CCACHE_CONFIG);
+ cfg = readl(ccache.base + SIFIVE_CCACHE_CONFIG);
pr_info("%llu banks, %llu ways, sets/bank=%llu, bytes/block=%llu\n",
FIELD_GET(SIFIVE_CCACHE_CONFIG_BANK_MASK, cfg),
FIELD_GET(SIFIVE_CCACHE_CONFIG_WAYS_MASK, cfg),
BIT_ULL(FIELD_GET(SIFIVE_CCACHE_CONFIG_SETS_MASK, cfg)),
BIT_ULL(FIELD_GET(SIFIVE_CCACHE_CONFIG_BLKS_MASK, cfg)));

- cfg = readl(ccache_base + SIFIVE_CCACHE_WAYENABLE);
+ cfg = readl(ccache.base + SIFIVE_CCACHE_WAYENABLE);
pr_info("Index of the largest way enabled: %u\n", cfg);
}

@@ -153,9 +153,9 @@ static void ccache_flush_range(phys_addr_t start, size_t len)
for (line = ALIGN_DOWN(start, SIFIVE_CCACHE_LINE_SIZE); line < end;
line += SIFIVE_CCACHE_LINE_SIZE) {
#ifdef CONFIG_32BIT
- writel(line >> 4, ccache_base + SIFIVE_CCACHE_FLUSH32);
+ writel(line >> 4, ccache.base + SIFIVE_CCACHE_FLUSH32);
#else
- writeq(line, ccache_base + SIFIVE_CCACHE_FLUSH64);
+ writeq(line, ccache.base + SIFIVE_CCACHE_FLUSH64);
#endif
mb();
}
@@ -170,7 +170,7 @@ static const struct riscv_nonstd_cache_ops ccache_mgmt_ops __initconst = {

static int ccache_largest_wayenabled(void)
{
- return readl(ccache_base + SIFIVE_CCACHE_WAYENABLE) & 0xFF;
+ return readl(ccache.base + SIFIVE_CCACHE_WAYENABLE) & 0xFF;
}

static ssize_t number_of_ways_enabled_show(struct device *dev,
@@ -206,41 +206,41 @@ static irqreturn_t ccache_int_handler(int irq, void *device)
unsigned int add_h, add_l;

if (irq == g_irq[DIR_CORR]) {
- add_h = readl(ccache_base + SIFIVE_CCACHE_DIRECCFIX_HIGH);
- add_l = readl(ccache_base + SIFIVE_CCACHE_DIRECCFIX_LOW);
+ add_h = readl(ccache.base + SIFIVE_CCACHE_DIRECCFIX_HIGH);
+ add_l = readl(ccache.base + SIFIVE_CCACHE_DIRECCFIX_LOW);
pr_err("DirError @ 0x%08X.%08X\n", add_h, add_l);
/* Reading this register clears the DirError interrupt sig */
- readl(ccache_base + SIFIVE_CCACHE_DIRECCFIX_COUNT);
+ readl(ccache.base + SIFIVE_CCACHE_DIRECCFIX_COUNT);
atomic_notifier_call_chain(&ccache_err_chain,
SIFIVE_CCACHE_ERR_TYPE_CE,
"DirECCFix");
}
if (irq == g_irq[DIR_UNCORR]) {
- add_h = readl(ccache_base + SIFIVE_CCACHE_DIRECCFAIL_HIGH);
- add_l = readl(ccache_base + SIFIVE_CCACHE_DIRECCFAIL_LOW);
+ add_h = readl(ccache.base + SIFIVE_CCACHE_DIRECCFAIL_HIGH);
+ add_l = readl(ccache.base + SIFIVE_CCACHE_DIRECCFAIL_LOW);
/* Reading this register clears the DirFail interrupt sig */
- readl(ccache_base + SIFIVE_CCACHE_DIRECCFAIL_COUNT);
+ readl(ccache.base + SIFIVE_CCACHE_DIRECCFAIL_COUNT);
atomic_notifier_call_chain(&ccache_err_chain,
SIFIVE_CCACHE_ERR_TYPE_UE,
"DirECCFail");
panic("CCACHE: DirFail @ 0x%08X.%08X\n", add_h, add_l);
}
if (irq == g_irq[DATA_CORR]) {
- add_h = readl(ccache_base + SIFIVE_CCACHE_DATECCFIX_HIGH);
- add_l = readl(ccache_base + SIFIVE_CCACHE_DATECCFIX_LOW);
+ add_h = readl(ccache.base + SIFIVE_CCACHE_DATECCFIX_HIGH);
+ add_l = readl(ccache.base + SIFIVE_CCACHE_DATECCFIX_LOW);
pr_err("DataError @ 0x%08X.%08X\n", add_h, add_l);
/* Reading this register clears the DataError interrupt sig */
- readl(ccache_base + SIFIVE_CCACHE_DATECCFIX_COUNT);
+ readl(ccache.base + SIFIVE_CCACHE_DATECCFIX_COUNT);
atomic_notifier_call_chain(&ccache_err_chain,
SIFIVE_CCACHE_ERR_TYPE_CE,
"DatECCFix");
}
if (irq == g_irq[DATA_UNCORR]) {
- add_h = readl(ccache_base + SIFIVE_CCACHE_DATECCFAIL_HIGH);
- add_l = readl(ccache_base + SIFIVE_CCACHE_DATECCFAIL_LOW);
+ add_h = readl(ccache.base + SIFIVE_CCACHE_DATECCFAIL_HIGH);
+ add_l = readl(ccache.base + SIFIVE_CCACHE_DATECCFAIL_LOW);
pr_err("DataFail @ 0x%08X.%08X\n", add_h, add_l);
/* Reading this register clears the DataFail interrupt sig */
- readl(ccache_base + SIFIVE_CCACHE_DATECCFAIL_COUNT);
+ readl(ccache.base + SIFIVE_CCACHE_DATECCFAIL_COUNT);
atomic_notifier_call_chain(&ccache_err_chain,
SIFIVE_CCACHE_ERR_TYPE_UE,
"DatECCFail");
@@ -298,8 +298,8 @@ static int __init sifive_ccache_init(void)

quirks = (uintptr_t)match->data;

- ccache_base = of_iomap(np, 0);
- if (!ccache_base) {
+ ccache.base = of_iomap(np, 0);
+ if (!ccache.base) {
rc = -ENOMEM;
goto err_node_put;
}
@@ -335,7 +335,7 @@ static int __init sifive_ccache_init(void)
return 0;

err_unmap:
- iounmap(ccache_base);
+ iounmap(ccache.base);
err_node_put:
of_node_put(np);
return rc;
diff --git a/include/soc/sifive/sifive_ccache.h b/include/soc/sifive/sifive_ccache.h
index 4d4ed49388a0..85fd1ff1355a 100644
--- a/include/soc/sifive/sifive_ccache.h
+++ b/include/soc/sifive/sifive_ccache.h
@@ -7,6 +7,10 @@
#ifndef __SOC_SIFIVE_CCACHE_H
#define __SOC_SIFIVE_CCACHE_H

+struct sifive_ccache {
+ void __iomem *base;
+};
+
extern int register_sifive_ccache_error_notifier(struct notifier_block *nb);
extern int unregister_sifive_ccache_error_notifier(struct notifier_block *nb);

--
2.44.0


2024-04-10 23:23:32

by Samuel Holland

[permalink] [raw]
Subject: [PATCH 4/4] cache: sifive_ccache: Add EDAC and PMU as auxiliary devices

This allows the child drivers to access the Composable Cache device's
MMIO space and ties them to the lifecycle of the main platform device.

Currently, the EDAC driver is probed regardless of whether a Composable
Cache device actually exists on the system. Once converted to use the
auxiliary device bus, it will only be probed when needed.

Signed-off-by: Samuel Holland <[email protected]>
---

drivers/cache/Kconfig | 1 +
drivers/cache/sifive_ccache.c | 42 ++++++++++++++++++++++++++++++
include/soc/sifive/sifive_ccache.h | 4 +++
3 files changed, 47 insertions(+)

diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig
index 9345ce4976d7..08e0415eba80 100644
--- a/drivers/cache/Kconfig
+++ b/drivers/cache/Kconfig
@@ -11,6 +11,7 @@ config AX45MP_L2_CACHE
config SIFIVE_CCACHE
bool "Sifive Composable Cache controller"
depends on ARCH_SIFIVE || ARCH_STARFIVE
+ select AUXILIARY_BUS
help
Support for the composable cache controller on SiFive platforms.

diff --git a/drivers/cache/sifive_ccache.c b/drivers/cache/sifive_ccache.c
index 42dac39c41cf..c7aa6f360967 100644
--- a/drivers/cache/sifive_ccache.c
+++ b/drivers/cache/sifive_ccache.c
@@ -9,6 +9,7 @@
#define pr_fmt(fmt) "CCACHE: " fmt

#include <linux/align.h>
+#include <linux/auxiliary_bus.h>
#include <linux/debugfs.h>
#include <linux/interrupt.h>
#include <linux/of_irq.h>
@@ -249,6 +250,39 @@ static irqreturn_t ccache_int_handler(int irq, void *device)
return IRQ_HANDLED;
}

+static void sifive_ccache_del_aux_dev(void *adev)
+{
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+static int sifive_ccache_add_aux_dev(struct device *dev, struct auxiliary_device *adev,
+ const char *name)
+{
+ int rc;
+
+ adev->dev.parent = dev;
+ adev->name = name;
+
+ rc = auxiliary_device_init(adev);
+ if (rc)
+ return rc;
+
+ rc = auxiliary_device_add(adev);
+ if (rc)
+ goto err_uninit;
+
+ rc = devm_add_action_or_reset(dev, sifive_ccache_del_aux_dev, adev);
+ if (rc)
+ return rc;
+
+ return 0;
+
+err_uninit:
+ auxiliary_device_uninit(adev);
+ return rc;
+}
+
static int sifive_ccache_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -274,6 +308,14 @@ static int sifive_ccache_probe(struct platform_device *pdev)
return dev_err_probe(dev, rc, "Could not request IRQ %d\n", g_irq[i]);
}

+ rc = sifive_ccache_add_aux_dev(dev, &ccache.edac_dev, "edac");
+ if (rc)
+ return rc;
+
+ rc = sifive_ccache_add_aux_dev(dev, &ccache.pmu_dev, "pmu");
+ if (rc)
+ return rc;
+
return 0;
}

diff --git a/include/soc/sifive/sifive_ccache.h b/include/soc/sifive/sifive_ccache.h
index 85fd1ff1355a..034dc8b6e4e4 100644
--- a/include/soc/sifive/sifive_ccache.h
+++ b/include/soc/sifive/sifive_ccache.h
@@ -7,8 +7,12 @@
#ifndef __SOC_SIFIVE_CCACHE_H
#define __SOC_SIFIVE_CCACHE_H

+#include <linux/auxiliary_bus.h>
+
struct sifive_ccache {
void __iomem *base;
+ struct auxiliary_device edac_dev;
+ struct auxiliary_device pmu_dev;
};

extern int register_sifive_ccache_error_notifier(struct notifier_block *nb);
--
2.44.0


2024-04-12 12:53:27

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 0/4] cache: sifive_ccache: Auxiliary device support

On Wed, Apr 10, 2024 at 04:22:02PM -0700, Samuel Holland wrote:
> As of commit c90847bcbfb6 ("cache: sifive_ccache: Partially convert to a
> platform driver"), the cache subsystem binds a platform driver to the
> Composable Cache's DT node. This prevents the perf subsystem from doing
> the same for the new PMU driver[1]. To allow using both drivers at the
> same time without conflicts or module linkage dependencies, attach the
> PMU driver to the auxiliary device bus. While at it, prepare to use the
> auxiliary device bus for the EDAC driver as well, which significantly
> simplifies that driver. The actual EDAC driver conversion has to wait
> another development cycle to avoid dependencies between git trees.


I'm not really keen on the partial conversion, I'd like to see a
complete conversion where the ccache driver calls
"sifive_register_ccache_pmu()" and "sifive_register_ccache_edac()"
and has no part in creating the aux device itself, like Philipp and
Stephen asked me to do here for the clock/reset drivers on PolarFire SoC:
https://lore.kernel.org/all/20240409-shallow-voice-c84ed791bc7d@spud/

Thanks,
Conor.


Attachments:
(No filename) (1.12 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-24 20:41:19

by Alexandre Ghiti

[permalink] [raw]
Subject: Re: [PATCH 0/4] cache: sifive_ccache: Auxiliary device support

Hi Samuel,

On 11/04/2024 01:22, Samuel Holland wrote:
> As of commit c90847bcbfb6 ("cache: sifive_ccache: Partially convert to a
> platform driver"), the cache subsystem binds a platform driver to the
> Composable Cache's DT node. This prevents the perf subsystem from doing
> the same for the new PMU driver[1]. To allow using both drivers at the
> same time without conflicts or module linkage dependencies, attach the
> PMU driver to the auxiliary device bus. While at it, prepare to use the
> auxiliary device bus for the EDAC driver as well, which significantly
> simplifies that driver. The actual EDAC driver conversion has to wait
> another development cycle to avoid dependencies between git trees.
>
> [1]: https://lore.kernel.org/linux-riscv/[email protected]/
>
>
> Samuel Holland (4):
> cache: sifive_ccache: Silence unused variable warning
> cache: sifive_ccache: Use of_iomap() helper
> cache: sifive_ccache: Export base address for child drivers
> cache: sifive_ccache: Add EDAC and PMU as auxiliary devices
>
> drivers/cache/Kconfig | 1 +
> drivers/cache/sifive_ccache.c | 94 +++++++++++++++++++++---------
> include/soc/sifive/sifive_ccache.h | 8 +++
> 3 files changed, 74 insertions(+), 29 deletions(-)
>

I took a quick look and I'm not sure if the whole patchset is a fix for
6.9 or if only patch 1 is?

Alex


2024-04-24 21:25:54

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 0/4] cache: sifive_ccache: Auxiliary device support

On Wed, Apr 24, 2024 at 10:19:18PM +0200, Alexandre Ghiti wrote:
> Hi Samuel,
>
> On 11/04/2024 01:22, Samuel Holland wrote:
> > As of commit c90847bcbfb6 ("cache: sifive_ccache: Partially convert to a
> > platform driver"), the cache subsystem binds a platform driver to the
> > Composable Cache's DT node. This prevents the perf subsystem from doing
> > the same for the new PMU driver[1]. To allow using both drivers at the
> > same time without conflicts or module linkage dependencies, attach the
> > PMU driver to the auxiliary device bus. While at it, prepare to use the
> > auxiliary device bus for the EDAC driver as well, which significantly
> > simplifies that driver. The actual EDAC driver conversion has to wait
> > another development cycle to avoid dependencies between git trees.
> >
> > [1]: https://lore.kernel.org/linux-riscv/[email protected]/
> >
> >
> > Samuel Holland (4):
> > cache: sifive_ccache: Silence unused variable warning
> > cache: sifive_ccache: Use of_iomap() helper
> > cache: sifive_ccache: Export base address for child drivers
> > cache: sifive_ccache: Add EDAC and PMU as auxiliary devices
> >
> > drivers/cache/Kconfig | 1 +
> > drivers/cache/sifive_ccache.c | 94 +++++++++++++++++++++---------
> > include/soc/sifive/sifive_ccache.h | 8 +++
> > 3 files changed, 74 insertions(+), 29 deletions(-)
> >
>
> I took a quick look and I'm not sure if the whole patchset is a fix for 6.9
> or if only patch 1 is?

Only the first patch is IMO, but it is a fix for a patch I applied so
it'll go this week via the soc tree with those auto-update fixes.


Attachments:
(No filename) (1.66 kB)
signature.asc (235.00 B)
Download all attachments