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

Reply via email to