Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 20 Nov 2019 12:23:00 -0500
From:   "Gabriel L. Somlo" <gsomlo@gmail.com>
To:     Mateusz Holenko <mholenko@antmicro.com>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jslaby@suse.com>, devicetree@vger.kernel.org,
        linux-serial@vger.kernel.org, Stafford Horne <shorne@gmail.com>,
        Karol Gugala <kgugala@antmicro.com>,
        Mateusz Holenko <mholenko@antmicro.com>,
        Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        "Paul E. McKenney" <paulmck@linux.ibm.com>,
        Filip Kokosinski <fkokosinski@internships.antmicro.com>,
        Joel Stanley <joel@jms.id.au>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Maxime Ripard <mripard@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Heiko Stuebner <heiko@sntech.de>,
        Sam Ravnborg <sam@ravnborg.org>,
        Icenowy Zheng <icenowy@aosc.io>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/4] litex: add common LiteX header
Message-ID: <20191120172300.GA15391@hedwig.ini.cmu.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191023114634.13657-2-mholenko@antmicro.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> It provides helper CSR access functions used by all
> LiteX drivers.
> 
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> 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@antmicro.com>
> +M:	Mateusz Holenko <mholenko@antmicro.com>
> +S:	Maintained
> +F:	include/linux/litex.h
> +
>  LIVE PATCHING
>  M:	Josh Poimboeuf <jpoimboe@redhat.com>
>  M:	Jiri Kosina <jikos@kernel.org>
> 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 */