| Message ID | 1380911156-14112-1-git-send-email-dmurphy@ti.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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,
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. >> 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,
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,
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. 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,
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
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,
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,
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,
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) {
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