On Wed, 16 Mar 2022 20:06:33 +0000,
Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote:
>
> Now that all the DT drivers have switched to platform_get_irq() we can now
> safely drop the static setup of IRQ resource from DT core code.
>
> With the above change hierarchical setup of irq domains is no longer
> bypassed and thus allowing hierarchical interrupt domains to describe
> interrupts using "interrupts" DT property.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> Hi All,
>
> Sending this as RFC as couple of more drivers need to hit -rc yet with
> the platform_get_irq() change while that is in progress I wanted to get
> some feedback on this patch.
>
> Cheers,
> Prabhakar
> ---
> drivers/of/platform.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 793350028906..6890f7fe556f 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -114,35 +114,31 @@ struct platform_device *of_device_alloc(struct device_node *np,
> struct device *parent)
> {
> struct platform_device *dev;
> - int rc, i, num_reg = 0, num_irq;
> + int rc, i, num_reg = 0;
> struct resource *res, temp_res;
>
> dev = platform_device_alloc("", PLATFORM_DEVID_NONE);
> if (!dev)
> return NULL;
>
> - /* count the io and irq resources */
> + /* count the io resources */
> while (of_address_to_resource(np, num_reg, &temp_res) == 0)
> num_reg++;
> - num_irq = of_irq_count(np);
>
> /* Populate the resource table */
> - if (num_irq || num_reg) {
> - res = kcalloc(num_irq + num_reg, sizeof(*res), GFP_KERNEL);
> + if (num_reg) {
> + res = kcalloc(num_reg, sizeof(*res), GFP_KERNEL);
> if (!res) {
> platform_device_put(dev);
> return NULL;
> }
>
> - dev->num_resources = num_reg + num_irq;
> + dev->num_resources = num_reg;
> dev->resource = res;
> for (i = 0; i < num_reg; i++, res++) {
> rc = of_address_to_resource(np, i, res);
> WARN_ON(rc);
> }
> - if (of_irq_to_resource_table(np, res, num_irq) != num_irq)
> - pr_debug("not all legacy IRQ resources mapped for %pOFn\n",
> - np);
> }
>
> dev->dev.of_node = of_node_get(np);
I think this definitely goes in the right direction by not eagerly
populating resources without a driver actually needing it. If anything
breaks, that should be seen as an opportunity to fix the users of this
misfeature. I booted a couple of boxes with this patch, and nothing
caught fire, so:
Acked-by: Marc Zyngier <maz@xxxxxxxxxx>
Tested-by: Marc Zyngier <maz@xxxxxxxxxx>
M.
--
Without deviation from the norm, progress is not possible.
