re: usbhist support for urtwn

2019-11-25 Thread matthew green
Andreas Gustafsson writes:
> matthew green wrote:
> > [misread everything]

ah, please go ahead.


.mrg.


re: usbhist support for urtwn

2019-11-25 Thread Andreas Gustafsson
matthew green wrote:
> this looks fine, but if you feel up to it, you can re-combine
> the single DPRINTF() -> two calls using URTWNHIST_CALLARGS().
> if_aue.c is a recent file converted to this method.  it has
> the advantage of consuming fewer kernhist slots -- 1 line vs 2.

There is a separate flag bit DBG_FN in urtwn_debug that enables
logging of function calls, and I have already translated the sequence
URTWNHIST_CALLED(); DPRINTFN(DBG_FN, ...) into URTWNHIST_CALLARGS(...).
I have deliberately avoided folding DPRINTFN() calls enabled by other
flag bits into URTWNHIST_CALLARGS() so that they can still be enabled
and disabled separately from the logging of function calls.

> nit:  the new DPRINTFN(), URTWNHIST_CALLED(), and
> URTWNHIST_CALLARGS() macros should use the do { .. } while(0)
> idiom the old DPRINTFN() did so they remain/are safe for us in
> all contexts.

They already do, they're just formatted differently.
-- 
Andreas Gustafsson, g...@gson.org


re: usbhist support for urtwn

2019-11-25 Thread matthew green
Andreas Gustafsson writes:
> Hi all,
> 
> I have this patch to replace the debug printfs in sys/dev/usb/if_urtwn.c
> by usbhist calls, roughly modelled after the use of usbhist in if_axe.c:
> 
>   https://www.gson.org/netbsd/patches/urtwn-usbhist.patch
> 
> OK to commit?

this looks fine, but if you feel up to it, you can re-combine
the single DPRINTF() -> two calls using URTWNHIST_CALLARGS().
if_aue.c is a recent file converted to this method.  it has
the advantage of consuming fewer kernhist slots -- 1 line vs 2.

nit:  the new DPRINTFN(), URTWNHIST_CALLED(), and
URTWNHIST_CALLARGS() macros should use the do { .. } while(0)
idiom the old DPRINTFN() did so they remain/are safe for us in
all contexts.

thanks!


.mrg.


Re: usbhist support for urtwn

2019-11-25 Thread Christos Zoulas
In article <24027.34627.564698.872...@guava.gson.org>,
Andreas Gustafsson   wrote:
>Hi all,
>
>I have this patch to replace the debug printfs in sys/dev/usb/if_urtwn.c
>by usbhist calls, roughly modelled after the use of usbhist in if_axe.c:
>
>  https://www.gson.org/netbsd/patches/urtwn-usbhist.patch
>
>OK to commit?

I'd probably 0x%jx -> %#jx but otherwise LGTM.

christos