Hi Dmitry,

thanks for taking a look at this!

On Tue, Jul 16, 2019 at 08:47:44PM +0200, Dmitry Torokhov wrote:

Hi Ronald,

On Fri, Apr 19, 2019 at 01:19:26AM -0700, Ronald Tschalär wrote:

quoted

The keyboard and trackpad on recent MacBook's (since 8,1) and
MacBookPro's (13,* and 14,*) are attached to an SPI controller instead
of USB, as previously. The higher level protocol is not publicly
documented and hence has been reverse engineered. As a consequence there
are still a number of unknown fields and commands. However, the known
parts have been working well and received extensive testing and use.

In order for this driver to work, the proper SPI drivers need to be
loaded too; for MB8,1 these are spi_pxa2xx_platform and spi_pxa2xx_pci;
for all others they are spi_pxa2xx_platform and intel_lpss_pci. For this
reason enabling this driver in the config implies enabling the above
drivers.

I applied the patch (but changed __u8 to u8 as that's preferred form for
inside the kernel, and added error handling for input_mt_init_slots) but
we need to do some more work on the driver.

Looks good.

My main issue is with registering touchpad device asynchronously,
independent from the probe() function. This means (as far as I can tell)
that any error is not really appropriately handled (as by that time it
is too late to signal errors from probe()) and devm functions are not
going to be called, leaving remnants of the resources in memory on
driver unload. It also brings in issues with suspend/resume (what
happens if you suspend really quickly while device is not registered
yet?), etc, etc.

Yes, the lack of error propagation also bothered me a bit when I
introduced the dev-info command, but I thought doing synchronous
I/O operations in a probe function wasn't kosher. Happy to rectify
that though.

Can we switch to calling DEV_INFO command synchronously from probe()? If
we are concerned about it taking relatively long time we can always
annotate the driver as having probe_type = PROBE_PREFER_ASYNCHRONOUS so
that other devices can be probed simultaneously with applespi.

Normally the dev-info retrieval takes about 15ms - I presume that's
fast enough?

Attached is a patch that does this now (on top of your changes above).
The cancelling of outstanding spi requests in the error case is a bit
ugly (I wish there were an exported spi-flush/spi-wait-for-queue-empty
function or similar), but otherwise it's fairly straightforward.


  Cheers,

  Ronald