linux-kernel - Re: [PATCH 1/5] of: Add of_property_read_[u32,s32]

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] [day] [month] [year] [list]

Message-ID: <20260120222155.GA1226342-robh@kernel.org>
Date: Tue, 20 Jan 2026 16:21:55 -0600
From: Rob Herring <robh@...nel.org>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
Cc: Saravana Kannan <saravanak@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>,
	Alexis Czezar Torreno <alexisczezar.torreno@...log.com>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Peng Fan <peng.fan@....com>
Subject: Re: [PATCH 1/5] of: Add of_property_read_[u32,s32]_default

On Mon, Jan 19, 2026 at 10:02:54AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@....com>
> 
> Introduce new helper functions of_property_read_u32_default() and
> of_property_read_s32_default() to simplify reading optional device tree
> properties with a default value.
> 
> A very common pattern in drivers is to provide a default value and let
> of_property_read_*() override it when the property is present, e.g.:
> 
>     Y = Y_DEFAULT;
>     of_property_read_u32(np, "prop", &Y);

This is how defaults were intended to be handled.
> 
> or equivalently, checking the return value explicitly:
> 
>     ret = of_property_read_u32(np, "prop", &val);
>     if (ret)
>         Y = Y_DEFAULT;
>     else
>         Y = val;

This is usually only needed if the variable type is different. Probably 
the better fix is fix the type difference.

> Both forms express the same intent: the property is optional and a
> well-defined default should be used if it cannot be read.
> 
> With the new helper, this can be expressed more directly as:
> 
>     Y = of_property_read_u32_default(np, "prop", Y_DEFAULT);
> 
> The helpers intentionally ignore the error code and return either the
> parsed value or the supplied default. They are meant for optional
> properties only. Callers that need to handle or propagate errors should
> continue using of_property_read_*() directly.

What about u8, u16, etc. and device_property_read_*? I'm really on the 
fence whether this is all worth it...

We may also want to do something like of_property_read() implemented 
using C11 _Generic(). Not sure if that's worth the churn either. It 
would make doing some type checks harder. For example I could extract 
all property names from of_property_read_u32() calls and check their 
size against the schemas. (I have the first half of that already.) Using 
_Generic() would make that harder or impossible.

Rob