On 2017-01-19 07:36, Lorenzo Pieralisi wrote:
On Wed, Jan 18, 2017 at 09:45:56PM +0200, Andy Shevchenko wrote:
[...]
> +/**
> + * acpi_irq_get - Look for the ACPI IRQ resource with the given index and
> + * use it to initialize the given Linux IRQ resource.
> + * @handle ACPI device handle
> + * @index ACPI IRQ resource index to lookup
> + * @res Linux IRQ resource to initialize
Ah, you missed colons after field names:
* @field1:
> + *
> + * Return:
Next line: 0 on success
> + * -EINVAL if an error occurs
> + * -EPROBE_DEFER if the IRQ lookup/conversion failed
> + */
> +int acpi_irq_get(acpi_handle handle, unsigned int index, struct resource *res)
> +{
> + int rc;
Put this last in the definition block.
> + struct irq_fwspec fwspec;
> + struct irq_domain *domain;
> + unsigned long flags;
> +
> + rc = acpi_irq_parse_one(handle, index, &fwspec, &flags);
> + if (rc)
> + return rc;
> +
> + domain = irq_find_matching_fwnode(fwspec.fwnode, DOMAIN_BUS_ANY);
> + if (!domain)
> + return -EPROBE_DEFER;
Hmm... Could it be other issues here?
That's a good point. Here probing should be deferred only if we know a
driver capable of handling fwspec.fwnode will have a chance to be
probed/initialized eventually right (which basically means it is
compiled in the kernel and we hope it will probed successfully) ? I am
not sure there is an easy way to detect that in ACPI at the moment, I
suspect it is time we added a linker section (or augment the existing
one used for irqchip early MADT parsing - IRQCHIP_ACPI_DECLARE) for
this
purpose I do not see any other option (apart from leaving devices in
the
deferred probe list if a driver for the irqchip represented by
fwspec.fwnode is not present in the kernel, which is a bit sloppy, or
resorting to checking Kconfig entries to detect compile time enabled
irqchip drivers).
I'll add the linker section suggested by Lorenzo for the extra check.
We can do that check in acpi_get_irq_source_fwhandle and make the lookup
fail if the device is not listed in the table.
That way we can keep this check as-is.
Thoughts?
> +
> + rc = irq_create_fwspec_mapping(&fwspec);
> + if (rc <= 0)
> + return -EINVAL;
> +
> + res->start = rc;
> + res->end = rc;
> + res->flags = flags;
Perhaps struct resource *r should be a parameter to
acpi_irq_parse_one().
Yeah but then you would end up with flags initialized in
acpi_irq_parse_one() and the other resource params (ie start, end)
here,
I do not think it is nicer.
The reason this took this form is that we are trying to get some
symmetry
with what of_irq_get does.
Thanks,
Agustin
Thanks,
Lorenzo
[...]
With Best Regards,
Andy Shevchenko
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
