On Thu, Jun 27, 2019 at 01:02:32PM +0200, Rafael J. Wysocki wrote: > On Thu, Jun 27, 2019 at 12:23 PM Charles Keepax > <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, Jun 27, 2019 at 11:39:10AM +0200, Rafael J. Wysocki wrote: > > > On Wed, Jun 26, 2019 at 5:36 PM Charles Keepax > > > <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote: > > That is fine, I don't have any problem with the helper living > > within our driver instead. Basically the issue from my side is I > > need to read 6 different device tree properties all of which > > require this behaviour, ie. read a variable number of fixed > > groups and check I have whole groups. Open coding this for each > > call is indeed only going to be 5-10 lines of code > > Exactly two: > > n = device_property_count_u32(dev, name); > ret = device_property_read_u32_array(dev, propname, val, n > nval ? nval : n); > > And I would be fine with adding wrappers like this (and for the other > data types too for that matter). > > It would take more lines if you wanted to complain about the format, > but as pointed out above, that would need to be done in the caller > anyway. > Ok I think that helps me to follow the situation. If device_property_count cuts down the code required and leaves the majority of the code as printing the messages which then wants to live in the end driver anyways it probably isn't worth adding a core helper for this. Thank you for the review and the explanation. I will update the driver patches to use the new function and resend those. Thanks, Charles > > for each one but since there are 6 of them it makes sense to put those 5-10 > > lines into a helper and have 5-10 lines not 30-60 lines. Seemed > > the helper might be generally more useful, but if it is not then > > it can go back into the driver.
