On Fri, Sep 28, 2007 at 03:35:54PM +0200, Carl-Daniel Hailfinger wrote:
> On 27.09.2007 16:30, Ward Vandewege wrote:
> > On Thu, Sep 27, 2007 at 11:31:14AM +0200, Carl-Daniel Hailfinger wrote:
> >> No problem. Can you commit with your changes? Changelog follows:
> >>
> >> Add preliminary SPI flash identification support for SPI chips attached
> >> to ITE IT8716F Super I/O. Right now this is hardcoded to the Gigabyte
> >> M57SLI board. It works only with rev 2.0 of the board, but it will bail
> >> out on earlier versions, so no damage can occur.
> >>
> >> Signed-off-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]>
> > 
> > Acked-by: Ward Vandewege <[EMAIL PROTECTED]>
> > 
> > and committed in r2811
> 
> Thanks for committing!
> 
> Here is a patch against current svn which aims to restructure SPI flash
> support in a more reasonable way. Please note that the patch is not
> ready to be committed, but it should work much better than the existing

Yep, Signed-off-by missing for this one (yeah, there's one for the older
version, but still)...


> one, namely without the dirty "do everything in board_enable functions"
> hack. Functions still have to be moved to another file, but for initial
> testing, this should be fine.
> 
> Regards,
> Carl-Daniel
> 
> Index: util/flashrom/flash.h
> ===================================================================
> --- util/flashrom/flash.h     (Revision 2811)
> +++ util/flashrom/flash.h     (Arbeitskopie)
> @@ -68,8 +68,31 @@
>  #define AT_29C040A           0xA4
>  #define AT_29C020            0xDA
>  
> +#define EON_ID                       0x1C
> +/* EN25 chips are SPI, first byte of device id is memory type,
> +   second byte of device id is log(bitsize)-9 */
> +#define EN_25B05             0x2010  /* 2^19 kbit or 2^16 kByte */
> +#define EN_25B10             0x2011
> +#define EN_25B20             0x2012
> +#define EN_25B40             0x2013
> +#define EN_25B80             0x2014
> +#define EN_25B16             0x2015
> +#define EN_25B32             0x2016
> +
>  #define MX_ID                        0xC2    /* Macronix (MX) */
>  #define MX_29F002            0xB0
> +/* MX25L chips are SPI, first byte of device id is memory type,
> +   second byte of device id is log(bitsize)-9 */
> +#define MX_25L512            0x2010  /* 2^19 kbit or 2^16 kByte */
> +#define MX_25L1005           0x2011
> +#define MX_25L2005           0x2012
> +#define MX_25L4005           0x2013  /* MX25L4005{,A} */
> +#define MX_25L8005           0x2014
> +#define MX_25L1605           0x2015  /* MX25L1605{,A,D} */
> +#define MX_25L3205           0x2016  /* MX25L3205{,A} */
> +#define MX_25L6405           0x2017  /* MX25L3205{,D} */
> +#define MX_25L1635D          0x2415
> +#define MX_25L3235D          0x2416

Maybe we should separate out the IDs for SPI chips in a different
section of the header file (to not confuse people into assuing they're
LPC parts or so)?

  
>  #define SHARP_ID             0xB0    /* Sharp */
>  #define SHARP_LHF00L04               0xCF
> @@ -182,6 +205,8 @@
>  int linuxbios_init(void);
>  extern char *lb_part, *lb_vendor;
>  
> +int probe_spi(struct flashchip *flash);

Can be
  int probe_spi(const struct flashchip *flash);
I think.


> Index: util/flashrom/board_enable.c
> ===================================================================
> --- util/flashrom/board_enable.c      (Revision 2811)
> +++ util/flashrom/board_enable.c      (Arbeitskopie)
> @@ -96,35 +96,100 @@
>       return flashport;
>  }
>  
> -static void it8716_serial_rdid(uint16_t port)
> +/* The IT8716 only supports commands with length 1,2,4,5 bytes including

IT8716F please, as per datasheet. AFAICS all ITEs should have the F at the
end of the name. Applies to several places in the code and comments.


> +   command byte and can not read more than 3 bytes from the device.
> +   This function expects writearr[0] to be the first byte sent to the device,
> +   whereas the IT8716 splits commands internally into address and non-address
> +   commands with the address in inverse wire order. That's why the register
> +   ordering in case 4 and 5 may seem strange. */
> +static int it8716_spi_command(uint16_t port, unsigned char writecnt, 
> unsigned char readcnt, const unsigned char *writearr, unsigned char *readarr)

If the size of writecnt/readcnt/writearr/readarr etc. is fixed/specified in
the datasheet then please make it uint16_t etc. instead of the "generic"
unsigned char and friends.


> +static int it8716_serial_rdid(uint16_t port, unsigned char *readarr)
> +{
> +     /* RDID is 0x9f */
> +     const unsigned char cmd[] = {0x9f};

#define RDID 0x9f

at the top of the file?


> +
> +     if (it8716_spi_command(port, 1, 3, cmd, readarr))
> +             return 1;
> +     printf("RDID returned %02x %02x %02x\n", readarr[0], readarr[1], 
> readarr[2]);
> +     return 0;
> +}
> +
> +static uint16_t it8716_flashport = 0;

Move this to the top of the file, please.


>  static int it87xx_probe_serial_flash(const char *name)
>  {
> -     uint16_t flashport;
> -     flashport = find_ite_serial_flash_port(0x2e);
> -     if (flashport)
> -             it8716_serial_rdid(flashport);
> -     flashport = find_ite_serial_flash_port(0x4e);
> -     if (flashport)
> -             it8716_serial_rdid(flashport);
> +     it8716_flashport = find_ite_serial_flash_port(0x2e);
> +     if (!it8716_flashport)
> +             it8716_flashport = find_ite_serial_flash_port(0x4e);

#defines for 0x2e and 0x4e.


> +     return (!it8716_flashport);
> +}
> +
> +int probe_spi(struct flashchip *flash)
> +{
> +     unsigned char readarr[3];
> +     uint8_t manuf_id;
> +     uint16_t model_id;
> +     if (it8716_flashport) {
> +             it8716_serial_rdid(it8716_flashport, readarr);
> +             manuf_id = readarr[0];
> +             model_id = (readarr[1] << 8) | readarr[2];
> +             printf_debug("%s: id1 0x%x, id2 0x%x\n", __FUNCTION__, 
> manuf_id, model_id);
> +             if (manuf_id == flash->manufacture_id && model_id == 
> flash->model_id)
> +                     return 1;
> +     }
> +
>       return 0;
>  }


Looks good otherwise, but should be tested of course.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

Attachment: signature.asc
Description: Digital signature

-- 
linuxbios mailing list
linuxbios@linuxbios.org
http://www.linuxbios.org/mailman/listinfo/linuxbios

Reply via email to