| Message ID | 20230625202547.174647-1-dmitry.baryshkov@linaro.org |
|---|---|
| Headers | show |
| Series | ARM: qcom: apq8064: support CPU frequency scaling | expand |
On Sun, 25 Jun 2023 23:25:26 +0300, Dmitry Baryshkov wrote: > The L2 cache device on Qualcomm Krait platforms controls the supplying > voltages and the cache frequency. Add corresponding bindings for this > device. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > .../bindings/cache/qcom,krait-l2-cache.yaml | 75 +++++++++++++++++++ > include/dt-bindings/soc/qcom,krait-l2-cache.h | 12 +++ > 2 files changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/cache/qcom,krait-l2-cache.yaml > create mode 100644 include/dt-bindings/soc/qcom,krait-l2-cache.h > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cache/sifive,ccache0.example.dtb: cache-controller@2010000: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cache/sifive,ccache0.example.dtb: cache-controller@2010000: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cache/sifive,ccache0.example.dtb: cache-controller@2010000: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cache/sifive,ccache0.example.dtb: cache-controller@2010000: Unevaluated properties are not allowed ('compatible', 'interrupts', 'memory-region', 'reg' were unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible', 'l3-cache' were unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l3-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l3-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l3-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l3-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.example.dtb: l3-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible', 'l3-cache' were unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l3-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l3-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l3-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l3-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l3-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible:0: 'qcom,krait-l2-cache' was expected from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: compatible: ['cache'] is too short from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: '#interconnect-cells' is a required property from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.example.dtb: l2-cache: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/cache/qcom,krait-l2-cache.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230625202547.174647-6-dmitry.baryshkov@linaro.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 25.06.2023 22:25, Dmitry Baryshkov wrote: > Define bindings for the Qualcomm Krait CPU and L2 clock controller. This > device is used on old Qualcomm SoCs (APQ8064, MSM8960) and supports up > to 4 core clocks and a separate L2 clock. Furthermore, L2 clock is > represented as the interconnect to facilitate L2 frequency scaling > together with scaling the CPU frequencies. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Worth noting that there's no Krait cluster containing more than 4 cores and the last SoC using this uarch was released 10y ago, so we can quite confidently say that the max cpu no won't change. Konrad > include/dt-bindings/clock/qcom,krait-cc.h | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > create mode 100644 include/dt-bindings/clock/qcom,krait-cc.h > > diff --git a/include/dt-bindings/clock/qcom,krait-cc.h b/include/dt-bindings/clock/qcom,krait-cc.h > new file mode 100644 > index 000000000000..ff69a0a968d8 > --- /dev/null > +++ b/include/dt-bindings/clock/qcom,krait-cc.h > @@ -0,0 +1,17 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > +/* > + * Copyright (C) 2023 Linaro Ltd. All rights reserved. > + */ > + > +#ifndef __DT_BINDINGS_CLOCK_QCOM_KRAIT_CC_H > +#define __DT_BINDINGS_CLOCK_QCOM_KRAIT_CC_H > + > +#define KRAIT_CPU_0 0 > +#define KRAIT_CPU_1 1 > +#define KRAIT_CPU_2 2 > +#define KRAIT_CPU_3 3 > +#define KRAIT_L2 4 > + > +#define KRAIT_NUM_CLOCKS 5 > + > +#endif
On 25.06.2023 22:25, Dmitry Baryshkov wrote: > The SPM / SAW2 device also provides a voltage regulator functionality > with optional AVS (Adaptive Voltage Scaling) support. The exact register > sequence and voltage ranges differs from device to device. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/soc/qcom/spm.c | 205 ++++++++++++++++++++++++++++++++++++++++- > include/soc/qcom/spm.h | 9 ++ > 2 files changed, 212 insertions(+), 2 deletions(-) > > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c > index a6cbeb40831b..3c16a7e1710c 100644 > --- a/drivers/soc/qcom/spm.c > +++ b/drivers/soc/qcom/spm.c > @@ -9,19 +9,31 @@ > #include <linux/kernel.h> > #include <linux/init.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/linear_range.h> > #include <linux/module.h> > #include <linux/slab.h> > #include <linux/of.h> > #include <linux/of_address.h> > #include <linux/of_device.h> > +#include <linux/bitfield.h> This addition is very out-of-order > #include <linux/err.h> > #include <linux/platform_device.h> > +#include <linux/regulator/driver.h> > +#include <linux/smp.h> > #include <soc/qcom/spm.h> > > +#define FIELD_SET(current, mask, val) \ > + (((current) & ~(mask)) | FIELD_PREP((mask), (val))) > + > #define SPM_CTL_INDEX 0x7f > #define SPM_CTL_INDEX_SHIFT 4 > #define SPM_CTL_EN BIT(0) > > +#define SPM_1_1_AVS_CTL_AVS_ENABLED BIT(27) > +#define SPM_AVS_CTL_MIN_VLVL (0x3f << 10) > +#define SPM_AVS_CTL_MAX_VLVL (0x3f << 17) GENMASK > + > enum spm_reg { > SPM_REG_CFG, > SPM_REG_SPM_CTL, > @@ -31,10 +43,12 @@ enum spm_reg { > SPM_REG_PMIC_DATA_1, > SPM_REG_VCTL, > SPM_REG_SEQ_ENTRY, > - SPM_REG_SPM_STS, > + SPM_REG_STS0, > + SPM_REG_STS1, > SPM_REG_PMIC_STS, > SPM_REG_AVS_CTL, > SPM_REG_AVS_LIMIT, > + SPM_REG_RST, > SPM_REG_NR, > }; > > @@ -171,6 +185,10 @@ static const struct spm_reg_data spm_reg_8226_cpu = { > > static const u16 spm_reg_offset_v1_1[SPM_REG_NR] = { > [SPM_REG_CFG] = 0x08, > + [SPM_REG_STS0] = 0x0c, > + [SPM_REG_STS1] = 0x10, > + [SPM_REG_VCTL] = 0x14, > + [SPM_REG_AVS_CTL] = 0x18, > [SPM_REG_SPM_CTL] = 0x20, > [SPM_REG_PMIC_DLY] = 0x24, > [SPM_REG_PMIC_DATA_0] = 0x28, > @@ -178,7 +196,12 @@ static const u16 spm_reg_offset_v1_1[SPM_REG_NR] = { > [SPM_REG_SEQ_ENTRY] = 0x80, > }; > > +static void smp_set_vdd_v1_1(void *data); > + > /* SPM register data for 8064 */ > +static struct linear_range spm_v1_1_regulator_range = > + REGULATOR_LINEAR_RANGE(700000, 0, 56, 12500); > + > static const struct spm_reg_data spm_reg_8064_cpu = { > .reg_offset = spm_reg_offset_v1_1, > .spm_cfg = 0x1F, > @@ -189,6 +212,10 @@ static const struct spm_reg_data spm_reg_8064_cpu = { > 0x10, 0x54, 0x30, 0x0C, 0x24, 0x30, 0x0F }, > .start_index[PM_SLEEP_MODE_STBY] = 0, > .start_index[PM_SLEEP_MODE_SPC] = 2, > + .set_vdd = smp_set_vdd_v1_1, > + .range = &spm_v1_1_regulator_range, > + .init_uV = 1300000, > + .ramp_delay = 1250, > }; > > static inline void spm_register_write(struct spm_driver_data *drv, > @@ -240,6 +267,179 @@ void spm_set_low_power_mode(struct spm_driver_data *drv, > spm_register_write_sync(drv, SPM_REG_SPM_CTL, ctl_val); > } > > +static int spm_set_voltage_sel(struct regulator_dev *rdev, unsigned int selector) > +{ > + struct spm_driver_data *drv = rdev_get_drvdata(rdev); > + > + drv->volt_sel = selector; > + > + /* Always do the SAW register writes on the corresponding CPU */ > + return smp_call_function_single(drv->reg_cpu, drv->reg_data->set_vdd, drv, true); > +} > + > +static int spm_get_voltage_sel(struct regulator_dev *rdev) > +{ > + struct spm_driver_data *drv = rdev_get_drvdata(rdev); > + > + return drv->volt_sel; > +} > + > +static const struct regulator_ops spm_reg_ops = { > + .set_voltage_sel = spm_set_voltage_sel, > + .get_voltage_sel = spm_get_voltage_sel, > + .list_voltage = regulator_list_voltage_linear_range, > + .set_voltage_time_sel = regulator_set_voltage_time_sel, > +}; > + > +static void smp_set_vdd_v1_1(void *data) > +{ > + struct spm_driver_data *drv = data; > + unsigned int vlevel = drv->volt_sel; > + unsigned int vctl, data0, data1, avs_ctl, sts; > + bool avs_enabled; Reverse-Christmas-tree? > + > + vlevel |= 0x80; /* band */ That's conveniently 1<<7.. do we know if it's a significant number or just a bit that does something within that field? > + > + avs_ctl = spm_register_read(drv, SPM_REG_AVS_CTL); > + vctl = spm_register_read(drv, SPM_REG_VCTL); > + data0 = spm_register_read(drv, SPM_REG_PMIC_DATA_0); > + data1 = spm_register_read(drv, SPM_REG_PMIC_DATA_1); > + > + avs_enabled = avs_ctl & SPM_1_1_AVS_CTL_AVS_ENABLED; > + > + /* If AVS is enabled, switch it off during the voltage change */ > + if (avs_enabled) { > + avs_ctl &= ~SPM_1_1_AVS_CTL_AVS_ENABLED; > + spm_register_write(drv, SPM_REG_AVS_CTL, avs_ctl); > + } > + > + /* Kick the state machine back to idle */ > + spm_register_write(drv, SPM_REG_RST, 1); > + > + vctl = FIELD_SET(vctl, 0xff, vlevel); > + data0 = FIELD_SET(data0, 0xff, vlevel); > + data1 = FIELD_SET(data1, 0x3f, vlevel); > + data1 = FIELD_SET(data1, 0x3f << 16, vlevel); GENMASK > + > + spm_register_write(drv, SPM_REG_VCTL, vctl); > + spm_register_write(drv, SPM_REG_PMIC_DATA_0, data0); > + spm_register_write(drv, SPM_REG_PMIC_DATA_1, data1); > + > + if (read_poll_timeout_atomic(spm_register_read, > + sts, sts == vlevel, > + 1, 200, false, > + drv, SPM_REG_STS1)) { Not sure if misaligned or thunderfox is acting up again > + dev_err_ratelimited(drv->dev, "timeout setting the voltage (%x %x)!\n", sts, vlevel); > + goto enable_avs; > + } > + > + if (avs_enabled) { > + unsigned int max_avs = vlevel & 0x3f; GENMASK > + unsigned int min_avs = max(max_avs, 4U) - 4; So it's 0 or >= (1<<31 - 4)? > + avs_ctl = FIELD_SET(avs_ctl, SPM_AVS_CTL_MIN_VLVL, min_avs); > + avs_ctl = FIELD_SET(avs_ctl, SPM_AVS_CTL_MAX_VLVL, max_avs); > + spm_register_write(drv, SPM_REG_AVS_CTL, avs_ctl); > + } > + > +enable_avs: > + if (avs_enabled) { > + avs_ctl |= SPM_1_1_AVS_CTL_AVS_ENABLED; > + spm_register_write(drv, SPM_REG_AVS_CTL, avs_ctl); > + } > +} > + > +static int spm_get_cpu(struct device *dev) > +{ > + int cpu; > + bool found; Reverse-Christmas-tree? > + > + for_each_possible_cpu(cpu) { > + struct device_node *cpu_node, *saw_node; As long as Linux is running, there should be at least one CPU up, so this always gets entered, perhaps the definitions could be moved to the main function body > + > + cpu_node = of_cpu_device_node_get(cpu); > + if (!cpu_node) > + continue; > + > + saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0); > + found = (saw_node == dev->of_node); The error checking here works out, but it's a bit cryptic.. Though I'm not opposed to saving 3 cycles on slow and old CPUs :D > + of_node_put(saw_node); > + of_node_put(cpu_node); > + > + if (found) > + return cpu; > + } > + > + /* L2 SPM is not bound to any CPU, tie it to CPU0 */ > + > + return 0; > +} > + > +#ifdef CONFIG_REGULATOR > +static int spm_register_regulator(struct device *dev, struct spm_driver_data *drv) > +{ > + struct regulator_config config = { > + .dev = dev, > + .driver_data = drv, > + }; > + struct regulator_desc *rdesc; > + struct regulator_dev *rdev; > + int ret; > + bool found; Reverse-Christmas-tree? Konrad > + > + if (!drv->reg_data->set_vdd) > + return 0; > + > + rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL); > + if (!rdesc) > + return -ENOMEM; > + > + rdesc->name = "spm"; > + rdesc->of_match = of_match_ptr("regulator"); > + rdesc->type = REGULATOR_VOLTAGE; > + rdesc->owner = THIS_MODULE; > + rdesc->ops = &spm_reg_ops; > + > + rdesc->linear_ranges = drv->reg_data->range; > + rdesc->n_linear_ranges = 1; > + rdesc->n_voltages = rdesc->linear_ranges[rdesc->n_linear_ranges - 1].max_sel + 1; > + rdesc->ramp_delay = drv->reg_data->ramp_delay; > + > + drv->reg_cpu = spm_get_cpu(dev); > + dev_dbg(dev, "SAW2 bound to CPU %d\n", drv->reg_cpu); > + > + /* > + * Program initial voltage, otherwise registration will also try > + * setting the voltage, which might result in undervolting the CPU. > + */ > + drv->volt_sel = DIV_ROUND_UP(drv->reg_data->init_uV - rdesc->min_uV, > + rdesc->uV_step); > + ret = linear_range_get_selector_high(drv->reg_data->range, > + drv->reg_data->init_uV, > + &drv->volt_sel, > + &found); > + if (ret) { > + dev_err(dev, "Initial uV value out of bounds\n"); > + return ret; > + } > + > + /* Always do the SAW register writes on the corresponding CPU */ > + smp_call_function_single(drv->reg_cpu, drv->reg_data->set_vdd, drv, true); > + > + rdev = devm_regulator_register(dev, rdesc, &config); > + if (IS_ERR(rdev)) { > + dev_err(dev, "failed to register regulator\n"); > + return PTR_ERR(rdev); > + } > + > + return 0; > +} > +#else > +static int spm_register_regulator(struct device *dev, struct spm_driver_data *drv) > +{ > + return 0; > +} > +#endif > + > static const struct of_device_id spm_match_table[] = { > { .compatible = "qcom,sdm660-gold-saw2-v4.1-l2", > .data = &spm_reg_660_gold_l2 }, > @@ -292,6 +492,7 @@ static int spm_dev_probe(struct platform_device *pdev) > return -ENODEV; > > drv->reg_data = match_id->data; > + drv->dev = &pdev->dev; > platform_set_drvdata(pdev, drv); > > /* Write the SPM sequences first.. */ > @@ -319,7 +520,7 @@ static int spm_dev_probe(struct platform_device *pdev) > if (drv->reg_data->reg_offset[SPM_REG_SPM_CTL]) > spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY); > > - return 0; > + return spm_register_regulator(&pdev->dev, drv); > } > > static struct platform_driver spm_driver = { > diff --git a/include/soc/qcom/spm.h b/include/soc/qcom/spm.h > index 4951f9d8b0bd..9859ebe42003 100644 > --- a/include/soc/qcom/spm.h > +++ b/include/soc/qcom/spm.h > @@ -30,11 +30,20 @@ struct spm_reg_data { > u32 avs_limit; > u8 seq[MAX_SEQ_DATA]; > u8 start_index[PM_SLEEP_MODE_NR]; > + > + smp_call_func_t set_vdd; > + /* for now we support only a single range */ > + struct linear_range *range; > + unsigned int ramp_delay; > + unsigned int init_uV; > }; > > struct spm_driver_data { > void __iomem *reg_base; > const struct spm_reg_data *reg_data; > + struct device *dev; > + unsigned int volt_sel; > + int reg_cpu; > }; > > void spm_set_low_power_mode(struct spm_driver_data *drv,
On 26/06/2023 14:50, Konrad Dybcio wrote: > On 25.06.2023 22:25, Dmitry Baryshkov wrote: >> Scaling the frequencies on some of Qualcomm Krait platforms (e.g. >> APQ8064) also requires scaling of the L2 cache frequency. As the >> l2-cache device node is places under /cpus/ path, it is not created by >> default by the OF code. Create corresponding device here. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- > I think a generic solution (i.e. for each cpu node call > of_platform_populate in drivers/of/platform.c : > of_platform_default_populate_init) could be beneficial Yep. I thought about it, but I saw no direct benefit for it. Note, that we do not instantiate cpu devices directly. But, maybe something like /devices/system/cache/foo would make sense. > > Konrad >> drivers/cpufreq/qcom-cpufreq-nvmem.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c >> index a88b6fe5db50..ab78ef1531d0 100644 >> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c >> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c >> @@ -380,6 +380,7 @@ static int __init qcom_cpufreq_init(void) >> { >> struct device_node *np = of_find_node_by_path("/"); >> const struct of_device_id *match; >> + unsigned int cpu; >> int ret; >> >> if (!np) >> @@ -390,6 +391,25 @@ static int __init qcom_cpufreq_init(void) >> if (!match) >> return -ENODEV; >> >> + for_each_possible_cpu(cpu) { >> + struct device *dev = get_cpu_device(cpu); >> + struct device_node *cache; >> + struct platform_device *pdev; >> + >> + cache = of_find_next_cache_node(dev->of_node); >> + if (!cache) >> + continue; >> + >> + if (of_device_is_compatible(cache, "qcom,krait-l2-cache")) { >> + pdev = of_platform_device_create(cache, NULL, NULL); >> + if (IS_ERR(pdev)) >> + pr_err("%s: %pe, failed to create L2 cache node\n", __func__, pdev); >> + /* the error is not fatal */ >> + } >> + >> + of_node_put(cache); >> + } >> + >> ret = platform_driver_register(&qcom_cpufreq_driver); >> if (unlikely(ret < 0)) >> return ret;
On 26/06/2023 14:47, Konrad Dybcio wrote: > On 25.06.2023 22:25, Dmitry Baryshkov wrote: >> The SPM / SAW2 device also provides a voltage regulator functionality >> with optional AVS (Adaptive Voltage Scaling) support. The exact register >> sequence and voltage ranges differs from device to device. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/soc/qcom/spm.c | 205 ++++++++++++++++++++++++++++++++++++++++- >> include/soc/qcom/spm.h | 9 ++ >> 2 files changed, 212 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c >> index a6cbeb40831b..3c16a7e1710c 100644 >> --- a/drivers/soc/qcom/spm.c >> +++ b/drivers/soc/qcom/spm.c >> @@ -9,19 +9,31 @@ >> #include <linux/kernel.h> >> #include <linux/init.h> >> #include <linux/io.h> >> +#include <linux/iopoll.h> >> +#include <linux/linear_range.h> >> #include <linux/module.h> >> #include <linux/slab.h> >> #include <linux/of.h> >> #include <linux/of_address.h> >> #include <linux/of_device.h> >> +#include <linux/bitfield.h> > This addition is very out-of-order The whole list is out of order. Proably I should sort it first > >> #include <linux/err.h> >> #include <linux/platform_device.h> >> +#include <linux/regulator/driver.h> >> +#include <linux/smp.h> >> #include <soc/qcom/spm.h> >> >> +#define FIELD_SET(current, mask, val) \ >> + (((current) & ~(mask)) | FIELD_PREP((mask), (val))) >> + >> #define SPM_CTL_INDEX 0x7f >> #define SPM_CTL_INDEX_SHIFT 4 >> #define SPM_CTL_EN BIT(0) >> >> +#define SPM_1_1_AVS_CTL_AVS_ENABLED BIT(27) >> +#define SPM_AVS_CTL_MIN_VLVL (0x3f << 10) >> +#define SPM_AVS_CTL_MAX_VLVL (0x3f << 17) > GENMASK ack > >> + >> enum spm_reg { >> SPM_REG_CFG, >> SPM_REG_SPM_CTL, >> @@ -31,10 +43,12 @@ enum spm_reg { >> SPM_REG_PMIC_DATA_1, >> SPM_REG_VCTL, >> SPM_REG_SEQ_ENTRY, >> - SPM_REG_SPM_STS, >> + SPM_REG_STS0, >> + SPM_REG_STS1, >> SPM_REG_PMIC_STS, >> SPM_REG_AVS_CTL, >> SPM_REG_AVS_LIMIT, >> + SPM_REG_RST, >> SPM_REG_NR, >> }; >> >> @@ -171,6 +185,10 @@ static const struct spm_reg_data spm_reg_8226_cpu = { >> >> static const u16 spm_reg_offset_v1_1[SPM_REG_NR] = { >> [SPM_REG_CFG] = 0x08, >> + [SPM_REG_STS0] = 0x0c, >> + [SPM_REG_STS1] = 0x10, >> + [SPM_REG_VCTL] = 0x14, >> + [SPM_REG_AVS_CTL] = 0x18, >> [SPM_REG_SPM_CTL] = 0x20, >> [SPM_REG_PMIC_DLY] = 0x24, >> [SPM_REG_PMIC_DATA_0] = 0x28, >> @@ -178,7 +196,12 @@ static const u16 spm_reg_offset_v1_1[SPM_REG_NR] = { >> [SPM_REG_SEQ_ENTRY] = 0x80, >> }; >> >> +static void smp_set_vdd_v1_1(void *data); >> + >> /* SPM register data for 8064 */ >> +static struct linear_range spm_v1_1_regulator_range = >> + REGULATOR_LINEAR_RANGE(700000, 0, 56, 12500); >> + >> static const struct spm_reg_data spm_reg_8064_cpu = { >> .reg_offset = spm_reg_offset_v1_1, >> .spm_cfg = 0x1F, >> @@ -189,6 +212,10 @@ static const struct spm_reg_data spm_reg_8064_cpu = { >> 0x10, 0x54, 0x30, 0x0C, 0x24, 0x30, 0x0F }, >> .start_index[PM_SLEEP_MODE_STBY] = 0, >> .start_index[PM_SLEEP_MODE_SPC] = 2, >> + .set_vdd = smp_set_vdd_v1_1, >> + .range = &spm_v1_1_regulator_range, >> + .init_uV = 1300000, >> + .ramp_delay = 1250, >> }; >> >> static inline void spm_register_write(struct spm_driver_data *drv, >> @@ -240,6 +267,179 @@ void spm_set_low_power_mode(struct spm_driver_data *drv, >> spm_register_write_sync(drv, SPM_REG_SPM_CTL, ctl_val); >> } >> >> +static int spm_set_voltage_sel(struct regulator_dev *rdev, unsigned int selector) >> +{ >> + struct spm_driver_data *drv = rdev_get_drvdata(rdev); >> + >> + drv->volt_sel = selector; >> + >> + /* Always do the SAW register writes on the corresponding CPU */ >> + return smp_call_function_single(drv->reg_cpu, drv->reg_data->set_vdd, drv, true); >> +} >> + >> +static int spm_get_voltage_sel(struct regulator_dev *rdev) >> +{ >> + struct spm_driver_data *drv = rdev_get_drvdata(rdev); >> + >> + return drv->volt_sel; >> +} >> + >> +static const struct regulator_ops spm_reg_ops = { >> + .set_voltage_sel = spm_set_voltage_sel, >> + .get_voltage_sel = spm_get_voltage_sel, >> + .list_voltage = regulator_list_voltage_linear_range, >> + .set_voltage_time_sel = regulator_set_voltage_time_sel, >> +}; >> + >> +static void smp_set_vdd_v1_1(void *data) >> +{ >> + struct spm_driver_data *drv = data; >> + unsigned int vlevel = drv->volt_sel; >> + unsigned int vctl, data0, data1, avs_ctl, sts; >> + bool avs_enabled; > Reverse-Christmas-tree? > >> + >> + vlevel |= 0x80; /* band */ > That's conveniently 1<<7.. do we know if it's a significant number > or just a bit that does something within that field? More like 2 << 6. A bit of the issue is that PMIC_DATA_n format is PMIC-specific. I'll see what I can do here. > >> + >> + avs_ctl = spm_register_read(drv, SPM_REG_AVS_CTL); >> + vctl = spm_register_read(drv, SPM_REG_VCTL); >> + data0 = spm_register_read(drv, SPM_REG_PMIC_DATA_0); >> + data1 = spm_register_read(drv, SPM_REG_PMIC_DATA_1); >> + >> + avs_enabled = avs_ctl & SPM_1_1_AVS_CTL_AVS_ENABLED; >> + >> + /* If AVS is enabled, switch it off during the voltage change */ >> + if (avs_enabled) { >> + avs_ctl &= ~SPM_1_1_AVS_CTL_AVS_ENABLED; >> + spm_register_write(drv, SPM_REG_AVS_CTL, avs_ctl); >> + } >> + >> + /* Kick the state machine back to idle */ >> + spm_register_write(drv, SPM_REG_RST, 1); >> + >> + vctl = FIELD_SET(vctl, 0xff, vlevel); >> + data0 = FIELD_SET(data0, 0xff, vlevel); >> + data1 = FIELD_SET(data1, 0x3f, vlevel); >> + data1 = FIELD_SET(data1, 0x3f << 16, vlevel); > GENMASK > >> + >> + spm_register_write(drv, SPM_REG_VCTL, vctl); >> + spm_register_write(drv, SPM_REG_PMIC_DATA_0, data0); >> + spm_register_write(drv, SPM_REG_PMIC_DATA_1, data1); >> + >> + if (read_poll_timeout_atomic(spm_register_read, >> + sts, sts == vlevel, >> + 1, 200, false, >> + drv, SPM_REG_STS1)) { > Not sure if misaligned or thunderfox is acting up again off-by-one, I'll fix it. > >> + dev_err_ratelimited(drv->dev, "timeout setting the voltage (%x %x)!\n", sts, vlevel); >> + goto enable_avs; >> + } >> + >> + if (avs_enabled) { >> + unsigned int max_avs = vlevel & 0x3f; > GENMASK > >> + unsigned int min_avs = max(max_avs, 4U) - 4; > So it's 0 or >= (1<<31 - 4)? > >> + avs_ctl = FIELD_SET(avs_ctl, SPM_AVS_CTL_MIN_VLVL, min_avs); >> + avs_ctl = FIELD_SET(avs_ctl, SPM_AVS_CTL_MAX_VLVL, max_avs); >> + spm_register_write(drv, SPM_REG_AVS_CTL, avs_ctl); >> + } >> + >> +enable_avs: >> + if (avs_enabled) { >> + avs_ctl |= SPM_1_1_AVS_CTL_AVS_ENABLED; >> + spm_register_write(drv, SPM_REG_AVS_CTL, avs_ctl); >> + } >> +} >> + >> +static int spm_get_cpu(struct device *dev) >> +{ >> + int cpu; >> + bool found; > Reverse-Christmas-tree? > >> + >> + for_each_possible_cpu(cpu) { >> + struct device_node *cpu_node, *saw_node; > As long as Linux is running, there should be at least one CPU up, > so this always gets entered, perhaps the definitions could be moved > to the main function body Huh, I'm not sure that I got you correct here. Do you mean movign cpu_node and saw_node to the top of spm_get_cpu()? > >> + >> + cpu_node = of_cpu_device_node_get(cpu); >> + if (!cpu_node) >> + continue; >> + >> + saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0); >> + found = (saw_node == dev->of_node); > The error checking here works out, but it's a bit cryptic.. Though > I'm not opposed to saving 3 cycles on slow and old CPUs :D It's not the error checking per se. We have to put both nodes before returning. So an alternative might me: saw_node = ... if (saw_node == cpu_node) { of_node_put(saw_node); of_node_put(cpu_node); return cpu; } of_node_put(saw_node); of_node_put(cpu_node); But it looks worse to me. Did you mean this kind of code or was something else on your mind? > >> + of_node_put(saw_node); >> + of_node_put(cpu_node); >> + >> + if (found) >> + return cpu; >> + } >> + >> + /* L2 SPM is not bound to any CPU, tie it to CPU0 */ >> + >> + return 0; >> +} >> + >> +#ifdef CONFIG_REGULATOR >> +static int spm_register_regulator(struct device *dev, struct spm_driver_data *drv) >> +{ >> + struct regulator_config config = { >> + .dev = dev, >> + .driver_data = drv, >> + }; >> + struct regulator_desc *rdesc; >> + struct regulator_dev *rdev; >> + int ret; >> + bool found; > Reverse-Christmas-tree? > > Konrad >> + >> + if (!drv->reg_data->set_vdd) >> + return 0; >> + >> + rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL); >> + if (!rdesc) >> + return -ENOMEM; >> + >> + rdesc->name = "spm"; >> + rdesc->of_match = of_match_ptr("regulator"); >> + rdesc->type = REGULATOR_VOLTAGE; >> + rdesc->owner = THIS_MODULE; >> + rdesc->ops = &spm_reg_ops; >> + >> + rdesc->linear_ranges = drv->reg_data->range; >> + rdesc->n_linear_ranges = 1; >> + rdesc->n_voltages = rdesc->linear_ranges[rdesc->n_linear_ranges - 1].max_sel + 1; >> + rdesc->ramp_delay = drv->reg_data->ramp_delay; >> + >> + drv->reg_cpu = spm_get_cpu(dev); >> + dev_dbg(dev, "SAW2 bound to CPU %d\n", drv->reg_cpu); >> + >> + /* >> + * Program initial voltage, otherwise registration will also try >> + * setting the voltage, which might result in undervolting the CPU. >> + */ >> + drv->volt_sel = DIV_ROUND_UP(drv->reg_data->init_uV - rdesc->min_uV, >> + rdesc->uV_step); >> + ret = linear_range_get_selector_high(drv->reg_data->range, >> + drv->reg_data->init_uV, >> + &drv->volt_sel, >> + &found); >> + if (ret) { >> + dev_err(dev, "Initial uV value out of bounds\n"); >> + return ret; >> + } >> + >> + /* Always do the SAW register writes on the corresponding CPU */ >> + smp_call_function_single(drv->reg_cpu, drv->reg_data->set_vdd, drv, true); >> + >> + rdev = devm_regulator_register(dev, rdesc, &config); >> + if (IS_ERR(rdev)) { >> + dev_err(dev, "failed to register regulator\n"); >> + return PTR_ERR(rdev); >> + } >> + >> + return 0; >> +} >> +#else >> +static int spm_register_regulator(struct device *dev, struct spm_driver_data *drv) >> +{ >> + return 0; >> +} >> +#endif >> + >> static const struct of_device_id spm_match_table[] = { >> { .compatible = "qcom,sdm660-gold-saw2-v4.1-l2", >> .data = &spm_reg_660_gold_l2 }, >> @@ -292,6 +492,7 @@ static int spm_dev_probe(struct platform_device *pdev) >> return -ENODEV; >> >> drv->reg_data = match_id->data; >> + drv->dev = &pdev->dev; >> platform_set_drvdata(pdev, drv); >> >> /* Write the SPM sequences first.. */ >> @@ -319,7 +520,7 @@ static int spm_dev_probe(struct platform_device *pdev) >> if (drv->reg_data->reg_offset[SPM_REG_SPM_CTL]) >> spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY); >> >> - return 0; >> + return spm_register_regulator(&pdev->dev, drv); >> } >> >> static struct platform_driver spm_driver = { >> diff --git a/include/soc/qcom/spm.h b/include/soc/qcom/spm.h >> index 4951f9d8b0bd..9859ebe42003 100644 >> --- a/include/soc/qcom/spm.h >> +++ b/include/soc/qcom/spm.h >> @@ -30,11 +30,20 @@ struct spm_reg_data { >> u32 avs_limit; >> u8 seq[MAX_SEQ_DATA]; >> u8 start_index[PM_SLEEP_MODE_NR]; >> + >> + smp_call_func_t set_vdd; >> + /* for now we support only a single range */ >> + struct linear_range *range; >> + unsigned int ramp_delay; >> + unsigned int init_uV; >> }; >> >> struct spm_driver_data { >> void __iomem *reg_base; >> const struct spm_reg_data *reg_data; >> + struct device *dev; >> + unsigned int volt_sel; >> + int reg_cpu; >> }; >> >> void spm_set_low_power_mode(struct spm_driver_data *drv,
On 25.06.2023 22:25, Dmitry Baryshkov wrote: > The SAW2 device should describe the regulator constraints rather than > just declaring that it has the regulator. > > Drop the 'regulator' property. If/when CPU voltage scaling is > implemented for this platform, proper regulator nodes show be added > instead. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi > index f0ef86fadc9d..ad3c922843c7 100644 > --- a/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi > +++ b/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi > @@ -350,31 +350,26 @@ acc3: power-manager@b0b8000 { > saw0: regulator@b089000 { > compatible = "qcom,saw2"; > reg = <0x0b089000 0x1000>, <0x0b009000 0x1000>; > - regulator; > }; > > saw1: regulator@b099000 { > compatible = "qcom,saw2"; > reg = <0x0b099000 0x1000>, <0x0b009000 0x1000>; > - regulator; > }; > > saw2: regulator@b0a9000 { > compatible = "qcom,saw2"; > reg = <0x0b0a9000 0x1000>, <0x0b009000 0x1000>; > - regulator; > }; > > saw3: regulator@b0b9000 { > compatible = "qcom,saw2"; > reg = <0x0b0b9000 0x1000>, <0x0b009000 0x1000>; > - regulator; > }; > > saw_l2: regulator@b012000 { > compatible = "qcom,saw2"; > reg = <0xb012000 0x1000>; > - regulator; > }; > > blsp1_uart1: serial@78af000 {
On 25.06.2023 22:25, Dmitry Baryshkov wrote: > The SAW2 device should describe the regulator constraints rather than > just declaring that it has the regulator. > > Drop the 'regulator' property. If/when CPU voltage scaling is > implemented for this platform, proper regulator node show be added > instead. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > arch/arm/boot/dts/qcom/qcom-msm8974.dtsi | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi > index aeca504918a0..dffab32c757d 100644 > --- a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi > @@ -416,7 +416,6 @@ saw3: power-controller@f90b9000 { > saw_l2: power-controller@f9012000 { > compatible = "qcom,saw2"; > reg = <0xf9012000 0x1000>; > - regulator; > }; > > acc0: power-manager@f9088000 {
On 25.06.2023 22:25, Dmitry Baryshkov wrote: > Declare CPU frequency-scaling properties. Each CPU has its own clock, > how however? > all CPUs have the same OPP table. Voltage scaling is not (yet) > enabled with this patch. It will be enabled later. Risky business. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 170 +++++++++++++++++++++++ > 1 file changed, 170 insertions(+) > > diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > index ac07170c702f..e4d2fd48d843 100644 > --- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > +++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > @@ -2,11 +2,13 @@ > /dts-v1/; > > #include <dt-bindings/clock/qcom,gcc-msm8960.h> > +#include <dt-bindings/clock/qcom,krait-cc.h> > #include <dt-bindings/clock/qcom,lcc-msm8960.h> > #include <dt-bindings/reset/qcom,gcc-msm8960.h> > #include <dt-bindings/clock/qcom,mmcc-msm8960.h> > #include <dt-bindings/clock/qcom,rpmcc.h> > #include <dt-bindings/soc/qcom,gsbi.h> > +#include <dt-bindings/soc/qcom,krait-l2-cache.h> > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > / { > @@ -45,6 +47,12 @@ CPU0: cpu@0 { > qcom,acc = <&acc0>; > qcom,saw = <&saw0>; > cpu-idle-states = <&CPU_SPC>; > + clocks = <&kraitcc KRAIT_CPU_0>; > + clock-names = "cpu"; > + clock-latency = <100000>; > + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; > + operating-points-v2 = <&cpu_opp_table>; > + #cooling-cells = <2>; > }; > > CPU1: cpu@1 { > @@ -56,6 +64,12 @@ CPU1: cpu@1 { > qcom,acc = <&acc1>; > qcom,saw = <&saw1>; > cpu-idle-states = <&CPU_SPC>; > + clocks = <&kraitcc KRAIT_CPU_1>; > + clock-names = "cpu"; > + clock-latency = <100000>; > + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; > + operating-points-v2 = <&cpu_opp_table>; > + #cooling-cells = <2>; > }; > > CPU2: cpu@2 { > @@ -67,6 +81,12 @@ CPU2: cpu@2 { > qcom,acc = <&acc2>; > qcom,saw = <&saw2>; > cpu-idle-states = <&CPU_SPC>; > + clocks = <&kraitcc KRAIT_CPU_2>; > + clock-names = "cpu"; > + clock-latency = <100000>; > + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; > + operating-points-v2 = <&cpu_opp_table>; > + #cooling-cells = <2>; > }; > > CPU3: cpu@3 { > @@ -78,6 +98,12 @@ CPU3: cpu@3 { > qcom,acc = <&acc3>; > qcom,saw = <&saw3>; > cpu-idle-states = <&CPU_SPC>; > + clocks = <&kraitcc KRAIT_CPU_3>; > + clock-names = "cpu"; > + clock-latency = <100000>; > + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; > + operating-points-v2 = <&cpu_opp_table>; > + #cooling-cells = <2>; > }; > > L2: l2-cache { > @@ -196,6 +222,121 @@ CPU_SPC: spc { > }; > }; > > + cpu_opp_table: opp-table-cpu { > + compatible = "operating-points-v2-krait-cpu"; > + nvmem-cells = <&speedbin_efuse>; > + > + /* > + * Voltage thresholds are <target min max> > + */ What voltage thresholds? > + opp-384000000 { > + opp-hz = /bits/ 64 <384000000>; > + opp-peak-kBps = <384000>; > + opp-supported-hw = <0x4007>; > + /* > + * higher latency as it requires switching between > + * clock sources > + */ > + clock-latency-ns = <244144>; > + }; > + > + opp-486000000 { > + opp-hz = /bits/ 64 <486000000>; > + opp-peak-kBps = <648000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-594000000 { > + opp-hz = /bits/ 64 <594000000>; > + opp-peak-kBps = <648000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-702000000 { > + opp-hz = /bits/ 64 <702000000>; > + opp-peak-kBps = <648000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-810000000 { > + opp-hz = /bits/ 64 <810000000>; > + opp-peak-kBps = <648000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-918000000 { > + opp-hz = /bits/ 64 <918000000>; > + opp-peak-kBps = <648000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-1026000000 { > + opp-hz = /bits/ 64 <1026000000>; > + opp-peak-kBps = <648000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-1134000000 { > + opp-hz = /bits/ 64 <1134000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-1242000000 { > + opp-hz = /bits/ 64 <1242000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-1350000000 { > + opp-hz = /bits/ 64 <1350000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-1458000000 { > + opp-hz = /bits/ 64 <1458000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x4007>; > + }; > + > + opp-1512000000 { > + opp-hz = /bits/ 64 <1512000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x4001>; > + }; > + > + opp-1566000000 { > + opp-hz = /bits/ 64 <1566000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x06>; > + }; > + > + opp-1674000000 { > + opp-hz = /bits/ 64 <1674000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x06>; > + }; > + > + opp-1728000000 { > + opp-hz = /bits/ 64 <1728000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x02>; > + }; > + > + opp-1782000000 { > + opp-hz = /bits/ 64 <1782000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x04>; > + }; > + > + opp-1890000000 { > + opp-hz = /bits/ 64 <1890000000>; > + opp-peak-kBps = <1134000>; > + opp-supported-hw = <0x04>; > + }; > + }; > + > memory@0 { > device_type = "memory"; > reg = <0x0 0x0>; > @@ -312,6 +453,32 @@ sleep_clk: sleep_clk { > }; > }; > > + kraitcc: clock-controller { > + compatible = "qcom,krait-cc-v1"; Are we sure we don't wanna rework this compatible? Check the comment in drivers/clk/qcom/krait-cc.c : krait_add_sec_mux() > + clocks = <&gcc PLL9>, /* hfpll0 */ > + <&gcc PLL10>, /* hfpll1 */ > + <&gcc PLL16>, /* hfpll2 */ > + <&gcc PLL17>, /* hfpll3 */ > + <&gcc PLL12>, /* hfpll_l2 */ > + <&acc0>, > + <&acc1>, > + <&acc2>, > + <&acc3>, > + <&l2cc>; > + clock-names = "hfpll0", > + "hfpll1", > + "hfpll2", > + "hfpll3", > + "hfpll_l2", > + "acpu0_aux", > + "acpu1_aux", > + "acpu2_aux", > + "acpu3_aux", > + "acpu_l2_aux"; > + #clock-cells = <1>; > + #interconnect-cells = <1>; > + }; > + > sfpb_mutex: hwmutex { > compatible = "qcom,sfpb-mutex"; > syscon = <&sfpb_wrapper_mutex 0x604 0x4>; > @@ -933,6 +1100,9 @@ qfprom: qfprom@700000 { > #address-cells = <1>; > #size-cells = <1>; > ranges; > + speedbin_efuse: speedbin@c0 { > + reg = <0x0c0 0x4>; > + }; Newline between properties and subnodes & between individual subnodes, please Konrad > tsens_calib: calib@404 { > reg = <0x404 0x10>; > };
On 25.06.2023 22:25, Dmitry Baryshkov wrote: > Add additional constraints to the CPUfreq-related regulators, it is > better be safe than sorry there. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- I'd say this and similar patches could go earlier in the series.. fwiw: Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > .../boot/dts/qcom/qcom-apq8064-asus-nexus7-flo.dts | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064-asus-nexus7-flo.dts b/arch/arm/boot/dts/qcom/qcom-apq8064-asus-nexus7-flo.dts > index c57c27cd8a20..9f5d72727356 100644 > --- a/arch/arm/boot/dts/qcom/qcom-apq8064-asus-nexus7-flo.dts > +++ b/arch/arm/boot/dts/qcom/qcom-apq8064-asus-nexus7-flo.dts > @@ -218,9 +218,9 @@ s1 { > bias-pull-down; > }; > > - /* msm otg HSUSB_VDDCX */ > + /* msm otg HSUSB_VDDCX and VDD_DIG */ > s3 { > - regulator-min-microvolt = <500000>; > + regulator-min-microvolt = <950000>; > regulator-max-microvolt = <1150000>; > qcom,switch-mode-frequency = <4800000>; > }; > @@ -301,6 +301,12 @@ l23 { > bias-pull-down; > }; > > + /* VDD_MEM */ > + l24 { > + regulator-min-microvolt = <1050000>; > + regulator-max-microvolt = <1150000>; > + }; > + > /* > * tabla2x-slim-CDC_VDDA_A_1P2V > * tabla2x-slim-VDDD_CDC_D > @@ -329,8 +335,12 @@ lvs6 { > /* > * mipi_dsi.1-dsi1_vddio > * pil_riva-pll_vdd > + * HFPLL regulator > */ > lvs7 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-boot-on; > bias-pull-down; > }; > };
On 25.06.2023 22:25, Dmitry Baryshkov wrote: > Add additional constraints to the CPUfreq-related regulators, it is > better be safe than sorry there. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- same comment as in p20 Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > .../arm/boot/dts/qcom/qcom-apq8064-ifc6410.dts | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom/qcom-apq8064-ifc6410.dts > index 96307550523a..ad3cd45362df 100644 > --- a/arch/arm/boot/dts/qcom/qcom-apq8064-ifc6410.dts > +++ b/arch/arm/boot/dts/qcom/qcom-apq8064-ifc6410.dts > @@ -215,8 +215,8 @@ s1 { > }; > > s3 { > - regulator-min-microvolt = <1000000>; > - regulator-max-microvolt = <1400000>; > + regulator-min-microvolt = <950000>; > + regulator-max-microvolt = <1150000>; > qcom,switch-mode-frequency = <4800000>; > }; > > @@ -262,6 +262,12 @@ l23 { > bias-pull-down; > }; > > + l24 { > + regulator-min-microvolt = <1050000>; > + regulator-max-microvolt = <1150000>; > + bias-pull-down; > + }; > + > lvs1 { > bias-pull-down; > }; > @@ -269,6 +275,14 @@ lvs1 { > lvs6 { > bias-pull-down; > }; > + > + /* HFPLL regulator */ > + lvs7 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-boot-on; > + regulator-always-on; > + }; > }; > }; >
On 26/06/2023 19:40, Konrad Dybcio wrote: > On 25.06.2023 22:25, Dmitry Baryshkov wrote: >> Declare CPU frequency-scaling properties. Each CPU has its own clock, >> how > however? yes > >> all CPUs have the same OPP table. Voltage scaling is not (yet) >> enabled with this patch. It will be enabled later. > Risky business. But it works :D > >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 170 +++++++++++++++++++++++ >> 1 file changed, 170 insertions(+) >> >> diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi >> index ac07170c702f..e4d2fd48d843 100644 >> --- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi >> +++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi >> @@ -2,11 +2,13 @@ >> /dts-v1/; >> >> #include <dt-bindings/clock/qcom,gcc-msm8960.h> >> +#include <dt-bindings/clock/qcom,krait-cc.h> >> #include <dt-bindings/clock/qcom,lcc-msm8960.h> >> #include <dt-bindings/reset/qcom,gcc-msm8960.h> >> #include <dt-bindings/clock/qcom,mmcc-msm8960.h> >> #include <dt-bindings/clock/qcom,rpmcc.h> >> #include <dt-bindings/soc/qcom,gsbi.h> >> +#include <dt-bindings/soc/qcom,krait-l2-cache.h> >> #include <dt-bindings/interrupt-controller/irq.h> >> #include <dt-bindings/interrupt-controller/arm-gic.h> >> / { >> @@ -45,6 +47,12 @@ CPU0: cpu@0 { >> qcom,acc = <&acc0>; >> qcom,saw = <&saw0>; >> cpu-idle-states = <&CPU_SPC>; >> + clocks = <&kraitcc KRAIT_CPU_0>; >> + clock-names = "cpu"; >> + clock-latency = <100000>; >> + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; >> + operating-points-v2 = <&cpu_opp_table>; >> + #cooling-cells = <2>; >> }; >> >> CPU1: cpu@1 { >> @@ -56,6 +64,12 @@ CPU1: cpu@1 { >> qcom,acc = <&acc1>; >> qcom,saw = <&saw1>; >> cpu-idle-states = <&CPU_SPC>; >> + clocks = <&kraitcc KRAIT_CPU_1>; >> + clock-names = "cpu"; >> + clock-latency = <100000>; >> + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; >> + operating-points-v2 = <&cpu_opp_table>; >> + #cooling-cells = <2>; >> }; >> >> CPU2: cpu@2 { >> @@ -67,6 +81,12 @@ CPU2: cpu@2 { >> qcom,acc = <&acc2>; >> qcom,saw = <&saw2>; >> cpu-idle-states = <&CPU_SPC>; >> + clocks = <&kraitcc KRAIT_CPU_2>; >> + clock-names = "cpu"; >> + clock-latency = <100000>; >> + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; >> + operating-points-v2 = <&cpu_opp_table>; >> + #cooling-cells = <2>; >> }; >> >> CPU3: cpu@3 { >> @@ -78,6 +98,12 @@ CPU3: cpu@3 { >> qcom,acc = <&acc3>; >> qcom,saw = <&saw3>; >> cpu-idle-states = <&CPU_SPC>; >> + clocks = <&kraitcc KRAIT_CPU_3>; >> + clock-names = "cpu"; >> + clock-latency = <100000>; >> + interconnects = <&L2 MASTER_KRAIT_L2 &L2 SLAVE_KRAIT_L2>; >> + operating-points-v2 = <&cpu_opp_table>; >> + #cooling-cells = <2>; >> }; >> >> L2: l2-cache { >> @@ -196,6 +222,121 @@ CPU_SPC: spc { >> }; >> }; >> >> + cpu_opp_table: opp-table-cpu { >> + compatible = "operating-points-v2-krait-cpu"; >> + nvmem-cells = <&speedbin_efuse>; >> + >> + /* >> + * Voltage thresholds are <target min max> >> + */ > What voltage thresholds? Ack, should be moved to the next patch. > >> + opp-384000000 { >> + opp-hz = /bits/ 64 <384000000>; >> + opp-peak-kBps = <384000>; >> + opp-supported-hw = <0x4007>; >> + /* >> + * higher latency as it requires switching between >> + * clock sources >> + */ >> + clock-latency-ns = <244144>; >> + }; >> + >> + opp-486000000 { >> + opp-hz = /bits/ 64 <486000000>; >> + opp-peak-kBps = <648000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-594000000 { >> + opp-hz = /bits/ 64 <594000000>; >> + opp-peak-kBps = <648000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-702000000 { >> + opp-hz = /bits/ 64 <702000000>; >> + opp-peak-kBps = <648000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-810000000 { >> + opp-hz = /bits/ 64 <810000000>; >> + opp-peak-kBps = <648000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-918000000 { >> + opp-hz = /bits/ 64 <918000000>; >> + opp-peak-kBps = <648000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-1026000000 { >> + opp-hz = /bits/ 64 <1026000000>; >> + opp-peak-kBps = <648000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-1134000000 { >> + opp-hz = /bits/ 64 <1134000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-1242000000 { >> + opp-hz = /bits/ 64 <1242000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-1350000000 { >> + opp-hz = /bits/ 64 <1350000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-1458000000 { >> + opp-hz = /bits/ 64 <1458000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x4007>; >> + }; >> + >> + opp-1512000000 { >> + opp-hz = /bits/ 64 <1512000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x4001>; >> + }; >> + >> + opp-1566000000 { >> + opp-hz = /bits/ 64 <1566000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x06>; >> + }; >> + >> + opp-1674000000 { >> + opp-hz = /bits/ 64 <1674000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x06>; >> + }; >> + >> + opp-1728000000 { >> + opp-hz = /bits/ 64 <1728000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x02>; >> + }; >> + >> + opp-1782000000 { >> + opp-hz = /bits/ 64 <1782000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x04>; >> + }; >> + >> + opp-1890000000 { >> + opp-hz = /bits/ 64 <1890000000>; >> + opp-peak-kBps = <1134000>; >> + opp-supported-hw = <0x04>; >> + }; >> + }; >> + >> memory@0 { >> device_type = "memory"; >> reg = <0x0 0x0>; >> @@ -312,6 +453,32 @@ sleep_clk: sleep_clk { >> }; >> }; >> >> + kraitcc: clock-controller { >> + compatible = "qcom,krait-cc-v1"; > Are we sure we don't wanna rework this compatible? Check the comment in > drivers/clk/qcom/krait-cc.c : krait_add_sec_mux() I remember that comment. I'd rather not introduce another compat string for such old hw. Would there be any direct benefits? > > >> + clocks = <&gcc PLL9>, /* hfpll0 */ >> + <&gcc PLL10>, /* hfpll1 */ >> + <&gcc PLL16>, /* hfpll2 */ >> + <&gcc PLL17>, /* hfpll3 */ >> + <&gcc PLL12>, /* hfpll_l2 */ >> + <&acc0>, >> + <&acc1>, >> + <&acc2>, >> + <&acc3>, >> + <&l2cc>; >> + clock-names = "hfpll0", >> + "hfpll1", >> + "hfpll2", >> + "hfpll3", >> + "hfpll_l2", >> + "acpu0_aux", >> + "acpu1_aux", >> + "acpu2_aux", >> + "acpu3_aux", >> + "acpu_l2_aux"; >> + #clock-cells = <1>; >> + #interconnect-cells = <1>; >> + }; >> + >> sfpb_mutex: hwmutex { >> compatible = "qcom,sfpb-mutex"; >> syscon = <&sfpb_wrapper_mutex 0x604 0x4>; >> @@ -933,6 +1100,9 @@ qfprom: qfprom@700000 { >> #address-cells = <1>; >> #size-cells = <1>; >> ranges; >> + speedbin_efuse: speedbin@c0 { >> + reg = <0x0c0 0x4>; >> + }; > Newline between properties and subnodes & between individual subnodes, > please ack. > > Konrad >> tsens_calib: calib@404 { >> reg = <0x404 0x10>; >> };
On 26.06.2023 21:49, Dmitry Baryshkov wrote: > On 26/06/2023 19:40, Konrad Dybcio wrote: >> On 25.06.2023 22:25, Dmitry Baryshkov wrote: >>> Declare CPU frequency-scaling properties. Each CPU has its own clock, >>> how >> however? > > yes > >> >>> all CPUs have the same OPP table. Voltage scaling is not (yet) >>> enabled with this patch. It will be enabled later. >> Risky business. > > But it works :D On your machine ;) [...] >>> + kraitcc: clock-controller { >>> + compatible = "qcom,krait-cc-v1"; >> Are we sure we don't wanna rework this compatible? Check the comment in >> drivers/clk/qcom/krait-cc.c : krait_add_sec_mux() > > I remember that comment. I'd rather not introduce another compat string for such old hw. Would there be any direct benefits? > I'd say that the one we have here never made much sense.. Perhaps (since nobody used it for 10 years) it would make sense to remodel it.. Konrad >> >> >>> + clocks = <&gcc PLL9>, /* hfpll0 */ >>> + <&gcc PLL10>, /* hfpll1 */ >>> + <&gcc PLL16>, /* hfpll2 */ >>> + <&gcc PLL17>, /* hfpll3 */ >>> + <&gcc PLL12>, /* hfpll_l2 */ >>> + <&acc0>, >>> + <&acc1>, >>> + <&acc2>, >>> + <&acc3>, >>> + <&l2cc>; >>> + clock-names = "hfpll0", >>> + "hfpll1", >>> + "hfpll2", >>> + "hfpll3", >>> + "hfpll_l2", >>> + "acpu0_aux", >>> + "acpu1_aux", >>> + "acpu2_aux", >>> + "acpu3_aux", >>> + "acpu_l2_aux"; >>> + #clock-cells = <1>; >>> + #interconnect-cells = <1>; >>> + }; >>> + >>> sfpb_mutex: hwmutex { >>> compatible = "qcom,sfpb-mutex"; >>> syscon = <&sfpb_wrapper_mutex 0x604 0x4>; >>> @@ -933,6 +1100,9 @@ qfprom: qfprom@700000 { >>> #address-cells = <1>; >>> #size-cells = <1>; >>> ranges; >>> + speedbin_efuse: speedbin@c0 { >>> + reg = <0x0c0 0x4>; >>> + }; >> Newline between properties and subnodes & between individual subnodes, >> please > > ack. > >> >> Konrad >>> tsens_calib: calib@404 { >>> reg = <0x404 0x10>; >>> }; >
On Tue, 27 Jun 2023 at 15:13, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 26.06.2023 21:49, Dmitry Baryshkov wrote: > > On 26/06/2023 19:40, Konrad Dybcio wrote: > >> On 25.06.2023 22:25, Dmitry Baryshkov wrote: > >>> Declare CPU frequency-scaling properties. Each CPU has its own clock, > >>> how > >> however? > > > > yes > > > >> > >>> all CPUs have the same OPP table. Voltage scaling is not (yet) > >>> enabled with this patch. It will be enabled later. > >> Risky business. > > > > But it works :D > On your machine ;) On two nexus-7 and one ifc6410. > > [...] > > >>> + kraitcc: clock-controller { > >>> + compatible = "qcom,krait-cc-v1"; > >> Are we sure we don't wanna rework this compatible? Check the comment in > >> drivers/clk/qcom/krait-cc.c : krait_add_sec_mux() > > > > I remember that comment. I'd rather not introduce another compat string for such old hw. Would there be any direct benefits? > > > I'd say that the one we have here never made much sense.. Perhaps (since > nobody used it for 10 years) it would make sense to remodel it.. Well we have the bindings for this driver. And also it was used by the OpenWRT people, IIRC. Thus I don't feel comfortable with throwing out old compat strings. > > Konrad > >> > >> > >>> + clocks = <&gcc PLL9>, /* hfpll0 */ > >>> + <&gcc PLL10>, /* hfpll1 */ > >>> + <&gcc PLL16>, /* hfpll2 */ > >>> + <&gcc PLL17>, /* hfpll3 */ > >>> + <&gcc PLL12>, /* hfpll_l2 */ > >>> + <&acc0>, > >>> + <&acc1>, > >>> + <&acc2>, > >>> + <&acc3>, > >>> + <&l2cc>; > >>> + clock-names = "hfpll0", > >>> + "hfpll1", > >>> + "hfpll2", > >>> + "hfpll3", > >>> + "hfpll_l2", > >>> + "acpu0_aux", > >>> + "acpu1_aux", > >>> + "acpu2_aux", > >>> + "acpu3_aux", > >>> + "acpu_l2_aux"; > >>> + #clock-cells = <1>; > >>> + #interconnect-cells = <1>; > >>> + }; > >>> + > >>> sfpb_mutex: hwmutex { > >>> compatible = "qcom,sfpb-mutex"; > >>> syscon = <&sfpb_wrapper_mutex 0x604 0x4>; > >>> @@ -933,6 +1100,9 @@ qfprom: qfprom@700000 { > >>> #address-cells = <1>; > >>> #size-cells = <1>; > >>> ranges; > >>> + speedbin_efuse: speedbin@c0 { > >>> + reg = <0x0c0 0x4>; > >>> + }; > >> Newline between properties and subnodes & between individual subnodes, > >> please > > > > ack. > > > >> > >> Konrad > >>> tsens_calib: calib@404 { > >>> reg = <0x404 0x10>; > >>> }; > >
On 27.06.2023 16:11, Dmitry Baryshkov wrote: > On Tue, 27 Jun 2023 at 15:13, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> On 26.06.2023 21:49, Dmitry Baryshkov wrote: >>> On 26/06/2023 19:40, Konrad Dybcio wrote: >>>> On 25.06.2023 22:25, Dmitry Baryshkov wrote: >>>>> Declare CPU frequency-scaling properties. Each CPU has its own clock, >>>>> how >>>> however? >>> >>> yes >>> >>>> >>>>> all CPUs have the same OPP table. Voltage scaling is not (yet) >>>>> enabled with this patch. It will be enabled later. >>>> Risky business. >>> >>> But it works :D >> On your machine ;) > > On two nexus-7 and one ifc6410. > >> >> [...] >> >>>>> + kraitcc: clock-controller { >>>>> + compatible = "qcom,krait-cc-v1"; >>>> Are we sure we don't wanna rework this compatible? Check the comment in >>>> drivers/clk/qcom/krait-cc.c : krait_add_sec_mux() >>> >>> I remember that comment. I'd rather not introduce another compat string for such old hw. Would there be any direct benefits? >>> >> I'd say that the one we have here never made much sense.. Perhaps (since >> nobody used it for 10 years) it would make sense to remodel it.. > > Well we have the bindings for this driver. And also it was used by the > OpenWRT people, IIRC. > Thus I don't feel comfortable with throwing out old compat strings. Oh, OK Konrad > >> >> Konrad >>>> >>>> >>>>> + clocks = <&gcc PLL9>, /* hfpll0 */ >>>>> + <&gcc PLL10>, /* hfpll1 */ >>>>> + <&gcc PLL16>, /* hfpll2 */ >>>>> + <&gcc PLL17>, /* hfpll3 */ >>>>> + <&gcc PLL12>, /* hfpll_l2 */ >>>>> + <&acc0>, >>>>> + <&acc1>, >>>>> + <&acc2>, >>>>> + <&acc3>, >>>>> + <&l2cc>; >>>>> + clock-names = "hfpll0", >>>>> + "hfpll1", >>>>> + "hfpll2", >>>>> + "hfpll3", >>>>> + "hfpll_l2", >>>>> + "acpu0_aux", >>>>> + "acpu1_aux", >>>>> + "acpu2_aux", >>>>> + "acpu3_aux", >>>>> + "acpu_l2_aux"; >>>>> + #clock-cells = <1>; >>>>> + #interconnect-cells = <1>; >>>>> + }; >>>>> + >>>>> sfpb_mutex: hwmutex { >>>>> compatible = "qcom,sfpb-mutex"; >>>>> syscon = <&sfpb_wrapper_mutex 0x604 0x4>; >>>>> @@ -933,6 +1100,9 @@ qfprom: qfprom@700000 { >>>>> #address-cells = <1>; >>>>> #size-cells = <1>; >>>>> ranges; >>>>> + speedbin_efuse: speedbin@c0 { >>>>> + reg = <0x0c0 0x4>; >>>>> + }; >>>> Newline between properties and subnodes & between individual subnodes, >>>> please >>> >>> ack. >>> >>>> >>>> Konrad >>>>> tsens_calib: calib@404 { >>>>> reg = <0x404 0x10>; >>>>> }; >>> > > >
On Sun, 25 Jun 2023 23:25:22 +0300, Dmitry Baryshkov wrote: > Exted the opp-v2-kryo-cpu.yaml to support defining OPP tables for the > previous generation of Qualcomm CPUs, 32-bit Krait-based platforms. > > It makes no sense to use 'operating-points-v2-kryo-cpu' compatibility > node for the Krait cores. Add support for the Krait-specific > 'operating-points-v2-krait-cpu' compatibility string and the relevant > opp-microvolt subclasses properties. > > The listed opp-supported-hw values are applicable only to msm8996 / > msm8996pro platforms. Remove the enum as other platforms will use other > bit values. It makes little sense to list all possible values for all > the platforms here. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > Acked-by: Rob Herring <robh@kernel.org>
On Sun, 25 Jun 2023 23:25:25 +0300, Dmitry Baryshkov wrote: > Define bindings for the Qualcomm Krait CPU and L2 clock controller. This > device is used on old Qualcomm SoCs (APQ8064, MSM8960) and supports up > to 4 core clocks and a separate L2 clock. Furthermore, L2 clock is > represented as the interconnect to facilitate L2 frequency scaling > together with scaling the CPU frequencies. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > include/dt-bindings/clock/qcom,krait-cc.h | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > create mode 100644 include/dt-bindings/clock/qcom,krait-cc.h > Acked-by: Rob Herring <robh@kernel.org>
On 26/06/2023 14:50, Konrad Dybcio wrote: > On 25.06.2023 22:25, Dmitry Baryshkov wrote: >> Scaling the frequencies on some of Qualcomm Krait platforms (e.g. >> APQ8064) also requires scaling of the L2 cache frequency. As the >> l2-cache device node is places under /cpus/ path, it is not created by >> default by the OF code. Create corresponding device here. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- > I think a generic solution (i.e. for each cpu node call > of_platform_populate in drivers/of/platform.c : > of_platform_default_populate_init) could be beneficial After giving it a lot of thought, I'm not brave enough to register all CPU-like devices (especially since some of them are registered by other means). So let's keep it this way, unless we see a bigger demand of populating cache devices.
Implement CPUFreq support for one of the oldest supported Qualcomm platforms, APQ8064. Each core has independent power and frequency control. Additionally the L2 cache is scaled to follow the CPU frequencies (failure to do so results in strange semi-random crashes). Core voltage is controlled through the SAW2 devices, one for each core. The L2 has two regulators, vdd-mem and vdd-dig. Changes since v1: - Added separate Krait L2 cache device driver - Moved vdd-mem and vdd-dig scaling to the L2 cache device (Christian, Stephen Gerhold) - Fixed the 'INTERCONNECT' in the guarding define for krait-cc bindings (Stephen Boyd) - Made SAW2's regulator property -> node handling clear (Krzysztof) - Dropped the 'regulator' property from all SAW2 devices. Dmitry Baryshkov (26): dt-bindings: opp: opp-v2-kryo-cpu: support Qualcomm Krait SoCs dt-bindings: soc: qcom: merge qcom,saw2.txt into qcom,spm.yaml dt-bindings: soc: qcom: qcom,saw2: define optional regulator node dt-bindings: clock: qcom,krait-cc: Krait core clock controller dt-bindings: cache: describe L2 cache on Qualcomm Krait platforms interconnect: icc-clk: add support for scaling using OPP clk: qcom: krait-cc: rewrite driver to use clk_hw instead of clk soc: qcom: spm: add support for voltage regulator cpufreq: qcom-nvmem: create L2 cache device cpufreq: qcom-nvmem: also accept operating-points-v2-krait-cpu cpufreq: qcom-nvmem: drop pvs_ver for format a fuses cpufreq: qcom-nvmem: provide separate configuration data for apq8064 soc: qcom: Add driver for Qualcomm Krait L2 cache scaling ARM: dts: qcom: apq8064: rename SAW nodes to power-manager ARM: dts: qcom: apq8064: declare SAW2 regulators ARM: dts: qcom: apq8064: add L2 cache scaling ARM: dts: qcom: apq8064: add simple CPUFreq support ARM: dts: qcom: apq8064: provide voltage scaling tables ARM: dts: qcom: apq8064: enable passive CPU cooling ARM: dts: qcom: apq8064-asus-nexus7-flo: constraint cpufreq regulators ARM: dts: qcom: apq8064-ifc6410: constraint cpufreq regulators ARM: dts: qcom: msm8960: declare SAW2 regulators ARM: dts: qcom: apq8084: drop 'regulator' property from SAW2 device ARM: dts: qcom: msm8974: drop 'regulator' property from SAW2 device ARM: dts: qcom: ipq4019: drop 'regulator' property from SAW2 devices ARM: dts: qcom: ipq8064: drop 'regulator' property from SAW2 devices .../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -- .../bindings/cache/qcom,krait-l2-cache.yaml | 75 ++ .../bindings/opp/opp-v2-kryo-cpu.yaml | 12 +- .../qcom/{qcom,spm.yaml => qcom,saw2.yaml} | 39 +- .../dts/qcom/qcom-apq8064-asus-nexus7-flo.dts | 14 +- .../boot/dts/qcom/qcom-apq8064-ifc6410.dts | 18 +- arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 671 +++++++++++++++++- arch/arm/boot/dts/qcom/qcom-apq8084.dtsi | 1 - arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi | 5 - arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 2 - arch/arm/boot/dts/qcom/qcom-msm8960.dtsi | 12 +- arch/arm/boot/dts/qcom/qcom-msm8974.dtsi | 1 - drivers/clk/qcom/krait-cc.c | 141 ++-- drivers/cpufreq/qcom-cpufreq-nvmem.c | 76 +- drivers/interconnect/icc-clk.c | 13 +- drivers/soc/qcom/Kconfig | 9 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/krait-l2-cache.c | 190 +++++ drivers/soc/qcom/spm.c | 205 +++++- include/dt-bindings/clock/qcom,krait-cc.h | 17 + include/dt-bindings/soc/qcom,krait-l2-cache.h | 12 + include/linux/interconnect-clk.h | 1 + include/soc/qcom/spm.h | 9 + 23 files changed, 1403 insertions(+), 179 deletions(-) delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt create mode 100644 Documentation/devicetree/bindings/cache/qcom,krait-l2-cache.yaml rename Documentation/devicetree/bindings/soc/qcom/{qcom,spm.yaml => qcom,saw2.yaml} (57%) create mode 100644 drivers/soc/qcom/krait-l2-cache.c create mode 100644 include/dt-bindings/clock/qcom,krait-cc.h create mode 100644 include/dt-bindings/soc/qcom,krait-l2-cache.h