On 2/27/2020 5:43 PM, Andy Shevchenko wrote:
On Thu, Feb 27, 2020 at 9:54 AM Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx> wrote:
...
+static int intel_cbphy_iphy_dt_parse(struct intel_combo_phy *cbphy,
dt -> fwnode
Ditto for other similar function names.
Sure, it looks appropriate for intel_cbphy_iphy_dt_parse() ->
intel_cbphy_iphy_fwnode_parse().
Whereas for intel_cbphy_dt_parse() i will keep it unchanged, because it
is calling devm_*, devm_platform_*, fwnode_* APIs to traverse dt node.
How do you know that it will be DT node?
I can't say it from the function parameters: Is any of them takes of_node?
Got it, All the functions are traversing through device only. I will
change intel_cbphy_dt_parse() to intel_cbphy_fwnode_parse().
(PS: My intention is something different. As the function is fetching
device tree node entries so kept is as *_dt_parse() )
+ struct fwnode_handle *fwnode, int idx)
...
+ struct fwnode_reference_args ref;
+ struct device *dev = cbphy->dev;
+ struct fwnode_handle *fwnode;
+ struct platform_device *pdev;
+ int i, ret;
+ u32 prop;
I guess the following would be better:
In the v2 patch, for int i = 0 you mentioned to do initialization at the
user, instead of doing at declaration.
So i followed the same for "pdev" and "fwnode" which are being used
after few lines of the code . It looked good in the perspective of code
readability.
No, it is different. For the loop counter is better to have closer to
the loop, for the more global thingy like platform device it makes it
actually harder to find.
When you do assignments you have to think about the variable meaning
and scope. Scope is different for loop counter versus the mentioned
rest.
Understand. I will follow the same and keep a note for future drivers too.
Thanks for detail explanation.
Regards,
Dilip
.
