lkml.org 
[lkml]   [2009]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] Driver Core: Add platform device arch data V2
    From
    Date
    Magnus Damm <magnus.damm@gmail.com> writes:

    > 2009/6/2 Rafael J. Wysocki <rjw@sisk.pl>:
    >> On Monday 01 June 2009, Rafael J. Wysocki wrote:
    >>> On Monday 01 June 2009, Magnus Damm wrote:
    >>> > From: Magnus Damm <damm@igel.co.jp>
    >>> >
    >>> > Allow architecture specific data in struct platform_device V2.
    >>> > The structure pdev_archdata is added to struct platform_device,
    >>> > similar to struct dev_archdata in struct device.
    >>> >
    >>> > Useful for architecture code that needs to keep extra data
    >>> > associated with each platform device. This data shall not
    >>> > be accessed by platform drivers, only architecture code.
    >>> >
    >>> > Needed for platform device runtime PM.
    >>>
    >>> What exactly do you need this data for?
    >
    > I'd like to keep a hardware block id associated with each platform
    > device on our SoC.
    > Please have a look at "PATCH [04/04] sh: Runtime platform device PM mockup",
    > http://patchwork.kernel.org/patch/26421/

    And in OMAP, we will keep a pointer to an SoC-specific struct of
    HW specific data to be used in idle/wakeup decision making.

    >> Anyway, I think you can introduce something like:
    >>
    >> struct <your arch>_platform_device {
    >>    struct platform_device dev;
    >>    <some type> <your arch data>;
    >>    ...
    >> };
    >>
    >> define your platform devices using the struct above and pass its dev member to
    >> the functions that need 'struct platform_device' as an argument.
    >>
    >> Then you won't need to add arch members to 'struct platform_device' itself.
    >
    > Thanks for your suggestion. I'm usually a friend of wrapping
    > structures and using offsetof(), but in this case I don't think it
    > will work very well.

    Neither do I in this case...

    > I'd like to keep a SoC specific hardware block id in this architecture
    > specific struct. Then let the arch specific functions
    > platform_device_idle() and platform_device_wakeup() use this hardware
    > block id to locate which clocks to stop and which power domains to
    > fiddle with within the SoC. If we only consider this on-SoC case then
    > wrapping and offsetof() works well.
    >
    > 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). And since idle() and wakeup() work with struct
    > platform device, with a wrapped data structure we need some way to
    > check if the platform data is actually wrapped and offsetof() is
    > valid. I guess we could use some platform device specific flag for
    > this, but that seems overly complicated in my opinion. And modifying
    > idle() and wakup() to take arch specific structures is not so good
    > since we want to use the same platform driver on multiple
    > architectures.

    Also, there many cases where platform_devices are not declared
    statically and using the wrapper method doesn't work well if you are
    using platform_device_alloc(). In addition to not being able to use
    container_of() etc. the memory allocated potentially lasts longer than
    the existence of the platform_device.

    I have a patch lying around that extended platform_device_alloc() to
    take an extra size arg for platform-specific extentions (like
    netdev_alloc() and some others like it) but I never got ambitious
    enough to change all the users of platform_device_alloc().

    Kevin


    > My mockup code that keeps keeps the hardware block id in the platform
    > device arch specific data works well since the hardware block id with
    > value zero is a special case. The value zero means "external non-soc
    > device", so a "regular" board specific struct platform_device that do
    > not setup arch specific data can just be skipped in idle()/wakeup().
    >
    > 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.
    >
    > Maybe there are better ways to solve this? I think arch specific data
    > in struct platform_device is pretty straight forward though.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2009-06-02 17:41    [W:0.029 / U:0.372 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site