Re: [PATCH 1/2] clk: samsung: exynos5422: add missing parent GSCL block clocks

[Mike Turquette]

On 12/22, Sylwester Nawrocki wrote:
> Mike,
> 
> On 22/12/15 19:44, Mike Turquette wrote:
> > This is superseded by the pull request found in Message-ID:
> > <5671a456.9030...@samsung.com>, correct?
> 
> The two pull requests are based on same branch, first 2 commits are
> tagged with for-4.5-clk-exynos5420 tag and the whole branch is tagged
> as clk-samsung-4.5. I assumed it's fine to use just the below tag for
> arm-soc.

OK, I'll interpret that as a "yes".

Thanks,
Mike

> 
> >>> > > The following changes since commit 
> >>> > > 9f9499ae8e6415cefc4fe0a96ad0e27864353c89:
> >>> > > 
> >>> > >   Linux 4.4-rc5 (2015-12-13 17:42:58 -0800)
> >>> > > 
> >>> > > are available in the git repository at:
> >>> > > 
> >>> > >   git://linuxtv.org/snawrocki/samsung.git tags/for-4.5-clk-exynos5420
> >>> > > 
> >>> > > for you to fetch changes up to 
> >>> > > bee4f87f01dc30fcf9e05eb55b833f89fd9bb4f4:
> >>> > > 
> >>> > >   clk: samsung: exynos5420: add cpu clock configuration data and 
> >>> > > instantiate cpu clock (2015-12-16 16:35:26 +0100)
> >>> > > 
> >>> > > 
> >>> > > Samsung exynos5420 SoC clk subsystem support updates:
> >>> > > instantiation of the cpu clocks and addition of the GSCL
> >>> > > IP parent clocks to the list of available consumer clocks.
> >>> > > 
> >>> > > 
> >>> > > Marek Szyprowski (1):
> >>> > >   clk: samsung: exynos542x: add missing parent GSCL block clocks
> >>> > > 
> >>> > > Thomas Abraham (1):
> >>> > >   clk: samsung: exynos5420: add cpu clock configuration data and 
> >>> > > instantiate cpu clock
> >>> > > 
> >>> > >  drivers/clk/samsung/clk-exynos5420.c   |   66 
> >>> > > +---
> >>> > >  include/dt-bindings/clock/exynos5420.h |4 ++
> >>> > >  2 files changed, 64 insertions(+), 6 deletions(-)
> 
> --
> Regards,
> Sylwester
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] clk: samsung: exynos5422: add missing parent GSCL block clocks

[Mike Turquette]

On 12/17, Krzysztof Kozlowski wrote:
> On 17.12.2015 02:23, Sylwester Nawrocki wrote:
> > Krzysztof,
> > 
> > On 09/12/15 14:36, Krzysztof Kozlowski wrote:
> >> W dniu 09.12.2015 o 19:14, Sylwester Nawrocki pisze:
>  Adding Stephen and linux-clk at Cc.
> 
>  On 09/12/15 05:49, Krzysztof Kozlowski wrote:
> >> On 08.12.2015 22:46, Marek Szyprowski wrote:
> >> This patch adds clocks, which are required for preserving parent 
> >> clock
> >> configuration on GSCL power domain on/off.
> >>
> >> Signed-off-by: Marek Szyprowski 
> >> ---
> >>  drivers/clk/samsung/clk-exynos5420.c   | 8 
> >>  include/dt-bindings/clock/exynos5420.h | 2 ++
> >>  2 files changed, 6 insertions(+), 4 deletions(-)
> >>
> >> I suppose that, with ack from clock folks, this can go through 
> >> samsung-soc?
> 
>  I guess it makes more sense that making a stable branch with just
>  this patch to be pulled into arm-soc and clk tree. I'm fine with
>  applying this patch through arm-soc, but I think we also need
>  Mike's or Stephen ack for this.
> 
>  Acked-by: Sylwester Nawrocki 
> >>
> >> I am fine with the branch approach (actually in such cases I make them
> >> anyway just in case).
> >>
> >> As you suggested I'll wait for Mike's or Stepen's acks.
> > 
> > I've put the $subject patch and the other exynos5420 patch which 
> > was a dependency for dts patches (both touching include/dt-bindings/
> > clock/exynos5420.h) onto a stable branch. Below are details if 
> > you need to pull.

This is superseded by the pull request found in Message-ID:
<5671a456.9030...@samsung.com>, correct?

Thanks,
Mike

> 
> Great! Thanks Sylwester.
> BR,
> Krzysztof
> 
> 
> > 
> > 
> > The following changes since commit 9f9499ae8e6415cefc4fe0a96ad0e27864353c89:
> > 
> >   Linux 4.4-rc5 (2015-12-13 17:42:58 -0800)
> > 
> > are available in the git repository at:
> > 
> >   git://linuxtv.org/snawrocki/samsung.git tags/for-4.5-clk-exynos5420
> > 
> > for you to fetch changes up to bee4f87f01dc30fcf9e05eb55b833f89fd9bb4f4:
> > 
> >   clk: samsung: exynos5420: add cpu clock configuration data and 
> > instantiate cpu clock (2015-12-16 16:35:26 +0100)
> > 
> > 
> > Samsung exynos5420 SoC clk subsystem support updates:
> > instantiation of the cpu clocks and addition of the GSCL
> > IP parent clocks to the list of available consumer clocks.
> > 
> > 
> > Marek Szyprowski (1):
> >   clk: samsung: exynos542x: add missing parent GSCL block clocks
> > 
> > Thomas Abraham (1):
> >   clk: samsung: exynos5420: add cpu clock configuration data and 
> > instantiate cpu clock
> > 
> >  drivers/clk/samsung/clk-exynos5420.c   |   66 
> > +---
> >  include/dt-bindings/clock/exynos5420.h |4 ++
> >  2 files changed, 64 insertions(+), 6 deletions(-)
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] clk: samsung: exynos5422: add missing parent GSCL block clocks

[Mike Turquette]

On 12/17, Krzysztof Kozlowski wrote:
> On 17.12.2015 02:23, Sylwester Nawrocki wrote:
> > Krzysztof,
> > 
> > On 09/12/15 14:36, Krzysztof Kozlowski wrote:
> >> W dniu 09.12.2015 o 19:14, Sylwester Nawrocki pisze:
>  Adding Stephen and linux-clk at Cc.
> 
>  On 09/12/15 05:49, Krzysztof Kozlowski wrote:
> >> On 08.12.2015 22:46, Marek Szyprowski wrote:
> >> This patch adds clocks, which are required for preserving parent 
> >> clock
> >> configuration on GSCL power domain on/off.
> >>
> >> Signed-off-by: Marek Szyprowski 
> >> ---
> >>  drivers/clk/samsung/clk-exynos5420.c   | 8 
> >>  include/dt-bindings/clock/exynos5420.h | 2 ++
> >>  2 files changed, 6 insertions(+), 4 deletions(-)
> >>
> >> I suppose that, with ack from clock folks, this can go through 
> >> samsung-soc?
> 
>  I guess it makes more sense that making a stable branch with just
>  this patch to be pulled into arm-soc and clk tree. I'm fine with
>  applying this patch through arm-soc, but I think we also need
>  Mike's or Stephen ack for this.
> 
>  Acked-by: Sylwester Nawrocki 
> >>
> >> I am fine with the branch approach (actually in such cases I make them
> >> anyway just in case).
> >>
> >> As you suggested I'll wait for Mike's or Stepen's acks.
> > 
> > I've put the $subject patch and the other exynos5420 patch which 
> > was a dependency for dts patches (both touching include/dt-bindings/
> > clock/exynos5420.h) onto a stable branch. Below are details if 
> > you need to pull.

This is superseded by the pull request found in Message-ID:
<5671a456.9030...@samsung.com>, correct?

Thanks,
Mike

> 
> Great! Thanks Sylwester.
> BR,
> Krzysztof
> 
> 
> > 
> > 
> > The following changes since commit 9f9499ae8e6415cefc4fe0a96ad0e27864353c89:
> > 
> >   Linux 4.4-rc5 (2015-12-13 17:42:58 -0800)
> > 
> > are available in the git repository at:
> > 
> >   git://linuxtv.org/snawrocki/samsung.git tags/for-4.5-clk-exynos5420
> > 
> > for you to fetch changes up to bee4f87f01dc30fcf9e05eb55b833f89fd9bb4f4:
> > 
> >   clk: samsung: exynos5420: add cpu clock configuration data and 
> > instantiate cpu clock (2015-12-16 16:35:26 +0100)
> > 
> > 
> > Samsung exynos5420 SoC clk subsystem support updates:
> > instantiation of the cpu clocks and addition of the GSCL
> > IP parent clocks to the list of available consumer clocks.
> > 
> > 
> > Marek Szyprowski (1):
> >   clk: samsung: exynos542x: add missing parent GSCL block clocks
> > 
> > Thomas Abraham (1):
> >   clk: samsung: exynos5420: add cpu clock configuration data and 
> > instantiate cpu clock
> > 
> >  drivers/clk/samsung/clk-exynos5420.c   |   66 
> > +---
> >  include/dt-bindings/clock/exynos5420.h |4 ++
> >  2 files changed, 64 insertions(+), 6 deletions(-)
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] clk: samsung: exynos5422: add missing parent GSCL block clocks

[Mike Turquette]

On 12/22, Sylwester Nawrocki wrote:
> Mike,
> 
> On 22/12/15 19:44, Mike Turquette wrote:
> > This is superseded by the pull request found in Message-ID:
> > <5671a456.9030...@samsung.com>, correct?
> 
> The two pull requests are based on same branch, first 2 commits are
> tagged with for-4.5-clk-exynos5420 tag and the whole branch is tagged
> as clk-samsung-4.5. I assumed it's fine to use just the below tag for
> arm-soc.

OK, I'll interpret that as a "yes".

Thanks,
Mike

> 
> >>> > > The following changes since commit 
> >>> > > 9f9499ae8e6415cefc4fe0a96ad0e27864353c89:
> >>> > > 
> >>> > >   Linux 4.4-rc5 (2015-12-13 17:42:58 -0800)
> >>> > > 
> >>> > > are available in the git repository at:
> >>> > > 
> >>> > >   git://linuxtv.org/snawrocki/samsung.git tags/for-4.5-clk-exynos5420
> >>> > > 
> >>> > > for you to fetch changes up to 
> >>> > > bee4f87f01dc30fcf9e05eb55b833f89fd9bb4f4:
> >>> > > 
> >>> > >   clk: samsung: exynos5420: add cpu clock configuration data and 
> >>> > > instantiate cpu clock (2015-12-16 16:35:26 +0100)
> >>> > > 
> >>> > > 
> >>> > > Samsung exynos5420 SoC clk subsystem support updates:
> >>> > > instantiation of the cpu clocks and addition of the GSCL
> >>> > > IP parent clocks to the list of available consumer clocks.
> >>> > > 
> >>> > > 
> >>> > > Marek Szyprowski (1):
> >>> > >   clk: samsung: exynos542x: add missing parent GSCL block clocks
> >>> > > 
> >>> > > Thomas Abraham (1):
> >>> > >   clk: samsung: exynos5420: add cpu clock configuration data and 
> >>> > > instantiate cpu clock
> >>> > > 
> >>> > >  drivers/clk/samsung/clk-exynos5420.c   |   66 
> >>> > > +---
> >>> > >  include/dt-bindings/clock/exynos5420.h |4 ++
> >>> > >  2 files changed, 64 insertions(+), 6 deletions(-)
> 
> --
> Regards,
> Sylwester
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/4] scheduler-based cpu frequency scaling

[Mike Turquette]

On Mon, May 4, 2015 at 4:27 PM, Rafael J. Wysocki  wrote:
> On Monday, May 04, 2015 03:10:37 PM Michael Turquette wrote:
>> This series implements an event-driven cpufreq governor that scales cpu
>> frequency as a function of cfs runqueue utilization. The intent of this RFC 
>> is
>> to get some discussion going about how the scheduler can become the policy
>> engine for selecting cpu frequency, what limitations exist and what design do
>> we want to take to get to a solution.
>>
>> This work is a different take on the patches I posted in November:
>> http://lkml.kernel.org/r/<1413958051-7103-1-git-send-email-mturque...@linaro.org>
>>
>> This series depends on having frequency-invariant representations for load.
>> This requires Vincent's recently merged cpu capacity rework patches, as well 
>> as
>> a new patch from Morten included here. Morten's patch will likely make an
>> appearance in his energy aware scheduling v4 series.
>>
>> Thanks to Juri Lelli  for contributing to the development
>> of the governor.
>>
>> A git branch with these patches can be pulled from here:
>> https://git.linaro.org/people/mike.turquette/linux.git sched-freq
>>
>> Smoke testing has been done on an OMAP4 Pandaboard and an Exynos 5800
>> Chromebook2. Extensive benchmarking and regression testing has not yet been
>> done. Before sinking too much time into extensive testing I'd like to get
>> feedback on the general design.
>>
>> Michael Turquette (3):
>>   sched: sched feature for cpu frequency selection
>>   sched: export get_cpu_usage & capacity_orig_of
>>   sched: cpufreq_sched_cfs: PELT-based cpu frequency scaling
>>
>> Morten Rasmussen (1):
>>   arm: Frequency invariant scheduler load-tracking support
>>
>>  arch/arm/include/asm/topology.h |   7 +
>>  arch/arm/kernel/smp.c   |  53 ++-
>>  arch/arm/kernel/topology.c  |  17 +++
>>  drivers/cpufreq/Kconfig |  24 
>>  include/linux/cpufreq.h |   3 +
>>  kernel/sched/Makefile   |   1 +
>>  kernel/sched/cpufreq_cfs.c  | 311 
>> 
>>  kernel/sched/fair.c |  48 +++
>>  kernel/sched/features.h |   6 +
>>  kernel/sched/sched.h|  39 +
>>  10 files changed, 475 insertions(+), 34 deletions(-)
>>  create mode 100644 kernel/sched/cpufreq_cfs.c
>
> Can you *please* always CC PM-related patches to linux-pm?

Will do. Apologies for the oversight.

Regards,
Mike

>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/4] scheduler-based cpu frequency scaling

[Mike Turquette]

On Mon, May 4, 2015 at 4:27 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
 On Monday, May 04, 2015 03:10:37 PM Michael Turquette wrote:
 This series implements an event-driven cpufreq governor that scales cpu
 frequency as a function of cfs runqueue utilization. The intent of this RFC 
 is
 to get some discussion going about how the scheduler can become the policy
 engine for selecting cpu frequency, what limitations exist and what design do
 we want to take to get to a solution.

 This work is a different take on the patches I posted in November:
 http://lkml.kernel.org/r/1413958051-7103-1-git-send-email-mturque...@linaro.org

 This series depends on having frequency-invariant representations for load.
 This requires Vincent's recently merged cpu capacity rework patches, as well 
 as
 a new patch from Morten included here. Morten's patch will likely make an
 appearance in his energy aware scheduling v4 series.

 Thanks to Juri Lelli juri.le...@arm.com for contributing to the development
 of the governor.

 A git branch with these patches can be pulled from here:
 https://git.linaro.org/people/mike.turquette/linux.git sched-freq

 Smoke testing has been done on an OMAP4 Pandaboard and an Exynos 5800
 Chromebook2. Extensive benchmarking and regression testing has not yet been
 done. Before sinking too much time into extensive testing I'd like to get
 feedback on the general design.

 Michael Turquette (3):
   sched: sched feature for cpu frequency selection
   sched: export get_cpu_usage  capacity_orig_of
   sched: cpufreq_sched_cfs: PELT-based cpu frequency scaling

 Morten Rasmussen (1):
   arm: Frequency invariant scheduler load-tracking support

  arch/arm/include/asm/topology.h |   7 +
  arch/arm/kernel/smp.c   |  53 ++-
  arch/arm/kernel/topology.c  |  17 +++
  drivers/cpufreq/Kconfig |  24 
  include/linux/cpufreq.h |   3 +
  kernel/sched/Makefile   |   1 +
  kernel/sched/cpufreq_cfs.c  | 311 
 
  kernel/sched/fair.c |  48 +++
  kernel/sched/features.h |   6 +
  kernel/sched/sched.h|  39 +
  10 files changed, 475 insertions(+), 34 deletions(-)
  create mode 100644 kernel/sched/cpufreq_cfs.c

 Can you *please* always CC PM-related patches to linux-pm?

Will do. Apologies for the oversight.

Regards,
Mike



 --
 I speak only for myself.
 Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: AM335x OMAP2 common clock external fixed-clock registration

[Mike Turquette]

On Wed, Apr 15, 2015 at 12:47 PM, Michael Welling  wrote:
> On Wed, Apr 15, 2015 at 09:43:30PM +0300, Tero Kristo wrote:
>> On 04/15/2015 05:09 PM, Michael Welling wrote:
>> >On Wed, Apr 15, 2015 at 09:34:48AM +0300, Tero Kristo wrote:
>> >>On 04/15/2015 12:17 AM, Michael Welling wrote:
>> >>>Greetings,
>> >>>
>> >>>I have developed an AM3354 based SoM and it uses an external SI5351 clock
>> >>>generator to drive the clock inputs for an external duart and I2S audio
>> >>>master clock. With the registration according to the documentation the
>> >>>reference clock is not being detected and hence the clock generator is
>> >>>not working as expect.
>> >>>
>> >>>After trying many different things, I started to look around the mailing
>> >>>lists to find information related to this issue.
>> >>>
>> >>>I came acrossed post that has the exact same issue:
>> >>>https://lkml.org/lkml/2013/2/18/468
>> >>>
>> >>>Seeing as the patch did not land upstream, I am wondering if there is
>> >>>a solution that I am not seeing.
>> >>>
>> >>>I am willing to provide a patch given appropriate guidance.
>> >>
>> >>Hi Michael,
>> >>
>> >>The info on the email you referenced is kind of obsolete, TI SoCs
>> >>are calling of_clk_init() during boot now, and thus external clock
>> >>nodes should be registered fine also. Maybe you can provide the
>> >>actual DTS patch you are trying out so we can help better...? Are
>> >
>> >See attached patch and console output.
>>
>> I see a bug in your dt data.
>>
>> 
>>
>> + clocks {
>> + ref27: ref27 {
>> + #clock-cells = <0>;
>> + compatibale = "fixed-clock";
>>
>> This should be compatible, right? DT is annoying in that it doesn't
>> verify property names.
>>
>
> Ooops.
>
> Now the clock appears in /sys/kernel/debug/clk:
> root@som3517-som200:/sys/kernel/debug/clk# cat clk_summary
>clock enable_cnt  prepare_cntrate   
> accuracy   phase
> 
>  ref27002700  
> 0 0
> ...
>
> There is still an issue with the si5351.
>
> I had to comment out the clk_put here for the frequency to show up:
> http://lxr.free-electrons.com/source/drivers/clk/clk-si5351.c#L1133
>
> Ideas?

What is the most recent upstream commit that you are based on?

Regards,
Mike

>
>> + clock-frequency = <2700>;
>> + };
>> + };
>>
>> -Tero
>>
>> >
>> >>you seeing any boot time error / warning prints for your new clock?
>> >
>> >With the debug messages on you see that the reference clock is not being
>> >detected.
>> >
>> >Whilest debugging I found that the of_clk_get is returning an error no 
>> >matter
>> >which clock I pass it:
>> >http://lxr.free-electrons.com/source/drivers/clk/clk-si5351.c#L1131
>> >
>> >>
>> >>-Tero
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: AM335x OMAP2 common clock external fixed-clock registration

[Mike Turquette]

On Wed, Apr 15, 2015 at 12:47 PM, Michael Welling mwell...@ieee.org wrote:
 On Wed, Apr 15, 2015 at 09:43:30PM +0300, Tero Kristo wrote:
 On 04/15/2015 05:09 PM, Michael Welling wrote:
 On Wed, Apr 15, 2015 at 09:34:48AM +0300, Tero Kristo wrote:
 On 04/15/2015 12:17 AM, Michael Welling wrote:
 Greetings,
 
 I have developed an AM3354 based SoM and it uses an external SI5351 clock
 generator to drive the clock inputs for an external duart and I2S audio
 master clock. With the registration according to the documentation the
 reference clock is not being detected and hence the clock generator is
 not working as expect.
 
 After trying many different things, I started to look around the mailing
 lists to find information related to this issue.
 
 I came acrossed post that has the exact same issue:
 https://lkml.org/lkml/2013/2/18/468
 
 Seeing as the patch did not land upstream, I am wondering if there is
 a solution that I am not seeing.
 
 I am willing to provide a patch given appropriate guidance.
 
 Hi Michael,
 
 The info on the email you referenced is kind of obsolete, TI SoCs
 are calling of_clk_init() during boot now, and thus external clock
 nodes should be registered fine also. Maybe you can provide the
 actual DTS patch you are trying out so we can help better...? Are
 
 See attached patch and console output.

 I see a bug in your dt data.

 snip

 + clocks {
 + ref27: ref27 {
 + #clock-cells = 0;
 + compatibale = fixed-clock;

 This should be compatible, right? DT is annoying in that it doesn't
 verify property names.


 Ooops.

 Now the clock appears in /sys/kernel/debug/clk:
 root@som3517-som200:/sys/kernel/debug/clk# cat clk_summary
clock enable_cnt  prepare_cntrate   
 accuracy   phase
 
  ref27002700  
 0 0
 ...

 There is still an issue with the si5351.

 I had to comment out the clk_put here for the frequency to show up:
 http://lxr.free-electrons.com/source/drivers/clk/clk-si5351.c#L1133

 Ideas?

What is the most recent upstream commit that you are based on?

Regards,
Mike


 + clock-frequency = 2700;
 + };
 + };

 -Tero

 
 you seeing any boot time error / warning prints for your new clock?
 
 With the debug messages on you see that the reference clock is not being
 detected.
 
 Whilest debugging I found that the of_clk_get is returning an error no 
 matter
 which clock I pass it:
 http://lxr.free-electrons.com/source/drivers/clk/clk-si5351.c#L1131
 
 
 -Tero

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Update the clk tree for linux-next

[Mike Turquette]

Hi Stephen R.,

Stephen Boyd has kindly agreed to co-maintain the clk framework with
me and we have shared commit access to a new git tree at kernel.org:

https://git.kernel.org/cgit/linux/kernel/git/clk/linux.git/

Can you update linux-next to pull from this tree instead of the
git.linaro.org address? They are currently mirrors but the
git.kernel.org tree is the canonical tree from this day forth. The
full path to pull from is:

git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next

This is already updated in MAINTAINERS.

Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Update the clk tree for linux-next

[Mike Turquette]

Hi Stephen R.,

Stephen Boyd has kindly agreed to co-maintain the clk framework with
me and we have shared commit access to a new git tree at kernel.org:

https://git.kernel.org/cgit/linux/kernel/git/clk/linux.git/

Can you update linux-next to pull from this tree instead of the
git.linaro.org address? They are currently mirrors but the
git.kernel.org tree is the canonical tree from this day forth. The
full path to pull from is:

git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next

This is already updated in MAINTAINERS.

Thanks,
Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: mediatek: Export CPU mux clocks for CPU frequency control

[Mike Turquette]

Quoting Viresh Kumar (2015-03-05 00:59:50)
> On 5 March 2015 at 13:12, Sascha Hauer  wrote:
> > We have clk_set_parent for changing the parent and clk_set_rate to
> > change the rate. Use the former for changing the parent and the latter
> > for changing the rate. What you are interested in is changing the
> > parent, so use clk_set_parent for this and not abuse a side effect
> > of clk_set_rate.
> 
> clk_set_rate() for CPUs clock is responsible to change clock rate
> of the CPU. Whether it plays with PLLs or muxes, its not that relevant.

Agreed.

> 
> > My suggestion is to take another approach. Implement clk_set_rate for
> > these muxes and in the set_rate hook:
> >
> > - switch mux to intermediate PLL parent
> > - call clk_set_rate() for the real parent PLL
> > - switch mux back to real parent PLL
> >
> > This way the things happening behind the scenes are completely transparent
> > to the cpufreq driver and you can use cpufreq-dt as is without changes.
> 
> CPUFreq wants to change to intermediate frequency by itself against
> some magic change behind the scene. The major requirement for that
> comes from the fact that we want to send PRE/POST freq notifiers on
> which loops-per-jiffie depends.

I assume you are saying that you want to update loops-per-jiffie while
at an intermediate frequency. Why? This operation should not take very
long.

Imagine a (hypothetical?) processor that changes frequency in many small
steps until it converges to the target rate. Would you want to update
lpj for every step?

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: mediatek: Export CPU mux clocks for CPU frequency control

[Mike Turquette]

Quoting Viresh Kumar (2015-03-05 03:02:06)
> On 5 March 2015 at 16:21, Sascha Hauer  wrote:
> > Given the variance of different SoCs I don't think it makes sense
> > to try to handle all these cases. Instead the cpufreq-dt driver
> > should just call clk_set_rate() on the CPU clock with the desired
> > target frequency. Everything else should be handled in the clock
> > driver which has intimate knowledge about the SoC anyway.
> 
> I agree..
> 
> @Russell: I wanted to ask you this since sometime..
> 
> On CPU-freq changes we fire PRE/POST notifiers and they are
> used for updating loops_per_jiffies which then controls delays.
> 
> Now, it is fine to do that for normal frequencies, but what should be
> the approach for intermediate frequencies ?
> 
> Intermediate freqs: On some platforms changing PLL's straight away
> isn't considered safe and so we switch parent to another stable clock,
> change PLL rate and switch back.
> 
> The *wild* thought I earlier had was to fire these notifiers for even these
> intermediate frequencies, otherwise some of the delays will end before
> they should have and that *might* cause other problems.
> 
> I wanted to know what do you (and other champs) think about this..
> 
> Thanks in advance for your advice.

Sorry, I am not who you asked for advice but I will chime in anyways ;-)

I really hate this intermediate frequency stuff in cpufreq. As we
discussed over lunch in Hong Kong, it does not scale. What if there two
intermediate frequencies? What if a processor steps frequency up in 5KHz
increments (albeit very quickly) until it converges to the target rate?

Or what if we have more complex levels of intermediate frequencies?
Stealing Sascha's excellent ascii art:

---
CPU_PLL ---|   |
   |   |
   |   |- CPU
   |   |
IM_PLL |   |
---

Where the sequence is as follows:

1) set IM_PLL to a new freq
2) reparent CPU to IM_PLL
3) configure CPU_PLL
4) reparent CPU to CPU_PLL
5) restore IM_PLL to original frequency

Steps #1 and #5 are new in this example.

I don't see how the concept of an intermediate frequency in the cpufreq
core could ever scale gracefully to handle corner cases. They may be
hypothetical now but I'd rather see us dodge this mistake.

Furthermore any intermediate-frequency property in a Devicetree binding
would suffer the same fate. Trying to neatly encode some weird sequence
into this generic thing will get very ugly very fast.

For proof please look at clk-divider.c, clk-gate.c, clk-mux.c or
clk-composite.c and you'll see the result of the slow accumulation of
lots and lots of hardware corner cases onto generic code. If I had known
then what I know now I would not have created those generic clock types
and I would have tried for an abstraction layer between generic stuff
(e.g. find the best divider) and the real hardware stuff (write to the
register). Instead I kept all of it together and now things are super
ugly.

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: mediatek: Export CPU mux clocks for CPU frequency control

[Mike Turquette]

Quoting Viresh Kumar (2015-03-05 03:02:06)
 On 5 March 2015 at 16:21, Sascha Hauer s.ha...@pengutronix.de wrote:
  Given the variance of different SoCs I don't think it makes sense
  to try to handle all these cases. Instead the cpufreq-dt driver
  should just call clk_set_rate() on the CPU clock with the desired
  target frequency. Everything else should be handled in the clock
  driver which has intimate knowledge about the SoC anyway.
 
 I agree..
 
 @Russell: I wanted to ask you this since sometime..
 
 On CPU-freq changes we fire PRE/POST notifiers and they are
 used for updating loops_per_jiffies which then controls delays.
 
 Now, it is fine to do that for normal frequencies, but what should be
 the approach for intermediate frequencies ?
 
 Intermediate freqs: On some platforms changing PLL's straight away
 isn't considered safe and so we switch parent to another stable clock,
 change PLL rate and switch back.
 
 The *wild* thought I earlier had was to fire these notifiers for even these
 intermediate frequencies, otherwise some of the delays will end before
 they should have and that *might* cause other problems.
 
 I wanted to know what do you (and other champs) think about this..
 
 Thanks in advance for your advice.

Sorry, I am not who you asked for advice but I will chime in anyways ;-)

I really hate this intermediate frequency stuff in cpufreq. As we
discussed over lunch in Hong Kong, it does not scale. What if there two
intermediate frequencies? What if a processor steps frequency up in 5KHz
increments (albeit very quickly) until it converges to the target rate?

Or what if we have more complex levels of intermediate frequencies?
Stealing Sascha's excellent ascii art:

---
CPU_PLL ---|   |
   |   |
   |   |- CPU
   |   |
IM_PLL |   |
---

Where the sequence is as follows:

1) set IM_PLL to a new freq
2) reparent CPU to IM_PLL
3) configure CPU_PLL
4) reparent CPU to CPU_PLL
5) restore IM_PLL to original frequency

Steps #1 and #5 are new in this example.

I don't see how the concept of an intermediate frequency in the cpufreq
core could ever scale gracefully to handle corner cases. They may be
hypothetical now but I'd rather see us dodge this mistake.

Furthermore any intermediate-frequency property in a Devicetree binding
would suffer the same fate. Trying to neatly encode some weird sequence
into this generic thing will get very ugly very fast.

For proof please look at clk-divider.c, clk-gate.c, clk-mux.c or
clk-composite.c and you'll see the result of the slow accumulation of
lots and lots of hardware corner cases onto generic code. If I had known
then what I know now I would not have created those generic clock types
and I would have tried for an abstraction layer between generic stuff
(e.g. find the best divider) and the real hardware stuff (write to the
register). Instead I kept all of it together and now things are super
ugly.

Regards,
Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: mediatek: Export CPU mux clocks for CPU frequency control

[Mike Turquette]

Quoting Viresh Kumar (2015-03-05 00:59:50)
 On 5 March 2015 at 13:12, Sascha Hauer s.ha...@pengutronix.de wrote:
  We have clk_set_parent for changing the parent and clk_set_rate to
  change the rate. Use the former for changing the parent and the latter
  for changing the rate. What you are interested in is changing the
  parent, so use clk_set_parent for this and not abuse a side effect
  of clk_set_rate.
 
 clk_set_rate() for CPUs clock is responsible to change clock rate
 of the CPU. Whether it plays with PLLs or muxes, its not that relevant.

Agreed.

 
  My suggestion is to take another approach. Implement clk_set_rate for
  these muxes and in the set_rate hook:
 
  - switch mux to intermediate PLL parent
  - call clk_set_rate() for the real parent PLL
  - switch mux back to real parent PLL
 
  This way the things happening behind the scenes are completely transparent
  to the cpufreq driver and you can use cpufreq-dt as is without changes.
 
 CPUFreq wants to change to intermediate frequency by itself against
 some magic change behind the scene. The major requirement for that
 comes from the fact that we want to send PRE/POST freq notifiers on
 which loops-per-jiffie depends.

I assume you are saying that you want to update loops-per-jiffie while
at an intermediate frequency. Why? This operation should not take very
long.

Imagine a (hypothetical?) processor that changes frequency in many small
steps until it converges to the target rate. Would you want to update
lpj for every step?

Regards,
Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 4/6] clk: at91: implement suspend/resume for the PMC irqchip

[Mike Turquette]

Quoting Boris Brezillon (2015-03-02 01:18:16)
> The irq line used by the PMC block is shared with several peripherals
> including the init timer which is registering its handler with
> IRQF_NO_SUSPEND.
> Implement the appropriate suspend/resume callback for the PMC irqchip,
> and inform irq core that PMC irq handler can be safely called while
> the system is suspended by setting IRQF_COND_SUSPEND.
> 
> Signed-off-by: Boris Brezillon 

Looks OK to me. I'm not picking it due to the dependency you listed in
the cover letter.

Regards,
Mike

> ---
>  drivers/clk/at91/pmc.c | 20 +++-
>  drivers/clk/at91/pmc.h |  1 +
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
> index f07c815..3f27d21 100644
> --- a/drivers/clk/at91/pmc.c
> +++ b/drivers/clk/at91/pmc.c
> @@ -89,12 +89,29 @@ static int pmc_irq_set_type(struct irq_data *d, unsigned 
> type)
> return 0;
>  }
>  
> +static void pmc_irq_suspend(struct irq_data *d)
> +{
> +   struct at91_pmc *pmc = irq_data_get_irq_chip_data(d);
> +
> +   pmc->imr = pmc_read(pmc, AT91_PMC_IMR);
> +   pmc_write(pmc, AT91_PMC_IDR, pmc->imr);
> +}
> +
> +static void pmc_irq_resume(struct irq_data *d)
> +{
> +   struct at91_pmc *pmc = irq_data_get_irq_chip_data(d);
> +
> +   pmc_write(pmc, AT91_PMC_IER, pmc->imr);
> +}
> +
>  static struct irq_chip pmc_irq = {
> .name = "PMC",
> .irq_disable = pmc_irq_mask,
> .irq_mask = pmc_irq_mask,
> .irq_unmask = pmc_irq_unmask,
> .irq_set_type = pmc_irq_set_type,
> +   .irq_suspend = pmc_irq_suspend,
> +   .irq_resume = pmc_irq_resume,
>  };
>  
>  static struct lock_class_key pmc_lock_class;
> @@ -224,7 +241,8 @@ static struct at91_pmc *__init at91_pmc_init(struct 
> device_node *np,
> goto out_free_pmc;
>  
> pmc_write(pmc, AT91_PMC_IDR, 0x);
> -   if (request_irq(pmc->virq, pmc_irq_handler, IRQF_SHARED, "pmc", pmc))
> +   if (request_irq(pmc->virq, pmc_irq_handler,
> +   IRQF_SHARED | IRQF_COND_SUSPEND, "pmc", pmc))
> goto out_remove_irqdomain;
>  
> return pmc;
> diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
> index 52d2041..69abb08 100644
> --- a/drivers/clk/at91/pmc.h
> +++ b/drivers/clk/at91/pmc.h
> @@ -33,6 +33,7 @@ struct at91_pmc {
> spinlock_t lock;
> const struct at91_pmc_caps *caps;
> struct irq_domain *irqdomain;
> +   u32 imr;
>  };
>  
>  static inline void pmc_lock(struct at91_pmc *pmc)
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] clk: divider: three exactness fixes (and a rant)

[Mike Turquette]

Quoting Stephen Boyd (2015-03-09 12:05:34)
> On 03/09/15 02:58, Philipp Zabel wrote:
> > Am Freitag, den 06.03.2015, 11:40 -0800 schrieb Stephen Boyd:
> >> On 03/06/15 11:28, Uwe Kleine-König wrote:
> >>> Hello Mike,
> >>>
> >>> On Fri, Mar 06, 2015 at 10:57:30AM -0800, Mike Turquette wrote:
> >>>> Quoting Uwe Kleine-König (2015-02-21 02:40:22)
> >>>>> Hello,
> >>>>>
> >>>>> TLDR: only apply patch 1 and rip of CLK_DIVIDER_ROUND_CLOSEST.
> >>>>>
> >>>>> I stared at clk-divider.c for some time now given Sascha's failing test
> >>>>> case. I found a fix for the failure (which happens to be what Sascha
> >>>>> suspected).
> >>>>>
> >>>>> The other two patches fix problems only present when handling dividers
> >>>>> that have CLK_DIVIDER_ROUND_CLOSEST set. Note that these are still
> >>>>> heavily broken however. So having a 4bit-divider and a parent clk of
> >>>>> 1 (as in Sascha's test case) requesting
> >>>>>
> >>>>> clk_set_rate(clk, 666)
> >>>>>
> >>>>> sets the rate to 625 (div=15) instead of 667 (div=16). The reason is the
> >>>>> choice of parent_rate in clk_divider_bestdiv's loop is wrong for
> >>>>> CLK_DIVIDER_ROUND_CLOSEST (with and without patch 1). A fix here is
> >>>>> non-trivial and for sure more than one rate must be tested here. This is
> >>>>> complicated by the fact that clk_round_rate might return a value bigger
> >>>>> than the requested rate which convinces me (once more) that it's a bad
> >>>>> idea to allow that. Even if this was fixed for .round_rate,
> >>>>> clk_divider_set_rate is still broken because it also uses
> >>>>>
> >>>>> div = DIV_ROUND_UP(parent_rate, rate);
> >>>>>
> >>>>> to calculate the (pretended) best divider to get near rate.
> >>>>>
> >>>>> Note this makes at least two reasons to remove support for
> >>>>> CLK_DIVIDER_ROUND_CLOSEST!
> >>>>>
> >>>>> Instead I'd favour creating a function
> >>>>>
> >>>>> clk_round_rate_nearest
> >>>>>
> >>>>> as was suggested some time ago by Soren Brinkmann and me[1] that doesn't
> >>>> Uwe,
> >>>>
> >>>> Thanks for the fixes. I'm thinking of taking all three for 4.0. I also
> >>>> agree on clk_round_rate_nearest (along with a _ceil and _floor version
> >>>> as well). That's something we can do for 4.1 probably.
> >>> I'd say that we make round_rate the _floor version. I guess in most
> >>> cases that already does the right thing. Also I think 4.1 is very
> >>> ambitious, so my suggestion for 4.1 is:
> >>>
> >>>  - add a WARN_ON_ONCE to clk_round_rate catching calls that return a
> >>>value bigger than requested.
> >>>  - implement clk_round_rate_nearest using clk_round_rate and the
> >>>assumption that it returns a value that is <= the requested rate.
> >>>I think without that there are too many special cases to handle and
> >>>probably not even a reliable way to determine the nearest rate.
> >>>  - while we're at it tightening the requirements for clk_round_rate
> >>>let's also specify the expected rounding. I'd vote for the
> >>>mathematical rounding, that is
> >>>
> >>> clk_round_rate(someclk, 333)
> >>>
> >>>explicitly is allowed to return a rate bigger than 333 as long as it
> >>>is less than 333.5.
> >>>
> >>> At one point while developing patch 1 I had the dividers fixed for the
> >>> rounding issue. I think I still have that patch somewhere so can post it
> >>> as RFC.
> >>>
> >> Why do we need clk_round_rate_nearest? We have rate constraints now so
> >> drivers should be moving towards requesting a rate that's within a
> >> tolerance they can accept if they even care to enforce anything like
> >> that. Eventually clk_round_rate() returning a value smaller or larger
> >> than what it's called with won't matter as long as what the
> >> implementation does fits within the constraints that consumers specify.
> >> It may even be possible to remove clk_

Re: [PATCH 0/3] clk: divider: three exactness fixes (and a rant)

[Mike Turquette]

Quoting Stephen Boyd (2015-03-09 12:05:34)
 On 03/09/15 02:58, Philipp Zabel wrote:
  Am Freitag, den 06.03.2015, 11:40 -0800 schrieb Stephen Boyd:
  On 03/06/15 11:28, Uwe Kleine-König wrote:
  Hello Mike,
 
  On Fri, Mar 06, 2015 at 10:57:30AM -0800, Mike Turquette wrote:
  Quoting Uwe Kleine-König (2015-02-21 02:40:22)
  Hello,
 
  TLDR: only apply patch 1 and rip of CLK_DIVIDER_ROUND_CLOSEST.
 
  I stared at clk-divider.c for some time now given Sascha's failing test
  case. I found a fix for the failure (which happens to be what Sascha
  suspected).
 
  The other two patches fix problems only present when handling dividers
  that have CLK_DIVIDER_ROUND_CLOSEST set. Note that these are still
  heavily broken however. So having a 4bit-divider and a parent clk of
  1 (as in Sascha's test case) requesting
 
  clk_set_rate(clk, 666)
 
  sets the rate to 625 (div=15) instead of 667 (div=16). The reason is the
  choice of parent_rate in clk_divider_bestdiv's loop is wrong for
  CLK_DIVIDER_ROUND_CLOSEST (with and without patch 1). A fix here is
  non-trivial and for sure more than one rate must be tested here. This is
  complicated by the fact that clk_round_rate might return a value bigger
  than the requested rate which convinces me (once more) that it's a bad
  idea to allow that. Even if this was fixed for .round_rate,
  clk_divider_set_rate is still broken because it also uses
 
  div = DIV_ROUND_UP(parent_rate, rate);
 
  to calculate the (pretended) best divider to get near rate.
 
  Note this makes at least two reasons to remove support for
  CLK_DIVIDER_ROUND_CLOSEST!
 
  Instead I'd favour creating a function
 
  clk_round_rate_nearest
 
  as was suggested some time ago by Soren Brinkmann and me[1] that doesn't
  Uwe,
 
  Thanks for the fixes. I'm thinking of taking all three for 4.0. I also
  agree on clk_round_rate_nearest (along with a _ceil and _floor version
  as well). That's something we can do for 4.1 probably.
  I'd say that we make round_rate the _floor version. I guess in most
  cases that already does the right thing. Also I think 4.1 is very
  ambitious, so my suggestion for 4.1 is:
 
   - add a WARN_ON_ONCE to clk_round_rate catching calls that return a
 value bigger than requested.
   - implement clk_round_rate_nearest using clk_round_rate and the
 assumption that it returns a value that is = the requested rate.
 I think without that there are too many special cases to handle and
 probably not even a reliable way to determine the nearest rate.
   - while we're at it tightening the requirements for clk_round_rate
 let's also specify the expected rounding. I'd vote for the
 mathematical rounding, that is
 
  clk_round_rate(someclk, 333)
 
 explicitly is allowed to return a rate bigger than 333 as long as it
 is less than 333.5.
 
  At one point while developing patch 1 I had the dividers fixed for the
  rounding issue. I think I still have that patch somewhere so can post it
  as RFC.
 
  Why do we need clk_round_rate_nearest? We have rate constraints now so
  drivers should be moving towards requesting a rate that's within a
  tolerance they can accept if they even care to enforce anything like
  that. Eventually clk_round_rate() returning a value smaller or larger
  than what it's called with won't matter as long as what the
  implementation does fits within the constraints that consumers specify.
  It may even be possible to remove clk_round_rate() as a consumer API.
  If I have to provide a panel pixel clock I usually want to get a rate as
  close as possible to the specified typical rate, but within the
  specified limits.
 
  Assume a panel with 70 MHz ideal pixel clock and a valid range of 60 MHz
  to 80 MHz. If the clock supply supports two frequencies within that
  range, 60 MHz and 72 MHz, I'd prefer 72 MHz to be chosen over 60 Mhz.
 
 
 
 Hm.. Maybe we should tweak the arguments to clk_set_range() to have a
 typical rate? So instead of the current API:
 
   int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned
 long max)
 
 we should have
 
  int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned
 long typical, unsigned long max)
 
 with the semantics that we'll set the rate within the min,max
 constraints and try to get the rate as close to the typical rate as
 possible? That would match quite a few datasheets out there that specify
 these triplets.

This suffers from the same problem that round_rate has today, which is
the question of rounding policy. When you say that we want to get as
close as possible, how do we decide between equivalent values? We need
to make a default policy, document it and stick to it. E.g:

clk_set_rate_range(clk, 100, 110, 120);

Let's say that round_rate gives us possible values of 108 and 112, both
of which are two Hz away from the typical value of 110Hz. One is not
closer than the other. Which do we choose? Let's figure out a sane
default

Re: [PATCH v2 4/6] clk: at91: implement suspend/resume for the PMC irqchip

[Mike Turquette]

Quoting Boris Brezillon (2015-03-02 01:18:16)
 The irq line used by the PMC block is shared with several peripherals
 including the init timer which is registering its handler with
 IRQF_NO_SUSPEND.
 Implement the appropriate suspend/resume callback for the PMC irqchip,
 and inform irq core that PMC irq handler can be safely called while
 the system is suspended by setting IRQF_COND_SUSPEND.
 
 Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com

Looks OK to me. I'm not picking it due to the dependency you listed in
the cover letter.

Regards,
Mike

 ---
  drivers/clk/at91/pmc.c | 20 +++-
  drivers/clk/at91/pmc.h |  1 +
  2 files changed, 20 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
 index f07c815..3f27d21 100644
 --- a/drivers/clk/at91/pmc.c
 +++ b/drivers/clk/at91/pmc.c
 @@ -89,12 +89,29 @@ static int pmc_irq_set_type(struct irq_data *d, unsigned 
 type)
 return 0;
  }
  
 +static void pmc_irq_suspend(struct irq_data *d)
 +{
 +   struct at91_pmc *pmc = irq_data_get_irq_chip_data(d);
 +
 +   pmc-imr = pmc_read(pmc, AT91_PMC_IMR);
 +   pmc_write(pmc, AT91_PMC_IDR, pmc-imr);
 +}
 +
 +static void pmc_irq_resume(struct irq_data *d)
 +{
 +   struct at91_pmc *pmc = irq_data_get_irq_chip_data(d);
 +
 +   pmc_write(pmc, AT91_PMC_IER, pmc-imr);
 +}
 +
  static struct irq_chip pmc_irq = {
 .name = PMC,
 .irq_disable = pmc_irq_mask,
 .irq_mask = pmc_irq_mask,
 .irq_unmask = pmc_irq_unmask,
 .irq_set_type = pmc_irq_set_type,
 +   .irq_suspend = pmc_irq_suspend,
 +   .irq_resume = pmc_irq_resume,
  };
  
  static struct lock_class_key pmc_lock_class;
 @@ -224,7 +241,8 @@ static struct at91_pmc *__init at91_pmc_init(struct 
 device_node *np,
 goto out_free_pmc;
  
 pmc_write(pmc, AT91_PMC_IDR, 0x);
 -   if (request_irq(pmc-virq, pmc_irq_handler, IRQF_SHARED, pmc, pmc))
 +   if (request_irq(pmc-virq, pmc_irq_handler,
 +   IRQF_SHARED | IRQF_COND_SUSPEND, pmc, pmc))
 goto out_remove_irqdomain;
  
 return pmc;
 diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
 index 52d2041..69abb08 100644
 --- a/drivers/clk/at91/pmc.h
 +++ b/drivers/clk/at91/pmc.h
 @@ -33,6 +33,7 @@ struct at91_pmc {
 spinlock_t lock;
 const struct at91_pmc_caps *caps;
 struct irq_domain *irqdomain;
 +   u32 imr;
  };
  
  static inline void pmc_lock(struct at91_pmc *pmc)
 -- 
 1.9.1
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 1/6] clk: add of_clk_get_parent_rate function

[Mike Turquette]

Quoting Ray Jui (2015-03-06 12:07:13)
> Hi Mike,
> 
> On 3/6/2015 11:55 AM, Mike Turquette wrote:
> > Quoting Sascha Hauer (2015-02-26 00:43:19)
> >> On Wed, Feb 25, 2015 at 11:42:44PM -0800, Ray Jui wrote:
> >>> On 2/25/2015 10:51 PM, Sascha Hauer wrote:
> >>>> On Wed, Feb 25, 2015 at 10:13:15PM -0800, Ray Jui wrote:
> >>>>> Hi Sascha,
> >>>>>
> >>>>> On 2/25/2015 9:54 PM, Sascha Hauer wrote:
> >>>>>> Hi Ray,
> >>>>>>
> >>>>>> On Wed, Feb 04, 2015 at 04:55:00PM -0800, Ray Jui wrote:
> >>>>>>> Sometimes a clock needs to know the rate of its parent before itself 
> >>>>>>> is
> >>>>>>> registered to the framework. An example is that a PLL may need to
> >>>>>>> initialize itself to a specific VCO frequency, before registering to 
> >>>>>>> the
> >>>>>>> framework. The parent rate needs to be known, for PLL multipliers and
> >>>>>>> divisors to be configured properly.
> >>>>>>>
> >>>>>>> Introduce helper function of_clk_get_parent_rate, which can be used to
> >>>>>>> obtain the parent rate of a clock, given a device node and index.
> >>>>>>
> >>>>>> I can't see how this patch helps you. First it's not guaranteed that
> >>>>>> the parent is already registered, what do you do in this case?
> >>>>>
> >>>>> In the case when clock parent is not found, as you can see from the
> >>>>> code, it simply returns zero, just like other clk get rate APIs.
> >>>>
> >>>> Yes, but what do you do with the 0 result then in your PLL 
> >>>> initialization?
> >>>>
> >>>
> >>> As of the current code, it fails the PLL frequency initialization and
> >>> bails out. Thinking about it more, it actually makes more sense to just
> >>> warn and still go ahead to register the clock, in which case it will use
> >>> whatever default frequency after chip power on reset or a frequency
> >>> configured in the bootloader.
> >>>
> >>>>>
> >>>>> I thought the order of clock registration is based on order of the clock
> >>>>> nodes in device tree. It makes sense to me to declare the parent clock
> >>>>> before a child clock, so it's guaranteed that the parent is registered
> >>>>> before the child.
> >>>>
> >>>> No, you can't rely on that. The order of the device nodes may happen to
> >>>> define the order of clock initialization now, but that may change.
> >>>> device nodes are usually ordered by bus addresses, not by intended
> >>>> initialization order. Even if you reorder them everything must still
> >>>> work.
> >>>>
> >>>
> >>> Okay I get your point that the order of device nodes may not be relied
> >>> on for device initialization order. But then another mechanism should be
> >>> deployed to give developers the option to decide on the clock
> >>> initialization sequence. It can be optional but it should be there.
> >>>
> >>>>>
> >>>>>> Then the clock framework doesn't require that you initialize the PLL
> >>>>>> before registering. That can be done in the clk ops later.
> >>>>>
> >>>>> Sure it's not mandatory. But what's wrong with me choosing to initialize
> >>>>> the PLL clock to a known frequency before registering it to the 
> >>>>> framework?
> >>>>
> >>>> Appearantly you don't know the (input) frequency of the PLL when
> >>>> registering it to the framework, so the question must be: What's wrong
> >>>> with keeping it uninitialized?
> >>>>
> >>>> If the PLL is unused then you don't care about it's initialization
> >>>> status. If it happens to be enabled by a bootloader and still unused
> >>>> at late_initcall time the clock framework will disable it so you
> >>>> have a known state then. If a consumer for the PLL appears it's its
> >>>> job to initialize it through the clk api.
> >>>>
> >>>> Sascha
> >>>>
> >>>
> >>> Okay, what we need here is to initialize th

Re: [PATCH v5 1/6] clk: add of_clk_get_parent_rate function

[Mike Turquette]

Quoting Sascha Hauer (2015-02-26 00:43:19)
> On Wed, Feb 25, 2015 at 11:42:44PM -0800, Ray Jui wrote:
> > On 2/25/2015 10:51 PM, Sascha Hauer wrote:
> > > On Wed, Feb 25, 2015 at 10:13:15PM -0800, Ray Jui wrote:
> > >> Hi Sascha,
> > >>
> > >> On 2/25/2015 9:54 PM, Sascha Hauer wrote:
> > >>> Hi Ray,
> > >>>
> > >>> On Wed, Feb 04, 2015 at 04:55:00PM -0800, Ray Jui wrote:
> >  Sometimes a clock needs to know the rate of its parent before itself is
> >  registered to the framework. An example is that a PLL may need to
> >  initialize itself to a specific VCO frequency, before registering to 
> >  the
> >  framework. The parent rate needs to be known, for PLL multipliers and
> >  divisors to be configured properly.
> > 
> >  Introduce helper function of_clk_get_parent_rate, which can be used to
> >  obtain the parent rate of a clock, given a device node and index.
> > >>>
> > >>> I can't see how this patch helps you. First it's not guaranteed that
> > >>> the parent is already registered, what do you do in this case?
> > >>
> > >> In the case when clock parent is not found, as you can see from the
> > >> code, it simply returns zero, just like other clk get rate APIs.
> > > 
> > > Yes, but what do you do with the 0 result then in your PLL initialization?
> > > 
> > 
> > As of the current code, it fails the PLL frequency initialization and
> > bails out. Thinking about it more, it actually makes more sense to just
> > warn and still go ahead to register the clock, in which case it will use
> > whatever default frequency after chip power on reset or a frequency
> > configured in the bootloader.
> > 
> > >>
> > >> I thought the order of clock registration is based on order of the clock
> > >> nodes in device tree. It makes sense to me to declare the parent clock
> > >> before a child clock, so it's guaranteed that the parent is registered
> > >> before the child.
> > > 
> > > No, you can't rely on that. The order of the device nodes may happen to
> > > define the order of clock initialization now, but that may change.
> > > device nodes are usually ordered by bus addresses, not by intended
> > > initialization order. Even if you reorder them everything must still
> > > work.
> > > 
> > 
> > Okay I get your point that the order of device nodes may not be relied
> > on for device initialization order. But then another mechanism should be
> > deployed to give developers the option to decide on the clock
> > initialization sequence. It can be optional but it should be there.
> > 
> > >>
> > >>> Then the clock framework doesn't require that you initialize the PLL
> > >>> before registering. That can be done in the clk ops later.
> > >>
> > >> Sure it's not mandatory. But what's wrong with me choosing to initialize
> > >> the PLL clock to a known frequency before registering it to the 
> > >> framework?
> > > 
> > > Appearantly you don't know the (input) frequency of the PLL when
> > > registering it to the framework, so the question must be: What's wrong
> > > with keeping it uninitialized?
> > > 
> > > If the PLL is unused then you don't care about it's initialization
> > > status. If it happens to be enabled by a bootloader and still unused
> > > at late_initcall time the clock framework will disable it so you
> > > have a known state then. If a consumer for the PLL appears it's its
> > > job to initialize it through the clk api.
> > > 
> > > Sascha
> > > 
> > 
> > Okay, what we need here is to initialize the PLL to a desired frequency,
> > based on device tree settings (since it will be configured differently,
> > among different boards). This is a PLL that 1) has limited options of
> > frequencies which it can be configured to, and 2) has multiple child
> > clocks, where is a more suitable place to initialize it to the desired
> > frequency than right before registering it to the framework? I know a
> > lot of people do it in the bootloader, but I thought we should be given
> > the flexibility of configuring it in the kernel.
> > 
> > When you say "consumers", do you mean 1) the device driver that uses the
> > PLL; or 2) the device driver that use the child clock of the PLL? If
> > it's case 1), then we don't really have a device driver that directly
> > uses the PLL, and I thought that's quite normal, as most PLLs don't
> > directly feed into any peripherals.
> 
> I meant 1) and 2). Before a consumer comes along the state of the PLL
> doesn't matter. When a consumer shows up it has to call
> clk_prepare_enable which (directly or indirectly) will enable your PLL.
> Then it's still time to apply the default settings you found out during
> probe of the PLL.

My review comments are really for iproc_pll_setup() in patch #3, but the
discussion is here so I'll respond to this thread.

I think the root of this problem is that your pll clk_ops does not
support .set_rate. That is why your clock driver hacks in a call to
pll_set_rate in iproc_pll_setup.

Due to the above shortcoming 

Re: [PATCH v3 0/4] clk: st: New always-on clock domain

[Mike Turquette]

Quoting Lee Jones (2015-03-04 04:00:03)
> Mike,
> 
> Do you want me to resend this set with Robert's Reviewed-by applied,
> or are you happy to apply it yourself?

No need for the resend. I am hoping for a final review from a DT human.

This approach looks fine to me. In practice I think it is restricted to
hardware blocks that don't exist in DT yet (e.g. no driver, in the case
of your interconnect) and that restriction is probably for the best.

Regards,
Mike

> 
> > v2 => v3:
> >   - Ensure DT actually reflects h/w
> > - i.e. Nodes should not contain a mishmash of different IP
> >   blocks, but should identify related h/w.  In the current
> >   example we use interconnects
> >   - Change naming from clkdomain to clk-always-on
> >   - Place "do not abuse" warning in documentation
> > 
> > v1 => v2:
> >   - Turned the ST specific driver into a generic one
> >   
> > Hardware can have a bunch of clocks which must not be turned off.
> > If drivers a) fail to obtain a reference to any of these or b) give
> > up a previously obtained reference during suspend, the common clk
> > framework will attempt to turn them off and the hardware will
> > subsequently die.  The only way to recover from this failure is to
> > restart.
> >   
> > To avoid either of these two scenarios from catastrophically
> > disabling the running system we have implemented a clock domain
> > where clocks are consumed and references are taken, thus preventing
> > them from being shut down by the framework.
> > 
> > *** BLURB HERE ***
> > 
> > Lee Jones (4):
> >   ARM: sti: stih407-family: Supply defines for CLOCKGEN A0
> >   ARM: sti: stih407-family: Add platform interconnects to always-on clk
> > domain
> >   clk: Provide an always-on clock domain framework
> >   clk: dt: Introduce always-on clock domain documentation
> > 
> >  .../devicetree/bindings/clock/clk-always-on.txt| 35 
> >  arch/arm/boot/dts/stih407-family.dtsi  | 15 ++
> >  drivers/clk/Makefile   |  1 +
> >  drivers/clk/clk-always-on.c| 62 
> > ++
> >  include/dt-bindings/clock/stih407-clks.h   |  4 ++
> >  5 files changed, 117 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/clock/clk-always-on.txt
> >  create mode 100644 drivers/clk/clk-always-on.c
> > 
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] clk: divider: three exactness fixes (and a rant)

[Mike Turquette]

Quoting Uwe Kleine-König (2015-02-21 02:40:22)
> Hello,
> 
> TLDR: only apply patch 1 and rip of CLK_DIVIDER_ROUND_CLOSEST.
> 
> I stared at clk-divider.c for some time now given Sascha's failing test
> case. I found a fix for the failure (which happens to be what Sascha
> suspected).
> 
> The other two patches fix problems only present when handling dividers
> that have CLK_DIVIDER_ROUND_CLOSEST set. Note that these are still
> heavily broken however. So having a 4bit-divider and a parent clk of
> 1 (as in Sascha's test case) requesting
> 
> clk_set_rate(clk, 666)
> 
> sets the rate to 625 (div=15) instead of 667 (div=16). The reason is the
> choice of parent_rate in clk_divider_bestdiv's loop is wrong for
> CLK_DIVIDER_ROUND_CLOSEST (with and without patch 1). A fix here is
> non-trivial and for sure more than one rate must be tested here. This is
> complicated by the fact that clk_round_rate might return a value bigger
> than the requested rate which convinces me (once more) that it's a bad
> idea to allow that. Even if this was fixed for .round_rate,
> clk_divider_set_rate is still broken because it also uses
> 
> div = DIV_ROUND_UP(parent_rate, rate);
> 
> to calculate the (pretended) best divider to get near rate.
> 
> Note this makes at least two reasons to remove support for
> CLK_DIVIDER_ROUND_CLOSEST!
> 
> Instead I'd favour creating a function
> 
> clk_round_rate_nearest
> 
> as was suggested some time ago by Soren Brinkmann and me[1] that doesn't

Uwe,

Thanks for the fixes. I'm thinking of taking all three for 4.0. I also
agree on clk_round_rate_nearest (along with a _ceil and _floor version
as well). That's something we can do for 4.1 probably.

There are currently 3 users of CLK_DIVIDER_ROUND_CLOSEST:

Loongson
QCOM
ST

So now is probably the right time to remove the flag if we're going to
do it.

What do you think?

Regards,
Mike

> need any clk type specific knowledge. This would mean that not the
> divider (or clk in general) would have to know that returning a slightly
> bigger rate than requested is OK but the caller which is fine (and even
> better) in my eyes. This would simplify clk-divider.c (and probably
> others) and give support for "nearest match" for all clock types without
> type specific implementation. (Note that it might even make sense to use
> a different metric for "nearest", instead of minimizing
> 
> abs(target - rate)
> 
> you might want to minimize
> 
> abs(target / rate - 1)
> 
> instead.
> 
> Converting the clk framework to 64 bit rates was discussed earlier
> already, too, and I wonder if we should fix rounding issues (a bit) in
> the same transition such that 
> 
> clk_set_rate(clk, 333)
> 
> allows the clk to be set to 333.33 Hz and let clk_get_rate
> return 333 in this case.
> 
> Also I'd vote to return 0 or -ESOMETHING if a requested rate is too low
> to be set. This would simplify some special casing I think and makes the
> request
> 
> clk_round_rate(clk, x) <= x
> 
> consistent.
> 
> Best regards
> Uwe
> 
> [1] https://lkml.org/lkml/2014/5/14/698
> 
> Uwe Kleine-König (3):
>   clk: divider: fix calculation of maximal parent rate for a given
> divider
>   clk: divider: fix selection of divider when rounding to closest
>   clk: divider: fix calculation of initial best divider when rounding to
> closest
> 
>  drivers/clk/clk-divider.c | 27 +--
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: divider: return real rate instead of divider value

[Mike Turquette]

Quoting Stephen Boyd (2015-02-25 16:13:15)
> On 02/24/15 02:39, Heiko Stübner wrote:
> > Commit bca9690b9426 ("clk: divider: Make generic for usage elsewhere")
> > returned only the divider value for read-only dividers instead of the
> > actual rate.
> >
> > Fixes: bca9690b9426 ("clk: divider: Make generic for usage elsewhere")
> > Signed-off-by: Heiko Stuebner 
> >
> 
> Oops. Thanks.
> 
> Acked-by: Stephen Boyd 

Applied to clk-fixes.

Regards,
Mike

> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 1/6] clk: add of_clk_get_parent_rate function

[Mike Turquette]

Quoting Ray Jui (2015-03-06 12:07:13)
 Hi Mike,
 
 On 3/6/2015 11:55 AM, Mike Turquette wrote:
  Quoting Sascha Hauer (2015-02-26 00:43:19)
  On Wed, Feb 25, 2015 at 11:42:44PM -0800, Ray Jui wrote:
  On 2/25/2015 10:51 PM, Sascha Hauer wrote:
  On Wed, Feb 25, 2015 at 10:13:15PM -0800, Ray Jui wrote:
  Hi Sascha,
 
  On 2/25/2015 9:54 PM, Sascha Hauer wrote:
  Hi Ray,
 
  On Wed, Feb 04, 2015 at 04:55:00PM -0800, Ray Jui wrote:
  Sometimes a clock needs to know the rate of its parent before itself 
  is
  registered to the framework. An example is that a PLL may need to
  initialize itself to a specific VCO frequency, before registering to 
  the
  framework. The parent rate needs to be known, for PLL multipliers and
  divisors to be configured properly.
 
  Introduce helper function of_clk_get_parent_rate, which can be used to
  obtain the parent rate of a clock, given a device node and index.
 
  I can't see how this patch helps you. First it's not guaranteed that
  the parent is already registered, what do you do in this case?
 
  In the case when clock parent is not found, as you can see from the
  code, it simply returns zero, just like other clk get rate APIs.
 
  Yes, but what do you do with the 0 result then in your PLL 
  initialization?
 
 
  As of the current code, it fails the PLL frequency initialization and
  bails out. Thinking about it more, it actually makes more sense to just
  warn and still go ahead to register the clock, in which case it will use
  whatever default frequency after chip power on reset or a frequency
  configured in the bootloader.
 
 
  I thought the order of clock registration is based on order of the clock
  nodes in device tree. It makes sense to me to declare the parent clock
  before a child clock, so it's guaranteed that the parent is registered
  before the child.
 
  No, you can't rely on that. The order of the device nodes may happen to
  define the order of clock initialization now, but that may change.
  device nodes are usually ordered by bus addresses, not by intended
  initialization order. Even if you reorder them everything must still
  work.
 
 
  Okay I get your point that the order of device nodes may not be relied
  on for device initialization order. But then another mechanism should be
  deployed to give developers the option to decide on the clock
  initialization sequence. It can be optional but it should be there.
 
 
  Then the clock framework doesn't require that you initialize the PLL
  before registering. That can be done in the clk ops later.
 
  Sure it's not mandatory. But what's wrong with me choosing to initialize
  the PLL clock to a known frequency before registering it to the 
  framework?
 
  Appearantly you don't know the (input) frequency of the PLL when
  registering it to the framework, so the question must be: What's wrong
  with keeping it uninitialized?
 
  If the PLL is unused then you don't care about it's initialization
  status. If it happens to be enabled by a bootloader and still unused
  at late_initcall time the clock framework will disable it so you
  have a known state then. If a consumer for the PLL appears it's its
  job to initialize it through the clk api.
 
  Sascha
 
 
  Okay, what we need here is to initialize the PLL to a desired frequency,
  based on device tree settings (since it will be configured differently,
  among different boards). This is a PLL that 1) has limited options of
  frequencies which it can be configured to, and 2) has multiple child
  clocks, where is a more suitable place to initialize it to the desired
  frequency than right before registering it to the framework? I know a
  lot of people do it in the bootloader, but I thought we should be given
  the flexibility of configuring it in the kernel.
 
  When you say consumers, do you mean 1) the device driver that uses the
  PLL; or 2) the device driver that use the child clock of the PLL? If
  it's case 1), then we don't really have a device driver that directly
  uses the PLL, and I thought that's quite normal, as most PLLs don't
  directly feed into any peripherals.
 
  I meant 1) and 2). Before a consumer comes along the state of the PLL
  doesn't matter. When a consumer shows up it has to call
  clk_prepare_enable which (directly or indirectly) will enable your PLL.
  Then it's still time to apply the default settings you found out during
  probe of the PLL.
  
  My review comments are really for iproc_pll_setup() in patch #3, but the
  discussion is here so I'll respond to this thread.
  
  I think the root of this problem is that your pll clk_ops does not
  support .set_rate. That is why your clock driver hacks in a call to
  pll_set_rate in iproc_pll_setup.
  
  Due to the above shortcoming you also do not use the assigned-clock-rate
  infrastructure to set your pll rate at registration-time. There is no
  reason for your driver to re-invent this logic. iproc_pll_setup is
  fetching the clock-frequency property from DT

Re: [PATCH] clk: divider: return real rate instead of divider value

[Mike Turquette]

Quoting Stephen Boyd (2015-02-25 16:13:15)
 On 02/24/15 02:39, Heiko Stübner wrote:
  Commit bca9690b9426 (clk: divider: Make generic for usage elsewhere)
  returned only the divider value for read-only dividers instead of the
  actual rate.
 
  Fixes: bca9690b9426 (clk: divider: Make generic for usage elsewhere)
  Signed-off-by: Heiko Stuebner he...@sntech.de
 
 
 Oops. Thanks.
 
 Acked-by: Stephen Boyd sb...@codeaurora.org

Applied to clk-fixes.

Regards,
Mike

 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] clk: divider: three exactness fixes (and a rant)

[Mike Turquette]

Quoting Uwe Kleine-König (2015-02-21 02:40:22)
 Hello,
 
 TLDR: only apply patch 1 and rip of CLK_DIVIDER_ROUND_CLOSEST.
 
 I stared at clk-divider.c for some time now given Sascha's failing test
 case. I found a fix for the failure (which happens to be what Sascha
 suspected).
 
 The other two patches fix problems only present when handling dividers
 that have CLK_DIVIDER_ROUND_CLOSEST set. Note that these are still
 heavily broken however. So having a 4bit-divider and a parent clk of
 1 (as in Sascha's test case) requesting
 
 clk_set_rate(clk, 666)
 
 sets the rate to 625 (div=15) instead of 667 (div=16). The reason is the
 choice of parent_rate in clk_divider_bestdiv's loop is wrong for
 CLK_DIVIDER_ROUND_CLOSEST (with and without patch 1). A fix here is
 non-trivial and for sure more than one rate must be tested here. This is
 complicated by the fact that clk_round_rate might return a value bigger
 than the requested rate which convinces me (once more) that it's a bad
 idea to allow that. Even if this was fixed for .round_rate,
 clk_divider_set_rate is still broken because it also uses
 
 div = DIV_ROUND_UP(parent_rate, rate);
 
 to calculate the (pretended) best divider to get near rate.
 
 Note this makes at least two reasons to remove support for
 CLK_DIVIDER_ROUND_CLOSEST!
 
 Instead I'd favour creating a function
 
 clk_round_rate_nearest
 
 as was suggested some time ago by Soren Brinkmann and me[1] that doesn't

Uwe,

Thanks for the fixes. I'm thinking of taking all three for 4.0. I also
agree on clk_round_rate_nearest (along with a _ceil and _floor version
as well). That's something we can do for 4.1 probably.

There are currently 3 users of CLK_DIVIDER_ROUND_CLOSEST:

Loongson
QCOM
ST

So now is probably the right time to remove the flag if we're going to
do it.

What do you think?

Regards,
Mike

 need any clk type specific knowledge. This would mean that not the
 divider (or clk in general) would have to know that returning a slightly
 bigger rate than requested is OK but the caller which is fine (and even
 better) in my eyes. This would simplify clk-divider.c (and probably
 others) and give support for nearest match for all clock types without
 type specific implementation. (Note that it might even make sense to use
 a different metric for nearest, instead of minimizing
 
 abs(target - rate)
 
 you might want to minimize
 
 abs(target / rate - 1)
 
 instead.
 
 Converting the clk framework to 64 bit rates was discussed earlier
 already, too, and I wonder if we should fix rounding issues (a bit) in
 the same transition such that 
 
 clk_set_rate(clk, 333)
 
 allows the clk to be set to 333.33 Hz and let clk_get_rate
 return 333 in this case.
 
 Also I'd vote to return 0 or -ESOMETHING if a requested rate is too low
 to be set. This would simplify some special casing I think and makes the
 request
 
 clk_round_rate(clk, x) = x
 
 consistent.
 
 Best regards
 Uwe
 
 [1] https://lkml.org/lkml/2014/5/14/698
 
 Uwe Kleine-König (3):
   clk: divider: fix calculation of maximal parent rate for a given
 divider
   clk: divider: fix selection of divider when rounding to closest
   clk: divider: fix calculation of initial best divider when rounding to
 closest
 
  drivers/clk/clk-divider.c | 27 +--
  1 file changed, 13 insertions(+), 14 deletions(-)
 
 -- 
 2.1.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 0/4] clk: st: New always-on clock domain

[Mike Turquette]

Quoting Lee Jones (2015-03-04 04:00:03)
 Mike,
 
 Do you want me to resend this set with Robert's Reviewed-by applied,
 or are you happy to apply it yourself?

No need for the resend. I am hoping for a final review from a DT human.

This approach looks fine to me. In practice I think it is restricted to
hardware blocks that don't exist in DT yet (e.g. no driver, in the case
of your interconnect) and that restriction is probably for the best.

Regards,
Mike

 
  v2 = v3:
- Ensure DT actually reflects h/w
  - i.e. Nodes should not contain a mishmash of different IP
blocks, but should identify related h/w.  In the current
example we use interconnects
- Change naming from clkdomain to clk-always-on
- Place do not abuse warning in documentation
  
  v1 = v2:
- Turned the ST specific driver into a generic one

  Hardware can have a bunch of clocks which must not be turned off.
  If drivers a) fail to obtain a reference to any of these or b) give
  up a previously obtained reference during suspend, the common clk
  framework will attempt to turn them off and the hardware will
  subsequently die.  The only way to recover from this failure is to
  restart.

  To avoid either of these two scenarios from catastrophically
  disabling the running system we have implemented a clock domain
  where clocks are consumed and references are taken, thus preventing
  them from being shut down by the framework.
  
  *** BLURB HERE ***
  
  Lee Jones (4):
ARM: sti: stih407-family: Supply defines for CLOCKGEN A0
ARM: sti: stih407-family: Add platform interconnects to always-on clk
  domain
clk: Provide an always-on clock domain framework
clk: dt: Introduce always-on clock domain documentation
  
   .../devicetree/bindings/clock/clk-always-on.txt| 35 
   arch/arm/boot/dts/stih407-family.dtsi  | 15 ++
   drivers/clk/Makefile   |  1 +
   drivers/clk/clk-always-on.c| 62 
  ++
   include/dt-bindings/clock/stih407-clks.h   |  4 ++
   5 files changed, 117 insertions(+)
   create mode 100644 
  Documentation/devicetree/bindings/clock/clk-always-on.txt
   create mode 100644 drivers/clk/clk-always-on.c
  
 
 -- 
 Lee Jones
 Linaro STMicroelectronics Landing Team Lead
 Linaro.org │ Open source software for ARM SoCs
 Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 1/6] clk: add of_clk_get_parent_rate function

[Mike Turquette]

Quoting Sascha Hauer (2015-02-26 00:43:19)
 On Wed, Feb 25, 2015 at 11:42:44PM -0800, Ray Jui wrote:
  On 2/25/2015 10:51 PM, Sascha Hauer wrote:
   On Wed, Feb 25, 2015 at 10:13:15PM -0800, Ray Jui wrote:
   Hi Sascha,
  
   On 2/25/2015 9:54 PM, Sascha Hauer wrote:
   Hi Ray,
  
   On Wed, Feb 04, 2015 at 04:55:00PM -0800, Ray Jui wrote:
   Sometimes a clock needs to know the rate of its parent before itself is
   registered to the framework. An example is that a PLL may need to
   initialize itself to a specific VCO frequency, before registering to 
   the
   framework. The parent rate needs to be known, for PLL multipliers and
   divisors to be configured properly.
  
   Introduce helper function of_clk_get_parent_rate, which can be used to
   obtain the parent rate of a clock, given a device node and index.
  
   I can't see how this patch helps you. First it's not guaranteed that
   the parent is already registered, what do you do in this case?
  
   In the case when clock parent is not found, as you can see from the
   code, it simply returns zero, just like other clk get rate APIs.
   
   Yes, but what do you do with the 0 result then in your PLL initialization?
   
  
  As of the current code, it fails the PLL frequency initialization and
  bails out. Thinking about it more, it actually makes more sense to just
  warn and still go ahead to register the clock, in which case it will use
  whatever default frequency after chip power on reset or a frequency
  configured in the bootloader.
  
  
   I thought the order of clock registration is based on order of the clock
   nodes in device tree. It makes sense to me to declare the parent clock
   before a child clock, so it's guaranteed that the parent is registered
   before the child.
   
   No, you can't rely on that. The order of the device nodes may happen to
   define the order of clock initialization now, but that may change.
   device nodes are usually ordered by bus addresses, not by intended
   initialization order. Even if you reorder them everything must still
   work.
   
  
  Okay I get your point that the order of device nodes may not be relied
  on for device initialization order. But then another mechanism should be
  deployed to give developers the option to decide on the clock
  initialization sequence. It can be optional but it should be there.
  
  
   Then the clock framework doesn't require that you initialize the PLL
   before registering. That can be done in the clk ops later.
  
   Sure it's not mandatory. But what's wrong with me choosing to initialize
   the PLL clock to a known frequency before registering it to the 
   framework?
   
   Appearantly you don't know the (input) frequency of the PLL when
   registering it to the framework, so the question must be: What's wrong
   with keeping it uninitialized?
   
   If the PLL is unused then you don't care about it's initialization
   status. If it happens to be enabled by a bootloader and still unused
   at late_initcall time the clock framework will disable it so you
   have a known state then. If a consumer for the PLL appears it's its
   job to initialize it through the clk api.
   
   Sascha
   
  
  Okay, what we need here is to initialize the PLL to a desired frequency,
  based on device tree settings (since it will be configured differently,
  among different boards). This is a PLL that 1) has limited options of
  frequencies which it can be configured to, and 2) has multiple child
  clocks, where is a more suitable place to initialize it to the desired
  frequency than right before registering it to the framework? I know a
  lot of people do it in the bootloader, but I thought we should be given
  the flexibility of configuring it in the kernel.
  
  When you say consumers, do you mean 1) the device driver that uses the
  PLL; or 2) the device driver that use the child clock of the PLL? If
  it's case 1), then we don't really have a device driver that directly
  uses the PLL, and I thought that's quite normal, as most PLLs don't
  directly feed into any peripherals.
 
 I meant 1) and 2). Before a consumer comes along the state of the PLL
 doesn't matter. When a consumer shows up it has to call
 clk_prepare_enable which (directly or indirectly) will enable your PLL.
 Then it's still time to apply the default settings you found out during
 probe of the PLL.

My review comments are really for iproc_pll_setup() in patch #3, but the
discussion is here so I'll respond to this thread.

I think the root of this problem is that your pll clk_ops does not
support .set_rate. That is why your clock driver hacks in a call to
pll_set_rate in iproc_pll_setup.

Due to the above shortcoming you also do not use the assigned-clock-rate
infrastructure to set your pll rate at registration-time. There is no
reason for your driver to re-invent this logic. iproc_pll_setup is
fetching the clock-frequency property from DT and then trying to set
that rate. Instead please use the generic code.

Re: linux-next: build failure after merge of the clk tree

[Mike Turquette]

Quoting Geert Uytterhoeven (2015-03-04 01:56:42)
> On Fri, Feb 27, 2015 at 6:42 PM, Stephen Boyd  wrote:
> > The problem is the patch was written before struct clk_core moved into
> > the clk.c file and then applied after it moved. So before the move the
> > order of includes would cause the struct definition to be before the
> > place where the tracepoint macros were expanded. The fix is to move the
> > tracepoint include after the struct clk_core definition:
> >
> > -8<
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 9aee501b8284..392477033990 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -22,9 +22,6 @@
> >  #include 
> >  #include 
> >
> > -#define CREATE_TRACE_POINTS
> > -#include 
> > -
> >  #include "clk.h"
> >
> >  static DEFINE_SPINLOCK(enable_lock);
> > @@ -80,6 +77,9 @@ struct clk_core {
> > struct kref ref;
> >  };
> >
> > +#define CREATE_TRACE_POINTS
> > +#include 
> > +
> >  struct clk {
> > struct clk_core *core;
> > const char *dev_id;
> 
> Acked-by: Geert Uytterhoeven 

Thanks, I've rolled this into the offending commit.

Regards,
Mike

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: build failure after merge of the clk tree

[Mike Turquette]

Quoting Geert Uytterhoeven (2015-03-04 01:56:42)
 On Fri, Feb 27, 2015 at 6:42 PM, Stephen Boyd sb...@codeaurora.org wrote:
  The problem is the patch was written before struct clk_core moved into
  the clk.c file and then applied after it moved. So before the move the
  order of includes would cause the struct definition to be before the
  place where the tracepoint macros were expanded. The fix is to move the
  tracepoint include after the struct clk_core definition:
 
  -8
 
  diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
  index 9aee501b8284..392477033990 100644
  --- a/drivers/clk/clk.c
  +++ b/drivers/clk/clk.c
  @@ -22,9 +22,6 @@
   #include linux/init.h
   #include linux/sched.h
 
  -#define CREATE_TRACE_POINTS
  -#include trace/events/clk.h
  -
   #include clk.h
 
   static DEFINE_SPINLOCK(enable_lock);
  @@ -80,6 +77,9 @@ struct clk_core {
  struct kref ref;
   };
 
  +#define CREATE_TRACE_POINTS
  +#include trace/events/clk.h
  +
   struct clk {
  struct clk_core *core;
  const char *dev_id;
 
 Acked-by: Geert Uytterhoeven geert+rene...@glider.be

Thanks, I've rolled this into the offending commit.

Regards,
Mike

 
 Gr{oetje,eeting}s,
 
 Geert
 
 --
 Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
 ge...@linux-m68k.org
 
 In personal conversations with technical people, I call myself a hacker. But
 when I'm talking to journalists I just say programmer or something like 
 that.
 -- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT PULL] clk/samsung: clk support for Exynos 5433 SoC

[Mike Turquette]

Quoting Chanwoo Choi (2015-02-27 16:52:59)
> Dear Mike and Sylwester,
> 
> Gently ping.

Hello,

I'll merge this into clk-next towards 4.1 later this week. v3.19 was
released on February 8, so this merge request came too late for
inclusion into 4.0.

Regards,
Mike

> 
> Best Regards,
> Chanwoo Choi
> 
> On Tue, Feb 24, 2015 at 8:48 AM, Chanwoo Choi  wrote:
> > Dear Mike and Sylwester,
> >
> > This pull-request was not merged on Linux 4.0-rc1.
> > Did you have any plan about it?
> >
> > Best Regards,
> > Chanwoo Choi
> >
> > On 02/06/2015 04:44 AM, Sylwester Nawrocki wrote:
> >> Hi Mike,
> >>
> >> This pull request includes driver for clock controller of the Exynos
> >> 5433 SoC.  As the hardware is quite complex, with many peripherals and
> >> corresponding clock management units the driver is rather huge.  I guess
> >> it will require a bit more cleanups than last time to balance lines
> >> introduced in this patch set... Please review and pull if it looks OK.
> >>
> >> The following changes since commit 
> >> e64fb42da4c6c713cfc7cad607e97e0773fa41ff:
> >>
> >>   clk: samsung: exynos4: Add divider clock id for memory bus frequency
> >> (2015-01-28 15:51:17 +0100)
> >>
> >> are available in the git repository at:
> >>
> >>   git://linuxtv.org/snawrocki/samsung.git tags/v3.20-exynos5433-clk
> >>
> >> for you to fetch changes up to b2f0e5f28e0686c0d5db238357a2e32555e97633:
> >>
> >>   clk: samsung: exynos5433: Move CLK_SCLK_HDMI_SPDIF_DISP clock to CMU_TOP
> >> domain (2015-02-05 19:31:09 +0100)
> >>
> >> 
> >> Clock controller driver for Exynos 5433 SoC.
> >>
> >> 
> >> Chanwoo Choi (22):
> >>   clk: samsung: exynos5433: Add binding document for Exynos5433 clock 
> >> domains
> >>   clk: samsung: exynos5433: Add clocks using common clock framework
> >>   clk: samsung: exynos5433: Add MUX clocks of CMU_TOP domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_PERIC domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_PERIS domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_G2D domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_MIF domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_DISP domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_AUD domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_BUS{0|1|2} domains
> >>   clk: samsung: exynos5433: Add missing clocks for CMU_FSYS domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_G3D domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_GSCL domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_APOLLO domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_ATLAS domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_MSCL domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_MFC domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_HEVC domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_ISP domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_CAM0 domain
> >>   clk: samsung: exynos5433: Add clocks for CMU_CAM1 domain
> >>   clk: samsung: exynos5433: Move CLK_SCLK_HDMI_SPDIF_DISP clock to 
> >> CMU_TOP domain
> >>
> >> Inha Song (1):
> >>   clk: samsung: Add CLKOUT driver support for Exynos5433 SoC
> >>
> >>  .../devicetree/bindings/clock/exynos5433-clock.txt |  462 ++
> >>  drivers/clk/samsung/Makefile   |1 +
> >>  drivers/clk/samsung/clk-exynos-clkout.c|2 +
> >>  drivers/clk/samsung/clk-exynos5433.c   | 5423 
> >> 
> >>  include/dt-bindings/clock/exynos5433.h | 1403 +
> >>  5 files changed, 7291 insertions(+)
> >>  create mode 100644 
> >> Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> >>  create mode 100644 drivers/clk/samsung/clk-exynos5433.c
> >>  create mode 100644 include/dt-bindings/clock/exynos5433.h
> >>
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe 
> > linux-samsung-soc" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT PULL] clk/samsung: clk support for Exynos 5433 SoC

[Mike Turquette]

Quoting Chanwoo Choi (2015-02-27 16:52:59)
 Dear Mike and Sylwester,
 
 Gently ping.

Hello,

I'll merge this into clk-next towards 4.1 later this week. v3.19 was
released on February 8, so this merge request came too late for
inclusion into 4.0.

Regards,
Mike

 
 Best Regards,
 Chanwoo Choi
 
 On Tue, Feb 24, 2015 at 8:48 AM, Chanwoo Choi cw00.c...@samsung.com wrote:
  Dear Mike and Sylwester,
 
  This pull-request was not merged on Linux 4.0-rc1.
  Did you have any plan about it?
 
  Best Regards,
  Chanwoo Choi
 
  On 02/06/2015 04:44 AM, Sylwester Nawrocki wrote:
  Hi Mike,
 
  This pull request includes driver for clock controller of the Exynos
  5433 SoC.  As the hardware is quite complex, with many peripherals and
  corresponding clock management units the driver is rather huge.  I guess
  it will require a bit more cleanups than last time to balance lines
  introduced in this patch set... Please review and pull if it looks OK.
 
  The following changes since commit 
  e64fb42da4c6c713cfc7cad607e97e0773fa41ff:
 
clk: samsung: exynos4: Add divider clock id for memory bus frequency
  (2015-01-28 15:51:17 +0100)
 
  are available in the git repository at:
 
git://linuxtv.org/snawrocki/samsung.git tags/v3.20-exynos5433-clk
 
  for you to fetch changes up to b2f0e5f28e0686c0d5db238357a2e32555e97633:
 
clk: samsung: exynos5433: Move CLK_SCLK_HDMI_SPDIF_DISP clock to CMU_TOP
  domain (2015-02-05 19:31:09 +0100)
 
  
  Clock controller driver for Exynos 5433 SoC.
 
  
  Chanwoo Choi (22):
clk: samsung: exynos5433: Add binding document for Exynos5433 clock 
  domains
clk: samsung: exynos5433: Add clocks using common clock framework
clk: samsung: exynos5433: Add MUX clocks of CMU_TOP domain
clk: samsung: exynos5433: Add clocks for CMU_PERIC domain
clk: samsung: exynos5433: Add clocks for CMU_PERIS domain
clk: samsung: exynos5433: Add clocks for CMU_G2D domain
clk: samsung: exynos5433: Add clocks for CMU_MIF domain
clk: samsung: exynos5433: Add clocks for CMU_DISP domain
clk: samsung: exynos5433: Add clocks for CMU_AUD domain
clk: samsung: exynos5433: Add clocks for CMU_BUS{0|1|2} domains
clk: samsung: exynos5433: Add missing clocks for CMU_FSYS domain
clk: samsung: exynos5433: Add clocks for CMU_G3D domain
clk: samsung: exynos5433: Add clocks for CMU_GSCL domain
clk: samsung: exynos5433: Add clocks for CMU_APOLLO domain
clk: samsung: exynos5433: Add clocks for CMU_ATLAS domain
clk: samsung: exynos5433: Add clocks for CMU_MSCL domain
clk: samsung: exynos5433: Add clocks for CMU_MFC domain
clk: samsung: exynos5433: Add clocks for CMU_HEVC domain
clk: samsung: exynos5433: Add clocks for CMU_ISP domain
clk: samsung: exynos5433: Add clocks for CMU_CAM0 domain
clk: samsung: exynos5433: Add clocks for CMU_CAM1 domain
clk: samsung: exynos5433: Move CLK_SCLK_HDMI_SPDIF_DISP clock to 
  CMU_TOP domain
 
  Inha Song (1):
clk: samsung: Add CLKOUT driver support for Exynos5433 SoC
 
   .../devicetree/bindings/clock/exynos5433-clock.txt |  462 ++
   drivers/clk/samsung/Makefile   |1 +
   drivers/clk/samsung/clk-exynos-clkout.c|2 +
   drivers/clk/samsung/clk-exynos5433.c   | 5423 
  
   include/dt-bindings/clock/exynos5433.h | 1403 +
   5 files changed, 7291 insertions(+)
   create mode 100644 
  Documentation/devicetree/bindings/clock/exynos5433-clock.txt
   create mode 100644 drivers/clk/samsung/clk-exynos5433.c
   create mode 100644 include/dt-bindings/clock/exynos5433.h
 
 
  --
  To unsubscribe from this list: send the line unsubscribe 
  linux-samsung-soc in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: qcom: Fix i2c frequency table

[Mike Turquette]

Quoting Stephen Boyd (2015-02-23 13:30:28)
> PXO is 25MHz, not 27MHz. Fix the table.
> 
> Fixes: 24d8fba44af3 "clk: qcom: Add support for IPQ8064's global
> clock controller (GCC)"
> 
> Signed-off-by: Stephen Boyd 

Applied to clk-next.

Regards,
Mike

> ---
>  drivers/clk/qcom/gcc-ipq806x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> index cbdc31dea7f4..a015bb06c09b 100644
> --- a/drivers/clk/qcom/gcc-ipq806x.c
> +++ b/drivers/clk/qcom/gcc-ipq806x.c
> @@ -525,8 +525,8 @@ static struct freq_tbl clk_tbl_gsbi_qup[] = {
> { 1080, P_PXO,  1, 2,  5 },
> { 1506, P_PLL8, 1, 2, 51 },
> { 2400, P_PLL8, 4, 1,  4 },
> +   { 2500, P_PXO,  1, 0,  0 },
> { 2560, P_PLL8, 1, 1, 15 },
> -   { 2700, P_PXO,  1, 0,  0 },
> { 4800, P_PLL8, 4, 1,  2 },
> { 5120, P_PLL8, 1, 2, 15 },
> { }
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: build failure after merge of the clk tree

[Mike Turquette]

On Thu, Feb 26, 2015 at 1:09 AM, Krzysztof Kozlowski
 wrote:
> On czw, 2015-02-26 at 13:14 +1100, Stephen Rothwell wrote:
>> Hi Mike,
>>
>> After merging the clk tree, today's linux-next build (x86_64 allmodconfig)
>> failed like this:
>>
>> drivers/clk/clk.c: In function 'clk_disable_unused_subtree':
>> drivers/clk/clk.c:514:3: error: label 'out' used but not defined
>>goto out;
>>^
>>
>> Caused by commit a2146f032294 ("clk: Use lockdep asserts to find
>> missing hold of prepare_lock").  Commit c440525cb967 ("clk: Remove
>> unneeded NULL checks") removed that label along with the NULL check
>> that a2146f032294 reintroduces (was this a bad rebase?).  Please do
>> simple build tests.
>>
>> I have used the clk tree from next-20150225 for today.
>
> Mike,
>
> It seems that my patch did not applied cleanly and the merge introduced
> such artifacts. My patch adds only lockdep_asserts and does not
> influence the program flow. I can rebase and resend the patch if you
> wish.

It's OK. I fixed it up locally the same day that I applied it but
didn't push the change out to my mirror in time to catch -next. It
will be fixed the next time Stephen pulls the clk tree.

Regards,
Mike

>
> Best regards,
> Krzysztof
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: build failure after merge of the clk tree

[Mike Turquette]

On Thu, Feb 26, 2015 at 1:09 AM, Krzysztof Kozlowski
k.kozlow...@samsung.com wrote:
 On czw, 2015-02-26 at 13:14 +1100, Stephen Rothwell wrote:
 Hi Mike,

 After merging the clk tree, today's linux-next build (x86_64 allmodconfig)
 failed like this:

 drivers/clk/clk.c: In function 'clk_disable_unused_subtree':
 drivers/clk/clk.c:514:3: error: label 'out' used but not defined
goto out;
^

 Caused by commit a2146f032294 (clk: Use lockdep asserts to find
 missing hold of prepare_lock).  Commit c440525cb967 (clk: Remove
 unneeded NULL checks) removed that label along with the NULL check
 that a2146f032294 reintroduces (was this a bad rebase?).  Please do
 simple build tests.

 I have used the clk tree from next-20150225 for today.

 Mike,

 It seems that my patch did not applied cleanly and the merge introduced
 such artifacts. My patch adds only lockdep_asserts and does not
 influence the program flow. I can rebase and resend the patch if you
 wish.

It's OK. I fixed it up locally the same day that I applied it but
didn't push the change out to my mirror in time to catch -next. It
will be fixed the next time Stephen pulls the clk tree.

Regards,
Mike


 Best regards,
 Krzysztof

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: qcom: Fix i2c frequency table

[Mike Turquette]

Quoting Stephen Boyd (2015-02-23 13:30:28)
 PXO is 25MHz, not 27MHz. Fix the table.
 
 Fixes: 24d8fba44af3 clk: qcom: Add support for IPQ8064's global
 clock controller (GCC)
 
 Signed-off-by: Stephen Boyd sb...@codeaurora.org

Applied to clk-next.

Regards,
Mike

 ---
  drivers/clk/qcom/gcc-ipq806x.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
 index cbdc31dea7f4..a015bb06c09b 100644
 --- a/drivers/clk/qcom/gcc-ipq806x.c
 +++ b/drivers/clk/qcom/gcc-ipq806x.c
 @@ -525,8 +525,8 @@ static struct freq_tbl clk_tbl_gsbi_qup[] = {
 { 1080, P_PXO,  1, 2,  5 },
 { 1506, P_PLL8, 1, 2, 51 },
 { 2400, P_PLL8, 4, 1,  4 },
 +   { 2500, P_PXO,  1, 0,  0 },
 { 2560, P_PLL8, 1, 1, 15 },
 -   { 2700, P_PXO,  1, 0,  0 },
 { 4800, P_PLL8, 4, 1,  2 },
 { 5120, P_PLL8, 1, 2, 15 },
 { }
 -- 
 The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: clk_set_parent() with current parent shouldn't fail

[Mike Turquette]

Quoting Philipp Zabel (2015-02-03 01:42:34)
> Am Montag, den 02.02.2015, 14:11 -0800 schrieb Stephen Boyd:
> > If a driver calls clk_set_parent(clk, parent) and parent is the
> > current parent of clk we shouldn't fail in any case.
> > Unfortunately if clk is a read-only mux we return -ENOSYS
> > because we think we can't change the parent, except for in this
> > special case where we don't actually need to change the parent at
> > all. Return 0 in such a situation.
> > 
> > Cc: Philipp Zabel 
> > Signed-off-by: Stephen Boyd 
> 
> Acked-by: Philipp Zabel 

Applied to clk-next.

Regards,
Mike

> 
> regards
> Philipp
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] clk: Add tracepoints for hardware operations

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 14:37:41)
> It's useful to have tracepoints around operations that change the
> hardware state so that we can debug clock hardware performance
> and operations. Four basic types of events are supported: on/off
> events for enable, disable, prepare, unprepare that only record
> an event and a clock name, rate changing events for
> clk_set_{min_,max_}rate{_range}(), phase changing events for
> clk_set_phase() and parent changing events for clk_set_parent().
> 
> Cc: Steven Rostedt 
> Signed-off-by: Stephen Boyd 

Applied to clk-next.

Thanks,
Mike

> ---
> 
> Changes since v1:
>  * Fixed missing trace_*_complete() tracepoints
> 
> Note this is based on another patch titled "clk: Missing
> set_phase op is an error"
> 
>  drivers/clk/clk.c  |  56 ++---
>  include/trace/events/clk.h | 198 
> +
>  2 files changed, 244 insertions(+), 10 deletions(-)
>  create mode 100644 include/trace/events/clk.h
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 670c8c86ce9f..3ffadce9a0c2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -22,6 +22,9 @@
>  #include 
>  #include 
>  
> +#define CREATE_TRACE_POINTS
> +#include 
> +
>  #include "clk.h"
>  
>  static DEFINE_SPINLOCK(enable_lock);
> @@ -448,10 +451,12 @@ static void clk_unprepare_unused_subtree(struct 
> clk_core *clk)
> return;
>  
> if (clk_core_is_prepared(clk)) {
> +   trace_clk_unprepare(clk);
> if (clk->ops->unprepare_unused)
> clk->ops->unprepare_unused(clk->hw);
> else if (clk->ops->unprepare)
> clk->ops->unprepare(clk->hw);
> +   trace_clk_unprepare_complete(clk);
> }
>  }
>  
> @@ -478,10 +483,12 @@ static void clk_disable_unused_subtree(struct clk_core 
> *clk)
>  * back to .disable
>  */
> if (clk_core_is_enabled(clk)) {
> +   trace_clk_disable(clk);
> if (clk->ops->disable_unused)
> clk->ops->disable_unused(clk->hw);
> else if (clk->ops->disable)
> clk->ops->disable(clk->hw);
> +   trace_clk_disable_complete(clk);
> }
>  
>  unlock_out:
> @@ -861,9 +868,12 @@ static void clk_core_unprepare(struct clk_core *clk)
>  
> WARN_ON(clk->enable_count > 0);
>  
> +   trace_clk_unprepare(clk);
> +
> if (clk->ops->unprepare)
> clk->ops->unprepare(clk->hw);
>  
> +   trace_clk_unprepare_complete(clk);
> clk_core_unprepare(clk->parent);
>  }
>  
> @@ -901,12 +911,16 @@ static int clk_core_prepare(struct clk_core *clk)
> if (ret)
> return ret;
>  
> -   if (clk->ops->prepare) {
> +   trace_clk_prepare(clk);
> +
> +   if (clk->ops->prepare)
> ret = clk->ops->prepare(clk->hw);
> -   if (ret) {
> -   clk_core_unprepare(clk->parent);
> -   return ret;
> -   }
> +
> +   trace_clk_prepare_complete(clk);
> +
> +   if (ret) {
> +   clk_core_unprepare(clk->parent);
> +   return ret;
> }
> }
>  
> @@ -953,9 +967,13 @@ static void clk_core_disable(struct clk_core *clk)
> if (--clk->enable_count > 0)
> return;
>  
> +   trace_clk_disable(clk);
> +
> if (clk->ops->disable)
> clk->ops->disable(clk->hw);
>  
> +   trace_clk_disable_complete(clk);
> +
> clk_core_disable(clk->parent);
>  }
>  
> @@ -1008,12 +1026,16 @@ static int clk_core_enable(struct clk_core *clk)
> if (ret)
> return ret;
>  
> -   if (clk->ops->enable) {
> +   trace_clk_enable(clk);
> +
> +   if (clk->ops->enable)
> ret = clk->ops->enable(clk->hw);
> -   if (ret) {
> -   clk_core_disable(clk->parent);
> -   return ret;
> -   }
> +
> +   trace_clk_enable_complete(clk);
> +
> +   if (ret) {
> +   clk_core_disable(clk->parent);
> +   return ret;
> }
> }
>  
> @@ -1438,10 +1460,14 @@ static int __clk_set_parent(struct clk_core *clk, 
> struct clk_core *parent,
>  
> old_parent = __clk_set_parent_before(clk, parent);
>  
> +   trace_clk_set_parent(clk, parent);
> +
> /* change clock input source */
> if (parent && clk->ops->set_parent)
> ret = clk->ops->set_parent(clk->hw, p_index);
>  
> +   trace_clk_set_parent_complete(clk, parent);
> +
> if (ret) {
> flags = clk_enable_lock();
>   

Re: [PATCH] clk: Missing set_phase op is an error

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 14:09:43)
> If a clock's clk_ops doesn't have the set_phase op set we should
> return an error from clk_set_phase(). This way clock consumers
> know that when they tried to set a phase it didn't work, as
> opposed to the current behavior where the return value is 0
> meaning success.
> 
> Signed-off-by: Stephen Boyd 

Applied to clk-next.

Thanks,
Mike

> ---
>  drivers/clk/clk.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index a29daf9edea4..b82714a84f5e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2069,10 +2069,10 @@ EXPORT_SYMBOL_GPL(clk_set_parent);
>   */
>  int clk_set_phase(struct clk *clk, int degrees)
>  {
> -   int ret = 0;
> +   int ret = -EINVAL;
>  
> if (!clk)
> -   goto out;
> +   return 0;
>  
> /* sanity check degrees */
> degrees %= 360;
> @@ -2081,18 +2081,14 @@ int clk_set_phase(struct clk *clk, int degrees)
>  
> clk_prepare_lock();
>  
> -   if (!clk->core->ops->set_phase)
> -   goto out_unlock;
> -
> -   ret = clk->core->ops->set_phase(clk->core->hw, degrees);
> +   if (clk->core->ops->set_phase)
> +   ret = clk->core->ops->set_phase(clk->core->hw, degrees);
>  
> if (!ret)
> clk->core->phase = degrees;
>  
> -out_unlock:
> clk_prepare_unlock();
>  
> -out:
> return ret;
>  }
>  EXPORT_SYMBOL_GPL(clk_set_phase);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: fractional-divider: support for divider bypassing

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 11:42:55)
> On 02/02/15 05:37, Heikki Krogerus wrote:
> > If the divider or multiplier values values are 0 in the
> 
> s/values//
> 
> > register, bypassing the divider and returning the parent
> > clock rate in clk_fd_recalc_rate().
> >
> > Signed-off-by: Heikki Krogerus 
> > ---
> 
> Reviewed-by: Stephen Boyd 

Applied to clk-next.

Regards,
Mike

> 
> >  drivers/clk/clk-fractional-divider.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/clk/clk-fractional-divider.c 
> > b/drivers/clk/clk-fractional-divider.c
> > index dc91da7..34d6c51 100644
> > --- a/drivers/clk/clk-fractional-divider.c
> > +++ b/drivers/clk/clk-fractional-divider.c
> > @@ -36,6 +36,9 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
> >   m = (val & fd->mmask) >> fd->mshift;
> >   n = (val & fd->nmask) >> fd->nshift;
> >  
> > + if (!n || !m)
> > + return parent_rate;
> > +
> >   ret = (u64)parent_rate * m;
> >   do_div(ret, n);
> >  
> 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: fractional-divider: support for divider bypassing

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 11:42:55)
> On 02/02/15 05:37, Heikki Krogerus wrote:
> > If the divider or multiplier values values are 0 in the
> 
> s/values//
> 
> > register, bypassing the divider and returning the parent
> > clock rate in clk_fd_recalc_rate().
> >
> > Signed-off-by: Heikki Krogerus 
> > ---
> 
> Reviewed-by: Stephen Boyd 

Applied to clk-next.

Regards,
Mike

> 
> >  drivers/clk/clk-fractional-divider.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/clk/clk-fractional-divider.c 
> > b/drivers/clk/clk-fractional-divider.c
> > index dc91da7..34d6c51 100644
> > --- a/drivers/clk/clk-fractional-divider.c
> > +++ b/drivers/clk/clk-fractional-divider.c
> > @@ -36,6 +36,9 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
> >   m = (val & fd->mmask) >> fd->mshift;
> >   n = (val & fd->nmask) >> fd->nshift;
> >  
> > + if (!n || !m)
> > + return parent_rate;
> > +
> >   ret = (u64)parent_rate * m;
> >   do_div(ret, n);
> >  
> 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] MSM8960 LCC fixes

[Mike Turquette]

Quoting Stephen Boyd (2015-01-29 15:38:10)
> A couple of small fixes found while testing the audio clock control
> on apq8064.

Applied to clk-fixes.

Regards,
Mike

> 
> Stephen Boyd (3):
>   clk: qcom: lcc-msm8960: Fix slimbus n and m val offsets
>   clk: qcom: lcc-msm8960: Fix PLL rate detection
>   clk: qcom: Add PLL4 vote clock
> 
>  drivers/clk/qcom/gcc-msm8960.c | 13 +
>  drivers/clk/qcom/lcc-msm8960.c |  6 +++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: qcom: fix platform_no_drv_owner.cocci warnings

[Mike Turquette]

Quoting Stephen Boyd (2015-01-28 12:49:32)
> On 01/27/15 23:00, kbuild test robot wrote:
> > drivers/clk/qcom/lcc-ipq806x.c:465:3-8: No need to set .owner here. The 
> > core will do it.
> >
> >  Remove .owner field if calls are used which set it automatically
> >
> > Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
> >
> > CC: Rajendra Nayak 
> > Signed-off-by: Fengguang Wu 
> 
> Reviewed-by: Stephen Boyd 

Applied.

Regards,
Mike

> 
> Thanks. This code was reposted from a time when this wasn't done
> automatically.
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: qcom: fix platform_no_drv_owner.cocci warnings

[Mike Turquette]

Quoting Stephen Boyd (2015-01-28 12:51:05)
> On 01/27/15 23:11, kbuild test robot wrote:
> > drivers/clk/qcom/lcc-msm8960.c:577:3-8: No need to set .owner here. The 
> > core will do it.
> >
> >  Remove .owner field if calls are used which set it automatically
> >
> > Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
> >
> > CC: Stephen Boyd 
> > Signed-off-by: Fengguang Wu 
> > ---
> 
> Reviewed-by: Stephen Boyd 

Applied.

Regards,
Mike

> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC 1/3] i2c: Enhancement of i2c API to address circular lock dependency problem

[Mike Turquette]

Quoting Mark Brown (2015-01-18 05:41:24)
> On Sun, Jan 18, 2015 at 11:54:56AM +0100, Krzysztof Kozlowski wrote:
> > W dniu 18.01.2015 o 07:30, Tomasz Figa pisze:
> 
> > >So, the question is, do we actually have hardware that _really_
> > >requires _actual_ preparation or all the clk_prepare_enable()s in I2C
> > >drivers (at least in i2c-s3c2410) are just to simplify the code?
> 
> > I completely forgot that you already thought about this deadlock in 2014. I
> > think we can try the no-prepare way for i2c-s3c2410. However this would be
> > only workaround for specific chip. Other buses (like SPI) would require
> > similar changes.
> 
> Right, and it's every single driver which would need an update too which
> is a bit icky and sad.  On the other hand a more detailed fix is going
> to involve trying to make the clock API locking more fine grained which
> isn't fun.

Not fun is right. Please see Stephen's attempt here:

http://lkml.kernel.org/r/<1409792466-5092-1-git-send-email-sb...@codeaurora.org>

I'm hoping this approach will be revisited soon.

Regards,
Mike

> 
> > I wondered why this was not observed (at least not observed by me with
> > lockdep) on Gear 2 (Rinato) board. This is quite similar case: the S2MPS14
> > PMIC provides regulators and 32kHz clocks. I think it is exactly the same
> > pattern as for max77686... but somehow lockdep never reported that deadlock
> > there.
> 
> Mostly the clocks on PMICs never get changed at runtime which helps a
> lot here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: Use lockdep asserts to find missing hold of prepare_lock

[Mike Turquette]

Quoting Krzysztof Kozlowski (2015-01-09 00:28:10)
> Add lockdep asserts for holding the prepare_lock to all functions
> marking this as a requirement in description. Add this to private and
> exported functions so all locking misuse could be detected during
> debugging.
> 
> Signed-off-by: Krzysztof Kozlowski 

Applied.

Thanks,
Mike

> ---
>  drivers/clk/clk.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index f4963b7d4e17..cdbfaa34ace4 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -432,6 +432,8 @@ static void clk_unprepare_unused_subtree(struct clk *clk)
>  {
> struct clk *child;
>  
> +   lockdep_assert_held(_lock);
> +
> if (!clk)
> return;
>  
> @@ -458,6 +460,8 @@ static void clk_disable_unused_subtree(struct clk *clk)
> struct clk *child;
> unsigned long flags;
>  
> +   lockdep_assert_held(_lock);
> +
> if (!clk)
> goto out;
>  
> @@ -947,6 +951,8 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned 
> long rate)
> struct clk *parent;
> struct clk_hw *parent_hw;
>  
> +   lockdep_assert_held(_lock);
> +
> if (!clk)
> return 0;
>  
> @@ -1040,6 +1046,8 @@ static void __clk_recalc_accuracies(struct clk *clk)
> unsigned long parent_accuracy = 0;
> struct clk *child;
>  
> +   lockdep_assert_held(_lock);
> +
> if (clk->parent)
> parent_accuracy = clk->parent->accuracy;
>  
> @@ -1104,6 +1112,8 @@ static void __clk_recalc_rates(struct clk *clk, 
> unsigned long msg)
> unsigned long parent_rate = 0;
> struct clk *child;
>  
> +   lockdep_assert_held(_lock);
> +
> old_rate = clk->rate;
>  
> if (clk->parent)
> @@ -1297,6 +1307,8 @@ static int __clk_speculate_rates(struct clk *clk, 
> unsigned long parent_rate)
> unsigned long new_rate;
> int ret = NOTIFY_DONE;
>  
> +   lockdep_assert_held(_lock);
> +
> new_rate = clk_recalc(clk, parent_rate);
>  
> /* abort rate change if a driver returns NOTIFY_BAD or NOTIFY_STOP */
> @@ -2110,6 +2122,8 @@ static void __clk_release(struct kref *ref)
> struct clk *clk = container_of(ref, struct clk, ref);
> int i = clk->num_parents;
>  
> +   lockdep_assert_held(_lock);
> +
> kfree(clk->parents);
> while (--i >= 0)
> kfree(clk->parent_names[i]);
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/4] clk: Provide an always-on clock domain framework

[Mike Turquette]

Quoting Lee Jones (2015-02-25 07:48:08)
> On Wed, 25 Feb 2015, Rob Herring wrote:
> 
> > On Mon, Feb 23, 2015 at 11:23 AM, Mike Turquette  
> > wrote:
> > > Quoting Lee Jones (2015-02-18 08:15:00)
> > >> Much h/w contain clocks which if turned off would prove fatal.  The
> > >> only way to recover is to restart the board(s).  This driver takes
> > >> references to clocks which are required to be always-on in order to
> > >> prevent the common clk framework from trying to turn them off during
> > >> the clk_disabled_unused() procedure.
> > 
> > [...]
> > 
> > >> +static int ao_clock_domain_probe(struct platform_device *pdev)
> > >> +{
> > >> +   struct device_node *np = pdev->dev.of_node;
> > >> +   int nclks, i;
> > >> +
> > >> +   nclks = of_count_phandle_with_args(np, "clocks", "#clock-cells");
> > >
> > > Minor nitpick: please use of_clk_get_parent_count. I spent a solid 5
> > > minutes writing that function and I need people to use it so I can get a
> > > return on my investment.
> > >
> > > Otherwise the patch looks good. I believe that this method is targeting
> > > always-on clock in a production environment, which is different from the
> > > CLK_IGNORE_UNUSED stuff which typically is helpful while bringing up new
> > > hardware or dealing with a platform that has incomplete driver support.
> > 
> > There is also the usecase of keep clocks on until I load a module that
> > properly handles my hardware (e.g simplefb). We have a simplefb node
> > with clocks and the simplefb driver jumps thru some hoops to hand-off
> > clocks to the real driver. I don't really like it and don't want to
> > see more examples. And there is the case of I thought I would never
> > manage this clock, but kernel subsystems evolve and now I want to
> > manage a clock. This should not require a DT update to do so.
> > 
> > Neither of these may be Lee's usecase, but I want to see them covered
> > by the binding.
> > 
> > > I wonder if there is a clever way for existing clock providers
> > > (expressed in DT) to use this without having to create a separate node
> > > of clocks with the "always-on-clk-domain" flag. Possibly the common
> > > clock binding could declare some always-on flag that is standardized?
> > > Then the framework core could use this code easily. Not sure if that is
> > > a good idea though...
> > 
> > I would prefer to see the always on clocks just listed within the
> > clock controller's node rather than creating made up nodes with clock
> > properties.
> 
> > This should be always-on until claimed IMO, but that
> > aspect is the OS's problem, not a DT problem.
> 
> I disagree with this point.  There are likely to be many unclaimed,
> but perfectly gateable clocks in a system, which will consume power
> unnecessarily.  The clk framework does the right thing by turning all
> unclaimed clocks off IMHO.  This only leaves a small use-case where we
> need to artificially claim some which must not be gated.

I might have misread both of your mails, but I think you two are
actually in agreement. You both support a common property which lists
the always-on clocks inside of the common clock binding, no?

> 
> The other way to do is, as you mentioned is list the clocks which must
> stay on in the clock source node, but this will still require a
> binding.  It will also require a much more complicated framework
> driver.
> 
> clkprovider@ {
> always-on-clks = <1, 2, 4, 5, 7>;
> };

This should pose no burden on the driver. Since always-on-clks is in the
common clock binding it should be handled by the framework core. At
clk_register-time we can check for always-on-clks, walk the list and see
if we have a match. It's ugly O(n^2) but it works.

Thoughts?

Mike

> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: fractional-divider: support for divider bypassing

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 11:42:55)
 On 02/02/15 05:37, Heikki Krogerus wrote:
  If the divider or multiplier values values are 0 in the
 
 s/values//
 
  register, bypassing the divider and returning the parent
  clock rate in clk_fd_recalc_rate().
 
  Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
  ---
 
 Reviewed-by: Stephen Boyd sb...@codeaurora.org

Applied to clk-next.

Regards,
Mike

 
   drivers/clk/clk-fractional-divider.c | 3 +++
   1 file changed, 3 insertions(+)
 
  diff --git a/drivers/clk/clk-fractional-divider.c 
  b/drivers/clk/clk-fractional-divider.c
  index dc91da7..34d6c51 100644
  --- a/drivers/clk/clk-fractional-divider.c
  +++ b/drivers/clk/clk-fractional-divider.c
  @@ -36,6 +36,9 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
m = (val  fd-mmask)  fd-mshift;
n = (val  fd-nmask)  fd-nshift;
   
  + if (!n || !m)
  + return parent_rate;
  +
ret = (u64)parent_rate * m;
do_div(ret, n);
   
 
 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: qcom: fix platform_no_drv_owner.cocci warnings

[Mike Turquette]

Quoting Stephen Boyd (2015-01-28 12:51:05)
 On 01/27/15 23:11, kbuild test robot wrote:
  drivers/clk/qcom/lcc-msm8960.c:577:3-8: No need to set .owner here. The 
  core will do it.
 
   Remove .owner field if calls are used which set it automatically
 
  Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
 
  CC: Stephen Boyd sb...@codeaurora.org
  Signed-off-by: Fengguang Wu fengguang...@intel.com
  ---
 
 Reviewed-by: Stephen Boyd sb...@codeaurora.org

Applied.

Regards,
Mike

 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: qcom: fix platform_no_drv_owner.cocci warnings

[Mike Turquette]

Quoting Stephen Boyd (2015-01-28 12:49:32)
 On 01/27/15 23:00, kbuild test robot wrote:
  drivers/clk/qcom/lcc-ipq806x.c:465:3-8: No need to set .owner here. The 
  core will do it.
 
   Remove .owner field if calls are used which set it automatically
 
  Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
 
  CC: Rajendra Nayak rna...@codeaurora.org
  Signed-off-by: Fengguang Wu fengguang...@intel.com
 
 Reviewed-by: Stephen Boyd sb...@codeauroa.org

Applied.

Regards,
Mike

 
 Thanks. This code was reposted from a time when this wasn't done
 automatically.
 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: fractional-divider: support for divider bypassing

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 11:42:55)
 On 02/02/15 05:37, Heikki Krogerus wrote:
  If the divider or multiplier values values are 0 in the
 
 s/values//
 
  register, bypassing the divider and returning the parent
  clock rate in clk_fd_recalc_rate().
 
  Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
  ---
 
 Reviewed-by: Stephen Boyd sb...@codeaurora.org

Applied to clk-next.

Regards,
Mike

 
   drivers/clk/clk-fractional-divider.c | 3 +++
   1 file changed, 3 insertions(+)
 
  diff --git a/drivers/clk/clk-fractional-divider.c 
  b/drivers/clk/clk-fractional-divider.c
  index dc91da7..34d6c51 100644
  --- a/drivers/clk/clk-fractional-divider.c
  +++ b/drivers/clk/clk-fractional-divider.c
  @@ -36,6 +36,9 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
m = (val  fd-mmask)  fd-mshift;
n = (val  fd-nmask)  fd-nshift;
   
  + if (!n || !m)
  + return parent_rate;
  +
ret = (u64)parent_rate * m;
do_div(ret, n);
   
 
 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: Use lockdep asserts to find missing hold of prepare_lock

[Mike Turquette]

Quoting Krzysztof Kozlowski (2015-01-09 00:28:10)
 Add lockdep asserts for holding the prepare_lock to all functions
 marking this as a requirement in description. Add this to private and
 exported functions so all locking misuse could be detected during
 debugging.
 
 Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com

Applied.

Thanks,
Mike

 ---
  drivers/clk/clk.c | 14 ++
  1 file changed, 14 insertions(+)
 
 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index f4963b7d4e17..cdbfaa34ace4 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -432,6 +432,8 @@ static void clk_unprepare_unused_subtree(struct clk *clk)
  {
 struct clk *child;
  
 +   lockdep_assert_held(prepare_lock);
 +
 if (!clk)
 return;
  
 @@ -458,6 +460,8 @@ static void clk_disable_unused_subtree(struct clk *clk)
 struct clk *child;
 unsigned long flags;
  
 +   lockdep_assert_held(prepare_lock);
 +
 if (!clk)
 goto out;
  
 @@ -947,6 +951,8 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned 
 long rate)
 struct clk *parent;
 struct clk_hw *parent_hw;
  
 +   lockdep_assert_held(prepare_lock);
 +
 if (!clk)
 return 0;
  
 @@ -1040,6 +1046,8 @@ static void __clk_recalc_accuracies(struct clk *clk)
 unsigned long parent_accuracy = 0;
 struct clk *child;
  
 +   lockdep_assert_held(prepare_lock);
 +
 if (clk-parent)
 parent_accuracy = clk-parent-accuracy;
  
 @@ -1104,6 +1112,8 @@ static void __clk_recalc_rates(struct clk *clk, 
 unsigned long msg)
 unsigned long parent_rate = 0;
 struct clk *child;
  
 +   lockdep_assert_held(prepare_lock);
 +
 old_rate = clk-rate;
  
 if (clk-parent)
 @@ -1297,6 +1307,8 @@ static int __clk_speculate_rates(struct clk *clk, 
 unsigned long parent_rate)
 unsigned long new_rate;
 int ret = NOTIFY_DONE;
  
 +   lockdep_assert_held(prepare_lock);
 +
 new_rate = clk_recalc(clk, parent_rate);
  
 /* abort rate change if a driver returns NOTIFY_BAD or NOTIFY_STOP */
 @@ -2110,6 +2122,8 @@ static void __clk_release(struct kref *ref)
 struct clk *clk = container_of(ref, struct clk, ref);
 int i = clk-num_parents;
  
 +   lockdep_assert_held(prepare_lock);
 +
 kfree(clk-parents);
 while (--i = 0)
 kfree(clk-parent_names[i]);
 -- 
 1.9.1
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC 1/3] i2c: Enhancement of i2c API to address circular lock dependency problem

[Mike Turquette]

Quoting Mark Brown (2015-01-18 05:41:24)
 On Sun, Jan 18, 2015 at 11:54:56AM +0100, Krzysztof Kozlowski wrote:
  W dniu 18.01.2015 o 07:30, Tomasz Figa pisze:
 
  So, the question is, do we actually have hardware that _really_
  requires _actual_ preparation or all the clk_prepare_enable()s in I2C
  drivers (at least in i2c-s3c2410) are just to simplify the code?
 
  I completely forgot that you already thought about this deadlock in 2014. I
  think we can try the no-prepare way for i2c-s3c2410. However this would be
  only workaround for specific chip. Other buses (like SPI) would require
  similar changes.
 
 Right, and it's every single driver which would need an update too which
 is a bit icky and sad.  On the other hand a more detailed fix is going
 to involve trying to make the clock API locking more fine grained which
 isn't fun.

Not fun is right. Please see Stephen's attempt here:

http://lkml.kernel.org/r/1409792466-5092-1-git-send-email-sb...@codeaurora.org

I'm hoping this approach will be revisited soon.

Regards,
Mike

 
  I wondered why this was not observed (at least not observed by me with
  lockdep) on Gear 2 (Rinato) board. This is quite similar case: the S2MPS14
  PMIC provides regulators and 32kHz clocks. I think it is exactly the same
  pattern as for max77686... but somehow lockdep never reported that deadlock
  there.
 
 Mostly the clocks on PMICs never get changed at runtime which helps a
 lot here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] MSM8960 LCC fixes

[Mike Turquette]

Quoting Stephen Boyd (2015-01-29 15:38:10)
 A couple of small fixes found while testing the audio clock control
 on apq8064.

Applied to clk-fixes.

Regards,
Mike

 
 Stephen Boyd (3):
   clk: qcom: lcc-msm8960: Fix slimbus n and m val offsets
   clk: qcom: lcc-msm8960: Fix PLL rate detection
   clk: qcom: Add PLL4 vote clock
 
  drivers/clk/qcom/gcc-msm8960.c | 13 +
  drivers/clk/qcom/lcc-msm8960.c |  6 +++---
  2 files changed, 16 insertions(+), 3 deletions(-)
 
 -- 
 The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/4] clk: Provide an always-on clock domain framework

[Mike Turquette]

Quoting Lee Jones (2015-02-25 07:48:08)
 On Wed, 25 Feb 2015, Rob Herring wrote:
 
  On Mon, Feb 23, 2015 at 11:23 AM, Mike Turquette mturque...@linaro.org 
  wrote:
   Quoting Lee Jones (2015-02-18 08:15:00)
   Much h/w contain clocks which if turned off would prove fatal.  The
   only way to recover is to restart the board(s).  This driver takes
   references to clocks which are required to be always-on in order to
   prevent the common clk framework from trying to turn them off during
   the clk_disabled_unused() procedure.
  
  [...]
  
   +static int ao_clock_domain_probe(struct platform_device *pdev)
   +{
   +   struct device_node *np = pdev-dev.of_node;
   +   int nclks, i;
   +
   +   nclks = of_count_phandle_with_args(np, clocks, #clock-cells);
  
   Minor nitpick: please use of_clk_get_parent_count. I spent a solid 5
   minutes writing that function and I need people to use it so I can get a
   return on my investment.
  
   Otherwise the patch looks good. I believe that this method is targeting
   always-on clock in a production environment, which is different from the
   CLK_IGNORE_UNUSED stuff which typically is helpful while bringing up new
   hardware or dealing with a platform that has incomplete driver support.
  
  There is also the usecase of keep clocks on until I load a module that
  properly handles my hardware (e.g simplefb). We have a simplefb node
  with clocks and the simplefb driver jumps thru some hoops to hand-off
  clocks to the real driver. I don't really like it and don't want to
  see more examples. And there is the case of I thought I would never
  manage this clock, but kernel subsystems evolve and now I want to
  manage a clock. This should not require a DT update to do so.
  
  Neither of these may be Lee's usecase, but I want to see them covered
  by the binding.
  
   I wonder if there is a clever way for existing clock providers
   (expressed in DT) to use this without having to create a separate node
   of clocks with the always-on-clk-domain flag. Possibly the common
   clock binding could declare some always-on flag that is standardized?
   Then the framework core could use this code easily. Not sure if that is
   a good idea though...
  
  I would prefer to see the always on clocks just listed within the
  clock controller's node rather than creating made up nodes with clock
  properties.
 
  This should be always-on until claimed IMO, but that
  aspect is the OS's problem, not a DT problem.
 
 I disagree with this point.  There are likely to be many unclaimed,
 but perfectly gateable clocks in a system, which will consume power
 unnecessarily.  The clk framework does the right thing by turning all
 unclaimed clocks off IMHO.  This only leaves a small use-case where we
 need to artificially claim some which must not be gated.

I might have misread both of your mails, but I think you two are
actually in agreement. You both support a common property which lists
the always-on clocks inside of the common clock binding, no?

 
 The other way to do is, as you mentioned is list the clocks which must
 stay on in the clock source node, but this will still require a
 binding.  It will also require a much more complicated framework
 driver.
 
 clkprovider@ {
 always-on-clks = 1, 2, 4, 5, 7;
 };

This should pose no burden on the driver. Since always-on-clks is in the
common clock binding it should be handled by the framework core. At
clk_register-time we can check for always-on-clks, walk the list and see
if we have a match. It's ugly O(n^2) but it works.

Thoughts?

Mike

 -- 
 Lee Jones
 Linaro STMicroelectronics Landing Team Lead
 Linaro.org │ Open source software for ARM SoCs
 Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: Missing set_phase op is an error

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 14:09:43)
 If a clock's clk_ops doesn't have the set_phase op set we should
 return an error from clk_set_phase(). This way clock consumers
 know that when they tried to set a phase it didn't work, as
 opposed to the current behavior where the return value is 0
 meaning success.
 
 Signed-off-by: Stephen Boyd sb...@codeaurora.org

Applied to clk-next.

Thanks,
Mike

 ---
  drivers/clk/clk.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index a29daf9edea4..b82714a84f5e 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -2069,10 +2069,10 @@ EXPORT_SYMBOL_GPL(clk_set_parent);
   */
  int clk_set_phase(struct clk *clk, int degrees)
  {
 -   int ret = 0;
 +   int ret = -EINVAL;
  
 if (!clk)
 -   goto out;
 +   return 0;
  
 /* sanity check degrees */
 degrees %= 360;
 @@ -2081,18 +2081,14 @@ int clk_set_phase(struct clk *clk, int degrees)
  
 clk_prepare_lock();
  
 -   if (!clk-core-ops-set_phase)
 -   goto out_unlock;
 -
 -   ret = clk-core-ops-set_phase(clk-core-hw, degrees);
 +   if (clk-core-ops-set_phase)
 +   ret = clk-core-ops-set_phase(clk-core-hw, degrees);
  
 if (!ret)
 clk-core-phase = degrees;
  
 -out_unlock:
 clk_prepare_unlock();
  
 -out:
 return ret;
  }
  EXPORT_SYMBOL_GPL(clk_set_phase);
 -- 
 The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] clk: Add tracepoints for hardware operations

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 14:37:41)
 It's useful to have tracepoints around operations that change the
 hardware state so that we can debug clock hardware performance
 and operations. Four basic types of events are supported: on/off
 events for enable, disable, prepare, unprepare that only record
 an event and a clock name, rate changing events for
 clk_set_{min_,max_}rate{_range}(), phase changing events for
 clk_set_phase() and parent changing events for clk_set_parent().
 
 Cc: Steven Rostedt rost...@goodmis.org
 Signed-off-by: Stephen Boyd sb...@codeaurora.org

Applied to clk-next.

Thanks,
Mike

 ---
 
 Changes since v1:
  * Fixed missing trace_*_complete() tracepoints
 
 Note this is based on another patch titled clk: Missing
 set_phase op is an error
 
  drivers/clk/clk.c  |  56 ++---
  include/trace/events/clk.h | 198 
 +
  2 files changed, 244 insertions(+), 10 deletions(-)
  create mode 100644 include/trace/events/clk.h
 
 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index 670c8c86ce9f..3ffadce9a0c2 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -22,6 +22,9 @@
  #include linux/init.h
  #include linux/sched.h
  
 +#define CREATE_TRACE_POINTS
 +#include trace/events/clk.h
 +
  #include clk.h
  
  static DEFINE_SPINLOCK(enable_lock);
 @@ -448,10 +451,12 @@ static void clk_unprepare_unused_subtree(struct 
 clk_core *clk)
 return;
  
 if (clk_core_is_prepared(clk)) {
 +   trace_clk_unprepare(clk);
 if (clk-ops-unprepare_unused)
 clk-ops-unprepare_unused(clk-hw);
 else if (clk-ops-unprepare)
 clk-ops-unprepare(clk-hw);
 +   trace_clk_unprepare_complete(clk);
 }
  }
  
 @@ -478,10 +483,12 @@ static void clk_disable_unused_subtree(struct clk_core 
 *clk)
  * back to .disable
  */
 if (clk_core_is_enabled(clk)) {
 +   trace_clk_disable(clk);
 if (clk-ops-disable_unused)
 clk-ops-disable_unused(clk-hw);
 else if (clk-ops-disable)
 clk-ops-disable(clk-hw);
 +   trace_clk_disable_complete(clk);
 }
  
  unlock_out:
 @@ -861,9 +868,12 @@ static void clk_core_unprepare(struct clk_core *clk)
  
 WARN_ON(clk-enable_count  0);
  
 +   trace_clk_unprepare(clk);
 +
 if (clk-ops-unprepare)
 clk-ops-unprepare(clk-hw);
  
 +   trace_clk_unprepare_complete(clk);
 clk_core_unprepare(clk-parent);
  }
  
 @@ -901,12 +911,16 @@ static int clk_core_prepare(struct clk_core *clk)
 if (ret)
 return ret;
  
 -   if (clk-ops-prepare) {
 +   trace_clk_prepare(clk);
 +
 +   if (clk-ops-prepare)
 ret = clk-ops-prepare(clk-hw);
 -   if (ret) {
 -   clk_core_unprepare(clk-parent);
 -   return ret;
 -   }
 +
 +   trace_clk_prepare_complete(clk);
 +
 +   if (ret) {
 +   clk_core_unprepare(clk-parent);
 +   return ret;
 }
 }
  
 @@ -953,9 +967,13 @@ static void clk_core_disable(struct clk_core *clk)
 if (--clk-enable_count  0)
 return;
  
 +   trace_clk_disable(clk);
 +
 if (clk-ops-disable)
 clk-ops-disable(clk-hw);
  
 +   trace_clk_disable_complete(clk);
 +
 clk_core_disable(clk-parent);
  }
  
 @@ -1008,12 +1026,16 @@ static int clk_core_enable(struct clk_core *clk)
 if (ret)
 return ret;
  
 -   if (clk-ops-enable) {
 +   trace_clk_enable(clk);
 +
 +   if (clk-ops-enable)
 ret = clk-ops-enable(clk-hw);
 -   if (ret) {
 -   clk_core_disable(clk-parent);
 -   return ret;
 -   }
 +
 +   trace_clk_enable_complete(clk);
 +
 +   if (ret) {
 +   clk_core_disable(clk-parent);
 +   return ret;
 }
 }
  
 @@ -1438,10 +1460,14 @@ static int __clk_set_parent(struct clk_core *clk, 
 struct clk_core *parent,
  
 old_parent = __clk_set_parent_before(clk, parent);
  
 +   trace_clk_set_parent(clk, parent);
 +
 /* change clock input source */
 if (parent  clk-ops-set_parent)
 ret = clk-ops-set_parent(clk-hw, p_index);
  
 +   trace_clk_set_parent_complete(clk, parent);
 +
 if (ret) {
 flags = clk_enable_lock();
 clk_reparent(clk, old_parent);
 @@ -1665,6 +1691,7 @@ static void clk_change_rate(struct clk_core *clk)
  
 if 

Re: [PATCH] clk: clk_set_parent() with current parent shouldn't fail

[Mike Turquette]

Quoting Philipp Zabel (2015-02-03 01:42:34)
 Am Montag, den 02.02.2015, 14:11 -0800 schrieb Stephen Boyd:
  If a driver calls clk_set_parent(clk, parent) and parent is the
  current parent of clk we shouldn't fail in any case.
  Unfortunately if clk is a read-only mux we return -ENOSYS
  because we think we can't change the parent, except for in this
  special case where we don't actually need to change the parent at
  all. Return 0 in such a situation.
  
  Cc: Philipp Zabel p.za...@pengutronix.de
  Signed-off-by: Stephen Boyd sb...@codeaurora.org
 
 Acked-by: Philipp Zabel p.za...@pengutronix.de

Applied to clk-next.

Regards,
Mike

 
 regards
 Philipp
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/4] clk: Provide an always-on clock domain framework

[Mike Turquette]

Quoting Lee Jones (2015-02-18 08:15:00)
> Much h/w contain clocks which if turned off would prove fatal.  The
> only way to recover is to restart the board(s).  This driver takes
> references to clocks which are required to be always-on in order to
> prevent the common clk framework from trying to turn them off during
> the clk_disabled_unused() procedure.
> 
> Signed-off-by: Lee Jones 
> ---
>  drivers/clk/Makefile|  1 +
>  drivers/clk/clkdomain.c | 63 
> +
>  2 files changed, 64 insertions(+)
>  create mode 100644 drivers/clk/clkdomain.c
> 
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index d5fba5b..d9e2718 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_HAVE_CLK)  += clk-devres.o
>  obj-$(CONFIG_CLKDEV_LOOKUP)+= clkdev.o
>  obj-$(CONFIG_COMMON_CLK)   += clk.o
>  obj-$(CONFIG_COMMON_CLK)   += clk-divider.o
> +obj-$(CONFIG_COMMON_CLK)   += clkdomain.o
>  obj-$(CONFIG_COMMON_CLK)   += clk-fixed-factor.o
>  obj-$(CONFIG_COMMON_CLK)   += clk-fixed-rate.o
>  obj-$(CONFIG_COMMON_CLK)   += clk-gate.o
> diff --git a/drivers/clk/clkdomain.c b/drivers/clk/clkdomain.c
> new file mode 100644
> index 000..8c83f99
> --- /dev/null
> +++ b/drivers/clk/clkdomain.c

Naming is hard. I'm not sure clkdomain.c is the best expression. Maybe
clk-alwon.c? I see ALWON used alot amongst hardware people who live in a
world of eight-character naming limitations.

> @@ -0,0 +1,63 @@
> +/*
> + * ST Clock Domain
> + *
> + * Copyright (C) 2015 STMicroelectronics – All Rights Reserved
> + *
> + * Author: Lee Jones 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 

If this header still existed I would berate you mercilessly. Luckily for
you it no longer exists and only causes a compile error ;-)

> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static void ao_clock_domain_hog_clock(struct platform_device *pdev, int 
> index)
> +{
> +   struct device_node *np = pdev->dev.of_node;
> +   struct clk *clk;
> +   int ret;
> +
> +   clk = of_clk_get(np, index);
> +   if (IS_ERR(clk)) {
> +   dev_warn(>dev, "Failed get clock %s[%d]: %li\n",
> +np->full_name, index, PTR_ERR(clk));
> +   return;
> +   }
> +
> +   ret = clk_prepare_enable(clk);
> +   if (ret)
> +   dev_warn(>dev, "Failed to enable clock: %s\n", 
> clk->name);
> +}
> +
> +static int ao_clock_domain_probe(struct platform_device *pdev)
> +{
> +   struct device_node *np = pdev->dev.of_node;
> +   int nclks, i;
> +
> +   nclks = of_count_phandle_with_args(np, "clocks", "#clock-cells");

Minor nitpick: please use of_clk_get_parent_count. I spent a solid 5
minutes writing that function and I need people to use it so I can get a
return on my investment.

Otherwise the patch looks good. I believe that this method is targeting
always-on clock in a production environment, which is different from the
CLK_IGNORE_UNUSED stuff which typically is helpful while bringing up new
hardware or dealing with a platform that has incomplete driver support.

I wonder if there is a clever way for existing clock providers
(expressed in DT) to use this without having to create a separate node
of clocks with the "always-on-clk-domain" flag. Possibly the common
clock binding could declare some always-on flag that is standardized?
Then the framework core could use this code easily. Not sure if that is
a good idea though...

Regards,
Mike

> +
> +   for (i = 0; i < nclks; i++)
> +   ao_clock_domain_hog_clock(pdev, i);
> +
> +   return 0;
> +}
> +
> +static const struct of_device_id ao_clock_domain_match[] = {
> +   { .compatible = "always-on-clk-domain" },
> +   { },
> +};
> +
> +static struct platform_driver ao_clock_domain_driver = {
> +   .probe = ao_clock_domain_probe,
> +   .driver = {
> +   .name = "always-on-clk-domain",
> +   .of_match_table = ao_clock_domain_match,
> +   },
> +};
> +module_platform_driver(ao_clock_domain_driver);
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/4] clk: Provide an always-on clock domain framework

[Mike Turquette]

Quoting Lee Jones (2015-02-18 08:15:00)
 Much h/w contain clocks which if turned off would prove fatal.  The
 only way to recover is to restart the board(s).  This driver takes
 references to clocks which are required to be always-on in order to
 prevent the common clk framework from trying to turn them off during
 the clk_disabled_unused() procedure.
 
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
  drivers/clk/Makefile|  1 +
  drivers/clk/clkdomain.c | 63 
 +
  2 files changed, 64 insertions(+)
  create mode 100644 drivers/clk/clkdomain.c
 
 diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
 index d5fba5b..d9e2718 100644
 --- a/drivers/clk/Makefile
 +++ b/drivers/clk/Makefile
 @@ -3,6 +3,7 @@ obj-$(CONFIG_HAVE_CLK)  += clk-devres.o
  obj-$(CONFIG_CLKDEV_LOOKUP)+= clkdev.o
  obj-$(CONFIG_COMMON_CLK)   += clk.o
  obj-$(CONFIG_COMMON_CLK)   += clk-divider.o
 +obj-$(CONFIG_COMMON_CLK)   += clkdomain.o
  obj-$(CONFIG_COMMON_CLK)   += clk-fixed-factor.o
  obj-$(CONFIG_COMMON_CLK)   += clk-fixed-rate.o
  obj-$(CONFIG_COMMON_CLK)   += clk-gate.o
 diff --git a/drivers/clk/clkdomain.c b/drivers/clk/clkdomain.c
 new file mode 100644
 index 000..8c83f99
 --- /dev/null
 +++ b/drivers/clk/clkdomain.c

Naming is hard. I'm not sure clkdomain.c is the best expression. Maybe
clk-alwon.c? I see ALWON used alot amongst hardware people who live in a
world of eight-character naming limitations.

 @@ -0,0 +1,63 @@
 +/*
 + * ST Clock Domain
 + *
 + * Copyright (C) 2015 STMicroelectronics – All Rights Reserved
 + *
 + * Author: Lee Jones lee.jo...@linaro.org
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +
 +#include linux/clk-private.h

If this header still existed I would berate you mercilessly. Luckily for
you it no longer exists and only causes a compile error ;-)

 +#include linux/clk-provider.h
 +#include linux/of_address.h
 +#include linux/of.h
 +#include linux/platform_device.h
 +
 +static void ao_clock_domain_hog_clock(struct platform_device *pdev, int 
 index)
 +{
 +   struct device_node *np = pdev-dev.of_node;
 +   struct clk *clk;
 +   int ret;
 +
 +   clk = of_clk_get(np, index);
 +   if (IS_ERR(clk)) {
 +   dev_warn(pdev-dev, Failed get clock %s[%d]: %li\n,
 +np-full_name, index, PTR_ERR(clk));
 +   return;
 +   }
 +
 +   ret = clk_prepare_enable(clk);
 +   if (ret)
 +   dev_warn(pdev-dev, Failed to enable clock: %s\n, 
 clk-name);
 +}
 +
 +static int ao_clock_domain_probe(struct platform_device *pdev)
 +{
 +   struct device_node *np = pdev-dev.of_node;
 +   int nclks, i;
 +
 +   nclks = of_count_phandle_with_args(np, clocks, #clock-cells);

Minor nitpick: please use of_clk_get_parent_count. I spent a solid 5
minutes writing that function and I need people to use it so I can get a
return on my investment.

Otherwise the patch looks good. I believe that this method is targeting
always-on clock in a production environment, which is different from the
CLK_IGNORE_UNUSED stuff which typically is helpful while bringing up new
hardware or dealing with a platform that has incomplete driver support.

I wonder if there is a clever way for existing clock providers
(expressed in DT) to use this without having to create a separate node
of clocks with the always-on-clk-domain flag. Possibly the common
clock binding could declare some always-on flag that is standardized?
Then the framework core could use this code easily. Not sure if that is
a good idea though...

Regards,
Mike

 +
 +   for (i = 0; i  nclks; i++)
 +   ao_clock_domain_hog_clock(pdev, i);
 +
 +   return 0;
 +}
 +
 +static const struct of_device_id ao_clock_domain_match[] = {
 +   { .compatible = always-on-clk-domain },
 +   { },
 +};
 +
 +static struct platform_driver ao_clock_domain_driver = {
 +   .probe = ao_clock_domain_probe,
 +   .driver = {
 +   .name = always-on-clk-domain,
 +   .of_match_table = ao_clock_domain_match,
 +   },
 +};
 +module_platform_driver(ao_clock_domain_driver);
 -- 
 1.9.1
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] clk: changes for 3.20

[Mike Turquette]

The following changes since commit e36f014edff70fc02b3d3d79cead1d58f289332e:

  Linux 3.19-rc7 (2015-02-01 20:07:21 -0800)

are available in the git repository at:

  https://git.linaro.org/people/mike.turquette/linux.git tags/clk-for-linus-3.20

for you to fetch changes up to ec02ace8ca0a50eef430d3676de5c5fa978b0e29:

  clk: Only recalculate the rate if needed (2015-02-19 19:29:19 -0800)


The clock framework changes for 3.20 contain the usual driver additions,
enhancements and fixes mostly for ARM32, ARM64, MIPS and Power-based
devices. Additionaly the framework core underwent a bit of surgery with
two major changes. The boundary between the clock core and clock
providers (e.g clock drivers) is now more well defined with dedicated
provider helper functions. struct clk no longer maps 1:1 with the
hardware clock but is a true per-user cookie which helps us tracker
users of hardware clocks and debug bad behavior. The second major change
is the addition of rate constraints for clocks. Rate ranges are now
supported which are analogous to the voltage ranges in the regulator
framework. Unfortunately these changes to the core created some
breakeage. We think we fixed it all up but for this reason there are
lots of last minute commits trying to undo the damage.


Andrew Bresticker (1):
  clk: tegra: SDMMC controllers are on APB

Arnd Bergmann (1):
  clk: omap: compile legacy omap3 clocks conditionally

Chanwoo Choi (4):
  clk: samsung: Change the return value of samsung_cmu_register_one()
  clk: samsung: exynos3250: Use samsung_cmu_register_one() to simplify code
  clk: samsung: exynos4415: Use samsung_cmu_register_one() to simplify code
  clk: samsung: exynos4: Add divider clock id for memory bus frequency

Chen-Yu Tsai (10):
  clk: sunxi: Remove ahb1_sdram from sun6i/sun8i protected clocks list
  clk: sunxi: unify sun6i AHB1 clock with proper PLL6 pre-divider
  ARM: dts: sun6i: Unify ahb1 clock nodes
  ARM: dts: sun8i: Unify ahb1 clock nodes
  ARM: dts: sun8i: Add PLL6 and MBUS clock nodes
  clk: sunxi: Fix factor clocks usage for sun9i core clocks
  clk: sunxi: Propagate rate changes to parent for mux clocks
  clk: sunxi: Add a common setup function for mmc module clocks
  clk: sunxi: Add mod0 and mmc module clock support for A80
  clk: sunxi: Add driver for A80 MMC config clocks/resets

Doug Anderson (2):
  clk: rockchip: Add CLK_SET_RATE_PARENT to sclk_uart clocks
  clk: rockchip: rk3288: Make s2r reliable by switching PLLs to slow mode

Emil Medve (9):
  clk: qoriq: Fix checkpatch type PARENTHESIS_ALIGNMENT
  clk: qoriq: Fix checkpatch type ALLOC_WITH_MULTIPLY
  clk: qoriq: Fix checkpatch type ALLOC_SIZEOF_STRUCT
  clk: qoriq: Fix checkpatch type OOM_MESSAGE
  clk: qoriq: Make local symbol 'static'
  clk: qoriq: Replace kzalloc() with kmalloc()
  clk: qoriq: Use pr_fmt()
  powerpc/corenet: Enable CLK_QORIQ
  clk: qoriq: Add support for the platform PLL

Geert Uytterhoeven (2):
  clk: shmobile: div6: Avoid changing divisor in .disable()
  clk: shmobile: div6: Avoid division by zero in .round_rate()

Hans de Goede (4):
  clk: sunxi: Give sunxi_factors_register a registers parameter
  clk: sunxi: Make the mod0 clk driver also a platform driver
  clk: sunxi: rewrite sun9i_a80_get_pll4_factors()
  sunxi: clk: Set sun6i-pll1 n_start = 1

Heiko Stuebner (3):
  clk: rockchip: add id for watchdog pclk on rk3288
  Merge branch 'v3.20-clk/new-ids' into v3.20-clk/next
  clk: rockchip: add a dummy clock for the watchdog pclk on rk3288

Hisashi Nakamura (1):
  clk: shmobile: Add r8a7793 support

Huang Lin (1):
  clk: rockchip: add clock IDs for the PVTM clocks

Javier Martinez Canillas (3):
  clk: Don't dereference parent clock if is NULL
  clk: Add __clk_hw_set_clk helper function
  clk: Replace explicit clk assignment with __clk_hw_set_clk

Josh Cartwright (1):
  clk: qcom: Add support for regmap divider clocks

Kever Yang (2):
  clk: rockchip: add clock ID for usbphy480m_src
  clk: rockchip: use the clock ID for usbphy480m_src

Kevin Hao (2):
  powerpc: call of_clk_init() from time_init()
  clk: ppc-corenet: fix section mismatch warning

Krzysztof Kozlowski (2):
  clk: Add clk_unregister_{divider, gate, mux} to close memory leak
  clk: exynos-audss: Fix memory leak on driver unbind or probe failure

Mark Zhang (1):
  clk: tegra: Define PLLD_DSI and remove dsia(b)_mux

Max Filippov (1):
  clk: TI CDCE706 clock synthesizer driver

Maxime Ripard (5):
  clk: sunxi: Rework MMC phase clocks
  ARM: sunxi: dt: Add sample and output mmc clocks
  mmc: sunxi: Convert MMC driver to the standard clock phase API
  clk: sunxi: Remove custom phase function
  clk: Export phase functions

Re: [PATCH RFC v9 01/20] clk: divider: Correct parent clk round rate if no bestdiv is normally found

[Mike Turquette]

Quoting Russell King - ARM Linux (2015-02-16 03:27:24)
> On Fri, Feb 13, 2015 at 07:57:13PM +0100, Sascha Hauer wrote:
> > I agree that it's a bit odd, but I think it has to be like this.
> > Consider that you request a rate of 100Hz, but the clock can only
> > produce 99.5Hz, so due to rounding clk_round_rate() returns 99Hz.
> > Now when you request 99Hz from clk_set_rate() the 99.5Hz value
> > can't be used because it's too high.
> 
> Math rounding rules normally state that anything of .5 and greater
> should be rounded up, not rounded down.  So, for 99.5Hz, you really
> ought to be returning 100Hz, not 99Hz.
> 
> However, you do have a point for 99.4Hz, which would be returned as
> 99Hz, and when set, it would result in something which isn't 99.4Hz.

More practically, this again raises the issue of whether or not unsigned
long rate should be in millihertz or something other than hertz.

And then that question again raises the issue of making rate 64-bit...

Regards,
Mike

> 
> -- 
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC v9 01/20] clk: divider: Correct parent clk round rate if no bestdiv is normally found

[Mike Turquette]

Quoting Russell King - ARM Linux (2015-02-16 03:27:24)
 On Fri, Feb 13, 2015 at 07:57:13PM +0100, Sascha Hauer wrote:
  I agree that it's a bit odd, but I think it has to be like this.
  Consider that you request a rate of 100Hz, but the clock can only
  produce 99.5Hz, so due to rounding clk_round_rate() returns 99Hz.
  Now when you request 99Hz from clk_set_rate() the 99.5Hz value
  can't be used because it's too high.
 
 Math rounding rules normally state that anything of .5 and greater
 should be rounded up, not rounded down.  So, for 99.5Hz, you really
 ought to be returning 100Hz, not 99Hz.
 
 However, you do have a point for 99.4Hz, which would be returned as
 99Hz, and when set, it would result in something which isn't 99.4Hz.

More practically, this again raises the issue of whether or not unsigned
long rate should be in millihertz or something other than hertz.

And then that question again raises the issue of making rate 64-bit...

Regards,
Mike

 
 -- 
 FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
 according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] clk: changes for 3.20

[Mike Turquette]

The following changes since commit e36f014edff70fc02b3d3d79cead1d58f289332e:

  Linux 3.19-rc7 (2015-02-01 20:07:21 -0800)

are available in the git repository at:

  https://git.linaro.org/people/mike.turquette/linux.git tags/clk-for-linus-3.20

for you to fetch changes up to ec02ace8ca0a50eef430d3676de5c5fa978b0e29:

  clk: Only recalculate the rate if needed (2015-02-19 19:29:19 -0800)


The clock framework changes for 3.20 contain the usual driver additions,
enhancements and fixes mostly for ARM32, ARM64, MIPS and Power-based
devices. Additionaly the framework core underwent a bit of surgery with
two major changes. The boundary between the clock core and clock
providers (e.g clock drivers) is now more well defined with dedicated
provider helper functions. struct clk no longer maps 1:1 with the
hardware clock but is a true per-user cookie which helps us tracker
users of hardware clocks and debug bad behavior. The second major change
is the addition of rate constraints for clocks. Rate ranges are now
supported which are analogous to the voltage ranges in the regulator
framework. Unfortunately these changes to the core created some
breakeage. We think we fixed it all up but for this reason there are
lots of last minute commits trying to undo the damage.


Andrew Bresticker (1):
  clk: tegra: SDMMC controllers are on APB

Arnd Bergmann (1):
  clk: omap: compile legacy omap3 clocks conditionally

Chanwoo Choi (4):
  clk: samsung: Change the return value of samsung_cmu_register_one()
  clk: samsung: exynos3250: Use samsung_cmu_register_one() to simplify code
  clk: samsung: exynos4415: Use samsung_cmu_register_one() to simplify code
  clk: samsung: exynos4: Add divider clock id for memory bus frequency

Chen-Yu Tsai (10):
  clk: sunxi: Remove ahb1_sdram from sun6i/sun8i protected clocks list
  clk: sunxi: unify sun6i AHB1 clock with proper PLL6 pre-divider
  ARM: dts: sun6i: Unify ahb1 clock nodes
  ARM: dts: sun8i: Unify ahb1 clock nodes
  ARM: dts: sun8i: Add PLL6 and MBUS clock nodes
  clk: sunxi: Fix factor clocks usage for sun9i core clocks
  clk: sunxi: Propagate rate changes to parent for mux clocks
  clk: sunxi: Add a common setup function for mmc module clocks
  clk: sunxi: Add mod0 and mmc module clock support for A80
  clk: sunxi: Add driver for A80 MMC config clocks/resets

Doug Anderson (2):
  clk: rockchip: Add CLK_SET_RATE_PARENT to sclk_uart clocks
  clk: rockchip: rk3288: Make s2r reliable by switching PLLs to slow mode

Emil Medve (9):
  clk: qoriq: Fix checkpatch type PARENTHESIS_ALIGNMENT
  clk: qoriq: Fix checkpatch type ALLOC_WITH_MULTIPLY
  clk: qoriq: Fix checkpatch type ALLOC_SIZEOF_STRUCT
  clk: qoriq: Fix checkpatch type OOM_MESSAGE
  clk: qoriq: Make local symbol 'static'
  clk: qoriq: Replace kzalloc() with kmalloc()
  clk: qoriq: Use pr_fmt()
  powerpc/corenet: Enable CLK_QORIQ
  clk: qoriq: Add support for the platform PLL

Geert Uytterhoeven (2):
  clk: shmobile: div6: Avoid changing divisor in .disable()
  clk: shmobile: div6: Avoid division by zero in .round_rate()

Hans de Goede (4):
  clk: sunxi: Give sunxi_factors_register a registers parameter
  clk: sunxi: Make the mod0 clk driver also a platform driver
  clk: sunxi: rewrite sun9i_a80_get_pll4_factors()
  sunxi: clk: Set sun6i-pll1 n_start = 1

Heiko Stuebner (3):
  clk: rockchip: add id for watchdog pclk on rk3288
  Merge branch 'v3.20-clk/new-ids' into v3.20-clk/next
  clk: rockchip: add a dummy clock for the watchdog pclk on rk3288

Hisashi Nakamura (1):
  clk: shmobile: Add r8a7793 support

Huang Lin (1):
  clk: rockchip: add clock IDs for the PVTM clocks

Javier Martinez Canillas (3):
  clk: Don't dereference parent clock if is NULL
  clk: Add __clk_hw_set_clk helper function
  clk: Replace explicit clk assignment with __clk_hw_set_clk

Josh Cartwright (1):
  clk: qcom: Add support for regmap divider clocks

Kever Yang (2):
  clk: rockchip: add clock ID for usbphy480m_src
  clk: rockchip: use the clock ID for usbphy480m_src

Kevin Hao (2):
  powerpc: call of_clk_init() from time_init()
  clk: ppc-corenet: fix section mismatch warning

Krzysztof Kozlowski (2):
  clk: Add clk_unregister_{divider, gate, mux} to close memory leak
  clk: exynos-audss: Fix memory leak on driver unbind or probe failure

Mark Zhang (1):
  clk: tegra: Define PLLD_DSI and remove dsia(b)_mux

Max Filippov (1):
  clk: TI CDCE706 clock synthesizer driver

Maxime Ripard (5):
  clk: sunxi: Rework MMC phase clocks
  ARM: sunxi: dt: Add sample and output mmc clocks
  mmc: sunxi: Convert MMC driver to the standard clock phase API
  clk: sunxi: Remove custom phase function
  clk: Export phase functions

Re: [PATCH] clk: Only recalculate the rate if needed

[Mike Turquette]

Quoting Tomeu Vizoso (2015-02-06 06:13:01)
> We don't really need to recalculate the effective rate of a clock when a
> per-user clock is removed, if the constraints of the later aren't
> limiting the requested rate.
> 
> This was causing problems with clocks that never had a rate set before,
> as rate_req would be zero. Though this could be considered a bug in the
> implementation of those clocks, this should be checked somewhere else.
> 
> Cc: Thierry Reding 
> Cc: Peter De Schrijver 
> Signed-off-by: Tomeu Vizoso 

Fixes: 1c8e600440c7 ("clk: Add rate constraints to clocks")

Applied to clk-next. With this fix sunxi no longer vomits WARNs
everywhere due to divide-by-zero in the following path:

of_clk_init -> parent_ready -> __clk_put

Thanks,
Mike

> 
> ---
> 
> This applies on top of https://lkml.org/lkml/2015/2/5/769
> ---
>  drivers/clk/clk.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index a7d37c3..4ea2d53 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2664,7 +2664,11 @@ void __clk_put(struct clk *clk)
> clk_prepare_lock();
>  
> hlist_del(>clks_node);
> -   clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
> +
> +   if (clk->min_rate > clk->core->req_rate ||
> +   clk->max_rate < clk->core->req_rate)
> +   clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
> +
> owner = clk->core->owner;
> kref_put(>core->ref, __clk_release);
>  
> -- 
> 1.9.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 13/13] mfd: Add support for the MediaTek MT6397 PMIC

[Mike Turquette]

Quoting Lee Jones (2015-02-19 04:13:04)
> On Thu, 19 Feb 2015, Sascha Hauer wrote:
> 
> > On Thu, Feb 19, 2015 at 08:43:49AM +, Lee Jones wrote:
> > > On Thu, 19 Feb 2015, Sascha Hauer wrote:
> > > 
> > > > > > Looks okay to me now.
> > > > > > 
> > > > > > Acked-by: Lee Jones 
> > > > > > 
> > > > > > What's the merge plan for this set?
> > > > > 
> > > > > Patches 1-9 are clock related an several of them have review comments
> > > > > that need to be addressed. I wonder if a V2 series can break out the
> > > > > various subsystems bits from each other?
> > > > 
> > > > I'll send a new series later this day. These used to be two series, but
> > > > the PMIC wrapper patches depend on the clock and reset controllers, also
> > > > the device nodes depend on the clock/reset defines from the clock
> > > > support patches. What do you suggest? In the early days of a SoC
> > > > everything seems to depend on everything.
> > > 
> > > Only build dependencies count.  So long as the Kconfigs are setup
> > > correct, there shouldn't be any issue in taking patches in one
> > > subsystem at a time.
> > 
> > The dts snippets need the files in include/dt-bindings, so indeed this
> > is a build dependency. However, this comes only in with the dts changes.
> > 
> > So here's the plan:
> > 
> > - Mike takes the clk patches
> > - Matthias takes the pmic wrapper driver (in drivers/soc/mediatek/)
> > - You take the MT6397 core driver.
> 
> Sounds reasonable.  Just ensure that each set is orthogonal and builds
> (or doesn't attempt to) and we'll be in a good place.

Agreed.

Regards,
Mike

> 
> > I'll queue up the dts changes locally and ask Arnd to take these after
> > next -rc1 so that all dependencies are in. Unfortunately this means that
> > the patches can't be tested until everything is together after next
> > -rc1.
> 
> I'm sure you will be diligent enough to test the interoperability of
> the sets combined.  Failing that we can deal with any unavoidable
> fall-out during the -rcs.
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Stephen Boyd (2015-02-06 11:30:18)
> On 02/06/15 05:39, Russell King - ARM Linux wrote:
> > On Thu, Feb 05, 2015 at 05:35:28PM -0800, Stephen Boyd wrote:
> >
> >> From what I can tell this code is
> >> now broken because we made all clk getting functions (there's quite a
> >> few...) return unique pointers every time they're called. It seems that
> >> the driver wants to know if extclk and clk are the same so it can do
> >> something differently in kirkwood_set_rate(). Do we need some sort of
> >> clk_equal(struct clk *a, struct clk *b) function for drivers like this?
> > Well, the clocks in question are the SoC internal clock (which is more or
> > less fixed, but has a programmable divider) and an externally supplied
> > clock, and the IP has a multiplexer on its input which allows us to select
> > between those two sources.
> >
> > If it were possible to bind both to the same clock, it wouldn't be a
> > useful configuration - nothing would be gained from doing so in terms of
> > available rates.
> >
> > What the comparison is there for is to catch the case with legacy lookups
> > where a clkdev lookup entry with a NULL connection ID results in matching
> > any connection ID passed to clk_get().  If the patch changes this, then
> > we will have a regression - and this is something which needs fixing
> > _before_ we do this "return unique clocks".
> >
> 
> Ok. It seems that we might need a clk_equal() or similar API after all.
> My understanding is that this driver is calling clk_get() twice with
> NULL for the con_id and then "extclk" in attempts to get the SoC
> internal clock and the externally supplied clock. If we're using legacy
> lookups then both clk_get() calls may map to the same clk_lookup entry
> and before Tomeu's patch that returns unique clocks the driver could
> detect this case and know that there isn't an external clock. Looking at
> arch/arm/mach-dove/common.c it seems that there is only one lookup per
> device and it has a wildcard NULL for con_id so both clk_get() calls
> here are going to find the same lookup and get a unique struct clk pointer.
> 
> Why don't we make the legacy lookup more specific and actually indicate
> "internal" for the con_id? Then the external clock would fail to be
> found, but we can detect that case and figure out that it's not due to
> probe defer, but instead due to the fact that there really isn't any
> mapping. It looks like the code is already prepared for this anyway.
> 
> 8<
> 
> From: Stephen Boyd 
> Subject: [PATCH] ARM: dove: Remove wildcard from mvebu-audio device clk lookup
> 
> This i2s driver is using the wildcard nature of clkdev lookups to
> figure out if there's an external clock or not. It does this by
> calling clk_get() twice with NULL for the con_id first and then
> "external" for the con_id the second time around and then
> compares the two pointers. With DT the wildcard feature of
> clk_get() is gone and so the driver has to handle an error from
> the second clk_get() call as meaning "no external clock
> specified". Let's use that logic even with clk lookups to
> simplify the code and remove the struct clk pointer comparisons
> which may not work in the future when clk_get() returns unique
> pointers.
> 
> Signed-off-by: Stephen Boyd 

Russell et al,

I'm happy to take this patch through the clock tree (where the problem
shows up) with an ack.

Regards,
Mike

> ---
>  arch/arm/mach-dove/common.c   |  4 ++--
>  sound/soc/kirkwood/kirkwood-i2s.c | 13 -
>  2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c
> index 0d1a89298ece..f290fc944cc1 100644
> --- a/arch/arm/mach-dove/common.c
> +++ b/arch/arm/mach-dove/common.c
> @@ -124,8 +124,8 @@ static void __init dove_clk_init(void)
> orion_clkdev_add(NULL, "sdhci-dove.1", sdio1);
> orion_clkdev_add(NULL, "orion_nand", nand);
> orion_clkdev_add(NULL, "cafe1000-ccic.0", camera);
> -   orion_clkdev_add(NULL, "mvebu-audio.0", i2s0);
> -   orion_clkdev_add(NULL, "mvebu-audio.1", i2s1);
> +   orion_clkdev_add("internal", "mvebu-audio.0", i2s0);
> +   orion_clkdev_add("internal", "mvebu-audio.1", i2s1);
> orion_clkdev_add(NULL, "mv_crypto", crypto);
> orion_clkdev_add(NULL, "dove-ac97", ac97);
> orion_clkdev_add(NULL, "dove-pdma", pdma);
> diff --git a/sound/soc/kirkwood/kirkwood-i2s.c 
> b/sound/soc/kirkwood/kirkwood-i2s.c
> index def7d8260c4e..0bfeb712a997 100644
> --- a/sound/soc/kirkwood/kirkwood-i2s.c
> +++ b/sound/soc/kirkwood/kirkwood-i2s.c
> @@ -564,7 +564,7 @@ static int kirkwood_i2s_dev_probe(struct platform_device 
> *pdev)
> return -EINVAL;
> }
>  
> -   priv->clk = devm_clk_get(>dev, np ? "internal" : NULL);
> +   priv->clk = devm_clk_get(>dev, "internal");
> if (IS_ERR(priv->clk)) {
> dev_err(>dev, "no clock\n");
> return PTR_ERR(priv->clk);
> @@ 

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Stephen Boyd (2015-02-06 11:30:18)
 On 02/06/15 05:39, Russell King - ARM Linux wrote:
  On Thu, Feb 05, 2015 at 05:35:28PM -0800, Stephen Boyd wrote:
 
  From what I can tell this code is
  now broken because we made all clk getting functions (there's quite a
  few...) return unique pointers every time they're called. It seems that
  the driver wants to know if extclk and clk are the same so it can do
  something differently in kirkwood_set_rate(). Do we need some sort of
  clk_equal(struct clk *a, struct clk *b) function for drivers like this?
  Well, the clocks in question are the SoC internal clock (which is more or
  less fixed, but has a programmable divider) and an externally supplied
  clock, and the IP has a multiplexer on its input which allows us to select
  between those two sources.
 
  If it were possible to bind both to the same clock, it wouldn't be a
  useful configuration - nothing would be gained from doing so in terms of
  available rates.
 
  What the comparison is there for is to catch the case with legacy lookups
  where a clkdev lookup entry with a NULL connection ID results in matching
  any connection ID passed to clk_get().  If the patch changes this, then
  we will have a regression - and this is something which needs fixing
  _before_ we do this return unique clocks.
 
 
 Ok. It seems that we might need a clk_equal() or similar API after all.
 My understanding is that this driver is calling clk_get() twice with
 NULL for the con_id and then extclk in attempts to get the SoC
 internal clock and the externally supplied clock. If we're using legacy
 lookups then both clk_get() calls may map to the same clk_lookup entry
 and before Tomeu's patch that returns unique clocks the driver could
 detect this case and know that there isn't an external clock. Looking at
 arch/arm/mach-dove/common.c it seems that there is only one lookup per
 device and it has a wildcard NULL for con_id so both clk_get() calls
 here are going to find the same lookup and get a unique struct clk pointer.
 
 Why don't we make the legacy lookup more specific and actually indicate
 internal for the con_id? Then the external clock would fail to be
 found, but we can detect that case and figure out that it's not due to
 probe defer, but instead due to the fact that there really isn't any
 mapping. It looks like the code is already prepared for this anyway.
 
 8
 
 From: Stephen Boyd sb...@codeaurora.org
 Subject: [PATCH] ARM: dove: Remove wildcard from mvebu-audio device clk lookup
 
 This i2s driver is using the wildcard nature of clkdev lookups to
 figure out if there's an external clock or not. It does this by
 calling clk_get() twice with NULL for the con_id first and then
 external for the con_id the second time around and then
 compares the two pointers. With DT the wildcard feature of
 clk_get() is gone and so the driver has to handle an error from
 the second clk_get() call as meaning no external clock
 specified. Let's use that logic even with clk lookups to
 simplify the code and remove the struct clk pointer comparisons
 which may not work in the future when clk_get() returns unique
 pointers.
 
 Signed-off-by: Stephen Boyd sb...@codeaurora.org

Russell et al,

I'm happy to take this patch through the clock tree (where the problem
shows up) with an ack.

Regards,
Mike

 ---
  arch/arm/mach-dove/common.c   |  4 ++--
  sound/soc/kirkwood/kirkwood-i2s.c | 13 -
  2 files changed, 6 insertions(+), 11 deletions(-)
 
 diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c
 index 0d1a89298ece..f290fc944cc1 100644
 --- a/arch/arm/mach-dove/common.c
 +++ b/arch/arm/mach-dove/common.c
 @@ -124,8 +124,8 @@ static void __init dove_clk_init(void)
 orion_clkdev_add(NULL, sdhci-dove.1, sdio1);
 orion_clkdev_add(NULL, orion_nand, nand);
 orion_clkdev_add(NULL, cafe1000-ccic.0, camera);
 -   orion_clkdev_add(NULL, mvebu-audio.0, i2s0);
 -   orion_clkdev_add(NULL, mvebu-audio.1, i2s1);
 +   orion_clkdev_add(internal, mvebu-audio.0, i2s0);
 +   orion_clkdev_add(internal, mvebu-audio.1, i2s1);
 orion_clkdev_add(NULL, mv_crypto, crypto);
 orion_clkdev_add(NULL, dove-ac97, ac97);
 orion_clkdev_add(NULL, dove-pdma, pdma);
 diff --git a/sound/soc/kirkwood/kirkwood-i2s.c 
 b/sound/soc/kirkwood/kirkwood-i2s.c
 index def7d8260c4e..0bfeb712a997 100644
 --- a/sound/soc/kirkwood/kirkwood-i2s.c
 +++ b/sound/soc/kirkwood/kirkwood-i2s.c
 @@ -564,7 +564,7 @@ static int kirkwood_i2s_dev_probe(struct platform_device 
 *pdev)
 return -EINVAL;
 }
  
 -   priv-clk = devm_clk_get(pdev-dev, np ? internal : NULL);
 +   priv-clk = devm_clk_get(pdev-dev, internal);
 if (IS_ERR(priv-clk)) {
 dev_err(pdev-dev, no clock\n);
 return PTR_ERR(priv-clk);
 @@ -579,14 +579,9 @@ static int kirkwood_i2s_dev_probe(struct platform_device 
 *pdev)
 if 

Re: [PATCH 13/13] mfd: Add support for the MediaTek MT6397 PMIC

[Mike Turquette]

Quoting Lee Jones (2015-02-19 04:13:04)
 On Thu, 19 Feb 2015, Sascha Hauer wrote:
 
  On Thu, Feb 19, 2015 at 08:43:49AM +, Lee Jones wrote:
   On Thu, 19 Feb 2015, Sascha Hauer wrote:
   
  Looks okay to me now.
  
  Acked-by: Lee Jones lee.jo...@linaro.org
  
  What's the merge plan for this set?
 
 Patches 1-9 are clock related an several of them have review comments
 that need to be addressed. I wonder if a V2 series can break out the
 various subsystems bits from each other?

I'll send a new series later this day. These used to be two series, but
the PMIC wrapper patches depend on the clock and reset controllers, also
the device nodes depend on the clock/reset defines from the clock
support patches. What do you suggest? In the early days of a SoC
everything seems to depend on everything.
   
   Only build dependencies count.  So long as the Kconfigs are setup
   correct, there shouldn't be any issue in taking patches in one
   subsystem at a time.
  
  The dts snippets need the files in include/dt-bindings, so indeed this
  is a build dependency. However, this comes only in with the dts changes.
  
  So here's the plan:
  
  - Mike takes the clk patches
  - Matthias takes the pmic wrapper driver (in drivers/soc/mediatek/)
  - You take the MT6397 core driver.
 
 Sounds reasonable.  Just ensure that each set is orthogonal and builds
 (or doesn't attempt to) and we'll be in a good place.

Agreed.

Regards,
Mike

 
  I'll queue up the dts changes locally and ask Arnd to take these after
  next -rc1 so that all dependencies are in. Unfortunately this means that
  the patches can't be tested until everything is together after next
  -rc1.
 
 I'm sure you will be diligent enough to test the interoperability of
 the sets combined.  Failing that we can deal with any unavoidable
 fall-out during the -rcs.
 
 -- 
 Lee Jones
 Linaro STMicroelectronics Landing Team Lead
 Linaro.org │ Open source software for ARM SoCs
 Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: Only recalculate the rate if needed

[Mike Turquette]

Quoting Tomeu Vizoso (2015-02-06 06:13:01)
 We don't really need to recalculate the effective rate of a clock when a
 per-user clock is removed, if the constraints of the later aren't
 limiting the requested rate.
 
 This was causing problems with clocks that never had a rate set before,
 as rate_req would be zero. Though this could be considered a bug in the
 implementation of those clocks, this should be checked somewhere else.
 
 Cc: Thierry Reding thierry.red...@gmail.com
 Cc: Peter De Schrijver pdeschrij...@nvidia.com
 Signed-off-by: Tomeu Vizoso tomeu.viz...@collabora.com

Fixes: 1c8e600440c7 (clk: Add rate constraints to clocks)

Applied to clk-next. With this fix sunxi no longer vomits WARNs
everywhere due to divide-by-zero in the following path:

of_clk_init - parent_ready - __clk_put

Thanks,
Mike

 
 ---
 
 This applies on top of https://lkml.org/lkml/2015/2/5/769
 ---
  drivers/clk/clk.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index a7d37c3..4ea2d53 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -2664,7 +2664,11 @@ void __clk_put(struct clk *clk)
 clk_prepare_lock();
  
 hlist_del(clk-clks_node);
 -   clk_core_set_rate_nolock(clk-core, clk-core-req_rate);
 +
 +   if (clk-min_rate  clk-core-req_rate ||
 +   clk-max_rate  clk-core-req_rate)
 +   clk_core_set_rate_nolock(clk-core, clk-core-req_rate);
 +
 owner = clk-core-owner;
 kref_put(clk-core-ref, __clk_release);
  
 -- 
 1.9.3
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Revert "clk: mxs: Fix invalid 32-bit access to frac registers"

[Mike Turquette]

Quoting Fabio Estevam (2015-02-12 13:12:37)
> On Thu, Feb 12, 2015 at 6:30 PM, Stefan Wahren  wrote:
> > Revert commit 039e59707507 (clk: mxs: Fix invalid 32-bit access to frac
> > registers), because it leads to a faulty spi communication on mx28evk.
> >
> > Signed-off-by: Stefan Wahren 
> > Reported-by: Fabio Estevam 
> 
> Thanks, Stefan
> 
> Tested-by: Fabio Estevam 

Applied to clk-next.

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] MIPS: Alchemy: Remove bogus args from alchemy_clk_fgcs_detr

[Mike Turquette]

Quoting Stephen Boyd (2015-02-13 08:39:10)
> On 02/13/15 05:34, Tomeu Vizoso wrote:
> > They were added to this function by mistake when they were added to the
> > clk_ops.determine_rate callback.
> >
> > Fixes: 1c8e600440c7 ("clk: Add rate constraints to clocks")
> > Signed-off-by: Tomeu Vizoso 
> 
> Reported-by: kbuild test robot 
> Reviewed-by: Stephen Boyd 

Applied to clk-next.

Regards,
Mike

> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] MIPS: Alchemy: Remove bogus args from alchemy_clk_fgcs_detr

[Mike Turquette]

Quoting Stephen Boyd (2015-02-13 08:39:10)
 On 02/13/15 05:34, Tomeu Vizoso wrote:
  They were added to this function by mistake when they were added to the
  clk_ops.determine_rate callback.
 
  Fixes: 1c8e600440c7 (clk: Add rate constraints to clocks)
  Signed-off-by: Tomeu Vizoso tomeu.viz...@collabora.com
 
 Reported-by: kbuild test robot fengguang...@intel.com
 Reviewed-by: Stephen Boyd sb...@codeaurora.org

Applied to clk-next.

Regards,
Mike

 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Revert clk: mxs: Fix invalid 32-bit access to frac registers

[Mike Turquette]

Quoting Fabio Estevam (2015-02-12 13:12:37)
 On Thu, Feb 12, 2015 at 6:30 PM, Stefan Wahren stefan.wah...@i2se.com wrote:
  Revert commit 039e59707507 (clk: mxs: Fix invalid 32-bit access to frac
  registers), because it leads to a faulty spi communication on mx28evk.
 
  Signed-off-by: Stefan Wahren stefan.wah...@i2se.com
  Reported-by: Fabio Estevam fabio.este...@freescale.com
 
 Thanks, Stefan
 
 Tested-by: Fabio Estevam fabio.este...@freescale.com

Applied to clk-next.

Regards,
Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/3] clkdev: Always allocate a struct clk and call __clk_get() w/ CCF

[Mike Turquette]

Quoting Stephen Boyd (2015-02-06 11:42:43)
> of_clk_get_by_clkspec() returns a struct clk pointer but it
> doesn't create a new handle for the consumers when we're using
> the common clock framework. Instead it just returns whatever the
> clk provider hands out. When the consumers go to call clk_put()
> we get an Oops.

Applied to clk-next.

Regards,
Mike

> 
> Unable to handle kernel paging request at virtual address 00200200
> pgd = c0004000
> [00200200] *pgd=
> Internal error: Oops: 805 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc1-00104-ga251361a-dirty 
> #992
> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> task: ee00b000 ti: ee088000 task.ti: ee088000
> PC is at __clk_put+0x24/0xd0
> LR is at clk_prepare_lock+0xc/0xec
> pc : []lr : []psr: 2153
> sp : ee089de8  ip :   fp : 
> r10: ee02f480  r9 : 0001  r8 : 
> r7 : ee031cc0  r6 : ee089e08  r5 :   r4 : ee02f480
> r3 : 00100100  r2 : 00200200  r1 : 091e  r0 : 0001
> Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> Control: 10c5387d  Table: 4000404a  DAC: 0015
> Process swapper/0 (pid: 1, stack limit = 0xee088238)
> Stack: (0xee089de8 to 0xee08a000)
> 9de0:   ee7c8f14 c03f0ec8 ee089e08  c0718dc8 0001
> 9e00:  c04ee0f0 ee7e0844 0001 0181 c04edb58 ee2bd320 
> 9e20:  c011dc5c ee16a1e0   c0718dc8 ee16a1e0 ee2bd1e0
> 9e40: c0641740 ee16a1e0  ee2bd320 c0718dc8 ee1d3e10 ee1d3e10 
> 9e60: c0769a88  c0718dc8   c02c3124 c02c310c ee1d3e10
> 9e80: c07b4eec  c0769a88 c02c1d0c ee1d3e10 c0769a88 ee1d3e44 
> 9ea0: c07091dc c02c1eb8  c0769a88 c02c1e2c c02c0544 ee005478 ee1676c0
> 9ec0: c0769a88 ee3a4e80 c0760ce8 c02c150c c0669b90 c0769a88 c0746cd8 c0769a88
> 9ee0: c0746cd8 ee2bc4c0 c0778c00 c02c24e0  c0746cd8 c0746cd8 c07091f0
> 9f00:  c0008944 c04f405c 0025 ee00b000 6153 c074ab00 
> 9f20:  c074ab90 6153  ef7fca5d c050860c 00b6 c0036b88
> 9f40: c065ecc4 c06bc728 0006 0006 c074ab30 ef7fca40 c0739bdc 0006
> 9f60: c0718dbc c0778c00 00b6 c0718dc8 c06ed598 c06edd64 0006 0006
> 9f80: c06ed598 c003b438  c04e64f4    
> 9fa0:  c04e64fc  c000e838    
> 9fc0:        
> 9fe0:     0013  c0c0c0c0 c0c0c0c0
> [] (__clk_put) from [] (of_clk_set_defaults+0xe0/0x2c0)
> [] (of_clk_set_defaults) from [] 
> (platform_drv_probe+0x18/0xa4)
> [] (platform_drv_probe) from [] 
> (driver_probe_device+0x10c/0x22c)
> [] (driver_probe_device) from [] 
> (__driver_attach+0x8c/0x90)
> [] (__driver_attach) from [] (bus_for_each_dev+0x54/0x88)
> [] (bus_for_each_dev) from [] (bus_add_driver+0xd4/0x1d0)
> [] (bus_add_driver) from [] (driver_register+0x78/0xf4)
> [] (driver_register) from [] (fimc_md_init+0x14/0x30)
> [] (fimc_md_init) from [] (do_one_initcall+0x80/0x1d0)
> [] (do_one_initcall) from [] 
> (kernel_init_freeable+0x108/0x1d4)
> [] (kernel_init_freeable) from [] (kernel_init+0x8/0xec)
> [] (kernel_init) from [] (ret_from_fork+0x14/0x3c)
> Code: ebfff4ae e5943014 e5942018 e353 (e5823000)
> 
> Let's create a per-user handle here so that clk_put() can
> properly unlink it and free the handle. Now that we allocate a
> clk structure here we need to free it if __clk_get() fails so
> bury the __clk_get() call in __of_clk_get_from_provider(). We
> need to handle the same problem in clk_get_sys() so export
> __clk_free_clk() to clkdev.c and do the same thing, except let's
> use a union to make this code #ifdef free.
> 
> This fixes the above crash, properly calls __clk_get() when
> of_clk_get_from_provider() is called, and cleans up the clk
> structure on the error path of clk_get_sys().
> 
> Fixes: 035a61c314eb "clk: Make clk API return per-user struct clk instances"
> Reported-by: Sylwester Nawrocki 
> Reported-by: Alban Browaeys 
> Tested-by: Sylwester Nawrocki 
> Tested-by: Alban Browaeys 
> Reviewed-by: Tomeu Vizoso 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/clk/clk.c| 18 
>  drivers/clk/clk.h| 19 -
>  drivers/clk/clkdev.c | 59 
> 
>  3 files changed, 59 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 113456030d66..5469d7714f5d 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2382,7 +2382,7 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const 
> char *dev_id,
> return clk;
>  }
>  
> -static void __clk_free_clk(struct clk *clk)
> +void __clk_free_clk(struct clk *clk)
>  {
> clk_prepare_lock();
> hlist_del(>child_node);
> @@ -2894,7 +2894,8 @@ void 

Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers

[Mike Turquette]

Quoting Marek Vasut (2015-02-10 14:07:51)
> On Tuesday, February 10, 2015 at 10:54:52 PM, Fabio Estevam wrote:
> > Hi Stefan,
> > 
> > On Tue, Feb 10, 2015 at 7:24 PM, Stefan Wahren  
> > wrote:
> > > thanks this is very helpful. I built the linux-next for my Duckbill and
> > > add the SSP2 section from imx28-evk.dts into imx28-duckbill.dts.
> > > 
> > > Without my patch i get the following for HW_CLKCTRL_FRAC0:
> > > 
> > > ./memwatch -a 0x800401B0
> > > 
> > > 0x800401b0: 0x5e5b5513
> > > 
> > > With my patch i get:
> > > 
> > > ./memwatch -a 0x800401B0
> > > 
> > > 0x800401b0: 0x5e1b5513
> > > 
> > > So it looks like a problem in my patch.
> > 
> > Yes, let's try to get the same value HW_CLKCTRL_FRAC0.
> > 
> > > I orded such a flash chip, but it will take some time until i can use it
> > > with my hardware.
> > 
> > Thanks for doing this.
> > 
> > Maybe if you find out a way to fix the calculation of
> > HW_CLKCTRL_FRAC0, then I can test it on my board.
> 
> Hi,
> 
> the difference is this 0x40 bit, right ? That's _STABLE bit, so it means
> the clock are not stable, right ? Why would that happen in the first place?

Can you dump all the registers related to all of the clocks provided by
your clock driver, both with and without your patch? Then just diff
them. Might be more deltas than just this one register.

Regards,
Mike

> 
> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/3] clkdev: Always allocate a struct clk and call __clk_get() w/ CCF

[Mike Turquette]

Quoting Stephen Boyd (2015-02-06 11:42:43)
 of_clk_get_by_clkspec() returns a struct clk pointer but it
 doesn't create a new handle for the consumers when we're using
 the common clock framework. Instead it just returns whatever the
 clk provider hands out. When the consumers go to call clk_put()
 we get an Oops.

Applied to clk-next.

Regards,
Mike

 
 Unable to handle kernel paging request at virtual address 00200200
 pgd = c0004000
 [00200200] *pgd=
 Internal error: Oops: 805 [#1] PREEMPT SMP ARM
 Modules linked in:
 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc1-00104-ga251361a-dirty 
 #992
 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 task: ee00b000 ti: ee088000 task.ti: ee088000
 PC is at __clk_put+0x24/0xd0
 LR is at clk_prepare_lock+0xc/0xec
 pc : [c03eef38]lr : [c03ec1f4]psr: 2153
 sp : ee089de8  ip :   fp : 
 r10: ee02f480  r9 : 0001  r8 : 
 r7 : ee031cc0  r6 : ee089e08  r5 :   r4 : ee02f480
 r3 : 00100100  r2 : 00200200  r1 : 091e  r0 : 0001
 Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
 Control: 10c5387d  Table: 4000404a  DAC: 0015
 Process swapper/0 (pid: 1, stack limit = 0xee088238)
 Stack: (0xee089de8 to 0xee08a000)
 9de0:   ee7c8f14 c03f0ec8 ee089e08  c0718dc8 0001
 9e00:  c04ee0f0 ee7e0844 0001 0181 c04edb58 ee2bd320 
 9e20:  c011dc5c ee16a1e0   c0718dc8 ee16a1e0 ee2bd1e0
 9e40: c0641740 ee16a1e0  ee2bd320 c0718dc8 ee1d3e10 ee1d3e10 
 9e60: c0769a88  c0718dc8   c02c3124 c02c310c ee1d3e10
 9e80: c07b4eec  c0769a88 c02c1d0c ee1d3e10 c0769a88 ee1d3e44 
 9ea0: c07091dc c02c1eb8  c0769a88 c02c1e2c c02c0544 ee005478 ee1676c0
 9ec0: c0769a88 ee3a4e80 c0760ce8 c02c150c c0669b90 c0769a88 c0746cd8 c0769a88
 9ee0: c0746cd8 ee2bc4c0 c0778c00 c02c24e0  c0746cd8 c0746cd8 c07091f0
 9f00:  c0008944 c04f405c 0025 ee00b000 6153 c074ab00 
 9f20:  c074ab90 6153  ef7fca5d c050860c 00b6 c0036b88
 9f40: c065ecc4 c06bc728 0006 0006 c074ab30 ef7fca40 c0739bdc 0006
 9f60: c0718dbc c0778c00 00b6 c0718dc8 c06ed598 c06edd64 0006 0006
 9f80: c06ed598 c003b438  c04e64f4    
 9fa0:  c04e64fc  c000e838    
 9fc0:        
 9fe0:     0013  c0c0c0c0 c0c0c0c0
 [c03eef38] (__clk_put) from [c03f0ec8] (of_clk_set_defaults+0xe0/0x2c0)
 [c03f0ec8] (of_clk_set_defaults) from [c02c3124] 
 (platform_drv_probe+0x18/0xa4)
 [c02c3124] (platform_drv_probe) from [c02c1d0c] 
 (driver_probe_device+0x10c/0x22c)
 [c02c1d0c] (driver_probe_device) from [c02c1eb8] 
 (__driver_attach+0x8c/0x90)
 [c02c1eb8] (__driver_attach) from [c02c0544] (bus_for_each_dev+0x54/0x88)
 [c02c0544] (bus_for_each_dev) from [c02c150c] (bus_add_driver+0xd4/0x1d0)
 [c02c150c] (bus_add_driver) from [c02c24e0] (driver_register+0x78/0xf4)
 [c02c24e0] (driver_register) from [c07091f0] (fimc_md_init+0x14/0x30)
 [c07091f0] (fimc_md_init) from [c0008944] (do_one_initcall+0x80/0x1d0)
 [c0008944] (do_one_initcall) from [c06edd64] 
 (kernel_init_freeable+0x108/0x1d4)
 [c06edd64] (kernel_init_freeable) from [c04e64fc] (kernel_init+0x8/0xec)
 [c04e64fc] (kernel_init) from [c000e838] (ret_from_fork+0x14/0x3c)
 Code: ebfff4ae e5943014 e5942018 e353 (e5823000)
 
 Let's create a per-user handle here so that clk_put() can
 properly unlink it and free the handle. Now that we allocate a
 clk structure here we need to free it if __clk_get() fails so
 bury the __clk_get() call in __of_clk_get_from_provider(). We
 need to handle the same problem in clk_get_sys() so export
 __clk_free_clk() to clkdev.c and do the same thing, except let's
 use a union to make this code #ifdef free.
 
 This fixes the above crash, properly calls __clk_get() when
 of_clk_get_from_provider() is called, and cleans up the clk
 structure on the error path of clk_get_sys().
 
 Fixes: 035a61c314eb clk: Make clk API return per-user struct clk instances
 Reported-by: Sylwester Nawrocki s.nawro...@samsung.com
 Reported-by: Alban Browaeys alban.browa...@gmail.com
 Tested-by: Sylwester Nawrocki s.nawro...@samsung.com
 Tested-by: Alban Browaeys pra...@yahoo.com
 Reviewed-by: Tomeu Vizoso tomeu.viz...@collabora.com
 Signed-off-by: Stephen Boyd sb...@codeaurora.org
 ---
  drivers/clk/clk.c| 18 
  drivers/clk/clk.h| 19 -
  drivers/clk/clkdev.c | 59 
 
  3 files changed, 59 insertions(+), 37 deletions(-)
 
 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index 113456030d66..5469d7714f5d 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -2382,7 +2382,7 @@ struct clk *__clk_create_clk(struct clk_hw *hw, 

Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers

[Mike Turquette]

Quoting Marek Vasut (2015-02-10 14:07:51)
 On Tuesday, February 10, 2015 at 10:54:52 PM, Fabio Estevam wrote:
  Hi Stefan,
  
  On Tue, Feb 10, 2015 at 7:24 PM, Stefan Wahren stefan.wah...@i2se.com 
  wrote:
   thanks this is very helpful. I built the linux-next for my Duckbill and
   add the SSP2 section from imx28-evk.dts into imx28-duckbill.dts.
   
   Without my patch i get the following for HW_CLKCTRL_FRAC0:
   
   ./memwatch -a 0x800401B0
   
   0x800401b0: 0x5e5b5513
   
   With my patch i get:
   
   ./memwatch -a 0x800401B0
   
   0x800401b0: 0x5e1b5513
   
   So it looks like a problem in my patch.
  
  Yes, let's try to get the same value HW_CLKCTRL_FRAC0.
  
   I orded such a flash chip, but it will take some time until i can use it
   with my hardware.
  
  Thanks for doing this.
  
  Maybe if you find out a way to fix the calculation of
  HW_CLKCTRL_FRAC0, then I can test it on my board.
 
 Hi,
 
 the difference is this 0x40 bit, right ? That's _STABLE bit, so it means
 the clock are not stable, right ? Why would that happen in the first place?

Can you dump all the registers related to all of the clocks provided by
your clock driver, both with and without your patch? Then just diff
them. Might be more deltas than just this one register.

Regards,
Mike

 
 Best regards,
 Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()

[Mike Turquette]

Quoting Laurent Pinchart (2015-02-05 09:19:14)
> Hi Sergei,
> 
> On Thursday 05 February 2015 01:14:46 Sergei Shtylyov wrote:
> > On 02/05/2015 01:04 AM, Sergei Shtylyov wrote:
> > > Anyone may call clk_round_rate() with a zero rate value, so we have to
> > > protect against that.
> > > 
> > > Signed-off-by: Geert Uytterhoeven 
> >  
> >  Acked-by: Wolfram Sang 
> >  
> >  I agree that this should not be fixed in the core because the fixup is
> >  really driver dependant.
> >  
> > >>> Dunno, zero frequency seems generally insane to me.
> > >> 
> > >> It is useful to find the lowest frequency a clock can support. Basically
> > >> it is a search for the floor frequency.
> > >> 
> > > Why not just use 1? Or are you assuming that some hardware could actually
> > > support 0 Hz?
> > 
> > Replying to myself: yes, this has happened to me, when I forgot to override
> > the EXTAL frequency in the board .dts file (default was 0).
> 
> So it was a good thing that the driver crashed, it let you find a bug ;-)
> 
> Jokes aside, a zero frequency is the usual way to find the lowest frequency, 
> but I agree that there aren't many integers between 0 and 1. Mike, do you 
> have 
> an opinion ?

Yes, I think we should support passing a zero rate for two reasons:

1) it's crazy to not sanitize a value that is passed into a function and
used as a divisor. This is not really a shortcoming of the framework.

2) during the fractional divider discussion there was the idea of making
unsigned long rate into something like millihertz. E.g. rate = 1000 is
1Hz. If we start cheating by passing a rate of 1 into .round_rate, then
we've just created a bug for ourselves if we ever move to millihertz.

Regards,
Mike

> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()

[Mike Turquette]

Quoting Laurent Pinchart (2015-02-05 09:19:14)
 Hi Sergei,
 
 On Thursday 05 February 2015 01:14:46 Sergei Shtylyov wrote:
  On 02/05/2015 01:04 AM, Sergei Shtylyov wrote:
   Anyone may call clk_round_rate() with a zero rate value, so we have to
   protect against that.
   
   Signed-off-by: Geert Uytterhoeven geert+rene...@glider.be
   
   Acked-by: Wolfram Sang wsa+rene...@sang-engineering.com
   
   I agree that this should not be fixed in the core because the fixup is
   really driver dependant.
   
   Dunno, zero frequency seems generally insane to me.
   
   It is useful to find the lowest frequency a clock can support. Basically
   it is a search for the floor frequency.
   
   Why not just use 1? Or are you assuming that some hardware could actually
   support 0 Hz?
  
  Replying to myself: yes, this has happened to me, when I forgot to override
  the EXTAL frequency in the board .dts file (default was 0).
 
 So it was a good thing that the driver crashed, it let you find a bug ;-)
 
 Jokes aside, a zero frequency is the usual way to find the lowest frequency, 
 but I agree that there aren't many integers between 0 and 1. Mike, do you 
 have 
 an opinion ?

Yes, I think we should support passing a zero rate for two reasons:

1) it's crazy to not sanitize a value that is passed into a function and
used as a divisor. This is not really a shortcoming of the framework.

2) during the fractional divider discussion there was the idea of making
unsigned long rate into something like millihertz. E.g. rate = 1000 is
1Hz. If we start cheating by passing a rate of 1 into .round_rate, then
we've just created a bug for ourselves if we ever move to millihertz.

Regards,
Mike

 
 -- 
 Regards,
 
 Laurent Pinchart
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()

[Mike Turquette]

Quoting Wolfram Sang (2015-02-04 09:32:34)
> On Wed, Feb 04, 2015 at 01:27:21PM +0100, Geert Uytterhoeven wrote:
> > Anyone may call clk_round_rate() with a zero rate value, so we have to
> > protect against that.
> > 
> > Signed-off-by: Geert Uytterhoeven 
> 
> Acked-by: Wolfram Sang 
> 
> I agree that this should not be fixed in the core because the fixup is
> really driver dependant.
> 

Applied to clk-next.

Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()

[Mike Turquette]

Quoting Sergei Shtylyov (2015-02-04 09:45:14)
> Hello.
> 
> On 02/04/2015 08:32 PM, Wolfram Sang wrote:
> 
> >> Anyone may call clk_round_rate() with a zero rate value, so we have to
> >> protect against that.
> 
> >> Signed-off-by: Geert Uytterhoeven 
> 
> > Acked-by: Wolfram Sang 
> 
> > I agree that this should not be fixed in the core because the fixup is
> > really driver dependant.
> 
> Dunno, zero frequency seems generally insane to me.

It is useful to find the lowest frequency a clock can support. Basically
it is a search for the floor frequency.

Regards,
Mike

> 
> WBR, Sergei
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()

[Mike Turquette]

Quoting Sergei Shtylyov (2015-02-04 09:45:14)
 Hello.
 
 On 02/04/2015 08:32 PM, Wolfram Sang wrote:
 
  Anyone may call clk_round_rate() with a zero rate value, so we have to
  protect against that.
 
  Signed-off-by: Geert Uytterhoeven geert+rene...@glider.be
 
  Acked-by: Wolfram Sang wsa+rene...@sang-engineering.com
 
  I agree that this should not be fixed in the core because the fixup is
  really driver dependant.
 
 Dunno, zero frequency seems generally insane to me.

It is useful to find the lowest frequency a clock can support. Basically
it is a search for the floor frequency.

Regards,
Mike

 
 WBR, Sergei
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()

[Mike Turquette]

Quoting Wolfram Sang (2015-02-04 09:32:34)
 On Wed, Feb 04, 2015 at 01:27:21PM +0100, Geert Uytterhoeven wrote:
  Anyone may call clk_round_rate() with a zero rate value, so we have to
  protect against that.
  
  Signed-off-by: Geert Uytterhoeven geert+rene...@glider.be
 
 Acked-by: Wolfram Sang wsa+rene...@sang-engineering.com
 
 I agree that this should not be fixed in the core because the fixup is
 really driver dependant.
 

Applied to clk-next.

Thanks,
Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers

[Mike Turquette]

Quoting Fabio Estevam (2015-01-30 11:28:32)
> On Fri, Jan 30, 2015 at 5:20 PM, Stefan Wahren  wrote:
> > According to i.MX23 and i.MX28 reference manual [1],[2] the fractional
> > clock control register is 32-bit wide, but is separated in 4 parts.
> > So write instructions must not apply to more than 1 part at once.
> >
> > The clk init for the i.MX28 violates this restriction and all the other
> > accesses on that register suggest that there isn't such a restriction.
> >
> > This patch restricts the access to this register to byte instructions and
> > extends the comment in the init functions.
> >
> > Btw the imx23 init now uses a R-M-W sequence just like imx28 init
> > to avoid any clock glitches.
> >
> > The changes has been tested with a i.MX23 and a i.MX28 board.
> >
> > [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf
> > [2] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
> >
> > Signed-off-by: Stefan Wahren 
> > Reviewed-by: Marek Vasut 
> 
> Reviewed-by: Fabio Estevam 

Applied to clk-next.

Thanks,
Mike

> 
> Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the clk tree with the arm-soc tree

[Mike Turquette]

Quoting Stephen Rothwell (2015-02-02 21:31:40)
> Hi Mike,
> 
> Today's linux-next merge of the clk tree got a conflict in
> arch/arm/mach-omap2/cclock3xxx_data.c between commit ca662ee7b8a8
> ("ARM: OMAP2+: Remove unused ti81xx platform init code") from the
> arm-soc tree and commit d6540b193719 ("ARM: OMAP3: remove legacy clock
> data") from the clk tree.
> 
> I fixed it up (the latter removed the file, so I did that) and can
> carry the fix as necessary (no action is required).

That is the correct fix.

Thanks,
Mike

> 
> -- 
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the clk tree with the arm-soc tree

[Mike Turquette]

Quoting Stephen Rothwell (2015-02-02 21:31:40)
 Hi Mike,
 
 Today's linux-next merge of the clk tree got a conflict in
 arch/arm/mach-omap2/cclock3xxx_data.c between commit ca662ee7b8a8
 (ARM: OMAP2+: Remove unused ti81xx platform init code) from the
 arm-soc tree and commit d6540b193719 (ARM: OMAP3: remove legacy clock
 data) from the clk tree.
 
 I fixed it up (the latter removed the file, so I did that) and can
 carry the fix as necessary (no action is required).

That is the correct fix.

Thanks,
Mike

 
 -- 
 Cheers,
 Stephen Rothwells...@canb.auug.org.au
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers

[Mike Turquette]

Quoting Fabio Estevam (2015-01-30 11:28:32)
 On Fri, Jan 30, 2015 at 5:20 PM, Stefan Wahren stefan.wah...@i2se.com wrote:
  According to i.MX23 and i.MX28 reference manual [1],[2] the fractional
  clock control register is 32-bit wide, but is separated in 4 parts.
  So write instructions must not apply to more than 1 part at once.
 
  The clk init for the i.MX28 violates this restriction and all the other
  accesses on that register suggest that there isn't such a restriction.
 
  This patch restricts the access to this register to byte instructions and
  extends the comment in the init functions.
 
  Btw the imx23 init now uses a R-M-W sequence just like imx28 init
  to avoid any clock glitches.
 
  The changes has been tested with a i.MX23 and a i.MX28 board.
 
  [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf
  [2] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
 
  Signed-off-by: Stefan Wahren stefan.wah...@i2se.com
  Reviewed-by: Marek Vasut ma...@denx.de
 
 Reviewed-by: Fabio Estevam fabio.este...@freescale.com

Applied to clk-next.

Thanks,
Mike

 
 Thanks
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 14:35:59)
> On 02/02/15 13:31, Julia Lawall wrote:
> >
> > On Mon, 2 Feb 2015, Stephen Boyd wrote:
> >
> >> Julia,
> >>
> >> Is there a way we can write a coccinelle script to check for this? The
> >> goal being to find all drivers that are comparing struct clk pointers or
> >> attempting to dereference them. There are probably other frameworks that
> >> could use the same type of check (regulator, gpiod, reset, pwm, etc.).
> >> Probably anything that has a get/put API.
> > Comparing or dereferencing pointers of a particular type should be 
> > straightforward to check for.  Is there an example of how to use the 
> > parent_index value to fix the problem?
> >
> 
> I'm not sure how to fix this case with parent_index values generically.
> I imagine it would be highly specific to the surrounding code to the
> point where we couldn't fix it automatically. For example, if it's a clk
> consumer (not provider like in this case) using an index wouldn't be the
> right fix. We would need some sort of clk API that we don't currently
> have and I would wonder why clock consumers even care to compare such
> pointers in the first place.

Ack. Is there precedent for a "Don't do that" kind of response?

Regards,
Mike

> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Tony Lindgren (2015-02-02 12:44:02)
> * Tero Kristo  [150202 11:35]:
> > On 02/01/2015 11:24 PM, Mike Turquette wrote:
> > >Quoting Tomeu Vizoso (2015-01-23 03:03:30)
> > >
> > >AFAICT this doesn't break anything, but booting on OMAP3+ results in
> > >noisy WARNs.
> > >
> > >I think the correct fix is to replace clk_bypass and clk_ref pointers
> > >with a simple integer parent_index. In fact we already have this index.
> > >See how the pointers are populated in ti_clk_register_dpll:
> > 
> > The problem is we still need to be able to get runtime parent clock rates
> > (the parent rate may change also), so simple index value is not sufficient.
> > We need a handle of some sort to the bypass/ref clocks. The DPLL code
> > generally requires knowledge of the bypass + reference clock rates to work
> > properly, as it calculates the M/N values based on these.
> > 
> > Shall I change the DPLL code to check against clk_hw pointers or what is the
> > preferred approach here? The patch at the end does this and fixes the dpll
> > related warnings.
> > 
> > Btw, the rate constraints patch broke boot for me completely, but sounds
> > like you reverted it already.
> 
> Thanks Tero, looks like your fix fixes all the issues I'm seeing with
> commit 59cf3fcf9baf. That is noisy dmesg, dpll_abe_ck not locking
> on 4430sdp, and off-idle not working for omap3.
> 
> I could not get the patch to apply, below is what I applied manually.
> 
> Mike, If possible, maybe fold this into 59cf3fcf9baf? It applies with
> some fuzz on that too. And inn that case, please feel also to add the
> following for Tomeu's patch:
> 
> Tested-by: Tony Lindgren 

Done and done. Things look good in my testing. I've pushed another
branch out to the mirrors and hopefully the autobuild/autoboot testing
will give us the green light.

This implementation can be revisited probably after 3.19 comes out if
Tero doesn't like using clk_hw directly, or if we provide a better
interface.

Thanks,
Mike

> 
> 8<
> From: Tero Kristo 
> Date: Mon, 2 Feb 2015 12:17:00 -0800
> Subject: [PATCH] ARM: OMAP3+: clock: dpll: fix logic for comparing parent 
> clocks
> 
> DPLL code uses reference and bypass clock pointers for determining runtime
> properties for these clocks, like parent clock rates.
> As clock API now returns per-user clock structs, using a global handle
> in the clock driver code does not work properly anymore. Fix this by
> using the clk_hw instead, and comparing this against the parents.
> 
> Fixes: 59cf3fcf9baf ("clk: Make clk API return per-user struct clk instances")
> Signed-off-by: Tero Kristo 
> [t...@atomide.com: updated to apply]
> Signed-off-by: Tony Lindgren 
> 
> --- a/arch/arm/mach-omap2/dpll3xxx.c
> +++ b/arch/arm/mach-omap2/dpll3xxx.c
> @@ -410,7 +410,7 @@ int omap3_noncore_dpll_enable(struct clk_hw *hw)
> struct clk_hw_omap *clk = to_clk_hw_omap(hw);
> int r;
> struct dpll_data *dd;
> -   struct clk *parent;
> +   struct clk_hw *parent;
>  
> dd = clk->dpll_data;
> if (!dd)
> @@ -427,13 +427,13 @@ int omap3_noncore_dpll_enable(struct clk_hw *hw)
> }
> }
>  
> -   parent = __clk_get_parent(hw->clk);
> +   parent = __clk_get_hw(__clk_get_parent(hw->clk));
>  
> if (__clk_get_rate(hw->clk) == __clk_get_rate(dd->clk_bypass)) {
> -   WARN_ON(parent != dd->clk_bypass);
> +   WARN_ON(parent != __clk_get_hw(dd->clk_bypass));
> r = _omap3_noncore_dpll_bypass(clk);
> } else {
> -   WARN_ON(parent != dd->clk_ref);
> +   WARN_ON(parent != __clk_get_hw(dd->clk_ref));
> r = _omap3_noncore_dpll_lock(clk);
> }
>  
> @@ -549,7 +549,8 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, 
> unsigned long rate,
> if (!dd)
> return -EINVAL;
>  
> -   if (__clk_get_parent(hw->clk) != dd->clk_ref)
> +   if (__clk_get_hw(__clk_get_parent(hw->clk)) !=
> +   __clk_get_hw(dd->clk_ref))
> return -EINVAL;
>  
> if (dd->last_rounded_rate == 0)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Tero Kristo (2015-02-02 11:32:01)
> On 02/01/2015 11:24 PM, Mike Turquette wrote:
> > Quoting Tomeu Vizoso (2015-01-23 03:03:30)
> >> Moves clock state to struct clk_core, but takes care to change as little 
> >> API as
> >> possible.
> >>
> >> struct clk_hw still has a pointer to a struct clk, which is the
> >> implementation's per-user clk instance, for backwards compatibility.
> >>
> >> The struct clk that clk_get_parent() returns isn't owned by the caller, 
> >> but by
> >> the clock implementation, so the former shouldn't call clk_put() on it.
> >>
> >> Because some boards in mach-omap2 still register clocks statically, their 
> >> clock
> >> registration had to be updated to take into account that the clock 
> >> information
> >> is stored in struct clk_core now.
> >
> > Tero, Paul & Tony,
> >
> > Tomeu's patch unveils a problem with omap3_noncore_dpll_enable and
> > struct dpll_data, namely this snippet from
> > arch/arm/mach-omap2/dpll3xxx.c:
> >
> >  parent = __clk_get_parent(hw->clk);
> >
> >  if (__clk_get_rate(hw->clk) == __clk_get_rate(dd->clk_bypass)) {
> >  WARN(parent != dd->clk_bypass,
> >  "here0, parent name is %s, bypass name is 
> > %s\n",
> >  __clk_get_name(parent), 
> > __clk_get_name(dd->clk_bypass));
> >  r = _omap3_noncore_dpll_bypass(clk);
> >  } else {
> >  WARN(parent != dd->clk_ref,
> >  "here1, parent name is %s, ref name is 
> > %s\n",
> >  __clk_get_name(parent), 
> > __clk_get_name(dd->clk_ref));
> >  r = _omap3_noncore_dpll_lock(clk);
> >  }
> >
> > struct dpll_data has members clk_ref and clk_bypass which are struct clk
> > pointers. This was always a bit of a violation of the clk.h contract
> > since drivers are not supposed to deref struct clk pointers. Now that we
> > generate unique pointers for each call to clk_get (clk_ref & clk_bypass
> > are populated by of_clk_get in ti_clk_register_dpll) then the pointer
> > comparisons above will never be equal (even if they resolve down to the
> > same struct clk_core). I added the verbose traces to the WARNs above to
> > illustrate the point: the names are always the same but the pointers
> > differ.
> >
> > AFAICT this doesn't break anything, but booting on OMAP3+ results in
> > noisy WARNs.
> >
> > I think the correct fix is to replace clk_bypass and clk_ref pointers
> > with a simple integer parent_index. In fact we already have this index.
> > See how the pointers are populated in ti_clk_register_dpll:
> 
> The problem is we still need to be able to get runtime parent clock 
> rates (the parent rate may change also), so simple index value is not 
> sufficient. We need a handle of some sort to the bypass/ref clocks. The 
> DPLL code generally requires knowledge of the bypass + reference clock 
> rates to work properly, as it calculates the M/N values based on these.

We can maybe introduce something like of_clk_get_parent_rate, as we have
analogous stuff for getting parent names and indexes. Without
introducing a new helper you could probably just do:

clk_ref = clk_get_parent_by_index(dpll_clk, 0);
ref_rate = clk_get_rate(clk_ref);

clk_bypass = clk_get_parent_by_index(dpll_clk, 1);
bypass_rate = clk_get_rate(clk_bypass);

Currently the semantics around this call are weird. It seems like it
would create a new struct clk pointer but it does not. So don't call
clk_put on clk_ref and clk_bypass yet. That might change in the future
as we iron out this brave new world that we all live in. Probably best
to leave a FIXME in there.

Stephen & Tomeu, let me know if I got any of that wrong.

> 
> Shall I change the DPLL code to check against clk_hw pointers or what is 
> the preferred approach here? The patch at the end does this and fixes 
> the dpll related warnings.

Yes, for now that is fine, but feels a bit hacky to me. I don't know
honestly, let me sleep on it. Anyways for 3.20 that is perfectly fine
but we might want to switch to something like the scheme above.

> 
> Btw, the rate constraints patch broke boot for me completely, but sounds 
> like you reverted it already.

Fixed with Stephen's patch from last week. Thanks for dealing with all
the breakage so promptly. It has helped a lot!

Regards,
Mike

> 
> -Tero
> 
> 
> 
> Aut

Re: [PATCH] clk: Add tracepoints for hardware operations

[Mike Turquette]

Quoting Steven Rostedt (2015-02-02 08:00:33)
> On Fri, 30 Jan 2015 16:16:11 -0800
> Stephen Boyd  wrote:
> 
> > It's useful to have tracepoints around operations that change the
> > hardware state so that we can debug clock hardware performance
> > and operations. Four basic types of events are supported: on/off
> > events for enable, disable, prepare, unprepare that only record
> > an event and a clock name, rate changing events for
> > clk_set_{min_,max_}rate{_range}(), phase changing events for
> > clk_set_phase() and parent changing events for clk_set_parent().
> > 
> > Cc: Steven Rostedt 
> 
> I don't see anything wrong with the implementation of the tracepoints.
> Now whether or not they are useful is up to the clk maintainer to
> decide.

Steven,

Thanks for the review. Stephen Boyd is now co-maintaining the framework
by the way.

Regards,
Mike

> 
> > Signed-off-by: Stephen Boyd 
> > ---
> >  drivers/clk/clk.c  |  32 
> >  include/trace/events/clk.h | 198 
> > +
> >  2 files changed, 230 insertions(+)
> >  create mode 100644 include/trace/events/clk.h
> > 
> 
> 
>   
> >  unlock_out:
> > @@ -861,9 +868,12 @@ static void clk_core_unprepare(struct clk_core *clk)
> >  
> >   WARN_ON(clk->enable_count > 0);
> >  
> > + trace_clk_unprepare(clk);
> > +
> >   if (clk->ops->unprepare)
> >   clk->ops->unprepare(clk->hw);
> >  
> > + trace_clk_unprepare_complete(clk);
> >   clk_core_unprepare(clk->parent);
> 
> I guess you do not care about the clk_core_unprepare time.
> 
> >  }
> >  
> > @@ -901,6 +911,8 @@ static int clk_core_prepare(struct clk_core *clk)
> >   if (ret)
> >   return ret;
> >  
> > + trace_clk_prepare(clk);
> > +
> >   if (clk->ops->prepare) {
> >   ret = clk->ops->prepare(clk->hw);
> >   if (ret) {
> > @@ -908,6 +920,8 @@ static int clk_core_prepare(struct clk_core *clk)
> >   return ret;
> >   }
> >   }
> > +
> > + trace_clk_prepare_complete(clk);
> 
> I'm curious to why you do not put the tracepoint within the if
> statement, and only show the tracepoints if the clock prepare is
> actually called. Also, if you exit out with that return, will you tools
> be OK with seeing the clk_prepare but not the clk_prepare_complete?
> 
> 
> >   }
> >  
> >   clk->prepare_count++;
> > @@ -953,9 +967,13 @@ static void clk_core_disable(struct clk_core *clk)
> >   if (--clk->enable_count > 0)
> >   return;
> >  
> > + trace_clk_disable(clk);
> > +
> >   if (clk->ops->disable)
> >   clk->ops->disable(clk->hw);
> >  
> > + trace_clk_disable_complete(clk);
> > +
> >   clk_core_disable(clk->parent);
> >  }
> >  
> > @@ -1008,6 +1026,7 @@ static int clk_core_enable(struct clk_core *clk)
> >   if (ret)
> >   return ret;
> >  
> > + trace_clk_enable(clk);
> >   if (clk->ops->enable) {
> >   ret = clk->ops->enable(clk->hw);
> >   if (ret) {
> > @@ -1015,6 +1034,7 @@ static int clk_core_enable(struct clk_core *clk)
> >   return ret;
> >   }
> >   }
> > + trace_clk_enable_complete(clk);
> 
> Same here.
> 
> -- Steve
> 
> >   }
> >  
> >   clk->enable_count++;
> > @@ -1383,6 +1403,8 @@ static struct clk_core 
> > *__clk_set_parent_before(struct clk_core *clk,
> >   unsigned long flags;
> >   struct clk_core *old_parent = clk->parent;
> >  
> > + trace_clk_set_parent(clk, parent);
> > +
> >   /*
> >* Migrate prepare state between parents and prevent race with
> >* clk_enable().
> > @@ -1427,6 +1449,8 @@ static void __clk_set_parent_after(struct clk_core 
> > *core,
> >   clk_core_disable(old_parent);
> >   clk_core_unprepare(old_parent);
> >   }
> > +
> > + trace_clk_set_parent_complete(core, parent);
> >  }
> >  
> >  static int __clk_set_parent(struct clk_core *clk, struct clk_core *parent,
> > @@ -1663,6 +1687,8 @@ static void clk_change_rate(struct clk_core *clk)
> >   else if (clk->parent)
> >   best_parent_rate = clk->parent->rate;
> >  
> > + trace_clk_set_rate(clk, clk->new_rate);
> > +
> >   if (clk->new_parent && clk->new_parent != clk->parent) {
> >   old_parent = __clk_set_parent_before(clk, clk->new_parent);
> >  
> > @@ -1681,6 +1707,8 @@ static void clk_change_rate(struct clk_core *clk)
> >   if (!skip_set_rate && clk->ops->set_rate)
> >   clk->ops->set_rate(clk->hw, clk->new_rate, best_parent_rate);
> >  
> > + trace_clk_set_rate_complete(clk, clk->new_rate);
> > +
> >   clk->rate = clk_recalc(clk, best_parent_rate);
> >  
> >   if (clk->notifier_count && old_rate != clk->rate)
> > @@ -2081,6 

Re: [PATCH v13 4/6] clk: Add rate constraints to clocks

[Mike Turquette]

Quoting Tony Lindgren (2015-02-02 08:12:37)
> * Geert Uytterhoeven  [150202 00:03]:
> > On Sun, Feb 1, 2015 at 11:18 PM, Mike Turquette  
> > wrote:
> > > Quoting Tomeu Vizoso (2015-01-31 10:36:22)
> > >> On 31 January 2015 at 02:31, Stephen Boyd  wrote:
> > >> > On 01/29, Stephen Boyd wrote:
> > >> >> On 01/29/15 05:31, Geert Uytterhoeven wrote:
> > >> >> > Hi Tomeu, Mike,
> > >> >> >
> > >> >> > On Fri, Jan 23, 2015 at 12:03 PM, Tomeu Vizoso
> > >> >> >  wrote:
> > >> >> >> --- a/drivers/clk/clk.c
> > >> >> >> +++ b/drivers/clk/clk.c
> > >> >> >> @@ -2391,25 +2543,24 @@ int __clk_get(struct clk *clk)
> > >> >> >> return 1;
> > >> >> >>  }
> > >> >> >>
> > >> >> >> -static void clk_core_put(struct clk_core *core)
> > >> >> >> +void __clk_put(struct clk *clk)
> > >> >> >>  {
> > >> >> >> struct module *owner;
> > >> >> >>
> > >> >> >> -   owner = core->owner;
> > >> >> >> +   if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
> > >> >> >> +   return;
> > >> >> >>
> > >> >> >> clk_prepare_lock();
> > >> >> >> -   kref_put(>ref, __clk_release);
> > >> >> >> +
> > >> >> >> +   hlist_del(>child_node);
> > >> >> >> +   clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
> > >> >> > At this point, clk->core->req_rate is still zero, causing
> > >> >> > cpg_div6_clock_round_rate() to be called with a zero "rate" 
> > >> >> > parameter,
> > >> >> > e.g. on r8a7791:
> > >> >>
> > >> >> Hmm.. I wonder if we should assign core->req_rate to be the same as
> > >> >> core->rate during __clk_init()? That would make this call to
> > >> >> clk_core_set_rate_nolock() a nop in this case.
> > >> >>
> > >> >
> > >> > Here's a patch to do this
> > >> >
> > >> > ---8<
> > >> > From: Stephen Boyd 
> > >> > Subject: [PATCH] clk: Assign a requested rate by default
> > >> >
> > >> > We need to assign a requested rate here so that we avoid
> > >> > requesting a rate of 0 on clocks when we remove clock consumers.
> > >>
> > >> Hi, this looks good to me.
> > >>
> > >> Reviewed-by: Tomeu Vizoso 
> > >
> > > It seems to fix the total boot failure on OMAPs, and hopefully does the
> > > same for SH Mobile and others. I've squashed this into Tomeu's rate
> > > constraints patch to maintain bisect.
> > 
> > Yes, it fixes shmobile. .round_rate() is now called with a sane value of 
> > rate.
> 
> Looks like next-20150202 now produces tons of the following errors,
> these from omap4:

next-20150202 is the rolled-back changes from last Friday. I removed the
clock constraints patch and in doing so also rolled back the TI clock
driver migration and clk-private.h removal patches. Those are all back
in clk-next as of last night and it looks as though they missed being
pulled into todays linux-next by a matter of minutes :-/

> 
> [   10.568206] [ cut here ]
> [   10.568206] WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:925 
> clk_disable+0x28/0x34()
> [   10.568237] Modules linked in:
> [   10.568237] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW   
> 3.19.0-rc6-next-20150202 #2037
> [   10.568237] Hardware name: Generic OMAP4 (Flattened Device Tree)
> [   10.568267] [] (unwind_backtrace) from [] 
> (show_stack+0x10/0x14)
> [   10.568267] [] (show_stack) from [] 
> (dump_stack+0x84/0x9c)
> [   10.568267] [] (dump_stack) from [] 
> (warn_slowpath_common+0x7c/0xb8)
> [   10.568298] [] (warn_slowpath_common) from [] 
> (warn_slowpath_null+0x1c/0x24)
> [   10.568298] [] (warn_slowpath_null) from [] 
> (clk_disable+0x28/0x34)
> [   10.568328] [] (clk_disable) from [] 
> (_disable_clocks+0x18/0x68)
> [   10.568328] [] (_disable_clocks) from [] 
> (_idle+0x10c/0x214)
> [   10.568328] [] (_idle) from [] (_setup+0x338/0x410)
> [   10.568359] [] (_setup) from [] 
> (omap_hwmod_for_each+0x34/0x60)
> [   10.568359] [

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Tony Lindgren (2015-02-02 12:44:02)
 * Tero Kristo t-kri...@ti.com [150202 11:35]:
  On 02/01/2015 11:24 PM, Mike Turquette wrote:
  Quoting Tomeu Vizoso (2015-01-23 03:03:30)
  
  AFAICT this doesn't break anything, but booting on OMAP3+ results in
  noisy WARNs.
  
  I think the correct fix is to replace clk_bypass and clk_ref pointers
  with a simple integer parent_index. In fact we already have this index.
  See how the pointers are populated in ti_clk_register_dpll:
  
  The problem is we still need to be able to get runtime parent clock rates
  (the parent rate may change also), so simple index value is not sufficient.
  We need a handle of some sort to the bypass/ref clocks. The DPLL code
  generally requires knowledge of the bypass + reference clock rates to work
  properly, as it calculates the M/N values based on these.
  
  Shall I change the DPLL code to check against clk_hw pointers or what is the
  preferred approach here? The patch at the end does this and fixes the dpll
  related warnings.
  
  Btw, the rate constraints patch broke boot for me completely, but sounds
  like you reverted it already.
 
 Thanks Tero, looks like your fix fixes all the issues I'm seeing with
 commit 59cf3fcf9baf. That is noisy dmesg, dpll_abe_ck not locking
 on 4430sdp, and off-idle not working for omap3.
 
 I could not get the patch to apply, below is what I applied manually.
 
 Mike, If possible, maybe fold this into 59cf3fcf9baf? It applies with
 some fuzz on that too. And inn that case, please feel also to add the
 following for Tomeu's patch:
 
 Tested-by: Tony Lindgren t...@atomide.com

Done and done. Things look good in my testing. I've pushed another
branch out to the mirrors and hopefully the autobuild/autoboot testing
will give us the green light.

This implementation can be revisited probably after 3.19 comes out if
Tero doesn't like using clk_hw directly, or if we provide a better
interface.

Thanks,
Mike

 
 8
 From: Tero Kristo t-kri...@ti.com
 Date: Mon, 2 Feb 2015 12:17:00 -0800
 Subject: [PATCH] ARM: OMAP3+: clock: dpll: fix logic for comparing parent 
 clocks
 
 DPLL code uses reference and bypass clock pointers for determining runtime
 properties for these clocks, like parent clock rates.
 As clock API now returns per-user clock structs, using a global handle
 in the clock driver code does not work properly anymore. Fix this by
 using the clk_hw instead, and comparing this against the parents.
 
 Fixes: 59cf3fcf9baf (clk: Make clk API return per-user struct clk instances)
 Signed-off-by: Tero Kristo t-kri...@ti.com
 [t...@atomide.com: updated to apply]
 Signed-off-by: Tony Lindgren t...@atomide.com
 
 --- a/arch/arm/mach-omap2/dpll3xxx.c
 +++ b/arch/arm/mach-omap2/dpll3xxx.c
 @@ -410,7 +410,7 @@ int omap3_noncore_dpll_enable(struct clk_hw *hw)
 struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 int r;
 struct dpll_data *dd;
 -   struct clk *parent;
 +   struct clk_hw *parent;
  
 dd = clk-dpll_data;
 if (!dd)
 @@ -427,13 +427,13 @@ int omap3_noncore_dpll_enable(struct clk_hw *hw)
 }
 }
  
 -   parent = __clk_get_parent(hw-clk);
 +   parent = __clk_get_hw(__clk_get_parent(hw-clk));
  
 if (__clk_get_rate(hw-clk) == __clk_get_rate(dd-clk_bypass)) {
 -   WARN_ON(parent != dd-clk_bypass);
 +   WARN_ON(parent != __clk_get_hw(dd-clk_bypass));
 r = _omap3_noncore_dpll_bypass(clk);
 } else {
 -   WARN_ON(parent != dd-clk_ref);
 +   WARN_ON(parent != __clk_get_hw(dd-clk_ref));
 r = _omap3_noncore_dpll_lock(clk);
 }
  
 @@ -549,7 +549,8 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, 
 unsigned long rate,
 if (!dd)
 return -EINVAL;
  
 -   if (__clk_get_parent(hw-clk) != dd-clk_ref)
 +   if (__clk_get_hw(__clk_get_parent(hw-clk)) !=
 +   __clk_get_hw(dd-clk_ref))
 return -EINVAL;
  
 if (dd-last_rounded_rate == 0)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Tero Kristo (2015-02-02 11:32:01)
 On 02/01/2015 11:24 PM, Mike Turquette wrote:
  Quoting Tomeu Vizoso (2015-01-23 03:03:30)
  Moves clock state to struct clk_core, but takes care to change as little 
  API as
  possible.
 
  struct clk_hw still has a pointer to a struct clk, which is the
  implementation's per-user clk instance, for backwards compatibility.
 
  The struct clk that clk_get_parent() returns isn't owned by the caller, 
  but by
  the clock implementation, so the former shouldn't call clk_put() on it.
 
  Because some boards in mach-omap2 still register clocks statically, their 
  clock
  registration had to be updated to take into account that the clock 
  information
  is stored in struct clk_core now.
 
  Tero, Paul  Tony,
 
  Tomeu's patch unveils a problem with omap3_noncore_dpll_enable and
  struct dpll_data, namely this snippet from
  arch/arm/mach-omap2/dpll3xxx.c:
 
   parent = __clk_get_parent(hw-clk);
 
   if (__clk_get_rate(hw-clk) == __clk_get_rate(dd-clk_bypass)) {
   WARN(parent != dd-clk_bypass,
   here0, parent name is %s, bypass name is 
  %s\n,
   __clk_get_name(parent), 
  __clk_get_name(dd-clk_bypass));
   r = _omap3_noncore_dpll_bypass(clk);
   } else {
   WARN(parent != dd-clk_ref,
   here1, parent name is %s, ref name is 
  %s\n,
   __clk_get_name(parent), 
  __clk_get_name(dd-clk_ref));
   r = _omap3_noncore_dpll_lock(clk);
   }
 
  struct dpll_data has members clk_ref and clk_bypass which are struct clk
  pointers. This was always a bit of a violation of the clk.h contract
  since drivers are not supposed to deref struct clk pointers. Now that we
  generate unique pointers for each call to clk_get (clk_ref  clk_bypass
  are populated by of_clk_get in ti_clk_register_dpll) then the pointer
  comparisons above will never be equal (even if they resolve down to the
  same struct clk_core). I added the verbose traces to the WARNs above to
  illustrate the point: the names are always the same but the pointers
  differ.
 
  AFAICT this doesn't break anything, but booting on OMAP3+ results in
  noisy WARNs.
 
  I think the correct fix is to replace clk_bypass and clk_ref pointers
  with a simple integer parent_index. In fact we already have this index.
  See how the pointers are populated in ti_clk_register_dpll:
 
 The problem is we still need to be able to get runtime parent clock 
 rates (the parent rate may change also), so simple index value is not 
 sufficient. We need a handle of some sort to the bypass/ref clocks. The 
 DPLL code generally requires knowledge of the bypass + reference clock 
 rates to work properly, as it calculates the M/N values based on these.

We can maybe introduce something like of_clk_get_parent_rate, as we have
analogous stuff for getting parent names and indexes. Without
introducing a new helper you could probably just do:

clk_ref = clk_get_parent_by_index(dpll_clk, 0);
ref_rate = clk_get_rate(clk_ref);

clk_bypass = clk_get_parent_by_index(dpll_clk, 1);
bypass_rate = clk_get_rate(clk_bypass);

Currently the semantics around this call are weird. It seems like it
would create a new struct clk pointer but it does not. So don't call
clk_put on clk_ref and clk_bypass yet. That might change in the future
as we iron out this brave new world that we all live in. Probably best
to leave a FIXME in there.

Stephen  Tomeu, let me know if I got any of that wrong.

 
 Shall I change the DPLL code to check against clk_hw pointers or what is 
 the preferred approach here? The patch at the end does this and fixes 
 the dpll related warnings.

Yes, for now that is fine, but feels a bit hacky to me. I don't know
honestly, let me sleep on it. Anyways for 3.20 that is perfectly fine
but we might want to switch to something like the scheme above.

 
 Btw, the rate constraints patch broke boot for me completely, but sounds 
 like you reverted it already.

Fixed with Stephen's patch from last week. Thanks for dealing with all
the breakage so promptly. It has helped a lot!

Regards,
Mike

 
 -Tero
 
 
 
 Author: Tero Kristo t-kri...@ti.com
 Date:   Mon Feb 2 17:19:17 2015 +0200
 
  ARM: OMAP3+: clock: dpll: fix logic for comparing parent clocks
 
  DPLL code uses reference and bypass clock pointers for determining 
 runtime
  properties for these clocks, like parent clock rates.
 
  As clock API now returns per-user clock structs, using a global handle
  in the clock driver code does not work properly anymore. Fix this by
  using the clk_hw instead, and comparing this against the parents.
 
  Signed-off-by: Tero Kristo t-kri...@ti.com
  Fixes: 59cf3fcf9baf (clk: Make clk API return per-user struct clk 
 instances)
 
 diff --git a/arch/arm/mach-omap2

Re: [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

[Mike Turquette]

Quoting Stephen Boyd (2015-02-02 14:35:59)
 On 02/02/15 13:31, Julia Lawall wrote:
 
  On Mon, 2 Feb 2015, Stephen Boyd wrote:
 
  Julia,
 
  Is there a way we can write a coccinelle script to check for this? The
  goal being to find all drivers that are comparing struct clk pointers or
  attempting to dereference them. There are probably other frameworks that
  could use the same type of check (regulator, gpiod, reset, pwm, etc.).
  Probably anything that has a get/put API.
  Comparing or dereferencing pointers of a particular type should be 
  straightforward to check for.  Is there an example of how to use the 
  parent_index value to fix the problem?
 
 
 I'm not sure how to fix this case with parent_index values generically.
 I imagine it would be highly specific to the surrounding code to the
 point where we couldn't fix it automatically. For example, if it's a clk
 consumer (not provider like in this case) using an index wouldn't be the
 right fix. We would need some sort of clk API that we don't currently
 have and I would wonder why clock consumers even care to compare such
 pointers in the first place.

Ack. Is there precedent for a Don't do that kind of response?

Regards,
Mike

 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v13 4/6] clk: Add rate constraints to clocks

[Mike Turquette]

Quoting Tony Lindgren (2015-02-02 08:12:37)
 * Geert Uytterhoeven ge...@linux-m68k.org [150202 00:03]:
  On Sun, Feb 1, 2015 at 11:18 PM, Mike Turquette mturque...@linaro.org 
  wrote:
   Quoting Tomeu Vizoso (2015-01-31 10:36:22)
   On 31 January 2015 at 02:31, Stephen Boyd sb...@codeaurora.org wrote:
On 01/29, Stephen Boyd wrote:
On 01/29/15 05:31, Geert Uytterhoeven wrote:
 Hi Tomeu, Mike,

 On Fri, Jan 23, 2015 at 12:03 PM, Tomeu Vizoso
 tomeu.viz...@collabora.com wrote:
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -2391,25 +2543,24 @@ int __clk_get(struct clk *clk)
 return 1;
  }

 -static void clk_core_put(struct clk_core *core)
 +void __clk_put(struct clk *clk)
  {
 struct module *owner;

 -   owner = core-owner;
 +   if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
 +   return;

 clk_prepare_lock();
 -   kref_put(core-ref, __clk_release);
 +
 +   hlist_del(clk-child_node);
 +   clk_core_set_rate_nolock(clk-core, clk-core-req_rate);
 At this point, clk-core-req_rate is still zero, causing
 cpg_div6_clock_round_rate() to be called with a zero rate 
 parameter,
 e.g. on r8a7791:
   
Hmm.. I wonder if we should assign core-req_rate to be the same as
core-rate during __clk_init()? That would make this call to
clk_core_set_rate_nolock() a nop in this case.
   
   
Here's a patch to do this
   
---8
From: Stephen Boyd sb...@codeaurora.org
Subject: [PATCH] clk: Assign a requested rate by default
   
We need to assign a requested rate here so that we avoid
requesting a rate of 0 on clocks when we remove clock consumers.
  
   Hi, this looks good to me.
  
   Reviewed-by: Tomeu Vizoso tomeu.viz...@collabora.com
  
   It seems to fix the total boot failure on OMAPs, and hopefully does the
   same for SH Mobile and others. I've squashed this into Tomeu's rate
   constraints patch to maintain bisect.
  
  Yes, it fixes shmobile. .round_rate() is now called with a sane value of 
  rate.
 
 Looks like next-20150202 now produces tons of the following errors,
 these from omap4:

next-20150202 is the rolled-back changes from last Friday. I removed the
clock constraints patch and in doing so also rolled back the TI clock
driver migration and clk-private.h removal patches. Those are all back
in clk-next as of last night and it looks as though they missed being
pulled into todays linux-next by a matter of minutes :-/

 
 [   10.568206] [ cut here ]
 [   10.568206] WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:925 
 clk_disable+0x28/0x34()
 [   10.568237] Modules linked in:
 [   10.568237] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW   
 3.19.0-rc6-next-20150202 #2037
 [   10.568237] Hardware name: Generic OMAP4 (Flattened Device Tree)
 [   10.568267] [c0015bdc] (unwind_backtrace) from [c001222c] 
 (show_stack+0x10/0x14)
 [   10.568267] [c001222c] (show_stack) from [c05d2014] 
 (dump_stack+0x84/0x9c)
 [   10.568267] [c05d2014] (dump_stack) from [c003ea90] 
 (warn_slowpath_common+0x7c/0xb8)
 [   10.568298] [c003ea90] (warn_slowpath_common) from [c003eb68] 
 (warn_slowpath_null+0x1c/0x24)
 [   10.568298] [c003eb68] (warn_slowpath_null) from [c04c1ffc] 
 (clk_disable+0x28/0x34)
 [   10.568328] [c04c1ffc] (clk_disable) from [c0025b3c] 
 (_disable_clocks+0x18/0x68)
 [   10.568328] [c0025b3c] (_disable_clocks) from [c0026f14] 
 (_idle+0x10c/0x214)
 [   10.568328] [c0026f14] (_idle) from [c0855fac] (_setup+0x338/0x410)
 [   10.568359] [c0855fac] (_setup) from [c0027360] 
 (omap_hwmod_for_each+0x34/0x60)
 [   10.568359] [c0027360] (omap_hwmod_for_each) from [c08563c4] 
 (__omap_hwmod_setup_all+0x30/0x40)
 [   10.568389] [c08563c4] (__omap_hwmod_setup_all) from [c0008a04] 
 (do_one_initcall+0x80/0x1dc)
 [   10.568389] [c0008a04] (do_one_initcall) from [c0848ea0] 
 (kernel_init_freeable+0x204/0x2d0)
 [   10.568420] [c0848ea0] (kernel_init_freeable) from [c05cdab8] 
 (kernel_init+0x8/0xec)
 [   10.568420] [c05cdab8] (kernel_init) from [c000e790] 
 (ret_from_fork+0x14/0x24)
 [   10.568420] ---[ end trace cb88537fdc8fa211 ]---

This looks like mis-matched enable/disable calls. We now have unique
struct clk pointers for every call to clk_get. I haven't yet looked
through the hwmod code but I have a feeling that we're doing something
like this:

/* enable clock */
my_clk = clk_get(...);
clk_prepare_enable(my_clk);
clk_put(my_clk);

/* do some work */
do_work();

/* disable clock */
my_clk = clk_get(...);
clk_disable_unprepare(my_clk);
clk_put(my_clk);

The above pattern no longer works since my_clk will be two different
unique pointers, but it really should be one stable pointer across the
whole usage of the clk. E.g:

/* enable clock */
my_clk = clk_get(...);
clk_prepare_enable(my_clk

Re: [PATCH] clk: Add tracepoints for hardware operations

[Mike Turquette]

Quoting Steven Rostedt (2015-02-02 08:00:33)
 On Fri, 30 Jan 2015 16:16:11 -0800
 Stephen Boyd sb...@codeaurora.org wrote:
 
  It's useful to have tracepoints around operations that change the
  hardware state so that we can debug clock hardware performance
  and operations. Four basic types of events are supported: on/off
  events for enable, disable, prepare, unprepare that only record
  an event and a clock name, rate changing events for
  clk_set_{min_,max_}rate{_range}(), phase changing events for
  clk_set_phase() and parent changing events for clk_set_parent().
  
  Cc: Steven Rostedt rost...@goodmis.org
 
 I don't see anything wrong with the implementation of the tracepoints.
 Now whether or not they are useful is up to the clk maintainer to
 decide.

Steven,

Thanks for the review. Stephen Boyd is now co-maintaining the framework
by the way.

Regards,
Mike

 
  Signed-off-by: Stephen Boyd sb...@codeaurora.org
  ---
   drivers/clk/clk.c  |  32 
   include/trace/events/clk.h | 198 
  +
   2 files changed, 230 insertions(+)
   create mode 100644 include/trace/events/clk.h
  
 
 
   
   unlock_out:
  @@ -861,9 +868,12 @@ static void clk_core_unprepare(struct clk_core *clk)
   
WARN_ON(clk-enable_count  0);
   
  + trace_clk_unprepare(clk);
  +
if (clk-ops-unprepare)
clk-ops-unprepare(clk-hw);
   
  + trace_clk_unprepare_complete(clk);
clk_core_unprepare(clk-parent);
 
 I guess you do not care about the clk_core_unprepare time.
 
   }
   
  @@ -901,6 +911,8 @@ static int clk_core_prepare(struct clk_core *clk)
if (ret)
return ret;
   
  + trace_clk_prepare(clk);
  +
if (clk-ops-prepare) {
ret = clk-ops-prepare(clk-hw);
if (ret) {
  @@ -908,6 +920,8 @@ static int clk_core_prepare(struct clk_core *clk)
return ret;
}
}
  +
  + trace_clk_prepare_complete(clk);
 
 I'm curious to why you do not put the tracepoint within the if
 statement, and only show the tracepoints if the clock prepare is
 actually called. Also, if you exit out with that return, will you tools
 be OK with seeing the clk_prepare but not the clk_prepare_complete?
 
 
}
   
clk-prepare_count++;
  @@ -953,9 +967,13 @@ static void clk_core_disable(struct clk_core *clk)
if (--clk-enable_count  0)
return;
   
  + trace_clk_disable(clk);
  +
if (clk-ops-disable)
clk-ops-disable(clk-hw);
   
  + trace_clk_disable_complete(clk);
  +
clk_core_disable(clk-parent);
   }
   
  @@ -1008,6 +1026,7 @@ static int clk_core_enable(struct clk_core *clk)
if (ret)
return ret;
   
  + trace_clk_enable(clk);
if (clk-ops-enable) {
ret = clk-ops-enable(clk-hw);
if (ret) {
  @@ -1015,6 +1034,7 @@ static int clk_core_enable(struct clk_core *clk)
return ret;
}
}
  + trace_clk_enable_complete(clk);
 
 Same here.
 
 -- Steve
 
}
   
clk-enable_count++;
  @@ -1383,6 +1403,8 @@ static struct clk_core 
  *__clk_set_parent_before(struct clk_core *clk,
unsigned long flags;
struct clk_core *old_parent = clk-parent;
   
  + trace_clk_set_parent(clk, parent);
  +
/*
 * Migrate prepare state between parents and prevent race with
 * clk_enable().
  @@ -1427,6 +1449,8 @@ static void __clk_set_parent_after(struct clk_core 
  *core,
clk_core_disable(old_parent);
clk_core_unprepare(old_parent);
}
  +
  + trace_clk_set_parent_complete(core, parent);
   }
   
   static int __clk_set_parent(struct clk_core *clk, struct clk_core *parent,
  @@ -1663,6 +1687,8 @@ static void clk_change_rate(struct clk_core *clk)
else if (clk-parent)
best_parent_rate = clk-parent-rate;
   
  + trace_clk_set_rate(clk, clk-new_rate);
  +
if (clk-new_parent  clk-new_parent != clk-parent) {
old_parent = __clk_set_parent_before(clk, clk-new_parent);
   
  @@ -1681,6 +1707,8 @@ static void clk_change_rate(struct clk_core *clk)
if (!skip_set_rate  clk-ops-set_rate)
clk-ops-set_rate(clk-hw, clk-new_rate, best_parent_rate);
   
  + trace_clk_set_rate_complete(clk, clk-new_rate);
  +
clk-rate = clk_recalc(clk, best_parent_rate);
   
if (clk-notifier_count  old_rate != clk-rate)
  @@ -2081,6 +2109,8 @@ int clk_set_phase(struct clk *clk, int degrees)
   
clk_prepare_lock();
   
  + trace_clk_set_phase(clk-core, degrees);
  +
if (!clk-core-ops-set_phase)
goto out_unlock;
   
  @@ -2090,6 +2120,8 @@ int 

Re: Clock Regression in next-20150130 caused by cb75a8fcd14e

[Mike Turquette]

Quoting Tony Lindgren (2015-01-30 17:04:44)
> Hi all,
> 
> Looks like commit cb75a8fcd14e ("clk: Add rate constraints to clocks")
> causes a regression on at least omaps where the serial console either
> does not show anything, or just prints garbage.
> 
> Reverting cb75a8fcd14e makes things work again on next-20150130.
> 
> Any ideas?

Stephen posted a patch[0] to fix this. I've squashed that into Tomeu's
commit that you reference above and my Panda board is booting fine once
again.

[0] http://lkml.kernel.org/r/<20150131013158.ga4...@codeaurora.org>

Please let me know if any other issues pop up. There are new WARNs for
OMAP3+ boards introduced by a different patch from Tomeu, but this is
really because the OMAP DPLL and FAPLL code are dereferencing struct clk
pointers when they should not be and is a separate issue from the
constraints patch (with a separate email thread).

Regards,
Mike

> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v13 4/6] clk: Add rate constraints to clocks

[Mike Turquette]

Quoting Tomeu Vizoso (2015-01-31 10:36:22)
> On 31 January 2015 at 02:31, Stephen Boyd  wrote:
> > On 01/29, Stephen Boyd wrote:
> >> On 01/29/15 05:31, Geert Uytterhoeven wrote:
> >> > Hi Tomeu, Mike,
> >> >
> >> > On Fri, Jan 23, 2015 at 12:03 PM, Tomeu Vizoso
> >> >  wrote:
> >> >> --- a/drivers/clk/clk.c
> >> >> +++ b/drivers/clk/clk.c
> >> >> @@ -2391,25 +2543,24 @@ int __clk_get(struct clk *clk)
> >> >> return 1;
> >> >>  }
> >> >>
> >> >> -static void clk_core_put(struct clk_core *core)
> >> >> +void __clk_put(struct clk *clk)
> >> >>  {
> >> >> struct module *owner;
> >> >>
> >> >> -   owner = core->owner;
> >> >> +   if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
> >> >> +   return;
> >> >>
> >> >> clk_prepare_lock();
> >> >> -   kref_put(>ref, __clk_release);
> >> >> +
> >> >> +   hlist_del(>child_node);
> >> >> +   clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
> >> > At this point, clk->core->req_rate is still zero, causing
> >> > cpg_div6_clock_round_rate() to be called with a zero "rate" parameter,
> >> > e.g. on r8a7791:
> >>
> >> Hmm.. I wonder if we should assign core->req_rate to be the same as
> >> core->rate during __clk_init()? That would make this call to
> >> clk_core_set_rate_nolock() a nop in this case.
> >>
> >
> > Here's a patch to do this
> >
> > ---8<
> > From: Stephen Boyd 
> > Subject: [PATCH] clk: Assign a requested rate by default
> >
> > We need to assign a requested rate here so that we avoid
> > requesting a rate of 0 on clocks when we remove clock consumers.
> 
> Hi, this looks good to me.
> 
> Reviewed-by: Tomeu Vizoso 

It seems to fix the total boot failure on OMAPs, and hopefully does the
same for SH Mobile and others. I've squashed this into Tomeu's rate
constraints patch to maintain bisect.

Regards,
Mike

> 
> Thanks,
> 
> Tomeu
> 
> > Signed-off-by: Stephen Boyd 
> > ---
> >  drivers/clk/clk.c | 8 +---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index a29daf9edea4..8416ed1c40be 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -2142,6 +2142,7 @@ int __clk_init(struct device *dev, struct clk 
> > *clk_user)
> > struct clk_core *orphan;
> > struct hlist_node *tmp2;
> > struct clk_core *clk;
> > +   unsigned long rate;
> >
> > if (!clk_user)
> > return -EINVAL;
> > @@ -2266,12 +2267,13 @@ int __clk_init(struct device *dev, struct clk 
> > *clk_user)
> >  * then rate is set to zero.
> >  */
> > if (clk->ops->recalc_rate)
> > -   clk->rate = clk->ops->recalc_rate(clk->hw,
> > +   rate = clk->ops->recalc_rate(clk->hw,
> > clk_core_get_rate_nolock(clk->parent));
> > else if (clk->parent)
> > -   clk->rate = clk->parent->rate;
> > +   rate = clk->parent->rate;
> > else
> > -   clk->rate = 0;
> > +   rate = 0;
> > +   clk->rate = clk->req_rate = rate;
> >
> > /*
> >  * walk the list of orphan clocks and reparent any that are 
> > children of
> > --
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/