"clk: add non CONFIG_HAVE_CLK routines" commit

"clk: add non CONFIG_HAVE_CLK routines" commit

[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:

  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.

Re: [linux-next] "clk: add non CONFIG_HAVE_CLK routines" commit

[viresh kumar]

[-- Attachment #1.1: Type: text/plain, Size: 1393 bytes --]

On May 23, 2012 5:18 AM, "Paul Gortmaker" <paul.gortmaker@windriver.com>
wrote:
>
> 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
>

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

[-- Attachment #1.2: Type: text/html, Size: 1807 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[viresh kumar]

Hi Paul/Russell,

On Wed, May 23, 2012 at 12:46 AM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> 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

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

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[Fengguang Wu]

> Following are present in mach-netx/fb.c:
[snip]
> 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

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[viresh kumar]

On 12/06/12 12:09, Fengguang Wu wrote:
> 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.

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[Russell King - ARM Linux]

On Tue, Jun 12, 2012 at 11:57:07AM +0100, viresh kumar wrote:
> 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.

Re: [linux-next] "clk: add non CONFIG_HAVE_CLK routines" commit

[viresh kumar]

On 12/06/12 15:08, Russell King - ARM Linux wrote:
> On Tue, Jun 12, 2012 at 11:57:07AM +0100, viresh kumar wrote:
>> 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.

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.

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[Sascha Hauer]

Hi Viresh,

On Tue, Jun 12, 2012 at 11:57:07AM +0100, viresh kumar wrote:
> Hi Paul/Russell,
> 
> On Wed, May 23, 2012 at 12:46 AM, Paul Gortmaker
> <paul.gortmaker@windriver.com> wrote:
> > 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
> 
> 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?

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

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-5555 |

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[Russell King - ARM Linux]

On Thu, Jun 14, 2012 at 12:10:40AM +0200, Sascha Hauer wrote:
> 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.

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[Sascha Hauer]

On Thu, Jun 14, 2012 at 11:04:25AM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 14, 2012 at 12:10:40AM +0200, Sascha Hauer wrote:
> > 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

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

Re: "clk: add non CONFIG_HAVE_CLK routines" commit

[viresh kumar]

On Thu, Jun 14, 2012 at 11:18 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 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