mbox series

[RFC,v2,0/2] wifi: Nordic nRF70 series

Message ID 20250422175918.585022-1-artur@conclusive.pl
Headers show
Series wifi: Nordic nRF70 series | expand

Message

Artur Rojek April 22, 2025, 5:59 p.m. UTC
Hi all,

this is v2 of the Nordic nRF70 series.
The order of patches has been swapped to reflect the Device Tree
submission rules (bindings before implementation). I also replaced
the 'net: wireless:' prefix with 'wifi:' for the series, so hopefully
this time it correctly shows up in Patchwork.

Patch [1/2] now resolves all issues uncovered by dt_binding_check.
All gpio based properties have also been replaced with *-supply and
interrupts properties, where appropriate, and their usage clarified in
respective description fields.

Patch [2/2] addresses the same gpio usage concerns, now utilizing
the regulator API. Another major change is migration of
nrf7002_qfn_rf_params from being an array into a struct, in order to
better access its individual fields. All the remaining concerns from v1
have been addressed as well. 

As this is RFC, and none of my questions from v1 have been answered, I
will feature them again, while also adding a new one:

1) Nordic gave us permission to upstream the firmware blob [1] required
   to use this driver. As that needs to go through separate
   linux-firmware repository and is subject to different licensing,
   should I try to upstream it in parallel with this series, or does it
   need to wait until the kernel driver gets in? 

2) In AP mode, for each connected peer I maintain a pending queue for TX
   skbs that can't be transmitted while the peer is in power save mode.
   I then use a wiphy_work (nrf70_pending_worker) to move the collected
   skbs into a single hw queue once the peer is able to receive again.
   This means there can be multiple workers putting skbs onto the hw
   queue at any given time. As this scheme relies on the wiphy_work
   workqueue, can I assume that multiple workers will be able to run in
   parallel, even on a system with a single CPU? If not, what would be
   a better solution to the above problem?

3) nRF70 hardware communicates using byte packed, little-endian
   payloads (documented in nrf70_cmds.h). As these can get very large
   and complicated, I decided against writing some sort of endianness
   conversion scheme, and simply dropped big endian support by this
   driver. Is that acceptable?

4) Please put particular attention to the wiphy configuration. I am not
   100% confident I got all the flags/features/band caps right.

5) Should I add myself to the MAINTAINERS file regarding this driver, or
   is that not mandatory?

Cheers,
Artur

[1] https://github.com/nrfconnect/sdk-nrfxlib/raw/refs/heads/main/nrf_wifi/bin/ncs/default/nrf70.bin

Artur Rojek (2):
  dt-bindings: wifi: Add support for Nordic nRF70
  wifi: Add Nordic nRF70 series Wi-Fi driver

 .../bindings/net/wireless/nordic,nrf70.yaml   |   71 +
 drivers/net/wireless/Kconfig                  |    1 +
 drivers/net/wireless/Makefile                 |    1 +
 drivers/net/wireless/nordic/Kconfig           |   26 +
 drivers/net/wireless/nordic/Makefile          |    3 +
 drivers/net/wireless/nordic/nrf70.c           | 4703 +++++++++++++++++
 drivers/net/wireless/nordic/nrf70_cmds.h      | 1137 ++++
 drivers/net/wireless/nordic/nrf70_rf_params.h |   65 +
 8 files changed, 6007 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/nordic,nrf70.yaml
 create mode 100644 drivers/net/wireless/nordic/Kconfig
 create mode 100644 drivers/net/wireless/nordic/Makefile
 create mode 100644 drivers/net/wireless/nordic/nrf70.c
 create mode 100644 drivers/net/wireless/nordic/nrf70_cmds.h
 create mode 100644 drivers/net/wireless/nordic/nrf70_rf_params.h

Comments

Johannes Berg April 25, 2025, 6:11 p.m. UTC | #1
On Tue, 2025-04-22 at 19:59 +0200, Artur Rojek wrote:
> 1) Nordic gave us permission to upstream the firmware blob [1] required
>    to use this driver. As that needs to go through separate
>    linux-firmware repository and is subject to different licensing,
>    should I try to upstream it in parallel with this series, or does it
>    need to wait until the kernel driver gets in?

I have no idea. Chicken and egg, I guess.

> 2) In AP mode, for each connected peer I maintain a pending queue for TX
>    skbs that can't be transmitted while the peer is in power save mode.
>    I then use a wiphy_work (nrf70_pending_worker) to move the collected
>    skbs into a single hw queue once the peer is able to receive again.
>    This means there can be multiple workers putting skbs onto the hw
>    queue at any given time. As this scheme relies on the wiphy_work
>    workqueue, can I assume that multiple workers will be able to run in
>    parallel, even on a system with a single CPU? If not, what would be
>    a better solution to the above problem?

wiphy_work() is fully serialized regardless of the number of CPUs, it's
guaranteed that the wiphy mutex is held for the work execution, after
all.

johannes
Arend van Spriel April 28, 2025, 8:45 a.m. UTC | #2
+ Josh

On 4/25/2025 8:11 PM, Johannes Berg wrote:
> On Tue, 2025-04-22 at 19:59 +0200, Artur Rojek wrote:
>> 1) Nordic gave us permission to upstream the firmware blob [1] required
>>     to use this driver. As that needs to go through separate
>>     linux-firmware repository and is subject to different licensing,
>>     should I try to upstream it in parallel with this series, or does it
>>     need to wait until the kernel driver gets in?
> 
> I have no idea. Chicken and egg, I guess.

It used to be a check by the linux-firmware maintainer if the firmware 
blob is referenced in kernel module info. I assume the main linux tree 
is checked and not linux-next.

Regards,
Arend
Josh Boyer April 28, 2025, 5:10 p.m. UTC | #3
On Mon, Apr 28, 2025 at 4:46 AM Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> + Josh
>
> On 4/25/2025 8:11 PM, Johannes Berg wrote:
> > On Tue, 2025-04-22 at 19:59 +0200, Artur Rojek wrote:
> >> 1) Nordic gave us permission to upstream the firmware blob [1] required
> >>     to use this driver. As that needs to go through separate
> >>     linux-firmware repository and is subject to different licensing,
> >>     should I try to upstream it in parallel with this series, or does it
> >>     need to wait until the kernel driver gets in?
> >
> > I have no idea. Chicken and egg, I guess.

Parallel is fine.

> It used to be a check by the linux-firmware maintainer if the firmware
> blob is referenced in kernel module info. I assume the main linux tree
> is checked and not linux-next.

Eh.  We check for net-new firmware just to make sure we aren't growing
the repo for something that will never be used by an in-tree driver,
but if the driver is in linux-next that's good enough.

josh