| Message ID | 20250308-synaptics-rmi4-v3-0-215d3e7289a2@ixit.cz |
|---|---|
| Headers | show |
| Series | Subject: [PATCH v3 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers | expand |
On Sat, Mar 08, 2025 at 03:08:37PM +0100, David Heidelberg wrote: > From: Caleb Connolly <caleb.connolly@linaro.org> > > This new property allows devices to specify some register values which > are missing on units with third party replacement displays. These > displays use unofficial touch ICs which only implement a subset of the > RMI4 specification. These are different ICs, so they have their own compatibles. Why this cannot be deduced from the compatible? > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > Signed-off-by: David Heidelberg <david@ixit.cz> > --- > Documentation/devicetree/bindings/input/syna,rmi4.yaml | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml > index b522c8d3ce0db719ff379f2fefbdca79e73d027c..a80ec0c052cb1b7278f0832dd18ebd3256bc0874 100644 > --- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml > +++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml > @@ -49,6 +49,24 @@ properties: > description: > Delay to wait after powering on the device. > > + syna,pdt-fallback-desc: > + $ref: /schemas/types.yaml#/definitions/uint8-matrix > + description: > + This property provides fallback values for certain register fields that > + are missing on devices using third-party replacement displays. > + These unofficial displays contain touch controllers that claim RMI4 > + compliance but fail to populate the function_number and function_version > + registers of their Page Descriptor Table (PDT) entries. > + > + Since the number of required fallback entries depends on the number of > + Page Descriptor Tables supported by a given device, this property > + should be provided on a best-effort basis. > + > + items: min/maxItems here > + items: > + - description: The 5th byte of the PDT entry (function number) > + - description: The 4th byte of the PDT entry (version value) Best regards, Krzysztof
Hi David, Please at least give me a heads up if you're going to resend a patch series of mine. I understand it's an old series but I don't think that courtesy is too much to ask. On 3/8/25 14:08, David Heidelberg via B4 Relay wrote: > With the growing popularity of running upstream Linux on mobile devices, > we're beginning to run into more and more edgecases. The OnePlus 6 is a > fairly well supported 2018 era smartphone, selling over a million units > in it's first 22 days. With this level of popularity, it's almost > inevitable that we get third party replacement displays, and as a > result, replacement touchscreen controllers. > > The OnePlus 6 shipped with an extremely usecase specific touchscreen > driver, it implemented only the bare minimum parts of the highly generic > rmi4 protocol, instead hardcoding most of the register addresses. > > As a result, the third party touchscreen controllers that are often > found in replacement screens, implement only the registers that the > downstream driver reads from. They additionally have other restrictions > such as heavy penalties on unaligned reads. > > This series attempts to implement the necessary workaround to support > some of these chips with the rmi4 driver. Although it's worth noting > that at the time of writing there are other unofficial controllers in > the wild that don't work even with these patches. > > We have been shipping these patches in postmarketOS for the last several > months, and they are known to not cause any regressions on the OnePlus > 6/6T (with the official Synaptics controller), however I don't own any > other rmi4 hardware to further validate this. > > --- > Changes since v2: > - reworded dt-bindings property description > - fixed the rmi_driver_of_probe definition for non device-tree builds. > - fixed some indentation issues reported by checkpatch > - change rmi_pdt_entry_is_valid() variable to unsigned > - Link to v2: https://patchwork.kernel.org/project/linux-input/cover/20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org/ Please use lore links > > Changes since v1: > - Improve dt-bindings patch (thanks Rob) > - Add missing cast in patch 5 to fix the pointer arithmetic > - Link to v1: https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org > > --- > Caleb Connolly (2): > dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc > Input: synaptics-rmi4 - handle duplicate/unknown PDT entries > > methanal (5): > Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs > Input: synaptics-rmi4 - f55: handle zero electrode count > Input: synaptics-rmi4 - don't do unaligned reads in IRQ context > Input: synaptics-rmi4 - read product ID on aftermarket touch ICs > Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes > > .../devicetree/bindings/input/syna,rmi4.yaml | 18 +++ > drivers/input/rmi4/rmi_driver.c | 140 +++++++++++++++++---- > drivers/input/rmi4/rmi_driver.h | 8 ++ > drivers/input/rmi4/rmi_f01.c | 14 +++ > drivers/input/rmi4/rmi_f12.c | 117 +++++++++++++---- > drivers/input/rmi4/rmi_f55.c | 5 + > include/linux/rmi.h | 3 + > 7 files changed, 258 insertions(+), 47 deletions(-) > --- > base-commit: 0a2f889128969dab41861b6e40111aa03dc57014 > change-id: 20250308-synaptics-rmi4-c832b2f73ceb > > Best regards,
Hello Caleb, I'm very sorry about that. Next time I include your patches in the series I'll definitely send you heads up. David On 10/03/2025 11:04, Caleb Connolly wrote: > Hi David, > > Please at least give me a heads up if you're going to resend a patch > series of mine. I understand it's an old series but I don't think that > courtesy is too much to ask. > > On 3/8/25 14:08, David Heidelberg via B4 Relay wrote: >> With the growing popularity of running upstream Linux on mobile devices, >> we're beginning to run into more and more edgecases. The OnePlus 6 is a >> fairly well supported 2018 era smartphone, selling over a million units >> in it's first 22 days. With this level of popularity, it's almost >> inevitable that we get third party replacement displays, and as a >> result, replacement touchscreen controllers. >> >> The OnePlus 6 shipped with an extremely usecase specific touchscreen >> driver, it implemented only the bare minimum parts of the highly generic >> rmi4 protocol, instead hardcoding most of the register addresses. >> As a result, the third party touchscreen controllers that are often >> found in replacement screens, implement only the registers that the >> downstream driver reads from. They additionally have other restrictions >> such as heavy penalties on unaligned reads. >> This series attempts to implement the necessary workaround to support >> some of these chips with the rmi4 driver. Although it's worth noting >> that at the time of writing there are other unofficial controllers in >> the wild that don't work even with these patches. >> We have been shipping these patches in postmarketOS for the last several >> months, and they are known to not cause any regressions on the OnePlus >> 6/6T (with the official Synaptics controller), however I don't own any >> other rmi4 hardware to further validate this. >> >> --- >> Changes since v2: >> - reworded dt-bindings property description >> - fixed the rmi_driver_of_probe definition for non device-tree builds. >> - fixed some indentation issues reported by checkpatch >> - change rmi_pdt_entry_is_valid() variable to unsigned >> - Link to v2: https://patchwork.kernel.org/project/linux-input/ >> cover/20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org/ > > Please use lore links >> >> Changes since v1: >> - Improve dt-bindings patch (thanks Rob) >> - Add missing cast in patch 5 to fix the pointer arithmetic >> - Link to v1: https://lore.kernel.org/r/20230929-caleb-rmi4-quirks- >> v1-0-cc3c703f022d@linaro.org >> >> --- >> Caleb Connolly (2): >> dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc >> Input: synaptics-rmi4 - handle duplicate/unknown PDT entries >> >> methanal (5): >> Input: synaptics-rmi4 - f12: use hardcoded values for >> aftermarket touch ICs >> Input: synaptics-rmi4 - f55: handle zero electrode count >> Input: synaptics-rmi4 - don't do unaligned reads in IRQ context >> Input: synaptics-rmi4 - read product ID on aftermarket touch ICs >> Input: synaptics-rmi4 - support fallback values for PDT >> descriptor bytes >> >> .../devicetree/bindings/input/syna,rmi4.yaml | 18 +++ >> drivers/input/rmi4/rmi_driver.c | 140 +++++++++++ >> ++++++---- >> drivers/input/rmi4/rmi_driver.h | 8 ++ >> drivers/input/rmi4/rmi_f01.c | 14 +++ >> drivers/input/rmi4/rmi_f12.c | 117 +++++++++++ >> ++---- >> drivers/input/rmi4/rmi_f55.c | 5 + >> include/linux/rmi.h | 3 + >> 7 files changed, 258 insertions(+), 47 deletions(-) >> --- >> base-commit: 0a2f889128969dab41861b6e40111aa03dc57014 >> change-id: 20250308-synaptics-rmi4-c832b2f73ceb >> >> Best regards, >
On Sat, Mar 08, 2025 at 03:08:40PM +0100, David Heidelberg via B4 Relay wrote: > From: methanal <baclofen@tuta.io> > > Some third party ICs claim to support f55 but report an electrode count > of 0. Catch this and bail out early so that we don't confuse the i2c bus > with 0 sized reads. > > Signed-off-by: methanal <baclofen@tuta.io> > [simplify code, adjust wording] > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > drivers/input/rmi4/rmi_f55.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/input/rmi4/rmi_f55.c b/drivers/input/rmi4/rmi_f55.c > index 488adaca4dd00482cd1106d813b32871092c83a0..ad2ef14ae9f4e897473db43334792cc3de966d52 100644 > --- a/drivers/input/rmi4/rmi_f55.c > +++ b/drivers/input/rmi4/rmi_f55.c > @@ -52,6 +52,11 @@ static int rmi_f55_detect(struct rmi_function *fn) > > f55->num_rx_electrodes = f55->qry[F55_NUM_RX_OFFSET]; > f55->num_tx_electrodes = f55->qry[F55_NUM_TX_OFFSET]; > + if (!f55->num_rx_electrodes || !f55->num_tx_electrodes) { > + rmi_dbg(RMI_DEBUG_FN, &fn->dev, > + "F55 query returned no electrodes, giving up\n"); > + return 0; 0 here means "successfully detected" and will result in F55 probe succeeding. I expect you want -EINVAL or -ENODEV here. Thanks.
Hi David, On Sat, Mar 08, 2025 at 03:08:38PM +0100, David Heidelberg via B4 Relay wrote: > From: Caleb Connolly <caleb.connolly@linaro.org> > > Some third party rmi4-compatible ICs don't expose their PDT entries > very well. Add a few checks to skip duplicate entries as well as entries > for unsupported functions. > > This is required to support some phones with third party displays. > > Validated on a stock OnePlus 6T (original parts): > manufacturer: Synaptics, product: S3706B, fw id: 2852315 > > Co-developed-by: methanal <baclofen@tuta.io> > Signed-off-by: methanal <baclofen@tuta.io> > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > Signed-off-by: David Heidelberg <david@ixit.cz> > --- > drivers/input/rmi4/rmi_driver.c | 47 +++++++++++++++++++++++++++++++++++------ > drivers/input/rmi4/rmi_driver.h | 6 ++++++ > 2 files changed, 47 insertions(+), 6 deletions(-) > > diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c > index 2168b6cd7167334d44553c9c566f870a4e034179..51c23a407b2731d5b6eaefe9cae6288f97316e34 100644 > --- a/drivers/input/rmi4/rmi_driver.c > +++ b/drivers/input/rmi4/rmi_driver.c > @@ -493,12 +493,44 @@ static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt, > fd->function_version = pdt->function_version; > } > > +static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev, > + struct pdt_scan_state *state, u8 fn) > +{ > + unsigned int i; > + > + switch (fn) { > + case 0x01: > + case 0x03: > + case 0x11: > + case 0x12: > + case 0x30: > + case 0x34: > + case 0x3a: > + case 0x54: > + case 0x55: This mean that we need to update this code any time there is new function introduced. I'd rather we did not do that. The driver should be able to handle unknown functions. > + break; > + > + default: > + rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev, > + "PDT has unknown function number %#02x\n", fn); > + return false; > + } > + > + for (i = 0; i < state->pdt_count; i++) { > + if (state->pdts[i] == fn) > + return false; > + } > + > + state->pdts[state->pdt_count++] = fn; Duplicate detection could be handled thorough a bitmap. Thanks.
Hi Dmitry, On 3/10/25 19:10, Dmitry Torokhov wrote: > Hi David, > > On Sat, Mar 08, 2025 at 03:08:38PM +0100, David Heidelberg via B4 Relay wrote: >> From: Caleb Connolly <caleb.connolly@linaro.org> >> >> Some third party rmi4-compatible ICs don't expose their PDT entries >> very well. Add a few checks to skip duplicate entries as well as entries >> for unsupported functions. >> >> This is required to support some phones with third party displays. >> >> Validated on a stock OnePlus 6T (original parts): >> manufacturer: Synaptics, product: S3706B, fw id: 2852315 >> >> Co-developed-by: methanal <baclofen@tuta.io> >> Signed-off-by: methanal <baclofen@tuta.io> >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >> Signed-off-by: David Heidelberg <david@ixit.cz> >> --- >> drivers/input/rmi4/rmi_driver.c | 47 +++++++++++++++++++++++++++++++++++------ >> drivers/input/rmi4/rmi_driver.h | 6 ++++++ >> 2 files changed, 47 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c >> index 2168b6cd7167334d44553c9c566f870a4e034179..51c23a407b2731d5b6eaefe9cae6288f97316e34 100644 >> --- a/drivers/input/rmi4/rmi_driver.c >> +++ b/drivers/input/rmi4/rmi_driver.c >> @@ -493,12 +493,44 @@ static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt, >> fd->function_version = pdt->function_version; >> } >> >> +static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev, >> + struct pdt_scan_state *state, u8 fn) >> +{ >> + unsigned int i; >> + >> + switch (fn) { >> + case 0x01: >> + case 0x03: >> + case 0x11: >> + case 0x12: >> + case 0x30: >> + case 0x34: >> + case 0x3a: >> + case 0x54: >> + case 0x55: > > This mean that we need to update this code any time there is new > function introduced. I'd rather we did not do that. The driver should be > able to handle unknown functions. Synaptics don't produce new RMI4 based tech anymore afaik, they have moved on. So I don't think there will be new functions being added here. Unfortunately in my experience the fake touch ICs report completely random values here, so there really isn't a good way to handle this otherwise. What if this list rather than being hardcoded here was just using the fn_handlers[] array from rmi_bus.c? Kind regards, > >> + break; >> + >> + default: >> + rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev, >> + "PDT has unknown function number %#02x\n", fn); >> + return false; >> + } >> + >> + for (i = 0; i < state->pdt_count; i++) { >> + if (state->pdts[i] == fn) >> + return false; >> + } >> + >> + state->pdts[state->pdt_count++] = fn; > > Duplicate detection could be handled thorough a bitmap. > > Thanks. >
With the growing popularity of running upstream Linux on mobile devices, we're beginning to run into more and more edgecases. The OnePlus 6 is a fairly well supported 2018 era smartphone, selling over a million units in it's first 22 days. With this level of popularity, it's almost inevitable that we get third party replacement displays, and as a result, replacement touchscreen controllers. The OnePlus 6 shipped with an extremely usecase specific touchscreen driver, it implemented only the bare minimum parts of the highly generic rmi4 protocol, instead hardcoding most of the register addresses. As a result, the third party touchscreen controllers that are often found in replacement screens, implement only the registers that the downstream driver reads from. They additionally have other restrictions such as heavy penalties on unaligned reads. This series attempts to implement the necessary workaround to support some of these chips with the rmi4 driver. Although it's worth noting that at the time of writing there are other unofficial controllers in the wild that don't work even with these patches. We have been shipping these patches in postmarketOS for the last several months, and they are known to not cause any regressions on the OnePlus 6/6T (with the official Synaptics controller), however I don't own any other rmi4 hardware to further validate this. --- Changes since v2: - reworded dt-bindings property description - fixed the rmi_driver_of_probe definition for non device-tree builds. - fixed some indentation issues reported by checkpatch - change rmi_pdt_entry_is_valid() variable to unsigned - Link to v2: https://patchwork.kernel.org/project/linux-input/cover/20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org/ Changes since v1: - Improve dt-bindings patch (thanks Rob) - Add missing cast in patch 5 to fix the pointer arithmetic - Link to v1: https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org --- Caleb Connolly (2): dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc Input: synaptics-rmi4 - handle duplicate/unknown PDT entries methanal (5): Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs Input: synaptics-rmi4 - f55: handle zero electrode count Input: synaptics-rmi4 - don't do unaligned reads in IRQ context Input: synaptics-rmi4 - read product ID on aftermarket touch ICs Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes .../devicetree/bindings/input/syna,rmi4.yaml | 18 +++ drivers/input/rmi4/rmi_driver.c | 140 +++++++++++++++++---- drivers/input/rmi4/rmi_driver.h | 8 ++ drivers/input/rmi4/rmi_f01.c | 14 +++ drivers/input/rmi4/rmi_f12.c | 117 +++++++++++++---- drivers/input/rmi4/rmi_f55.c | 5 + include/linux/rmi.h | 3 + 7 files changed, 258 insertions(+), 47 deletions(-) --- base-commit: 0a2f889128969dab41861b6e40111aa03dc57014 change-id: 20250308-synaptics-rmi4-c832b2f73ceb Best regards,