Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Apr 2007 13:27:27 +0800
From:      "Rong-en Fan" <grafan@gmail.com>
To:        "Nate Lawson" <nate@root.org>
Cc:        acpi@freebsd.org, mobile@freebsd.org
Subject:   Re: acpi_ibm(4): new radio kill switch (readonly) sysctl
Message-ID:  <6eb82e0704172227y6c62846l24c9aafab299cb9@mail.gmail.com>
In-Reply-To: <4625A70C.1000506@root.org>
References:  <6eb82e0704171645n5f7b2ca6h41b41016cdafad24@mail.gmail.com> <4625601C.9000201@root.org> <6eb82e0704172055l5bddca81t5b7e9e45a297a839@mail.gmail.com> <4625A70C.1000506@root.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 4/18/07, Nate Lawson <nate@root.org> wrote:
> Rong-en Fan wrote:
> > On 4/18/07, Nate Lawson <nate@root.org> 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;

Sorry, I update the patch. It should be correct now.
Should have morning coffee first :-)

Regards,
Rong-En Fan

>
> --
> Nate
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6eb82e0704172227y6c62846l24c9aafab299cb9>