On 06/12/12 00:42, the mail apparently from Alan Stern included:
On Wed, 5 Dec 2012, Andy Green wrote:
The details of this aren't clear yet. For instance, should the device
core try to match the port with the asset info, or should this be done
by the USB code when the port is created?
Currently what I have (this is before changing it to pm domain, but this
should be unchanged) lets the asset define a callback op which will
receive notifications for all added child devices that have the device
the asset is bound to as an ancestor.
So you would bind an asset to the host controller device...
static struct device_asset assets_ehci_omap0[] = {
{
.name = "-0:1.0/port1",
.asset = assets_ehci_omap0_smsc_port,
.ops = &device_descendant_attach_default_asset_ops,
},
{ }
};
...with this descendant filter callback pointing to a generic "end of
the device path" matcher.
when device_descendant_attach_default_asset_ops() sees the child that
was foretold has appeared (and it only hears about children of
ehci-omap.0 in this case), it binds the assets pointed to by .asset to
the new child before its probe. assets_ehci_omap0_smsc_port is an array
of the actual regulator and clock that need switching by the child. So
the effect is to magic the right assets to the child device just before
it gets added (and probe called etc).
This is working well and the matcher helper is small and simple.
Right. The question is should it be done in this somewhat roundabout
way (you've got two separate assets for setting up this one thing), or
should the USB port code be responsible for explicitly checking if
there are any applicable assets when the port is created?
The advantange of the second approach is that it doesn't involve
checking all the ancestors every time a new device is added (and it
involves only one asset). The disadvantage is that it works only for
USB ports.
It's done in two steps to strongly filter candidate child devices
against being children of a known platform device. If you go around
that, you will be back to full device path matching with wildcards at
the USB child to determine if he is "the one". So that's a feature not
an issue I think.
I can see doing it generically or not is equally a political issue as a
technical one, but I think if we bother to add this kind of support we
should prefer to do it generally. It's going to be about the same
amount of code.
As Tony Lindgren said, even board-omap4panda.c itself is deprecated, to
project platform info into USB stack you either have to add DT code into
usb stack then to go find things directly, or provide a generic
methodology like assets which have the dt bindings done outside of any
subsystem and apply their operations outside the subsystem too.
To answer the question, we need to know how other subsystems might
want to use the asset-matching approach. My guess is that only a small
number of device types would care about assets (in the same way that
assets matter to USB ports but not to other USB device types). And it
might not be necessary to check against every ancestor; we might know
beforehand where to check (just as the USB port would know to look for
an asset attached to the host controller device).
Yes I think it boils down to "buses" in general can benefit from this.
They're the thing that dynamically - later - create child devices you
might need to target with what was ultimately platform information.
On Panda the other bus thing that can benefit is the WLAN module on
SDIO. In fact that's a very illuminating case for your question above.
Faced with exactly the same problem, that they needed to project
platform info on to SDIO-probed device instance to tell it what clock
rate it had been given, they invented an api which you can see today in
board-omap4panda.c and other boards there, wl12xx_set_platform_data().
This is from board-4430sdp:
static struct wl12xx_platform_data omap4_sdp4430_wlan_data __initdata = {
.board_ref_clock = WL12XX_REFCLOCK_26,
.board_tcxo_clock = WL12XX_TCXOCLOCK_26,
};
...
ret = wl12xx_set_platform_data(&omap4_sdp4430_wlan_data);
You can find the other end of it here in
drivers/net/wireless/ti/wlcore/wl12xx_platform_data.c
static struct wl12xx_platform_data *platform_data;
int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
{
if (platform_data)
return -EBUSY;
if (!data)
return -EINVAL;
platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
if (!platform_data)
return -ENOMEM;
return 0;
}
when the driver for the device instance wants to "get" its platform data
it reads the static pointer via another api. Obviously if you want two
modules on your platform that's not so good.
I doubt that's the only SDIO device that wants to know platform info.
So I think by providing a generic way we help other buses and devices.
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
