| Message ID | 20221212123311.146261-4-manivannan.sadhasivam@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Qcom: LLCC/EDAC: Fix base address used for LLCC banks | expand |
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. > > Also, let's get rid of reg-names property as it is not needed anymore. > The driver is expected to parse the reg field based on index to get the > addresses of each LLCC banks. > > Cc: <stable@vger.kernel.org> # 5.4 > Fixes: ba0411ddd133 ("arm64: dts: sdm845: Add device node for Last level cache controller") > Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 65032b94b46d..683b861e060d 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -2132,8 +2132,9 @@ uart15: serial@a9c000 { > > llcc: system-cache-controller@1100000 { > compatible = "qcom,sdm845-llcc"; > - reg = <0 0x01100000 0 0x31000>, <0 0x01300000 0 0x50000>; > - reg-names = "llcc_base", "llcc_broadcast_base"; > + reg = <0 0x01100000 0 0x50000>, <0 0x01180000 0 0x50000>, > + <0 0x01200000 0 0x50000>, <0 0x01280000 0 0x50000>, > + <0 0x01300000 0 0x50000>; > interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>; > }; > Reviewed-by: Sai Prakash Ranjan <quic_saipraka@quicinc.com>
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. > > Also, let's get rid of reg-names property as it is not needed anymore. > The driver is expected to parse the reg field based on index to get the > addresses of each LLCC banks. > > Cc: <stable@vger.kernel.org> # 5.4 No, you cannot backport it. You will break users. > Fixes: ba0411ddd133 ("arm64: dts: sdm845: Add device node for Last level cache controller") > Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 65032b94b46d..683b861e060d 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -2132,8 +2132,9 @@ uart15: serial@a9c000 { > > llcc: system-cache-controller@1100000 { > compatible = "qcom,sdm845-llcc"; > - reg = <0 0x01100000 0 0x31000>, <0 0x01300000 0 0x50000>; > - reg-names = "llcc_base", "llcc_broadcast_base"; Once property was made required, you cannot remove it. What if other bindings user depends on it? Please instead keep/update the reg-names and/or mark it as deprecated. It must stay in DTS for some time. Best regards, Krzysztof
On Tue, Dec 13, 2022 at 05:27:45PM +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. > > > > Also, let's get rid of reg-names property as it is not needed anymore. > > The driver is expected to parse the reg field based on index to get the > > addresses of each LLCC banks. > > > > Cc: <stable@vger.kernel.org> # 5.4 > > No, you cannot backport it. You will break users. > If the driver change gets backported, it will break users, isn't it? > > Fixes: ba0411ddd133 ("arm64: dts: sdm845: Add device node for Last level cache controller") > > Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index 65032b94b46d..683b861e060d 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -2132,8 +2132,9 @@ uart15: serial@a9c000 { > > > > llcc: system-cache-controller@1100000 { > > compatible = "qcom,sdm845-llcc"; > > - reg = <0 0x01100000 0 0x31000>, <0 0x01300000 0 0x50000>; > > - reg-names = "llcc_base", "llcc_broadcast_base"; > > Once property was made required, you cannot remove it. What if other > bindings user depends on it? > > Please instead keep/update the reg-names and/or mark it as deprecated. > It must stay in DTS for some time. > Fair enough. I will mark it as deprecated in binding and will keep it in dts. Thanks, Mani > Best regards, > Krzysztof >
On 13/12/2022 18:13, Manivannan Sadhasivam wrote: > On Tue, Dec 13, 2022 at 05:27:45PM +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. >>> >>> Also, let's get rid of reg-names property as it is not needed anymore. >>> The driver is expected to parse the reg field based on index to get the >>> addresses of each LLCC banks. >>> >>> Cc: <stable@vger.kernel.org> # 5.4 >> >> No, you cannot backport it. You will break users. >> > > If the driver change gets backported, it will break users, isn't it? Whether driver change gets backported or not - all out of tree kernel users, other systems, firmwares/bootloaders are broken and backporting driver piece will not fix it. By this backport you mean that the change can go alone to v5.4 kernel (you did not write here dependency on other backport) and I wonder if v5.4 kernel works with this patch... > >>> Fixes: ba0411ddd133 ("arm64: dts: sdm845: Add device node for Last level cache controller") >>> Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com> >>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >>> --- >>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi >>> index 65032b94b46d..683b861e060d 100644 >>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi >>> @@ -2132,8 +2132,9 @@ uart15: serial@a9c000 { >>> >>> llcc: system-cache-controller@1100000 { >>> compatible = "qcom,sdm845-llcc"; >>> - reg = <0 0x01100000 0 0x31000>, <0 0x01300000 0 0x50000>; >>> - reg-names = "llcc_base", "llcc_broadcast_base"; >> >> Once property was made required, you cannot remove it. What if other >> bindings user depends on it? >> >> Please instead keep/update the reg-names and/or mark it as deprecated. >> It must stay in DTS for some time. >> > > Fair enough. I will mark it as deprecated in binding and will keep it in dts. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 65032b94b46d..683b861e060d 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -2132,8 +2132,9 @@ uart15: serial@a9c000 { llcc: system-cache-controller@1100000 { compatible = "qcom,sdm845-llcc"; - reg = <0 0x01100000 0 0x31000>, <0 0x01300000 0 0x50000>; - reg-names = "llcc_base", "llcc_broadcast_base"; + reg = <0 0x01100000 0 0x50000>, <0 0x01180000 0 0x50000>, + <0 0x01200000 0 0x50000>, <0 0x01280000 0 0x50000>, + <0 0x01300000 0 0x50000>; interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>; };
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. Also, let's get rid of reg-names property as it is not needed anymore. The driver is expected to parse the reg field based on index to get the addresses of each LLCC banks. Cc: <stable@vger.kernel.org> # 5.4 Fixes: ba0411ddd133 ("arm64: dts: sdm845: Add device node for Last level cache controller") Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)