Re: [PATCH v1 3/3] drivers: mtd: chips: add support for the dual die stacked PNOR

[Bean Huo <beanhuo@outlook.com>]

Boris,
>>
>> +/*
>> + * The dual die stacked device comprises two identical dies which connected
>                                                                   ^are
fixed in next patch.

>> + * in parallel.But there is only one die being selected each time according

   >               ^ missing space after the period
fixed in next patch.

>> + * to maximum address line A[max]. When A[max] == 0, the lower die is selected,
>> + * when A[max] == 1, the upper die is selected. This function will reture the
>
 >                                                               ^return
>> + * CFI unlock-command base address according to accessing address.
>Is it really just about unlock commands?

Except unlock commands, other commands also need but don't need to add
0x555/0x2AA. And current parallel nor driver already does this way.
so we don't need to change.

>> + */
>> +static loff_t get_cmd_base_address(struct map_info *map, struct flchip *chip,
>> +             loff_t offset)
>> +{
>> +     struct cfi_private *cfi = map->fldrv_priv;
>>
>>+     unsigned long cmd_base_addr = chip->start;
>>+
>> +     if (cfi->device_stack == CFI_DEVICESTACK_2DIE) {
>>+             if (offset >= (1 << (cfi->cfiq->DevSize - 1)))
>>+                     cmd_base_addr += (1 << (cfi->cfiq->DevSize - 1));
>> +     }

>Could be done in a more generic way:

 >    unsigned long die = offset >> (cfi->cfiq->DevSize - 1);

>        if (cfi->device_stack == 1) {
 >               WARN_ON(die);
  >              return cmd_base_addr;
  >      }

   >     cmd_base_addr += die << (cfi->cfiq->DevSize - 1);

it will be included in next patch, thanks.

...
>> -     cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
>> +     /* For the dual die device, rebase the command base address according

>Please do not use net-style comments.

To be honest, I don't know what is net-style comments.

>--
>Boris Brezillon, Bootlin (formerly Free Electrons)
>Embedded Linux and Kernel engineering
>https://bootlin.com