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;
MIME-Version: 1.0
References: <20190320223853.1209-1-alistair.francis@wdc.com> <20190320234858.GA5304@infradead.org>
In-Reply-To: <20190320234858.GA5304@infradead.org>
From:   Alistair Francis <alistair23@gmail.com>
Date:   Wed, 20 Mar 2019 17:04:58 -0700
Message-ID: <CAKmqyKPsoi7F_NNLKcm269=VAgdvsUV7HdF3dtkpMBzaKbHhtA@mail.gmail.com>
Subject: Re: [PATCH] irqchip: plic: Fix priority base offset
To:     Christoph Hellwig <hch@infradead.org>
Cc:     Alistair Francis <Alistair.Francis@wdc.com>,
        "palmer@sifive.com" <palmer@sifive.com>,
        "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Mar 20, 2019 at 4:49 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Mar 20, 2019 at 10:39:52PM +0000, Alistair Francis wrote:
> > According to the FU540 and E31 manuals the PLIC source priority
> > address starts at an offset of 0x04 and not 0x00. To aviod confusion
> > update the address and source offset to match the documentation. This
> > causes no difference in functionality.
>
> Well, it starts at 0x00, but the first one is reserved.  If you think
> that is too confusing I'd rather throw in a comment explaining this
> fact rather than making the calculating more complicated.

It doesn't mention that it starts at 0 when you look here:
https://sifive.cdn.prismic.io/sifive%2F834354f0-08e6-423c-bf1f-0cb58ef14061_fu540-c000-v1.0.pdf
(pg. 61).

I agree that it's the same as starting as 0 and reserving the first
one, but this means that the macros are actually at the wrong address
and we need to remember not to use 0. This ends up forgotten and I
have seen bugs in OpenSBI and QEMU as they have a similar
implementation. It's possible some future code update will forget this
as well.

I think it's much clearer to define the correct values and just
subtract 1, the calculation really isn't more complicated.

Alistair