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

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Fri, 23 Feb 2018 11:43:21 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> 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.

Sorry, I don't understand, probably because I lack some background on
CFI. I'll try to document myself if I find some time.

> 
> >> + */
> >> +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.

Net-style comment:

/* blablabla
 * blablabla
 */

Kernel-style comment:

/*
 * blablabla
 * blablabla
 */

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