Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Fri, 08 Dec 2023 22:59:56 +0100
Message-Id: <CXJAG826ZTNA.2F8WOGVNYADKP@gimli.ms.mff.cuni.cz>
From:   "Karel Balej" <karelb@gimli.ms.mff.cuni.cz>
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>
Subject: Re: [PATCH v3 5/5] input/touchscreen: imagis: add support for
 IST3032C
To:     "Markuss Broks" <markuss.broks@gmail.com>,
        "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>
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>
In-Reply-To: <89f7e7de-c574-49ab-885d-c6d4427fe64f@gmail.com>
Precedence: bulk

Markuss,

thank you for the review.

> > diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchsc=
reen/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
> >  =20
> > +#define IST3032C_WHOAMI			0x32c
> > +

> Perhaps it should be ordered in alphabetic/alphanumeric order,=20
> 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.

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.

Best regards,
K. B.