[flashrom] [PATCH] Refactor ITE SuperI/O detection/access

Luc Verhaegen libv at skynet.be
Fri Dec 18 10:25:58 CET 2009


On Fri, Dec 18, 2009 at 05:12:19AM +0100, Carl-Daniel Hailfinger wrote:
> On 08.12.2009 11:52, Luc Verhaegen wrote:
> > On Mon, Nov 16, 2009 at 02:16:08AM +0100, Carl-Daniel Hailfinger wrote:
> >   
> >> Refactor ITE SuperI/O detection/access.
> >> The new code reduces duplications and paves the way for generic SuperI/O
> >> detection.
> >> Additional cost: We enter/exit the SuperI/O configuration mode twice.
> >> Once for detecting which SuperI/O it is, and once for actually changing
> >> the settings.
> >> Additional benefit: This has the potential of allowing some sort of
> >> primitive laptop detection (by way of checking whether an EC or a simple
> >> SuperI/O is present).
> >>
> >> I have added a call to a not yet implemented Winbond SuperI/O detection
> >> function inside #if 0 to make extension more straighforward.
> >>
> >> This affects most SiS chipsets and IT87 SPI translation, so tests in
> >> verbose mode would be appreciated. In theory, functionality should be
> >> unchanged.
> >>
> >> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> >>     
> >
> > NACK :)
> >   
> 
> Heh.
> 
> > The code in the chipset enable needs to go :)
> >   
> 
> #if 0 for now and moved to board_enable.c. Can postpone/drop if you want.
> 
> 
> > Apart from that, we need this, we need general superio detection, and
> >   
> 
> Done.
> 
> 
> > the write enables moved out of board_enable.c,
> 
> Not so 100% sure about that. Newer boards may use these settings as
> GPIOs for other purposes.
> 
> 
> > plus some gpio line setting code per superio.
> >   
> 
> True. Separate patch, though.
> 
> 
> > For the IT8705, we need to work out what we want to do, there are two 
> > similar routines in board_enable.c, and this code below is the third.
> >
> > We need a single one, and for the time being, it needs to be done
> > depending on a board match.
> >   
> 
> Hmmm. I'd postpone that to a separate patch once we agree upon the
> architecture and implementation for this one.
> 
> 
> > Also, do we need the ite spi probe before we do flash detection?
> >   
> 
> Absolutely. It happens in the board enable code right now, but we can
> kill a boatload of them if we run the it86spi detection always (well,
> always in the case a matching superio is present). If we don't run IT87
> SPI probe/setup before the first flash access, flashrom will NOT know
> that SPI is supported, and flashrom will therefore skip all SPI probing.
> That would effectively disable IT87 SPI completely.
> 
> 
> > So toss out the superio from the chipset enable (patch filed), and then 
> > alter the patch here to provide us with more general superio 
> > infrastructure :)
> >
> > We need this.
> >   
> 
> Next try. I focused on the generic infrastructure aspect and will leave
> conversion of non-IT8712/IT8716 code to later patches.
> SuperI/O detection now happens unconditionally and before the chipset
> enable. We could run it after chipset enable, but it definitely has to
> happen before board enable. I don't have a preference either way.
> The board enable table should contain a struct superio for better
> matching, but I'll leave that to a separate patch once this is tested
> and committed.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> 
> Index: flashrom-superio_ite_refactor/flash.h
> ===================================================================
> --- flashrom-superio_ite_refactor/flash.h	(Revision 807)
> +++ flashrom-superio_ite_refactor/flash.h	(Arbeitskopie)
> @@ -331,6 +331,14 @@
>  
>  /* internal.c */
>  #if NEED_PCI == 1
> +struct superio {
> +	uint16_t vendor;
> +	uint16_t port;
> +	uint16_t model;
> +};
> +extern struct superio superio;
> +#define SUPERIO_VENDOR_NONE	0x0
> +#define SUPERIO_VENDOR_ITE	0x1
>  struct pci_dev *pci_dev_find_filter(struct pci_filter filter);
>  struct pci_dev *pci_dev_find_vendorclass(uint16_t vendor, uint16_t class);
>  struct pci_dev *pci_dev_find(uint16_t vendor, uint16_t device);
> @@ -340,6 +348,7 @@
>  void get_io_perms(void);
>  void release_io_perms(void);
>  #if INTERNAL_SUPPORT == 1
> +void probe_superio(void);
>  int internal_init(void);
>  int internal_shutdown(void);
>  void internal_chip_writeb(uint8_t val, chipaddr addr);
> @@ -544,6 +553,7 @@
>  extern uint16_t it8716f_flashport;
>  void enter_conf_mode_ite(uint16_t port);
>  void exit_conf_mode_ite(uint16_t port);
> +struct superio probe_superio_ite(void);
>  int it87spi_init(void);
>  int it87xx_probe_spi_flash(const char *name);
>  int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> Index: flashrom-superio_ite_refactor/it87spi.c
> ===================================================================
> --- flashrom-superio_ite_refactor/it87spi.c	(Revision 807)
> +++ flashrom-superio_ite_refactor/it87spi.c	(Arbeitskopie)
> @@ -54,19 +54,56 @@
>  	sio_write(port, 0x02, 0x02);
>  }
>  
> -static uint16_t find_ite_spi_flash_port(uint16_t port)
> +uint16_t probe_id_ite(uint16_t port)
>  {
> -	uint8_t tmp = 0;
> -	char *portpos = NULL;
> -	uint16_t id, flashport = 0;
> +	uint16_t id;
>  
>  	enter_conf_mode_ite(port);
> -
>  	id = sio_read(port, CHIP_ID_BYTE1_REG) << 8;
>  	id |= sio_read(port, CHIP_ID_BYTE2_REG);
> +	exit_conf_mode_ite(port);
>  
> -	/* TODO: Handle more IT87xx if they support flash translation */
> -	if (0x8716 == id || 0x8718 == id) {
> +	return id;
> +}
> +
> +struct superio probe_superio_ite(void)
> +{
> +	struct superio ret = {};
> +	uint16_t ite_ports[] = {ITE_SUPERIO_PORT1, ITE_SUPERIO_PORT2, 0};
> +	uint16_t *i = ite_ports;
> +
> +	i++;
> +	ret.vendor = SUPERIO_VENDOR_ITE;
> +	for (; *i; i++) {
> +		ret.port = *i;
> +		ret.model = probe_id_ite(ret.port);
> +		switch (ret.model >> 8) {
> +		case 0x82:
> +		case 0x86:
> +		case 0x87:
> +			printf_debug("Found ITE SuperI/O, id %04hx\n",
> +				     ret.model);
> +			return ret;
> +		}
> +	}
> +
> +	/* No good ID found. */
> +	ret.vendor = SUPERIO_VENDOR_NONE;
> +	ret.port = 0;
> +	ret.model = 0;
> +	return ret;
> +}
> +
> +static uint16_t find_ite_spi_flash_port(uint16_t port, uint16_t id)
> +{
> +	uint8_t tmp = 0;
> +	char *portpos = NULL;
> +	uint16_t flashport = 0;
> +
> +	switch (id) {
> +	case 0x8716:
> +	case 0x8718:
> +		enter_conf_mode_ite(port);
>  		/* NOLDN, reg 0x24, mask out lowest bit (suspend) */
>  		tmp = sio_read(port, 0x24) & 0xFE;
>  		printf("Serial flash segment 0x%08x-0x%08x %sabled\n",
> @@ -105,17 +142,21 @@
>  			sio_write(port, 0x64, (flashport >> 8));
>  			sio_write(port, 0x65, (flashport & 0xff));
>  		}
> +		exit_conf_mode_ite(port);
> +		break;
> +	/* TODO: Handle more IT87xx if they support flash translation */
> +	default:
> +		printf("SuperI/O ID %04hx is not on the controller list.\n", id);
>  	}
> -	exit_conf_mode_ite(port);
>  	return flashport;
>  }
>  
>  int it87spi_common_init(void)
>  {
> -	it8716f_flashport = find_ite_spi_flash_port(ITE_SUPERIO_PORT1);
> +	if (superio.vendor != SUPERIO_VENDOR_ITE)
> +		return 1;
>  
> -	if (!it8716f_flashport)
> -		it8716f_flashport = find_ite_spi_flash_port(ITE_SUPERIO_PORT2);
> +	it8716f_flashport = find_ite_spi_flash_port(superio.port, superio.model);
>  
>  	if (it8716f_flashport)
>  		spi_controller = SPI_CONTROLLER_IT87XX;
> @@ -129,6 +170,8 @@
>  	int ret;
>  
>  	get_io_perms();
> +	/* Probe for the SuperI/O chip and fill global struct superio. */
> +	probe_superio();
>  	ret = it87spi_common_init();
>  	if (!ret) {
>  		buses_supported = CHIP_BUSTYPE_SPI;
> Index: flashrom-superio_ite_refactor/internal.c
> ===================================================================
> --- flashrom-superio_ite_refactor/internal.c	(Revision 807)
> +++ flashrom-superio_ite_refactor/internal.c	(Arbeitskopie)
> @@ -125,6 +125,17 @@
>  }
>  
>  #if INTERNAL_SUPPORT == 1
> +struct superio superio = {};
> +
> +void probe_superio(void)
> +{
> +	superio = probe_superio_ite();
> +#if 0	/* Winbond SuperI/O code is not yet available. */
> +	if (superio.vendor == SUPERIO_VENDOR_NONE)
> +		superio = probe_superio_winbond();
> +#endif
> +}
> +
>  int internal_init(void)
>  {
>  	int ret = 0;
> @@ -142,6 +153,9 @@
>  	 */
>  	coreboot_init();
>  
> +	/* Probe for the SuperI/O chip and fill global struct superio. */
> +	probe_superio();
> +
>  	/* try to enable it. Failure IS an option, since not all motherboards
>  	 * really need this to be done, etc., etc.
>  	 */
> Index: flashrom-superio_ite_refactor/board_enable.c
> ===================================================================
> --- flashrom-superio_ite_refactor/board_enable.c	(Revision 807)
> +++ flashrom-superio_ite_refactor/board_enable.c	(Arbeitskopie)
> @@ -66,6 +66,35 @@
>  	OUTB(tmp | (data & mask), port + 1);
>  }
>  
> +/* Not used yet. */
> +#if 0
> +static int enable_flash_decode_superio(void)
> +{
> +	int ret;
> +	uint8_t tmp;
> +
> +	switch (superio.vendor) {
> +	case SUPERIO_VENDOR_NONE:
> +		ret = -1;
> +		break;
> +	case SUPERIO_VENDOR_ITE:
> +		enter_conf_mode_ite(superio.port);
> +		/* Enable flash mapping. Works for most old ITE style SuperI/O. */
> +		tmp = sio_read(superio.port, 0x24);
> +		tmp |= 0xfc;
> +		sio_write(superio.port, 0x24, tmp);
> +		exit_conf_mode_ite(superio.port);
> +		ret = 0;
> +		break;
> +	default:
> +		printf_debug("Unhandled SuperI/O type!\n");
> +		ret = -1;
> +		break;
> +	}
> +	return ret;
> +}
> +#endif
> +
>  /**
>   * Winbond W83627HF: Raise GPIO24.
>   *
> 
> 
> -- 
> Developer quote of the month: 
> "We are juggling too many chainsaws and flaming arrows and tigers."
> 

This stuff needs a lot of work on top, but as a first start:

Acked-by: Luc Verhaegen <libv at skynet.be>

Luc Verhaegen.




More information about the flashrom mailing list