On 27 August 2014 09:28, Kalle Valo <[email protected]> wrote: > Michal Kazior <[email protected]> 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 <[email protected]> > > [...] > >> /* 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.
Beats me. I plan to remove this in the future unless I find it's actually necessary to be kept. Michał _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
