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

[Marek Vasut <marek.vasut@gmail.com>]

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