Re: [PATCH v2 14/16] iio: health: max30100: do not use internal iio_dev lock

From: "Nuno Sá" <noname.nuno@gmail.com>
To: "Andy Shevchenko" <andy.shevchenko@gmail.com>,
	"Nuno Sá" <nuno.sa@analog.com>
Cc: linux-amlogic@lists.infradead.org, linux-imx@nxp.com,
	 linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	 linux-rockchip@lists.infradead.org,
	Heiko Stuebner <heiko@sntech.de>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Jyoti Bhayana <jbhayana@google.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Andriy Tryshnivskyy <andriy.tryshnivskyy@opensynergy.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Cixi Geng <cixi.geng1@unisoc.com>,
	Baolin Wang <baolin.wang@linux.alibaba.com>,
	 Ciprian Regus <ciprian.regus@analog.com>,
	Fabio Estevam <festevam@gmail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Alexandru Ardelean <aardelean@deviqon.com>,
	 Florian Boor <florian.boor@kernelconcepts.de>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Orson Zhai <orsonzhai@gmail.com>, Chen-Yu Tsai <wens@csie.org>,
	Chunyan Zhang <zhang.lyra@gmail.com>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Haibo Chen <haibo.chen@nxp.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Jonathan Cameron <jic23@kernel.org>
Subject: Re: [PATCH v2 14/16] iio: health: max30100: do not use internal iio_dev lock
Date: Wed, 05 Oct 2022 10:09:29 +0200	[thread overview]
Message-ID: <0bad9d1ef45d0a85b7d25a4ee50914ea93936bb2.camel@gmail.com> (raw)
In-Reply-To: <CAHp75Vfv4BntoDPOo_MXZ33dVEKYj1KW_Ocd5QL_Ez06HJ33iw@mail.gmail.com>

On Tue, 2022-10-04 at 17:13 +0300, Andy Shevchenko wrote:
> On Tue, Oct 4, 2022 at 4:49 PM Nuno Sá <nuno.sa@analog.com> wrote:
> > 
> > The pattern used in this device does not quite fit in the
> > iio_device_claim_direct_mode() typical usage. In this case,
> > iio_buffer_enabled() was being used not to prevent the raw access
> > but to
> > allow it. Hence, let's make use of the new
> > iio_device_claim_buffer_mode() API to make sure we stay in buffered
> > mode
> > during the complete read.
> 
> ...
> 
> > +               if (iio_device_claim_buffer_mode(indio_dev)) {
> >                         ret = -EAGAIN;
> 
> Why is the error code shadowed? Isn't it better to return exactly the
> one you resend to the upper caller(s)? Each unclear error code
> shadowing should be at least explained.
> >           }

I'm keeping the same error that was returned before. Changing the error
code returned to userspace can break some apps relying on it. But if
everyone is ok with assuming the risk and changing it, fine by me.

- Nuno Sá

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic