On 20.02.2023 14:53, Devi Priya wrote: > Hi Konrad, > > Thanks for taking time to review the patch! I appreciate your gratitude, but please don't toppost (a.k.a don't reply in the first lines of the email), that's rather frowned upon on LKML. > > On 2/17/2023 8:20 PM, Konrad Dybcio wrote: >> >> >> On 17.02.2023 15:20, Devi Priya wrote: >>> Add RPM Glink, RPM message RAM and SMPA1 regulator >>> nodes to support frequency scaling on IPQ9574 >>> >>> Signed-off-by: Devi Priya <quic_devipriy@xxxxxxxxxxx> >>> --- >>> Changes in V2: >>> - Splitted the RPM and CPU Freq changes to individual patches >>> - Moved the regulators node to Board DT >>> - Dropped the regulator-always-on property >>> - Updated the compatible in regulators node with the existing >>> mp5496 compatible >>> >>> arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts | 11 +++++++++++ >>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 17 +++++++++++++++++ >>> 2 files changed, 28 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts >>> index 21b53f34ce84..8a6caaeb0c4b 100644 >>> --- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts >>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts >>> @@ -57,6 +57,17 @@ >>> status = "okay"; >>> }; >>> +&rpm_requests { >>> + regulators { >>> + compatible = "qcom,rpm-mp5496-regulators"; >>> + >>> + ipq9574_s1: s1 { >>> + regulator-min-microvolt = <587500>; >>> + regulator-max-microvolt = <1075000>; >>> + }; >>> + }; >>> +}; >> This belongs in a separate patch. >> > Do you recommend to move this change to the below patch in the next spin? > [PATCH V2 6/6]arm64: dts: qcom: ipq9574: Add cpufreq support Sounds good Also, I think you missed a newline before &rpm_requests now that I look at it. Konrad >>> + >>> &sdhc_1 { >>> pinctrl-0 = <&sdc_default_state>; >>> pinctrl-names = "default"; >>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi >>> index d20f3c7383f5..2f300cbab93e 100644 >>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi >>> @@ -133,6 +133,11 @@ >>> #size-cells = <2>; >>> ranges; >>> + rpm_msg_ram: rpm@60000 { >> Since this is a part of the MMIO region and not a part of DRAM, >> we generally put this node under /soc with the compatible of >> qcom,rpm-msg-ram and without no-map. >> >> And the node name then should be sram@. > Sure, okay. Will update this in V3 >> >>> + reg = <0x0 0x00060000 0x0 0x6000>; >>> + no-map; >>> + }; >>> + >>> tz_region: tz@4a600000 { >>> reg = <0x0 0x4a600000 0x0 0x400000>; >>> no-map; >>> @@ -768,6 +773,18 @@ >>> }; >>> }; >>> + rpm-glink { >> Alphabetically this should come before /soc. > Okay >> >> Konrad >>> + compatible = "qcom,glink-rpm"; >>> + interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>; >>> + qcom,rpm-msg-ram = <&rpm_msg_ram>; >>> + mboxes = <&apcs_glb 0>; >>> + >>> + rpm_requests: glink-channel { >>> + compatible = "qcom,rpm-ipq9574"; >>> + qcom,glink-channels = "rpm_requests"; >>> + }; >>> + }; >>> + >>> timer { >>> compatible = "arm,armv8-timer"; >>> interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > Best Regards, > Devi Priya