[U-Boot] [PATCHv1 0/4] dm: cache: add dm cache driver

[U-Boot] [PATCHv1 0/4] dm: cache: add dm cache driver

[Dinh Nguyen]

Hi,

Add a UCLASS_CACHE dm driver to handling the configuration of cache
settings. Place this new driver under /drivers/cache. In this initial
revision, the driver is only configuring what I think are essential cache
settings. The more comprehensive cache settings can be done in the OS.

Dinh

Dinh Nguyen (4):
  ARM: pl310: Add macro's for handling tag and data latency mask
  dm: cache: Create a uclass for cache controller
  configs: socfpga: add imply pl310 cache controller
  ARM: socfpga: use the pl310 driver to configure the cache

 arch/arm/Kconfig             |  1 +
 arch/arm/include/asm/pl310.h |  3 ++
 arch/arm/mach-socfpga/misc.c | 16 ++-----
 drivers/Kconfig              |  2 +
 drivers/Makefile             |  1 +
 drivers/cache/Kconfig        | 22 ++++++++++
 drivers/cache/Makefile       |  3 ++
 drivers/cache/cache-l2x0.c   | 82 ++++++++++++++++++++++++++++++++++++
 drivers/cache/cache-uclass.c | 13 ++++++
 include/dm/uclass-id.h       |  1 +
 10 files changed, 131 insertions(+), 13 deletions(-)
 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

-- 
2.20.0

[U-Boot] [PATCHv1 1/4] ARM: pl310: Add macro's for handling tag and data latency mask

[Dinh Nguyen]

Add the PL310 macros for latency control setup, read and write bits.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 arch/arm/include/asm/pl310.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h
index b83978b1cc..f69e9e45f8 100644
--- a/arch/arm/include/asm/pl310.h
+++ b/arch/arm/include/asm/pl310.h
@@ -18,6 +18,9 @@
 #define L310_SHARED_ATT_OVERRIDE_ENABLE		(1 << 22)
 #define L310_AUX_CTRL_DATA_PREFETCH_MASK	(1 << 28)
 #define L310_AUX_CTRL_INST_PREFETCH_MASK	(1 << 29)
+#define L310_LATENCY_CTRL_SETUP(n)		((n) << 0)
+#define L310_LATENCY_CTRL_RD(n)		((n) << 4)
+#define L310_LATENCY_CTRL_WR(n)		((n) << 8)
 
 #define L2X0_CACHE_ID_PART_MASK     (0xf << 6)
 #define L2X0_CACHE_ID_PART_L310     (3 << 6)
-- 
2.20.0

[U-Boot] [PATCHv1 1/4] ARM: pl310: Add macro's for handling tag and data latency mask

[Marek Vasut]

On 3/8/19 5:16 PM, Dinh Nguyen wrote:
> Add the PL310 macros for latency control setup, read and write bits.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  arch/arm/include/asm/pl310.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h
> index b83978b1cc..f69e9e45f8 100644
> --- a/arch/arm/include/asm/pl310.h
> +++ b/arch/arm/include/asm/pl310.h
> @@ -18,6 +18,9 @@
>  #define L310_SHARED_ATT_OVERRIDE_ENABLE		(1 << 22)
>  #define L310_AUX_CTRL_DATA_PREFETCH_MASK	(1 << 28)
>  #define L310_AUX_CTRL_INST_PREFETCH_MASK	(1 << 29)
> +#define L310_LATENCY_CTRL_SETUP(n)		((n) << 0)
> +#define L310_LATENCY_CTRL_RD(n)		((n) << 4)
> +#define L310_LATENCY_CTRL_WR(n)		((n) << 8)
>  
>  #define L2X0_CACHE_ID_PART_MASK     (0xf << 6)
>  #define L2X0_CACHE_ID_PART_L310     (3 << 6)
> 

Reviewed-by: Marek Vasut <marex@denx.de>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCHv1 2/4] dm: cache: Create a uclass for cache controller

[Dinh Nguyen]

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 ++++++++++++++++++++++++++++++++++++
 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

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.
+
+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.
+
+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);
+
+	/*Disable the L2 Cache */
+	clrbits_le32(&regs->pl310_ctrl, L2X0_CTRL_EN);
+
+	saved_reg = readl(&regs->pl310_aux_ctrl);
+	if (dev_read_u32(dev, "prefetch-data", &prefetch) == 0) {
+		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;
+}
+
+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*/
 	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

[U-Boot] [PATCHv1 2/4] dm: cache: Create a uclass for cache controller

[Marek Vasut]

On 3/8/19 5:16 PM, Dinh Nguyen 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 ++++++++++++++++++++++++++++++++++++
>  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
> 
> 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.
> +
> +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.
> +
> +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);
> +
> +	/*Disable the L2 Cache */
> +	clrbits_le32(&regs->pl310_ctrl, L2X0_CTRL_EN);
> +
> +	saved_reg = readl(&regs->pl310_aux_ctrl);
> +	if (dev_read_u32(dev, "prefetch-data", &prefetch) == 0) {
> +		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;
> +}
> +
> +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*/

Maybe we can recycle UCLASS_CPU or something for this ?

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


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCHv1 2/4] dm: cache: Create a uclass for cache controller

[Simon Glass]

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
>

[U-Boot] [PATCHv1 3/4] configs: socfpga: add imply pl310 cache controller

[Dinh Nguyen]

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"
-- 
2.20.0

[U-Boot] [PATCHv1 3/4] configs: socfpga: add imply pl310 cache controller

[Marek Vasut]

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>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCHv1 4/4] ARM: socfpga: use the pl310 driver to configure the cache

[Dinh Nguyen]

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)
-- 
2.20.0

[U-Boot] [PATCHv1 4/4] ARM: socfpga: use the pl310 driver to configure the cache

[Marek Vasut]

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>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCHv1 0/4] dm: cache: add dm cache driver

[Marek Vasut]

On 3/8/19 5:16 PM, Dinh Nguyen wrote:
> Hi,
> 
> Add a UCLASS_CACHE dm driver to handling the configuration of cache
> settings. Place this new driver under /drivers/cache. In this initial
> revision, the driver is only configuring what I think are essential cache
> settings. The more comprehensive cache settings can be done in the OS.
> 
> Dinh
> 
> Dinh Nguyen (4):
>   ARM: pl310: Add macro's for handling tag and data latency mask
>   dm: cache: Create a uclass for cache controller
>   configs: socfpga: add imply pl310 cache controller
>   ARM: socfpga: use the pl310 driver to configure the cache
> 
>  arch/arm/Kconfig             |  1 +
>  arch/arm/include/asm/pl310.h |  3 ++
>  arch/arm/mach-socfpga/misc.c | 16 ++-----
>  drivers/Kconfig              |  2 +
>  drivers/Makefile             |  1 +
>  drivers/cache/Kconfig        | 22 ++++++++++
>  drivers/cache/Makefile       |  3 ++
>  drivers/cache/cache-l2x0.c   | 82 ++++++++++++++++++++++++++++++++++++
>  drivers/cache/cache-uclass.c | 13 ++++++
>  include/dm/uclass-id.h       |  1 +
>  10 files changed, 131 insertions(+), 13 deletions(-)
>  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
> 

Looks OK to me

-- 
Best regards,
Marek Vasut