Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 18 Aug 2022 22:22:31 +0300 | Subject | Re: [PATCH linux-phy v2 2/4] device property: Add {fwnode/device}_get_tx_p2p_amplitude() |
| |
On Wed, Aug 17, 2022 at 11:09 PM Marek Behún <kabel@kernel.org> wrote: > > Add functions fwnode_get_tx_p2p_amplitude() and > device_get_tx_p2p_amplitude() that parse the 'tx-p2p-microvolt' and > 'tx-p2p-microvolt-names' properties and return peak to peak transmit > amplitude in microvolts for given PHY mode. > > The functions search for mode name in 'tx-p2p-microvolt-names' property, > and return amplitude at the corresponding index in the 'tx-p2p-microvolt' > property. > > If given mode is not matched in 'tx-p2p-microvolt-names' array, the mode > name is generalized (for example "pcie3" -> "pcie" -> "default", or > "usb-ss" -> "usb" -> "default"). > > If the 'tx-p2p-microvolt-names' is not present, the 'tx-p2p-microvolt' > property is expected to contain only one value, which is considered > default, and will be returned for any mode.
It's very specific to a domain. NAK for putting it to the generic code, otherwise explain how it can be useful outside of the PHY world.
...
> + cnt = fwnode_property_string_array_count(fwnode, names_prop); > + if (!cnt || cnt == -EINVAL) > + /* > + * If the names property does not exist or is empty, we expect > + * the values property to contain only one, default value. > + */ > + return fwnode_property_read_u32(fwnode, vals_prop, amplitude); > + else if (cnt < 0) > + return cnt;
You may count the values and read them all, and then check the names and compare count to the read values. In such a case you don't need too many (overlapped) checks. I think the current implementation is far from being optimal. Take your time and try to get rid of 20% of lines in this function. I believe it's doable.
...
> + * Gets the peak to peak transmit amplitude in microvolts for a given PHY mode > + * by parsing the 'tx-p2p-microvolt' and 'tx-p2p-microvolt-names' properties. > + * If amplitude is not specified for @mode exactly, tries a more generic mode, > + * and if that isn't specified, tries "default".
Gets --> Get tries --> try
Otherwise add a subject to the sentences.
-- With Best Regards, Andy Shevchenko
| |