| Message ID | 20250302144731.117409-1-jerome.pouiller@silabs.com |
|---|---|
| Headers | show |
| Series | wfx: add support for WoWLAN on Silabs WF200 | expand |
On Sun, 2025-03-02 at 15:47 +0100, Jérôme Pouiller wrote: > This is the initial support for Wake-on-WLAN of Silicon WF200 chipset. This > version focus on the power management control. For now, the filtering > capabilities of the chip are not exposed. So any multicast frame (= any ARP > request) will wake up the host. > > I have this series of patches in my git tree for a while. I hesitated to > send it because the code is based on a proof of concept and I don't have > access to the hardware anymore. > > Therefore, this feature is experimental. However, the only way to reach > this code is to run "iw phy phy0 wowlan enable" or explicitly enable it in > /sys. So, I believe it makes sense to merged it in the stable tree. Thus, I > hope some users will be able to report their success (or their failure). > > v2: > - Fix compilation issue reported by "kernel test robot"[1]. Member > 'wowlan' only exist if CONFIG_PM. You should probably check patchwork too - now that we're running some checks, a missing 'static' jumped out: https://patchwork.kernel.org/project/linux-wireless/list/?series=939353 johannes
On Monday 3 March 2025 09:20:11 CET Johannes Berg wrote: > On Sun, 2025-03-02 at 15:47 +0100, Jérôme Pouiller wrote: > > This is the initial support for Wake-on-WLAN of Silicon WF200 chipset. This > > version focus on the power management control. For now, the filtering > > capabilities of the chip are not exposed. So any multicast frame (= any ARP > > request) will wake up the host. > > > > I have this series of patches in my git tree for a while. I hesitated to > > send it because the code is based on a proof of concept and I don't have > > access to the hardware anymore. > > > > Therefore, this feature is experimental. However, the only way to reach > > this code is to run "iw phy phy0 wowlan enable" or explicitly enable it in > > /sys. So, I believe it makes sense to merged it in the stable tree. Thus, I > > hope some users will be able to report their success (or their failure). > > > > v2: > > - Fix compilation issue reported by "kernel test robot"[1]. Member > > 'wowlan' only exist if CONFIG_PM. > > You should probably check patchwork too - now that we're running some > checks, a missing 'static' jumped out: > > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-wireless/list/?series=939353__;!!N30Cs7Jr!Ru1BMHVjq3TxTX0c6bA3EDM0EzKM7JcHyeEUJ3_-J4TFLsLpY-jDSudnehLp3DzQ4Nutky9oKreyyjRstvmwuPVT0tM$ Patchwork also reports two warnings that I am going to ignore: - "Target tree name not specified in the subject", I assume it is "wireless-next", but in the doubt I prefer to refrain. - Lines are larger then 80 columns. Checkpatch.pl now accepts up to 100 columns. I am not aware any local exception in net/, right?
On Tue, 2025-03-04 at 16:22 +0100, Jérôme Pouiller wrote: > > Patchwork also reports two warnings that I am going to ignore: > > - "Target tree name not specified in the subject", I assume it > is "wireless-next", but in the doubt I prefer to refrain. It should be wireless-next for anything that isn't fixes for the current cycle, and please do add it - without it the checker won't always be able to pick up the patches to test them: https://lore.kernel.org/linux-wireless/ec3a3d891acfe5ed8763271a1df4151d75daf25f.camel@sipsolutions.net/ > - Lines are larger then 80 columns. Checkpatch.pl now accepts up > to 100 columns. I am not aware any local exception in net/, right? It looks like that's not documented (https://docs.kernel.org/process/maintainer-netdev.html), but I had a conversation with Jakub about this in the past and he prefers to have the checks still at 80 because people were, in his telling, abusing it in a way and making really long lines for no good reason. I'm not going to be super strict about it, but I'd encourage everyone who sees that warning to see if they can do better. In this particular case, it's just a comment, so could trivially be wrapped, but I'm not going to complain about 85 columns. If someone's going to 100 columns with (text) comments though then I think that'd raise some eyebrows. Narrower text is easier to read anyway. johannes
On Wednesday 5 March 2025 08:40:51 CET Johannes Berg wrote: > On Tue, 2025-03-04 at 16:22 +0100, Jérôme Pouiller wrote: > > > > Patchwork also reports two warnings that I am going to ignore: > > > > - "Target tree name not specified in the subject", I assume it > > is "wireless-next", but in the doubt I prefer to refrain. > > It should be wireless-next for anything that isn't fixes for the current > cycle, and please do add it - without it the checker won't always be > able to pick up the patches to test them: > > https://urldefense.com/v3/__https://lore.kernel.org/linux-wireless/ec3a3d891acfe5ed8763271a1df4151d75daf25f.camel@sipsolutions.net/__;!!N30Cs7Jr!X-PjgfbhIZWbgAa9xgbQsoUtAFxrhIPOL3GoEq_3Nan4ktwxzvTu7V17Q3HSxfYgjtdupGn3xRoIJwxLu9f0CcZx3Ys$ > > > - Lines are larger then 80 columns. Checkpatch.pl now accepts up > > to 100 columns. I am not aware any local exception in net/, right? > > It looks like that's not documented > (https://urldefense.com/v3/__https://docs.kernel.org/process/maintainer-netdev.html__;!!N30Cs7Jr!X-PjgfbhIZWbgAa9xgbQsoUtAFxrhIPOL3GoEq_3Nan4ktwxzvTu7V17Q3HSxfYgjtdupGn3xRoIJwxLu9f0sNiJZZA$ ), but I had a > conversation with Jakub about this in the past and he prefers to have > the checks still at 80 because people were, in his telling, abusing it > in a way and making really long lines for no good reason. > > I'm not going to be super strict about it, but I'd encourage everyone > who sees that warning to see if they can do better. > > In this particular case, it's just a comment, so could trivially be > wrapped, but I'm not going to complain about 85 columns. If someone's > going to 100 columns with (text) comments though then I think that'd > raise some eyebrows. Narrower text is easier to read anyway. Thank you for the detailed answer. I will send a new version in a couple of days. Thus the various robots have time to test it.
On Wed, 2025-03-05 at 16:18 +0100, Jérôme Pouiller wrote: > > I will send a new version in a couple of days. Thus the various robots > have time to test it. Any particular reason? I don't think you need to break the lines in these two minor cases, it's only a couple of characters wider. IOW, I think I'd take these patches, but if you have something else to change about them I guess let me know. johannes
On Thursday 6 March 2025 09:10:05 CET Johannes Berg wrote: > > On Wed, 2025-03-05 at 16:18 +0100, Jérôme Pouiller wrote: > > > > I will send a new version in a couple of days. Thus the various robots > > have time to test it. > > Any particular reason? I don't think you need to break the lines in > these two minor cases, it's only a couple of characters wider. I have not opinion. I do as you prefer. > IOW, I think I'd take these patches, but if you have something else to > change about them I guess let me know. I though the missing target tree name was a blocker. If this is not a blocker, then you can merge it.
On Thu, 2025-03-06 at 09:35 +0100, Jérôme Pouiller wrote: > > I though the missing target tree name was a blocker. > Ah, no. That's just because it doesn't _always_ manage to apply it. It's only a blocker if that's the only thing it reported, but here it just said it was missing but went ahead anyway. Honestly, I have no idea why all that is, sorry. johannes
This is the initial support for Wake-on-WLAN of Silicon WF200 chipset. This version focus on the power management control. For now, the filtering capabilities of the chip are not exposed. So any multicast frame (= any ARP request) will wake up the host. I have this series of patches in my git tree for a while. I hesitated to send it because the code is based on a proof of concept and I don't have access to the hardware anymore. Therefore, this feature is experimental. However, the only way to reach this code is to run "iw phy phy0 wowlan enable" or explicitly enable it in /sys. So, I believe it makes sense to merged it in the stable tree. Thus, I hope some users will be able to report their success (or their failure). v2: - Fix compilation issue reported by "kernel test robot"[1]. Member 'wowlan' only exist if CONFIG_PM. [1] https://lore.kernel.org/oe-kbuild-all/202503021057.5qCOqraa-lkp@intel.com/ Jérôme Pouiller (5): wifi: wfx: align declarations between bus_spi.c and bus_sdio.c wifi: wfx: declare support for WoWLAN wifi: wfx: allow SPI device to wake up the host wifi: wfx: allow SDIO device to wake up the host wifi: wfx: allow to enable WoWLAN using NL80211 drivers/net/wireless/silabs/wfx/bus.h | 1 + drivers/net/wireless/silabs/wfx/bus_sdio.c | 52 ++++++++++++++++++++++ drivers/net/wireless/silabs/wfx/bus_spi.c | 45 +++++++++++++++++-- drivers/net/wireless/silabs/wfx/main.c | 12 +++++ drivers/net/wireless/silabs/wfx/sta.c | 25 +++++++++++ drivers/net/wireless/silabs/wfx/sta.h | 3 ++ 6 files changed, 134 insertions(+), 4 deletions(-)