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.
--
Kalle Valo
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
