[PATCH dev-5.0 3/4] mtd: spi-nor: aspeed: use memcpy_fromio() to capture the optimization buffer
Cédric Le Goater
clg at kaod.org
Fri Apr 19 16:02:22 AEST 2019
On 4/19/19 3:03 AM, Andrew Jeffery wrote:
>
>
> On Fri, 19 Apr 2019, at 06:53, Milton Miller II wrote:
>> About 04/17/2019 09:20AM in some timezone, Cédric Le Goater wrote:
>>
>>
>>> aspeed_smc_read_from_ahb() only reads the first word which is not
>>> what
>>> we want. We want to capture a CALIBRATE_BUF_SIZE size window of the
>>> flash contents to optimize the read.
>>>
>>
>> NACK
>>
>> This justifcation is false. The routine reads the whole buffer
>> because it calls the _rep routine and takes the size.
It doesn't AFAICT looking at other drivers and also from my test.
>> In addition, the comment just before aspeed_smc_read_from_ahb
>> tells why memcpy_fromio and memcpy_toio are broken on 32 bit
That might have been the case on Linux 4.7. It doesn't seem to be
the case anymore. See below.
>> arm, and this is still the case judging from the recent bug
>> reportfrom a Nuvaton user [1].
>>
>> [1] https://github.com/openbmc/openbmc/issues/3521
>>
>> Andrew, Please revert this patch.
I don't think you have enough convincing arguments for that.
> Yeah, I had a briefly nagging thought about this when I applied it. I
> should have gone with that gut instinct and inspected it in more
> depth.
>
> Thanks for calling it out.
I think this needs more thinking and tests. Witherspoon and palmetto
are fine. What's the problem.
> Reverting for dev-5.0, this is an isolated issue (arguably should have
> been separated from the current series).
We have been using memcpy_fromio for reads for a very long time
since 4.13 in the Aspeed SMC driver on *all* platforms*.
See aspeed_smc_read() for the "Command mode".
C.
>
> Andrew
>
>>
>>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>>> ---
>>> drivers/mtd/spi-nor/aspeed-smc.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c
>>> b/drivers/mtd/spi-nor/aspeed-smc.c
>>> index 1437732fdea1..7e289ecb1c99 100644
>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>> @@ -796,8 +796,7 @@ static bool aspeed_smc_check_reads(struct
>>> aspeed_smc_chip *chip,
>>> int i;
>>>
>>> for (i = 0; i < 10; i++) {
>>> - aspeed_smc_read_from_ahb(test_buf, chip->ahb_base,
>>> - CALIBRATE_BUF_SIZE);
>>> + memcpy_fromio(test_buf, chip->ahb_base, CALIBRATE_BUF_SIZE);
>>> if (memcmp(test_buf, golden_buf, CALIBRATE_BUF_SIZE) != 0)
>>> return false;
>>> }
>>> @@ -921,8 +920,7 @@ static int aspeed_smc_optimize_read(struct
>>> aspeed_smc_chip *chip,
>>>
>>> writel(chip->ctl_val[smc_read], chip->ctl);
>>>
>>> - aspeed_smc_read_from_ahb(golden_buf, chip->ahb_base,
>>> - CALIBRATE_BUF_SIZE);
>>> + memcpy_fromio(golden_buf, chip->ahb_base, CALIBRATE_BUF_SIZE);
>>>
>>> /* Establish our read mode with freq field set to 0 (HCLK/16) */
>>> chip->ctl_val[smc_read] = save_read_val & 0xfffff0ff;
>>> --
>>> 2.20.1
>>>
>>>
>>
>>
More information about the openbmc
mailing list