Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 23 Mar 2023 11:38:29 +0000
From:   Mark Brown <broonie@kernel.org>
To:     jerome Neanne <jneanne@baylibre.com>
Cc:     Esteban Blanc <eblanc@baylibre.com>, linus.walleij@linaro.org,
        lgirdwood@gmail.com, a.zummo@towertech.it,
        alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org,
        linux-gpio@vger.kernel.org, linux-rtc@vger.kernel.org,
        jpanis@baylibre.com
Subject: Re: [PATCH INTERNAL v1 3/3] regulator: tps6594-regulator: Add driver
 for TI TPS6594 regulators
Message-ID: <e08686be-0b46-403b-b3cd-3462db92dd60@sirena.org.uk>
References: <20230224133129.887203-1-eblanc@baylibre.com>
 <20230224133129.887203-4-eblanc@baylibre.com>
 <Y/i+wVSy+eQxDFJ3@sirena.org.uk>
 <bd4aa2ad-4535-94ca-7630-846546ae3d82@baylibre.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="sqyr3pdl018Qcdzn"
Content-Disposition: inline
In-Reply-To: <bd4aa2ad-4535-94ca-7630-846546ae3d82@baylibre.com>
Precedence: bulk


--sqyr3pdl018Qcdzn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

--sqyr3pdl018Qcdzn
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmQcOjQACgkQJNaLcl1U
h9Auuwf+PN5kS1OlVilZrrkY1SzeCUq5W+chBmTUcyD85lp5xFNkACPMHE1lYwjE
uVj1L15Dqzt9wbKb5i5/c1p2aiumc4Uz6EYkgn5JIQ5ozBBUI9JYzhyWdPOWKw4/
Vr3NvPPkXLy/na7nRyuGJKmexIo/189mXkIRya3jDLIV7iI0arIc4wdXRJDe3YyO
DbFIaBVG3nudg6GGf1zKJ9rrkB/HMd4WFfbDURoQm4yicpTx2sP7SQQ4zk1KC4QT
hcRVB60RoXOYaESfSvPavmEwMXcrNgQrGCiePgvOUHI1VTMtqDN/3DfM+HD5YgNF
pN2QCR1QG69NJC6jSt3EpqYcayQsJw==
=T/Ls
-----END PGP SIGNATURE-----

--sqyr3pdl018Qcdzn--