[PATCH] fsl mc-bus: Move mc-io destroy to bus remove

[PATCH] fsl mc-bus: Move mc-io destroy to bus remove

[Bharat Bhushan]

fsl_mc_device_add() is called when a new mc device is added and
fsl_mc_device_remove() is called when a mc device is removed.
Now mc portals (fsl_create_mc_io) are not created during device
addition i.e fsl_mc_device_add(). But mc portal are being destroyed
when an mc device are removed i.e fsl_mc_device_remove(), which is
not correct way of reverting what is done during device addition.

Now this leads to the problem with VFIO driver where we do not want
the portal to be destroyed when an mc-device is removed. VFIO
creates one mc-portal for all dprc-containers bound to it and it
destroys the mc portal when driver itself is removed.

This change moves the mc-portal destroy part to bus-remove for
mc-bus and dprc-driver remove for child DPRCs. This also makes
fsl_mc_device_remove() exactly reverting what is done in
fsl_mc_device_add() with respect to mc portal creation.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
---
 drivers/staging/fsl-mc/bus/dprc-driver.c | 3 +++
 drivers/staging/fsl-mc/bus/mc-bus.c      | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
index 7fc4717..14f9c82 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -801,6 +801,9 @@ static int dprc_remove(struct fsl_mc_device *mc_dev)
 		dev_set_msi_domain(&mc_dev->dev, NULL);
 	}
 
+	fsl_destroy_mc_io(mc_dev->mc_io);
+	mc_dev->mc_io = NULL;
+
 	dev_info(&mc_dev->dev, "DPRC device unbound from driver");
 	return 0;
 }
diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c
index 4053643..702dfc9 100644
--- a/drivers/staging/fsl-mc/bus/mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/mc-bus.c
@@ -550,10 +550,6 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
 
 	if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) {
 		mc_bus = to_fsl_mc_bus(mc_dev);
-		if (mc_dev->mc_io) {
-			fsl_destroy_mc_io(mc_dev->mc_io);
-			mc_dev->mc_io = NULL;
-		}
 
 		if (fsl_mc_is_root_dprc(&mc_dev->dev)) {
 			if (atomic_read(&root_dprc_count) > 0)
@@ -781,6 +777,10 @@ static int fsl_mc_bus_remove(struct platform_device *pdev)
 		return -EINVAL;
 
 	fsl_mc_device_remove(mc->root_mc_bus_dev);
+
+	fsl_destroy_mc_io(mc->root_mc_bus_dev->mc_io);
+	mc->root_mc_bus_dev->mc_io = NULL;
+
 	dev_info(&pdev->dev, "Root MC bus device removed");
 	return 0;
 }
-- 
1.9.3

RE: [PATCH] fsl mc-bus: Move mc-io destroy to bus remove

[Stuart Yoder]

> -----Original Message-----
> From: Bharat Bhushan [mailto:Bharat.Bhushan@nxp.com]
> Sent: Thursday, June 16, 2016 1:52 AM
> To: gregkh@linuxfoundation.org; Stuart Yoder <stuart.yoder@nxp.com>
> Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Bharat Bhushan <bharat.bhushan@nxp.com>
> Subject: [PATCH] fsl mc-bus: Move mc-io destroy to bus remove
> 
> fsl_mc_device_add() is called when a new mc device is added and
> fsl_mc_device_remove() is called when a mc device is removed.
> Now mc portals (fsl_create_mc_io) are not created during device
> addition i.e fsl_mc_device_add(). But mc portal are being destroyed
> when an mc device are removed i.e fsl_mc_device_remove(), which is
> not correct way of reverting what is done during device addition.
> 
> Now this leads to the problem with VFIO driver where we do not want
> the portal to be destroyed when an mc-device is removed. VFIO
> creates one mc-portal for all dprc-containers bound to it and it
> destroys the mc portal when driver itself is removed.
> 
> This change moves the mc-portal destroy part to bus-remove for
> mc-bus and dprc-driver remove for child DPRCs. This also makes
> fsl_mc_device_remove() exactly reverting what is done in
> fsl_mc_device_add() with respect to mc portal creation.

I think we need to improve and clarify this commit message.
This change is needed, but I don't think the message
makes it clear what is going on.

> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> ---
>  drivers/staging/fsl-mc/bus/dprc-driver.c | 3 +++
>  drivers/staging/fsl-mc/bus/mc-bus.c      | 8 ++++----
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
> index 7fc4717..14f9c82 100644
> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> @@ -801,6 +801,9 @@ static int dprc_remove(struct fsl_mc_device *mc_dev)
>  		dev_set_msi_domain(&mc_dev->dev, NULL);
>  	}
> 
> +	fsl_destroy_mc_io(mc_dev->mc_io);
> +	mc_dev->mc_io = NULL;
> +

We need to do the destroy here, but we need to add a check and
only do this for child containers.

Thanks,
Stuart