On Fri, 28 Feb 2014, Dan Williams wrote:
> The current port name "portX" is ambiguous. Before adding more port
> messages rename ports to "<hub-device-name>-portX"
>
> This is an ABI change, but the suspicion is that it will go unnoticed as
> the port power control implementation has been broken since its
> introduction. If however, someone was relying on the old name we can
> add sysfs links from the old name to the new name.
>
> Additionally, it unifies/simplifies port dev_printk messages and modifies
> instances of:
> dev_XXX(hub->intfdev, ..."port %d"...
> dev_XXX(&hdev->dev, ..."port%d"...
> into:
> dev_XXX(&port_dev->dev, ...
>
> Now that the names are unique usb_port devices it would be nice if they
> could be included in /sys/bus/usb. However, it turns out that this
> breaks 'lsusb -t'. For now, create a dummy port driver so that print
> messages are prefixed "usb 1-1-port3" rather than the
> subsystem-ambiguous " 1-1-port3".
>
> Finally, it corrects an odd usage of sscanf("port%d") in usb-acpi.c.
>
> Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
This looks all right.
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 4e967f613e70..6014a790441f 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -408,26 +408,25 @@ static int set_port_feature(struct usb_device *hdev, int port1, int feature)
> * USB 2.0 spec Section 11.24.2.7.1.10 and table 11-7
> * for info about using port indicators
> */
> -static void set_port_led(
> - struct usb_hub *hub,
> - int port1,
> - int selector
> -)
> +static void set_port_led(struct usb_hub *hub, int port1, int selector)
> {
> - int status = set_port_feature(hub->hdev, (selector << 8) | port1,
> + struct usb_port *port_dev = hub->ports[port1 - 1];
> + int status;
> + char *led;
> +
> + status = set_port_feature(hub->hdev, (selector << 8) | port1,
> USB_PORT_FEAT_INDICATOR);
> - if (status < 0)
> - dev_dbg (hub->intfdev,
> - "port %d indicator %s status %d\n",
> - port1,
> - ({ char *s; switch (selector) {
> - case HUB_LED_AMBER: s = "amber"; break;
> - case HUB_LED_GREEN: s = "green"; break;
> - case HUB_LED_OFF: s = "off"; break;
> - case HUB_LED_AUTO: s = "auto"; break;
> - default: s = "??"; break;
> - } s; }),
> - status);
> + if (selector == HUB_LED_AMBER)
> + led = "amber";
> + else if (selector == HUB_LED_GREEN)
> + led = "green";
> + else if (selector == HUB_LED_OFF)
> + led = "off";
> + else if (selector == HUB_LED_AUTO)
> + led = "auto";
> + else
> + led = "??";
This ought to be a "switch" statement. Otherwise,
Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Alan Stern
--
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
