Patchwork [RFC] National Semiconductor DP83815 NIC patch

login
register
about
Submitter Andrew Morgan
Date 2010-06-08 00:29:01
Message ID <4C0D8ECD.6000401@ziltro.com>
Download mbox | patch
Permalink /patch/1486/
State Accepted
Commit r1095
Headers show

Comments

Andrew Morgan - 2010-06-08 00:29:01
On 07/06/10 23:47, Carl-Daniel Hailfinger wrote:
> Are you sure? AFAICS your code can't support more than 64 kB because it
> truncates the address to 16 bits. Due to that, it should definitely set
> max_rom_decode.parallel. You can try changing the address mask, and if
> that give you good readbacks, you can still increase the size. However,
> in the end every programmer with parallel flash has to set this limit to
> make flashing safe for users.
>    

Good point. :)
I have set it to 128K now, see comment in patch. I hope the comment is ok.

> TODO:
> Please send a patch which sets max_rom_decode.parallel to a size which
> makes sense (i.e. 65536 with the current code) and please add printing
> of the programmer PCI devices to print.c and print_wiki.c.
> It would be cool if you could add some info to the man page as well.
> Just copy and paste from an existing programmer there.
Done with the exception of the man page, as CONFIG_NICNATSEMI is off by 
default it wouldn't make sense to be in the man page yet, and I don't 
really know the syntax. I could probably just copy '...nicrealtek...' 
like I have done in other places though... ;)

print.c doesn't pad the PCI bus IDs: (0020/0022)
National Semiconductor DP83815/DP83816 [100b:20] (untested)
National Semiconductor DP83820 [100b:22] (untested)

The attached patch adds nicnatsemi to print.c and print_wiki.c, changes 
the address mask to use MA0-MA16 and sets the maximum decode size to 128KB.

Signed-off-by Andrew Morgan <ziltro@ziltro.com>
Andrew Morgan - 2010-06-08 01:29:59
On 08/06/10 01:29, Andrew Morgan wrote:
> I have set it to 128K now, see comment in patch. I hope the comment is 
> ok.
>
The 128KB comment might not be needed any more. It seems that 128KB is 
the correct maximum size.
Using a multimeter I found that the voltage on A16 does change during a 
read. The other thing I found is that the network card uses 3.3v for the 
boot ROM, not 5v! As I have been doing all my tests with a 5v±10% 256KB 
flash chip, and one of the address lines was floating, I'm not surprised 
there was random data being read. (I am surprised that erase seemed to 
work.)

So now all I need to do is find a 128KB 3.3v DIP32 parallel flash chip...
Marko Kraljevic - 2010-06-09 01:24:25
Hi Andrew,
This probably isn't too important, Most (all?) parallel flash can jive
with TTL levels (ie. >= 2V is high, <= 0,8V is low, in-between is
garbage).

eg. this datasheet (29F002) - see the Vil and Vih specifications.
http://www.datasheetcatalog.org/datasheets/150/489768_DS.pdf

However, it is essential that VCC be 5V, give or take. 3,3V will cause
errors. Are you saying Vcc is 3,3V? Or just the data/addr lines?

-Mark

On Mon, Jun 7, 2010 at 7:29 PM, Andrew Morgan <ziltro@ziltro.com> wrote:
> On 08/06/10 01:29, Andrew Morgan wrote:
>>
>> I have set it to 128K now, see comment in patch. I hope the comment is ok.
>>
> The 128KB comment might not be needed any more. It seems that 128KB is the
> correct maximum size.
> Using a multimeter I found that the voltage on A16 does change during a
> read. The other thing I found is that the network card uses 3.3v for the
> boot ROM, not 5v! As I have been doing all my tests with a 5v±10% 256KB
> flash chip, and one of the address lines was floating, I'm not surprised
> there was random data being read. (I am surprised that erase seemed to
> work.)
>
> So now all I need to do is find a 128KB 3.3v DIP32 parallel flash chip...
>
> --
>
> Andrew.
>
>
> _______________________________________________
> flashrom mailing list
> flashrom@flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
Andrew Morgan - 2010-06-09 13:47:06
On 09/06/10 02:24, Marko Kraljevic wrote:
> Hi Andrew,
> This probably isn't too important, Most (all?) parallel flash can jive
> with TTL levels (ie.>= 2V is high,<= 0,8V is low, in-between is
> garbage).
>
> eg. this datasheet (29F002) - see the Vil and Vih specifications.
> http://www.datasheetcatalog.org/datasheets/150/489768_DS.pdf
>
> However, it is essential that VCC be 5V, give or take. 3,3V will cause
> errors. Are you saying Vcc is 3,3V? Or just the data/addr lines?
>    
Vcc is 3.3v!
I wonder if that's why they didn't put a socket on the board... :)
Carl-Daniel Hailfinger - 2010-07-21 15:12:03
Hi Andrew,

thanks for your patch.

On 08.06.2010 02:29, Andrew Morgan wrote:
> On 07/06/10 23:47, Carl-Daniel Hailfinger wrote:
>> Are you sure? AFAICS your code can't support more than 64 kB because it
>> truncates the address to 16 bits. Due to that, it should definitely set
>> max_rom_decode.parallel. You can try changing the address mask, and if
>> that give you good readbacks, you can still increase the size. However,
>> in the end every programmer with parallel flash has to set this limit to
>> make flashing safe for users.
>>    
>
> Good point. :)
> I have set it to 128K now, see comment in patch. I hope the comment is
> ok.
>
>> TODO:
>> Please send a patch which sets max_rom_decode.parallel to a size which
>> makes sense (i.e. 65536 with the current code) and please add printing
>> of the programmer PCI devices to print.c and print_wiki.c.
>> It would be cool if you could add some info to the man page as well.
>> Just copy and paste from an existing programmer there.
> Done with the exception of the man page, as CONFIG_NICNATSEMI is off
> by default it wouldn't make sense to be in the man page yet, and I
> don't really know the syntax. I could probably just copy
> '...nicrealtek...' like I have done in other places though... ;)
>
> print.c doesn't pad the PCI bus IDs: (0020/0022)
> National Semiconductor DP83815/DP83816 [100b:20] (untested)
> National Semiconductor DP83820 [100b:22] (untested)
>
> The attached patch adds nicnatsemi to print.c and print_wiki.c,
> changes the address mask to use MA0-MA16 and sets the maximum decode
> size to 128KB.
>
> Signed-off-by Andrew Morgan <ziltro@ziltro.com>

Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
and committed in r1095.

Regards,
Carl-Daniel

Patch

Index: print_wiki.c
===================================================================
--- print_wiki.c	(revision 1039)
+++ print_wiki.c	(working copy)
@@ -280,6 +280,9 @@ 
 	print_supported_pcidevs_wiki(nics_realtek);
 	print_supported_pcidevs_wiki(nics_realteksmc1211);
 #endif
+#if CONFIG_NICNATSEMI == 1
+	print_supported_pcidevs_wiki(nics_natsemi);
+#endif
 #if CONFIG_GFXNVIDIA == 1
 	print_supported_pcidevs_wiki(gfx_nvidia);
 #endif
Index: nicnatsemi.c
===================================================================
--- nicnatsemi.c	(revision 1039)
+++ nicnatsemi.c	(working copy)
@@ -43,6 +43,14 @@ 
 
 	buses_supported = CHIP_BUSTYPE_PARALLEL;
 
+	/* The datasheet shows address lines MA0-MA16 in one place and MA0-MA15
+	 * in another. My NIC has MA16 connected to A16 on the boot ROM socket
+	 * so I'm assuming it is accessible. If not then next line wants to be
+	 * max_rom_decode.parallel = 65536; and the mask in the read/write
+	 * functions below wants to be 0x0000FFFF.
+	 */
+	max_rom_decode.parallel = 131072;
+
 	return 0;
 }
 
@@ -56,7 +64,7 @@ 
 
 void nicnatsemi_chip_writeb(uint8_t val, chipaddr addr)
 {
-	OUTL((uint32_t)addr & 0x0000FFFF, io_base_addr + BOOT_ROM_ADDR);
+	OUTL((uint32_t)addr & 0x0001FFFF, io_base_addr + BOOT_ROM_ADDR);
 	/*
 	 * The datasheet requires 32 bit accesses to this register, but it seems
 	 * that requirement might only apply if the register is memory mapped.
@@ -70,7 +78,7 @@ 
 
 uint8_t nicnatsemi_chip_readb(const chipaddr addr)
 {
-	OUTL(((uint32_t)addr & 0x0000FFFF), io_base_addr + BOOT_ROM_ADDR);
+	OUTL(((uint32_t)addr & 0x0001FFFF), io_base_addr + BOOT_ROM_ADDR);
 	/*
 	 * The datasheet requires 32 bit accesses to this register, but it seems
 	 * that requirement might only apply if the register is memory mapped.
Index: print.c
===================================================================
--- print.c	(revision 1039)
+++ print.c	(working copy)
@@ -219,7 +219,7 @@ 
 		print_supported_boards_helper(boards_known, "boards");
 		print_supported_boards_helper(laptops_known, "laptops");
 #endif
-#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT >= 1
+#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT >= 1
 		printf("\nSupported PCI devices flashrom can use "
 		       "as programmer:\n\n");
 #endif
@@ -230,6 +230,9 @@ 
 		print_supported_pcidevs(nics_realtek);
 		print_supported_pcidevs(nics_realteksmc1211);
 #endif
+#if CONFIG_NICNATSEMI == 1
+		print_supported_pcidevs(nics_natsemi);
+#endif
 #if CONFIG_GFXNVIDIA == 1
 		print_supported_pcidevs(gfx_nvidia);
 #endif