diff mbox

[v2,4/4] spi: Use Apple device properties in absence of ACPI resources

Message ID bca7fb9e406bbfa9ee7e8457cacd34418ef689be.1498636759.git.lukas@wunner.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lukas Wunner June 28, 2017, 5:20 p.m. UTC
MacBooks and MacBook Pros introduced since 2015 return empty _CRS data
for SPI slaves, causing device initialization to fail.  Most of the
information that would normally be conveyed via _CRS is available
through ACPI device properties instead, so take advantage of them.

The meaning and appropriate usage of the device properties was reverse
engineered by Ronald Tschalär and carried over from these commits
authored by him:

https://github.com/cb22/macbook12-spi-driver/commit/9a416d699ef4
https://github.com/cb22/macbook12-spi-driver/commit/0c34936ed9a1

According to Ronald, the device properties have the following meaning:

spiSclkPeriod   /* period in ns */
spiWordSize     /* in number of bits */
spiBitOrder     /* 1 = MSB_FIRST, 0 = LSB_FIRST */
spiSPO          /* clock polarity: 0 = low, 1 = high */
spiSPH          /* clock phase: 0 = first, 1 = second */
spiCSDelay      /* delay between cs and receive on reads in 10 us */
resetA2RUsec    /* active-to-receive delay? */
resetRecUsec    /* receive delay? */

Cc: Mark Brown <broonie@kernel.org>
Cc: Federico Lorenzi <florenzi@gmail.com>
Reported-by: Leif Liddy <leif.liddy@gmail.com>
Tested-by: Ronald Tschalär <ronald@innovation.ch>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Changes v1 -> v2:
- Move IS_ENABLED() and dmi_match() conditionals into callee. (Mika)
- Allow acpi_spi_parse_apple_properties() to augment or override _CRS
  by calling it unconditionally rather than only if _CRS is empty.

 drivers/spi/spi.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

Mark Brown June 28, 2017, 6:27 p.m. UTC | #1
On Wed, Jun 28, 2017 at 07:20:19PM +0200, Lukas Wunner wrote:
> MacBooks and MacBook Pros introduced since 2015 return empty _CRS data
> for SPI slaves, causing device initialization to fail.  Most of the
> information that would normally be conveyed via _CRS is available
> through ACPI device properties instead, so take advantage of them.

Acked-by: Mark Brown <broonie@kernel.org>
Andy Shevchenko June 28, 2017, 7:14 p.m. UTC | #2
On Wed, Jun 28, 2017 at 8:20 PM, Lukas Wunner <lukas@wunner.de> wrote:
> MacBooks and MacBook Pros introduced since 2015 return empty _CRS data
> for SPI slaves, causing device initialization to fail.  Most of the
> information that would normally be conveyed via _CRS is available
> through ACPI device properties instead, so take advantage of them.

> spiSclkPeriod   /* period in ns */

> +#include <linux/math64.h>

> +       if (!acpi_dev_get_property(dev, "spiSclkPeriod", ACPI_TYPE_BUFFER, &o))
> +               spi->max_speed_hz  = div64_u64(NSEC_PER_SEC,
> +                                              *(u64 *)o->buffer.pointer);

Why do you need 64 bit division here? It's obviously 32. It makes
nonsense to have variable more than NSEC_PER_SEC (if one paranoid
enough it could be checked).

> +       if (!acpi_dev_get_property(dev, "spiWordSize", ACPI_TYPE_BUFFER, &o))
> +               spi->bits_per_word = *(u64 *)o->buffer.pointer;
> +

> +       if (!acpi_dev_get_property(dev, "spiBitOrder", ACPI_TYPE_BUFFER, &o) &&
> +           !*(u64 *)o->buffer.pointer)

In such cases I would prefer the notation like
ret = func();
if (!ret && ...)
 ...

Also we usually name object variable as obj, though it's minor here.

> +               spi->mode |= SPI_LSB_FIRST;
Mika Westerberg June 29, 2017, 7:54 a.m. UTC | #3
On Wed, Jun 28, 2017 at 07:20:19PM +0200, Lukas Wunner wrote:
> MacBooks and MacBook Pros introduced since 2015 return empty _CRS data
> for SPI slaves, causing device initialization to fail.  Most of the
> information that would normally be conveyed via _CRS is available
> through ACPI device properties instead, so take advantage of them.
> 
> The meaning and appropriate usage of the device properties was reverse
> engineered by Ronald Tschalär and carried over from these commits
> authored by him:
> 
> https://github.com/cb22/macbook12-spi-driver/commit/9a416d699ef4
> https://github.com/cb22/macbook12-spi-driver/commit/0c34936ed9a1
> 
> According to Ronald, the device properties have the following meaning:
> 
> spiSclkPeriod   /* period in ns */
> spiWordSize     /* in number of bits */
> spiBitOrder     /* 1 = MSB_FIRST, 0 = LSB_FIRST */
> spiSPO          /* clock polarity: 0 = low, 1 = high */
> spiSPH          /* clock phase: 0 = first, 1 = second */
> spiCSDelay      /* delay between cs and receive on reads in 10 us */
> resetA2RUsec    /* active-to-receive delay? */
> resetRecUsec    /* receive delay? */
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Federico Lorenzi <florenzi@gmail.com>
> Reported-by: Leif Liddy <leif.liddy@gmail.com>
> Tested-by: Ronald Tschalär <ronald@innovation.ch>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 89254a55eb2e..51b7bdf841e4 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -21,6 +21,8 @@ 
 #include <linux/cache.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
+#include <linux/dmi.h>
+#include <linux/math64.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
@@ -1685,6 +1687,32 @@  static void of_register_spi_devices(struct spi_master *master) { }
 #endif
 
 #ifdef CONFIG_ACPI
+static void acpi_spi_parse_apple_properties(struct spi_device *spi)
+{
+	struct acpi_device *dev = ACPI_COMPANION(&spi->dev);
+	const union acpi_object *o;
+
+	if (!IS_ENABLED(CONFIG_X86) || !dmi_match(DMI_SYS_VENDOR, "Apple Inc."))
+		return;
+
+	if (!acpi_dev_get_property(dev, "spiSclkPeriod", ACPI_TYPE_BUFFER, &o))
+		spi->max_speed_hz  = div64_u64(NSEC_PER_SEC,
+					       *(u64 *)o->buffer.pointer);
+
+	if (!acpi_dev_get_property(dev, "spiWordSize", ACPI_TYPE_BUFFER, &o))
+		spi->bits_per_word = *(u64 *)o->buffer.pointer;
+
+	if (!acpi_dev_get_property(dev, "spiBitOrder", ACPI_TYPE_BUFFER, &o) &&
+	    !*(u64 *)o->buffer.pointer)
+		spi->mode |= SPI_LSB_FIRST;
+	if (!acpi_dev_get_property(dev, "spiSPO", ACPI_TYPE_BUFFER, &o) &&
+	     *(u64 *)o->buffer.pointer)
+		spi->mode |= SPI_CPOL;
+	if (!acpi_dev_get_property(dev, "spiSPH", ACPI_TYPE_BUFFER, &o) &&
+	     *(u64 *)o->buffer.pointer)
+		spi->mode |= SPI_CPHA;
+}
+
 static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
 {
 	struct spi_device *spi = data;
@@ -1758,6 +1786,8 @@  static acpi_status acpi_register_spi_device(struct spi_master *master,
 				     acpi_spi_add_resource, spi);
 	acpi_dev_free_resource_list(&resource_list);
 
+	acpi_spi_parse_apple_properties(spi);
+
 	if (ret < 0 || !spi->max_speed_hz) {
 		spi_dev_put(spi);
 		return AE_OK;