netdev - Re: [PATCH v4 1/5] dt-bindings: wireless: wilc1000: Document WILC3000 compatible string

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52e7b6d2-5d31-4ae1-bf1d-44e63a22774d@bootlin.com>
Date: Tue, 3 Sep 2024 18:09:10 +0200
From: Alexis Lothoré <alexis.lothore@...tlin.com>
To: Marek Vasut <marex@...x.de>, linux-wireless@...r.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 "David S. Miller" <davem@...emloft.net>,
 Adham Abozaeid <adham.abozaeid@...rochip.com>,
 Ajay Singh <ajay.kathat@...rochip.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>, Conor Dooley
 <conor+dt@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Kalle Valo <kvalo@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rob Herring <robh@...nel.org>, devicetree@...r.kernel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH v4 1/5] dt-bindings: wireless: wilc1000: Document WILC3000
 compatible string

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@...aro.org>
> Signed-off-by: Marek Vasut <marex@...x.de>

[...]

>  .../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
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 */
      wifi {
        compatible = "microchip,wilc3000-wlan";
        [...]
      };
      bt {
        compatible = "microchip,wilc3000-bt";
        XXXX; /* some link to the uart controller connected to the chip */
        [...]
      };
    };
  };

(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 ?

Thanks,
Alexis

[1] https://www.microchip.com/en-us/product/atwilc3000
[2]
https://github.com/linux4sam/linux-at91/blob/linux-6.6-mchp/drivers/net/wireless/microchip/wilc1000/bt.c

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com