Re: [PATCH v3 08/25] bus: mhi: ep: Add support for registering MHI endpoint controllers

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Mon, Feb 14, 2022 at 05:04:23PM -0800, Hemant Kumar wrote:
> Hi Mani,
> 
> On 2/12/2022 10:21 AM, Manivannan Sadhasivam wrote:
> > This commit adds support for registering MHI endpoint controller drivers
> > with the MHI endpoint stack. MHI endpoint controller drivers manages
> > the interaction with the host machines such as x86. They are also the
> > MHI endpoint bus master in charge of managing the physical link between the
> > host and endpoint device.
> > 
> > The endpoint controller driver encloses all information about the
> > underlying physical bus like PCIe. The registration process involves
> > parsing the channel configuration and allocating an MHI EP device.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >   drivers/bus/mhi/Kconfig       |   1 +
> >   drivers/bus/mhi/Makefile      |   3 +
> >   drivers/bus/mhi/ep/Kconfig    |  10 ++
> >   drivers/bus/mhi/ep/Makefile   |   2 +
> >   drivers/bus/mhi/ep/internal.h | 160 +++++++++++++++++++++++
> >   drivers/bus/mhi/ep/main.c     | 234 ++++++++++++++++++++++++++++++++++
> >   include/linux/mhi_ep.h        | 143 +++++++++++++++++++++
> >   7 files changed, 553 insertions(+)
> >   create mode 100644 drivers/bus/mhi/ep/Kconfig
> >   create mode 100644 drivers/bus/mhi/ep/Makefile
> >   create mode 100644 drivers/bus/mhi/ep/internal.h
> >   create mode 100644 drivers/bus/mhi/ep/main.c
> >   create mode 100644 include/linux/mhi_ep.h
> > 

[...]

> > +#define MHI_CTRL_INT_STATUS_A7			0x4
> can we get rid of all instances of "_A7" as this corresponds to Cortex-A7,
> in future this can change? At MHI core layer, we can avoid this naming
> convetion, even though register names are inculding them now and may change
> to something different later. This MHI EP driver would still be used for
> those new cortex vers.

Since these registers are not documented by the spec, I just went with
the register definition available for SDX55. But you are right, and Alex
too, that it may change in future.

I'll remove the A7 suffix.

Thanks,
Mani

> > +#define MHI_CTRL_INT_STATUS_A7_MSK		BIT(0)
> > +#define MHI_CTRL_INT_STATUS_CRDB_MSK		BIT(1)
> > +#define MHI_CHDB_INT_STATUS_A7_n(n)		(0x28 + 0x4 * (n))
> > +#define MHI_ERDB_INT_STATUS_A7_n(n)		(0x38 + 0x4 * (n))
> > +
> [..]
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a
> Linux Foundation Collaborative Project