Re: [PATCH] clkdev: Add default clkdev.h

Re: [PATCH] clkdev: Add default clkdev.h

[Geert Uytterhoeven]

On Tue, Jul 3, 2012 at 5:23 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> --- a/include/asm-generic/Kbuild.asm
> +++ b/include/asm-generic/Kbuild.asm
> @@ -13,6 +13,10 @@ ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/a.out.h \
>  header-y += a.out.h
>  endif
>
> +ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/clkdev.h),)
> +generic-y += clkdev.h
> +endif

Quoting Sam:
"Any use of wildcards in include/asm-generic/Kbuild.asm is bogus."

http://lkml.indiana.edu/hypermail/linux/kernel/1206.2/01507.html

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] clkdev: Add default clkdev.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

On Wed, Jul 04, 2012 at 10:00:50AM +0200, Geert Uytterhoeven wrote:

> Quoting Sam:
> "Any use of wildcards in include/asm-generic/Kbuild.asm is bogus."

> http://lkml.indiana.edu/hypermail/linux/kernel/1206.2/01507.html

Without actually explaining why which isn't terribly helpful :/

As far as I can tell it looks like this is for headers which may
optionally be absent but clkdev.h is a header which should never be
absent, either the architecture defines it or it uses the generic one.

As I said further up the thread having to go round ever single
architecture and get them to actually apply patches for stuff like this
is needlessly painful.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clkdev: Add default clkdev.h

[Arnd Bergmann]

On Tuesday 03 July 2012, Mark Brown wrote:
> On Tue, Jul 03, 2012 at 12:00:59PM +1000, Stephen Rothwell wrote:
> > On Mon,  2 Jul 2012 19:04:10 +0100 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > > +#define __clk_get(clk)     ({ 1; })
> > > +#define __clk_put(clk)     do { } while (0)
> 
> > So why not:
> 
> > struct clk;
> 
> > static inline int __clk_get(struct clk *clk) { return 1; }
> > static inline void __clk_put(struct clk *clk) ( )
> 
> No idea, I'm just taking the existing default header and making it more
> generally available.

I prefer Stephen's version because that maintains the type checking.
However, that is not a controversial point at all IMHO. The question
that we need to decide on is whether we want automatic "generic-y"
statements in Kbuild.asm.

I'm not opposed to the idea, but I can imagine some arch maintainers
fine it confusing. When you first posted the patch, I spent some
time trying to get the wildcard logic moved to scripts/Makefile.asm-generic
and scripts/Makefile.headersinst but couldn't figure it out in the end.

I would definitely prefer being able to just write 

generic-y += clkdev.h 

in include/asm-generic/Kbuild.asm, rather than 

ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/clkdev.h),)
generic-y += clkdev.h
endif

Maybe Michal Marek or someone on the kbuild mailing list can help
out with a solution for that.

	Arnd

Re: [PATCH] clkdev: Add default clkdev.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

On Tue, Jul 03, 2012 at 11:16:05AM +0000, Arnd Bergmann wrote:
> On Tuesday 03 July 2012, Mark Brown wrote:

> > No idea, I'm just taking the existing default header and making it more
> > generally available.

> I prefer Stephen's version because that maintains the type checking.

I agree, I've already made the change locally.

> However, that is not a controversial point at all IMHO. The question
> that we need to decide on is whether we want automatic "generic-y"
> statements in Kbuild.asm.

Well, it does seem to defeat a large part of the point of having generic
headers if they're not there by default - it means that if you add a
header you need to go round every single architecture adding the header
explicitly which is cumbersome and annoying, especially with the less
actively maintained architectures.  On the other hand if you do things
without adding a generic header then people whine about not using the
generic header mechanism...

> I would definitely prefer being able to just write 

> generic-y += clkdev.h 

I agree but I don't see this as something that should block having the
facility.  Or if it does I guess I'll need to try to either fix clkdev
to use the arch include optionally or go round all the architectures
either of which is going to be needlessly painful.

> Maybe Michal Marek or someone on the kbuild mailing list can help
> out with a solution for that.

We could do that incrementally...

The reason I'm doing this is that the lack of a widely available clock
API is a considerable pain point for a lot of the stuff I work on and
this is the major blocker to just enabling the default implementation on
architectures that don't support it already which would be a big step
forwards.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clkdev: Add default clkdev.h

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

Hi Mark,

On Tue, 3 Jul 2012 12:48:44 +0100 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> > I would definitely prefer being able to just write 
> 
> > generic-y += clkdev.h 
> 
> I agree but I don't see this as something that should block having the
> facility.  Or if it does I guess I'll need to try to either fix clkdev
> to use the arch include optionally or go round all the architectures
> either of which is going to be needlessly painful.

True.

> > Maybe Michal Marek or someone on the kbuild mailing list can help
> > out with a solution for that.
> 
> We could do that incrementally...

Yes, indeed it should be a separate change.

> The reason I'm doing this is that the lack of a widely available clock
> API is a considerable pain point for a lot of the stuff I work on and
> this is the major blocker to just enabling the default implementation on
> architectures that don't support it already which would be a big step
> forwards.

Agreed.  Please submit this patch and we can concentrate on this other
stuff later.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clkdev: Add default clkdev.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Tue, Jul 03, 2012 at 10:05:12PM +1000, Stephen Rothwell wrote:
> On Tue, 3 Jul 2012 12:48:44 +0100 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > The reason I'm doing this is that the lack of a widely available clock
> > API is a considerable pain point for a lot of the stuff I work on and
> > this is the major blocker to just enabling the default implementation on
> > architectures that don't support it already which would be a big step
> > forwards.

> Agreed.  Please submit this patch and we can concentrate on this other
> stuff later.

Perhaps if you give me a couple of hours...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clkdev: Add default clkdev.h

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 915 bytes --]

Hi Mark,

On Tue, 3 Jul 2012 13:33:22 +0100 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> On Tue, Jul 03, 2012 at 10:05:12PM +1000, Stephen Rothwell wrote:
> > On Tue, 3 Jul 2012 12:48:44 +0100 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > > The reason I'm doing this is that the lack of a widely available clock
> > > API is a considerable pain point for a lot of the stuff I work on and
> > > this is the major blocker to just enabling the default implementation on
> > > architectures that don't support it already which would be a big step
> > > forwards.
> 
> > Agreed.  Please submit this patch and we can concentrate on this other
> > stuff later.
> 
> Perhaps if you give me a couple of hours...

Sorry, I wasn't meaning to rush you, just agreeing that the two things
are separate.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clkdev: Add default clkdev.h

[Arnd Bergmann]

On Tuesday 03 July 2012, Stephen Rothwell wrote:
> > > Maybe Michal Marek or someone on the kbuild mailing list can help
> > > out with a solution for that.
> > 
> > We could do that incrementally...
> 
> Yes, indeed it should be a separate change.

Ok, let's do that then. What tree should we use for merging it?

In theory I should take it to some asm-generic tree, but since
I get very little traffic there, I'm usually happy to just provide
an Ack and let anyone who needs the patch merge it through their
trees.

	Arnd

Re: [PATCH] clkdev: Add default clkdev.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

On Tue, Jul 03, 2012 at 01:39:34PM +0000, Arnd Bergmann wrote:

> Ok, let's do that then. What tree should we use for merging it?

> In theory I should take it to some asm-generic tree, but since
> I get very little traffic there, I'm usually happy to just provide
> an Ack and let anyone who needs the patch merge it through their
> trees.

I guess one of the clock trees which probably means Mike?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clkdev: Add default clkdev.h

[Mike Turquette]

On 20120703-14:47, Mark Brown wrote:
> On Tue, Jul 03, 2012 at 01:39:34PM +0000, Arnd Bergmann wrote:
> 
> > Ok, let's do that then. What tree should we use for merging it?
> 
> > In theory I should take it to some asm-generic tree, but since
> > I get very little traffic there, I'm usually happy to just provide
> > an Ack and let anyone who needs the patch merge it through their
> > trees.
> 
> I guess one of the clock trees which probably means Mike?

Hi Mark,

Russell owns clkdev so I'd prefer he take this patch.

Thanks,
Mike

Re: [PATCH] clkdev: Add default clkdev.h

[Russell King - ARM Linux]

On Mon, Jul 09, 2012 at 03:23:32PM -0700, Mike Turquette wrote:
> On 20120703-14:47, Mark Brown wrote:
> > On Tue, Jul 03, 2012 at 01:39:34PM +0000, Arnd Bergmann wrote:
> > 
> > > Ok, let's do that then. What tree should we use for merging it?
> > 
> > > In theory I should take it to some asm-generic tree, but since
> > > I get very little traffic there, I'm usually happy to just provide
> > > an Ack and let anyone who needs the patch merge it through their
> > > trees.
> > 
> > I guess one of the clock trees which probably means Mike?
> 
> Hi Mark,
> 
> Russell owns clkdev so I'd prefer he take this patch.

Note that I'm not going to be taking any further patches for at least
the next 16 days (as I'm away), and I'm not going to even guarantee
that I'll pick up on anything on the mailing lists during that period.

Re: [PATCH] clkdev: Add default clkdev.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

On Tue, Jul 10, 2012 at 12:30:22AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 09, 2012 at 03:23:32PM -0700, Mike Turquette wrote:

> > Russell owns clkdev so I'd prefer he take this patch.

> Note that I'm not going to be taking any further patches for at least
> the next 16 days (as I'm away), and I'm not going to even guarantee
> that I'll pick up on anything on the mailing lists during that period.

Oh dear, that's a bit sad - given that the merge window may well open
it'll mean we'll end up with at least one additional release where we
can't get the clock API available as standard :(

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clkdev: Add default clkdev.h

[Arnd Bergmann]

On Tuesday 10 July 2012, Mark Brown wrote:
> On Tue, Jul 10, 2012 at 12:30:22AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jul 09, 2012 at 03:23:32PM -0700, Mike Turquette wrote:
> 
> > > Russell owns clkdev so I'd prefer he take this patch.
> 
> > Note that I'm not going to be taking any further patches for at least
> > the next 16 days (as I'm away), and I'm not going to even guarantee
> > that I'll pick up on anything on the mailing lists during that period.
> 
> Oh dear, that's a bit sad - given that the merge window may well open
> it'll mean we'll end up with at least one additional release where we
> can't get the clock API available as standard :(

I can start an asm-generic branch in the arm-soc tree then (or maybe
a new tree) and apply the patch there.

	Arnd

Re: [PATCH] clkdev: Add default clkdev.h

[Paul Mundt]

On Tue, Jul 10, 2012 at 06:07:28PM +0000, Arnd Bergmann wrote:
> On Tuesday 10 July 2012, Mark Brown wrote:
> > On Tue, Jul 10, 2012 at 12:30:22AM +0100, Russell King - ARM Linux wrote:
> > > On Mon, Jul 09, 2012 at 03:23:32PM -0700, Mike Turquette wrote:
> > 
> > > > Russell owns clkdev so I'd prefer he take this patch.
> > 
> > > Note that I'm not going to be taking any further patches for at least
> > > the next 16 days (as I'm away), and I'm not going to even guarantee
> > > that I'll pick up on anything on the mailing lists during that period.
> > 
> > Oh dear, that's a bit sad - given that the merge window may well open
> > it'll mean we'll end up with at least one additional release where we
> > can't get the clock API available as standard :(
> 
> I can start an asm-generic branch in the arm-soc tree then (or maybe
> a new tree) and apply the patch there.
> 
Given that this patch has nothing at all to do with ARM, why not just
take it through the asm-generic tree?

Re: [PATCH] clkdev: Add default clkdev.h

[Arnd Bergmann]

On Wednesday 11 July 2012, Paul Mundt wrote:
> > I can start an asm-generic branch in the arm-soc tree then (or maybe
> > a new tree) and apply the patch there.
> > 
> Given that this patch has nothing at all to do with ARM, why not just
> take it through the asm-generic tree?

Just my personal lazyness. I haven't had an asm-generic tree for a while,
and my main working tree is a mirror of the arm-soc tree.

On the other hand, I looked up the maintainers file and it points to
git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git
so maybe I should restore that one after all or remove the reference
if I don't.

	Arnd