Skip to contentSkip to conversation

Create separate header for ehci-dbgp driver

PATCH New Chris Rorvick 1 participant 11 messages Nov 5, 2014 USB

lore
Share & tools
lore

Chris Rorvick

author
#0 lore raw

cover Create separate header for ehci-dbgp driver

The FUSBH200 and FOTG210 are not EHCI-compatible and require standalone drivers. See discussion at:

http://comments.gmane.org/gmane.linux.usb.general/84169

But these controllers do implement an EHCI-compatible debug port and therefore leverage the ehci-dbgp driver. Rather than pulling in the necessary declarations from <linux/usb/ehci_def.h>, each driver copies this code into their own header. The goal of this series is to pull the ehci-dbgp related code into its own header to remove the need for this redundancy.

Regards,

Chris Rorvick

Chris Rorvick (10):
usb: Create separate header for ehci-dbgp
fusbh200: Make Xen notification consistent with EHCI
fusbh200: Remove superfluous macro definitions
fusbh200: Remove duplicate ehci-dbgp declarations
fusbh200: Use ehci_dbg_port struct
fotg210: Make Xen notification consistent with EHCI
fotg210: Remove superfluous macro definitions
fotg210: Remove duplicate ehci-dbgp declarations
fotg210: Use ehci_dbg_port struct
usb: Remove __init from early_dbgp_init() prototype

 drivers/usb/host/fotg210.h    | 62 ++------------------------------
 drivers/usb/host/fusbh200.h   | 62 ++------------------------------
 include/linux/usb/ehci-dbgp.h | 83 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/ehci_def.h  | 65 ++-------------------------------
 4 files changed, 91 insertions(+), 181 deletions(-)
 create mode 100644 include/linux/usb/ehci-dbgp.h
Signature · 1 line
1.9.3
author
#1 lore raw

PATCH 1/10 usb: Create separate header for ehci-dbgp

The FUSBH200 and FOTG210 controllers implement sufficiently EHCI- compatible debug ports to leverage ehci-dbgp from their respective drivers. Rather than including <linux/usb/ehci_def.h> header, though, they replicate the necessary declarations in their own headers. Move the ehci-dbgp stuff into its own header as a first step towards removing this redundancy.

Signed-off-by Chris Rorvick <[email protected]>
---
 include/linux/usb/ehci-dbgp.h | 84 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/ehci_def.h  | 65 ++-------------------------------
 2 files changed, 86 insertions(+), 63 deletions(-)
 create mode 100644 include/linux/usb/ehci-dbgp.h
include/linux/usb/ehci-dbgp.h new file +84 -0
new file mode 100644
index 0000000..796c1cd
@@ -0,0 +1,84 @@
+/*
+ * Standalone EHCI usb debug driver
+ *
+ * Originally written by:
+ *  Eric W. Biederman" <[email protected]> and
+ *  Yinghai Lu <[email protected]>
+ *
+ * Changes for early/late printk and HW errata:
+ *  Jason Wessel <[email protected]>
+ *  Copyright (C) 2009 Wind River Systems, Inc.
+ *
+ */
+
+#ifndef __LINUX_USB_EHCI_DBGP_H
+#define __LINUX_USB_EHCI_DBGP_H
+
+#include <linux/console.h>
+#include <linux/types.h>
+
+/* Appendix C, Debug port ... intended for use with special "debug devices"
+ * that can help if there's no serial console.  (nonstandard enumeration.)
+ */
+struct ehci_dbg_port {
+	u32	control;
+#define DBGP_OWNER	(1<<30)
+#define DBGP_ENABLED	(1<<28)
+#define DBGP_DONE	(1<<16)
+#define DBGP_INUSE	(1<<10)
+#define DBGP_ERRCODE(x)	(((x)>>7)&0x07)
+#	define DBGP_ERR_BAD	1
+#	define DBGP_ERR_SIGNAL	2
+#define DBGP_ERROR	(1<<6)
+#define DBGP_GO		(1<<5)
+#define DBGP_OUT	(1<<4)
+#define DBGP_LEN(x)	(((x)>>0)&0x0f)
+	u32	pids;
+#define DBGP_PID_GET(x)		(((x)>>16)&0xff)
+#define DBGP_PID_SET(data, tok)	(((data)<<8)|(tok))
+	u32	data03;
+	u32	data47;
+	u32	address;
+#define DBGP_EPADDR(dev, ep)	(((dev)<<8)|(ep))
+};
+
+#ifdef CONFIG_EARLY_PRINTK_DBGP
+#include <linux/init.h>
+extern int __init early_dbgp_init(char *s);
+extern struct console early_dbgp_console;
+#endif /* CONFIG_EARLY_PRINTK_DBGP */
+
+struct usb_hcd;
+
+#ifdef CONFIG_XEN_DOM0
+extern int xen_dbgp_reset_prep(struct usb_hcd *);
+extern int xen_dbgp_external_startup(struct usb_hcd *);
+#else
+static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
+{
+	return 1; /* Shouldn't this be 0? */
+}
+
+static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
+{
+	return -1;
+}
+#endif
+
+#ifdef CONFIG_EARLY_PRINTK_DBGP
+/* Call backs from ehci host driver to ehci debug driver */
+extern int dbgp_external_startup(struct usb_hcd *);
+extern int dbgp_reset_prep(struct usb_hcd *);
+#else
+static inline int dbgp_reset_prep(struct usb_hcd *hcd)
+{
+	return xen_dbgp_reset_prep(hcd);
+}
+
+static inline int dbgp_external_startup(struct usb_hcd *hcd)
+{
+	return xen_dbgp_external_startup(hcd);
+}
+#endif
+
+#endif /* __LINUX_USB_EHCI_DBGP_H */
include/linux/usb/ehci_def.h +2 -63

75 lines not on this page · raw message

Signature · 1 line
1.9.3
author
#2 lore raw

PATCH 2/10 fusbh200: Make Xen notificaiton consistent with EHCI

In reply to Chris Rorvick

If CONFIG_XEN_DOM0 is enabled, the ehci-dbgp driver notifies Xen of controller reset events via xen_dbgp_reset_prep() and xen_dbgp_external_startup() (via calls to xen_dbgp_op().) Otherwise <linux/usb/ehci_def.h> defines them as no-ops to disable this logic.

The fusbh200 driver copies much of the dbgp code from ehci_def.h, but it unconditionally defines the Xen hooks as no-ops, effectively disabling these notifications when CONFIG_EARLY_PRINTK_DBGP is disabled. When enabled, though, notifying Xen is dependent on CONFIG_XEN_DOM0 due to fusbh200 leveraging the ehci-dbgp driver.

The following table compares the implementations of xen_dbgp_reset_prep() and xen_dbgp_external_startup() in the ehci-dbgp and fusbh200 drivers under the relevant configurations:

  EARLY_PRINTK_DBGP?  XEN_DOM0?  ehci-dbgp      fusbh200
  ------------------  ---------  -------------  -------------
  n                   n          no-op          no-op
  n                   y          xen_dbgp_op()  no-op
  y                   n          no-op          no-op
  y                   y          xen_dbgp_op()  xen_dbgp_op()

This suggests that fusbh200 is, at best, indifferent to whether Xen is notified of these events. Make fusbh200 consistent with ehci-dbgp as a step towards consolidating this code duplication.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fusbh200.h | 5 +++++
 1 file changed, 5 insertions(+)
drivers/usb/host/fusbh200.h +5 -0
index 6b719e0..6e7b8c1 100644
@@ -318,6 +318,10 @@ extern struct console early_dbgp_console;
 
 struct usb_hcd;
 
+#ifdef CONFIG_XEN_DOM0
+extern int xen_dbgp_reset_prep(struct usb_hcd *);
+extern int xen_dbgp_external_startup(struct usb_hcd *);
+#else
 static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
 {
 	return 1; /* Shouldn't this be 0? */

@@ -327,6 +331,7 @@ static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
 {
 	return -1;
 }
+#endif
 
 #ifdef CONFIG_EARLY_PRINTK_DBGP
 /* Call backs from fusbh200 host driver to fusbh200 debug driver */
Signature · 1 line
1.9.3
author
#3 lore raw

PATCH 3/10 fusbh200: Remove superfluous macro definitions

In reply to Chris Rorvick

The fusbh200_dbg_port struct is a copy of the ehci_dbg_port definition in the <linux/usb/ehci_def.h> header. Embedded in this definition are a number of macros which came along for the ride. These macros are not used in the fusbh200 driver and will conflict those in the new <linux/usb/ehci-dbgp.h> header.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fusbh200.h | 14 --------------
 1 file changed, 14 deletions(-)
drivers/usb/host/fusbh200.h +0 -14
index 6e7b8c1..da7152b 100644
@@ -290,24 +290,10 @@ struct fusbh200_regs {
  */
 struct fusbh200_dbg_port {
 	u32	control;
-#define DBGP_OWNER	(1<<30)
-#define DBGP_ENABLED	(1<<28)
-#define DBGP_DONE	(1<<16)
-#define DBGP_INUSE	(1<<10)
-#define DBGP_ERRCODE(x)	(((x)>>7)&0x07)
-#	define DBGP_ERR_BAD	1
-#	define DBGP_ERR_SIGNAL	2
-#define DBGP_ERROR	(1<<6)
-#define DBGP_GO		(1<<5)
-#define DBGP_OUT	(1<<4)
-#define DBGP_LEN(x)	(((x)>>0)&0x0f)
 	u32	pids;
-#define DBGP_PID_GET(x)		(((x)>>16)&0xff)
-#define DBGP_PID_SET(data, tok)	(((data)<<8)|(tok))
 	u32	data03;
 	u32	data47;
 	u32	address;
-#define DBGP_EPADDR(dev, ep)	(((dev)<<8)|(ep))
 };
 
 #ifdef CONFIG_EARLY_PRINTK_DBGP
Signature · 1 line
1.9.3
author
#4 lore raw

PATCH 4/10 fusbh200: Remove duplicate ehci-dbgp declarations

In reply to Chris Rorvick

Now that ehci-dbgp has its own header, use it rather than duplicating the declarations, etc.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fusbh200.h | 40 ++--------------------------------------
 1 file changed, 2 insertions(+), 38 deletions(-)
drivers/usb/host/fusbh200.h +2 -38
index da7152b..47aecfc 100644
@@ -1,6 +1,8 @@
 #ifndef __LINUX_FUSBH200_H
 #define __LINUX_FUSBH200_H
 
+#include <linux/usb/ehci-dbgp.h>
+
 /* definitions used for the EHCI driver */
 
 /*

@@ -296,44 +298,6 @@ struct fusbh200_dbg_port {
 	u32	address;
 };
 
-#ifdef CONFIG_EARLY_PRINTK_DBGP
-#include <linux/init.h>
-extern int __init early_dbgp_init(char *s);
-extern struct console early_dbgp_console;
-#endif /* CONFIG_EARLY_PRINTK_DBGP */
-
-struct usb_hcd;
-
-#ifdef CONFIG_XEN_DOM0
-extern int xen_dbgp_reset_prep(struct usb_hcd *);
-extern int xen_dbgp_external_startup(struct usb_hcd *);
-#else
-static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
-{
-	return 1; /* Shouldn't this be 0? */
-}
-
-static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
-{
-	return -1;
-}
-#endif
-
-#ifdef CONFIG_EARLY_PRINTK_DBGP
-/* Call backs from fusbh200 host driver to fusbh200 debug driver */
-extern int dbgp_external_startup(struct usb_hcd *);
-extern int dbgp_reset_prep(struct usb_hcd *hcd);
-#else
-static inline int dbgp_reset_prep(struct usb_hcd *hcd)
-{
-	return xen_dbgp_reset_prep(hcd);
-}
-static inline int dbgp_external_startup(struct usb_hcd *hcd)
-{
-	return xen_dbgp_external_startup(hcd);
-}
-#endif
-
 /*-------------------------------------------------------------------------*/
 
 #define	QTD_NEXT(fusbh200, dma)	cpu_to_hc32(fusbh200, (u32)dma)
Signature · 1 line
1.9.3
author
#5 lore raw

PATCH 5/10 fusbh200: Use ehci_dbg_port struct

In reply to Chris Rorvick

The FUSBH200 debug port has a EHCI-compatible register layout so there is no need to define a custom struct.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fusbh200.h | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)
drivers/usb/host/fusbh200.h +1 -12
index 47aecfc..d6e5b3d 100644
@@ -86,7 +86,7 @@ struct fusbh200_hcd {			/* one per controller */
 	/* glue to PCI and HCD framework */
 	struct fusbh200_caps __iomem *caps;
 	struct fusbh200_regs __iomem *regs;
-	struct fusbh200_dbg_port __iomem *debug;
+	struct ehci_dbg_port __iomem *debug;
 
 	__u32			hcs_params;	/* cached register copy */
 	spinlock_t		lock;

@@ -287,17 +287,6 @@ struct fusbh200_regs {
 #define BMIER_VBUS_ERR_EN	(1<<0)
 };
 
-/* Appendix C, Debug port ... intended for use with special "debug devices"
- * that can help if there's no serial console.  (nonstandard enumeration.)
- */
-struct fusbh200_dbg_port {
-	u32	control;
-	u32	pids;
-	u32	data03;
-	u32	data47;
-	u32	address;
-};
-
 /*-------------------------------------------------------------------------*/
 
 #define	QTD_NEXT(fusbh200, dma)	cpu_to_hc32(fusbh200, (u32)dma)
Signature · 1 line
1.9.3
author
#6 lore raw

PATCH 6/10 fotg210: Make Xen notification consistent with EHCI

In reply to Chris Rorvick

If CONFIG_XEN_DOM0 is enabled, the ehci-dbgp driver notifies Xen of controller reset events via xen_dbgp_reset_prep() and xen_dbgp_external_startup() (via calls to xen_dbgp_op().) Otherwise <linux/usb/ehci_def.h> defines them as no-ops to disable this logic.

The fotg210 driver copies much of the dbgp code from ehci_def.h, but it unconditionally defines the Xen hooks as no-ops, effectively disabling these notifications when CONFIG_EARLY_PRINTK_DBGP is disabled. When enabled, though, notifying Xen is dependent on CONFIG_XEN_DOM0 due to fotg210 leveraging the ehci-dbgp driver.

The following table compares the implementations of xen_dbgp_reset_prep() and xen_dbgp_external_startup() in the ehci-dbgp and fotg210 drivers under the relevant configurations:

  EARLY_PRINTK_DBGP?  XEN_DOM0?  ehci-dbgp      fotg210
  ------------------  ---------  -------------  -------------
  n                   n          no-op          no-op
  n                   y          xen_dbgp_op()  no-op
  y                   n          no-op          no-op
  y                   y          xen_dbgp_op()  xen_dbgp_op()

This suggests that fotg210 is, at best, indifferent to whether Xen is notified of these events. Make fotg210 consistent with ehci-dbgp as a step towards consolidating this code duplication.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fotg210.h | 5 +++++
 1 file changed, 5 insertions(+)
drivers/usb/host/fotg210.h +5 -0
index ac6cd1b..98c9670 100644
@@ -326,6 +326,10 @@ extern struct console early_dbgp_console;
 
 struct usb_hcd;
 
+#ifdef CONFIG_XEN_DOM0
+extern int xen_dbgp_reset_prep(struct usb_hcd *);
+extern int xen_dbgp_external_startup(struct usb_hcd *);
+#else
 static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
 {
 	return 1; /* Shouldn't this be 0? */

@@ -335,6 +339,7 @@ static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
 {
 	return -1;
 }
+#endif
 
 #ifdef CONFIG_EARLY_PRINTK_DBGP
 /* Call backs from fotg210 host driver to fotg210 debug driver */
Signature · 1 line
1.9.3
author
#7 lore raw

PATCH 7/10 fotg210: Remove superfluous macro definitions

In reply to Chris Rorvick

The fotg210_dbg_port struct is a copy of the ehci_dbg_port definition in the <linux/usb/ehci_def.h> header. Embedded in this definition are a number of macros which came along for the ride. These macros are not used in the fotg210 driver and will conflict those in the new <linux/usb/ehci-dbgp.h> header.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fotg210.h | 14 --------------
 1 file changed, 14 deletions(-)
drivers/usb/host/fotg210.h +0 -14
index 98c9670..c2e5134 100644
@@ -298,24 +298,10 @@ struct fotg210_regs {
  */
 struct fotg210_dbg_port {
 	u32	control;
-#define DBGP_OWNER	(1<<30)
-#define DBGP_ENABLED	(1<<28)
-#define DBGP_DONE	(1<<16)
-#define DBGP_INUSE	(1<<10)
-#define DBGP_ERRCODE(x)	(((x)>>7)&0x07)
-#	define DBGP_ERR_BAD	1
-#	define DBGP_ERR_SIGNAL	2
-#define DBGP_ERROR	(1<<6)
-#define DBGP_GO		(1<<5)
-#define DBGP_OUT	(1<<4)
-#define DBGP_LEN(x)	(((x)>>0)&0x0f)
 	u32	pids;
-#define DBGP_PID_GET(x)		(((x)>>16)&0xff)
-#define DBGP_PID_SET(data, tok)	(((data)<<8)|(tok))
 	u32	data03;
 	u32	data47;
 	u32	address;
-#define DBGP_EPADDR(dev, ep)	(((dev)<<8)|(ep))
 };
 
 #ifdef CONFIG_EARLY_PRINTK_DBGP
Signature · 1 line
1.9.3
author
#8 lore raw

PATCH 8/10 fotg210: Remove duplicate ehci-dbgp declarations

In reply to Chris Rorvick

Now that ehci-dbgp has its own header, use it rather than duplicating the declarations, etc.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fotg210.h | 40 ++--------------------------------------
 1 file changed, 2 insertions(+), 38 deletions(-)
drivers/usb/host/fotg210.h +2 -38
index c2e5134..975d9bb 100644
@@ -1,6 +1,8 @@
 #ifndef __LINUX_FOTG210_H
 #define __LINUX_FOTG210_H
 
+#include <linux/usb/ehci-dbgp.h>
+
 /* definitions used for the EHCI driver */
 
 /*

@@ -304,44 +306,6 @@ struct fotg210_dbg_port {
 	u32	address;
 };
 
-#ifdef CONFIG_EARLY_PRINTK_DBGP
-#include <linux/init.h>
-extern int __init early_dbgp_init(char *s);
-extern struct console early_dbgp_console;
-#endif /* CONFIG_EARLY_PRINTK_DBGP */
-
-struct usb_hcd;
-
-#ifdef CONFIG_XEN_DOM0
-extern int xen_dbgp_reset_prep(struct usb_hcd *);
-extern int xen_dbgp_external_startup(struct usb_hcd *);
-#else
-static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
-{
-	return 1; /* Shouldn't this be 0? */
-}
-
-static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
-{
-	return -1;
-}
-#endif
-
-#ifdef CONFIG_EARLY_PRINTK_DBGP
-/* Call backs from fotg210 host driver to fotg210 debug driver */
-extern int dbgp_external_startup(struct usb_hcd *);
-extern int dbgp_reset_prep(struct usb_hcd *hcd);
-#else
-static inline int dbgp_reset_prep(struct usb_hcd *hcd)
-{
-	return xen_dbgp_reset_prep(hcd);
-}
-static inline int dbgp_external_startup(struct usb_hcd *hcd)
-{
-	return xen_dbgp_external_startup(hcd);
-}
-#endif
-
 /*-------------------------------------------------------------------------*/
 
 #define	QTD_NEXT(fotg210, dma)	cpu_to_hc32(fotg210, (u32)dma)
Signature · 1 line
1.9.3
author
#9 lore raw

PATCH 9/10 fotg210: Use ehci_dbg_port struct

In reply to Chris Rorvick

The FUSBH200 debug port has a EHCI-compatible register layout so there is no need to define a custom struct.

Signed-off-by Chris Rorvick <[email protected]>
---
 drivers/usb/host/fotg210.h | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)
drivers/usb/host/fotg210.h +1 -12
index 975d9bb..3bad178 100644
@@ -86,7 +86,7 @@ struct fotg210_hcd {			/* one per controller */
 	/* glue to PCI and HCD framework */
 	struct fotg210_caps __iomem *caps;
 	struct fotg210_regs __iomem *regs;
-	struct fotg210_dbg_port __iomem *debug;
+	struct ehci_dbg_port __iomem *debug;
 
 	__u32			hcs_params;	/* cached register copy */
 	spinlock_t		lock;

@@ -295,17 +295,6 @@ struct fotg210_regs {
 #define GMIR_MDEV_INT	(1 << 0)
 };
 
-/* Appendix C, Debug port ... intended for use with special "debug devices"
- * that can help if there's no serial console.  (nonstandard enumeration.)
- */
-struct fotg210_dbg_port {
-	u32	control;
-	u32	pids;
-	u32	data03;
-	u32	data47;
-	u32	address;
-};
-
 /*-------------------------------------------------------------------------*/
 
 #define	QTD_NEXT(fotg210, dma)	cpu_to_hc32(fotg210, (u32)dma)
Signature · 1 line
1.9.3
author
#10 lore raw

PATCH 10/10 usb: Remove __init from early_dbgp_init() prototype

In reply to Chris Rorvick

Specifying these attributes in both the prototype and the function definition is unnecessary and could cause confusion or bugs if they are inconsistent. As such, __init should only be specified at the function definition.

Keith Owens suggested this as a janitorial task on LKML several years ago:

https://lkml.org/lkml/2006/1/14/305

Signed-off-by Chris Rorvick <[email protected]>
---
 include/linux/usb/ehci-dbgp.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
include/linux/usb/ehci-dbgp.h +1 -2
index 796c1cd..7344d9e 100644
@@ -43,8 +43,7 @@ struct ehci_dbg_port {
 };
 
 #ifdef CONFIG_EARLY_PRINTK_DBGP
-#include <linux/init.h>
-extern int __init early_dbgp_init(char *s);
+extern int early_dbgp_init(char *s);
 extern struct console early_dbgp_console;
 #endif /* CONFIG_EARLY_PRINTK_DBGP */
Signature · 1 line
1.9.3