| Message ID | 20200626065738.93412-7-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Fri, Jun 26, 2020 at 07:57:35AM +0100, Lee Jones wrote: > Until now the aforementioned return value has been ignored. This is only aforementioned in the subject. > /* Store default strobe info */ > ret = tps65217_reg_read(tps, regulators[i].bypass_reg, &val); > + if (ret) > + return ret; > + We should really be logging an error there rather than just returning, that way it's a bit more apparent to someone debugging things what went wrong if there is actually a problem.
On Fri, 26 Jun 2020, Mark Brown wrote: > On Fri, Jun 26, 2020 at 07:57:35AM +0100, Lee Jones wrote: > > > Until now the aforementioned return value has been ignored. > > This is only aforementioned in the subject. > > > /* Store default strobe info */ > > ret = tps65217_reg_read(tps, regulators[i].bypass_reg, &val); > > + if (ret) > > + return ret; > > + > > We should really be logging an error there rather than just returning, > that way it's a bit more apparent to someone debugging things what went > wrong if there is actually a problem. Would you like me to fix that up subsequently? -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
On Fri, Jun 26, 2020 at 04:31:19PM +0100, Lee Jones wrote: > On Fri, 26 Jun 2020, Mark Brown wrote: > > We should really be logging an error there rather than just returning, > > that way it's a bit more apparent to someone debugging things what went > > wrong if there is actually a problem. > Would you like me to fix that up subsequently? That'd be great, yeah!
diff --git a/drivers/regulator/tps65217-regulator.c b/drivers/regulator/tps65217-regulator.c index f2d3a4a9f3e77..3f5ea029e2e3f 100644 --- a/drivers/regulator/tps65217-regulator.c +++ b/drivers/regulator/tps65217-regulator.c @@ -254,6 +254,9 @@ static int tps65217_regulator_probe(struct platform_device *pdev) /* Store default strobe info */ ret = tps65217_reg_read(tps, regulators[i].bypass_reg, &val); + if (ret) + return ret; + tps->strobes[i] = val & regulators[i].bypass_mask; }
Until now the aforementioned return value has been ignored. Previous and current calls to tps65217_reg_read() return instantly when the value is not 0, so let's do that. Fixes the following W=1 warning: drivers/regulator/tps65217-regulator.c: In function ‘tps65217_regulator_probe’: drivers/regulator/tps65217-regulator.c:227:9: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 227 | int i, ret; | ^~~ Cc: Russ Dill <Russ.Dill@ti.com> Cc: Keerthy <j-keerthy@ti.com> Cc: AnilKumar Ch <anilkumar@ti.com> Cc: linux-omap@vger.kernel.org Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/regulator/tps65217-regulator.c | 3 +++ 1 file changed, 3 insertions(+) -- 2.25.1