Hi Dinh,

On Fri, 8 Mar 2019 at 09:17, Dinh Nguyen dinguyen@kernel.org wrote:

The cache controller driver configures the cache settings that can be found in the device tree files.

This initial revision only configures basic settings(data & instruction prefetch, shared-override, data & tag latency). I believe these are the settings that affect performance the most. Comprehensive settings can be done by the OS.

Signed-off-by: Dinh Nguyen dinguyen@kernel.org

drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/cache/Kconfig | 22 ++++++++++ drivers/cache/Makefile | 3 ++ drivers/cache/cache-l2x0.c | 82 ++++++++++++++++++++++++++++++++++++

This looks like a driver, rather than the uclass itself, so should go in a separate patch.

drivers/cache/cache-uclass.c | 13 ++++++ include/dm/uclass-id.h | 1 + 7 files changed, 124 insertions(+) create mode 100644 drivers/cache/Kconfig create mode 100644 drivers/cache/Makefile create mode 100644 drivers/cache/cache-l2x0.c create mode 100644 drivers/cache/cache-uclass.c

Also please add a sandbox cache driver and some tests in test/dm/cache.c

diff --git a/drivers/Kconfig b/drivers/Kconfig index f24351ac4f..842201b753 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -14,6 +14,8 @@ source "drivers/block/Kconfig"

source "drivers/bootcount/Kconfig"

+source "drivers/cache/Kconfig"

source "drivers/clk/Kconfig"

source "drivers/cpu/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index a7bba3ed56..0a00096332 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -77,6 +77,7 @@ obj-$(CONFIG_BIOSEMU) += bios_emulator/ obj-y += block/ obj-y += board/ obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/ +obj-y += cache/ obj-$(CONFIG_CPU) += cpu/ obj-y += crypto/ obj-$(CONFIG_FASTBOOT) += fastboot/ diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig new file mode 100644 index 0000000000..d6b2b6762a --- /dev/null +++ b/drivers/cache/Kconfig @@ -0,0 +1,22 @@ +# +# Cache controllers +#

+menu "Cache Controller drivers"

+config CACHE

•   bool "Enable Driver Model for Cache drivers"
  

•   depends on DM
  

•   help
  

•     Enable driver model for cache controllers.
  

Please add more documentation here. What exactly is a cache controller (for CPU?) and what can you control with it?

Normally the uclass documentation goes in its header file. Since you don't have one, you could put it in the uclass file. You should mention that the settings can come from the DT and that there are no uclass operations supported, only setting up the cache.

+config L2X0_CACHE

•   tristate "PL310 cache driver"
  

•   select CACHE
  

•   depends on ARM
  

•   help
  

•     This driver is for the PL310 cache controller commonly found on
  

•     ARMv7(32-bit) devices. The driver configures the cache settings
  

•     found in the device tree.
  

Again this should be in another patch.

+endmenu diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile new file mode 100644 index 0000000000..fca37de0a8 --- /dev/null +++ b/drivers/cache/Makefile @@ -0,0 +1,3 @@

+obj-$(CONFIG_CACHE) += cache-uclass.o +obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o diff --git a/drivers/cache/cache-l2x0.c b/drivers/cache/cache-l2x0.c new file mode 100644 index 0000000000..cdd6ddb59b --- /dev/null +++ b/drivers/cache/cache-l2x0.c @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Copyright (C) 2019 Intel Corporation <www.intel.com>
• */

+#include <common.h> +#include <command.h> +#include <dm.h>

+#include <asm/io.h> +#include <asm/pl310.h>

+static void l2c310_of_parse(struct udevice *dev) +{

•   u32 tag[3] = { 0, 0, 0 };
  

•   u32 saved_reg, prefetch;
  

•   int ret;
  

•   struct pl310_regs *regs = (struct pl310_regs *)devfdt_get_addr(dev);
  

This should normally be read (perhaps into struct l2c310_priv *) in l2x0_ofdata_to_platdata().

However in this case you don't need the setting later, so it seems OK to do this. Please use dev_read_...() API always (not devfdt which doesn't work on livetree).

•   /*Disable the L2 Cache */
  

Space after /*

•   clrbits_le32(&regs->pl310_ctrl, L2X0_CTRL_EN);
  

•   saved_reg = readl(&regs->pl310_aux_ctrl);
  

•   if (dev_read_u32(dev, "prefetch-data", &prefetch) == 0) {
  

I'm not sure about your error handling here. Are these properties optional? Is there a binding file you can bring in from Linux?

If these are just optional, then this is OK, but please:

if (!dev_read...())

•           if (prefetch)
  

•                   saved_reg |= L310_AUX_CTRL_DATA_PREFETCH_MASK;
  

•           else
  

•                   saved_reg &= ~L310_AUX_CTRL_DATA_PREFETCH_MASK;
  

•   }
  

•   if (dev_read_u32(dev, "prefetch-instr", &prefetch) == 0) {
  

•           if (prefetch)
  

•                   saved_reg |= L310_AUX_CTRL_INST_PREFETCH_MASK;
  

•           else
  

•                   saved_reg &= ~L310_AUX_CTRL_INST_PREFETCH_MASK;
  

•   }
  

•   saved_reg |= dev_read_bool(dev, "arm,shared-override");
  

•   writel(saved_reg, &regs->pl310_aux_ctrl);
  

•   saved_reg = readl(&regs->pl310_tag_latency_ctrl);
  

•   if (dev_read_u32_array(dev, "arm,tag-latency", tag, 3) == 0)
  

•           saved_reg |= L310_LATENCY_CTRL_RD(tag[0] - 1) |
  

•                        L310_LATENCY_CTRL_WR(tag[1] - 1) |
  

•                        L310_LATENCY_CTRL_SETUP(tag[2] - 1);
  

•   writel(saved_reg, &regs->pl310_tag_latency_ctrl);
  

•   saved_reg = readl(&regs->pl310_data_latency_ctrl);
  

•   if (dev_read_u32_array(dev, "arm,data-latency", tag, 3) == 0)
  

•           saved_reg |= L310_LATENCY_CTRL_RD(tag[0] - 1) |
  

•                        L310_LATENCY_CTRL_WR(tag[1] - 1) |
  

•                        L310_LATENCY_CTRL_SETUP(tag[2] - 1);
  

•   writel(saved_reg, &regs->pl310_data_latency_ctrl);
  

•   /* Enable the L2 cache */
  

•   setbits_le32(&regs->pl310_ctrl, L2X0_CTRL_EN);
  

+}

+static int l2x0_ofdata_to_platdata(struct udevice *dev) +{

•   return 0;
  

+}

You can drop this function.

+static int l2x0_probe(struct udevice *dev) +{

•   l2c310_of_parse(dev);
  

•   return 0;
  

+}

+static const struct udevice_id l2x0_ids[] = {

•   { .compatible = "arm,pl310-cache" },
  

•   {}
  

+};

+U_BOOT_DRIVER(pl310_cache) = {

•   .name   = "pl310_cache",
  

•   .id     = UCLASS_CACHE,
  

•   .of_match = l2x0_ids,
  

•   .probe  = l2x0_probe,
  

•   .ofdata_to_platdata = l2x0_ofdata_to_platdata,
  

•   .flags  = DM_FLAG_PRE_RELOC,
  

+}; diff --git a/drivers/cache/cache-uclass.c b/drivers/cache/cache-uclass.c new file mode 100644 index 0000000000..27c1706bc1 --- /dev/null +++ b/drivers/cache/cache-uclass.c @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright Intel

??

• */

+#include <common.h> +#include <dm.h>

+UCLASS_DRIVER(cache) = {

•   .id             = UCLASS_CACHE,
  

•   .name           = "cache",
  

•   .post_bind      = dm_scan_fdt_dev,
  

+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 86e59781b0..b0eef19be7 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -34,6 +34,7 @@ enum uclass_id { UCLASS_BLK, /* Block device */ UCLASS_BOARD, /* Device information from hardware */ UCLASS_BOOTCOUNT, /* Bootcount backing store */

•   UCLASS_CACHE,           /* Cache controller*/
  

space before *.

    UCLASS_CLK,             /* Clock source, e.g. used by peripherals */
    UCLASS_CPU,             /* CPU, typically part of an SoC */
    UCLASS_CROS_EC,         /* Chrome OS EC */

-- 2.20.0

On 3/8/19 5:16 PM, Dinh Nguyen wrote:

Select the PL310 UCLASS_CACHE driver for SoCFPGA.

Signed-off-by: Dinh Nguyen dinguyen@kernel.org

arch/arm/Kconfig | 1 + 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index f42eccef80..f4c6262bb0 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -845,6 +845,7 @@ config ARCH_SOCFPGA imply SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE imply SPL_SPI_FLASH_SUPPORT imply SPL_SPI_SUPPORT

• imply L2X0_CACHE

config ARCH_SUNXI bool "Support sunxi (Allwinner) SoCs"

Reviewed-by: Marek Vasut marex@denx.de

On 3/8/19 5:16 PM, Dinh Nguyen wrote:

Find the UCLASS_CACHE driver to configure the cache controller's settings.

Signed-off-by: Dinh Nguyen dinguyen@kernel.org

arch/arm/mach-socfpga/misc.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c index fcf211d62b..34d8c4c51b 100644 --- a/arch/arm/mach-socfpga/misc.c +++ b/arch/arm/mach-socfpga/misc.c @@ -59,20 +59,10 @@ void enable_caches(void) #ifdef CONFIG_SYS_L2_PL310 void v7_outer_cache_enable(void) {

• /* Disable the L2 cache */
• clrbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
• writel(0x111, &pl310->pl310_tag_latency_ctrl);
• writel(0x121, &pl310->pl310_data_latency_ctrl);
• /* enable BRESP, instruction and data prefetch, full line of zeroes */
• setbits_le32(&pl310->pl310_aux_ctrl,

•      L310_AUX_CTRL_DATA_PREFETCH_MASK |
  

•      L310_AUX_CTRL_INST_PREFETCH_MASK |
  

•      L310_SHARED_ATT_OVERRIDE_ENABLE);
  

• struct udevice *dev;

• /* Enable the L2 cache */
• setbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);

• if (uclass_get_device(UCLASS_CACHE, 0, &dev))

• pr_err("cache controller driver NOT found!\n");
  

}

void v7_outer_cache_disable(void)

Reviewed-by: Marek Vasut marex@denx.de