| Message ID | 20250212-wilc3000_bt-v1-1-9609b784874e@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bluetooth: hci_wilc: add new bluetooth driver | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/SubjectPrefix | fail | "Bluetooth: " prefix is not specified in the subject |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
| tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
| tedd_an/TestRunner_iso-tester | fail | TestRunner_iso-tester: BUG: KASAN: slab-use-after-free in iso_conn_hold_unless_zero+0x78/0x1c0 |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | fail | TestRunner_mgmt-tester: Total: 490, Passed: 483 (98.6%), Failed: 3, Not Run: 4 |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=933239 ---Test result--- Test Summary: CheckPatch PENDING 0.23 seconds GitLint PENDING 0.20 seconds SubjectPrefix FAIL 1.93 seconds BuildKernel PASS 25.72 seconds CheckAllWarning PASS 28.21 seconds CheckSparse PASS 32.61 seconds BuildKernel32 PASS 25.49 seconds TestRunnerSetup PASS 450.87 seconds TestRunner_l2cap-tester PASS 21.43 seconds TestRunner_iso-tester FAIL 65.35 seconds TestRunner_bnep-tester PASS 5.04 seconds TestRunner_mgmt-tester FAIL 123.64 seconds TestRunner_rfcomm-tester PASS 7.87 seconds TestRunner_sco-tester PASS 9.78 seconds TestRunner_ioctl-tester PASS 8.67 seconds TestRunner_mesh-tester PASS 6.31 seconds TestRunner_smp-tester PASS 10.92 seconds TestRunner_userchan-tester PASS 5.35 seconds IncrementalBuild PENDING 0.60 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: SubjectPrefix - FAIL Desc: Check subject contains "Bluetooth" prefix Output: "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject "Bluetooth: " prefix is not specified in the subject ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: BUG: KASAN: slab-use-after-free in iso_conn_hold_unless_zero+0x78/0x1c0 Total: 125, Passed: 121 (96.8%), Failed: 0, Not Run: 4 ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 490, Passed: 483 (98.6%), Failed: 3, Not Run: 4 Failed Test Cases LL Privacy - Add Device 3 (AL is full) Failed 0.186 seconds LL Privacy - Set Flags 1 (Add to RL) Failed 0.138 seconds LL Privacy - Set Flags 3 (2 Devices to RL) Failed 0.174 seconds ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
On Wed, Feb 12, 2025 at 04:46:20PM +0100, Alexis Lothoré wrote: > WILC3000 is a combo chip providing 802.11b/g/n and Bluetooth 5. The wlan > part is exposed either through SDIO or SPI interface, and the bluetooth > part is exposed through uart. The notable peculiarity of this chip is > that the bluetooth part is not fully autonomous: its firmware is not > loaded through UART interface but through SDIO/SPI interface, so the > bluetooth description needs a reference to the wlan part to get access > to the corresponding bus. > > Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> > --- > .../net/bluetooth/microchip,wilc3000-bt.yaml | 41 ++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/bluetooth/microchip,wilc3000-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/microchip,wilc3000-bt.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..2a83ca3ad90b26fd619b574bc343bee9654a1e43 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/bluetooth/microchip,wilc3000-bt.yaml > @@ -0,0 +1,41 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/bluetooth/microchip,wilc3000-bt.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip Bluetooth chips > + > +description: > + This binding describes UART-attached Microchip bluetooth chips. These > + chips are dual-radio chips supporting WiFi and Bluetooth. The bluetooth > + side works with standard HCI commands over 4-wires UART (with flow > + control) > + > +maintainers: > + - Alexis Lothoré <alexis.lothore@bootlin.com> > + > +properties: > + compatible: > + enum: > + - microchip,wilc3000-bt > + > + wlan: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Phandle to the wlan part of the combo chip No resources here and judging by the driver everything is part of wifi. Either you wrote it to match driver or indeed hardware is like that. In the first case, why this cannot be part of WiFi with phandle to serial bus? In the second case, this needs to be proper hardware description. Best regards, Krzysztof
Hi Krzysztof, On 2/13/25 10:25, Krzysztof Kozlowski wrote: >> + wlan: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + Phandle to the wlan part of the combo chip > > No resources here and judging by the driver everything is part of wifi. > Either you wrote it to match driver or indeed hardware is like that. In > the first case, why this cannot be part of WiFi with phandle to serial > bus? In the second case, this needs to be proper hardware description First, I'd like to reclarify what the chip exactly is, to make sure that we are talking about the same thing. The wilc3000 ([1]) is a single physical device packaging two different discrete modules inside (one for 802.11, one for bluetooth). The WLAN part has its own binding integrated in upstream kernel ([2]) and is based on a similar chip in the same family (wilc1000, which only have 802.11, and so only SPI/SDIO, no UART). Now that it is said, no, I did not write this binding only aiming to match the new driver. I tried to base this description on how similar WLAN/BT combo chips are usually described (based on those which have existing bindings), and they seem to describe distinctly the two internal parts of those chips as well. For those who use HCI commands over uart for the bluetooth part, they expose a dedicated child node of a serial controller (distinct from the wlan part, described as another node on PCI/SDIO/SPI/etc). The hardware architecture for wilc3000 is similar to those, so since the serial bus is the primary interface to operate the bluetooth part inside the chip, doesn't it makes sense to have it under a serial controller node (and then to refer to wlan for the additional operations needed on sdio/spi), than the other way around ? About the lack of other resources in the new node: there are indeed additional resources that affect bluetooth, but I am not sure how it should be handled: there are for example a reset input and a chip enable input exposed by this chip, which in fact do not only affect the WLAN part but the two parts inside the chip. But those are currently described and handled by the WLAN part. I guess that an improvement regarding this point could then be to move those out of the wlan binding, and find a way to describe it as shared resources between those two parts of the chip, but how should it be handled ? Is it ok to remove those from an existing binding (and if so, where to put those ? It is not bluetooth specific neither) ? Is the issue rather about current WILC3000 binding kind of mixing overall chip description and internal wlan part description ? Thanks, Alexis [1] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/IEEE-802.11-b-g-n-Link-Controller-Module-with-Integrated-Bluetooth-5.0-DS70005327B.pdf [2] https://elixir.bootlin.com/linux/v6.12.6/source/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml > Best regards, > Krzysztof >
On 13/02/2025 16:25, Alexis Lothoré wrote: > Hi Krzysztof, > > On 2/13/25 10:25, Krzysztof Kozlowski wrote: >>> + wlan: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: >>> + Phandle to the wlan part of the combo chip >> >> No resources here and judging by the driver everything is part of wifi. >> Either you wrote it to match driver or indeed hardware is like that. In >> the first case, why this cannot be part of WiFi with phandle to serial >> bus? In the second case, this needs to be proper hardware description > > First, I'd like to reclarify what the chip exactly is, to make sure that we are > talking about the same thing. The wilc3000 ([1]) is a single physical device > packaging two different discrete modules inside (one for 802.11, one for > bluetooth). The WLAN part has its own binding integrated in upstream kernel > ([2]) and is based on a similar chip in the same family (wilc1000, which only > have 802.11, and so only SPI/SDIO, no UART). > > Now that it is said, no, I did not write this binding only aiming to match the > new driver. I tried to base this description on how similar WLAN/BT combo chips > are usually described (based on those which have existing bindings), and they > seem to describe distinctly the two internal parts of those chips as well. For Yes, because BT part is isolated enough that you datasheet tells which resources belong to it and DT describes these resources. You do not have any resources here. > those who use HCI commands over uart for the bluetooth part, they expose a > dedicated child node of a serial controller (distinct from the wlan part, > described as another node on PCI/SDIO/SPI/etc). The hardware architecture for > wilc3000 is similar to those, so since the serial bus is the primary interface > to operate the bluetooth part inside the chip, doesn't it makes sense to have it > under a serial controller node (and then to refer to wlan for the additional > operations needed on sdio/spi), than the other way around ? There is little benefit in describing one device in two places. This even lead to probe ordering issues and power sequencing problems, for example being explicitly addressed by recent power sequencing work from Bartosz by creating *third* node... You have no resources here, so I cannot even imagine inventing something in that third (PMU) node. But anyway in case of WCN devices, the reason of power sequencing patches, BT and WiFi are separate, can be enabled separately, can be even shipped one without another (I think). Not your case. Your BT needs WiFi to load firmware. > > About the lack of other resources in the new node: there are indeed additional > resources that affect bluetooth, but I am not sure how it should be handled: You sent us then incomplete hardware description. So you got review like this. > there are for example a reset input and a chip enable input exposed by this > chip, which in fact do not only affect the WLAN part but the two parts inside > the chip. But those are currently described and handled by the WLAN part. I So everything is already defined in WiFi part and this node is not really necessary. > guess that an improvement regarding this point could then be to move those out > of the wlan binding, and find a way to describe it as shared resources between > those two parts of the chip, but how should it be handled ? Is it ok to remove > those from an existing binding (and if so, where to put those ? It is not > bluetooth specific neither) ? Is the issue rather about current WILC3000 binding > kind of mixing overall chip description and internal wlan part description ? Datasheet shows this as one device, not two, not three. Reset and chip enable, based on datasheet, are not specific to BT. They reset/enable entire chip, I assume, so it is even better proof that your HW description is not correct. How this device is supposed to work if you do not enable WiFi - do not deassert reset from Wifi driver? BT will stay in reset. Really, and that's correct hardware representation? This is exactly the power sequencing problem one more time and you keep asking exactly the same questions and repeating exactly the same mistakes. This is incomplete and, IMO, incorrect hardware description. Read all previous discussions, read/listen/watch the talks about power sequencing (there were two on LPCs - one with Abel to describe the problem, year later to solve it). Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/bluetooth/microchip,wilc3000-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/microchip,wilc3000-bt.yaml new file mode 100644 index 0000000000000000000000000000000000000000..2a83ca3ad90b26fd619b574bc343bee9654a1e43 --- /dev/null +++ b/Documentation/devicetree/bindings/net/bluetooth/microchip,wilc3000-bt.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/bluetooth/microchip,wilc3000-bt.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip Bluetooth chips + +description: + This binding describes UART-attached Microchip bluetooth chips. These + chips are dual-radio chips supporting WiFi and Bluetooth. The bluetooth + side works with standard HCI commands over 4-wires UART (with flow + control) + +maintainers: + - Alexis Lothoré <alexis.lothore@bootlin.com> + +properties: + compatible: + enum: + - microchip,wilc3000-bt + + wlan: + $ref: /schemas/types.yaml#/definitions/phandle + description: + Phandle to the wlan part of the combo chip + +required: + - compatible + - wlan + +additionalProperties: false + +examples: + - | + serial { + bluetooth { + compatible = "microchip,wilc3000-bt"; + wlan = <&wifi>; + }; + };
WILC3000 is a combo chip providing 802.11b/g/n and Bluetooth 5. The wlan part is exposed either through SDIO or SPI interface, and the bluetooth part is exposed through uart. The notable peculiarity of this chip is that the bluetooth part is not fully autonomous: its firmware is not loaded through UART interface but through SDIO/SPI interface, so the bluetooth description needs a reference to the wlan part to get access to the corresponding bus. Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> --- .../net/bluetooth/microchip,wilc3000-bt.yaml | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+)