2009-01-08 09:50:27

by Davide Rizzo

[permalink] [raw]
Subject: [PATCH 2/3] Driver for user access to internal clocks

Specific for Samsung S3C24xx platforms
Allows any driver to get a device specific clock.
It recognizes also the form clkname.X

Signed-off-by: Davide Rizzo <[email protected]>
---
diff -urNp linux-2.6.28/arch/arm/plat-s3c24xx/clock.c
linux-2.6.28.elpa/arch/arm/plat-s3c24xx/clock.c
--- linux-2.6.28/arch/arm/plat-s3c24xx/clock.c 2008-12-25
00:26:37.000000000 +0100
+++ linux-2.6.28.elpa/arch/arm/plat-s3c24xx/clock.c 2009-01-07
19:25:07.000000000 +0100
@@ -67,11 +67,19 @@ static int clk_null_enable(struct clk *c

struct clk *clk_get(struct device *dev, const char *id)
{
+ long idno;
+ char *name = (char *)id;
+ char *dotpos = strrchr(id, '.');
struct clk *p;
struct clk *clk = ERR_PTR(-ENOENT);
- int idno;

- if (dev == NULL || dev->bus != &platform_bus_type)
+ if (dotpos) {
+ int err = strict_strtol(dotpos + 1, 10, &idno);
+ if (err)
+ return ERR_PTR(err);
+ name = kstrdup(id, GFP_KERNEL);
+ name[dotpos - id] = '\0';
+ } else if (dev == NULL || dev->bus != &platform_bus_type)
idno = -1;
else
idno = to_platform_device(dev)->id;
@@ -80,7 +88,7 @@ struct clk *clk_get(struct device *dev,

list_for_each_entry(p, &clocks, list) {
if (p->id == idno &&
- strcmp(id, p->name) == 0 &&
+ strcmp(name, p->name) == 0 &&
try_module_get(p->owner)) {
clk = p;
break;
@@ -92,7 +100,7 @@ struct clk *clk_get(struct device *dev,

if (IS_ERR(clk)) {
list_for_each_entry(p, &clocks, list) {
- if (p->id == -1 && strcmp(id, p->name) == 0 &&
+ if (p->id == -1 && strcmp(name, p->name) == 0 &&
try_module_get(p->owner)) {
clk = p;
break;
@@ -101,6 +109,9 @@ struct clk *clk_get(struct device *dev,
}

mutex_unlock(&clocks_mutex);
+
+ if (dotpos)
+ kfree(name);
return clk;
}


2009-01-08 12:20:08

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 2/3] Driver for user access to internal clocks

On Thu, Jan 08, 2009 at 10:50:03AM +0100, Davide Rizzo wrote:
> Specific for Samsung S3C24xx platforms
> Allows any driver to get a device specific clock.
> It recognizes also the form clkname.X
>
> Signed-off-by: Davide Rizzo <[email protected]>
> ---
> diff -urNp linux-2.6.28/arch/arm/plat-s3c24xx/clock.c
> linux-2.6.28.elpa/arch/arm/plat-s3c24xx/clock.c
> --- linux-2.6.28/arch/arm/plat-s3c24xx/clock.c 2008-12-25
> 00:26:37.000000000 +0100
> +++ linux-2.6.28.elpa/arch/arm/plat-s3c24xx/clock.c 2009-01-07
> 19:25:07.000000000 +0100
> @@ -67,11 +67,19 @@ static int clk_null_enable(struct clk *c
>
> struct clk *clk_get(struct device *dev, const char *id)
> {
> + long idno;
> + char *name = (char *)id;
> + char *dotpos = strrchr(id, '.');

you've borken the const of the clk_get() call. I think that
this is either something that the caller needs to deal with
as all the drivers will be passing a 'dev' structure in to
help identify common clocks.

> struct clk *p;
> struct clk *clk = ERR_PTR(-ENOENT);
> - int idno;
>
> - if (dev == NULL || dev->bus != &platform_bus_type)
> + if (dotpos) {
> + int err = strict_strtol(dotpos + 1, 10, &idno);
> + if (err)
> + return ERR_PTR(err);
> + name = kstrdup(id, GFP_KERNEL);
> + name[dotpos - id] = '\0';
> + } else if (dev == NULL || dev->bus != &platform_bus_type)
> idno = -1;
> else
> idno = to_platform_device(dev)->id;
> @@ -80,7 +88,7 @@ struct clk *clk_get(struct device *dev,
>
> list_for_each_entry(p, &clocks, list) {
> if (p->id == idno &&
> - strcmp(id, p->name) == 0 &&
> + strcmp(name, p->name) == 0 &&
> try_module_get(p->owner)) {
> clk = p;
> break;
> @@ -92,7 +100,7 @@ struct clk *clk_get(struct device *dev,
>
> if (IS_ERR(clk)) {
> list_for_each_entry(p, &clocks, list) {
> - if (p->id == -1 && strcmp(id, p->name) == 0 &&
> + if (p->id == -1 && strcmp(name, p->name) == 0 &&
> try_module_get(p->owner)) {
> clk = p;
> break;
> @@ -101,6 +109,9 @@ struct clk *clk_get(struct device *dev,
> }
>
> mutex_unlock(&clocks_mutex);
> +
> + if (dotpos)
> + kfree(name);
> return clk;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Ben ([email protected], http://www.fluff.org/)

'a smiley only costs 4 bytes'

2009-01-08 14:13:18

by Davide Rizzo

[permalink] [raw]
Subject: Re: [PATCH 2/3] Driver for user access to internal clocks

> you've borken the const of the clk_get() call. I think that
> this is either something that the caller needs to deal with
> as all the drivers will be passing a 'dev' structure in to
> help identify common clocks.
>
As I modified it the clk_get() function it should be compatible with
the past, simply now it recognizes ALSO the format clkname.X to allow
a generic driver to have access to all clocks.
The other way to achieve this is to simulate a specific id in a fake
dev parameter, but it's awful.
For the driver to be generic, all low-level drivers should recognize
the clkname.X format. But this is the standard syntax for devices
name, so I think this should be correct.
Also clk_for_each() and clk_name() should be added to clock
infrastructure, so they should be implemented in all drivers.
Maybe this should be proposed and discussed with the clock
infrastructure maintainer, but I didn't find him in the MAINTAINER
list. Do you know who is ?
Where do you think the generic user access driver should be placed ?