[PATCH] flashrom: SB600 SPI: Kill unused variable

[Carl-Daniel Hailfinger]

SB600 SPI: Kill unused variable.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-sb600spi_unneeded_struct/sb600spi.c
===================================================================
--- flashrom-sb600spi_unneeded_struct/sb600spi.c	(Revision 622)
+++ flashrom-sb600spi_unneeded_struct/sb600spi.c	(Arbeitskopie)
@@ -25,6 +25,7 @@
 #include "flash.h"
 #include "spi.h"

+/* This struct is unused, but helps visualize the SB600 SPI BAR layout. */
 struct sb600_spi_controller {
    unsigned int spi_cntrl0;	/* 00h */
    unsigned int restrictedcmd1;	/* 04h */
@@ -36,7 +37,6 @@
    unsigned int spi_fakeid;	/* 1Ch */
 };

-struct sb600_spi_controller *spi_bar = NULL;
 uint8_t *sb600_spibar;

int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
@@ -111,8 +111,6 @@

writecnt--;

-	spi_bar = (struct sb600_spi_controller *) sb600_spibar;
-
    printf_debug("%s, cmd=%x, writecnt=%x, readcnt=%x\n",
    	     __func__, cmd, writecnt, readcnt);

[Luc Verhaegen]

On Mon, Jun 22, 2009 at 04:31:44PM +0200, Carl-Daniel Hailfinger wrote:
> SB600 SPI: Kill unused variable.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-sb600spi_unneeded_struct/sb600spi.c
>
> --- flashrom-sb600spi_unneeded_struct/sb600spi.c	(Revision 622)
> +++ flashrom-sb600spi_unneeded_struct/sb600spi.c	(Arbeitskopie)
> @@ -25,6 +25,7 @@
>  #include "flash.h"
>  #include "spi.h"
>
> +/* This struct is unused, but helps visualize the SB600 SPI BAR layout. */
>  struct sb600_spi_controller {
>    unsigned int spi_cntrl0;	/* 00h */
>    unsigned int restrictedcmd1;	/* 04h */
> @@ -36,7 +37,6 @@
>    unsigned int spi_fakeid;	/* 1Ch */
>  };
>
> -struct sb600_spi_controller *spi_bar = NULL;
>  uint8_t *sb600_spibar;
>
> int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> @@ -111,8 +111,6 @@
>
> writecnt--;
> spi_bar = (struct sb600_spi_controller *) sb600_spibar;
> printf_debug("%s, cmd=%x, writecnt=%x, readcnt=%x\n",
>        __func__, cmd, writecnt, readcnt);

If this builds, then what can be wrong with this :)

I guess you are keeping this struct because publically available 
documentation is perhaps not as clear as it should be? Why not comment 
it completely so that no-one else will complain about it in future? If 
it is a comment, people are more likely to leave it or scroll over it, 
if it remains a useless struct definition, then people will eventually 
stumble over it and try to remove it, and therefor possibly remove 
useful information.

Please also make reset_internal_fifo_pointer and execute_command static 
so thatthese symbols, when unused, will turn up in the build as well.

Apart from that, of course:
Acked-by: Luc Verhaegen

Luc Verhaegen.

[Carl-Daniel Hailfinger]

On 23.06.2009 00:09, Luc Verhaegen wrote:
> On Mon, Jun 22, 2009 at 04:31:44PM +0200, Carl-Daniel Hailfinger wrote:
> > SB600 SPI: Kill unused variable.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> If this builds, then what can be wrong with this :)
>
> I guess you are keeping this struct because publically available 
> documentation is perhaps not as clear as it should be?

Well, the documentation is pretty readable, but the struct is a handy
compressed reference and useful if you want to avoid reading dozens of
datasheet pages.
> Why not comment 
> it completely so that no-one else will complain about it in future?

Done.
> Please also make reset_internal_fifo_pointer and execute_command static 
> so thatthese symbols, when unused, will turn up in the build as well.

Done.
> Apart from that, of course:
> Acked-by: Luc Verhaegen

Thanks, committed in r623.

Regards,
Carl-Daniel

[Bao, Zheng]

Originally I defined the struct and access the controller by the member
of the struct. But the code couldn't work at first. Then I changed every
"a->b" to "a[XX]". Obviously it doesn't make any sense. Now the code has
been working so far, but I haven't got passion to change them back.
Maybe later I will do it.

Zheng

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Carl-Daniel
Hailfinger
Sent: Tuesday, June 23, 2009 8:49 AM
To: Luc Verhaegen
Cc: Coreboot
Subject: Re: [coreboot] [PATCH] flashrom: SB600 SPI: Kill unused
variable

On 23.06.2009 00:09, Luc Verhaegen wrote:
> On Mon, Jun 22, 2009 at 04:31:44PM +0200, Carl-Daniel Hailfinger

wrote:
> > SB600 SPI: Kill unused variable.
> >
> > Signed-off-by: Carl-Daniel Hailfinger

> >
>
> If this builds, then what can be wrong with this :)
>
> I guess you are keeping this struct because publically available 
> documentation is perhaps not as clear as it should be?

Well, the documentation is pretty readable, but the struct is a handy
compressed reference and useful if you want to avoid reading dozens of
datasheet pages.
> Why not comment 
> it completely so that no-one else will complain about it in future?

Done.
> Please also make reset_internal_fifo_pointer and execute_command

static
> so thatthese symbols, when unused, will turn up in the build as well.

Done.
> Apart from that, of course:
> Acked-by: Luc Verhaegen

Thanks, committed in r623.

Regards,
Carl-Daniel