linux-kernel - Re: [PATCH v7] SATA: OCTEON: support SATA on OCTEON platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4427989.vm6oM7UICv@wuerfel>
Date:	Fri, 05 Feb 2016 16:10:40 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>
Cc:	Aleksey Makarov <aleksey.makarov@...iumnetworks.com>,
	tj@...nel.org, hdegoede@...hat.com, david.daney@...ium.com,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-ide@...r.kernel.org
Subject: Re: [PATCH v7] SATA: OCTEON: support SATA on OCTEON platform

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:
> >
> > Hi Zubair,
> >
> >> +    void __iomem *base;
> >
> > [..]
> >
> >> +    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