From: lee.jones@linaro.org (Lee Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/5] mach-ux500: add a SoC ID (serial) callback for the u8500
Date: Fri, 02 Sep 2011 16:16:03 +0100 [thread overview]
Message-ID: <4E60F333.1010902@linaro.org> (raw)
In-Reply-To: <201109021622.50591.arnd@arndb.de>
On 02/09/11 15:22, Arnd Bergmann wrote:
> On Thursday 01 September 2011, Lee Jones wrote:
>> +static const char *db8500_get_soc_id(void)
>> +{
>> + void __iomem *uid_base;
>> + char buf[1024];
>> + ssize_t sz = 0;
>> + int i;
>> +
>> + uid_base = __io_address(U8500_BB_UID_BASE);
>> + for (i = 0; i < U8500_BB_UID_LENGTH; i++) {
>> + sz += sprintf(buf + sz, "%08x", readl(uid_base + i * sizeof(u32)));
>> + }
>> + return kasprintf(GFP_KERNEL, "%s", buf);
>> +}
>
> You will get a warning from the stack checker here, about putting a 1024 byte string
> on the stack. Also, I still think it's bad to just access the U8500_BB_UID_BASE
> from a compile-time constant. Since this gets called from a function that knows
> the base address of the DB8500 register area, better pass the device in there
> so that you end up with something like
>
> static void __devinit db8500_read_soc_id(struct db8500_dev *dev)
> {
> u32 __iomem *uid = dev->base + U8500_BB_UID_OFFSET;
> snprintf(dev->soc_id, sizeof (dev->soc_id), "%08x%08x%08x%08x%08x",
> readl(uid[0]), readl(uid[1]), readl(uid[2]), readl(uid[3]), readl(uid[4]));
> }
No problem.
Where did you get dev->base from though?
> The style you use here is preexisting in the db8500 code, but you should
> not keep adding more of that crap.
Not meaning to pass the buck at all, but this isn't my code.
It's remnant from when I took this over (starting to wish I hadn't ;))
> All the code like
> #define db8500_add_i2c0(pdata) \
> dbx500_add_i2c(0, U8500_I2C0_BASE, IRQ_DB8500_I2C0, pdata)
> #define db8500_add_i2c1(pdata) \
> dbx500_add_i2c(1, U8500_I2C1_BASE, IRQ_DB8500_I2C1, pdata)
>
> should never really have been there. What you want to do for this is
> to call this from the code that initializes the db8500 controller, and
> pass the board specific pdata into the db8500 init function, along
> with the i2c client data.
Not my domain. Speak to Linus W. :)
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2011-09-02 15:16 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-01 12:27 [PATCH 1/5] Framework for exporting System-on-Chip information via sysfs Lee Jones
2011-09-01 12:27 ` [PATCH 2/5] Add documentation for new sysfs devices/soc functionality Lee Jones
2011-09-01 12:27 ` [PATCH 3/5] mach-ux500: export System-on-Chip information ux500 via sysfs Lee Jones
2011-09-02 14:31 ` Arnd Bergmann
2011-09-01 12:27 ` [PATCH 4/5] mach-ux500: move top level platform devices in sysfs to /sys/devices/soc/X Lee Jones
2011-09-01 12:27 ` [PATCH 5/5] mach-ux500: add a SoC ID (serial) callback for the u8500 Lee Jones
2011-09-02 14:22 ` Arnd Bergmann
2011-09-02 15:16 ` Lee Jones [this message]
2011-09-02 15:56 ` Arnd Bergmann
2011-09-01 23:34 ` [PATCH 1/5] Framework for exporting System-on-Chip information via sysfs Greg KH
2011-09-02 8:44 ` Lee Jones
2011-09-02 9:29 ` Jamie Iles
2011-09-02 9:37 ` Lee Jones
2011-09-02 9:56 ` Jamie Iles
2011-09-02 16:31 ` Greg KH
2011-09-02 17:22 ` Arnd Bergmann
2011-09-02 18:14 ` Greg KH
2011-09-06 9:41 ` Lee Jones
2011-09-06 19:33 ` Arnd Bergmann
2011-09-06 19:45 ` Greg KH
2011-09-07 6:14 ` Lee Jones
2011-09-02 14:02 ` Arnd Bergmann
2011-09-02 16:24 ` Greg KH
2011-09-02 17:32 ` Arnd Bergmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E60F333.1010902@linaro.org \
--to=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.