Received-SPF: pass (google.com: domain of linux-kernel+bounces-205523-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Gregory CLEMENT <gregory.clement@bootlin.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>, Kamel BOUHARA
 <kamel.bouhara@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>,
 Henrik Rydberg <rydberg@bitmath.org>, linux-input@vger.kernel.org,
 linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Marco Felsch
 <m.felsch@pengutronix.de>, Jeff LaBundy <jeff@labundy.com>,
 catalin.popescu@leica-geosystems.com, mark.satterthwaite@touchnetix.com,
 Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
 bsp-development.geo@leica-geosystems.com
Subject: Re: [PATCH v13 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
In-Reply-To: <ZmIq9jmkZtEaGw19@google.com>
References: <20240603153929.29218-1-kamel.bouhara@bootlin.com>
 <20240603153929.29218-4-kamel.bouhara@bootlin.com>
 <Zl5ZmYyntq7OJOvZ@google.com> <20240605124746.GA57733@tpx1.home>
 <7ca4a22f903313128de5c0f65a49b319@bootlin.com>
 <ZmIq9jmkZtEaGw19@google.com>
Date: Fri, 07 Jun 2024 09:23:36 +0200
Message-ID: <87bk4dfc7b.fsf@BLaptop.bootlin.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Hello Dmitry,

> On Wed, Jun 05, 2024 at 03:48:20PM +0200, Kamel BOUHARA wrote:
>> [...]
>>=20
>> > > > +
>> > > > +	error =3D devm_request_threaded_irq(dev, client->irq, NULL,
>> > > > +					  axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
>> > > > +	if (error) {
>> > > > +		dev_info(dev, "Request irq failed, falling back to polling mode=
");
>> > >=20
>> > > I do not think you should fall back to polling mode if you fail to g=
et
>> > > interrupt. If it was not specified (client->irq) then I can see that
>> > > we
>> > > might want to fall back, but if the system configured for using
>> > > interrupt and you can not get it you should bail out.
>> > >=20
>> >=20
>> > Yes, clear, the polling mode can be decorrelated to the irq not provid=
ed
>> > case.
>>=20
>> Just to make sure I understood, is this what you propose ?
>>=20
>> if (client->irq) {
>>         error =3D devm_request_threaded_irq(...)
>>         if (error) {
>> 		dev_warn(dev, "failed to request IRQ\n");
>> 		client->irq =3D 0;
>>          }
>> }
>>=20
>> if(!client->irq)=C2=A0{
>>     // setup polling stuff
>>     ...
>> }
>
> No, if you fail to acquire interrupt that was described in ACPI/DT it
> should be treated as an error, like this:
>
> 	if (client->irq) {
> 		error =3D devm_request_threaded_irq(...)
> 		if (error)
> 			return dev_err_probe(...);
> 	} else {
> 		.. set up polling ..
> 	}
>
> This also makes sure that if interrupt controller is not ready and
> requests probe deferral we will not end up with device in polling
> mode.

In the case of probe deferral, I see the benefit of treating it as an
error. However, in the other case, I find it better to fall back to
polling mode with a big error message than just exiting in error. As a
user, I think we prefer having a degraded feature over not having the
feature at all.

So what about something like:

	if (client->irq) {
		error =3D devm_request_threaded_irq(...)
		if (error =3D=3D -EPROBE_DEFER)
			return dev_err_probe(...);
		dev_warn("Big error message");
                client->irq =3D 0;
	}
	if (!client->irq) {
		.. set up polling ..
	}

Gregory

>
> Thanks.
>
> --=20
> Dmitry