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

[Marek Vasut]

On 4/2/20 10:54 PM, Tom Rini wrote:
[...]

>>>>>> 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?
>>
>> It was ... altera ?
> 
> Sorry, I'm missing how that's an answer to the question.  This came in
> basically right at the start of the merge window.

I don't have an A10 available right now, so what can I do ?

>>> 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?
>>
>> Can we revert the patch which broke arria10 instead ? It did work
>> before, so who knows how many other ill side effects there are ...
> 
> I'm not in favor of reverting 82de42fa1468 unless the answer is "that
> commit was wrong" and not "that commit caused other problems to surface,
> that we can fix".  I am quite willing to say "we need to delay the
> release to test changes for problems that've surfaced and verify more
> hardware is fine".

If I understand this correctly, the clock code did scan the DT and bound
all the clocks, so that drivers can use them. That's how bind function
should work, it creates instances of devices, but without probing them.
This doesn't work anymore if I understand it correctly, but that means
the basic premise of DT is broken ? And the solution is to do more magic
in probe function , which I don't think even works here ?