Updates for 4.14

[roadrunner2]

This contains the work to exclude the DSDT hacks on kernels 4.14+, as promised in #29 (comment) - tested on 4.14-rc1. Also included are some README updates and a small #ifdef fix.

[cb22]

@roadrunner2 great, this looks good to me. Unfortunately I cannot test at the moment, since I'm away and I've messed up my Linux installation (again).

I'm happy to merge it in if you are.

I guess at this point, we could probably start looking at doing some cleanup of the driver and getting it ready to send it upstream, now that it can attach without any hacks? (or not?)

[roadrunner2]

@cb22 On the one hand these changes are quite safe - other than a little code shuffling and conditional compilation, nothing changes. So yes, I'm quite comfortable with them. On the other hand there's no big hurry to get these in: things will work find on 4.14 even without these changes (it'll just keep using my hack instead of the new core features). So I'm fine either way, merging now or later.

Regarding cleanup, agreed: I've had that pretty much next on my list. Besides cleanup there is one piece of functionality mising: the code currently assumes the touchpad data always arrives in single 256-byte packet, but when more than 6 fingers are pressed (and the palm sometimes triggers that too) it can be split among 2 256-byte packets. As I head mentioned elsewhere I managed to figure out a few more of the header fields, which provide the info to recognize continuation packets, so I need to make some changes to handle that. After that there's some small stuff like #44 , though I think we could consider upstreaming even without that.

[Dunedan]

I can confirm the driver continues to work fine with this PR under Linux 4.14rc1. 👍

[l1k]

Regarding upstreaming, a few things that caught my eye while briefly looking over the code:

• applespi_setup_spi_message() is called on every read and write even though the message seems to be identical every time? Call this once for rd_m and wr_m during ->probe.
• Can the code be rearranged to get rid of the forward declaration for applespi_send_cmd_msg()?
• There are some // comments but kernel coding style uses /* */
• In how far can the code duplicated from the BCM5974 driver be reduced?

[roadrunner2]

@1lk Thanks for the feedback. My responses are:

• calling applespi_setup_spi_message() once: I don't think that's safe, as the message is modified by the spi driver (at the very least the status and some length fields); while it may be possible to trace the code and determine exactly what is modified, I think that would be brittle and unsafe.
• removing forward declaration of applespi_send_cmd_msg(): no, that can't be removed, or least not without replacing it with a different forward declaration. The reason is that there is a depedency loop: that function passes a callback, applespi_async_write_complete(), which calls applespi_cmd_msg_complete(), which in turn calls applespi_send_cmd_msg(). So one of these needs a forward declaration.
• comments: agreed, fixed, together with some other formatting issues.
• removing duplicated code: not sure there's much duplicated code left, but would have to investigate more closely.

[l1k]

calling applespi_setup_spi_message() once: I don't think that's safe, as the message is modified by the spi driver (at the very least the status and some length fields); while it may be possible to trace the code and determine exactly what is modified, I think that would be brittle and unsafe.

A lot of IIO drivers do that, see e.g. mcp320x.c: struct mcp320x contains the spi_message, two spi_transfers and the tx_buf and rx_buf. The ->probe hook initializes the spi_transfers with the buffers, then initializes the spi_message with the spi_transfers. The spi_message is then used again and again and again by mcp320x_adc_conversion().

[roadrunner2]

I see - thanks for the pointer. I've made this change now (see #48).