From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Mateusz Holenko <mholenko@antmicro.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>, 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: Wed, 20 Nov 2019 20:26:48 +0100 [thread overview]
Message-ID: <20191120192648.GA3087498@kroah.com> (raw)
In-Reply-To: <20191023114634.13657-2-mholenko@antmicro.com>
On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> +#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
I just noticed this.
Ick, this is not good. You will run into problems in the future with
this, I can guarantee it. What about systems where the CPU is one
endian and the hardware in the other? It will happen trust us.
Make these real functions (inline is nice) and pass in the pointer to
the device so you can test for it and call the correct function based on
the cpu/hardware type.
And what about bitfields? What endian are they for your
system/hardware?
Almost no kernel code should EVER be testing __LITTLE_ENDIAN, don't add
to it as it is not a good idea.
thanks,
greg k-h
next prev parent reply other threads:[~2019-11-20 19:26 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
2019-11-07 9:27 ` Mateusz Holenko
2019-11-20 17:23 ` Gabriel L. Somlo
2019-11-20 19:26 ` Greg Kroah-Hartman [this message]
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=20191120192648.GA3087498@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=fkokosinski@internships.antmicro.com \
--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=robh+dt@kernel.org \
--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).