Messages in this thread | | | Subject | Re: [PATCH v3 2/7] bus: mhi: core: Add support for reading MHI info from device | From | Jeffrey Hugo <> | Date | Wed, 25 Mar 2020 13:04:32 -0600 |
| |
On 3/24/2020 8:38 AM, Jeffrey Hugo wrote: > On 3/24/2020 12:10 AM, Manivannan Sadhasivam wrote: >> The MHI register base has several registers used for getting the MHI >> specific information such as version, family, major, and minor numbers >> from the device. This information can be used by the controller drivers >> for usecases such as applying quirks for a specific revision etc... >> >> While at it, let's also rearrange the local variables >> in mhi_register_controller(). >> >> Suggested-by: Hemant Kumar <hemantk@codeaurora.org> >> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> --- >> drivers/bus/mhi/core/init.c | 19 +++++++++++++++++-- >> drivers/bus/mhi/core/internal.h | 10 ++++++++++ >> include/linux/mhi.h | 17 +++++++++++++++++ >> 3 files changed, 44 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c >> index eb7f556a8531..d136f6c6ca78 100644 >> --- a/drivers/bus/mhi/core/init.c >> +++ b/drivers/bus/mhi/core/init.c >> @@ -802,12 +802,12 @@ static int parse_config(struct mhi_controller >> *mhi_cntrl, >> int mhi_register_controller(struct mhi_controller *mhi_cntrl, >> struct mhi_controller_config *config) >> { >> - int ret; >> - int i; >> struct mhi_event *mhi_event; >> struct mhi_chan *mhi_chan; >> struct mhi_cmd *mhi_cmd; >> struct mhi_device *mhi_dev; >> + u32 soc_info; >> + int ret, i; >> if (!mhi_cntrl) >> return -EINVAL; >> @@ -874,6 +874,21 @@ int mhi_register_controller(struct mhi_controller >> *mhi_cntrl, >> mhi_cntrl->unmap_single = mhi_unmap_single_no_bb; >> } >> + /* Read the MHI device info */ >> + ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, >> + SOC_HW_VERSION_OFFS, &soc_info); >> + if (ret) >> + goto error_alloc_dev; >> + >> + mhi_cntrl->family_number = (soc_info & >> SOC_HW_VERSION_FAM_NUM_BMSK) >> >> + SOC_HW_VERSION_FAM_NUM_SHFT; >> + mhi_cntrl->device_number = (soc_info & >> SOC_HW_VERSION_DEV_NUM_BMSK) >> >> + SOC_HW_VERSION_DEV_NUM_SHFT; >> + mhi_cntrl->major_version = (soc_info & >> SOC_HW_VERSION_MAJOR_VER_BMSK) >> >> + SOC_HW_VERSION_MAJOR_VER_SHFT; >> + mhi_cntrl->minor_version = (soc_info & >> SOC_HW_VERSION_MINOR_VER_BMSK) >> >> + SOC_HW_VERSION_MINOR_VER_SHFT; >> + >> /* Register controller with MHI bus */ >> mhi_dev = mhi_alloc_device(mhi_cntrl); >> if (IS_ERR(mhi_dev)) { >> diff --git a/drivers/bus/mhi/core/internal.h >> b/drivers/bus/mhi/core/internal.h >> index 18066302e6e2..5deadfaa053a 100644 >> --- a/drivers/bus/mhi/core/internal.h >> +++ b/drivers/bus/mhi/core/internal.h >> @@ -196,6 +196,16 @@ extern struct bus_type mhi_bus_type; >> #define BHIE_RXVECSTATUS_STATUS_XFER_COMPL (0x02) >> #define BHIE_RXVECSTATUS_STATUS_ERROR (0x03) >> +#define SOC_HW_VERSION_OFFS (0x224) >> +#define SOC_HW_VERSION_FAM_NUM_BMSK (0xF0000000) >> +#define SOC_HW_VERSION_FAM_NUM_SHFT (28) >> +#define SOC_HW_VERSION_DEV_NUM_BMSK (0x0FFF0000) >> +#define SOC_HW_VERSION_DEV_NUM_SHFT (16) >> +#define SOC_HW_VERSION_MAJOR_VER_BMSK (0x0000FF00) >> +#define SOC_HW_VERSION_MAJOR_VER_SHFT (8) >> +#define SOC_HW_VERSION_MINOR_VER_BMSK (0x000000FF) >> +#define SOC_HW_VERSION_MINOR_VER_SHFT (0) > > I'm tempted to give reviewed-by, however it occurs to me that I don't > see this in the MHI spec. I'm looking at Rev E, which as far as I am > aware is the latest. > > Hemant, is this in the spec, and if so, what Rev? > > I'm concerned that if its not in the spec, we may have an issue with > some device not implementing this as expected. >
Reviewed-by: Jeffrey Hugo <jhugo@codeaurora.org>
I talked with Hemant offline, and this is queued up for the next spec rev. I'm satisfied with that.
-- Jeffrey Hugo Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
| |