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

[Ang, Chee Hong]

> On 4/2/20 4:34 AM, Simon Glass wrote:
> > Hi,
> >
> > On Tue, 31 Mar 2020 at 20:33, Ang, Chee Hong <chee.hong.ang at intel.com>
> wrote:
> >>
> >>> Hi Marek,
> >>>
> >>> On Wed, 11 Mar 2020 at 05:55, Marek Vasut <marex at denx.de> wrote:
> >>>>
> >>>> On 3/11/20 12:50 PM, Simon Glass wrote:
> >>>>> Hi,
> >>>>
> >>>> Hi,
> >>>>
> >>>>> On Mon, 9 Mar 2020 at 02:22, <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().
> >>>>>
> >>>>> 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.
> >>>>
> >>>> This seems wrong. The clock driver should be able to instantiate
> >>>> devices and read their ofdata without probing them. That is one of
> >>>> the core design principles of the DM.
> >>>
> >>> That's a different question. Yes you can read ofdata without probing a
> device.
> >>> That's why we have two methods.
> >>>
> >>> The point I am making is that at present there is no requirement
> >>> that a parent's ofdata be read before a child's ofdata is read. But
> >>> there is a requirement that a parent be probed before a child is probed.
> >>>
> >>>>
> >>>>> 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?
> >>>>
> >>>> Why is it brave ? That's how it always was, the DT is already
> >>>> there, so why wouldn't you be able to read it.
> >>>
> >>> That was my thinking too. But we are finding in a few situations
> >>> that the child's ofdata depends on the parent's. For example, the
> >>> parent may have a base address, or a range mapping, or something
> >>> else that is needed for the child to correctly get its base address, etc.
> >>>
> >>>>
> >>>>> 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.
> >>>>
> >>>> I think this is how it worked before already.
> >>>
> >>> Well effectively, yes, because ofdata and probe were joined together.
> >
> >> Simon, do you have plan to fix this DM core issue ?
> >
> > I'm not sure it definitely should be changed. But I'll do a patch and
> > see how it looks.
> 
> Do I understand it correctly that the patch
> 82de42fa14682d408da935adfb0f935354c5008f actually completely breaks
> SoCFPGA ? Then I would say this is a release blocker ?
Yes. A10 SPL won't boot at all. It crashes during the clock manager setup.