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

Marek Vasut
2017-04-11T10:42:34Z

On 04/11/2017 12:18 PM, Cédric Le Goater wrote:
> 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 ?

No, I don't think so, this is a chip property which can be derived from
the compatible string, right ?

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

--
Best regards,
Marek Vasut