spi: tegra: avoid unsigned->signed->unsigned promotion

From: Michal Nazarewicz <mina86@mina86.com>

u8 type, which is unsigned, is promoted to int, which is singned, when
doing a binary shift.  Then, on 64-bit machines, it is further promoted
to unsigned long which may lead to more significant half of the value
to be all ones.  To avoid this, explicitly promote to an unsigned type.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/spi/spi-tegra114.c      | 4 ++--
 drivers/spi/spi-tegra20-slink.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

On Fri, Nov 29, 2013 at 06:12:44PM +0100, Michal Nazarewicz wrote:
> From: Michal Nazarewicz <mina86@mina86.com>
> 
> u8 type, which is unsigned, is promoted to int, which is singned, when
> doing a binary shift.  Then, on 64-bit machines, it is further promoted
> to unsigned long which may lead to more significant half of the value
> to be all ones.  To avoid this, explicitly promote to an unsigned type.

So this would only become an problem on 64-bit Tegra SoCs, right? I
suspect that there are quite a few places where something similar is
done and which suffer from the same issue.

But I don't think there's a real issue here. Even on 64-bit platforms,
writel() will only write 32-bit registers, and therefore discard the
upper 32 bits.

I think in general perhaps a more proper fix would be to use only u32
instead of unsigned long for these cases, since then the problem goes
away. u32 is also the type of the value used by writel() so it makes
perfect sense to use it.

Thierry

> On Fri, Nov 29, 2013 at 06:12:44PM +0100, Michal Nazarewicz wrote:
>> u8 type, which is unsigned, is promoted to int, which is singned, when
>> doing a binary shift.  Then, on 64-bit machines, it is further promoted
>> to unsigned long which may lead to more significant half of the value
>> to be all ones.  To avoid this, explicitly promote to an unsigned type.

On Fri, Nov 29 2013, Thierry Reding wrote:
> But I don't think there's a real issue here. Even on 64-bit platforms,
> writel() will only write 32-bit registers, and therefore discard the
> upper 32 bits.
>
> I think in general perhaps a more proper fix would be to use only u32
> instead of unsigned long for these cases, since then the problem goes
> away. u32 is also the type of the value used by writel() so it makes
> perfect sense to use it.

Yes, you're absolutely right.  I dunno why I missed the fact it's used
in writel only.  I'll prepare a new patch soon(ish).

This series has only been compile-tested, but it does not introduce any
behaviour changes.

Michal Nazarewicz (3):
  spi: tegra114: use u32 for 32-bit register values
  spi: tegra20-slink: use u32 for 32-bit register values
  spi: tegra20-sflash: use u32 for 32-bit register values

 drivers/spi/spi-tegra114.c       | 98 ++++++++++++++--------------------------
 drivers/spi/spi-tegra20-sflash.c | 22 ++++-----
 drivers/spi/spi-tegra20-slink.c  | 97 ++++++++++++++++-----------------------
 3 files changed, 84 insertions(+), 133 deletions(-)

On 12/08/2013 08:35 AM, Michal Nazarewicz wrote:
> This series has only been compile-tested, but it does not introduce any
> behaviour changes.

The series,
Tested-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Stephen Warren <swarren@nvidia.com>

... although I will observe that the patch subjects, and even patch
descriptions, don't describe all the other unrelated code
transformations that are rolled into these patches...
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Dec 08, 2013 at 04:35:08PM +0100, Michal Nazarewicz wrote:
> This series has only been compile-tested, but it does not introduce any
> behaviour changes.

Applied, thanks.  Like Stephen says please do make sure your commits do
as they're described...