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
--
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
