Re: [PATCH 01/10] mtd: spi-nor: aspeed: fix the AST2400 SMC window size

Cédric Le Goater
2017-04-11T10:18:52Z

On 04/06/2017 09:17 PM, Marek Vasut wrote:
> On 04/06/2017 06:56 PM, Cédric Le Goater wrote:
>> The window of the Aspeed AST2400 SMC Controller to map chips on the
>> AHB Bus has a 256MB size. 64MB is the default size for the first chip
>> select and not the full window range: [ 0x20000000 - 0x2FFFFFFF ].
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> drivers/mtd/spi-nor/aspeed-smc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>> index 56051d30f000..aa2c647c8507 100644
>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>> @@ -48,7 +48,7 @@ static void aspeed_smc_chip_set_4b_spi_2400(struct aspeed_smc_chip *chip);
>> static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip);
>>
>> static const struct aspeed_smc_info fmc_2400_info = {
>> - .maxsize = 64 * 1024 * 1024,
>> + .maxsize = 256 * 1024 * 1024,
>
> How does the second part of reg property in DT relate to this ?
> DT binding example has it set to 0x02000000 = 32 MiB ...

Thanks for pointing this out. There is an inconsistency and
a redundancy here ... The patch is wrong as I am mixing up
different limits. Anyhow, there are some concerns to discuss.

The DT binding is the size of the overall AHB window in which
a controller can directly memory access the chips bound to
itself. This is for the Command mode.

The .maxsize attribute, not being used until this patchset,
holds the size limit of a chip that the controller supports
to do direct memory access. This limit is 64MB on the AST2400
and 256MB (full window) on the AST2500.

I think that this value should be moved in the DT also ?

The second thing to do is to retrieve the size of the overall
AHB window from the DT and stop using the .maxsize attribute
in the subsequent patches configuring the segment registers
and introducing the command mode.

But the SPI controller window on the AST2400 is :

[ 0x30000000 - 0x3FFFFFFF ]

which is a problem because, if I remember well, it requires us
to configure CONFIG_VMSPLIT_2G to reserve space for the iomap.
I am not sure how to correctly handle that case, today we are
just defining a small 32MB window.

Thanks,

C.