[PATCH] Add atapromise programmer

[Joseph Lehner]

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 Lehner]

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

Send patch as text this time.

[Carl-Daniel Hailfinger]

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
>
> 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
> 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]

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
> >
> > 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
> > 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

[Joseph Lehner]

Hi Carl-Daniel,

I've updated the patch to address some of the issues you pointed out, namely:

*) Remove the allow32k param
*) Renaming fixup_chip to limit_chip (and also make it work when NOT using -c
<flashchip>)
*) Remove the bridge fixup/break stuff because it's not actually neccessary (turns
out the bridge was misconfigured because I had to hot-plug the controller, after a
failed flash prevented my machine from booting).

Regarding your other questions, inline answers follow.

On 2016-01-13 11:48, Carl-Daniel Hailfinger wrote:
> Hi Joseph,
>
> a few followup questions:
>
> Could you please supply the output of "lspci -nnvvvxxx" (as root) for
> the device you tested?

04:01.0 Mass storage controller [0180]: Promise Technology, Inc. PDC20267
(FastTrak100/Ultra100) [105a:4d30] (rev 02)
    Subsystem: Promise Technology, Inc. Ultra100 [105a:4d33]
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR-
FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort-
> SERR- <PERR- INTx-

Latency: 32
    Interrupt: pin A routed to IRQ 15
    Region 0: I/O ports at d0f0 [size=8]
    Region 1: I/O ports at d0e0 [size=4]
    Region 2: I/O ports at d0d0 [size=8]
    Region 3: I/O ports at d0c0 [size=4]
    Region 4: I/O ports at d080 [size=64]
    Region 5: Memory at df300000 (32-bit, non-prefetchable) [size=128K]
    Expansion ROM at df340000 [disabled] [size=64K]
    Capabilities: [58] Power Management version 1
    	Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
    	Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
00: 5a 10 30 4d 07 00 10 02 02 00 80 01 00 20 00 00
10: f1 d0 00 00 e1 d0 00 00 d1 d0 00 00 c1 d0 00 00
20: 81 d0 00 00 00 00 30 df 00 00 00 00 5a 10 33 4d
30: 00 00 34 df 58 00 00 00 00 00 00 00 0f 01 00 00
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50: ce 3b 00 00 00 00 00 00 01 00 01 00 00 00 00 00
60: 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Note that BAR5 is listed as having a size of 128 kB, but when reading 128 kB from
this physical address, the data repeats every 32 kB. The same thing happens when
writing images larger than 32 kB: Writing to address 0x8000 actually writes to
0x0000 (i.e. an address mask of 0x7fff).
> 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.

I don't, but I'll try to find out.
> 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.

See my comment regarding the lspci output.
> > > > 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.

Removed in the attached patch.
> > > > Signed-off-by: Joseph C. Lehner
> > >
> > > 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
> > > 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...

Done.
> > > 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.

Removed.
> > > +{
> > > 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.

Done.
> > > +{
> > > 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.

As mentioned in my first mail, there is no documentation at all on these chips apart
from the kernel drivers in Linux and FreeBSD, which obviously don't handle
flash-related stuff. This OUTL/BAR5 combination is also used by PTIFLASH (Promise's
DOS-only flash utility).
> > > +}
> > >
> > > +#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.

Thanks!
> > 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.

I'm wondering if flashrom should maybe provide an option to flash images smaller
than the flash chip's capacity (something along the lines of -P/--partial-flash).
IMHO this would be a much more elegant solution than hacking the chip size.
> Regards,
> Carl-Daniel

Regards,
Joseph

[Joseph C. Lehner]

Just a quick update: A16 and A15 on my Ultra100 are connected to GND, so that
explains the 32 kB limit. I've updated the docs in the source accordingly and
attached the new patch.

On 2016-01-13 13:00, Joseph Lehner wrote:
> Hi Carl-Daniel,
>
> I've updated the patch to address some of the issues you pointed out, namely:
>
> *) Remove the allow32k param
> *) Renaming fixup_chip to limit_chip (and also make it work when NOT using -c
> <flashchip>)
> *) Remove the bridge fixup/break stuff because it's not actually neccessary (turns
> out the bridge was misconfigured because I had to hot-plug the controller, after a
> failed flash prevented my machine from booting).
>
> Regarding your other questions, inline answers follow.
>
> On 2016-01-13 11:48, Carl-Daniel Hailfinger wrote:
> > Hi Joseph,
> >
> > a few followup questions:
> >
> > Could you please supply the output of "lspci -nnvvvxxx" (as root) for
> > the device you tested?
>
> 04:01.0 Mass storage controller [0180]: Promise Technology, Inc. PDC20267
> (FastTrak100/Ultra100) [105a:4d30] (rev 02)
>    Subsystem: Promise Technology, Inc. Ultra100 [105a:4d33]
>    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR-
> FastB2B- DisINTx-
>    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort-
> > SERR- <PERR- INTx-
>
> Latency: 32
>    Interrupt: pin A routed to IRQ 15
>    Region 0: I/O ports at d0f0 [size=8]
>    Region 1: I/O ports at d0e0 [size=4]
>    Region 2: I/O ports at d0d0 [size=8]
>    Region 3: I/O ports at d0c0 [size=4]
>    Region 4: I/O ports at d080 [size=64]
>    Region 5: Memory at df300000 (32-bit, non-prefetchable) [size=128K]
>    Expansion ROM at df340000 [disabled] [size=64K]
>    Capabilities: [58] Power Management version 1
>    	Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
>    	Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
> 00: 5a 10 30 4d 07 00 10 02 02 00 80 01 00 20 00 00
> 10: f1 d0 00 00 e1 d0 00 00 d1 d0 00 00 c1 d0 00 00
> 20: 81 d0 00 00 00 00 30 df 00 00 00 00 5a 10 33 4d
> 30: 00 00 34 df 58 00 00 00 00 00 00 00 0f 01 00 00
> 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 50: ce 3b 00 00 00 00 00 00 01 00 01 00 00 00 00 00
> 60: 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00
> 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>
> Note that BAR5 is listed as having a size of 128 kB, but when reading 128 kB from
> this physical address, the data repeats every 32 kB. The same thing happens when
> writing images larger than 32 kB: Writing to address 0x8000 actually writes to
> 0x0000 (i.e. an address mask of 0x7fff).
> > 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.
>
> I don't, but I'll try to find out.
> > 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.
>
> See my comment regarding the lspci output.
> > > > > 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.
>
> Removed in the attached patch.
> > > > > Signed-off-by: Joseph C. Lehner
> > > >
> > > > 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
> > > > 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...
>
> Done.
> > > > 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.
>
> Removed.
> > > > +{
> > > > 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.
>
> Done.
> > > > +{
> > > > 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.
>
> As mentioned in my first mail, there is no documentation at all on these chips apart
> from the kernel drivers in Linux and FreeBSD, which obviously don't handle
> flash-related stuff. This OUTL/BAR5 combination is also used by PTIFLASH (Promise's
> DOS-only flash utility).
> > > > +}
> > > >
> > > > +#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.
>
> Thanks!
> > > 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.
>
> I'm wondering if flashrom should maybe provide an option to flash images smaller
> than the flash chip's capacity (something along the lines of -P/--partial-flash).
> IMHO this would be a much more elegant solution than hacking the chip size.
> > Regards,
> > Carl-Daniel
>
> Regards,
> Joseph

[Carl-Daniel Hailfinger]

Hi Joseph,

great work!

Your patch would benefit from some changes in general flashrom
infrastructure, but we're pretty busy with our main jobs and won't be
able to implement that soon. Holding your patch back would be
counterproductive, so we've decided to merge it in the next few days.

May I request three changes, though:
- Please default CONFIG_ATAPROMISE to no. This is only temporary until
we have a clean chip size trimming infrastructure.
- Please provide a warning message in atapromise_init similar to the
suggestion below.
- Please write a man page entry for your programmer driver.

Thanks.

On 13.01.2016 14:13, Joseph C. Lehner wrote:
> Just a quick update: A16 and A15 on my Ultra100 are connected to GND, so that
> explains the 32 kB limit. I've updated the docs in the source accordingly and
> attached the new patch.
>
> On 2016-01-13 13:00, Joseph Lehner wrote:
> > > Hi Carl-Daniel,
> > >
> > > I've updated the patch to address some of the issues you pointed out, namely:
> > >
> > > *) Remove the allow32k param
> > > *) Renaming fixup_chip to limit_chip (and also make it work when NOT using -c
> > > <flashchip>)
> > > *) Remove the bridge fixup/break stuff because it's not actually neccessary (turns
> > > out the bridge was misconfigured because I had to hot-plug the controller, after a
> > > failed flash prevented my machine from booting).
> > >
> > > Regarding your other questions, inline answers follow.
> > >
> > > On 2016-01-13 11:48, Carl-Daniel Hailfinger wrote:
> > > > > Hi Joseph,
> > > > >
> > > > > a few followup questions:
> > > > >
> > > > > Could you please supply the output of "lspci -nnvvvxxx" (as root) for
> > > > > the device you tested?
> > >
> > > 04:01.0 Mass storage controller [0180]: Promise Technology, Inc. PDC20267
> > > (FastTrak100/Ultra100) [105a:4d30] (rev 02)
> > >    Subsystem: Promise Technology, Inc. Ultra100 [105a:4d33]
> > >    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR-
> > > FastB2B- DisINTx-
> > >    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort-
> > > > > SERR- <PERR- INTx-
> > >
> > > Latency: 32
> > >    Interrupt: pin A routed to IRQ 15
> > >    Region 0: I/O ports at d0f0 [size=8]
> > >    Region 1: I/O ports at d0e0 [size=4]
> > >    Region 2: I/O ports at d0d0 [size=8]
> > >    Region 3: I/O ports at d0c0 [size=4]
> > >    Region 4: I/O ports at d080 [size=64]
> > >    Region 5: Memory at df300000 (32-bit, non-prefetchable) [size=128K]
> > >    Expansion ROM at df340000 [disabled] [size=64K]
> > >    Capabilities: [58] Power Management version 1
> > >    	Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
> > >    	Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
> > > 00: 5a 10 30 4d 07 00 10 02 02 00 80 01 00 20 00 00
> > > 10: f1 d0 00 00 e1 d0 00 00 d1 d0 00 00 c1 d0 00 00
> > > 20: 81 d0 00 00 00 00 30 df 00 00 00 00 5a 10 33 4d
> > > 30: 00 00 34 df 58 00 00 00 00 00 00 00 0f 01 00 00
> > > 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 50: ce 3b 00 00 00 00 00 00 01 00 01 00 00 00 00 00
> > > 60: 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00
> > > 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >
> > > Note that BAR5 is listed as having a size of 128 kB, but when reading 128 kB from
> > > this physical address, the data repeats every 32 kB. The same thing happens when
> > > writing images larger than 32 kB: Writing to address 0x8000 actually writes to
> > > 0x0000 (i.e. an address mask of 0x7fff).
> > > > > 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.
> > >
> > > I don't, but I'll try to find out.
> > > > > 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.
> > >
> > > See my comment regarding the lspci output.
> > > > > > >>>>> 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.
> > >
> > > Removed in the attached patch.
> > > > > > >>>>> Signed-off-by: Joseph C. Lehner 
> > > > > > >>> 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 
> > > > > > >>> + *
> > > > > > >>> + * 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...
> > >
> > > Done.
> > > > > > >
> > > > > > >
> > > > > > >>> + *
> > > > > > >>> + * 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.
> > >
> > > Removed.
> > > > > > >
> > > > > > >>> +{
> > > > > > >>> +	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.
> > > > > > >
> > >
> > > Done.
> > > > > > >
> > > > > > >>> +{
> > > > > > >>> +	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.
> > >
> > > As mentioned in my first mail, there is no documentation at all on these chips apart
> > > from the kernel drivers in Linux and FreeBSD, which obviously don't handle
> > > flash-related stuff. This OUTL/BAR5 combination is also used by PTIFLASH (Promise's
> > > DOS-only flash utility).
> > > > > > >>> +}
> > > > > > >>> +
> > > > > > >>> +#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.
> > >
> > > Thanks!
> > > > > > >
> > > > > > > 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.
> > >
> > > I'm wondering if flashrom should maybe provide an option to flash images smaller
> > > than the flash chip's capacity (something along the lines of -P/--partial-flash).
> > > IMHO this would be a much more elegant solution than hacking the chip size.
> > > > > Regards,
> > > > > Carl-Daniel
> > >
> > > Regards,
> > > Joseph
>
> atapromise.patch.txt
>
> 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..55f568e
> --- /dev/null
> +++ b/atapromise.c
> @@ -0,0 +1,182 @@
> [...]
> +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;
> 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;
> }
> rom_size = dev->rom_size > MAX_ROM_DECODE ? MAX_ROM_DECODE : dev->rom_size;
> atapromise_bar = (uint8_t*)rphysmap("Promise", rom_base_addr, rom_size);
> if (atapromise_bar == ERROR_PTR) {
> return 1;
> }
> max_rom_decode.parallel = rom_size;

Something like this (80 column wrap for the output please)
msg_pwarn("This device may not be used as a generic programmer because
it will leave anything outside the first 32 kB of the flash chip in an
undefined state. It works fine for the purpose of updating the firmware
of this device.\n");
> register_par_master(&par_master_atapromise, BUS_PARALLEL);
> return 0;
>
> +}

Regards,
Carl-Daniel

[Joseph C. Lehner]

Hi Carl-Daniel,

I've addressed the issues you pointed out. Thanks for your input,
here is the updated patch.

Regards
Joseph

On 2016-01-13 22:53, Carl-Daniel Hailfinger wrote:
> Hi Joseph,
>
> great work!
>
> Your patch would benefit from some changes in general flashrom
> infrastructure, but we're pretty busy with our main jobs and won't be
> able to implement that soon. Holding your patch back would be
> counterproductive, so we've decided to merge it in the next few days.
>
> May I request three changes, though:
> Please default CONFIG_ATAPROMISE to no. This is only temporary until
>
> we have a clean chip size trimming infrastructure.
> Please provide a warning message in atapromise_init similar to the
>
> suggestion below.
> Please write a man page entry for your programmer driver.
>
> Thanks.
>
> On 13.01.2016 14:13, Joseph C. Lehner wrote:
> > Just a quick update: A16 and A15 on my Ultra100 are connected to GND, so that
> > explains the 32 kB limit. I've updated the docs in the source accordingly and
> > attached the new patch.
> >
> > On 2016-01-13 13:00, Joseph Lehner wrote:
> > > > Hi Carl-Daniel,
> > > >
> > > > I've updated the patch to address some of the issues you pointed out, namely:
> > > >
> > > > *) Remove the allow32k param
> > > > *) Renaming fixup_chip to limit_chip (and also make it work when NOT using -c
> > > > <flashchip>)
> > > > *) Remove the bridge fixup/break stuff because it's not actually neccessary (turns
> > > > out the bridge was misconfigured because I had to hot-plug the controller, after a
> > > > failed flash prevented my machine from booting).
> > > >
> > > > Regarding your other questions, inline answers follow.
> > > >
> > > > On 2016-01-13 11:48, Carl-Daniel Hailfinger wrote:
> > > > > > Hi Joseph,
> > > > > >
> > > > > > a few followup questions:
> > > > > >
> > > > > > Could you please supply the output of "lspci -nnvvvxxx" (as root) for
> > > > > > the device you tested?
> > > >
> > > > 04:01.0 Mass storage controller [0180]: Promise Technology, Inc. PDC20267
> > > > (FastTrak100/Ultra100) [105a:4d30] (rev 02)
> > > >    Subsystem: Promise Technology, Inc. Ultra100 [105a:4d33]
> > > >    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR-
> > > > FastB2B- DisINTx-
> > > >    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort-
> > > > > > SERR- <PERR- INTx-
> > > >
> > > > Latency: 32
> > > >    Interrupt: pin A routed to IRQ 15
> > > >    Region 0: I/O ports at d0f0 [size=8]
> > > >    Region 1: I/O ports at d0e0 [size=4]
> > > >    Region 2: I/O ports at d0d0 [size=8]
> > > >    Region 3: I/O ports at d0c0 [size=4]
> > > >    Region 4: I/O ports at d080 [size=64]
> > > >    Region 5: Memory at df300000 (32-bit, non-prefetchable) [size=128K]
> > > >    Expansion ROM at df340000 [disabled] [size=64K]
> > > >    Capabilities: [58] Power Management version 1
> > > >    	Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
> > > >    	Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
> > > > 00: 5a 10 30 4d 07 00 10 02 02 00 80 01 00 20 00 00
> > > > 10: f1 d0 00 00 e1 d0 00 00 d1 d0 00 00 c1 d0 00 00
> > > > 20: 81 d0 00 00 00 00 30 df 00 00 00 00 5a 10 33 4d
> > > > 30: 00 00 34 df 58 00 00 00 00 00 00 00 0f 01 00 00
> > > > 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 50: ce 3b 00 00 00 00 00 00 01 00 01 00 00 00 00 00
> > > > 60: 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00 04 f3 4f 00
> > > > 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > >
> > > > Note that BAR5 is listed as having a size of 128 kB, but when reading 128 kB from
> > > > this physical address, the data repeats every 32 kB. The same thing happens when
> > > > writing images larger than 32 kB: Writing to address 0x8000 actually writes to
> > > > 0x0000 (i.e. an address mask of 0x7fff).
> > > > > > 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.
> > > >
> > > > I don't, but I'll try to find out.
> > > > > > 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.
> > > >
> > > > See my comment regarding the lspci output.
> > > > > > >>>>>> 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.
> > > >
> > > > Removed in the attached patch.
> > > > > > >>>>>> Signed-off-by: Joseph C. Lehner 
> > > > > > >>>> 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 
> > > > > > >>>> + *
> > > > > > >>>> + * 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...
> > > >
> > > > Done.
> > > > > > >>
> > > > > > >>
> > > > > > >>>> + *
> > > > > > >>>> + * 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.
> > > >
> > > > Removed.
> > > > > > >>
> > > > > > >>>> +{
> > > > > > >>>> +	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.
> > > > > > >>
> > > >
> > > > Done.
> > > > > > >>
> > > > > > >>>> +{
> > > > > > >>>> +	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.
> > > >
> > > > As mentioned in my first mail, there is no documentation at all on these chips apart
> > > > from the kernel drivers in Linux and FreeBSD, which obviously don't handle
> > > > flash-related stuff. This OUTL/BAR5 combination is also used by PTIFLASH (Promise's
> > > > DOS-only flash utility).
> > > > > > >>>> +}
> > > > > > >>>> +
> > > > > > >>>> +#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.
> > > >
> > > > Thanks!
> > > > > > >>
> > > > > > >> 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.
> > > >
> > > > I'm wondering if flashrom should maybe provide an option to flash images smaller
> > > > than the flash chip's capacity (something along the lines of -P/--partial-flash).
> > > > IMHO this would be a much more elegant solution than hacking the chip size.
> > > > > > Regards,
> > > > > > Carl-Daniel
> > > >
> > > > Regards,
> > > > Joseph
> >
> > atapromise.patch.txt
> >
> > 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..55f568e
> > --- /dev/null
> > +++ b/atapromise.c
> > @@ -0,0 +1,182 @@
> > [...]
> > +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;
> > 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;
> > }
> > rom_size = dev->rom_size > MAX_ROM_DECODE ? MAX_ROM_DECODE : dev->rom_size;
> > atapromise_bar = (uint8_t*)rphysmap("Promise", rom_base_addr, rom_size);
> > if (atapromise_bar == ERROR_PTR) {
> > return 1;
> > }
> > max_rom_decode.parallel = rom_size;
>
> Something like this (80 column wrap for the output please)
> msg_pwarn("This device may not be used as a generic programmer because
> it will leave anything outside the first 32 kB of the flash chip in an
> undefined state. It works fine for the purpose of updating the firmware
> of this device.\n");
> > register_par_master(&par_master_atapromise, BUS_PARALLEL);
> > return 0;
> >
> > +}
>
> Regards,
> Carl-Daniel

[Carl-Daniel Hailfinger]

Hi Joseph,

thank you very much. I have added the requirements section to the man
page and changed some whitespace and a comment in atapromise.c.

Patch follows at the end of this mail. If you don't have any objections,
may I ask you to sign off on that latest iteration, then I'll commit.
Thanks again for submitting the patch and for addressing all our concerns.

Regards,
Carl-Daniel

On 14.01.2016 13:23, Joseph C. Lehner wrote:
> I've addressed the issues you pointed out. Thanks for your input,
> here is the updated patch.
>
> On 2016-01-13 22:53, Carl-Daniel Hailfinger wrote:
> > Hi Joseph,
> >
> > great work!
> >
> > Your patch would benefit from some changes in general flashrom
> > infrastructure, but we're pretty busy with our main jobs and won't be
> > able to implement that soon. Holding your patch back would be
> > counterproductive, so we've decided to merge it in the next few days.
> >
> > May I request three changes, though:
> > Please default CONFIG_ATAPROMISE to no. This is only temporary until
> >
> > we have a clean chip size trimming infrastructure.
> > Please provide a warning message in atapromise_init similar to the
> >
> > suggestion below.
> > Please write a man page entry for your programmer driver.
> >
> > Thanks.
> >
> > On 13.01.2016 14:13, Joseph C. Lehner wrote:
> > > Just a quick update: A16 and A15 on my Ultra100 are connected to GND, so that
> > > explains the 32 kB limit. I've updated the docs in the source accordingly and
> > > attached the new patch.

Index: flashrom-atapromise/Makefile
===================================================================
--- flashrom-atapromise/Makefile	(Revision 1915)
+++ flashrom-atapromise/Makefile	(Arbeitskopie)
@@ -227,6 +227,11 @@
 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
@@ -449,6 +454,9 @@
 # VIA VT6421A LPC memory support
 CONFIG_ATAVIA ?= yes

+# Promise ATA controller support.
+CONFIG_ATAPROMISE ?= no
+
 # Always enable FT2232 SPI dongles for now.
 CONFIG_FT2232_SPI ?= yes

@@ -626,6 +634,12 @@
 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
Index: flashrom-atapromise/atapromise.c
===================================================================
--- flashrom-atapromise/atapromise.c	(Revision 0)
+++ flashrom-atapromise/atapromise.c	(Arbeitskopie)
@@ -0,0 +1,177 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright (C) 2015 Joseph C. Lehner 
+ *
+ * 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.
+ *
+ * While the tested Ultra100 controller used a 128 kB MX29F001T chip, A16 and A15 showed continuity to ground,
+ * thus limiting the the programmer on this card to 32 kB. Without other controllers to test this programmer on,
+ * this is currently a hard limit. Note that ROM files for these controllers are 16 kB only.
+ *
+ * Since flashrom does not support accessing flash chips larger than the size limit of the programmer (the
+ * tested Ultra100 uses a 128 kB MX29F001T chip), the chip size is hackishly adjusted in atapromise_limit_chip.
+ */
+
+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 void atapromise_limit_chip(struct flashchip *chip)
+{
+	static uint32_t last_model_id = 0;
+	unsigned int i, size;
+
+	if (chip->model_id == last_model_id)
+		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_pdbg("Failed to adjust size of chip "%s" (%d kB).\n", chip->name,
+				 chip->total_size);
+		}
+	}
+
+	last_model_id = chip->model_id;
+}
+
+int atapromise_init(void)
+{
+	struct pci_dev *dev = NULL;
+
+	if (rget_io_perms())
+		return 1;
+
+	dev = pcidev_init(ata_promise, PCI_BASE_ADDRESS_4);
+	if (!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;
+	}
+
+	rom_size = dev->rom_size > MAX_ROM_DECODE ? MAX_ROM_DECODE : dev->rom_size;
+	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);
+
+	msg_pwarn("Do not use this device as a generic programmer. It will leave anything outside\n"
+		  "the first %zu kB of the flash chip in an undefined state. It works fine for the\n"
+		  "purpose of updating the firmware of this device (padding may neccessary).\n",
+		  rom_size / 1024);
+
+	return 0;
+}
+
+static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr)
+{
+	uint32_t data;
+
+	atapromise_limit_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_limit_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
Index: flashrom-atapromise/flashrom.8.tmpl
===================================================================
--- flashrom-atapromise/flashrom.8.tmpl	(Revision 1915)
+++ flashrom-atapromise/flashrom.8.tmpl	(Arbeitskopie)
@@ -233,6 +233,8 @@
 .sp
 .BR "* atavia" " (for flash ROMs on VIA VT6421A SATA controllers)"
 .sp
+.BR "* atapromise" " (for flash ROMs on Promise PDC2026x ATA/RAID controllers)"
+.sp
 .BR "* it8212" " (for flash ROMs on ITE IT8212F ATA/RAID controller)"
 .sp
 .BR "* ft2232_spi" " (for SPI flash ROMs attached to an FT2232/FT4232H/FT232H family \
@@ -636,7 +638,7 @@
 .SS
 .BR "nic3com" , " nicrealtek" , " nicnatsemi" , " nicintel", " nicintel_eeprom"\
 , " nicintel_spi" , " gfxnvidia" , " ogp_spi" , " drkaiser" , " satasii"\
-, " satamv" , " atahpt", " atavia " and " it8212 " programmers
+, " satamv" , " atahpt", " atavia ", " atapromise " and " it8212 " programmers
 .IP
 These programmers have an option to specify the PCI address of the card
 your want to use, which must be specified if more than one card supported
@@ -669,6 +671,13 @@
 For more information please see
 .URLB https://flashrom.org/VT6421A "its wiki page" .
 .SS
+.BR "atapromise " programmer
+.IP
+This programmer is currently limited to 32 kB, regardless of the actual size of the flash chip. This stems
+from the fact that, on the tested device (a Promise Ultra100), not all of the chip's address lines were
+actually connected. You may use this programmer to flash firmware updates, since these are only 16 kB in
+size (padding to 32 kB is required).
+.SS
 .BR "nicintel_eeprom " programmer
 .IP
 This is the first programmer module in flashrom that does not provide access to NOR flash chips but EEPROMs
@@ -1059,8 +1068,8 @@
 .BR satasii ", " nicintel ", " nicintel_eeprom " and " nicintel_spi
 need PCI configuration space read access and raw memory access.
 .sp
-.B satamv
-needs PCI configuration space read access, raw I/O port access and raw memory
+.BR satamv " and " atapromise
+need PCI configuration space read access, raw I/O port access and raw memory
 access.
 .sp
 .B serprog
@@ -1076,7 +1085,7 @@
 needs no access permissions at all.
 .sp
 .BR internal ", " nic3com ", " nicrealtek ", " nicnatsemi ", "
-.BR gfxnvidia ", " drkaiser ", " satasii ", " satamv ", " atahpt" and " atavia
+.BR gfxnvidia ", " drkaiser ", " satasii ", " satamv ", " atahpt ", " atavia " and " atapromise
 have to be run as superuser/root, and need additional raw access permission.
 .sp
 .BR serprog ", " buspirate_spi ", " dediprog ", " usbblaster_spi ", " ft2232_spi " and " pickit2_spi
Index: flashrom-atapromise/flashrom.c
===================================================================
--- flashrom-atapromise/flashrom.c	(Revision 1915)
+++ flashrom-atapromise/flashrom.c	(Arbeitskopie)
@@ -182,6 +182,18 @@
    },
 #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",
Index: flashrom-atapromise/programmer.h
===================================================================
--- flashrom-atapromise/programmer.h	(Revision 1915)
+++ flashrom-atapromise/programmer.h	(Arbeitskopie)
@@ -57,6 +57,9 @@
 #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 @@
 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);

[Joseph C. Lehner]

Hi Carl-Daniel,

looks fine to me, thank you for your efforts!

Regards,
Joseph

Signed-off-by: Joseph C. Lehner

On 2016-01-16 23:35, Carl-Daniel Hailfinger wrote:
> Hi Joseph,
>
> thank you very much. I have added the requirements section to the man
> page and changed some whitespace and a comment in atapromise.c.
>
> Patch follows at the end of this mail. If you don't have any objections,
> may I ask you to sign off on that latest iteration, then I'll commit.
> Thanks again for submitting the patch and for addressing all our concerns.
>
> Regards,
> Carl-Daniel
> Index: flashrom-atapromise/Makefile
>
> --- flashrom-atapromise/Makefile	(Revision 1915)
> +++ flashrom-atapromise/Makefile	(Arbeitskopie)
> @@ -227,6 +227,11 @@
>  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
> @@ -449,6 +454,9 @@
>  # VIA VT6421A LPC memory support
>  CONFIG_ATAVIA ?= yes
>
> +# Promise ATA controller support.
> +CONFIG_ATAPROMISE ?= no
>
> # Always enable FT2232 SPI dongles for now.
>  CONFIG_FT2232_SPI ?= yes
>
> @@ -626,6 +634,12 @@
>  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
> Index: flashrom-atapromise/atapromise.c
> ===================================================================
> --- flashrom-atapromise/atapromise.c	(Revision 0)
> +++ flashrom-atapromise/atapromise.c	(Arbeitskopie)
> @@ -0,0 +1,177 @@
> +/*
> This file is part of the flashrom project.
> Copyright (C) 2015 Joseph C. Lehner
> 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.
> While the tested Ultra100 controller used a 128 kB MX29F001T chip, A16 and A15 showed continuity to ground,
> thus limiting the the programmer on this card to 32 kB. Without other controllers to test this programmer on,
> this is currently a hard limit. Note that ROM files for these controllers are 16 kB only.
> Since flashrom does not support accessing flash chips larger than the size limit of the programmer (the
> tested Ultra100 uses a 128 kB MX29F001T chip), the chip size is hackishly adjusted in atapromise_limit_chip.
> */
>
> +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 void atapromise_limit_chip(struct flashchip *chip)
> +{
> static uint32_t last_model_id = 0;
> unsigned int i, size;
> if (chip->model_id == last_model_id)
> 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_pdbg("Failed to adjust size of chip \"%s\" (%d kB).\n", chip->name,
> 		 chip->total_size);
> }
> }
> last_model_id = chip->model_id;
>
> +}
>
> +int atapromise_init(void)
> +{
> struct pci_dev *dev = NULL;
> if (rget_io_perms())
> return 1;
> dev = pcidev_init(ata_promise, PCI_BASE_ADDRESS_4);
> if (!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;
> }
> rom_size = dev->rom_size > MAX_ROM_DECODE ? MAX_ROM_DECODE : dev->rom_size;
> 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);
> msg_pwarn("Do not use this device as a generic programmer. It will leave anything outside\n"
>   "the first %zu kB of the flash chip in an undefined state. It works fine for the\n"
>   "purpose of updating the firmware of this device (padding may neccessary).\n",
>   rom_size / 1024);
> return 0;
>
> +}
>
> +static void atapromise_chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr)
> +{
> uint32_t data;
> atapromise_limit_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_limit_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
> Index: flashrom-atapromise/flashrom.8.tmpl
> ===================================================================
> --- flashrom-atapromise/flashrom.8.tmpl	(Revision 1915)
> +++ flashrom-atapromise/flashrom.8.tmpl	(Arbeitskopie)
> @@ -233,6 +233,8 @@
>  .sp
>  .BR "* atavia" " (for flash ROMs on VIA VT6421A SATA controllers)"
>  .sp
> +.BR "* atapromise" " (for flash ROMs on Promise PDC2026x ATA/RAID controllers)"
> +.sp
>  .BR "* it8212" " (for flash ROMs on ITE IT8212F ATA/RAID controller)"
>  .sp
>  .BR "* ft2232_spi" " (for SPI flash ROMs attached to an FT2232/FT4232H/FT232H family \
> @@ -636,7 +638,7 @@
>  .SS
>  .BR "nic3com" , " nicrealtek" , " nicnatsemi" , " nicintel", " nicintel_eeprom"\
>  , " nicintel_spi" , " gfxnvidia" , " ogp_spi" , " drkaiser" , " satasii"\
> -, " satamv" , " atahpt", " atavia " and " it8212 " programmers
> +, " satamv" , " atahpt", " atavia ", " atapromise " and " it8212 " programmers
>  .IP
>  These programmers have an option to specify the PCI address of the card
>  your want to use, which must be specified if more than one card supported
> @@ -669,6 +671,13 @@
>  For more information please see
>  .URLB https://flashrom.org/VT6421A "its wiki page" .
>  .SS
> +.BR "atapromise " programmer
> +.IP
> +This programmer is currently limited to 32 kB, regardless of the actual size of the flash chip. This stems
> +from the fact that, on the tested device (a Promise Ultra100), not all of the chip's address lines were
> +actually connected. You may use this programmer to flash firmware updates, since these are only 16 kB in
> +size (padding to 32 kB is required).
> +.SS
>  .BR "nicintel_eeprom " programmer
>  .IP
>  This is the first programmer module in flashrom that does not provide access to NOR flash chips but EEPROMs
> @@ -1059,8 +1068,8 @@
>  .BR satasii ", " nicintel ", " nicintel_eeprom " and " nicintel_spi
>  need PCI configuration space read access and raw memory access.
>  .sp
> -.B satamv
> -needs PCI configuration space read access, raw I/O port access and raw memory
> +.BR satamv " and " atapromise
> +need PCI configuration space read access, raw I/O port access and raw memory
>  access.
>  .sp
>  .B serprog
> @@ -1076,7 +1085,7 @@
>  needs no access permissions at all.
>  .sp
>  .BR internal ", " nic3com ", " nicrealtek ", " nicnatsemi ", "
> -.BR gfxnvidia ", " drkaiser ", " satasii ", " satamv ", " atahpt" and " atavia
> +.BR gfxnvidia ", " drkaiser ", " satasii ", " satamv ", " atahpt ", " atavia " and " atapromise
>  have to be run as superuser/root, and need additional raw access permission.
>  .sp
>  .BR serprog ", " buspirate_spi ", " dediprog ", " usbblaster_spi ", " ft2232_spi " and " pickit2_spi
> Index: flashrom-atapromise/flashrom.c
> ===================================================================
> --- flashrom-atapromise/flashrom.c	(Revision 1915)
> +++ flashrom-atapromise/flashrom.c	(Arbeitskopie)
> @@ -182,6 +182,18 @@
>    },
>  #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",
> Index: flashrom-atapromise/programmer.h
> ===================================================================
> --- flashrom-atapromise/programmer.h	(Revision 1915)
> +++ flashrom-atapromise/programmer.h	(Arbeitskopie)
> @@ -57,6 +57,9 @@
>  #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 @@
>  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);

[Carl-Daniel Hailfinger]

Hi Joseph,

thanks!
Acked-by: Carl-Daniel Hailfinger 
and committed in r1916 with the following commit message:

Add atapromise programmer

Supported controllers are Promise PDC20262 (FastTrak66/Ultra66),
PDC20265 (FastTrak100 Lite/Ultra100), PDC20267 (FastTrak100/Ultra100).
At least the Ultra100 only has address lines A0-A14 wired up, limiting
addressable chip size to 32 kB. The flash chips mounted on those
controllers usually is 128 kB, i.e. parts of the flash chip are
inaccessible. As a workaround, the driver implicitly truncates the
size of all flash chips to 32 kB. Works well for the factory installed
flash.
Do NOT use as a generic programmer for chips >32 kB.

Signed-off-by: Joseph C. Lehner 
Acked-by: Carl-Daniel Hailfinger 
Acked-by: Urja Rannikko

Regards,
Carl-Daniel

On 16.01.2016 23:41, Joseph C. Lehner wrote:
> Hi Carl-Daniel,
>
> looks fine to me, thank you for your efforts!
>
> Regards,
> Joseph
>
> Signed-off-by: Joseph C. Lehner
>
> On 2016-01-16 23:35, Carl-Daniel Hailfinger wrote:
> > > Hi Joseph,
> > >
> > > thank you very much. I have added the requirements section to the man
> > > page and changed some whitespace and a comment in atapromise.c.
> > >
> > > Patch follows at the end of this mail. If you don't have any objections,
> > > may I ask you to sign off on that latest iteration, then I'll commit.
> > > Thanks again for submitting the patch and for addressing all our concerns.
> > >
> > > Regards,
> > > Carl-Daniel
> > >
> > > Index: flashrom-atapromise/Makefile

[Carl-Daniel Hailfinger]

Hi Joseph,

side note: Urja added his Acked-by on IRC.

Regards,
Carl-Daniel

On 17.01.2016 00:46, Carl-Daniel Hailfinger wrote:
> Hi Joseph,
>
> thanks!
> Acked-by: Carl-Daniel Hailfinger 
> and committed in r1916 with the following commit message:
>
> Add atapromise programmer
>
> Supported controllers are Promise PDC20262 (FastTrak66/Ultra66),
> PDC20265 (FastTrak100 Lite/Ultra100), PDC20267 (FastTrak100/Ultra100).
> At least the Ultra100 only has address lines A0-A14 wired up, limiting
> addressable chip size to 32 kB. The flash chips mounted on those
> controllers usually is 128 kB, i.e. parts of the flash chip are
> inaccessible. As a workaround, the driver implicitly truncates the
> size of all flash chips to 32 kB. Works well for the factory installed
> flash.
> Do NOT use as a generic programmer for chips >32 kB.
>
> Signed-off-by: Joseph C. Lehner 
> Acked-by: Carl-Daniel Hailfinger 
> Acked-by: Urja Rannikko
>
> Regards,
> Carl-Daniel