On Fri, 22 Dec 2023 at 05:19, Hector Martin <marcan@xxxxxxxxx> wrote: > > > > On 2023/12/21 18:57, Arend van Spriel wrote: > > - SHA-cyfmac-dev-list@xxxxxxxxxxxx > > > > On 12/21/2023 1:49 AM, Hector Martin wrote: > >> > >> > >> On 2023/12/21 4:36, Arend van Spriel wrote: > >>> On 12/20/2023 7:14 PM, Hector Martin wrote: > >>>> > >>>> > >>>> On 2023/12/20 19:20, Kalle Valo wrote: > >>>>> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: > >>>>> > >>>>>>> Just recently a patch was posted to remove the Infineon list from > >>>>>>> MAINTAINERS because that company cares so little they have literally > >>>>>>> stopped accepting emails from us. Meanwhile they are telling their > >>>>>>> customers that they do not recommend upstream brcmfmac and they should > >>>>>>> use their downstream driver [1]. > >>>>>> > >>>>>> Unquestionably broadcom is not helping maintain things, and I think it > >>>>>> should matter. > >>>>>> > >>>>>> As Hector says, they point to their random driver dumps on their site > >>>>>> that you can't even download unless you are a "Broadcom community > >>>>>> member" or whatever, and hey - any company that works that way should > >>>>>> be seen as pretty much hostile to any actual maintenance and proper > >>>>>> development. > >>>>> > >>>>> Sadly this is the normal in the wireless world. All vendors focus on the > >>>>> latest generation, currently it's Wi-Fi 7, and lose interest on older > >>>>> generations. And vendors lose focus on the upstream drivers even faster, > >>>>> usually after a customer project ends. > >>>>> > >>>>> So in practise what we try to do is keep the drivers working somehow on > >>>>> our own, even after the vendors are long gone. If we would deliberately > >>>>> allow breaking drivers because vendor/corporations don't support us, I > >>>>> suspect we would have sevaral broken drivers in upstream. > >>>>> > >>>>>> If Daniel and Hector are responsive to actual problem reports for the > >>>>>> changes they cause, I do think that should count a lot. > >>>>> > >>>>> Sure, but they could also respect to the review comments. I find Arend's > >>>>> proposal is reasonable and that's what I would implement in v2. We > >>>>> (linux-wireless) make abstractions to workaround firmware problems or > >>>>> interface conflicts all the time, just look at ath10k for example. I > >>>>> would not be surprised if we need to add even more abstractions to > >>>>> brcmfmac in the future. And Arend is the expert here, he has best > >>>>> knowledge of Broadcom devices and I trust him. > >>>>> > >>>>> Has anyone even investigated what it would need to implement Arend's > >>>>> proposal? At least I don't see any indication of that. > >>>> > >>>> Of course we can implement it (and we will as we actually got a report > >>>> of this patch breaking Cypress now, finally). > >>>> > >>>> The question was never whether it could be done, we're already doing a > >>>> bunch of abstractions to deal with just the Broadcom-only side of things > >>>> too. The point I was trying to make is that we need to *know* what > >>>> firmware abstractions we need and *why* they are needed. We can't just > >>>> say, for every change, "well, nobody knows if the existing code works or > >>>> not, so let's just add an abstraction just in case the change breaks > >>>> something". As far as anyone involved in the discussions until now could > >>>> tell, this code was just something some Cypress person dumped upstream, > >>>> and nobody involved was being responsive to any of our inquiries, so > >>>> there was no way to be certain it worked at all, whether it was > >>>> supported in public firmware, or anything else. > >>>> > >>>> *Now* that we know the existing code is actually functional and not just > >>>> dead/broken, and that the WSEC approach is conversely not functional on > >>>> the Cypress firmwares, it makes sense to introduce an abstraction. > >>> > >>> Just a quick look in the git history could have told you that it was not > >>> just dumped upstream and at least one person was using it and extended > >>> it for 802.11r support (fast-roaming): > >>> > >>> > >>> author Paweł Drewniak <czajernia@xxxxxxxxx> 2021-08-24 23:13:30 +0100 > >>> committer Kalle Valo <kvalo@xxxxxxxxxxxxxx> 2021-08-29 11:33:07 +0300 > >>> commit 4b51de063d5310f1fb297388b7955926e63e45c9 (patch) > >>> tree ba2ccb5cbd055d482a8daa263f5e53531c07667f > >>> /drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > >>> parent 81f9ebd43659320a88cae8ed5124c50b4d47ab66 (diff) > >>> download wireless-4b51de063d5310f1fb297388b7955926e63e45c9.tar.gz > >>> brcmfmac: Add WPA3 Personal with FT to supported cipher suites > >>> This allows the driver to connect to BSSIDs supporting SAE with 802.11r. > >>> Tested on Raspberry Pi 4 Model B (STA) and UniFi 6LR/OpenWRT 21.02.0-rc2. > >>> AP was set to 'sae-mixed' (WPA2/3 Personal). > >>> > >>> Signed-off-by: Paweł Drewniak <czajernia@xxxxxxxxx> > >>> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx> > >>> Link: https://lore.kernel.org/r/20210824221330.3847139-1-czajernia@xxxxxxxxx > >> > >> Sure, but we also had user reports of it *not* actually working (maybe > >> it regressed?). We didn't know whether it was functional with the > >> linux-firmware blobs in any way, I wanted confirmation of that. And we > >> also didn't know that the patch *would* break it at all; perhaps the > >> Cypress firmware had also grown support for the WSEC mechanism. > >> > >> That's why I wanted someone to actually confirm the code worked (in some > >> subset of cases) and the patch didn't, before starting to introduce > >> conditionals. There is, of course, also the element that the Cypress > >> side has been long unmaintained, and if nobody is testing/giving > >> feedback/complaining, perhaps it's better to err on the side of maybe > >> breaking something and see if that gets someone to come out of the > >> woodwork if it really breaks, rather than tiptoeing around the code > >> without knowing what's going on and without anyone actually testing things. > > > > That is because you distrust the intent that Cypress was really > > contributing. They were and I trusted them to not just throw in a > > feature like WPA3. When Infineon took over they went mute. Upon > > reviewing your patch (again) I also sent an email to them asking > > specifically about the status of the sae_password interface. I did not > > use the mailing list which indeed bounces these days (hence removed > > them) but the last living soul that I had contact with about a year ago > > whether they were still comitted to be involved. I guess out of > > politeness or embarrassment I got confirmation they were and never heard > > from him again. The query about the sae_password interface is still pending. > > If only corporate acquisition politics didn't repeatedly throw a wrench > into this one... :/ > > This is where we are though, Infineon clearly doesn't care, so it's time > to move on. > > >> It's not about this *specific* patch, it's about the general situation > >> of not being able to touch firmware interfaces "just in case Cypress > >> breaks" being unsustainable in the long term. I wasn't pushing back > >> because I think this particular one will be hard, I was pushing back > >> because I can read the tea leaves and see this is not going to end well > >> if it's the approach we start taking for everything. We *need* someone > >> to be testing patches on Cypress, we can't just "try not to touch it" > >> and cross our fingers. That just ends in disaster, we are not going to > >> succeed in not breaking it either way and it's going to make the driver > >> worse. > > > > I admire you ability of reading tea leaves. You saw the Grim I reckon. > > Admittedly your responses on every comment from my side (or Kalle for > > that matter) was polarizing every discussion. That is common way people > > treat each other nowadays especially online where a conversation is just > > a pile of text going shit. It does not bring out the best in me either, > > but it was draining every ounce of energy from me so better end it by > > stepping out. > > The hilariously outdated kernel development model surely doesn't help > either (I've stated my opinion on this quite a few times if you've > followed around) ;) > > This stuff gets *really* frustrating when you're trying to improve what > is, I hope we can all admit, an undermaintained driver (that is not to > say it's anyone's fault personally), and end up getting held back due to > everything from coding style nitpicks to people not having the time to > be responsive. It's just not helpful. It's important to know when to > step aside and let people actually get stuff done. > > When Daniel started sending me brcmfmac patches downstream, I took a > look at a few of them, decided he knew what he was doing, and just > started pulling in his branches wholesale. Was it perfect? No, I had to > debug at least one regression at one point. But it took me less time to > do that than it would've to go through the commits with a fine toothed > comb, so it was clearly the right decision. > > That is not to say that should be the standard upstream (we make a point > of moving fast and breaking things more downstream, since it's a proving > ground for what eventually will be upstreamed), but I think it does > demonstrate the kind of delegation ability that is sorely lacking in > many drivers and subsystems in the kernel these days. Maintainers become > entrenched in their position, long beyond the point where they have the > time/motivation/ability to drive the code forward, and end up in the way > of new people who are trying to make a difference. I think Linus knows > full well the kernel maintainer community is stagnating. > > That doesn't mean people should step down entirely. But it does mean > they need to recognize when this is happening and, at least, proactively > try to bring new people in, instead of just continuing to play a > gatekeeping role. The role of maintainers should not be that of a wall > people have to climb over to get their changes in, it should be to guide > new contributors and help onboard people who can contribute, as peers > and eventually as future maintainers. > > Kalle, in the other thread you said "this is not fun anymore, this is > more like a business with requirements and demands coming from > everywhere.". That's what it feels like to us when our changes get > rejected because the local vars aren't in reverse Christmas tree order, > or because our commit messages have "v2:" in them. It feels like some > manager is trying to justify their position by creating busywork for > everyone else. Nobody should actually care about any of those things, > and if they do, they need to step back and really ask themselves how > they ended up believing that. If the goal is to enforce a reasonable > shared coding style so things don't spiral into chaos, FFS, let's just > do what every other project does these days and adopt clang-format. Then > *all* of us can stop wasting time on these trivialities and go back to > getting stuff done. And really, nobody cares about commit messages as > long as the tags are right, the subject line is succinct, and the > important information is in there. Extra stuff never hurt anyone. > > > I added the ground work for multi-vendor support, but have not decided > > on the approach to take. Abstract per firmware interface primitive or > > simply have a cfg80211.c and fwil_types.h per vendor OR implement a > > vendor-specific cfg80211 callback and override the default callback > > during the driver attach, ie. in brcmf_fwvid_wcc_attach(). The latter > > duplicates things, but lean towards that as it may be easier on the > > long-term. What do your tea leaves tell you ;-) > > FWIW, I was hoping you'd stay on at least as a reviewer. Your > contributions are valuable. You obviously know the driver and hardware > much better than most people. I encourage you to, at least, post a v2 of > the MAINTAINERS patch with yourself as an R: line. I generally agree with this email, especially that Arend should stay on as a reviewer/maintainer. We need more people as either maintainers/contributors/reviewers/code writers/testers, not less, to delegate, co-maintain, test, merge, make the code more portable to many wifi devices, etc. What really matters at the end of the day I guess is writing the code that works across all the devices and testing it. Which is why I spread awareness about this area, got 100s of responses, especially on Linkedin, there's at least a portion of these that want to help, in good spirits. Is mise le meas/Regards, Eric Curtin > > As far as the actual driver abstraction architecture, I'm going to leave > it to Daniel to decide what makes the most sense, since he's the one > introducing new mechanisms for that already. There's always room for > refactoring later though, depending on the direction things take with > the vendor split. BTW, clang-format also makes refactoring a lot less > painful ;) > > - Hector >
