On 2018-09-22 08:53, Brian Norris wrote:
Hi again!

One last (?) comment:

On Thu, Aug 30, 2018 at 10:29:42AM +0800, Carl Huang wrote:

diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index 677535b..25ee1c6 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c

@@ -918,6 +919,190 @@ static int ath10k_hw_qca6174_enable_pll_clock(struct ath10k *ar)

+static int ath10k_hw_diag_segment_download(struct ath10k *ar,
+                                          const void *buffer,
+                                          u32 address,
+                                          u32 length)
+{
+       if (address >= DRAM_BASE_ADDRESS + REGION_ACCESS_SIZE_LIMIT)
+               /* Needs to change MSB for memory write */
+               return ath10k_hw_diag_segment_msb_download(ar, buffer,
+                                                          address, length);
+       else
+               return ath10k_hif_diag_write(ar, address, buffer, length);
+}
+
+int ath10k_hw_diag_fast_download(struct ath10k *ar,
+                                u32 address,
+                                const void *buffer,
+                                u32 length)
+{
+       const u8 *buf = buffer;
+       bool sgmt_end = false;
+       u32 base_addr = 0;
+       u32 base_len = 0;
+       u32 left = 0;
+       struct bmi_segmented_file_header *hdr;
+       struct bmi_segmented_metadata *metadata;
+       int ret = 0;
+
+       if (length < sizeof(*hdr))
+               return -EINVAL;
+
+       /* check firmware header. If it has no correct magic number
+        * or it's compressed, returns error.
+        */
+       hdr = (struct bmi_segmented_file_header *)buf;
+       if (hdr->magic_num != BMI_SGMTFILE_MAGIC_NUM) {
+               ath10k_dbg(ar, ATH10K_DBG_BOOT,
+                          "Not a supported firmware, magic_num:0x%x\n",
+                          hdr->magic_num);
+               return -EINVAL;
+       }
+
+       if (hdr->file_flags != 0) {
+               ath10k_dbg(ar, ATH10K_DBG_BOOT,
+                          "Not a supported firmware, file_flags:0x%x\n",
+                          hdr->file_flags);
+               return -EINVAL;
+       }
+
+       metadata = (struct bmi_segmented_metadata *)hdr->data;
+       left = length - sizeof(*hdr);
+
+       while (left > 0) {
+               base_addr = metadata->addr;
+               base_len = metadata->length;
+               buf = metadata->data;
+               left -= sizeof(*metadata);
+
+               switch (base_len) {
...
+               default:
+                       if (base_len > left) {
+                               /* sanity check */
+                               ath10k_warn(ar,
+                                           "firmware has invalid segment length, %d 
> %d\n",
+                                           base_len, left);
+                               ret = -EINVAL;
+                               break;
+                       }
+
+                       ret = ath10k_hw_diag_segment_download(ar,
+                                                             buf,
+                                                             base_addr,
+                                                             base_len);

This 'base_len' is determined by the firmware blob and in common cases
is over 500K. The PCI implementation currently tries to
dma_alloc_coherent() a bounce buffer for this. Many systems can't
acquire that large of contiguous DMA memory reliably, so this is isn't
very effective. Can we improve the strategy here? Do you lose a lot of
speed if you do this in smaller (a few pages?) chunks instead?


It's a sound complaint. I'll submit a patch for it.


Brian

+
+                       if (ret)
+                               ath10k_warn(ar,
+                                           "failed to download firmware via diag 
interface:%d\n",
+                                           ret);
+                       break;
...

Reply via email to