[PATCH dev-5.0 1/4] mtd: spi-nor: aspeed: introduce a aspeed_smc_default_read() helper

[Cédric Le Goater]

On 4/18/19 8:07 AM, Andrew Jeffery wrote:
> 
> 
> On Thu, 18 Apr 2019, at 03:40, Alexander Amelkin wrote:
>> 17.04.2019 16:39, Cédric Le Goater wrote:
>>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>>> ---
>>>  drivers/mtd/spi-nor/aspeed-smc.c | 19 ++++++++++++-------
>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>>> index ddf7ae78aa0a..ee3059b27c07 100644
>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>> @@ -882,6 +882,17 @@ static const uint32_t aspeed_smc_hclk_divs[] = {
>>>  };
>>>  #define ASPEED_SMC_HCLK_DIV(i) (aspeed_smc_hclk_divs[(i) - 1] << 8)
>>>  
>>> +static u32 aspeed_smc_default_read(struct aspeed_smc_chip *chip)
>>> +{
>>> +	return (chip->ctl_val[smc_read] & 0x2000) |
>>
>> Cédric, isn't this a good time to get rid of these cached ctl_val values?
>>
>> Why not read/modify/store the actual register?
>>
>> What's the profit of this caching?

These values are computed when the controller is probed and when 
the chip devices are scanned (which takes a couple of seconds to 
do the training). They are the default values used when the flash 
contents is accessed through the flash MMIO window. 

When in User mode, for register operation, erase, write, other 
values are used, see read/write_reg operations, and we don't want 
to recompute the same values, it takes time and the HW has not 
changed anyhow.

It is also safer as some FW still hack the value in the back of 
the Linux driver so we should not trust how the HW has been 
configured. I believe HIOMAP is available for P8 also now ?

The state of chips is another problem. If the 4B enablement in 
the chip was dropped and if the controller is still configured 
to use 4B, then that's a problem. I agree. Using 4B opcodes only 
should help in the future.  

> Reasonable question, but I've merged the change as-is to dev-5.0 on the
> basis that it helps fix read corruption on AC-cycles.
> 
> We can address this in a follow-up patch if necessary.

spi-mem should be taken into account now, which requires a large 
rewrite of the driver. 

Cheers,

C.

> 
> Cheers,
> 
> Andrew
> 
>>
>> With best regards,
>> Alexander Amelkin,
>> Leading BMC Software Engineer, YADRO
>> https://yadro.com
>>
>>
>>
>> Attachments:
>> * signature.asc