linux-kernel - Re: xsysace driver support on arches other than PPC/Microblaze

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] [day] [month] [year] [list]

Message-Id: <201306191651.37568.arnd@arndb.de>
Date:	Wed, 19 Jun 2013 16:51:37 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Alexey Brodkin <Alexey.Brodkin@...opsys.com>
Cc:	Andy Shevchenko <andy.shevchenko@...il.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Michal Simek <monstr@...str.eu>,
	Grant Likely <grant.likely@...retlab.ca>,
	"Benjamin Herrenschmidt" <benh@...nel.crashing.org>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	"Geert Uytterhoeven" <geert@...ux-m68k.org>,
	"dahinds@...rs.sourceforge.net" <dahinds@...rs.sourceforge.net>,
	Mischa Jonker <Mischa.Jonker@...opsys.com>
Subject: Re: xsysace driver support on arches other than PPC/Microblaze

On Wednesday 19 June 2013, Alexey Brodkin wrote:
> ============
> static void ace_out_be16(struct ace_device *ace, int reg, u16 val)
> {
>         out_be16(ace->baseaddr + reg, val);
> }
> 
> static void ace_dataout_be16(struct ace_device *ace)
> {
>         int i = ACE_FIFO_SIZE / 2;
>         u16 *src = ace->data_ptr;
>         while (i--)
>                 out_le16(ace->baseaddr + 0x40, *src++);
>         ace->data_ptr = src;
> }
> ============
> 
>  From it you may see that one high-level big-endian accessor 
> ("ace_out_be16") uses big-endian low-level accessor ("out_be16") while 
> another high-level big-endian accessor ("ace_dataout_be16") uses 
> little-endian low-level accessor ("out_be16").
> 
> It seems like access to 16-bit data words should be done always with LE 
> accessors (after all it's always just a window to a device's memory).

This is a very long story, but I think the code is right. The point is
that ace_out_be16 accesses a single register that is defined as big-endian,
while ace_dataout_be16 accesses a byte stream in 16-bit units but has to
copy each byte in the same order that they are stored in memory on the
CPU. It usually takes me half an hour to wrap my head around this,
but yes it is this crazy and a lot of drivers do the same thing.

To simplify the above, ace_dataout_be16 should just call ioread16_rep()
which does the loop in the right endianess, while ace_out_be16
could be changed to use iowrite16_be(), which is the similar to
out_be16 but more portable.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/