Re: [PATCH] Driver Core: Add platform device arch data V2

[Magnus Damm]

On Wed, Jun 3, 2009 at 5:45 PM, Rafael J. Wysocki<rjw@xxxxxxx> wrote:
> On Tuesday 02 June 2009, Magnus Damm wrote:
>> However, a typical embedded system has a wide range of platform
>> devices. Some are for the SoC itself and some are for external
>> devices, like on board ethernet controlllers (not on chip like the SoC
>> platform devices).
>
> Why don't we use the arch-specific wrapping for all platform devices on this
> arch, the SoC ones and the non-SoC ones?  Is there any particular reason not
> to do that?

There are quite a few in-tree platform device drivers for SuperH that
can run on both ARM and SuperH. Maybe even MIPS in the future, who
knows. And we already have a perfectly fine platform device interface
that we can share, so I don't see any reason why we should invent
something new all of a sudden.

> If idle() and wakeup() are supposed to be platform-specific, and quite frankly
> I don't see how they could be implemented in a generic way, they don't even
> need to operate on struct platform_device objects.

The implementation can be platform specific, but the interface should
be shared across architectures. This so we can use the same device
driver on multiple architectures.

If you dislike the idea of operating on struct platform_device then
how about letting idle()/wakeup() or enable()/disable() take a pointer
to struct device instead of struct platform_device?

In my opinion it makes sense to allow architectures to add data to
struct platform_device since it's platform devices we're going to do
runtime PM on. For architectures not using struct pdev_archdata there
is really no additional runtime cost.

>> If you guys dislike adding arch specific data to struct platform
>> device then for SuperH we can just (mis)use the arch specific data in
>> struct device instead. I'm afraid that solution wastes memory since
>> the data will only be used for platform devices anyway. So I prefer
>> adding arch specific data to struct platform_device instead of struct
>> device if possible.
>
> That's generally OK, but I'd like to get convinced that there's no better
> way indeed.

The earlier version did something ugly with Kconfig. That's even worse IMO.

> What I personally don't like about the patch is the duplication of empty
> struct pdev_archdata for quite a number of architectures.  It's not nice and
> I wonder if there's a way to avoid it.

I don't think there's any other way around that than Kconfig. The per
arch duplication of empty structures is already done for struct device
so it can't be that bad.

Thanks for your help!

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/