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

Alistair Popple
2019-11-08T09:36:28Z

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

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

On Friday, 8 November 2019 4:19:45 PM AEDT Joel Stanley wrote:
> From: Andrew Jeffery <andrew@aj.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@aj.id.au>
> Signed-off-by: Joel Stanley <joel@jms.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.
>