Hi -
platform_data is a well established way in Linux to pass configuration
data up to on-board assets from a machine file like mach-xyz.c. It's
also supported to pass platform_data up to devices that are probed
asynchronously from busses like i2c as well, which is very handy.
However AFAIK it's not possible to bind platform_data to probed USB
devices as it stands.
There are now boards which have on-board USB assets, for example OMAP4
Panda which has a USB <-> Ethernet bridge wired up permanently. It'd be
convenient to also be able to pass optional platform_data to these
devices when they are asynchronously probed.
So what's the feeling about a new api to register an array of
platform_data pointers bound to static "devpath" names in the machine file?
When a usb device is instantiated, it can check through this array if it
exists, matching on devname, and attach the platform_data to the
underlying probed usb device's dev->platform_data, which it seems is
currently unused.
The particular use that suggested this is on Panda, it would be ideal to
be able to set a flag in the usb device's platform data that forces it
to be named eth%d since it's a hardwired asset on the board with an RJ45
socket.
Comments, implementation suggestions, enquiries as to my level of crack
consumption etc welcomed ^^
-Andy
Hi Andy,
On Friday 11 March 2011, Andy Green wrote:
>
> platform_data is a well established way in Linux to pass configuration
> data up to on-board assets from a machine file like mach-xyz.c. It's
> also supported to pass platform_data up to devices that are probed
> asynchronously from busses like i2c as well, which is very handy.
>
> However AFAIK it's not possible to bind platform_data to probed USB
> devices as it stands.
>
> There are now boards which have on-board USB assets, for example OMAP4
> Panda which has a USB <-> Ethernet bridge wired up permanently. It'd be
> convenient to also be able to pass optional platform_data to these
> devices when they are asynchronously probed.
Sorry, but I think it's a bad idea to use platform data for this.
USB is a bus that we can reasonably probe, while we use platform_data
only for stuff that we can by definition not probe at all.
> The particular use that suggested this is on Panda, it would be ideal to
> be able to set a flag in the usb device's platform data that forces it
> to be named eth%d since it's a hardwired asset on the board with an RJ45
> socket.
I understand the problem, but IMHO there really needs to be a better
solution for this. As far as I understand, the underlying problem is
that USB ethernet devices with a proper MAC address get a different
device name from devices with a generated random MAC address, and
the people that designed this board were trying to save a few cents
by not allocating a MAC address for the ethernet device [1], right?
I believe we should fix this particular problem locally, instead of
coming up with generic infrastructure for broken hardware.
One relatively easy solution would be to allow passing a MAC address
to this particular device driver as a module parameter, and generating
a fixed MAC address in the regular vendor space when installing the
boot loader. That would also solve the problem that a DHCP server
currently assigns a new IP address to a panda board at every boot.
Arnd
---
[1] https://bugs.launchpad.net/linux-linaro/+bug/622429
On 03/11/2011 12:31 PM, Somebody in the thread at some point said:
Hi -
>> platform_data is a well established way in Linux to pass configuration
>> data up to on-board assets from a machine file like mach-xyz.c. It's
>> also supported to pass platform_data up to devices that are probed
>> asynchronously from busses like i2c as well, which is very handy.
>>
>> However AFAIK it's not possible to bind platform_data to probed USB
>> devices as it stands.
>>
>> There are now boards which have on-board USB assets, for example OMAP4
>> Panda which has a USB<-> Ethernet bridge wired up permanently. It'd be
>> convenient to also be able to pass optional platform_data to these
>> devices when they are asynchronously probed.
>
> Sorry, but I think it's a bad idea to use platform data for this.
> USB is a bus that we can reasonably probe, while we use platform_data
> only for stuff that we can by definition not probe at all.
...
> One relatively easy solution would be to allow passing a MAC address
> to this particular device driver as a module parameter, and generating
> a fixed MAC address in the regular vendor space when installing the
> boot loader. That would also solve the problem that a DHCP server
> currently assigns a new IP address to a panda board at every boot.
I suggested this myself on #15 of the bug you linked to. However, it
seems to me there is a generic issue here and it is not to do with
whether stuff can be discovered on the bus or not; bugging the usbnet
guys to accept devname-based commandline params and solving it for
usbnet only would be covering up the actual issue.
The I2C implementation does not limit itself to providing I2C addresses
and binding to driver names so it can be probed, it also provides for
sending platform_data into the device when it is instantiated. That is
very useful and I don't think the I2C guys will be accepting patches to
remove that capability.
There is no reason I can see that onboard USB assets should continue to
be treated differently to miss out on the same capability because they
are USB and not I2C, particularly as a permanently NULL platform_data
pointer is already sitting there in the usb_device's .dev already
exactly for this use.
-Andy
Hi,
On 03/11/2011 02:44 PM, ext Andy Green wrote:
>
> I suggested this myself on #15 of the bug you linked to. However, it
> seems to me there is a generic issue here and it is not to do with
> whether stuff can be discovered on the bus or not; bugging the usbnet
> guys to accept devname-based commandline params and solving it for
> usbnet only would be covering up the actual issue.
I fail to see the problem. USB is a dynamically probed bus.
Whether the USB peripheral is hard-wired to the board or not, it is
still dynamically probed. The board files are not expected to know
anything about USB peripherals prior to enumeration, so platform data
does not make sense for USB.
>
> The I2C implementation does not limit itself to providing I2C addresses
> and binding to driver names so it can be probed, it also provides for
> sending platform_data into the device when it is instantiated. That is
> very useful and I don't think the I2C guys will be accepting patches to
> remove that capability.
Platform data makes perfect sense for I2C because it is not a
dynamically probed bus and board writers need to define which I2C chips
are present on the board.
>
> There is no reason I can see that onboard USB assets should continue to
> be treated differently to miss out on the same capability because they
> are USB and not I2C, particularly as a permanently NULL platform_data
> pointer is already sitting there in the usb_device's .dev already
> exactly for this use.
What do you want to set in platform data? the ethernet device name?
Isn't that better done in user space using udev rules?
--
regards,
-roger
On 03/11/2011 04:42 PM, Roger Quadros wrote:
> I fail to see the problem. USB is a dynamically probed bus.
> Whether the USB peripheral is hard-wired to the board or not, it is
> still dynamically probed. The board files are not expected to know
> anything about USB peripherals prior to enumeration, so platform data
> does not make sense for USB.
sorry, my e-mail editor's line wrapping setting got messed up.
regards,
-roger
On Friday 11 March 2011, Roger Quadros wrote:
> > There is no reason I can see that onboard USB assets should continue to
> > be treated differently to miss out on the same capability because they
> > are USB and not I2C, particularly as a permanently NULL platform_data
> > pointer is already sitting there in the usb_device's .dev already
> > exactly for this use.
>
> What do you want to set in platform data? the ethernet device name?
> Isn't that better done in user space using udev rules?
A udev rule would solve the problem at hand, but I'd consider that
an ugly workaround as well. The naming in the kernel is really
bogus -- any USB device that has a fixed address gets treated
as eth0, while others become usb0, with the same driver,
see the code fragment below.
A lot of things depend on ethernet device naming, which you
may consider to be broken, but it won't change any time soon.
Changing all of them to be named eth%d would of course break
other tools, so that is not an option either.
The most simple way to solve this particular problem is to
remove the check for "net->dev_addr [0] & 0x02", which only
has any effect on the smsc75xx and smsc95xx drivers, both of
which are for real ethernet devices, not point-to-point
USB links.
That would however still leave the problem of the missing
mac address, which is not good if you want to work with the
system using dhcp.
Arnd
---
strcpy (net->name, "usb%d");
// heuristic: "usb%d" for links we know are two-host,
// else "eth%d" when there's reasonable doubt. userspace
// can rename the link if it knows better.
if ((dev->driver_info->flags & FLAG_ETHER) != 0 &&
(net->dev_addr [0] & 0x02) == 0)
strcpy (net->name, "eth%d");
/* WLAN devices should always be named "wlan%d" */
if ((dev->driver_info->flags & FLAG_WLAN) != 0)
strcpy(net->name, "wlan%d");
/* WWAN devices should always be named "wwan%d" */
if ((dev->driver_info->flags & FLAG_WWAN) != 0)
strcpy(net->name, "wwan%d");
On Fri, Mar 11, 2011 at 01:31:13PM +0100, Arnd Bergmann wrote:
> I understand the problem, but IMHO there really needs to be a better
> solution for this. As far as I understand, the underlying problem is
> that USB ethernet devices with a proper MAC address get a different
> device name from devices with a generated random MAC address, and
> the people that designed this board were trying to save a few cents
> by not allocating a MAC address for the ethernet device [1], right?
> I believe we should fix this particular problem locally, instead of
> coming up with generic infrastructure for broken hardware.
It's arguable if this stuff is broken at all, from a hardware design
point of view it's perfectly reasonable and if you're shipping volumes
in the millions very small savings add up to interesting numbers easily.
> One relatively easy solution would be to allow passing a MAC address
> to this particular device driver as a module parameter, and generating
> a fixed MAC address in the regular vendor space when installing the
> boot loader. That would also solve the problem that a DHCP server
> currently assigns a new IP address to a panda board at every boot.
There's other uses for this stuff, it seems to be quite common for
various radio related things to end up hanging off USB too.
Hi,
On 03/11/2011 05:22 PM, ext Arnd Bergmann wrote:
> On Friday 11 March 2011, Roger Quadros wrote:
>>> There is no reason I can see that onboard USB assets should continue to
>>> be treated differently to miss out on the same capability because they
>>> are USB and not I2C, particularly as a permanently NULL platform_data
>>> pointer is already sitting there in the usb_device's .dev already
>>> exactly for this use.
>>
>> What do you want to set in platform data? the ethernet device name?
>> Isn't that better done in user space using udev rules?
>
> A udev rule would solve the problem at hand, but I'd consider that
> an ugly workaround as well. The naming in the kernel is really
> bogus -- any USB device that has a fixed address gets treated
> as eth0, while others become usb0, with the same driver,
> see the code fragment below.
but isn't that sensible? a real ethernet hardware will have a MAC
address. no?
> The most simple way to solve this particular problem is to
> remove the check for "net->dev_addr [0] & 0x02", which only
> has any effect on the smsc75xx and smsc95xx drivers, both of
> which are for real ethernet devices, not point-to-point
> USB links.
>
> That would however still leave the problem of the missing
> mac address, which is not good if you want to work with the
> system using dhcp.
The USB ethernet peripheral does not have a globally unique MAC ??
--
regards,
-roger
On Friday 11 March 2011, Mark Brown wrote:
> On Fri, Mar 11, 2011 at 01:31:13PM +0100, Arnd Bergmann wrote:
>
> > I understand the problem, but IMHO there really needs to be a better
> > solution for this. As far as I understand, the underlying problem is
> > that USB ethernet devices with a proper MAC address get a different
> > device name from devices with a generated random MAC address, and
> > the people that designed this board were trying to save a few cents
> > by not allocating a MAC address for the ethernet device [1], right?
>
> > I believe we should fix this particular problem locally, instead of
> > coming up with generic infrastructure for broken hardware.
>
> It's arguable if this stuff is broken at all, from a hardware design
> point of view it's perfectly reasonable and if you're shipping volumes
> in the millions very small savings add up to interesting numbers easily.
It may be reasonable if you don't expect anyone to connect the
device to an ethernet port, but in that case you could save much
more by removing the ethernet chip and the socket along with the
eeprom.
Really, any machine without a fixed MAC address is a huge pain
for users, just google for "pandaboard mac address" to see
how much work this has caused people.
Arnd
On Friday 11 March 2011, Roger Quadros wrote:
> > A udev rule would solve the problem at hand, but I'd consider that
> > an ugly workaround as well. The naming in the kernel is really
> > bogus -- any USB device that has a fixed address gets treated
> > as eth0, while others become usb0, with the same driver,
> > see the code fragment below.
>
> but isn't that sensible? a real ethernet hardware will have a MAC
> address. no?
I have just verified with my Pandaboard that the pins on the SMSC9514
usb ethernet that are meant to be connected to a serial EEPROM are
indeed not connected anywhere.
Arnd
On Fri, Mar 11, 2011 at 04:54:03PM +0100, Arnd Bergmann wrote:
> On Friday 11 March 2011, Mark Brown wrote:
> > It's arguable if this stuff is broken at all, from a hardware design
> > point of view it's perfectly reasonable and if you're shipping volumes
> > in the millions very small savings add up to interesting numbers easily.
> It may be reasonable if you don't expect anyone to connect the
> device to an ethernet port, but in that case you could save much
> more by removing the ethernet chip and the socket along with the
> eeprom.
> Really, any machine without a fixed MAC address is a huge pain
> for users, just google for "pandaboard mac address" to see
> how much work this has caused people.
I'm not familiar with the Pandaboard but most of the devices I've worked
with that do this have unique MAC addresses but they store in other
locations on the device (typically in flash).
Like I say, it's not just MAC addresses that can need configuring this
way - it can be other random "you're wired up this way" type
information that would normally be figured out from the USB IDs.
On Fri, Mar 11, 2011 at 09:50:32AM +0000, Andy Green wrote:
> The particular use that suggested this is on Panda, it would be
> ideal to be able to set a flag in the usb device's platform data
> that forces it to be named eth%d since it's a hardwired asset on the
> board with an RJ45 socket.
If you _really_ need to name your network devices in a specific order,
then use the userspace tools we already have to do this. That is what
they were created for, why ignore them?
thanks,
greg k-h
On Fri, Mar 11, 2011 at 04:03:09PM +0000, Mark Brown wrote:
> On Fri, Mar 11, 2011 at 04:54:03PM +0100, Arnd Bergmann wrote:
> > On Friday 11 March 2011, Mark Brown wrote:
>
> > > It's arguable if this stuff is broken at all, from a hardware design
> > > point of view it's perfectly reasonable and if you're shipping volumes
> > > in the millions very small savings add up to interesting numbers easily.
>
> > It may be reasonable if you don't expect anyone to connect the
> > device to an ethernet port, but in that case you could save much
> > more by removing the ethernet chip and the socket along with the
> > eeprom.
>
> > Really, any machine without a fixed MAC address is a huge pain
> > for users, just google for "pandaboard mac address" to see
> > how much work this has caused people.
>
> I'm not familiar with the Pandaboard but most of the devices I've worked
> with that do this have unique MAC addresses but they store in other
> locations on the device (typically in flash).
>
> Like I say, it's not just MAC addresses that can need configuring this
> way - it can be other random "you're wired up this way" type
> information that would normally be figured out from the USB IDs.
And all of that should be done in userspace, like all other device
names, I still fail to see what is so different here from any other type
of system.
thanks,
greg k-h
On 03/11/2011 04:08 PM, Somebody in the thread at some point said:
> On Fri, Mar 11, 2011 at 09:50:32AM +0000, Andy Green wrote:
>> The particular use that suggested this is on Panda, it would be
>> ideal to be able to set a flag in the usb device's platform data
>> that forces it to be named eth%d since it's a hardwired asset on the
>> board with an RJ45 socket.
>
> If you _really_ need to name your network devices in a specific order,
> then use the userspace tools we already have to do this. That is what
> they were created for, why ignore them?
I think maybe discussion of this use-case, its usbnet specificity, and
the alternative options to achieve that have derailed discussion about
what I was actually asking.
Is it true that for on-board devices, it can sometimes be legitimate and
useful to be able to deliver platform_data from the board file through
to stuff on a USB bus, same as you would for memory mapped, I2C, other
busses?
Or is that it since it is USB, it can never be useful or legitimate, no
matter what different kind of wired-up on-board USB device it is, to
have the board definition file configure the driver for that instantiation?
-Andy
On 03/11/2011 04:03 PM, Somebody in the thread at some point said:
> On Fri, Mar 11, 2011 at 04:54:03PM +0100, Arnd Bergmann wrote:
>> On Friday 11 March 2011, Mark Brown wrote:
>
>>> It's arguable if this stuff is broken at all, from a hardware design
>>> point of view it's perfectly reasonable and if you're shipping volumes
>>> in the millions very small savings add up to interesting numbers easily.
>
>> It may be reasonable if you don't expect anyone to connect the
>> device to an ethernet port, but in that case you could save much
>> more by removing the ethernet chip and the socket along with the
>> eeprom.
>
>> Really, any machine without a fixed MAC address is a huge pain
>> for users, just google for "pandaboard mac address" to see
>> how much work this has caused people.
>
> I'm not familiar with the Pandaboard but most of the devices I've worked
> with that do this have unique MAC addresses but they store in other
> locations on the device (typically in flash).
>
> Like I say, it's not just MAC addresses that can need configuring this
> way - it can be other random "you're wired up this way" type
> information that would normally be figured out from the USB IDs.
Yes that's exactly why I was thinking it's a class of requirement that
could reasonably be a little API and extending platform_data to it. So
anyone with onboard USB device can take advantage if they need to,
because I guess we see gradually more boards like that.
The driver knows well all about the actual device, but there is a class
of configuration information that is defined by the physical board
itself - as you say "how it is wired" - and needs to be passed into the
driver to inform it of its "functional configuration". When that
functional configuration information is a feature of the board alone,
actually the board definition file is the right place for it.
-Andy
On Fri, Mar 11, 2011 at 08:14:21AM -0800, Greg KH wrote:
> On Fri, Mar 11, 2011 at 04:03:09PM +0000, Mark Brown wrote:
> > Like I say, it's not just MAC addresses that can need configuring this
> > way - it can be other random "you're wired up this way" type
> > information that would normally be figured out from the USB IDs.
> And all of that should be done in userspace, like all other device
> names, I still fail to see what is so different here from any other type
> of system.
I'm actually not that fussed about the MAC address use case itself and
do tend to agree with you that that's handlable in userspace but for the
other things that might need to be configured there's a lot of things
that for non-discoverable buses we're currently passing through platform
data so switching to bouncing things through userspace would itself be a
substantial change from other systems.
You can't in general rely on the system being neatly abstracted and
while discoverable buses do avoid many problems in this sort of area
there are still things which can only be discovered through reference to
the schematics or similar, especially if you care about the microamps.
On Fri, Mar 11, 2011 at 04:27:59PM +0000, Mark Brown wrote:
> On Fri, Mar 11, 2011 at 08:14:21AM -0800, Greg KH wrote:
> > On Fri, Mar 11, 2011 at 04:03:09PM +0000, Mark Brown wrote:
>
> > > Like I say, it's not just MAC addresses that can need configuring this
> > > way - it can be other random "you're wired up this way" type
> > > information that would normally be figured out from the USB IDs.
>
> > And all of that should be done in userspace, like all other device
> > names, I still fail to see what is so different here from any other type
> > of system.
>
> I'm actually not that fussed about the MAC address use case itself and
> do tend to agree with you that that's handlable in userspace but for the
> other things that might need to be configured there's a lot of things
> that for non-discoverable buses we're currently passing through platform
> data so switching to bouncing things through userspace would itself be a
> substantial change from other systems.
But USB _is_ discoverable, that's my point.
> You can't in general rely on the system being neatly abstracted and
> while discoverable buses do avoid many problems in this sort of area
> there are still things which can only be discovered through reference to
> the schematics or similar, especially if you care about the microamps.
Again, that's not USB, so it isn't relevant here.
thanks,
greg k-h
On Fri, Mar 11, 2011 at 04:20:46PM +0000, Andy Green wrote:
> On 03/11/2011 04:08 PM, Somebody in the thread at some point said:
> >On Fri, Mar 11, 2011 at 09:50:32AM +0000, Andy Green wrote:
> >>The particular use that suggested this is on Panda, it would be
> >>ideal to be able to set a flag in the usb device's platform data
> >>that forces it to be named eth%d since it's a hardwired asset on the
> >>board with an RJ45 socket.
> >
> >If you _really_ need to name your network devices in a specific order,
> >then use the userspace tools we already have to do this. That is what
> >they were created for, why ignore them?
>
> I think maybe discussion of this use-case, its usbnet specificity,
> and the alternative options to achieve that have derailed discussion
> about what I was actually asking.
>
> Is it true that for on-board devices, it can sometimes be legitimate
> and useful to be able to deliver platform_data from the board file
> through to stuff on a USB bus, same as you would for memory mapped,
> I2C, other busses?
>
> Or is that it since it is USB, it can never be useful or legitimate,
> no matter what different kind of wired-up on-board USB device it is,
> to have the board definition file configure the driver for that
> instantiation?
Since it is USB, it is always discoverable, so it doesn't make any sense
to have this type of thing.
And since your only example was a network device, I think you proved
your point :)
thanks,
greg k-h
On 03/11/2011 04:36 PM, Somebody in the thread at some point said:
>> Is it true that for on-board devices, it can sometimes be legitimate
>> and useful to be able to deliver platform_data from the board file
>> through to stuff on a USB bus, same as you would for memory mapped,
>> I2C, other busses?
>>
>> Or is that it since it is USB, it can never be useful or legitimate,
>> no matter what different kind of wired-up on-board USB device it is,
>> to have the board definition file configure the driver for that
>> instantiation?
>
> Since it is USB, it is always discoverable, so it doesn't make any sense
> to have this type of thing.
>
> And since your only example was a network device, I think you proved
> your point :)
Well, thanks for the clear answer ^^
Have a nice weekend.
-Andy
On Fri, 11 Mar 2011, Andy Green wrote:
> > Like I say, it's not just MAC addresses that can need configuring this
> > way - it can be other random "you're wired up this way" type
> > information that would normally be figured out from the USB IDs.
>
> Yes that's exactly why I was thinking it's a class of requirement that
> could reasonably be a little API and extending platform_data to it. So
> anyone with onboard USB device can take advantage if they need to,
> because I guess we see gradually more boards like that.
>
> The driver knows well all about the actual device, but there is a class
> of configuration information that is defined by the physical board
> itself - as you say "how it is wired" - and needs to be passed into the
> driver to inform it of its "functional configuration". When that
> functional configuration information is a feature of the board alone,
> actually the board definition file is the right place for it.
I don't see the point of doing this for USB devices, or hot-pluggable
devices in general.
Suppose you bought another USB device that was just like the on-board
one, and plugged it into the system. Why should the driver need to
treat the on-board device any different from the hot-plugged device?
Or to put it another way... With external, hot-plugged USB devices,
there is no need to know "how it is wired". The fact that it is on a
USB bus is the only information necessary. Why does anyone need to
know more than this for on-board USB devices?
Alan Stern
On Fri, Mar 11, 2011 at 08:35:22AM -0800, Greg KH wrote:
> On Fri, Mar 11, 2011 at 04:27:59PM +0000, Mark Brown wrote:
> > I'm actually not that fussed about the MAC address use case itself and
> > do tend to agree with you that that's handlable in userspace but for the
> > other things that might need to be configured there's a lot of things
> > that for non-discoverable buses we're currently passing through platform
> > data so switching to bouncing things through userspace would itself be a
> > substantial change from other systems.
> But USB _is_ discoverable, that's my point.
USB itself is discoverable but the when the USB bus you're looking at is
one that's soldered down onto a board in a specific design all bets are
off regarding how complete the information you get will be. On a basic
level the designers may have done things like omit the configuration
EEPROMs that would set the device IDs that the driver should be relying
on to identify the hardware configuration. There may be other, nastier,
things going on.
> > You can't in general rely on the system being neatly abstracted and
> > while discoverable buses do avoid many problems in this sort of area
> > there are still things which can only be discovered through reference to
> > the schematics or similar, especially if you care about the microamps.
> Again, that's not USB, so it isn't relevant here.
You really can't make this assumption about discoverable buses on
embedded devices. The discoverability will get you most of the way
there but not always all of the way there.
On 03/11/2011 04:45 PM, Somebody in the thread at some point said:
Hi -
> Or to put it another way... With external, hot-plugged USB devices,
> there is no need to know "how it is wired". The fact that it is on a
> USB bus is the only information necessary. Why does anyone need to
> know more than this for on-board USB devices?
For example, the USB device is a chip with option pins. On the board it
is placed on, some of the option pins are tied in a particular way that
impacts its actual function, but can't be seen from the chip itself.
The driver covers all the options, but it needs to be told which mode
the chip was wired up for.
Another example, it's a USB chip with GPIO pins, analogous to a I2C GPIO
extender. Some of the GPIO are wired to LEDs also on the board, which
you want to expose as generic GPIO. The board definition file is in a
position to do all that because it knows what the board is and what it
is wired up to.
That the USB chips in these examples are 'discoverable' has nothing to
do with anything. In fact the board definition file has knowledge about
the "functional implemntation" of the instances of those chips -- just
exactly those instances soldered to the board. If you plugged another
of these chips, the board definition file has nothing to say about it
because they are not "on the board" and in-scope for it.
-Andy
On Fri, Mar 11, 2011 at 11:45:20AM -0500, Alan Stern wrote:
> Or to put it another way... With external, hot-plugged USB devices,
> there is no need to know "how it is wired". The fact that it is on a
> USB bus is the only information necessary. Why does anyone need to
> know more than this for on-board USB devices?
Examples I'm aware of include things like power control beyond what the
USB spec does and not programming device identification or configuration
information that would normally be initialised by the hardware, relying
on the AP to program it at runtime.
On Fri, Mar 11, 2011 at 04:48:50PM +0000, Mark Brown wrote:
> On Fri, Mar 11, 2011 at 08:35:22AM -0800, Greg KH wrote:
> > On Fri, Mar 11, 2011 at 04:27:59PM +0000, Mark Brown wrote:
>
> > > I'm actually not that fussed about the MAC address use case itself and
> > > do tend to agree with you that that's handlable in userspace but for the
> > > other things that might need to be configured there's a lot of things
> > > that for non-discoverable buses we're currently passing through platform
> > > data so switching to bouncing things through userspace would itself be a
> > > substantial change from other systems.
>
> > But USB _is_ discoverable, that's my point.
>
> USB itself is discoverable but the when the USB bus you're looking at is
> one that's soldered down onto a board in a specific design all bets are
> off regarding how complete the information you get will be. On a basic
> level the designers may have done things like omit the configuration
> EEPROMs that would set the device IDs that the driver should be relying
> on to identify the hardware configuration. There may be other, nastier,
> things going on.
Then you use the existing platform data for your USB host controller
driver. Doesn't that work today just fine?
> > > You can't in general rely on the system being neatly abstracted and
> > > while discoverable buses do avoid many problems in this sort of area
> > > there are still things which can only be discovered through reference to
> > > the schematics or similar, especially if you care about the microamps.
>
> > Again, that's not USB, so it isn't relevant here.
>
> You really can't make this assumption about discoverable buses on
> embedded devices. The discoverability will get you most of the way
> there but not always all of the way there.
Then the bus is not really USB, sorry. USB is discoverable, _and_ can
support enumeration in non-deterministic ways. If people are using it
in other ways then it is not USB and is something else.
thanks,
greg k-h
On Fri, Mar 11, 2011 at 08:56:42AM -0800, Greg KH wrote:
> On Fri, Mar 11, 2011 at 04:48:50PM +0000, Mark Brown wrote:
> > USB itself is discoverable but the when the USB bus you're looking at is
> > one that's soldered down onto a board in a specific design all bets are
> > off regarding how complete the information you get will be. On a basic
> > level the designers may have done things like omit the configuration
> > EEPROMs that would set the device IDs that the driver should be relying
> > on to identify the hardware configuration. There may be other, nastier,
> > things going on.
> Then you use the existing platform data for your USB host controller
> driver. Doesn't that work today just fine?
Wrong end of the bus. This stuff is simple enough to deal with in a
system specific fashion, the standard solution would be to patch the
relevant drivers to hard code whatever is required.
> > You really can't make this assumption about discoverable buses on
> > embedded devices. The discoverability will get you most of the way
> > there but not always all of the way there.
> Then the bus is not really USB, sorry. USB is discoverable, _and_ can
> support enumeration in non-deterministic ways. If people are using it
> in other ways then it is not USB and is something else.
That's certainly a valid way of looking at things but it doesn't really
move mainline support for systems which do stuff like this forward.
On the bright side I don't generally have to work on this stuff myself :)
On Fri, Mar 11, 2011 at 04:51:53PM +0000, Andy Green wrote:
> On 03/11/2011 04:45 PM, Somebody in the thread at some point said:
>
> Hi -
>
> >Or to put it another way... With external, hot-plugged USB devices,
> >there is no need to know "how it is wired". The fact that it is on a
> >USB bus is the only information necessary. Why does anyone need to
> >know more than this for on-board USB devices?
>
> For example, the USB device is a chip with option pins. On the
> board it is placed on, some of the option pins are tied in a
> particular way that impacts its actual function, but can't be seen
> from the chip itself. The driver covers all the options, but it
> needs to be told which mode the chip was wired up for.
Then that information is in the driver that was written for that
specific device, it is NOT a class device if it requires this type of
information to work properly.
> Another example, it's a USB chip with GPIO pins, analogous to a I2C
> GPIO extender. Some of the GPIO are wired to LEDs also on the
> board, which you want to expose as generic GPIO. The board
> definition file is in a position to do all that because it knows
> what the board is and what it is wired up to.
>
> That the USB chips in these examples are 'discoverable' has nothing
> to do with anything. In fact the board definition file has
> knowledge about the "functional implemntation" of the instances of
> those chips -- just exactly those instances soldered to the board.
> If you plugged another of these chips, the board definition file has
> nothing to say about it because they are not "on the board" and
> in-scope for it.
Then put that information in the specific driver for this type of
device.
Also, do you have a real example of a USB driver today that needs this?
thanks,
greg k-h
On Fri, Mar 11, 2011 at 05:08:07PM +0000, Mark Brown wrote:
> On Fri, Mar 11, 2011 at 08:56:42AM -0800, Greg KH wrote:
> > On Fri, Mar 11, 2011 at 04:48:50PM +0000, Mark Brown wrote:
>
> > > USB itself is discoverable but the when the USB bus you're looking at is
> > > one that's soldered down onto a board in a specific design all bets are
> > > off regarding how complete the information you get will be. On a basic
> > > level the designers may have done things like omit the configuration
> > > EEPROMs that would set the device IDs that the driver should be relying
> > > on to identify the hardware configuration. There may be other, nastier,
> > > things going on.
>
> > Then you use the existing platform data for your USB host controller
> > driver. Doesn't that work today just fine?
>
> Wrong end of the bus. This stuff is simple enough to deal with in a
> system specific fashion, the standard solution would be to patch the
> relevant drivers to hard code whatever is required.
What drivers need this? Specifics please.
> > > You really can't make this assumption about discoverable buses on
> > > embedded devices. The discoverability will get you most of the way
> > > there but not always all of the way there.
>
> > Then the bus is not really USB, sorry. USB is discoverable, _and_ can
> > support enumeration in non-deterministic ways. If people are using it
> > in other ways then it is not USB and is something else.
>
> That's certainly a valid way of looking at things but it doesn't really
> move mainline support for systems which do stuff like this forward.
I would brand these types of systems "extremely broken" :)
Anyway, specifics are the best way forward if anyone has such a messed
up system.
thanks,
greg k-h
On Fri, Mar 11, 2011 at 09:23:45AM -0800, Greg KH wrote:
> On Fri, Mar 11, 2011 at 05:08:07PM +0000, Mark Brown wrote:
> > Wrong end of the bus. This stuff is simple enough to deal with in a
> > system specific fashion, the standard solution would be to patch the
> > relevant drivers to hard code whatever is required.
> What drivers need this? Specifics please.
I can't discuss specific systems I've seen this on unfortunately. I
guess the most obvious thing in mainline would be applying quirks like
various drivers have based on data other than device ID information -
it's pretty much the same problem.
On 03/11/2011 05:08 PM, Somebody in the thread at some point said:
> On Fri, Mar 11, 2011 at 04:51:53PM +0000, Andy Green wrote:
>> On 03/11/2011 04:45 PM, Somebody in the thread at some point said:
>>
>> Hi -
>>
>>> Or to put it another way... With external, hot-plugged USB devices,
>>> there is no need to know "how it is wired". The fact that it is on a
>>> USB bus is the only information necessary. Why does anyone need to
>>> know more than this for on-board USB devices?
>>
>> For example, the USB device is a chip with option pins. On the
>> board it is placed on, some of the option pins are tied in a
>> particular way that impacts its actual function, but can't be seen
>> from the chip itself. The driver covers all the options, but it
>> needs to be told which mode the chip was wired up for.
>
> Then that information is in the driver that was written for that
> specific device, it is NOT a class device if it requires this type of
> information to work properly.
I don't believe I referred to class devices anywhere. It does not
matter if the main chip function is class device or not.
If there is any kind of "functional implementation" knowledge that is
outside the chip and driver itself, it has to be held somewhere, and
applied appropriately. platform_data from the board definition file is
the established place for that knowledge that is specific to a board.
An example of the same idea in a different context is OMAP4 I2C IP.
There is a single I2C driver for all OMAP, and it adapts according to
which revision of the IP it finds. In the perfect case, the driver
could thoroughly figure out what to do based on what it saw at runtime.
However different CPUs wired up their busses to these IP unit at
different widths. It meant according to that wiring topology
information, you had to shift the register address by different amounts
to access it.
Even though the driver had perfect domain knowledge of its IP core,
there was some additional "functional configuration" knowledge that it
HAD to be passed to succeed to operate correctly. It could not infer
this information from inside because this information was not in the
configuration of the IP core itself. The information described how the
IP core "was wired".
They solve this by defining this cpu-specific information in
cpu-specific files in the mach-omap2 dir and passing it up by platform_data.
> Also, do you have a real example of a USB driver today that needs this?
I think you find without devpath -> platform_data mapping, the kind of
layout given above is made quite difficult to support in Linux.
Saying, "oh go away and do it in userspace" actually means special
scripts that grep /proc/cpuinfo to reproduce the knowledge inherent
already in the board definition file, although it's out of your hair
nicely, it leaves me feeling we reached an overall solution that is
"less good than it should be".
-Andy
On Fri, 11 Mar 2011, Andy Green wrote:
> I don't believe I referred to class devices anywhere. It does not
> matter if the main chip function is class device or not.
It matters because the class specification for a USB device is never
going to mention information sources that are outside the USB protocol,
such as board definitions. Consequently a class driver will never need
to use such a thing.
> If there is any kind of "functional implementation" knowledge that is
> outside the chip and driver itself, it has to be held somewhere, and
> applied appropriately. platform_data from the board definition file is
> the established place for that knowledge that is specific to a board.
Since essentially all of the USB drivers currently in the kernel _are_
class drivers (at least, I'm not aware of any non-trivial exceptions),
this means none of the existing USB drivers should need to access any
platform data.
Of course, this doesn't rule out the possibility of platform-specific
USB drivers that _do_ need this information.
> > Also, do you have a real example of a USB driver today that needs this?
>
> I think you find without devpath -> platform_data mapping, the kind of
> layout given above is made quite difficult to support in Linux.
What would be needed to support such a mapping? It seems to me that we
probably have all the necessary ingredients in place already.
Alan Stern
On Fri, Mar 11, 2011 at 06:09:19PM +0000, Andy Green wrote:
> >Also, do you have a real example of a USB driver today that needs this?
>
> I think you find without devpath -> platform_data mapping, the kind
> of layout given above is made quite difficult to support in Linux.
So you have devices today that you can not support in Linux because of
this problem? Please provide us specifics about them and we will gladly
work to solve this. But without such information, hand-waving is going
to get you nowhere.
> Saying, "oh go away and do it in userspace" actually means special
> scripts that grep /proc/cpuinfo to reproduce the knowledge inherent
> already in the board definition file, although it's out of your hair
> nicely, it leaves me feeling we reached an overall solution that is
> "less good than it should be".
I'm saying "do network device naming in userspace" as that is where it
belongs.
If you have other type of configuration you need to do, again, provide
us specifics please and we will be glad to work on the proper solution.
thanks,
greg k-h
On 03/11/2011 07:12 PM, Somebody in the thread at some point said:
Hi -
>> I don't believe I referred to class devices anywhere. It does not
>> matter if the main chip function is class device or not.
>
> It matters because the class specification for a USB device is never
> going to mention information sources that are outside the USB protocol,
> such as board definitions. Consequently a class driver will never need
> to use such a thing.
Is a gadget driver a class driver? Because I can set the MAC address
for my g_ether from the kernel commandline which is most definitely an
"information source outside the USB protocol". That is exactly the kind
of thing I am talking about enabling also to be taken from
usb_device->dev.platform_data.
>>> Also, do you have a real example of a USB driver today that needs this?
>>
>> I think you find without devpath -> platform_data mapping, the kind of
>> layout given above is made quite difficult to support in Linux.
>
> What would be needed to support such a mapping? It seems to me that we
> probably have all the necessary ingredients in place already.
What I was imagining is getting the platform data structs from a header
in include/linux/
#include <linux/onboard_device_1.h>
#include <linux/onboard_device_2.h>
Using a .name defined to be the first member to match to specific bus
member devpath prepended with bus class:
struct onboard_device_1_platform_data device_1_platform_data {
.name = "usb\1-0",
.some_feature = 1,
};
struct onboard_device_2_platform_data device_2_platform_data {
.name = "usb\1-1",
.some_feature = 2,
};
Aggregating them into an array
void *devpath_to_platform_data_mapping[] = {
device_1_platform_data,
device_2_platform_data,
};
Registering the array in the machine file
register_platform_data_by_devpath(devpath_to_platform_data_mapping,
ARRAY_SIZE(devpath_to_platform_data_mapping));
It needs to set two globals for pointer and member count
void
register_platform_data_by_devpath(void *map, int count)
{
platform_data_mapping = map;
platform_data_mapping_count = count;
}
Later, when a USB device is instantiated, if platform_data_mapping is
not NULL, it checks to see if the devname it just synthesized matches
any in the table that have static usb/ prefix, and if so, sets
usb_device->dev.platform_data to the table entry.
Well, it's an RFC so if you have a better plan I am all ears.
-Andy
On Fri, Mar 11, 2011 at 08:05:56PM +0000, Andy Green wrote:
> On 03/11/2011 07:12 PM, Somebody in the thread at some point said:
>
> Hi -
>
> >>I don't believe I referred to class devices anywhere. It does not
> >>matter if the main chip function is class device or not.
> >
> >It matters because the class specification for a USB device is never
> >going to mention information sources that are outside the USB protocol,
> >such as board definitions. Consequently a class driver will never need
> >to use such a thing.
>
> Is a gadget driver a class driver?
Some are, but you are now on the "other side" of the USB bus.
> Because I can set the MAC address for my g_ether from the kernel
> commandline which is most definitely an "information source outside
> the USB protocol". That is exactly the kind of thing I am talking
> about enabling also to be taken from usb_device->dev.platform_data.
You don't have a usb_device for a gadget device. USB Gadgets are a
totally different world here, don't confuse them please.
> >>>Also, do you have a real example of a USB driver today that needs this?
> >>
> >>I think you find without devpath -> platform_data mapping, the kind of
> >>layout given above is made quite difficult to support in Linux.
> >
> >What would be needed to support such a mapping? It seems to me that we
> >probably have all the necessary ingredients in place already.
>
> What I was imagining is getting the platform data structs from a
> header in include/linux/
>
> #include <linux/onboard_device_1.h>
> #include <linux/onboard_device_2.h>
>
> Using a .name defined to be the first member to match to specific
> bus member devpath prepended with bus class:
You can't rely on USB bus numbers to be the same EVER. These are
assigned by the host computer.
> struct onboard_device_1_platform_data device_1_platform_data {
> .name = "usb\1-0",
> .some_feature = 1,
> };
>
> struct onboard_device_2_platform_data device_2_platform_data {
> .name = "usb\1-1",
> .some_feature = 2,
> };
>
> Aggregating them into an array
>
> void *devpath_to_platform_data_mapping[] = {
> device_1_platform_data,
> device_2_platform_data,
> };
>
> Registering the array in the machine file
>
> register_platform_data_by_devpath(devpath_to_platform_data_mapping,
> ARRAY_SIZE(devpath_to_platform_data_mapping));
>
> It needs to set two globals for pointer and member count
>
> void
> register_platform_data_by_devpath(void *map, int count)
> {
> platform_data_mapping = map;
> platform_data_mapping_count = count;
> }
>
> Later, when a USB device is instantiated, if platform_data_mapping
> is not NULL, it checks to see if the devname it just synthesized
> matches any in the table that have static usb/ prefix, and if so,
> sets usb_device->dev.platform_data to the table entry.
>
> Well, it's an RFC so if you have a better plan I am all ears.
I am totally confused, and as you are messing with bus id numbers, and
confusing usb gadgets and drivers, I think you are as well.
So, are you talking about USB gadgets, or USB drivers here? They are
different and have different requirements and responsibilities.
confused,
greg k-h
On 03/11/2011 08:21 PM, Somebody in the thread at some point said:
>> Is a gadget driver a class driver?
>
> Some are, but you are now on the "other side" of the USB bus.
The goal of this is to allow board-specific data to define platform_data
in any bus members.
>> Because I can set the MAC address for my g_ether from the kernel
>> commandline which is most definitely an "information source outside
>> the USB protocol". That is exactly the kind of thing I am talking
>> about enabling also to be taken from usb_device->dev.platform_data.
>
> You don't have a usb_device for a gadget device. USB Gadgets are a
> totally different world here, don't confuse them please.
Okay; but all that matters for this is that usb_gadget has a struct
device composed in it, which in turn has a platform_data. Gadget
drivers can also benefit from the mapping scheme the same way.
I wrote up this RFC after a quick check struct device exists in the usb
objects and grepping that nothing uses platform_data already. If it
isn't killed dead during this I will make workable patches for comment.
>> Using a .name defined to be the first member to match to specific
>> bus member devpath prepended with bus class:
>
> You can't rely on USB bus numbers to be the same EVER. These are
> assigned by the host computer.
Alright: is there a better invariant nomenclature to talk about "the
gadget / host that is running on a particular pair of pins"? That's the
ultimate definition of what's wired to what.
It's okay if the nomenclature differs for gadget or host because we can
prepend a namepace token that is checked for before looking for a match
in each case.
>> Well, it's an RFC so if you have a better plan I am all ears.
>
> I am totally confused, and as you are messing with bus id numbers, and
> confusing usb gadgets and drivers, I think you are as well.
>
> So, are you talking about USB gadgets, or USB drivers here? They are
> different and have different requirements and responsibilities.
Both can make use of platform_data. At the moment both gadget and host
USB devices have NULL platform data members sitting there. What this
proposal is about is being able to teleport structs into those pointers
from the board file, when gadget or host are instantiated that match.
After that, drivers that are interested to take direction about anything
can tell what they're willing to look at in their platform_data struct
definition.
-Andy
On Fri, Mar 11, 2011 at 09:07:52PM +0000, Andy Green wrote:
> On 03/11/2011 08:21 PM, Somebody in the thread at some point said:
>
> >>Is a gadget driver a class driver?
> >
> >Some are, but you are now on the "other side" of the USB bus.
>
> The goal of this is to allow board-specific data to define
> platform_data in any bus members.
Ok, but I still don't think that's a worthy goal, as for some buses
(i.e. USB), it makes no sense.
> >>Because I can set the MAC address for my g_ether from the kernel
> >>commandline which is most definitely an "information source outside
> >>the USB protocol". That is exactly the kind of thing I am talking
> >>about enabling also to be taken from usb_device->dev.platform_data.
> >
> >You don't have a usb_device for a gadget device. USB Gadgets are a
> >totally different world here, don't confuse them please.
>
> Okay; but all that matters for this is that usb_gadget has a struct
> device composed in it, which in turn has a platform_data. Gadget
> drivers can also benefit from the mapping scheme the same way.
Perhaps.
> I wrote up this RFC after a quick check struct device exists in the
> usb objects and grepping that nothing uses platform_data already.
> If it isn't killed dead during this I will make workable patches for
> comment.
Let's see workable patches before I shoot it down any further :)
> >>Using a .name defined to be the first member to match to specific
> >>bus member devpath prepended with bus class:
> >
> >You can't rely on USB bus numbers to be the same EVER. These are
> >assigned by the host computer.
>
> Alright: is there a better invariant nomenclature to talk about "the
> gadget / host that is running on a particular pair of pins"? That's
> the ultimate definition of what's wired to what.
For USB, there is no such thing, sorry. You can see the "device
connected to the 3rd port of the second hub" but that's a mess and those
numbers can (and do) change randomly at boot time.
> It's okay if the nomenclature differs for gadget or host because we
> can prepend a namepace token that is checked for before looking for
> a match in each case.
I'd have to see your patch for how you do this, but again note that USB
device numbering is NOT deterministic in any way. Just like PCI device
numbering isn't. You would have the same problem there as well if you
were to want to do this type of thing.
> >>Well, it's an RFC so if you have a better plan I am all ears.
> >
> >I am totally confused, and as you are messing with bus id numbers, and
> >confusing usb gadgets and drivers, I think you are as well.
> >
> >So, are you talking about USB gadgets, or USB drivers here? They are
> >different and have different requirements and responsibilities.
>
> Both can make use of platform_data. At the moment both gadget and
> host USB devices have NULL platform data members sitting there.
Host USB devices are different, as I noted before.
> What this proposal is about is being able to teleport structs into
> those pointers from the board file, when gadget or host are
> instantiated that match. After that, drivers that are interested to
> take direction about anything can tell what they're willing to look
> at in their platform_data struct definition.
Ok, let's see code before continuing to argue.
thanks,
greg k-h
On Fri, 2011-03-11 at 09:50 +0000, Andy Green wrote:
> Hi -
>
> platform_data is a well established way in Linux to pass configuration
> data up to on-board assets from a machine file like mach-xyz.c. It's
> also supported to pass platform_data up to devices that are probed
> asynchronously from busses like i2c as well, which is very handy.
.../...
That looks like something the device-tree would deal with nicely :-)
Cheers,
Ben.
> However AFAIK it's not possible to bind platform_data to probed USB
> devices as it stands.
>
> There are now boards which have on-board USB assets, for example OMAP4
> Panda which has a USB <-> Ethernet bridge wired up permanently. It'd be
> convenient to also be able to pass optional platform_data to these
> devices when they are asynchronously probed.
>
> So what's the feeling about a new api to register an array of
> platform_data pointers bound to static "devpath" names in the machine file?
>
> When a usb device is instantiated, it can check through this array if it
> exists, matching on devname, and attach the platform_data to the
> underlying probed usb device's dev->platform_data, which it seems is
> currently unused.
>
> The particular use that suggested this is on Panda, it would be ideal to
> be able to set a flag in the usb device's platform data that forces it
> to be named eth%d since it's a hardwired asset on the board with an RJ45
> socket.
>
> Comments, implementation suggestions, enquiries as to my level of crack
> consumption etc welcomed ^^
>
> -Andy
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Fri, 2011-03-11 at 16:41 +0000, Andy Green wrote:
> >> Is it true that for on-board devices, it can sometimes be legitimate
> >> and useful to be able to deliver platform_data from the board file
> >> through to stuff on a USB bus, same as you would for memory mapped,
> >> I2C, other busses?
> >>
> >> Or is that it since it is USB, it can never be useful or legitimate,
> >> no matter what different kind of wired-up on-board USB device it is,
> >> to have the board definition file configure the driver for that
> >> instantiation?
> >
> > Since it is USB, it is always discoverable, so it doesn't make any sense
> > to have this type of thing.
> >
> > And since your only example was a network device, I think you proved
> > your point :)
Actually there are reasons to pass that sort of static informations for
"discoverable" devices, essentially when such devices are wired on a
board. MAC addresses come to mind (how many on-board NICs don't have a
MAC SEEPROM wired to them ? Even on PCI we've seen that...) etc...
However, the device-tree would deal with that quite nicely. It doesn't
require you to represent discoverable devices but you -can- do it, and
it's typically a good idea for soldered devices for that reason.
The USB binding for device-trees uses physical port numbers in Hubs as
the primary addressing scheme so it would work just fine to represent
the USB devices soldered on the board as well.
And here you get your transport. The USB core wouldn't need more than a
handful of lines of code to generically update the usb_device's struct
device embedded device_node pointer when a match exists, so the driver
only needs to check if the device is linked to a device-tree node, and
if it is, fetch informations from there.
As to how the device-tree is constructed, all the usual stuff applies
here, can be in flash, can be "fixed up" by FW or platform code (to
stick a unique MAC address from flash for example or calculated from a
serial number etc....).
But you have you "transport" mechanism sorted.
Cheers,
Ben.
On 03/11/2011 09:44 PM, Somebody in the thread at some point said:
> I'd have to see your patch for how you do this, but again note that USB
> device numbering is NOT deterministic in any way. Just like PCI device
> numbering isn't. You would have the same problem there as well if you
> were to want to do this type of thing.
Alright, I will aim to make an implementation this weekend and we can see.
-Andy
On Fri, Mar 11, 2011 at 2:50 AM, Andy Green <[email protected]> wrote:
> Hi -
>
> platform_data is a well established way in Linux to pass configuration data
> up to on-board assets from a machine file like mach-xyz.c. ?It's also
> supported to pass platform_data up to devices that are probed asynchronously
> from busses like i2c as well, which is very handy.
>
> However AFAIK it's not possible to bind platform_data to probed USB devices
> as it stands.
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.
g.
On 03/11/2011 10:45 PM, Somebody in the thread at some point said:
> On Fri, Mar 11, 2011 at 2:50 AM, Andy Green<[email protected]> wrote:
>> Hi -
>>
>> platform_data is a well established way in Linux to pass configuration data
>> up to on-board assets from a machine file like mach-xyz.c. It's also
>> supported to pass platform_data up to devices that are probed asynchronously
>> from busses like i2c as well, which is very handy.
>>
>> However AFAIK it's not possible to bind platform_data to probed USB devices
>> as it stands.
>
> 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.
$ grep platform_data drivers/* -R | wc -l
2110
-Andy
On Fri, Mar 11, 2011 at 3:47 PM, Andy Green <[email protected]> wrote:
> On 03/11/2011 10:45 PM, Somebody in the thread at some point said:
>>
>> On Fri, Mar 11, 2011 at 2:50 AM, Andy Green<[email protected]> ?wrote:
>>>
>>> Hi -
>>>
>>> platform_data is a well established way in Linux to pass configuration
>>> data
>>> up to on-board assets from a machine file like mach-xyz.c. ?It's also
>>> supported to pass platform_data up to devices that are probed
>>> asynchronously
>>> from busses like i2c as well, which is very handy.
>>>
>>> However AFAIK it's not possible to bind platform_data to probed USB
>>> devices
>>> as it stands.
>>
>> 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.
>
> $ grep platform_data drivers/* -R | wc -l
> 2110
I'm not disputing that it is widely used; and it does make a certain
amount of sense for static device registrations, but it is not a nice
interface, and it is a horrible idea for dynamic devices.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
On Fri, 11 Mar 2011, Greg KH wrote:
> > Alright: is there a better invariant nomenclature to talk about "the
> > gadget / host that is running on a particular pair of pins"? That's
> > the ultimate definition of what's wired to what.
>
> For USB, there is no such thing, sorry. You can see the "device
> connected to the 3rd port of the second hub" but that's a mess and those
> numbers can (and do) change randomly at boot time.
The situation isn't quite that bad.
For gadgets this issue doesn't really arise. Until very recently it
wasn't possible to have more that one gadget driver loaded at any time
(in fact, it still may not be possible -- I don't recall whether the
changes have been merged yet). In any case, gadget drivers are usually
build as modules, so any necessary information could be passed as a
module parameter.
For USB devices, the topology path below the root hub _is_ fixed. If
you know that a particular device is hardwired to port 2 of a
particular root hub, that's not going to change. The problem is how to
identify the root hub.
This depends on the platform. On PCs, USB host controllers are usually
PCI devices. The only way to identify them is by the PCI device name.
Although these names don't change often, they _can_ change. On an
embedded system there usually are only a couple of host controllers and
they have more or less fixed names.
Alan Stern
On 03/12/2011 04:00 PM, Somebody in the thread at some point said:
Hi -
> This depends on the platform. On PCs, USB host controllers are usually
> PCI devices. The only way to identify them is by the PCI device name.
> Although these names don't change often, they _can_ change. On an
> embedded system there usually are only a couple of host controllers and
> they have more or less fixed names.
Right, on embedded boards typically the board definition file is
responsible for calling platform_add_device() to instantiate the usb
host / gadget / otg controllers. That is done deterministically each
boot. This async platform_data support is just targeted at stuff that
is fixed on to the board, so as you note it's reasonable to expect to be
able to bind to it if the bus itself is instantiated by the same file
synchronously.
I have sent a working RFC patchset to the lists now which implements
platform support for generic async platform_data; usb core and usbnet
specific platform_data; and finally board definition support for Panda
onboard usbnet device by leveraging the introduced generic support.
Beagle XM also has the same issues that can be solved by async
platform_data from the board definition file.
-Andy
On Friday 11 March 2011, Grant Likely wrote:
> On Fri, Mar 11, 2011 at 2:50 AM, Andy Green <[email protected]> wrote:
> > Hi -
> >
> > platform_data is a well established way in Linux to pass configuration data
> > up to on-board assets from a machine file like mach-xyz.c. It's also
> > supported to pass platform_data up to devices that are probed asynchronously
> > from busses like i2c as well, which is very handy.
> >
> > However AFAIK it's not possible to bind platform_data to probed USB devices
> > as it stands.
>
> 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'd still hope that we could do without either for hotpluggable
devices, but a device tree object fits better here, because
we already have ways to associate the data in the device node
with any device, not just platform devices, and the key/value
pairs make it much more flexible than platform data.
Arnd
On Fri, 11 Mar 2011, Greg KH wrote:
> On Fri, Mar 11, 2011 at 05:08:07PM +0000, Mark Brown wrote:
> > On Fri, Mar 11, 2011 at 08:56:42AM -0800, Greg KH wrote:
> > > On Fri, Mar 11, 2011 at 04:48:50PM +0000, Mark Brown wrote:
> >
> > > > USB itself is discoverable but the when the USB bus you're looking at is
> > > > one that's soldered down onto a board in a specific design all bets are
> > > > off regarding how complete the information you get will be. On a basic
> > > > level the designers may have done things like omit the configuration
> > > > EEPROMs that would set the device IDs that the driver should be relying
> > > > on to identify the hardware configuration. There may be other, nastier,
> > > > things going on.
> >
> > > Then you use the existing platform data for your USB host controller
> > > driver. Doesn't that work today just fine?
> >
> > Wrong end of the bus. This stuff is simple enough to deal with in a
> > system specific fashion, the standard solution would be to patch the
> > relevant drivers to hard code whatever is required.
>
> What drivers need this? Specifics please.
Let me quote Arnd Bergmann:
|I have just verified with my Pandaboard that the pins on the SMSC9514
|usb ethernet that are meant to be connected to a serial EEPROM are
|indeed not connected anywhere.
> I would brand these types of systems "extremely broken" :)
Indeed. But to hardware people, modifying the software is always cheaper.
> Anyway, specifics are the best way forward if anyone has such a messed
> up system.
PandaBoards are becoming quite popular.
Nicolas
On Wed, Mar 16, 2011 at 10:14:01PM -0400, Nicolas Pitre wrote:
> > What drivers need this? Specifics please.
>
> Let me quote Arnd Bergmann:
>
> |I have just verified with my Pandaboard that the pins on the SMSC9514
> |usb ethernet that are meant to be connected to a serial EEPROM are
> |indeed not connected anywhere.
>
> > I would brand these types of systems "extremely broken" :)
>
> Indeed. But to hardware people, modifying the software is always cheaper.
Oh, I know this all too well. I have lost months of my life to replace
a $0.05 part in a device in a past job :(
> > Anyway, specifics are the best way forward if anyone has such a messed
> > up system.
>
> PandaBoards are becoming quite popular.
I have one right here.
But can't this device be detected by the usb device id and the quirk
added that way? Like all other "odd" USB devices are currently handled?
thanks,
greg k-h
On Thu, Mar 17, 2011 at 01:13:20PM -0700, Greg KH wrote:
> On Wed, Mar 16, 2011 at 10:14:01PM -0400, Nicolas Pitre wrote:
> > PandaBoards are becoming quite popular.
> I have one right here.
> But can't this device be detected by the usb device id and the quirk
> added that way? Like all other "odd" USB devices are currently handled?
It's going to be an off the shelf USB ethernet controller. I'd be
astonished if the board-configurable device IDs weren't set from the
same SEPROM that the MAC address is so it'd just show up as a generic
chip of whatever kind.
On Thu, 17 Mar 2011, Greg KH wrote:
> On Wed, Mar 16, 2011 at 10:14:01PM -0400, Nicolas Pitre wrote:
> > > What drivers need this? Specifics please.
> >
> > Let me quote Arnd Bergmann:
> >
> > |I have just verified with my Pandaboard that the pins on the SMSC9514
> > |usb ethernet that are meant to be connected to a serial EEPROM are
> > |indeed not connected anywhere.
> >
> > > Anyway, specifics are the best way forward if anyone has such a messed
> > > up system.
> >
> > PandaBoards are becoming quite popular.
>
> I have one right here.
>
> But can't this device be detected by the usb device id and the quirk
> added that way? Like all other "odd" USB devices are currently handled?
It is not the device which is odd, but rather the environment in which
it is being used for this specific case. I'd expect to see the same
SMSC9514 chip also used in off-the-shelf USB-to-Ethernet dongles with no
quirks needed.
However, a different part of the kernel knows already perfectly well
when it is actually running on that specific board. Hence the desire to
come up with a mechanism allowing to tell the driver about a quirk for
the particular device instance on this board without having to do the
heavy round trip through user space.
Nicolas
On Thu, Mar 17, 2011 at 08:18:35PM +0000, Mark Brown wrote:
> On Thu, Mar 17, 2011 at 01:13:20PM -0700, Greg KH wrote:
> > On Wed, Mar 16, 2011 at 10:14:01PM -0400, Nicolas Pitre wrote:
>
> > > PandaBoards are becoming quite popular.
>
> > I have one right here.
>
> > But can't this device be detected by the usb device id and the quirk
> > added that way? Like all other "odd" USB devices are currently handled?
>
> It's going to be an off the shelf USB ethernet controller. I'd be
> astonished if the board-configurable device IDs weren't set from the
> same SEPROM that the MAC address is so it'd just show up as a generic
> chip of whatever kind.
Huh? All USB controllers you buy have the ability to set the vendor and
product id, so you should always be able to key off of that.
Isn't that the case here?
thanks,
greg k-h
On Thu, Mar 17, 2011 at 01:26:14PM -0700, Greg KH wrote:
> On Thu, Mar 17, 2011 at 08:18:35PM +0000, Mark Brown wrote:
> > It's going to be an off the shelf USB ethernet controller. I'd be
> > astonished if the board-configurable device IDs weren't set from the
> > same SEPROM that the MAC address is so it'd just show up as a generic
> > chip of whatever kind.
> Huh? All USB controllers you buy have the ability to set the vendor and
> product id, so you should always be able to key off of that.
> Isn't that the case here?
They generally the same facility that includes the ability to set the
MAC address so if one's not been provided it seems optimistic to expect
the other.
The way this is normally done is that the ethernet controller can be
attached to a SEPROM which it reads when it powers on. This will
contain a number of device configuration parameters, including the
vendor IDs and the MAC address, which will be configured before the
device makes itself available on the bus. If the system integrator has
omitted the SEPROM then the device will come up with defaults, usually
something like all zeros.
On Thu, Mar 17, 2011 at 09:24:49PM +0000, Mark Brown wrote:
> On Thu, Mar 17, 2011 at 01:26:14PM -0700, Greg KH wrote:
> > On Thu, Mar 17, 2011 at 08:18:35PM +0000, Mark Brown wrote:
>
> > > It's going to be an off the shelf USB ethernet controller. I'd be
> > > astonished if the board-configurable device IDs weren't set from the
> > > same SEPROM that the MAC address is so it'd just show up as a generic
> > > chip of whatever kind.
>
> > Huh? All USB controllers you buy have the ability to set the vendor and
> > product id, so you should always be able to key off of that.
>
> > Isn't that the case here?
>
> They generally the same facility that includes the ability to set the
> MAC address so if one's not been provided it seems optimistic to expect
> the other.
{sigh} You are probably right :(
> The way this is normally done is that the ethernet controller can be
> attached to a SEPROM which it reads when it powers on. This will
> contain a number of device configuration parameters, including the
> vendor IDs and the MAC address, which will be configured before the
> device makes itself available on the bus. If the system integrator has
> omitted the SEPROM then the device will come up with defaults, usually
> something like all zeros.
Ok, then again, let's deal with this on a case-by-case basis please, as
this is going to be a "rare" thing in the real world. I don't want to
encourage _any_ engineers to think that this is ok to do for USB
devices.
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?
Heck, it could be fixed today on future versions of this board, if
anyone from TI actually cared :(
thanks,
greg k-h
On Thu, Mar 17, 2011 at 05:03:17PM -0400, Nicolas Pitre wrote:
> On Thu, 17 Mar 2011, Greg KH wrote:
>
> > On Wed, Mar 16, 2011 at 10:14:01PM -0400, Nicolas Pitre wrote:
> > > > What drivers need this? Specifics please.
> > >
> > > Let me quote Arnd Bergmann:
> > >
> > > |I have just verified with my Pandaboard that the pins on the SMSC9514
> > > |usb ethernet that are meant to be connected to a serial EEPROM are
> > > |indeed not connected anywhere.
> > >
> > > > Anyway, specifics are the best way forward if anyone has such a messed
> > > > up system.
> > >
> > > PandaBoards are becoming quite popular.
> >
> > I have one right here.
> >
> > But can't this device be detected by the usb device id and the quirk
> > added that way? Like all other "odd" USB devices are currently handled?
>
> It is not the device which is odd, but rather the environment in which
> it is being used for this specific case. I'd expect to see the same
> SMSC9514 chip also used in off-the-shelf USB-to-Ethernet dongles with no
> quirks needed.
>
> However, a different part of the kernel knows already perfectly well
> when it is actually running on that specific board. Hence the desire to
> come up with a mechanism allowing to tell the driver about a quirk for
> the particular device instance on this board without having to do the
> heavy round trip through user space.
It's not "heavy" at all, as it happens today, for every device found in
the system.
Again, we have this infrastructure to do this, in place, for this very
reason, why else would we have created it, to ignore it?
Anyway, as stated before, if someone has a specific patch for the
pandaboard, send it on, and I'll consider it. Extra bonus points for
curse words describing the hardware people who are causing this to be
required.
thanks,
greg k-h
On Thu, Mar 17, 2011 at 02:31:08PM -0700, Greg KH wrote:
> On Thu, Mar 17, 2011 at 09:24:49PM +0000, Mark Brown wrote:
> > The way this is normally done is that the ethernet controller can be
> > attached to a SEPROM which it reads when it powers on. This will
> > contain a number of device configuration parameters, including the
> > vendor IDs and the MAC address, which will be configured before the
> > device makes itself available on the bus. If the system integrator has
> > omitted the SEPROM then the device will come up with defaults, usually
> > something like all zeros.
> Ok, then again, let's deal with this on a case-by-case basis please, as
> this is going to be a "rare" thing in the real world. I don't want to
> encourage _any_ engineers to think that this is ok to do for USB
> devices.
It's really not at all rare in the embedded world - the overwhelming
majority of the systems I've worked on have omitted the SEPROMS for
components that are soldered down in the system. It'd be pretty insane
to do it for things that are distinct USB devices but that's not the
case.
There are good solid engineering reasons for doing things this way if
you've got any prospect of shifting any kind of volume, as well as the
cost saving achieved by removing a component you also simplify and most
likely speed up the production process as you can reduce the number of
components that need to be programmed on each system that gets built.
> 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.
On Thu, Mar 17, 2011 at 09:40:42PM +0000, Mark Brown wrote:
> On Thu, Mar 17, 2011 at 02:31:08PM -0700, Greg KH wrote:
> > On Thu, Mar 17, 2011 at 09:24:49PM +0000, Mark Brown wrote:
>
> > > The way this is normally done is that the ethernet controller can be
> > > attached to a SEPROM which it reads when it powers on. This will
> > > contain a number of device configuration parameters, including the
> > > vendor IDs and the MAC address, which will be configured before the
> > > device makes itself available on the bus. If the system integrator has
> > > omitted the SEPROM then the device will come up with defaults, usually
> > > something like all zeros.
>
> > Ok, then again, let's deal with this on a case-by-case basis please, as
> > this is going to be a "rare" thing in the real world. I don't want to
> > encourage _any_ engineers to think that this is ok to do for USB
> > devices.
>
> It's really not at all rare in the embedded world - the overwhelming
> majority of the systems I've worked on have omitted the SEPROMS for
> components that are soldered down in the system. It'd be pretty insane
> to do it for things that are distinct USB devices but that's not the
> case.
>
> There are good solid engineering reasons for doing things this way if
> you've got any prospect of shifting any kind of volume, as well as the
> cost saving achieved by removing a component you also simplify and most
> likely speed up the production process as you can reduce the number of
> components that need to be programmed on each system that gets built.
>
> > 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? :)
Best of luck,
greg k-h
On Thursday 17 March 2011 21:26:14 Greg KH wrote:
> > It's going to be an off the shelf USB ethernet controller. I'd be
> > astonished if the board-configurable device IDs weren't set from the
> > same SEPROM that the MAC address is so it'd just show up as a generic
> > chip of whatever kind.
>
> Huh? All USB controllers you buy have the ability to set the vendor and
> product id, so you should always be able to key off of that.
>
> Isn't that the case here?
The same EEPROM that is missing here carries both the MAC address and
the vendor/product ID overrides.
Arnd
On Thu, Mar 17, 2011 at 11:11:36PM +0100, Arnd Bergmann wrote:
> On Thursday 17 March 2011 21:26:14 Greg KH wrote:
> > > It's going to be an off the shelf USB ethernet controller. I'd be
> > > astonished if the board-configurable device IDs weren't set from the
> > > same SEPROM that the MAC address is so it'd just show up as a generic
> > > chip of whatever kind.
> >
> > Huh? All USB controllers you buy have the ability to set the vendor and
> > product id, so you should always be able to key off of that.
> >
> > Isn't that the case here?
>
> The same EEPROM that is missing here carries both the MAC address and
> the vendor/product ID overrides.
{sigh}
It's as if BIOS programmers are now laying out board designs...
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 <[email protected]>
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");
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 <[email protected]>
<snip>
Very nice.
Andy and Mark, would this patch work for you?
thanks,
greg k-h
On 03/17/2011 10:53 PM, Somebody in the thread at some point said:
>> Not tested!
>>
>> Signed-off-by: Arnd Bergmann<[email protected]>
>
> <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
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
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 <[email protected]>
>
> 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");
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<[email protected]>
> >
> ><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
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<[email protected]>
> >
> ><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.
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<[email protected]>
>>>
>>> <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
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
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<[email protected]>
> > >
> > ><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
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
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
On 03/18/2011 12:11 AM, ext Arnd Bergmann wrote:
> On Thursday 17 March 2011 21:26:14 Greg KH wrote:
>>> It's going to be an off the shelf USB ethernet controller. I'd be
>>> astonished if the board-configurable device IDs weren't set from the
>>> same SEPROM that the MAC address is so it'd just show up as a generic
>>> chip of whatever kind.
>>
>> Huh? All USB controllers you buy have the ability to set the vendor and
>> product id, so you should always be able to key off of that.
>>
>> Isn't that the case here?
>
> The same EEPROM that is missing here carries both the MAC address and
> the vendor/product ID overrides.
>
Just curious, how do Panda boards manage to get the MAC address? Is it
programmed in some other flash memory?
The LAN9514 datasheet says
"If a properly configured EEPROM is not detected, it is the
responsibility of the Host LAN Driver to set the IEEE addresses."
So how does the Host LAN driver know what MAC address it should use?
--
regards,
-roger
On Friday 18 March 2011, Roger Quadros wrote:
> Just curious, how do Panda boards manage to get the MAC address? Is it
> programmed in some other flash memory?
>
> The LAN9514 datasheet says
> "If a properly configured EEPROM is not detected, it is the
> responsibility of the Host LAN Driver to set the IEEE addresses."
>
> So how does the Host LAN driver know what MAC address it should use?
This is the problem we're trying to work out. The EEPROM is not
present, and user space has no way of knowing which MAC address
to set to which device. Note that there might be multiple
USB devices of the same type that are indistinguishable to user
space, so setting the MAC address there would not be completely
safe.
Arnd
On 03/18/2011 11:01 AM, ext Arnd Bergmann wrote:
> On Friday 18 March 2011, Roger Quadros wrote:
>> Just curious, how do Panda boards manage to get the MAC address? Is it
>> programmed in some other flash memory?
>>
>> The LAN9514 datasheet says
>> "If a properly configured EEPROM is not detected, it is the
>> responsibility of the Host LAN Driver to set the IEEE addresses."
>>
>> So how does the Host LAN driver know what MAC address it should use?
>
> This is the problem we're trying to work out. The EEPROM is not
> present, and user space has no way of knowing which MAC address
> to set to which device. Note that there might be multiple
> USB devices of the same type that are indistinguishable to user
> space, so setting the MAC address there would not be completely
> safe.
>
And what happens if you need to use u-boot or some bootloader with
ethernet network support?
Wouldn't it be better to have the MAC address programmed at the
bootloader level?
--
regards,
-roger
On 03/18/2011 09:55 AM, Somebody in the thread at some point said:
>>> So how does the Host LAN driver know what MAC address it should use?
>>
>> This is the problem we're trying to work out. The EEPROM is not
>> present, and user space has no way of knowing which MAC address
>> to set to which device. Note that there might be multiple
>> USB devices of the same type that are indistinguishable to user
>> space, so setting the MAC address there would not be completely
>> safe.
>>
>
> And what happens if you need to use u-boot or some bootloader with
> ethernet network support?
>
> Wouldn't it be better to have the MAC address programmed at the
> bootloader level?
Generally trying to have stuff half done in bootloader and half in Linux
makes things fragile and makes another place for trouble to hide (in
addition to bootloader state such as U-Boot env or boot.scr or
what-have-you needing distro control for updates).
If your bootloader is bloated enough to target network operations under
its own steam, it's bloated enough to set a random MAC or use the same
CPU ID scheme. But it doesn't mean that Linux should not assert the
state it wants when it runs.
-Andy
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
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.
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
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 [email protected]
> 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
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.
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
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
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
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
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
On Fri, Mar 18, 2011 at 11:52 AM, Andy Green <[email protected]> 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.
On Fri, Mar 18, 2011 at 2:06 PM, Arnd Bergmann <[email protected]> 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.
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.
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
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.
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.
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
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
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.
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
[CC'ed interested parties in retrospect and resent in plain text]
On 11 March 2011 15:20, Andy Green <[email protected]> wrote:
>
> Hi -
>
> platform_data is a well established way in Linux to pass configuration data up to on-board assets from a machine file like mach-xyz.c. ?It's also supported to pass platform_data up to devices that are probed asynchronously from busses like i2c as well, which is very handy.
>
> However AFAIK it's not possible to bind platform_data to probed USB devices as it stands.
>
> There are now boards which have on-board USB assets, for example OMAP4 Panda which has a USB <-> Ethernet bridge wired up permanently. ?It'd be convenient to also be able to pass optional platform_data to these devices when they are asynchronously probed.
>
> So what's the feeling about a new api to register an array of platform_data pointers bound to static "devpath" names in the machine file?
>
> When a usb device is instantiated, it can check through this array if it exists, matching on devname, and attach the platform_data to the underlying probed usb device's dev->platform_data, which it seems is currently unused.
>
> The particular use that suggested this is on Panda, it would be ideal to be able to set a flag in the usb device's platform data that forces it to be named eth%d since it's a hardwired asset on the board with an RJ45 socket.
>
> Comments, implementation suggestions, enquiries as to my level of crack consumption etc welcomed ^^
Hi Andy,
Personally, I wouldn't have bothered thinking about some kernel-wide
solution to the Panda SMSC9514 issue. I think defining Panda specific
udev rules to 'rename the SMSC9614 on USB1(?) to ETH0' is sufficient and
legal to do. Bus enumeration algos change neither often nor enough.
I believe there would be far riskier assumptions in filesystems already.
But I do agree it is nice to have system wide solutions whenever due.
Like this attempted patchset. Which is based upon two questions :-
Q(a) Can discoverable buses(USB, SDIO etc) legally need platform_data
or similar?
Q(b) If yes, what 'key' is most suitable for identifying the right device
to attach the data to ?
(a) We already have a good example, of Panda's missing MAC. I am sure there
would be more to think of.
Though it is illegal for a NIC to not have MAC address, no spec demands the
MAC be on some EEPROM or like. Theoretically, the NIC vendor could
hand me a NIC
and a note with it's unique MAC address scribbled :)
As Mark already noted, savings pile if we could save eeproms when a device is
expected to ship in tens of thousands.
IIANM, Greg acknowledged passing MAC via board specific data
structure(albeit via DT)
It's a different matter that DT has many hearts to win(at least in ARM Linux)
So, perhaps the answer to Q(a) is - Yes.
(b) IMHO, though stable enough, the most obvious method of 'devpath association'
is indeed not future-proof.
Having parent pointers to compare sounds like a bit too intrusive.
People might want to suggest?
Thanks.
On Tue, Mar 22, 2011 at 08:32:28PM +0530, Jaswinder Singh wrote:
> It's a different matter that DT has many hearts to win(at least in ARM
> Linux)
> So, perhaps the answer to Q(a) is - Yes.
It's not really a question of hearts to win. Even for people who are
fully won over the implementation just isn't there in mainline and it's
not clear when it'll be delivered. Right now device tree can only be
used on SPARC, PowerPC and (I beleive) some x86 platforms.
On 22 March 2011 20:42, Mark Brown <[email protected]> wrote:
> On Tue, Mar 22, 2011 at 08:32:28PM +0530, Jaswinder Singh wrote:
>
>> It's a different matter that DT has many hearts to win(at least in ARM
>> Linux)
>> So, perhaps the answer to Q(a) is - Yes.
>
> It's not really a question of hearts to win. ?Even for people who are
> fully won over the implementation just isn't there in mainline and it's
> not clear when it'll be delivered. ?Right now device tree can only be
> used on SPARC, PowerPC and (I beleive) some x86 platforms.
Well, the most important heart of all is yet to be won - RMK.
And perhaps the least important (and easiest) of all too - I :)
On 03/22/2011 03:05 PM, Somebody in the thread at some point said:
Hi -
> Personally, I wouldn't have bothered thinking about some kernel-wide
> solution to the Panda SMSC9514 issue. I think defining Panda specific
> udev rules to 'rename the SMSC9614 on USB1(?) to ETH0' is sufficient and
> legal to do. Bus enumeration algos change neither often nor enough.
> I believe there would be far riskier assumptions in filesystems already.
Not sure you got all the points there. The interface index is not
targeted at all, it's the interface class. That is why I only talk
about usb%d vs eth%d.
If you think about what userland has to do to correct that, it involves
userland understanding that it is running on specifically a Panda board
or other boards that need mangling, and it is looking at the device that
is specifically the onboard one. (You cannot do it from VID/PID because
that same VID/PID can turn up in a second pluggable adapter case --
still an smsc95xx you see -- where you really would want it called
usb%d.) This is why elsewhere I refer to this as a "userland board
quirk database" being needed to solve it in userland. If it was
simpler, sure, I wouldn't bother looking to guide the driver's choice in
kernel because you can as pointed out meddle them from userland. But if
you look into what has to be done in Userland it's nothing like a normal
udev handler based only on vid/pid. Userland is actually super allergic
to knowing directly what it is running on and making decisions based on
that, for good reasons, and desperately wants to be generic leaving all
board quirks for the kernel to hide. And the machine definition file is
designed to understand board-specific information.
In any event, the thread established that particular problem is unlikely
to get solved in the mainline kernel. So other than correct any
misconception, there's no point going further into it as it stands.
> But I do agree it is nice to have system wide solutions whenever due.
> Like this attempted patchset. Which is based upon two questions :-
>
> Q(a) Can discoverable buses(USB, SDIO etc) legally need platform_data
> or similar?
>
> Q(b) If yes, what 'key' is most suitable for identifying the right device
> to attach the data to ?
>
> (a) We already have a good example, of Panda's missing MAC. I am sure there
> would be more to think of.
Having done the SDIO async platform_data support now, it turns out SDIO
WLAN modules really do need platform_data already and at least in the
(mainline) wl12xx case, go to some lengths to work around not having an
api to deliver it.
> Though it is illegal for a NIC to not have MAC address, no spec demands the
> MAC be on some EEPROM or like. Theoretically, the NIC vendor could
> hand me a NIC
> and a note with it's unique MAC address scribbled :)
> As Mark already noted, savings pile if we could save eeproms when a device is
> expected to ship in tens of thousands.
> IIANM, Greg acknowledged passing MAC via board specific data
> structure(albeit via DT)
> It's a different matter that DT has many hearts to win(at least in ARM Linux)
> So, perhaps the answer to Q(a) is - Yes.
>
> (b) IMHO, though stable enough, the most obvious method of 'devpath association'
> is indeed not future-proof.
What're you thinking it's not future-proof against? As a
path-structured string, it seems it should be ideal considering the same
path elements are used in, eg, sysfs as string elements to describe
device paths. Since the strings are in the same tree as the things that
generate them, it only means updating the strings if the generator for
them changes.
> Having parent pointers to compare sounds like a bit too intrusive.
> People might want to suggest?
I spoke for half an hour with Grant Likely yesterday and showed him my
patchset that now covers SDIO async platform_data and wl12xx WLAN
particularly as found on Panda. He asked for a couple of days to see if
he could figure any different approach -- he made a point of saying non
Device-Tree interestingly -- that he liked better. I am not sure that
he will find anything he likes better since after discussing it with
him, his objection is platform_data itself, and that's what stuff like
these WLAN modules on SDIO are already set up for. Anyway I will study
any suggestion.
What I saw from wl12xx though is there are already mainline drivers that
can benefit from just delivering their existing platform_data via async
platform_data api. So I plan to wait for Grant's comments and release a
new RFC patchset in the coming days.
-Andy
On 22 March 2011 21:34, Andy Green <[email protected]> wrote:
> On 03/22/2011 03:05 PM, Somebody in the thread at some point said:
>
> Hi -
>
>> ?Personally, I wouldn't have bothered thinking about some kernel-wide
>> solution to the Panda SMSC9514 issue. I think defining Panda specific
>> ?udev rules to 'rename the SMSC9614 on USB1(?) to ETH0' is sufficient and
>> legal to do. Bus enumeration algos change neither often nor enough.
>> I believe there would be far riskier assumptions in filesystems already.
>
> Not sure you got all the points there. ?The interface index is not targeted
> at all, it's the interface class. ?That is why I only talk about usb%d vs
> eth%d.
>
> If you think about what userland has to do to correct that, it involves
> userland understanding that it is running on specifically a Panda board or
> other boards that need mangling, and it is looking at the device that is
> specifically the onboard one. ?(You cannot do it from VID/PID because that
> same VID/PID can turn up in a second pluggable adapter case -- still an
> smsc95xx you see -- where you really would want it called usb%d.) ?This is
> why elsewhere I refer to this as a "userland board quirk database" being
> needed to solve it in userland. ?If it was simpler, sure, I wouldn't bother
> looking to guide the driver's choice in kernel because you can as pointed
> out meddle them from userland. ?But if you look into what has to be done in
> Userland it's nothing like a normal udev handler based only on vid/pid.
> ?Userland is actually super allergic to knowing directly what it is running
> on and making decisions based on that, for good reasons, and desperately
> wants to be generic leaving all board quirks for the kernel to hide. ?And
> the machine definition file is designed to understand board-specific
> information.
I thought I understood. You want the RJ-45 port of _Panda_ be
called ethN(say eth0) rather than usbN, while other USB-Ethernet adaptors,
if any and SMSC9514, connected to the USB downstream ports of
LAN9514 be named usbN.
If yes, isn't that possible by specifying a _Panda_ specific udev rule
to find and rename a network interface based only on its type and
devpath without needing its MAC address? Greg ?
The udev rule could be a part of some hwpack.
>> ? ?Though it is illegal for a NIC to not have MAC address, no spec demands
>> the
>> ?MAC be on some EEPROM or like. Theoretically, the NIC vendor could
>> hand me a NIC
>> and a note with it's unique MAC address scribbled :)
>> ?As Mark already noted, savings pile if we could save eeproms when a
>> device is
>> expected to ship in tens of thousands.
>> ?IIANM, Greg acknowledged passing MAC via board specific data
>> structure(albeit via DT)
>> It's a different matter that DT has many hearts to win(at least in ARM
>> Linux)
>> So, perhaps the answer to Q(a) is - Yes.
>>
>> (b) IMHO, though stable enough, the most obvious method of 'devpath
>> association'
>> ?is indeed not future-proof.
>
> What're you thinking it's not future-proof against?
Even if the rootfilesystem remains the same, the devpath association will
fail if, say, Linux USB core decides to number usb busses in reverse order as
a part of some code restructuring.
After all the USB spec doesn't say about the bus/port ordering.
Thanks.
On 03/22/2011 06:19 PM, Somebody in the thread at some point said:
Hi -
>>> Personally, I wouldn't have bothered thinking about some kernel-wide
>>> solution to the Panda SMSC9514 issue. I think defining Panda specific
>>> udev rules to 'rename the SMSC9614 on USB1(?) to ETH0' is sufficient and
>>> legal to do. Bus enumeration algos change neither often nor enough.
>>> I believe there would be far riskier assumptions in filesystems already.
>>
>> Not sure you got all the points there. The interface index is not targeted
>> at all, it's the interface class. That is why I only talk about usb%d vs
>> eth%d.
> I thought I understood. You want the RJ-45 port of _Panda_ be
> called ethN(say eth0) rather than usbN, while other USB-Ethernet adaptors,
> if any and SMSC9514, connected to the USB downstream ports of
> LAN9514 be named usbN.
That's right, your talk of usb1 eth0 made it sound like it had gotten
mixed up with interface numbering business again. I guess you did have
the point then.
> If yes, isn't that possible by specifying a _Panda_ specific udev rule
> to find and rename a network interface based only on its type and
> devpath without needing its MAC address? Greg ?
> The udev rule could be a part of some hwpack.
We are in a privileged position to suggest to stick all kinds in our
distro support that's specific to the target, and maintain it. However
just doing that relies on the current situation that people are cooking
rootfs-es that only work on one target. With the move to multi-target
single kernel images, and the corresponding multi-target bootloader
support along the same lines that's coming, it'll eventually be possible
to provide single SD images that run on many targets with common rootfs.
Adding more static hacks into hwpacks on the assumption it is "a panda
rootfs" goes in the wrong direction for that.
If it was solved with a flag ultimately in the board definition file of
the kernel, however that manifests itself eventually to do the actual
job, because that should be the single place all board-specific
knowledge is coming from as it stands, it'll just work on all distros
without contaminating them with our userland quirk database (be it held
in hwpacks or the initscripts).
>>> (b) IMHO, though stable enough, the most obvious method of 'devpath
>>> association'
>>> is indeed not future-proof.
>>
>> What're you thinking it's not future-proof against?
>
> Even if the rootfilesystem remains the same, the devpath association will
> fail if, say, Linux USB core decides to number usb busses in reverse order as
> a part of some code restructuring.
> After all the USB spec doesn't say about the bus/port ordering.
No that won't hurt anything since, as I said, the static paths are in
the same tree, and can be updated to the new -- equally deterministic --
name at the same time as people start numbering their buses backwards.
For example, the Panda SDIO module WLAN function is at path
"mmc1:0001:2", if a patchset comes and decides to call these guys sdio0
on that bus instead it just has to grep the board definition files for
mmc.\: and fix those up to the appropriate sdioN convention at the same
time and everything is cool.
-Andy
On 23 March 2011 00:07, Andy Green <[email protected]> wrote:
> On 03/22/2011 06:19 PM, Somebody in the thread at some point said:
>> If yes, isn't that possible by specifying a _Panda_ specific udev rule
>> to find and rename a network interface based only on its type and
>> devpath without needing its MAC address? ?Greg ?
>> The udev rule could be a part of some hwpack.
>
> We are in a privileged position to suggest to stick all kinds in our distro
> support that's specific to the target, and maintain it. ?However just doing
> that relies on the current situation that people are cooking rootfs-es that
> only work on one target. ?With the move to multi-target single kernel
> images, and the corresponding multi-target bootloader support along the same
> lines that's coming, it'll eventually be possible to provide single SD
> images that run on many targets with common rootfs. ?Adding more static
> hacks into hwpacks on the assumption it is "a panda rootfs" goes in the
> wrong direction for that.
Yes I know it's not a very elegant solution.
But we must remember, we are catering to users that lose sleep over
not having to call 'fixed RJ-45 port over USB' as eth0 :)
>>>> (b) IMHO, though stable enough, the most obvious method of 'devpath
>>>> association'
>>>> ?is indeed not future-proof.
>>>
>>> What're you thinking it's not future-proof against?
>>
>> Even if the rootfilesystem remains the same, the devpath association will
>> fail if, say, Linux USB core decides to number usb busses in reverse order
>> as
>> a part of some code restructuring.
>> After all the USB spec doesn't say about the bus/port ordering.
>
> No that won't hurt anything since, as I said, the static paths are in the
> same tree, and can be updated to the new -- equally deterministic -- name at
> the same time as people start numbering their buses backwards. For example,
> the Panda SDIO module WLAN function is at path "mmc1:0001:2", if a patchset
> comes and decides to call these guys sdio0 on that bus instead it just has
> to grep the board definition files for mmc.\: and fix those up to the
> appropriate sdioN convention at the same time and everything is cool.
I was not just talking about mmc -> sdio change. Rather the bus/port numbering.
Say, mmc1:0001:2 becomes mmc7:0201:1 by some new addressing scheme.
I agree the fix is as easy as it is to break old devpath assumption in
this case.
And yes, bus numbering also depends on things like modprobe order, which may not
be fixed for all platforms. Any change there too can break the devpath
association.
Let's see what other veterans suggest.
Best of luck.
On 03/22/2011 06:59 PM, Somebody in the thread at some point said:
> On 23 March 2011 00:07, Andy Green<[email protected]> wrote:
>> On 03/22/2011 06:19 PM, Somebody in the thread at some point said:
>
>>> If yes, isn't that possible by specifying a _Panda_ specific udev rule
>>> to find and rename a network interface based only on its type and
>>> devpath without needing its MAC address? Greg ?
>>> The udev rule could be a part of some hwpack.
>>
>> We are in a privileged position to suggest to stick all kinds in our distro
>> support that's specific to the target, and maintain it. However just doing
>> that relies on the current situation that people are cooking rootfs-es that
>> only work on one target. With the move to multi-target single kernel
>> images, and the corresponding multi-target bootloader support along the same
>> lines that's coming, it'll eventually be possible to provide single SD
>> images that run on many targets with common rootfs. Adding more static
>> hacks into hwpacks on the assumption it is "a panda rootfs" goes in the
>> wrong direction for that.
> Yes I know it's not a very elegant solution.
> But we must remember, we are catering to users that lose sleep over
> not having to call 'fixed RJ-45 port over USB' as eth0 :)
The characteristics of a kernel and userland solution are not the same
though.
If we fix it via the kernel's definitive understanding of what board it
is on, then I can put an, eg, fedora rootfs on it which doesn't have
hwpack concept, and I get the same result. The usb / eth thing is
indeed cosmetic and trivial in one sense but in another it is a valid
example IMO where it's the kernel's job to abstract the details of the
actual board out properly so userland does not ever have to know.
> I was not just talking about mmc -> sdio change. Rather the bus/port numbering.
> Say, mmc1:0001:2 becomes mmc7:0201:1 by some new addressing scheme.
> I agree the fix is as easy as it is to break old devpath assumption in
> this case.
OK so you and I at least agree that there is no reasonable "future
proofing" issue here.
> And yes, bus numbering also depends on things like modprobe order, which may not
> be fixed for all platforms. Any change there too can break the devpath
> association.
Well it has been a big thread, but I made it clear already this async
platform data stuff is only usable where the bus drivers and bus host
instantiation are built-ins done in the machine definition file.
They're the prerequisites for the determinism that is required, and it's
the common SoC case that it is already so.
The actual device drivers themselves, like wl12xx, can be in modules
though just fine, what is targeted is its path on a specified bus and
that is set by the bus driver. Whenever that logical device matching
that path does get instantiated, it gets tagged and that is working for
wl12xx case with the actual device driver in modules.
-Andy
> Q(b) If yes, what 'key' is most suitable for identifying the right
> device
> to attach the data to ?
We already pointed out that in the case of soldered-in device, the
"path" as in the series of -physical- ports leading to the device is
going to be stable.
For anything else, there's no solution. If the device doesn't have a
unique ID, you are toast, period.
On the other hand, I still think platform_data suck big time. Grant made
some good points about the lack of proper typing for example. I believe
that a device-tree based approach is much better in the long run (and
more flexible) despite Andy odd quasi-religious aversion for it.
Cheers,
Ben.
On 03/22/2011 09:08 PM, Somebody in the thread at some point said:
>> Q(b) If yes, what 'key' is most suitable for identifying the right
>> device
>> to attach the data to ?
>
> We already pointed out that in the case of soldered-in device, the
> "path" as in the series of -physical- ports leading to the device is
> going to be stable.
Agreed.
> For anything else, there's no solution. If the device doesn't have a
> unique ID, you are toast, period.
If you mean pluggable device tagging then I also agree it's for sure not
in scope for this kind of action.
> On the other hand, I still think platform_data suck big time. Grant made
> some good points about the lack of proper typing for example. I believe
Well, you share that opinion with Grant at least, although reasons to
back it up were in shorter supply. "Proper typing" as a serious issue
with platform_data rather than just being an impressive-sounding claim
-- or a "good point" -- doesn't resonate with me at all -->
There is proper typing for the all the members of the platform_data
structs on both sides; both the board definition file and the driver
both deal with a member u8 mac[6] as a u8 mac[6], say. The only time a
void * is involved is so the generic struct device can point to the
driver-specific platform_data struct. I guess we agree about that much,
that the scope of untyped members is just void * one per struct device,
to the platform_data. Otherwise the thing is typesafe either side for
members.
If the board definition file put a pointer to the wrong platform data
struct in a device, it's true it'd accept it at compiletime and crash
and burn at runtime. But it'd be a gross bug in the code that'd always
be wrong, like attaching platform data for configuring a usb host to a
uart device or so. It seems the guy writing this will notice quickly
his uart doesn't work or blows OOPSes and investigate and permanently
fix it. The fallout of the void * involved is limited to that situation
alone.
Likewise if the device path mapping stuff works as it seems is now
generally accepted it would within restrictions typical for SoC
environment, you either wrote code to deliver the right or the wrong
platform_data there. If it can't be you soldered a WLAN module on mmc1,
you attach async platform_data appropriate for the WLAN module's driver,
but actually, a WiMAX driver got applied and now the platform_data via
the void * is wrong; the whole situation would be wrong because the
wrong driver came.
So while I see the point about void * and no type checking, it doesn't
allow any more subtle error than pointing at the wrong or the right
platform_data struct in the code. In practice, you will notice that
whatever it is you thought you were setting via platform_data is getting
set to something uncontrolled if it is "not the platform_data you are
looking for" and go fix it in the code. For these reasons of its
problem being in such a narrow domain, it seems to me quite a weak stick
to wave at platform_data.
> that a device-tree based approach is much better in the long run (and
> more flexible) despite Andy odd quasi-religious aversion for it.
As Mark Brown wrote earlier about this, the Device Tree "implementation
just isn't there in mainline".
-Andy
On Tue, 2011-03-22 at 22:37 +0000, Andy Green wrote:
> > that a device-tree based approach is much better in the long run
> (and
> > more flexible) despite Andy odd quasi-religious aversion for it.
>
> As Mark Brown wrote earlier about this, the Device Tree
> "implementation
> just isn't there in mainline".
Right and will take even longer to get there as long as short sighted
people like yourself appear to run some kind of religious battle against
it for no good technical reason that I can fathom so far.
Ben.
On Wed, 23 Mar 2011, Benjamin Herrenschmidt wrote:
> On Tue, 2011-03-22 at 22:37 +0000, Andy Green wrote:
> > > that a device-tree based approach is much better in the long run
> > (and
> > > more flexible) despite Andy odd quasi-religious aversion for it.
> >
> > As Mark Brown wrote earlier about this, the Device Tree
> > "implementation
> > just isn't there in mainline".
>
> Right and will take even longer to get there as long as short sighted
> people like yourself appear to run some kind of religious battle against
> it for no good technical reason that I can fathom so far.
Sorry Ben, but you are the one who sounds like a priest here, having
invoked the "religious" qualifier twice in a row in this thread.
I think that Andy is asking absurdly good questions which are backed by
candid logic and reasoning. If anything, his arguments are purely
technical and extremely practical. And so far all he's got for answers
was rather subjective, emotionally charged and even dogmatic.
With regards to DT on ARM I'm rather "softly" convinced this is a good
thing. However seeing a persisting lack of truly technical answers to
Andy's questions is rather disturbing, and makes me wish for much more
than the current hype around DT which appears to fall flat when
challenged.
There is one hard fact that no one can ignore: DT support on ARM still
has a long way to go before it is truly usable. The world just can't
stop turning until this is ready.
Nicolas
> Sorry Ben, but you are the one who sounds like a priest here, having
> invoked the "religious" qualifier twice in a row in this thread.
Time to burn a few books then :-)
> I think that Andy is asking absurdly good questions which are backed by
> candid logic and reasoning. If anything, his arguments are purely
> technical and extremely practical. And so far all he's got for answers
> was rather subjective, emotionally charged and even dogmatic.
I think the technical aspects have been essentially answered. The
important part of the thread is what the "right" approach to identifying
the on-board device (vs. an equivalent device being hot-plugged), and I
think we all agreed this has to be the physical port "path" (in case
there's hubs or switches on the way).
The device-tree and whatever other udev based solutions using physical
path are both "transports" in a sense for that same information and so
in the grand scheme of thing equivalent.
The argument boils down to should we encourage adding an additional
in-kernel platform_data based hooks for dynamically probed busses such
as USB as well ? or go for a udev based solution for the time being
which would probably work as well in practice and focus on getting the
device-tree based solution for the long term instead.
> With regards to DT on ARM I'm rather "softly" convinced this is a good
> thing. However seeing a persisting lack of truly technical answers to
> Andy's questions is rather disturbing, and makes me wish for much more
> than the current hype around DT which appears to fall flat when
> challenged.
I don't think any technical answer is missing. Dynamic platform data is
possible and in fact the platform could do it today using bus notifiers
and "hooking up" the platform data on the fly if needed I suppose.
Does it make sense however to add platform data for generic probed
devices ? I don't think so.
For some reason Andy doesn't seem to consider the lack of type
information as a problem, Grant and I do, I don't know where we can go
from there, it's a very technical argument and I don't feel like I need
to teach people on this list what are the drawbacks on relying on void *
pointers to structures attached to devices like that that -will- go
wrong.
I simply believe that this is the wrong solution for the problem. I
would very much prefer having a way for the driver to retrieve "platform
attributes".
This is essentially what the name/value properties of the device-tree
provide, but it could be abstracted in a way that doesn't require the
device-tree and allows drivers to be unchanged whether the thing is
backed with a DT or by platform callbacks as long as there's a minimum
amount of thought given to naming the property reasonably.
For some reason I didn't find Andy answers conductive to a reasonable
discussion and indeed I admit I probably switched to dismiss/troll mode
a bit too quickly, so let's the heat go down a bit here and see if we
can find a common ground.
If you guys can agree on my above proposal, we could maybe come up with
some "get_device_attribute(dev, name)" kind of interface, that would
then boild down to platform calls or device-tree transparently (or arch
calls optionally populated by generic device-tree based helpers
etc....).
This would be much better I believe that having those random structures
hooked up to void * pointers floating around and also generally more
flexible vs. platforms that provide or don't provide some of this
information (platforms might provide a subset etc...)
> There is one hard fact that no one can ignore: DT support on ARM still
> has a long way to go before it is truly usable. The world just can't
> stop turning until this is ready.
Right but more efforts could be put into making that happen :-)
Cheers
Ben.
>
> Nicolas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Wed, 23 Mar 2011, Benjamin Herrenschmidt wrote:
> I think the technical aspects have been essentially answered. The
> important part of the thread is what the "right" approach to identifying
> the on-board device (vs. an equivalent device being hot-plugged), and I
> think we all agreed this has to be the physical port "path" (in case
> there's hubs or switches on the way).
Yes, no one appears to disagree there.
> The argument boils down to should we encourage adding an additional
> in-kernel platform_data based hooks for dynamically probed busses such
> as USB as well ? or go for a udev based solution for the time being
> which would probably work as well in practice and focus on getting the
> device-tree based solution for the long term instead.
The udev solution has its set of drawbacks. Using udev is perfect for
arbitrary user policies. But here we're talking about a particular
device which happens to be soldered on a particular board, and adding
that knowledge to udev which is a separately maintained piece of
software from the kernel is rather redundant while the kernel already
knows perfectly well on which hardware it is running. And as Andy
pointed out, the kernel's job is to abstract hardware peculiarities, not
to force them to user space.
> > With regards to DT on ARM I'm rather "softly" convinced this is a good
> > thing. However seeing a persisting lack of truly technical answers to
> > Andy's questions is rather disturbing, and makes me wish for much more
> > than the current hype around DT which appears to fall flat when
> > challenged.
>
> I don't think any technical answer is missing. Dynamic platform data is
> possible and in fact the platform could do it today using bus notifiers
> and "hooking up" the platform data on the fly if needed I suppose.
... or even perform the needed intra-kernel API call to do the desired
change to the registered device if possible, such as overriding the
random MAC that the driver creates.
> Does it make sense however to add platform data for generic probed
> devices ? I don't think so.
Generally speaking, this wouldn't make sense. but this is a case where
a generically probed device happens to be used in a very specific
hardware design with its own quirks. in that very particular case then
it certainly makes some sense.
But if a standard notifier callback can be invoked once the device has
been probed and before udev is invoked, and if that notifier callback
can modify the defaults that were set by the generic drivers, then I
don't think there would be a need for any additional infrastructure that
only a very few oddball devices would use anyway.
> For some reason Andy doesn't seem to consider the lack of type
> information as a problem, Grant and I do, I don't know where we can go
> from there, it's a very technical argument and I don't feel like I need
> to teach people on this list what are the drawbacks on relying on void *
> pointers to structures attached to devices like that that -will- go
> wrong.
Still, as Andy said, the void pointer is quickly converted to a fully
typed structure, and then it is rather hard to have a driver working if
the platform data structure is mismatched between the driver and the
actual device registration. So, while this is true that there is a
possibility for misuse, in practice this is rather unlikely to go very
far without being noticed, and therefore this argument alone is rather
weak in support of a significant world order change.
> I simply believe that this is the wrong solution for the problem. I
> would very much prefer having a way for the driver to retrieve "platform
> attributes".
No dispute there. But some of us also would prefer a more pragmatic
solution in the mean time, given DT is not there yet.
> This is essentially what the name/value properties of the device-tree
> provide, but it could be abstracted in a way that doesn't require the
> device-tree and allows drivers to be unchanged whether the thing is
> backed with a DT or by platform callbacks as long as there's a minimum
> amount of thought given to naming the property reasonably.
>
> For some reason I didn't find Andy answers conductive to a reasonable
> discussion and indeed I admit I probably switched to dismiss/troll mode
> a bit too quickly, so let's the heat go down a bit here and see if we
> can find a common ground.
I don't think Andy is rejecting any alternatives, given it is practical
in the short term. That unfortunately pretty much rules out DT for the
time being.
> If you guys can agree on my above proposal, we could maybe come up with
> some "get_device_attribute(dev, name)" kind of interface, that would
> then boild down to platform calls or device-tree transparently (or arch
> calls optionally populated by generic device-tree based helpers
> etc....).
I wholeheartedly agree with that idea. One thing that bothers me about DT
is its tendency to show up deep into the guts of kernel code all over
the place. If a generic abstraction can be put in place, such as this
get_device_attribute() example, then it is far more convenient to use
DT, or _eventually_ use DT, or even _not_ use it in some cases but
something else, without battling over the actual interface.
> This would be much better I believe that having those random structures
> hooked up to void * pointers floating around and also generally more
> flexible vs. platforms that provide or don't provide some of this
> information (platforms might provide a subset etc...)
Agreed.
> > There is one hard fact that no one can ignore: DT support on ARM still
> > has a long way to go before it is truly usable. The world just can't
> > stop turning until this is ready.
>
> Right but more efforts could be put into making that happen :-)
Indeed. But that would be a discussion for another thread.
Nicolas
On Wed, Mar 23, 2011 at 12:21:41AM -0400, Nicolas Pitre wrote:
> On Wed, 23 Mar 2011, Benjamin Herrenschmidt wrote:
> > The argument boils down to should we encourage adding an additional
> > in-kernel platform_data based hooks for dynamically probed busses such
> > as USB as well ? or go for a udev based solution for the time being
> > which would probably work as well in practice and focus on getting the
> > device-tree based solution for the long term instead.
>
> The udev solution has its set of drawbacks. Using udev is perfect for
> arbitrary user policies. But here we're talking about a particular
> device which happens to be soldered on a particular board, and adding
> that knowledge to udev which is a separately maintained piece of
> software from the kernel is rather redundant while the kernel already
> knows perfectly well on which hardware it is running. And as Andy
> pointed out, the kernel's job is to abstract hardware peculiarities, not
> to force them to user space.
{sigh}
This is the _exact_ same argument that the server manufacturers tried to
do for the naming of the ethernet pci devices and naming of them to
match up with the physical label that is on the back of the machine.
Because of that, please use the same tools that they created to solve
this problem. It's done in userspace, in a way that works for everyone
and on all distros, even your embedded one.
I do not understand why everyone is so resistant to this solution? Have
you all tried it out and found problems with it?
thanks,
greg k-h
On Wed, 2011-03-23 at 00:21 -0400, Nicolas Pitre wrote:
>
> I wholeheartedly agree with that idea. One thing that bothers me about DT
> is its tendency to show up deep into the guts of kernel code all over
> the place. If a generic abstraction can be put in place, such as this
> get_device_attribute() example, then it is far more convenient to use
> DT, or _eventually_ use DT, or even _not_ use it in some cases but
> something else, without battling over the actual interface
I've always tried to take the approach on powerpc when designing an
important feature such as the virtual IRQ remapping, to make the
device-tree use a "useful add-on" without making it entirely necessary.
To continue on that example, the low level irq_domain (irq_host but
being renamed) only carries the OF pointer as a "token" (I think at some
point I even had it a void *), and the low level APIs for establishing
the mapping between a domain/hw_irq and virq don't rely on any
device-tree bits at all.
However, helpers built on top of that (along with one "translation"
callback in the irq domain itself) provide the device-tree integration
which makes the whole thing a lot easier to use.
But we still use the low level bits in some areas, old machines with
broken DT, hard wired interrupt numbers coming from firmware, etc...
Cheers,
Ben.
On 03/23/2011 03:23 AM, Somebody in the thread at some point said:
>
>> Sorry Ben, but you are the one who sounds like a priest here, having
>> invoked the "religious" qualifier twice in a row in this thread.
>
> Time to burn a few books then :-)
>
>> I think that Andy is asking absurdly good questions which are backed by
>> candid logic and reasoning. If anything, his arguments are purely
>> technical and extremely practical. And so far all he's got for answers
>> was rather subjective, emotionally charged and even dogmatic.
>
> I think the technical aspects have been essentially answered. The
> important part of the thread is what the "right" approach to identifying
> the on-board device (vs. an equivalent device being hot-plugged), and I
> think we all agreed this has to be the physical port "path" (in case
> there's hubs or switches on the way).
Well I am glad there is consensus on that, although this was the initial
approach in the patches already.
For me the most important issues I wanted to understand from the RFC
were: "do others see it as generally useful to target async probed
devices with data from the board definition file", and was there
appetite to use the apis elsewhere in the kernel to solve the problems
in front of me in Panda.
I learned that among people that understood what the RFC was about there
was also consensus it's useful, although some people demand Device Tree
implementation, I think we got beyond arguing against the abstract idea
overall: it is accepted it would be valuable. So this is encouraging as
far as it goes.
For subsystems using it, in the initial USB case discussion got mired
first in understanding the idea at all from USB perspective, and then
lost down well-trodden positions on what the actual data is used for in
the included use-cases. The same is happening here -->
> The device-tree and whatever other udev based solutions using physical
> path are both "transports" in a sense for that same information and so
> in the grand scheme of thing equivalent.
>
> The argument boils down to should we encourage adding an additional
> in-kernel platform_data based hooks for dynamically probed busses such
Well, these are not hooks in the usual usage of the word.
> as USB as well ? or go for a udev based solution for the time being
> which would probably work as well in practice and focus on getting the
> device-tree based solution for the long term instead.
There is no udev solution for what is being done currently by the async
platform_data patchset with SDIO WLAN. The patches are out there and in
use already. The only reason I don't post them here as round 2 of the
RFC yet is because Grant wanted a couple of days and politically it's
expedient for me to agree to that.
>> With regards to DT on ARM I'm rather "softly" convinced this is a good
>> thing. However seeing a persisting lack of truly technical answers to
>> Andy's questions is rather disturbing, and makes me wish for much more
>> than the current hype around DT which appears to fall flat when
>> challenged.
>
> I don't think any technical answer is missing. Dynamic platform data is
> possible and in fact the platform could do it today using bus notifiers
> and "hooking up" the platform data on the fly if needed I suppose.
Ah now, steady on. I was very surprised actually and I still am in this
thread how incomplete the thinking and implementation is for Device Tree
interoperation with platform_data. Surely interoperation with the
established way of doing things should have been very high on the list
of things to have an answer for before embarking on everything else. In
fact, how to do that in drivers seems to be being figured out by you
guys as you go along in this thread as if my RFC is the first time that
particular critical bit of Device Tree rubber is meeting the real road.
I have no idea why the burden of that has appeared on my particular back.
When I described the issues I see with Arnd's approach fracturing
support for new features and bugfixes in drivers, it was waved off
because of patches that were "just posted" that apparently "solve" this.
Where they were posted, what they do, how they solve it was not told
and that strand of the discussion was killed. However it seemed like
the single most important issue to come out of the thread for Device
Tree and we take it back up again in this post.
> Does it make sense however to add platform data for generic probed
> devices ? I don't think so.
>
> For some reason Andy doesn't seem to consider the lack of type
> information as a problem, Grant and I do, I don't know where we can go
> from there, it's a very technical argument and I don't feel like I need
> to teach people on this list what are the drawbacks on relying on void *
> pointers to structures attached to devices like that that -will- go
> wrong.
I spent some time last night writing up exactly why I don't consider it
a real issue in exactly the platform_data case. During that I agree
typesafety is generally good and void * generally bad. So, it's more ad
hominem to try to make out you "need to teach people" about that; it's
consistent with what seems to be a quite arrogant culture around Device
Tree that it is the higher path and anyone questioning or disagreeing
with it is a fool / religious / odd. But when I study the specific case
of void * in platform_data, I find there is no real special problem
caused by it - and you did not show any.
> I simply believe that this is the wrong solution for the problem. I
> would very much prefer having a way for the driver to retrieve "platform
> attributes".
>
> This is essentially what the name/value properties of the device-tree
> provide, but it could be abstracted in a way that doesn't require the
> device-tree and allows drivers to be unchanged whether the thing is
> backed with a DT or by platform callbacks as long as there's a minimum
> amount of thought given to naming the property reasonably.
>
> For some reason I didn't find Andy answers conductive to a reasonable
> discussion and indeed I admit I probably switched to dismiss/troll mode
I don't mind being called "full of shit" under these circumstances
though, because it is useful to gauge how insecure people are about
their arguments, and after all, everyone reading has their own mind to
make up.
> a bit too quickly, so let's the heat go down a bit here and see if we
> can find a common ground.
>
> If you guys can agree on my above proposal, we could maybe come up with
> some "get_device_attribute(dev, name)" kind of interface, that would
> then boild down to platform calls or device-tree transparently (or arch
> calls optionally populated by generic device-tree based helpers
> etc....).
What is so surprising is that you seem to start to think about this on
my RFC thread in mid-March 2011.
OK. So it seems your proposal to interoperate with platform_data should
be studied. How exactly will it work with existing platform_data
structs? It will work with existing platform_data structs, will it?
> This would be much better I believe that having those random structures
> hooked up to void * pointers floating around and also generally more
But there are a huge number of users of platform_data in mainline
already we can agree. Are you talking about a mass conversion of those
to eliminating platform_data so they use your preferred token query model?
If so, could I possibly suggest sticking your own neck above the parapet
on that and issuing your own RFC to defend, as I have? You could call
it, eg, "RFC: change every driver using platform_data to use token
queries", and you know, explain what that effort actually buys anyone.
In the meanwhile, I can get on with my implementation and gladly change
it to yours when - not if, of course - it hits mainline.
> flexible vs. platforms that provide or don't provide some of this
> information (platforms might provide a subset etc...)
>
>> There is one hard fact that no one can ignore: DT support on ARM still
>> has a long way to go before it is truly usable. The world just can't
>> stop turning until this is ready.
>
> Right but more efforts could be put into making that happen :-)
I think the first additional effort needs to start at home on that one
and think through Device Tree and kernel policy on interoperation with
existing driver implementations using platform_data. Just being sniffy
about platform_data for reasons you can't back up when challenged won't
cut it IMO.
-Andy
> > Does it make sense however to add platform data for generic probed
> > devices ? I don't think so.
>
> Generally speaking, this wouldn't make sense. but this is a case where
> a generically probed device happens to be used in a very specific
> hardware design with its own quirks. in that very particular case then
> it certainly makes some sense.
If it's a very specific hardware design it can do its own very specific
internal private kernel patch, or little config app in user space. There
isn't a valid reason to inflict that complexity on the other 99.999999%
of users.
> actual device registration. So, while this is true that there is a
> possibility for misuse, in practice this is rather unlikely to go very
> far without being noticed, and therefore this argument alone is rather
> weak in support of a significant world order change.
You mean it'll get missed and years later will get compiled into some
other device and a joker with a custom USB widget will add the ids to his
widget and declare it an exploit ?
The other thing being overlooked is that for board specific horrors you
can still put them in udev quite easily if your udev happens to know how
to parse some kind of firmware provided table. That might even be bits of
your device tree.
The kernel simply shouldn't get involved in machine specific namespace
fights.
Alan
> There is no udev solution for what is being done currently by the async
> platform_data patchset with SDIO WLAN. The patches are out there and in
> use already. The only reason I don't post them here as round 2 of the
> RFC yet is because Grant wanted a couple of days and politically it's
> expedient for me to agree to that.
Kernel policy has always been that just because some vendor has deployed
an interface doesn't mean we care one iota about it or consider it an
argument for the solution. In some cases in fact it bcomes the working
demo of why it was a bad idea.
> But there are a huge number of users of platform_data in mainline
> already we can agree. Are you talking about a mass conversion of those
> to eliminating platform_data so they use your preferred token query model?
Ultimately that will probably be worth doing once we have some kind of
meaningful model. We've actually been slowly beating platform devices
into making more use of the resource data it does have when possible, and
we have various parsers between platform data and device specific
platform data structs that are horrible glue-together jobs that it would
be nice to stomp on.
> I think the first additional effort needs to start at home on that one
> and think through Device Tree and kernel policy on interoperation with
> existing driver implementations using platform_data. Just being sniffy
> about platform_data for reasons you can't back up when challenged won't
> cut it IMO.
Much of this depends upon what the data is and what it changes. Things
like interface names for example shouldn't be kernel mangled on the whim
of random board vendors. They follow a unified behaviour *across*
platforms, which is what matters most in the bigger picture.
Other stuff is to all intents and purposes sometimes hard coded into
drivers because while they are USB drivers they are checking specific
vendor id fields and changing behaviour on them. In some cases those
vendor id fields are actually a specific hardwired device.
Alan
On 03/23/2011 09:47 AM, Somebody in the thread at some point said:
>> There is no udev solution for what is being done currently by the async
>> platform_data patchset with SDIO WLAN. The patches are out there and in
>> use already. The only reason I don't post them here as round 2 of the
>> RFC yet is because Grant wanted a couple of days and politically it's
>> expedient for me to agree to that.
>
> Kernel policy has always been that just because some vendor has deployed
> an interface doesn't mean we care one iota about it or consider it an
> argument for the solution. In some cases in fact it bcomes the working
> demo of why it was a bad idea.
To be clear, this is not about any funny business at the interface on
the hardware.
The SDIO patches target wl12xx that is already in mainline and already
using literally platform_data. Because there's no neater way on offer,
it currently -- in mainline -- does it by having a built-in stub with
its own Kconfig, that copies platform_data from the board definition
file into a private malloc'd buffer, then uses it by getting a pointer
to the copy from another private api in the driver. All this in a
specific driver.
Considering the driver for SDIO WLAN modules like this must be told
critical stuff like which irq line they are wired up to, which is an
attribute of the physical board, having the async platform_data support
is making things cleaner and more generic in mainline directly in this
case at least.
>> But there are a huge number of users of platform_data in mainline
>> already we can agree. Are you talking about a mass conversion of those
>> to eliminating platform_data so they use your preferred token query model?
>
> Ultimately that will probably be worth doing once we have some kind of
> meaningful model. We've actually been slowly beating platform devices
> into making more use of the resource data it does have when possible, and
> we have various parsers between platform data and device specific
> platform data structs that are horrible glue-together jobs that it would
> be nice to stomp on.
For devices that are probed asynchronously though, resource and
platform_data have the same issue they can't be attached until the
device is instantiated.
>> I think the first additional effort needs to start at home on that one
>> and think through Device Tree and kernel policy on interoperation with
>> existing driver implementations using platform_data. Just being sniffy
>> about platform_data for reasons you can't back up when challenged won't
>> cut it IMO.
>
> Much of this depends upon what the data is and what it changes. Things
> like interface names for example shouldn't be kernel mangled on the whim
> of random board vendors. They follow a unified behaviour *across*
> platforms, which is what matters most in the bigger picture.
>
> Other stuff is to all intents and purposes sometimes hard coded into
> drivers because while they are USB drivers they are checking specific
> vendor id fields and changing behaviour on them. In some cases those
> vendor id fields are actually a specific hardwired device.
It's the case for even usbnet, which is using a broken heuristic to
decide what to call the interface not even based on vid / pid.
Anyway I am reconciled changing device names to fit the physical board
setup isn't going to fly, it's more important to find out if async
platform_data as a concept will fly.
Before this RFC there's no generic support for the concept, although
that hasn't stopped it being implemented in wl12xx already at individual
mainline driver level. And before this RFC, I don't believe there was
any concept of async tagging through bus path for soldered-on assets in
Device Tree either.
-Andy
> Well I am glad there is consensus on that, although this was the initial
> approach in the patches already.
>
> For me the most important issues I wanted to understand from the RFC
> were: "do others see it as generally useful to target async probed
> devices with data from the board definition file", and was there
> appetite to use the apis elsewhere in the kernel to solve the problems
> in front of me in Panda.
There is definitely some use into providing what I call "auxilliary"
data to dynamically probed devices (I'm not a fan of the "async"
terminology here), however I believe that doing so by perpetuating the
platform_data mechanism isn't the right way to go.
> I learned that among people that understood what the RFC was about there
> was also consensus it's useful, although some people demand Device Tree
> implementation, I think we got beyond arguing against the abstract idea
> overall: it is accepted it would be valuable. So this is encouraging as
> far as it goes.
Right.
> For subsystems using it, in the initial USB case discussion got mired
> first in understanding the idea at all from USB perspective, and then
> lost down well-trodden positions on what the actual data is used for in
> the included use-cases. The same is happening here -->
Note that the discussion is valid for PCI as well. A typical example is
old style PCI 2.x (still quite common) where interrupt lines for on
board devices are wired to random GPIO :-)
Here too we have an old ad-hoc solution which tends to be subtely
different for all architectures, tho at least drivers generally don't
care. The basic problem is the same tho.
> > The device-tree and whatever other udev based solutions using physical
> > path are both "transports" in a sense for that same information and so
> > in the grand scheme of thing equivalent.
> >
> > The argument boils down to should we encourage adding an additional
> > in-kernel platform_data based hooks for dynamically probed busses such
>
> Well, these are not hooks in the usual usage of the word.
>
> > as USB as well ? or go for a udev based solution for the time being
> > which would probably work as well in practice and focus on getting the
> > device-tree based solution for the long term instead.
>
> There is no udev solution for what is being done currently by the async
> platform_data patchset with SDIO WLAN. The patches are out there and in
> use already. The only reason I don't post them here as round 2 of the
> RFC yet is because Grant wanted a couple of days and politically it's
> expedient for me to agree to that.
I haven't looked at your SDIO work nor have I time to get into details
here. However I'm concerned about the "big picture" if you like and
eager to move toward a generally more flexible model than platform_data
style data structures.
I don't believe that the legacy of platform_data for platform_devices is
relevant here as we are -not- talking about platform_devices, but
generally devices that have their own "struct device" subclass, and thus
have the potential of using new tools to fix those drivers without
necessarily engaging into an immediate conversion of all the existing
platform_devices in the tree.
> >> With regards to DT on ARM I'm rather "softly" convinced this is a good
> >> thing. However seeing a persisting lack of truly technical answers to
> >> Andy's questions is rather disturbing, and makes me wish for much more
> >> than the current hype around DT which appears to fall flat when
> >> challenged.
> >
> > I don't think any technical answer is missing. Dynamic platform data is
> > possible and in fact the platform could do it today using bus notifiers
> > and "hooking up" the platform data on the fly if needed I suppose.
>
> Ah now, steady on. I was very surprised actually and I still am in this
> thread how incomplete the thinking and implementation is for Device Tree
> interoperation with platform_data.
It's been generally an evolving process, with trial and error, as is
common with the Linux kernel. Nobody wants a big flag day which requires
a full conversion of the entire kernel.
Among ideas that have been proposed have been the idea of having stubs
generating pseudo-device nodes from platform_data for devices in order
to enable the drivers to use a single interface for example. That didn't
really go through tho. However, whatever the final approach will be, the
point remains that for drivers that have not so far been "contaminated"
by the platform_data paradigm, we have the opportunity to try something
better, which can then, maybe, be retro-fitted on a case by case basis
to platform drivers, for example as such drivers get converted to also
be capable of using the device-tree.
This is where I'm trying to propose a middle ground with the idea of
named properties. It goes toward the direction that platform_device has
been trying to to with the struct resource, but there's only so much you
can do with these and they are showing their limits when it comes to
expose arbitrary informations. MAC addresses are an example but there
are many more, from clock bindings, power control, PHY data, connector
informations, yadadada... which all can potentially apply to on-board
USB or PCI based devices.
> Surely interoperation with the
> established way of doing things should have been very high on the list
> of things to have an answer for before embarking on everything else. In
> fact, how to do that in drivers seems to be being figured out by you
> guys as you go along in this thread as if my RFC is the first time that
> particular critical bit of Device Tree rubber is meeting the real road.
> I have no idea why the burden of that has appeared on my particular back.
>
> When I described the issues I see with Arnd's approach fracturing
> support for new features and bugfixes in drivers, it was waved off
> because of patches that were "just posted" that apparently "solve" this.
> Where they were posted, what they do, how they solve it was not told
> and that strand of the discussion was killed. However it seemed like
> the single most important issue to come out of the thread for Device
> Tree and we take it back up again in this post.
Rant rant rant :-) The device-tree is a well proven mechanism on some
platforms, the question is mostly whether we want it to become the
standard way to go, and whether we can find out a reasonable approach to
migration that doesn't involve requiring an immediate conversion of all
drivers in the tree.
To some extent I think that an approach that can isolate the drivers
from the "transport" mechanism (device-tree or hard wired platform data
style data structures) would be best and that's where I coming with my
proposal of introducing a concept of named properties.
Cheers,
Ben.
On Wednesday 23 March 2011, Andy Green wrote:
> It's the case for even usbnet, which is using a broken heuristic to
> decide what to call the interface not even based on vid / pid.
I agree that the heuristics in usbnet is less than helpful here,
which among other things leads people to mixing up the two problems
of fixing the device naming and fixing the MAC address assignment.
Even though I know Greg disagrees, I'd still prefer sanitising the
rules for the default device name that usbnet assigns. Simply letting
a device driver flag "this is always an external ethernet, not
a point-to-point connection" would be enough to solve this problem,
and take some of the heat out of the discussion for how to solve
the MAC address assignment.
> Before this RFC there's no generic support for the concept, although
> that hasn't stopped it being implemented in wl12xx already at individual
> mainline driver level. And before this RFC, I don't believe there was
> any concept of async tagging through bus path for soldered-on assets in
> Device Tree either.
For the device tree, the method has always been to assign the device_node
pointer to the device when the parent bus gets probed, that has not changed.
The new idea that came as a result of your RFC is to use the same mechanism
that we use elsewhere for USB as well, with the existing USB binding.
Arnd
On 03/23/2011 10:32 AM, Somebody in the thread at some point said:
> On Wednesday 23 March 2011, Andy Green wrote:
>> It's the case for even usbnet, which is using a broken heuristic to
>> decide what to call the interface not even based on vid / pid.
>
> I agree that the heuristics in usbnet is less than helpful here,
> which among other things leads people to mixing up the two problems
> of fixing the device naming and fixing the MAC address assignment.
>
> Even though I know Greg disagrees, I'd still prefer sanitising the
> rules for the default device name that usbnet assigns. Simply letting
> a device driver flag "this is always an external ethernet, not
> a point-to-point connection" would be enough to solve this problem,
> and take some of the heat out of the discussion for how to solve
> the MAC address assignment.
Completely agree, I have a happy feeling being able to say that too.
But I already see there's no path through Greg let alone Alan, so it'll
have to be dealt with a less good way.
>> Before this RFC there's no generic support for the concept, although
>> that hasn't stopped it being implemented in wl12xx already at individual
>> mainline driver level. And before this RFC, I don't believe there was
>> any concept of async tagging through bus path for soldered-on assets in
>> Device Tree either.
>
> For the device tree, the method has always been to assign the device_node
> pointer to the device when the parent bus gets probed, that has not changed.
> The new idea that came as a result of your RFC is to use the same mechanism
> that we use elsewhere for USB as well, with the existing USB binding.
Fair enough.
-Andy
On Wed, Mar 23, 2011 at 09:38:47AM +0000, Alan Cox wrote:
> > Generally speaking, this wouldn't make sense. but this is a case where
> > a generically probed device happens to be used in a very specific
> > hardware design with its own quirks. in that very particular case then
> > it certainly makes some sense.
> If it's a very specific hardware design it can do its own very specific
> internal private kernel patch, or little config app in user space. There
> isn't a valid reason to inflict that complexity on the other 99.999999%
> of users.
Just to be clear this sort of stuff is not, in general, a particularly
obscure problem for embedded systems. General good practice in hardware
design is to remove components to achieve cost savings and improvements
in manufacturability and things like configuration SEPROMs tend to be
among the first things to go. Vendors producing reference boards for
these markets will tend go for the cost downs on such devices even if
the volumes are relatively low to ensure that their reference designs
are directly usable in end projects, and reference designs tend to be
disproportionately visible to people working with the kernel.
In the specific case of MACs and device names for network adaptors we
have userspace solutions which are obscuring the discussion but there
are other things which get configured this way which one would usually
expect to be handled in kernel.
> > rules for the default device name that usbnet assigns. Simply letting
> > a device driver flag "this is always an external ethernet, not
> > a point-to-point connection" would be enough to solve this problem,
> > and take some of the heat out of the discussion for how to solve
> > the MAC address assignment.
>
> Completely agree, I have a happy feeling being able to say that too.
> But I already see there's no path through Greg let alone Alan, so it'll
> have to be dealt with a less good way.
Having a USB net driver flag the fact it knows from its vid/did that it
is probably P2P rather than net (or vice versa) isn't something I see a
problem with, it's probably quite helpful for various network
autoconfigurator guesses. What we don't want however is the kernel
deciding the naming scheme should change. Name tweaks are really policy,
knowing if an interface vid/did say its P2P or net is merely information
that can guide a policy in user space.
Now if your user space uses that flag to issue ifrenames fine, that's
your choice. It's probably sufficient just to set IFF_POINTOPOINT
appropriately on the net interface flags to make all that work ?
Alan
On 03/23/2011 10:56 AM, Somebody in the thread at some point said:
>>> rules for the default device name that usbnet assigns. Simply letting
>>> a device driver flag "this is always an external ethernet, not
>>> a point-to-point connection" would be enough to solve this problem,
>>> and take some of the heat out of the discussion for how to solve
>>> the MAC address assignment.
>>
>> Completely agree, I have a happy feeling being able to say that too.
>> But I already see there's no path through Greg let alone Alan, so it'll
>> have to be dealt with a less good way.
>
> Having a USB net driver flag the fact it knows from its vid/did that it
> is probably P2P rather than net (or vice versa) isn't something I see a
> problem with, it's probably quite helpful for various network
> autoconfigurator guesses. What we don't want however is the kernel
> deciding the naming scheme should change. Name tweaks are really policy,
> knowing if an interface vid/did say its P2P or net is merely information
> that can guide a policy in user space.
>
> Now if your user space uses that flag to issue ifrenames fine, that's
> your choice. It's probably sufficient just to set IFF_POINTOPOINT
> appropriately on the net interface flags to make all that work ?
The problem is "[my] user space" in this case is generic Ubuntu
basically. It'd be a new thing to that, that it had to understand it
woke up on specifically a Panda and do Panda quirks like clear
IFF_POINTTOPOINT on a magic interface. From a userland perspective, I
would fairly I think term it "stinking it up" with board-specific
quirks, so far generic distro userlands are AFAIK able to avoid that
whole class of hack. I mean there's nothing in Fedora that snoops
/proc/cpuinfo to see what it is running on and then do some special
scripts at init AFAIK, that is in fact what you are suggesting unless I
badly misunderstand you.
The issue is Panda has smsc95xx USB <-> Ethernet bridge chip soldered on
the board wired up to a fixed USB Host port. This is indistinguishable
from an smsc95xx you can buy in a pluggable USB / Ethernet dongle, and
since there's no MAC address EEPROM on the board it could use, to the
point they can report the same vid:pid; but you'd expect the dongle to
come as usb%d as udev would already have it perfectly correctly. So
vid:pid alone is not a basis for distinguishing what to do here.
That is why I characterize the issue as being one of board-specific
physical knowledge that should live in the kernel's board definition
file (Arnd would characterize it similarly but involving Device Tree,
but the same basic deal). The board definition file knows, not that all
device of that vid:pid should choose eth%d not usb%d which is not its
business to know, but that the usb device on the specific wired-up USB
port to the RJ45 soldered on the board should be guided a particular way
when the interface name is registered. That can only be done properly
in-kernel by passing in a flag to usbnet specific to the device instance
to guide its choice.
-Andy
> business to know, but that the usb device on the specific wired-up USB
> port to the RJ45 soldered on the board should be guided a particular way
> when the interface name is registered. That can only be done properly
> in-kernel by passing in a flag to usbnet specific to the device instance
> to guide its choice.
To my mind the MAC case is a bit different to the name case. It's really
not about policy in that situation it's a USB widget with a non standard
"retrieve mac address" method.
You could fix it up in user space from firmware tables, or in either from
looking in your device tree.
There is one way you could actually hide all this in your board specific
gunge at the moment without touching the core kernel I think. Take a look
at how register_netdev_notifier() works.
That will give you a NETDEV_REGISTER event for each device being
registered so you can then pick out your usb network device.
It might need some slightly unclean knowledge of the usb net driver in
your glue but in the short term until there is a nice device tree type
answer it would allow you to get your MAC address set up properly I
think ?
Alan
On 03/23/2011 11:34 AM, Somebody in the thread at some point said:
>> business to know, but that the usb device on the specific wired-up USB
>> port to the RJ45 soldered on the board should be guided a particular way
>> when the interface name is registered. That can only be done properly
>> in-kernel by passing in a flag to usbnet specific to the device instance
>> to guide its choice.
>
> To my mind the MAC case is a bit different to the name case. It's really
> not about policy in that situation it's a USB widget with a non standard
> "retrieve mac address" method.
>
> You could fix it up in user space from firmware tables, or in either from
> looking in your device tree.
>
> There is one way you could actually hide all this in your board specific
> gunge at the moment without touching the core kernel I think. Take a look
> at how register_netdev_notifier() works.
>
> That will give you a NETDEV_REGISTER event for each device being
> registered so you can then pick out your usb network device.
>
> It might need some slightly unclean knowledge of the usb net driver in
> your glue but in the short term until there is a nice device tree type
> answer it would allow you to get your MAC address set up properly I
> think ?
Sounds like a good way... it's register_netdevice_notifier().
I'll study this and try adapt the patches accordingly.
Thanks a lot for this hint.
-Andy
On Wed, 23 Mar 2011, Alan Cox wrote:
> Much of this depends upon what the data is and what it changes. Things
> like interface names for example shouldn't be kernel mangled on the whim
> of random board vendors. They follow a unified behaviour *across*
> platforms, which is what matters most in the bigger picture.
And that's exactly what we're trying to restore on the Panda. It
happens that the Panda has an ethernet connector backed by an USB to
Ethernet bridge, but the kernel currently decides to rename its network
device from eth%d to usb%d, and assign it a random MAC address on each
boot. What we want _is_ a more unified behaviour with other platforms.
> Other stuff is to all intents and purposes sometimes hard coded into
> drivers because while they are USB drivers they are checking specific
> vendor id fields and changing behaviour on them. In some cases those
> vendor id fields are actually a specific hardwired device.
Here we're talking about a plain standard chipset but which happens in
this case to be soldered directly on the board without the usual EEPROM
which would have been used to distinguish this particular case
otherwise. All we have left is the board specific code where the kernel
knows on which board it is dealing with, from where we can tell that one
particular instance of that USB device should be used in some unexpected
way in order to present user space with an unified behaviour with other
similar platforms.
Nicolas
On Wed, Mar 23, 2011 at 10:53:35AM +0000, Mark Brown wrote:
> In the specific case of MACs and device names for network adaptors we
> have userspace solutions which are obscuring the discussion but there
> are other things which get configured this way which one would usually
> expect to be handled in kernel.
No, this isn't obsucuring the discussion, it's exactly the point here.
I asked for concrete examples of a need for this type of thing (i.e.
platform data on USB devices), and this was the only need cited. I
then pointed out that this is correctly solved in userspace, as it has
for other devices like this, and that it's not a valid example of this
need.
So again, this problem, for this device, has been solved in userspace
without any kernel changes needed. If there are other devices that you,
or anyone else, feels needs platform data in a USB device, please point
them out to me, and I will be glad to address them in a case-by-case
basis as done here.
thanks,
greg k-h
On Wed, Mar 23, 2011 at 08:04:13AM -0700, Greg KH wrote:
> I asked for concrete examples of a need for this type of thing (i.e.
> platform data on USB devices), and this was the only need cited. I
No, it isn't. I previously mentioned the example of a system where the
ethernet controller was wired to several PHYs none of which were
connected to the data lines. The driver needed to be told to use
special PHY detection logic to work on this system. In general anything
which may be configured via the SEPROM may need overriding from software
on a system without the SEPROM - consulting the datasheet for a USB
ethernet controller should give an idea of the things that might need to
be configured.
On Wed, 23 Mar 2011, Benjamin Herrenschmidt wrote:
> Among ideas that have been proposed have been the idea of having stubs
> generating pseudo-device nodes from platform_data for devices in order
> to enable the drivers to use a single interface for example. That didn't
> really go through tho. However, whatever the final approach will be, the
> point remains that for drivers that have not so far been "contaminated"
> by the platform_data paradigm, we have the opportunity to try something
> better, which can then, maybe, be retro-fitted on a case by case basis
> to platform drivers, for example as such drivers get converted to also
> be capable of using the device-tree.
>
> This is where I'm trying to propose a middle ground with the idea of
> named properties. It goes toward the direction that platform_device has
> been trying to to with the struct resource, but there's only so much you
> can do with these and they are showing their limits when it comes to
> expose arbitrary informations. MAC addresses are an example but there
> are many more, from clock bindings, power control, PHY data, connector
> informations, yadadada... which all can potentially apply to on-board
> USB or PCI based devices.
I think your middle ground proposal is more than that. Conceptually, it
positions DT as one of the possible device configuration backends
instead of making DT the sanctioned configuration frontend. To me this
is very important as I don't want to be stuck with DT support where it
would simply be an unwanted burden and overhead. By having this generic
abstraction in the middle, it is then possible to decouple DT from the
actual drivers, and then use DT because it makes sense and not just
because there is no way around it. And who knows when some other non-DT
configuration scheme may appear in the industry where the simple
addition of another backend would make things more straight forward than
having to construct synthetic FTDs on the fly.
Nicolas
On 03/23/2011 03:04 PM, Somebody in the thread at some point said:
> On Wed, Mar 23, 2011 at 10:53:35AM +0000, Mark Brown wrote:
>> In the specific case of MACs and device names for network adaptors we
>> have userspace solutions which are obscuring the discussion but there
>> are other things which get configured this way which one would usually
>> expect to be handled in kernel.
>
> No, this isn't obsucuring the discussion, it's exactly the point here.
>
> I asked for concrete examples of a need for this type of thing (i.e.
> platform data on USB devices), and this was the only need cited. I
> then pointed out that this is correctly solved in userspace, as it has
> for other devices like this, and that it's not a valid example of this
> need.
>
> So again, this problem, for this device, has been solved in userspace
> without any kernel changes needed. If there are other devices that you,
Not sure what you mean by 'again', 'solved in userspace' and so on.
Your saying: "go away and do it in userspace" is different from it being
"solved in userspace" in reality. The mess it would create in userspace
isn't acceptable. The outcome of this is it's not fixed in the driver
nor userspace.
I am currently trying working around it elsewhere in kernel along the
lines of Alan's idea, since I am not able to fix the actual driver.
-Andy
On Wed, Mar 23, 2011 at 11:32:02AM +0100, Arnd Bergmann wrote:
> On Wednesday 23 March 2011, Andy Green wrote:
> > It's the case for even usbnet, which is using a broken heuristic to
> > decide what to call the interface not even based on vid / pid.
>
> I agree that the heuristics in usbnet is less than helpful here,
> which among other things leads people to mixing up the two problems
> of fixing the device naming and fixing the MAC address assignment.
>
> Even though I know Greg disagrees, I'd still prefer sanitising the
> rules for the default device name that usbnet assigns.
I'm all for that, but recall, we can't because of previous
implementations and past history. I.e. you will break working systems
if you do this. So we are stuck with what we have at the moment, sorry.
thanks,
greg k-h
On Wed, 23 Mar 2011, Greg KH wrote:
> On Wed, Mar 23, 2011 at 10:53:35AM +0000, Mark Brown wrote:
> > In the specific case of MACs and device names for network adaptors we
> > have userspace solutions which are obscuring the discussion but there
> > are other things which get configured this way which one would usually
> > expect to be handled in kernel.
>
> No, this isn't obsucuring the discussion, it's exactly the point here.
>
> I asked for concrete examples of a need for this type of thing (i.e.
> platform data on USB devices), and this was the only need cited. I
> then pointed out that this is correctly solved in userspace, as it has
> for other devices like this, and that it's not a valid example of this
> need.
It was also pointed out that, unlike those examples for which the user
space solution has been developed to deal with other devices not exactly
like this, that in this case determining the specific instance of the
device that require a quirk is extremely cumbersome, hackish, fragile,
and way more ugly and complex to maintain than some in-kernel solution
where the actual knowledge of the specific board and device instance is
obvious. Again we're not after some particular customization for this
particular board, but rather after a way to make the kernel behavior
uniform across similar platforms. From that point people are still free
to use udev to apply customizations over the kernel default as they see
fit. It is that kernel default that we want to get.
> So again, this problem, for this device, has been solved in userspace
> without any kernel changes needed.
No it has not. We are not going to convince every distributions to
start accumulating complex and fragile user space hacks into their udev
rules which would be justifiably seen as unneeded cruft for exceptional
cases when the kernel can make things look regular and uniform to user
space with much less code and infrastructure. And so far it seems that
everyone else agrees with dealing with such issues in the kernel, while
the actual implementation is still debated.
Nicolas
On Wednesday 23 March 2011, Andy Green wrote:
> > So again, this problem, for this device, has been solved in userspace
> > without any kernel changes needed. If there are other devices that you,
>
> Not sure what you mean by 'again', 'solved in userspace' and so on.
> Your saying: "go away and do it in userspace" is different from it being
> "solved in userspace" in reality. The mess it would create in userspace
> isn't acceptable. The outcome of this is it's not fixed in the driver
> nor userspace.
I agree, doing the device naming in user space to solve this problem would
require identifying the board with the particular bug in a table that
is kept by the distro, and we currently don't even have a way to identify
a specific board. I certainly wouldn't want to rely on things like the
recently proposed /sys/devices/socinfo for identifying whether or not we
want to rename a USB ethernet device.
Arnd
On Wednesday 23 March 2011, Greg KH wrote:
> On Wed, Mar 23, 2011 at 11:32:02AM +0100, Arnd Bergmann wrote:
> > On Wednesday 23 March 2011, Andy Green wrote:
> > > It's the case for even usbnet, which is using a broken heuristic to
> > > decide what to call the interface not even based on vid / pid.
> >
> > I agree that the heuristics in usbnet is less than helpful here,
> > which among other things leads people to mixing up the two problems
> > of fixing the device naming and fixing the MAC address assignment.
> >
> > Even though I know Greg disagrees, I'd still prefer sanitising the
> > rules for the default device name that usbnet assigns.
>
> I'm all for that, but recall, we can't because of previous
> implementations and past history. I.e. you will break working systems
> if you do this. So we are stuck with what we have at the moment, sorry.
I would still consider it a bug fix and argue that the number of actual
people impacted by the current behaviour is much larger than the potential
number people that might suffer from changing the behavior in an incompatible
way, given that:
* The common case for this hardware seems to be to ship with an eeprom
with a valid mac address, as identified by the fact that f4e8ab7c
"smsc95xx: generate random MAC address once, not every ifup" only recently
got into the kernel as a bug fix for the random case, though the driver
has been around since 2008.
* A google search on the term "smsc95xx" find almost exclusively discussions
about how to get the device to come up with a proper name on the panda
board, and various proposed workarounds.
* The comment in usbnet.c about this suggests that the intent has always
been what is proposed now, it just doesn't match the code:
// heuristic: "usb%d" for links we know are two-host,
// else "eth%d" when there's reasonable doubt. userspace
// can rename the link if it knows better.
if ((dev->driver_info->flags & FLAG_ETHER) != 0 &&
(net->dev_addr [0] & 0x02) == 0)
strcpy (net->name, "eth%d");
In case of smsc95xx, we *know* that it's ethernet and not two-host,
there is no reasonable doubt about this. The code almost makes sense
on generic cdc devices, but not for anything else.
Arnd
On Wed, Mar 23, 2011 at 05:12:05PM +0100, Arnd Bergmann wrote:
> On Wednesday 23 March 2011, Greg KH wrote:
> > On Wed, Mar 23, 2011 at 11:32:02AM +0100, Arnd Bergmann wrote:
> > > On Wednesday 23 March 2011, Andy Green wrote:
> > > > It's the case for even usbnet, which is using a broken heuristic to
> > > > decide what to call the interface not even based on vid / pid.
> > >
> > > I agree that the heuristics in usbnet is less than helpful here,
> > > which among other things leads people to mixing up the two problems
> > > of fixing the device naming and fixing the MAC address assignment.
> > >
> > > Even though I know Greg disagrees, I'd still prefer sanitising the
> > > rules for the default device name that usbnet assigns.
> >
> > I'm all for that, but recall, we can't because of previous
> > implementations and past history. I.e. you will break working systems
> > if you do this. So we are stuck with what we have at the moment, sorry.
>
> I would still consider it a bug fix and argue that the number of actual
> people impacted by the current behaviour is much larger than the potential
> number people that might suffer from changing the behavior in an incompatible
> way, given that:
>
> * The common case for this hardware seems to be to ship with an eeprom
> with a valid mac address, as identified by the fact that f4e8ab7c
> "smsc95xx: generate random MAC address once, not every ifup" only recently
> got into the kernel as a bug fix for the random case, though the driver
> has been around since 2008.
>
> * A google search on the term "smsc95xx" find almost exclusively discussions
> about how to get the device to come up with a proper name on the panda
> board, and various proposed workarounds.
>
> * The comment in usbnet.c about this suggests that the intent has always
> been what is proposed now, it just doesn't match the code:
>
> // heuristic: "usb%d" for links we know are two-host,
> // else "eth%d" when there's reasonable doubt. userspace
> // can rename the link if it knows better.
> if ((dev->driver_info->flags & FLAG_ETHER) != 0 &&
> (net->dev_addr [0] & 0x02) == 0)
> strcpy (net->name, "eth%d");
>
> In case of smsc95xx, we *know* that it's ethernet and not two-host,
> there is no reasonable doubt about this. The code almost makes sense
> on generic cdc devices, but not for anything else.
Ok, let's fix it up and see who complains :)
thanks,
greg k-h
On 03/23/2011 04:22 PM, Somebody in the thread at some point said:
>> In case of smsc95xx, we *know* that it's ethernet and not two-host,
>> there is no reasonable doubt about this. The code almost makes sense
>> on generic cdc devices, but not for anything else.
>
> Ok, let's fix it up and see who complains :)
OK... what approach are you alright with being used to signal the fix up
selection of interface name in usbnet? Flag set in async platform_data
from board definition file as in the existing tested patches is OK?
-Andy
The documentation for the USB ethernet devices suggests that
only some devices are supposed to use usb0 as the network interface
name instead of eth0. The logic used there, and documented in
Kconfig for CDC is that eth0 will be used when the mac address
is a globally assigned one, but usb0 is used for the locally
managed range that is typically used on point-to-point links.
Unfortunately, this has caused a lot of pain on the smsc95xx
device that is used on the popular pandaboard without an
EEPROM to store the MAC address, which causes the driver to
call random_ether_address().
Obviously, there should be a proper MAC addressed assigned to
the device, and discussions are ongoing about how to solve
this, but this patch at least makes sure that the default
interface naming gets a little saner and matches what the
user can expect based on the documentation, including for
new devices.
The approach taken here is to flag whether a device might be a
point-to-point link with the new FLAG_PTP setting in the usbnet
driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
it is not sure (e.g. cdc_ether), or just one of the two.
The usbnet framework only looks at the MAC address for device
naming if both flags are set, otherwise it trusts the flag.
Signed-off-by: Arnd Bergmann <[email protected]>
Cc: Andy Green <[email protected]>
---
drivers/net/usb/cdc_eem.c | 2 +-
drivers/net/usb/cdc_ether.c | 2 +-
drivers/net/usb/cdc_ncm.c | 2 +-
drivers/net/usb/cdc_subset.c | 8 ++++++++
drivers/net/usb/gl620a.c | 2 +-
drivers/net/usb/net1080.c | 2 +-
drivers/net/usb/plusb.c | 2 +-
drivers/net/usb/rndis_host.c | 2 +-
drivers/net/usb/smsc75xx.c | 2 +-
drivers/net/usb/smsc95xx.c | 2 +-
drivers/net/usb/usbnet.c | 3 ++-
drivers/net/usb/zaurus.c | 8 ++++----
drivers/usb/serial/usb_wwan.c | 2 +-
include/linux/usb/usbnet.h | 2 ++
Not tested yet, could someone try this out on a panda board
and ideally on a CDC device as well?
diff --git a/drivers/net/usb/cdc_eem.c b/drivers/net/usb/cdc_eem.c
index 5f3b976..4d6bcb8 100644
--- a/drivers/net/usb/cdc_eem.c
+++ b/drivers/net/usb/cdc_eem.c
@@ -340,7 +340,7 @@ next:
static const struct driver_info eem_info = {
.description = "CDC EEM Device",
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_PTP,
.bind = eem_bind,
.rx_fixup = eem_rx_fixup,
.tx_fixup = eem_tx_fixup,
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 9a60e41..6dc89d0 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -450,7 +450,7 @@ static int cdc_manage_power(struct usbnet *dev, int on)
static const struct driver_info cdc_info = {
.description = "CDC Ethernet Device",
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_PTP,
// .check_connect = cdc_check_connect,
.bind = cdc_bind,
.unbind = usbnet_cdc_unbind,
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 7113168..a6a026a 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1237,7 +1237,7 @@ static int cdc_ncm_manage_power(struct usbnet *dev, int status)
static const struct driver_info cdc_ncm_info = {
.description = "CDC NCM",
- .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET,
+ .flags = FLAG_PTP | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.check_connect = cdc_ncm_check_connect,
diff --git a/drivers/net/usb/cdc_subset.c b/drivers/net/usb/cdc_subset.c
index ca39ace..fbe2b1b 100644
--- a/drivers/net/usb/cdc_subset.c
+++ b/drivers/net/usb/cdc_subset.c
@@ -89,6 +89,7 @@ static int always_connected (struct usbnet *dev)
static const struct driver_info ali_m5632_info = {
.description = "ALi M5632",
+ .flags = FLAG_PTP,
};
#endif
@@ -110,6 +111,7 @@ static const struct driver_info ali_m5632_info = {
static const struct driver_info an2720_info = {
.description = "AnchorChips/Cypress 2720",
+ .flags = FLAG_PTP,
// no reset available!
// no check_connect available!
@@ -132,6 +134,7 @@ static const struct driver_info an2720_info = {
static const struct driver_info belkin_info = {
.description = "Belkin, eTEK, or compatible",
+ .flags = FLAG_PTP,
};
#endif /* CONFIG_USB_BELKIN */
@@ -157,6 +160,7 @@ static const struct driver_info belkin_info = {
static const struct driver_info epson2888_info = {
.description = "Epson USB Device",
.check_connect = always_connected,
+ .flags = FLAG_PTP,
.in = 4, .out = 3,
};
@@ -173,6 +177,7 @@ static const struct driver_info epson2888_info = {
#define HAVE_HARDWARE
static const struct driver_info kc2190_info = {
.description = "KC Technology KC-190",
+ .flags = FLAG_PTP,
};
#endif /* CONFIG_USB_KC2190 */
@@ -200,16 +205,19 @@ static const struct driver_info kc2190_info = {
static const struct driver_info linuxdev_info = {
.description = "Linux Device",
.check_connect = always_connected,
+ .flags = FLAG_PTP,
};
static const struct driver_info yopy_info = {
.description = "Yopy",
.check_connect = always_connected,
+ .flags = FLAG_PTP,
};
static const struct driver_info blob_info = {
.description = "Boot Loader OBject",
.check_connect = always_connected,
+ .flags = FLAG_PTP,
};
#endif /* CONFIG_USB_ARMLINUX */
diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
index dcd57c3..972ee83 100644
--- a/drivers/net/usb/gl620a.c
+++ b/drivers/net/usb/gl620a.c
@@ -193,7 +193,7 @@ static int genelink_bind(struct usbnet *dev, struct usb_interface *intf)
static const struct driver_info genelink_info = {
.description = "Genesys GeneLink",
- .flags = FLAG_FRAMING_GL | FLAG_NO_SETINT,
+ .flags = FLAG_PTP | FLAG_FRAMING_GL | FLAG_NO_SETINT,
.bind = genelink_bind,
.rx_fixup = genelink_rx_fixup,
.tx_fixup = genelink_tx_fixup,
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index ba72a72..f8a294e 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -560,7 +560,7 @@ static int net1080_bind(struct usbnet *dev, struct usb_interface *intf)
static const struct driver_info net1080_info = {
.description = "NetChip TurboCONNECT",
- .flags = FLAG_FRAMING_NC,
+ .flags = FLAG_PTP | FLAG_FRAMING_NC,
.bind = net1080_bind,
.reset = net1080_reset,
.check_connect = net1080_check_connect,
diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c
index 08ad269..0ac7845 100644
--- a/drivers/net/usb/plusb.c
+++ b/drivers/net/usb/plusb.c
@@ -96,7 +96,7 @@ static int pl_reset(struct usbnet *dev)
static const struct driver_info prolific_info = {
.description = "Prolific PL-2301/PL-2302",
- .flags = FLAG_NO_SETINT,
+ .flags = FLAG_PTP | FLAG_NO_SETINT,
/* some PL-2302 versions seem to fail usb_set_interface() */
.reset = pl_reset,
};
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index dd8a4ad..bf2ab6e 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -573,7 +573,7 @@ EXPORT_SYMBOL_GPL(rndis_tx_fixup);
static const struct driver_info rndis_info = {
.description = "RNDIS device",
- .flags = FLAG_ETHER | FLAG_FRAMING_RN | FLAG_NO_SETINT,
+ .flags = FLAG_ETHER | FLAG_PTP | FLAG_FRAMING_RN | FLAG_NO_SETINT,
.bind = rndis_bind,
.unbind = rndis_unbind,
.status = rndis_status,
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 753ee6e..404a475 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -1244,7 +1244,7 @@ static const struct driver_info smsc75xx_info = {
.rx_fixup = smsc75xx_rx_fixup,
.tx_fixup = smsc75xx_tx_fixup,
.status = smsc75xx_status,
- .flags = FLAG_ETHER | FLAG_SEND_ZLP,
+ .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
};
static const struct usb_device_id products[] = {
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index bc86f4b..c98d3a7 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1231,7 +1231,7 @@ static const struct driver_info smsc95xx_info = {
.rx_fixup = smsc95xx_rx_fixup,
.tx_fixup = smsc95xx_tx_fixup,
.status = smsc95xx_status,
- .flags = FLAG_ETHER | FLAG_SEND_ZLP,
+ .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
};
static const struct usb_device_id products[] = {
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 95c41d5..b339d3f 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1376,7 +1376,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
// else "eth%d" when there's reasonable doubt. userspace
// can rename the link if it knows better.
if ((dev->driver_info->flags & FLAG_ETHER) != 0 &&
- (net->dev_addr [0] & 0x02) == 0)
+ ((dev->driver_info->flags & FLAG_PTP) == 0 ||
+ (net->dev_addr [0] & 0x02) == 0))
strcpy (net->name, "eth%d");
/* WLAN devices should always be named "wlan%d" */
if ((dev->driver_info->flags & FLAG_WLAN) != 0)
diff --git a/drivers/net/usb/zaurus.c b/drivers/net/usb/zaurus.c
index 3eb0b16..e906700 100644
--- a/drivers/net/usb/zaurus.c
+++ b/drivers/net/usb/zaurus.c
@@ -102,7 +102,7 @@ static int always_connected (struct usbnet *dev)
static const struct driver_info zaurus_sl5x00_info = {
.description = "Sharp Zaurus SL-5x00",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_PTP | FLAG_FRAMING_Z,
.check_connect = always_connected,
.bind = zaurus_bind,
.unbind = usbnet_cdc_unbind,
@@ -112,7 +112,7 @@ static const struct driver_info zaurus_sl5x00_info = {
static const struct driver_info zaurus_pxa_info = {
.description = "Sharp Zaurus, PXA-2xx based",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_PTP | FLAG_FRAMING_Z,
.check_connect = always_connected,
.bind = zaurus_bind,
.unbind = usbnet_cdc_unbind,
@@ -122,7 +122,7 @@ static const struct driver_info zaurus_pxa_info = {
static const struct driver_info olympus_mxl_info = {
.description = "Olympus R1000",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_PTP | FLAG_FRAMING_Z,
.check_connect = always_connected,
.bind = zaurus_bind,
.unbind = usbnet_cdc_unbind,
@@ -258,7 +258,7 @@ bad_desc:
static const struct driver_info bogus_mdlm_info = {
.description = "pseudo-MDLM (BLAN) device",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_PTP | FLAG_FRAMING_Z,
.check_connect = always_connected,
.tx_fixup = zaurus_tx_fixup,
.bind = blan_mdlm_bind,
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 44842c8..5ecd8e7 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -97,6 +97,8 @@ struct driver_info {
#define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
+#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
+
/*
* Indicates to usbnet, that USB driver accumulates multiple IP packets.
* Affects statistic (counters) and short packet handling.
On 03/23/2011 04:56 PM, Somebody in the thread at some point said:
> The approach taken here is to flag whether a device might be a
> point-to-point link with the new FLAG_PTP setting in the usbnet
> driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
> it is not sure (e.g. cdc_ether), or just one of the two.
> The usbnet framework only looks at the MAC address for device
> naming if both flags are set, otherwise it trusts the flag.
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index bc86f4b..c98d3a7 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -1231,7 +1231,7 @@ static const struct driver_info smsc95xx_info = {
> - .flags = FLAG_ETHER | FLAG_SEND_ZLP,
> + .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
> if ((dev->driver_info->flags& FLAG_ETHER) != 0&&
> + ((dev->driver_info->flags& FLAG_PTP) == 0 ||
> + (net->dev_addr [0]& 0x02) == 0))
> strcpy (net->name, "eth%d");
So it just takes the approach that all smsc95xx are going to be eth%d?
Sounds good to me.
-Andy
On Wednesday 23 March 2011, Andy Green wrote:
> > diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> > index bc86f4b..c98d3a7 100644
> > --- a/drivers/net/usb/smsc95xx.c
> > +++ b/drivers/net/usb/smsc95xx.c
> > @@ -1231,7 +1231,7 @@ static const struct driver_info smsc95xx_info = {
>
> > - .flags = FLAG_ETHER | FLAG_SEND_ZLP,
> > + .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
>
> > if ((dev->driver_info->flags& FLAG_ETHER) != 0&&
> > + ((dev->driver_info->flags& FLAG_PTP) == 0 ||
> > + (net->dev_addr [0]& 0x02) == 0))
> > strcpy (net->name, "eth%d");
>
> So it just takes the approach that all smsc95xx are going to be eth%d?
Right, and all other drivers that are obviously ethernet-only, including
future drivers.
Arnd
On Wednesday 23 March 2011, Arnd Bergmann wrote:
> --- a/drivers/net/usb/smsc75xx.c
> +++ b/drivers/net/usb/smsc75xx.c
> @@ -1244,7 +1244,7 @@ static const struct driver_info smsc75xx_info = {
> .rx_fixup = smsc75xx_rx_fixup,
> .tx_fixup = smsc75xx_tx_fixup,
> .status = smsc75xx_status,
> - .flags = FLAG_ETHER | FLAG_SEND_ZLP,
> + .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
> };
>
> static const struct usb_device_id products[] = {
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index bc86f4b..c98d3a7 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -1231,7 +1231,7 @@ static const struct driver_info smsc95xx_info = {
> .rx_fixup = smsc95xx_rx_fixup,
> .tx_fixup = smsc95xx_tx_fixup,
> .status = smsc95xx_status,
> - .flags = FLAG_ETHER | FLAG_SEND_ZLP,
> + .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
> };
>
These two hunks are from an earlier version of the patch and should not
be there, they break compilation. I'll wait for more comments and
then send a fixed version.
Arnd
On 03/23/2011 12:13 PM, Arnd Bergmann wrote:
> On Wednesday 23 March 2011, Arnd Bergmann wrote:
>
>> --- a/drivers/net/usb/smsc75xx.c
>> +++ b/drivers/net/usb/smsc75xx.c
>> @@ -1244,7 +1244,7 @@ static const struct driver_info smsc75xx_info = {
>> .rx_fixup = smsc75xx_rx_fixup,
>> .tx_fixup = smsc75xx_tx_fixup,
>> .status = smsc75xx_status,
>> - .flags = FLAG_ETHER | FLAG_SEND_ZLP,
>> + .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
>> };
>>
>> static const struct usb_device_id products[] = {
>> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
>> index bc86f4b..c98d3a7 100644
>> --- a/drivers/net/usb/smsc95xx.c
>> +++ b/drivers/net/usb/smsc95xx.c
>> @@ -1231,7 +1231,7 @@ static const struct driver_info smsc95xx_info = {
>> .rx_fixup = smsc95xx_rx_fixup,
>> .tx_fixup = smsc95xx_tx_fixup,
>> .status = smsc95xx_status,
>> - .flags = FLAG_ETHER | FLAG_SEND_ZLP,
>> + .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
>> };
>>
>>
> These two hunks are from an earlier version of the patch and should not
> be there, they break compilation. I'll wait for more comments and
> then send a fixed version.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
tested with 2.6.38 using omap2plus_defconfig with EHCI and smsc95xx
turned on minus the FLAG_REALLY_ETHER. appears to work properly, assigns
as eth0.
i ran some basic net tests with no issues.
Dave Anders
On Wed, Mar 23, 2011 at 05:56:39PM +0100, Arnd Bergmann wrote:
> The documentation for the USB ethernet devices suggests that
> only some devices are supposed to use usb0 as the network interface
> name instead of eth0. The logic used there, and documented in
> Kconfig for CDC is that eth0 will be used when the mac address
> is a globally assigned one, but usb0 is used for the locally
> managed range that is typically used on point-to-point links.
>
> Unfortunately, this has caused a lot of pain on the smsc95xx
> device that is used on the popular pandaboard without an
> EEPROM to store the MAC address, which causes the driver to
> call random_ether_address().
>
> Obviously, there should be a proper MAC addressed assigned to
> the device, and discussions are ongoing about how to solve
> this, but this patch at least makes sure that the default
> interface naming gets a little saner and matches what the
> user can expect based on the documentation, including for
> new devices.
>
> The approach taken here is to flag whether a device might be a
> point-to-point link with the new FLAG_PTP setting in the usbnet
> driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
> it is not sure (e.g. cdc_ether), or just one of the two.
> The usbnet framework only looks at the MAC address for device
> naming if both flags are set, otherwise it trusts the flag.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Cc: Andy Green <[email protected]>
Looks good to me, but some questions:
> drivers/usb/serial/usb_wwan.c | 2 +-
You don't modify this file in the diff, what caused this to show up in
the diffstat?
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -97,6 +97,8 @@ struct driver_info {
>
> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
>
> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
"PTP"? What does that stand for?
curious,
greg k-h
On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
> Looks good to me, but some questions:
>
> > drivers/usb/serial/usb_wwan.c | 2 +-
>
> You don't modify this file in the diff, what caused this to show up in
> the diffstat?
A stale change I had to do to get the kernel to build on my
machine. The original problem seems fixed now. I had removed
this hunk from the patch but forgot to remove it from the diffstat.
> > --- a/include/linux/usb/usbnet.h
> > +++ b/include/linux/usb/usbnet.h
> > @@ -97,6 +97,8 @@ struct driver_info {
> >
> > #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
> >
> > +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
>
> "PTP"? What does that stand for?
point-to-point, I'll improve the comment to spell it out when I send the
fixed version.
Arnd
On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
> On Mar 23, 2011 8:36 PM, "Arnd Bergmann" <[email protected]> wrote:
> >
> > On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
> > > > @@ -97,6 +97,8 @@ struct driver_info {
> > > >
> > > > #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
> > > >
> > > > +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
> > >
> > > "PTP"? What does that stand for?
> >
> > point-to-point, I'll improve the comment to spell it out when I send the
> > fixed version.
>
> I think P2P could be better.
Yes, good idea.
Arnd
On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
> > On Mar 23, 2011 8:36 PM, "Arnd Bergmann" <[email protected]> wrote:
> > >
> > > On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
> > > > > @@ -97,6 +97,8 @@ struct driver_info {
> > > > >
> > > > > #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
> > > > >
> > > > > +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
> > > >
> > > > "PTP"? What does that stand for?
> > >
> > > point-to-point, I'll improve the comment to spell it out when I send the
> > > fixed version.
> >
> > I think P2P could be better.
>
> Yes, good idea.
that's peer-to-peer.
OTOH, I knew that PTP was point-to-point.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
>>>>>> @@ -97,6 +97,8 @@ struct driver_info {
>>>>>>
>>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
>>>>>>
>>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
>>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
>>>>> "PTP"? What does that stand for?
>>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann" <[email protected]> wrote:
>>>> point-to-point, I'll improve the comment to spell it out when
>>>> I send the fixed version.
>> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
>>> I think P2P could be better.
> On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
>> Yes, good idea.
Randy Dunlap <[email protected]> writes:
> that's peer-to-peer.
>
> OTOH, I knew that PTP was point-to-point.
It can be any of that, depending on context. For me PTP is more like
Picture Transport Protocol, whereas "2" between two letters is usually
"to".
Just my two cents though, no strong feelings or anything.
--
Best regards, _ _
.o. | Liege of Serenly Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-<mina86-mina86.com>-<jid:mina86-jabber.org>--ooO--(_)--Ooo--
On Thu, 24 Mar 2011 00:17:34 +0100 Michal Nazarewicz wrote:
> >>>>>> @@ -97,6 +97,8 @@ struct driver_info {
> >>>>>>
> >>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
> >>>>>>
> >>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
>
> >>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
> >>>>> "PTP"? What does that stand for?
>
> >>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann" <[email protected]> wrote:
> >>>> point-to-point, I'll improve the comment to spell it out when
> >>>> I send the fixed version.
>
> >> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
> >>> I think P2P could be better.
>
> > On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
> >> Yes, good idea.
>
> Randy Dunlap <[email protected]> writes:
> > that's peer-to-peer.
> >
> > OTOH, I knew that PTP was point-to-point.
>
> It can be any of that, depending on context. For me PTP is more like
> Picture Transport Protocol, whereas "2" between two letters is usually
> "to".
>
> Just my two cents though, no strong feelings or anything.
Agreed.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
On 03/23/11 16:17, Michal Nazarewicz wrote:
>>>>>>> @@ -97,6 +97,8 @@ struct driver_info {
>>>>>>>
>>>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
>>>>>>>
>>>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
>
>>>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
>>>>>> "PTP"? What does that stand for?
>
>>>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann"<[email protected]> wrote:
>>>>> point-to-point, I'll improve the comment to spell it out when
>>>>> I send the fixed version.
>
>>> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
>>>> I think P2P could be better.
>
>> On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
>>> Yes, good idea.
>
> Randy Dunlap<[email protected]> writes:
>> that's peer-to-peer.
>>
>> OTOH, I knew that PTP was point-to-point.
>
> It can be any of that, depending on context. For me PTP is more like
> Picture Transport Protocol, whereas "2" between two letters is usually
> "to".
>
Well, my 2 cents, picture transport protocol is so obviously different
than flags for network interfaces it does not cause a mental collision.
However P2P is about to become a huge wifi issue, which definitely is in
the network space. So don't confuse things in the near future, don't use
P2P.
Maybe a completely different, not so overused flag name would be better.
Regards, Steve
On Wed, 2011-03-23 at 16:38 -0700, Steve Calfee wrote:
> On 03/23/11 16:17, Michal Nazarewicz wrote:
> >>>>>>> @@ -97,6 +97,8 @@ struct driver_info {
> >>>>>>>
> >>>>>>> #define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
> >>>>>>>
> >>>>>>> +#define FLAG_PTP 0x1000 /* maybe use "usb%d" names */
> >
> >>>>> On Wednesday 23 March 2011 19:46:50 Greg KH wrote:
> >>>>>> "PTP"? What does that stand for?
> >
> >>>> On Mar 23, 2011 8:36 PM, "Arnd Bergmann"<[email protected]> wrote:
> >>>>> point-to-point, I'll improve the comment to spell it out when
> >>>>> I send the fixed version.
> >
> >>> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
> >>>> I think P2P could be better.
> >
> >> On Wed, 23 Mar 2011 20:57:18 +0100 Arnd Bergmann wrote:
> >>> Yes, good idea.
> >
> > Randy Dunlap<[email protected]> writes:
> >> that's peer-to-peer.
> >>
> >> OTOH, I knew that PTP was point-to-point.
> >
> > It can be any of that, depending on context. For me PTP is more like
> > Picture Transport Protocol, whereas "2" between two letters is usually
> > "to".
> >
> Well, my 2 cents, picture transport protocol is so obviously different
> than flags for network interfaces it does not cause a mental collision.
PTP is also Precision Time Protocol, which *is* used on network
interfaces (maybe not USB-connected interfaces though).
Anyway, these USB cable interfaces can presumably be bridged to standard
Ethernet, so they are really no more point-to-point than any other
Ethernet link has been since the demise of the hub.
> However P2P is about to become a huge wifi issue, which definitely is in
> the network space. So don't confuse things in the near future, don't use
> P2P.
>
> Maybe a completely different, not so overused flag name would be better.
How about FLAG_NON_IEEE, meaning that the physical layer is not based on
an IEEE 802.3, 802.11 or other standard physical layer.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
On 03/23/2011 05:11 PM, Somebody in the thread at some point said:
> On Wednesday 23 March 2011, Andy Green wrote:
>>> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
>>> index bc86f4b..c98d3a7 100644
>>> --- a/drivers/net/usb/smsc95xx.c
>>> +++ b/drivers/net/usb/smsc95xx.c
>>> @@ -1231,7 +1231,7 @@ static const struct driver_info smsc95xx_info = {
>>
>>> - .flags = FLAG_ETHER | FLAG_SEND_ZLP,
>>> + .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_REALLY_ETHER,
>>
>>> if ((dev->driver_info->flags& FLAG_ETHER) != 0&&
>>> + ((dev->driver_info->flags& FLAG_PTP) == 0 ||
>>> + (net->dev_addr [0]& 0x02) == 0))
>>> strcpy (net->name, "eth%d");
>>
>> So it just takes the approach that all smsc95xx are going to be eth%d?
>
> Right, and all other drivers that are obviously ethernet-only, including
> future drivers.
I moved my tree over to using this patch now, it's working fine on Panda
/ smsc95xx as one would expect. Guys with pluggable smsc95xx are going
to find their device comes as eth%d not usb%d but that seems perfectly
defensible as how it always should have been.
So, nice job.
-Andy
On Thursday 24 March 2011, Ben Hutchings wrote:
> On Wed, 2011-03-23 at 16:38 -0700, Steve Calfee wrote:
> > On 03/23/11 16:17, Michal Nazarewicz wrote:
> > >>> On Wednesday 23 March 2011 20:53:13 Michał Nazarewicz wrote:
> > >>>> I think P2P could be better.
> > >>
> > >> OTOH, I knew that PTP was point-to-point.
> > >
> > > It can be any of that, depending on context. For me PTP is more like
> > > Picture Transport Protocol, whereas "2" between two letters is usually
> > > "to".
> > >
> > Well, my 2 cents, picture transport protocol is so obviously different
> > than flags for network interfaces it does not cause a mental collision.
>
> PTP is also Precision Time Protocol, which *is* used on network
> interfaces (maybe not USB-connected interfaces though).
>
> How about FLAG_NON_IEEE, meaning that the physical layer is not based on
> an IEEE 802.3, 802.11 or other standard physical layer.
I think that doesn't really express the meaning, since FLAG_WWAN is
presumably also not IEEE, right?
Thanks for the bike shedding everyone, I'll just use my own color then
and call it FLAG_POINTTOPOINT.
Arnd
The documentation for the USB ethernet devices suggests that
only some devices are supposed to use usb0 as the network interface
name instead of eth0. The logic used there, and documented in
Kconfig for CDC is that eth0 will be used when the mac address
is a globally assigned one, but usb0 is used for the locally
managed range that is typically used on point-to-point links.
Unfortunately, this has caused a lot of pain on the smsc95xx
device that is used on the popular pandaboard without an
EEPROM to store the MAC address, which causes the driver to
call random_ether_address().
Obviously, there should be a proper MAC addressed assigned to
the device, and discussions are ongoing about how to solve
this, but this patch at least makes sure that the default
interface naming gets a little saner and matches what the
user can expect based on the documentation, including for
new devices.
The approach taken here is to flag whether a device might be a
point-to-point link with the new FLAG_PTP setting in the usbnet
driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
it is not sure (e.g. cdc_ether), or just one of the two.
The usbnet framework only looks at the MAC address for device
naming if both flags are set, otherwise it trusts the flag.
Signed-off-by: Arnd Bergmann <[email protected]>
Cc: Andy Green <[email protected]>
Cc: [email protected]
---
drivers/net/usb/cdc_eem.c | 2 +-
drivers/net/usb/cdc_ether.c | 2 +-
drivers/net/usb/cdc_ncm.c | 2 +-
drivers/net/usb/cdc_subset.c | 8 ++++++++
drivers/net/usb/gl620a.c | 2 +-
drivers/net/usb/net1080.c | 2 +-
drivers/net/usb/plusb.c | 2 +-
drivers/net/usb/rndis_host.c | 2 +-
drivers/net/usb/usbnet.c | 3 ++-
drivers/net/usb/zaurus.c | 8 ++++----
include/linux/usb/usbnet.h | 2 ++
11 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/net/usb/cdc_eem.c b/drivers/net/usb/cdc_eem.c
index 5f3b976..8f12854 100644
--- a/drivers/net/usb/cdc_eem.c
+++ b/drivers/net/usb/cdc_eem.c
@@ -340,7 +340,7 @@ next:
static const struct driver_info eem_info = {
.description = "CDC EEM Device",
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_POINTTOPOINT,
.bind = eem_bind,
.rx_fixup = eem_rx_fixup,
.tx_fixup = eem_tx_fixup,
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 9a60e41..98b2bbd 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -450,7 +450,7 @@ static int cdc_manage_power(struct usbnet *dev, int on)
static const struct driver_info cdc_info = {
.description = "CDC Ethernet Device",
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_POINTTOPOINT,
// .check_connect = cdc_check_connect,
.bind = cdc_bind,
.unbind = usbnet_cdc_unbind,
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 7113168..967371f 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1237,7 +1237,7 @@ static int cdc_ncm_manage_power(struct usbnet *dev, int status)
static const struct driver_info cdc_ncm_info = {
.description = "CDC NCM",
- .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET,
+ .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.check_connect = cdc_ncm_check_connect,
diff --git a/drivers/net/usb/cdc_subset.c b/drivers/net/usb/cdc_subset.c
index ca39ace..fc5f13d 100644
--- a/drivers/net/usb/cdc_subset.c
+++ b/drivers/net/usb/cdc_subset.c
@@ -89,6 +89,7 @@ static int always_connected (struct usbnet *dev)
static const struct driver_info ali_m5632_info = {
.description = "ALi M5632",
+ .flags = FLAG_POINTTOPOINT,
};
#endif
@@ -110,6 +111,7 @@ static const struct driver_info ali_m5632_info = {
static const struct driver_info an2720_info = {
.description = "AnchorChips/Cypress 2720",
+ .flags = FLAG_POINTTOPOINT,
// no reset available!
// no check_connect available!
@@ -132,6 +134,7 @@ static const struct driver_info an2720_info = {
static const struct driver_info belkin_info = {
.description = "Belkin, eTEK, or compatible",
+ .flags = FLAG_POINTTOPOINT,
};
#endif /* CONFIG_USB_BELKIN */
@@ -157,6 +160,7 @@ static const struct driver_info belkin_info = {
static const struct driver_info epson2888_info = {
.description = "Epson USB Device",
.check_connect = always_connected,
+ .flags = FLAG_POINTTOPOINT,
.in = 4, .out = 3,
};
@@ -173,6 +177,7 @@ static const struct driver_info epson2888_info = {
#define HAVE_HARDWARE
static const struct driver_info kc2190_info = {
.description = "KC Technology KC-190",
+ .flags = FLAG_POINTTOPOINT,
};
#endif /* CONFIG_USB_KC2190 */
@@ -200,16 +205,19 @@ static const struct driver_info kc2190_info = {
static const struct driver_info linuxdev_info = {
.description = "Linux Device",
.check_connect = always_connected,
+ .flags = FLAG_POINTTOPOINT,
};
static const struct driver_info yopy_info = {
.description = "Yopy",
.check_connect = always_connected,
+ .flags = FLAG_POINTTOPOINT,
};
static const struct driver_info blob_info = {
.description = "Boot Loader OBject",
.check_connect = always_connected,
+ .flags = FLAG_POINTTOPOINT,
};
#endif /* CONFIG_USB_ARMLINUX */
diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
index dcd57c3..c4cfd1d 100644
--- a/drivers/net/usb/gl620a.c
+++ b/drivers/net/usb/gl620a.c
@@ -193,7 +193,7 @@ static int genelink_bind(struct usbnet *dev, struct usb_interface *intf)
static const struct driver_info genelink_info = {
.description = "Genesys GeneLink",
- .flags = FLAG_FRAMING_GL | FLAG_NO_SETINT,
+ .flags = FLAG_POINTTOPOINT | FLAG_FRAMING_GL | FLAG_NO_SETINT,
.bind = genelink_bind,
.rx_fixup = genelink_rx_fixup,
.tx_fixup = genelink_tx_fixup,
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index ba72a72..01db460 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -560,7 +560,7 @@ static int net1080_bind(struct usbnet *dev, struct usb_interface *intf)
static const struct driver_info net1080_info = {
.description = "NetChip TurboCONNECT",
- .flags = FLAG_FRAMING_NC,
+ .flags = FLAG_POINTTOPOINT | FLAG_FRAMING_NC,
.bind = net1080_bind,
.reset = net1080_reset,
.check_connect = net1080_check_connect,
diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c
index 08ad269..823c537 100644
--- a/drivers/net/usb/plusb.c
+++ b/drivers/net/usb/plusb.c
@@ -96,7 +96,7 @@ static int pl_reset(struct usbnet *dev)
static const struct driver_info prolific_info = {
.description = "Prolific PL-2301/PL-2302",
- .flags = FLAG_NO_SETINT,
+ .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT,
/* some PL-2302 versions seem to fail usb_set_interface() */
.reset = pl_reset,
};
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index dd8a4ad..5994a25 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -573,7 +573,7 @@ EXPORT_SYMBOL_GPL(rndis_tx_fixup);
static const struct driver_info rndis_info = {
.description = "RNDIS device",
- .flags = FLAG_ETHER | FLAG_FRAMING_RN | FLAG_NO_SETINT,
+ .flags = FLAG_ETHER | FLAG_POINTTOPOINT | FLAG_FRAMING_RN | FLAG_NO_SETINT,
.bind = rndis_bind,
.unbind = rndis_unbind,
.status = rndis_status,
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 95c41d5..c5b6cfb 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1376,7 +1376,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
// else "eth%d" when there's reasonable doubt. userspace
// can rename the link if it knows better.
if ((dev->driver_info->flags & FLAG_ETHER) != 0 &&
- (net->dev_addr [0] & 0x02) == 0)
+ ((dev->driver_info->flags & FLAG_POINTTOPOINT) == 0 ||
+ (net->dev_addr [0] & 0x02) == 0))
strcpy (net->name, "eth%d");
/* WLAN devices should always be named "wlan%d" */
if ((dev->driver_info->flags & FLAG_WLAN) != 0)
diff --git a/drivers/net/usb/zaurus.c b/drivers/net/usb/zaurus.c
index 3eb0b16..241756e 100644
--- a/drivers/net/usb/zaurus.c
+++ b/drivers/net/usb/zaurus.c
@@ -102,7 +102,7 @@ static int always_connected (struct usbnet *dev)
static const struct driver_info zaurus_sl5x00_info = {
.description = "Sharp Zaurus SL-5x00",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_POINTTOPOINT | FLAG_FRAMING_Z,
.check_connect = always_connected,
.bind = zaurus_bind,
.unbind = usbnet_cdc_unbind,
@@ -112,7 +112,7 @@ static const struct driver_info zaurus_sl5x00_info = {
static const struct driver_info zaurus_pxa_info = {
.description = "Sharp Zaurus, PXA-2xx based",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_POINTTOPOINT | FLAG_FRAMING_Z,
.check_connect = always_connected,
.bind = zaurus_bind,
.unbind = usbnet_cdc_unbind,
@@ -122,7 +122,7 @@ static const struct driver_info zaurus_pxa_info = {
static const struct driver_info olympus_mxl_info = {
.description = "Olympus R1000",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_POINTTOPOINT | FLAG_FRAMING_Z,
.check_connect = always_connected,
.bind = zaurus_bind,
.unbind = usbnet_cdc_unbind,
@@ -258,7 +258,7 @@ bad_desc:
static const struct driver_info bogus_mdlm_info = {
.description = "pseudo-MDLM (BLAN) device",
- .flags = FLAG_FRAMING_Z,
+ .flags = FLAG_POINTTOPOINT | FLAG_FRAMING_Z,
.check_connect = always_connected,
.tx_fixup = zaurus_tx_fixup,
.bind = blan_mdlm_bind,
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 44842c8..1ef9aa0 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -97,6 +97,8 @@ struct driver_info {
#define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
+#define FLAG_POINTTOPOINT 0x1000 /* possibly use "usb%d" names */
+
/*
* Indicates to usbnet, that USB driver accumulates multiple IP packets.
* Affects statistic (counters) and short packet handling.
On 03/24/2011 01:15 PM, Somebody in the thread at some point said:
> The documentation for the USB ethernet devices suggests that
> only some devices are supposed to use usb0 as the network interface
> name instead of eth0. The logic used there, and documented in
> Kconfig for CDC is that eth0 will be used when the mac address
> is a globally assigned one, but usb0 is used for the locally
> managed range that is typically used on point-to-point links.
>
> Unfortunately, this has caused a lot of pain on the smsc95xx
> device that is used on the popular pandaboard without an
> EEPROM to store the MAC address, which causes the driver to
> call random_ether_address().
>
> Obviously, there should be a proper MAC addressed assigned to
> the device, and discussions are ongoing about how to solve
> this, but this patch at least makes sure that the default
> interface naming gets a little saner and matches what the
> user can expect based on the documentation, including for
> new devices.
>
> The approach taken here is to flag whether a device might be a
> point-to-point link with the new FLAG_PTP setting in the usbnet
> driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
> it is not sure (e.g. cdc_ether), or just one of the two.
> The usbnet framework only looks at the MAC address for device
> naming if both flags are set, otherwise it trusts the flag.
>
> Signed-off-by: Arnd Bergmann<[email protected]>
> Cc: Andy Green<[email protected]>
> Cc: [email protected]
For Panda case at least,
Tested-by: Andy Green <[email protected]>
-Andy
On Thu, 24 Mar 2011, Arnd Bergmann wrote:
> The documentation for the USB ethernet devices suggests that
> only some devices are supposed to use usb0 as the network interface
> name instead of eth0. The logic used there, and documented in
> Kconfig for CDC is that eth0 will be used when the mac address
> is a globally assigned one, but usb0 is used for the locally
> managed range that is typically used on point-to-point links.
>
> Unfortunately, this has caused a lot of pain on the smsc95xx
> device that is used on the popular pandaboard without an
> EEPROM to store the MAC address, which causes the driver to
> call random_ether_address().
>
> Obviously, there should be a proper MAC addressed assigned to
> the device, and discussions are ongoing about how to solve
> this, but this patch at least makes sure that the default
> interface naming gets a little saner and matches what the
> user can expect based on the documentation, including for
> new devices.
>
> The approach taken here is to flag whether a device might be a
> point-to-point link with the new FLAG_PTP setting in the usbnet
> driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
You updated the flag name in the patch but not in the description.
> it is not sure (e.g. cdc_ether), or just one of the two.
> The usbnet framework only looks at the MAC address for device
> naming if both flags are set, otherwise it trusts the flag.
Alan Stern
On Thu, Mar 24, 2011 at 2:15 PM, Arnd Bergmann <[email protected]> wrote:
>
> The approach taken here is to flag whether a device might be a
> point-to-point link with the new FLAG_PTP setting in the usbnet
> driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
> it is not sure (e.g. cdc_ether), or just one of the two.
> The usbnet framework only looks at the MAC address for device
> naming if both flags are set, otherwise it trusts the flag.
Should this paragraph above be a clue for the flag name?
Sorry for late comment, but having flag called FLAG_POINTTOPOINT is really
confusing. ptp, p2p terms are heavily used and will mislead folks.
Would it be better to call it something like IGNORE_MAC_ADDRESS if this is the
feature you are targeting?
/Alexey
On Tue, Mar 22, 2011 at 03:12:48PM +0000, Mark Brown wrote:
> On Tue, Mar 22, 2011 at 08:32:28PM +0530, Jaswinder Singh wrote:
>
> > It's a different matter that DT has many hearts to win(at least in ARM
> > Linux)
> > So, perhaps the answer to Q(a) is - Yes.
>
> It's not really a question of hearts to win. Even for people who are
> fully won over the implementation just isn't there in mainline and it's
> not clear when it'll be delivered. Right now device tree can only be
> used on SPARC, PowerPC and (I beleive) some x86 platforms.
MIPS and Microblaze too plus out-of-tree NIOS.
Regardless, I agree 100%. This isn't (er, shouldn't be) a device tree
debate. The problem does need to be solved for existing platforms.
g.
On Wed, Mar 23, 2011 at 10:06:16AM +0000, Andy Green wrote:
> On 03/23/2011 09:47 AM, Somebody in the thread at some point said:
> >>There is no udev solution for what is being done currently by the async
> >>platform_data patchset with SDIO WLAN. The patches are out there and in
> >>use already. The only reason I don't post them here as round 2 of the
> >>RFC yet is because Grant wanted a couple of days and politically it's
> >>expedient for me to agree to that.
> >
> >Kernel policy has always been that just because some vendor has deployed
> >an interface doesn't mean we care one iota about it or consider it an
> >argument for the solution. In some cases in fact it bcomes the working
> >demo of why it was a bad idea.
>
> To be clear, this is not about any funny business at the interface
> on the hardware.
>
> The SDIO patches target wl12xx that is already in mainline and
> already using literally platform_data. Because there's no neater
> way on offer, it currently -- in mainline -- does it by having a
> built-in stub with its own Kconfig, that copies platform_data from
> the board definition file into a private malloc'd buffer, then uses
> it by getting a pointer to the copy from another private api in the
> driver. All this in a specific driver.
I've spent some time looking at the wl12xx driver code, and while the
data symbol it is using happens to be called 'platform_data', it isn't
actually the same thing as we're debating in this thread.
The platform_data in drivers/net/wireless/wl12xx_platform_data.c is a
strongly typed pointer to a 'struct wl12xx_platform_data', which
doesn't have the deficiencies associated with
(struct device*)->platform_data. It isn't a void* travelling through
the device model without guarantees that the right thing will get
dereferenced on the other end.
The fact that it has /other/ deficiencies is I imagine exactly why
you want to be rid of it, and rightly so. Immediately obvious is that
the way it currently is done means that there can never be more than
one wl12xx in the system.
I've got a proposal for a solution. I'll get it written up as
quickly as possible and send it out soon.
g.
On 03/24/2011 07:17 PM, Somebody in the thread at some point said:
Hi -
>> The SDIO patches target wl12xx that is already in mainline and
>> already using literally platform_data. Because there's no neater
>> way on offer, it currently -- in mainline -- does it by having a
>> built-in stub with its own Kconfig, that copies platform_data from
>> the board definition file into a private malloc'd buffer, then uses
>> it by getting a pointer to the copy from another private api in the
>> driver. All this in a specific driver.
>
> I've spent some time looking at the wl12xx driver code, and while the
> data symbol it is using happens to be called 'platform_data', it isn't
> actually the same thing as we're debating in this thread.
>
> The platform_data in drivers/net/wireless/wl12xx_platform_data.c is a
> strongly typed pointer to a 'struct wl12xx_platform_data', which
> doesn't have the deficiencies associated with
> (struct device*)->platform_data. It isn't a void* travelling through
> the device model without guarantees that the right thing will get
> dereferenced on the other end.
I see you're left exactly as you were found by the earlier discussion of
how dangerous or not that void * is in this particular case.
> The fact that it has /other/ deficiencies is I imagine exactly why
> you want to be rid of it, and rightly so. Immediately obvious is that
> the way it currently is done means that there can never be more than
> one wl12xx in the system.
Actually what I wanted from the start is to fix some issues for TI /
Panda and provide a generic way for other people to do something similar
as a side-effect, because delivery of this configuration information,
one way or another, is a generic issue.
Arnd seems to have sold his "change everything to eth%d" fix so that one
is off my plate.
I have a solution just at Panda level thanks to Alan Cox's workaround
for MAC setting on ethernet and WLAN I'll issue shortly, that does not
address the real problems that could be addressed with another approach
at the right layer.
That leaves my async platform_data code just fixing wl12xx access to its
own platform_data because that's what I found to clean up so far. And
then -->
> I've got a proposal for a solution. I'll get it written up as
> quickly as possible and send it out soon.
I see. Isn't it a bit curious you got this sudden off-piste interest in
improving the platform_data situation of wl12xx after it turns up in
this RFC? I mean I like having my work done for me, if that's how it is
maybe we can work out a system ^^
-Andy
On Thursday 24 March 2011, Alexey Orishko wrote:
> On Thu, Mar 24, 2011 at 2:15 PM, Arnd Bergmann <[email protected]> wrote:
>
> >
> > The approach taken here is to flag whether a device might be a
> > point-to-point link with the new FLAG_PTP setting in the usbnet
> > driver_info. A driver can set both FLAG_PTP and FLAG_ETHER if
> > it is not sure (e.g. cdc_ether), or just one of the two.
>
> > The usbnet framework only looks at the MAC address for device
> > naming if both flags are set, otherwise it trusts the flag.
>
> Should this paragraph above be a clue for the flag name?
> Sorry for late comment, but having flag called FLAG_POINTTOPOINT is really
> confusing. ptp, p2p terms are heavily used and will mislead folks.
>
> Would it be better to call it something like IGNORE_MAC_ADDRESS if this is the
> feature you are targeting?
That would be a different way of looking at it. FLAG_POINTTOPOINT
describes what the device is (a USB cable connecting two hosts), and
that flag can be used for various things, where the only thing
we currently do is the netif naming.
FLAG_IGNORE_MAC_ADDRESS as you suggest describes the implementation
of the device naming, not why that is done.
The intent here was to some something that makes sense next to
FLAG_ETHER, FLAG_WWAN and FLAG_WLAN. I think FLAG_POINTTOPOINT
describes this best, although I'd also be happy with FLAG_PTP,
FLAG_P2P, FLAG_CABLE or FLAG_USBCABLE.
Arnd
On Fri, Mar 25, 2011 at 12:57 PM, Arnd Bergmann <[email protected]> wrote:
>
> That would be a different way of looking at it. ?FLAG_POINTTOPOINT
> describes what the device is (a USB cable connecting two hosts), and
> that flag can be used for various things, where the only thing
> we currently do is the netif naming.
>
For example, cdc_ether and cdc-ncm drivers can be used in different use cases:
a) when device terminates the IP traffic
or
b) where device is a wireless router.
In both cases ethernet frames are sent over usb cable and terminated
in device (eth header stripped), so it is point-to-point link for ethernet, but
looking from IP layer is not p2p link for case b).
Please, explain, based on your idea, do we set this flag in both cases or not?
Do you want to use the same netif name for both use cases described above?
/alexey
On Friday 25 March 2011, Alexey Orishko wrote:
> On Fri, Mar 25, 2011 at 12:57 PM, Arnd Bergmann <[email protected]> wrote:
> >
> > That would be a different way of looking at it. FLAG_POINTTOPOINT
> > describes what the device is (a USB cable connecting two hosts), and
> > that flag can be used for various things, where the only thing
> > we currently do is the netif naming.
> >
>
> For example, cdc_ether and cdc-ncm drivers can be used in different use cases:
> a) when device terminates the IP traffic
> or
> b) where device is a wireless router.
>
> In both cases ethernet frames are sent over usb cable and terminated
> in device (eth header stripped), so it is point-to-point link for ethernet, but
> looking from IP layer is not p2p link for case b).
>
> Please, explain, based on your idea, do we set this flag in both cases or not?
> Do you want to use the same netif name for both use cases described above?
>
Most importantly, I want to keep the current rules, so that nothing breaks
for existing users.
For cdc_ether and cdc-ncm devices, my patch always sets both FLAG_ETHER and
FLAG_POINTTOPOINT, because the driver has no way to find out which of the
two is actually there.
The usb-net core driver interprets this as meaning that it has to decide
for the name based on something else, and that happens to be the presence
of a globally assigned MAC address. I don't think that keying off the MAC
address here is a particularly good idea, but that's what the driver has
always done.
Arnd