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

[Marek Vasut]

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 ?