Re: [flashrom] [PATCH] Revert MMIO space writes on shutdown as needed.

[Carl-Daniel Hailfinger]

Ping?

On 18.11.2010 02:37, Carl-Daniel Hailfinger wrote:
> Revert MMIO space writes on shutdown as needed.
> Reversible MMIO space writes now use rmmio_write*().
> MMIO space writes which are one-shot (e.g. communication with some chip)
> should continue to use the permanent mmio_write* variants.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-mmio_shutdown_restore/hwaccess.c
>
> --- flashrom-mmio_shutdown_restore/hwaccess.c	(Revision 1234)
> +++ flashrom-mmio_shutdown_restore/hwaccess.c	(Arbeitskopie)
> @@ -184,3 +184,71 @@
>  {
>    return le_to_cpu32(mmio_readl(addr));
>  }
>
> +enum mmio_write_type {
> mmio_write_type_b,
> mmio_write_type_w,
> mmio_write_type_l,
>
> +};
>
> +struct undo_mmio_write_data {
> void *addr;
> int reg;
> enum mmio_write_type type;
> union {
> uint8_t bdata;
> uint16_t wdata;
> uint32_t ldata;
> };
>
> +};
>
> +void undo_mmio_write(void *p)
> +{
> struct undo_mmio_write_data *data = p;
> msg_pdbg("Restoring MMIO space at %p\n", data->addr);
> switch (data->type) {
> case mmio_write_type_b:
> mmio_writeb(data->bdata, data->addr);
> break;
> case mmio_write_type_w:
> mmio_writeb(data->wdata, data->addr);
> break;
> case mmio_write_type_l:
> mmio_writeb(data->ldata, data->addr);
> break;
> }
> /* p was allocated in register_undo_mmio_write. */
> free(p);
>
> +}
>
> +#define register_undo_mmio_write(a, c)					\
> +{									\
> struct undo_mmio_write_data *undo_mmio_write_data;		\
> undo_mmio_write_data = malloc(sizeof(struct undo_mmio_write_data)); \
> undo_mmio_write_data->addr = a;				\
> undo_mmio_write_data->type = mmio_write_type_##c;		\
> undo_mmio_write_data->c##data = mmio_read##c(a);		\
> register_shutdown(undo_mmio_write, undo_mmio_write_data);	\
>
> +}
>
> +#define register_undo_mmio_writeb(a) register_undo_mmio_write(a, b)
> +#define register_undo_mmio_writew(a) register_undo_mmio_write(a, w)
> +#define register_undo_mmio_writel(a) register_undo_mmio_write(a, l)
>
> +void rmmio_writeb(uint8_t val, void *addr)
> +{
> register_undo_mmio_writeb(addr);
> mmio_writeb(val, addr);
>
> +}
>
> +void rmmio_writew(uint16_t val, void *addr)
> +{
> register_undo_mmio_writew(addr);
> mmio_writew(val, addr);
>
> +}
>
> +void rmmio_writel(uint32_t val, void *addr)
> +{
> register_undo_mmio_writel(addr);
> mmio_writel(val, addr);
>
> +}
> Index: flashrom-mmio_shutdown_restore/ichspi.c
> ===================================================================
> --- flashrom-mmio_shutdown_restore/ichspi.c	(Revision 1234)
> +++ flashrom-mmio_shutdown_restore/ichspi.c	(Arbeitskopie)
> @@ -382,16 +382,16 @@
>    switch (spi_controller) {
>    case SPI_CONTROLLER_ICH7:
>    case SPI_CONTROLLER_VIA:
> REGWRITE16(ICH7_REG_PREOP, preop);
> REGWRITE16(ICH7_REG_OPTYPE, optype);
> REGWRITE32(ICH7_REG_OPMENU, opmenu[0]);
> REGWRITE32(ICH7_REG_OPMENU + 4, opmenu[1]);
> rmmio_writew(preop, ich_spibar + ICH7_REG_PREOP);
> rmmio_writew(optype, ich_spibar + ICH7_REG_OPTYPE);
> rmmio_writew(opmenu[0], ich_spibar + ICH7_REG_OPMENU);
> rmmio_writew(opmenu[1], ich_spibar + ICH7_REG_OPMENU + 4);
> break;
> case SPI_CONTROLLER_ICH9:
> REGWRITE16(ICH9_REG_PREOP, preop);
> REGWRITE16(ICH9_REG_OPTYPE, optype);
> REGWRITE32(ICH9_REG_OPMENU, opmenu[0]);
> REGWRITE32(ICH9_REG_OPMENU + 4, opmenu[1]);
> rmmio_writew(preop, ich_spibar + ICH9_REG_PREOP);
> rmmio_writew(optype, ich_spibar + ICH9_REG_OPTYPE);
> rmmio_writew(opmenu[0], ich_spibar + ICH9_REG_OPMENU);
> rmmio_writew(opmenu[1], ich_spibar + ICH9_REG_OPMENU + 4);
> break;
> default:
> msg_perr("%s: unsupported chipset\n", __func__);
>
> @@ -409,16 +409,20 @@
>  {
>    switch (spi_controller) {
>    case SPI_CONTROLLER_ICH7:
> mmio_writel(minaddr, ich_spibar + 0x50);
> rmmio_writel(minaddr, ich_spibar + 0x50);
> ichspi_bbar = mmio_readl(ich_spibar + 0x50);
> /* We don't have any option except complaining. */
> /* We don't have any option except complaining. And if the write
>  * failed, the restore will fail as well, so no problem there.
>  */
> if (ichspi_bbar != minaddr)
> 	msg_perr("Setting BBAR failed!\n");
> break;
> case SPI_CONTROLLER_ICH9:
> mmio_writel(minaddr, ich_spibar + 0xA0);
> rmmio_writel(minaddr, ich_spibar + 0xA0);
> ichspi_bbar = mmio_readl(ich_spibar + 0xA0);
> /* We don't have any option except complaining. */
> /* We don't have any option except complaining. And if the write
>  * failed, the restore will fail as well, so no problem there.
>  */
> if (ichspi_bbar != minaddr)
> 	msg_perr("Setting BBAR failed!\n");
> break;
> Index: flashrom-mmio_shutdown_restore/nicintel_spi.c
>
> --- flashrom-mmio_shutdown_restore/nicintel_spi.c	(Revision 1234)
> +++ flashrom-mmio_shutdown_restore/nicintel_spi.c	(Arbeitskopie)
> @@ -149,6 +149,7 @@
>
> nicintel_spibar = physmap("Intel Gigabit NIC w/ SPI flash",
>    			  io_base_addr, 4096);
> /* Automatic restore is not possible because we only change two bits. */
> tmp = pci_mmio_readl(nicintel_spibar + EECD);
> tmp &= ~FLASH_WRITES_DISABLED;
> tmp |= FLASH_WRITES_ENABLED;
>
> @@ -168,6 +169,7 @@
>  {
>    uint32_t tmp;
> /* Automatic restore is not possible because we only change two bits. */
> tmp = pci_mmio_readl(nicintel_spibar + EECD);
> tmp &= ~FLASH_WRITES_ENABLED;
> tmp |= FLASH_WRITES_DISABLED;
> Index: flashrom-mmio_shutdown_restore/programmer.h
>
> --- flashrom-mmio_shutdown_restore/programmer.h	(Revision 1234)
> +++ flashrom-mmio_shutdown_restore/programmer.h	(Arbeitskopie)
> @@ -311,6 +311,9 @@
>  #define pci_mmio_readb mmio_le_readb
>  #define pci_mmio_readw mmio_le_readw
>  #define pci_mmio_readl mmio_le_readl
> +void rmmio_writeb(uint8_t val, void *addr);
> +void rmmio_writew(uint16_t val, void *addr);
> +void rmmio_writel(uint32_t val, void *addr);
>
> /* programmer.c */
>  int noop_shutdown(void);