linux-kernel - Re: [PATCH INTERNAL v1 3/3] regulator: tps6594-regulator: Add driver for TI TPS6594 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 for Android: free password hash cracker in your pocket

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

Message-ID: <3d684dec-338a-085f-3ef1-1642e5067fb2@baylibre.com>
Date:   Fri, 24 Mar 2023 09:00:01 +0100
From:   jerome Neanne <jneanne@...libre.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Esteban Blanc <eblanc@...libre.com>, linus.walleij@...aro.org,
        lgirdwood@...il.com, a.zummo@...ertech.it,
        alexandre.belloni@...tlin.com, linux-kernel@...r.kernel.org,
        linux-gpio@...r.kernel.org, linux-rtc@...r.kernel.org,
        jpanis@...libre.com
Subject: Re: [PATCH INTERNAL v1 3/3] regulator: tps6594-regulator: Add driver
 for TI TPS6594 regulators



On 23/03/2023 12:38, Mark Brown wrote:
> On Thu, Mar 23, 2023 at 10:12:21AM +0100, jerome Neanne wrote:
> 
>>> This would be simpler and you wouldn't need this lookup function if the
>>> regulator descriptions included their IRQ names, then you could just
>>> request the interrupts while registering the regulators.
> 
>> I changed the code to follow your recommendations then now in case of a
>> multiphase buck, only one set of interrupt is requested.
> 
>> buck2, buck3, buck4 are not associated to a regulator device because buck1
>> registers control all the multiphase bucks (only one logic regulator).
>> Consequently the mapping for the associated interrupts does not occur.
>> I'm not sure it's the right option.
>> Do you suggest to keep it like that for multiphase?
>> Is it better to request all the interrupts anyway and map it to the same
>> rdev?
> 
> Do the other interrupts do anything useful for this configuration?  With
> a lot of hardware the whole control interface gets merged into one which
> includes the interrupts.
> 
Discussed the point with TI in //. In case of multiphase buck ex: buck12
All the control is delegated to buck1 registers but there is still a 
possibility that an interrupt triggers on buck2 (overcurrent typically).
I slightly changed the logic so that all the interrupts are registered 
even in multiphase mode. In that case interrupts for buck2 are attached 
to rdev buck12.
>>>> +		error = devm_request_threaded_irq(tps->dev, irq, NULL,
>>>> +						  tps6594_regulator_irq_handler,
>>>> +						  IRQF_ONESHOT,
>>>> +						  irq_type->irq_name,
>>>> +						  &irq_data[i]);
>>>> +		if (error) {
>>>> +			dev_err(tps->dev, "failed to request %s IRQ %d: %d\n",
>>>> +				irq_type->irq_name, irq, error);
>>>> +			return error;
>>>> +		}
> 
>>> This leaks all previously requested interrupts.
> 
>> I'm not sure to understand this sentence correctly. You mean all the
>> interrupts already requested are still allocated after the error occurs?
> 
> Yes, I'd either not registered the devm or thought there was some other
> interrupt wasn't devm.
All the interrupts are requested with devm, then should be fine.