PLATFORM: Support for async platform_data
Subsystems (2)
cover PLATFORM: Support for async platform_data
The following series adds two apis which allow board definition files to register bindings between device paths (like usb1/1-1/1-1.1) and platform data. It's for use in the case that there are hardwired chips on a board that need to be configured from the board definition file, but the creation of the device is asynchronous to the boot action. Beagle XM and Panda are both real-world examples where there are hardwired chips on the PCB with fixed interconnect using USB protocol that can benefit from platform_data for the usual reasons.
This patcheset is the platform part of a three part set which when taken as a whole, allows the board definition file to configure USB-wired onboard network interface assets.
---
Andy Green (4):
PLATFORM: Add some documentation to platform docs about async platform_data
PLATFORM: Introduce async platform_data attach api
PLATFORM: Introduce registration function for async platform data maps
PLATFORM: introduce structure to bind async platform data to a dev path name
Documentation/driver-model/platform.txt | 25 +++++++++ drivers/base/platform.c | 91 +++++++++++++++++++++++++++++++ include/linux/platform_device.h | 22 +++++++ 3 files changed, 138 insertions(+), 0 deletions(-)
Signature · 1 line
Signature
RFC PATCH 1/4 PLATFORM: introduce structure to bind async platform data to a dev path name
This structure allows tagging arbitrary platform_data that can't be attached to a device until after it is probed, with the device path name that it is to be attached to.
---
include/linux/platform_device.h | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-)
include/linux/platform_device.h +17 -0
@@ -193,4 +193,21 @@ static inline char *early_platform_driver_setup_func(void) \ }#endif /* MODULE */ +/** +* platform_async_platform_data - maps a known bus + device name on to +* platform_data to be attached to that device +* when it is eventually instantiated. For use +* with onboard devices on buses that probe +* asynchronously. Device path fields must +* be separated with '/'. +* @device_path: bus / device path, eg, "usb1/1-1/1-1.1" +* @platform_data: platform_data to attach to device matching the +* device_path +*/ + +struct platform_async_platform_data { +const char *device_path; +void *platform_data; +}; +#endif /* _PLATFORM_DEVICE_H_ */
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Hi,
Quoted 26 lines · Andy Green
This structure allows tagging arbitrary
platform_datathat can't be attached to a device until after it is probed, with the device path name that it is to be attached to.Signed-off-by Andy Green <[email protected]>---
include/linux/platform_device.h | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-)include/linux/platform_device.h +8
-0@@ -193,4 +193,21 @@ static inline char *early_platform_driver_setup_func(void) \ }#endif /* MODULE */ +/** +* platform_async_platform_data - maps a known bus + device name on to +* platform_data to be attached to that device +* when it is eventually instantiated. For use +* with onboard devices on buses that probe +* asynchronously. Device path fields must +* be separated with '/'. +* @device_path: bus / device path, eg, "usb1/1-1/1-1.1"
include/linux/platform_device.h static inline char *early_platform_driver_setup_func(void) \ · line 213
Quoted 10 lines
include/linux/platform_device.h +9
-0+* @platform_data: platform_data to attach to device matching the +* device_path +*/ + +struct platform_async_platform_data { +const char *device_path; +void *platform_data; +}; +#endif /* _PLATFORM_DEVICE_H_ */
Using device paths for this purpose seems to be very fragile to me. Isn't there any better solution?
Rafael
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
On 03/12/2011 11:29 PM, Somebody in the thread at some point said:
Hi -
Quoted 5 lines · Andy Green
Quoted 3 lines
This structure allows tagging arbitrary
platform_datathat can't be attached to a device until after it is probed, with the device path name that it is to be attached to.
Quoted 9 lines
include/linux/platform_device.h static inline char *early_platform_driver_setup_func(void) \ · line 213
Quoted 6 lines
include/linux/platform_device.h +5
-0+struct platform_async_platform_data { +const char *device_path; +void *platform_data; +}; +#endif /* _PLATFORM_DEVICE_H_ */ Using device paths for this purpose seems to be very fragile to me. Isn't there any better solution?
Given that this targets board definition files which commonly do the platform_add_device for the USB bus controller synchronously, and the bus-connected devices it is aimed at are soldered on to the board connected to specific bus controllers, the bus paths are completely deterministic.
-Andy
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Quoted 19 lines · Andy Green
On 03/12/2011 11:29 PM, Somebody in the thread at some point said:
Hi -
Quoted 5 lines · Andy Green
Quoted 3 lines
This structure allows tagging arbitrary
platform_datathat can't be attached to a device until after it is probed, with the device path name that it is to be attached to.Quoted 9 lines
include/linux/platform_device.h static inline char *early_platform_driver_setup_func(void) \ · line 213
Quoted 6 lines
include/linux/platform_device.h +5
-0+struct platform_async_platform_data { +const char *device_path; +void *platform_data; +}; +#endif /* _PLATFORM_DEVICE_H_ */ Using device paths for this purpose seems to be very fragile to me. Isn't there any better solution?
Quoted 5 lines
Given that this targets board definition files which commonly do the
platform_add_devicefor the USB bus controller synchronously, and the bus-connected devices it is aimed at are soldered on to the board connected to specific bus controllers, the bus paths are completely deterministic.
No they are not.
The physical layout is deterministic, but the bus number, and device number, is not. You are using the bus number here in this path, so that is not going to work, sorry.
thanks,
greg k-h
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Quoted 9 lines · Somebody in the thread at some point
Quoted 8 lines
Quoted 2 lines
Using device paths for this purpose seems to be very fragile to me. Isn't there any better solution?
Given that this targets board definition files which commonly do the
platform_add_devicefor the USB bus controller synchronously, and the bus-connected devices it is aimed at are soldered on to the board connected to specific bus controllers, the bus paths are completely deterministic.
Quoted 5 lines
No they are not.
The physical layout is deterministic, but the bus number, and device number, is not. You are using the bus number here in this path, so that is not going to work, sorry.
Okay. This is not a PC we are talking about.
If the platform / board definition file is registering the USB hosts synchronously at boot time, the driver is composed into the monolithic kernel, there are no PCI busses or whatever on the SoC, the bus indexing is totally deterministic. This is extremely common in the platform / SoC case and is the case the patchset is targeted at. Even further, the only time you'd use it is to reach a USB asset that is wired up the same board permanently as well.
Anyway this seems moot by now.
-Andy
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Quoted 27 lines · Andy Green
Quoted 9 lines · Somebody in the thread at some point
Quoted 8 lines
Quoted 2 lines
Using device paths for this purpose seems to be very fragile to me. Isn't there any better solution?
Given that this targets board definition files which commonly do the
platform_add_devicefor the USB bus controller synchronously, and the bus-connected devices it is aimed at are soldered on to the board connected to specific bus controllers, the bus paths are completely deterministic.Quoted 5 lines
No they are not.
The physical layout is deterministic, but the bus number, and device number, is not. You are using the bus number here in this path, so that is not going to work, sorry.
Okay. This is not a PC we are talking about.
If the platform / board definition file is registering the USB hosts synchronously at boot time, the driver is composed into the monolithic kernel, there are no PCI busses or whatever on the SoC, the bus indexing is totally deterministic. This is extremely common in the platform / SoC case and is the case the patchset is targeted at. Even further, the only time you'd use it is to reach a USB asset that is wired up the same board permanently as well.
Quoted 1 line
Anyway this seems moot by now.
However, if you add a new infrastructure like this, it should be at least usable on systems that you description doesn't apply to.
Thanks,
Rafael
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Quoted 10 lines · Somebody in the thread at some point
Quoted 10 lines
Okay. This is not a PC we are talking about.
If the platform / board definition file is registering the USB hosts synchronously at boot time, the driver is composed into the monolithic kernel, there are no PCI busses or whatever on the SoC, the bus indexing is totally deterministic. This is extremely common in the platform / SoC case and is the case the patchset is targeted at. Even further, the only time you'd use it is to reach a USB asset that is wired up the same board permanently as well.
Quoted 4 lines
Quoted 1 line
Anyway this seems moot by now.
However, if you add a new infrastructure like this, it should be at least usable on systems that you description doesn't apply to.
Sounds reasonable, except the platform data is coming from a board-specific board definition file at boot-time to talk about assets that are on fixed interfaces of a specific board. It's not really applicable to wider generic bus use, just like platform_data usually isn't and has to be targeted at "device at XYZ on I2C bus n" with knowledge of what driver is bound to that device. Despite that "impedence mismatch", it covers the SoC onboard USB asset case just fine as it is.
If you mean though, that the patch series implements new configuration options in usbnet that are anyway interesting to expose for the general case, I can see the point but don't know enough about udev / usb subsystem internals to suggest a way to expose the options nicely.
It's also commented the "right" thing to do is to have the driver set the device up to wrong defaults like inappropriate interface name / random MAC and have userland clean up the mess, rather than have the board file ask the driver to set things appropriately.
-Andy
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Quoted 12 lines · Andy Green
Quoted 10 lines · Somebody in the thread at some point
Quoted 10 lines
Okay. This is not a PC we are talking about.
If the platform / board definition file is registering the USB hosts synchronously at boot time, the driver is composed into the monolithic kernel, there are no PCI busses or whatever on the SoC, the bus indexing is totally deterministic. This is extremely common in the platform / SoC case and is the case the patchset is targeted at. Even further, the only time you'd use it is to reach a USB asset that is wired up the same board permanently as well.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Anyway this seems moot by now.
However, if you add a new infrastructure like this, it should be at least usable on systems that you description doesn't apply to.
Sounds reasonable, except the platform data is coming from a board-specific board definition file at boot-time to talk about assets that are on fixed interfaces of a specific board.
I'm not sure how this contradicts what I said above.
Quoted 5 lines
It's not really applicable to wider generic bus use, just like
platform_datausually isn't and has to be targeted at "device at XYZ on I2C bus n" with knowledge of what driver is bound to that device. Despite that "impedence mismatch", it covers the SoC onboard USB asset case just fine as it is.
So, you want to have a mechanism telling the driver "if the device happens to have this particular path, use that platform data", right?
And it works because the initialization code kind of knows what path the device is going to be at, so it can predict that and provide the mathing data.
Unfortunately, this relies on how device paths are constructed at the moment, so if this approach is adopted in general, it will prevent us from changing that way in the future (or at least it will make that very difficult).
Perhaps you could use some other kind of device identification here?
Rafael
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Quoted 2 lines · Somebody in the thread at some point
So, you want to have a mechanism telling the driver "if the device happens to have this particular path, use that platform data", right?
Yeah.
Quoted 8 lines
And it works because the initialization code kind of knows what path the device is going to be at, so it can predict that and provide the mathing data.
Unfortunately, this relies on how device paths are constructed at the moment, so if this approach is adopted in general, it will prevent us from changing that way in the future (or at least it will make that very difficult).
Perhaps you could use some other kind of device identification here?
I'm sorry what prevents you changing paths in the future?
Nothing does, if you change the bus tag from like usb1 to UsB_1 you just fix up the strings in the board definition files at the same time, they are sitting there in the same tree and
grep platform_async_platform_data arch/* -R
will show them all up in one hit. It's no different if you changed the name of a driver, you'd patch the board definition files with devices that need to bind to that driver name to uplevel them to the new name.
The board definition file for these SoC cases usually has access to a pointer to the host controller directly since it instantiates them, if this was the only stumbling point and you thought it helped something the matching system can look for that particular pointer being a parent of the candidate probed device.
-Andy
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
Quoted 22 lines · Andy Green
Quoted 2 lines · Somebody in the thread at some point
So, you want to have a mechanism telling the driver "if the device happens to have this particular path, use that platform data", right?
Yeah.
Quoted 8 lines
And it works because the initialization code kind of knows what path the device is going to be at, so it can predict that and provide the mathing data.
Unfortunately, this relies on how device paths are constructed at the moment, so if this approach is adopted in general, it will prevent us from changing that way in the future (or at least it will make that very difficult).
Perhaps you could use some other kind of device identification here?
I'm sorry what prevents you changing paths in the future?
Nothing does, if you change the bus tag from like usb1 to
UsB_1you just fix up the strings in the board definition files at the same time, they are sitting there in the same tree and
Quoted 5 lines
grep
platform_async_platform_dataarch/*-Rwill show them all up in one hit. It's no different if you changed the name of a driver, you'd patch the board definition files with devices that need to bind to that driver name to uplevel them to the new name.
Well, I'm in the process of doing something similar with sysdevs and, trust me, it is not fun. :-/
Quoted 5 lines
The board definition file for these SoC cases usually has access to a pointer to the host controller directly since it instantiates them, if this was the only stumbling point and you thought it helped something the matching system can look for that particular pointer being a parent of the candidate probed device.
I think that would have been better than matching based on device paths.
Whether or not it's the only stumbling point depends on what other people think.
Thanks,
Rafael
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
In reply to Andy GreenQuoted 11 lines · Andy Green
Quoted 9 lines · Somebody in the thread at some point
Quoted 8 lines
Quoted 2 lines
Using device paths for this purpose seems to be very fragile to me. Isn't there any better solution?
Given that this targets board definition files which commonly do the
platform_add_devicefor the USB bus controller synchronously, and the bus-connected devices it is aimed at are soldered on to the board connected to specific bus controllers, the bus paths are completely deterministic.
Quoted 7 lines
Quoted 5 lines
No they are not.
The physical layout is deterministic, but the bus number, and device number, is not. You are using the bus number here in this path, so that is not going to work, sorry.
Okay. This is not a PC we are talking about.
I know that, and again, it doesn't matter.
You CAN NOT GUARANTEE the USB device ordering of bus numbers or device numbers. It's that simple.
Quoted 4 lines
If the platform / board definition file is registering the USB hosts synchronously at boot time, the driver is composed into the monolithic kernel, there are no PCI busses or whatever on the SoC, the bus indexing is totally deterministic.
Not true, it could change for a number of reasons, not the least being your kernel version changed.
So again NEVER rely on this, bad things could happen in the field when you least expect it.
thanks,
greg k-h
Re: [RFC PATCH 1/4] PLATFORM: introduce structure to bind async platform data to a dev path name
On 03/13/2011 04:14 PM, Somebody in the thread at some point said:
Hi -
Quoted 8 lines
You CAN NOT GUARANTEE the USB device ordering of bus numbers or device numbers. It's that simple.
Quoted 4 lines
If the platform / board definition file is registering the USB hosts synchronously at boot time, the driver is composed into the monolithic kernel, there are no PCI busses or whatever on the SoC, the bus indexing is totally deterministic.
Quoted 5 lines
Not true, it could change for a number of reasons, not the least being your kernel version changed.
So again NEVER rely on this, bad things could happen in the field when you least expect it.
Okay, I can't see how and you did not explain how, but let's agree with what you are saying.
Unlike the Shiny Device Tree path where the binding device path string is in the bootloader, with this patch series the binding device path is in the board definition file, ie, part of the same kernel. If an upgrade breaks it, the guy can look in /sys on his new broken kernel, find the new path and uplevel it to that and he's consistent again. If he goes back to an older kernel, it still works consistently (unlike if he updated his bootloader that now only knows the new way).
However as I said to Rafael if he thought bus name part of this path was too shaky, and you also think it is, it can be changed to use a pointer to the host controller since that's also coming from platform or board definition file directly in this kind of SoC implementation and is available.
-Andy
RFC PATCH 2/4 PLATFORM: Introduce registration function for async platform data maps
In reply to Andy GreenThis adds a small platform API that lets a board definition file register a mapping structure for asynchronously probed devices so platform_data can be attached to them at probe time.
---
drivers/base/platform.c | 21 +++++++++++++++++++++ include/linux/platform_device.h | 3 +++ 2 files changed, 24 insertions(+), 0 deletions(-)
drivers/base/platform.c +21 -0
@@ -31,6 +31,12 @@ struct device platform_bus = { };EXPORT_SYMBOL_GPL (platform_bus); +struct platform_async_platform_data *platform_async_platform_data_map; +EXPORT_SYMBOL_GPL (platform_async_platform_data_map); + +int platform_async_platform_data_count; +EXPORT_SYMBOL_GPL (platform_async_platform_data_count); +/** * platform_get_resource - get a resource for a device * @dev: platform device
@@ -1332,3 +1338,18 @@ void __init early_platform_cleanup(void) } } +/** +* platform_async_platform_data_register - register an array of async- +* probed bus / device names mapping to +* plaform_data for that device. +* @map: the array of devname vs platform_data mapping structs +* @count: the count of structs in the @map array +*/ + +void platform_async_platform_data_register ( +struct platform_async_platform_data *map,int count) +{ + platform_async_platform_data_map = map; + platform_async_platform_data_count = count; +} +EXPORT_SYMBOL_GPL (platform_async_platform_data_register);
include/linux/platform_device.h +3 -0
@@ -210,4 +210,7 @@ struct platform_async_platform_data { void *platform_data; }; +extern void platform_async_platform_data_register ( +struct platform_async_platform_data *map,int count); +#endif /* _PLATFORM_DEVICE_H_ */
RFC PATCH 3/4 PLATFORM: Introduce async platform_data attach api
In reply to Andy GreenThis introduces a platform API so busses can allow platform_data to be attached to any struct device they create from probing in one step.
The function checks through the async platform_data map if one was previously registered, and checks the device's device path for itself and its parents against the mapped device path names.
If it sees a match, it attaches the associated platform_data and sets that map entry's device_path to NULL so no further time is spent trying to match it.
---
drivers/base/platform.c | 70 +++++++++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 2 + 2 files changed, 72 insertions(+), 0 deletions(-)
drivers/base/platform.c +70 -0
@@ -1353,3 +1353,73 @@ void platform_async_platform_data_register( platform_async_platform_data_count = count; }EXPORT_SYMBOL_GPL (platform_async_platform_data_register); + +/** +* platform_async_platform_data_attach - if there is any async devname map +* defined, go through it looking for a match with the +* device's path considering its parent device names. +* If a match is found, attach the corresponding +* platform_data and the entry in the map table set to +* NULL so it won't be looked for again. +* +* @dev: device to have data attched to if it matches any map entry +*/ +void platform_async_platform_data_attach (struct device *dev) +{ +struct platform_async_platform_data *map; +const char *path; +int count; +const char *p; +int len; +struct device *devn; + + map = platform_async_platform_data_map; + count = platform_async_platform_data_count; + +while (count--) { + +if (map->device_path ==NULL ) { + map++; +continue ; + } + + p = map->device_path +strlen (map->device_path); + devn = dev; + +while (devn) { + + path =dev_name (devn); + len =strlen (path); + +if ((p - map->device_path) < len) { + devn =NULL ; +continue ; + } + + p -= len; + +if (strncmp (path, p, len)) { + devn =NULL ; +continue ; + } + + devn = devn->parent; +if (p == map->device_path) { +dev_info (dev,"Attched async platform data\n" ); + dev->platform_data = map->platform_data; + map->device_path =NULL ; +return ; + } + +if (devn !=NULL && (p - map->device_path) <2 ) + devn =NULL ; + + p--; +if (devn !=NULL && *p !='/' ) + devn =NULL ; + } + + map++; + } +} +EXPORT_SYMBOL_GPL (platform_async_platform_data_attach);
include/linux/platform_device.h +2 -0
@@ -213,4 +213,6 @@ struct platform_async_platform_data {extern void platform_async_platform_data_register (struct platform_async_platform_data *map,int count); +extern void platform_async_platform_data_attach (struct device *dev); +#endif /* _PLATFORM_DEVICE_H_ */
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 10 lines · Andy Green
This introduces a platform API so busses can allow
platform_datato be attached to any struct device they create from probing in one step.The function checks through the async
platform_datamap if one was previously registered, and checks the device's device path for itself and its parents against the mapped device path names.If it sees a match, it attaches the associated
platform_dataand sets that map entry'sdevice_pathto NULL so no further time is spent trying to match it.
This _really_ should just use the device tree stuff, that is what it is for, please don't duplicate it here in a not-as-flexible way.
Why wouldn't that work for you instead for this type of thing?
thanks,
greg k-h
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 8 lines · Greg KH
Quoted 7 lines · Andy Green
This introduces a platform API so busses can allow
platform_datato be attached to any struct device they create from probing in one step.The function checks through the async
platform_datamap if one was previously registered, and checks the device's device path for itself and its parents against the mapped device path names.
Quoted 6 lines
Quoted 3 lines
If it sees a match, it attaches the associated
platform_dataand sets that map entry'sdevice_pathto NULL so no further time is spent trying to match it.This _really_ should just use the device tree stuff, that is what it is for, please don't duplicate it here in a not-as-flexible way.
I agree.
@Andy: If it doesn't work for you for some reason, please let us know the usage case that is not covered (in detail).
Thanks,
Rafael
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 13 lines · Somebody in the thread at some point
Quoted 12 lines · Greg KH
Quoted 10 lines · Andy Green
This introduces a platform API so busses can allow
platform_datato be attached to any struct device they create from probing in one step.The function checks through the async
platform_datamap if one was previously registered, and checks the device's device path for itself and its parents against the mapped device path names.If it sees a match, it attaches the associated
platform_dataand sets that map entry'sdevice_pathto NULL so no further time is spent trying to match it.
Quoted 7 lines
Quoted 2 lines
This _really_ should just use the device tree stuff, that is what it is for, please don't duplicate it here in a not-as-flexible way.
I agree.
@Andy: If it doesn't work for you for some reason, please let us know the usage case that is not covered (in detail).
The device tree stuff does not yet exist in a workable way, platform_data is established everywhere except USB bus. Device tree brings in bootloader version as a dependency: this method doesn't.
Using platform / board definition files to define and configure the SoC and most or all of its onboard assets is a technique widely deployed in SoC board definition files. The patch just very lightly extends platform_data to be workable with USB bus devices like it is the other buses, within the goal of being able to configure onboard assets at boot-time, so it's quite unremarkable from that POV.
However if you don't have that perspective on it, and think about it on a PC where it is not intended to be used, no doubt the patchset's approach is hard to understand as useful.
Device Tree will be a nice improvement in many ways when it is done, in this particular case though presumably it'll have to patch usbnet and smsc95xx in a similar way to offer similar configurability.
In the meanwhile, either Panda and Beagle will stay as they are for these misfeatures or specific userlands will have to be stunk up with /proc/cpuinfo-grepping board-specific ditro-by-distro quirk management.
Anyway, thanks for all the comments on this RFC.
-Andy
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 17 lines · Andy Green
Quoted 16 lines · Somebody in the thread at some point
Quoted 14 lines · Greg KH
Quoted 10 lines · Andy Green
This introduces a platform API so busses can allow
platform_datato be attached to any struct device they create from probing in one step.The function checks through the async
platform_datamap if one was previously registered, and checks the device's device path for itself and its parents against the mapped device path names.If it sees a match, it attaches the associated
platform_dataand sets that map entry'sdevice_pathto NULL so no further time is spent trying to match it.This _really_ should just use the device tree stuff, that is what it is for, please don't duplicate it here in a not-as-flexible way.
Quoted 8 lines
Quoted 4 lines
I agree.
@Andy: If it doesn't work for you for some reason, please let us know the usage case that is not covered (in detail).
The device tree stuff does not yet exist in a workable way,
platform_datais established everywhere except USB bus. Device tree brings in bootloader version as a dependency: this method doesn't.
It is not the same device tree we are talking about. :-)
I mean device hierarchy (and I guess Greg meant the same).
Thanks,
Rafael
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 8 lines · Somebody in the thread at some point
Quoted 8 lines
Quoted 7 lines
Quoted 2 lines
This _really_ should just use the device tree stuff, that is what it is for, please don't duplicate it here in a not-as-flexible way.
I agree.
@Andy: If it doesn't work for you for some reason, please let us know the usage case that is not covered (in detail).
Quoted 7 lines
Quoted 3 lines
The device tree stuff does not yet exist in a workable way,
platform_datais established everywhere except USB bus. Device tree brings in bootloader version as a dependency: this method doesn't.It is not the same device tree we are talking about. :-)
I mean device hierarchy (and I guess Greg meant the same).
I see. Elsewhere on the previous thread people were proposing to use New Shiny Device Tree, hence the confusion.
I am using the old style device tree to walk the device's parent path. What were you guys actually suggesting to do differently via the device tree then that's cleaner?
-Andy
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 14 lines · Andy Green
Quoted 12 lines · Somebody in the thread at some point
Quoted 11 lines
Quoted 7 lines
Quoted 2 lines
This _really_ should just use the device tree stuff, that is what it is for, please don't duplicate it here in a not-as-flexible way.
I agree.
@Andy: If it doesn't work for you for some reason, please let us know the usage case that is not covered (in detail).
The device tree stuff does not yet exist in a workable way,
platform_datais established everywhere except USB bus. Device tree brings in bootloader version as a dependency: this method doesn't.
Quoted 6 lines
Quoted 3 lines
It is not the same device tree we are talking about. :-)
I mean device hierarchy (and I guess Greg meant the same).
I see. Elsewhere on the previous thread people were proposing to use New Shiny Device Tree, hence the confusion.
Yes, I meant the "new shiny device tree" work from Grant, who in an earlier message, said that this could all be done using that instead of your proposal.
thanks,
greg k-h
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 17 lines · Somebody in the thread at some point
Quoted 16 lines · Andy Green
Quoted 14 lines · Somebody in the thread at some point
Quoted 11 lines
Quoted 7 lines
Quoted 2 lines
This _really_ should just use the device tree stuff, that is what it is for, please don't duplicate it here in a not-as-flexible way.
I agree.
@Andy: If it doesn't work for you for some reason, please let us know the usage case that is not covered (in detail).
The device tree stuff does not yet exist in a workable way,
platform_datais established everywhere except USB bus. Device tree brings in bootloader version as a dependency: this method doesn't.It is not the same device tree we are talking about. :-)
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
I mean device hierarchy (and I guess Greg meant the same).
I see. Elsewhere on the previous thread people were proposing to use New Shiny Device Tree, hence the confusion.
Yes, I meant the "new shiny device tree" work from Grant, who in an earlier message, said that this could all be done using that instead of your proposal.
That is what I took you to mean, since I already use oldstyle device tree as far as I could see it was possible. So I have no idea what Rafael thought you or he meant by strongly agreeing with you when he was mistaken that thought you meant oldstyle device tree. Anyway never mind.
Well I never heard mentioned before that Device Tree targets asynchronously probed device configuration. If it does, and can do the same effective as the first patchset, then I guess that will (when it exists) fulfil a similar job and that'd be fine.
But what this overall patch set does in panda.c, usbnet and smsc95xx will need the same work done on it either way to deliver the same new configuration features in the driver side, via Shiny New Device Tree or whatever.
So when there's a bit more of Device Tree in evidence, are you going to accept Device-tree based patches in usbnet etc along these lines, or does that trigger the "do it in userspace" response, in which case we are both wasting each others' time continuing to discuss this at all?
-Andy
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 5 lines · Andy Green
So when there's a bit more of Device Tree in evidence, are you going to accept Device-tree based patches in usbnet etc along these lines, or does that trigger the "do it in userspace" response, in which case we are both wasting each others' time continuing to discuss this at all?
As mentioned numerous times before, network naming stuff happens in userspace, not in the kernel, so no matter what infrastructure is added, insisting on naming the network device 'eth0' is not going to happen within the kernel. Please use the tools we have today to do this with no kernel changes.
As for other changes, it all depends on what you need to accomplish, right? Those will be gladly reviewed on a case-by-case basis.
thanks,
greg k-h
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 9 lines · Somebody in the thread at some point
Quoted 5 lines · Andy Green
So when there's a bit more of Device Tree in evidence, are you going to accept Device-tree based patches in usbnet etc along these lines, or does that trigger the "do it in userspace" response, in which case we are both wasting each others' time continuing to discuss this at all?
As mentioned numerous times before, network naming stuff happens in userspace, not in the kernel, so no matter what infrastructure is added,
The naming of that network interface happens in smsc95xx, in kernel: it uses a dodgy heuristic to choose between usb%d eth%d and others and gets it wrong in this case due to stuff outside its view. There's nothing golden and wonderful about that which needs protecting from outside hints.
Quoted 6 lines
insisting on naming the network device 'eth0' is not going to happen within the kernel. Please use the tools we have today to do this with no kernel changes.
As for other changes, it all depends on what you need to accomplish, right? Those will be gladly reviewed on a case-by-case basis.
The immediate thing I would have liked to accomplish is to help smsc95xx make the right decision about naming and to use the MAC address in platform_data. It seemed this was a general issue though, so I generalized how it was done. But I don't have examples on my desk of boards with soldered-on USB other than smsc95xx Ethernet bridge.
At least, I am grateful we get down to brass tacks now and you clearly reject allowing the driver to take guidance to act more appropriately for its environment in the first place. Therefore, this won't get fixed by Device Tree either.
So, we don't need to discuss this any further: thanks again for your time.
-Andy
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 8 lines · Andy Green
Quoted 7 lines · Somebody in the thread at some point
Quoted 5 lines · Andy Green
So when there's a bit more of Device Tree in evidence, are you going to accept Device-tree based patches in usbnet etc along these lines, or does that trigger the "do it in userspace" response, in which case we are both wasting each others' time continuing to discuss this at all?
Quoted 8 lines
Quoted 2 lines
As mentioned numerous times before, network naming stuff happens in userspace, not in the kernel, so no matter what infrastructure is added,
The naming of that network interface happens in smsc95xx, in kernel:
it uses a dodgy heuristic to choose between usb%d eth%d and others and gets it wrong in this case due to stuff outside its view. There's nothing golden and wonderful about that which needs protecting from outside hints.
Then why not always do this in userspace correctly? It's the _exact_ same problem that the server companies have in naming their network devices in a proper manner (i.e. the port with the 0 label on it wants to always be eth0). We have the tools today to solve this issue, in a consistant and proper manner, please use them.
Quoted 14 lines
Quoted 6 lines
insisting on naming the network device 'eth0' is not going to happen within the kernel. Please use the tools we have today to do this with no kernel changes.
As for other changes, it all depends on what you need to accomplish, right? Those will be gladly reviewed on a case-by-case basis.
The immediate thing I would have liked to accomplish is to help smsc95xx make the right decision about naming and to use the MAC address in
platform_data. It seemed this was a general issue though, so I generalized how it was done. But I don't have examples on my desk of boards with soldered-on USB other than smsc95xx Ethernet bridge.
Quoted 4 lines
At least, I am grateful we get down to brass tacks now and you clearly reject allowing the driver to take guidance to act more appropriately for its environment in the first place. Therefore, this won't get fixed by Device Tree either.
I beg to differ. I'm stating that USB drivers can not use their numbering to do anything with them as that is never going to be deterministic.
That's all. You seem to disagree with this.
Quoted 2 lines
So, we don't need to discuss this any further: thanks again for your time.
Ok, so does this mean that you are convinced that I am correct here, or are just giving up and going to keep your patches outside of the kernel, or something else?
confused,
greg k-h
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 18 lines · Somebody in the thread at some point
Quoted 8 lines · Andy Green
Quoted 7 lines · Somebody in the thread at some point
Quoted 5 lines · Andy Green
So when there's a bit more of Device Tree in evidence, are you going to accept Device-tree based patches in usbnet etc along these lines, or does that trigger the "do it in userspace" response, in which case we are both wasting each others' time continuing to discuss this at all?
Quoted 8 lines
Quoted 2 lines
As mentioned numerous times before, network naming stuff happens in userspace, not in the kernel, so no matter what infrastructure is added,
The naming of that network interface happens in smsc95xx, in kernel:
it uses a dodgy heuristic to choose between usb%d eth%d and others and gets it wrong in this case due to stuff outside its view. There's nothing golden and wonderful about that which needs protecting from outside hints.
Quoted 5 lines
Then why not always do this in userspace correctly? It's the _exact_ same problem that the server companies have in naming their network devices in a proper manner (i.e. the port with the 0 label on it wants to always be eth0). We have the tools today to solve this issue, in a consistant and proper manner, please use them.
It is not at all the same problem.
Here, the usbnet driver tries to figure out what kind of device label it should use, the numbering is then dependent on the label and there's no problem coming from the numbering, nor do the patches touch that.
The first issue for usbnet is right now, it has no real insight into what the appropriate interface name is because it is out of its view if the USB Ethernet bridge is soldered onboard or not. You know, it is also not in a generic userspace's view what is soldered on that board either, so I can only read your pushing of that 'solution' as "get this out of my hair". The one place that can properly know it right now is the board definition file in the kernel, maybe Device Tree too in the future.
To do it what you are calling the "correct" way in userland, you are okay with the driver selecting the wrong interface name, condemning userland to regenerate board-specific knowledge from grepping /proc/cpuinfo, inferring in userland what can easily and justly be known in the board definition file in kernel, and overriding the wrong interface name. In all that, Caesar's hands are clean alright, but don't tell me this is a good job and the correct solution.
Despite not having the information, the driver does have go at a heuristic, but because net->dev_addr[] is always coming from random_ether_addr(), which rightly forces [0] & 2 to 1 indicating it's a private namespace MAC address, the heuristic is broken and never selects eth%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) + /* + * 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. Async + * platform_data can also override this if it knows better + * based on knowledge of what this link is wired up to. + */
+ if ((((dev->driver_info->flags & FLAG_ETHER) != 0 &&
+ (net->dev_addr[0] & 0x02) == 0)) ||
+ (pdata && pdata->flags &
+ USBNET_PLATDATA_FLAG__FORCE_ETH_IFNAME))
strcpy (net->name, "eth%d");
static inline void random_ether_addr(u8 *addr)
{
get_random_bytes (addr, ETH_ALEN);
addr [0] &= 0xfe; /* clear multicast bit */
addr [0] |= 0x02; /* set local assignment bit (IEEE802) */
}If you look further down, it is making an attempt to choose the right name at interface creation time by taking information from other sources already
/* WLAN devices should always be named "wlan%d" */
if ((dev->driver_info->flags & FLAG_WLAN) != 0)My patch just has it also look if there is a pdata->flags to see if the board definition file is guiding it to choose eth%d.
The second issue for usbnet which the patchset solves is that right now, it will always give a random MAC address that differs each boot, driving dhpcd insane.
Now once again, the existing code will take information from other sources to guide it about that too. In smsc95xx usbnet driver, it looks to see if there is an EEPROM attached to the chip and will replace the random MAC address with the one from there if there is. The patch extends that to check usbnet platform_data to see if a MAC address has been sent up from the board definition file.
static void smsc95xx_init_mac_address(struct usbnet *dev) {
+ struct usbnet_platform_data *pdata = dev->udev->dev.platform_data;
+
+ /*
+ * if netdev platform data has taken responsibility for forcing + * the MAC then nothing to do here + */
+
+ if (pdata && pdata->flags & USBNET_PLATDATA_FLAG__USE_MAC)
+ return;
+
/* try reading mac address from EEPROM */
if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN,
dev->net->dev_addr) == 0) {So given what it already does in a broken way for these valid implementations of smsc95xx with no EEPROM, I really do not understand what sanctity of these drivers is being violated by having it do the same things better using a couple of extra lines with platform_data.
For sure, in any other bus, we will never have this discussion about if the device driver should use relevant platform_data to be guided about what to do, that is what platform_data is for and it is very widely used indeed.
Quoted 19 lines
Quoted 14 lines
Quoted 6 lines
insisting on naming the network device 'eth0' is not going to happen within the kernel. Please use the tools we have today to do this with no kernel changes.
As for other changes, it all depends on what you need to accomplish, right? Those will be gladly reviewed on a case-by-case basis.
The immediate thing I would have liked to accomplish is to help smsc95xx make the right decision about naming and to use the MAC address in
platform_data. It seemed this was a general issue though, so I generalized how it was done. But I don't have examples on my desk of boards with soldered-on USB other than smsc95xx Ethernet bridge.Quoted 4 lines
At least, I am grateful we get down to brass tacks now and you clearly reject allowing the driver to take guidance to act more appropriately for its environment in the first place. Therefore, this won't get fixed by Device Tree either.
Quoted 5 lines
I beg to differ. I'm stating that USB drivers can not use their numbering to do anything with them as that is never going to be deterministic.
That's all. You seem to disagree with this.
Actually as I indicated I am willing to pretend to agree it applies to these SoC cases (no argument it is certainly so in a non-SoC PC type situation where the board definition file / platform_data is not used), and offered to adapt the bus selection to use a usb host interface pointer from the board definition file that should solve that objection.
Quoted 6 lines
Quoted 2 lines
So, we don't need to discuss this any further: thanks again for your time.
Ok, so does this mean that you are convinced that I am correct here, or are just giving up and going to keep your patches outside of the kernel, or something else?
It means I acknowledge you are the guy in the way of solving these issues in usbnet, and that these patches will be discarded without your approval. Normally, for me offering patches upstream is a side issue because I am trying to deliver some product, so with this level of pushback I would just shrug and hold it in my tree. In this case though I am being paid to help TI solve issues upstream, the patches do solve real issues for them, but they're trying to eliminate BSP tree concept in favour of everything upstream, so there is no other place for them.
I'm willing to put in effort as I have in this thread explaining things more clearly when the proposal was not understood, and writing patches so it is unambiguous, adapting to feedback that improves things or even explains why the proposal isn't usable, but I also understand that where you are wrong and are simply filtering out the arguments to the contrary due to dogma, I am wasting my time, your time, you will stay wrong and the driver will stay broken. In the meanwhile, I will sleep badly and my wife will be unhappy with me for burning a weekend on it, so if there's no useful path forward let's not protract the agony for either of us.
-Andy
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 32 lines · Andy Green
Quoted 18 lines · Somebody in the thread at some point
Quoted 8 lines · Andy Green
Quoted 7 lines · Somebody in the thread at some point
Quoted 5 lines · Andy Green
So when there's a bit more of Device Tree in evidence, are you going to accept Device-tree based patches in usbnet etc along these lines, or does that trigger the "do it in userspace" response, in which case we are both wasting each others' time continuing to discuss this at all?
Quoted 8 lines
Quoted 2 lines
As mentioned numerous times before, network naming stuff happens in userspace, not in the kernel, so no matter what infrastructure is added,
The naming of that network interface happens in smsc95xx, in kernel:
it uses a dodgy heuristic to choose between usb%d eth%d and others and gets it wrong in this case due to stuff outside its view. There's nothing golden and wonderful about that which needs protecting from outside hints.Quoted 5 lines
Then why not always do this in userspace correctly? It's the _exact_ same problem that the server companies have in naming their network devices in a proper manner (i.e. the port with the 0 label on it wants to always be eth0). We have the tools today to solve this issue, in a consistant and proper manner, please use them.
It is not at all the same problem.
Here, the usbnet driver tries to figure out what kind of device label it should use, the numbering is then dependent on the label and there's no problem coming from the numbering, nor do the patches touch that.
Quoted 8 lines
The first issue for usbnet is right now, it has no real insight into what the appropriate interface name is because it is out of its view if the USB Ethernet bridge is soldered onboard or not. You know, it is also not in a generic userspace's view what is soldered on that board either, so I can only read your pushing of that 'solution' as "get this out of my hair". The one place that can properly know it right now is the board definition file in the kernel, maybe Device Tree too in the future.
It's not up to the driver to "know" this at all.
Quoted 8 lines
To do it what you are calling the "correct" way in userland, you are okay with the driver selecting the wrong interface name, condemning userland to regenerate board-specific knowledge from grepping
/proc/cpuinfo, inferring in userland what can easily and justly be known in the board definition file in kernel, and overriding the wrong interface name. In all that, Caesar's hands are clean alright, but don't tell me this is a good job and the correct solution.
It really is. How do you know that I don't want to name this device something else than you feel you should? Do you really want to tell me to recompile my kernel just to change the name of the network device?
No, it has been determined a long time ago that network naming things like this are to be done in userspace. It's an argument that has come and gone many years ago, sorry. See all of the wonderful, and simple, tools we have today in userspace to handle this type of thing. Distros can use them how ever they see fit, and even better, users can configure them! That means they don't have to rebuild their kernels, which is a bit unreasonable, don't you think?
Quoted 13 lines
Despite not having the information, the driver does have go at a heuristic, but because net->dev_addr[] is always coming from
random_ether_addr(), which rightly forces [0] & 2 to 1 indicating it's a private namespace MAC address, the heuristic is broken and never selects eth%d.(quoted diff) +2
-5- // 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) + /* + * heuristic: "usb%d" for links we know are two-host,
Review
Quoted 10 lines
(quoted diff) +9
-0+ * else "eth%d" when there's reasonable doubt. userspace + * can rename the link if it knows better. Async + * platform_data can also override this if it knows better + * based on knowledge of what this link is wired up to. + */ + if ((((dev->driver_info->flags & FLAG_ETHER) != 0 && + (net->dev_addr[0] & 0x02) == 0)) || + (pdata && pdata->flags & + USBNET_PLATDATA_FLAG__FORCE_ETH_IFNAME)) strcpy (net->name, "eth%d");
Perhaps we should just always name these things 'eth%d'? Oh wait, as it really is a USB device, they are supposed to be called 'usb%d' as determined (again) a long time ago.
If a distro/board manufacturer wants to hide the fact that this really is a usb device by renaming it to eth0, then again, it can. But don't force the kernel to have that policy in it.
Quoted 12 lines
static inline void
random_ether_addr(u8 *addr){ get_random_bytes (addr, ETH_ALEN); addr [0] &= 0xfe; /* clear multicast bit */ addr [0] |= 0x02; /* set local assignment bit (IEEE802) */ }If you look further down, it is making an attempt to choose the right name at interface creation time by taking information from other sources already
Quoted 5 lines
/* WLAN devices should always be named "wlan%d" */ if ((dev->driver_info->flags & FLAG_WLAN) != 0)My patch just has it also look if there is a
pdata->flagsto see if the board definition file is guiding it to choose eth%d.
Ick, well, I'm not saying that cleaning up this driver is not a bad idea at all, feel free to submit patches for it that are self-contained.
Quoted 27 lines
The second issue for usbnet which the patchset solves is that right now, it will always give a random MAC address that differs each boot, driving dhpcd insane.
Now once again, the existing code will take information from other sources to guide it about that too. In smsc95xx usbnet driver, it looks to see if there is an EEPROM attached to the chip and will replace the random MAC address with the one from there if there is. The patch extends that to check usbnet
platform_datato see if a MAC address has been sent up from the board definition file.static void
smsc95xx_init_mac_address(struct usbnet *dev){(quoted diff) +10
-0+ struct usbnet_platform_data *pdata = dev->udev->dev.platform_data; + + /* + * if netdev platform data has taken responsibility for forcing + * the MAC then nothing to do here + */ + + if (pdata && pdata->flags & USBNET_PLATDATA_FLAG__USE_MAC) + return; + /* try reading mac address from EEPROM */ if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN, dev->net->dev_addr) == 0) {
Quoted 5 lines
So given what it already does in a broken way for these valid implementations of smsc95xx with no EEPROM, I really do not understand what sanctity of these drivers is being violated by having it do the same things better using a couple of extra lines with
platform_data.
You can set MAC addresses from userspace, why not do that here for systems in which the hardware designers messed things up so bad it takes a programmer to fix their brokenness?
Quoted 4 lines
For sure, in any other bus, we will never have this discussion about if the device driver should use relevant
platform_datato be guided about what to do, that is whatplatform_datais for and it is very widely used indeed.
Are you sure? Try me on one of the other "real" busses out there like PCI and see how all of these same arguments are identical :)
The abuse of platform_data today does not give you the right to persist in it. Look at the work of the device tree developers to fix this problem as proof of this.
Quoted 10 lines
Quoted 6 lines
Quoted 2 lines
So, we don't need to discuss this any further: thanks again for your time.
Ok, so does this mean that you are convinced that I am correct here, or are just giving up and going to keep your patches outside of the kernel, or something else?
It means I acknowledge you are the guy in the way of solving these issues in usbnet, and that these patches will be discarded without your approval.
Again, because you can solve them in userspace! Why is that not a correct solution? It should make you even happier as you don't have to have any kernel patches that are not upstream, you can drop them entirely and just set up some of your distro's config files and be done with it.
thanks,
greg k-h
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 7 lines · Greg KH
No, it has been determined a long time ago that network naming things like this are to be done in userspace. It's an argument that has come and gone many years ago, sorry. See all of the wonderful, and simple, tools we have today in userspace to handle this type of thing. Distros can use them how ever they see fit, and even better, users can configure them! That means they don't have to rebuild their kernels, which is a bit unreasonable, don't you think?
...
Quoted 7 lines
Perhaps we should just always name these things 'eth%d'? Oh wait, as it really is a USB device, they are supposed to be called 'usb%d' as determined (again) a long time ago.
If a distro/board manufacturer wants to hide the fact that this really is a usb device by renaming it to eth0, then again, it can. But don't force the kernel to have that policy in it.
This argument does sound contradictory. If network interface naming should be left entirely up to userspace, then why doesn't the kernel always generate names of the form "eth%d"? Why not rip all that stuff about "usb%d" or "wlan%d" out of the driver entirely?
(Apart from the fact that this would be a user-visible change in kernel policy and would break a large number of systems...)
Alan Stern
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
Quoted 20 lines · Alan Stern
Quoted 7 lines · Greg KH
No, it has been determined a long time ago that network naming things like this are to be done in userspace. It's an argument that has come and gone many years ago, sorry. See all of the wonderful, and simple, tools we have today in userspace to handle this type of thing. Distros can use them how ever they see fit, and even better, users can configure them! That means they don't have to rebuild their kernels, which is a bit unreasonable, don't you think?
...
Quoted 7 lines
Perhaps we should just always name these things 'eth%d'? Oh wait, as it really is a USB device, they are supposed to be called 'usb%d' as determined (again) a long time ago.
If a distro/board manufacturer wants to hide the fact that this really is a usb device by renaming it to eth0, then again, it can. But don't force the kernel to have that policy in it.
Quoted 7 lines
This argument does sound contradictory. If network interface naming should be left entirely up to userspace, then why doesn't the kernel always generate names of the form "eth%d"? Why not rip all that stuff about "usb%d" or "wlan%d" out of the driver entirely?
(Apart from the fact that this would be a user-visible change in kernel policy and would break a large number of systems...)
I think that is the only reason it is sticking around.
thanks,
greg k-h
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
In reply to Greg KHQuoted 9 lines · Greg KH
Quoted 4 lines · Andy Green
For sure, in any other bus, we will never have this discussion about if the device driver should use relevant
platform_datato be guided about what to do, that is whatplatform_datais for and it is very widely used indeed.Are you sure? Try me on one of the other "real" busses out there like PCI and see how all of these same arguments are identical :)
You were asking for concrete examples up thread, and this brings one to mind - I used to work on a system where we had to modify the PCI ethernet controller driver to tell it that while it did have two PHYs attached to the MDIO bus they were nothing to do with the data interface of the ethernet controller which was instead directly wired to a fixed configuration device. This sort of thing could be handled by the device tree but...
Quoted 3 lines
The abuse of
platform_datatoday does not give you the right to persist in it. Look at the work of the device tree developers to fix this problem as proof of this.
...the problem with device tree is that it's only available on a limited subset of architectures at the minute, with progress towards deploying it more widely being somewhat slow.
Re: [RFC PATCH 3/4] PLATFORM: Introduce async platform_data attach api
In reply to Greg KHQuoted 10 lines · Somebody in the thread at some point
Quoted 8 lines
The first issue for usbnet is right now, it has no real insight into what the appropriate interface name is because it is out of its view if the USB Ethernet bridge is soldered onboard or not. You know, it is also not in a generic userspace's view what is soldered on that board either, so I can only read your pushing of that 'solution' as "get this out of my hair". The one place that can properly know it right now is the board definition file in the kernel, maybe Device Tree too in the future.
It's not up to the driver to "know" this at all.
Right, it's a matter for the board definition file to know about the board. Not sure how many times I wrote that on this thread, but evidently not enough. It can then send specific functional configuration information to the subsystems on the board so they can adapt automatically to that environment without stinking up everything with private quirk information per-board in the drivers. That is the idea of platform_data and it works really successfully.
It's pointless for you to talk about Device Tree when we'll allegedly have the capability to deliver functional configuration data to USB devices from board support files, but you claim it's forever evil to use it, so what's the point of discussing it? I dunno why you're bothering and it's getting less interesting the more distorted your argumentation is becoming trying to hold the line against fixing the drivers.
Quoted 9 lines
Quoted 8 lines
To do it what you are calling the "correct" way in userland, you are okay with the driver selecting the wrong interface name, condemning userland to regenerate board-specific knowledge from grepping
/proc/cpuinfo, inferring in userland what can easily and justly be known in the board definition file in kernel, and overriding the wrong interface name. In all that, Caesar's hands are clean alright, but don't tell me this is a good job and the correct solution.
Quoted 3 lines
It really is. How do you know that I don't want to name this device something else than you feel you should? Do you really want to tell me to recompile my kernel just to change the name of the network device?
You know by default if it's going to do it, it ought to try to be correct if it's possible, it sounds strange how lassaiz-faire you are about these drivers doing the wrong thing. As Alan pointed out that you should either do it well or not bother to do it and have all network devices called xxxN until your "wonderful" and "simple" userland rename is mandated. Never mind userland will have to hold a board quirk database and query the driver to find out which is WLAN or whatever: simplicity.
Nobody said about removing network interface naming, what we're talking about is making the driver a touch better so it does the right thing first time, adaptively to its platform.
Hey, did you see the smsc95xx driver is configuring its mac address from an EEPROM if it's connected instead of letting userland do it, holy crap. I bet those guys who get their MAC address automagically are super upset the kernel is cheating userland of some simple wonderfulness.
Ha ha no, just kidding.
Thanks to the other guys for giving their opinions frankly -- I am particularly appreciate Mark got the idea immediately -- and I hope everyone has a nice evening.
-Andy
RFC PATCH 4/4 PLATFORM: Add some documentation to platform docs about async platform_data
In reply to Andy Green---
Documentation/driver-model/platform.txt | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-)
Documentation/driver-model/platform.txt +25 -0
@@ -96,6 +96,31 @@ System setup also associates those clocks with the device, so that that calls to clk_get(&pdev->dev, clock_name) return them as needed. +Providing platform_data to onboard devices on asynchronously probed buses +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Some boards have fixed onboard assets that would normally be dealt with +by using plaform_data set in the usual way in the board definition file, +but cannot directly do so because they are on a bus that is probed +asynchronously. For emebedded devices, often the broard definition +file is adding platform devices for the buses involved, like USB host +and gadget, in a fixed order using platform_add_devices(), so the device +path of these fixed soldered-on-the-board assets is deterministic. + +For buses and devices that are named deterministically at boot-time, +you can define platform_data that binds to these devices when they are +probed by declaring a map of device paths to platform_data in your +board definition file before adding the bus platform devices, using + + void platform_register_async_platform_data( + struct platform_async_platform_data *map, int count); + +Buses that participate in this scheme will then check this mapping list +for corresponding platform_data as they are probed and apply it +automatically. An example device path mapping is + + "usb1/1-1/1-1.1" + + Legacy Drivers: Device Probing ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Some drivers are not fully converted to the driver model, because they take