From: Rob Herring <robh@kernel.org>
To: Mateusz Holenko <mholenko@antmicro.com>
Cc: 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>,
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
Date: Fri, 25 Oct 2019 19:03:45 -0500 [thread overview]
Message-ID: <20191026000345.GA10810@bogus> (raw)
In-Reply-To: <20191023114634.13657-2-mholenko@antmicro.com>
On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> 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)
> +
> +#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)
Defining custom accessors makes it harder for others to understand
the code. The __raw_readl/writel accessors are native endian, so use
those. One difference though is they don't have a memory barrier, but
based on the below functions, you may want to do your own barrier
anyways. And if DMA is not involved you don't need the barriers either.
The _OFF variants don't add anything. LITEX_READ_REG(addr + off) is just
as easy to read if not easier than LITEX_READ_REG_OFF(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));
> + }
> +}
The problem with this is it hides whether you need to do any locking.
Normally, you would assume a register access is atomic when it's not
here. You could add locking in this function, but then you're doing
locking even when not needed.
It doesn't look like you actually use this in your series, so maybe
remove until you do.
> +
> +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 */
> --
> 2.23.0
next prev parent reply other threads:[~2019-10-26 0:03 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-23 9:46 [PATCH v2 0/4] LiteUART serial driver Mateusz Holenko
2019-10-23 9:46 ` [PATCH v2 1/4] dt-bindings: vendor: add vendor prefix for LiteX Mateusz Holenko
2019-10-25 23:36 ` Rob Herring
2019-10-23 9:47 ` [PATCH v2 2/4] litex: add common LiteX header Mateusz Holenko
2019-10-26 0:03 ` Rob Herring [this message]
2019-11-07 9:27 ` Mateusz Holenko
2019-11-20 17:23 ` Gabriel L. Somlo
2019-11-20 19:26 ` Greg Kroah-Hartman
2019-11-26 9:02 ` Mateusz Holenko
2019-11-26 9:19 ` Greg Kroah-Hartman
2019-11-29 15:39 ` Mateusz Holenko
2019-10-23 9:47 ` [PATCH v2 3/4] dt-bindings: serial: document LiteUART bindings Mateusz Holenko
2019-10-26 0:14 ` Rob Herring
2019-10-23 9:47 ` [PATCH v2 4/4] drivers/tty/serial: add LiteUART driver Mateusz Holenko
2019-10-26 0:13 ` Rob Herring
2019-11-07 9:33 ` Mateusz Holenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191026000345.GA10810@bogus \
--to=robh@kernel.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=fkokosinski@internships.antmicro.com \
--cc=gregkh@linuxfoundation.org \
--cc=heiko@sntech.de \
--cc=icenowy@aosc.io \
--cc=joel@jms.id.au \
--cc=jslaby@suse.com \
--cc=kgugala@antmicro.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mchehab+samsung@kernel.org \
--cc=mholenko@antmicro.com \
--cc=mripard@kernel.org \
--cc=paulmck@linux.ibm.com \
--cc=sam@ravnborg.org \
--cc=shawnguo@kernel.org \
--cc=shorne@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).