Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <da6d2828-41db-4c29-a1aa-024b1fbcc43a@gmail.com>
Date:   Sat, 9 Dec 2023 22:50:52 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 5/5] input/touchscreen: imagis: add support for
 IST3032C
To:     Karel Balej <karelb@gimli.ms.mff.cuni.cz>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        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, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     =?UTF-8?Q?Duje_Mihanovi=C4=87?= <duje.mihanovic@skole.hr>,
        ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org,
        Karel Balej <balejk@matfyz.cz>
References: <20231202125948.10345-1-karelb@gimli.ms.mff.cuni.cz>
 <20231202125948.10345-6-karelb@gimli.ms.mff.cuni.cz>
 <89f7e7de-c574-49ab-885d-c6d4427fe64f@gmail.com>
 <CXJAG826ZTNA.2F8WOGVNYADKP@gimli.ms.mff.cuni.cz>
Content-Language: en-US
From:   Markuss Broks <markuss.broks@gmail.com>
In-Reply-To: <CXJAG826ZTNA.2F8WOGVNYADKP@gimli.ms.mff.cuni.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Karel,

On 12/8/23 23:59, Karel Balej wrote:
> Markuss,
>
> thank you for the review.
>
>>> diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
>>> index 84a02672ac47..41f28e6e9cb1 100644
>>> --- a/drivers/input/touchscreen/imagis.c
>>> +++ b/drivers/input/touchscreen/imagis.c
>>> @@ -35,6 +35,8 @@
>>>    #define IST3038B_REG_CHIPID		0x30
>>>    #define IST3038B_WHOAMI			0x30380b
>>>    
>>> +#define IST3032C_WHOAMI			0x32c
>>> +
>> Perhaps it should be ordered in alphabetic/alphanumeric order,
>> alternatively, the chip ID values could be grouped.
> Here I followed suit and just started a new section for the new chip,
> except there is only one entry. I do agree that it would be better to
> sort the chips alphanumerically and I am actually surprised that I
> didn't do that - but now I see that the chips that you added are not
> sorted either, so it might be because of that.
>
> I propose to definitely swap the order of the sections, putting 32C
> first, then 38B and 38C at the end (from top to bottom). The chip ID
> values could then still be grouped in a new section, but I think I would
> actually prefer to keep them as parts of the respective sections as it
> is now, although it is in no way a strong preference.
We could do that, yeah. It is not a problem right now since there's only 
3 models supported, but it would maker sense and set some order for when 
we'd have more supported devices.
>
> Please let me know whether you agree with this or have a different
> preference. And if the former, please confirm that I can add your
> Reviewed-by trailer to the patch modified in such a way.
Yeah, it's fine.
>
> Best regards,
> K. B.

- Markuss