Re: iwm(4): Also reset sc_uc.uc_intr flag on 8000 hw. Gets rid of 1s tsleep.

2016-11-01 Thread Stefan Sperling
On Tue, Oct 11, 2016 at 04:50:09PM +0200, Stefan Sperling wrote:
> On Tue, Oct 11, 2016 at 04:29:24PM +0200, Imre Vadasz wrote:
> > Hi,
> > 
> > The sc->sc_uc.uc_intr flag isn't getting reset to 0 in
> > iwm_load_firmware_8000(), so the
> > /* wait for the firmware to load */
> > for (w = 0; !sc->sc_uc.uc_intr && w < 10; w++) {
> > err = tsleep(>sc_uc, 0, "iwmuc", hz/10);
> > }
> > loop is immediately aborting on AC8260 hardware without properly waiting
> > for the IWM_ALIVE notification (except on the very first time a firmware
> > was loaded). This issue was workarounded by the fixed 1s tsleep afterwards.
> > /*
> >  * Give the firmware some time to initialize.
> >  * Accessing it too early causes errors.
> >  */
> > tsleep(, PCATCH, "iwmfwinit", hz);
> > 
> > 
> > Everything works fine in iwm(4) without that 1s tsleep if I also reset
> > sc->sc_uc.uc_intr in the family 8000 case.
> 
> Thank you! OK stsp@ (I can't test + commit this myself right now.)

I have committed this. Thanks again.
 
> > 
> > Index: sys/dev/pci/if_iwm.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> > retrieving revision 1.144
> > diff -u -r1.144 if_iwm.c
> > --- sys/dev/pci/if_iwm.c8 Oct 2016 14:37:48 -   1.144
> > +++ sys/dev/pci/if_iwm.c11 Oct 2016 14:08:05 -
> > @@ -2820,8 +2820,6 @@
> > uint32_t dlen;
> > uint32_t offset;
> >  
> > -   sc->sc_uc.uc_intr = 0;
> > -
> > fws = >sc_fw.fw_sects[ucode_type];
> > for (i = 0; i < fws->fw_count; i++) {
> > data = fws->fw_sect[i].fws_data;
> > @@ -2924,7 +2922,6 @@
> > int first_ucode_section;
> >  
> > fws = >sc_fw.fw_sects[ucode_type];
> > -
> > /* configure the ucode to be ready to get the secured image */
> > /* release CPU reset */
> > iwm_write_prph(sc, IWM_RELEASE_CPU_RESET, IWM_RELEASE_CPU_RESET_BIT);
> > @@ -2943,6 +2940,8 @@
> >  {
> > int err, w;
> >  
> > +   sc->sc_uc.uc_intr = 0;
> > +
> > if (sc->sc_device_family == IWM_DEVICE_FAMILY_8000)
> > err = iwm_load_firmware_8000(sc, ucode_type);
> > else
> > @@ -2957,12 +2956,6 @@
> > }
> > if (err || !sc->sc_uc.uc_ok)
> > printf("%s: could not load firmware\n", DEVNAME(sc));
> > -
> > -   /*
> > -* Give the firmware some time to initialize.
> > -* Accessing it too early causes errors.
> > -*/
> > -   tsleep(, PCATCH, "iwmfwinit", hz);
> >  
> > return err;
> >  }
> > 
> 



Re: iwm(4): Also reset sc_uc.uc_intr flag on 8000 hw. Gets rid of 1s tsleep.

2016-10-11 Thread Stefan Sperling
On Tue, Oct 11, 2016 at 04:29:24PM +0200, Imre Vadasz wrote:
> Hi,
> 
> The sc->sc_uc.uc_intr flag isn't getting reset to 0 in
> iwm_load_firmware_8000(), so the
>   /* wait for the firmware to load */
>   for (w = 0; !sc->sc_uc.uc_intr && w < 10; w++) {
>   err = tsleep(>sc_uc, 0, "iwmuc", hz/10);
>   }
> loop is immediately aborting on AC8260 hardware without properly waiting
> for the IWM_ALIVE notification (except on the very first time a firmware
> was loaded). This issue was workarounded by the fixed 1s tsleep afterwards.
>   /*
>* Give the firmware some time to initialize.
>* Accessing it too early causes errors.
>*/
>   tsleep(, PCATCH, "iwmfwinit", hz);
> 
> 
> Everything works fine in iwm(4) without that 1s tsleep if I also reset
> sc->sc_uc.uc_intr in the family 8000 case.

Thank you! OK stsp@ (I can't test + commit this myself right now.)

> 
> Index: sys/dev/pci/if_iwm.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> retrieving revision 1.144
> diff -u -r1.144 if_iwm.c
> --- sys/dev/pci/if_iwm.c  8 Oct 2016 14:37:48 -   1.144
> +++ sys/dev/pci/if_iwm.c  11 Oct 2016 14:08:05 -
> @@ -2820,8 +2820,6 @@
>   uint32_t dlen;
>   uint32_t offset;
>  
> - sc->sc_uc.uc_intr = 0;
> -
>   fws = >sc_fw.fw_sects[ucode_type];
>   for (i = 0; i < fws->fw_count; i++) {
>   data = fws->fw_sect[i].fws_data;
> @@ -2924,7 +2922,6 @@
>   int first_ucode_section;
>  
>   fws = >sc_fw.fw_sects[ucode_type];
> -
>   /* configure the ucode to be ready to get the secured image */
>   /* release CPU reset */
>   iwm_write_prph(sc, IWM_RELEASE_CPU_RESET, IWM_RELEASE_CPU_RESET_BIT);
> @@ -2943,6 +2940,8 @@
>  {
>   int err, w;
>  
> + sc->sc_uc.uc_intr = 0;
> +
>   if (sc->sc_device_family == IWM_DEVICE_FAMILY_8000)
>   err = iwm_load_firmware_8000(sc, ucode_type);
>   else
> @@ -2957,12 +2956,6 @@
>   }
>   if (err || !sc->sc_uc.uc_ok)
>   printf("%s: could not load firmware\n", DEVNAME(sc));
> -
> - /*
> -  * Give the firmware some time to initialize.
> -  * Accessing it too early causes errors.
> -  */
> - tsleep(, PCATCH, "iwmfwinit", hz);
>  
>   return err;
>  }
> 



iwm(4): Also reset sc_uc.uc_intr flag on 8000 hw. Gets rid of 1s tsleep.

2016-10-11 Thread Imre Vadasz
Hi,

The sc->sc_uc.uc_intr flag isn't getting reset to 0 in
iwm_load_firmware_8000(), so the
/* wait for the firmware to load */
for (w = 0; !sc->sc_uc.uc_intr && w < 10; w++) {
err = tsleep(>sc_uc, 0, "iwmuc", hz/10);
}
loop is immediately aborting on AC8260 hardware without properly waiting
for the IWM_ALIVE notification (except on the very first time a firmware
was loaded). This issue was workarounded by the fixed 1s tsleep afterwards.
/*
 * Give the firmware some time to initialize.
 * Accessing it too early causes errors.
 */
tsleep(, PCATCH, "iwmfwinit", hz);


Everything works fine in iwm(4) without that 1s tsleep if I also reset
sc->sc_uc.uc_intr in the family 8000 case.

Index: sys/dev/pci/if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.144
diff -u -r1.144 if_iwm.c
--- sys/dev/pci/if_iwm.c8 Oct 2016 14:37:48 -   1.144
+++ sys/dev/pci/if_iwm.c11 Oct 2016 14:08:05 -
@@ -2820,8 +2820,6 @@
uint32_t dlen;
uint32_t offset;
 
-   sc->sc_uc.uc_intr = 0;
-
fws = >sc_fw.fw_sects[ucode_type];
for (i = 0; i < fws->fw_count; i++) {
data = fws->fw_sect[i].fws_data;
@@ -2924,7 +2922,6 @@
int first_ucode_section;
 
fws = >sc_fw.fw_sects[ucode_type];
-
/* configure the ucode to be ready to get the secured image */
/* release CPU reset */
iwm_write_prph(sc, IWM_RELEASE_CPU_RESET, IWM_RELEASE_CPU_RESET_BIT);
@@ -2943,6 +2940,8 @@
 {
int err, w;
 
+   sc->sc_uc.uc_intr = 0;
+
if (sc->sc_device_family == IWM_DEVICE_FAMILY_8000)
err = iwm_load_firmware_8000(sc, ucode_type);
else
@@ -2957,12 +2956,6 @@
}
if (err || !sc->sc_uc.uc_ok)
printf("%s: could not load firmware\n", DEVNAME(sc));
-
-   /*
-* Give the firmware some time to initialize.
-* Accessing it too early causes errors.
-*/
-   tsleep(, PCATCH, "iwmfwinit", hz);
 
return err;
 }