Michal Kazior <michal.kaz...@tieto.com> writes: > Some copy engine structures are target specific > and are uploaded to the device during > init/configuration. > > This also cleans up a bit diag_mem_read/write > implicit byteswap mess leaving only > diag_access_read/write with an implicit endianess > byteswap. > > Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
[...] > /* Write 4B data to Target memory or register */ > static int ath10k_pci_diag_write_access(struct ath10k *ar, u32 address, > u32 data) > { > /* Assume range doesn't cross this boundary */ > if (address >= DRAM_BASE_ADDRESS) > - return ath10k_pci_diag_write_mem(ar, address, &data, > - sizeof(u32)); > + return ath10k_pci_diag_write32(ar, address, data); Nothing wrong with your patch, but I really despise functions with split personality like this one. The caller should know which area it's writing to. And we have similar stuff in ath10k_pci_diag_read_mem() as well: /* * This code cannot handle reads to non-memory space. Redirect to the * register read fn but preserve the multi word read capability of * this fn */ if (address < DRAM_BASE_ADDRESS) { if (!IS_ALIGNED(address, 4) || !IS_ALIGNED((unsigned long)data, 4)) return -EIO; while ((nbytes >= 4) && ((ret = ath10k_pci_diag_read_access( ar, address, (u32 *)data)) == 0)) { nbytes -= sizeof(u32); address += sizeof(u32); data += sizeof(u32); } return ret; } Can you guess what's the idea behind this? I would prefer that we get rid of all the ugly _access() functions in pci.c. -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k