lkml.org 
[lkml]   [2013]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 7/9] of/platform: Resolve interrupt references at probe time
    On Tue, Sep 17, 2013 at 01:04:06PM +0000, Strashko, Grygorii wrote:
    > Hi Thierry,
    >
    > On 09/16/2013 11:32 AM, Thierry Reding wrote:> Interrupt references are currently resolved very early (when a device is
    > > created). This has the disadvantage that it will fail in cases where the
    > > interrupt parent hasn't been probed and no IRQ domain for it has been
    > > registered yet. To work around that various drivers use explicit
    > > initcall ordering to force interrupt parents to be probed before devices
    > > that need them are created. That's error prone and doesn't always work.
    > > If a platform device uses an interrupt line connected to a different
    > > platform device (such as a GPIO controller), both will be created in the
    > > same batch, and the GPIO controller won't have been probed by its driver
    > > when the depending platform device is created. Interrupt resolution will
    > > fail in that case.
    > >
    > > Another common workaround is for drivers to explicitly resolve interrupt
    > > references at probe time. This is suboptimal, however, because it will
    > > require every driver to duplicate the code.
    > >
    > > This patch adds support for late interrupt resolution to the platform
    > > driver core, by resolving the references right before a device driver's
    > > .probe() function will be called. This not only delays the resolution
    > > until a much later time (giving interrupt parents a better chance of
    > > being probed in the meantime), but it also allows the platform driver
    > > core to queue the device for deferred probing if the interrupt parent
    > > hasn't registered its IRQ domain yet.
    > >
    > > Signed-off-by: Thierry Reding <treding@nvidia.com>
    > > ---
    > > drivers/base/platform.c | 4 ++++
    > > drivers/of/platform.c | 43 +++++++++++++++++++++++++++++++++++++------
    > > include/linux/of_platform.h | 7 +++++++
    > > 3 files changed, 48 insertions(+), 6 deletions(-)
    > >
    > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
    > > index 4f8bef3..8dcf835 100644
    > > --- a/drivers/base/platform.c
    > > +++ b/drivers/base/platform.c
    > > @@ -481,6 +481,10 @@ static int platform_drv_probe(struct device *_dev)
    >
    > Should it be the part of really_probe()? Isn't it?

    really_probe() takes a struct device and is in fact called by all types
    of devices. This code, however, is highly platform_device specific, so I
    don't think we can do it in really_probe().

    Unfortunately every device type has its own way of storing interrupts.
    Platform devices store them as resources, I2C clients store them as a
    separate field in struct i2c_client, etc.

    > > +int of_platform_probe(struct platform_device *pdev)
    > > +{
    > > + struct device_node *np = pdev->dev.of_node;
    > > + int num_irq, ret = 0;
    > > +
    > > + if (!pdev->dev.of_node)
    > > + return 0;
    > > +
    > > + num_irq = of_irq_count(pdev->dev.of_node);
    > > + if (num_irq > 0) {
    > > + struct resource *res = pdev->resource;
    > > + int num_reg = pdev->num_resources;
    > > + int num = num_reg + num_irq;
    > > +
    > > + res = krealloc(res, num * sizeof(*res), GFP_KERNEL);
    > > + if (!res)
    > > + return -ENOMEM;
    > > +
    > > + pdev->num_resources = num;
    > > + pdev->resource = res;
    > > + res += num_reg;
    >
    > What will happen if Driver probe is failed or deferred?
    > Seems resource table size will grow each time the Driver probe is
    > deferred or failed.

    That's a very good point. I think what we can do is check whether the
    total number of resources that the device has (pdev->num_resources)
    corresponds to num_reg + num_irq and skip in that case. That and...

    > > + ret = of_irq_to_resource_table(np, res, num_irq);
    > > + if (ret < 0)
    > > + return ret;

    ... updating pdev->num_resources after this point should cover all
    cases. Do you see any other potential problems?
    Thierry
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2013-09-18 13:01    [W:4.332 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site