linux-kernel - Re: [PATCH v3 2/6] mux: Add helper functions for getting optional and selected mux-state

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: <fd901bba-3203-46c2-b282-c5ad04128fd3@solid-run.com>
Date: Mon, 29 Dec 2025 12:01:17 +0000
From: Josua Mayer <josua@...id-run.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
CC: Ulf Hansson <ulf.hansson@...aro.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>,
	Marc Kleine-Budde <mkl@...gutronix.de>, Vincent Mailhol <mailhol@...nel.org>,
	Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.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>, Vignesh R
	<vigneshr@...com>, Janusz Krzysztofik <jmkrzyszt@...il.com>, Andi Shyti
	<andi.shyti@...nel.org>, Mikhail Anikin <mikhail.anikin@...id-run.com>, Yazan
 Shhady <yazan.shhady@...id-run.com>, Jon Nettleton <jon@...id-run.com>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
	"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: Re: [PATCH v3 2/6] mux: Add helper functions for getting optional and
 selected mux-state


Am 22.12.25 um 11:08 schrieb Geert Uytterhoeven:
> Hi Josua,
>
> On Wed, 10 Dec 2025 at 18:39, 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:
>>
>> - devm_mux_state_get_optional:
>>   Get a mux-state if specified in dt, return NULL 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 return -ENOENT in case dt
>> did not specify a mux-state or -control matching given name (if valid).
>> This matches of_parse_phandle_with_args semantics which also returns
>> -ENOENT if the property does nto exists, or its value is zero.
>>
>> The new helper functions check for ENOENT to return NULL for optional
>> muxes.
>>
>> 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. This is removed as the returned error code (-ENOENT) is clear.
>>
>> The second error message is based on of_parse_phandle_with_args return
>> value. In case mux description is missing from DT, it returns -ENOENT.
>> Print error message only for other error codes.
>>
>> This ensures that the new helper functions will not confuse the user
>> either.
>>
>> Signed-off-by: Josua Mayer <josua@...id-run.com>
> Thanks for your patch!
>
>> --- a/drivers/mux/core.c
>> +++ b/drivers/mux/core.c
>> @@ -542,11 +542,8 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>>                 else
>>                         index = of_property_match_string(np, "mux-control-names",
>>                                                          mux_name);
>> -               if (index < 0) {
>> -                       dev_err(dev, "mux controller '%s' not found\n",
>> -                               mux_name);
>> -                       return ERR_PTR(index);
>> -               }
>> +               if (index < 0)
>> +                       return ERR_PTR(-ENOENT);
>>         }
>>
>>         if (state)
>> @@ -558,8 +555,10 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>>                                                  "mux-controls", "#mux-control-cells",
>>                                                  index, &args);
>>         if (ret) {
>> -               dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
>> -                       np, state ? "state" : "control", mux_name ?: "", index);
>> +               if (ret != -ENOENT)
> I think the non-optional variant should still print an error message in
> case of -ENOENT, else this has to be duplicated in all drivers using it.
>
> This is typically handled by having a non-printing core helper function,
> and having printing non-optional, and non-printing/ignoring optional wrappers
> around the former.

I would prefer letting drivers use dev_err_probe.
Silent helper functions can more easily share code between them ...

If this is a strong preference I can rework the error behaviour and modify
the relevant mux_state_get and mux_control_get.

>
>> +                       dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
>> +                               np, state ? "state" : "control",
>> +                               mux_name ?: "", index);
>>                 return ERR_PTR(ret);
>>         }
>>
> Gr{oetje,eeting}s,
>
>                         Geert
>