[U-Boot,v2,4/7] mach-snapdragon: Fix UART clock flow

Message ID 20180516091342.7509-4-ramon.fried@gmail.com
State Accepted
Delegated to: Tom Rini
Headers show
Series
  • [U-Boot,v2,1/7] db820c: set clk node to be probed before relocation
Related show

Commit Message

Ramon Fried May 16, 2018, 9:13 a.m.
UART clock enabling flow was wrong.
Changed the flow according to downstream implementation in LK.

Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---
 arch/arm/mach-snapdragon/clock-apq8016.c      | 23 +++++++++++++------
 arch/arm/mach-snapdragon/clock-apq8096.c      |  4 ++--
 arch/arm/mach-snapdragon/clock-snapdragon.c   | 17 +++++++++++++-
 arch/arm/mach-snapdragon/clock-snapdragon.h   |  9 ++++++--
 .../include/mach/sysmap-apq8016.h             |  1 +
 5 files changed, 42 insertions(+), 12 deletions(-)

Comments

Tom Rini May 28, 2018, 7:12 p.m. | #1
On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:

> UART clock enabling flow was wrong.
> Changed the flow according to downstream implementation in LK.
> 
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>

Applied to u-boot/master, thanks!
Jorge Ramirez-Ortiz May 28, 2018, 7:19 p.m. | #2
On 05/28/2018 09:12 PM, Tom Rini wrote:
> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>
>> UART clock enabling flow was wrong.
>> Changed the flow according to downstream implementation in LK.
>>
>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> Applied to u-boot/master, thanks!
>

Ramon, did you re-test this one on the 820 as we discussed?
Sorry Tom, when I tested this on Friday it broke my 820 (I should have 
reported it to the ML).

I think it introduces a regression but I'll let Ramon to confirm.
Ramon Fried May 28, 2018, 7:24 p.m. | #3
On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
<jramirez@baylibre.com> wrote:
> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>
>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>
>>> UART clock enabling flow was wrong.
>>> Changed the flow according to downstream implementation in LK.
>>>
>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>
>> Applied to u-boot/master, thanks!
>>
>
> Ramon, did you re-test this one on the 820 as we discussed?
> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
> reported it to the ML).
>
> I think it introduces a regression but I'll let Ramon to confirm.
Hi.
It's funny, I'm debugging it now. don't have any conclusions yet but I
was under the assumption that it won't get merged as it was
missing Reviewed-by.
Let me get back to you on these one in couple of hours.
Thanks,
Ramon.
>
Tom Rini May 28, 2018, 7:26 p.m. | #4
On Mon, May 28, 2018 at 10:24:36PM +0300, Ramon Fried wrote:
> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
> <jramirez@baylibre.com> wrote:
> > On 05/28/2018 09:12 PM, Tom Rini wrote:
> >>
> >> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
> >>
> >>> UART clock enabling flow was wrong.
> >>> Changed the flow according to downstream implementation in LK.
> >>>
> >>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> >>
> >> Applied to u-boot/master, thanks!
> >>
> >
> > Ramon, did you re-test this one on the 820 as we discussed?
> > Sorry Tom, when I tested this on Friday it broke my 820 (I should have
> > reported it to the ML).
> >
> > I think it introduces a regression but I'll let Ramon to confirm.
> Hi.
> It's funny, I'm debugging it now. don't have any conclusions yet but I
> was under the assumption that it won't get merged as it was
> missing Reviewed-by.
> Let me get back to you on these one in couple of hours.

Yeah, sorry guys, I didn't see anything in public about problems so I
fixed up the one new warning.  Please let me know if you need me to
revert these or if it's an "easy" fix, thanks!
Ramon Fried May 28, 2018, 7:28 p.m. | #5
On Mon, May 28, 2018 at 10:26 PM, Tom Rini <trini@konsulko.com> wrote:
> On Mon, May 28, 2018 at 10:24:36PM +0300, Ramon Fried wrote:
>> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>> <jramirez@baylibre.com> wrote:
>> > On 05/28/2018 09:12 PM, Tom Rini wrote:
>> >>
>> >> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>> >>
>> >>> UART clock enabling flow was wrong.
>> >>> Changed the flow according to downstream implementation in LK.
>> >>>
>> >>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>> >>
>> >> Applied to u-boot/master, thanks!
>> >>
>> >
>> > Ramon, did you re-test this one on the 820 as we discussed?
>> > Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>> > reported it to the ML).
>> >
>> > I think it introduces a regression but I'll let Ramon to confirm.
>> Hi.
>> It's funny, I'm debugging it now. don't have any conclusions yet but I
>> was under the assumption that it won't get merged as it was
>> missing Reviewed-by.
>> Let me get back to you on these one in couple of hours.
>
> Yeah, sorry guys, I didn't see anything in public about problems so I
> fixed up the one new warning.  Please let me know if you need me to
> revert these or if it's an "easy" fix, thanks!
Sure.
I'll keep you posted. but without any doubt, the following patch is
necessary to make it work:
https://patchwork.ozlabs.org/patch/921407/
Can you please merge it as well ?
>
> --
> Tom
Tom Rini May 28, 2018, 7:35 p.m. | #6
On Mon, May 28, 2018 at 10:28:51PM +0300, Ramon Fried wrote:
> On Mon, May 28, 2018 at 10:26 PM, Tom Rini <trini@konsulko.com> wrote:
> > On Mon, May 28, 2018 at 10:24:36PM +0300, Ramon Fried wrote:
> >> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
> >> <jramirez@baylibre.com> wrote:
> >> > On 05/28/2018 09:12 PM, Tom Rini wrote:
> >> >>
> >> >> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
> >> >>
> >> >>> UART clock enabling flow was wrong.
> >> >>> Changed the flow according to downstream implementation in LK.
> >> >>>
> >> >>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> >> >>
> >> >> Applied to u-boot/master, thanks!
> >> >>
> >> >
> >> > Ramon, did you re-test this one on the 820 as we discussed?
> >> > Sorry Tom, when I tested this on Friday it broke my 820 (I should have
> >> > reported it to the ML).
> >> >
> >> > I think it introduces a regression but I'll let Ramon to confirm.
> >> Hi.
> >> It's funny, I'm debugging it now. don't have any conclusions yet but I
> >> was under the assumption that it won't get merged as it was
> >> missing Reviewed-by.
> >> Let me get back to you on these one in couple of hours.
> >
> > Yeah, sorry guys, I didn't see anything in public about problems so I
> > fixed up the one new warning.  Please let me know if you need me to
> > revert these or if it's an "easy" fix, thanks!
> Sure.
> I'll keep you posted. but without any doubt, the following patch is
> necessary to make it work:
> https://patchwork.ozlabs.org/patch/921407/
> Can you please merge it as well ?

0x0800 is still pretty small.  We're talking about (and oops, I think I
need to for v2018.07..) bump it to 0x2000 for all TI platforms.  Can you
v2 doing that size if it works for you (it ought...) and as a default
VAL if ARCH_SNAPDRAGON ?  Thanks!
Ramon Fried May 28, 2018, 7:37 p.m. | #7
On Mon, May 28, 2018 at 10:35 PM, Tom Rini <trini@konsulko.com> wrote:
> On Mon, May 28, 2018 at 10:28:51PM +0300, Ramon Fried wrote:
>> On Mon, May 28, 2018 at 10:26 PM, Tom Rini <trini@konsulko.com> wrote:
>> > On Mon, May 28, 2018 at 10:24:36PM +0300, Ramon Fried wrote:
>> >> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>> >> <jramirez@baylibre.com> wrote:
>> >> > On 05/28/2018 09:12 PM, Tom Rini wrote:
>> >> >>
>> >> >> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>> >> >>
>> >> >>> UART clock enabling flow was wrong.
>> >> >>> Changed the flow according to downstream implementation in LK.
>> >> >>>
>> >> >>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>> >> >>
>> >> >> Applied to u-boot/master, thanks!
>> >> >>
>> >> >
>> >> > Ramon, did you re-test this one on the 820 as we discussed?
>> >> > Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>> >> > reported it to the ML).
>> >> >
>> >> > I think it introduces a regression but I'll let Ramon to confirm.
>> >> Hi.
>> >> It's funny, I'm debugging it now. don't have any conclusions yet but I
>> >> was under the assumption that it won't get merged as it was
>> >> missing Reviewed-by.
>> >> Let me get back to you on these one in couple of hours.
>> >
>> > Yeah, sorry guys, I didn't see anything in public about problems so I
>> > fixed up the one new warning.  Please let me know if you need me to
>> > revert these or if it's an "easy" fix, thanks!
>> Sure.
>> I'll keep you posted. but without any doubt, the following patch is
>> necessary to make it work:
>> https://patchwork.ozlabs.org/patch/921407/
>> Can you please merge it as well ?
>
> 0x0800 is still pretty small.  We're talking about (and oops, I think I
> need to for v2018.07..) bump it to 0x2000 for all TI platforms.  Can you
> v2 doing that size if it works for you (it ought...) and as a default
> VAL if ARCH_SNAPDRAGON ?  Thanks!
Sure. I'll test and resend.
>
> --
> Tom
Ramon Fried May 28, 2018, 7:48 p.m. | #8
On Mon, May 28, 2018 at 10:24 PM, Ramon Fried <ramon.fried@gmail.com> wrote:
> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
> <jramirez@baylibre.com> wrote:
>> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>>
>>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>>
>>>> UART clock enabling flow was wrong.
>>>> Changed the flow according to downstream implementation in LK.
>>>>
>>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>>
>>> Applied to u-boot/master, thanks!
>>>
>>
>> Ramon, did you re-test this one on the 820 as we discussed?
>> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>> reported it to the ML).
>>
>> I think it introduces a regression but I'll let Ramon to confirm.
> Hi.
> It's funny, I'm debugging it now. don't have any conclusions yet but I
> was under the assumption that it won't get merged as it was
> missing Reviewed-by.
> Let me get back to you on these one in couple of hours.
> Thanks,
> Ramon.
>>
I just toasted my 820 board...:(
I can only get a replacement by Thursday.
I can't find any explanation why the 820 should be affected, as the
clock configuration for it is empty.
and it's pre-initalized by the uart.
Jorge, you previously tested from my branch, care to test from master
to see if it's working ?
Thanks.
Ramon.
Jorge Ramirez-Ortiz May 28, 2018, 7:59 p.m. | #9
On 05/28/2018 09:48 PM, Ramon Fried wrote:
> On Mon, May 28, 2018 at 10:24 PM, Ramon Fried <ramon.fried@gmail.com> wrote:
>> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>> <jramirez@baylibre.com> wrote:
>>> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>>>
>>>>> UART clock enabling flow was wrong.
>>>>> Changed the flow according to downstream implementation in LK.
>>>>>
>>>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>>> Applied to u-boot/master, thanks!
>>>>
>>> Ramon, did you re-test this one on the 820 as we discussed?
>>> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>>> reported it to the ML).
>>>
>>> I think it introduces a regression but I'll let Ramon to confirm.
>> Hi.
>> It's funny, I'm debugging it now. don't have any conclusions yet but I
>> was under the assumption that it won't get merged as it was
>> missing Reviewed-by.
>> Let me get back to you on these one in couple of hours.
>> Thanks,
>> Ramon.
> I just toasted my 820 board...:(
> I can only get a replacement by Thursday.
> I can't find any explanation why the 820 should be affected, as the
> clock configuration for it is empty.
> and it's pre-initalized by the uart.
> Jorge, you previously tested from my branch, care to test from master
> to see if it's working ?

so just pull your master branch again and retest?


> Thanks.
> Ramon.
Ramon Fried May 28, 2018, 8:01 p.m. | #10
On Mon, May 28, 2018 at 10:59 PM, Jorge Ramirez-Ortiz
<jramirez@baylibre.com> wrote:
> On 05/28/2018 09:48 PM, Ramon Fried wrote:
>>
>> On Mon, May 28, 2018 at 10:24 PM, Ramon Fried <ramon.fried@gmail.com>
>> wrote:
>>>
>>> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>>> <jramirez@baylibre.com> wrote:
>>>>
>>>> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>>>>
>>>>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>>>>
>>>>>> UART clock enabling flow was wrong.
>>>>>> Changed the flow according to downstream implementation in LK.
>>>>>>
>>>>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>>>>
>>>>> Applied to u-boot/master, thanks!
>>>>>
>>>> Ramon, did you re-test this one on the 820 as we discussed?
>>>> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>>>> reported it to the ML).
>>>>
>>>> I think it introduces a regression but I'll let Ramon to confirm.
>>>
>>> Hi.
>>> It's funny, I'm debugging it now. don't have any conclusions yet but I
>>> was under the assumption that it won't get merged as it was
>>> missing Reviewed-by.
>>> Let me get back to you on these one in couple of hours.
>>> Thanks,
>>> Ramon.
>>
>> I just toasted my 820 board...:(
>> I can only get a replacement by Thursday.
>> I can't find any explanation why the 820 should be affected, as the
>> clock configuration for it is empty.
>> and it's pre-initalized by the uart.
>> Jorge, you previously tested from my branch, care to test from master
>> to see if it's working ?
>
>
> so just pull your master branch again and retest?
not mine. upstream master.
>
>
>> Thanks.
>> Ramon.
>
>
Jorge Ramirez-Ortiz May 28, 2018, 8:07 p.m. | #11
On 05/28/2018 10:01 PM, Ramon Fried wrote:
> On Mon, May 28, 2018 at 10:59 PM, Jorge Ramirez-Ortiz
> <jramirez@baylibre.com> wrote:
>> On 05/28/2018 09:48 PM, Ramon Fried wrote:
>>> On Mon, May 28, 2018 at 10:24 PM, Ramon Fried <ramon.fried@gmail.com>
>>> wrote:
>>>> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>>>> <jramirez@baylibre.com> wrote:
>>>>> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>>>>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>>>>>
>>>>>>> UART clock enabling flow was wrong.
>>>>>>> Changed the flow according to downstream implementation in LK.
>>>>>>>
>>>>>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>>>>> Applied to u-boot/master, thanks!
>>>>>>
>>>>> Ramon, did you re-test this one on the 820 as we discussed?
>>>>> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>>>>> reported it to the ML).
>>>>>
>>>>> I think it introduces a regression but I'll let Ramon to confirm.
>>>> Hi.
>>>> It's funny, I'm debugging it now. don't have any conclusions yet but I
>>>> was under the assumption that it won't get merged as it was
>>>> missing Reviewed-by.
>>>> Let me get back to you on these one in couple of hours.
>>>> Thanks,
>>>> Ramon.
>>> I just toasted my 820 board...:(
>>> I can only get a replacement by Thursday.
>>> I can't find any explanation why the 820 should be affected, as the
>>> clock configuration for it is empty.
>>> and it's pre-initalized by the uart.
>>> Jorge, you previously tested from my branch, care to test from master
>>> to see if it's working ?
>>
>> so just pull your master branch again and retest?
> not mine. upstream master.

2 days ago ffada23 db820c: set clk node to be probed before 
relocation          Ramon Fr..[Tom Rini]
2 days ago 0ed34aa Remove CONFIG_MVGBE from 
config_whitelist.txt                Chris Pa..[Tom Rini]
2 days ago 5ce9aca PCI: Document 
pciauto_region_allocate()                      Tuomas T..[Tom Rini]
2 days ago d71975a PCI: autoconfig: Don't allocate 64-bit addresses to 
32-bit.. Tuomas T..[Tom Rini]
2 days ago ed12a89 PCI: Add newlines to debug prints in 
pci_auto_common.c       Tuomas T..[Tom Rini]

the one on top is the last commit that works on the 820 from the current 
uboot master branch.

>>
>>> Thanks.
>>> Ramon.
>>
Ramon Fried May 28, 2018, 8:14 p.m. | #12
On Mon, May 28, 2018 at 11:07 PM, Jorge Ramirez-Ortiz
<jramirez@baylibre.com> wrote:
> On 05/28/2018 10:01 PM, Ramon Fried wrote:
>>
>> On Mon, May 28, 2018 at 10:59 PM, Jorge Ramirez-Ortiz
>> <jramirez@baylibre.com> wrote:
>>>
>>> On 05/28/2018 09:48 PM, Ramon Fried wrote:
>>>>
>>>> On Mon, May 28, 2018 at 10:24 PM, Ramon Fried <ramon.fried@gmail.com>
>>>> wrote:
>>>>>
>>>>> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>>>>> <jramirez@baylibre.com> wrote:
>>>>>>
>>>>>> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>>>>>>
>>>>>>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>>>>>>
>>>>>>>> UART clock enabling flow was wrong.
>>>>>>>> Changed the flow according to downstream implementation in LK.
>>>>>>>>
>>>>>>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>>>>>>
>>>>>>> Applied to u-boot/master, thanks!
>>>>>>>
>>>>>> Ramon, did you re-test this one on the 820 as we discussed?
>>>>>> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>>>>>> reported it to the ML).
>>>>>>
>>>>>> I think it introduces a regression but I'll let Ramon to confirm.
>>>>>
>>>>> Hi.
>>>>> It's funny, I'm debugging it now. don't have any conclusions yet but I
>>>>> was under the assumption that it won't get merged as it was
>>>>> missing Reviewed-by.
>>>>> Let me get back to you on these one in couple of hours.
>>>>> Thanks,
>>>>> Ramon.
>>>>
>>>> I just toasted my 820 board...:(
>>>> I can only get a replacement by Thursday.
>>>> I can't find any explanation why the 820 should be affected, as the
>>>> clock configuration for it is empty.
>>>> and it's pre-initalized by the uart.
>>>> Jorge, you previously tested from my branch, care to test from master
>>>> to see if it's working ?
>>>
>>>
>>> so just pull your master branch again and retest?
>>
>> not mine. upstream master.
>
>
> 2 days ago ffada23 db820c: set clk node to be probed before relocation
> Ramon Fr..[Tom Rini]
> 2 days ago 0ed34aa Remove CONFIG_MVGBE from config_whitelist.txt
> Chris Pa..[Tom Rini]
> 2 days ago 5ce9aca PCI: Document pciauto_region_allocate()
> Tuomas T..[Tom Rini]
> 2 days ago d71975a PCI: autoconfig: Don't allocate 64-bit addresses to
> 32-bit.. Tuomas T..[Tom Rini]
> 2 days ago ed12a89 PCI: Add newlines to debug prints in pci_auto_common.c
> Tuomas T..[Tom Rini]
>
> the one on top is the last commit that works on the 820 from the current
> uboot master branch.
Thanks Jorge.
I see where the problem is. there's no clock defintions in 820 device tree,
so msm_uart_clk_init() fails. I'll add dummy definitions meanwhile and
after I'll finish
with 410, I'll work on the correct clock values for 820. nevertheless
the UART will work as
LK already initalizes it with the right values.
expect a fix patch shortly.

Thanks,
Ramon.
>
>>>
>>>> Thanks.
>>>> Ramon.
>>>
>>>
>
Ramon Fried May 28, 2018, 8:25 p.m. | #13
On Mon, May 28, 2018 at 11:14 PM, Ramon Fried <ramon.fried@gmail.com> wrote:
> On Mon, May 28, 2018 at 11:07 PM, Jorge Ramirez-Ortiz
> <jramirez@baylibre.com> wrote:
>> On 05/28/2018 10:01 PM, Ramon Fried wrote:
>>>
>>> On Mon, May 28, 2018 at 10:59 PM, Jorge Ramirez-Ortiz
>>> <jramirez@baylibre.com> wrote:
>>>>
>>>> On 05/28/2018 09:48 PM, Ramon Fried wrote:
>>>>>
>>>>> On Mon, May 28, 2018 at 10:24 PM, Ramon Fried <ramon.fried@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>>>>>> <jramirez@baylibre.com> wrote:
>>>>>>>
>>>>>>> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>>>>>>>
>>>>>>>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>>>>>>>
>>>>>>>>> UART clock enabling flow was wrong.
>>>>>>>>> Changed the flow according to downstream implementation in LK.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>>>>>>>
>>>>>>>> Applied to u-boot/master, thanks!
>>>>>>>>
>>>>>>> Ramon, did you re-test this one on the 820 as we discussed?
>>>>>>> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>>>>>>> reported it to the ML).
>>>>>>>
>>>>>>> I think it introduces a regression but I'll let Ramon to confirm.
>>>>>>
>>>>>> Hi.
>>>>>> It's funny, I'm debugging it now. don't have any conclusions yet but I
>>>>>> was under the assumption that it won't get merged as it was
>>>>>> missing Reviewed-by.
>>>>>> Let me get back to you on these one in couple of hours.
>>>>>> Thanks,
>>>>>> Ramon.
>>>>>
>>>>> I just toasted my 820 board...:(
>>>>> I can only get a replacement by Thursday.
>>>>> I can't find any explanation why the 820 should be affected, as the
>>>>> clock configuration for it is empty.
>>>>> and it's pre-initalized by the uart.
>>>>> Jorge, you previously tested from my branch, care to test from master
>>>>> to see if it's working ?
>>>>
>>>>
>>>> so just pull your master branch again and retest?
>>>
>>> not mine. upstream master.
>>
>>
>> 2 days ago ffada23 db820c: set clk node to be probed before relocation
>> Ramon Fr..[Tom Rini]
>> 2 days ago 0ed34aa Remove CONFIG_MVGBE from config_whitelist.txt
>> Chris Pa..[Tom Rini]
>> 2 days ago 5ce9aca PCI: Document pciauto_region_allocate()
>> Tuomas T..[Tom Rini]
>> 2 days ago d71975a PCI: autoconfig: Don't allocate 64-bit addresses to
>> 32-bit.. Tuomas T..[Tom Rini]
>> 2 days ago ed12a89 PCI: Add newlines to debug prints in pci_auto_common.c
>> Tuomas T..[Tom Rini]
>>
>> the one on top is the last commit that works on the 820 from the current
>> uboot master branch.
> Thanks Jorge.
> I see where the problem is. there's no clock defintions in 820 device tree,
> so msm_uart_clk_init() fails. I'll add dummy definitions meanwhile and
> after I'll finish
> with 410, I'll work on the correct clock values for 820. nevertheless
> the UART will work as
> LK already initalizes it with the right values.
> expect a fix patch shortly.
>
> Thanks,
> Ramon.
Jorge, I just sent you a fix. can you test it and if it works I'll
push it upstream.
Thanks ! and sorry for the trouble...
>>
>>>>
>>>>> Thanks.
>>>>> Ramon.
>>>>
>>>>
>>
Jorge Ramirez-Ortiz May 28, 2018, 9:07 p.m. | #14
On 05/28/2018 10:25 PM, Ramon Fried wrote:
> On Mon, May 28, 2018 at 11:14 PM, Ramon Fried <ramon.fried@gmail.com> wrote:
>> On Mon, May 28, 2018 at 11:07 PM, Jorge Ramirez-Ortiz
>> <jramirez@baylibre.com> wrote:
>>> On 05/28/2018 10:01 PM, Ramon Fried wrote:
>>>> On Mon, May 28, 2018 at 10:59 PM, Jorge Ramirez-Ortiz
>>>> <jramirez@baylibre.com> wrote:
>>>>> On 05/28/2018 09:48 PM, Ramon Fried wrote:
>>>>>> On Mon, May 28, 2018 at 10:24 PM, Ramon Fried <ramon.fried@gmail.com>
>>>>>> wrote:
>>>>>>> On Mon, May 28, 2018 at 10:19 PM, Jorge Ramirez-Ortiz
>>>>>>> <jramirez@baylibre.com> wrote:
>>>>>>>> On 05/28/2018 09:12 PM, Tom Rini wrote:
>>>>>>>>> On Wed, May 16, 2018 at 12:13:39PM +0300, Ramon Fried wrote:
>>>>>>>>>
>>>>>>>>>> UART clock enabling flow was wrong.
>>>>>>>>>> Changed the flow according to downstream implementation in LK.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>>>>>>>>> Applied to u-boot/master, thanks!
>>>>>>>>>
>>>>>>>> Ramon, did you re-test this one on the 820 as we discussed?
>>>>>>>> Sorry Tom, when I tested this on Friday it broke my 820 (I should have
>>>>>>>> reported it to the ML).
>>>>>>>>
>>>>>>>> I think it introduces a regression but I'll let Ramon to confirm.
>>>>>>> Hi.
>>>>>>> It's funny, I'm debugging it now. don't have any conclusions yet but I
>>>>>>> was under the assumption that it won't get merged as it was
>>>>>>> missing Reviewed-by.
>>>>>>> Let me get back to you on these one in couple of hours.
>>>>>>> Thanks,
>>>>>>> Ramon.
>>>>>> I just toasted my 820 board...:(
>>>>>> I can only get a replacement by Thursday.
>>>>>> I can't find any explanation why the 820 should be affected, as the
>>>>>> clock configuration for it is empty.
>>>>>> and it's pre-initalized by the uart.
>>>>>> Jorge, you previously tested from my branch, care to test from master
>>>>>> to see if it's working ?
>>>>>
>>>>> so just pull your master branch again and retest?
>>>> not mine. upstream master.
>>>
>>> 2 days ago ffada23 db820c: set clk node to be probed before relocation
>>> Ramon Fr..[Tom Rini]
>>> 2 days ago 0ed34aa Remove CONFIG_MVGBE from config_whitelist.txt
>>> Chris Pa..[Tom Rini]
>>> 2 days ago 5ce9aca PCI: Document pciauto_region_allocate()
>>> Tuomas T..[Tom Rini]
>>> 2 days ago d71975a PCI: autoconfig: Don't allocate 64-bit addresses to
>>> 32-bit.. Tuomas T..[Tom Rini]
>>> 2 days ago ed12a89 PCI: Add newlines to debug prints in pci_auto_common.c
>>> Tuomas T..[Tom Rini]
>>>
>>> the one on top is the last commit that works on the 820 from the current
>>> uboot master branch.
>> Thanks Jorge.
>> I see where the problem is. there's no clock defintions in 820 device tree,
>> so msm_uart_clk_init() fails. I'll add dummy definitions meanwhile and
>> after I'll finish
>> with 410, I'll work on the correct clock values for 820. nevertheless
>> the UART will work as
>> LK already initalizes it with the right values.
>> expect a fix patch shortly.
>>
>> Thanks,
>> Ramon.
> Jorge, I just sent you a fix. can you test it and if it works I'll
> push it upstream.

yes I can see the console now so that fix is good.

however there must be some other regression lurking because the system 
wont boot a kernel from the SD card
Jorge Ramirez-Ortiz May 28, 2018, 9:11 p.m. | #15
On 05/28/2018 11:07 PM, Jorge Ramirez-Ortiz wrote:
>> Jorge, I just sent you a fix. can you test it and if it works I'll
>> push it upstream.
>
> yes I can see the console now so that fix is good.
>
> however there must be some other regression lurking because the system 
> wont boot a kernel from the SD card 

sorry forgot the trace.
I think you might have fixed this one as well Ramon (I remember you 
mentioning it in some other thread?)

U-Boot 2018.05-00430-g5ce9aca (May 28 2018 - 23:09:02) [15/592]
Qualcomm-DragonBoard 820C

DRAM:  3 GiB
PSCI:  v1.0
MMC:   sdhci@74a4900: 0
In:    serial@75b0000
Out:   serial@75b0000
Err:   serial@75b0000
Net:   Net Initialization Skipped
No ethernet found.
Hit any key to stop autoboot:  0
Card did not respond to voltage select! <---------------
Ramon Fried May 28, 2018, 9:28 p.m. | #16
On Tue, May 29, 2018, 12:11 AM Jorge Ramirez-Ortiz
<jramirez@baylibre.com> wrote:
>
> On 05/28/2018 11:07 PM, Jorge Ramirez-Ortiz wrote:
>
> Jorge, I just sent you a fix. can you test it and if it works I'll
> push it upstream.
>
>
> yes I can see the console now so that fix is good.
>
> however there must be some other regression lurking because the system wont boot a kernel from the SD card
>
Sorry, I don't recall seeing this ever, perhaps the regression is
caused by another patch ?
are you sure it's from the latest uart fixes ? can you bisect ?
Thanks,
Ramon.
>
> sorry forgot the trace.
> I think you might have fixed this one as well Ramon (I remember you mentioning it in some other thread?)
>
> U-Boot 2018.05-00430-g5ce9aca (May 28 2018 - 23:09:02)                                                                                                                                       [15/592]
> Qualcomm-DragonBoard 820C
>
> DRAM:  3 GiB
> PSCI:  v1.0
> MMC:   sdhci@74a4900: 0
> In:    serial@75b0000
> Out:   serial@75b0000
> Err:   serial@75b0000
> Net:   Net Initialization Skipped
> No ethernet found.
> Hit any key to stop autoboot:  0
> Card did not respond to voltage select! <---------------
>

Patch

diff --git a/arch/arm/mach-snapdragon/clock-apq8016.c b/arch/arm/mach-snapdragon/clock-apq8016.c
index 9c0cc1c22c..6e4a0ccb90 100644
--- a/arch/arm/mach-snapdragon/clock-apq8016.c
+++ b/arch/arm/mach-snapdragon/clock-apq8016.c
@@ -17,7 +17,6 @@ 
 
 /* GPLL0 clock control registers */
 #define GPLL0_STATUS_ACTIVE BIT(17)
-#define APCS_GPLL_ENA_VOTE_GPLL0 BIT(0)
 
 static const struct bcr_regs sdc_regs[] = {
 	{
@@ -36,11 +35,17 @@  static const struct bcr_regs sdc_regs[] = {
 	}
 };
 
-static struct gpll0_ctrl gpll0_ctrl = {
+static struct pll_vote_clk gpll0_vote_clk = {
 	.status = GPLL0_STATUS,
 	.status_bit = GPLL0_STATUS_ACTIVE,
 	.ena_vote = APCS_GPLL_ENA_VOTE,
-	.vote_bit = APCS_GPLL_ENA_VOTE_GPLL0,
+	.vote_bit = BIT(0),
+};
+
+static struct vote_clk gcc_blsp1_ahb_clk = {
+	.cbcr_reg = BLSP1_AHB_CBCR,
+	.ena_vote = APCS_CLOCK_BRANCH_ENA_VOTE,
+	.vote_bit = BIT(10),
 };
 
 /* SDHCI */
@@ -55,7 +60,7 @@  static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
 	/* 800Mhz/div, gpll0 */
 	clk_rcg_set_rate_mnd(priv->base, &sdc_regs[slot], div, 0, 0,
 			     CFG_CLK_SRC_GPLL0);
-	clk_enable_gpll0(priv->base, &gpll0_ctrl);
+	clk_enable_gpll0(priv->base, &gpll0_vote_clk);
 	clk_enable_cbc(priv->base + SDCC_APPS_CBCR(slot));
 
 	return rate;
@@ -72,12 +77,16 @@  static const struct bcr_regs uart2_regs = {
 /* UART: 115200 */
 static int clk_init_uart(struct msm_clk_priv *priv)
 {
-	/* Enable iface clk */
-	clk_enable_cbc(priv->base + BLSP1_AHB_CBCR);
+	/* Enable AHB clock */
+	clk_enable_vote_clk(priv->base, &gcc_blsp1_ahb_clk);
+
 	/* 7372800 uart block clock @ GPLL0 */
 	clk_rcg_set_rate_mnd(priv->base, &uart2_regs, 1, 144, 15625,
 			     CFG_CLK_SRC_GPLL0);
-	clk_enable_gpll0(priv->base, &gpll0_ctrl);
+
+	/* Vote for gpll0 clock */
+	clk_enable_gpll0(priv->base, &gpll0_vote_clk);
+
 	/* Enable core clk */
 	clk_enable_cbc(priv->base + BLSP1_UART2_APPS_CBCR);
 
diff --git a/arch/arm/mach-snapdragon/clock-apq8096.c b/arch/arm/mach-snapdragon/clock-apq8096.c
index 008649a4c6..628c38785b 100644
--- a/arch/arm/mach-snapdragon/clock-apq8096.c
+++ b/arch/arm/mach-snapdragon/clock-apq8096.c
@@ -27,7 +27,7 @@  static const struct bcr_regs sdc_regs = {
 	.D = SDCC2_D,
 };
 
-static const struct gpll0_ctrl gpll0_ctrl = {
+static const struct pll_vote_clk gpll0_vote_clk = {
 	.status = GPLL0_STATUS,
 	.status_bit = GPLL0_STATUS_ACTIVE,
 	.ena_vote = APCS_GPLL_ENA_VOTE,
@@ -41,7 +41,7 @@  static int clk_init_sdc(struct msm_clk_priv *priv, uint rate)
 	clk_enable_cbc(priv->base + SDCC2_AHB_CBCR);
 	clk_rcg_set_rate_mnd(priv->base, &sdc_regs, div, 0, 0,
 			     CFG_CLK_SRC_GPLL0);
-	clk_enable_gpll0(priv->base, &gpll0_ctrl);
+	clk_enable_gpll0(priv->base, &gpll0_vote_clk);
 	clk_enable_cbc(priv->base + SDCC2_APPS_CBCR);
 
 	return rate;
diff --git a/arch/arm/mach-snapdragon/clock-snapdragon.c b/arch/arm/mach-snapdragon/clock-snapdragon.c
index f738f57043..85526186c6 100644
--- a/arch/arm/mach-snapdragon/clock-snapdragon.c
+++ b/arch/arm/mach-snapdragon/clock-snapdragon.c
@@ -30,7 +30,7 @@  void clk_enable_cbc(phys_addr_t cbcr)
 		;
 }
 
-void clk_enable_gpll0(phys_addr_t base, const struct gpll0_ctrl *gpll0)
+void clk_enable_gpll0(phys_addr_t base, const struct pll_vote_clk *gpll0)
 {
 	if (readl(base + gpll0->status) & gpll0->status_bit)
 		return; /* clock already enabled */
@@ -41,6 +41,21 @@  void clk_enable_gpll0(phys_addr_t base, const struct gpll0_ctrl *gpll0)
 		;
 }
 
+#define BRANCH_ON_VAL (0)
+#define BRANCH_NOC_FSM_ON_VAL BIT(29)
+#define BRANCH_CHECK_MASK GENMASK(31, 28)
+
+void clk_enable_vote_clk(phys_addr_t base, const struct vote_clk *vclk)
+{
+	u32 val;
+
+	setbits_le32(base + vclk->ena_vote, vclk->vote_bit);
+	do {
+		val = readl(base + vclk->cbcr_reg);
+		val &= BRANCH_CHECK_MASK;
+	} while ((val != BRANCH_ON_VAL) && (val != BRANCH_NOC_FSM_ON_VAL));
+}
+
 #define APPS_CMD_RGCR_UPDATE BIT(0)
 
 /* Update clock command via CMD_RGCR */
diff --git a/arch/arm/mach-snapdragon/clock-snapdragon.h b/arch/arm/mach-snapdragon/clock-snapdragon.h
index 2cff4f8a06..3ae21099c2 100644
--- a/arch/arm/mach-snapdragon/clock-snapdragon.h
+++ b/arch/arm/mach-snapdragon/clock-snapdragon.h
@@ -11,13 +11,18 @@ 
 #define CFG_CLK_SRC_GPLL0 (1 << 8)
 #define CFG_CLK_SRC_MASK  (7 << 8)
 
-struct gpll0_ctrl {
+struct pll_vote_clk {
 	uintptr_t status;
 	int status_bit;
 	uintptr_t ena_vote;
 	int vote_bit;
 };
 
+struct vote_clk {
+	uintptr_t cbcr_reg;
+	uintptr_t ena_vote;
+	int vote_bit;
+};
 struct bcr_regs {
 	uintptr_t cfg_rcgr;
 	uintptr_t cmd_rcgr;
@@ -30,7 +35,7 @@  struct msm_clk_priv {
 	phys_addr_t base;
 };
 
-void clk_enable_gpll0(phys_addr_t base, const struct gpll0_ctrl *pll0);
+void clk_enable_gpll0(phys_addr_t base, const struct pll_vote_clk *gpll0);
 void clk_bcr_update(phys_addr_t apps_cmd_rgcr);
 void clk_enable_cbc(phys_addr_t cbcr);
 void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs,
diff --git a/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h b/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h
index ae784387fa..520e2e6bd7 100644
--- a/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h
+++ b/arch/arm/mach-snapdragon/include/mach/sysmap-apq8016.h
@@ -13,6 +13,7 @@ 
 /* Clocks: (from CLK_CTL_BASE)  */
 #define GPLL0_STATUS			(0x2101C)
 #define APCS_GPLL_ENA_VOTE		(0x45000)
+#define APCS_CLOCK_BRANCH_ENA_VOTE (0x45004)
 
 #define SDCC_BCR(n)			((n * 0x1000) + 0x41000)
 #define SDCC_CMD_RCGR(n)		((n * 0x1000) + 0x41004)