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

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

[Ramon Fried]

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@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;
 
 		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 {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Jorge Ramirez-Ortiz]

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@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 {

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

[Peter Robinson]

On Tue, Apr 17, 2018 at 1:01 PM, Ramon Fried <ramon.fried@linaro.org> 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@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;

u-boot specific DT bits should be put in a u-boot.dts[i] file, see in
arch/arm/dts/ for other examples, it makes syncing DT changes from the
linux kernel more straight forward.

>                 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 {
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

[Jorge Ramirez-Ortiz]

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

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

[Ramon Fried]

On 18 April 2018 at 11:15, Peter Robinson <pbrobinson@gmail.com> wrote:
> On Tue, Apr 17, 2018 at 1:01 PM, Ramon Fried <ramon.fried@linaro.org> 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@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;
>
> u-boot specific DT bits should be put in a u-boot.dts[i] file, see in
> arch/arm/dts/ for other examples, it makes syncing DT changes from the
> linux kernel more straight forward.
Thanks for the feedback.
I'll resend the the patch with the correction.
>
>>                 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 {
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot

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

[Ramon Fried]

On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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

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

[Jorge Ramirez-Ortiz]

On 04/20/2018 01:02 PM, Ramon Fried wrote:
> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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.

Good news is that since misc_init_f runs before relocation you can check 
that GD_FLG_RELOC was not set either so it is indeed being probed before 
relocation (which makes sense after adding the reloc property).

maybe you can have a look?

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

[Ramon Fried]

On 20 April 2018 at 14:14, Jorge Ramirez-Ortiz <jramirez@baylibre.com> wrote:
> On 04/20/2018 01:02 PM, Ramon Fried wrote:
>>
>> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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.
>
> Good news is that since misc_init_f runs before relocation you can check
> that GD_FLG_RELOC was not set either so it is indeed being probed before
> relocation (which makes sense after adding the reloc property).
>
> maybe you can have a look?
>
>

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

[Jorge Ramirez-Ortiz]

On 04/20/2018 01:22 PM, Ramon Fried wrote:
> On 20 April 2018 at 14:14, Jorge Ramirez-Ortiz <jramirez@baylibre.com> wrote:
>> On 04/20/2018 01:02 PM, Ramon Fried wrote:
>>> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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.

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

[Ramon Fried]

On 20 April 2018 at 14:50, Jorge Ramirez-Ortiz <jramirez@baylibre.com> wrote:
> On 04/20/2018 01:22 PM, Ramon Fried wrote:
>>
>> On 20 April 2018 at 14:14, Jorge Ramirez-Ortiz <jramirez@baylibre.com>
>> wrote:
>>>
>>> On 04/20/2018 01:02 PM, Ramon Fried wrote:
>>>>
>>>> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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 :)
>

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

[Jorge Ramirez-Ortiz]

On 04/20/2018 03:46 PM, Ramon Fried wrote:
> On 20 April 2018 at 14:50, Jorge Ramirez-Ortiz <jramirez@baylibre.com> wrote:
>> On 04/20/2018 01:22 PM, Ramon Fried wrote:
>>> On 20 April 2018 at 14:14, Jorge Ramirez-Ortiz <jramirez@baylibre.com>
>>> wrote:
>>>> On 04/20/2018 01:02 PM, Ramon Fried wrote:
>>>>> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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

[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@baylibre.com> wrote:
> On 04/20/2018 03:46 PM, Ramon Fried wrote:
>>
>> On 20 April 2018 at 14:50, Jorge Ramirez-Ortiz <jramirez@baylibre.com>
>> wrote:
>>>
>>> On 04/20/2018 01:22 PM, Ramon Fried wrote:
>>>>
>>>> On 20 April 2018 at 14:14, Jorge Ramirez-Ortiz <jramirez@baylibre.com>
>>>> wrote:
>>>>>
>>>>> On 04/20/2018 01:02 PM, Ramon Fried wrote:
>>>>>>
>>>>>> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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.
>
>
>

[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@linaro.org> wrote:
> On 20 April 2018 at 17:45, Jorge Ramirez-Ortiz <jramirez@baylibre.com> wrote:
>> On 04/20/2018 03:46 PM, Ramon Fried wrote:
>>>
>>> On 20 April 2018 at 14:50, Jorge Ramirez-Ortiz <jramirez@baylibre.com>
>>> wrote:
>>>>
>>>> On 04/20/2018 01:22 PM, Ramon Fried wrote:
>>>>>
>>>>> On 20 April 2018 at 14:14, Jorge Ramirez-Ortiz <jramirez@baylibre.com>
>>>>> wrote:
>>>>>>
>>>>>> On 04/20/2018 01:02 PM, Ramon Fried wrote:
>>>>>>>
>>>>>>> On 18 April 2018 at 13:15, Jorge Ramirez-Ortiz <jramirez@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@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.
>>
>>

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

[Jorge Ramirez-Ortiz]

On 04/20/2018 09:07 PM, Ramon Fried wrote:
>> 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.

ah of course, the db820 - what I was using to validate- doesnt probe 
early despite the change because the uart node doesnt use the clock.
cool, looks good to me.