[PATCH v2 3/3] ath10k: make target endianess more explicit

[Michal Kazior]

On 27 August 2014 09:28, Kalle Valo <kvalo at qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior at 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.kazior at 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.

Beats me. I plan to remove this in the future unless I find it's
actually necessary to be kept.


Michał