From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756804AbcAZJLU (ORCPT ); Tue, 26 Jan 2016 04:11:20 -0500 Received: from mout.kundenserver.de ([212.227.126.131]:50414 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756574AbcAZJJu (ORCPT ); Tue, 26 Jan 2016 04:09:50 -0500 From: Arnd Bergmann To: Johannes Berg Cc: Mark Brown , Simon Arlott , linux-kernel@vger.kernel.org Subject: Re: [PATCH] Revert "regmap-mmio: Use native endianness for read/write" Date: Tue, 26 Jan 2016 10:09:35 +0100 Message-ID: <6962431.EnoXCqILtZ@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1453796736.2759.2.camel@sipsolutions.net> References: <1453759675-28461-1-git-send-email-johannes@sipsolutions.net> <20160125235254.GZ6588@sirena.org.uk> <1453796736.2759.2.camel@sipsolutions.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:UEKE2qp94SpFDdF+o3YvvLjrjSElXCnpP9WdINqq2gNK/rWCF7s bxfImNlXDJIX2RoK4GQto6jOX9m0b1XqyVteIS6grQM5nTiX4JOgbOmGgXT8PiqXkC0ke4J Cgr121dnoy/MO4DTiuDVhD79iDq7z+IBeQuqvTPQg0aG/7zYe1J9OkDHPv/DtMuzLjkyq8f S6oyOfGxHqFELoIO83djg== X-UI-Out-Filterresults: notjunk:1;V01:K0:mHDyhthvYPE=:aT1cMYMWSWQCSPWVdIB44S ijMsd6/AzZrL2ruPgHuJexTq30CazyfoGkxJk2FRuM85NbrMICIdnV0mknKlNY+G48+C2HxVv hgFQeKKFKK2ofr63HwB8WxTuZtJSaSDkUGpnQtrmZZLma5+0kz5RCnhkqD48KwFxyc2InH41o jO7DaSvdz2z/Bigjj18EktPG7POdUa1brWUmM5jr5XFp7lsH40ZW1eQp/MbSLM7wS44DNnHNI uhq8u/B2GboUGQ5iJP1pdljVwEJrS5mCWzId7/csdLR2poRIMtdnnucgBRHivmE4nU2LWPNEc 85BH7ZeXYrvWIdrE/ygKyRAo8mDFFS2O7WnWWSeSsZrzbtU/eiDz3PHSpXUGJF1r31BCQUdVv r9NJvMb+8eFfzjFS4tD7SL5WVYXBPbwwbrDliwy6N8eQ5csf/GGC6uLVxzJNUqsngIu0TGVID 8vv/qo+XjLoYTmtfB3oD6e4t9V806bLc8ZtF5p8e7tguJzE8Fm10kTFZrFntLwas8cLQyX5+1 AEO+BUnjTf7p39TkmXCxSCna3uwvhljywQPSgf/zs6YgpmMla4hONyOjZVMuIcerOGB9aSgDZ HyYAf7/SXVThzIGSCZI6PGBtF1fhybRtWf3lZju4DZCAfAtbTL9uk/GeAh8+ox3DJeFcaCokx 8QmUlX1rv4EZD4XaSpa3eZ9T4n9PxswDfHPfPwva4g8byo7uBEb0Kf8PsyV085qql/o8rMGPk eM/zshxg+78qYFH6Mw5SBj8YxLFelzpyf2St0tNKfJ1dAI06pgdHjgLE3AaH8Npeu8rSGhSQX z3XyP4TykjF/A0z+wtNvHcchkTBEMAifh9HkpCWcPmJOPvmstc= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 26 January 2016 09:25:36 Johannes Berg wrote: > On Mon, 2016-01-25 at 23:52 +0000, Mark Brown wrote: > > > - Make the default for MMIO regmaps be explicitly little endian with > > either an ifdef for MIPS to keep it working or an explict native > > endianness tag in the DT instead of the straight revert to LE (the > > latter seems better). > > This makes sense, and I agree that the latter is better. +1 > > - Convert the MMIO regmap to use reg_read() and reg_write() with > > implementations using either readX() or ioread_*be() and > > equivalents for write. This means the core does no endianness > > swapping and it's all in the bus. > > I don't think there's ioread64be/iowrite64be, and I'm also not entirely > sure how that works since it uses __raw_* internally in lib/iomap.c? Again, it's complicated: * We should probably add ioread64be()/iowrite64be() on *64bit* architectures for consistency with readq/writeq * On 32-bit architectures, you generally cannot do 64-bit atomic I/O operations, and we have two implementations that do it nonatomically, depending on how a device is wired to the bus, see include/linux/io-64-nonatomic-{hi-lo,lo-hi}.h. I think we should just not go there for regmap unless we absolutely have to. > > Unfortunately that all sounds a bit too big for v4.5... perhaps a > > combination of a revert of the implementation and the addition of the > > native tag to the DT for v4.5 followed by the reworking of the bus > > for v4.6, I really would rather keep the DT change in v4.5 since > > specifying LE is just bad and we don't want that to propagate any > > more than it has to. > > Yes, that makes sense. Sounds good. > > From this I also conclude that we need to improve our testing of big > > endian ARM systems since nobody managed to notice this all the time > > this was cooking in -next. > > To my knowledge before I did a couple of days ago nobody ever ran i.MX6 > in big endian mode :) At least nobody ever talked about it in public if they did, because they would have to do the same patches you wrote. There is still one open question about the defaults: I think we all agree that there is no way we can change the default for compatible="syscon" devices on ARM to from little-endian to cpu-endian, as that would break everything. Annotating the MIPS dts files as "cpu-endian" and leaving the rest to default to "little" is probably best here. However, we have some freedom at the regmap-mmio level, which we can sanitize in 4.6 if we want to make it more consistent with the rest of regmap. We have around 50 callers of {devm_,}regmap_init_mmio() and almost all of them do not specify endianess but expect little-endian behavior. We can change all existing instances to set REGMAP_ENDIAN_NATIVE explicitly and change regmap_init_mmio() to return an error if the caller does not specify a particular endianess (big, little, native). This is a tradeoff between interface simplicity (defaulting to LE is convenient for most people) and consistency (all other regmap interfaces default to native if I understood Mark right). Arnd