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.palt...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
