Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 22 Mar 2021 08:58:35 +0100
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Sven Van Asbroeck <thesven73@gmail.com>
Cc:     Clemens Gruber <clemens.gruber@pqgruber.com>,
        linux-pwm@vger.kernel.org,
        Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>, Lee Jones <lee.jones@linaro.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        David Jander <david@protonic.nl>
Subject: Re: [PATCH v5 1/7] pwm: pca9685: Switch to atomic API
Message-ID: <YFhOK7YuOtTI+gfa@orome.fritz.box>
References: <20201215212228.185517-1-clemens.gruber@pqgruber.com>
 <CAGngYiWbQ2STTgh2OwJTqQ-niBDbbn+OdMkk7PMzYnrZWzSy9Q@mail.gmail.com>
 <X9uL13GA1uDbLJiG@workstation.tuxnet>
 <CAGngYiW7vcJjz36xsBYx5n7=j1_5sE5a1AGpqC3Jj+tw0+FAXQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="41IcEhGcM6XGCGcU"
Content-Disposition: inline
In-Reply-To: <CAGngYiW7vcJjz36xsBYx5n7=j1_5sE5a1AGpqC3Jj+tw0+FAXQ@mail.gmail.com>
User-Agent: Mutt/2.0.6 (98f8cb83) (2021-03-06)
Precedence: bulk


--41IcEhGcM6XGCGcU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.gruber@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.
>=20
> I think that checking the regmap_read/write return values is overkill
> in this driver. These functions can't realistically fail, except if the i=
2c
> 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.

Thierry

--41IcEhGcM6XGCGcU
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmBYTigACgkQ3SOs138+
s6EZZA/9HXIKvPJZRGitdRoOS8/DxFIp1cj14qpTGMCCTsWsPXNqQ6KrpZczn3Pk
+3MlE3rvl6K9EDgNLDywsN5FaQ+HRJp7wdWUZOaH099uB/t+jIj4EDFU7WfzZYiO
Lj4D2h21zYnKNqhzjb6Xukv36Ym4Xob01sa66N2LL/JoOCUBtvZrkcDFlO5fCuTO
TAEklnpFdfYyYrOUXxe8EEnn6R4DUDQLnPiSPZvvG8xYd1gNFF2TEv1izR8Hc7sj
x0cwQVyvEKd2tQC+TR+jQ2usw4slkpxleKhAdhUEGePk/Z+uthb01gzI/fGpJBC+
UQuwjX9Uu3McTfOu7pyl2M7ObDGrmelTcSioLewWYYryM2qNAV6hTaCgmi5P6boC
FoRb5U39bkoSN77fFMbOi+/7Nr0pC1lMqdN4zWIFKF+zzZ6qUlntf33+CcF7VYYF
bD9W845SbE4QMA/O8UM7PNPAToqjXQEBpnWFMX73g8Am4xs0hXRFtbgCtwy4bqGv
qrQ4xLR5uuleZ7lZz1qKgd8YCvqNFosUZK0Eab3LLUy5hR2AFbzYfm4XY7/Kln9f
gVE6eVcy7Qr+inPZfmJTM33RpxoA0bA4BVaPxx5KmI/C7PMLMSgar7yqVO2cPpQB
sHVRrZKZM2UTUtAjD604mDa+BPUWsw7y0w32ZabvNcE1DJLy134=
=n6By
-----END PGP SIGNATURE-----

--41IcEhGcM6XGCGcU--