diff mbox

[06/35] mfd: ab8500-core: Sysfs chip id modification

Message ID 1360933026-30325-7-git-send-email-lee.jones@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Lee Jones Feb. 15, 2013, 12:56 p.m. UTC 
From: Marcus Cooper <marcus.xm.cooper@stericsson.com>

On newer variants of the ab a version and revision value can be
read. This modification merges these two values to create a
chip id.

Signed-off-by: Marcus Cooper <marcus.xm.cooper@stericsson.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Reviewed-by: Mattias WALLIN <mattias.wallin@stericsson.com>
Tested-by: Jonas ABERG <jonas.aberg@stericsson.com>
---
 drivers/mfd/ab8500-core.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann Feb. 19, 2013, 10:04 p.m. UTC | #1 
On Friday 15 February 2013, Lee Jones wrote:
>         struct ab8500 *ab8500;
> +       int chip_id = -EINVAL;
>
>         ab8500 = dev_get_drvdata(dev);
> -       return sprintf(buf, "%#x\n", ab8500 ? ab8500->chip_id : -EINVAL);
> +       if(ab8500) {
> +               chip_id = ab8500->chip_id;
> +               if((is_ab8505(ab8500) || is_ab9540(ab8500)) && ab8500->version != 0xFF)
> +                       chip_id = (ab8500->version << 8) | chip_id;
> +       }
> +       return sprintf(buf, "%#x\n", chip_id);
>  }

What's the use of printing "ffffffea" for unknown versions here?

	Arnd
Lee Jones Feb. 20, 2013, 8:13 a.m. UTC | #2 
On Tue, 19 Feb 2013, Arnd Bergmann wrote:

> On Friday 15 February 2013, Lee Jones wrote:
> >         struct ab8500 *ab8500;
> > +       int chip_id = -EINVAL;
> >
> >         ab8500 = dev_get_drvdata(dev);
> > -       return sprintf(buf, "%#x\n", ab8500 ? ab8500->chip_id : -EINVAL);
> > +       if(ab8500) {
> > +               chip_id = ab8500->chip_id;
> > +               if((is_ab8505(ab8500) || is_ab9540(ab8500)) && ab8500->version != 0xFF)
> > +                       chip_id = (ab8500->version << 8) | chip_id;
> > +       }
> > +       return sprintf(buf, "%#x\n", chip_id);
> >  }
> 
> What's the use of printing "ffffffea" for unknown versions here?

You mean instead of -EINVAL? No idea, Marcus?
Code Kipper Feb. 20, 2013, 8:53 a.m. UTC | #3 
On 20 February 2013 09:13, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 19 Feb 2013, Arnd Bergmann wrote:
>
>> On Friday 15 February 2013, Lee Jones wrote:
>> >         struct ab8500 *ab8500;
>> > +       int chip_id = -EINVAL;
>> >
>> >         ab8500 = dev_get_drvdata(dev);
>> > -       return sprintf(buf, "%#x\n", ab8500 ? ab8500->chip_id : -EINVAL);
>> > +       if(ab8500) {
>> > +               chip_id = ab8500->chip_id;
>> > +               if((is_ab8505(ab8500) || is_ab9540(ab8500)) && ab8500->version != 0xFF)
>> > +                       chip_id = (ab8500->version << 8) | chip_id;
>> > +       }
>> > +       return sprintf(buf, "%#x\n", chip_id);
>> >  }
>>
>> What's the use of printing "ffffffea" for unknown versions here?
>
> You mean instead of -EINVAL? No idea, Marcus?
>
Looks like I'm guilty of just making the minimal changes. Arnd is
right though, getting ffffffea(-EINVAL) back is pretty useless.
I'll have to check user land to see what is using this. Maybe not
printing and returning 0 should be the correct behaviour
when an unknown version is found.
Lee Jones Feb. 20, 2013, 10:43 a.m. UTC | #4 
On Wed, 20 Feb 2013, Marcus Cooper wrote:

> On 20 February 2013 09:13, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 19 Feb 2013, Arnd Bergmann wrote:
> >
> >> On Friday 15 February 2013, Lee Jones wrote:
> >> >         struct ab8500 *ab8500;
> >> > +       int chip_id = -EINVAL;
> >> >
> >> >         ab8500 = dev_get_drvdata(dev);
> >> > -       return sprintf(buf, "%#x\n", ab8500 ? ab8500->chip_id : -EINVAL);
> >> > +       if(ab8500) {
> >> > +               chip_id = ab8500->chip_id;
> >> > +               if((is_ab8505(ab8500) || is_ab9540(ab8500)) && ab8500->version != 0xFF)
> >> > +                       chip_id = (ab8500->version << 8) | chip_id;
> >> > +       }
> >> > +       return sprintf(buf, "%#x\n", chip_id);
> >> >  }
> >>
> >> What's the use of printing "ffffffea" for unknown versions here?
> >
> > You mean instead of -EINVAL? No idea, Marcus?
> >
> Looks like I'm guilty of just making the minimal changes. Arnd is
> right though, getting ffffffea(-EINVAL) back is pretty useless.
> I'll have to check user land to see what is using this.

Yes, it would be useful to know how it's parsed.

Thanks Marcus.

> Maybe not
> printing and returning 0 should be the correct behaviour
> when an unknown version is found.
diff mbox

Patch

diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index 7c20728..678afd0 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -1095,9 +1095,15 @@  static ssize_t show_chip_id(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
 	struct ab8500 *ab8500;
+	int chip_id = -EINVAL;
 
 	ab8500 = dev_get_drvdata(dev);
-	return sprintf(buf, "%#x\n", ab8500 ? ab8500->chip_id : -EINVAL);
+	if(ab8500) {
+		chip_id = ab8500->chip_id;
+		if((is_ab8505(ab8500) || is_ab9540(ab8500)) && ab8500->version != 0xFF)
+			chip_id = (ab8500->version << 8) | chip_id;
+	}
+	return sprintf(buf, "%#x\n", chip_id);
 }
 
 /*