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: <203a4b25-05ab-e12d-9fba-8bda385dda1b@linaro.org>
Date:   Wed, 29 Mar 2023 13:18:28 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 0/2] Resolve MPM register space situation
Content-Language: en-US
To:     Shawn Guo <shawn.guo@linaro.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Marijn Suijten <marijn.suijten@somainline.org>,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org
References: <20230328-topic-msgram_mpm-v1-0-1b788a5f5a33@linaro.org>
 <20230329035859.GD3554086@dragon>
From:   Konrad Dybcio <konrad.dybcio@linaro.org>
In-Reply-To: <20230329035859.GD3554086@dragon>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 29.03.2023 05:58, Shawn Guo wrote:
> On Tue, Mar 28, 2023 at 12:02:51PM +0200, Konrad Dybcio wrote:
>> The MPM (and some other things, irrelevant to this patchset) resides
>> (as far as the ARM cores are concerned, anyway) in a MMIO-mapped region
>> that's a portion of the RPM (low-power management core)'s RAM, known
>> as the RPM Message RAM. Representing this relation in the Device Tree
>> creates some challenges, as one would either have to treat a memory
>> region as a bus, map nodes in a way such that their reg-s would be
>> overlapping, or supply the nodes with a slice of that region.
>>
>> This series implements the third option, by adding a qcom,rpm-msg-ram
>> property, which has been used for some drivers poking into this region
>> before. Bindings ABI compatibility is preserved through keeping the
>> "normal" (a.k.a read the reg property and map that region) way of
>> passing the register space.
>>
>> Example representation with this patchset:
>>
>> / {
>> 	[...]
>>
>> 	mpm: interrupt-controller {
>> 		compatible = "qcom,mpm";
>> 		qcom,rpm-msg-ram = <&apss_mpm>;
>> 		[...]
>> 	};
>>
>> 	[...]
>>
>> 	soc: soc@0 {
>> 		[...]
>>
>> 		rpm_msg_ram: sram@45f0000 {
>> 			compatible = "qcom,rpm-msg-ram", "mmio-sram";
>> 			reg = <0 0x045f0000 0 0x7000>;
>> 			#address-cells = <1>;
>> 			#size-cells = <1>;
>> 			ranges = <0 0x0 0x045f0000 0x7000>;
>>
>> 			apss_mpm: sram@1b8 {
>> 				reg = <0x1b8 0x48>;
> 
> Per "vMPM register map" in the driver, the slice size should be 0x44
> instead of 0x48.  Is there one register missing from the driver
> comment?
Yeah we should be using 0x44..

> 
> PS. It seems the "n" formula in the driver comment should be corrected
> as below.
> 
>   n = DIV_ROUND_UP(pin_cnt, 32) - 1
Or since we're counting from zero, the ENABLEn should become
ENABLE(n-1) etc.

Konrad

> 
> Shawn
> 
>> 			};
>> 		};
>> 	};
>> };