diff mbox series

[v5] bus: mhi: core: Fix device hierarchy

Message ID 1606310689-7189-1-git-send-email-loic.poulain@linaro.org
State Superseded
Headers show
Series [v5] bus: mhi: core: Fix device hierarchy | expand

Commit Message

Loic Poulain Nov. 25, 2020, 1:24 p.m. UTC
This patch fixes the hierarchical structure of MHI devices. Indeed,
MHI client devices are directly 'enumerated' from the mhi controller
and therefore must be direct descendants/children of their mhi
controller device, in accordance with the Linux Device Model.

Today both MHI clients and controller devices are at the same level,
this patch ensures that MHI controller is parent of its client devices.

The hierarchy is especially important for power management (safe
suspend/resume order). It is also useful for userspace to determine
relationship between MHI client devices and controllers.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

---
 v2: fix commit message
 v3: reword commit message
 v4: fix device destroy
 v5: fix debugfs device show

 drivers/bus/mhi/core/debugfs.c |  4 +++-
 drivers/bus/mhi/core/init.c    | 10 +++++++++-
 drivers/bus/mhi/core/pm.c      |  4 ++--
 3 files changed, 14 insertions(+), 4 deletions(-)

-- 
2.7.4

Comments

Manivannan Sadhasivam Nov. 25, 2020, 3:06 p.m. UTC | #1
On Wed, Nov 25, 2020 at 02:24:49PM +0100, Loic Poulain wrote:
> This patch fixes the hierarchical structure of MHI devices. Indeed,

> MHI client devices are directly 'enumerated' from the mhi controller

> and therefore must be direct descendants/children of their mhi

> controller device, in accordance with the Linux Device Model.

> 

> Today both MHI clients and controller devices are at the same level,

> this patch ensures that MHI controller is parent of its client devices.

> 

> The hierarchy is especially important for power management (safe

> suspend/resume order). It is also useful for userspace to determine

> relationship between MHI client devices and controllers.

> 

> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>


Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>


Hemant, Bhaumik: Can you please look into this change?

Thanks,
Mani

> ---

>  v2: fix commit message

>  v3: reword commit message

>  v4: fix device destroy

>  v5: fix debugfs device show

> 

>  drivers/bus/mhi/core/debugfs.c |  4 +++-

>  drivers/bus/mhi/core/init.c    | 10 +++++++++-

>  drivers/bus/mhi/core/pm.c      |  4 ++--

>  3 files changed, 14 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c

> index 3a48801..7d43138 100644

> --- a/drivers/bus/mhi/core/debugfs.c

> +++ b/drivers/bus/mhi/core/debugfs.c

> @@ -159,7 +159,9 @@ static int mhi_debugfs_devices_show(struct seq_file *m, void *d)

>  		return -ENODEV;

>  	}

>  

> -	device_for_each_child(mhi_cntrl->cntrl_dev, m, mhi_device_info_show);

> +	/* Show controller and client(s) info */

> +	mhi_device_info_show(&mhi_cntrl->mhi_dev->dev, m);

> +	device_for_each_child(&mhi_cntrl->mhi_dev->dev, m, mhi_device_info_show);

>  

>  	return 0;

>  }

> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c

> index 436221c..c7a7354 100644

> --- a/drivers/bus/mhi/core/init.c

> +++ b/drivers/bus/mhi/core/init.c

> @@ -1137,7 +1137,15 @@ struct mhi_device *mhi_alloc_device(struct mhi_controller *mhi_cntrl)

>  	device_initialize(dev);

>  	dev->bus = &mhi_bus_type;

>  	dev->release = mhi_release_device;

> -	dev->parent = mhi_cntrl->cntrl_dev;

> +

> +	if (mhi_cntrl->mhi_dev) {

> +		/* for MHI client devices, parent is the MHI controller device */

> +		dev->parent = &mhi_cntrl->mhi_dev->dev;

> +	} else {

> +		/* for MHI controller device, parent is the bus device (e.g. pci device) */

> +		dev->parent = mhi_cntrl->cntrl_dev;

> +	}

> +

>  	mhi_dev->mhi_cntrl = mhi_cntrl;

>  	mhi_dev->dev_wake = 0;

>  

> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c

> index a671f58..681960c 100644

> --- a/drivers/bus/mhi/core/pm.c

> +++ b/drivers/bus/mhi/core/pm.c

> @@ -504,7 +504,7 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl)

>  	wake_up_all(&mhi_cntrl->state_event);

>  

>  	dev_dbg(dev, "Reset all active channels and remove MHI devices\n");

> -	device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device);

> +	device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device);

>  

>  	mutex_lock(&mhi_cntrl->pm_mutex);

>  

> @@ -637,7 +637,7 @@ static void mhi_pm_sys_error_transition(struct mhi_controller *mhi_cntrl)

>  	wake_up_all(&mhi_cntrl->state_event);

>  

>  	dev_dbg(dev, "Reset all active channels and remove MHI devices\n");

> -	device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device);

> +	device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device);

>  

>  	mutex_lock(&mhi_cntrl->pm_mutex);

>  

> -- 

> 2.7.4

>
Hemant Kumar Nov. 25, 2020, 8:32 p.m. UTC | #2
On 11/25/20 5:24 AM, Loic Poulain wrote:
> This patch fixes the hierarchical structure of MHI devices. Indeed,

> MHI client devices are directly 'enumerated' from the mhi controller

> and therefore must be direct descendants/children of their mhi

> controller device, in accordance with the Linux Device Model.

> 

> Today both MHI clients and controller devices are at the same level,

> this patch ensures that MHI controller is parent of its client devices.

> 

> The hierarchy is especially important for power management (safe

> suspend/resume order). It is also useful for userspace to determine

> relationship between MHI client devices and controllers.

> 

> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>


Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>


The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Manivannan Sadhasivam Nov. 28, 2020, 7:14 a.m. UTC | #3
On Wed, Nov 25, 2020 at 02:24:49PM +0100, Loic Poulain wrote:
> This patch fixes the hierarchical structure of MHI devices. Indeed,

> MHI client devices are directly 'enumerated' from the mhi controller

> and therefore must be direct descendants/children of their mhi

> controller device, in accordance with the Linux Device Model.

> 

> Today both MHI clients and controller devices are at the same level,

> this patch ensures that MHI controller is parent of its client devices.

> 

> The hierarchy is especially important for power management (safe

> suspend/resume order). It is also useful for userspace to determine

> relationship between MHI client devices and controllers.

> 

> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>


Applied to mhi-next!

Thanks,
Mani

> ---

>  v2: fix commit message

>  v3: reword commit message

>  v4: fix device destroy

>  v5: fix debugfs device show

> 

>  drivers/bus/mhi/core/debugfs.c |  4 +++-

>  drivers/bus/mhi/core/init.c    | 10 +++++++++-

>  drivers/bus/mhi/core/pm.c      |  4 ++--

>  3 files changed, 14 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c

> index 3a48801..7d43138 100644

> --- a/drivers/bus/mhi/core/debugfs.c

> +++ b/drivers/bus/mhi/core/debugfs.c

> @@ -159,7 +159,9 @@ static int mhi_debugfs_devices_show(struct seq_file *m, void *d)

>  		return -ENODEV;

>  	}

>  

> -	device_for_each_child(mhi_cntrl->cntrl_dev, m, mhi_device_info_show);

> +	/* Show controller and client(s) info */

> +	mhi_device_info_show(&mhi_cntrl->mhi_dev->dev, m);

> +	device_for_each_child(&mhi_cntrl->mhi_dev->dev, m, mhi_device_info_show);

>  

>  	return 0;

>  }

> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c

> index 436221c..c7a7354 100644

> --- a/drivers/bus/mhi/core/init.c

> +++ b/drivers/bus/mhi/core/init.c

> @@ -1137,7 +1137,15 @@ struct mhi_device *mhi_alloc_device(struct mhi_controller *mhi_cntrl)

>  	device_initialize(dev);

>  	dev->bus = &mhi_bus_type;

>  	dev->release = mhi_release_device;

> -	dev->parent = mhi_cntrl->cntrl_dev;

> +

> +	if (mhi_cntrl->mhi_dev) {

> +		/* for MHI client devices, parent is the MHI controller device */

> +		dev->parent = &mhi_cntrl->mhi_dev->dev;

> +	} else {

> +		/* for MHI controller device, parent is the bus device (e.g. pci device) */

> +		dev->parent = mhi_cntrl->cntrl_dev;

> +	}

> +

>  	mhi_dev->mhi_cntrl = mhi_cntrl;

>  	mhi_dev->dev_wake = 0;

>  

> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c

> index a671f58..681960c 100644

> --- a/drivers/bus/mhi/core/pm.c

> +++ b/drivers/bus/mhi/core/pm.c

> @@ -504,7 +504,7 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl)

>  	wake_up_all(&mhi_cntrl->state_event);

>  

>  	dev_dbg(dev, "Reset all active channels and remove MHI devices\n");

> -	device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device);

> +	device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device);

>  

>  	mutex_lock(&mhi_cntrl->pm_mutex);

>  

> @@ -637,7 +637,7 @@ static void mhi_pm_sys_error_transition(struct mhi_controller *mhi_cntrl)

>  	wake_up_all(&mhi_cntrl->state_event);

>  

>  	dev_dbg(dev, "Reset all active channels and remove MHI devices\n");

> -	device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device);

> +	device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device);

>  

>  	mutex_lock(&mhi_cntrl->pm_mutex);

>  

> -- 

> 2.7.4

>
patchwork-bot+linux-arm-msm@kernel.org Dec. 29, 2020, 8:15 p.m. UTC | #4
Hello:

This patch was applied to qcom/linux.git (refs/heads/for-next):

On Wed, 25 Nov 2020 14:24:49 +0100 you wrote:
> This patch fixes the hierarchical structure of MHI devices. Indeed,

> MHI client devices are directly 'enumerated' from the mhi controller

> and therefore must be direct descendants/children of their mhi

> controller device, in accordance with the Linux Device Model.

> 

> Today both MHI clients and controller devices are at the same level,

> this patch ensures that MHI controller is parent of its client devices.

> 

> [...]


Here is the summary with links:
  - [v5] bus: mhi: core: Fix device hierarchy
    https://git.kernel.org/qcom/c/10ea8bcda5ae

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c
index 3a48801..7d43138 100644
--- a/drivers/bus/mhi/core/debugfs.c
+++ b/drivers/bus/mhi/core/debugfs.c
@@ -159,7 +159,9 @@  static int mhi_debugfs_devices_show(struct seq_file *m, void *d)
 		return -ENODEV;
 	}
 
-	device_for_each_child(mhi_cntrl->cntrl_dev, m, mhi_device_info_show);
+	/* Show controller and client(s) info */
+	mhi_device_info_show(&mhi_cntrl->mhi_dev->dev, m);
+	device_for_each_child(&mhi_cntrl->mhi_dev->dev, m, mhi_device_info_show);
 
 	return 0;
 }
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 436221c..c7a7354 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -1137,7 +1137,15 @@  struct mhi_device *mhi_alloc_device(struct mhi_controller *mhi_cntrl)
 	device_initialize(dev);
 	dev->bus = &mhi_bus_type;
 	dev->release = mhi_release_device;
-	dev->parent = mhi_cntrl->cntrl_dev;
+
+	if (mhi_cntrl->mhi_dev) {
+		/* for MHI client devices, parent is the MHI controller device */
+		dev->parent = &mhi_cntrl->mhi_dev->dev;
+	} else {
+		/* for MHI controller device, parent is the bus device (e.g. pci device) */
+		dev->parent = mhi_cntrl->cntrl_dev;
+	}
+
 	mhi_dev->mhi_cntrl = mhi_cntrl;
 	mhi_dev->dev_wake = 0;
 
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index a671f58..681960c 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -504,7 +504,7 @@  static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl)
 	wake_up_all(&mhi_cntrl->state_event);
 
 	dev_dbg(dev, "Reset all active channels and remove MHI devices\n");
-	device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device);
+	device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device);
 
 	mutex_lock(&mhi_cntrl->pm_mutex);
 
@@ -637,7 +637,7 @@  static void mhi_pm_sys_error_transition(struct mhi_controller *mhi_cntrl)
 	wake_up_all(&mhi_cntrl->state_event);
 
 	dev_dbg(dev, "Reset all active channels and remove MHI devices\n");
-	device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device);
+	device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device);
 
 	mutex_lock(&mhi_cntrl->pm_mutex);