[U-Boot] [PATCHv1 2/4] dm: cache: Create a uclass for cache controller
Simon Glass
sjg at chromium.org
Sun Mar 10 21:51:25 UTC 2019
Hi Dinh,
On Fri, 8 Mar 2019 at 09:17, Dinh Nguyen <dinguyen at 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 at 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(®s->pl310_ctrl, L2X0_CTRL_EN);
> +
> + saved_reg = readl(®s->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, ®s->pl310_aux_ctrl);
> +
> + saved_reg = readl(®s->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, ®s->pl310_tag_latency_ctrl);
> +
> + saved_reg = readl(®s->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, ®s->pl310_data_latency_ctrl);
> +
> + /* Enable the L2 cache */
> + setbits_le32(®s->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
>
More information about the U-Boot
mailing list