On Tue, Jun 3, 2014 at 8:39 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 2 Jun 2014, Dan Williams wrote:
>
>> A rather asymmetric response to the compile breakage reported by -next
>> and the kbuild robot:
>>
>> drivers/usb/core/hub.c: In function 'port_event':
>> drivers/usb/core/hub.c:4853:2: error: implicit declaration of function
>> 'hub_handle_remote_wakeup' [-Werror=implicit-function-declaration]
>>
>> Caused by commits af376a461cf0 ("usb: refactor port handling in
>> hub_events()") and 7e73be227b15 ("usb: hub_handle_remote_wakeup()
>> depends on CONFIG_PM_RUNTIME=y") This build has CONFIG_PM not set ...
>>
>> Move all code in hub.c that is #ifdef CONFIG_PM guarded to hub_pm.c. It
>> ends up needing to mark 11 routines in hub.c as global so that hub_pm.c
>> can consume them. It's not perfect as it still requires #ifdef
>> CONFIG_PM_RUNTIME in hub_pm.c itself, but certainly gets us closer to
>> the "no ifdef in .c files" ideal.
>
> Mention also that some declarations (for routines not used outside of
> usbcore) got moved from include/linux/usb.h to drivers/usb/core/usb.h.
>
>> Alan: "The general idea seems okay."
>> http://marc.info/?l=linux-usb&m=140146126207527&w=2
>
> Not really needed for the patch description.
>
>
>> --- /dev/null
>> +++ b/drivers/usb/core/hub_pm.c
>
> ...
>
>> +int usb_disable_ltm(struct usb_device *udev)
>> +{
>> + struct usb_hcd *hcd = bus_to_hcd(udev->bus);
>> +
>> + /* Check if the roothub and device supports LTM. */
>> + if (!usb_device_supports_ltm(hcd->self.root_hub) ||
>> + !usb_device_supports_ltm(udev))
>> + return 0;
>> +
>> + /* Clear Feature LTM Enable can only be sent if the device is
>> + * configured.
>> + */
>> + if (!udev->actconfig)
>> + return 0;
>> +
>> + return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>> + USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
>> + USB_DEVICE_LTM_ENABLE, 0, NULL, 0,
>> + USB_CTRL_SET_TIMEOUT);
>> +}
>> +EXPORT_SYMBOL_GPL(usb_disable_ltm);
>
> This is one of the routines whose declaration got moved out of
> include/linux/usb.h. Since the declarations aren't available to code
> outside of usbcore, the functions shouldn't be EXPORTed.
I caught some of these cases, but yes, I'll audit for any others.
>
>> +#ifdef CONFIG_PM_RUNTIME
> ...
>> +#else
>> +
>> +static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
>> + u16 portstatus, u16 portchange)
>> +{
>> + return 0;
>> +}
>> +
>> +#endif
>
> Not needed any more.
>
True.
>> --- a/include/linux/usb.h
>> +++ b/include/linux/usb.h
>> @@ -690,15 +690,6 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
>> { }
>> #endif
>>
>> -extern int usb_disable_lpm(struct usb_device *udev);
>> -extern void usb_enable_lpm(struct usb_device *udev);
>> -/* Same as above, but these functions lock/unlock the bandwidth_mutex. */
>> -extern int usb_unlocked_disable_lpm(struct usb_device *udev);
>> -extern void usb_unlocked_enable_lpm(struct usb_device *udev);
>> -
>> -extern int usb_disable_ltm(struct usb_device *udev);
>> -extern void usb_enable_ltm(struct usb_device *udev);
>> -
>> static inline bool usb_device_supports_ltm(struct usb_device *udev)
>> {
>> if (udev->speed != USB_SPEED_SUPER || !udev->bos || !udev->bos->ss_cap)
>
> Why didn't you move this definition along the declarations above?
>
Oversight.
--
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
