Hi. On Wed, 1 Apr 2020 at 20:44, Ang, Chee Hong <[email protected]> wrote: > > > On 4/2/20 4:34 AM, Simon Glass wrote: > > > Hi, > > > > > > On Tue, 31 Mar 2020 at 20:33, Ang, Chee Hong <[email protected]> > > wrote: > > >> > > >>> Hi Marek, > > >>> > > >>> On Wed, 11 Mar 2020 at 05:55, Marek Vasut <[email protected]> wrote: > > >>>> > > >>>> On 3/11/20 12:50 PM, Simon Glass wrote: > > >>>>> Hi, > > >>>> > > >>>> Hi, > > >>>> > > >>>>> On Mon, 9 Mar 2020 at 02:22, <[email protected]> wrote: > > >>>>>> > > >>>>>> From: Chee Hong Ang <[email protected]> > > >>>>>> > > >>>>>> 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.
This came in right at the beginning of the cycle. I thought the purpose of the 3-month cycle was to allow time to test? I do plan to try out changing the behaviour to read a parent's ofdata before the child, but I am not comfortable adding such a major change just before a release. It could have any number of ill effects. Can you update the clock driver? E.g. you could move some of the code from socfpga_a10_ofdata_to_platdata() to a probe() method? Regards, Simon
