Hi,
On Mon, 9 Mar 2020 at 02:22, chee.hong.ang@intel.com wrote:
From: Chee Hong Ang chee.hong.ang@intel.com
This commit (82de42fa14682d408da935adfb0f935354c5008f) calls child's ofdata_to_platdata() method before the parent is probed in dm core. This has caused the driver no longer able to get the correct parent clock's register base in the ofdata_to_platdata() method because the parent clocks will only be probed after the child's ofdata_to_platdata(). To resolve this, the clock parent's register base will only be retrieved by the child in probe() method instead of ofdata_to_platdata().
I think one thing that is going on here is that DM allows ofdata to be read for a device before its parent devices have been read, but it requires that parent devices be probed before their children.
The idea is that it should be possible to read the ofdata for a node without needing to have done so for parents. But perhaps this assumption is too brave?
I suspect we could change this, so that device_ofdata_to_platdata() first calls itself on its parent.
I can think of various reasons why this change might be desirable.
Signed-off-by: Chee Hong Ang chee.hong.ang@intel.com
drivers/clk/altera/clk-arria10.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/altera/clk-arria10.c b/drivers/clk/altera/clk-arria10.c index affeb31..b7eed94 100644 --- a/drivers/clk/altera/clk-arria10.c +++ b/drivers/clk/altera/clk-arria10.c @@ -274,6 +274,8 @@ static int socfpga_a10_clk_bind(struct udevice *dev) static int socfpga_a10_clk_probe(struct udevice *dev) { struct socfpga_a10_clk_platdata *plat = dev_get_platdata(dev);
struct socfpga_a10_clk_platdata *pplat;struct udevice *pdev; const void *fdt = gd->fdt_blob; int offset = dev_of_offset(dev);@@ -281,6 +283,21 @@ static int socfpga_a10_clk_probe(struct udevice *dev)
socfpga_a10_handoff_workaround(dev);
if (!fdt_node_check_compatible(fdt, offset, "altr,clk-mgr")) {
This is strange to me. I think the idea here is to detect whether this is the root clk node or one of the ones created in the bind() function above. Is that right?
If so, it should be enough to say:
if (dev_ofvalid(dev))
If you actually need to distinguish between different compatible strings, you can use the .data member in your udevice_id table, with dev_get_driver_data().
plat->regs = devfdt_get_addr(dev);} else {pdev = dev_get_parent(dev);if (!pdev)return -ENODEV;pplat = dev_get_platdata(pdev);if (!pplat)return -EINVAL;plat->ctl_reg = dev_read_u32_default(dev, "reg", 0x0);plat->regs = pplat->regs;}if (!fdt_node_check_compatible(fdt, offset, "altr,socfpga-a10-pll-clock")) { /* Main PLL has 3 upstream clock */@@ -304,29 +321,8 @@ static int socfpga_a10_clk_probe(struct udevice *dev) static int socfpga_a10_ofdata_to_platdata(struct udevice *dev) { struct socfpga_a10_clk_platdata *plat = dev_get_platdata(dev);
struct socfpga_a10_clk_platdata *pplat;struct udevice *pdev;const void *fdt = gd->fdt_blob; unsigned int divreg[3], gatereg[2];int ret, offset = dev_of_offset(dev);u32 regs;regs = dev_read_u32_default(dev, "reg", 0x0);if (!fdt_node_check_compatible(fdt, offset, "altr,clk-mgr")) {plat->regs = devfdt_get_addr(dev);} else {pdev = dev_get_parent(dev);if (!pdev)return -ENODEV;pplat = dev_get_platdata(pdev);if (!pplat)return -EINVAL;plat->ctl_reg = regs;plat->regs = pplat->regs;}
int ret; plat->type = SOCFPGA_A10_CLK_UNKNOWN_CLK;-- 2.7.4
Regards, Simon