Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 9 Sep 2021 16:37:03 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Marc Zyngier <maz@kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jason Cooper <jason@lakedaemon.net>,
        Sumit Garg <sumit.garg@linaro.org>,
        Valentin Schneider <Valentin.Schneider@arm.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Gregory Clement <gregory.clement@bootlin.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Android Kernel Team <kernel-team@android.com>,
        stable <stable@vger.kernel.org>,
        Magnus Damm <damm+renesas@opensource.se>,
        Niklas =?iso-8859-1?Q?S=F6derlund?= 
        <niklas.soderlund+renesas@ragnatech.se>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v2 07/17] irqchip/gic: Atomically update affinity
Message-ID: <YToqH4phGwq4/MPQ@shell.armlinux.org.uk>
References: <20200624195811.435857-1-maz@kernel.org>
 <20200624195811.435857-8-maz@kernel.org>
 <CAMuHMdV+Ev47K5NO8XHsanSq5YRMCHn2gWAQyV-q2LpJVy9HiQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMuHMdV+Ev47K5NO8XHsanSq5YRMCHn2gWAQyV-q2LpJVy9HiQ@mail.gmail.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Thu, Sep 09, 2021 at 05:22:01PM +0200, Geert Uytterhoeven wrote:
> Despite the ARM Generic Interrupt Controller Architecture Specification
> (both version 1.0 and 2.0) stating that the Interrupt Processor Targets
> Registers are byte-accessible, the EMMA Mobile EV2 User's Manual
> states that the interrupt registers can be accessed via the APB bus,
> in 32-bit units.  Using byte accesses locks up the system.

Fun. Seems someone can't read ARMs documentation. Even the old
ARM IHI 0048B.b document I have for the GIC from 2013 states
"In addition, the GICD_IPRIORITYRn, GICD_ITARGETSRn, GICD_CPENDSGIRn,
and GICD_SPENDSGIRn registers support byte accesses."

However, this kind of thing is sadly not uncommon. There's been a
similar issue with the PL011 UART driver as well - some platforms
require 16-bit accesses instead of normal 32-bit accesses.

> Unfortunately I only have remote access to the board showing the
> issue.  I did check that adding the writeb_relaxed() before the
> writel_relaxed() that was used before also causes a lock-up, so the
> issue is not an endian mismatch.
> Looking at the driver history, these registers have always been
> accessed using 32-bit accesses before.  As byte accesses lead
> indeed to simpler code, I'm wondering if they had been tried before,
> and caused issues before?
> 
> Since you said the locking was bogus before, due to the reliance on
> the BL_SWITCHER option, I'm not suggesting a plain revert, but I'm
> wondering what kind of locking you suggest to use instead?

If byte accesses are not going to be workable, then the only
answer _is_ a read-modify-write with working locking.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!