| Message ID | 20180417120134.23904-1-ramon.fried@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | db410c: set clk node to be probed before relocation | expand |
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@1800000 { > compatible = "qcom,gcc-apq8016"; > reg = <0x1800000 0x80000>; > #address-cells = <0x1>; > #size-cells = <0x0>; > + u-boot,dm-pre-reloc; > }; > > serial@78b0000 {
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@1800000 { > compatible = "qcom,gcc-apq8016"; > reg = <0x1800000 0x80000>; > #address-cells = <0x1>; > #size-cells = <0x0>; > + u-boot,dm-pre-reloc; > }; > > serial@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@lists.denx.de > https://lists.denx.de/listinfo/u-boot
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@1800000 { >> compatible = "qcom,gcc-apq8016"; >> reg = <0x1800000 0x80000>; >> #address-cells = <0x1>; >> #size-cells = <0x0>; >> + u-boot,dm-pre-reloc; >> }; >> serial@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.
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@1800000 { >> compatible = "qcom,gcc-apq8016"; >> reg = <0x1800000 0x80000>; >> #address-cells = <0x1>; >> #size-cells = <0x0>; >> + u-boot,dm-pre-reloc; >> }; >> >> serial@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@lists.denx.de >> https://lists.denx.de/listinfo/u-boot
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@1800000 { >>> compatible = "qcom,gcc-apq8016"; >>> reg = <0x1800000 0x80000>; >>> #address-cells = <0x1>; >>> #size-cells = <0x0>; >>> + u-boot,dm-pre-reloc; >>> }; >>> serial@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
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@1800000 { >>>> compatible = "qcom,gcc-apq8016"; >>>> reg = <0x1800000 0x80000>; >>>> #address-cells = <0x1>; >>>> #size-cells = <0x0>; >>>> + u-boot,dm-pre-reloc; >>>> }; >>>> serial@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?
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@1800000 { >>>>> compatible = "qcom,gcc-apq8016"; >>>>> reg = <0x1800000 0x80000>; >>>>> #address-cells = <0x1>; >>>>> #size-cells = <0x0>; >>>>> + u-boot,dm-pre-reloc; >>>>> }; >>>>> serial@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? > >
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@1800000 { >>>>>> compatible = "qcom,gcc-apq8016"; >>>>>> reg = <0x1800000 0x80000>; >>>>>> #address-cells = <0x1>; >>>>>> #size-cells = <0x0>; >>>>>> + u-boot,dm-pre-reloc; >>>>>> }; >>>>>> serial@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.
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@1800000 { >>>>>>> compatible = "qcom,gcc-apq8016"; >>>>>>> reg = <0x1800000 0x80000>; >>>>>>> #address-cells = <0x1>; >>>>>>> #size-cells = <0x0>; >>>>>>> + u-boot,dm-pre-reloc; >>>>>>> }; >>>>>>> serial@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 :) >
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@1800000 { >>>>>>>> compatible = "qcom,gcc-apq8016"; >>>>>>>> reg = <0x1800000 0x80000>; >>>>>>>> #address-cells = <0x1>; >>>>>>>> #size-cells = <0x0>; >>>>>>>> + u-boot,dm-pre-reloc; >>>>>>>> }; >>>>>>>> serial@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
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@1800000 { >>>>>>>>> compatible = "qcom,gcc-apq8016"; >>>>>>>>> reg = <0x1800000 0x80000>; >>>>>>>>> #address-cells = <0x1>; >>>>>>>>> #size-cells = <0x0>; >>>>>>>>> + u-boot,dm-pre-reloc; >>>>>>>>> }; >>>>>>>>> serial@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. > > >
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@1800000 { >>>>>>>>>> compatible = "qcom,gcc-apq8016"; >>>>>>>>>> reg = <0x1800000 0x80000>; >>>>>>>>>> #address-cells = <0x1>; >>>>>>>>>> #size-cells = <0x0>; >>>>>>>>>> + u-boot,dm-pre-reloc; >>>>>>>>>> }; >>>>>>>>>> serial@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. >> >>
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.
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@1800000 { compatible = "qcom,gcc-apq8016"; reg = <0x1800000 0x80000>; #address-cells = <0x1>; #size-cells = <0x0>; + u-boot,dm-pre-reloc; }; serial@78b0000 {
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(+)