| Message ID | 1606324841-217570-4-git-send-email-john.garry@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support managed interrupts for platform devices | expand |
On 2020-11-25 17:20, John Garry wrote: > Add a function to tear down the work which was done in > platform_get_irq() > for when the device driver is done with the irq. > > For ACPI companion devices the irq resource is set as disabled, as this > resource is configured from platform_get_irq()->acpi_irq_get() and > requires > resetting. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > drivers/base/platform.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 88aef93eb4dd..3eeda3746701 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -289,6 +289,20 @@ int platform_irq_count(struct platform_device > *dev) > } > EXPORT_SYMBOL_GPL(platform_irq_count); > > +void platform_put_irq(struct platform_device *dev, unsigned int num) > +{ > + unsigned int virq = platform_get_irq(dev, num); I find it pretty odd to have to recompute the interrupt number, which in turn results in a domain lookup. It things were refcounted (they aren't yet), irq_dispose_mapping() would have no effect. <pedant> It also goes against the usual construct where if you obtain an object based on some parameters, the release happens by specifying the object itself, and not the parameters that lead to the object. </pedant> > + > + irq_dispose_mapping(virq); > + if (has_acpi_companion(&dev->dev)) { > + struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, > + num); > + > + if (r) > + acpi_dev_irqresource_disabled(r, 0); It looks to me that the ACPI thing is what needs to be promoted to a first class function, releasing all the resources that have used by a given device. Thanks, M. -- Jazz is not dead. It just smells funny...
On 26/11/2020 09:28, Marc Zyngier wrote: > On 2020-11-25 17:20, John Garry wrote: >> Add a function to tear down the work which was done in platform_get_irq() >> for when the device driver is done with the irq. >> >> For ACPI companion devices the irq resource is set as disabled, as this >> resource is configured from platform_get_irq()->acpi_irq_get() and >> requires >> resetting. >> >> Signed-off-by: John Garry <john.garry@huawei.com> >> --- >> drivers/base/platform.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/base/platform.c b/drivers/base/platform.c >> index 88aef93eb4dd..3eeda3746701 100644 >> --- a/drivers/base/platform.c >> +++ b/drivers/base/platform.c >> @@ -289,6 +289,20 @@ int platform_irq_count(struct platform_device *dev) >> } >> EXPORT_SYMBOL_GPL(platform_irq_count); >> Hi Marc, >> +void platform_put_irq(struct platform_device *dev, unsigned int num) >> +{ >> + unsigned int virq = platform_get_irq(dev, num); > > I find it pretty odd to have to recompute the interrupt number, > which in turn results in a domain lookup. Well we do have the virq available, but then we need to pass the virq and device irq index. But maybe I somehow reverse-lookup the ACPI res somehow from virq, such that we don't require the irq device index. > It things were refcounted > (they aren't yet), irq_dispose_mapping() would have no effect. > > <pedant> > It also goes against the usual construct where if you obtain an object > based on some parameters, the release happens by specifying the object > itself, and not the parameters that lead to the object. > </pedant> Yes, ideally we can use virq. > >> + >> + irq_dispose_mapping(virq); >> + if (has_acpi_companion(&dev->dev)) { >> + struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, >> + num); >> + >> + if (r) >> + acpi_dev_irqresource_disabled(r, 0); > > It looks to me that the ACPI thing is what needs to be promoted to a > first class function, releasing all the resources that have used by > a given device. This is just clearing the irq resource flags, but it could be reasonable (to promote). Thanks, John
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 88aef93eb4dd..3eeda3746701 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -289,6 +289,20 @@ int platform_irq_count(struct platform_device *dev) } EXPORT_SYMBOL_GPL(platform_irq_count); +void platform_put_irq(struct platform_device *dev, unsigned int num) +{ + unsigned int virq = platform_get_irq(dev, num); + + irq_dispose_mapping(virq); + if (has_acpi_companion(&dev->dev)) { + struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, + num); + + if (r) + acpi_dev_irqresource_disabled(r, 0); + } +} + /** * platform_get_resource_byname - get a resource for a device by name * @dev: platform device
Add a function to tear down the work which was done in platform_get_irq() for when the device driver is done with the irq. For ACPI companion devices the irq resource is set as disabled, as this resource is configured from platform_get_irq()->acpi_irq_get() and requires resetting. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/base/platform.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)