[v4,4/8] qcom: spm-devices: Add SPM device manager for the SoC

Message ID 1408486537-6358-5-git-send-email-lina.iyer@linaro.org
State New
Headers show

Commit Message

Lina Iyer Aug. 19, 2014, 10:15 p.m.
Each cpu or an L2$ has an SPM device. They are identical instances of
the same SPM block. This allows for multiple instances be grouped and
managed collectively. spm-devices.c is the SPM device manager managing
multiple SPM devices on top of the driver layer.

Device configuration of each SPM is picked up from the DTS. The hardware
configuration of each of the SPM is handled by the spm.c driver.

Signed-off-by: Praveen Chidamabram <pchidamb@codeaurora.org>
Signed-off-by: Murali Nalajala <mnalajal@codeaurora.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/devicetree/bindings/arm/msm/spm.txt |  47 +++++
 drivers/soc/qcom/Kconfig                          |   8 +
 drivers/soc/qcom/Makefile                         |   2 +-
 drivers/soc/qcom/spm-devices.c                    | 198 ++++++++++++++++++++++
 include/soc/qcom/spm.h                            |  34 ++++
 5 files changed, 288 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/spm.txt
 create mode 100644 drivers/soc/qcom/spm-devices.c
 create mode 100644 include/soc/qcom/spm.h

Comments

Stephen Boyd Aug. 25, 2014, 11:40 p.m. | #1
On 08/19/14 15:15, Lina Iyer wrote:
> diff --git a/Documentation/devicetree/bindings/arm/msm/spm.txt b/Documentation/devicetree/bindings/arm/msm/spm.txt
> new file mode 100644
> index 0000000..318e024
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/spm.txt

We already have a binding document for SAW. Please add stuff there
because SPM is just a component of SAW.

> @@ -0,0 +1,47 @@
> +* Subsystem Power Manager (SAW2)
> +
> +S4 generation of MSMs have SPM hardware blocks to control the Application
> +Processor Sub-System power. These SPM blocks run individual state machine
> +to determine what the core (L2 or Krait/Scorpion) would do when the WFI
> +instruction is executed by the core.
> +
> +The devicetree representation of the SPM block should be:
> +
> +Required properties
> +
> +- compatible: Could be one of -
> +		"qcom,spm-v2.1"
> +		"qcom,spm-v3.0"
> +- reg: The physical address and the size of the SPM's memory mapped registers
> +- qcom,cpu: phandle for the CPU that the SPM block is attached to.
> +	This field is required on only for SPMs that control the CPU.

We have a phandle from the CPU/L2 to the SAW, so this isn't necessary.

> +- qcom,saw2-cfg: SAW2 configuration register

Why? Compatible should indicate where this register is.

> +- qcom,saw2-spm-dly: Provides the values for the SPM delay command in the SPM
> +	sequence

This is actually three values packed into one register for three
different selectable delays, right? We don't typically do register jam
tables in DT. Perhaps it should be split out into 3 different
properties. Or maybe it shouldn't be specified in DT at all and should
be determined algorithmically from the command sequences? From what I
can tell most of the sequences don't even use these delays.

> +- qcom,saw2-spm-ctl: The SPM control register

Why? Compatible should indicate where this register is.

> +
> +Optional properties
> +
> +- qcom,saw2-spm-cmd-wfi: The WFI command sequence
> +- qcom,saw2-spm-cmd-ret: The Retention command sequence
> +- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
> +- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence. This sequence may
> +	turn off other SoC components.
> +- qcom,saw2-spm-cmd-gdhs: GDHS (Globally Distributed Head Switch) command
> +	sequence. This sequence will retain the memory but turn off the logic.

I wonder if these should be properties of the idle states? That way the
driver isn't searching for them by name in DT, instead it knows what
state is associated with what sequence that the SPM needs to have
programmed.

> +-
> +Example:
> +	spm@f9089000 {
> +		compatible = "qcom,spm-v2.1";
> +		#address-cells = <1>;
> +		#size-cells = <1>;

Why is this in the example? Are there subnodes?

> +		reg = <0xf9089000 0x1000>;
> +		qcom,cpu = <&CPU0>;
> +		qcom,saw2-cfg = <0x1>;
> +		qcom,saw2-spm-dly= <0x20000400>;
> +		qcom,saw2-spm-ctl = <0x1>;
> +		qcom,saw2-spm-cmd-wfi = [03 0b 0f];
> +		qcom,saw2-spm-cmd-spc = [00 20 50 80 60 70 10 92
> +				a0 b0 03 68 70 3b 92 a0 b0
> +				82 2b 50 10 30 02 22 30 0f];
> +	};
> diff --git a/drivers/soc/qcom/spm-devices.c b/drivers/soc/qcom/spm-devices.c
> new file mode 100644
> index 0000000..2175a81
> --- /dev/null
> +++ b/drivers/soc/qcom/spm-devices.c
> @@ -0,0 +1,198 @@
> +/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +
> +#include <soc/qcom/spm.h>
> +
> +#include "spm-drv.h"
> +
> +/**
> + * All related information for an SPM device
> + * Helps manage the collective.
> + */
> +struct msm_spm_device {
> +	bool initialized;
> +	struct msm_spm_driver_data drv;
> +};
> +
> +static DEFINE_PER_CPU_SHARED_ALIGNED(struct msm_spm_device, msm_cpu_spm_device);
> +
> +/**
> + * msm_spm_set_low_power_mode() - Configure SPM start address for low power mode
> + * @mode: SPM LPM mode to enter
> + */
> +int msm_spm_set_low_power_mode(unsigned int mode)
> +{
> +	struct msm_spm_device *dev = &__get_cpu_var(msm_cpu_spm_device);
> +	int ret = -EINVAL;
> +
> +	if (!dev->initialized)
> +		return -ENXIO;
> +
> +	if (mode == MSM_SPM_MODE_DISABLED)
> +		ret = msm_spm_drv_set_spm_enable(&dev->drv, false);
> +	else if (!msm_spm_drv_set_spm_enable(&dev->drv, true))
> +		ret = msm_spm_drv_set_low_power_mode(&dev->drv, mode);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(msm_spm_set_low_power_mode);
> +
> +static int get_cpu_id(struct device_node *node)
> +{
> +	struct device_node *cpu_node;
> +	u32 cpu;
> +	int ret = -EINVAL;
> +	char *key = "qcom,cpu";
> +
> +	cpu_node = of_parse_phandle(node, key, 0);
> +	if (cpu_node) {
> +		for_each_possible_cpu(cpu) {
> +			if (of_get_cpu_node(cpu, NULL) == cpu_node)
> +				return cpu;
> +		}
> +	}
> +	return ret;
> +}
> +
> +static struct msm_spm_device *msm_spm_get_device(struct platform_device *pdev)
> +{
> +	struct msm_spm_device *dev = NULL;
> +	int cpu = get_cpu_id(pdev->dev.of_node);
> +
> +	if ((cpu >= 0) && cpu < num_possible_cpus())
> +		dev = &per_cpu(msm_cpu_spm_device, cpu);
> +
> +	return dev;
> +}
> +
> +static int msm_spm_dev_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	int i;
> +	struct device_node *node = pdev->dev.of_node;
> +	char *key;
> +	uint32_t val;
> +	struct msm_spm_mode modes[MSM_SPM_MODE_NR];
> +	struct msm_spm_device *spm_dev;
> +	struct resource *res;
> +	uint32_t mode_count = 0;
> +
> +	struct spm_of {
> +		char *key;
> +		uint32_t id;
> +	};
> +
> +	/* SPM Configuration registers */
> +	struct spm_of spm_of_data[] = {
> +		{"qcom,saw2-clk-div", MSM_SPM_REG_SAW2_CFG},
> +		{"qcom,saw2-enable", MSM_SPM_REG_SAW2_SPM_CTL},
> +		{"qcom,saw2-delays", MSM_SPM_REG_SAW2_SPM_DLY},
> +	};
> +
> +	/* SPM sleep sequences */
> +	struct spm_of mode_of_data[] = {
> +		{"qcom,saw2-spm-cmd-wfi", MSM_SPM_MODE_CLOCK_GATING},
> +		{"qcom,saw2-spm-cmd-spc", MSM_SPM_MODE_POWER_COLLAPSE},
> +		{"qcom,saw2-spm-cmd-ret", MSM_SPM_MODE_RETENTION},
> +	};
> +
> +	 /* Get the right SPM device */
> +	spm_dev = msm_spm_get_device(pdev);
> +	if (IS_ERR_OR_NULL(spm_dev))
> +		return -EINVAL;
> +
> +	/* Get the SAW start address */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		ret = -EINVAL;
> +		goto fail;
> +	}
> +	spm_dev->drv.reg_base_addr = devm_ioremap(&pdev->dev, res->start,
> +					resource_size(res));
> +	if (!spm_dev->drv.reg_base_addr) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	/* Read the SPM configuration register values */
> +	for (i = 0; i < ARRAY_SIZE(spm_of_data); i++) {
> +		ret = of_property_read_u32(node, spm_of_data[i].key, &val);
> +		if (ret)
> +			continue;
> +		spm_dev->drv.reg_shadow[spm_of_data[i].id] = val;
> +	}
> +
> +	/* Read the byte arrays for the SPM sleep sequences */
> +	for (i = 0; i < ARRAY_SIZE(mode_of_data); i++) {
> +		modes[mode_count].start_addr = 0;
> +		key = mode_of_data[i].key;
> +		modes[mode_count].cmd =
> +			(uint8_t *)of_get_property(node, key, &val);
> +		if (!modes[mode_count].cmd)
> +			continue;
> +		modes[mode_count].mode = mode_of_data[i].id;
> +		mode_count++;
> +	}
> +
> +	spm_dev->drv.modes = devm_kcalloc(&pdev->dev, mode_count,
> +						sizeof(modes[0]), GFP_KERNEL);
> +	if (!spm_dev->drv.modes)
> +		return -ENOMEM;
> +	spm_dev->drv.num_modes = mode_count;
> +	memcpy(spm_dev->drv.modes, &modes[0], sizeof(modes[0]) * mode_count);
> +
> +	/* Initialize the hardware */
> +	ret = msm_spm_drv_init(&spm_dev->drv);
> +	if (ret) {
> +		kfree(spm_dev->drv.modes);
> +		return ret;
> +	}
> +
> +	spm_dev->initialized = true;
> +	return ret;
> +
> +fail:
> +	dev_err(&pdev->dev, "SPM device probe failed: %d\n", ret);
> +	return ret;
> +}
> +
> +static struct of_device_id msm_spm_match_table[] = {

const.
Lina Iyer Aug. 26, 2014, 12:31 a.m. | #2
On Mon, Aug 25, 2014 at 04:40:33PM -0700, Stephen Boyd wrote:
>On 08/19/14 15:15, Lina Iyer wrote:
>> diff --git a/Documentation/devicetree/bindings/arm/msm/spm.txt b/Documentation/devicetree/bindings/arm/msm/spm.txt
>> new file mode 100644
>> index 0000000..318e024
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/msm/spm.txt
>
>We already have a binding document for SAW. Please add stuff there
>because SPM is just a component of SAW.
>
I agree that SPM is just a component of the SAW. But the document there
seems to indicate regulator details, totally unrelated to the actual SAW
hardware functionality.

>> @@ -0,0 +1,47 @@
>> +* Subsystem Power Manager (SAW2)
>> +
>> +S4 generation of MSMs have SPM hardware blocks to control the Application
>> +Processor Sub-System power. These SPM blocks run individual state machine
>> +to determine what the core (L2 or Krait/Scorpion) would do when the WFI
>> +instruction is executed by the core.
>> +
>> +The devicetree representation of the SPM block should be:
>> +
>> +Required properties
>> +
>> +- compatible: Could be one of -
>> +		"qcom,spm-v2.1"
>> +		"qcom,spm-v3.0"
>> +- reg: The physical address and the size of the SPM's memory mapped registers
>> +- qcom,cpu: phandle for the CPU that the SPM block is attached to.
>> +	This field is required on only for SPMs that control the CPU.
>
>We have a phandle from the CPU/L2 to the SAW, so this isn't necessary.
>
Sorry, I dont understand. Care to explain pls? Its necessary to know
what SPM instance controls which CPU or L2, so as to pick the right SPM
device to configure.

>> +- qcom,saw2-cfg: SAW2 configuration register
>
>Why? Compatible should indicate where this register is.
>
There are multiple versions of saw handled by the same driver and
distinguished by the version register. These SAW revisions differ in the
register offset organization. The variable holds the value to be
configured in the saw2-cfg register. I will update the documentation to
be more clear.

>> +- qcom,saw2-spm-dly: Provides the values for the SPM delay command in the SPM
>> +	sequence
>
>This is actually three values packed into one register for three
>different selectable delays, right? We don't typically do register jam
>tables in DT. Perhaps it should be split out into 3 different
>properties. Or maybe it shouldn't be specified in DT at all and should
>be determined algorithmically from the command sequences? From what I
>can tell most of the sequences don't even use these delays.
>
Not at all sequences use the delays. These cannot be determined
algorithmatically, They may be added to the sequence for changes in
hardware. Let me revisit the sequences to see if they need to be set
with the current sequence in use.

>> +- qcom,saw2-spm-ctl: The SPM control register
>
>Why? Compatible should indicate where this register is.
>
See above.

>> +
>> +Optional properties
>> +
>> +- qcom,saw2-spm-cmd-wfi: The WFI command sequence
>> +- qcom,saw2-spm-cmd-ret: The Retention command sequence
>> +- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
>> +- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence. This sequence may
>> +	turn off other SoC components.
>> +- qcom,saw2-spm-cmd-gdhs: GDHS (Globally Distributed Head Switch) command
>> +	sequence. This sequence will retain the memory but turn off the logic.
>
>I wonder if these should be properties of the idle states? That way the
>driver isn't searching for them by name in DT, instead it knows what
>state is associated with what sequence that the SPM needs to have
>programmed.
>
I see the relation you are seeing. But its not a property of the idle
state. Its an SoC specific property that the idle uses to indicate a
state. Better off lying here. I doubt there would be a good support for
holding SoC specific stuff in the ARM idle-states nodes.

>> +-
>> +Example:
>> +	spm@f9089000 {
>> +		compatible = "qcom,spm-v2.1";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>
>Why is this in the example? Are there subnodes?
>
No, there arent. I guess I carried over from downstream.

>> +		reg = <0xf9089000 0x1000>;
>> +		qcom,cpu = <&CPU0>;
>> +		qcom,saw2-cfg = <0x1>;
>> +		qcom,saw2-spm-dly= <0x20000400>;
>> +		qcom,saw2-spm-ctl = <0x1>;
>> +		qcom,saw2-spm-cmd-wfi = [03 0b 0f];
>> +		qcom,saw2-spm-cmd-spc = [00 20 50 80 60 70 10 92
>> +				a0 b0 03 68 70 3b 92 a0 b0
>> +				82 2b 50 10 30 02 22 30 0f];
>> +	};
>> diff --git a/drivers/soc/qcom/spm-devices.c b/drivers/soc/qcom/spm-devices.c
>> new file mode 100644
>> index 0000000..2175a81
>> --- /dev/null
>> +++ b/drivers/soc/qcom/spm-devices.c
>> @@ -0,0 +1,198 @@
>> +/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/delay.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/err.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include <soc/qcom/spm.h>
>> +
>> +#include "spm-drv.h"
>> +
>> +/**
>> + * All related information for an SPM device
>> + * Helps manage the collective.
>> + */
>> +struct msm_spm_device {
>> +	bool initialized;
>> +	struct msm_spm_driver_data drv;
>> +};
>> +
>> +static DEFINE_PER_CPU_SHARED_ALIGNED(struct msm_spm_device, msm_cpu_spm_device);
>> +
>> +/**
>> + * msm_spm_set_low_power_mode() - Configure SPM start address for low power mode
>> + * @mode: SPM LPM mode to enter
>> + */
>> +int msm_spm_set_low_power_mode(unsigned int mode)
>> +{
>> +	struct msm_spm_device *dev = &__get_cpu_var(msm_cpu_spm_device);
>> +	int ret = -EINVAL;
>> +
>> +	if (!dev->initialized)
>> +		return -ENXIO;
>> +
>> +	if (mode == MSM_SPM_MODE_DISABLED)
>> +		ret = msm_spm_drv_set_spm_enable(&dev->drv, false);
>> +	else if (!msm_spm_drv_set_spm_enable(&dev->drv, true))
>> +		ret = msm_spm_drv_set_low_power_mode(&dev->drv, mode);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(msm_spm_set_low_power_mode);
>> +
>> +static int get_cpu_id(struct device_node *node)
>> +{
>> +	struct device_node *cpu_node;
>> +	u32 cpu;
>> +	int ret = -EINVAL;
>> +	char *key = "qcom,cpu";
>> +
>> +	cpu_node = of_parse_phandle(node, key, 0);
>> +	if (cpu_node) {
>> +		for_each_possible_cpu(cpu) {
>> +			if (of_get_cpu_node(cpu, NULL) == cpu_node)
>> +				return cpu;
>> +		}
>> +	}
>> +	return ret;
>> +}
>> +
>> +static struct msm_spm_device *msm_spm_get_device(struct platform_device *pdev)
>> +{
>> +	struct msm_spm_device *dev = NULL;
>> +	int cpu = get_cpu_id(pdev->dev.of_node);
>> +
>> +	if ((cpu >= 0) && cpu < num_possible_cpus())
>> +		dev = &per_cpu(msm_cpu_spm_device, cpu);
>> +
>> +	return dev;
>> +}
>> +
>> +static int msm_spm_dev_probe(struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	int i;
>> +	struct device_node *node = pdev->dev.of_node;
>> +	char *key;
>> +	uint32_t val;
>> +	struct msm_spm_mode modes[MSM_SPM_MODE_NR];
>> +	struct msm_spm_device *spm_dev;
>> +	struct resource *res;
>> +	uint32_t mode_count = 0;
>> +
>> +	struct spm_of {
>> +		char *key;
>> +		uint32_t id;
>> +	};
>> +
>> +	/* SPM Configuration registers */
>> +	struct spm_of spm_of_data[] = {
>> +		{"qcom,saw2-clk-div", MSM_SPM_REG_SAW2_CFG},
>> +		{"qcom,saw2-enable", MSM_SPM_REG_SAW2_SPM_CTL},
>> +		{"qcom,saw2-delays", MSM_SPM_REG_SAW2_SPM_DLY},
>> +	};
>> +
>> +	/* SPM sleep sequences */
>> +	struct spm_of mode_of_data[] = {
>> +		{"qcom,saw2-spm-cmd-wfi", MSM_SPM_MODE_CLOCK_GATING},
>> +		{"qcom,saw2-spm-cmd-spc", MSM_SPM_MODE_POWER_COLLAPSE},
>> +		{"qcom,saw2-spm-cmd-ret", MSM_SPM_MODE_RETENTION},
>> +	};
>> +
>> +	 /* Get the right SPM device */
>> +	spm_dev = msm_spm_get_device(pdev);
>> +	if (IS_ERR_OR_NULL(spm_dev))
>> +		return -EINVAL;
>> +
>> +	/* Get the SAW start address */
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		ret = -EINVAL;
>> +		goto fail;
>> +	}
>> +	spm_dev->drv.reg_base_addr = devm_ioremap(&pdev->dev, res->start,
>> +					resource_size(res));
>> +	if (!spm_dev->drv.reg_base_addr) {
>> +		ret = -ENOMEM;
>> +		goto fail;
>> +	}
>> +
>> +	/* Read the SPM configuration register values */
>> +	for (i = 0; i < ARRAY_SIZE(spm_of_data); i++) {
>> +		ret = of_property_read_u32(node, spm_of_data[i].key, &val);
>> +		if (ret)
>> +			continue;
>> +		spm_dev->drv.reg_shadow[spm_of_data[i].id] = val;
>> +	}
>> +
>> +	/* Read the byte arrays for the SPM sleep sequences */
>> +	for (i = 0; i < ARRAY_SIZE(mode_of_data); i++) {
>> +		modes[mode_count].start_addr = 0;
>> +		key = mode_of_data[i].key;
>> +		modes[mode_count].cmd =
>> +			(uint8_t *)of_get_property(node, key, &val);
>> +		if (!modes[mode_count].cmd)
>> +			continue;
>> +		modes[mode_count].mode = mode_of_data[i].id;
>> +		mode_count++;
>> +	}
>> +
>> +	spm_dev->drv.modes = devm_kcalloc(&pdev->dev, mode_count,
>> +						sizeof(modes[0]), GFP_KERNEL);
>> +	if (!spm_dev->drv.modes)
>> +		return -ENOMEM;
>> +	spm_dev->drv.num_modes = mode_count;
>> +	memcpy(spm_dev->drv.modes, &modes[0], sizeof(modes[0]) * mode_count);
>> +
>> +	/* Initialize the hardware */
>> +	ret = msm_spm_drv_init(&spm_dev->drv);
>> +	if (ret) {
>> +		kfree(spm_dev->drv.modes);
>> +		return ret;
>> +	}
>> +
>> +	spm_dev->initialized = true;
>> +	return ret;
>> +
>> +fail:
>> +	dev_err(&pdev->dev, "SPM device probe failed: %d\n", ret);
>> +	return ret;
>> +}
>> +
>> +static struct of_device_id msm_spm_match_table[] = {
>
>const.
>
Will change.
>
>-- 
>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>hosted by The Linux Foundation
>
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Aug. 26, 2014, 2:17 a.m. | #3
On 08/25/14 17:31, Lina Iyer wrote:
> On Mon, Aug 25, 2014 at 04:40:33PM -0700, Stephen Boyd wrote:
>> On 08/19/14 15:15, Lina Iyer wrote:
>>> diff --git a/Documentation/devicetree/bindings/arm/msm/spm.txt
>>> b/Documentation/devicetree/bindings/arm/msm/spm.txt
>>> new file mode 100644
>>> index 0000000..318e024
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/msm/spm.txt
>>
>> We already have a binding document for SAW. Please add stuff there
>> because SPM is just a component of SAW.
>>
> I agree that SPM is just a component of the SAW. But the document there
> seems to indicate regulator details, totally unrelated to the actual SAW
> hardware functionality.

Huh? The SAW binding should be extended with whatever properties are
necessary. Probably the only thing we need is the delays. Everything
else can be determined from the compatible?

SAW has a connection to the PMIC, does it not? If it isn't directly
connected we can come up with a different name for the node, but just
because the node name in the example is misleading doesn't mean we
should completely disregard what we already have.

>
>>> @@ -0,0 +1,47 @@
>>> +* Subsystem Power Manager (SAW2)
>>> +
>>> +S4 generation of MSMs have SPM hardware blocks to control the
>>> Application
>>> +Processor Sub-System power. These SPM blocks run individual state
>>> machine
>>> +to determine what the core (L2 or Krait/Scorpion) would do when the
>>> WFI
>>> +instruction is executed by the core.
>>> +
>>> +The devicetree representation of the SPM block should be:
>>> +
>>> +Required properties
>>> +
>>> +- compatible: Could be one of -
>>> +        "qcom,spm-v2.1"
>>> +        "qcom,spm-v3.0"
>>> +- reg: The physical address and the size of the SPM's memory mapped
>>> registers
>>> +- qcom,cpu: phandle for the CPU that the SPM block is attached to.
>>> +    This field is required on only for SPMs that control the CPU.
>>
>> We have a phandle from the CPU/L2 to the SAW, so this isn't necessary.
>>
> Sorry, I dont understand. Care to explain pls? Its necessary to know
> what SPM instance controls which CPU or L2, so as to pick the right SPM
> device to configure.

We have a phandle in the CPU nodes pointing to the SAW that is
associated with that CPU (qcom,saw). SPM is a part of a SAW. Thus there
is no need for this qcom,cpu property once the SAW node is used.
Instead, we can search the CPU and cache nodes for a qcom,saw that
matches the of_node for the platform device we're probing.

>
>>> +- qcom,saw2-cfg: SAW2 configuration register
>>
>> Why? Compatible should indicate where this register is.
>>
> There are multiple versions of saw handled by the same driver and
> distinguished by the version register. These SAW revisions differ in the
> register offset organization. The variable holds the value to be
> configured in the saw2-cfg register. I will update the documentation to
> be more clear.
>>> +- qcom,saw2-spm-ctl: The SPM control register
>>
>> Why? Compatible should indicate where this register is.
>>
> See above. 

Ah this is more register jam table in DT? cfg should probably be
described in desired clock rate and then the driver can figure out the
value by multiplying that the input clock rate. spm-ctl looks like it's
usually used to describe "enable", which seems rather obvious. Why isn't
the driver always writing the enable bit (bit 0)?

>
>>> +- qcom,saw2-spm-dly: Provides the values for the SPM delay command
>>> in the SPM
>>> +    sequence
>>
>> This is actually three values packed into one register for three
>> different selectable delays, right? We don't typically do register jam
>> tables in DT. Perhaps it should be split out into 3 different
>> properties. Or maybe it shouldn't be specified in DT at all and should
>> be determined algorithmically from the command sequences? From what I
>> can tell most of the sequences don't even use these delays.
>>
> Not at all sequences use the delays. These cannot be determined
> algorithmatically, They may be added to the sequence for changes in
> hardware. Let me revisit the sequences to see if they need to be set
> with the current sequence in use.
>

I was thinking perhaps these should be more structured binary blobs that
indicate the delays that would be necessary in the first 3 bytes or
something and then the command sequence after that.

        <delay1> <delay2> <delay3> <sequence>

Or perhaps

        <num delays=N> <delayN-1> <delayN> <sequence>

and then the code would parse these first few bytes and compress them
into 3 values that are written into the register.

BTW, I wonder if these sequences should be firmware blobs? Or at least,
different files that we then /incbin/ into the final DT blob (if DT
reviewers approve putting blobs like this into the kernel, Cc'ed the DT
list just in case).

>
>
>>> +
>>> +Optional properties
>>> +
>>> +- qcom,saw2-spm-cmd-wfi: The WFI command sequence
>>> +- qcom,saw2-spm-cmd-ret: The Retention command sequence
>>> +- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
>>> +- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence. This
>>> sequence may
>>> +    turn off other SoC components.
>>> +- qcom,saw2-spm-cmd-gdhs: GDHS (Globally Distributed Head Switch)
>>> command
>>> +    sequence. This sequence will retain the memory but turn off the
>>> logic.
>>
>> I wonder if these should be properties of the idle states? That way the
>> driver isn't searching for them by name in DT, instead it knows what
>> state is associated with what sequence that the SPM needs to have
>> programmed.
>>
> I see the relation you are seeing. But its not a property of the idle
> state. Its an SoC specific property that the idle uses to indicate a
> state. Better off lying here. I doubt there would be a good support for
> holding SoC specific stuff in the ARM idle-states nodes.
>
>

What isn't specifically related to the 1) idle state and 2) CPU/L2/etc.
that the idle state is used in? I would think that by pointing to
different idle states from different CPU nodes we could cover all cases.
What is the SoC specific stuff in here?
Lina Iyer Aug. 26, 2014, 3:33 p.m. | #4
On Mon, Aug 25, 2014 at 07:17:15PM -0700, Stephen Boyd wrote:
> On 08/25/14 17:31, Lina Iyer wrote:
>> On Mon, Aug 25, 2014 at 04:40:33PM -0700, Stephen Boyd wrote:
>>> On 08/19/14 15:15, Lina Iyer wrote:
>>>> diff --git a/Documentation/devicetree/bindings/arm/msm/spm.txt
>>>> b/Documentation/devicetree/bindings/arm/msm/spm.txt
>>>> new file mode 100644
>>>> index 0000000..318e024
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/arm/msm/spm.txt
>>> 
>>> We already have a binding document for SAW. Please add stuff there
>>> because SPM is just a component of SAW.
>>> 
>> I agree that SPM is just a component of the SAW. But the document there
>> seems to indicate regulator details, totally unrelated to the actual SAW
>> hardware functionality.
> 
> Huh? The SAW binding should be extended with whatever properties are
> necessary. Probably the only thing we need is the delays. Everything
> else can be determined from the compatible?
> 
Thats not true. There are many voltage related properties that are yet
to come. Not everything else can be determined from the compatible flag.
The idea behind having SPM nodes in the DT is that different SoCs may
have the same version of SPM but may have to talk to different
components. The delays, the PMIC register and the PMIC data all change
with the components that SPM communicates.

> SAW has a connection to the PMIC, does it not? If it isn't directly
> connected we can come up with a different name for the node, but just
> because the node name in the example is misleading doesn't mean we
> should completely disregard what we already have.
> 
Yes the SAW has a connection to the PMIC. But what the nodes represent
are just regulator nodes and have no bearing on the SAW. You could use
SPMI/I2C writes to convey the same value without having to go through
SAW using those nodes. I have no reasonable understanding as to why they
are called SAW regulators in the first place. They are just regulators.
The mechanism of using SPM to communicate with the PMIC does not exist
in the kernel and is not part of those nodes. So adding to that
confusion wasn't a wise choice for me.

>> 
>>>> @@ -0,0 +1,47 @@
>>>> +* Subsystem Power Manager (SAW2)
>>>> +
>>>> +S4 generation of MSMs have SPM hardware blocks to control the
>>>> Application
>>>> +Processor Sub-System power. These SPM blocks run individual state
>>>> machine
>>>> +to determine what the core (L2 or Krait/Scorpion) would do when the
>>>> WFI
>>>> +instruction is executed by the core.
>>>> +
>>>> +The devicetree representation of the SPM block should be:
>>>> +
>>>> +Required properties
>>>> +
>>>> +- compatible: Could be one of -
>>>> +        "qcom,spm-v2.1"
>>>> +        "qcom,spm-v3.0"
>>>> +- reg: The physical address and the size of the SPM's memory mapped
>>>> registers
>>>> +- qcom,cpu: phandle for the CPU that the SPM block is attached to.
>>>> +    This field is required on only for SPMs that control the CPU.
>>> 
>>> We have a phandle from the CPU/L2 to the SAW, so this isn't necessary.
>>> 
>> Sorry, I dont understand. Care to explain pls? Its necessary to know
>> what SPM instance controls which CPU or L2, so as to pick the right SPM
>> device to configure.
> 
> We have a phandle in the CPU nodes pointing to the SAW that is
> associated with that CPU (qcom,saw). SPM is a part of a SAW. Thus there
> is no need for this qcom,cpu property once the SAW node is used.
> Instead, we can search the CPU and cache nodes for a qcom,saw that
> matches the of_node for the platform device we're probing.
> 
I agree thats doable. The cpu node can point to the SPM node or the
otherway as it is done here. I dont have a strong preference eitherway.
Lets resolve the earlier and we may have a solution for this, in our
hands.

>> 
>>>> +- qcom,saw2-cfg: SAW2 configuration register
>>> 
>>> Why? Compatible should indicate where this register is.
>>> 
>> There are multiple versions of saw handled by the same driver and
>> distinguished by the version register. These SAW revisions differ in the
>> register offset organization. The variable holds the value to be
>> configured in the saw2-cfg register. I will update the documentation to
>> be more clear.
>>>> +- qcom,saw2-spm-ctl: The SPM control register
>>> 
>>> Why? Compatible should indicate where this register is.
>>> 
>> See above.
> 
> Ah this is more register jam table in DT? cfg should probably be
> described in desired clock rate and then the driver can figure out the
> value by multiplying that the input clock rate. spm-ctl looks like it's
> usually used to describe "enable", which seems rather obvious. Why isn't
> the driver always writing the enable bit (bit 0)?
> 
Kumar already had suggested that. I changed the code, but forgot to
update the documentation, which I noticed and amended in my local tree.
Waiting for some more comments and some changes in cpuidle stuff before
I submit the next rev. Thanks.
That said, figuring out the input clock rate and the multiplication is
still unnecessary. There are not much options on the hardware to change
clock rate and different parameters that will work well on a SoC. SPMs
are generic state machines and are designed to support multiple QCOM
SoCs and hence the configuration options. They dont generally run at
different clock rates and different configurations on the same SoCs.

>> 
>>>> +- qcom,saw2-spm-dly: Provides the values for the SPM delay command
>>>> in the SPM
>>>> +    sequence
>>> 
>>> This is actually three values packed into one register for three
>>> different selectable delays, right? We don't typically do register jam
>>> tables in DT. Perhaps it should be split out into 3 different
>>> properties. Or maybe it shouldn't be specified in DT at all and should
>>> be determined algorithmically from the command sequences? From what I
>>> can tell most of the sequences don't even use these delays.
>>> 
>> Not at all sequences use the delays. These cannot be determined
>> algorithmatically, They may be added to the sequence for changes in
>> hardware. Let me revisit the sequences to see if they need to be set
>> with the current sequence in use.
>> 
> 
> I was thinking perhaps these should be more structured binary blobs that
> indicate the delays that would be necessary in the first 3 bytes or
> something and then the command sequence after that.
> 
>        <delay1> <delay2> <delay3> <sequence>
> 
> Or perhaps
> 
>        <num delays=N> <delayN-1> <delayN> <sequence>
> 
> and then the code would parse these first few bytes and compress them
> into 3 values that are written into the register.
> 
Phew..A lot of splits and merge to get the SPM configured. The hardware
specs dictate a certain value, most of the time you just go with that
for that SoC and dont meddle around with these values. Given that, I
wonder how much benefit do we get by these reorganizations and
interpretations? Dont get me wrong, I am not resisting change. I just am
trying to find justification for making changes downstream as well.

> BTW, I wonder if these sequences should be firmware blobs? Or at least,
> different files that we then /incbin/ into the final DT blob (if DT
> reviewers approve putting blobs like this into the kernel, Cc'ed the DT
> list just in case).
> 
I dint know better. Hence this place, I am not sure the
advantages/disadvantages of putting the sequences in firmware blobs. Let
me hunt for some information.

>> 
>> 
>>>> +
>>>> +Optional properties
>>>> +
>>>> +- qcom,saw2-spm-cmd-wfi: The WFI command sequence
>>>> +- qcom,saw2-spm-cmd-ret: The Retention command sequence
>>>> +- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
>>>> +- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence. This
>>>> sequence may
>>>> +    turn off other SoC components.
>>>> +- qcom,saw2-spm-cmd-gdhs: GDHS (Globally Distributed Head Switch)
>>>> command
>>>> +    sequence. This sequence will retain the memory but turn off the
>>>> logic.
>>> 
>>> I wonder if these should be properties of the idle states? That way the
>>> driver isn't searching for them by name in DT, instead it knows what
>>> state is associated with what sequence that the SPM needs to have
>>> programmed.
>>> 
>> I see the relation you are seeing. But its not a property of the idle
>> state. Its an SoC specific property that the idle uses to indicate a
>> state. Better off lying here. I doubt there would be a good support for
>> holding SoC specific stuff in the ARM idle-states nodes.
>> 
>> 
> 
> What isn't specifically related to the 1) idle state and 2) CPU/L2/etc.
> that the idle state is used in? I would think that by pointing to
> different idle states from different CPU nodes we could cover all cases.
> What is the SoC specific stuff in here?
> 
SPM is specific to SoC. I am not sure if people are open to pointing to
SPM and its sequences from a node thats generic to all ARM cores. If
thats allowed, I will be open to add these to the idle-state nodes, just
not sure what it buys us. There is no commonality to other vendor's
SoCs.

> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kumar Gala Aug. 27, 2014, 2 p.m. | #5
On Aug 19, 2014, at 5:15 PM, Lina Iyer <lina.iyer@linaro.org> wrote:

> Each cpu or an L2$ has an SPM device. They are identical instances of
> the same SPM block. This allows for multiple instances be grouped and
> managed collectively. spm-devices.c is the SPM device manager managing
> multiple SPM devices on top of the driver layer.
> 
> Device configuration of each SPM is picked up from the DTS. The hardware
> configuration of each of the SPM is handled by the spm.c driver.
> 
> Signed-off-by: Praveen Chidamabram <pchidamb@codeaurora.org>
> Signed-off-by: Murali Nalajala <mnalajal@codeaurora.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> Documentation/devicetree/bindings/arm/msm/spm.txt |  47 +++++
> drivers/soc/qcom/Kconfig                          |   8 +
> drivers/soc/qcom/Makefile                         |   2 +-
> drivers/soc/qcom/spm-devices.c                    | 198 ++++++++++++++++++++++
> include/soc/qcom/spm.h                            |  34 ++++
> 5 files changed, 288 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/arm/msm/spm.txt
> create mode 100644 drivers/soc/qcom/spm-devices.c
> create mode 100644 include/soc/qcom/spm.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/msm/spm.txt b/Documentation/devicetree/bindings/arm/msm/spm.txt
> new file mode 100644
> index 0000000..318e024
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/spm.txt
> @@ -0,0 +1,47 @@
> +* Subsystem Power Manager (SAW2)
> +
> +S4 generation of MSMs have SPM hardware blocks to control the Application
> +Processor Sub-System power. These SPM blocks run individual state machine
> +to determine what the core (L2 or Krait/Scorpion) would do when the WFI
> +instruction is executed by the core.
> +
> +The devicetree representation of the SPM block should be:
> +
> +Required properties
> +
> +- compatible: Could be one of -
> +		"qcom,spm-v2.1"
> +		"qcom,spm-v3.0"
> +- reg: The physical address and the size of the SPM's memory mapped registers
> +- qcom,cpu: phandle for the CPU that the SPM block is attached to.
> +	This field is required on only for SPMs that control the CPU.

This should probably moved to optional since not ALL spm nodes would have it.

> +- qcom,saw2-cfg: SAW2 configuration register
> +- qcom,saw2-spm-dly: Provides the values for the SPM delay command in the SPM
> +	sequence
> +- qcom,saw2-spm-ctl: The SPM control register

Where is the code that uses "qcom,saw2-spm-dly” & "qcom,saw2-spm-ctl"

> +
> +Optional properties
> +
> +- qcom,saw2-spm-cmd-wfi: The WFI command sequence
> +- qcom,saw2-spm-cmd-ret: The Retention command sequence
> +- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
> +- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence. This sequence may
> +	turn off other SoC components.
> +- qcom,saw2-spm-cmd-gdhs: GDHS (Globally Distributed Head Switch) command
> +	sequence. This sequence will retain the memory but turn off the logic.
> +-
> +Example:
> +	spm@f9089000 {
> +		compatible = "qcom,spm-v2.1";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		reg = <0xf9089000 0x1000>;
> +		qcom,cpu = <&CPU0>;
> +		qcom,saw2-cfg = <0x1>;
> +		qcom,saw2-spm-dly= <0x20000400>;
> +		qcom,saw2-spm-ctl = <0x1>;
> +		qcom,saw2-spm-cmd-wfi = [03 0b 0f];
> +		qcom,saw2-spm-cmd-spc = [00 20 50 80 60 70 10 92
> +				a0 b0 03 68 70 3b 92 a0 b0
> +				82 2b 50 10 30 02 22 30 0f];
> +	};
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 7dcd554..d467767 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -11,3 +11,11 @@ config QCOM_GSBI
> 
> config QCOM_SCM
> 	bool
> +
> +config QCOM_PM
> +	bool "Qualcomm Power Management"
> +	depends on PM && ARCH_QCOM && OF

ARCH_QCOM implies OF

> +	help
> +	  QCOM Platform specific power driver to manage cores and L2 low power
> +	  modes. It interface with various system drivers to put the cores in
> +	  low power modes.
> 



> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lina Iyer Aug. 27, 2014, 3:35 p.m. | #6
On Wed, Aug 27, 2014 at 09:00:40AM -0500, Kumar Gala wrote:
>
>On Aug 19, 2014, at 5:15 PM, Lina Iyer <lina.iyer@linaro.org> wrote:
>
>> Each cpu or an L2$ has an SPM device. They are identical instances of
>> the same SPM block. This allows for multiple instances be grouped and
>> managed collectively. spm-devices.c is the SPM device manager managing
>> multiple SPM devices on top of the driver layer.
>>
>> Device configuration of each SPM is picked up from the DTS. The hardware
>> configuration of each of the SPM is handled by the spm.c driver.
>>
>> Signed-off-by: Praveen Chidamabram <pchidamb@codeaurora.org>
>> Signed-off-by: Murali Nalajala <mnalajal@codeaurora.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>> Documentation/devicetree/bindings/arm/msm/spm.txt |  47 +++++
>> drivers/soc/qcom/Kconfig                          |   8 +
>> drivers/soc/qcom/Makefile                         |   2 +-
>> drivers/soc/qcom/spm-devices.c                    | 198 ++++++++++++++++++++++
>> include/soc/qcom/spm.h                            |  34 ++++
>> 5 files changed, 288 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/devicetree/bindings/arm/msm/spm.txt
>> create mode 100644 drivers/soc/qcom/spm-devices.c
>> create mode 100644 include/soc/qcom/spm.h
>>
>> diff --git a/Documentation/devicetree/bindings/arm/msm/spm.txt b/Documentation/devicetree/bindings/arm/msm/spm.txt
>> new file mode 100644
>> index 0000000..318e024
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/msm/spm.txt
>> @@ -0,0 +1,47 @@
>> +* Subsystem Power Manager (SAW2)
>> +
>> +S4 generation of MSMs have SPM hardware blocks to control the Application
>> +Processor Sub-System power. These SPM blocks run individual state machine
>> +to determine what the core (L2 or Krait/Scorpion) would do when the WFI
>> +instruction is executed by the core.
>> +
>> +The devicetree representation of the SPM block should be:
>> +
>> +Required properties
>> +
>> +- compatible: Could be one of -
>> +		"qcom,spm-v2.1"
>> +		"qcom,spm-v3.0"
>> +- reg: The physical address and the size of the SPM's memory mapped registers
>> +- qcom,cpu: phandle for the CPU that the SPM block is attached to.
>> +	This field is required on only for SPMs that control the CPU.
>
>This should probably moved to optional since not ALL spm nodes would have it.
>
OK.
>> +- qcom,saw2-cfg: SAW2 configuration register
>> +- qcom,saw2-spm-dly: Provides the values for the SPM delay command in the SPM
>> +	sequence
>> +- qcom,saw2-spm-ctl: The SPM control register
>
>Where is the code that uses "qcom,saw2-spm-dly” & "qcom,saw2-spm-ctl"
>
Sorry. Updated the code, but not the documentation in this version. I
noticed well after I sent the patch out. Will be fixed in the next
revision.

>> +
>> +Optional properties
>> +
>> +- qcom,saw2-spm-cmd-wfi: The WFI command sequence
>> +- qcom,saw2-spm-cmd-ret: The Retention command sequence
>> +- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
>> +- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence. This sequence may
>> +	turn off other SoC components.
>> +- qcom,saw2-spm-cmd-gdhs: GDHS (Globally Distributed Head Switch) command
>> +	sequence. This sequence will retain the memory but turn off the logic.
>> +-
>> +Example:
>> +	spm@f9089000 {
>> +		compatible = "qcom,spm-v2.1";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		reg = <0xf9089000 0x1000>;
>> +		qcom,cpu = <&CPU0>;
>> +		qcom,saw2-cfg = <0x1>;
>> +		qcom,saw2-spm-dly= <0x20000400>;
>> +		qcom,saw2-spm-ctl = <0x1>;
>> +		qcom,saw2-spm-cmd-wfi = [03 0b 0f];
>> +		qcom,saw2-spm-cmd-spc = [00 20 50 80 60 70 10 92
>> +				a0 b0 03 68 70 3b 92 a0 b0
>> +				82 2b 50 10 30 02 22 30 0f];
>> +	};
>> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
>> index 7dcd554..d467767 100644
>> --- a/drivers/soc/qcom/Kconfig
>> +++ b/drivers/soc/qcom/Kconfig
>> @@ -11,3 +11,11 @@ config QCOM_GSBI
>>
>> config QCOM_SCM
>> 	bool
>> +
>> +config QCOM_PM
>> +	bool "Qualcomm Power Management"
>> +	depends on PM && ARCH_QCOM && OF
>
>ARCH_QCOM implies OF
>
OK.
>> +	help
>> +	  QCOM Platform specific power driver to manage cores and L2 low power
>> +	  modes. It interface with various system drivers to put the cores in
>> +	  low power modes.
>>
>
>
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>-- 
>Employee of Qualcomm Innovation Center, Inc.
>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/arm/msm/spm.txt b/Documentation/devicetree/bindings/arm/msm/spm.txt
new file mode 100644
index 0000000..318e024
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/spm.txt
@@ -0,0 +1,47 @@ 
+* Subsystem Power Manager (SAW2)
+
+S4 generation of MSMs have SPM hardware blocks to control the Application
+Processor Sub-System power. These SPM blocks run individual state machine
+to determine what the core (L2 or Krait/Scorpion) would do when the WFI
+instruction is executed by the core.
+
+The devicetree representation of the SPM block should be:
+
+Required properties
+
+- compatible: Could be one of -
+		"qcom,spm-v2.1"
+		"qcom,spm-v3.0"
+- reg: The physical address and the size of the SPM's memory mapped registers
+- qcom,cpu: phandle for the CPU that the SPM block is attached to.
+	This field is required on only for SPMs that control the CPU.
+- qcom,saw2-cfg: SAW2 configuration register
+- qcom,saw2-spm-dly: Provides the values for the SPM delay command in the SPM
+	sequence
+- qcom,saw2-spm-ctl: The SPM control register
+
+Optional properties
+
+- qcom,saw2-spm-cmd-wfi: The WFI command sequence
+- qcom,saw2-spm-cmd-ret: The Retention command sequence
+- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
+- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence. This sequence may
+	turn off other SoC components.
+- qcom,saw2-spm-cmd-gdhs: GDHS (Globally Distributed Head Switch) command
+	sequence. This sequence will retain the memory but turn off the logic.
+-
+Example:
+	spm@f9089000 {
+		compatible = "qcom,spm-v2.1";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0xf9089000 0x1000>;
+		qcom,cpu = <&CPU0>;
+		qcom,saw2-cfg = <0x1>;
+		qcom,saw2-spm-dly= <0x20000400>;
+		qcom,saw2-spm-ctl = <0x1>;
+		qcom,saw2-spm-cmd-wfi = [03 0b 0f];
+		qcom,saw2-spm-cmd-spc = [00 20 50 80 60 70 10 92
+				a0 b0 03 68 70 3b 92 a0 b0
+				82 2b 50 10 30 02 22 30 0f];
+	};
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 7dcd554..d467767 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -11,3 +11,11 @@  config QCOM_GSBI
 
 config QCOM_SCM
 	bool
+
+config QCOM_PM
+	bool "Qualcomm Power Management"
+	depends on PM && ARCH_QCOM && OF
+	help
+	  QCOM Platform specific power driver to manage cores and L2 low power
+	  modes. It interface with various system drivers to put the cores in
+	  low power modes.
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 20b329f..9457b2a 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,4 +1,4 @@ 
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
-obj-$(CONFIG_QCOM_PM)	+=	spm.o
+obj-$(CONFIG_QCOM_PM)	+=	spm.o spm-devices.o
 CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
 obj-$(CONFIG_QCOM_SCM) += scm.o scm-boot.o
diff --git a/drivers/soc/qcom/spm-devices.c b/drivers/soc/qcom/spm-devices.c
new file mode 100644
index 0000000..2175a81
--- /dev/null
+++ b/drivers/soc/qcom/spm-devices.c
@@ -0,0 +1,198 @@ 
+/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+
+#include <soc/qcom/spm.h>
+
+#include "spm-drv.h"
+
+/**
+ * All related information for an SPM device
+ * Helps manage the collective.
+ */
+struct msm_spm_device {
+	bool initialized;
+	struct msm_spm_driver_data drv;
+};
+
+static DEFINE_PER_CPU_SHARED_ALIGNED(struct msm_spm_device, msm_cpu_spm_device);
+
+/**
+ * msm_spm_set_low_power_mode() - Configure SPM start address for low power mode
+ * @mode: SPM LPM mode to enter
+ */
+int msm_spm_set_low_power_mode(unsigned int mode)
+{
+	struct msm_spm_device *dev = &__get_cpu_var(msm_cpu_spm_device);
+	int ret = -EINVAL;
+
+	if (!dev->initialized)
+		return -ENXIO;
+
+	if (mode == MSM_SPM_MODE_DISABLED)
+		ret = msm_spm_drv_set_spm_enable(&dev->drv, false);
+	else if (!msm_spm_drv_set_spm_enable(&dev->drv, true))
+		ret = msm_spm_drv_set_low_power_mode(&dev->drv, mode);
+
+	return ret;
+}
+EXPORT_SYMBOL(msm_spm_set_low_power_mode);
+
+static int get_cpu_id(struct device_node *node)
+{
+	struct device_node *cpu_node;
+	u32 cpu;
+	int ret = -EINVAL;
+	char *key = "qcom,cpu";
+
+	cpu_node = of_parse_phandle(node, key, 0);
+	if (cpu_node) {
+		for_each_possible_cpu(cpu) {
+			if (of_get_cpu_node(cpu, NULL) == cpu_node)
+				return cpu;
+		}
+	}
+	return ret;
+}
+
+static struct msm_spm_device *msm_spm_get_device(struct platform_device *pdev)
+{
+	struct msm_spm_device *dev = NULL;
+	int cpu = get_cpu_id(pdev->dev.of_node);
+
+	if ((cpu >= 0) && cpu < num_possible_cpus())
+		dev = &per_cpu(msm_cpu_spm_device, cpu);
+
+	return dev;
+}
+
+static int msm_spm_dev_probe(struct platform_device *pdev)
+{
+	int ret;
+	int i;
+	struct device_node *node = pdev->dev.of_node;
+	char *key;
+	uint32_t val;
+	struct msm_spm_mode modes[MSM_SPM_MODE_NR];
+	struct msm_spm_device *spm_dev;
+	struct resource *res;
+	uint32_t mode_count = 0;
+
+	struct spm_of {
+		char *key;
+		uint32_t id;
+	};
+
+	/* SPM Configuration registers */
+	struct spm_of spm_of_data[] = {
+		{"qcom,saw2-clk-div", MSM_SPM_REG_SAW2_CFG},
+		{"qcom,saw2-enable", MSM_SPM_REG_SAW2_SPM_CTL},
+		{"qcom,saw2-delays", MSM_SPM_REG_SAW2_SPM_DLY},
+	};
+
+	/* SPM sleep sequences */
+	struct spm_of mode_of_data[] = {
+		{"qcom,saw2-spm-cmd-wfi", MSM_SPM_MODE_CLOCK_GATING},
+		{"qcom,saw2-spm-cmd-spc", MSM_SPM_MODE_POWER_COLLAPSE},
+		{"qcom,saw2-spm-cmd-ret", MSM_SPM_MODE_RETENTION},
+	};
+
+	 /* Get the right SPM device */
+	spm_dev = msm_spm_get_device(pdev);
+	if (IS_ERR_OR_NULL(spm_dev))
+		return -EINVAL;
+
+	/* Get the SAW start address */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -EINVAL;
+		goto fail;
+	}
+	spm_dev->drv.reg_base_addr = devm_ioremap(&pdev->dev, res->start,
+					resource_size(res));
+	if (!spm_dev->drv.reg_base_addr) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	/* Read the SPM configuration register values */
+	for (i = 0; i < ARRAY_SIZE(spm_of_data); i++) {
+		ret = of_property_read_u32(node, spm_of_data[i].key, &val);
+		if (ret)
+			continue;
+		spm_dev->drv.reg_shadow[spm_of_data[i].id] = val;
+	}
+
+	/* Read the byte arrays for the SPM sleep sequences */
+	for (i = 0; i < ARRAY_SIZE(mode_of_data); i++) {
+		modes[mode_count].start_addr = 0;
+		key = mode_of_data[i].key;
+		modes[mode_count].cmd =
+			(uint8_t *)of_get_property(node, key, &val);
+		if (!modes[mode_count].cmd)
+			continue;
+		modes[mode_count].mode = mode_of_data[i].id;
+		mode_count++;
+	}
+
+	spm_dev->drv.modes = devm_kcalloc(&pdev->dev, mode_count,
+						sizeof(modes[0]), GFP_KERNEL);
+	if (!spm_dev->drv.modes)
+		return -ENOMEM;
+	spm_dev->drv.num_modes = mode_count;
+	memcpy(spm_dev->drv.modes, &modes[0], sizeof(modes[0]) * mode_count);
+
+	/* Initialize the hardware */
+	ret = msm_spm_drv_init(&spm_dev->drv);
+	if (ret) {
+		kfree(spm_dev->drv.modes);
+		return ret;
+	}
+
+	spm_dev->initialized = true;
+	return ret;
+
+fail:
+	dev_err(&pdev->dev, "SPM device probe failed: %d\n", ret);
+	return ret;
+}
+
+static struct of_device_id msm_spm_match_table[] = {
+	{.compatible = "qcom,spm-v2.1"},
+	{},
+};
+
+static struct platform_driver msm_spm_device_driver = {
+	.probe = msm_spm_dev_probe,
+	.driver = {
+		.name = "spm-v2",
+		.owner = THIS_MODULE,
+		.of_match_table = msm_spm_match_table,
+	},
+};
+
+static int __init msm_spm_device_init(void)
+{
+	return platform_driver_register(&msm_spm_device_driver);
+}
+device_initcall(msm_spm_device_init);
diff --git a/include/soc/qcom/spm.h b/include/soc/qcom/spm.h
new file mode 100644
index 0000000..99a1d12
--- /dev/null
+++ b/include/soc/qcom/spm.h
@@ -0,0 +1,34 @@ 
+/* Copyright (c) 2010-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __QCOM_SPM_H
+#define __QCOM_SPM_H
+
+enum {
+	MSM_SPM_MODE_DISABLED,
+	MSM_SPM_MODE_CLOCK_GATING,
+	MSM_SPM_MODE_RETENTION,
+	MSM_SPM_MODE_GDHS,
+	MSM_SPM_MODE_POWER_COLLAPSE,
+	MSM_SPM_MODE_NR
+};
+
+struct msm_spm_device;
+
+#if defined(CONFIG_QCOM_PM)
+int msm_spm_set_low_power_mode(unsigned int mode);
+#else /* defined(CONFIG_QCOM_PM) */
+static inline int msm_spm_set_low_power_mode(unsigned int mode)
+{ return -ENOSYS; }
+#endif  /* defined (CONFIG_QCOM_PM) */
+
+#endif  /* __QCOM_SPM_H */