Re: [PATCH linux-phy v2 2/4] device property: Add {fwnode/device}_get_tx_p2p_amplitude()

[Marek Behún]

On Thu, 18 Aug 2022 23:10:09 +0300
Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:

> Yes, I have no objection to put it there, just that the above
> justification doesn't allow it to be in the generic code (yes, we may
> still have some awkward APIs in the property.c and ideally they should
> be moved to the respective subsystems).

OK

> > > You may count the values and read them all,  
> >
> > What do you mean? Count the values and read them all via one
> > call to fwnode_property_string_array_count() ?  
> 
> No, you obviously may not read them via string_array APIs, esp. one
> that is related to counting.
> 
> Count the vals first, read them all (it seems you need it in all
> branches of your flow). Then count names and compare them to the
> number of values, and so on... Also try to assign "default" only once.

1. there is one branch where I don't need to read the values: when the
   "-names" property does not exist, the DT binding documentation says
   that the value property should only contain one value, the default
   one. So in that case I early return
     return fwnode_property_read_u32(fwnode, vals_prop, amplitude);

2. I thought that I shouldn't check whether the size of the
   "tx-p2p-microvolt-names" array is equal to the size of
   "tx-p2p-microvolt". Rob Herring says (if I understand correctly) that
   kernel shouldn't validate device-tree, that we have dt-schema for
   that...

Marek