All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] USB: OMAP1: Tahvo USB support for 770
@ 2013-06-18 17:06 ` Aaro Koskinen
  0 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:06 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Aaro Koskinen

Hi,

These patches add support for Tahvo USB transceiver and allow using both
host and peripheral modes on Nokia 770.

Tested (peripheral mode, host mode, vbus detection) with 3.10-rc6.

History:
	v3: Delete accidental #include <mach/usb.h> from patch 3.
	    Drop board file changes, already queued to linux-omap.
	v2: http://marc.info/?l=linux-omap&m=137138976406242&w=2
	    Use extcon framework to trigger OTG driver mode changes.
	v1: http://marc.info/?l=linux-omap&m=137081763029385&w=2

Earlier RFC versions:
	http://marc.info/?l=linux-omap&m=136553710000679&w=2
	http://marc.info/?l=linux-omap&m=136266725827698&w=2

Aaro Koskinen (4):
  ARM: OMAP1: USB: move omap_usb_config to platform data
  USB: OMAP1: add extcon to platform data
  USB: OMAP1: OTG controller driver
  USB: OMAP1: Tahvo USB transceiver driver

 arch/arm/mach-omap1/include/mach/usb.h  |  38 +--
 drivers/usb/phy/Kconfig                 |  24 ++
 drivers/usb/phy/Makefile                |   2 +
 drivers/usb/phy/phy-omap-otg.c          | 169 ++++++++++++
 drivers/usb/phy/phy-tahvo.c             | 448 ++++++++++++++++++++++++++++++++
 include/linux/platform_data/usb-omap1.h |  53 ++++
 6 files changed, 697 insertions(+), 37 deletions(-)
 create mode 100644 drivers/usb/phy/phy-omap-otg.c
 create mode 100644 drivers/usb/phy/phy-tahvo.c
 create mode 100644 include/linux/platform_data/usb-omap1.h

A.

-- 
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 0/4] USB: OMAP1: Tahvo USB support for 770
@ 2013-06-18 17:06 ` Aaro Koskinen
  0 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

These patches add support for Tahvo USB transceiver and allow using both
host and peripheral modes on Nokia 770.

Tested (peripheral mode, host mode, vbus detection) with 3.10-rc6.

History:
	v3: Delete accidental #include <mach/usb.h> from patch 3.
	    Drop board file changes, already queued to linux-omap.
	v2: http://marc.info/?l=linux-omap&m=137138976406242&w=2
	    Use extcon framework to trigger OTG driver mode changes.
	v1: http://marc.info/?l=linux-omap&m=137081763029385&w=2

Earlier RFC versions:
	http://marc.info/?l=linux-omap&m=136553710000679&w=2
	http://marc.info/?l=linux-omap&m=136266725827698&w=2

Aaro Koskinen (4):
  ARM: OMAP1: USB: move omap_usb_config to platform data
  USB: OMAP1: add extcon to platform data
  USB: OMAP1: OTG controller driver
  USB: OMAP1: Tahvo USB transceiver driver

 arch/arm/mach-omap1/include/mach/usb.h  |  38 +--
 drivers/usb/phy/Kconfig                 |  24 ++
 drivers/usb/phy/Makefile                |   2 +
 drivers/usb/phy/phy-omap-otg.c          | 169 ++++++++++++
 drivers/usb/phy/phy-tahvo.c             | 448 ++++++++++++++++++++++++++++++++
 include/linux/platform_data/usb-omap1.h |  53 ++++
 6 files changed, 697 insertions(+), 37 deletions(-)
 create mode 100644 drivers/usb/phy/phy-omap-otg.c
 create mode 100644 drivers/usb/phy/phy-tahvo.c
 create mode 100644 include/linux/platform_data/usb-omap1.h

A.

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/4] ARM: OMAP1: USB: move omap_usb_config to platform data
  2013-06-18 17:06 ` Aaro Koskinen
@ 2013-06-18 17:06   ` Aaro Koskinen
  -1 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:06 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb, linux-omap, linux-arm-kernel; +Cc: Aaro Koskinen

Move omap_usb_config to platform data, so that OTG driver can include it.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap1/include/mach/usb.h  | 38 +-----------------------
 include/linux/platform_data/usb-omap1.h | 51 +++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 37 deletions(-)
 create mode 100644 include/linux/platform_data/usb-omap1.h

diff --git a/arch/arm/mach-omap1/include/mach/usb.h b/arch/arm/mach-omap1/include/mach/usb.h
index 45e5ac7..2c26305 100644
--- a/arch/arm/mach-omap1/include/mach/usb.h
+++ b/arch/arm/mach-omap1/include/mach/usb.h
@@ -8,43 +8,7 @@
 #define	is_usb0_device(config)	0
 #endif
 
-struct omap_usb_config {
-	/* Configure drivers according to the connectors on your board:
-	 *  - "A" connector (rectagular)
-	 *	... for host/OHCI use, set "register_host".
-	 *  - "B" connector (squarish) or "Mini-B"
-	 *	... for device/gadget use, set "register_dev".
-	 *  - "Mini-AB" connector (very similar to Mini-B)
-	 *	... for OTG use as device OR host, initialize "otg"
-	 */
-	unsigned	register_host:1;
-	unsigned	register_dev:1;
-	u8		otg;	/* port number, 1-based:  usb1 == 2 */
-
-	u8		hmc_mode;
-
-	/* implicitly true if otg:  host supports remote wakeup? */
-	u8		rwc;
-
-	/* signaling pins used to talk to transceiver on usbN:
-	 *  0 == usbN unused
-	 *  2 == usb0-only, using internal transceiver
-	 *  3 == 3 wire bidirectional
-	 *  4 == 4 wire bidirectional
-	 *  6 == 6 wire unidirectional (or TLL)
-	 */
-	u8		pins[3];
-
-	struct platform_device *udc_device;
-	struct platform_device *ohci_device;
-	struct platform_device *otg_device;
-
-	u32 (*usb0_init)(unsigned nwires, unsigned is_device);
-	u32 (*usb1_init)(unsigned nwires);
-	u32 (*usb2_init)(unsigned nwires, unsigned alt_pingroup);
-
-	int (*ocpi_enable)(void);
-};
+#include <linux/platform_data/usb-omap1.h>
 
 void omap_otg_init(struct omap_usb_config *config);
 
diff --git a/include/linux/platform_data/usb-omap1.h b/include/linux/platform_data/usb-omap1.h
new file mode 100644
index 0000000..8c7764d
--- /dev/null
+++ b/include/linux/platform_data/usb-omap1.h
@@ -0,0 +1,51 @@
+/*
+ * Platform data for OMAP1 USB
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive for
+ * more details.
+ */
+#ifndef __LINUX_USB_OMAP1_H
+#define __LINUX_USB_OMAP1_H
+
+#include <linux/platform_device.h>
+
+struct omap_usb_config {
+	/* Configure drivers according to the connectors on your board:
+	 *  - "A" connector (rectagular)
+	 *	... for host/OHCI use, set "register_host".
+	 *  - "B" connector (squarish) or "Mini-B"
+	 *	... for device/gadget use, set "register_dev".
+	 *  - "Mini-AB" connector (very similar to Mini-B)
+	 *	... for OTG use as device OR host, initialize "otg"
+	 */
+	unsigned	register_host:1;
+	unsigned	register_dev:1;
+	u8		otg;	/* port number, 1-based:  usb1 == 2 */
+
+	u8		hmc_mode;
+
+	/* implicitly true if otg:  host supports remote wakeup? */
+	u8		rwc;
+
+	/* signaling pins used to talk to transceiver on usbN:
+	 *  0 == usbN unused
+	 *  2 == usb0-only, using internal transceiver
+	 *  3 == 3 wire bidirectional
+	 *  4 == 4 wire bidirectional
+	 *  6 == 6 wire unidirectional (or TLL)
+	 */
+	u8		pins[3];
+
+	struct platform_device *udc_device;
+	struct platform_device *ohci_device;
+	struct platform_device *otg_device;
+
+	u32 (*usb0_init)(unsigned nwires, unsigned is_device);
+	u32 (*usb1_init)(unsigned nwires);
+	u32 (*usb2_init)(unsigned nwires, unsigned alt_pingroup);
+
+	int (*ocpi_enable)(void);
+};
+
+#endif /* __LINUX_USB_OMAP1_H */
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 1/4] ARM: OMAP1: USB: move omap_usb_config to platform data
@ 2013-06-18 17:06   ` Aaro Koskinen
  0 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:06 UTC (permalink / raw)
  To: linux-arm-kernel

Move omap_usb_config to platform data, so that OTG driver can include it.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap1/include/mach/usb.h  | 38 +-----------------------
 include/linux/platform_data/usb-omap1.h | 51 +++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 37 deletions(-)
 create mode 100644 include/linux/platform_data/usb-omap1.h

diff --git a/arch/arm/mach-omap1/include/mach/usb.h b/arch/arm/mach-omap1/include/mach/usb.h
index 45e5ac7..2c26305 100644
--- a/arch/arm/mach-omap1/include/mach/usb.h
+++ b/arch/arm/mach-omap1/include/mach/usb.h
@@ -8,43 +8,7 @@
 #define	is_usb0_device(config)	0
 #endif
 
-struct omap_usb_config {
-	/* Configure drivers according to the connectors on your board:
-	 *  - "A" connector (rectagular)
-	 *	... for host/OHCI use, set "register_host".
-	 *  - "B" connector (squarish) or "Mini-B"
-	 *	... for device/gadget use, set "register_dev".
-	 *  - "Mini-AB" connector (very similar to Mini-B)
-	 *	... for OTG use as device OR host, initialize "otg"
-	 */
-	unsigned	register_host:1;
-	unsigned	register_dev:1;
-	u8		otg;	/* port number, 1-based:  usb1 == 2 */
-
-	u8		hmc_mode;
-
-	/* implicitly true if otg:  host supports remote wakeup? */
-	u8		rwc;
-
-	/* signaling pins used to talk to transceiver on usbN:
-	 *  0 == usbN unused
-	 *  2 == usb0-only, using internal transceiver
-	 *  3 == 3 wire bidirectional
-	 *  4 == 4 wire bidirectional
-	 *  6 == 6 wire unidirectional (or TLL)
-	 */
-	u8		pins[3];
-
-	struct platform_device *udc_device;
-	struct platform_device *ohci_device;
-	struct platform_device *otg_device;
-
-	u32 (*usb0_init)(unsigned nwires, unsigned is_device);
-	u32 (*usb1_init)(unsigned nwires);
-	u32 (*usb2_init)(unsigned nwires, unsigned alt_pingroup);
-
-	int (*ocpi_enable)(void);
-};
+#include <linux/platform_data/usb-omap1.h>
 
 void omap_otg_init(struct omap_usb_config *config);
 
diff --git a/include/linux/platform_data/usb-omap1.h b/include/linux/platform_data/usb-omap1.h
new file mode 100644
index 0000000..8c7764d
--- /dev/null
+++ b/include/linux/platform_data/usb-omap1.h
@@ -0,0 +1,51 @@
+/*
+ * Platform data for OMAP1 USB
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive for
+ * more details.
+ */
+#ifndef __LINUX_USB_OMAP1_H
+#define __LINUX_USB_OMAP1_H
+
+#include <linux/platform_device.h>
+
+struct omap_usb_config {
+	/* Configure drivers according to the connectors on your board:
+	 *  - "A" connector (rectagular)
+	 *	... for host/OHCI use, set "register_host".
+	 *  - "B" connector (squarish) or "Mini-B"
+	 *	... for device/gadget use, set "register_dev".
+	 *  - "Mini-AB" connector (very similar to Mini-B)
+	 *	... for OTG use as device OR host, initialize "otg"
+	 */
+	unsigned	register_host:1;
+	unsigned	register_dev:1;
+	u8		otg;	/* port number, 1-based:  usb1 == 2 */
+
+	u8		hmc_mode;
+
+	/* implicitly true if otg:  host supports remote wakeup? */
+	u8		rwc;
+
+	/* signaling pins used to talk to transceiver on usbN:
+	 *  0 == usbN unused
+	 *  2 == usb0-only, using internal transceiver
+	 *  3 == 3 wire bidirectional
+	 *  4 == 4 wire bidirectional
+	 *  6 == 6 wire unidirectional (or TLL)
+	 */
+	u8		pins[3];
+
+	struct platform_device *udc_device;
+	struct platform_device *ohci_device;
+	struct platform_device *otg_device;
+
+	u32 (*usb0_init)(unsigned nwires, unsigned is_device);
+	u32 (*usb1_init)(unsigned nwires);
+	u32 (*usb2_init)(unsigned nwires, unsigned alt_pingroup);
+
+	int (*ocpi_enable)(void);
+};
+
+#endif /* __LINUX_USB_OMAP1_H */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/4] USB: OMAP1: add extcon to platform data
  2013-06-18 17:06 ` Aaro Koskinen
@ 2013-06-18 17:06     ` Aaro Koskinen
  -1 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:06 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Aaro Koskinen

Add extcon field to platform data.

Signed-off-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>
---
 include/linux/platform_data/usb-omap1.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/platform_data/usb-omap1.h b/include/linux/platform_data/usb-omap1.h
index 8c7764d..43b5ce1 100644
--- a/include/linux/platform_data/usb-omap1.h
+++ b/include/linux/platform_data/usb-omap1.h
@@ -23,6 +23,8 @@ struct omap_usb_config {
 	unsigned	register_dev:1;
 	u8		otg;	/* port number, 1-based:  usb1 == 2 */
 
+	const char	*extcon;	/* extcon device for OTG */
+
 	u8		hmc_mode;
 
 	/* implicitly true if otg:  host supports remote wakeup? */
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/4] USB: OMAP1: add extcon to platform data
@ 2013-06-18 17:06     ` Aaro Koskinen
  0 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:06 UTC (permalink / raw)
  To: linux-arm-kernel

Add extcon field to platform data.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 include/linux/platform_data/usb-omap1.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/platform_data/usb-omap1.h b/include/linux/platform_data/usb-omap1.h
index 8c7764d..43b5ce1 100644
--- a/include/linux/platform_data/usb-omap1.h
+++ b/include/linux/platform_data/usb-omap1.h
@@ -23,6 +23,8 @@ struct omap_usb_config {
 	unsigned	register_dev:1;
 	u8		otg;	/* port number, 1-based:  usb1 == 2 */
 
+	const char	*extcon;	/* extcon device for OTG */
+
 	u8		hmc_mode;
 
 	/* implicitly true if otg:  host supports remote wakeup? */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/4] USB: OMAP1: OTG controller driver
  2013-06-18 17:06 ` Aaro Koskinen
@ 2013-06-18 17:07     ` Aaro Koskinen
  -1 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:07 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Aaro Koskinen

Transceivers need to manage OTG controller state on OMAP1 to enable
switching between peripheral and host modes. Provide a driver for that.

Signed-off-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>
---
 drivers/usb/phy/Kconfig        |  10 +++
 drivers/usb/phy/Makefile       |   1 +
 drivers/usb/phy/phy-omap-otg.c | 169 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 drivers/usb/phy/phy-omap-otg.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 7ef3eb8..14a50bd 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -135,6 +135,16 @@ config USB_GPIO_VBUS
 	  optionally control of a D+ pullup GPIO as well as a VBUS
 	  current limit regulator.
 
+config OMAP_OTG
+	tristate "OMAP USB OTG controller driver"
+	depends on ARCH_OMAP_OTG && EXTCON
+	help
+	  Enable this to support some transceivers on OMAP1 platforms. OTG
+	  controller is needed to switch between host and peripheral modes.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called omap-otg.
+
 config USB_ISP1301
 	tristate "NXP ISP1301 USB transceiver support"
 	depends on USB || USB_GADGET
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index a9169cb..c7f391b 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ISP1301_OMAP)		+= phy-isp1301-omap.o
 obj-$(CONFIG_MV_U3D_PHY)		+= phy-mv-u3d-usb.o
 obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-nop.o
 obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
+obj-$(CONFIG_OMAP_OTG)			+= phy-omap-otg.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_OMAP_USB3)			+= phy-omap-usb3.o
 obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c
new file mode 100644
index 0000000..11598cd
--- /dev/null
+++ b/drivers/usb/phy/phy-omap-otg.c
@@ -0,0 +1,169 @@
+/*
+ * OMAP OTG controller driver
+ *
+ * Based on code from tahvo-usb.c and isp1301_omap.c drivers.
+ *
+ * Copyright (C) 2005-2006 Nokia Corporation
+ * Copyright (C) 2004 Texas Instruments
+ * Copyright (C) 2004 David Brownell
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/usb-omap1.h>
+
+struct otg_device {
+	void __iomem			*base;
+	bool				id;
+	bool				vbus;
+	struct extcon_specific_cable_nb	vbus_dev;
+	struct extcon_specific_cable_nb	id_dev;
+	struct notifier_block		vbus_nb;
+	struct notifier_block		id_nb;
+};
+
+#define OMAP_OTG_CTRL		0x0c
+#define OMAP_OTG_ASESSVLD	(1 << 20)
+#define OMAP_OTG_BSESSEND	(1 << 19)
+#define OMAP_OTG_BSESSVLD	(1 << 18)
+#define OMAP_OTG_VBUSVLD	(1 << 17)
+#define OMAP_OTG_ID		(1 << 16)
+#define OMAP_OTG_XCEIV_OUTPUTS \
+	(OMAP_OTG_ASESSVLD | OMAP_OTG_BSESSEND | OMAP_OTG_BSESSVLD | \
+	 OMAP_OTG_VBUSVLD  | OMAP_OTG_ID)
+
+static void omap_otg_ctrl(struct otg_device *otg_dev, u32 outputs)
+{
+	u32 l;
+
+	l = readl(otg_dev->base + OMAP_OTG_CTRL);
+	l &= ~OMAP_OTG_XCEIV_OUTPUTS;
+	l |= outputs;
+	writel(l, otg_dev->base + OMAP_OTG_CTRL);
+}
+
+static void omap_otg_set_mode(struct otg_device *otg_dev)
+{
+	if (!otg_dev->id && otg_dev->vbus)
+		/* Set B-session valid. */
+		omap_otg_ctrl(otg_dev, OMAP_OTG_ID | OMAP_OTG_BSESSVLD);
+	else if (otg_dev->vbus)
+		/* Set A-session valid. */
+		omap_otg_ctrl(otg_dev, OMAP_OTG_ASESSVLD);
+	else if (!otg_dev->id)
+		/* Set B-session end to indicate no VBUS. */
+		omap_otg_ctrl(otg_dev, OMAP_OTG_ID | OMAP_OTG_BSESSEND);
+}
+
+static int omap_otg_id_notifier(struct notifier_block *nb,
+				unsigned long event, void *ptr)
+{
+	struct otg_device *otg_dev = container_of(nb, struct otg_device, id_nb);
+
+	otg_dev->id = event;
+	omap_otg_set_mode(otg_dev);
+
+	return NOTIFY_DONE;
+}
+
+static int omap_otg_vbus_notifier(struct notifier_block *nb,
+				  unsigned long event, void *ptr)
+{
+	struct otg_device *otg_dev = container_of(nb, struct otg_device,
+						  vbus_nb);
+
+	otg_dev->vbus = event;
+	omap_otg_set_mode(otg_dev);
+
+	return NOTIFY_DONE;
+}
+
+static int omap_otg_probe(struct platform_device *pdev)
+{
+	const struct omap_usb_config *config = pdev->dev.platform_data;
+	struct otg_device *otg_dev;
+	struct extcon_dev *extcon;
+	int ret;
+	u32 rev;
+
+	if (!config || !config->extcon)
+		return -ENODEV;
+
+	extcon = extcon_get_extcon_dev(config->extcon);
+	if (!extcon)
+		return -EPROBE_DEFER;
+
+	otg_dev = devm_kzalloc(&pdev->dev, sizeof(*otg_dev), GFP_KERNEL);
+	if (!otg_dev)
+		return -ENOMEM;
+
+	otg_dev->base = devm_ioremap_resource(&pdev->dev, &pdev->resource[0]);
+	if (IS_ERR(otg_dev->base))
+		return PTR_ERR(otg_dev->base);
+
+	otg_dev->id_nb.notifier_call = omap_otg_id_notifier;
+	otg_dev->vbus_nb.notifier_call = omap_otg_vbus_notifier;
+
+	ret = extcon_register_interest(&otg_dev->id_dev, config->extcon,
+				       "USB-HOST", &otg_dev->id_nb);
+	if (ret)
+		return ret;
+
+	ret = extcon_register_interest(&otg_dev->vbus_dev, config->extcon,
+				       "USB", &otg_dev->vbus_nb);
+	if (ret) {
+		extcon_unregister_interest(&otg_dev->id_dev);
+		return ret;
+	}
+
+	otg_dev->id = extcon_get_cable_state(extcon, "USB-HOST");
+	otg_dev->vbus = extcon_get_cable_state(extcon, "USB");
+	omap_otg_set_mode(otg_dev);
+
+	rev = readl(otg_dev->base);
+
+	dev_info(&pdev->dev,
+		 "OMAP USB OTG controller rev %d.%d (%s, id=%d, vbus=%d)\n",
+		 (rev >> 4) & 0xf, rev & 0xf, config->extcon, otg_dev->id,
+		 otg_dev->vbus);
+
+	return 0;
+}
+
+static int omap_otg_remove(struct platform_device *pdev)
+{
+	struct otg_device *otg_dev = platform_get_drvdata(pdev);
+
+	extcon_unregister_interest(&otg_dev->id_dev);
+	extcon_unregister_interest(&otg_dev->vbus_dev);
+
+	return 0;
+}
+
+static struct platform_driver omap_otg_driver = {
+	.probe		= omap_otg_probe,
+	.remove		= omap_otg_remove,
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "omap_otg",
+	},
+};
+module_platform_driver(omap_otg_driver);
+
+MODULE_DESCRIPTION("OMAP USB OTG controller driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>");
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/4] USB: OMAP1: OTG controller driver
@ 2013-06-18 17:07     ` Aaro Koskinen
  0 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:07 UTC (permalink / raw)
  To: linux-arm-kernel

Transceivers need to manage OTG controller state on OMAP1 to enable
switching between peripheral and host modes. Provide a driver for that.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/usb/phy/Kconfig        |  10 +++
 drivers/usb/phy/Makefile       |   1 +
 drivers/usb/phy/phy-omap-otg.c | 169 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 drivers/usb/phy/phy-omap-otg.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 7ef3eb8..14a50bd 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -135,6 +135,16 @@ config USB_GPIO_VBUS
 	  optionally control of a D+ pullup GPIO as well as a VBUS
 	  current limit regulator.
 
+config OMAP_OTG
+	tristate "OMAP USB OTG controller driver"
+	depends on ARCH_OMAP_OTG && EXTCON
+	help
+	  Enable this to support some transceivers on OMAP1 platforms. OTG
+	  controller is needed to switch between host and peripheral modes.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called omap-otg.
+
 config USB_ISP1301
 	tristate "NXP ISP1301 USB transceiver support"
 	depends on USB || USB_GADGET
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index a9169cb..c7f391b 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ISP1301_OMAP)		+= phy-isp1301-omap.o
 obj-$(CONFIG_MV_U3D_PHY)		+= phy-mv-u3d-usb.o
 obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-nop.o
 obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
+obj-$(CONFIG_OMAP_OTG)			+= phy-omap-otg.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_OMAP_USB3)			+= phy-omap-usb3.o
 obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c
new file mode 100644
index 0000000..11598cd
--- /dev/null
+++ b/drivers/usb/phy/phy-omap-otg.c
@@ -0,0 +1,169 @@
+/*
+ * OMAP OTG controller driver
+ *
+ * Based on code from tahvo-usb.c and isp1301_omap.c drivers.
+ *
+ * Copyright (C) 2005-2006 Nokia Corporation
+ * Copyright (C) 2004 Texas Instruments
+ * Copyright (C) 2004 David Brownell
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/usb-omap1.h>
+
+struct otg_device {
+	void __iomem			*base;
+	bool				id;
+	bool				vbus;
+	struct extcon_specific_cable_nb	vbus_dev;
+	struct extcon_specific_cable_nb	id_dev;
+	struct notifier_block		vbus_nb;
+	struct notifier_block		id_nb;
+};
+
+#define OMAP_OTG_CTRL		0x0c
+#define OMAP_OTG_ASESSVLD	(1 << 20)
+#define OMAP_OTG_BSESSEND	(1 << 19)
+#define OMAP_OTG_BSESSVLD	(1 << 18)
+#define OMAP_OTG_VBUSVLD	(1 << 17)
+#define OMAP_OTG_ID		(1 << 16)
+#define OMAP_OTG_XCEIV_OUTPUTS \
+	(OMAP_OTG_ASESSVLD | OMAP_OTG_BSESSEND | OMAP_OTG_BSESSVLD | \
+	 OMAP_OTG_VBUSVLD  | OMAP_OTG_ID)
+
+static void omap_otg_ctrl(struct otg_device *otg_dev, u32 outputs)
+{
+	u32 l;
+
+	l = readl(otg_dev->base + OMAP_OTG_CTRL);
+	l &= ~OMAP_OTG_XCEIV_OUTPUTS;
+	l |= outputs;
+	writel(l, otg_dev->base + OMAP_OTG_CTRL);
+}
+
+static void omap_otg_set_mode(struct otg_device *otg_dev)
+{
+	if (!otg_dev->id && otg_dev->vbus)
+		/* Set B-session valid. */
+		omap_otg_ctrl(otg_dev, OMAP_OTG_ID | OMAP_OTG_BSESSVLD);
+	else if (otg_dev->vbus)
+		/* Set A-session valid. */
+		omap_otg_ctrl(otg_dev, OMAP_OTG_ASESSVLD);
+	else if (!otg_dev->id)
+		/* Set B-session end to indicate no VBUS. */
+		omap_otg_ctrl(otg_dev, OMAP_OTG_ID | OMAP_OTG_BSESSEND);
+}
+
+static int omap_otg_id_notifier(struct notifier_block *nb,
+				unsigned long event, void *ptr)
+{
+	struct otg_device *otg_dev = container_of(nb, struct otg_device, id_nb);
+
+	otg_dev->id = event;
+	omap_otg_set_mode(otg_dev);
+
+	return NOTIFY_DONE;
+}
+
+static int omap_otg_vbus_notifier(struct notifier_block *nb,
+				  unsigned long event, void *ptr)
+{
+	struct otg_device *otg_dev = container_of(nb, struct otg_device,
+						  vbus_nb);
+
+	otg_dev->vbus = event;
+	omap_otg_set_mode(otg_dev);
+
+	return NOTIFY_DONE;
+}
+
+static int omap_otg_probe(struct platform_device *pdev)
+{
+	const struct omap_usb_config *config = pdev->dev.platform_data;
+	struct otg_device *otg_dev;
+	struct extcon_dev *extcon;
+	int ret;
+	u32 rev;
+
+	if (!config || !config->extcon)
+		return -ENODEV;
+
+	extcon = extcon_get_extcon_dev(config->extcon);
+	if (!extcon)
+		return -EPROBE_DEFER;
+
+	otg_dev = devm_kzalloc(&pdev->dev, sizeof(*otg_dev), GFP_KERNEL);
+	if (!otg_dev)
+		return -ENOMEM;
+
+	otg_dev->base = devm_ioremap_resource(&pdev->dev, &pdev->resource[0]);
+	if (IS_ERR(otg_dev->base))
+		return PTR_ERR(otg_dev->base);
+
+	otg_dev->id_nb.notifier_call = omap_otg_id_notifier;
+	otg_dev->vbus_nb.notifier_call = omap_otg_vbus_notifier;
+
+	ret = extcon_register_interest(&otg_dev->id_dev, config->extcon,
+				       "USB-HOST", &otg_dev->id_nb);
+	if (ret)
+		return ret;
+
+	ret = extcon_register_interest(&otg_dev->vbus_dev, config->extcon,
+				       "USB", &otg_dev->vbus_nb);
+	if (ret) {
+		extcon_unregister_interest(&otg_dev->id_dev);
+		return ret;
+	}
+
+	otg_dev->id = extcon_get_cable_state(extcon, "USB-HOST");
+	otg_dev->vbus = extcon_get_cable_state(extcon, "USB");
+	omap_otg_set_mode(otg_dev);
+
+	rev = readl(otg_dev->base);
+
+	dev_info(&pdev->dev,
+		 "OMAP USB OTG controller rev %d.%d (%s, id=%d, vbus=%d)\n",
+		 (rev >> 4) & 0xf, rev & 0xf, config->extcon, otg_dev->id,
+		 otg_dev->vbus);
+
+	return 0;
+}
+
+static int omap_otg_remove(struct platform_device *pdev)
+{
+	struct otg_device *otg_dev = platform_get_drvdata(pdev);
+
+	extcon_unregister_interest(&otg_dev->id_dev);
+	extcon_unregister_interest(&otg_dev->vbus_dev);
+
+	return 0;
+}
+
+static struct platform_driver omap_otg_driver = {
+	.probe		= omap_otg_probe,
+	.remove		= omap_otg_remove,
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "omap_otg",
+	},
+};
+module_platform_driver(omap_otg_driver);
+
+MODULE_DESCRIPTION("OMAP USB OTG controller driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Aaro Koskinen <aaro.koskinen@iki.fi>");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
  2013-06-18 17:06 ` Aaro Koskinen
@ 2013-06-18 17:07     ` Aaro Koskinen
  -1 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:07 UTC (permalink / raw)
  To: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Aaro Koskinen

Add Tahvo USB transceiver driver.

Based on old code from linux-omap tree. The original driver was written
by Juha Yrjölä, Tony Lindgren, and Timo Teräs.

Signed-off-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>
---
 drivers/usb/phy/Kconfig     |  14 ++
 drivers/usb/phy/Makefile    |   1 +
 drivers/usb/phy/phy-tahvo.c | 448 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 463 insertions(+)
 create mode 100644 drivers/usb/phy/phy-tahvo.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 14a50bd..a5c7937 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -145,6 +145,20 @@ config OMAP_OTG
 	  This driver can also be built as a module. If so, the module
 	  will be called omap-otg.
 
+config TAHVO_USB
+	tristate "Tahvo USB transceiver driver"
+	depends on MFD_RETU && EXTCON
+	help
+	  Enable this to support USB transceiver on Tahvo. This is used
+	  at least on Nokia 770.
+
+config TAHVO_USB_HOST_BY_DEFAULT
+	depends on TAHVO_USB
+	boolean "Device in USB host mode by default"
+	help
+	  Say Y here, if you want the device to enter USB host mode
+	  by default on bootup.
+
 config USB_ISP1301
 	tristate "NXP ISP1301 USB transceiver support"
 	depends on USB || USB_GADGET
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index c7f391b..f3984d1 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -13,6 +13,7 @@ phy-fsl-usb2-objs			:= phy-fsl-usb.o phy-fsm-usb.o
 obj-$(CONFIG_FSL_USB2_OTG)		+= phy-fsl-usb2.o
 obj-$(CONFIG_ISP1301_OMAP)		+= phy-isp1301-omap.o
 obj-$(CONFIG_MV_U3D_PHY)		+= phy-mv-u3d-usb.o
+obj-$(CONFIG_TAHVO_USB)			+= phy-tahvo.o
 obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-nop.o
 obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
 obj-$(CONFIG_OMAP_OTG)			+= phy-omap-otg.o
diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
new file mode 100644
index 0000000..a2d96a2
--- /dev/null
+++ b/drivers/usb/phy/phy-tahvo.c
@@ -0,0 +1,448 @@
+/*
+ * Tahvo USB transceiver driver
+ *
+ * Copyright (C) 2005-2006 Nokia Corporation
+ *
+ * Parts copied from isp1301_omap.c.
+ * Copyright (C) 2004 Texas Instruments
+ * Copyright (C) 2004 David Brownell
+ *
+ * Original driver written by Juha Yrjölä, Tony Lindgren and Timo Teräs.
+ * Modified for Retu/Tahvo MFD by Aaro Koskinen.
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/usb.h>
+#include <linux/extcon.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb/otg.h>
+#include <linux/mfd/retu.h>
+#include <linux/usb/gadget.h>
+#include <linux/platform_device.h>
+
+#define DRIVER_NAME     "tahvo-usb"
+
+#define TAHVO_REG_IDSR	0x02
+#define TAHVO_REG_USBR	0x06
+
+#define USBR_SLAVE_CONTROL	(1 << 8)
+#define USBR_VPPVIO_SW		(1 << 7)
+#define USBR_SPEED		(1 << 6)
+#define USBR_REGOUT		(1 << 5)
+#define USBR_MASTER_SW2		(1 << 4)
+#define USBR_MASTER_SW1		(1 << 3)
+#define USBR_SLAVE_SW		(1 << 2)
+#define USBR_NSUSPEND		(1 << 1)
+#define USBR_SEMODE		(1 << 0)
+
+#define TAHVO_MODE_HOST		0
+#define TAHVO_MODE_PERIPHERAL	1
+
+struct tahvo_usb {
+	struct platform_device	*pt_dev;
+	struct usb_phy		phy;
+	int			vbus_state;
+	struct mutex		serialize;
+	struct clk		*ick;
+	int			irq;
+	int			tahvo_mode;
+	struct extcon_dev	extcon;
+};
+
+static const char *tahvo_cable[] = {
+	"USB-HOST",
+	"USB",
+	NULL,
+};
+
+static ssize_t vbus_state_show(struct device *device,
+			       struct device_attribute *attr, char *buf)
+{
+	struct tahvo_usb *tu = dev_get_drvdata(device);
+	return sprintf(buf, "%d\n", tu->vbus_state);
+}
+static DEVICE_ATTR(vbus_state, 0444, vbus_state_show, NULL);
+
+static void check_vbus_state(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+	int reg, prev_state;
+
+	reg = retu_read(rdev, TAHVO_REG_IDSR);
+	if (reg & TAHVO_STAT_VBUS) {
+		switch (tu->phy.state) {
+		case OTG_STATE_B_IDLE:
+			/* Enable the gadget driver */
+			if (tu->phy.otg->gadget)
+				usb_gadget_vbus_connect(tu->phy.otg->gadget);
+			tu->phy.state = OTG_STATE_B_PERIPHERAL;
+			break;
+		case OTG_STATE_A_IDLE:
+			/*
+			 * Session is now valid assuming the USB hub is driving
+			 * Vbus.
+			 */
+			tu->phy.state = OTG_STATE_A_HOST;
+			break;
+		default:
+			break;
+		}
+		dev_info(&tu->pt_dev->dev, "USB cable connected\n");
+	} else {
+		switch (tu->phy.state) {
+		case OTG_STATE_B_PERIPHERAL:
+			if (tu->phy.otg->gadget)
+				usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
+			tu->phy.state = OTG_STATE_B_IDLE;
+			break;
+		case OTG_STATE_A_HOST:
+			tu->phy.state = OTG_STATE_A_IDLE;
+			break;
+		default:
+			break;
+		}
+		dev_info(&tu->pt_dev->dev, "USB cable disconnected\n");
+	}
+
+	prev_state = tu->vbus_state;
+	tu->vbus_state = reg & TAHVO_STAT_VBUS;
+	if (prev_state != tu->vbus_state) {
+		extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
+		sysfs_notify(&tu->pt_dev->dev.kobj, NULL, "vbus_state");
+	}
+}
+
+static void tahvo_usb_become_host(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+
+	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
+
+	/* Power up the transceiver in USB host mode */
+	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
+		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
+	tu->phy.state = OTG_STATE_A_IDLE;
+
+	check_vbus_state(tu);
+}
+
+static void tahvo_usb_stop_host(struct tahvo_usb *tu)
+{
+	tu->phy.state = OTG_STATE_A_IDLE;
+}
+
+static void tahvo_usb_become_peripheral(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+
+	extcon_set_cable_state(&tu->extcon, "USB-HOST", false);
+
+	/* Power up transceiver and set it in USB peripheral mode */
+	retu_write(rdev, TAHVO_REG_USBR, USBR_SLAVE_CONTROL | USBR_REGOUT |
+		   USBR_NSUSPEND | USBR_SLAVE_SW);
+	tu->phy.state = OTG_STATE_B_IDLE;
+
+	check_vbus_state(tu);
+}
+
+static void tahvo_usb_stop_peripheral(struct tahvo_usb *tu)
+{
+	if (tu->phy.otg->gadget)
+		usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
+	tu->phy.state = OTG_STATE_B_IDLE;
+}
+
+static void tahvo_usb_power_off(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+
+	/* Disable gadget controller if any */
+	if (tu->phy.otg->gadget)
+		usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
+
+	/* Power off transceiver */
+	retu_write(rdev, TAHVO_REG_USBR, 0);
+	tu->phy.state = OTG_STATE_UNDEFINED;
+}
+
+static int tahvo_usb_set_suspend(struct usb_phy *dev, int suspend)
+{
+	struct tahvo_usb *tu = container_of(dev, struct tahvo_usb, phy);
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+	u16 w;
+
+	dev_dbg(&tu->pt_dev->dev, "%s\n", __func__);
+
+	w = retu_read(rdev, TAHVO_REG_USBR);
+	if (suspend)
+		w &= ~USBR_NSUSPEND;
+	else
+		w |= USBR_NSUSPEND;
+	retu_write(rdev, TAHVO_REG_USBR, w);
+
+	return 0;
+}
+
+static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
+
+	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
+
+	if (otg == NULL)
+		return -ENODEV;
+
+	mutex_lock(&tu->serialize);
+
+	if (host == NULL) {
+		if (tu->tahvo_mode == TAHVO_MODE_HOST)
+			tahvo_usb_power_off(tu);
+		otg->host = NULL;
+		mutex_unlock(&tu->serialize);
+		return 0;
+	}
+
+	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
+		otg->host = NULL;
+		tahvo_usb_become_host(tu);
+	}
+
+	otg->host = host;
+
+	mutex_unlock(&tu->serialize);
+
+	return 0;
+}
+
+static int tahvo_usb_set_peripheral(struct usb_otg *otg,
+				    struct usb_gadget *gadget)
+{
+	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
+
+	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, gadget);
+
+	if (!otg)
+		return -ENODEV;
+
+	mutex_lock(&tu->serialize);
+
+	if (!gadget) {
+		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
+			tahvo_usb_power_off(tu);
+		tu->phy.otg->gadget = NULL;
+		mutex_unlock(&tu->serialize);
+		return 0;
+	}
+
+	tu->phy.otg->gadget = gadget;
+	if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
+		tahvo_usb_become_peripheral(tu);
+
+	mutex_unlock(&tu->serialize);
+
+	return 0;
+}
+
+static irqreturn_t tahvo_usb_vbus_interrupt(int irq, void *_tu)
+{
+	struct tahvo_usb *tu = _tu;
+
+	mutex_lock(&tu->serialize);
+	check_vbus_state(tu);
+	mutex_unlock(&tu->serialize);
+
+	return IRQ_HANDLED;
+}
+
+static ssize_t otg_mode_show(struct device *device,
+			     struct device_attribute *attr, char *buf)
+{
+	struct tahvo_usb *tu = dev_get_drvdata(device);
+
+	switch (tu->tahvo_mode) {
+	case TAHVO_MODE_HOST:
+		return sprintf(buf, "host\n");
+	case TAHVO_MODE_PERIPHERAL:
+		return sprintf(buf, "peripheral\n");
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t otg_mode_store(struct device *device,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct tahvo_usb *tu = dev_get_drvdata(device);
+	int r;
+
+	mutex_lock(&tu->serialize);
+	if (count >= 4 && strncmp(buf, "host", 4) == 0) {
+		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
+			tahvo_usb_stop_peripheral(tu);
+		tu->tahvo_mode = TAHVO_MODE_HOST;
+		if (tu->phy.otg->host) {
+			dev_info(device, "HOST mode: host controller present\n");
+			tahvo_usb_become_host(tu);
+		} else {
+			dev_info(device, "HOST mode: no host controller, powering off\n");
+			tahvo_usb_power_off(tu);
+		}
+		r = strlen(buf);
+	} else if (count >= 10 && strncmp(buf, "peripheral", 10) == 0) {
+		if (tu->tahvo_mode == TAHVO_MODE_HOST)
+			tahvo_usb_stop_host(tu);
+		tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
+		if (tu->phy.otg->gadget) {
+			dev_info(device, "PERIPHERAL mode: gadget driver present\n");
+			tahvo_usb_become_peripheral(tu);
+		} else {
+			dev_info(device, "PERIPHERAL mode: no gadget driver, powering off\n");
+			tahvo_usb_power_off(tu);
+		}
+		r = strlen(buf);
+	} else {
+		r = -EINVAL;
+	}
+	mutex_unlock(&tu->serialize);
+
+	return r;
+}
+
+static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);
+
+static int tahvo_usb_probe(struct platform_device *pdev)
+{
+	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
+	struct tahvo_usb *tu;
+	int ret;
+
+	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
+	if (!tu)
+		return -ENOMEM;
+
+	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
+				   GFP_KERNEL);
+	if (!tu->phy.otg)
+		return -ENOMEM;
+
+	tu->pt_dev = pdev;
+
+	/* Default mode */
+#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
+	tu->tahvo_mode = TAHVO_MODE_HOST;
+#else
+	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
+#endif
+
+	mutex_init(&tu->serialize);
+
+	tu->ick = devm_clk_get(&pdev->dev, "usb_l4_ick");
+	if (!IS_ERR(tu->ick))
+		clk_enable(tu->ick);
+
+	/*
+	 * Set initial state, so that we generate kevents only on state changes.
+	 */
+	tu->vbus_state = retu_read(rdev, TAHVO_REG_IDSR) & TAHVO_STAT_VBUS;
+
+	tu->extcon.name = DRIVER_NAME;
+	tu->extcon.supported_cable = tahvo_cable;
+	ret = extcon_dev_register(&tu->extcon, &pdev->dev);
+	if (ret) {
+		dev_err(&pdev->dev, "could not register extcon device: %d\n",
+			ret);
+		return ret;
+	}
+
+	/* Set the initial cable state. */
+	extcon_set_cable_state(&tu->extcon, "USB-HOST",
+			       tu->tahvo_mode == TAHVO_MODE_HOST);
+	extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
+
+	tu->irq = platform_get_irq(pdev, 0);
+	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, 0,
+				   "tahvo-vbus", tu);
+	if (ret) {
+		dev_err(&pdev->dev, "could not register tahvo-vbus irq: %d\n",
+			ret);
+		goto err_disable_clk;
+	}
+
+	/* Attributes */
+	ret = device_create_file(&pdev->dev, &dev_attr_vbus_state);
+	ret |= device_create_file(&pdev->dev, &dev_attr_otg_mode);
+	if (ret)
+		dev_err(&pdev->dev, "attribute creation failed: %d\n", ret);
+
+	/* Create OTG interface */
+	tahvo_usb_power_off(tu);
+	tu->phy.dev = &pdev->dev;
+	tu->phy.state = OTG_STATE_UNDEFINED;
+	tu->phy.label = DRIVER_NAME;
+	tu->phy.set_suspend = tahvo_usb_set_suspend;
+
+	tu->phy.otg->phy = &tu->phy;
+	tu->phy.otg->set_host = tahvo_usb_set_host;
+	tu->phy.otg->set_peripheral = tahvo_usb_set_peripheral;
+
+	ret = usb_add_phy(&tu->phy, USB_PHY_TYPE_USB2);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "cannot register USB transceiver: %d\n",
+			ret);
+		goto err_free_irq;
+	}
+
+	dev_set_drvdata(&pdev->dev, tu);
+
+	return 0;
+
+err_free_irq:
+	free_irq(tu->irq, tu);
+err_disable_clk:
+	if (!IS_ERR(tu->ick))
+		clk_disable(tu->ick);
+
+	return ret;
+}
+
+static int tahvo_usb_remove(struct platform_device *pdev)
+{
+	struct tahvo_usb *tu = platform_get_drvdata(pdev);
+
+	free_irq(tu->irq, tu);
+	usb_remove_phy(&tu->phy);
+	device_remove_file(&pdev->dev, &dev_attr_vbus_state);
+	device_remove_file(&pdev->dev, &dev_attr_otg_mode);
+	extcon_dev_unregister(&tu->extcon);
+	if (!IS_ERR(tu->ick))
+		clk_disable(tu->ick);
+
+	return 0;
+}
+
+static struct platform_driver tahvo_usb_driver = {
+	.probe		= tahvo_usb_probe,
+	.remove		= tahvo_usb_remove,
+	.driver		= {
+		.name	= "tahvo-usb",
+		.owner	= THIS_MODULE,
+	},
+};
+module_platform_driver(tahvo_usb_driver);
+
+MODULE_DESCRIPTION("Tahvo USB transceiver driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Juha Yrjölä, Tony Lindgren, and Timo Teräs");
+MODULE_AUTHOR("Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>");
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
@ 2013-06-18 17:07     ` Aaro Koskinen
  0 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-06-18 17:07 UTC (permalink / raw)
  To: linux-arm-kernel

Add Tahvo USB transceiver driver.

Based on old code from linux-omap tree. The original driver was written
by Juha Yrj?l?, Tony Lindgren, and Timo Ter?s.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/usb/phy/Kconfig     |  14 ++
 drivers/usb/phy/Makefile    |   1 +
 drivers/usb/phy/phy-tahvo.c | 448 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 463 insertions(+)
 create mode 100644 drivers/usb/phy/phy-tahvo.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 14a50bd..a5c7937 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -145,6 +145,20 @@ config OMAP_OTG
 	  This driver can also be built as a module. If so, the module
 	  will be called omap-otg.
 
+config TAHVO_USB
+	tristate "Tahvo USB transceiver driver"
+	depends on MFD_RETU && EXTCON
+	help
+	  Enable this to support USB transceiver on Tahvo. This is used
+	  at least on Nokia 770.
+
+config TAHVO_USB_HOST_BY_DEFAULT
+	depends on TAHVO_USB
+	boolean "Device in USB host mode by default"
+	help
+	  Say Y here, if you want the device to enter USB host mode
+	  by default on bootup.
+
 config USB_ISP1301
 	tristate "NXP ISP1301 USB transceiver support"
 	depends on USB || USB_GADGET
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index c7f391b..f3984d1 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -13,6 +13,7 @@ phy-fsl-usb2-objs			:= phy-fsl-usb.o phy-fsm-usb.o
 obj-$(CONFIG_FSL_USB2_OTG)		+= phy-fsl-usb2.o
 obj-$(CONFIG_ISP1301_OMAP)		+= phy-isp1301-omap.o
 obj-$(CONFIG_MV_U3D_PHY)		+= phy-mv-u3d-usb.o
+obj-$(CONFIG_TAHVO_USB)			+= phy-tahvo.o
 obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-nop.o
 obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
 obj-$(CONFIG_OMAP_OTG)			+= phy-omap-otg.o
diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
new file mode 100644
index 0000000..a2d96a2
--- /dev/null
+++ b/drivers/usb/phy/phy-tahvo.c
@@ -0,0 +1,448 @@
+/*
+ * Tahvo USB transceiver driver
+ *
+ * Copyright (C) 2005-2006 Nokia Corporation
+ *
+ * Parts copied from isp1301_omap.c.
+ * Copyright (C) 2004 Texas Instruments
+ * Copyright (C) 2004 David Brownell
+ *
+ * Original driver written by Juha Yrj?l?, Tony Lindgren and Timo Ter?s.
+ * Modified for Retu/Tahvo MFD by Aaro Koskinen.
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/usb.h>
+#include <linux/extcon.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb/otg.h>
+#include <linux/mfd/retu.h>
+#include <linux/usb/gadget.h>
+#include <linux/platform_device.h>
+
+#define DRIVER_NAME     "tahvo-usb"
+
+#define TAHVO_REG_IDSR	0x02
+#define TAHVO_REG_USBR	0x06
+
+#define USBR_SLAVE_CONTROL	(1 << 8)
+#define USBR_VPPVIO_SW		(1 << 7)
+#define USBR_SPEED		(1 << 6)
+#define USBR_REGOUT		(1 << 5)
+#define USBR_MASTER_SW2		(1 << 4)
+#define USBR_MASTER_SW1		(1 << 3)
+#define USBR_SLAVE_SW		(1 << 2)
+#define USBR_NSUSPEND		(1 << 1)
+#define USBR_SEMODE		(1 << 0)
+
+#define TAHVO_MODE_HOST		0
+#define TAHVO_MODE_PERIPHERAL	1
+
+struct tahvo_usb {
+	struct platform_device	*pt_dev;
+	struct usb_phy		phy;
+	int			vbus_state;
+	struct mutex		serialize;
+	struct clk		*ick;
+	int			irq;
+	int			tahvo_mode;
+	struct extcon_dev	extcon;
+};
+
+static const char *tahvo_cable[] = {
+	"USB-HOST",
+	"USB",
+	NULL,
+};
+
+static ssize_t vbus_state_show(struct device *device,
+			       struct device_attribute *attr, char *buf)
+{
+	struct tahvo_usb *tu = dev_get_drvdata(device);
+	return sprintf(buf, "%d\n", tu->vbus_state);
+}
+static DEVICE_ATTR(vbus_state, 0444, vbus_state_show, NULL);
+
+static void check_vbus_state(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+	int reg, prev_state;
+
+	reg = retu_read(rdev, TAHVO_REG_IDSR);
+	if (reg & TAHVO_STAT_VBUS) {
+		switch (tu->phy.state) {
+		case OTG_STATE_B_IDLE:
+			/* Enable the gadget driver */
+			if (tu->phy.otg->gadget)
+				usb_gadget_vbus_connect(tu->phy.otg->gadget);
+			tu->phy.state = OTG_STATE_B_PERIPHERAL;
+			break;
+		case OTG_STATE_A_IDLE:
+			/*
+			 * Session is now valid assuming the USB hub is driving
+			 * Vbus.
+			 */
+			tu->phy.state = OTG_STATE_A_HOST;
+			break;
+		default:
+			break;
+		}
+		dev_info(&tu->pt_dev->dev, "USB cable connected\n");
+	} else {
+		switch (tu->phy.state) {
+		case OTG_STATE_B_PERIPHERAL:
+			if (tu->phy.otg->gadget)
+				usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
+			tu->phy.state = OTG_STATE_B_IDLE;
+			break;
+		case OTG_STATE_A_HOST:
+			tu->phy.state = OTG_STATE_A_IDLE;
+			break;
+		default:
+			break;
+		}
+		dev_info(&tu->pt_dev->dev, "USB cable disconnected\n");
+	}
+
+	prev_state = tu->vbus_state;
+	tu->vbus_state = reg & TAHVO_STAT_VBUS;
+	if (prev_state != tu->vbus_state) {
+		extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
+		sysfs_notify(&tu->pt_dev->dev.kobj, NULL, "vbus_state");
+	}
+}
+
+static void tahvo_usb_become_host(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+
+	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
+
+	/* Power up the transceiver in USB host mode */
+	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
+		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
+	tu->phy.state = OTG_STATE_A_IDLE;
+
+	check_vbus_state(tu);
+}
+
+static void tahvo_usb_stop_host(struct tahvo_usb *tu)
+{
+	tu->phy.state = OTG_STATE_A_IDLE;
+}
+
+static void tahvo_usb_become_peripheral(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+
+	extcon_set_cable_state(&tu->extcon, "USB-HOST", false);
+
+	/* Power up transceiver and set it in USB peripheral mode */
+	retu_write(rdev, TAHVO_REG_USBR, USBR_SLAVE_CONTROL | USBR_REGOUT |
+		   USBR_NSUSPEND | USBR_SLAVE_SW);
+	tu->phy.state = OTG_STATE_B_IDLE;
+
+	check_vbus_state(tu);
+}
+
+static void tahvo_usb_stop_peripheral(struct tahvo_usb *tu)
+{
+	if (tu->phy.otg->gadget)
+		usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
+	tu->phy.state = OTG_STATE_B_IDLE;
+}
+
+static void tahvo_usb_power_off(struct tahvo_usb *tu)
+{
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+
+	/* Disable gadget controller if any */
+	if (tu->phy.otg->gadget)
+		usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
+
+	/* Power off transceiver */
+	retu_write(rdev, TAHVO_REG_USBR, 0);
+	tu->phy.state = OTG_STATE_UNDEFINED;
+}
+
+static int tahvo_usb_set_suspend(struct usb_phy *dev, int suspend)
+{
+	struct tahvo_usb *tu = container_of(dev, struct tahvo_usb, phy);
+	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
+	u16 w;
+
+	dev_dbg(&tu->pt_dev->dev, "%s\n", __func__);
+
+	w = retu_read(rdev, TAHVO_REG_USBR);
+	if (suspend)
+		w &= ~USBR_NSUSPEND;
+	else
+		w |= USBR_NSUSPEND;
+	retu_write(rdev, TAHVO_REG_USBR, w);
+
+	return 0;
+}
+
+static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
+
+	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
+
+	if (otg == NULL)
+		return -ENODEV;
+
+	mutex_lock(&tu->serialize);
+
+	if (host == NULL) {
+		if (tu->tahvo_mode == TAHVO_MODE_HOST)
+			tahvo_usb_power_off(tu);
+		otg->host = NULL;
+		mutex_unlock(&tu->serialize);
+		return 0;
+	}
+
+	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
+		otg->host = NULL;
+		tahvo_usb_become_host(tu);
+	}
+
+	otg->host = host;
+
+	mutex_unlock(&tu->serialize);
+
+	return 0;
+}
+
+static int tahvo_usb_set_peripheral(struct usb_otg *otg,
+				    struct usb_gadget *gadget)
+{
+	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
+
+	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, gadget);
+
+	if (!otg)
+		return -ENODEV;
+
+	mutex_lock(&tu->serialize);
+
+	if (!gadget) {
+		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
+			tahvo_usb_power_off(tu);
+		tu->phy.otg->gadget = NULL;
+		mutex_unlock(&tu->serialize);
+		return 0;
+	}
+
+	tu->phy.otg->gadget = gadget;
+	if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
+		tahvo_usb_become_peripheral(tu);
+
+	mutex_unlock(&tu->serialize);
+
+	return 0;
+}
+
+static irqreturn_t tahvo_usb_vbus_interrupt(int irq, void *_tu)
+{
+	struct tahvo_usb *tu = _tu;
+
+	mutex_lock(&tu->serialize);
+	check_vbus_state(tu);
+	mutex_unlock(&tu->serialize);
+
+	return IRQ_HANDLED;
+}
+
+static ssize_t otg_mode_show(struct device *device,
+			     struct device_attribute *attr, char *buf)
+{
+	struct tahvo_usb *tu = dev_get_drvdata(device);
+
+	switch (tu->tahvo_mode) {
+	case TAHVO_MODE_HOST:
+		return sprintf(buf, "host\n");
+	case TAHVO_MODE_PERIPHERAL:
+		return sprintf(buf, "peripheral\n");
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t otg_mode_store(struct device *device,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct tahvo_usb *tu = dev_get_drvdata(device);
+	int r;
+
+	mutex_lock(&tu->serialize);
+	if (count >= 4 && strncmp(buf, "host", 4) == 0) {
+		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
+			tahvo_usb_stop_peripheral(tu);
+		tu->tahvo_mode = TAHVO_MODE_HOST;
+		if (tu->phy.otg->host) {
+			dev_info(device, "HOST mode: host controller present\n");
+			tahvo_usb_become_host(tu);
+		} else {
+			dev_info(device, "HOST mode: no host controller, powering off\n");
+			tahvo_usb_power_off(tu);
+		}
+		r = strlen(buf);
+	} else if (count >= 10 && strncmp(buf, "peripheral", 10) == 0) {
+		if (tu->tahvo_mode == TAHVO_MODE_HOST)
+			tahvo_usb_stop_host(tu);
+		tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
+		if (tu->phy.otg->gadget) {
+			dev_info(device, "PERIPHERAL mode: gadget driver present\n");
+			tahvo_usb_become_peripheral(tu);
+		} else {
+			dev_info(device, "PERIPHERAL mode: no gadget driver, powering off\n");
+			tahvo_usb_power_off(tu);
+		}
+		r = strlen(buf);
+	} else {
+		r = -EINVAL;
+	}
+	mutex_unlock(&tu->serialize);
+
+	return r;
+}
+
+static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);
+
+static int tahvo_usb_probe(struct platform_device *pdev)
+{
+	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
+	struct tahvo_usb *tu;
+	int ret;
+
+	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
+	if (!tu)
+		return -ENOMEM;
+
+	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
+				   GFP_KERNEL);
+	if (!tu->phy.otg)
+		return -ENOMEM;
+
+	tu->pt_dev = pdev;
+
+	/* Default mode */
+#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
+	tu->tahvo_mode = TAHVO_MODE_HOST;
+#else
+	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
+#endif
+
+	mutex_init(&tu->serialize);
+
+	tu->ick = devm_clk_get(&pdev->dev, "usb_l4_ick");
+	if (!IS_ERR(tu->ick))
+		clk_enable(tu->ick);
+
+	/*
+	 * Set initial state, so that we generate kevents only on state changes.
+	 */
+	tu->vbus_state = retu_read(rdev, TAHVO_REG_IDSR) & TAHVO_STAT_VBUS;
+
+	tu->extcon.name = DRIVER_NAME;
+	tu->extcon.supported_cable = tahvo_cable;
+	ret = extcon_dev_register(&tu->extcon, &pdev->dev);
+	if (ret) {
+		dev_err(&pdev->dev, "could not register extcon device: %d\n",
+			ret);
+		return ret;
+	}
+
+	/* Set the initial cable state. */
+	extcon_set_cable_state(&tu->extcon, "USB-HOST",
+			       tu->tahvo_mode == TAHVO_MODE_HOST);
+	extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
+
+	tu->irq = platform_get_irq(pdev, 0);
+	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, 0,
+				   "tahvo-vbus", tu);
+	if (ret) {
+		dev_err(&pdev->dev, "could not register tahvo-vbus irq: %d\n",
+			ret);
+		goto err_disable_clk;
+	}
+
+	/* Attributes */
+	ret = device_create_file(&pdev->dev, &dev_attr_vbus_state);
+	ret |= device_create_file(&pdev->dev, &dev_attr_otg_mode);
+	if (ret)
+		dev_err(&pdev->dev, "attribute creation failed: %d\n", ret);
+
+	/* Create OTG interface */
+	tahvo_usb_power_off(tu);
+	tu->phy.dev = &pdev->dev;
+	tu->phy.state = OTG_STATE_UNDEFINED;
+	tu->phy.label = DRIVER_NAME;
+	tu->phy.set_suspend = tahvo_usb_set_suspend;
+
+	tu->phy.otg->phy = &tu->phy;
+	tu->phy.otg->set_host = tahvo_usb_set_host;
+	tu->phy.otg->set_peripheral = tahvo_usb_set_peripheral;
+
+	ret = usb_add_phy(&tu->phy, USB_PHY_TYPE_USB2);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "cannot register USB transceiver: %d\n",
+			ret);
+		goto err_free_irq;
+	}
+
+	dev_set_drvdata(&pdev->dev, tu);
+
+	return 0;
+
+err_free_irq:
+	free_irq(tu->irq, tu);
+err_disable_clk:
+	if (!IS_ERR(tu->ick))
+		clk_disable(tu->ick);
+
+	return ret;
+}
+
+static int tahvo_usb_remove(struct platform_device *pdev)
+{
+	struct tahvo_usb *tu = platform_get_drvdata(pdev);
+
+	free_irq(tu->irq, tu);
+	usb_remove_phy(&tu->phy);
+	device_remove_file(&pdev->dev, &dev_attr_vbus_state);
+	device_remove_file(&pdev->dev, &dev_attr_otg_mode);
+	extcon_dev_unregister(&tu->extcon);
+	if (!IS_ERR(tu->ick))
+		clk_disable(tu->ick);
+
+	return 0;
+}
+
+static struct platform_driver tahvo_usb_driver = {
+	.probe		= tahvo_usb_probe,
+	.remove		= tahvo_usb_remove,
+	.driver		= {
+		.name	= "tahvo-usb",
+		.owner	= THIS_MODULE,
+	},
+};
+module_platform_driver(tahvo_usb_driver);
+
+MODULE_DESCRIPTION("Tahvo USB transceiver driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Juha Yrj?l?, Tony Lindgren, and Timo Ter?s");
+MODULE_AUTHOR("Aaro Koskinen <aaro.koskinen@iki.fi>");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
  2013-06-18 17:07     ` Aaro Koskinen
@ 2013-07-08  7:21         ` Felipe Balbi
  -1 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2013-07-08  7:21 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 6908 bytes --]

Hi,

On Tue, Jun 18, 2013 at 08:07:01PM +0300, Aaro Koskinen wrote:
> +static ssize_t vbus_state_show(struct device *device,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	struct tahvo_usb *tu = dev_get_drvdata(device);
> +	return sprintf(buf, "%d\n", tu->vbus_state);
> +}
> +static DEVICE_ATTR(vbus_state, 0444, vbus_state_show, NULL);

VBUS status sounds generic enough, also, you should be printing on/off
here instead of tahvo-specific values. Some users might not have access
to the docs, or even the kernel sources, to understand what that integer
means.

> +static void tahvo_usb_become_host(struct tahvo_usb *tu)
> +{
> +	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
> +
> +	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
> +
> +	/* Power up the transceiver in USB host mode */
> +	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
> +		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
> +	tu->phy.state = OTG_STATE_A_IDLE;
> +
> +	check_vbus_state(tu);
> +}
> +
> +static void tahvo_usb_stop_host(struct tahvo_usb *tu)
> +{
> +	tu->phy.state = OTG_STATE_A_IDLE;

shouldn't you undo tahvo_usb_become_host() here ? I mean, set the
transceiver to SLAVE ?

> +static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> +	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
> +
> +	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
> +
> +	if (otg == NULL)
> +		return -ENODEV;
> +
> +	mutex_lock(&tu->serialize);
> +
> +	if (host == NULL) {
> +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> +			tahvo_usb_power_off(tu);
> +		otg->host = NULL;
> +		mutex_unlock(&tu->serialize);
> +		return 0;
> +	}
> +
> +	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
> +		otg->host = NULL;
> +		tahvo_usb_become_host(tu);
> +	}
> +
> +	otg->host = host;
> +
> +	mutex_unlock(&tu->serialize);
> +
> +	return 0;
> +}
> +
> +static int tahvo_usb_set_peripheral(struct usb_otg *otg,
> +				    struct usb_gadget *gadget)

I want to get rid of the extra 'gadget' and 'host' parameters on
->set_host() and ->set_peripheral(). Nobody really uses them other than:

my_phy->phy.otg->{gadget,host} = {gadget,host};

For that, I need all other PHYs to stop relying on those parameters and
I'll cook patches for that for v3.12 merge window.

> +static ssize_t otg_mode_store(struct device *device,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct tahvo_usb *tu = dev_get_drvdata(device);
> +	int r;
> +
> +	mutex_lock(&tu->serialize);
> +	if (count >= 4 && strncmp(buf, "host", 4) == 0) {
> +		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
> +			tahvo_usb_stop_peripheral(tu);
> +		tu->tahvo_mode = TAHVO_MODE_HOST;
> +		if (tu->phy.otg->host) {
> +			dev_info(device, "HOST mode: host controller present\n");
> +			tahvo_usb_become_host(tu);
> +		} else {
> +			dev_info(device, "HOST mode: no host controller, powering off\n");
> +			tahvo_usb_power_off(tu);
> +		}
> +		r = strlen(buf);
> +	} else if (count >= 10 && strncmp(buf, "peripheral", 10) == 0) {
> +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> +			tahvo_usb_stop_host(tu);
> +		tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> +		if (tu->phy.otg->gadget) {
> +			dev_info(device, "PERIPHERAL mode: gadget driver present\n");
> +			tahvo_usb_become_peripheral(tu);
> +		} else {
> +			dev_info(device, "PERIPHERAL mode: no gadget driver, powering off\n");
> +			tahvo_usb_power_off(tu);
> +		}
> +		r = strlen(buf);
> +	} else {
> +		r = -EINVAL;
> +	}
> +	mutex_unlock(&tu->serialize);
> +
> +	return r;
> +}
> +
> +static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);

this looks like something which would be very nice if implemented
generically. Since you, anyway miss the documentation in
Documentation/ABI/ and need to respin the patch, how about making this
generic ?

> +static int tahvo_usb_probe(struct platform_device *pdev)
> +{
> +	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
> +	struct tahvo_usb *tu;
> +	int ret;
> +
> +	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
> +	if (!tu)
> +		return -ENOMEM;
> +
> +	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
> +				   GFP_KERNEL);
> +	if (!tu->phy.otg)
> +		return -ENOMEM;
> +
> +	tu->pt_dev = pdev;
> +
> +	/* Default mode */
> +#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
> +	tu->tahvo_mode = TAHVO_MODE_HOST;
> +#else
> +	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> +#endif

should you call become_host()/become_peripheral() here ?

> +	mutex_init(&tu->serialize);
> +
> +	tu->ick = devm_clk_get(&pdev->dev, "usb_l4_ick");
> +	if (!IS_ERR(tu->ick))
> +		clk_enable(tu->ick);
> +
> +	/*
> +	 * Set initial state, so that we generate kevents only on state changes.
> +	 */
> +	tu->vbus_state = retu_read(rdev, TAHVO_REG_IDSR) & TAHVO_STAT_VBUS;
> +
> +	tu->extcon.name = DRIVER_NAME;
> +	tu->extcon.supported_cable = tahvo_cable;
> +	ret = extcon_dev_register(&tu->extcon, &pdev->dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not register extcon device: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	/* Set the initial cable state. */
> +	extcon_set_cable_state(&tu->extcon, "USB-HOST",
> +			       tu->tahvo_mode == TAHVO_MODE_HOST);
> +	extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
> +
> +	tu->irq = platform_get_irq(pdev, 0);
> +	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, 0,
> +				   "tahvo-vbus", tu);

since your hardirq handler is NULL, you *must* pass IRQF_ONESHOT. /Could
also use devm_request_threaded_irq()

> +	if (ret) {
> +		dev_err(&pdev->dev, "could not register tahvo-vbus irq: %d\n",
> +			ret);
> +		goto err_disable_clk;
> +	}
> +
> +	/* Attributes */
> +	ret = device_create_file(&pdev->dev, &dev_attr_vbus_state);
> +	ret |= device_create_file(&pdev->dev, &dev_attr_otg_mode);

heh, Greg wrote a very interesting article recently (look at his blog)
discussing kernel/userland races wrt creation of sysfs files.

> +	/* Create OTG interface */
> +	tahvo_usb_power_off(tu);
> +	tu->phy.dev = &pdev->dev;
> +	tu->phy.state = OTG_STATE_UNDEFINED;
> +	tu->phy.label = DRIVER_NAME;
> +	tu->phy.set_suspend = tahvo_usb_set_suspend;
> +
> +	tu->phy.otg->phy = &tu->phy;
> +	tu->phy.otg->set_host = tahvo_usb_set_host;
> +	tu->phy.otg->set_peripheral = tahvo_usb_set_peripheral;
> +
> +	ret = usb_add_phy(&tu->phy, USB_PHY_TYPE_USB2);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "cannot register USB transceiver: %d\n",
> +			ret);
> +		goto err_free_irq;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, tu);

what if someone accesses the sysfs file you've already created before
you call dev_set_drvdata? (not sure  if it's possible).

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
@ 2013-07-08  7:21         ` Felipe Balbi
  0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2013-07-08  7:21 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Tue, Jun 18, 2013 at 08:07:01PM +0300, Aaro Koskinen wrote:
> +static ssize_t vbus_state_show(struct device *device,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	struct tahvo_usb *tu = dev_get_drvdata(device);
> +	return sprintf(buf, "%d\n", tu->vbus_state);
> +}
> +static DEVICE_ATTR(vbus_state, 0444, vbus_state_show, NULL);

VBUS status sounds generic enough, also, you should be printing on/off
here instead of tahvo-specific values. Some users might not have access
to the docs, or even the kernel sources, to understand what that integer
means.

> +static void tahvo_usb_become_host(struct tahvo_usb *tu)
> +{
> +	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
> +
> +	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
> +
> +	/* Power up the transceiver in USB host mode */
> +	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
> +		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
> +	tu->phy.state = OTG_STATE_A_IDLE;
> +
> +	check_vbus_state(tu);
> +}
> +
> +static void tahvo_usb_stop_host(struct tahvo_usb *tu)
> +{
> +	tu->phy.state = OTG_STATE_A_IDLE;

shouldn't you undo tahvo_usb_become_host() here ? I mean, set the
transceiver to SLAVE ?

> +static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> +	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
> +
> +	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
> +
> +	if (otg == NULL)
> +		return -ENODEV;
> +
> +	mutex_lock(&tu->serialize);
> +
> +	if (host == NULL) {
> +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> +			tahvo_usb_power_off(tu);
> +		otg->host = NULL;
> +		mutex_unlock(&tu->serialize);
> +		return 0;
> +	}
> +
> +	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
> +		otg->host = NULL;
> +		tahvo_usb_become_host(tu);
> +	}
> +
> +	otg->host = host;
> +
> +	mutex_unlock(&tu->serialize);
> +
> +	return 0;
> +}
> +
> +static int tahvo_usb_set_peripheral(struct usb_otg *otg,
> +				    struct usb_gadget *gadget)

I want to get rid of the extra 'gadget' and 'host' parameters on
->set_host() and ->set_peripheral(). Nobody really uses them other than:

my_phy->phy.otg->{gadget,host} = {gadget,host};

For that, I need all other PHYs to stop relying on those parameters and
I'll cook patches for that for v3.12 merge window.

> +static ssize_t otg_mode_store(struct device *device,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct tahvo_usb *tu = dev_get_drvdata(device);
> +	int r;
> +
> +	mutex_lock(&tu->serialize);
> +	if (count >= 4 && strncmp(buf, "host", 4) == 0) {
> +		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
> +			tahvo_usb_stop_peripheral(tu);
> +		tu->tahvo_mode = TAHVO_MODE_HOST;
> +		if (tu->phy.otg->host) {
> +			dev_info(device, "HOST mode: host controller present\n");
> +			tahvo_usb_become_host(tu);
> +		} else {
> +			dev_info(device, "HOST mode: no host controller, powering off\n");
> +			tahvo_usb_power_off(tu);
> +		}
> +		r = strlen(buf);
> +	} else if (count >= 10 && strncmp(buf, "peripheral", 10) == 0) {
> +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> +			tahvo_usb_stop_host(tu);
> +		tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> +		if (tu->phy.otg->gadget) {
> +			dev_info(device, "PERIPHERAL mode: gadget driver present\n");
> +			tahvo_usb_become_peripheral(tu);
> +		} else {
> +			dev_info(device, "PERIPHERAL mode: no gadget driver, powering off\n");
> +			tahvo_usb_power_off(tu);
> +		}
> +		r = strlen(buf);
> +	} else {
> +		r = -EINVAL;
> +	}
> +	mutex_unlock(&tu->serialize);
> +
> +	return r;
> +}
> +
> +static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);

this looks like something which would be very nice if implemented
generically. Since you, anyway miss the documentation in
Documentation/ABI/ and need to respin the patch, how about making this
generic ?

> +static int tahvo_usb_probe(struct platform_device *pdev)
> +{
> +	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
> +	struct tahvo_usb *tu;
> +	int ret;
> +
> +	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
> +	if (!tu)
> +		return -ENOMEM;
> +
> +	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
> +				   GFP_KERNEL);
> +	if (!tu->phy.otg)
> +		return -ENOMEM;
> +
> +	tu->pt_dev = pdev;
> +
> +	/* Default mode */
> +#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
> +	tu->tahvo_mode = TAHVO_MODE_HOST;
> +#else
> +	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> +#endif

should you call become_host()/become_peripheral() here ?

> +	mutex_init(&tu->serialize);
> +
> +	tu->ick = devm_clk_get(&pdev->dev, "usb_l4_ick");
> +	if (!IS_ERR(tu->ick))
> +		clk_enable(tu->ick);
> +
> +	/*
> +	 * Set initial state, so that we generate kevents only on state changes.
> +	 */
> +	tu->vbus_state = retu_read(rdev, TAHVO_REG_IDSR) & TAHVO_STAT_VBUS;
> +
> +	tu->extcon.name = DRIVER_NAME;
> +	tu->extcon.supported_cable = tahvo_cable;
> +	ret = extcon_dev_register(&tu->extcon, &pdev->dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not register extcon device: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	/* Set the initial cable state. */
> +	extcon_set_cable_state(&tu->extcon, "USB-HOST",
> +			       tu->tahvo_mode == TAHVO_MODE_HOST);
> +	extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
> +
> +	tu->irq = platform_get_irq(pdev, 0);
> +	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, 0,
> +				   "tahvo-vbus", tu);

since your hardirq handler is NULL, you *must* pass IRQF_ONESHOT. /Could
also use devm_request_threaded_irq()

> +	if (ret) {
> +		dev_err(&pdev->dev, "could not register tahvo-vbus irq: %d\n",
> +			ret);
> +		goto err_disable_clk;
> +	}
> +
> +	/* Attributes */
> +	ret = device_create_file(&pdev->dev, &dev_attr_vbus_state);
> +	ret |= device_create_file(&pdev->dev, &dev_attr_otg_mode);

heh, Greg wrote a very interesting article recently (look at his blog)
discussing kernel/userland races wrt creation of sysfs files.

> +	/* Create OTG interface */
> +	tahvo_usb_power_off(tu);
> +	tu->phy.dev = &pdev->dev;
> +	tu->phy.state = OTG_STATE_UNDEFINED;
> +	tu->phy.label = DRIVER_NAME;
> +	tu->phy.set_suspend = tahvo_usb_set_suspend;
> +
> +	tu->phy.otg->phy = &tu->phy;
> +	tu->phy.otg->set_host = tahvo_usb_set_host;
> +	tu->phy.otg->set_peripheral = tahvo_usb_set_peripheral;
> +
> +	ret = usb_add_phy(&tu->phy, USB_PHY_TYPE_USB2);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "cannot register USB transceiver: %d\n",
> +			ret);
> +		goto err_free_irq;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, tu);

what if someone accesses the sysfs file you've already created before
you call dev_set_drvdata? (not sure  if it's possible).

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130708/fbd977f2/attachment.sig>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
  2013-07-08  7:21         ` Felipe Balbi
@ 2013-07-08 19:44             ` Aaro Koskinen
  -1 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-07-08 19:44 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi,

On Mon, Jul 08, 2013 at 10:21:09AM +0300, Felipe Balbi wrote:
> On Tue, Jun 18, 2013 at 08:07:01PM +0300, Aaro Koskinen wrote:
> > +static ssize_t vbus_state_show(struct device *device,
> > +			       struct device_attribute *attr, char *buf)
> > +{
> > +	struct tahvo_usb *tu = dev_get_drvdata(device);
> > +	return sprintf(buf, "%d\n", tu->vbus_state);
> > +}
> > +static DEVICE_ATTR(vbus_state, 0444, vbus_state_show, NULL);
> 
> VBUS status sounds generic enough, also, you should be printing on/off
> here instead of tahvo-specific values. Some users might not have access
> to the docs, or even the kernel sources, to understand what that integer
> means.

Ok, I'll fix.

> > +static void tahvo_usb_become_host(struct tahvo_usb *tu)
> > +{
> > +	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
> > +
> > +	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
> > +
> > +	/* Power up the transceiver in USB host mode */
> > +	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
> > +		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
> > +	tu->phy.state = OTG_STATE_A_IDLE;
> > +
> > +	check_vbus_state(tu);
> > +}
> > +
> > +static void tahvo_usb_stop_host(struct tahvo_usb *tu)
> > +{
> > +	tu->phy.state = OTG_STATE_A_IDLE;
> 
> shouldn't you undo tahvo_usb_become_host() here ? I mean, set the
> transceiver to SLAVE ?

tahvo_usb_become_peripheral() (or power_off) is always called after this
function. Actually, these stop functions can be eliminated altogether
as they are only called from a single place...

> > +static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
> > +{
> > +	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
> > +
> > +	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
> > +
> > +	if (otg == NULL)
> > +		return -ENODEV;
> > +
> > +	mutex_lock(&tu->serialize);
> > +
> > +	if (host == NULL) {
> > +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> > +			tahvo_usb_power_off(tu);
> > +		otg->host = NULL;
> > +		mutex_unlock(&tu->serialize);
> > +		return 0;
> > +	}
> > +
> > +	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
> > +		otg->host = NULL;
> > +		tahvo_usb_become_host(tu);
> > +	}
> > +
> > +	otg->host = host;
> > +
> > +	mutex_unlock(&tu->serialize);
> > +
> > +	return 0;
> > +}
> > +
> > +static int tahvo_usb_set_peripheral(struct usb_otg *otg,
> > +				    struct usb_gadget *gadget)
> 
> I want to get rid of the extra 'gadget' and 'host' parameters on
> ->set_host() and ->set_peripheral(). Nobody really uses them other than:
> 
> my_phy->phy.otg->{gadget,host} = {gadget,host};
> 
> For that, I need all other PHYs to stop relying on those parameters and
> I'll cook patches for that for v3.12 merge window.

How will the PHY know if there is a host/gadget driver present? I guess
I will need to wait for those patches...

> > +static ssize_t otg_mode_store(struct device *device,
> > +			      struct device_attribute *attr,
> > +			      const char *buf, size_t count)
> > +{
> > +	struct tahvo_usb *tu = dev_get_drvdata(device);
> > +	int r;
> > +
> > +	mutex_lock(&tu->serialize);
> > +	if (count >= 4 && strncmp(buf, "host", 4) == 0) {
> > +		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
> > +			tahvo_usb_stop_peripheral(tu);
> > +		tu->tahvo_mode = TAHVO_MODE_HOST;
> > +		if (tu->phy.otg->host) {
> > +			dev_info(device, "HOST mode: host controller present\n");
> > +			tahvo_usb_become_host(tu);
> > +		} else {
> > +			dev_info(device, "HOST mode: no host controller, powering off\n");
> > +			tahvo_usb_power_off(tu);
> > +		}
> > +		r = strlen(buf);
> > +	} else if (count >= 10 && strncmp(buf, "peripheral", 10) == 0) {
> > +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> > +			tahvo_usb_stop_host(tu);
> > +		tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> > +		if (tu->phy.otg->gadget) {
> > +			dev_info(device, "PERIPHERAL mode: gadget driver present\n");
> > +			tahvo_usb_become_peripheral(tu);
> > +		} else {
> > +			dev_info(device, "PERIPHERAL mode: no gadget driver, powering off\n");
> > +			tahvo_usb_power_off(tu);
> > +		}
> > +		r = strlen(buf);
> > +	} else {
> > +		r = -EINVAL;
> > +	}
> > +	mutex_unlock(&tu->serialize);
> > +
> > +	return r;
> > +}
> > +
> > +static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);
> 
> this looks like something which would be very nice if implemented
> generically. Since you, anyway miss the documentation in
> Documentation/ABI/ and need to respin the patch, how about making this
> generic ?

Ok, I'll try to come up with something...

> > +static int tahvo_usb_probe(struct platform_device *pdev)
> > +{
> > +	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
> > +	struct tahvo_usb *tu;
> > +	int ret;
> > +
> > +	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
> > +	if (!tu)
> > +		return -ENOMEM;
> > +
> > +	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
> > +				   GFP_KERNEL);
> > +	if (!tu->phy.otg)
> > +		return -ENOMEM;
> > +
> > +	tu->pt_dev = pdev;
> > +
> > +	/* Default mode */
> > +#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
> > +	tu->tahvo_mode = TAHVO_MODE_HOST;
> > +#else
> > +	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> > +#endif
> 
> should you call become_host()/become_peripheral() here ?

Not needed. Once the PHY is registered, the framework will call set_host /
set_peripheral and the HW gets set to correct state.

> > +	mutex_init(&tu->serialize);
> > +
> > +	tu->ick = devm_clk_get(&pdev->dev, "usb_l4_ick");
> > +	if (!IS_ERR(tu->ick))
> > +		clk_enable(tu->ick);
> > +
> > +	/*
> > +	 * Set initial state, so that we generate kevents only on state changes.
> > +	 */
> > +	tu->vbus_state = retu_read(rdev, TAHVO_REG_IDSR) & TAHVO_STAT_VBUS;
> > +
> > +	tu->extcon.name = DRIVER_NAME;
> > +	tu->extcon.supported_cable = tahvo_cable;
> > +	ret = extcon_dev_register(&tu->extcon, &pdev->dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "could not register extcon device: %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	/* Set the initial cable state. */
> > +	extcon_set_cable_state(&tu->extcon, "USB-HOST",
> > +			       tu->tahvo_mode == TAHVO_MODE_HOST);
> > +	extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
> > +
> > +	tu->irq = platform_get_irq(pdev, 0);
> > +	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, 0,
> > +				   "tahvo-vbus", tu);
> 
> since your hardirq handler is NULL, you *must* pass IRQF_ONESHOT.

Ok.

> /Could also use devm_request_threaded_irq()

Does not really help much, since the IRQ has to be freed manually anyway
(to ensure it's disabled before usb_remove_phy(); also looks like it's
currently enabled too early...).

> > +	if (ret) {
> > +		dev_err(&pdev->dev, "could not register tahvo-vbus irq: %d\n",
> > +			ret);
> > +		goto err_disable_clk;
> > +	}
> > +
> > +	/* Attributes */
> > +	ret = device_create_file(&pdev->dev, &dev_attr_vbus_state);
> > +	ret |= device_create_file(&pdev->dev, &dev_attr_otg_mode);
> 
> heh, Greg wrote a very interesting article recently (look at his blog)
> discussing kernel/userland races wrt creation of sysfs files.

[...]

> what if someone accesses the sysfs file you've already created before
> you call dev_set_drvdata? (not sure  if it's possible).

Yes, that's a bug, will fix.

Thanks for comments,

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
@ 2013-07-08 19:44             ` Aaro Koskinen
  0 siblings, 0 replies; 16+ messages in thread
From: Aaro Koskinen @ 2013-07-08 19:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Jul 08, 2013 at 10:21:09AM +0300, Felipe Balbi wrote:
> On Tue, Jun 18, 2013 at 08:07:01PM +0300, Aaro Koskinen wrote:
> > +static ssize_t vbus_state_show(struct device *device,
> > +			       struct device_attribute *attr, char *buf)
> > +{
> > +	struct tahvo_usb *tu = dev_get_drvdata(device);
> > +	return sprintf(buf, "%d\n", tu->vbus_state);
> > +}
> > +static DEVICE_ATTR(vbus_state, 0444, vbus_state_show, NULL);
> 
> VBUS status sounds generic enough, also, you should be printing on/off
> here instead of tahvo-specific values. Some users might not have access
> to the docs, or even the kernel sources, to understand what that integer
> means.

Ok, I'll fix.

> > +static void tahvo_usb_become_host(struct tahvo_usb *tu)
> > +{
> > +	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
> > +
> > +	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
> > +
> > +	/* Power up the transceiver in USB host mode */
> > +	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
> > +		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
> > +	tu->phy.state = OTG_STATE_A_IDLE;
> > +
> > +	check_vbus_state(tu);
> > +}
> > +
> > +static void tahvo_usb_stop_host(struct tahvo_usb *tu)
> > +{
> > +	tu->phy.state = OTG_STATE_A_IDLE;
> 
> shouldn't you undo tahvo_usb_become_host() here ? I mean, set the
> transceiver to SLAVE ?

tahvo_usb_become_peripheral() (or power_off) is always called after this
function. Actually, these stop functions can be eliminated altogether
as they are only called from a single place...

> > +static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
> > +{
> > +	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
> > +
> > +	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
> > +
> > +	if (otg == NULL)
> > +		return -ENODEV;
> > +
> > +	mutex_lock(&tu->serialize);
> > +
> > +	if (host == NULL) {
> > +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> > +			tahvo_usb_power_off(tu);
> > +		otg->host = NULL;
> > +		mutex_unlock(&tu->serialize);
> > +		return 0;
> > +	}
> > +
> > +	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
> > +		otg->host = NULL;
> > +		tahvo_usb_become_host(tu);
> > +	}
> > +
> > +	otg->host = host;
> > +
> > +	mutex_unlock(&tu->serialize);
> > +
> > +	return 0;
> > +}
> > +
> > +static int tahvo_usb_set_peripheral(struct usb_otg *otg,
> > +				    struct usb_gadget *gadget)
> 
> I want to get rid of the extra 'gadget' and 'host' parameters on
> ->set_host() and ->set_peripheral(). Nobody really uses them other than:
> 
> my_phy->phy.otg->{gadget,host} = {gadget,host};
> 
> For that, I need all other PHYs to stop relying on those parameters and
> I'll cook patches for that for v3.12 merge window.

How will the PHY know if there is a host/gadget driver present? I guess
I will need to wait for those patches...

> > +static ssize_t otg_mode_store(struct device *device,
> > +			      struct device_attribute *attr,
> > +			      const char *buf, size_t count)
> > +{
> > +	struct tahvo_usb *tu = dev_get_drvdata(device);
> > +	int r;
> > +
> > +	mutex_lock(&tu->serialize);
> > +	if (count >= 4 && strncmp(buf, "host", 4) == 0) {
> > +		if (tu->tahvo_mode == TAHVO_MODE_PERIPHERAL)
> > +			tahvo_usb_stop_peripheral(tu);
> > +		tu->tahvo_mode = TAHVO_MODE_HOST;
> > +		if (tu->phy.otg->host) {
> > +			dev_info(device, "HOST mode: host controller present\n");
> > +			tahvo_usb_become_host(tu);
> > +		} else {
> > +			dev_info(device, "HOST mode: no host controller, powering off\n");
> > +			tahvo_usb_power_off(tu);
> > +		}
> > +		r = strlen(buf);
> > +	} else if (count >= 10 && strncmp(buf, "peripheral", 10) == 0) {
> > +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> > +			tahvo_usb_stop_host(tu);
> > +		tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> > +		if (tu->phy.otg->gadget) {
> > +			dev_info(device, "PERIPHERAL mode: gadget driver present\n");
> > +			tahvo_usb_become_peripheral(tu);
> > +		} else {
> > +			dev_info(device, "PERIPHERAL mode: no gadget driver, powering off\n");
> > +			tahvo_usb_power_off(tu);
> > +		}
> > +		r = strlen(buf);
> > +	} else {
> > +		r = -EINVAL;
> > +	}
> > +	mutex_unlock(&tu->serialize);
> > +
> > +	return r;
> > +}
> > +
> > +static DEVICE_ATTR(otg_mode, 0644, otg_mode_show, otg_mode_store);
> 
> this looks like something which would be very nice if implemented
> generically. Since you, anyway miss the documentation in
> Documentation/ABI/ and need to respin the patch, how about making this
> generic ?

Ok, I'll try to come up with something...

> > +static int tahvo_usb_probe(struct platform_device *pdev)
> > +{
> > +	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
> > +	struct tahvo_usb *tu;
> > +	int ret;
> > +
> > +	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
> > +	if (!tu)
> > +		return -ENOMEM;
> > +
> > +	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
> > +				   GFP_KERNEL);
> > +	if (!tu->phy.otg)
> > +		return -ENOMEM;
> > +
> > +	tu->pt_dev = pdev;
> > +
> > +	/* Default mode */
> > +#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
> > +	tu->tahvo_mode = TAHVO_MODE_HOST;
> > +#else
> > +	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> > +#endif
> 
> should you call become_host()/become_peripheral() here ?

Not needed. Once the PHY is registered, the framework will call set_host /
set_peripheral and the HW gets set to correct state.

> > +	mutex_init(&tu->serialize);
> > +
> > +	tu->ick = devm_clk_get(&pdev->dev, "usb_l4_ick");
> > +	if (!IS_ERR(tu->ick))
> > +		clk_enable(tu->ick);
> > +
> > +	/*
> > +	 * Set initial state, so that we generate kevents only on state changes.
> > +	 */
> > +	tu->vbus_state = retu_read(rdev, TAHVO_REG_IDSR) & TAHVO_STAT_VBUS;
> > +
> > +	tu->extcon.name = DRIVER_NAME;
> > +	tu->extcon.supported_cable = tahvo_cable;
> > +	ret = extcon_dev_register(&tu->extcon, &pdev->dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "could not register extcon device: %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	/* Set the initial cable state. */
> > +	extcon_set_cable_state(&tu->extcon, "USB-HOST",
> > +			       tu->tahvo_mode == TAHVO_MODE_HOST);
> > +	extcon_set_cable_state(&tu->extcon, "USB", tu->vbus_state);
> > +
> > +	tu->irq = platform_get_irq(pdev, 0);
> > +	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt, 0,
> > +				   "tahvo-vbus", tu);
> 
> since your hardirq handler is NULL, you *must* pass IRQF_ONESHOT.

Ok.

> /Could also use devm_request_threaded_irq()

Does not really help much, since the IRQ has to be freed manually anyway
(to ensure it's disabled before usb_remove_phy(); also looks like it's
currently enabled too early...).

> > +	if (ret) {
> > +		dev_err(&pdev->dev, "could not register tahvo-vbus irq: %d\n",
> > +			ret);
> > +		goto err_disable_clk;
> > +	}
> > +
> > +	/* Attributes */
> > +	ret = device_create_file(&pdev->dev, &dev_attr_vbus_state);
> > +	ret |= device_create_file(&pdev->dev, &dev_attr_otg_mode);
> 
> heh, Greg wrote a very interesting article recently (look at his blog)
> discussing kernel/userland races wrt creation of sysfs files.

[...]

> what if someone accesses the sysfs file you've already created before
> you call dev_set_drvdata? (not sure  if it's possible).

Yes, that's a bug, will fix.

Thanks for comments,

A.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
  2013-07-08 19:44             ` Aaro Koskinen
@ 2013-07-09 11:34               ` Felipe Balbi
  -1 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2013-07-09 11:34 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: Felipe Balbi, linux-usb, linux-omap, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 3968 bytes --]

Hi,

On Mon, Jul 08, 2013 at 10:44:02PM +0300, Aaro Koskinen wrote:
> > > +static void tahvo_usb_become_host(struct tahvo_usb *tu)
> > > +{
> > > +	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
> > > +
> > > +	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
> > > +
> > > +	/* Power up the transceiver in USB host mode */
> > > +	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
> > > +		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
> > > +	tu->phy.state = OTG_STATE_A_IDLE;
> > > +
> > > +	check_vbus_state(tu);
> > > +}
> > > +
> > > +static void tahvo_usb_stop_host(struct tahvo_usb *tu)
> > > +{
> > > +	tu->phy.state = OTG_STATE_A_IDLE;
> > 
> > shouldn't you undo tahvo_usb_become_host() here ? I mean, set the
> > transceiver to SLAVE ?
> 
> tahvo_usb_become_peripheral() (or power_off) is always called after this
> function. Actually, these stop functions can be eliminated altogether
> as they are only called from a single place...

alright, I guess GCC is already inlining them anyway, your choice.

> > > +static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
> > > +{
> > > +	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
> > > +
> > > +	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
> > > +
> > > +	if (otg == NULL)
> > > +		return -ENODEV;
> > > +
> > > +	mutex_lock(&tu->serialize);
> > > +
> > > +	if (host == NULL) {
> > > +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> > > +			tahvo_usb_power_off(tu);
> > > +		otg->host = NULL;
> > > +		mutex_unlock(&tu->serialize);
> > > +		return 0;
> > > +	}
> > > +
> > > +	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
> > > +		otg->host = NULL;
> > > +		tahvo_usb_become_host(tu);
> > > +	}
> > > +
> > > +	otg->host = host;
> > > +
> > > +	mutex_unlock(&tu->serialize);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int tahvo_usb_set_peripheral(struct usb_otg *otg,
> > > +				    struct usb_gadget *gadget)
> > 
> > I want to get rid of the extra 'gadget' and 'host' parameters on
> > ->set_host() and ->set_peripheral(). Nobody really uses them other than:
> > 
> > my_phy->phy.otg->{gadget,host} = {gadget,host};
> > 
> > For that, I need all other PHYs to stop relying on those parameters and
> > I'll cook patches for that for v3.12 merge window.
> 
> How will the PHY know if there is a host/gadget driver present? I guess
> I will need to wait for those patches...

It won't. We're assuming that if the link asks to become a host, it
should already know that there is a host side available :-)

> > > +static int tahvo_usb_probe(struct platform_device *pdev)
> > > +{
> > > +	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
> > > +	struct tahvo_usb *tu;
> > > +	int ret;
> > > +
> > > +	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
> > > +	if (!tu)
> > > +		return -ENOMEM;
> > > +
> > > +	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
> > > +				   GFP_KERNEL);
> > > +	if (!tu->phy.otg)
> > > +		return -ENOMEM;
> > > +
> > > +	tu->pt_dev = pdev;
> > > +
> > > +	/* Default mode */
> > > +#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
> > > +	tu->tahvo_mode = TAHVO_MODE_HOST;
> > > +#else
> > > +	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> > > +#endif
> > 
> > should you call become_host()/become_peripheral() here ?
> 
> Not needed. Once the PHY is registered, the framework will call set_host /
> set_peripheral and the HW gets set to correct state.

ok good, thanks

> > /Could also use devm_request_threaded_irq()
> 
> Does not really help much, since the IRQ has to be freed manually anyway
> (to ensure it's disabled before usb_remove_phy(); also looks like it's
> currently enabled too early...).

you miss a free() in the error path. Switching to
devm_request_threaded_irq() would save you the trouble of having to call
that explicitly.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver
@ 2013-07-09 11:34               ` Felipe Balbi
  0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2013-07-09 11:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Jul 08, 2013 at 10:44:02PM +0300, Aaro Koskinen wrote:
> > > +static void tahvo_usb_become_host(struct tahvo_usb *tu)
> > > +{
> > > +	struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
> > > +
> > > +	extcon_set_cable_state(&tu->extcon, "USB-HOST", true);
> > > +
> > > +	/* Power up the transceiver in USB host mode */
> > > +	retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
> > > +		   USBR_MASTER_SW2 | USBR_MASTER_SW1);
> > > +	tu->phy.state = OTG_STATE_A_IDLE;
> > > +
> > > +	check_vbus_state(tu);
> > > +}
> > > +
> > > +static void tahvo_usb_stop_host(struct tahvo_usb *tu)
> > > +{
> > > +	tu->phy.state = OTG_STATE_A_IDLE;
> > 
> > shouldn't you undo tahvo_usb_become_host() here ? I mean, set the
> > transceiver to SLAVE ?
> 
> tahvo_usb_become_peripheral() (or power_off) is always called after this
> function. Actually, these stop functions can be eliminated altogether
> as they are only called from a single place...

alright, I guess GCC is already inlining them anyway, your choice.

> > > +static int tahvo_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
> > > +{
> > > +	struct tahvo_usb *tu = container_of(otg->phy, struct tahvo_usb, phy);
> > > +
> > > +	dev_dbg(&tu->pt_dev->dev, "%s %p\n", __func__, host);
> > > +
> > > +	if (otg == NULL)
> > > +		return -ENODEV;
> > > +
> > > +	mutex_lock(&tu->serialize);
> > > +
> > > +	if (host == NULL) {
> > > +		if (tu->tahvo_mode == TAHVO_MODE_HOST)
> > > +			tahvo_usb_power_off(tu);
> > > +		otg->host = NULL;
> > > +		mutex_unlock(&tu->serialize);
> > > +		return 0;
> > > +	}
> > > +
> > > +	if (tu->tahvo_mode == TAHVO_MODE_HOST) {
> > > +		otg->host = NULL;
> > > +		tahvo_usb_become_host(tu);
> > > +	}
> > > +
> > > +	otg->host = host;
> > > +
> > > +	mutex_unlock(&tu->serialize);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int tahvo_usb_set_peripheral(struct usb_otg *otg,
> > > +				    struct usb_gadget *gadget)
> > 
> > I want to get rid of the extra 'gadget' and 'host' parameters on
> > ->set_host() and ->set_peripheral(). Nobody really uses them other than:
> > 
> > my_phy->phy.otg->{gadget,host} = {gadget,host};
> > 
> > For that, I need all other PHYs to stop relying on those parameters and
> > I'll cook patches for that for v3.12 merge window.
> 
> How will the PHY know if there is a host/gadget driver present? I guess
> I will need to wait for those patches...

It won't. We're assuming that if the link asks to become a host, it
should already know that there is a host side available :-)

> > > +static int tahvo_usb_probe(struct platform_device *pdev)
> > > +{
> > > +	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
> > > +	struct tahvo_usb *tu;
> > > +	int ret;
> > > +
> > > +	tu = devm_kzalloc(&pdev->dev, sizeof(*tu), GFP_KERNEL);
> > > +	if (!tu)
> > > +		return -ENOMEM;
> > > +
> > > +	tu->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*tu->phy.otg),
> > > +				   GFP_KERNEL);
> > > +	if (!tu->phy.otg)
> > > +		return -ENOMEM;
> > > +
> > > +	tu->pt_dev = pdev;
> > > +
> > > +	/* Default mode */
> > > +#ifdef CONFIG_TAHVO_USB_HOST_BY_DEFAULT
> > > +	tu->tahvo_mode = TAHVO_MODE_HOST;
> > > +#else
> > > +	tu->tahvo_mode = TAHVO_MODE_PERIPHERAL;
> > > +#endif
> > 
> > should you call become_host()/become_peripheral() here ?
> 
> Not needed. Once the PHY is registered, the framework will call set_host /
> set_peripheral and the HW gets set to correct state.

ok good, thanks

> > /Could also use devm_request_threaded_irq()
> 
> Does not really help much, since the IRQ has to be freed manually anyway
> (to ensure it's disabled before usb_remove_phy(); also looks like it's
> currently enabled too early...).

you miss a free() in the error path. Switching to
devm_request_threaded_irq() would save you the trouble of having to call
that explicitly.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130709/213b3138/attachment.sig>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2013-07-09 11:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-18 17:06 [PATCH v3 0/4] USB: OMAP1: Tahvo USB support for 770 Aaro Koskinen
2013-06-18 17:06 ` Aaro Koskinen
2013-06-18 17:06 ` [PATCH v3 1/4] ARM: OMAP1: USB: move omap_usb_config to platform data Aaro Koskinen
2013-06-18 17:06   ` Aaro Koskinen
     [not found] ` <1371575221-360-1-git-send-email-aaro.koskinen-X3B1VOXEql0@public.gmane.org>
2013-06-18 17:06   ` [PATCH v3 2/4] USB: OMAP1: add extcon " Aaro Koskinen
2013-06-18 17:06     ` Aaro Koskinen
2013-06-18 17:07   ` [PATCH v3 3/4] USB: OMAP1: OTG controller driver Aaro Koskinen
2013-06-18 17:07     ` Aaro Koskinen
2013-06-18 17:07   ` [PATCH v3 4/4] USB: OMAP1: Tahvo USB transceiver driver Aaro Koskinen
2013-06-18 17:07     ` Aaro Koskinen
     [not found]     ` <1371575221-360-5-git-send-email-aaro.koskinen-X3B1VOXEql0@public.gmane.org>
2013-07-08  7:21       ` Felipe Balbi
2013-07-08  7:21         ` Felipe Balbi
     [not found]         ` <20130708072109.GA16635-S8G//mZuvNWo5Im9Ml3/Zg@public.gmane.org>
2013-07-08 19:44           ` Aaro Koskinen
2013-07-08 19:44             ` Aaro Koskinen
2013-07-09 11:34             ` Felipe Balbi
2013-07-09 11:34               ` Felipe Balbi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.