Re: [PATCH] rndis_host: Set random MAC for ZTE MF910
Hi Bjørn, On Thu, Jul 14, 2016 at 12:23 AM, Bjørn Mork wrote: > > Or how about the more generic?: > > if (bp[0] & 0x02) > eth_hw_addr_random(net); > else > ether_addr_copy(net->dev_addr, bp); > > That would catch similar screwups from other vendors too. Great idea, thanks. After submitting the patch I found some other devices with a similar bug, and there are probably even more out there. I will update patch and resubmit. -Kristian -- 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] rndis_host: Set random MAC for ZTE MF910
On Thu, Jul 14, 2016 at 9:54 AM, Kristian Evensen wrote: > Hi Bjørn, > > On Thu, Jul 14, 2016 at 12:23 AM, Bjørn Mork wrote: >> >> Or how about the more generic?: >> >> if (bp[0] & 0x02) >> eth_hw_addr_random(net); >> else >> ether_addr_copy(net->dev_addr, bp); >> >> That would catch similar screwups from other vendors too. > > Great idea, thanks. After submitting the patch I found some other > devices with a similar bug, and there are probably even more out > there. I will update patch and resubmit. Oh, and I forgot to say, please ignore this patch. I will change the title and description to better describe the functionality. -Kristian -- 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 net-next] rndis_host: Set valid random MAC on buggy devices
From: Kristian Evensen Some devices of the same type all export the same, random MAC address. This behavior has been seen on the ZTE MF910, MF823 and MF831, and there are probably more devices out there. Fix this by generating a valid random MAC address if we read a random MAC from device. Also, changed the memcpy() to ether_addr_copy(), as pointed out by checkpatch. Suggested-by: Bjørn Mork Signed-off-by: Kristian Evensen --- drivers/net/usb/rndis_host.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c index 524a47a281..4f4f71b 100644 --- a/drivers/net/usb/rndis_host.c +++ b/drivers/net/usb/rndis_host.c @@ -428,7 +428,11 @@ generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags) dev_err(&intf->dev, "rndis get ethaddr, %d\n", retval); goto halt_fail_and_release; } - memcpy(net->dev_addr, bp, ETH_ALEN); + + if (bp[0] & 0x02) + eth_hw_addr_random(net); + else + ether_addr_copy(net->dev_addr, bp); /* set a nonzero filter to enable data transfers */ memset(u.set, 0, sizeof *u.set); -- 2.5.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/4] usbnet: move the CDC parser into USB core
Oliver Neukum writes: > The dependencies were impossible to handle preventing > drivers for CDC devices not which are not network drivers > from using the common parser. > > Signed-off-by: Oliver Neukum > --- > drivers/net/usb/usbnet.c | 138 > drivers/usb/core/message.c | 153 > + > 2 files changed, 153 insertions(+), 138 deletions(-) Yes, we really want to keep the net and usb CDC parsing in the same place, and making all usb drivers depend on usbnet is of course not an option. > +/** > + * cdc_parse_cdc_header - parse the extra headers present in CDC devices > + * @hdr: the place to put the results of the parsing > + * @intf: the interface for which parsing is requested > + * @buffer: pointer to the extra headers to be parsed > + * @buflen: length of the extra headers > + * > + * This evaluates the extra headers present in CDC devices which > + * bind the interfaces for data and control and provide details > + * about the capabilities of the device. > + * > + * Return: number of bytes of the buffer parsed or -EINVAL > + * if the header is contradictory beyond salvage > + */ The description of Return sounds like a good interface to me. But, it's not what's implemented, is it? > +int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr, .. > + int cnt = 0; > + > + memset(hdr, 0x00, sizeof(struct usb_cdc_parsed_header)); > + hdr->phonet_magic_present = false; > + while (buflen > 0) { .. > + cnt++; > +next_desc: > + buflen -= elength; > + buffer += elength; > + } > + hdr->usb_cdc_union_desc = union_header; > + hdr->usb_cdc_header_desc = header; > + hdr->usb_cdc_mdlm_detail_desc = detail; > + hdr->usb_cdc_mdlm_desc = desc; > + hdr->usb_cdc_ether_desc = ether; > + return cnt; > +} This looks like it just counts the number of CDC descriptors and returns that count, possibly 0. It will not -EINVAL or any other negative number AFAICS. Unless I'm missing something? Bjørn -- 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: xhci-plat: Add generic PHY support
Generic phy support added in xhci platform driver. In the case of multiple phys to the xhci controller, this approach is helpful to pass multiple phandles to xhci platform driver from xhci device node. Signed-off-by: Srinath Mannam Reviewed-by: Ray Jui Reviewed-by: Scott Branden diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 676ea45..c734e2e 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -84,6 +85,52 @@ static int xhci_plat_start(struct usb_hcd *hcd) return xhci_run(hcd); } +static int xhci_platform_phy_on(struct usb_hcd *hcd) +{ + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); + int ret, phy_num; + + if (hcd->usb_phy) { + ret = usb_phy_init(hcd->usb_phy); + if (ret) + return ret; + } + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { + ret = phy_init(priv->phys[phy_num]); + if (ret) + goto err_exit_phy; + ret = phy_power_on(priv->phys[phy_num]); + if (ret) { + phy_exit(priv->phys[phy_num]); + goto err_exit_phy; + } + } + + return 0; + +err_exit_phy: + while (--phy_num >= 0) { + phy_power_off(priv->phys[phy_num]); + phy_exit(priv->phys[phy_num]); + } + + return ret; +} + +static void xhci_platform_phy_off(struct usb_hcd *hcd) +{ + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); + int phy_num; + + if (hcd->usb_phy) + usb_phy_shutdown(hcd->usb_phy); + + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { + phy_power_off(priv->phys[phy_num]); + phy_exit(priv->phys[phy_num]); + } +} + #ifdef CONFIG_OF static const struct xhci_plat_priv xhci_plat_marvell_armada = { .init_quirk = xhci_mvebu_mbus_init_quirk, @@ -146,8 +193,9 @@ static int xhci_plat_probe(struct platform_device *pdev) struct resource *res; struct usb_hcd *hcd; struct clk *clk; + struct xhci_plat_priv *priv; int ret; - int irq; + int irq, phy_num; if (usb_disabled()) return -ENODEV; @@ -199,10 +247,10 @@ static int xhci_plat_probe(struct platform_device *pdev) } xhci = hcd_to_xhci(hcd); + priv = hcd_to_xhci_priv(hcd); match = of_match_node(usb_xhci_of_match, node); if (match) { const struct xhci_plat_priv *priv_match = match->data; - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); /* Just copy data for now */ if (priv_match) @@ -233,12 +281,32 @@ static int xhci_plat_probe(struct platform_device *pdev) if (ret == -EPROBE_DEFER) goto put_usb3_hcd; hcd->usb_phy = NULL; - } else { - ret = usb_phy_init(hcd->usb_phy); - if (ret) + } + + priv->num_phys = of_count_phandle_with_args(pdev->dev.of_node, + "phys", "#phy-cells"); + + if (priv->num_phys > 0) { + priv->phys = devm_kcalloc(&pdev->dev, priv->num_phys, + sizeof(struct phy *), GFP_KERNEL); + if (!priv->phys) + return -ENOMEM; + } else + priv->num_phys = 0; + + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { + priv->phys[phy_num] = devm_of_phy_get_by_index( + &pdev->dev, pdev->dev.of_node, phy_num); + if (IS_ERR(priv->phys[phy_num])) { + ret = PTR_ERR(priv->phys[phy_num]); goto put_usb3_hcd; + } } + ret = xhci_platform_phy_on(hcd); + if (ret < 0) + goto put_usb3_hcd; + ret = usb_add_hcd(hcd, irq, IRQF_SHARED); if (ret) goto disable_usb_phy; @@ -254,7 +322,7 @@ dealloc_usb2_hcd: usb_remove_hcd(hcd); disable_usb_phy: - usb_phy_shutdown(hcd->usb_phy); + xhci_platform_phy_off(hcd); put_usb3_hcd: usb_put_hcd(xhci->shared_hcd); @@ -276,7 +344,7 @@ static int xhci_plat_remove(struct platform_device *dev) struct clk *clk = xhci->clk; usb_remove_hcd(xhci->shared_hcd); - usb_phy_shutdown(hcd->usb_phy); + xhci_platform_phy_off(hcd); usb_remove_hcd(hcd); usb_put_hcd(xhci->shared_hcd); diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h index 9af0cb4..1fe9c21 100644 --- a/drivers/usb/host/xhci-plat.h +++ b/drivers/usb/host/xhci-plat.h @@ -15,6 +15,8 @@ struct xhci_plat_
Re: [PATCH] usb: xhci-plat: Add generic PHY support
++ Felipe Balbi Thanks & Regards, Srinath Mannam. On Thu, Jul 14, 2016 at 2:03 PM, Srinath Mannam wrote: > Generic phy support added in xhci platform driver. > In the case of multiple phys to the xhci controller, this approach > is helpful to pass multiple phandles to xhci platform driver from > xhci device node. > > Signed-off-by: Srinath Mannam > Reviewed-by: Ray Jui > Reviewed-by: Scott Branden > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index 676ea45..c734e2e 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -84,6 +85,52 @@ static int xhci_plat_start(struct usb_hcd *hcd) > return xhci_run(hcd); > } > > +static int xhci_platform_phy_on(struct usb_hcd *hcd) > +{ > + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); > + int ret, phy_num; > + > + if (hcd->usb_phy) { > + ret = usb_phy_init(hcd->usb_phy); > + if (ret) > + return ret; > + } > + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { > + ret = phy_init(priv->phys[phy_num]); > + if (ret) > + goto err_exit_phy; > + ret = phy_power_on(priv->phys[phy_num]); > + if (ret) { > + phy_exit(priv->phys[phy_num]); > + goto err_exit_phy; > + } > + } > + > + return 0; > + > +err_exit_phy: > + while (--phy_num >= 0) { > + phy_power_off(priv->phys[phy_num]); > + phy_exit(priv->phys[phy_num]); > + } > + > + return ret; > +} > + > +static void xhci_platform_phy_off(struct usb_hcd *hcd) > +{ > + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); > + int phy_num; > + > + if (hcd->usb_phy) > + usb_phy_shutdown(hcd->usb_phy); > + > + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { > + phy_power_off(priv->phys[phy_num]); > + phy_exit(priv->phys[phy_num]); > + } > +} > + > #ifdef CONFIG_OF > static const struct xhci_plat_priv xhci_plat_marvell_armada = { > .init_quirk = xhci_mvebu_mbus_init_quirk, > @@ -146,8 +193,9 @@ static int xhci_plat_probe(struct platform_device *pdev) > struct resource *res; > struct usb_hcd *hcd; > struct clk *clk; > + struct xhci_plat_priv *priv; > int ret; > - int irq; > + int irq, phy_num; > > if (usb_disabled()) > return -ENODEV; > @@ -199,10 +247,10 @@ static int xhci_plat_probe(struct platform_device *pdev) > } > > xhci = hcd_to_xhci(hcd); > + priv = hcd_to_xhci_priv(hcd); > match = of_match_node(usb_xhci_of_match, node); > if (match) { > const struct xhci_plat_priv *priv_match = match->data; > - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); > > /* Just copy data for now */ > if (priv_match) > @@ -233,12 +281,32 @@ static int xhci_plat_probe(struct platform_device *pdev) > if (ret == -EPROBE_DEFER) > goto put_usb3_hcd; > hcd->usb_phy = NULL; > - } else { > - ret = usb_phy_init(hcd->usb_phy); > - if (ret) > + } > + > + priv->num_phys = of_count_phandle_with_args(pdev->dev.of_node, > + "phys", "#phy-cells"); > + > + if (priv->num_phys > 0) { > + priv->phys = devm_kcalloc(&pdev->dev, priv->num_phys, > + sizeof(struct phy *), GFP_KERNEL); > + if (!priv->phys) > + return -ENOMEM; > + } else > + priv->num_phys = 0; > + > + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { > + priv->phys[phy_num] = devm_of_phy_get_by_index( > + &pdev->dev, pdev->dev.of_node, phy_num); > + if (IS_ERR(priv->phys[phy_num])) { > + ret = PTR_ERR(priv->phys[phy_num]); > goto put_usb3_hcd; > + } > } > > + ret = xhci_platform_phy_on(hcd); > + if (ret < 0) > + goto put_usb3_hcd; > + > ret = usb_add_hcd(hcd, irq, IRQF_SHARED); > if (ret) > goto disable_usb_phy; > @@ -254,7 +322,7 @@ dealloc_usb2_hcd: > usb_remove_hcd(hcd); > > disable_usb_phy: > - usb_phy_shutdown(hcd->usb_phy); > + xhci_platform_phy_off(hcd); > > put_usb3_hcd: > usb_put_hcd(xhci->shared_hcd); > @@ -276,7 +344,7 @@ static int xhci_plat_remove(struct platform_device *dev) > struct clk *clk = xhci->clk; > > usb_remov
[PATCH v7 0/5] support rockchip dwc3 driver
This series add support for rockchip dwc3 driver, and add additional optional properties for specific platforms (e.g., rockchip rk3399 platform). William Wu (5): usb: dwc3: of-simple: add compatible for rockchip rk3399 usb: dwc3: add dis_u2_freeclk_exists_quirk usb: dwc3: make usb2 phy utmi interface configurable in DT usb: dwc3: add dis_del_phy_power_chg_quirk usb: dwc3: rockchip: add devicetree bindings documentation Documentation/devicetree/bindings/usb/dwc3.txt | 8 +++ .../devicetree/bindings/usb/rockchip,dwc3.txt | 59 ++ drivers/usb/dwc3/core.c| 35 + drivers/usb/dwc3/core.h| 18 +++ drivers/usb/dwc3/dwc3-of-simple.c | 1 + 5 files changed, 121 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt -- 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 v7 3/5] usb: dwc3: make usb2 phy utmi interface configurable in DT
Add snps,phyif-utmi-width devicetree property to configure the UTMI+ PHY with an 8- or 16-bit interface. UTMI+ PHY interface is a hardware property, and it's platform dependent. Normally,the PHYIF can be configured during coreconsultant. But for some specific USB cores(e.g. rk3399 SoC DWC3), the default PHYIF configuration value is fault, so we need to reconfigure it by software. And refer to the DWC3 databook, the GUSB2PHYCFG.USBTRDTIM must be set to the corresponding value according to the UTMI+ PHY interface. Signed-off-by: William Wu --- Changes in v7: - remove quirk and use only one property to configure utmi (Heiko, Rob Herring) Changes in v6: - use '-' instead of '_' in dts (Rob Herring) Changes in v5: - None Changes in v4: - rebase on top of balbi testing/next, remove pdata (balbi) Changes in v3: - None Changes in v2: - add a quirk for phyif_utmi (balbi) Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ drivers/usb/dwc3/core.c| 25 + drivers/usb/dwc3/core.h| 10 ++ 3 files changed, 38 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt index 020b0e9..00cc541 100644 --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ b/Documentation/devicetree/bindings/usb/dwc3.txt @@ -47,6 +47,9 @@ Optional properties: - snps,hird-threshold: HIRD threshold - snps,hsphy_interface: High-Speed PHY interface selection between "utmi" for UTMI+ and "ulpi" for ULPI when the DWC_USB3_HSPHY_INTERFACE has value 3. + - snps,phyif-utmi-width: the value to configure the core to support a UTMI+ PHY + with an 8- or 16-bit interface. Value 8 select 8-bit + interface, value 16 select 16-bit interface. - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ register for post-silicon frame length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 0b7bfd2..40c54db 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -408,6 +408,8 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) static int dwc3_phy_setup(struct dwc3 *dwc) { u32 reg; + u32 usbtrdtim; + u8 phyif; int ret; reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)); @@ -503,6 +505,16 @@ static int dwc3_phy_setup(struct dwc3 *dwc) if (dwc->dis_u2_freeclk_exists_quirk) reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS; + if (dwc->phyif_utmi_width > 0) { + reg &= ~(DWC3_GUSB2PHYCFG_PHYIF_MASK | + DWC3_GUSB2PHYCFG_USBTRDTIM_MASK); + usbtrdtim = (dwc->phyif_utmi_width == 16) ? + USBTRDTIM_UTMI_16_BIT : USBTRDTIM_UTMI_8_BIT; + phyif = (dwc->phyif_utmi_width == 16) ? 1 : 0; + reg |= DWC3_GUSB2PHYCFG_PHYIF(phyif) | + DWC3_GUSB2PHYCFG_USBTRDTIM(usbtrdtim); + } + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); return 0; @@ -900,6 +912,19 @@ static int dwc3_probe(struct platform_device *pdev) "snps,is-utmi-l1-suspend"); device_property_read_u8(dev, "snps,hird-threshold", &hird_threshold); + + ret = device_property_read_u8(dev, "snps,phyif-utmi-width", + &dwc->phyif_utmi_width); + if (ret < 0) { + dwc->phyif_utmi_width = 0; + } else if (dwc->phyif_utmi_width != 16 && + dwc->phyif_utmi_width != 8) { + dev_err(dev, "unsupported utmi interface width %d\n", + dwc->phyif_utmi_width); + ret = -EINVAL; + goto err0; + } + dwc->usb3_lpm_capable = device_property_read_bool(dev, "snps,usb3_lpm_capable"); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index f321a5c..99a72c7 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -203,6 +203,12 @@ #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6) #define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4) #define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) +#define DWC3_GUSB2PHYCFG_PHYIF(n) (n << 3) +#define DWC3_GUSB2PHYCFG_PHYIF_MASKDWC3_GUSB2PHYCFG_PHYIF(1) +#define DWC3_GUSB2PHYCFG_USBTRDTIM(n) (n << 10) +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASKDWC3_GUSB2PHYCFG_USBTRDTIM(0xf) +#define USBTRDTIM_UTMI_8_BIT 9 +#define USBTRDTIM_UTMI_16_BIT 5 /* Global USB2 PHY Vendor Control Register */ #define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25) @@ -770,6 +776,9 @@ struct dwc3_scratchpad_array { * @test_mode_nr: test feature selector * @lpm_nyet_threshold: LPM NYET response threshold * @hird_threshold: HIRD threshold + * @phyif_utmi_width: UTMI+ PHY interfa
[PATCH v7 2/5] usb: dwc3: add dis_u2_freeclk_exists_quirk
Add a quirk to clear the GUSB2PHYCFG.U2_FREECLK_EXISTS bit, which specifies whether the USB2.0 PHY provides a free-running PHY clock, which is active when the clock control input is active. Signed-off-by: William Wu --- Changes in v7: - None Changes in v6: - use '-' instead of '_' in dts (Rob Herring) Changes in v5: - None Changes in v4: - rebase on top of balbi testing/next, remove pdata (balbi) Changes in v3: - None Changes in v2: - None Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ drivers/usb/dwc3/core.c| 5 + drivers/usb/dwc3/core.h| 5 + 3 files changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt index 7d7ce08..020b0e9 100644 --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ b/Documentation/devicetree/bindings/usb/dwc3.txt @@ -39,6 +39,9 @@ Optional properties: disabling the suspend signal to the PHY. - snps,dis_rxdet_inp3_quirk: when set core will disable receiver detection in PHY P3 power state. + - snps,dis-u2-freeclk-exists-quirk: when set, clear the u2_freeclk_exists + in GUSB2PHYCFG, specify that USB2 PHY doesn't provide + a free-running PHY clock. - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal utmi_l1_suspend_n, false when asserts utmi_sleep_n - snps,hird-threshold: HIRD threshold diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9466431..0b7bfd2 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -500,6 +500,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc) if (dwc->dis_enblslpm_quirk) reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM; + if (dwc->dis_u2_freeclk_exists_quirk) + reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS; + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); return 0; @@ -924,6 +927,8 @@ static int dwc3_probe(struct platform_device *pdev) "snps,dis_enblslpm_quirk"); dwc->dis_rxdet_inp3_quirk = device_property_read_bool(dev, "snps,dis_rxdet_inp3_quirk"); + dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev, + "snps,dis-u2-freeclk-exists-quirk"); dwc->tx_de_emphasis_quirk = device_property_read_bool(dev, "snps,tx_de_emphasis_quirk"); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 45d6de5..f321a5c 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -199,6 +199,7 @@ /* Global USB2 PHY Configuration Register */ #define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31) +#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS (1 << 30) #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6) #define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4) #define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) @@ -799,6 +800,9 @@ struct dwc3_scratchpad_array { * @dis_u2_susphy_quirk: set if we disable usb2 suspend phy * @dis_enblslpm_quirk: set if we clear enblslpm in GUSB2PHYCFG, * disabling the suspend signal to the PHY. + * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists + * in GUSB2PHYCFG, specify that USB2 PHY doesn't + * provide a free-running PHY clock. * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk * @tx_de_emphasis: Tx de-emphasis value * 0 - -6dB de-emphasis @@ -942,6 +946,7 @@ struct dwc3 { unsigneddis_u2_susphy_quirk:1; unsigneddis_enblslpm_quirk:1; unsigneddis_rxdet_inp3_quirk:1; + unsigneddis_u2_freeclk_exists_quirk:1; unsignedtx_de_emphasis_quirk:1; unsignedtx_de_emphasis:2; -- 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 v7 4/5] usb: dwc3: add dis_del_phy_power_chg_quirk
Add a quirk to clear the GUSB3PIPECTL.DELAYP1TRANS bit, which specifies whether disable delay PHY power change from P0 to P1/P2/P3 when link state changing from U0 to U1/U2/U3 respectively. Signed-off-by: William Wu --- Changes in v7: - None Changes in v6: - use '-' instead of '_' in dts (Rob Herring) Changes in v5: - None Changes in v4: - rebase on top of balbi testing/next, remove pdata (balbi) Changes in v3: - None Changes in v2: - None Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++ drivers/usb/dwc3/core.c| 5 + drivers/usb/dwc3/core.h| 3 +++ 3 files changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt index 00cc541..7832e19 100644 --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ b/Documentation/devicetree/bindings/usb/dwc3.txt @@ -42,6 +42,8 @@ Optional properties: - snps,dis-u2-freeclk-exists-quirk: when set, clear the u2_freeclk_exists in GUSB2PHYCFG, specify that USB2 PHY doesn't provide a free-running PHY clock. + - snps,dis-del-phy-power-chg-quirk: when set core will change PHY power + from P0 to P1/P2/P3 without delay. - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal utmi_l1_suspend_n, false when asserts utmi_sleep_n - snps,hird-threshold: HIRD threshold diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 40c54db..bff5ae4 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -450,6 +450,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc) if (dwc->dis_u3_susphy_quirk) reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; + if (dwc->dis_del_phy_power_chg_quirk) + reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE; + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg); reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); @@ -954,6 +957,8 @@ static int dwc3_probe(struct platform_device *pdev) "snps,dis_rxdet_inp3_quirk"); dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev, "snps,dis-u2-freeclk-exists-quirk"); + dwc->dis_del_phy_power_chg_quirk = device_property_read_bool(dev, + "snps,dis-del-phy-power-chg-quirk"); dwc->tx_de_emphasis_quirk = device_property_read_bool(dev, "snps,tx_de_emphasis_quirk"); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 99a72c7..753d977a 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -812,6 +812,8 @@ struct dwc3_scratchpad_array { * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists * in GUSB2PHYCFG, specify that USB2 PHY doesn't * provide a free-running PHY clock. + * @dis_del_phy_power_chg_quirk: set if we disable delay phy power + * change quirk. * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk * @tx_de_emphasis: Tx de-emphasis value * 0 - -6dB de-emphasis @@ -957,6 +959,7 @@ struct dwc3 { unsigneddis_enblslpm_quirk:1; unsigneddis_rxdet_inp3_quirk:1; unsigneddis_u2_freeclk_exists_quirk:1; + unsigneddis_del_phy_power_chg_quirk:1; unsignedtx_de_emphasis_quirk:1; unsignedtx_de_emphasis:2; -- 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 v7 1/5] usb: dwc3: of-simple: add compatible for rockchip rk3399
Rockchip platform merely enable usb3 clocks and populate its children. So we can use this generic glue layer to support Rockchip dwc3. Signed-off-by: William Wu --- Changes in v7: - None Changes in v6: - None Changes in v5: - change compatible from "rockchip,dwc3" to "rockchip,rk3399-dwc3" (Heiko) Changes in v4: - None Changes in v3: - None Changes in v2: - sort the list of_dwc3_simple_match (Doug) drivers/usb/dwc3/dwc3-of-simple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c index 9743353..05c9349 100644 --- a/drivers/usb/dwc3/dwc3-of-simple.c +++ b/drivers/usb/dwc3/dwc3-of-simple.c @@ -161,6 +161,7 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = { static const struct of_device_id of_dwc3_simple_match[] = { { .compatible = "qcom,dwc3" }, + { .compatible = "rockchip,rk3399-dwc3" }, { .compatible = "xlnx,zynqmp-dwc3" }, { /* Sentinel */ } }; -- 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 v7 5/5] usb: dwc3: rockchip: add devicetree bindings documentation
This patch adds the devicetree documentation required for Rockchip USB3.0 core wrapper consisting of USB3.0 IP from Synopsys. It supports DRD mode, and could operate in device mode (SS, HS, FS) and host mode (SS, HS, FS, LS). Signed-off-by: William Wu Acked-by: Rob Herring --- Changes in v7: - add Acked-by (Rob Herring) Changes in v6: - rename bus_clk, and add usbdrd3_1 node as an example (Heiko) Changes in v5: - rename clock-names, and remove unnecessary clocks (Heiko) Changes in v4: - modify commit log, and add phy documentation location (Sergei) Changes in v3: - add dwc3 address (balbi) Changes in v2: - add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (balbi, Brian) .../devicetree/bindings/usb/rockchip,dwc3.txt | 59 ++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt new file mode 100644 index 000..0536a93 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt @@ -0,0 +1,59 @@ +Rockchip SuperSpeed DWC3 USB SoC controller + +Required properties: +- compatible: should contain "rockchip,rk3399-dwc3" for rk3399 SoC +- clocks: A list of phandle + clock-specifier pairs for the + clocks listed in clock-names +- clock-names: Should contain the following: + "ref_clk"Controller reference clk, have to be 24 MHz + "suspend_clk"Controller suspend clk, have to be 24 MHz or 32 KHz + "bus_clk"Master/Core clock, have to be >= 62.5 MHz for SS + operation and >= 30MHz for HS operation + "grf_clk"Controller grf clk + +Required child node: +A child node must exist to represent the core DWC3 IP block. The name of +the node is not important. The content of the node is defined in dwc3.txt. + +Phy documentation is provided in the following places: +Documentation/devicetree/bindings/phy/rockchip,dwc3-usb-phy.txt + +Example device nodes: + + usbdrd3_0: usb@fe80 { + compatible = "rockchip,rk3399-dwc3"; + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>, +<&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_GRF>; + clock-names = "ref_clk", "suspend_clk", + "bus_clk", "grf_clk"; + #address-cells = <2>; + #size-cells = <2>; + ranges; + status = "disabled"; + usbdrd_dwc3_0: dwc3@fe80 { + compatible = "snps,dwc3"; + reg = <0x0 0xfe80 0x0 0x10>; + interrupts = ; + dr_mode = "otg"; + status = "disabled"; + }; + }; + + usbdrd3_1: usb@fe90 { + compatible = "rockchip,rk3399-dwc3"; + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru SCLK_USB3OTG1_SUSPEND>, +<&cru ACLK_USB3OTG1>, <&cru ACLK_USB3_GRF>; + clock-names = "ref_clk", "suspend_clk", + "bus_clk", "grf_clk"; + #address-cells = <2>; + #size-cells = <2>; + ranges; + status = "disabled"; + usbdrd_dwc3_1: dwc3@fe90 { + compatible = "snps,dwc3"; + reg = <0x0 0xfe90 0x0 0x10>; + interrupts = ; + dr_mode = "otg"; + status = "disabled"; + }; + }; -- 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 2/4] cdc-acm: use the common parser
Oliver Neukum writes: > This introduces the common parser for extra CDC headers now that it no longer > depends on usbnet. > > Signed-off-by: Oliver Neukum > --- > drivers/usb/class/cdc-acm.c | 68 > +++-- > 1 file changed, 10 insertions(+), 58 deletions(-) > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c > index def5a54..1620542 100644 > --- a/drivers/usb/class/cdc-acm.c > +++ b/drivers/usb/class/cdc-acm.c > @@ -1147,6 +1147,7 @@ static int acm_probe(struct usb_interface *intf, > { > struct usb_cdc_union_desc *union_header = NULL; > struct usb_cdc_country_functional_desc *cfd = NULL; > + struct usb_cdc_call_mgmt_descriptor *cmd = NULL; > unsigned char *buffer = intf->altsetting->extra; > int buflen = intf->altsetting->extralen; > struct usb_interface *control_interface; This patch looks good to me. Feel free to add Reviewed-by: Bjørn Mork if you want to. But if you are spinning a new series, then I have one minor comment: Do we really need all these additional descriptor pointers? Or at least, do we need them in this scope? acm_probe() is very long and it is difficult to keep track of all the variables. Moving some of the variables to a more local scope would help. BTW, "cmd" is often used for "command", so I'n not sure it's a good name for this descriptor, as this grep shows: bjorn@miraculix:/usr/local/src/git/linux$ git grep -n cmd drivers/usb/class/cdc-acm.c drivers/usb/class/cdc-acm.c:1010: unsigned int cmd, unsigned long arg) drivers/usb/class/cdc-acm.c:1015: switch (cmd) { drivers/usb/class/cdc-acm.c:1150: struct usb_cdc_call_mgmt_descriptor *cmd = NULL; drivers/usb/class/cdc-acm.c:1214: cmd = hdr.usb_cdc_call_mgmt_descriptor; drivers/usb/class/cdc-acm.c:1215: if (cmd) drivers/usb/class/cdc-acm.c:1216: call_interface_num = cmd->bDataInterface; But as you also see, it is only used once to assign call_interface_num. If the length of that assignment is the problem, then I suggest a) renaming hdr => h saves you two characters everywhere the descriptors are used and reducess call_interface_num = h.usb_cdc_call_mgmt_descriptor->bDataInterface; to 83 columns width. b) or moving the variable into the if () scope: if (hdr.usb_cdc_call_mgmt_descriptor) { struct usb_cdc_call_mgmt_descriptor *cm; cm = hdr.usb_cdc_call_mgmt_descriptor; call_interface_num = cmd->cm; } Personally, I find b very awkward. IMHO, it would be better to either break the 80 column rule or break the assigment into two lines. In the longer run, I think we also should consider if the long field names in struct usb_cdc_parsed_header helps on readability. It makes the names self explanatory, but that does not help when you cannot use the field name directly because it is so long... Anyway, back to cdc-acm: "union_header" is also used only once, and only inside an else scope: bjorn@miraculix:/usr/local/src/git/linux$ git grep -n union_header drivers/usb/class/cdc-acm.c drivers/usb/class/cdc-acm.c:1148: struct usb_cdc_union_desc *union_header = NULL; drivers/usb/class/cdc-acm.c:1213: union_header = hdr.usb_cdc_union_desc; drivers/usb/class/cdc-acm.c:1218: if (!union_header) { drivers/usb/class/cdc-acm.c:1239: control_interface = usb_ifnum_to_if(usb_dev, union_header->bMasterInterface0); drivers/usb/class/cdc-acm.c:1240: data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = union_header->bSlaveInterface0)); Those assignments are already breaking 80 columns. And that nested assignment in there - I didn't notice that before. That's ugly. This could just as well be rewritten to if (!hdr.usb_cdc_union_desc) { .. } else { control_interface = usb_ifnum_to_if(usb_dev, hdr.usb_cdc_union_desc->bMasterInterface0); data_interface_num = hdr.usb_cdc_union_desc->->bSlaveInterface0; data_interface = usb_ifnum_to_if(usb_dev, data_interface_num); } And then the last one. "cfd" is also used only in a single block. It's used multiple times there, so a local variable makes sense though: bjorn@miraculix:/usr/local/src/git/linux$ git grep -n cfd drivers/usb/class/cdc-acm.c drivers/usb/class/cdc-acm.c:1149: struct usb_cdc_country_functional_desc *cfd = NULL; drivers/usb/class/cdc-acm.c:1438: cfd = hdr.usb_cdc_country_functional_desc; drivers/usb/class/cdc-acm.c:1439: if (cfd) { /* export the country data */ drivers/usb/class/cdc-acm.c:1440: acm->country_codes = kmalloc(cfd->bLength - 4, GFP_KERNEL); drivers/usb/class/cdc-acm.c:1443: acm->country_code_size = cfd->bLength - 4; drivers/usb/class/cdc-acm.c:1444: memcpy(acm->country_codes, (u8 *)&cfd->wCountyCode0, drivers/usb/class/cdc-acm.c:1445:
Re: [PATCH 4/4] cdc-wdm: use the common CDC parser
Oliver Neukum writes: > Now that the common parser resides in USB core, it can > be used for CDC-WDM. Looking good to me. Nice code reduction without any additional cost here. Reviewed-by: Bjørn Mork Bjørn -- 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 3/4] cdc-acm: cleanup error handling
Oliver Neukum writes: > A small update to unify error handling during probe(). > > Signed-off-by: Oliver Neukum > --- > drivers/usb/class/cdc-acm.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c > index 1620542..2e5dea8 100644 > --- a/drivers/usb/class/cdc-acm.c > +++ b/drivers/usb/class/cdc-acm.c > @@ -1161,7 +1161,6 @@ static int acm_probe(struct usb_interface *intf, > int minor; > int ctrlsize, readsize; > u8 *buf; > - u8 ac_management_function = 0; > int call_interface_num = -1; > int data_interface_num = -1; > unsigned long quirks; This should be been in patch #2 > @@ -1329,11 +1328,8 @@ made_compressed_probe: > goto alloc_fail; > > minor = acm_alloc_minor(acm); > - if (minor < 0) { > - dev_err(&intf->dev, "no more free acm devices\n"); > - kfree(acm); > - return -ENODEV; > - } > + if (minor < 0) > + goto alloc_fail1; > > ctrlsize = usb_endpoint_maxp(epctrl); > readsize = usb_endpoint_maxp(epread) * > @@ -1524,6 +1520,7 @@ alloc_fail4: > usb_free_coherent(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma); > alloc_fail2: > acm_release_minor(acm); > +alloc_fail1: > kfree(acm); > alloc_fail: > return rv; Reviewed-by: Bjørn Mork -- 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: serial: use variable for status
This patch turns status in a variable read once from the URB. The long term plan is to deliver status to the callback. In addition it makes the code a bit more elegant. Signed-off-by: Oliver Neukum --- drivers/usb/serial/generic.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c index ae8c036..85cf469 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -351,6 +351,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb) unsigned char *data = urb->transfer_buffer; unsigned long flags; int i; + int status = urb->status; for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) { if (urb == port->read_urbs[i]) @@ -360,22 +361,22 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb) dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i, urb->actual_length); - switch (urb->status) { + switch (status) { case 0: break; case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: dev_dbg(&port->dev, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; case -EPIPE: dev_err(&port->dev, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; default: dev_dbg(&port->dev, "%s - nonzero urb status: %d\n", - __func__, urb->status); + __func__, status); goto resubmit; } @@ -400,6 +401,7 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) unsigned long flags; struct usb_serial_port *port = urb->context; int i; + int status = urb->status; for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) { if (port->write_urbs[i] == urb) @@ -410,22 +412,22 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) set_bit(i, &port->write_urbs_free); spin_unlock_irqrestore(&port->lock, flags); - switch (urb->status) { + switch (status) { case 0: break; case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: dev_dbg(&port->dev, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; case -EPIPE: dev_err_console(port, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; default: dev_err_console(port, "%s - nonzero urb status: %d\n", - __func__, urb->status); + __func__, status); goto resubmit; } -- 2.1.4 -- 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: serial: use variable for status
Oliver Neukum writes: > @@ -360,22 +361,22 @@ void usb_serial_generic_read_bulk_callback(struct urb > *urb) > > dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i, > urb->actual_length); > - switch (urb->status) { > + switch (status) { > case 0: > break; > case -ENOENT: > case -ECONNRESET: > case -ESHUTDOWN: > dev_dbg(&port->dev, "%s - urb stopped: %d\n", > - __func__, urb->status); > + __func__, status); __func__ is redundant in dev_dbg(). Removing it will allow you to unbreak these lines, making the code look a lot nicer. Bjørn -- 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/1] usb: add HCD providers
On Wed, Jul 13, 2016 at 04:40:53PM +0200, Rafał Miłecki wrote: > On 13 July 2016 at 15:50, Felipe Balbi wrote: > > Rafał Miłecki writes: > >> On 13 July 2016 at 15:20, Felipe Balbi > >> wrote: > >>> Rafał Miłecki writes: > Hi again, > > This is my second try of getting HCD providers into usb subsystem. > > During discussion of V1 I realized there are about 26 drivers adding a > single HCD and all of them would need to be modified. So instead I > decided to put relevant code in usb_add_hcd. It checks if the HCD we > register is a primary one and if so, it registers a proper provider. > > Please note that of_hcd_xlate_simple was also extended to allow getting > shared HCD (which is used e.g. in case of XHCI). > > So now you can have something like: > > ohci: ohci@21000 { > #usb-cells = <0>; > compatible = "generic-ohci"; > reg = <0x1000 0x1000>; > interrupts = ; > }; > > ehci: ehci@22000 { > #usb-cells = <0>; > compatible = "generic-ehci"; > reg = <0x2000 0x1000>; > interrupts = ; > }; > > xhci: xhci@23000 { > #usb-cells = <1>; > compatible = "generic-xhci"; > reg = <0x3000 0x1000>; > interrupts = ; > }; > > The last (second) patch is not supposed to be applied, it's used only as > a proof and example of how providers can be used. > >>> > >>> nowhere here (or in previous patch) you clarify why exactly you need > >>> this. What is your LED trigger supposed to do? Why can't it handle ports > >>> changing number in different boots? Why do we need this at all? Why is > >>> your code DT-specific? > >>> > >>> There are still too many 'unknowns' here. > >> > >> Are you sure you saw my reply to Peter's question? > >> > >> http://www.spinics.net/lists/linux-usb/msg143708.html > >> http://marc.info/?l=linux-usb&m=146838735627093&w=2 > >> > >> I think it should answer (some of?) your questions. Can you read it > >> and see if it gets a bit clearer? > > > > well, all that says is that you're writing a LED trigger to toggle LED > > when a USB device gets added to a specified port. I don't think you need > > the actual port number for that. You should have a phandle to the actual > > port, whatever its number is, or a phandle to the (root-)Hub and a port > > number from that hub. > > > > The problem, really, is that DT descriptor of USB Hosts is very, very > > minimal. Perhaps there's something more extensively defined from the > > original Open Firmware USB Addendum. > > Thanks for your effort and looking at this closely. You're right, I'm > interested in referencing USB ports, but I'm using controller phandle > (and then I specify ports manually). > > Having each port described by DT would be helpful, it's just something > I didn't find implemented, so I started looking for different ways. It > seems I should have picked a different solution. > > So should I work on describing USB ports in DT instead? This looks > like a complex thing to describe, so I'd like to ask for some guidance > first. What do you think about following schema/example? > > ohci@1000 { > compatible = "generic-ohci"; > reg = <0x1000 0x1000>; > interrupts = ; > > primary-hcd { > ohci_port0: port@0 { > reg = <0>; > }; > > ohci_port1: port@1 { > reg = <1>; > }; > } > }; > > ehci@2000 { > compatible = "generic-ehci"; > reg = <0x2000 0x1000>; > interrupts = ; > > primary-hcd { > ehci_port0: port@0 { > reg = <0>; > }; > > ehci_port1: port@1 { > reg = <1>; > }; > } > }; > > xhci@3000 { > compatible = "generic-xhci"; > reg = <0x3000 0x1000>; > interrupts = ; > > primary-hcd { > }; > > shared-hcd { > xhci_port0: port@0 { > reg = <0>; > }; > } > }; > > With such a DT struct, how could I query port for a Linux-assigned number? > > For example with OHCI, EHCI and XHCI drivers compiled, Linux assigns > number 4 to my XHCI's shared HCD's root hub: > xhci-hcd 18023000.xhci: xHCI Host Controller > xhci-hcd 18023000.xhci: new USB bus registered, assigned bus number 4 > hub 4-0:1.0: USB hub found > hub 4-0:1.0: 1 port detected > > If I disable OHCI and EHCI I get: > xhci-hcd xhci-hcd.0: xHCI Host Controller > xhci-hcd xhci-hcd.0: new USB bus registered, assigned bus number 2 > hub 2-0:1.0: USB hub found > hub 2-0:1.0: 1 port detected > > So I need my "usbport" trigger driver to be able to get "4-1" in the > first case and "2-1" in the second case. I guess I shou
Re: [PATCH] usb: xhci-plat: Add generic PHY support
Hi, Srinath Mannam writes: > Generic phy support added in xhci platform driver. > In the case of multiple phys to the xhci controller, this approach > is helpful to pass multiple phandles to xhci platform driver from > xhci device node. > > Signed-off-by: Srinath Mannam > Reviewed-by: Ray Jui > Reviewed-by: Scott Branden I have no issues with this, but Mathias is off for a few weeks. -- balbi signature.asc Description: PGP signature
Re: [PATCH V2 0/1] usb: add HCD providers
Hi, Peter Chen writes: > On Wed, Jul 13, 2016 at 04:40:53PM +0200, Rafał Miłecki wrote: >> On 13 July 2016 at 15:50, Felipe Balbi wrote: >> > Rafał Miłecki writes: >> >> On 13 July 2016 at 15:20, Felipe Balbi >> >> wrote: >> >>> Rafał Miłecki writes: >> Hi again, >> >> This is my second try of getting HCD providers into usb subsystem. >> >> During discussion of V1 I realized there are about 26 drivers adding a >> single HCD and all of them would need to be modified. So instead I >> decided to put relevant code in usb_add_hcd. It checks if the HCD we >> register is a primary one and if so, it registers a proper provider. >> >> Please note that of_hcd_xlate_simple was also extended to allow getting >> shared HCD (which is used e.g. in case of XHCI). >> >> So now you can have something like: >> >> ohci: ohci@21000 { >> #usb-cells = <0>; >> compatible = "generic-ohci"; >> reg = <0x1000 0x1000>; >> interrupts = ; >> }; >> >> ehci: ehci@22000 { >> #usb-cells = <0>; >> compatible = "generic-ehci"; >> reg = <0x2000 0x1000>; >> interrupts = ; >> }; >> >> xhci: xhci@23000 { >> #usb-cells = <1>; >> compatible = "generic-xhci"; >> reg = <0x3000 0x1000>; >> interrupts = ; >> }; >> >> The last (second) patch is not supposed to be applied, it's used only as >> a proof and example of how providers can be used. >> >>> >> >>> nowhere here (or in previous patch) you clarify why exactly you need >> >>> this. What is your LED trigger supposed to do? Why can't it handle ports >> >>> changing number in different boots? Why do we need this at all? Why is >> >>> your code DT-specific? >> >>> >> >>> There are still too many 'unknowns' here. >> >> >> >> Are you sure you saw my reply to Peter's question? >> >> >> >> http://www.spinics.net/lists/linux-usb/msg143708.html >> >> http://marc.info/?l=linux-usb&m=146838735627093&w=2 >> >> >> >> I think it should answer (some of?) your questions. Can you read it >> >> and see if it gets a bit clearer? >> > >> > well, all that says is that you're writing a LED trigger to toggle LED >> > when a USB device gets added to a specified port. I don't think you need >> > the actual port number for that. You should have a phandle to the actual >> > port, whatever its number is, or a phandle to the (root-)Hub and a port >> > number from that hub. >> > >> > The problem, really, is that DT descriptor of USB Hosts is very, very >> > minimal. Perhaps there's something more extensively defined from the >> > original Open Firmware USB Addendum. >> >> Thanks for your effort and looking at this closely. You're right, I'm >> interested in referencing USB ports, but I'm using controller phandle >> (and then I specify ports manually). >> >> Having each port described by DT would be helpful, it's just something >> I didn't find implemented, so I started looking for different ways. It >> seems I should have picked a different solution. >> >> So should I work on describing USB ports in DT instead? This looks >> like a complex thing to describe, so I'd like to ask for some guidance >> first. What do you think about following schema/example? >> >> ohci@1000 { >> compatible = "generic-ohci"; >> reg = <0x1000 0x1000>; >> interrupts = ; >> >> primary-hcd { >> ohci_port0: port@0 { >> reg = <0>; >> }; >> >> ohci_port1: port@1 { >> reg = <1>; >> }; >> } >> }; >> >> ehci@2000 { >> compatible = "generic-ehci"; >> reg = <0x2000 0x1000>; >> interrupts = ; >> >> primary-hcd { >> ehci_port0: port@0 { >> reg = <0>; >> }; >> >> ehci_port1: port@1 { >> reg = <1>; >> }; >> } >> }; >> >> xhci@3000 { >> compatible = "generic-xhci"; >> reg = <0x3000 0x1000>; >> interrupts = ; >> >> primary-hcd { >> }; >> >> shared-hcd { >> xhci_port0: port@0 { >> reg = <0>; >> }; >> } >> }; >> >> With such a DT struct, how could I query port for a Linux-assigned number? >> >> For example with OHCI, EHCI and XHCI drivers compiled, Linux assigns >> number 4 to my XHCI's shared HCD's root hub: >> xhci-hcd 18023000.xhci: xHCI Host Controller >> xhci-hcd 18023000.xhci: new USB bus registered, assigned bus number 4 >> hub 4-0:1.0: USB hub found >> hub 4-0:1.0: 1 port detected >> >> If I disable OHCI and EHCI I get: >> xhci-hcd xhci-hcd.0: xHCI Host Controller >> xhci-hcd xhci-hcd.0: new USB bus registered, assigned bus number 2 >> hub 2-0:1.0: USB hub
[PATCH] usb: MAINTAINERS: Oliver Neukum is the new uas maintainer
Oliver Neukum is taking over uas maintainership from me and Gerd Hoffmann. Cc: Oliver Neukum Cc: Gerd Hoffmann Signed-off-by: Hans de Goede --- MAINTAINERS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5c728d1..9c220f0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11801,8 +11801,7 @@ S: Maintained F: drivers/net/wireless/ath/ar5523/ USB ATTACHED SCSI -M: Hans de Goede -M: Gerd Hoffmann +M: Oliver Neukum L: linux-usb@vger.kernel.org L: linux-s...@vger.kernel.org S: Maintained -- 2.7.4 -- 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: MAINTAINERS: Oliver Neukum is the new uas maintainer
On Thu, 2016-07-14 at 14:26 +0200, Hans de Goede wrote: > Oliver Neukum is taking over uas maintainership from me and > Gerd Hoffmann. > > Cc: Oliver Neukum > Cc: Gerd Hoffmann > Signed-off-by: Hans de Goede 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
[PATCH v2 1/5] usb: serial: use variable for status
This patch turns status in a variable read once from the URB. The long term plan is to deliver status to the callback. In addition it makes the code a bit more elegant. Signed-off-by: Oliver Neukum --- drivers/usb/serial/generic.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c index ae8c036..85cf469 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -351,6 +351,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb) unsigned char *data = urb->transfer_buffer; unsigned long flags; int i; + int status = urb->status; for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) { if (urb == port->read_urbs[i]) @@ -360,22 +361,22 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb) dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i, urb->actual_length); - switch (urb->status) { + switch (status) { case 0: break; case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: dev_dbg(&port->dev, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; case -EPIPE: dev_err(&port->dev, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; default: dev_dbg(&port->dev, "%s - nonzero urb status: %d\n", - __func__, urb->status); + __func__, status); goto resubmit; } @@ -400,6 +401,7 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) unsigned long flags; struct usb_serial_port *port = urb->context; int i; + int status = urb->status; for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) { if (port->write_urbs[i] == urb) @@ -410,22 +412,22 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) set_bit(i, &port->write_urbs_free); spin_unlock_irqrestore(&port->lock, flags); - switch (urb->status) { + switch (status) { case 0: break; case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: dev_dbg(&port->dev, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; case -EPIPE: dev_err_console(port, "%s - urb stopped: %d\n", - __func__, urb->status); + __func__, status); return; default: dev_err_console(port, "%s - nonzero urb status: %d\n", - __func__, urb->status); + __func__, status); goto resubmit; } -- 2.1.4 -- 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 4/5] usb: serial: visor: clean up dev_dbg
dev_dbg() already can state the function name. No need to state it again explicitly. Signed-off-by: Oliver Neukum --- drivers/usb/serial/visor.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c index 337a0be..b970ee9 100644 --- a/drivers/usb/serial/visor.c +++ b/drivers/usb/serial/visor.c @@ -244,8 +244,7 @@ static int visor_open(struct tty_struct *tty, struct usb_serial_port *port) result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); if (result) dev_err(&port->dev, - "%s - failed submitting interrupt urb, error %d\n", - __func__, result); + "failed submitting interrupt urb, error %d\n", result); } exit: return result; @@ -284,12 +283,12 @@ static void visor_read_int_callback(struct urb *urb) case -ENOENT: case -ESHUTDOWN: /* this urb is terminated, clean up */ - dev_dbg(&port->dev, "%s - urb shutting down with status: %d\n", - __func__, status); + dev_dbg(&port->dev, "urb shutting down with status: %d\n", + status); return; default: - dev_dbg(&port->dev, "%s - nonzero urb status received: %d\n", - __func__, status); + dev_dbg(&port->dev, "nonzero urb status received: %d\n", + status); goto exit; } -- 2.1.4 -- 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 5/5] usb: serial: whiteheat: clean up dev_dbg
dev_dbg() already can state the function name. No need to state it again explicitly. Signed-off-by: Oliver Neukum --- drivers/usb/serial/whiteheat.c | 37 ++--- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/drivers/usb/serial/whiteheat.c b/drivers/usb/serial/whiteheat.c index d3ea90b..b795f34 100644 --- a/drivers/usb/serial/whiteheat.c +++ b/drivers/usb/serial/whiteheat.c @@ -542,15 +542,15 @@ static void command_port_read_callback(struct urb *urb) command_info = usb_get_serial_port_data(command_port); if (!command_info) { - dev_dbg(&urb->dev->dev, "%s - command_info is NULL, exiting.\n", __func__); + dev_dbg(&urb->dev->dev, "command_info is NULL, exiting.\n"); return; } if (!urb->actual_length) { - dev_dbg(&urb->dev->dev, "%s - empty response, exiting.\n", __func__); + dev_dbg(&urb->dev->dev, "empty response, exiting.\n"); return; } if (status) { - dev_dbg(&urb->dev->dev, "%s - nonzero urb status: %d\n", __func__, status); + dev_dbg(&urb->dev->dev, "nonzero urb status: %d\n", status); if (status != -ENOENT) command_info->command_finished = WHITEHEAT_CMD_FAILURE; wake_up(&command_info->wait_command); @@ -568,7 +568,7 @@ static void command_port_read_callback(struct urb *urb) } else if (data[0] == WHITEHEAT_EVENT) { /* These are unsolicited reports from the firmware, hence no waiting command to wakeup */ - dev_dbg(&urb->dev->dev, "%s - event received\n", __func__); + dev_dbg(&urb->dev->dev, "event received\n"); } else if ((data[0] == WHITEHEAT_GET_DTR_RTS) && (urb->actual_length - 1 <= sizeof(command_info->result_buffer))) { memcpy(command_info->result_buffer, &data[1], @@ -576,13 +576,12 @@ static void command_port_read_callback(struct urb *urb) command_info->command_finished = WHITEHEAT_CMD_COMPLETE; wake_up(&command_info->wait_command); } else - dev_dbg(&urb->dev->dev, "%s - bad reply from firmware\n", __func__); + dev_dbg(&urb->dev->dev, "bad reply from firmware\n"); /* Continue trying to always read */ result = usb_submit_urb(command_port->read_urb, GFP_ATOMIC); if (result) - dev_dbg(&urb->dev->dev, "%s - failed resubmitting read urb, error %d\n", - __func__, result); + dev_dbg(&urb->dev->dev, "failed resubmitting read urb, error %d\n", result); } @@ -600,7 +599,7 @@ static int firm_send_command(struct usb_serial_port *port, __u8 command, int retval = 0; int t; - dev_dbg(dev, "%s - command %d\n", __func__, command); + dev_dbg(dev, "command %d\n", command); command_port = port->serial->port[COMMAND_PORT]; command_info = usb_get_serial_port_data(command_port); @@ -613,7 +612,7 @@ static int firm_send_command(struct usb_serial_port *port, __u8 command, command_port->write_urb->transfer_buffer_length = datasize + 1; retval = usb_submit_urb(command_port->write_urb, GFP_NOIO); if (retval) { - dev_dbg(dev, "%s - submit urb failed\n", __func__); + dev_dbg(dev, "submit urb failed\n"); goto exit; } @@ -624,19 +623,19 @@ static int firm_send_command(struct usb_serial_port *port, __u8 command, usb_kill_urb(command_port->write_urb); if (command_info->command_finished == false) { - dev_dbg(dev, "%s - command timed out.\n", __func__); + dev_dbg(dev, "command timed out.\n"); retval = -ETIMEDOUT; goto exit; } if (command_info->command_finished == WHITEHEAT_CMD_FAILURE) { - dev_dbg(dev, "%s - command failed.\n", __func__); + dev_dbg(dev, "command failed.\n"); retval = -EIO; goto exit; } if (command_info->command_finished == WHITEHEAT_CMD_COMPLETE) { - dev_dbg(dev, "%s - command completed.\n", __func__); + dev_dbg(dev, "command completed.\n"); switch (command) { case WHITEHEAT_GET_DTR_RTS: info = usb_get_serial_port_data(port); @@ -688,7 +687,7 @@ static void firm_setup_port(struct tty_struct *tty) default: case CS8: port_settings.bits = 8; break; } - dev_dbg(dev, "%s - data bits = %d\n", __func__, port_settings.bits); + dev_dbg(dev, "data bits = %d\n", port_settings.bits); /* determine the parity */ if (cflag & PARENB) @@ -704,14 +703,14 @@ static void firm_setup_port(struct tty_struct *tty) port_settings.par
[PATCH v2 3/5] usb: serial: usb_wwan.c: clean up dev_dbg
dev_dbg() already can state the function name. No need to state it again explicitly. Signed-off-by: Oliver Neukum --- drivers/usb/serial/usb_wwan.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 3dfdfc8..0c128a5 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -193,7 +193,7 @@ int usb_wwan_ioctl(struct tty_struct *tty, { struct usb_serial_port *port = tty->driver_data; - dev_dbg(&port->dev, "%s cmd 0x%04x\n", __func__, cmd); + dev_dbg(&port->dev, "cmd 0x%04x\n", cmd); switch (cmd) { case TIOCGSERIAL: @@ -206,7 +206,7 @@ int usb_wwan_ioctl(struct tty_struct *tty, break; } - dev_dbg(&port->dev, "%s arg not supported\n", __func__); + dev_dbg(&port->dev, "arg not supported\n"); return -ENOIOCTLCMD; } @@ -226,7 +226,7 @@ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, portdata = usb_get_serial_port_data(port); intfdata = usb_get_serial_data(port->serial); - dev_dbg(&port->dev, "%s: write (%d chars)\n", __func__, count); + dev_dbg(&port->dev, "write (%d chars)\n", count); i = 0; left = count; @@ -243,7 +243,7 @@ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, usb_unlink_urb(this_urb); continue; } - dev_dbg(&port->dev, "%s: endpoint %d buf %d\n", __func__, + dev_dbg(&port->dev, "endpoint %d buf %d\n", usb_pipeendpoint(this_urb->pipe), i); err = usb_autopm_get_interface_async(port->serial->interface); @@ -284,7 +284,7 @@ int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port, } count -= left; - dev_dbg(&port->dev, "%s: wrote (did %d)\n", __func__, count); + dev_dbg(&port->dev, "wrote (did %d)\n", count); return count; } EXPORT_SYMBOL(usb_wwan_write); @@ -303,15 +303,15 @@ static void usb_wwan_indat_callback(struct urb *urb) dev = &port->dev; if (status) { - dev_dbg(dev, "%s: nonzero status: %d on endpoint %02x.\n", - __func__, status, endpoint); + dev_dbg(dev, "nonzero status: %d on endpoint %02x.\n", + status, endpoint); } else { if (urb->actual_length) { tty_insert_flip_string(&port->port, data, urb->actual_length); tty_flip_buffer_push(&port->port); } else - dev_dbg(dev, "%s: empty read urb received\n", __func__); + dev_dbg(dev, "empty read urb received\n"); } /* Resubmit urb so we continue receiving */ err = usb_submit_urb(urb, GFP_ATOMIC); @@ -369,7 +369,7 @@ int usb_wwan_write_room(struct tty_struct *tty) data_len += OUT_BUFLEN; } - dev_dbg(&port->dev, "%s: %d\n", __func__, data_len); + dev_dbg(&port->dev, "room left: %d\n", data_len); return data_len; } EXPORT_SYMBOL(usb_wwan_write_room); @@ -391,7 +391,7 @@ int usb_wwan_chars_in_buffer(struct tty_struct *tty) if (this_urb && test_bit(i, &portdata->out_busy)) data_len += this_urb->transfer_buffer_length; } - dev_dbg(&port->dev, "%s: %d\n", __func__, data_len); + dev_dbg(&port->dev, "room used: %d\n", data_len); return data_len; } EXPORT_SYMBOL(usb_wwan_chars_in_buffer); -- 2.1.4 -- 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 / 5]cleanup of serial drivers
This assigns urb->status to a variable and as Bjorn notes, these drivers were often mechanically converted to dev_dbg and abuse it. So here's a cleanup. -- 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 2/5] usb: serial: removing redundant __func__
dev_dbg already states the function it is called from. Printing it again is wasted space. Signed-off-by: Oliver Neukum --- drivers/usb/serial/generic.c | 33 + 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c index 85cf469..3f7e3b1 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -221,7 +221,7 @@ int usb_serial_generic_write_room(struct tty_struct *tty) room = kfifo_avail(&port->write_fifo); spin_unlock_irqrestore(&port->lock, flags); - dev_dbg(&port->dev, "%s - returns %d\n", __func__, room); + dev_dbg(&port->dev, "returns %d\n", room); return room; } @@ -238,7 +238,7 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct *tty) chars = kfifo_len(&port->write_fifo) + port->tx_bytes; spin_unlock_irqrestore(&port->lock, flags); - dev_dbg(&port->dev, "%s - returns %d\n", __func__, chars); + dev_dbg(&port->dev, "returns %d\n", chars); return chars; } EXPORT_SYMBOL_GPL(usb_serial_generic_chars_in_buffer); @@ -261,8 +261,8 @@ void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout) if (timeout) period = min_t(unsigned long, period, timeout); - dev_dbg(&port->dev, "%s - timeout = %u ms, period = %u ms\n", - __func__, jiffies_to_msecs(timeout), + dev_dbg(&port->dev, "timeout = %u ms, period = %u ms\n", + jiffies_to_msecs(timeout), jiffies_to_msecs(period)); expire = jiffies + timeout; while (!port->serial->type->tx_empty(port)) { @@ -283,7 +283,7 @@ static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port, if (!test_and_clear_bit(index, &port->read_urbs_free)) return 0; - dev_dbg(&port->dev, "%s - urb %d\n", __func__, index); + dev_dbg(&port->dev, "urb %d\n", index); res = usb_submit_urb(port->read_urbs[index], mem_flags); if (res) { @@ -359,24 +359,20 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb) } set_bit(i, &port->read_urbs_free); - dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i, - urb->actual_length); + dev_dbg(&port->dev, "urb %d, len %d\n", i, urb->actual_length); switch (status) { case 0: break; case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: - dev_dbg(&port->dev, "%s - urb stopped: %d\n", - __func__, status); + dev_dbg(&port->dev, "urb stopped: %d\n", status); return; case -EPIPE: - dev_err(&port->dev, "%s - urb stopped: %d\n", - __func__, status); + dev_err(&port->dev, "urb stopped: %d\n", status); return; default: - dev_dbg(&port->dev, "%s - nonzero urb status: %d\n", - __func__, status); + dev_dbg(&port->dev, "nonzero urb status: %d\n", status); goto resubmit; } @@ -418,16 +414,13 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: - dev_dbg(&port->dev, "%s - urb stopped: %d\n", - __func__, status); + dev_dbg(&port->dev, "urb stopped: %d\n", status); return; case -EPIPE: - dev_err_console(port, "%s - urb stopped: %d\n", - __func__, status); + dev_err_console(port, "urb stopped: %d\n", status); return; default: - dev_err_console(port, "%s - nonzero urb status: %d\n", - __func__, status); + dev_err_console(port, "nonzero urb status: %d\n", status); goto resubmit; } @@ -582,7 +575,7 @@ void usb_serial_handle_dcd_change(struct usb_serial_port *usb_port, { struct tty_port *port = &usb_port->port; - dev_dbg(&usb_port->dev, "%s - status %d\n", __func__, status); + dev_dbg(&usb_port->dev, "status %d\n", status); if (tty) { struct tty_ldisc *ld = tty_ldisc_ref(tty); -- 2.1.4 -- 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 4/5] cdc-wdm: use the common CDC parser
Now that the common parser resides in USB core, it can be used for CDC-WDM. Signed-off-by: Oliver Neukum --- drivers/usb/class/cdc-wdm.c | 30 +- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 61ea879..337948c 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -875,38 +875,18 @@ static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id) int rv = -EINVAL; struct usb_host_interface *iface; struct usb_endpoint_descriptor *ep; - struct usb_cdc_dmm_desc *dmhd; + struct usb_cdc_parsed_header hdr; u8 *buffer = intf->altsetting->extra; int buflen = intf->altsetting->extralen; u16 maxcom = WDM_DEFAULT_BUFSIZE; if (!buffer) goto err; - while (buflen > 2) { - if (buffer[1] != USB_DT_CS_INTERFACE) { - dev_err(&intf->dev, "skipping garbage\n"); - goto next_desc; - } - switch (buffer[2]) { - case USB_CDC_HEADER_TYPE: - break; - case USB_CDC_DMM_TYPE: - dmhd = (struct usb_cdc_dmm_desc *)buffer; - maxcom = le16_to_cpu(dmhd->wMaxCommand); - dev_dbg(&intf->dev, - "Finding maximum buffer length: %d", maxcom); - break; - default: - dev_err(&intf->dev, - "Ignoring extra header, type %d, length %d\n", - buffer[2], buffer[0]); - break; - } -next_desc: - buflen -= buffer[0]; - buffer += buffer[0]; - } + cdc_parse_cdc_header(&hdr, intf, buffer, buflen); + + if (hdr.usb_cdc_dmm_desc) + maxcom = le16_to_cpu(hdr.usb_cdc_dmm_desc->wMaxCommand); iface = intf->cur_altsetting; if (iface->desc.bNumEndpoints != 1) -- 2.1.4 -- 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/5] cdc-acm: use the common parser
This introduces the common parser for extra CDC headers now that it no longer depends on usbnet. Signed-off-by: Oliver Neukum --- drivers/usb/class/cdc-acm.c | 69 +++-- 1 file changed, 10 insertions(+), 59 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 94a14f5..70bd642 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1147,6 +1147,7 @@ static int acm_probe(struct usb_interface *intf, { struct usb_cdc_union_desc *union_header = NULL; struct usb_cdc_country_functional_desc *cfd = NULL; + struct usb_cdc_call_mgmt_descriptor *cmd = NULL; unsigned char *buffer = intf->altsetting->extra; int buflen = intf->altsetting->extralen; struct usb_interface *control_interface; @@ -1155,18 +1156,16 @@ static int acm_probe(struct usb_interface *intf, struct usb_endpoint_descriptor *epread = NULL; struct usb_endpoint_descriptor *epwrite = NULL; struct usb_device *usb_dev = interface_to_usbdev(intf); + struct usb_cdc_parsed_header hdr; struct acm *acm; int minor; int ctrlsize, readsize; u8 *buf; - u8 ac_management_function = 0; - u8 call_management_function = 0; int call_interface_num = -1; int data_interface_num = -1; unsigned long quirks; int num_rx_buf; int i; - unsigned int elength = 0; int combined_interfaces = 0; struct device *tty_dev; int rv = -ENOMEM; @@ -1210,61 +1209,11 @@ static int acm_probe(struct usb_interface *intf, } } - while (buflen > 0) { - elength = buffer[0]; - if (!elength) { - dev_err(&intf->dev, "skipping garbage byte\n"); - elength = 1; - goto next_desc; - } - if (buffer[1] != USB_DT_CS_INTERFACE) { - dev_err(&intf->dev, "skipping garbage\n"); - goto next_desc; - } - - switch (buffer[2]) { - case USB_CDC_UNION_TYPE: /* we've found it */ - if (elength < sizeof(struct usb_cdc_union_desc)) - goto next_desc; - if (union_header) { - dev_err(&intf->dev, "More than one " - "union descriptor, skipping ...\n"); - goto next_desc; - } - union_header = (struct usb_cdc_union_desc *)buffer; - break; - case USB_CDC_COUNTRY_TYPE: /* export through sysfs*/ - if (elength < sizeof(struct usb_cdc_country_functional_desc)) - goto next_desc; - cfd = (struct usb_cdc_country_functional_desc *)buffer; - break; - case USB_CDC_HEADER_TYPE: /* maybe check version */ - break; /* for now we ignore it */ - case USB_CDC_ACM_TYPE: - if (elength < 4) - goto next_desc; - ac_management_function = buffer[3]; - break; - case USB_CDC_CALL_MANAGEMENT_TYPE: - if (elength < 5) - goto next_desc; - call_management_function = buffer[3]; - call_interface_num = buffer[4]; - break; - default: - /* -* there are LOTS more CDC descriptors that -* could legitimately be found here. -*/ - dev_dbg(&intf->dev, "Ignoring descriptor: " - "type %02x, length %ud\n", - buffer[2], elength); - break; - } -next_desc: - buflen -= elength; - buffer += elength; - } + cdc_parse_cdc_header(&hdr, intf, buffer, buflen); + union_header = hdr.usb_cdc_union_desc; + cmd = hdr.usb_cdc_call_mgmt_descriptor; + if (cmd) + call_interface_num = cmd->bDataInterface; if (!union_header) { if (call_interface_num > 0) { @@ -1394,7 +1343,8 @@ made_compressed_probe: acm->data = data_interface; acm->minor = minor; acm->dev = usb_dev; - acm->ctrl_caps = ac_management_function; + if (hdr.usb_cdc_acm_descriptor) + acm->ctrl_caps = hdr.usb_cdc_acm_descriptor->bmCapabilities; if (quirks & NO_CAP_LINE) acm->ctrl_caps &= ~USB_CDC_CAP_LINE; acm->ctrlsize = ctrlsize; @@ -1488,6 +1438,7 @@ made_compressed_probe: if (i < 0)
[PATCH 1/5] usbnet: move the CDC parser into USB core
The dependencies were impossible to handle preventing drivers for CDC devices not which are not network drivers from using the common parser. Signed-off-by: Oliver Neukum --- drivers/net/usb/usbnet.c | 138 drivers/usb/core/message.c | 153 + 2 files changed, 153 insertions(+), 138 deletions(-) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 61ba464..b56d78a 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -42,7 +42,6 @@ #include #include #include -#include #include #include #include @@ -1968,143 +1967,6 @@ out: return err; } -int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr, - struct usb_interface *intf, - u8 *buffer, - int buflen) -{ - /* duplicates are ignored */ - struct usb_cdc_union_desc *union_header = NULL; - - /* duplicates are not tolerated */ - struct usb_cdc_header_desc *header = NULL; - struct usb_cdc_ether_desc *ether = NULL; - struct usb_cdc_mdlm_detail_desc *detail = NULL; - struct usb_cdc_mdlm_desc *desc = NULL; - - unsigned int elength; - int cnt = 0; - - memset(hdr, 0x00, sizeof(struct usb_cdc_parsed_header)); - hdr->phonet_magic_present = false; - while (buflen > 0) { - elength = buffer[0]; - if (!elength) { - dev_err(&intf->dev, "skipping garbage byte\n"); - elength = 1; - goto next_desc; - } - if (buffer[1] != USB_DT_CS_INTERFACE) { - dev_err(&intf->dev, "skipping garbage\n"); - goto next_desc; - } - - switch (buffer[2]) { - case USB_CDC_UNION_TYPE: /* we've found it */ - if (elength < sizeof(struct usb_cdc_union_desc)) - goto next_desc; - if (union_header) { - dev_err(&intf->dev, "More than one union descriptor, skipping ...\n"); - goto next_desc; - } - union_header = (struct usb_cdc_union_desc *)buffer; - break; - case USB_CDC_COUNTRY_TYPE: - if (elength < sizeof(struct usb_cdc_country_functional_desc)) - goto next_desc; - hdr->usb_cdc_country_functional_desc = - (struct usb_cdc_country_functional_desc *)buffer; - break; - case USB_CDC_HEADER_TYPE: - if (elength != sizeof(struct usb_cdc_header_desc)) - goto next_desc; - if (header) - return -EINVAL; - header = (struct usb_cdc_header_desc *)buffer; - break; - case USB_CDC_ACM_TYPE: - if (elength < sizeof(struct usb_cdc_acm_descriptor)) - goto next_desc; - hdr->usb_cdc_acm_descriptor = - (struct usb_cdc_acm_descriptor *)buffer; - break; - case USB_CDC_ETHERNET_TYPE: - if (elength != sizeof(struct usb_cdc_ether_desc)) - goto next_desc; - if (ether) - return -EINVAL; - ether = (struct usb_cdc_ether_desc *)buffer; - break; - case USB_CDC_CALL_MANAGEMENT_TYPE: - if (elength < sizeof(struct usb_cdc_call_mgmt_descriptor)) - goto next_desc; - hdr->usb_cdc_call_mgmt_descriptor = - (struct usb_cdc_call_mgmt_descriptor *)buffer; - break; - case USB_CDC_DMM_TYPE: - if (elength < sizeof(struct usb_cdc_dmm_desc)) - goto next_desc; - hdr->usb_cdc_dmm_desc = - (struct usb_cdc_dmm_desc *)buffer; - break; - case USB_CDC_MDLM_TYPE: - if (elength < sizeof(struct usb_cdc_mdlm_desc *)) - goto next_desc; - if (desc) - return -EINVAL; - desc = (struct usb_cdc_mdlm_desc *)buffer; - break; - case USB_CDC_MDLM_DETAIL_TYPE: - if (elength < sizeof(struct usb_cdc_mdlm_detail_desc *)) - goto next_desc; - if (detail) -
[PATCH 5/5] cdc-acm: beautify probe()
This removes some overly long lines by renaming variables and giving them local scope. Signed-off-by: Oliver Neukum --- drivers/usb/class/cdc-acm.c | 44 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 1857fad..369bde0 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1146,8 +1146,7 @@ static int acm_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_cdc_union_desc *union_header = NULL; - struct usb_cdc_country_functional_desc *cfd = NULL; - struct usb_cdc_call_mgmt_descriptor *cmd = NULL; + struct usb_cdc_call_mgmt_descriptor *cmgmd = NULL; unsigned char *buffer = intf->altsetting->extra; int buflen = intf->altsetting->extralen; struct usb_interface *control_interface; @@ -1156,13 +1155,13 @@ static int acm_probe(struct usb_interface *intf, struct usb_endpoint_descriptor *epread = NULL; struct usb_endpoint_descriptor *epwrite = NULL; struct usb_device *usb_dev = interface_to_usbdev(intf); - struct usb_cdc_parsed_header hdr; + struct usb_cdc_parsed_header h; struct acm *acm; int minor; int ctrlsize, readsize; u8 *buf; - int call_interface_num = -1; - int data_interface_num = -1; + int call_intf_num = -1; + int data_intf_num = -1; unsigned long quirks; int num_rx_buf; int i; @@ -1209,20 +1208,22 @@ static int acm_probe(struct usb_interface *intf, } } - cdc_parse_cdc_header(&hdr, intf, buffer, buflen); - union_header = hdr.usb_cdc_union_desc; - cmd = hdr.usb_cdc_call_mgmt_descriptor; - if (cmd) - call_interface_num = cmd->bDataInterface; + cdc_parse_cdc_header(&h, intf, buffer, buflen); + union_header = h.usb_cdc_union_desc; + cmgmd = h.usb_cdc_call_mgmt_descriptor; + if (cmgmd) + call_intf_num = cmgmd->bDataInterface; if (!union_header) { - if (call_interface_num > 0) { + if (call_intf_num > 0) { dev_dbg(&intf->dev, "No union descriptor, using call management descriptor\n"); /* quirks for Droids MuIn LCD */ - if (quirks & NO_DATA_INTERFACE) + if (quirks & NO_DATA_INTERFACE) { data_interface = usb_ifnum_to_if(usb_dev, 0); - else - data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = call_interface_num)); + } else { + data_intf_num = call_intf_num; + data_interface = usb_ifnum_to_if(usb_dev, data_intf_num); + } control_interface = intf; } else { if (intf->cur_altsetting->desc.bNumEndpoints != 3) { @@ -1236,8 +1237,9 @@ static int acm_probe(struct usb_interface *intf, } } } else { + data_intf_num = union_header->bSlaveInterface0; control_interface = usb_ifnum_to_if(usb_dev, union_header->bMasterInterface0); - data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = union_header->bSlaveInterface0)); + data_interface = usb_ifnum_to_if(usb_dev, data_intf_num); } if (!control_interface || !data_interface) { @@ -1245,7 +1247,7 @@ static int acm_probe(struct usb_interface *intf, return -ENODEV; } - if (data_interface_num != call_interface_num) + if (data_intf_num != call_intf_num) dev_dbg(&intf->dev, "Separate call control interface. That is not fully supported.\n"); if (control_interface == data_interface) { @@ -1340,8 +1342,8 @@ made_compressed_probe: acm->data = data_interface; acm->minor = minor; acm->dev = usb_dev; - if (hdr.usb_cdc_acm_descriptor) - acm->ctrl_caps = hdr.usb_cdc_acm_descriptor->bmCapabilities; + if (h.usb_cdc_acm_descriptor) + acm->ctrl_caps = h.usb_cdc_acm_descriptor->bmCapabilities; if (quirks & NO_CAP_LINE) acm->ctrl_caps &= ~USB_CDC_CAP_LINE; acm->ctrlsize = ctrlsize; @@ -1435,8 +1437,10 @@ made_compressed_probe: if (i < 0) goto alloc_fail7; - cfd = hdr.usb_cdc_country_functional_desc; - if (cfd) { /* export the country data */ + if (h.usb_cdc_country_functional_desc) { /* export the country data */ + struct usb_cdc_country_functional_desc * cfd = + h.usb_cdc_country_functional_desc; + acm->country_codes = kmalloc(cfd->bLength - 4, GFP_KE
[PATCH 0 / 5] move the common CDC parser
Experience has shown that making all CDC drivers depend on usbnet is not practical, because some of them are not network drivers. So this patch moves the common parser from usbnet into the messages helpers of usbcore. The rest of the series applies it to the non-network CDC drivers. I hope it can go through Greg's tree although it touches usbnet. -- 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 3/5] cdc-acm: cleanup error handling
A small update to unify error handling during probe(). Signed-off-by: Oliver Neukum --- drivers/usb/class/cdc-acm.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 70bd642..1857fad 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1328,11 +1328,8 @@ made_compressed_probe: goto alloc_fail; minor = acm_alloc_minor(acm); - if (minor < 0) { - dev_err(&intf->dev, "no more free acm devices\n"); - kfree(acm); - return -ENODEV; - } + if (minor < 0) + goto alloc_fail1; ctrlsize = usb_endpoint_maxp(epctrl); readsize = usb_endpoint_maxp(epread) * @@ -1523,6 +1520,7 @@ alloc_fail4: usb_free_coherent(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma); alloc_fail2: acm_release_minor(acm); +alloc_fail1: kfree(acm); alloc_fail: return rv; -- 2.1.4 -- 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: ehci-platform: use helper variables in probe function
On Wed, 13 Jul 2016, Rafał Miłecki wrote: > Probing function was using &dev->dev and dev->dev.of_node over 20 times > so I believe it made sense to use helper variables for both of them. > To avoid some uncommon variable name for struct device I first replaced > existing dev variable with pdev. > > Signed-off-by: Rafał Miłecki Okay except for... > - priv->num_phys = of_count_phandle_with_args(dev->dev.of_node, > - "phys", "#phy-cells"); > + priv->num_phys = of_count_phandle_with_args(np, "phys", > + "#phy-cells"); Please indent continuation lines two tab stops beyond the original line, to match the style in the rest of the source file. With that change, Acked-by: Alan Stern 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] usb3: Fixed usb3 device is not detected in s0 when hotplug usb3 disk under S3
On Thu, 14 Jul 2016, Huang, Huki wrote: > On Mon, 11 Jul 2016: Alan Stern wrote: > > > On Mon, 11 Jul 2016, Huang, Huki wrote: > > > > When end user inserts a usb3 device and put dut to s3. > > > What does "dut" mean? > > DUT : Device under test Don't call it that in the patch description. "Device" here means "USB device", whereas you mean "USB host system". > > > Then hotplug the usb3 disk under s3. > > > Do you mean that the device is unplugged and the USB disk then is plugged > > into the same port? Or do you mean that the device remains plugged in and > > the disk is plugged into a different port? > > Yes, the USB disk is unplugged and plugged on the same port when the system > under s3 Is the "usb3 device" you mentioned above the same as the "usb3 disk"? If they are the same, why do you use two different terms for them? > > > The device will be lost upon resuming from s3. > > > If the device is unplugged then it _should_ be lost. > > > > There is a corner case that the hub->change_bits for the usb3 port > > > is not set when usb port change event happens. > > > Under what conditions does this corner case occur? > > The USB disk is unplugged and plugged on the same port under s3. > > > > This will cause hub driver ignore the device enumeration in the end > > > of port_event in hub.c > > > > > > Signed-off-by: huki.hu...@intel.com > > > --- > > > drivers/usb/core/hub.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index > > > bee1351..859adcb 100644 > > > --- a/drivers/usb/core/hub.c > > > +++ b/drivers/usb/core/hub.c > > > @@ -4747,6 +4747,7 @@ static void hub_port_connect(struct usb_hub > > >*hub, int port1, u16 portstatus, > > > if (hcd->usb_phy && !hdev->parent) > > > > > >usb_phy_notify_disconnect(hcd->usb_phy, udev->speed); > > > usb_disconnect(&port_dev->child); > > > + set_bit(port1, hub->change_bits); > > > Why is this needed? The only reason for setting hub->change_bits is > > to tell hub_event() that it needs to call port_event() and to tell > > port_event() that it needs to call hub_port_connect_change(). Once > > hub_port_connect_change() is running, there is no reason to set > > hub->change_bits. > > When the USB disk is unplugged and plugged on the same port under s3. > You can see a series of disconnect/connect events happen. > There is a chance driver receives a port_event but not enumerate device due > to change_bits is not set. > > When usb_disconnect is called, it doesn't set_bit(port1, > hub->change_bits); Then port_event is called again very soon after > the above usb_disconnect is called. > > In the end of the port_event function. > The connect_change is not true so that device is not enumerated. >if (connect_change) >hub_port_connect_change(hub, port1, portstatus, portchange); I think what you need to do is change the code in hub_activate(). That's where we check for changes to a port's status that occurred while the system was suspended. In particular, pay attention to the code following the "init2:" label. However, I think the following part of that function: } else if (portstatus & USB_PORT_STAT_ENABLE) { bool port_resumed = (portstatus & USB_PORT_STAT_LINK_STATE) == USB_SS_PORT_LS_U0; /* The power session apparently survived the resume. * If there was an overcurrent or suspend change * (i.e., remote wakeup request), have hub_wq * take care of it. Look at the port link state * for USB 3.0 hubs, since they don't have a suspend * change bit, and they don't set the port link change * bit on device-initiated resume. */ if (portchange || (hub_is_superspeed(hub->hdev) && port_resumed)) set_bit(port1, hub->change_bits); } else if (udev->persist_enabled) { #ifdef CONFIG_PM udev->reset_resume = 1; #endif /* Don't set the change_bits when the device * was powered off. */ if (test_bit(port1, hub->power_bits)) set_bit(port1, hub->change_bits); should already be doing what you want. If it isn't, you need to figure out why. 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: MAINTAINERS: Oliver Neukum is the new uas maintainer
On Do, 2016-07-14 at 14:26 +0200, Hans de Goede wrote: > Oliver Neukum is taking over uas maintainership from me and > Gerd Hoffmann. > > Cc: Oliver Neukum > Cc: Gerd Hoffmann > Signed-off-by: Hans de Goede Acked-by: Gerd Hoffmann -- 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: ehci-platform: use helper variables in probe function
On 14 July 2016 at 16:11, Alan Stern wrote: > On Wed, 13 Jul 2016, Rafał Miłecki wrote: > >> Probing function was using &dev->dev and dev->dev.of_node over 20 times >> so I believe it made sense to use helper variables for both of them. >> To avoid some uncommon variable name for struct device I first replaced >> existing dev variable with pdev. >> >> Signed-off-by: Rafał Miłecki > > Okay except for... > >> - priv->num_phys = of_count_phandle_with_args(dev->dev.of_node, >> - "phys", "#phy-cells"); >> + priv->num_phys = of_count_phandle_with_args(np, "phys", >> + "#phy-cells"); > > Please indent continuation lines two tab stops beyond the original > line, to match the style in the rest of the source file. I'm afraid this file doesn't have any consistent coding style for line breaks. 1) dma_coerce_mask_and_coherent One extra tab after line break. 2) devm_kcalloc Two extra tabs and 4 spaces. No real alignment noticed. 3) devm_of_phy_get_by_index Two extra tabs 4) devm_reset_control_get_shared_by_index Three extra tabs With these pointed, do you still think I should use two extra tabs? If so, I'll send V2 as you suggested. Just let me know. -- Rafał -- 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/1] usb: add HCD providers
On 14 July 2016 at 11:48, Peter Chen wrote: > On Wed, Jul 13, 2016 at 04:40:53PM +0200, Rafał Miłecki wrote: >> On 13 July 2016 at 15:50, Felipe Balbi wrote: >> > Rafał Miłecki writes: >> >> On 13 July 2016 at 15:20, Felipe Balbi >> >> wrote: >> >>> Rafał Miłecki writes: >> Hi again, >> >> This is my second try of getting HCD providers into usb subsystem. >> >> During discussion of V1 I realized there are about 26 drivers adding a >> single HCD and all of them would need to be modified. So instead I >> decided to put relevant code in usb_add_hcd. It checks if the HCD we >> register is a primary one and if so, it registers a proper provider. >> >> Please note that of_hcd_xlate_simple was also extended to allow getting >> shared HCD (which is used e.g. in case of XHCI). >> >> So now you can have something like: >> >> ohci: ohci@21000 { >> #usb-cells = <0>; >> compatible = "generic-ohci"; >> reg = <0x1000 0x1000>; >> interrupts = ; >> }; >> >> ehci: ehci@22000 { >> #usb-cells = <0>; >> compatible = "generic-ehci"; >> reg = <0x2000 0x1000>; >> interrupts = ; >> }; >> >> xhci: xhci@23000 { >> #usb-cells = <1>; >> compatible = "generic-xhci"; >> reg = <0x3000 0x1000>; >> interrupts = ; >> }; >> >> The last (second) patch is not supposed to be applied, it's used only as >> a proof and example of how providers can be used. >> >>> >> >>> nowhere here (or in previous patch) you clarify why exactly you need >> >>> this. What is your LED trigger supposed to do? Why can't it handle ports >> >>> changing number in different boots? Why do we need this at all? Why is >> >>> your code DT-specific? >> >>> >> >>> There are still too many 'unknowns' here. >> >> >> >> Are you sure you saw my reply to Peter's question? >> >> >> >> http://www.spinics.net/lists/linux-usb/msg143708.html >> >> http://marc.info/?l=linux-usb&m=146838735627093&w=2 >> >> >> >> I think it should answer (some of?) your questions. Can you read it >> >> and see if it gets a bit clearer? >> > >> > well, all that says is that you're writing a LED trigger to toggle LED >> > when a USB device gets added to a specified port. I don't think you need >> > the actual port number for that. You should have a phandle to the actual >> > port, whatever its number is, or a phandle to the (root-)Hub and a port >> > number from that hub. >> > >> > The problem, really, is that DT descriptor of USB Hosts is very, very >> > minimal. Perhaps there's something more extensively defined from the >> > original Open Firmware USB Addendum. >> >> Thanks for your effort and looking at this closely. You're right, I'm >> interested in referencing USB ports, but I'm using controller phandle >> (and then I specify ports manually). >> >> Having each port described by DT would be helpful, it's just something >> I didn't find implemented, so I started looking for different ways. It >> seems I should have picked a different solution. >> >> So should I work on describing USB ports in DT instead? This looks >> like a complex thing to describe, so I'd like to ask for some guidance >> first. What do you think about following schema/example? >> >> ohci@1000 { >> compatible = "generic-ohci"; >> reg = <0x1000 0x1000>; >> interrupts = ; >> >> primary-hcd { >> ohci_port0: port@0 { >> reg = <0>; >> }; >> >> ohci_port1: port@1 { >> reg = <1>; >> }; >> } >> }; >> >> ehci@2000 { >> compatible = "generic-ehci"; >> reg = <0x2000 0x1000>; >> interrupts = ; >> >> primary-hcd { >> ehci_port0: port@0 { >> reg = <0>; >> }; >> >> ehci_port1: port@1 { >> reg = <1>; >> }; >> } >> }; >> >> xhci@3000 { >> compatible = "generic-xhci"; >> reg = <0x3000 0x1000>; >> interrupts = ; >> >> primary-hcd { >> }; >> >> shared-hcd { >> xhci_port0: port@0 { >> reg = <0>; >> }; >> } >> }; >> >> With such a DT struct, how could I query port for a Linux-assigned number? >> >> For example with OHCI, EHCI and XHCI drivers compiled, Linux assigns >> number 4 to my XHCI's shared HCD's root hub: >> xhci-hcd 18023000.xhci: xHCI Host Controller >> xhci-hcd 18023000.xhci: new USB bus registered, assigned bus number 4 >> hub 4-0:1.0: USB hub found >> hub 4-0:1.0: 1 port detected >> >> If I disable OHCI and EHCI I get: >> xhci-hcd xhci-hcd.0: xHCI Host Controller >> xhci-hcd xhci-hcd.0: new USB bus registered, assigned bus number 2 >> hub 2-0:1.0: USB
[PATCH] r8152: add MODULE_VERSION
ethtool -i provides a driver version that is hard coded. Export the same value via "modinfo". Signed-off-by: Grant Grundler --- drivers/net/usb/r8152.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 0da72d3..1c01ed5 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -4359,3 +4359,4 @@ module_usb_driver(rtl8152_driver); MODULE_AUTHOR(DRIVER_AUTHOR); MODULE_DESCRIPTION(DRIVER_DESC); MODULE_LICENSE("GPL"); +MODULE_VERSION(DRIVER_VERSION); -- 2.8.0.rc3.226.g39d4020 -- 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
USBIP problem with windows client
Hi! I'm trying to get working USBIP server on 3.16.0 kernel and winxp client (windows driver is from sf.net). I did 'usbip attach' to my device (hp scanner) on server and started usbipd. Windows machine can successfully obtain list of devices available on server, but when I'm trying to attach it to client, it fails with 'cannot find device' error. They same scenario with linux client works perfectly. I've tracked down this error to the fact that windows client during attach process is trying to request not only list of devices, but also needs at least one interface for every device to pass interfaceClass/Subclass/Procotol to host driver. At the same time, usbipd is obtaining information about usb devices via udev, but after 'usbip attach' command (which binds usb device to vhci_hcd) bNumInterfaces sys file (and corresponding udev attr request) always return empty string. This results in zero number of interfaces on devices and error in windows usbip client. At the same time, 'lsusb -v' always shows correct bNumInterfaces values regardless of attach status of device. Do you have any suggestions how to solve this? -- wbr, Max Lapan -- 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 PATCH 0/5] USB Audio Gadget refactoring
Ping? On Wed, Jun 8, 2016 at 11:03 AM, Ruslan Bilovol wrote: > Hi guys, > > Any feedback on this patch series? Has anybody had a chance to test it? > > Regards, > Ruslan > > On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol > wrote: >> I came to this patch series when wanted to do two things: >> - use UAC1 as virtual ALSA sound card on gadget side, >>just like UAC2 is used so it's possible to do rate >>resampling >> - have both playback/capture support in UAC1 >> >> Since I wanted to have same behavior for both UAC1/UAC2, >> obviously I've got an utility part (u_audio.c) for >> virtual ALSA sound card handling like we have >> for ethernet(u_ether) or serial(u_serial) functions. >> Function-specific parts (f_uac1/f_uac2) became almost >> as storage for class-specfic USB descriptors, some >> boilerplate for configfs, binding and few USB >> config request handling. >> >> Major change to f_uac1 it that it can't do >> direct play to existing ALSA sound card anymore, >> representing audio on gadget side as virtual >> ALSA sound card where audio streams are simply >> sinked to and sourced from it, so it may break >> current usecase for some people (and that's why >> it's RFC). >> >> Luckily, it's possible to use existing user-space >> applications for audio routing between Audio Gadget >> and real sound card. I personally use alsaloop tool >> from alsautils and have ability to create PCM >> loopback between two different ALSA cards using >> rate resampling, which is not possible with previous >> "direct play to ALSA card" approach in f_uac1. >> >> While here, also dropped redundant platform >> driver/device creation in f_uac2 driver as well as >> "never implemented" volume/mute functionality in f_uac1 >> that made this work even easier to do. >> >> This series is tested with both legacy g_audio.ko and >> modern configfs approaches under Ubuntu 14.04 (UAC1 and >> UAC2) and under Windows7 x64 (UAC1 only) having >> perfect results in all cases. >> >> Some changes may have lack of good description that may >> be obvious for me but not so clear for others, but I >> hope to fix it in next versions. >> >> Comments, testing are welcome. >> >> Ruslan Bilovol (5): >> usb: gadget: f_uac2: remove platform driver/device creation >> usb: gadget: f_uac2: split out audio core >> usb: gadget: f_uac1: drop volume/mute functionality >> usb: gadget: f_uac1: switch to u_audio core utilities >> usb: gadget: f_uac1: add capture support >> >> drivers/usb/gadget/Kconfig| 13 +- >> drivers/usb/gadget/function/Makefile | 3 +- >> drivers/usb/gadget/function/f_uac1.c | 842 >> +- >> drivers/usb/gadget/function/f_uac2.c | 778 --- >> drivers/usb/gadget/function/u_audio.c | 632 + >> drivers/usb/gadget/function/u_audio.h | 93 >> drivers/usb/gadget/function/u_uac1.c | 314 - >> drivers/usb/gadget/function/u_uac1.h | 71 +-- >> drivers/usb/gadget/legacy/Kconfig | 1 + >> drivers/usb/gadget/legacy/audio.c | 54 ++- >> 10 files changed, 1208 insertions(+), 1593 deletions(-) >> create mode 100644 drivers/usb/gadget/function/u_audio.c >> create mode 100644 drivers/usb/gadget/function/u_audio.h >> delete mode 100644 drivers/usb/gadget/function/u_uac1.c >> >> -- >> 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 5/5] usb: serial: whiteheat: clean up dev_dbg
On Thu, Jul 14, 2016 at 03:01:44PM +0200, Oliver Neukum wrote: > dev_dbg() already can state the function name. No it doesn't. It shows the device, and driver bound to that device, but not a function name. > No need to state it again explicitly. I think it might still be needed here, if the strings are not unique for the file. 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 v2 2/5] usb: serial: removing redundant __func__
On Thu, Jul 14, 2016 at 03:01:41PM +0200, Oliver Neukum wrote: > dev_dbg already states the function it is called from. Printing > it again is wasted space. Nope, it's not printed "again", test it and see :) -- 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 4/5] usb: serial: visor: clean up dev_dbg
On Thu, Jul 14, 2016 at 03:01:43PM +0200, Oliver Neukum wrote: > dev_dbg() already can state the function name. > No need to state it again explicitly. Nope, not true :) -- 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 3/5] usb: serial: usb_wwan.c: clean up dev_dbg
On Thu, Jul 14, 2016 at 03:01:42PM +0200, Oliver Neukum wrote: > dev_dbg() already can state the function name. > No need to state it again explicitly. Same comment as before... -- 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 2/5] usb: serial: removing redundant __func__
On Fri, 2016-07-15 at 06:45 +0900, Greg KH wrote: > On Thu, Jul 14, 2016 at 03:01:41PM +0200, Oliver Neukum wrote: > > dev_dbg already states the function it is called from. Printing > > it again is wasted space. > > Nope, it's not printed "again", test it and see :) > > echo -n "module usbcore +pf" > /sys/kernel/debug/dynamic_debug/control [39806.943024] hub_event: hub 3-0:1.0: state 7 ports 15 chg evt 0040 [39806.943056] hub_port_connect_change: usb usb3-port6: status 0101, change 0001, 12 Mb/s [39807.099516] hub_port_debounce: usb usb3-port6: debounce total 125ms stable 100ms status 0x101 [39807.211472] usb 3-6: new high-speed USB device number 26 using xhci_hcd [39807.339929] usb_get_langid: usb 3-6: default language 0x0409 [39807.340368] usb_new_device: usb 3-6: udev 26, busnum 3, minor = 281 [39807.340372] usb 3-6: New USB device found, idVendor=22b8, idProduct=2e62 [39807.340373] usb 3-6: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [39807.340374] usb 3-6: Product: XT1052 [39807.340375] usb 3-6: Manufacturer: motorola [39807.340376] usb 3-6: SerialNumber: TA64300IE6 [39807.340505] usb_probe_device: usb 3-6: usb_probe_device [39807.340508] usb_choose_configuration: usb 3-6: configuration #1 chosen from 1 choice [39807.341226] usb_set_configuration: usb 3-6: adding 3-6:1.0 (config #1, interface 0) [39807.341421] hub_event: hub 3-0:1.0: state 7 ports 15 chg evt 0040 [39808.352289] usb 3-6: usbfs: process 2391 (ThreadWeaver::T) did not claim interface 0 before use [39808.352302] usb_forced_unbind_intf: usbfs 3-6:1.0: forced unbind [39808.352322] usb_hcd_flush_endpoint: xhci_hcd :00:14.0: shutdown urb 8805e5d25840 ep1in-bulk [39808.467534] usb 3-6: reset high-speed USB device number 26 using xhci_hcd And the source shows that the function is included: #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)\ static struct _ddebug __aligned(8) \ __attribute__((section("__verbose"))) name = { \ .modname = KBUILD_MODNAME, \ .function = __func__, \ .filename = __FILE__, \ .format = (fmt),\ .lineno = __LINE__, \ .flags = _DPRINTK_FLAGS_DEFAULT, \ } #define dynamic_pr_debug(fmt, ...) \ do {\ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \ __dynamic_pr_debug(&descriptor, pr_fmt(fmt),\ ##__VA_ARGS__); \ } while (0) #define dynamic_dev_dbg(dev, fmt, ...) \ do {\ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)) \ __dynamic_dev_dbg(&descriptor, dev, fmt,\ ##__VA_ARGS__); \ } while (0) Maybe I am dense. Could you elaborate? 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 v2 2/5] usb: serial: removing redundant __func__
On Fri, Jul 15, 2016 at 12:06:55AM +0200, Oliver Neukum wrote: > On Fri, 2016-07-15 at 06:45 +0900, Greg KH wrote: > > On Thu, Jul 14, 2016 at 03:01:41PM +0200, Oliver Neukum wrote: > > > dev_dbg already states the function it is called from. Printing > > > it again is wasted space. > > > > Nope, it's not printed "again", test it and see :) > > > > > echo -n "module usbcore +pf" > /sys/kernel/debug/dynamic_debug/control > > [39806.943024] hub_event: hub 3-0:1.0: state 7 ports 15 chg evt 0040 > [39806.943056] hub_port_connect_change: usb usb3-port6: status 0101, change > 0001, 12 Mb/s > [39807.099516] hub_port_debounce: usb usb3-port6: debounce total 125ms stable > 100ms status 0x101 > [39807.211472] usb 3-6: new high-speed USB device number 26 using xhci_hcd > [39807.339929] usb_get_langid: usb 3-6: default language 0x0409 > [39807.340368] usb_new_device: usb 3-6: udev 26, busnum 3, minor = 281 > [39807.340372] usb 3-6: New USB device found, idVendor=22b8, idProduct=2e62 > [39807.340373] usb 3-6: New USB device strings: Mfr=1, Product=2, > SerialNumber=3 > [39807.340374] usb 3-6: Product: XT1052 > [39807.340375] usb 3-6: Manufacturer: motorola > [39807.340376] usb 3-6: SerialNumber: TA64300IE6 > [39807.340505] usb_probe_device: usb 3-6: usb_probe_device Doh, nevermind, that's what I get for writing emails early in the morning while jet-lagged. Sorry for the noise, you are correct. 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 v2 5/5] usb: serial: whiteheat: clean up dev_dbg
Greg KH writes: > On Thu, Jul 14, 2016 at 03:01:44PM +0200, Oliver Neukum wrote: >> dev_dbg() already can state the function name. > > No it doesn't. It shows the device, and driver bound to that device, > but not a function name. Use dynamic debugging. You can have the function name, line number or whatever. Are there anyone still using these debug messages without dynamic debugging? If so, then more incentives to enable dynamic debugging can only do good :) Bjørn -- 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: ehci-platform: use helper variables in probe function
On Thu, 14 Jul 2016, Rafał Miłecki wrote: > > Okay except for... > > > >> - priv->num_phys = of_count_phandle_with_args(dev->dev.of_node, > >> - "phys", "#phy-cells"); > >> + priv->num_phys = of_count_phandle_with_args(np, "phys", > >> + "#phy-cells"); > > > > Please indent continuation lines two tab stops beyond the original > > line, to match the style in the rest of the source file. > > I'm afraid this file doesn't have any consistent coding style for line breaks. > > 1) dma_coerce_mask_and_coherent > One extra tab after line break. > > 2) devm_kcalloc > Two extra tabs and 4 spaces. No real alignment noticed. > > 3) devm_of_phy_get_by_index > Two extra tabs > > 4) devm_reset_control_get_shared_by_index > Three extra tabs > > With these pointed, do you still think I should use two extra tabs? If > so, I'll send V2 as you suggested. Just let me know. You're right, it's a mess. If you send in an updated patch with two extra tabs, I'll create a style-only patch that fixes the other alignment issues. Alan PS: The probe routine in ohci-platform.c could use the same kind of local variables. Would you like to send in patch for that routine too? -- 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
bug 120981 - usb controller reset / disconnect -
I am also able to reproduce this USB disconnect glitch on a Dell Xeon workstation. Is there any way I can get more verbose output to dmesg: On Sat, Jun 25, 2016 at 12:29:39PM +, bugzilla-dae...@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=120981 > > Bug ID: 120981 >Summary: 4.6.x VIA VL805 USB 3.0 controller resets device > making it unusable until re-plugged >Product: Drivers >Version: 2.5 > Kernel Version: 4.6.x Reproduced on 4.6.2, will try on 4.6.4. Any suggestions on how to gather additional logging info would be appreciated. -- 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 5/5] usb: serial: whiteheat: clean up dev_dbg
On Fri, Jul 15, 2016 at 02:36:27AM +0200, Bjørn Mork wrote: > Greg KH writes: > > On Thu, Jul 14, 2016 at 03:01:44PM +0200, Oliver Neukum wrote: > >> dev_dbg() already can state the function name. > > > > No it doesn't. It shows the device, and driver bound to that device, > > but not a function name. > > Use dynamic debugging. You can have the function name, line number or > whatever. Yeah, I was wrong, my mistake, I blame a lack of coffee... -- 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] usb: typec: Introduce USB PD sink state machine driver and add PD sink support for Intel BXT PMIC Type-C phy
This series introduce a USB PD(Power Delivery) sink port simple state machine driver and adds USB PD sink port support for Intel BXT Whiskey Cove PMIC Type-C phy driver. This series depends on these two patches: https://lkml.org/lkml/2016/6/29/349 https://lkml.org/lkml/2016/6/29/350 Bin Gao (1): usb: typec: Add USB Power Delivery sink port support Chandra Sekhar Anagani (1): usb: typec: add PD sink port support for Intel Whiskey Cove PMIC USB Type-C PHY driver drivers/usb/typec/Kconfig | 13 + drivers/usb/typec/Makefile | 1 + drivers/usb/typec/pd_sink.c | 967 + include/linux/usb/pd_message.h | 371 include/linux/usb/pd_sink.h | 286 drivers/usb/typec/typec_wcove.c | 289 6 files changed, 1901 insertions(+), 26 deletions(-) create mode 100644 drivers/usb/typec/pd_sink.c create mode 100644 include/linux/usb/pd_message.h create mode 100644 include/linux/usb/pd_sink.h -- 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] usb: typec: Add USB Power Delivery sink port support
This patch implements a simple USB Power Delivery sink port state machine. It assumes the hardware only handles PD packet transmitting and receiving over the CC line of the USB Type-C connector. The state transition is completely controlled by software. This patch only implement the sink port function and it doesn't support source port and port swap yet. This patch depends on these two patches: https://lkml.org/lkml/2016/6/29/349 https://lkml.org/lkml/2016/6/29/350 Signed-off-by: Bin Gao --- drivers/usb/typec/Kconfig | 13 + drivers/usb/typec/Makefile | 1 + drivers/usb/typec/pd_sink.c| 967 + include/linux/usb/pd_message.h | 371 include/linux/usb/pd_sink.h| 286 5 files changed, 1638 insertions(+) create mode 100644 drivers/usb/typec/pd_sink.c create mode 100644 include/linux/usb/pd_message.h create mode 100644 include/linux/usb/pd_sink.h diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig index 7a345a4..a04a900 100644 --- a/drivers/usb/typec/Kconfig +++ b/drivers/usb/typec/Kconfig @@ -4,12 +4,25 @@ menu "USB PD and Type-C drivers" config TYPEC tristate +config USB_PD_SINK + bool "USB Power Delivery Sink Port State Machine Driver" + select TYPEC + help + Enable this to support USB PD(Power Delivery) Sink port. + This driver implements a simple USB PD sink state machine. + The underlying TypeC phy driver is responsible for cable + plug/unplug event, port orientation detection, transmitting + and receiving PD messages. This driver only process messages + received by the TypeC phy driver and maintain the sink port's + state machine. + config TYPEC_WCOVE tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver" depends on ACPI depends on INTEL_SOC_PMIC depends on INTEL_PMC_IPC select TYPEC + select USB_PD_SINK help This driver adds support for USB Type-C detection on Intel Broxton platforms that have Intel Whiskey Cove PMIC. The driver can detect the diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile index b9cb862..a62eb57 100644 --- a/drivers/usb/typec/Makefile +++ b/drivers/usb/typec/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_TYPEC)+= typec.o obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o +obj-$(CONFIG_USB_PD_SINK) += pd_sink.o diff --git a/drivers/usb/typec/pd_sink.c b/drivers/usb/typec/pd_sink.c new file mode 100644 index 000..374bdef --- /dev/null +++ b/drivers/usb/typec/pd_sink.c @@ -0,0 +1,967 @@ +/* + * pd_sink.c - USB PD (Power Delivery) sink port state machine driver + * + * This driver implements a simple USB PD sink port state machine. + * It assumes the upper layer, i.e. the user of this driver, handles + * the PD message receiving and transmitting. The upper layer receives + * PD messages from the Source, queues them to us, and when processing + * the received message we'll call upper layer's transmitting function + * to send PD messages to the source. + * The sink port state machine is maintained in this driver but we also + * broadcast some important PD messages to upper layer as events. + * + * Copyright (C) 2016 Intel Corporation + * + * Author: Bin Gao + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include +#include +#include + +#define MAKE_HEADER(port, header, msg, objs) \ +do { \ + header->type = msg; \ + header->data_role = PD_DATA_ROLE_UFP; \ + header->revision = port->pd_rev; \ + header->power_role = PD_POWER_ROLE_SINK; \ + header->id = roll_msg_id(port); \ + header->nr_objs = objs; \ + header->extended = PD_MSG_NOT_EXTENDED; \ +} while (0) + +static struct pd_sink_port *sink_ports[MAX_NR_SINK_PORTS]; +static int nr_ports; + +BLOCKING_NOTIFIER_HEAD(pd_sink_notifier_list); + +static char *state_strings[] = { + "WAIT_FOR_SOURCE_CAPABILITY", + "REQUEST_SENT", + "ACCEPT_RECEIVED", + "POWER_SUPPLY_READY", +}; + +/* Control messages */ +static char *cmsg_strings[] = { + "GOODCRC", /* 1 */ + "GOTOMIN", /* 2 */ + "ACCEPT", /* 3 */ + "REJECT", /* 4 */ + "PING", /* 5 */ + "PS_RDY", /* 6 */ + "GET_SRC_CAP", /* 7 */ + "GET_SINK_CAP", /* 8 */ + "DR_SWAP", /* 9 */ + "PR_SWAP", /* 10 */ + "VCONN_SWAP", /* 11 */ + "WAIT", /* 12 */ + "SOFT_RESET", /* 13 */ + "RESERVED
[PATCH 2/2] usb: typec: add PD sink port support for Intel Whiskey Cove PMIC Typc-C PHY driver
From: Chandra Sekhar Anagani This adds PD sink port support for the USB Type-C PHY on Intel WhiskeyCove PMIC which is available on some of the Intel Broxton SoC based platforms. This patch depends on these two patches: https://lkml.org/lkml/2016/6/29/349 https://lkml.org/lkml/2016/6/29/350 Signed-off-by: Chandra Sekhar Anagani --- drivers/usb/typec/typec_wcove.c | 289 1 file changed, 263 insertions(+), 26 deletions(-) diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c index c7c2d28..a4250ba 100644 --- a/drivers/usb/typec/typec_wcove.c +++ b/drivers/usb/typec/typec_wcove.c @@ -3,6 +3,7 @@ * * Copyright (C) 2016 Intel Corporation * Author: Heikki Krogerus + * Author: Chandra Sekhar Anagani * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -10,9 +11,11 @@ */ #include +#include #include #include #include +#include #include #include @@ -25,6 +28,7 @@ #define USBC_CONTROL3 0x7003 #define USBC_CC1_CTRL 0x7004 #define USBC_CC2_CTRL 0x7005 +#define USBC_CC_SEL0x7006 #define USBC_STATUS1 0x7007 #define USBC_STATUS2 0x7008 #define USBC_STATUS3 0x7009 @@ -32,7 +36,16 @@ #define USBC_IRQ2 0x7016 #define USBC_IRQMASK1 0x7017 #define USBC_IRQMASK2 0x7018 - +#define USBC_PD_CFG1 0x7019 +#define USBC_PD_CFG2 0x701a +#define USBC_PD_CFG3 0x701b +#define USBC_PD_STATUS 0x701c +#define USBC_RX_STATUS 0x701d +#define USBC_RX_INFO 0x701e +#define USBC_TX_CMD0x701f +#define USBC_TX_INFO 0x7020 +#define USBC_RX_DATA_START 0x7028 +#define USBC_TX_DATA_START 0x7047 /* Register bits */ #define USBC_CONTROL1_MODE_DRP(r) ((r & ~0x7) | 4) @@ -44,7 +57,9 @@ #define USBC_CONTROL3_PD_DIS BIT(1) #define USBC_CC_CTRL_VCONN_EN BIT(1) +#define USBC_CC_CTRL_TX_EN BIT(2) +#define USBC_CC_SEL_CCSEL (BIT(0) | BIT(1)) #define USBC_STATUS1_DET_ONGOING BIT(6) #define USBC_STATUS1_RSLT(r) (r & 0xf) #define USBC_RSLT_NOTHING 0 @@ -79,11 +94,44 @@ USBC_IRQ2_RX_HR | USBC_IRQ2_RX_CR | \ USBC_IRQ2_TX_SUCCESS | USBC_IRQ2_TX_FAIL) +#define USBC_PD_CFG1_ID_FILL BIT(7) + +#define USBC_PD_CFG2_SOP_RXBIT(0) + +#define USBC_PD_CFG3_SR_SOP2 (BIT(7) | BIT(6)) +#define USBC_PD_CFG3_SR_SOP1 (BIT(5) | BIT(4)) +#define USBC_PD_CFG3_SR_SOP0 (BIT(3) | BIT(2)) +#define USBC_PD_CFG3_DATAROLE BIT(1) +#define USBC_PD_CFG3_PWRROLE BIT(0) + +#define USBC_TX_CMD_TXBUF_RDY BIT(0) +#define USBC_TX_CMD_TX_START BIT(1) +#define USBC_TX_CMD_TXBUF_CMD(r) ((r >> 5) & 0x7) + +#define USBC_TX_INFO_TX_SOP(BIT(0) | BIT(1) | BIT(2)) +#define USBC_TX_INFO_TX_RETRIES(BIT(3) | BIT(4) | BIT(5)) + +#define USBC_RX_STATUS_RX_DATA BIT(7) +#define USBC_RX_STATUS_RX_OVERRUN BIT(6) +#define USBC_RX_STATUS_RX_CLEARBIT(0) + +#define USBC_PD_STATUS_RX_RSLT(r) ((r >> 3) & 0x7) +#define USBC_PD_STATUS_TX_RSLT(r) (r & 0x7) + +#define USBC_RX_INFO_RXBYTES(r)((r >> 3) & 0x1f) +#define USBC_RX_INFO_RX_SOP(r) (r & 0x7) + +#define USBC_PD_RX_BUF_LEN 30 +#define USBC_PD_TX_BUF_LEN 30 + struct wcove_typec { + int pd_port_num; struct mutex lock; /* device lock */ struct device *dev; struct regmap *regmap; struct typec_port *port; + struct pd_sink_port pd_port; + struct completion complete; struct typec_capability cap; struct typec_connection con; struct typec_partner partner; @@ -106,6 +154,50 @@ enum wcove_typec_role { WCOVE_ROLE_DEVICE, }; +static struct sink_ps profiles[] = { + + { + .ps_type = PS_TYPE_FIXED, + .ps_fixed = { + .voltage_fixed = 100, /* 5V/50mV = 100 */ + .current_default = 90, /* 900mA/10mA = 90 */ + .current_max= 90, /* 900mA/10mA = 90 */ + }, + + }, + + { + .ps_type = PS_TYPE_FIXED, + .ps_fixed = { + .voltage_fixed = 100, + .current_default = 300, + .current_max= 300, + }, + }, + + { + .ps_type = PS_TYPE_FIXED, + .ps_fixed = { + .voltage_fixed = 240, + .current_default = 300, + .current_max= 300, + }, + }, + +}; + +static struct pd_sink_profile profile = { + .hw_goodcrc_tx =
Re: [PATCH V2 0/1] usb: add HCD providers
On Thu, Jul 14, 2016 at 05:52:43PM +0200, Rafał Miłecki wrote: > On 14 July 2016 at 11:48, Peter Chen wrote: > > On Wed, Jul 13, 2016 at 04:40:53PM +0200, Rafał Miłecki wrote: > >> On 13 July 2016 at 15:50, Felipe Balbi > >> wrote: > >> > Rafał Miłecki writes: > >> >> On 13 July 2016 at 15:20, Felipe Balbi > >> >> wrote: > >> >>> Rafał Miłecki writes: > >> Hi again, > >> > >> This is my second try of getting HCD providers into usb subsystem. > >> > >> During discussion of V1 I realized there are about 26 drivers adding a > >> single HCD and all of them would need to be modified. So instead I > >> decided to put relevant code in usb_add_hcd. It checks if the HCD we > >> register is a primary one and if so, it registers a proper provider. > >> > >> Please note that of_hcd_xlate_simple was also extended to allow > >> getting > >> shared HCD (which is used e.g. in case of XHCI). > >> > >> So now you can have something like: > >> > >> ohci: ohci@21000 { > >> #usb-cells = <0>; > >> compatible = "generic-ohci"; > >> reg = <0x1000 0x1000>; > >> interrupts = ; > >> }; > >> > >> ehci: ehci@22000 { > >> #usb-cells = <0>; > >> compatible = "generic-ehci"; > >> reg = <0x2000 0x1000>; > >> interrupts = ; > >> }; > >> > >> xhci: xhci@23000 { > >> #usb-cells = <1>; > >> compatible = "generic-xhci"; > >> reg = <0x3000 0x1000>; > >> interrupts = ; > >> }; > >> > >> The last (second) patch is not supposed to be applied, it's used only > >> as > >> a proof and example of how providers can be used. > >> >>> > >> >>> nowhere here (or in previous patch) you clarify why exactly you need > >> >>> this. What is your LED trigger supposed to do? Why can't it handle > >> >>> ports > >> >>> changing number in different boots? Why do we need this at all? Why is > >> >>> your code DT-specific? > >> >>> > >> >>> There are still too many 'unknowns' here. > >> >> > >> >> Are you sure you saw my reply to Peter's question? > >> >> > >> >> http://www.spinics.net/lists/linux-usb/msg143708.html > >> >> http://marc.info/?l=linux-usb&m=146838735627093&w=2 > >> >> > >> >> I think it should answer (some of?) your questions. Can you read it > >> >> and see if it gets a bit clearer? > >> > > >> > well, all that says is that you're writing a LED trigger to toggle LED > >> > when a USB device gets added to a specified port. I don't think you need > >> > the actual port number for that. You should have a phandle to the actual > >> > port, whatever its number is, or a phandle to the (root-)Hub and a port > >> > number from that hub. > >> > > >> > The problem, really, is that DT descriptor of USB Hosts is very, very > >> > minimal. Perhaps there's something more extensively defined from the > >> > original Open Firmware USB Addendum. > >> > >> Thanks for your effort and looking at this closely. You're right, I'm > >> interested in referencing USB ports, but I'm using controller phandle > >> (and then I specify ports manually). > >> > >> Having each port described by DT would be helpful, it's just something > >> I didn't find implemented, so I started looking for different ways. It > >> seems I should have picked a different solution. > >> > >> So should I work on describing USB ports in DT instead? This looks > >> like a complex thing to describe, so I'd like to ask for some guidance > >> first. What do you think about following schema/example? > >> > >> ohci@1000 { > >> compatible = "generic-ohci"; > >> reg = <0x1000 0x1000>; > >> interrupts = ; > >> > >> primary-hcd { > >> ohci_port0: port@0 { > >> reg = <0>; > >> }; > >> > >> ohci_port1: port@1 { > >> reg = <1>; > >> }; > >> } > >> }; > >> > >> ehci@2000 { > >> compatible = "generic-ehci"; > >> reg = <0x2000 0x1000>; > >> interrupts = ; > >> > >> primary-hcd { > >> ehci_port0: port@0 { > >> reg = <0>; > >> }; > >> > >> ehci_port1: port@1 { > >> reg = <1>; > >> }; > >> } > >> }; > >> > >> xhci@3000 { > >> compatible = "generic-xhci"; > >> reg = <0x3000 0x1000>; > >> interrupts = ; > >> > >> primary-hcd { > >> }; > >> > >> shared-hcd { > >> xhci_port0: port@0 { > >> reg = <0>; > >> }; > >> } > >> }; > >> > >> With such a DT struct, how could I query port for a Linux-assigned number? > >> > >> For example with OHCI, EHCI and XHCI drivers compiled, Linux assigns > >> number 4 to my XHCI's shared HCD's r
[PATCH V2] usb: ehci-platform: use helper variables in probe function
Probing function was using &dev->dev and dev->dev.of_node over 20 times so I believe it made sense to use helper variables for both of them. To avoid some uncommon variable name for struct device I first replaced existing dev variable with pdev. Signed-off-by: Rafał Miłecki Acked-by: Alan Stern --- V2: Use 2 extra tabs after line break to match one of already used coding styles. --- drivers/usb/host/ehci-platform.c | 65 +++- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c index 6816b8c..d67dd92 100644 --- a/drivers/usb/host/ehci-platform.c +++ b/drivers/usb/host/ehci-platform.c @@ -143,11 +143,13 @@ static struct usb_ehci_pdata ehci_platform_defaults = { .power_off =ehci_platform_power_off, }; -static int ehci_platform_probe(struct platform_device *dev) +static int ehci_platform_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; struct usb_hcd *hcd; struct resource *res_mem; - struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev); + struct usb_ehci_pdata *pdata = dev_get_platdata(dev); struct ehci_platform_priv *priv; struct ehci_hcd *ehci; int err, irq, phy_num, clk = 0, rst; @@ -162,52 +164,49 @@ static int ehci_platform_probe(struct platform_device *dev) if (!pdata) pdata = &ehci_platform_defaults; - err = dma_coerce_mask_and_coherent(&dev->dev, + err = dma_coerce_mask_and_coherent(dev, pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32)); if (err) { - dev_err(&dev->dev, "Error: DMA mask configuration failed\n"); + dev_err(dev, "Error: DMA mask configuration failed\n"); return err; } - irq = platform_get_irq(dev, 0); + irq = platform_get_irq(pdev, 0); if (irq < 0) { - dev_err(&dev->dev, "no irq provided"); + dev_err(dev, "no irq provided"); return irq; } - hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev, -dev_name(&dev->dev)); + hcd = usb_create_hcd(&ehci_platform_hc_driver, dev, dev_name(dev)); if (!hcd) return -ENOMEM; - platform_set_drvdata(dev, hcd); - dev->dev.platform_data = pdata; + platform_set_drvdata(pdev, hcd); + dev->platform_data = pdata; priv = hcd_to_ehci_priv(hcd); ehci = hcd_to_ehci(hcd); - if (pdata == &ehci_platform_defaults && dev->dev.of_node) { - if (of_property_read_bool(dev->dev.of_node, "big-endian-regs")) + if (pdata == &ehci_platform_defaults && np) { + if (of_property_read_bool(np, "big-endian-regs")) ehci->big_endian_mmio = 1; - if (of_property_read_bool(dev->dev.of_node, "big-endian-desc")) + if (of_property_read_bool(np, "big-endian-desc")) ehci->big_endian_desc = 1; - if (of_property_read_bool(dev->dev.of_node, "big-endian")) + if (of_property_read_bool(np, "big-endian")) ehci->big_endian_mmio = ehci->big_endian_desc = 1; - if (of_property_read_bool(dev->dev.of_node, - "needs-reset-on-resume")) + if (of_property_read_bool(np, "needs-reset-on-resume")) priv->reset_on_resume = true; - if (of_property_read_bool(dev->dev.of_node, - "has-transaction-translator")) + if (of_property_read_bool(np, "has-transaction-translator")) hcd->has_tt = 1; - priv->num_phys = of_count_phandle_with_args(dev->dev.of_node, - "phys", "#phy-cells"); + priv->num_phys = of_count_phandle_with_args(np, "phys", + "#phy-cells"); if (priv->num_phys > 0) { - priv->phys = devm_kcalloc(&dev->dev, priv->num_phys, + priv->phys = devm_kcalloc(dev, priv->num_phys, sizeof(struct phy *), GFP_KERNEL); if (!priv->phys) return -ENOMEM; @@ -216,7 +215,7 @@ static int ehci_platform_probe(struct platform_device *dev) for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { priv->phys[phy_num] = devm_of_phy_get_by_index( - &dev->dev, dev->dev.of_node, phy_num); + dev, np, phy_num); if (IS_ERR(priv->phys[phy_num])) { err = PTR_ERR(priv->phys[phy_num]);
[PATCH] usb: ohci-platform: use helper variables in probe function
Probing function was using &dev->dev and dev->dev.of_node over 20 times so I believe it made sense to use helper variables for both of them. To avoid some uncommon variable name for struct device I first replaced existing dev variable with pdev. Signed-off-by: Rafał Miłecki --- drivers/usb/host/ohci-platform.c | 66 +++- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c index 898b740..d768c87 100644 --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -111,11 +111,13 @@ static struct usb_ohci_pdata ohci_platform_defaults = { .power_off =ohci_platform_power_off, }; -static int ohci_platform_probe(struct platform_device *dev) +static int ohci_platform_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; struct usb_hcd *hcd; struct resource *res_mem; - struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev); + struct usb_ohci_pdata *pdata = dev_get_platdata(dev); struct ohci_platform_priv *priv; struct ohci_hcd *ohci; int err, irq, phy_num, clk = 0, rst = 0; @@ -130,47 +132,45 @@ static int ohci_platform_probe(struct platform_device *dev) if (!pdata) pdata = &ohci_platform_defaults; - err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32)); + err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); if (err) return err; - irq = platform_get_irq(dev, 0); + irq = platform_get_irq(pdev, 0); if (irq < 0) { - dev_err(&dev->dev, "no irq provided"); + dev_err(dev, "no irq provided"); return irq; } - hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev, - dev_name(&dev->dev)); + hcd = usb_create_hcd(&ohci_platform_hc_driver, dev, dev_name(dev)); if (!hcd) return -ENOMEM; - platform_set_drvdata(dev, hcd); - dev->dev.platform_data = pdata; + platform_set_drvdata(pdev, hcd); + dev->platform_data = pdata; priv = hcd_to_ohci_priv(hcd); ohci = hcd_to_ohci(hcd); - if (pdata == &ohci_platform_defaults && dev->dev.of_node) { - if (of_property_read_bool(dev->dev.of_node, "big-endian-regs")) + if (pdata == &ohci_platform_defaults && np) { + if (of_property_read_bool(np, "big-endian-regs")) ohci->flags |= OHCI_QUIRK_BE_MMIO; - if (of_property_read_bool(dev->dev.of_node, "big-endian-desc")) + if (of_property_read_bool(np, "big-endian-desc")) ohci->flags |= OHCI_QUIRK_BE_DESC; - if (of_property_read_bool(dev->dev.of_node, "big-endian")) + if (of_property_read_bool(np, "big-endian")) ohci->flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC; - if (of_property_read_bool(dev->dev.of_node, "no-big-frame-no")) + if (of_property_read_bool(np, "no-big-frame-no")) ohci->flags |= OHCI_QUIRK_FRAME_NO; - of_property_read_u32(dev->dev.of_node, "num-ports", -&ohci->num_ports); + of_property_read_u32(np, "num-ports", &ohci->num_ports); - priv->num_phys = of_count_phandle_with_args(dev->dev.of_node, - "phys", "#phy-cells"); + priv->num_phys = of_count_phandle_with_args(np, "phys", + "#phy-cells"); if (priv->num_phys > 0) { - priv->phys = devm_kcalloc(&dev->dev, priv->num_phys, + priv->phys = devm_kcalloc(dev, priv->num_phys, sizeof(struct phy *), GFP_KERNEL); if (!priv->phys) return -ENOMEM; @@ -178,8 +178,8 @@ static int ohci_platform_probe(struct platform_device *dev) priv->num_phys = 0; for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { - priv->phys[phy_num] = devm_of_phy_get_by_index( - &dev->dev, dev->dev.of_node, phy_num); + priv->phys[phy_num] = devm_of_phy_get_by_index(dev, np, + phy_num); if (IS_ERR(priv->phys[phy_num])) { err = PTR_ERR(priv->phys[phy_num]); goto err_put_hcd; @@ -187,7 +187,7 @@ static int ohci_platform_probe(struct platform_device *dev) } for (clk = 0; clk < OHCI_MAX_CLKS; clk++) { - priv->clks[clk] = of_clk_get(dev->dev.of_node, clk); +
Re: [PATCH V2 0/1] usb: add HCD providers
On 15 July 2016 at 04:28, Peter Chen wrote: > On Thu, Jul 14, 2016 at 05:52:43PM +0200, Rafał Miłecki wrote: >> On 14 July 2016 at 11:48, Peter Chen wrote: >> > On Wed, Jul 13, 2016 at 04:40:53PM +0200, Rafał Miłecki wrote: >> >> Thanks for your effort and looking at this closely. You're right, I'm >> >> interested in referencing USB ports, but I'm using controller phandle >> >> (and then I specify ports manually). >> >> >> >> Having each port described by DT would be helpful, it's just something >> >> I didn't find implemented, so I started looking for different ways. It >> >> seems I should have picked a different solution. >> >> >> >> So should I work on describing USB ports in DT instead? This looks >> >> like a complex thing to describe, so I'd like to ask for some guidance >> >> first. What do you think about following schema/example? >> >> >> >> ohci@1000 { >> >> compatible = "generic-ohci"; >> >> reg = <0x1000 0x1000>; >> >> interrupts = ; >> >> >> >> primary-hcd { >> >> ohci_port0: port@0 { >> >> reg = <0>; >> >> }; >> >> >> >> ohci_port1: port@1 { >> >> reg = <1>; >> >> }; >> >> } >> >> }; >> >> >> >> ehci@2000 { >> >> compatible = "generic-ehci"; >> >> reg = <0x2000 0x1000>; >> >> interrupts = ; >> >> >> >> primary-hcd { >> >> ehci_port0: port@0 { >> >> reg = <0>; >> >> }; >> >> >> >> ehci_port1: port@1 { >> >> reg = <1>; >> >> }; >> >> } >> >> }; >> >> >> >> xhci@3000 { >> >> compatible = "generic-xhci"; >> >> reg = <0x3000 0x1000>; >> >> interrupts = ; >> >> >> >> primary-hcd { >> >> }; >> >> >> >> shared-hcd { >> >> xhci_port0: port@0 { >> >> reg = <0>; >> >> }; >> >> } >> >> }; >> >> >> >> With such a DT struct, how could I query port for a Linux-assigned number? >> >> >> >> For example with OHCI, EHCI and XHCI drivers compiled, Linux assigns >> >> number 4 to my XHCI's shared HCD's root hub: >> >> xhci-hcd 18023000.xhci: xHCI Host Controller >> >> xhci-hcd 18023000.xhci: new USB bus registered, assigned bus number 4 >> >> hub 4-0:1.0: USB hub found >> >> hub 4-0:1.0: 1 port detected >> >> >> >> If I disable OHCI and EHCI I get: >> >> xhci-hcd xhci-hcd.0: xHCI Host Controller >> >> xhci-hcd xhci-hcd.0: new USB bus registered, assigned bus number 2 >> >> hub 2-0:1.0: USB hub found >> >> hub 2-0:1.0: 1 port detected >> >> >> >> So I need my "usbport" trigger driver to be able to get "4-1" in the >> >> first case and "2-1" in the second case. I guess I should use >> >> &xhci_port0 but what then? How could I translate it into >> >> Linux-assigned numbering? >> >> >> > >> > For your current design, you need to fix shared hcd for xHCI problem, >> > since xHCI has two buses. >> > >> > Below I supply another thought, please check if it is feasible. >> > In below design, you don't need to change any usb codes. >> > >> > dts: >> > >> > led_1 { >> > led_gpio_1; >> > usb_port = &ohci_port0, &ehci_port1; >> > } >> > >> > led_2 { >> > led_gpio_2; >> > usb_port = &xhci_port0, &xhci_port1; >> > } >> > >> > ohci@1000 { >> > compatible = "generic-ohci"; >> > reg = <0x1000 0x1000>; >> > interrupts = ; >> > >> > ohci_port0: port@0 { >> > reg = <0>; >> > }; >> > >> > ohci_port1: port@1 { >> > reg = <1>; >> > }; >> > }; >> > >> > ehci@2000 { >> > compatible = "generic-ehci"; >> > reg = <0x2000 0x1000>; >> > interrupts = ; >> > >> > ehci_port0: port@0 { >> > reg = <0>; >> > }; >> > >> > ehci_port1: port@1 { >> > reg = <1>; >> > }; >> > }; >> > >> > xhci@3000 { >> > compatible = "generic-xhci"; >> > reg = <0x3000 0x1000>; >> > interrupts = ; >> > >> > /* for xhci, port 0 - [N-1] is USB3, N - [M-1] is USB2/1. >> > * The port 0 and port N is the same physical port >> > */ >> > xhci_port0: port@0 { >> > reg = <0>; >> > }; >> > >> > xhci_port1: port@1 { >> > reg = <1>; >> > }; >> > >> > }; >> > >> > At code, compare the usb_device's device_node at usbport_trig_notify >> > if it is at led_1's usb device list, light on it. >> >> This is quite interesting idea, thanks! >> >> So I got following checking code: >> >> count = of_count_phandle_with_args(np, "usb-ports", NULL); >> for (i = 0; i < count; i++) { >> of_parse_phandle_with_args(np, "usb-ports", NULL, i, &args); >> of_property_read_u32(args.np, "reg", &port); >> if (args.np->parent == usb_dev->bus->control
Re: bug 120981 - usb controller reset / disconnect -
On Thu, Jul 14, 2016 at 09:13:28PM -0400, Warren Postma wrote: > I am also able to reproduce this USB disconnect glitch on a Dell Xeon > workstation. Is there any way I can get more verbose output to dmesg: > > > On Sat, Jun 25, 2016 at 12:29:39PM +, > bugzilla-dae...@bugzilla.kernel.org wrote: > > https://bugzilla.kernel.org/show_bug.cgi?id=120981 > > > > Bug ID: 120981 > >Summary: 4.6.x VIA VL805 USB 3.0 controller resets device > > making it unusable until re-plugged > >Product: Drivers > >Version: 2.5 > > Kernel Version: 4.6.x > > Reproduced on 4.6.2, will try on 4.6.4. Any suggestions on how to > gather additional logging info would be appreciated. Try to enable dynamic debug (CONFIG_DYNAMIC_DEBUG), and run below before testing: echo "file drivers/usb/* +p" > /sys/kernel/debug/dynamic_debug/control -- Best Regards, Peter Chen -- 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/1] usb: add HCD providers
On Fri, Jul 15, 2016 at 07:48:11AM +0200, Rafał Miłecki wrote: > >> > Below I supply another thought, please check if it is feasible. > >> > In below design, you don't need to change any usb codes. > >> > > >> > dts: > >> > > >> > led_1 { > >> > led_gpio_1; > >> > usb_port = &ohci_port0, &ehci_port1; > >> > } > >> > > >> > led_2 { > >> > led_gpio_2; > >> > usb_port = &xhci_port0, &xhci_port1; > >> > } > >> > > >> > ohci@1000 { > >> > compatible = "generic-ohci"; > >> > reg = <0x1000 0x1000>; > >> > interrupts = ; > >> > > >> > ohci_port0: port@0 { > >> > reg = <0>; > >> > }; > >> > > >> > ohci_port1: port@1 { > >> > reg = <1>; > >> > }; > >> > }; > >> > > >> > ehci@2000 { > >> > compatible = "generic-ehci"; > >> > reg = <0x2000 0x1000>; > >> > interrupts = ; > >> > > >> > ehci_port0: port@0 { > >> > reg = <0>; > >> > }; > >> > > >> > ehci_port1: port@1 { > >> > reg = <1>; > >> > }; > >> > }; > >> > > >> > xhci@3000 { > >> > compatible = "generic-xhci"; > >> > reg = <0x3000 0x1000>; > >> > interrupts = ; > >> > > >> > /* for xhci, port 0 - [N-1] is USB3, N - [M-1] is USB2/1. > >> > * The port 0 and port N is the same physical port > >> > */ > >> > xhci_port0: port@0 { > >> > reg = <0>; > >> > }; > >> > > >> > xhci_port1: port@1 { > >> > reg = <1>; > >> > }; > >> > > >> > }; > >> > > >> > At code, compare the usb_device's device_node at usbport_trig_notify > >> > if it is at led_1's usb device list, light on it. > >> > >> This is quite interesting idea, thanks! > >> > >> So I got following checking code: > >> > >> count = of_count_phandle_with_args(np, "usb-ports", NULL); > >> for (i = 0; i < count; i++) { > >> of_parse_phandle_with_args(np, "usb-ports", NULL, i, &args); > >> of_property_read_u32(args.np, "reg", &port); > >> if (args.np->parent == usb_dev->bus->controller->of_node && > >> port == usb_dev->portnum) { > >> of_node_put(args.np); > >> return true; > >> } > >> of_node_put(args.np); > >> } > >> return false; > > > > No, compares the USB port directly. > > > > count = of_count_phandle_with_args(np, "usb-ports", NULL); > > for (i = 0; i < count; i++) { > > of_parse_phandle_with_args(np, "usb-ports", NULL, i, &args); > > if (args.np == usb_dev->dev.of_node) > > of_node_put(args.np); > > return true; > > } > > of_node_put(args.np); > > } > > return false; > > If we mean to use usb_dev->dev.of_node I *need* to modify USB > subsystem, since this pointer is never being set by the current code. > > [ 71.410505] usb 1-1: new high-speed USB device number 2 using ehci-platform > [ 71.564928] [usbport_trig_notify] usb_dev:c6ac1000 &usb_dev->dev:c6ac1068 > [ 71.579874] [usbport_trig_notify] dev_name(&usb_dev->dev):1-1 > [ 71.586580] [usbport_trig_notify] usb_dev->dev.of_node: (null) > > Or am I missing something? > You may need below patches: commit 69bec725985324e79b1c47ea287815ac4ddb0521 Author: Peter Chen Date: Fri Feb 19 17:26:15 2016 +0800 USB: core: let USB device know device node commit 7222c832254a75dcd67d683df75753d4a4e125bb Author: Nicolai Stange Date: Thu Mar 17 23:53:02 2016 +0100 usb/core: usb_alloc_dev(): fix setting of ->portnum > > >> This works, but I see 3 more problems: > >> > >> 1) How to access list of available USB devices during activation? > > > > You mean during LED activation? eg your usbport_trig_activate? > > Why do you need it? > > Yes, I mean usbport_trig_activate. If user plugs in USB device and > *then* activates this trigger, we want to set a proper initial state. > We can't only depend on USB_DEVICE_ADD. > Oh, I see, I asked it before. Either you need to register USB notifier before activation Or you need to implement something like usb_node_to_dev eg: like usb_for_each_dev. If device's state is USB_STATE_CONFIGURED this USB device is available. > > >> 2) What about support for non-DT platforms in usbport driver? Should I > >> still allow specifying ports manually? Are you OK with that? > > > > I am afraid I still don't know how to do it for non-DT platforms. > > You can show your design. > > Please take a look at > [PATCH] leds: trigger: Introduce an USB port trigger > https://lkml.org/lkml/2016/7/11/305 > > Basically my idea was to support: > echo usbport > trigger > echo 4-1 > new_port > echo 2-1 > new_port > I know your patch, how you plan to support non-DT platforms before? > > >> 3) What about devices with internal hubs? Should we describe their USB > >> ports in DT as well? Any idea how to do this? > > > > Well, the HUB must be hard-wired on board for this LED trigger case. > > So, you can described USB top
Re: [PATCH 1/2] usb: typec: Add USB Power Delivery sink port support
On Thu, 2016-07-14 at 19:14 -0700, Bin Gao wrote: > This patch implements a simple USB Power Delivery sink port state machine. > It assumes the hardware only handles PD packet transmitting and receiving > over the CC line of the USB Type-C connector. The state transition is > completely controlled by software. This patch only implement the sink port > function and it doesn't support source port and port swap yet. > > +/* > + * For any message we send, we must get a GOODCRC message from the Source. > + * The USB PD spec says the time should be measured between the last bit > + * of the sending message's EOP has been transmitted and the last bit of > + * the receiving GOODCRC message's EOP has been received. The allowed time > + * is minimal 0.9 ms and maximal 1.1 ms. However, this measurement is > + * performed in physical layer. When it reaches to the OS and this driver, > + * the actual time is difficult to predict because of the scheduling, > + * context switch, interrupt preemption and nesting, etc. So we only define > + * a safe timeout value (PD_TIMEOUT_GOODCRC) which is large enough to take > + * account of all software related latency. > + */ > +static int send_message(struct pd_sink_port *port, void *buf, int len, > + u8 msg, bool ctrl_msg, enum sop_type sop_type) > +{ > + if (ctrl_msg && msg == PD_CMSG_GOODCRC) { > + port->tx(port->port, port->tx_priv, buf, len, sop_type); > + print_message(port->port, true, msg, false); > + return 0; > + } > + > + port->tx(port->port, port->tx_priv, buf, len, sop_type); > + print_message(port->port, ctrl_msg, msg, false); > + > + if (!port->hw_goodcrc_rx) { > + port->waiting_goodcrc = true; > + start_timer(port, PD_TIMEOUT_GOODCRC, goodcrc_timeout); > + } > + > + port->last_sent_msg = msg; > + port->last_msg_ctrl = ctrl_msg; > + > + return 0; > +} > + > +static void ack_message(struct pd_sink_port *port, int msg_id) > +{ > + struct pd_msg_header *header = kzalloc(PD_MSG_HEADER_LEN, GFP_KERNEL); This must be GFP_NOIO. We are in a cycle that can lead to deadlock. Assume we are waiting for a request for more power to process IO which we need to ack. 1. memory allocation leads to laundering, blocks on freeing memory 2. launderer decides to perform IO which needs more power 3. more power has already been requested, wait for it to be granted 4. BANG - DEADLOCK > + > + if (!header) { > + MAKE_HEADER(port, header, PD_CMSG_GOODCRC, 0); > + send_message(port, header, PD_MSG_HEADER_LEN, > + PD_CMSG_GOODCRC, true, SOP); > +}} > + > +static int handle_goodcrc(struct pd_sink_port *port, u8 msg_id) > +{ > + if (is_waiting_goodcrc(port, msg_id)) { > + hrtimer_cancel(&port->tx_timer); > + clear_waiting_goodcrc(port); > + switch (msg_id) { > + case PD_DMSG_REQUEST: > + port->state = PD_SINK_STATE_REQUEST_SENT; > + break; > + case PD_DMSG_SINK_CAP: > + pr_info("Got GOODCRC for SINK_CAPABILITY message\n"); > + default: > + break; > + } > + } else > + pr_warn("Unexpected GOODCRC message received.\n"); > + > + return 0; > +} > + > +static void handle_accept(struct pd_sink_port *port) > +{ > + enum pd_sink_state state = port->state; > + > + if (state != PD_SINK_STATE_REQUEST_SENT) { > + pr_err("ACCEPT received but not expected, sink state: %s\n", > + state_to_string(state)); > + return; > + } > + > + port->state = PD_SINK_STATE_ACCEPT_RECEIVED; > +} > + > +/* > + * The Source may send REJECT message as a response to REQUEST, PR_SWAP, > + * DR_SWAP or VCONN_SWAP message. Since we only send REQUEST to the Source > + * so we're sure the REJECT is for our REQUEST message. > + */ > +static void handle_reject(struct pd_sink_port *port) > +{ > + enum pd_sink_state state = port->state; > + > + if (state == PD_SINK_STATE_REQUEST_SENT) > + /* Broadcast to upper layer */ > + blocking_notifier_call_chain(&pd_sink_notifier_list, > + PD_SINK_EVENT_REJECT | port->port << 8, NULL); > + else > + pr_err("REJECT received but not expected, sink state: %s\n", > + state_to_string(state)); > +} > + > +static void handle_not_supported(struct pd_sink_port *port) > +{ > + pr_err("sink port %d: %s message %s is not supported by Source\n", > + port->port, port->last_msg_ctrl ? "Control" : "Data", > + msg_to_string(port->last_msg_ctrl, > + port->last_sent_msg & PD_MSG_TYPE_MASK)); > +} > + > +/* > + * The Wait Message is a valid response to a Request, a PR_Swap, DR_Swap or >
Re: [patch] usb: gadget: fsl_qe_udc: off by one in setup_received_handle()
On Wed, Jul 13, 2016 at 01:14:33PM +0300, Dan Carpenter wrote: > The udc->eps[] array has USB_MAX_ENDPOINTS elements so > should be >=. > > Fixes: 3948f0e0c999 ('usb: add Freescale QE/CPM USB peripheral controller > driver') > Signed-off-by: Dan Carpenter > > diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c > b/drivers/usb/gadget/udc/fsl_qe_udc.c > index 93d28cb..cf8819a 100644 > --- a/drivers/usb/gadget/udc/fsl_qe_udc.c > +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c > @@ -2053,7 +2053,7 @@ static void setup_received_handle(struct qe_udc *udc, > struct qe_ep *ep; > > if (wValue != 0 || wLength != 0 > - || pipe > USB_MAX_ENDPOINTS) > + || pipe >= USB_MAX_ENDPOINTS) > break; > ep = &udc->eps[pipe]; > Acked-by: Peter Chen -- Best Regards, Peter Chen -- 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