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

Manivannan Sadhasivam
2022-02-16T17:33:22Z

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