[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150422092110.GA19596@intel.com>
Date: Wed, 22 Apr 2015 17:21:10 +0800
From: Zhuang Jin Can <jin.can.zhuang@...el.com>
To: gregkh@...uxfoundation.org, rafael.j.wysocki@...el.com,
stern@...land.harvard.edu, dan.j.williams@...el.com,
pmladek@...e.cz, peter.chen@...escale.com, jwerner@...omium.org,
linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org, mathias.nyman@...ux.intel.com,
david.a.cohen@...ux.intel.com
Subject: Re: [PATCH] usb: core: add usb3 lpm sysfs
+ Mathias and David.
Hi Mathias,
Please help to review this patch.
Thanks
Jincan
On Sun, Apr 19, 2015 at 11:46:12AM +0800, Zhuang Jin Can wrote:
> Some usb3 devices may not support usb3 lpm well.
> The patch adds a sysfs to enable/disable u1 or u2 of the port.The
> settings apply to both before and after device enumeration.
> Supported values are "0" - u1 and u2 are disabled, "u1" - only u1 is
> enabled, "u2" - only u2 is enabled, "u1_u2" - u1 and u2 are enabled.
>
> The interface is useful for testing some USB3 devices during
> development, and provides a way to disable usb3 lpm if the issues can
> not be fixed in final products.
>
> Signed-off-by: Zhuang Jin Can <jin.can.zhuang@...el.com>
> ---
> Documentation/ABI/testing/sysfs-bus-usb | 10 ++++
> drivers/usb/core/hub.c | 16 +++++-
> drivers/usb/core/hub.h | 4 ++
> drivers/usb/core/port.c | 89 ++++++++++++++++++++++++++++++-
> 4 files changed, 116 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
> index e5cc763..32fc689 100644
> --- a/Documentation/ABI/testing/sysfs-bus-usb
> +++ b/Documentation/ABI/testing/sysfs-bus-usb
> @@ -179,3 +179,13 @@ Description:
> Supported values are 0 - 15.
> More information on how besl values map to microseconds can be found in
> USB 2.0 ECN Errata for Link Power Management, section 4.10)
> +
> +What: /sys/bus/usb/devices/.../(hub interface)/portX/usb3_lpm
> +Date: March 2015
> +Contact: Zhuang Jin Can <jin.can.zhuang@...el.com>
> +Description:
> + Some USB3.0 devices may not support usb3 lpm well.
> + usb3_lpm attribute allows enabling/disabling usb3 lpm of the port.
> + It takes effect both before and after a usb device is enumerated.
> + Supported values are "0" if u1 and u2 are disabled, "u1" if only u1 is
> + enabled, "u2" if only u2 is enabled, "u1_u2" if u1 and u2 are enabled.
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index d7c3d5a..7732a41 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -3996,6 +3996,8 @@ EXPORT_SYMBOL_GPL(usb_unlocked_disable_lpm);
> void usb_enable_lpm(struct usb_device *udev)
> {
> struct usb_hcd *hcd;
> + struct usb_hub *hub;
> + struct usb_port *port_dev;
>
> if (!udev || !udev->parent ||
> udev->speed != USB_SPEED_SUPER ||
> @@ -4015,8 +4017,18 @@ void usb_enable_lpm(struct usb_device *udev)
> if (udev->lpm_disable_count > 0)
> return;
>
> - usb_enable_link_state(hcd, udev, USB3_LPM_U1);
> - usb_enable_link_state(hcd, udev, USB3_LPM_U2);
> + hub = usb_hub_to_struct_hub(udev->parent);
> + if (!hub) {
> + dev_err(&udev->dev, "can't enable lpm, usb_hub is null.\n");
> + return;
> + }
> + port_dev = hub->ports[udev->portnum - 1];
> +
> + if (port_dev->u1_is_enabled)
> + usb_enable_link_state(hcd, udev, USB3_LPM_U1);
> +
> + if (port_dev->u2_is_enabled)
> + usb_enable_link_state(hcd, udev, USB3_LPM_U2);
> }
> EXPORT_SYMBOL_GPL(usb_enable_lpm);
>
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 688817f..1ae060e 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -92,6 +92,8 @@ struct usb_hub {
> * @status_lock: synchronize port_event() vs usb_port_{suspend|resume}
> * @portnum: port index num based one
> * @is_superspeed cache super-speed status
> + * @u1_is_enabled: whether u1 should be enabled.
> + * @u2_is_enabled: whether u2 should be enabled.
> */
> struct usb_port {
> struct usb_device *child;
> @@ -104,6 +106,8 @@ struct usb_port {
> struct mutex status_lock;
> u8 portnum;
> unsigned int is_superspeed:1;
> + unsigned int u1_is_enabled:1;
> + unsigned int u2_is_enabled:1;
> };
>
> #define to_usb_port(_dev) \
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 2106183..7f4e6c7 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -50,6 +50,72 @@ static ssize_t connect_type_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(connect_type);
>
> +static ssize_t usb3_lpm_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> + const char *p;
> +
> + if (port_dev->u1_is_enabled) {
> + if (port_dev->u2_is_enabled)
> + p = "u1_u2";
> + else
> + p = "u1";
> + } else {
> + if (port_dev->u2_is_enabled)
> + p = "u2";
> + else
> + p = "0";
> + }
> +
> + return sprintf(buf, "%s\n", p);
> +}
> +
> +static ssize_t usb3_lpm_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> + struct usb_device *udev = port_dev->child;
> + struct usb_hcd *hcd;
> +
> + if (!strncmp(buf, "u1_u2", 5)) {
> + port_dev->u1_is_enabled = 1;
> + port_dev->u2_is_enabled = 1;
> +
> + } else if (!strncmp(buf, "u1", 2)) {
> + port_dev->u1_is_enabled = 1;
> + port_dev->u2_is_enabled = 0;
> +
> + } else if (!strncmp(buf, "u2", 2)) {
> + port_dev->u1_is_enabled = 0;
> + port_dev->u2_is_enabled = 1;
> +
> + } else if (!strncmp(buf, "0", 1)) {
> + port_dev->u1_is_enabled = 0;
> + port_dev->u2_is_enabled = 0;
> + } else
> + return -EINVAL;
> +
> + /* If device is connected to the port, disable & enable lpm
> + * to make new u1 u2 setting take effect immediately
> + */
> + if (udev) {
> + hcd = bus_to_hcd(udev->bus);
> + if (!hcd)
> + return -EINVAL;
> + usb_lock_device(udev);
> + mutex_lock(hcd->bandwidth_mutex);
> + if (!usb_disable_lpm(udev))
> + usb_enable_lpm(udev);
> + mutex_unlock(hcd->bandwidth_mutex);
> + usb_unlock_device(udev);
> + }
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(usb3_lpm);
> +
> static struct attribute *port_dev_attrs[] = {
> &dev_attr_connect_type.attr,
> NULL,
> @@ -64,6 +130,21 @@ static const struct attribute_group *port_dev_group[] = {
> NULL,
> };
>
> +static struct attribute *port_dev_usb3_attrs[] = {
> + &dev_attr_usb3_lpm.attr,
> + NULL,
> +};
> +
> +static struct attribute_group port_dev_usb3_attr_grp = {
> + .attrs = port_dev_usb3_attrs,
> +};
> +
> +static const struct attribute_group *port_dev_usb3_group[] = {
> + &port_dev_attr_grp,
> + &port_dev_usb3_attr_grp,
> + NULL,
> +};
> +
> static void usb_port_device_release(struct device *dev)
> {
> struct usb_port *port_dev = to_usb_port(dev);
> @@ -401,6 +482,7 @@ static void find_and_link_peer(struct usb_hub *hub, int port1)
> int usb_hub_create_port_device(struct usb_hub *hub, int port1)
> {
> struct usb_port *port_dev;
> + struct usb_device *hdev = hub->hdev;
> int retval;
>
> port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL);
> @@ -417,7 +499,12 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
> port_dev->portnum = port1;
> set_bit(port1, hub->power_bits);
> port_dev->dev.parent = hub->intfdev;
> - port_dev->dev.groups = port_dev_group;
> + if (hub_is_superspeed(hdev)) {
> + port_dev->u1_is_enabled = 1;
> + port_dev->u2_is_enabled = 1;
> + port_dev->dev.groups = port_dev_usb3_group;
> + } else
> + port_dev->dev.groups = port_dev_group;
> port_dev->dev.type = &usb_port_device_type;
> port_dev->dev.driver = &usb_port_driver;
> if (hub_is_superspeed(hub->hdev))
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists