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

From: Jeffrey Hugo
Date: Fri Mar 04 2022 - 16:47:07 EST


On 2/17/2022 2:53 AM, Manivannan Sadhasivam wrote:
On Tue, Feb 15, 2022 at 02:02:41PM -0600, Alex Elder wrote:

[...]

+#define MHI_REG_OFFSET 0x100
+#define BHI_REG_OFFSET 0x200

Rather than defining the REG_OFFSET values here and adding
them to every definition below, why not have the base
address used (e.g., in mhi_write_reg_field()) be adjusted
by the constant amount?

I'm just looking at mhi_init_mmio() (in the existing code)
as an example, but for example, the base address used
comes from mhi_cntrl->regs. Can you instead just define
a pointer somewhere that is the base of the MHI register
range, which is already offset by the appropriate amount?


I've defined two set of APIs for MHI and BHI read/write. They will add the
respective offsets.


While you are making changes, maybe don't have a set BHI_REG_OFFSET? Sure, I think it is always 0x200, but that is a convention and nothing I've seen in the spec mandates it. You can derive it from the bhi offset register.

This way, if it ever moves in some future chip, this code should just work.