linux-kernel - Re: [RFC PATCH 4/6] regulator: bd96801: ROHM BD96801 PMIC regulators

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <edb25c16-3f63-4f5b-97e5-363cf199d780@kernel.org>
Date: Wed, 3 Apr 2024 16:24:49 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>,
 Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc: Lee Jones <lee@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 4/6] regulator: bd96801: ROHM BD96801 PMIC regulators

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