Re: [PATCH v2 2/2] Input: edt-ft5x06 - add ft5426

[Andreas Kemnade <andreas@kemnade.info>]

On Thu, 25 Apr 2024 22:54:13 -0400
Felix Kaechele <felix@kaechele.ca> wrote:

> On 2024-04-25 12:54, Andreas Kemnade wrote:
> > On Fri, 5 Apr 2024 20:21:19 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >  
> >> On Fri, Apr 5, 2024 at 7:28 PM Andreas Kemnade <andreas@kemnade.info> wrote: >>> On Fri, 5 Apr 2024 18:13:45 +0300  
> >>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> 
> ...
> 
> >>>> Why a different vendor prefix?  
> 
> ...
> 
> >>> I sorted by the numbers. Looking at datasheets for other controllers I see >>> https://www.displayfuture.com/Display/datasheet/controller/FT5x06.pdf
> >>> it only mentions FocalTech Systems Co., Ltd.  
> >>
> >> But does the driver use that? AFAICS it uses edt. Perhaps it's due to
> >> a business split, not to my knowledge anyway.  
> 
> I've been looking into this over the past few weeks as I was working on 
> mainline support for an Android device.
> And please forgive me if any of the following is not fully accurate, I'm 
> not an industry expert.
> 
> After some research, my understanding of this is as follows:
> 
> - There are companies that make touch ICs, LCD driver ICs and sometimes 
> even ICs that are both. Focaltech or Himax are examples of such companies.
> 
> - There are companies that make LCMs. These are complete assemblies of 
> panel, backlight, touch layer and driver circuitry PCBs. This is what 
> OEMs generally purchase when they design a consumer device. Emerging 
> Display Technologies Corp. (EDT) is such a LCM manufacturing company. 
> More often than not LCM manufacturers do not make their own driver ICs.
> 
> LCM manufacturers include ICs from Focaltech in their LCMs.
> To my knowledge Focaltech is not a manufacturer of LCMs.
> 
> As such, an interpretation of the compatible string "edt,edt-ft5406" 
> could be: Unspecified EDT LCM with Focaltech FT5406 IC.
> 
>  From my perspective, more correct would either be something like 
> "edt,etm070001bdh6" (the LCM by EDT that contains this IC, especially if 
> it had model specific quirks) or "focaltech,ft5406".
> But "edt,edt-ft5406" is incorrect if being specific is the goal here.
> Given that the driver predates much of the DT binding rigour it's what 
> we have now though.
> 
I think focaltech,ft5406 is better because it is consistent with other drivers/
bindings. We do not specify the display it is used on on other touchscreen
bindings. We do not specify the actual LEDs behind a LED interface chip.
And often the chip name is more easily to find out than the name of a
display.
[...]

> I don't think that's how the compatible strings are used today, but it 
> is what would make sense in my opinion.
> 
> > Looking around I found this:
> >              if (tsdata->version == EV_FT)
> >                          swap(x, y);
> > ...
> >                 case 0x59:  /* Evervision Display with FT5xx6 TS */
> >                          tsdata->version = EV_FT;
> >
> > I need swap(x.y), I am using touchscreen-swapped-x-y property now.
> > So evervision prefix?  
> 
> The compatible string doesn't have any bearing on whether x and y are 
> swapped. The driver relies on its device detection heuristic for that 
> determination.

Well, I think, yes, it could have a bearing, on some models, maybe x is
swapped with some pressure value in some record.

> Ideally, the driver would allow describing this property
> 
>    1. in the devicetree using the "touchscreen-swapped-x-y" property 
> from the common touchscreen bindings
> 
It allows that right now.
Well, the devicetree should be driver/os independent, so it describes the
hardware. so the driver should spit out the records specified as x
in the chip manual as x and the ones specified as y as y.

If the display is then wired up to the controller in an unusual way,
it can be compensated by the devicetree specification (if we go the
focaltech,ftxxxx road instead of edt,etm070001bdh6). 

But datasheets... where are you? So we need a heuristic. I guess
the order used most is probably the one used in the data sheet.

>    2. by extending the edt_i2c_chip_data struct to hold that property 
> and set it based on the compatible string if it is, in fact, a property 
> of that specific IC
> 
yes, agreed.

Regards,
Andreas