On Wed, May 26, 2010 at 2:34 PM, Justin P. Mattock <justinmattock@xxxxxxxxx> wrote: > On 05/26/2010 02:05 PM, Luis R. Rodriguez wrote: >> >> On Wed, May 26, 2010 at 10:38 AM, Justin P. Mattock >> <justinmattock@xxxxxxxxx> wrote: >> >>> >>> On 05/26/2010 10:18 AM, John W. Linville wrote: >>> >>>> >>>> On Wed, May 26, 2010 at 10:14:16AM -0700, Justin P. Mattock wrote: >>>> >>>> >>>>> >>>>> Disable the leds on ath9k for Apple products, since >>>>> there is no leds on any of there machines(or non that I can find!!). >>>>> >>>>> Signed-off-by: Justin P. Mattock<justinmattock@xxxxxxxxx> >>>>> >> >> Our team has confirmed Apple devices do not have LEDs so a change like >> this is OK but this patch is wrong anyway. More details below. >> >> >>>>> >>>>> void ath_init_leds(struct ath_softc *sc) >>>>> { >>>>> char *trigger; >>>>> int ret; >>>>> >>>>> + /* Apple has no leds lights for their wireless. */ >>>>> + if (dmi_check_system(dmi_system_table)> 0) >>>>> + return -ENODEV; >>>>> + else >>>>> + >>>>> if (AR_SREV_9287(sc->sc_ah)) >>>>> sc->sc_ah->led_pin = ATH_LED_PIN_9287; >>>>> else >>>>> >>>>> >>>> >>>> Surely we don't need the 'else'. >>>> >>>> Does enabling the LEDs on these systems cause any problems? >>>> >>>> John >>>> >>>> >>> >>> I picked up the idea, from this patch: >>> http://kerneltrap.org/mailarchive/linux-kernel/2010/1/20/4530535 >>> while looking into a bug for ath9k(thinking maybe leds >>> are causing something, which want the case) >>> >>> so Id have to say I don't think the leds cause issue's, >>> if anything just a wasted symlink, call, or whatever >>> in /sys/class/leds/* >>> >> >> The patch is wrong anyway, ath_init_leds() is void and yet you return >> a value. If this is really needed I'd also prefer if you define a bool >> or something and use a ah->ignore_leds and set this to true if the DMI >> stuff checks out for Apple up hw init. Then check for ignore_leds >> prior to calling ath_init_leds() and also avoid it if set. Also check >> for ingore_leds upon device cleanup and ignore the >> acancel_delayed_work_sync(&sc->ath_led_blink_work) and >> ath_deinit_leds() and skip that. >> >> But given that it is not needed it seems a lot of work for something >> that is a non-issue. >> >> Luis >> >> > > tough to say!! I was looking into a bug a few months back, and > thought hmm.. maybe something is being called with the leds > but the leds are not there causing some thing to crap out.. > > so out of curiosity I remembered the dmi disable patch, and > semi somewhat pieced it together(just to see the results of disabling the > leds). > unfortunately still hit the strange thing with wpa_supplicant and ath9k(the > disconnect thing). > > as for the above procedure(I can give that a try.. but it might > take a while...) OK thanks for the details, best just ignore it unless you can prove it fixes an issue. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html