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

Cédric Le Goater
2017-04-11T16:10:43Z

On 04/11/2017 12:42 PM, Marek Vasut wrote:
> 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 ?

yes.

C.

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