Re: [PATCH 1/5] of: Add of_property_read_[u32,s32]_default

[Peng Fan <peng.fan@oss.nxp.com>]

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