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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aXMb/bkMoaOiM6sX@shlinux89>
Date: Fri, 23 Jan 2026 14:58:05 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Rob Herring <robh@...nel.org>
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

Hi Rob,

On Tue, Jan 20, 2026 at 04:21:55PM -0600, Rob Herring wrote:
>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.

I see. There are a few places that use above style to set default value,
no type conversion.

such as
drivers/clk/socfpga/clk-gate-a10.c:71
drivers/clk/socfpga/clk-periph-a10.c:90

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

My cocci only reports some u32 usage using style "if.. else..".
Let me write a cocci pattern to scan device_property_read_* usage.

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

I see. _Generic may not be a good idea if it makes type checks harder.

Thanks
Peng

>
>Rob
>