[v5,01/44] dt-bindings: clock: Add new bindings for TI Davinci PLL clocks
diff mbox

Message ID CAHCN7xKV+Rwep3WiY2aS49iLu+oiZmjtUN2QK+E=hVkqdSPwhw@mail.gmail.com
State Under Review
Headers show

Commit Message

Adam Ford Jan. 11, 2018, 9:34 p.m. UTC
On Thu, Jan 11, 2018 at 3:04 PM, David Lechner <david@lechnology.com> wrote:
> On 01/11/2018 02:58 PM, Adam Ford wrote:
>>
>> On Thu, Jan 11, 2018 at 2:04 PM, David Lechner <david@lechnology.com>
>> wrote:
>>>
>>> On 01/11/2018 12:50 PM, Adam Ford wrote:
>>>>
>>>>
>>>> On Thu, Jan 11, 2018 at 12:29 PM, David Lechner <david@lechnology.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> If removing the "clk_ignore_unused" option causes the board to not
>>>>> boot,
>>>>> then we still have problems that need to be fixed, so please also test
>>>>> without this option.
>>>>
>>>>
>>>>
>>>> Without this option, it still does not boot.  Without device tree it
>>>> hangs after:
>>>>
>>>> [snip]
>>>>
>>>> NET: Registered protocol family 17
>>>> Loading compiled-in X.509 certificates
>>>> console [netcon0] enabled
>>>> netconsole: network logging started
>>>> davinci_emac davinci_emac.1: using random MAC addr: 5e:38:1a:1f:4f:77
>>>> mmc0: host does not support reading read-only switch, assuming
>>>> write-enable
>>>> hctosys: unable to open rtc device (rtc0)
>>>> mmc0: new high speed SDHC card at address b368
>>>>
>>>>
>>>> With device tree it hangs after:
>>>>
>>>> [snip]
>>>> mmc0: host does not support reading read-only switch, assuming
>>>> write-enable
>>>> mmc0: new high speed SDHC card at address b368
>>>> mmcblk0: mmc0:b368 00000 3.75 GiB
>>>>    mmcblk0: p1 p2
>>>> pca953x 0-0020: 0-0020 supply vcc not found, using dummy regulator
>>>> pca953x 0-0020: failed reading register
>>>> pca953x: probe of 0-0020 failed with error -121
>>>> console [netcon0] enabled
>>>> netconsole: network logging started
>>>> davinci_emac 1e20000.ethernet: incompatible machine/device type for
>>>> reading mac address
>>>> hctosys: unable to open rtc device (rtc0)
>>>>
>>>>
>>>
>>> Please try this change:
>>>
>>> diff --git a/drivers/clk/davinci/psc-da850.c
>>> b/drivers/clk/davinci/psc-da850.c
>>> index 3fd6b49..a526cc2 100644
>>> --- a/drivers/clk/davinci/psc-da850.c
>>> +++ b/drivers/clk/davinci/psc-da850.c
>>> @@ -17,7 +17,7 @@ static const struct davinci_psc_clk_info
>>> da850_psc0_info[]
>>> __initconst = {
>>>          LPSC(0, 0, tpcc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>>          LPSC(1, 0, tptc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>>          LPSC(2, 0, tptc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>> -       LPSC(3, 0, aemif, pll0_sysclk3, 0),
>>> +       LPSC(3, 0, aemif, pll0_sysclk3, LPSC_ALWAYS_ENABLED),
>>>          LPSC(4, 0, spi0, pll0_sysclk2, 0),
>>>          LPSC(5, 0, mmcsd0, pll0_sysclk2, 0),
>>>          LPSC(6, 0, aintc, pll0_sysclk4, LPSC_ALWAYS_ENABLED),
>>>
>>>
>>>
>>> If that does not work, try adding LPSC_ALWAYS_ENABLED to all of the
>>> clocks
>>> in this file and then eliminate them one by one until you find which one
>>> is
>>> preventing boot.
>>>
>> Unfortunately, that didn't work.  I switch all the entries in both
>> tables that had a 0 to LPSC_ALWAYS_ENABLED, but no luck booting.
>>
>>> If it still does not boot, there is a similar DIVCLK_ALWAYS_ENABLED flag
>>> in
>>> drivers/clk/davinci/pll-da850.c that you can repeat the exercise with.
>>> Add
>>> DIVCLK_ALWAYS_ENABLED to all of the clocks there and then eliminate it
>>> one
>>> by one until you find the clock that is causing the problem.
>>
>>
>> Still no good news.  I switched all the entries with a 0 to
>> DIVCLK_ALWAYS_ENABLED and it still didn't finish booting.
>>
>> I wonder if Sekhar Nori might have some suggestions.  I didn't look at
>> the code or try to understand it.  I just changed the settings.
>>>
>>>
>
> Even if a clock had another flag besides zero, you will need to add
> LPSC_ALWAYS_ENABLED by or-ing it with the other flag.
>

[snip]


Thanks for clarifying.  I was able to make it work with the following patch:

 };


If you have an updated patch series with those two fixes, I add my
name to the tested-by list.


>
>
[snip]
>
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Lechner Jan. 11, 2018, 9:46 p.m. UTC | #1
On 01/11/2018 03:34 PM, Adam Ford wrote:
> On Thu, Jan 11, 2018 at 3:04 PM, David Lechner <david@lechnology.com> wrote:
>> On 01/11/2018 02:58 PM, Adam Ford wrote:
>>>
>>> On Thu, Jan 11, 2018 at 2:04 PM, David Lechner <david@lechnology.com>
>>> wrote:
>>>>
>>>> On 01/11/2018 12:50 PM, Adam Ford wrote:
>>>>>
>>>>>
>>>>> On Thu, Jan 11, 2018 at 12:29 PM, David Lechner <david@lechnology.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> If removing the "clk_ignore_unused" option causes the board to not
>>>>>> boot,
>>>>>> then we still have problems that need to be fixed, so please also test
>>>>>> without this option.
>>>>>
>>>>>
>>>>>
>>>>> Without this option, it still does not boot.  Without device tree it
>>>>> hangs after:
>>>>>
>>>>> [snip]
>>>>>
>>>>> NET: Registered protocol family 17
>>>>> Loading compiled-in X.509 certificates
>>>>> console [netcon0] enabled
>>>>> netconsole: network logging started
>>>>> davinci_emac davinci_emac.1: using random MAC addr: 5e:38:1a:1f:4f:77
>>>>> mmc0: host does not support reading read-only switch, assuming
>>>>> write-enable
>>>>> hctosys: unable to open rtc device (rtc0)
>>>>> mmc0: new high speed SDHC card at address b368
>>>>>
>>>>>
>>>>> With device tree it hangs after:
>>>>>
>>>>> [snip]
>>>>> mmc0: host does not support reading read-only switch, assuming
>>>>> write-enable
>>>>> mmc0: new high speed SDHC card at address b368
>>>>> mmcblk0: mmc0:b368 00000 3.75 GiB
>>>>>     mmcblk0: p1 p2
>>>>> pca953x 0-0020: 0-0020 supply vcc not found, using dummy regulator
>>>>> pca953x 0-0020: failed reading register
>>>>> pca953x: probe of 0-0020 failed with error -121
>>>>> console [netcon0] enabled
>>>>> netconsole: network logging started
>>>>> davinci_emac 1e20000.ethernet: incompatible machine/device type for
>>>>> reading mac address
>>>>> hctosys: unable to open rtc device (rtc0)
>>>>>
>>>>>
>>>>
>>>> Please try this change:
>>>>
>>>> diff --git a/drivers/clk/davinci/psc-da850.c
>>>> b/drivers/clk/davinci/psc-da850.c
>>>> index 3fd6b49..a526cc2 100644
>>>> --- a/drivers/clk/davinci/psc-da850.c
>>>> +++ b/drivers/clk/davinci/psc-da850.c
>>>> @@ -17,7 +17,7 @@ static const struct davinci_psc_clk_info
>>>> da850_psc0_info[]
>>>> __initconst = {
>>>>           LPSC(0, 0, tpcc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>>>           LPSC(1, 0, tptc0, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>>>           LPSC(2, 0, tptc1, pll0_sysclk2, LPSC_ALWAYS_ENABLED),
>>>> -       LPSC(3, 0, aemif, pll0_sysclk3, 0),
>>>> +       LPSC(3, 0, aemif, pll0_sysclk3, LPSC_ALWAYS_ENABLED),
>>>>           LPSC(4, 0, spi0, pll0_sysclk2, 0),
>>>>           LPSC(5, 0, mmcsd0, pll0_sysclk2, 0),
>>>>           LPSC(6, 0, aintc, pll0_sysclk4, LPSC_ALWAYS_ENABLED),
>>>>
>>>>
>>>>
>>>> If that does not work, try adding LPSC_ALWAYS_ENABLED to all of the
>>>> clocks
>>>> in this file and then eliminate them one by one until you find which one
>>>> is
>>>> preventing boot.
>>>>
>>> Unfortunately, that didn't work.  I switch all the entries in both
>>> tables that had a 0 to LPSC_ALWAYS_ENABLED, but no luck booting.
>>>
>>>> If it still does not boot, there is a similar DIVCLK_ALWAYS_ENABLED flag
>>>> in
>>>> drivers/clk/davinci/pll-da850.c that you can repeat the exercise with.
>>>> Add
>>>> DIVCLK_ALWAYS_ENABLED to all of the clocks there and then eliminate it
>>>> one
>>>> by one until you find the clock that is causing the problem.
>>>
>>>
>>> Still no good news.  I switched all the entries with a 0 to
>>> DIVCLK_ALWAYS_ENABLED and it still didn't finish booting.
>>>
>>> I wonder if Sekhar Nori might have some suggestions.  I didn't look at
>>> the code or try to understand it.  I just changed the settings.
>>>>
>>>>
>>
>> Even if a clock had another flag besides zero, you will need to add
>> LPSC_ALWAYS_ENABLED by or-ing it with the other flag.
>>
> 
> [snip]
> 
> 
> Thanks for clarifying.  I was able to make it work with the following patch:
> 
> diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
> index 3b4583d..a76b8682 100644
> --- a/drivers/clk/davinci/psc-da850.c
> +++ b/drivers/clk/davinci/psc-da850.c
> @@ -25,7 +25,7 @@ static const struct davinci_psc_clk_info
> da850_psc0_info[] __initconst = {
>          LPSC(9, 0, uart0, pll0_sysclk2, 0),
>          LPSC(13, 0, pruss, pll0_sysclk2, 0),
>          LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
> -       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
> +       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET |
> LPSC_ALWAYS_ENABLED),
>          { }
>   };
> 
> 
> If you have an updated patch series with those two fixes, I add my
> name to the tested-by list.
> 
> 
>>
>>
> [snip]
>>

Great! Thanks again for testing.

Sekhar, have you had a chance to look at the rest of the patches in the
series?

I'll wait a bit before I send a v6 to see if any other comments come.

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori Jan. 12, 2018, 6:03 a.m. UTC | #2
On Friday 12 January 2018 03:16 AM, David Lechner wrote:
> 
> Sekhar, have you had a chance to look at the rest of the patches in the
> series?

Not yet. Sorry about the slow (and piecemeal) review.

> 
> I'll wait a bit before I send a v6 to see if any other comments come.

Yes. I will let you know once I am done reviewing the whole series.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori Jan. 16, 2018, 11:22 a.m. UTC | #3
Hi Adam, David,

On Friday 12 January 2018 03:04 AM, Adam Ford wrote:
> Thanks for clarifying.  I was able to make it work with the following patch:
> 
> diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
> index 3b4583d..a76b8682 100644
> --- a/drivers/clk/davinci/psc-da850.c
> +++ b/drivers/clk/davinci/psc-da850.c
> @@ -25,7 +25,7 @@ static const struct davinci_psc_clk_info
> da850_psc0_info[] __initconst = {
>         LPSC(9, 0, uart0, pll0_sysclk2, 0),
>         LPSC(13, 0, pruss, pll0_sysclk2, 0),
>         LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
> -       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
> +       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET |
> LPSC_ALWAYS_ENABLED),

Keeping the DSP clock always enabled was not needed earlier AFAICS, so
this needs to be investigated.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Ford Jan. 16, 2018, 12:21 p.m. UTC | #4
On Tue, Jan 16, 2018 at 5:22 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> Hi Adam, David,
>
> On Friday 12 January 2018 03:04 AM, Adam Ford wrote:
>> Thanks for clarifying.  I was able to make it work with the following patch:
>>
>> diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
>> index 3b4583d..a76b8682 100644
>> --- a/drivers/clk/davinci/psc-da850.c
>> +++ b/drivers/clk/davinci/psc-da850.c
>> @@ -25,7 +25,7 @@ static const struct davinci_psc_clk_info
>> da850_psc0_info[] __initconst = {
>>         LPSC(9, 0, uart0, pll0_sysclk2, 0),
>>         LPSC(13, 0, pruss, pll0_sysclk2, 0),
>>         LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
>> -       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
>> +       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET |
>> LPSC_ALWAYS_ENABLED),
>
> Keeping the DSP clock always enabled was not needed earlier AFAICS, so
> this needs to be investigated.

I was testing the DA850-evm and found it was required or the DA850
wouldn't boot.  I don't know enough of why to explain it.  I went
through all the clocks as suggested by David, and this one-line patch
fixed the hanging problem I had.  Without it the AM1808 board I have
won't boot.

adam
>
> Thanks,
> Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Lechner Jan. 16, 2018, 4:41 p.m. UTC | #5
On 01/16/2018 06:21 AM, Adam Ford wrote:
> On Tue, Jan 16, 2018 at 5:22 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>> Hi Adam, David,
>>
>> On Friday 12 January 2018 03:04 AM, Adam Ford wrote:
>>> Thanks for clarifying.  I was able to make it work with the following patch:
>>>
>>> diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
>>> index 3b4583d..a76b8682 100644
>>> --- a/drivers/clk/davinci/psc-da850.c
>>> +++ b/drivers/clk/davinci/psc-da850.c
>>> @@ -25,7 +25,7 @@ static const struct davinci_psc_clk_info
>>> da850_psc0_info[] __initconst = {
>>>          LPSC(9, 0, uart0, pll0_sysclk2, 0),
>>>          LPSC(13, 0, pruss, pll0_sysclk2, 0),
>>>          LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
>>> -       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
>>> +       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET |
>>> LPSC_ALWAYS_ENABLED),
>>
>> Keeping the DSP clock always enabled was not needed earlier AFAICS, so
>> this needs to be investigated.
> 
> I was testing the DA850-evm and found it was required or the DA850
> wouldn't boot.  I don't know enough of why to explain it.  I went
> through all the clocks as suggested by David, and this one-line patch
> fixed the hanging problem I had.  Without it the AM1808 board I have
> won't boot.

That's interesting because 1) the AM1808 does not have a DSP and 2) I
am using an AM1808 based system as well that boots without having this
always enabled (AM1808BZWT4 to be exact).

I guess I will leave this change out of the next revision so that we
don't forget that this needs to be addressed. Sekhar found a bug in
the new PSC driver with the LPSC_FORCE bit, so maybe that could be
the difference.

Adam, before the changes in this series, did you have
CONFIG_DAVINCI_RESET_CLOCKS=y in your config?
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/clk/davinci/psc-da850.c b/drivers/clk/davinci/psc-da850.c
index 3b4583d..a76b8682 100644
--- a/drivers/clk/davinci/psc-da850.c
+++ b/drivers/clk/davinci/psc-da850.c
@@ -25,7 +25,7 @@  static const struct davinci_psc_clk_info
da850_psc0_info[] __initconst = {
        LPSC(9, 0, uart0, pll0_sysclk2, 0),
        LPSC(13, 0, pruss, pll0_sysclk2, 0),
        LPSC(14, 0, arm, pll0_sysclk6, LPSC_ALWAYS_ENABLED),
-       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET),
+       LPSC(15, 1, dsp, pll0_sysclk1, LPSC_FORCE | LPSC_LOCAL_RESET |
LPSC_ALWAYS_ENABLED),
        { }