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

Submitted by Cédric Le Goater on April 6, 2017, 4:56 p.m.

Details

Message ID 1491497808-25487-2-git-send-email-clg@kaod.org
State New
Delegated to: Cyrille Pitchen
Headers show

Commit Message

Cédric Le Goater April 6, 2017, 4:56 p.m.
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(-)

Comments

Marek Vasut April 6, 2017, 7:17 p.m.
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 ...

>  	.nce = 5,
>  	.hastype = true,
>  	.we0 = 16,
>
Cédric Le Goater April 11, 2017, 10:18 a.m.
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.
Marek Vasut April 11, 2017, 10:42 a.m.
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. 
>
Cédric Le Goater April 11, 2017, 4:10 p.m.
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. 
>>
> 
>

Patch hide | download patch | download mbox

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,
 	.nce = 5,
 	.hastype = true,
 	.we0 = 16,