| 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
| ||
|
Message-ID: <CAPDyKFrvSp6tmhP=2=9kca1d3+b6MezvxJWwMOW=Gv6D8TKrgA@mail.gmail.com>
Date: Fri, 30 Jan 2026 11:28:24 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Josua Mayer <josua@...id-run.com>, Marc Kleine-Budde <mkl@...gutronix.de>,
Vincent Mailhol <mailhol@...nel.org>, Vinod Koul <vkoul@...nel.org>,
Neil Armstrong <neil.armstrong@...aro.org>, Peter Rosin <peda@...ntia.se>,
Aaro Koskinen <aaro.koskinen@....fi>, Andreas Kemnade <andreas@...nade.info>,
Kevin Hilman <khilman@...libre.com>, Roger Quadros <rogerq@...nel.org>,
Tony Lindgren <tony@...mide.com>, Janusz Krzysztofik <jmkrzyszt@...il.com>, Vignesh R <vigneshr@...com>,
Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Geert Uytterhoeven <geert+renesas@...der.be>, Magnus Damm <magnus.damm@...il.com>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Yazan Shhady <yazan.shhady@...id-run.com>, Jon Nettleton <jon@...id-run.com>,
Mikhail Anikin <mikhail.anikin@...id-run.com>, linux-can@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-omap@...r.kernel.org, linux-i2c@...r.kernel.org,
linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
linux-renesas-soc@...r.kernel.org, Dan Carpenter <dan.carpenter@...aro.org>
Subject: Re: [PATCH v7 2/7] mux: Add helper functions for getting optional and
selected mux-state
On Thu, 29 Jan 2026 at 18:46, Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
>
> On Thu, 29 Jan 2026 at 17:06, Ulf Hansson <ulf.hansson@...aro.org> wrote:
> > On Wed, 28 Jan 2026 at 15:46, Josua Mayer <josua@...id-run.com> wrote:
> > >
> > > In-tree phy-can-transceiver driver has already implemented a local
> > > version of devm_mux_state_get_optional.
> > >
> > > The omap-i2c driver gets and selects an optional mux in its probe
> > > function without using any helper.
> > >
> > > Add new helper functions covering both aforementioned use-cases:
> > >
> > > - mux_control_get_optional:
> > > Get a mux-control if specified in dt, return NULL otherwise.
> > > - devm_mux_state_get_optional:
> > > Get a mux-state if specified in dt, return NULL otherwise.
> > > - devm_mux_state_get_selected:
> > > Get and select a mux-state specified in dt, return error otherwise.
> > > - devm_mux_state_get_optional_selected:
> > > Get and select a mux-state if specified in dt, return error or NULL.
> > >
> > > Existing mux_get helper function is changed to take an extra argument
> > > indicating whether the mux is optional.
> > > In this case no error is printed, and NULL returned in case of ENOENT.
> > >
> > > Calling code is adapted to handle NULL return case, and to pass optional
> > > argument as required.
> > >
> > > To support automatic deselect for _selected helper, a new structure is
> > > created storing an exit pointer similar to clock core which is called on
> > > release.
> > >
> > > To facilitate code sharing between optional/mandatory/selected helpers,
> > > a new internal helper function is added to handle quiet (optional) and
> > > verbose (mandatory) errors, as well as storing the correct callback for
> > > devm release: __devm_mux_state_get
> > >
> > > Due to this structure devm_mux_state_get_*_selected can no longer print
> > > a useful error message when select fails. Instead callers should print
> > > errors where needed.
> > >
> > > Commit e153fdea9db04 ("phy: can-transceiver: Re-instate "mux-states"
> > > property presence check") noted that "mux_get() always prints an error
> > > message in case of an error, including when the property is not present,
> > > confusing the user."
> > >
> > > The first error message covers the case that a mux name is not matched
> > > in dt. The second error message is based on of_parse_phandle_with_args
> > > return value.
> > >
> > > In optional case no error is printed and NULL is returned.
> > > This ensures that the new helper functions will not confuse the user
> > > either.
> > >
> > > With the addition of optional helper functions it became clear that
> > > drivers should compile and link even if CONFIG_MULTIPLEXER was not enabled.
> > > Add stubs for all symbols exported by mux core.
> > >
> > > Signed-off-by: Josua Mayer <josua@...id-run.com>
> > > ---
> > > drivers/mux/core.c | 178 ++++++++++++++++++++++++++++++++++++-------
> > > include/linux/mux/consumer.h | 108 +++++++++++++++++++++++++-
> > > 2 files changed, 253 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> > > index a3840fe0995f..b01ec126caaf 100644
> > > --- a/drivers/mux/core.c
> > > +++ b/drivers/mux/core.c
> >
> > [...]
> >
> > > static void devm_mux_state_release(struct device *dev, void *res)
> > > {
> > > - struct mux_state *mstate = *(struct mux_state **)res;
> > > + struct devm_mux_state_state *devm_state = res;
> > >
> > > - mux_state_put(mstate);
> > > + if (devm_state->exit)
> > > + devm_state->exit(devm_state->mstate);
> > > +
> > > + mux_state_put(devm_state->mstate);
> > > }
> > >
> > > /**
> > > - * devm_mux_state_get() - Get the mux-state for a device, with resource
> > > - * management.
> > > - * @dev: The device that needs a mux-control.
> > > - * @mux_name: The name identifying the mux-control.
> > > + * __devm_mux_state_get() - Get the optional mux-state for a device,
> > > + * with resource management.
> > > + * @dev: The device that needs a mux-state.
> > > + * @mux_name: The name identifying the mux-state.
> > > + * @optional: Whether to return NULL and silence errors when mux doesn't exist.
> > > + * @init: Optional function pointer for mux-state object initialisation.
> > > + * @exit: Optional function pointer for mux-state object cleanup on release.
> > > *
> > > * Return: Pointer to the mux-state, or an ERR_PTR with a negative errno.
> > > */
> > > -struct mux_state *devm_mux_state_get(struct device *dev,
> > > - const char *mux_name)
> > > +static struct mux_state *__devm_mux_state_get(struct device *dev, const char *mux_name,
> > > + bool optional,
> > > + int (*init)(struct mux_state *mstate),
> > > + int (*exit)(struct mux_state *mstate))
> > > {
> > > - struct mux_state **ptr, *mstate;
> > > + struct devm_mux_state_state *devm_state;
> > > + struct mux_state *mstate;
> > > + int ret;
> > >
> > > - ptr = devres_alloc(devm_mux_state_release, sizeof(*ptr), GFP_KERNEL);
> > > - if (!ptr)
> > > + devm_state = devres_alloc(devm_mux_state_release, sizeof(*devm_state), GFP_KERNEL);
> > > + if (!devm_state)
> > > return ERR_PTR(-ENOMEM);
> > >
> > > - mstate = mux_state_get(dev, mux_name);
> > > - if (IS_ERR(mstate)) {
> > > - devres_free(ptr);
> > > - return mstate;
> > > + mstate = mux_state_get(dev, mux_name, optional);
> > > + if (IS_ERR_OR_NULL(mstate)) {
> > > + ret = PTR_ERR(mstate);
> >
> > Should this be PTR_ERR_OR_ZERO?
>
> "mux_state_get() never returns NULL"
> https://lore.kernel.org/202601221036.J0kR78Uw-lkp@intel.com
By looking at the code in mux_state_get() and mux_get() that $subject
patch changes, it intends to add a "bool optional" as in-parameter. If
it's set, it looks like the intent is to allow returning NULL, which
makes sense to me.
Anyway, there seems to be some additional smatch warnings to fix
before this is ready to go.
[...]
Kind regards
Uffe
Powered by blists - more mailing lists