Hi Thierry, On Mon, Mar 22, 2021 at 08:58:35AM +0100, Thierry Reding wrote: > On Thu, Dec 17, 2020 at 12:10:10PM -0500, Sven Van Asbroeck wrote: > > On Thu, Dec 17, 2020 at 11:48 AM Clemens Gruber > > <clemens.gru...@pqgruber.com> wrote: > > > > > > I can initialize the values to 0 of course and check the file for other > > > places with missing initializations. > > > > > > Or would it be better to check the return codes of regmap_read/write in > > > such cases? I'm not sure. > > > > I think that checking the regmap_read/write return values is overkill > > in this driver. These functions can't realistically fail, except if the i2c > > bus is bad, i.e. h/w failure or intermittency. And that's an externality > > which I believe we can ignore. > > I think there are (rare) occasions where it's fine to not check for > errors, i.e. if you definitively know that calls can't fail. However, > given that this uses regmap and you don't really know what's backing > this, I think it's always better to err on the side of caution and > properly check the return values. > > The fact that this can be externally caused is actually a reason why > we shouldn't be ignoring any errors. If there's a chip that's hogging > the I2C bus or if you've even just mistyped the I2C client's address > in DT, it's better if the PWM driver tells you with an error message > than if it is silently ignoring the errors and keeps you guessing at > why the PWM isn't behaving the way it should. > > Granted, the error code isn't always going to pinpoint exactly what's > going wrong, but for serious errors often the I2C bus driver will let > you know with an extra error message. However, it's much easier to go > looking for that error message if the PWM driver lets you know that > something went wrong. > > Please just add full checking of regmap operations.
OK, I will create a separate patch adding these checks in the next series. This will lead to > 20 additional dev_err statements, let me know if I should instead just return the error code and not add dev_err's for every failed regmap operation. Clemens
