[PATCH linux dev-4.7 1/8] mtd: spi-nor: aspeed: fix setting of the register control value for writes

[Cédric Le Goater]

On 11/07/2016 03:20 AM, Joel Stanley wrote:
> On Sat, Nov 5, 2016 at 3:30 AM, Cédric Le Goater <clg at kaod.org> wrote:
>> The setting of the register control value for writes depends on the
>> base value which was defined only if the mask applied to it changed
>> the register control value.
>>
>> Fix that.
>>
>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>> ---
>>  drivers/mtd/spi-nor/aspeed-smc.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>> index f65379bb2287..75ba73ef0660 100644
>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>> @@ -756,7 +756,7 @@ static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
>>  {
>>         struct aspeed_smc_controller *controller = chip->controller;
>>         const struct aspeed_smc_info *info = controller->info;
>> -       u32 reg;
>> +       u32 reg, base_reg;
>>
>>         /*
>>          * Always turn on the write enable bit to allow opcodes to be
>> @@ -789,12 +789,13 @@ static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
>>         reg = readl(chip->ctl);
>>         dev_dbg(controller->dev, "control register: %08x\n", reg);
>>
>> -       if ((reg & CONTROL_SPI_KEEP_MASK) != reg) {
>> -               chip->ctl_val[smc_base] = reg & CONTROL_SPI_KEEP_MASK;
>> +       base_reg = reg & CONTROL_SPI_KEEP_MASK;
>> +       if (base_reg != reg) {
>>                 dev_info(controller->dev,
>>                          "control register changed to: %08x\n",
> 
> Is this the kind of message we would see if someone else (the host, or
> pflash) was changing the register while the kernel driver was loaded?
 
It's more to "warn" that the kernel is changing a value that was 
calculated by hw (strapping or other) or by uboot which does timing 
calibration using DMAs.

> If so, we might want to make this a dev_warn or even a WARN in order
> make it really clear that a bad thing has happened.

but yes, a warn might be better in this case.

Thanks,

C. 


> Cheers,
> 
> Joel
> 
>> -                        chip->ctl_val[smc_base]);
>> +                        base_reg);
>>         }
>> +       chip->ctl_val[smc_base] = base_reg;
>>
>>         /*
>>          * Retain the prior value of the control register as the
>> --
>> 2.7.4
>>