Patchwork Add atapromise programmer

login
register
about
Submitter Joseph Lehner
Date 2016-01-13 00:17:52
Message ID <569597B0.5030306@gmail.com>
Download mbox | patch
Permalink /patch/4356/
State Superseded
Headers show

Comments

Joseph Lehner - 2016-01-13 00:17:52
On 2016-01-13 00:29, Joseph Lehner wrote:
> This patch adds a new programmer that supports (admittedly ancient) Promise ATA
> controllers. I've successfully tested this on an Ultra100 controller I had lying
> around. Mine uses a 128 kB MX29F001T flash chip, but I was not able to write
> anything past the first page (32 kB), which is not that much of a problem, since the
> option ROM files for these controllers are only 16 kB).
> 
> It uses a (dirty?) hack to limit the chip size to 16 kB (so you can flash unmodified
> images) and provides an "allow32k=y" option, allowing you to flash larger images.
> 
> Signed-off-by: Joseph C. Lehner <joseph.c.lehner@gmail.com>

Send patch as text this time.
Carl-Daniel Hailfinger - 2016-01-13 01:44:57
Hi Joseph,

thanks for your patch!


On 13.01.2016 01:17, Joseph Lehner wrote:
> On 2016-01-13 00:29, Joseph Lehner wrote:
>> This patch adds a new programmer that supports (admittedly ancient) Promise ATA
>> controllers. I've successfully tested this on an Ultra100 controller I had lying
>> around. Mine uses a 128 kB MX29F001T flash chip, but I was not able to write
>> anything past the first page (32 kB), which is not that much of a problem, since the
>> option ROM files for these controllers are only 16 kB).

Oh wow. That's just ugly.


>> It uses a (dirty?) hack to limit the chip size to 16 kB (so you can flash unmodified
>> images) and provides an "allow32k=y" option, allowing you to flash larger images.

IMHO it would make more sense to always limit the size to 32k, and tell
people upon programmer init that they can just
cat download.rom download.rom > toflash.rom
or fill the first 16kiB with 0xff
for any firmware downloaded from the manufacturer.
This would reduce the complexity of the driver at the cost of a bit more
work for the user.

>> Signed-off-by: Joseph C. Lehner <joseph.c.lehner@gmail.com>
> Send patch as text this time.

Nice patch. Review follows inline.


> --- /dev/null
> +++ b/atapromise.c
> @@ -0,0 +1,259 @@
> +/*
> + * This file is part of the flashrom project.
> + *
> + * Copyright (C) 2015 Joseph C. Lehner <joseph.c.lehner@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> + */
> +
> +#if defined(__i386__) || defined(__x86_64__)
> +
> +#include <string.h>
> +#include <stdlib.h>
> +#include "flash.h"
> +#include "programmer.h"
> +#include "hwaccess.h"
> +
> +#define MAX_ROM_DECODE (32 * 1024)
> +#define ADDR_MASK (MAX_ROM_DECODE - 1)
> +
> +/*
> + * In the absence of any public docs on the PDC2026x family, this programmer
> + * was created through a mix of reverse-engineering and trial and error.
> + *
> + * The only device tested is an Ultra100 controller, but the logic for
> + * programming the other 2026x controllers is the same, so it should,
> + * in theory, work for those as well.
> + *
> + * This programmer is limited to the first 32 kB, which should be sufficient,

Are you sure it's the lowest 32 kiB, not the highest 32 kiB?


> + * given that the ROM files for these controllers are 16 kB. Since flashrom
> + * does not support flashing images smaller than the detected flash chip
> + * (the tested Ultra100 uses a 128 kB MX29F001T chip), the chip size
> + * is hackishly adjusted in atapromise_fixup_chip.

As mentioned above, 16 kiB support could be delegated to the user in
some way. That said, we also need the chip-size fixup during probing.
Might make sense to handle this in the generic flashrom core because
someone abusing the atapromise programmer as a generic flash programmer
would probably like to get a warning instead of missing data...


> + *
> + * To flash 32 kB files, use "allow32k=y".
> + */
> +
> +static uint32_t io_base_addr = 0;
> +static uint32_t rom_base_addr = 0;
> +
> +static uint8_t *atapromise_bar = NULL;
> +static size_t rom_size = 0;
> +
> +const struct dev_entry ata_promise[] = {
> +	{0x105a, 0x4d38, NT, "Promise", "PDC20262 (FastTrak66/Ultra66)"},
> +	{0x105a, 0x0d30, NT, "Promise", "PDC20265 (FastTrak100 Lite/Ultra100)"},
> +	{0x105a, 0x4d30, OK, "Promise", "PDC20267 (FastTrak100/Ultra100)"},
> +	{0},
> +};
> +
> +static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val,
> +				chipaddr addr);
> +static uint8_t atapromise_chip_readb(const struct flashctx *flash,
> +		const chipaddr addr);
> +
> +static const struct par_master par_master_atapromise = {
> +		.chip_readb		= atapromise_chip_readb,
> +		.chip_readw		= fallback_chip_readw,
> +		.chip_readl		= fallback_chip_readl,
> +		.chip_readn		= fallback_chip_readn,
> +		.chip_writeb	= atapromise_chip_writeb,
> +		.chip_writew	= fallback_chip_writew,
> +		.chip_writel	= fallback_chip_writel,
> +		.chip_writen	= fallback_chip_writen,
> +};
> +
> +void *atapromise_map(const char *descr, uintptr_t phys_addr, size_t len)
> +{
> +	/* In case fallback_map ever returns something other than NULL. */
> +	return NULL;
> +}
> +
> +static struct pci_dev *atapromise_find_bridge(struct pci_dev *dev)
> +{
> +	struct pci_dev *br;
> +	uint8_t bus_sec, bus_sub, htype;
> +
> +	for (br = dev->access->devices; br; br = br->next) {
> +		/* Don't use br->hdrtype here! */
> +		htype = pci_read_byte(br, PCI_HEADER_TYPE) & 0x7f;
> +		if (htype != PCI_HEADER_TYPE_BRIDGE)
> +			continue;
> +
> +		bus_sec = pci_read_byte(br, PCI_SECONDARY_BUS);
> +		bus_sub = pci_read_byte(br, PCI_SUBORDINATE_BUS);
> +
> +		if (dev->bus >= bus_sec && dev->bus <= bus_sub) {
> +			msg_pdbg("Device is behind bridge %04x:%04x, BDF %02x:%02x.%x.\n",
> +					br->vendor_id, br->device_id, br->bus, br->dev, br->func);
> +			return br;
> +		}
> +	}
> +
> +	return NULL;
> +}

This function is generic and could be useful in pcidev.c. That said, so
far we only need it for this driver.


> +
> +static int atapromise_fixup_bridge(struct pci_dev *dev)

Does it fix the bridge config, or does it possibly break the bridge
config? AFAICS if two PCI devices are behind the bridge and if the
non-Promise device has a memory region far away from the Promise memory
region, the whole range including both ranges and the unclaimed space in
between will be used as memory window of the bridge. AFAICS that may
cause havoc (crash, corruption) if some other PCI device behind another
bridge uses memory regions in between. I don't see a way to handle this
cleanly without a lot more code (and even then), though.


> +{
> +	struct pci_dev *br;
> +	uint16_t reg16;
> +
> +	/* TODO: What about chained bridges? */
> +	br = atapromise_find_bridge(dev);
> +	if (br) {
> +		/* Make sure that the bridge memory windows are set correctly. */
> +		reg16 = pci_read_word(dev, PCI_BASE_ADDRESS_5 + 2) & 0xfff0;
> +		if (reg16 < pci_read_word(br, PCI_MEMORY_BASE)) {
> +			msg_pdbg("Adjusting memory base of bridge to %04x.\n", reg16);
> +			rpci_write_word(br, PCI_MEMORY_BASE, reg16);
> +		}
> +
> +		reg16 += (MAX_ROM_DECODE / 1024);
> +
> +		if (reg16 < pci_read_word(br, PCI_MEMORY_LIMIT)) {

Inverted logic here?


> +			msg_pdbg("Adjusting memory limit of bridge to %04x.\n", reg16);
> +			rpci_write_word(br, PCI_MEMORY_LIMIT, reg16);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void atapromise_fixup_chip(struct flashchip *chip)

I wouldn't call it fixup. Mangle or limit might be better words. That
said, given the hardware limitations, there's not much you can do unless
you figure out a way to perform indirect access.


> +{
> +	static bool once = false;
> +	unsigned int i, size;
> +
> +	if (once)
> +		return;
> +
> +	size = chip->total_size * 1024;
> +	if (size > rom_size)
> +	{
> +		/* Undefine all block_erasers that don't operate on the whole chip,
> +		 * and adjust the eraseblock size of the one that does.
> +		 */
> +		for (i = 0; i < NUM_ERASEFUNCTIONS; ++i) {
> +			if (chip->block_erasers[i].eraseblocks[0].size != size) {
> +				chip->block_erasers[i].eraseblocks[0].count = 0;
> +				chip->block_erasers[i].block_erase = NULL;
> +			} else {
> +				chip->block_erasers[i].eraseblocks[0].size = rom_size;
> +				break;
> +			}
> +		}
> +
> +		if (i != NUM_ERASEFUNCTIONS) {
> +			chip->total_size = rom_size / 1024;
> +			if (chip->page_size > rom_size)
> +				chip->page_size = rom_size;
> +		} else {
> +			msg_pwarn("Failed to adjust size of chip \"%s\" (%d kB).\n",
> +					chip->name, chip->total_size);
> +		}
> +	}
> +
> +	once = true;
> +}
> +
> +static bool atapromise_allow32k()
> +{
> +	bool ret;
> +	char *p;
> +
> +	p = extract_programmer_param("allow32k");
> +	if (!p) {
> +		return false;
> +	}
> +
> +	ret = p[0] == '1' || p[0] == 'y' || p[0] == 'Y';
> +	free(p);
> +	return ret;
> +}
> +
> +int atapromise_init(void)
> +{
> +	struct pci_dev *dev = NULL;
> +	char *param_32k = NULL;
> +
> +	if (rget_io_perms())
> +		return 1;
> +
> +	dev = pcidev_init(ata_promise, PCI_BASE_ADDRESS_4);
> +	if (!dev)
> +		return 1;
> +
> +	if (atapromise_fixup_bridge(dev))
> +		return 1;
> +
> +	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_4) & 0xfffe;
> +	if (!io_base_addr) {
> +		return 1;
> +	}
> +
> +	/* Not exactly sure what this does, because flashing seems to work
> +	 * well without it. However, PTIFLASH does it, so we do it too.
> +	 */
> +	OUTB(1, io_base_addr + 0x10);
> +
> +	rom_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_5);
> +	if (!rom_base_addr) {
> +		msg_pdbg("Failed to read BAR5\n");
> +		return 1;
> +	}
> +
> +	if (atapromise_allow32k()) {
> +			if (dev->rom_size < (32 * 1024)) {
> +				msg_perr("ROM size is reported as %zu kB. Cannot flash 32 kB "
> +						"files.\n", dev->rom_size);
> +				return 1;
> +			}
> +			rom_size = 32 * 1024;
> +	} else {
> +		/* Default to 16 kB, so we can flash unpadded images */
> +		rom_size = 16 * 1024;
> +	}
> +
> +	atapromise_bar = (uint8_t*)rphysmap("Promise", rom_base_addr, rom_size);
> +	if (atapromise_bar == ERROR_PTR) {
> +		return 1;
> +	}
> +
> +	max_rom_decode.parallel = rom_size;
> +	register_par_master(&par_master_atapromise, BUS_PARALLEL);
> +
> +	return 0;
> +}
> +
> +static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val,
> +				chipaddr addr)
> +{
> +	uint32_t data;
> +
> +	atapromise_fixup_chip(flash->chip);

Calling atapromise_fixup_chip once for every chip before beginning probe
instead of once per access would speed up things, but flashrom currently
has no infrastructure to accommodate this. That said, it should be easy
to add the infrastructure.


> +	data = (rom_base_addr + (addr & ADDR_MASK)) << 8 | val;
> +	OUTL(data, io_base_addr + 0x14);
> +}
> +
> +static uint8_t atapromise_chip_readb(const struct flashctx *flash,
> +				  const chipaddr addr)
> +{
> +	atapromise_fixup_chip(flash->chip);
> +	return pci_mmio_readb(atapromise_bar + (addr & ADDR_MASK));
> +}
> +
> +#else
> +#error PCI port I/O access is not supported on this architecture yet.
> +#endif
> diff --git a/flashrom.c b/flashrom.c
> index 113a05b..5503c05 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -182,6 +182,18 @@ const struct programmer_entry programmer_table[] = {
>  	},
>  #endif
>  
> +#if CONFIG_ATAPROMISE == 1
> +	{
> +		.name			= "atapromise",
> +		.type			= PCI,
> +		.devs.dev		= ata_promise,
> +		.init			= atapromise_init,
> +		.map_flash_region	= atapromise_map,
> +		.unmap_flash_region	= fallback_unmap,
> +		.delay			= internal_delay,
> +	},
> +#endif
> +
>  #if CONFIG_IT8212 == 1
>  	{
>  		.name			= "it8212",

Overall, I'd say it's really good quality code.

I'd like to have Stefan's input on the chip size mangling and how to
solve the problem of flashrom changing chip contents blindly for mangled
chips.

Regards,
Carl-Daniel
Carl-Daniel Hailfinger - 2016-01-13 10:48:21
Hi Joseph,

a few followup questions:

Could you please supply the output of "lspci -nnvvvxxx" (as root) for
the device you tested?

Do you know if all address lines (A0-A16) of the flash chip are
connected? If not, which are? This would help us determine how much of
the flash chip the hardware should be able to address.

Oh, and a few more comments below...

On 13.01.2016 02:44, Carl-Daniel Hailfinger wrote:
> thanks for your patch!
>
>
> On 13.01.2016 01:17, Joseph Lehner wrote:
>> On 2016-01-13 00:29, Joseph Lehner wrote:
>>> This patch adds a new programmer that supports (admittedly ancient) Promise ATA
>>> controllers. I've successfully tested this on an Ultra100 controller I had lying
>>> around. Mine uses a 128 kB MX29F001T flash chip, but I was not able to write
>>> anything past the first page (32 kB),

My suspicion is that you may have been successful writing to bigger
addresses, but the read method discarded anything above the 32 kB limit
so you could not see what you had written. I can't prove my suspicion,
though.


>>> which is not that much of a problem, since the
>>> option ROM files for these controllers are only 16 kB).
> Oh wow. That's just ugly.
>
>
>>> It uses a (dirty?) hack to limit the chip size to 16 kB (so you can flash unmodified
>>> images) and provides an "allow32k=y" option, allowing you to flash larger images.
> IMHO it would make more sense to always limit the size to 32k, and tell
> people upon programmer init that they can just
> cat download.rom download.rom > toflash.rom
> or fill the first 16kiB with 0xff
> for any firmware downloaded from the manufacturer.
> This would reduce the complexity of the driver at the cost of a bit more
> work for the user.
>
>>> Signed-off-by: Joseph C. Lehner <joseph.c.lehner@gmail.com>
>> Send patch as text this time.
> Nice patch. Review follows inline.
>
>
>> --- /dev/null
>> +++ b/atapromise.c
>> @@ -0,0 +1,259 @@
>> +/*
>> + * This file is part of the flashrom project.
>> + *
>> + * Copyright (C) 2015 Joseph C. Lehner <joseph.c.lehner@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
>> + */
>> +
>> +#if defined(__i386__) || defined(__x86_64__)
>> +
>> +#include <string.h>
>> +#include <stdlib.h>
>> +#include "flash.h"
>> +#include "programmer.h"
>> +#include "hwaccess.h"
>> +
>> +#define MAX_ROM_DECODE (32 * 1024)
>> +#define ADDR_MASK (MAX_ROM_DECODE - 1)
>> +
>> +/*
>> + * In the absence of any public docs on the PDC2026x family, this programmer
>> + * was created through a mix of reverse-engineering and trial and error.
>> + *
>> + * The only device tested is an Ultra100 controller, but the logic for
>> + * programming the other 2026x controllers is the same, so it should,
>> + * in theory, work for those as well.
>> + *
>> + * This programmer is limited to the first 32 kB, which should be sufficient,
> Are you sure it's the lowest 32 kiB, not the highest 32 kiB?
>
>
>> + * given that the ROM files for these controllers are 16 kB. Since flashrom
>> + * does not support flashing images smaller than the detected flash chip
>> + * (the tested Ultra100 uses a 128 kB MX29F001T chip), the chip size
>> + * is hackishly adjusted in atapromise_fixup_chip.
> As mentioned above, 16 kiB support could be delegated to the user in
> some way. That said, we also need the chip-size fixup during probing.
> Might make sense to handle this in the generic flashrom core because
> someone abusing the atapromise programmer as a generic flash programmer
> would probably like to get a warning instead of missing data...
>
>
>> + *
>> + * To flash 32 kB files, use "allow32k=y".
>> + */
>> +
>> +static uint32_t io_base_addr = 0;
>> +static uint32_t rom_base_addr = 0;
>> +
>> +static uint8_t *atapromise_bar = NULL;
>> +static size_t rom_size = 0;
>> +
>> +const struct dev_entry ata_promise[] = {
>> +	{0x105a, 0x4d38, NT, "Promise", "PDC20262 (FastTrak66/Ultra66)"},
>> +	{0x105a, 0x0d30, NT, "Promise", "PDC20265 (FastTrak100 Lite/Ultra100)"},
>> +	{0x105a, 0x4d30, OK, "Promise", "PDC20267 (FastTrak100/Ultra100)"},
>> +	{0},
>> +};
>> +
>> +static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val,
>> +				chipaddr addr);
>> +static uint8_t atapromise_chip_readb(const struct flashctx *flash,
>> +		const chipaddr addr);
>> +
>> +static const struct par_master par_master_atapromise = {
>> +		.chip_readb		= atapromise_chip_readb,
>> +		.chip_readw		= fallback_chip_readw,
>> +		.chip_readl		= fallback_chip_readl,
>> +		.chip_readn		= fallback_chip_readn,
>> +		.chip_writeb	= atapromise_chip_writeb,
>> +		.chip_writew	= fallback_chip_writew,
>> +		.chip_writel	= fallback_chip_writel,
>> +		.chip_writen	= fallback_chip_writen,
>> +};
>> +
>> +void *atapromise_map(const char *descr, uintptr_t phys_addr, size_t len)
>> +{
>> +	/* In case fallback_map ever returns something other than NULL. */
>> +	return NULL;
>> +}
>> +
>> +static struct pci_dev *atapromise_find_bridge(struct pci_dev *dev)
>> +{
>> +	struct pci_dev *br;
>> +	uint8_t bus_sec, bus_sub, htype;
>> +
>> +	for (br = dev->access->devices; br; br = br->next) {
>> +		/* Don't use br->hdrtype here! */
>> +		htype = pci_read_byte(br, PCI_HEADER_TYPE) & 0x7f;
>> +		if (htype != PCI_HEADER_TYPE_BRIDGE)
>> +			continue;
>> +
>> +		bus_sec = pci_read_byte(br, PCI_SECONDARY_BUS);
>> +		bus_sub = pci_read_byte(br, PCI_SUBORDINATE_BUS);
>> +
>> +		if (dev->bus >= bus_sec && dev->bus <= bus_sub) {
>> +			msg_pdbg("Device is behind bridge %04x:%04x, BDF %02x:%02x.%x.\n",
>> +					br->vendor_id, br->device_id, br->bus, br->dev, br->func);
>> +			return br;
>> +		}
>> +	}
>> +
>> +	return NULL;
>> +}
> This function is generic and could be useful in pcidev.c. That said, so
> far we only need it for this driver.
>
>
>> +
>> +static int atapromise_fixup_bridge(struct pci_dev *dev)
> Does it fix the bridge config, or does it possibly break the bridge
> config? AFAICS if two PCI devices are behind the bridge and if the
> non-Promise device has a memory region far away from the Promise memory
> region, the whole range including both ranges and the unclaimed space in
> between will be used as memory window of the bridge. AFAICS that may
> cause havoc (crash, corruption) if some other PCI device behind another
> bridge uses memory regions in between. I don't see a way to handle this
> cleanly without a lot more code (and even then), though.
>
>
>> +{
>> +	struct pci_dev *br;
>> +	uint16_t reg16;
>> +
>> +	/* TODO: What about chained bridges? */
>> +	br = atapromise_find_bridge(dev);
>> +	if (br) {
>> +		/* Make sure that the bridge memory windows are set correctly. */
>> +		reg16 = pci_read_word(dev, PCI_BASE_ADDRESS_5 + 2) & 0xfff0;
>> +		if (reg16 < pci_read_word(br, PCI_MEMORY_BASE)) {
>> +			msg_pdbg("Adjusting memory base of bridge to %04x.\n", reg16);
>> +			rpci_write_word(br, PCI_MEMORY_BASE, reg16);
>> +		}
>> +
>> +		reg16 += (MAX_ROM_DECODE / 1024);
>> +
>> +		if (reg16 < pci_read_word(br, PCI_MEMORY_LIMIT)) {
> Inverted logic here?
>
>
>> +			msg_pdbg("Adjusting memory limit of bridge to %04x.\n", reg16);
>> +			rpci_write_word(br, PCI_MEMORY_LIMIT, reg16);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void atapromise_fixup_chip(struct flashchip *chip)
> I wouldn't call it fixup. Mangle or limit might be better words. That
> said, given the hardware limitations, there's not much you can do unless
> you figure out a way to perform indirect access.
>
>
>> +{
>> +	static bool once = false;
>> +	unsigned int i, size;
>> +
>> +	if (once)
>> +		return;
>> +
>> +	size = chip->total_size * 1024;
>> +	if (size > rom_size)
>> +	{
>> +		/* Undefine all block_erasers that don't operate on the whole chip,
>> +		 * and adjust the eraseblock size of the one that does.
>> +		 */
>> +		for (i = 0; i < NUM_ERASEFUNCTIONS; ++i) {
>> +			if (chip->block_erasers[i].eraseblocks[0].size != size) {
>> +				chip->block_erasers[i].eraseblocks[0].count = 0;
>> +				chip->block_erasers[i].block_erase = NULL;
>> +			} else {
>> +				chip->block_erasers[i].eraseblocks[0].size = rom_size;
>> +				break;
>> +			}
>> +		}
>> +
>> +		if (i != NUM_ERASEFUNCTIONS) {
>> +			chip->total_size = rom_size / 1024;
>> +			if (chip->page_size > rom_size)
>> +				chip->page_size = rom_size;
>> +		} else {
>> +			msg_pwarn("Failed to adjust size of chip \"%s\" (%d kB).\n",
>> +					chip->name, chip->total_size);
>> +		}
>> +	}
>> +
>> +	once = true;
>> +}
>> +
>> +static bool atapromise_allow32k()
>> +{
>> +	bool ret;
>> +	char *p;
>> +
>> +	p = extract_programmer_param("allow32k");
>> +	if (!p) {
>> +		return false;
>> +	}
>> +
>> +	ret = p[0] == '1' || p[0] == 'y' || p[0] == 'Y';
>> +	free(p);
>> +	return ret;
>> +}
>> +
>> +int atapromise_init(void)
>> +{
>> +	struct pci_dev *dev = NULL;
>> +	char *param_32k = NULL;
>> +
>> +	if (rget_io_perms())
>> +		return 1;
>> +
>> +	dev = pcidev_init(ata_promise, PCI_BASE_ADDRESS_4);
>> +	if (!dev)
>> +		return 1;
>> +
>> +	if (atapromise_fixup_bridge(dev))
>> +		return 1;
>> +
>> +	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_4) & 0xfffe;
>> +	if (!io_base_addr) {
>> +		return 1;
>> +	}
>> +
>> +	/* Not exactly sure what this does, because flashing seems to work
>> +	 * well without it. However, PTIFLASH does it, so we do it too.
>> +	 */
>> +	OUTB(1, io_base_addr + 0x10);
>> +
>> +	rom_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_5);
>> +	if (!rom_base_addr) {
>> +		msg_pdbg("Failed to read BAR5\n");
>> +		return 1;
>> +	}
>> +
>> +	if (atapromise_allow32k()) {
>> +			if (dev->rom_size < (32 * 1024)) {
>> +				msg_perr("ROM size is reported as %zu kB. Cannot flash 32 kB "
>> +						"files.\n", dev->rom_size);
>> +				return 1;
>> +			}
>> +			rom_size = 32 * 1024;
>> +	} else {
>> +		/* Default to 16 kB, so we can flash unpadded images */
>> +		rom_size = 16 * 1024;
>> +	}
>> +
>> +	atapromise_bar = (uint8_t*)rphysmap("Promise", rom_base_addr, rom_size);
>> +	if (atapromise_bar == ERROR_PTR) {
>> +		return 1;
>> +	}
>> +
>> +	max_rom_decode.parallel = rom_size;
>> +	register_par_master(&par_master_atapromise, BUS_PARALLEL);
>> +
>> +	return 0;
>> +}
>> +
>> +static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val,
>> +				chipaddr addr)
>> +{
>> +	uint32_t data;
>> +
>> +	atapromise_fixup_chip(flash->chip);
> Calling atapromise_fixup_chip once for every chip before beginning probe
> instead of once per access would speed up things, but flashrom currently
> has no infrastructure to accommodate this. That said, it should be easy
> to add the infrastructure.
>
>
>> +	data = (rom_base_addr + (addr & ADDR_MASK)) << 8 | val;
>> +	OUTL(data, io_base_addr + 0x14);
>> +}
>> +
>> +static uint8_t atapromise_chip_readb(const struct flashctx *flash,
>> +				  const chipaddr addr)
>> +{
>> +	atapromise_fixup_chip(flash->chip);
>> +	return pci_mmio_readb(atapromise_bar + (addr & ADDR_MASK));

You're using an asymmetric interface here. You write to the flash chip
with OUTL, but you read from the flash chip with readb from another BAR.
My hope is that there is another way to read flash contents via OUT*/IN*
which doesn't suffer from the address limitation.


>> +}
>> +
>> +#else
>> +#error PCI port I/O access is not supported on this architecture yet.
>> +#endif
>> diff --git a/flashrom.c b/flashrom.c
>> index 113a05b..5503c05 100644
>> --- a/flashrom.c
>> +++ b/flashrom.c
>> @@ -182,6 +182,18 @@ const struct programmer_entry programmer_table[] = {
>>  	},
>>  #endif
>>  
>> +#if CONFIG_ATAPROMISE == 1
>> +	{
>> +		.name			= "atapromise",
>> +		.type			= PCI,
>> +		.devs.dev		= ata_promise,
>> +		.init			= atapromise_init,
>> +		.map_flash_region	= atapromise_map,
>> +		.unmap_flash_region	= fallback_unmap,
>> +		.delay			= internal_delay,
>> +	},
>> +#endif
>> +
>>  #if CONFIG_IT8212 == 1
>>  	{
>>  		.name			= "it8212",
> Overall, I'd say it's really good quality code.
>
> I'd like to have Stefan's input on the chip size mangling and how to
> solve the problem of flashrom changing chip contents blindly for mangled
> chips.

Regards,
Carl-Daniel

Patch

diff --git a/Makefile b/Makefile
index de8464d..effd741 100644
--- a/Makefile
+++ b/Makefile
@@ -217,6 +217,11 @@  UNSUPPORTED_FEATURES += CONFIG_ATAVIA=yes
 else
 override CONFIG_ATAVIA = no
 endif
+ifeq ($(CONFIG_ATAPROMISE), yes)
+UNSUPPORTED_FEATURES += CONFIG_ATAPROMISE=yes
+else
+override CONFIG_ATAPROMISE = no
+endif
 ifeq ($(CONFIG_IT8212), yes)
 UNSUPPORTED_FEATURES += CONFIG_IT8212=yes
 else
@@ -439,6 +444,9 @@  CONFIG_ATAHPT ?= no
 # VIA VT6421A LPC memory support
 CONFIG_ATAVIA ?= yes
 
+# Promise ATA controller support.
+CONFIG_ATAPROMISE ?= yes
+
 # Always enable FT2232 SPI dongles for now.
 CONFIG_FT2232_SPI ?= yes
 
@@ -616,6 +624,12 @@  PROGRAMMER_OBJS += atavia.o
 NEED_PCI := yes
 endif
 
+ifeq ($(CONFIG_ATAPROMISE), yes)
+FEATURE_CFLAGS += -D'CONFIG_ATAPROMISE=1'
+PROGRAMMER_OBJS += atapromise.o
+NEED_PCI := yes
+endif
+
 ifeq ($(CONFIG_IT8212), yes)
 FEATURE_CFLAGS += -D'CONFIG_IT8212=1'
 PROGRAMMER_OBJS += it8212.o
diff --git a/atapromise.c b/atapromise.c
new file mode 100644
index 0000000..bf31286
--- /dev/null
+++ b/atapromise.c
@@ -0,0 +1,259 @@ 
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright (C) 2015 Joseph C. Lehner <joseph.c.lehner@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+ */
+
+#if defined(__i386__) || defined(__x86_64__)
+
+#include <string.h>
+#include <stdlib.h>
+#include "flash.h"
+#include "programmer.h"
+#include "hwaccess.h"
+
+#define MAX_ROM_DECODE (32 * 1024)
+#define ADDR_MASK (MAX_ROM_DECODE - 1)
+
+/*
+ * In the absence of any public docs on the PDC2026x family, this programmer
+ * was created through a mix of reverse-engineering and trial and error.
+ *
+ * The only device tested is an Ultra100 controller, but the logic for
+ * programming the other 2026x controllers is the same, so it should,
+ * in theory, work for those as well.
+ *
+ * This programmer is limited to the first 32 kB, which should be sufficient,
+ * given that the ROM files for these controllers are 16 kB. Since flashrom
+ * does not support flashing images smaller than the detected flash chip
+ * (the tested Ultra100 uses a 128 kB MX29F001T chip), the chip size
+ * is hackishly adjusted in atapromise_fixup_chip.
+ *
+ * To flash 32 kB files, use "allow32k=y".
+ */
+
+static uint32_t io_base_addr = 0;
+static uint32_t rom_base_addr = 0;
+
+static uint8_t *atapromise_bar = NULL;
+static size_t rom_size = 0;
+
+const struct dev_entry ata_promise[] = {
+	{0x105a, 0x4d38, NT, "Promise", "PDC20262 (FastTrak66/Ultra66)"},
+	{0x105a, 0x0d30, NT, "Promise", "PDC20265 (FastTrak100 Lite/Ultra100)"},
+	{0x105a, 0x4d30, OK, "Promise", "PDC20267 (FastTrak100/Ultra100)"},
+	{0},
+};
+
+static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val,
+				chipaddr addr);
+static uint8_t atapromise_chip_readb(const struct flashctx *flash,
+		const chipaddr addr);
+
+static const struct par_master par_master_atapromise = {
+		.chip_readb		= atapromise_chip_readb,
+		.chip_readw		= fallback_chip_readw,
+		.chip_readl		= fallback_chip_readl,
+		.chip_readn		= fallback_chip_readn,
+		.chip_writeb	= atapromise_chip_writeb,
+		.chip_writew	= fallback_chip_writew,
+		.chip_writel	= fallback_chip_writel,
+		.chip_writen	= fallback_chip_writen,
+};
+
+void *atapromise_map(const char *descr, uintptr_t phys_addr, size_t len)
+{
+	/* In case fallback_map ever returns something other than NULL. */
+	return NULL;
+}
+
+static struct pci_dev *atapromise_find_bridge(struct pci_dev *dev)
+{
+	struct pci_dev *br;
+	uint8_t bus_sec, bus_sub, htype;
+
+	for (br = dev->access->devices; br; br = br->next) {
+		/* Don't use br->hdrtype here! */
+		htype = pci_read_byte(br, PCI_HEADER_TYPE) & 0x7f;
+		if (htype != PCI_HEADER_TYPE_BRIDGE)
+			continue;
+
+		bus_sec = pci_read_byte(br, PCI_SECONDARY_BUS);
+		bus_sub = pci_read_byte(br, PCI_SUBORDINATE_BUS);
+
+		if (dev->bus >= bus_sec && dev->bus <= bus_sub) {
+			msg_pdbg("Device is behind bridge %04x:%04x, BDF %02x:%02x.%x.\n",
+					br->vendor_id, br->device_id, br->bus, br->dev, br->func);
+			return br;
+		}
+	}
+
+	return NULL;
+}
+
+static int atapromise_fixup_bridge(struct pci_dev *dev)
+{
+	struct pci_dev *br;
+	uint16_t reg16;
+
+	/* TODO: What about chained bridges? */
+	br = atapromise_find_bridge(dev);
+	if (br) {
+		/* Make sure that the bridge memory windows are set correctly. */
+		reg16 = pci_read_word(dev, PCI_BASE_ADDRESS_5 + 2) & 0xfff0;
+		if (reg16 < pci_read_word(br, PCI_MEMORY_BASE)) {
+			msg_pdbg("Adjusting memory base of bridge to %04x.\n", reg16);
+			rpci_write_word(br, PCI_MEMORY_BASE, reg16);
+		}
+
+		reg16 += (MAX_ROM_DECODE / 1024);
+
+		if (reg16 < pci_read_word(br, PCI_MEMORY_LIMIT)) {
+			msg_pdbg("Adjusting memory limit of bridge to %04x.\n", reg16);
+			rpci_write_word(br, PCI_MEMORY_LIMIT, reg16);
+		}
+	}
+
+	return 0;
+}
+
+static void atapromise_fixup_chip(struct flashchip *chip)
+{
+	static bool once = false;
+	unsigned int i, size;
+
+	if (once)
+		return;
+
+	size = chip->total_size * 1024;
+	if (size > rom_size)
+	{
+		/* Undefine all block_erasers that don't operate on the whole chip,
+		 * and adjust the eraseblock size of the one that does.
+		 */
+		for (i = 0; i < NUM_ERASEFUNCTIONS; ++i) {
+			if (chip->block_erasers[i].eraseblocks[0].size != size) {
+				chip->block_erasers[i].eraseblocks[0].count = 0;
+				chip->block_erasers[i].block_erase = NULL;
+			} else {
+				chip->block_erasers[i].eraseblocks[0].size = rom_size;
+				break;
+			}
+		}
+
+		if (i != NUM_ERASEFUNCTIONS) {
+			chip->total_size = rom_size / 1024;
+			if (chip->page_size > rom_size)
+				chip->page_size = rom_size;
+		} else {
+			msg_pwarn("Failed to adjust size of chip \"%s\" (%d kB).\n",
+					chip->name, chip->total_size);
+		}
+	}
+
+	once = true;
+}
+
+static bool atapromise_allow32k()
+{
+	bool ret;
+	char *p;
+
+	p = extract_programmer_param("allow32k");
+	if (!p) {
+		return false;
+	}
+
+	ret = p[0] == '1' || p[0] == 'y' || p[0] == 'Y';
+	free(p);
+	return ret;
+}
+
+int atapromise_init(void)
+{
+	struct pci_dev *dev = NULL;
+	char *param_32k = NULL;
+
+	if (rget_io_perms())
+		return 1;
+
+	dev = pcidev_init(ata_promise, PCI_BASE_ADDRESS_4);
+	if (!dev)
+		return 1;
+
+	if (atapromise_fixup_bridge(dev))
+		return 1;
+
+	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_4) & 0xfffe;
+	if (!io_base_addr) {
+		return 1;
+	}
+
+	/* Not exactly sure what this does, because flashing seems to work
+	 * well without it. However, PTIFLASH does it, so we do it too.
+	 */
+	OUTB(1, io_base_addr + 0x10);
+
+	rom_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_5);
+	if (!rom_base_addr) {
+		msg_pdbg("Failed to read BAR5\n");
+		return 1;
+	}
+
+	if (atapromise_allow32k()) {
+			if (dev->rom_size < (32 * 1024)) {
+				msg_perr("ROM size is reported as %zu kB. Cannot flash 32 kB "
+						"files.\n", dev->rom_size);
+				return 1;
+			}
+			rom_size = 32 * 1024;
+	} else {
+		/* Default to 16 kB, so we can flash unpadded images */
+		rom_size = 16 * 1024;
+	}
+
+	atapromise_bar = (uint8_t*)rphysmap("Promise", rom_base_addr, rom_size);
+	if (atapromise_bar == ERROR_PTR) {
+		return 1;
+	}
+
+	max_rom_decode.parallel = rom_size;
+	register_par_master(&par_master_atapromise, BUS_PARALLEL);
+
+	return 0;
+}
+
+static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val,
+				chipaddr addr)
+{
+	uint32_t data;
+
+	atapromise_fixup_chip(flash->chip);
+	data = (rom_base_addr + (addr & ADDR_MASK)) << 8 | val;
+	OUTL(data, io_base_addr + 0x14);
+}
+
+static uint8_t atapromise_chip_readb(const struct flashctx *flash,
+				  const chipaddr addr)
+{
+	atapromise_fixup_chip(flash->chip);
+	return pci_mmio_readb(atapromise_bar + (addr & ADDR_MASK));
+}
+
+#else
+#error PCI port I/O access is not supported on this architecture yet.
+#endif
diff --git a/flashrom.c b/flashrom.c
index 113a05b..5503c05 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -182,6 +182,18 @@  const struct programmer_entry programmer_table[] = {
 	},
 #endif
 
+#if CONFIG_ATAPROMISE == 1
+	{
+		.name			= "atapromise",
+		.type			= PCI,
+		.devs.dev		= ata_promise,
+		.init			= atapromise_init,
+		.map_flash_region	= atapromise_map,
+		.unmap_flash_region	= fallback_unmap,
+		.delay			= internal_delay,
+	},
+#endif
+
 #if CONFIG_IT8212 == 1
 	{
 		.name			= "it8212",
diff --git a/programmer.h b/programmer.h
index 97f0ffa..ff7cf5f 100644
--- a/programmer.h
+++ b/programmer.h
@@ -57,6 +57,9 @@  enum programmer {
 #if CONFIG_ATAVIA == 1
 	PROGRAMMER_ATAVIA,
 #endif
+#if CONFIG_ATAPROMISE == 1
+	PROGRAMMER_ATAPROMISE,
+#endif
 #if CONFIG_IT8212 == 1
 	PROGRAMMER_IT8212,
 #endif
@@ -460,6 +463,13 @@  void *atavia_map(const char *descr, uintptr_t phys_addr, size_t len);
 extern const struct dev_entry ata_via[];
 #endif
 
+/* atapromise.c */
+#if CONFIG_ATAPROMISE == 1
+int atapromise_init(void);
+void *atapromise_map(const char *descr, uintptr_t phys_addr, size_t len);
+extern const struct dev_entry ata_promise[];
+#endif
+
 /* it8212.c */
 #if CONFIG_IT8212 == 1
 int it8212_init(void);