linux-kernel - Re: [PATCH v2 11/11] fsi: aspeed: Fix OPB0 byte order register values

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]
Date:   Fri, 08 Nov 2019 20:31:12 +1100
From:   Alistair Popple <alistair@...ple.id.au>
To:     Joel Stanley <joel@....id.au>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Jeremy Kerr <jk@...abs.org>, Andrew Jeffery <andrew@...id.au>,
        Eddie James <eajames@...ux.ibm.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-fsi@...ts.ozlabs.org
Subject: Re: [PATCH v2 11/11] fsi: aspeed: Fix OPB0 byte order register values

OPB data mirroring is pretty special, glad someone has figured it out and made 
some artwork in the process.

Acked-by: Alistair Popple <alistair@...ple.id.au>

On Friday, 8 November 2019 4:19:45 PM AEDT Joel Stanley wrote:
> From: Andrew Jeffery <andrew@...id.au>
> 
> The data byte order selection registers in the APB2OPB primarily expose some
> internal plumbing necessary to get correct write accesses onto the OPB.
> OPB write cycles require "data mirroring" across the 32-bit data bus to
> support variable data width slaves that don't implement "byte enables".
> For slaves that do implement byte enables the master can signal which
> bytes on the data bus the slave should consider valid.
> 
> The data mirroring behaviour is specified by the following table:
> 
>     +-----------------+----------+-----------------------------------+
>     |                 |          |          32-bit Data Bus          |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |         |       |          |         |         |       |       |
>     |   ABus  | Mn_BE |  Request |   Dbus  |   Dbus  |  Dbus |  Dbus |
>     | (30:31) | (0:3) | Transfer |   0:7   |   8:15  | 16:23 | 24:31 |
>     |         |       |   Size   |  byte0  |  byte1  | byte2 | byte3 |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    00   |  1111 | fullword |  byte0  |  byte1  | byte2 | byte3 |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    00   |  1110 | halfword |  byte0  |  byte1  | byte2 |       |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    01   |  0111 |   byte   | _byte1_ |  byte1  | byte2 | byte3 |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    00   |  1100 | halfword |  byte0  |  byte1  |       |       |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    01   |  0110 |   byte   | _byte1_ |  byte1  | byte2 |       |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    10   |  0011 | halfword | _byte2_ | _byte3_ | byte2 | byte3 |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    00   |  1000 |   byte   |  byte0  |         |       |       |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    01   |  0100 |   byte   | _byte1_ |  byte1  |       |       |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    10   |  0010 |   byte   | _byte2_ |         | byte2 |       |
>     +---------+-------+----------+---------+---------+-------+-------+
>     |    11   |  0001 |   byte   | _byte3_ | _byte3_ |       | byte3 |
>     +---------+-------+----------+---------+---------+-------+-------+
> 
> Mirrored data values are highlighted by underscores in the Dbus columns.
> The values in the ABus and Request Transfer Size columns correspond to
> values in the field names listed in the write data order select register
> descriptions.
> 
> Similar configuration registers are exposed for reads which enables the
> secondary purpose of configuring hardware endian conversions. It appears the
> data bus byte order is switched around in hardware so set the registers such
> that we can access the correct values for all widths. The values were
> determined by experimentation on hardware against fixed CFAM register
> values to configure the read data order, then in combination with the
> table above and the register layout documentation in the AST2600
> datasheet performing write/read cycles to configure the write data order
> registers.
> 
> Signed-off-by: Andrew Jeffery <andrew@...id.au>
> Signed-off-by: Joel Stanley <joel@....id.au>
> ---
>  drivers/fsi/fsi-master-aspeed.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-
aspeed.c
> index 95e226ac78b9..f49742b310c2 100644
> --- a/drivers/fsi/fsi-master-aspeed.c
> +++ b/drivers/fsi/fsi-master-aspeed.c
> @@ -459,11 +459,11 @@ static int fsi_master_aspeed_probe(struct 
platform_device *pdev)
>  	writel(fsi_base, aspeed->base + OPB_FSI_BASE);
>  
>  	/* Set read data order */
> -	writel(0x0011bb1b, aspeed->base + OPB0_READ_ORDER1);
> +	writel(0x00030b1b, aspeed->base + OPB0_READ_ORDER1);
>  
>  	/* Set write data order */
> -	writel(0x0011bb1b, aspeed->base + OPB0_WRITE_ORDER1);
> -	writel(0xffaa5500, aspeed->base + OPB0_WRITE_ORDER2);
> +	writel(0x0011101b, aspeed->base + OPB0_WRITE_ORDER1);
> +	writel(0x0c330f3f, aspeed->base + OPB0_WRITE_ORDER2);
>  
>  	/*
>  	 * Select OPB0 for all operations.
>