Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-08-12 Thread Heiko Stübner
Am Dienstag, 11. August 2015, 15:34:09 schrieb Stephen Boyd:
> On 07/30, Maxime Ripard wrote:
> > On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
> > > did this lead anywhere meanwhile.
> > > 
> > > Last I remember the change to orphan handling made sunxi fail, but
> > > I'm still hoping to get this usable at some point :-)
> > 
> > To be honest, I don't know what the current status is. I haven't get
> > any news since that mail.
> > 
> > I started to move a significant portion of our clocks to
> > CLK_OF_DECLARE, but not all of them are (which probably make the
> > situation worse for the time being).
> 
> Sorry, I think we're still waiting for the critical clocks stuff
> to settle down so that Sunxi can use that instead of calling
> clk_prepare_enable() in init code. For now, I've put the first
> patch of the two into clk-next so that we can have proper orphan
> tracking. Hopefully we resolve critical clocks soon so that we
> can merge the defer part.

great :-)

For the defer-part you came up with a better solution than mine anyway, if I 
remember correctly.
--
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/2] clk: improve handling of orphan clocks

2015-08-12 Thread Heiko Stübner
Am Dienstag, 11. August 2015, 15:34:09 schrieb Stephen Boyd:
 On 07/30, Maxime Ripard wrote:
  On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
   did this lead anywhere meanwhile.
   
   Last I remember the change to orphan handling made sunxi fail, but
   I'm still hoping to get this usable at some point :-)
  
  To be honest, I don't know what the current status is. I haven't get
  any news since that mail.
  
  I started to move a significant portion of our clocks to
  CLK_OF_DECLARE, but not all of them are (which probably make the
  situation worse for the time being).
 
 Sorry, I think we're still waiting for the critical clocks stuff
 to settle down so that Sunxi can use that instead of calling
 clk_prepare_enable() in init code. For now, I've put the first
 patch of the two into clk-next so that we can have proper orphan
 tracking. Hopefully we resolve critical clocks soon so that we
 can merge the defer part.

great :-)

For the defer-part you came up with a better solution than mine anyway, if I 
remember correctly.
--
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/2] clk: improve handling of orphan clocks

2015-08-11 Thread Stephen Boyd
On 07/30, Maxime Ripard wrote:
> On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
> > 
> > did this lead anywhere meanwhile.
> > 
> > Last I remember the change to orphan handling made sunxi fail, but
> > I'm still hoping to get this usable at some point :-)
> 
> To be honest, I don't know what the current status is. I haven't get
> any news since that mail.
> 
> I started to move a significant portion of our clocks to
> CLK_OF_DECLARE, but not all of them are (which probably make the
> situation worse for the time being).
> 

Sorry, I think we're still waiting for the critical clocks stuff
to settle down so that Sunxi can use that instead of calling
clk_prepare_enable() in init code. For now, I've put the first
patch of the two into clk-next so that we can have proper orphan
tracking. Hopefully we resolve critical clocks soon so that we
can merge the defer part.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-08-11 Thread Stephen Boyd
On 07/30, Maxime Ripard wrote:
 On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
  
  did this lead anywhere meanwhile.
  
  Last I remember the change to orphan handling made sunxi fail, but
  I'm still hoping to get this usable at some point :-)
 
 To be honest, I don't know what the current status is. I haven't get
 any news since that mail.
 
 I started to move a significant portion of our clocks to
 CLK_OF_DECLARE, but not all of them are (which probably make the
 situation worse for the time being).
 

Sorry, I think we're still waiting for the critical clocks stuff
to settle down so that Sunxi can use that instead of calling
clk_prepare_enable() in init code. For now, I've put the first
patch of the two into clk-next so that we can have proper orphan
tracking. Hopefully we resolve critical clocks soon so that we
can merge the defer part.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-07-30 Thread Maxime Ripard
Hi Heiko,

On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
> > > Also we can try to see if critical clocks aren't being forced on by
> > > applying this patch and looking for clk_get() failures
> > 
> > And that shows that the CPU and DDR clocks are not protected, which
> > obviously is pretty mad.
> > 
> > I've mass converted all our probing code to use OF_CLK_DECLARE, and
> > make things work again.
> > 
> > http://code.bulix.org/5goa5j-88345?raw
> > 
> > Is this an acceptable solution?
> > 
> > We were already moving to this, I'm not really fond of doing this like
> > that, but I guess this whole debacle makes it necessary.
> 
> 
> did this lead anywhere meanwhile.
> 
> Last I remember the change to orphan handling made sunxi fail, but
> I'm still hoping to get this usable at some point :-)

To be honest, I don't know what the current status is. I haven't get
any news since that mail.

I started to move a significant portion of our clocks to
CLK_OF_DECLARE, but not all of them are (which probably make the
situation worse for the time being).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-07-30 Thread Maxime Ripard
Hi Heiko,

On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
   Also we can try to see if critical clocks aren't being forced on by
   applying this patch and looking for clk_get() failures
  
  And that shows that the CPU and DDR clocks are not protected, which
  obviously is pretty mad.
  
  I've mass converted all our probing code to use OF_CLK_DECLARE, and
  make things work again.
  
  http://code.bulix.org/5goa5j-88345?raw
  
  Is this an acceptable solution?
  
  We were already moving to this, I'm not really fond of doing this like
  that, but I guess this whole debacle makes it necessary.
 
 
 did this lead anywhere meanwhile.
 
 Last I remember the change to orphan handling made sunxi fail, but
 I'm still hoping to get this usable at some point :-)

To be honest, I don't know what the current status is. I haven't get
any news since that mail.

I started to move a significant portion of our clocks to
CLK_OF_DECLARE, but not all of them are (which probably make the
situation worse for the time being).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-07-27 Thread Heiko Stübner
Hi Maxime, Stephen,

Am Freitag, 8. Mai 2015, 12:02:47 schrieb Maxime Ripard:
> On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> > On 05/07/15 08:17, Kevin Hilman wrote:
> > > On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
wrote:
> > >> On 05/01/15 15:07, Heiko Stübner wrote:
> > >>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > > Instead I guess we could hook it less deep into clk_get_sys, like in
> > > the
> > > following patch?
> >  
> >  It looks like it will work at least, but still I'd prefer to keep the
> >  orphan check contained to clk.c. How about this compile tested only
> >  patch?
> > >>> 
> > >>> I gave this a spin on my rk3288-firefly board. It still boots, the
> > >>> clock tree looks the same and it also still defers nicely in the
> > >>> scenario I needed it for. The implementation also looks nice - and of
> > >>> course much more compact than my check in two places :-) . I don't
> > >>> know if you want to put this as follow-up on top or fold it into the
> > >>> original orphan-check, so in any case
> > >>> 
> > >>> Tested-by: Heiko Stuebner 
> > >>> Reviewed-by: Heiko Stuebner 
> > >> 
> > >> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > >> with
> > >> my patch and a note that it's based on an earlier patch from you.
> > > 
> > > It appears this has landed in linux-next in the form of 882667c1fcf1
> > > clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > for sunxi in today's linux-next[1] were bisected down to that patch.
> > > 
> > > I confirmed that reverting that commit on top of next/master gets
> > > sunxi booting again.
> > 
> > Thanks for the report. I've removed the two clk orphan patches from
> > clk-next. Would it be possible to try with next-20150507 and
> > clk_ignore_unused on the command line?
> 
> This makes it work, but it's not really an option.
> 
> > Also we can try to see if critical clocks aren't being forced on by
> > applying this patch and looking for clk_get() failures
> 
> And that shows that the CPU and DDR clocks are not protected, which
> obviously is pretty mad.
> 
> I've mass converted all our probing code to use OF_CLK_DECLARE, and
> make things work again.
> 
> http://code.bulix.org/5goa5j-88345?raw
> 
> Is this an acceptable solution?
> 
> We were already moving to this, I'm not really fond of doing this like
> that, but I guess this whole debacle makes it necessary.


did this lead anywhere meanwhile.

Last I remember the change to orphan handling made sunxi fail, but I'm still 
hoping to get this usable at some point :-)


Thanks
Heiko

--
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/2] clk: improve handling of orphan clocks

2015-07-27 Thread Heiko Stübner
Hi Maxime, Stephen,

Am Freitag, 8. Mai 2015, 12:02:47 schrieb Maxime Ripard:
 On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
  On 05/07/15 08:17, Kevin Hilman wrote:
   On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
wrote:
   On 05/01/15 15:07, Heiko Stübner wrote:
   Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
   Instead I guess we could hook it less deep into clk_get_sys, like in
   the
   following patch?
   
   It looks like it will work at least, but still I'd prefer to keep the
   orphan check contained to clk.c. How about this compile tested only
   patch?
   
   I gave this a spin on my rk3288-firefly board. It still boots, the
   clock tree looks the same and it also still defers nicely in the
   scenario I needed it for. The implementation also looks nice - and of
   course much more compact than my check in two places :-) . I don't
   know if you want to put this as follow-up on top or fold it into the
   original orphan-check, so in any case
   
   Tested-by: Heiko Stuebner he...@sntech.de
   Reviewed-by: Heiko Stuebner he...@sntech.de
   
   Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
   with
   my patch and a note that it's based on an earlier patch from you.
   
   It appears this has landed in linux-next in the form of 882667c1fcf1
   clk: prevent orphan clocks from being used.  A bunch of boot failures
   for sunxi in today's linux-next[1] were bisected down to that patch.
   
   I confirmed that reverting that commit on top of next/master gets
   sunxi booting again.
  
  Thanks for the report. I've removed the two clk orphan patches from
  clk-next. Would it be possible to try with next-20150507 and
  clk_ignore_unused on the command line?
 
 This makes it work, but it's not really an option.
 
  Also we can try to see if critical clocks aren't being forced on by
  applying this patch and looking for clk_get() failures
 
 And that shows that the CPU and DDR clocks are not protected, which
 obviously is pretty mad.
 
 I've mass converted all our probing code to use OF_CLK_DECLARE, and
 make things work again.
 
 http://code.bulix.org/5goa5j-88345?raw
 
 Is this an acceptable solution?
 
 We were already moving to this, I'm not really fond of doing this like
 that, but I guess this whole debacle makes it necessary.


did this lead anywhere meanwhile.

Last I remember the change to orphan handling made sunxi fail, but I'm still 
hoping to get this usable at some point :-)


Thanks
Heiko

--
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/2] clk: improve handling of orphan clocks

2015-05-13 Thread Maxime Ripard
On Wed, May 13, 2015 at 01:44:50PM -0700, Kevin Hilman wrote:
> On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
>  wrote:
> > On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
> >> Maxime Ripard  writes:
> >>
> >> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> >> >> On 05/08/15 03:02, Maxime Ripard wrote:
> >> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
> >> >> >>> wrote:
> >> >>  On 05/01/15 15:07, Heiko Stübner wrote:
> >> >> > Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >> >> >
> >> >> >>> Instead I guess we could hook it less deep into clk_get_sys, 
> >> >> >>> like in the
> >> >> >>> following patch?
> >> >> >> It looks like it will work at least, but still I'd prefer to 
> >> >> >> keep the
> >> >> >> orphan check contained to clk.c. How about this compile tested 
> >> >> >> only patch?
> >> >> > I gave this a spin on my rk3288-firefly board. It still boots, 
> >> >> > the clock tree
> >> >> > looks the same and it also still defers nicely in the scenario I 
> >> >> > needed it
> >> >> > for. The implementation also looks nice - and of course much more 
> >> >> > compact than
> >> >> > my check in two places :-) . I don't know if you want to put this 
> >> >> > as follow-up
> >> >> > on top or fold it into the original orphan-check, so in any case
> >> >> >
> >> >> > Tested-by: Heiko Stuebner 
> >> >> > Reviewed-by: Heiko Stuebner 
> >> >>  Thanks. I'm leaning towards tossing your patch 2/2 and replacing 
> >> >>  it with
> >> >>  my patch and a note that it's based on an earlier patch from you.
> >> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot 
> >> >> >>> failures
> >> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >> >> >>>
> >> >> >>> I confirmed that reverting that commit on top of next/master gets
> >> >> >>> sunxi booting again.
> >> >> >>>
> >> >> >>>
> >> >> >> Thanks for the report. I've removed the two clk orphan patches from
> >> >> >> clk-next. Would it be possible to try with next-20150507 and
> >> >> >> clk_ignore_unused on the command line?
> >> >> > This makes it work, but it's not really an option.
> >> >> >
> >> >>
> >> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
> >> >
> >> > I'm too, but it does fix things here.
> >>
> >> To be more precise on what I tested.  I used next-20150507 and tested on
> >> 4 different sunxi platforms.  First test was "normal" commandline,
> >> second was with clk_ignore_unused appended:
> >>
> >>   - cubie: fail, fail
> >>   - cubie2: fail, fail
> >>   - bananpi: fail, pass
> >>   - cubietruck: fail, pass
> >>
> >> So it seems to have some effect, but by itself, doesn't fix the issue.
> >
> > It's very odd, I actually tried with a cubie2 here...
> >
> > I'm booting on an initramfs and not MMC though, but I can't see how
> > that can be related to our issue...
> 
> I'm booting an initramfs too.

Then I don't know :)

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-13 Thread Kevin Hilman
On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
 wrote:
> On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
>> Maxime Ripard  writes:
>>
>> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> >> On 05/08/15 03:02, Maxime Ripard wrote:
>> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
>> >> >>> wrote:
>> >>  On 05/01/15 15:07, Heiko Stübner wrote:
>> >> > Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >> >
>> >> >>> Instead I guess we could hook it less deep into clk_get_sys, like 
>> >> >>> in the
>> >> >>> following patch?
>> >> >> It looks like it will work at least, but still I'd prefer to keep 
>> >> >> the
>> >> >> orphan check contained to clk.c. How about this compile tested 
>> >> >> only patch?
>> >> > I gave this a spin on my rk3288-firefly board. It still boots, the 
>> >> > clock tree
>> >> > looks the same and it also still defers nicely in the scenario I 
>> >> > needed it
>> >> > for. The implementation also looks nice - and of course much more 
>> >> > compact than
>> >> > my check in two places :-) . I don't know if you want to put this 
>> >> > as follow-up
>> >> > on top or fold it into the original orphan-check, so in any case
>> >> >
>> >> > Tested-by: Heiko Stuebner 
>> >> > Reviewed-by: Heiko Stuebner 
>> >>  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it 
>> >>  with
>> >>  my patch and a note that it's based on an earlier patch from you.
>> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
>> >> >>>
>> >> >>> I confirmed that reverting that commit on top of next/master gets
>> >> >>> sunxi booting again.
>> >> >>>
>> >> >>>
>> >> >> Thanks for the report. I've removed the two clk orphan patches from
>> >> >> clk-next. Would it be possible to try with next-20150507 and
>> >> >> clk_ignore_unused on the command line?
>> >> > This makes it work, but it's not really an option.
>> >> >
>> >>
>> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
>> >
>> > I'm too, but it does fix things here.
>>
>> To be more precise on what I tested.  I used next-20150507 and tested on
>> 4 different sunxi platforms.  First test was "normal" commandline,
>> second was with clk_ignore_unused appended:
>>
>>   - cubie: fail, fail
>>   - cubie2: fail, fail
>>   - bananpi: fail, pass
>>   - cubietruck: fail, pass
>>
>> So it seems to have some effect, but by itself, doesn't fix the issue.
>
> It's very odd, I actually tried with a cubie2 here...
>
> I'm booting on an initramfs and not MMC though, but I can't see how
> that can be related to our issue...

I'm booting an initramfs too.

Kevin
--
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/2] clk: improve handling of orphan clocks

2015-05-13 Thread Maxime Ripard
On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
> Maxime Ripard  writes:
> 
> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> >> On 05/08/15 03:02, Maxime Ripard wrote:
> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
> >> >>> wrote:
> >>  On 05/01/15 15:07, Heiko Stübner wrote:
> >> > Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >> >
> >> >>> Instead I guess we could hook it less deep into clk_get_sys, like 
> >> >>> in the
> >> >>> following patch?
> >> >> It looks like it will work at least, but still I'd prefer to keep 
> >> >> the
> >> >> orphan check contained to clk.c. How about this compile tested only 
> >> >> patch?
> >> > I gave this a spin on my rk3288-firefly board. It still boots, the 
> >> > clock tree
> >> > looks the same and it also still defers nicely in the scenario I 
> >> > needed it
> >> > for. The implementation also looks nice - and of course much more 
> >> > compact than
> >> > my check in two places :-) . I don't know if you want to put this as 
> >> > follow-up
> >> > on top or fold it into the original orphan-check, so in any case
> >> >
> >> > Tested-by: Heiko Stuebner 
> >> > Reviewed-by: Heiko Stuebner 
> >>  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it 
> >>  with
> >>  my patch and a note that it's based on an earlier patch from you.
> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >> >>>
> >> >>> I confirmed that reverting that commit on top of next/master gets
> >> >>> sunxi booting again.
> >> >>>
> >> >>>
> >> >> Thanks for the report. I've removed the two clk orphan patches from
> >> >> clk-next. Would it be possible to try with next-20150507 and
> >> >> clk_ignore_unused on the command line?
> >> > This makes it work, but it's not really an option.
> >> >
> >> 
> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
> >
> > I'm too, but it does fix things here.
> 
> To be more precise on what I tested.  I used next-20150507 and tested on
> 4 different sunxi platforms.  First test was "normal" commandline,
> second was with clk_ignore_unused appended:
> 
>   - cubie: fail, fail
>   - cubie2: fail, fail
>   - bananpi: fail, pass
>   - cubietruck: fail, pass
> 
> So it seems to have some effect, but by itself, doesn't fix the issue.

It's very odd, I actually tried with a cubie2 here...

I'm booting on an initramfs and not MMC though, but I can't see how
that can be related to our issue...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-13 Thread Kevin Hilman
Maxime Ripard  writes:

> On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> On 05/08/15 03:02, Maxime Ripard wrote:
>> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
>> >>> wrote:
>>  On 05/01/15 15:07, Heiko Stübner wrote:
>> > Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >
>> >>> Instead I guess we could hook it less deep into clk_get_sys, like in 
>> >>> the
>> >>> following patch?
>> >> It looks like it will work at least, but still I'd prefer to keep the
>> >> orphan check contained to clk.c. How about this compile tested only 
>> >> patch?
>> > I gave this a spin on my rk3288-firefly board. It still boots, the 
>> > clock tree
>> > looks the same and it also still defers nicely in the scenario I 
>> > needed it
>> > for. The implementation also looks nice - and of course much more 
>> > compact than
>> > my check in two places :-) . I don't know if you want to put this as 
>> > follow-up
>> > on top or fold it into the original orphan-check, so in any case
>> >
>> > Tested-by: Heiko Stuebner 
>> > Reviewed-by: Heiko Stuebner 
>>  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>  my patch and a note that it's based on an earlier patch from you.
>> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
>> >>>
>> >>> I confirmed that reverting that commit on top of next/master gets
>> >>> sunxi booting again.
>> >>>
>> >>>
>> >> Thanks for the report. I've removed the two clk orphan patches from
>> >> clk-next. Would it be possible to try with next-20150507 and
>> >> clk_ignore_unused on the command line?
>> > This makes it work, but it's not really an option.
>> >
>> 
>> Hmm.. I thought it didn't fix it for Kevin. Confused.
>
> I'm too, but it does fix things here.

To be more precise on what I tested.  I used next-20150507 and tested on
4 different sunxi platforms.  First test was "normal" commandline,
second was with clk_ignore_unused appended:

  - cubie: fail, fail
  - cubie2: fail, fail
  - bananpi: fail, pass
  - cubietruck: fail, pass

So it seems to have some effect, but by itself, doesn't fix the issue.

Kevin
--
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/2] clk: improve handling of orphan clocks

2015-05-13 Thread Maxime Ripard
On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> On 05/08/15 03:02, Maxime Ripard wrote:
> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  wrote:
>  On 05/01/15 15:07, Heiko Stübner wrote:
> > Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >
> >>> Instead I guess we could hook it less deep into clk_get_sys, like in 
> >>> the
> >>> following patch?
> >> It looks like it will work at least, but still I'd prefer to keep the
> >> orphan check contained to clk.c. How about this compile tested only 
> >> patch?
> > I gave this a spin on my rk3288-firefly board. It still boots, the 
> > clock tree
> > looks the same and it also still defers nicely in the scenario I needed 
> > it
> > for. The implementation also looks nice - and of course much more 
> > compact than
> > my check in two places :-) . I don't know if you want to put this as 
> > follow-up
> > on top or fold it into the original orphan-check, so in any case
> >
> > Tested-by: Heiko Stuebner 
> > Reviewed-by: Heiko Stuebner 
>  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>  my patch and a note that it's based on an earlier patch from you.
> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >>>
> >>> I confirmed that reverting that commit on top of next/master gets
> >>> sunxi booting again.
> >>>
> >>>
> >> Thanks for the report. I've removed the two clk orphan patches from
> >> clk-next. Would it be possible to try with next-20150507 and
> >> clk_ignore_unused on the command line?
> > This makes it work, but it's not really an option.
> >
> 
> Hmm.. I thought it didn't fix it for Kevin. Confused.

I'm too, but it does fix things here.

> >> Also we can try to see if critical clocks aren't being forced on by
> >> applying this patch and looking for clk_get() failures
> > And that shows that the CPU and DDR clocks are not protected, which
> > obviously is pretty mad.
> >
> > I've mass converted all our probing code to use OF_CLK_DECLARE, and
> > make things work again.
> >
> > http://code.bulix.org/5goa5j-88345?raw
> >
> > Is this an acceptable solution?
> >
> > We were already moving to this, I'm not really fond of doing this like
> > that, but I guess this whole debacle makes it necessary.
> >
> 
> I wonder why we can't switch out the clk_ops on the affected platforms +
> clocks to be read-only (at least for the enable/disable part)? That
> would fix it just the same right? I wasn't around for the original
> discussion regarding this always-on stuff so perhaps I've missed something.

We're using clk-gate, so that would require to recode our own
driver. That change seemed less invasive, while fixing the issue and
ensuring that we wouldn't have any orphan clock, which seems to be
hunted down these days...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-13 Thread Maxime Ripard
On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
 On 05/08/15 03:02, Maxime Ripard wrote:
  On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
  On 05/07/15 08:17, Kevin Hilman wrote:
  On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org wrote:
  On 05/01/15 15:07, Heiko Stübner wrote:
  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
 
  Instead I guess we could hook it less deep into clk_get_sys, like in 
  the
  following patch?
  It looks like it will work at least, but still I'd prefer to keep the
  orphan check contained to clk.c. How about this compile tested only 
  patch?
  I gave this a spin on my rk3288-firefly board. It still boots, the 
  clock tree
  looks the same and it also still defers nicely in the scenario I needed 
  it
  for. The implementation also looks nice - and of course much more 
  compact than
  my check in two places :-) . I don't know if you want to put this as 
  follow-up
  on top or fold it into the original orphan-check, so in any case
 
  Tested-by: Heiko Stuebner he...@sntech.de
  Reviewed-by: Heiko Stuebner he...@sntech.de
  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
  my patch and a note that it's based on an earlier patch from you.
  It appears this has landed in linux-next in the form of 882667c1fcf1
  clk: prevent orphan clocks from being used.  A bunch of boot failures
  for sunxi in today's linux-next[1] were bisected down to that patch.
 
  I confirmed that reverting that commit on top of next/master gets
  sunxi booting again.
 
 
  Thanks for the report. I've removed the two clk orphan patches from
  clk-next. Would it be possible to try with next-20150507 and
  clk_ignore_unused on the command line?
  This makes it work, but it's not really an option.
 
 
 Hmm.. I thought it didn't fix it for Kevin. Confused.

I'm too, but it does fix things here.

  Also we can try to see if critical clocks aren't being forced on by
  applying this patch and looking for clk_get() failures
  And that shows that the CPU and DDR clocks are not protected, which
  obviously is pretty mad.
 
  I've mass converted all our probing code to use OF_CLK_DECLARE, and
  make things work again.
 
  http://code.bulix.org/5goa5j-88345?raw
 
  Is this an acceptable solution?
 
  We were already moving to this, I'm not really fond of doing this like
  that, but I guess this whole debacle makes it necessary.
 
 
 I wonder why we can't switch out the clk_ops on the affected platforms +
 clocks to be read-only (at least for the enable/disable part)? That
 would fix it just the same right? I wasn't around for the original
 discussion regarding this always-on stuff so perhaps I've missed something.

We're using clk-gate, so that would require to recode our own
driver. That change seemed less invasive, while fixing the issue and
ensuring that we wouldn't have any orphan clock, which seems to be
hunted down these days...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-13 Thread Kevin Hilman
Maxime Ripard maxime.rip...@free-electrons.com writes:

 On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
 On 05/08/15 03:02, Maxime Ripard wrote:
  On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
  On 05/07/15 08:17, Kevin Hilman wrote:
  On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
  wrote:
  On 05/01/15 15:07, Heiko Stübner wrote:
  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
 
  Instead I guess we could hook it less deep into clk_get_sys, like in 
  the
  following patch?
  It looks like it will work at least, but still I'd prefer to keep the
  orphan check contained to clk.c. How about this compile tested only 
  patch?
  I gave this a spin on my rk3288-firefly board. It still boots, the 
  clock tree
  looks the same and it also still defers nicely in the scenario I 
  needed it
  for. The implementation also looks nice - and of course much more 
  compact than
  my check in two places :-) . I don't know if you want to put this as 
  follow-up
  on top or fold it into the original orphan-check, so in any case
 
  Tested-by: Heiko Stuebner he...@sntech.de
  Reviewed-by: Heiko Stuebner he...@sntech.de
  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
  my patch and a note that it's based on an earlier patch from you.
  It appears this has landed in linux-next in the form of 882667c1fcf1
  clk: prevent orphan clocks from being used.  A bunch of boot failures
  for sunxi in today's linux-next[1] were bisected down to that patch.
 
  I confirmed that reverting that commit on top of next/master gets
  sunxi booting again.
 
 
  Thanks for the report. I've removed the two clk orphan patches from
  clk-next. Would it be possible to try with next-20150507 and
  clk_ignore_unused on the command line?
  This makes it work, but it's not really an option.
 
 
 Hmm.. I thought it didn't fix it for Kevin. Confused.

 I'm too, but it does fix things here.

To be more precise on what I tested.  I used next-20150507 and tested on
4 different sunxi platforms.  First test was normal commandline,
second was with clk_ignore_unused appended:

  - cubie: fail, fail
  - cubie2: fail, fail
  - bananpi: fail, pass
  - cubietruck: fail, pass

So it seems to have some effect, but by itself, doesn't fix the issue.

Kevin
--
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/2] clk: improve handling of orphan clocks

2015-05-13 Thread Maxime Ripard
On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
 Maxime Ripard maxime.rip...@free-electrons.com writes:
 
  On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
  On 05/08/15 03:02, Maxime Ripard wrote:
   On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
   On 05/07/15 08:17, Kevin Hilman wrote:
   On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
   wrote:
   On 05/01/15 15:07, Heiko Stübner wrote:
   Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
  
   Instead I guess we could hook it less deep into clk_get_sys, like 
   in the
   following patch?
   It looks like it will work at least, but still I'd prefer to keep 
   the
   orphan check contained to clk.c. How about this compile tested only 
   patch?
   I gave this a spin on my rk3288-firefly board. It still boots, the 
   clock tree
   looks the same and it also still defers nicely in the scenario I 
   needed it
   for. The implementation also looks nice - and of course much more 
   compact than
   my check in two places :-) . I don't know if you want to put this as 
   follow-up
   on top or fold it into the original orphan-check, so in any case
  
   Tested-by: Heiko Stuebner he...@sntech.de
   Reviewed-by: Heiko Stuebner he...@sntech.de
   Thanks. I'm leaning towards tossing your patch 2/2 and replacing it 
   with
   my patch and a note that it's based on an earlier patch from you.
   It appears this has landed in linux-next in the form of 882667c1fcf1
   clk: prevent orphan clocks from being used.  A bunch of boot failures
   for sunxi in today's linux-next[1] were bisected down to that patch.
  
   I confirmed that reverting that commit on top of next/master gets
   sunxi booting again.
  
  
   Thanks for the report. I've removed the two clk orphan patches from
   clk-next. Would it be possible to try with next-20150507 and
   clk_ignore_unused on the command line?
   This makes it work, but it's not really an option.
  
  
  Hmm.. I thought it didn't fix it for Kevin. Confused.
 
  I'm too, but it does fix things here.
 
 To be more precise on what I tested.  I used next-20150507 and tested on
 4 different sunxi platforms.  First test was normal commandline,
 second was with clk_ignore_unused appended:
 
   - cubie: fail, fail
   - cubie2: fail, fail
   - bananpi: fail, pass
   - cubietruck: fail, pass
 
 So it seems to have some effect, but by itself, doesn't fix the issue.

It's very odd, I actually tried with a cubie2 here...

I'm booting on an initramfs and not MMC though, but I can't see how
that can be related to our issue...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-13 Thread Maxime Ripard
On Wed, May 13, 2015 at 01:44:50PM -0700, Kevin Hilman wrote:
 On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
 maxime.rip...@free-electrons.com wrote:
  On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
  Maxime Ripard maxime.rip...@free-electrons.com writes:
 
   On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
   On 05/08/15 03:02, Maxime Ripard wrote:
On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
On 05/07/15 08:17, Kevin Hilman wrote:
On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
wrote:
On 05/01/15 15:07, Heiko Stübner wrote:
Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
   
Instead I guess we could hook it less deep into clk_get_sys, 
like in the
following patch?
It looks like it will work at least, but still I'd prefer to 
keep the
orphan check contained to clk.c. How about this compile tested 
only patch?
I gave this a spin on my rk3288-firefly board. It still boots, 
the clock tree
looks the same and it also still defers nicely in the scenario I 
needed it
for. The implementation also looks nice - and of course much more 
compact than
my check in two places :-) . I don't know if you want to put this 
as follow-up
on top or fold it into the original orphan-check, so in any case
   
Tested-by: Heiko Stuebner he...@sntech.de
Reviewed-by: Heiko Stuebner he...@sntech.de
Thanks. I'm leaning towards tossing your patch 2/2 and replacing 
it with
my patch and a note that it's based on an earlier patch from you.
It appears this has landed in linux-next in the form of 882667c1fcf1
clk: prevent orphan clocks from being used.  A bunch of boot 
failures
for sunxi in today's linux-next[1] were bisected down to that patch.
   
I confirmed that reverting that commit on top of next/master gets
sunxi booting again.
   
   
Thanks for the report. I've removed the two clk orphan patches from
clk-next. Would it be possible to try with next-20150507 and
clk_ignore_unused on the command line?
This makes it work, but it's not really an option.
   
  
   Hmm.. I thought it didn't fix it for Kevin. Confused.
  
   I'm too, but it does fix things here.
 
  To be more precise on what I tested.  I used next-20150507 and tested on
  4 different sunxi platforms.  First test was normal commandline,
  second was with clk_ignore_unused appended:
 
- cubie: fail, fail
- cubie2: fail, fail
- bananpi: fail, pass
- cubietruck: fail, pass
 
  So it seems to have some effect, but by itself, doesn't fix the issue.
 
  It's very odd, I actually tried with a cubie2 here...
 
  I'm booting on an initramfs and not MMC though, but I can't see how
  that can be related to our issue...
 
 I'm booting an initramfs too.

Then I don't know :)

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-13 Thread Kevin Hilman
On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
maxime.rip...@free-electrons.com wrote:
 On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
 Maxime Ripard maxime.rip...@free-electrons.com writes:

  On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
  On 05/08/15 03:02, Maxime Ripard wrote:
   On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
   On 05/07/15 08:17, Kevin Hilman wrote:
   On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
   wrote:
   On 05/01/15 15:07, Heiko Stübner wrote:
   Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
  
   Instead I guess we could hook it less deep into clk_get_sys, like 
   in the
   following patch?
   It looks like it will work at least, but still I'd prefer to keep 
   the
   orphan check contained to clk.c. How about this compile tested 
   only patch?
   I gave this a spin on my rk3288-firefly board. It still boots, the 
   clock tree
   looks the same and it also still defers nicely in the scenario I 
   needed it
   for. The implementation also looks nice - and of course much more 
   compact than
   my check in two places :-) . I don't know if you want to put this 
   as follow-up
   on top or fold it into the original orphan-check, so in any case
  
   Tested-by: Heiko Stuebner he...@sntech.de
   Reviewed-by: Heiko Stuebner he...@sntech.de
   Thanks. I'm leaning towards tossing your patch 2/2 and replacing it 
   with
   my patch and a note that it's based on an earlier patch from you.
   It appears this has landed in linux-next in the form of 882667c1fcf1
   clk: prevent orphan clocks from being used.  A bunch of boot failures
   for sunxi in today's linux-next[1] were bisected down to that patch.
  
   I confirmed that reverting that commit on top of next/master gets
   sunxi booting again.
  
  
   Thanks for the report. I've removed the two clk orphan patches from
   clk-next. Would it be possible to try with next-20150507 and
   clk_ignore_unused on the command line?
   This makes it work, but it's not really an option.
  
 
  Hmm.. I thought it didn't fix it for Kevin. Confused.
 
  I'm too, but it does fix things here.

 To be more precise on what I tested.  I used next-20150507 and tested on
 4 different sunxi platforms.  First test was normal commandline,
 second was with clk_ignore_unused appended:

   - cubie: fail, fail
   - cubie2: fail, fail
   - bananpi: fail, pass
   - cubietruck: fail, pass

 So it seems to have some effect, but by itself, doesn't fix the issue.

 It's very odd, I actually tried with a cubie2 here...

 I'm booting on an initramfs and not MMC though, but I can't see how
 that can be related to our issue...

I'm booting an initramfs too.

Kevin
--
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/2] clk: improve handling of orphan clocks

2015-05-12 Thread Stephen Boyd
On 05/08/15 03:02, Maxime Ripard wrote:
> On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> On 05/07/15 08:17, Kevin Hilman wrote:
>>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  wrote:
 On 05/01/15 15:07, Heiko Stübner wrote:
> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>
>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>> following patch?
>> It looks like it will work at least, but still I'd prefer to keep the
>> orphan check contained to clk.c. How about this compile tested only 
>> patch?
> I gave this a spin on my rk3288-firefly board. It still boots, the clock 
> tree
> looks the same and it also still defers nicely in the scenario I needed it
> for. The implementation also looks nice - and of course much more compact 
> than
> my check in two places :-) . I don't know if you want to put this as 
> follow-up
> on top or fold it into the original orphan-check, so in any case
>
> Tested-by: Heiko Stuebner 
> Reviewed-by: Heiko Stuebner 
 Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
 my patch and a note that it's based on an earlier patch from you.
>>> It appears this has landed in linux-next in the form of 882667c1fcf1
>>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>>> for sunxi in today's linux-next[1] were bisected down to that patch.
>>>
>>> I confirmed that reverting that commit on top of next/master gets
>>> sunxi booting again.
>>>
>>>
>> Thanks for the report. I've removed the two clk orphan patches from
>> clk-next. Would it be possible to try with next-20150507 and
>> clk_ignore_unused on the command line?
> This makes it work, but it's not really an option.
>

Hmm.. I thought it didn't fix it for Kevin. Confused.

>> Also we can try to see if critical clocks aren't being forced on by
>> applying this patch and looking for clk_get() failures
> And that shows that the CPU and DDR clocks are not protected, which
> obviously is pretty mad.
>
> I've mass converted all our probing code to use OF_CLK_DECLARE, and
> make things work again.
>
> http://code.bulix.org/5goa5j-88345?raw
>
> Is this an acceptable solution?
>
> We were already moving to this, I'm not really fond of doing this like
> that, but I guess this whole debacle makes it necessary.
>

I wonder why we can't switch out the clk_ops on the affected platforms +
clocks to be read-only (at least for the enable/disable part)? That
would fix it just the same right? I wasn't around for the original
discussion regarding this always-on stuff so perhaps I've missed something.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-12 Thread Stephen Boyd
On 05/08/15 03:02, Maxime Ripard wrote:
 On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
 On 05/07/15 08:17, Kevin Hilman wrote:
 On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org wrote:
 On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:

 Instead I guess we could hook it less deep into clk_get_sys, like in the
 following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only 
 patch?
 I gave this a spin on my rk3288-firefly board. It still boots, the clock 
 tree
 looks the same and it also still defers nicely in the scenario I needed it
 for. The implementation also looks nice - and of course much more compact 
 than
 my check in two places :-) . I don't know if you want to put this as 
 follow-up
 on top or fold it into the original orphan-check, so in any case

 Tested-by: Heiko Stuebner he...@sntech.de
 Reviewed-by: Heiko Stuebner he...@sntech.de
 Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
 my patch and a note that it's based on an earlier patch from you.
 It appears this has landed in linux-next in the form of 882667c1fcf1
 clk: prevent orphan clocks from being used.  A bunch of boot failures
 for sunxi in today's linux-next[1] were bisected down to that patch.

 I confirmed that reverting that commit on top of next/master gets
 sunxi booting again.


 Thanks for the report. I've removed the two clk orphan patches from
 clk-next. Would it be possible to try with next-20150507 and
 clk_ignore_unused on the command line?
 This makes it work, but it's not really an option.


Hmm.. I thought it didn't fix it for Kevin. Confused.

 Also we can try to see if critical clocks aren't being forced on by
 applying this patch and looking for clk_get() failures
 And that shows that the CPU and DDR clocks are not protected, which
 obviously is pretty mad.

 I've mass converted all our probing code to use OF_CLK_DECLARE, and
 make things work again.

 http://code.bulix.org/5goa5j-88345?raw

 Is this an acceptable solution?

 We were already moving to this, I'm not really fond of doing this like
 that, but I guess this whole debacle makes it necessary.


I wonder why we can't switch out the clk_ops on the affected platforms +
clocks to be read-only (at least for the enable/disable part)? That
would fix it just the same right? I wasn't around for the original
discussion regarding this always-on stuff so perhaps I've missed something.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Tero Kristo

On 05/07/2015 09:18 PM, Stephen Boyd wrote:

On 05/07/15 01:22, Tero Kristo wrote:

On 05/02/2015 02:40 AM, Stephen Boyd wrote:

On 05/01/15 15:07, Heiko Stübner wrote:

Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:


Instead I guess we could hook it less deep into clk_get_sys, like
in the
following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only
patch?

I gave this a spin on my rk3288-firefly board. It still boots, the
clock tree
looks the same and it also still defers nicely in the scenario I
needed it
for. The implementation also looks nice - and of course much more
compact than
my check in two places :-) . I don't know if you want to put this as
follow-up
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner 
Reviewed-by: Heiko Stuebner 


Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.


FWIW, just gave a try for these two patches on all TI boards I have
access to.

Tested-by: Tero Kristo 

I didn't try your evolved patch though, as you don't seem to have made
your mind yet.



Thanks. Can you try the evolved patch? It's in linux-next now as commit
882667c1fcf1, and it seems to at least break sunxi boot. I'd be
interested if it broke TI boards.


Just tried it out, boots fine on all these:

  : Board   : Boot commitlog
 1: am335x-evm  : PASS 4.1.0-rc2-next-20150507   am335x-evm.txt
 2: am335x-evmsk: PASS 4.1.0-rc2-next-20150507   am335x-sk.txt
 3: am3517-evm  : PASS 4.1.0-rc2-next-20150507   am3517-evm.txt
 4: am43x-epos-evm  : PASS 4.1.0-rc2-next-20150507   am43xx-epos.txt
 5: am437x-gp-evm   : PASS 4.1.0-rc2-next-20150507   am43xx-gpevm.txt
 6: am57xx-evm  : PASS 4.1.0-rc2-next-20150507   am57xx-evm.txt
 7: omap3-beagle-xm : PASS 4.1.0-rc2-next-20150507   beagleboard.txt
 8: omap3-beagle: PASS 4.1.0-rc2-next-20150507 
beagleboard-vanilla.txt

 9: am335x-boneblack: PASS 4.1.0-rc2-next-20150507   beaglebone-black.txt
10: am335x-bone : PASS 4.1.0-rc2-next-20150507   beaglebone.txt
11: dra7xx-evm  : PASS 4.1.0-rc2-next-20150507   dra7xx-evm.txt
12: omap3-n900  : PASS 4.1.0-rc2-next-20150507   n900.txt
13: omap5-uevm  : PASS 4.1.0-rc2-next-20150507   omap5-evm.txt
14: omap4-panda-es  : PASS 4.1.0-rc2-next-20150507   pandaboard-es.txt
15: omap4-panda : PASS 4.1.0-rc2-next-20150507   pandaboard-vanilla.txt
16: omap2430-sdp: PASS 4.1.0-rc2-next-20150507   sdp2430.txt
17: omap3430-sdp: PASS 4.1.0-rc2-next-20150507   sdp3430.txt
18: omap4-sdp-es23plus: PASS 4.1.0-rc2-next-20150507   sdp4430.txt
TOTAL = 18 boards, Booted Boards = 18, No Boot boards = 0

TI boards do not have any orphan clocks.

-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: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Maxime Ripard
On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> On 05/07/15 08:17, Kevin Hilman wrote:
> > On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  wrote:
> >> On 05/01/15 15:07, Heiko Stübner wrote:
> >>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >>>
> > Instead I guess we could hook it less deep into clk_get_sys, like in the
> > following patch?
>  It looks like it will work at least, but still I'd prefer to keep the
>  orphan check contained to clk.c. How about this compile tested only 
>  patch?
> >>> I gave this a spin on my rk3288-firefly board. It still boots, the clock 
> >>> tree
> >>> looks the same and it also still defers nicely in the scenario I needed it
> >>> for. The implementation also looks nice - and of course much more compact 
> >>> than
> >>> my check in two places :-) . I don't know if you want to put this as 
> >>> follow-up
> >>> on top or fold it into the original orphan-check, so in any case
> >>>
> >>> Tested-by: Heiko Stuebner 
> >>> Reviewed-by: Heiko Stuebner 
> >> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >> my patch and a note that it's based on an earlier patch from you.
> > It appears this has landed in linux-next in the form of 882667c1fcf1
> > clk: prevent orphan clocks from being used.  A bunch of boot failures
> > for sunxi in today's linux-next[1] were bisected down to that patch.
> >
> > I confirmed that reverting that commit on top of next/master gets
> > sunxi booting again.
> >
> >
> 
> Thanks for the report. I've removed the two clk orphan patches from
> clk-next. Would it be possible to try with next-20150507 and
> clk_ignore_unused on the command line?

This makes it work, but it's not really an option.

> Also we can try to see if critical clocks aren't being forced on by
> applying this patch and looking for clk_get() failures

And that shows that the CPU and DDR clocks are not protected, which
obviously is pretty mad.

I've mass converted all our probing code to use OF_CLK_DECLARE, and
make things work again.

http://code.bulix.org/5goa5j-88345?raw

Is this an acceptable solution?

We were already moving to this, I'm not really fond of doing this like
that, but I guess this whole debacle makes it necessary.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Sascha Hauer
On Fri, May 08, 2015 at 11:30:19AM +0200, Heiko Stübner wrote:
> Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
> > On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> > > On 05/07, Kevin Hilman wrote:
> > > > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd  
> wrote:
> > > > > On 05/07/15 08:17, Kevin Hilman wrote:
> > > > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
> wrote:
> > > > >>> On 05/01/15 15:07, Heiko Stübner wrote:
> > > >  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > > > >> Instead I guess we could hook it less deep into clk_get_sys, like
> > > > >> in the
> > > > >> following patch?
> > > > > 
> > > > > It looks like it will work at least, but still I'd prefer to keep
> > > > > the
> > > > > orphan check contained to clk.c. How about this compile tested
> > > > > only patch?
> > > >  
> > > >  I gave this a spin on my rk3288-firefly board. It still boots, the
> > > >  clock tree looks the same and it also still defers nicely in the
> > > >  scenario I needed it for. The implementation also looks nice - and
> > > >  of course much more compact than my check in two places :-) . I
> > > >  don't know if you want to put this as follow-up on top or fold it
> > > >  into the original orphan-check, so in any case
> > > >  
> > > >  Tested-by: Heiko Stuebner 
> > > >  Reviewed-by: Heiko Stuebner 
> > > > >>> 
> > > > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > > > >>> with
> > > > >>> my patch and a note that it's based on an earlier patch from you.
> > > > >> 
> > > > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > > > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > > > >> 
> > > > >> I confirmed that reverting that commit on top of next/master gets
> > > > >> sunxi booting again.
> > > > > 
> > > > > Thanks for the report. I've removed the two clk orphan patches from
> > > > > clk-next. Would it be possible to try with next-20150507 and
> > > > > clk_ignore_unused on the command line?
> > > > 
> > > > That doesn't help.  I tried on cubieboard2 and bananapi.
> > > 
> > > Thanks for trying.
> > > 
> > > > > Also we can try to see if
> > > > > critical clocks aren't being forced on by applying this patch and
> > > > > looking for clk_get() failures
> > > > 
> > > > From cubieboard2, there's a few that look rather important:
> > > > 
> > > > [0.00] Additional per-CPU info printed with stalls.
> > > > [0.00] Build-time adjustment of leaf fanout to 32.
> > > > [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > > > [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32,
> > > > nr_cpu_ids=2
> > > > [0.00] NR_IRQS:16 nr_irqs:16 16
> > > > [0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> > > > [0.00] Failed to enable critical clock cpu
> > > > [0.00] Failed to enable critical clock pll5_ddr
> > > > [0.00] Failed to enable critical clock ahb_sdram
> > > > [0.00] Architected cp15 timer(s) running at 24.00MHz (virt).
> > > 
> > > Ok. So it seems we need to come up with some solution to the
> > > "critical clocks" problem that doesn't require the individual
> > > clock drivers to call clk_prepare_enable().
> > 
> > I'm getting more and more unsure if we can really handle the complexity
> > we get by allowing to register orphaned clocks. On one hand we can't
> > handle the orphaned clocks properly when we do a clk_prepare/enable on
> > them, on the other hand we run into trouble when we forbid to
> > prepare/enable them. The fact that clocks can become orphans by
> > reparenting them makes it even more complicated.
> > Maybe allowing orphans is something that has to be revisited.
> 
> hmm, I don't see it this drastic. I was expecting a lot more fallout from 
> changing the behaviour of orphaned clocks over all arches using the CCF.
> 
> From the kernelci-boards only the Sunxi-ones seem to have been affected at 
> all 
> and also only because they need a "regulator-always-on" equivalent in the 
> CCF, 
> which currently hijacks prepare/enable inside the clock driver to achive this.

Mediatek has the same case. Here we needs some clock to stay always on and
these clocks get registered before the PLLs they depend on. This means
they can't be enabled right after registration.

Keeping the prepare/enable count correct and syncing the software state
with the hardware state is made quite complicated with orphaned clocks
and I suspect more bugs here. Even now it's hard to move in the
clock framework without breaking other stuff, this won't get easier with
more bug fixes.

So I think asking what allowing orphaned clocks really buys us is valid.
Of course we would need to find a way to get the initialisation order
straight which will 

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Heiko Stübner
Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
> On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> > On 05/07, Kevin Hilman wrote:
> > > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd  
wrote:
> > > > On 05/07/15 08:17, Kevin Hilman wrote:
> > > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
wrote:
> > > >>> On 05/01/15 15:07, Heiko Stübner wrote:
> > >  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > > >> Instead I guess we could hook it less deep into clk_get_sys, like
> > > >> in the
> > > >> following patch?
> > > > 
> > > > It looks like it will work at least, but still I'd prefer to keep
> > > > the
> > > > orphan check contained to clk.c. How about this compile tested
> > > > only patch?
> > >  
> > >  I gave this a spin on my rk3288-firefly board. It still boots, the
> > >  clock tree looks the same and it also still defers nicely in the
> > >  scenario I needed it for. The implementation also looks nice - and
> > >  of course much more compact than my check in two places :-) . I
> > >  don't know if you want to put this as follow-up on top or fold it
> > >  into the original orphan-check, so in any case
> > >  
> > >  Tested-by: Heiko Stuebner 
> > >  Reviewed-by: Heiko Stuebner 
> > > >>> 
> > > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > > >>> with
> > > >>> my patch and a note that it's based on an earlier patch from you.
> > > >> 
> > > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > > >> 
> > > >> I confirmed that reverting that commit on top of next/master gets
> > > >> sunxi booting again.
> > > > 
> > > > Thanks for the report. I've removed the two clk orphan patches from
> > > > clk-next. Would it be possible to try with next-20150507 and
> > > > clk_ignore_unused on the command line?
> > > 
> > > That doesn't help.  I tried on cubieboard2 and bananapi.
> > 
> > Thanks for trying.
> > 
> > > > Also we can try to see if
> > > > critical clocks aren't being forced on by applying this patch and
> > > > looking for clk_get() failures
> > > 
> > > From cubieboard2, there's a few that look rather important:
> > > 
> > > [0.00] Additional per-CPU info printed with stalls.
> > > [0.00] Build-time adjustment of leaf fanout to 32.
> > > [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > > [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32,
> > > nr_cpu_ids=2
> > > [0.00] NR_IRQS:16 nr_irqs:16 16
> > > [0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> > > [0.00] Failed to enable critical clock cpu
> > > [0.00] Failed to enable critical clock pll5_ddr
> > > [0.00] Failed to enable critical clock ahb_sdram
> > > [0.00] Architected cp15 timer(s) running at 24.00MHz (virt).
> > 
> > Ok. So it seems we need to come up with some solution to the
> > "critical clocks" problem that doesn't require the individual
> > clock drivers to call clk_prepare_enable().
> 
> I'm getting more and more unsure if we can really handle the complexity
> we get by allowing to register orphaned clocks. On one hand we can't
> handle the orphaned clocks properly when we do a clk_prepare/enable on
> them, on the other hand we run into trouble when we forbid to
> prepare/enable them. The fact that clocks can become orphans by
> reparenting them makes it even more complicated.
> Maybe allowing orphans is something that has to be revisited.

hmm, I don't see it this drastic. I was expecting a lot more fallout from 
changing the behaviour of orphaned clocks over all arches using the CCF.

>From the kernelci-boards only the Sunxi-ones seem to have been affected at all 
and also only because they need a "regulator-always-on" equivalent in the CCF, 
which currently hijacks prepare/enable inside the clock driver to achive this.

On the Rockchip clocks we have something similar, with the only difference that 
it is done after all clocks are registered and does not need to access the 3 
orphans we have at this point due to their source clock coming from an 
external i2c-connected chip that gets probed a lot later.

--
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/2] clk: improve handling of orphan clocks

2015-05-08 Thread Sascha Hauer
On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> On 05/07, Kevin Hilman wrote:
> > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd  wrote:
> > > On 05/07/15 08:17, Kevin Hilman wrote:
> > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  
> > >> wrote:
> > >>> On 05/01/15 15:07, Heiko Stübner wrote:
> >  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > 
> > >> Instead I guess we could hook it less deep into clk_get_sys, like in 
> > >> the
> > >> following patch?
> > > It looks like it will work at least, but still I'd prefer to keep the
> > > orphan check contained to clk.c. How about this compile tested only 
> > > patch?
> >  I gave this a spin on my rk3288-firefly board. It still boots, the 
> >  clock tree
> >  looks the same and it also still defers nicely in the scenario I 
> >  needed it
> >  for. The implementation also looks nice - and of course much more 
> >  compact than
> >  my check in two places :-) . I don't know if you want to put this as 
> >  follow-up
> >  on top or fold it into the original orphan-check, so in any case
> > 
> >  Tested-by: Heiko Stuebner 
> >  Reviewed-by: Heiko Stuebner 
> > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> > >>> my patch and a note that it's based on an earlier patch from you.
> > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > >>
> > >> I confirmed that reverting that commit on top of next/master gets
> > >> sunxi booting again.
> > >>
> > >>
> > >
> > > Thanks for the report. I've removed the two clk orphan patches from
> > > clk-next. Would it be possible to try with next-20150507 and
> > > clk_ignore_unused on the command line?
> > 
> > That doesn't help.  I tried on cubieboard2 and bananapi.
> 
> Thanks for trying.
> 
> > 
> > > Also we can try to see if
> > > critical clocks aren't being forced on by applying this patch and
> > > looking for clk_get() failures
> > 
> > From cubieboard2, there's a few that look rather important:
> > 
> > [0.00] Additional per-CPU info printed with stalls.
> > [0.00] Build-time adjustment of leaf fanout to 32.
> > [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
> > [0.00] NR_IRQS:16 nr_irqs:16 16
> > [0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> > [0.00] Failed to enable critical clock cpu
> > [0.00] Failed to enable critical clock pll5_ddr
> > [0.00] Failed to enable critical clock ahb_sdram
> > [0.00] Architected cp15 timer(s) running at 24.00MHz (virt).
> 
> Ok. So it seems we need to come up with some solution to the
> "critical clocks" problem that doesn't require the individual
> clock drivers to call clk_prepare_enable().

I'm getting more and more unsure if we can really handle the complexity
we get by allowing to register orphaned clocks. On one hand we can't
handle the orphaned clocks properly when we do a clk_prepare/enable on
them, on the other hand we run into trouble when we forbid to
prepare/enable them. The fact that clocks can become orphans by
reparenting them makes it even more complicated.
Maybe allowing orphans is something that has to be revisited.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
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/2] clk: improve handling of orphan clocks

2015-05-08 Thread Stephen Boyd
On 05/07, Kevin Hilman wrote:
> On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd  wrote:
> > On 05/07/15 08:17, Kevin Hilman wrote:
> >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  wrote:
> >>> On 05/01/15 15:07, Heiko Stübner wrote:
>  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> 
> >> Instead I guess we could hook it less deep into clk_get_sys, like in 
> >> the
> >> following patch?
> > It looks like it will work at least, but still I'd prefer to keep the
> > orphan check contained to clk.c. How about this compile tested only 
> > patch?
>  I gave this a spin on my rk3288-firefly board. It still boots, the clock 
>  tree
>  looks the same and it also still defers nicely in the scenario I needed 
>  it
>  for. The implementation also looks nice - and of course much more 
>  compact than
>  my check in two places :-) . I don't know if you want to put this as 
>  follow-up
>  on top or fold it into the original orphan-check, so in any case
> 
>  Tested-by: Heiko Stuebner 
>  Reviewed-by: Heiko Stuebner 
> >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >>> my patch and a note that it's based on an earlier patch from you.
> >> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> for sunxi in today's linux-next[1] were bisected down to that patch.
> >>
> >> I confirmed that reverting that commit on top of next/master gets
> >> sunxi booting again.
> >>
> >>
> >
> > Thanks for the report. I've removed the two clk orphan patches from
> > clk-next. Would it be possible to try with next-20150507 and
> > clk_ignore_unused on the command line?
> 
> That doesn't help.  I tried on cubieboard2 and bananapi.

Thanks for trying.

> 
> > Also we can try to see if
> > critical clocks aren't being forced on by applying this patch and
> > looking for clk_get() failures
> 
> From cubieboard2, there's a few that look rather important:
> 
> [0.00] Additional per-CPU info printed with stalls.
> [0.00] Build-time adjustment of leaf fanout to 32.
> [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
> [0.00] NR_IRQS:16 nr_irqs:16 16
> [0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> [0.00] Failed to enable critical clock cpu
> [0.00] Failed to enable critical clock pll5_ddr
> [0.00] Failed to enable critical clock ahb_sdram
> [0.00] Architected cp15 timer(s) running at 24.00MHz (virt).

Ok. So it seems we need to come up with some solution to the
"critical clocks" problem that doesn't require the individual
clock drivers to call clk_prepare_enable().

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Stephen Boyd
On 05/07, Kevin Hilman wrote:
 On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd sb...@codeaurora.org wrote:
  On 05/07/15 08:17, Kevin Hilman wrote:
  On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org wrote:
  On 05/01/15 15:07, Heiko Stübner wrote:
  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
 
  Instead I guess we could hook it less deep into clk_get_sys, like in 
  the
  following patch?
  It looks like it will work at least, but still I'd prefer to keep the
  orphan check contained to clk.c. How about this compile tested only 
  patch?
  I gave this a spin on my rk3288-firefly board. It still boots, the clock 
  tree
  looks the same and it also still defers nicely in the scenario I needed 
  it
  for. The implementation also looks nice - and of course much more 
  compact than
  my check in two places :-) . I don't know if you want to put this as 
  follow-up
  on top or fold it into the original orphan-check, so in any case
 
  Tested-by: Heiko Stuebner he...@sntech.de
  Reviewed-by: Heiko Stuebner he...@sntech.de
  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
  my patch and a note that it's based on an earlier patch from you.
  It appears this has landed in linux-next in the form of 882667c1fcf1
  clk: prevent orphan clocks from being used.  A bunch of boot failures
  for sunxi in today's linux-next[1] were bisected down to that patch.
 
  I confirmed that reverting that commit on top of next/master gets
  sunxi booting again.
 
 
 
  Thanks for the report. I've removed the two clk orphan patches from
  clk-next. Would it be possible to try with next-20150507 and
  clk_ignore_unused on the command line?
 
 That doesn't help.  I tried on cubieboard2 and bananapi.

Thanks for trying.

 
  Also we can try to see if
  critical clocks aren't being forced on by applying this patch and
  looking for clk_get() failures
 
 From cubieboard2, there's a few that look rather important:
 
 [0.00] Additional per-CPU info printed with stalls.
 [0.00] Build-time adjustment of leaf fanout to 32.
 [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
 [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
 [0.00] NR_IRQS:16 nr_irqs:16 16
 [0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
 [0.00] Failed to enable critical clock cpu
 [0.00] Failed to enable critical clock pll5_ddr
 [0.00] Failed to enable critical clock ahb_sdram
 [0.00] Architected cp15 timer(s) running at 24.00MHz (virt).

Ok. So it seems we need to come up with some solution to the
critical clocks problem that doesn't require the individual
clock drivers to call clk_prepare_enable().

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Sascha Hauer
On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
 On 05/07, Kevin Hilman wrote:
  On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd sb...@codeaurora.org wrote:
   On 05/07/15 08:17, Kevin Hilman wrote:
   On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
   wrote:
   On 05/01/15 15:07, Heiko Stübner wrote:
   Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
  
   Instead I guess we could hook it less deep into clk_get_sys, like in 
   the
   following patch?
   It looks like it will work at least, but still I'd prefer to keep the
   orphan check contained to clk.c. How about this compile tested only 
   patch?
   I gave this a spin on my rk3288-firefly board. It still boots, the 
   clock tree
   looks the same and it also still defers nicely in the scenario I 
   needed it
   for. The implementation also looks nice - and of course much more 
   compact than
   my check in two places :-) . I don't know if you want to put this as 
   follow-up
   on top or fold it into the original orphan-check, so in any case
  
   Tested-by: Heiko Stuebner he...@sntech.de
   Reviewed-by: Heiko Stuebner he...@sntech.de
   Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
   my patch and a note that it's based on an earlier patch from you.
   It appears this has landed in linux-next in the form of 882667c1fcf1
   clk: prevent orphan clocks from being used.  A bunch of boot failures
   for sunxi in today's linux-next[1] were bisected down to that patch.
  
   I confirmed that reverting that commit on top of next/master gets
   sunxi booting again.
  
  
  
   Thanks for the report. I've removed the two clk orphan patches from
   clk-next. Would it be possible to try with next-20150507 and
   clk_ignore_unused on the command line?
  
  That doesn't help.  I tried on cubieboard2 and bananapi.
 
 Thanks for trying.
 
  
   Also we can try to see if
   critical clocks aren't being forced on by applying this patch and
   looking for clk_get() failures
  
  From cubieboard2, there's a few that look rather important:
  
  [0.00] Additional per-CPU info printed with stalls.
  [0.00] Build-time adjustment of leaf fanout to 32.
  [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
  [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
  [0.00] NR_IRQS:16 nr_irqs:16 16
  [0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
  [0.00] Failed to enable critical clock cpu
  [0.00] Failed to enable critical clock pll5_ddr
  [0.00] Failed to enable critical clock ahb_sdram
  [0.00] Architected cp15 timer(s) running at 24.00MHz (virt).
 
 Ok. So it seems we need to come up with some solution to the
 critical clocks problem that doesn't require the individual
 clock drivers to call clk_prepare_enable().

I'm getting more and more unsure if we can really handle the complexity
we get by allowing to register orphaned clocks. On one hand we can't
handle the orphaned clocks properly when we do a clk_prepare/enable on
them, on the other hand we run into trouble when we forbid to
prepare/enable them. The fact that clocks can become orphans by
reparenting them makes it even more complicated.
Maybe allowing orphans is something that has to be revisited.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
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/2] clk: improve handling of orphan clocks

2015-05-08 Thread Heiko Stübner
Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
 On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
  On 05/07, Kevin Hilman wrote:
   On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd sb...@codeaurora.org 
wrote:
On 05/07/15 08:17, Kevin Hilman wrote:
On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
wrote:
On 05/01/15 15:07, Heiko Stübner wrote:
Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
Instead I guess we could hook it less deep into clk_get_sys, like
in the
following patch?

It looks like it will work at least, but still I'd prefer to keep
the
orphan check contained to clk.c. How about this compile tested
only patch?

I gave this a spin on my rk3288-firefly board. It still boots, the
clock tree looks the same and it also still defers nicely in the
scenario I needed it for. The implementation also looks nice - and
of course much more compact than my check in two places :-) . I
don't know if you want to put this as follow-up on top or fold it
into the original orphan-check, so in any case

Tested-by: Heiko Stuebner he...@sntech.de
Reviewed-by: Heiko Stuebner he...@sntech.de

Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
with
my patch and a note that it's based on an earlier patch from you.

It appears this has landed in linux-next in the form of 882667c1fcf1
clk: prevent orphan clocks from being used.  A bunch of boot failures
for sunxi in today's linux-next[1] were bisected down to that patch.

I confirmed that reverting that commit on top of next/master gets
sunxi booting again.

Thanks for the report. I've removed the two clk orphan patches from
clk-next. Would it be possible to try with next-20150507 and
clk_ignore_unused on the command line?
   
   That doesn't help.  I tried on cubieboard2 and bananapi.
  
  Thanks for trying.
  
Also we can try to see if
critical clocks aren't being forced on by applying this patch and
looking for clk_get() failures
   
   From cubieboard2, there's a few that look rather important:
   
   [0.00] Additional per-CPU info printed with stalls.
   [0.00] Build-time adjustment of leaf fanout to 32.
   [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
   [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32,
   nr_cpu_ids=2
   [0.00] NR_IRQS:16 nr_irqs:16 16
   [0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
   [0.00] Failed to enable critical clock cpu
   [0.00] Failed to enable critical clock pll5_ddr
   [0.00] Failed to enable critical clock ahb_sdram
   [0.00] Architected cp15 timer(s) running at 24.00MHz (virt).
  
  Ok. So it seems we need to come up with some solution to the
  critical clocks problem that doesn't require the individual
  clock drivers to call clk_prepare_enable().
 
 I'm getting more and more unsure if we can really handle the complexity
 we get by allowing to register orphaned clocks. On one hand we can't
 handle the orphaned clocks properly when we do a clk_prepare/enable on
 them, on the other hand we run into trouble when we forbid to
 prepare/enable them. The fact that clocks can become orphans by
 reparenting them makes it even more complicated.
 Maybe allowing orphans is something that has to be revisited.

hmm, I don't see it this drastic. I was expecting a lot more fallout from 
changing the behaviour of orphaned clocks over all arches using the CCF.

From the kernelci-boards only the Sunxi-ones seem to have been affected at all 
and also only because they need a regulator-always-on equivalent in the CCF, 
which currently hijacks prepare/enable inside the clock driver to achive this.

On the Rockchip clocks we have something similar, with the only difference that 
it is done after all clocks are registered and does not need to access the 3 
orphans we have at this point due to their source clock coming from an 
external i2c-connected chip that gets probed a lot later.

--
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/2] clk: improve handling of orphan clocks

2015-05-08 Thread Maxime Ripard
On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
 On 05/07/15 08:17, Kevin Hilman wrote:
  On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org wrote:
  On 05/01/15 15:07, Heiko Stübner wrote:
  Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
 
  Instead I guess we could hook it less deep into clk_get_sys, like in the
  following patch?
  It looks like it will work at least, but still I'd prefer to keep the
  orphan check contained to clk.c. How about this compile tested only 
  patch?
  I gave this a spin on my rk3288-firefly board. It still boots, the clock 
  tree
  looks the same and it also still defers nicely in the scenario I needed it
  for. The implementation also looks nice - and of course much more compact 
  than
  my check in two places :-) . I don't know if you want to put this as 
  follow-up
  on top or fold it into the original orphan-check, so in any case
 
  Tested-by: Heiko Stuebner he...@sntech.de
  Reviewed-by: Heiko Stuebner he...@sntech.de
  Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
  my patch and a note that it's based on an earlier patch from you.
  It appears this has landed in linux-next in the form of 882667c1fcf1
  clk: prevent orphan clocks from being used.  A bunch of boot failures
  for sunxi in today's linux-next[1] were bisected down to that patch.
 
  I confirmed that reverting that commit on top of next/master gets
  sunxi booting again.
 
 
 
 Thanks for the report. I've removed the two clk orphan patches from
 clk-next. Would it be possible to try with next-20150507 and
 clk_ignore_unused on the command line?

This makes it work, but it's not really an option.

 Also we can try to see if critical clocks aren't being forced on by
 applying this patch and looking for clk_get() failures

And that shows that the CPU and DDR clocks are not protected, which
obviously is pretty mad.

I've mass converted all our probing code to use OF_CLK_DECLARE, and
make things work again.

http://code.bulix.org/5goa5j-88345?raw

Is this an acceptable solution?

We were already moving to this, I'm not really fond of doing this like
that, but I guess this whole debacle makes it necessary.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Sascha Hauer
On Fri, May 08, 2015 at 11:30:19AM +0200, Heiko Stübner wrote:
 Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
  On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
   On 05/07, Kevin Hilman wrote:
On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd sb...@codeaurora.org 
 wrote:
 On 05/07/15 08:17, Kevin Hilman wrote:
 On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org 
 wrote:
 On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
 Instead I guess we could hook it less deep into clk_get_sys, like
 in the
 following patch?
 
 It looks like it will work at least, but still I'd prefer to keep
 the
 orphan check contained to clk.c. How about this compile tested
 only patch?
 
 I gave this a spin on my rk3288-firefly board. It still boots, the
 clock tree looks the same and it also still defers nicely in the
 scenario I needed it for. The implementation also looks nice - and
 of course much more compact than my check in two places :-) . I
 don't know if you want to put this as follow-up on top or fold it
 into the original orphan-check, so in any case
 
 Tested-by: Heiko Stuebner he...@sntech.de
 Reviewed-by: Heiko Stuebner he...@sntech.de
 
 Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
 with
 my patch and a note that it's based on an earlier patch from you.
 
 It appears this has landed in linux-next in the form of 882667c1fcf1
 clk: prevent orphan clocks from being used.  A bunch of boot failures
 for sunxi in today's linux-next[1] were bisected down to that patch.
 
 I confirmed that reverting that commit on top of next/master gets
 sunxi booting again.
 
 Thanks for the report. I've removed the two clk orphan patches from
 clk-next. Would it be possible to try with next-20150507 and
 clk_ignore_unused on the command line?

That doesn't help.  I tried on cubieboard2 and bananapi.
   
   Thanks for trying.
   
 Also we can try to see if
 critical clocks aren't being forced on by applying this patch and
 looking for clk_get() failures

From cubieboard2, there's a few that look rather important:

[0.00] Additional per-CPU info printed with stalls.
[0.00] Build-time adjustment of leaf fanout to 32.
[0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
[0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32,
nr_cpu_ids=2
[0.00] NR_IRQS:16 nr_irqs:16 16
[0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
[0.00] Failed to enable critical clock cpu
[0.00] Failed to enable critical clock pll5_ddr
[0.00] Failed to enable critical clock ahb_sdram
[0.00] Architected cp15 timer(s) running at 24.00MHz (virt).
   
   Ok. So it seems we need to come up with some solution to the
   critical clocks problem that doesn't require the individual
   clock drivers to call clk_prepare_enable().
  
  I'm getting more and more unsure if we can really handle the complexity
  we get by allowing to register orphaned clocks. On one hand we can't
  handle the orphaned clocks properly when we do a clk_prepare/enable on
  them, on the other hand we run into trouble when we forbid to
  prepare/enable them. The fact that clocks can become orphans by
  reparenting them makes it even more complicated.
  Maybe allowing orphans is something that has to be revisited.
 
 hmm, I don't see it this drastic. I was expecting a lot more fallout from 
 changing the behaviour of orphaned clocks over all arches using the CCF.
 
 From the kernelci-boards only the Sunxi-ones seem to have been affected at 
 all 
 and also only because they need a regulator-always-on equivalent in the 
 CCF, 
 which currently hijacks prepare/enable inside the clock driver to achive this.

Mediatek has the same case. Here we needs some clock to stay always on and
these clocks get registered before the PLLs they depend on. This means
they can't be enabled right after registration.

Keeping the prepare/enable count correct and syncing the software state
with the hardware state is made quite complicated with orphaned clocks
and I suspect more bugs here. Even now it's hard to move in the
clock framework without breaking other stuff, this won't get easier with
more bug fixes.

So I think asking what allowing orphaned clocks really buys us is valid.
Of course we would need to find a way to get the initialisation order
straight which will probably cause some pain.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-08 Thread Tero Kristo

On 05/07/2015 09:18 PM, Stephen Boyd wrote:

On 05/07/15 01:22, Tero Kristo wrote:

On 05/02/2015 02:40 AM, Stephen Boyd wrote:

On 05/01/15 15:07, Heiko Stübner wrote:

Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:


Instead I guess we could hook it less deep into clk_get_sys, like
in the
following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only
patch?

I gave this a spin on my rk3288-firefly board. It still boots, the
clock tree
looks the same and it also still defers nicely in the scenario I
needed it
for. The implementation also looks nice - and of course much more
compact than
my check in two places :-) . I don't know if you want to put this as
follow-up
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner he...@sntech.de
Reviewed-by: Heiko Stuebner he...@sntech.de


Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.


FWIW, just gave a try for these two patches on all TI boards I have
access to.

Tested-by: Tero Kristo t-kri...@ti.com

I didn't try your evolved patch though, as you don't seem to have made
your mind yet.



Thanks. Can you try the evolved patch? It's in linux-next now as commit
882667c1fcf1, and it seems to at least break sunxi boot. I'd be
interested if it broke TI boards.


Just tried it out, boots fine on all these:

  : Board   : Boot commitlog
 1: am335x-evm  : PASS 4.1.0-rc2-next-20150507   am335x-evm.txt
 2: am335x-evmsk: PASS 4.1.0-rc2-next-20150507   am335x-sk.txt
 3: am3517-evm  : PASS 4.1.0-rc2-next-20150507   am3517-evm.txt
 4: am43x-epos-evm  : PASS 4.1.0-rc2-next-20150507   am43xx-epos.txt
 5: am437x-gp-evm   : PASS 4.1.0-rc2-next-20150507   am43xx-gpevm.txt
 6: am57xx-evm  : PASS 4.1.0-rc2-next-20150507   am57xx-evm.txt
 7: omap3-beagle-xm : PASS 4.1.0-rc2-next-20150507   beagleboard.txt
 8: omap3-beagle: PASS 4.1.0-rc2-next-20150507 
beagleboard-vanilla.txt

 9: am335x-boneblack: PASS 4.1.0-rc2-next-20150507   beaglebone-black.txt
10: am335x-bone : PASS 4.1.0-rc2-next-20150507   beaglebone.txt
11: dra7xx-evm  : PASS 4.1.0-rc2-next-20150507   dra7xx-evm.txt
12: omap3-n900  : PASS 4.1.0-rc2-next-20150507   n900.txt
13: omap5-uevm  : PASS 4.1.0-rc2-next-20150507   omap5-evm.txt
14: omap4-panda-es  : PASS 4.1.0-rc2-next-20150507   pandaboard-es.txt
15: omap4-panda : PASS 4.1.0-rc2-next-20150507   pandaboard-vanilla.txt
16: omap2430-sdp: PASS 4.1.0-rc2-next-20150507   sdp2430.txt
17: omap3430-sdp: PASS 4.1.0-rc2-next-20150507   sdp3430.txt
18: omap4-sdp-es23plus: PASS 4.1.0-rc2-next-20150507   sdp4430.txt
TOTAL = 18 boards, Booted Boards = 18, No Boot boards = 0

TI boards do not have any orphan clocks.

-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: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-07 Thread Kevin Hilman
On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd  wrote:
> On 05/07/15 08:17, Kevin Hilman wrote:
>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  wrote:
>>> On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:

>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>> following patch?
> It looks like it will work at least, but still I'd prefer to keep the
> orphan check contained to clk.c. How about this compile tested only patch?
 I gave this a spin on my rk3288-firefly board. It still boots, the clock 
 tree
 looks the same and it also still defers nicely in the scenario I needed it
 for. The implementation also looks nice - and of course much more compact 
 than
 my check in two places :-) . I don't know if you want to put this as 
 follow-up
 on top or fold it into the original orphan-check, so in any case

 Tested-by: Heiko Stuebner 
 Reviewed-by: Heiko Stuebner 
>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>> my patch and a note that it's based on an earlier patch from you.
>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> for sunxi in today's linux-next[1] were bisected down to that patch.
>>
>> I confirmed that reverting that commit on top of next/master gets
>> sunxi booting again.
>>
>>
>
> Thanks for the report. I've removed the two clk orphan patches from
> clk-next. Would it be possible to try with next-20150507 and
> clk_ignore_unused on the command line?

That doesn't help.  I tried on cubieboard2 and bananapi.

> Also we can try to see if
> critical clocks aren't being forced on by applying this patch and
> looking for clk_get() failures

>From cubieboard2, there's a few that look rather important:

[0.00] Additional per-CPU info printed with stalls.
[0.00] Build-time adjustment of leaf fanout to 32.
[0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
[0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
[0.00] NR_IRQS:16 nr_irqs:16 16
[0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
[0.00] Failed to enable critical clock cpu
[0.00] Failed to enable critical clock pll5_ddr
[0.00] Failed to enable critical clock ahb_sdram
[0.00] Architected cp15 timer(s) running at 24.00MHz (virt).

Kevin
--
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/2] clk: improve handling of orphan clocks

2015-05-07 Thread Stephen Boyd
On 05/07/15 08:17, Kevin Hilman wrote:
> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  wrote:
>> On 05/01/15 15:07, Heiko Stübner wrote:
>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>
> Instead I guess we could hook it less deep into clk_get_sys, like in the
> following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only patch?
>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock 
>>> tree
>>> looks the same and it also still defers nicely in the scenario I needed it
>>> for. The implementation also looks nice - and of course much more compact 
>>> than
>>> my check in two places :-) . I don't know if you want to put this as 
>>> follow-up
>>> on top or fold it into the original orphan-check, so in any case
>>>
>>> Tested-by: Heiko Stuebner 
>>> Reviewed-by: Heiko Stuebner 
>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> my patch and a note that it's based on an earlier patch from you.
> It appears this has landed in linux-next in the form of 882667c1fcf1
> clk: prevent orphan clocks from being used.  A bunch of boot failures
> for sunxi in today's linux-next[1] were bisected down to that patch.
>
> I confirmed that reverting that commit on top of next/master gets
> sunxi booting again.
>
>

Thanks for the report. I've removed the two clk orphan patches from
clk-next. Would it be possible to try with next-20150507 and
clk_ignore_unused on the command line? Also we can try to see if
critical clocks aren't being forced on by applying this patch and
looking for clk_get() failures

--8<--

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7e1e2bd189b6..d88585b680bb 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1347,6 +1347,9 @@ static void __init sunxi_init_clocks(const char 
*clocks[], int nclocks)
 
if (!IS_ERR(clk))
clk_prepare_enable(clk);
+   else
+   pr_err("Failed to enable critical clock %s\n",
+   clocks[i]);
}
 }
 

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-07 Thread Stephen Boyd
On 05/07/15 01:22, Tero Kristo wrote:
> On 05/02/2015 02:40 AM, Stephen Boyd wrote:
>> On 05/01/15 15:07, Heiko Stübner wrote:
>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>
> Instead I guess we could hook it less deep into clk_get_sys, like
> in the
> following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only
 patch?
>>> I gave this a spin on my rk3288-firefly board. It still boots, the
>>> clock tree
>>> looks the same and it also still defers nicely in the scenario I
>>> needed it
>>> for. The implementation also looks nice - and of course much more
>>> compact than
>>> my check in two places :-) . I don't know if you want to put this as
>>> follow-up
>>> on top or fold it into the original orphan-check, so in any case
>>>
>>> Tested-by: Heiko Stuebner 
>>> Reviewed-by: Heiko Stuebner 
>>
>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> my patch and a note that it's based on an earlier patch from you.
>
> FWIW, just gave a try for these two patches on all TI boards I have
> access to.
>
> Tested-by: Tero Kristo 
>
> I didn't try your evolved patch though, as you don't seem to have made
> your mind yet.
>

Thanks. Can you try the evolved patch? It's in linux-next now as commit
882667c1fcf1, and it seems to at least break sunxi boot. I'd be
interested if it broke TI boards.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-07 Thread Kevin Hilman
On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd  wrote:
> On 05/01/15 15:07, Heiko Stübner wrote:
>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>
 Instead I guess we could hook it less deep into clk_get_sys, like in the
 following patch?
>>> It looks like it will work at least, but still I'd prefer to keep the
>>> orphan check contained to clk.c. How about this compile tested only patch?
>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> looks the same and it also still defers nicely in the scenario I needed it
>> for. The implementation also looks nice - and of course much more compact 
>> than
>> my check in two places :-) . I don't know if you want to put this as 
>> follow-up
>> on top or fold it into the original orphan-check, so in any case
>>
>> Tested-by: Heiko Stuebner 
>> Reviewed-by: Heiko Stuebner 
>
> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> my patch and a note that it's based on an earlier patch from you.

It appears this has landed in linux-next in the form of 882667c1fcf1
clk: prevent orphan clocks from being used.  A bunch of boot failures
for sunxi in today's linux-next[1] were bisected down to that patch.

I confirmed that reverting that commit on top of next/master gets
sunxi booting again.

Kevin

[1] http://kernelci.org/boot/all/job/next/kernel/next-20150507/
--
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/2] clk: improve handling of orphan clocks

2015-05-07 Thread Tero Kristo

On 05/02/2015 02:40 AM, Stephen Boyd wrote:

On 05/01/15 15:07, Heiko Stübner wrote:

Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:


Instead I guess we could hook it less deep into clk_get_sys, like in the
following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only patch?

I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
looks the same and it also still defers nicely in the scenario I needed it
for. The implementation also looks nice - and of course much more compact than
my check in two places :-) . I don't know if you want to put this as follow-up
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner 
Reviewed-by: Heiko Stuebner 


Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.


FWIW, just gave a try for these two patches on all TI boards I have 
access to.


Tested-by: Tero Kristo 

I didn't try your evolved patch though, as you don't seem to have made 
your mind yet.


-Tero







This also brings up an existing problem with clk_unregister() where
orphaned clocks are sitting out there useable by drivers when their
parent is unregistered. That code could use some work to atomically
switch all the orphaned clocks over to use the nodrv_ops.

Not sure I understand this correctly yet, but when these children get
orphaned, switched to the clk_nodrv_ops, they won't get their original ops
back if the parent reappears.

So I guess we would need to store the original ops in secondary property of
struct clk_core and I guess simply bind the ops-switch to the orphan state
update?


Yep. We'll need to store away the original ops in case we need to put
them back. Don't feel obligated to fix this either. It would certainly
be nice if someone tried to fix this case at some point, but it's not
like things are any worse off right now.



--
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/2] clk: improve handling of orphan clocks

2015-05-07 Thread Stephen Boyd
On 05/07/15 01:22, Tero Kristo wrote:
 On 05/02/2015 02:40 AM, Stephen Boyd wrote:
 On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:

 Instead I guess we could hook it less deep into clk_get_sys, like
 in the
 following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only
 patch?
 I gave this a spin on my rk3288-firefly board. It still boots, the
 clock tree
 looks the same and it also still defers nicely in the scenario I
 needed it
 for. The implementation also looks nice - and of course much more
 compact than
 my check in two places :-) . I don't know if you want to put this as
 follow-up
 on top or fold it into the original orphan-check, so in any case

 Tested-by: Heiko Stuebner he...@sntech.de
 Reviewed-by: Heiko Stuebner he...@sntech.de

 Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
 my patch and a note that it's based on an earlier patch from you.

 FWIW, just gave a try for these two patches on all TI boards I have
 access to.

 Tested-by: Tero Kristo t-kri...@ti.com

 I didn't try your evolved patch though, as you don't seem to have made
 your mind yet.


Thanks. Can you try the evolved patch? It's in linux-next now as commit
882667c1fcf1, and it seems to at least break sunxi boot. I'd be
interested if it broke TI boards.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-07 Thread Kevin Hilman
On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd sb...@codeaurora.org wrote:
 On 05/07/15 08:17, Kevin Hilman wrote:
 On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org wrote:
 On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:

 Instead I guess we could hook it less deep into clk_get_sys, like in the
 following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only patch?
 I gave this a spin on my rk3288-firefly board. It still boots, the clock 
 tree
 looks the same and it also still defers nicely in the scenario I needed it
 for. The implementation also looks nice - and of course much more compact 
 than
 my check in two places :-) . I don't know if you want to put this as 
 follow-up
 on top or fold it into the original orphan-check, so in any case

 Tested-by: Heiko Stuebner he...@sntech.de
 Reviewed-by: Heiko Stuebner he...@sntech.de
 Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
 my patch and a note that it's based on an earlier patch from you.
 It appears this has landed in linux-next in the form of 882667c1fcf1
 clk: prevent orphan clocks from being used.  A bunch of boot failures
 for sunxi in today's linux-next[1] were bisected down to that patch.

 I confirmed that reverting that commit on top of next/master gets
 sunxi booting again.



 Thanks for the report. I've removed the two clk orphan patches from
 clk-next. Would it be possible to try with next-20150507 and
 clk_ignore_unused on the command line?

That doesn't help.  I tried on cubieboard2 and bananapi.

 Also we can try to see if
 critical clocks aren't being forced on by applying this patch and
 looking for clk_get() failures

From cubieboard2, there's a few that look rather important:

[0.00] Additional per-CPU info printed with stalls.
[0.00] Build-time adjustment of leaf fanout to 32.
[0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
[0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
[0.00] NR_IRQS:16 nr_irqs:16 16
[0.00] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
[0.00] Failed to enable critical clock cpu
[0.00] Failed to enable critical clock pll5_ddr
[0.00] Failed to enable critical clock ahb_sdram
[0.00] Architected cp15 timer(s) running at 24.00MHz (virt).

Kevin
--
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/2] clk: improve handling of orphan clocks

2015-05-07 Thread Tero Kristo

On 05/02/2015 02:40 AM, Stephen Boyd wrote:

On 05/01/15 15:07, Heiko Stübner wrote:

Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:


Instead I guess we could hook it less deep into clk_get_sys, like in the
following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only patch?

I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
looks the same and it also still defers nicely in the scenario I needed it
for. The implementation also looks nice - and of course much more compact than
my check in two places :-) . I don't know if you want to put this as follow-up
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner he...@sntech.de
Reviewed-by: Heiko Stuebner he...@sntech.de


Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.


FWIW, just gave a try for these two patches on all TI boards I have 
access to.


Tested-by: Tero Kristo t-kri...@ti.com

I didn't try your evolved patch though, as you don't seem to have made 
your mind yet.


-Tero







This also brings up an existing problem with clk_unregister() where
orphaned clocks are sitting out there useable by drivers when their
parent is unregistered. That code could use some work to atomically
switch all the orphaned clocks over to use the nodrv_ops.

Not sure I understand this correctly yet, but when these children get
orphaned, switched to the clk_nodrv_ops, they won't get their original ops
back if the parent reappears.

So I guess we would need to store the original ops in secondary property of
struct clk_core and I guess simply bind the ops-switch to the orphan state
update?


Yep. We'll need to store away the original ops in case we need to put
them back. Don't feel obligated to fix this either. It would certainly
be nice if someone tried to fix this case at some point, but it's not
like things are any worse off right now.



--
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/2] clk: improve handling of orphan clocks

2015-05-07 Thread Stephen Boyd
On 05/07/15 08:17, Kevin Hilman wrote:
 On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org wrote:
 On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:

 Instead I guess we could hook it less deep into clk_get_sys, like in the
 following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only patch?
 I gave this a spin on my rk3288-firefly board. It still boots, the clock 
 tree
 looks the same and it also still defers nicely in the scenario I needed it
 for. The implementation also looks nice - and of course much more compact 
 than
 my check in two places :-) . I don't know if you want to put this as 
 follow-up
 on top or fold it into the original orphan-check, so in any case

 Tested-by: Heiko Stuebner he...@sntech.de
 Reviewed-by: Heiko Stuebner he...@sntech.de
 Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
 my patch and a note that it's based on an earlier patch from you.
 It appears this has landed in linux-next in the form of 882667c1fcf1
 clk: prevent orphan clocks from being used.  A bunch of boot failures
 for sunxi in today's linux-next[1] were bisected down to that patch.

 I confirmed that reverting that commit on top of next/master gets
 sunxi booting again.



Thanks for the report. I've removed the two clk orphan patches from
clk-next. Would it be possible to try with next-20150507 and
clk_ignore_unused on the command line? Also we can try to see if
critical clocks aren't being forced on by applying this patch and
looking for clk_get() failures

--8--

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7e1e2bd189b6..d88585b680bb 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1347,6 +1347,9 @@ static void __init sunxi_init_clocks(const char 
*clocks[], int nclocks)
 
if (!IS_ERR(clk))
clk_prepare_enable(clk);
+   else
+   pr_err(Failed to enable critical clock %s\n,
+   clocks[i]);
}
 }
 

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-07 Thread Kevin Hilman
On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd sb...@codeaurora.org wrote:
 On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:

 Instead I guess we could hook it less deep into clk_get_sys, like in the
 following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only patch?
 I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
 looks the same and it also still defers nicely in the scenario I needed it
 for. The implementation also looks nice - and of course much more compact 
 than
 my check in two places :-) . I don't know if you want to put this as 
 follow-up
 on top or fold it into the original orphan-check, so in any case

 Tested-by: Heiko Stuebner he...@sntech.de
 Reviewed-by: Heiko Stuebner he...@sntech.de

 Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
 my patch and a note that it's based on an earlier patch from you.

It appears this has landed in linux-next in the form of 882667c1fcf1
clk: prevent orphan clocks from being used.  A bunch of boot failures
for sunxi in today's linux-next[1] were bisected down to that patch.

I confirmed that reverting that commit on top of next/master gets
sunxi booting again.

Kevin

[1] http://kernelci.org/boot/all/job/next/kernel/next-20150507/
--
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/2] clk: improve handling of orphan clocks

2015-05-01 Thread Stephen Boyd
On 05/01/15 15:07, Heiko Stübner wrote:
> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>
>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>> following patch?
>> It looks like it will work at least, but still I'd prefer to keep the
>> orphan check contained to clk.c. How about this compile tested only patch?
> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
> looks the same and it also still defers nicely in the scenario I needed it 
> for. The implementation also looks nice - and of course much more compact 
> than 
> my check in two places :-) . I don't know if you want to put this as 
> follow-up 
> on top or fold it into the original orphan-check, so in any case
>
> Tested-by: Heiko Stuebner 
> Reviewed-by: Heiko Stuebner 

Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.

>
>
>> This also brings up an existing problem with clk_unregister() where
>> orphaned clocks are sitting out there useable by drivers when their
>> parent is unregistered. That code could use some work to atomically
>> switch all the orphaned clocks over to use the nodrv_ops.
> Not sure I understand this correctly yet, but when these children get 
> orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
> back if the parent reappears.
>
> So I guess we would need to store the original ops in secondary property of 
> struct clk_core and I guess simply bind the ops-switch to the orphan state 
> update?

Yep. We'll need to store away the original ops in case we need to put
them back. Don't feel obligated to fix this either. It would certainly
be nice if someone tried to fix this case at some point, but it's not
like things are any worse off right now.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-01 Thread Heiko Stübner
Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> On 05/01/15 12:59, Heiko Stübner wrote:
> > Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
> >> On 04/22, Heiko Stuebner wrote:
> >>> Using orphan clocks can introduce strange behaviour as they don't have
> >>> rate information at all and also of course don't track
> >>> 
> >>> This v2/v3 takes into account suggestions from Stephen Boyd to not try
> >>> to
> >>> walk the clock tree at runtime but instead keep track of orphan states
> >>> on clock tree changes and making it mandatory for everybody from the
> >>> start as orphaned clocks should not be used at all.
> >>> 
> >>> 
> >>> This fixes an issue on most rk3288 platforms, where some soc-clocks
> >>> are supplied by a 32khz clock from an external i2c-chip which often
> >>> is only probed later in the boot process and maybe even after the
> >>> drivers using these soc-clocks like the tsadc temperature sensor.
> >>> In this case the driver using the clock should of course defer probing
> >>> until the clock is actually usable.
> >>> 
> >>> 
> >>> As this changes the behaviour for orphan clocks, it would of course
> >>> benefit from more testing than on my Rockchip boards. To keep the
> >>> recipent-list reasonable and not spam to much I selected one (the
> >>> topmost)
> >>> from the get_maintainer output of each drivers/clk entry.
> >>> Hopefully some will provide Tested-by-tags :-)
> >> 
> >>  I don't see any Tested-by: tags yet . I've
> >> put these two patches on a separate branch "defer-orphans" and
> >> pushed it to clk-next so we can give it some more exposure.
> >> 
> >> Unfortunately this doesn't solve the orphan problem for non-OF
> >> providers. What if we did the orphan check in __clk_create_clk()
> >> instead and returned an error pointer for orphans? I suspect that
> >> will solve all cases, right?
> > 
> > hmm, clk_register also uses __clk_create_clk, which in turn would prevent
> > registering orphan-clocks at all, I'd think.
> > As on my platform I'm dependant on orphan clocks (the soc-level clock gets
> > registerted as part of the big clock controller way before the i2c-based
> > supplying clock), I'd rather not touch this :-) .
> 
> Have no fear! We should just change clk_register() to call a
> __clk_create_clk() type function that doesn't check for orphan status.

ok :-D


> > Instead I guess we could hook it less deep into clk_get_sys, like in the
> > following patch?
> 
> It looks like it will work at least, but still I'd prefer to keep the
> orphan check contained to clk.c. How about this compile tested only patch?

I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
looks the same and it also still defers nicely in the scenario I needed it 
for. The implementation also looks nice - and of course much more compact than 
my check in two places :-) . I don't know if you want to put this as follow-up 
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner 
Reviewed-by: Heiko Stuebner 


> This also brings up an existing problem with clk_unregister() where
> orphaned clocks are sitting out there useable by drivers when their
> parent is unregistered. That code could use some work to atomically
> switch all the orphaned clocks over to use the nodrv_ops.

Not sure I understand this correctly yet, but when these children get 
orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
back if the parent reappears.

So I guess we would need to store the original ops in secondary property of 
struct clk_core and I guess simply bind the ops-switch to the orphan state 
update?


> 
> 8<-
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 30d45c657a07..1d23daa42dd2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct
> clk_core *core) }
>  #endif
> 
> -static bool clk_is_orphan(const struct clk *clk)
> -{
> - if (!clk)
> - return false;
> -
> - return clk->core->orphan;
> -}
> -
>  /**
>   * __clk_init - initialize the data structures in a struct clk
>   * @dev: device initializing this clk, placeholder for now
> @@ -2420,15 +2412,11 @@ out:
>   return ret;
>  }
> 
> -struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
> -  const char *con_id)
> +static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
> +  const char *con_id)
>  {
>   struct clk *clk;
> 
> - /* This is to allow this function to be chained to others */
> - if (!hw || IS_ERR(hw))
> - return (struct clk *) hw;
> -
>   clk = kzalloc(sizeof(*clk), GFP_KERNEL);
>   if (!clk)
>   return ERR_PTR(-ENOMEM);
> @@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const
> char *dev_id, return clk;
>  }
> 
> +struct clk *__clk_create_clk(struct clk_hw *hw, const 

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-01 Thread Stephen Boyd
On 05/01/15 12:59, Heiko Stübner wrote:
> Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
>> On 04/22, Heiko Stuebner wrote:
>>> Using orphan clocks can introduce strange behaviour as they don't have
>>> rate information at all and also of course don't track
>>>
>>> This v2/v3 takes into account suggestions from Stephen Boyd to not try to
>>> walk the clock tree at runtime but instead keep track of orphan states
>>> on clock tree changes and making it mandatory for everybody from the
>>> start as orphaned clocks should not be used at all.
>>>
>>>
>>> This fixes an issue on most rk3288 platforms, where some soc-clocks
>>> are supplied by a 32khz clock from an external i2c-chip which often
>>> is only probed later in the boot process and maybe even after the
>>> drivers using these soc-clocks like the tsadc temperature sensor.
>>> In this case the driver using the clock should of course defer probing
>>> until the clock is actually usable.
>>>
>>>
>>> As this changes the behaviour for orphan clocks, it would of course
>>> benefit from more testing than on my Rockchip boards. To keep the
>>> recipent-list reasonable and not spam to much I selected one (the topmost)
>>> from the get_maintainer output of each drivers/clk entry.
>>> Hopefully some will provide Tested-by-tags :-)
>>  I don't see any Tested-by: tags yet . I've
>> put these two patches on a separate branch "defer-orphans" and
>> pushed it to clk-next so we can give it some more exposure.
>>
>> Unfortunately this doesn't solve the orphan problem for non-OF
>> providers. What if we did the orphan check in __clk_create_clk()
>> instead and returned an error pointer for orphans? I suspect that
>> will solve all cases, right?
> hmm, clk_register also uses __clk_create_clk, which in turn would prevent
> registering orphan-clocks at all, I'd think.
> As on my platform I'm dependant on orphan clocks (the soc-level clock gets
> registerted as part of the big clock controller way before the i2c-based
> supplying clock), I'd rather not touch this :-) .

Have no fear! We should just change clk_register() to call a
__clk_create_clk() type function that doesn't check for orphan status.

>
> Instead I guess we could hook it less deep into clk_get_sys, like in the
> following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only patch?

This also brings up an existing problem with clk_unregister() where
orphaned clocks are sitting out there useable by drivers when their
parent is unregistered. That code could use some work to atomically
switch all the orphaned clocks over to use the nodrv_ops.

8<-

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 30d45c657a07..1d23daa42dd2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
-{
-   if (!clk)
-   return false;
-
-   return clk->core->orphan;
-}
-
 /**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:   device initializing this clk, placeholder for now
@@ -2420,15 +2412,11 @@ out:
return ret;
 }
 
-struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
-const char *con_id)
+static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
+const char *con_id)
 {
struct clk *clk;
 
-   /* This is to allow this function to be chained to others */
-   if (!hw || IS_ERR(hw))
-   return (struct clk *) hw;
-
clk = kzalloc(sizeof(*clk), GFP_KERNEL);
if (!clk)
return ERR_PTR(-ENOMEM);
@@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const 
char *dev_id,
return clk;
 }
 
+struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
+const char *con_id)
+{
+   /* This is to allow this function to be chained to others */
+   if (!hw || IS_ERR(hw))
+   return (struct clk *) hw;
+
+   if (hw->core->orphan)
+   return ERR_PTR(-EPROBE_DEFER);
+
+   return clk_hw_create_clk(hw, dev_id, con_id);
+}
+
 void __clk_free_clk(struct clk *clk)
 {
clk_prepare_lock();
@@ -2511,7 +2512,7 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
 
INIT_HLIST_HEAD(>clks);
 
-   hw->clk = __clk_create_clk(hw, NULL, NULL);
+   hw->clk = clk_hw_create_clk(hw, NULL, NULL);
if (IS_ERR(hw->clk)) {
ret = PTR_ERR(hw->clk);
goto fail_parent_names_copy;
@@ -2958,10 +2959,6 @@ struct clk *__of_clk_get_from_provider(struct 
of_phandle_args *clkspec,
if (provider->node == clkspec->np)
clk = provider->get(clkspec, provider->data);
if (!IS_ERR(clk)) {
-  

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-01 Thread Heiko Stübner
Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
> On 04/22, Heiko Stuebner wrote:
> > Using orphan clocks can introduce strange behaviour as they don't have
> > rate information at all and also of course don't track
> > 
> > This v2/v3 takes into account suggestions from Stephen Boyd to not try to
> > walk the clock tree at runtime but instead keep track of orphan states
> > on clock tree changes and making it mandatory for everybody from the
> > start as orphaned clocks should not be used at all.
> > 
> > 
> > This fixes an issue on most rk3288 platforms, where some soc-clocks
> > are supplied by a 32khz clock from an external i2c-chip which often
> > is only probed later in the boot process and maybe even after the
> > drivers using these soc-clocks like the tsadc temperature sensor.
> > In this case the driver using the clock should of course defer probing
> > until the clock is actually usable.
> > 
> > 
> > As this changes the behaviour for orphan clocks, it would of course
> > benefit from more testing than on my Rockchip boards. To keep the
> > recipent-list reasonable and not spam to much I selected one (the topmost)
> > from the get_maintainer output of each drivers/clk entry.
> > Hopefully some will provide Tested-by-tags :-)
> 
>  I don't see any Tested-by: tags yet . I've
> put these two patches on a separate branch "defer-orphans" and
> pushed it to clk-next so we can give it some more exposure.
> 
> Unfortunately this doesn't solve the orphan problem for non-OF
> providers. What if we did the orphan check in __clk_create_clk()
> instead and returned an error pointer for orphans? I suspect that
> will solve all cases, right?

hmm, clk_register also uses __clk_create_clk, which in turn would prevent
registering orphan-clocks at all, I'd think.
As on my platform I'm dependant on orphan clocks (the soc-level clock gets
registerted as part of the big clock controller way before the i2c-based
supplying clock), I'd rather not touch this :-) .

Instead I guess we could hook it less deep into clk_get_sys, like in the
following patch?


 8< - >8 -
From: Heiko Stuebner 
Date: Fri, 1 May 2015 21:50:46 +0200
Subject: [PATCH] clk: prevent orphan access on non-devicetree platforms too

The orphan-check in __of_clk_get_from_provider only prevents orphan-access
on devicetree platforms. To bring non-dt platforms to the same level of
functionality let clk_get_sys (called from clk_get for non-dt platforms)
also check for orphans and return -EPROBE_DEFER in that case.

Signed-off-by: Heiko Stuebner 
---
 drivers/clk/clk.c| 2 +-
 drivers/clk/clk.h| 5 +
 drivers/clk/clkdev.c | 5 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 36d1a01..167d0bf 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2220,7 +2220,7 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
+bool clk_is_orphan(const struct clk *clk)
 {
if (!clk)
return false;
diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h
index 00b35a1..b8a6061 100644
--- a/drivers/clk/clk.h
+++ b/drivers/clk/clk.h
@@ -20,6 +20,7 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args 
*clkspec,
 struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 const char *con_id);
 void __clk_free_clk(struct clk *clk);
+bool clk_is_orphan(const struct clk *clk);
 #else
 /* All these casts to avoid ifdefs in clkdev... */
 static inline struct clk *
@@ -32,5 +33,9 @@ static struct clk_hw *__clk_get_hw(struct clk *clk)
 {
return (struct clk_hw *)clk;
 }
+static inline bool clk_is_orphan(const struct clk *clk)
+{
+   return false;
+}
 
 #endif
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 1fcb6ef..ad96775 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -177,6 +177,11 @@ struct clk *clk_get_sys(const char *dev_id, const char 
*con_id)
if (!cl)
goto out;
 
+   if (clk_is_orphan(cl->clk)) {
+   clk = ERR_PTR(-EPROBE_DEFER);
+   goto out;
+   }
+
clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id);
if (IS_ERR(clk))
goto out;
-- 
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/2] clk: improve handling of orphan clocks

2015-05-01 Thread Stephen Boyd
On 05/01/15 15:07, Heiko Stübner wrote:
 Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:

 Instead I guess we could hook it less deep into clk_get_sys, like in the
 following patch?
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only patch?
 I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
 looks the same and it also still defers nicely in the scenario I needed it 
 for. The implementation also looks nice - and of course much more compact 
 than 
 my check in two places :-) . I don't know if you want to put this as 
 follow-up 
 on top or fold it into the original orphan-check, so in any case

 Tested-by: Heiko Stuebner he...@sntech.de
 Reviewed-by: Heiko Stuebner he...@sntech.de

Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.



 This also brings up an existing problem with clk_unregister() where
 orphaned clocks are sitting out there useable by drivers when their
 parent is unregistered. That code could use some work to atomically
 switch all the orphaned clocks over to use the nodrv_ops.
 Not sure I understand this correctly yet, but when these children get 
 orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
 back if the parent reappears.

 So I guess we would need to store the original ops in secondary property of 
 struct clk_core and I guess simply bind the ops-switch to the orphan state 
 update?

Yep. We'll need to store away the original ops in case we need to put
them back. Don't feel obligated to fix this either. It would certainly
be nice if someone tried to fix this case at some point, but it's not
like things are any worse off right now.

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-05-01 Thread Heiko Stübner
Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
 On 04/22, Heiko Stuebner wrote:
  Using orphan clocks can introduce strange behaviour as they don't have
  rate information at all and also of course don't track
  
  This v2/v3 takes into account suggestions from Stephen Boyd to not try to
  walk the clock tree at runtime but instead keep track of orphan states
  on clock tree changes and making it mandatory for everybody from the
  start as orphaned clocks should not be used at all.
  
  
  This fixes an issue on most rk3288 platforms, where some soc-clocks
  are supplied by a 32khz clock from an external i2c-chip which often
  is only probed later in the boot process and maybe even after the
  drivers using these soc-clocks like the tsadc temperature sensor.
  In this case the driver using the clock should of course defer probing
  until the clock is actually usable.
  
  
  As this changes the behaviour for orphan clocks, it would of course
  benefit from more testing than on my Rockchip boards. To keep the
  recipent-list reasonable and not spam to much I selected one (the topmost)
  from the get_maintainer output of each drivers/clk entry.
  Hopefully some will provide Tested-by-tags :-)
 
 grumble I don't see any Tested-by: tags yet /grumble. I've
 put these two patches on a separate branch defer-orphans and
 pushed it to clk-next so we can give it some more exposure.
 
 Unfortunately this doesn't solve the orphan problem for non-OF
 providers. What if we did the orphan check in __clk_create_clk()
 instead and returned an error pointer for orphans? I suspect that
 will solve all cases, right?

hmm, clk_register also uses __clk_create_clk, which in turn would prevent
registering orphan-clocks at all, I'd think.
As on my platform I'm dependant on orphan clocks (the soc-level clock gets
registerted as part of the big clock controller way before the i2c-based
supplying clock), I'd rather not touch this :-) .

Instead I guess we could hook it less deep into clk_get_sys, like in the
following patch?


 8 - 8 -
From: Heiko Stuebner he...@sntech.de
Date: Fri, 1 May 2015 21:50:46 +0200
Subject: [PATCH] clk: prevent orphan access on non-devicetree platforms too

The orphan-check in __of_clk_get_from_provider only prevents orphan-access
on devicetree platforms. To bring non-dt platforms to the same level of
functionality let clk_get_sys (called from clk_get for non-dt platforms)
also check for orphans and return -EPROBE_DEFER in that case.

Signed-off-by: Heiko Stuebner he...@sntech.de
---
 drivers/clk/clk.c| 2 +-
 drivers/clk/clk.h| 5 +
 drivers/clk/clkdev.c | 5 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 36d1a01..167d0bf 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2220,7 +2220,7 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
+bool clk_is_orphan(const struct clk *clk)
 {
if (!clk)
return false;
diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h
index 00b35a1..b8a6061 100644
--- a/drivers/clk/clk.h
+++ b/drivers/clk/clk.h
@@ -20,6 +20,7 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args 
*clkspec,
 struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 const char *con_id);
 void __clk_free_clk(struct clk *clk);
+bool clk_is_orphan(const struct clk *clk);
 #else
 /* All these casts to avoid ifdefs in clkdev... */
 static inline struct clk *
@@ -32,5 +33,9 @@ static struct clk_hw *__clk_get_hw(struct clk *clk)
 {
return (struct clk_hw *)clk;
 }
+static inline bool clk_is_orphan(const struct clk *clk)
+{
+   return false;
+}
 
 #endif
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 1fcb6ef..ad96775 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -177,6 +177,11 @@ struct clk *clk_get_sys(const char *dev_id, const char 
*con_id)
if (!cl)
goto out;
 
+   if (clk_is_orphan(cl-clk)) {
+   clk = ERR_PTR(-EPROBE_DEFER);
+   goto out;
+   }
+
clk = __clk_create_clk(__clk_get_hw(cl-clk), dev_id, con_id);
if (IS_ERR(clk))
goto out;
-- 
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/2] clk: improve handling of orphan clocks

2015-05-01 Thread Stephen Boyd
On 05/01/15 12:59, Heiko Stübner wrote:
 Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
 On 04/22, Heiko Stuebner wrote:
 Using orphan clocks can introduce strange behaviour as they don't have
 rate information at all and also of course don't track

 This v2/v3 takes into account suggestions from Stephen Boyd to not try to
 walk the clock tree at runtime but instead keep track of orphan states
 on clock tree changes and making it mandatory for everybody from the
 start as orphaned clocks should not be used at all.


 This fixes an issue on most rk3288 platforms, where some soc-clocks
 are supplied by a 32khz clock from an external i2c-chip which often
 is only probed later in the boot process and maybe even after the
 drivers using these soc-clocks like the tsadc temperature sensor.
 In this case the driver using the clock should of course defer probing
 until the clock is actually usable.


 As this changes the behaviour for orphan clocks, it would of course
 benefit from more testing than on my Rockchip boards. To keep the
 recipent-list reasonable and not spam to much I selected one (the topmost)
 from the get_maintainer output of each drivers/clk entry.
 Hopefully some will provide Tested-by-tags :-)
 grumble I don't see any Tested-by: tags yet /grumble. I've
 put these two patches on a separate branch defer-orphans and
 pushed it to clk-next so we can give it some more exposure.

 Unfortunately this doesn't solve the orphan problem for non-OF
 providers. What if we did the orphan check in __clk_create_clk()
 instead and returned an error pointer for orphans? I suspect that
 will solve all cases, right?
 hmm, clk_register also uses __clk_create_clk, which in turn would prevent
 registering orphan-clocks at all, I'd think.
 As on my platform I'm dependant on orphan clocks (the soc-level clock gets
 registerted as part of the big clock controller way before the i2c-based
 supplying clock), I'd rather not touch this :-) .

Have no fear! We should just change clk_register() to call a
__clk_create_clk() type function that doesn't check for orphan status.


 Instead I guess we could hook it less deep into clk_get_sys, like in the
 following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only patch?

This also brings up an existing problem with clk_unregister() where
orphaned clocks are sitting out there useable by drivers when their
parent is unregistered. That code could use some work to atomically
switch all the orphaned clocks over to use the nodrv_ops.

8-

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 30d45c657a07..1d23daa42dd2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
-{
-   if (!clk)
-   return false;
-
-   return clk-core-orphan;
-}
-
 /**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:   device initializing this clk, placeholder for now
@@ -2420,15 +2412,11 @@ out:
return ret;
 }
 
-struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
-const char *con_id)
+static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
+const char *con_id)
 {
struct clk *clk;
 
-   /* This is to allow this function to be chained to others */
-   if (!hw || IS_ERR(hw))
-   return (struct clk *) hw;
-
clk = kzalloc(sizeof(*clk), GFP_KERNEL);
if (!clk)
return ERR_PTR(-ENOMEM);
@@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const 
char *dev_id,
return clk;
 }
 
+struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
+const char *con_id)
+{
+   /* This is to allow this function to be chained to others */
+   if (!hw || IS_ERR(hw))
+   return (struct clk *) hw;
+
+   if (hw-core-orphan)
+   return ERR_PTR(-EPROBE_DEFER);
+
+   return clk_hw_create_clk(hw, dev_id, con_id);
+}
+
 void __clk_free_clk(struct clk *clk)
 {
clk_prepare_lock();
@@ -2511,7 +2512,7 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
 
INIT_HLIST_HEAD(core-clks);
 
-   hw-clk = __clk_create_clk(hw, NULL, NULL);
+   hw-clk = clk_hw_create_clk(hw, NULL, NULL);
if (IS_ERR(hw-clk)) {
ret = PTR_ERR(hw-clk);
goto fail_parent_names_copy;
@@ -2958,10 +2959,6 @@ struct clk *__of_clk_get_from_provider(struct 
of_phandle_args *clkspec,
if (provider-node == clkspec-np)
clk = provider-get(clkspec, provider-data);
if (!IS_ERR(clk)) {
-   if (clk_is_orphan(clk)) {
-   clk = 

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-05-01 Thread Heiko Stübner
Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
 On 05/01/15 12:59, Heiko Stübner wrote:
  Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
  On 04/22, Heiko Stuebner wrote:
  Using orphan clocks can introduce strange behaviour as they don't have
  rate information at all and also of course don't track
  
  This v2/v3 takes into account suggestions from Stephen Boyd to not try
  to
  walk the clock tree at runtime but instead keep track of orphan states
  on clock tree changes and making it mandatory for everybody from the
  start as orphaned clocks should not be used at all.
  
  
  This fixes an issue on most rk3288 platforms, where some soc-clocks
  are supplied by a 32khz clock from an external i2c-chip which often
  is only probed later in the boot process and maybe even after the
  drivers using these soc-clocks like the tsadc temperature sensor.
  In this case the driver using the clock should of course defer probing
  until the clock is actually usable.
  
  
  As this changes the behaviour for orphan clocks, it would of course
  benefit from more testing than on my Rockchip boards. To keep the
  recipent-list reasonable and not spam to much I selected one (the
  topmost)
  from the get_maintainer output of each drivers/clk entry.
  Hopefully some will provide Tested-by-tags :-)
  
  grumble I don't see any Tested-by: tags yet /grumble. I've
  put these two patches on a separate branch defer-orphans and
  pushed it to clk-next so we can give it some more exposure.
  
  Unfortunately this doesn't solve the orphan problem for non-OF
  providers. What if we did the orphan check in __clk_create_clk()
  instead and returned an error pointer for orphans? I suspect that
  will solve all cases, right?
  
  hmm, clk_register also uses __clk_create_clk, which in turn would prevent
  registering orphan-clocks at all, I'd think.
  As on my platform I'm dependant on orphan clocks (the soc-level clock gets
  registerted as part of the big clock controller way before the i2c-based
  supplying clock), I'd rather not touch this :-) .
 
 Have no fear! We should just change clk_register() to call a
 __clk_create_clk() type function that doesn't check for orphan status.

ok :-D


  Instead I guess we could hook it less deep into clk_get_sys, like in the
  following patch?
 
 It looks like it will work at least, but still I'd prefer to keep the
 orphan check contained to clk.c. How about this compile tested only patch?

I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
looks the same and it also still defers nicely in the scenario I needed it 
for. The implementation also looks nice - and of course much more compact than 
my check in two places :-) . I don't know if you want to put this as follow-up 
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner he...@sntech.de
Reviewed-by: Heiko Stuebner he...@sntech.de


 This also brings up an existing problem with clk_unregister() where
 orphaned clocks are sitting out there useable by drivers when their
 parent is unregistered. That code could use some work to atomically
 switch all the orphaned clocks over to use the nodrv_ops.

Not sure I understand this correctly yet, but when these children get 
orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
back if the parent reappears.

So I guess we would need to store the original ops in secondary property of 
struct clk_core and I guess simply bind the ops-switch to the orphan state 
update?


 
 8-
 
 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index 30d45c657a07..1d23daa42dd2 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct
 clk_core *core) }
  #endif
 
 -static bool clk_is_orphan(const struct clk *clk)
 -{
 - if (!clk)
 - return false;
 -
 - return clk-core-orphan;
 -}
 -
  /**
   * __clk_init - initialize the data structures in a struct clk
   * @dev: device initializing this clk, placeholder for now
 @@ -2420,15 +2412,11 @@ out:
   return ret;
  }
 
 -struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 -  const char *con_id)
 +static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
 +  const char *con_id)
  {
   struct clk *clk;
 
 - /* This is to allow this function to be chained to others */
 - if (!hw || IS_ERR(hw))
 - return (struct clk *) hw;
 -
   clk = kzalloc(sizeof(*clk), GFP_KERNEL);
   if (!clk)
   return ERR_PTR(-ENOMEM);
 @@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const
 char *dev_id, return clk;
  }
 
 +struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 +  const char *con_id)
 +{
 + /* This is to allow this function to be chained to others */
 + if (!hw || 

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

2015-04-30 Thread Stephen Boyd
On 04/22, Heiko Stuebner wrote:
> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track 
> 
> This v2/v3 takes into account suggestions from Stephen Boyd to not try to
> walk the clock tree at runtime but instead keep track of orphan states
> on clock tree changes and making it mandatory for everybody from the
> start as orphaned clocks should not be used at all.
> 
> 
> This fixes an issue on most rk3288 platforms, where some soc-clocks
> are supplied by a 32khz clock from an external i2c-chip which often
> is only probed later in the boot process and maybe even after the
> drivers using these soc-clocks like the tsadc temperature sensor.
> In this case the driver using the clock should of course defer probing
> until the clock is actually usable.
> 
> 
> As this changes the behaviour for orphan clocks, it would of course
> benefit from more testing than on my Rockchip boards. To keep the
> recipent-list reasonable and not spam to much I selected one (the topmost)
> from the get_maintainer output of each drivers/clk entry.
> Hopefully some will provide Tested-by-tags :-)
> 

 I don't see any Tested-by: tags yet . I've
put these two patches on a separate branch "defer-orphans" and
pushed it to clk-next so we can give it some more exposure.

Unfortunately this doesn't solve the orphan problem for non-OF
providers. What if we did the orphan check in __clk_create_clk()
instead and returned an error pointer for orphans? I suspect that
will solve all cases, right?

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-04-30 Thread Stephen Boyd
On 04/22, Heiko Stuebner wrote:
 Using orphan clocks can introduce strange behaviour as they don't have
 rate information at all and also of course don't track 
 
 This v2/v3 takes into account suggestions from Stephen Boyd to not try to
 walk the clock tree at runtime but instead keep track of orphan states
 on clock tree changes and making it mandatory for everybody from the
 start as orphaned clocks should not be used at all.
 
 
 This fixes an issue on most rk3288 platforms, where some soc-clocks
 are supplied by a 32khz clock from an external i2c-chip which often
 is only probed later in the boot process and maybe even after the
 drivers using these soc-clocks like the tsadc temperature sensor.
 In this case the driver using the clock should of course defer probing
 until the clock is actually usable.
 
 
 As this changes the behaviour for orphan clocks, it would of course
 benefit from more testing than on my Rockchip boards. To keep the
 recipent-list reasonable and not spam to much I selected one (the topmost)
 from the get_maintainer output of each drivers/clk entry.
 Hopefully some will provide Tested-by-tags :-)
 

grumble I don't see any Tested-by: tags yet /grumble. I've
put these two patches on a separate branch defer-orphans and
pushed it to clk-next so we can give it some more exposure.

Unfortunately this doesn't solve the orphan problem for non-OF
providers. What if we did the orphan check in __clk_create_clk()
instead and returned an error pointer for orphans? I suspect that
will solve all cases, right?

-- 
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 v3 0/2] clk: improve handling of orphan clocks

2015-04-26 Thread Robert Jarzmik
Heiko Stuebner  writes:

> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track
Ok, everything works as before on some pxa platforms.

As pxa clocks support is not even fully mainline, this test is not the best you
could get.

Cheers.

-- 
Robert
--
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/2] clk: improve handling of orphan clocks

2015-04-26 Thread Robert Jarzmik
Heiko Stuebner he...@sntech.de writes:

 Using orphan clocks can introduce strange behaviour as they don't have
 rate information at all and also of course don't track
Ok, everything works as before on some pxa platforms.

As pxa clocks support is not even fully mainline, this test is not the best you
could get.

Cheers.

-- 
Robert
--
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/2] clk: improve handling of orphan clocks

2015-04-25 Thread Heiko Stübner
Hi Stefan,

Am Samstag, 25. April 2015, 14:23:39 schrieb Stefan Wahren:
> > Heiko Stuebner  hat am 22. April 2015 um 22:53
> > geschrieben:
> > 
> > 
> > Using orphan clocks can introduce strange behaviour as they don't have
> > rate information at all and also of course don't track
> > 
> > [...]
> > 
> > 
> > As this changes the behaviour for orphan clocks, it would of course
> > benefit from more testing than on my Rockchip boards. To keep the
> > recipent-list reasonable and not spam to much I selected one (the topmost)
> > from the get_maintainer output of each drivers/clk entry.
> > Hopefully some will provide Tested-by-tags :-)
> 
> excuse me for my naive question, but what kind of tests do you expect
> (beside applying the patches)?

just a "everything that worked before still works" :-)

Using orphaned clocks should already produce strange issues most of the time - 
for example I see clk_disable mismatches when suspending a rk3288 board, 
before this patchset.

But nevertheless we now disallow the usage of orphaned clocks completely while 
before it was possible to knowingly/unknowingly use them.

And while hopefully most drivers should handle an EPROBE_DEFER from clk_get 
just fine, there may still be one or two lurking around that would need fixing 
then ;-)


Heiko
--
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/2] clk: improve handling of orphan clocks

2015-04-25 Thread Stefan Wahren
Hi Heiko,

> Heiko Stuebner  hat am 22. April 2015 um 22:53 geschrieben:
>
>
> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track
>
> [...]
>
>
> As this changes the behaviour for orphan clocks, it would of course
> benefit from more testing than on my Rockchip boards. To keep the
> recipent-list reasonable and not spam to much I selected one (the topmost)
> from the get_maintainer output of each drivers/clk entry.
> Hopefully some will provide Tested-by-tags :-)
>

excuse me for my naive question, but what kind of tests do you expect (beside
applying the patches)?

Stefan

>
> Thanks
> Heiko
>
--
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/2] clk: improve handling of orphan clocks

2015-04-25 Thread Stefan Wahren
Hi Heiko,

 Heiko Stuebner he...@sntech.de hat am 22. April 2015 um 22:53 geschrieben:


 Using orphan clocks can introduce strange behaviour as they don't have
 rate information at all and also of course don't track

 [...]


 As this changes the behaviour for orphan clocks, it would of course
 benefit from more testing than on my Rockchip boards. To keep the
 recipent-list reasonable and not spam to much I selected one (the topmost)
 from the get_maintainer output of each drivers/clk entry.
 Hopefully some will provide Tested-by-tags :-)


excuse me for my naive question, but what kind of tests do you expect (beside
applying the patches)?

Stefan


 Thanks
 Heiko

--
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/2] clk: improve handling of orphan clocks

2015-04-25 Thread Heiko Stübner
Hi Stefan,

Am Samstag, 25. April 2015, 14:23:39 schrieb Stefan Wahren:
  Heiko Stuebner he...@sntech.de hat am 22. April 2015 um 22:53
  geschrieben:
  
  
  Using orphan clocks can introduce strange behaviour as they don't have
  rate information at all and also of course don't track
  
  [...]
  
  
  As this changes the behaviour for orphan clocks, it would of course
  benefit from more testing than on my Rockchip boards. To keep the
  recipent-list reasonable and not spam to much I selected one (the topmost)
  from the get_maintainer output of each drivers/clk entry.
  Hopefully some will provide Tested-by-tags :-)
 
 excuse me for my naive question, but what kind of tests do you expect
 (beside applying the patches)?

just a everything that worked before still works :-)

Using orphaned clocks should already produce strange issues most of the time - 
for example I see clk_disable mismatches when suspending a rk3288 board, 
before this patchset.

But nevertheless we now disallow the usage of orphaned clocks completely while 
before it was possible to knowingly/unknowingly use them.

And while hopefully most drivers should handle an EPROBE_DEFER from clk_get 
just fine, there may still be one or two lurking around that would need fixing 
then ;-)


Heiko
--
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/


[PATCH v3 0/2] clk: improve handling of orphan clocks

2015-04-22 Thread Heiko Stuebner
Using orphan clocks can introduce strange behaviour as they don't have
rate information at all and also of course don't track 

This v2/v3 takes into account suggestions from Stephen Boyd to not try to
walk the clock tree at runtime but instead keep track of orphan states
on clock tree changes and making it mandatory for everybody from the
start as orphaned clocks should not be used at all.


This fixes an issue on most rk3288 platforms, where some soc-clocks
are supplied by a 32khz clock from an external i2c-chip which often
is only probed later in the boot process and maybe even after the
drivers using these soc-clocks like the tsadc temperature sensor.
In this case the driver using the clock should of course defer probing
until the clock is actually usable.


As this changes the behaviour for orphan clocks, it would of course
benefit from more testing than on my Rockchip boards. To keep the
recipent-list reasonable and not spam to much I selected one (the topmost)
from the get_maintainer output of each drivers/clk entry.
Hopefully some will provide Tested-by-tags :-)


Thanks
Heiko

changes since v2:
adapt to comments from Stephen Boyd
- rename to clk_core_update_orphan_status
- use bools instead of 0/1 for the status
- remove redundant check in clk_is_orphan
changes since v1:
- track orphan status on clock tree changes instead of walking the
  tree on clk_get operations
- make get-deferals mandatory for everybody

Cc: Boris Brezillon 
Cc: Alex Elder 
Cc: Alexandre Belloni 
Cc: Stephen Warren 
Cc: Max Filippov 
Cc: ker...@pengutronix.de
Cc: Zhangfei Gao 
Cc: Santosh Shilimkar 
Cc: Chao Xie 
Cc: Jason Cooper 
Cc: Stefan Wahren 
Cc: Andrew Bresticker 
Cc: Robert Jarzmik 
Cc: Georgi Djakov 
Cc: Sylwester Nawrocki 
Cc: Geert Uytterhoeven 
Cc: Barry Song 
Cc: Dinh Nguyen 
Cc: Viresh Kumar 
Cc: Gabriel FERNANDEZ 
Cc: emi...@elopez.com.ar
Cc: Peter De Schrijver 
Cc: Tero Kristo 
Cc: Ulf Hansson 
Cc: Pawel Moll 
Cc: Michal Simek 


Heiko Stuebner (2):
  clk: track the orphan status of clocks and their children
  clk: prevent orphan clocks from being used

 drivers/clk/clk.c | 46 +++---
 1 file changed, 43 insertions(+), 3 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/


[PATCH v3 0/2] clk: improve handling of orphan clocks

2015-04-22 Thread Heiko Stuebner
Using orphan clocks can introduce strange behaviour as they don't have
rate information at all and also of course don't track 

This v2/v3 takes into account suggestions from Stephen Boyd to not try to
walk the clock tree at runtime but instead keep track of orphan states
on clock tree changes and making it mandatory for everybody from the
start as orphaned clocks should not be used at all.


This fixes an issue on most rk3288 platforms, where some soc-clocks
are supplied by a 32khz clock from an external i2c-chip which often
is only probed later in the boot process and maybe even after the
drivers using these soc-clocks like the tsadc temperature sensor.
In this case the driver using the clock should of course defer probing
until the clock is actually usable.


As this changes the behaviour for orphan clocks, it would of course
benefit from more testing than on my Rockchip boards. To keep the
recipent-list reasonable and not spam to much I selected one (the topmost)
from the get_maintainer output of each drivers/clk entry.
Hopefully some will provide Tested-by-tags :-)


Thanks
Heiko

changes since v2:
adapt to comments from Stephen Boyd
- rename to clk_core_update_orphan_status
- use bools instead of 0/1 for the status
- remove redundant check in clk_is_orphan
changes since v1:
- track orphan status on clock tree changes instead of walking the
  tree on clk_get operations
- make get-deferals mandatory for everybody

Cc: Boris Brezillon boris.brezil...@free-electrons.com
Cc: Alex Elder el...@linaro.org
Cc: Alexandre Belloni alexandre.bell...@free-electrons.com
Cc: Stephen Warren swar...@wwwdotorg.org
Cc: Max Filippov jcmvb...@gmail.com
Cc: ker...@pengutronix.de
Cc: Zhangfei Gao zhangfei@linaro.org
Cc: Santosh Shilimkar ssant...@kernel.org
Cc: Chao Xie chao@marvell.com
Cc: Jason Cooper ja...@lakedaemon.net
Cc: Stefan Wahren stefan.wah...@i2se.com
Cc: Andrew Bresticker abres...@chromium.org
Cc: Robert Jarzmik robert.jarz...@free.fr
Cc: Georgi Djakov georgi.dja...@linaro.org
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Geert Uytterhoeven geert+rene...@glider.be
Cc: Barry Song bao...@kernel.org
Cc: Dinh Nguyen dingu...@opensource.altera.com
Cc: Viresh Kumar viresh.li...@gmail.com
Cc: Gabriel FERNANDEZ gabriel.fernan...@st.com
Cc: emi...@elopez.com.ar
Cc: Peter De Schrijver pdeschrij...@nvidia.com
Cc: Tero Kristo t-kri...@ti.com
Cc: Ulf Hansson ulf.hans...@linaro.org
Cc: Pawel Moll pawel.m...@arm.com
Cc: Michal Simek michal.si...@xilinx.com


Heiko Stuebner (2):
  clk: track the orphan status of clocks and their children
  clk: prevent orphan clocks from being used

 drivers/clk/clk.c | 46 +++---
 1 file changed, 43 insertions(+), 3 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/