| 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: <70305238-a9eb-4803-a5ed-5a6cd4d618d2@solid-run.com>
Date: Tue, 3 Feb 2026 11:28:21 +0000
From: Josua Mayer <josua@...id-run.com>
To: Ulf Hansson <ulf.hansson@...aro.org>, Geert Uytterhoeven
<geert@...ux-m68k.org>
CC: 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-can@...r.kernel.org>, "linux-phy@...ts.infradead.org"
<linux-phy@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-omap@...r.kernel.org"
<linux-omap@...r.kernel.org>, "linux-i2c@...r.kernel.org"
<linux-i2c@...r.kernel.org>, "linux-mmc@...r.kernel.org"
<linux-mmc@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-renesas-soc@...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 30/01/2026 12:28, Ulf Hansson wrote:
> 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.
The intent behind "optional" parameter was to conditionally
silence certain error paths that would eventually lead to
at least the exported devm_* functions to return NULL.
I'll review my logic ...
>
> 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