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;
Message-ID: <3d684dec-338a-085f-3ef1-1642e5067fb2@baylibre.com>
Date:   Fri, 24 Mar 2023 09:00:01 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH INTERNAL v1 3/3] regulator: tps6594-regulator: Add driver
 for TI TPS6594 regulators
Content-Language: en-US
To:     Mark Brown <broonie@kernel.org>
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
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>
 <e08686be-0b46-403b-b3cd-3462db92dd60@sirena.org.uk>
From:   jerome Neanne <jneanne@baylibre.com>
In-Reply-To: <e08686be-0b46-403b-b3cd-3462db92dd60@sirena.org.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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.