Re: acpi_ibm(4): new radio kill switch (readonly) sysctl

Nate Lawson <[email protected]>
Newsgroups gmane.os.freebsd.devel.mobile
Message-ID <[email protected]>
Rong-en Fan wrote:
> On 4/18/07, Nate Lawson <[email protected]> wrote:
>> Rong-en Fan wrote:
>> > As pointed out by Henrik Brix Andersen, I adds a sysctl entry
>> > that shows the status of radio kill switch found on some ThinkPad:
>> >
>> > http://people.freebsd.org/~rafan/acpi_ibm_killswitch.diff
>> >
>> > dev.acpi_ibm.0.killswitch = 0 means the switch is off. It seems that
>> > no acpi event will be generated when the value changes (actually,
>> > my x60 does not generate any events when I presses FN+something).
>> > Otherwise, we can hook it in devd.conf and remove wireless driver when
>> > kill switch is on...
>> >
>> > Any comments?
>>
>> Seems fine to me.  But as to the name of the sysctl -- it should be more
>> logical.  How about renaming it to dev.acpi_ibm.0.radio_enable and when
>> 1, the radio is enabled?  Even if you have to invert the logic of the
>> ACPI method, it would make more sense to users.  They don't need to know
>> what's going on under the hood.
> 
> Good idea. I updated the patch:
> 
> http://people.freebsd.org/~rafan/acpi_ibm_radio_switch.diff
> 
> If you have ThinkPad other than X60, please help test this.

This code seems suspect:

+	case ACPI_IBM_METHOD_RADIO_SWITCH:
+		acpi_GetInteger(sc->handle, IBM_NAME_RADIO_SWITCH_GET, &val);
+		sc->radio_switch_state = val;
+		val = (val != 0);
+		break;

The switch state is set to the return value of the AML method.  Then if
it is 0, val is set to 0 and if it is 1, val is set to 1.  Don't you
mean to invert val?  If so, this should be sufficient:

	/* Invert the radio kill switch for the user. */
	sc->radio_switch_state = !val;

-- 
Nate
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-mobile
To unsubscribe, send any mail to "[email protected]"
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.