diff mbox series

arm64: dts: qcom: sc7180: Fix the LLCC base register size

Message ID 20200817040417.11111-1-saiprakash.ranjan@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series arm64: dts: qcom: sc7180: Fix the LLCC base register size | expand

Commit Message

Sai Prakash Ranjan Aug. 17, 2020, 4:04 a.m. UTC
There is only one LLCC logical bank on SC7180 SoC of size
0x50000(320KB) not 2MB, so correct the size and fix copy
paste mistake from SDM845 which had 4 logical banks.

Fixes: 7cee5c742899 ("arm64: dts: qcom: sc7180: Fix node order")
Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache controller node")
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Doug Anderson Aug. 17, 2020, 9:12 p.m. UTC | #1
Hi,

On Sun, Aug 16, 2020 at 9:04 PM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> There is only one LLCC logical bank on SC7180 SoC of size
> 0x50000(320KB) not 2MB, so correct the size and fix copy
> paste mistake from SDM845 which had 4 logical banks.

I guess SDM845 not only has 4 banks but each bank is bigger?  At first
I thought "yeah, 4 banks and 4 * 0x5 = 0x20" except that's not true in
hex.  ;-)


> Fixes: 7cee5c742899 ("arm64: dts: qcom: sc7180: Fix node order")
> Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache controller node")
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Without having any documentation ,this seems sane to me.  I guess it
doesn't do a whole lot because the driver just reads one register from
this whole space (at 0x0003000c bytes off).  So it's just a cleanup,
or is it needed to actually fix something?

...the fact that there's a status register in the middle of this seems
strange, though.  Your commit message makes it sound as if this range
is describing the size of the cache itself and then I would think that
this was the address range where you could read from the cache memory
directly, but that doesn't seem to mesh in my mind with there being a
status register.  Hrm.  Am I just confused as usual?


-Doug
Sai Prakash Ranjan Aug. 18, 2020, 7:38 a.m. UTC | #2
Hi,

On 2020-08-18 02:42, Doug Anderson wrote:
> Hi,
> 
> On Sun, Aug 16, 2020 at 9:04 PM Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>> 
>> There is only one LLCC logical bank on SC7180 SoC of size
>> 0x50000(320KB) not 2MB, so correct the size and fix copy
>> paste mistake from SDM845 which had 4 logical banks.
> 
> I guess SDM845 not only has 4 banks but each bank is bigger?  At first
> I thought "yeah, 4 banks and 4 * 0x5 = 0x20" except that's not true in
> hex.  ;-)
> 

Hehe, no I didn't mean 0x5 * 4 = 0x20 because I mentioned 320KB and 2MB 
specifically
for the same reason in case people think that ;) I just meant that we 
are correcting
the copied size from SDM845, but I agree I need to make it clear in the 
commit msg
which warrants a V2.

> 
>> Fixes: 7cee5c742899 ("arm64: dts: qcom: sc7180: Fix node order")
>> Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache 
>> controller node")
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Without having any documentation ,this seems sane to me.  I guess it
> doesn't do a whole lot because the driver just reads one register from
> this whole space (at 0x0003000c bytes off).  So it's just a cleanup,
> or is it needed to actually fix something?
> 

No, it is not required to fix any functional problems but is correcting 
the
wrong size which I think qualifies for a fixes tag? I don't have a 
strong opinion
though, so I can remove the tag if you feel strongly about it.

> ...the fact that there's a status register in the middle of this seems
> strange, though.  Your commit message makes it sound as if this range
> is describing the size of the cache itself and then I would think that
> this was the address range where you could read from the cache memory
> directly, but that doesn't seem to mesh in my mind with there being a
> status register.  Hrm.  Am I just confused as usual?
> 

It's not describing the cache size, it is the LLCC(LLC Controller) 
register space.
But I believe that the confusion is because of my commit msg, so I will 
post a v2
clearing this with something like below (I have removed the confusing 4 
banks info
of SDM845).

"
There is one LLCC logical bank(LLCC0) on SC7180 SoC and the size of the 
LLCC0 base
is 0x50000(320KB) not 2MB, so correct the size and fix copy paste 
mistake
carried over from SDM845.
"

Thanks,
Sai
Doug Anderson Aug. 18, 2020, 1:56 p.m. UTC | #3
Hi,

On Tue, Aug 18, 2020 at 12:38 AM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> There is one LLCC logical bank(LLCC0) on SC7180 SoC and the size of the
> LLCC0 base
> is 0x50000(320KB) not 2MB, so correct the size and fix copy paste
> mistake
> carried over from SDM845.

Sounds fine, thanks!

-Doug
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index d46b3833e52f..e875f6c3b663 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2618,7 +2618,7 @@  dc_noc: interconnect@9160000 {
 
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc7180-llcc";
-			reg = <0 0x09200000 0 0x200000>, <0 0x09600000 0 0x50000>;
+			reg = <0 0x09200000 0 0x50000>, <0 0x09600000 0 0x50000>;
 			reg-names = "llcc_base", "llcc_broadcast_base";
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};