[PATCH 0/4 v2] ks8851: support for read/write MAC address from eeprom

[PATCH 0/4 v2] ks8851: support for read/write MAC address from eeprom

[Sebastien Jan]

Provides the ability to update the ks8851 companion eeprom content (including the
MAC address) through ethtool or debugfs (for MAC address only).

Differences with v1:
 - added the support of eeprom access through ethtool
 - different patches split

Patch set tested on OMAP4-based test board. Build tested on net-2.6 head.

Applies on current net-2.6 head.

Sebastien Jan (4):
  ks8851: Add caching of CCR register
  ks8851: Low level functions for read/write to companion eeprom
  ks8851: companion eeprom access through ethtool
  ks8851: read/write MAC address on companion eeprom through debugfs

 drivers/net/ks8851.c |  570 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/ks8851.h |   14 ++-
 2 files changed, 583 insertions(+), 1 deletions(-)

[PATCH 1/4 v2] ks8851: Add caching of CCR register

[Sebastien Jan]

CCR register contains information on companion eeprom availability.

Signed-off-by: Sebastien Jan <s-jan@ti.com>
---
 drivers/net/ks8851.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
index 9e9f9b3..a84e500 100644
--- a/drivers/net/ks8851.c
+++ b/drivers/net/ks8851.c
@@ -76,7 +76,9 @@ union ks8851_tx_hdr {
  * @msg_enable: The message flags controlling driver output (see ethtool).
  * @fid: Incrementing frame id tag.
  * @rc_ier: Cached copy of KS_IER.
+ * @rc_ccr: Cached copy of KS_CCR.
  * @rc_rxqcr: Cached copy of KS_RXQCR.
+ * @eeprom_size: Companion eeprom size in Bytes, 0 if no eeprom
  *
  * The @lock ensures that the chip is protected when certain operations are
  * in progress. When the read or write packet transfer is in progress, most
@@ -107,6 +109,8 @@ struct ks8851_net {
 
 	u16			rc_ier;
 	u16			rc_rxqcr;
+	u16			rc_ccr;
+	u16			eeprom_size;
 
 	struct mii_if_info	mii;
 	struct ks8851_rxctrl	rxctrl;
@@ -1279,6 +1283,14 @@ static int __devinit ks8851_probe(struct spi_device *spi)
 		goto err_id;
 	}
 
+	/* cache the contents of the CCR register for EEPROM, etc. */
+	ks->rc_ccr = ks8851_rdreg16(ks, KS_CCR);
+
+	if (ks->rc_ccr & CCR_EEPROM)
+		ks->eeprom_size = 128;
+	else
+		ks->eeprom_size = 0;
+
 	ks8851_read_selftest(ks);
 	ks8851_init_mac(ks);
 
-- 
1.6.3.3

Re: [PATCH 1/4 v2] ks8851: Add caching of CCR register

[David Miller]

From: Sebastien Jan <s-jan@ti.com>
Date: Wed,  5 May 2010 20:45:52 +0200

> CCR register contains information on companion eeprom availability.
> 
> Signed-off-by: Sebastien Jan <s-jan@ti.com>

APplied.

[PATCH 2/4 v2] ks8851: Low level functions for read/write to companion eeprom

[Sebastien Jan]

Low-level functions provide 16bits words read and write capability
to ks8851 companion eeprom.

Signed-off-by: Sebastien Jan <s-jan@ti.com>
---
 drivers/net/ks8851.c |  228 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/ks8851.h |   14 +++-
 2 files changed, 241 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
index a84e500..787f9df 100644
--- a/drivers/net/ks8851.c
+++ b/drivers/net/ks8851.c
@@ -1044,6 +1044,234 @@ static const struct net_device_ops ks8851_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 };
 
+/* Companion eeprom access */
+
+enum {	/* EEPROM programming states */
+	EEPROM_CONTROL,
+	EEPROM_ADDRESS,
+	EEPROM_DATA,
+	EEPROM_COMPLETE
+};
+
+/**
+ * ks8851_eeprom_read - read a 16bits word in ks8851 companion EEPROM
+ * @dev: The network device the PHY is on.
+ * @addr: EEPROM address to read
+ *
+ * eeprom_size: used to define the data coding length. Can be changed
+ * through debug-fs.
+ *
+ * Programs a read on the EEPROM using ks8851 EEPROM SW access feature.
+ * Warning: The READ feature is not supported on ks8851 revision 0.
+ *
+ * Rough programming model:
+ *  - on period start: set clock high and read value on bus
+ *  - on period / 2: set clock low and program value on bus
+ *  - start on period / 2
+ */
+unsigned int ks8851_eeprom_read(struct net_device *dev, unsigned int addr)
+{
+	struct ks8851_net *ks = netdev_priv(dev);
+	int eepcr;
+	int ctrl = EEPROM_OP_READ;
+	int state = EEPROM_CONTROL;
+	int bit_count = EEPROM_OP_LEN - 1;
+	unsigned int data = 0;
+	int dummy;
+	unsigned int addr_len;
+
+	addr_len = (ks->eeprom_size == 128) ? 6 : 8;
+
+	/* start transaction: chip select high, authorize write */
+	mutex_lock(&ks->lock);
+	eepcr = EEPCR_EESA | EEPCR_EESRWA;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	eepcr |= EEPCR_EECS;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	mutex_unlock(&ks->lock);
+
+	while (state != EEPROM_COMPLETE) {
+		/* falling clock period starts... */
+		/* set EED_IO pin for control and address */
+		eepcr &= ~EEPCR_EEDO;
+		switch (state) {
+		case EEPROM_CONTROL:
+			eepcr |= ((ctrl >> bit_count) & 1) << 2;
+			if (bit_count-- <= 0) {
+				bit_count = addr_len - 1;
+				state = EEPROM_ADDRESS;
+			}
+			break;
+		case EEPROM_ADDRESS:
+			eepcr |= ((addr >> bit_count) & 1) << 2;
+			bit_count--;
+			break;
+		case EEPROM_DATA:
+			/* Change to receive mode */
+			eepcr &= ~EEPCR_EESRWA;
+			break;
+		}
+
+		/* lower clock  */
+		eepcr &= ~EEPCR_EESCK;
+
+		mutex_lock(&ks->lock);
+		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+		mutex_unlock(&ks->lock);
+
+		/* waitread period / 2 */
+		udelay(EEPROM_SK_PERIOD / 2);
+
+		/* rising clock period starts... */
+
+		/* raise clock */
+		mutex_lock(&ks->lock);
+		eepcr |= EEPCR_EESCK;
+		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+		mutex_unlock(&ks->lock);
+
+		/* Manage read */
+		switch (state) {
+		case EEPROM_ADDRESS:
+			if (bit_count < 0) {
+				bit_count = EEPROM_DATA_LEN - 1;
+				state = EEPROM_DATA;
+			}
+			break;
+		case EEPROM_DATA:
+			mutex_lock(&ks->lock);
+			dummy = ks8851_rdreg16(ks, KS_EEPCR);
+			mutex_unlock(&ks->lock);
+			data |= ((dummy >> EEPCR_EESB_OFFSET) & 1) << bit_count;
+			if (bit_count-- <= 0)
+				state = EEPROM_COMPLETE;
+			break;
+		}
+
+		/* wait period / 2 */
+		udelay(EEPROM_SK_PERIOD / 2);
+	}
+
+	/* close transaction */
+	mutex_lock(&ks->lock);
+	eepcr &= ~EEPCR_EECS;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	eepcr = 0;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	mutex_unlock(&ks->lock);
+
+	return data;
+}
+
+/**
+ * ks8851_eeprom_write - write a 16bits word in ks8851 companion EEPROM
+ * @dev: The network device the PHY is on.
+ * @op: operand (can be WRITE, EWEN, EWDS)
+ * @addr: EEPROM address to write
+ * @data: data to write
+ *
+ * eeprom_size: used to define the data coding length. Can be changed
+ * through debug-fs.
+ *
+ * Programs a write on the EEPROM using ks8851 EEPROM SW access feature.
+ *
+ * Note that a write enable is required before writing data.
+ *
+ * Rough programming model:
+ *  - on period start: set clock high
+ *  - on period / 2: set clock low and program value on bus
+ *  - start on period / 2
+ */
+void ks8851_eeprom_write(struct net_device *dev, unsigned int op,
+					unsigned int addr, unsigned int data)
+{
+	struct ks8851_net *ks = netdev_priv(dev);
+	int eepcr;
+	int state = EEPROM_CONTROL;
+	int bit_count = EEPROM_OP_LEN - 1;
+	unsigned int addr_len;
+
+	addr_len = (ks->eeprom_size == 128) ? 6 : 8;
+
+	switch (op) {
+	case EEPROM_OP_EWEN:
+		addr = 0x30;
+	break;
+	case EEPROM_OP_EWDS:
+		addr = 0;
+		break;
+	}
+
+	/* start transaction: chip select high, authorize write */
+	mutex_lock(&ks->lock);
+	eepcr = EEPCR_EESA | EEPCR_EESRWA;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	eepcr |= EEPCR_EECS;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	mutex_unlock(&ks->lock);
+
+	while (state != EEPROM_COMPLETE) {
+		/* falling clock period starts... */
+		/* set EED_IO pin for control and address */
+		eepcr &= ~EEPCR_EEDO;
+		switch (state) {
+		case EEPROM_CONTROL:
+			eepcr |= ((op >> bit_count) & 1) << 2;
+			if (bit_count-- <= 0) {
+				bit_count = addr_len - 1;
+				state = EEPROM_ADDRESS;
+			}
+			break;
+		case EEPROM_ADDRESS:
+			eepcr |= ((addr >> bit_count) & 1) << 2;
+			if (bit_count-- <= 0) {
+				if (op == EEPROM_OP_WRITE) {
+					bit_count = EEPROM_DATA_LEN - 1;
+					state = EEPROM_DATA;
+				} else {
+					state = EEPROM_COMPLETE;
+				}
+			}
+			break;
+		case EEPROM_DATA:
+			eepcr |= ((data >> bit_count) & 1) << 2;
+			if (bit_count-- <= 0)
+				state = EEPROM_COMPLETE;
+			break;
+		}
+
+		/* lower clock  */
+		eepcr &= ~EEPCR_EESCK;
+
+		mutex_lock(&ks->lock);
+		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+		mutex_unlock(&ks->lock);
+
+		/* wait period / 2 */
+		udelay(EEPROM_SK_PERIOD / 2);
+
+		/* rising clock period starts... */
+
+		/* raise clock */
+		eepcr |= EEPCR_EESCK;
+		mutex_lock(&ks->lock);
+		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+		mutex_unlock(&ks->lock);
+
+		/* wait period / 2 */
+		udelay(EEPROM_SK_PERIOD / 2);
+	}
+
+	/* close transaction */
+	mutex_lock(&ks->lock);
+	eepcr &= ~EEPCR_EECS;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	eepcr = 0;
+	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
+	mutex_unlock(&ks->lock);
+
+}
+
 /* ethtool support */
 
 static void ks8851_get_drvinfo(struct net_device *dev,
diff --git a/drivers/net/ks8851.h b/drivers/net/ks8851.h
index f52c312..537fb06 100644
--- a/drivers/net/ks8851.h
+++ b/drivers/net/ks8851.h
@@ -25,12 +25,24 @@
 #define OBCR_ODS_16mA				(1 << 6)
 
 #define KS_EEPCR				0x22
+#define EEPCR_EESRWA				(1 << 5)
 #define EEPCR_EESA				(1 << 4)
-#define EEPCR_EESB				(1 << 3)
+#define EEPCR_EESB_OFFSET			3
+#define EEPCR_EESB				(1 << EEPCR_EESB_OFFSET)
 #define EEPCR_EEDO				(1 << 2)
 #define EEPCR_EESCK				(1 << 1)
 #define EEPCR_EECS				(1 << 0)
 
+#define EEPROM_OP_LEN				3	/* bits:*/
+#define EEPROM_OP_READ				0x06
+#define EEPROM_OP_EWEN				0x04
+#define EEPROM_OP_WRITE				0x05
+#define EEPROM_OP_EWDS				0x14
+
+#define EEPROM_DATA_LEN				16	/* 16 bits EEPROM */
+#define EEPROM_WRITE_TIME			4	/* wrt ack time in ms */
+#define EEPROM_SK_PERIOD			400	/* in us */
+
 #define KS_MBIR					0x24
 #define MBIR_TXMBF				(1 << 12)
 #define MBIR_TXMBFA				(1 << 11)
-- 
1.6.3.3

Re: [PATCH 2/4 v2] ks8851: Low level functions for read/write to companion eeprom

[David Miller]

From: Sebastien Jan <s-jan@ti.com>
Date: Wed,  5 May 2010 20:45:53 +0200

> Low-level functions provide 16bits words read and write capability
> to ks8851 companion eeprom.
> 
> Signed-off-by: Sebastien Jan <s-jan@ti.com>

Applied.

Re: [PATCH 2/4 v2] ks8851: Low level functions for read/write to companion eeprom

[Ben Dooks]

On Thu, May 06, 2010 at 12:20:12AM -0700, David Miller wrote:
> From: Sebastien Jan <s-jan@ti.com>
> Date: Wed,  5 May 2010 20:45:53 +0200
> 
> > Low-level functions provide 16bits words read and write capability
> > to ks8851 companion eeprom.
> > 
> > Signed-off-by: Sebastien Jan <s-jan@ti.com>
> 
> Applied.

So I take it the patches that used the drivers/misc/eeprom/eeprom_93cx6.c
are not going to be merged?

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [PATCH 2/4 v2] ks8851: Low level functions for read/write to companion eeprom

[David Miller]

From: Ben Dooks <ben-linux@fluff.org>
Date: Tue, 11 May 2010 02:23:54 +0100

> On Thu, May 06, 2010 at 12:20:12AM -0700, David Miller wrote:
>> From: Sebastien Jan <s-jan@ti.com>
>> Date: Wed,  5 May 2010 20:45:53 +0200
>> 
>> > Low-level functions provide 16bits words read and write capability
>> > to ks8851 companion eeprom.
>> > 
>> > Signed-off-by: Sebastien Jan <s-jan@ti.com>
>> 
>> Applied.
> 
> So I take it the patches that used the drivers/misc/eeprom/eeprom_93cx6.c
> are not going to be merged?

What makes you think that?  Feel free to submit such a patch and find
out for real.

If you want things done differently from the start, review patches
faster :-)

Re: [PATCH 2/4 v2] ks8851: Low level functions for read/write to companion eeprom

[Ben Dooks]

On Wed, May 05, 2010 at 08:45:53PM +0200, Sebastien Jan wrote:
> Low-level functions provide 16bits words read and write capability
> to ks8851 companion eeprom.

Please use the eeprom interface that was added already
 
> Signed-off-by: Sebastien Jan <s-jan@ti.com>
> ---
>  drivers/net/ks8851.c |  228 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/net/ks8851.h |   14 +++-
>  2 files changed, 241 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
> index a84e500..787f9df 100644
> --- a/drivers/net/ks8851.c
> +++ b/drivers/net/ks8851.c
> @@ -1044,6 +1044,234 @@ static const struct net_device_ops ks8851_netdev_ops = {
>  	.ndo_validate_addr	= eth_validate_addr,
>  };
>  
> +/* Companion eeprom access */
> +
> +enum {	/* EEPROM programming states */
> +	EEPROM_CONTROL,
> +	EEPROM_ADDRESS,
> +	EEPROM_DATA,
> +	EEPROM_COMPLETE
> +};
> +
> +/**
> + * ks8851_eeprom_read - read a 16bits word in ks8851 companion EEPROM
> + * @dev: The network device the PHY is on.
> + * @addr: EEPROM address to read
> + *
> + * eeprom_size: used to define the data coding length. Can be changed
> + * through debug-fs.
> + *
> + * Programs a read on the EEPROM using ks8851 EEPROM SW access feature.
> + * Warning: The READ feature is not supported on ks8851 revision 0.
> + *
> + * Rough programming model:
> + *  - on period start: set clock high and read value on bus
> + *  - on period / 2: set clock low and program value on bus
> + *  - start on period / 2
> + */
> +unsigned int ks8851_eeprom_read(struct net_device *dev, unsigned int addr)
> +{
> +	struct ks8851_net *ks = netdev_priv(dev);
> +	int eepcr;
> +	int ctrl = EEPROM_OP_READ;
> +	int state = EEPROM_CONTROL;
> +	int bit_count = EEPROM_OP_LEN - 1;
> +	unsigned int data = 0;
> +	int dummy;
> +	unsigned int addr_len;
> +
> +	addr_len = (ks->eeprom_size == 128) ? 6 : 8;
> +
> +	/* start transaction: chip select high, authorize write */
> +	mutex_lock(&ks->lock);
> +	eepcr = EEPCR_EESA | EEPCR_EESRWA;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	eepcr |= EEPCR_EECS;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	mutex_unlock(&ks->lock);
> +
> +	while (state != EEPROM_COMPLETE) {
> +		/* falling clock period starts... */
> +		/* set EED_IO pin for control and address */
> +		eepcr &= ~EEPCR_EEDO;
> +		switch (state) {
> +		case EEPROM_CONTROL:
> +			eepcr |= ((ctrl >> bit_count) & 1) << 2;
> +			if (bit_count-- <= 0) {
> +				bit_count = addr_len - 1;
> +				state = EEPROM_ADDRESS;
> +			}
> +			break;
> +		case EEPROM_ADDRESS:
> +			eepcr |= ((addr >> bit_count) & 1) << 2;
> +			bit_count--;
> +			break;
> +		case EEPROM_DATA:
> +			/* Change to receive mode */
> +			eepcr &= ~EEPCR_EESRWA;
> +			break;
> +		}
> +
> +		/* lower clock  */
> +		eepcr &= ~EEPCR_EESCK;
> +
> +		mutex_lock(&ks->lock);
> +		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +		mutex_unlock(&ks->lock);
> +
> +		/* waitread period / 2 */
> +		udelay(EEPROM_SK_PERIOD / 2);
> +
> +		/* rising clock period starts... */
> +
> +		/* raise clock */
> +		mutex_lock(&ks->lock);
> +		eepcr |= EEPCR_EESCK;
> +		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +		mutex_unlock(&ks->lock);
> +
> +		/* Manage read */
> +		switch (state) {
> +		case EEPROM_ADDRESS:
> +			if (bit_count < 0) {
> +				bit_count = EEPROM_DATA_LEN - 1;
> +				state = EEPROM_DATA;
> +			}
> +			break;
> +		case EEPROM_DATA:
> +			mutex_lock(&ks->lock);
> +			dummy = ks8851_rdreg16(ks, KS_EEPCR);
> +			mutex_unlock(&ks->lock);
> +			data |= ((dummy >> EEPCR_EESB_OFFSET) & 1) << bit_count;
> +			if (bit_count-- <= 0)
> +				state = EEPROM_COMPLETE;
> +			break;
> +		}
> +
> +		/* wait period / 2 */
> +		udelay(EEPROM_SK_PERIOD / 2);
> +	}
> +
> +	/* close transaction */
> +	mutex_lock(&ks->lock);
> +	eepcr &= ~EEPCR_EECS;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	eepcr = 0;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	mutex_unlock(&ks->lock);
> +
> +	return data;
> +}
> +
> +/**
> + * ks8851_eeprom_write - write a 16bits word in ks8851 companion EEPROM
> + * @dev: The network device the PHY is on.
> + * @op: operand (can be WRITE, EWEN, EWDS)
> + * @addr: EEPROM address to write
> + * @data: data to write
> + *
> + * eeprom_size: used to define the data coding length. Can be changed
> + * through debug-fs.
> + *
> + * Programs a write on the EEPROM using ks8851 EEPROM SW access feature.
> + *
> + * Note that a write enable is required before writing data.
> + *
> + * Rough programming model:
> + *  - on period start: set clock high
> + *  - on period / 2: set clock low and program value on bus
> + *  - start on period / 2
> + */
> +void ks8851_eeprom_write(struct net_device *dev, unsigned int op,
> +					unsigned int addr, unsigned int data)
> +{
> +	struct ks8851_net *ks = netdev_priv(dev);
> +	int eepcr;
> +	int state = EEPROM_CONTROL;
> +	int bit_count = EEPROM_OP_LEN - 1;
> +	unsigned int addr_len;
> +
> +	addr_len = (ks->eeprom_size == 128) ? 6 : 8;
> +
> +	switch (op) {
> +	case EEPROM_OP_EWEN:
> +		addr = 0x30;
> +	break;
> +	case EEPROM_OP_EWDS:
> +		addr = 0;
> +		break;
> +	}
> +
> +	/* start transaction: chip select high, authorize write */
> +	mutex_lock(&ks->lock);
> +	eepcr = EEPCR_EESA | EEPCR_EESRWA;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	eepcr |= EEPCR_EECS;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	mutex_unlock(&ks->lock);
> +
> +	while (state != EEPROM_COMPLETE) {
> +		/* falling clock period starts... */
> +		/* set EED_IO pin for control and address */
> +		eepcr &= ~EEPCR_EEDO;
> +		switch (state) {
> +		case EEPROM_CONTROL:
> +			eepcr |= ((op >> bit_count) & 1) << 2;
> +			if (bit_count-- <= 0) {
> +				bit_count = addr_len - 1;
> +				state = EEPROM_ADDRESS;
> +			}
> +			break;
> +		case EEPROM_ADDRESS:
> +			eepcr |= ((addr >> bit_count) & 1) << 2;
> +			if (bit_count-- <= 0) {
> +				if (op == EEPROM_OP_WRITE) {
> +					bit_count = EEPROM_DATA_LEN - 1;
> +					state = EEPROM_DATA;
> +				} else {
> +					state = EEPROM_COMPLETE;
> +				}
> +			}
> +			break;
> +		case EEPROM_DATA:
> +			eepcr |= ((data >> bit_count) & 1) << 2;
> +			if (bit_count-- <= 0)
> +				state = EEPROM_COMPLETE;
> +			break;
> +		}
> +
> +		/* lower clock  */
> +		eepcr &= ~EEPCR_EESCK;
> +
> +		mutex_lock(&ks->lock);
> +		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +		mutex_unlock(&ks->lock);
> +
> +		/* wait period / 2 */
> +		udelay(EEPROM_SK_PERIOD / 2);
> +
> +		/* rising clock period starts... */
> +
> +		/* raise clock */
> +		eepcr |= EEPCR_EESCK;
> +		mutex_lock(&ks->lock);
> +		ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +		mutex_unlock(&ks->lock);
> +
> +		/* wait period / 2 */
> +		udelay(EEPROM_SK_PERIOD / 2);
> +	}
> +
> +	/* close transaction */
> +	mutex_lock(&ks->lock);
> +	eepcr &= ~EEPCR_EECS;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	eepcr = 0;
> +	ks8851_wrreg16(ks, KS_EEPCR, eepcr);
> +	mutex_unlock(&ks->lock);
> +
> +}
> +
>  /* ethtool support */
>  
>  static void ks8851_get_drvinfo(struct net_device *dev,
> diff --git a/drivers/net/ks8851.h b/drivers/net/ks8851.h
> index f52c312..537fb06 100644
> --- a/drivers/net/ks8851.h
> +++ b/drivers/net/ks8851.h
> @@ -25,12 +25,24 @@
>  #define OBCR_ODS_16mA				(1 << 6)
>  
>  #define KS_EEPCR				0x22
> +#define EEPCR_EESRWA				(1 << 5)
>  #define EEPCR_EESA				(1 << 4)
> -#define EEPCR_EESB				(1 << 3)
> +#define EEPCR_EESB_OFFSET			3
> +#define EEPCR_EESB				(1 << EEPCR_EESB_OFFSET)
>  #define EEPCR_EEDO				(1 << 2)
>  #define EEPCR_EESCK				(1 << 1)
>  #define EEPCR_EECS				(1 << 0)
>  
> +#define EEPROM_OP_LEN				3	/* bits:*/
> +#define EEPROM_OP_READ				0x06
> +#define EEPROM_OP_EWEN				0x04
> +#define EEPROM_OP_WRITE				0x05
> +#define EEPROM_OP_EWDS				0x14
> +
> +#define EEPROM_DATA_LEN				16	/* 16 bits EEPROM */
> +#define EEPROM_WRITE_TIME			4	/* wrt ack time in ms */
> +#define EEPROM_SK_PERIOD			400	/* in us */
> +
>  #define KS_MBIR					0x24
>  #define MBIR_TXMBF				(1 << 12)
>  #define MBIR_TXMBFA				(1 << 11)
> -- 
> 1.6.3.3
> 

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[PATCH 3/4 v2] ks8851: companion eeprom access through ethtool

[Sebastien Jan]

Accessing ks8851 companion eeprom permits modifying the ks8851 stored
MAC address.

Example how to change the MAC address using ethtool, to set the
01:23:45:67:89:AB MAC address:
$ echo "0:AB8976452301" | xxd -r > mac.bin
$ sudo ethtool -E eth0 magic 0x8870 offset 2 < mac.bin

Signed-off-by: Sebastien Jan <s-jan@ti.com>
---
 drivers/net/ks8851.c |  114 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 114 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
index 787f9df..8e38c36 100644
--- a/drivers/net/ks8851.c
+++ b/drivers/net/ks8851.c
@@ -1318,6 +1318,117 @@ static int ks8851_nway_reset(struct net_device *dev)
 	return mii_nway_restart(&ks->mii);
 }
 
+static int ks8851_get_eeprom_len(struct net_device *dev)
+{
+	struct ks8851_net *ks = netdev_priv(dev);
+	return ks->eeprom_size;
+}
+
+static int ks8851_get_eeprom(struct net_device *dev,
+			    struct ethtool_eeprom *eeprom, u8 *bytes)
+{
+	struct ks8851_net *ks = netdev_priv(dev);
+	u16 *eeprom_buff;
+	int first_word;
+	int last_word;
+	int ret_val = 0;
+	u16 i;
+
+	if (eeprom->len == 0)
+		return -EINVAL;
+
+	if (eeprom->len > ks->eeprom_size)
+		return -EINVAL;
+
+	eeprom->magic = ks8851_rdreg16(ks, KS_CIDER);
+
+	first_word = eeprom->offset >> 1;
+	last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+
+	eeprom_buff = kmalloc(sizeof(u16) *
+			(last_word - first_word + 1), GFP_KERNEL);
+	if (!eeprom_buff)
+		return -ENOMEM;
+
+	for (i = 0; i < last_word - first_word + 1; i++)
+		eeprom_buff[i] = ks8851_eeprom_read(dev, first_word + 1);
+
+	/* Device's eeprom is little-endian, word addressable */
+	for (i = 0; i < last_word - first_word + 1; i++)
+		le16_to_cpus(&eeprom_buff[i]);
+
+	memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1), eeprom->len);
+	kfree(eeprom_buff);
+
+	return ret_val;
+}
+
+static int ks8851_set_eeprom(struct net_device *dev,
+			    struct ethtool_eeprom *eeprom, u8 *bytes)
+{
+	struct ks8851_net *ks = netdev_priv(dev);
+	u16 *eeprom_buff;
+	void *ptr;
+	int max_len;
+	int first_word;
+	int last_word;
+	int ret_val = 0;
+	u16 i;
+
+	if (eeprom->len == 0)
+		return -EOPNOTSUPP;
+
+	if (eeprom->len > ks->eeprom_size)
+		return -EINVAL;
+
+	if (eeprom->magic != ks8851_rdreg16(ks, KS_CIDER))
+		return -EFAULT;
+
+	first_word = eeprom->offset >> 1;
+	last_word = (eeprom->offset + eeprom->len - 1) >> 1;
+	max_len = (last_word - first_word + 1) * 2;
+	eeprom_buff = kmalloc(max_len, GFP_KERNEL);
+	if (!eeprom_buff)
+		return -ENOMEM;
+
+	ptr = (void *)eeprom_buff;
+
+	if (eeprom->offset & 1) {
+		/* need read/modify/write of first changed EEPROM word */
+		/* only the second byte of the word is being modified */
+		eeprom_buff[0] = ks8851_eeprom_read(dev, first_word);
+		ptr++;
+	}
+	if ((eeprom->offset + eeprom->len) & 1)
+		/* need read/modify/write of last changed EEPROM word */
+		/* only the first byte of the word is being modified */
+		eeprom_buff[last_word - first_word] =
+					ks8851_eeprom_read(dev, last_word);
+
+
+	/* Device's eeprom is little-endian, word addressable */
+	le16_to_cpus(&eeprom_buff[0]);
+	le16_to_cpus(&eeprom_buff[last_word - first_word]);
+
+	memcpy(ptr, bytes, eeprom->len);
+
+	for (i = 0; i < last_word - first_word + 1; i++)
+		eeprom_buff[i] = cpu_to_le16(eeprom_buff[i]);
+
+	ks8851_eeprom_write(dev, EEPROM_OP_EWEN, 0, 0);
+
+	for (i = 0; i < last_word - first_word + 1; i++) {
+		ks8851_eeprom_write(dev, EEPROM_OP_WRITE, first_word + i,
+							eeprom_buff[i]);
+		mdelay(EEPROM_WRITE_TIME);
+	}
+
+	ks8851_eeprom_write(dev, EEPROM_OP_EWDS, 0, 0);
+
+	kfree(eeprom_buff);
+	return ret_val;
+}
+
 static const struct ethtool_ops ks8851_ethtool_ops = {
 	.get_drvinfo	= ks8851_get_drvinfo,
 	.get_msglevel	= ks8851_get_msglevel,
@@ -1326,6 +1437,9 @@ static const struct ethtool_ops ks8851_ethtool_ops = {
 	.set_settings	= ks8851_set_settings,
 	.get_link	= ks8851_get_link,
 	.nway_reset	= ks8851_nway_reset,
+	.get_eeprom_len	= ks8851_get_eeprom_len,
+	.get_eeprom	= ks8851_get_eeprom,
+	.set_eeprom	= ks8851_set_eeprom,
 };
 
 /* MII interface controls */
-- 
1.6.3.3

Re: [PATCH 3/4 v2] ks8851: companion eeprom access through ethtool

[David Miller]

From: Sebastien Jan <s-jan@ti.com>
Date: Wed,  5 May 2010 20:45:54 +0200

> Accessing ks8851 companion eeprom permits modifying the ks8851 stored
> MAC address.
> 
> Example how to change the MAC address using ethtool, to set the
> 01:23:45:67:89:AB MAC address:
> $ echo "0:AB8976452301" | xxd -r > mac.bin
> $ sudo ethtool -E eth0 magic 0x8870 offset 2 < mac.bin
> 
> Signed-off-by: Sebastien Jan <s-jan@ti.com>

Applied.

[PATCH 4/4 v2] ks8851: read/write MAC address on companion eeprom through debugfs

[Sebastien Jan]

A more elegant alternative to ethtool for updating the ks8851
MAC address stored on its companion eeprom.
Using this debugfs interface does not require any knowledge on the
ks8851 companion eeprom organization to update the MAC address.

Example to write 01:23:45:67:89:AB MAC address to the companion
eeprom (assuming debugfs is mounted in /sys/kernel/debug):
$ echo "01:23:45:67:89:AB" > /sys/kernel/debug/ks8851/mac_eeprom

Signed-off-by: Sebastien Jan <s-jan@ti.com>
---
 drivers/net/ks8851.c |  216 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 216 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
index 8e38c36..1b62ec5 100644
--- a/drivers/net/ks8851.c
+++ b/drivers/net/ks8851.c
@@ -22,6 +22,11 @@
 
 #include <linux/spi/spi.h>
 
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+#include <linux/ctype.h>
+#endif
+
 #include "ks8851.h"
 
 /**
@@ -1550,6 +1555,214 @@ static int ks8851_read_selftest(struct ks8851_net *ks)
 	return 0;
 }
 
+#ifdef CONFIG_DEBUG_FS
+static struct dentry *ks8851_dir;
+
+static int ks8851_mac_eeprom_open(struct inode *inode, struct file *file)
+{
+	file->private_data = inode->i_private;
+	return 0;
+}
+
+static int ks8851_mac_eeprom_release(struct inode *inode, struct file *file)
+{
+	return 0;
+}
+
+static loff_t ks8851_mac_eeprom_seek(struct file *file, loff_t off, int whence)
+{
+	return 0;
+}
+
+/**
+ * ks8851_mac_eeprom_read - Read a MAC address in ks8851 companion EEPROM
+ *
+ * Warning: The READ feature is not supported on ks8851 revision 0.
+ */
+static ssize_t ks8851_mac_eeprom_read(struct file *filep, char __user *buff,
+						size_t count, loff_t *offp)
+{
+	ssize_t ret;
+	struct net_device *dev = filep->private_data;
+	char str[50];
+	unsigned int data;
+	unsigned char mac_addr[6];
+
+	if (*offp > 0) {
+		ret = 0;
+		goto ks8851_cnt_rd_bk;
+	}
+
+	data = ks8851_eeprom_read(dev, 1);
+	mac_addr[5] = data & 0xFF;
+	mac_addr[4] = (data >> 8) & 0xFF;
+	data = ks8851_eeprom_read(dev, 2);
+	mac_addr[3] = data & 0xFF;
+	mac_addr[2] = (data >> 8) & 0xFF;
+	data = ks8851_eeprom_read(dev, 3);
+	mac_addr[1] = data & 0xFF;
+	mac_addr[0] = (data >> 8) & 0xFF;
+
+	sprintf(str, "%02x:%02x:%02x:%02x:%02x:%02x\n", mac_addr[0],
+			mac_addr[1], mac_addr[2], mac_addr[3], mac_addr[4],
+								mac_addr[5]);
+
+	ret = strlen(str);
+
+	if (copy_to_user((void __user *)buff, str, ret)) {
+		dev_err(&dev->dev, "ks8851 - copy_to_user failed\n");
+		ret = 0;
+	} else {
+		*offp = ret;
+	}
+
+ks8851_cnt_rd_bk:
+	return ret;
+}
+
+/*
+ * Split the buffer `buf' into ':'-separated words.
+ * Return the number of words or <0 on error.
+ */
+#define isdelimiter(c)	((c) == ':')
+static int ks8851_debug_tokenize(char *buf, char *words[], int maxwords)
+{
+	int nwords = 0;
+
+	while (*buf) {
+		char *end;
+
+		/* Skip leading whitespace */
+		while (*buf && isspace(*buf))
+			buf++;
+		if (!*buf)
+			break;	/* oh, it was trailing whitespace */
+
+		/* Run `end' over a word */
+		for (end = buf ; *end && !isdelimiter(*end) ; end++)
+			;
+		/* `buf' is the start of the word, `end' is one past the end */
+
+		if (nwords == maxwords)
+			return -EINVAL;	/* ran out of words[] before bytes */
+		if (*end)
+			*end++ = '\0';	/* terminate the word */
+		words[nwords++] = buf;
+		buf = end;
+	}
+	return nwords;
+}
+
+/**
+ * ks8851_mac_eeprom_write - Write a MAC address in ks8851 companion EEPROM
+ *
+ */
+static ssize_t ks8851_mac_eeprom_write(struct file *filep,
+			const char __user *buff, size_t count, loff_t *offp)
+{
+	struct net_device *dev = filep->private_data;
+	ssize_t ret;
+#define MAXWORDS 6
+	int nwords, i;
+	char *words[MAXWORDS];
+	char tmpbuf[256];
+	unsigned long mac_addr[6];
+
+	if (count == 0)
+		return 0;
+	if (count > sizeof(tmpbuf)-1)
+		return -E2BIG;
+	if (copy_from_user(tmpbuf, buff, count))
+		return -EFAULT;
+	tmpbuf[count] = '\0';
+	dev_dbg(&dev->dev, "%s: read %d bytes from userspace\n",
+			__func__, (int)count);
+
+	nwords = ks8851_debug_tokenize(tmpbuf, words, MAXWORDS);
+	if (nwords != 6) {
+		dev_warn(&dev->dev,
+		"ks8851 MAC address write to EEPROM requires a MAC address " \
+						"like 01:23:45:67:89:AB\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < 6; i++)
+		strict_strtoul(words[i], 16, &mac_addr[i]);
+
+	ks8851_eeprom_write(dev, EEPROM_OP_EWEN, 0, 0);
+
+	ks8851_eeprom_write(dev, EEPROM_OP_WRITE, 1,
+						mac_addr[4] << 8 | mac_addr[5]);
+	mdelay(EEPROM_WRITE_TIME);
+	ks8851_eeprom_write(dev, EEPROM_OP_WRITE, 2,
+						mac_addr[2] << 8 | mac_addr[3]);
+	mdelay(EEPROM_WRITE_TIME);
+	ks8851_eeprom_write(dev, EEPROM_OP_WRITE, 3,
+						mac_addr[0] << 8 | mac_addr[1]);
+	mdelay(EEPROM_WRITE_TIME);
+
+	ks8851_eeprom_write(dev, EEPROM_OP_EWDS, 0, 0);
+
+	dev_dbg(&dev->dev, "MAC address %02lx.%02lx.%02lx.%02lx.%02lx.%02lx "\
+			"written to EEPROM\n", mac_addr[0], mac_addr[1],
+			mac_addr[2], mac_addr[3], mac_addr[4], mac_addr[5]);
+
+	ret = count;
+	*offp += count;
+	return ret;
+}
+
+static const struct file_operations ks8851_mac_eeprom_fops = {
+	.open		= ks8851_mac_eeprom_open,
+	.read		= ks8851_mac_eeprom_read,
+	.write		= ks8851_mac_eeprom_write,
+	.llseek		= ks8851_mac_eeprom_seek,
+	.release	= ks8851_mac_eeprom_release,
+};
+
+int __init ks8851_debug_init(struct net_device *dev)
+{
+	struct ks8851_net *ks = netdev_priv(dev);
+	mode_t mac_access = S_IWUGO;
+
+	if (ks->rc_ccr & CCR_EEPROM) {
+		ks8851_dir = debugfs_create_dir("ks8851", NULL);
+		if (IS_ERR(ks8851_dir))
+			return PTR_ERR(ks8851_dir);
+
+		/* Check ks8851 version and features */
+		mutex_lock(&ks->lock);
+		if (CIDER_REV_GET(ks8851_rdreg16(ks, KS_CIDER)) > 0)
+			mac_access |= S_IRUGO;
+		mutex_unlock(&ks->lock);
+
+		debugfs_create_file("mac_eeprom", mac_access, ks8851_dir, dev,
+						&ks8851_mac_eeprom_fops);
+		debugfs_create_u32("eeprom_size", S_IRUGO | S_IWUGO,
+					ks8851_dir, (u32 *) &(ks->eeprom_size));
+	} else {
+		ks8851_dir = NULL;
+	}
+	return 0;
+}
+
+void ks8851_debug_exit(void)
+{
+	if (ks8851_dir)
+		debugfs_remove_recursive(ks8851_dir);
+}
+#else
+int __init ks8851_debug_init(struct net_device *dev)
+{
+	return 0;
+};
+
+void ks8851_debug_exit(void)
+{
+};
+#endif /* CONFIG_DEBUG_FS */
+
+
 /* driver bus management functions */
 
 static int __devinit ks8851_probe(struct spi_device *spi)
@@ -1649,6 +1862,8 @@ static int __devinit ks8851_probe(struct spi_device *spi)
 		goto err_netdev;
 	}
 
+	ks8851_debug_init(ndev);
+
 	dev_info(&spi->dev, "revision %d, MAC %pM, IRQ %d\n",
 		 CIDER_REV_GET(ks8851_rdreg16(ks, KS_CIDER)),
 		 ndev->dev_addr, ndev->irq);
@@ -1672,6 +1887,7 @@ static int __devexit ks8851_remove(struct spi_device *spi)
 	if (netif_msg_drv(priv))
 		dev_info(&spi->dev, "remove");
 
+	ks8851_debug_exit();
 	unregister_netdev(priv->netdev);
 	free_irq(spi->irq, priv);
 	free_netdev(priv->netdev);
-- 
1.6.3.3

Re: [PATCH 4/4 v2] ks8851: read/write MAC address on companion eeprom through debugfs

[David Miller]

From: Sebastien Jan <s-jan@ti.com>
Date: Wed,  5 May 2010 20:45:55 +0200

> A more elegant alternative to ethtool for updating the ks8851
> MAC address stored on its companion eeprom.
> Using this debugfs interface does not require any knowledge on the
> ks8851 companion eeprom organization to update the MAC address.
> 
> Example to write 01:23:45:67:89:AB MAC address to the companion
> eeprom (assuming debugfs is mounted in /sys/kernel/debug):
> $ echo "01:23:45:67:89:AB" > /sys/kernel/debug/ks8851/mac_eeprom
> 
> Signed-off-by: Sebastien Jan <s-jan@ti.com>

Elegant?  This commit message is the biggest lie ever told.

What makes your ethernet driver so god-damn special that it deserves
to have a private, completely unique, and obscure interface for
setting the permanent ethernet address of a network device?

Tell me how damn elegant it is that users have to learn about this
special, unique, and common with no other driver, interface for
performing this task?

Tell me how damn elegant it is when another driver wants to provide
users with a way to do this too, and they (like you) come up with
their own unique and different interface for doing this.

No, this is the most inelegant patch ever conceived because it totally
ignores the way in which we handle issues like this.

There is no way in the world I'm applying this garbage patch, sorry.

We have an ETHTOOL_GPERMADDR, add a new ETHTOOL_SPERMADDR operation
and then any driver (not just your's) can portably provide this
facility and users will have one, and only one, way of performing this
task.

Re: [PATCH 4/4 v2] ks8851: read/write MAC address on companion eeprom through debugfs

[Sebastien Jan]

Hi david,

On Thursday 06 May 2010 09:25:08 David Miller wrote:
> From: Sebastien Jan <s-jan@ti.com>
> Date: Wed,  5 May 2010 20:45:55 +0200
> 
> > A more elegant alternative to ethtool for updating the ks8851
> > MAC address stored on its companion eeprom.
> > Using this debugfs interface does not require any knowledge on the
> > ks8851 companion eeprom organization to update the MAC address.
> >
> > Example to write 01:23:45:67:89:AB MAC address to the companion
> > eeprom (assuming debugfs is mounted in /sys/kernel/debug):
> > $ echo "01:23:45:67:89:AB" > /sys/kernel/debug/ks8851/mac_eeprom
> >
> > Signed-off-by: Sebastien Jan <s-jan@ti.com>
> 
> Elegant?  This commit message is the biggest lie ever told.
> 
> What makes your ethernet driver so god-damn special that it deserves
> to have a private, completely unique, and obscure interface for
> setting the permanent ethernet address of a network device?
> 
> Tell me how damn elegant it is that users have to learn about this
> special, unique, and common with no other driver, interface for
> performing this task?
> 
> Tell me how damn elegant it is when another driver wants to provide
> users with a way to do this too, and they (like you) come up with
> their own unique and different interface for doing this.
> 
> No, this is the most inelegant patch ever conceived because it totally
> ignores the way in which we handle issues like this.
> 
> There is no way in the world I'm applying this garbage patch, sorry.
> 
> We have an ETHTOOL_GPERMADDR, add a new ETHTOOL_SPERMADDR operation
> and then any driver (not just your's) can portably provide this
> facility and users will have one, and only one, way of performing this
> task.
> 

I agree that my commit message was probably too provocative, sorry for that.

Thank you for shedding some light on ETHTOOL_GPERMADDR and ETHTOOL_SPERMADDR. 
I will look into these interfaces for a proper and generic implementation.