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

[Ramon Fried]

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.
>
>
>