Return-Path: <cyrus@holtmann.org>
From: "An, Tedd" <tedd.an@intel.com>
To: "Hedberg, Johan" <johan.hedberg@intel.com>
CC: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
	Marcel Holtmann <marcel@holtmann.org>
Subject: Re: [PATCH] Bluetooth: Cancel the hci_request timeout if it
 received expected event
Date: Tue, 23 Dec 2014 19:09:05 +0000
Message-ID: <D0BEFBBC.573B%tedd.an@intel.com>
References: <20141222190142.59176a26@tedd-test>
 <20141223185511.GA5686@t440s.P-661HNU-F1>
In-Reply-To: <20141223185511.GA5686@t440s.P-661HNU-F1>
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
List-ID: <linux-bluetooth.vger.kernel.org>

Hi Johan

On 12/23/14, 10:55 AM, "Johan Hedberg" <johan.hedberg@intel.com> wrote:

>Hi Tedd,
>
>On Mon, Dec 22, 2014, Tedd Ho-Jeong An wrote:
>> This patch cancels the hci_request timeout work if the expected event
>> is recevied.
>
>The timer you're canceling isn't hci_request specific, so I'm not sure
>why you're making that reference here? You might want to provide some
>more detailed explanation in the commit message.

When the command is sent with __hci_cmd_sync_ev() and expected event is
other than Command_Complete or
Command_Status, especially, vendor specific event (0xFF), the function
returns the skb filled with
event parameters properly, but there is tx timeout error because the
cmd_timer is not canceled.

I will send out updated patch with details.


>
>> +		if (opcode !=3D HCI_OP_NOP)
>> +			cancel_delayed_work(&hdev->cmd_timer);
>
>I don't think the check for HCI_OP_NOP is necessary here. The opcode is
>from the original command that was sent and it can't be HCI_OP_NOP. The
>other places checking for this before calling cancel_delayed_work() take
>their opcode from the cmd_status/cmd_complete events where it has
>special meaning (i.e. a spontaneous event generated by the controller).

Got it.
=20
>
>Johan