Re: [PATCH 5/6] arm64: dts: qcom: msm8953-flipkart-rimob: Enable touchscreen

[Konrad Dybcio]

On 3/11/26 2:16 PM, Dmitry Baryshkov wrote:
> On Wed, Mar 11, 2026 at 01:47:48PM +0100, Konrad Dybcio wrote:
>> On 3/10/26 11:49 PM, Dmitry Baryshkov wrote:
>>> On Tue, Mar 10, 2026 at 02:08:40PM +0100, Konrad Dybcio wrote:
>>>> On 3/8/26 4:52 PM, Cristian Cozzolino via B4 Relay wrote:
>>>>> From: Cristian Cozzolino <cristian_ci@xxxxxxxxxxxxxx>
>>>>>
>>>>> This device uses a Goodix GT5688 touch controller, connected to i2c_3.
>>>>> Add it to the device tree.
>>>>>
>>>>> Signed-off-by: Cristian Cozzolino <cristian_ci@xxxxxxxxxxxxxx>
>>>>> ---
>>>>>  .../arm64/boot/dts/qcom/msm8953-flipkart-rimob.dts | 32 ++++++++++++++++++++++
>>>>>  1 file changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/msm8953-flipkart-rimob.dts b/arch/arm64/boot/dts/qcom/msm8953-flipkart-rimob.dts
>>>>> index 7b2849405462..709ea6fc9fbb 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/msm8953-flipkart-rimob.dts
>>>>> +++ b/arch/arm64/boot/dts/qcom/msm8953-flipkart-rimob.dts
>>>>> @@ -94,6 +94,31 @@ &hsusb_phy {
>>>>>  	status = "okay";
>>>>>  };
>>>>>  
>>>>> +&i2c_3 {
>>>>> +	status = "okay";
>>>>> +
>>>>> +	touchscreen@5d {
>>>>> +		compatible = "goodix,gt5688";
>>>>> +		reg = <0x5d>;
>>>>> +
>>>>> +		interrupts-extended = <&tlmm 65 IRQ_TYPE_LEVEL_LOW>;
>>>>
>>>> interrupts *and* irq-gpios sounds wrong.. and I think the driver doesn't
>>>> even consume the former. Trying to read through some of that, I think
>>>
>>> I think you need both, take a look, there are enough users of
>>> ts->gpiod_int.
>>
>> I said irq-gpios was necessary, interrupts is not. The only retrieval
>> happens through:
>>
>> if (soc_intel_is_cht() && ts->gpio_count == 2 && ts->gpio_int_idx != -1) {
>> 	irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
>> 	if (irq > 0 && irq != ts->client->irq) {
>> 		dev_warn(dev, "Overriding IRQ %d -> %d\n", ts->client->irq, irq);
>> 		ts->client->irq = irq;
>> 	}
>> }
> 
> static int goodix_request_irq(struct goodix_ts_data *ts)
> {
>         if (!ts->client->irq)
>                 return 0;
> 
>         return devm_request_threaded_irq(&ts->client->dev, ts->client->irq,
>                                          NULL, goodix_ts_irq_handler,
>                                          ts->irq_flags, ts->client->name, ts);
> }
> 
> I thought that i2c_client->irq is handled by the core.

Ohhhh that explains things

Cristian, please ignore my request to remove it then

Konrad