[PATCH 10/10] mtd: spi-nor: aspeed: optimize read mode

[Marek Vasut]

On 04/11/2017 10:13 AM, Cédric Le Goater wrote:
> Hello Marek,
> 
> On 04/06/2017 09:28 PM, Marek Vasut wrote:
>> On 04/06/2017 06:56 PM, Cédric Le Goater wrote:
>>> This is only for SPI controllers as U-Boot should have done it already
>>> for the FMC controller using DMAs.
>>>
>>> The algo is based on the one found in the OpenPOWER pflash tool. It
>>> first reads a golden buffer at low speed and then performs reads with
>>> different clocks and delay cycles settings to find the fastest
>>> configuration for the chip.
>>>
>>> It can be deactivated at boot time with the kernel parameter :
>>
>> Something tells me this is likely gonna be pretty flaky and might lead
>> to unreliable configurations. The hardware manufacturer should be able
>> to determine the limits of the hardware and those should be put into DT
>> at manufacturing time IMO.
> 
> It is not a common method but we have been using it on many OpenPOWER
> platforms (P8) using the AST2400 (palmetto, habanero, firestone, 
> garrison, etc) and also on the newer P9 platforms using the AST2500
> (romulus, zaius, witherspoon). So I would say that experience 'proves' 
> that it works. We didn't invent the SPI Timing Calibration algo, it is
> described in the specs of the manufacturer.
> 
> Also, all these platforms use sockets to hold the different flash 
> modules of the system. We don't know in advance what we will find 
> and so it makes difficult to put any timing in the DT.    
> 
> And the values of the "data input delay cycle" (REG94) depend on the 
> chip model and on the length of the wires of the board. If we were to 
> hard-code such values in the DT, we would need to put only safe and slow 
> settings known to work in all configurations. which is a bit what we 
> do in the current driver using really slow ones.

Aha, thanks for clarifying.

> Here is a proposal. We could activate this algo using a property such
> as :
>  
> 	"speed-mode" = "freq" or "auto-adjust"
> 
> How's that ? 

Thinking about it a bit more, I think having a module parameter is the
right approach here ... or maybe compile-time switch. This shouldn't be
in DT as it's not HW property.

> Thanks,
> 
> C.
> 


-- 
Best regards,
Marek Vasut