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

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Wed, Jun 28, 2017 at 8:20 PM, Lukas Wunner <lukas@xxxxxxxxx> 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;

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html