Re: RFC: Platform data for onboard USB assets

From: Andy Green
Date: Tue Mar 22 2011 - 18:37:53 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/