| Message ID | 20230217142030.16012-6-quic_devipriy@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V2,1/6] dt-bindings: soc: qcom: smd-rpm: Add IPQ9574 compatible | expand |
Hi Konrad, Thanks for taking time to review the patch! 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@quicinc.com> >> --- >> 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 >> + >> &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
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@quicinc.com> >>> --- >>> 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
On 2/20/2023 8:06 PM, Konrad Dybcio wrote: > > > 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. > Sure, understood! >> >> 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@quicinc.com> >>>> --- >>>> 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. Sure, will take care of that in V3 > > 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
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>; + }; + }; +}; + &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 { + reg = <0x0 0x00060000 0x0 0x6000>; + no-map; + }; + tz_region: tz@4a600000 { reg = <0x0 0x4a600000 0x0 0x400000>; no-map; @@ -768,6 +773,18 @@ }; }; + rpm-glink { + 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)>,
Add RPM Glink, RPM message RAM and SMPA1 regulator nodes to support frequency scaling on IPQ9574 Signed-off-by: Devi Priya <quic_devipriy@quicinc.com> --- 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(+)