[PATCH] Add comments to nicrealtek

[Carl-Daniel Hailfinger]

I just read nicrealtek and realized that the code uses magic constants,
but they are not explained. That's OK if you know the datasheet well,
but for casual readers some comments are really helpful.

I'm not sure whether we want to disable hardware flash access forever
without enabling it again on shutdown. A few other places made me wonder
as well. I've added FIXME comments in those places.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-nicrealtek_comments/nicrealtek.c
===================================================================
--- flashrom-nicrealtek_comments/nicrealtek.c	(Revision 1028)
+++ flashrom-nicrealtek_comments/nicrealtek.c	(Arbeitskopie)
@@ -65,6 +65,7 @@

int nicrealtek_shutdown(void)
 {
+	/* FIXME: We forgot to disable software access again. */
    free(programmer_param);
    pci_cleanup(pacc);
    release_io_perms();
@@ -73,8 +74,14 @@

void nicrealtek_chip_writeb(uint8_t val, chipaddr addr)
 {
+	/* Output addr and data, set WE to 0, set OE to 1, set CS to 0,
+	 * enable software access.
+	 */
    OUTL(((uint32_t)addr & 0x01FFFF) | 0x0A0000 | (val << 24),
         io_base_addr + BIOS_ROM_ADDR);
+	/* Output addr and data, set WE to 1, set OE to 1, set CS to 1,
+	 * enable software access.
+	 */
    OUTL(((uint32_t)addr & 0x01FFFF) | 0x1E0000 | (val << 24),
         io_base_addr + BIOS_ROM_ADDR);
 }
@@ -83,11 +90,20 @@
 {
    uint8_t val;

+	/* FIXME: Can we skip reading the old data and simply use 0? */
+	/* Read old data. */
    val = INB(io_base_addr + BIOS_ROM_DATA);
+	/* Output new addr and old data, set WE to 1, set OE to 0, set CS to 0,
+	 * enable software access.
+	 */
    OUTL(((uint32_t)addr & 0x01FFFF) | 0x060000 | (val << 24),
         io_base_addr + BIOS_ROM_ADDR);

+	/* Read new data. */
    val = INB(io_base_addr + BIOS_ROM_DATA);
+	/* Output addr and new data, set WE to 1, set OE to 1, set CS to 1,
+	 * enable software access.
+	 */
    OUTL(((uint32_t)addr & 0x01FFFF) | 0x1E0000 | (val << 24),
         io_base_addr + BIOS_ROM_ADDR);

[Andrew Morgan]

I have had a read through the datasheet and the comments in the patch 
and it looks good to me.

Acked-by: Andrew Morgan

[Carl-Daniel Hailfinger]

On 05.06.2010 02:01, Andrew Morgan wrote:
> I have had a read through the datasheet and the comments in the patch
> and it looks good to me.
>
> Acked-by: Andrew Morgan

Thanks, committed in r1046.

Regards,
Carl-Daniel