dm9000: add set_mac_address()

dm9000: add set_mac_address()

[Michael Trimarchi]

Implement set_mac_address() for the dm9000 driver.  This allows changing
the mac address of the interface.

Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
---

--- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/drivers/net/dm9000.c	2007-08-12 21:42:50.000000000 +0200
@@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
 static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
 static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
 			   int value);
+static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
+
 static u16 read_srom_word(board_info_t *, int);
 static void dm9000_rx(struct net_device *);
 static void dm9000_hash_table(struct net_device *);
@@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
 	ndev->stop		 = &dm9000_stop;
 	ndev->get_stats		 = &dm9000_get_stats;
 	ndev->set_multicast_list = &dm9000_hash_table;
+	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	ndev->poll_controller	 = &dm9000_poll_controller;
 #endif
@@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
 	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
 }
 
-#ifdef DM9000_PROGRAM_EEPROM
 /*
  * Write a word data to SROM
  */
@@ -1002,6 +1004,34 @@ write_srom_word(board_info_t * db, int o
 }
 
 /*
+ * dm9000_eth_set_mac_address
+ *
+ * Change the interface's mac address.
+ *
+ */
+static int 
+dm9000_eth_set_mac_address(struct net_device *dev, void *p)
+{
+	board_info_t *db = (board_info_t *) dev->priv;
+	struct sockaddr *addr = p;
+	int i;
+
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
+	if (netif_running(dev))
+		return -EBUSY;
+
+	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
+
+	for (i = 0; i < 3; i++)
+		write_srom_word(db, i, ((u16 *)(addr->sa_data))[i]);
+
+	return 0;
+}
+
+#ifdef DM9000_PROGRAM_EEPROM
+/*
  * Only for development:
  * Here we write static data to the eeprom in case
  * we don't have valid content on a new board

Re: dm9000: add set_mac_address()

[michael trimarchi]

On Sun, Aug 12, 2007 at 11:45:30PM +0200, Michael Buesch wrote:
> On Sunday 12 August 2007 23:38:08 Michael Buesch wrote:
> > On Sunday 12 August 2007 21:27:41 Michael Trimarchi wrote:
> > > Implement set_mac_address() for the dm9000 driver.  This allows changing
> > > the mac address of the interface.
[...]
> > 
> > That is broken on BigEndian architectures.
ok. I'll check it.
> > 
> 
> And is it really desired to change the mac in the ROM here?
> I'd say this should only write the mac to the device registers
> (filters and so on).
Many drivers do that in this way. You can change the MAC address for example
after flashing an arm board using ifconfig eth0 hw <mac address>. 
> 
> -- 
> Greetings Michael.
Cheers

dm9000: add set_mac_address() v2

[michael trimarchi]

Implement set_mac_address() for the dm9000 driver.  This allows changing
the mac address of the interface. Fix BigEndian problem.
 
Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
---

--- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/drivers/net/dm9000.c	2007-08-13 00:15:38.000000000 +0200
@@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
 static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
 static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
 			   int value);
+static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
+
 static u16 read_srom_word(board_info_t *, int);
 static void dm9000_rx(struct net_device *);
 static void dm9000_hash_table(struct net_device *);
@@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
 	ndev->stop		 = &dm9000_stop;
 	ndev->get_stats		 = &dm9000_get_stats;
 	ndev->set_multicast_list = &dm9000_hash_table;
+	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	ndev->poll_controller	 = &dm9000_poll_controller;
 #endif
@@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
 	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
 }
 
-#ifdef DM9000_PROGRAM_EEPROM
 /*
  * Write a word data to SROM
  */
@@ -1002,6 +1004,35 @@ write_srom_word(board_info_t * db, int o
 }
 
 /*
+ * dm9000_eth_set_mac_address
+ *
+ * Change the interface's mac address.
+ *
+ */
+static int 
+dm9000_eth_set_mac_address(struct net_device *dev, void *p)
+{
+	board_info_t *db = (board_info_t *) dev->priv;
+	struct sockaddr *addr = p;
+	int i;
+
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
+	if (netif_running(dev))
+		return -EBUSY;
+
+	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
+
+	for (i = 0; i < 3; i++)
+		write_srom_word(db, i,
+				cpu_to_le16(((u16 *) (addr->sa_data))[i]));
+
+	return 0;
+}
+
+#ifdef DM9000_PROGRAM_EEPROM
+/*
  * Only for development:
  * Here we write static data to the eeprom in case
  * we don't have valid content on a new board

Re: dm9000: add set_mac_address()

[Michael Buesch]

On Sunday 12 August 2007 21:27:41 Michael Trimarchi wrote:
> Implement set_mac_address() for the dm9000 driver.  This allows changing
> the mac address of the interface.
> 
> Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
> ---
> 
> --- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.22/drivers/net/dm9000.c	2007-08-12 21:42:50.000000000 +0200
> @@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
>  static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
>  static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
>  			   int value);
> +static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
> +
>  static u16 read_srom_word(board_info_t *, int);
>  static void dm9000_rx(struct net_device *);
>  static void dm9000_hash_table(struct net_device *);
> @@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
>  	ndev->stop		 = &dm9000_stop;
>  	ndev->get_stats		 = &dm9000_get_stats;
>  	ndev->set_multicast_list = &dm9000_hash_table;
> +	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	ndev->poll_controller	 = &dm9000_poll_controller;
>  #endif
> @@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
>  	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
>  }
>  
> -#ifdef DM9000_PROGRAM_EEPROM
>  /*
>   * Write a word data to SROM
>   */
> @@ -1002,6 +1004,34 @@ write_srom_word(board_info_t * db, int o
>  }
>  
>  /*
> + * dm9000_eth_set_mac_address
> + *
> + * Change the interface's mac address.
> + *
> + */
> +static int 
> +dm9000_eth_set_mac_address(struct net_device *dev, void *p)
> +{
> +	board_info_t *db = (board_info_t *) dev->priv;
> +	struct sockaddr *addr = p;
> +	int i;
> +
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
> +	if (netif_running(dev))
> +		return -EBUSY;
> +
> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> +
> +	for (i = 0; i < 3; i++)
> +		write_srom_word(db, i, ((u16 *)(addr->sa_data))[i]);

That is broken on BigEndian architectures.

-- 
Greetings Michael.

Re: dm9000: add set_mac_address()

[Michael Buesch]

On Sunday 12 August 2007 23:38:08 Michael Buesch wrote:
> On Sunday 12 August 2007 21:27:41 Michael Trimarchi wrote:
> > Implement set_mac_address() for the dm9000 driver.  This allows changing
> > the mac address of the interface.
> > 
> > Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
> > ---
> > 
> > --- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
> > +++ linux-2.6.22/drivers/net/dm9000.c	2007-08-12 21:42:50.000000000 +0200
> > @@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
> >  static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
> >  static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
> >  			   int value);
> > +static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
> > +
> >  static u16 read_srom_word(board_info_t *, int);
> >  static void dm9000_rx(struct net_device *);
> >  static void dm9000_hash_table(struct net_device *);
> > @@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
> >  	ndev->stop		 = &dm9000_stop;
> >  	ndev->get_stats		 = &dm9000_get_stats;
> >  	ndev->set_multicast_list = &dm9000_hash_table;
> > +	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
> >  #ifdef CONFIG_NET_POLL_CONTROLLER
> >  	ndev->poll_controller	 = &dm9000_poll_controller;
> >  #endif
> > @@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
> >  	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
> >  }
> >  
> > -#ifdef DM9000_PROGRAM_EEPROM
> >  /*
> >   * Write a word data to SROM
> >   */
> > @@ -1002,6 +1004,34 @@ write_srom_word(board_info_t * db, int o
> >  }
> >  
> >  /*
> > + * dm9000_eth_set_mac_address
> > + *
> > + * Change the interface's mac address.
> > + *
> > + */
> > +static int 
> > +dm9000_eth_set_mac_address(struct net_device *dev, void *p)
> > +{
> > +	board_info_t *db = (board_info_t *) dev->priv;
> > +	struct sockaddr *addr = p;
> > +	int i;
> > +
> > +	if (!is_valid_ether_addr(addr->sa_data))
> > +		return -EADDRNOTAVAIL;
> > +
> > +	if (netif_running(dev))
> > +		return -EBUSY;
> > +
> > +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> > +
> > +	for (i = 0; i < 3; i++)
> > +		write_srom_word(db, i, ((u16 *)(addr->sa_data))[i]);
> 
> That is broken on BigEndian architectures.
> 

And is it really desired to change the mac in the ROM here?
I'd say this should only write the mac to the device registers
(filters and so on).

-- 
Greetings Michael.

Re: dm9000: add set_mac_address() v2

[Michael Buesch]

On Sunday 12 August 2007 23:20:09 michael trimarchi wrote:
> Implement set_mac_address() for the dm9000 driver.  This allows changing
> the mac address of the interface. Fix BigEndian problem.
>  
> Signed-off-by: Michael Trimarchi <trimarchi@gandalf.sssup.it>
> ---
> 
> --- linux-2.6.22/drivers/net/dm9000.c.orig	2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.22/drivers/net/dm9000.c	2007-08-13 00:15:38.000000000 +0200
> @@ -161,6 +161,8 @@ static irqreturn_t dm9000_interrupt(int,
>  static int dm9000_phy_read(struct net_device *dev, int phyaddr_unsused, int reg);
>  static void dm9000_phy_write(struct net_device *dev, int phyaddr_unused, int reg,
>  			   int value);
> +static int dm9000_eth_set_mac_address(struct net_device *dev, void *p);
> +
>  static u16 read_srom_word(board_info_t *, int);
>  static void dm9000_rx(struct net_device *);
>  static void dm9000_hash_table(struct net_device *);
> @@ -546,6 +548,7 @@ dm9000_probe(struct platform_device *pde
>  	ndev->stop		 = &dm9000_stop;
>  	ndev->get_stats		 = &dm9000_get_stats;
>  	ndev->set_multicast_list = &dm9000_hash_table;
> +	ndev->set_mac_address	 = &dm9000_eth_set_mac_address;
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	ndev->poll_controller	 = &dm9000_poll_controller;
>  #endif
> @@ -986,7 +989,6 @@ read_srom_word(board_info_t * db, int of
>  	return (ior(db, DM9000_EPDRL) + (ior(db, DM9000_EPDRH) << 8));
>  }
>  
> -#ifdef DM9000_PROGRAM_EEPROM
>  /*
>   * Write a word data to SROM
>   */
> @@ -1002,6 +1004,35 @@ write_srom_word(board_info_t * db, int o
>  }
>  
>  /*
> + * dm9000_eth_set_mac_address
> + *
> + * Change the interface's mac address.
> + *
> + */
> +static int 
> +dm9000_eth_set_mac_address(struct net_device *dev, void *p)
> +{
> +	board_info_t *db = (board_info_t *) dev->priv;
> +	struct sockaddr *addr = p;
> +	int i;
> +
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
> +	if (netif_running(dev))
> +		return -EBUSY;
> +
> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> +
> +	for (i = 0; i < 3; i++)
> +		write_srom_word(db, i,
> +				cpu_to_le16(((u16 *) (addr->sa_data))[i]));

Nope.

write_srom_word(db, i, le16_to_cpu(((__le16 *) (addr->sa_data))[i]));

-- 
Greetings Michael.

Re: dm9000: add set_mac_address() v2

[Michael Trimarchi]

>> +		return -EBUSY;
>> +
>> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
>> +
>> +	for (i = 0; i < 3; i++)
>> +		write_srom_word(db, i,
>> +				cpu_to_le16(((u16 *) (addr->sa_data))[i]));
>>     
>
> Nope.
>
> write_srom_word(db, i, le16_to_cpu(((__le16 *) (addr->sa_data))[i]));
>
>   
Are you sure? I don't know how the dm9000 reads the eeprom back.

regards
Michael

Re: dm9000: add set_mac_address() v2

[Michael Buesch]

On Monday 13 August 2007 00:45:17 Michael Trimarchi wrote:
> 
> >> +		return -EBUSY;
> >> +
> >> +	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
> >> +
> >> +	for (i = 0; i < 3; i++)
> >> +		write_srom_word(db, i,
> >> +				cpu_to_le16(((u16 *) (addr->sa_data))[i]));
> >>     
> >
> > Nope.
> >
> > write_srom_word(db, i, le16_to_cpu(((__le16 *) (addr->sa_data))[i]));
> >
> >   
> Are you sure?

Yes I am. cpu_to_le16 simply doesn't make any sense at all here,
while le16_to_cpu does make sense and is indeed correct.
Though they both generate the same asm code. Running sparse
also tells you more about this. ;)

-- 
Greetings Michael.

Re: dm9000: add set_mac_address()

[Jeff Garzik]

set_mac_address should not write to the SROM, as Michael noted.

The proper operations are:

     probe time:
	read MAC address from SROM

     dev open (interface up):
	write dev->dev_addr[] to RX filter (or identity) registers

EEPROM update support is available separately, via an ethtool sub-ioctl.

	Jeff

Re: dm9000: add set_mac_address()

[Michael Trimarchi]

>
>     probe time:
>     read MAC address from SROM
>
>     dev open (interface up):
>     write dev->dev_addr[] to RX filter (or identity) registers
>
> EEPROM update support is available separately, via an ethtool sub-ioctl.
>
>     Jeff
>
ok

regards
michael

Re: dm9000: add set_mac_address()

[Ben Dooks]

On Sun, Aug 12, 2007 at 07:08:52PM -0400, Jeff Garzik wrote:
> set_mac_address should not write to the SROM, as Michael noted.
> 
> The proper operations are:
> 
>     probe time:
> 	read MAC address from SROM
> 
>     dev open (interface up):
> 	write dev->dev_addr[] to RX filter (or identity) registers
> 
> EEPROM update support is available separately, via an ethtool sub-ioctl.

This sounds sensible, I am working on adding ethtool support
and should have a patch available in the next day or so.

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'