linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8] Bluetooth: btqca: inject command complete event during fw download
@ 2019-04-30  0:10 Matthias Kaehlcke
  2019-04-30  0:14 ` Matthias Kaehlcke
  2019-04-30  6:27 ` kbuild test robot
  0 siblings, 2 replies; 5+ messages in thread
From: Matthias Kaehlcke @ 2019-04-30  0:10 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, linux-kernel, Balakrishna Godavarthi,
	Harish Bandi, Rocky Liao, Matthias Kaehlcke

From: Balakrishna Godavarthi <bgodavar@codeaurora.org>

From: Balakrishna Godavarthi <bgodavar@codeaurora.org>

Latest qualcomm chips are not sending an command complete event for
every firmware packet sent to chip. They only respond with a vendor
specific event for the last firmware packet. This optimization will
decrease the BT ON time. Due to this we are seeing a timeout error
message logs on the console during firmware download. Now we are
injecting a command complete event once we receive an vendor specific
event for the last RAM firmware packet.

Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v8:
- renamed QCA_HCI_CC_SUCCESS to QCA_HCI_CC_OPCODE
- use 0xFC00 as opcode of the injected event instead of 0
- added Matthias' tags from the v7 review
---
 drivers/bluetooth/btqca.c | 39 ++++++++++++++++++++++++++++++++++++++-
 drivers/bluetooth/btqca.h |  3 +++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index cc12eecd9e4d..ef765ea881b8 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -144,6 +144,7 @@ static void qca_tlv_check_data(struct rome_config *config,
 		 * In case VSE is skipped, only the last segment is acked.
 		 */
 		config->dnld_mode = tlv_patch->download_mode;
+		config->dnld_type = config->dnld_mode;
 
 		BT_DBG("Total Length           : %d bytes",
 		       le32_to_cpu(tlv_patch->total_size));
@@ -264,6 +265,31 @@ static int qca_tlv_send_segment(struct hci_dev *hdev, int seg_size,
 	return err;
 }
 
+static int qca_inject_cmd_complete_event(struct hci_dev *hdev)
+{
+	struct hci_event_hdr *hdr;
+	struct hci_ev_cmd_complete *evt;
+	struct sk_buff *skb;
+
+	skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*evt) + 1, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = skb_put(skb, sizeof(*hdr));
+	hdr->evt = HCI_EV_CMD_COMPLETE;
+	hdr->plen = sizeof(*evt) + 1;
+
+	evt = skb_put(skb, sizeof(*evt));
+	evt->ncmd = 1;
+	evt->opcode = HCI_OP_NOP;
+
+	skb_put_u8(skb, QCA_HCI_CC_SUCCESS);
+
+	hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
+
+	return hci_recv_frame(hdev, skb);
+}
+
 static int qca_download_firmware(struct hci_dev *hdev,
 				  struct rome_config *config)
 {
@@ -297,11 +323,22 @@ static int qca_download_firmware(struct hci_dev *hdev,
 		ret = qca_tlv_send_segment(hdev, segsize, segment,
 					    config->dnld_mode);
 		if (ret)
-			break;
+			goto out;
 
 		segment += segsize;
 	}
 
+	/* Latest qualcomm chipsets are not sending a command complete event
+	 * for every fw packet sent. They only respond with a vendor specific
+	 * event for the last packet. This optimization in the chip will
+	 * decrease the BT in initialization time. Here we will inject a command
+	 * complete event to avoid a command timeout error message.
+	 */
+	if ((config->dnld_type == ROME_SKIP_EVT_VSE_CC ||
+	    config->dnld_type == ROME_SKIP_EVT_VSE))
+		return qca_inject_cmd_complete_event(hdev);
+
+out:
 	release_firmware(fw);
 
 	return ret;
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 4c4fe2b5b7b7..595abcdaed2d 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -41,6 +41,8 @@
 #define QCA_WCN3990_POWERON_PULSE	0xFC
 #define QCA_WCN3990_POWEROFF_PULSE	0xC0
 
+#define QCA_HCI_CC_OPCODE		0xFC00
+
 enum qca_baudrate {
 	QCA_BAUDRATE_115200 	= 0,
 	QCA_BAUDRATE_57600,
@@ -82,6 +84,7 @@ struct rome_config {
 	char fwname[64];
 	uint8_t user_baud_rate;
 	enum rome_tlv_dnld_mode dnld_mode;
+	enum rome_tlv_dnld_mode dnld_type;
 };
 
 struct edl_event_hdr {
-- 
2.21.0.593.g511ec345e18-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] Bluetooth: btqca: inject command complete event during fw download
  2019-04-30  0:10 [PATCH v8] Bluetooth: btqca: inject command complete event during fw download Matthias Kaehlcke
@ 2019-04-30  0:14 ` Matthias Kaehlcke
  2019-04-30  6:27 ` kbuild test robot
  1 sibling, 0 replies; 5+ messages in thread
From: Matthias Kaehlcke @ 2019-04-30  0:14 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, linux-kernel, Balakrishna Godavarthi,
	Harish Bandi, Rocky Liao

On Mon, Apr 29, 2019 at 05:10:24PM -0700, Matthias Kaehlcke wrote:
> From: Balakrishna Godavarthi <bgodavar@codeaurora.org>
> 
> From: Balakrishna Godavarthi <bgodavar@codeaurora.org>
> 
> Latest qualcomm chips are not sending an command complete event for
> every firmware packet sent to chip. They only respond with a vendor
> specific event for the last firmware packet. This optimization will
> decrease the BT ON time. Due to this we are seeing a timeout error
> message logs on the console during firmware download. Now we are
> injecting a command complete event once we receive an vendor specific
> event for the last RAM firmware packet.
> 
> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v8:
> - renamed QCA_HCI_CC_SUCCESS to QCA_HCI_CC_OPCODE
> - use 0xFC00 as opcode of the injected event instead of 0
> - added Matthias' tags from the v7 review
> ---
>  drivers/bluetooth/btqca.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  drivers/bluetooth/btqca.h |  3 +++
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index cc12eecd9e4d..ef765ea881b8 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -144,6 +144,7 @@ static void qca_tlv_check_data(struct rome_config *config,
>  		 * In case VSE is skipped, only the last segment is acked.
>  		 */
>  		config->dnld_mode = tlv_patch->download_mode;
> +		config->dnld_type = config->dnld_mode;
>  
>  		BT_DBG("Total Length           : %d bytes",
>  		       le32_to_cpu(tlv_patch->total_size));
> @@ -264,6 +265,31 @@ static int qca_tlv_send_segment(struct hci_dev *hdev, int seg_size,
>  	return err;
>  }
>  
> +static int qca_inject_cmd_complete_event(struct hci_dev *hdev)
> +{
> +	struct hci_event_hdr *hdr;
> +	struct hci_ev_cmd_complete *evt;
> +	struct sk_buff *skb;
> +
> +	skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*evt) + 1, GFP_KERNEL);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	hdr = skb_put(skb, sizeof(*hdr));
> +	hdr->evt = HCI_EV_CMD_COMPLETE;
> +	hdr->plen = sizeof(*evt) + 1;
> +
> +	evt = skb_put(skb, sizeof(*evt));
> +	evt->ncmd = 1;
> +	evt->opcode = HCI_OP_NOP;
> +
> +	skb_put_u8(skb, QCA_HCI_CC_SUCCESS);
> +
> +	hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
> +
> +	return hci_recv_frame(hdev, skb);
> +}
> +
>  static int qca_download_firmware(struct hci_dev *hdev,
>  				  struct rome_config *config)
>  {
> @@ -297,11 +323,22 @@ static int qca_download_firmware(struct hci_dev *hdev,
>  		ret = qca_tlv_send_segment(hdev, segsize, segment,
>  					    config->dnld_mode);
>  		if (ret)
> -			break;
> +			goto out;
>  
>  		segment += segsize;
>  	}
>  
> +	/* Latest qualcomm chipsets are not sending a command complete event
> +	 * for every fw packet sent. They only respond with a vendor specific
> +	 * event for the last packet. This optimization in the chip will
> +	 * decrease the BT in initialization time. Here we will inject a command
> +	 * complete event to avoid a command timeout error message.
> +	 */
> +	if ((config->dnld_type == ROME_SKIP_EVT_VSE_CC ||
> +	    config->dnld_type == ROME_SKIP_EVT_VSE))
> +		return qca_inject_cmd_complete_event(hdev);
> +
> +out:
>  	release_firmware(fw);
>  
>  	return ret;
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index 4c4fe2b5b7b7..595abcdaed2d 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -41,6 +41,8 @@
>  #define QCA_WCN3990_POWERON_PULSE	0xFC
>  #define QCA_WCN3990_POWEROFF_PULSE	0xC0
>  
> +#define QCA_HCI_CC_OPCODE		0xFC00
> +
>  enum qca_baudrate {
>  	QCA_BAUDRATE_115200 	= 0,
>  	QCA_BAUDRATE_57600,
> @@ -82,6 +84,7 @@ struct rome_config {
>  	char fwname[64];
>  	uint8_t user_baud_rate;
>  	enum rome_tlv_dnld_mode dnld_mode;
> +	enum rome_tlv_dnld_mode dnld_type;
>  };
>  
>  struct edl_event_hdr {

In the v7 review (https://lore.kernel.org/patchwork/patch/1028118/) 3
months ago Marcel said he isn't convinced that this solution is
needed, but didn't follow up on the discussion, so this is the best we
have at the moment. Let's please drive it towards a solution.

Thanks

Matthias

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] Bluetooth: btqca: inject command complete event during fw download
  2019-04-30  0:10 [PATCH v8] Bluetooth: btqca: inject command complete event during fw download Matthias Kaehlcke
  2019-04-30  0:14 ` Matthias Kaehlcke
@ 2019-04-30  6:27 ` kbuild test robot
  2019-04-30 15:38   ` Matthias Kaehlcke
  1 sibling, 1 reply; 5+ messages in thread
From: kbuild test robot @ 2019-04-30  6:27 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: kbuild-all, Marcel Holtmann, Johan Hedberg, linux-bluetooth,
	linux-kernel, Balakrishna Godavarthi, Harish Bandi, Rocky Liao,
	Matthias Kaehlcke

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

Hi Matthias,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on next-20190429]
[cannot apply to v5.1-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/Bluetooth-btqca-inject-command-complete-event-during-fw-download/20190430-125407
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/bluetooth/btqca.c: In function 'qca_inject_cmd_complete_event':
>> drivers/bluetooth/btqca.c:286:18: error: 'QCA_HCI_CC_SUCCESS' undeclared (first use in this function); did you mean 'QCA_HCI_CC_OPCODE'?
     skb_put_u8(skb, QCA_HCI_CC_SUCCESS);
                     ^~~~~~~~~~~~~~~~~~
                     QCA_HCI_CC_OPCODE
   drivers/bluetooth/btqca.c:286:18: note: each undeclared identifier is reported only once for each function it appears in

vim +286 drivers/bluetooth/btqca.c

   267	
   268	static int qca_inject_cmd_complete_event(struct hci_dev *hdev)
   269	{
   270		struct hci_event_hdr *hdr;
   271		struct hci_ev_cmd_complete *evt;
   272		struct sk_buff *skb;
   273	
   274		skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*evt) + 1, GFP_KERNEL);
   275		if (!skb)
   276			return -ENOMEM;
   277	
   278		hdr = skb_put(skb, sizeof(*hdr));
   279		hdr->evt = HCI_EV_CMD_COMPLETE;
   280		hdr->plen = sizeof(*evt) + 1;
   281	
   282		evt = skb_put(skb, sizeof(*evt));
   283		evt->ncmd = 1;
   284		evt->opcode = HCI_OP_NOP;
   285	
 > 286		skb_put_u8(skb, QCA_HCI_CC_SUCCESS);
   287	
   288		hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
   289	
   290		return hci_recv_frame(hdev, skb);
   291	}
   292	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56157 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] Bluetooth: btqca: inject command complete event during fw download
  2019-04-30  6:27 ` kbuild test robot
@ 2019-04-30 15:38   ` Matthias Kaehlcke
  2019-05-01  5:18     ` Balakrishna Godavarthi
  0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2019-04-30 15:38 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Marcel Holtmann, Johan Hedberg, linux-bluetooth,
	linux-kernel, Balakrishna Godavarthi, Harish Bandi, Rocky Liao

On Tue, Apr 30, 2019 at 02:27:33PM +0800, kbuild test robot wrote:
> Hi Matthias,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on bluetooth-next/master]
> [also build test ERROR on next-20190429]
> [cannot apply to v5.1-rc7]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/Bluetooth-btqca-inject-command-complete-event-during-fw-download/20190430-125407
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 8.1.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=8.1.0 make.cross ARCH=xtensa 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/bluetooth/btqca.c: In function 'qca_inject_cmd_complete_event':
> >> drivers/bluetooth/btqca.c:286:18: error: 'QCA_HCI_CC_SUCCESS' undeclared (first use in this function); did you mean 'QCA_HCI_CC_OPCODE'?
>      skb_put_u8(skb, QCA_HCI_CC_SUCCESS);
>                      ^~~~~~~~~~~~~~~~~~
>                      QCA_HCI_CC_OPCODE
>    drivers/bluetooth/btqca.c:286:18: note: each undeclared identifier is reported only once for each function it appears in
> 
> vim +286 drivers/bluetooth/btqca.c
> 
>    267	
>    268	static int qca_inject_cmd_complete_event(struct hci_dev *hdev)
>    269	{
>    270		struct hci_event_hdr *hdr;
>    271		struct hci_ev_cmd_complete *evt;
>    272		struct sk_buff *skb;
>    273	
>    274		skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*evt) + 1, GFP_KERNEL);
>    275		if (!skb)
>    276			return -ENOMEM;
>    277	
>    278		hdr = skb_put(skb, sizeof(*hdr));
>    279		hdr->evt = HCI_EV_CMD_COMPLETE;
>    280		hdr->plen = sizeof(*evt) + 1;
>    281	
>    282		evt = skb_put(skb, sizeof(*evt));
>    283		evt->ncmd = 1;
>    284		evt->opcode = HCI_OP_NOP;
>    285	
>  > 286		skb_put_u8(skb, QCA_HCI_CC_SUCCESS);

Oh, I changed it in my tree, but somehow missed to include this file
in the commit ...

I'll fix it in the next version. Since I expect the change to remain
controversial I'll wait a bit for other comments before sending out
v9.

Thanks

Matthias

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8] Bluetooth: btqca: inject command complete event during fw download
  2019-04-30 15:38   ` Matthias Kaehlcke
@ 2019-05-01  5:18     ` Balakrishna Godavarthi
  0 siblings, 0 replies; 5+ messages in thread
From: Balakrishna Godavarthi @ 2019-05-01  5:18 UTC (permalink / raw)
  To: Matthias Kaehlcke, Harish Bandi
  Cc: kbuild test robot, kbuild-all, Marcel Holtmann, Johan Hedberg,
	linux-bluetooth, linux-kernel, Harish Bandi, Rocky Liao

Hi Harish,

On 2019-04-30 21:08, Matthias Kaehlcke wrote:
> On Tue, Apr 30, 2019 at 02:27:33PM +0800, kbuild test robot wrote:
>> Hi Matthias,
>> 
>> Thank you for the patch! Yet something to improve:
>> 
>> [auto build test ERROR on bluetooth-next/master]
>> [also build test ERROR on next-20190429]
>> [cannot apply to v5.1-rc7]
>> [if your patch is applied to the wrong git tree, please drop us a note 
>> to help improve the system]
>> 
>> url:    
>> https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/Bluetooth-btqca-inject-command-complete-event-during-fw-download/20190430-125407
>> base:   
>> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git 
>> master
>> config: xtensa-allyesconfig (attached as .config)
>> compiler: xtensa-linux-gcc (GCC) 8.1.0
>> reproduce:
>>         wget 
>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
>> -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         # save the attached .config to linux build tree
>>         GCC_VERSION=8.1.0 make.cross ARCH=xtensa
>> 
>> If you fix the issue, kindly add following tag
>> Reported-by: kbuild test robot <lkp@intel.com>
>> 
>> All errors (new ones prefixed by >>):
>> 
>>    drivers/bluetooth/btqca.c: In function 
>> 'qca_inject_cmd_complete_event':
>> >> drivers/bluetooth/btqca.c:286:18: error: 'QCA_HCI_CC_SUCCESS' undeclared (first use in this function); did you mean 'QCA_HCI_CC_OPCODE'?
>>      skb_put_u8(skb, QCA_HCI_CC_SUCCESS);
>>                      ^~~~~~~~~~~~~~~~~~
>>                      QCA_HCI_CC_OPCODE
>>    drivers/bluetooth/btqca.c:286:18: note: each undeclared identifier 
>> is reported only once for each function it appears in
>> 
>> vim +286 drivers/bluetooth/btqca.c
>> 
>>    267
>>    268	static int qca_inject_cmd_complete_event(struct hci_dev *hdev)
>>    269	{
>>    270		struct hci_event_hdr *hdr;
>>    271		struct hci_ev_cmd_complete *evt;
>>    272		struct sk_buff *skb;
>>    273
>>    274		skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*evt) + 1, 
>> GFP_KERNEL);
>>    275		if (!skb)
>>    276			return -ENOMEM;
>>    277
>>    278		hdr = skb_put(skb, sizeof(*hdr));
>>    279		hdr->evt = HCI_EV_CMD_COMPLETE;
>>    280		hdr->plen = sizeof(*evt) + 1;
>>    281
>>    282		evt = skb_put(skb, sizeof(*evt));
>>    283		evt->ncmd = 1;
>>    284		evt->opcode = HCI_OP_NOP;
>>    285
>>  > 286		skb_put_u8(skb, QCA_HCI_CC_SUCCESS);
> 
> Oh, I changed it in my tree, but somehow missed to include this file
> in the commit ...
> 
> I'll fix it in the next version. Since I expect the change to remain
> controversial I'll wait a bit for other comments before sending out
> v9.
> 
> Thanks
> 
> Matthias

[Bala]: can you check whether this change  is applicable for wcn3998 as 
well.

@Matthias, Thanks Matthias for taking Up :)

-- 
Regards
Balakrishna.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-05-01  5:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-30  0:10 [PATCH v8] Bluetooth: btqca: inject command complete event during fw download Matthias Kaehlcke
2019-04-30  0:14 ` Matthias Kaehlcke
2019-04-30  6:27 ` kbuild test robot
2019-04-30 15:38   ` Matthias Kaehlcke
2019-05-01  5:18     ` Balakrishna Godavarthi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).