[U-Boot] [PATCH v4 1/2] SPI Flash: add support of sst26wf* flash ICs protection ops

[Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>]

Hi Jagan,

On Mon, 2018-04-09 at 16:52 +0530, Jagan Teki wrote:
> On Mon, Apr 9, 2018 at 4:27 PM, Eugeniy Paltsev
> <Eugeniy.Paltsev@synopsys.com> wrote:
> > sst26wf flash series block protection implementation differs
> > from other SST series, so add specific implementation
> > flash_lock/flash_unlock/flash_is_locked functions for sst26wf
> > flash ICs.
> > 
> > +enum flash_lock_status {
> > +       SF_UNLOCKED             = 0,
> > +       SF_LOCKED               = 1,
> > +};
> 
> Try to use existing relevant error codes.

Hmm, I can return something like EACCES (positive value) if flash is locked. Is it OK?

> > +
> >  #define SPI_FLASH_3B_ADDR_LEN          3
> > +enum lock_ctl {
> > +       CTL_LOCK,
> > +       CTL_UNLOCK,
> > +       CTL_CHECK
> > +};
> 
> SST26_CTL_* ?
OK.

> > 
> > +#if defined(CONFIG_SPI_FLASH_SST)
> 
> Please move this code inside of existing #ifdef of SST (commented same
> in previous version)
OK.

[snip]
> > 
> >  #ifdef CONFIG_SPI_FLASH_MACRONIX
> >  static int macronix_quad_enable(struct spi_flash *flash)
> > @@ -1033,6 +1189,15 @@ int spi_flash_scan(struct spi_flash *flash)
> >         }
> >  #endif
> > 
> > +/* sst26wf series block protection implementation differs from other series */
> > +#if defined(CONFIG_SPI_FLASH_SST)
> > +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST && info->id[1] == 0x26) {
> > +               flash->flash_lock = sst26_lock;
> > +               flash->flash_unlock = sst26_unlock;
> > +               flash->flash_is_locked = sst26_is_locked;
> > +       }
> > +#endif
> 
> Previous version comment need to fix here? (about switch case with
> existing lock ops)?

I guess switch case isn't suitable here:
we need check several parameters (JEDEC_MFR and Device Type) and we need to check
CONFIG_SPI_FLASH_*** ifdefs.

Also we have only two lock ops: "stm_*" and "sst26_*" (which I add by this patch).

So I guess using switch here is unnecessarily.

> Jagan.
-- 
 Eugeniy Paltsev