linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).