Re: [RFC PATCH 0/6] Support ROHM BD96801 scalable PMIC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi dee Ho peeps,

On 4/5/24 12:19, Matti Vaittinen wrote:
On 4/4/24 16:15, Matti Vaittinen wrote:
Hi Mark,

On 4/4/24 15:09, Mark Brown wrote:
On Thu, Apr 04, 2024 at 10:26:34AM +0300, Matti Vaittinen wrote:

1. Should we be able to have more than 1 IRQ domain / device?
2. Should regmap_irq support having more than 1 HWIRQ

I would expect each parent interrupt to show up as a separate remap_irq.

...
So if we arrange to supply a name when we register multiple domains
things should work fine?

After my latest findings, yes, I think so. How to do this correctly is beyond me though. The __irq_domain_create() seems to me that the name is meant to be the dt-node name when the controller is backed by a real dt-node. Naming of the irq_domain_alloc_named_fwnode() sounds to me like it is only intended to be used when there is no real fwnode. All suggestions appreciated. Using the:
irq_domain_update_bus_token(intb_domain, DOMAIN_BUS_WIRED);
feels like a dirty hack, and won't scale if there is more HWIRQs.

I tried taking a look at this again.

If we wanted to support multiple HWIRQs / regmap-IRQ controller, it would require us to duplicate almost everything in the struct regmap_irq_chip for every new parent IRQ. The status/mask register information, IRQ type, etc. Naturally, it would require also duplicating lot of the data contained in the struct regmap_irq_chip_data. I am not sure if this could be done so the change is not reflected in the existing IRQ data initialization macros etc. Furthermore, some API changes would be required like changes to regmap_irq_get_domain().

I am a bit afraid this change, if implemented in regmap-IRQ, would be very intrusive and potentially impact large amount of callers. But more importantly, looking the amount of data that should be duplicated per new HWIRQ makes me think that an IRQ controller is really a product of a parent IRQ, not product of the device. Hence, assuming there is only one IRQ controller instance / device does not feel any more correct than assuming there is an IRQ controller instance / parent IRQ. Same thinking applies to IRQ domains.

Thus, forcing the regmap-IRQ to support multiple parents instead of having own regmap-IRQ instance / parent IRQ feels like fitting square item to a round hole. I am sure fixing all the bugs I caused would give donate a lot of EXP-points though :rolleyes:

Question is, should I still try?

Another option I see, is trying to think if irq-domain name could be changed. (This is what the RFC v3 does, [ab]using the irq_domain_update_bus_token()). I was a bit put off by the idea of 'instantiating' multiple domains (or regmap-IRQ controllers) from a single node, but more I think of this, more I lean towards it. Besides, this is not something completely odd, I think MFD devices do this anyways. (Instantiate multiple [sub]devices from single DT-node). I would love to get an opinion from someone who knows the 'fundamentals' of the IRQ domains, and possibly some pointer for the right approach.

Finally we might also consider adding own sub-node in DT for each parent IRQ - but this feels very wrong to me.

All in all, I am having very hard time trying to think how to proceed. The last option for me is to skip support for the ERRB IRQ from the BD96801 driver, which would leave this problem to the next person working with a device providing multiple physical IRQs.

Yours,
	-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux