DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 712972086C
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.1 \(3445.101.1\))
Subject: Re: [Patch v1] Bluetooth: Add Rfkill driver for Intel Bluetooth
 controller
From:   Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <CAKdAkRTfBV67=KGmimsc6obeHcjgo63UpG=Jn+C=NFDQ2cNE=A@mail.gmail.com>
Date:   Wed, 7 Nov 2018 19:48:13 +0100
Cc:     chethan tn <chethantn@gmail.com>,
        Chethan T N <chethan.tumkur.narayan@intel.com>,
        Bluez mailing list <linux-bluetooth@vger.kernel.org>,
        amit.k.bag@intel.com, raghuram.hegde@intel.com,
        sukumar.ghorai@intel.com, rajatja@chromium.org
Content-Transfer-Encoding: 8BIT
Message-Id: <4F7A2AA5-2CEC-4F2E-9691-099A6906EF0B@holtmann.org>
References: <1540907764-26294-1-git-send-email-chethan.tumkur.narayan@intel.com>
 <CAMH2TCoG80TCheJqPhu3vxQAvgALnSh8g=jbw_QFM9BOTBq1XA@mail.gmail.com>
 <20181107014427.GA1121@dtor-ws>
 <CD6DE466-5E73-473C-8943-684FBAEF337E@holtmann.org>
 <CAKdAkRTfBV67=KGmimsc6obeHcjgo63UpG=Jn+C=NFDQ2cNE=A@mail.gmail.com>
To:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Dmitry,

>>>> We are planning to further implement the followings, kindly please
>>>> provide your suggestions.
>>>> 1. To handle more than 1 Intel BT controller connected to platform,
>>>> will keep list of the objects in "static const struct acpi_device_id
>>>> intel_bt_rfkill_acpi_match[] ". And keep a list of "struct
>>>> intel_bt_rfkill_dev" for each of the acpi object.
>>>> 2.  With this implementation from user space RF kill for the device
>>>> object is achieved, however need to map the rfkill object with the
>>>> corresponding "hdev" so that on error from the controller kernel can
>>>> do the reset through this RF Kill driver.
>>> 
>>> I am confused, why you model a generic chip reset functionality via
>>> RFKill subsystem. As far as I understand, the issue is that you want to
>>> be able to reset the chip when it gets confused and not actually disable
>>> the chip/stop it from emitting RF signals.
>>> 
>>> I believe this functionality should be contained in the driver and you
>>> simply need to come with a way to tie the adapter instance with data in
>>> ACPI, probably based on physical USB connection.
>> 
>> it is impossible to do that in the driver since what the GPIO is doing is to push the USB device off the bus. So you actually see an USB disconnect and a new re-enumeration when it comes back. Meaning the driver knows nothing during that time.
> 
> The driver would know that it is in the middle of resetting the
> device. The fact that the device disappears from the bus is not a big
> deal. You just need to make sure you finish the reset task running
> before finishing teardown of the device in disconnect method.

it is a big deal and it is unpredictable. We can not magically assign resources to a device that is about to go away. And more importantly you have no idea which device goes away.

>> This is a classic soft RFKILL switch like we have seen in the early Thinkpads.
> 
> It is not RFkill as, as far as I understand, it does not guarantee
> that it actually blocks the transmitter. It really is a reset line and
> its purpose is to unwedge the chip, not keep it in the off state. I do
> not think there is a reason to export this as RFkill switch to
> userspace and then build infrastructure to recognize that this is
> special kind of RFkill switch that can be used to work around issues
> in the controller. Have the driver assert and deassert it to kick
> itself off the bus, the USB hotplug will take care of the rest.

It is a RFKILL switch since the device looses power and with also turns RF off. As I said, this is the same as with the old Thinkpads where we have done exactly the same.

Regards

Marcel