Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f9c8e0d4-1d0c-cefc-866c-046d4d374576@gmail.com>
Date:   Sun, 9 Apr 2023 16:32:27 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Subject: Re: [PATCH 2/5] Input: add driver for Focaltech FTS touchscreen
Content-Language: en-US
To:     Joel Selvaraj <joelselvaraj.oss@gmail.com>,
        Caleb Connolly <caleb@connolly.tech>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Henrik Rydberg <rydberg@bitmath.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Robert Jarzmik <robert.jarzmik@free.fr>,
        Jeff LaBundy <jeff@labundy.com>,
        Neil Armstrong <neil.armstrong@linaro.org>,
        Jean Delvare <jdelvare@suse.de>,
        Max Krummenacher <max.krummenacher@toradex.com>,
        Job Noorman <job@noorman.info>,
        Alistair Francis <alistair@alistair23.me>,
        Chris Morgan <macromorgan@hotmail.com>
Cc:     linux-input@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org
References: <20230312093249.1846993-1-joelselvaraj.oss@gmail.com>
 <20230312093249.1846993-3-joelselvaraj.oss@gmail.com>
 <68b05c43-5808-5792-9b57-aeafffe84149@gmail.com>
 <d40faca2-fe5d-5b5a-eefe-68eb3e5e8125@gmail.com>
From:   Markuss Broks <markuss.broks@gmail.com>
In-Reply-To: <d40faca2-fe5d-5b5a-eefe-68eb3e5e8125@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Joel,

Sorry for responding late:

On 3/13/23 00:21, Joel Selvaraj wrote:
> Hi Markuss,
>
> Thanks for the quick review! I agree with most of your comments and will
> fix them in a v2 soon. I have a few doubts as discussed below.
>
> On 12/03/23 15:40, Markuss Broks wrote:
>
>> Why is the _ratelimited variant necessary?
> I assumed in case of the interrupt working, but i2c reads fail for some
> reason, it would spam a lot of error messages if the user touches the
> screen continuously, like a swipe up gesture or something.
>
> I referred to ad7879 touchscreen's irq handling code [1] and thought
> it's probably best to do this, to be on the safe side. I will remove
> this if it's not needed in v2.
>
>> Overall, I think it's better to cast the data type to a struct, which
>> would make this seem with less random.
> Sorry, I am not sure I got this right. Do you mean I create an array of
> struct called say "fts_point" that stores the x, y, type, etc. info of
> all the points, then report it separately. Like similar to something
> done by the auo-pixcir touchscreen driver [2]?

By that I meant doing something like the Zinitix driver[1] does. It has 
a struct data type for whatever you read from hardware, e.g.

struct point_coord {
     __le16    x;
     __le16    y;
...
};

from that driver. That way you can cast the data read to that struct and 
have it look a bit nicer.

This is just a suggestion though, you have the final choice in what 
design you choose for your code :)

>
> If I didn't get this correctly, can you show me some code in mainline,
> that does it? It would be very helpful.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/ad7879.c?h=v6.3-rc1#n250
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/auo-pixcir-ts.c?h=v6.3-rc1#n162
>
>> - Markuss
> Thanks,
> Joel
[1]
https://elixir.free-electrons.com/linux/v6.3-rc5/source/drivers/input/touchscreen/zinitix.c


- Markuss