Unused functions...

[Carl-Daniel Hailfinger]

We have a few unused functions in our codebase.

Should we
- mark then static (may cause gcc to complain if -Wunused-function is
enabled)
- put them inside #if 0 (code will bitrot)
- remove them from the source base entirely (some may be used in the future)

jedec.c:
data_polling_jedec (maybe some chip in the future won't support
toggle_jedec)

stm50flw0x0x.c:
erase_sector_stm50flw0x0x (keep?)

m29f400bt.c:
write_m29f400bt (hm, probably kill)

82802ab.c:
erase_82802ab (kill, old-style erase)

pm49fl00x.c:
lock_49fl00x (for the future)

sst49lfxxxc.c:
unlock_block_49lfxxxc (strange, don't we use that?)

spi.c:
default_spi_send_command (if one chip driver ever supports only
multicommand)

spi25.c:
spi_write_status_enable (we should use it, some chips require that)
spi_write_disable (does that make sense?)
spi_aai_write (need some write infrastructure changes for this)
spi_chip_erase_d8 (kill)

flashrom.c:
chip_writel (will we ever need it?)
chip_writen (this one might be usable for a few chip drivers)
chip_readl (will we ever need it?)
list_programmers (no idea)
register_shutdown (needed for ECs)
need_erase (needed for partial write)

internal.c:
pci_dev_find_filter (hmmm)

Regards,
Carl-Daniel

[Sean Nelson]

On 3/25/10 4:31 PM, Carl-Daniel Hailfinger wrote:
> We have a few unused functions in our codebase.
>
> Should we
> mark then static (may cause gcc to complain if -Wunused-function is
>
> enabled)
> put them inside #if 0 (code will bitrot)
> remove them from the source base entirely (some may be used in the future)
>
> jedec.c:
> data_polling_jedec (maybe some chip in the future won't support
> toggle_jedec)

It's unused, but you bring up a good point.
> stm50flw0x0x.c:
> erase_sector_stm50flw0x0x (keep?)

We should keep it and fill in it's use for those chips.
> m29f400bt.c:
> write_m29f400bt (hm, probably kill)

We really should be using write_m29f400bt instead of 
write_coreboot_m29f400bt, since the coreboot function only utilize the 
first four 64k blocks.
> 82802ab.c:
> erase_82802ab (kill, old-style erase)

Maybe convert to erase_chip_82802ab(struct flashchip *flash, unsigned 
int block, unsigned int size)
> pm49fl00x.c:
> lock_49fl00x (for the future)

Agreed.
> sst49lfxxxc.c:
> unlock_block_49lfxxxc (strange, don't we use that?)

Might be used later if we move to a unlock block structure.
> spi.c:
> default_spi_send_command (if one chip driver ever supports only
> multicommand)

Keep?
> spi25.c:
> spi_write_status_enable (we should use it, some chips require that)

Agreed.
> spi_write_disable (does that make sense?)

That's kinda like lock_49fl00x.
> spi_aai_write (need some write infrastructure changes for this)

I don't even know what that is.
> spi_chip_erase_d8 (kill)

Maybe keep for future?
> flashrom.c:
> chip_writel (will we ever need it?)
> chip_writen (this one might be usable for a few chip drivers)
> chip_readl (will we ever need it?)
> list_programmers (no idea)
> register_shutdown (needed for ECs)
> need_erase (needed for partial write)
>
> internal.c:
> pci_dev_find_filter (hmmm)

The above functions I don't have an opinion on, since I haven't dealt 
with them yet. As for what we should do about the unused functions, I 
think we should put them in #if 0. That way we have them around for 
later on. OTOH, we should also be stripping out any unused code. If we 
strip it out and we need it later, someone could just as easily write a 
new, better function.

~ Sean