[PATCH 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ohci-exynos.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 9cf80cb..aa64fb5 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -120,9 +120,8 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); err = -ENOMEM; goto fail_io; } -- 1.7.10.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 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-tegra.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 572634c..ccc6433 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -411,9 +411,8 @@ static int tegra_ehci_probe(struct platform_device *pdev) } hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, resource_size(res)); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); err = -ENOMEM; goto cleanup_clk_en; } -- 1.7.10.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/6] usb: host: ehci-spear: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-spear.c |9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/usb/host/ehci-spear.c b/drivers/usb/host/ehci-spear.c index 8bd915b..6d84cf2 100644 --- a/drivers/usb/host/ehci-spear.c +++ b/drivers/usb/host/ehci-spear.c @@ -106,15 +106,8 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev) hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - if (!devm_request_mem_region(pdev-dev, hcd-rsrc_start, hcd-rsrc_len, - driver-description)) { - retval = -EBUSY; - goto err_put_hcd; - } - - hcd-regs = devm_ioremap(pdev-dev, hcd-rsrc_start, hcd-rsrc_len); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (hcd-regs == NULL) { - dev_dbg(pdev-dev, error mapping memory\n); retval = -ENOMEM; goto err_put_hcd; } -- 1.7.10.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 1/6] usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-exynos.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 7f425ac..bccb6f1 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -135,9 +135,8 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); err = -ENOMEM; goto fail_io; } -- 1.7.10.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/6] usb: host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-msm.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c index f341651..99989aa 100644 --- a/drivers/usb/host/ehci-msm.c +++ b/drivers/usb/host/ehci-msm.c @@ -96,9 +96,8 @@ static int ehci_msm_probe(struct platform_device *pdev) hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, hcd-rsrc_start, hcd-rsrc_len); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (!hcd-regs) { - dev_err(pdev-dev, ioremap failed\n); ret = -ENOMEM; goto put_hcd; } -- 1.7.10.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 3/6] usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-mv.c |8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c index bd61612..5d03787 100644 --- a/drivers/usb/host/ehci-mv.c +++ b/drivers/usb/host/ehci-mv.c @@ -176,10 +176,8 @@ static int mv_ehci_probe(struct platform_device *pdev) goto err_put_hcd; } - ehci_mv-phy_regs = devm_ioremap(pdev-dev, r-start, -resource_size(r)); + ehci_mv-phy_regs = devm_ioremap_resource(pdev-dev, r); if (!ehci_mv-phy_regs) { - dev_err(pdev-dev, failed to map phy I/O memory\n); retval = -EFAULT; goto err_put_hcd; } @@ -191,10 +189,8 @@ static int mv_ehci_probe(struct platform_device *pdev) goto err_put_hcd; } - ehci_mv-cap_regs = devm_ioremap(pdev-dev, r-start, -resource_size(r)); + ehci_mv-cap_regs = devm_ioremap_resource(pdev-dev, r); if (ehci_mv-cap_regs == NULL) { - dev_err(pdev-dev, failed to map I/O memory\n); retval = -EFAULT; goto err_put_hcd; } -- 1.7.10.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/6] usb: host: Cleanup for ioremap'ing hcd memory
Based on 'usb-next' branch of Greg's usb tree. devm_ioremap_resource() API is advantageous over devm_ioremap() and should therefore be preferred to request any ioremap'ed address for hcd. Vivek Gautam (6): usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-spear: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap drivers/usb/host/ehci-exynos.c |3 +-- drivers/usb/host/ehci-msm.c|3 +-- drivers/usb/host/ehci-mv.c |8 ++-- drivers/usb/host/ehci-spear.c |9 + drivers/usb/host/ehci-tegra.c |3 +-- drivers/usb/host/ohci-exynos.c |3 +-- 6 files changed, 7 insertions(+), 22 deletions(-) -- 1.7.10.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 1/6] usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap
Sat, 10 May 2014 15:26:58 +0530 от Vivek Gautam gautam.vi...@samsung.com: Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-exynos.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 7f425ac..bccb6f1 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -135,9 +135,8 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); err = -ENOMEM; goto fail_io; } You should check this as: if (IS_ERR(hcd-regs)) { err = PTR_ERR(hcd-regs); ... Same in other patches in this series. ---
Re: Status of PLX USB3380?
Hello I am evaluating the chip right now with the manufacturer driver. As Amit said there are some problems in the probe/remove, but also there are some static variables on the irq handler that are a mistake and the code is also a bit too scrambled. Since the driver also has to support legacy mode (net2280) I am planning to modifiy the net2280 driver to support the enhanded funcionality. Right now I am working on a tool to read/write the eeprom to test as much functionality as possible. Once it is a bit more tested I would love to share it and of course merge it to upstream if the code looks ok :) @Amit, You are using the chip as host/device legacy/enhanced? Regards On Fri, May 9, 2014 at 7:39 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Thu, May 08, 2014 at 11:38:43PM -0007, Amit Uttamchandani wrote: Should I send the driver as a git patch directly to you or to the mailing list? always mailing list, I don't merge anybody's branches -- balbi -- Ricardo Ribalda -- 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/6] usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap
Hi, On Sat, May 10, 2014 at 3:36 PM, Alexander Shiyan shc_w...@mail.ru wrote: Sat, 10 May 2014 15:26:58 +0530 от Vivek Gautam gautam.vi...@samsung.com: Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-exynos.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 7f425ac..bccb6f1 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -135,9 +135,8 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); err = -ENOMEM; goto fail_io; } You should check this as: if (IS_ERR(hcd-regs)) { err = PTR_ERR(hcd-regs); ... Thanks for pointing out. Will change this. Same in other patches in this series. --- -- Best Regards Vivek Gautam Samsung RD Institute, Bangalore India -- 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 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ohci-exynos.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 9cf80cb..dec691d 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -120,10 +120,9 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); - if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); - err = -ENOMEM; + hcd-regs = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(hcd-regs)) { + err = PTR_ERR(hcd-regs); goto fail_io; } -- 1.7.10.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 3/6] usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-mv.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c index bd61612..08147c3 100644 --- a/drivers/usb/host/ehci-mv.c +++ b/drivers/usb/host/ehci-mv.c @@ -176,11 +176,9 @@ static int mv_ehci_probe(struct platform_device *pdev) goto err_put_hcd; } - ehci_mv-phy_regs = devm_ioremap(pdev-dev, r-start, -resource_size(r)); - if (!ehci_mv-phy_regs) { - dev_err(pdev-dev, failed to map phy I/O memory\n); - retval = -EFAULT; + ehci_mv-phy_regs = devm_ioremap_resource(pdev-dev, r); + if (IS_ERR(ehci_mv-phy_regs)) { + retval = PTR_ERR(ehci_mv-phy_regs); goto err_put_hcd; } @@ -191,11 +189,9 @@ static int mv_ehci_probe(struct platform_device *pdev) goto err_put_hcd; } - ehci_mv-cap_regs = devm_ioremap(pdev-dev, r-start, -resource_size(r)); - if (ehci_mv-cap_regs == NULL) { - dev_err(pdev-dev, failed to map I/O memory\n); - retval = -EFAULT; + ehci_mv-cap_regs = devm_ioremap_resource(pdev-dev, r); + if (IS_ERR(ehci_mv-cap_regs)) { + retval = PTR_ERR(ehci_mv-cap_regs); goto err_put_hcd; } -- 1.7.10.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 2/6] usb: host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-msm.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c index f341651..982c09b 100644 --- a/drivers/usb/host/ehci-msm.c +++ b/drivers/usb/host/ehci-msm.c @@ -96,10 +96,9 @@ static int ehci_msm_probe(struct platform_device *pdev) hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, hcd-rsrc_start, hcd-rsrc_len); - if (!hcd-regs) { - dev_err(pdev-dev, ioremap failed\n); - ret = -ENOMEM; + hcd-regs = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(hcd-regs)) { + ret = PTR_ERR(hcd-regs); goto put_hcd; } -- 1.7.10.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/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-tegra.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 572634c..6fdcb8a 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -411,10 +411,9 @@ static int tegra_ehci_probe(struct platform_device *pdev) } hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, resource_size(res)); - if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); - err = -ENOMEM; + hcd-regs = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(hcd-regs)) { + err = PTR_ERR(hcd-regs); goto cleanup_clk_en; } ehci-caps = hcd-regs + 0x100; -- 1.7.10.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/6] usb: host: ehci-spear: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-spear.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/ehci-spear.c b/drivers/usb/host/ehci-spear.c index 8bd915b..1d59958 100644 --- a/drivers/usb/host/ehci-spear.c +++ b/drivers/usb/host/ehci-spear.c @@ -106,16 +106,9 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev) hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - if (!devm_request_mem_region(pdev-dev, hcd-rsrc_start, hcd-rsrc_len, - driver-description)) { - retval = -EBUSY; - goto err_put_hcd; - } - - hcd-regs = devm_ioremap(pdev-dev, hcd-rsrc_start, hcd-rsrc_len); - if (hcd-regs == NULL) { - dev_dbg(pdev-dev, error mapping memory\n); - retval = -ENOMEM; + hcd-regs = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(hcd-regs)) { + retval = PTR_ERR(hcd-regs); goto err_put_hcd; } -- 1.7.10.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 1/6] usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap
Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-exynos.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 7f425ac..c23a6d9 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -135,10 +135,9 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); - if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); - err = -ENOMEM; + hcd-regs = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(hcd-regs)) { + err = PTR_ERR(hcd-regs); goto fail_io; } -- 1.7.10.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 0/6] usb: host: Cleanup for ioremap'ing hcd memory
Based on 'usb-next' branch of Greg's usb tree. devm_ioremap_resource() API is advantageous over devm_ioremap() and should therefore be preferred to request any ioremap'ed address for hcd. Changes from v1: - Changed the way returned pointer is checked for error value as pointed out in the review comment in the mailing list. Vivek Gautam (6): usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-spear: Use devm_ioremap_resource instead of devm_ioremap usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap drivers/usb/host/ehci-exynos.c |7 +++ drivers/usb/host/ehci-msm.c|7 +++ drivers/usb/host/ehci-mv.c | 16 ++-- drivers/usb/host/ehci-spear.c | 13 +++-- drivers/usb/host/ehci-tegra.c |7 +++ drivers/usb/host/ohci-exynos.c |7 +++ 6 files changed, 21 insertions(+), 36 deletions(-) -- 1.7.10.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 7/7] ARM: sunxi: dt: add APP4-EVB1 board support
From: Boris BREZILLON boris.brezil...@free-electrons.com The APP4 EVB1 development boards embeds an A31, together with some NAND, one SD card slot, and one SDIO + UART WiFi and Bluetooth chip, a few I2C buses, USB, and a LCD display. Signed-off-by: Boris BREZILLON boris.brezil...@free-electrons.com Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com --- arch/arm/boot/dts/Makefile| 1 + arch/arm/boot/dts/sun6i-a31-app4-evb1.dts | 57 +++ 2 files changed, 58 insertions(+) create mode 100644 arch/arm/boot/dts/sun6i-a31-app4-evb1.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index ffa3f5ef27d3..d50c0895a9d5 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -343,6 +343,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += \ sun5i-a10s-olinuxino-micro.dtb \ sun5i-a13-olinuxino.dtb \ sun5i-a13-olinuxino-micro.dtb \ + sun6i-a31-app4-evb1.dtb \ sun6i-a31-colombus.dtb \ sun6i-a31-m9.dtb \ sun7i-a20-cubieboard2.dtb \ diff --git a/arch/arm/boot/dts/sun6i-a31-app4-evb1.dts b/arch/arm/boot/dts/sun6i-a31-app4-evb1.dts new file mode 100644 index ..2bbf8867362b --- /dev/null +++ b/arch/arm/boot/dts/sun6i-a31-app4-evb1.dts @@ -0,0 +1,57 @@ +/* + * Copyright 2014 Boris Brezillon + * + * Boris Brezillon boris.brezil...@free-electrons.com + * + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +/dts-v1/; +/include/ sun6i-a31.dtsi +/include/ sunxi-common-regulators.dtsi + +/ { + model = Allwinner A31 APP4 EVB1 Evaluation Board; + compatible = allwinner,app4-evb1, allwinner,sun6i-a31; + + chosen { + bootargs = earlyprintk console=ttyS0,115200; + }; + + soc@01c0 { + pio: pinctrl@01c20800 { + usb1_vbus_pin_a: usb1_vbus_pin@0 { + allwinner,pins = PH27; + allwinner,function = gpio_out; + allwinner,drive = 0; + allwinner,pull = 0; + }; + }; + + usbphy: phy@01c19400 { + usb1_vbus-supply = reg_usb1_vbus; + status = okay; + }; + + ehci0: usb@01c1a000 { + status = okay; + }; + + uart0: serial@01c28000 { + pinctrl-names = default; + pinctrl-0 = uart0_pins_a; + status = okay; + }; + }; + + reg_usb1_vbus: usb1-vbus { + pinctrl-0 = usb1_vbus_pin_a; + gpio = pio 7 27 0; + status = okay; + }; +}; -- 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 v2 6/7] ARM: sun6i: dt: Add support for the USB controllers
The A31 has two ECHI/OHCI controllers, and one OHCI-only phy-less controller. Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com --- arch/arm/boot/dts/sun6i-a31.dtsi | 77 1 file changed, 77 insertions(+) diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi index 13aa56ed5858..5e9f01af6d99 100644 --- a/arch/arm/boot/dts/sun6i-a31.dtsi +++ b/arch/arm/boot/dts/sun6i-a31.dtsi @@ -341,6 +341,83 @@ status = disabled; }; + usbphy: phy@01c19400 { + compatible = allwinner,sun6i-a31-usb-phy; + reg = 0x01c19400 0x10, + 0x01c1a800 0x4, + 0x01c1b800 0x4; + reg-names = phy_ctrl, + pmu1, + pmu2; + clocks = usb_clk 8, +usb_clk 9, +usb_clk 10; + clock-names = usb0_phy, + usb1_phy, + usb2_phy; + resets = usb_clk 0, +usb_clk 1, +usb_clk 2; + reset-names = usb0_reset, + usb1_reset, + usb2_reset; + status = disabled; + #phy-cells = 1; + }; + + ehci0: usb@01c1a000 { + compatible = allwinner,sun6i-a31-ehci, generic-ehci; + reg = 0x01c1a000 0x100; + interrupts = 0 72 4; + clocks = ahb1_gates 26; + resets = ahb1_rst 26; + phys = usbphy 1; + phy-names = usb; + status = disabled; + }; + + ohci0: usb@01c1a400 { + compatible = allwinner,sun6i-a31-ohci, generic-ohci; + reg = 0x01c1a400 0x100; + interrupts = 0 73 4; + clocks = ahb1_gates 29, usb_clk 16; + resets = ahb1_rst 29; + phys = usbphy 1; + phy-names = usb; + status = disabled; + }; + + ehci1: usb@01c1b000 { + compatible = allwinner,sun6i-a31-ehci, generic-ehci; + reg = 0x01c1b000 0x100; + interrupts = 0 74 4; + clocks = ahb1_gates 27; + resets = ahb1_rst 27; + phys = usbphy 2; + phy-names = usb; + status = disabled; + }; + + ohci1: usb@01c1b400 { + compatible = allwinner,sun6i-a31-ohci, generic-ohci; + reg = 0x01c1b400 0x100; + interrupts = 0 75 4; + clocks = ahb1_gates 30, usb_clk 17; + resets = ahb1_rst 30; + phys = usbphy 2; + phy-names = usb; + status = disabled; + }; + + ohci2: usb@01c1c000 { + compatible = allwinner,sun6i-a31-ohci, generic-ohci; + reg = 0x01c1c400 0x100; + interrupts = 0 77 4; + clocks = ahb1_gates 31, usb_clk 18; + resets = ahb1_rst 31; + status = disabled; + }; + pio: pinctrl@01c20800 { compatible = allwinner,sun6i-a31-pinctrl; reg = 0x01c20800 0x400; -- 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 v2 4/7] usb: ehci-platform: add optional reset controller retrieval
From: Boris BREZILLON boris.brezil...@free-electrons.com On the Allwinner's A31 SoC the reset line connected to the EHCI IP has to be deasserted for the EHCI block to be usable. Add support for an optional reset controller that will be deasserted on power off and asserted on power on. Signed-off-by: Boris BREZILLON boris.brezil...@free-electrons.com Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com --- Documentation/devicetree/bindings/usb/usb-ehci.txt | 1 + drivers/usb/host/ehci-platform.c | 18 ++ 2 files changed, 19 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt index ff151ec084c4..43c1a4e06767 100644 --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt @@ -15,6 +15,7 @@ Optional properties: - clocks : a list of phandle + clock specifier pairs - phys : phandle + phy specifier pair - phy-names : usb + - resets : phandle + reset specifier pair Example (Sequoia 440EPx): ehci@e300 { diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c index c7dd93aad20c..da89f274aa76 100644 --- a/drivers/usb/host/ehci-platform.c +++ b/drivers/usb/host/ehci-platform.c @@ -29,6 +29,7 @@ #include linux/of.h #include linux/phy/phy.h #include linux/platform_device.h +#include linux/reset.h #include linux/usb.h #include linux/usb/hcd.h #include linux/usb/ehci_pdriver.h @@ -41,6 +42,7 @@ struct ehci_platform_priv { struct clk *clks[EHCI_MAX_CLKS]; + struct reset_control *rst; struct phy *phy; }; @@ -206,6 +208,19 @@ static int ehci_platform_probe(struct platform_device *dev) break; } } + + priv-rst = devm_reset_control_get_optional(dev-dev, + NULL); + if (IS_ERR(priv-rst)) { + err = PTR_ERR(priv-rst); + if (err == -EPROBE_DEFER) + goto err_put_clks; + priv-rst = NULL; + } else { + err = reset_control_deassert(priv-rst); + if (err) + goto err_put_clks; + } } if (pdata-big_endian_desc) @@ -280,6 +295,9 @@ static int ehci_platform_remove(struct platform_device *dev) if (pdata-power_off) pdata-power_off(dev); + if (priv-rst) + reset_control_assert(priv-rst); + for (clk = 0; clk EHCI_MAX_CLKS priv-clks[clk]; clk++) clk_put(priv-clks[clk]); -- 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 v2 0/7] Add Allwinner A31 USB support
Hi everyone, This patchset adds support for the USB controllers found in the Allwinner A31. While the design is similar to the earlier Allwinner SoCs that are already supported, a few details here and there change, like the fact that the PHYs now have one clock per phy, while it used to be only one for all the PHYs. Thanks, Maxime Changes from v1: - Moved the reset assertion/deassertion to probe/remove - Moved the dedicated_clocks to the probe function instead of the private structure since it was the only user Boris BREZILLON (2): usb: ehci-platform: add optional reset controller retrieval ARM: sunxi: dt: add APP4-EVB1 board support Maxime Ripard (5): clk: sunxi: Implement A31 USB clock ARM: sun6i: Add the USB clocks to the DTSI. phy: usb: sunxi: Introduce Allwinner A31 USB PHY support usb: ohci-platform: Enable optional use of reset controller ARM: sun6i: dt: Add support for the USB controllers Documentation/devicetree/bindings/usb/usb-ehci.txt | 1 + Documentation/devicetree/bindings/usb/usb-ohci.txt | 1 + arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/sun6i-a31-app4-evb1.dts | 57 ++ arch/arm/boot/dts/sun6i-a31.dtsi | 88 ++ drivers/clk/sunxi/clk-sunxi.c | 6 ++ drivers/phy/phy-sun4i-usb.c| 35 ++--- drivers/usb/host/ehci-platform.c | 18 + drivers/usb/host/ohci-platform.c | 18 + 9 files changed, 216 insertions(+), 9 deletions(-) create mode 100644 arch/arm/boot/dts/sun6i-a31-app4-evb1.dts -- 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 v2 2/7] ARM: sun6i: Add the USB clocks to the DTSI.
The USB clocks of the A31 seems to be parented to the 24MHz oscillator, and handle the clocks for the USB phys and OHCI devices. Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com --- arch/arm/boot/dts/sun6i-a31.dtsi | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi index eec1afa257a5..13aa56ed5858 100644 --- a/arch/arm/boot/dts/sun6i-a31.dtsi +++ b/arch/arm/boot/dts/sun6i-a31.dtsi @@ -269,6 +269,17 @@ clocks = osc24M, pll6; clock-output-names = spi3; }; + + usb_clk: clk@01c200cc { + #clock-cells = 1; + #reset-cells = 1; + compatible = allwinner,sun6i-a31-usb-clk; + reg = 0x01c200cc 0x4; + clocks = osc24M; + clock-output-names = usb_phy0, usb_phy1, usb_phy2, +usb_ohci0, usb_ohci1, +usb_ohci2; + }; }; soc@01c0 { -- 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 v2 3/7] phy: usb: sunxi: Introduce Allwinner A31 USB PHY support
The USB phy controller in the A31 differs mostly from the older controllers because it has a clock dedicated for each phy, while the older ones were having a single clock for all the phys. Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com --- drivers/phy/phy-sun4i-usb.c | 35 ++- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c index e6e6c4ba7145..8bd89430d945 100644 --- a/drivers/phy/phy-sun4i-usb.c +++ b/drivers/phy/phy-sun4i-usb.c @@ -61,7 +61,6 @@ #define MAX_PHYS 3 struct sun4i_usb_phy_data { - struct clk *clk; void __iomem *base; struct mutex mutex; int num_phys; @@ -71,6 +70,7 @@ struct sun4i_usb_phy_data { void __iomem *pmu; struct regulator *vbus; struct reset_control *reset; + struct clk *clk; int index; } phys[MAX_PHYS]; }; @@ -146,13 +146,13 @@ static int sun4i_usb_phy_init(struct phy *_phy) struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy); int ret; - ret = clk_prepare_enable(data-clk); + ret = clk_prepare_enable(phy-clk); if (ret) return ret; ret = reset_control_deassert(phy-reset); if (ret) { - clk_disable_unprepare(data-clk); + clk_disable_unprepare(phy-clk); return ret; } @@ -170,11 +170,10 @@ static int sun4i_usb_phy_init(struct phy *_phy) static int sun4i_usb_phy_exit(struct phy *_phy) { struct sun4i_usb_phy *phy = phy_get_drvdata(_phy); - struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy); sun4i_usb_phy_passby(phy, 0); reset_control_assert(phy-reset); - clk_disable_unprepare(data-clk); + clk_disable_unprepare(phy-clk); return 0; } @@ -228,8 +227,10 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) struct phy_provider *phy_provider; struct reset_control *reset; struct regulator *vbus; + bool dedicated_clocks; struct resource *res; struct phy *phy; + struct clk *clk; char name[16]; int i; @@ -249,15 +250,20 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) else data-disc_thresh = 2; + if (of_device_is_compatible(np, allwinner,sun6i-a31-usb-phy)) + dedicated_clocks = true; + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, phy_ctrl); data-base = devm_ioremap_resource(dev, res); if (IS_ERR(data-base)) return PTR_ERR(data-base); - data-clk = devm_clk_get(dev, usb_phy); - if (IS_ERR(data-clk)) { - dev_err(dev, could not get usb_phy clock\n); - return PTR_ERR(data-clk); + if (!dedicated_clocks) { + clk = devm_clk_get(dev, usb_phy); + if (IS_ERR(clk)) { + dev_err(dev, could not get usb_phy clock\n); + return PTR_ERR(clk); + } } /* Skip 0, 0 is the phy for otg which is not yet supported. */ @@ -270,6 +276,15 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) vbus = NULL; } + if (dedicated_clocks) { + snprintf(name, sizeof(name), usb%d_phy, i); + clk = devm_clk_get(dev, name); + if (IS_ERR(clk)) { + dev_err(dev, failed to get clock %s\n, name); + return PTR_ERR(clk); + } + } + snprintf(name, sizeof(name), usb%d_reset, i); reset = devm_reset_control_get(dev, name); if (IS_ERR(reset)) { @@ -296,6 +311,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) data-phys[i].pmu = pmu; data-phys[i].vbus = vbus; data-phys[i].reset = reset; + data-phys[i].clk = clk; data-phys[i].index = i; phy_set_drvdata(phy, data-phys[i]); } @@ -311,6 +327,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) static const struct of_device_id sun4i_usb_phy_of_match[] = { { .compatible = allwinner,sun4i-a10-usb-phy }, { .compatible = allwinner,sun5i-a13-usb-phy }, + { .compatible = allwinner,sun6i-a31-usb-phy }, { .compatible = allwinner,sun7i-a20-usb-phy }, { }, }; -- 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 v2 5/7] usb: ohci-platform: Enable optional use of reset controller
The OHCI controllers used in the Allwinner A31 are asserted in reset using a global reset controller. Add optional support for such a controller in the OHCI platform driver. Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com --- Documentation/devicetree/bindings/usb/usb-ohci.txt | 1 + drivers/usb/host/ohci-platform.c | 18 ++ 2 files changed, 19 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt index 45f67d91e888..b968a1aea995 100644 --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt @@ -12,6 +12,7 @@ Optional properties: - clocks : a list of phandle + clock specifier pairs - phys : phandle + phy specifier pair - phy-names : usb +- resets : phandle + reset specifier pair Example: diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c index b6002c951c5c..25073112abea 100644 --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -24,6 +24,7 @@ #include linux/err.h #include linux/phy/phy.h #include linux/platform_device.h +#include linux/reset.h #include linux/usb/ohci_pdriver.h #include linux/usb.h #include linux/usb/hcd.h @@ -36,6 +37,7 @@ struct ohci_platform_priv { struct clk *clks[OHCI_MAX_CLKS]; + struct reset_control *rst; struct phy *phy; }; @@ -191,6 +193,19 @@ static int ohci_platform_probe(struct platform_device *dev) break; } } + + priv-rst = devm_reset_control_get_optional(dev-dev, + NULL); + if (IS_ERR(priv-rst)) { + err = PTR_ERR(priv-rst); + if (err == -EPROBE_DEFER) + goto err_put_clks; + priv-rst = NULL; + } else { + err = reset_control_deassert(priv-rst); + if (err) + goto err_put_clks; + } } if (pdata-big_endian_desc) @@ -266,6 +281,9 @@ static int ohci_platform_remove(struct platform_device *dev) if (pdata-power_off) pdata-power_off(dev); + if (priv-rst) + reset_control_assert(priv-rst); + for (clk = 0; clk OHCI_MAX_CLKS priv-clks[clk]; clk++) clk_put(priv-clks[clk]); -- 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 v2 1/7] clk: sunxi: Implement A31 USB clock
The A31 USB clock slightly differ from its older counterparts, mostly because it has a different gate for each PHY, while the older one had a single gate for all the phy. Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com --- drivers/clk/sunxi/clk-sunxi.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c index bd7dc733c1ca..d9bab75f128b 100644 --- a/drivers/clk/sunxi/clk-sunxi.c +++ b/drivers/clk/sunxi/clk-sunxi.c @@ -972,6 +972,11 @@ static const struct gates_data sun5i_a13_usb_gates_data __initconst = { .reset_mask = 0x03, }; +static const struct gates_data sun6i_a31_usb_gates_data __initconst = { + .mask = { BIT(18) | BIT(17) | BIT(16) | BIT(10) | BIT(9) | BIT(8) }, + .reset_mask = BIT(2) | BIT(1) | BIT(0), +}; + static void __init sunxi_gates_clk_setup(struct device_node *node, struct gates_data *data) { @@ -1267,6 +1272,7 @@ static const struct of_device_id clk_gates_match[] __initconst = { {.compatible = allwinner,sun6i-a31-apb2-gates-clk, .data = sun6i_a31_apb2_gates_data,}, {.compatible = allwinner,sun4i-a10-usb-clk, .data = sun4i_a10_usb_gates_data,}, {.compatible = allwinner,sun5i-a13-usb-clk, .data = sun5i_a13_usb_gates_data,}, + {.compatible = allwinner,sun6i-a31-usb-clk, .data = sun6i_a31_usb_gates_data,}, {} }; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i.MX6 USB OTG support is broken on linux-next
+ Robin and David, On Sat, May 10, 2014 at 07:10:05PM +0800, Peter Chen wrote: On Fri, May 09, 2014 at 09:00:47PM +0800, Shawn Guo wrote: I'm running next-20140508 kernel on imx6q-sabresd board with USB mouse/keyboard connected to OTG port. It works well on 3.15-rc but is broken on recent linux-next kernel with the message like below. ... usb 1-1: device v413c p2107 is not supported hub 1-0:1.0: unable to enumerate USB device on port 1 With the help from Peter, I found that the issue shows up only when CONFIG_USB_OTG_FSM is enabled. The option is enabled by commit 8fd2f1f (ARM: imx_v6_v7_defconfig: Enable drivers for i.MX51 USB Host support.) from IMX tree. I guess this is a sign that chipidea otg_fsm driver is buggy? I do not think dropping CONFIG_USB_OTG_FSM selection from imx_v6_v7_defconfig is a solution, and we need to fix the issue in usb driver, right? No, Shawn. The solution of this problem is drop CONFIG_USB_OTG_FSM, I guess Denis added it wrongly. According to Embedded Hosts OTG spec, OTG devices and Embedded Hosts both have Targeted Host functionality, and each Targeted Host has its TPL (Targeted Peripheral List), only the devices are at TPL are supported by Targeted Host. Current Linux TPL only contains limited devices, each Targeted Host needs to update its TPL when it makes product, (The comments at otg_whitelist.h also mention it), and the devices are not at TPL will not be supported, and will report to user as an unsupported device error, at current Linux configuration, once the CONFIG_USB_OTG_FSM and CONFIG_USB_OTG are chosen, the host will be Targeted Host. The reason why Shawn met this problem is: the imx_v6_v7_defconfig set CONFIG_USB_OTG_FSM, so the CONFIG_USB_OTG and CONFIG_USB_OTG_WHITELIST are set according to dependency, it makes imx6 device as Targeted Host, but the devices connected at Shawn's board are not at TPL, so they are not be supported. In a word the CONFIG_USB_OTG_FSM should not be chosen for defauly kernel configuration. The CONFIG_USB_OTG_FSM should only be chosen by default when this device goes to make product and the TPL is updated accordingly. Thanks for the explanation, Peter. So this is what I see from USB Kconfig. config USB_OTG_FSM tristate USB 2.0 OTG FSM implementation depends on USB select USB_OTG select USB_PHY config USB_OTG_WHITELIST bool Rely on OTG Targeted Peripherals List depends on USB_OTG || EXPERT default y if USB_OTG I see that USB_OTG_FSM has a dependency on USB_OTG, and USB_OTG_WHITELIST depends on USB_OTG, but USB_OTG does *not* depends on USB_OTG_WHITELIST. I'm not sure why we have 'default y if USB_OTG' for USB_OTG_WHITELIST, when this option only makes sense for product producers per your explanation. So maybe dropping the 'default y' is the right fix? diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig index cb8e991..9081757 100644 --- a/drivers/usb/core/Kconfig +++ b/drivers/usb/core/Kconfig @@ -65,7 +65,6 @@ config USB_OTG config USB_OTG_WHITELIST bool Rely on OTG Targeted Peripherals List depends on USB_OTG || EXPERT - default y if USB_OTG help If you say Y here, the otg_whitelist.h file will be used as a product whitelist, so USB peripherals not listed there will be Shawn -- 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: i.MX6 USB OTG support is broken on linux-next
On Fri, May 09, 2014 at 09:00:47PM +0800, Shawn Guo wrote: I'm running next-20140508 kernel on imx6q-sabresd board with USB mouse/keyboard connected to OTG port. It works well on 3.15-rc but is broken on recent linux-next kernel with the message like below. ... usb 1-1: device v413c p2107 is not supported hub 1-0:1.0: unable to enumerate USB device on port 1 With the help from Peter, I found that the issue shows up only when CONFIG_USB_OTG_FSM is enabled. The option is enabled by commit 8fd2f1f (ARM: imx_v6_v7_defconfig: Enable drivers for i.MX51 USB Host support.) from IMX tree. I guess this is a sign that chipidea otg_fsm driver is buggy? I do not think dropping CONFIG_USB_OTG_FSM selection from imx_v6_v7_defconfig is a solution, and we need to fix the issue in usb driver, right? No, Shawn. The solution of this problem is drop CONFIG_USB_OTG_FSM, I guess Denis added it wrongly. According to Embedded Hosts OTG spec, OTG devices and Embedded Hosts both have Targeted Host functionality, and each Targeted Host has its TPL (Targeted Peripheral List), only the devices are at TPL are supported by Targeted Host. Current Linux TPL only contains limited devices, each Targeted Host needs to update its TPL when it makes product, (The comments at otg_whitelist.h also mention it), and the devices are not at TPL will not be supported, and will report to user as an unsupported device error, at current Linux configuration, once the CONFIG_USB_OTG_FSM and CONFIG_USB_OTG are chosen, the host will be Targeted Host. The reason why Shawn met this problem is: the imx_v6_v7_defconfig set CONFIG_USB_OTG_FSM, so the CONFIG_USB_OTG and CONFIG_USB_OTG_WHITELIST are set according to dependency, it makes imx6 device as Targeted Host, but the devices connected at Shawn's board are not at TPL, so they are not be supported. In a word the CONFIG_USB_OTG_FSM should not be chosen for defauly kernel configuration. The CONFIG_USB_OTG_FSM should only be chosen by default when this device goes to make product and the TPL is updated accordingly. Peter -- 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/6] usb: host: Cleanup for ioremap'ing hcd memory
Sat, 10 May 2014 17:30:04 +0530 от Vivek Gautam gautam.vi...@samsung.com: Based on 'usb-next' branch of Greg's usb tree. devm_ioremap_resource() API is advantageous over devm_ioremap() and should therefore be preferred to request any ioremap'ed address for hcd. Changes from v1: - Changed the way returned pointer is checked for error value as pointed out in the review comment in the mailing list. hcd-rsrc_len field can be removed entirely, since I cannot find any reason how this filed can be used in the drivers now. This is of course should be in an another series. This one looks good. ---
[PATCH usb-linus] usb: cdc-wdm: export cdc-wdm uapi header
The include/uapi/linux/usb/cdc-wdm.h header defines cdc-wdm userspace APIs and should be exported by make headers_install. Cc: sta...@vger.kernel.org # 3.10, 3.12, 3.14 Fixes: 3edce1cf813a (USB: cdc-wdm: implement IOCTL_WDM_MAX_COMMAND) Signed-off-by: Bjørn Mork bj...@mork.no --- Seems like I accidentally left this out when adding the uapi file, and didn't notice until I was trying to use it today... include/uapi/linux/usb/Kbuild | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/usb/Kbuild b/include/uapi/linux/usb/Kbuild index 6cb4ea826834..4cc4d6e7e523 100644 --- a/include/uapi/linux/usb/Kbuild +++ b/include/uapi/linux/usb/Kbuild @@ -1,6 +1,7 @@ # UAPI Header export list header-y += audio.h header-y += cdc.h +header-y += cdc-wdm.h header-y += ch11.h header-y += ch9.h header-y += functionfs.h -- 2.0.0.rc2 -- 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/7] usb: ehci-platform: add optional reset controller retrieval
On Sat, 10 May 2014, Maxime Ripard wrote: From: Boris BREZILLON boris.brezil...@free-electrons.com On the Allwinner's A31 SoC the reset line connected to the EHCI IP has to be deasserted for the EHCI block to be usable. Add support for an optional reset controller that will be deasserted on power off and asserted on power on. Signed-off-by: Boris BREZILLON boris.brezil...@free-electrons.com Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com Reviewed-by: Hans de Goede hdego...@redhat.com This basically is good. fine. I have only two comments, and one of them is a matter of taste rather than substance. @@ -206,6 +208,19 @@ static int ehci_platform_probe(struct platform_device *dev) break; } } + + priv-rst = devm_reset_control_get_optional(dev-dev, + NULL); I hate the style that matches arguments on a continuation line with the opening paren of the function call, for a couple of reasons. Instead I simply indent continuation lines by two or more tab stops. But some people seem to be incurably attached to it. + if (IS_ERR(priv-rst)) { + err = PTR_ERR(priv-rst); + if (err == -EPROBE_DEFER) + goto err_put_clks; + priv-rst = NULL; + } else { + err = reset_control_deassert(priv-rst); + if (err) + goto err_put_clks; + } } if (pdata-big_endian_desc) The new code was added inside an if statement, which will cause it to apply only to OF devices. Is there any reason not to put the new code outside the if statement, so it applies to all devices? 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
Weird I/O errors with USB Hard disk
hi all: I have a USB hard disk and when I play specific file. it will show below message ( I purpose enable usb/storage/transport.c debug message about urb debug) what makes me confused are 1. Does Unhandled sense code mean the SCSI Request Sense command? 2. if #1 is correct, there should be any error handling for not keep sending following commands. Why usb driver keep clear this halt enpoint? isn't there any timeout mechanism no matter in scsi layer or usb layer? appreciate your help [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]sd 0:0:0:0: [sda] Unhandled sense code [22:06:48]sd 0:0:0:0: [sda] Result: hostbyte=0x10 driverbyte=0x08 [22:06:48]sd 0:0:0:0: [sda] Sense Key : 0x3 [current] [22:06:48]sd 0:0:0:0: [sda] ASC=0x11 ASCQ=0x0 [22:06:48]sd 0:0:0:0: [sda] CDB: cdb[0]=0x28: 28 00 00 24 42 22 00 00 f0 00 [22:06:48]end_request: critical target error, dev sda, sector 2376226 [22:06:48]-- short read transfer [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]-- short read transfer [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]-- short read transfer [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]-- short read transfer [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]-- short read transfer [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]-- short read transfer [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]-- short read transfer [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 [22:06:49]-- short read transfer [22:06:49]clearing endpoint halt for pipe 0xc0008280 .. -- 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: Weird I/O errors with USB Hard disk
On Sat, 2014-05-10 at 22:38 +0800, loody wrote: hi all: I have a USB hard disk and when I play specific file. it will show below message ( I purpose enable usb/storage/transport.c debug message about urb debug) what makes me confused are 1. Does Unhandled sense code mean the SCSI Request Sense command? Error handling couldn't correct the error so it was passed up. 2. if #1 is correct, there should be any error handling for not keep sending following commands. Why usb driver keep clear this halt enpoint? isn't there any timeout mechanism no matter in scsi layer or usb layer? appreciate your help [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]sd 0:0:0:0: [sda] Unhandled sense code [22:06:48]sd 0:0:0:0: [sda] Result: hostbyte=0x10 driverbyte=0x08 [22:06:48]sd 0:0:0:0: [sda] Sense Key : 0x3 [current] [22:06:48]sd 0:0:0:0: [sda] ASC=0x11 ASCQ=0x0 [22:06:48]sd 0:0:0:0: [sda] CDB: cdb[0]=0x28: 28 00 00 24 42 22 00 00 f0 00 [22:06:48]end_request: critical target error, dev sda, sector 2376226 If you compiled your kernel with SCSI messages it would tell you that this is a medium error, unrecovered read error in sector 2376226 The device is failing and the sector cannot be recovered. James -- 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: i.MX6 USB OTG support is broken on linux-next
On Sat, May 10, 2014 at 09:18:36PM +0800, Shawn Guo wrote: + Robin and David, On Sat, May 10, 2014 at 07:10:05PM +0800, Peter Chen wrote: On Fri, May 09, 2014 at 09:00:47PM +0800, Shawn Guo wrote: I'm running next-20140508 kernel on imx6q-sabresd board with USB mouse/keyboard connected to OTG port. It works well on 3.15-rc but is broken on recent linux-next kernel with the message like below. ... usb 1-1: device v413c p2107 is not supported hub 1-0:1.0: unable to enumerate USB device on port 1 With the help from Peter, I found that the issue shows up only when CONFIG_USB_OTG_FSM is enabled. The option is enabled by commit 8fd2f1f (ARM: imx_v6_v7_defconfig: Enable drivers for i.MX51 USB Host support.) from IMX tree. I guess this is a sign that chipidea otg_fsm driver is buggy? I do not think dropping CONFIG_USB_OTG_FSM selection from imx_v6_v7_defconfig is a solution, and we need to fix the issue in usb driver, right? No, Shawn. The solution of this problem is drop CONFIG_USB_OTG_FSM, I guess Denis added it wrongly. According to Embedded Hosts OTG spec, OTG devices and Embedded Hosts both have Targeted Host functionality, and each Targeted Host has its TPL (Targeted Peripheral List), only the devices are at TPL are supported by Targeted Host. Current Linux TPL only contains limited devices, each Targeted Host needs to update its TPL when it makes product, (The comments at otg_whitelist.h also mention it), and the devices are not at TPL will not be supported, and will report to user as an unsupported device error, at current Linux configuration, once the CONFIG_USB_OTG_FSM and CONFIG_USB_OTG are chosen, the host will be Targeted Host. The reason why Shawn met this problem is: the imx_v6_v7_defconfig set CONFIG_USB_OTG_FSM, so the CONFIG_USB_OTG and CONFIG_USB_OTG_WHITELIST are set according to dependency, it makes imx6 device as Targeted Host, but the devices connected at Shawn's board are not at TPL, so they are not be supported. In a word the CONFIG_USB_OTG_FSM should not be chosen for defauly kernel configuration. The CONFIG_USB_OTG_FSM should only be chosen by default when this device goes to make product and the TPL is updated accordingly. Thanks for the explanation, Peter. So this is what I see from USB Kconfig. config USB_OTG_FSM tristate USB 2.0 OTG FSM implementation depends on USB select USB_OTG select USB_PHY config USB_OTG_WHITELIST bool Rely on OTG Targeted Peripherals List depends on USB_OTG || EXPERT default y if USB_OTG I see that USB_OTG_FSM has a dependency on USB_OTG, and USB_OTG_WHITELIST depends on USB_OTG, but USB_OTG does *not* depends on USB_OTG_WHITELIST. I'm not sure why we have 'default y if USB_OTG' for USB_OTG_WHITELIST, when this option only makes sense for product producers per your explanation. So maybe dropping the 'default y' is the right fix? diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig index cb8e991..9081757 100644 --- a/drivers/usb/core/Kconfig +++ b/drivers/usb/core/Kconfig @@ -65,7 +65,6 @@ config USB_OTG config USB_OTG_WHITELIST bool Rely on OTG Targeted Peripherals List depends on USB_OTG || EXPERT - default y if USB_OTG help If you say Y here, the otg_whitelist.h file will be used as a product whitelist, so USB peripherals not listed there will be Shawn As Peter suggested, do not enable OTG_FSM in defconfig since there are very few HNPSRP capable device in market. With OTG_FSM enabled, even USB_OTG_WHITELIST is not selected, the OTG port still can't work the same as before if you connect a normal usb device to it. Li Jun -- 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 04/11] net: cdc_ncm: support rx_max/tx_max updates when running
Finish the rx_max/tx_max setup by flushing buffers and informing usbnet about the changes. This way, the settings can be modified while the netdev is up and running. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 31 +-- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index ad29cde75f41..466922a8af4d 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -82,11 +82,20 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) min, max, val); } + /* usbnet use these values for sizing rx queues */ + dev-rx_urb_size = val; + /* inform device about NTB input size changes */ if (val != ctx-rx_max) { __le32 dwNtbInMaxSize = cpu_to_le32(val); dev_info(dev-intf-dev, setting rx_max = %u\n, val); + + /* need to unlink rx urbs before increasing buffer size */ + if (netif_running(dev-net) dev-rx_urb_size ctx-rx_max) + usbnet_unlink_rx_urbs(dev); + + /* tell device to use new size */ if (usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE, USB_TYPE_CLASS | USB_DIR_OUT | USB_RECIP_INTERFACE, @@ -110,7 +119,6 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) } if (val != ctx-tx_max) dev_info(dev-intf-dev, setting tx_max = %u\n, val); - ctx-tx_max = val; /* Adding a pad byte here if necessary simplifies the handling * in cdc_ncm_fill_tx_frame, making tx_max always represent @@ -119,13 +127,24 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) * We cannot use dev-maxpacket here because this is called from * .bind which is called before usbnet sets up dev-maxpacket */ - if (ctx-tx_max != le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize) - ctx-tx_max % usb_maxpacket(dev-udev, dev-out, 1) == 0) - ctx-tx_max++; + if (val != le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize) + val % usb_maxpacket(dev-udev, dev-out, 1) == 0) + val++; + + /* we might need to flush any pending tx buffers if running */ + if (netif_running(dev-net) val ctx-tx_max) { + netif_tx_lock_bh(dev-net); + usbnet_start_xmit(NULL, dev-net); + ctx-tx_max = val; + netif_tx_unlock_bh(dev-net); + } else { + ctx-tx_max = val; + } - /* usbnet use these values for sizing tx/rx queues */ dev-hard_mtu = ctx-tx_max; - dev-rx_urb_size = ctx-rx_max; + + /* max qlen depend on hard_mtu and rx_urb_size */ + usbnet_update_max_qlen(dev); } /* helpers for NCM and MBIM differences */ -- 2.0.0.rc2 -- 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 11/11] net: cdc_ncm: remove redundant disconnected flag
Calling netif_carrier_{on,off} is sufficient. There is no need to duplicate the carrier state in a driver specific flag. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c| 19 ++- drivers/net/usb/huawei_cdc_ncm.c | 13 - include/linux/usb/cdc_ncm.h | 1 - 3 files changed, 2 insertions(+), 31 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index c65145671521..114d56b9fe29 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -1353,11 +1353,10 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb) * USB_CDC_NOTIFY_NETWORK_CONNECTION notification shall be * sent by device after USB_CDC_NOTIFY_SPEED_CHANGE. */ - ctx-connected = le16_to_cpu(event-wValue); netif_info(dev, link, dev-net, network connection: %sconnected\n, - ctx-connected ? : dis); - usbnet_link_change(dev, ctx-connected, 0); + !!event-wValue ? : dis); + usbnet_link_change(dev, !!event-wValue, 0); break; case USB_CDC_NOTIFY_SPEED_CHANGE: @@ -1377,23 +1376,11 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb) } } -static int cdc_ncm_check_connect(struct usbnet *dev) -{ - struct cdc_ncm_ctx *ctx; - - ctx = (struct cdc_ncm_ctx *)dev-data[0]; - if (ctx == NULL) - return 1; /* disconnected */ - - return !ctx-connected; -} - static const struct driver_info cdc_ncm_info = { .description = CDC NCM, .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET, .bind = cdc_ncm_bind, .unbind = cdc_ncm_unbind, - .check_connect = cdc_ncm_check_connect, .manage_power = usbnet_manage_power, .status = cdc_ncm_status, .rx_fixup = cdc_ncm_rx_fixup, @@ -1407,7 +1394,6 @@ static const struct driver_info wwan_info = { | FLAG_WWAN, .bind = cdc_ncm_bind, .unbind = cdc_ncm_unbind, - .check_connect = cdc_ncm_check_connect, .manage_power = usbnet_manage_power, .status = cdc_ncm_status, .rx_fixup = cdc_ncm_rx_fixup, @@ -1421,7 +1407,6 @@ static const struct driver_info wwan_noarp_info = { | FLAG_WWAN | FLAG_NOARP, .bind = cdc_ncm_bind, .unbind = cdc_ncm_unbind, - .check_connect = cdc_ncm_check_connect, .manage_power = usbnet_manage_power, .status = cdc_ncm_status, .rx_fixup = cdc_ncm_rx_fixup, diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c index 312178d7b698..f9822bc75425 100644 --- a/drivers/net/usb/huawei_cdc_ncm.c +++ b/drivers/net/usb/huawei_cdc_ncm.c @@ -172,24 +172,11 @@ err: return ret; } -static int huawei_cdc_ncm_check_connect(struct usbnet *usbnet_dev) -{ - struct cdc_ncm_ctx *ctx; - - ctx = (struct cdc_ncm_ctx *)usbnet_dev-data[0]; - - if (ctx == NULL) - return 1; /* disconnected */ - - return !ctx-connected; -} - static const struct driver_info huawei_cdc_ncm_info = { .description = Huawei CDC NCM device, .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN, .bind = huawei_cdc_ncm_bind, .unbind = huawei_cdc_ncm_unbind, - .check_connect = huawei_cdc_ncm_check_connect, .manage_power = huawei_cdc_ncm_manage_power, .rx_fixup = cdc_ncm_rx_fixup, .tx_fixup = cdc_ncm_tx_fixup, diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h index 1921bdadd9ab..dc175b1debdf 100644 --- a/include/linux/usb/cdc_ncm.h +++ b/include/linux/usb/cdc_ncm.h @@ -118,7 +118,6 @@ struct cdc_ncm_ctx { u16 tx_ndp_modulus; u16 tx_seq; u16 rx_seq; - u16 connected; u16 min_tx_pkt; /* statistics */ -- 2.0.0.rc2 -- 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 08/11] net: cdc_ncm/cdc_mbim: adding NCM protocol statiscics
To have an idea of the effects of the protocol coalescing it's useful to have some counters showing the different aspects. Due to the assymetrical usbnet interface the netdev rx_bytes counter has been counting real received payload, while the tx_bytes counter has included the NCM/MBIM framing overhead. This overhead can be many times the payload because of the aggressive padding strategy of this driver, and will vary a lot depending on device and traffic. With very few exceptions, users are only interested in the payload size. Having an somewhat accurate payload byte counter is particularily important for modbile broadband devices, which many NCM devices and of course all MBIM devices are. Users and userspace applications will use this counter to monitor account quotas. Having protocol specific counters for the overhead, we are now able to correct the tx_bytes netdev counter so that it shows the real payload Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_mbim.c | 6 +++ drivers/net/usb/cdc_ncm.c | 91 + include/linux/usb/cdc_ncm.h | 11 ++ 3 files changed, 108 insertions(+) diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c index c9f3281506af..9008e8946a50 100644 --- a/drivers/net/usb/cdc_mbim.c +++ b/drivers/net/usb/cdc_mbim.c @@ -295,6 +295,7 @@ static int cdc_mbim_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in) struct usb_cdc_ncm_dpe16 *dpe16; int ndpoffset; int loopcount = 50; /* arbitrary max preventing infinite loop */ + u32 payload = 0; u8 *c; u16 tci; @@ -354,6 +355,7 @@ next_ndp: if (!skb) goto error; usbnet_skb_return(dev, skb); + payload += len; /* count payload bytes in this NTB */ } } err_ndp: @@ -362,6 +364,10 @@ err_ndp: if (ndpoffset loopcount--) goto next_ndp; + /* update stats */ + ctx-rx_overhead += skb_in-len - payload; + ctx-rx_ntbs++; + return 1; error: return 0; diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index e28b530bff3a..a28c964b35a9 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -65,6 +65,68 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx); static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer); static struct usb_driver cdc_ncm_driver; +struct cdc_ncm_stats { + char stat_string[ETH_GSTRING_LEN]; + int sizeof_stat; + int stat_offset; +}; + +#define CDC_NCM_STAT(str, m) { \ + .stat_string = str, \ + .sizeof_stat = sizeof(((struct cdc_ncm_ctx *)0)-m), \ + .stat_offset = offsetof(struct cdc_ncm_ctx, m) } +#define CDC_NCM_SIMPLE_STAT(m) CDC_NCM_STAT(__stringify(m), m) + +static const struct cdc_ncm_stats cdc_ncm_gstrings_stats[] = { + CDC_NCM_SIMPLE_STAT(tx_reason_ntb_full), + CDC_NCM_SIMPLE_STAT(tx_reason_ndp_full), + CDC_NCM_SIMPLE_STAT(tx_reason_timeout), + CDC_NCM_SIMPLE_STAT(tx_reason_max_datagram), + CDC_NCM_SIMPLE_STAT(tx_overhead), + CDC_NCM_SIMPLE_STAT(tx_ntbs), + CDC_NCM_SIMPLE_STAT(rx_overhead), + CDC_NCM_SIMPLE_STAT(rx_ntbs), +}; + +static int cdc_ncm_get_sset_count(struct net_device __always_unused *netdev, int sset) +{ + switch (sset) { + case ETH_SS_STATS: + return ARRAY_SIZE(cdc_ncm_gstrings_stats); + default: + return -EOPNOTSUPP; + } +} + +static void cdc_ncm_get_ethtool_stats(struct net_device *netdev, + struct ethtool_stats __always_unused *stats, + u64 *data) +{ + struct usbnet *dev = netdev_priv(netdev); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; + int i; + char *p = NULL; + + for (i = 0; i ARRAY_SIZE(cdc_ncm_gstrings_stats); i++) { + p = (char *)ctx + cdc_ncm_gstrings_stats[i].stat_offset; + data[i] = (cdc_ncm_gstrings_stats[i].sizeof_stat == sizeof(u64)) ? *(u64 *)p : *(u32 *)p; + } +} + +static void cdc_ncm_get_strings(struct net_device __always_unused *netdev, u32 stringset, u8 *data) +{ + u8 *p = data; + int i; + + switch (stringset) { + case ETH_SS_STATS: + for (i = 0; i ARRAY_SIZE(cdc_ncm_gstrings_stats); i++) { + memcpy(p, cdc_ncm_gstrings_stats[i].stat_string, ETH_GSTRING_LEN); + p += ETH_GSTRING_LEN; + } + } +} + static int cdc_ncm_get_coalesce(struct net_device *netdev, struct ethtool_coalesce *ec) { @@ -117,6 +179,9 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = { .get_msglevel = usbnet_get_msglevel, .set_msglevel = usbnet_set_msglevel,
[PATCH net-next 03/11] net: cdc_ncm: split .bind device initialization
Now that we have split out the part of the device setup which MUST be done with the data interface in altsetting 0, we can delay the rest of the initialization. This allows us to move some of post-init buffer size config from bind to the appropriate setup function. The purpose of this refactoring is to collect all code adjusting the rx_max and tx_max buffers in one place, so that it is easier to call it from multiple call sites. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index d2e9b56c27ff..ad29cde75f41 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -111,6 +111,21 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) if (val != ctx-tx_max) dev_info(dev-intf-dev, setting tx_max = %u\n, val); ctx-tx_max = val; + + /* Adding a pad byte here if necessary simplifies the handling +* in cdc_ncm_fill_tx_frame, making tx_max always represent +* the real skb max size. +* +* We cannot use dev-maxpacket here because this is called from +* .bind which is called before usbnet sets up dev-maxpacket +*/ + if (ctx-tx_max != le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize) + ctx-tx_max % usb_maxpacket(dev-udev, dev-out, 1) == 0) + ctx-tx_max++; + + /* usbnet use these values for sizing tx/rx queues */ + dev-hard_mtu = ctx-tx_max; + dev-rx_urb_size = ctx-rx_max; } /* helpers for NCM and MBIM differences */ @@ -316,9 +331,6 @@ static int cdc_ncm_setup(struct usbnet *dev) { struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; - /* initialize basic device settings */ - cdc_ncm_init(dev); - /* clamp rx_max and tx_max and inform device */ cdc_ncm_update_rxtx_max(dev, le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize), le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize)); @@ -525,8 +537,8 @@ advance: goto error2; } - /* initialize data interface */ - if (cdc_ncm_setup(dev)) + /* initialize basic device settings */ + if (cdc_ncm_init(dev)) goto error2; /* configure data interface */ @@ -555,18 +567,8 @@ advance: dev_info(intf-dev, MAC-Address: %pM\n, dev-net-dev_addr); } - /* usbnet use these values for sizing tx/rx queues */ - dev-hard_mtu = ctx-tx_max; - dev-rx_urb_size = ctx-rx_max; - - /* cdc_ncm_setup will override dwNtbOutMaxSize if it is -* outside the sane range. Adding a pad byte here if necessary -* simplifies the handling in cdc_ncm_fill_tx_frame, making -* tx_max always represent the real skb max size. -*/ - if (ctx-tx_max != le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize) - ctx-tx_max % usb_maxpacket(dev-udev, dev-out, 1) == 0) - ctx-tx_max++; + /* finish setting up the device specific data */ + cdc_ncm_setup(dev); return 0; -- 2.0.0.rc2 -- 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 01/11] net: cdc_ncm: split out rx_max/tx_max update of setup
Split out the part of setup dealing with updating the rx_max and tx_max buffer sizes so that this code can be reused for dynamically updating the limits. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 81 +-- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 549dbac710ed..87a32edf7ea5 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -65,6 +65,54 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx); static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer); static struct usb_driver cdc_ncm_driver; +/* handle rx_max and tx_max changes */ +static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) +{ + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; + u8 iface_no = ctx-control-cur_altsetting-desc.bInterfaceNumber; + u32 val, max, min; + + /* clamp new_rx to sane values */ + min = min_t(u32, USB_CDC_NCM_NTB_MIN_IN_SIZE, le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize)); + max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_RX, le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize)); + + val = clamp_t(u32, new_rx, min, max); + if (val != new_rx) { + dev_dbg(dev-intf-dev, rx_max must be in the [%u, %u] range. Using %u\n, + min, max, val); + } + + /* inform device about NTB input size changes */ + if (val != ctx-rx_max) { + __le32 dwNtbInMaxSize = cpu_to_le32(val); + + dev_info(dev-intf-dev, setting rx_max = %u\n, val); + if (usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE, +USB_TYPE_CLASS | USB_DIR_OUT +| USB_RECIP_INTERFACE, +0, iface_no, dwNtbInMaxSize, 4) 0) + dev_dbg(dev-intf-dev, Setting NTB Input Size failed\n); + else + ctx-rx_max = val; + } + + /* clamp new_tx to sane values */ + min = CDC_NCM_MIN_HDR_SIZE + ctx-max_datagram_size; + max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize)); + + /* some devices set dwNtbOutMaxSize too low for the above default */ + min = min(min, max); + + val = clamp_t(u32, new_tx, min, max); + if (val != new_tx) { + dev_dbg(dev-intf-dev, tx_max must be in the [%u, %u] range. Using %u\n, + min, max, val); + } + if (val != ctx-tx_max) + dev_info(dev-intf-dev, setting tx_max = %u\n, val); + ctx-tx_max = val; +} + static int cdc_ncm_setup(struct usbnet *dev) { struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; @@ -132,37 +180,8 @@ static int cdc_ncm_setup(struct usbnet *dev) (ctx-tx_max_datagrams CDC_NCM_DPT_DATAGRAMS_MAX)) ctx-tx_max_datagrams = CDC_NCM_DPT_DATAGRAMS_MAX; - /* verify maximum size of received NTB in bytes */ - if (ctx-rx_max USB_CDC_NCM_NTB_MIN_IN_SIZE) { - dev_dbg(dev-intf-dev, Using min receive length=%d\n, - USB_CDC_NCM_NTB_MIN_IN_SIZE); - ctx-rx_max = USB_CDC_NCM_NTB_MIN_IN_SIZE; - } - - if (ctx-rx_max CDC_NCM_NTB_MAX_SIZE_RX) { - dev_dbg(dev-intf-dev, Using default maximum receive length=%d\n, - CDC_NCM_NTB_MAX_SIZE_RX); - ctx-rx_max = CDC_NCM_NTB_MAX_SIZE_RX; - } - - /* inform device about NTB input size changes */ - if (ctx-rx_max != le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize)) { - __le32 dwNtbInMaxSize = cpu_to_le32(ctx-rx_max); - - err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE, - USB_TYPE_CLASS | USB_DIR_OUT - | USB_RECIP_INTERFACE, - 0, iface_no, dwNtbInMaxSize, 4); - if (err 0) - dev_dbg(dev-intf-dev, Setting NTB Input Size failed\n); - } - - /* verify maximum size of transmitted NTB in bytes */ - if (ctx-tx_max CDC_NCM_NTB_MAX_SIZE_TX) { - dev_dbg(dev-intf-dev, Using default maximum transmit length=%d\n, - CDC_NCM_NTB_MAX_SIZE_TX); - ctx-tx_max = CDC_NCM_NTB_MAX_SIZE_TX; - } + /* clamp rx_max and tx_max and inform device */ + cdc_ncm_update_rxtx_max(dev, le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize), le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize)); /* * verify that the structure alignment is: -- 2.0.0.rc2 -- 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 07/11] net: cdc_ncm: set reasonable padding limits
We pad frames larger than X to maximum size for devices which don't need a ZLP after maximum sized frames. This allows the device to optimize its transfers for one fixed buffer size. X was arbitrarily set at 512 bytes regardless of real buffer maximum, causing extreme overheads due to excessive padding of larger tx buffers. Limit the padding to at most 3 full USB packets, still allowing the overhead to payload ratio of 3/1. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 8 ++-- include/linux/usb/cdc_ncm.h | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index faa494c0377e..e28b530bff3a 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -201,6 +201,10 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) /* max qlen depend on hard_mtu and rx_urb_size */ usbnet_update_max_qlen(dev); + + /* never pad more than 3 full USB packets per transfer */ + ctx-min_tx_pkt = clamp_t(u16, ctx-tx_max - 3 * usb_maxpacket(dev-udev, dev-out, 1), + CDC_NCM_MIN_TX_PKT, ctx-tx_max); } /* helpers for NCM and MBIM differences */ @@ -936,7 +940,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign) /* variables will be reset at next call */ } - /* If collected data size is less or equal CDC_NCM_MIN_TX_PKT + /* If collected data size is less or equal ctx-min_tx_pkt * bytes, we send buffers as it is. If we get more data, it * would be more efficient for USB HS mobile device with DMA * engine to receive a full size NTB, than canceling DMA @@ -946,7 +950,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign) * a ZLP after full sized NTBs. */ if (!(dev-driver_info-flags FLAG_SEND_ZLP) - skb_out-len CDC_NCM_MIN_TX_PKT) + skb_out-len ctx-min_tx_pkt) memset(skb_put(skb_out, ctx-tx_max - skb_out-len), 0, ctx-tx_max - skb_out-len); else if ((skb_out-len % dev-maxpacket) == 0) diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h index bbc4ce9ffef5..0aac70ee23b6 100644 --- a/include/linux/usb/cdc_ncm.h +++ b/include/linux/usb/cdc_ncm.h @@ -115,6 +115,7 @@ struct cdc_ncm_ctx { u16 tx_seq; u16 rx_seq; u16 connected; + u16 min_tx_pkt; }; u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf); -- 2.0.0.rc2 -- 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 10/11] net: cdc_ncm: fix argument alignment
Reported-by: Joe Perches j...@perches.com Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 7d315b983e7e..c65145671521 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -1316,14 +1316,14 @@ cdc_ncm_speed_change(struct usbnet *dev, */ if ((tx_speed 100) (rx_speed 100)) { netif_info(dev, link, dev-net, - %u mbit/s downlink %u mbit/s uplink\n, - (unsigned int)(rx_speed / 100U), - (unsigned int)(tx_speed / 100U)); + %u mbit/s downlink %u mbit/s uplink\n, + (unsigned int)(rx_speed / 100U), + (unsigned int)(tx_speed / 100U)); } else { netif_info(dev, link, dev-net, - %u kbit/s downlink %u kbit/s uplink\n, - (unsigned int)(rx_speed / 1000U), - (unsigned int)(tx_speed / 1000U)); + %u kbit/s downlink %u kbit/s uplink\n, + (unsigned int)(rx_speed / 1000U), + (unsigned int)(tx_speed / 1000U)); } } -- 2.0.0.rc2 -- 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 09/11] net: cdc_ncm: use sane defaults for rx/tx buffers
Lots of devices request much larger buffers than reasonable. This cause real problems for users of hosts with limited resources. Reducing the default buffer size to 16kB for such devices is a reasonable trade-off between allowing them to aggregate traffic and avoiding memory exhaustion on resource restrained hosts. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 12 ++-- include/linux/usb/cdc_ncm.h | 4 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index a28c964b35a9..7d315b983e7e 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -480,10 +480,18 @@ static void cdc_ncm_fix_modulus(struct usbnet *dev) static int cdc_ncm_setup(struct usbnet *dev) { struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; + u32 def_rx, def_tx; + + /* be conservative when selecting intial buffer size to +* increase the number of hosts this will work for +*/ + def_rx = min_t(u32, CDC_NCM_NTB_DEF_SIZE_RX, + le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize)); + def_tx = min_t(u32, CDC_NCM_NTB_DEF_SIZE_TX, + le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize)); /* clamp rx_max and tx_max and inform device */ - cdc_ncm_update_rxtx_max(dev, le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize), - le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize)); + cdc_ncm_update_rxtx_max(dev, def_rx, def_tx); /* sanitize the modulus and remainder values */ cdc_ncm_fix_modulus(dev); diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h index 4eeaf6db20c8..1921bdadd9ab 100644 --- a/include/linux/usb/cdc_ncm.h +++ b/include/linux/usb/cdc_ncm.h @@ -52,6 +52,10 @@ #defineCDC_NCM_NTB_MAX_SIZE_TX 32768 /* bytes */ #defineCDC_NCM_NTB_MAX_SIZE_RX 32768 /* bytes */ +/* Initial NTB length */ +#defineCDC_NCM_NTB_DEF_SIZE_TX 16384 /* bytes */ +#defineCDC_NCM_NTB_DEF_SIZE_RX 16384 /* bytes */ + /* Minimum value for MaxDatagramSize, ch. 6.2.9 */ #defineCDC_NCM_MIN_DATAGRAM_SIZE 1514/* bytes */ -- 2.0.0.rc2 -- 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 00/11] cdc_ncm: add buffer tuning and stats using ethtool
The first 9 patches of this set are unchanged from the RFC I sent more than a week ago. The 2 last patches are leftover minor cleanups, which I found laying around from last years cleaning job. David Laight was the only one commenting on the RFC, and I have interpreted his comment as a wish for further work on the usbnet framework, and not a request for any changes to this patch set. Hence this submission of without changes to patch 1-9, which I described like this for the RFC: I have got quite a few reports from frustrated users of OpenWRT hosts trying to use some powerful LTE modem, but not achieving full speed. This is typically caused by a combination of big buffers and little memory, giving in allocation errors and bad performance as a result. This series is an attempt to let users adjust the size of these buffers without having to rebuild the driver. Patches 1 - 4 are mostly rearranging existing code, in preparing for the dynamic buffer size changes. Patch 5 adds userspace control (ab)using the ethtool coalescing API. This isn't a perfect match, which is the main reason why I post this series as a RFC. Patch 6 is an unrelated framing optimization, reducing the overhead quite a bit and allowing for better use of smaller buffers. Patch 7 changes the way we calculate frame padding cutoff. The problem with big buffers is made much worse by the current padding strategy where zero padding often can account for more than 90% of the frames. Patch 8 add some counters giving some insight into how well the NCM/MBIM protocol works, supporting further tuning. Patch 9 reduce the initial maximum buffer size from 32kB to 16kB in an attempt to make the default better suit all. It is still possible to tune this up again to the old fixed max, using the new tuning knobs. I must admit that I had higher hopes for this series before I tested it on my own modems. One really unexpected result was that one of the MBIM modems accepted the new rx buffer size we set, but happily continued sending buffers of the same size as before. Needless to say: This did not work very well... So don't really expect to be able to use any values with any given device. Firmware implementations are still... I don't think I have words suitable for a public mailing list. But I am hoping this will help the many users who have had success rebuilding the driver with lower fixed limits. Please test and/or comment! And a short description of the 2 new patches since the RFC: Patch 10 is a follow-up to a comment Joe Perches made in November 2013. I don't always forget :-) Patch 11 removes the redundant connected driver state, and the associated .check_connect callbacks. Bjørn Mork (11): net: cdc_ncm: split out rx_max/tx_max update of setup net: cdc_ncm: factor out one-time device initialization net: cdc_ncm: split .bind device initialization net: cdc_ncm: support rx_max/tx_max updates when running net: cdc_ncm: use ethtool to tune coalescing settings net: cdc_ncm: use true max dgram count for header estimates net: cdc_ncm: set reasonable padding limits net: cdc_ncm/cdc_mbim: adding NCM protocol statiscics net: cdc_ncm: use sane defaults for rx/tx buffers net: cdc_ncm: fix argument alignment net: cdc_ncm: remove redundant disconnected flag drivers/net/usb/cdc_mbim.c | 6 + drivers/net/usb/cdc_ncm.c| 576 --- drivers/net/usb/huawei_cdc_ncm.c | 13 - include/linux/usb/cdc_ncm.h | 33 ++- 4 files changed, 442 insertions(+), 186 deletions(-) -- 2.0.0.rc2 -- 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 02/11] net: cdc_ncm: factor out one-time device initialization
Split the parts of setup dealing with device initialization from parts just setting defaults for attributes which might be changed after initialization. Some commands of the device initialization are only allowed when the data interface is in its disabled altsetting, so we must separate them out of we are to allow rerunning parts of setup. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 251 -- 1 file changed, 155 insertions(+), 96 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 87a32edf7ea5..d2e9b56c27ff 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -113,19 +113,51 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) ctx-tx_max = val; } -static int cdc_ncm_setup(struct usbnet *dev) +/* helpers for NCM and MBIM differences */ +static u8 cdc_ncm_flags(struct usbnet *dev) { struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; - u32 val; - u8 flags; - u8 iface_no; - int err; - int eth_hlen; - u16 mbim_mtu; - u16 ntb_fmt_supported; - __le16 max_datagram_size; - iface_no = ctx-control-cur_altsetting-desc.bInterfaceNumber; + if (cdc_ncm_comm_intf_is_mbim(dev-intf-cur_altsetting) ctx-mbim_desc) + return ctx-mbim_desc-bmNetworkCapabilities; + if (ctx-func_desc) + return ctx-func_desc-bmNetworkCapabilities; + return 0; +} + +static int cdc_ncm_eth_hlen(struct usbnet *dev) +{ + if (cdc_ncm_comm_intf_is_mbim(dev-intf-cur_altsetting)) + return 0; + return ETH_HLEN; +} + +static u32 cdc_ncm_min_dgram_size(struct usbnet *dev) +{ + if (cdc_ncm_comm_intf_is_mbim(dev-intf-cur_altsetting)) + return CDC_MBIM_MIN_DATAGRAM_SIZE; + return CDC_NCM_MIN_DATAGRAM_SIZE; +} + +static u32 cdc_ncm_max_dgram_size(struct usbnet *dev) +{ + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; + + if (cdc_ncm_comm_intf_is_mbim(dev-intf-cur_altsetting) ctx-mbim_desc) + return le16_to_cpu(ctx-mbim_desc-wMaxSegmentSize); + if (ctx-ether_desc) + return le16_to_cpu(ctx-ether_desc-wMaxSegmentSize); + return CDC_NCM_MAX_DATAGRAM_SIZE; +} + +/* initial one-time device setup. MUST be called with the data interface + * in altsetting 0 + */ +static int cdc_ncm_init(struct usbnet *dev) +{ + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; + u8 iface_no = ctx-control-cur_altsetting-desc.bInterfaceNumber; + int err; err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS, USB_TYPE_CLASS | USB_DIR_IN @@ -137,7 +169,35 @@ static int cdc_ncm_setup(struct usbnet *dev) return err; /* GET_NTB_PARAMETERS is required */ } - /* read correct set of parameters according to device mode */ + /* set CRC Mode */ + if (cdc_ncm_flags(dev) USB_CDC_NCM_NCAP_CRC_MODE) { + dev_dbg(dev-intf-dev, Setting CRC mode off\n); + err = usbnet_write_cmd(dev, USB_CDC_SET_CRC_MODE, + USB_TYPE_CLASS | USB_DIR_OUT + | USB_RECIP_INTERFACE, + USB_CDC_NCM_CRC_NOT_APPENDED, + iface_no, NULL, 0); + if (err 0) + dev_err(dev-intf-dev, SET_CRC_MODE failed\n); + } + + /* set NTB format, if both formats are supported. +* +* The host shall only send this command while the NCM Data +* Interface is in alternate setting 0. +*/ + if (le16_to_cpu(ctx-ncm_parm.bmNtbFormatsSupported) USB_CDC_NCM_NTH32_SIGN) { + dev_dbg(dev-intf-dev, Setting NTB format to 16-bit\n); + err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_FORMAT, + USB_TYPE_CLASS | USB_DIR_OUT + | USB_RECIP_INTERFACE, + USB_CDC_NCM_NTB16_FORMAT, + iface_no, NULL, 0); + if (err 0) + dev_err(dev-intf-dev, SET_NTB_FORMAT failed\n); + } + + /* set initial device values */ ctx-rx_max = le32_to_cpu(ctx-ncm_parm.dwNtbInMaxSize); ctx-tx_max = le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize); ctx-tx_remainder = le16_to_cpu(ctx-ncm_parm.wNdpOutPayloadRemainder); @@ -145,43 +205,73 @@ static int cdc_ncm_setup(struct usbnet *dev) ctx-tx_ndp_modulus = le16_to_cpu(ctx-ncm_parm.wNdpOutAlignment); /* devices prior to NCM Errata shall set this field to zero */ ctx-tx_max_datagrams = le16_to_cpu(ctx-ncm_parm.wNtbOutMaxDatagrams); - ntb_fmt_supported = le16_to_cpu(ctx-ncm_parm.bmNtbFormatsSupported); - - /* there are
[PATCH net-next 05/11] net: cdc_ncm: use ethtool to tune coalescing settings
Datagram coalescing is an integral part of the NCM and MBIM protocols, intended to reduce the interrupt load primarily on the device end of the USB link. As with all coalescing solutions, there is a trade-off between buffering and interrupts. The current defaults are based on the assumption that device side buffers should be the limiting factor. However, many modern high speed LTE modems suffers from buffer-bloat, making this assumption fail. This results in suboptimal performance due to excessive coalescing. And in cases where such modems are connected to cheap embedded hosts there is often severe buffer allocation issues, giving very noticable performance degredation . A start on improving this is going from build time hard coded limits to per device user configurable limits. The ethtool coalescing API was selected as user interface because, although the tuned values are buffer sizes, these settings directly control datagram coalescing. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 68 +++-- include/linux/usb/cdc_ncm.h | 6 +++- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 466922a8af4d..b20c82c19e02 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -65,6 +65,62 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx); static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer); static struct usb_driver cdc_ncm_driver; +static int cdc_ncm_get_coalesce(struct net_device *netdev, + struct ethtool_coalesce *ec) +{ + struct usbnet *dev = netdev_priv(netdev); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; + + /* assuming maximum sized dgrams and ignoring NDPs */ + ec-rx_max_coalesced_frames = ctx-rx_max / ctx-max_datagram_size; + ec-tx_max_coalesced_frames = ctx-tx_max / ctx-max_datagram_size; + + /* the timer will fire CDC_NCM_TIMER_PENDING_CNT times in a row */ + ec-tx_coalesce_usecs = (ctx-timer_interval * CDC_NCM_TIMER_PENDING_CNT) / NSEC_PER_USEC; + return 0; +} + +static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx); + +static int cdc_ncm_set_coalesce(struct net_device *netdev, + struct ethtool_coalesce *ec) +{ + struct usbnet *dev = netdev_priv(netdev); + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0]; + u32 new_rx_max = ctx-rx_max; + u32 new_tx_max = ctx-tx_max; + + /* assuming maximum sized dgrams and a single NDP */ + if (ec-rx_max_coalesced_frames) + new_rx_max = ec-rx_max_coalesced_frames * ctx-max_datagram_size; + if (ec-tx_max_coalesced_frames) + new_tx_max = ec-tx_max_coalesced_frames * ctx-max_datagram_size; + + if (ec-tx_coalesce_usecs + (ec-tx_coalesce_usecs CDC_NCM_TIMER_INTERVAL_MIN * CDC_NCM_TIMER_PENDING_CNT || +ec-tx_coalesce_usecs CDC_NCM_TIMER_INTERVAL_MAX * CDC_NCM_TIMER_PENDING_CNT)) + return -EINVAL; + ctx-timer_interval = ec-tx_coalesce_usecs * NSEC_PER_USEC / CDC_NCM_TIMER_PENDING_CNT; + + /* inform device of new values */ + if (new_rx_max != ctx-rx_max || new_tx_max != ctx-tx_max) + cdc_ncm_update_rxtx_max(dev, new_rx_max, new_tx_max); + return 0; +} + +static const struct ethtool_ops cdc_ncm_ethtool_ops = { + .get_settings = usbnet_get_settings, + .set_settings = usbnet_set_settings, + .get_link = usbnet_get_link, + .nway_reset= usbnet_nway_reset, + .get_drvinfo = usbnet_get_drvinfo, + .get_msglevel = usbnet_get_msglevel, + .set_msglevel = usbnet_set_msglevel, + .get_ts_info = ethtool_op_get_ts_info, + .get_coalesce = cdc_ncm_get_coalesce, + .set_coalesce = cdc_ncm_set_coalesce, +}; + /* handle rx_max and tx_max changes */ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) { @@ -250,6 +306,9 @@ static int cdc_ncm_init(struct usbnet *dev) (ctx-tx_max_datagrams CDC_NCM_DPT_DATAGRAMS_MAX)) ctx-tx_max_datagrams = CDC_NCM_DPT_DATAGRAMS_MAX; + /* initial coalescing timer interval */ + ctx-timer_interval = CDC_NCM_TIMER_INTERVAL_USEC * NSEC_PER_USEC; + return 0; } @@ -589,6 +648,9 @@ advance: /* finish setting up the device specific data */ cdc_ncm_setup(dev); + /* override ethtool_ops */ + dev-net-ethtool_ops = cdc_ncm_ethtool_ops; + return 0; error2: @@ -857,7 +919,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign) ctx-tx_curr_skb = skb_out; goto exit_no_skb; - } else if ((n ctx-tx_max_datagrams) (ready2send == 0)) { + } else if
[PATCH net-next 06/11] net: cdc_ncm: use true max dgram count for header estimates
Many newer NCM and MBIM devices will request a maximum tx datagram count which is much smaller than our hardcoded absolute max. We can reduce the overhead without sacrificing any of the simplicity for these devices, by simply using the true negotiated count in when calculated the maximum NTH and NDP header sizes. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_ncm.c | 9 ++--- include/linux/usb/cdc_ncm.h | 10 +- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index b20c82c19e02..faa494c0377e 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -162,7 +162,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx) } /* clamp new_tx to sane values */ - min = CDC_NCM_MIN_HDR_SIZE + ctx-max_datagram_size; + min = ctx-max_datagram_size + ctx-max_ndp_size + sizeof(struct usb_cdc_ncm_nth16); max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx-ncm_parm.dwNtbOutMaxSize)); /* some devices set dwNtbOutMaxSize too low for the above default */ @@ -306,6 +306,9 @@ static int cdc_ncm_init(struct usbnet *dev) (ctx-tx_max_datagrams CDC_NCM_DPT_DATAGRAMS_MAX)) ctx-tx_max_datagrams = CDC_NCM_DPT_DATAGRAMS_MAX; + /* set up maximum NDP size */ + ctx-max_ndp_size = sizeof(struct usb_cdc_ncm_ndp16) + (ctx-tx_max_datagrams + 1) * sizeof(struct usb_cdc_ncm_dpe16); + /* initial coalescing timer interval */ ctx-timer_interval = CDC_NCM_TIMER_INTERVAL_USEC * NSEC_PER_USEC; @@ -789,7 +792,7 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_ cdc_ncm_align_tail(skb, ctx-tx_ndp_modulus, 0, ctx-tx_max); /* verify that there is room for the NDP and the datagram (reserve) */ - if ((ctx-tx_max - skb-len - reserve) CDC_NCM_NDP_SIZE) + if ((ctx-tx_max - skb-len - reserve) ctx-max_ndp_size) return NULL; /* link to it */ @@ -799,7 +802,7 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_ nth16-wNdpIndex = cpu_to_le16(skb-len); /* push a new empty NDP */ - ndp16 = (struct usb_cdc_ncm_ndp16 *)memset(skb_put(skb, CDC_NCM_NDP_SIZE), 0, CDC_NCM_NDP_SIZE); + ndp16 = (struct usb_cdc_ncm_ndp16 *)memset(skb_put(skb, ctx-max_ndp_size), 0, ctx-max_ndp_size); ndp16-dwSignature = sign; ndp16-wLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_ndp16) + sizeof(struct usb_cdc_ncm_dpe16)); return ndp16; diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h index 07ad8c3284a6..bbc4ce9ffef5 100644 --- a/include/linux/usb/cdc_ncm.h +++ b/include/linux/usb/cdc_ncm.h @@ -76,15 +76,6 @@ #define CDC_NCM_TIMER_INTERVAL_MIN 5UL #define CDC_NCM_TIMER_INTERVAL_MAX (15UL * USEC_PER_SEC) -/* The following macro defines the minimum header space */ -#defineCDC_NCM_MIN_HDR_SIZE \ - (sizeof(struct usb_cdc_ncm_nth16) + sizeof(struct usb_cdc_ncm_ndp16) + \ - (CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16)) - -#define CDC_NCM_NDP_SIZE \ - (sizeof(struct usb_cdc_ncm_ndp16) + \ - (CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16)) - #define cdc_ncm_comm_intf_is_mbim(x) ((x)-desc.bInterfaceSubClass == USB_CDC_SUBCLASS_MBIM \ (x)-desc.bInterfaceProtocol == USB_CDC_PROTO_NONE) #define cdc_ncm_data_intf_is_mbim(x) ((x)-desc.bInterfaceProtocol == USB_CDC_MBIM_PROTO_NTB) @@ -110,6 +101,7 @@ struct cdc_ncm_ctx { atomic_t stop; u64 timer_interval; + u32 max_ndp_size; u32 tx_timer_pending; u32 tx_curr_frame_num; -- 2.0.0.rc2 -- 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-linus] usb: cdc-wdm: export cdc-wdm uapi header
On Sat, May 10, 2014 at 04:31:40PM +0200, Bjørn Mork wrote: The include/uapi/linux/usb/cdc-wdm.h header defines cdc-wdm userspace APIs and should be exported by make headers_install. Cc: sta...@vger.kernel.org # 3.10, 3.12, 3.14 Fixes: 3edce1cf813a (USB: cdc-wdm: implement IOCTL_WDM_MAX_COMMAND) Signed-off-by: Bjørn Mork bj...@mork.no --- Seems like I accidentally left this out when adding the uapi file, and didn't notice until I was trying to use it today... I guess no one uses that ioctl :) I'll queue this up for 3.15-final, thanks. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Weird I/O errors with USB Hard disk
hi James: 2014-05-10 23:10 GMT+08:00 James Bottomley james.bottom...@hansenpartnership.com: On Sat, 2014-05-10 at 22:38 +0800, loody wrote: hi all: I have a USB hard disk and when I play specific file. it will show below message ( I purpose enable usb/storage/transport.c debug message about urb debug) what makes me confused are 1. Does Unhandled sense code mean the SCSI Request Sense command? Error handling couldn't correct the error so it was passed up. 2. if #1 is correct, there should be any error handling for not keep sending following commands. Why usb driver keep clear this halt enpoint? isn't there any timeout mechanism no matter in scsi layer or usb layer? appreciate your help [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]sd 0:0:0:0: [sda] Unhandled sense code [22:06:48]sd 0:0:0:0: [sda] Result: hostbyte=0x10 driverbyte=0x08 [22:06:48]sd 0:0:0:0: [sda] Sense Key : 0x3 [current] [22:06:48]sd 0:0:0:0: [sda] ASC=0x11 ASCQ=0x0 [22:06:48]sd 0:0:0:0: [sda] CDB: cdb[0]=0x28: 28 00 00 24 42 22 00 00 f0 00 [22:06:48]end_request: critical target error, dev sda, sector 2376226 If you compiled your kernel with SCSI messages it would tell you that this is a medium error, unrecovered read error in sector 2376226 The device is failing and the sector cannot be recovered. in my opinion, the OS will not hang up when this problem happen, right? But my platform is always trying to clear halt endpoint, and it seems Block or File system layer still try to access this sector. is that related to file node open method or file read method? such that read bad sector will hang up the OS. -- Appreciate your help, -- 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: Weird I/O errors with USB Hard disk
On Sun, 2014-05-11 at 00:43 +0800, loody wrote: hi James: 2014-05-10 23:10 GMT+08:00 James Bottomley james.bottom...@hansenpartnership.com: On Sat, 2014-05-10 at 22:38 +0800, loody wrote: hi all: I have a USB hard disk and when I play specific file. it will show below message ( I purpose enable usb/storage/transport.c debug message about urb debug) what makes me confused are 1. Does Unhandled sense code mean the SCSI Request Sense command? Error handling couldn't correct the error so it was passed up. 2. if #1 is correct, there should be any error handling for not keep sending following commands. Why usb driver keep clear this halt enpoint? isn't there any timeout mechanism no matter in scsi layer or usb layer? appreciate your help [22:06:48]clearing endpoint halt for pipe 0xc0008280 [22:06:48]sd 0:0:0:0: [sda] Unhandled sense code [22:06:48]sd 0:0:0:0: [sda] Result: hostbyte=0x10 driverbyte=0x08 [22:06:48]sd 0:0:0:0: [sda] Sense Key : 0x3 [current] [22:06:48]sd 0:0:0:0: [sda] ASC=0x11 ASCQ=0x0 [22:06:48]sd 0:0:0:0: [sda] CDB: cdb[0]=0x28: 28 00 00 24 42 22 00 00 f0 00 [22:06:48]end_request: critical target error, dev sda, sector 2376226 If you compiled your kernel with SCSI messages it would tell you that this is a medium error, unrecovered read error in sector 2376226 The device is failing and the sector cannot be recovered. in my opinion, the OS will not hang up when this problem happen, right? But my platform is always trying to clear halt endpoint, and it seems Block or File system layer still try to access this sector. That depends on what's being done and the configuration of the layers above. For this error SCSI passes up as fast as possible. The log implied some churn below in USB. On a real FS, the page should eventually be marked as error, which should cause this to stop, but there are a variety of daemons which poke the disk, it could be one of them. is that related to file node open method or file read method? such that read bad sector will hang up the OS. Depends, it's definitely possible to trigger repeat behaviour with a DIO read because there's no memory of the error in the page cache. There may be other ways. James -- 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 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
Hello. On 05/10/2014 01:57 PM, Vivek Gautam wrote: Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/host/ehci-tegra.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 572634c..ccc6433 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -411,9 +411,8 @@ static int tegra_ehci_probe(struct platform_device *pdev) } hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, resource_size(res)); + hcd-regs = devm_ioremap_resource(pdev-dev, res); if (!hcd-regs) { This has to be changed as well as devm_ioremap_resource() returns error, not NULL. - dev_err(pdev-dev, Failed to remap I/O memory\n); err = -ENOMEM; This needs to be changed as well, to pass up the error code devm_ioremap_resource() returned. WBR, Sergei -- 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 2/4] net: cdc_mbim: reject IP packets on DSS VLANs
DSS VLANs are pseudo network interfaces representing arbitrary data streams, and specifically not IP. Preventing spurious IP packets can sometimes be a hassle. The kernel will for example send an IPv6 Router Solicit when the interface is brought up unless the user has been careful enough to disable IPv6 first. Such packets forwared to a MBIM DSS session will look like spurious noise to the device, and can cause it to log an error or even malfunction. Drop all IP packets on the designated DSS VLANs to prevent such unwanted spurious transmissions. Cc: Greg Suarez gsua...@smithmicro.com Reported-by: Arnaud Desmier adesm...@sequans.com Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_mbim.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c index dbc19fd945a9..beb8b95160a5 100644 --- a/drivers/net/usb/cdc_mbim.c +++ b/drivers/net/usb/cdc_mbim.c @@ -237,6 +237,8 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb c[3] = tci; break; case 0x0100: /* VLAN ID 256 - 511 */ + if (is_ip) + goto error; sign = cpu_to_le32(USB_CDC_MBIM_NDP16_DSS_SIGN); c = (u8 *)sign; c[3] = tci; -- 2.0.0.rc2 -- 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 1/4] net: cdc_mbim: optionally use VLAN ID 4094 for IP session 0
The cdc_mbim driver maps 802.1q VLANs to MBIM IP and DSS sessions. MBIM IP session 0 is handled as an exception and is mapped to untagged frames. This patch adds optional support for remapping MBIM IP session 0 to 802.1q VLAN ID 4094 instead. The default behaviour is not changed. The new behaviour is triggered by adding a link for this previously unsupported VLAN. The untagged mapping was chosen initially to support the assumed most common use case: Most current MBIM devices only support a single IP session (i.e. session 0 only), and using untagged frames lets the users completely ignore the additonal complexity of the multiplexing layer. But when the multiplexing features of MBIM are used, then this netdev gets a double meaning: It becomes the master interface for all the VLAN subdevs the additional sessions are mapped to, while still serving as the untagged IP interface for session 0. This can be problematic, especially when using Device Service Streams (DSS), as have become apparent recently with the availability of devices with real DSS support. Some use cases need to e.g set a MTU which is higher than allowed for IP Session 0. The dual role also leads to the situation where the IP Session 0 interface cannot be taken down without breaking unrelated IP or DSS sessions - a devastating side effect which applications managing a simple IP session cannot be expected to be aware of. A typical DHCP client will assume that it should bring the interface down after releasing the IP lease. These problems can be avoided by tagging IP session 0 packets too, making this session similar to all other multiplexed sessions. This redefines the main netdev as an upper master interface only. Cc: Greg Suarez gsua...@smithmicro.com Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_mbim.c | 74 ++ 1 file changed, 68 insertions(+), 6 deletions(-) diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c index 076aa8a179fb..dbc19fd945a9 100644 --- a/drivers/net/usb/cdc_mbim.c +++ b/drivers/net/usb/cdc_mbim.c @@ -24,13 +24,21 @@ #include net/ipv6.h #include net/addrconf.h +/* alternative VLAN for IP session 0 if not untagged */ +#define MBIM_IPS0_VID 4094 + /* driver specific data - must match cdc_ncm usage */ struct cdc_mbim_state { struct cdc_ncm_ctx *ctx; atomic_t pmcount; struct usb_driver *subdriver; - struct usb_interface *control; - struct usb_interface *data; + unsigned long _unused; + unsigned long flags; +}; + +/* flags for the cdc_mbim_state.flags field */ +enum cdc_mbim_flags { + FLAG_IPS0_VLAN = 1 0,/* IP session 0 is tagged */ }; /* using a counter to merge subdriver requests with our own into a combined state */ @@ -62,6 +70,42 @@ static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status) return cdc_mbim_manage_power(dev, status); } +static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid) +{ + struct usbnet *dev = netdev_priv(netdev); + struct cdc_mbim_state *info = (void *)dev-data; + + /* creation of this VLAN is a request to tag IP session 0 */ + if (vid == MBIM_IPS0_VID) + info-flags |= FLAG_IPS0_VLAN; + else + if (vid = 512) /* we don't map these to MBIM session */ + return -EINVAL; + return 0; +} + +static int cdc_mbim_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 vid) +{ + struct usbnet *dev = netdev_priv(netdev); + struct cdc_mbim_state *info = (void *)dev-data; + + /* this is a request for an untagged IP session 0 */ + if (vid == MBIM_IPS0_VID) + info-flags = ~FLAG_IPS0_VLAN; + return 0; +} + +static const struct net_device_ops cdc_mbim_netdev_ops = { + .ndo_open = usbnet_open, + .ndo_stop = usbnet_stop, + .ndo_start_xmit = usbnet_start_xmit, + .ndo_tx_timeout = usbnet_tx_timeout, + .ndo_change_mtu = usbnet_change_mtu, + .ndo_set_mac_address = eth_mac_addr, + .ndo_validate_addr= eth_validate_addr, + .ndo_vlan_rx_add_vid = cdc_mbim_rx_add_vid, + .ndo_vlan_rx_kill_vid = cdc_mbim_rx_kill_vid, +}; static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf) { @@ -101,7 +145,10 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf) dev-net-flags |= IFF_NOARP; /* no need to put the VLAN tci in the packet headers */ - dev-net-features |= NETIF_F_HW_VLAN_CTAG_TX; + dev-net-features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_FILTER; + + /* monitor VLAN additions and removals */ + dev-net-netdev_ops = cdc_mbim_netdev_ops; err: return ret; } @@ -164,12 +211,24 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
[PATCH net-next 4/4] net: cdc_ncm/cdc_mbim: rework probing of NCM/MBIM functions
The NCM class match in the cdc_mbim driver is confusing and cause unexpected behaviour. The USB core guarantees that a USB interface is in altsetting 0 when probing starts. This means that devices implementing a NCM 1.0 backwards compatible MBIM function (a NCM/MBIM function) always hit the NCM entry in the cdc_mbim driver match table. Such functions will never match any of the MBIM entries. This causes unexpeced behaviour for cases where the NCM and MBIM entries are differet, which is currently the case for all except Ericsson devices. Improve the probing of NCM/MBIM functions by looking up the device again in the cdc_mbim match table after switching to the MBIM identity. The shared altsetting selection is updated to better accommodate the new probing logic, returning the preferred altsetting for the control interface instead of the data interface. The control interface altsetting update is moved to the cdc_mbim driver. It is never necessary to change the control interface altsetting for NCM. Cc: Greg Suarez gsua...@smithmicro.com Reported by: Yu-an Shih ys...@nvidia.com Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/cdc_mbim.c | 43 +-- drivers/net/usb/cdc_ncm.c | 27 +-- include/linux/usb/cdc_ncm.h | 2 +- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c index beb8b95160a5..cfbd9ba65bc5 100644 --- a/drivers/net/usb/cdc_mbim.c +++ b/drivers/net/usb/cdc_mbim.c @@ -107,15 +107,54 @@ static const struct net_device_ops cdc_mbim_netdev_ops = { .ndo_vlan_rx_kill_vid = cdc_mbim_rx_kill_vid, }; +/* Change the control interface altsetting and update the .driver_info + * pointer if the matching entry after changing class codes points to + * a different struct + */ +static int cdc_mbim_set_ctrlalt(struct usbnet *dev, struct usb_interface *intf, u8 alt) +{ + struct usb_driver *driver = to_usb_driver(intf-dev.driver); + const struct usb_device_id *id; + struct driver_info *info; + int ret; + + ret = usb_set_interface(dev-udev, + intf-cur_altsetting-desc.bInterfaceNumber, + alt); + if (ret) + return ret; + + id = usb_match_id(intf, driver-id_table); + if (!id) + return -ENODEV; + + info = (struct driver_info *)id-driver_info; + if (info != dev-driver_info) { + dev_dbg(intf-dev, driver_info updated to '%s'\n, + info-description); + dev-driver_info = info; + } + return 0; +} + static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf) { struct cdc_ncm_ctx *ctx; struct usb_driver *subdriver = ERR_PTR(-ENODEV); int ret = -ENODEV; - u8 data_altsetting = cdc_ncm_select_altsetting(dev, intf); + u8 data_altsetting = 1; struct cdc_mbim_state *info = (void *)dev-data; - /* Probably NCM, defer for cdc_ncm_bind */ + /* should we change control altsetting on a NCM/MBIM function? */ + if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) { + data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM; + ret = cdc_mbim_set_ctrlalt(dev, intf, CDC_NCM_COMM_ALTSETTING_MBIM); + if (ret) + goto err; + ret = -ENODEV; + } + + /* we will hit this for NCM/MBIM functions if prefer_mbim is false */ if (!cdc_ncm_comm_intf_is_mbim(intf-cur_altsetting)) goto err; diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 549dbac710ed..a1e0ba2a3a42 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -541,10 +541,10 @@ void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf) } EXPORT_SYMBOL_GPL(cdc_ncm_unbind); -/* Select the MBIM altsetting iff it is preferred and available, - * returning the number of the corresponding data interface altsetting +/* Return the number of the MBIM control interface altsetting iff it + * is preferred and available, */ -u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf) +u8 cdc_ncm_select_altsetting(struct usb_interface *intf) { struct usb_host_interface *alt; @@ -563,15 +563,15 @@ u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf) * the rules given in section 6 (USB Device Model) of this * specification. */ - if (prefer_mbim intf-num_altsetting == 2) { + if (intf-num_altsetting 2) + return intf-cur_altsetting-desc.bAlternateSetting; + + if (prefer_mbim) { alt = usb_altnum_to_altsetting(intf, CDC_NCM_COMM_ALTSETTING_MBIM); - if (alt cdc_ncm_comm_intf_is_mbim(alt) - !usb_set_interface(dev-udev, -
[PATCH net-next 3/4] net: cdc_mbim: add driver documentation
An initial attempt on describing some of the odd APIs provided by this driver. Cc: Greg Suarez gsua...@smithmicro.com Signed-off-by: Bjørn Mork bj...@mork.no --- Documentation/networking/cdc_mbim.txt | 339 ++ 1 file changed, 339 insertions(+) create mode 100644 Documentation/networking/cdc_mbim.txt diff --git a/Documentation/networking/cdc_mbim.txt b/Documentation/networking/cdc_mbim.txt new file mode 100644 index ..a15ea602aa52 --- /dev/null +++ b/Documentation/networking/cdc_mbim.txt @@ -0,0 +1,339 @@ + cdc_mbim - Driver for CDC MBIM Mobile Broadband modems + + +The cdc_mbim driver supports USB devices conforming to the Universal +Serial Bus Communications Class Subclass Specification for Mobile +Broadband Interface Model [1], which is a further development of +Universal Serial Bus Communications Class Subclass Specifications for +Network Control Model Devices [2] optimized for Mobile Broadband +devices, aka 3G/LTE modems. + + +Command Line Parameters +=== + +The cdc_mbim driver has no parameters of its own. But the probing +behaviour for NCM 1.0 backwards compatible MBIM functions (an +NCM/MBIM function as defined in section 3.2 of [1]) is affected +by a cdc_ncm driver parameter: + +prefer_mbim +--- +Type: Boolean +Valid Range: N/Y (0-1) +Default Value: Y (MBIM is preferred) + +This parameter sets the system policy for NCM/MBIM functions. Such +functions will be handled by either the cdc_ncm driver or the cdc_mbim +driver depending on the prefer_mbim setting. Setting prefer_mbim=N +makes the cdc_mbim driver ignore these functions and lets the cdc_ncm +driver handle them instead. + +The parameter is writable, and can be changed at any time. A manual +unbind/bind is required to make the change effective for NCM/MBIM +functions bound to the wrong driver + + +Basic usage +=== + +MBIM functions are inactive when unmanaged. The cdc_mbim driver only +provides an userspace interface to the MBIM control channel, and will +not participate in the management of the function. This implies that a +userspace MBIM management application always is required to enable a +MBIM function. + +Such userspace applications includes, but are not limited to: + - mbimcli (included with the libmbim [3] library), and + - ModemManager [4] + +Establishing a MBIM IP session reequires at least these actions by the +management application: + - open the control channel + - configure network connection settings + - connect to network + - configure IP interface + +Management application development +-- +The driver - userspace interfaces are described below. The MBIM +control channel protocol is described in [1]. + + +MBIM control channel userspace ABI +== + +/dev/cdc-wdmX character device +-- +The driver creates a two-way pipe to the MBIM function control channel +using the cdc-wdm driver as a subdriver. The userspace end of the +control channel pipe is a /dev/cdc-wdmX character device. + +The cdc_mbim driver does not process or police messages on the control +channel. The channel is fully delegated to the userspace management +application. It is therefore up to this application to ensure that it +complies with all the control channel requirements in [1]. + +The cdc-wdmX device is created as a child of the MBIM control +interface USB device. The character device associated with a specific +MBIM function can be looked up using sysfs. For example: + + bjorn@nemi:~$ ls /sys/bus/usb/drivers/cdc_mbim/2-4:2.12/usbmisc + cdc-wdm0 + + bjorn@nemi:~$ grep . /sys/bus/usb/drivers/cdc_mbim/2-4:2.12/usbmisc/cdc-wdm0/dev + 180:0 + + +USB configuration descriptors +- +The wMaxControlMessage field of the CDC MBIM functional descriptor +limits the maximum control message size. The managament application is +responsible for negotiating a control message size complying with the +requirements in section 9.3.1 of [1], taking this descriptor field +into consideration. + +The userspace application can access the CDC MBIM functional +descriptor of a MBIM function using either of the two USB +configuration descriptor kernel interfaces described in [6] or [7]. + +See also the ioctl documentation below. + + +Fragmentation +- +The userspace application is responsible for all control message +fragmentation and defragmentaion, as described in section 9.5 of [1]. + + +/dev/cdc-wdmX write() +- +The MBIM control messages from the management application *must not* +exceed the negotiated control message size. + + +/dev/cdc-wdmX read() + +The management application *must* accept control messages of up the +negotiated control message size. + + +/dev/cdc-wdmX ioctl() + +IOCTL_WDM_MAX_COMMAND: Get Maximum Command Size +This
[PATCH net-next 0/4] cdc_mbim: cleanups and new features
This series depends on commit 6b5eeb7f874b (net: cdc_mbim: handle unaccelerated VLAN tagged frames), which is currently in net but not yet in net-next. Patch 4 might have a minor context collision with the cdc_ncm: add buffer tuning and stats using ethtool series I just posted for review. Please let me know if I should submit an adjusted version in either direction. These two series' are otherwise completely independent of each other. The major new feature here is in patch 1, which I hope will solve some problems with the original design without changing the existing API, optionally allowing IP session 0 to be treated like any other MBIM session. The rest are some minor cleanups and finally some documentation of the current driver APIs, after this series has been applied. I started feeling a bit more mortal than usual lately, which probably is healthy, and realized that I should put some of the stuff in my head in a somewhat less volatile storage. Bjørn Mork (4): net: cdc_mbim: optionally use VLAN ID 4094 for IP session 0 net: cdc_mbim: reject IP packets on DSS VLANs net: cdc_mbim: add driver documentation net: cdc_ncm/cdc_mbim: rework probing of NCM/MBIM functions Documentation/networking/cdc_mbim.txt | 339 ++ drivers/net/usb/cdc_mbim.c| 119 +++- drivers/net/usb/cdc_ncm.c | 27 ++- include/linux/usb/cdc_ncm.h | 2 +- 4 files changed, 464 insertions(+), 23 deletions(-) create mode 100644 Documentation/networking/cdc_mbim.txt -- 2.0.0.rc2 -- 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] [resend] net: get rid of SET_ETHTOOL_OPS
net: get rid of SET_ETHTOOL_OPS Dave Miller mentioned he'd like to see SET_ETHTOOL_OPS gone. This does that. Mostly done via coccinelle script: @@ struct ethtool_ops *ops; struct net_device *dev; @@ - SET_ETHTOOL_OPS(dev, ops); + dev-ethtool_ops = ops; Compile tested only, but I'd seriously wonder if this broke anything. Suggested-by: Dave Miller da...@davemloft.net Signed-off-by: Wilfried Klaebe w-l...@lebenslange-mailadresse.de --- Applies against v3.15-rc4 and v3.15-rc5 likewise. resend: trimmed CC list. Thanks to Dave for notifying me. diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c index c4b3940..078cadd 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c @@ -105,5 +105,5 @@ static const struct ethtool_ops ipoib_ethtool_ops = { void ipoib_set_ethtool_ops(struct net_device *dev) { - SET_ETHTOOL_OPS(dev, ipoib_ethtool_ops); + dev-ethtool_ops = ipoib_ethtool_ops; } diff --git a/drivers/net/ethernet/3com/3c509.c b/drivers/net/ethernet/3com/3c509.c index 35df0b9..a968654 100644 --- a/drivers/net/ethernet/3com/3c509.c +++ b/drivers/net/ethernet/3com/3c509.c @@ -534,7 +534,7 @@ static int el3_common_init(struct net_device *dev) /* The EL3-specific entries in the device structure. */ dev-netdev_ops = netdev_ops; dev-watchdog_timeo = TX_TIMEOUT; - SET_ETHTOOL_OPS(dev, ethtool_ops); + dev-ethtool_ops = ethtool_ops; err = register_netdev(dev); if (err) { diff --git a/drivers/net/ethernet/3com/3c589_cs.c b/drivers/net/ethernet/3com/3c589_cs.c index 063557e..f18647c 100644 --- a/drivers/net/ethernet/3com/3c589_cs.c +++ b/drivers/net/ethernet/3com/3c589_cs.c @@ -218,7 +218,7 @@ static int tc589_probe(struct pcmcia_device *link) dev-netdev_ops = el3_netdev_ops; dev-watchdog_timeo = TX_TIMEOUT; - SET_ETHTOOL_OPS(dev, netdev_ethtool_ops); + dev-ethtool_ops = netdev_ethtool_ops; return tc589_config(link); } diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c index 465cc71..e13b046 100644 --- a/drivers/net/ethernet/3com/typhoon.c +++ b/drivers/net/ethernet/3com/typhoon.c @@ -2435,7 +2435,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) netif_napi_add(dev, tp-napi, typhoon_poll, 16); dev-watchdog_timeo = TX_TIMEOUT; - SET_ETHTOOL_OPS(dev, typhoon_ethtool_ops); + dev-ethtool_ops = typhoon_ethtool_ops; /* We can handle scatter gather, up to 16 entries, and * we can do IP checksumming (only version 4, doh...) diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c index 171d73c..40dbbf7 100644 --- a/drivers/net/ethernet/adaptec/starfire.c +++ b/drivers/net/ethernet/adaptec/starfire.c @@ -784,7 +784,7 @@ static int starfire_init_one(struct pci_dev *pdev, dev-netdev_ops = netdev_ops; dev-watchdog_timeo = TX_TIMEOUT; - SET_ETHTOOL_OPS(dev, ethtool_ops); + dev-ethtool_ops = ethtool_ops; netif_napi_add(dev, np-napi, netdev_poll, max_interrupt_work); diff --git a/drivers/net/ethernet/alteon/acenic.c b/drivers/net/ethernet/alteon/acenic.c index 1517e9df..9a6991b 100644 --- a/drivers/net/ethernet/alteon/acenic.c +++ b/drivers/net/ethernet/alteon/acenic.c @@ -476,7 +476,7 @@ static int acenic_probe_one(struct pci_dev *pdev, dev-watchdog_timeo = 5*HZ; dev-netdev_ops = ace_netdev_ops; - SET_ETHTOOL_OPS(dev, ace_ethtool_ops); + dev-ethtool_ops = ace_ethtool_ops; /* we only display this string ONCE */ if (!boards_found) diff --git a/drivers/net/ethernet/altera/altera_tse_ethtool.c b/drivers/net/ethernet/altera/altera_tse_ethtool.c index 319ca74..8ac4bd2 100644 --- a/drivers/net/ethernet/altera/altera_tse_ethtool.c +++ b/drivers/net/ethernet/altera/altera_tse_ethtool.c @@ -231,5 +231,5 @@ static const struct ethtool_ops tse_ethtool_ops = { void altera_tse_set_ethtool_ops(struct net_device *netdev) { - SET_ETHTOOL_OPS(netdev, tse_ethtool_ops); + netdev-ethtool_ops = tse_ethtool_ops; } diff --git a/drivers/net/ethernet/amd/amd8111e.c b/drivers/net/ethernet/amd/amd8111e.c index 26efaaa..068dc7c 100644 --- a/drivers/net/ethernet/amd/amd8111e.c +++ b/drivers/net/ethernet/amd/amd8111e.c @@ -1900,7 +1900,7 @@ static int amd8111e_probe_one(struct pci_dev *pdev, /* Initialize driver entry points */ dev-netdev_ops = amd8111e_netdev_ops; - SET_ETHTOOL_OPS(dev, ops); + dev-ethtool_ops = ops; dev-irq =pdev-irq; dev-watchdog_timeo = AMD8111E_TX_TIMEOUT; netif_napi_add(dev, lp-napi, amd8111e_rx_poll, 32); diff --git a/drivers/net/ethernet/amd/au1000_eth.c b/drivers/net/ethernet/amd/au1000_eth.c index a2bd91e..a78e4c1 100644 --- a/drivers/net/ethernet/amd/au1000_eth.c +++
RE: i.MX6 USB OTG support is broken on linux-next
On Sat, May 10, 2014 at 07:10:05PM +0800, Peter Chen wrote: On Fri, May 09, 2014 at 09:00:47PM +0800, Shawn Guo wrote: I'm running next-20140508 kernel on imx6q-sabresd board with USB mouse/keyboard connected to OTG port. It works well on 3.15-rc but is broken on recent linux-next kernel with the message like below. ... usb 1-1: device v413c p2107 is not supported hub 1-0:1.0: unable to enumerate USB device on port 1 With the help from Peter, I found that the issue shows up only when CONFIG_USB_OTG_FSM is enabled. The option is enabled by commit 8fd2f1f (ARM: imx_v6_v7_defconfig: Enable drivers for i.MX51 USB Host support.) from IMX tree. I guess this is a sign that chipidea otg_fsm driver is buggy? I do not think dropping CONFIG_USB_OTG_FSM selection from imx_v6_v7_defconfig is a solution, and we need to fix the issue in usb driver, right? No, Shawn. The solution of this problem is drop CONFIG_USB_OTG_FSM, I guess Denis added it wrongly. According to Embedded Hosts OTG spec, OTG devices and Embedded Hosts both have Targeted Host functionality, and each Targeted Host has its TPL (Targeted Peripheral List), only the devices are at TPL are supported by Targeted Host. Current Linux TPL only contains limited devices, each Targeted Host needs to update its TPL when it makes product, (The comments at otg_whitelist.h also mention it), and the devices are not at TPL will not be supported, and will report to user as an unsupported device error, at current Linux configuration, once the CONFIG_USB_OTG_FSM and CONFIG_USB_OTG are chosen, the host will be Targeted Host. The reason why Shawn met this problem is: the imx_v6_v7_defconfig set CONFIG_USB_OTG_FSM, so the CONFIG_USB_OTG and CONFIG_USB_OTG_WHITELIST are set according to dependency, it makes imx6 device as Targeted Host, but the devices connected at Shawn's board are not at TPL, so they are not be supported. In a word the CONFIG_USB_OTG_FSM should not be chosen for defauly kernel configuration. The CONFIG_USB_OTG_FSM should only be chosen by default when this device goes to make product and the TPL is updated accordingly. Thanks for the explanation, Peter. So this is what I see from USB Kconfig. config USB_OTG_FSM tristate USB 2.0 OTG FSM implementation depends on USB select USB_OTG select USB_PHY config USB_OTG_WHITELIST bool Rely on OTG Targeted Peripherals List depends on USB_OTG || EXPERT default y if USB_OTG I see that USB_OTG_FSM has a dependency on USB_OTG, and USB_OTG_WHITELIST depends on USB_OTG, but USB_OTG does *not* depends on USB_OTG_WHITELIST. I'm not sure why we have 'default y if USB_OTG' for USB_OTG_WHITELIST, when this option only makes sense for product producers per your explanation. So maybe dropping the 'default y' is the right fix? As I explained before: OTG devices and Embedded Hosts both have Targeted Host functionality, and each Targeted Host has its TPL (Targeted Peripheral List), only the devices are at TPL are supported by Targeted Host. So when the board is OTG EH (CONFIG_USB_OTG is set), it should have TPL according to spec. In fact, even the CONFIG_USB_OTG_WHITELIST is not set, the non-TPL devices have not worked well, eg, it will not be enumerated again after it has disconnected from host 10 seconds later due to vbus has turned off by host. Peter diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig index cb8e991..9081757 100644 --- a/drivers/usb/core/Kconfig +++ b/drivers/usb/core/Kconfig @@ -65,7 +65,6 @@ config USB_OTG config USB_OTG_WHITELIST bool Rely on OTG Targeted Peripherals List depends on USB_OTG || EXPERT - default y if USB_OTG help If you say Y here, the otg_whitelist.h file will be used as a product whitelist, so USB peripherals not listed there will be Shawn -- 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: i.MX6 USB OTG support is broken on linux-next
On Sat, May 10, 2014 at 09:57:39PM +0800, Li Jun wrote: As Peter suggested, do not enable OTG_FSM in defconfig since there are very few HNPSRP capable device in market. With OTG_FSM enabled, even USB_OTG_WHITELIST is not selected, the OTG port still can't work the same as before if you connect a normal usb device to it. Oh, okay. In that case, I will have to drop OTG_FSM from defconfig. Thanks for the info. Shawn -- 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: i.MX6 USB OTG support is broken on linux-next
On Sun, May 11, 2014 at 08:40:25AM +0800, Chen Peter-B29397 wrote: As I explained before: OTG devices and Embedded Hosts both have Targeted Host functionality, and each Targeted Host has its TPL (Targeted Peripheral List), only the devices are at TPL are supported by Targeted Host. So when the board is OTG EH (CONFIG_USB_OTG is set), it should have TPL according to spec. In fact, even the CONFIG_USB_OTG_WHITELIST is not set, the non-TPL devices have not worked well, eg, it will not be enumerated again after it has disconnected from host 10 seconds later due to vbus has turned off by host. So all in all, it sounds to me that in my case, USB_OTG_FSM shouldn't be set, neither in defconfig nor manually in menuconfig, right? What about USB_OTG? Can I set it in defconfig or manually in menuconfig? Shawn -- 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