Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
On Mon, Sep 13, 2010 at 12:47:43PM -0700, Joe Perches wrote:
> Signed-off-by: Joe Perches
> ---
> drivers/i2c/busses/i2c-stu300.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index 495be45..2f7c09c 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
> struct resource *res;
> int bus_nr;
> int ret = 0;
> - char clk_name[] = "I2C0";
> + char clk_name[sizeof("I2Cx")];
>
> dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
> if (!dev) {
> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> }
>
> bus_nr = pdev->id;
> - clk_name[3] += (char)bus_nr;
> + sprintf(clk_name, "I2C%c", '0' + bus_nr);
> dev->clk = clk_get(&pdev->dev, clk_name);
> if (IS_ERR(dev->clk)) {
> ret = PTR_ERR(dev->clk);
personally, i'd prefer to see snprintf() used, to ensure that this sort
of thing never goes wrong if people decde to change the string. It is
a pitty there's no good way of temporarily allocating data an having it
freed in C.
As such, apologies to Russell about the abuse of the clk_get() call, but
let's not fix that just now.
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
On Tue, Sep 14, 2010 at 01:57:59PM +0100, Jonathan Cameron wrote:
> On 09/14/10 08:36, Lothar Waßmann wrote:
> > Hi,
> >
> > Jonathan Cameron writes:
> >> Commit message is somewhat inaccurate...
> >>
> >> On 09/13/10 20:47, Joe Perches wrote:
> >>> Signed-off-by: Joe Perches
> >>> ---
> >>> drivers/i2c/busses/i2c-stu300.c |4 ++--
> >>> 1 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/i2c/busses/i2c-stu300.c
> >>> b/drivers/i2c/busses/i2c-stu300.c
> >>> index 495be45..2f7c09c 100644
> >>> --- a/drivers/i2c/busses/i2c-stu300.c
> >>> +++ b/drivers/i2c/busses/i2c-stu300.c
> >>> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
> >>> struct resource *res;
> >>> int bus_nr;
> >>> int ret = 0;
> >>> - char clk_name[] = "I2C0";
> >>> + char clk_name[sizeof("I2Cx")];
> >>>
> >>> dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
> >>> if (!dev) {
> >>> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> >>> }
> >>>
> >>> bus_nr = pdev->id;
> >>> - clk_name[3] += (char)bus_nr;
> >>> + sprintf(clk_name, "I2C%c", '0' + bus_nr);
> >> I'm guessing that there are never more than a couple of these.
> >> Why is this method a better bet than just putting %d?
> >>
> > '%c' will only ever produce one byte of output while '%d' may
> > produce up to 11 bytes depending on the value of bus_nr thus
> > overflowing the buffer.
> Then use an snprintf, or apply a check to ensure it isn't bigger than
> 9.
>
> If you don't mind having clocks named i2c$ or something equally
> silly then I guess this is fine. To my mind, if that is possible
> this is a bug that should be fixed rather than avoided.
Even better, fix the implementation so that it conforms to the API
spec rather than doing its own thing with the consumer connection ID
argument and ignoring the struct device argument.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
On 09/14/10 08:36, Lothar Waßmann wrote:
> Hi,
>
> Jonathan Cameron writes:
>> Commit message is somewhat inaccurate...
>>
>> On 09/13/10 20:47, Joe Perches wrote:
>>> Signed-off-by: Joe Perches
>>> ---
>>> drivers/i2c/busses/i2c-stu300.c |4 ++--
>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-stu300.c
>>> b/drivers/i2c/busses/i2c-stu300.c
>>> index 495be45..2f7c09c 100644
>>> --- a/drivers/i2c/busses/i2c-stu300.c
>>> +++ b/drivers/i2c/busses/i2c-stu300.c
>>> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
>>> struct resource *res;
>>> int bus_nr;
>>> int ret = 0;
>>> - char clk_name[] = "I2C0";
>>> + char clk_name[sizeof("I2Cx")];
>>>
>>> dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
>>> if (!dev) {
>>> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
>>> }
>>>
>>> bus_nr = pdev->id;
>>> - clk_name[3] += (char)bus_nr;
>>> + sprintf(clk_name, "I2C%c", '0' + bus_nr);
>> I'm guessing that there are never more than a couple of these.
>> Why is this method a better bet than just putting %d?
>>
> '%c' will only ever produce one byte of output while '%d' may
> produce up to 11 bytes depending on the value of bus_nr thus
> overflowing the buffer.
Then use an snprintf, or apply a check to ensure it isn't bigger than
9.
If you don't mind having clocks named i2c$ or something equally
silly then I guess this is fine. To my mind, if that is possible
this is a bug that should be fixed rather than avoided.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
Hi,
Jonathan Cameron writes:
> Commit message is somewhat inaccurate...
>
> On 09/13/10 20:47, Joe Perches wrote:
> > Signed-off-by: Joe Perches
> > ---
> > drivers/i2c/busses/i2c-stu300.c |4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-stu300.c
> > b/drivers/i2c/busses/i2c-stu300.c
> > index 495be45..2f7c09c 100644
> > --- a/drivers/i2c/busses/i2c-stu300.c
> > +++ b/drivers/i2c/busses/i2c-stu300.c
> > @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
> > struct resource *res;
> > int bus_nr;
> > int ret = 0;
> > - char clk_name[] = "I2C0";
> > + char clk_name[sizeof("I2Cx")];
> >
> > dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
> > if (!dev) {
> > @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> > }
> >
> > bus_nr = pdev->id;
> > - clk_name[3] += (char)bus_nr;
> > + sprintf(clk_name, "I2C%c", '0' + bus_nr);
> I'm guessing that there are never more than a couple of these.
> Why is this method a better bet than just putting %d?
>
'%c' will only ever produce one byte of output while '%d' may
produce up to 11 bytes depending on the value of bus_nr thus
overflowing the buffer.
Lothar Waßmann
--
___
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | i...@karo-electronics.de
___
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
On Mon, 2010-09-13 at 21:08 +0100, Jonathan Cameron wrote: > Commit message is somewhat inaccurate... Yeah, sorry 'bout that. That's what I get for using a script. I did write an intro with more complete description. > > diff --git a/drivers/i2c/busses/i2c-stu300.c > > b/drivers/i2c/busses/i2c-stu300.c > > @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev) > > } > > > > bus_nr = pdev->id; > > - clk_name[3] += (char)bus_nr; > > + sprintf(clk_name, "I2C%c", '0' + bus_nr); > I'm guessing that there are never more than a couple of these. > Why is this method a better bet than just putting %d? It tries to standardize the style use and it avoids possible future checkpatch warnings of: char foo[] = "bar" char array could possibly be static const. There was another use with "%1.1d" somewhere. The end result is the same, so I don't really care much if this sort of change is applied or not. The possible checkpatch message could just be considered noise but Mike Frysinger seemed to prefer it, so I thought I could try to accommodate him. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
Commit message is somewhat inaccurate...
On 09/13/10 20:47, Joe Perches wrote:
> Signed-off-by: Joe Perches
> ---
> drivers/i2c/busses/i2c-stu300.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index 495be45..2f7c09c 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
> struct resource *res;
> int bus_nr;
> int ret = 0;
> - char clk_name[] = "I2C0";
> + char clk_name[sizeof("I2Cx")];
>
> dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
> if (!dev) {
> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> }
>
> bus_nr = pdev->id;
> - clk_name[3] += (char)bus_nr;
> + sprintf(clk_name, "I2C%c", '0' + bus_nr);
I'm guessing that there are never more than a couple of these.
Why is this method a better bet than just putting %d?
> dev->clk = clk_get(&pdev->dev, clk_name);
> if (IS_ERR(dev->clk)) {
> ret = PTR_ERR(dev->clk);
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/25] drivers/i2c: Use static const char arrays
On Mon, Sep 13, 2010 at 12:47:43PM -0700, Joe Perches wrote:
> Signed-off-by: Joe Perches
> ---
> drivers/i2c/busses/i2c-stu300.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index 495be45..2f7c09c 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
> struct resource *res;
> int bus_nr;
> int ret = 0;
> - char clk_name[] = "I2C0";
> + char clk_name[sizeof("I2Cx")];
>
> dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
> if (!dev) {
> @@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
> }
>
> bus_nr = pdev->id;
> - clk_name[3] += (char)bus_nr;
> + sprintf(clk_name, "I2C%c", '0' + bus_nr);
If people would implement the clk API correctly (matching by struct device
rather than giving every clock a name and trying to use the const char *
as the sole matching token), stuff like this would not be necessary.
The second argument to clk_get() is supposed to be a _consumer_ name
(iow, something to determine which clock for the struct device you want)
rather than a producer name.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/25] drivers/i2c: Use static const char arrays
Signed-off-by: Joe Perches
---
drivers/i2c/busses/i2c-stu300.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
index 495be45..2f7c09c 100644
--- a/drivers/i2c/busses/i2c-stu300.c
+++ b/drivers/i2c/busses/i2c-stu300.c
@@ -871,7 +871,7 @@ stu300_probe(struct platform_device *pdev)
struct resource *res;
int bus_nr;
int ret = 0;
- char clk_name[] = "I2C0";
+ char clk_name[sizeof("I2Cx")];
dev = kzalloc(sizeof(struct stu300_dev), GFP_KERNEL);
if (!dev) {
@@ -881,7 +881,7 @@ stu300_probe(struct platform_device *pdev)
}
bus_nr = pdev->id;
- clk_name[3] += (char)bus_nr;
+ sprintf(clk_name, "I2C%c", '0' + bus_nr);
dev->clk = clk_get(&pdev->dev, clk_name);
if (IS_ERR(dev->clk)) {
ret = PTR_ERR(dev->clk);
--
1.7.3.rc1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
