Re: [RFC] [PATCH 3/3] IRQ: irq domain: defer of irq ressoure resolve at platform_drv_probe

[Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 05/28/2013 10:08 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Today in the current of implementation we populate all the ressources

typo

> at of_platform_populate time. But this leed to a chicken-egg dilemat

2 typos

> some the irq present in DT are from platform_device too. And you can
> not resolve them as of_platform_populate. So delay the populate of irq
> at platform_drv_probe.
> 
> And if the irq_domain is not yet present just defer the probe (GPIO as example)
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> Cc: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> Cc: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/base/platform.c     |    5 +++++
>  drivers/of/platform.c       |   29 +++++++++++++++++++++++++++--
>  include/linux/of_platform.h |    7 +++++++
>  3 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 9eda842..c0f7906 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -13,6 +13,7 @@
>  #include <linux/string.h>
>  #include <linux/platform_device.h>
>  #include <linux/of_device.h>
> +#include <linux/of_platform.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/dma-mapping.h>
> @@ -484,6 +485,10 @@ static int platform_drv_probe(struct device *_dev)
>  	struct platform_device *dev = to_platform_device(_dev);
>  	int ret;
>  
> +	ret = of_device_init_irq(dev);
> +	if (ret)
> +		return ret;
> +
>  	if (ACPI_HANDLE(_dev))
>  		acpi_dev_pm_attach(_dev, true);
>  
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 00a0971..c290943 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -131,6 +131,32 @@ void of_device_make_bus_id(struct device *dev)
>  }
>  
>  /**
> + * of_device_alloc_irq - initialize irq of an platfrom_device

typo

> + * @dev: plaform_device to work on

same word, different typo.

> + */
> +int of_device_init_irq(struct platform_device *dev)
> +{
> +	struct device_node *np = dev->dev.of_node;
> +	int num_irq;
> +	int ret;
> +	struct resource *res = dev->resource;
> +
> +	if (!np)
> +		return 0;
> +
> +	num_irq = of_irq_count(np);
> +	if (!num_irq)
> +		return 0;
> +
> +	res += dev->num_resources - num_irq;

This looks fragile to me as it relies on how of_device_alloc happens to
be implemented.

> +	ret = of_irq_to_resource_table(np, res, num_irq);
> +	if (ret != num_irq)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +
> +/**
>   * of_device_alloc - Allocate and initialize an of_device
>   * @np: device node to assign to device
>   * @bus_id: Name to assign to the device.  May be null to use default name.
> @@ -152,7 +178,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
>  	if (of_can_translate_address(np))
>  		while (of_address_to_resource(np, num_reg, &temp_res) == 0)
>  			num_reg++;
> -	num_irq = of_irq_valid_count(np);
> +	num_irq = of_irq_count(np);
>  
>  	/* Populate the resource table */
>  	if (num_irq || num_reg) {
> @@ -168,7 +194,6 @@ struct platform_device *of_device_alloc(struct device_node *np,
>  			rc = of_address_to_resource(np, i, res);
>  			WARN_ON(rc);
>  		}
> -		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
>  	}
>  
>  	dev->dev.of_node = of_node_get(np);
> diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h
> index 3863a4d..b5f9bb6 100644
> --- a/include/linux/of_platform.h
> +++ b/include/linux/of_platform.h
> @@ -79,6 +79,7 @@ extern const struct of_device_id of_default_bus_match_table[];
>  extern struct platform_device *of_device_alloc(struct device_node *np,
>  					 const char *bus_id,
>  					 struct device *parent);
> +extern int of_device_init_irq(struct platform_device *dev);
>  extern struct platform_device *of_find_device_by_node(struct device_node *np);
>  
>  #ifdef CONFIG_OF_ADDRESS /* device reg helpers depend on OF_ADDRESS */
> @@ -101,6 +102,7 @@ extern int of_platform_populate(struct device_node *root,
>  #if !defined(CONFIG_OF_ADDRESS)
>  struct of_dev_auxdata;
>  struct device;
> +struct platform_device;
>  static inline int of_platform_populate(struct device_node *root,
>  					const struct of_device_id *matches,
>  					const struct of_dev_auxdata *lookup,
> @@ -108,6 +110,11 @@ static inline int of_platform_populate(struct device_node *root,
>  {
>  	return -ENODEV;
>  }
> +
> +static inline int of_device_init_irq(struct platform_device *dev)
> +{
> +	return 0;
> +}
>  #endif /* !CONFIG_OF_ADDRESS */
>  
>  #endif	/* _LINUX_OF_PLATFORM_H */
>