On 03/09/2024 18:09, Alexis Lothoré wrote:
> Hello everyone,
>
> On 8/29/24 02:44, Marek Vasut wrote:
>> Document compatible string for the WILC3000 chip. The chip is similar
>> to WILC1000, except that the register layout is slightly different and
>> it does not support WPA3/SAE.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>> Signed-off-by: Marek Vasut <marex@xxxxxxx>
>
> [...]
>
>> .../bindings/net/wireless/microchip,wilc1000.yaml | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
>> index 2460ccc082371..5d40f22765bb6 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
>> +++ b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
>> @@ -16,7 +16,11 @@ description:
>>
>> properties:
>> compatible:
>> - const: microchip,wilc1000
>> + oneOf:
>> + - items:
>> + - const: microchip,wilc3000
>> + - const: microchip,wilc1000
>> + - const: microchip,wilc1000
>>
>> reg: true
>
> Following this series first revision, I have been taking a look at how to
> implement bluetooth feature for wilc3000 (the chip supports Bluetooth LE through
> a separated UART, see [1]), and I am facing some constraints. I feel like the
> possible solutions would conflict with this new binding, so even if I am a bit
> late to the party, I would like to expose the issue before the binding is merged
> in case we can find something which would allow to add bluetooth support without
> too much pain after the wlan part.
>
> Downstream driver currently does not implement bluetooth as a standard bluetooth
> driver (module in drivers/bluetooth, registering a HCI device) but only performs
> a minimal set of operations directly in the wlan part ([2]). Getting a version
> valid for upstream would need the following points to be addressed:
> 1. despite being controlled from a serial port for nominal operations, the
> bluetooth part also depends on the "wlan" bus (spi or sdio) for initialization
> 2. yet init steps are not performed on any kind of subsystem ops but through
> writes to a custom chardev
> 3. the driver does not register itself a hci interface, it is expected to be
> done by userspace (hciattach).
>
> It is only after those 3 steps that the chip can be used with standard hci
> commands over serial port. IMHO 1 is the biggest point, because it means that
> **a bluetooth driver for wilc3000 needs access to the bus used by wlan part**
> (so only describing the bluetooth part of the chip as a child node of an uart
> controller is not enough). Aside from bus access, I also expect some
> interactions between bluetooth and wifi (eg: power management, sleep/wakeup)
>
> After considering multiple solutions to try to share this bus between existing
> wlan driver and a new bt driver (mfd device, auxiliary bus, device link + some
Driver design should not have impact on bindings.
> handles, etc), my current best guess is to convert wilc driver to a MFD driver
> for wilc3000. I guess some work can be done so that the driver can still be
> shared between wilc1000 and wilc3000 _while_ remaining compatible with current
> wilc1000 description, but it would impact the DT description for wilc3000, which
> would need to switch from this:
>
> spi {
> wifi@0 {
> compatible = "microchip,wilc3000";
> [...]
> };
> };
>
> To something like this:
>
> spi {
> wilc@0 {
> compatible = "microchip,wilc3000"; /* mfd driver */
I do not see any reason why... or rather: What is MFD here? MFD is Linux
stuff and we talk about hardware.
> wifi {
> compatible = "microchip,wilc3000-wlan";
Why? Just merge it to parent...
> [...]
> };
> bt {
> compatible = "microchip,wilc3000-bt";
> XXXX; /* some link to the uart controller connected to the chip */
That's not how we represent UART devices. I don't understand why do you
need these - if for power sequencing, then use power sequencing
framework and describe associated hardware (there are some talks coming
about it in 2 weeks). If for something else, then for what?
> [...]
> };
> };
> };
>
> (and similar thing when wilc is driven over a sdio bus)
>
> Any opinion on this ? Would it make sense to describe wilc3000 chip that way ?
>
You described drivers, not wilc3000 chip...
Best regards,
Krzysztof
