Skip to contentSkip to conversation

ARM/usb: add PHY for Lubbock platform

PATCH New Dmitry Eremin-Solenikov 5 participants 11 messages Nov 29, 2014 – Jan 15, 2015

Subsystems (4)
Arm (32-bit) USB Arm sub-architectures PXA2xx/PXA3xx support
lore
Share & tools
lore

Dmitry Eremin-Solenikov

author
#0 lore raw

v2 cover ARM/usb: add PHY for Lubbock platform

This is the second iteration of a compile-tested-only PHY for the Lubbock platform. Could you please take a look and test.

Changes since V1:
- Switched to using threaded IRQs in a phy
- This allowed me to drop workqueue scheduling
- Dropped mach/lubbock.h from pxa25x_udc.h header.

The following changes since commit f114040e3ea6e07372334ade75d1ee0775c355e1:

Linux 3.18-rc1 (2014-10-19 18:08:38 -0700)

are available in the git repository at:

git.infradead.org:public_git/zaurus.git lubbock

for you to fetch changes up to 3ef110afa33e9e5c2ab77a4ababd279f8bd4e95c:

usb: gadget: drop lubbock-specific code from pxa25x_udc (2014-11-30 00:51:31 +0300)

----------------------------------------------------------------
Dmitry Eremin-Solenikov (3):
ARM: pxa: lubbock: add declaration of vbus tranceiver
usb: phy: add lubbock phy driver
usb: gadget: drop lubbock-specific code from pxa25x_udc

 arch/arm/mach-pxa/lubbock.c         |   6 +
 drivers/usb/gadget/udc/pxa25x_udc.c |  61 ----------
 drivers/usb/gadget/udc/pxa25x_udc.h |   5 -
 drivers/usb/phy/Kconfig             |  10 ++
 drivers/usb/phy/Makefile            |   1 +
 drivers/usb/phy/phy-lubbock.c       | 220 ++++++++++++++++++++++++++++++++++++
 6 files changed, 237 insertions(+), 66 deletions(-)
 create mode 100644 drivers/usb/phy/phy-lubbock.c
author
#1 lore raw

PATCH v2 1/3 ARM: pxa: lubbock: add declaration of vbus tranceiver

Add a declaration of lubbock-vbus device, used as UDC transceiver on Lubbock platform.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 arch/arm/mach-pxa/lubbock.c | 6 ++++++
 1 file changed, 6 insertions(+)
arch/arm/mach-pxa/lubbock.c +6 -0
index d8a1be6..5e2dcf7 100644
@@ -383,11 +383,17 @@ static struct platform_device lubbock_flash_device[2] = {
 	},
 };
 
+static struct platform_device lubbock_vbus = {
+	.name	= "lubbock-vbus",
+	.id	= -1,
+};
+
 static struct platform_device *devices[] __initdata = {
 	&sa1111_device,
 	&smc91x_device,
 	&lubbock_flash_device[0],
 	&lubbock_flash_device[1],
+	&lubbock_vbus,
 };
 
 static struct pxafb_mode_info sharp_lm8v31_mode = {
Signature · 1 line
2.1.3
author
#2 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

In reply to Dmitry Eremin-Solenikov

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy driver determines connector/VBUS status by reading CPLD register. Also it uses a work to call into udc stack, instead of pinging vbus session right from irq handler.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 drivers/usb/phy/Kconfig       |  10 ++
 drivers/usb/phy/Makefile      |   1 +
 drivers/usb/phy/phy-lubbock.c | 220 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 231 insertions(+)
 create mode 100644 drivers/usb/phy/phy-lubbock.c
drivers/usb/phy/Kconfig +10 -0
index 0cd1f44..98b1682 100644
@@ -137,6 +137,16 @@ config USB_ISP1301
 	  To compile this driver as a module, choose M here: the
 	  module will be called phy-isp1301.
 
+config USB_LUBBOCK
+	tristate "USB VBUS PHY Driver for DBPXA250 Lubbock platform"
+	depends on ARCH_LUBBOCK
+	help
+	  Say Y here to add support for the USB Gadget VBUS tranceiver driver
+	  for DBPXA250 (Lubbock) platform.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called phy-lubbock.
+
 config USB_MSM_OTG
 	tristate "Qualcomm on-chip USB OTG controller support"
 	depends on (USB || USB_GADGET) && (ARCH_MSM || ARCH_QCOM || COMPILE_TEST)
drivers/usb/phy/Makefile +1 -0
index 75f2bba..0fe461c 100644
@@ -19,6 +19,7 @@ obj-$(CONFIG_TWL6030_USB)		+= phy-twl6030-usb.o
 obj-$(CONFIG_USB_EHCI_TEGRA)		+= phy-tegra-usb.o
 obj-$(CONFIG_USB_GPIO_VBUS)		+= phy-gpio-vbus-usb.o
 obj-$(CONFIG_USB_ISP1301)		+= phy-isp1301.o
+obj-$(CONFIG_USB_LUBBOCK)		+= phy-lubbock.o
 obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
 obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
 obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
drivers/usb/phy/phy-lubbock.c new file +220 -0

220 lines not on this page · raw message

Signature · 1 line
2.1.3
#3 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

Dmitry,

Quoted 2 lines · Dmitry Eremin-Solenikov

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy

[]

drivers/usb/phy/Kconfig config USB_ISP1301 · line 144

Quoted 13 lines
drivers/usb/phy/Kconfig +5 -0
index 0cd1f44..98b1682 100644
@@ -137,6 +137,16 @@ config USB_ISP1301
 	  To compile this driver as a module, choose M here: the
 	  module will be called phy-isp1301.
 
+config USB_LUBBOCK
+	tristate "USB VBUS PHY Driver for DBPXA250 Lubbock platform"
+	depends on ARCH_LUBBOCK
+	help
+	  Say Y here to add support for the USB Gadget VBUS tranceiver driver

                                                        ^^^^^^^^^^
                                                        transceiver

drivers/usb/phy/Makefile

Quoted 10 lines
drivers/usb/phy/Kconfig +5 -0
+	  for DBPXA250 (Lubbock) platform.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called phy-lubbock.
+
 config USB_MSM_OTG
 	tristate "Qualcomm on-chip USB OTG controller support"
 	depends on (USB || USB_GADGET) && (ARCH_MSM || ARCH_QCOM || COMPILE_TEST)
drivers/usb/phy/Makefile +1 -0
+};

[]

Quoted 9 lines
drivers/usb/phy/Makefile +5 -0
+module_platform_driver(lubbock_vbus_driver);
+
+MODULE_DESCRIPTION("OTG transceiver driver for DBPXA250 Lubbock platform");
+MODULE_AUTHOR("Dmitry Eremin-Solenikov");
+MODULE_LICENSE("GPL v2");
Signature · 3 lines
2.1.3

--

[]

Signature · 1 line
- Jeremiah Mahler
maintainer
#4 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

In reply to Dmitry Eremin-Solenikov
Quoted 4 lines · Dmitry Eremin-Solenikov

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy driver determines connector/VBUS status by reading CPLD register. Also it uses a work to call into udc stack, instead of pinging vbus session right from irq handler.

This comment is not accurate anymore, right ? The work call, etc ...

Moreover, I have this compile error:
drivers/built-in.o: In function `lubbock_vbus_remove':
/home/rj/mio_linux/kernel/drivers/usb/phy/phy-lubbock.c:200: undefined reference to usb_remove_phy' drivers/built-in.o: In function lubbock_vbus_probe':
/home/rj/mio_linux/kernel/drivers/usb/phy/phy-lubbock.c:186: undefined reference to `usb_add_phy' Makefile:922: recipe for target 'vmlinux' failed

A select in Kconfig is missing, right ?
And then :
genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 294 lubbock-vbus lubbock-vbus: can't request irq 294, err: -22
lubbock-vbus: probe of lubbock-vbus failed with error -22

drivers/usb/phy/phy-lubbock.c line 33

Quoted 3 lines
drivers/usb/phy/phy-lubbock.c new file +3 -0
+static int is_vbus_powered(void)
+{
+	return !(LUB_MISC_RD && BIT(9));

That BIT(9) is a bit ugly. Moreover the "&&" is certainly wrong. A define somewhere would be fine.

drivers/usb/phy/phy-lubbock.c line 36

Quoted 3 lines
drivers/usb/phy/phy-lubbock.c new file +3 -0
+}
+
+static void lubbock_vbus_handle(struct lubbock_vbus_data *lubbock_vbus)

I have not reviewed that one thoroughly ...

Quoted 19 lines
drivers/usb/phy/phy-lubbock.c new file +19 -0
+
+/* VBUS change IRQ handler */
+static irqreturn_t lubbock_vbus_irq(int irq, void *data)
+{
+	struct platform_device *pdev = data;
+	struct lubbock_vbus_data *lubbock_vbus = platform_get_drvdata(pdev);
+	struct usb_otg *otg = lubbock_vbus->phy.otg;
+
+	dev_dbg(&pdev->dev, "VBUS %s (gadget: %s)\n",
+		is_vbus_powered() ? "supplied" : "inactive",
+		otg->gadget ? otg->gadget->name : "none");
+
+	switch (irq) {
+	case LUBBOCK_USB_IRQ:
+		disable_irq(LUBBOCK_USB_IRQ);
+		enable_irq(LUBBOCK_USB_DISC_IRQ);
+		break;
+	case LUBBOCK_USB_DISC_IRQ:
+		disable_irq(LUBBOCK_USB_DISC_IRQ);

drivers/usb/phy/phy-lubbock.c line 94

Quoted 10 lines
drivers/usb/phy/phy-lubbock.c new file +10 -0
+		enable_irq(LUBBOCK_USB_IRQ);
+		break;
+	default:
+		return IRQ_NONE;
+	}
+
+	/*
+	 * No need to use workqueue here - we are in a threded handler,
+	 * so we can sleep.
+	 */

What if a new interrupt occurs in here, and preempts this thread.

drivers/usb/phy/phy-lubbock.c line 96

Quoted 2 lines
drivers/usb/phy/phy-lubbock.c new file +2 -0
+	if (otg->gadget)
+		lubbock_vbus_handle(lubbock_vbus);

I think the enable_irq() call should be here. I can't have an ordering problem at this point, right ?

Quoted 5 lines
drivers/usb/phy/phy-lubbock.c new file +5 -0
+	err = devm_request_threaded_irq(&pdev->dev, LUBBOCK_USB_DISC_IRQ,
+			NULL, lubbock_vbus_irq, 0, "vbus disconnect", pdev);
+	if (err) {
+		dev_err(&pdev->dev, "can't request irq %i, err: %d\n",
+			LUBBOCK_USB_DISC_IRQ, err);

drivers/usb/phy/phy-lubbock.c line 183

Quoted 10 lines
drivers/usb/phy/phy-lubbock.c new file +10 -0
+		return err;
+	}
+
+	err = devm_request_threaded_irq(&pdev->dev, LUBBOCK_USB_IRQ,
+			NULL, lubbock_vbus_irq, 0, "vbus connect", pdev);
+	if (err) {
+		dev_err(&pdev->dev, "can't request irq %i, err: %d\n",
+			LUBBOCK_USB_IRQ, err);
+		return err;
+	}

Here you have both interrupts enabled, this will mean one interrupt at least will fire. And of course the other one will be enabled a second time, hence imbalance.

If you want to have an initial status of disconnected gadget, just enable ti connect interrupt at probing.

Cheers.

Signature · 1 line
Robert
#5 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

In reply to Dmitry Eremin-Solenikov
Quoted 9 lines · Dmitry Eremin-Solenikov

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy driver determines connector/VBUS status by reading CPLD register. Also it uses a work to call into udc stack, instead of pinging vbus session right from irq handler.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 drivers/usb/phy/Kconfig       |  10 ++
 drivers/usb/phy/Makefile      |   1 +

new phy drivers under drivers/phy only, sorry.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150108/1b0d65ee/attachment.sig>
author
#6 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

Hello,

2015-01-08 19:58 GMT+03:00 Felipe Balbi <[email protected]>:

Quoted 6 lines · Dmitry Eremin-Solenikov
Quoted 5 lines

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy driver determines connector/VBUS status by reading CPLD register. Also it uses a work to call into udc stack, instead of pinging vbus session right from irq handler.

Quoted 6 lines
Quoted 4 lines
Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 drivers/usb/phy/Kconfig       |  10 ++
 drivers/usb/phy/Makefile      |   1 +

new phy drivers under drivers/phy only, sorry.

Hmm. How do drivers/phy drivers coordinate with usb gadget subsystem? I see none of them calling usb_gadget_vbus_connect/disconnect().

Signature · 2 lines
With best wishes
Dmitry
#7 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

Quoted 14 lines · Dmitry Eremin-Solenikov

Hello,

2015-01-08 19:58 GMT+03:00 Felipe Balbi <[email protected]>:

Quoted 11 lines · Dmitry Eremin-Solenikov
Quoted 9 lines

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy driver determines connector/VBUS status by reading CPLD register. Also it uses a work to call into udc stack, instead of pinging vbus session right from irq handler.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 drivers/usb/phy/Kconfig       |  10 ++
 drivers/usb/phy/Makefile      |   1 +
Quoted 4 lines
Quoted 1 line

new phy drivers under drivers/phy only, sorry.

Hmm. How do drivers/phy drivers coordinate with usb gadget subsystem? I see none of them calling usb_gadget_vbus_connect/disconnect().

I'll leave that to Kishon, since he wrote drivers/phy. Kishon, any hints?

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150112/3d24ea70/attachment-0001.sig>
#8 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

Hi,

Quoted 15 lines · Felipe Balbi
Quoted 14 lines · Dmitry Eremin-Solenikov

Hello,

2015-01-08 19:58 GMT+03:00 Felipe Balbi <[email protected]>:

Quoted 11 lines · Dmitry Eremin-Solenikov
Quoted 9 lines

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy driver determines connector/VBUS status by reading CPLD register. Also it uses a work to call into udc stack, instead of pinging vbus session right from irq handler.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 drivers/usb/phy/Kconfig       |  10 ++
 drivers/usb/phy/Makefile      |   1 +
Quoted 7 lines
Quoted 4 lines
Quoted 1 line

new phy drivers under drivers/phy only, sorry.

Hmm. How do drivers/phy drivers coordinate with usb gadget subsystem? I see none of them calling usb_gadget_vbus_connect/disconnect().

I'll leave that to Kishon, since he wrote drivers/phy. Kishon, any hints?

Extcon framework can be used to detect the connector status. So the PHY driver should register with the extcon framework if it has the capability to determine the connector status and the gadget driver should register with the extcon framework if it has to receive the connector status.

I'm not sure if we should be adding these extcon APIs inside the PHY framework itself as all PHYs might not have the capability to detect the connector status.

Thanks
Kishon

author
#9 lore raw

PATCH v2 2/3 usb: phy: add lubbock phy driver

Hello,

2015-01-13 9:10 GMT+03:00 Kishon Vijay Abraham I <[email protected]>:

Quoted 23 lines

Hi,

Quoted 20 lines · Felipe Balbi
Quoted 18 lines · Dmitry Eremin-Solenikov

Hello,

2015-01-08 19:58 GMT+03:00 Felipe Balbi <[email protected]>:

Quoted 6 lines · Dmitry Eremin-Solenikov
Quoted 5 lines

Extract lubbock-specific code from pxa25x_udc driver. As a bonus, phy driver determines connector/VBUS status by reading CPLD register. Also it uses a work to call into udc stack, instead of pinging vbus session right from irq handler.

Quoted 6 lines
Quoted 4 lines
Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 drivers/usb/phy/Kconfig       |  10 ++
 drivers/usb/phy/Makefile      |   1 +

new phy drivers under drivers/phy only, sorry.

Hmm. How do drivers/phy drivers coordinate with usb gadget subsystem? I see none of them calling usb_gadget_vbus_connect/disconnect().

Quoted 7 lines
Quoted 2 lines

I'll leave that to Kishon, since he wrote drivers/phy. Kishon, any hints?

Extcon framework can be used to detect the connector status. So the PHY driver should register with the extcon framework if it has the capability to determine the connector status and the gadget driver should register with the extcon framework if it has to receive the connector status.

If I understand correctly, this means the whole usb gadget/otg subsystem needs to be redesigned/refactored to support extconn drivers. Is it planned already? Can we still submit drivers to an old usb-phy subsystem as 'new phy' subsystem does not provide us necessary integration with usb-gadget subsystem?

Quoted 3 lines

I'm not sure if we should be adding these extcon APIs inside the PHY framework itself as all PHYs might not have the capability to detect the connector status.

In fact I have several devices for which I had to implement a simple usb-phy that is able to control D+ pullup, but isn't able to detect VBUS presense and thus has to assume that the device is always connected.

Signature · 2 lines
With best wishes
Dmitry
author
#10 lore raw

PATCH v2 3/3 usb: gadget: drop lubbock-specific code from pxa25x_udc

In reply to Dmitry Eremin-Solenikov

Now, as there is a special phy for lubbock platforms, drop all lubbock-specific code from pxa25x udc driver. It should use phy instead.

Signed-off-by Dmitry Eremin-Solenikov <[email protected]>
---
 drivers/usb/gadget/udc/pxa25x_udc.c | 61 -------------------------------------
 drivers/usb/gadget/udc/pxa25x_udc.h |  5 ---
 2 files changed, 66 deletions(-)
drivers/usb/gadget/udc/pxa25x_udc.c +0 -61
index 42f7eeb..e3d0715 100644
@@ -56,10 +56,6 @@
 #include <mach/hardware.h>
 #endif
 
-#ifdef CONFIG_ARCH_LUBBOCK
-#include <mach/lubbock.h>
-#endif
-
 /*
  * This driver handles the USB Device Controller (UDC) in Intel's PXA 25x
  * series processors.  The UDC for the IXP 4xx series is very similar.

@@ -1334,43 +1330,6 @@ static int pxa25x_udc_stop(struct usb_gadget*g,
 
 /*-------------------------------------------------------------------------*/
 
-#ifdef CONFIG_ARCH_LUBBOCK
-
-/* Lubbock has separate connect and disconnect irqs.  More typical designs
- * use one GPIO as the VBUS IRQ, and another to control the D+ pullup.
- */
-
-static irqreturn_t
-lubbock_vbus_irq(int irq, void *_dev)
-{
-	struct pxa25x_udc	*dev = _dev;
-	int			vbus;
-
-	dev->stats.irqs++;
-	switch (irq) {
-	case LUBBOCK_USB_IRQ:
-		vbus = 1;
-		disable_irq(LUBBOCK_USB_IRQ);
-		enable_irq(LUBBOCK_USB_DISC_IRQ);
-		break;
-	case LUBBOCK_USB_DISC_IRQ:
-		vbus = 0;
-		disable_irq(LUBBOCK_USB_DISC_IRQ);
-		enable_irq(LUBBOCK_USB_IRQ);
-		break;
-	default:
-		return IRQ_NONE;
-	}
-
-	pxa25x_udc_vbus_session(&dev->gadget, vbus);
-	return IRQ_HANDLED;
-}
-
-#endif
-
-
-/*-------------------------------------------------------------------------*/
-
 static inline void clear_ep_state (struct pxa25x_udc *dev)
 {
 	unsigned i;

@@ -2154,26 +2113,6 @@ static int pxa25x_udc_probe(struct platform_device *pdev)
 	}
 	dev->got_irq = 1;
 
-#ifdef CONFIG_ARCH_LUBBOCK
-	if (machine_is_lubbock()) {
-		retval = devm_request_irq(&pdev->dev, LUBBOCK_USB_DISC_IRQ,
-					  lubbock_vbus_irq, 0, driver_name,
-					  dev);
-		if (retval != 0) {
-			pr_err("%s: can't get irq %i, err %d\n",
-				driver_name, LUBBOCK_USB_DISC_IRQ, retval);
-			goto err;
-		}
-		retval = devm_request_irq(&pdev->dev, LUBBOCK_USB_IRQ,
-					  lubbock_vbus_irq, 0, driver_name,
-					  dev);
-		if (retval != 0) {
-			pr_err("%s: can't get irq %i, err %d\n",
-				driver_name, LUBBOCK_USB_IRQ, retval);
-			goto err;
-		}
-	} else
-#endif
 	create_debug_files(dev);
 
 	retval = usb_add_gadget_udc(&pdev->dev, &dev->gadget);
drivers/usb/gadget/udc/pxa25x_udc.h +0 -5
index 3fe5931..59c6bbc 100644
@@ -130,11 +130,6 @@ struct pxa25x_udc {
 
 /*-------------------------------------------------------------------------*/
 
-#ifdef CONFIG_ARCH_LUBBOCK
-#include <mach/lubbock.h>
-/* lubbock can also report usb connect/disconnect irqs */
-#endif
-
 static struct pxa25x_udc *the_controller;
 
 /*-------------------------------------------------------------------------*/
Signature · 1 line
2.1.3