Re: [PATCH v2] irq domain: drop IRQ_DOMAIN_HIERARCHY option, make it always on

[Randy Dunlap <rdunlap@xxxxxxxxxxxxx>]

Hi Thomas,

On 3/23/23 13:53, Thomas Gleixner wrote:
> On Sun, Mar 12 2023 at 19:39, Randy Dunlap wrote:
>> In preparation for dropping the IRQ_DOMAIN Kconfig option (effectively
>> making it always set/on), first drop IRQ_DOMAIN_HIERARCHY as an option,
>> making its code always set/on.
>>
>> This has been built successfully on all ARCHes except hexagon,
>> both 32-bit and 64-bit where applicable.
> 
> I really like where this is going, but reviewing this is a pain. I tried
> to split it up into more digestable pieces:
> 
>    https://tglx.de/~tglx/patches.tar
> 
> That's not completely equivalent to your patch as I did some of the
> changes below. It builds on various oddball architectures with
> IRQ_DOMAIN=n, but is otherwise completely untested.
> 
> It should be actually trivial after that to make IRQ_DOMAIN def_bool y
> and then gradually remove the IRQ_DOMAIN selects and ifdeffery.

Yeah, that may be the best & simplest approach.

Or just use your patches.tar.

>> v2: add stubs in include/linux/irqdomain.h for the config case of
>> IRQ_DOMAIN is not set. If these are not added, there will be plenty
>> of build errors (not so much for modern arches as for older ones).
> 
> I'm not really convinced that all of these stubs are required. Why would
> there suddenly be a requirement to expose stubs for functions which
> depend on CONFIG_IRQ_DOMAIN=y already today just by removing the
> hierarchy config?

All of those stubs were added because I had configs/builds that were
failing due to them. I didn't just add them for fun or "completeness."

I just checked and I didn't save all of those failing configs or
output files that contain the errors.


> Even exposing stubs for functions which have been only available via
> CONFIG_IRQ_DOMAIN_HIERARCHY is questionable simply because there cannot
> be any code which invokes them unconditionally if
> CONFIG_IRQ_DOMAIN_HIERARCHY=n today.
> 
> IOW, the sum of required stubs cannot be larger than number of stubs
> required today.
> 
> If there is code which has a #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY then
> this needs to be changed to CONFIG_IRQ_DOMAIN or the required functions
> have to be exposed unconditionally, right?

Yes, s/CONFIG_IRQ_DOMAIN_HIERARCHY/CONFIG_IRQ_DOMAIN/ in code.


>> diff -- a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -361,7 +361,7 @@ config GPIO_IXP4XX
>>  	depends on OF
>>  	select GPIO_GENERIC
>>  	select GPIOLIB_IRQCHIP
>> -	select IRQ_DOMAIN_HIERARCHY
>> +	select IRQ_DOMAIN
> 
> IRQ_DOMAIN is already selected by GPIOLIB_IRQCHIP, so this select is
> redundant for all GPIO configs which select GPIOLIB_IRQCHIP.

Ack.

I'm perfectly happy to use either Marc's patch that he posted on
2023-FEB-14 (https://lore.kernel.org/all/86y1p0xbqd.wl-maz@xxxxxxxxxx/)
or your patches. Both of you know your way around this better than I do.

Thanks for your review and feedback.

-- 
~Randy