On 2/12/22 12:21 PM, Manivannan Sadhasivam wrote:
Add support for processing the transfer ring from host. For the transfer
ring associated with DL channel, the xfer callback will simply invoked.
For the case of UL channel, the ring elements will be read in a buffer
till the write pointer and later passed to the client driver using the
xfer callback.
The client drivers should provide the callbacks for both UL and DL
channels during registration.
I think you already checked and guaranteed that.
I have a question and suggestion below. But it could
be considered an optimization that could be implemented
in the future, so:
Reviewed-by: Alex Elder <elder@xxxxxxxxxx>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
---
drivers/bus/mhi/ep/main.c | 49 +++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index b937c6cda9ba..baf383a4857b 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -439,6 +439,55 @@ static int mhi_ep_read_channel(struct mhi_ep_cntrl *mhi_cntrl,
return 0;
}
+int mhi_ep_process_tre_ring(struct mhi_ep_ring *ring, struct mhi_ep_ring_element *el)
+{
+ struct mhi_ep_cntrl *mhi_cntrl = ring->mhi_cntrl;
+ struct mhi_result result = {};
+ u32 len = MHI_EP_DEFAULT_MTU;
+ struct mhi_ep_chan *mhi_chan;
+ int ret;
+
+ mhi_chan = &mhi_cntrl->mhi_chan[ring->ch_id];
+
+ /*
+ * Bail out if transfer callback is not registered for the channel.
+ * This is most likely due to the client driver not loaded at this point.
+ */
+ if (!mhi_chan->xfer_cb) {
+ dev_err(&mhi_chan->mhi_dev->dev, "Client driver not available\n");
+ return -ENODEV;
+ }
+
+ if (ring->ch_id % 2) {
+ /* DL channel */
+ result.dir = mhi_chan->dir;
+ mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
+ } else {
+ /* UL channel */
+ do {
+ result.buf_addr = kzalloc(len, GFP_KERNEL);
So you allocate an 8KB buffer into which you copy
received data, then pass that to the ->xfer_cb()
function. Then you free that buffer. Repeatedly.
Two questions about this:
- This suggests that after copying the data in, the
->xfer_cb() function will copy it again, is that
correct?
- If that is correct, why not just reuse the same 8KB
buffer, allocated once outside the loop?
It might also be nice to consider whether you could
allocate the buffer here and have the ->xfer_cb()
function be responsible for freeing it (and ideally,
pass it along rather than copying it again).
+ if (!result.buf_addr)
+ return -ENOMEM;
+
+ ret = mhi_ep_read_channel(mhi_cntrl, ring, &result, len);
+ if (ret < 0) {
+ dev_err(&mhi_chan->mhi_dev->dev, "Failed to read channel\n");
+ kfree(result.buf_addr);
+ return ret;
+ }
+
+ result.dir = mhi_chan->dir;
+ mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
+ kfree(result.buf_addr);
+ result.bytes_xferd = 0;
+
+ /* Read until the ring becomes empty */
+ } while (!mhi_ep_queue_is_empty(mhi_chan->mhi_dev, DMA_TO_DEVICE));
+ }
+
+ return 0;
+}
+
static int mhi_ep_cache_host_cfg(struct mhi_ep_cntrl *mhi_cntrl)
{
struct device *dev = &mhi_cntrl->mhi_dev->dev;
