Re: RFC: Platform data for onboard USB assets

Re: RFC: Platform data for onboard USB assets

[Arnd Bergmann]

On Thursday 17 March 2011 22:47:36 Greg KH wrote:
> > > Patches to fix this, for this specific PandaBoard controller are gladly
> > > accepted.  What's odd is this is explicitly a Linux development board,
> > > so you would think that this could have been caught, and fixed, in the
> > > hardware a long time ago, right?
> > 
> > The way everyone resolves this stuff is by patching their kernel
> > locally.
> 
> Well, that means that the device tree work is going to be useful here,
> right?  :)

I like the idea. Let's make this the first use case where a lot of
people will want to have the device tree on ARM. The patch to the
driver to check for a mac-address property is trivial, and we
can probably come up with a decent way of parsing the device
tree for USB devices, after all there is an existing spec for
it (http://playground.sun.com/1275/bindings/usb/usb-1_0.ps).

	Arnd

8<------
[PATCH] net/smscx5xx: demonstrate use of device tree for mac address

This takes the MAC address for smsc75xx/smsc95xx USB network devices
from a the device tree. This is required to get a usable persistent
address on the popular beagleboard, whose hardware designers
accidentally forgot that an ethernet device really requires an a
MAC address to be functional.

The smsc75xx and smsc95xx drivers are just two copies of the
same code, so better fix both.

Not tested!

Signed-off-by: Arnd Bergmann <arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 753ee6e..0420209 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -29,6 +29,7 @@
 #include <linux/crc32.h>
 #include <linux/usb/usbnet.h>
 #include <linux/slab.h>
+#include <linux/of_device.h>
 #include "smsc75xx.h"
 
 #define SMSC_CHIPNAME			"smsc75xx"
@@ -658,6 +659,8 @@ static int smsc75xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 
 static void smsc75xx_init_mac_address(struct usbnet *dev)
 {
+	void *address;
+
 	/* try reading mac address from EEPROM */
 	if (smsc75xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
 			dev->net->dev_addr) == 0) {
@@ -669,6 +672,13 @@ static void smsc75xx_init_mac_address(struct usbnet *dev)
 		}
 	}
 
+	address = of_get_property(dev->udev->dev.of_node,
+				  "local-mac-address", NULL);
+	if (address) {
+		memcpy(dev->net->dev_addr, address, ETH_ALEN);
+		return;
+	}
+
 	/* no eeprom, or eeprom values are invalid. generate random MAC */
 	random_ether_addr(dev->net->dev_addr);
 	netif_dbg(dev, ifup, dev->net, "MAC address set to random_ether_addr");
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index bc86f4b..74585fb 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -29,6 +29,7 @@
 #include <linux/crc32.h>
 #include <linux/usb/usbnet.h>
 #include <linux/slab.h>
+#include <linux/of_device.h>
 #include "smsc95xx.h"
 
 #define SMSC_CHIPNAME			"smsc95xx"
@@ -639,6 +640,8 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 
 static void smsc95xx_init_mac_address(struct usbnet *dev)
 {
+	void *address;
+
 	/* try reading mac address from EEPROM */
 	if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
 			dev->net->dev_addr) == 0) {
@@ -649,6 +652,13 @@ static void smsc95xx_init_mac_address(struct usbnet *dev)
 		}
 	}
 
+	address = of_get_property(dev->udev->dev.of_node,
+				  "local-mac-address", NULL);
+	if (address) {
+		memcpy(dev->net->dev_addr, address, ETH_ALEN);
+		return;
+	}
+
 	/* no eeprom, or eeprom values are invalid. generate random MAC */
 	random_ether_addr(dev->net->dev_addr);
 	netif_dbg(dev, ifup, dev->net, "MAC address set to random_ether_addr\n");

Re: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/17/2011 10:33 PM, Somebody in the thread at some point said:
> On Thursday 17 March 2011 22:47:36 Greg KH wrote:
>>>> Patches to fix this, for this specific PandaBoard controller are gladly
>>>> accepted.  What's odd is this is explicitly a Linux development board,
>>>> so you would think that this could have been caught, and fixed, in the
>>>> hardware a long time ago, right?
>>>
>>> The way everyone resolves this stuff is by patching their kernel
>>> locally.
>>
>> Well, that means that the device tree work is going to be useful here,
>> right?  :)
>
> I like the idea. Let's make this the first use case where a lot of

You changed your first opinion about tagging "dynamically probed 
devices" with what is effectively platform_data, cool.

> people will want to have the device tree on ARM. The patch to the
> driver to check for a mac-address property is trivial, and we
> can probably come up with a decent way of parsing the device
> tree for USB devices, after all there is an existing spec for
> it (http://playground.sun.com/1275/bindings/usb/usb-1_0.ps).

It doesn't do it already then.

That spec you pointed to from 1998 is obviously going to be a whole 
subproject doing the binding, it seems to fingerprint devices by VID/PID 
if I understood it.

What's the plan for leveraging that level of generality on "dynamically 
probed devices"?  I mean I know what I want to use this for and the 
platform_data scheme covers all the soldered-on-the-board cases fine.

Is there actually a need for sort of not platform_data but universal 
vid_pid_specific_usb_device_option_data coming from the board definition 
file or bootloader for *pluggable* usb devices?  udev seems to be well 
established doing that already in a generic, not-platform-specific way 
that can go in all distros and so on nicely.  Maybe this is just my 
impoverished imagination and people do want, say, some kinds of USB mice 
to operate at higher DPI or whatever when plugged in a specific board 
because it is that board.

BTW the whole RFC patchset I sent was tested on real Panda, including 
the platform end which actually exists.

-Andy

Re: RFC: Platform data for onboard USB assets

[Arnd Bergmann]

On Friday 18 March 2011, Andy Green wrote:
> On 03/17/2011 10:33 PM, Somebody in the thread at some point said:
> > On Thursday 17 March 2011 22:47:36 Greg KH wrote:
> >>
> >> Well, that means that the device tree work is going to be useful here,
> >> right?  :)
> >
> > I like the idea. Let's make this the first use case where a lot of
> 
> You changed your first opinion about tagging "dynamically probed 
> devices" with what is effectively platform_data, cool.

I still don't like the idea of attaching platform_data to more
devices, when we try to move people away from that in other
parts of the kernel, because of the known deficiencies.

Passing a MAC address in a device tree property is a
well-established method that is used on many drivers, and
is portable across operating systems and architectures.

> > people will want to have the device tree on ARM. The patch to the
> > driver to check for a mac-address property is trivial, and we
> > can probably come up with a decent way of parsing the device
> > tree for USB devices, after all there is an existing spec for
> > it (http://playground.sun.com/1275/bindings/usb/usb-1_0.ps).
> 
> It doesn't do it already then.
> 
> That spec you pointed to from 1998 is obviously going to be a whole 
> subproject doing the binding, it seems to fingerprint devices by VID/PID 
> if I understood it.

We don't need to implement the entire binding. My point was that
if we implement a way to attach a device_node to a usb_device, we
should do it in a way that is compatible with that binding, rather
than coming up with a new way.

Most importantly, you can ignore the entire "compatible"-
and "name"-property matching. I think we only need to look
at the "reg" property to walk the bus structure, and any
usb_device we find during the USB hub probing simply gets
linked to its device_node.

> What's the plan for leveraging that level of generality on "dynamically 
> probed devices"?  I mean I know what I want to use this for and the 
> platform_data scheme covers all the soldered-on-the-board cases fine.

Isn't there also a device tree based OMAP tree that can boot on
Panda and is just waiting to get merged?
 
> Is there actually a need for sort of not platform_data but universal 
> vid_pid_specific_usb_device_option_data coming from the board definition 
> file or bootloader for *pluggable* usb devices?  udev seems to be well 
> established doing that already in a generic, not-platform-specific way 
> that can go in all distros and so on nicely.  Maybe this is just my 
> impoverished imagination and people do want, say, some kinds of USB mice 
> to operate at higher DPI or whatever when plugged in a specific board 
> because it is that board.

It should really not be tied to a specific board, and there is a lot
of work going on to remove the need for board-specific source code
files, replacing it all with data structures.

My impression so far is that attaching a struct device_node to
static USB devices can be useful in general, but I wouldn't
go so far to suggest using this for dynamically probed devices.

	Arnd

Re: RFC: Platform data for onboard USB assets

[Mark Brown]

On Fri, Mar 18, 2011 at 04:00:09PM +0100, Arnd Bergmann wrote:
> On Friday 18 March 2011, Andy Green wrote:

> > What's the plan for leveraging that level of generality on "dynamically 
> > probed devices"?  I mean I know what I want to use this for and the 
> > platform_data scheme covers all the soldered-on-the-board cases fine.

> Isn't there also a device tree based OMAP tree that can boot on
> Panda and is just waiting to get merged?

Did the stuff with multiple interrupt controllers get resolved?  That's
pretty important for practical use.  There have been quite a few
iterations of the device tree patches for ARM over the last release but
the iterations I looked at all seemed to be bugfix type stuff.
--
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: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 03:00 PM, Somebody in the thread at some point said:

Hi -

>> You changed your first opinion about tagging "dynamically probed
>> devices" with what is effectively platform_data, cool.
>
> I still don't like the idea of attaching platform_data to more
> devices, when we try to move people away from that in other
> parts of the kernel, because of the known deficiencies.

Whatever way you look at it, data delivered into the driver by Device 
Tree is fundamentally the same action as delivering data into the driver 
by platform_data.  Yes, you query by named element with device context, 
but you end up with the same answer as if you dereference a 
platform_data member.  There are no "known deficiencies" to 
platform_data for this action either, at least, not known to me, I don't 
think lack of typechecking on the pointer itself is an issue given the 
accuracy it can be targeted to a soldered-on-the-board device.

> Passing a MAC address in a device tree property is a
> well-established method that is used on many drivers, and
> is portable across operating systems and architectures.

If you're talking about Device Tree, that itself is not at all "well 
established" let alone servicing drivers from it.  Like I say I don't 
want to seem like I am down on it, but it is very new indeed let's face 
it and few drivers are using it for functional configuration information 
compared to vast numbers using platform_data.

=====> If Device Tree APIs is mandated to implement functionality fixes 
to drivers and platform_data is blocked for this, then we end up with 
different, rotting functionality for platform_data basis and drivers 
that remain broken on the many, many, platforms that don't have and will 
never have Device Tree.  That does NOT sound like the right approach.

I guess the grand plan is to eliminate platform_data by overwhelming it 
with Device Tree refactoring.  But each driver has to be tested and each 
board definition file changed... that is a huge, huge undertaking that 
will not happen in any kind of medium and perhaps not long term either.

So they will have to coexist for a very long while.  A policy of denying 
fixes to platform_data users by enforcing introduction of Device Tree 
APIs and making platform_data users out as troglodytes does not sound 
workable.

> We don't need to implement the entire binding. My point was that
> if we implement a way to attach a device_node to a usb_device, we
> should do it in a way that is compatible with that binding, rather
> than coming up with a new way.

That document is of no interest outside open firmware / Device Tree 
implementation since it is specific to it, and there is no value to 
reference it for a platform_data based solution.

> My impression so far is that attaching a struct device_node to
> static USB devices can be useful in general, but I wouldn't
> go so far to suggest using this for dynamically probed devices.

At least we agree there's no point to target pluggable devices with 
either solution, in which case platform_data and Device Tree provide the 
same end result, plus or minus extra query API.

By the way I intend shortly to extend my patchset to cover Panda WLAN 
case via probed MMC / SDIO device in the same way as USB.  So there will 
then be a second use case for my async platform_data patchset via a 
different subsystem.  Of course, maybe it just doubles the number of 
beatings ^^

-Andy
--
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: RFC: Platform data for onboard USB assets

[David Anders]

On 03/18/2011 12:52 PM, Andy Green wrote:
> On 03/18/2011 03:00 PM, Somebody in the thread at some point said:
>
> Hi -
>
>    
>>> You changed your first opinion about tagging "dynamically probed
>>> devices" with what is effectively platform_data, cool.
>>>        
>> I still don't like the idea of attaching platform_data to more
>> devices, when we try to move people away from that in other
>> parts of the kernel, because of the known deficiencies.
>>      
> Whatever way you look at it, data delivered into the driver by Device
> Tree is fundamentally the same action as delivering data into the driver
> by platform_data.  Yes, you query by named element with device context,
> but you end up with the same answer as if you dereference a
> platform_data member.  There are no "known deficiencies" to
> platform_data for this action either, at least, not known to me, I don't
> think lack of typechecking on the pointer itself is an issue given the
> accuracy it can be targeted to a soldered-on-the-board device.
>
>    
>> Passing a MAC address in a device tree property is a
>> well-established method that is used on many drivers, and
>> is portable across operating systems and architectures.
>>      
> If you're talking about Device Tree, that itself is not at all "well
> established" let alone servicing drivers from it.  Like I say I don't
> want to seem like I am down on it, but it is very new indeed let's face
> it and few drivers are using it for functional configuration information
> compared to vast numbers using platform_data.
>
> =====>  If Device Tree APIs is mandated to implement functionality fixes
> to drivers and platform_data is blocked for this, then we end up with
> different, rotting functionality for platform_data basis and drivers
> that remain broken on the many, many, platforms that don't have and will
> never have Device Tree.  That does NOT sound like the right approach.
>
> I guess the grand plan is to eliminate platform_data by overwhelming it
> with Device Tree refactoring.  But each driver has to be tested and each
> board definition file changed... that is a huge, huge undertaking that
> will not happen in any kind of medium and perhaps not long term either.
>
> So they will have to coexist for a very long while.  A policy of denying
> fixes to platform_data users by enforcing introduction of Device Tree
> APIs and making platform_data users out as troglodytes does not sound
> workable.
>
>    
>> We don't need to implement the entire binding. My point was that
>> if we implement a way to attach a device_node to a usb_device, we
>> should do it in a way that is compatible with that binding, rather
>> than coming up with a new way.
>>      
> That document is of no interest outside open firmware / Device Tree
> implementation since it is specific to it, and there is no value to
> reference it for a platform_data based solution.
>
>    
>> My impression so far is that attaching a struct device_node to
>> static USB devices can be useful in general, but I wouldn't
>> go so far to suggest using this for dynamically probed devices.
>>      
> At least we agree there's no point to target pluggable devices with
> either solution, in which case platform_data and Device Tree provide the
> same end result, plus or minus extra query API.
>
> By the way I intend shortly to extend my patchset to cover Panda WLAN
> case via probed MMC / SDIO device in the same way as USB.  So there will
> then be a second use case for my async platform_data patchset via a
> different subsystem.  Of course, maybe it just doubles the number of
> beatings ^^
>
> -Andy
> --
> 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/
>    


just a note on some of things discussed. the mac address issue applies 
to many development boards including the BeagleBoard XM which uses the 
same SMSC LAN9514 with no eeprom.

if you look at the drivers/net/usb/smsc95xx.c you'll find:

static void smsc95xx_init_mac_address(struct usbnet *dev)
{
     /* try reading mac address from EEPROM */
     if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
             dev->net->dev_addr) == 0) {
         if (is_valid_ether_addr(dev->net->dev_addr)) {
             /* eeprom values are valid so use them */
             netif_dbg(dev, ifup, dev->net, "MAC address read from 
EEPROM\n");
             return;
         }
     }

     /* no eeprom, or eeprom values are invalid. generate random MAC */
     random_ether_addr(dev->net->dev_addr);
     netif_dbg(dev, ifup, dev->net, "MAC address set to 
random_ether_addr\n");
}


this is how the mac address is assigned when no eeprom is present.


if you grep in drivers/net you will find a wide range of network devices 
that use the random_ether_addr() function:


grep -r -e "random_ether_addr" * | wc -l
61

Dave Anders



--
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: RFC: Platform data for onboard USB assets

[Mark Brown]

On Fri, Mar 18, 2011 at 01:20:12PM -0500, David Anders wrote:

> if you grep in drivers/net you will find a wide range of network
> devices that use the random_ether_addr() function:

This is a slightly different case to the one where device tree is most
useful which is the case where there is a MAC assigned to the system but
it's been stored in an alternative location and needs to be programmed
into the NIC by software.

Re: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 06:25 PM, Somebody in the thread at some point said:

Hi -

>> if you grep in drivers/net you will find a wide range of network
>> devices that use the random_ether_addr() function:
>
> This is a slightly different case to the one where device tree is most
> useful which is the case where there is a MAC assigned to the system but
> it's been stored in an alternative location and needs to be programmed
> into the NIC by software.

 From an earlier discussion in IRC, I know David's point is the presence 
of so many calls to random_ether_addr() suggests the "crap, there is no 
EEPROM" state can be reached by all those drivers.

In which case, they are all potential consumers of a MAC "stored in an 
alternative location and needs to be programmed into the NIC by 
software" solution, which he also thinks is needed.

-Andy
--
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: RFC: Platform data for onboard USB assets

[Arnd Bergmann]

On Friday 18 March 2011, Andy Green wrote:
> In which case, they are all potential consumers of a MAC "stored in an 
> alternative location and needs to be programmed into the NIC by 
> software" solution, which he also thinks is needed.

Note that there is also of_get_mac_address(), which is meant to
deal with this exact problem.

	Arnd

Re: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 09:11 PM, Somebody in the thread at some point said:
> On Friday 18 March 2011, Andy Green wrote:
>> In which case, they are all potential consumers of a MAC "stored in an
>> alternative location and needs to be programmed into the NIC by
>> software" solution, which he also thinks is needed.
>
> Note that there is also of_get_mac_address(), which is meant to
> deal with this exact problem.

Curiously grep finds it in 7 drivers...

Oh it only solves any problem if you are using Open Firmware, like 
almost everything is not using.

I see.

-Andy


--
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: RFC: Platform data for onboard USB assets

[Arnd Bergmann]

On Friday 18 March 2011 18:52:13 Andy Green wrote:
> On 03/18/2011 03:00 PM, Somebody in the thread at some point said:
> 
> >> You changed your first opinion about tagging "dynamically probed
> >> devices" with what is effectively platform_data, cool.
> >
> > I still don't like the idea of attaching platform_data to more
> > devices, when we try to move people away from that in other
> > parts of the kernel, because of the known deficiencies.
> 
> Whatever way you look at it, data delivered into the driver by Device 
> Tree is fundamentally the same action as delivering data into the driver 
> by platform_data.  Yes, you query by named element with device context, 
> but you end up with the same answer as if you dereference a 
> platform_data member.

Yes. Both approaches are essentially hacks to deal with the
deficiencies of hardware probing in existing systems.

In a perfect world, we would need neither the device tree nor
any platform data at all, because we'd be able to ask the hardware
or the fictionary correct firmware about what the properties
of the hardware are. This works to a surprisingly large extent
on server hardware, but much less so on typical embedded systems.

> There are no "known deficiencies" to 
> platform_data for this action either, at least, not known to me, I don't 
> think lack of typechecking on the pointer itself is an issue given the 
> accuracy it can be targeted to a soldered-on-the-board device.

The main deficiency of platform_data is that it's very inflexible,
you have to write code for each new board you want to support,
something that we've generally moved away from in Linux a decade
ago.

Another problem is that you need to hardcode data structures,
which is arguably less flexible than key/value pairs.

> > Passing a MAC address in a device tree property is a
> > well-established method that is used on many drivers, and
> > is portable across operating systems and architectures.
> 
> If you're talking about Device Tree, that itself is not at all "well 
> established" let alone servicing drivers from it.  Like I say I don't 
> want to seem like I am down on it, but it is very new indeed let's face 
> it and few drivers are using it for functional configuration information 
> compared to vast numbers using platform_data.

I mean specifically passing MAC addresses using the device tree.

$ git grep -l "local-mac-address" | wc -l
142

That is 142 unique files in the kernel referencing this (mostly
powerpc, but also some others), both device drivers and dts files,
plus a handful of drivers that use the nonstandard "mac-address"
property instead of "local-mac-address".

> =====> If Device Tree APIs is mandated to implement functionality fixes 
> to drivers and platform_data is blocked for this, then we end up with 
> different, rotting functionality for platform_data basis and drivers 
> that remain broken on the many, many, platforms that don't have and will 
> never have Device Tree.  That does NOT sound like the right approach.

See the device tree fragment patches that Grant just posted.
It should become really easy to combine both methods, or to
gradually migrate without breaking anything.

> I guess the grand plan is to eliminate platform_data by overwhelming it 
> with Device Tree refactoring.  But each driver has to be tested and each 
> board definition file changed... that is a huge, huge undertaking that 
> will not happen in any kind of medium and perhaps not long term either.
> 
> So they will have to coexist for a very long while.  A policy of denying 
> fixes to platform_data users by enforcing introduction of Device Tree 
> APIs and making platform_data users out as troglodytes does not sound 
> workable.

No, that is not the plan. The platform data is well-established
for deeply embedded systems (blackfin, arm-nommu, mips32, ...) where you
never want to build a kernel for multiple boards anyway. We also
have ways to generate platform_data from the device tree properties
to allow the same driver to be used by systems with or without
full device trees, and we have the fragments I mentioned that work
in the opposite way.

No need to be so negative here, there is no fundamental problem.

The only point I'm trying to get across is that the general move
is away from hardcoding settings in device drivers and per-board
platform_data towards probing stuff properly where possible, and
using device_node properties is the preferred method for new
code where that is not possible.

The USB layer is not architecture specific, so when we add hacks
to it for dealing with nondiscoverable hardware properties, we
should use methods that are accepted everywhere, which platform
data is not.

	Arnd
--
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: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 08:06 PM, Somebody in the thread at some point said:

Hi -

> The main deficiency of platform_data is that it's very inflexible,
> you have to write code for each new board you want to support,
> something that we've generally moved away from in Linux a decade
> ago.

Well: Greg was also reduced to explaining that device renaming in 
userland was decided "a long time ago".  It's not argumentation, it is 
an appeal to an alleged tradition.

You think that striving away to create this Device Tree description of a 
specific board and maintaining it in a bootloader is LESS work somehow 
that registering platform devices in an array in the board definition 
file?  I think not.

> Another problem is that you need to hardcode data structures,
> which is arguably less flexible than key/value pairs.

After you dereferenced your static string via your API, and I 
dereferenced my platform_data member, we both end up with a pointer to 
data.  Board definition file also gets a chance to set that data at 
runtime: for example, take a look at the MAC-setting part of my 
patchset.  What exactly was your point?

> That is 142 unique files in the kernel referencing this (mostly
> powerpc, but also some others), both device drivers and dts files,

Powerpc would appear to be fairly considered as legacy, not the future.

>> =====>  If Device Tree APIs is mandated to implement functionality fixes
>> to drivers and platform_data is blocked for this, then we end up with
>> different, rotting functionality for platform_data basis and drivers
>> that remain broken on the many, many, platforms that don't have and will
>> never have Device Tree.  That does NOT sound like the right approach.
>
> See the device tree fragment patches that Grant just posted.
> It should become really easy to combine both methods, or to
> gradually migrate without breaking anything.

I take it Grant got over his initial offhand opinion of this RFC -->

''Oh, please no.

platform_data is an ugly non-type-checked anonymous pointer.  If you
need to pass data to a driver, use something better designed.  A
device tree fragment would work, or provide some kind of query api.
platform_data is definitely the wrong approach.''

I'm super happy if he mastered his distress and suddenly -- no doubt 
completely asynchronously to this thread -- decided to interoperate with 
platform_data as equals.  If that is indeed what has happened.

> The USB layer is not architecture specific, so when we add hacks
> to it for dealing with nondiscoverable hardware properties, we
> should use methods that are accepted everywhere, which platform
> data is not.

EVERY struct device has a platform_data member.

In the very deepest sense, platform_data is already accepted EVERYWHERE 
including USB and MMC / SDIO devices.

It is not platform_data that has to make its case.

-Andy

Re: RFC: Platform data for onboard USB assets

[Mark Brown]

On Fri, Mar 18, 2011 at 09:33:00PM +0000, Andy Green wrote:

> Well: Greg was also reduced to explaining that device renaming in  
> userland was decided "a long time ago".  It's not argumentation, it is  
> an appeal to an alleged tradition.

The story with device renaming is fairly simple - nobody could agree on
what the ideal names should be and different userlands ended up wanting
different things so rather than try to keep everyone happy the kernel
picked the simplest policy possible and let userland override it to its
heart's content.

> You think that striving away to create this Device Tree description of a  
> specific board and maintaining it in a bootloader is LESS work somehow  
> that registering platform devices in an array in the board definition  
> file?  I think not.

It's more the fact that it can be distributed separately to the kernel
which reduces the pressure to mainline the basic board description stuff
for ongoing maintinance.
--
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: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 11:25 PM, Somebody in the thread at some point said:
> On Fri, Mar 18, 2011 at 09:33:00PM +0000, Andy Green wrote:
>
>> Well: Greg was also reduced to explaining that device renaming in
>> userland was decided "a long time ago".  It's not argumentation, it is
>> an appeal to an alleged tradition.
>
> The story with device renaming is fairly simple - nobody could agree on
> what the ideal names should be and different userlands ended up wanting
> different things so rather than try to keep everyone happy the kernel
> picked the simplest policy possible and let userland override it to its
> heart's content.
>
>> You think that striving away to create this Device Tree description of a
>> specific board and maintaining it in a bootloader is LESS work somehow
>> that registering platform devices in an array in the board definition
>> file?  I think not.
>
> It's more the fact that it can be distributed separately to the kernel
> which reduces the pressure to mainline the basic board description stuff
> for ongoing maintinance.

However that was not the claim.

The claim was that there is a burden with platform_data that it is 
"inflexible", which I dealt with separately, and -->

''...you have to write code for each new board you want to support,
something that we've generally moved away from in Linux a decade
ago. ''

You very much "have to write code for each new board you want to 
support" with Device Tree, so this point is bogus when contrasting the 
attributes of platform_data against Device Tree.

-Andy
--
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: RFC: Platform data for onboard USB assets

[Grant Likely]

On Fri, Mar 18, 2011 at 2:06 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> No, that is not the plan. The platform data is well-established
> for deeply embedded systems (blackfin, arm-nommu, mips32, ...) where you
> never want to build a kernel for multiple boards anyway. We also
> have ways to generate platform_data from the device tree properties
> to allow the same driver to be used by systems with or without
> full device trees, and we have the fragments I mentioned that work
> in the opposite way.

Not actually true.  We have drivers that can use both platform_data
and device tree data, and for platform devices the resource table is
automatically populated with irqs and register ranges, but
platform_data is driver specific which prevents any automatic
translation.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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: RFC: Platform data for onboard USB assets

[Benjamin Herrenschmidt]

On Fri, 2011-03-18 at 21:06 +0100, Arnd Bergmann wrote:
> 
> In a perfect world, we would need neither the device tree nor
> any platform data at all, because we'd be able to ask the hardware
> or the fictionary correct firmware about what the properties
> of the hardware are. This works to a surprisingly large extent
> on server hardware, but much less so on typical embedded systems.

Properties of the HW per-se but also binding information, ie, what is
connected to what outside of the main bus path (think clock/power
control etc...). Even server / desktop is affected here, and nobody sane
thinks ACPI is a -good- solution here tho it works mostly on x86 :-)

Cheers,
Ben.

Re: RFC: Platform data for onboard USB assets

[Grant Likely]

On Fri, Mar 18, 2011 at 11:52 AM, Andy Green <andy-/Zus8d0mwwtBDgjK7y7TUQ@public.gmane.org> wrote:
> On 03/18/2011 03:00 PM, Somebody in the thread at some point said:
>
> Hi -
>
>>> You changed your first opinion about tagging "dynamically probed
>>> devices" with what is effectively platform_data, cool.
>>
>> I still don't like the idea of attaching platform_data to more
>> devices, when we try to move people away from that in other
>> parts of the kernel, because of the known deficiencies.
>
> Whatever way you look at it, data delivered into the driver by Device Tree
> is fundamentally the same action as delivering data into the driver by
> platform_data.  Yes, you query by named element with device context, but you
> end up with the same answer as if you dereference a platform_data member.
>  There are no "known deficiencies" to platform_data for this action either,
> at least, not known to me, I don't think lack of typechecking on the pointer
> itself is an issue given the accuracy it can be targeted to a
> soldered-on-the-board device.

Apologies if we got a little carried away on the device tree side
topic; it is something that needs to be investigated regardless and
that unfortunately ended up co-opting this thread.  You're right that
on ARM device tree is optional and a solution is required for
!CONFIG_OF.

However, at issue here is that platform_data sucks hard, and
asynchronous platform data sucks harder.  I think I can go out on a
limb and say that platform_data is viewed with distaste by more people
than just Arnd and me.  It sucks because it is an anonymous pointer
with absolutely zero chance of verifying that the driver has the right
thing when it comes out the other end at the driver.  This means the
very real possibility of dereferencing the wrong structure and the
kernel oopsing or worse.

For current {platform,i2c,spi} device users, there is at least some
level of protection in that the specific driver and the specific pdata
is specified at exactly the same time in the same place in the source
code.  If a driver changes the pdata structure it expects, then it is
easy to find all the users (at least for all in-tree users), but it
still sucks.  gcc cannot tell us when it is wrong, and the kernel
cannot validate it at runtime.  One reason the DT has been preferred
over platform_data in this discussion is because it does *not* carry
the risk of dereferencing the wrong structure.

Asynchronously attached pdata sucks harder because the selected driver
is completely dissociated from the pdata type.  Not even the i2c and
spi board info structures have this issue.  At least the board info
structures have the driver and the pdata settings co-located.

I certainly have no intention of trying to migrate
{platform,i2c,spi}_device away from platform_data, but I will actively
nack any attempt to bring it into other subsystems.  There are better
ways.  Device tree is one option, but I will accept other approaches.
Using domain specific api, such as to retrieve the correct MAC address
is one idea that's been raised.  Regardless, the ability to validate
the data passed to the driver, either at compile or runtime, is a hard
requirement in my mind.

>> Passing a MAC address in a device tree property is a
>> well-established method that is used on many drivers, and
>> is portable across operating systems and architectures.
>
> If you're talking about Device Tree, that itself is not at all "well
> established" let alone servicing drivers from it.  Like I say I don't want
> to seem like I am down on it, but it is very new indeed let's face it and
> few drivers are using it for functional configuration information compared
> to vast numbers using platform_data.
>
> =====> If Device Tree APIs is mandated to implement functionality fixes to
> drivers and platform_data is blocked for this, then we end up with
> different, rotting functionality for platform_data basis and drivers that
> remain broken on the many, many, platforms that don't have and will never
> have Device Tree.  That does NOT sound like the right approach.
>
> I guess the grand plan is to eliminate platform_data by overwhelming it with
> Device Tree refactoring.  But each driver has to be tested and each board
> definition file changed... that is a huge, huge undertaking that will not
> happen in any kind of medium and perhaps not long term either.

Nope, not the plan.

> At least we agree there's no point to target pluggable devices with either
> solution, in which case platform_data and Device Tree provide the same end
> result, plus or minus extra query API.

Right, if it is detectable it has no business being described
anywhere, whether it be platform_data, a dt node, or something else.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 09:28 PM, Somebody in the thread at some point said:

Hi -

> Apologies if we got a little carried away on the device tree side
> topic; it is something that needs to be investigated regardless and
> that unfortunately ended up co-opting this thread.  You're right that
> on ARM device tree is optional and a solution is required for
> !CONFIG_OF.

I appreciate your candour.

> However, at issue here is that platform_data sucks hard, and
> asynchronous platform data sucks harder.  I think I can go out on a

Personally, I spend a lot of my life trying to back up assertions with 
provable statements and logic.

> limb and say that platform_data is viewed with distaste by more people
> than just Arnd and me.  It sucks because it is an anonymous pointer
> with absolutely zero chance of verifying that the driver has the right
> thing when it comes out the other end at the driver.  This means the
> very real possibility of dereferencing the wrong structure and the
> kernel oopsing or worse.

... and if there is no problem with indeterminism for targeting that 
pointer, what you are saying is just blather.

In fact the normal use for platform_data is to be pointed to by the very 
same struct that defines the device in board definition file.  There is 
NO chance of any dropped ball if the author of the board definition file 
had it right: none.  And again, any error here is deterministic, so you 
are talking about a case where the board definition file author screwed 
it up and didn't bother to test: it is always wrong.  Okay; it is true 
that if the author writes crap and doesn't test it the result is not 
good.  Same goes for Device Tree.

So this claim against platform_data is worthless.

> Asynchronously attached pdata sucks harder because the selected driver
> is completely dissociated from the pdata type.  Not even the i2c and
> spi board info structures have this issue.  At least the board info
> structures have the driver and the pdata settings co-located.

Hm.  I am not sure how many times I used the phrase "hardwired", or 
"wired on the board" or similar, but I think it must add up by now. 
This leads to determinism.

> I certainly have no intention of trying to migrate
> {platform,i2c,spi}_device away from platform_data, but I will actively
> nack any attempt to bring it into other subsystems.  There are better

Correct me if I am wrong, but if you deploy logic to lead to NAKing 
stuff that seems wrong to you, it makes you a valuable member of the 
community.  If you cannot actually explain what the problem is 
coherently -- perhaps especially when it touches upon stuff in conflict 
with your personal hobby-horse -- then you should carefully consider if 
a NAK is appropriate or if you lose credibility by making such threats 
not backed up by logic, but - it seems to me - emotion.

I do not mind if I am fairly NAKed.  That has happened in the past and I 
made good efforts to understand what I missed and and learn.

What I find so difficult in this thread is the very poor argumentation 
deployed against my proposal.  You are actually reduced to arguing by 
authority, "because I am in a position to NAK you, I will, until you 
give up" is your approach.  I just have contempt for it, Grant.

It tells me you do not actually have faith in your own position, or you 
would be explaining my stupidity in clear terms "even I could understand".
	
I already have good reasons to continue and do the SDIO implementation: 
your opinion is not a factor, so NAK away how you feel you need to so 
you feel better.

> ways.  Device tree is one option, but I will accept other approaches.

That's good, because I have patches for my approach, I hope you will 
give them the consideration they deserve.

> Using domain specific api, such as to retrieve the correct MAC address
> is one idea that's been raised.  Regardless, the ability to validate
> the data passed to the driver, either at compile or runtime, is a hard
> requirement in my mind.

And in the (usual SoC) case where there is no indeterminism and the data 
is always as intended?  Your point is again worthless.

>> At least we agree there's no point to target pluggable devices with either
>> solution, in which case platform_data and Device Tree provide the same end
>> result, plus or minus extra query API.
>
> Right, if it is detectable it has no business being described
> anywhere, whether it be platform_data, a dt node, or something else.

So sad that you, head Device Tree dude, don't seem to understand there 
is a class of information not available at the CPU; not available at the 
IP unit, but which must be passed in externally, eg, OMAP I2C bus width 
mapping.

-Andy
--
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: RFC: Platform data for onboard USB assets

[Benjamin Herrenschmidt]

On Thu, 2011-03-17 at 23:22 +0000, Andy Green wrote:
> 
> Is there actually a need for sort of not platform_data but universal 
> vid_pid_specific_usb_device_option_data coming from the board
> definition 
> file or bootloader for *pluggable* usb devices?  udev seems to be
> well 
> established doing that already in a generic, not-platform-specific
> way 
> that can go in all distros and so on nicely.  Maybe this is just my 
> impoverished imagination and people do want, say, some kinds of USB
> mice 
> to operate at higher DPI or whatever when plugged in a specific board 
> because it is that board. 

Except that vid/pid are the -WRONG- way to do that. Your device is
on-board, the proper way to identify it uniquely is it's topological
location, ie path of HW port numbers.

Cheers,
Ben.

Re: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 10:37 PM, Somebody in the thread at some point said:
> On Thu, 2011-03-17 at 23:22 +0000, Andy Green wrote:
>>
>> Is there actually a need for sort of not platform_data but universal
>> vid_pid_specific_usb_device_option_data coming from the board
>> definition
>> file or bootloader for *pluggable* usb devices?  udev seems to be
>> well
>> established doing that already in a generic, not-platform-specific
>> way
>> that can go in all distros and so on nicely.  Maybe this is just my
>> impoverished imagination and people do want, say, some kinds of USB
>> mice
>> to operate at higher DPI or whatever when plugged in a specific board
>> because it is that board.
>
> Except that vid/pid are the -WRONG- way to do that. Your device is
> on-board, the proper way to identify it uniquely is it's topological
> location, ie path of HW port numbers.

Thanks Ben.

I'll go change that VID/PID stuff in my patches right away.

-Andy

Re: RFC: Platform data for onboard USB assets

[Greg KH]

On Thu, Mar 17, 2011 at 11:33:01PM +0100, Arnd Bergmann wrote:
> On Thursday 17 March 2011 22:47:36 Greg KH wrote:
> > > > Patches to fix this, for this specific PandaBoard controller are gladly
> > > > accepted.  What's odd is this is explicitly a Linux development board,
> > > > so you would think that this could have been caught, and fixed, in the
> > > > hardware a long time ago, right?
> > > 
> > > The way everyone resolves this stuff is by patching their kernel
> > > locally.
> > 
> > Well, that means that the device tree work is going to be useful here,
> > right?  :)
> 
> I like the idea. Let's make this the first use case where a lot of
> people will want to have the device tree on ARM. The patch to the
> driver to check for a mac-address property is trivial, and we
> can probably come up with a decent way of parsing the device
> tree for USB devices, after all there is an existing spec for
> it (http://playground.sun.com/1275/bindings/usb/usb-1_0.ps).
> 
> 	Arnd
> 
> 8<------
> [PATCH] net/smscx5xx: demonstrate use of device tree for mac address
> 
> This takes the MAC address for smsc75xx/smsc95xx USB network devices
> from a the device tree. This is required to get a usable persistent
> address on the popular beagleboard, whose hardware designers
> accidentally forgot that an ethernet device really requires an a
> MAC address to be functional.
> 
> The smsc75xx and smsc95xx drivers are just two copies of the
> same code, so better fix both.
> 
> Not tested!
> 
> Signed-off-by: Arnd Bergmann <arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

<snip>

Very nice.

Andy and Mark, would this patch work for you?

thanks,

greg k-h

Re: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/17/2011 10:53 PM, Somebody in the thread at some point said:

>> Not tested!
>>
>> Signed-off-by: Arnd Bergmann<arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> <snip>
>
> Very nice.
>
> Andy and Mark, would this patch work for you?

You do realize this untested patch depends on 13 year old vapour 
definition of general usb device tagging in Device Tree that does not 
exist yet?

-Andy
--
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: RFC: Platform data for onboard USB assets

[Greg KH]

On Thu, Mar 17, 2011 at 11:18:41PM +0000, Andy Green wrote:
> On 03/17/2011 10:53 PM, Somebody in the thread at some point said:
> 
> >>Not tested!
> >>
> >>Signed-off-by: Arnd Bergmann<arnd.bergmann@linaro.org>
> >
> ><snip>
> >
> >Very nice.
> >
> >Andy and Mark, would this patch work for you?
> 
> You do realize this untested patch depends on 13 year old vapour
> definition of general usb device tagging in Device Tree that does
> not exist yet?

As you get to create your own device tree for your board, it will then
exist, right?

thanks,

greg k-h

Re: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/17/2011 11:25 PM, Somebody in the thread at some point said:
> On Thu, Mar 17, 2011 at 11:18:41PM +0000, Andy Green wrote:
>> On 03/17/2011 10:53 PM, Somebody in the thread at some point said:
>>
>>>> Not tested!
>>>>
>>>> Signed-off-by: Arnd Bergmann<arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>
>>> <snip>
>>>
>>> Very nice.
>>>
>>> Andy and Mark, would this patch work for you?
>>
>> You do realize this untested patch depends on 13 year old vapour
>> definition of general usb device tagging in Device Tree that does
>> not exist yet?
>
> As you get to create your own device tree for your board, it will then
> exist, right?

No, if you read Arnd's post you will find Device Tree does not support 
targeting USB devices yet, and if you further read the 1998 document he 
points to as the basis of actually implementing it, it seems to me at 
least it'll be a little project yet to do that on Linux side.

That is why his "very nice" patch is untested, it literally doesn't work 
as things stand so he is unable to test it.  Yes, you will also need a 
device tree for your board.  When you ask if it "works for me", the 
answer is it doesn't work for me, the author nor anybody else.

The Device Tree guys at first trashing and then co-opting this RFC has 
the tendency to make me sound like a Device Tree basher.  Actually I 
don't think it's evil so long as it reflects the reality and necessity 
that it is completely optional, there is an elephant in the room about 
that and how it competes with platform_data.  At the moment Device Tree 
is "all things to all men", and few things stay like that for long.

-Andy
--
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: RFC: Platform data for onboard USB assets

[Benjamin Herrenschmidt]

On Fri, 2011-03-18 at 07:42 +0000, Andy Green wrote:
> 
> No, if you read Arnd's post you will find Device Tree does not support 
> targeting USB devices yet, and if you further read the 1998 document he 
> points to as the basis of actually implementing it, it seems to me at 
> least it'll be a little project yet to do that on Linux side.

You are getting seriously full of sh*t here. Read again what Arnd said
rather than twisting it to try to make your point.

The age of the OF binding document for USB has no relevance. The fact
that it's a very simple binding however does. As Arnd mention, nobody
says that somebody/something has to generate a full and complete
representation of USB for your board, that would be beyond the scope.

However, it should be trivial to layout in the fixed part of the board
device-tree nodes representing on-board (fixed) devices with the right
amount of properties for allowing the linkage to happen between those
nodes and the Linux usb_device object.

Basically it boils down to one property afaik, "reg". That's about it
(oh and possibly #address-cells and compatible for hubs that are on the
way to the device if any). About 15mn of work if you are familiar with
the .dts syntax, maybe twice as much if you are not.  

Cheers,
Ben.

Re: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 10:54 PM, Somebody in the thread at some point said:
> On Fri, 2011-03-18 at 07:42 +0000, Andy Green wrote:
>>
>> No, if you read Arnd's post you will find Device Tree does not support
>> targeting USB devices yet, and if you further read the 1998 document he
>> points to as the basis of actually implementing it, it seems to me at
>> least it'll be a little project yet to do that on Linux side.
>
> You are getting seriously full of sh*t here. Read again what Arnd said
> rather than twisting it to try to make your point.
>
> The age of the OF binding document for USB has no relevance. The fact

OK, thanks for pointing out I am full of shit.

-Andy
--
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: RFC: Platform data for onboard USB assets

[Grant Likely]

On Thu, Mar 17, 2011 at 11:18:41PM +0000, Andy Green wrote:
> On 03/17/2011 10:53 PM, Somebody in the thread at some point said:
> 
> >>Not tested!
> >>
> >>Signed-off-by: Arnd Bergmann<arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >
> ><snip>
> >
> >Very nice.
> >
> >Andy and Mark, would this patch work for you?
> 
> You do realize this untested patch depends on 13 year old vapour
> definition of general usb device tagging in Device Tree that does
> not exist yet?

IIRC, not vapour.  I believe this binding is currently used by Open
Firmware on existing PowerPC, SPARC and x86 machines.  Linux doesn't
use the binding because up to this point Linux hasn't cared about how
firmware initialized the usb bus.  It just reinitializes everything
anyway.

g.

Re: RFC: Platform data for onboard USB assets

[Arnd Bergmann]

On Friday 18 March 2011, Grant Likely wrote:
> On Thu, Mar 17, 2011 at 11:18:41PM +0000, Andy Green wrote:
> > On 03/17/2011 10:53 PM, Somebody in the thread at some point said:
> > 
> > >>Not tested!
> > >>
> > >>Signed-off-by: Arnd Bergmann<arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > >
> > ><snip>
> > >
> > >Very nice.
> > >
> > >Andy and Mark, would this patch work for you?
> > 
> > You do realize this untested patch depends on 13 year old vapour
> > definition of general usb device tagging in Device Tree that does
> > not exist yet?
> 
> IIRC, not vapour.  I believe this binding is currently used by Open
> Firmware on existing PowerPC, SPARC and x86 machines.  Linux doesn't
> use the binding because up to this point Linux hasn't cared about how
> firmware initialized the usb bus.  It just reinitializes everything
> anyway.

I'm not proposing to use the binding for the complete USB probing, that
would just duplicate the probing code that we already have. We could
howeve check some properties from the binding against what the Linux
drivers see.

Most importantly, we can assign the device_node pointer for each
hardwired USB device to the usb_device structure, that should be
really simple. The only reason why I pointed to the spec is to
make sure we don't put incompatible properties in the tree and
instead just do whatever we need according to the spec but leave
out all the optional parts.

	Arnd

Re: RFC: Platform data for onboard USB assets

[Grant Likely]

On Thu, Mar 17, 2011 at 11:33:01PM +0100, Arnd Bergmann wrote:
> On Thursday 17 March 2011 22:47:36 Greg KH wrote:
> > > > Patches to fix this, for this specific PandaBoard controller are gladly
> > > > accepted.  What's odd is this is explicitly a Linux development board,
> > > > so you would think that this could have been caught, and fixed, in the
> > > > hardware a long time ago, right?
> > > 
> > > The way everyone resolves this stuff is by patching their kernel
> > > locally.
> > 
> > Well, that means that the device tree work is going to be useful here,
> > right?  :)
> 
> I like the idea. Let's make this the first use case where a lot of
> people will want to have the device tree on ARM. The patch to the
> driver to check for a mac-address property is trivial, and we
> can probably come up with a decent way of parsing the device
> tree for USB devices, after all there is an existing spec for
> it (http://playground.sun.com/1275/bindings/usb/usb-1_0.ps).
> 

It would be fairly easy to handle.  In the model we've been using for
the flattened tree so far, nodes for detectable are optional and
almost always been omitted (as opposed to OpenFirmware which always
populates the devices, whether they are detectable or not).

However, it's always been an option to populate nodes for devices that
can also be detected if additional data needs to be supplied to make
it work.  For example, providing IRQ swizzle data for PCI host
controllers.

In this case, there needs to be a generic mechanism for attaching
device node pointers to USB devices when the device is attached or
removed from the bus from the perspective of Linux.

The USB binding that you linked uses a single cell containing the hub
or host contoller port to address a usb device.  As long as the device
tree topology mirrors the topology of the USB tree, and providing that
an of_node is associated with the USB host controller device in Linux,
then the USB core code should have sufficient knowledge to set and
clear each USB device's .of_node pointer as devices are attached and
removed.

The patch below also looks right to me.  I believe it also has the
advantage of u-boot already knowing how to update the
local-mac-address property at boot time.

g.

> 	Arnd
> 
> 8<------
> [PATCH] net/smscx5xx: demonstrate use of device tree for mac address
> 
> This takes the MAC address for smsc75xx/smsc95xx USB network devices
> from a the device tree. This is required to get a usable persistent
> address on the popular beagleboard, whose hardware designers
> accidentally forgot that an ethernet device really requires an a
> MAC address to be functional.
> 
> The smsc75xx and smsc95xx drivers are just two copies of the
> same code, so better fix both.
> 
> Not tested!
> 
> Signed-off-by: Arnd Bergmann <arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
> index 753ee6e..0420209 100644
> --- a/drivers/net/usb/smsc75xx.c
> +++ b/drivers/net/usb/smsc75xx.c
> @@ -29,6 +29,7 @@
>  #include <linux/crc32.h>
>  #include <linux/usb/usbnet.h>
>  #include <linux/slab.h>
> +#include <linux/of_device.h>
>  #include "smsc75xx.h"
>  
>  #define SMSC_CHIPNAME			"smsc75xx"
> @@ -658,6 +659,8 @@ static int smsc75xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
>  
>  static void smsc75xx_init_mac_address(struct usbnet *dev)
>  {
> +	void *address;
> +
>  	/* try reading mac address from EEPROM */
>  	if (smsc75xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
>  			dev->net->dev_addr) == 0) {
> @@ -669,6 +672,13 @@ static void smsc75xx_init_mac_address(struct usbnet *dev)
>  		}
>  	}
>  
> +	address = of_get_property(dev->udev->dev.of_node,
> +				  "local-mac-address", NULL);
> +	if (address) {
> +		memcpy(dev->net->dev_addr, address, ETH_ALEN);
> +		return;
> +	}
> +
>  	/* no eeprom, or eeprom values are invalid. generate random MAC */
>  	random_ether_addr(dev->net->dev_addr);
>  	netif_dbg(dev, ifup, dev->net, "MAC address set to random_ether_addr");
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index bc86f4b..74585fb 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -29,6 +29,7 @@
>  #include <linux/crc32.h>
>  #include <linux/usb/usbnet.h>
>  #include <linux/slab.h>
> +#include <linux/of_device.h>
>  #include "smsc95xx.h"
>  
>  #define SMSC_CHIPNAME			"smsc95xx"
> @@ -639,6 +640,8 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
>  
>  static void smsc95xx_init_mac_address(struct usbnet *dev)
>  {
> +	void *address;
> +
>  	/* try reading mac address from EEPROM */
>  	if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
>  			dev->net->dev_addr) == 0) {
> @@ -649,6 +652,13 @@ static void smsc95xx_init_mac_address(struct usbnet *dev)
>  		}
>  	}
>  
> +	address = of_get_property(dev->udev->dev.of_node,
> +				  "local-mac-address", NULL);
> +	if (address) {
> +		memcpy(dev->net->dev_addr, address, ETH_ALEN);
> +		return;
> +	}
> +
>  	/* no eeprom, or eeprom values are invalid. generate random MAC */
>  	random_ether_addr(dev->net->dev_addr);
>  	netif_dbg(dev, ifup, dev->net, "MAC address set to random_ether_addr\n");
--
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: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/17/2011 11:27 PM, Somebody in the thread at some point said:

> The patch below also looks right to me.  I believe it also has the
> advantage of u-boot already knowing how to update the
> local-mac-address property at boot time.

In my (tested, working, complete) patch series, I allow platform_data 
based override of MAC at usbnet level, so all the drivers can benefit 
from it.

Is this not a case of "small thinking" from a Device Tree perspective 
that Arnd's patch only targets smsc95xx?  Or did I miss some 
disadvantage to allowing this functional configuration option at usbnet 
layer?

-Andy

Re: RFC: Platform data for onboard USB assets

[Arnd Bergmann]

On Friday 18 March 2011, Andy Green wrote:
> On 03/17/2011 11:27 PM, Somebody in the thread at some point said:
> 
> > The patch below also looks right to me.  I believe it also has the
> > advantage of u-boot already knowing how to update the
> > local-mac-address property at boot time.
> 
> In my (tested, working, complete) patch series, I allow platform_data 
> based override of MAC at usbnet level, so all the drivers can benefit 
> from it.
>
> Is this not a case of "small thinking" from a Device Tree perspective 
> that Arnd's patch only targets smsc95xx?  Or did I miss some 
> disadvantage to allowing this functional configuration option at usbnet 
> layer?

I think either way works (usb-net or individual drivers), the difference is 
which information you use when both a hardware MAC address and the
local-mac-address property are used. Your patch uses the local-mac-address,
mine would use the hardware mac address and only fall back to the
property if there is no other one.

I still need to look at your patch series, I didn't realize you had
already sent it.

	Arnd
--
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: RFC: Platform data for onboard USB assets

[Andy Green]

On 03/18/2011 08:25 AM, Somebody in the thread at some point said:
> On Friday 18 March 2011, Andy Green wrote:
>> On 03/17/2011 11:27 PM, Somebody in the thread at some point said:
>>
>>> The patch below also looks right to me.  I believe it also has the
>>> advantage of u-boot already knowing how to update the
>>> local-mac-address property at boot time.
>>
>> In my (tested, working, complete) patch series, I allow platform_data
>> based override of MAC at usbnet level, so all the drivers can benefit
>> from it.
>>
>> Is this not a case of "small thinking" from a Device Tree perspective
>> that Arnd's patch only targets smsc95xx?  Or did I miss some
>> disadvantage to allowing this functional configuration option at usbnet
>> layer?
>
> I think either way works (usb-net or individual drivers), the difference is
> which information you use when both a hardware MAC address and the
> local-mac-address property are used. Your patch uses the local-mac-address,
> mine would use the hardware mac address and only fall back to the
> property if there is no other one.
>
> I still need to look at your patch series, I didn't realize you had
> already sent it.

Yeah I sent it last Saturday.  Whether the MAC override from 
platform_data has precedence over EEPROM info is a matter of taste, in 
this set it overrides even EEPROM.  Note the smsc95xx patch crept into 
the Panda-specific set.

Platform series: http://marc.info/?l=linux-kernel&m=129996915023642&w=2
USB Host + Usbnet series: 
http://marc.info/?l=linux-kernel&m=129996966324111&w=2
Panda-specific: http://marc.info/?l=linux-kernel&m=129997032724779&w=2

-Andy
--
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