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

[Alex Elder]

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@xxxxxxxxxx>
> >
> > 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