On 2/22/22 2:18 AM, Manivannan Sadhasivam wrote:
On Tue, Feb 15, 2022 at 04:39:30PM -0600, Alex Elder wrote:
On 2/12/22 12:21 PM, Manivannan Sadhasivam wrote:
Add support for processing MHI endpoint interrupts such as control
interrupt, command interrupt and channel interrupt from the host.
The interrupts will be generated in the endpoint device whenever host
writes to the corresponding doorbell registers. The doorbell logic
is handled inside the hardware internally.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
Unless I'm mistaken, you have some bugs here.
Beyond that, I question whether you should be using workqueues
for handling all interrupts. For now, it's fine, but there
might be room for improvement after this is accepted upstream
(using threaded interrupt handlers, for example).
Only reason I didn't use bottom halves is that the memory for TRE buffers need
to be allocated each time, so essentially the caller should not sleep.
Threaded interrupt handlers can sleep. If scheduled, they run
immediately after hard interrupt handlers. For receive buffers,
yes, replacing a receive buffer just consumed would require an
allocation, but for transmit I think it might be possible to
avoid the need to do a memory allocation. (Things to think
about at some future date.)
This is currently a limitation of iATU where there are only 8 windows for
mapping the host memory and each memory region size is also limited.
Those are hard limitations, and probably what constrains you the most.
-Alex
-Alex
---
drivers/bus/mhi/ep/main.c | 113 +++++++++++++++++++++++++++++++++++++-
include/linux/mhi_ep.h | 2 +
2 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index ccb3c2795041..072b872e735b 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -185,6 +185,56 @@ static void mhi_ep_ring_worker(struct work_struct *work)
}
}
+static void mhi_ep_queue_channel_db(struct mhi_ep_cntrl *mhi_cntrl,
+ unsigned long ch_int, u32 ch_idx)
+{
+ struct device *dev = &mhi_cntrl->mhi_dev->dev;
+ struct mhi_ep_ring_item *item;
+ struct mhi_ep_ring *ring;
+ unsigned int i;
Why not u32 i? And why is the ch_int argument unsigned long? The value
passed in is a u32.
for_each_set_bit() expects the 2nd argument to be of type "unsigned long".
Thanks,
Mani
