Re: Adding rfkill support to thinkpad_acpi

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

 



On Tue, 22 May 2007, Dmitry Torokhov wrote:
> > 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.

Sure, and as I told you I am okay with that.  But I want the hint that a
poll would be welcome *right now* by the rfkill class.

Whether I will do the poll to get a fresh value or return a cached one is my
problem as the driver, and not rfkill's... unless you need the interface to
always have a fresh value *and document it*.

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

And I replied that I know of that, and that it is hardly a problem that is
present only in thinkpad-acpi, most of the sysfs interface is buggy that way

I will fix it, it is already in the planning stages and I even have some
alpha-release code to do it for tempertatures and fans around.

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

Fortunately, there really are no reports of rogue firmwares messing with the
radios, even on thinkpads that are especially bad about this.  And I told
you the sideway control paths in thinkpads were the expected use case, and
not bugs (whether it is good system design or not is something else).

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

fake as in "user didn't press the radio hotkey".  Sorry, I misused the term.
We are in sync about this, you want the event, and I understand I should
provide it and why.

> I do not want to merge button processing (input layer task) with
> rfkill. Keep them separaed. They are logically separated. There are

Nor do I!

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

We agree on that.

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

No :(   But I don't want to depend on rfkill-input to get basic radio state
right either.  This goes well with the logical separation between rfkill and
rfkill-input.  And AFAIK, this is not even a point of contention between us.

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

I very much doubt it for the private attributes.  Not that I would have
anything against it, btw.  I am actively working on trying to get rid of as
many of them as I can.  I am very much for generic interfaces.

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

And there it is.  I want the hint to know when that poll is needed.  I want
to, in practice, be able to move the radio status knowledge from the sysfs
class to the driver when the driver is prepared to do it.

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

Which will happen in thinkpad-acpi when I split it up in various modules,
yes.

So, can you give me the interface to keep them separate (i.e. a way to set
the radio state inside the class)?  I understand you don't have anything
against this, but I'd need to know what the final patch to rfkill would be
like, as I will have to backport it to 2.6.20.

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

No, but it is as special as a notebook gets, with more than one million
machines out there.  If your class can't talk to thinkpads, it is not
generic enough.

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

Err, sorry, I probably lost track of the ammount of patches re. this issue I
have been through, so I better ask: which one exactly?

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

I already remove from sysfs and the rest of the system any non-installed or
unsupported hardware (as long as thinkpad-acpi is the driver for it).  If we
find out there is a way to know the BIOS is force-disabling the radios,
thinkpad-acpi will act as if the radios were not installed at all.  If there
is no way to know the BIOS is hardware-killing the device, the device will
be there.

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

(1) has no time window where userspace is misled (subject to rate-limiting
if needed, since the issue was brought up), and will never have anything to
do with rfkill-input.

(2) doesn't help me optimize polling events by itself, and may have to
involve rfkill-input.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

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