| Message ID | 20220416025637.83484-1-y.oudjana@protonmail.com |
|---|---|
| Headers | show |
| Series | Add support for MSM8996 Pro | expand |
On 16/04/2022 04:56, Yassine Oudjana wrote: > Add CBF clock and reg. > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> > Acked-by: Rob Herring <robh@kernel.org> > --- > .../devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml > index a20cb10636dd..325f8aef53b2 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml > @@ -10,8 +10,8 @@ maintainers: > - Loic Poulain <loic.poulain@linaro.org> > > description: | > - Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for Power cluster > - and clock 1 is for Perf cluster. > + Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for Power cluster, > + clock 1 is for Perf cluster, and clock 2 is for Coherent bus fabric (CBF). > > properties: > compatible: > @@ -19,7 +19,9 @@ properties: > - qcom,msm8996-apcc > > reg: > - maxItems: 1 > + items: > + - description: Cluster clock registers > + - description: CBF clock registers This breaks the ABI (which might be okay or might be not, but was not mentioned in the commit) and breaks existing DTSes. Please fix them before this patch. Best regards, Krzysztof
On 16/04/2022 04:56, Yassine Oudjana wrote: > MSM8996 Pro (MSM8996SG) has a /4 divisor on the CBF clock > instead of /2. This allows it to reach a lower minimum frequency > of 192000000Hz compared to 307200000Hz on regular MSM8996. > Add support for setting the CBF clock divisor to /4 for MSM8996 Pro. > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> Your From does not match Signed-off-by, here and in all other patches. Please fix the author in the commits and then send a v3. Best regards, Krzysztof
On 16/04/2022 04:56, Yassine Oudjana wrote: > Add a new DTSI for MSM8996 Pro (MSM8996SG) with msm-id and CPU/GPU OPPs. > CBF OPPs and CPR parameters will be added to it as well once support for > CBF scaling and CPR is introduced. > Thank you for your patch. There is something to discuss/improve. > + > +#include "msm8996.dtsi" > + > +/* > + * MSM8996 Pro (also known as MSM8996SG) is a revision of MSM8996 with > + * different CPU, CBF and GPU frequencies as well as CPR parameters. > + */ > +/delete-node/ &cluster0_opp; > +/delete-node/ &cluster1_opp; > + > +/ { > + qcom,msm-id = <305 0x10000>; > + > + cluster0_opp: opp_table0 { No underscores in node names. If this comes from original file, should be fixed in separate commit. Best regards, Krzysztof
On Mon, Apr 18 2022 at 18:04:08 +0200, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 16/04/2022 04:56, Yassine Oudjana wrote: >> Add CBF clock and reg. >> >> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> >> Acked-by: Rob Herring <robh@kernel.org> >> --- >> .../devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 10 >> ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >> b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >> index a20cb10636dd..325f8aef53b2 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >> @@ -10,8 +10,8 @@ maintainers: >> - Loic Poulain <loic.poulain@linaro.org> >> >> description: | >> - Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for >> Power cluster >> - and clock 1 is for Perf cluster. >> + Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for >> Power cluster, >> + clock 1 is for Perf cluster, and clock 2 is for Coherent bus >> fabric (CBF). >> >> properties: >> compatible: >> @@ -19,7 +19,9 @@ properties: >> - qcom,msm8996-apcc >> >> reg: >> - maxItems: 1 >> + items: >> + - description: Cluster clock registers >> + - description: CBF clock registers > > This breaks the ABI (which might be okay or might be not, but was not > mentioned in the commit) and breaks existing DTSes. Please fix them > before this patch. This is only documenting changes made in an earlier patch[1] this series depends on, and the DTSes are fixed in another patch[2] that is also listed as a dependency in the cover letter (both patches aren't applied yet). Shouldn't the ABI changes should be mentioned in those patches instead? [1] https://lore.kernel.org/linux-arm-msm/20210528192541.1120703-1-konrad.dybcio@somainline.org/ [2] https://lore.kernel.org/linux-arm-msm/20210528192541.1120703-2-konrad.dybcio@somainline.org/
On 18/04/2022 21:12, Yassine Oudjana wrote: > > On Mon, Apr 18 2022 at 18:04:08 +0200, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 16/04/2022 04:56, Yassine Oudjana wrote: >>> Add CBF clock and reg. >>> >>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> >>> Acked-by: Rob Herring <robh@kernel.org> >>> --- >>> .../devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 10 >>> ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >>> b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >>> index a20cb10636dd..325f8aef53b2 100644 >>> --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >>> +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml >>> @@ -10,8 +10,8 @@ maintainers: >>> - Loic Poulain <loic.poulain@linaro.org> >>> >>> description: | >>> - Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for >>> Power cluster >>> - and clock 1 is for Perf cluster. >>> + Qualcomm CPU clock controller for MSM8996 CPUs, clock 0 is for >>> Power cluster, >>> + clock 1 is for Perf cluster, and clock 2 is for Coherent bus >>> fabric (CBF). >>> >>> properties: >>> compatible: >>> @@ -19,7 +19,9 @@ properties: >>> - qcom,msm8996-apcc >>> >>> reg: >>> - maxItems: 1 >>> + items: >>> + - description: Cluster clock registers >>> + - description: CBF clock registers >> >> This breaks the ABI (which might be okay or might be not, but was not >> mentioned in the commit) and breaks existing DTSes. Please fix them >> before this patch. > > This is only documenting changes made in an earlier patch[1] this > series depends on, So this other patch breaks the ABI. Was it accepted? The patch you wrote here should go together with the clock change. > and the DTSes are fixed in another patch[2] that > is also listed as a dependency in the cover letter (both patches > aren't applied yet). Shouldn't the ABI changes should be mentioned in > those patches instead? They should be mentioned in the clock driver or here, because usually they come together. :) Best regards, Krzysztof
MSM8996 Pro (also known as MSM8996SG) is a newer revision of MSM8996 with different CPU/CBF/GPU frequencies and CPR parameters. Its CBF clock also has a different divisor. This series handles the difference in the CBF clock and adds a new DTSI for MSM8996 Pro with CPU and GPU OPPs. It also removes reading msm-id from SMEM in qcom-cpufreq-nvmem as it becomes no longer necessary with the introduction. of a separate device tree. Separating MSM8996 and MSM8996 Pro will help with implementing CBF scaling and CPR; as they have different CPR parameters and CPU:CBF OPP mapping which is difficult to implement in the same cluster OPP tables. Dependencies: - clk: qcom: msm8996-cpu: Add CBF support https://lore.kernel.org/linux-arm-msm/20210528192541.1120703-1-konrad.dybcio@somainline.org/#t - arm64: dts: qcom: msm8996: Add support for the CBF clock https://lore.kernel.org/linux-arm-msm/20210528192541.1120703-2-konrad.dybcio@somainline.org/ Changes since v1: - Rebase DT changes on already merged patches[1][2]. - Add more details to commit messages. - Split removing MSM8996 Pro speed bin bits from opp-supported-hw into a separate patch. - Rename msm8996-xiaomi-scorpio.dts to msm8996pro-xiaomi-scorpio.dts [1] https://lore.kernel.org/linux-arm-msm/20220203072226.51482-1-y.oudjana@protonmail.com/T/#m6e1341ccfa50d11d221ba8c618f73c21a83b8acb [2] https://lore.kernel.org/linux-arm-msm/20220203072226.51482-1-y.oudjana@protonmail.com/T/#m36f194cd9da1fee7058a88412985aab10c499fa7 Yassine Oudjana (9): dt-bindings: clk: qcom: msm8996-apcc: Add CBF dt-bindings: clk: qcom: msm8996-apcc: Add MSM8996 Pro compatible clk: qcom: msm8996-cpu: Add MSM8996 Pro CBF support cpufreq: qcom_cpufreq_nvmem: Simplify reading kryo speedbin dt-bindings: opp: opp-v2-kryo-cpu: Remove SMEM arm64: dts: qcom: msm8996: Remove MSM8996 Pro speed bins from cluster OPP tables dt-bindings: arm: qcom: Add MSM8996 Pro compatible arm64: dts: qcom: msm8996: Add MSM8996 Pro support arm64: dts: qcom: msm8996-xiaomi-scorpio: Use MSM8996 Pro .../devicetree/bindings/arm/qcom.yaml | 5 + .../bindings/clock/qcom,msm8996-apcc.yaml | 11 +- .../bindings/opp/opp-v2-kryo-cpu.yaml | 56 ++-- arch/arm64/boot/dts/qcom/Makefile | 2 +- .../boot/dts/qcom/msm8996-xiaomi-common.dtsi | 3 - .../boot/dts/qcom/msm8996-xiaomi-gemini.dts | 1 + arch/arm64/boot/dts/qcom/msm8996.dtsi | 82 ++--- ...rpio.dts => msm8996pro-xiaomi-scorpio.dts} | 4 +- arch/arm64/boot/dts/qcom/msm8996pro.dtsi | 281 ++++++++++++++++++ drivers/clk/qcom/clk-cpu-8996.c | 61 ++-- drivers/cpufreq/Kconfig.arm | 1 - drivers/cpufreq/qcom-cpufreq-nvmem.c | 75 +---- 12 files changed, 410 insertions(+), 172 deletions(-) rename arch/arm64/boot/dts/qcom/{msm8996-xiaomi-scorpio.dts => msm8996pro-xiaomi-scorpio.dts} (99%) create mode 100644 arch/arm64/boot/dts/qcom/msm8996pro.dtsi