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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <e08686be-0b46-403b-b3cd-3462db92dd60@sirena.org.uk>
Date:   Thu, 23 Mar 2023 11:38:29 +0000
From:   Mark Brown <broonie@...nel.org>
To:     jerome Neanne <jneanne@...libre.com>
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 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.

> > > +		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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)