[PATCH v1 1/2] clk: socfpga: Read the clock parent's register base in probe function

[Ang, Chee Hong]

> On 3/9/20 9:21 AM, chee.hong.ang at intel.com wrote:
> > From: Chee Hong Ang <chee.hong.ang at 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().
> 
> You should be able to bind the drivers and resolve their register offsets without
> probing them, so this look more like a bug in the driver core ?
The problem is the children clock driver need to resolve/derive their register base
from their clock parents. With this new change, clock parents are still not yet
being initialized when the children clock drivers need to resolve their register base
from their parent.
A10 is not booting in mainline due to this issue.
I can't think of a better way to fix this. Should we fix the clock driver itself or the
DM core ?
> 
> > Signed-off-by: Chee Hong Ang <chee.hong.ang at 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")) {
> > +		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;
> >
> >
> 
> 
> --
> Best regards,
> Marek Vasut