Re: [PATCH v2 04/15] mux: add mux_chip_resume() function

From: Thomas Richard <thomas.richard@bootlin.com>
To: "Peter Rosin" <peda@axentia.se>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Bartosz Golaszewski" <brgl@bgdev.pl>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Tony Lindgren" <tony@atomide.com>,
	"Haojian Zhuang" <haojian.zhuang@linaro.org>,
	"Vignesh R" <vigneshr@ti.com>,
	"Aaro Koskinen" <aaro.koskinen@iki.fi>,
	"Janusz Krzysztofik" <jmkrzyszt@gmail.com>,
	"Andi Shyti" <andi.shyti@kernel.org>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Tom Joseph" <tjoseph@cadence.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
	linux-i2c@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-pci@vger.kernel.org, gregory.clement@bootlin.com,
	theo.lebrun@bootlin.com, thomas.petazzoni@bootlin.com,
	u-kumar1@ti.com
Subject: Re: [PATCH v2 04/15] mux: add mux_chip_resume() function
Date: Tue, 30 Jan 2024 17:24:55 +0100	[thread overview]
Message-ID: <344da0db-55ef-4445-8e14-9c2f53e0c33c@bootlin.com> (raw)
In-Reply-To: <6568893d-13c7-ef1f-9c3f-88de0701c7aa@axentia.se>

On 1/30/24 09:25, Peter Rosin wrote:
> Hi!
> 
> 2024-01-26 at 15:36, Thomas Richard wrote:
>> The mux_chip_resume() function restores a mux_chip using the cached state
>> of each mux.
>>
>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
>> ---
>>  drivers/mux/core.c         | 27 +++++++++++++++++++++++++++
>>  include/linux/mux/driver.h |  1 +
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
>> index 775816112932..896f74b34eb8 100644
>> --- a/drivers/mux/core.c
>> +++ b/drivers/mux/core.c
>> @@ -215,6 +215,33 @@ void mux_chip_free(struct mux_chip *mux_chip)
>>  }
>>  EXPORT_SYMBOL_GPL(mux_chip_free);
>>  
>> +/**
>> + * mux_chip_resume() - restores the mux-chip state
>> + * @mux_chip: The mux-chip to resume.
>> + *
>> + * Restores the mux-chip state.
>> + *
>> + * Return: Zero on success or a negative errno on error.
>> + */
>> +int mux_chip_resume(struct mux_chip *mux_chip)
>> +{
>> +	int ret, i;
>> +
>> +	for (i = 0; i < mux_chip->controllers; ++i) {
>> +		struct mux_control *mux = &mux_chip->mux[i];
>> +
>> +		if (mux->cached_state != MUX_CACHE_UNKNOWN) {
>> +			ret = mux_control_set(mux, mux->cached_state);
>> +			if (ret < 0) {
>> +				dev_err(&mux_chip->dev, "unable to restore state\n");
>> +				return ret;
> 
> I'm don't know what is expected of the core resume code on error,
> but is it ok to return on first failure? Is it not better to try
> to restore all muxes and return zero if all is well or the first
> failure when something is up?
> 
> But maybe the resume is completely dead anyway if there is any
> failure? In that case the above early return is fine, I guess...
> 

In the first iteration of this series (when it was done in mmio driver),
it restored all muxes and returned zero or the first failure.
I don't know why I changed the behaviour.
For me it's better to try to restores all muxes.

-- 
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel