[RESEND PATCH 1/2 v2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[RESEND PATCH 1/2 v2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Nipun Gupta]

Following is the current scenario when the devices are probed:

  FSL_MC Bus probe ---> dprc probe ---> dprc devices scan --->
    probe devices in DPRC container ---> allocate IRQ's

In case the devices being probed in the DPRC container need the IRQ's;
probing of that device will fail.

In current scenario the devices which need IRQ's such as DPIO gets
deferred because they aren't registered when first time the probing
of these devices is done in the dprc scan.
So they are probed once IRQ's have been allocated.

In case where DPRC probing itself gets deferred, which does in case
IOMMU is enabled; all the devices in DPRC container gets probed before
IRQ's are allocated. This causes devices using IRQ's (such as DPIO)
to fail.

So having IRQ's allocated before any of the devices in the DPRC container
are probed is more legitimate.

After this patch following is the flow of execution:

  FSL_MC Bus probe ---> dprc probe ---> dprc devices scan --->
    allocate IRQ's ---> probe of devices in DPRC container.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---
Resending v2 as missed mentioning the changes in the version
Changes in v2:
  - Polished the commit message
  - Minor updation in the comment added in the code 'Allocate IRQ's...'

 drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index 06df528..f4bde58 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
  * dprc_scan_objects - Discover objects in a DPRC
  *
  * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
- * @total_irq_count: total number of IRQs needed by objects in the DPRC.
+ * @total_irq_count: If argument is provided the function populates the
+ * total number of IRQs created by objects in the DPRC.
  *
  * Detects objects added and removed from a DPRC and synchronizes the
  * state of the Linux bus driver, MC by adding and removing
@@ -228,6 +229,7 @@ static int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
 	int error;
 	unsigned int irq_count = mc_bus_dev->obj_desc.irq_count;
 	struct fsl_mc_obj_desc *child_obj_desc_array = NULL;
+	struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
 
 	error = dprc_get_obj_count(mc_bus_dev->mc_io,
 				   0,
@@ -297,7 +299,26 @@ static int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
 		}
 	}
 
-	*total_irq_count = irq_count;
+	/*
+	 * Allocate IRQ's before binding the scanned devices with their
+	 * respective drivers.
+	 */
+	if (dev_get_msi_domain(&mc_bus_dev->dev) && !mc_bus->irq_resources) {
+		if (irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS) {
+			dev_warn(&mc_bus_dev->dev,
+				 "IRQs needed (%u) exceed IRQs preallocated (%u)\n",
+				 irq_count, FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
+		}
+
+		error = fsl_mc_populate_irq_pool(mc_bus,
+				FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
+		if (error < 0)
+			return error;
+	}
+
+	if (total_irq_count)
+		*total_irq_count = irq_count;
+
 	dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
 			    num_child_objects);
 
@@ -322,7 +343,6 @@ static int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
 static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
 {
 	int error;
-	unsigned int irq_count;
 	struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
 
 	fsl_mc_init_all_resource_pools(mc_bus_dev);
@@ -331,29 +351,14 @@ static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
 	 * Discover objects in the DPRC:
 	 */
 	mutex_lock(&mc_bus->scan_mutex);
-	error = dprc_scan_objects(mc_bus_dev, &irq_count);
+	error = dprc_scan_objects(mc_bus_dev, NULL);
 	mutex_unlock(&mc_bus->scan_mutex);
-	if (error < 0)
-		goto error;
-
-	if (dev_get_msi_domain(&mc_bus_dev->dev) && !mc_bus->irq_resources) {
-		if (irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS) {
-			dev_warn(&mc_bus_dev->dev,
-				 "IRQs needed (%u) exceed IRQs preallocated (%u)\n",
-				 irq_count, FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
-		}
-
-		error = fsl_mc_populate_irq_pool(
-				mc_bus,
-				FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
-		if (error < 0)
-			goto error;
+	if (error < 0) {
+		fsl_mc_cleanup_all_resource_pools(mc_bus_dev);
+		return error;
 	}
 
 	return 0;
-error:
-	fsl_mc_cleanup_all_resource_pools(mc_bus_dev);
-	return error;
 }
 
 /**
-- 
1.9.1

[RESEND PATCH 2/2 v2] staging: fsl-mc: do not print error in case of defer probe error

[Nipun Gupta]

Devices on MC bus can be deferred because of dependencies on other modules
(such as IOMMU). Those are not the actual errors; as probing is again
done by the kernel at later stages. So this patch avoids the error print
in such case.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---
Resending v2 as missed mentioning the changes in the version
Changes in v2:
  - Added commit message

 drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
index 409f2b9..10cee00 100644
--- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
@@ -171,7 +171,8 @@ static int fsl_mc_driver_probe(struct device *dev)
 
 	error = mc_drv->probe(mc_dev);
 	if (error < 0) {
-		dev_err(dev, "%s failed: %d\n", __func__, error);
+		if (error != -EPROBE_DEFER)
+			dev_err(dev, "%s failed: %d\n", __func__, error);
 		return error;
 	}
 
-- 
1.9.1

RE: [RESEND PATCH 1/2 v2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Bharat Bhushan]

> -----Original Message-----
> From: Nipun Gupta [mailto:nipun.gupta@nxp.com]
> Sent: Monday, December 11, 2017 9:16 PM
> To: Laurentiu Tudor <laurentiu.tudor@nxp.com>; stuyoder@gmail.com;
> Bharat Bhushan <bharat.bhushan@nxp.com>; gregkh@linuxfoundation.org;
> cakturk@gmail.com; bretth256@gmail.com; arnd@arndb.de
> Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org; Nipun Gupta
> <nipun.gupta@nxp.com>
> Subject: [RESEND PATCH 1/2 v2] staging: fsl-mc: Allocate IRQ's before
> scanning DPRC objects
> 
> Following is the current scenario when the devices are probed:
> 
>   FSL_MC Bus probe ---> dprc probe ---> dprc devices scan --->
>     probe devices in DPRC container ---> allocate IRQ's
> 
> In case the devices being probed in the DPRC container need the IRQ's;
> probing of that device will fail.
> 
> In current scenario the devices which need IRQ's such as DPIO gets deferred
> because they aren't registered when first time the probing of these devices
> is done in the dprc scan.
> So they are probed once IRQ's have been allocated.
> 
> In case where DPRC probing itself gets deferred, which does in case IOMMU
> is enabled; all the devices in DPRC container gets probed before IRQ's are
> allocated. This causes devices using IRQ's (such as DPIO) to fail.
> 
> So having IRQ's allocated before any of the devices in the DPRC container are
> probed is more legitimate.
> 
> After this patch following is the flow of execution:
> 
>   FSL_MC Bus probe ---> dprc probe ---> dprc devices scan --->
>     allocate IRQ's ---> probe of devices in DPRC container.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>

Looks ok to me,

Reviewed-by: Bharat Bhushan <bharat.bhushan@nxp.com>

> ---
> Resending v2 as missed mentioning the changes in the version Changes in v2:
>   - Polished the commit message
>   - Minor updation in the comment added in the code 'Allocate IRQ's...'
> 
>  drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++++++++++++++++++---------
> -----
>  1 file changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-
> mc/bus/dprc-driver.c
> index 06df528..f4bde58 100644
> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct
> fsl_mc_device *mc_bus_dev,
>   * dprc_scan_objects - Discover objects in a DPRC
>   *
>   * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
> + * @total_irq_count: If argument is provided the function populates the
> + * total number of IRQs created by objects in the DPRC.
>   *
>   * Detects objects added and removed from a DPRC and synchronizes the
>   * state of the Linux bus driver, MC by adding and removing @@ -228,6
> +229,7 @@ static int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev,
>  	int error;
>  	unsigned int irq_count = mc_bus_dev->obj_desc.irq_count;
>  	struct fsl_mc_obj_desc *child_obj_desc_array = NULL;
> +	struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
> 
>  	error = dprc_get_obj_count(mc_bus_dev->mc_io,
>  				   0,
> @@ -297,7 +299,26 @@ static int dprc_scan_objects(struct fsl_mc_device
> *mc_bus_dev,
>  		}
>  	}
> 
> -	*total_irq_count = irq_count;
> +	/*
> +	 * Allocate IRQ's before binding the scanned devices with their
> +	 * respective drivers.
> +	 */
> +	if (dev_get_msi_domain(&mc_bus_dev->dev) && !mc_bus-
> >irq_resources) {
> +		if (irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS) {
> +			dev_warn(&mc_bus_dev->dev,
> +				 "IRQs needed (%u) exceed IRQs preallocated
> (%u)\n",
> +				 irq_count,
> FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
> +		}
> +
> +		error = fsl_mc_populate_irq_pool(mc_bus,
> +				FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
> +		if (error < 0)
> +			return error;
> +	}
> +
> +	if (total_irq_count)
> +		*total_irq_count = irq_count;
> +
>  	dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
>  			    num_child_objects);
> 
> @@ -322,7 +343,6 @@ static int dprc_scan_objects(struct fsl_mc_device
> *mc_bus_dev,  static int dprc_scan_container(struct fsl_mc_device
> *mc_bus_dev)  {
>  	int error;
> -	unsigned int irq_count;
>  	struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
> 
>  	fsl_mc_init_all_resource_pools(mc_bus_dev);
> @@ -331,29 +351,14 @@ static int dprc_scan_container(struct fsl_mc_device
> *mc_bus_dev)
>  	 * Discover objects in the DPRC:
>  	 */
>  	mutex_lock(&mc_bus->scan_mutex);
> -	error = dprc_scan_objects(mc_bus_dev, &irq_count);
> +	error = dprc_scan_objects(mc_bus_dev, NULL);
>  	mutex_unlock(&mc_bus->scan_mutex);
> -	if (error < 0)
> -		goto error;
> -
> -	if (dev_get_msi_domain(&mc_bus_dev->dev) && !mc_bus-
> >irq_resources) {
> -		if (irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS) {
> -			dev_warn(&mc_bus_dev->dev,
> -				 "IRQs needed (%u) exceed IRQs preallocated
> (%u)\n",
> -				 irq_count,
> FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
> -		}
> -
> -		error = fsl_mc_populate_irq_pool(
> -				mc_bus,
> -				FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
> -		if (error < 0)
> -			goto error;
> +	if (error < 0) {
> +		fsl_mc_cleanup_all_resource_pools(mc_bus_dev);
> +		return error;
>  	}
> 
>  	return 0;
> -error:
> -	fsl_mc_cleanup_all_resource_pools(mc_bus_dev);
> -	return error;
>  }
> 
>  /**
> --
> 1.9.1