Re: Adding rfkill support to thinkpad_acpi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/22/07, Henrique de Moraes Holschuh <hmh@xxxxxxxxxx> wrote:
> On Mon, 21 May 2007, Dmitry Torokhov wrote:
> > On Monday 21 May 2007 13:22, Henrique de Moraes Holschuh wrote:
> > > On Mon, 21 May 2007, Dmitry Torokhov wrote:
> > > > RFkill is not only about "please disable radios", it _does_ provide
> > > > the standard sysfs interface to control the radios as well.
> > >
> > > To control?  Fine, if it was really controlling them.  The problem is that
> > > thinkpad-acpi needs it to *interface* with the firmware that really controls
> > > the radios: it is not like thinkpad-acpi or the kernel is in *control* of
> > > the radios.
> >
> > Ok, it interfaces the firmware that controls the radio in case of
> > thinkpad_acpi. Is that precise enough?
>
> Yep. But I was not being picky, there is a big difference: when you are in
> control of something, you don't have to take kindly to its status changing
> behind you (rfkill is perfect for that as it stands).  When you are
> interfacing to something, you have to ask it the status.  THAT is my whole
> point, and rfkill is not 100% complete for the later yet.
>
> > > My issue with rfkill is that if the user queries the status of the rfkill
> > > switch (for what thinkpad-acpi would need it for), he should get the status
> > > of the rfkill switch in hardware, and not of whatever the kernel thinks it
> > > is.
> > >
> > > > You know I got curious and looked at thinkpad_acpi driver and adding
> > > > rfkill support for bluetooth to it turned out to be pretty easy. What
> > >
> > > It is easy, indeed... until the user slides the hardware rfkill switch to
> > > "kill it".  Now the kernel thinks the thing is on, and it has died.  Etc.
> > > And *nothing* you do can get the radio back on, unless the user slides the
> > > hardware switch back to the on position, at which point it is anyone's guess
> > > if the firmware in a given thinkpad will turn it ON, or leave it off if it
> > > was off in software...
> >
> > And once you implement a polling input device for your slider that should
> > work too. I know you don't like polling. But what you seem to be missing
> > is that you still need to notify userspace that your radio state is changed
> > so you need to poll anyway.
>
> Well, we are repeating all the talks we had.  So let me describe my position
> in more exact terms, to clarify it.
>
> 1. I have nothing against polling when it is the only way to do something,
> as long as it can be kept at a reasonable rate to not waste too many
> resources (ACPI access is *expensive*).  But I really don't take kindly to
> doing it with any higher a frequency than what is strictly needed, and that
> means I want any reasonable methods we could employ to reduce that frequency
> to be available.

You as the driver writer can decide how often your driver should poll
hardware it controls. If your hardware does not require polling to
determine status changes do not poll.

> 2. I don't take kindly to incomplete solutions when they could be complete
> without any big compromises or costs.  This applies to thinkpad-acpi as well
> as anything else.  So I won't be accepting rfkill patches for thinkpad-acpi
> until I am convinced there is no better way to do things that is at the same
> time reasonable in terms of effort for rfkill and others.

I told in my other mail why checking status every time someone touches
sysfs attribute might not be a good idea - an application could spin
reading it constantly and consuming your precious resources.

> 3. rfkill as it stands is very fine if you control the hardware.  It is not
> if there are sidepaths to the hardware you do not control: the current
> design asks for the underlying driver to poll constantly and kick the
> hardware back to the state rfkill wants (this is what you guys told me to
> do, and I argued that was not the thinkpad user case),

No what we told is that if there exists a [rogue] firmare that decides
to switch radio on and off on its own then the driver has to poll
hardware state and restore to original setting based on current system
policy. Apparently there is no known firmwares that do that.

> OR to send a fake
> rfkill key pressed event to the input layer side of rfkill to kick the
> entire rfkill structure to the state it should be (which is what thinkpads
> require).  Too much of a round trip, and too fragile IMO.

It is not fake. There is really an event signalling that user switched
off the radio. And the rest of the system shoudl be notified and act
accordingly. Ther could be other cards that need to be turned off,
some other actions to be carried out by userspace.

I do not want to merge button processing (input layer task) with
rfkill. Keep them separaed. They are logically separated. There are
keyboards with keys that generated KEY_WLAN events that do not control
hardware state directly. You may even want to remap "Pause" key on
your AT keyboard to send KEY_WLAN and it shoudl still be able to
control your cards that allow programmable switching.

And I tink routing through rfkill-input in absence of userspace
interest in controlling a switch is OK too. Can you measure what kind
of overhead it causes?

And hopefully one day we could remove procsfs and thinkpad private
sysfs attributes completely.

>
> 4. reading the rfkill status is mostly useless if you are not controlling
> the hardware.  It can easily be wrong for long time windows until the
> underlying driver notices the discrepancy and does something about it (see
> (3) above).  This is *easy* to fix, and you have the patch.  Give us a hook
> that tells the underlying driver that rfkill is going to need the status
> now, and the driver can decide what to do that is best for its particular
> device.

I do not see what is there to fix. You are going to poll your switch.

> 5. the round trip through the input side of rfkill ain't nice.  Can we have
> a complete interface in the lower rfkill layer that lets us set the radio
> state of rfkill, please (I understand you are ok with this from previous
> posts)?

No, keep them logically separate (the driver may implement both
functions, rfkill and input device) but we need to support cases when
we only have one piece in one driver and another one in second driver.

> With these changes, we could plug thinkpad-acpi to rfkill in a way that
> makes sense for how it is supposed to work *in ThinkPads* (I am not talking
> about any other hardware or driver here):

And we need a solution that works well for everyone. Thinkpad is not
that special.

> 1. In thinkpads, the **user** has sidepaths to control the radio. We must
> honour them.  This means that *thinkpads* would update rfkill status of the
> radio instead of updating the radio to whatever state rfkill thought it was
> in.  I am not claiming this is the right way to do it for any other platform
> or radio hardware.
>

I believe my patch addresses that.

> 2. *Some* thinkpads allow one to know if the hardware rfkill slider switch
> changed state (known method so far requires constant polling).  We would add
> this support, and interface to rfkill-input to let it know the user changed
> the switch.
>

OK, good.

> 3. Many thinkpads have hardware radio-kill overrides that force the radios
> to be always off, typically in the BIOS (I guess it toggles some GPIO pin,
> because it activates the hardware radio-kill pin of IPW2200 chips).  We
> would be able to handle these as well: rfkill would be told in no certain
> terms that the radio is off, and would not think it is on even for a second.
>

No, I have a better solution - do not create anyhting for hardware
disabled in BIOS. There is no point.

> 1. If the **user** causes the firmware to disable the radios, we notice it
> and don't report to anything that the radio is still enabled.
>
> 2. I am ok with adding polling to notify userspace and the rfkill input
> layer that we detected that the radios changed state.
>

What is the dference between 1 and 2?

> 3. It is very very probable that we can monitor most thinkpad's hardware
> rfkill switches, even if it requires polling. thinkpad-acpi would generate
> rfkill switch events for these to userspace and the rfkill input layer.
>

OK.

-- 
Dmitry

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux