2021-02-05 11:34:11

by Ravi Kumar Bokka

[permalink] [raw]
Subject: [PATCH] drivers: nvmem: Fix voltage settings for QTI qfprom-efuse

QFPROM controller hardware requires 1.8V min for fuse blowing.
So, this change sets the voltage to 1.8V, required to blow the fuse
for qfprom-efuse controller.

To disable fuse blowing, we set the voltage to 0V since this may
be a shared rail and may be able to run at a lower rate when we're
not blowing fuses.

Fixes: 93b4e49f8c86 ("nvmem: qfprom: Add fuse blowing support")
Reported-by: Douglas Anderson <[email protected]>
Suggested-by: Douglas Anderson <[email protected]>
Signed-off-by: Ravi Kumar Bokka <[email protected]>
---
drivers/nvmem/qfprom.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 6cace24..100d69d 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -127,6 +127,16 @@ static void qfprom_disable_fuse_blowing(const struct qfprom_priv *priv,
{
int ret;

+ /*
+ * This may be a shared rail and may be able to run at a lower rate
+ * when we're not blowing fuses. At the moment, the regulator framework
+ * applies voltage constraints even on disabled rails, so remove our
+ * constraints and allow the rail to be adjusted by other users.
+ */
+ ret = regulator_set_voltage(priv->vcc, 0, INT_MAX);
+ if (ret)
+ dev_warn(priv->dev, "Failed to set 0 voltage (ignoring)\n");
+
ret = regulator_disable(priv->vcc);
if (ret)
dev_warn(priv->dev, "Failed to disable regulator (ignoring)\n");
@@ -172,6 +182,17 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
goto err_clk_prepared;
}

+ /*
+ * Hardware requires 1.8V min for fuse blowing; this may be
+ * a rail shared do don't specify a max--regulator constraints
+ * will handle.
+ */
+ ret = regulator_set_voltage(priv->vcc, 1800000, INT_MAX);
+ if (ret) {
+ dev_err(priv->dev, "Failed to set 1.8 voltage\n");
+ goto err_clk_rate_set;
+ }
+
ret = regulator_enable(priv->vcc);
if (ret) {
dev_err(priv->dev, "Failed to enable regulator\n");
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.


2021-02-05 23:30:18

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] drivers: nvmem: Fix voltage settings for QTI qfprom-efuse

Hi,

On Fri, Feb 5, 2021 at 3:29 AM Ravi Kumar Bokka <[email protected]> wrote:
>
> QFPROM controller hardware requires 1.8V min for fuse blowing.
> So, this change sets the voltage to 1.8V, required to blow the fuse
> for qfprom-efuse controller.
>
> To disable fuse blowing, we set the voltage to 0V since this may
> be a shared rail and may be able to run at a lower rate when we're
> not blowing fuses.
>
> Fixes: 93b4e49f8c86 ("nvmem: qfprom: Add fuse blowing support")
> Reported-by: Douglas Anderson <[email protected]>
> Suggested-by: Douglas Anderson <[email protected]>
> Signed-off-by: Ravi Kumar Bokka <[email protected]>
> ---
> drivers/nvmem/qfprom.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> index 6cace24..100d69d 100644
> --- a/drivers/nvmem/qfprom.c
> +++ b/drivers/nvmem/qfprom.c
> @@ -127,6 +127,16 @@ static void qfprom_disable_fuse_blowing(const struct qfprom_priv *priv,
> {
> int ret;
>
> + /*
> + * This may be a shared rail and may be able to run at a lower rate
> + * when we're not blowing fuses. At the moment, the regulator framework
> + * applies voltage constraints even on disabled rails, so remove our
> + * constraints and allow the rail to be adjusted by other users.

Some year maybe I'll try to fix the regulator framework to not count
voltage constraints for disbled rails, or perhaps have it be optional.
;-) In theory it should be much easier after the patches we already
landed not to count current requests for disabled rails...


> + */
> + ret = regulator_set_voltage(priv->vcc, 0, INT_MAX);
> + if (ret)
> + dev_warn(priv->dev, "Failed to set 0 voltage (ignoring)\n");
> +
> ret = regulator_disable(priv->vcc);
> if (ret)
> dev_warn(priv->dev, "Failed to disable regulator (ignoring)\n");
> @@ -172,6 +182,17 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
> goto err_clk_prepared;
> }
>
> + /*
> + * Hardware requires 1.8V min for fuse blowing; this may be
> + * a rail shared do don't specify a max--regulator constraints
> + * will handle.
> + */
> + ret = regulator_set_voltage(priv->vcc, 1800000, INT_MAX);
> + if (ret) {
> + dev_err(priv->dev, "Failed to set 1.8 voltage\n");
> + goto err_clk_rate_set;
> + }
> +

Looks right to me. Assuming that this works.

Reviewed-by: Douglas Anderson <[email protected]>

2021-03-25 07:18:46

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH] drivers: nvmem: Fix voltage settings for QTI qfprom-efuse


On 2/5/2021 8:25 PM, Doug Anderson wrote:
> Hi,
>
> On Fri, Feb 5, 2021 at 3:29 AM Ravi Kumar Bokka <[email protected]> wrote:
>>
>> QFPROM controller hardware requires 1.8V min for fuse blowing.
>> So, this change sets the voltage to 1.8V, required to blow the fuse
>> for qfprom-efuse controller.
>>
>> To disable fuse blowing, we set the voltage to 0V since this may
>> be a shared rail and may be able to run at a lower rate when we're
>> not blowing fuses.
>>
>> Fixes: 93b4e49f8c86 ("nvmem: qfprom: Add fuse blowing support")
>> Reported-by: Douglas Anderson <[email protected]>
>> Suggested-by: Douglas Anderson <[email protected]>
>> Signed-off-by: Ravi Kumar Bokka <[email protected]>
>> ---
>> drivers/nvmem/qfprom.c | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
>> index 6cace24..100d69d 100644
>> --- a/drivers/nvmem/qfprom.c
>> +++ b/drivers/nvmem/qfprom.c
>> @@ -127,6 +127,16 @@ static void qfprom_disable_fuse_blowing(const struct qfprom_priv *priv,
>> {
>> int ret;
>>
>> + /*
>> + * This may be a shared rail and may be able to run at a lower rate
>> + * when we're not blowing fuses. At the moment, the regulator framework
>> + * applies voltage constraints even on disabled rails, so remove our
>> + * constraints and allow the rail to be adjusted by other users.
>
> Some year maybe I'll try to fix the regulator framework to not count
> voltage constraints for disbled rails, or perhaps have it be optional.
> ;-) In theory it should be much easier after the patches we already
> landed not to count current requests for disabled rails...
>
>
>> + */
>> + ret = regulator_set_voltage(priv->vcc, 0, INT_MAX);
>> + if (ret)
>> + dev_warn(priv->dev, "Failed to set 0 voltage (ignoring)\n");
>> +
>> ret = regulator_disable(priv->vcc);
>> if (ret)
>> dev_warn(priv->dev, "Failed to disable regulator (ignoring)\n");
>> @@ -172,6 +182,17 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
>> goto err_clk_prepared;
>> }
>>
>> + /*
>> + * Hardware requires 1.8V min for fuse blowing; this may be
>> + * a rail shared do don't specify a max--regulator constraints
>> + * will handle.
>> + */
>> + ret = regulator_set_voltage(priv->vcc, 1800000, INT_MAX);
>> + if (ret) {
>> + dev_err(priv->dev, "Failed to set 1.8 voltage\n");
>> + goto err_clk_rate_set;
>> + }
>> +
>
> Looks right to me. Assuming that this works.
>
> Reviewed-by: Douglas Anderson <[email protected]>

Srini, any plans to queue this up for merge?

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

2021-03-25 14:40:49

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH] drivers: nvmem: Fix voltage settings for QTI qfprom-efuse



On 25/03/2021 07:15, Rajendra Nayak wrote:
>>>
>>
>> Looks right to me.  Assuming that this works.
>>
>> Reviewed-by: Douglas Anderson <[email protected]>
>
> Srini, any plans to queue this up for merge?

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/nvmem?id=4d57a383a437e63909ccfe15b8a116f7ff5bb673

This is already in next queued up for next merge.

thanks,
srini