The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.
On SC7180, there is only one LLCC bank available. So only change needed is
to remove the reg-names property from LLCC node to conform to the binding.
The driver is expected to parse the reg field based on index to get the
addresses of each LLCC banks.
Cc: <[email protected]> # 5.6
Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache controller node")
Reported-by: Parikshit Pareek <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index f71cf21a8dd8..b0d524bbf051 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2759,7 +2759,6 @@ dc_noc: interconnect@9160000 {
system-cache-controller@9200000 {
compatible = "qcom,sc7180-llcc";
reg = <0 0x09200000 0 0x50000>, <0 0x09600000 0 0x50000>;
- reg-names = "llcc_base", "llcc_broadcast_base";
interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
};
--
2.25.1
On 12/12/2022 6:03 PM, Manivannan Sadhasivam wrote:
> The LLCC block has several banks each with a different base address
> and holes in between. So it is not a correct approach to cover these
> banks with a single offset/size. Instead, the individual bank's base
> address needs to be specified in devicetree with the exact size.
>
> On SC7180, there is only one LLCC bank available. So only change needed is
> to remove the reg-names property from LLCC node to conform to the binding.
>
> The driver is expected to parse the reg field based on index to get the
> addresses of each LLCC banks.
>
> Cc: <[email protected]> # 5.6
> Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache controller node")
> Reported-by: Parikshit Pareek <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index f71cf21a8dd8..b0d524bbf051 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -2759,7 +2759,6 @@ dc_noc: interconnect@9160000 {
> system-cache-controller@9200000 {
> compatible = "qcom,sc7180-llcc";
> reg = <0 0x09200000 0 0x50000>, <0 0x09600000 0 0x50000>;
> - reg-names = "llcc_base", "llcc_broadcast_base";
> interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
> };
>
Reviewed-by: Sai Prakash Ranjan <[email protected]>
On 12/12/2022 13:33, Manivannan Sadhasivam wrote:
> The LLCC block has several banks each with a different base address
> and holes in between. So it is not a correct approach to cover these
> banks with a single offset/size. Instead, the individual bank's base
> address needs to be specified in devicetree with the exact size.
>
> On SC7180, there is only one LLCC bank available. So only change needed is
> to remove the reg-names property from LLCC node to conform to the binding.
>
> The driver is expected to parse the reg field based on index to get the
> addresses of each LLCC banks.
>
> Cc: <[email protected]> # 5.6
Oh, no, there is no single bug here. Binding from v5.6+ (which cannot be
changed) required/defined such reg-names. This is neither a bug nor
possible to backport.
> Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache controller node")
Drop.
> Reported-by: Parikshit Pareek <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index f71cf21a8dd8..b0d524bbf051 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -2759,7 +2759,6 @@ dc_noc: interconnect@9160000 {
> system-cache-controller@9200000 {
> compatible = "qcom,sc7180-llcc";
> reg = <0 0x09200000 0 0x50000>, <0 0x09600000 0 0x50000>;
> - reg-names = "llcc_base", "llcc_broadcast_base";
That's an ABI break...
> interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
> };
>
Best regards,
Krzysztof
On Tue, Dec 13, 2022 at 05:30:09PM +0100, Krzysztof Kozlowski wrote:
> On 12/12/2022 13:33, Manivannan Sadhasivam wrote:
> > The LLCC block has several banks each with a different base address
> > and holes in between. So it is not a correct approach to cover these
> > banks with a single offset/size. Instead, the individual bank's base
> > address needs to be specified in devicetree with the exact size.
> >
> > On SC7180, there is only one LLCC bank available. So only change needed is
> > to remove the reg-names property from LLCC node to conform to the binding.
> >
> > The driver is expected to parse the reg field based on index to get the
> > addresses of each LLCC banks.
> >
> > Cc: <[email protected]> # 5.6
>
> Oh, no, there is no single bug here. Binding from v5.6+ (which cannot be
> changed) required/defined such reg-names. This is neither a bug nor
> possible to backport.
>
> > Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache controller node")
>
> Drop.
>
> > Reported-by: Parikshit Pareek <[email protected]>
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > index f71cf21a8dd8..b0d524bbf051 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > @@ -2759,7 +2759,6 @@ dc_noc: interconnect@9160000 {
> > system-cache-controller@9200000 {
> > compatible = "qcom,sc7180-llcc";
> > reg = <0 0x09200000 0 0x50000>, <0 0x09600000 0 0x50000>;
> > - reg-names = "llcc_base", "llcc_broadcast_base";
>
> That's an ABI break...
>
As agreed, I will keep reg-names in dts for now.
Thanks,
Mani
> > interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
> > };
> >
>
> Best regards,
> Krzysztof
>
--
மணிவண்ணன் சதாசிவம்