From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
arnd@arndb.de, lee@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Revert "mfd: syscon: Remove repetition of the regmap_get_val_endian()"
Date: Sat, 8 Oct 2022 22:07:37 +0300 [thread overview]
Message-ID: <Y0HKeTWneX12OP+Y@smile.fi.intel.com> (raw)
In-Reply-To: <CAHk-=wiqN9EJ6zKXh21EQ2CV-B7_oDJKy73+yhRwtbNMWCzfVA@mail.gmail.com>
On Sat, Oct 08, 2022 at 09:45:16AM -0700, Linus Torvalds wrote:
> On Sat, Oct 8, 2022 at 8:47 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > This reverts commit 72a95859728a7866522e6633818bebc1c2519b17. It broke
> > reboots on big-endian MIPS and MIPS64 malta QEMU instances, which use
> > the syscon driver. Little-endian is not effected, which means likely
> > it's important to handle regmap_get_val_endian() in this function after
> > all.
>
> Hmm. The revert may indeed be the right thing to do, but we're still
> early in the release process, so let's go through the channels.
>
> I do note that commit 72a95859728a points to commit 0dbdb76c0ca8
> ("regmap: mmio: Parse endianness definitions from DT") as the reason
> why it's not necessary any more, but that commit
>
> (a) doesn't seem to set config->val_format_endian (which somebody may
> care about). It does set the operation pointers etc, but doesn't set
> that field.
It should.
of_syscon_register() calls to regmap_init_mmio() with syscon_config data
structure as a parameter.
Before 72a95859728a the of_syscon_register() fills the val_format_endian with
something it parses from DT. After that commit the default value (0) is
REGMAP_ENDIAN_DEFAULT. Now when __regmap_init_mmio_clk() is called it
creates a context base on DT since the field is 0.
> (b) it uses regmap_get_val_endian(), which doesn't actually even look
> at the OF properties unless config->val_format_endian is
> REGMAP_ENDIAN_DEFAULT
Which is 0!
> so the code that commit 72a95859728a removed was actually quite a bit
> different from the code in commit 0dbdb76c0ca8.
>
> Maybe the problem is related to those semantic differences, and is
> easy to fix for somebody who knows what the heck that stuff is doing.
But while looking into this, I think I know what is going on,
of_syscon_register() calls regmap API with dev == NULL, hence
fwnode == NULL, hence nothing to read from DT.
But default (via regmap bus configuration) is LE and LE works fine.
> And if not, please just send me the revert through the normal channels. Ok?
Yeah, revert is a good move here.
For real deduplication we need to either add a regmap_get_val_endian() kind
that takes fwnode as a parameter and call it explicitly or propagate fwnode
to __regmap_init_mmio_clk() somehow.
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2022-10-08 19:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-06 9:02 [GIT PULL] MFD for v6.1 Lee Jones
2022-10-07 19:20 ` pr-tracker-bot
2022-10-08 15:39 ` Jason A. Donenfeld
2022-10-08 15:47 ` [PATCH] Revert "mfd: syscon: Remove repetition of the regmap_get_val_endian()" Jason A. Donenfeld
2022-10-08 16:45 ` Linus Torvalds
2022-10-08 19:07 ` Andy Shevchenko [this message]
2022-10-10 7:48 ` Lee Jones
2022-10-10 15:25 ` Jason A. Donenfeld
2022-10-11 7:39 ` Lee Jones
2022-10-11 9:44 ` Andy Shevchenko
2022-10-11 9:44 ` Andy Shevchenko
2022-10-18 7:23 ` Lee Jones
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=Y0HKeTWneX12OP+Y@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=Jason@zx2c4.com \
--cc=arnd@arndb.de \
--cc=lee@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).