[PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Dan Murphy]

If the smsc95xx does not have a valid MAC address stored within
the eeprom then a random number is generated.  The MAC can also
be set by uBoot but the smsc95xx does not have a way to read this.

Create the binding for the smsc95xx so that uBoot can set the MAC
and the code can retrieve the MAC from the modified DTB file.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 Documentation/devicetree/bindings/net/smsc95xx.txt |   17 ++++++++++++++
 drivers/net/usb/smsc95xx.c                         |   24 ++++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/smsc95xx.txt

diff --git a/Documentation/devicetree/bindings/net/smsc95xx.txt b/Documentation/devicetree/bindings/net/smsc95xx.txt
new file mode 100644
index 0000000..4c37280
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc95xx.txt
@@ -0,0 +1,17 @@
+* Smart Mixed-Signal Connectivity (SMSC) 95xx Controller
+
+Required properties:
+None
+
+Optional properties:
+- mac-address - Read the mac address that was stored by uBoot
+- local-address - Read the mac address that was stored by uBoot
+- address - Read the mac address that was stored by uBoot
+
+Examples:
+
+smsc0: smsc95xx@0 {
+	/* Filled in by U-Boot */
+	mac-address = [ 00 00 00 00 00 00 ];
+};
+
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 3f38ba8..baee0bd 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -31,6 +31,9 @@
 #include <linux/crc32.h>
 #include <linux/usb/usbnet.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+
 #include "smsc95xx.h"
 
 #define SMSC_CHIPNAME			"smsc95xx"
@@ -62,6 +65,8 @@
 #define SUSPEND_ALLMODES		(SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
 					 SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
 
+#define SMSC95XX_OF_NAME 	"/smsc95xx@"
+
 struct smsc95xx_priv {
 	u32 mac_cr;
 	u32 hash_hi;
@@ -767,6 +772,25 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 
 static void smsc95xx_init_mac_address(struct usbnet *dev)
 {
+
+#ifdef CONFIG_OF
+	struct device_node *ap;
+	const char *mac = NULL;
+	char *of_name = SMSC95XX_OF_NAME;
+
+	sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id);
+	ap = of_find_node_by_path(of_name);
+	if (ap) {
+		mac = of_get_mac_address(ap);
+		if (is_valid_ether_addr(mac)) {
+			/* Device tree has a mac for this so use that */
+			memcpy(dev->net->dev_addr, mac, ETH_ALEN);
+			netif_dbg(dev, ifup, dev->net, "MAC address read from DTB\n");
+			return;
+		}
+	}
+#endif
+
 	/* try reading mac address from EEPROM */
 	if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
 			dev->net->dev_addr) == 0) {
-- 
1.7.9.5

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Ming Lei]

On Sat, Oct 5, 2013 at 2:25 AM, Dan Murphy <dmurphy@ti.com> wrote:
> If the smsc95xx does not have a valid MAC address stored within
> the eeprom then a random number is generated.  The MAC can also
> be set by uBoot but the smsc95xx does not have a way to read this.
>
> Create the binding for the smsc95xx so that uBoot can set the MAC
> and the code can retrieve the MAC from the modified DTB file.

Suppose there are two smsc95xx usbnet devices connected to usb bus, and
one is built-in, another is hotplug device, can your patch handle the situation
correctly?

>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  Documentation/devicetree/bindings/net/smsc95xx.txt |   17 ++++++++++++++
>  drivers/net/usb/smsc95xx.c                         |   24 ++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/smsc95xx.txt
>
> diff --git a/Documentation/devicetree/bindings/net/smsc95xx.txt b/Documentation/devicetree/bindings/net/smsc95xx.txt
> new file mode 100644
> index 0000000..4c37280
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/smsc95xx.txt
> @@ -0,0 +1,17 @@
> +* Smart Mixed-Signal Connectivity (SMSC) 95xx Controller
> +
> +Required properties:
> +None
> +
> +Optional properties:
> +- mac-address - Read the mac address that was stored by uBoot
> +- local-address - Read the mac address that was stored by uBoot
> +- address - Read the mac address that was stored by uBoot
> +
> +Examples:
> +
> +smsc0: smsc95xx@0 {
> +       /* Filled in by U-Boot */
> +       mac-address = [ 00 00 00 00 00 00 ];
> +};
> +
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 3f38ba8..baee0bd 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -31,6 +31,9 @@
>  #include <linux/crc32.h>
>  #include <linux/usb/usbnet.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_net.h>
> +
>  #include "smsc95xx.h"
>
>  #define SMSC_CHIPNAME                  "smsc95xx"
> @@ -62,6 +65,8 @@
>  #define SUSPEND_ALLMODES               (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
>                                          SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
>
> +#define SMSC95XX_OF_NAME       "/smsc95xx@"
> +
>  struct smsc95xx_priv {
>         u32 mac_cr;
>         u32 hash_hi;
> @@ -767,6 +772,25 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
>
>  static void smsc95xx_init_mac_address(struct usbnet *dev)
>  {
> +
> +#ifdef CONFIG_OF
> +       struct device_node *ap;
> +       const char *mac = NULL;
> +       char *of_name = SMSC95XX_OF_NAME;
> +
> +       sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id);
> +       ap = of_find_node_by_path(of_name);
> +       if (ap) {
> +               mac = of_get_mac_address(ap);
> +               if (is_valid_ether_addr(mac)) {
> +                       /* Device tree has a mac for this so use that */
> +                       memcpy(dev->net->dev_addr, mac, ETH_ALEN);
> +                       netif_dbg(dev, ifup, dev->net, "MAC address read from DTB\n");
> +                       return;
> +               }
> +       }
> +#endif
> +
>         /* try reading mac address from EEPROM */
>         if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
>                         dev->net->dev_addr) == 0) {
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


Thanks,
-- 
Ming Lei

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Dan Murphy]

On 10/06/2013 10:05 AM, Ming Lei wrote:
> On Sat, Oct 5, 2013 at 2:25 AM, Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> wrote:
>> If the smsc95xx does not have a valid MAC address stored within
>> the eeprom then a random number is generated.  The MAC can also
>> be set by uBoot but the smsc95xx does not have a way to read this.
>>
>> Create the binding for the smsc95xx so that uBoot can set the MAC
>> and the code can retrieve the MAC from the modified DTB file.
> Suppose there are two smsc95xx usbnet devices connected to usb bus, and
> one is built-in, another is hotplug device, can your patch handle the situation
> correctly?

Look at this line in the patch below

sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id);

I am appending the dev ID of the device to the of_name here.  As long as init_mac_address is called, the dev.id and the uBoot
entry match then yes.


>> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/net/smsc95xx.txt |   17 ++++++++++++++
>>  drivers/net/usb/smsc95xx.c                         |   24 ++++++++++++++++++++
>>  2 files changed, 41 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/smsc95xx.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/smsc95xx.txt b/Documentation/devicetree/bindings/net/smsc95xx.txt
>> new file mode 100644
>> index 0000000..4c37280
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/smsc95xx.txt
>> @@ -0,0 +1,17 @@
>> +* Smart Mixed-Signal Connectivity (SMSC) 95xx Controller
>> +
>> +Required properties:
>> +None
>> +
>> +Optional properties:
>> +- mac-address - Read the mac address that was stored by uBoot
>> +- local-address - Read the mac address that was stored by uBoot
>> +- address - Read the mac address that was stored by uBoot
>> +
>> +Examples:
>> +
>> +smsc0: smsc95xx@0 {
>> +       /* Filled in by U-Boot */
>> +       mac-address = [ 00 00 00 00 00 00 ];
>> +};
>> +
>> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
>> index 3f38ba8..baee0bd 100644
>> --- a/drivers/net/usb/smsc95xx.c
>> +++ b/drivers/net/usb/smsc95xx.c
>> @@ -31,6 +31,9 @@
>>  #include <linux/crc32.h>
>>  #include <linux/usb/usbnet.h>
>>  #include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/of_net.h>
>> +
>>  #include "smsc95xx.h"
>>
>>  #define SMSC_CHIPNAME                  "smsc95xx"
>> @@ -62,6 +65,8 @@
>>  #define SUSPEND_ALLMODES               (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
>>                                          SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
>>
>> +#define SMSC95XX_OF_NAME       "/smsc95xx@"
>> +
>>  struct smsc95xx_priv {
>>         u32 mac_cr;
>>         u32 hash_hi;
>> @@ -767,6 +772,25 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
>>
>>  static void smsc95xx_init_mac_address(struct usbnet *dev)
>>  {
>> +
>> +#ifdef CONFIG_OF
>> +       struct device_node *ap;
>> +       const char *mac = NULL;
>> +       char *of_name = SMSC95XX_OF_NAME;
>> +
>> +       sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id);
>> +       ap = of_find_node_by_path(of_name);
>> +       if (ap) {
>> +               mac = of_get_mac_address(ap);
>> +               if (is_valid_ether_addr(mac)) {
>> +                       /* Device tree has a mac for this so use that */
>> +                       memcpy(dev->net->dev_addr, mac, ETH_ALEN);
>> +                       netif_dbg(dev, ifup, dev->net, "MAC address read from DTB\n");
>> +                       return;
>> +               }
>> +       }
>> +#endif
>> +
>>         /* try reading mac address from EEPROM */
>>         if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
>>                         dev->net->dev_addr) == 0) {
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>
> Thanks,


-- 
------------------
Dan Murphy

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Ming Lei]

On Mon, Oct 7, 2013 at 1:31 AM, Dan Murphy <dmurphy@ti.com> wrote:
> On 10/06/2013 10:05 AM, Ming Lei wrote:
>> On Sat, Oct 5, 2013 at 2:25 AM, Dan Murphy <dmurphy@ti.com> wrote:
>>> If the smsc95xx does not have a valid MAC address stored within
>>> the eeprom then a random number is generated.  The MAC can also
>>> be set by uBoot but the smsc95xx does not have a way to read this.
>>>
>>> Create the binding for the smsc95xx so that uBoot can set the MAC
>>> and the code can retrieve the MAC from the modified DTB file.
>> Suppose there are two smsc95xx usbnet devices connected to usb bus, and
>> one is built-in, another is hotplug device, can your patch handle the situation
>> correctly?
>
> Look at this line in the patch below
>
> sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id);
>
> I am appending the dev ID of the device to the of_name here.  As long as init_mac_address is called, the dev.id and the uBoot
> entry match then yes.

Currently, non-root-hub usb device is created and added into usb bus without
any platform(device tree) knowledge, so you can't expect the match here.

Also not mention the two smsc95xx devices may attach to two different
usb host controllers(buses).

Thanks,
-- 
Ming Lei

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Dan Murphy]

On 10/07/2013 06:42 AM, Ming Lei wrote:
> On Mon, Oct 7, 2013 at 1:31 AM, Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> wrote:
>> On 10/06/2013 10:05 AM, Ming Lei wrote:
>>> On Sat, Oct 5, 2013 at 2:25 AM, Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> wrote:
>>>> If the smsc95xx does not have a valid MAC address stored within
>>>> the eeprom then a random number is generated.  The MAC can also
>>>> be set by uBoot but the smsc95xx does not have a way to read this.
>>>>
>>>> Create the binding for the smsc95xx so that uBoot can set the MAC
>>>> and the code can retrieve the MAC from the modified DTB file.
>>> Suppose there are two smsc95xx usbnet devices connected to usb bus, and
>>> one is built-in, another is hotplug device, can your patch handle the situation
>>> correctly?
>> Look at this line in the patch below
>>
>> sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id);
>>
>> I am appending the dev ID of the device to the of_name here.  As long as init_mac_address is called, the dev.id and the uBoot
>> entry match then yes.
> Currently, non-root-hub usb device is created and added into usb bus without
> any platform(device tree) knowledge, so you can't expect the match here.

That is correct.  Platform/dev tree will have no concept of the PnP USB dongle therefore there should be no entry in either.

I will need to test this issue with a PnP usb->ethernet dongle.

Dan




> Also not mention the two smsc95xx devices may attach to two different
> usb host controllers(buses).
>
> Thanks,


-- 
------------------
Dan Murphy

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Dan Murphy]

Ming

On 10/07/2013 06:42 AM, Ming Lei wrote:
> On Mon, Oct 7, 2013 at 1:31 AM, Dan Murphy <dmurphy@ti.com> wrote:
>> On 10/06/2013 10:05 AM, Ming Lei wrote:
>>> On Sat, Oct 5, 2013 at 2:25 AM, Dan Murphy <dmurphy@ti.com> wrote:
>>>> If the smsc95xx does not have a valid MAC address stored within
>>>> the eeprom then a random number is generated.  The MAC can also
>>>> be set by uBoot but the smsc95xx does not have a way to read this.
>>>>
>>>> Create the binding for the smsc95xx so that uBoot can set the MAC
>>>> and the code can retrieve the MAC from the modified DTB file.
>>> Suppose there are two smsc95xx usbnet devices connected to usb bus, and
>>> one is built-in, another is hotplug device, can your patch handle the situation
>>> correctly?
>> Look at this line in the patch below
>>
>> sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id);
>>
>> I am appending the dev ID of the device to the of_name here.  As long as init_mac_address is called, the dev.id and the uBoot
>> entry match then yes.
> Currently, non-root-hub usb device is created and added into usb bus without
> any platform(device tree) knowledge, so you can't expect the match here.
>
> Also not mention the two smsc95xx devices may attach to two different
> usb host controllers(buses).
>
> Thanks,

You are correct I don't expect a match for PnP devices only devices that are hard wired.

After thinking of it I should move the OF code below the EEPROM code as the EEPROM should take preference over the DT code.

I will need to post V2 for that.

Dan

-- 
------------------
Dan Murphy

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Ming Lei]

On Thu, Oct 10, 2013 at 8:08 PM, Dan Murphy <dmurphy@ti.com> wrote:
>
> You are correct I don't expect a match for PnP devices only devices that are hard wired.
>
> After thinking of it I should move the OF code below the EEPROM code as the EEPROM should take preference over the DT code.
>
> I will need to post V2 for that.

Your patch still doesn't deal with two smsc95xx devices built in
one board.

The problem is a generic one, looks it is better to do it when OF supports
discoverable bus.


Thanks,
-- 
Ming Lei

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Dan Murphy]

On 10/10/2013 07:39 AM, Ming Lei wrote:
> On Thu, Oct 10, 2013 at 8:08 PM, Dan Murphy <dmurphy@ti.com> wrote:
>> You are correct I don't expect a match for PnP devices only devices that are hard wired.
>>
>> After thinking of it I should move the OF code below the EEPROM code as the EEPROM should take preference over the DT code.
>>
>> I will need to post V2 for that.
> Your patch still doesn't deal with two smsc95xx devices built in
> one board.

Is there a board that has 2 built in smsc devices?

This is all dependent on what the dev.id is of udev.

>
> The problem is a generic one, looks it is better to do it when OF supports
> discoverable bus.
>
>
> Thanks,


-- 
------------------
Dan Murphy

Re: [PATCH] usbnet: smsc95xx: Add device tree input for MAC address

[Ming Lei]

On Thu, Oct 10, 2013 at 8:47 PM, Dan Murphy <dmurphy@ti.com> wrote:
>
> Is there a board that has 2 built in smsc devices?

I don't know, maybe there isn't, but driver should be generic enough, and
as I said it is a generic problem, and people are discussing it, so suggest
to read previous discussions first before post your V2.

Below link is one I remembered, but I think you can find more:

https://lkml.org/lkml/2013/8/11/100

> This is all dependent on what the dev.id is of udev.

I am not sure why it depends on udev, and udev can't
see the device util it is added to system.

Thanks,
-- 
Ming Lei