On Wed, Oct 18, 2006 at 04:37:08PM +0200, Michael Buesch wrote:

> > @@ -257,7 +263,11 @@ void bcm43xx_leds_update(struct bcm43xx_
> >                     continue;
> >  #endif /* CONFIG_BCM43XX_DEBUG */
> >             default:
> > -                   assert(0);
> > +                   if (bcm43xx_max_led_err) {
> > +                           printkl(KERN_INFO PFX "Bad value in 
> > leds_update,"
> > +                                   " led->behaviour: 0x%x\n", 
> > led->behaviour);
> > +                           --bcm43xx_max_led_err;
> > +                   }
> 
> I'd call this message bloat. ;) This is the first time the assertion
> triggers since it was added.
> You could instead remove the assert(), remove bcm43xx_max_led_err
> and use dprintkl instead of printkl.
> 
> >             };
> >  
> >             if (led->activelow)
> > Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_leds.h
> > ===================================================================
> > --- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_leds.h
> > +++ wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_leds.h
> > @@ -24,6 +24,10 @@ struct bcm43xx_led {
> >  
> >  #define BCM43xx_LED_BEHAVIOUR              0x7F
> >  #define BCM43xx_LED_ACTIVELOW              0x80
> > +#define BCM43xx_LED_BCM4303_0              0x2B
> > +#define BCM43xx_LED_BCM4303_1              0x78
> > +#define BCM43xx_LED_BCM4303_2              0x2E
> > +#define BCM43xx_LED_BCM4303_3              0x19
> 
> Add these to the enum below, instead, please.
> The defines above are masks and not the behaviour values.
> 
> >  enum { /* LED behaviour values */
> >     BCM43xx_LED_OFF,
> >     BCM43xx_LED_ON,

I took Michael's suggestions before merging your patch...FYI! :-)

John
-- 
John W. Linville
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to