* [PATCH v5 01/10] dt-bindings: soc: qcom: merge qcom,saw2.txt into qcom,spm.yaml
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
@ 2023-12-07 13:06 ` Dmitry Baryshkov
2023-12-07 13:06 ` [PATCH v5 02/10] dt-bindings: soc: qcom: qcom,saw2: define optional regulator node Dmitry Baryshkov
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:06 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree, Rob Herring
The Qualcomm SPM / SAW2 device is described in two bindigns files:
arm/msm/qcom,saw2.txt and soc/qcom/qcom,spm.yaml. Merge the former into
the latter, adding detailed device node description. While we are at it,
also rename qcom,spm.yaml to qcom,saw2.yaml to follow the actual
compatible used for these devices.
The regulator property is retained as is. It will be changed in the
later patches.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
.../devicetree/bindings/arm/msm/qcom,saw2.txt | 58 -------------------
.../qcom/{qcom,spm.yaml => qcom,saw2.yaml} | 26 +++++++--
2 files changed, 20 insertions(+), 64 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
rename Documentation/devicetree/bindings/soc/qcom/{qcom,spm.yaml => qcom,saw2.yaml} (64%)
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
deleted file mode 100644
index c0e3c3a42bea..000000000000
--- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
+++ /dev/null
@@ -1,58 +0,0 @@
-SPM AVS Wrapper 2 (SAW2)
-
-The SAW2 is a wrapper around the Subsystem Power Manager (SPM) and the
-Adaptive Voltage Scaling (AVS) hardware. The SPM is a programmable
-power-controller that transitions a piece of hardware (like a processor or
-subsystem) into and out of low power modes via a direct connection to
-the PMIC. It can also be wired up to interact with other processors in the
-system, notifying them when a low power state is entered or exited.
-
-Multiple revisions of the SAW hardware are supported using these Device Nodes.
-SAW2 revisions differ in the register offset and configuration data. Also, the
-same revision of the SAW in different SoCs may have different configuration
-data due the differences in hardware capabilities. Hence the SoC name, the
-version of the SAW hardware in that SoC and the distinction between cpu (big
-or Little) or cache, may be needed to uniquely identify the SAW register
-configuration and initialization data. The compatible string is used to
-indicate this parameter.
-
-PROPERTIES
-
-- compatible:
- Usage: required
- Value type: <string>
- Definition: Must have
- "qcom,saw2"
- A more specific value could be one of:
- "qcom,apq8064-saw2-v1.1-cpu"
- "qcom,msm8226-saw2-v2.1-cpu"
- "qcom,msm8974-saw2-v2.1-cpu"
- "qcom,apq8084-saw2-v2.1-cpu"
-
-- reg:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: the first element specifies the base address and size of
- the register region. An optional second element specifies
- the base address and size of the alias register region.
-
-- regulator:
- Usage: optional
- Value type: boolean
- Definition: Indicates that this SPM device acts as a regulator device
- device for the core (CPU or Cache) the SPM is attached
- to.
-
-Example 1:
-
- power-controller@2099000 {
- compatible = "qcom,saw2";
- reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
- regulator;
- };
-
-Example 2:
- saw0: power-controller@f9089000 {
- compatible = "qcom,apq8084-saw2-v2.1-cpu", "qcom,saw2";
- reg = <0xf9089000 0x1000>, <0xf9009000 0x1000>;
- };
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
similarity index 64%
rename from Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
rename to Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
index 20c8cd38ff0d..84b3f01d590c 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,spm.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
@@ -1,18 +1,25 @@
# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
%YAML 1.2
---
-$id: http://devicetree.org/schemas/soc/qcom/qcom,spm.yaml#
+$id: http://devicetree.org/schemas/soc/qcom/qcom,saw2.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Qualcomm Subsystem Power Manager
+title: Qualcomm Subsystem Power Manager / SPM AVS Wrapper 2 (SAW2)
maintainers:
- Andy Gross <agross@kernel.org>
- Bjorn Andersson <bjorn.andersson@linaro.org>
description: |
- This binding describes the Qualcomm Subsystem Power Manager, used to control
- the peripheral logic surrounding the application cores in Qualcomm platforms.
+ The Qualcomm Subsystem Power Manager is used to control the peripheral logic
+ surrounding the application cores in Qualcomm platforms.
+
+ The SAW2 is a wrapper around the Subsystem Power Manager (SPM) and the
+ Adaptive Voltage Scaling (AVS) hardware. The SPM is a programmable
+ power-controller that transitions a piece of hardware (like a processor or
+ subsystem) into and out of low power modes via a direct connection to
+ the PMIC. It can also be wired up to interact with other processors in the
+ system, notifying them when a low power state is entered or exited.
properties:
compatible:
@@ -34,8 +41,15 @@ properties:
- const: qcom,saw2
reg:
- description: Base address and size of the SPM register region
- maxItems: 1
+ items:
+ - description: Base address and size of the SPM register region
+ - description: Base address and size of the alias register region
+ minItems: 1
+
+ regulator:
+ type: boolean
+ description: Indicates that this SPM device acts as a regulator device
+ device for the core (CPU or Cache) the SPM is attached to.
required:
- compatible
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 02/10] dt-bindings: soc: qcom: qcom,saw2: define optional regulator node
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
2023-12-07 13:06 ` [PATCH v5 01/10] dt-bindings: soc: qcom: merge qcom,saw2.txt into qcom,spm.yaml Dmitry Baryshkov
@ 2023-12-07 13:06 ` Dmitry Baryshkov
2023-12-07 13:06 ` [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator Dmitry Baryshkov
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:06 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree, Rob Herring
The SAW2 device can optionally provide a voltage regulator supplying the
CPU core, cluster or L2 cache. Change the boolean 'regulator' property
into a proper regulator description. This breaks schema compatibility
for the sake of properly describing the regulator.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
.../devicetree/bindings/soc/qcom/qcom,saw2.yaml | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
index 84b3f01d590c..a2d871ba8c45 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
@@ -47,7 +47,7 @@ properties:
minItems: 1
regulator:
- type: boolean
+ $ref: /schemas/regulator/regulator.yaml#
description: Indicates that this SPM device acts as a regulator device
device for the core (CPU or Cache) the SPM is attached to.
@@ -96,4 +96,17 @@ examples:
reg = <0x17912000 0x1000>;
};
+ - |
+ /*
+ * Example 3: SAW2 with the bundled regulator definition.
+ */
+ power-manager@2089000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
+ reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
+
+ regulator {
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1300000>;
+ };
+ };
...
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
2023-12-07 13:06 ` [PATCH v5 01/10] dt-bindings: soc: qcom: merge qcom,saw2.txt into qcom,spm.yaml Dmitry Baryshkov
2023-12-07 13:06 ` [PATCH v5 02/10] dt-bindings: soc: qcom: qcom,saw2: define optional regulator node Dmitry Baryshkov
@ 2023-12-07 13:06 ` Dmitry Baryshkov
2023-12-07 17:12 ` Stephan Gerhold
2023-12-07 13:06 ` [PATCH v5 04/10] ARM: dts: qcom: apq8064: rename SAW nodes to power-manager Dmitry Baryshkov
` (6 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:06 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
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 | 221 ++++++++++++++++++++++++++++++++++++++++-
include/soc/qcom/spm.h | 9 ++
2 files changed, 225 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
index 2f0b1bfe7658..595e2afb2141 100644
--- a/drivers/soc/qcom/spm.c
+++ b/drivers/soc/qcom/spm.c
@@ -6,20 +6,40 @@
* SAW power controller driver
*/
-#include <linux/kernel.h>
+#include <linux/bitfield.h>
+#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/linear_range.h>
#include <linux/module.h>
-#include <linux/slab.h>
#include <linux/of.h>
-#include <linux/err.h>
#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/smp.h>
+
+#include <linux/regulator/driver.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)
+/* These registers might be specific to SPM 1.1 */
+#define SPM_VCTL_VLVL GENMASK(7, 0)
+#define SPM_PMIC_DATA_0_VLVL GENMASK(7, 0)
+#define SPM_PMIC_DATA_1_MIN_VSEL GENMASK(5, 0)
+#define SPM_PMIC_DATA_1_MAX_VSEL GENMASK(21, 16)
+
+#define SPM_1_1_AVS_CTL_AVS_ENABLED BIT(27)
+#define SPM_AVS_CTL_MAX_VLVL GENMASK(22, 17)
+#define SPM_AVS_CTL_MIN_VLVL GENMASK(15, 10)
+
enum spm_reg {
SPM_REG_CFG,
SPM_REG_SPM_CTL,
@@ -29,10 +49,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,
};
@@ -169,6 +191,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,
@@ -176,7 +202,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,
@@ -187,6 +218,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,
@@ -238,6 +273,181 @@ 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 vctl, data0, data1, avs_ctl, sts;
+ unsigned int vlevel, volt_sel;
+ bool avs_enabled;
+
+ volt_sel = drv->volt_sel;
+ vlevel = volt_sel | 0x80; /* band */
+
+ 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, SPM_VCTL_VLVL, vlevel);
+ data0 = FIELD_SET(data0, SPM_PMIC_DATA_0_VLVL, vlevel);
+ data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MIN_VSEL, volt_sel);
+ data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MAX_VSEL, volt_sel);
+
+ 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)) {
+ dev_err_ratelimited(drv->dev, "timeout setting the voltage (%x %x)!\n", sts, vlevel);
+ goto enable_avs;
+ }
+
+ if (avs_enabled) {
+ unsigned int max_avs = volt_sel;
+ unsigned int min_avs = max(max_avs, 4U) - 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;
+
+ for_each_possible_cpu(cpu) {
+ struct device_node *cpu_node, *saw_node;
+
+ 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);
+ 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;
+
+ 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 },
@@ -288,6 +498,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.. */
@@ -315,7 +526,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,
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator
2023-12-07 13:06 ` [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator Dmitry Baryshkov
@ 2023-12-07 17:12 ` Stephan Gerhold
2023-12-07 20:33 ` Dmitry Baryshkov
0 siblings, 1 reply; 15+ messages in thread
From: Stephan Gerhold @ 2023-12-07 17:12 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, linux-arm-msm, devicetree
On Thu, Dec 07, 2023 at 04:06:56PM +0300, 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>
There is some overlap here with the spmi_saw_set_vdd() functionality in
qcom_spmi-regulator.c, at least for the SoCs with SPMI PMICs (e.g.
MSM8974 etc). You don't add support for these in this patch series yet
but I think it would be good to clarify how we would expect to handle
those. In other words:
- Would we handle them in qcom_spmi-regulator.c and keep the code in
the spm.c driver only for the non-SPMI platforms?
- Should we add this in a SSBI regulator driver instead for consistency?
- Or should we move the existing functionality in qcom_spmi-regulator.c
to here?
> ---
> drivers/soc/qcom/spm.c | 221 ++++++++++++++++++++++++++++++++++++++++-
> include/soc/qcom/spm.h | 9 ++
> 2 files changed, 225 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
> index 2f0b1bfe7658..595e2afb2141 100644
> --- a/drivers/soc/qcom/spm.c
> +++ b/drivers/soc/qcom/spm.c
> [...]
> @@ -238,6 +273,181 @@ 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 vctl, data0, data1, avs_ctl, sts;
> + unsigned int vlevel, volt_sel;
> + bool avs_enabled;
> +
> + volt_sel = drv->volt_sel;
> + vlevel = volt_sel | 0x80; /* band */
> +
> + 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, SPM_VCTL_VLVL, vlevel);
> + data0 = FIELD_SET(data0, SPM_PMIC_DATA_0_VLVL, vlevel);
> + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MIN_VSEL, volt_sel);
> + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MAX_VSEL, volt_sel);
> +
> + 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)) {
> + dev_err_ratelimited(drv->dev, "timeout setting the voltage (%x %x)!\n", sts, vlevel);
> + goto enable_avs;
> + }
> +
> + if (avs_enabled) {
> + unsigned int max_avs = volt_sel;
> + unsigned int min_avs = max(max_avs, 4U) - 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;
> +
> + for_each_possible_cpu(cpu) {
> + struct device_node *cpu_node, *saw_node;
> +
> + 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);
> + 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 */
Is this necessary? I would kind of expect that it's only important that
this doesn't happen in parallel on multiple CPUs. The lock in the
regulator core should already ensure that, though. It's somewhat
expensive to schedule on other cores, especially if they are currently
idle and power collapsed.
I don't have too much experience with these really old platforms but at
least on MSM8916 I can't think of any reason that would make the CPU0
more special for the L2 cache than the others (MSM8916 has the regulator
stuff only in the L2 SAW since there is just one CPU power rail).
Thanks,
Stephan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator
2023-12-07 17:12 ` Stephan Gerhold
@ 2023-12-07 20:33 ` Dmitry Baryshkov
2023-12-07 21:34 ` Stephan Gerhold
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 20:33 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, linux-arm-msm, devicetree
On Thu, 7 Dec 2023 at 19:12, Stephan Gerhold <stephan@gerhold.net> wrote:
>
> On Thu, Dec 07, 2023 at 04:06:56PM +0300, 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>
>
> There is some overlap here with the spmi_saw_set_vdd() functionality in
> qcom_spmi-regulator.c, at least for the SoCs with SPMI PMICs (e.g.
> MSM8974 etc). You don't add support for these in this patch series yet
> but I think it would be good to clarify how we would expect to handle
> those. In other words:
>
> - Would we handle them in qcom_spmi-regulator.c and keep the code in
> the spm.c driver only for the non-SPMI platforms?
>
> - Should we add this in a SSBI regulator driver instead for consistency?
>
> - Or should we move the existing functionality in qcom_spmi-regulator.c
> to here?
The spmi_saw_set_vdd() is a definite hack and ideally it should be
dropped. It is not possible, though, existing msm8996 DT uses that
spmi/saw regulator to power on the CPU cores. We have to remain
compatible with that hack.
But my intent is to have all other platforms use the spm.c (and
migrate msm8996 at some point too).
>
> > ---
> > drivers/soc/qcom/spm.c | 221 ++++++++++++++++++++++++++++++++++++++++-
> > include/soc/qcom/spm.h | 9 ++
> > 2 files changed, 225 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
> > index 2f0b1bfe7658..595e2afb2141 100644
> > --- a/drivers/soc/qcom/spm.c
> > +++ b/drivers/soc/qcom/spm.c
> > [...]
> > @@ -238,6 +273,181 @@ 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 vctl, data0, data1, avs_ctl, sts;
> > + unsigned int vlevel, volt_sel;
> > + bool avs_enabled;
> > +
> > + volt_sel = drv->volt_sel;
> > + vlevel = volt_sel | 0x80; /* band */
> > +
> > + 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, SPM_VCTL_VLVL, vlevel);
> > + data0 = FIELD_SET(data0, SPM_PMIC_DATA_0_VLVL, vlevel);
> > + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MIN_VSEL, volt_sel);
> > + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MAX_VSEL, volt_sel);
> > +
> > + 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)) {
> > + dev_err_ratelimited(drv->dev, "timeout setting the voltage (%x %x)!\n", sts, vlevel);
> > + goto enable_avs;
> > + }
> > +
> > + if (avs_enabled) {
> > + unsigned int max_avs = volt_sel;
> > + unsigned int min_avs = max(max_avs, 4U) - 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;
> > +
> > + for_each_possible_cpu(cpu) {
> > + struct device_node *cpu_node, *saw_node;
> > +
> > + 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);
> > + 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 */
>
> Is this necessary? I would kind of expect that it's only important that
> this doesn't happen in parallel on multiple CPUs. The lock in the
> regulator core should already ensure that, though. It's somewhat
> expensive to schedule on other cores, especially if they are currently
> idle and power collapsed.
If I understand correctly, it is the other way around. From the msm
kernels I see that CPU SPM calls are scheduled to be executed only on
the corresponding CPU/core. For L2 we didn't have the CPU, so to keep
the same code path I selected for them to be executed on CPU0. At this
point I'm not even sure if this cpu0 comes from the downstream tree or
not.
>
> I don't have too much experience with these really old platforms but at
> least on MSM8916 I can't think of any reason that would make the CPU0
> more special for the L2 cache than the others (MSM8916 has the regulator
> stuff only in the L2 SAW since there is just one CPU power rail).
>
> Thanks,
> Stephan
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator
2023-12-07 20:33 ` Dmitry Baryshkov
@ 2023-12-07 21:34 ` Stephan Gerhold
2023-12-08 0:25 ` Dmitry Baryshkov
0 siblings, 1 reply; 15+ messages in thread
From: Stephan Gerhold @ 2023-12-07 21:34 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, linux-arm-msm, devicetree
On Thu, Dec 07, 2023 at 10:33:31PM +0200, Dmitry Baryshkov wrote:
> On Thu, 7 Dec 2023 at 19:12, Stephan Gerhold <stephan@gerhold.net> wrote:
> > On Thu, Dec 07, 2023 at 04:06:56PM +0300, 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>
> >
> > There is some overlap here with the spmi_saw_set_vdd() functionality in
> > qcom_spmi-regulator.c, at least for the SoCs with SPMI PMICs (e.g.
> > MSM8974 etc). You don't add support for these in this patch series yet
> > but I think it would be good to clarify how we would expect to handle
> > those. In other words:
> >
> > - Would we handle them in qcom_spmi-regulator.c and keep the code in
> > the spm.c driver only for the non-SPMI platforms?
> >
> > - Should we add this in a SSBI regulator driver instead for consistency?
> >
> > - Or should we move the existing functionality in qcom_spmi-regulator.c
> > to here?
>
> The spmi_saw_set_vdd() is a definite hack and ideally it should be
> dropped. It is not possible, though, existing msm8996 DT uses that
> spmi/saw regulator to power on the CPU cores. We have to remain
> compatible with that hack.
> But my intent is to have all other platforms use the spm.c (and
> migrate msm8996 at some point too).
Thanks, sounds reasonable.
>
> >
> > > ---
> > > drivers/soc/qcom/spm.c | 221 ++++++++++++++++++++++++++++++++++++++++-
> > > include/soc/qcom/spm.h | 9 ++
> > > 2 files changed, 225 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
> > > index 2f0b1bfe7658..595e2afb2141 100644
> > > --- a/drivers/soc/qcom/spm.c
> > > +++ b/drivers/soc/qcom/spm.c
> > > [...]
> > > @@ -238,6 +273,181 @@ 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 vctl, data0, data1, avs_ctl, sts;
> > > + unsigned int vlevel, volt_sel;
> > > + bool avs_enabled;
> > > +
> > > + volt_sel = drv->volt_sel;
> > > + vlevel = volt_sel | 0x80; /* band */
> > > +
> > > + 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, SPM_VCTL_VLVL, vlevel);
> > > + data0 = FIELD_SET(data0, SPM_PMIC_DATA_0_VLVL, vlevel);
> > > + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MIN_VSEL, volt_sel);
> > > + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MAX_VSEL, volt_sel);
> > > +
> > > + 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)) {
> > > + dev_err_ratelimited(drv->dev, "timeout setting the voltage (%x %x)!\n", sts, vlevel);
> > > + goto enable_avs;
> > > + }
> > > +
> > > + if (avs_enabled) {
> > > + unsigned int max_avs = volt_sel;
> > > + unsigned int min_avs = max(max_avs, 4U) - 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;
> > > +
> > > + for_each_possible_cpu(cpu) {
> > > + struct device_node *cpu_node, *saw_node;
> > > +
> > > + 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);
> > > + 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 */
> >
> > Is this necessary? I would kind of expect that it's only important that
> > this doesn't happen in parallel on multiple CPUs. The lock in the
> > regulator core should already ensure that, though. It's somewhat
> > expensive to schedule on other cores, especially if they are currently
> > idle and power collapsed.
>
> If I understand correctly, it is the other way around. From the msm
> kernels I see that CPU SPM calls are scheduled to be executed only on
> the corresponding CPU/core. For L2 we didn't have the CPU, so to keep
> the same code path I selected for them to be executed on CPU0. At this
> point I'm not even sure if this cpu0 comes from the downstream tree or
> not.
>
Hm, does 8064/8960 set any voltages through the L2 SPM at all? From a
quick look I see only the core SPMs being used as regulators, the L2 SPM
seems to be only used for idling.
I found a msm_spm_apcs_set_vdd() [1] but:
- That one skips the smp_call stuff and calls msm_spm_drv_set_vdd()
directly. In other words, there is no scheduling to CPU0.
- It seems to be only used in krait-regulator which is I think
8974-specific?
If you don't need the regulator on the L2 SPM you could just put an
error here rather than the CPU0 fallback. Or alternatively I'd try to
avoid the smp_call_function_single() for L2 since I don't see why those
should need to be called on a specific CPU.
Thanks,
Stephan
[1]: https://git.codelinaro.org/clo/la/kernel/msm/-/blob/LA.AF.1.2.1-08410-8064.0/arch/arm/mach-msm/spm_devices.c#L257
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator
2023-12-07 21:34 ` Stephan Gerhold
@ 2023-12-08 0:25 ` Dmitry Baryshkov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-08 0:25 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, linux-arm-msm, devicetree
On Thu, 7 Dec 2023 at 23:34, Stephan Gerhold <stephan@gerhold.net> wrote:
>
> On Thu, Dec 07, 2023 at 10:33:31PM +0200, Dmitry Baryshkov wrote:
> > On Thu, 7 Dec 2023 at 19:12, Stephan Gerhold <stephan@gerhold.net> wrote:
> > > On Thu, Dec 07, 2023 at 04:06:56PM +0300, 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>
> > >
> > > There is some overlap here with the spmi_saw_set_vdd() functionality in
> > > qcom_spmi-regulator.c, at least for the SoCs with SPMI PMICs (e.g.
> > > MSM8974 etc). You don't add support for these in this patch series yet
> > > but I think it would be good to clarify how we would expect to handle
> > > those. In other words:
> > >
> > > - Would we handle them in qcom_spmi-regulator.c and keep the code in
> > > the spm.c driver only for the non-SPMI platforms?
> > >
> > > - Should we add this in a SSBI regulator driver instead for consistency?
> > >
> > > - Or should we move the existing functionality in qcom_spmi-regulator.c
> > > to here?
> >
> > The spmi_saw_set_vdd() is a definite hack and ideally it should be
> > dropped. It is not possible, though, existing msm8996 DT uses that
> > spmi/saw regulator to power on the CPU cores. We have to remain
> > compatible with that hack.
> > But my intent is to have all other platforms use the spm.c (and
> > migrate msm8996 at some point too).
>
> Thanks, sounds reasonable.
>
> >
> > >
> > > > ---
> > > > drivers/soc/qcom/spm.c | 221 ++++++++++++++++++++++++++++++++++++++++-
> > > > include/soc/qcom/spm.h | 9 ++
> > > > 2 files changed, 225 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
> > > > index 2f0b1bfe7658..595e2afb2141 100644
> > > > --- a/drivers/soc/qcom/spm.c
> > > > +++ b/drivers/soc/qcom/spm.c
> > > > [...]
> > > > @@ -238,6 +273,181 @@ 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 vctl, data0, data1, avs_ctl, sts;
> > > > + unsigned int vlevel, volt_sel;
> > > > + bool avs_enabled;
> > > > +
> > > > + volt_sel = drv->volt_sel;
> > > > + vlevel = volt_sel | 0x80; /* band */
> > > > +
> > > > + 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, SPM_VCTL_VLVL, vlevel);
> > > > + data0 = FIELD_SET(data0, SPM_PMIC_DATA_0_VLVL, vlevel);
> > > > + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MIN_VSEL, volt_sel);
> > > > + data1 = FIELD_SET(data1, SPM_PMIC_DATA_1_MAX_VSEL, volt_sel);
> > > > +
> > > > + 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)) {
> > > > + dev_err_ratelimited(drv->dev, "timeout setting the voltage (%x %x)!\n", sts, vlevel);
> > > > + goto enable_avs;
> > > > + }
> > > > +
> > > > + if (avs_enabled) {
> > > > + unsigned int max_avs = volt_sel;
> > > > + unsigned int min_avs = max(max_avs, 4U) - 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;
> > > > +
> > > > + for_each_possible_cpu(cpu) {
> > > > + struct device_node *cpu_node, *saw_node;
> > > > +
> > > > + 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);
> > > > + 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 */
> > >
> > > Is this necessary? I would kind of expect that it's only important that
> > > this doesn't happen in parallel on multiple CPUs. The lock in the
> > > regulator core should already ensure that, though. It's somewhat
> > > expensive to schedule on other cores, especially if they are currently
> > > idle and power collapsed.
> >
> > If I understand correctly, it is the other way around. From the msm
> > kernels I see that CPU SPM calls are scheduled to be executed only on
> > the corresponding CPU/core. For L2 we didn't have the CPU, so to keep
> > the same code path I selected for them to be executed on CPU0. At this
> > point I'm not even sure if this cpu0 comes from the downstream tree or
> > not.
> >
>
> Hm, does 8064/8960 set any voltages through the L2 SPM at all? From a
> quick look I see only the core SPMs being used as regulators, the L2 SPM
> seems to be only used for idling.
>
> I found a msm_spm_apcs_set_vdd() [1] but:
>
> - That one skips the smp_call stuff and calls msm_spm_drv_set_vdd()
> directly. In other words, there is no scheduling to CPU0.
> - It seems to be only used in krait-regulator which is I think
> 8974-specific?
Indeed, L2 SAW doesn't seem to be used for voltage setting on 8064.
Let's drop its regulator completely and add it back if it gets
required one day.
>
> If you don't need the regulator on the L2 SPM you could just put an
> error here rather than the CPU0 fallback. Or alternatively I'd try to
> avoid the smp_call_function_single() for L2 since I don't see why those
> should need to be called on a specific CPU.
>
> Thanks,
> Stephan
>
> [1]: https://git.codelinaro.org/clo/la/kernel/msm/-/blob/LA.AF.1.2.1-08410-8064.0/arch/arm/mach-msm/spm_devices.c#L257
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 04/10] ARM: dts: qcom: apq8064: rename SAW nodes to power-manager
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
` (2 preceding siblings ...)
2023-12-07 13:06 ` [PATCH v5 03/10] soc: qcom: spm: add support for voltage regulator Dmitry Baryshkov
@ 2023-12-07 13:06 ` Dmitry Baryshkov
2023-12-07 13:06 ` [PATCH v5 05/10] ARM: dts: qcom: apq8064: declare SAW2 regulators Dmitry Baryshkov
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:06 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
Per the power-domain.yaml, the power-controller node name is reserved
for power-domain providers. Rename SAW2 nodes to 'power-manager', the
name which is suggested by qcom,spm.yaml
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
index 95ac25e1a3b4..6832030c2c88 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
@@ -365,25 +365,25 @@ acc3: clock-controller@20b8000 {
#clock-cells = <0>;
};
- saw0: power-controller@2089000 {
+ saw0: power-manager@2089000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw1: power-controller@2099000 {
+ saw1: power-manager@2099000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw2: power-controller@20a9000 {
+ saw2: power-manager@20a9000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020a9000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw3: power-controller@20b9000 {
+ saw3: power-manager@20b9000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020b9000 0x1000>, <0x02009000 0x1000>;
regulator;
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 05/10] ARM: dts: qcom: apq8064: declare SAW2 regulators
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
` (3 preceding siblings ...)
2023-12-07 13:06 ` [PATCH v5 04/10] ARM: dts: qcom: apq8064: rename SAW nodes to power-manager Dmitry Baryshkov
@ 2023-12-07 13:06 ` Dmitry Baryshkov
2023-12-07 13:06 ` [PATCH v5 06/10] ARM: dts: qcom: msm8960: " Dmitry Baryshkov
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:06 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
The SAW2 (SPM and AVS Wrapper) among other things is yet another way to
handle CPU-related PMIC regulators. Provide a way to control voltage of
these regulators.
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
index 6832030c2c88..44c65ebf6c73 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
@@ -368,25 +368,41 @@ acc3: clock-controller@20b8000 {
saw0: power-manager@2089000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
- regulator;
+
+ saw0_vreg: regulator {
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1300000>;
+ };
};
saw1: power-manager@2099000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
- regulator;
+
+ saw1_vreg: regulator {
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1300000>;
+ };
};
saw2: power-manager@20a9000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020a9000 0x1000>, <0x02009000 0x1000>;
- regulator;
+
+ saw2_vreg: regulator {
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1300000>;
+ };
};
saw3: power-manager@20b9000 {
compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020b9000 0x1000>, <0x02009000 0x1000>;
- regulator;
+
+ saw3_vreg: regulator {
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1300000>;
+ };
};
sps_sic_non_secure: sps-sic-non-secure@12100000 {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 06/10] ARM: dts: qcom: msm8960: declare SAW2 regulators
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
` (4 preceding siblings ...)
2023-12-07 13:06 ` [PATCH v5 05/10] ARM: dts: qcom: apq8064: declare SAW2 regulators Dmitry Baryshkov
@ 2023-12-07 13:06 ` Dmitry Baryshkov
2023-12-07 13:07 ` [PATCH v5 07/10] ARM: dts: qcom: apq8084: drop 'regulator' property from SAW2 device Dmitry Baryshkov
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:06 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
The SAW2 (SPM and AVS Wrapper) among other things is yet another way to
handle CPU-related PMIC regulators. Provide a way to control voltage of
these regulators.
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
arch/arm/boot/dts/qcom/qcom-msm8960.dtsi | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
index f420740e068e..0ab340405784 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
@@ -223,13 +223,21 @@ acc1: clock-controller@2098000 {
saw0: regulator@2089000 {
compatible = "qcom,saw2";
reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
- regulator;
+
+ saw0_vreg: regulator {
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1300000>;
+ };
};
saw1: regulator@2099000 {
compatible = "qcom,saw2";
reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
- regulator;
+
+ saw1_vreg: regulator {
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <1300000>;
+ };
};
gsbi5: gsbi@16400000 {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 07/10] ARM: dts: qcom: apq8084: drop 'regulator' property from SAW2 device
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
` (5 preceding siblings ...)
2023-12-07 13:06 ` [PATCH v5 06/10] ARM: dts: qcom: msm8960: " Dmitry Baryshkov
@ 2023-12-07 13:07 ` Dmitry Baryshkov
2023-12-07 13:07 ` [PATCH v5 08/10] ARM: dts: qcom: msm8974: " Dmitry Baryshkov
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
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.
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
arch/arm/boot/dts/qcom/qcom-apq8084.dtsi | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8084.dtsi
index 2b1f9d0fb510..24bc2cbfbd96 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-apq8084.dtsi
@@ -652,7 +652,6 @@ saw3: power-controller@f90b9000 {
saw_l2: power-controller@f9012000 {
compatible = "qcom,saw2";
reg = <0xf9012000 0x1000>;
- regulator;
};
acc0: power-manager@f9088000 {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 08/10] ARM: dts: qcom: msm8974: drop 'regulator' property from SAW2 device
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
` (6 preceding siblings ...)
2023-12-07 13:07 ` [PATCH v5 07/10] ARM: dts: qcom: apq8084: drop 'regulator' property from SAW2 device Dmitry Baryshkov
@ 2023-12-07 13:07 ` Dmitry Baryshkov
2023-12-07 13:07 ` [PATCH v5 09/10] ARM: dts: qcom: ipq4019: drop 'regulator' property from SAW2 devices Dmitry Baryshkov
2023-12-07 13:07 ` [PATCH v5 10/10] ARM: dts: qcom: ipq8064: " Dmitry Baryshkov
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
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.
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
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 ee202f3f161e..3e1e88d69c2e 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
@@ -349,7 +349,6 @@ apcs: syscon@f9011000 {
saw_l2: power-controller@f9012000 {
compatible = "qcom,saw2";
reg = <0xf9012000 0x1000>;
- regulator;
};
watchdog@f9017000 {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 09/10] ARM: dts: qcom: ipq4019: drop 'regulator' property from SAW2 devices
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
` (7 preceding siblings ...)
2023-12-07 13:07 ` [PATCH v5 08/10] ARM: dts: qcom: msm8974: " Dmitry Baryshkov
@ 2023-12-07 13:07 ` Dmitry Baryshkov
2023-12-07 13:07 ` [PATCH v5 10/10] ARM: dts: qcom: ipq8064: " Dmitry Baryshkov
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
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.
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
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 9844e0b7cff9..d7bd97997ff9 100644
--- a/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi
@@ -353,31 +353,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 {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 10/10] ARM: dts: qcom: ipq8064: drop 'regulator' property from SAW2 devices
2023-12-07 13:06 [PATCH v5 00/10] soc: qcom: spm: add support for SPM regulator Dmitry Baryshkov
` (8 preceding siblings ...)
2023-12-07 13:07 ` [PATCH v5 09/10] ARM: dts: qcom: ipq4019: drop 'regulator' property from SAW2 devices Dmitry Baryshkov
@ 2023-12-07 13:07 ` Dmitry Baryshkov
9 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-12-07 13:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree
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.
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
index c3677440b786..191d1cb27cb7 100644
--- a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
@@ -589,7 +589,6 @@ acc0: clock-controller@2088000 {
saw0: regulator@2089000 {
compatible = "qcom,saw2";
reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
- regulator;
};
acc1: clock-controller@2098000 {
@@ -604,7 +603,6 @@ acc1: clock-controller@2098000 {
saw1: regulator@2099000 {
compatible = "qcom,saw2";
reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
- regulator;
};
nss_common: syscon@3000000 {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread