El mar, 19 jul 2022 a las 15:08, Stefan Sperling (<s...@stsp.name>) escribió:
> On Tue, Jul 19, 2022 at 05:01:48PM +0200, Patrick Wildt wrote: > > No, it's not inverted. If bwfm_pci_send_mb_data() fails we want to > > re-init the device, that's what both Linux and OpenBSD are doing. > > Right. I came to the same conclusion while taking a brief look. > > > Better questions are: why is bwfm_pci_send_mb_data() failing? Why is > > there a pending mb command? > > Well, it looks like bwfm_pci_send_mb_data() does in fact succeed. > The incorrect patch effectively treats success as failure, and thus > demonstrates that our current code does not detect an error there. > > The failure is elsewhere: bwfm_fwvar_set_int() in bwfm_init() is failing. > Probably because the device isn't resumed yet even though it should be. > > > And: why doesn't cleanup and reactivate work? > > As I understand the bug report, by treating bwfm_pci_send_mb_data() success > as a failure case, we end up running bwfm_cleanup() and bwfm_pci_cleanup(), > and now the device resumes just fine. > Yes, exactly. When the A/C charger is connected bwfm_pci_send_mb_data() succeeds. As you say, treated as a failure, bwfm_cleanup() and bwfm_pci_cleanup() are called and the device resumes correctly. When the laptop is running from battery bwfm_pci_intmask() succeeds and bwfm_cleanup() and bwfm_pci_cleanup() are called. Furthermore, when running on battery it's good that only bwfm_pci_intmask() is evaluated. If bwfm_pci_send_mb_data() is called while running on battery, when resuming, the laptop freezes completely and becomes unresponsive. I actually forgot to check if that's because we don't get to the point where the keyboard or other devices are reattached and the system just comes to a halt. > So the Linux code we've ported does somehow manage to resume the device > from a weird state. But we fail to do that. I don't know why, but is that > not the real question we need to ask? I tried printing my way through a great deal of variables in several files called when resuming and wakeup and I couldn't find any value (other than bwfm_pci_intmask()) that was different when resuming either with the A/C charger connected or while running solely on battery, so sadly I don't have an answer to this yet. > Could we always do a full device reset as a workaround? Perhaps that is what we should be doing anyway? Maybe the Linux way of > resuming this device is somehow incompatible with the way our system works?