Hi Masahiro,
On 29 December 2017 at 10:00, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> dev_read_u32_default() always returns something even when the property
> is missing. So, it is impossible to do nothing in the case. One
> solution is to use ofnode_read_u32() instead, but adding dev_read_u32()
> will be helpful.
>
> BTW, Linux has an equvalent function, device_property_read_u32();
> it is clearer that it reads a property. I cannot understand the
> behavior of dev_read_u32() from its name.
I decided that 'property' was an unnecessary word since there is
nothing else you can ready a value from in the DT.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
> BTW, I do not understand why we want *_default helpers.
> It is pretty easy to do equivalent without _default.
>
> priv->val = default; /* default in case property is missing */
> dev_read_u32(dev, "foo-property", &priv->val);
>
Your example explains it. Instead, we can do:
priv->val = dev_read_u32_default(dev,"foo-property", default);
which is simpler to understand and does not need a comment.
> On the other hand, if we want to do nothing for missing property,
> we will end up with clumsy code.
>
> /* we do not want to overwrite priv->val if property is missing */
> if (dev_read_bool(dev, "foo-property") {
> /* default value '0' will not be used anyway */
> priv->val = dev_read_u32_default(dev, "foo-property", 0);
> }
priv->val = dev_read_u32_default(dev,"foo-property", priv->val);
>
> One more BTW, it was just painful to write equivalent code twice
> for CONFIG_DM_DEV_READ_INLINE.
This helps reduce code size for the case where we don't use livetree
(SPL). Any ideas on how to make it easier?
>
>
> drivers/core/read.c | 5 +++++
> include/dm/read.h | 16 ++++++++++++++++
> 2 files changed, 21 insertions(+)
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
