Re: [PATCH v3 16/25] bus: mhi: ep: Add support for powering up the MHI endpoint stack

[Alex Elder <elder@linaro.org>]

On 2/22/22 3:08 AM, Manivannan Sadhasivam wrote:
> On Tue, Feb 15, 2022 at 04:39:37PM -0600, Alex Elder wrote:
>> On 2/12/22 12:21 PM, Manivannan Sadhasivam wrote:
>>> Add support for MHI endpoint power_up that includes initializing the MMIO
>>> and rings, caching the host MHI registers, and setting the MHI state to M0.
>>> After registering the MHI EP controller, the stack has to be powered up
>>> for usage.
>>>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>
>> Very little to say on this one.		-Alex
>>
>>> ---
>>>    drivers/bus/mhi/ep/internal.h |   6 +
>>>    drivers/bus/mhi/ep/main.c     | 229 ++++++++++++++++++++++++++++++++++
>>>    include/linux/mhi_ep.h        |  22 ++++
>>>    3 files changed, 257 insertions(+)
>>>
>>> diff --git a/drivers/bus/mhi/ep/internal.h b/drivers/bus/mhi/ep/internal.h
>>> index e4e8f06c2898..ee8c5974f0c0 100644
>>> --- a/drivers/bus/mhi/ep/internal.h
>>> +++ b/drivers/bus/mhi/ep/internal.h
>>> @@ -242,4 +242,10 @@ int mhi_ep_set_m0_state(struct mhi_ep_cntrl *mhi_cntrl);
>>>    int mhi_ep_set_m3_state(struct mhi_ep_cntrl *mhi_cntrl);
>>>    int mhi_ep_set_ready_state(struct mhi_ep_cntrl *mhi_cntrl);
>>> +/* MHI EP memory management functions */
>>> +int mhi_ep_alloc_map(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, size_t size,
>>> +		     phys_addr_t *phys_ptr, void __iomem **virt);
>>> +void mhi_ep_unmap_free(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, phys_addr_t phys,
>>> +		       void __iomem *virt, size_t size);
>>> +
>>>    #endif
>>> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> 
> [...]
> 
>>> +
>>> +static void mhi_ep_enable_int(struct mhi_ep_cntrl *mhi_cntrl)
>>> +{
>>
>> Are channel doorbell interrupts enabled separately now?
>> (There was previously an enable_chdb_interrupts() call.)
>>
> 
> Doorbell interrupts are enabled when the corresponding channel gets started.
> Enabling all interrupts here triggers spurious irqs as some of the interrupts
> associated with hw channels always get triggered.

This is excellent.  Thanks for the explanation.	-Alex

> 
> Thanks,
> Mani