| Message ID | 20210521134437.v2.1.Id1c70158722750aec0673d60c12e46a9c66bbfed@changeid |
|---|---|
| State | Accepted |
| Commit | 6feba6a62c577e98bd9214b73c17860166ac8b91 |
| Headers | show |
| Series | [v2] PM: AVS: qcom-cpr: Use nvmem_cell_read_variable_le_u32() | expand |
On Fri, May 21, 2021 at 01:44:53PM -0700, Douglas Anderson wrote: > Let's delete the private function cpr_read_efuse() since it does the > basically the same thing as the new API call > nvmem_cell_read_variable_le_u32(). > > Differences between the new API call and the old private function: > * less error printing (I assume this is OK). > * will give an error if the value doesn't fit in 32-bits (the old code > would have truncated silently). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > I haven't done any more than compile-test this. Mostly I'm just > writing this patch because it helped provide inspiration for the > general API function. > > Changes in v2: > - Resending v1 as a singleton patch; dependency is merged in mainline. > > drivers/soc/qcom/cpr.c | 43 +++++------------------------------------- > 1 file changed, 5 insertions(+), 38 deletions(-) Acked-by: Niklas Cassel <nks@flawful.org>
On Fri 21 May 15:44 CDT 2021, Douglas Anderson wrote: > Let's delete the private function cpr_read_efuse() since it does the > basically the same thing as the new API call > nvmem_cell_read_variable_le_u32(). > > Differences between the new API call and the old private function: > * less error printing (I assume this is OK). > * will give an error if the value doesn't fit in 32-bits (the old code > would have truncated silently). > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > I haven't done any more than compile-test this. Mostly I'm just > writing this patch because it helped provide inspiration for the > general API function. > > Changes in v2: > - Resending v1 as a singleton patch; dependency is merged in mainline. > > drivers/soc/qcom/cpr.c | 43 +++++------------------------------------- > 1 file changed, 5 insertions(+), 38 deletions(-) > > diff --git a/drivers/soc/qcom/cpr.c b/drivers/soc/qcom/cpr.c > index b24cc77d1889..4ce8e816154f 100644 > --- a/drivers/soc/qcom/cpr.c > +++ b/drivers/soc/qcom/cpr.c > @@ -801,38 +801,6 @@ static int cpr_set_performance_state(struct generic_pm_domain *domain, > return ret; > } > > -static int cpr_read_efuse(struct device *dev, const char *cname, u32 *data) > -{ > - struct nvmem_cell *cell; > - ssize_t len; > - char *ret; > - int i; > - > - *data = 0; > - > - cell = nvmem_cell_get(dev, cname); > - if (IS_ERR(cell)) { > - if (PTR_ERR(cell) != -EPROBE_DEFER) > - dev_err(dev, "undefined cell %s\n", cname); > - return PTR_ERR(cell); > - } > - > - ret = nvmem_cell_read(cell, &len); > - nvmem_cell_put(cell); > - if (IS_ERR(ret)) { > - dev_err(dev, "can't read cell %s\n", cname); > - return PTR_ERR(ret); > - } > - > - for (i = 0; i < len; i++) > - *data |= ret[i] << (8 * i); > - > - kfree(ret); > - dev_dbg(dev, "efuse read(%s) = %x, bytes %zd\n", cname, *data, len); > - > - return 0; > -} > - > static int > cpr_populate_ring_osc_idx(struct cpr_drv *drv) > { > @@ -843,8 +811,7 @@ cpr_populate_ring_osc_idx(struct cpr_drv *drv) > int ret; > > for (; fuse < end; fuse++, fuses++) { > - ret = cpr_read_efuse(drv->dev, fuses->ring_osc, > - &data); > + ret = nvmem_cell_read_variable_le_u32(drv->dev, fuses->ring_osc, &data); > if (ret) > return ret; > fuse->ring_osc_idx = data; > @@ -863,7 +830,7 @@ static int cpr_read_fuse_uV(const struct cpr_desc *desc, > u32 bits = 0; > int ret; > > - ret = cpr_read_efuse(drv->dev, init_v_efuse, &bits); > + ret = nvmem_cell_read_variable_le_u32(drv->dev, init_v_efuse, &bits); > if (ret) > return ret; > > @@ -932,7 +899,7 @@ static int cpr_fuse_corner_init(struct cpr_drv *drv) > } > > /* Populate target quotient by scaling */ > - ret = cpr_read_efuse(drv->dev, fuses->quotient, &fuse->quot); > + ret = nvmem_cell_read_variable_le_u32(drv->dev, fuses->quotient, &fuse->quot); > if (ret) > return ret; > > @@ -1001,7 +968,7 @@ static int cpr_calculate_scaling(const char *quot_offset, > prev_fuse = fuse - 1; > > if (quot_offset) { > - ret = cpr_read_efuse(drv->dev, quot_offset, "_diff); > + ret = nvmem_cell_read_variable_le_u32(drv->dev, quot_offset, "_diff); > if (ret) > return ret; > > @@ -1701,7 +1668,7 @@ static int cpr_probe(struct platform_device *pdev) > * initialized after attaching to the power domain, > * since it depends on the CPU's OPP table. > */ > - ret = cpr_read_efuse(dev, "cpr_fuse_revision", &cpr_rev); > + ret = nvmem_cell_read_variable_le_u32(dev, "cpr_fuse_revision", &cpr_rev); > if (ret) > return ret; > > -- > 2.31.1.818.g46aad6cb9e-goog >
Rafael / Bjorn, On Fri, May 21, 2021 at 1:45 PM Douglas Anderson <dianders@chromium.org> wrote: > > Let's delete the private function cpr_read_efuse() since it does the > basically the same thing as the new API call > nvmem_cell_read_variable_le_u32(). > > Differences between the new API call and the old private function: > * less error printing (I assume this is OK). > * will give an error if the value doesn't fit in 32-bits (the old code > would have truncated silently). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > I haven't done any more than compile-test this. Mostly I'm just > writing this patch because it helped provide inspiration for the > general API function. > > Changes in v2: > - Resending v1 as a singleton patch; dependency is merged in mainline. > > drivers/soc/qcom/cpr.c | 43 +++++------------------------------------- > 1 file changed, 5 insertions(+), 38 deletions(-) Are either of you interested in landing this? I guess Rafael landed most of the recent changes to this driver, but it used to be under the 'power' directory. Now that it's under 'drivers/soc/qcom' maybe it should go through Bjorn's tree? -Doug
On Sat, 24 Jul 2021 at 01:05, Doug Anderson <dianders@chromium.org> wrote: > > Rafael / Bjorn, > > On Fri, May 21, 2021 at 1:45 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > Let's delete the private function cpr_read_efuse() since it does the > > basically the same thing as the new API call > > nvmem_cell_read_variable_le_u32(). > > > > Differences between the new API call and the old private function: > > * less error printing (I assume this is OK). > > * will give an error if the value doesn't fit in 32-bits (the old code > > would have truncated silently). > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > I haven't done any more than compile-test this. Mostly I'm just > > writing this patch because it helped provide inspiration for the > > general API function. > > > > Changes in v2: > > - Resending v1 as a singleton patch; dependency is merged in mainline. > > > > drivers/soc/qcom/cpr.c | 43 +++++------------------------------------- > > 1 file changed, 5 insertions(+), 38 deletions(-) > > Are either of you interested in landing this? I guess Rafael landed > most of the recent changes to this driver, but it used to be under the > 'power' directory. Now that it's under 'drivers/soc/qcom' maybe it > should go through Bjorn's tree? Yes, that was exactly the intent of moving the files. I guess Bjorn has simply been busy. Kind regards Uffe
On Fri 23 Jul 18:05 CDT 2021, Doug Anderson wrote: > Rafael / Bjorn, > > On Fri, May 21, 2021 at 1:45 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > Let's delete the private function cpr_read_efuse() since it does the > > basically the same thing as the new API call > > nvmem_cell_read_variable_le_u32(). > > > > Differences between the new API call and the old private function: > > * less error printing (I assume this is OK). > > * will give an error if the value doesn't fit in 32-bits (the old code > > would have truncated silently). > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > I haven't done any more than compile-test this. Mostly I'm just > > writing this patch because it helped provide inspiration for the > > general API function. > > > > Changes in v2: > > - Resending v1 as a singleton patch; dependency is merged in mainline. > > > > drivers/soc/qcom/cpr.c | 43 +++++------------------------------------- > > 1 file changed, 5 insertions(+), 38 deletions(-) > > Are either of you interested in landing this? I guess Rafael landed > most of the recent changes to this driver, but it used to be under the > 'power' directory. Now that it's under 'drivers/soc/qcom' maybe it > should go through Bjorn's tree? > Yes, it makes sense to pick it up through my tree, unfortunately I forgot about this move and ignored the patch in my queue based on the $subject. I've picked it up now. Regards, Bjorn
diff --git a/drivers/soc/qcom/cpr.c b/drivers/soc/qcom/cpr.c index b24cc77d1889..4ce8e816154f 100644 --- a/drivers/soc/qcom/cpr.c +++ b/drivers/soc/qcom/cpr.c @@ -801,38 +801,6 @@ static int cpr_set_performance_state(struct generic_pm_domain *domain, return ret; } -static int cpr_read_efuse(struct device *dev, const char *cname, u32 *data) -{ - struct nvmem_cell *cell; - ssize_t len; - char *ret; - int i; - - *data = 0; - - cell = nvmem_cell_get(dev, cname); - if (IS_ERR(cell)) { - if (PTR_ERR(cell) != -EPROBE_DEFER) - dev_err(dev, "undefined cell %s\n", cname); - return PTR_ERR(cell); - } - - ret = nvmem_cell_read(cell, &len); - nvmem_cell_put(cell); - if (IS_ERR(ret)) { - dev_err(dev, "can't read cell %s\n", cname); - return PTR_ERR(ret); - } - - for (i = 0; i < len; i++) - *data |= ret[i] << (8 * i); - - kfree(ret); - dev_dbg(dev, "efuse read(%s) = %x, bytes %zd\n", cname, *data, len); - - return 0; -} - static int cpr_populate_ring_osc_idx(struct cpr_drv *drv) { @@ -843,8 +811,7 @@ cpr_populate_ring_osc_idx(struct cpr_drv *drv) int ret; for (; fuse < end; fuse++, fuses++) { - ret = cpr_read_efuse(drv->dev, fuses->ring_osc, - &data); + ret = nvmem_cell_read_variable_le_u32(drv->dev, fuses->ring_osc, &data); if (ret) return ret; fuse->ring_osc_idx = data; @@ -863,7 +830,7 @@ static int cpr_read_fuse_uV(const struct cpr_desc *desc, u32 bits = 0; int ret; - ret = cpr_read_efuse(drv->dev, init_v_efuse, &bits); + ret = nvmem_cell_read_variable_le_u32(drv->dev, init_v_efuse, &bits); if (ret) return ret; @@ -932,7 +899,7 @@ static int cpr_fuse_corner_init(struct cpr_drv *drv) } /* Populate target quotient by scaling */ - ret = cpr_read_efuse(drv->dev, fuses->quotient, &fuse->quot); + ret = nvmem_cell_read_variable_le_u32(drv->dev, fuses->quotient, &fuse->quot); if (ret) return ret; @@ -1001,7 +968,7 @@ static int cpr_calculate_scaling(const char *quot_offset, prev_fuse = fuse - 1; if (quot_offset) { - ret = cpr_read_efuse(drv->dev, quot_offset, "_diff); + ret = nvmem_cell_read_variable_le_u32(drv->dev, quot_offset, "_diff); if (ret) return ret; @@ -1701,7 +1668,7 @@ static int cpr_probe(struct platform_device *pdev) * initialized after attaching to the power domain, * since it depends on the CPU's OPP table. */ - ret = cpr_read_efuse(dev, "cpr_fuse_revision", &cpr_rev); + ret = nvmem_cell_read_variable_le_u32(dev, "cpr_fuse_revision", &cpr_rev); if (ret) return ret;
Let's delete the private function cpr_read_efuse() since it does the basically the same thing as the new API call nvmem_cell_read_variable_le_u32(). Differences between the new API call and the old private function: * less error printing (I assume this is OK). * will give an error if the value doesn't fit in 32-bits (the old code would have truncated silently). Signed-off-by: Douglas Anderson <dianders@chromium.org> --- I haven't done any more than compile-test this. Mostly I'm just writing this patch because it helped provide inspiration for the general API function. Changes in v2: - Resending v1 as a singleton patch; dependency is merged in mainline. drivers/soc/qcom/cpr.c | 43 +++++------------------------------------- 1 file changed, 5 insertions(+), 38 deletions(-)