| Message ID | 1627581885-32165-5-git-send-email-sibis@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Fixup register offsets to support per core L3 DCVS | expand |
On Thu 29 Jul 13:04 CDT 2021, Sibi Sankar wrote: > Fixup the register regions used by the cpufreq node on SM8350 SoC to > support per core L3 DCVS. > That sounds good, but why are you dropping the platform-specific compatible? Regards, Bjorn > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi > index a631d58166b1..d0a5a5568602 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > @@ -967,11 +967,10 @@ > }; > > cpufreq_hw: cpufreq@18591000 { > - compatible = "qcom,sm8350-cpufreq-epss", "qcom,cpufreq-epss"; > - reg = <0 0x18591000 0 0x1000>, > - <0 0x18592000 0 0x1000>, > - <0 0x18593000 0 0x1000>; > - reg-names = "freq-domain0", "freq-domain1", "freq-domain2"; > + compatible = "qcom,cpufreq-epss"; > + reg = <0 0x18591100 0 0x900>, > + <0 0x18592100 0 0x900>, > + <0 0x18593100 0 0x900>; > > clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_GPLL0>; > clock-names = "xo", "alternate"; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On Wed, Aug 04, 2021 at 05:59:04PM -0500, Bjorn Andersson wrote: > On Thu 29 Jul 13:04 CDT 2021, Sibi Sankar wrote: > > > Fixup the register regions used by the cpufreq node on SM8350 SoC to > > support per core L3 DCVS. > > > > That sounds good, but why are you dropping the platform-specific > compatible? > I also stared at this and the patch that changes the code for a while. My understanding is that removing the platform-specific compatible is part of not breaking 'old' DTBs. Old DTBs for SM8350 contain the larger register regions and must be paired with 'epss_sm8250_soc_data' (driver code) which has the 'old' 'reg_perf_state' offset. New SM8350 DTs only have the 'qcom,cpufreq-epss' compatible, which pairs their smaller register regions with 'epss_soc_data' with the new 'reg_perf_state' offset. It is super-confusing that the platform-specific compatible string is missing. The binding should probably mention that the two platform-specific compatible strings are for backwards compatibility only and should not be added to new or existing DT files that don't have them already. Maybe a 'qcom,sm8350-cpufreq-epss-v2' or similar should be added to avoid/reduce possible confusion and have to option to add SM8350 specific code later.
On 2021-08-05 05:28, Matthias Kaehlcke wrote: > On Wed, Aug 04, 2021 at 05:59:04PM -0500, Bjorn Andersson wrote: >> On Thu 29 Jul 13:04 CDT 2021, Sibi Sankar wrote: >> >> > Fixup the register regions used by the cpufreq node on SM8350 SoC to >> > support per core L3 DCVS. >> > >> >> That sounds good, but why are you dropping the platform-specific >> compatible? >> > > I also stared at this and the patch that changes the code for a while. > > My understanding is that removing the platform-specific compatible is > part > of not breaking 'old' DTBs. Old DTBs for SM8350 contain the larger > register > regions and must be paired with 'epss_sm8250_soc_data' (driver code) > which > has the 'old' 'reg_perf_state' offset. New SM8350 DTs only have the > 'qcom,cpufreq-epss' compatible, which pairs their smaller register > regions > with 'epss_soc_data' with the new 'reg_perf_state' offset. > > It is super-confusing that the platform-specific compatible string is > missing. The binding should probably mention that the two > platform-specific compatible strings are for backwards compatibility > only and should not be added to new or existing DT files that don't > have them already. Maybe a 'qcom,sm8350-cpufreq-epss-v2' or similar > should be added to avoid/reduce possible confusion and have to option > to add SM8350 specific code later. Bjorn, https://patchwork.kernel.org/project/linux-arm-msm/cover/1629458622-4915-1-git-send-email-okukatla@codeaurora.org/ This series affects the design of the l3 provider support for sc7280 which will be in a position to land in ~1-2 respins. So, please share an early ACK or NACK regarding the register re-ordering series so that we can plan to get alternate acceptable versions out faster on the list.
diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi index a631d58166b1..d0a5a5568602 100644 --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi @@ -967,11 +967,10 @@ }; cpufreq_hw: cpufreq@18591000 { - compatible = "qcom,sm8350-cpufreq-epss", "qcom,cpufreq-epss"; - reg = <0 0x18591000 0 0x1000>, - <0 0x18592000 0 0x1000>, - <0 0x18593000 0 0x1000>; - reg-names = "freq-domain0", "freq-domain1", "freq-domain2"; + compatible = "qcom,cpufreq-epss"; + reg = <0 0x18591100 0 0x900>, + <0 0x18592100 0 0x900>, + <0 0x18593100 0 0x900>; clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_GPLL0>; clock-names = "xo", "alternate";
Fixup the register regions used by the cpufreq node on SM8350 SoC to support per core L3 DCVS. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- arch/arm64/boot/dts/qcom/sm8350.dtsi | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)