| Message ID | 20230118122817.42466-4-francesco@dolcini.it (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bluetooth: hci_mrvl: Add serdev support for 88W8997 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Hi Francesco,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on bluetooth-next/master bluetooth/master horms-ipvs/master net/master net-next/master linus/master v6.2-rc4 next-20230118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230118122817.42466-4-francesco%40dolcini.it
patch subject: [PATCH v1 3/4] Bluetooth: hci_mrvl: Add serdev support for 88W8997
config: i386-randconfig-s002 (https://download.01.org/0day-ci/archive/20230119/202301190442.n6v4c2yc-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/2ae116c8ad209e0bf11559519915e511c44c28be
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919
git checkout 2ae116c8ad209e0bf11559519915e511c44c28be
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 olddefconfig
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/bluetooth/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/bluetooth/hci_mrvl.c:450:36: warning: 'mrvl_proto_8997' defined but not used [-Wunused-const-variable=]
450 | static const struct hci_uart_proto mrvl_proto_8997 = {
| ^~~~~~~~~~~~~~~
vim +/mrvl_proto_8997 +450 drivers/bluetooth/hci_mrvl.c
449
> 450 static const struct hci_uart_proto mrvl_proto_8997 = {
451 .id = HCI_UART_MRVL,
452 .name = "Marvell 8997",
453 .init_speed = 115200,
454 .oper_speed = 3000000,
455 .open = mrvl_open,
456 .close = mrvl_close,
457 .flush = mrvl_flush,
458 .set_baudrate = mrvl_set_baudrate,
459 .recv = mrvl_recv,
460 .enqueue = mrvl_enqueue,
461 .dequeue = mrvl_dequeue,
462 };
463
Hi Francesco,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on bluetooth-next/master bluetooth/master horms-ipvs/master net/master net-next/master linus/master v6.2-rc5 next-20230123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230118122817.42466-4-francesco%40dolcini.it
patch subject: [PATCH v1 3/4] Bluetooth: hci_mrvl: Add serdev support for 88W8997
config: hexagon-randconfig-r021-20230123 (https://download.01.org/0day-ci/archive/20230124/202301241423.sEVD92vC-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2ae116c8ad209e0bf11559519915e511c44c28be
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919
git checkout 2ae116c8ad209e0bf11559519915e511c44c28be
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/bluetooth/ lib/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/bluetooth/hci_mrvl.c:12:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/bluetooth/hci_mrvl.c:12:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/bluetooth/hci_mrvl.c:12:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
>> drivers/bluetooth/hci_mrvl.c:450:36: warning: unused variable 'mrvl_proto_8997' [-Wunused-const-variable]
static const struct hci_uart_proto mrvl_proto_8997 = {
^
7 warnings generated.
vim +/mrvl_proto_8997 +450 drivers/bluetooth/hci_mrvl.c
449
> 450 static const struct hci_uart_proto mrvl_proto_8997 = {
451 .id = HCI_UART_MRVL,
452 .name = "Marvell 8997",
453 .init_speed = 115200,
454 .oper_speed = 3000000,
455 .open = mrvl_open,
456 .close = mrvl_close,
457 .flush = mrvl_flush,
458 .set_baudrate = mrvl_set_baudrate,
459 .recv = mrvl_recv,
460 .enqueue = mrvl_enqueue,
461 .dequeue = mrvl_dequeue,
462 };
463
Hi Francesco, On Wed, Jan 18, 2023 at 4:30 AM Francesco Dolcini <francesco@dolcini.it> wrote: > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com> > > Add serdev support for the 88W8997 from NXP (previously Marvell). It > includes support for changing the baud rate. The command to change the > baud rate is taken from the user manual UM11483 Rev. 9 in section 7 > (Bring-up of Bluetooth interfaces) from NXP. > > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > --- > drivers/bluetooth/hci_mrvl.c | 88 +++++++++++++++++++++++++++++++++--- > 1 file changed, 81 insertions(+), 7 deletions(-) > > diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c > index fbc3f7c3a5c7..86f548998a18 100644 > --- a/drivers/bluetooth/hci_mrvl.c > +++ b/drivers/bluetooth/hci_mrvl.c > @@ -27,10 +27,12 @@ > #define MRVL_ACK 0x5A > #define MRVL_NAK 0xBF > #define MRVL_RAW_DATA 0x1F > +#define MRVL_SET_BAUDRATE 0xFC09 > > enum { > STATE_CHIP_VER_PENDING, > STATE_FW_REQ_PENDING, > + STATE_FW_LOADED, > }; > > struct mrvl_data { > @@ -254,6 +256,14 @@ static int mrvl_recv(struct hci_uart *hu, const void *data, int count) > if (!test_bit(HCI_UART_REGISTERED, &hu->flags)) > return -EUNATCH; > > + /* We might receive some noise when there is no firmware loaded. Therefore, > + * we drop data if the firmware is not loaded yet and if there is no fw load > + * request pending. > + */ > + if (!test_bit(STATE_FW_REQ_PENDING, &mrvl->flags) && > + !test_bit(STATE_FW_LOADED, &mrvl->flags)) > + return count; > + > mrvl->rx_skb = h4_recv_buf(hu->hdev, mrvl->rx_skb, data, count, > mrvl_recv_pkts, > ARRAY_SIZE(mrvl_recv_pkts)); > @@ -354,6 +364,7 @@ static int mrvl_load_firmware(struct hci_dev *hdev, const char *name) > static int mrvl_setup(struct hci_uart *hu) > { > int err; > + struct mrvl_data *mrvl = hu->priv; > > hci_uart_set_flow_control(hu, true); > > @@ -367,9 +378,9 @@ static int mrvl_setup(struct hci_uart *hu) > hci_uart_wait_until_sent(hu); > > if (hu->serdev) > - serdev_device_set_baudrate(hu->serdev, 3000000); > + serdev_device_set_baudrate(hu->serdev, hu->oper_speed); > else > - hci_uart_set_baudrate(hu, 3000000); > + hci_uart_set_baudrate(hu, hu->oper_speed); > > hci_uart_set_flow_control(hu, false); > > @@ -377,13 +388,56 @@ static int mrvl_setup(struct hci_uart *hu) > if (err) > return err; > > + set_bit(STATE_FW_LOADED, &mrvl->flags); > + > + return 0; > +} > + > +static int mrvl_set_baudrate(struct hci_uart *hu, unsigned int speed) > +{ > + int err; > + struct sk_buff *skb; > + struct mrvl_data *mrvl = hu->priv; > + __le32 speed_le = cpu_to_le32(speed); > + > + /* The firmware might be loaded by the Wifi driver over SDIO. We wait > + * up to 10s for the CTS to go up. Afterward, we know that the firmware > + * is ready. > + */ > + err = serdev_device_wait_for_cts(hu->serdev, true, 10000); > + if (err) { > + bt_dev_err(hu->hdev, "Wait for CTS failed with %d\n", err); > + return err; > + } > + > + set_bit(STATE_FW_LOADED, &mrvl->flags); > + > + skb = __hci_cmd_sync(hu->hdev, MRVL_SET_BAUDRATE, > + sizeof(speed_le), &speed_le, > + HCI_INIT_TIMEOUT); > + if (IS_ERR(skb)) { > + bt_dev_err(hu->hdev, "send command failed: %ld", PTR_ERR(skb)); > + return PTR_ERR(skb); > + } > + kfree_skb(skb); If you don't care about the skb just the command status use __hci_cmd_sync_status instead. > + > + serdev_device_set_baudrate(hu->serdev, speed); > + > + /* We forcefully have to send a command to the bluetooth module so that > + * the driver detects it after a baudrate change. This is foreseen by > + * hci_serdev by setting HCI_UART_VND_DETECT which then causes a dummy > + * local version read. > + */ > + set_bit(HCI_UART_VND_DETECT, &hu->hdev_flags); > + > return 0; > } > > -static const struct hci_uart_proto mrvl_proto = { > +static const struct hci_uart_proto mrvl_proto_8897 = { > .id = HCI_UART_MRVL, > .name = "Marvell", > .init_speed = 115200, > + .oper_speed = 3000000, > .open = mrvl_open, > .close = mrvl_close, > .flush = mrvl_flush, > @@ -393,18 +447,37 @@ static const struct hci_uart_proto mrvl_proto = { > .dequeue = mrvl_dequeue, > }; > > +static const struct hci_uart_proto mrvl_proto_8997 = { > + .id = HCI_UART_MRVL, > + .name = "Marvell 8997", > + .init_speed = 115200, > + .oper_speed = 3000000, > + .open = mrvl_open, > + .close = mrvl_close, > + .flush = mrvl_flush, > + .set_baudrate = mrvl_set_baudrate, > + .recv = mrvl_recv, > + .enqueue = mrvl_enqueue, > + .dequeue = mrvl_dequeue, > +}; > + > static int mrvl_serdev_probe(struct serdev_device *serdev) > { > struct mrvl_serdev *mrvldev; > + const struct hci_uart_proto *mrvl_proto = device_get_match_data(&serdev->dev); > > mrvldev = devm_kzalloc(&serdev->dev, sizeof(*mrvldev), GFP_KERNEL); > if (!mrvldev) > return -ENOMEM; > > + mrvldev->hu.oper_speed = mrvl_proto->oper_speed; > + if (mrvl_proto->set_baudrate) > + of_property_read_u32(serdev->dev.of_node, "max-speed", &mrvldev->hu.oper_speed); > + > mrvldev->hu.serdev = serdev; > serdev_device_set_drvdata(serdev, mrvldev); > > - return hci_uart_register_device(&mrvldev->hu, &mrvl_proto); > + return hci_uart_register_device(&mrvldev->hu, mrvl_proto); > } > > static void mrvl_serdev_remove(struct serdev_device *serdev) > @@ -416,7 +489,8 @@ static void mrvl_serdev_remove(struct serdev_device *serdev) > > #ifdef CONFIG_OF > static const struct of_device_id mrvl_bluetooth_of_match[] = { > - { .compatible = "mrvl,88w8897" }, > + { .compatible = "mrvl,88w8897", .data = &mrvl_proto_8897}, > + { .compatible = "mrvl,88w8997", .data = &mrvl_proto_8997}, > { }, > }; > MODULE_DEVICE_TABLE(of, mrvl_bluetooth_of_match); > @@ -435,12 +509,12 @@ int __init mrvl_init(void) > { > serdev_device_driver_register(&mrvl_serdev_driver); > > - return hci_uart_register_proto(&mrvl_proto); > + return hci_uart_register_proto(&mrvl_proto_8897); > } > > int __exit mrvl_deinit(void) > { > serdev_device_driver_unregister(&mrvl_serdev_driver); > > - return hci_uart_unregister_proto(&mrvl_proto); > + return hci_uart_unregister_proto(&mrvl_proto_8897); > } > -- > 2.25.1 >
Hi Francesco, On Mon, Jan 23, 2023 at 10:38 PM kernel test robot <lkp@intel.com> wrote: > > Hi Francesco, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on robh/for-next] > [also build test WARNING on bluetooth-next/master bluetooth/master horms-ipvs/master net/master net-next/master linus/master v6.2-rc5 next-20230123] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919 > base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next > patch link: https://lore.kernel.org/r/20230118122817.42466-4-francesco%40dolcini.it > patch subject: [PATCH v1 3/4] Bluetooth: hci_mrvl: Add serdev support for 88W8997 > config: hexagon-randconfig-r021-20230123 (https://download.01.org/0day-ci/archive/20230124/202301241423.sEVD92vC-lkp@intel.com/config) > compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a) > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/intel-lab-lkp/linux/commit/2ae116c8ad209e0bf11559519915e511c44c28be > git remote add linux-review https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919 > git checkout 2ae116c8ad209e0bf11559519915e511c44c28be > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/bluetooth/ lib/ > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > In file included from drivers/bluetooth/hci_mrvl.c:12: > In file included from include/linux/skbuff.h:17: > In file included from include/linux/bvec.h:10: > In file included from include/linux/highmem.h:12: > In file included from include/linux/hardirq.h:11: > In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: > In file included from include/asm-generic/hardirq.h:17: > In file included from include/linux/irq.h:20: > In file included from include/linux/io.h:13: > In file included from arch/hexagon/include/asm/io.h:334: > include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > val = __raw_readb(PCI_IOBASE + addr); > ~~~~~~~~~~ ^ > include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); > ~~~~~~~~~~ ^ > include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu' > #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) > ^ > In file included from drivers/bluetooth/hci_mrvl.c:12: > In file included from include/linux/skbuff.h:17: > In file included from include/linux/bvec.h:10: > In file included from include/linux/highmem.h:12: > In file included from include/linux/hardirq.h:11: > In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: > In file included from include/asm-generic/hardirq.h:17: > In file included from include/linux/irq.h:20: > In file included from include/linux/io.h:13: > In file included from arch/hexagon/include/asm/io.h:334: > include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); > ~~~~~~~~~~ ^ > include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu' > #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) > ^ > In file included from drivers/bluetooth/hci_mrvl.c:12: > In file included from include/linux/skbuff.h:17: > In file included from include/linux/bvec.h:10: > In file included from include/linux/highmem.h:12: > In file included from include/linux/hardirq.h:11: > In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: > In file included from include/asm-generic/hardirq.h:17: > In file included from include/linux/irq.h:20: > In file included from include/linux/io.h:13: > In file included from arch/hexagon/include/asm/io.h:334: > include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > __raw_writeb(value, PCI_IOBASE + addr); > ~~~~~~~~~~ ^ > include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); > ~~~~~~~~~~ ^ > include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); > ~~~~~~~~~~ ^ > >> drivers/bluetooth/hci_mrvl.c:450:36: warning: unused variable 'mrvl_proto_8997' [-Wunused-const-variable] > static const struct hci_uart_proto mrvl_proto_8997 = { This last error seems to be caused by your changes, please fix it.
On Fri, Feb 10, 2023 at 10:48:08AM -0800, Luiz Augusto von Dentz wrote: > Hi Francesco, > > On Mon, Jan 23, 2023 at 10:38 PM kernel test robot <lkp@intel.com> wrote: > > > > Hi Francesco, > > > > Thank you for the patch! Perhaps something to improve: > > > > [auto build test WARNING on robh/for-next] > > [also build test WARNING on bluetooth-next/master bluetooth/master horms-ipvs/master net/master net-next/master linus/master v6.2-rc5 next-20230123] > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > And when submitting patch, we suggest to use '--base' as documented in > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next > > patch link: https://lore.kernel.org/r/20230118122817.42466-4-francesco%40dolcini.it > > patch subject: [PATCH v1 3/4] Bluetooth: hci_mrvl: Add serdev support for 88W8997 > > config: hexagon-randconfig-r021-20230123 (https://download.01.org/0day-ci/archive/20230124/202301241423.sEVD92vC-lkp@intel.com/config) > > compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a) > > reproduce (this is a W=1 build): > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # https://github.com/intel-lab-lkp/linux/commit/2ae116c8ad209e0bf11559519915e511c44c28be > > git remote add linux-review https://github.com/intel-lab-lkp/linux > > git fetch --no-tags linux-review Francesco-Dolcini/dt-bindings-bluetooth-marvell-add-88W8997-DT-binding/20230118-210919 > > git checkout 2ae116c8ad209e0bf11559519915e511c44c28be > > # save the config file > > mkdir build_dir && cp config build_dir/.config > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/bluetooth/ lib/ > > > > If you fix the issue, kindly add following tag where applicable > > | Reported-by: kernel test robot <lkp@intel.com> > > > > All warnings (new ones prefixed by >>): > > > > In file included from drivers/bluetooth/hci_mrvl.c:12: > > In file included from include/linux/skbuff.h:17: > > In file included from include/linux/bvec.h:10: > > In file included from include/linux/highmem.h:12: > > In file included from include/linux/hardirq.h:11: > > In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: > > In file included from include/asm-generic/hardirq.h:17: > > In file included from include/linux/irq.h:20: > > In file included from include/linux/io.h:13: > > In file included from arch/hexagon/include/asm/io.h:334: > > include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > val = __raw_readb(PCI_IOBASE + addr); > > ~~~~~~~~~~ ^ > > include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); > > ~~~~~~~~~~ ^ > > include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu' > > #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) > > ^ > > In file included from drivers/bluetooth/hci_mrvl.c:12: > > In file included from include/linux/skbuff.h:17: > > In file included from include/linux/bvec.h:10: > > In file included from include/linux/highmem.h:12: > > In file included from include/linux/hardirq.h:11: > > In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: > > In file included from include/asm-generic/hardirq.h:17: > > In file included from include/linux/irq.h:20: > > In file included from include/linux/io.h:13: > > In file included from arch/hexagon/include/asm/io.h:334: > > include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); > > ~~~~~~~~~~ ^ > > include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu' > > #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) > > ^ > > In file included from drivers/bluetooth/hci_mrvl.c:12: > > In file included from include/linux/skbuff.h:17: > > In file included from include/linux/bvec.h:10: > > In file included from include/linux/highmem.h:12: > > In file included from include/linux/hardirq.h:11: > > In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: > > In file included from include/asm-generic/hardirq.h:17: > > In file included from include/linux/irq.h:20: > > In file included from include/linux/io.h:13: > > In file included from arch/hexagon/include/asm/io.h:334: > > include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > __raw_writeb(value, PCI_IOBASE + addr); > > ~~~~~~~~~~ ^ > > include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); > > ~~~~~~~~~~ ^ > > include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); > > ~~~~~~~~~~ ^ > > >> drivers/bluetooth/hci_mrvl.c:450:36: warning: unused variable 'mrvl_proto_8997' [-Wunused-const-variable] > > static const struct hci_uart_proto mrvl_proto_8997 = { > > This last error seems to be caused by your changes, please fix it. this is supposed to be fixed in v2.
diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c index fbc3f7c3a5c7..86f548998a18 100644 --- a/drivers/bluetooth/hci_mrvl.c +++ b/drivers/bluetooth/hci_mrvl.c @@ -27,10 +27,12 @@ #define MRVL_ACK 0x5A #define MRVL_NAK 0xBF #define MRVL_RAW_DATA 0x1F +#define MRVL_SET_BAUDRATE 0xFC09 enum { STATE_CHIP_VER_PENDING, STATE_FW_REQ_PENDING, + STATE_FW_LOADED, }; struct mrvl_data { @@ -254,6 +256,14 @@ static int mrvl_recv(struct hci_uart *hu, const void *data, int count) if (!test_bit(HCI_UART_REGISTERED, &hu->flags)) return -EUNATCH; + /* We might receive some noise when there is no firmware loaded. Therefore, + * we drop data if the firmware is not loaded yet and if there is no fw load + * request pending. + */ + if (!test_bit(STATE_FW_REQ_PENDING, &mrvl->flags) && + !test_bit(STATE_FW_LOADED, &mrvl->flags)) + return count; + mrvl->rx_skb = h4_recv_buf(hu->hdev, mrvl->rx_skb, data, count, mrvl_recv_pkts, ARRAY_SIZE(mrvl_recv_pkts)); @@ -354,6 +364,7 @@ static int mrvl_load_firmware(struct hci_dev *hdev, const char *name) static int mrvl_setup(struct hci_uart *hu) { int err; + struct mrvl_data *mrvl = hu->priv; hci_uart_set_flow_control(hu, true); @@ -367,9 +378,9 @@ static int mrvl_setup(struct hci_uart *hu) hci_uart_wait_until_sent(hu); if (hu->serdev) - serdev_device_set_baudrate(hu->serdev, 3000000); + serdev_device_set_baudrate(hu->serdev, hu->oper_speed); else - hci_uart_set_baudrate(hu, 3000000); + hci_uart_set_baudrate(hu, hu->oper_speed); hci_uart_set_flow_control(hu, false); @@ -377,13 +388,56 @@ static int mrvl_setup(struct hci_uart *hu) if (err) return err; + set_bit(STATE_FW_LOADED, &mrvl->flags); + + return 0; +} + +static int mrvl_set_baudrate(struct hci_uart *hu, unsigned int speed) +{ + int err; + struct sk_buff *skb; + struct mrvl_data *mrvl = hu->priv; + __le32 speed_le = cpu_to_le32(speed); + + /* The firmware might be loaded by the Wifi driver over SDIO. We wait + * up to 10s for the CTS to go up. Afterward, we know that the firmware + * is ready. + */ + err = serdev_device_wait_for_cts(hu->serdev, true, 10000); + if (err) { + bt_dev_err(hu->hdev, "Wait for CTS failed with %d\n", err); + return err; + } + + set_bit(STATE_FW_LOADED, &mrvl->flags); + + skb = __hci_cmd_sync(hu->hdev, MRVL_SET_BAUDRATE, + sizeof(speed_le), &speed_le, + HCI_INIT_TIMEOUT); + if (IS_ERR(skb)) { + bt_dev_err(hu->hdev, "send command failed: %ld", PTR_ERR(skb)); + return PTR_ERR(skb); + } + kfree_skb(skb); + + serdev_device_set_baudrate(hu->serdev, speed); + + /* We forcefully have to send a command to the bluetooth module so that + * the driver detects it after a baudrate change. This is foreseen by + * hci_serdev by setting HCI_UART_VND_DETECT which then causes a dummy + * local version read. + */ + set_bit(HCI_UART_VND_DETECT, &hu->hdev_flags); + return 0; } -static const struct hci_uart_proto mrvl_proto = { +static const struct hci_uart_proto mrvl_proto_8897 = { .id = HCI_UART_MRVL, .name = "Marvell", .init_speed = 115200, + .oper_speed = 3000000, .open = mrvl_open, .close = mrvl_close, .flush = mrvl_flush, @@ -393,18 +447,37 @@ static const struct hci_uart_proto mrvl_proto = { .dequeue = mrvl_dequeue, }; +static const struct hci_uart_proto mrvl_proto_8997 = { + .id = HCI_UART_MRVL, + .name = "Marvell 8997", + .init_speed = 115200, + .oper_speed = 3000000, + .open = mrvl_open, + .close = mrvl_close, + .flush = mrvl_flush, + .set_baudrate = mrvl_set_baudrate, + .recv = mrvl_recv, + .enqueue = mrvl_enqueue, + .dequeue = mrvl_dequeue, +}; + static int mrvl_serdev_probe(struct serdev_device *serdev) { struct mrvl_serdev *mrvldev; + const struct hci_uart_proto *mrvl_proto = device_get_match_data(&serdev->dev); mrvldev = devm_kzalloc(&serdev->dev, sizeof(*mrvldev), GFP_KERNEL); if (!mrvldev) return -ENOMEM; + mrvldev->hu.oper_speed = mrvl_proto->oper_speed; + if (mrvl_proto->set_baudrate) + of_property_read_u32(serdev->dev.of_node, "max-speed", &mrvldev->hu.oper_speed); + mrvldev->hu.serdev = serdev; serdev_device_set_drvdata(serdev, mrvldev); - return hci_uart_register_device(&mrvldev->hu, &mrvl_proto); + return hci_uart_register_device(&mrvldev->hu, mrvl_proto); } static void mrvl_serdev_remove(struct serdev_device *serdev) @@ -416,7 +489,8 @@ static void mrvl_serdev_remove(struct serdev_device *serdev) #ifdef CONFIG_OF static const struct of_device_id mrvl_bluetooth_of_match[] = { - { .compatible = "mrvl,88w8897" }, + { .compatible = "mrvl,88w8897", .data = &mrvl_proto_8897}, + { .compatible = "mrvl,88w8997", .data = &mrvl_proto_8997}, { }, }; MODULE_DEVICE_TABLE(of, mrvl_bluetooth_of_match); @@ -435,12 +509,12 @@ int __init mrvl_init(void) { serdev_device_driver_register(&mrvl_serdev_driver); - return hci_uart_register_proto(&mrvl_proto); + return hci_uart_register_proto(&mrvl_proto_8897); } int __exit mrvl_deinit(void) { serdev_device_driver_unregister(&mrvl_serdev_driver); - return hci_uart_unregister_proto(&mrvl_proto); + return hci_uart_unregister_proto(&mrvl_proto_8897); }