Re: [PATCH] usb: host: ehci: allow ehci_* symbols to be unused

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]

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@ti.com>
> > ---
> > 
> > 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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]