On 11/13/2023 12:18 AM, Jeffrey Hugo wrote:
On 11/11/2023 8:59 PM, Baochen Qiang wrote:
On 11/11/2023 1:14 AM, Jeffrey Hugo wrote:
On 11/10/2023 3:21 AM, Kalle Valo wrote:
From: Baochen Qiang <quic_bqiang@xxxxxxxxxxx>
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@xxxxxxxxxxx>
Signed-off-by: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
---
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"?
It means MHI devices with a type of MHI_DEVICE_XFER. See also
mhi_destroy_device();
I think you are confused about my comment. "thats" is not correct
English. I was suggesting you replace it with "that are", but there
are many ways to reword the comment.
Sorry for misunderstood your comment. Will refine it in next version.
+ 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.
Thanks for pointing that. Yes, it is not correct to treat all
channels as autoqueue regardless of its initial configuration. So how
about change as below:
Seems ok.
/* The difference between mhi_prepare_for_transfer_autoqueue() and
mhi_prepare_for_transfer() comes from how to treat downlink channel */
mhi_prepare_for_transfer_dev(struct device *dev, ...)
{
...
dl_chan = mhi_dev->dl_chan;
...
if (dl_chan->pre_alloc)
mhi_prepare_for_transfer_autoqueue(dev);
else
mhi_prepare_for_transfer(dev);
}
/* And then iterate all devices and call
mhi_prepare_for_transfer_dev() for each. */
int mhi_prepare_all_for_transfer(struct mhi_controller *mhi_cntrl)
{
return device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL,
mhi_prepare_for_transfer_dev);
}
EXPORT_SYMBOL_GPL(mhi_prepare_all_for_transfer);
