On Tue, Feb 27, 2018 at 10:55:05AM +0100, Jan Schreiber wrote: > On 02/27/18 09:06, Stefan Sperling wrote: > > On Mon, Feb 26, 2018 at 11:55:34PM +0100, j...@posteo.de wrote: > >> When connecting to a wifi network messages like "iwm0: unhandled firmware > >> response 0xff/0xb8000010 rx ring" appear. > >> After looking at https://patchwork.kernel.org/patch/9869017/ I came up with > >> following patch. > >> The link also explains what is happening. > > > > Thanks for this! > > > >> I'm not sure if it makes sense to log the firmware responses like the linux > >> kernel does. > >> They don't appear to have any negative effect so I decided against it. > > > > Agreed. There is no point in logging this. > > > > There are two problems with your diff: > > > > The first is that somehow it got corrupted (whitespace got mangled, lines > > got wrapped which shouldn't have been). Please check your mailer settings. > > Tip: You can mail a diff to yourself and then try to apply it until you've > > figured out how to make your mail client work right. I've quoted the diff > > I received below as-is, so if you save this mail body and feed it into > > patch(1) you will see what problems I saw. > > In this particular instance it's not a huge burden because the diff adds > > just 3 lines, but I basically had to rewrite your patch from scratch to > > apply it, which is no fun, especially for larger diffs. > > Sorry for that. Seems my config was eaten. Again.
This new diff is still broken (looks like tabs got converted to spaces)? You obviously didn't try to send a diff to yourself first :( I am just going to apply it manually now. But please invest time into fixing this properly before submitting any more diffs. For every diff you send out there is a potential amount of N people who will try to test it. Problems like this are just distracting and a waste of time for anyone volunteering some of their own time to try to help you. Regardless, I am very grateful for the work you did in tracking this down. This has been a long-standing problem and you've provided the missing link to information which allowed us to understand why it happened. |diff --git sys/dev/pci/if_iwm.c sys/dev/pci/if_iwm.c |index 476c61f0b3d..af3eeb76978 100644 |--- sys/dev/pci/if_iwm.c |+++ sys/dev/pci/if_iwm.c -------------------------- Patching file if_iwm.c using Plan A... Hunk #1 failed at 7316. 1 out of 1 hunks failed--saving rejects to if_iwm.c.rej Hmm... The next patch looks like a unified diff to me... The text leading up to this was: -------------------------- |diff --git sys/dev/pci/if_iwmreg.h sys/dev/pci/if_iwmreg.h |index 3c5cfea7b23..06eef017015 100644 |--- sys/dev/pci/if_iwmreg.h |+++ sys/dev/pci/if_iwmreg.h -------------------------- Patching file if_iwmreg.h using Plan A... Hunk #1 failed at 1813. Hunk #2 failed at 5823. 2 out of 2 hunks failed--saving rejects to if_iwmreg.h.rej Hmm... Ignoring the trailing garbage. done > > The next problem: You're catching this notification in a case block which > > handles commands we expect to get a response for, but we don't even want > > to parse the data contained in this notification. > > I think it would be better to move it to a different case: block which just > > does a 'break;'. There should be some of those already so you could add it > > there, or you could introduce a new case: block, whichever you prefer. > > I moved the catch which is marked as ignored. Thanks for the review! > > diff --git sys/dev/pci/if_iwm.c sys/dev/pci/if_iwm.c > index 476c61f0b3d..af3eeb76978 100644 > --- sys/dev/pci/if_iwm.c > +++ sys/dev/pci/if_iwm.c > @@ -7316,6 +7316,10 @@ iwm_notif_intr(struct iwm_softc *sc) > break; > } > > + case IWM_WIDE_ID(IWM_SYSTEM_GROUP, > + IWM_FSEQ_VER_MISMATCH_NOTIFICATION): > + break; > + > /* > * Firmware versions 21 and 22 generate some DEBUG_LOG_MSG > * messages. Just ignore them for now. > diff --git sys/dev/pci/if_iwmreg.h sys/dev/pci/if_iwmreg.h > index 3c5cfea7b23..06eef017015 100644 > --- sys/dev/pci/if_iwmreg.h > +++ sys/dev/pci/if_iwmreg.h > @@ -1813,6 +1813,9 @@ struct iwm_agn_scd_bc_tbl { > #define IWM_NET_DETECT_HOTSPOTS_CMD 0x58 > #define IWM_NET_DETECT_HOTSPOTS_QUERY_CMD 0x59 > > +/* system group command IDs */ > +#define IWM_FSEQ_VER_MISMATCH_NOTIFICATION 0xff > + > #define IWM_REPLY_MAX 0xff > > /** > @@ -5820,6 +5823,7 @@ iwm_cmd_id(uint8_t opcode, uint8_t groupid, uint8_t > version) > > /* due to the conversion, this group is special */ > #define IWM_ALWAYS_LONG_GROUP 1 > +#define IWM_SYSTEM_GROUP 4 > > struct iwm_cmd_header { > uint8_t code; >