[PATCH] btusb: Add shutdown routine for BTUSB_INTEL_NEW devices

[PATCH] btusb: Add shutdown routine for BTUSB_INTEL_NEW devices

[raghuram.hegde]

From: Raghuram Hegde <raghuram.hegde@intel.com>

btusb_shutdown_intel routine shall reset the controller
and stop all BT operation.
Advantages:
	1. Power save on the platform
	2. Host and controller will be in Sync.

Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
---
 drivers/bluetooth/btusb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5de0c2e59b97..66483ca3d870 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3139,6 +3139,7 @@ static int btusb_probe(struct usb_interface *intf,
		hdev->manufacturer = 2;
		hdev->send = btusb_send_frame_intel;
		hdev->setup = btusb_setup_intel_new;
+		hdev->shutdown = btusb_shutdown_intel;
		hdev->hw_error = btintel_hw_error;
		hdev->set_diag = btintel_set_diag;
		hdev->set_bdaddr = btintel_set_bdaddr;
--
2.7.4

Re: [PATCH] btusb: Add shutdown routine for BTUSB_INTEL_NEW devices

[Marcel Holtmann]

Hi Raghuram,

> btusb_shutdown_intel routine shall reset the controller
> and stop all BT operation.
> Advantages:
> 	1. Power save on the platform
> 	2. Host and controller will be in Sync.
> 
> Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> ---
> drivers/bluetooth/btusb.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 5de0c2e59b97..66483ca3d870 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3139,6 +3139,7 @@ static int btusb_probe(struct usb_interface *intf,
> 		hdev->manufacturer = 2;
> 		hdev->send = btusb_send_frame_intel;
> 		hdev->setup = btusb_setup_intel_new;
> +		hdev->shutdown = btusb_shutdown_intel;
> 		hdev->hw_error = btintel_hw_error;
> 		hdev->set_diag = btintel_set_diag;
> 		hdev->set_bdaddr = btintel_set_bdaddr;

I assumed that this was only needed for the older ROM versions of the Intel controllers and not the newer RAM versions. I have been told they don’t inherit the LED issue that we tried to fix with this. Please read the comments in btusb_shutdown_intel and amend comments if needed and provide a detailed commit message.

Regards

Marcel

Re: [PATCH] btusb: Add shutdown routine for BTUSB_INTEL_NEW devices

[chethan tn]

Hi Marcel,

On Mon, Jan 28, 2019 at 5:37 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Raghuram,
>
> > btusb_shutdown_intel routine shall reset the controller
> > and stop all BT operation.
> > Advantages:
> >       1. Power save on the platform
> >       2. Host and controller will be in Sync.
> >
> > Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
> > Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > ---
> > drivers/bluetooth/btusb.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 5de0c2e59b97..66483ca3d870 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -3139,6 +3139,7 @@ static int btusb_probe(struct usb_interface *intf,
> >               hdev->manufacturer = 2;
> >               hdev->send = btusb_send_frame_intel;
> >               hdev->setup = btusb_setup_intel_new;
> > +             hdev->shutdown = btusb_shutdown_intel;
> >               hdev->hw_error = btintel_hw_error;
> >               hdev->set_diag = btintel_set_diag;
> >               hdev->set_bdaddr = btintel_set_bdaddr;
>
> I assumed that this was only needed for the older ROM versions of the Intel controllers and not the newer RAM versions. I have been told they don’t inherit the LED issue that we tried to fix with this. Please read the comments in btusb_shutdown_intel and amend comments if needed and provide a detailed commit message.
Yes you're absolutely right about the LED issue on the older ROM products.
But in the recent day we have observed that in case BT
operation(Inquiry/LE Scan) were triggered through the stack and
further BT was turned off through "hciconfig hci0 down". In this case
controller would active doing BT operation and consume power and also
might cause race condition on the next BT on as the controller might
try to push the events that were queued up before processing the reset
command. So to make sure when BT is turned off either through stack or
through command line thought it would be better approach to reset the
controller(This is applicable for ROM or RAM products).
>
> Regards
>
> Marcel
>

Re: [PATCH] btusb: Add shutdown routine for BTUSB_INTEL_NEW devices

[Marcel Holtmann]

Hi Chethan,

>>> btusb_shutdown_intel routine shall reset the controller
>>> and stop all BT operation.
>>> Advantages:
>>>      1. Power save on the platform
>>>      2. Host and controller will be in Sync.
>>> 
>>> Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
>>> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
>>> ---
>>> drivers/bluetooth/btusb.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>> 
>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>> index 5de0c2e59b97..66483ca3d870 100644
>>> --- a/drivers/bluetooth/btusb.c
>>> +++ b/drivers/bluetooth/btusb.c
>>> @@ -3139,6 +3139,7 @@ static int btusb_probe(struct usb_interface *intf,
>>>              hdev->manufacturer = 2;
>>>              hdev->send = btusb_send_frame_intel;
>>>              hdev->setup = btusb_setup_intel_new;
>>> +             hdev->shutdown = btusb_shutdown_intel;
>>>              hdev->hw_error = btintel_hw_error;
>>>              hdev->set_diag = btintel_set_diag;
>>>              hdev->set_bdaddr = btintel_set_bdaddr;
>> 
>> I assumed that this was only needed for the older ROM versions of the Intel controllers and not the newer RAM versions. I have been told they don’t inherit the LED issue that we tried to fix with this. Please read the comments in btusb_shutdown_intel and amend comments if needed and provide a detailed commit message.
> Yes you're absolutely right about the LED issue on the older ROM products.
> But in the recent day we have observed that in case BT
> operation(Inquiry/LE Scan) were triggered through the stack and
> further BT was turned off through "hciconfig hci0 down". In this case
> controller would active doing BT operation and consume power and also
> might cause race condition on the next BT on as the controller might
> try to push the events that were queued up before processing the reset
> command. So to make sure when BT is turned off either through stack or
> through command line thought it would be better approach to reset the
> controller(This is applicable for ROM or RAM products).

what about creating a btusb_shutdown_intel_new() that just sends the HCI Reset command. That should be enough to ensure that anything in the radio scheduler gets cancelled and cleaned up.

Regards

Marcel

Re: [PATCH] btusb: Add shutdown routine for BTUSB_INTEL_NEW devices

[chethan tn]

Hi Marcel

On Mon, Jan 28, 2019 at 9:30 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Chethan,
>
> >>> btusb_shutdown_intel routine shall reset the controller
> >>> and stop all BT operation.
> >>> Advantages:
> >>>      1. Power save on the platform
> >>>      2. Host and controller will be in Sync.
> >>>
> >>> Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
> >>> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> >>> ---
> >>> drivers/bluetooth/btusb.c | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >>> index 5de0c2e59b97..66483ca3d870 100644
> >>> --- a/drivers/bluetooth/btusb.c
> >>> +++ b/drivers/bluetooth/btusb.c
> >>> @@ -3139,6 +3139,7 @@ static int btusb_probe(struct usb_interface *intf,
> >>>              hdev->manufacturer = 2;
> >>>              hdev->send = btusb_send_frame_intel;
> >>>              hdev->setup = btusb_setup_intel_new;
> >>> +             hdev->shutdown = btusb_shutdown_intel;
> >>>              hdev->hw_error = btintel_hw_error;
> >>>              hdev->set_diag = btintel_set_diag;
> >>>              hdev->set_bdaddr = btintel_set_bdaddr;
> >>
> >> I assumed that this was only needed for the older ROM versions of the Intel controllers and not the newer RAM versions. I have been told they don’t inherit the LED issue that we tried to fix with this. Please read the comments in btusb_shutdown_intel and amend comments if needed and provide a detailed commit message.
> > Yes you're absolutely right about the LED issue on the older ROM products.
> > But in the recent day we have observed that in case BT
> > operation(Inquiry/LE Scan) were triggered through the stack and
> > further BT was turned off through "hciconfig hci0 down". In this case
> > controller would active doing BT operation and consume power and also
> > might cause race condition on the next BT on as the controller might
> > try to push the events that were queued up before processing the reset
> > command. So to make sure when BT is turned off either through stack or
> > through command line thought it would be better approach to reset the
> > controller(This is applicable for ROM or RAM products).
>
> what about creating a btusb_shutdown_intel_new() that just sends the HCI Reset command. That should be enough to ensure that anything in the radio scheduler gets cancelled and cleaned up.
Sure,  will make the changes accordingly and send the new patch.
>
> Regards
>
> Marcel
>
Thanks and Regards
Chethan