Sergei Shtylyov wrote:
Hello.
Daniel Mack wrote:
This is a proposal for a slim framework to abstract USB transceivers
from their actual hardware access methods. The idea is that processor
platforms provide accesor functions, the tranceiver drivers provide
higher level functions and the board support code connects them
together.
The design is kept as simple and slim as possible.
Yet it seems to be a duplicate design...
Signed-off-by: Daniel Mack <daniel@xxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxx>
Cc: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
Cc: linux-usb@xxxxxxxxxxxxxxx
---
include/linux/usb/xcvr.h | 71 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
create mode 100644 include/linux/usb/xcvr.h
diff --git a/include/linux/usb/xcvr.h b/include/linux/usb/xcvr.h
new file mode 100644
index 0000000..5d53e32
--- /dev/null
+++ b/include/linux/usb/xcvr.h
@@ -0,0 +1,71 @@
+#ifndef __LINUX_USB_XCVR_H
+#define __LINUX_USB_XCVR_H
+
+struct usb_xcvr;
+
+struct usb_xcvr_access_ops {
+ int (*read)(struct usb_xcvr *xcvr, u32 reg);
+ int (*write)(struct usb_xcvr *xcvr, u32 val, u32 reg);
+};
+
+struct usb_xcvr_driver {
+ int (*init)(struct usb_xcvr *xcvr);
+ void (*shutdown)(struct usb_xcvr *xcvr);
+ int (*set_vbus)(struct usb_xcvr *xcvr, bool en);
+};
+
+struct usb_xcvr {
+ struct usb_xcvr_access_ops *access;
+ struct usb_xcvr_driver *driver;
+ void __iomem *access_priv;
+
+ /* only set this if you don't want the lowlevel driver to
+ * handle this */
+ int (*set_vbus)(struct usb_xcvr *xcvr, bool en);
+};
+
+static inline int usb_xcvr_init(struct usb_xcvr *xcvr)
+{
+ if (xcvr->driver && xcvr->driver->init)
+ return xcvr->driver->init(xcvr);
+
+ return -EINVAL;
+}
+
+static inline void usb_xcvr_shutdown(struct usb_xcvr *xcvr)
+{
+ if (xcvr->driver && xcvr->driver->shutdown)
+ xcvr->driver->shutdown(xcvr);
+}
+
+static inline int usb_xcvr_set_vbus(struct usb_xcvr *xcvr, bool en)
+{
+ if (xcvr->set_vbus)
+ return xcvr->set_vbus(xcvr, en);
+
+ if (xcvr->driver && xcvr->driver->set_vbus)
+ return xcvr->driver->set_vbus(xcvr, en);
+
+ return -EINVAL;
+}
+
+/* lowlowel access helpers */
+
+static inline int usb_xcvr_read(struct usb_xcvr *xcvr, u32 reg)
+{
+ if (xcvr->access->read)
+ return xcvr->access->read(xcvr, reg);
+
+ return -EINVAL;
+}
+
+static inline int usb_xcvr_write(struct usb_xcvr *xcvr, u32 val, u32 reg)
+{
+ if (xcvr->access->write)
+ return xcvr->access->write(xcvr, val, reg);
+
+ return -EINVAL;
+}
+
+#endif /* __LINUX_USB_XCVR_H */
+
Looks like you're kind of duplicating what's already in
<linux/usb/otg.h>. More so because your transceiver is also OTG one, so it
seems it should be using 'struct otg_transceiver' already defined there.
I have looked at the otg transceiver current implementation. I have the
feeling (correct me if I'm wrong) that they do not achieve the same
purpose. It is more dedicated to operations like changing the state of
the transceiver (set to host/gadget mode).
In Daniel's implemementation, the functions are more related to the
power and platform specific parts (initialisation, shutdown, set_vbus).
The otg_transceiver struct could be used, but shouldn't it be extended ?
If not how would I for instance implement init and set_vbus ?
Val
--
Valentin Longchamp, PhD Student, EPFL-STI-LSRO1
valentin.longchamp@xxxxxxx, Phone: +41216937827
http://people.epfl.ch/valentin.longchamp
MEA3485, Station 9, CH-1015 Lausanne
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
