lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 26 Nov 2020 09:28:50 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     John Garry <john.garry@...wei.com>
Cc:     jejb@...ux.ibm.com, martin.petersen@...cle.com, lenb@...nel.org,
        rjw@...ysocki.net, gregkh@...uxfoundation.org, tglx@...utronix.de,
        linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        linuxarm@...wei.com, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v3 3/5] driver core: platform: Add platform_put_irq()

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@...wei.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...

Powered by blists - more mailing lists