Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 22 Feb 2022 14:38:16 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To:     Alex Elder <elder@linaro.org>
Cc:     mhi@lists.linux.dev, quic_hemantk@quicinc.com,
        quic_bbhatt@quicinc.com, quic_jhugo@quicinc.com,
        vinod.koul@linaro.org, bjorn.andersson@linaro.org,
        dmitry.baryshkov@linaro.org, quic_vbadigan@quicinc.com,
        quic_cang@quicinc.com, quic_skananth@quicinc.com,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 16/25] bus: mhi: ep: Add support for powering up the
 MHI endpoint stack
Message-ID: <20220222090816.GD5029@thinkpad>
References: <20220212182117.49438-1-manivannan.sadhasivam@linaro.org>
 <20220212182117.49438-17-manivannan.sadhasivam@linaro.org>
 <10240bc5-ef9f-7555-402e-57ca2b0b0a14@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <10240bc5-ef9f-7555-402e-57ca2b0b0a14@linaro.org>
Precedence: bulk

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.

Thanks,
Mani