| Message ID | 1538992182-3717-1-git-send-email-cjhuang@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: allocate small size dma memory in ath10k_pci_diag_write_mem | expand |
Hi Carl, On Mon, Oct 08, 2018 at 05:49:42PM +0800, Carl Huang wrote: > ath10k_pci_diag_write_mem may allocate big size of the dma memory > based on the parameter nbytes. Take firmware diag download as > example, the biggest size is about 500K. In some systems, the > allocation is likely to fail because it can't acquire such a large > contiguous dma memory. > > The fix is to allocate a small size dma memory. In the loop, > driver copies the data to the allocated dma memory and writes to > the destination until all the data is written. > > Tested with QCA6174 PCI with > firmware-6.bin_WLAN.RM.4.4.1-00119-QCARMSWP-1, this also affects > QCA9377 PCI. > > Signed-off-by: Carl Huang <cjhuang@codeaurora.org> Thanks for the patch! With one small comment, I think this otherwise looks good, so feel free to add my: Reviewed-by: Brian Norris <briannorris@chomium.org> > --- > drivers/net/wireless/ath/ath10k/pci.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c > index 873dbb6..1872671 100644 > --- a/drivers/net/wireless/ath/ath10k/pci.c > +++ b/drivers/net/wireless/ath/ath10k/pci.c > @@ -1071,7 +1071,7 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, > struct ath10k_ce *ce = ath10k_ce_priv(ar); > int ret = 0; > u32 *buf; > - unsigned int completed_nbytes, orig_nbytes, remaining_bytes; > + unsigned int completed_nbytes, alloc_nbytes, remaining_bytes; > struct ath10k_ce_pipe *ce_diag; > void *data_buf = NULL; > u32 ce_data; /* Host buffer address in CE space */ ^^ I don't think this is needed any more. > @@ -1088,9 +1088,10 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, > * 1) 4-byte alignment > * 2) Buffer in DMA-able space > */ > - orig_nbytes = nbytes; > + alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT); > + > data_buf = (unsigned char *)dma_alloc_coherent(ar->dev, > - orig_nbytes, > + alloc_nbytes, > &ce_data_base, > GFP_ATOMIC); > if (!data_buf) { > @@ -1098,9 +1099,6 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, > goto done; > } > > - /* Copy caller's data to allocated DMA buf */ > - memcpy(data_buf, data, orig_nbytes); > - > /* > * The address supplied by the caller is in the > * Target CPU virtual address space. > @@ -1113,12 +1111,15 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, > */ > address = ath10k_pci_targ_cpu_to_ce_addr(ar, address); > > - remaining_bytes = orig_nbytes; > + remaining_bytes = nbytes; > ce_data = ce_data_base; Because you no longer need to increment 'ce_data', you can just pass 'ce_data_base' to ath10k_ce_send_nolock() now. Regards, Brian > while (remaining_bytes) { > /* FIXME: check cast */ > nbytes = min_t(int, remaining_bytes, DIAG_TRANSFER_LIMIT); > > + /* Copy caller's data to allocated DMA buf */ > + memcpy(data_buf, data, nbytes); > + > /* Set up to receive directly into Target(!) address */ > ret = ce_diag->ops->ce_rx_post_buf(ce_diag, &address, address); > if (ret != 0) > @@ -1171,12 +1172,12 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, > > remaining_bytes -= nbytes; > address += nbytes; > - ce_data += nbytes; > + data += nbytes; > } > > done: > if (data_buf) { > - dma_free_coherent(ar->dev, orig_nbytes, data_buf, > + dma_free_coherent(ar->dev, alloc_nbytes, data_buf, > ce_data_base); > } > > -- > 2.7.4 >
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c index 873dbb6..1872671 100644 --- a/drivers/net/wireless/ath/ath10k/pci.c +++ b/drivers/net/wireless/ath/ath10k/pci.c @@ -1071,7 +1071,7 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, struct ath10k_ce *ce = ath10k_ce_priv(ar); int ret = 0; u32 *buf; - unsigned int completed_nbytes, orig_nbytes, remaining_bytes; + unsigned int completed_nbytes, alloc_nbytes, remaining_bytes; struct ath10k_ce_pipe *ce_diag; void *data_buf = NULL; u32 ce_data; /* Host buffer address in CE space */ @@ -1088,9 +1088,10 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, * 1) 4-byte alignment * 2) Buffer in DMA-able space */ - orig_nbytes = nbytes; + alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT); + data_buf = (unsigned char *)dma_alloc_coherent(ar->dev, - orig_nbytes, + alloc_nbytes, &ce_data_base, GFP_ATOMIC); if (!data_buf) { @@ -1098,9 +1099,6 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, goto done; } - /* Copy caller's data to allocated DMA buf */ - memcpy(data_buf, data, orig_nbytes); - /* * The address supplied by the caller is in the * Target CPU virtual address space. @@ -1113,12 +1111,15 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, */ address = ath10k_pci_targ_cpu_to_ce_addr(ar, address); - remaining_bytes = orig_nbytes; + remaining_bytes = nbytes; ce_data = ce_data_base; while (remaining_bytes) { /* FIXME: check cast */ nbytes = min_t(int, remaining_bytes, DIAG_TRANSFER_LIMIT); + /* Copy caller's data to allocated DMA buf */ + memcpy(data_buf, data, nbytes); + /* Set up to receive directly into Target(!) address */ ret = ce_diag->ops->ce_rx_post_buf(ce_diag, &address, address); if (ret != 0) @@ -1171,12 +1172,12 @@ int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address, remaining_bytes -= nbytes; address += nbytes; - ce_data += nbytes; + data += nbytes; } done: if (data_buf) { - dma_free_coherent(ar->dev, orig_nbytes, data_buf, + dma_free_coherent(ar->dev, alloc_nbytes, data_buf, ce_data_base); }
ath10k_pci_diag_write_mem may allocate big size of the dma memory based on the parameter nbytes. Take firmware diag download as example, the biggest size is about 500K. In some systems, the allocation is likely to fail because it can't acquire such a large contiguous dma memory. The fix is to allocate a small size dma memory. In the loop, driver copies the data to the allocated dma memory and writes to the destination until all the data is written. Tested with QCA6174 PCI with firmware-6.bin_WLAN.RM.4.4.1-00119-QCARMSWP-1, this also affects QCA9377 PCI. Signed-off-by: Carl Huang <cjhuang@codeaurora.org> --- drivers/net/wireless/ath/ath10k/pci.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)