Skip to contentSkip to conversation

"clk: add non CONFIG_HAVE_CLK routines" commit

linux-next Paul Gortmaker 6 participants 11 messages May 22, 2012 – Jun 14, 2012 Linux-next tree

lore
Share & tools
lore

Paul Gortmaker

author
#0 lore raw

Hi Viresh,

The above listed commit, in linux-next as:

commit ebbf0cb5d39cc3e22ef4c425475e76b7f45027de

"clk: add non CONFIG_HAVE_CLK routines"

shows up as failures in the netx_defconfig, since there are duplicate stub functions between your changes and the file below:

arch/arm/mach-netx/fb.c:72:6: error: redefinition of 'clk_disable' include/linux/clk.h:299:51: note: previous definition of 'clk_disable' was here arch/arm/mach-netx/fb.c:76:5: error: redefinition of 'clk_set_rate' include/linux/clk.h:306:50: note: previous definition of 'clk_set_rate' was here arch/arm/mach-netx/fb.c:81:5: error: redefinition of 'clk_enable' include/linux/clk.h:294:50: note: previous definition of 'clk_enable' was here arch/arm/mach-netx/fb.c:86:13: error: redefinition of 'clk_get' include/linux/clk.h:280:58: note: previous definition of 'clk_get' was here arch/arm/mach-netx/fb.c:91:6: error: redefinition of 'clk_put' include/linux/clk.h:290:51: note: previous definition of 'clk_put' was here make[2]: *** [arch/arm/mach-netx/fb.o] Error 1

Can you have a look please, and delete the overlap?

Thanks,
Paul.

#1 lore raw
Quoted 7 lines · Paul Gortmaker

Hi Viresh,

The above listed commit, in linux-next as:

commit ebbf0cb5d39cc3e22ef4c425475e76b7f45027de

Quoted 8 lines

"clk: add non CONFIG_HAVE_CLK routines"

shows up as failures in the netx_defconfig, since there are duplicate stub functions between your changes and the file below:

arch/arm/mach-netx/fb.c:72:6: error: redefinition of 'clk_disable' include/linux/clk.h:299:51: note: previous definition of 'clk_disable'

was here

Quoted 2 lines

arch/arm/mach-netx/fb.c:76:5: error: redefinition of 'clk_set_rate' include/linux/clk.h:306:50: note: previous definition of 'clk_set_rate'

was here

Quoted 2 lines

arch/arm/mach-netx/fb.c:81:5: error: redefinition of 'clk_enable' include/linux/clk.h:294:50: note: previous definition of 'clk_enable'

was here

Quoted 2 lines

arch/arm/mach-netx/fb.c:86:13: error: redefinition of 'clk_get' include/linux/clk.h:280:58: note: previous definition of 'clk_get' was

here

Quoted 2 lines

arch/arm/mach-netx/fb.c:91:6: error: redefinition of 'clk_put' include/linux/clk.h:290:51: note: previous definition of 'clk_put' was

here

Quoted 2 lines

make[2]: *** [arch/arm/mach-netx/fb.o] Error 1

Hi Paul,

I don't have access to any Linux machine for now as i am on leave.

Actually i have left ST and will join the next company in few weeks.

If you can provide a patch for this, i can review it.

--
Viresh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120525/80f39cb7/attachment-0001.html>

#2 lore raw
In reply to Paul Gortmaker

Hi Paul/Russell,

Quoted 12 lines · Paul Gortmaker

Hi Viresh,

The above listed commit, in linux-next as:

commit ebbf0cb5d39cc3e22ef4c425475e76b7f45027de

? ?"clk: add non CONFIG_HAVE_CLK routines"

shows up as failures in the netx_defconfig, since there are duplicate stub functions between your changes and the file below:

Quoted 11 lines

?arch/arm/mach-netx/fb.c:72:6: error: redefinition of 'clk_disable' ?include/linux/clk.h:299:51: note: previous definition of 'clk_disable' was here ?arch/arm/mach-netx/fb.c:76:5: error: redefinition of 'clk_set_rate' ?include/linux/clk.h:306:50: note: previous definition of 'clk_set_rate' was here ?arch/arm/mach-netx/fb.c:81:5: error: redefinition of 'clk_enable' ?include/linux/clk.h:294:50: note: previous definition of 'clk_enable' was here ?arch/arm/mach-netx/fb.c:86:13: error: redefinition of 'clk_get' ?include/linux/clk.h:280:58: note: previous definition of 'clk_get' was here ?arch/arm/mach-netx/fb.c:91:6: error: redefinition of 'clk_put' ?include/linux/clk.h:290:51: note: previous definition of 'clk_put' was here ?make[2]: *** [arch/arm/mach-netx/fb.o] Error 1

Following are present in mach-netx/fb.c:

void clk_disable(struct clk *clk)
{
}

int clk_set_rate(struct clk *clk, unsigned long rate)

{
	return 0;
}

int clk_enable(struct clk *clk)

{
	return 0;
}

void clk_put(struct clk *clk)
{
}

struct clk *clk_get(struct device *dev, const char *id)

{
	return dev && strcmp(dev_name(dev), "fb") == 0 ? NULL : ERR_PTR(-ENOENT);
}

I can remove first four without any issues, but just can't remove the last one. The dummy clk_get() always returns NULL, whereas this one returns NULL only for fb device.

How should I fix this?

--
Viresh

#3 lore raw
Quoted 1 line

Following are present in mach-netx/fb.c:

[snip]

Quoted 10 lines

struct clk *clk_get(struct device *dev, const char *id)

{
	return dev && strcmp(dev_name(dev), "fb") == 0 ? NULL : ERR_PTR(-ENOENT);
}

I can remove first four without any issues, but just can't remove the last one. The dummy clk_get() always returns NULL, whereas this one returns NULL only for fb device.

How should I fix this?

Another thing I'm not sure is that the typical clk_get() callers only tests IS_ERR() on the returned value. As a clk newbie, I don't know whether there will be lots of clk users seeing this (new) NULL value on !CONFIG_HAVE_CLK and whether the current behavior is correct..

Thanks,
Fengguang

#4 lore raw
Quoted 4 lines · Fengguang Wu

Another thing I'm not sure is that the typical clk_get() callers only tests IS_ERR() on the returned value. As a clk newbie, I don't know whether there will be lots of clk users seeing this (new) NULL value on !CONFIG_HAVE_CLK and whether the current behavior is correct..

There have been long discussions over it in past, check this thread

https://lkml.org/lkml/2012/4/24/389

--
Viresh

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#5 lore raw
In reply to viresh kumar
Quoted 8 lines · viresh kumar

struct clk *clk_get(struct device *dev, const char *id)

{
	return dev && strcmp(dev_name(dev), "fb") == 0 ? NULL : ERR_PTR(-ENOENT);
}

I can remove first four without any issues, but just can't remove the last one. The dummy clk_get() always returns NULL, whereas this one returns NULL only for fb device.

This isn't a totally dummy clk implementation. What it's doing is returning a dummy clock for the framebuffer, and explicitly failing everything else (because we have no need to support any other clocks.)

If we want to use the dummy clk stuff in its entirety, and just return the dummy clock for all clk_get()s on this platform, then there isn't a problem. If we want to keep this behaviour, we need to use clkdev with the dummy clk implementation.

#6 lore raw
Quoted 10 lines · Russell King - ARM Linux
Quoted 8 lines · viresh kumar

struct clk *clk_get(struct device *dev, const char *id)

{
     return dev && strcmp(dev_name(dev), "fb") == 0 ? NULL : ERR_PTR(-ENOENT);
}

I can remove first four without any issues, but just can't remove the last one. The dummy clk_get() always returns NULL, whereas this one returns NULL only for fb device.

Quoted 8 lines

This isn't a totally dummy clk implementation. What it's doing is returning a dummy clock for the framebuffer, and explicitly failing everything else (because we have no need to support any other clocks.)

If we want to use the dummy clk stuff in its entirety, and just return the dummy clock for all clk_get()s on this platform, then there isn't a problem. If we want to keep this behaviour, we need to use clkdev with the dummy clk implementation.

First one is easier and I would go with that, if maintainer of that file/module accepts it.

@Sascha: Are you maintaining this module?

--
Viresh

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

#7 lore raw
In reply to viresh kumar

Hi Viresh,

Quoted 54 lines · viresh kumar

Hi Paul/Russell,

Quoted 12 lines · Paul Gortmaker

Hi Viresh,

The above listed commit, in linux-next as:

commit ebbf0cb5d39cc3e22ef4c425475e76b7f45027de

? ?"clk: add non CONFIG_HAVE_CLK routines"

shows up as failures in the netx_defconfig, since there are duplicate stub functions between your changes and the file below:

Quoted 11 lines

?arch/arm/mach-netx/fb.c:72:6: error: redefinition of 'clk_disable' ?include/linux/clk.h:299:51: note: previous definition of 'clk_disable' was here ?arch/arm/mach-netx/fb.c:76:5: error: redefinition of 'clk_set_rate' ?include/linux/clk.h:306:50: note: previous definition of 'clk_set_rate' was here ?arch/arm/mach-netx/fb.c:81:5: error: redefinition of 'clk_enable' ?include/linux/clk.h:294:50: note: previous definition of 'clk_enable' was here ?arch/arm/mach-netx/fb.c:86:13: error: redefinition of 'clk_get' ?include/linux/clk.h:280:58: note: previous definition of 'clk_get' was here ?arch/arm/mach-netx/fb.c:91:6: error: redefinition of 'clk_put' ?include/linux/clk.h:290:51: note: previous definition of 'clk_put' was here ?make[2]: *** [arch/arm/mach-netx/fb.o] Error 1

Following are present in mach-netx/fb.c:

void clk_disable(struct clk *clk)
{
}

int clk_set_rate(struct clk *clk, unsigned long rate)

{
	return 0;
}

int clk_enable(struct clk *clk)

{
	return 0;
}

void clk_put(struct clk *clk)
{
}

struct clk *clk_get(struct device *dev, const char *id)

{
	return dev && strcmp(dev_name(dev), "fb") == 0 ? NULL : ERR_PTR(-ENOENT);
}
Quoted 5 lines

I can remove first four without any issues, but just can't remove the last one. The dummy clk_get() always returns NULL, whereas this one returns NULL only for fb device.

How should I fix this?

As the netx does not have fb support in mainline you can just drop this one aswell. It has no use.

Sascha

Signature · 4 lines
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-5555 |
#8 lore raw
Quoted 2 lines · Sascha Hauer

As the netx does not have fb support in mainline you can just drop this one aswell. It has no use.

Are you sure?

Grepping for clcd in arch/arm/mach-netx shows that nxdb500 and nxeb500hmi seem to have all the necessary stuff to support the AMBA CLCD driver.

#9 lore raw
Quoted 8 lines · Russell King - ARM Linux
Quoted 2 lines · Sascha Hauer

As the netx does not have fb support in mainline you can just drop this one aswell. It has no use.

Are you sure?

Grepping for clcd in arch/arm/mach-netx shows that nxdb500 and nxeb500hmi seem to have all the necessary stuff to support the AMBA CLCD driver.

Damn, you are right. I wasn't aware the netx has a AMBA cell and just grepped for netx in drivers/video.
In this case I suggest to just return NULL for every clock.

Sascha

Signature · 4 lines
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-5555 |
#10 lore raw
Quoted 3 lines · Sascha Hauer

Damn, you are right. I wasn't aware the netx has a AMBA cell and just grepped for netx in drivers/video.
In this case I suggest to just return NULL for every clock.

And dropping fb's clk code would still be the right solution, as dummy clk routines will
do that for you.

--
viresh