[U-Boot] [PATCH] db410c: set clk node to be probed before relocation

[Ramon Fried]

On 20 April 2018 at 21:38, Ramon Fried <ramon.fried at linaro.org> wrote:
> On 20 April 2018 at 17:45, Jorge Ramirez-Ortiz <jramirez at baylibre.com> wrote:
>> On 04/20/2018 03:46 PM, Ramon Fried wrote:
>>>
>>> On 20 April 2018 at 14:50, Jorge Ramirez-Ortiz <jramirez at baylibre.com>
>>> wrote:
>>>>
>>>> On 04/20/2018 01:22 PM, Ramon Fried wrote:
>>>>>
>>>>> On 20 April 2018 at 14:14, Jorge Ramirez-Ortiz <jramirez at baylibre.com>
>>>>> wrote:
>>>>>>
>>>>>> On 04/20/2018 01:02 PM, Ramon Fried wrote:
>>>>>>>
>>>>>>> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez at baylibre.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On 04/18/2018 09:02 AM, Jorge Ramirez-Ortiz wrote:
>>>>>>>>>
>>>>>>>>> On 04/17/2018 02:01 PM, Ramon Fried wrote:
>>>>>>>>>>
>>>>>>>>>> The clock node is used by the serial driver and it's needed
>>>>>>>>>> before relocation.
>>>>>>>>>> This patch ensures that the msm-serial driver can actually
>>>>>>>>>> use the clock node.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Ramon Fried <ramon.fried at linaro.org>
>>>>>>>>>> ---
>>>>>>>>>>      arch/arm/dts/dragonboard410c.dts | 2 ++
>>>>>>>>>>      1 file changed, 2 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/dts/dragonboard410c.dts
>>>>>>>>>> b/arch/arm/dts/dragonboard410c.dts
>>>>>>>>>> index 5ccfe7f8c8..f37ef5d523 100644
>>>>>>>>>> --- a/arch/arm/dts/dragonboard410c.dts
>>>>>>>>>> +++ b/arch/arm/dts/dragonboard410c.dts
>>>>>>>>>> @@ -38,12 +38,14 @@
>>>>>>>>>>              #size-cells = <0x1>;
>>>>>>>>>>              ranges = <0x0 0x0 0x0 0xffffffff>;
>>>>>>>>>>              compatible = "simple-bus";
>>>>>>>>>> +        u-boot,dm-pre-reloc;
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think the intent is to make dm-pre-reloc legacy.
>>>>>>>>> New platforms should be using "u-boot,dm-spl" even if
>>>>>>>>> CONFIG_SPL_BUILD
>>>>>>>>> is
>>>>>>>>> not enabled (I believe it should work)
>>>>>>>>>
>>>>>>>>>>                clkc: qcom,gcc at 1800000 {
>>>>>>>>>>                  compatible = "qcom,gcc-apq8016";
>>>>>>>>>>                  reg = <0x1800000 0x80000>;
>>>>>>>>>>                  #address-cells = <0x1>;
>>>>>>>>>>                  #size-cells = <0x0>;
>>>>>>>>>> +            u-boot,dm-pre-reloc;
>>>>>>>>>>              };
>>>>>>>>>>                serial at 78b0000 {
>>>>>>>>>
>>>>>>>>>
>>>>>>>> another question is, how will you probe the clock driver before the
>>>>>>>> uart?
>>>>>>>> I think even if you probed in misc_init_f it is already too late.
>>>>>>>>
>>>>>>>> other than that - + Peter Robinson's comments- looks good.
>>>>>>>>
>>>>>>> The clock is probed because the uart driver asks for it. it's actually
>>>>>>> already exists in the code, but wasn't used because
>>>>>>> the clock wasn't set to dm-pre-reloc
>>>>>>
>>>>>>
>>>>>> um, are you sure? that is not what I see during my tests but I could be
>>>>>> wrong - or something else might be happening in uboot
>>>>>>
>>>>>> you can create a misc_init_f for the board that retrieves the clock
>>>>>> driver
>>>>>> by name and forces a probe; I can see that the probe happens (ie, the
>>>>>> DM_FLAG_ACTIVATED was _not_ set for that driver) which is not what I
>>>>>> believe
>>>>>> you expect since you want the clock driver to be probed before the
>>>>>> uart.
>>>>>
>>>>> It doesn't need to be probed before the uart, the uart is probed first
>>>>> and when it asks for the
>>>>> clock node it's forcing a probe. it's tested.
>>>>
>>>>
>>>> can you post a trace. just dump the value of GD_FLG_RELOC on clock the
>>>> probe
>>>> function.
>>>
>>> The serial isn't  initialized at this time, I'll save the value and
>>> print afterwards :)
>>
>>
>> of course.
>>  I think it makes sense to add some debug statement near DM_FLAG_ACTIVATED
>> in device_probe...the driver should not be probed a second time if it was
>> successfully probed early before relocation
> Actually, I already verified that. the driver probes twice, once
> before and once after relocation.
> In case of serial, it's not a big deal to re-initialize the hardware,
> but I agree it's worthless.
>>
Ok. just checked, it's first probed before relocation.
>>
>>