Re: [PATCH v5 4/4] ARM: Add support for Hisilicon Kunpeng L3 cache controller
From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-01-28 14:26:08
Also in:
linux-devicetree, lkml
On Sat, Jan 16, 2021 at 4:27 AM Zhen Lei [off-list ref] wrote:
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile + +static void l3cache_maint_common(u32 range, u32 op_type) +{ + u32 reg; + + reg = readl(l3_ctrl_base + L3_MAINT_CTRL); + reg &= ~(L3_MAINT_RANGE_MASK | L3_MAINT_TYPE_MASK); + reg |= range | op_type; + reg |= L3_MAINT_STATUS_START; + writel(reg, l3_ctrl_base + L3_MAINT_CTRL);
Are there contents of L3_MAINT_CTRL that need to be preserved across calls and can not be inferred? A 'readl()' is often expensive, so it might be more efficient if you can avoid that.
+static inline void l3cache_flush_all_nolock(void)
+{
+ l3cache_maint_common(L3_MAINT_RANGE_ALL, L3_MAINT_TYPE_FLUSH);
+}
+
+static void l3cache_flush_all(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&l3cache_lock, flags);
+ l3cache_flush_all_nolock();
+ spin_unlock_irqrestore(&l3cache_lock, flags);
+}I see that cache-l2x0 uses raw_spin_lock_irqsave() instead of spin_lock_irqsave(), to avoid preemption in the middle of a cache operation. This is probably a good idea here as well. I also see that l2x0 uses readl_relaxed(), to avoid a deadlock in l2x0_cache_sync(). This may also be beneficial for performance reasons, so it might be helpful to compare performance overhead. On the other hand, readl()/writel() are usually the safe choice, as those avoid the need to argue over whether the relaxed versions are safe in all corner cases.
+static int __init l3cache_init(void)
+{
+ u32 reg;
+ struct device_node *node;
+
+ node = of_find_matching_node(NULL, l3cache_ids);
+ if (!node)
+ return -ENODEV;I think the initcall should return '0' to indicate success when running a kernel with this driver built-in on a platform that does not have this device.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/mm/cache-kunpeng-l3.h b/arch/arm/mm/cache-kunpeng-l3.h new file mode 100644 index 000000000000000..9ef6a53e7d4db49 --- /dev/null +++ b/arch/arm/mm/cache-kunpeng-l3.h@@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __CACHE_KUNPENG_L3_H +#define __CACHE_KUNPENG_L3_H + +#define L3_CACHE_LINE_SHITF 6
I would suggest moving the contents of the header file into the .c file,
since there is only a single user of these macros.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel