Re: [PATCH v4 3/4] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework
Hi Tomasz,
On Thu, May 1, 2014 at 10:46 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
Hi Vivek,
Please see my comments inline.
Thanks for the review, please find my answers inline.
On 30.04.2014 07:19, Vivek Gautam wrote:
Add support to consume phy provided by Generic phy framework.
Keeping the support for older usb-phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ohci-exynos.
Once we move to new phy in the device nodes for ohci, we can
remove the support for older phys.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Cc: Jingoo Han jg1@samsung.com
Cc: Alan Stern st...@rowland.harvard.edu
---
Changes from v3:
- Calling usb_phy_shutdown() when exynos_ohci_phy_enable() is failing.
- Made exynos_ohci_phy_disable() return void, since its return value
did not serve any purpose.
- Calling clk_disable_unprepare() in exynos_ohci_resume() when
exynos_ohci_phy_enable() is failed.
.../devicetree/bindings/usb/exynos-usb.txt | 19 +++
drivers/usb/host/ohci-exynos.c | 128
+---
2 files changed, 132 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..a90c973 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -38,6 +38,15 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are OHCI phys, they should be listed here.
+ One phy per port. Each port should have following entries:
+ - reg: port number on OHCI controller, e.g
+ On Exynos5250, port 0 is USB2.0 otg phy
+ port 1 is HSIC phy0
+ port 2 is HSIC phy1
+ - phys: from the *Generic PHY* bindings, specifying phy used by
port.
+ - phy-names: from the *Generic PHY* bindings, specifying name of
phy
+used by the port.
I think you don't need this property for this binding, as the PHYs are being
requested by indices (in fact only index 0 is used).
True, that phy-names property is not used, since PHYs are being
requested using indices.
We can remove this.
Example:
usb@1212 {
@@ -47,6 +56,16 @@ Example:
clocks = clock 285;
clock-names = usbhost;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ port@0 {
+ reg = 0;
+ phys = usb2phy 1;
+ phy-names = host;
Ditto.
will remove this.
+ status = disabled;
+ };
+
};
DWC3
diff --git a/drivers/usb/host/ohci-exynos.c
b/drivers/usb/host/ohci-exynos.c
index 05f00e3..f90bf9a 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -18,6 +18,7 @@
#include linux/module.h
#include linux/of.h
#include linux/platform_device.h
+#include linux/phy/phy.h
#include linux/usb/phy.h
#include linux/usb/samsung_usb_phy.h
#include linux/usb.h
@@ -33,28 +34,122 @@ static struct hc_driver __read_mostly
exynos_ohci_hc_driver;
#define to_exynos_ohci(hcd) (struct exynos_ohci_hcd
*)(hcd_to_ohci(hcd)-priv)
+#define PHY_NUMBER 3
nit: A blank line would be nice here to separate from the struct below.
Ok, will add one.
By the way, doesn't the number of PHY ports depend on platform? Of course
right now the driver supports only Exynos = 4210 SoCs with the generic PHY
interface, so it might be changed in further patches.
Yes, the number of PHY ports will be platform dependent feature.
In subsequent patches we can add support to count the number of PHYs,
or rather in this patch
itself, when we do -
for_each_available_child_of_node(dev-of_node, child) {
we can keep a count of how many child nodes we found, and then
configure those many.
As such the host controller drivers will have 'host' and 'hsic' PHYs.
struct exynos_ohci_hcd {
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
+ struct phy *phy_g[PHY_NUMBER];
};
-static void exynos_ohci_phy_enable(struct device *dev)
+static int exynos_ohci_get_phy(struct device *dev,
+ struct exynos_ohci_hcd *exynos_ohci)
+{
+ struct device_node *child;
+ struct phy *phy;
+ int phy_number;
+ int ret = 0;
+
+ exynos_ohci-phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(exynos_ohci-phy)) {
+ ret = PTR_ERR(exynos_ohci-phy);
+ /* This is the case when PHY config is disabled */
+ if (ret == -ENXIO || ret == -ENODEV)
[no subject]
Bestauml;tigen Sie Ihre 500,000,00 Euro -- 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 v4 3/4] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework
On Fri, May 2, 2014 at 2:55 PM, Vivek Gautam gautam.vi...@samsung.com wrote:
Hi Tomasz,
On Thu, May 1, 2014 at 10:46 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
Hi Vivek,
Please see my comments inline.
Thanks for the review, please find my answers inline.
On 30.04.2014 07:19, Vivek Gautam wrote:
Add support to consume phy provided by Generic phy framework.
Keeping the support for older usb-phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ohci-exynos.
Once we move to new phy in the device nodes for ohci, we can
remove the support for older phys.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Cc: Jingoo Han jg1@samsung.com
Cc: Alan Stern st...@rowland.harvard.edu
---
Changes from v3:
- Calling usb_phy_shutdown() when exynos_ohci_phy_enable() is failing.
- Made exynos_ohci_phy_disable() return void, since its return value
did not serve any purpose.
- Calling clk_disable_unprepare() in exynos_ohci_resume() when
exynos_ohci_phy_enable() is failed.
.../devicetree/bindings/usb/exynos-usb.txt | 19 +++
drivers/usb/host/ohci-exynos.c | 128
+---
2 files changed, 132 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..a90c973 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -38,6 +38,15 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are OHCI phys, they should be listed here.
+ One phy per port. Each port should have following entries:
+ - reg: port number on OHCI controller, e.g
+ On Exynos5250, port 0 is USB2.0 otg phy
+ port 1 is HSIC phy0
+ port 2 is HSIC phy1
+ - phys: from the *Generic PHY* bindings, specifying phy used by
port.
+ - phy-names: from the *Generic PHY* bindings, specifying name of
phy
+used by the port.
I think you don't need this property for this binding, as the PHYs are being
requested by indices (in fact only index 0 is used).
True, that phy-names property is not used, since PHYs are being
requested using indices.
We can remove this.
Example:
usb@1212 {
@@ -47,6 +56,16 @@ Example:
clocks = clock 285;
clock-names = usbhost;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ port@0 {
+ reg = 0;
+ phys = usb2phy 1;
+ phy-names = host;
Ditto.
will remove this.
+ status = disabled;
+ };
+
};
DWC3
diff --git a/drivers/usb/host/ohci-exynos.c
b/drivers/usb/host/ohci-exynos.c
index 05f00e3..f90bf9a 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -18,6 +18,7 @@
#include linux/module.h
#include linux/of.h
#include linux/platform_device.h
+#include linux/phy/phy.h
#include linux/usb/phy.h
#include linux/usb/samsung_usb_phy.h
#include linux/usb.h
@@ -33,28 +34,122 @@ static struct hc_driver __read_mostly
exynos_ohci_hc_driver;
#define to_exynos_ohci(hcd) (struct exynos_ohci_hcd
*)(hcd_to_ohci(hcd)-priv)
+#define PHY_NUMBER 3
nit: A blank line would be nice here to separate from the struct below.
Ok, will add one.
By the way, doesn't the number of PHY ports depend on platform? Of course
right now the driver supports only Exynos = 4210 SoCs with the generic PHY
interface, so it might be changed in further patches.
Yes, the number of PHY ports will be platform dependent feature.
In subsequent patches we can add support to count the number of PHYs,
or rather in this patch
itself, when we do -
for_each_available_child_of_node(dev-of_node, child) {
we can keep a count of how many child nodes we found, and then
configure those many.
As such the host controller drivers will have 'host' and 'hsic' PHYs.
struct exynos_ohci_hcd {
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
+ struct phy *phy_g[PHY_NUMBER];
};
-static void exynos_ohci_phy_enable(struct device *dev)
+static int exynos_ohci_get_phy(struct device *dev,
+ struct exynos_ohci_hcd *exynos_ohci)
+{
+ struct device_node *child;
+ struct phy *phy;
+ int phy_number;
+ int ret = 0;
+
+ exynos_ohci-phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(exynos_ohci-phy)) {
+ ret = PTR_ERR(exynos_ohci-phy);
+ /* This is
Re: musb_hdrc and tusb6010 loop
01.05.2014 23:51, Felipe Balbi пишет: I don't think you need that. Have tusb_get_revision() run only one during tusb6010 probe/init function and cache the returned value inside musb-revision or something like that, then remove all other calls to tusb_get_revision() and have tusb6010_omap.c check revision using if (musb-revision = TUSB_REV30) and your problem is solved. Do you think that it is possible to add another tusb6010-specific field to struct musb, which seems to be general interface? -- 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
Status of PLX USB3380?
Hello I have seen on the list that there was a previous interest on giving support for this chipset. Is anyone currently working on it? Apparently the chip can be configured in legacy mode being register compatible with the net2280 chipset. Has this been tested? The driver from the manufacturer explicitly mentions LGPLv2 and GPL, so I guess they can be used for inspiration. Thanks! -- 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
[PATCH v5 3/4] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework
Add support to consume phy provided by Generic phy framework.
Keeping the support for older usb-phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ohci-exynos.
Once we move to new phy in the device nodes for ohci, we can
remove the support for older phys.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Cc: Jingoo Han jg1@samsung.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
Changes from v4:
- Removed 'phy-names' property from the bindings since we don't need it.
- Restructured exynos_ohci_get_phy() function to handle error codes as
well as return relevant error codes effectively.
- Added IS_ERR() check for PHYs in exynos_ohci_phy_enable()/disable().
Changes from v3:
- Calling usb_phy_shutdown() when exynos_ohci_phy_enable() is failing.
- Made exynos_ohci_phy_disable() return void, since its return value
did not serve any purpose.
- Calling clk_disable_unprepare() in exynos_ohci_resume() when
exynos_ohci_phy_enable() is failed.
.../devicetree/bindings/usb/exynos-usb.txt | 16 +++
drivers/usb/host/ohci-exynos.c | 120 +---
2 files changed, 120 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..49a9c6f 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -38,6 +38,13 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are OHCI phys, they should be listed here.
+ One phy per port. Each port should have following entries:
+ - reg: port number on OHCI controller, e.g
+ On Exynos5250, port 0 is USB2.0 otg phy
+ port 1 is HSIC phy0
+ port 2 is HSIC phy1
+ - phys: from the *Generic PHY* bindings, specifying phy used by port.
Example:
usb@1212 {
@@ -47,6 +54,15 @@ Example:
clocks = clock 285;
clock-names = usbhost;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ port@0 {
+ reg = 0;
+ phys = usb2phy 1;
+ status = disabled;
+ };
+
};
DWC3
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 05f00e3..e66d391 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -18,6 +18,7 @@
#include linux/module.h
#include linux/of.h
#include linux/platform_device.h
+#include linux/phy/phy.h
#include linux/usb/phy.h
#include linux/usb/samsung_usb_phy.h
#include linux/usb.h
@@ -33,28 +34,112 @@ static struct hc_driver __read_mostly
exynos_ohci_hc_driver;
#define to_exynos_ohci(hcd) (struct exynos_ohci_hcd *)(hcd_to_ohci(hcd)-priv)
+#define PHY_NUMBER 3
+
struct exynos_ohci_hcd {
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
+ struct phy *phy_g[PHY_NUMBER];
};
-static void exynos_ohci_phy_enable(struct device *dev)
+static int exynos_ohci_get_phy(struct device *dev,
+ struct exynos_ohci_hcd *exynos_ohci)
+{
+ struct device_node *child;
+ struct phy *phy;
+ int phy_number;
+ int ret = 0;
+
+ exynos_ohci-phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(exynos_ohci-phy)) {
+ ret = PTR_ERR(exynos_ohci-phy);
+ if (ret != -ENXIO ret != -ENODEV) {
+ dev_err(dev, no usb2 phy configured\n);
+ return ret;
+ }
+ dev_dbg(dev, Failed to get usb2 phy\n);
+ exynos_ohci-phy = ERR_PTR(-ENODEV);
+ } else {
+ exynos_ohci-otg = exynos_ohci-phy-otg;
+ }
+
+ /*
+* Getting generic phy:
+* We are keeping both types of phys as a part of transiting OHCI
+* to generic phy framework, so as to maintain backward compatibilty
+* with old DTB.
+* If there are existing devices using DTB files built from them,
+* to remove the support for old bindings in this driver,
+* we need to make sure that such devices have their DTBs
+* updated to ones built from new DTS.
+*/
+ for_each_available_child_of_node(dev-of_node, child) {
+ ret = of_property_read_u32(child, reg, phy_number);
+ if (ret) {
+ dev_err(dev, Failed to parse device tree\n);
+ of_node_put(child);
+ return ret;
+ }
+
+ if (phy_number = PHY_NUMBER) {
+ dev_err(dev, Invalid number of PHYs\n);
+
[PATCH v11 4/4] usb: ehci-exynos: Change to use phy provided by the generic phy framework
From: Kamil Debski k.deb...@samsung.com
Add the phy provider, supplied by new Exynos-usb2phy using
Generic phy framework.
Keeping the support for older USB phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ehci-exynos.
Once we move to new phy in the device nodes for ehci, we can
remove the support for older phys.
Signed-off-by: Kamil Debski k.deb...@samsung.com
[gautam.vi...@samsung.com: Addressed review comments from mailing list]
[gautam.vi...@samsung.com: Kept the code for old usb-phy, and just
added support for new exynos5-usb2phy in generic phy framework]
[gautam.vi...@samsung.com: Edited the commit message]
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Cc: Jingoo Han jg1@samsung.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
Changes from v10:
- Removed 'phy-names' property from the bindings since we don't need it.
- Restructured exynos_ehci_get_phy() function to handle error codes as
well as return relevant error codes effectively.
- Added IS_ERR() check for PHYs in exynos_ehci_phy_enable()/disable().
Changes from v9:
- Calling usb_phy_shutdown() when exynos_ehci_phy_enable() is failing.
- Made exynos_ehci_phy_disable() return void, since its return value
did not serve any purpose.
- Calling clk_disable_unprepare() in exynos_ehci_resume() when
exynos_ehci_phy_enable() is failed.
.../devicetree/bindings/usb/exynos-usb.txt | 15 +++
drivers/usb/host/ehci-exynos.c | 131 +---
2 files changed, 126 insertions(+), 20 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index 49a9c6f..a3b5990 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -12,6 +12,13 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are EHCI phys, they should be listed here.
+ One phy per port. Each port should have following entries:
+ - reg: port number on EHCI controller, e.g
+ On Exynos5250, port 0 is USB2.0 otg phy
+ port 1 is HSIC phy0
+ port 2 is HSIC phy1
+ - phys: from the *Generic PHY* bindings; specifying phy used by port.
Optional properties:
- samsung,vbus-gpio: if present, specifies the GPIO that
@@ -27,6 +34,14 @@ Example:
clocks = clock 285;
clock-names = usbhost;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ port@0 {
+ reg = 0;
+ phys = usb2phy 1;
+ status = disabled;
+ };
};
OHCI
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 4d763dc..e8b518a 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -19,6 +19,7 @@
#include linux/module.h
#include linux/of.h
#include linux/of_gpio.h
+#include linux/phy/phy.h
#include linux/platform_device.h
#include linux/usb/phy.h
#include linux/usb/samsung_usb_phy.h
@@ -42,14 +43,106 @@
static const char hcd_name[] = ehci-exynos;
static struct hc_driver __read_mostly exynos_ehci_hc_driver;
+#define PHY_NUMBER 3
+
struct exynos_ehci_hcd {
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
+ struct phy *phy_g[PHY_NUMBER];
};
#define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)-priv)
+static int exynos_ehci_get_phy(struct device *dev,
+ struct exynos_ehci_hcd *exynos_ehci)
+{
+ struct device_node *child;
+ struct phy *phy;
+ int phy_number;
+ int ret = 0;
+
+ exynos_ehci-phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(exynos_ehci-phy)) {
+ ret = PTR_ERR(exynos_ehci-phy);
+ if (ret != -ENXIO ret != -ENODEV) {
+ dev_err(dev, no usb2 phy configured\n);
+ return ret;
+ }
+ dev_dbg(dev, Failed to get usb2 phy\n);
+ exynos_ehci-phy = ERR_PTR(-ENODEV);
+ } else {
+ exynos_ehci-otg = exynos_ehci-phy-otg;
+ }
+
+ for_each_available_child_of_node(dev-of_node, child) {
+ ret = of_property_read_u32(child, reg, phy_number);
+ if (ret) {
+ dev_err(dev, Failed to parse device tree\n);
+ of_node_put(child);
+ return ret;
+ }
+
+ if (phy_number = PHY_NUMBER) {
+ dev_err(dev, Invalid number of PHYs\n);
+ of_node_put(child);
+ return -EINVAL;
+
[PATCH v3 2/4] usb: ehci-exynos: Use struct device instead of platform_device
Change to use struct device instead of struct platform_device
for some static functions.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Acked-by: Jingoo Han jg1@samsung.com
---
Changes from v2:
- none
drivers/usb/host/ehci-exynos.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 7f425ac..4d763dc 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -50,9 +50,8 @@ struct exynos_ehci_hcd {
#define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)-priv)
-static void exynos_setup_vbus_gpio(struct platform_device *pdev)
+static void exynos_setup_vbus_gpio(struct device *dev)
{
- struct device *dev = pdev-dev;
int err;
int gpio;
@@ -88,7 +87,7 @@ static int exynos_ehci_probe(struct platform_device *pdev)
if (err)
return err;
- exynos_setup_vbus_gpio(pdev);
+ exynos_setup_vbus_gpio(pdev-dev);
hcd = usb_create_hcd(exynos_ehci_hc_driver,
pdev-dev, dev_name(pdev-dev));
--
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 v5 0/4] usb: ehci/ohci-exynos: Move to generic phy framework
Based and tested on 'usb-next' branch of Greg's usb tree, with relevant device tree patches[1] Hi Alan, I have included your Acked-by in all the four patches, but there has been some amount of restructuring as suggested by Tomasz Figa. Please let me know if you have some comments on this patch-series. Changes from v4: - Addressed review comments for restructuring exynos_ohci_get_phy() and exynos_ehci_get_phy(); and thereby adding proper checks in exynos_ohci_phy_enable()/disable() as well as exynos_ehci_phy_enable()/ disable(). Changes from v3: - Calling usb_phy_shutdown() when exynos_o/ehci_phy_enable() is failing. - Made exynos_o/ehci_phy_disable() return void, since its return value did not serve any purpose. - Calling clk_disable_unprepare() in exynos_o/ehci_resume() when exynos_o/ehci_phy_enable() is failed. Changes from v2: - Added two patches in the series for some cleanup. usb: ohci-exynos: Use struct device instead of platform_device usb: ehci-exynos: Use struct device instead of platform_device - Addressed review comments. -- Moved exynos_ohci_phyg_on()/off routines inside exynos_ohci_phy_enable()/disable. -- Added functions exynos_ehci_phy_enable() and exynos_ehci_phy_disable() and moved exynos_ehci_phyg_on()/off routines respectively in them. -- Added necessary checks. [1]: [PATCH 0/4] dts: Add usb2phy to Exynos 5250/5420 https://lkml.org/lkml/2014/4/30/119 Kamil Debski (1): usb: ehci-exynos: Change to use phy provided by the generic phy framework Vivek Gautam (3): usb: ohci-exynos: Use struct device instead of platform_device usb: ehci-exynos: Use struct device instead of platform_device usb: ohci-exynos: Add facility to use phy provided by the generic phy framework .../devicetree/bindings/usb/exynos-usb.txt | 31 + drivers/usb/host/ehci-exynos.c | 136 drivers/usb/host/ohci-exynos.c | 134 +++ 3 files changed, 254 insertions(+), 47 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 v5 0/4] usb: ehci/ohci-exynos: Move to generic phy framework
On 05/02/14 21:47, Vivek Gautam wrote: Based and tested on 'usb-next' branch of Greg's usb tree, with relevant device tree patches[1] Hi Alan, I have included your Acked-by in all the four patches, but there has been some amount of restructuring as suggested by Tomasz Figa. Please let me know if you have some comments on this patch-series. Changes from v4: - Addressed review comments for restructuring exynos_ohci_get_phy() and exynos_ehci_get_phy(); and thereby adding proper checks in exynos_ohci_phy_enable()/disable() as well as exynos_ehci_phy_enable()/ disable(). Changes from v3: - Calling usb_phy_shutdown() when exynos_o/ehci_phy_enable() is failing. - Made exynos_o/ehci_phy_disable() return void, since its return value did not serve any purpose. - Calling clk_disable_unprepare() in exynos_o/ehci_resume() when exynos_o/ehci_phy_enable() is failed. Changes from v2: - Added two patches in the series for some cleanup. usb: ohci-exynos: Use struct device instead of platform_device usb: ehci-exynos: Use struct device instead of platform_device - Addressed review comments. -- Moved exynos_ohci_phyg_on()/off routines inside exynos_ohci_phy_enable()/disable. -- Added functions exynos_ehci_phy_enable() and exynos_ehci_phy_disable() and moved exynos_ehci_phyg_on()/off routines respectively in them. -- Added necessary checks. [1]: [PATCH 0/4] dts: Add usb2phy to Exynos 5250/5420 https://lkml.org/lkml/2014/4/30/119 Kamil Debski (1): usb: ehci-exynos: Change to use phy provided by the generic phy framework Vivek Gautam (3): usb: ohci-exynos: Use struct device instead of platform_device usb: ehci-exynos: Use struct device instead of platform_device usb: ohci-exynos: Add facility to use phy provided by the generic phy framework .../devicetree/bindings/usb/exynos-usb.txt | 31 + drivers/usb/host/ehci-exynos.c | 136 drivers/usb/host/ohci-exynos.c | 134 +++ 3 files changed, 254 insertions(+), 47 deletions(-) This series looks good to me, please feel free to add my ack on this series, Acked-by: Kukjin Kim kgene@samsung.com Thanks, Kukjin -- 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: Status of PLX USB3380?
Hi, On Fri, May 02, 2014 at 02:11:30PM +0200, Ricardo Ribalda Delgado wrote: I have seen on the list that there was a previous interest on giving support for this chipset. Is anyone currently working on it? no Apparently the chip can be configured in legacy mode being register compatible with the net2280 chipset. Has this been tested? no The driver from the manufacturer explicitly mentions LGPLv2 and GPL, so I guess they can be used for inspiration. sure, patches are welcome. I don't have access to the HW. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] USB: serial: fix sysfs-attribute removal deadlock
On Mon, Apr 28, 2014 at 08:39:47AM +0800, Li Zhong wrote: Yes, maybe try to get the module reference is not bad before writing to driver attributes, as it doesn't make much sense to really call the callbacks for the driver attribute if the driver is being unload. Please don't do that spuriously. Active protection is the primary mechanism for that sort of protection and adding spurious things just make them confusing. And after we get the reference, it is safe for us to break the active. But if we don't have such real cases(lockdep warnings), we actually don't need to break it. Yeah, for cases where active protection should be broken, other measures should be taken to prevent the underlying data structure / code from going away. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/9] usb: gadget: OS String support
On Wed, Apr 30, 2014 at 09:36:01PM -0400, Michal Nazarewicz wrote: On Thu, Apr 24 2014, Andrzej Pietrasiewicz wrote: There is a custom (non-USB IF) extension to the USB standard: http://msdn.microsoft.com/library/windows/hardware/gg463182 The said extension is maintained by Microsoft for Microsoft. Yet it is fairly common for various devices to use it, and a popular proprietary operating system expects devices to provide OS descriptors, so Linux-based USB gadgets whishing to be able to talk to a variety of operating systems should be able to provide the OS descriptors. This patch adds optional support for gadgets whishing to expose the so called OS String under index 0xEE of language 0. The contents of the string is generated based on the qw_sign array and b_vendor_code. Interested gadgets need to set the cdev-use_os_string flag, fill cdev-qw_sign with appropriate values and fill cdev-b_vendor_code with a value of their choice. This patch does not however implement responding to any vendor-specific USB requests. Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com any updates here ? Should I expect a new version in time for v3.16 ? cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: dwc3: gadget: fix burst size corruption
Hi, On Thu, May 01, 2014 at 10:15:00AM -0500, Felipe Balbi wrote: On Thu, May 01, 2014 at 09:45:17AM -0400, Alan Stern wrote: On Thu, 1 May 2014, Zhuang Jin Can wrote: again, you found a bug on the gadget driver. Fix that. composite.c guarantees that for those functions which don't pass bMaxBurst, gadget-maxburst will be set to *at least* 1. I agree the real fix should be in the gadget driver. The patch intents to prevent hibernatition from being corrupted by a bad gadget driver. If OEMs develop their own gadget driver forgetting to call config_ep_by_speed(), it'll turn out to be everything works except dwc3 hibernation, and they'll complain to dwc3. f_ffs is an example has SuperSpeed support but doesn't call config_ep_by_speed(). It's just for robustness, and dwc3 is not doing anything wrong. It did cause me a long time to figure out why the hibernation was broken. You could include the check, for the sake of robustness, in dwc3 -- but if it fails, you should write a message to the kernel log saying that the gadget driver needs to be fixed. I admit the fix is too paranoid. Thanks your comment. Also, if we're adding something to dwc3, we need to add to other USB3-capable UDCs too. Namely dummy and marvel's. So I think the fix is not valuable to you. Thanks for your comment. And I'm new to communitiy, hope you can bear with me:) Jincan -- 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: dwc3: gadget: giveback request if start transfer fail
Hi, On Thu, May 01, 2014 at 10:13:28AM -0500, Felipe Balbi wrote: On Thu, May 01, 2014 at 04:44:52PM -0400, Zhuang Jin Can wrote: On Wed, Apr 30, 2014 at 02:58:29PM -0500, Felipe Balbi wrote: On Thu, May 01, 2014 at 02:36:08AM -0400, Zhuang Jin Can wrote: At least we should giveback the current request to the gadget. Otherwise, the gadget will be stuck without knowing anything. It was oberved that the failure can happen if the request is queued when the run/stop bit of controller is not set. why is your gadget queueing any requests before calling -udc_start() ? A better question, what modification have you done to udc-core.c which broke this ? udc-core *always* calls -udc_start() by the time you load a gadget driver so this case will *never* happen. Whatever modification you did, broke this assumption and I will *not* accept this patch because the bug is elsewhere and *not* in mainline kernel. It's found in Android using kernel 3.10.20. Android has its own usb_composite_driver usb/gadget/android.c (not in mainline), and it so you found something on an old kernel using an out-of-tree gadget driver. allows userspace to disconnect the pullup (i.e clear run/stop bit in dwc3) and remove the gadget functions like adb, mtp and then add new functions like rndis, acm. The problem is when you disconnect the pullup, a gadget maybe in the middle of queuing a request, and result in the start transfer cmd failure. I think this is also a common issue for other Android gadget needs to learn how to cope with that. Agree. usb_composite_drivers too. Normally, if one of the gadget deactivate its own function, the pullup will be disconnected, other gadgets won't get notified until their requests are failed. So it makes dwc3 more robust to deal with these situations. Right, but Android gadget can run on top of several other UDCs and you want to have a single one of them cope with android's bug ? You'd be better off getting google to accept a bugfix to the android gadget, since that's where the problem lies. I agree. I'll try to push the fix to google. It's really hard to fix the race condition (for me), as any gadget or /sys/class/udc/soft_connect can just disconnect the pullup anytime they want. The only thing I can do is giving back the request to the gadget if the condition happens. Jincan -- 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: dwc3: gadget: giveback request if start transfer fail
Hi, On Sat, May 03, 2014 at 12:05:41AM -0400, Zhuang Jin Can wrote: On Thu, May 01, 2014 at 10:13:28AM -0500, Felipe Balbi wrote: On Thu, May 01, 2014 at 04:44:52PM -0400, Zhuang Jin Can wrote: On Wed, Apr 30, 2014 at 02:58:29PM -0500, Felipe Balbi wrote: On Thu, May 01, 2014 at 02:36:08AM -0400, Zhuang Jin Can wrote: At least we should giveback the current request to the gadget. Otherwise, the gadget will be stuck without knowing anything. It was oberved that the failure can happen if the request is queued when the run/stop bit of controller is not set. why is your gadget queueing any requests before calling -udc_start() ? A better question, what modification have you done to udc-core.c which broke this ? udc-core *always* calls -udc_start() by the time you load a gadget driver so this case will *never* happen. Whatever modification you did, broke this assumption and I will *not* accept this patch because the bug is elsewhere and *not* in mainline kernel. It's found in Android using kernel 3.10.20. Android has its own usb_composite_driver usb/gadget/android.c (not in mainline), and it so you found something on an old kernel using an out-of-tree gadget driver. allows userspace to disconnect the pullup (i.e clear run/stop bit in dwc3) and remove the gadget functions like adb, mtp and then add new functions like rndis, acm. The problem is when you disconnect the pullup, a gadget maybe in the middle of queuing a request, and result in the start transfer cmd failure. I think this is also a common issue for other Android gadget needs to learn how to cope with that. Agree. usb_composite_drivers too. Normally, if one of the gadget deactivate its own function, the pullup will be disconnected, other gadgets won't get notified until their requests are failed. So it makes dwc3 more robust to deal with these situations. Right, but Android gadget can run on top of several other UDCs and you want to have a single one of them cope with android's bug ? You'd be better off getting google to accept a bugfix to the android gadget, since that's where the problem lies. I agree. I'll try to push the fix to google. alright, thanks It's really hard to fix the race condition (for me), as any gadget or /sys/class/udc/soft_connect can just disconnect the pullup anytime they want. The only thing I can do is giving back the request to the gadget if the condition happens. even in that case, gadget driver's -disconnect() will be called and that should be enough to tell the gadget driver 'hey, don't queue anything right now because you're not talking to any host'. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 3/4] usb: ohci-exynos: Add facility to use phy provided by the generic phy framework
Hi Vivek,
This looks much better, but there still are some issues. Please see my
comments inline.
On 02.05.2014 14:47, Vivek Gautam wrote:
Add support to consume phy provided by Generic phy framework.
Keeping the support for older usb-phy intact right now, in order
to prevent any functionality break in absence of relevant
device tree side change for ohci-exynos.
Once we move to new phy in the device nodes for ohci, we can
remove the support for older phys.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Cc: Jingoo Han jg1@samsung.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
Changes from v4:
- Removed 'phy-names' property from the bindings since we don't need it.
- Restructured exynos_ohci_get_phy() function to handle error codes as
well as return relevant error codes effectively.
- Added IS_ERR() check for PHYs in exynos_ohci_phy_enable()/disable().
Changes from v3:
- Calling usb_phy_shutdown() when exynos_ohci_phy_enable() is failing.
- Made exynos_ohci_phy_disable() return void, since its return value
did not serve any purpose.
- Calling clk_disable_unprepare() in exynos_ohci_resume() when
exynos_ohci_phy_enable() is failed.
.../devicetree/bindings/usb/exynos-usb.txt | 16 +++
drivers/usb/host/ohci-exynos.c | 120 +---
2 files changed, 120 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..49a9c6f 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -38,6 +38,13 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are OHCI phys, they should be listed here.
+ One phy per port. Each port should have following entries:
+ - reg: port number on OHCI controller, e.g
+ On Exynos5250, port 0 is USB2.0 otg phy
+ port 1 is HSIC phy0
+ port 2 is HSIC phy1
+ - phys: from the *Generic PHY* bindings, specifying phy used by port.
Example:
usb@1212 {
@@ -47,6 +54,15 @@ Example:
clocks = clock 285;
clock-names = usbhost;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ port@0 {
+ reg = 0;
+ phys = usb2phy 1;
+ status = disabled;
+ };
+
};
DWC3
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 05f00e3..e66d391 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -18,6 +18,7 @@
#include linux/module.h
#include linux/of.h
#include linux/platform_device.h
+#include linux/phy/phy.h
#include linux/usb/phy.h
#include linux/usb/samsung_usb_phy.h
#include linux/usb.h
@@ -33,28 +34,112 @@ static struct hc_driver __read_mostly
exynos_ohci_hc_driver;
#define to_exynos_ohci(hcd) (struct exynos_ohci_hcd *)(hcd_to_ohci(hcd)-priv)
+#define PHY_NUMBER 3
+
struct exynos_ohci_hcd {
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
+ struct phy *phy_g[PHY_NUMBER];
};
-static void exynos_ohci_phy_enable(struct device *dev)
+static int exynos_ohci_get_phy(struct device *dev,
+ struct exynos_ohci_hcd *exynos_ohci)
+{
+ struct device_node *child;
+ struct phy *phy;
+ int phy_number;
+ int ret = 0;
+
+ exynos_ohci-phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(exynos_ohci-phy)) {
+ ret = PTR_ERR(exynos_ohci-phy);
+ if (ret != -ENXIO ret != -ENODEV) {
+ dev_err(dev, no usb2 phy configured\n);
+ return ret;
+ }
+ dev_dbg(dev, Failed to get usb2 phy\n);
+ exynos_ohci-phy = ERR_PTR(-ENODEV);
Here you already have exynos_ohci-phy set to an ERR_PTR() value, which
was returned by devm_usb_get_phy(). There is no need to overwrite it.
+ } else {
+ exynos_ohci-otg = exynos_ohci-phy-otg;
+ }
+
+ /*
+* Getting generic phy:
+* We are keeping both types of phys as a part of transiting OHCI
+* to generic phy framework, so as to maintain backward compatibilty
+* with old DTB.
+* If there are existing devices using DTB files built from them,
+* to remove the support for old bindings in this driver,
+* we need to make sure that such devices have their DTBs
+* updated to ones built from new DTS.
+*/
+ for_each_available_child_of_node(dev-of_node, child) {
+ ret = of_property_read_u32(child, reg, phy_number);
+
Re: [PATCH v7 14/16] usb: resume (wakeup) child device when port is powered on
On Thu, 1 May 2014, Dan Williams wrote: I've been testing this and the pm_request_resume() ends up leaving the usb device enabled indefinitely. It needs to be paired with a pm_runtime_autosuspend(), but at that point why not just add a usb_autoresume_device_async() helper. Why didn't pm_request_resume() result in an autosuspend? I just tested this patch on my system: Index: usb-3.15/drivers/usb/core/sysfs.c === --- usb-3.15.orig/drivers/usb/core/sysfs.c +++ usb-3.15/drivers/usb/core/sysfs.c @@ -11,6 +11,7 @@ #include linux/kernel.h +#include linux/pm_runtime.h #include linux/string.h #include linux/usb.h #include linux/usb/quirks.h @@ -55,6 +56,8 @@ static ssize_t bMaxPower_show(struct dev if (actconfig) rc = sprintf(buf, %dmA\n, usb_get_max_power(udev, actconfig)); usb_unlock_device(udev); + dev_info(dev, calling request_resume\n); + pm_request_resume(dev); return rc; } static DEVICE_ATTR_RO(bMaxPower); With no USB devices plugged into bus 1 and the root hub suspended, doing cat /sys/bus/usb/devices/usb1/bMaxPower caused the root hub to resume and suspend again shortly thereafter. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bugs in xhci-hcd isochronous support
On Tue, 22 Apr 2014, Russel Hughes wrote:
More importantly, the routine sets urb-start_frame to the current
value of the frame counter. This is completely wrong; urb-start_frame
is supposed to be the (micro-)frame number for when the transfer
begins, not when the transfer was submitted.
As far as I can tell, the only way to do this correctly is to set the
Frame ID field (with SIA = 0) in the first TD of an isochronous stream,
and then set SIA = 1 in all the following TDs (see 4.11.2.5). That
way, xhci-hcd will know exactly when the stream begins, so it can keep
track of the frame in which each URB starts. Dealing with underruns is
left as an exercise for the implementer...
Let me know if you want any changes tested using my DAC that reliably
shows the problem.
Russel, here's a patch you can test. It's only a partial fix for the
problem, because it doesn't handle over/underruns. Still, it would be
nice to see if the patch makes any difference in normal operation.
Even if it doesn't fix the problem, please post a short stretch (a few
hundred lines) from a usbmon trace with the patch installed.
Alan Stern
Index: usb-3.15/drivers/usb/host/xhci-ring.c
===
--- usb-3.15.orig/drivers/usb/host/xhci-ring.c
+++ usb-3.15/drivers/usb/host/xhci-ring.c
@@ -3153,7 +3153,7 @@ static void check_trb_math(struct urb *u
static void giveback_first_trb(struct xhci_hcd *xhci, int slot_id,
unsigned int ep_index, unsigned int stream_id, int start_cycle,
- struct xhci_generic_trb *start_trb)
+ struct xhci_generic_trb *start_trb, bool ring_doorbell)
{
/*
* Pass all the TRBs to the hardware at once and make sure this write
@@ -3164,7 +3164,8 @@ static void giveback_first_trb(struct xh
start_trb-field[3] |= cpu_to_le32(start_cycle);
else
start_trb-field[3] = cpu_to_le32(~TRB_CYCLE);
- xhci_ring_ep_doorbell(xhci, slot_id, ep_index, stream_id);
+ if (ring_doorbell)
+ xhci_ring_ep_doorbell(xhci, slot_id, ep_index, stream_id);
}
/*
@@ -3406,7 +3407,7 @@ static int queue_bulk_sg_tx(struct xhci_
check_trb_math(urb, num_trbs, running_total);
giveback_first_trb(xhci, slot_id, ep_index, urb-stream_id,
- start_cycle, start_trb);
+ start_cycle, start_trb, true);
return 0;
}
@@ -3545,7 +3546,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *
check_trb_math(urb, num_trbs, running_total);
giveback_first_trb(xhci, slot_id, ep_index, urb-stream_id,
- start_cycle, start_trb);
+ start_cycle, start_trb, true);
return 0;
}
@@ -3662,7 +3663,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *
field | TRB_IOC | TRB_TYPE(TRB_STATUS) |
ep_ring-cycle_state);
giveback_first_trb(xhci, slot_id, ep_index, 0,
- start_cycle, start_trb);
+ start_cycle, start_trb, true);
return 0;
}
@@ -3742,8 +3743,10 @@ static unsigned int xhci_get_last_burst_
/* This is for isoc transfer */
static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
- struct urb *urb, int slot_id, unsigned int ep_index)
+ struct urb *urb, int slot_id, unsigned int ep_index,
+ unsigned esit)
{
+ struct xhci_virt_ep *ep;
struct xhci_ring *ep_ring;
struct urb_priv *urb_priv;
struct xhci_td *td;
@@ -3756,8 +3759,11 @@ static int xhci_queue_isoc_tx(struct xhc
u64 start_addr, addr;
int i, j;
bool more_trbs_coming;
+ bool new_isoc_stream;
+ unsigned start_uframe;
- ep_ring = xhci-devs[slot_id]-eps[ep_index].ring;
+ ep = xhci-devs[slot_id]-eps[ep_index];
+ ep_ring = ep-ring;
num_tds = urb-number_of_packets;
if (num_tds 1) {
@@ -3765,6 +3771,8 @@ static int xhci_queue_isoc_tx(struct xhc
return -EINVAL;
}
+ new_isoc_stream = list_empty(urb-ep-urb_list);
+
start_addr = (u64) urb-transfer_dma;
start_trb = ep_ring-enqueue-generic;
start_cycle = ep_ring-cycle_state;
@@ -3826,10 +3834,6 @@ static int xhci_queue_isoc_tx(struct xhc
field |= ep_ring-cycle_state;
}
- /* Only set interrupt on short packet for IN EPs */
- if (usb_urb_dir_in(urb))
- field |= TRB_ISP;
-
/* Chain all the TRBs together; clear the chain bit in
* the last TRB to indicate it's the last TRB in the
* chain.
@@ -3895,8 +3899,52 @@ static int xhci_queue_isoc_tx(struct xhc
}
xhci_to_hcd(xhci)-self.bandwidth_isoc_reqs++;
+ /*
+* For new
[RFC 1/9] 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.rc0
--
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
[RFC 4/9] 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.rc0
--
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
[RFC 5/9] 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
[RFC 8/9] 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,
[RFC 7/9] 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.rc0
--
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
[RFC 2/9] 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
[RFC 3/9] 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.rc0
--
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
[RFC 0/9] cdc_ncm: add buffer tuning and stats using ethtool
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! Bjørn Mork (9): 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 drivers/net/usb/cdc_mbim.c | 6 + drivers/net/usb/cdc_ncm.c | 545 +--- include/linux/usb/cdc_ncm.h | 32 ++- 3 files changed, 434 insertions(+), 149 deletions(-) -- 2.0.0.rc0 -- 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
[RFC 6/9] 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.rc0
--
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,stable] net: cdc_ncm: fix buffer overflow
Commit 4d619f625a60 (net: cdc_ncm: no point in filling up the NTBs if we send ZLPs) changed the padding logic for devices with the ZLP flag set. This meant that frames of any size will be sent without additional padding, except for the single byte added if the size is a multiple of the USB packet size. But if the unpadded size is identical to the maximum frame size, and the maximum size is a multiplum of the USB packet size, then this one-byte padding will overflow the buffer. Prevent padding if already at maximum frame size, letting usbnet transmit a ZLP instead in this case. Fixes: 4d619f625a60 (net: cdc_ncm: no point in filling up the NTBs if we send ZLPs) Reported by: Yu-an Shih ys...@nvidia.com Signed-off-by: Bjørn Mork bj...@mork.no --- Please add this to the stable v3.13 and v3.14 queues as well. Thanks. drivers/net/usb/cdc_ncm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 549dbac710ed..9a2bd11943eb 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -785,7 +785,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign) skb_out-len CDC_NCM_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) + else if (skb_out-len ctx-tx_max (skb_out-len % dev-maxpacket) == 0) *skb_put(skb_out, 1) = 0; /* force short packet */ /* set final frame length */ -- 2.0.0.rc0 -- 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,stable] net: cdc_mbim: __vlan_find_dev_deep need rcu_read_lock
Fixes this warning introduced by commit 5b8f15f78e6f
(net: cdc_mbim: handle IPv6 Neigbor Solicitations):
===
[ INFO: suspicious RCU usage. ]
3.15.0-rc3 #213 Tainted: GW O
---
net/8021q/vlan_core.c:69 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 1
no locks held by ksoftirqd/0/3.
stack backtrace:
CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: GW O 3.15.0-rc3 #213
Hardware name: LENOVO 2776LEG/2776LEG, BIOS 6EET55WW (3.15 ) 12/19/2011
0001 880232533bf0 813a5ee6 0006
880232530090 880232533c20 81076b94 0081
8802085ac000 88007fc8ea00 880232533c50
Call Trace:
[813a5ee6] dump_stack+0x4e/0x68
[81076b94] lockdep_rcu_suspicious+0xfa/0x103
[813978a6] __vlan_find_dev_deep+0x54/0x94
[a04a1938] cdc_mbim_rx_fixup+0x379/0x66a [cdc_mbim]
[813ab76f] ? _raw_spin_unlock_irqrestore+0x3a/0x49
[81079671] ? trace_hardirqs_on_caller+0x192/0x1a1
[a059bd10] usbnet_bh+0x59/0x287 [usbnet]
[8104067d] tasklet_action+0xbb/0xcd
[81040057] __do_softirq+0x14c/0x30d
[81040237] run_ksoftirqd+0x1f/0x50
[8105f13e] smpboot_thread_fn+0x172/0x18e
[8105efcc] ? SyS_setgroups+0xdf/0xdf
[810594b0] kthread+0xb5/0xbd
[813a84b1] ? __wait_for_common+0x13b/0x170
[810593fb] ? __kthread_parkme+0x5c/0x5c
[813b147c] ret_from_fork+0x7c/0xb0
[810593fb] ? __kthread_parkme+0x5c/0x5c
Fixes: 5b8f15f78e6f (net: cdc_mbim: handle IPv6 Neigbor Solicitations)
Signed-off-by: Bjørn Mork bj...@mork.no
---
Please add this to the stable v3.13 and v3.14 queues as well. Thanks.
drivers/net/usb/cdc_mbim.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index c9f3281506af..34b5c5cc27ed 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -204,13 +204,16 @@ static void do_neigh_solicit(struct usbnet *dev, u8 *buf,
u16 tci)
return;
/* need to send the NA on the VLAN dev, if any */
- if (tci)
+ if (tci) {
+ rcu_read_lock();
netdev = __vlan_find_dev_deep(dev-net, htons(ETH_P_8021Q),
- tci);
- else
+ tci VLAN_VID_MASK);
+ rcu_read_unlock();
+ if (!netdev)
+ return;
+ } else {
netdev = dev-net;
- if (!netdev)
- return;
+ }
in6_dev = in6_dev_get(netdev);
if (!in6_dev)
--
2.0.0.rc0
--
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 net,stable] net: cdc_mbim: __vlan_find_dev_deep need rcu_read_lock
On Fri, 2014-05-02 at 23:28 +0200, Bjørn Mork wrote:
Fixes this warning introduced by commit 5b8f15f78e6f
(net: cdc_mbim: handle IPv6 Neigbor Solicitations):
===
[ INFO: suspicious RCU usage. ]
3.15.0-rc3 #213 Tainted: GW O
---
net/8021q/vlan_core.c:69 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 1
no locks held by ksoftirqd/0/3.
stack backtrace:
CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: GW O 3.15.0-rc3 #213
Hardware name: LENOVO 2776LEG/2776LEG, BIOS 6EET55WW (3.15 ) 12/19/2011
0001 880232533bf0 813a5ee6 0006
880232530090 880232533c20 81076b94 0081
8802085ac000 88007fc8ea00 880232533c50
Call Trace:
[813a5ee6] dump_stack+0x4e/0x68
[81076b94] lockdep_rcu_suspicious+0xfa/0x103
[813978a6] __vlan_find_dev_deep+0x54/0x94
[a04a1938] cdc_mbim_rx_fixup+0x379/0x66a [cdc_mbim]
[813ab76f] ? _raw_spin_unlock_irqrestore+0x3a/0x49
[81079671] ? trace_hardirqs_on_caller+0x192/0x1a1
[a059bd10] usbnet_bh+0x59/0x287 [usbnet]
[8104067d] tasklet_action+0xbb/0xcd
[81040057] __do_softirq+0x14c/0x30d
[81040237] run_ksoftirqd+0x1f/0x50
[8105f13e] smpboot_thread_fn+0x172/0x18e
[8105efcc] ? SyS_setgroups+0xdf/0xdf
[810594b0] kthread+0xb5/0xbd
[813a84b1] ? __wait_for_common+0x13b/0x170
[810593fb] ? __kthread_parkme+0x5c/0x5c
[813b147c] ret_from_fork+0x7c/0xb0
[810593fb] ? __kthread_parkme+0x5c/0x5c
Fixes: 5b8f15f78e6f (net: cdc_mbim: handle IPv6 Neigbor Solicitations)
Signed-off-by: Bjørn Mork bj...@mork.no
---
Please add this to the stable v3.13 and v3.14 queues as well. Thanks.
drivers/net/usb/cdc_mbim.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index c9f3281506af..34b5c5cc27ed 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -204,13 +204,16 @@ static void do_neigh_solicit(struct usbnet *dev, u8
*buf, u16 tci)
return;
/* need to send the NA on the VLAN dev, if any */
- if (tci)
+ if (tci) {
+ rcu_read_lock();
netdev = __vlan_find_dev_deep(dev-net, htons(ETH_P_8021Q),
- tci);
- else
+ tci VLAN_VID_MASK);
+ rcu_read_unlock();
+ if (!netdev)
+ return;
+ } else {
netdev = dev-net;
- if (!netdev)
- return;
+ }
in6_dev = in6_dev_get(netdev);
if (!in6_dev)
While this 'removes' the warning, this doesn't solve the fundamental
problem.
If you write :
rcu_read_lock();
netdev = __vlan_find_dev_deep(...)
rcu_read_unlock();
Then you cannot dereference netdev safely after the unlock.
In order to do so, you need to take a reference on netdev (aka
dev_hold()) before doing rcu_read_unlock();
And of course, release it later (aka dev_put()) when you are done with
netdev.
--
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: Status of PLX USB3380?
On Fri, May 02, 2014 at 02:11:30PM +0200, Ricardo Ribalda Delgado wrote: Hello I have seen on the list that there was a previous interest on giving support for this chipset. Is anyone currently working on it? Apparently the chip can be configured in legacy mode being register compatible with the net2280 chipset. Has this been tested? The driver from the manufacturer explicitly mentions LGPLv2 and GPL, so I guess they can be used for inspiration. We use the PLX USB3380 here with the driver from the manufacturer. Their drivers are tested for a specific kernel but its not too hard to compile them using the latest kernels. We have got them to work in 3.13 with some changes in the module cleanup to keep it from crashing. Regarding functionality, it works as expected. Hope this helps, Amit -- 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: Status of PLX USB3380?
Hi, On Fri, May 02, 2014 at 03:22:07PM -0700, Amit Uttamchandani wrote: On Fri, May 02, 2014 at 02:11:30PM +0200, Ricardo Ribalda Delgado wrote: Hello I have seen on the list that there was a previous interest on giving support for this chipset. Is anyone currently working on it? Apparently the chip can be configured in legacy mode being register compatible with the net2280 chipset. Has this been tested? The driver from the manufacturer explicitly mentions LGPLv2 and GPL, so I guess they can be used for inspiration. We use the PLX USB3380 here with the driver from the manufacturer. Their drivers are tested for a specific kernel but its not too hard to compile them using the latest kernels. We have got them to work in 3.13 with some changes in the module cleanup to keep it from crashing. Regarding functionality, it works as expected. if original driver is GPL, how about sending it for inclusion in mainline ? If anybody wants to sponsor one USB3380 for my testing, I'd be very thankful too as that'll help me make sure things get tested before going upstream. BTW, do you use the one with a single port or the one with 2 ports ? cheers -- balbi signature.asc Description: Digital signature
[PATCH] USB: ehci-hub: wait for RESUME finished when hub try to clear SUSPEND
We use usb ehci to connect with modem and run stress test on ehci
remote wake. Sometimes usb disconnect. We add more debug ftrace
(Kernel version: 3.10) and list the key log to show how problem
happened.
idle-0 [000] d.h2 26879.385095: ehci_irq: irq status 1008c PPCE FLR PCD
idle-0 [000] d.h2 26879.385099: ehci_irq: rh_state[2] hcd-state[132]
pstatus[0][238014c5] suspended_ports[1] reset_done[0]
...-12873 [000] d..1 26879.393536: ehci_hub_control: GetStatus port:1 status
238014c5 17 ERR POWER sig=k SUSPEND RESUME PE CONNECT
...-12873 [000] d..1 26879.393549: ehci_hub_control: typeReq [2301] wIndex[1]
wValue[2]
...-12873 [000] d..1 26879.393553: ehci_hub_control:
[ehci_hub_control]line[891] port[0] hostpc_reg [44000202]-[44000202]
idle-0 [001] ..s. 26879.403122: ehci_hub_status_data:
wgq[ehci_hub_status_data] ignore_oc[0] resuming_ports[1]
...-12873 [000] d..1 26879.413379: ehci_hub_control:
[ehci_hub_control]line[907] port[0] write portsc_reg[238014c5]
reset_done[2105769]
...-12873 [000] d..1 26879.453173: ehci_hub_control: GetStatus port:1 status
23801885 17 ERR POWER sig=j SUSPEND PE CONNECT
...-12873 [000] 26879.473158: check_port_resume_type: port 1 status
.0507 after resume, -19
...-12873 [000] 26879.473160: usb_port_resume: status = -19 after
check_port_resume_type
...-12873 [000] 26879.473161: usb_port_resume: can't resume, status -19
...-12873 [000] 26879.473162: hub_port_logical_disconnect: logical
disconnect on port 1
There is a in-band remote wakeup and controller run in k-state. Then kernel
driver(ClearPortFeature/USB_PORT_FEAT_SUSPEND) write RESUME|LS(k-state) bit
into controller. It makes controller status weird. It's defined in EHCI
controller spec(Revision 1.0), If it has enabled remote wake-up, a K-state
on the bus will turn the transceiver clock and generate an interrupt. The
software will then have to wait 20 ms for the resume to complete and the
port to go back to an active state. In this case Kernel should wait for
the wakeup finished, then judge what should do next step.
We have some thought and give a patch. This patch is to wait for controller
RESUME finished when hub try to clear port SUSPEND feature.
Signed-off-by: xiao jin jin.x...@intel.com
Reviewed-by: David Cohen david.a.co...@linux.intel.com
---
drivers/usb/host/ehci-hub.c |7 +++
include/linux/usb/ehci_def.h |5 +
2 files changed, 12 insertions(+)
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 7ae0c4d..09a8b6b 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -935,6 +935,13 @@ static int ehci_hub_control (
break;
}
#endif
+ if ((temp PORT_RESUME)
+((temp PORT_LS_MASK) == PORT_K_STATE)) {
+ ehci_handshake(ehci, status_reg,
+ PORT_RESUME, 0, 2 /* 20msec */);
+ temp = ehci_readl(ehci, status_reg);
+ temp = ~PORT_RWC_BITS;
+ }
if (!(temp PORT_SUSPEND))
break;
if ((temp PORT_PE) == 0)
diff --git a/include/linux/usb/ehci_def.h b/include/linux/usb/ehci_def.h
index daec99a..0f0f919 100644
--- a/include/linux/usb/ehci_def.h
+++ b/include/linux/usb/ehci_def.h
@@ -149,6 +149,11 @@ struct ehci_regs {
#define PORT_POWER (112) /* true: has power (see PPC) */
#define PORT_USB11(x) (((x)(310)) == (110)) /* USB 1.1 device */
/* 11:10 for detecting lowspeed devices (reset vs release ownership) */
+#define PORT_LS_MASK (0x310) /* line status */
+#define PORT_SE0_STATE (010)
+#define PORT_K_STATE (110)
+#define PORT_J_STATE (210)
+#define PORT_UNDEFINED_STATE (310)
/* 9 reserved */
#define PORT_LPM (19) /* LPM transaction */
#define PORT_RESET (18) /* reset port */
--
1.7.9.5
--
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
