Hi Masahiro,
On 29 December 2017 at 10:00, Masahiro Yamada yamada.masahiro@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.masahiro@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