lkml.org 
[lkml]   [2011]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
    On Fri, 1 Apr 2011 13:20:31 +0200
    Samuel Ortiz <sameo@linux.intel.com> wrote:

    > Hi Grant,
    >
    > On Thu, Mar 31, 2011 at 05:05:22PM -0600, Grant Likely wrote:
    [...]
    > > Gah. Not all devices instantiated via mfd will be an mfd device,
    > > which means that the driver may very well expect an *entirely
    > > different* platform_device pointer; which further means a very high
    > > potential of incorrectly dereferenced structures (as evidenced by a
    > > patch series that is not bisectable). For instance, the xilinx ip
    > > cores are used by more than just mfd.
    > I agree. Since the vast majority of the MFD subdevices are MFD
    > specific IPs, I overlooked that part. The impacted drivers are the
    > timberdale and the DaVinci voice codec ones.

    Can you please provide pointers to what you're referring to? The only
    code that I could find that created platform devices prefixed with
    'timb-' or named 'xilinx_spi' was drivers/mfd/timberdale.c.



    > To fix that problem I propose 2 alternatives:
    >
    > 1) When declaring the sub devices cells, the MFD driver should
    > specify an mfd_data_size value for sub devices that are not MFD
    > specific. It's the MFD driver responsibility to set the cell
    > properly, and the non MFD specific drivers are kept MFD agnostic.
    > See my patch below for the timberdale case.
    >
    > 2) Revert the mfd_get_data() call for getting sub devices platform
    > data pointers. That was introduced to ease the MFD cell sharing work,
    > so if we take this route we'll need the cs5535 MFD driver to pass its
    > cells as platform_data pointer. Andres, can you confirm that this
    > would be fine for the mfd_clone_cell() routine to keep working ?

    It would break mfd_clone_cell, as it uses mfd_get_cell to grab the one
    to clone. We could change it to accept the cell as an argument. It
    would also break mfd_cell_enable/disable, of course.



    >
    > Patch for solution 1:
    >
    >
    > drivers/mfd/mfd-core.c | 13 ++++++++++---
    > drivers/mfd/timberdale.c | 11 +++++++++++
    > include/linux/mfd/core.h | 1 +
    > drivers/i2c/busses/i2c-ocores.c | 3 +--
    > drivers/i2c/busses/i2c-xiic.c | 3 +--
    > drivers/net/ks8842.c | 3 +--
    > drivers/spi/xilinx_spi.c | 3 +--
    > 7 files changed, 26 insertions(+), 11 deletions(-)
    >
    > diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
    > index d01574d..8abe510 100644
    > --- a/drivers/mfd/mfd-core.c
    > +++ b/drivers/mfd/mfd-core.c
    > @@ -75,9 +75,16 @@ static int mfd_add_device(struct device *parent,
    > int id,
    > pdev->dev.parent = parent;
    >
    > - ret = platform_device_add_data(pdev, cell, sizeof(*cell));
    > - if (ret)
    > - goto fail_res;
    > + if (cell->mfd_data_size > 0) {
    > + ret = platform_device_add_data(pdev,
    > + cell->mfd_data,
    > cell->mfd_data_size);
    > + if (ret)
    > + goto fail_res;
    > + } else {
    > + ret = platform_device_add_data(pdev, cell,
    > sizeof(*cell));
    > + if (ret)
    > + goto fail_res;
    > + }
    >
    > for (r = 0; r < cell->num_resources; r++) {
    > res[r].name = cell->resources[r].name;


    \
     
     \ /
      Last update: 2011-04-01 19:51    [W:0.026 / U:0.612 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site