Re: [PATCH RFC 2/8] bus: mhi: host: add new interfaces to handle MHI channels directly

[Jeffrey Hugo <quic_jhugo@quicinc.com>]

On 11/10/2023 3:21 AM, Kalle Valo wrote:
> From: Baochen Qiang <quic_bqiang@quicinc.com>
> 
> When using mhi_power_down_no_destroy() MHI hosts need to unprepare MHI channels
> by themselves.  Similarly, MHI stack will also not create new MHI device since
> old devices were not destroyed, so MHI hosts need to prepare channels as well.
> Hence add these two interfaces to make that possible.
> 
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30
> 
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
> ---
>   drivers/bus/mhi/host/main.c | 91 +++++++++++++++++++++++++++++++++++++
>   include/linux/mhi.h         | 18 ++++++++
>   2 files changed, 109 insertions(+)
> 
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index dcf627b36e82..9bcf8a49c000 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -1667,6 +1667,49 @@ int mhi_prepare_for_transfer_autoqueue(struct mhi_device *mhi_dev)
>   }
>   EXPORT_SYMBOL_GPL(mhi_prepare_for_transfer_autoqueue);
>   
> +static int __mhi_prepare_for_transfer_autoqueue(struct device *dev, void *data)
> +{
> +	struct mhi_device *mhi_dev;
> +	struct mhi_chan *ul_chan, *dl_chan;
> +	enum mhi_ee_type ee = MHI_EE_MAX;
> +
> +	if (dev->bus != &mhi_bus_type)
> +		return 0;
> +
> +	mhi_dev = to_mhi_device(dev);
> +
> +	/* Only prepare virtual devices thats attached to bus */

"that are"?

> +	if (mhi_dev->dev_type == MHI_DEVICE_CONTROLLER)
> +		return 0;
> +
> +	ul_chan = mhi_dev->ul_chan;
> +	dl_chan = mhi_dev->dl_chan;
> +
> +	/*
> +	 * If execution environment is specified, remove only those devices that
> +	 * started in them based on ee_mask for the channels as we move on to a
> +	 * different execution environment
> +	 */
> +	if (data)
> +		ee = *(enum mhi_ee_type *)data;
> +
> +	if (ul_chan && ee != MHI_EE_MAX && !(ul_chan->ee_mask & BIT(ee)))
> +		return 0;
> +
> +
> +	if (dl_chan && ee != MHI_EE_MAX && !(dl_chan->ee_mask & BIT(ee)))
> +		return 0;
> +
> +	return mhi_prepare_for_transfer_autoqueue(mhi_dev);
> +}
> +
> +int mhi_prepare_all_for_transfer_autoqueue(struct mhi_controller *mhi_cntrl)
> +{
> +	return device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL,
> +				     __mhi_prepare_for_transfer_autoqueue);
> +}
> +EXPORT_SYMBOL_GPL(mhi_prepare_all_for_transfer_autoqueue);

This seems broken.  It appears to configure all channels as autoqueue, 
regardless of how the controller initially configured them.  This would 
only be safe to use if all channels were configured for autoqueue, but 
would silently cause issues otherwise.


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k