Re: [PATCH v4] irqchip/ls-extirq: fix invalid wait context by avoiding to use regmap

[Sean Anderson]

On 7/28/22 12:19 PM, Vladimir Oltean wrote:
> 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?

Oh, interesting, we don't have a device in this driver.

>> > 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.
> 

Yeah I suppose it's not a terrible burden here.

--Sean