[PATCH 13/15] phy: Add driver for Exynos DP PHY
From: Jingoo Han jg1@samsung.com
Add a PHY provider driver for the Samsung Exynos SoC Display Port PHY.
Signed-off-by: Jingoo Han jg1@samsung.com
Reviewed-by: Tomasz Figa t.f...@samsung.com
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
.../devicetree/bindings/phy/samsung-phy.txt|8 ++
drivers/phy/Kconfig|6 ++
drivers/phy/Makefile |1 +
drivers/phy/phy-exynos-dp-video.c | 111
4 files changed, 126 insertions(+)
create mode 100644 drivers/phy/phy-exynos-dp-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 5ff208c..c0fccaa 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as
follows:
1 - MIPI DSIM 0,
2 - MIPI CSIS 1,
3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series Display Port PHY
+-
+
+Required properties:
+- compatible : should be samsung,exynos5250-dp-video-phy;
+- reg : offset and length of the Display Port PHY register set;
+- #phy-cells : from the generic PHY bindings, must be 0;
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 6f446d0..ed0b1b8 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -19,4 +19,10 @@ config PHY_EXYNOS_MIPI_VIDEO
help
Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+ tristate EXYNOS SoC series Display Port PHY driver
+ depends on OF
+ help
+ Support for Display Port PHY found on Samsung EXYNOS SoCs.
endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 71d8841..0fd1340 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
diff --git a/drivers/phy/phy-exynos-dp-video.c
b/drivers/phy/phy-exynos-dp-video.c
new file mode 100644
index 000..3c8e247
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,111 @@
+/*
+ * Samsung EXYNOS SoC series Display Port PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han jg1@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/io.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/phy/phy.h
+#include linux/platform_device.h
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE (1 0)
+
+struct exynos_dp_video_phy {
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+ u32 reg;
+
+ reg = readl(state-regs);
+ if (on)
+ reg |= EXYNOS_DPTX_PHY_ENABLE;
+ else
+ reg = ~EXYNOS_DPTX_PHY_ENABLE;
+ writel(reg, state-regs);
+
+ return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+ .power_on = exynos_dp_video_phy_power_on,
+ .power_off = exynos_dp_video_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_dp_video_phy *state;
+ struct device *dev = pdev-dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ struct phy *phy;
+
+ state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ state-regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(state-regs))
+ return PTR_ERR(state-regs);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(dev, 0, exynos_dp_video_phy_ops, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, failed to create Display Port PHY\n);
+ return PTR_ERR(phy);
+ }
+ phy_set_drvdata(phy,
[PATCH 04/15] ARM: OMAP: USB: Add phy binding information
In order for controllers to get PHY in case of non dt boot, the phy
binding information (phy device name) should be added in the platform
data of the controller.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
arch/arm/mach-omap2/usb-musb.c |3 +++
include/linux/usb/musb.h |3 +++
2 files changed, 6 insertions(+)
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 8c4de27..6aa7cbf 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -85,6 +85,9 @@ void __init usb_musb_init(struct omap_musb_board_data
*musb_board_data)
musb_plat.mode = board_data-mode;
musb_plat.extvbus = board_data-extvbus;
+ if (cpu_is_omap34xx())
+ musb_plat.phy_label = twl4030;
+
if (soc_is_am35xx()) {
oh_name = am35x_otg_hs;
name = musb-am35x;
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 053c268..596f8c8 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -104,6 +104,9 @@ struct musb_hdrc_platform_data {
/* for clk_get() */
const char *clock;
+ /* phy label */
+ const char *phy_label;
+
/* (HOST or OTG) switch VBUS on/off */
int (*set_vbus)(struct device *dev, int is_on);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/15] usb: musb: omap2430: use the new generic PHY framework
Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
and usb_phy_set_suspend is replaced with power_on and
power_off to align with the new PHY framework.
musb-xceiv can't be removed as of now because musb core uses xceiv.state and
xceiv.otg. Once there is a separate state machine to handle otg, these can be
moved out of xceiv and then we can start using the generic PHY framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
drivers/usb/musb/Kconfig |1 +
drivers/usb/musb/musb_core.c |1 +
drivers/usb/musb/musb_core.h |3 +++
drivers/usb/musb/omap2430.c | 26 --
4 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 797e3fd..01381ac 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -76,6 +76,7 @@ config USB_MUSB_TUSB6010
config USB_MUSB_OMAP2PLUS
tristate OMAP2430 and onwards
depends on ARCH_OMAP2PLUS
+ depends on GENERIC_PHY
config USB_MUSB_AM35X
tristate AM35x
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 29a24ce..cca12c0 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1814,6 +1814,7 @@ musb_init_controller(struct device *dev, int nIrq, void
__iomem *ctrl)
musb-min_power = plat-min_power;
musb-ops = plat-platform_ops;
musb-port_mode = plat-mode;
+ musb-phy_label = plat-phy_label;
/* The musb_platform_init() call:
* - adjusts musb-mregs
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 7d341c3..8f017ab 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -46,6 +46,7 @@
#include linux/usb.h
#include linux/usb/otg.h
#include linux/usb/musb.h
+#include linux/phy/phy.h
struct musb;
struct musb_hw_ep;
@@ -346,6 +347,7 @@ struct musb {
u16 int_tx;
struct usb_phy *xceiv;
+ struct phy *phy;
int nIrq;
unsignedirq_wake:1;
@@ -424,6 +426,7 @@ struct musb {
unsigneddouble_buffer_not_ok:1;
struct musb_hdrc_config *config;
+ const char *phy_label;
#ifdef MUSB_CONFIG_PROC_FS
struct proc_dir_entry *proc_entry;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 6708a3b..87dac0f 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -348,11 +348,21 @@ static int omap2430_musb_init(struct musb *musb)
* up through ULPI. TWL4030-family PMICs include one,
* which needs a driver, drivers aren't always needed.
*/
- if (dev-parent-of_node)
+ if (dev-parent-of_node) {
+ musb-phy = devm_phy_get(dev-parent, usb2-phy);
+
+ /* We can't totally remove musb-xceiv as of now because
+* musb core uses xceiv.state and xceiv.otg. Once we have
+* a separate state machine to handle otg, these can be moved
+* out of xceiv and then we can start using the generic PHY
+* framework
+*/
musb-xceiv = devm_usb_get_phy_by_phandle(dev-parent,
usb-phy, 0);
- else
+ } else {
musb-xceiv = devm_usb_get_phy_dev(dev, 0);
+ musb-phy = devm_phy_get(dev, musb-phy_label);
+ }
if (IS_ERR(musb-xceiv)) {
status = PTR_ERR(musb-xceiv);
@@ -364,6 +374,10 @@ static int omap2430_musb_init(struct musb *musb)
return -EPROBE_DEFER;
}
+ if (IS_ERR(musb-phy)) {
+ pr_err(HS USB OTG: no PHY configured\n);
+ return PTR_ERR(musb-phy);
+ }
musb-isr = omap2430_musb_interrupt;
status = pm_runtime_get_sync(dev);
@@ -397,7 +411,7 @@ static int omap2430_musb_init(struct musb *musb)
if (glue-status != OMAP_MUSB_UNKNOWN)
omap_musb_set_mailbox(glue);
- usb_phy_init(musb-xceiv);
+ phy_init(musb-phy);
pm_runtime_put_noidle(musb-controller);
return 0;
@@ -460,6 +474,7 @@ static int omap2430_musb_exit(struct musb *musb)
del_timer_sync(musb_idle_timer);
omap2430_low_level_exit(musb);
+ phy_exit(musb-phy);
return 0;
}
@@ -633,7 +648,7 @@ static int omap2430_runtime_suspend(struct device *dev)
OTG_INTERFSEL);
omap2430_low_level_exit(musb);
- usb_phy_set_suspend(musb-xceiv, 1);
+ phy_power_off(musb-phy);
}
return 0;
@@ -648,8 +663,7 @@ static int omap2430_runtime_resume(struct device *dev)
omap2430_low_level_init(musb);
musb_writel(musb-mregs, OTG_INTERFSEL,
[PATCH 07/15] usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2
Now that omap-usb2 is adapted to the new generic PHY framework,
*set_suspend* ops can be removed from omap-usb2 driver.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Acked-by: Felipe Balbi ba...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
---
drivers/usb/phy/phy-omap-usb2.c | 25 -
1 file changed, 25 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 751b30f..3f2b125 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -97,29 +97,6 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
return 0;
}
-static int omap_usb2_suspend(struct usb_phy *x, int suspend)
-{
- u32 ret;
- struct omap_usb *phy = phy_to_omapusb(x);
-
- if (suspend !phy-is_suspended) {
- omap_control_usb_phy_power(phy-control_dev, 0);
- pm_runtime_put_sync(phy-dev);
- phy-is_suspended = 1;
- } else if (!suspend phy-is_suspended) {
- ret = pm_runtime_get_sync(phy-dev);
- if (ret 0) {
- dev_err(phy-dev, get_sync failed with err %d\n,
- ret);
- return ret;
- }
- omap_control_usb_phy_power(phy-control_dev, 1);
- phy-is_suspended = 0;
- }
-
- return 0;
-}
-
static int omap_usb_power_off(struct phy *x)
{
struct omap_usb *phy = phy_get_drvdata(x);
@@ -167,7 +144,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-phy.dev= phy-dev;
phy-phy.label = omap-usb2;
- phy-phy.set_suspend= omap_usb2_suspend;
phy-phy.otg= otg;
phy-phy.type = USB_PHY_TYPE_USB2;
@@ -182,7 +158,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
return -ENODEV;
}
- phy-is_suspended = 1;
omap_control_usb_phy_power(phy-control_dev, 0);
otg-set_host = omap_usb_set_host;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/15] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Sylwester Nawrocki s.nawro...@samsung.com
Add a PHY provider driver for the Samsung S5P/Exynos SoC MIPI CSI-2
receiver and MIPI DSI transmitter DPHYs.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
.../devicetree/bindings/phy/samsung-phy.txt| 14 ++
drivers/phy/Kconfig|9 ++
drivers/phy/Makefile |3 +-
drivers/phy/phy-exynos-mipi-video.c| 169
4 files changed, 194 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/phy/samsung-phy.txt
create mode 100644 drivers/phy/phy-exynos-mipi-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
b/Documentation/devicetree/bindings/phy/samsung-phy.txt
new file mode 100644
index 000..5ff208c
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -0,0 +1,14 @@
+Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY
+-
+
+Required properties:
+- compatible : should be samsung,s5pv210-mipi-video-phy;
+- reg : offset and length of the MIPI DPHY register set;
+- #phy-cells : from the generic phy bindings, must be 1;
+
+For samsung,s5pv210-mipi-video-phy compatible PHYs the second cell in
+the PHY specifier identifies the PHY and its meaning is as follows:
+ 0 - MIPI CSIS 0,
+ 1 - MIPI DSIM 0,
+ 2 - MIPI CSIS 1,
+ 3 - MIPI DSIM 1.
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5f85909..6f446d0 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -11,3 +11,12 @@ menuconfig GENERIC_PHY
devices present in the kernel. This layer will have the generic
API by which phy drivers can create PHY using the phy framework and
phy users can obtain reference to the PHY.
+
+if GENERIC_PHY
+
+config PHY_EXYNOS_MIPI_VIDEO
+ tristate S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver
+ help
+ Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
+ S5P and EXYNOS SoCs.
+endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9e9560f..71d8841 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -2,4 +2,5 @@
# Makefile for the phy drivers.
#
-obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-exynos-mipi-video.c
b/drivers/phy/phy-exynos-mipi-video.c
new file mode 100644
index 000..7e7fcd7
--- /dev/null
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -0,0 +1,169 @@
+/*
+ * Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki s.nawro...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/io.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/phy/phy.h
+#include linux/platform_device.h
+#include linux/spinlock.h
+
+/* MIPI_PHYn_CONTROL register offset: n = 0..1 */
+#define EXYNOS_MIPI_PHY_CONTROL(n) ((n) * 4)
+#define EXYNOS_MIPI_PHY_ENABLE (1 0)
+#define EXYNOS_MIPI_PHY_SRESETN(1 1)
+#define EXYNOS_MIPI_PHY_MRESETN(1 2)
+#define EXYNOS_MIPI_PHY_RESET_MASK (3 1)
+
+enum exynos_mipi_phy_id {
+ EXYNOS_MIPI_PHY_ID_CSIS0,
+ EXYNOS_MIPI_PHY_ID_DSIM0,
+ EXYNOS_MIPI_PHY_ID_CSIS1,
+ EXYNOS_MIPI_PHY_ID_DSIM1,
+ EXYNOS_MIPI_PHYS_NUM
+};
+
+#define IS_EXYNOS_MIPI_DSIM_PHY_ID(id) \
+ ((id) == EXYNOS_MIPI_PHY_ID_DSIM0 || (id) == EXYNOS_MIPI_PHY_ID_DSIM0)
+
+struct exynos_mipi_video_phy {
+ spinlock_t slock;
+ struct phy *phys[EXYNOS_MIPI_PHYS_NUM];
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_mipi_video_phy *state,
+ enum exynos_mipi_phy_id id, unsigned int on)
+{
+ void __iomem *addr;
+ u32 reg, reset;
+
+ addr = state-regs + EXYNOS_MIPI_PHY_CONTROL(id / 2);
+
+ if (IS_EXYNOS_MIPI_DSIM_PHY_ID(id))
+ reset = EXYNOS_MIPI_PHY_MRESETN;
+ else
+ reset = EXYNOS_MIPI_PHY_SRESETN;
+
+ spin_lock(state-slock);
+ reg = readl(addr);
+ if (on)
+ reg |= reset;
+ else
+ reg = ~reset;
+ writel(reg, addr);
+
+ /* Clear ENABLE bit only if MRESETN, SRESETN bits are not set. */
+ if (on)
+ reg |= EXYNOS_MIPI_PHY_ENABLE;
+ else if (!(reg EXYNOS_MIPI_PHY_RESET_MASK))
+ reg = ~EXYNOS_MIPI_PHY_ENABLE;
+
+ writel(reg, addr);
+
[PATCH 08/15] usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops
Now that twl4030-usb is adapted to the new generic PHY framework,
*set_suspend* and *phy_init* ops can be removed from twl4030-usb driver.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Acked-by: Felipe Balbi ba...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
---
drivers/usb/phy/phy-twl4030-usb.c | 57 +
1 file changed, 13 insertions(+), 44 deletions(-)
diff --git a/drivers/usb/phy/phy-twl4030-usb.c
b/drivers/usb/phy/phy-twl4030-usb.c
index 9051756..44f8b1b 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -422,25 +422,20 @@ static void twl4030_phy_power(struct twl4030_usb *twl,
int on)
}
}
-static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
+static int twl4030_phy_power_off(struct phy *phy)
{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
if (twl-asleep)
- return;
+ return 0;
twl4030_phy_power(twl, 0);
twl-asleep = 1;
dev_dbg(twl-dev, %s\n, __func__);
-}
-
-static int twl4030_phy_power_off(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- twl4030_phy_suspend(twl, 0);
return 0;
}
-static void __twl4030_phy_resume(struct twl4030_usb *twl)
+static void __twl4030_phy_power_on(struct twl4030_usb *twl)
{
twl4030_phy_power(twl, 1);
twl4030_i2c_access(twl, 1);
@@ -449,11 +444,13 @@ static void __twl4030_phy_resume(struct twl4030_usb *twl)
twl4030_i2c_access(twl, 0);
}
-static void twl4030_phy_resume(struct twl4030_usb *twl)
+static int twl4030_phy_power_on(struct phy *phy)
{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
if (!twl-asleep)
- return;
- __twl4030_phy_resume(twl);
+ return 0;
+ __twl4030_phy_power_on(twl);
twl-asleep = 0;
dev_dbg(twl-dev, %s\n, __func__);
@@ -466,13 +463,6 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
cancel_delayed_work(twl-id_workaround_work);
schedule_delayed_work(twl-id_workaround_work, HZ);
}
-}
-
-static int twl4030_phy_power_on(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- twl4030_phy_resume(twl);
return 0;
}
@@ -604,9 +594,9 @@ static void twl4030_id_workaround_work(struct work_struct
*work)
}
}
-static int twl4030_usb_phy_init(struct usb_phy *phy)
+static int twl4030_phy_init(struct phy *phy)
{
- struct twl4030_usb *twl = phy_to_twl(phy);
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
enum omap_musb_vbus_id_status status;
/*
@@ -621,32 +611,13 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
if (status == OMAP_MUSB_ID_GROUND || status == OMAP_MUSB_VBUS_VALID) {
omap_musb_mailbox(twl-linkstat);
- twl4030_phy_resume(twl);
+ twl4030_phy_power_on(phy);
}
sysfs_notify(twl-dev-kobj, NULL, vbus);
return 0;
}
-static int twl4030_phy_init(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- return twl4030_usb_phy_init(twl-phy);
-}
-
-static int twl4030_set_suspend(struct usb_phy *x, int suspend)
-{
- struct twl4030_usb *twl = phy_to_twl(x);
-
- if (suspend)
- twl4030_phy_suspend(twl, 1);
- else
- twl4030_phy_resume(twl);
-
- return 0;
-}
-
static int twl4030_set_peripheral(struct usb_otg *otg,
struct usb_gadget *gadget)
{
@@ -717,8 +688,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
twl-phy.label = twl4030;
twl-phy.otg= otg;
twl-phy.type = USB_PHY_TYPE_USB2;
- twl-phy.set_suspend= twl4030_set_suspend;
- twl-phy.init = twl4030_usb_phy_init;
otg-phy= twl-phy;
otg-set_host = twl4030_set_host;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/15] video: exynos_mipi_dsim: Use the generic PHY driver
From: Sylwester Nawrocki s.nawro...@samsung.com
Use the generic PHY API instead of the platform callback to control
the MIPI DSIM DPHY. The 'phy_label' field is added to the platform
data structure to allow PHY lookup on non-dt platforms.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
Acked-by: Donghwa Lee dh09@samsung.com
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
drivers/video/exynos/exynos_mipi_dsi.c | 19 ++-
include/video/exynos_mipi_dsim.h |6 --
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c
b/drivers/video/exynos/exynos_mipi_dsi.c
index 32e5406..248e444 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -30,6 +30,7 @@
#include linux/interrupt.h
#include linux/kthread.h
#include linux/notifier.h
+#include linux/phy/phy.h
#include linux/regulator/consumer.h
#include linux/pm_runtime.h
#include linux/err.h
@@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct
mipi_dsim_device *dsim, int power)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, true);
+ phy_power_on(dsim-phy);
clk_enable(dsim-clock);
@@ -373,6 +373,10 @@ static int exynos_mipi_dsi_probe(struct platform_device
*pdev)
return ret;
}
+ dsim-phy = devm_phy_get(pdev-dev, dsim_pd-phy_label);
+ if (IS_ERR(dsim-phy))
+ return PTR_ERR(dsim-phy);
+
dsim-clock = devm_clk_get(pdev-dev, dsim0);
if (IS_ERR(dsim-clock)) {
dev_err(pdev-dev, failed to get dsim clock source\n);
@@ -439,8 +443,7 @@ static int exynos_mipi_dsi_probe(struct platform_device
*pdev)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, true);
+ phy_power_on(dsim-phy);
exynos_mipi_update_cfg(dsim);
@@ -504,9 +507,8 @@ static int exynos_mipi_dsi_suspend(struct device *dev)
if (client_drv client_drv-suspend)
client_drv-suspend(client_dev);
- /* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, false);
+ /* disable MIPI-DSI PHY. */
+ phy_power_off(dsim-phy);
clk_disable(dsim-clock);
@@ -536,8 +538,7 @@ static int exynos_mipi_dsi_resume(struct device *dev)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, true);
+ phy_power_on(dsim-phy);
clk_enable(dsim-clock);
diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h
index 89dc88a..0e7e43b 100644
--- a/include/video/exynos_mipi_dsim.h
+++ b/include/video/exynos_mipi_dsim.h
@@ -216,6 +216,7 @@ struct mipi_dsim_config {
* automatically.
* @e_clk_src: select byte clock source.
* @pd: pointer to MIPI-DSI driver platform data.
+ * @phy: pointer to the generic PHY
*/
struct mipi_dsim_device {
struct device *dev;
@@ -236,6 +237,7 @@ struct mipi_dsim_device {
boolsuspended;
struct mipi_dsim_platform_data *pd;
+ struct phy *phy;
};
/*
@@ -248,7 +250,7 @@ struct mipi_dsim_device {
* @enabled: indicate whether mipi controller got enabled or not.
* @lcd_panel_info: pointer for lcd panel specific structure.
* this structure specifies width, height, timing and polarity and so on.
- * @phy_enable: pointer to a callback controlling D-PHY enable/reset
+ * @phy_label: the generic PHY label
*/
struct mipi_dsim_platform_data {
charlcd_panel_name[PANEL_NAME_SIZE];
@@ -257,7 +259,7 @@ struct mipi_dsim_platform_data {
unsigned intenabled;
void*lcd_panel_info;
- int (*phy_enable)(struct platform_device *pdev, bool on);
+ const char *phy_label;
};
/*
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework
Used the generic PHY framework API to create the PHY. Now the power off and
power on are done in omap_usb_power_off and omap_usb_power_on respectively.
However using the old USB PHY library cannot be completely removed
because OTG is intertwined with PHY and moving to the new framework
will break OTG. Once we have a separate OTG state machine, we
can get rid of the USB PHY library.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
drivers/usb/phy/Kconfig |1 +
drivers/usb/phy/phy-omap-usb2.c | 45 +++
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 3622fff..cc55993 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
config OMAP_USB2
tristate OMAP USB2 PHY Driver
depends on ARCH_OMAP2PLUS
+ depends on GENERIC_PHY
select OMAP_CONTROL_USB
help
Enable this to support the transceiver that is part of SOC. This
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..751b30f 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
#include linux/pm_runtime.h
#include linux/delay.h
#include linux/usb/omap_control_usb.h
+#include linux/phy/phy.h
/**
* omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int
suspend)
return 0;
}
+static int omap_usb_power_off(struct phy *x)
+{
+ struct omap_usb *phy = phy_get_drvdata(x);
+
+ omap_control_usb_phy_power(phy-control_dev, 0);
+
+ return 0;
+}
+
+static int omap_usb_power_on(struct phy *x)
+{
+ struct omap_usb *phy = phy_get_drvdata(x);
+
+ omap_control_usb_phy_power(phy-control_dev, 1);
+
+ return 0;
+}
+
+static struct phy_ops ops = {
+ .power_on = omap_usb_power_on,
+ .power_off = omap_usb_power_off,
+ .owner = THIS_MODULE,
+};
+
static int omap_usb2_probe(struct platform_device *pdev)
{
struct omap_usb *phy;
+ struct phy *generic_phy;
struct usb_otg *otg;
+ struct phy_provider *phy_provider;
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-phy.otg= otg;
phy-phy.type = USB_PHY_TYPE_USB2;
+ phy_provider = devm_of_phy_provider_register(phy-dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
phy-control_dev = omap_get_control_dev();
if (IS_ERR(phy-control_dev)) {
dev_dbg(pdev-dev, Failed to get control device\n);
@@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
otg-start_srp = omap_usb_start_srp;
otg-phy= phy-phy;
+ platform_set_drvdata(pdev, phy);
+ pm_runtime_enable(phy-dev);
+
+ generic_phy = devm_phy_create(phy-dev, 0, ops, omap-usb2);
+ if (IS_ERR(generic_phy))
+ return PTR_ERR(generic_phy);
+
+ phy_set_drvdata(generic_phy, phy);
+
phy-wkupclk = devm_clk_get(phy-dev, usb_phy_cm_clk32k);
if (IS_ERR(phy-wkupclk)) {
dev_err(pdev-dev, unable to get usb_phy_cm_clk32k\n);
@@ -174,10 +215,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
usb_add_phy_dev(phy-phy);
- platform_set_drvdata(pdev, phy);
-
- pm_runtime_enable(phy-dev);
-
return 0;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/15] PHY framework
Added a generic PHY framework that 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. This framework will be of use only to devices that uses external PHY (PHY functionality is not embedded within the controller). 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 to increase code maintainability. Comments to make PHY as bus wasn't done because PHY devices can be part of other bus and making a same device attached to multiple bus leads to bad design. If the PHY driver has to send notification on connect/disconnect, the PHY driver should make use of the extcon framework. Using this susbsystem to use extcon framwork will have to be analysed. Exynos MIPI CSIS/DSIM PHY and Displayport PHY have started using this framework. Have included those patches also in this series. twl4030-usb and omap-usb2 have also been adapted to this framework. These patches are also available @ git://gitorious.org/linuxphy/linuxphy.git tags/phy-for-v3.12 Jingoo Han (3): phy: Add driver for Exynos DP PHY video: exynos_dp: remove non-DT support for Exynos Display Port video: exynos_dp: Use the generic PHY driver Kishon Vijay Abraham I (8): drivers: phy: add generic PHY framework usb: phy: omap-usb2: use the new generic PHY framework usb: phy: twl4030: use the new generic PHY framework ARM: OMAP: USB: Add phy binding information ARM: dts: omap: update usb_otg_hs data usb: musb: omap2430: use the new generic PHY framework usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2 usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops Sylwester Nawrocki (4): phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs video: exynos_mipi_dsim: Use the generic PHY driver exynos4-is: Use the generic MIPI CSIS PHY driver ARM: Samsung: Remove the MIPI PHY setup code .../devicetree/bindings/phy/phy-bindings.txt | 66 +++ .../devicetree/bindings/phy/samsung-phy.txt| 22 + Documentation/devicetree/bindings/usb/omap-usb.txt |5 + Documentation/devicetree/bindings/usb/usb-phy.txt |6 + .../devicetree/bindings/video/exynos_dp.txt| 18 +- Documentation/phy.txt | 129 + MAINTAINERS|7 + arch/arm/boot/dts/omap3-beagle-xm.dts |2 + arch/arm/boot/dts/omap3-evm.dts|2 + arch/arm/boot/dts/omap3-overo.dtsi |2 + arch/arm/boot/dts/omap4.dtsi |3 + arch/arm/boot/dts/twl4030.dtsi |1 + arch/arm/mach-exynos/include/mach/regs-pmu.h |5 - arch/arm/mach-omap2/usb-musb.c |3 + arch/arm/mach-s5pv210/include/mach/regs-clock.h|4 - arch/arm/plat-samsung/Kconfig |5 - arch/arm/plat-samsung/Makefile |1 - arch/arm/plat-samsung/setup-mipiphy.c | 60 --- drivers/Kconfig|2 + drivers/Makefile |2 + drivers/media/platform/exynos4-is/mipi-csis.c | 16 +- drivers/phy/Kconfig| 28 + drivers/phy/Makefile |7 + drivers/phy/phy-core.c | 544 drivers/phy/phy-exynos-dp-video.c | 111 drivers/phy/phy-exynos-mipi-video.c| 169 ++ drivers/usb/musb/Kconfig |1 + drivers/usb/musb/musb_core.c |1 + drivers/usb/musb/musb_core.h |3 + drivers/usb/musb/omap2430.c| 26 +- drivers/usb/phy/Kconfig|1 + drivers/usb/phy/phy-omap-usb2.c| 60 ++- drivers/usb/phy/phy-twl4030-usb.c | 63 ++- drivers/video/exynos/Kconfig |2 +- drivers/video/exynos/exynos_dp_core.c | 132 ++--- drivers/video/exynos/exynos_dp_core.h | 110 drivers/video/exynos/exynos_dp_reg.c |2 - drivers/video/exynos/exynos_mipi_dsi.c | 19 +- include/linux/phy/phy.h| 344 + include/linux/platform_data/mipi-csis.h| 11 +- include/linux/usb/musb.h |3 + include/video/exynos_dp.h | 131 - include/video/exynos_mipi_dsim.h |6 +- 43 files changed, 1746 insertions(+), 389 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt create mode 100644 Documentation/devicetree/bindings/phy/samsung-phy.txt create mode 100644 Documentation/phy.txt delete mode 100644
[PATCH 01/15] drivers: phy: add generic PHY framework
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
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Acked-by: Felipe Balbi ba...@ti.com
Tested-by: Sylwester Nawrocki s.nawro...@samsung.com
---
.../devicetree/bindings/phy/phy-bindings.txt | 66 +++
Documentation/phy.txt | 129 +
MAINTAINERS|7 +
drivers/Kconfig|2 +
drivers/Makefile |2 +
drivers/phy/Kconfig| 13 +
drivers/phy/Makefile |5 +
drivers/phy/phy-core.c | 544
include/linux/phy/phy.h| 344 +
9 files changed, 1112 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
+
+PHY device node
+===
+
+Required Properties:
+#phy-cells:Number of cells in a PHY specifier; The meaning of all those
+ cells is defined by the binding for the phy node. The PHY
+ provider can use the values in cells to find the appropriate
+ PHY.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg = ...;
+.
+.
+#phy-cells = 1;
+.
+.
+};
+
+That node describes an IP block (PHY provider) that implements 2 different
PHYs.
+In order to differentiate between these 2 PHYs, an additonal specifier should
be
+given while trying to get a reference to it.
+
+PHY user node
+=
+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+phy-names : the names of the PHY corresponding to the PHYs present in the
+ *phys* phandle
+
+Example 1:
+usb1: usb_otg_ss@xxx {
+compatible = xxx;
+reg = xxx;
+.
+.
+phys = usb2_phy, usb3_phy;
+phy-names = usb2phy, usb3phy;
+.
+.
+};
+
+This node represents a controller that uses two PHYs, one for usb2 and one for
+usb3.
+
+Example 2:
+usb2: usb_otg_ss@xxx {
+compatible = xxx;
+reg = xxx;
+.
+.
+phys = phys 1;
+phy-names = usbphy;
+.
+.
+};
+
+This node represents a controller that uses one of the PHYs of the PHY provider
+device defined previously. Note that the phy handle has an additional specifier
+1 to differentiate between the two PHYs.
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 000..05f8fda
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,129 @@
+ PHY SUBSYSTEM
+ Kishon Vijay Abraham I kis...@ti.com
+
+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
[PATCH 03/15] usb: phy: twl4030: use the new generic PHY framework
Used the generic PHY framework API to create the PHY. For powering on
and powering off the PHY, power_on and power_off ops are used. Once the
MUSB OMAP glue is adapted to the new framework, the suspend and resume
ops of usb phy library will be removed.
However using the old usb phy library cannot be completely removed
because otg is intertwined with phy and moving to the new
framework completely will break otg. Once we have a separate otg state machine,
we can get rid of the usb phy library.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Acked-by: Felipe Balbi ba...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
---
drivers/usb/phy/phy-twl4030-usb.c | 50 -
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/phy/phy-twl4030-usb.c
b/drivers/usb/phy/phy-twl4030-usb.c
index 8f78d2d..9051756 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -33,6 +33,7 @@
#include linux/io.h
#include linux/delay.h
#include linux/usb/otg.h
+#include linux/phy/phy.h
#include linux/usb/musb-omap.h
#include linux/usb/ulpi.h
#include linux/i2c/twl.h
@@ -431,6 +432,14 @@ static void twl4030_phy_suspend(struct twl4030_usb *twl,
int controller_off)
dev_dbg(twl-dev, %s\n, __func__);
}
+static int twl4030_phy_power_off(struct phy *phy)
+{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+ twl4030_phy_suspend(twl, 0);
+ return 0;
+}
+
static void __twl4030_phy_resume(struct twl4030_usb *twl)
{
twl4030_phy_power(twl, 1);
@@ -459,6 +468,14 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
}
}
+static int twl4030_phy_power_on(struct phy *phy)
+{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+ twl4030_phy_resume(twl);
+ return 0;
+}
+
static int twl4030_usb_ldo_init(struct twl4030_usb *twl)
{
/* Enable writing to power configuration registers */
@@ -602,13 +619,22 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
status = twl4030_usb_linkstat(twl);
twl-linkstat = status;
- if (status == OMAP_MUSB_ID_GROUND || status == OMAP_MUSB_VBUS_VALID)
+ if (status == OMAP_MUSB_ID_GROUND || status == OMAP_MUSB_VBUS_VALID) {
omap_musb_mailbox(twl-linkstat);
+ twl4030_phy_resume(twl);
+ }
sysfs_notify(twl-dev-kobj, NULL, vbus);
return 0;
}
+static int twl4030_phy_init(struct phy *phy)
+{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+ return twl4030_usb_phy_init(twl-phy);
+}
+
static int twl4030_set_suspend(struct usb_phy *x, int suspend)
{
struct twl4030_usb *twl = phy_to_twl(x);
@@ -646,13 +672,22 @@ static int twl4030_set_host(struct usb_otg *otg, struct
usb_bus *host)
return 0;
}
+static const struct phy_ops ops = {
+ .init = twl4030_phy_init,
+ .power_on = twl4030_phy_power_on,
+ .power_off = twl4030_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
static int twl4030_usb_probe(struct platform_device *pdev)
{
struct twl4030_usb_data *pdata = pdev-dev.platform_data;
struct twl4030_usb *twl;
+ struct phy *phy;
int status, err;
struct usb_otg *otg;
struct device_node *np = pdev-dev.of_node;
+ struct phy_provider *phy_provider;
twl = devm_kzalloc(pdev-dev, sizeof *twl, GFP_KERNEL);
if (!twl)
@@ -689,6 +724,19 @@ static int twl4030_usb_probe(struct platform_device *pdev)
otg-set_host = twl4030_set_host;
otg-set_peripheral = twl4030_set_peripheral;
+ phy_provider = devm_of_phy_provider_register(twl-dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(twl-dev, 0, ops, twl4030);
+ if (IS_ERR(phy)) {
+ dev_dbg(pdev-dev, Failed to create PHY\n);
+ return PTR_ERR(phy);
+ }
+
+ phy_set_drvdata(phy, twl);
+
/* init spinlock for workqueue */
spin_lock_init(twl-lock);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/15] ARM: OMAP: USB: Add phy binding information
* Kishon Vijay Abraham I kis...@ti.com [130717 23:53]:
In order for controllers to get PHY in case of non dt boot, the phy
binding information (phy device name) should be added in the platform
data of the controller.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
arch/arm/mach-omap2/usb-musb.c |3 +++
include/linux/usb/musb.h |3 +++
2 files changed, 6 insertions(+)
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 8c4de27..6aa7cbf 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -85,6 +85,9 @@ void __init usb_musb_init(struct omap_musb_board_data
*musb_board_data)
musb_plat.mode = board_data-mode;
musb_plat.extvbus = board_data-extvbus;
+ if (cpu_is_omap34xx())
+ musb_plat.phy_label = twl4030;
+
if (soc_is_am35xx()) {
oh_name = am35x_otg_hs;
name = musb-am35x;
I don't think there's a USB PHY on non-twl4030 chips, so this should
be OK:
Acked-by: Tony Lindgren t...@atomide.com
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 053c268..596f8c8 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -104,6 +104,9 @@ struct musb_hdrc_platform_data {
/* for clk_get() */
const char *clock;
+ /* phy label */
+ const char *phy_label;
+
/* (HOST or OTG) switch VBUS on/off */
int (*set_vbus)(struct device *dev, int is_on);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/15] ARM: dts: omap: update usb_otg_hs data
* Kishon Vijay Abraham I kis...@ti.com [130717 23:53]:
Updated the usb_otg_hs dt data to include the *phy* and *phy-names*
binding in order for the driver to use the new generic PHY framework.
Also updated the Documentation to include the binding information.
The PHY binding information can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Acked-by: Felipe Balbi ba...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
In general the .dts changes should be separate to avoid pointless
merge conflicts. But sounds like things will stop working for
USB unless we do it like this so:
Acked-by: Tony Lindgren t...@atomide.com
---
Documentation/devicetree/bindings/usb/omap-usb.txt |5 +
Documentation/devicetree/bindings/usb/usb-phy.txt |6 ++
arch/arm/boot/dts/omap3-beagle-xm.dts |2 ++
arch/arm/boot/dts/omap3-evm.dts|2 ++
arch/arm/boot/dts/omap3-overo.dtsi |2 ++
arch/arm/boot/dts/omap4.dtsi |3 +++
arch/arm/boot/dts/twl4030.dtsi |1 +
7 files changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 57e71f6..825790d 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -19,6 +19,9 @@ OMAP MUSB GLUE
- power : Should be 50. This signifies the controller can supply up to
100mA when operating in host mode.
- usb-phy : the phandle for the PHY device
+ - phys : the phandle for the PHY device (used by generic PHY framework)
+ - phy-names : the names of the PHY corresponding to the PHYs present in the
+ *phy* phandle.
Optional properties:
- ctrl-module : phandle of the control module this glue uses to write to
@@ -33,6 +36,8 @@ usb_otg_hs: usb_otg_hs@4a0ab000 {
num-eps = 16;
ram-bits = 12;
ctrl-module = omap_control_usb;
+ phys = usb2_phy;
+ phy-names = usb2-phy;
};
Board specific device node entry
diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt
b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 61496f5..c0245c8 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -5,6 +5,8 @@ OMAP USB2 PHY
Required properties:
- compatible: Should be ti,omap-usb2
- reg : Address and length of the register set for the device.
+ - #phy-cells: determine the number of cells that should be given in the
+ phandle while referencing this phy.
Optional properties:
- ctrl-module : phandle of the control module used by PHY driver to power on
@@ -16,6 +18,7 @@ usb2phy@4a0ad080 {
compatible = ti,omap-usb2;
reg = 0x4a0ad080 0x58;
ctrl-module = omap_control_usb;
+ #phy-cells = 0;
};
OMAP USB3 PHY
@@ -25,6 +28,8 @@ Required properties:
- reg : Address and length of the register set for the device.
- reg-names: The names of the register addresses corresponding to the
registers
filled in reg.
+ - #phy-cells: determine the number of cells that should be given in the
+ phandle while referencing this phy.
Optional properties:
- ctrl-module : phandle of the control module used by PHY driver to power on
@@ -39,4 +44,5 @@ usb3phy@4a084400 {
0x4a084c00 0x40;
reg-names = phy_rx, phy_tx, pll_ctrl;
ctrl-module = omap_control_usb;
+ #phy-cells = 0;
};
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts
b/arch/arm/boot/dts/omap3-beagle-xm.dts
index afdb164..533b2da 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -144,6 +144,8 @@
usb_otg_hs {
interface-type = 0;
usb-phy = usb2_phy;
+ phys = usb2_phy;
+ phy-names = usb2-phy;
mode = 3;
power = 50;
};
diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts
index 7d4329d..4134dd0 100644
--- a/arch/arm/boot/dts/omap3-evm.dts
+++ b/arch/arm/boot/dts/omap3-evm.dts
@@ -70,6 +70,8 @@
usb_otg_hs {
interface-type = 0;
usb-phy = usb2_phy;
+ phys = usb2_phy;
+ phy-names = usb2-phy;
mode = 3;
power = 50;
};
diff --git a/arch/arm/boot/dts/omap3-overo.dtsi
b/arch/arm/boot/dts/omap3-overo.dtsi
index 8f1abec..a461d2f 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -76,6 +76,8 @@
usb_otg_hs {
interface-type = 0;
usb-phy = usb2_phy;
+ phys = usb2_phy;
+ phy-names = usb2-phy;
mode = 3;
power = 50;
};
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..1e8e2fe 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -520,6 +520,7 @@
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
+struct phy_provider *__of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args));
+struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args))
+
+__of_phy_provider_register and __devm_of_phy_provider_register can be used to
+register the phy_provider and it takes device, owner and of_xlate as
+arguments. For the dt boot case, all PHY providers should use one of the
above
+2 APIs to register the PHY provider.
Why do you have __ for the prefix of a public function? Is that really
the way that OF handles this type of thing?
--- /dev/null
+++ b/drivers/phy/Kconfig
@@ -0,0 +1,13 @@
+#
+# PHY
+#
+
+menuconfig GENERIC_PHY
+ tristate PHY Subsystem
+ help
+ Generic PHY support.
+
+ This framework is designed to provide a generic interface for PHY
+ devices present in the kernel. This layer will have the generic
+ API by which phy drivers can create PHY using the phy framework and
+ phy users can obtain reference to the PHY.
Again, please reverse this. The drivers that use it should select it,
not depend on it, which will then enable this option. I will never know
if I need to enable it, and based on your follow-on patches, if I don't,
drivers that were working just fine, now disappeared from my build,
which isn't nice, and a pain to notice and fix up.
+/**
+ * phy_create() - create a new phy
+ * @dev: device that is creating the new phy
+ * @id: id of the phy
+ * @ops: function pointers for performing phy operations
+ * @label: label given to the phy
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
+ const char *label)
+{
+ int ret;
+ struct phy *phy;
+
+ if (!dev) {
+ dev_WARN(dev, no device provided for PHY\n);
+ ret = -EINVAL;
+ goto err0;
+ }
+
+ phy = kzalloc(sizeof(*phy), GFP_KERNEL);
+ if (!phy) {
+ ret = -ENOMEM;
+ goto err0;
+ }
+
+ device_initialize(phy-dev);
+ mutex_init(phy-mutex);
+
+ phy-dev.class = phy_class;
+ phy-dev.parent = dev;
+ phy-dev.of_node = dev-of_node;
+ phy-id = id;
+ phy-ops = ops;
+ phy-label = kstrdup(label, GFP_KERNEL);
+
+ ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
Your naming is odd, no phy anywhere in it? You rely on the sender to
never send a duplicate name.id pair? Why not create your own ids based
on the number of phys in the system, like almost all other classes and
subsystems do?
+static inline int phy_pm_runtime_get(struct phy *phy)
+{
+ if (WARN(IS_ERR(phy), Invalid PHY reference\n))
+ return -EINVAL;
Why would phy ever not be valid and a error pointer? And why dump the
stack if that happens, that seems really extreme.
+
+ if (!pm_runtime_enabled(phy-dev))
+ return -ENOTSUPP;
+
+ return pm_runtime_get(phy-dev);
+}
This, and the other inline functions in this .h file seem huge, why are
they inline and not in the .c file? There's no speed issues, and it
should save space overall in the .c file. Please move them.
+static inline int phy_init(struct phy *phy)
+{
+ int ret;
+
+ ret = phy_pm_runtime_get_sync(phy);
+ if (ret 0 ret != -ENOTSUPP)
+ return ret;
+
+ mutex_lock(phy-mutex);
+ if (phy-init_count++ == 0 phy-ops-init) {
+ ret = phy-ops-init(phy);
+ if (ret 0) {
+ dev_err(phy-dev, phy init failed -- %d\n, ret);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(phy-mutex);
+ phy_pm_runtime_put(phy);
+ return ret;
+}
+
+static inline int phy_exit(struct phy *phy)
+{
+ int ret;
+
+ ret = phy_pm_runtime_get_sync(phy);
+ if (ret 0 ret != -ENOTSUPP)
+ return ret;
+
+ mutex_lock(phy-mutex);
+ if (--phy-init_count == 0 phy-ops-exit) {
+ ret = phy-ops-exit(phy);
+ if (ret 0) {
+ dev_err(phy-dev, phy exit failed -- %d\n, ret);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(phy-mutex);
+ phy_pm_runtime_put(phy);
+ return ret;
+}
+
+static inline int phy_power_on(struct phy *phy)
+{
+ int ret = -ENOTSUPP;
+
+ ret = phy_pm_runtime_get_sync(phy);
+ if (ret 0 ret != -ENOTSUPP)
+ return ret;
+
+ mutex_lock(phy-mutex);
+ if (phy-power_count++ == 0 phy-ops-power_on) {
+ ret = phy-ops-power_on(phy);
+ if (ret 0) {
+ dev_err(phy-dev, phy
Re: [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework
On Thu, Jul 18, 2013 at 12:16:11PM +0530, Kishon Vijay Abraham I wrote:
Used the generic PHY framework API to create the PHY. Now the power off and
power on are done in omap_usb_power_off and omap_usb_power_on respectively.
However using the old USB PHY library cannot be completely removed
because OTG is intertwined with PHY and moving to the new framework
will break OTG. Once we have a separate OTG state machine, we
can get rid of the USB PHY library.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
drivers/usb/phy/Kconfig |1 +
drivers/usb/phy/phy-omap-usb2.c | 45
+++
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 3622fff..cc55993 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
config OMAP_USB2
tristate OMAP USB2 PHY Driver
depends on ARCH_OMAP2PLUS
+ depends on GENERIC_PHY
select OMAP_CONTROL_USB
help
Enable this to support the transceiver that is part of SOC. This
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..751b30f 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
#include linux/pm_runtime.h
#include linux/delay.h
#include linux/usb/omap_control_usb.h
+#include linux/phy/phy.h
/**
* omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int
suspend)
return 0;
}
+static int omap_usb_power_off(struct phy *x)
+{
+ struct omap_usb *phy = phy_get_drvdata(x);
+
+ omap_control_usb_phy_power(phy-control_dev, 0);
+
+ return 0;
+}
+
+static int omap_usb_power_on(struct phy *x)
+{
+ struct omap_usb *phy = phy_get_drvdata(x);
+
+ omap_control_usb_phy_power(phy-control_dev, 1);
+
+ return 0;
+}
+
+static struct phy_ops ops = {
+ .power_on = omap_usb_power_on,
+ .power_off = omap_usb_power_off,
+ .owner = THIS_MODULE,
+};
+
static int omap_usb2_probe(struct platform_device *pdev)
{
struct omap_usb *phy;
+ struct phy *generic_phy;
struct usb_otg *otg;
+ struct phy_provider *phy_provider;
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-phy.otg= otg;
phy-phy.type = USB_PHY_TYPE_USB2;
+ phy_provider = devm_of_phy_provider_register(phy-dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
phy-control_dev = omap_get_control_dev();
if (IS_ERR(phy-control_dev)) {
dev_dbg(pdev-dev, Failed to get control device\n);
@@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
otg-start_srp = omap_usb_start_srp;
otg-phy= phy-phy;
+ platform_set_drvdata(pdev, phy);
+ pm_runtime_enable(phy-dev);
+
+ generic_phy = devm_phy_create(phy-dev, 0, ops, omap-usb2);
+ if (IS_ERR(generic_phy))
+ return PTR_ERR(generic_phy);
So, if I have two of these controllers in my system, I can't create the
second phy because the name for it will be identical to the first?
That's why the phy core should handle the id, and not rely on the
drivers to set it, as they have no idea how many they have in the
system.
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
[PATCH v4 4/5] USB: gadget: atmel_usba: prepare clk before calling enable
Replace clk_enable/disable with clk_prepare_enable/disable_unprepare to
avoid common clk framework warnings.
Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com
---
drivers/usb/gadget/atmel_usba_udc.c | 28 +---
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 1d97222..f018017 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1772,6 +1772,7 @@ out:
static int atmel_usba_start(struct usb_gadget *gadget,
struct usb_gadget_driver *driver)
{
+ int ret = 0;
struct usba_udc *udc = container_of(gadget, struct usba_udc, gadget);
unsigned long flags;
@@ -1781,8 +1782,14 @@ static int atmel_usba_start(struct usb_gadget *gadget,
udc-driver = driver;
spin_unlock_irqrestore(udc-lock, flags);
- clk_enable(udc-pclk);
- clk_enable(udc-hclk);
+ ret = clk_prepare_enable(udc-pclk);
+ if (ret)
+ goto out;
+ ret = clk_prepare_enable(udc-hclk);
+ if (ret) {
+ clk_disable_unprepare(udc-pclk);
+ goto out;
+ }
DBG(DBG_GADGET, registered driver `%s'\n, driver-driver.name);
@@ -1797,9 +1804,11 @@ static int atmel_usba_start(struct usb_gadget *gadget,
usba_writel(udc, CTRL, USBA_ENABLE_MASK);
usba_writel(udc, INT_ENB, USBA_END_OF_RESET);
}
+
+out:
spin_unlock_irqrestore(udc-lock, flags);
- return 0;
+ return ret;
}
static int atmel_usba_stop(struct usb_gadget *gadget,
@@ -1822,8 +1831,8 @@ static int atmel_usba_stop(struct usb_gadget *gadget,
udc-driver = NULL;
- clk_disable(udc-hclk);
- clk_disable(udc-pclk);
+ clk_disable_unprepare(udc-hclk);
+ clk_disable_unprepare(udc-pclk);
DBG(DBG_GADGET, unregistered driver `%s'\n, driver-driver.name);
@@ -2022,10 +2031,14 @@ static int __init usba_udc_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, udc);
/* Make sure we start from a clean slate */
- clk_enable(pclk);
+ ret = clk_prepare_enable(pclk);
+ if (ret) {
+ dev_err(pdev-dev, Unable to enable pclk, aborting.\n);
+ goto err_clk_enable;
+ }
toggle_bias(0);
usba_writel(udc, CTRL, USBA_DISABLE_MASK);
- clk_disable(pclk);
+ clk_disable_unprepare(pclk);
if (pdev-dev.of_node)
udc-usba_ep = atmel_udc_of_init(pdev, udc);
@@ -2081,6 +2094,7 @@ err_add_udc:
free_irq(irq, udc);
err_request_irq:
err_alloc_ep:
+err_clk_enable:
iounmap(udc-fifo);
err_map_fifo:
iounmap(udc-regs);
--
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] usb: udc: add gadget state kobject uevent
Hi,
On Wed, Jul 17, 2013 at 11:37:35AM -0400, Alan Stern wrote:
On Wed, 17 Jul 2013, Felipe Balbi wrote:
On Wed, Jul 17, 2013 at 09:04:54PM +0800, Rong Wang wrote:
Hi Felipe,
On Wed, Jul 17, 2013 at 3:57 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Mon, Jul 15, 2013 at 11:31:17PM -0700, Greg KH wrote:
The question is since we default GADGET, so the g_mass_storage.ko is
installed early but connecting to a host PC
is randomly, But the udev has no idea when a host PC connects our
device.
So we consider it's reasonable to let the udev know the GADGET
device state.
Is there any alternative to our question?
I thought we already export events for gadget device states, have you
looked for them? I can't dig through the code at the moment, but this
seems like a pretty common issue...
Felipe, any ideas?
we already expose that in sysfs. IIRC udev can act on sysfs changes,
no ?
I do not know if udev can polling sysfs file content change. I'll study
this.
But the change is triggered by calling usb_gadget_set_state, and I find
composite framework do not call this. Then we should do this common work
in every udc driver?
yes. Only the UDC driver knows when the controller is moving among those
states.
Not quite. Only the gadget driver knows when the transition between
ADDRESS and CONFIGURED occurs. This should be added to composite.c.
that's not entirely true :-) See how we handle that in dwc3:
| static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
| {
| enum usb_device_state state = dwc-gadget.state;
| u32 cfg;
| int ret;
| u32 reg;
|
| dwc-start_config_issued = false;
| cfg = le16_to_cpu(ctrl-wValue);
|
| switch (state) {
| case USB_STATE_DEFAULT:
| return -EINVAL;
| break;
|
| case USB_STATE_ADDRESS:
| ret = dwc3_ep0_delegate_req(dwc, ctrl);
| /* if the cfg matches and the cfg is non zero */
| if (cfg (!ret || (ret == USB_GADGET_DELAYED_STATUS))) {
| usb_gadget_set_state(dwc-gadget,
| USB_STATE_CONFIGURED);
|
| /*
|* Enable transition to U1/U2 state when
|* nothing is pending from application.
|*/
| reg = dwc3_readl(dwc-regs, DWC3_DCTL);
| reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA);
| dwc3_writel(dwc-regs, DWC3_DCTL, reg);
|
| dwc-resize_fifos = true;
| dev_dbg(dwc-dev, resize fifos flag SET\n);
| }
| break;
|
| case USB_STATE_CONFIGURED:
| ret = dwc3_ep0_delegate_req(dwc, ctrl);
| if (!cfg)
| usb_gadget_set_state(dwc-gadget,
| USB_STATE_ADDRESS);
| break;
| default:
| ret = -EINVAL;
| }
| return ret;
| }
Also, until other gadget drivers add notifications to the other cases, I
don't think it's wise to add a transition from NOTATTACHED to
CONFIGURED.
But I have one change I'll send for the gadget notifications, I'm just
trying to get a new OMAP5 board to test because the FTDI chip on mine
died and I have no console :-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: udc: add gadget state kobject uevent
Hi Peter, On Wed, Jul 17, 2013 at 10:36 AM, Peter Chen peter.c...@freescale.com wrote: On Mon, Jul 15, 2013 at 11:31:17PM -0700, Greg KH wrote: On Tue, Jul 16, 2013 at 11:49:07AM +0800, Rong Wang wrote: Hi Greg, The USB on our platform can change roles between HOST and GADGET, but it is not capable of OTG. That kind of sounds like the definition of OTG :) When the USB changes between roles the udev will run some scripts automatically according to the udev rules. What exactly does udev/userspace see when the roles change? And what can trigger the change in roles? The default role is GADGET, and we bind the g_mass_storage to the USB GADGET role. We should secure the back end file storage between the device and the host PC connecting to our device. We need to know when the GADGET is really connect to a host PC, then we can umount the file on the device and export it to the g_mass_storage. I thought you already get an event for this, otherwise no one would be able to properly deal with this. The question is since we default GADGET, so the g_mass_storage.ko is installed early but connecting to a host PC is randomly, But the udev has no idea when a host PC connects our device. So we consider it's reasonable to let the udev know the GADGET device state. Is there any alternative to our question? I thought we already export events for gadget device states, have you looked for them? I can't dig through the code at the moment, but this seems like a pretty common issue... If I understand correctly, what Rong wants is udev can be notified the udc state changes, like connect/disconnect event. Currently, we only export it to /sys. OK. Thanks for your share. And you develop a new utility rather than udev to monitor that file? And you probably create a work queue in your udc driver to do this work? -- 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 -- 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: udc: add gadget state kobject uevent
On Wed, Jul 17, 2013 at 9:27 PM, Felipe Balbi ba...@ti.com wrote: On Wed, Jul 17, 2013 at 09:04:54PM +0800, Rong Wang wrote: Hi Felipe, On Wed, Jul 17, 2013 at 3:57 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Mon, Jul 15, 2013 at 11:31:17PM -0700, Greg KH wrote: The question is since we default GADGET, so the g_mass_storage.ko is installed early but connecting to a host PC is randomly, But the udev has no idea when a host PC connects our device. So we consider it's reasonable to let the udev know the GADGET device state. Is there any alternative to our question? I thought we already export events for gadget device states, have you looked for them? I can't dig through the code at the moment, but this seems like a pretty common issue... Felipe, any ideas? we already expose that in sysfs. IIRC udev can act on sysfs changes, no ? I do not know if udev can polling sysfs file content change. I'll study this. But the change is triggered by calling usb_gadget_set_state, and I find composite framework do not call this. Then we should do this common work in every udc driver? yes. Only the UDC driver knows when the controller is moving among those states. OK. I got that. But I think it may be more reasonable for the udc driver to maintain a work queue to handle the state change since this operation mostly happen in ISR ? -- balbi -- 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: udc: add gadget state kobject uevent
Hi,
On Thu, Jul 18, 2013 at 04:33:43PM +0800, Rong Wang wrote:
On Wed, Jul 17, 2013 at 9:27 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Jul 17, 2013 at 09:04:54PM +0800, Rong Wang wrote:
Hi Felipe,
On Wed, Jul 17, 2013 at 3:57 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Mon, Jul 15, 2013 at 11:31:17PM -0700, Greg KH wrote:
The question is since we default GADGET, so the g_mass_storage.ko is
installed early but connecting to a host PC
is randomly, But the udev has no idea when a host PC connects our
device.
So we consider it's reasonable to let the udev know the GADGET device
state.
Is there any alternative to our question?
I thought we already export events for gadget device states, have you
looked for them? I can't dig through the code at the moment, but this
seems like a pretty common issue...
Felipe, any ideas?
we already expose that in sysfs. IIRC udev can act on sysfs changes,
no ?
I do not know if udev can polling sysfs file content change. I'll study
this.
But the change is triggered by calling usb_gadget_set_state, and I find
composite framework do not call this. Then we should do this common work
in every udc driver?
yes. Only the UDC driver knows when the controller is moving among those
states.
OK. I got that.
But I think it may be more reasonable for the udc driver to maintain a
work queue
to handle the state change since this operation mostly happen in ISR ?
And that's the patch I want to test. Adding a workqueue to every single
UDC will be too much, so I tried to hide it in udc-core.c. I agree with
you we need to pass the sysfs_notification to a separate workqueue
though. If you can test the patch below, that would be great.
commit d32521bd775d48b600e67f23d363d70f71597ffd
Author: Felipe Balbi ba...@ti.com
Date: Wed Jul 17 11:09:49 2013 +0300
usb: gadget: udc-core: move sysfs_notify() to a workqueue
usb_gadget_set_state() will call sysfs_notify()
which might sleep. Some users might want to call
usb_gadget_set_state() from the very IRQ handler
which actually changes the gadget state.
Instead of having every UDC driver add their own
workqueue for such a simple notification, we're
adding it generically to our struct usb_gadget,
so the details are hidden from all UDC drivers.
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..b0d91b1 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -23,6 +23,7 @@
#include linux/list.h
#include linux/err.h
#include linux/dma-mapping.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* - */
+static void usb_gadget_state_work(struct work_struct *work)
+{
+ struct usb_gadget *gadget = work_to_gadget(work);
+
+ sysfs_notify(gadget-dev.kobj, NULL, status);
+}
+
void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ schedule_work(gadget-work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
@@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err1;
dev_set_name(gadget-dev, gadget);
+ INIT_WORK(gadget-work, usb_gadget_state_work);
gadget-dev.parent = parent;
dma_set_coherent_mask(gadget-dev, parent-coherent_dma_mask);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index f1b0dca..942ef5e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -22,6 +22,7 @@
#include linux/slab.h
#include linux/scatterlist.h
#include linux/types.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
struct usb_ep;
@@ -475,6 +476,7 @@ struct usb_gadget_ops {
/**
* struct usb_gadget - represents a usb slave device
+ * @work: (internal use) Workqueue to be used for sysfs_notify()
* @ops: Function pointers used to access hardware-specific operations.
* @ep0: Endpoint zero, used when reading or writing responses to
* driver setup() requests
@@ -520,6 +522,7 @@ struct usb_gadget_ops {
* device is acting as a B-Peripheral (so is_a_peripheral is false).
*/
struct usb_gadget {
+ struct work_struct work;
/* readonly to gadget driver */
const struct usb_gadget_ops *ops;
struct usb_ep *ep0;
@@ -538,6 +541,7 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
};
+#define work_to_gadget(w) (container_of((w), struct usb_gadget, work))
static inline void set_gadget_data(struct
[PATCH 5/6] ARM: dts: omap3-beagle-xm: Add USB Host support
Provide RESET controller and Power regulator for the USB PHY,
the USB Host port mode and the PHY device. Provide
pin multiplexer information for USB host pins.
We also relocate omap3_pmx_core pin definations so that they
are close to omap3_pmx_wkup pin definations.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/omap3-beagle-xm.dts | 75 +
1 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts
b/arch/arm/boot/dts/omap3-beagle-xm.dts
index afdb164..2273c24 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -69,6 +69,33 @@
};
};
+
+ /* HS USB Port 2 RESET */
+ hsusb2_reset: hsusb2_reset {
+ compatible = gpio-reset;
+ reset-gpios = gpio5 19 GPIO_ACTIVE_LOW; /* gpio_147 */
+ reset-delay-us = 7;
+ initially-in-reset;
+ #reset-cells = 0;
+ };
+
+ /* HS USB Port 2 Power */
+ hsusb2_power: hsusb2_power_reg {
+ compatible = regulator-fixed;
+ regulator-name = hsusb2_vbus;
+ regulator-min-microvolt = 330;
+ regulator-max-microvolt = 330;
+ gpio = twl_gpio 18 0;/* GPIO LEDA */
+ startup-delay-us = 7;
+ };
+
+ /* HS USB Host PHY on PORT 2 */
+ hsusb2_phy: hsusb2_phy {
+ compatible = usb-nop-xceiv;
+ resets = hsusb2_reset;
+ reset-names = reset;
+ vcc-supply = hsusb2_power;
+ };
};
omap3_pmx_wkup {
@@ -79,6 +106,37 @@
};
};
+omap3_pmx_core {
+ pinctrl-names = default;
+ pinctrl-0 =
+ hsusbb2_pins
+ ;
+
+ uart3_pins: pinmux_uart3_pins {
+ pinctrl-single,pins =
+ 0x16e (PIN_INPUT | PIN_OFF_WAKEUPENABLE | MUX_MODE0) /*
uart3_rx_irrx.uart3_rx_irrx */
+ 0x170 (PIN_OUTPUT | MUX_MODE0) /*
uart3_tx_irtx.uart3_tx_irtx OUTPUT | MODE0 */
+ ;
+ };
+
+ hsusbb2_pins: pinmux_hsusbb2_pins {
+ pinctrl-single,pins =
+ 0x5c0 (PIN_OUTPUT | MUX_MODE3) /*
etk_d10.hsusb2_clk */
+ 0x5c2 (PIN_OUTPUT | MUX_MODE3) /*
etk_d11.hsusb2_stp */
+ 0x5c4 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d12.hsusb2_dir */
+ 0x5c6 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d13.hsusb2_nxt */
+ 0x5c8 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d14.hsusb2_data0 */
+ 0x5cA (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d15.hsusb2_data1 */
+ 0x1a4 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi1_cs3.hsusb2_data2 */
+ 0x1a6 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_clk.hsusb2_data7 */
+ 0x1a8 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_simo.hsusb2_data4 */
+ 0x1aa (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_somi.hsusb2_data5 */
+ 0x1ac (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_cs0.hsusb2_data6 */
+ 0x1ae (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_cs1.hsusb2_data3 */
+ ;
+ };
+};
+
i2c1 {
clock-frequency = 260;
@@ -148,15 +206,6 @@
power = 50;
};
-omap3_pmx_core {
- uart3_pins: pinmux_uart3_pins {
- pinctrl-single,pins =
- 0x16e (PIN_INPUT | PIN_OFF_WAKEUPENABLE | MUX_MODE0) /*
uart3_rx_irrx.uart3_rx_irrx */
- 0x170 (PIN_OUTPUT | MUX_MODE0) /*
uart3_tx_irtx.uart3_tx_irtx OUTPUT | MODE0 */
- ;
- };
-};
-
uart3 {
pinctrl-names = default;
pinctrl-0 = uart3_pins;
@@ -166,3 +215,11 @@
pinctrl-names = default;
pinctrl-0 = gpio1_pins;
};
+
+usbhshost {
+ port2-mode = ehci-phy;
+};
+
+usbhsehci {
+ phys = 0 hsusb2_phy;
+};
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] USB: phy: phy-nop: Use RESET controller for managing the reset line
Hi, Till now we were modelling the RESET line as a voltage regulator and using the regulator framework to manage it. [1] introduces a GPIO based reset controller driver. We use that to manage the PHY reset line, at least for DT boots. For legacy boots, will still need to use the regulator framework for reset lines. Felipe, The first patch is for you. The Kconfig change might conflict if you apply this on top of my PHY Kconfig cleanup series. Benoit, Patches 2 to 6 are for you. NOTE: All patches depend on [1] which is yet to be merged in. cheers, -roger [1] - http://thread.gmane.org/gmane.linux.drivers.devicetree/41348 Roger Quadros (6): usb: phy-nop: Use RESET Controller for managing the reset line ARM: dts: omap3-beagle: Use reset-gpio driver for hsusb2_reset ARM: dts: omap4-panda: Use reset-gpio driver for hsusb1_reset ARM: dts: omap5-uevm: Use reset-gpio driver for hsusb2_reset ARM: dts: omap3-beagle-xm: Add USB Host support ARM: dts: omap3-beagle: Make USB host pin naming consistent .../devicetree/bindings/usb/usb-nop-xceiv.txt | 10 ++- arch/arm/boot/dts/omap3-beagle-xm.dts | 75 +--- arch/arm/boot/dts/omap3-beagle.dts | 41 +-- arch/arm/boot/dts/omap4-panda-common.dtsi | 22 ++ arch/arm/boot/dts/omap5-uevm.dts | 17 ++--- drivers/usb/phy/Kconfig|1 + drivers/usb/phy/phy-nop.c | 48 ++--- 7 files changed, 146 insertions(+), 68 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] ARM: dts: omap3-beagle: Make USB host pin naming consistent
Use a common naming scheme mode0name.modename flags for the
USB host pins to be consistent.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/omap3-beagle.dts | 24
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-beagle.dts
b/arch/arm/boot/dts/omap3-beagle.dts
index 01fbad6..27aed65 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -100,18 +100,18 @@
hsusbb2_pins: pinmux_hsusbb2_pins {
pinctrl-single,pins =
- 0x5c0 (PIN_OUTPUT | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_clk */
- 0x5c2 (PIN_OUTPUT | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_stp */
- 0x5c4 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dir */
- 0x5c6 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_nxt */
- 0x5c8 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat0 */
- 0x5cA (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat1 */
- 0x1a4 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat2 */
- 0x1a6 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat3 */
- 0x1a8 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat4 */
- 0x1aa (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat5 */
- 0x1ac (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat6 */
- 0x1ae (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
usbb2_ulpitll_clk.usbb1_ulpiphy_dat7 */
+ 0x5c0 (PIN_OUTPUT | MUX_MODE3) /*
etk_d10.hsusb2_clk */
+ 0x5c2 (PIN_OUTPUT | MUX_MODE3) /*
etk_d11.hsusb2_stp */
+ 0x5c4 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d12.hsusb2_dir */
+ 0x5c6 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d13.hsusb2_nxt */
+ 0x5c8 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d14.hsusb2_data0 */
+ 0x5cA (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
etk_d15.hsusb2_data1 */
+ 0x1a4 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi1_cs3.hsusb2_data2 */
+ 0x1a6 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_clk.hsusb2_data7 */
+ 0x1a8 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_simo.hsusb2_data4 */
+ 0x1aa (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_somi.hsusb2_data5 */
+ 0x1ac (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_cs0.hsusb2_data6 */
+ 0x1ae (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_cs1.hsusb2_data3 */
;
};
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] usb: phy-nop: Use RESET Controller for managing the reset line
Till now we were modelling the RESET line as a voltage regulator and
using the regulator framework to manage it.
[1] introduces a GPIO based reset controller driver. We use that
to manage the PHY reset line, at least for DT boots. For legacy boots,
will still need to use the regulator framework for reset lines.
[1] - http://thread.gmane.org/gmane.linux.drivers.devicetree/41348
Signed-off-by: Roger Quadros rog...@ti.com
---
.../devicetree/bindings/usb/usb-nop-xceiv.txt | 10 +++--
drivers/usb/phy/Kconfig|1 +
drivers/usb/phy/phy-nop.c | 48 +++-
3 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
index d7e2726..5c3e978 100644
--- a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
+++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
@@ -15,7 +15,9 @@ Optional properties:
- vcc-supply: phandle to the regulator that provides RESET to the PHY.
-- reset-supply: phandle to the regulator that provides power to the PHY.
+- resets: phandle to the reset controller.
+
+- reset-names: must be reset
Example:
@@ -25,10 +27,10 @@ Example:
clocks = osc 0;
clock-names = main_clk;
vcc-supply = hsusb1_vcc_regulator;
- reset-supply = hsusb1_reset_regulator;
+ resets = hsusb1_reset;
+ reset-names = reset
};
hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
-hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
-controls RESET.
+hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset controls RESET.
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 3622fff..213b5ad 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -58,6 +58,7 @@ config MV_U3D_PHY
config NOP_USB_XCEIV
tristate NOP USB Transceiver Driver
+ depends on RESET_CONTROLLER
help
This driver is to be used by all the usb transceiver which are either
built-in with usb ip or which are autonomous and doesn't require any
diff --git a/drivers/usb/phy/phy-nop.c b/drivers/usb/phy/phy-nop.c
index 55445e5d..a2cbda5 100644
--- a/drivers/usb/phy/phy-nop.c
+++ b/drivers/usb/phy/phy-nop.c
@@ -35,13 +35,15 @@
#include linux/clk.h
#include linux/regulator/consumer.h
#include linux/of.h
+#include linux/reset.h
struct nop_usb_xceiv {
struct usb_phy phy;
struct device *dev;
struct clk *clk;
struct regulator *vcc;
- struct regulator *reset;
+ struct regulator *reset_reg; /* only used for non-DT boot */
+ struct reset_control *reset;
};
static struct platform_device *pd;
@@ -82,9 +84,14 @@ static int nop_init(struct usb_phy *phy)
if (!IS_ERR(nop-clk))
clk_enable(nop-clk);
+ /* De-assert RESET */
+ if (!IS_ERR(nop-reset_reg)) {
+ if (regulator_enable(nop-reset_reg))
+ dev_err(phy-dev, Failed to de-assert reset\n);
+ }
+
if (!IS_ERR(nop-reset)) {
- /* De-assert RESET */
- if (regulator_enable(nop-reset))
+ if (reset_control_deassert(nop-reset))
dev_err(phy-dev, Failed to de-assert reset\n);
}
@@ -95,9 +102,14 @@ static void nop_shutdown(struct usb_phy *phy)
{
struct nop_usb_xceiv *nop = dev_get_drvdata(phy-dev);
+ /* Assert RESET */
+ if (!IS_ERR(nop-reset_reg)) {
+ if (regulator_disable(nop-reset_reg))
+ dev_err(phy-dev, Failed to assert reset\n);
+ }
+
if (!IS_ERR(nop-reset)) {
- /* Assert RESET */
- if (regulator_disable(nop-reset))
+ if (reset_control_assert(nop-reset))
dev_err(phy-dev, Failed to assert reset\n);
}
@@ -166,7 +178,7 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- needs_reset = of_property_read_bool(node, reset-supply);
+ needs_reset = of_property_read_bool(node, resets);
} else if (pdata) {
type = pdata-type;
@@ -205,12 +217,26 @@ static int nop_usb_xceiv_probe(struct platform_device
*pdev)
return -EPROBE_DEFER;
}
- nop-reset = devm_regulator_get(pdev-dev, reset);
- if (IS_ERR(nop-reset)) {
- dev_dbg(pdev-dev, Error getting reset regulator: %ld\n,
+ nop-reset_reg = ERR_PTR(-EINVAL);
+ nop-reset = ERR_PTR(-EINVAL);
+
+ if (dev-of_node) {
+ nop-reset = devm_reset_control_get(dev, reset);
+
[PATCH 3/6] ARM: dts: omap4-panda: Use reset-gpio driver for hsusb1_reset
We no longer need to model a RESET line as a regulator since
we have the reset-gpio driver available.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/omap4-panda-common.dtsi | 22 --
1 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi
b/arch/arm/boot/dts/omap4-panda-common.dtsi
index faa95b5..e9b0a91 100644
--- a/arch/arm/boot/dts/omap4-panda-common.dtsi
+++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
@@ -60,20 +60,13 @@
AFMR, Line In;
};
- /*
-* Temp hack: Need to be replaced with the proper gpio-controlled
-* reset driver as soon it will be merged.
-* http://thread.gmane.org/gmane.linux.drivers.devicetree/36830
-*/
/* HS USB Port 1 RESET */
- hsusb1_reset: hsusb1_reset_reg {
- compatible = regulator-fixed;
- regulator-name = hsusb1_reset;
- regulator-min-microvolt = 330;
- regulator-max-microvolt = 330;
- gpio = gpio2 30 0; /* gpio_62 */
- startup-delay-us = 7;
- enable-active-high;
+ hsusb1_reset: hsusb1_reset {
+ compatible = gpio-reset;
+ reset-gpios = gpio2 30 GPIO_ACTIVE_LOW; /* gpio_62 */
+ reset-delay-us = 7;
+ initially-in-reset;
+ #reset-cells = 0;
};
/* HS USB Port 1 Power */
@@ -97,7 +90,8 @@
/* HS USB Host PHY on PORT 1 */
hsusb1_phy: hsusb1_phy {
compatible = usb-nop-xceiv;
- reset-supply = hsusb1_reset;
+ resets = hsusb1_reset;
+ reset-names = reset;
vcc-supply = hsusb1_power;
/**
* FIXME:
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] ARM: dts: omap3-beagle: Use reset-gpio driver for hsusb2_reset
We no longer need to model a RESET line as a regulator since
we have the reset-gpio driver available.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/omap3-beagle.dts | 17 -
1 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-beagle.dts
b/arch/arm/boot/dts/omap3-beagle.dts
index dfd8310..01fbad6 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -45,14 +45,12 @@
};
/* HS USB Port 2 RESET */
- hsusb2_reset: hsusb2_reset_reg {
- compatible = regulator-fixed;
- regulator-name = hsusb2_reset;
- regulator-min-microvolt = 330;
- regulator-max-microvolt = 330;
- gpio = gpio5 19 0; /* gpio_147 */
- startup-delay-us = 7;
- enable-active-high;
+ hsusb2_reset: hsusb2_reset {
+ compatible = gpio-reset;
+ reset-gpios = gpio5 19 GPIO_ACTIVE_LOW; /* gpio_147 */
+ reset-delay-us = 7;
+ initially-in-reset;
+ #reset-cells = 0;
};
/* HS USB Port 2 Power */
@@ -68,7 +66,8 @@
/* HS USB Host PHY on PORT 2 */
hsusb2_phy: hsusb2_phy {
compatible = usb-nop-xceiv;
- reset-supply = hsusb2_reset;
+ resets = hsusb2_reset;
+ reset-names = reset;
vcc-supply = hsusb2_power;
};
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] ARM: dts: omap5-uevm: Use reset-gpio driver for hsusb2_reset
We no longer need to model a RESET line as a regulator since
we have the reset-gpio driver available.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/omap5-uevm.dts | 17 -
1 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts
index 08b7267..08650f9 100644
--- a/arch/arm/boot/dts/omap5-uevm.dts
+++ b/arch/arm/boot/dts/omap5-uevm.dts
@@ -28,20 +28,19 @@
};
/* HS USB Port 2 RESET */
- hsusb2_reset: hsusb2_reset_reg {
- compatible = regulator-fixed;
- regulator-name = hsusb2_reset;
- regulator-min-microvolt = 330;
- regulator-max-microvolt = 330;
- gpio = gpio3 16 GPIO_ACTIVE_HIGH; /* gpio3_80 HUB_NRESET */
- startup-delay-us = 7;
- enable-active-high;
+ hsusb2_reset: hsusb2_reset {
+ compatible = gpio-reset;
+ reset-gpios = gpio2 30 GPIO_ACTIVE_LOW; /* gpio3_80
HUB_NRESET */
+ reset-delay-us = 7;
+ initially-in-reset;
+ #reset-cells = 0;
};
/* HS USB Host PHY on PORT 2 */
hsusb2_phy: hsusb2_phy {
compatible = usb-nop-xceiv;
- reset-supply = hsusb2_reset;
+ resets = hsusb2_reset;
+ reset_names = reset;
/**
* FIXME
* Put the right clock phandle here when available
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
Hi,
On Thursday 18 July 2013 12:50 PM, Greg KH wrote:
On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
+struct phy_provider *__of_phy_provider_register(struct device *dev,
+struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+struct of_phandle_args *args));
+struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
+struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+struct of_phandle_args *args))
+
+__of_phy_provider_register and __devm_of_phy_provider_register can be used
to
+register the phy_provider and it takes device, owner and of_xlate as
+arguments. For the dt boot case, all PHY providers should use one of the
above
+2 APIs to register the PHY provider.
Why do you have __ for the prefix of a public function? Is that really
the way that OF handles this type of thing?
I have a macro of_phy_provider_register/devm_of_phy_provider_register that
calls these functions and should be used by the PHY drivers. Probably I should
make a mention of it in the Documentation.
--- /dev/null
+++ b/drivers/phy/Kconfig
@@ -0,0 +1,13 @@
+#
+# PHY
+#
+
+menuconfig GENERIC_PHY
+tristate PHY Subsystem
+help
+ Generic PHY support.
+
+ This framework is designed to provide a generic interface for PHY
+ devices present in the kernel. This layer will have the generic
+ API by which phy drivers can create PHY using the phy framework and
+ phy users can obtain reference to the PHY.
Again, please reverse this. The drivers that use it should select it,
not depend on it, which will then enable this option. I will never know
if I need to enable it, and based on your follow-on patches, if I don't,
drivers that were working just fine, now disappeared from my build,
which isn't nice, and a pain to notice and fix up.
ok.
+/**
+ * phy_create() - create a new phy
+ * @dev: device that is creating the new phy
+ * @id: id of the phy
+ * @ops: function pointers for performing phy operations
+ * @label: label given to the phy
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
+const char *label)
+{
+int ret;
+struct phy *phy;
+
+if (!dev) {
+dev_WARN(dev, no device provided for PHY\n);
+ret = -EINVAL;
+goto err0;
+}
+
+phy = kzalloc(sizeof(*phy), GFP_KERNEL);
+if (!phy) {
+ret = -ENOMEM;
+goto err0;
+}
+
+device_initialize(phy-dev);
+mutex_init(phy-mutex);
+
+phy-dev.class = phy_class;
+phy-dev.parent = dev;
+phy-dev.of_node = dev-of_node;
+phy-id = id;
+phy-ops = ops;
+phy-label = kstrdup(label, GFP_KERNEL);
+
+ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
Your naming is odd, no phy anywhere in it? You rely on the sender to
never send a duplicate name.id pair? Why not create your own ids based
on the number of phys in the system, like almost all other classes and
subsystems do?
hmm.. some PHY drivers use the id they provide to perform some of their
internal operation as in [1] (This is used only if a single PHY provider
implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO
to give the PHY drivers an option to use auto id.
[1] -
http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
+static inline int phy_pm_runtime_get(struct phy *phy)
+{
+if (WARN(IS_ERR(phy), Invalid PHY reference\n))
+return -EINVAL;
Why would phy ever not be valid and a error pointer? And why dump the
stack if that happens, that seems really extreme.
hmm.. there might be cases where the same controller in one soc needs PHY
control and in some other soc does not need PHY control. In such cases, we
might get error pointer here.
I'll change WARN to dev_err.
+
+if (!pm_runtime_enabled(phy-dev))
+return -ENOTSUPP;
+
+return pm_runtime_get(phy-dev);
+}
This, and the other inline functions in this .h file seem huge, why are
they inline and not in the .c file? There's no speed issues, and it
should save space overall in the .c file. Please move them.
ok
+static inline int phy_init(struct phy *phy)
+{
+int ret;
+
+ret = phy_pm_runtime_get_sync(phy);
+if (ret 0 ret != -ENOTSUPP)
+return ret;
+
+mutex_lock(phy-mutex);
+if (phy-init_count++ == 0 phy-ops-init) {
+ret = phy-ops-init(phy);
+if (ret 0) {
+dev_err(phy-dev, phy init failed -- %d\n, ret);
+goto out;
+}
+}
+
+out:
+mutex_unlock(phy-mutex);
+phy_pm_runtime_put(phy);
+return ret;
+}
+
+static inline int phy_exit(struct phy *phy)
+{
+int ret;
+
+ret = phy_pm_runtime_get_sync(phy);
+if (ret 0 ret != -ENOTSUPP)
Re: [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework
On Thursday 18 July 2013 12:51 PM, Greg KH wrote:
On Thu, Jul 18, 2013 at 12:16:11PM +0530, Kishon Vijay Abraham I wrote:
Used the generic PHY framework API to create the PHY. Now the power off and
power on are done in omap_usb_power_off and omap_usb_power_on respectively.
However using the old USB PHY library cannot be completely removed
because OTG is intertwined with PHY and moving to the new framework
will break OTG. Once we have a separate OTG state machine, we
can get rid of the USB PHY library.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
drivers/usb/phy/Kconfig |1 +
drivers/usb/phy/phy-omap-usb2.c | 45
+++
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 3622fff..cc55993 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
config OMAP_USB2
tristate OMAP USB2 PHY Driver
depends on ARCH_OMAP2PLUS
+depends on GENERIC_PHY
select OMAP_CONTROL_USB
help
Enable this to support the transceiver that is part of SOC. This
diff --git a/drivers/usb/phy/phy-omap-usb2.c
b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..751b30f 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
#include linux/pm_runtime.h
#include linux/delay.h
#include linux/usb/omap_control_usb.h
+#include linux/phy/phy.h
/**
* omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int
suspend)
return 0;
}
+static int omap_usb_power_off(struct phy *x)
+{
+struct omap_usb *phy = phy_get_drvdata(x);
+
+omap_control_usb_phy_power(phy-control_dev, 0);
+
+return 0;
+}
+
+static int omap_usb_power_on(struct phy *x)
+{
+struct omap_usb *phy = phy_get_drvdata(x);
+
+omap_control_usb_phy_power(phy-control_dev, 1);
+
+return 0;
+}
+
+static struct phy_ops ops = {
+.power_on = omap_usb_power_on,
+.power_off = omap_usb_power_off,
+.owner = THIS_MODULE,
+};
+
static int omap_usb2_probe(struct platform_device *pdev)
{
struct omap_usb *phy;
+struct phy *generic_phy;
struct usb_otg *otg;
+struct phy_provider *phy_provider;
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-phy.otg= otg;
phy-phy.type = USB_PHY_TYPE_USB2;
+phy_provider = devm_of_phy_provider_register(phy-dev,
+of_phy_simple_xlate);
+if (IS_ERR(phy_provider))
+return PTR_ERR(phy_provider);
+
phy-control_dev = omap_get_control_dev();
if (IS_ERR(phy-control_dev)) {
dev_dbg(pdev-dev, Failed to get control device\n);
@@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
otg-start_srp = omap_usb_start_srp;
otg-phy= phy-phy;
+platform_set_drvdata(pdev, phy);
+pm_runtime_enable(phy-dev);
+
+generic_phy = devm_phy_create(phy-dev, 0, ops, omap-usb2);
+if (IS_ERR(generic_phy))
+return PTR_ERR(generic_phy);
So, if I have two of these controllers in my system, I can't create the
second phy because the name for it will be identical to the first?
That's why the phy core should handle the id, and not rely on the
drivers to set it, as they have no idea how many they have in the
system.
hmm.. for such cases I'll have something like PLATFORM_DEVID_AUTO.
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] ARM: omap2: fix musb usage for n8x0
On 07/16/2013 08:52 PM, Daniel Mack wrote:
Commit b7e2e75a8c (usb: gadget: drop unused USB_GADGET_MUSB_HDRC)
dropped a config symbol that was unused by the musb core, but it turns
out that board support code had references to it.
As the core now has a fall-back to host-only mode if support for
dual-role is not compiled in, so we can just pass MUSB_OTG as
mode from board files.
Signed-off-by: Daniel Mack zon...@gmail.com
Reported-and-tested-by: Aaro Koskinen aaro.koski...@iki.fi
I'm testing musb as OTG on beagleboard (old one, not Beagle-xm). And
using the latest kernel.org version with this patch applied I see the
following messages while booting (repeatedly):
[4.998168] usb usb1: bus auto-suspend, wakeup 1
[5.003112] musb_bus_suspend 2457: trying to suspend as b_idle while active
[5.010498] usb usb1: bus suspend fail, err -16
[5.015289] hub 1-0:1.0: hub_resume
[5.019073] hub 1-0:1.0: state 7 ports 1 chg evt
[5.024963] hub 1-0:1.0: hub_suspend
[5.028778] usb usb1: bus auto-suspend, wakeup 1
...
This is without a cable connected to the OTG port.
Any ideas what might be missing here?
BTW: I enabled USB support for beagle in the dts this way:
+usb_otg_hs {
+ interface-type = 0;
+ usb-phy = usb2_phy;
+ mode = 3;
+ power = 50;
+};
Thanks,
Stefan
--
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] ARM: omap2: fix musb usage for n8x0
Hi, On Thursday 18 July 2013 02:44 PM, Stefan Roese wrote: On 07/16/2013 08:52 PM, Daniel Mack wrote: Commit b7e2e75a8c (usb: gadget: drop unused USB_GADGET_MUSB_HDRC) dropped a config symbol that was unused by the musb core, but it turns out that board support code had references to it. As the core now has a fall-back to host-only mode if support for dual-role is not compiled in, so we can just pass MUSB_OTG as mode from board files. Signed-off-by: Daniel Mack zon...@gmail.com Reported-and-tested-by: Aaro Koskinen aaro.koski...@iki.fi I'm testing musb as OTG on beagleboard (old one, not Beagle-xm). And using the latest kernel.org version with this patch applied I see the following messages while booting (repeatedly): [4.998168] usb usb1: bus auto-suspend, wakeup 1 [5.003112] musb_bus_suspend 2457: trying to suspend as b_idle while active [5.010498] usb usb1: bus suspend fail, err -16 [5.015289] hub 1-0:1.0: hub_resume [5.019073] hub 1-0:1.0: state 7 ports 1 chg evt [5.024963] hub 1-0:1.0: hub_suspend [5.028778] usb usb1: bus auto-suspend, wakeup 1 ... This is without a cable connected to the OTG port. Any ideas what might be missing here? Even I observed these prints when I have dual mode enabled. When kept as gadget only mode I dint see these prints. However if you connect a cable, you should still see that enumeration should succeed. Not sure why those prints come though :-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: [PATCH] usb: udc: add gadget state kobject uevent
Hi Felipe,
Thanks, I'll test the patch.
But sysfs_notify(gadget-dev.kobj, NULL, status), status or state ?
I notice that DEVICE_ATTR(state, S_IRUGO, usb_gadget_state_show, NULL)
On Thu, Jul 18, 2013 at 4:40 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Thu, Jul 18, 2013 at 04:33:43PM +0800, Rong Wang wrote:
On Wed, Jul 17, 2013 at 9:27 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Jul 17, 2013 at 09:04:54PM +0800, Rong Wang wrote:
Hi Felipe,
On Wed, Jul 17, 2013 at 3:57 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Mon, Jul 15, 2013 at 11:31:17PM -0700, Greg KH wrote:
The question is since we default GADGET, so the g_mass_storage.ko is
installed early but connecting to a host PC
is randomly, But the udev has no idea when a host PC connects our
device.
So we consider it's reasonable to let the udev know the GADGET
device state.
Is there any alternative to our question?
I thought we already export events for gadget device states, have you
looked for them? I can't dig through the code at the moment, but this
seems like a pretty common issue...
Felipe, any ideas?
we already expose that in sysfs. IIRC udev can act on sysfs changes,
no ?
I do not know if udev can polling sysfs file content change. I'll study
this.
But the change is triggered by calling usb_gadget_set_state, and I find
composite framework do not call this. Then we should do this common work
in every udc driver?
yes. Only the UDC driver knows when the controller is moving among those
states.
OK. I got that.
But I think it may be more reasonable for the udc driver to maintain a
work queue
to handle the state change since this operation mostly happen in ISR ?
And that's the patch I want to test. Adding a workqueue to every single
UDC will be too much, so I tried to hide it in udc-core.c. I agree with
you we need to pass the sysfs_notification to a separate workqueue
though. If you can test the patch below, that would be great.
commit d32521bd775d48b600e67f23d363d70f71597ffd
Author: Felipe Balbi ba...@ti.com
Date: Wed Jul 17 11:09:49 2013 +0300
usb: gadget: udc-core: move sysfs_notify() to a workqueue
usb_gadget_set_state() will call sysfs_notify()
which might sleep. Some users might want to call
usb_gadget_set_state() from the very IRQ handler
which actually changes the gadget state.
Instead of having every UDC driver add their own
workqueue for such a simple notification, we're
adding it generically to our struct usb_gadget,
so the details are hidden from all UDC drivers.
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..b0d91b1 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -23,6 +23,7 @@
#include linux/list.h
#include linux/err.h
#include linux/dma-mapping.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* -
*/
+static void usb_gadget_state_work(struct work_struct *work)
+{
+ struct usb_gadget *gadget = work_to_gadget(work);
+
+ sysfs_notify(gadget-dev.kobj, NULL, status);
+}
+
void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ schedule_work(gadget-work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
@@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err1;
dev_set_name(gadget-dev, gadget);
+ INIT_WORK(gadget-work, usb_gadget_state_work);
gadget-dev.parent = parent;
dma_set_coherent_mask(gadget-dev, parent-coherent_dma_mask);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index f1b0dca..942ef5e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -22,6 +22,7 @@
#include linux/slab.h
#include linux/scatterlist.h
#include linux/types.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
struct usb_ep;
@@ -475,6 +476,7 @@ struct usb_gadget_ops {
/**
* struct usb_gadget - represents a usb slave device
+ * @work: (internal use) Workqueue to be used for sysfs_notify()
* @ops: Function pointers used to access hardware-specific operations.
* @ep0: Endpoint zero, used when reading or writing responses to
* driver setup() requests
@@ -520,6 +522,7 @@ struct usb_gadget_ops {
* device is acting as a B-Peripheral (so is_a_peripheral is false).
*/
struct usb_gadget {
+ struct work_struct work;
/* readonly to gadget driver */
const struct usb_gadget_ops *ops;
Re: [PATCH] ARM: omap2: fix musb usage for n8x0
On 07/18/2013 11:18 AM, Kishon Vijay Abraham I wrote: I'm testing musb as OTG on beagleboard (old one, not Beagle-xm). And using the latest kernel.org version with this patch applied I see the following messages while booting (repeatedly): [4.998168] usb usb1: bus auto-suspend, wakeup 1 [5.003112] musb_bus_suspend 2457: trying to suspend as b_idle while active [5.010498] usb usb1: bus suspend fail, err -16 [5.015289] hub 1-0:1.0: hub_resume [5.019073] hub 1-0:1.0: state 7 ports 1 chg evt [5.024963] hub 1-0:1.0: hub_suspend [5.028778] usb usb1: bus auto-suspend, wakeup 1 ... This is without a cable connected to the OTG port. Any ideas what might be missing here? Even I observed these prints when I have dual mode enabled. When kept as gadget only mode I dint see these prints. Yes. With gadget-only I don't see those messages. Thanks for the hint. However if you connect a cable, you should still see that enumeration should succeed. Not sure why those prints come though :-s No. When configured as dual-role these endless messages are still there with a cable connected to the PC USB host port (musb gadget mode). Thanks, Stefan -- 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] musb: don't reset endpoint data toggle on blackfin
Reset endpoint data toggle would lead to failure for musb RTL version 1.9 on blackfin. Signed-off-by: Scott Jiang scott.jiang.li...@gmail.com --- drivers/usb/musb/musb_host.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index a9695f5..0bc59fd 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -590,7 +590,10 @@ musb_rx_reinit(struct musb *musb, struct musb_qh *qh, struct musb_hw_ep *ep) WARNING(rx%d, packet/%d ready?\n, ep-epnum, musb_readw(ep-regs, MUSB_RXCOUNT)); - musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG); + if (csr MUSB_TXCSR_MODE) + musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG); + else + musb_h_flush_rxfifo(ep, 0); } /* target addr and (for multipoint) hub addr/port */ @@ -786,7 +789,7 @@ static void musb_ep_program(struct musb *musb, u8 epnum, if (usb_gettoggle(urb-dev, qh-epnum, 1)) csr |= MUSB_TXCSR_H_WR_DATATOGGLE | MUSB_TXCSR_H_DATATOGGLE; - else + else if (csr MUSB_TXCSR_MODE) csr |= MUSB_TXCSR_CLRDATATOG; } -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] EHCI: add to memory barrier to updating hw_next
On Thu, Jul 18, 2013 at 9:30 AM, Gioh Kim gioh@lge.com wrote: Thanks for your reply. -Original Message- From: Ming Lei [mailto:tom.leim...@gmail.com] Sent: Wednesday, July 17, 2013 5:52 PM To: 김기오 Cc: Alan Stern; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Mark Salter; namhyung@lge.com; Minchan Kim; Chanho Min; Jong-Sung Kim; linux-arm-kernel Subject: Re: [PATCH] [RFC] EHCI: add to memory barrier to updating hw_next Cc: ARM list On Wed, Jul 17, 2013 at 1:03 PM, 김기오 gioh@lge.com wrote: Hi, I have a missing urb completion problem on ARMv7 based platform. I thought the above problem was caused by coherent memory between the EHCI device and CPU so I tryied to allocates device type memory for EHCI via dma_declare_coherent_memory at machine initialization step so that EHCI always allocates from those device type memory. It seems to solve the issue because I didn't see any problem. But I am not sure it is acceptable solution. So I applied the patch https://lkml.org/lkml/2011/8/31/344. But it could not solve the problem so that I added another wmb() as my patch, and now my platform works fine. I remember the previous problem reported on Pandaboard firstly was fixed by Will's commit 11ed0ba1(ARM: 7161/1: errata: no automatic store buffer drain), so could you try to enable PL310_ERRATA_769419 and see if your problem can be fixed? I also know the errata but it didn't work for my platform. I am not sure what's the exact problem and what wmb I added could solve but I just think the problem is related to store buffer flush of hw_next. Yes, per the above commit, but it might be one hardware problem. Anyway, important thing is that it fixed my problem so I expect you expert guys could find what I am missing and a right solution. IMHO, the patch might miss updating hw_next pointer. Am I correct? I understand the wmb() is just memory barrier, not write-buffer flush. But it is true that wmb() can flush write buffer in ARM. Anyhow I think that memory type, normal memory, non-cacheable, may have a problem for some devices that needs device type memory. Currently on ARMv7 DMA coherent buffer has to be bufferable, in theory we might need one API to flush CPU write buffer, as described in Documentation/DMA-API-HOWTO.txt: Also, on some platforms your driver may need to flush CPU write buffers in much the same way as it needs to flush write buffers found in PCI bridges (such as by reading a register's value after writing it). But actually on hardware without the problem(such as A15), I don't see any effect without flushing store buffer explicitly. The problem of my platform is occurring when it has very heavy traffic such as HD video streaming service or very many small images display. Could you describe your test case in a bit detail? Which USB applications/drivers may trigger your URB completion miss problem? Under what kind of video streaming service(USB video or only kind of heavy load)? I guess that HC could have a use-after-free problem like following situation. 1. A qtd which is not at the queue head should be removed in qh_completions(). 2. The last-hw_next become be pointing at the next qtd but the hw_next value is delayed in write-buffer. 3. The qtd is removed in the list. 4. The qtd is freed into DMA pool and re-allocated for another urb. 5. HC try to process last-hw_next and it is pointing re-allocated qtd. What do you think about it? Is it possible? I understand it might not be possible because: when 'stopped' is set, that said the HC might not advance the queue. But I don't understand why 'last-hw_next' is patched here under 'stopped' situation. Even the 'stopped' case may be seldom triggered, do you know under which condition the stopped is triggered in your problem?(stall, short read or others) Thanks, -- Ming Lei -- 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: udc-core: move sysfs_notify() to a workqueue
usb_gadget_set_state() will call sysfs_notify()
which might sleep. Some users might want to call
usb_gadget_set_state() from the very IRQ handler
which actually changes the gadget state.
Instead of having every UDC driver add their own
workqueue for such a simple notification, we're
adding it generically to our struct usb_gadget,
so the details are hidden from all UDC drivers.
Signed-off-by: Felipe Balbi ba...@ti.com
---
Tested on OMAP5 uEVM.
drivers/usb/gadget/udc-core.c | 11 ++-
include/linux/usb/gadget.h| 4
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..b0d91b1 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -23,6 +23,7 @@
#include linux/list.h
#include linux/err.h
#include linux/dma-mapping.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* - */
+static void usb_gadget_state_work(struct work_struct *work)
+{
+ struct usb_gadget *gadget = work_to_gadget(work);
+
+ sysfs_notify(gadget-dev.kobj, NULL, status);
+}
+
void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ schedule_work(gadget-work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
@@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err1;
dev_set_name(gadget-dev, gadget);
+ INIT_WORK(gadget-work, usb_gadget_state_work);
gadget-dev.parent = parent;
dma_set_coherent_mask(gadget-dev, parent-coherent_dma_mask);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index f1b0dca..942ef5e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -22,6 +22,7 @@
#include linux/slab.h
#include linux/scatterlist.h
#include linux/types.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
struct usb_ep;
@@ -475,6 +476,7 @@ struct usb_gadget_ops {
/**
* struct usb_gadget - represents a usb slave device
+ * @work: (internal use) Workqueue to be used for sysfs_notify()
* @ops: Function pointers used to access hardware-specific operations.
* @ep0: Endpoint zero, used when reading or writing responses to
* driver setup() requests
@@ -520,6 +522,7 @@ struct usb_gadget_ops {
* device is acting as a B-Peripheral (so is_a_peripheral is false).
*/
struct usb_gadget {
+ struct work_struct work;
/* readonly to gadget driver */
const struct usb_gadget_ops *ops;
struct usb_ep *ep0;
@@ -538,6 +541,7 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
};
+#define work_to_gadget(w) (container_of((w), struct usb_gadget, work))
static inline void set_gadget_data(struct usb_gadget *gadget, void *data)
{ dev_set_drvdata(gadget-dev, data); }
--
1.8.3.3.754.g9c3c367
--
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: udc: add gadget state kobject uevent
Hi,
On Thu, Jul 18, 2013 at 11:40:41AM +0300, Felipe Balbi wrote:
The question is since we default GADGET, so the g_mass_storage.ko is
installed early but connecting to a host PC
is randomly, But the udev has no idea when a host PC connects our
device.
So we consider it's reasonable to let the udev know the GADGET
device state.
Is there any alternative to our question?
I thought we already export events for gadget device states, have you
looked for them? I can't dig through the code at the moment, but this
seems like a pretty common issue...
Felipe, any ideas?
we already expose that in sysfs. IIRC udev can act on sysfs changes,
no ?
I do not know if udev can polling sysfs file content change. I'll study
this.
But the change is triggered by calling usb_gadget_set_state, and I find
composite framework do not call this. Then we should do this common work
in every udc driver?
yes. Only the UDC driver knows when the controller is moving among those
states.
OK. I got that.
But I think it may be more reasonable for the udc driver to maintain a
work queue
to handle the state change since this operation mostly happen in ISR ?
And that's the patch I want to test. Adding a workqueue to every single
UDC will be too much, so I tried to hide it in udc-core.c. I agree with
you we need to pass the sysfs_notification to a separate workqueue
though. If you can test the patch below, that would be great.
commit d32521bd775d48b600e67f23d363d70f71597ffd
Author: Felipe Balbi ba...@ti.com
Date: Wed Jul 17 11:09:49 2013 +0300
usb: gadget: udc-core: move sysfs_notify() to a workqueue
usb_gadget_set_state() will call sysfs_notify()
which might sleep. Some users might want to call
usb_gadget_set_state() from the very IRQ handler
which actually changes the gadget state.
Instead of having every UDC driver add their own
workqueue for such a simple notification, we're
adding it generically to our struct usb_gadget,
so the details are hidden from all UDC drivers.
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..b0d91b1 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -23,6 +23,7 @@
#include linux/list.h
#include linux/err.h
#include linux/dma-mapping.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* -
*/
+static void usb_gadget_state_work(struct work_struct *work)
+{
+ struct usb_gadget *gadget = work_to_gadget(work);
+
+ sysfs_notify(gadget-dev.kobj, NULL, status);
+}
+
void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ schedule_work(gadget-work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
@@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err1;
dev_set_name(gadget-dev, gadget);
+ INIT_WORK(gadget-work, usb_gadget_state_work);
gadget-dev.parent = parent;
dma_set_coherent_mask(gadget-dev, parent-coherent_dma_mask);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index f1b0dca..942ef5e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -22,6 +22,7 @@
#include linux/slab.h
#include linux/scatterlist.h
#include linux/types.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
struct usb_ep;
@@ -475,6 +476,7 @@ struct usb_gadget_ops {
/**
* struct usb_gadget - represents a usb slave device
+ * @work: (internal use) Workqueue to be used for sysfs_notify()
* @ops: Function pointers used to access hardware-specific operations.
* @ep0: Endpoint zero, used when reading or writing responses to
* driver setup() requests
@@ -520,6 +522,7 @@ struct usb_gadget_ops {
* device is acting as a B-Peripheral (so is_a_peripheral is false).
*/
struct usb_gadget {
+ struct work_struct work;
/* readonly to gadget driver */
const struct usb_gadget_ops *ops;
struct usb_ep *ep0;
@@ -538,6 +541,7 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
};
+#define work_to_gadget(w)(container_of((w), struct usb_gadget, work))
static inline void set_gadget_data(struct usb_gadget *gadget, void *data)
{ dev_set_drvdata(gadget-dev, data); }
nevermind, colleague borrowed me his omap5 board. It's tested, will send
a proper patch now.
--
balbi
Re: [PATCH] usb: udc: add gadget state kobject uevent
On Thu, Jul 18, 2013 at 05:28:19PM +0800, Rong Wang wrote: Hi Felipe, Thanks, I'll test the patch. But sysfs_notify(gadget-dev.kobj, NULL, status), status or state ? I notice that DEVICE_ATTR(state, S_IRUGO, usb_gadget_state_show, NULL) good eyes, please send a patch which I'll queue on this -rc and Cc: sta...@vger.kernel.org. I'll rewrite my patch on top of the patched -rc later. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: udc: add gadget state kobject uevent
Hi Felipe,
Here's the patch. If you are OK with it, I'll send it to the list formally.
Thanks.
-
usb: gadget: udc-core: make udc state attribute name consistent
The name of udc state attribute file under sysfs is
registered as state, while usb_gadget_set_state
take it as status when it's going to update.
Here it is made consistent as state.
Signed-off-by: Rong Wang rong.w...@csr.com
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..5514822 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -105,7 +105,7 @@ void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ sysfs_notify(gadget-dev.kobj, NULL, state);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
On Thu, Jul 18, 2013 at 6:10 PM, Felipe Balbi ba...@ti.com wrote:
On Thu, Jul 18, 2013 at 05:28:19PM +0800, Rong Wang wrote:
Hi Felipe,
Thanks, I'll test the patch.
But sysfs_notify(gadget-dev.kobj, NULL, status), status or state ?
I notice that DEVICE_ATTR(state, S_IRUGO, usb_gadget_state_show, NULL)
good eyes, please send a patch which I'll queue on this -rc and Cc:
sta...@vger.kernel.org.
I'll rewrite my patch on top of the patched -rc later.
--
balbi
--
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: udc: add gadget state kobject uevent
On Thu, Jul 18, 2013 at 06:54:38PM +0800, Rong Wang wrote:
Hi Felipe,
Here's the patch. If you are OK with it, I'll send it to the list formally.
Thanks.
-
usb: gadget: udc-core: make udc state attribute name consistent
The name of udc state attribute file under sysfs is
registered as state, while usb_gadget_set_state
take it as status when it's going to update.
Here it is made consistent as state.
Signed-off-by: Rong Wang rong.w...@csr.com
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..5514822 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -105,7 +105,7 @@ void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ sysfs_notify(gadget-dev.kobj, NULL, state);
just fix the indentation and prevent gmail from mangling the tabs into
spaces, you're good to go :-)
--
balbi
signature.asc
Description: Digital signature
Re: [RFC] ux500 dma short transfers on MUSB
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. On 07/11/2013 06:58 PM, Greg KH wrote: Now, the way I understand it is, you tell musb that the complete transfer of 256 bytes has ended instead one byte that really happened. Is my assumption wrong? What do you mean by tell musb? Of course the transfer has completed, that's all the device sent to the host controller, so it has to complete the transfer and send that on up to the driver that requested the urb. I don't understand the question/problem you are asking here, care to be more descriptive? Okay. musb offloads the actual transfer to the DMA engine it is using. Once it does so, it relies on whatever comes back from dma engine regarding transfer complete, transferred size etc. In case of ux500-dma (as far as I can tell) musb forwards the RX request to the DMA engine, which will receive one byte instead of the requested 256bytes. Since the DMA engine did not inform musb about the correct transfer size, musb will complete that URB with 256 bytes. If you take a look on ux500_dma_callback() you will see the line: ux500_channel-channel.actual_len = ux500_channel-cur_len; -actual_len is what musb thinks got transferred. -cur_len is what musb asked to transfer. I don't see where the case of a shorter transfer is considered. Again I have no HW I was just browsing. [0] http://www.mail-archive.com/linux-usb@vger.kernel.org/msg24190.html 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] ARM: omap2: fix musb usage for n8x0
On 18.07.2013 11:40, Stefan Roese wrote: On 07/18/2013 11:18 AM, Kishon Vijay Abraham I wrote: I'm testing musb as OTG on beagleboard (old one, not Beagle-xm). And using the latest kernel.org version with this patch applied I see the following messages while booting (repeatedly): [4.998168] usb usb1: bus auto-suspend, wakeup 1 [5.003112] musb_bus_suspend 2457: trying to suspend as b_idle while active [5.010498] usb usb1: bus suspend fail, err -16 [5.015289] hub 1-0:1.0: hub_resume [5.019073] hub 1-0:1.0: state 7 ports 1 chg evt [5.024963] hub 1-0:1.0: hub_suspend [5.028778] usb usb1: bus auto-suspend, wakeup 1 ... This is without a cable connected to the OTG port. Any ideas what might be missing here? Even I observed these prints when I have dual mode enabled. When kept as gadget only mode I dint see these prints. Yes. With gadget-only I don't see those messages. Thanks for the hint. So in which mode does your port operate then? Gadget or dual-role? And I take it this does not happen with 3.10? Unfortunately, I have no board here anymore with such a port in dual-role mode ... 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] ARM: omap2: fix musb usage for n8x0
On 07/18/2013 02:02 PM, Daniel Mack wrote: On 18.07.2013 11:40, Stefan Roese wrote: On 07/18/2013 11:18 AM, Kishon Vijay Abraham I wrote: I'm testing musb as OTG on beagleboard (old one, not Beagle-xm). And using the latest kernel.org version with this patch applied I see the following messages while booting (repeatedly): [4.998168] usb usb1: bus auto-suspend, wakeup 1 [5.003112] musb_bus_suspend 2457: trying to suspend as b_idle while active [5.010498] usb usb1: bus suspend fail, err -16 [5.015289] hub 1-0:1.0: hub_resume [5.019073] hub 1-0:1.0: state 7 ports 1 chg evt [5.024963] hub 1-0:1.0: hub_suspend [5.028778] usb usb1: bus auto-suspend, wakeup 1 ... This is without a cable connected to the OTG port. Any ideas what might be missing here? Even I observed these prints when I have dual mode enabled. When kept as gadget only mode I dint see these prints. Yes. With gadget-only I don't see those messages. Thanks for the hint. So in which mode does your port operate then? Gadget or dual-role? Its the original Beagleboard (revision C2), so I suppose its dual-role. Or am I mistaken here? And I take it this does not happen with 3.10? I just checked v3.10 as again. No, I'm not seeing these messages here. Thanks, Stefan -- 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: usb-skeleton.c: add retry for nonblocking read
From: Chen Wang unicorn_w...@outlook.com
Updated skel_read() in usb-skeleton.c. When there is no data in the buffer, we
would allow retry for both blocking and nonblocking cases. Original logic give
retry only for blocking case. Actually we can also allow retry for nonblocking
case. This will reuse the existing retry logic and handle the return of -EAGAIN
in one place. Also if the data to be read is short and can be retrieved in
quick time, we can also give a chance for nonblocking case and may catch the
data and copy it back to userspace in one read() call too.
Signed-off-by: Chen Wang unicorn_w...@outlook.com
---
--- linux-3.11-rc1/drivers/usb/usb-skeleton.c.orig 2013-07-18
19:35:23.559780152 +0800
+++ linux-3.11-rc1/drivers/usb/usb-skeleton.c 2013-07-18 19:38:11.546779516
+0800
@@ -325,9 +325,8 @@ retry:
rv = skel_do_read_io(dev, count);
if (rv 0)
goto exit;
- else if (!(file-f_flags O_NONBLOCK))
+ else
goto retry;
- rv = -EAGAIN;
}
exit:
mutex_unlock(dev-io_mutex);
N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{焙柒��^n�r■�z���h�ㄨ��Ⅷ �G���h�(�茛j���m赇z罐��帼f＂�h���~�m�
RE: [PATCH]USB: usb-skeleton.c: add retry for nonblocking read
Sorry, please ignore this email, I find some tabs are lost after my paste and copy, sorry for this. I will send another email. From: unicorn_w...@outlook.com To: linux-usb@vger.kernel.org CC: oneu...@suse.de; gre...@linuxfoundation.org Subject: [PATCH]USB: usb-skeleton.c: add retry for nonblocking read Date: Thu, 18 Jul 2013 12:54:04 + From: Chen Wang unicorn_w...@outlook.com Updated skel_read() in usb-skeleton.c. When there is no data in the buffer, we would allow retry for both blocking and nonblocking cases. Original logic give retry only for blocking case. Actually we can also allow retry for nonblocking case. This will reuse the existing retry logic and handle the return of -EAGAIN in one place. Also if the data to be read is short and can be retrieved in quick time, we can also give a chance for nonblocking case and may catch the data and copy it back to userspace in one read() call too. Signed-off-by: Chen Wang unicorn_w...@outlook.com --- --- linux-3.11-rc1/drivers/usb/usb-skeleton.c.orig 2013-07-18 19:35:23.559780152 +0800 +++ linux-3.11-rc1/drivers/usb/usb-skeleton.c 2013-07-18 19:38:11.546779516 +0800 @@ -325,9 +325,8 @@ retry: rv = skel_do_read_io(dev, count); if (rv 0) goto exit; - else if (!(file-f_flags O_NONBLOCK)) + else goto retry; - rv = -EAGAIN; } exit: mutex_unlock(dev-io_mutex);
[PATCH]USB: usb-skeleton: add retry for nonblocking read
From: Chen Wang unicorn_w...@outlook.com Updated skel_read() in usb-skeleton.c. When there is no data in the buffer, we would allow retry for both blocking and nonblocking cases. Original logic gives retry only for blocking case. Actually we can also allow retry for nonblocking case. This will reuse the existing retry logic and handle the return of -EAGAIN in one place. Also if the data to be read is short and can be retrieved in quick time, we can also give a chance for nonblocking case and may catch the data and copy it back to userspace in one read() call too. Signed-off-by: Chen Wang unicorn_w...@outlook.com --- --- linux-3.11-rc1/drivers/usb/usb-skeleton.c.orig 2013-07-18 19:35:23.559780152 +0800 +++ linux-3.11-rc1/drivers/usb/usb-skeleton.c 2013-07-18 19:38:11.546779516 +0800 @@ -325,9 +325,8 @@ retry: rv = skel_do_read_io(dev, count); if (rv 0) goto exit; - else if (!(file-f_flags O_NONBLOCK)) + else goto retry; - rv = -EAGAIN; } exit: mutex_unlock(dev-io_mutex); -- 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: usb-skeleton: add retry for nonblocking read
Sorry, I find some tabs are still missed even I sent out as plain txt by outlook. Please ignore this again, I will try to resolve this before submit it. From: unicorn_w...@outlook.com To: linux-usb@vger.kernel.org CC: oneu...@suse.de; gre...@linuxfoundation.org; unicorn_w...@outlook.com Subject: [PATCH]USB: usb-skeleton: add retry for nonblocking read Date: Thu, 18 Jul 2013 13:09:20 + From: Chen Wang unicorn_w...@outlook.com Updated skel_read() in usb-skeleton.c. When there is no data in the buffer, we would allow retry for both blocking and nonblocking cases. Original logic gives retry only for blocking case. Actually we can also allow retry for nonblocking case. This will reuse the existing retry logic and handle the return of -EAGAIN in one place. Also if the data to be read is short and can be retrieved in quick time, we can also give a chance for nonblocking case and may catch the data and copy it back to userspace in one read() call too. Signed-off-by: Chen Wang unicorn_w...@outlook.com --- --- linux-3.11-rc1/drivers/usb/usb-skeleton.c.orig 2013-07-18 19:35:23.559780152 +0800 +++ linux-3.11-rc1/drivers/usb/usb-skeleton.c 2013-07-18 19:38:11.546779516 +0800 @@ -325,9 +325,8 @@ retry: rv = skel_do_read_io(dev, count); if (rv 0) goto exit; - else if (!(file-f_flags O_NONBLOCK)) + else goto retry; - rv = -EAGAIN; } exit: mutex_unlock(dev-io_mutex);
Re: [PATCH] usb: udc: add gadget state kobject uevent
On Thu, 18 Jul 2013, Felipe Balbi wrote:
yes. Only the UDC driver knows when the controller is moving among those
states.
Not quite. Only the gadget driver knows when the transition between
ADDRESS and CONFIGURED occurs. This should be added to composite.c.
that's not entirely true :-) See how we handle that in dwc3:
| static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest
*ctrl)
| {
| enum usb_device_state state = dwc-gadget.state;
| u32 cfg;
| int ret;
| u32 reg;
|
| dwc-start_config_issued = false;
| cfg = le16_to_cpu(ctrl-wValue);
|
| switch (state) {
| case USB_STATE_DEFAULT:
| return -EINVAL;
| break;
|
| case USB_STATE_ADDRESS:
| ret = dwc3_ep0_delegate_req(dwc, ctrl);
| /* if the cfg matches and the cfg is non zero */
| if (cfg (!ret || (ret == USB_GADGET_DELAYED_STATUS))) {
| usb_gadget_set_state(dwc-gadget,
| USB_STATE_CONFIGURED);
In theory, this should not occur until the gadget driver has finished
the transition to the CONFIGURED state, which doesn't occur until later
in the case of USB_GADGET_DELAYED_STATUS.
|
| /*
| * Enable transition to U1/U2 state when
| * nothing is pending from application.
| */
| reg = dwc3_readl(dwc-regs, DWC3_DCTL);
| reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA);
| dwc3_writel(dwc-regs, DWC3_DCTL, reg);
|
| dwc-resize_fifos = true;
| dev_dbg(dwc-dev, resize fifos flag SET\n);
| }
| break;
|
| case USB_STATE_CONFIGURED:
| ret = dwc3_ep0_delegate_req(dwc, ctrl);
| if (!cfg)
| usb_gadget_set_state(dwc-gadget,
| USB_STATE_ADDRESS);
No check on the value of ret? What if the request was rejected?
| break;
| default:
| ret = -EINVAL;
| }
| return ret;
| }
This illustrates the folly of exposing your code to public review. :-)
Nevertheless, I take your point.
Also, until other gadget drivers add notifications to the other cases, I
don't think it's wise to add a transition from NOTATTACHED to
CONFIGURED.
Another good point.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: udc-core: move sysfs_notify() to a workqueue
On Thu, 18 Jul 2013, Felipe Balbi wrote:
usb_gadget_set_state() will call sysfs_notify()
which might sleep. Some users might want to call
usb_gadget_set_state() from the very IRQ handler
which actually changes the gadget state.
Instead of having every UDC driver add their own
workqueue for such a simple notification, we're
adding it generically to our struct usb_gadget,
so the details are hidden from all UDC drivers.
Signed-off-by: Felipe Balbi ba...@ti.com
---
Tested on OMAP5 uEVM.
drivers/usb/gadget/udc-core.c | 11 ++-
include/linux/usb/gadget.h| 4
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..b0d91b1 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -23,6 +23,7 @@
#include linux/list.h
#include linux/err.h
#include linux/dma-mapping.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* -
*/
+static void usb_gadget_state_work(struct work_struct *work)
+{
+ struct usb_gadget *gadget = work_to_gadget(work);
+
+ sysfs_notify(gadget-dev.kobj, NULL, status);
+}
+
void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ schedule_work(gadget-work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
@@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err1;
dev_set_name(gadget-dev, gadget);
+ INIT_WORK(gadget-work, usb_gadget_state_work);
gadget-dev.parent = parent;
dma_set_coherent_mask(gadget-dev, parent-coherent_dma_mask);
Deallocation of the gadget structure races with the work routine. You
need to wait for any scheduled work to complete when the gadget is
unregistered.
Also, what happens if two state transitions occur before the work queue
gets around to executing the work routine?
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]USB: usb-skeleton.c: add retry for nonblocking read
-- 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: How should we handle isochronous underruns?
On Thu, 18 Jul 2013, Ming Lei wrote: On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 4 Jul 2013, Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: If so, your coming change may break ABI because as you described that the flag should be set in the first URB of a new stream, but some user-space drivers may not set it before. Even USB audio driver doesn't set it in current -next tree. I had some more ideas about this. Instead of requiring drivers to set URB_ISO_ASAP on just the first URB of an isochronous stream, we could ask drivers to call usb_reset_endpoint() between streams. This would tell the HCD that the next URB marks the start of a new stream, with no need for a special flag. This idea still requires changes from old drivers. Also it might be not appropriate to call usb_reset_endpoint() in this case because other host controller's implementation may do other unnecessary thing for this situation. Perhaps. I doubt that HCDs need to do anything when they reset an isochronous endpoint, but you're right. It's safer to avoid the issue. Another possibility, which would be even simpler, is for HCDs to assume that if the endpoint's queue has been empty for more than 100 ms then a new stream is starting. Then drivers wouldn't have to do anything special at all. (Unless they are stopping and restarting streams very rapidly, in which case something like usb_reset_endpoint() would be In this case, we may use changing altsetting to decide start of streaming. Yes indeed. But that could still require changes to old drivers. To be even more safe, unlinking an URB should mark the end of a stream. That's what snd-usb-audio does when it closes a stream; it kills all the outstanding URBs. necessary.) IMO, this one should be OK, and looks it is a bit similar with unlinking empty interrupt qh. In fact, it's necessary: ehci-hcd doesn't keep track of the state of a stream beyond 512 ms or so. 100 ms might even be overkill. No stream should ever have a gap that large unless something has gone drastically wrong, in which case it doesn't much matter what we do. (Except perhaps for isochronous endpoints with a period of 128 ms or more. I have never heard of such things; have you?) 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: usb-skeleton.c: add retry for nonblocking read
Please ignore this blank email, sorry for my wrong send. On Thu, Jul 18, 2013 at 10:04 PM, Chen Wang unicornxx.w...@gmail.com wrote: -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] EHCI: add to memory barrier to updating hw_next
On Thu, 18 Jul 2013, Ming Lei wrote: I guess that HC could have a use-after-free problem like following situation. 1. A qtd which is not at the queue head should be removed in qh_completions(). 2. The last-hw_next become be pointing at the next qtd but the hw_next value is delayed in write-buffer. 3. The qtd is removed in the list. 4. The qtd is freed into DMA pool and re-allocated for another urb. 5. HC try to process last-hw_next and it is pointing re-allocated qtd. What do you think about it? Is it possible? I understand it might not be possible because: when 'stopped' is set, that said the HC might not advance the queue. But I don't understand why 'last-hw_next' is patched here under 'stopped' situation. It should not be possible. When stopped is set, the QH gets unlinked and relinked before it can start up again. Relinking involves some memory barriers, so the qTD will not be accessed again by the HC. last-hw_next gets patched because the qTD might belong to some URB in the middle of the queue that is being unlinked. The URBs before it and after it will still be active, so the queue link has to be updated. Even the 'stopped' case may be seldom triggered, do you know under which condition the stopped is triggered in your problem?(stall, short read or others) I was going to ask the same question. This particular piece of code gets executed _only_ when an URB is unlinked. Not during any other kind of error. 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
Help r/e gadgetfs bulk xfer problem on Overo gumstix
All, I'm running into problems with the gadgetfs usb.c bulk transport example, and am looking for some advice as to how to approach this problem. I am using the source code found at http://www.linux-usb.org/gadget/usb.c, which I compile for an overo gumstix board. Basically, I use a test program using libusb on linux, and I only ever see zeros reported back from the block of data that I read. I have looked at the data packets sent by the overo board using a hardware usb analyzer, and these only contain zero bytes. I am (unfortunately) locked to using kernel version 2.6.37 on this texas insturments DM3730 processor. My question is: Are there any known problems with gadgetfs or the musb code that may cause these symptoms? Cheers, -Brett -- 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: Help r/e gadgetfs bulk xfer problem on Overo gumstix
On Thu, 18 Jul 2013, Breton M. Saunders wrote: All, I'm running into problems with the gadgetfs usb.c bulk transport example, and am looking for some advice as to how to approach this problem. I am using the source code found at http://www.linux-usb.org/gadget/usb.c, which I compile for an overo gumstix board. Basically, I use a test program using libusb on linux, and I only ever see zeros reported back from the block of data that I read. I have looked at the data packets sent by the overo board using a hardware usb analyzer, and these only contain zero bytes. Do you specify the -p 1 option when starting the gadgetfs program? I am (unfortunately) locked to using kernel version 2.6.37 on this texas insturments DM3730 processor. My question is: Are there any known problems with gadgetfs or the musb code that may cause these symptoms? Not as far as I know. 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]USB: usb-skeleton.c: add retry for nonblocking read
-- 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] usbserial: append Petatel NP10T device to GSM modems list
This patch was tested on 3.10.1 kernel.
Same models of Petatel NP10T modems have different device IDs.
Unfortunately they have no additional revision information on a board
which may treat them as different devices. Currently I've seen only
two NP10T devices with various IDs. Possibly Petatel NP10T list will
be appended upon devices with new IDs will appear.
--- linux-3.10.1/drivers/usb/serial/option.c2013-07-01
01:13:29.0 +0300
+++ linux-3.10.1.np10t/drivers/usb/serial/option.c 2013-07-18
16:15:36.902600324 +0300
@@ -446,7 +446,8 @@ static void option_instat_callback(struc
/* Hyundai Petatel Inc. products */
#define PETATEL_VENDOR_ID 0x1ff4
-#define PETATEL_PRODUCT_NP10T 0x600e
+#define PETATEL_PRODUCT_NP10T_600A 0x600a
+#define PETATEL_PRODUCT_NP10T_600E 0x600e
/* TP-LINK Incorporated products */
#define TPLINK_VENDOR_ID 0x2357
@@ -1333,7 +1334,8 @@ static const struct usb_device_id option
{ USB_DEVICE_AND_INTERFACE_INFO(MEDIATEK_VENDOR_ID,
MEDIATEK_PRODUCT_DC_4COM2, 0xff, 0x02, 0x01) },
{ USB_DEVICE_AND_INTERFACE_INFO(MEDIATEK_VENDOR_ID,
MEDIATEK_PRODUCT_DC_4COM2, 0xff, 0x00, 0x00) },
{ USB_DEVICE(CELLIENT_VENDOR_ID, CELLIENT_PRODUCT_MEN200) },
- { USB_DEVICE(PETATEL_VENDOR_ID, PETATEL_PRODUCT_NP10T) },
+ { USB_DEVICE(PETATEL_VENDOR_ID, PETATEL_PRODUCT_NP10T_600A) },
+ { USB_DEVICE(PETATEL_VENDOR_ID, PETATEL_PRODUCT_NP10T_600E) },
{ USB_DEVICE(TPLINK_VENDOR_ID, TPLINK_PRODUCT_MA180),
.driver_info = (kernel_ulong_t)net_intf4_blacklist },
{ USB_DEVICE(CHANGHONG_VENDOR_ID, CHANGHONG_PRODUCT_CH690) },
Signed-off-by: Daniil Bolsun dan.bol...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/21] usb/gadget: f_mass_storage: split fsg_common initialization into a number of functions
On Wednesday, July 17, 2013 5:44 PM Alan Stern wrote:
On Wed, 17 Jul 2013, Andrzej Pietrasiewicz wrote:
Hello Alan,
Hello.
@@ -179,7 +179,7 @@ EXPORT_SYMBOL(fsg_ss_function); void
fsg_lun_close(struct fsg_lun *curlun) {
if (curlun-filp) {
- LDBG(curlun, close backing file\n);
+ pr_debug(close backing file\n);
Here and in lots of other places, you have replaced a log message
that includes the LUN's name and driver with a message that includes
neither. If somebody reads the system log after this change, how
are they going to know _which_ LUN had its backing file closed?
That information should be included in the message.
I should have been more explicit in the commit log.
My main concern with the LDBG and its friends is that they need to
have a device to operate on.
So far this hasn't been an issue, because the luns have been
represented in sysfs. By the way, using sysfs for this kind of things
could be considered abuse. In a configfs-based gadget the luns are
represented in configfs instead, so there is no device to use for
dev_dbg and similar; the devices are used in the legacy
g_mass_storage.ko, though.
As far as I can tell the only reason each lun contains a struct device
is for it to appear in sysfs, and the only reason for it to appear in
sysfs is to have a file-like attribute for the user to read from/
write to. Now that configfs is taking over this responsibility, there
is no need for struct device. This is what I think, I guess that
Michal's explanation would be authoritative.
@Michal: Could you throw some light on the subject?
That's okay; I originally wrote the code that registers LUNs in sysfs.
You're right; I did it so that there would be file-like attributes for the
user to access. With configfs the device structures aren't needed.
Meanwhile I kept the struct device in struct fsg_lun for the sake of
Compatibility and it should be kept until the g_.ko modules are
dropped altogether (which might be a long time).
Why not just change the definitions of LDBG, LINFO, and so on
instead of changing all the places where they get called?
If lun's device is to be omitted, there is no point in passing luns to
the said macros, which means changing each macro invocation anyway.
However, if a lun were useful in the macro your suggestion is
absolutely right.
I strongly feel that the macros should print out the LUN's name as a
prefix to the rest of the message. Indeed, if there are several instances
of the mass-storage function running (perhaps in multiple interfaces or
attached to multiple UDCs) then the prefix should also indicate the
instance the LUN belongs to.
These names don't have to be device names. But they should be sufficient
to identify the particular LUN the message is about.
You are right, I have an idea, I will post a v2 series soon.
Thanks,
Andrzej
--
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: usb-skeleton.c: add retry for nonblocking read
On Thu, Jul 18, 2013 at 10:34:33PM +0800, Chen Wang wrote: Opps, I find my email address is still wrong. Let me correct it and post again. The mailing list does not accept html messages, and I can't accept a patch in html format. Please read the file, Documentation/email_clients.txt for some hints on how to fix your client to send patches in a format that we can accept them in. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Thu, Jul 18, 2013 at 02:29:52PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 18 July 2013 12:50 PM, Greg KH wrote:
On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
+struct phy_provider *__of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args));
+struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args))
+
+__of_phy_provider_register and __devm_of_phy_provider_register can be
used to
+register the phy_provider and it takes device, owner and of_xlate as
+arguments. For the dt boot case, all PHY providers should use one of the
above
+2 APIs to register the PHY provider.
Why do you have __ for the prefix of a public function? Is that really
the way that OF handles this type of thing?
I have a macro of_phy_provider_register/devm_of_phy_provider_register that
calls these functions and should be used by the PHY drivers. Probably I should
make a mention of it in the Documentation.
Yes, mention those as you never want to be calling __* functions
directly, right?
+ ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
Your naming is odd, no phy anywhere in it? You rely on the sender to
never send a duplicate name.id pair? Why not create your own ids based
on the number of phys in the system, like almost all other classes and
subsystems do?
hmm.. some PHY drivers use the id they provide to perform some of their
internal operation as in [1] (This is used only if a single PHY provider
implements multiple PHYS). Probably I'll add an option like
PLATFORM_DEVID_AUTO
to give the PHY drivers an option to use auto id.
[1] -
http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
No, who cares about the id? No one outside of the phy core ever should,
because you pass back the only pointer that they really do care about,
if they need to do anything with the device. Use that, and then you can
rip out all of the search for a phy by a string logic, as that's not
needed either. Just stick to the pointer, it's easier, and safer that
way.
+static inline int phy_pm_runtime_get(struct phy *phy)
+{
+ if (WARN(IS_ERR(phy), Invalid PHY reference\n))
+ return -EINVAL;
Why would phy ever not be valid and a error pointer? And why dump the
stack if that happens, that seems really extreme.
hmm.. there might be cases where the same controller in one soc needs PHY
control and in some other soc does not need PHY control. In such cases, we
might get error pointer here.
I'll change WARN to dev_err.
I still don't understand. You have control over the code that calls
these functions, just ensure that they pass in a valid pointer, it's
that simple. Or am I missing something?
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 2/6] USB: OHCI: make ohci-omap a separate driver
On Tue, 25 Jun 2013, Manjunath Goudar wrote:
Separate the TI OHCI OMAP1/2 host controller driver from ohci-hcd
host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM;
it would be nice to have in 3.11.
I'm sorry it took so long to review this; I have been very busy.
This patch is almost right. There are just a couple of problems
involving the clock_power calls.
@@ -354,12 +358,6 @@ static int usb_hcd_omap_probe (const struct hc_driver
*driver,
goto err2;
}
- ohci = hcd_to_ohci(hcd);
- ohci_hcd_init(ohci);
-
- host_initialized = 0;
- host_enabled = 1;
-
irq = platform_get_irq(pdev, 0);
if (irq 0) {
retval = -ENXIO;
@@ -369,11 +367,7 @@ static int usb_hcd_omap_probe (const struct hc_driver
*driver,
if (retval)
goto err3;
- host_initialized = 1;
-
- if (!host_enabled)
- omap_ohci_clock_power(0);
-
+ omap_ohci_clock_power(0);
Since host_enabled was always 1 at this point, omap_ohci_clock_power()
would never be called. You should remove it.
@@ -402,6 +396,8 @@ err0:
static inline void
usb_hcd_omap_remove (struct usb_hcd *hcd, struct platform_device *pdev)
{
+ dev_dbg(hcd-self.controller, stopping USB Controller\n);
+ omap_ohci_clock_power(0);
usb_remove_hcd(hcd);
if (!IS_ERR_OR_NULL(hcd-phy)) {
(void) otg_set_host(hcd-phy-otg, 0);
omap_ohci_clock_power() should be called after usb_remove_hcd(), not
before. This reason is simple: Until usb_remove_hcd() is called, the
controller is still running and therefore needs to have a valid clock
signal.
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: question about bus_suspend and bus_resume
On Wed, 17 Jul 2013, Alan Stern wrote: On Wed, 17 Jul 2013, Thomas Pugliese wrote: On Wed, 17 Jul 2013, Alan Stern wrote: On Wed, 17 Jul 2013, Thomas Pugliese wrote: Hi, What is the expected behavior if a host controller does not implement bus_suspend and bus_resume? I am seeing that with the HWA HCD, kworker and khubd peg the CPU at 100% because the kernel is constantly calling hcd_bus_suspend and hcd_bus_resume. The calls to hcd_bus_suspend and hcd_bus_resume fail because hcd-bus_suspend and hcd-bus_resume are NULL. I have reproduced this behaviour using 3.11rc1 all the way back to 3.9.2. I think the expected behavior is close to what you described. :-) Seriously, AFAIK nobody thought about this case. To prevent this problem, the HCD's start routine would have to disable runtime PM for the root hub, by calling pm_runtime_disable(hcd-self.root_hub.dev); Alan Stern Hi Alan. Thanks for the response. I tried your suggestion of calling pm_runtime_disable in the HCD start routine but this causes the set configuration request to fail on the root hub. The output looks like this: usb usb6: usb_probe_device usb usb6: configuration #1 chosen from 1 choice usb usb6: can't set config #1, error -13 Ah. Okay, I haven't tried to do this before; it's not surprising that something would go wrong. What appears to happen is that usb_set_configuration calls usb_autoresume_device which eventually results in a call to rpm_resume which fails with -EACCES. This causes usb_set_configuration to bail out. Is there a better place to call pm_runtime_disable that would allow the root hub to successfully start? Looks like a better way to prevent runtime PM is to make sure the usage counter is always 0. Calling pm_runtime_get_noresume() instead of pm_runtime_disable() will accomplish this. If you're interested, the full set of runtime-PM calls relating to new USB devices can be found in core/hub.c:usb_new_device(). Alan Stern Calling pm_runtime_get_noresume() instead of pm_runtime_disable() does the trick. The root hub enumerates correctly and no longer thrashes attempting bus_suspend/bus_resume. Moving forward, if I wanted to implement proper support for bus_suspend and bus_resume in the HWA, what actions would be required when those functions are called? I have a pretty good idea how to put the wireless medium into a suspended state that is similar to wired USB suspend. I am less clear on what needs to be done to the software state (URB tracking, etc..) when bus_suspend is called. I apologize if this is already documented somehwere. I did a fair amount of searching but could only find info on device suspend not bus_suspend. Thanks, Tom -- 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: Help r/e gadgetfs bulk xfer problem on Overo gumstix
Alan, Thanks for your advice, I believe I had a stupid coding error. Things are working now. Cheers, -Brett On 18/07/13 15:13, Alan Stern wrote: On Thu, 18 Jul 2013, Breton M. Saunders wrote: All, I'm running into problems with the gadgetfs usb.c bulk transport example, and am looking for some advice as to how to approach this problem. I am using the source code found at http://www.linux-usb.org/gadget/usb.c, which I compile for an overo gumstix board. Basically, I use a test program using libusb on linux, and I only ever see zeros reported back from the block of data that I read. I have looked at the data packets sent by the overo board using a hardware usb analyzer, and these only contain zero bytes. Do you specify the -p 1 option when starting the gadgetfs program? I am (unfortunately) locked to using kernel version 2.6.37 on this texas insturments DM3730 processor. My question is: Are there any known problems with gadgetfs or the musb code that may cause these symptoms? Not as far as I know. 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 -- 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Gordon Hollingworth Sent: Wednesday, July 17, 2013 11:00 PM I'd suggest just adding a Raspberry Pi Foundation copyright. Is that OK or do you need names for SOB? Ah yes, thanks for reminding me. Yes, I will need SOBs from the main contributors. And they must be real names, popcornmix is not acceptable as far as I understand. Thanks. -- Paul -- 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
Hi Paul,
The transfer scheduler in the dwc2 driver is pretty basic, not to
mention buggy. It works fairly well with just a couple of devices
plugged in, but if you add, say, multiple devices with periodic
endpoints, the scheduler breaks down and can't even enumerate all
the devices.
This seems related to a patch I made last year for the dwc_otg driver.
On RT3052, we only have 4 host channels, so it was easy to run out of
them using a 3G stick and a hub. The 3G sticks hog up 2 host channels
because they keep NAKing their bulk in transfers and 2 for intr
endpoints, leaving none for any other transfers. I created a patch to
allow canceling a NAKing host channel, but I suspect that this
microframe scheduler might help in this case as well, because it allows
sharing a single host channel for all periodic transfers, I think,
leaving the other three for bulk transfers. It might still be useful to
forward port my patch at some point, since that would in theory allow
multiple blocking endpoints to be used at the same time.
To fix this, import the microframe scheduler patch from the
driver in the downstream Raspberry Pi kernel, which is based on
the Synopsys vendor driver. That patch was done by the guys from
raspberrypi.org, including at least Gordon Hollingsworth and
popcornmix.
I have added a driver parameter for this, enabled by default, in
case anyone has problems with it and needs to disable it. I don't
think we should add a DT binding for that, though, since I plan to
remove the option once any bugs are fixed.
Some general remarks:
It seems that this patch doesn't include just the microframe scheduling,
but also the NAK holdoff patch (which was a separate patch in the
downstream kernel IIRC and seems like a separate feature in any case)
and other unrelated and unused bits.
Furthermore, I wonder about how this scheduler works exactly. What I see
is:
- the number usecs needed for a single transfer in a periodic qh is
calculated
- When the qh is scheduled, the first of the 8 microframes with enough
usecs available is picked for this qh (disregarding FS transfers that
don't fit in 1 microframe for now)
However, this seems correct only for endpoints with an interval of 8
microframes? If an isoc endpoint has an interval of 1, it should be
scheduled in _every_ microframe, right?
The old code was pessimistic (the dwc2_check_periodic_bandwidth
essentially assumes an interval of 1 for everything) but the new code is
actually too optimistic (as it essentially assumes an interval of 8 for
everything).
This leads me to believe that this code works because it makes the
scheduler way to optimistic and because it removes the one channel per
periodic endpoint policy (which is way too optimistic), but it would
break when adding too much (periodic) devices (in nasty ways, no nice
not enough bandwidth messages).
Overall, I don't think this patch is not even nearly ready to be
merged...
There's a lot more detailed comments inline in the code below.
Signed-off-by: Paul Zimmerman pa...@synopsys.com
---
Gordon, I would like to add a copyright notice for the work you
guys did on this (thanks!). Can you send me the names and dates I
should add?
This patch should be applied on top of staging: dwc2: add driver
parameter to set AHB config register value or else it won't apply
cleanly.
drivers/staging/dwc2/core.c | 21
drivers/staging/dwc2/core.h | 47 +---
drivers/staging/dwc2/hcd.c | 94 +---
drivers/staging/dwc2/hcd.h | 27 +++--
drivers/staging/dwc2/hcd_ddma.c | 13 ++-
drivers/staging/dwc2/hcd_intr.c | 59 +++---
drivers/staging/dwc2/hcd_queue.c | 236
---
drivers/staging/dwc2/pci.c | 1 +
8 files changed, 417 insertions(+), 81 deletions(-)
diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c
index a090f79..16afc06 100644
--- a/drivers/staging/dwc2/core.c
+++ b/drivers/staging/dwc2/core.c
@@ -2612,6 +2612,26 @@ int dwc2_set_param_otg_ver(struct dwc2_hsotg *hsotg,
int val)
return retval;
}
+int dwc2_set_param_uframe_sched(struct dwc2_hsotg *hsotg, int val)
+{
+ int retval = 0;
+
+ if (DWC2_PARAM_TEST(val, 0, 1)) {
+ if (val = 0) {
+ dev_err(hsotg-dev,
+ '%d' invalid for parameter uframe_sched\n,
+ val);
+ dev_err(hsotg-dev, uframe_sched must be 0 or 1\n);
+ }
+ val = 1;
+ dev_dbg(hsotg-dev, Setting uframe_sched to %d\n, val);
+ retval = -EINVAL;
+ }
+
+ hsotg-core_params-uframe_sched = val;
+ return retval;
+}
+
/*
* This function is called during module intialization to pass module
parameters
* for the DWC_otg core. It returns non-0 if any parameters are invalid.
@@ -2658,6 +2678,7 @@ int dwc2_set_parameters(struct dwc2_hsotg
Re: question about bus_suspend and bus_resume
On Thu, 18 Jul 2013, Thomas Pugliese wrote: Calling pm_runtime_get_noresume() instead of pm_runtime_disable() does the trick. The root hub enumerates correctly and no longer thrashes attempting bus_suspend/bus_resume. Good. Moving forward, if I wanted to implement proper support for bus_suspend and bus_resume in the HWA, what actions would be required when those functions are called? I have a pretty good idea how to put the wireless medium into a suspended state that is similar to wired USB suspend. I am less clear on what needs to be done to the software state (URB tracking, etc..) when bus_suspend is called. I apologize if this is already documented somehwere. I did a fair amount of searching but could only find info on device suspend not bus_suspend. It probably isn't documented anywhere. The USB core takes care of most of the work. By the time bus_suspend is called, no URBs are active. All you have to do is put the bus into the suspended state (with wired USB this means you have to turn off the Start Of Frame packets) and make the proper arrangements for enabling wakeup if the root hub's do_remote_wakeup bit is set. There may be other stuff related to managing the driver's private data structures; you must know about all that already. In wired USB, wakeup events include: port connect change (plug or unplug), port overcurrent change, and remote wakeup request received from a downstream device. In theory, remote wakeup requests are supposed to be treated as wakeup events even if the root hub's do_remote_wakeup bit isn't set, but not all hardware is capable of enabling those events without enabling the others as well. I don't know what qualifies as a wakeup event for wireless USB. The only tricky thing you may have to watch out for is the race between bus suspend and resuming a child. If one of the devices on the bus is in the middle of resuming, and if the root hub is supposed to be enabled for wakeup, then bus_suspend should fail with -EBUSY. After a runtime suspend of the bus, if you do get a wakeup event, you handle it by calling usb_hcd_resume_root_hub(). For system suspend, you have to make the appropriate platform-specific arrangements so that wakeup events will actually cause the system to wake up. 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
On Thu, Jul 18, 2013 at 04:45:55PM +, Paul Zimmerman wrote: From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Gordon Hollingworth Sent: Wednesday, July 17, 2013 11:00 PM I'd suggest just adding a Raspberry Pi Foundation copyright. Is that OK or do you need names for SOB? Ah yes, thanks for reminding me. Yes, I will need SOBs from the main contributors. And they must be real names, popcornmix is not acceptable as far as I understand. Thanks. Yes, I need real names, no pseudonyms. 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
On 07/18/2013 10:45 AM, Paul Zimmerman wrote: From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Gordon Hollingworth Sent: Wednesday, July 17, 2013 11:00 PM I'd suggest just adding a Raspberry Pi Foundation copyright. Is that OK or do you need names for SOB? Ah yes, thanks for reminding me. Yes, I will need SOBs from the main contributors. And they must be real names, popcornmix is not acceptable as far as I understand. Thanks. BTW, Dom has previously given permission to me (I think in an email posted to a public list) to modify any S-o-b lines that say popcornmix as the real name to say Dom Cobley instead. Of course, this is only relevant if the commit in question is already s-o-b popcornmix. And I am assuming that popcornmix is always Dom. (Greg, I assume this kind of agreement is fine, and it's not the case that Dom would need to amend all his commits himself to change the s-o-b?) -- 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
On Thu, Jul 18, 2013 at 11:34:47AM -0600, Stephen Warren wrote: On 07/18/2013 10:45 AM, Paul Zimmerman wrote: From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Gordon Hollingworth Sent: Wednesday, July 17, 2013 11:00 PM I'd suggest just adding a Raspberry Pi Foundation copyright. Is that OK or do you need names for SOB? Ah yes, thanks for reminding me. Yes, I will need SOBs from the main contributors. And they must be real names, popcornmix is not acceptable as far as I understand. Thanks. BTW, Dom has previously given permission to me (I think in an email posted to a public list) to modify any S-o-b lines that say popcornmix as the real name to say Dom Cobley instead. Of course, this is only relevant if the commit in question is already s-o-b popcornmix. And I am assuming that popcornmix is always Dom. (Greg, I assume this kind of agreement is fine, and it's not the case that Dom would need to amend all his commits himself to change the s-o-b?) Yes, I have seen Dom's email that said that, so it's fine for you to edit the patches to do so, I just can't apply patches with popcornmix in the changelogs. 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: How should we handle isochronous underruns?
Alan Stern wrote: On Thu, 18 Jul 2013, Ming Lei wrote: On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 4 Jul 2013, Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: I had some more ideas about this. Instead of requiring drivers to set URB_ISO_ASAP on just the first URB of an isochronous stream, we could ask drivers to call usb_reset_endpoint() between streams. This would tell the HCD that the next URB marks the start of a new stream, with no need for a special flag. This idea still requires changes from old drivers. Also it might be not appropriate to call usb_reset_endpoint() in this case because other host controller's implementation may do other unnecessary thing for this situation. Perhaps. I doubt that HCDs need to do anything when they reset an isochronous endpoint, but you're right. It's safer to avoid the issue. Another possibility, which would be even simpler, is for HCDs to assume that if the endpoint's queue has been empty for more than 100 ms then a new stream is starting. Then drivers wouldn't have to do anything special at all. (Unless they are stopping and restarting streams very rapidly, ... which happens when a stream is restarted after an error, or when two sound files are played back-to-back. The audio driver will always explicitly restart the stream (because checking whether the timeout has elapsed would be too much of a bother), so the timeout is not useful in practice. In this case, we may use changing altsetting to decide start of streaming. Yes indeed. But that could still require changes to old drivers. To be even more safe, unlinking an URB should mark the end of a stream. That's what snd-usb-audio does when it closes a stream; it kills all the outstanding URBs. But it might be possible that the queue is empty at that point. In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. 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: xhci driver problem
On Thu, Jul 18, 2013 at 11:03:32AM -0700, Gene Kopan wrote: Hello Sarah, I am having an xhci_hcd problem on my ASUS U47A laptop (running linux Mint 14) failing to mount an ASUS Xtion PRO Live RGBD camera: Jun 5 14:39:44 whiz7u kernel: [15698.052800] xhci_hcd :00:14.0: Not enough bandwidth. Proposed: 1663, Max: 1607 It doesn't matter if the camera is plugged into a USB 2 or USB 3 port. The camera works fine with ehci drivers on a USB2 only machine running linux Mint 14. Is there a fix in the pipeline for this problem? If so, how should I track it? Has the camera ever worked on this machine before? Is there a previous version of the kernel that worked? Jun 5 14:39:44 whiz7u kernel: [15698.052800] xhci_hcd :00:14.0: Not enough bandwidth. Proposed: 1663, Max: 1607 Jun 5 14:39:44 whiz7u kernel: [15698.052809] xhci_hcd :00:14.0: Not enough bandwidth Jun 5 14:39:44 whiz7u kernel: [15698.052820] usb 3-1: can't set config #1, error -12 Your camera is taking up too much of the xHCI bus bandwidth. Not by much, but the host simply won't be able to provide that much bandwidth. What other USB devices do you have attached to the system? If you unplug them, and re-plug in the camera, does it work? Please send me the output of `sudo lsusb -v` with only the camera plugged in, if it still doesn't work. Sarah Sharp -- 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 6/6] USB: OHCI: make ohci-s3c2410 a separate driver
On Tue, 25 Jun 2013, Manjunath Goudar wrote: Separate the Samsung OHCI S3C host controller driver from ohci-hcd host code so that it can be built as a separate driver module. This work is part of enabling multi-platform kernels on ARM; it would be nice to have in 3.11. This patch looks very good. I have only two very small nits: diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c index e125770..b0f6644 100644 --- a/drivers/usb/host/ohci-s3c2410.c +++ b/drivers/usb/host/ohci-s3c2410.c @@ -19,17 +19,34 @@ * This file is licenced under the GPL. */ -#include linux/platform_device.h #include linux/clk.h +#include linux/io.h +#include linux/kernel.h +#include linux/module.h +#include linux/platform_device.h #include linux/platform_data/usb-ohci-s3c2410.h +#include linux/usb.h +#include linux/usb/hcd.h + +#include ohci.h + #define valid_port(idx) ((idx) == 1 || (idx) == 2) /* clock device associated with the hcd */ + +#define DRIVER_DESC OHCI S3C driver + +static const char hcd_name[] = ohci-s3c; + static struct clk *clk; static struct clk *usb_clk; +static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq, + u16 wValue, u16 wIndex, char *buf, u16 wLength); +static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf); + /* forward definitions */ Do you understand what forward definitions means? It refers to declarations of functions whose actual code is given later. In other words, your declarations of orig_ohci_hub_control and orig_ohci_hub_status_data belong just after this comment, not just before it. @@ -93,7 +110,7 @@ ohci_s3c2410_hub_status_data(struct usb_hcd *hcd, char *buf) int orig; int portno; - orig = ohci_hub_status_data(hcd, buf); + orig = orig_ohci_hub_status_data(hcd, buf); Since you're changing the line anyway, you should get rid of the extra space before the '='. After you make those two changes, you can add my Acked-by. 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 V3 5/6] USB: OHCI: make ohci-at91 a separate driver
On Tue, 25 Jun 2013, Manjunath Goudar wrote:
Separate the TI OHCI Atmel host controller driver from ohci-hcd
host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM;
it would be nice to have in 3.11.
This looks okay except for some very minor issues.
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 46c2f42..e4dc9ab 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -390,6 +390,14 @@ config USB_OHCI_HCD_SPEAR
Enables support for the on-chip OHCI controller on
ST SPEAr chips.
+config USB_OHCI_HCD_AT91
+tristate Support for Atmel on-chip OHCI USB controller
+depends on USB_OHCI_HCD ARCH_AT91
+default y
+---help---
+ Enables support for the on-chip OHCI controller on
+ Atmel chips.
Get rid of the extra space after tristate.
@@ -686,7 +630,11 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev,
pm_message_t mesg)
* REVISIT: some boards will be able to turn VBUS off...
*/
if (at91_suspend_entering_slow_clock()) {
- ohci_usb_reset (ohci);
+ ohci-hc_control = ohci_readl (ohci, ohci-regs-control);
+ ohci-hc_control = OHCI_CTRL_RWC;
+ ohci_writel (ohci, ohci-hc_control, ohci-regs-control);
+ ohci-rh_state = OHCI_RH_HALTED;
Even though you're just copying the code, don't also copy its mistakes.
Get rid of the extra spaces before the '(' characters.
+static void __exit ohci_at91_cleanup(void)
+{
+ platform_driver_unregister(ohci_hcd_at91_driver);
+}
+module_exit(ohci_at91_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE(GPL);
Please move the existing MODULE_ALIAS line down here with these lines.
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: How should we handle isochronous underruns?
On Thu, 18 Jul 2013, Clemens Ladisch wrote: In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. I prefer a function call over the flag. The function call can easily be issued just once, but the completion routine would have to clear the flag every time the URB gets used. Maybe we can use usb_reset_endpoint() for this purpose after all. It is a perfect fit, because we want to tell the HCD to reset the isochronous endpoint back to the start of stream state. A search under drivers/ shows that only a few HCDs other than ehci currently implement the endpoint_reset method: xhci, whci, dwc2, and ozwpan. It would not be hard to fix them up to ignore calls for isochronous endpoints. Any objections to this approach? 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
From: Matthijs Kooijman [mailto:matth...@stdin.nl]
Sent: Thursday, July 18, 2013 10:19 AM
This seems related to a patch I made last year for the dwc_otg driver.
On RT3052, we only have 4 host channels, so it was easy to run out of
them using a 3G stick and a hub. The 3G sticks hog up 2 host channels
because they keep NAKing their bulk in transfers and 2 for intr
endpoints, leaving none for any other transfers. I created a patch to
allow canceling a NAKing host channel, but I suspect that this
microframe scheduler might help in this case as well, because it allows
sharing a single host channel for all periodic transfers, I think,
leaving the other three for bulk transfers. It might still be useful to
forward port my patch at some point, since that would in theory allow
multiple blocking endpoints to be used at the same time.
Sure.
It seems that this patch doesn't include just the microframe scheduling,
but also the NAK holdoff patch (which was a separate patch in the
downstream kernel IIRC and seems like a separate feature in any case)
and other unrelated and unused bits.
Yep. I thought it was part of the microframe scheduler, but I see
now it's not. I will split that into a separate patch.
Furthermore, I wonder about how this scheduler works exactly. What I see
is:
- the number usecs needed for a single transfer in a periodic qh is
calculated
- When the qh is scheduled, the first of the 8 microframes with enough
usecs available is picked for this qh (disregarding FS transfers that
don't fit in 1 microframe for now)
However, this seems correct only for endpoints with an interval of 8
microframes? If an isoc endpoint has an interval of 1, it should be
scheduled in _every_ microframe, right?
The old code was pessimistic (the dwc2_check_periodic_bandwidth
essentially assumes an interval of 1 for everything) but the new code is
actually too optimistic (as it essentially assumes an interval of 8 for
everything).
This leads me to believe that this code works because it makes the
scheduler way to optimistic and because it removes the one channel per
periodic endpoint policy (which is way too optimistic), but it would
break when adding too much (periodic) devices (in nasty ways, no nice
not enough bandwidth messages).
I can't speak to exactly how it works, because I am not familiar with
the code. I just know it fixes some things on the Raspberry Pi. And it
has been in the released Pi kernel for quite some time.
Overall, I don't think this patch is not even nearly ready to be
merged...
Well, I disagree :)
@@ -74,6 +74,9 @@ enum dwc2_lx_state {
* 0 - HNP and SRP capable (default)
* 1 - SRP Only capable
* 2 - No HNP/SRP capable
+ * @otg_ver:OTG version supported
+ * 0 - 1.3
+ * 1 - 2.0
* @dma_enable: Specifies whether to use slave or DMA mode for
accessing
* the data FIFOs. The driver will automatically
detect the
* value for this parameter if none is specified.
@@ -90,20 +93,10 @@ enum dwc2_lx_state {
* the attached device and the value of phy_type.
* 0 - High Speed (default)
* 1 - Full Speed
- * @host_support_fs_ls_low_power: Specifies whether low power mode is
supported
- * when attached to a Full Speed or Low Speed device
in
- * host mode.
- * 0 - Don't support low power mode (default)
- * 1 - Support low power mode
- * @host_ls_low_power_phy_clk: Specifies the PHY clock rate in low power
mode
- * when connected to a Low Speed device in host mode.
This
- * parameter is applicable only if
- * host_support_fs_ls_low_power is enabled. If
phy_type is
- * set to FS then defaults to 6 MHZ otherwise 48 MHZ.
- * 0 - 48 MHz
- * 1 - 6 MHz
* @enable_dynamic_fifo: 0 - Use coreConsultant-specified FIFO size
parameters
* 1 - Allow dynamic FIFO sizing (default)
+ * @en_multiple_tx_fifo: Specifies whether dedicated per-endpoint transmit
FIFOs
+ * are enabled
* @host_rx_fifo_size: Number of 4-byte words in the Rx FIFO in host mode
when
* dynamic FIFO sizing is enabled
* 16 to 32768 (default 1024)
@@ -145,9 +138,19 @@ enum dwc2_lx_state {
* 0 - No (default)
* 1 - Yes
* @ulpi_fs_ls: True to make ULPI phy operate in FS/LS mode only
+ * @host_support_fs_ls_low_power: Specifies whether low power mode is
supported
+ * when
RE: [PATCH 0/3] staging: dwc2: Add some dma-related defensive programming
From: Matthijs Kooijman [mailto:matth...@stdin.nl] Sent: Wednesday, July 17, 2013 9:17 AM Here's a resend of three patches I sent a while ago as part of a bigger series. These were the unimportant patches to be included later, so here they are again. Gr. Matthijs Matthijs Kooijman (3): staging: dwc2: disable dma when no dma_mask was setup staging: dwc2: when dma is disabled, clear hcd-self.uses_dma staging: dwc2: Don't touch the dma_mask when dma is disabled drivers/staging/dwc2/hcd.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) Hi Matthijs, You can add my acked-by to these three. But you forgot to CC Greg on these, so you will need to resend them. -- Paul -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
On Thu, Jul 18, 2013 at 04:28:01PM -0400, Peter Hurley wrote: [ +cc Sarah Sharp, linux-usb ] On 07/18/2013 09:21 AM, Nestor Lopez Casado wrote: This reverts commit 8af6c08830b1ae114d1a8b548b1f8b056e068887. This patch re-adds the workaround introduced by 596264082f10dd4 which was reverted by 8af6c08830b1ae114. The original patch 596264 was needed to overcome a situation where the hid-core would drop incoming reports while probe() was being executed. This issue was solved by c849a6143bec520af which added hid_device_io_start() and hid_device_io_stop() that enable a specific hid driver to opt-in for input reports while its probe() is being executed. Commit a9dd22b730857347 modified hid-logitech-dj so as to use the functionality added to hid-core. Having done that, workaround 596264 was no longer necessary and was reverted by 8af6c08. We now encounter a different problem that ends up 'again' thwarting the Unifying receiver enumeration. The problem is time and usb controller dependent. Ocasionally the reports sent to the usb receiver to start the paired devices enumeration fail with -EPIPE and the receiver never gets to enumerate the paired devices. With dcd9006b1b053c7b1c the problem was hidden as the call to the usb driver became asynchronous and none was catching the error from the failing URB. As the root cause for this failing SET_REPORT is not understood yet, -possibly a race on the usb controller drivers or a problem with the Unifying receiver- reintroducing this workaround solves the problem. Before we revert to using the workaround, I'd like to suggest that this new hidden problem may be an interaction with the xhci_hcd host controller driver only. Looking at the related bug, the OP indicates the machine only has USB3 ports. Additionally, comments #7, #100, and #104 of the original bug report [1] add additional information that would seem to confirm this suspicion. Question: does this USB device need a control transfer to reset its endpoints when the endpoints are not actually halted? If so, yes, that is a known xHCI driver bug that needs to be fixed. The xHCI host will not accept a Reset Endpoint command when the endpoints are not actually halted, but the USB core will send the control transfer to reset the endpoint. That means the device and host toggles will be out of sync, and all messages will start to fail with -EPIPE. Can the OP capture a usbmon trace when the device starts failing? That will reveal whether this actually is the issue. dmesg output with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on would also be helpful. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
[ +cc Sarah Sharp, linux-usb ]
On 07/18/2013 09:21 AM, Nestor Lopez Casado wrote:
This reverts commit 8af6c08830b1ae114d1a8b548b1f8b056e068887.
This patch re-adds the workaround introduced by 596264082f10dd4
which was reverted by 8af6c08830b1ae114.
The original patch 596264 was needed to overcome a situation where
the hid-core would drop incoming reports while probe() was being
executed.
This issue was solved by c849a6143bec520af which added
hid_device_io_start() and hid_device_io_stop() that enable a specific
hid driver to opt-in for input reports while its probe() is being
executed.
Commit a9dd22b730857347 modified hid-logitech-dj so as to use the
functionality added to hid-core. Having done that, workaround 596264
was no longer necessary and was reverted by 8af6c08.
We now encounter a different problem that ends up 'again' thwarting
the Unifying receiver enumeration. The problem is time and usb controller
dependent. Ocasionally the reports sent to the usb receiver to start
the paired devices enumeration fail with -EPIPE and the receiver never
gets to enumerate the paired devices.
With dcd9006b1b053c7b1c the problem was hidden as the call to the usb
driver became asynchronous and none was catching the error from the
failing URB.
As the root cause for this failing SET_REPORT is not understood yet,
-possibly a race on the usb controller drivers or a problem with the
Unifying receiver- reintroducing this workaround solves the problem.
Before we revert to using the workaround, I'd like to suggest that
this new hidden problem may be an interaction with the xhci_hcd host
controller driver only.
Looking at the related bug, the OP indicates the machine only has
USB3 ports. Additionally, comments #7, #100, and #104 of the original
bug report [1] add additional information that would seem to confirm
this suspicion.
Let me add I have this USB device running on the uhci_hcd driver
with or without this workaround on v3.10.
Overall what this workaround does is: If an input report from an
unknown device is received, then a (re)enumeration is performed.
related bug:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1194649
I thought I saw someone reporting this problem recently on LKML;
where is the Reported-by so that Sarah can follow-up with the
reporter directly, if desired?
Regards,
Peter Hurley
[1]
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1039143
Signed-off-by: Nestor Lopez Casado nlopezca...@logitech.com
---
drivers/hid/hid-logitech-dj.c | 45 +
drivers/hid/hid-logitech-dj.h |1 +
2 files changed, 46 insertions(+)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index db3192b..0d13389 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -192,6 +192,7 @@ static struct hid_ll_driver logi_dj_ll_driver;
static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
size_t count,
unsigned char report_type);
+static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev
*djrcv_dev);
static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev
*djrcv_dev,
struct dj_report *dj_report)
@@ -232,6 +233,7 @@ static void logi_dj_recv_add_djhid_device(struct
dj_receiver_dev *djrcv_dev,
if (dj_report-report_params[DEVICE_PAIRED_PARAM_SPFUNCTION]
SPFUNCTION_DEVICE_LIST_EMPTY) {
dbg_hid(%s: device list is empty\n, __func__);
+ djrcv_dev-querying_devices = false;
return;
}
@@ -242,6 +244,12 @@ static void logi_dj_recv_add_djhid_device(struct
dj_receiver_dev *djrcv_dev,
return;
}
+ if (djrcv_dev-paired_dj_devices[dj_report-device_index]) {
+ /* The device is already known. No need to reallocate it. */
+ dbg_hid(%s: device is already known\n, __func__);
+ return;
+ }
+
dj_hiddev = hid_allocate_device();
if (IS_ERR(dj_hiddev)) {
dev_err(djrcv_hdev-dev, %s: hid_allocate_device failed\n,
@@ -305,6 +313,7 @@ static void delayedwork_callback(struct work_struct *work)
struct dj_report dj_report;
unsigned long flags;
int count;
+ int retval;
dbg_hid(%s\n, __func__);
@@ -337,6 +346,25 @@ static void delayedwork_callback(struct work_struct *work)
logi_dj_recv_destroy_djhid_device(djrcv_dev, dj_report);
break;
default:
+ /* A normal report (i. e. not belonging to a pair/unpair notification)
+* arriving here, means that the report arrived but we did not have a
+* paired dj_device associated to the report's device_index, this
+* means that the original device paired notification corresponding
+* to this dj_device never arrived to this driver.
[PATCH] mmc: Allow sdhci platform drivers to set quirks2 from platform data
Signed-off-by: Al Cooper alcoop...@gmail.com
---
drivers/mmc/host/sdhci-pltfm.c |4 +++-
drivers/mmc/host/sdhci-pltfm.h |1 +
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 3145a78..e605509 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -149,8 +149,10 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device
*pdev,
host-ops = pdata-ops;
else
host-ops = sdhci_pltfm_ops;
- if (pdata)
+ if (pdata) {
host-quirks = pdata-quirks;
+ host-quirks2 = pdata-quirks2;
+ }
host-irq = platform_get_irq(pdev, 0);
if (!request_mem_region(iomem-start, resource_size(iomem),
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 153b6c5..2940ee3 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -18,6 +18,7 @@
struct sdhci_pltfm_data {
struct sdhci_ops *ops;
unsigned int quirks;
+ unsigned int quirks2;
};
struct sdhci_pltfm_host {
--
1.7.6
--
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] mmc: Allow sdhci platform drivers to set quirks2 from platform data
Hello. On 07/19/2013 02:31 AM, Al Cooper wrote: Signed-off-by: Al Cooper alcoop...@gmail.com --- drivers/mmc/host/sdhci-pltfm.c |4 +++- drivers/mmc/host/sdhci-pltfm.h |1 + 2 files changed, 4 insertions(+), 1 deletions(-) Hm, what this patch has to do with linux-usb? WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: Add Device Tree support to XHCI Platform driver
Add Device Tree match table. Setup dma_mask and coherent_dma_mask
if they're not already set.
Signed-off-by: Al Cooper alcoop...@gmail.com
---
drivers/usb/host/xhci-plat.c | 18 ++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 51e22bf..60b3ff4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -14,6 +14,8 @@
#include linux/platform_device.h
#include linux/module.h
#include linux/slab.h
+#include linux/of.h
+#include linux/dma-mapping.h
#include xhci.h
@@ -91,6 +93,16 @@ static int xhci_plat_probe(struct platform_device *pdev)
int ret;
int irq;
+ /*
+* Right now device-tree probed devices don't get dma_mask set.
+* Since shared usb code relies on it, set it here for now.
+* Once we have dma capability bindings this can go away.
+*/
+ if (!pdev-dev.dma_mask)
+ pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
+ if (!pdev-dev.coherent_dma_mask)
+ pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
if (usb_disabled())
return -ENODEV;
@@ -186,11 +198,17 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = xhci-hcd },
+ {},
+};
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
MODULE_ALIAS(platform:xhci-hcd);
--
1.7.6
--
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: xhci driver problem
On Thu, Jul 18, 2013 at 01:49:04PM -0700, Gene Kopan wrote: Hi Sarah, thanks for helping. I answer your questions below. On 07/18/2013 12:55 PM, Sarah Sharp wrote: On Thu, Jul 18, 2013 at 11:03:32AM -0700, Gene Kopan wrote: Hello Sarah, I am having an xhci_hcd problem on my ASUS U47A laptop (running linux Mint 14) failing to mount an ASUS Xtion PRO Live RGBD camera: works fine on my older usb2 tower machine running mint 14 (same kernel) and ubuntu 10.04 Right, that's not an xHCI host. And you probably don't have the internal webcam. Jun 5 14:39:44 whiz7u kernel: [15698.052800] xhci_hcd :00:14.0: Not enough bandwidth. Proposed: 1663, Max: 1607 Jun 5 14:39:44 whiz7u kernel: [15698.052809] xhci_hcd :00:14.0: Not enough bandwidth Jun 5 14:39:44 whiz7u kernel: [15698.052820] usb 3-1: can't set config #1, error -12 Your camera is taking up too much of the xHCI bus bandwidth. Not by much, but the host simply won't be able to provide that much bandwidth. how can that be when the camera works fine on my older usb2 machine? What other USB devices do you have attached to the system? If you unplug them, and re-plug in the camera, does it work? The ASUS u47a laptop also has a built-in video camera (which works fine). I don't know how to disable it Find the camera in the lsusb (and `lsusb -t`) output, and then follow these directions to unbind the driver: http://lwn.net/Articles/143397/ Then try using your other camera. If that doesn't work, you may have to unconfigure the device by running something like: root@xanatos:/sys/bus/usb/devices/3-1.6# echo 0 bConfigurationValue Alan, is that the right command? It fails on my machine with -EINVAL. I have seen others complaining about this problem with xhci_hcd drivers and this (Primesense)camera. I could find a pointer to such a discussion if that is useful. Let me know if I can provide other info. If that works, hopefully you can point them to the same solution. Sarah Sharp -- 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: Add Device Tree support to XHCI Platform driver
Hello.
On 07/19/2013 02:35 AM, Al Cooper wrote:
Add Device Tree match table. Setup dma_mask and coherent_dma_mask
if they're not already set.
Signed-off-by: Al Cooper alcoop...@gmail.com
[...]
@@ -186,11 +198,17 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = xhci-hcd },
I think just xhci would be enough. We're describing the device, not its
driver with this prop.
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: [PATCH] usb: Add Device Tree support to XHCI Platform driver
On Thu, Jul 18, 2013 at 06:35:13PM -0400, Al Cooper wrote:
Add Device Tree match table. Setup dma_mask and coherent_dma_mask
if they're not already set.
Signed-off-by: Al Cooper alcoop...@gmail.com
---
drivers/usb/host/xhci-plat.c | 18 ++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 51e22bf..60b3ff4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -14,6 +14,8 @@
#include linux/platform_device.h
#include linux/module.h
#include linux/slab.h
+#include linux/of.h
+#include linux/dma-mapping.h
#include xhci.h
@@ -91,6 +93,16 @@ static int xhci_plat_probe(struct platform_device *pdev)
int ret;
int irq;
+ /*
+ * Right now device-tree probed devices don't get dma_mask set.
+ * Since shared usb code relies on it, set it here for now.
+ * Once we have dma capability bindings this can go away.
+ */
+ if (!pdev-dev.dma_mask)
+ pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
+ if (!pdev-dev.coherent_dma_mask)
+ pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
Xenia is already working on a patch to fix the xhci-plat's DMA mask:
http://marc.info/?l=linux-usbm=137304523616938w=2
Please rebase this patch on top of her patch.
if (usb_disabled())
return -ENODEV;
@@ -186,11 +198,17 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = xhci-hcd },
+ {},
+};
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
MODULE_ALIAS(platform:xhci-hcd);
--
1.7.6
Sarah Sharp
--
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: add the suspend/resume functionality
Hi Felipe, My bad, this patch got dropped. I've refreshed it against Greg's usb-next branch. Do you still want me to push this? Again, my apologies for dropping this. Sarah Sharp On Tue, Feb 12, 2013 at 11:07:30PM +0200, Felipe Balbi wrote: Hi, On Tue, Feb 12, 2013 at 12:47:23PM -0800, Sarah Sharp wrote: On Mon, Feb 11, 2013 at 11:57:33AM +0200, Felipe Balbi wrote: From: Vikas Sajjan vikas.saj...@linaro.org Adds power management support to xHCI platform driver. This patch facilitates the transition of xHCI host controller between S0 and S3/S4 power states, during suspend/resume cycles. Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com Signed-off-by: Vikas C Sajjan vikas.saj...@linaro.org CC: Doug Anderson diand...@chromium.org Signed-off-by: Felipe Balbi ba...@ti.com --- Hi Sarah, can you carry this patch for v3.10 merge window ? I have refreshed it against v3.8-rc7 and dropped the check for HC_STATE_SUSPENDED which we have moved to xhci_suspend() itself. Sure, I can carry the revised patch for 3.10. I assume Greg isn't accepting new patches for 3.9 at this point? you assumed correctly ;-) -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 1/2] xhci: Refactor port status into a new function.
The hub control function is *way* too long. Refactor it into a new
function, and document the side effects of calling that function.
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 210 ---
1 files changed, 119 insertions(+), 91 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1d35459..83fc636 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -534,6 +534,118 @@ void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32
status, u16 wIndex)
}
}
+/*
+ * Converts a raw xHCI port status into the format that external USB 2.0 or USB
+ * 3.0 hubs use.
+ *
+ * Possible side effects:
+ * - Mark a port as being done with device resume,
+ *and ring the endpoint doorbells.
+ * - Stop the Synopsys redriver Compliance Mode polling.
+ */
+static u32 xhci_get_port_status(struct usb_hcd *hcd,
+ struct xhci_bus_state *bus_state,
+ __le32 __iomem **port_array,
+ u16 wIndex, u32 raw_port_status)
+{
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ u32 status = 0;
+ int slot_id;
+
+ /* wPortChange bits */
+ if (raw_port_status PORT_CSC)
+ status |= USB_PORT_STAT_C_CONNECTION 16;
+ if (raw_port_status PORT_PEC)
+ status |= USB_PORT_STAT_C_ENABLE 16;
+ if ((raw_port_status PORT_OCC))
+ status |= USB_PORT_STAT_C_OVERCURRENT 16;
+ if ((raw_port_status PORT_RC))
+ status |= USB_PORT_STAT_C_RESET 16;
+ /* USB3.0 only */
+ if (hcd-speed == HCD_USB3) {
+ if ((raw_port_status PORT_PLC))
+ status |= USB_PORT_STAT_C_LINK_STATE 16;
+ if ((raw_port_status PORT_WRC))
+ status |= USB_PORT_STAT_C_BH_RESET 16;
+ }
+
+ if (hcd-speed != HCD_USB3) {
+ if ((raw_port_status PORT_PLS_MASK) == XDEV_U3
+(raw_port_status PORT_POWER))
+ status |= USB_PORT_STAT_SUSPEND;
+ }
+ if ((raw_port_status PORT_PLS_MASK) == XDEV_RESUME
+ !DEV_SUPERSPEED(raw_port_status)) {
+ if ((raw_port_status PORT_RESET) ||
+ !(raw_port_status PORT_PE))
+ return 0x;
+ if (time_after_eq(jiffies,
+ bus_state-resume_done[wIndex])) {
+ xhci_dbg(xhci, Resume USB2 port %d\n,
+ wIndex + 1);
+ bus_state-resume_done[wIndex] = 0;
+ clear_bit(wIndex, bus_state-resuming_ports);
+ xhci_set_link_state(xhci, port_array, wIndex,
+ XDEV_U0);
+ xhci_dbg(xhci, set port %d resume\n,
+ wIndex + 1);
+ slot_id = xhci_find_slot_id_by_port(hcd, xhci,
+ wIndex + 1);
+ if (!slot_id) {
+ xhci_dbg(xhci, slot_id is zero\n);
+ return 0x;
+ }
+ xhci_ring_device(xhci, slot_id);
+ bus_state-port_c_suspend |= 1 wIndex;
+ bus_state-suspended_ports = ~(1 wIndex);
+ } else {
+ /*
+* The resume has been signaling for less than
+* 20ms. Report the port status as SUSPEND,
+* let the usbcore check port status again
+* and clear resume signaling later.
+*/
+ status |= USB_PORT_STAT_SUSPEND;
+ }
+ }
+ if ((raw_port_status PORT_PLS_MASK) == XDEV_U0
+(raw_port_status PORT_POWER)
+(bus_state-suspended_ports (1 wIndex))) {
+ bus_state-suspended_ports = ~(1 wIndex);
+ if (hcd-speed != HCD_USB3)
+ bus_state-port_c_suspend |= 1 wIndex;
+ }
+ if (raw_port_status PORT_CONNECT) {
+ status |= USB_PORT_STAT_CONNECTION;
+ status |= xhci_port_speed(raw_port_status);
+ }
+ if (raw_port_status PORT_PE)
+ status |= USB_PORT_STAT_ENABLE;
+ if (raw_port_status PORT_OC)
+ status |= USB_PORT_STAT_OVERCURRENT;
+ if (raw_port_status PORT_RESET)
+ status |= USB_PORT_STAT_RESET;
+ if (raw_port_status PORT_POWER) {
+ if (hcd-speed == HCD_USB3)
+ status |= USB_SS_PORT_STAT_POWER;
+ else
+ status |= USB_PORT_STAT_POWER;
+ }
+ /* Update Port Link State
Re: [PATCH 1/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
On 07/18/2013 06:09 PM, Sarah Sharp wrote: On Thu, Jul 18, 2013 at 04:28:01PM -0400, Peter Hurley wrote: [ +cc Sarah Sharp, linux-usb ] On 07/18/2013 09:21 AM, Nestor Lopez Casado wrote: This reverts commit 8af6c08830b1ae114d1a8b548b1f8b056e068887. This patch re-adds the workaround introduced by 596264082f10dd4 which was reverted by 8af6c08830b1ae114. The original patch 596264 was needed to overcome a situation where the hid-core would drop incoming reports while probe() was being executed. This issue was solved by c849a6143bec520af which added hid_device_io_start() and hid_device_io_stop() that enable a specific hid driver to opt-in for input reports while its probe() is being executed. Commit a9dd22b730857347 modified hid-logitech-dj so as to use the functionality added to hid-core. Having done that, workaround 596264 was no longer necessary and was reverted by 8af6c08. We now encounter a different problem that ends up 'again' thwarting the Unifying receiver enumeration. The problem is time and usb controller dependent. Ocasionally the reports sent to the usb receiver to start the paired devices enumeration fail with -EPIPE and the receiver never gets to enumerate the paired devices. With dcd9006b1b053c7b1c the problem was hidden as the call to the usb driver became asynchronous and none was catching the error from the failing URB. As the root cause for this failing SET_REPORT is not understood yet, -possibly a race on the usb controller drivers or a problem with the Unifying receiver- reintroducing this workaround solves the problem. Before we revert to using the workaround, I'd like to suggest that this new hidden problem may be an interaction with the xhci_hcd host controller driver only. Looking at the related bug, the OP indicates the machine only has USB3 ports. Additionally, comments #7, #100, and #104 of the original bug report [1] add additional information that would seem to confirm this suspicion. Question: does this USB device need a control transfer to reset its endpoints when the endpoints are not actually halted? If so, yes, that is a known xHCI driver bug that needs to be fixed. The xHCI host will not accept a Reset Endpoint command when the endpoints are not actually halted, but the USB core will send the control transfer to reset the endpoint. That means the device and host toggles will be out of sync, and all messages will start to fail with -EPIPE. Can the OP capture a usbmon trace when the device starts failing? That will reveal whether this actually is the issue. dmesg output with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on would also be helpful. Sarah, I forwarded your usbmon capture request to the OP in the bug report (I don't have an email address for the reporter). As far as getting printk output from a custom kernel, I think that may be beyond the reporter's capability. Perhaps one of the Ubuntu devs triaging this bug could provide a test kernel for the OP with those options on. Joseph, would you be willing to do that? Regards, Peter Hurley -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 0/2] xhci: Show USB 2.0 Link PM status
Greetings! Alex has been working on some lsusb patches to show the USB 2.0 link status on xHCI hosts. In order to see whether an xHCI rootport is in the lower power link state L1, there needs to be some xHCI driver changes to show the USB 2.0 link status. Please review. Thanks, Sarah Sharp The following changes since commit 106cbc1702dfc6dda5c04df10a47bf888e8bf3d6: usb: xhci: add the suspend/resume functionality (2013-07-18 16:15:35 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git fun-usb2-lpm-status for you to fetch changes up to 1c8e13431f27e1d454ccd82d201374a889aac4f6: xhci: Report USB 2.1 link status for L1 (2013-07-18 16:15:37 -0700) Sarah Sharp (2): xhci: Refactor port status into a new function. xhci: Report USB 2.1 link status for L1 drivers/usb/host/xhci-hub.c | 221 +-- 1 files changed, 129 insertions(+), 92 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
[RFC 2/2] xhci: Report USB 2.1 link status for L1
USB 2.1 devices can go into a lower power link state, L1. When they are
active, they are in the L0 state. The L1 transition can be purely
driven by software, or some USB host controllers (including some xHCI
1.0 hosts) allow the host hardware to track idleness and automatically
place a port into L1.
The USB 2.1 Link Power Management ECN gives a way for USB 2.1 hubs that
support LPM to report that a port is in L1. The port status bit 5 will
be set when the port is in L1. The xHCI host reports the root port as
being in 'U2' when the devices is in L1, and as being in 'U0' when the
port is active (in L0).
Translate the xHCI USB 2.1 link status into the format external hubs
use, and pass the L1 status up to the USB core and tools like lsusb.
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 15 ---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 83fc636..93f3fdf 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -461,8 +461,15 @@ void xhci_test_and_clear_bit(struct xhci_hcd *xhci, __le32
__iomem **port_array,
}
}
+/* Updates Link Status for USB 2.1 port */
+static void xhci_hub_report_usb2_link_state(u32 *status, u32 status_reg)
+{
+ if ((status_reg PORT_PLS_MASK) == XDEV_U2)
+ *status |= USB_PORT_STAT_L1;
+}
+
/* Updates Link Status for super Speed port */
-static void xhci_hub_report_link_state(u32 *status, u32 status_reg)
+static void xhci_hub_report_usb3_link_state(u32 *status, u32 status_reg)
{
u32 pls = status_reg PORT_PLS_MASK;
@@ -631,14 +638,16 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
else
status |= USB_PORT_STAT_POWER;
}
- /* Update Port Link State for super speed ports*/
+ /* Update Port Link State */
if (hcd-speed == HCD_USB3) {
- xhci_hub_report_link_state(status, raw_port_status);
+ xhci_hub_report_usb3_link_state(status, raw_port_status);
/*
* Verify if all USB3 Ports Have entered U0 already.
* Delete Compliance Mode Timer if so.
*/
xhci_del_comp_mod_timer(xhci, raw_port_status, wIndex);
+ } else {
+ xhci_hub_report_usb2_link_state(status, raw_port_status);
}
if (bus_state-port_c_suspend (1 wIndex))
status |= 1 USB_PORT_FEAT_C_SUSPEND;
--
1.7.9
--
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] backports: backport drvdata = NULL core driver fixes
From: Luis R. Rodriguez mcg...@do-not-panic.com
The Linux kernel had tons of code which at times cleared the
drvdata upon probe failure or release. There are however a bunch
of drivers that didn't clear this.
Commit 0998d063 implmented clearing this upon device_release_driver()
and dealt with probe failure on driver_probe_device(). After this the
kernel was cleaned up separately with *tons* of patches to remove all
these driver specific settings given that the clearing is now done
internally by the device core.
Instead of ifdef'ing code back in for older code where it was properly
in place backport this by piggy backing the new required code upon the
calls used in place. There is a small race here upon device_release_driver()
but we can live with that theoretical race.
Due to the way we hack this backport we can't use a separate namespace
as we have with other symbols.
mcgrof@frijol ~/linux-stable (git::master)$ git describe --contains \
0998d0631001288a5974afc0b2a5f568bcdecb4d
v3.6-rc1~99^2~14^2~17
commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
Author: Hans de Goede hdego...@redhat.com
Date: Wed May 23 00:09:34 2012 +0200
device-core: Ensure drvdata = NULL when no driver is bound
1) drvdata is for a driver to store a pointer to driver specific data
2) If no driver is bound, there is no driver specific data associated with
the device
3) Thus logically drvdata should be NULL if no driver is bound.
But many drivers don't clear drvdata on device_release, or set drvdata
early on in probe and leave it set on probe error. Both of which results
in a dangling pointer in drvdata.
This patch enforce for drvdata to be NULL after device_release or on probe
failure.
Signed-off-by: Hans de Goede hdego...@redhat.com
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Tested with ckmake against next-20130618:
1 2.6.24 [ OK ]
2 2.6.25 [ OK ]
3 2.6.26 [ OK ]
4 2.6.27 [ OK ]
5 2.6.28 [ OK ]
6 2.6.29 [ OK ]
7 2.6.30 [ OK ]
8 2.6.31 [ OK ]
9 2.6.32 [ OK ]
10 2.6.33 [ OK ]
11 2.6.34 [ OK ]
12 2.6.35 [ OK ]
13 2.6.36 [ OK ]
14 2.6.37 [ OK ]
15 2.6.38 [ OK ]
16 2.6.39 [ OK ]
17 3.0.79 [ OK ]
18 3.1.10 [ OK ]
19 3.10-rc1[ OK ]
20 3.2.45 [ OK ]
21 3.3.8 [ OK ]
22 3.4.46 [ OK ]
23 3.5.7 [ OK ]
24 3.6.11 [ OK ]
25 3.7.10 [ OK ]
26 3.8.13 [ OK ]
27 3.9.3 [ OK ]
real32m2.332s
user860m23.688s
sys 121m20.840s
Cc: Hans de Goede hdego...@redhat.com
Cc: Julia Lawall julia.law...@lip6.fr
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Jiri Slaby jsl...@suse.cz
Cc: Jiri Kosina jkos...@suse.cz
Cc: Felix Fietkau n...@openwrt.org
Signed-off-by: Luis R. Rodriguez mcg...@do-not-panic.com
---
backport/backport-include/linux/device.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/backport/backport-include/linux/device.h
b/backport/backport-include/linux/device.h
index c2f80e2..ba55d0e 100644
--- a/backport/backport-include/linux/device.h
+++ b/backport/backport-include/linux/device.h
@@ -176,4 +176,23 @@ extern int dev_set_name(struct device *dev, const char
*name, ...)
__attribute__((format(printf, 2, 3)));
#endif
+#if LINUX_VERSION_CODE = KERNEL_VERSION(3,6,0)
+#define driver_probe_device(__drv, __dev) \
+({ \
+ int ret;\
+ ret = (driver_probe_device)(__drv, __dev); \
+ if (ret)\
+ dev_set_drvdata(__dev, NULL); \
+ return ret; \
+})
+
+#define device_release_driver(__dev) \
+({ \
+ (device_release_driver)(__dev); \
+ device_lock(__dev); \
+ dev_set_drvdata(__dev, NULL); \
+ device_unlock(__dev); \
+})
+#endif /* LINUX_VERSION_CODE = KERNEL_VERSION(3,6,0) */
+
#endif /* __BACKPORT_DEVICE_H */
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] device-core: Ensure drvdata = NULL when no driver is bound
On Tue, May 22, 2012 at 3:09 PM, Hans de Goede hdego...@redhat.com wrote:
1) drvdata is for a driver to store a pointer to driver specific data
2) If no driver is bound, there is no driver specific data associated with
the device
3) Thus logically drvdata should be NULL if no driver is bound.
But many drivers don't clear drvdata on device_release, or set drvdata
early on in probe and leave it set on probe error. Both of which results
in a dangling pointer in drvdata.
This patch enforce for drvdata to be NULL after device_release or on probe
failure.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
drivers/base/dd.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 1b1cbb5..9a1e970 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -283,6 +283,7 @@ probe_failed:
devres_release_all(dev);
driver_sysfs_remove(dev);
dev-driver = NULL;
+ dev_set_drvdata(dev, NULL);
if (ret == -EPROBE_DEFER) {
/* Driver requested deferred probing */
@@ -487,6 +488,7 @@ static void __device_release_driver(struct device *dev)
drv-remove(dev);
devres_release_all(dev);
dev-driver = NULL;
+ dev_set_drvdata(dev, NULL);
klist_remove(dev-p-knode_driver);
if (dev-bus)
blocking_notifier_call_chain(dev-bus-p-bus_notifier,
Just as a heads up for backports:
Because of this patch a huge series of cleanups were made across the
kernel removing redundant setting of the driver data to NULL that was
spread out through the kernel. An example type patch is
785ec305b26ee23e0efb834b5cd0dd24070ba1bf. This is an example type of
collateral evolution that doesn't cause compilation issues if the
upstream code is used but if not addressed could potentially cause an
issue. That is the only way to catch these are by policing /
proactively reviewing kernel changes.
I'll post a patch that backports this to enable usage of the code
as-is upstream on older kernels without having to put the code that
was being removed for each driver specifically shortly.
Luis
--
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: Add Device Tree support to XHCI Platform driver
On 07/19/2013 02:12 AM, Sarah Sharp wrote:
On Thu, Jul 18, 2013 at 06:35:13PM -0400, Al Cooper wrote:
Add Device Tree match table. Setup dma_mask and coherent_dma_mask
if they're not already set.
Signed-off-by: Al Cooper alcoop...@gmail.com
---
drivers/usb/host/xhci-plat.c | 18 ++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 51e22bf..60b3ff4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -14,6 +14,8 @@
#include linux/platform_device.h
#include linux/module.h
#include linux/slab.h
+#include linux/of.h
+#include linux/dma-mapping.h
#include xhci.h
@@ -91,6 +93,16 @@ static int xhci_plat_probe(struct platform_device *pdev)
int ret;
int irq;
+ /*
+* Right now device-tree probed devices don't get dma_mask set.
+* Since shared usb code relies on it, set it here for now.
+* Once we have dma capability bindings this can go away.
+*/
+ if (!pdev-dev.dma_mask)
+ pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
+ if (!pdev-dev.coherent_dma_mask)
+ pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
Xenia is already working on a patch to fix the xhci-plat's DMA mask:
http://marc.info/?l=linux-usbm=137304523616938w=2
Please rebase this patch on top of her patch.
if (usb_disabled())
return -ENODEV;
@@ -186,11 +198,17 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = xhci-hcd },
+ {},
+};
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
MODULE_ALIAS(platform:xhci-hcd);
--
1.7.6
Sarah Sharp
Hi Al and Sarah,
I think it is the best idea to add the initialization of dma mask in the
probe function.
In that way, will be no need to set the uses_dma flag in the
xhci_plat_setup(), which
as Alan said should not be set manually in the hcd.
One thing that bothers me is if a 32bit dma bitmask will be supported on any
host system, so that there is no need to call dma_set_mask() (or
dma_set_coherent_mask())
for 32bit dma bitmask and check on the return value.
Also, what if dma_mask is set but it is assigned a value less than
0x. The above
patch will not set it to 32bits.
Maybe I make up strange senarios now, but this is due to lack of
knowledge :)
regards,
ksenia
--
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: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 5:20 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 18 Jul 2013, Clemens Ladisch wrote: In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. I prefer a function call over the flag. The function call can easily be issued just once, but the completion routine would have to clear the flag every time the URB gets used. IMO, one good candidate is to do it in usb_set_interface(), and we may avoid changes on most of drivers which are using isoc endpoints, also it is reasonable, see blow: From 5.6.3 Isochronous Transfer Packet Size Constraints of USB spec 2.0: All device default interface settings must not include any isochronous endpoints with non-zero data payload sizes (specified via wMaxPacketSize in the endpoint descriptor). Alternate interface settings may specify non-zero data payload sizes for isochronous endpoints. that said all drivers which are using isoc endpoints have to call usb_set_interface(altsetting) explicitly before starting isoc schedule. So for isoc drivers, it is very reasonable to call usb_set_interface(altsetting) before starting streaming and call usb_set_interface(0) after stopping streaming. I think we may document this usage, then use this info as starting/stopping stream flag. Looks both uvc and uac drivers support this way. If one driver doesn't call usb_set_interface(0) after streaming off, we can think it as buggy since the device still consumes bus bandwidth unnecessary and may cause other device's bandwidth requirement not satisfied on same bus. Maybe we can use usb_reset_endpoint() for this purpose after all. It is a perfect fit, because we want to tell the HCD to reset the isochronous endpoint back to the start of stream state. I suggest not to introduce extra starting stream function to usb_reset_endpoint(), and if we have to do this, I suggest to add a new API for the purpose cleanly. A search under drivers/ shows that only a few HCDs other than ehci currently implement the endpoint_reset method: xhci, whci, dwc2, and ozwpan. It would not be hard to fix them up to ignore calls for isochronous endpoints. From xhci_endpoint_reset(), looks the hcd doesn't rule out isoc endpoint. Actually 'start of stream' should be done inside usbcore because it is HCD-independent function, so it is better to provide the information from usbcore and let HCD use it if HCD needs the flag. Thanks, -- Ming Lei -- 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: usb-skeleton.c: add retry for nonblocking read
From: Chen Wang unicornxx.w...@gmail.com Updated skel_read() in usb-skeleton.c. When there is no data in the buffer, we would allow retry for both blocking and nonblocking cases. Original logic give retry only for blocking case. Actually we can also allow retry for nonblocking case. This will reuse the existing retry logic and handle the return of -EAGAIN in one place. Also if the data to be read is short and can be retrieved in quick time, we can also give a chance for nonblocking case and may catch the data and copy it back to userspace in one read() call too. Signed-off-by: Chen Wang unicornxx.w...@gmail.com --- --- linux-3.11-rc1/drivers/usb/usb-skeleton.c.orig 2013-07-18 19:35:23.559780152 +0800 +++ linux-3.11-rc1/drivers/usb/usb-skeleton.c 2013-07-18 19:38:11.546779516 +0800 @@ -325,9 +325,8 @@ retry: rv = skel_do_read_io(dev, count); if (rv 0) goto exit; - else if (!(file-f_flags O_NONBLOCK)) + else goto retry; - rv = -EAGAIN; } exit: mutex_unlock(dev-io_mutex); -- 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: usb-skeleton.c: add retry for nonblocking read
Thanks, I resent my patch today after reading the doc, it does help. The email still use the same subject, not sure if this will submerge my latest submission, do I need to use a new subject, though I have not thought of a better new one. Anyway, appreciated all of your great help! On Thu, 2013-07-18 at 08:46 -0700, Greg KH wrote: On Thu, Jul 18, 2013 at 10:34:33PM +0800, Chen Wang wrote: Opps, I find my email address is still wrong. Let me correct it and post again. The mailing list does not accept html messages, and I can't accept a patch in html format. Please read the file, Documentation/email_clients.txt for some hints on how to fix your client to send patches in a format that we can accept them in. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]USB: usb-skeleton.c: add retry for nonblocking read
On Fri, Jul 19, 2013 at 10:20:07AM +0800, Chen Wang wrote: Thanks, I resent my patch today after reading the doc, it does help. The email still use the same subject, not sure if this will submerge my latest submission, do I need to use a new subject, though I have not thought of a better new one. Anyway, appreciated all of your great help! No problem, I got the patch, I'll queue it up in a few days, give me a chance to catch up with my pending patch queue. Don't worry, it's not lost. 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] [RFC] EHCI: add to memory barrier to updating hw_next
On Thu, Jul 18, 2013 at 10:08 PM, Alan Stern st...@rowland.harvard.edu wrote:
On Thu, 18 Jul 2013, Ming Lei wrote:
I guess that HC could have a use-after-free problem like following
situation.
1. A qtd which is not at the queue head should be removed in
qh_completions().
2. The last-hw_next become be pointing at the next qtd but the hw_next
value is delayed in write-buffer.
3. The qtd is removed in the list.
4. The qtd is freed into DMA pool and re-allocated for another urb.
5. HC try to process last-hw_next and it is pointing re-allocated qtd.
What do you think about it? Is it possible?
I understand it might not be possible because: when 'stopped' is set, that
said the HC might not advance the queue. But I don't understand why
'last-hw_next' is patched here under 'stopped' situation.
It should not be possible. When stopped is set, the QH gets unlinked
and relinked before it can start up again. Relinking involves some
memory barriers, so the qTD will not be accessed again by the HC.
last-hw_next gets patched because the qTD might belong to some URB in
the middle of the queue that is being unlinked. The URBs before it and
after it will still be active, so the queue link has to be updated.
'stopped' is set under below situations:
- unlink over or shutdown
- halt
- short packet(URB_SHORT_NOT_OK)
Looks EHCI won't advance the queue(qh) any more on above situations, so I
think last-hw_next might not need patching.
Even the 'stopped' case may be seldom triggered, do you know under
which condition the stopped is triggered in your problem?(stall, short read
or others)
I was going to ask the same question. This particular piece of code
gets executed _only_ when an URB is unlinked. Not during any other
kind of error.
The code may run under 'halt' or short packet(URB_SHORT_NOT_OK) too.
If Gioh's problem falls to these two situations, below patch might be helpful.
Because the qh will be unlinked when there is fault in the endpoint(halt, short
packet), maybe it is safer to complete these URBs after the qh becomes
unlinked, like what the blew patch does:
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index b637a65..6a65e0a 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -454,6 +454,10 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
}
}
+ /* complete URBs after unlinking */
+ if (stopped state != QH_STATE_IDLE)
+ goto exit;
+
/* unless we already know the urb's status, collect qtd status
* and update count of bytes transferred. in common short read
* cases with only one data qtd (including control transfers),
@@ -489,15 +493,6 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
}
}
- /* if we're removing something not at the queue head,
-* patch the hardware queue pointer.
-*/
- if (stopped qtd-qtd_list.prev != qh-qtd_list) {
- last = list_entry (qtd-qtd_list.prev,
- struct ehci_qtd, qtd_list);
- last-hw_next = qtd-hw_next;
- }
-
/* remove qtd; it's recycled after possible urb completion */
list_del (qtd-qtd_list);
last = qtd;
@@ -520,7 +515,7 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
/* Otherwise the caller must unlink the QH. */
}
-
+ exit:
/* restore original state; caller must unlink or relink */
qh-qh_state = state;
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: class: move checking 'actual' code block into checking 'buffer[1]' code block
Hello Maintainers:
Please help check this patch when you have time.
Thanks.
On 07/11/2013 09:08 AM, Chen Gang wrote:
Hello Maintainers:
Please help check this patch when you have time, thanks.
BTW: this uninitialized variable warning may not be found by gcc
compiler (which a gcc bug exists almost 10 years).
Thanks.
On 07/02/2013 12:06 PM, Chen Gang wrote:
The variable 'actual' is only used in checking 'buffer[1]' code
block, so need move it into, or it may not be initialized.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/class/usbtmc.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 609dbc2..42d62c9 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -786,7 +786,7 @@ usbtmc_clear_check_status:
goto exit;
}
-if (buffer[1] == 1)
+if (buffer[1] == 1) {
do {
dev_dbg(dev, Reading from bulk in EP\n);
@@ -805,11 +805,13 @@ usbtmc_clear_check_status:
} while ((actual == max_size)
(n USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
-if (actual == max_size) {
-dev_err(dev, Couldn't clear device buffer within %d cycles\n,
-USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
-rv = -EPERM;
-goto exit;
+if (actual == max_size) {
+dev_err(dev,
+Couldn't clear device buffer within %d
cycles\n,
+USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
+rv = -EPERM;
+goto exit;
+}
}
goto usbtmc_clear_check_status;
--
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: class: move checking 'actual' code block into checking 'buffer[1]' code block
Hi Chen Gang, On Fri, Jul 19, 2013 at 12:21 PM, Chen Gang gang.c...@asianux.com wrote: Hello Maintainers: Please help check this patch when you have time. Looks your patch is correct, and I think Greg will handle your patch when usb-next tree is open. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: class: move checking 'actual' code block into checking 'buffer[1]' code block
On 07/19/2013 12:43 PM, Ming Lei wrote: Hi Chen Gang, On Fri, Jul 19, 2013 at 12:21 PM, Chen Gang gang.c...@asianux.com wrote: Hello Maintainers: Please help check this patch when you have time. Looks your patch is correct, and I think Greg will handle your patch when usb-next tree is open. Thank you very much for your information. Thanks, -- Ming Lei -- 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 -- Chen Gang -- 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
