linux-kernel - Re: [PATCH v2 04/15] mux: add mux_chip

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: <6568893d-13c7-ef1f-9c3f-88de0701c7aa@axentia.se>
Date: Tue, 30 Jan 2024 09:25:21 +0100
From: Peter Rosin <peda@...ntia.se>
To: Thomas Richard <thomas.richard@...tlin.com>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
 <brgl@...ev.pl>, Andy Shevchenko <andy@...nel.org>,
 Tony Lindgren <tony@...mide.com>, Haojian Zhuang
 <haojian.zhuang@...aro.org>, Vignesh R <vigneshr@...com>,
 Aaro Koskinen <aaro.koskinen@....fi>,
 Janusz Krzysztofik <jmkrzyszt@...il.com>, Andi Shyti
 <andi.shyti@...nel.org>, Vinod Koul <vkoul@...nel.org>,
 Kishon Vijay Abraham I <kishon@...nel.org>,
 Philipp Zabel <p.zabel@...gutronix.de>, Tom Joseph <tjoseph@...ence.com>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Krzysztof Wilczyński <kw@...ux.com>,
 Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>
Cc: linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-omap@...r.kernel.org,
 linux-i2c@...r.kernel.org, linux-phy@...ts.infradead.org,
 linux-pci@...r.kernel.org, gregory.clement@...tlin.com,
 theo.lebrun@...tlin.com, thomas.petazzoni@...tlin.com, u-kumar1@...com
Subject: Re: [PATCH v2 04/15] mux: add mux_chip_resume() function

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@...tlin.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...

Cheers,
Peter

> +			}
> +		}
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_resume);
> +
>  static void devm_mux_chip_release(struct device *dev, void *res)
>  {
>  	struct mux_chip *mux_chip = *(struct mux_chip **)res;
> diff --git a/include/linux/mux/driver.h b/include/linux/mux/driver.h
> index 18824064f8c0..2a7e5ec5d540 100644
> --- a/include/linux/mux/driver.h
> +++ b/include/linux/mux/driver.h
> @@ -88,6 +88,7 @@ struct mux_chip *mux_chip_alloc(struct device *dev,
>  int mux_chip_register(struct mux_chip *mux_chip);
>  void mux_chip_unregister(struct mux_chip *mux_chip);
>  void mux_chip_free(struct mux_chip *mux_chip);
> +int mux_chip_resume(struct mux_chip *mux_chip);
>  
>  struct mux_chip *devm_mux_chip_alloc(struct device *dev,
>  				     unsigned int controllers,
>