| Message ID | 20250227-topic-sm8650-pmu-ppi-partition-v3-1-0f6feeefe50f@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: sm8650: switch to 4 interrupt cells to add PPI partitions for PMUs | expand |
On 27.02.2025 5:04 PM, Neil Armstrong wrote: > The ARM PMU interrupt is sometimes defined as IRQ_TYPE_LEVEL_LOW, > or IRQ_TYPE_LEVEL_HIGH, but downstream and recent platforms used the > IRQ_TYPE_LEVEL_HIGH flag so align the SM8650 definition to have a > functional PMU working. > > Fixes: c8a346e408cb ("arm64: dts: qcom: Split PMU nodes for heterogeneous CPUs") > Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi") > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- I couldn't find anything to back this up, not inside, not on Arm's website, but downstream agrees, so.. Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
On Thu, Feb 27, 2025 at 06:50:31PM +0100, Konrad Dybcio wrote: > On 27.02.2025 5:04 PM, Neil Armstrong wrote: > > The ARM PMU interrupt is sometimes defined as IRQ_TYPE_LEVEL_LOW, > > or IRQ_TYPE_LEVEL_HIGH, but downstream and recent platforms used the > > IRQ_TYPE_LEVEL_HIGH flag so align the SM8650 definition to have a > > functional PMU working. > > > > Fixes: c8a346e408cb ("arm64: dts: qcom: Split PMU nodes for heterogeneous CPUs") > > Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi") > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > > --- > > I couldn't find anything to back this up, not inside, not on Arm's > website, but downstream agrees, so.. > The GIC doesn't really have a notion of LOW vs HIGH in the programmable registers. This is generally a design time parameter, e.g. for GIC-600: Level-sensitive PPI signals are active-LOW by default, as with previous Arm GIC implementations. However, individual PPI signals can be inverted and synchronized using parameters gic600_<config_name>_PPI<ppi_id>_<cpu_number>_<ppi_number>_<INV/SYNC>. https://developer.arm.com/documentation/100336/0106/components-and-configuration/redistributor/redistributor-ppi-signals For Linux it shouldn't really matter, because gic_configure_irq() ignores the polarity and looks only at IRQ_TYPE_LEVEL_MASK. If you still want this to represent the actual truth, then all hints I can find only back this down (not up) I'm afraid: Arm® Cortex®‑A520 Core Technical Reference Manual Arm® Cortex®-A720 Core Technical Reference Manual Arm® Cortex®-X4 Core Technical Reference Manual 17.2 Performance monitors interrupts When the PMU generates an interrupt, the nPMUIRQ[n] output is driven LOW. https://developer.arm.com/documentation/102517/0004/Performance-Monitors-Extension-support-/Performance-monitors-interrupts https://developer.arm.com/documentation/102530/0002/Performance-Monitors-Extension-support-/Performance-monitors-interrupts https://developer.arm.com/documentation/102484/0003/Performance-Monitors-Extension-support-/Performance-monitors-interrupts So please check if this patch is really needed, otherwise I'd suggest dropping it. Thanks, Stephan
On 27/02/2025 19:48, Stephan Gerhold wrote: > On Thu, Feb 27, 2025 at 06:50:31PM +0100, Konrad Dybcio wrote: >> On 27.02.2025 5:04 PM, Neil Armstrong wrote: >>> The ARM PMU interrupt is sometimes defined as IRQ_TYPE_LEVEL_LOW, >>> or IRQ_TYPE_LEVEL_HIGH, but downstream and recent platforms used the >>> IRQ_TYPE_LEVEL_HIGH flag so align the SM8650 definition to have a >>> functional PMU working. >>> >>> Fixes: c8a346e408cb ("arm64: dts: qcom: Split PMU nodes for heterogeneous CPUs") >>> Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi") >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >> >> I couldn't find anything to back this up, not inside, not on Arm's >> website, but downstream agrees, so.. >> > > The GIC doesn't really have a notion of LOW vs HIGH in the programmable > registers. This is generally a design time parameter, e.g. for GIC-600: > > Level-sensitive PPI signals are active-LOW by default, as with > previous Arm GIC implementations. However, individual PPI signals can > be inverted and synchronized using parameters > gic600_<config_name>_PPI<ppi_id>_<cpu_number>_<ppi_number>_<INV/SYNC>. > > https://developer.arm.com/documentation/100336/0106/components-and-configuration/redistributor/redistributor-ppi-signals > > For Linux it shouldn't really matter, because gic_configure_irq() > ignores the polarity and looks only at IRQ_TYPE_LEVEL_MASK. > > If you still want this to represent the actual truth, then all hints > I can find only back this down (not up) I'm afraid: > > Arm® Cortex®‑A520 Core Technical Reference Manual > Arm® Cortex®-A720 Core Technical Reference Manual > Arm® Cortex®-X4 Core Technical Reference Manual > > 17.2 Performance monitors interrupts > When the PMU generates an interrupt, the nPMUIRQ[n] output is driven LOW. > > https://developer.arm.com/documentation/102517/0004/Performance-Monitors-Extension-support-/Performance-monitors-interrupts > https://developer.arm.com/documentation/102530/0002/Performance-Monitors-Extension-support-/Performance-monitors-interrupts > https://developer.arm.com/documentation/102484/0003/Performance-Monitors-Extension-support-/Performance-monitors-interrupts > > So please check if this patch is really needed, otherwise I'd suggest > dropping it. Thanks a lot for looking into this ! I guess I'll drop this, and we may harmonize all qcom dtsi with LOW. Neil > > Thanks, > Stephan
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi index de960bcaf3ccf6e2be47bf63a02effbfb75241bf..895f70cf6f57a84dda38604789d5ad6d80471944 100644 --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi @@ -1417,17 +1417,17 @@ opp-3302400000 { pmu-a520 { compatible = "arm,cortex-a520-pmu"; - interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>; + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>; }; pmu-a720 { compatible = "arm,cortex-a720-pmu"; - interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>; + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>; }; pmu-x4 { compatible = "arm,cortex-x4-pmu"; - interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>; + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>; }; psci {
The ARM PMU interrupt is sometimes defined as IRQ_TYPE_LEVEL_LOW, or IRQ_TYPE_LEVEL_HIGH, but downstream and recent platforms used the IRQ_TYPE_LEVEL_HIGH flag so align the SM8650 definition to have a functional PMU working. Fixes: c8a346e408cb ("arm64: dts: qcom: Split PMU nodes for heterogeneous CPUs") Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi") Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- arch/arm64/boot/dts/qcom/sm8650.dtsi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)