All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ARM: Rework and correct barrier definitions
Date: Fri, 2 Sep 2016 09:00:32 -0400	[thread overview]
Message-ID: <20160902130032.GA4990@bill-the-cat> (raw)
In-Reply-To: <AM4PR0401MB1732BE9A9B10135E188E1EC09AE00@AM4PR0401MB1732.eurprd04.prod.outlook.com>

On Tue, Aug 30, 2016 at 09:50:47PM +0000, york sun wrote:
> On 08/01/2016 03:54 PM, Tom Rini wrote:
> > As part of testing booting Linux kernels on Rockchip devices, it was
> > discovered by Ziyuan Xu and Sandy Patterson that we had multiple and for
> > some cases incomplete isb definitions.  This was causing a failure to
> > boot of the Linux kernel.
> >
> > In order to solve this problem as well as cover any corner cases that we
> > may also have had a number of changes are made in order to consolidate
> > things.  First, <asm/barriers.h> now becomes the source of isb/dsb/dmb
> > definitions.  This however introduces another complexity.  Due to
> > needing to build SPL for 32bit tegra with -march=armv4 we need to borrow
> > the __LINUX_ARM_ARCH__ logic from the Linux Kernel in a more complete
> > form.  Move this from arch/arm/lib/Makefile to arch/arm/Makefile and add
> > a comment about it.  Now that we can always know what the target CPU is
> > capable off we can get always do the correct thing for the barrier.  The
> > final part of this is that need to be consistent everywhere and call
> > isb()/dsb()/dmb() and NOT call ISB/DSB/DMB in some cases and the
> > function names in others.
> >
> > Reported-by: Ziyuan Xu <xzy.xu@rock-chips.com>
> > Reported-by: Sandy Patterson <apatterson@sightlogix.com>
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> > Good work on figuring this out guys.  Please test and ack this on your
> > hardware as well.  I've given this a boot test on one of my platforms
> > and built it for all ARM targets.
> > ---
> 
> <snip>
> 
> >  arch/arm/include/asm/barriers.h            | 11 +++++++++--
> >  arch/arm/include/asm/io.h                  |  4 ++--
> 
> <snip>
> 
> > diff --git a/arch/arm/include/asm/barriers.h b/arch/arm/include/asm/barriers.h
> > index 37870f9..04784b7 100644
> > --- a/arch/arm/include/asm/barriers.h
> > +++ b/arch/arm/include/asm/barriers.h
> > @@ -30,15 +30,22 @@
> >
> >  #endif /* !CONFIG_ARM64 */
> >
> > -#if defined(__ARM_ARCH_7A__) || defined(CONFIG_ARM64)
> > +#if __LINUX_ARM_ARCH__ >= 7
> >  #define ISB	asm volatile ("isb sy" : : : "memory")
> >  #define DSB	asm volatile ("dsb sy" : : : "memory")
> >  #define DMB	asm volatile ("dmb sy" : : : "memory")
> > -#else
> > +#elif __LINUX_ARM_ARCH__ == 6
> >  #define ISB	CP15ISB
> >  #define DSB	CP15DSB
> >  #define DMB	CP15DMB
> > +#else
> > +#define ISB	asm volatile ("" : : : "memory")
> > +#define DSB	CP15DSB
> > +#define DMB	asm volatile ("" : : : "memory")
> >  #endif
> >
> > +#define isb()	ISB
> > +#define dsb()	DSB
> > +#define dmb()	DMB
> >  #endif	/* __ASSEMBLY__ */
> >  #endif	/* __BARRIERS_H__ */
> > diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> > index 6121f1d..5834f5b 100644
> > --- a/arch/arm/include/asm/io.h
> > +++ b/arch/arm/include/asm/io.h
> > @@ -25,6 +25,7 @@
> >  #include <linux/types.h>
> >  #include <asm/byteorder.h>
> >  #include <asm/memory.h>
> > +#include <asm/barriers.h>
> >  #if 0	/* XXX###XXX */
> >  #include <asm/arch/hardware.h>
> >  #endif	/* XXX###XXX */
> > @@ -136,8 +137,7 @@ static inline void __raw_readsl(unsigned long addr, void *data, int longlen)
> >   * TODO: The kernel offers some more advanced versions of barriers, it might
> >   * have some advantages to use them instead of the simple one here.
> >   */
> > -#define mb()		asm volatile("dsb sy" : : : "memory")
> > -#define dmb()		__asm__ __volatile__ ("" : : : "memory")
> > +#define mb()		dsb()
> 
> Tom,
> 
> The replacement of dmb() broke ls2080ardb_nand, the spl boot part. I 
> don't quite understand why dmb() was "", and why "dmb" is broken for spl.

Very strange.  Have you tried different compilers?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160902/0b33c6e7/attachment.sig>

  reply	other threads:[~2016-09-02 13:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-01 22:54 [U-Boot] [PATCH] ARM: Rework and correct barrier definitions Tom Rini
2016-08-02  0:37 ` Ziyuan Xu
2016-08-02  0:39   ` Tom Rini
2016-08-02 12:28     ` Sandy Patterson
2016-08-04  7:49     ` Ziyuan Xu
2016-08-02 18:13 ` Stephen Warren
2016-08-06  0:57 ` Tom Rini
2016-08-30 21:27   ` york sun
2016-08-30 21:50 ` york sun
2016-09-02 13:00   ` Tom Rini [this message]
2016-09-02 16:16     ` york sun
2016-08-30 22:01 ` york sun

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=20160902130032.GA4990@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.