[PATCH v2 2/6] usb: phy: msm: move global regulators variables to driver state
From: "Ivan T. Ivanov"
Signed-off-by: Ivan T. Ivanov
---
drivers/usb/phy/phy-msm-usb.c | 82 -
include/linux/usb/msm_hsusb.h |3 ++
2 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 7b74b26..6cc4801c 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -58,47 +58,43 @@
#define USB_PHY_VDD_DIG_VOL_MIN100 /* uV */
#define USB_PHY_VDD_DIG_VOL_MAX132 /* uV */
-static struct regulator *hsusb_3p3;
-static struct regulator *hsusb_1p8;
-static struct regulator *hsusb_vddcx;
-
static int msm_hsusb_init_vddcx(struct msm_otg *motg, int init)
{
int ret = 0;
if (init) {
- hsusb_vddcx = regulator_get(motg->phy.dev, "HSUSB_VDDCX");
- if (IS_ERR(hsusb_vddcx)) {
+ motg->vddcx = regulator_get(motg->phy.dev, "HSUSB_VDDCX");
+ if (IS_ERR(motg->vddcx)) {
dev_err(motg->phy.dev, "unable to get hsusb vddcx\n");
- return PTR_ERR(hsusb_vddcx);
+ return PTR_ERR(motg->vddcx);
}
- ret = regulator_set_voltage(hsusb_vddcx,
+ ret = regulator_set_voltage(motg->vddcx,
USB_PHY_VDD_DIG_VOL_MIN,
USB_PHY_VDD_DIG_VOL_MAX);
if (ret) {
dev_err(motg->phy.dev, "unable to set the voltage "
"for hsusb vddcx\n");
- regulator_put(hsusb_vddcx);
+ regulator_put(motg->vddcx);
return ret;
}
- ret = regulator_enable(hsusb_vddcx);
+ ret = regulator_enable(motg->vddcx);
if (ret) {
dev_err(motg->phy.dev, "unable to enable hsusb
vddcx\n");
- regulator_put(hsusb_vddcx);
+ regulator_put(motg->vddcx);
}
} else {
- ret = regulator_set_voltage(hsusb_vddcx, 0,
+ ret = regulator_set_voltage(motg->vddcx, 0,
USB_PHY_VDD_DIG_VOL_MAX);
if (ret)
dev_err(motg->phy.dev, "unable to set the voltage "
"for hsusb vddcx\n");
- ret = regulator_disable(hsusb_vddcx);
+ ret = regulator_disable(motg->vddcx);
if (ret)
dev_err(motg->phy.dev, "unable to disable hsusb
vddcx\n");
- regulator_put(hsusb_vddcx);
+ regulator_put(motg->vddcx);
}
return ret;
@@ -109,38 +105,38 @@ static int msm_hsusb_ldo_init(struct msm_otg *motg, int
init)
int rc = 0;
if (init) {
- hsusb_3p3 = regulator_get(motg->phy.dev, "HSUSB_3p3");
- if (IS_ERR(hsusb_3p3)) {
+ motg->v3p3 = regulator_get(motg->phy.dev, "HSUSB_3p3");
+ if (IS_ERR(motg->v3p3)) {
dev_err(motg->phy.dev, "unable to get hsusb 3p3\n");
- return PTR_ERR(hsusb_3p3);
+ return PTR_ERR(motg->v3p3);
}
- rc = regulator_set_voltage(hsusb_3p3, USB_PHY_3P3_VOL_MIN,
+ rc = regulator_set_voltage(motg->v3p3, USB_PHY_3P3_VOL_MIN,
USB_PHY_3P3_VOL_MAX);
if (rc) {
dev_err(motg->phy.dev, "unable to set voltage level "
"for hsusb 3p3\n");
goto put_3p3;
}
- rc = regulator_enable(hsusb_3p3);
+ rc = regulator_enable(motg->v3p3);
if (rc) {
dev_err(motg->phy.dev, "unable to enable the hsusb
3p3\n");
goto put_3p3;
}
- hsusb_1p8 = regulator_get(motg->phy.dev, "HSUSB_1p8");
- if (IS_ERR(hsusb_1p8)) {
+ motg->v1p8 = regulator_get(motg->phy.dev, "HSUSB_1p8");
+ if (IS_ERR(motg->v1p8)) {
dev_err(motg->phy.dev, "unable to get hsusb 1p8\n");
- rc = PTR_ERR(hsusb_1p8);
+ rc = PTR_ERR(motg->v1p8);
goto disable_3p3;
}
- rc = regulator_set_voltage(hsusb_1p8, USB_PHY_1P8_VOL_MIN,
+ rc = regulator_set_voltage(motg->v1p8, USB_PHY_1P8_VOL_MIN,
USB_PHY_1P8_VOL_MAX);
if (rc) {
dev_err(motg->phy.dev, "unable to set voltage level "
"for hsusb 1p8\n");
goto put_1p8;
}
- rc = regulator_enable(hsusb_1p8);
+ rc =
[PATCH v2 4/6] usb: phy: msm: Remove unnecessarily check for valid regulators.
From: "Ivan T. Ivanov"
Whether regulators are available or not is checked at driver
probe. If they are not available driver will refuse to load,
so no need to check them again.
Signed-off-by: Ivan T. Ivanov
---
drivers/usb/phy/phy-msm-usb.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index c72f7c1..0e93dbc 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -159,16 +159,6 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg,
int on)
{
int ret = 0;
- if (!motg->v1p8 || IS_ERR(motg->v1p8)) {
- pr_err("%s: HSUSB_1p8 is not initialized\n", __func__);
- return -ENODEV;
- }
-
- if (!motg->v3p3 || IS_ERR(motg->v3p3)) {
- pr_err("%s: HSUSB_3p3 is not initialized\n", __func__);
- return -ENODEV;
- }
-
if (on) {
ret = regulator_set_optimum_mode(motg->v1p8,
USB_PHY_1P8_HPM_LOAD);
--
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
[PATCH v2 0/6] usb: phy: msm: Fixes and cleanups
From: "Ivan T. Ivanov" v2 -- * Fix compilation issue in patch 1 * Separate regulator changes * Merge devm_ related changes to one commit * Drop Lindent patch v1 -- Following patches make initial cleanup of usb phy found in the Qualcomm chipsets. Changes include: * Build time error fix. * Move driver to Managed Device Resource allocation. * Checkpatch warnings and error fixes * Removed usage of global regulators variables. Ivan T. Ivanov (6): usb: phy: msm: Move mach depndend code to platform data usb: phy: msm: move global regulators variables to driver state usb: phy: msm: Migrate to Managed Device Resource allocation usb: phy: msm: Remove unnecessarily check for valid regulators. usb: phy: msm: Fix WARNING: quoted string split across lines usb: phy: msm: Fix WARNING: Prefer seq_puts to seq_printf arch/arm/mach-msm/board-msm7x30.c | 35 arch/arm/mach-msm/board-qsd8x50.c | 35 drivers/usb/phy/phy-msm-usb.c | 342 + include/linux/usb/msm_hsusb.h |6 + 4 files changed, 198 insertions(+), 220 deletions(-) -- 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
[PATCH v2 5/6] usb: phy: msm: Fix WARNING: quoted string split across lines
From: "Ivan T. Ivanov"
This fixes checkpatch.pl warnings.
Signed-off-by: Ivan T. Ivanov
---
drivers/usb/phy/phy-msm-usb.c | 33 +++--
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 0e93dbc..118bd0a 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -67,8 +67,7 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
USB_PHY_VDD_DIG_VOL_MIN,
USB_PHY_VDD_DIG_VOL_MAX);
if (ret) {
- dev_err(motg->phy.dev, "unable to set the voltage "
- "for hsusb vddcx\n");
+ dev_err(motg->phy.dev, "Cannot set vddcx voltage\n");
return ret;
}
@@ -79,8 +78,7 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
ret = regulator_set_voltage(motg->vddcx, 0,
USB_PHY_VDD_DIG_VOL_MAX);
if (ret)
- dev_err(motg->phy.dev, "unable to set the voltage "
- "for hsusb vddcx\n");
+ dev_err(motg->phy.dev, "Cannot set vddcx voltage\n");
ret = regulator_disable(motg->vddcx);
if (ret)
dev_err(motg->phy.dev, "unable to disable hsusb
vddcx\n");
@@ -97,8 +95,7 @@ static int msm_hsusb_ldo_init(struct msm_otg *motg, int init)
rc = regulator_set_voltage(motg->v3p3, USB_PHY_3P3_VOL_MIN,
USB_PHY_3P3_VOL_MAX);
if (rc) {
- dev_err(motg->phy.dev, "unable to set voltage level "
- "for hsusb 3p3\n");
+ dev_err(motg->phy.dev, "Cannot set v3p3 voltage\n");
goto exit;
}
rc = regulator_enable(motg->v3p3);
@@ -109,8 +106,7 @@ static int msm_hsusb_ldo_init(struct msm_otg *motg, int
init)
rc = regulator_set_voltage(motg->v1p8, USB_PHY_1P8_VOL_MIN,
USB_PHY_1P8_VOL_MAX);
if (rc) {
- dev_err(motg->phy.dev, "unable to set voltage level "
- "for hsusb 1p8\n");
+ dev_err(motg->phy.dev, "Cannot set v1p8 voltage\n");
goto disable_3p3;
}
rc = regulator_enable(motg->v1p8);
@@ -144,8 +140,7 @@ static int msm_hsusb_config_vddcx(struct msm_otg *motg, int
high)
ret = regulator_set_voltage(motg->vddcx, min_vol, max_vol);
if (ret) {
- pr_err("%s: unable to set the voltage for regulator "
- "HSUSB_VDDCX\n", __func__);
+ dev_err(motg->phy.dev, "Cannot set vddcx voltage\n");
return ret;
}
@@ -163,15 +158,13 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg,
int on)
ret = regulator_set_optimum_mode(motg->v1p8,
USB_PHY_1P8_HPM_LOAD);
if (ret < 0) {
- pr_err("%s: Unable to set HPM of the regulator "
- "HSUSB_1p8\n", __func__);
+ pr_err("Could not set HPM for v1p8\n");
return ret;
}
ret = regulator_set_optimum_mode(motg->v3p3,
USB_PHY_3P3_HPM_LOAD);
if (ret < 0) {
- pr_err("%s: Unable to set HPM of the regulator "
- "HSUSB_3p3\n", __func__);
+ pr_err("Could not set HPM for v3p3\n");
regulator_set_optimum_mode(motg->v1p8,
USB_PHY_1P8_LPM_LOAD);
return ret;
@@ -180,13 +173,11 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg,
int on)
ret = regulator_set_optimum_mode(motg->v1p8,
USB_PHY_1P8_LPM_LOAD);
if (ret < 0)
- pr_err("%s: Unable to set LPM of the regulator "
- "HSUSB_1p8\n", __func__);
+ pr_err("Could not set LPM for v1p8\n");
ret = regulator_set_optimum_mode(motg->v3p3,
USB_PHY_3P3_LPM_LOAD);
if (ret < 0)
- pr_err("%s: Unable to set LPM of the regulator "
- "HSUSB_3p3\n", __func__);
+ pr_err("Could not set LPM for v3p3\n");
}
pr_debug("reg (%s)\n", on ? "HPM" : "LPM");
@@ -519,8 +510,7 @@ static int msm_otg_resume(struct msm_otg *motg)
* PHY. USB state can not be restored. Re-insertion
* of USB cable is
[PATCH v2 3/6] usb: phy: msm: Migrate to Managed Device Resource allocation
From: "Ivan T. Ivanov"
Move memory, regulators, clocks and irq allocation to
devm_* variants. Properly check for valid clk handles.
Signed-off-by: Ivan T. Ivanov
---
drivers/usb/phy/phy-msm-usb.c | 188 -
1 file changed, 71 insertions(+), 117 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 6cc4801c..c72f7c1 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -63,27 +63,18 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
int ret = 0;
if (init) {
- motg->vddcx = regulator_get(motg->phy.dev, "HSUSB_VDDCX");
- if (IS_ERR(motg->vddcx)) {
- dev_err(motg->phy.dev, "unable to get hsusb vddcx\n");
- return PTR_ERR(motg->vddcx);
- }
-
ret = regulator_set_voltage(motg->vddcx,
USB_PHY_VDD_DIG_VOL_MIN,
USB_PHY_VDD_DIG_VOL_MAX);
if (ret) {
dev_err(motg->phy.dev, "unable to set the voltage "
"for hsusb vddcx\n");
- regulator_put(motg->vddcx);
return ret;
}
ret = regulator_enable(motg->vddcx);
- if (ret) {
+ if (ret)
dev_err(motg->phy.dev, "unable to enable hsusb
vddcx\n");
- regulator_put(motg->vddcx);
- }
} else {
ret = regulator_set_voltage(motg->vddcx, 0,
USB_PHY_VDD_DIG_VOL_MAX);
@@ -93,8 +84,6 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
ret = regulator_disable(motg->vddcx);
if (ret)
dev_err(motg->phy.dev, "unable to disable hsusb
vddcx\n");
-
- regulator_put(motg->vddcx);
}
return ret;
@@ -105,53 +94,38 @@ static int msm_hsusb_ldo_init(struct msm_otg *motg, int
init)
int rc = 0;
if (init) {
- motg->v3p3 = regulator_get(motg->phy.dev, "HSUSB_3p3");
- if (IS_ERR(motg->v3p3)) {
- dev_err(motg->phy.dev, "unable to get hsusb 3p3\n");
- return PTR_ERR(motg->v3p3);
- }
-
rc = regulator_set_voltage(motg->v3p3, USB_PHY_3P3_VOL_MIN,
USB_PHY_3P3_VOL_MAX);
if (rc) {
dev_err(motg->phy.dev, "unable to set voltage level "
"for hsusb 3p3\n");
- goto put_3p3;
+ goto exit;
}
rc = regulator_enable(motg->v3p3);
if (rc) {
dev_err(motg->phy.dev, "unable to enable the hsusb
3p3\n");
- goto put_3p3;
- }
- motg->v1p8 = regulator_get(motg->phy.dev, "HSUSB_1p8");
- if (IS_ERR(motg->v1p8)) {
- dev_err(motg->phy.dev, "unable to get hsusb 1p8\n");
- rc = PTR_ERR(motg->v1p8);
- goto disable_3p3;
+ goto exit;
}
rc = regulator_set_voltage(motg->v1p8, USB_PHY_1P8_VOL_MIN,
USB_PHY_1P8_VOL_MAX);
if (rc) {
dev_err(motg->phy.dev, "unable to set voltage level "
"for hsusb 1p8\n");
- goto put_1p8;
+ goto disable_3p3;
}
rc = regulator_enable(motg->v1p8);
if (rc) {
dev_err(motg->phy.dev, "unable to enable the hsusb
1p8\n");
- goto put_1p8;
+ goto disable_3p3;
}
return 0;
}
regulator_disable(motg->v1p8);
-put_1p8:
- regulator_put(motg->v1p8);
disable_3p3:
regulator_disable(motg->v3p3);
-put_3p3:
- regulator_put(motg->v3p3);
+exit:
return rc;
}
@@ -479,7 +453,7 @@ static int msm_otg_suspend(struct msm_otg *motg)
clk_disable_unprepare(motg->pclk);
clk_disable_unprepare(motg->clk);
- if (motg->core_clk)
+ if (!IS_ERR(motg->core_clk))
clk_disable_unprepare(motg->core_clk);
if (!IS_ERR(motg->pclk_src))
@@ -519,7 +493,7 @@ static int msm_otg_resume(struct msm_otg *motg)
clk_prepare_enable(motg->pclk);
clk_prepare_enable(motg->clk);
- if (motg->core_clk)
+ if (!IS_ERR(motg->core_clk))
clk_prepare_enable(motg->core_clk);
if (motg->pdata->phy_type == SNPS_28NM_INTEGRATED_PHY &&
@@ -1393,13 +1367,14 @@ static int __init msm_otg_probe(struct platform_device
*pdev
[PATCH v2 6/6] usb: phy: msm: Fix WARNING: Prefer seq_puts to seq_printf
From: "Ivan T. Ivanov"
This fixes checkpatch.pl warnings.
Signed-off-by: Ivan T. Ivanov
---
drivers/usb/phy/phy-msm-usb.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 118bd0a..e370435 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1204,13 +1204,13 @@ static int msm_otg_mode_show(struct seq_file *s, void
*unused)
switch (otg->phy->state) {
case OTG_STATE_A_HOST:
- seq_printf(s, "host\n");
+ seq_puts(s, "host\n");
break;
case OTG_STATE_B_PERIPHERAL:
- seq_printf(s, "peripheral\n");
+ seq_puts(s, "peripheral\n");
break;
default:
- seq_printf(s, "none\n");
+ seq_puts(s, "none\n");
break;
}
--
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
[PATCH v2 1/6] usb: phy: msm: Move mach depndend code to platform data
From: "Ivan T. Ivanov"
This patch fix compilation error and is an intermediate step
before the addition of DeviceTree support for newer targets.
Fix suggested here: https://lkml.org/lkml/2013/6/19/381
Cc: David Brown
Cc: Daniel Walker
Cc: Bryan Huntsman
Cc: Felipe Balbi
Cc: Greg Kroah-Hartman
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Signed-off-by: Ivan T. Ivanov
---
arch/arm/mach-msm/board-msm7x30.c | 35 +++
arch/arm/mach-msm/board-qsd8x50.c | 35 +++
drivers/usb/phy/phy-msm-usb.c | 55 ++---
include/linux/usb/msm_hsusb.h |3 ++
4 files changed, 87 insertions(+), 41 deletions(-)
diff --git a/arch/arm/mach-msm/board-msm7x30.c
b/arch/arm/mach-msm/board-msm7x30.c
index db3d8c0..6b35953 100644
--- a/arch/arm/mach-msm/board-msm7x30.c
+++ b/arch/arm/mach-msm/board-msm7x30.c
@@ -31,6 +31,7 @@
#include
#include
+#include
#include
#include
@@ -61,10 +62,44 @@ static int hsusb_phy_init_seq[] = {
-1
};
+static int hsusb_link_clk_reset(struct clk *link_clk, bool assert)
+{
+ int ret;
+
+ if (assert) {
+ ret = clk_reset(link_clk, CLK_RESET_ASSERT);
+ if (ret)
+ pr_err("usb hs_clk assert failed\n");
+ } else {
+ ret = clk_reset(link_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err("usb hs_clk deassert failed\n");
+ }
+ return ret;
+}
+
+static int hsusb_phy_clk_reset(struct clk *phy_clk)
+{
+ int ret;
+
+ ret = clk_reset(phy_clk, CLK_RESET_ASSERT);
+ if (ret) {
+ pr_err("usb phy clk assert failed\n");
+ return ret;
+ }
+ usleep_range(1, 12000);
+ ret = clk_reset(phy_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err("usb phy clk deassert failed\n");
+ return ret;
+}
+
static struct msm_otg_platform_data msm_otg_pdata = {
.phy_init_seq = hsusb_phy_init_seq,
.mode = USB_PERIPHERAL,
.otg_control= OTG_PHY_CONTROL,
+ .link_clk_reset = hsusb_link_clk_reset,
+ .phy_clk_reset = hsusb_phy_clk_reset,
};
struct msm_gpiomux_config msm_gpiomux_configs[GPIOMUX_NGPIOS] = {
diff --git a/arch/arm/mach-msm/board-qsd8x50.c
b/arch/arm/mach-msm/board-qsd8x50.c
index f14a73d..eb013f7 100644
--- a/arch/arm/mach-msm/board-qsd8x50.c
+++ b/arch/arm/mach-msm/board-qsd8x50.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include "devices.h"
@@ -82,10 +83,44 @@ static int hsusb_phy_init_seq[] = {
-1
};
+static int hsusb_link_clk_reset(struct clk *link_clk, bool assert)
+{
+ int ret;
+
+ if (assert) {
+ ret = clk_reset(link_clk, CLK_RESET_ASSERT);
+ if (ret)
+ pr_err("usb hs_clk assert failed\n");
+ } else {
+ ret = clk_reset(link_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err("usb hs_clk deassert failed\n");
+ }
+ return ret;
+}
+
+static int hsusb_phy_clk_reset(struct clk *phy_clk)
+{
+ int ret;
+
+ ret = clk_reset(phy_clk, CLK_RESET_ASSERT);
+ if (ret) {
+ pr_err("usb phy clk assert failed\n");
+ return ret;
+ }
+ usleep_range(1, 12000);
+ ret = clk_reset(phy_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err("usb phy clk deassert failed\n");
+ return ret;
+}
+
static struct msm_otg_platform_data msm_otg_pdata = {
.phy_init_seq = hsusb_phy_init_seq,
.mode = USB_PERIPHERAL,
.otg_control= OTG_PHY_CONTROL,
+ .link_clk_reset = hsusb_link_clk_reset,
+ .phy_clk_reset = hsusb_phy_clk_reset,
};
static struct platform_device *devices[] __initdata = {
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index d08f334..7b74b26 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -40,8 +40,6 @@
#include
#include
-#include
-
#define MSM_USB_BASE (motg->regs)
#define DRIVER_NAME"msm_otg"
@@ -306,51 +304,20 @@ static void ulpi_init(struct msm_otg *motg)
}
}
-static int msm_otg_link_clk_reset(struct msm_otg *motg, bool assert)
-{
- int ret;
-
- if (assert) {
- ret = clk_reset(motg->clk, CLK_RESET_ASSERT);
- if (ret)
- dev_err(motg->phy.dev, "usb hs_clk assert failed\n");
- } else {
- ret = clk_reset(motg->clk, CLK_RESET_DEASSERT);
- if (ret)
- dev_err(motg->phy.dev, "usb hs_clk deassert failed\n");
- }
- return ret;
-}
-
-static int msm_otg_phy_clk_reset(struct msm_otg *motg)
-{
- int ret;
-
- ret =
Chipidea tree is out of date one month
Hi Alex, Your chipidea tree is out of date more than one month, https://github.com/virtuoso/linux-ci/commits/ci-for-greg You have not queued any patches since June 26th, below is the oldest patch you have not queued. http://marc.info/?l=linux-usb&m=137227196006173&w=2 There are two patches you said you will send to Greg after -rc1, but now, Greg's -rc3 is ready, the patches are still not there. http://marc.info/?l=linux-usb&m=137344357113728&w=2 Besides, please give more comments about patchset, Eg, I sent patchset about 13 patches in it, but you only gave one comment for one patch. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel 3.10.3 "reset SuperSpeed USB device number 2 using xhci_hcd"
On 07/28/13 20:28, Alan Stern wrote: On Sat, 27 Jul 2013, Stuart Foster wrote: On 07/27/13 20:34, Alan Stern wrote: On Sat, 27 Jul 2013, Stuart Foster wrote: On 07/27/13 15:58, Alan Stern wrote: On Sat, 27 Jul 2013, Stuart Foster wrote: Hi, I have started having problems with an external USB 3 diskdrive, the problems started when I moved from the 3.10.2 kernel to 3.10.3. The machine is a ASUS M5A97 PRO, BIOS 1604. On Sat, 27 Jul 2013, Ed Tomlinson wrote: Hi Same problem here with 3.10.3. Dmesg filtered with 'hci'. Each of you, please post a usbmon trace showing what happens when the drive is plugged in. Alan Stern Hi Alan Herewith the log you requested. The trace shows that something is sending an invalid INQUIRY command (one with a 512-byte transfer length) to the drive. Normally such things are done by user programs, but in this case it looks more like the command came from somewhere in the kernel. I have no idea where. Did you change anything besides the kernel when going from 3.10.2 to 3.10.3? If you boot now into a 3.10.2 kernel, does the drive work? What happens if you plug the drive into a USB-2 port? (I expect it will make no difference at all.) Alan Stern Hi Alan, The config between 3.10.2 and 3.10.3 did not change. The drive works correctly on 3.10.2 (I usually keep the previous kernel available just in case). On 3.10.3 the drive fails in a similar manner when plugged into a USB-2 port. For interest I have tried the USB-3 drive in the USB-2 ports on an IBM thinkpad R51 laptop that is also running 3.10.3 and that fails also. Conversely I have tried a USB-2 drive on both the IBM and my ASUS machine and that works fine on 3.10.3 in all available USB ports. Thank you. I was able to duplicate the problem on my own machine and track down the bug. It was introduced by commit 98dcc2946adb (SCSI: sd: Update WRITE SAME heuristics). This commit adds a call to scsi_get_vpd_page() in sd_read_write_same() without first checking sd_try_extended_inquiry(). As noted in the latter routine, VPD inquiries will crash some devices. Martin, this bug needs to be fixed in 3.11. As far as the stable kernels are concerned, the best thing for now may simply be to revert it. Alan Stern Hi Alan, Thanks for the update I will look forward to a solution. regards Stuart Foster -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: dwc3: core: modify IO memory resource after deferred probe completes
From: "Ivan T. Ivanov"
When deferred probe happens driver will try to ioremap multiple times
and will fail. Memory resource.start variable is a global variable,
modifications in this field will be accumulated on every probe.
Fix this by moving the above operations after driver hold all
required PHY's.
Signed-off-by: Ivan T. Ivanov
---
Felipe, Paul if you get better solution please go ahead and ignore
this patch.
v2 - inline comments
drivers/usb/dwc3/core.c | 31 ---
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 607bef8..c257117 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -384,21 +384,6 @@ static int dwc3_probe(struct platform_device *pdev)
dev_err(dev, "missing memory resource\n");
return -ENODEV;
}
- dwc->xhci_resources[0].start = res->start;
- dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
- DWC3_XHCI_REGS_END;
- dwc->xhci_resources[0].flags = res->flags;
- dwc->xhci_resources[0].name = res->name;
-
- res->start += DWC3_GLOBALS_REGS_START;
-
-/*
- * Request memory region but exclude xHCI regs,
- * since it will be requested by the xhci-plat driver.
- */
- regs = devm_ioremap_resource(dev, res);
- if (IS_ERR(regs))
- return PTR_ERR(regs);
if (node) {
dwc->maximum_speed = of_usb_get_maximum_speed(node);
@@ -452,6 +437,22 @@ static int dwc3_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ dwc->xhci_resources[0].start = res->start;
+ dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
+ DWC3_XHCI_REGS_END;
+ dwc->xhci_resources[0].flags = res->flags;
+ dwc->xhci_resources[0].name = res->name;
+
+ res->start += DWC3_GLOBALS_REGS_START;
+
+ /*
+* Request memory region but exclude xHCI regs,
+* since it will be requested by the xhci-plat driver.
+*/
+ regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+
usb_phy_set_suspend(dwc->usb2_phy, 0);
usb_phy_set_suspend(dwc->usb3_phy, 0);
--
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
Re: [PATCH v2 1/3] net/usb/r815x: replace USB buffer from stack to DMA-able
From: Oliver Neukum Date: Mon, 29 Jul 2013 07:20:24 +0200 > On Sat, 2013-07-27 at 20:21 -0700, David Miller wrote: >> From: Hayes Wang >> Date: Thu, 25 Jul 2013 15:59:02 +0800 >> >> > Some USB buffers use stack which may not be DMA-able. >> > Use the buffers from kmalloc to replace those one. >> > >> > Signed-off-by: Hayes Wang >> >> I don't think it's reasonable to kmalloc() a small integer every time >> you want to use a USB message transfer to read or write chip >> registers. >> >> Instead, add a scratch buffer to struct r8152 which is allocated once >> at driver attach time and which you can use for the transfers. >> >> I think you only need an array of two u32's so something like: >> >> u32 transfer_buf[2]; >> >> ought to be sufficient. > > We cannot do that. It would violate the rules about DMA coherency. > We must not touch the same cacheline while DMA is in operation. Good point, then it'll need to be: u32 *transfer_buf; and allocated appropriately. -- 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: Buffer size for ALSA USB PCM audio
At Thu, 25 Jul 2013 10:50:49 -0400 (EDT),
Alan Stern wrote:
>
> On Thu, 25 Jul 2013, Takashi Iwai wrote:
>
> > > Besides, what's the reason for allocating 10 URBs if you're not going
> > > to submit more than 2 of them at a time?
> >
> > I don't know how you deduced 10 urbs in your example,
>
> I made it up. :-)
>
> > but in general,
> > ep->nurbs is calculated so that the driver can hold at least two
> > ep->periods (i.e. double buffer). The USB audio driver has
> > essentially two buffers: an internal hardware buffer via URBs and an
> > intermediate buffer via vmalloc. The latter is exposed to user-space
> > and its content is copied to the URBs appropriately via complete
> > callbacks.
> >
> > Due to this design, we just need two periods for URB buffers,
> > ideally, no matter how many periods are used in the latter buffer
> > specified by user. That's why no buffer_size is needed in ep->nurbs
> > estimation. The actual calculation is, however, a bit more
> > complicated to achieve enough fine-grained updates but yet not too big
> > buffer size. I guess this can be simplified / improved.
>
> Ah, that makes sense. Thanks for the explanation.
>
> The existing code has a problem: Under some conditions the URB queue
> will be too short. EHCI requires at least 10 microframes on the queue
> at all times (even when an URB is completing and is therefore no longer
> on the queue) to avoid underruns. Well, 9 microframes is probably good
> enough, but I'll use 10 to be safe. A typical arrangement of 2 URBs
> each with 8 packets each (at 1 packet/uframe) violates this constraint,
> and I have seen underruns in practice.
>
> The patch below fixes this problem and drastically simplifies the
> calculation. How does it look?
Looks good through a quick glance. But I'd rather like to let review
Clemens and Daniel as I already forgot the old voodoo logic of the
current driver code :)
Thanks!
Takashi
>
> Alan Stern
>
>
>
> Index: usb-3.10/sound/usb/endpoint.c
> ===
> --- usb-3.10.orig/sound/usb/endpoint.c
> +++ usb-3.10/sound/usb/endpoint.c
> @@ -575,6 +575,7 @@ static int data_ep_set_params(struct snd
> struct snd_usb_endpoint *sync_ep)
> {
> unsigned int maxsize, i, urb_packs, total_packs, packs_per_ms;
> + unsigned int min_queued_packs, max_packs;
> int is_playback = usb_pipeout(ep->pipe);
> int frame_bits = snd_pcm_format_physical_width(pcm_format) * channels;
>
> @@ -608,11 +609,21 @@ static int data_ep_set_params(struct snd
> else
> ep->curpacksize = maxsize;
>
> - if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL)
> + if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL) {
> packs_per_ms = 8 >> ep->datainterval;
> - else
> +
> + /* high speed needs 10 USB uframes on the queue at all times */
> + min_queued_packs = DIV_ROUND_UP(10, 8 / packs_per_ms);
> + max_packs = MAX_PACKS_HS;
> + } else {
> packs_per_ms = 1;
>
> + /* full speed needs one USB frame on the queue at all times */
> + min_queued_packs = 1;
> + max_packs = MAX_PACKS;
> + }
> + max_packs = min(max_packs, MAX_QUEUE * packs_per_ms);
> +
> if (is_playback && !snd_usb_endpoint_implicit_feedback_sink(ep)) {
> urb_packs = max(ep->chip->nrpacks, 1);
> urb_packs = min(urb_packs, (unsigned int) MAX_PACKS);
> @@ -625,42 +636,18 @@ static int data_ep_set_params(struct snd
> if (sync_ep && !snd_usb_endpoint_implicit_feedback_sink(ep))
> urb_packs = min(urb_packs, 1U << sync_ep->syncinterval);
>
> - /* decide how many packets to be used */
> - if (is_playback && !snd_usb_endpoint_implicit_feedback_sink(ep)) {
> - unsigned int minsize, maxpacks;
> - /* determine how small a packet can be */
> - minsize = (ep->freqn >> (16 - ep->datainterval))
> - * (frame_bits >> 3);
> - /* with sync from device, assume it can be 12% lower */
> - if (sync_ep)
> - minsize -= minsize >> 3;
> - minsize = max(minsize, 1u);
> - total_packs = (period_bytes + minsize - 1) / minsize;
> - /* we need at least two URBs for queueing */
> - if (total_packs < 2) {
> - total_packs = 2;
> - } else {
> - /* and we don't want too long a queue either */
> - maxpacks = max(MAX_QUEUE * packs_per_ms, urb_packs * 2);
> - total_packs = min(total_packs, maxpacks);
> - }
> - } else {
> - while (urb_packs > 1 && urb_packs * maxsize >= period_bytes)
> - urb_packs >>= 1;
> - total_packs = MAX_URBS * urb_packs;
> - }
> -
> - ep->nurbs = (total_p
[PATCH 0/2] usb: chipidea: fixes for v3.11-rc3
Hi, Here are two small chipidea fixes for v3.11. Fabio Estevam (1): usb: chipidea: cast PORTSC_PTS and DEVLC_PTS macros Peter Chen (1): usb: chipidea: fix the build error with randconfig drivers/usb/chipidea/Kconfig | 4 ++-- drivers/usb/chipidea/bits.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: chipidea: cast PORTSC_PTS and DEVLC_PTS macros
From: Fabio Estevam Fix the following build warnings on x86: drivers/usb/chipidea/core.c: In function 'hw_phymode_configure': drivers/usb/chipidea/core.c:226:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] drivers/usb/chipidea/core.c:230:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] drivers/usb/chipidea/core.c:243:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] drivers/usb/chipidea/core.c:246:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] Reported-by: Felipe Balbi Signed-off-by: Fabio Estevam Signed-off-by: Alexander Shishkin --- drivers/usb/chipidea/bits.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h index aefa026..1b23e35 100644 --- a/drivers/usb/chipidea/bits.h +++ b/drivers/usb/chipidea/bits.h @@ -50,7 +50,7 @@ #define PORTSC_PTC(0x0FUL << 16) /* PTS and PTW for non lpm version only */ #define PORTSC_PTS(d) \ - d) & 0x3) << 30) | (((d) & 0x4) ? BIT(25) : 0)) + (u32)d) & 0x3) << 30) | (((d) & 0x4) ? BIT(25) : 0)) #define PORTSC_PTWBIT(28) #define PORTSC_STSBIT(29) @@ -59,7 +59,7 @@ #define DEVLC_PSPD_HS (0x02UL << 25) #define DEVLC_PTW BIT(27) #define DEVLC_STS BIT(28) -#define DEVLC_PTS(d) (((d) & 0x7) << 29) +#define DEVLC_PTS(d) (u32)(((d) & 0x7) << 29) /* Encoding for DEVLC_PTS and PORTSC_PTS */ #define PTS_UTMI 0 -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: chipidea: fix the build error with randconfig
From: Peter Chen Using below configs, the compile will have error: ERROR: "ehci_init_driver" undefined! .config: CONFIG_USB_CHIPIDEA=m CONFIG_USB_CHIPIDEA_HOST=y CONFIG_USB_CHIPIDEA_DEBUG=y The reason is chipidea host uses symbol from ehci, but ehci is not compiled. Let the chipidea host depend on ehci even it is built as module. Signed-off-by: Peter Chen Signed-off-by: Alexander Shishkin --- drivers/usb/chipidea/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig index eb2aa2e..d1bd8ef 100644 --- a/drivers/usb/chipidea/Kconfig +++ b/drivers/usb/chipidea/Kconfig @@ -12,7 +12,7 @@ if USB_CHIPIDEA config USB_CHIPIDEA_UDC bool "ChipIdea device controller" - depends on USB_GADGET=y || USB_CHIPIDEA=m + depends on USB_GADGET=y || (USB_CHIPIDEA=m && USB_GADGET=m) help Say Y here to enable device controller functionality of the ChipIdea driver. @@ -20,7 +20,7 @@ config USB_CHIPIDEA_UDC config USB_CHIPIDEA_HOST bool "ChipIdea host controller" depends on USB=y - depends on USB_EHCI_HCD=y || USB_CHIPIDEA=m + depends on USB_EHCI_HCD=y || (USB_CHIPIDEA=m && USB_EHCI_HCD=m) select USB_EHCI_ROOT_HUB_TT help Say Y here to enable host controller functionality of the -- 1.8.3.2 -- 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] chipidea: ci_hdrc_imx: Remove unused variable 'res'
Fabio Estevam writes: > From: Fabio Estevam > > 'res' is not used anywhere, so let's get rid of it. > > Signed-off-by: Fabio Estevam Applied and pushed, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] include: Convert ethernet mac address declarations to use ETH_ALEN
On Sunday, July 28, 2013 10:29:04 PM Joe Perches wrote:
> It's convenient to have ethernet mac addresses use
> ETH_ALEN to be able to grep for them a bit easier and
> also to ensure that the addresses are __aligned(2).
>
> Add #include as necessary.
>
> Signed-off-by: Joe Perches
> ---
> include/acpi/actbl2.h | 4 ++-
> include/linux/dm9000.h | 4 ++-
> include/linux/fs_enet_pd.h | 3 ++-
> include/linux/ieee80211.h | 59
> +
> include/linux/mlx4/device.h | 11
> include/linux/mlx4/qp.h | 5 ++--
> include/linux/mv643xx_eth.h | 3 ++-
> include/linux/sh_eth.h | 3 ++-
> include/linux/smsc911x.h| 3 ++-
> include/linux/uwb/spec.h| 5 ++--
> include/media/tveeprom.h| 4 ++-
> include/net/irda/irlan_common.h | 3 ++-
> 12 files changed, 61 insertions(+), 46 deletions(-)
>
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index ffaac0e..3f0f11c 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -44,6 +44,8 @@
> #ifndef __ACTBL2_H__
> #define __ACTBL2_H__
>
> +#include
> +
>
> /***
> *
> * Additional ACPI Tables (2)
> @@ -605,7 +607,7 @@ struct acpi_ibft_nic {
> u8 secondary_dns[16];
> u8 dhcp[16];
> u16 vlan;
> - u8 mac_address[6];
> + u8 mac_address[ETH_ALEN];
> u16 pci_address;
> u16 name_length;
> u16 name_offset;
Please don't touch this file.
It comes from a code base outside of the kernel and should be kept in sync with
the upstream.
Thanks,
Rafael
> diff --git a/include/linux/dm9000.h b/include/linux/dm9000.h
> index 96e8769..841925f 100644
> --- a/include/linux/dm9000.h
> +++ b/include/linux/dm9000.h
> @@ -14,6 +14,8 @@
> #ifndef __DM9000_PLATFORM_DATA
> #define __DM9000_PLATFORM_DATA __FILE__
>
> +#include
> +
> /* IO control flags */
>
> #define DM9000_PLATF_8BITONLY(0x0001)
> @@ -27,7 +29,7 @@
>
> struct dm9000_plat_data {
> unsigned intflags;
> - unsigned char dev_addr[6];
> + unsigned char dev_addr[ETH_ALEN];
>
> /* allow replacement IO routines */
>
> diff --git a/include/linux/fs_enet_pd.h b/include/linux/fs_enet_pd.h
> index 51b7934..343d82a 100644
> --- a/include/linux/fs_enet_pd.h
> +++ b/include/linux/fs_enet_pd.h
> @@ -18,6 +18,7 @@
>
> #include
> #include
> +#include
> #include
>
> #define FS_ENET_NAME "fs_enet"
> @@ -135,7 +136,7 @@ struct fs_platform_info {
> const struct fs_mii_bus_info *bus_info;
>
> int rx_ring, tx_ring; /* number of buffers on rx */
> - __u8 macaddr[6];/* mac address */
> + __u8 macaddr[ETH_ALEN]; /* mac address */
> int rx_copybreak; /* limit we copy small frames */
> int use_napi; /* use NAPI*/
> int napi_weight;/* NAPI weight */
> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> index b0dc87a..4e101af 100644
> --- a/include/linux/ieee80211.h
> +++ b/include/linux/ieee80211.h
> @@ -16,6 +16,7 @@
> #define LINUX_IEEE80211_H
>
> #include
> +#include
> #include
>
> /*
> @@ -209,28 +210,28 @@ static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2)
> struct ieee80211_hdr {
> __le16 frame_control;
> __le16 duration_id;
> - u8 addr1[6];
> - u8 addr2[6];
> - u8 addr3[6];
> + u8 addr1[ETH_ALEN];
> + u8 addr2[ETH_ALEN];
> + u8 addr3[ETH_ALEN];
> __le16 seq_ctrl;
> - u8 addr4[6];
> + u8 addr4[ETH_ALEN];
> } __packed __aligned(2);
>
> struct ieee80211_hdr_3addr {
> __le16 frame_control;
> __le16 duration_id;
> - u8 addr1[6];
> - u8 addr2[6];
> - u8 addr3[6];
> + u8 addr1[ETH_ALEN];
> + u8 addr2[ETH_ALEN];
> + u8 addr3[ETH_ALEN];
> __le16 seq_ctrl;
> } __packed __aligned(2);
>
> struct ieee80211_qos_hdr {
> __le16 frame_control;
> __le16 duration_id;
> - u8 addr1[6];
> - u8 addr2[6];
> - u8 addr3[6];
> + u8 addr1[ETH_ALEN];
> + u8 addr2[ETH_ALEN];
> + u8 addr3[ETH_ALEN];
> __le16 seq_ctrl;
> __le16 qos_ctrl;
> } __packed __aligned(2);
> @@ -608,8 +609,8 @@ struct ieee80211s_hdr {
> u8 flags;
> u8 ttl;
> __le32 seqnum;
> - u8 eaddr1[6];
> - u8 eaddr2[6];
> + u8 eaddr1[ETH_ALEN];
> + u8 eaddr2[ETH_ALEN];
> } __packed __aligned(2);
>
> /* Mesh flags */
> @@ -758,7 +759,7 @@ struct ieee80211_rann_ie {
> u8 rann_flags;
> u8 rann_hopcount;
> u8 rann_ttl;
> - u8 rann_addr[6];
> + u8 rann_addr[ETH_ALEN];
> __le32 rann_seq;
> __le32 rann_interval;
> __le32 rann_metric;
> @@ -802,9 +803,9 @@ enum ieee80211_vht_opmode_bits {
> struct ieee80211_mgmt {
> __le16 frame_control;
> __le16 duration;
> - u8 da
[GIT PULL] usb: fixes for v3.11-rc3
Hi Greg, Here's my second set of fixes. Nothing major, mostly related to recent conversion to configfs on the gadget framework. Let me know if you want me to change anything. The following changes since commit 690c70bab1ebab0782cf9c316b726e2dddebc411: usb: phy: omap-usb3: fix dpll clock index (2013-07-15 13:05:30 +0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/fixes-for-v3.11-rc3 for you to fetch changes up to 1894870eb4240399fabc6f0cb8c6fff4e6edbe83: usb: gadget: udc-core: fix the typo of udc state attribute (2013-07-29 14:15:38 +0300) usb: fixes for v3.11-rc3 Here are some fixes for v3.11-rc3. Mostly related to the recent conversion to configfs done on the gadget drivers, but we also have a fix for MUSB resources on platforms which need 3 resources instead of 2, and a fix for the sysfs_notify() call on udc-core.c which was notifying an unexistent file. Andrzej Pietrasiewicz (4): usb: gadget: ether: put_usb_function on unbind usb: gadget: free opts struct on error recovery usb: gadget: multi: fix error return code in cdc_do_config() usb: gadget: f_phonet: remove unused preprocessor conditional Kishon Vijay Abraham I (1): usb: musb: fix resource passed from glue layer to musb Rong Wang (1): usb: gadget: udc-core: fix the typo of udc state attribute drivers/usb/gadget/ether.c| 14 ++ drivers/usb/gadget/f_ecm.c| 7 +-- drivers/usb/gadget/f_eem.c| 7 +-- drivers/usb/gadget/f_ncm.c| 7 +-- drivers/usb/gadget/f_phonet.c | 9 + drivers/usb/gadget/f_rndis.c | 7 +-- drivers/usb/gadget/f_subset.c | 7 +-- drivers/usb/gadget/multi.c| 10 +++--- drivers/usb/gadget/udc-core.c | 2 +- drivers/usb/musb/omap2430.c | 7 ++- drivers/usb/musb/tusb6010.c | 7 ++- 11 files changed, 56 insertions(+), 28 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/16] dmaengine: add transfered member to dma_async_tx_descriptor
On Mon, Jul 22, 2013 at 08:10:06PM +0200, Sebastian Andrzej Siewior wrote:
> In USB RX path it is possible that the we receive less bytes than
> requested. Take the following example:
> The driver for USB-to-UART submits an URB with 256 bytes in size and the
> dmaengine driver in turn programs a transfer of 256 bytes. The transfer
> is programmed and the dma engine waits for the data to arrive. Once data
> is sent on the UART the dma engine begins to move data. If there was
> only one data byte in the USB packet received then the DMA engine will
> only move one byte due to USB restrictions / rules. The real size of the
> transfer has to be notified to the user / caller so he set this to the
> caller.
> This patch adds the transfered member to the dma_async_tx_descriptor
> where the caller can obtain the final size.
>
> Cc: Vinod Koul
> Cc: Dan Williams
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> include/linux/dmaengine.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index cb286b1..c3a4635 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -403,6 +403,8 @@ typedef void (*dma_async_tx_callback)(void
> *dma_async_param);
> * @tx_submit: set the prepared descriptor(s) to be executed by the engine
> * @callback: routine to call after this operation is complete
> * @callback_param: general parameter to pass to the callback routine
> + * @transfered: number of bytes that were really transfered in case the
> channel
> + * transfered less than requested.
> * ---async_tx api specific fields---
> * @next: at completion submit this descriptor
> * @parent: pointer to the next level up in the dependency chain
> @@ -416,6 +418,7 @@ struct dma_async_tx_descriptor {
> dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
> dma_async_tx_callback callback;
> void *callback_param;
> + unsigned int transfered;
I think this should be doable with residue as well. You can return DMA_SUCCESS
for status and fill in what is NOT transfered indicating that txn completed but
lesser number of bytes are valid
~Vinod
--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: phy: rename nop_usb_xceiv => usb_phy_gen_xceiv
On Fri, Jul 26, 2013 at 10:15:54PM +0200, Sebastian Andrzej Siewior wrote:
> The "nop" driver isn't a do-nothing-stub but supports a couple functions
> like clock on/off or is able to use a voltage regulator. This patch
> simply renames the driver to "generic" since it is easy possible to
> extend it by a simple function istead of writing a complete driver.
>
> Signed-off-by: Sebastian Andrzej Siewior
to me, this is great but I need Tony's Ack for it. Let's Cc Tony and
linux-omap
> ---
> arch/arm/mach-omap2/board-omap3beagle.c | 2 +-
> arch/arm/mach-omap2/board-omap3evm.c | 2 +-
> arch/arm/mach-omap2/board-omap3pandora.c | 2 +-
> arch/arm/mach-omap2/usb-host.c | 8 +++---
> drivers/usb/dwc3/dwc3-exynos.c | 6 ++--
> drivers/usb/dwc3/dwc3-pci.c | 6 ++--
> drivers/usb/phy/Makefile | 2 +-
> drivers/usb/phy/{phy-nop.c => phy-generic.c} | 42
> ++--
> include/linux/usb/nop-usb-xceiv.h| 2 +-
> 9 files changed, 36 insertions(+), 36 deletions(-)
> rename drivers/usb/phy/{phy-nop.c => phy-generic.c} (84%)
>
> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c
> b/arch/arm/mach-omap2/board-omap3beagle.c
> index 04c1165..f595b23 100644
> --- a/arch/arm/mach-omap2/board-omap3beagle.c
> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
> @@ -279,7 +279,7 @@ static struct regulator_consumer_supply
> beagle_vsim_supply[] = {
> static struct gpio_led gpio_leds[];
>
> /* PHY's VCC regulator might be added later, so flag that we need it */
> -static struct nop_usb_xceiv_platform_data hsusb2_phy_data = {
> +static struct usb_phy_gen_xceiv_platform_data hsusb2_phy_data = {
> .needs_vcc = true,
> };
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c
> b/arch/arm/mach-omap2/board-omap3evm.c
> index 8c02626..5d98ef0 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -468,7 +468,7 @@ struct wl12xx_platform_data omap3evm_wlan_data __initdata
> = {
> static struct regulator_consumer_supply omap3evm_vaux2_supplies[] = {
> REGULATOR_SUPPLY("VDD_CSIPHY1", "omap3isp"),/* OMAP ISP */
> REGULATOR_SUPPLY("VDD_CSIPHY2", "omap3isp"),/* OMAP ISP */
> - REGULATOR_SUPPLY("vcc", "nop_usb_xceiv.2"), /* hsusb port 2 */
> + REGULATOR_SUPPLY("vcc", "usb_phy_gen_xceiv.2"), /* hsusb port 2 */
> REGULATOR_SUPPLY("vaux2", NULL),
> };
>
> diff --git a/arch/arm/mach-omap2/board-omap3pandora.c
> b/arch/arm/mach-omap2/board-omap3pandora.c
> index b1547a0..d2b455e 100644
> --- a/arch/arm/mach-omap2/board-omap3pandora.c
> +++ b/arch/arm/mach-omap2/board-omap3pandora.c
> @@ -352,7 +352,7 @@ static struct regulator_consumer_supply
> pandora_vcc_lcd_supply[] = {
> };
>
> static struct regulator_consumer_supply pandora_usb_phy_supply[] = {
> - REGULATOR_SUPPLY("vcc", "nop_usb_xceiv.2"), /* hsusb port 2 */
> + REGULATOR_SUPPLY("vcc", "usb_phy_gen_xceiv.2"), /* hsusb port 2 */
> };
>
> /* ads7846 on SPI and 2 nub controllers on I2C */
> diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
> index 2eb19d4..b54bd06 100644
> --- a/arch/arm/mach-omap2/usb-host.c
> +++ b/arch/arm/mach-omap2/usb-host.c
> @@ -349,7 +349,7 @@ static struct fixed_voltage_config hsusb_reg_config = {
> /* .init_data filled later */
> };
>
> -static const char *nop_name = "nop_usb_xceiv"; /* NOP PHY driver */
> +static const char *nop_name = "usb_phy_gen_xceiv"; /* NOP PHY driver */
> static const char *reg_name = "reg-fixed-voltage"; /* Regulator driver */
>
> /**
> @@ -460,9 +460,9 @@ int usbhs_init_phys(struct usbhs_phy_data *phy, int
> num_phys)
> pdevinfo.name = nop_name;
> pdevinfo.id = phy->port;
> pdevinfo.data = phy->platform_data;
> - pdevinfo.size_data = sizeof(struct nop_usb_xceiv_platform_data);
> -
> - scnprintf(phy_id, MAX_STR, "nop_usb_xceiv.%d",
> + pdevinfo.size_data =
> + sizeof(struct usb_phy_gen_xceiv_platform_data);
> + scnprintf(phy_id, MAX_STR, "usb_phy_gen_xceiv.%d",
> phy->port);
> pdev = platform_device_register_full(&pdevinfo);
> if (IS_ERR(pdev)) {
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 9a8a5e1..1a83fb3 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -38,13 +38,13 @@ struct dwc3_exynos {
>
> static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos)
> {
> - struct nop_usb_xceiv_platform_data pdata;
> + struct usb_phy_gen_xceiv_platform_data pdata;
> struct platform_device *pdev;
> int ret;
>
> memset(&pdata, 0x00, sizeof(pdata));
>
> - pdev = platform_device_alloc("nop_usb_xceiv", PLATFORM_DEVID_AUTO);
> + pdev = platform_device_a
Re: [PATCH 2/4] usb: phy: add am335x pieces to generic phy
On Fri, Jul 26, 2013 at 10:15:55PM +0200, Sebastian Andrzej Siewior wrote: > This patch copies the phy support bits from dsps into the generic phy > driver. Most code can be re-used except for the on/off. > The additional am335x can be removed once we have a phy driver that does > more than this. > > Signed-off-by: Sebastian Andrzej Siewior this is good too, but looking at the amount of am335x-specific code in the driver now it makes me wonder if we want to convert phy-generic.c into a library... -- balbi signature.asc Description: Digital signature
Re: [RFC] ux500 dma & short transfers on MUSB
On Thu, Jul 18, 2013 at 01:21:55PM +0200, Sebastian Andrzej Siewior wrote:
> On 07/11/2013 07:14 PM, Sebastian Andrzej Siewior wrote:
>
> Dan, Vinod,
>
> do you guys have an idea how the dma driver could inform its user how
> much of the requested data got really transferred? This requirement
> seems unique to USB where this happens and is not an error. Below an
> reply to Greg where I tried to explain the problem. The original thread
> started at [0].
> I've been browsing by some drivers and did not find anything close to
> this. The UART drivers which use DMA seem to know the exact number of
> bytes in advance. The dmaengine_tx_status() seems to serve a different
> purpose.
Please look into the residue field
/**
* struct dma_tx_state - filled in to report the status of
* a transfer.
* @last: last completed DMA cookie
* @used: last issued DMA cookie (i.e. the one in progress)
* @residue: the remaining number of bytes left to transmit
* on the selected transfer for states DMA_IN_PROGRESS and
* DMA_PAUSED if this is implemented in the driver, else 0
*/
struct dma_tx_state {
dma_cookie_t last;
dma_cookie_t used;
u32 residue;
};
Typically this is set to 0 when DMA_SUCCESS is returned for the
dmaengine_tx_status() API. But in your case I feel its perfectly valid to return
DMA_SUCCESS but with a non zero residue indiactaing how much was not transmitted
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] usb: phy: msm: Move mach depndend code to platform data
Hi,
On Mon, Jul 29, 2013 at 10:04:19AM +0300, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov"
>
> This patch fix compilation error and is an intermediate step
> before the addition of DeviceTree support for newer targets.
> Fix suggested here: https://lkml.org/lkml/2013/6/19/381
>
> Cc: David Brown
> Cc: Daniel Walker
> Cc: Bryan Huntsman
> Cc: Felipe Balbi
> Cc: Greg Kroah-Hartman
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
>
> Signed-off-by: Ivan T. Ivanov
> ---
> arch/arm/mach-msm/board-msm7x30.c | 35 +++
> arch/arm/mach-msm/board-qsd8x50.c | 35 +++
I need acks for these.
> drivers/usb/phy/phy-msm-usb.c | 55
> ++---
> include/linux/usb/msm_hsusb.h |3 ++
> 4 files changed, 87 insertions(+), 41 deletions(-)
>
> diff --git a/arch/arm/mach-msm/board-msm7x30.c
> b/arch/arm/mach-msm/board-msm7x30.c
> index db3d8c0..6b35953 100644
> --- a/arch/arm/mach-msm/board-msm7x30.c
> +++ b/arch/arm/mach-msm/board-msm7x30.c
> @@ -31,6 +31,7 @@
> #include
>
> #include
> +#include
> #include
> #include
>
> @@ -61,10 +62,44 @@ static int hsusb_phy_init_seq[] = {
> -1
> };
>
> +static int hsusb_link_clk_reset(struct clk *link_clk, bool assert)
looks like you should be using the reset controller framework ?
(drivers/reset)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 3/6] usb: phy: msm: Migrate to Managed Device Resource allocation
Hi,
On Mon, Jul 29, 2013 at 10:04:21AM +0300, Ivan T. Ivanov wrote:
> motg->irq = platform_get_irq(pdev, 0);
> - if (!motg->irq) {
> + if (motg->irq < 0) {
looks like this particular hunk isn't part of $subject.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC] ux500 dma & short transfers on MUSB
On 07/29/2013 01:44 PM, Vinod Koul wrote:
> Please look into the residue field
>
> /**
> * struct dma_tx_state - filled in to report the status of
> * a transfer.
> * @last: last completed DMA cookie
> * @used: last issued DMA cookie (i.e. the one in progress)
> * @residue: the remaining number of bytes left to transmit
> * on the selected transfer for states DMA_IN_PROGRESS and
> * DMA_PAUSED if this is implemented in the driver, else 0
> */
> struct dma_tx_state {
> dma_cookie_t last;
> dma_cookie_t used;
> u32 residue;
> };
>
> Typically this is set to 0 when DMA_SUCCESS is returned for the
> dmaengine_tx_status() API. But in your case I feel its perfectly valid to
> return
> DMA_SUCCESS but with a non zero residue indiactaing how much was not
> transmitted
Thank you very much for that feedback. This has been already pointed
out to me and I converted my driver to use this. Once I finish adding
a few pieces I re-post the driver.
>
> ~Vinod
>
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/16] dmaengine: add transfered member to dma_async_tx_descriptor
On 07/29/2013 01:39 PM, Vinod Koul wrote:
>> @@ -416,6 +418,7 @@ struct dma_async_tx_descriptor {
>> dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
>> dma_async_tx_callback callback;
>> void *callback_param;
>> +unsigned int transfered;
> I think this should be doable with residue as well. You can return DMA_SUCCESS
> for status and fill in what is NOT transfered indicating that txn completed
> but
> lesser number of bytes are valid
Yes, will do.
>
> ~Vinod
>
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()
Hi, On Fri, Jul 26, 2013 at 07:26:05PM +0400, Alexey Khoroshilov wrote: > On 07/25/2013 09:30 PM, Felipe Balbi wrote: > > On Wed, Jul 24, 2013 at 12:20:17AM +0400, Alexey Khoroshilov wrote: > >> mv_u3d_nuke() expects to be calles with ep->u3d->lock held, > >> because mv_u3d_done() does. But mv_u3d_ep_disable() calls it > >> without lock that can lead to unpleasant consequences. > >> > >> Found by Linux Driver Verification project (linuxtesting.org). > >> > >> Signed-off-by: Alexey Khoroshilov > > which commit introduced the bug ? Which kernels are affected by this bug ? > The bug is present from the very beginning: commit 3d4eb9d of 15 June 2012. > So it is in the mainline since v3.5. Alright, do you want to have the same fix in stable kernels ? Is it necessary at all or do we consider it 'never worked before' and send it in the next merge window ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()
On 07/29/2013 04:52 PM, Felipe Balbi wrote: > Hi, > > On Fri, Jul 26, 2013 at 07:26:05PM +0400, Alexey Khoroshilov wrote: >> On 07/25/2013 09:30 PM, Felipe Balbi wrote: >>> On Wed, Jul 24, 2013 at 12:20:17AM +0400, Alexey Khoroshilov wrote: mv_u3d_nuke() expects to be calles with ep->u3d->lock held, because mv_u3d_done() does. But mv_u3d_ep_disable() calls it without lock that can lead to unpleasant consequences. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov >>> which commit introduced the bug ? Which kernels are affected by this bug ? >> The bug is present from the very beginning: commit 3d4eb9d of 15 June 2012. >> So it is in the mainline since v3.5. > Alright, do you want to have the same fix in stable kernels ? Is it > necessary at all or do we consider it 'never worked before' and send it > in the next merge window ? It is a tricky point. From one point of view it 'never worked before', but as far as the bug leads to a data race with unpredictable consequences I would prefer to have it fixed within 3.11 timeframe. -- Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()
On Mon, Jul 29, 2013 at 05:15:58PM +0400, Alexey Khoroshilov wrote: > On 07/29/2013 04:52 PM, Felipe Balbi wrote: > > Hi, > > > > On Fri, Jul 26, 2013 at 07:26:05PM +0400, Alexey Khoroshilov wrote: > >> On 07/25/2013 09:30 PM, Felipe Balbi wrote: > >>> On Wed, Jul 24, 2013 at 12:20:17AM +0400, Alexey Khoroshilov wrote: > mv_u3d_nuke() expects to be calles with ep->u3d->lock held, > because mv_u3d_done() does. But mv_u3d_ep_disable() calls it > without lock that can lead to unpleasant consequences. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov > >>> which commit introduced the bug ? Which kernels are affected by this bug ? > >> The bug is present from the very beginning: commit 3d4eb9d of 15 June 2012. > >> So it is in the mainline since v3.5. > > Alright, do you want to have the same fix in stable kernels ? Is it > > necessary at all or do we consider it 'never worked before' and send it > > in the next merge window ? > > It is a tricky point. From one point of view it 'never worked before', > but as far as the bug leads to a data race with unpredictable > consequences I would prefer to have it fixed within 3.11 timeframe. Alright, I have already sent my pull request for v3.11-rc3, so once -rc4 is tagged (in about a week) I'll send this patch. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()
On Mon, Jul 29, 2013 at 04:52:12PM +0300, Felipe Balbi wrote:
> On Mon, Jul 29, 2013 at 05:15:58PM +0400, Alexey Khoroshilov wrote:
> > On 07/29/2013 04:52 PM, Felipe Balbi wrote:
> > > Hi,
> > >
> > > On Fri, Jul 26, 2013 at 07:26:05PM +0400, Alexey Khoroshilov wrote:
> > >> On 07/25/2013 09:30 PM, Felipe Balbi wrote:
> > >>> On Wed, Jul 24, 2013 at 12:20:17AM +0400, Alexey Khoroshilov wrote:
> > mv_u3d_nuke() expects to be calles with ep->u3d->lock held,
> > because mv_u3d_done() does. But mv_u3d_ep_disable() calls it
> > without lock that can lead to unpleasant consequences.
> >
> > Found by Linux Driver Verification project (linuxtesting.org).
> >
> > Signed-off-by: Alexey Khoroshilov
> > >>> which commit introduced the bug ? Which kernels are affected by this
> > >>> bug ?
> > >> The bug is present from the very beginning: commit 3d4eb9d of 15 June
> > >> 2012.
> > >> So it is in the mainline since v3.5.
> > > Alright, do you want to have the same fix in stable kernels ? Is it
> > > necessary at all or do we consider it 'never worked before' and send it
> > > in the next merge window ?
> >
> > It is a tricky point. From one point of view it 'never worked before',
> > but as far as the bug leads to a data race with unpredictable
> > consequences I would prefer to have it fixed within 3.11 timeframe.
>
> Alright, I have already sent my pull request for v3.11-rc3, so once -rc4
> is tagged (in about a week) I'll send this patch.
BTW, I just found another bug in mv_u3d_core.c, here's a patch:
From f6bb304cff095219ecfa6daa082ea834a9cf52bf Mon Sep 17 00:00:00 2001
From: Felipe Balbi
Date: Mon, 29 Jul 2013 16:58:29 +0300
Subject: [PATCH] usb: gadget: mv_u3d_core: don't call ->disconnect() from
stop_activity()
mv_u3d_stop_activity() should not be calling ->disconnect()
directly as udc-core will already handle that for us.
Signed-off-by: Felipe Balbi
---
Completely untested. It's clear that mv u3d driver
shouldn't be calling gadget_driver->disconnect()
at that time, but perhaps there are other bugs in the
driver which this will uncover.
drivers/usb/gadget/mv_u3d_core.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/usb/gadget/mv_u3d_core.c b/drivers/usb/gadget/mv_u3d_core.c
index 7acbca9..948f77a 100644
--- a/drivers/usb/gadget/mv_u3d_core.c
+++ b/drivers/usb/gadget/mv_u3d_core.c
@@ -1397,13 +1397,6 @@ void mv_u3d_stop_activity(struct mv_u3d *u3d, struct
usb_gadget_driver *driver)
list_for_each_entry(ep, &u3d->gadget.ep_list, ep.ep_list) {
mv_u3d_nuke(ep, -ESHUTDOWN);
}
-
- /* report disconnect; the driver is already quiesced */
- if (driver) {
- spin_unlock(&u3d->lock);
- driver->disconnect(&u3d->gadget);
- spin_lock(&u3d->lock);
- }
}
static void mv_u3d_irq_process_error(struct mv_u3d *u3d)
--
1.8.3.4.840.g6a90778
--
balbi
signature.asc
Description: Digital signature
Re: Ejected Nook (usb mass storage) prevents suspend
On Mon, 29 Jul 2013, Oliver Neukum wrote: > On Fri, 2013-07-26 at 16:31 -0400, Alan Stern wrote: > > > In addition to my earlier comment, the patch below should be applied. > > It will fix your immediate problem, although not in the best way. > > Alan, > > I think your diagnosis is correct, but not the fix. > This is run even in the runtime case. We might lose > data if the flush is not done. Actually no, the scsi_bus_suspend_common() routine does not run during runtime PM. It gets called only from scsi_bus_suspend(), scsi_bus_freeze(), and scsi_bus_poweroff(), which are all part of system sleep. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] usb: phy: phy-omap-control: Add API to power and wakeup
* George Cherian | 2013-07-19 18:04:34 [+0530]:
>diff --git a/drivers/usb/phy/phy-omap-control.c
>b/drivers/usb/phy/phy-omap-control.c
>index 1419ced..4f2502c 100644
>--- a/drivers/usb/phy/phy-omap-control.c
>+++ b/drivers/usb/phy/phy-omap-control.c
>@@ -46,6 +46,73 @@ struct device *omap_get_control_dev(void)
> EXPORT_SYMBOL_GPL(omap_get_control_dev);
>
> /**
>+ * omap_control_am335x_phy_wkup - PHY wakeup on/off using control
>+ *module
>+ * @dev: the control module device
>+ * @on: 0 to off and 1 to on PHY wakeup feature
>+ * @id: phy id 0 or 1 for phy instance 0 and 1 repectively
>+ *
>+ * AM PHY driver should call this API to enable or disable PHY wakeup.
>+ */
>+void omap_control_am335x_phy_wkup(struct device *dev, bool on, u8 id)
>+{
>+ u32 val;
>+ u32 *phy_wkup_reg;
>+ struct omap_control_usb *control_usb = dev_get_drvdata(dev);
>+
>+ if (control_usb->type != OMAP_CTRL_DEV_TYPE3 || id < 0 || id > 1)
>+ return;
>+
>+ phy_wkup_reg = control_usb->dev_conf + AM335X_USB_WKUP_OFFSET;
>+ val = readl(phy_wkup_reg);
Where do you get the memory from? In the DT patches I've made I export
one memory space of 8 bytes for each phy. This memory space applies
directly to the on/off register. I didn't export the register for wakeup
so far since I had no idea how this will be used.
So how do you get your re-mapped memory here?
>+
>+ if (on)
>+ val |= id ? AM335X_USB1_WKUP_CTRL_ENABLE :
>+ AM335X_USB0_WKUP_CTRL_ENABLE;
>+ else
>+ val &= id ? ~AM335X_USB1_WKUP_CTRL_ENABLE :
>+ ~AM335X_USB0_WKUP_CTRL_ENABLE;
>+
>+
>+ writel(val, phy_wkup_reg);
>+}
>+EXPORT_SYMBOL_GPL(omap_control_am335x_phy_wkup);
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] usb: fixes for v3.11-rc3
On Mon, Jul 29, 2013 at 03:17:58PM +0300, Felipe Balbi wrote: > Hi Greg, > > Here's my second set of fixes. Nothing major, mostly > related to recent conversion to configfs on the gadget > framework. > > Let me know if you want me to change anything. > > The following changes since commit 690c70bab1ebab0782cf9c316b726e2dddebc411: > > usb: phy: omap-usb3: fix dpll clock index (2013-07-15 13:05:30 +0300) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/fixes-for-v3.11-rc3 Pulled and pushed out, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Buffer size for ALSA USB PCM audio
On Mon, 29 Jul 2013, Takashi Iwai wrote: > > The existing code has a problem: Under some conditions the URB queue > > will be too short. EHCI requires at least 10 microframes on the queue > > at all times (even when an URB is completing and is therefore no longer > > on the queue) to avoid underruns. Well, 9 microframes is probably good > > enough, but I'll use 10 to be safe. A typical arrangement of 2 URBs > > each with 8 packets each (at 1 packet/uframe) violates this constraint, > > and I have seen underruns in practice. > > > > The patch below fixes this problem and drastically simplifies the > > calculation. How does it look? > > Looks good through a quick glance. But I'd rather like to let review > Clemens and Daniel as I already forgot the old voodoo logic of the > current driver code :) Okay. To recap, the bug I want to fix is that snd-usb-audio does not always keep the USB hardware queue sufficiently full. (There is at least one other minor bug -- the driver uses MAX_PACKS instead of MAX_PACKS_HS for high-speed devices.) Clemens remarked some time ago that keeping the queue full would be trivial, if only he knew how full it needed to be. The answer to that is given above. I have been trying to make the appropriate changes, but I'm not finding it "trivial". :-( Partly because I don't fully understand all the constraints that are already present, but also because it isn't a straightforward calculation. For a recording endpoint, it truly is simple because then the number of packets per URB never changes (as far as I can tell). For a playback endpoint, the number of packets gets reduced when necessary, to insure that none of the packets in the URB starts after the end of the current ALSA period. Since a period is defined in terms of the number of samples (ALSA frames) rather than an absolute time, the end of the period depends on the size of the packets. And this size can vary if the output endpoint has a feedback endpoint. The current driver seems to assume that endpoints with an _implicit_ feedback endpoint won't have variable-length packets. I don't understand why not; doesn't queue_pending_output_urbs() make the packet sizes match the sizes from the corresponding feedback endpoint? I'd appreciate any advice you can offer. 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: Kernel 3.10.3 "reset SuperSpeed USB device number 2 using xhci_hcd"
> "Alan" == Alan Stern writes:
Alan,
Alan> It was introduced by commit 98dcc2946adb (SCSI: sd: Update WRITE
Alan> SAME heuristics). This commit adds a call to scsi_get_vpd_page()
Alan> in sd_read_write_same() without first checking
Alan> sd_try_extended_inquiry(). As noted in the latter routine, VPD
Alan> inquiries will crash some devices.
Is REPORT SUPPORTED OPERATION CODES generally safe on USB devices?
The reason I didn't wrap the WRITE SAME heuristics in
sd_try_extended_inquiry() like I have done with most of the other VPDs
is that there are a ton of older SPI/SAS/FC devices that support WRITE
SAME just fine.
Alan> As far as the stable kernels are concerned, the best thing for now
Alan> may simply be to revert it.
No go. The above commit fixes issues for somebody else.
I could add an explicit check to the sd_read_write_same() function. But
how about we do the following instead?
--
Martin K. Petersen Oracle Linux Engineering
SCSI: Don't attempt to send extended INQUIRY command if skip_vpd_pages is set
If a device has the skip_vpd_pages flag set we should simply fail the
scsi_get_vpd_page() call.
Signed-off-by: Martin K. Petersen
Cc: Alan Stern
Cc: sta...@vger.kernel.org
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 3b1ea34..eaa808e 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -1031,6 +1031,9 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page,
unsigned char *buf,
{
int i, result;
+ if (sdev->skip_vpd_pages)
+ goto fail;
+
/* Ask for all the pages supported by this device */
result = scsi_vpd_inquiry(sdev, buf, 0, buf_len);
if (result)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Fri, Jul 26, 2013 at 02:33:38PM +0530, Kishon Vijay Abraham I wrote:
> Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while creating
> MUSB core device. So in usb_bind_phy (binds the controller with the PHY), the
> device name of the controller had *.auto* in it. Since with using
> PLATFORM_DEVID_AUTO, there is no way to know the exact device name in advance,
> the data given in usb_bind_phy became obsolete and usb_get_phy was failing.
> So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO. Corresponding
> change is done in board file here.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> arch/arm/mach-omap2/board-2430sdp.c |2 +-
> arch/arm/mach-omap2/board-3430sdp.c |2 +-
> arch/arm/mach-omap2/board-cm-t35.c |2 +-
> arch/arm/mach-omap2/board-devkit8000.c |2 +-
> arch/arm/mach-omap2/board-igep0020.c |2 +-
> arch/arm/mach-omap2/board-ldp.c |2 +-
> arch/arm/mach-omap2/board-omap3beagle.c |2 +-
> arch/arm/mach-omap2/board-omap3evm.c |2 +-
> arch/arm/mach-omap2/board-omap3logic.c |2 +-
> arch/arm/mach-omap2/board-omap3pandora.c |2 +-
> arch/arm/mach-omap2/board-omap3stalker.c |2 +-
> arch/arm/mach-omap2/board-omap3touchbook.c |2 +-
> arch/arm/mach-omap2/board-overo.c|2 +-
> arch/arm/mach-omap2/board-rm680.c|2 +-
> arch/arm/mach-omap2/board-rx51.c |2 +-
> arch/arm/mach-omap2/board-zoom-peripherals.c |2 +-
> 16 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-2430sdp.c
> b/arch/arm/mach-omap2/board-2430sdp.c
> index 244d8a5..17bb076 100644
> --- a/arch/arm/mach-omap2/board-2430sdp.c
> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> @@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
> omap_hsmmc_init(mmc);
>
> omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA | OMAP_PULL_UP);
> - usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> + usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
how about moving usb_bind_phy() calls to omap2430.c ?
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index f44e8b5..b6abc1a 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device *pdev)
pdata->board_data = data;
pdata->config = config;
+ } else {
+ /* bind the PHY */
+ usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
}
if (pdata->has_mailbox) {
The only problem is if some board comes with a PHY other than
twl4030_usb, but in that case we can add a separate compatible flag and
treat it properly. We only need to bind the phy in non-DT case anyway,
so that's temporary. It might be better than to reintroduce the IDR in
musb_core.c.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 2/4] phy: phy-amxxxx-usb: Add PHY driver for amxxxx platform
* George Cherian | 2013-07-19 18:04:35 [+0530]: >Adds phy driver support for am33xx platform, the host/device >peripheral controller shall get this phy object to control the phy >operations. If you rebase this on-top of the two instances patches I've sent earlier then you can bury patch 3 and 4, right? I don't like very much the way you obtain the phy id: >+ of_property_read_u32(np, "id", &phy->id); with the .dts changes I made you should able to use of_alias_get_id() instead. Let me look what you have additionaly: - usbotg_fck Is this really required? I have the phy as a child of the "main" device which has a hwmod property which is associated with this clock. So pm enable/ disable should also enable this clock if possible, right? - device wakeup via omap_control_am335x_phy_wkup() Now. that is one thing that the simple phy driver is missing. If you call a magic function for this to happen than I don't have to worry about the missing memory space for this function. So from what I see now, it is most likely the easiest thing to just add that wakeup to the phy driver I posted. Do you agree? If so we need to figure out where the memory for the wakeup register is comming from. We need also to ensure that both phys can not write at the same time. A look would be nice. >Signed-off-by: George Cherian Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel 3.10.3 "reset SuperSpeed USB device number 2 using xhci_hcd"
On Mon, 29 Jul 2013, Martin K. Petersen wrote: > > "Alan" == Alan Stern writes: > > Alan, > > Alan> It was introduced by commit 98dcc2946adb (SCSI: sd: Update WRITE > Alan> SAME heuristics). This commit adds a call to scsi_get_vpd_page() > Alan> in sd_read_write_same() without first checking > Alan> sd_try_extended_inquiry(). As noted in the latter routine, VPD > Alan> inquiries will crash some devices. > > Is REPORT SUPPORTED OPERATION CODES generally safe on USB devices? Broadly speaking, practically nothing is safe on USB devices. :-) The general rule is: If Windows XP doesn't use the command then it will crash something. In the case of these bug reports, the scsi_report_opcode() call returns -EINVAL. > The reason I didn't wrap the WRITE SAME heuristics in > sd_try_extended_inquiry() like I have done with most of the other VPDs > is that there are a ton of older SPI/SAS/FC devices that support WRITE > SAME just fine. > > > Alan> As far as the stable kernels are concerned, the best thing for now > Alan> may simply be to revert it. > > No go. The above commit fixes issues for somebody else. > > I could add an explicit check to the sd_read_write_same() function. But > how about we do the following instead? Stuart and Ed, does Martin's patch fix your problem? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] Documentation: devicetree: phy-tegra-usb: Add vbus-supply property for host mode PHYs
On 06/28/2013 06:33 AM, Mikko Perttunen wrote: > Add vbus-supply as an optional property for host mode phy-tegra-usb PHYs. Felipe, I see that you didn't merge this one patch from this series. Was there a reason for that? At this stage, I would consider it reasonable for DT binding documentation changes to go through the same tree as driver changes. -- 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: Panasonic DMC-ZS3 mass storage USB regression in 3.10.3
On Sun, 28 Jul 2013, Marc Meledandri wrote: > I've hit a USB regression from 3.10.2 (working) -> 3.10.3 (busted) with my > Panasonic DMC-ZS3 camera. I can no longer mount the mass storage device. > [ 197.976105] sd 6:0:0:0: [sdf] Write Protect is off > [ 197.976114] sd 6:0:0:0: [sdf] Mode Sense: 04 00 00 00 > [ 197.976730] sd 6:0:0:0: [sdf] No Caching mode page present > [ 197.976740] sd 6:0:0:0: [sdf] Assuming drive cache: write through > [ 228.144912] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci > [ 259.216526] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci > [ 290.192146] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci > [ 321.167769] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci See this email thread: http://marc.info/?l=linux-usb&m=137503976012984&w=2 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: Audio I/O parameters
On Sun, 28 Jul 2013, James Stone wrote: > On Sat, Jul 27, 2013 at 6:45 PM, Alan Stern wrote: > > On Sat, 27 Jul 2013, James Stone wrote: > > > >> OK. So this seems to have solved the starting jack at low latencies > >> problem, but I am still getting sporadic cannot submit urb (err = -18) > >> under normal use. Will try to add some more info to the #1191603 > >> report if I can get it to happen while logging IRQ. > > > > Do these errors occur at the start of a session or somewhere in the > > middle? > > > > If they occur in the middle, they indicate possible underruns. The > > patch below will greatly reduce the number of these errors (probably to > > the point where you don't see any at all), although it won't fix > > possible underruns. > > > > OK - this patch didn't help - still seeing these cannot submit urb > (err = -18) errors coming up at random times (this time while the > computer was idling). Do you think you can get a usbmon trace showing one of those errors? Or would the trace file end up being hopelessly large? 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: Kernel 3.10.3 "reset SuperSpeed USB device number 2 using xhci_hcd"
> "Alan" == Alan Stern writes: Alan> In the case of these bug reports, the scsi_report_opcode() call Alan> returns -EINVAL. Oh, right. Because I actually do an explicit SCSI version check in scsi_report_opcode(). That's OK, then... -- Martin K. Petersen Oracle Linux Engineering -- 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: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
Hi Kishon,
A small fix follows inline.
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Kishon Vijay Abraham I
> Sent: Friday, July 26, 2013 2:49 PM
>
> The PHY framework provides a set of APIs for the PHY drivers to
> create/destroy a PHY and APIs for the PHY users to obtain a reference
> to the PHY with or without using phandle. For dt-boot, the PHY drivers
> should also register *PHY provider* with the framework.
>
> PHY drivers should create the PHY by passing id and ops like init, exit,
> power_on and power_off. This framework is also pm runtime enabled.
>
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for dt binding can be found
> at Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> Cc: Tomasz Figa
> Cc: Greg Kroah-Hartman
> Signed-off-by: Kishon Vijay Abraham I
> Acked-by: Felipe Balbi
> Tested-by: Sylwester Nawrocki
> ---
> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
> Documentation/phy.txt | 166 +
> MAINTAINERS|8 +
> drivers/Kconfig|2 +
> drivers/Makefile |2 +
> drivers/phy/Kconfig| 18 +
> drivers/phy/Makefile |5 +
> drivers/phy/phy-core.c | 714
>
> include/linux/phy/phy.h| 270
> 9 files changed, 1251 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-
> bindings.txt
> create mode 100644 Documentation/phy.txt create mode 100644
> drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create
> mode 100644 drivers/phy/phy-core.c create mode 100644
> include/linux/phy/phy.h
>
[snip]
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h new file
> mode 100644 index 000..e444b23
> --- /dev/null
> +++ b/include/linux/phy/phy.h
> @@ -0,0 +1,270 @@
[snip]
> +struct phy_init_data {
> + unsigned int num_consumers;
> + struct phy_consumer *consumers;
> +};
> +
> +#define PHY_CONSUMER(_dev_name, _port) \
> +{\
> + .dev_name = _dev_name,\
> + .port = _port,\
> +}
> +
> +#define to_phy(dev) (container_of((dev), struct phy, dev))
> +
> +#define of_phy_provider_register(dev, xlate)\
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +
> +#define devm_of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
I think this should be:
+ __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
Right?
> +
> +static inline void phy_set_drvdata(struct phy *phy, void *data) {
> + dev_set_drvdata(&phy->dev, data);
> +}
> +
> +static inline void *phy_get_drvdata(struct phy *phy) {
> + return dev_get_drvdata(&phy->dev);
> +}
> +
[snip]
Best wishes,
--
Kamil Debski
Linux Kernel Developer
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Monday 29 July 2013 08:36 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Jul 26, 2013 at 02:33:38PM +0530, Kishon Vijay Abraham I wrote:
>> Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while creating
>> MUSB core device. So in usb_bind_phy (binds the controller with the PHY), the
>> device name of the controller had *.auto* in it. Since with using
>> PLATFORM_DEVID_AUTO, there is no way to know the exact device name in
>> advance,
>> the data given in usb_bind_phy became obsolete and usb_get_phy was failing.
>> So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO. Corresponding
>> change is done in board file here.
>>
>> Signed-off-by: Kishon Vijay Abraham I
>> ---
>> arch/arm/mach-omap2/board-2430sdp.c |2 +-
>> arch/arm/mach-omap2/board-3430sdp.c |2 +-
>> arch/arm/mach-omap2/board-cm-t35.c |2 +-
>> arch/arm/mach-omap2/board-devkit8000.c |2 +-
>> arch/arm/mach-omap2/board-igep0020.c |2 +-
>> arch/arm/mach-omap2/board-ldp.c |2 +-
>> arch/arm/mach-omap2/board-omap3beagle.c |2 +-
>> arch/arm/mach-omap2/board-omap3evm.c |2 +-
>> arch/arm/mach-omap2/board-omap3logic.c |2 +-
>> arch/arm/mach-omap2/board-omap3pandora.c |2 +-
>> arch/arm/mach-omap2/board-omap3stalker.c |2 +-
>> arch/arm/mach-omap2/board-omap3touchbook.c |2 +-
>> arch/arm/mach-omap2/board-overo.c|2 +-
>> arch/arm/mach-omap2/board-rm680.c|2 +-
>> arch/arm/mach-omap2/board-rx51.c |2 +-
>> arch/arm/mach-omap2/board-zoom-peripherals.c |2 +-
>> 16 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-2430sdp.c
>> b/arch/arm/mach-omap2/board-2430sdp.c
>> index 244d8a5..17bb076 100644
>> --- a/arch/arm/mach-omap2/board-2430sdp.c
>> +++ b/arch/arm/mach-omap2/board-2430sdp.c
>> @@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
>> omap_hsmmc_init(mmc);
>>
>> omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA | OMAP_PULL_UP);
>> -usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
>> +usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
>
> how about moving usb_bind_phy() calls to omap2430.c ?
>
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index f44e8b5..b6abc1a 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device *pdev)
>
> pdata->board_data = data;
> pdata->config = config;
> + } else {
> + /* bind the PHY */
> + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
This looks like a hack IMHO to workaround the usb phy library. otherwise it is
similar to get_phy_by_name.
> }
>
> if (pdata->has_mailbox) {
>
> The only problem is if some board comes with a PHY other than
> twl4030_usb, but in that case we can add a separate compatible flag and
> treat it properly. We only need to bind the phy in non-DT case anyway,
right.
> so that's temporary. It might be better than to reintroduce the IDR in
> musb_core.c.
that’s needed for generic phy framework anyway :-s
Thanks
Kishon
--
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: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
On Monday 29 July 2013 08:58 PM, Kamil Debski wrote:
> Hi Kishon,
>
> A small fix follows inline.
>
>> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
>> ow...@vger.kernel.org] On Behalf Of Kishon Vijay Abraham I
>> Sent: Friday, July 26, 2013 2:49 PM
>>
>> The PHY framework provides a set of APIs for the PHY drivers to
>> create/destroy a PHY and APIs for the PHY users to obtain a reference
>> to the PHY with or without using phandle. For dt-boot, the PHY drivers
>> should also register *PHY provider* with the framework.
>>
>> PHY drivers should create the PHY by passing id and ops like init, exit,
>> power_on and power_off. This framework is also pm runtime enabled.
>>
>> The documentation for the generic PHY framework is added in
>> Documentation/phy.txt and the documentation for dt binding can be found
>> at Documentation/devicetree/bindings/phy/phy-bindings.txt
>>
>> Cc: Tomasz Figa
>> Cc: Greg Kroah-Hartman
>> Signed-off-by: Kishon Vijay Abraham I
>> Acked-by: Felipe Balbi
>> Tested-by: Sylwester Nawrocki
>> ---
>> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
>> Documentation/phy.txt | 166 +
>> MAINTAINERS|8 +
>> drivers/Kconfig|2 +
>> drivers/Makefile |2 +
>> drivers/phy/Kconfig| 18 +
>> drivers/phy/Makefile |5 +
>> drivers/phy/phy-core.c | 714
>>
>> include/linux/phy/phy.h| 270
>> 9 files changed, 1251 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/phy-
>> bindings.txt
>> create mode 100644 Documentation/phy.txt create mode 100644
>> drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create
>> mode 100644 drivers/phy/phy-core.c create mode 100644
>> include/linux/phy/phy.h
>>
>
> [snip]
>
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h new file
>> mode 100644 index 000..e444b23
>> --- /dev/null
>> +++ b/include/linux/phy/phy.h
>> @@ -0,0 +1,270 @@
>
> [snip]
>
>> +struct phy_init_data {
>> +unsigned int num_consumers;
>> +struct phy_consumer *consumers;
>> +};
>> +
>> +#define PHY_CONSUMER(_dev_name, _port) \
>> +{ \
>> +.dev_name = _dev_name,\
>> +.port = _port,\
>> +}
>> +
>> +#define to_phy(dev) (container_of((dev), struct phy, dev))
>> +
>> +#define of_phy_provider_register(dev, xlate)\
>> +__of_phy_provider_register((dev), THIS_MODULE, (xlate))
>> +
>> +#define devm_of_phy_provider_register(dev, xlate) \
>> +__of_phy_provider_register((dev), THIS_MODULE, (xlate))
>
> I think this should be:
> + __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
> Right?
right.. thanks for spotting this.
Regards
Kishon
--
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: Panasonic DMC-ZS3 mass storage USB regression in 3.10.3
On Mon, Jul 29, 2013 at 11:24 AM, Alan Stern wrote: > On Sun, 28 Jul 2013, Marc Meledandri wrote: > >> I've hit a USB regression from 3.10.2 (working) -> 3.10.3 (busted) with my >> Panasonic DMC-ZS3 camera. I can no longer mount the mass storage device. > >> [ 197.976105] sd 6:0:0:0: [sdf] Write Protect is off >> [ 197.976114] sd 6:0:0:0: [sdf] Mode Sense: 04 00 00 00 >> [ 197.976730] sd 6:0:0:0: [sdf] No Caching mode page present >> [ 197.976740] sd 6:0:0:0: [sdf] Assuming drive cache: write through >> [ 228.144912] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci >> [ 259.216526] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci >> [ 290.192146] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci >> [ 321.167769] usb 6-1.7: reset high-speed USB device number 3 using ehci-pci > > See this email thread: > > http://marc.info/?l=linux-usb&m=137503976012984&w=2 > > Alan Stern > Thanks Alan. I thought it might be the same issue, but wasn't sure if the other thread was specific to USB 3.0 devices. -- 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: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
On 07/26/2013 02:49 PM, Kishon Vijay Abraham I wrote:
> The PHY framework provides a set of APIs for the PHY drivers to
> create/destroy a PHY and APIs for the PHY users to obtain a reference to the
> PHY with or without using phandle. For dt-boot, the PHY drivers should
> also register *PHY provider* with the framework.
>
> PHY drivers should create the PHY by passing id and ops like init, exit,
> power_on and power_off. This framework is also pm runtime enabled.
>
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for dt binding can be found at
> Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> Cc: Tomasz Figa
> Cc: Greg Kroah-Hartman
> Signed-off-by: Kishon Vijay Abraham I
> Acked-by: Felipe Balbi
> Tested-by: Sylwester Nawrocki
> ---
> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
> Documentation/phy.txt | 166 +
> MAINTAINERS|8 +
> drivers/Kconfig|2 +
> drivers/Makefile |2 +
> drivers/phy/Kconfig| 18 +
> drivers/phy/Makefile |5 +
> drivers/phy/phy-core.c | 714
>
> include/linux/phy/phy.h| 270
> 9 files changed, 1251 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
> create mode 100644 Documentation/phy.txt
> create mode 100644 drivers/phy/Kconfig
> create mode 100644 drivers/phy/Makefile
> create mode 100644 drivers/phy/phy-core.c
> create mode 100644 include/linux/phy/phy.h
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
> b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> new file mode 100644
> index 000..8ae844f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> @@ -0,0 +1,66 @@
> +This document explains only the device tree data binding. For general
> +information about PHY subsystem refer to Documentation/phy.txt
[...]
> @@ -0,0 +1,166 @@
> + PHY SUBSYSTEM
> + Kishon Vijay Abraham I
> +
> +This document explains the Generic PHY Framework along with the APIs
> provided,
> +and how-to-use.
> +
> +1. Introduction
> +
> +*PHY* is the abbreviation for physical layer. It is used to connect a device
> +to the physical medium e.g., the USB controller has a PHY to provide
> functions
> +such as serialization, de-serialization, encoding, decoding and is
> responsible
> +for obtaining the required data transmission rate. Note that some USB
> +controllers have PHY functionality embedded into it and others use an
> external
> +PHY. Other peripherals that use PHY include Wireless LAN, Ethernet,
> +SATA etc.
> +
> +The intention of creating this framework is to bring the PHY drivers spread
> +all over the Linux kernel to drivers/phy to increase code re-use and for
> +better code maintainability.
> +
> +This framework will be of use only to devices that use external PHY (PHY
> +functionality is not embedded within the controller).
> +
> +2. Registering/Unregistering the PHY provider
> +
> +PHY provider refers to an entity that implements one or more PHY instances.
> +For the simple case where the PHY provider implements only a single instance
> of
> +the PHY, the framework provides its own implementation of of_xlate in
> +of_phy_simple_xlate. If the PHY provider implements multiple instances, it
> +should provide its own implementation of of_xlate. of_xlate is used only for
> +dt boot case.
> +
> +#define of_phy_provider_register(dev, xlate)\
> +__of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +
> +#define devm_of_phy_provider_register(dev, xlate) \
> +__of_phy_provider_register((dev), THIS_MODULE, (xlate))
This needs to be:
__devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
as Kamil pointed out. We've tested it here with v9 and it makes
Bad Things happen. ;)
> +of_phy_provider_register and devm_of_phy_provider_register macros can be
> used to
> +register the phy_provider and it takes device and of_xlate as
> +arguments. For the dt boot case, all PHY providers should use one of the
> above
> +2 macros to register the PHY provider.
> +
> +void devm_of_phy_provider_unregister(struct device *dev,
> + struct phy_provider *phy_provider);
> +void of_phy_provider_unregister(struct phy_provider *phy_provider);
> +
> +devm_of_phy_provider_unregister and of_phy_provider_unregister can be used to
> +unregister the PHY.
> +
[...]
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> new file mode 100644
> index 000..f1d15e5
> --- /dev/null
> +++ b/drivers/phy/phy-core.c
> @@ -0,0 +1,714 @@
[...]
> +static struct phy *phy_lookup(struct device *device, const char *port)
> +{
> + unsign
Re: [PATCH 2/4] usb: phy: add am335x pieces to generic phy
* Felipe Balbi | 2013-07-29 15:22:18 [+0300]: >On Fri, Jul 26, 2013 at 10:15:55PM +0200, Sebastian Andrzej Siewior wrote: >> This patch copies the phy support bits from dsps into the generic phy >> driver. Most code can be re-used except for the on/off. >> The additional am335x can be removed once we have a phy driver that does >> more than this. >> >> Signed-off-by: Sebastian Andrzej Siewior > >this is good too, but looking at the amount of am335x-specific code in >the driver now it makes me wonder if we want to convert phy-generic.c >into a library... So I just looked over George Cherian phy driver. I think the simplest thing would be to add the usb wakeup path to the driver here. Earlier I hoped that we could just remove the am335x bits and avoid the library thingy :) Since the library probably makes sense: - before or after this patch - where do I get the memory for this wakeup register? It is somewhere in the "reset module" of the am335x Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Panasonic DMC-ZS3 mass storage USB regression in 3.10.3
On Mon, 29 Jul 2013, Marc Meledandri wrote: > On Mon, Jul 29, 2013 at 11:24 AM, Alan Stern > wrote: > > On Sun, 28 Jul 2013, Marc Meledandri wrote: > > > >> I've hit a USB regression from 3.10.2 (working) -> 3.10.3 (busted) with my > >> Panasonic DMC-ZS3 camera. I can no longer mount the mass storage device. > > > >> [ 197.976105] sd 6:0:0:0: [sdf] Write Protect is off > >> [ 197.976114] sd 6:0:0:0: [sdf] Mode Sense: 04 00 00 00 > >> [ 197.976730] sd 6:0:0:0: [sdf] No Caching mode page present > >> [ 197.976740] sd 6:0:0:0: [sdf] Assuming drive cache: write through > >> [ 228.144912] usb 6-1.7: reset high-speed USB device number 3 using > >> ehci-pci > >> [ 259.216526] usb 6-1.7: reset high-speed USB device number 3 using > >> ehci-pci > >> [ 290.192146] usb 6-1.7: reset high-speed USB device number 3 using > >> ehci-pci > >> [ 321.167769] usb 6-1.7: reset high-speed USB device number 3 using > >> ehci-pci > > > > See this email thread: > > > > http://marc.info/?l=linux-usb&m=137503976012984&w=2 > > > > Alan Stern > > > > Thanks Alan. > > I thought it might be the same issue, but wasn't sure if the other thread was > specific to USB 3.0 devices. It isn't, in spite of what the Subject: line says. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pl2303: fix the upper baud rate limit check for type_0/1 chips
Fixes the following regression that has been introduced recently with commit
b2d6d98fc7:
With type_0 and type_1 chips
- all baud rates < 1228800 baud are rounded up to 1228800 baud
- the device silently runs at 9600 baud for all baud rates > 1228800 baud
Signed-off-by: Frank Schäfer
---
drivers/usb/serial/pl2303.c |2 +-
1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 299a0ff..1e6de4c 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -303,7 +303,7 @@ static void pl2303_encode_baudrate(struct tty_struct *tty,
/* type_0, type_1 only support up to 1228800 baud */
if (spriv->type != HX)
- baud = max_t(int, baud, 1228800);
+ baud = min_t(int, baud, 1228800);
if (baud <= 115200) {
put_unaligned_le32(baud, buf);
--
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: Kernel 3.10.3 "reset SuperSpeed USB device number 2 using xhci_hcd"
On 07/29/13 16:23, Alan Stern wrote: On Mon, 29 Jul 2013, Martin K. Petersen wrote: "Alan" == Alan Stern writes: Alan, Alan> It was introduced by commit 98dcc2946adb (SCSI: sd: Update WRITE Alan> SAME heuristics). This commit adds a call to scsi_get_vpd_page() Alan> in sd_read_write_same() without first checking Alan> sd_try_extended_inquiry(). As noted in the latter routine, VPD Alan> inquiries will crash some devices. Is REPORT SUPPORTED OPERATION CODES generally safe on USB devices? Broadly speaking, practically nothing is safe on USB devices. :-) The general rule is: If Windows XP doesn't use the command then it will crash something. In the case of these bug reports, the scsi_report_opcode() call returns -EINVAL. The reason I didn't wrap the WRITE SAME heuristics in sd_try_extended_inquiry() like I have done with most of the other VPDs is that there are a ton of older SPI/SAS/FC devices that support WRITE SAME just fine. Alan> As far as the stable kernels are concerned, the best thing for now Alan> may simply be to revert it. No go. The above commit fixes issues for somebody else. I could add an explicit check to the sd_read_write_same() function. But how about we do the following instead? Stuart and Ed, does Martin's patch fix your problem? Alan Stern Hi Alan, The patch is good for me. thanks Stuart Foster -- 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 14/16] usb: musb: dsps: add MUSB_DEVCTL_SESSION back after removal
* Bin Liu | 2013-07-26 22:07:14 [-0500]:
>Sebastian,
Hi Bin,
>I have not tested it yet, but I believe I found why host mode works on
>TI 3.2 kernel but not on mainline. Please look at Line 786 in 3.2
>kernel musb_core.c [1].
>
>773 if ((int_usb & MUSB_INTR_DISCONNECT) && !musb->ignore_disconnect) {
>..
>785 if (musb->a_wait_bcon != 0 &&
>786 is_otg_enabled(musb))
>787 musb_platform_try_idle(musb,
>jiffies
>788 +
>msecs_to_jiffies(musb->a_wait_bcon));
>
>So when the device is unplugged, *_try_idle() is not called in host
>mode, then the SESSION bit will stay set. But in mainline kernel,
>*_try_idle() will be called regardless.
>
>Please let me know your thoughts.
I am not too familiar with what should happen. The is_otg_enabled() part
is gone since v3.7-rc1 via:
|commit 032ec49f5351e9cb242b1a1c367d14415043ab95
|Author: Felipe Balbi
|Date: Thu Nov 24 15:46:26 2011 +0200
|
|usb: musb: drop useless board_mode usage
|
|we are compiling the driver always with full OTG
|capabilities, so that board_mode trick becomes
|useless.
|
|Signed-off-by: Felipe Balbi
So you say, am335x-evm is not able to run OTG mode and may only run in
host mode and as a consequence it must not call musb_platform_try_idle()
because it throws that one bit away and there is no way to bring it back?
However, if I look at the tree you reference, I see in
arch/arm/mach-omap2/board-am335xevm.c:
| static struct omap_musb_board_data musb_board_data = {
| .interface_type = MUSB_INTERFACE_ULPI,
| .mode = MUSB_OTG,
| .power = 500,
| .instances = 1,
| };
…
| static void __init am335x_evm_init(void)
…
| usb_musb_init(&musb_board_data);
and it creates a "ti81xx-usbss" device.
So it creates a musb device with mode MUSB_OTG no matter what.
I agree that without that try_idle part things keep working but it
seems, that it is also called in the other tree.
>
>Regards,
>-Bin.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: External HDD does not work with 3.11-rc2
On Thu, 25 Jul 2013, Philipp Dreimann wrote: > On Wed, Jul 24, 2013 at 11:03 PM, Alan Stern > wrote: > > On Wed, 24 Jul 2013, Philipp Dreimann wrote: > > > >> Hello, > >> > >> one of my external HDDs does not work using 3.11-rc2. The drive works > >> using 3.10, and 3.9, except for a suspend/resume issue described here: > >> https://bugzilla.redhat.com/show_bug.cgi?id=984189 . > > The problem is caused by some program on your computer sending a > > command to the drive that it can't handle (SCSI INQUIRY command with a > > transfer length of 512). It is not a kernel problem. > > Thanks, I can reproduce the issue using kernel 3.9.9 and > $ sg_inq -l 512 /dev/sdb > > I am guessing that this is the reason for the suspend/resume issues > with 3.9 and 3.10 as well. A look at the program's code might reveal > why this was not triggered on 3.9 and 3.10 as bad as on 3.11... It turns out I was probably wrong. Take a look at this message: http://marc.info/?l=linux-scsi&m=137511040432420&w=2 The patch in that email may fix your problem. 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: Buffer size for ALSA USB PCM audio
Alan Stern wrote: > Clemens remarked some time ago that keeping the queue full would be > trivial, if only he knew how full it needed to be. The answer to that > is given above. I have been trying to make the appropriate changes, > but I'm not finding it "trivial". What I meant was that it would be trivial to change the lower bound of the period size (from which many queueing parameters are derived). > The current driver seems to assume that endpoints with an _implicit_ > feedback endpoint won't have variable-length packets. That's likely to be a bug. Regards, Clemens -- 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 14/16] usb: musb: dsps: add MUSB_DEVCTL_SESSION back after removal
Sebastian,
On Mon, Jul 29, 2013 at 11:53 AM, Sebastian Andrzej Siewior
wrote:
> So you say, am335x-evm is not able to run OTG mode and may only run in
> host mode and as a consequence it must not call musb_platform_try_idle()
> because it throws that one bit away and there is no way to bring it back?
I did not say AM335xEVM is not able to run OTG mode. The problem is in
OTG mode the SESSION bit will be cleared once the device is unplugged,
then there is no way the SESSION bit will come back in the current
mainline kernel.
The TI 3.2 kernel works around this OTG issue by toggling the SESSION
bit in b_idle handling in otg_timer(). The workaround makes most users
happy but it causes VBUS voltage pulsing. That is why I said I don't
know an ideal solution without a ID pin interrupt support.
>
> However, if I look at the tree you reference, I see in
> arch/arm/mach-omap2/board-am335xevm.c:
> | static struct omap_musb_board_data musb_board_data = {
> | .interface_type = MUSB_INTERFACE_ULPI,
> | .mode = MUSB_OTG,
> | .power = 500,
> | .instances = 1,
> | };
> …
> | static void __init am335x_evm_init(void)
> …
> | usb_musb_init(&musb_board_data);
>
> and it creates a "ti81xx-usbss" device.
>
> So it creates a musb device with mode MUSB_OTG no matter what.
I think you looked at a wrong file, maybe a wrong branch. Please check
[1], which defines
2644 * mode[0:3] = USB0PORT's mode
2645 * mode[4:7] = USB1PORT's mode
2646 * AM335X beta EVM has USB0 in OTG mode and USB1 in host mode.
2647 */
2648 .mode = (MUSB_HOST << 4) | MUSB_OTG,
> I agree that without that try_idle part things keep working but it
> seems, that it is also called in the other tree.
>
Then because AM335xEVM USB1 port does not call *try_idle() in this 3.2
kernel, its SESSION bit is always set.
But I don't have a clear idea how to solve this in the mainline kernel
since is_otg_enabled() has been cleaned.
>>
>>Regards,
>>-Bin.
>
> Sebastian
Regards,
-Bin.
[1]:
http://arago-project.org/git/projects/?p=linux-am33x.git;a=blob;f=arch/arm/mach-omap2/board-am335xevm.c;h=aec37f371342455e06626c47e0e92beb51930ed2;hb=refs/heads/v3.2-staging
--
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] pl2303: fix the upper baud rate limit check for type_0/1 chips
Hello. On 07/29/2013 08:33 PM, Frank Schäfer wrote: Fixes the following regression that has been introduced recently with commit b2d6d98fc7: Please also specify that commit's summary line in parens. With type_0 and type_1 chips - all baud rates < 1228800 baud are rounded up to 1228800 baud - the device silently runs at 9600 baud for all baud rates > 1228800 baud Signed-off-by: Frank Schäfer 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
Re: [RFC] xhci: add trace for debug messages related to changing contexts
Hi Xenia!
On Tue, Jul 23, 2013 at 06:14:55AM +0300, Xenia Ragiadakou wrote:
> This patch defines a new trace event, which is called xhci_dbg_context_change
> and belongs in the event class xhci_log_msg, and adds tracepoints for tracing
> the debug messages related to context updates performed with Configure
> Endpoint
> and Evaluate Context commands.
The patch looks good in general, but there's some print statements that
I want to be associated with a different trace point for host controller
"quirks". A quirk is hardware behavior that violates the hardware
specification, and software needs to work around that behavior. It
means the xHCI driver behaves differently when it runs on a quirky host.
Can you make a separate trace event for these functions? Perhaps called
xhci_dbg_quirks?
In the xHCI driver, there's quirks code that handles the Intel Panther
Point bandwidth calculations. Basically, that's any code that's wrapped
in if statements checking whether the XHCI_EP_LIMIT_QUIRK or
XHCI_SW_BW_CHECKING bit is set in xhci->quirks. That also includes
functions like xhci_reserve_bandwidth() and all the functions it calls.
For Fresco Logic hosts, there's print statements associated with
XHCI_RESET_EP_QUIRK in xhci_cleanup_stalled_ring(). There's probably
other print statements associated with quirky hosts, so look for print
statements after testing xhci->quirks, and I'll let you know which ones
should be included in the quirks tracepoint.
Basically, look around for code that is only called when a bit is set in
xhci->quirks, and add any print statements to the xhci_dbg_quirks trace
point.
Comments on specific statements are below:
> Signed-off-by: Xenia Ragiadakou
> ---
> drivers/usb/host/xhci-mem.c | 4 +++-
> drivers/usb/host/xhci-ring.c | 10 +++---
> drivers/usb/host/xhci-trace.h | 5 +
> drivers/usb/host/xhci.c | 36
> 4 files changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index b881bc1..03aa32d 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -26,6 +26,7 @@
> #include
>
> #include "xhci.h"
> +#include "xhci-trace.h"
>
> /*
> * Allocates a generic ring segment from the ring pool, sets the dma address,
> @@ -661,7 +662,8 @@ void xhci_setup_streams_ep_input_ctx(struct xhci_hcd
> *xhci,
>* fls(0) = 0, fls(0x1) = 1, fls(0x10) = 2, fls(0x100) = 3, etc.
>*/
> max_primary_streams = fls(stream_info->num_stream_ctxs) - 2;
> - xhci_dbg(xhci, "Setting number of stream ctx array entries to %u\n",
> + xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
> + "Setting number of stream ctx array entries to %u\n",
> 1 << (max_primary_streams + 1));
> ep_ctx->ep_info &= cpu_to_le32(~EP_MAXPSTREAMS_MASK);
> ep_ctx->ep_info |= cpu_to_le32(EP_MAXPSTREAMS(max_primary_streams)
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 1e57eaf..6adf293 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -67,6 +67,7 @@
> #include
> #include
> #include "xhci.h"
> +#include "xhci-trace.h"
>
> static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci,
> struct xhci_virt_device *virt_dev,
> @@ -1158,7 +1159,8 @@ static void handle_reset_ep_completion(struct xhci_hcd
> *xhci,
>* because the HW can't handle two commands being queued in a row.
>*/
> if (xhci->quirks & XHCI_RESET_EP_QUIRK) {
> - xhci_dbg(xhci, "Queueing configure endpoint command\n");
> + xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
> + "Queueing configure endpoint command\n");
> xhci_queue_configure_endpoint(xhci,
> xhci->devs[slot_id]->in_ctx->dma, slot_id,
> false);
This should be part of the quirks tracepoint.
> @@ -1444,7 +1446,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
> ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state;
> if (!(ep_state & EP_HALTED))
> goto bandwidth_change;
> - xhci_dbg(xhci, "Completed config ep cmd - "
> + xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
> + "Completed config ep cmd - "
> "last ep index = %d, state = %d\n",
> ep_index, ep_state);
> /* Clear internal halted state and restart ring(s) */
> @@ -1454,7 +1457,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
> break;
> }
> bandwidth_change:
> - xhci_dbg(xhci, "Completed config ep cmd\n");
> + xhci_dbg_trace(xhci, trace_xhci_dbg_contex
Re: [PATCH v2 1/6] usb: phy: msm: Move mach depndend code to platform data
On Mon, Jul 29, 2013 at 10:04:19AM +0300, Ivan T. Ivanov wrote: From: "Ivan T. Ivanov" This patch fix compilation error and is an intermediate step before the addition of DeviceTree support for newer targets. Fix suggested here: https://lkml.org/lkml/2013/6/19/381 Cc: David Brown Cc: Daniel Walker Cc: Bryan Huntsman Cc: Felipe Balbi Cc: Greg Kroah-Hartman Cc: linux-arm-ker...@lists.infradead.org Cc: linux-ker...@vger.kernel.org Cc: linux-usb@vger.kernel.org Signed-off-by: Ivan T. Ivanov --- arch/arm/mach-msm/board-msm7x30.c | 35 +++ arch/arm/mach-msm/board-qsd8x50.c | 35 +++ Acked-by: David Brown -- sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- 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 14/16] usb: musb: dsps: add MUSB_DEVCTL_SESSION back after removal
On 07/29/2013 07:26 PM, Bin Liu wrote: > Sebastian, Hi Bin, > I did not say AM335xEVM is not able to run OTG mode. The problem is in > OTG mode the SESSION bit will be cleared once the device is unplugged, > then there is no way the SESSION bit will come back in the current > mainline kernel. Yes, you did. > > The TI 3.2 kernel works around this OTG issue by toggling the SESSION > bit in b_idle handling in otg_timer(). The workaround makes most users > happy but it causes VBUS voltage pulsing. That is why I said I don't > know an ideal solution without a ID pin interrupt support. Now I understand more pieces of the puzzle. And the missing ID pin support is Am335x-evm specific since other am335x board may wire up that pin. But you say there is no way that the phy or anything else could help to avoid that pulsing. > I think you looked at a wrong file, maybe a wrong branch. Please check > [1], which defines > > 2644 * mode[0:3] = USB0PORT's mode > 2645 * mode[4:7] = USB1PORT's mode > 2646 * AM335X beta EVM has USB0 in OTG mode and USB1 in host mode. > 2647 */ > 2648 .mode = (MUSB_HOST << 4) | MUSB_OTG, Oh it is a long way. So looked into the wrong branch. >> I agree that without that try_idle part things keep working but it >> seems, that it is also called in the other tree. >> > > Then because AM335xEVM USB1 port does not call *try_idle() in this 3.2 > kernel, its SESSION bit is always set. > > But I don't have a clear idea how to solve this in the mainline kernel > since is_otg_enabled() has been cleaned. So now I think this is a board problem. What about switching port 1 from OTG to HOST only and avoiding kicking the timer in dsps_musb_try_idle() in such a case? It seems we only need to do this in OTG mode. > Regards, > -Bin. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] usb: chipidea: fixes for v3.11-rc3
On Mon, Jul 29, 2013 at 01:09:55PM +0300, Alexander Shishkin wrote: > Hi, > > Here are two small chipidea fixes for v3.11. Thanks, now applied. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Mon, Jul 29, 2013 at 08:59:26PM +0530, Kishon Vijay Abraham I wrote:
> >> Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while
> >> creating
> >> MUSB core device. So in usb_bind_phy (binds the controller with the PHY),
> >> the
> >> device name of the controller had *.auto* in it. Since with using
> >> PLATFORM_DEVID_AUTO, there is no way to know the exact device name in
> >> advance,
> >> the data given in usb_bind_phy became obsolete and usb_get_phy was failing.
> >> So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO. Corresponding
> >> change is done in board file here.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I
> >> ---
> >> arch/arm/mach-omap2/board-2430sdp.c |2 +-
> >> arch/arm/mach-omap2/board-3430sdp.c |2 +-
> >> arch/arm/mach-omap2/board-cm-t35.c |2 +-
> >> arch/arm/mach-omap2/board-devkit8000.c |2 +-
> >> arch/arm/mach-omap2/board-igep0020.c |2 +-
> >> arch/arm/mach-omap2/board-ldp.c |2 +-
> >> arch/arm/mach-omap2/board-omap3beagle.c |2 +-
> >> arch/arm/mach-omap2/board-omap3evm.c |2 +-
> >> arch/arm/mach-omap2/board-omap3logic.c |2 +-
> >> arch/arm/mach-omap2/board-omap3pandora.c |2 +-
> >> arch/arm/mach-omap2/board-omap3stalker.c |2 +-
> >> arch/arm/mach-omap2/board-omap3touchbook.c |2 +-
> >> arch/arm/mach-omap2/board-overo.c|2 +-
> >> arch/arm/mach-omap2/board-rm680.c|2 +-
> >> arch/arm/mach-omap2/board-rx51.c |2 +-
> >> arch/arm/mach-omap2/board-zoom-peripherals.c |2 +-
> >> 16 files changed, 16 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/board-2430sdp.c
> >> b/arch/arm/mach-omap2/board-2430sdp.c
> >> index 244d8a5..17bb076 100644
> >> --- a/arch/arm/mach-omap2/board-2430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> >> @@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
> >>omap_hsmmc_init(mmc);
> >>
> >>omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA | OMAP_PULL_UP);
> >> - usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> >> + usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
> >
> > how about moving usb_bind_phy() calls to omap2430.c ?
> >
> > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> > index f44e8b5..b6abc1a 100644
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device *pdev)
> >
> > pdata->board_data = data;
> > pdata->config = config;
> > + } else {
> > + /* bind the PHY */
> > + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
>
> This looks like a hack IMHO to workaround the usb phy library. otherwise it is
> similar to get_phy_by_name.
actually, this is a workaround to the fact that we're not creating all
platform_devices in arch/arm/mach-omap2/ :-)
If we had the musb allocation there, we could easily handle
usb_bind_phy()
> > so that's temporary. It might be better than to reintroduce the IDR in
> > musb_core.c.
>
> that’s needed for generic phy framework anyway :-s
right, but generic phy framework can handle everything just fine, the
only problem is that names are changing.
--
balbi
signature.asc
Description: Digital signature
Re: Buffer size for ALSA USB PCM audio
On Mon, 29 Jul 2013, Clemens Ladisch wrote: > Alan Stern wrote: > > Clemens remarked some time ago that keeping the queue full would be > > trivial, if only he knew how full it needed to be. The answer to that > > is given above. I have been trying to make the appropriate changes, > > but I'm not finding it "trivial". > > What I meant was that it would be trivial to change the lower bound of > the period size (from which many queueing parameters are derived). I doubt that would help. What really matters here is the relation between urb_packs (the maximum number of packets in an URB) and the number of packets in a period (which can vary as the device's clock frequency drifts). For a bizarre example of sort of thing that can happen, suppose urb_packs is 8 and and a period always consists of 8 packets. Then to occupy 10 uframes requires two URBs (assuming one packet per uframe). But now suppose a period might need 9 packets (say because the device's clock frequency is a little low, so it consumes samples less quickly and therefore a fixed number of samples takes more time). Then to occupy 10 uframes, two URBs would no longer be enough, because the numbers of packets in successive URBs could be 8, 1, 8, etc. Two URBs would occupy only 9 uframes. > > The current driver seems to assume that endpoints with an _implicit_ > > feedback endpoint won't have variable-length packets. > > That's likely to be a bug. data_ep_set_params() checks snd_usb_endpoint_implicit_feedback_sink() in three places. It looks like only the second place is correct. Can you verify whether the other two are right, and explain to me if they are? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/16] usb: musb: dsps: add MUSB_DEVCTL_SESSION back after removal
Sebastian, On Mon, Jul 29, 2013 at 12:51 PM, Sebastian Andrzej Siewior wrote: > On 07/29/2013 07:26 PM, Bin Liu wrote: >> Sebastian, > > Hi Bin, > >> I did not say AM335xEVM is not able to run OTG mode. The problem is in >> OTG mode the SESSION bit will be cleared once the device is unplugged, >> then there is no way the SESSION bit will come back in the current >> mainline kernel. > > Yes, you did. If I did, that was wrong statement. > >> >> The TI 3.2 kernel works around this OTG issue by toggling the SESSION >> bit in b_idle handling in otg_timer(). The workaround makes most users >> happy but it causes VBUS voltage pulsing. That is why I said I don't >> know an ideal solution without a ID pin interrupt support. > > Now I understand more pieces of the puzzle. And the missing ID pin > support is Am335x-evm specific since other am335x board may wire up > that pin. No, it is not board wiring issue. The AM335x EVM DOES have ID pin wired to the ID pin of the micro-AB receptacle. I said AM335x does not have IP pin INTERRUPT support, by which the kernel does not know when a device is plugged in without setting the SESSION bit. > But you say there is no way that the phy or anything else could help to > avoid that pulsing. > >> I think you looked at a wrong file, maybe a wrong branch. Please check >> [1], which defines >> >> 2644 * mode[0:3] = USB0PORT's mode >> 2645 * mode[4:7] = USB1PORT's mode >> 2646 * AM335X beta EVM has USB0 in OTG mode and USB1 in host mode. >> 2647 */ >> 2648 .mode = (MUSB_HOST << 4) | MUSB_OTG, > > Oh it is a long way. So looked into the wrong branch. > >>> I agree that without that try_idle part things keep working but it >>> seems, that it is also called in the other tree. >>> >> >> Then because AM335xEVM USB1 port does not call *try_idle() in this 3.2 >> kernel, its SESSION bit is always set. >> >> But I don't have a clear idea how to solve this in the mainline kernel >> since is_otg_enabled() has been cleaned. > > So now I think this is a board problem. What about switching port 1 > from OTG to HOST only and avoiding kicking the timer in > dsps_musb_try_idle() in such a case? It seems we only need to do this > in OTG mode. I have not read the MUSB driver in the mainline kernel yet. If somehow port 1 can could be set to HOST mode then the SESSION bit would stay, then that should solve the issue. > >> Regards, >> -Bin. > > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] xhci: add trace for debug messages related to changing contexts
Ah, looking over the patch again, there's a couple print statements
related to configure endpoint and evaluate context changes that you
missed.
Are you familiar with completions? If not, I suggest you read the
"Completions" section of Chapter 5 of Linux Device Drivers:
http://lwn.net/Kernel/LDD3/
When an xHCI command completes, the hardware places an event on the
event ring. That event includes a pointer to command on the command
ring, and a completion code. The xHCI driver will then receive an
interrupt, and it will figure out a command completed. The xHCI driver
will then signal to the function that's waiting on that completion.
In the Configure Endpoint or Evaluate Context command case, the function
xhci_configure_endpoint() calls
wait_for_completion_interruptible_timeout(). When the interrupt
happens, xhci_irq() in xhci-ring.c gets called, the driver figures out a
command completed, and handle_cmd_completion() is called. That function
may print statements relating to command completions.
The xhci_dbg_context_change trace function also needs to be used for
print statements in the TRB_TYPE(TRB_CONFIG_EP) case in the giant switch
statement in handle_cmd_completion().
Bonus points if you can refactor that case statement into a separate
static function called handle_configure_endpoint(). :)
Sarah Sharp
On Tue, Jul 23, 2013 at 06:14:55AM +0300, Xenia Ragiadakou wrote:
> This patch defines a new trace event, which is called xhci_dbg_context_change
> and belongs in the event class xhci_log_msg, and adds tracepoints for tracing
> the debug messages related to context updates performed with Configure
> Endpoint
> and Evaluate Context commands.
>
> Signed-off-by: Xenia Ragiadakou
> ---
> drivers/usb/host/xhci-mem.c | 4 +++-
> drivers/usb/host/xhci-ring.c | 10 +++---
> drivers/usb/host/xhci-trace.h | 5 +
> drivers/usb/host/xhci.c | 36
> 4 files changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index b881bc1..03aa32d 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -26,6 +26,7 @@
> #include
>
> #include "xhci.h"
> +#include "xhci-trace.h"
>
> /*
> * Allocates a generic ring segment from the ring pool, sets the dma address,
> @@ -661,7 +662,8 @@ void xhci_setup_streams_ep_input_ctx(struct xhci_hcd
> *xhci,
>* fls(0) = 0, fls(0x1) = 1, fls(0x10) = 2, fls(0x100) = 3, etc.
>*/
> max_primary_streams = fls(stream_info->num_stream_ctxs) - 2;
> - xhci_dbg(xhci, "Setting number of stream ctx array entries to %u\n",
> + xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
> + "Setting number of stream ctx array entries to %u\n",
> 1 << (max_primary_streams + 1));
> ep_ctx->ep_info &= cpu_to_le32(~EP_MAXPSTREAMS_MASK);
> ep_ctx->ep_info |= cpu_to_le32(EP_MAXPSTREAMS(max_primary_streams)
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 1e57eaf..6adf293 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -67,6 +67,7 @@
> #include
> #include
> #include "xhci.h"
> +#include "xhci-trace.h"
>
> static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci,
> struct xhci_virt_device *virt_dev,
> @@ -1158,7 +1159,8 @@ static void handle_reset_ep_completion(struct xhci_hcd
> *xhci,
>* because the HW can't handle two commands being queued in a row.
>*/
> if (xhci->quirks & XHCI_RESET_EP_QUIRK) {
> - xhci_dbg(xhci, "Queueing configure endpoint command\n");
> + xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
> + "Queueing configure endpoint command\n");
> xhci_queue_configure_endpoint(xhci,
> xhci->devs[slot_id]->in_ctx->dma, slot_id,
> false);
> @@ -1444,7 +1446,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
> ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state;
> if (!(ep_state & EP_HALTED))
> goto bandwidth_change;
> - xhci_dbg(xhci, "Completed config ep cmd - "
> + xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
> + "Completed config ep cmd - "
> "last ep index = %d, state = %d\n",
> ep_index, ep_state);
> /* Clear internal halted state and restart ring(s) */
> @@ -1454,7 +1457,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
> break;
> }
> bandwidth_change:
> - xhci_dbg(xhci, "Completed config ep cmd\n");
> + xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
> +
Re: [PATCH v2 1/6] usb: phy: msm: Move mach depndend code to platform data
Hi,
> > Cc: Bryan Huntsman
> > Cc: Felipe Balbi
> > Cc: Greg Kroah-Hartman
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: linux-ker...@vger.kernel.org
> > Cc: linux-usb@vger.kernel.org
> >
> > Signed-off-by: Ivan T. Ivanov
> > ---
> > arch/arm/mach-msm/board-msm7x30.c | 35 +++
> > arch/arm/mach-msm/board-qsd8x50.c | 35 +++
>
> I need acks for these.
>
> > drivers/usb/phy/phy-msm-usb.c | 55
> > ++---
> > include/linux/usb/msm_hsusb.h |3 ++
> > 4 files changed, 87 insertions(+), 41 deletions(-)
> >
> > diff --git a/arch/arm/mach-msm/board-msm7x30.c
> > b/arch/arm/mach-msm/board-msm7x30.c
> > index db3d8c0..6b35953 100644
> > --- a/arch/arm/mach-msm/board-msm7x30.c
> > +++ b/arch/arm/mach-msm/board-msm7x30.c
> > @@ -31,6 +31,7 @@
> > #include
> >
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -61,10 +62,44 @@ static int hsusb_phy_init_seq[] = {
> > -1
> > };
> >
> > +static int hsusb_link_clk_reset(struct clk *link_clk, bool assert)
>
> looks like you should be using the reset controller framework ?
> (drivers/reset)
Probably, but there still nothing in place in the msm,
which provide this functionality. I am looking it to
right now.
Regards,
Ivan
--
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: Buffer size for ALSA USB PCM audio
On 29.07.2013 20:20, Alan Stern wrote: > data_ep_set_params() checks snd_usb_endpoint_implicit_feedback_sink() > in three places. It looks like only the second place is correct. > > Can you verify whether the other two are right, and explain to me if > they are? Which version are you looking at? Eldad Zack recently posted work in that area, but I can't seem to find them anywhere yet in the sound tree. Takashi? Eldad, as you're likely more into the detail right now - any oppinion on Alan's findings? Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] include: Convert ethernet mac address declarations to use ETH_ALEN
On Mon, 2013-07-29 at 13:59 +0200, Rafael J. Wysocki wrote:
> On Sunday, July 28, 2013 10:29:04 PM Joe Perches wrote:
> > It's convenient to have ethernet mac addresses use
> > ETH_ALEN to be able to grep for them a bit easier and
> > also to ensure that the addresses are __aligned(2).
[]
> > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
[]
> > @@ -44,6 +44,8 @@
[]
> > +#include
> > +
[]
> > @@ -605,7 +607,7 @@ struct acpi_ibft_nic {
[]
> > - u8 mac_address[6];
> > + u8 mac_address[ETH_ALEN];
> Please don't touch this file.
>
> It comes from a code base outside of the kernel and should be kept in sync
> with
> the upstream.
Which files in include/acpi have this characteristic?
Perhaps an include/acpi/README is appropriate.
--
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: Buffer size for ALSA USB PCM audio
On Mon, 29 Jul 2013, Daniel Mack wrote: > On 29.07.2013 20:20, Alan Stern wrote: > > data_ep_set_params() checks snd_usb_endpoint_implicit_feedback_sink() > > in three places. It looks like only the second place is correct. > > > > Can you verify whether the other two are right, and explain to me if > > they are? > > Which version are you looking at? Eldad Zack recently posted work in > that area, but I can't seem to find them anywhere yet in the sound tree. I'm working with Linus's 3.11-rc3 kernel. > Takashi? > > Eldad, as you're likely more into the detail right now - any oppinion on > Alan's findings? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] include: Convert ethernet mac address declarations to use ETH_ALEN
On Monday, July 29, 2013 12:34:24 PM Joe Perches wrote:
> On Mon, 2013-07-29 at 13:59 +0200, Rafael J. Wysocki wrote:
> > On Sunday, July 28, 2013 10:29:04 PM Joe Perches wrote:
> > > It's convenient to have ethernet mac addresses use
> > > ETH_ALEN to be able to grep for them a bit easier and
> > > also to ensure that the addresses are __aligned(2).
> []
> > > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> []
> > > @@ -44,6 +44,8 @@
> []
> > > +#include
> > > +
> []
> > > @@ -605,7 +607,7 @@ struct acpi_ibft_nic {
> []
> > > - u8 mac_address[6];
> > > + u8 mac_address[ETH_ALEN];
>
> > Please don't touch this file.
> >
> > It comes from a code base outside of the kernel and should be kept in sync
> > with
> > the upstream.
>
> Which files in include/acpi have this characteristic?
Generally, all whose names start with "ac" except for acpi_bus.h,
acpi_drivers.h and acpi_numa.h.
> Perhaps an include/acpi/README is appropriate.
Yes, we can add one.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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: Audio I/O parameters
On Mon, Jul 29, 2013 at 4:25 PM, Alan Stern wrote: > On Sun, 28 Jul 2013, James Stone wrote: > >> On Sat, Jul 27, 2013 at 6:45 PM, Alan Stern >> wrote: >> > On Sat, 27 Jul 2013, James Stone wrote: >> > >> >> OK. So this seems to have solved the starting jack at low latencies >> >> problem, but I am still getting sporadic cannot submit urb (err = -18) >> >> under normal use. Will try to add some more info to the #1191603 >> >> report if I can get it to happen while logging IRQ. >> > >> > Do these errors occur at the start of a session or somewhere in the >> > middle? >> > >> > If they occur in the middle, they indicate possible underruns. The >> > patch below will greatly reduce the number of these errors (probably to >> > the point where you don't see any at all), although it won't fix >> > possible underruns. >> > >> >> OK - this patch didn't help - still seeing these cannot submit urb >> (err = -18) errors coming up at random times (this time while the >> computer was idling). > > Do you think you can get a usbmon trace showing one of those errors? > Or would the trace file end up being hopelessly large? > No - no way to get usbmon trace on this - it happens only every few hours at unpredictable times, and I can't work out what triggers it. J -- 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
USB Interrupt Transaction Scheduling
According to the USB 2.0 specification, 19 full speed interrupt endpoints (@ 64 bytes) can be scheduled. I have a couple of questions about this specification and the Linux kernel's implementation: 1. Does this number take the USB requirement that "no more than 90% of any frame be allocated for periodic full-speed transfers" into account? 2. Does the full speed limitation apply if full speed devices are connected to high speed hubs? 2a. If the high speed limitation is used: Does the scheduler multiplex each full speed device's split data packets over the 8 available micro-frames? We performed some testing, but I don't want to make assumptions on these results alone. Setup #1: Kernel 3.10.0 Connect 6 USB devices (each with 2 IN and 1 OUT interrupt endpoints @64 bytes) to USB 1.1 full speed hubs to a PC USB 2.0 port. The test application can communicate with all 18 endpoints. When we connected a 7th device, the test application is unable to open and access the device. This makes sense because that would be 21 full speed endpoints. Setup #2: Kernel 3.10.0 Connect 7 USB devices (each with 2 IN and 1 OUT interrupt endpoints @64 bytes) to USB 2.0 high speed hubs to a PC USB 2.0 port. The test application can communicate with all 21 endpoints. This appears to violate the full speed limitation; however, it wouldn't be violating the high speed limitation of 63 endpoints per micro-frame. Thanks for the help, -Nate -- 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: Audio I/O parameters
On Mon, Jul 29, 2013 at 9:41 PM, James Stone wrote: > On Mon, Jul 29, 2013 at 4:25 PM, Alan Stern wrote: >> On Sun, 28 Jul 2013, James Stone wrote: >> >>> On Sat, Jul 27, 2013 at 6:45 PM, Alan Stern >>> wrote: >>> > On Sat, 27 Jul 2013, James Stone wrote: >>> > >>> >> OK. So this seems to have solved the starting jack at low latencies >>> >> problem, but I am still getting sporadic cannot submit urb (err = -18) >>> >> under normal use. Will try to add some more info to the #1191603 >>> >> report if I can get it to happen while logging IRQ. >>> > >>> > Do these errors occur at the start of a session or somewhere in the >>> > middle? >>> > >>> > If they occur in the middle, they indicate possible underruns. The >>> > patch below will greatly reduce the number of these errors (probably to >>> > the point where you don't see any at all), although it won't fix >>> > possible underruns. >>> > >>> >>> OK - this patch didn't help - still seeing these cannot submit urb >>> (err = -18) errors coming up at random times (this time while the >>> computer was idling). >> >> Do you think you can get a usbmon trace showing one of those errors? >> Or would the trace file end up being hopelessly large? >> > > No - no way to get usbmon trace on this - it happens only every few > hours at unpredictable times, and I can't work out what triggers it. > > J OK, having said that, I just got it to happen - listening to audacity and just logging into Facebook (of all things!! Meh!) This is the contents of the trace file (as per instructions on bug #1191603) # tracer: irqsoff # # irqsoff latency trace v1.1.5 on 3.10.0-ver5 # # latency: 2173 us, #4/4, CPU#0 | (M:desktop VP:0, KP:0, SP:0 HP:0 #P:4) #- #| task: apt-check-3628 (uid:1000 nice:19 policy:0 rt_prio:0) #- # => started at: perf_event_update_userpage # => ended at: retint_careful # # # _--=> CPU# # / _-=> irqs-off #| / _=> need-resched #|| / _---=> hardirq/softirq #||| / _--=> preempt-depth # / delay # cmd pid | time | caller # \ / | \| / apt-chec-36280d.h.0us!: local_clock <-perf_event_update_userpage apt-chec-36280dN.. 2173us : trace_hardirqs_on_thunk <-retint_careful apt-chec-36280dN.. 2173us+: trace_hardirqs_on_caller <-retint_careful apt-chec-36280dN.. 2177us : => trace_hardirqs_on_thunk I will send the tail of the usbmon trace off-list. 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: USB Interrupt Transaction Scheduling
On Mon, Jul 29, 2013 at 08:43:01PM +, Stoddard, Nate (GE Healthcare) wrote: > According to the USB 2.0 specification, 19 full speed interrupt endpoints (@ > 64 bytes) can be scheduled. I have a couple of questions about this > specification and the Linux kernel's implementation: > > 1. Does this number take the USB requirement that "no more than 90% > of any frame be allocated for periodic full-speed transfers" into > account? > > 2. Does the full speed limitation apply if full speed devices are > connected to high speed hubs? > > 2a. If the high speed limitation is used: Does the scheduler > multiplex each full speed device's split data packets over the 8 > available micro-frames? Becides just testing to see if we do follow these things, what is your end goal here? Are you trying to determine how many devices you should be able to successfully plug into a machine and have them work properly? Or something else? > We performed some testing, but I don't want to make assumptions on these > results alone. > Setup #1: > Kernel 3.10.0 > Connect 6 USB devices (each with 2 IN and 1 OUT interrupt endpoints > @64 bytes) to USB 1.1 full speed hubs to a PC USB 2.0 port. The test > application can communicate with all 18 endpoints. When we connected > a 7th device, the test application is unable to open and access the > device. This makes sense because that would be 21 full speed > endpoints. Watch out when using USB 1.1 hubs plugged into a EHCI controller, there are some "tricky" things happening at times that can get messy. I'd not recommend doing that if you can avoid it. > Setup #2: > Kernel 3.10.0 > Connect 7 USB devices (each with 2 IN and 1 OUT interrupt endpoints > @64 bytes) to USB 2.0 high speed hubs to a PC USB 2.0 port. The test > application can communicate with all 21 endpoints. This appears to > violate the full speed limitation; however, it wouldn't be violating > the high speed limitation of 63 endpoints per micro-frame. Hey, if it works, don't complain :) You don't say what the devices are rated to be, full/low/high/super speed, which matters a lot here. 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: USB Interrupt Transaction Scheduling
On Mon, 29 Jul 2013, Stoddard, Nate (GE Healthcare) wrote: > According to the USB 2.0 specification, 19 full speed interrupt > endpoints (@ 64 bytes) can be scheduled. I have a couple of > questions about this specification and the Linux kernel's > implementation: > > 1. Does this number take the USB requirement that "no more than 90% > of any frame be allocated for periodic full-speed transfers" into > account? No. You can tell because the table says that there would be 37 bytes remaining, which is less than 10% of the 1500 bytes total. The text also mentions that the tables do not include the overhead associated with bit stuffing. You'll get a more accurate picture from the formulas in section 5.11.3. They indicate that only 15 64-byte full-speed interrupt transfers should be possible during a frame (about 59 us per transfer). > 2. Does the full speed limitation apply if full speed devices are > connected to high speed hubs? It applies to the subset of devices attached to any particular Transaction Translator. Some high-speed hubs have a separate TT for each port, whereas others have only a single TT. > 2a. If the high speed limitation is used: Does the scheduler > multiplex each full speed device's split data packets over the 8 > available micro-frames? To some extent. Not all 8 microframes are available (the spec prohibits sending a Start Split packet during microframe 6), and the current ehci-hcd implementation is not capable of using all the ones that are available. However, it is capable of using at least 4 of the 8 microframes. > We performed some testing, but I don't want to make assumptions on > these results alone. > Setup #1: > Kernel 3.10.0 > Connect 6 USB devices (each with 2 IN and 1 OUT interrupt endpoints > @64 bytes) to USB 1.1 full speed hubs to a PC USB 2.0 port. The test > application can communicate with all 18 endpoints. When we connected > a 7th device, the test application is unable to open and access the > device. This makes sense because that would be 21 full speed > endpoints. This doesn't sound right. What sort of host controller were you using? > Setup #2: > Kernel 3.10.0 > Connect 7 USB devices (each with 2 IN and 1 OUT interrupt endpoints > @64 bytes) to USB 2.0 high speed hubs to a PC USB 2.0 port. The test > application can communicate with all 21 endpoints. This appears to > violate the full speed limitation; however, it wouldn't be violating > the high speed limitation of 63 endpoints per micro-frame. The limitation you are referring to (Table 5-8 in the spec) is for interrupt transfers to high-speed devices. It does not apply to interrupt transfers to full-speed 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
Re: [PATCH v2 2/3] Documentation: devicetree: phy-tegra-usb: Add vbus-supply property for host mode PHYs
On Mon, Jul 29, 2013 at 09:24:46AM -0600, Stephen Warren wrote: > On 06/28/2013 06:33 AM, Mikko Perttunen wrote: > > Add vbus-supply as an optional property for host mode phy-tegra-usb PHYs. > > Felipe, I see that you didn't merge this one patch from this series. Was > there a reason for that? At this stage, I would consider it reasonable > for DT binding documentation changes to go through the same tree as > driver changes. no reason, just missed it. Will apply now. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 2/3] Documentation: devicetree: phy-tegra-usb: Add vbus-supply property for host mode PHYs
Hi,
On Tue, Jul 30, 2013 at 12:19:32AM +0300, Felipe Balbi wrote:
> On Mon, Jul 29, 2013 at 09:24:46AM -0600, Stephen Warren wrote:
> > On 06/28/2013 06:33 AM, Mikko Perttunen wrote:
> > > Add vbus-supply as an optional property for host mode phy-tegra-usb PHYs.
> >
> > Felipe, I see that you didn't merge this one patch from this series. Was
> > there a reason for that? At this stage, I would consider it reasonable
> > for DT binding documentation changes to go through the same tree as
> > driver changes.
>
> no reason, just missed it. Will apply now.
actually, patch _is_ in my tree:
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=next&id=80bd8a94712a0a9f2baeb03ad0546dd70f27ac4e
commit 80bd8a94712a0a9f2baeb03ad0546dd70f27ac4e
Author: Mikko Perttunen
Date: Wed Jul 17 10:37:50 2013 +0300
usb: tegra: Add vbus-supply property for host mode PHYs
Document vbus-supply as an optional property for host mode phy-tegra-usb
PHYs.
Signed-off-by: Mikko Perttunen
Reviewed-by: Stephen Warren
Tested-by: Stephen Warren
Signed-off-by: Felipe Balbi
diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
index c4c9e9e..59c4854 100644
--- a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
@@ -48,5 +48,5 @@ Optional properties:
peripheral means it is device controller
otg means it can operate as either ("on the go")
-Required properties for dr_mode == otg:
+VBUS control (required for dr_mode == host, optional for dr_mode == otg):
- vbus-supply: regulator for VBUS
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 2/3] Documentation: devicetree: phy-tegra-usb: Add vbus-supply property for host mode PHYs
On 07/29/2013 03:23 PM, Felipe Balbi wrote: > Hi, > > On Tue, Jul 30, 2013 at 12:19:32AM +0300, Felipe Balbi wrote: >> On Mon, Jul 29, 2013 at 09:24:46AM -0600, Stephen Warren wrote: >>> On 06/28/2013 06:33 AM, Mikko Perttunen wrote: Add vbus-supply as an optional property for host mode phy-tegra-usb PHYs. >>> >>> Felipe, I see that you didn't merge this one patch from this >>> series. Was there a reason for that? At this stage, I would >>> consider it reasonable for DT binding documentation changes to >>> go through the same tree as driver changes. >> >> no reason, just missed it. Will apply now. > > actually, patch _is_ in my tree: > > https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=next&id=80bd8a94712a0a9f2baeb03ad0546dd70f27ac4e Oh > so it is, thanks. I was thrown off by the modified patch subject; sorry. -- 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 v3] usb: phy-samsung-usb: Simplify PMU register handling
This patch simplifies the way the phy-samsung-usb code finds the correct
power management register to enable PHY clock gating. Previously, the
code would calculate the register address from a device tree supplied
base address and add an offset based on the PHY type.
Since every PHY has its own device tree entry and needs only one
register, we can just encode the address itself in the device tree and
remove the diffentiation in the code. The bitmask needed to specify the
bit within that register stays in place, allowing support for platforms
like s3c64xx that use different bits within the same register. Due to
this simplification, the whole complication of a Samsung-specific USB
PHY type can be removed from the PHY driver.
Signed-off-by: Julius Werner
---
arch/arm/boot/dts/exynos5250.dtsi | 4 +--
drivers/usb/phy/phy-samsung-usb.c | 51 +-
drivers/usb/phy/phy-samsung-usb.h | 34 ++---
drivers/usb/phy/phy-samsung-usb2.c | 34 +++--
drivers/usb/phy/phy-samsung-usb3.c | 13 +++---
5 files changed, 34 insertions(+), 102 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250.dtsi
b/arch/arm/boot/dts/exynos5250.dtsi
index ef57277..5a754d7 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -473,7 +473,7 @@
ranges;
usbphy-sys {
- reg = <0x10040704 0x8>;
+ reg = <0x10040704 0x4>;
};
};
@@ -505,7 +505,7 @@
ranges;
usbphy-sys {
- reg = <0x10040704 0x8>,
+ reg = <0x10040708 0x4>,
<0x10050230 0x4>;
};
};
diff --git a/drivers/usb/phy/phy-samsung-usb.c
b/drivers/usb/phy/phy-samsung-usb.c
index ac025ca..32c5264 100644
--- a/drivers/usb/phy/phy-samsung-usb.c
+++ b/drivers/usb/phy/phy-samsung-usb.c
@@ -27,7 +27,6 @@
#include
#include
#include
-#include
#include "phy-samsung-usb.h"
@@ -42,9 +41,9 @@ int samsung_usbphy_parse_dt(struct samsung_usbphy *sphy)
return -ENODEV;
}
- sphy->pmuregs = of_iomap(usbphy_sys, 0);
+ sphy->pmureg = of_iomap(usbphy_sys, 0);
- if (sphy->pmuregs == NULL) {
+ if (sphy->pmureg == NULL) {
dev_err(sphy->dev, "Can't get usb-phy pmu control register\n");
goto err0;
}
@@ -75,35 +74,25 @@ EXPORT_SYMBOL_GPL(samsung_usbphy_parse_dt);
*/
void samsung_usbphy_set_isolation_4210(struct samsung_usbphy *sphy, bool on)
{
- void __iomem *reg = NULL;
u32 reg_val;
- u32 en_mask = 0;
- if (!sphy->pmuregs) {
+ if (!sphy->pmureg) {
dev_warn(sphy->dev, "Can't set pmu isolation\n");
return;
}
- if (sphy->phy_type == USB_PHY_TYPE_DEVICE) {
- reg = sphy->pmuregs + sphy->drv_data->devphy_reg_offset;
- en_mask = sphy->drv_data->devphy_en_mask;
- } else if (sphy->phy_type == USB_PHY_TYPE_HOST) {
- reg = sphy->pmuregs + sphy->drv_data->hostphy_reg_offset;
- en_mask = sphy->drv_data->hostphy_en_mask;
- }
-
- reg_val = readl(reg);
+ reg_val = readl(sphy->pmureg);
if (on)
- reg_val &= ~en_mask;
+ reg_val &= ~sphy->drv_data->phy_en_mask;
else
- reg_val |= en_mask;
+ reg_val |= sphy->drv_data->phy_en_mask;
- writel(reg_val, reg);
+ writel(reg_val, sphy->pmureg);
if (sphy->drv_data->cpu_type == TYPE_EXYNOS4X12) {
- writel(reg_val, sphy->pmuregs + EXYNOS4X12_PHY_HSIC_CTRL0);
- writel(reg_val, sphy->pmuregs + EXYNOS4X12_PHY_HSIC_CTRL1);
+ writel(reg_val, sphy->pmureg + EXYNOS4X12_PHY_HSIC_CTRL0);
+ writel(reg_val, sphy->pmureg + EXYNOS4X12_PHY_HSIC_CTRL1);
}
}
EXPORT_SYMBOL_GPL(samsung_usbphy_set_isolation_4210);
@@ -111,7 +100,7 @@ EXPORT_SYMBOL_GPL(samsung_usbphy_set_isolation_4210);
/*
* Configure the mode of working of usb-phy here: HOST/DEVICE.
*/
-void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy)
+void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy, bool device_mode)
{
u32 reg;
@@ -122,31 +111,15 @@ void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy)
reg = readl(sphy->sysreg);
- if (sphy->phy_type == USB_PHY_TYPE_DEVICE)
+ if (device_mode)
reg &= ~EXYNOS_USB20PHY_CFG_HOST_LINK;
- else if (sphy->phy_type == USB_PHY_TYPE_HOST)
+ else
reg |= EXYNOS_USB20PHY_CFG_HOST_LINK;
writel(reg, sphy->sysreg);
}
EXPORT_SYMBOL_GPL(samsung_usbphy_cfg_sel);
-/*
- * PHYs are different for USB Device and USB Host.
- * This make sure that correct PHY type is selected before
- * any operation on PHY.
- */
-int samsung_usbphy_set_type(struct usb_phy *phy
RE: USB Interrupt Transaction Scheduling
> > Becides just testing to see if we do follow these things, what is your end > goal > here? Are you trying to determine how many devices you should be able to > successfully plug into a machine and have them work properly? > > Or something else? In our first release, we are considering 8 devices giving us a total of 24 interrupt endpoints at 64 bytes each. Not all endpoints would be sending data every millisecond, but we want the bandwidth reserved over bulk traffic. Based on the USB specification, we were investigating how to reduce the endpoint sizes to meet the limit in the standard. After this testing, it appears that we allocate more than what is listed in the USB 2.0 full-speed maximum limits in table 5.7. > > Watch out when using USB 1.1 hubs plugged into a EHCI controller, there are > some "tricky" things happening at times that can get messy. I'd not > recommend doing that if you can avoid it. > Our design uses USB 2.0 high speed hubs with the EHCI driver. The use of USB 1.1 hubs was for this testing only. > Setup #2: > Kernel 3.10.0 > Connect 7 USB devices (each with 2 IN and 1 OUT interrupt endpoints > @64 bytes) to USB 2.0 high speed hubs to a PC USB 2.0 port. The test > application can communicate with all 21 endpoints. This appears to > violate the full speed limitation; however, it wouldn't be violating > the high speed limitation of 63 endpoints per micro-frame. > > You don't say what the devices are rated to be, full/low/high/super speed, > which matters a lot here. > In both Setup #1 and Setup #2 the 7 USB devices were running USB 2.0 full speed. Thanks, -Nate -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: USB Interrupt Transaction Scheduling
> -Original Message- > > 1. Does this number take the USB requirement that "no more than 90% > > of any frame be allocated for periodic full-speed transfers" into > > account? > > No. You can tell because the table says that there would be 37 bytes > remaining, which is less than 10% of the 1500 bytes total. The text also > mentions that the tables do not include the overhead associated with bit > stuffing. > > You'll get a more accurate picture from the formulas in section 5.11.3. > They indicate that only 15 64-byte full-speed interrupt transfers should be > possible during a frame (about 59 us per transfer). > Thanks. > > 2. Does the full speed limitation apply if full speed devices are > > connected to high speed hubs? > > It applies to the subset of devices attached to any particular Transaction > Translator. Some high-speed hubs have a separate TT for each port, whereas > others have only a single TT. > Our design's USB topology is as follows: FS device #1 -| | FS device #2 -| | |-- HS Hub #1--|---| FS device #3 -| || FS device #4 -| || || -- HS Hub #3 -- | | USB 2.0 HS Port on USB Host FS device #5 -| || FS device #6 -| || |-- HS Hub #2 --|---| FS device #7 -| | FS device #8 -| | All of the hubs support Multi-TT. Based on this topology, I would assume Hub #1 and Hub #2 perform the FS splitting, and the EHCI controller on the USB host performs the FS un-splitting. Hub #3 would only be passing high speed traffic between Hubs 1/2 and the PC. Is this correct? Does this hub topology and Multi-TT support mean that mean each USB device could be able to support up to 15 64-byte interrupt endpoints? If that is true, then would the high speed 63 interrupt transfers (@ 64-byte) limit become the bottleneck? > > 2a. If the high speed limitation is used: Does the scheduler > > multiplex each full speed device's split data packets over the 8 > > available micro-frames? > > To some extent. Not all 8 microframes are available (the spec prohibits > sending a Start Split packet during microframe 6), and the current ehci-hcd > implementation is not capable of using all the ones that are available. > However, it is capable of using at least 4 of the > 8 microframes. USB high-speed interrupt transfers can still be sent over all of the 8 micro-frames? > > > We performed some testing, but I don't want to make assumptions on > > these results alone. > > Setup #1: > > Kernel 3.10.0 > > Connect 6 USB devices (each with 2 IN and 1 OUT interrupt endpoints > > @64 bytes) to USB 1.1 full speed hubs to a PC USB 2.0 port. The test > > application can communicate with all 18 endpoints. When we connected > > a 7th device, the test application is unable to open and access the > > device. This makes sense because that would be 21 full speed > > endpoints. > > This doesn't sound right. What sort of host controller were you using? The PC has XHCI, EHCI and OHCI enabled. Test setup #1 is connected to a USB 2.0 port and the lsusb output shows the FS devices on the USB 1.1 bus. I think this means the OHCI driver is in use, but I'm not certain. > > > Setup #2: > > Kernel 3.10.0 > > Connect 7 USB devices (each with 2 IN and 1 OUT interrupt endpoints > > @64 bytes) to USB 2.0 high speed hubs to a PC USB 2.0 port. The test > > application can communicate with all 21 endpoints. This appears to > > violate the full speed limitation; however, it wouldn't be violating > > the high speed limitation of 63 endpoints per micro-frame. > > The limitation you are referring to (Table 5-8 in the spec) is for interrupt > transfers to high-speed devices. It does not apply to interrupt transfers to > full-speed devices. > That makes sense. When FS and HS devices are mixed on a USB 2.0 hub (and USB 2.0 port on a PC), how is the interrupt transfer limitation calculated? Does a FS interrupt endpoint (@ 64 bytes) count as 1 of the HS 63 transfer limit? Or does the scheduler handle multiplexing the FS transfers over the available 252 (4 micro-frames with 63 transfers available)? Thank you for your help. I'm trying to get a better understanding of the USB limitations, so our design does not fail as the system expands. -Nate -- 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 v13 02/14] usb: chipidea: imx: remove vbus regulator operation
Hi Peter,
On Fri, Jul 26, 2013 at 05:18:18PM +0800, Peter Chen wrote:
> Since we have added vbus reguatlor operation at common
> host file (chipidea/host.c), the glue layer vbus operation
> isn't needed any more.
>
> Tested-by: Marek Vasut
> Signed-off-by: Peter Chen
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 30 +++---
> 1 files changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 14362c0..d06355e 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -31,7 +31,6 @@ struct ci_hdrc_imx_data {
> struct usb_phy *phy;
> struct platform_device *ci_pdev;
> struct clk *clk;
> - struct regulator *reg_vbus;
> };
>
> static const struct usbmisc_ops *usbmisc_ops;
> @@ -141,22 +140,13 @@ static int ci_hdrc_imx_probe(struct platform_device
> *pdev)
> goto err_clk;
> }
>
> - /* we only support host now, so enable vbus here */
> - data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> - if (!IS_ERR(data->reg_vbus)) {
> - ret = regulator_enable(data->reg_vbus);
> - if (ret) {
> - dev_err(&pdev->dev,
> - "Failed to enable vbus regulator, err=%d\n",
> - ret);
> - goto err_clk;
> - }
> - } else {
> - data->reg_vbus = NULL;
> - }
> -
> pdata.phy = data->phy;
>
> + /* Get the vbus regulator */
> + pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> + if (IS_ERR(pdata.reg_vbus))
> + pdata.reg_vbus = NULL;
> +
This hunk needs the reg_vbus variable from the previous patch, therefor
it should also be added in that patch. As the user of the variable is
the previous patch, it's the reason to swap their order.
Anyway, can't this be done in core.c instead? I don't see why other
users don't need this code.
> if (!pdev->dev.dma_mask)
> pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> if (!pdev->dev.coherent_dma_mask)
> @@ -167,7 +157,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> if (ret) {
> dev_err(&pdev->dev,
> "usbmisc init failed, ret=%d\n", ret);
> - goto err;
> + goto err_clk;
> }
> }
>
> @@ -179,7 +169,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> dev_err(&pdev->dev,
> "Can't register ci_hdrc platform device, err=%d\n",
> ret);
> - goto err;
> + goto err_clk;
> }
>
> if (usbmisc_ops && usbmisc_ops->post) {
> @@ -200,9 +190,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>
> disable_device:
> ci_hdrc_remove_device(data->ci_pdev);
> -err:
> - if (data->reg_vbus)
> - regulator_disable(data->reg_vbus);
> err_clk:
> clk_disable_unprepare(data->clk);
> return ret;
> @@ -215,9 +202,6 @@ static int ci_hdrc_imx_remove(struct platform_device
> *pdev)
> pm_runtime_disable(&pdev->dev);
> ci_hdrc_remove_device(data->ci_pdev);
>
> - if (data->reg_vbus)
> - regulator_disable(data->reg_vbus);
> -
> if (data->phy) {
> usb_phy_shutdown(data->phy);
> module_put(data->phy->dev->driver->owner);
> --
> 1.7.0.4
>
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
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 v13 03/14] usb: chipidea: imx: add return value check for devm_regulator_get
On Fri, Jul 26, 2013 at 05:18:19PM +0800, Peter Chen wrote:
> - If devm_regulator_get returns -EPROBE_DEFER, we also return
> -EPROBE_DEFER to wait regulator being ready later.
> - If devm_regulator_get returns -ENODEV, we think there is
> no "vbus-supply" node at DT, it means this board doesn't need
> vbus control.
> - If devm_regulator_get returns other error values, it means
> there are something wrong for getting this regulator.
>
> Signed-off-by: Peter Chen
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 14 --
> 1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index d06355e..0ced8c1 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -144,8 +144,18 @@ static int ci_hdrc_imx_probe(struct platform_device
> *pdev)
>
> /* Get the vbus regulator */
> pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> - if (IS_ERR(pdata.reg_vbus))
> - pdata.reg_vbus = NULL;
> + if (PTR_ERR(pdata.reg_vbus) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto err_clk;
> + } else if (PTR_ERR(pdata.reg_vbus) == -ENODEV) {
> + pdata.reg_vbus = NULL; /* no vbus regualator is needed */
> + } else if (IS_ERR(pdata.reg_vbus)) {
> + dev_err(&pdev->dev,
> + "Getting regulator error: %ld\n",
> + PTR_ERR(pdata.reg_vbus));
> + ret = PTR_ERR(pdata.reg_vbus);
> + goto err_clk;
> + }
>
> if (!pdev->dev.dma_mask)
> pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> --
This is wrong, you should squash that into the previous patch. And
as already mentioned, this can probably go into core.c as well.
Pick up the habit *not* to change code in one series which another patch
of the same series introduced. This only adds *dusty* unused history in the
patchstack that nobody needs. A *clean* and *coherent* series with discrete
patches is much easier to review and will get accepted much faster.
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: f_rndis: fix error return code in rndis_bind()
From: Wei Yongjun
Fix to return -EINVAL in the vendor param set error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/f_rndis.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c
index 717ed7f..8797563 100644
--- a/drivers/usb/gadget/f_rndis.c
+++ b/drivers/usb/gadget/f_rndis.c
@@ -794,8 +794,10 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *f)
if (rndis->manufacturer && rndis->vendorID &&
rndis_set_param_vendor(rndis->config, rndis->vendorID,
- rndis->manufacturer))
+ rndis->manufacturer)) {
+ status = -EINVAL;
goto fail;
+ }
/* NOTE: all that is done without knowing or caring about
* the network link ... which is unavailable to this code
--
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: Panasonic DMC-ZS3 mass storage USB regression in 3.10.3
On Mon, Jul 29, 2013 at 11:58 AM, Alan Stern wrote: > On Mon, 29 Jul 2013, Marc Meledandri wrote: > >> On Mon, Jul 29, 2013 at 11:24 AM, Alan Stern >> wrote: >> > On Sun, 28 Jul 2013, Marc Meledandri wrote: >> > >> >> I've hit a USB regression from 3.10.2 (working) -> 3.10.3 (busted) with my >> >> Panasonic DMC-ZS3 camera. I can no longer mount the mass storage device. >> > >> >> [ 197.976105] sd 6:0:0:0: [sdf] Write Protect is off >> >> [ 197.976114] sd 6:0:0:0: [sdf] Mode Sense: 04 00 00 00 >> >> [ 197.976730] sd 6:0:0:0: [sdf] No Caching mode page present >> >> [ 197.976740] sd 6:0:0:0: [sdf] Assuming drive cache: write through >> >> [ 228.144912] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> >> [ 259.216526] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> >> [ 290.192146] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> >> [ 321.167769] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> > >> > See this email thread: >> > >> > http://marc.info/?l=linux-usb&m=137503976012984&w=2 >> > >> > Alan Stern >> > >> >> Thanks Alan. >> >> I thought it might be the same issue, but wasn't sure if the other thread was >> specific to USB 3.0 devices. > > It isn't, in spite of what the Subject: line says. > > Alan Stern > Okay, I too can confirm the patch in the aforementioned other thread fixes my issue as well. Thanks for the quick fix Martin! On Mon, Jul 29, 2013 at 11:58 AM, Alan Stern wrote: > On Mon, 29 Jul 2013, Marc Meledandri wrote: > >> On Mon, Jul 29, 2013 at 11:24 AM, Alan Stern >> wrote: >> > On Sun, 28 Jul 2013, Marc Meledandri wrote: >> > >> >> I've hit a USB regression from 3.10.2 (working) -> 3.10.3 (busted) with my >> >> Panasonic DMC-ZS3 camera. I can no longer mount the mass storage device. >> > >> >> [ 197.976105] sd 6:0:0:0: [sdf] Write Protect is off >> >> [ 197.976114] sd 6:0:0:0: [sdf] Mode Sense: 04 00 00 00 >> >> [ 197.976730] sd 6:0:0:0: [sdf] No Caching mode page present >> >> [ 197.976740] sd 6:0:0:0: [sdf] Assuming drive cache: write through >> >> [ 228.144912] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> >> [ 259.216526] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> >> [ 290.192146] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> >> [ 321.167769] usb 6-1.7: reset high-speed USB device number 3 using >> >> ehci-pci >> > >> > See this email thread: >> > >> > http://marc.info/?l=linux-usb&m=137503976012984&w=2 >> > >> > Alan Stern >> > >> >> Thanks Alan. >> >> I thought it might be the same issue, but wasn't sure if the other thread was >> specific to USB 3.0 devices. > > It isn't, in spite of what the Subject: line says. > > Alan Stern > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: Fix Command Ring Stopped Event handling
Hi there,
We met a panic issue with a 3.5-based kernel, code at
git://kernel.ubuntu.com/ubuntu/ubuntu-quantal.git.
call trace as:
[ 27.508480] BUG: unable to handle kernel NULL pointer dereference
at 03c8
[ 27.544645] Call Trace:
[ 27.545060]
[ 27.545353] []
handle_set_deq_completion.isra.37+0x61/0x250
[ 27.546610] [] ? handle_stopped_cmd_ring+0x175/0x190
[ 27.547785] [] handle_cmd_completion+0x1d3/0x440
[ 27.548833] [] xhci_handle_event+0x14d/0x1d0
[ 27.549881] [] xhci_irq+0xa8/0x1d0
[ 27.550800] [] xhci_msi_irq+0x11/0x20
[ 27.551722] [] handle_irq_event_percpu+0x55/0x210
[ 27.552899] [] ? msi_set_mask_bit+0x6f/0x80
[ 27.553959] [] handle_irq_event+0x4e/0x80
[ 27.555007] [] handle_edge_irq+0x84/0x130
[ 27.556054] [] handle_irq+0x22/0x40
[ 27.556975] [] do_IRQ+0x5a/0xe0
[ 27.557770] [] common_interrupt+0x6a/0x6a
[ 27.558816]
[ 27.559105] [] ? arch_local_irq_enable+0xb/0xd
[ 27.561294] [] ? sched_clock_idle_wakeup_event+0x1a/0x20
[ 27.563067] [] acpi_idle_enter_bm+0x24a/0x28e
[ 27.564709] [] ? menu_select+0xe7/0x2e0
[ 27.566215] [] cpuidle_enter+0x19/0x20
[ 27.567716] [] cpuidle_idle_call+0xac/0x2a0
[ 27.569352] [] cpu_idle+0xcf/0x120
[ 27.570856] [] start_secondary+0xc3/0xc5
[ 27.572342] Code: 8d 44 37 20 48 8d 48 08 74 21 48 8b 49 08 31 c0
48 85 c9 74 0e 3b 51 08 77 09 48 8b 01 89 d2 48 8b 04 d0 5d c3 66 0f
1f 44 00 00 <48> 8b 40 08 5d c3 66 2e 0f 1f 84 00 00 00 00 00 55 48 89
e5 0f
[ 27.581968] RIP [] xhci_stream_id_to_ring+0x40/0x50
After applying this patch, this issue is fixed. And USB works as normal.
Tested-by: Yang Bai
On Sat, May 25, 2013 at 9:33 AM, Julius Werner wrote:
> The current XHCI code treats a command completion event with the
> COMP_CMD_STOP code as a slightly different version of COMP_CMD_ABORT. In
> particular, it puts the pointed-to command TRB through the normal
> command completion handlers. This is not how this event works.
>
> As XHCI spec 4.6.1.1 describes, unlike other command completion events
> Command Ring Stopped sets the Command TRB Pointer to the *current*
> Command Ring Dequeue Pointer. This is the command TRB that the XHCI will
> execute next, and not a command that has already been executed. The
> driver's internal command ring dequeue pointer should not be increased
> after this event, since it does not really mark a command completion...
> it's just a hint for the driver that execution is stopped now and where
> it will be picked up again if the ring gets restarted.
>
> This patch gets rid of the handle_stopped_command_ring() function and
> splits its behavior into two distinct parts for COMP_CMD_STOP and
> COMP_CMD_ABORT events. It ensures that the former no longer messes with
> the dequeue pointer, while the latter's behavior is unchanged. This
> prevents the hardware and software dequeue pointer from going out of
> sync during command cancellations, which can lead to a variety of
> problems (up to a total HCD death if the next command after the one that
> was cancelled is Stop Endpoint, and the stop_endpoint_watchdog won't see
> the command's completion because it got skipped by the software dequeue
> pointer).
>
> This patch should be backported to kernels as far as 3.0 that contain
> the commit b63f4053cc8aa22a98e3f9a97845afe6c15d0a0d "xHCI: handle
> command after aborting the command ring".
>
> Signed-off-by: Julius Werner
> ---
> drivers/usb/host/xhci-ring.c | 85
> +---
> 1 file changed, 33 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 1969c00..98b7673 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1314,47 +1314,11 @@ static int xhci_search_cmd_trb_in_cd_list(struct
> xhci_hcd *xhci,
> return 0;
> }
>
> -/*
> - * If the cmd_trb_comp_code is COMP_CMD_ABORT, we just check whether the
> - * trb pointed by the command ring dequeue pointer is the trb we want to
> - * cancel or not. And if the cmd_trb_comp_code is COMP_CMD_STOP, we will
> - * traverse the cancel_cmd_list to trun the all of the commands according
> - * to command descriptor to NO-OP trb.
> - */
> -static int handle_stopped_cmd_ring(struct xhci_hcd *xhci,
> - int cmd_trb_comp_code)
> -{
> - int cur_trb_is_good = 0;
> -
> - /* Searching the cmd trb pointed by the command ring dequeue
> -* pointer in command descriptor list. If it is found, free it.
> -*/
> - cur_trb_is_good = xhci_search_cmd_trb_in_cd_list(xhci,
> - xhci->cmd_ring->dequeue);
> -
> - if (cmd_trb_comp_code == COMP_CMD_ABORT)
> - xhci->cmd_ring_state = CMD_RING_STATE_STOPPED;
> - else if (cmd_trb_comp_code == COMP_CMD_STOP) {
> - /* traversing the cancel_cmd_list and canceling
> -* the command according to command descriptor
> -*/
> -
Re: [PATCH v13 02/14] usb: chipidea: imx: remove vbus regulator operation
On Tue, Jul 30, 2013 at 12:30:25AM +0200, Michael Grzeschik wrote:
> Hi Peter,
>
> On Fri, Jul 26, 2013 at 05:18:18PM +0800, Peter Chen wrote:
> > Since we have added vbus reguatlor operation at common
> > host file (chipidea/host.c), the glue layer vbus operation
> > isn't needed any more.
> >
> > Tested-by: Marek Vasut
> > Signed-off-by: Peter Chen
> > ---
> > drivers/usb/chipidea/ci_hdrc_imx.c | 30 +++---
> > 1 files changed, 7 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> > b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 14362c0..d06355e 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -31,7 +31,6 @@ struct ci_hdrc_imx_data {
> > struct usb_phy *phy;
> > struct platform_device *ci_pdev;
> > struct clk *clk;
> > - struct regulator *reg_vbus;
> > };
> >
> > static const struct usbmisc_ops *usbmisc_ops;
> > @@ -141,22 +140,13 @@ static int ci_hdrc_imx_probe(struct platform_device
> > *pdev)
> > goto err_clk;
> > }
> >
> > - /* we only support host now, so enable vbus here */
> > - data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > - if (!IS_ERR(data->reg_vbus)) {
> > - ret = regulator_enable(data->reg_vbus);
> > - if (ret) {
> > - dev_err(&pdev->dev,
> > - "Failed to enable vbus regulator, err=%d\n",
> > - ret);
> > - goto err_clk;
> > - }
> > - } else {
> > - data->reg_vbus = NULL;
> > - }
> > -
> > pdata.phy = data->phy;
> >
> > + /* Get the vbus regulator */
> > + pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > + if (IS_ERR(pdata.reg_vbus))
> > + pdata.reg_vbus = NULL;
> > +
>
> This hunk needs the reg_vbus variable from the previous patch, therefor
> it should also be added in that patch. As the user of the variable is
> the previous patch, it's the reason to swap their order.
>
The [1/14] implements the vbus operation at common code (host.c)
This one [2/14] implements how the glue layer get the vbus.
I am OK to swap above two, but I still can't see obvious reason.
> Anyway, can't this be done in core.c instead? I don't see why other
> users don't need this code.
>
We still not implement DT support at core.c, it is a big job, this
is the main reason, besides, Alex prefers platform things at
glue layer.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 03/14] usb: chipidea: imx: add return value check for devm_regulator_get
On Tue, Jul 30, 2013 at 12:47:17AM +0200, Michael Grzeschik wrote:
> On Fri, Jul 26, 2013 at 05:18:19PM +0800, Peter Chen wrote:
> > - If devm_regulator_get returns -EPROBE_DEFER, we also return
> > -EPROBE_DEFER to wait regulator being ready later.
> > - If devm_regulator_get returns -ENODEV, we think there is
> > no "vbus-supply" node at DT, it means this board doesn't need
> > vbus control.
> > - If devm_regulator_get returns other error values, it means
> > there are something wrong for getting this regulator.
> >
> > Signed-off-by: Peter Chen
> > ---
> > drivers/usb/chipidea/ci_hdrc_imx.c | 14 --
> > 1 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> > b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index d06355e..0ced8c1 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -144,8 +144,18 @@ static int ci_hdrc_imx_probe(struct platform_device
> > *pdev)
> >
> > /* Get the vbus regulator */
> > pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > - if (IS_ERR(pdata.reg_vbus))
> > - pdata.reg_vbus = NULL;
> > + if (PTR_ERR(pdata.reg_vbus) == -EPROBE_DEFER) {
> > + ret = -EPROBE_DEFER;
> > + goto err_clk;
> > + } else if (PTR_ERR(pdata.reg_vbus) == -ENODEV) {
> > + pdata.reg_vbus = NULL; /* no vbus regualator is needed */
> > + } else if (IS_ERR(pdata.reg_vbus)) {
> > + dev_err(&pdev->dev,
> > + "Getting regulator error: %ld\n",
> > + PTR_ERR(pdata.reg_vbus));
> > + ret = PTR_ERR(pdata.reg_vbus);
> > + goto err_clk;
> > + }
> >
> > if (!pdev->dev.dma_mask)
> > pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> > --
>
> This is wrong, you should squash that into the previous patch. And
> as already mentioned, this can probably go into core.c as well.
>
> Pick up the habit *not* to change code in one series which another patch
> of the same series introduced. This only adds *dusty* unused history in the
> patchstack that nobody needs. A *clean* and *coherent* series with discrete
> patches is much easier to review and will get accepted much faster.
>
My rule is do ONE thing at ONE patch, is it not correct?
Previous one[2/14]: Remove the vbus operation at imx glue layer.
This one [3/14]: Fix a bug that vbus may be gotten delay (EPROBE_DEFER),
and vbus is valid at this case.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] usb: phy: phy-omap-control: Add API to power and wakeup
On 7/29/2013 7:55 PM, Sebastian Andrzej Siewior wrote:
* George Cherian | 2013-07-19 18:04:34 [+0530]:
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index 1419ced..4f2502c 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -46,6 +46,73 @@ struct device *omap_get_control_dev(void)
EXPORT_SYMBOL_GPL(omap_get_control_dev);
/**
+ * omap_control_am335x_phy_wkup - PHY wakeup on/off using control
+ * module
+ * @dev: the control module device
+ * @on: 0 to off and 1 to on PHY wakeup feature
+ * @id: phy id 0 or 1 for phy instance 0 and 1 repectively
+ *
+ * AM PHY driver should call this API to enable or disable PHY wakeup.
+ */
+void omap_control_am335x_phy_wkup(struct device *dev, bool on, u8 id)
+{
+ u32 val;
+ u32 *phy_wkup_reg;
+ struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+
+ if (control_usb->type != OMAP_CTRL_DEV_TYPE3 || id < 0 || id > 1)
+ return;
+
+ phy_wkup_reg = control_usb->dev_conf + AM335X_USB_WKUP_OFFSET;
+ val = readl(phy_wkup_reg);
Where do you get the memory from? In the DT patches I've made I export
one memory space of 8 bytes for each phy. This memory space applies
directly to the on/off register.
Control module have 2 separate registers for phy on/off per instance
(offset 0x620 and 0x628), where as
wkup_ctrl is a shared control module register (offset 0x648). Currently
the control module driver maps
memory from 0x620 till beyond 0x648 and uses the same mapping for
writing to wkup_reg.
I didn't export the register for wakeup
so far since I had no idea how this will be used.
So how do you get your re-mapped memory here?
Mapping it per instance is not possible since its the same register (if
we use devm_* calls).
--
-George
--
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: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
Hi, On Monday 29 July 2013 09:21 PM, Sylwester Nawrocki wrote: > On 07/26/2013 02:49 PM, Kishon Vijay Abraham I wrote: >> The PHY framework provides a set of APIs for the PHY drivers to >> create/destroy a PHY and APIs for the PHY users to obtain a reference to the >> PHY with or without using phandle. For dt-boot, the PHY drivers should >> also register *PHY provider* with the framework. >> >> PHY drivers should create the PHY by passing id and ops like init, exit, >> power_on and power_off. This framework is also pm runtime enabled. >> >> The documentation for the generic PHY framework is added in >> Documentation/phy.txt and the documentation for dt binding can be found at >> Documentation/devicetree/bindings/phy/phy-bindings.txt >> >> Cc: Tomasz Figa >> Cc: Greg Kroah-Hartman >> Signed-off-by: Kishon Vijay Abraham I >> Acked-by: Felipe Balbi >> Tested-by: Sylwester Nawrocki >> --- >> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++ >> Documentation/phy.txt | 166 + >> MAINTAINERS|8 + >> drivers/Kconfig|2 + >> drivers/Makefile |2 + >> drivers/phy/Kconfig| 18 + >> drivers/phy/Makefile |5 + >> drivers/phy/phy-core.c | 714 >> >> include/linux/phy/phy.h| 270 >> 9 files changed, 1251 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt >> create mode 100644 Documentation/phy.txt >> create mode 100644 drivers/phy/Kconfig >> create mode 100644 drivers/phy/Makefile >> create mode 100644 drivers/phy/phy-core.c >> create mode 100644 include/linux/phy/phy.h >> >> diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt >> b/Documentation/devicetree/bindings/phy/phy-bindings.txt >> new file mode 100644 >> index 000..8ae844f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt >> @@ -0,0 +1,66 @@ >> +This document explains only the device tree data binding. For general >> +information about PHY subsystem refer to Documentation/phy.txt > [...] >> @@ -0,0 +1,166 @@ >> +PHY SUBSYSTEM >> + Kishon Vijay Abraham I >> + >> +This document explains the Generic PHY Framework along with the APIs >> provided, >> +and how-to-use. >> + >> +1. Introduction >> + >> +*PHY* is the abbreviation for physical layer. It is used to connect a device >> +to the physical medium e.g., the USB controller has a PHY to provide >> functions >> +such as serialization, de-serialization, encoding, decoding and is >> responsible >> +for obtaining the required data transmission rate. Note that some USB >> +controllers have PHY functionality embedded into it and others use an >> external >> +PHY. Other peripherals that use PHY include Wireless LAN, Ethernet, >> +SATA etc. >> + >> +The intention of creating this framework is to bring the PHY drivers spread >> +all over the Linux kernel to drivers/phy to increase code re-use and for >> +better code maintainability. >> + >> +This framework will be of use only to devices that use external PHY (PHY >> +functionality is not embedded within the controller). >> + >> +2. Registering/Unregistering the PHY provider >> + >> +PHY provider refers to an entity that implements one or more PHY instances. >> +For the simple case where the PHY provider implements only a single >> instance of >> +the PHY, the framework provides its own implementation of of_xlate in >> +of_phy_simple_xlate. If the PHY provider implements multiple instances, it >> +should provide its own implementation of of_xlate. of_xlate is used only for >> +dt boot case. >> + >> +#define of_phy_provider_register(dev, xlate)\ >> +__of_phy_provider_register((dev), THIS_MODULE, (xlate)) >> + >> +#define devm_of_phy_provider_register(dev, xlate) \ >> +__of_phy_provider_register((dev), THIS_MODULE, (xlate)) > > This needs to be: > > __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate)) > > as Kamil pointed out. We've tested it here with v9 and it makes > Bad Things happen. ;) > >> +of_phy_provider_register and devm_of_phy_provider_register macros can be >> used to >> +register the phy_provider and it takes device and of_xlate as >> +arguments. For the dt boot case, all PHY providers should use one of the >> above >> +2 macros to register the PHY provider. >> + >> +void devm_of_phy_provider_unregister(struct device *dev, >> +struct phy_provider *phy_provider); >> +void of_phy_provider_unregister(struct phy_provider *phy_provider); >> + >> +devm_of_phy_provider_unregister and of_phy_provider_unregister can be used >> to >> +unregister the PHY. >> + > [...] >> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c >> new file mode 100644 >> ind
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Monday 29 July 2013 11:24 PM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Jul 29, 2013 at 08:59:26PM +0530, Kishon Vijay Abraham I wrote:
Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while
creating
MUSB core device. So in usb_bind_phy (binds the controller with the PHY),
the
device name of the controller had *.auto* in it. Since with using
PLATFORM_DEVID_AUTO, there is no way to know the exact device name in
advance,
the data given in usb_bind_phy became obsolete and usb_get_phy was failing.
So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO. Corresponding
change is done in board file here.
Signed-off-by: Kishon Vijay Abraham I
---
arch/arm/mach-omap2/board-2430sdp.c |2 +-
arch/arm/mach-omap2/board-3430sdp.c |2 +-
arch/arm/mach-omap2/board-cm-t35.c |2 +-
arch/arm/mach-omap2/board-devkit8000.c |2 +-
arch/arm/mach-omap2/board-igep0020.c |2 +-
arch/arm/mach-omap2/board-ldp.c |2 +-
arch/arm/mach-omap2/board-omap3beagle.c |2 +-
arch/arm/mach-omap2/board-omap3evm.c |2 +-
arch/arm/mach-omap2/board-omap3logic.c |2 +-
arch/arm/mach-omap2/board-omap3pandora.c |2 +-
arch/arm/mach-omap2/board-omap3stalker.c |2 +-
arch/arm/mach-omap2/board-omap3touchbook.c |2 +-
arch/arm/mach-omap2/board-overo.c|2 +-
arch/arm/mach-omap2/board-rm680.c|2 +-
arch/arm/mach-omap2/board-rx51.c |2 +-
arch/arm/mach-omap2/board-zoom-peripherals.c |2 +-
16 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-omap2/board-2430sdp.c
b/arch/arm/mach-omap2/board-2430sdp.c
index 244d8a5..17bb076 100644
--- a/arch/arm/mach-omap2/board-2430sdp.c
+++ b/arch/arm/mach-omap2/board-2430sdp.c
@@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
omap_hsmmc_init(mmc);
omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA | OMAP_PULL_UP);
- usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+ usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
>>>
>>> how about moving usb_bind_phy() calls to omap2430.c ?
>>>
>>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
>>> index f44e8b5..b6abc1a 100644
>>> --- a/drivers/usb/musb/omap2430.c
>>> +++ b/drivers/usb/musb/omap2430.c
>>> @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device *pdev)
>>>
>>> pdata->board_data = data;
>>> pdata->config = config;
>>> + } else {
>>> + /* bind the PHY */
>>> + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
>>
>> This looks like a hack IMHO to workaround the usb phy library. otherwise it
>> is
>> similar to get_phy_by_name.
>
> actually, this is a workaround to the fact that we're not creating all
> platform_devices in arch/arm/mach-omap2/ :-)
>
> If we had the musb allocation there, we could easily handle
> usb_bind_phy()
>
>>> so that's temporary. It might be better than to reintroduce the IDR in
>>> musb_core.c.
>>
>> that’s needed for generic phy framework anyway :-s
>
> right, but generic phy framework can handle everything just fine, the
> only problem is that names are changing.
right. But if the names change, PHY framework wouldn't be able to return the
reference to the PHY.
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] phy: phy-amxxxx-usb: Add PHY driver for amxxxx platform
On 7/29/2013 8:47 PM, Sebastian Andrzej Siewior wrote: * George Cherian | 2013-07-19 18:04:35 [+0530]: Adds phy driver support for am33xx platform, the host/device peripheral controller shall get this phy object to control the phy operations. If you rebase this on-top of the two instances patches I've sent earlier then you can bury patch 3 and 4, right? Yes True. I posted it for completeness. I don't like very much the way you obtain the phy id: + of_property_read_u32(np, "id", &phy->id); with the .dts changes I made you should able to use of_alias_get_id() instead. Let me look what you have additionaly: - usbotg_fck Is this really required? I have the phy as a child of the "main" device which has a hwmod property which is associated with this clock. So pm enable/ disable should also enable this clock if possible, right? - device wakeup via omap_control_am335x_phy_wkup() Now. that is one thing that the simple phy driver is missing. If you call a magic function for this to happen than I don't have to worry about the missing memory space for this function. So from what I see now, it is most likely the easiest thing to just add that wakeup to the phy driver I posted. Do you agree? The whole idea of writing a seperate phy driver was to use the generic phy framework and most of the am devices have the same phy (eg am335x, am437x). Now since the register is shared in am335x for phy_wkup (Not in the case of am437x) how are you planning to map it. I feel if omap_control_usb can delegate the writes to phy_wkup, phy_on and phy_off, it makes the life simpler. Thoughts??? If so we need to figure out where the memory for the wakeup register is comming from. We need also to ensure that both phys can not write at the same time. A look would be nice. Signed-off-by: George Cherian Sebastian -- -George -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Tue, Jul 30, 2013 at 10:44:48AM +0530, Kishon Vijay Abraham I wrote:
> > On Mon, Jul 29, 2013 at 08:59:26PM +0530, Kishon Vijay Abraham I wrote:
> Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while
> creating
> MUSB core device. So in usb_bind_phy (binds the controller with the
> PHY), the
> device name of the controller had *.auto* in it. Since with using
> PLATFORM_DEVID_AUTO, there is no way to know the exact device name in
> advance,
> the data given in usb_bind_phy became obsolete and usb_get_phy was
> failing.
> So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO.
> Corresponding
> change is done in board file here.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> arch/arm/mach-omap2/board-2430sdp.c |2 +-
> arch/arm/mach-omap2/board-3430sdp.c |2 +-
> arch/arm/mach-omap2/board-cm-t35.c |2 +-
> arch/arm/mach-omap2/board-devkit8000.c |2 +-
> arch/arm/mach-omap2/board-igep0020.c |2 +-
> arch/arm/mach-omap2/board-ldp.c |2 +-
> arch/arm/mach-omap2/board-omap3beagle.c |2 +-
> arch/arm/mach-omap2/board-omap3evm.c |2 +-
> arch/arm/mach-omap2/board-omap3logic.c |2 +-
> arch/arm/mach-omap2/board-omap3pandora.c |2 +-
> arch/arm/mach-omap2/board-omap3stalker.c |2 +-
> arch/arm/mach-omap2/board-omap3touchbook.c |2 +-
> arch/arm/mach-omap2/board-overo.c|2 +-
> arch/arm/mach-omap2/board-rm680.c|2 +-
> arch/arm/mach-omap2/board-rx51.c |2 +-
> arch/arm/mach-omap2/board-zoom-peripherals.c |2 +-
> 16 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-2430sdp.c
> b/arch/arm/mach-omap2/board-2430sdp.c
> index 244d8a5..17bb076 100644
> --- a/arch/arm/mach-omap2/board-2430sdp.c
> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> @@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
> omap_hsmmc_init(mmc);
>
> omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA |
> OMAP_PULL_UP);
> -usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> +usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
> >>>
> >>> how about moving usb_bind_phy() calls to omap2430.c ?
> >>>
> >>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> >>> index f44e8b5..b6abc1a 100644
> >>> --- a/drivers/usb/musb/omap2430.c
> >>> +++ b/drivers/usb/musb/omap2430.c
> >>> @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device
> >>> *pdev)
> >>>
> >>> pdata->board_data = data;
> >>> pdata->config = config;
> >>> + } else {
> >>> + /* bind the PHY */
> >>> + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
> >>
> >> This looks like a hack IMHO to workaround the usb phy library. otherwise
> >> it is
> >> similar to get_phy_by_name.
> >
> > actually, this is a workaround to the fact that we're not creating all
> > platform_devices in arch/arm/mach-omap2/ :-)
> >
> > If we had the musb allocation there, we could easily handle
> > usb_bind_phy()
> >
> >>> so that's temporary. It might be better than to reintroduce the IDR in
> >>> musb_core.c.
> >>
> >> that’s needed for generic phy framework anyway :-s
> >
> > right, but generic phy framework can handle everything just fine, the
> > only problem is that names are changing.
>
> right. But if the names change, PHY framework wouldn't be able to return the
> reference to the PHY.
with my suggestion they can change whenever they want since we're using
dev_name() of the just-created musb platform_device. Right ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 2/3] Documentation: devicetree: phy-tegra-usb: Add vbus-supply property for host mode PHYs
On 07/30/2013 12:23 AM, Felipe Balbi wrote:
* PGP Signed by an unknown key
Hi,
On Tue, Jul 30, 2013 at 12:19:32AM +0300, Felipe Balbi wrote:
On Mon, Jul 29, 2013 at 09:24:46AM -0600, Stephen Warren wrote:
On 06/28/2013 06:33 AM, Mikko Perttunen wrote:
Add vbus-supply as an optional property for host mode phy-tegra-usb PHYs.
Felipe, I see that you didn't merge this one patch from this series. Was
there a reason for that? At this stage, I would consider it reasonable
for DT binding documentation changes to go through the same tree as
driver changes.
no reason, just missed it. Will apply now.
actually, patch _is_ in my tree:
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=next&id=80bd8a94712a0a9f2baeb03ad0546dd70f27ac4e
commit 80bd8a94712a0a9f2baeb03ad0546dd70f27ac4e
Author: Mikko Perttunen
Date: Wed Jul 17 10:37:50 2013 +0300
usb: tegra: Add vbus-supply property for host mode PHYs
Document vbus-supply as an optional property for host mode phy-tegra-usb
PHYs.
Signed-off-by: Mikko Perttunen
Reviewed-by: Stephen Warren
Tested-by: Stephen Warren
Signed-off-by: Felipe Balbi
diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
index c4c9e9e..59c4854 100644
--- a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
@@ -48,5 +48,5 @@ Optional properties:
peripheral means it is device controller
otg means it can operate as either ("on the go")
-Required properties for dr_mode == otg:
+VBUS control (required for dr_mode == host, optional for dr_mode == otg):
- vbus-supply: regulator for VBUS
Oops, looks like I've made a typo there while making the v2. This should
be, optional for dr_mode == host, required for dr_mode == otg instead of
the other way around. Can this fixup still be made?
Thanks, Mikko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Tuesday 30 July 2013 11:31 AM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jul 30, 2013 at 10:44:48AM +0530, Kishon Vijay Abraham I wrote:
>>> On Mon, Jul 29, 2013 at 08:59:26PM +0530, Kishon Vijay Abraham I wrote:
>> Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while
>> creating
>> MUSB core device. So in usb_bind_phy (binds the controller with the
>> PHY), the
>> device name of the controller had *.auto* in it. Since with using
>> PLATFORM_DEVID_AUTO, there is no way to know the exact device name in
>> advance,
>> the data given in usb_bind_phy became obsolete and usb_get_phy was
>> failing.
>> So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO.
>> Corresponding
>> change is done in board file here.
>>
>> Signed-off-by: Kishon Vijay Abraham I
>> ---
>> arch/arm/mach-omap2/board-2430sdp.c |2 +-
>> arch/arm/mach-omap2/board-3430sdp.c |2 +-
>> arch/arm/mach-omap2/board-cm-t35.c |2 +-
>> arch/arm/mach-omap2/board-devkit8000.c |2 +-
>> arch/arm/mach-omap2/board-igep0020.c |2 +-
>> arch/arm/mach-omap2/board-ldp.c |2 +-
>> arch/arm/mach-omap2/board-omap3beagle.c |2 +-
>> arch/arm/mach-omap2/board-omap3evm.c |2 +-
>> arch/arm/mach-omap2/board-omap3logic.c |2 +-
>> arch/arm/mach-omap2/board-omap3pandora.c |2 +-
>> arch/arm/mach-omap2/board-omap3stalker.c |2 +-
>> arch/arm/mach-omap2/board-omap3touchbook.c |2 +-
>> arch/arm/mach-omap2/board-overo.c|2 +-
>> arch/arm/mach-omap2/board-rm680.c|2 +-
>> arch/arm/mach-omap2/board-rx51.c |2 +-
>> arch/arm/mach-omap2/board-zoom-peripherals.c |2 +-
>> 16 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-2430sdp.c
>> b/arch/arm/mach-omap2/board-2430sdp.c
>> index 244d8a5..17bb076 100644
>> --- a/arch/arm/mach-omap2/board-2430sdp.c
>> +++ b/arch/arm/mach-omap2/board-2430sdp.c
>> @@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
>> omap_hsmmc_init(mmc);
>>
>> omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA |
>> OMAP_PULL_UP);
>> -usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
>> +usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
>
> how about moving usb_bind_phy() calls to omap2430.c ?
>
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index f44e8b5..b6abc1a 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device
> *pdev)
>
> pdata->board_data = data;
> pdata->config = config;
> + } else {
> + /* bind the PHY */
> + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
This looks like a hack IMHO to workaround the usb phy library. otherwise
it is
similar to get_phy_by_name.
>>>
>>> actually, this is a workaround to the fact that we're not creating all
>>> platform_devices in arch/arm/mach-omap2/ :-)
>>>
>>> If we had the musb allocation there, we could easily handle
>>> usb_bind_phy()
>>>
> so that's temporary. It might be better than to reintroduce the IDR in
> musb_core.c.
that’s needed for generic phy framework anyway :-s
>>>
>>> right, but generic phy framework can handle everything just fine, the
>>> only problem is that names are changing.
>>
>> right. But if the names change, PHY framework wouldn't be able to return the
>> reference to the PHY.
>
> with my suggestion they can change whenever they want since we're using
> dev_name() of the just-created musb platform_device. Right ?
right. But the PHY device can be created in a different place from where the
musb devices are created. And in the PHY framework, the PHY device should have
the list of controller device (names) it can support (PHY framework does not
maintain a separate list for binding like how we had in USB PHY library). e.g.
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92817.html. In such
cases how do we pass the device names. Also will the MUSB core device be
created before twl4030-usb PHY device?
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: of: fix build breakage caused by recent patches
commit 052a11d (usb: phy: make PHY driver selection possible by controller drivers) changed the rules on how drivers/usb/phy/of.c would be compiled and failed to update include/linux/usb/of.h accordingly. Because of that, we can fall into situations where of_usb_get_phy_mode() is redefined. In order to fix the error, we update the IS_ENABLED() check in include/linux/usb/of.h to reflect the condition where of.c is built. Reported-by: Stephen Rothwell Signed-off-by: Felipe Balbi --- include/linux/usb/of.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h index 5a7cb9e..8c38aa2 100644 --- a/include/linux/usb/of.h +++ b/include/linux/usb/of.h @@ -27,7 +27,7 @@ of_usb_get_maximum_speed(struct device_node *np) } #endif -#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_PHY) +#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_SUPPORT) enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np); #else static inline enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np) -- 1.8.3.4.840.g6a90778 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Tue, Jul 30, 2013 at 11:41:23AM +0530, Kishon Vijay Abraham I wrote:
> >> diff --git a/arch/arm/mach-omap2/board-2430sdp.c
> >> b/arch/arm/mach-omap2/board-2430sdp.c
> >> index 244d8a5..17bb076 100644
> >> --- a/arch/arm/mach-omap2/board-2430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> >> @@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
> >>omap_hsmmc_init(mmc);
> >>
> >>omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA |
> >> OMAP_PULL_UP);
> >> - usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> >> + usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
> >
> > how about moving usb_bind_phy() calls to omap2430.c ?
> >
> > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> > index f44e8b5..b6abc1a 100644
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device
> > *pdev)
> >
> > pdata->board_data = data;
> > pdata->config = config;
> > + } else {
> > + /* bind the PHY */
> > + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
>
> This looks like a hack IMHO to workaround the usb phy library. otherwise
> it is
> similar to get_phy_by_name.
> >>>
> >>> actually, this is a workaround to the fact that we're not creating all
> >>> platform_devices in arch/arm/mach-omap2/ :-)
> >>>
> >>> If we had the musb allocation there, we could easily handle
> >>> usb_bind_phy()
> >>>
> > so that's temporary. It might be better than to reintroduce the IDR in
> > musb_core.c.
>
> that’s needed for generic phy framework anyway :-s
> >>>
> >>> right, but generic phy framework can handle everything just fine, the
> >>> only problem is that names are changing.
> >>
> >> right. But if the names change, PHY framework wouldn't be able to return
> >> the
> >> reference to the PHY.
> >
> > with my suggestion they can change whenever they want since we're using
> > dev_name() of the just-created musb platform_device. Right ?
>
> right. But the PHY device can be created in a different place from where the
> musb devices are created. And in the PHY framework, the PHY device should have
this shouldn't be a problem. As long as the phy is created, all should
be good.
> the list of controller device (names) it can support (PHY framework does not
> maintain a separate list for binding like how we had in USB PHY library). e.g.
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92817.html. In such
this has nothing to do with $subject though. We talk about generic PHY
framework once all these PHY drivers are moved there :-)
> cases how do we pass the device names. Also will the MUSB core device be
> created before twl4030-usb PHY device?
and why would that be a problem ? We're telling the framework that the
musb device will use a phy with a name of 'twl4030'. If musb calls
usb_get_phy_dev() and doesn't find a phy, it'll return -EPROBE_DEFER and
try again later.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Tuesday 30 July 2013 11:48 AM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jul 30, 2013 at 11:41:23AM +0530, Kishon Vijay Abraham I wrote:
diff --git a/arch/arm/mach-omap2/board-2430sdp.c
b/arch/arm/mach-omap2/board-2430sdp.c
index 244d8a5..17bb076 100644
--- a/arch/arm/mach-omap2/board-2430sdp.c
+++ b/arch/arm/mach-omap2/board-2430sdp.c
@@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
omap_hsmmc_init(mmc);
omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA |
OMAP_PULL_UP);
- usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+ usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
>>>
>>> how about moving usb_bind_phy() calls to omap2430.c ?
>>>
>>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
>>> index f44e8b5..b6abc1a 100644
>>> --- a/drivers/usb/musb/omap2430.c
>>> +++ b/drivers/usb/musb/omap2430.c
>>> @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device
>>> *pdev)
>>>
>>> pdata->board_data = data;
>>> pdata->config = config;
>>> + } else {
>>> + /* bind the PHY */
>>> + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
>>
>> This looks like a hack IMHO to workaround the usb phy library. otherwise
>> it is
>> similar to get_phy_by_name.
>
> actually, this is a workaround to the fact that we're not creating all
> platform_devices in arch/arm/mach-omap2/ :-)
>
> If we had the musb allocation there, we could easily handle
> usb_bind_phy()
>
>>> so that's temporary. It might be better than to reintroduce the IDR in
>>> musb_core.c.
>>
>> that’s needed for generic phy framework anyway :-s
>
> right, but generic phy framework can handle everything just fine, the
> only problem is that names are changing.
right. But if the names change, PHY framework wouldn't be able to return
the
reference to the PHY.
>>>
>>> with my suggestion they can change whenever they want since we're using
>>> dev_name() of the just-created musb platform_device. Right ?
>>
>> right. But the PHY device can be created in a different place from where the
>> musb devices are created. And in the PHY framework, the PHY device should
>> have
>
> this shouldn't be a problem. As long as the phy is created, all should
> be good.
>
>> the list of controller device (names) it can support (PHY framework does not
>> maintain a separate list for binding like how we had in USB PHY library).
>> e.g.
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92817.html. In such
>
> this has nothing to do with $subject though. We talk about generic PHY
> framework once all these PHY drivers are moved there :-)
>
>> cases how do we pass the device names. Also will the MUSB core device be
>> created before twl4030-usb PHY device?
>
> and why would that be a problem ? We're telling the framework that the
> musb device will use a phy with a name of 'twl4030'. If musb calls
> usb_get_phy_dev() and doesn't find a phy, it'll return -EPROBE_DEFER and
> try again later.
I think we are talking about different problems here ;-) I'm trying to tell
using idr in MUSB core is needed for Generic PHY Framework. So in a way, the
Generic PHY Framework series depends on this patch series or else MUSB in OMAP3
platforms wont work after Generic PHY framework gets merged.
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: phy-samsung-usb: Simplify PMU register handling
On Mon, Jul 29, 2013 at 02:44:13PM -0700, Julius Werner wrote:
> This patch simplifies the way the phy-samsung-usb code finds the correct
> power management register to enable PHY clock gating. Previously, the
> code would calculate the register address from a device tree supplied
> base address and add an offset based on the PHY type.
>
> Since every PHY has its own device tree entry and needs only one
> register, we can just encode the address itself in the device tree and
> remove the diffentiation in the code. The bitmask needed to specify the
> bit within that register stays in place, allowing support for platforms
> like s3c64xx that use different bits within the same register. Due to
> this simplification, the whole complication of a Samsung-specific USB
> PHY type can be removed from the PHY driver.
>
> Signed-off-by: Julius Werner
alright, let's try to split this patch down, it's too large.
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> index ef57277..5a754d7 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -473,7 +473,7 @@
> ranges;
>
> usbphy-sys {
> - reg = <0x10040704 0x8>;
> + reg = <0x10040704 0x4>;
> };
> };
>
> @@ -505,7 +505,7 @@
> ranges;
>
> usbphy-sys {
> - reg = <0x10040704 0x8>,
> + reg = <0x10040708 0x4>,
> <0x10050230 0x4>;
> };
> };
> diff --git a/drivers/usb/phy/phy-samsung-usb.c
> b/drivers/usb/phy/phy-samsung-usb.c
> index ac025ca..32c5264 100644
> --- a/drivers/usb/phy/phy-samsung-usb.c
> +++ b/drivers/usb/phy/phy-samsung-usb.c
> @@ -27,7 +27,6 @@
> #include
> #include
> #include
> -#include
>
> #include "phy-samsung-usb.h"
>
> @@ -42,9 +41,9 @@ int samsung_usbphy_parse_dt(struct samsung_usbphy *sphy)
> return -ENODEV;
> }
>
> - sphy->pmuregs = of_iomap(usbphy_sys, 0);
> + sphy->pmureg = of_iomap(usbphy_sys, 0);
this rename can go in a separate patch.
> @@ -75,35 +74,25 @@ EXPORT_SYMBOL_GPL(samsung_usbphy_parse_dt);
> */
> void samsung_usbphy_set_isolation_4210(struct samsung_usbphy *sphy, bool on)
> {
> - void __iomem *reg = NULL;
> u32 reg_val;
> - u32 en_mask = 0;
>
> - if (!sphy->pmuregs) {
> + if (!sphy->pmureg) {
> dev_warn(sphy->dev, "Can't set pmu isolation\n");
> return;
> }
>
> - if (sphy->phy_type == USB_PHY_TYPE_DEVICE) {
> - reg = sphy->pmuregs + sphy->drv_data->devphy_reg_offset;
> - en_mask = sphy->drv_data->devphy_en_mask;
> - } else if (sphy->phy_type == USB_PHY_TYPE_HOST) {
> - reg = sphy->pmuregs + sphy->drv_data->hostphy_reg_offset;
> - en_mask = sphy->drv_data->hostphy_en_mask;
> - }
>
> - reg_val = readl(reg);
> + reg_val = readl(sphy->pmureg);
>
> if (on)
> - reg_val &= ~en_mask;
> + reg_val &= ~sphy->drv_data->phy_en_mask;
> else
> - reg_val |= en_mask;
> + reg_val |= sphy->drv_data->phy_en_mask;
removing the if case up there and updating this if here could be a patch
of its own.
> @@ -111,7 +100,7 @@ EXPORT_SYMBOL_GPL(samsung_usbphy_set_isolation_4210);
> /*
> * Configure the mode of working of usb-phy here: HOST/DEVICE.
> */
> -void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy)
> +void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy, bool device_mode)
passing this extra parameter could be a patch of its own.
> @@ -122,31 +111,15 @@ void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy)
>
> reg = readl(sphy->sysreg);
>
> - if (sphy->phy_type == USB_PHY_TYPE_DEVICE)
> + if (device_mode)
> reg &= ~EXYNOS_USB20PHY_CFG_HOST_LINK;
> - else if (sphy->phy_type == USB_PHY_TYPE_HOST)
> + else
> reg |= EXYNOS_USB20PHY_CFG_HOST_LINK;
>
> writel(reg, sphy->sysreg);
> }
> EXPORT_SYMBOL_GPL(samsung_usbphy_cfg_sel);
>
> -/*
> - * PHYs are different for USB Device and USB Host.
> - * This make sure that correct PHY type is selected before
> - * any operation on PHY.
> - */
> -int samsung_usbphy_set_type(struct usb_phy *phy,
> - enum samsung_usb_phy_type phy_type)
> -{
> - struct samsung_usbphy *sphy = phy_to_sphy(phy);
> -
> - sphy->phy_type = phy_type;
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(samsung_usbphy_set_type);
removing this function should be a patch of its own.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi,
On Tue, Jul 30, 2013 at 11:55:04AM +0530, Kishon Vijay Abraham I wrote:
> > On Tue, Jul 30, 2013 at 11:41:23AM +0530, Kishon Vijay Abraham I wrote:
> diff --git a/arch/arm/mach-omap2/board-2430sdp.c
> b/arch/arm/mach-omap2/board-2430sdp.c
> index 244d8a5..17bb076 100644
> --- a/arch/arm/mach-omap2/board-2430sdp.c
> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> @@ -233,7 +233,7 @@ static void __init omap_2430sdp_init(void)
> omap_hsmmc_init(mmc);
>
> omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA |
> OMAP_PULL_UP);
> -usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> +usb_bind_phy("musb-hdrc.0", 0, "twl4030_usb");
> >>>
> >>> how about moving usb_bind_phy() calls to omap2430.c ?
> >>>
> >>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> >>> index f44e8b5..b6abc1a 100644
> >>> --- a/drivers/usb/musb/omap2430.c
> >>> +++ b/drivers/usb/musb/omap2430.c
> >>> @@ -544,6 +544,9 @@ static int omap2430_probe(struct platform_device
> >>> *pdev)
> >>>
> >>> pdata->board_data = data;
> >>> pdata->config = config;
> >>> + } else {
> >>> + /* bind the PHY */
> >>> + usb_bind_phy(dev_name(&musb->dev), 0, "twl4030_usb");
> >>
> >> This looks like a hack IMHO to workaround the usb phy library.
> >> otherwise it is
> >> similar to get_phy_by_name.
> >
> > actually, this is a workaround to the fact that we're not creating all
> > platform_devices in arch/arm/mach-omap2/ :-)
> >
> > If we had the musb allocation there, we could easily handle
> > usb_bind_phy()
> >
> >>> so that's temporary. It might be better than to reintroduce the IDR in
> >>> musb_core.c.
> >>
> >> that’s needed for generic phy framework anyway :-s
> >
> > right, but generic phy framework can handle everything just fine, the
> > only problem is that names are changing.
>
> right. But if the names change, PHY framework wouldn't be able to return
> the
> reference to the PHY.
> >>>
> >>> with my suggestion they can change whenever they want since we're using
> >>> dev_name() of the just-created musb platform_device. Right ?
> >>
> >> right. But the PHY device can be created in a different place from where
> >> the
> >> musb devices are created. And in the PHY framework, the PHY device should
> >> have
> >
> > this shouldn't be a problem. As long as the phy is created, all should
> > be good.
> >
> >> the list of controller device (names) it can support (PHY framework does
> >> not
> >> maintain a separate list for binding like how we had in USB PHY library).
> >> e.g.
> >> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92817.html. In
> >> such
> >
> > this has nothing to do with $subject though. We talk about generic PHY
> > framework once all these PHY drivers are moved there :-)
> >
> >> cases how do we pass the device names. Also will the MUSB core device be
> >> created before twl4030-usb PHY device?
> >
> > and why would that be a problem ? We're telling the framework that the
> > musb device will use a phy with a name of 'twl4030'. If musb calls
> > usb_get_phy_dev() and doesn't find a phy, it'll return -EPROBE_DEFER and
> > try again later.
>
> I think we are talking about different problems here ;-) I'm trying to tell
> using idr in MUSB core is needed for Generic PHY Framework. So in a way, the
> Generic PHY Framework series depends on this patch series or else MUSB in
> OMAP3
> platforms wont work after Generic PHY framework gets merged.
then you just found a limitation in your framework, right ? :-) I mean,
imagine if now we have to add an IDR to every single user of your
framework because they could end up in systems with multiple instances
of the same IP ?
Now consider that you aim to have your framework be used by Network,
USB, SATA, Graphics, etc... Have you really only considered DT
platforms ? DT is quite easy since you can require folks to pass the
proper phandle, but drivers will want to use PLATFORM_DEVID_AUTO and
your framework needs to cope with that.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: gadget: f_rndis: fix error return code in rndis_bind()
On Tue, Jul 30, 2013 at 07:54:02AM +0800, Wei Yongjun wrote: > From: Wei Yongjun > > Fix to return -EINVAL in the vendor param set error handling > case instead of 0, as done elsewhere in this function. > > Signed-off-by: Wei Yongjun this bug is present in v3.5+, do we need to send this to stable ? -- balbi signature.asc Description: Digital signature
