Re: [PATCH v1] x86/platform/intel-mid: Enable bluetooth on Intel Edison

[Ingo Molnar]

* Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> Intel Edison has Wi-Fi + BT module attached and, since it's an SFI-enumerated
> platform, needs a platform data. Here we add bits to enable bluetooth device.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  arch/x86/platform/intel-mid/device_libs/Makefile   |   3 +-
>  .../platform/intel-mid/device_libs/platform_bt.c   | 108 +++++++++++++++++++++
>  2 files changed, 110 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_bt.c
> 
> diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> index 3dbde04febdc..53e0235e308f 100644
> --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> @@ -2,8 +2,9 @@
>  obj-$(subst m,y,$(CONFIG_PINCTRL_MERRIFIELD)) += platform_mrfld_pinctrl.o
>  # SDHCI Devices
>  obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI)) += platform_mrfld_sd.o
> -# WiFi
> +# WiFi + BT
>  obj-$(subst m,y,$(CONFIG_BRCMFMAC_SDIO)) += platform_bcm43xx.o
> +obj-$(subst m,y,$(CONFIG_BT_HCIUART_BCM)) += platform_bt.o
>  # IPC Devices
>  obj-$(subst m,y,$(CONFIG_MFD_INTEL_MSIC)) += platform_msic.o
>  obj-$(subst m,y,$(CONFIG_SND_MFLD_MACHINE)) += platform_msic_audio.o
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_bt.c b/arch/x86/platform/intel-mid/device_libs/platform_bt.c
> new file mode 100644
> index 000000000000..b8dcf7190e63
> --- /dev/null
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_bt.c
> @@ -0,0 +1,108 @@
> +/*
> + * Bluetooth platform data initialization file
> + *
> + * (C) Copyright 2017 Intel Corporation
> + * Author: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +
> +#include <linux/gpio/machine.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +#include <asm/intel-mid.h>
> +
> +struct bt_sfi_data {
> +	struct device *dev;
> +	const char *name;
> +	int (*setup)(struct bt_sfi_data *ddata);
> +};
> +
> +static struct gpiod_lookup_table tng_bt_sfi_gpio_table = {
> +	.dev_id	= "hci_bcm",
> +	.table	= {
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "device-wakeup", GPIO_ACTIVE_HIGH),
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "shutdown", GPIO_ACTIVE_HIGH),
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "host-wakeup", GPIO_ACTIVE_HIGH),

Minor nit: just out of general principle (because the rest of the code looks so 
nice) I'd properly tabulate the last column as well - something like:

		GPIO_LOOKUP("0000:00:0c.0", -1, "device-wakeup", GPIO_ACTIVE_HIGH),
		GPIO_LOOKUP("0000:00:0c.0", -1, "shutdown",      GPIO_ACTIVE_HIGH),
		GPIO_LOOKUP("0000:00:0c.0", -1, "host-wakeup",   GPIO_ACTIVE_HIGH),

Formatted that way the reviewer's eye can skip over those values in 100 
milliseconds, determining that all 3 values are GPIO_ACTIVE_HIGH.

Thanks,

	Ingo