Messages in this thread | | | Date | Wed, 3 Apr 2024 16:24:49 +0200 | Subject | Re: [RFC PATCH 4/6] regulator: bd96801: ROHM BD96801 PMIC regulators | From | Krzysztof Kozlowski <> |
| |
On 03/04/2024 09:38, Matti Vaittinen wrote: > Hi dee Ho Krzysztof, > > Heading to the Seattle? If so - Enjoy! It's a bummer I'm not able to > share a beer with you in ELC this time.
Second chance, I hope, will be Vienna in September.
..
>>> + >>> + rdesc = &pdata->regulator_data[0]; >>> + >>> + config.driver_data = pdata; >>> + config.regmap = pdata->regmap; >>> + config.dev = parent; >>> + >>> + ret = of_property_match_string(pdev->dev.parent->of_node, >>> + "interrupt-names", "errb"); >> This does not guarantee that interrupts are properly set up. > > Hmm. Yes, you're right. I'm not sure if I did think of this. > >> Don't you >> have some state shared between parent and this device where you could >> mark that interrupts are OK? > > There is currently no need to share/allocate any private data from the > MFD. We get the regmap using dev_get_regmap, and interrupts using the > platform_get_irq_byname(). Nothing else is shared between the MFD and > sub-devices. > > Considering the use of platform_get_irq_byname() - and how failures to > get 'errb' IRQs are silently ignored in bd96801_global_errb_irqs() and > in bd96801_rdev_errb_irqs() - this check is just a slight optimization > to not even try registering the errb IRQs if they're not found from the > device tree. So, I think things do not really go south even if we go to > "errb route" when the "errb" IRQs aren't successfully registered. > > Whether this warrants a comment, or if this check is just unnecessarily > complex can be pondered. Personally I think the purpose is pretty clear > and thus the complexity is not added that much - but yes, a comment > above call(s) to the platform_get_irq_byname() saying errb IRQs are not > guaranteed to be populated might be justified. >
Fine with me.
Best regards, Krzysztof
| |