[00/12] Qcom: LLCC/EDAC: Fix base address used for LLCC banks

  The Qualcomm LLCC/EDAC drivers were using a fixed register stride for
accessing the (Control and Status Regsiters) CSRs of each LLCC bank.
This offset only works for some SoCs like SDM845 for which driver support
was initially added.
    
But the later SoCs use different register stride that vary between the
banks with holes in-between. So it is not possible to use a single register
stride for accessing the CSRs of each bank. By doing so could result in a
crash with the current drivers. So far this crash is not reported since
EDAC_QCOM driver is not enabled in ARM64 defconfig and no one tested the
driver extensively by triggering the EDAC IRQ (that's where each bank
CSRs are accessed).
    
For fixing this issue, let's obtain the base address of each LLCC bank from
devicetree and get rid of the fixed stride.

This series affects multiple platforms but I have only tested this on
SM8250 and SM8450. Testing on other platforms is welcomed.

Thanks,
Mani

Manivannan Sadhasivam (12):
  dt-bindings: arm: msm: Update the maintainers for LLCC
  dt-bindings: arm: msm: Fix register regions used for LLCC banks
  arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks
  qcom: llcc/edac: Fix the base address used for accessing LLCC banks

 .../bindings/arm/msm/qcom,llcc.yaml           | 128 ++++++++++++++++--
 arch/arm64/boot/dts/qcom/sc7180.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sc7280.dtsi          |   5 +-
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        |  10 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm6350.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sm8150.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm8350.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm8450.dtsi          |   7 +-
 drivers/edac/qcom_edac.c                      |  14 +-
 drivers/soc/qcom/llcc-qcom.c                  |  64 +++++----
 include/linux/soc/qcom/llcc-qcom.h            |   4 +-
 13 files changed, 197 insertions(+), 67 deletions(-)
  

Hi Manivannan,

On Wed Dec 7, 2022 at 2:59 PM CET, Manivannan Sadhasivam wrote:
> The Qualcomm LLCC/EDAC drivers were using a fixed register stride for
> accessing the (Control and Status Regsiters) CSRs of each LLCC bank.
> This offset only works for some SoCs like SDM845 for which driver support
> was initially added.
>     
> But the later SoCs use different register stride that vary between the
> banks with holes in-between. So it is not possible to use a single register
> stride for accessing the CSRs of each bank. By doing so could result in a
> crash with the current drivers. So far this crash is not reported since
> EDAC_QCOM driver is not enabled in ARM64 defconfig and no one tested the
> driver extensively by triggering the EDAC IRQ (that's where each bank
> CSRs are accessed).
>     
> For fixing this issue, let's obtain the base address of each LLCC bank from
> devicetree and get rid of the fixed stride.
>
> This series affects multiple platforms but I have only tested this on
> SM8250 and SM8450. Testing on other platforms is welcomed.

If you can tell me *how* I can test it, I'd be happy to test the series
on sm6350, like how to trigger the EDAC IRQ.

So far without any extra patches I don't even see the driver probing,
with this in kconfig

  +CONFIG_EDAC=y
  +CONFIG_EDAC_QCOM=y

I do have /sys/bus/platform/drivers/qcom_llcc_edac at runtime but
nothing in there (except bind, uevent and unbind), and also nothing
interesting in dmesg with "llcc", with edac there's just this message:

  [    0.064800] EDAC MC: Ver: 3.0.0

From what I'm seeing now the edac driver is only registered if the
interrupt is specified but it doesn't seem like sm6350 (=lagoon) has
this irq? Downstream dts is just this:

	cache-controller@9200000 {
		compatible = "lagoon-llcc-v1";
		reg = <0x9200000 0x50000> , <0x9600000 0x50000>;
		reg-names = "llcc_base", "llcc_broadcast_base";
		cap-based-alloc-and-pwr-collapse;
	};

From looking at the downstream code, perhaps it's using the polling mode
there?

	/* Request for ecc irq */
	ecc_irq = llcc_driv_data->ecc_irq;
	if (ecc_irq < 0) {
		dev_info(dev, "No ECC IRQ; defaulting to polling mode\n");

Let me know what you think.

Regards
Luca

>
> Thanks,
> Mani
>
> Manivannan Sadhasivam (12):
>   dt-bindings: arm: msm: Update the maintainers for LLCC
>   dt-bindings: arm: msm: Fix register regions used for LLCC banks
>   arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks
>   arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks
>   qcom: llcc/edac: Fix the base address used for accessing LLCC banks
>
>  .../bindings/arm/msm/qcom,llcc.yaml           | 128 ++++++++++++++++--
>  arch/arm64/boot/dts/qcom/sc7180.dtsi          |   2 +-
>  arch/arm64/boot/dts/qcom/sc7280.dtsi          |   5 +-
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi        |  10 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi          |   7 +-
>  arch/arm64/boot/dts/qcom/sm6350.dtsi          |   2 +-
>  arch/arm64/boot/dts/qcom/sm8150.dtsi          |   7 +-
>  arch/arm64/boot/dts/qcom/sm8250.dtsi          |   7 +-
>  arch/arm64/boot/dts/qcom/sm8350.dtsi          |   7 +-
>  arch/arm64/boot/dts/qcom/sm8450.dtsi          |   7 +-
>  drivers/edac/qcom_edac.c                      |  14 +-
>  drivers/soc/qcom/llcc-qcom.c                  |  64 +++++----
>  include/linux/soc/qcom/llcc-qcom.h            |   4 +-
>  13 files changed, 197 insertions(+), 67 deletions(-)
>
> -- 
> 2.25.1
  

Hi Luca,

On Thu, Dec 08, 2022 at 10:16:27AM +0100, Luca Weiss wrote:
> Hi Manivannan,
> 
> On Wed Dec 7, 2022 at 2:59 PM CET, Manivannan Sadhasivam wrote:
> > The Qualcomm LLCC/EDAC drivers were using a fixed register stride for
> > accessing the (Control and Status Regsiters) CSRs of each LLCC bank.
> > This offset only works for some SoCs like SDM845 for which driver support
> > was initially added.
> >     
> > But the later SoCs use different register stride that vary between the
> > banks with holes in-between. So it is not possible to use a single register
> > stride for accessing the CSRs of each bank. By doing so could result in a
> > crash with the current drivers. So far this crash is not reported since
> > EDAC_QCOM driver is not enabled in ARM64 defconfig and no one tested the
> > driver extensively by triggering the EDAC IRQ (that's where each bank
> > CSRs are accessed).
> >     
> > For fixing this issue, let's obtain the base address of each LLCC bank from
> > devicetree and get rid of the fixed stride.
> >
> > This series affects multiple platforms but I have only tested this on
> > SM8250 and SM8450. Testing on other platforms is welcomed.
> 
> If you can tell me *how* I can test it, I'd be happy to test the series
> on sm6350, like how to trigger the EDAC IRQ.
> 

I suppose there is no manual way to trigger EDAC IRQ on Qcom platforms.
For testing the series, I manually called the EDAC IRQ handler to verify
that it doesn't crash reading the registers.

> So far without any extra patches I don't even see the driver probing,
> with this in kconfig
> 
>   +CONFIG_EDAC=y
>   +CONFIG_EDAC_QCOM=y
> 
> I do have /sys/bus/platform/drivers/qcom_llcc_edac at runtime but
> nothing in there (except bind, uevent and unbind), and also nothing
> interesting in dmesg with "llcc", with edac there's just this message:
> 
>   [    0.064800] EDAC MC: Ver: 3.0.0
> 
> From what I'm seeing now the edac driver is only registered if the
> interrupt is specified but it doesn't seem like sm6350 (=lagoon) has
> this irq? Downstream dts is just this:
> 

Right. The upstream EDAC driver only works in IRQ mode. So you need the
interrupts property in LLCC devicetree node for probing.

> 	cache-controller@9200000 {
> 		compatible = "lagoon-llcc-v1";
> 		reg = <0x9200000 0x50000> , <0x9600000 0x50000>;
> 		reg-names = "llcc_base", "llcc_broadcast_base";
> 		cap-based-alloc-and-pwr-collapse;
> 	};
> 
> From looking at the downstream code, perhaps it's using the polling mode
> there?
> 
> 	/* Request for ecc irq */
> 	ecc_irq = llcc_driv_data->ecc_irq;
> 	if (ecc_irq < 0) {
> 		dev_info(dev, "No ECC IRQ; defaulting to polling mode\n");
> 

In the next version, I will add polling support so that you can test the
series on your platform without any hacks.

Thanks,
Mani

> Let me know what you think.
> 
> Regards
> Luca
> 
> >
> > Thanks,
> > Mani
> >
> > Manivannan Sadhasivam (12):
> >   dt-bindings: arm: msm: Update the maintainers for LLCC
> >   dt-bindings: arm: msm: Fix register regions used for LLCC banks
> >   arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks
> >   arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks
> >   qcom: llcc/edac: Fix the base address used for accessing LLCC banks
> >
> >  .../bindings/arm/msm/qcom,llcc.yaml           | 128 ++++++++++++++++--
> >  arch/arm64/boot/dts/qcom/sc7180.dtsi          |   2 +-
> >  arch/arm64/boot/dts/qcom/sc7280.dtsi          |   5 +-
> >  arch/arm64/boot/dts/qcom/sc8280xp.dtsi        |  10 +-
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi          |   7 +-
> >  arch/arm64/boot/dts/qcom/sm6350.dtsi          |   2 +-
> >  arch/arm64/boot/dts/qcom/sm8150.dtsi          |   7 +-
> >  arch/arm64/boot/dts/qcom/sm8250.dtsi          |   7 +-
> >  arch/arm64/boot/dts/qcom/sm8350.dtsi          |   7 +-
> >  arch/arm64/boot/dts/qcom/sm8450.dtsi          |   7 +-
> >  drivers/edac/qcom_edac.c                      |  14 +-
> >  drivers/soc/qcom/llcc-qcom.c                  |  64 +++++----
> >  include/linux/soc/qcom/llcc-qcom.h            |   4 +-
> >  13 files changed, 197 insertions(+), 67 deletions(-)
> >
> > -- 
> > 2.25.1
>