[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220728161948.gzc7kekygrfdumq6@skbuf>
Date: Thu, 28 Jul 2022 16:19:50 +0000
From: Vladimir Oltean <vladimir.oltean@....com>
To: Sean Anderson <sean.anderson@...o.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Lee Jones <lee.jones@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Arnd Bergmann <arnd@...db.de>,
"Z.Q. Hou" <zhiqiang.hou@....com>, Biwen Li <biwen.li@....com>
Subject: Re: [PATCH v4] irqchip/ls-extirq: fix invalid wait context by
avoiding to use regmap
On Thu, Jul 28, 2022 at 11:43:40AM -0400, Sean Anderson wrote:
> > All of this should be rewritten to use of_device_is_big_endian(), and
> > reduce the whole thing to two cases (I don't think native endian makes
> > much sense anyway). I also wonder what the result is if none of these
> > properties is present...
>
> I think regmap_get_val_endian would be better here.
It needs a struct device.
> >> +
> >> + priv->regmap = regmap_init_mmio(NULL, base, &extirq_regmap_config);
>
> It could also be done automatically if we pass the syscon dev instead of
> NULL. The only downside is that some regmap error messages will use the
> syscon device
How do you get the struct device of the syscon?
> > Finally, what is the actual benefit of using a regmap here? It seems
> > like a very roundabout way of performing a RMW on a register whilst
> > holding a lock... Passing NULL for a device to regmap_init_mmio() also
> > seems to be an extremely rare idiom (only 5 cases in the tree), and
> > this doesn't seem completely right to me.
>
> The benefit is that you don't have to write (yet another) set of
> endian-converting read/write functions. The above (non-NULL) usage of
> regmap_init would also address your criticism here.
I don't have a particular attraction towards using regmap for a single
register either, to be honest.
Powered by blists - more mailing lists