Re: rfkill guidance

Re: rfkill guidance

[Jonathan Richardson]

+linux-wireless

On 16-09-27 12:25 AM, Johannes Berg wrote:
> Hi,
> 
> You should probably Cc the linux-wireless list, at least :)
> 
> But anyway.
> 
>> The driver enables/disables one or more regulators controlling a
>> bluetooth radio using the rfkill_ops set_block function. It's
>> different in that there is no bluetooth driver in the kernel - it's
>> controlled with some firmware that gets uploaded to a chip. The only
>> kernel usage is to power on/off the regulator(s).
> 
> Interesting, don't you have to talk to the BT somehow? So the BT can't
> ever be used from the kernel?

Not from the kernel, a user space app interacts with the BT chip and
kernel. More below.

> 
> Is rfkill really even the right concept here then? I mean, from the
> kernel's POV it doesn't even really have to know there's BT behind
> that, if there's no relation to BT in the kernel? Then again, the
> *user* might need to know?

> Would you be able to explain the overall system function of this a bit
> more?

Sure, I'll tell you what I can. A user space app handles interaction
between the kernel and BT radio. When the app is started, it enables the
regulator (via the rfkill driver and userspace sysfs interface) to power
up the BT chip. The app sends the firmware via a uart to the chip. The
chip loads the firmware then goes into a lower power mode. When the app
needs to talk to the chip it toggles a gpio (device wake) before sending
messages. If the kernel is in a low power mode and the BT chip receives
a message that requires handling by the app, then a host wake gpio is
driven to wake up the host (kernel).

In addition to the regulator(s) controlled by the rfkill driver I didn't
mention the two gpio's previously. The existing rfkill driver does
nothing with them. It just exports the global gpio # to sysfs which the
app can read to control via the gpio sysfs interface. These could be
hard coded in a startup script or something, they aren't that important
but it does handle the conversion to the global gpio# from 1 of 3 gpio
controllers on a Cygnus SOC. I'm not sure how to handle that, since the
gpio's vary across board variants.

I'm not sure rfkill is the right concept either but the app does use the
rfkill sysfs interface to control the regulators which in turn powers a
BT radio. The driver could not use the rfkill interface and provide its
own but would essentially be doing the same thing as rfkill-regulator.c.

> 
>> The driver is basically a wrapper around controlling regulators.
>> There's already rfkill-regulator.c but it has no DT support and only
>> controls 1 regulator. I was considering modifying it until I saw this
>> thread where Mark objected and it looks like the submitter abandoned
>> the patchset (http://www.serverphorums.com/read.php?12,870025).
> 
> I don't think that's a "killer" thread for this feature really. Yes,
> people have (quite possibly legitimate) concerns about that, but we
> should discuss that.
> 
>> I couldn't see any way to enable/disable regulators from sysfs so it
>> looks like the regulator kernel API needs to be used from some
>> driver.
> 
> Indeed, looks like Documentation/ABI/testing/sysfs-class-regulator is
> all read-only.

There's a regulator driver named userspace-consumer.c that allows
enable/disable from userspace, but it doesn't support DT either.

> 
>> Do you think adding DT support and multiple regulators to the
>> existing driver is the best way to add support for this? Our driver
>> basically duplicates this and I don't think can be upstreamed the way
>> it is.
> 
> Hard to say, really. Like I said, I'm not really _quite_ sure rfkill is
> even the right concept, since you're essentially toggling some
> hardware, but then again it *does* end up being a BT device.

Looking forward to more feedback. I see the options as either rfkill or
some regulator controller that allows enabling/disabling via sysfs.

> 
> It also looks like nobody is actually using rfkill-regulator anywhere
> (in the upstream kernel), so we could just port that entirely to DT,
> which would be nice.
> 
> johannes
> 

Thanks,
Jon

Re: rfkill guidance

[Johannes Berg]

Hi,

Mark, Marek, DT folks - the root of the discussion here is the old
rfkill-regulator device-tree support. I'm discussing some details of
the system here first, but below we get to the DT discussion.

The old thread is here:
http://thread.gmane.org/gmane.linux.kernel/1643236

> Sure, I'll tell you what I can. A user space app handles interaction
> between the kernel and BT radio. When the app is started, it enables
> the regulator (via the rfkill driver and userspace sysfs interface) 

You should probably consider /dev/rfkill over the sysfs interface :)

> to power up the BT chip. The app sends the firmware via a uart to the
> chip. The chip loads the firmware then goes into a lower power mode.
> When the app needs to talk to the chip it toggles a gpio (device
> wake) before sending messages. If the kernel is in a low power mode
> and the BT chip receives a message that requires handling by the app,
> then a host wake gpio is driven to wake up the host (kernel).

So the reason then for having it all done in userspace is that there's
a UART to communicate with it? Or is the reason that you just don't
want the kernel abstraction? There *are* UART BT drivers in the kernel,
as far as I can tell, what's the difference? Or is it that there's just
a completely different protocol, vs. the HCI protocol used normally?

At this point I'm just curious though, doesn't seem all that important
to the discussion :)

Although having a DT binding for a kernel driver for the device that
includes the regulator might change the discussion somewhat?

> In addition to the regulator(s) controlled by the rfkill driver I
> didn't mention the two gpio's previously. The existing rfkill driver
> does nothing with them. It just exports the global gpio # to sysfs
> which the app can read to control via the gpio sysfs interface. These
> could be hard coded in a startup script or something, they aren't
> that important but it does handle the conversion to the global gpio#
> from 1 of 3 gpio controllers on a Cygnus SOC. I'm not sure how to
> handle that, since the gpio's vary across board variants.

Could also just be exposed in the DT, I guess? Userspace can read the
DT, no? Not sure that actually gives you all the dynamic information
though that might be needed if there's any kind of mapping going on.

> > Indeed, looks like Documentation/ABI/testing/sysfs-class-regulator
> > is all read-only.
> 
> There's a regulator driver named userspace-consumer.c that allows
> enable/disable from userspace, but it doesn't support DT either.

Adding DT support there might make some sense, perhaps? Although that's
not really a useful DT binding at all since it won't provide any hints
as to how you'd use that, so ... maybe not so much.

As I read it, the main concern about or objection to the rfkill-
regulator DT binding was that it's not tied to a device, when it seems
that rfkill instances really should be living off a device that they
control. That's true for many, but not all devices.

Today we already have all the platform rfkill instances (like the
various ACPI ones) that don't live off a device that they control, but
instead control the platform's radio functionality. And in some cases,
that actually has very similar behaviour to what was proposed in the
previous patch, and what you're looking for, in that it sometimes kills
power to BT or WiFi chips when soft-disabled (or kicks them off the bus
in some other way). Particularly for the ones that call some random
ACPI function to deal with things, that can happen.

rfkill-regulator, with its platform driver today, more or less mirrors
that. You could even use that with a very simple (few lines of code)
platform module that just binds the regulators to rfkill-regulator, I
think.

Making this more flexible and configurable with DT would make sense to
me, less for the usage of rfkill-for-a-device (which could *also* be
done, but isn't what you need or what was discussed before), but more
for the usage of rfkill-for-the-platform.

Mark and DT folks - can you clarify your objections to the old patch?
Am I reading things correctly, and if so, what do you think about the
platform rfkill?

johannes

Re: rfkill guidance

[Johannes Berg]

> Today we already have all the platform rfkill instances (like the
> various ACPI ones) that don't live off a device that they control,
> but instead control the platform's radio functionality. And in some
> cases, that actually has very similar behaviour to what was proposed
> in the previous patch, and what you're looking for, in that it
> sometimes kills power to BT or WiFi chips when soft-disabled (or
> kicks them off the bus in some other way).

Actually, let me retract that a bit. Obviously they *do* control a
device or radio, but we don't actually know which one. And that can
actually become a problem, since we don't even know how they work etc.

So IOW, even with the ACPI stuff, there's no real "platform rfkill"
concept - it's always killing a specific radio.

There used to be some rfkill that was just reflecting the button state,
but I think we got rid of that in favour of reflecting button state
through hid/input, if it doesn't also control a radio.

So, actually, the more I think about it the more I agree with Mark's
objection. It doesn't really make sense to have a concept of an rfkill
and not know what radio it controls.

In your particular case, Jon, it's complicated by the fact that you
don't want to bind a kernel driver with the rfkill, not sure where
you'd get a device to tie the rfkill to then.

johannes

Re: rfkill guidance

[Marcel Holtmann]

Hi Johannes,

>> Today we already have all the platform rfkill instances (like the
>> various ACPI ones) that don't live off a device that they control,
>> but instead control the platform's radio functionality. And in some
>> cases, that actually has very similar behaviour to what was proposed
>> in the previous patch, and what you're looking for, in that it
>> sometimes kills power to BT or WiFi chips when soft-disabled (or
>> kicks them off the bus in some other way).
> 
> Actually, let me retract that a bit. Obviously they *do* control a
> device or radio, but we don't actually know which one. And that can
> actually become a problem, since we don't even know how they work etc.
> 
> So IOW, even with the ACPI stuff, there's no real "platform rfkill"
> concept - it's always killing a specific radio.
> 
> There used to be some rfkill that was just reflecting the button state,
> but I think we got rid of that in favour of reflecting button state
> through hid/input, if it doesn't also control a radio.
> 
> So, actually, the more I think about it the more I agree with Mark's
> objection. It doesn't really make sense to have a concept of an rfkill
> and not know what radio it controls.
> 
> In your particular case, Jon, it's complicated by the fact that you
> don't want to bind a kernel driver with the rfkill, not sure where
> you'd get a device to tie the rfkill to then.

we are actually taking away the RFKILL switches that are represented by specific GPIOs. We are mapping these into the Bluetooth driver itself. This has been done for Intel and Broadcom UART based Bluetooth chips. The power control is done via the Bluetooth subsystem and that is how it should be. Enumeration of these GPIOs is done from ACPI or eventually DT (which has not made it upstream yet).

In addition, we do not want this crazy Android side of things with custom vendor specific power control via userspace or whatever crazy is done there. It can be all done through the Bluetooth subsystem and still use Android on top of it. There is a concept called HCI User Channel that allows user space application to gain exclusive access to the HCI transport. It has the advantage that the kernel does all the driver integration and handles power control.

Regards

Marcel

Re: rfkill guidance

[Jonathan Richardson]

Thanks for your help. Your comment below clarifies what's missing.

On 16-09-27 11:38 PM, Johannes Berg wrote:
> Hi,
> 
> Mark, Marek, DT folks - the root of the discussion here is the old
> rfkill-regulator device-tree support. I'm discussing some details of
> the system here first, but below we get to the DT discussion.
> 
> The old thread is here:
> http://thread.gmane.org/gmane.linux.kernel/1643236
> 
>> Sure, I'll tell you what I can. A user space app handles interaction
>> between the kernel and BT radio. When the app is started, it enables
>> the regulator (via the rfkill driver and userspace sysfs interface) 
> 
> You should probably consider /dev/rfkill over the sysfs interface :)
> 
>> to power up the BT chip. The app sends the firmware via a uart to the
>> chip. The chip loads the firmware then goes into a lower power mode.
>> When the app needs to talk to the chip it toggles a gpio (device
>> wake) before sending messages. If the kernel is in a low power mode
>> and the BT chip receives a message that requires handling by the app,
>> then a host wake gpio is driven to wake up the host (kernel).
> 
> So the reason then for having it all done in userspace is that there's
> a UART to communicate with it? Or is the reason that you just don't
> want the kernel abstraction? There *are* UART BT drivers in the kernel,
> as far as I can tell, what's the difference? Or is it that there's just
> a completely different protocol, vs. the HCI protocol used normally?
> 
> At this point I'm just curious though, doesn't seem all that important
> to the discussion :)

This clarifies the issue, thanks. Basically the reason is history - this
user space code has been around for a while. It turns out the HCI BT
driver doesn't exist as it should. If it did, then the regulator could
be tied to the driver. Now I see what's missing.

We will push on the team responsible for the BT drivers but all I need
to know right now is that an rfkill driver isn't necessary.
> 
> Although having a DT binding for a kernel driver for the device that
> includes the regulator might change the discussion somewhat?
> 
>> In addition to the regulator(s) controlled by the rfkill driver I
>> didn't mention the two gpio's previously. The existing rfkill driver
>> does nothing with them. It just exports the global gpio # to sysfs
>> which the app can read to control via the gpio sysfs interface. These
>> could be hard coded in a startup script or something, they aren't
>> that important but it does handle the conversion to the global gpio#
>> from 1 of 3 gpio controllers on a Cygnus SOC. I'm not sure how to
>> handle that, since the gpio's vary across board variants.
> 
> Could also just be exposed in the DT, I guess? Userspace can read the
> DT, no? Not sure that actually gives you all the dynamic information
> though that might be needed if there's any kind of mapping going on.

I'm not sure there's any way to have the mapping done by just specifying
the gpio somewhere outside a driver. This gpio functionality I mentioned
should probably be handled by the BT HCI driver.

> 
>>> Indeed, looks like Documentation/ABI/testing/sysfs-class-regulator
>>> is all read-only.
>>
>> There's a regulator driver named userspace-consumer.c that allows
>> enable/disable from userspace, but it doesn't support DT either.
> 
> Adding DT support there might make some sense, perhaps? Although that's
> not really a useful DT binding at all since it won't provide any hints
> as to how you'd use that, so ... maybe not so much.
> 
> As I read it, the main concern about or objection to the rfkill-
> regulator DT binding was that it's not tied to a device, when it seems
> that rfkill instances really should be living off a device that they
> control. That's true for many, but not all devices.
> 
> Today we already have all the platform rfkill instances (like the
> various ACPI ones) that don't live off a device that they control, but
> instead control the platform's radio functionality. And in some cases,
> that actually has very similar behaviour to what was proposed in the
> previous patch, and what you're looking for, in that it sometimes kills
> power to BT or WiFi chips when soft-disabled (or kicks them off the bus
> in some other way). Particularly for the ones that call some random
> ACPI function to deal with things, that can happen.
> 
> rfkill-regulator, with its platform driver today, more or less mirrors
> that. You could even use that with a very simple (few lines of code)
> platform module that just binds the regulators to rfkill-regulator, I
> think.
> 
> Making this more flexible and configurable with DT would make sense to
> me, less for the usage of rfkill-for-a-device (which could *also* be
> done, but isn't what you need or what was discussed before), but more
> for the usage of rfkill-for-the-platform.
> 
> Mark and DT folks - can you clarify your objections to the old patch?
> Am I reading things correctly, and if so, what do you think about the
> platform rfkill?

There might be some need for this but for our purpose I think the right
path is to have a BT driver doing what this user space app is doing and
then integrate whatever is missing with this HCI User Channel.

> 
> johannes
> 

Thanks,
Jon

Re: rfkill guidance

[Johannes Berg]

> This clarifies the issue, thanks. Basically the reason is history -
> this user space code has been around for a while. It turns out the
> HCI BT driver doesn't exist as it should. If it did, then the
> regulator could be tied to the driver. Now I see what's missing.

So like what Marcel wrote over in his email, but with regulator vs.
GPIO. Seems completely reasonable for the BT HCI driver to support
that, if you ask me.

> We will push on the team responsible for the BT drivers but all I
> need to know right now is that an rfkill driver isn't necessary.

And indeed, that would get rid of the need for anything rfkill.

> There might be some need for this but for our purpose I think the
> right path is to have a BT driver doing what this user space app is
> doing and then integrate whatever is missing with this HCI User
> Channel.
> 

Sounds good to me, even if I have no idea (and don't really need to
know) what all that is :)

I'll look at killing the rfkill-regulator driver entirely though,
since, afaict, nothing uses it.

johannes