One more attempt at getting some feedback.
Original:
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.
I have done only minimal testing on this, and I don't use either of
these controller so my ability to test the changes is limited. But I
thought I'd push it out for comment to see if there was interest.
The only actual change should be when CONFIG_EARLY_PRINTK_DBGP is
disabled and CONFIG_XEN_DOM0 is enabled. Currently each of these does
not notify Xen of reset events under this configuration. Since these
events are propagated when CONFIG_EARLY_PRINTK_DBGP and CONFIG_XEN_DOM0
are both enabled, though, it seems that is not a problem (and maybe not
sending them in the former case is a bug?) Regardless, the motivation
for this change is for consistancy as a step towrads consolidation. As
I said above, I am not able to actually test these changes on either
controller.
First time submission, so I look forward to any feedback. If this is of
any interest I will work on testing the various configations and boot
parameters.
Regards,
Chris Rorvick
Chris Rorvick (10):
usb: Create separate header for ehci-dbgp
fusbh200: Make Xen notificaiton consistent with EHCI
fusbh200: Remove superfluous macro definitions
fusbh200: Remove duplicate ehci-dbgp declarations
fusbh200: Use ehci_dbg_port struct
fotg210: Make Xen notificaiton 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
--
1.9.3
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
diff --git a/include/linux/usb/ehci-dbgp.h b/include/linux/usb/ehci-dbgp.h
new file mode 100644
index 0000000..796c1cd
--- /dev/null
+++ b/include/linux/usb/ehci-dbgp.h
@@ -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 */
diff --git a/include/linux/usb/ehci_def.h b/include/linux/usb/ehci_def.h
index daec99a..966889a 100644
--- a/include/linux/usb/ehci_def.h
+++ b/include/linux/usb/ehci_def.h
@@ -19,6 +19,8 @@
#ifndef __LINUX_USB_EHCI_DEF_H
#define __LINUX_USB_EHCI_DEF_H
+#include <linux/usb/ehci-dbgp.h>
+
/* EHCI register interface, corresponds to EHCI Revision 0.95 specification */
/* Section 2.2 Host Controller Capability Registers */
@@ -190,67 +192,4 @@ struct ehci_regs {
#define USBMODE_EX_HC (3<<0) /* host controller mode */
};
-/* 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 *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_DEF_H */
--
1.9.3
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(-)
diff --git a/drivers/usb/host/fotg210.h b/drivers/usb/host/fotg210.h
index c2e5134..975d9bb 100644
--- a/drivers/usb/host/fotg210.h
+++ b/drivers/usb/host/fotg210.h
@@ -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)
--
1.9.3
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(-)
diff --git a/drivers/usb/host/fusbh200.h b/drivers/usb/host/fusbh200.h
index da7152b..47aecfc 100644
--- a/drivers/usb/host/fusbh200.h
+++ b/drivers/usb/host/fusbh200.h
@@ -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)
--
1.9.3
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(-)
diff --git a/include/linux/usb/ehci-dbgp.h b/include/linux/usb/ehci-dbgp.h
index 796c1cd..7344d9e 100644
--- a/include/linux/usb/ehci-dbgp.h
+++ b/include/linux/usb/ehci-dbgp.h
@@ -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 */
--
1.9.3
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(-)
diff --git a/drivers/usb/host/fotg210.h b/drivers/usb/host/fotg210.h
index 975d9bb..3bad178 100644
--- a/drivers/usb/host/fotg210.h
+++ b/drivers/usb/host/fotg210.h
@@ -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)
--
1.9.3
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(-)
diff --git a/drivers/usb/host/fotg210.h b/drivers/usb/host/fotg210.h
index 98c9670..c2e5134 100644
--- a/drivers/usb/host/fotg210.h
+++ b/drivers/usb/host/fotg210.h
@@ -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
--
1.9.3
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(+)
diff --git a/drivers/usb/host/fotg210.h b/drivers/usb/host/fotg210.h
index ac6cd1b..98c9670 100644
--- a/drivers/usb/host/fotg210.h
+++ b/drivers/usb/host/fotg210.h
@@ -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 */
--
1.9.3
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(-)
diff --git a/drivers/usb/host/fusbh200.h b/drivers/usb/host/fusbh200.h
index 47aecfc..d6e5b3d 100644
--- a/drivers/usb/host/fusbh200.h
+++ b/drivers/usb/host/fusbh200.h
@@ -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)
--
1.9.3
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(-)
diff --git a/drivers/usb/host/fusbh200.h b/drivers/usb/host/fusbh200.h
index 6e7b8c1..da7152b 100644
--- a/drivers/usb/host/fusbh200.h
+++ b/drivers/usb/host/fusbh200.h
@@ -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
--
1.9.3
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(+)
diff --git a/drivers/usb/host/fusbh200.h b/drivers/usb/host/fusbh200.h
index 6b719e0..6e7b8c1 100644
--- a/drivers/usb/host/fusbh200.h
+++ b/drivers/usb/host/fusbh200.h
@@ -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 */
--
1.9.3
On Sun, Nov 02, 2014 at 09:07:47PM -0600, Chris Rorvick wrote:
> One more attempt at getting some feedback.
>
> Original:
>
> 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.
>
> I have done only minimal testing on this, and I don't use either of
> these controller so my ability to test the changes is limited. But I
> thought I'd push it out for comment to see if there was interest.
>
> The only actual change should be when CONFIG_EARLY_PRINTK_DBGP is
> disabled and CONFIG_XEN_DOM0 is enabled. Currently each of these does
> not notify Xen of reset events under this configuration. Since these
> events are propagated when CONFIG_EARLY_PRINTK_DBGP and CONFIG_XEN_DOM0
> are both enabled, though, it seems that is not a problem (and maybe not
> sending them in the former case is a bug?) Regardless, the motivation
> for this change is for consistancy as a step towrads consolidation. As
> I said above, I am not able to actually test these changes on either
> controller.
>
> First time submission, so I look forward to any feedback. If this is of
> any interest I will work on testing the various configations and boot
> parameters.
>
> Regards,
>
> Chris Rorvick
>
> Chris Rorvick (10):
> usb: Create separate header for ehci-dbgp
> fusbh200: Make Xen notificaiton consistent with EHCI
> fusbh200: Remove superfluous macro definitions
> fusbh200: Remove duplicate ehci-dbgp declarations
> fusbh200: Use ehci_dbg_port struct
> fotg210: Make Xen notificaiton 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
It deletes lines of code overall, which looks good to me. Feel free to
resend without the "RFC" and I will queue it up.
thanks,
greg k-h
2014-11-03 4:07 GMT+01:00 Chris Rorvick:
> fusbh200: Make Xen notificaiton consistent with EHCI
> fotg210: Make Xen notificaiton consistent with EHCI
you may want to fix the spelling: s/notificaiton/notification/
--
Daniele Forsi
On Mon, Nov 3, 2014 at 5:09 PM, Greg Kroah-Hartman
<[email protected]> wrote:
> It deletes lines of code overall, which looks good to me. Feel free to
> resend without the "RFC" and I will queue it up.
>
> thanks,
>
> greg k-h
Done.
I went through the patches by hand to double check spelling based on
Daniele catching the s/notificaiton/notification/ typo but somehow
failed to make that fix to patch 02. I can resend if you'd like, sorry
for the silly error.
Chris