[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-tech
Subject:    Re: iwm: avoid 'mac clock not ready' panic
From:       Mark Kettenis <mark.kettenis () xs4all ! nl>
Date:       2021-05-30 20:40:46
Message-ID: 56130909182ccd80 () bloch ! sibelius ! xs4all ! nl
[Download RAW message or body]

> Date: Sun, 30 May 2021 22:26:09 +0200
> From: Stefan Sperling <stsp@stsp.name>
> 
> 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);
>  }
> 
> 

[prev in list] [next in list] [prev in thread] [next in thread]