Update TSC2007 to common touch axis handling #5791
Conversation
This adds the common touchscreen related configuration options found in touchscreen.yaml. This enables a few important options like being able to swap x and y or inverting either axis. Old vendor specific properties that are now available via touchscreen.yaml bindings have been replaced with the relevant common properties. Signed-off-by: Timon Skerutsch <kernel@diodes-delight.com>
Moving the bindings to yaml format and adding the common touchscreen.yaml properties. Old properties that are now covered via touchscreen.yaml have been removed and replaced by the relevant properties from touchscreen.yaml Signed-off-by: Timon Skerutsch <kernel@diodes-delight.com>
There was a problem hiding this comment.
Sorry for having missed this one.
DT binding change needs to be at least 2 patches - conversion of TXT to YAML as is, and then changing the functionality.
DT bindings are ABI, so you can't break existing users. Renaming parameters will cause regressions.
You've put yourself as maintainer in the binding - update MAINTAINERS too?
This is an unmodified driver from mainline, so I'm afraid we really do have to point you to upstream for a proper review and merge. We'll happily backport an accepted patch, but can't really take on the maintenance burden.
| @@ -6,6 +6,7 @@ | |||
|
|
|||
| #include <linux/i2c.h> | |||
| #include <linux/iio/iio.h> | |||
| #include <linux/input/touchscreen.h> | |||
| @@ -141,9 +142,8 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle) | |||
|
|
|||
| rt = ts->max_rt - rt; | |||
|
|
|||
| touchscreen_report_pos(ts->input, &ts->prop, tc.x, tc.y, false); | |||
There was a problem hiding this comment.
You've changed the order of this (which triggers ABS_X and ABS_Y events), wrt to BTN_TOUCH. Does that matter?
I suspect not as I believe input_sync is the trigger to do anything with all the events, but I couldn't say so definitively.
| @@ -234,29 +234,20 @@ static int tsc2007_probe_properties(struct device *dev, struct tsc2007 *ts) | |||
| u32 val32; | |||
| u64 val64; | |||
|
|
|||
| if (!device_property_read_u32(dev, "ti,max-rt", &val32)) | |||
There was a problem hiding this comment.
Dropping these causes a regression for any existing users. AIUI you need to parse these, and then overwrite with values from the common parsing if present.
| @@ -234,29 +234,20 @@ static int tsc2007_probe_properties(struct device *dev, struct tsc2007 *ts) | |||
| u32 val32; | |||
| u64 val64; | |||
|
|
|||
| if (!device_property_read_u32(dev, "ti,max-rt", &val32)) | |||
| if (!device_property_read_u32(dev, "touchscreen-max-pressure", &val32)) | |||
There was a problem hiding this comment.
touchscreen-max-pressure is read by touchscreen_parse_properties. Is there a need to read it in two locations?
Set the default to MAX_12BIT before calling touchscreen_parse_properties, and it will overwrite that default if found.
| @@ -353,10 +344,13 @@ static int tsc2007_probe(struct i2c_client *client, | |||
|
|
|||
| input_set_capability(input_dev, EV_KEY, BTN_TOUCH); | |||
|
|
|||
| /* Apply */ | |||
There was a problem hiding this comment.
This and line 350 appear to only be irrelevant changes.
|
Sorry, stuff happened on my end as well. Thank your for the feedback and review!
Right that is true. How is this typically handled, do old driver just never get updated or should it support both old and new bindings?
I was really unsure how this is handled and haven't found guidance on how things would look if something did not have a yaml binding before. Should I put myself as a maintainer there? It felt off to put the current driver maintainer in there if they haven't authored the yaml binding.
That I don't fully understood, could you expand what you mean with unmodified driver from mainline? Last time I asked the policy was that existing drivers could get a modifications accepted directly without going through mainline first and completely new drivers would have to go through mainline first. Did I misunderstand that? |
The driver currently lacks most of the essential configuration properties like invert, swap, min max values etc.
This adds support for the common handling of these values via touchscreen.yaml.
I also removed/updated some of the old vendor specific properties that can be covered via the generic touchscreen.yaml