Hi Tomas,
so I removed all the unneeded parties from this thread since there is no
point in spamming everybody with this.
> Add Intel Wireless MultiCom 3200 SDIO BT driver
> IWMC3200 is 4Wireless Com CHIP (GPS/BT/WiFi/WiMAX).
> wmc3200bt is derived from btsdio driver
>
> Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> ---
> drivers/bluetooth/Kconfig | 16 ++
> drivers/bluetooth/Makefile | 2 +
> drivers/bluetooth/iwmc3200bt.c | 553 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 571 insertions(+), 0 deletions(-)
> create mode 100644 drivers/bluetooth/iwmc3200bt.c
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 652367a..e520889 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -195,5 +195,21 @@ config BT_MRVL_SDIO
> Say Y here to compile support for Marvell BT-over-SDIO driver
> into the kernel or say M to compile it as module.
>
> +config BT_IWMC3200
> + tristate "Intel Wireless MultiCom 3200 SDIO BT driver"
> + depends on IWMC3200TOP && EXPERIMENTAL
> + help
> + Intel Wireless MultiCom 3200 SDIO Bluetooth driver
> +
> + Say Y here to compile support for IWMC3200 SDIO BT driver
> + into the kernel or say M to compile it as module.
> +
> +config BT_IWMC3200_DEBUG
> + bool "Enable verbose debugging for iwmc3200bt"
> + depends on BT_IWMC3200
> + help
> + Say Y here to enable IWMC3200 SDIO BT driver
> + verbose debugging
> +
> endmenu
>
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index b3f57d2..975036e 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -21,6 +21,8 @@ obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
> btmrvl-y := btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
>
> +obj-$(CONFIG_BT_IWMC3200) += iwmc3200bt.o
> +
sort this one before the Marvell driver since it is a one file only
driver.
> hci_uart-y := hci_ldisc.o
> hci_uart-$(CONFIG_BT_HCIUART_H4) += hci_h4.o
> hci_uart-$(CONFIG_BT_HCIUART_BCSP) += hci_bcsp.o
> diff --git a/drivers/bluetooth/iwmc3200bt.c b/drivers/bluetooth/iwmc3200bt.c
> new file mode 100644
> index 0000000..5aed7a3
> --- /dev/null
> +++ b/drivers/bluetooth/iwmc3200bt.c
> @@ -0,0 +1,553 @@
> +/*
> + * ibtsdio - Intel Wireless MultiCom 3200 Bluetooth SDIO Driver
> + * drivers/bluetooth/ibtsdio.c
> + *
> + * Copyright (C) 2009 Intel Corporation. All rights reserved.
> + *
> + * Based on drivers/bluetooth/btsdio.c
> + * Copyright (C) 2007 Cambridge Silicon Radio Ltd.
> + * Copyright (C) 2007 Marcel Holtmann <marcel@xxxxxxxxxxxx>
You mean "based on" or "90% copied" ;)
> +#ifdef CONFIG_BT_IWMC3200_DEBUG
> +#define IBT_DBG(func, fmt, arg...) \
> + dev_dbg(&((func)->dev), "%s: " fmt "\n" , __func__ , ## arg)
> +#define IBT_ERR(func, fmt, arg...) \
> + dev_err(&((func)->dev), "%s: " fmt "\n" , __func__ , ## arg)
> +#define IBT_DUMP(func, buf, size) do { \
> + char prefix_str[30 + 1]; \
> + scnprintf(prefix_str, 30, "%s %s", \
> + dev_driver_string(&((func)->dev)), \
> + dev_name(&((func)->dev))); \
> + print_hex_dump(KERN_DEBUG, prefix_str, DUMP_PREFIX_OFFSET, \
> + 16, 1, buf, size, false); \
> +} while (0)
> +
> +#define VD "-d"
> +#else
> +#define IBT_DBG(func, fmt, arg...)
> +#define IBT_ERR(func, fmt, arg...)
> +#define IBT_DUMP(func, buf, size)
> +#define VD
> +#endif /* CONFIG_BT_IWMC3200_DEBUG */
> +
> +#ifdef REPOSITORY_LABEL
> +#define RL REPOSITORY_LABEL
> +#else
> +#define RL local
> +#endif
All of this debug stuff has to go away. It is really not needed and
BT_DBG with dynamic_printk support seems more than enough.
> +#define IWMCBT_VERSION "0.1.6"
> +
> +#define DRIVER_VERSION IWMCBT_VERSION "-" __stringify(RL) VD
This one needs to be removed too. I don't care about the repository
label in upstream.
> +#define IWMC_BT_SDIO_DEVID (0x1406)
> +
> +static const struct sdio_device_id btsdio_table[] = {
> + /* IWMC3200BT (0x1406) */
> + { SDIO_DEVICE(SDIO_VENDOR_ID_INTEL, 0x1406)},
> + { } /* Terminating entry */
> +};
This is pointless. Just use the numbers directly. and put a proper
comment above SDIO_DEVICE with hardware this is. Also no need to repeat
the number in that comment.
> +
> +MODULE_DEVICE_TABLE(sdio, btsdio_table);
> +
> +struct btsdio_data {
> + struct hci_dev *hdev;
> + struct sdio_func *func;
> +
> + struct work_struct work;
> +
> + struct sk_buff_head txq;
> + unsigned char tx_buf[2048];
> + unsigned char rx_buf[2048];
> +};
I know that you guys copied btsdio driver, but if you change the name,
then also change the internal structure names. So this these should be
called iwmc3200_data and etc.
Also I prefer that we allocate tx_buf and rx_buf so that they are
ensured to be aligned and we don't have to play alignment tricks during
the packet sending and receiving.
> +#define REG_RDAT 0x00 /* Receiver Data */
> +#define REG_TDAT 0x00 /* Transmitter Data */
> +#define REG_PC_RRT 0x10 /* Read Packet Control */
> +#define REG_PC_WRT 0x11 /* Write Packet Control */
> +#define REG_RTC_STAT 0x12 /* Retry Control Status */
> +#define REG_RTC_SET 0x12 /* Retry Control Set */
> +#define REG_INTRD 0x13 /* Interrupt Indication */
> +#define REG_CL_INTRD 0x13 /* Interrupt Clear */
> +#define REG_EN_INTRD 0x14 /* Interrupt Enable */
> +#define REG_MD_STAT 0x20 /* Bluetooth Mode Status */
> +#define REG_H2D_COUNT_L 0x2C
> +#define REG_H2D_COUNT_H 0x2D
> +
> +#ifdef CONFIG_BT_IWMC3200_DEBUG
> +static char *type_names[] = {
> + "Illegal",
> + "HCI_COMMAND_PKT",
> + "HCI_ACLDATA_PKT",
> + "HCI_SCODATA_PKT",
> + "HCI_EVENT_PKT"
> +};
> +#endif /* CONFIG_BT_IWMC3200_DEBUG */
No unneeded debug extras please. This is a Bluetooth HCI driver. It is
really not that complicated.
<snip>
I need the debug cleanup first, before continuing looking at the rest of
the driver.
> +
> +module_init(btsdio_init);
> +module_exit(btsdio_exit);
> +
> +MODULE_AUTHOR("Gregory Paskar <gregory.paskar@xxxxxxxxx>");
> +MODULE_DESCRIPTION("IWMC Bluetooth SDIO driver ver " DRIVER_VERSION);
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("ibtsdio");
We are not using module aliases here. So that can be removed.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
