Re: [PATCH 2/2] typec: tcpm: Provide fwnode pointer as part of psy_cfg
On Tue, May 22, 2018 at 04:16:24PM +0100, Adam Thomson wrote: > For supply registration, provide fwnode pointer of the port device, > via the power_supply_config structure, to allow other psy drivers > to add us as a supplier. At present this only applies to DT > based platforms using the 'power-supplies' DT property, but in the > future should also work for ACPI platforms when the relevant support > is added to the power_supply core. > > Signed-off-by: Adam Thomson > Suggested-by: Heikki Krogerus Reviewed-by: Heikki Krogerus > --- > drivers/usb/typec/tcpm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > index 72996cc..0ccd2ce 100644 > --- a/drivers/usb/typec/tcpm.c > +++ b/drivers/usb/typec/tcpm.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -4500,6 +4501,7 @@ static int devm_tcpm_psy_register(struct tcpm_port > *port) > char *psy_name; > > psy_cfg.drv_data = port; > + psy_cfg.fwnode = dev_fwnode(port->dev); > psy_name = devm_kzalloc(port->dev, psy_name_len, GFP_KERNEL); > if (!psy_name) > return -ENOMEM; Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
Hi Rob, Thank you for the review! > From: Rob Herring, Sent: Wednesday, May 23, 2018 2:13 AM > > On Tue, May 22, 2018 at 09:01:07PM +0900, Yoshihiro Shimoda wrote: > > This patch adds role switch support for R-Car SoCs into the USB 3.0 > > peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0 > > dual-role device controller which has the USB 3.0 xHCI host and > > Renesas USB 3.0 peripheral. > > > > Unfortunately, the mode change register contains the USB 3.0 peripheral > > controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) > > manages this register now. However, in peripheral mode, the host > > should stop. Also the host hardware needs to reinitialize its own > > registers when the mode changes from peripheral to host mode. > > Otherwise, the host cannot work correctly (e.g. detect a device as > > high-speed). > > > > To achieve this by a driver, this role switch driver manages > > the mode change register and attach/release the xhci-plat driver. > > > > Signed-off-by: Yoshihiro Shimoda > > --- > > .../devicetree/bindings/usb/renesas_usb3.txt | 15 > > Please split bindings to a separate patch. Oops. I got it. > > drivers/usb/gadget/udc/Kconfig | 1 + > > drivers/usb/gadget/udc/renesas_usb3.c | 82 > > ++ > > 3 files changed, 98 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > index 2c071bb5..f6105aa 100644 > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > @@ -19,6 +19,9 @@ Required properties: > > Optional properties: > >- phys: phandle + phy specifier pair > >- phy-names: must be "usb" > > + - The connection to a usb3.0 host node needs by using OF graph bindings > > for > > +usb role switch. > > + - port@0 = USB3.0 host port. > > On the host side, this might conflict with the USB connector binding. > > I would either make sure this can work with the connector binding by > having 2 endpoints on the HS or SS port or just use the 'companion' > property defined in usb-generic.txt. I don't understand the first one now... This means the renesas_usb3 should follow USB connector binding and have 2 endpoints for the usb role switch to avoid the conflict like below? - port1@0: Super Speed (SS), present in SS capable connectors (from usb-connector.txt). - port1@1: USB3.0 host port. About the 'companion' in usb-generic.txt, the property intends to be used for EHCI or host side like the commit log [1]. If there is accept to use 'companion' for this patch, I think it will be simple to achieve this role switch feature. However, in last month, I submitted a similar patch [2] that has "renesas,host" property, but I got reply from Andy [3] and Heikki [4]. So, I'm trying to improve the device connection framework [5] now. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/usb/generic.txt?h=v4.17-rc6&id=5095cb89c62acc78b4cfaeb9a4072979d010510a [2] https://www.spinics.net/lists/linux-usb/msg167773.html [3] https://www.spinics.net/lists/linux-usb/msg167780.html [4] https://www.spinics.net/lists/linux-usb/msg167806.html [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/device_connection.rst Best regards, Yoshihiro Shimoda > Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] power: supply: Add fwnode pointer to power_supply_config struct
On Tue, May 22, 2018 at 04:16:23PM +0100, Adam Thomson wrote: > To allow users of the power supply framework to be hw description > agnostic, this commit adds the ability to pass a fwnode pointer, > via the power_supply_config structure, to the initialisation code > of the core, instead of explicitly specifying of_ndoe. If that > fwnode pointer is provided then it will automatically resolve down > to of_node on platforms which support it, otherwise it will be NULL. > > In the future, when ACPI support is added, this can be modified to > accommodate ACPI without the need to change calling code which > already provides the fwnode handle in this manner. > > Signed-off-by: Adam Thomson > Suggested-by: Heikki Krogerus Thanks Adam! FWIW: Reviewed-by: Heikki Krogerus > --- > drivers/power/supply/power_supply_core.c | 4 +++- > include/linux/power_supply.h | 2 ++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/power_supply_core.c > b/drivers/power/supply/power_supply_core.c > index ecd68c2..f57ab0a 100644 > --- a/drivers/power/supply/power_supply_core.c > +++ b/drivers/power/supply/power_supply_core.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include "power_supply.h" > > @@ -874,7 +875,8 @@ static void psy_unregister_cooler(struct power_supply > *psy) > psy->desc = desc; > if (cfg) { > psy->drv_data = cfg->drv_data; > - psy->of_node = cfg->of_node; > + psy->of_node = > + cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node; > psy->supplied_to = cfg->supplied_to; > psy->num_supplicants = cfg->num_supplicants; > } > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h > index 0c9a572..b21c4bd9 100644 > --- a/include/linux/power_supply.h > +++ b/include/linux/power_supply.h > @@ -199,6 +199,8 @@ enum power_supply_notifier_events { > /* Run-time specific power supply configuration */ > struct power_supply_config { > struct device_node *of_node; > + struct fwnode_handle *fwnode; > + > /* Driver private data */ > void *drv_data; Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: hub: Per-port setting to use old enumeration scheme
The "old" enumeration scheme is considerably faster (it takes ~294ms instead of ~439ms to get the descriptor). It is currently only possible to use the old scheme globally (/sys/module/usbcore/parameters/old_scheme_first), which is not desirable as the new scheme was introduced to increase compatibility with more devices. However, in our case, we care about time-to-active for a specific USB device (which we make the firmware for), on a specific port (that is pogo-pin based: not a standard USB port). This new sysfs option makes it possible to use the old scheme on a single port only. Signed-off-by: Nicolas Boichat --- There are other "quirks" that we could add to reduce further enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY to 10ms instead of 50ms as used currently), but the logic is quite similar, so it'd be good to have this reviewed first. drivers/usb/core/hub.c | 13 + drivers/usb/core/hub.h | 1 + drivers/usb/core/port.c | 23 +++ include/linux/usb.h | 7 +++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index c2d993d3816f0..f900f66a62856 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2636,7 +2636,7 @@ static unsigned hub_is_wusb(struct usb_hub *hub) #define SET_ADDRESS_TRIES 2 #define GET_DESCRIPTOR_TRIES 2 #define SET_CONFIG_TRIES (2 * (use_both_schemes + 1)) -#define USE_NEW_SCHEME(i) ((i) / 2 == (int)old_scheme_first) +#define USE_NEW_SCHEME(i, scheme) ((i) / 2 == (int)scheme) #define HUB_ROOT_RESET_TIME60 /* times are in msec */ #define HUB_SHORT_RESET_TIME 10 @@ -2651,12 +2651,16 @@ static unsigned hub_is_wusb(struct usb_hub *hub) * enumeration failures, so disable this enumeration scheme for USB3 * devices. */ -static bool use_new_scheme(struct usb_device *udev, int retry) +static bool use_new_scheme(struct usb_device *udev, int retry, + struct usb_port *port_dev) { + int old_scheme_first_port = + port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME; + if (udev->speed >= USB_SPEED_SUPER) return false; - return USE_NEW_SCHEME(retry); + return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first); } /* Is a USB 3.0 port in the Inactive or Compliance Mode state? @@ -4392,6 +4396,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, { struct usb_device *hdev = hub->hdev; struct usb_hcd *hcd = bus_to_hcd(hdev->bus); + struct usb_port *port_dev = hub->ports[port1 - 1]; int retries, operations, retval, i; unsigneddelay = HUB_SHORT_RESET_TIME; enum usb_device_speed oldspeed = udev->speed; @@ -4513,7 +4518,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, for (retries = 0; retries < GET_DESCRIPTOR_TRIES; (++retries, msleep(100))) { bool did_new_scheme = false; - if (use_new_scheme(udev, retry_counter)) { + if (use_new_scheme(udev, retry_counter, port_dev)) { struct usb_device_descriptor *buf; int r = 0; diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index 4dc769ee9c740..4accfb63f7dcb 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -98,6 +98,7 @@ struct usb_port { struct mutex status_lock; u32 over_current_count; u8 portnum; + u32 quirks; unsigned int is_superspeed:1; unsigned int usb3_lpm_u1_permit:1; unsigned int usb3_lpm_u2_permit:1; diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 6979bde87d310..4a21431953953 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -50,6 +50,28 @@ static ssize_t over_current_count_show(struct device *dev, } static DEVICE_ATTR_RO(over_current_count); +static ssize_t quirks_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct usb_port *port_dev = to_usb_port(dev); + + return sprintf(buf, "%08x\n", port_dev->quirks); +} + +static ssize_t quirks_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct usb_port *port_dev = to_usb_port(dev); + u32 value; + + if (kstrtou32(buf, 16, &value)) + return -EINVAL; + + port_dev->quirks = value; + return count; +} +static DEVICE_ATTR_RW(quirks); + static ssize_t usb3_lpm_permit_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -118,6 +140,7 @@ static DEVICE_ATTR_RW(usb3_lpm_permit); static struct attribute *port_dev_attrs[] = { &dev_attr_connect_type.attr, + &dev_attr_quirks.attr, &dev_attr_over_current_count.
Re: [PATCH v1 2/4] usb: dwc2: Modify dwc2_readl/writel functions prototype
Hi Gevorg, Thank you for the patch! Yet something to improve: [auto build test ERROR on usb/usb-testing] [also build test ERROR on v4.17-rc6] [cannot apply to balbi-usb/next next-20180517] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Gevorg-Sahakyan/usb-dwc2-Make-dwc2-endianness-agnostic/20180523-062909 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: i386-randconfig-x078-201820 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/usb/dwc2/gadget.c: In function 'dwc2_hsotg_ep_enable': >> drivers/usb/dwc2/gadget.c:3943:17: error: too few arguments to function >> 'dwc2_readl' u32 gsnpsid = dwc2_readl(hsotg->regs + GSNPSID); ^~ In file included from drivers/usb/dwc2/gadget.c:31:0: drivers/usb/dwc2/core.h:1085:19: note: declared here static inline u32 dwc2_readl(struct dwc2_hsotg *hsotg, u32 offset) ^~ vim +/dwc2_readl +3943 drivers/usb/dwc2/gadget.c a4f827714 drivers/usb/dwc2/gadget.c Vahram Aharonyan 2016-11-14 3769 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3770 /** 1f91b4cc0 drivers/usb/dwc2/gadget.c Felipe Balbi 2015-08-06 3771 * dwc2_hsotg_ep_enable - enable the given endpoint 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3772 * @ep: The USB endpint to configure 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3773 * @desc: The USB endpoint descriptor to configure with. 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3774 * 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3775 * This is called from the USB gadget code's usb_ep_enable(). 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3776 */ 1f91b4cc0 drivers/usb/dwc2/gadget.c Felipe Balbi 2015-08-06 3777 static int dwc2_hsotg_ep_enable(struct usb_ep *ep, 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3778 const struct usb_endpoint_descriptor *desc) 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3779 { 1f91b4cc0 drivers/usb/dwc2/gadget.c Felipe Balbi 2015-08-06 3780 struct dwc2_hsotg_ep *hs_ep = our_ep(ep); 941fcce4f drivers/usb/dwc2/gadget.c Dinh Nguyen2014-11-11 3781 struct dwc2_hsotg *hsotg = hs_ep->parent; 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3782 unsigned long flags; ca4c55ad8 drivers/usb/dwc2/gadget.c Mian Yousaf Kaukab 2015-01-09 3783 unsigned int index = hs_ep->index; 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3784 u32 epctrl_reg; 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3785 u32 epctrl; 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3786 u32 mps; ee2c40de6 drivers/usb/dwc2/gadget.c Vardan Mikayelyan 2016-11-08 3787 u32 mc; 837e9f00b drivers/usb/dwc2/gadget.c Vardan Mikayelyan 2016-05-25 3788 u32 mask; ca4c55ad8 drivers/usb/dwc2/gadget.c Mian Yousaf Kaukab 2015-01-09 3789 unsigned int dir_in; ca4c55ad8 drivers/usb/dwc2/gadget.c Mian Yousaf Kaukab 2015-01-09 3790 unsigned int i, val, size; 19c190f9e drivers/usb/gadget/s3c-hsotg.c Julia Lawall 2010-03-29 3791 int ret = 0; 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3792 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3793 dev_dbg(hsotg->dev, 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3794 "%s: ep %s: a 0x%02x, attr 0x%02x, mps 0x%04x, intr %d\n", 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3795 __func__, ep->name, desc->bEndpointAddress, desc->bmAttributes, 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3796 desc->wMaxPacketSize, desc->bInterval); 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3797 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3798 /* not to be called for EP0 */ 8c3d60927 drivers/usb/dwc2/gadget.c Vahram Aharonyan 2016-04-27 3799 if (index == 0) { 8c3d60927 drivers/usb/dwc2/gadget.c Vahram Aharonyan 2016-04-27 3800 dev_err(hsotg->dev, "%s: called for EP 0\n", __func__); 8c3d60927 drivers/usb/dwc2/gadget.c Vahram Aharonyan 2016-04-27 3801 return -EINVAL; 8c3d60927 drivers/usb/dwc2/gadget.c Vahram Aharonyan 2016-04-27 3802 } 5b7d70c6d drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02 3803 5b7d70c6d dr
Re: [PATCH v2] usb: xhci: force all memory allocations to node
On 5/22/2018 6:46 PM, Timur Tabi wrote: > On 5/22/18 1:55 PM, Adam Wallis wrote: >> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; > > Since you only use 'dev' to get the NUMA node, you might want to consider this > instead: > > int node = dev_to_node(xhci_to_hcd(xhci)->self.sysdev); > My original thinking with using dev was such that future functionality might use dev...in fact one of the functions already had dev exposed. However, I don't feel strongly about this. I will go with whatever Mathias prefers. -- Adam Wallis Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usbip: dynamically allocate idev by nports found in sysfs
On 05/22/2018 11:04 AM, Michael Grzeschik wrote: > As the amount of available ports varies by the kernels build > configuration. To remove the limitation of the fixed 128 ports > we allocate the amount of idevs by using the number we get > from the kernel. > > Signed-off-by: Michael Grzeschik > --- > v1 -> v2: - reworked memory allocation into one calloc call > - added error path on allocation failure > > tools/usb/usbip/libsrc/vhci_driver.c | 14 +- > tools/usb/usbip/libsrc/vhci_driver.h | 3 +-- > 2 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/vhci_driver.c > b/tools/usb/usbip/libsrc/vhci_driver.c > index c9c81614a66ad..6e2a9edfd1f0d 100644 > --- a/tools/usb/usbip/libsrc/vhci_driver.c > +++ b/tools/usb/usbip/libsrc/vhci_driver.c > @@ -242,13 +242,20 @@ static int read_record(int rhport, char *host, unsigned > long host_len, > > int usbip_vhci_driver_open(void) > { > + int nports = get_nports(); > I missed this error leg in my previous comments. get_nports() could return errorwhich is -1. > udev_context = udev_new(); > if (!udev_context) { > err("udev_new failed"); > return -1; > } > > - vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver)); > + vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver) + > + nports * sizeof(struct usbip_imported_device)); nports could be -1 at this point. > + if (!vhci_driver) { > + err("vhci_driver allocation failed"); > + return -1; > + } > > /* will be freed in usbip_driver_close() */ > vhci_driver->hc_device = > @@ -260,15 +267,12 @@ int usbip_vhci_driver_open(void) > goto err; > } > > - vhci_driver->nports = get_nports(); > + vhci_driver->nports = nports; > dbg("available ports: %d", vhci_driver->nports); > > if (vhci_driver->nports <= 0) { > err("no available ports"); > goto err; This check should move up along with the get_nports() call. > - } else if (vhci_driver->nports > MAXNPORT) { > - err("port number exceeds %d", MAXNPORT); > - goto err; > } > > vhci_driver->ncontrollers = get_ncontrollers(); > diff --git a/tools/usb/usbip/libsrc/vhci_driver.h > b/tools/usb/usbip/libsrc/vhci_driver.h > index 418b404d51210..6c9aca2167051 100644 > --- a/tools/usb/usbip/libsrc/vhci_driver.h > +++ b/tools/usb/usbip/libsrc/vhci_driver.h > @@ -13,7 +13,6 @@ > > #define USBIP_VHCI_BUS_TYPE "platform" > #define USBIP_VHCI_DEVICE_NAME "vhci_hcd.0" > -#define MAXNPORT 128 > > enum hub_speed { > HUB_SPEED_HIGH = 0, > @@ -41,7 +40,7 @@ struct usbip_vhci_driver { > > int ncontrollers; > int nports; > - struct usbip_imported_device idev[MAXNPORT]; > + struct usbip_imported_device idev[]; > }; > > > thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: xhci: force all memory allocations to node
On 5/22/18 1:55 PM, Adam Wallis wrote: + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; Since you only use 'dev' to get the NUMA node, you might want to consider this instead: int node = dev_to_node(xhci_to_hcd(xhci)->self.sysdev); -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/18] dt-bindings: usb: add bindings doc for HiSilicon STB xHCI host controller
On Tue, May 22, 2018 at 8:31 AM, Mathias Nyman wrote: > On 22.05.2018 13:06, Greg KH wrote: >> >> On Mon, May 21, 2018 at 04:39:50PM +0300, Mathias Nyman wrote: >>> >>> From: Jianguo Sun >>> >>> This commit adds bindings doc for HiSilicon STB xHCI host controller. >>> >>> Signed-off-by: Jianguo Sun >>> Signed-off-by: Mathias Nyman >> >> >> Don't you need an ack from the DT maintainers for new bindings? >> > > True. The comment Rob Herring had to V2 of the HiSilicon patchseries > was fixed in V3 (this one), but V3 was never acked. > > Rob, did V3 look ok? > > https://marc.info/?l=linux-usb&m=152627889413412&w=2 Yes, somehow I marked it that I'd replied, but didn't. Reviewed-by: Rob Herring Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: Fix HiKey regression caused by power_down feature
On Tue, May 22, 2018 at 7:24 AM, Minas Harutyunyan wrote: > Hi John, > > Please provide log with debug enabled configuration. Ok. Attached. For dmesg-broken.log: OTG removed at 77, and OTG plugged in at 82 For dmesg-with-patch.log: OTG removed at 31, OTG plugged in at 38. Note with the dmesg-with-patch (using my patch), things seem to be working ok. Though in the example above it may look odd, as usually it switches to to host mode, then when it detects a low speed device (a mouse which I had plugged in), it will reset to switch to low-speed, but I didn't wait long enough for the reset to complete and the mouse to be detected before re-plugging in the OTG port. > On 5/21/2018 11:41 PM, John Stultz wrote: >> On Mon, May 21, 2018 at 1:45 AM, Minas Harutyunyan >> wrote: >>> Hi John, >>> >>> On 5/19/2018 4:49 AM, John Stultz wrote: In 4.17-rc, commit 03ea6d6e9e1f ("usb: dwc2: Enable power down") caused the HiKey board to not correctly handle switching between usb-gadget and usb-host mode. Unplugging the OTG port would result in: > OTG port you mean MicroAB, Correct? Correct. > dwc2 driver loaded when some device connected to OTG port? Yes, I normally boot with the board plugged in to OTG as a peripheral/gadget for adb. > And below message printed after disconnect the device from OTG port? > [ 42.240973] dwc2 f72c.usb: dwc2_restore_host_registers: no host registers to restore [ 42.249066] dwc2 f72c.usb: dwc2_host_exit_hibernation: failed to restore host registers Correct. And the USB-host ports would not function. > USB-host ports - you mean 2 USB A-ports, connected to TS3USB221 HUB? Correct. > Switching ports between OTG and Host ports via TS3USB221 Switch > performing automatically or by some SW tool? Its done automatically, when the OTG cable is detected it the host ports are disabled and when the OTG port is empty the host ports are enabled. Let me know if you need anything else! thanks -john dmesg-broken.log.xz Description: Binary data dmesg-with-patch.log.xz Description: Binary data
Re: [PATCH 1/5] phy: allwinner: add phy driver for USB3 PHY on Allwinner H6 SoC
On Mon, May 07, 2018 at 11:18:13PM +0800, Icenowy Zheng wrote: > Allwinner H6 SoC contains a USB3 PHY (with USB2 DP/DM lines also > controlled). > > Add a driver for it. > > The register operations in this driver is mainly extracted from the BSP > USB3 driver. > > Signed-off-by: Icenowy Zheng > --- > .../bindings/phy/sun50i-usb3-phy.txt | 24 +++ Please split bindings to separate patch. > drivers/phy/allwinner/Kconfig | 13 ++ > drivers/phy/allwinner/Makefile| 1 + > drivers/phy/allwinner/phy-sun50i-usb3.c | 195 ++ > 4 files changed, 233 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt > create mode 100644 drivers/phy/allwinner/phy-sun50i-usb3.c > > diff --git a/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt > b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt > new file mode 100644 > index ..912d55f9f69d > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/sun50i-usb3-phy.txt > @@ -0,0 +1,24 @@ > +Allwinner sun50i USB3 PHY > +--- > + > +Required properties: > +- compatible : should be one of > + * allwinner,sun60i-h6-usb3-phy > +- reg : a list of offset + length pairs > +- #phy-cells : from the generic phy bindings, must be 0 > +- clocks : phandle + clock specifier for the phy clock > +- resets : phandle + reset specifier for the phy reset > + > +Optional Properties: > +- phy-supply : from the generic phy bindings, a phandle to a regulator that > +provides power to VBUS. > + > +Example: > + usb3phy: phy@521 { usb-phy@... > + compatible = "allwinner,sun50i-h6-usb3-phy"; > + reg = <0x521 0x1>; > + clocks = <&ccu CLK_USB_PHY1>; > + resets = <&ccu RST_USB_PHY1>; > + #phy-cells = <0>; > + status = "disabled"; Don't show status in examples. > + }; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] arm64: allwinner: h6: add USB3 device nodes
On Tue, May 08, 2018 at 11:31:27AM +0300, Sergei Shtylyov wrote: > Hello! > > On 5/7/2018 6:18 PM, Icenowy Zheng wrote: > > > Allwinner H6 SoC features USB3 functionality, with a DWC3 controller and > > a custom PHY. > > > > Add device tree nodes for them. > > > > Signed-off-by: Icenowy Zheng > > --- > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 38 > > 1 file changed, 38 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > index c72da8cd9ef5..9564c938717c 100644 > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi > > @@ -174,6 +174,44 @@ > > status = "disabled"; > > }; > > + usb3: usb@520 { > >I don't think is allowed for a node having no "reg" prop... Right. One way to fix is fill out ranges property because the unit address can come from either. However, there's work to deprecate doing DWC3 binding with a child node like this. See the series "usb: dwc3: support clocks and resets for DWC3 core". Please follow that. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for USB HCD instead of tasklets
On Tue, 22 May 2018, Sebastian Andrzej Siewior wrote: > On 2018-05-07 11:37:29 [-0400], Alan Stern wrote: > > > As far as I understand it there should be no deadlock. Without the > > > local_irq_save() things should not deadlock because the HCD invokes USB > > > driver's (usb-storage for instance) ->complete callback always in the > > > same way. If you mix the usb-driver with two different HCDs (say ehci > > > and xhci) then lockdep would complain. However, the locks which were > > > allocated by usb-storage for the ehci driver would never be used in the > > > context of the xhci driver. So lockdep would report a deacklock but > > > there wouldn't be one (again, if I got the piece right). > > > > That argument would be correct if the completion routines were the only > > places where the higher-level drivers (such as usb-storage or usbhid) > > acquired their locks. But we can't depend on this being true; you > > would have to audit each USB driver to make sure. > > an entry point for IRQ usage outside of the driver would be the usage of > hrtimer. Sorry, I don't understand that sentence at all. And I don't see how it could be relevant to the point I was trying to make. Consider, for example, drivers/hid/usbhid/hid-core.c. In that file, hid_io_error() is called by hid_irq_in(), which is an URB completion handler. hid_io_error() acquires usbhid_lock. Therefore it would be necessary to audit the usbhid driver to see whether interrupts are enabled or disabled any place where usbhid_lock is acquired. And in fact, hid_ctrl() (another completion handler) calls spin_lock(&usbhid->lock) -- this could cause problems for you. And usbhid->lock is acquired in other places, ones that are not inside completion handlers. None of this has anything to do with IRQ usage or hrtimers. > We have a flag to let the hrtimer run in softirq but yes, we > need to audit them. > > > > And I was thinking about converting all drivers to one model and then we > > > could get rid of the block I quoted above. > > > > > > If nobody rejects the approach as such I would go and start hacking… > > > > > > > And even for those two, the conversion will not be easy. Simply > > > > changing the giveback routines would violate the documented guarantees > > > > for isochronous transfers. > > > > > > The requirement was that the ISO urb is completed before the BULK urb, > > > right? > > > > No, that's not what I meant. For one thing, isochronous URBs don't > > need to complete before bulk URBs in general (although they do have > > higher priority). > > > > However, I was really referring to the kerneldoc for usb_submit_urb(). > > The part that talks about scheduling of isochronous and interrupt URBs > > lists a bunch of requirements. In order to meet these requirements > > some of the host controller drivers may rely on the fact that when they > > give back an URB, the URB's completion routine will return before the > > giveback call finishes. > > You mean the "Reserved Bandwidth Transfers:" paragraph? Paragraphs (plural). Three paragraphs, to be precise. > > It's possible to rewrite the HCDs not to rely on this (I did exactly > > that for ehci-hcd), but it is a nontrivial job. > > are you referring to commit 9118f9eb4f1e ("USB: EHCI: improve interrupt > qh unlink")? That one, plus: 46c73d1d3ebc ("USB: EHCI: handle isochronous underruns with tasklets") e4e18cbd52c8 ("USB: EHCI: code rearrangement in iso_stream_schedule()") 24f531371de1 ("USB: EHCI: accept very late isochronous URBs") 35371e4fbc3e ("USB: EHCI: improve ehci_endpoint_disable") Not all parts of all those commits were relevant, but as far as I recall, they each contributed something. And I may have omitted one or two commits by mistake. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: uvc: Utilise instance name in video name
Hi Kieran, Thank you for the patch. On Tuesday, 22 May 2018 18:29:49 EEST Kieran Bingham wrote: > From: Kieran Bingham > > With multiple UVC gadgets on a composite device, the device names become > indistinguishable from one another. > > Extend the gadget video name to incorporate the function instance name, > along side the existing UDC controller name. > > Signed-off-by: Kieran Bingham > --- > drivers/usb/gadget/function/f_uvc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/f_uvc.c > b/drivers/usb/gadget/function/f_uvc.c index 1affa8e3a974..65454a31ad68 > 100644 > --- a/drivers/usb/gadget/function/f_uvc.c > +++ b/drivers/usb/gadget/function/f_uvc.c > @@ -433,7 +433,9 @@ uvc_register_video(struct uvc_device *uvc) > uvc->vdev.release = video_device_release_empty; > uvc->vdev.vfl_dir = VFL_DIR_TX; > uvc->vdev.lock = &uvc->video.mutex; > - strlcpy(uvc->vdev.name, cdev->gadget->name, sizeof(uvc->vdev.name)); > + > + snprintf(uvc->vdev.name, sizeof(uvc->vdev.name), "%s:%s", > + cdev->gadget->name, uvc->func.fi->group.cg_item.ci_name); If the function is instantiated from the legacy g_webcam, the whole group structure will be zero, so ci_name will be empty. I think you should omit the ":" in that case. I also wonder whether the vdev.name field will always be long enough. If we can't guarantee that, would it make sense to instead expose the config item name through a custom sysfs property for the video device ? > video_set_drvdata(&uvc->vdev, uvc); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: xhci: force all memory allocations to node
The xhci driver forces DMA memory to be node aware, however, there are several ring-related memory allocations that are not memory node aware. This patch resolves those *alloc functions to be allocated on the proper memory node. Signed-off-by: Adam Wallis --- changes from v1: * This was rebased on top of Mathias' for-usb-next branch drivers/usb/host/xhci-mem.c | 57 ++--- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 0b61b77d..4fe7471 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -33,8 +33,9 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci, struct xhci_segment *seg; dma_addr_t dma; int i; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; - seg = kzalloc(sizeof *seg, flags); + seg = kzalloc_node(sizeof(*seg), flags, dev_to_node(dev)); if (!seg) return NULL; @@ -45,7 +46,8 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci, } if (max_packet) { - seg->bounce_buf = kzalloc(max_packet, flags); + seg->bounce_buf = kzalloc_node(max_packet, flags, + dev_to_node(dev)); if (!seg->bounce_buf) { dma_pool_free(xhci->segment_pool, seg->trbs, dma); kfree(seg); @@ -363,8 +365,9 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, { struct xhci_ring*ring; int ret; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; - ring = kzalloc(sizeof *(ring), flags); + ring = kzalloc_node(sizeof(*ring), flags, dev_to_node(dev)); if (!ring) return NULL; @@ -458,11 +461,12 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci, int type, gfp_t flags) { struct xhci_container_ctx *ctx; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; if ((type != XHCI_CTX_TYPE_DEVICE) && (type != XHCI_CTX_TYPE_INPUT)) return NULL; - ctx = kzalloc(sizeof(*ctx), flags); + ctx = kzalloc_node(sizeof(*ctx), flags, dev_to_node(dev)); if (!ctx) return NULL; @@ -615,6 +619,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci, struct xhci_ring *cur_ring; u64 addr; int ret; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; xhci_dbg(xhci, "Allocating %u streams and %u " "stream context array entries.\n", @@ -625,7 +630,8 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci, } xhci->cmd_ring_reserved_trbs++; - stream_info = kzalloc(sizeof(struct xhci_stream_info), mem_flags); + stream_info = kzalloc_node(sizeof(*stream_info), mem_flags, + dev_to_node(dev)); if (!stream_info) goto cleanup_trbs; @@ -633,9 +639,9 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci, stream_info->num_stream_ctxs = num_stream_ctxs; /* Initialize the array of virtual pointers to stream rings. */ - stream_info->stream_rings = kzalloc( - sizeof(struct xhci_ring *)*num_streams, - mem_flags); + stream_info->stream_rings = kcalloc_node( + num_streams, sizeof(struct xhci_ring *), mem_flags, + dev_to_node(dev)); if (!stream_info->stream_rings) goto cleanup_info; @@ -831,6 +837,7 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci, struct xhci_tt_bw_info *tt_info; unsigned intnum_ports; int i, j; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; if (!tt->multi) num_ports = 1; @@ -840,7 +847,8 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci, for (i = 0; i < num_ports; i++, tt_info++) { struct xhci_interval_bw_table *bw_table; - tt_info = kzalloc(sizeof(*tt_info), mem_flags); + tt_info = kzalloc_node(sizeof(*tt_info), mem_flags, + dev_to_node(dev)); if (!tt_info) goto free_tts; INIT_LIST_HEAD(&tt_info->tt_list); @@ -1641,7 +1649,8 @@ static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags) if (!num_sp) return 0; - xhci->scratchpad = kzalloc(sizeof(*xhci->scratchpad), flags); + xhci->scratchpad = kzalloc_node(sizeof(*xhci->scratchpad), flags, + dev_to_node(dev)); if (!xhci->scratchpad) goto fail_sp; @@ -1651,7 +1660,8 @@ stat
Re: dwc2: Regression on 96Boards Hikey due to enabling power down
+ John Stultz. John submitted a patch recently to temporarily fix the issue by disabling the power down feature in Hikey. https://lkml.org/lkml/2018/5/21/730 John, I have listed some of my findings to this thread which will help debugging the issue. Thanks, Mani On Thu, May 17, 2018 at 04:04:01PM +0530, Manivannan Sadhasivam wrote: > Hi Artur, > > Thanks for the reply! > > On Thu, May 17, 2018 at 09:10:06AM +, Artur Petrosyan wrote: > > Hi Mani, > > > > We need some detailed information to perform debugging. > > > > 1. Could you please share the documentation of "96Boards HiKey" board, at > > least dwc core configuration parameters. Or dump of GHWCFG1-4. > > You can find the HiKey documentation here: > https://github.com/96boards/documentation/tree/master/consumer/hikey/hardware-docs > > GHWCFG register dump: > > GHWCFG1 = 0x > GHWCFG2 = 0x23affc70 > GHWCFG3 = 0x0780d4e8 > GHWCFG4 = 0xfff00060 > > > 2. Could you please share with us full debug log of dwc2 loading and plug > > the USB device. > > Here is the full kernel log from the boot till USB device gets plugged in: > https://pastebin.ubuntu.com/p/3bZwWtk8wD/ > > > 3. From short debug log seen that Host exited from hibernation after USB > > device plugged to the port. Do you mean that enumeration process didn't > > start? If not, could you please dump registers after "dwc2 f72c.usb: > > Host hibernation restore complete" > > Yeah, I guess the enumeration process didn't happen at all. Here is the > register dump after plugging the device: > > https://pastebin.ubuntu.com/p/3vxdkFGgp6/ > > > 4. In which mode had the dwc2 been built. In host only mode or DRD mode? > > DWC2 in HiKey is built in Dual Role Device (DRD) mode. > > > 5. And you mention that USB controller's DP/DM out is connected to a switch > > which switches between a USB type C port and a HUB (USB2513B). Is the > > device connected to USB type C port or to HUB (USB2513B) ? Switching > > connection done before dwc2 load or after? > > > > The USB device is connected to one of the ports from HUB and there is one > gpio which is used for switching the USB ports between Type-C and HUB. By > default, this gpio is unused and it pulled low, which means HUB will get > selected before dwc2 gets loaded. > > Hope this information will help debugging the issue. > > Thanks, > Mani > > > Looking forward to your reply. > > > > Regards, > > Artur > > > > > > -Original Message- > > From: Manivannan Sadhasivam [mailto:manivannan.sadhasi...@linaro.org] > > Sent: Wednesday, May 16, 2018 17:23 > > To: linux-usb@vger.kernel.org > > Cc: john.y...@synopsys.com; mvar...@synopsys.com; > > arthur.petros...@synopsys.com; grigor.tovmas...@synopsys.com; > > felipe.ba...@linux.intel.com > > Subject: usb: dwc2: Regression on 96Boards Hikey due to enabling power down > > > > Hello, > > > > Commit 03ea6d6e9e1ff1b0222eb723eee5990d3511cc4d introduced the powerdown > > feature in USB DWC2 driver which stops USB from working on 96Boards HiKey > > board. > > > > During bootup, USB host controller goes into hibernation state and when any > > USB device is plugged onto the port, the host finishes executing the GPWRDN > > interrupt handler but still it didn't wake up. > > > > Below is the dmesg log after plugging a device onto USB port: > > > > [ 30.763414] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: > > dwc2_handle_gpwrdwn_intr called gpwrdn= 081411bb > > [ 30.763458] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: GPWRDN_LNSTSCHG > > [ 30.763492] dwc2 f72c.usb: dwc2_host_exit_hibernation: called with > > rem_wakeup = 1 reset = 0 > > [ 30.763623] dwc2 f72c.usb: dwc2_restore_essential_regs: restoring > > essential regs > > [ 30.763671] dwc2 f72c.usb: restore done generated here > > [ 30.976707] dwc2 f72c.usb: dwc2_restore_global_registers > > [ 30.976723] dwc2 f72c.usb: dwc2_restore_host_registers > > [ 30.976741] dwc2 f72c.usb: Host hibernation restore complete > > > > I tried to manually reset the HUB and PHY after plugging in the device but > > that didn't help. > > > > This issue has been detected in kernel 4.17-rc1. > > > > Can anyone shed some light here? Any help would be greatly appreciated! > > > > Note: On this board, the USB controller's DP/DM out is connected to a > > switch which switches between a USB type C port and a HUB (USB2513B). > > > > Thanks, > > Mani -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for USB HCD instead of tasklets
On 2018-05-07 11:37:29 [-0400], Alan Stern wrote: > > As far as I understand it there should be no deadlock. Without the > > local_irq_save() things should not deadlock because the HCD invokes USB > > driver's (usb-storage for instance) ->complete callback always in the > > same way. If you mix the usb-driver with two different HCDs (say ehci > > and xhci) then lockdep would complain. However, the locks which were > > allocated by usb-storage for the ehci driver would never be used in the > > context of the xhci driver. So lockdep would report a deacklock but > > there wouldn't be one (again, if I got the piece right). > > That argument would be correct if the completion routines were the only > places where the higher-level drivers (such as usb-storage or usbhid) > acquired their locks. But we can't depend on this being true; you > would have to audit each USB driver to make sure. an entry point for IRQ usage outside of the driver would be the usage of hrtimer. We have a flag to let the hrtimer run in softirq but yes, we need to audit them. > > And I was thinking about converting all drivers to one model and then we > > could get rid of the block I quoted above. > > > > If nobody rejects the approach as such I would go and start hacking… > > > > > And even for those two, the conversion will not be easy. Simply > > > changing the giveback routines would violate the documented guarantees > > > for isochronous transfers. > > > > The requirement was that the ISO urb is completed before the BULK urb, > > right? > > No, that's not what I meant. For one thing, isochronous URBs don't > need to complete before bulk URBs in general (although they do have > higher priority). > > However, I was really referring to the kerneldoc for usb_submit_urb(). > The part that talks about scheduling of isochronous and interrupt URBs > lists a bunch of requirements. In order to meet these requirements > some of the host controller drivers may rely on the fact that when they > give back an URB, the URB's completion routine will return before the > giveback call finishes. You mean the "Reserved Bandwidth Transfers:" paragraph? > It's possible to rewrite the HCDs not to rely on this (I did exactly > that for ehci-hcd), but it is a nontrivial job. are you referring to commit 9118f9eb4f1e ("USB: EHCI: improve interrupt qh unlink")? > Alan Stern Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/11] docs: fix broken references with multiple hints
On Wed, May 09, 2018 at 10:18:54AM -0300, Mauro Carvalho Chehab wrote: > The script: > ./scripts/documentation-file-ref-check --fix-rst > > Gives multiple hints for broken references on some files. > Manually use the one that applies for some files. > > Signed-off-by: Mauro Carvalho Chehab > --- > Documentation/ABI/obsolete/sysfs-gpio | 2 +- > .../devicetree/bindings/display/bridge/tda998x.txt| 2 +- Acked-by: Rob Herring > Documentation/trace/events.rst| 2 +- > Documentation/trace/tracepoint-analysis.rst | 2 +- > Documentation/translations/zh_CN/SubmittingDrivers| 2 +- > Documentation/translations/zh_CN/gpio.txt | 4 ++-- > MAINTAINERS | 2 +- > drivers/hid/usbhid/Kconfig| 2 +- > drivers/input/Kconfig | 4 ++-- > drivers/input/joystick/Kconfig| 4 ++-- > drivers/input/joystick/iforce/Kconfig | 2 +- > drivers/input/serio/Kconfig | 4 ++-- > drivers/staging/fsl-mc/bus/dpio/dpio-driver.txt | 2 +- > drivers/video/fbdev/skeletonfb.c | 8 > include/linux/tracepoint.h| 2 +- > security/device_cgroup.c | 2 +- > 16 files changed, 23 insertions(+), 23 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
On Tue, May 22, 2018 at 09:01:07PM +0900, Yoshihiro Shimoda wrote: > This patch adds role switch support for R-Car SoCs into the USB 3.0 > peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0 > dual-role device controller which has the USB 3.0 xHCI host and > Renesas USB 3.0 peripheral. > > Unfortunately, the mode change register contains the USB 3.0 peripheral > controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) > manages this register now. However, in peripheral mode, the host > should stop. Also the host hardware needs to reinitialize its own > registers when the mode changes from peripheral to host mode. > Otherwise, the host cannot work correctly (e.g. detect a device as > high-speed). > > To achieve this by a driver, this role switch driver manages > the mode change register and attach/release the xhci-plat driver. > > Signed-off-by: Yoshihiro Shimoda > --- > .../devicetree/bindings/usb/renesas_usb3.txt | 15 Please split bindings to a separate patch. > drivers/usb/gadget/udc/Kconfig | 1 + > drivers/usb/gadget/udc/renesas_usb3.c | 82 > ++ > 3 files changed, 98 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > index 2c071bb5..f6105aa 100644 > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > @@ -19,6 +19,9 @@ Required properties: > Optional properties: >- phys: phandle + phy specifier pair >- phy-names: must be "usb" > + - The connection to a usb3.0 host node needs by using OF graph bindings for > +usb role switch. > + - port@0 = USB3.0 host port. On the host side, this might conflict with the USB connector binding. I would either make sure this can work with the connector binding by having 2 endpoints on the HS or SS port or just use the 'companion' property defined in usb-generic.txt. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] driver core: don't hold dev's parent lock when using async probe
On Tue, 22 May 2018, martin_liu wrote: > SOC have internal I/O buses that can't be probed for devices. The > devices on the buses can be accessed directly without additinal > configuration required. This type of bus is represented as > "simple-bus". In some platforms, we name "soc" with "simple-bus" > attribute and many devices are hooked under and desribe them in DT > (device tree). > > In commit 'bf74ad5bc417 introduce ("[PATCH] Hold the device's > parent's lock during probe and remove")' to solve USB subsystem > lock sequence since usb device's characteristic. Thus "soc" > needs to be locked whenever a device and driver's probing > happen under "soc" bus. During this period, an async driver > tries to probe a device which is under the "soc" bus would be > blocked until previous driver finish the probing and release "soc" > lock. And the next probing under the "soc" bus need to wait for > async finish. Because of that, driver's async probe for init > time improvement will be shadowed. > > Since many devices don't have USB devices' characteristic, they > actually don't need parent's lock. However, in order to control > the risk and minimize the impact, we don't request parent's lock > only when a driver requests async probe. > > Async probe could have more benefit after we have this patch. > > Signed-off-by: martin_liu > --- > This RFC is asked to get some feedback since it involed driver > core and USB subsystem. I'm not familiar with USB subsystem and > not sure if we still need 'bf74ad5bc417 ("[PATCH] Hold the > device's parent's lock during probe and remove")' since it has > been there over 10 years. If we still need it and hard to fix it > , the simple way is to find a place not to allow USB subsystem > drivers to have async probe capability. Any suggestion is welcome. I don't think the "allows_async_probing" attribute is the best way to attack this. Some other approach, like a special-purpose flag, might be better. Yes, USB still needs to have parent's locks held during probing. Here's the reason. A USB device can have multiple interfaces, each bound to its own driver. A driver may sometimes need to issue a reset, but in USB there's no way to reset a single interface. Only the entire device can be reset, and of course this affects all the interfaces. Therefore a driver needs to acquire the device lock before it can issue a reset. The problem is that the driver's thread may already hold the device lock. During a normal probe sequence, for example, the interfaces get probed by the hub driver while it owns the device lock. But for probes under other circumstances (for example, if the user writes to the driver's "bind" attribute in sysfs), the device lock might not be held. A driver cannot tell these two cases apart. The only way to make it work all the time is to have the caller _always_ hold the device lock while the driver is probed (or the removed, for that matter). Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usbip: dynamically allocate idev by nports found in sysfs
As the amount of available ports varies by the kernels build configuration. To remove the limitation of the fixed 128 ports we allocate the amount of idevs by using the number we get from the kernel. Signed-off-by: Michael Grzeschik --- v1 -> v2: - reworked memory allocation into one calloc call - added error path on allocation failure tools/usb/usbip/libsrc/vhci_driver.c | 14 +- tools/usb/usbip/libsrc/vhci_driver.h | 3 +-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index c9c81614a66ad..6e2a9edfd1f0d 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -242,13 +242,20 @@ static int read_record(int rhport, char *host, unsigned long host_len, int usbip_vhci_driver_open(void) { + int nports = get_nports(); + udev_context = udev_new(); if (!udev_context) { err("udev_new failed"); return -1; } - vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver)); + vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver) + + nports * sizeof(struct usbip_imported_device)); + if (!vhci_driver) { + err("vhci_driver allocation failed"); + return -1; + } /* will be freed in usbip_driver_close() */ vhci_driver->hc_device = @@ -260,15 +267,12 @@ int usbip_vhci_driver_open(void) goto err; } - vhci_driver->nports = get_nports(); + vhci_driver->nports = nports; dbg("available ports: %d", vhci_driver->nports); if (vhci_driver->nports <= 0) { err("no available ports"); goto err; - } else if (vhci_driver->nports > MAXNPORT) { - err("port number exceeds %d", MAXNPORT); - goto err; } vhci_driver->ncontrollers = get_ncontrollers(); diff --git a/tools/usb/usbip/libsrc/vhci_driver.h b/tools/usb/usbip/libsrc/vhci_driver.h index 418b404d51210..6c9aca2167051 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.h +++ b/tools/usb/usbip/libsrc/vhci_driver.h @@ -13,7 +13,6 @@ #define USBIP_VHCI_BUS_TYPE "platform" #define USBIP_VHCI_DEVICE_NAME "vhci_hcd.0" -#define MAXNPORT 128 enum hub_speed { HUB_SPEED_HIGH = 0, @@ -41,7 +40,7 @@ struct usbip_vhci_driver { int ncontrollers; int nports; - struct usbip_imported_device idev[MAXNPORT]; + struct usbip_imported_device idev[]; }; -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] power: supply: Add fwnode pointer to power_supply_config struct
Hi, On Tue, May 22, 2018 at 04:16:23PM +0100, Adam Thomson wrote: > To allow users of the power supply framework to be hw description > agnostic, this commit adds the ability to pass a fwnode pointer, > via the power_supply_config structure, to the initialisation code > of the core, instead of explicitly specifying of_ndoe. If that > fwnode pointer is provided then it will automatically resolve down > to of_node on platforms which support it, otherwise it will be NULL. > > In the future, when ACPI support is added, this can be modified to > accommodate ACPI without the need to change calling code which > already provides the fwnode handle in this manner. > > Signed-off-by: Adam Thomson > Suggested-by: Heikki Krogerus > --- Reviewed-by: Sebastian Reichel -- Sebastian > drivers/power/supply/power_supply_core.c | 4 +++- > include/linux/power_supply.h | 2 ++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/power_supply_core.c > b/drivers/power/supply/power_supply_core.c > index ecd68c2..f57ab0a 100644 > --- a/drivers/power/supply/power_supply_core.c > +++ b/drivers/power/supply/power_supply_core.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include "power_supply.h" > > @@ -874,7 +875,8 @@ static void psy_unregister_cooler(struct power_supply > *psy) > psy->desc = desc; > if (cfg) { > psy->drv_data = cfg->drv_data; > - psy->of_node = cfg->of_node; > + psy->of_node = > + cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node; > psy->supplied_to = cfg->supplied_to; > psy->num_supplicants = cfg->num_supplicants; > } > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h > index 0c9a572..b21c4bd9 100644 > --- a/include/linux/power_supply.h > +++ b/include/linux/power_supply.h > @@ -199,6 +199,8 @@ enum power_supply_notifier_events { > /* Run-time specific power supply configuration */ > struct power_supply_config { > struct device_node *of_node; > + struct fwnode_handle *fwnode; > + > /* Driver private data */ > void *drv_data; > > -- > 1.9.1 > signature.asc Description: PGP signature
Re: [PATCH 2/2] typec: tcpm: Provide fwnode pointer as part of psy_cfg
On Tue, May 22, 2018 at 04:16:24PM +0100, Adam Thomson wrote: > For supply registration, provide fwnode pointer of the port device, > via the power_supply_config structure, to allow other psy drivers > to add us as a supplier. At present this only applies to DT > based platforms using the 'power-supplies' DT property, but in the > future should also work for ACPI platforms when the relevant support > is added to the power_supply core. > > Signed-off-by: Adam Thomson > Suggested-by: Heikki Krogerus Reviewed-by: Guenter Roeck > --- > drivers/usb/typec/tcpm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > index 72996cc..0ccd2ce 100644 > --- a/drivers/usb/typec/tcpm.c > +++ b/drivers/usb/typec/tcpm.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -4500,6 +4501,7 @@ static int devm_tcpm_psy_register(struct tcpm_port > *port) > char *psy_name; > > psy_cfg.drv_data = port; > + psy_cfg.fwnode = dev_fwnode(port->dev); > psy_name = devm_kzalloc(port->dev, psy_name_len, GFP_KERNEL); > if (!psy_name) > return -ENOMEM; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usbip: vhci_sysfs: fix potential Spectre v1
On 05/22/2018 10:56 AM, Shuah Khan wrote: On 05/18/2018 07:13 PM, Gustavo A. R. Silva wrote: pdev_nr and rhport can be controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre issue 'vhcis' drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre issue 'vhcis' drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre issue 'vhci->vhci_hcd_ss->vdev' drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre issue 'vhci->vhci_hcd_hs->vdev' Fix this by sanitizing pdev_nr and rhport before using them to index vhcis and vhci->vhci_hcd_ss->vdev respectively. Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 Cc: sta...@vger.kernel.org Signed-off-by: Gustavo A. R. Silva --- Changes in v3: - Pass the addresses of pdev_nr and rhport into valid_port and valid_args. Changes in v2: - Place the barriers into valid_port. drivers/usb/usbip/vhci_sysfs.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index 4880838..be37aec 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -10,6 +10,9 @@ #include #include +/* Hardening for Spectre-v1 */ +#include + #include "usbip_common.h" #include "vhci.h" @@ -205,16 +208,20 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci_hcd, __u32 rhport) return 0; } -static int valid_port(__u32 pdev_nr, __u32 rhport) +static int valid_port(__u32 *pdev_nr, __u32 *rhport) { - if (pdev_nr >= vhci_num_controllers) { - pr_err("pdev %u\n", pdev_nr); + if (*pdev_nr >= vhci_num_controllers) { + pr_err("pdev %u\n", *pdev_nr); return 0; } - if (rhport >= VHCI_HC_PORTS) { - pr_err("rhport %u\n", rhport); + *pdev_nr = array_index_nospec(*pdev_nr, vhci_num_controllers); + + if (*rhport >= VHCI_HC_PORTS) { + pr_err("rhport %u\n", *rhport); return 0; } + *rhport = array_index_nospec(*rhport, VHCI_HC_PORTS); + return 1; } @@ -232,7 +239,7 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr, pdev_nr = port_to_pdev_nr(port); rhport = port_to_rhport(port); - if (!valid_port(pdev_nr, rhport)) + if (!valid_port(&pdev_nr, &rhport)) return -EINVAL; hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); @@ -258,7 +265,8 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_WO(detach); -static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed) +static int valid_args(__u32 *pdev_nr, __u32 *rhport, + enum usb_device_speed speed) { if (!valid_port(pdev_nr, rhport)) { return 0; @@ -322,7 +330,7 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr, sockfd, devid, speed); /* check received parameters */ - if (!valid_args(pdev_nr, rhport, speed)) + if (!valid_args(&pdev_nr, &rhport, speed)) return -EINVAL; hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); Looks good to me. Thanks for taking care of this. Glad to help. :) Acked-by: Shuah Khan (Samsung OSG) Thanks -- Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbip: dynamically allocate idev by nports found in sysfs
Hi Michael, Thanks for the patch. Couple of comments below: On 05/18/2018 08:39 AM, Michael Grzeschik wrote: > As the amount of available ports varies by the kernels build > configuration. To remove the limitation of the fixed 128 ports > we allocate the amount of idevs by using the number we get > from the kernel. > > Signed-off-by: Michael Grzeschik > --- > tools/usb/usbip/libsrc/vhci_driver.c | 11 --- > tools/usb/usbip/libsrc/vhci_driver.h | 3 +-- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/vhci_driver.c > b/tools/usb/usbip/libsrc/vhci_driver.c > index c9c81614a66ad..9a8acfc7697fa 100644 > --- a/tools/usb/usbip/libsrc/vhci_driver.c > +++ b/tools/usb/usbip/libsrc/vhci_driver.c > @@ -266,11 +266,11 @@ int usbip_vhci_driver_open(void) > if (vhci_driver->nports <= 0) { > err("no available ports"); > goto err; > - } else if (vhci_driver->nports > MAXNPORT) { > - err("port number exceeds %d", MAXNPORT); > - goto err; > } > > + vhci_driver->idev = calloc(vhci_driver->nports, > + sizeof(struct usbip_imported_device)); > + Missing check for memory allocation failure. Please add it. > vhci_driver->ncontrollers = get_ncontrollers(); > dbg("available controllers: %d", vhci_driver->ncontrollers); > > @@ -287,6 +287,9 @@ int usbip_vhci_driver_open(void) > err: > udev_device_unref(vhci_driver->hc_device); > > + if (vhci_driver->idev) > + free(vhci_driver->idev); > + > if (vhci_driver) > free(vhci_driver); > > @@ -305,6 +308,8 @@ void usbip_vhci_driver_close(void) > > udev_device_unref(vhci_driver->hc_device); > > + free(vhci_driver->idev); > + > free(vhci_driver); > > vhci_driver = NULL; > diff --git a/tools/usb/usbip/libsrc/vhci_driver.h > b/tools/usb/usbip/libsrc/vhci_driver.h > index 418b404d51210..67dbd1551e159 100644 > --- a/tools/usb/usbip/libsrc/vhci_driver.h > +++ b/tools/usb/usbip/libsrc/vhci_driver.h > @@ -13,7 +13,6 @@ > > #define USBIP_VHCI_BUS_TYPE "platform" > #define USBIP_VHCI_DEVICE_NAME "vhci_hcd.0" > -#define MAXNPORT 128 > > enum hub_speed { > HUB_SPEED_HIGH = 0, > @@ -41,7 +40,7 @@ struct usbip_vhci_driver { > > int ncontrollers; > int nports; > - struct usbip_imported_device idev[MAXNPORT]; > + struct usbip_imported_device *idev; > }; > > > Rest looks good. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usbip: vhci_sysfs: fix potential Spectre v1
On 05/18/2018 07:13 PM, Gustavo A. R. Silva wrote: > pdev_nr and rhport can be controlled by user-space, hence leading to > a potential exploitation of the Spectre variant 1 vulnerability. > > This issue was detected with the help of Smatch: > drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre > issue 'vhcis' > drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre > issue 'vhcis' > drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre > issue 'vhci->vhci_hcd_ss->vdev' > drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre > issue 'vhci->vhci_hcd_hs->vdev' > > Fix this by sanitizing pdev_nr and rhport before using them to index > vhcis and vhci->vhci_hcd_ss->vdev respectively. > > Notice that given that speculation windows are large, the policy is > to kill the speculation on the first load and not worry if it can be > completed with a dependent load/store [1]. > > [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 > > Cc: sta...@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva > --- > Changes in v3: > - Pass the addresses of pdev_nr and rhport into valid_port and >valid_args. > > Changes in v2: > - Place the barriers into valid_port. > > drivers/usb/usbip/vhci_sysfs.c | 24 > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c > index 4880838..be37aec 100644 > --- a/drivers/usb/usbip/vhci_sysfs.c > +++ b/drivers/usb/usbip/vhci_sysfs.c > @@ -10,6 +10,9 @@ > #include > #include > > +/* Hardening for Spectre-v1 */ > +#include > + > #include "usbip_common.h" > #include "vhci.h" > > @@ -205,16 +208,20 @@ static int vhci_port_disconnect(struct vhci_hcd > *vhci_hcd, __u32 rhport) > return 0; > } > > -static int valid_port(__u32 pdev_nr, __u32 rhport) > +static int valid_port(__u32 *pdev_nr, __u32 *rhport) > { > - if (pdev_nr >= vhci_num_controllers) { > - pr_err("pdev %u\n", pdev_nr); > + if (*pdev_nr >= vhci_num_controllers) { > + pr_err("pdev %u\n", *pdev_nr); > return 0; > } > - if (rhport >= VHCI_HC_PORTS) { > - pr_err("rhport %u\n", rhport); > + *pdev_nr = array_index_nospec(*pdev_nr, vhci_num_controllers); > + > + if (*rhport >= VHCI_HC_PORTS) { > + pr_err("rhport %u\n", *rhport); > return 0; > } > + *rhport = array_index_nospec(*rhport, VHCI_HC_PORTS); > + > return 1; > } > > @@ -232,7 +239,7 @@ static ssize_t detach_store(struct device *dev, struct > device_attribute *attr, > pdev_nr = port_to_pdev_nr(port); > rhport = port_to_rhport(port); > > - if (!valid_port(pdev_nr, rhport)) > + if (!valid_port(&pdev_nr, &rhport)) > return -EINVAL; > > hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); > @@ -258,7 +265,8 @@ static ssize_t detach_store(struct device *dev, struct > device_attribute *attr, > } > static DEVICE_ATTR_WO(detach); > > -static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed > speed) > +static int valid_args(__u32 *pdev_nr, __u32 *rhport, > + enum usb_device_speed speed) > { > if (!valid_port(pdev_nr, rhport)) { > return 0; > @@ -322,7 +330,7 @@ static ssize_t attach_store(struct device *dev, struct > device_attribute *attr, >sockfd, devid, speed); > > /* check received parameters */ > - if (!valid_args(pdev_nr, rhport, speed)) > + if (!valid_args(&pdev_nr, &rhport, speed)) > return -EINVAL; > > hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); > Looks good to me. Thanks for taking care of this. Acked-by: Shuah Khan (Samsung OSG) -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] checking hub port status while USB 2.0 port is resuming.
On Tue, 22 May 2018, Anshuman Gupta wrote: > On Tue, May 22, 2018 at 09:53:09AM -0400, Alan Stern wrote: > > On Tue, 22 May 2018, Anshuman Gupta wrote: > > > Thanks Alan for your quick response. > > > Hi, > > > > > > When a xhci USB2.0 port is resuming and waiting for resume signaling, > > > completion of > > > USB_RESUME_TIMEOUT, it just reports the port status as > > > USB_PORT_STAT_SUSPEND, > > > and let the usbcore to check the port status again. > > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/host/xhci-hub.c#L960 > > > > > > > > > USB core just checks the port status in usb_port_resume function > > > and if port status is in suspended state, it clears the port suspend > > > feature. > > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3441 > > > > > > But when system resumes due to a USB remote wakeup event from suspend > > > state, > > > and port which causes remote wakeup is still waiting for resume signaling, > > > usb_port_resume function in this particular case clears the port suspend > > > feature, > > > and wait for 40ms again(USB_RESUME_TIMEOUT). > > > > > > Query regarding above case: > > > Here in this case USB core will miss the wake up event from the port > > > which causes > > > the remote wake. > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3444 > > > > What do you mean? You said above: "system resumes due to a USB remote > > wakeup event". The fact that the system is resuming indicates that it > > did _not_ miss the wakeup event. > > > What was i meaning, if system come out of suspend due to a key pressed on usb > keyboard. > For the above mention case, it will not raise any pm_wakeup_event so the > notification > of the pm_wakeup_event for usb keyboard will get missed. Okay, let's say you're right. What difference does it make if the pm_wakeup_event gets lost? Will that cause a problem? Alan Stern > > > If a port is already resuming and waiting for resume signaling, does it > > > make > > > sense to clear the port suspend feature and wait for 40ms again. > > > > When the system resumes from a suspend state, it wakes up _all_ the > > suspended USB ports. Waiting an extra 40 ms for one port won't make > > much difference. > > > > Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
spende
Hallo, Sie haben eine Spende von 2.800.000,00 Euro, ich habe die Amerikanische Lotterie in Amerika im Wert von 533 Millionen Dollar gewonnen, und ich gebe einen Teil davon als Teil einer Wohltätigkeitsorganisation. Kontaktieren Sie mich für weitere Informationen: richardwahl.donati...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: uvc: Utilise instance name in video name
From: Kieran Bingham With multiple UVC gadgets on a composite device, the device names become indistinguishable from one another. Extend the gadget video name to incorporate the function instance name, along side the existing UDC controller name. Signed-off-by: Kieran Bingham --- drivers/usb/gadget/function/f_uvc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 1affa8e3a974..65454a31ad68 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -433,7 +433,9 @@ uvc_register_video(struct uvc_device *uvc) uvc->vdev.release = video_device_release_empty; uvc->vdev.vfl_dir = VFL_DIR_TX; uvc->vdev.lock = &uvc->video.mutex; - strlcpy(uvc->vdev.name, cdev->gadget->name, sizeof(uvc->vdev.name)); + + snprintf(uvc->vdev.name, sizeof(uvc->vdev.name), "%s:%s", +cdev->gadget->name, uvc->func.fi->group.cg_item.ci_name); video_set_drvdata(&uvc->vdev, uvc); -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] typec: tcpm: Provide fwnode pointer as part of psy_cfg
For supply registration, provide fwnode pointer of the port device, via the power_supply_config structure, to allow other psy drivers to add us as a supplier. At present this only applies to DT based platforms using the 'power-supplies' DT property, but in the future should also work for ACPI platforms when the relevant support is added to the power_supply core. Signed-off-by: Adam Thomson Suggested-by: Heikki Krogerus --- drivers/usb/typec/tcpm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index 72996cc..0ccd2ce 100644 --- a/drivers/usb/typec/tcpm.c +++ b/drivers/usb/typec/tcpm.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -4500,6 +4501,7 @@ static int devm_tcpm_psy_register(struct tcpm_port *port) char *psy_name; psy_cfg.drv_data = port; + psy_cfg.fwnode = dev_fwnode(port->dev); psy_name = devm_kzalloc(port->dev, psy_name_len, GFP_KERNEL); if (!psy_name) return -ENOMEM; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] typec: tcpm: Populate fwnode for use in power_supply core
This patch set adds support for passing a fwnode pointer as part of the psy config structure, when intialising the psy core, so that calling code can be written to be agnostic of the HW description required for the platform. The TCPM code is updated subsequently to make use of this new field. For now the main FW support in the psy core is still just DT based but in the future ACPI will likely be added and can use this field. Adam Thomson (2): power: supply: Add fwnode pointer to power_supply_config struct typec: tcpm: Provide fwnode pointer as part of psy_cfg drivers/power/supply/power_supply_core.c | 4 +++- drivers/usb/typec/tcpm.c | 2 ++ include/linux/power_supply.h | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] power: supply: Add fwnode pointer to power_supply_config struct
To allow users of the power supply framework to be hw description agnostic, this commit adds the ability to pass a fwnode pointer, via the power_supply_config structure, to the initialisation code of the core, instead of explicitly specifying of_ndoe. If that fwnode pointer is provided then it will automatically resolve down to of_node on platforms which support it, otherwise it will be NULL. In the future, when ACPI support is added, this can be modified to accommodate ACPI without the need to change calling code which already provides the fwnode handle in this manner. Signed-off-by: Adam Thomson Suggested-by: Heikki Krogerus --- drivers/power/supply/power_supply_core.c | 4 +++- include/linux/power_supply.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index ecd68c2..f57ab0a 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include "power_supply.h" @@ -874,7 +875,8 @@ static void psy_unregister_cooler(struct power_supply *psy) psy->desc = desc; if (cfg) { psy->drv_data = cfg->drv_data; - psy->of_node = cfg->of_node; + psy->of_node = + cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node; psy->supplied_to = cfg->supplied_to; psy->num_supplicants = cfg->num_supplicants; } diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 0c9a572..b21c4bd9 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -199,6 +199,8 @@ enum power_supply_notifier_events { /* Run-time specific power supply configuration */ struct power_supply_config { struct device_node *of_node; + struct fwnode_handle *fwnode; + /* Driver private data */ void *drv_data; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Revised Renesas uPD72020x workaround for 32bit DMA issue
Hi, thanks a lot. In 4.16.9 that renesas USB3 controller is working again. So the positive, it did work for some time - hours, and reading from the attached harddisk was all right. Don't know if I'm allowed or requested to mention the remaining problems. The first thing I want to mention is, that in 4.12.7 the SMART values came through with this command: smartctl -a -d sat /dev/sdc . Now, this does not work. The second would be, now after some standby and resume cycles, it again fails. But working again after reboot, so for me that's nothing to worry about. % sudo smartctl -a -d sat -T permissive /dev/sdc smartctl 6.6 2017-11-05 r4594 [x86_64-linux-4.16.9-1-ARCH] (local build) Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org Read Device Identity failed: scsi error unsupported field in scsi command === START OF INFORMATION SECTION === Device Model: [No Information Found] Serial Number:[No Information Found] Firmware Version: [No Information Found] Device is:Not in smartctl database [for details use: -P showall] ATA Version is: [No Information Found] Local Time is:Mon May 21 22:59:04 2018 CEST SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 82-83 don't show if SMART supported. SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 85-87 don't show if SMART is enabled. A mandatory SMART command failed: exiting. To continue, add one or more '-T permissive' options. % sudo smartctl -a -d sat -T permissive -T permissive -T permissive -s on /dev/sdc smartctl 6.6 2017-11-05 r4594 [x86_64-linux-4.16.9-1-ARCH] (local build) Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org Read Device Identity failed: scsi error unsupported field in scsi command === START OF INFORMATION SECTION === Device Model: [No Information Found] Serial Number:[No Information Found] Firmware Version: [No Information Found] Device is:Not in smartctl database [for details use: -P showall] ATA Version is: [No Information Found] Local Time is:Mon May 21 22:59:39 2018 CEST SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 82-83 don't show if SMART supported. SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 85-87 don't show if SMART is enabled. Checking to be sure by trying SMART RETURN STATUS command. SMART support is: Unknown - Try option -s with argument 'on' to enable it. === START OF ENABLE/DISABLE COMMANDS SECTION === SMART Enable failed: scsi error unsupported field in scsi command A mandatory SMART command failed: exiting. To continue, add one or more '-T permissive' options. % sudo smartctl -a -d scsi /dev/sdc smartctl 6.6 2017-11-05 r4594 [x86_64-linux-4.16.9-1-ARCH] (local build) Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org === START OF INFORMATION SECTION === Vendor: Seagate Product: M3 Portable Revision: 9300 Compliance: SPC-4 User Capacity:4,000,787,029,504 bytes [4.00 TB] Logical block size: 512 bytes Physical block size: 4096 bytes Logical Unit id: xx Serial number:xx Device type: disk Local Time is:Mon May 21 23:00:00 2018 CEST SMART support is: Available - device has SMART capability. SMART support is: Disabled Temperature Warning: Disabled or Not Supported === START OF READ SMART DATA SECTION === SMART Health Status: OK Current Drive Temperature: 0 C Drive Trip Temperature:0 C Error Counter logging not supported Device does not support Self Test logging and here from 4.12.7: sudo smartctl -a -d sat /dev/sdc smartctl 6.6 2017-11-05 r4594 [x86_64-linux-4.12.7-1-ARCH] (local build) Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org === START OF INFORMATION SECTION === Model Family: Seagate Barracuda 2.5 5400 Device Model: ST4000LM024-2AN17V Serial Number:x LU WWN Device Id: 5 000c50 0a806a977 Firmware Version: 0001 User Capacity:4,000,787,030,016 bytes [4.00 TB] Sector Sizes: 512 bytes logical, 4096 bytes physical Rotation Rate:5526 rpm Form Factor: 2.5 inches Device is:In smartctl database [for details use: -P show] ATA Version is: ACS-3 T13/2161-D revision 5 SATA Version is: SATA 3.1, 6.0 Gb/s (current: 3.0 Gb/s) Local Time is:Fri May 11 15:28:56 2018 CEST SMART support is: Available - device has SMART capability. SMART support is: Enabled === START OF READ SMART DATA SECTION === SMART overall-health self-assessment test result: PASSED General SMART Values: Offline data collection status: (0x82) Offline data collection activity was completed without error. Auto Offline Data Collection: Enabled. Self-test execution status: ( 0) The previous self-test
Re: [PATCH] usb: dwc2: Fix HiKey regression caused by power_down feature
Hi John, Please provide log with debug enabled configuration. On 5/21/2018 11:41 PM, John Stultz wrote: > On Mon, May 21, 2018 at 1:45 AM, Minas Harutyunyan > wrote: >> Hi John, >> >> On 5/19/2018 4:49 AM, John Stultz wrote: >>> In 4.17-rc, commit 03ea6d6e9e1f ("usb: dwc2: Enable power down") >>> caused the HiKey board to not correctly handle switching between >>> usb-gadget and usb-host mode. >>> >>> Unplugging the OTG port would result in: OTG port you mean MicroAB, Correct? dwc2 driver loaded when some device connected to OTG port? And below message printed after disconnect the device from OTG port? >>> [ 42.240973] dwc2 f72c.usb: dwc2_restore_host_registers: no host >>> registers to restore >>> [ 42.249066] dwc2 f72c.usb: dwc2_host_exit_hibernation: failed to >>> restore host registers >>> >>> And the USB-host ports would not function. USB-host ports - you mean 2 USB A-ports, connected to TS3USB221 HUB? Switching ports between OTG and Host ports via TS3USB221 Switch performing automatically or by some SW tool? >>> >>> And plugging in the OTG port, we would see: >>> [ 46.046557] WARNING: CPU: 3 PID: 6 at drivers/usb/dwc2/gadget.c:260 >>> dwc2_hsotg_init_fifo+0x194/0x1a0 >>> [ 46.055761] CPU: 3 PID: 6 Comm: kworker/u16:0 Not tainted >>> 4.17.0-rc5-00030-ge67da8c #231 >>> [ 46.055767] Hardware name: HiKey Development Board (DT) >>> [ 46.055784] Workqueue: dwc2 dwc2_conn_id_status_change >>> ... >>> >> Could you please send full log to debug. > > Full dmesg log attached. > > I unplugged the usb-otg port at 136 > and replugged it back in at 141 > > >>>p->uframe_sched = false; >>>p->change_speed_quirk = true; >>> + p->power_down = false; >> >> power_down declared as int, suggested to update as follow: >> p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >> >> This can be accepted as temporary solution until we will fully debug >> hibernation feature for HiKey platform. > > Ok, will re-send with the suggested change above. > > thanks > -john > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config
Hi Heikki, > -Original Message- > From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com] > Sent: 2018年5月21日 21:13 > To: Jun Li > Cc: Mats Karrman ; robh...@kernel.org; > gre...@linuxfoundation.org; li...@roeck-us.net; a.ha...@samsung.com; > cw00.c...@samsung.com; shufan_...@richtek.com; Peter Chen > ; gso...@gmail.com; devicet...@vger.kernel.org; > linux-usb@vger.kernel.org; dl-linux-imx > Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port > power > and data config > > Hi Jun, > > Sorry for the delay. > > On Thu, May 17, 2018 at 02:41:31PM +, Jun Li wrote: > > Hi > > > -Original Message- > > > From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com] > > > Sent: 2018??5??17?? 22:24 > > > To: Jun Li > > > Cc: Mats Karrman ; robh...@kernel.org; > > > gre...@linuxfoundation.org; li...@roeck-us.net; a.ha...@samsung.com; > > > cw00.c...@samsung.com; shufan_...@richtek.com; Peter Chen > > > ; gso...@gmail.com; devicet...@vger.kernel.org; > > > linux-usb@vger.kernel.org; dl-linux-imx > > > Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic > > > port power and data config > > > > > > On Thu, May 17, 2018 at 02:05:41PM +, Jun Li wrote: > > > > Hi Heikki, > > > > > > > > > > I reread this patch and tried to see it more in the context of > > > > > > the other patches and the existing code. The naming of the > > > > > > existing string tables doesn't help in getting this right, however > > > > > > I have > a proposal: > > > > > > > > > > > > typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP > > > > > > typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD > > > > > > typec_find_power_role() to get to TYPEC_SINK/SOURCE > > > > > > > > > > > > and sometime, if the use should arise > > > > > > > > > > > > typec_find_data_role() to get to TYPEC_DEVICE/HOST > > > > > > > > > > > > I think it is fairly comprehensible, *_port_* concerns a > > > > > > capability and without *_port_* it is an actual state. Plus it > > > > > > matches the names of the constants. > > > > > > > > > > Well, there are now four things to consider: > > > > > > > > > > 1) the roles (power and data) the port is capable of supporting > > > > > 2) Try.SRC and Try.SNK, i.e. the preferred role > > > > > 3) the current roles > > > > > 4) the role(s) the user want's to limit the use of a port with > > > > > DRP ports > > > > > > > > > > The last one I don't know if it's relevant with these functions. > > > > > It's not information that we would get for example from firmware. > > > > > > > > > > I also don't think we need to use these functions with the > > > > > current roles the port is in. > > > > > > > > > > If the preferred role is "sink" or "source", so just like the > > > > > power role, we don't need separate function for it here. > > > > > > > > > > So isn't two functions all we need here: one for the power and > > > > > one for data role? > > > > > > > > Take power as an example, can we use one function to only look up > > > > typec_port_types[]? as capability(typec_port_type) and > > > > state(typec_role) are different enum, and the allowed strings are > different. > > > > > > > > static const char * const typec_roles[] = { > > > > [TYPEC_SINK]= "sink", > > > > [TYPEC_SOURCE] = "source", > > > > }; > > > > > > > > static const char * const typec_port_types[] = { > > > > [TYPEC_PORT_SRC] = "source", > > > > [TYPEC_PORT_SNK] = "sink", > > > > [TYPEC_PORT_DRP] = "dual", > > > > }; > > > > > > Where out side the class code we need to convert the current role, > > > the "state", with these functions? > > > > Currently, the only call site is getting the preferred power role from > > firmware. > > My point was that if we used enum typec_port_type with the prefered role, we > could use the helper for power role with prefered role. But never mind. Got your point. So let's follow Mats's suggestion on this, I will update a new version. Thanks Li Jun > > > Thanks, > > -- > heikki
[RFC] driver core: don't hold dev's parent lock when using async probe
SOC have internal I/O buses that can't be probed for devices. The devices on the buses can be accessed directly without additinal configuration required. This type of bus is represented as "simple-bus". In some platforms, we name "soc" with "simple-bus" attribute and many devices are hooked under and desribe them in DT (device tree). In commit 'bf74ad5bc417 introduce ("[PATCH] Hold the device's parent's lock during probe and remove")' to solve USB subsystem lock sequence since usb device's characteristic. Thus "soc" needs to be locked whenever a device and driver's probing happen under "soc" bus. During this period, an async driver tries to probe a device which is under the "soc" bus would be blocked until previous driver finish the probing and release "soc" lock. And the next probing under the "soc" bus need to wait for async finish. Because of that, driver's async probe for init time improvement will be shadowed. Since many devices don't have USB devices' characteristic, they actually don't need parent's lock. However, in order to control the risk and minimize the impact, we don't request parent's lock only when a driver requests async probe. Async probe could have more benefit after we have this patch. Signed-off-by: martin_liu --- This RFC is asked to get some feedback since it involed driver core and USB subsystem. I'm not familiar with USB subsystem and not sure if we still need 'bf74ad5bc417 ("[PATCH] Hold the device's parent's lock during probe and remove")' since it has been there over 10 years. If we still need it and hard to fix it , the simple way is to find a place not to allow USB subsystem drivers to have async probe capability. Any suggestion is welcome. drivers/base/bus.c | 19 +-- drivers/base/dd.c | 23 --- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/base/bus.c b/drivers/base/bus.c index ef6183306b40..6434333995d4 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -181,13 +181,15 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, struct bus_type *bus = bus_get(drv->bus); struct device *dev; int err = -ENODEV; + bool allow_async; + allow_async = driver_allows_async_probing(drv); dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == drv) { - if (dev->parent)/* Needed for USB */ + if (dev->parent && !allow_async)/* Needed for USB */ device_lock(dev->parent); device_release_driver(dev); - if (dev->parent) + if (dev->parent && !allow_async) device_unlock(dev->parent); err = count; } @@ -208,15 +210,17 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf, struct bus_type *bus = bus_get(drv->bus); struct device *dev; int err = -ENODEV; + bool allow_async; + allow_async = driver_allows_async_probing(drv); dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == NULL && driver_match_device(drv, dev)) { - if (dev->parent)/* Needed for USB */ + if (dev->parent && !allow_async)/* Needed for USB */ device_lock(dev->parent); device_lock(dev); err = driver_probe_device(drv, dev); device_unlock(dev); - if (dev->parent) + if (dev->parent && !allow_async) device_unlock(dev->parent); if (err > 0) { @@ -769,11 +773,14 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices); */ int device_reprobe(struct device *dev) { + bool allow_async; + if (dev->driver) { - if (dev->parent)/* Needed for USB */ + allow_async = driver_allows_async_probing(dev->driver); + if (dev->parent && !allow_async)/* Needed for USB */ device_lock(dev->parent); device_release_driver(dev); - if (dev->parent) + if (dev->parent && !allow_async) device_unlock(dev->parent); } return bus_rescan_devices_helper(dev, NULL); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index c9f54089429b..36aed1576c58 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -794,6 +794,7 @@ static int __driver_attach(struct device *dev, void *data) { struct device_driver *drv = data; int ret; + bool allow_async; /* * Lock device and try to bind to it. We drop the error @@ -817,13 +818,14 @@ static int __driver_attach(struct device *dev, void *data) return ret; } /* ret > 0 means positive match */ - if (dev->parent)/* Needed for USB */ + allow_async = driver_allows_async_probing(drv); + if (dev->parent && !allow_async)
Re: [Query] checking hub port status while USB 2.0 port is resuming.
On Tue, 22 May 2018, Anshuman Gupta wrote: > Hi, > > When a xhci USB2.0 port is resuming and waiting for resume signaling, > completion of > USB_RESUME_TIMEOUT, it just reports the port status as USB_PORT_STAT_SUSPEND, > and let the usbcore to check the port status again. > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/host/xhci-hub.c#L960 > > > USB core just checks the port status in usb_port_resume function > and if port status is in suspended state, it clears the port suspend feature. > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3441 > > But when system resumes due to a USB remote wakeup event from suspend state, > and port which causes remote wakeup is still waiting for resume signaling, > usb_port_resume function in this particular case clears the port suspend > feature, > and wait for 40ms again(USB_RESUME_TIMEOUT). > > Query regarding above case: > Here in this case USB core will miss the wake up event from the port which > causes > the remote wake. > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3444 What do you mean? You said above: "system resumes due to a USB remote wakeup event". The fact that the system is resuming indicates that it did _not_ miss the wakeup event. > If a port is already resuming and waiting for resume signaling, does it make > sense to clear the port suspend feature and wait for 40ms again. When the system resumes from a suspend state, it wakes up _all_ the suspended USB ports. Waiting an extra 40 ms for one port won't make much difference. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/12] usb: usbtmc: Add vendor specific/asynchronous read/write ioctls
On Tue, May 22, 2018 at 11:36:13AM +, gu...@kiener-muenchen.de wrote: > > Zitat von Greg KH : > > > On Mon, May 21, 2018 at 08:41:22PM +, gu...@kiener-muenchen.de wrote: > > > > > > Zitat von Greg KH : > > > > > > > On Fri, May 18, 2018 at 02:48:11PM +, gu...@kiener-muenchen.de > > > > wrote: > > > > > > > > > > Zitat von Greg KH : > > > > > > > > > > > On Thu, May 17, 2018 at 07:03:30PM +0200, Guido Kiener wrote: > > > > > > > +/* > > > > > > > + * usbtmc_message->flags: > > > > > > > + */ > > > > > > > +#define USBTMC_FLAG_ASYNC0x0001 > > > > > > > +#define USBTMC_FLAG_APPEND 0x0002 > > > > > > > +#define USBTMC_FLAG_IGNORE_TRAILER 0x0004 > > > > > > > + > > > > > > > +struct usbtmc_message { > > > > > > > + void __user *message; /* pointer to header and data */ > > > > > > > + __u64 transfer_size; /* size of bytes to transfer */ > > > > > > > + __u64 transferred; /* size of received/written bytes */ > > > > > > > + __u32 flags; /* bit 0: 0 = synchronous; 1 = asynchronous */ > > > > > > > +} __attribute__ ((packed)); > > > > > > > > > > > > Very odd structure. Your userspace pointer is going to be totally > > > > > > out > > > > > > of alignment on 32bit systems running on a 64bit kernel. Why have a > > > > > > separate pointer at all? Why not just put the mesage at the > > > end of this > > > > > > structure directly with something like: > > > > > > __u8 message[0]; > > > > > > ? > > > > > > > > > > > > Using a __u8 message[0] ends up with an extra malloc, memcpy, and free > > > operation in userland, since we always have to append the data of a given > > > user pointer to this struct. E.g. when I send 4 MByte on my (old) laptop > > > this takes about plus 6 ms, and decreases bandwidth from 31,0 MByte/s > > > to 29,5 MByte/s with USB 2.0. > > > > Really? That feels like you are doing something really odd. I guess > > you need to figure out how you get the data to/from userspace. > > A typically user API is something like this: > ssize_t write(int fd, const void *buf, size_t count); > ssize_t send(int sockfd, const void *buf, size_t len, int flags); > > We use a similar function: > ViStatus viWrite(ViSession vi, ViBuf buf, ViUInt32 count, ViPUInt32 retCount) > > When a user calls viWrite(vi, "*IDN? + 4 MB ...\n", 4000, &retcont) how > can we pass the big buf to the kernel without any copy operation? Your function looks _just_ like a normal write() call, why are you even trying to wrap it in an ioctl? Just use write()! > > > I hope this struct looks better (in compat mode): > > > > > > struct usbtmc_message { > > > __u64 transfer_size; /* size of bytes to transfer */ > > > __u64 transferred; /* size of received/written bytes */ > > > void __user *message; /* pointer to header and data */ > > > __u32 flags; /* bit 0: 0 = synchronous; 1 = asynchronous */ > > > } __attribute__ ((packed)); > > > > No, that will not work at all. Again, think about the size of that > > pointer. > > > > The compat structure is: > > struct compat_usbtmc_message { > u64 transfer_size; > u64 transferred; > compat_uptr_t message; > u32 flags; > } __packed; > > For AMD64 it works. Show me the size and layout of both structures for a 32bit and 64bit userspace please. There are tools that do this. Again, this is not how to do it! You do not put a variable sized variable in the middle of a structure. void * changes size! Either align everything properly or throw it at the end or even better yet, DON'T DO THIS AT ALL, JUST USE write()! :) > > > BTW the driver has no .compat_ioctl entry. > > > > Because you didn't need it until now. > > ioctl(fd,USBTMC_IOCTL_CLEAR) returns errno = 25 (=ENOTTY) in Linux > Kernel 4.15 when running test-raw32 created with > gcc -m32 test-raw.c -o test-raw32 > Does it work with other kernel versions? I have no idea what you are testing here, sorry. > > > So I wonder how it could work with 32 bit applications on 64 bit > > > systems before submission of our patches. Do I miss something? > > > > You are creating structures that have different sizes on those two > > different userspaces. Because of that, you would be forced to have a > > compat layer. The smart thing to do is to design the interface to not > > have that type of problem at all, don't create new apis that are not > > just 64-bit sane to start with. > > > > Copying memory is fast, really fast, much much faster than sending the > > data across the USB connection. If you are seeing major problems here, > > then I would first look at your userspace code, and then see what the > > kernel code does. > > I just do not like to be slower than libusb or other operating systems. If you do this correctly, your bottleneck will be the USB connection. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http:/
Re: [PATCH 02/18] dt-bindings: usb: add bindings doc for HiSilicon STB xHCI host controller
On 22.05.2018 13:06, Greg KH wrote: On Mon, May 21, 2018 at 04:39:50PM +0300, Mathias Nyman wrote: From: Jianguo Sun This commit adds bindings doc for HiSilicon STB xHCI host controller. Signed-off-by: Jianguo Sun Signed-off-by: Mathias Nyman Don't you need an ack from the DT maintainers for new bindings? True. The comment Rob Herring had to V2 of the HiSilicon patchseries was fixed in V3 (this one), but V3 was never acked. Rob, did V3 look ok? https://marc.info/?l=linux-usb&m=152627889413412&w=2 Thanks -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
This patch adds role switch support for R-Car SoCs into the USB 3.0 peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0 dual-role device controller which has the USB 3.0 xHCI host and Renesas USB 3.0 peripheral. Unfortunately, the mode change register contains the USB 3.0 peripheral controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) manages this register now. However, in peripheral mode, the host should stop. Also the host hardware needs to reinitialize its own registers when the mode changes from peripheral to host mode. Otherwise, the host cannot work correctly (e.g. detect a device as high-speed). To achieve this by a driver, this role switch driver manages the mode change register and attach/release the xhci-plat driver. Signed-off-by: Yoshihiro Shimoda --- .../devicetree/bindings/usb/renesas_usb3.txt | 15 drivers/usb/gadget/udc/Kconfig | 1 + drivers/usb/gadget/udc/renesas_usb3.c | 82 ++ 3 files changed, 98 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt index 2c071bb5..f6105aa 100644 --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt @@ -19,6 +19,9 @@ Required properties: Optional properties: - phys: phandle + phy specifier pair - phy-names: must be "usb" + - The connection to a usb3.0 host node needs by using OF graph bindings for +usb role switch. + - port@0 = USB3.0 host port. Example of R-Car H3 ES1.x: usb3_peri0: usb@ee02 { @@ -27,6 +30,12 @@ Example of R-Car H3 ES1.x: reg = <0 0xee02 0 0x400>; interrupts = ; clocks = <&cpg CPG_MOD 328>; + + port { + usb3_peri0_ep: endpoint { + remote-endpoint = <&usb3_host0_ep>; + }; + }; }; usb3_peri1: usb@ee06 { @@ -35,4 +44,10 @@ Example of R-Car H3 ES1.x: reg = <0 0xee06 0 0x400>; interrupts = ; clocks = <&cpg CPG_MOD 327>; + + port { + usb3_peri1_ep: endpoint { + remote-endpoint = <&usb3_host1_ep>; + }; + }; }; diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index b838cae..78823cd 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -193,6 +193,7 @@ config USB_RENESAS_USB3 tristate 'Renesas USB3.0 Peripheral controller' depends on ARCH_RENESAS || COMPILE_TEST depends on EXTCON && HAS_DMA + select USB_ROLE_SWITCH help Renesas USB3.0 Peripheral controller is a USB peripheral controller that supports super, high, and full speed USB 3.0 data transfers. diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c index 5caf78b..9667a5e 100644 --- a/drivers/usb/gadget/udc/renesas_usb3.c +++ b/drivers/usb/gadget/udc/renesas_usb3.c @@ -23,6 +23,7 @@ #include #include #include +#include /* register definitions */ #define USB3_AXI_INT_STA 0x008 @@ -335,6 +336,9 @@ struct renesas_usb3 { struct phy *phy; struct dentry *dentry; + struct usb_role_switch *role_sw; + struct device *host_dev; + struct renesas_usb3_ep *usb3_ep; int num_usb3_eps; @@ -2302,6 +2306,41 @@ static int renesas_usb3_set_selfpowered(struct usb_gadget *gadget, int is_self) .set_selfpowered= renesas_usb3_set_selfpowered, }; +static enum usb_role renesas_usb3_role_switch_get(struct device *dev) +{ + struct renesas_usb3 *usb3 = dev_get_drvdata(dev); + enum usb_role cur_role; + + pm_runtime_get_sync(dev); + cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE; + pm_runtime_put(dev); + + return cur_role; +} + +static int renesas_usb3_role_switch_set(struct device *dev, + enum usb_role role) +{ + struct renesas_usb3 *usb3 = dev_get_drvdata(dev); + struct device *host = usb3->host_dev; + enum usb_role cur_role = renesas_usb3_role_switch_get(dev); + + pm_runtime_get_sync(dev); + if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) { + device_release_driver(host); + usb3_set_mode(usb3, false); + } else if (cur_role == USB_ROLE_DEVICE && role == USB_ROLE_HOST) { + /* Must set the mode before device_attach of the host */ + usb3_set_mode(usb3, true); + /* This device_attach() might sleep */ + if (device_attach(host) < 0) + dev_err(dev, "device_attach(usb3_port) failed\n"); + } + pm_runtime_put(dev); + + return 0;
[PATCH/RFC v4 1/4] base: devcon: add graph parse in device_connection_find_match()
This patch adds graph parsing in device_connection_find_match(). The match function will be called with fwnode pointer in struct device_connection. So, a caller can check the matching by using it. Signed-off-by: Yoshihiro Shimoda --- Documentation/driver-api/device_connection.rst | 2 +- drivers/base/devcon.c | 15 +++ include/linux/device.h | 2 ++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Documentation/driver-api/device_connection.rst b/Documentation/driver-api/device_connection.rst index affbc556..cc6b1da 100644 --- a/Documentation/driver-api/device_connection.rst +++ b/Documentation/driver-api/device_connection.rst @@ -19,7 +19,7 @@ Device connections alone do not create a dependency between the two devices. They are only descriptions which are not tied to either of the devices directly. A dependency between the two devices exists only if one of the two endpoint devices requests a reference to the other. The descriptions themselves can be -defined in firmware (not yet supported) or they can be built-in. +defined in firmware or they can be built-in. Usage - diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c index d427e80..374bb39 100644 --- a/drivers/base/devcon.c +++ b/drivers/base/devcon.c @@ -7,6 +7,7 @@ */ #include +#include static DEFINE_MUTEX(devcon_lock); static LIST_HEAD(devcon_list); @@ -31,10 +32,24 @@ void *device_connection_find_match(struct device *dev, const char *con_id, struct device_connection *con; void *ret = NULL; int ep; + struct device_connection graph; + struct fwnode_handle *fwnode_ep; + struct fwnode_handle *remote; if (!match) return NULL; + fwnode_graph_for_each_endpoint(dev->fwnode, fwnode_ep) { + remote = fwnode_graph_get_remote_port_parent(fwnode_ep); + if (!remote) + continue; + + graph.fwnode = remote; + ret = match(&graph, 0, data); + if (!IS_ERR(ret)) + return ret; + } + mutex_lock(&devcon_lock); list_for_each_entry(con, &devcon_list, list) { diff --git a/include/linux/device.h b/include/linux/device.h index 0059b99..175907b 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -734,11 +734,13 @@ struct device_dma_parameters { * struct device_connection - Device Connection Descriptor * @endpoint: The names of the two devices connected together * @id: Unique identifier for the connection + * @fwnode: fwnode pointer for finding a connection from graph * @list: List head, private, for internal use only */ struct device_connection { const char *endpoint[2]; const char *id; + struct fwnode_handle*fwnode; struct list_headlist; }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 3/4] usb: gadget: udc: renesas_usb3: use usb role switch API
This patch uses usb role switch APIs if the register suceeeded. Signed-off-by: Yoshihiro Shimoda --- drivers/usb/gadget/udc/renesas_usb3.c | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c index 9667a5e..d6c11c9 100644 --- a/drivers/usb/gadget/udc/renesas_usb3.c +++ b/drivers/usb/gadget/udc/renesas_usb3.c @@ -338,6 +338,8 @@ struct renesas_usb3 { struct usb_role_switch *role_sw; struct device *host_dev; + struct work_struct role_work; + enum usb_role role; struct renesas_usb3_ep *usb3_ep; int num_usb3_eps; @@ -655,7 +657,15 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3) } } -static void usb3_set_mode(struct renesas_usb3 *usb3, bool host) +static void renesas_usb3_role_work(struct work_struct *work) +{ + struct renesas_usb3 *usb3 = container_of(work, struct renesas_usb3, +role_work); + + usb_role_switch_set_role(usb3->role_sw, usb3->role); +} + +static void _usb3_set_mode(struct renesas_usb3 *usb3, bool host) { if (host) usb3_clear_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON); @@ -663,6 +673,16 @@ static void usb3_set_mode(struct renesas_usb3 *usb3, bool host) usb3_set_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON); } +static void usb3_set_mode(struct renesas_usb3 *usb3, bool host) +{ + if (usb3->role_sw) { + usb3->role = host ? USB_ROLE_HOST : USB_ROLE_DEVICE; + schedule_work(&usb3->role_work); + } else { + _usb3_set_mode(usb3, host); + } +} + static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable) { if (enable) @@ -2328,10 +2348,10 @@ static int renesas_usb3_role_switch_set(struct device *dev, pm_runtime_get_sync(dev); if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) { device_release_driver(host); - usb3_set_mode(usb3, false); + _usb3_set_mode(usb3, false); } else if (cur_role == USB_ROLE_DEVICE && role == USB_ROLE_HOST) { /* Must set the mode before device_attach of the host */ - usb3_set_mode(usb3, true); + _usb3_set_mode(usb3, true); /* This device_attach() might sleep */ if (device_attach(host) < 0) dev_err(dev, "device_attach(usb3_port) failed\n"); @@ -2726,6 +2746,7 @@ static int renesas_usb3_probe(struct platform_device *pdev) if (ret < 0) goto err_dev_create; + INIT_WORK(&usb3->role_work, renesas_usb3_role_work); usb3->role_sw = usb_role_switch_register(&pdev->dev, &renesas_usb3_role_switch_desc); if (!IS_ERR(usb3->role_sw)) { -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 4/4] arm64: dts: renesas: r8a7795: add OF graph for usb role switch
This patch adds OF graph properties for usb role switch in r8a7795 into USB3.0 host/peripheral nodes. TODO: - need patches for other SoCs. Signed-off-by: Yoshihiro Shimoda --- arch/arm64/boot/dts/renesas/r8a7795.dtsi | 12 1 file changed, 12 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index 1d5e3ac..50d3312 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -1746,6 +1746,12 @@ power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; resets = <&cpg 328>; status = "disabled"; + + port { + usb3_host0_ep: endpoint { + remote-endpoint = <&usb3_peri0_ep>; + }; + }; }; usb3_peri0: usb@ee02 { @@ -1757,6 +1763,12 @@ power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; resets = <&cpg 328>; status = "disabled"; + + port { + usb3_peri0_ep: endpoint { + remote-endpoint = <&usb3_host0_ep>; + }; + }; }; usb_dmac0: dma-controller@e65a { -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 0/4] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs
This patch set is based on Felipe's usb.git / testing/next branch (commit id = 47265c067c0d129f3a0e94bc221293a780af9d78). I still marked this patch set as "RFC". I would like to know whether this way is good or not. About last discusstion with Heikki: https://patchwork.kernel.org/patch/10397635/ Changes from RFC v3: - Rebase latest usb.git / testing/next branch. - Add graph parse into device_connection_find_match(). - Use workqueue to call _usb3_set_mode() in patch 3. (I realized renesas_usb3_role_switch_set() cannot run on atomic because device_attach() might sleep.) Changes from RFC v2: - Add registering usb role switch into drivers/usb/gadget/udc/renesas_usb3 because hardware resource (a register) is shared and remove individual usb role switch driver/dt-bindings for R-Car. - Remove "usb_role_switch_get_by_graph" API because the renesas_usb3 driver doesn't need such API now. Changes from RFC: - Remove "device-connection-id" and "usb role switch driver" dt-bingings. - Remove drivers/of code. - Add a new API for find the connection by using graph on devcon.c and roles.c. - Use each new API on the rcar usb role switch and renesas_usb3 drivers. - Update the dtsi file for r8a7795. Yoshihiro Shimoda (4): base: devcon: add graph parse in device_connection_find_match() usb: gadget: udc: renesas_usb3: Add register of usb role switch usb: gadget: udc: renesas_usb3: use usb role switch API arm64: dts: renesas: r8a7795: add OF graph for usb role switch .../devicetree/bindings/usb/renesas_usb3.txt | 15 +++ Documentation/driver-api/device_connection.rst | 2 +- arch/arm64/boot/dts/renesas/r8a7795.dtsi | 12 +++ drivers/base/devcon.c | 15 +++ drivers/usb/gadget/udc/Kconfig | 1 + drivers/usb/gadget/udc/renesas_usb3.c | 105 - include/linux/device.h | 2 + 7 files changed, 150 insertions(+), 2 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next] USB: dwc3: make function dwc3_get_extcon() static
Fixes the following sparse warnings: drivers/usb/dwc3/drd.c:443:19: warning: symbol 'dwc3_get_extcon' was not declared. Should it be static? Signed-off-by: Wei Yongjun --- drivers/usb/dwc3/drd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c index 2706824..218371f 100644 --- a/drivers/usb/dwc3/drd.c +++ b/drivers/usb/dwc3/drd.c @@ -440,7 +440,7 @@ static int dwc3_drd_notifier(struct notifier_block *nb, return NOTIFY_DONE; } -struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc) +static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc) { struct device *dev = dwc->dev; struct device_node *np_phy, *np_conn; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/12] usb: usbtmc: Add vendor specific/asynchronous read/write ioctls
Zitat von Greg KH : On Mon, May 21, 2018 at 08:41:22PM +, gu...@kiener-muenchen.de wrote: Zitat von Greg KH : > On Fri, May 18, 2018 at 02:48:11PM +, gu...@kiener-muenchen.de wrote: > > > > Zitat von Greg KH : > > > > > On Thu, May 17, 2018 at 07:03:30PM +0200, Guido Kiener wrote: > > > > +/* > > > > + * usbtmc_message->flags: > > > > + */ > > > > +#define USBTMC_FLAG_ASYNC 0x0001 > > > > +#define USBTMC_FLAG_APPEND 0x0002 > > > > +#define USBTMC_FLAG_IGNORE_TRAILER 0x0004 > > > > + > > > > +struct usbtmc_message { > > > > + void __user *message; /* pointer to header and data */ > > > > + __u64 transfer_size; /* size of bytes to transfer */ > > > > + __u64 transferred; /* size of received/written bytes */ > > > > + __u32 flags; /* bit 0: 0 = synchronous; 1 = asynchronous */ > > > > +} __attribute__ ((packed)); > > > > > > Very odd structure. Your userspace pointer is going to be totally out > > > of alignment on 32bit systems running on a 64bit kernel. Why have a > > > separate pointer at all? Why not just put the mesage at the end of this > > > structure directly with something like: > > > __u8 message[0]; > > > ? > > > Using a __u8 message[0] ends up with an extra malloc, memcpy, and free operation in userland, since we always have to append the data of a given user pointer to this struct. E.g. when I send 4 MByte on my (old) laptop this takes about plus 6 ms, and decreases bandwidth from 31,0 MByte/s to 29,5 MByte/s with USB 2.0. Really? That feels like you are doing something really odd. I guess you need to figure out how you get the data to/from userspace. A typically user API is something like this: ssize_t write(int fd, const void *buf, size_t count); ssize_t send(int sockfd, const void *buf, size_t len, int flags); We use a similar function: ViStatus viWrite(ViSession vi, ViBuf buf, ViUInt32 count, ViPUInt32 retCount) When a user calls viWrite(vi, "*IDN? + 4 MB ...\n", 4000, &retcont) how can we pass the big buf to the kernel without any copy operation? Here is a sample algorithm with a minimum (4kB) of copy operation: See function: tmc_raw_write_common(..) in https://github.com/GuidoKiener/linux-usbtmc/blob/master/test-raw.c I hope this struct looks better (in compat mode): struct usbtmc_message { __u64 transfer_size; /* size of bytes to transfer */ __u64 transferred; /* size of received/written bytes */ void __user *message; /* pointer to header and data */ __u32 flags; /* bit 0: 0 = synchronous; 1 = asynchronous */ } __attribute__ ((packed)); No, that will not work at all. Again, think about the size of that pointer. The compat structure is: struct compat_usbtmc_message { u64 transfer_size; u64 transferred; compat_uptr_t message; u32 flags; } __packed; For AMD64 it works. BTW the driver has no .compat_ioctl entry. Because you didn't need it until now. ioctl(fd,USBTMC_IOCTL_CLEAR) returns errno = 25 (=ENOTTY) in Linux Kernel 4.15 when running test-raw32 created with gcc -m32 test-raw.c -o test-raw32 Does it work with other kernel versions? So I wonder how it could work with 32 bit applications on 64 bit systems before submission of our patches. Do I miss something? You are creating structures that have different sizes on those two different userspaces. Because of that, you would be forced to have a compat layer. The smart thing to do is to design the interface to not have that type of problem at all, don't create new apis that are not just 64-bit sane to start with. Copying memory is fast, really fast, much much faster than sending the data across the USB connection. If you are seeing major problems here, then I would first look at your userspace code, and then see what the kernel code does. I just do not like to be slower than libusb or other operating systems. How about you wait for this new api until you get all of the other stuff implemented/merged? It looks like you all need to really think about how this will all work out. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/33] usb: phy: use match_string() helper
Hi, On 2018/5/22 18:02, Greg Kroah-Hartman wrote: > On Tue, May 22, 2018 at 01:20:01AM +0300, Andy Shevchenko wrote: >> On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie wrote: >>> match_string() returns the index of an array for a matching string, >>> which can be used intead of open coded variant. >> >>> - int err, i; >>> + int ret; >> >> int err; >> >> would still work. > > And it reduces churn. I will keep it as err in next version. Thanks Yisheng > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Query] checking hub port status while USB 2.0 port is resuming.
Hi, When a xhci USB2.0 port is resuming and waiting for resume signaling, completion of USB_RESUME_TIMEOUT, it just reports the port status as USB_PORT_STAT_SUSPEND, and let the usbcore to check the port status again. https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/host/xhci-hub.c#L960 USB core just checks the port status in usb_port_resume function and if port status is in suspended state, it clears the port suspend feature. https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3441 But when system resumes due to a USB remote wakeup event from suspend state, and port which causes remote wakeup is still waiting for resume signaling, usb_port_resume function in this particular case clears the port suspend feature, and wait for 40ms again(USB_RESUME_TIMEOUT). Query regarding above case: Here in this case USB core will miss the wake up event from the port which causes the remote wake. https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3444 If a port is already resuming and waiting for resume signaling, does it make sense to clear the port suspend feature and wait for 40ms again. Thanks, Anshuman -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/18] dt-bindings: usb: add bindings doc for HiSilicon STB xHCI host controller
On Mon, May 21, 2018 at 04:39:50PM +0300, Mathias Nyman wrote: > From: Jianguo Sun > > This commit adds bindings doc for HiSilicon STB xHCI host controller. > > Signed-off-by: Jianguo Sun > Signed-off-by: Mathias Nyman Don't you need an ack from the DT maintainers for new bindings? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/33] usb: phy: use match_string() helper
On Tue, May 22, 2018 at 01:20:01AM +0300, Andy Shevchenko wrote: > On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie wrote: > > match_string() returns the index of an array for a matching string, > > which can be used intead of open coded variant. > > > - int err, i; > > + int ret; > > int err; > > would still work. And it reduces churn. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/12] usb: usbtmc: Add vendor specific/asynchronous read/write ioctls
On Mon, May 21, 2018 at 08:41:22PM +, gu...@kiener-muenchen.de wrote: > > Zitat von Greg KH : > > > On Fri, May 18, 2018 at 02:48:11PM +, gu...@kiener-muenchen.de wrote: > > > > > > Zitat von Greg KH : > > > > > > > On Thu, May 17, 2018 at 07:03:30PM +0200, Guido Kiener wrote: > > > > > +/* > > > > > + * usbtmc_message->flags: > > > > > + */ > > > > > +#define USBTMC_FLAG_ASYNC0x0001 > > > > > +#define USBTMC_FLAG_APPEND 0x0002 > > > > > +#define USBTMC_FLAG_IGNORE_TRAILER 0x0004 > > > > > + > > > > > +struct usbtmc_message { > > > > > + void __user *message; /* pointer to header and data */ > > > > > + __u64 transfer_size; /* size of bytes to transfer */ > > > > > + __u64 transferred; /* size of received/written bytes */ > > > > > + __u32 flags; /* bit 0: 0 = synchronous; 1 = asynchronous */ > > > > > +} __attribute__ ((packed)); > > > > > > > > Very odd structure. Your userspace pointer is going to be totally out > > > > of alignment on 32bit systems running on a 64bit kernel. Why have a > > > > separate pointer at all? Why not just put the mesage at the end of this > > > > structure directly with something like: > > > > __u8 message[0]; > > > > ? > > > > > > Using a __u8 message[0] ends up with an extra malloc, memcpy, and free > operation in userland, since we always have to append the data of a given > user pointer to this struct. E.g. when I send 4 MByte on my (old) laptop > this takes about plus 6 ms, and decreases bandwidth from 31,0 MByte/s > to 29,5 MByte/s with USB 2.0. Really? That feels like you are doing something really odd. I guess you need to figure out how you get the data to/from userspace. > I hope this struct looks better (in compat mode): > > struct usbtmc_message { > __u64 transfer_size; /* size of bytes to transfer */ > __u64 transferred; /* size of received/written bytes */ > void __user *message; /* pointer to header and data */ > __u32 flags; /* bit 0: 0 = synchronous; 1 = asynchronous */ > } __attribute__ ((packed)); No, that will not work at all. Again, think about the size of that pointer. > BTW the driver has no .compat_ioctl entry. Because you didn't need it until now. > So I wonder how it could work with 32 bit applications on 64 bit > systems before submission of our patches. Do I miss something? You are creating structures that have different sizes on those two different userspaces. Because of that, you would be forced to have a compat layer. The smart thing to do is to design the interface to not have that type of problem at all, don't create new apis that are not just 64-bit sane to start with. Copying memory is fast, really fast, much much faster than sending the data across the USB connection. If you are seeing major problems here, then I would first look at your userspace code, and then see what the kernel code does. How about you wait for this new api until you get all of the other stuff implemented/merged? It looks like you all need to really think about how this will all work out. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] usb: usbtmc: Add ioctl for generic requests on control pipe
On Mon, May 21, 2018 at 10:06:57PM +, gu...@kiener-muenchen.de wrote: > > Zitat von Greg KH : > > > On Fri, May 18, 2018 at 01:32:51PM +, gu...@kiener-muenchen.de wrote: > > > > > > Zitat von Greg KH : > > > > > > > On Thu, May 17, 2018 at 07:03:29PM +0200, Guido Kiener wrote: > > > > > Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the > > > > > control pipe. Used by specific applications of IVI Foundation, > > > > > Inc. to implement VISA API functions: viUsbControlIn/Out. > > > > > > > > > > Signed-off-by: Guido Kiener > > > > > Reviewed-by: Steve Bayless > > > > > --- > > > > > drivers/usb/class/usbtmc.c | 61 > > > > > > > > > > include/uapi/linux/usb/tmc.h | 15 + > > > > > 2 files changed, 76 insertions(+) > > > > > > > > > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c > > > > > index 152e2daa9644..00c2e51a23a7 100644 > > > > > --- a/drivers/usb/class/usbtmc.c > > > > > +++ b/drivers/usb/class/usbtmc.c > > > > > @@ -5,6 +5,7 @@ > > > > > * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany > > > > > * Copyright (C) 2008 Novell, Inc. > > > > > * Copyright (C) 2008 Greg Kroah-Hartman > > > > > + * Copyright (C) 2018, IVI Foundation, Inc. > > > > > */ > > > > > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > @@ -1263,6 +1264,62 @@ static int > > > > > usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data) > > > > > return rv; > > > > > } > > > > > > > > > > +static int usbtmc_ioctl_request(struct usbtmc_device_data *data, > > > > > + void __user *arg) > > > > > +{ > > > > > + struct device *dev = &data->intf->dev; > > > > > + struct usbtmc_ctrlrequest request; > > > > > + u8 *buffer = NULL; > > > > > + int rv; > > > > > + unsigned long res; > > > > > + > > > > > + res = copy_from_user(&request, arg, sizeof(struct > > > usbtmc_ctrlrequest)); > > > > > + if (res) > > > > > + return -EFAULT; > > > > > + > > > > > + buffer = kmalloc(min_t(u16, 256, request.req.wLength), > > > > > GFP_KERNEL); > > > > > > > > No validation that wLength is a sane number? > > > > > > Ooops. It should be buffer = kmalloc(max_t(u16, 256, request.req.wLength), > > > GFP_KERNEL); > > > Then all values of request.req.wLength (0..65535) are ok. > > > > Yes, that would make more sense. But you should still reject > > known-invalid values, and not just "silently fix them up". > > When I start here to refuse (current) unknown settings or flags, then I fear > that people always want us to change or allow new flags. That doesn't make sense, you always need to reject unknown values, or people will put crap in them and then get mad in the future when you want to use those fields/values for something else. > A device should always be able to deal with all possible values. But the kernel has to be sane and properly validate userspace values. > We > cannot prevent a > user from sending crazy messages to the device and I do not want to develop > something like a firewall here. Sure, I'm not saying to do that, you are not validating the other fields, just the length. The kernel will validate the other fields when you submit the urb. > > > > > + rv = usb_control_msg(data->usb_dev, > > > > > + usb_rcvctrlpipe(data->usb_dev, 0), > > > > > + request.req.bRequest, > > > > > + request.req.bRequestType, > > > > > + request.req.wValue, > > > > > + request.req.wIndex, > > > > > + buffer, request.req.wLength, > > > > > USB_CTRL_GET_TIMEOUT); > > > > > > > > request.req.wLength might not be the actual size of buffer here due to > > > > your above min_t() check. > > > > Still wrong given the check above. > > I'm missing something. I do not see the error. The size of buffer is just > at least 256 bytes. It doesn't matter when request.req.wLength < 256. If wLength is set to 1 but buffer really isn't that big, bad things happen... > I thought this helps the memory management, since we need not to deal with > different and odd memory sizes. Maybe we should just alloc always a size of > request.req.wLength. Again, you have to validate that number, if you want to bound it to 256, great, then bound it. Don't half-way do it please. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] typec: tcpm: Provide of_node pointer as part of psy_cfg
On Mon, May 21, 2018 at 03:34:53PM +, Adam Thomson wrote: > On 21 May 2018 15:56, Heikki Krogerus wrote: > > > On Mon, May 21, 2018 at 01:58:16PM +, Adam Thomson wrote: > > > Hi Heikki, > > > > > > On 21 May 2018 14:20, Heikki Krogerus wrote: > > > > > > > On Wed, May 16, 2018 at 05:00:46PM +0100, Adam Thomson wrote: > > > > > For supply registration, provide of_node pointer of the port device, > > > > > via the power_supply_config structure, to allow other psy drivers > > > > > to add us as a supplier using the 'power-supplies' DT property. > > > > > > > > > > Signed-off-by: Adam Thomson > > > > > --- > > > > > drivers/usb/typec/tcpm.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > > > > > index 72996cc..e7c0b95 100644 > > > > > --- a/drivers/usb/typec/tcpm.c > > > > > +++ b/drivers/usb/typec/tcpm.c > > > > > @@ -4500,6 +4500,7 @@ static int devm_tcpm_psy_register(struct > > > > > tcpm_port > > > > *port) > > > > > char *psy_name; > > > > > > > > > > psy_cfg.drv_data = port; > > > > > + psy_cfg.of_node = port->dev->of_node; > > > > > psy_name = devm_kzalloc(port->dev, psy_name_len, GFP_KERNEL); > > > > > if (!psy_name) > > > > > return -ENOMEM; > > > > > > > > Would it be possible to use fwnode here instead? It would mean that > > > > you add a member for it to the struct power_supply_config, and handle > > > > it separately in power_supply_core.c. You could just convert it to > > > > of_node there for now. > > > > > > > > That is just a request, I'm fine with this, but it would prepare this > > > > driver for all types of platforms, so less patching would be needed > > > > once we add ACPI support to the power_supply_core.c. > > > > > > Would the following commit from Hans, already present in > > > power_supply_core.c, > > > not fit the bill: > > > > > > [58a36bb06891ee779074db6ef84e98347c634d38] > > > power: supply: core: Add support for supplied-from device-property > > > > > > Or was that just meant as a stop gap for something more? > > > > I think the main idea with that patch was that it allows us to take > > advantage of build-in device properties, but I think we could actually > > improve also it if we had the fwnode handle assigned to the psy. We > > would not have to assume the parent has those device properties then. > > > > But ACPI actually defines a specific object called _PCL (power > > consumer list) for power source objects that we should one day check > > in power_supply_core.c. That's what I meant by ACPI support. > > Ok, fair enough. Makes sense to me. Thanks for clarifying. > > > > I have no problems adding something further, but I don't have a means to > > > verify > > > anything ACPI based, beyond a simple build test, so would ideally want > > > someone > > > to verify that path through the code. > > > > This is not really about ACPI. By using fwnode handle instead of > > of_node, we would continue to keep tcpm.c agnostic about the type of > > hw description. > > > > So I don't expect any ACPI support to be added to the > > power_supply_code.c at this point. You can use to_of_node(cfg->fwnode) > > in __power_supply_register() for now: > > > > psy->of_node = cfg->of_node; > > if (cfg->fwnode) > > psy->of_node = to_of_node(cfg->fwnode); > > OK, no problem. Will take a look and add that in. Thanks Adam! -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: tcm: fix spelling mistake: "Manufactor" -> "Manufacturer"
From: Colin Ian King Trivial fix to spelling mistake in usbg_us_strings array Signed-off-by: Colin Ian King --- drivers/usb/gadget/legacy/tcm_usb_gadget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/legacy/tcm_usb_gadget.c b/drivers/usb/gadget/legacy/tcm_usb_gadget.c index 682bf99dcf76..40870227999a 100644 --- a/drivers/usb/gadget/legacy/tcm_usb_gadget.c +++ b/drivers/usb/gadget/legacy/tcm_usb_gadget.c @@ -41,7 +41,7 @@ static struct usb_device_descriptor usbg_device_desc = { #define USB_G_STR_CONFIG USB_GADGET_FIRST_AVAIL_IDX static struct usb_string usbg_us_strings[] = { - [USB_GADGET_MANUFACTURER_IDX].s = "Target Manufactor", + [USB_GADGET_MANUFACTURER_IDX].s = "Target Manufacturer", [USB_GADGET_PRODUCT_IDX].s = "Target Product", [USB_GADGET_SERIAL_IDX].s = "0001", [USB_G_STR_CONFIG].s= "default config", -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver
Am Freitag, den 18.05.2018, 21:50 -0700 schrieb Alexander Kappner: > The ALWAYS_SYNC flag is currently honored by the usb-storage driver but not > UAS > and is required to work around devices that become unstable upon being > queried for cache. This code is taken straight from: > drivers/usb/storage/scsiglue.c:284 > > Signed-off-by: Alexander Kappner Acked-by: Oliver Neukum -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] Re: [PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work
Am Montag, den 21.05.2018, 13:47 -0400 schrieb Alan Stern: > On Fri, 18 May 2018, Alexander Kappner wrote: > > > v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and > > then > > adds the flag as quirks for the device at issue. This allows the G-Drive to > > work > > under both UAS and usb-storage. > > > > Alexander Kappner (2): > > [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver > > [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive > > > > drivers/usb/storage/uas.c | 6 ++ > > drivers/usb/storage/unusual_devs.h | 9 + > > drivers/usb/storage/unusual_uas.h | 9 + > > 3 files changed, 24 insertions(+) > > Acked-by: Alan Stern > > This is okay with me. Oliver, what do you think? Perfect. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tty: add missing const to termios hw-change helper
On Mon, May 21, 2018 at 06:41:30PM +0200, Greg Kroah-Hartman wrote: > On Mon, May 21, 2018 at 01:08:44PM +0200, Johan Hovold wrote: > > Add missing const qualifiers to the termios hw-change helper parameters, > > which is used by few USB serial drivers. This specifically allows the > > pl2303 driver to use const arguments in one of its helper as well. > > > > Cc: Greg Kroah-Hartman > > Cc: Jiri Slaby > > Signed-off-by: Johan Hovold > > --- > > > > Greg, are you fine with me taking this one through my tree, or do prefer > > I split out the pl2303 bits? > > > > Note that this helper is only used by a few USB serial drivers and that > > the pl2303 bits depend on a new patch targeted for -next. > > Your tree is fine: > > Acked-by: Greg Kroah-Hartman I made this patch a prerequisite of the pl2303 change so that I could use const there from the start instead (i.e. I dropped the pl2303 chunk from this one). Now applied. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: force all memory allocations to node
On 21.05.2018 19:42, Adam Wallis wrote: On 5/21/2018 9:53 AM, Mathias Nyman wrote: On 21.05.2018 15:56, Adam Wallis wrote: Not sure if if there's any benefit in allocating the scratchpad structures from a closer node, or any harm? xhci driver doesn't really access scratchpad that frequently. I don't see how it would hurt and I think it's easier to keep all of the allocations consistent...however, let me know if you want those backed out. I guess they don't do any harm The port related structures are changing completely, so xhci->usb2_ports xhci->usb3_ports and other related are going away. Just sent a series for that. So some rebasing is needed. Would it be easier to just rebase on top of your for-usb-next branch in that case? I had been only testing on top of 4.17rc5. Let me know what you think is best/easiest for you. Rebasing on top of my for-usb-next would be easiest for me Thanks Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html