Hi Uwe,
Le mardi 14 janvier 2014 à 09:19 +0100, Uwe Kleine-König a écrit :
> On Tue, Jan 14, 2014 at 08:18:29AM +0100, Yann Droneaud wrote:
> > Since commit 01dcc60a7cb8, platform_device_register_full() is
> > available to allocate and register a platform device.
> >
> > If a dma_mask is provided as part of platform_device_info,
> > platform_device_register_full() allocate memory for a u64
> > using kmalloc().
> >
> > A comment in the code state that "[t]his memory isn't freed
> > when the device is put".
> >
[...]
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 3a94b799f166..6e3e639fb886 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -157,7 +157,7 @@ EXPORT_SYMBOL_GPL(platform_add_devices);
> >
> > struct platform_object {
> > struct platform_device pdev;
> > - char name[1];
> > + char payload[0];
> I don't know the recent minimal versions needed to compile the kernel
> and since when gcc supports c99 flexible array members, but I would
> expect that they just work. Having said that I'd prefer using that one,
> i.e. use
> char payload[];
> > };
I'm not confident with flexible array when using sizeof(), offsetof(),
etc. I will try to use the c99 feature.
> > +static struct platform_device *platform_device_dmamask_alloc(const char
> > *name,
> > + int id)
> > +{
> > + struct platform_object *pa;
> > + const size_t padding = (((offsetof(struct platform_object, payload) +
> > + (__alignof__(u64) - 1)) &
> > + ~(__alignof__(u64) - 1)) -
> > + offsetof(struct platform_object, payload));
> > +
> > + pa = platform_object_alloc(padding + sizeof(u64) + strlen(name) + 1);
> > + if (pa) {
> > + char *payload = pa->payload + padding;
> > + /*
> > + * Conceptually dma_mask in struct device should not be a
> > pointer.
> > + * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
> > + */
> > + pa->pdev.dev.dma_mask = (void *)payload;
> > + payload += sizeof(u64);
> > + strcpy(payload, name);
> > + platform_object_init(pa, payload, id);
> > + }
> > +
> > + return pa ? &pa->pdev : NULL;
> > +}
> This looks all complicated. Did you think about spending the extra
> memory and add a dma_mask to platform_object? That should simplify the
> code quite a bit which probably is worth the extra memory being used.
>
You could have did it in the first place. But you choose to allocate a
chunk of memory for the u64. I believe there's a reason ;)
I will try to get some figure on the number of platform_device
registered with a dmamask versus without a dmamask: adding the u64 to
all platform_object might cost more memory than the extra code (1 branch
and a function).
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
