On Tue, Feb 14, 2012 at 10:06:47AM -0500, Alan Stern wrote: > On Tue, 14 Feb 2012, Felipe Balbi wrote: > > > not all platforms will use all of those ehci_* > > symbols on their hc_driver structure. Sometimes > > we might need to provide a modified version of > > a certain method or not provide it at all, as is > > the case with OMAPs which don't support port handoff > > feature. > > > > Whenever we compile a kernel for an OMAP board with > > EHCI enabled, we get compile warnings: > > > > drivers/usb/host/ehci-hub.c:1079: warning: 'ehci_relinquish_port' \ > > defined but not used > > drivers/usb/host/ehci-hub.c:1088: warning: 'ehci_port_handed_over' \ > > defined but not used > > > > In order to cleanup those warnings, we're adding > > __maybe_unused annotation to those functions. We're > > also trying to cope with other platforms which might > > have similar issues with different methods by adding > > the same annotation to all ehci_* functions. > > > > Signed-off-by: Felipe Balbi <balbi@xxxxxx> > > --- > > > > Alan, what do you think about this patch ? It fixes our warnings > > and tries to cope with other possible such cases. > > Some of this makes sense: ehci_port_change, ehci_relinquish_port, > and ehci_port_handed_over are fine. > > The others are questionable. What reason could there be for not > implementing ehci_bus_suspend and ehci_bus_resume? Neither of those > should involve much platform-specific work, if any. you never know what kind of silicon erratas might come up, right ? :-) > ehci_hub_status_data, ehci_hub_descriptor, and ehci_hub_control are > even more puzzling. These are things that _have_ to be implemented for > the driver to work at all. Platform code shouldn't replace them; it > most it should call them as subroutines and override the results when > necessary. There's no doubt it needs to be implemented, but the fact is that in some situations we might need to implement it differently because of whatever reason. See for example that I have a pending workaround to implement for OMAP3 EHCI where we need to reparent a clock before handling SetFeature(port_suspend) so I would need to reimplement echi_hub_control() in order not to expose that detail to core ehci. Because clock reparenting is something so ARCH-specific it makes no sense to come up with a "quirk" flag for that. Granted, the way I had envisioned, I would reimplement the particular request that needs special handling and call the generic ehci_hub_control() otherwise, but still. Like I said, you never know what kind of erratas could pop up. -- balbi
Attachment:
signature.asc
Description: Digital signature
