Re: [PATCH v2 2/4] litex: add common LiteX header

From: Gabriel L. Somlo
Date: Wed Nov 20 2019 - 12:23:07 EST


> It provides helper CSR access functions used by all
> LiteX drivers.
>
> Signed-off-by: Mateusz Holenko <mholenko@xxxxxxxxxxxx>
> ---
> This commit has been introduced in v2 of the patchset.
>
> MAINTAINERS | 6 +++++
> include/linux/litex.h | 59 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 65 insertions(+)
> create mode 100644 include/linux/litex.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 296de2b51c83..eaa51209bfb2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9493,6 +9493,12 @@ F: Documentation/misc-devices/lis3lv02d.rst
> F: drivers/misc/lis3lv02d/
> F: drivers/platform/x86/hp_accel.c
>
> +LITEX PLATFORM
> +M: Karol Gugala <kgugala@xxxxxxxxxxxx>
> +M: Mateusz Holenko <mholenko@xxxxxxxxxxxx>
> +S: Maintained
> +F: include/linux/litex.h
> +
> LIVE PATCHING
> M: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> M: Jiri Kosina <jikos@xxxxxxxxxx>
> diff --git a/include/linux/litex.h b/include/linux/litex.h
> new file mode 100644
> index 000000000000..e793d2d7c881
> --- /dev/null
> +++ b/include/linux/litex.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Common LiteX header providing
> + * helper functions for accessing CSRs.
> + *
> + * Copyright (C) 2019 Antmicro <www.antmicro.com>
> + */
> +
> +#ifndef _LINUX_LITEX_H
> +#define _LINUX_LITEX_H
> +
> +#include <linux/io.h>
> +#include <linux/types.h>
> +#include <linux/compiler_types.h>
> +
> +#define LITEX_REG_SIZE 0x4
> +#define LITEX_SUBREG_SIZE 0x1
> +#define LITEX_SUBREG_SIZE_BIT (LITEX_SUBREG_SIZE * 8)

Please be aware that LiteX supports "LITEX_SUBREG_SIZE" of up to 4,
and might go even as far as 8 in the near future. Hard coding this
here might be short sighted.

A good alternative might be to have "litex_subreg_size" or
"mmio-dw-bytes" passed in via a DT property of the SoC node, as
illustrated here:

https://github.com/litex-hub/linux-on-litex-vexriscv/pull/60

Thanks,
--Gabriel

> +
> +#ifdef __LITTLE_ENDIAN
> +# define LITEX_READ_REG(addr) ioread32(addr)
> +# define LITEX_READ_REG_OFF(addr, off) ioread32(addr + off)
> +# define LITEX_WRITE_REG(val, addr) iowrite32(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off) iowrite32(val, addr + off)
> +#else
> +# define LITEX_READ_REG(addr) ioread32be(addr)
> +# define LITEX_READ_REG_OFF(addr, off) ioread32be(addr + off)
> +# define LITEX_WRITE_REG(val, addr) iowrite32be(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off) iowrite32be(val, addr + off)
> +#endif
> +
> +/* Helper functions for manipulating LiteX registers */
> +
> +static inline void litex_set_reg(void __iomem *reg, u32 reg_size, u32 val)
> +{
> + u32 shifted_data, shift, i;
> +
> + for (i = 0; i < reg_size; ++i) {
> + shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> + shifted_data = val >> shift;
> + LITEX_WRITE_REG(shifted_data, reg + (LITEX_REG_SIZE * i));
> + }
> +}
> +
> +static inline u32 litex_get_reg(void __iomem *reg, u32 reg_size)
> +{
> + u32 shifted_data, shift, i;
> + u32 result = 0;
> +
> + for (i = 0; i < reg_size; ++i) {
> + shifted_data = LITEX_READ_REG(reg + (LITEX_REG_SIZE * i));
> + shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
> + result |= (shifted_data << shift);
> + }
> +
> + return result;
> +}
> +
> +#endif /* _LINUX_LITEX_H */