On Monday 25 March 2013, Alan Stern wrote: > On Mon, 25 Mar 2013, Felipe Balbi wrote: > > > this ehci_platform_defaults is quite a hack. Would be much better to see > > a proper re-factoring of the code so that it actually learns about DT > > and platform_data. > > > > So, if dev->dev.platform_data is NULL, you shouldn't error, rather you > > should just assume the default, rather than this quick little hack. > > > > Alan has final saying though. > > IMO, using ehci_platform_defaults is a way of assuming the default. > In other words, it's not a bad hack. I'm okay with this this approach > (in fact, it was my suggestion originally). I intentionally did not add any properties for the fields in the current usb_ehci_pdata. It's not possible to implement the callbacks using just DT, and the flags are all defined so that 'false' is the default that happens to be used by the compatible="wm,prizm-ehci" binding. We can easily extend this driver to also cover the flags or the caps_offset, or we can add support for regulators and clocks, but I would prefer to do those only when we actually add support for a device that needs them. > On the other hand, it would be nice to have a clearer way of indicating > that the driver was invoked because of a DT match, something better > than just noticing that dev->dev.platform_data is NULL. But I guess > this is a legitimate option even for regular platform drivers -- if > they don't have any specific requirements, they may as well pass a NULL > pointer instead of a pointer to an empty structure. I'm certainly fine doing it either way: leave the defaults for pdata=NULL or require either a DT match or a pdata pointer (or both, in case of auxdata, I guess). Arnd -- 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