Skip to contentSkip to conversation

kill include/linux/eeprom.h

PATCH In review Adrian Bunk 3 participants 5 messages Apr 19, 2005 – Apr 20, 2005

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

PATCH 1/1 kill include/linux/eeprom.h

This patch kills include/linux/eeprom.h .

Rationale:
- it's only used by one single driver
- most of this file are non-inline and non-static functions (sic)

This patch moves all required contents of this file into ns83820.c and removes include/linux/eeprom.h (and makes setup_ee_mem_bitbanger static).

If you think eeprom.h should be used more extensively, please consider:
- the code has to be moved from the header file to a .c file
- the currently empty write function has to be implemented
- even ns83820.c used only one of the 7 functions in eeprom.h

Noone did any of these during the more than 3 years eeprom.h already exists...

Signed-off-by Adrian Bunk <[email protected]>

---

This patch was already sent on:
- 17 Feb 2005

 drivers/net/ns83820.c  |   49 +++++++++++++-
 include/linux/eeprom.h |  136 -----------------------------------------
 2 files changed, 44 insertions(+), 141 deletions(-)
include/linux/eeprom.h deleted +0 -136
@@ -1,136 +0,0 @@
-/* credit winbond-840.c
- */
-#include <asm/io.h>
-struct eeprom_ops {
-	void	(*set_cs)(void *ee);
-	void	(*clear_cs)(void *ee);
-};
-
-#define EEPOL_EEDI	0x01
-#define EEPOL_EEDO	0x02
-#define EEPOL_EECLK	0x04
-#define EEPOL_EESEL	0x08
-
-struct eeprom {
-	void *dev;
-	struct eeprom_ops *ops;
-
-	void __iomem *	addr;
-
-	unsigned	ee_addr_bits;
-
-	unsigned	eesel;
-	unsigned	eeclk;
-	unsigned	eedo;
-	unsigned	eedi;
-	unsigned	polarity;
-	unsigned	ee_state;
-
-	spinlock_t	*lock;
-	u32		*cache;
-};
-
-
-u8   eeprom_readb(struct eeprom *ee, unsigned address);
-void eeprom_read(struct eeprom *ee, unsigned address, u8 *bytes,
-		unsigned count);
-void eeprom_writeb(struct eeprom *ee, unsigned address, u8 data);
-void eeprom_write(struct eeprom *ee, unsigned address, u8 *bytes,
-		unsigned count);
-
-/* The EEPROM commands include the alway-set leading bit. */
-enum EEPROM_Cmds {
-        EE_WriteCmd=(5 << 6), EE_ReadCmd=(6 << 6), EE_EraseCmd=(7 << 6),
-};
-
-void setup_ee_mem_bitbanger(struct eeprom *ee, void __iomem *memaddr, int eesel_bit, int eeclk_bit, int eedo_bit, int eedi_bit, unsigned polarity)
-{
-	ee->addr = memaddr;
-	ee->eesel = 1 << eesel_bit;
-	ee->eeclk = 1 << eeclk_bit;
-	ee->eedo = 1 << eedo_bit;
-	ee->eedi = 1 << eedi_bit;
-
-	ee->polarity = polarity;
-
-	*ee->cache = readl(ee->addr);
-}
-
-/* foo. put this in a .c file */
-static inline void eeprom_update(struct eeprom *ee, u32 mask, int pol)
-{
-	unsigned long flags;
-	u32 data;
-
-	spin_lock_irqsave(ee->lock, flags);
-	data = *ee->cache;
-
-	data &= ~mask;
-	if (pol)
-		data |= mask;
-
-	*ee->cache = data;
-//printk("update: %08x\n", data);
-	writel(data, ee->addr);
-	spin_unlock_irqrestore(ee->lock, flags);
-}
-
-void eeprom_clk_lo(struct eeprom *ee)
-{
-	int pol = !!(ee->polarity & EEPOL_EECLK);
-
-	eeprom_update(ee, ee->eeclk, pol);
-	udelay(2);
-}
-
-void eeprom_clk_hi(struct eeprom *ee)
-{
-	int pol = !!(ee->polarity & EEPOL_EECLK);
-
-	eeprom_update(ee, ee->eeclk, !pol);
-	udelay(2);
-}
-
-void eeprom_send_addr(struct eeprom *ee, unsigned address)
-{
-	int pol = !!(ee->polarity & EEPOL_EEDI);
-	unsigned i;
-	address |= 6 << 6;
-
-        /* Shift the read command bits out. */
-        for (i=0; i<11; i++) {
-		eeprom_update(ee, ee->eedi, ((address >> 10) & 1) ^ pol);
-		address <<= 1;
-		eeprom_clk_hi(ee);
-		eeprom_clk_lo(ee);
-        }
-	eeprom_update(ee, ee->eedi, pol);
-}
-
-u16   eeprom_readw(struct eeprom *ee, unsigned address)
-{
-	unsigned i;
-	u16	res = 0;
-
-	eeprom_clk_lo(ee);
-	eeprom_update(ee, ee->eesel, 1 ^ !!(ee->polarity & EEPOL_EESEL));
-	eeprom_send_addr(ee, address);
-
-	for (i=0; i<16; i++) {
-		u32 data;
-		eeprom_clk_hi(ee);
-		res <<= 1;
-		data = readl(ee->addr);
-//printk("eeprom_readw: %08x\n", data);
-		res |= !!(data & ee->eedo) ^ !!(ee->polarity & EEPOL_EEDO);
-		eeprom_clk_lo(ee);
-	}
-	eeprom_update(ee, ee->eesel, 0 ^ !!(ee->polarity & EEPOL_EESEL));
-
-	return res;
-}
-
-
-void eeprom_writeb(struct eeprom *ee, unsigned address, u8 data)
-{
-}
drivers/net/ns83820.c renamed from drivers/net/ns83820.c.old +44 -5

78 lines not on this page · raw message

#1 lore raw

At the very least your patch doesn't do a thorough enough job of removing the dead code -- there is no good reason to move the unused code into ns83820.c.

Also, someone needs to go around refactoring eeprom code out of the network drivers at some point.

-ben

#2 lore raw

No I think the eeprom routine should be saved as it is now for niche vendors and hobbyists. At the very least an accessible versions repository or CVS containing all prior versions of eeprom implementation should be available, a la a new " bitkeeper " or whatever retrieval interface Linus and collaborators decide on.

One project where linux+eeprom may be used: http://www.ethernut.de/api/if__var_8h.html

Nut OS API

int NutNetLoadConfig (CONST char *name)
Load network configuration from EEPROM.
int NutNetSaveConfig (void)
Save network configuration in EEPROM.

Nick

Quoted 10 lines · Benjamin LaHaise

At the very least your patch doesn't do a thorough enough job of removing the dead code -- there is no good reason to move the unused code into ns83820.c.

Also, someone needs to go around refactoring eeprom code out of the network drivers at some point.

-ben
-
To unsubscribe from this list: send the line "unsubscribe linux-net" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw
In reply to Benjamin LaHaise
Quoted 3 lines · Benjamin LaHaise

At the very least your patch doesn't do a thorough enough job of removing the dead code -- there is no good reason to move the unused code into ns83820.c.

Where does my patch do this?
Only the one actually used function setup_ee_mem_bitbanger is moved to ns83820.c .

Quoted 2 lines

Also, someone needs to go around refactoring eeprom code out of the network drivers at some point.

I have no problem with this, but it has to be done right.

Quoted 1 line

-ben

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#4 lore raw
Quoted 3 lines · Adrian Bunk

Where does my patch do this?
Only the one actually used function setup_ee_mem_bitbanger is moved to ns83820.c .

Which is dead code if you have removed the other users of the data structure it initialized. Please try to understand the intent of the code being cleaned up instead of just blindly deleting things, and be thorough.

-ben