linux-kernel - Re: [PATCH 1/3] spi: Add a spi

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: <5245E420.2030207@metafoo.de>
Date:	Fri, 27 Sep 2013 22:01:36 +0200
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Mark Brown <broonie@...nel.org>
CC:	Jean Delvare <khali@...ux-fr.org>,
	Guenter Roeck <linux@...ck-us.net>,
	Jonathan Cameron <jic23@...nel.org>, lm-sensors@...sensors.org,
	linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
	linux-spi@...r.kernel.org, viresh.linux@...il.com
Subject: Re: [PATCH 1/3] spi: Add a spi_w8r16be() helper

On 09/27/2013 09:22 PM, Mark Brown wrote:
> On Fri, Sep 27, 2013 at 08:46:56PM +0200, Lars-Peter Clausen wrote:
> 
>> According to the documentation of spi_w8r16() it is a feature.
> 
>> 	* The number is returned in wire-order, which is at least sometimes
>>  	* big-endian.
> 
> Indeed.  I don't think that's terribly well thought through though,
> especially not now we have annotations for endianness (as you noticed!).

I wouldn't mind updating spi_w8r16() to do the conversion to big-endian.
Especially considering that a driver using the function will probably always
need to do a endian conversion anyway to work correctly on both
endiannesses. We can add a LE variant if we should ever need it.

> 
>> There seem to be at least two users though which assume that the result is
>> in native endianness drivers/hwmon/ads7871.c and drivers/mfd/stmpe-spi.c
> 
> Yeah, I saw.  The ads7871 is just going to break when run on the
> opposite endianness to the one it was (hopefully) tested on since it
> doesn't make any effort I saw to cope with endianness.  Looking at the
> history it's not terribly obvious which that was but it'd be surprising
> to see a little endian register...
> 
> STMPE is doing a byte swap so it's another user for your function -
> it's for ARM systems so it'll be assuming that little endian is the
> native format.

The STMPE driver might be a candidate for regmap. Considering that the
driver was tested on a LE system it looks as if the driver first writes a 8
bit word for the address then reads a 16 bit word and ignores the upper bits
(of the BE word) and returns the lower 8 bits as the result. But the way I
understand the datasheet the 8-bit data word would follow right after the
8-bit address has been written, so I'm not sure what's going on there.
Viresh maybe you can shed some light on this. Btw. the register write
function of the STMPE driver also seems to be not endian safe.

- Lars

--
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/