diff mbox

RFkill for Bluetooth hard reset

Message ID D73ECEF0C711E34A8F972E4B87F540058FB55F8D@ORSMSX114.amr.corp.intel.com (mailing list archive)
State Not Applicable
Delegated to: Johannes Berg
Headers show

Commit Message

Ghorai, Sukumar July 8, 2018, 11:28 p.m. UTC
Hi,
We need an option to reset Bluetooth using GPIO toggle for a Linux product when
BT connected over USB (HCI interface). i.e I need out of band(OOB) signal for
USB to control/reset the BT. hence I opt to add ACPI device in RfKill gpio
driver(rfkill-gpio.c).

localhost /sys/class/rfkill # ls -l
total 0
lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill0 -> ../../devices/platform/INTL6205:00/rfkill/rfkill0
lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill2 -> ../../devices/pci0000:00/0000:00:1c.0/0000:01:00.0/ieee80211/phy0/rfkill2
lrwxrwxrwx. 1 root root 0 Jul  8 15:35 rfkill3 -> ../../devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/bluetooth/hci0/rfkill3

I try to use RFkill soft block option to hold/reset (toggle) GPIO. And changes
in rfkill-gpio.c as follows.
1/ reset pin configured as GPIOD_OUT_LOW, hence HW controller is not out of
reset.  I changed to high to work with our controller. 
How to configured as Low/High as a generic solution?

2/ As per existing implementation, the Shutdown gpio is mandatory for the 
framework to use. I feel we can make it an optional.

3/ Followed by /sys/class/rfkill/rfkill0/soft (1/0) is not in sync with
/sys/class/rfkill/rfkill0/uvent (RFKILL_STATE=0/1)

Please guide me to add an ACPI method to toggle the GPIO to reset the bluetooth. 

Thx & rgds,
Sukumar

Comments

Marcel Holtmann July 10, 2018, 6:46 p.m. UTC | #1
Hi Sukumar,

> We need an option to reset Bluetooth using GPIO toggle for a Linux product when
> BT connected over USB (HCI interface). i.e I need out of band(OOB) signal for
> USB to control/reset the BT. hence I opt to add ACPI device in RfKill gpio
> driver(rfkill-gpio.c).
> 
> localhost /sys/class/rfkill # ls -l
> total 0
> lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill0 -> ../../devices/platform/INTL6205:00/rfkill/rfkill0
> lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill2 -> ../../devices/pci0000:00/0000:00:1c.0/0000:01:00.0/ieee80211/phy0/rfkill2
> lrwxrwxrwx. 1 root root 0 Jul  8 15:35 rfkill3 -> ../../devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/bluetooth/hci0/rfkill3
> 
> I try to use RFkill soft block option to hold/reset (toggle) GPIO. And changes
> in rfkill-gpio.c as follows.
> 1/ reset pin configured as GPIOD_OUT_LOW, hence HW controller is not out of
> reset.  I changed to high to work with our controller. 
> How to configured as Low/High as a generic solution?
> 
> 2/ As per existing implementation, the Shutdown gpio is mandatory for the 
> framework to use. I feel we can make it an optional.
> 
> 3/ Followed by /sys/class/rfkill/rfkill0/soft (1/0) is not in sync with
> /sys/class/rfkill/rfkill0/uvent (RFKILL_STATE=0/1)
> 
> Please guide me to add an ACPI method to toggle the GPIO to reset the bluetooth. 

if the setup is that when these GPIOs are triggered, then the Bluetooth USB device disappears from the USB bus (and with that from sysfs), then it is fine to use an external RFKILL switch. This is similar to what Thinkpads used to do.

It is kinda okay to use RFKILL_TYPE_BLUETOOTH for this, but in reality this is RFKILL_TYPE_USB since it is killing the USB device. That Bluetooth gets also killed is just a side effect.

One thing that would be nice if ACPI would kinda link the RFKILL switch entry to the USB port so that you could relate on which USB device will be actually taken off the USB bus.

If this is _not_ killing the USB power to the device and taking it off the bus, then this way is all wrong. For wakeup and reset GPIOs, this then needs to be build into btusb.c like has been done for all the UART drivers.

Bluetooth at the moment does not have a hard kill switch in its subsystem (like WiFi does), but that could be added if the Bluetooth device stays on the bus and needs an external RFKILL switch. Bluetooth however does have a soft kill switch, but that is not going to help here.

So first of all state what the current behavior and design is.

Regards

Marcel
Ghorai, Sukumar July 10, 2018, 10:07 p.m. UTC | #2
>-----Original Message-----
>From: Marcel Holtmann [mailto:marcel@holtmann.org]
>Sent: Tuesday, July 10, 2018 11:46 AM
>To: Ghorai, Sukumar <sukumar.ghorai@intel.com>
>Cc: Johannes Berg <johannes@sipsolutions.net>; David S. Miller
><davem@davemloft.net>; open list:RFKILL <linux-wireless@vger.kernel.org>;
>Holtmann, Marcel <marcel.holtmann@intel.com>; Tumkur Narayan, Chethan
><chethan.tumkur.narayan@intel.com>
>Subject: Re: RFkill for Bluetooth hard reset
>
>Hi Sukumar,
>
>> We need an option to reset Bluetooth using GPIO toggle for a Linux
>> product when BT connected over USB (HCI interface). i.e I need out of
>> band(OOB) signal for USB to control/reset the BT. hence I opt to add
>> ACPI device in RfKill gpio driver(rfkill-gpio.c).
>>
>> localhost /sys/class/rfkill # ls -l
>> total 0
>> lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill0 ->
>> ../../devices/platform/INTL6205:00/rfkill/rfkill0
>> lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill2 ->
>> ../../devices/pci0000:00/0000:00:1c.0/0000:01:00.0/ieee80211/phy0/rfki
>> ll2 lrwxrwxrwx. 1 root root 0 Jul  8 15:35 rfkill3 ->
>> ../../devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/bluetooth/hci0/
>> rfkill3
>>
>> I try to use RFkill soft block option to hold/reset (toggle) GPIO. And
>> changes in rfkill-gpio.c as follows.
>> 1/ reset pin configured as GPIOD_OUT_LOW, hence HW controller is not
>> out of reset.  I changed to high to work with our controller.
>> How to configured as Low/High as a generic solution?
>>
>> 2/ As per existing implementation, the Shutdown gpio is mandatory for
>> the framework to use. I feel we can make it an optional.
>>
>> 3/ Followed by /sys/class/rfkill/rfkill0/soft (1/0) is not in sync
>> with /sys/class/rfkill/rfkill0/uvent (RFKILL_STATE=0/1)
>>
>> Please guide me to add an ACPI method to toggle the GPIO to reset the
>bluetooth.
I appreciate how imaginative you are. I appreciate your support.

>if the setup is that when these GPIOs are triggered, then the Bluetooth USB
>device disappears from the USB bus (and with that from sysfs), then it is fine to
>use an external RFKILL switch. This is similar to what Thinkpads used to do.
>
Platform exposes the ACPI device node. Platform GPIO binds to use for 
controlling reset lines of Bluetooth controller.  Sets the GPIO address low to 
chip disable i.e Device(bt controller) will disconnect from the bus. And config
gpio high triggered bt-chip reset line to boot and host re-enumerate the device. 
Hence OS UI/application can use the Linux system interface 
/sys/class/rfkill/rfkillx/ type and state) with airplane-mode or bluetooth OFF/ON. 
Or similar dbus interface.

>It is kinda okay to use RFKILL_TYPE_BLUETOOTH for this, but in reality this is
>RFKILL_TYPE_USB since it is killing the USB device. That Bluetooth gets also
>killed is just a side effect.
The platform will expose the dedicated ACPI device node (INTL6205) to control 
all Intel bluetooth controller only, when supported.

>One thing that would be nice if ACPI would kinda link the RFKILL switch entry to
>the USB port so that you could relate on which USB device will be actually taken
>off the USB bus.
>
>If this is _not_ killing the USB power to the device and taking it off the bus, then
>this way is all wrong. For wakeup and reset GPIOs, this then needs to be build
>into btusb.c like has been done for all the UART drivers.
To control the Bluetooth hardware, the driver to a combination of in-band bus
communication(USB) and coordinate with out-of-band signaling through
GPIO pins. And still looking for possibility to add the Out-of-band ACPI 
signaling mechanism to hook with btusb.c. 

>Bluetooth at the moment does not have a hard kill switch in its subsystem (like
>WiFi does), but that could be added if the Bluetooth device stays on the bus and
>needs an external RFKILL switch. Bluetooth however does have a soft kill switch,
>but that is not going to help here.
This GPIO based device (BT- controller) OFF/ON is similar to hard kill, as the device
disappears from USB bus and followed by re-enumerate after reset (OFF and ON) 

>So first of all state what the current behavior and design is.
>
>Regards
>Marcel
Marcel Holtmann July 11, 2018, 2:52 p.m. UTC | #3
Hi Sukumar,

>>> We need an option to reset Bluetooth using GPIO toggle for a Linux
>>> product when BT connected over USB (HCI interface). i.e I need out of
>>> band(OOB) signal for USB to control/reset the BT. hence I opt to add
>>> ACPI device in RfKill gpio driver(rfkill-gpio.c).
>>> 
>>> localhost /sys/class/rfkill # ls -l
>>> total 0
>>> lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill0 ->
>>> ../../devices/platform/INTL6205:00/rfkill/rfkill0
>>> lrwxrwxrwx. 1 root root 0 Jul  8 14:53 rfkill2 ->
>>> ../../devices/pci0000:00/0000:00:1c.0/0000:01:00.0/ieee80211/phy0/rfki
>>> ll2 lrwxrwxrwx. 1 root root 0 Jul  8 15:35 rfkill3 ->
>>> ../../devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/bluetooth/hci0/
>>> rfkill3
>>> 
>>> I try to use RFkill soft block option to hold/reset (toggle) GPIO. And
>>> changes in rfkill-gpio.c as follows.
>>> 1/ reset pin configured as GPIOD_OUT_LOW, hence HW controller is not
>>> out of reset.  I changed to high to work with our controller.
>>> How to configured as Low/High as a generic solution?
>>> 
>>> 2/ As per existing implementation, the Shutdown gpio is mandatory for
>>> the framework to use. I feel we can make it an optional.
>>> 
>>> 3/ Followed by /sys/class/rfkill/rfkill0/soft (1/0) is not in sync
>>> with /sys/class/rfkill/rfkill0/uvent (RFKILL_STATE=0/1)
>>> 
>>> Please guide me to add an ACPI method to toggle the GPIO to reset the
>> bluetooth.
> I appreciate how imaginative you are. I appreciate your support.
> 
>> if the setup is that when these GPIOs are triggered, then the Bluetooth USB
>> device disappears from the USB bus (and with that from sysfs), then it is fine to
>> use an external RFKILL switch. This is similar to what Thinkpads used to do.
>> 
> Platform exposes the ACPI device node. Platform GPIO binds to use for 
> controlling reset lines of Bluetooth controller.  Sets the GPIO address low to 
> chip disable i.e Device(bt controller) will disconnect from the bus. And config
> gpio high triggered bt-chip reset line to boot and host re-enumerate the device. 
> Hence OS UI/application can use the Linux system interface 
> /sys/class/rfkill/rfkillx/ type and state) with airplane-mode or bluetooth OFF/ON. 
> Or similar dbus interface.
> 
>> It is kinda okay to use RFKILL_TYPE_BLUETOOTH for this, but in reality this is
>> RFKILL_TYPE_USB since it is killing the USB device. That Bluetooth gets also
>> killed is just a side effect.
> The platform will expose the dedicated ACPI device node (INTL6205) to control 
> all Intel bluetooth controller only, when supported.
> 
>> One thing that would be nice if ACPI would kinda link the RFKILL switch entry to
>> the USB port so that you could relate on which USB device will be actually taken
>> off the USB bus.
>> 
>> If this is _not_ killing the USB power to the device and taking it off the bus, then
>> this way is all wrong. For wakeup and reset GPIOs, this then needs to be build
>> into btusb.c like has been done for all the UART drivers.
> To control the Bluetooth hardware, the driver to a combination of in-band bus
> communication(USB) and coordinate with out-of-band signaling through
> GPIO pins. And still looking for possibility to add the Out-of-band ACPI 
> signaling mechanism to hook with btusb.c. 

this is where you are wrong. It has nothing to do with Bluetooth in this case. It is the "stabbing in the back" kind of thing that you are doing with the hardware here. It is the "have you tried to switch your PC off and back on" approach. Or if you unplug the hardware and plug it back in.

The problem is that no relation between the ACPI GPIO information and the PCIe card with the USB lines connected exists. If you kill the USB power, then the btusb.ko driver will unregister the hci_dev and thus all device state is gone.

As said above, since this is killing the power, it is fine to do this via an external RFKILL_TYPE_BLUETOOTH switch. However it would be nice if that switch exposes somehow which USB port it is that has its device taken offline.

>> Bluetooth at the moment does not have a hard kill switch in its subsystem (like
>> WiFi does), but that could be added if the Bluetooth device stays on the bus and
>> needs an external RFKILL switch. Bluetooth however does have a soft kill switch,
>> but that is not going to help here.
> This GPIO based device (BT- controller) OFF/ON is similar to hard kill, as the device
> disappears from USB bus and followed by re-enumerate after reset (OFF and ON) 

There is a slight difference here if the USB device stays on the bus and is removed from the bus. This is what I am trying to say, there are 3 types of kill switches.

Regards

Marcel
diff mbox

Patch

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index da6d1cf4c11c..5054694c5b3b 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -112,17 +112,17 @@  static int rfkill_gpio_probe(struct platform_device *pdev)

        rfkill->clk = devm_clk_get(&pdev->dev, NULL);

-       gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
+       gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_HIGH);
        if (IS_ERR(gpio))
                return PTR_ERR(gpio);

        rfkill->reset_gpio = gpio;

-       gpio = devm_gpiod_get_optional(&pdev->dev, "shutdown", GPIOD_OUT_LOW);
-       if (IS_ERR(gpio))
-               return PTR_ERR(gpio);
+       //gpio = devm_gpiod_get_optional(&pdev->dev, "shutdown", GPIOD_OUT_LOW);
+       ////if (IS_ERR(gpio))
+       ////    return PTR_ERR(gpio);

-       rfkill->shutdown_gpio = gpio;
+       //rfkill->shutdown_gpio = gpio;

        /* Make sure at-least one of the GPIO is defined and that
         * a name is specified for this instance
@@ -170,6 +170,7 @@  static int rfkill_gpio_remove(struct platform_device *pdev)
 static const struct acpi_device_id rfkill_acpi_match[] = {
        { "BCM4752", RFKILL_TYPE_GPS },
        { "LNV4752", RFKILL_TYPE_GPS },
+       { "INTL6205", RFKILL_TYPE_BLUETOOTH },
        { },