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;
Message-ID: <a7b08bec-1692-4b77-cd22-6177aaa95c5e@quicinc.com>
Date:   Fri, 4 Mar 2022 14:46:59 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.0
Subject: Re: [PATCH v3 08/25] bus: mhi: ep: Add support for registering MHI
 endpoint controllers
Content-Language: en-US
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Alex Elder <elder@linaro.org>
CC:     <mhi@lists.linux.dev>, <quic_hemantk@quicinc.com>,
        <quic_bbhatt@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>
References: <20220212182117.49438-1-manivannan.sadhasivam@linaro.org>
 <20220212182117.49438-9-manivannan.sadhasivam@linaro.org>
 <4cc78936-b419-4738-b5b2-65c53be06f33@linaro.org>
 <20220217095319.GA11964@workstation>
From:   Jeffrey Hugo <quic_jhugo@quicinc.com>
In-Reply-To: <20220217095319.GA11964@workstation>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.