[U-Boot] [PATCH v2] fix CFI flash driver for 8-bit bus support

Luka Perkov
2012-02-14T00:51:28Z

Hi Wolfgang,

thank you for your pointers. Like I have said I'm not the one that
submited v1 and in v2 I only changed what was pointed after v1.

On Mon, Feb 13, 2012 at 11:32:30AM +0100, Wolfgang Denk wrote:
> > - udelay(1);
>
> This is an unrelated change. It has no place in this patch.

Yes, you are right. I'll leave this alone for now.

> Please submit separately, with respective comments for why uyou are
> doing this, if you really want to change this.
>
> > static flash_sect_t find_sector (flash_info_t * info, ulong addr)
> > {
> > static flash_sect_t saved_sector = 0; /* previously found sector */
> > - static flash_info_t *saved_info = 0; /* previously used flash bank */
> > flash_sect_t sector = saved_sector;
> >
> > - if ((info != saved_info) || (sector >= info->sector_count))
> > - sector = 0;
>
> I think this is bogus. Please clean up your patch!

Yes, this probably is not needed.

> > - p[i] = flash_read_uchar(info, start + i);
> > + p[i] = flash_read_uchar(info, start + (i * 2));
>
> On which systems / in which configurations has this code been tested?

Answer at the end of this mail.

> > - /* Issue FLASH reset command */
> > - flash_cmd_reset(info);
>
> really?
>
> > for (cfi_offset=0;
> > cfi_offset < sizeof(flash_offset_cfi) / sizeof(uint);
> > cfi_offset++) {
> > + /* Issue FLASH reset command */
> > + flash_cmd_reset(info);
>
> really??

Nobody commented this in v1. I'll test without this change.

> > if (flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP, 'Q')
> > - && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 1, 'R')
> > - && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 2, 'Y')) {
> > + && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 2, 'R')
> > + && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 4, 'Y')) {
> > flash_read_cfi(info, qry, FLASH_OFFSET_CFI_RESP,
> > sizeof(struct cfi_qry));

...

> Which configurations have been actually tested?

DLink DNS323, board that I'm working on (orion5x, arm926ejs). Also it
was tested in v1:

http://lists.denx.de/pipermail/u-boot/2011-April/089611.html

I will test this also on a MIPS board that I have in the next few days.
I did not have time to do it today.

I'll resend v3 after more testing and code reading.

Regards,
Luka