Re: [flashrom] [PATCH 2/5] dediprog: Tidy up commands and remove dead nonsense code.

[Stefan Tauner]

On Sat, 27 Jun 2015 17:24:43 -0700
David Hendricks  wrote:
> Mostly looks good to me, just a couple minor comments.
> > +enum dediprog_target {
> >   FLASH_TYPE_APPLICATION_FLASH_1  = 0,
> >   FLASH_TYPE_FLASH_CARD,
> >   FLASH_TYPE_APPLICATION_FLASH_2,
>
> FLASH_TYPE_SOCKET,        /* SF600 */

Thank you.
> > +enum dediprog_writemode {
> >   WRITE_MODE_PAGE_PGM                     = 1,
> >   WRITE_MODE_PAGE_WRITE                   = 2,
> >   WRITE_MODE_1B_AAI                       = 3,
> >   WRITE_MODE_2B_AAI                       = 4,
> >   WRITE_MODE_128B_PAGE                    = 5,
> >   WRITE_MODE_PAGE_AT26DF041               = 6,
> >   WRITE_MODE_SILICON_BLUE_FPGA            = 7,
> >   WRITE_MODE_64B_PAGE_NUMONYX_PCM         = 8,    /* unit of length 512 bytes */
> >   WRITE_MODE_4B_ADDR_256B_PAGE_PGM        = 9,
> >   WRITE_MODE_32B_PAGE_PGM_MXIC_512K       = 10,   /* unit of length 512 bytes */
> >   WRITE_MODE_4B_ADDR_256B_PAGE_PGM_0x12   = 11,
>
> 11 seems to only apply for the new protocol? The old protocol spec
> just says "For SF100" but I have no idea what that actually means.

I don't know... and I don't think they do either. :P Since it is present
in both specs I'll just leave it as is for now. The 0x12 opcode is
rather clear in the flash datasheets... it is just like normal page
write but with 4 mandatory bytes for the address. So it should be easy
to verify if the old firmware/protocol uses that... when we implement
those write modes eventually.

I have not seen an acked-by line in your mail... may I add it to the
respective commit?

[PATCH 2/5] dediprog: Tidy up commands and remove dead nonsense code.

[David Hendricks]

Sounds good to me.

Acked-by: David Hendricks 
On Jun 28, 2015 4:07 PM, "Stefan Tauner" 
wrote:
> On Sat, 27 Jun 2015 17:24:43 -0700
> David Hendricks  wrote:
> > Mostly looks good to me, just a couple minor comments.
> > > +enum dediprog_target {
> > >   FLASH_TYPE_APPLICATION_FLASH_1  = 0,
> > >   FLASH_TYPE_FLASH_CARD,
> > >   FLASH_TYPE_APPLICATION_FLASH_2,
> >
> > FLASH_TYPE_SOCKET,        /* SF600 */
>
> Thank you.
> > > +enum dediprog_writemode {
> > >   WRITE_MODE_PAGE_PGM                     = 1,
> > >   WRITE_MODE_PAGE_WRITE                   = 2,
> > >   WRITE_MODE_1B_AAI                       = 3,
> > >   WRITE_MODE_2B_AAI                       = 4,
> > >   WRITE_MODE_128B_PAGE                    = 5,
> > >   WRITE_MODE_PAGE_AT26DF041               = 6,
> > >   WRITE_MODE_SILICON_BLUE_FPGA            = 7,
> > >   WRITE_MODE_64B_PAGE_NUMONYX_PCM         = 8,    /* unit of
>
> length 512 bytes */
> > >   WRITE_MODE_4B_ADDR_256B_PAGE_PGM        = 9,
> > >   WRITE_MODE_32B_PAGE_PGM_MXIC_512K       = 10,   /* unit of
>
> length 512 bytes */
> > >   WRITE_MODE_4B_ADDR_256B_PAGE_PGM_0x12   = 11,
> >
> > 11 seems to only apply for the new protocol? The old protocol spec
> > just says "For SF100" but I have no idea what that actually means.
>
> I don't know... and I don't think they do either. :P Since it is present
> in both specs I'll just leave it as is for now. The 0x12 opcode is
> rather clear in the flash datasheets... it is just like normal page
> write but with 4 mandatory bytes for the address. So it should be easy
> to verify if the old firmware/protocol uses that... when we implement
> those write modes eventually.
>
> I have not seen an acked-by line in your mail... may I add it to the
> respective commit?