| Message ID | 1619161503-8784-1-git-send-email-rnayak@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | nvmem: qfprom: minor nit fixes, no functional change | expand |
Hi, On Fri, Apr 23, 2021 at 12:05 AM Rajendra Nayak <rnayak@codeaurora.org> wrote: > > Fix a missed newline, and update a comment which is stale > after the merge of '5a1bea2a: nvmem: qfprom: Add support for fuse > blowing on sc7280' > No other functional change in this patch. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > Sending a follow-up patch for these nits since they came in after > the previous patch was already pulled in > https://lore.kernel.org/patchwork/patch/1401964/ > > drivers/nvmem/qfprom.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c > index d6d3f24..b7d2060 100644 > --- a/drivers/nvmem/qfprom.c > +++ b/drivers/nvmem/qfprom.c > @@ -122,6 +122,7 @@ static const struct qfprom_soc_compatible_data sc7280_qfprom = { > .keepout = sc7280_qfprom_keepout, > .nkeepout = ARRAY_SIZE(sc7280_qfprom_keepout) > }; > + > /** > * qfprom_disable_fuse_blowing() - Undo enabling of fuse blowing. > * @priv: Our driver data. > @@ -195,7 +196,7 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv, > } > > /* > - * Hardware requires 1.8V min for fuse blowing; this may be > + * Hardware requires a min voltage for fuse blowing; this may be > * a rail shared do don't specify a max--regulator constraints > * will handle. > */ Thanks for sending these! Is there a reason you didn't do the "else if" change too? -Doug
On 4/23/2021 8:34 PM, Doug Anderson wrote: > Hi, > > On Fri, Apr 23, 2021 at 12:05 AM Rajendra Nayak <rnayak@codeaurora.org> wrote: >> >> Fix a missed newline, and update a comment which is stale >> after the merge of '5a1bea2a: nvmem: qfprom: Add support for fuse >> blowing on sc7280' >> No other functional change in this patch. >> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> --- >> Sending a follow-up patch for these nits since they came in after >> the previous patch was already pulled in >> https://lore.kernel.org/patchwork/patch/1401964/ >> >> drivers/nvmem/qfprom.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c >> index d6d3f24..b7d2060 100644 >> --- a/drivers/nvmem/qfprom.c >> +++ b/drivers/nvmem/qfprom.c >> @@ -122,6 +122,7 @@ static const struct qfprom_soc_compatible_data sc7280_qfprom = { >> .keepout = sc7280_qfprom_keepout, >> .nkeepout = ARRAY_SIZE(sc7280_qfprom_keepout) >> }; >> + >> /** >> * qfprom_disable_fuse_blowing() - Undo enabling of fuse blowing. >> * @priv: Our driver data. >> @@ -195,7 +196,7 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv, >> } >> >> /* >> - * Hardware requires 1.8V min for fuse blowing; this may be >> + * Hardware requires a min voltage for fuse blowing; this may be >> * a rail shared do don't specify a max--regulator constraints >> * will handle. >> */ > > Thanks for sending these! Is there a reason you didn't do the "else > if" change too? Oops, sorry, looks like i didn't scroll all the way down. Sending a v2 shortly. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Hi, On Fri, Apr 23, 2021 at 8:13 AM Joe Perches <joe@perches.com> wrote: > > On Fri, 2021-04-23 at 12:35 +0530, Rajendra Nayak wrote: > > Fix a missed newline, and update a comment which is stale > > after the merge of '5a1bea2a: nvmem: qfprom: Add support for fuse > > blowing on sc7280' > > No other functional change in this patch. > [] > > diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c > [] > > @@ -195,7 +196,7 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv, > > } > > > > /* > > - * Hardware requires 1.8V min for fuse blowing; this may be > > + * Hardware requires a min voltage for fuse blowing; this may be > > * a rail shared do don't specify a max--regulator constraints > > so don't? > > > * will handle. > > */ > > but the comment doesn't make much overall sense to me. What part of it doesn't make sense, exactly? Basically there's an external regulator (voltage rail) that can be programmed to a range of voltages. For simplicity, let's assume 1.7, 1.8, and 1.9 V. That regulator powers several things on the board / SoC. The places where the rail is hooked up can function over some range of voltages. Maybe some of the things it's hooked up with function fine between 1.6V and 1.9 V. Some between 1.6V and 1.8V. Some between 1.8V and 1.9V. For power savings we generally want the voltage rail to be as low as possible. In this case, the efuse hardware (and everything else the rail is hooked up to) work fine with a lower voltage, except when you're programming the efuse. When the efuse needs to be programmed then it needs a higher voltage. Thus, normally, the system is allowed to run this rail at some lower voltage. Maybe 1.7V. When we want to program, though, we've got to run it at 1.8V (or something higher on a new SoC). In general it's up to the board-level regulator constraints to keep everything happy. The person designing the board should setup a minimum and maximum voltage for each rail to make sure that no components are run at too high of a voltage or too low of a voltage. Presumably those board-level constraints take into account the voltage level needed to read the efuse and also take into account the maximum voltage so we don't damage the efuse. -Doug
On Mon, 2021-04-26 at 08:11 -0700, Doug Anderson wrote: > Hi, > > On Fri, Apr 23, 2021 at 8:13 AM Joe Perches <joe@perches.com> wrote: > > > > On Fri, 2021-04-23 at 12:35 +0530, Rajendra Nayak wrote: > > > Fix a missed newline, and update a comment which is stale > > > after the merge of '5a1bea2a: nvmem: qfprom: Add support for fuse > > > blowing on sc7280' > > > No other functional change in this patch. > > [] > > > diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c > > [] > > > @@ -195,7 +196,7 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv, > > > } > > > > > > /* > > > - * Hardware requires 1.8V min for fuse blowing; this may be > > > + * Hardware requires a min voltage for fuse blowing; this may be > > > * a rail shared do don't specify a max--regulator constraints > > > > so don't? > > > > > * will handle. > > > */ > > > > but the comment doesn't make much overall sense to me. > > What part of it doesn't make sense, exactly? max--regulator I think that should be something like: Hardware requires a minimum voltage for fuse blowing. This may be a shared rail so don't specify a maximum. Regulator constraints will handle (what exactly, it?)
diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c index d6d3f24..b7d2060 100644 --- a/drivers/nvmem/qfprom.c +++ b/drivers/nvmem/qfprom.c @@ -122,6 +122,7 @@ static const struct qfprom_soc_compatible_data sc7280_qfprom = { .keepout = sc7280_qfprom_keepout, .nkeepout = ARRAY_SIZE(sc7280_qfprom_keepout) }; + /** * qfprom_disable_fuse_blowing() - Undo enabling of fuse blowing. * @priv: Our driver data. @@ -195,7 +196,7 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv, } /* - * Hardware requires 1.8V min for fuse blowing; this may be + * Hardware requires a min voltage for fuse blowing; this may be * a rail shared do don't specify a max--regulator constraints * will handle. */
Fix a missed newline, and update a comment which is stale after the merge of '5a1bea2a: nvmem: qfprom: Add support for fuse blowing on sc7280' No other functional change in this patch. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- Sending a follow-up patch for these nits since they came in after the previous patch was already pulled in https://lore.kernel.org/patchwork/patch/1401964/ drivers/nvmem/qfprom.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)