On Thursday 04 February 2016 16:05:34 Zubair Lutfullah Kakakhel wrote:

Thank-you for the review.

On 04/02/16 12:24, Aleksey Makarov wrote:

quoted

Hi Zubair,

quoted

+    void __iomem *base;

[..]

quoted

+    cfg = cvmx_read_csr((uint64_t)base + CVMX_SATA_UCTL_SHIM_CFG);

sparse will complain here.  See Documentation/sparse.txt

Yes. sparse says

...
CHECK   drivers/ata/sata_octeon.c
drivers/ata/sata_octeon.c:50:30: warning: cast removes address space of expression
drivers/ata/sata_octeon.c:65:25: warning: cast removes address space of expression
...

Use of (__force uint64_t) removes the sparse warning. But it was
frowned upon by arnd.

He suggested a wrapper helper in asm/octeon/cvmx.h which handles iomem
addresses and __force behind the scenes rather than in the driver.

Obviously I was not suggesting to remove the __force without fixing
the address space mismatch first.

static inline void cvmx_write_csr_resource(void __iomem *csr_addr, uint64_t val)
{
        cvmx_write_csr((__force uint64_t)csr_addr, val)
}

Alternatives? Or should I resend with the above wrapper?

Maybe just name it cvmx_writeq()? This is close enough to the normal
writeq, except it does not serialize against DMA or spinlocks as
writeq does, and it assumes that the device is the same endianess
as the CPU, whereas writeq assumes that devices are fixed-endian.

	Arnd