[flashrom] [PATCH] Add Suport for Dr.Kaiser PC-Waechter

[Carl-Daniel Hailfinger]

Hi,

On 03.09.2009 01:12, Uwe Hermann wrote:
> On Wed, Sep 02, 2009 at 09:01:01PM +0200, TURBO J wrote:
>   
>> this patch adds support for a PCI "flash memory card" named "Dr. Kaiser
>> PC-Waechter".
>>     
>
> Great, thanks a lot for the patch!
>
> I committed it in r712 with some smaller changes.
>  - Small whitespace and coding style fixes.
>  - Made PCI_BASE_ADDRESS_2 support a bit more generic, see below.
>  - Added programmer to the manpage.
>   

Thanks!


>> +	// Map 128KB Flash Memory Window
>> +	drkaiser_mem=physmap("Dr.Kaiser PC-Waechter FLASH Memory",
>> +	pci_read_long(pcidev_dev,PCI_BASE_ADDRESS_2), 128*1024);
>>     
>
> Are you sure 128 KB is the maximum supported size here? The chip on your
> board is 128KB, but maybe if bigger chips were soldered on instead they
> would still work (depends on the FPGA, hardware wiring etc, I guess)?
>   

Interesting question. The peculiar flash chip mapping has a few other
side effects, though:

> +void drkaiser_chip_writeb(uint8_t val, chipaddr addr)
> +{
> +	mmio_writeb(val, drkaiser_bar + addr);
> +}
> +
> +uint8_t drkaiser_chip_readb(const chipaddr addr)
> +{
> +	return mmio_readb(drkaiser_bar + addr);
> +}
>   

Please note that addr is _not_ relative to chip start. For any given
chip, the addr of the lowest byte of the chip is 0x100000000-chipsize.
This means your accessor functions have to implement bounds checking and
either error out or wrap around. Example for wrapping around:

void drkaiser_chip_writeb(uint8_t val, chipaddr addr)
{
	mmio_writeb(val, (drkaiser_bar + addr) & ((1 << 17) - 1);
}


Example for aborting:

void drkaiser_chip_writeb(uint8_t val, chipaddr addr)
{
	/* chipaddr is unsigned */
	if (addr > (1 << 17)) {
		/* Bitch and complain and abort. */
		fprintf(stderr, "Waah! Accessing outside the BAR, addr=%08lx.\n",
			chipaddr);
		/* Maybe exit(1) when we're sure this will not trigger. */
		return;
	}
	mmio_writeb(val, (drkaiser_bar + addr) & ((1 << 17) - 1);
}


Otherwise we risk writing to and reading from arbitrary RAM/MMIO regions...

There is another problem in the generic PCI code, but that's not your
fault: We don't check if the BAR is 32bit or 64bit. That may cause some
problems on systems with 64bit address space which place device
resources above 4G.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/