> Date: Sun, 30 May 2021 22:26:09 +0200
> From: Stefan Sperling <[email protected]>
> 
> Steven observed a panic ("iwm0: mac clock not ready") while testing
> the iwm firmware update patch on a 9560 device.
> I've also seen this happen one time, at some point during development.
> 
> In hindsight it is a bad idea to look at hardware register state here.
> The point of iwm_nic_assert_locked() is to verify that iwm_nic_lock() has
> been called somewhere up in the call stack. Checking our own lock counter
> is sufficient for this purpose.
> 
> If locking the device worked then these registers had the expected state at
> that time and our lock counter was incremented. Apparently if the device runs
> into some issue later the state of these registers may change and trigger
> these panics. Instead we want to handle such failures gracefully and reset
> the device.
> 
> ok?

ok kettenis@

> (For the curious: iwm_nic_lock() is supposed to avoid a situation where the
> device enters some low power state while the driver expects the device to
> quickly respond to certain I/O requests. That's all I know.)
> 
> diff 385a08f3e862586df8f1803dfa09fc765a5c3610 /usr/src
> blob - 4b502468fea796f103fb84237146879e8c4df267
> file + sys/dev/pci/if_iwm.c
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -1069,11 +1069,6 @@ iwm_nic_lock(struct iwm_softc *sc)
>  void
>  iwm_nic_assert_locked(struct iwm_softc *sc)
>  {
> -     uint32_t reg = IWM_READ(sc, IWM_CSR_GP_CNTRL);
> -     if ((reg & IWM_CSR_GP_CNTRL_REG_FLAG_MAC_CLOCK_READY) == 0)
> -             panic("%s: mac clock not ready", DEVNAME(sc));
> -     if (reg & IWM_CSR_GP_CNTRL_REG_FLAG_GOING_TO_SLEEP)
> -             panic("%s: mac gone to sleep", DEVNAME(sc));
>       if (sc->sc_nic_locks <= 0)
>               panic("%s: nic locks counter %d", DEVNAME(sc), 
> sc->sc_nic_locks);
>  }
> 
> 

Reply via email to