Re: [PATCH v2] bus: mhi: core: Fix device hierarchy issue

[Jeffrey Hugo <jhugo@codeaurora.org>]

On 11/24/2020 9:18 AM, Loic Poulain wrote:
> A MHI client device should be child of the MHI controller device.
> Today both MHI controller and its MHI clients are direct children
> of the same bus device. This patch fixes the hierarchy.

Why?

I'm not particularly arguing for or against this change (I think it 
affects me slightly, but not in a breaking way), but this commit text 
seems pretty generic.  It doesn't really help me understand the 
relevance of this change.  It seems to be only describing what you are 
doing, but not the why.  How did you find this?  How does this affect 
the client drivers?  Does it make something the client drivers care 
about better?

To put this another way, "should" is an opinion, and you've provided no 
facts to assert why your opinion is superior to others.

> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>   v2: fix commit message
> 
>   drivers/bus/mhi/core/init.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> 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;
>   
> 


-- 
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.