Re: [U-Boot] [RFC PATCHv1 1/3] misc: pl310: add a misc driver for the pl310 cache controller

[Marek Vasut]

On 3/5/19 8:03 PM, Dinh Nguyen wrote:
> The driver will read the cache properties from the device tree file and
> set it up.
> 
> Signed-off-by: Dinh Nguyen <dingu...@kernel.org>
> ---
>  arch/arm/include/asm/pl310.h |  3 ++
>  drivers/misc/Kconfig         |  7 +++
>  drivers/misc/Makefile        |  1 +
>  drivers/misc/cache-l2x0.c    | 84 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 95 insertions(+)
>  create mode 100644 drivers/misc/cache-l2x0.c
> 
> 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)
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index d6e677fba8..c5c34b4dbb 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -13,6 +13,13 @@ config MISC
>         set of generic read, write and ioctl methods may be used to
>         access the device.
>  
> +config L2X0_CACHE
> +     bool "L2x0 Cache support"
> +     depends on MISC
> +     help
> +       Select this to enable a PL310 L2 Cache driver. The driver will
> +       configure the L2 Cache settings found in the device tree.

I wonder whether we don't need some drivers/plat or drivers/soc for this ?

>  config ALTERA_SYSID
>       bool "Altera Sysid support"
>       depends on MISC
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 6bdf5054f4..ea726f4668 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_SANDBOX) += spltest_sandbox.o
>  endif
>  endif
>  obj-$(CONFIG_ALI152X) += ali512x.o
> +obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o
>  obj-$(CONFIG_ALTERA_SYSID) += altera_sysid.o
>  obj-$(CONFIG_ATSHA204A) += atsha204a-i2c.o
>  obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o
> diff --git a/drivers/misc/cache-l2x0.c b/drivers/misc/cache-l2x0.c
> new file mode 100644
> index 0000000000..b31598b1cd
> --- /dev/null
> +++ b/drivers/misc/cache-l2x0.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2007 ARM Limited

2007 ? :)

> + * copied from Linux(arch/arm/mm/cache-l2x0.c
> + */
> +#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_i, prefetch_d;
> +     bool shared_override;
> +     struct pl310_regs *regs = (struct pl310_regs *)devfdt_get_addr(dev);
> +
> +     /*Disable the L2 Cache */

/*<SPACE>D

> +     clrbits_le32(&regs->pl310_ctrl, L2X0_CTRL_EN);
> +
> +     dev_read_u32(dev, "prefetch-data", &prefetch_d);
> +     dev_read_u32(dev, "prefetch-instr", &prefetch_i);
> +     shared_override = dev_read_bool(dev, "arm,shared-override");
> +
> +     saved_reg = readl(&regs->pl310_aux_ctrl);
> +     if (prefetch_d)
> +             saved_reg |= L310_AUX_CTRL_DATA_PREFETCH_MASK;
> +     if (prefetch_i)
> +             saved_reg |= L310_AUX_CTRL_INST_PREFETCH_MASK;
> +     if (shared_override)
> +             saved_reg |= L310_SHARED_ATT_OVERRIDE_ENABLE;
> +
> +     writel(saved_reg, &regs->pl310_aux_ctrl);
> +
> +     saved_reg = readl(&regs->pl310_tag_latency_ctrl);
> +     dev_read_u32_array(dev, "arm,tag-latency", tag, 3);
> +
> +     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);
> +     dev_read_u32_array(dev, "arm,data-latency", tag, 3);

What happens if the array isn't present ?
Should we _not_ configure the latencies in such case ?

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

Two newlines here ^ drop one

> +static const struct udevice_id l2x0_ids[] = {
> +     { .compatible = "arm,pl310-cache" },
> +     {}
> +};
> +
> +U_BOOT_DRIVER(pl310_cache) = {
> +     .name   = "pl310_cache",
> +     .id     = UCLASS_MISC,
> +     .of_match = l2x0_ids,
> +     .probe  = l2x0_probe,
> +     .ofdata_to_platdata = l2x0_ofdata_to_platdata,
> +     .flags  = DM_FLAG_PRE_RELOC,
> +};
> 


-- 
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot