2021-07-22 05:29:57

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH 0/3] nvmem: qfprom: Add binding updates and power-domain handling

qfprom devices on sc7280 have an additional requirement to vote on a power-domain
performance state to reliably blow fuses. Add the binding updates and handle this in
the driver, also add the DT node for sc7280 platform.

Rajendra Nayak (3):
dt-bindings: nvmem: qfprom: Add optional power-domains property
nvmem: qfprom: sc7280: Handle the additional power-domains vote
arm64: dts: qcom: sc7280: Add qfprom node

.../devicetree/bindings/nvmem/qcom,qfprom.yaml | 3 +++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 13 +++++++++++++
drivers/nvmem/qfprom.c | 22 ++++++++++++++++++++++
3 files changed, 38 insertions(+)

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2021-07-22 05:30:49

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH 3/3] arm64: dts: qcom: sc7280: Add qfprom node

Add the qfprom node and its properties for the sc7280 SoC.

Signed-off-by: Rajendra Nayak <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index a8c274a..8d2ffbd 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -436,6 +436,19 @@
#mbox-cells = <2>;
};

+ qfprom: efuse@784000 {
+ compatible = "qcom,sc7280-qfprom", "qcom,qfprom";
+ reg = <0 0x00784000 0 0xa20>,
+ <0 0x00780000 0 0xa20>,
+ <0 0x00782000 0 0x120>,
+ <0 0x00786000 0 0x1fff>;
+ clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
+ clock-names = "core";
+ power-domains = <&rpmhpd SC7280_MX>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ };
+
qupv3_id_0: geniqup@9c0000 {
compatible = "qcom,geni-se-qup";
reg = <0 0x009c0000 0 0x2000>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2021-07-22 05:30:56

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: nvmem: qfprom: Add optional power-domains property

qfprom devices on some SoCs need to vote on the performance state
of a power-domain, so add the power-domains optional property to the
bindings

Signed-off-by: Rajendra Nayak <[email protected]>
---
Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
index 861b205..a498a08 100644
--- a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
+++ b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
@@ -51,6 +51,9 @@ properties:
vcc-supply:
description: Our power supply.

+ power-domains:
+ description: A phandle to a power domain node.
+
# Needed if any child nodes are present.
"#address-cells":
const: 1
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2021-07-22 05:31:46

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH 2/3] nvmem: qfprom: sc7280: Handle the additional power-domains vote

On sc7280, to reliably blow fuses, we need an additional vote
on max performance state of 'MX' power-domain.
Add support for power-domain performance state voting in the
driver.

Signed-off-by: Rajendra Nayak <[email protected]>
---
drivers/nvmem/qfprom.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 81fbad5..4d0a576 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -12,6 +12,8 @@
#include <linux/mod_devicetable.h>
#include <linux/nvmem-provider.h>
#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
#include <linux/property.h>
#include <linux/regulator/consumer.h>

@@ -149,6 +151,11 @@ static void qfprom_disable_fuse_blowing(const struct qfprom_priv *priv,
if (ret)
dev_warn(priv->dev, "Failed to set 0 voltage (ignoring)\n");

+ if (priv->dev->pm_domain) {
+ dev_pm_genpd_set_performance_state(priv->dev, 0);
+ pm_runtime_put(priv->dev);
+ }
+
ret = regulator_disable(priv->vcc);
if (ret)
dev_warn(priv->dev, "Failed to disable regulator (ignoring)\n");
@@ -212,6 +219,16 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
goto err_clk_rate_set;
}

+ if (priv->dev->pm_domain) {
+ ret = pm_runtime_get_sync(priv->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(priv->dev);
+ dev_err(priv->dev, "Failed to enable power-domain\n");
+ goto err_reg_enable;
+ }
+ dev_pm_genpd_set_performance_state(priv->dev, INT_MAX);
+ }
+
old->timer_val = readl(priv->qfpconf + QFPROM_BLOW_TIMER_OFFSET);
old->accel_val = readl(priv->qfpconf + QFPROM_ACCEL_OFFSET);
writel(priv->soc_data->qfprom_blow_timer_value,
@@ -221,6 +238,8 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,

return 0;

+err_reg_enable:
+ regulator_disable(priv->vcc);
err_clk_rate_set:
clk_set_rate(priv->secclk, old->clk_rate);
err_clk_prepared:
@@ -420,6 +439,9 @@ static int qfprom_probe(struct platform_device *pdev)
econfig.reg_write = qfprom_reg_write;
}

+ if (dev->pm_domain)
+ pm_runtime_enable(dev);
+
nvmem = devm_nvmem_register(dev, &econfig);

return PTR_ERR_OR_ZERO(nvmem);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2021-07-23 16:47:25

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 2/3] nvmem: qfprom: sc7280: Handle the additional power-domains vote

Hi,

On Wed, Jul 21, 2021 at 10:29 PM Rajendra Nayak <[email protected]> wrote:
>
> On sc7280, to reliably blow fuses, we need an additional vote
> on max performance state of 'MX' power-domain.
> Add support for power-domain performance state voting in the
> driver.
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> ---
> drivers/nvmem/qfprom.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> index 81fbad5..4d0a576 100644
> --- a/drivers/nvmem/qfprom.c
> +++ b/drivers/nvmem/qfprom.c
> @@ -12,6 +12,8 @@
> #include <linux/mod_devicetable.h>
> #include <linux/nvmem-provider.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> #include <linux/property.h>
> #include <linux/regulator/consumer.h>
>
> @@ -149,6 +151,11 @@ static void qfprom_disable_fuse_blowing(const struct qfprom_priv *priv,
> if (ret)
> dev_warn(priv->dev, "Failed to set 0 voltage (ignoring)\n");
>
> + if (priv->dev->pm_domain) {
> + dev_pm_genpd_set_performance_state(priv->dev, 0);
> + pm_runtime_put(priv->dev);
> + }
> +
> ret = regulator_disable(priv->vcc);
> if (ret)
> dev_warn(priv->dev, "Failed to disable regulator (ignoring)\n");
> @@ -212,6 +219,16 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
> goto err_clk_rate_set;
> }
>
> + if (priv->dev->pm_domain) {
> + ret = pm_runtime_get_sync(priv->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(priv->dev);
> + dev_err(priv->dev, "Failed to enable power-domain\n");
> + goto err_reg_enable;
> + }
> + dev_pm_genpd_set_performance_state(priv->dev, INT_MAX);
> + }
> +
> old->timer_val = readl(priv->qfpconf + QFPROM_BLOW_TIMER_OFFSET);
> old->accel_val = readl(priv->qfpconf + QFPROM_ACCEL_OFFSET);
> writel(priv->soc_data->qfprom_blow_timer_value,
> @@ -221,6 +238,8 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
>
> return 0;
>
> +err_reg_enable:
> + regulator_disable(priv->vcc);
> err_clk_rate_set:
> clk_set_rate(priv->secclk, old->clk_rate);
> err_clk_prepared:
> @@ -420,6 +439,9 @@ static int qfprom_probe(struct platform_device *pdev)
> econfig.reg_write = qfprom_reg_write;
> }
>
> + if (dev->pm_domain)
> + pm_runtime_enable(dev);
> +

Where is the matching pm_runtime_disable()? Should be one in
.remove(), or use devm_add_action_or_reset() to wrap a call to it.

Also: do you really need to test for dev->pm_domain in your patch?
Seems like it should always be fine to call pm_runtime_enable() and
then always fine to call the get/put. ...and presumably always fine to
even set the performance state?

-Doug

2021-07-27 12:24:49

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH 2/3] nvmem: qfprom: sc7280: Handle the additional power-domains vote


On 7/23/2021 10:13 PM, Doug Anderson wrote:
> Hi,
>
> On Wed, Jul 21, 2021 at 10:29 PM Rajendra Nayak <[email protected]> wrote:
>>
>> On sc7280, to reliably blow fuses, we need an additional vote
>> on max performance state of 'MX' power-domain.
>> Add support for power-domain performance state voting in the
>> driver.
>>
>> Signed-off-by: Rajendra Nayak <[email protected]>
>> ---
>> drivers/nvmem/qfprom.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
>> index 81fbad5..4d0a576 100644
>> --- a/drivers/nvmem/qfprom.c
>> +++ b/drivers/nvmem/qfprom.c
>> @@ -12,6 +12,8 @@
>> #include <linux/mod_devicetable.h>
>> #include <linux/nvmem-provider.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/pm_runtime.h>
>> #include <linux/property.h>
>> #include <linux/regulator/consumer.h>
>>
>> @@ -149,6 +151,11 @@ static void qfprom_disable_fuse_blowing(const struct qfprom_priv *priv,
>> if (ret)
>> dev_warn(priv->dev, "Failed to set 0 voltage (ignoring)\n");
>>
>> + if (priv->dev->pm_domain) {
>> + dev_pm_genpd_set_performance_state(priv->dev, 0);
>> + pm_runtime_put(priv->dev);
>> + }
>> +
>> ret = regulator_disable(priv->vcc);
>> if (ret)
>> dev_warn(priv->dev, "Failed to disable regulator (ignoring)\n");
>> @@ -212,6 +219,16 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
>> goto err_clk_rate_set;
>> }
>>
>> + if (priv->dev->pm_domain) {
>> + ret = pm_runtime_get_sync(priv->dev);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(priv->dev);
>> + dev_err(priv->dev, "Failed to enable power-domain\n");
>> + goto err_reg_enable;
>> + }
>> + dev_pm_genpd_set_performance_state(priv->dev, INT_MAX);
>> + }
>> +
>> old->timer_val = readl(priv->qfpconf + QFPROM_BLOW_TIMER_OFFSET);
>> old->accel_val = readl(priv->qfpconf + QFPROM_ACCEL_OFFSET);
>> writel(priv->soc_data->qfprom_blow_timer_value,
>> @@ -221,6 +238,8 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
>>
>> return 0;
>>
>> +err_reg_enable:
>> + regulator_disable(priv->vcc);
>> err_clk_rate_set:
>> clk_set_rate(priv->secclk, old->clk_rate);
>> err_clk_prepared:
>> @@ -420,6 +439,9 @@ static int qfprom_probe(struct platform_device *pdev)
>> econfig.reg_write = qfprom_reg_write;
>> }
>>
>> + if (dev->pm_domain)
>> + pm_runtime_enable(dev);
>> +
>
> Where is the matching pm_runtime_disable()? Should be one in
> .remove(), or use devm_add_action_or_reset() to wrap a call to it.

Ah, right, i need to handle that.

>
> Also: do you really need to test for dev->pm_domain in your patch?
> Seems like it should always be fine to call pm_runtime_enable() and
> then always fine to call the get/put. ...and presumably always fine to
> even set the performance state?

Sure, i'll give it a try and see if that works or ends up throwing me
any warns, i'll repost with that or update if that does not work for
some reason. thanks for the review.

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation