On Wed, Dec 01, 2021 at 10:27:46PM +1000, Alistair Francis wrote:

 On Sat, Nov 6, 2021 at 4:47 PM Dmitry Torokhov
[off-list ref] wrote:

quoted

Hi Alistair,

On Wed, Nov 03, 2021 at 09:48:27PM +1000, Alistair Francis wrote:

quoted

From: Mylčne Josserand <redacted>

...

quoted

quoted

+                            si->tch_hdr.max,
+                            si->xy_mode + 3 + si->tch_hdr.ofs,
+                            si->tch_hdr.bofs);
+
+     num_cur_tch = tch.hdr;
+     if (num_cur_tch > max_tch) {
+             dev_err(dev, "Num touch err detected (n=%d)\n", num_cur_tch);
+             num_cur_tch = max_tch;
+     }
+
+     if (num_cur_tch == 0 && ts->num_prv_rec == 0)
+             return 0;
+
+     /* extract xy_data for all currently reported touches */
+     if (num_cur_tch)
+             cyttsp5_get_mt_touches(ts, &tch, num_cur_tch);
+     else
+             cyttsp5_mt_lift_all(ts);

Not needed with INPUT_MT_DROP_UNUSED.

I tried INPUT_MT_DROP_UNUSED and I still need this function

I am curious why that is. Probably call to input_mt_sync_frame() was in
a wrong place?

...

quoted

quoted

+
+static int cyttsp5_deassert_int(struct cyttsp5 *ts)
+{
+     u16 size;
+     u8 buf[2];
+     int rc;
+
+     rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, 2);
+     if (rc < 0)
+             return rc;
+
+     size = get_unaligned_le16(&buf[0]);
+     if (size == 2 || size == 0)
+             return 0;

If you were to use level interrupts this probably would not be needed.

I have tried with/without this and I still can't get level interrupts working.

Does the platform support level interrupts?

...

quoted

quoted

+static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
+                      const char *name)
+{
+     struct cyttsp5 *ts;
+     struct cyttsp5_sysinfo *si;
+     int rc = 0, i;
+
+     ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+     if (!ts)
+             return -ENOMEM;
+
+     /* Initialize device info */
+     ts->regmap = regmap;
+     ts->dev = dev;
+     si = &ts->sysinfo;
+     dev_set_drvdata(dev, ts);
+
+     /* Initialize mutexes and spinlocks */
+     mutex_init(&ts->system_lock);
+
+     /* Initialize wait queue */
+     init_waitqueue_head(&ts->wait_q);
+
+     /* Power up the device */
+     ts->vdd = regulator_get(dev, "vdd");

Do not mix managed an unmanaged resources. You are leaking this
regulator.

I'm not clear what I should do differently here.

devm_regulator_get().

quoted

quoted

+     if (IS_ERR(ts->vdd)) {
+             rc = PTR_ERR(ts->vdd);
+             dev_set_drvdata(dev, NULL);
+             kfree(ts);

You can't call kfree() for memory allocated with devm_kzalloc().

quoted

quoted

+             return rc;
+     }
+
+     rc = regulator_enable(ts->vdd);
+     if (rc) {
+             regulator_put(ts->vdd);
+             dev_set_drvdata(dev, NULL);
+             kfree(ts);
+             return rc;
+     }
+
+     /* Reset the gpio to be in a reset state */
+     ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+     if (IS_ERR(ts->reset_gpio)) {
+             rc = PTR_ERR(ts->reset_gpio);
+             dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
+             return rc;
+     }
+     gpiod_set_value(ts->reset_gpio, 1);
+
+     /* Need a delay to have device up */
+     msleep(20);
+
+     rc = devm_request_threaded_irq(dev, irq, NULL, cyttsp5_handle_irq,
+                                    IRQF_TRIGGER_FALLING | IRQF_ONESHOT,

Please do not override platform setup with hardcoded triggers. Also, it
is strongly recommended to use level interrupts for these peripherals.

This is also likely unsafe if controller is not completely shut off and
is capable of generating interrupts given input device is not yet
allocated.

I have dropped the `IRQF_TRIGGER_FALLING |`

I have tried to use level interrupts, but I can't get the device
working with them.

That is weird, does the interrupt controller support level interrupts?

Thanks.

-- 
Dmitry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel