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;
MIME-Version: 1.0
References: <20220120080347.1595379-1-laurent@vivier.eu> <20220120080347.1595379-3-laurent@vivier.eu>
In-Reply-To: <20220120080347.1595379-3-laurent@vivier.eu>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Thu, 20 Jan 2022 09:50:30 +0100
Message-ID: <CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com>
Subject: Re: [PATCH v11 2/5] tty: goldfish: introduce gf_ioread32()/gf_iowrite32()
To:     Laurent Vivier <laurent@vivier.eu>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Alessandro Zummo <a.zummo@towertech.it>,
        linux-rtc@vger.kernel.org, John Stultz <john.stultz@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Jiaxun Yang <jiaxun.yang@flygoat.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        linux-m68k <linux-m68k@lists.linux-m68k.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Stephen Boyd <sboyd@kernel.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jan 20, 2022 at 9:03 AM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Revert
> commit da31de35cd2f ("tty: goldfish: use __raw_writel()/__raw_readl()")
>
> to use accessors defined by the architecture.
>
> Define by default the accessor to be little-endian as we
> have only little-endian architectures using goldfish devices.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

The patch looks good, but the description seems wrong to me:

Talking about "little-endian architectures" makes no sense here, the
point is that the device was clearly defined as having little-endian
registers, and your earlier patch broke this driver when running
on big-endian kernels (if anyone ever tried this).

This means you should also add

Cc: stable@vger.kernel.org # v5.11+
Fixes: da31de35cd2f ("tty: goldfish: use __raw_writel()/__raw_readl()")

The fact that m68k gets this wrong is just a bug in qemu, but it's
probably impossible to fix that since there is no way of knowing which
other operating systems have started relying on that bug over the years.

It might be a good idea to revisit the qemu implementation and make
sure that the extra byteswap is only inserted on m68k and not on
other targets, but hopefully there are no new targets based on goldfish
anymore and we don't need to care.

        Arnd