* [PATCH 0/3] phy: ulpi bus
@ 2015-01-20 9:18 Heikki Krogerus
2015-01-20 9:18 ` [PATCH 1/3] phy: add bus for USB ULPI PHYs Heikki Krogerus
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-20 9:18 UTC (permalink / raw)
To: Kishon Vijay Abraham I; +Cc: Baolu Lu, linux-usb, linux-kernel
Heikki Krogerus (3):
phy: add bus for USB ULPI PHYs
usb: dwc3: add ULPI interface support
phy: ulpi: add driver for TI TUSB1210
MAINTAINERS | 7 ++
drivers/phy/Kconfig | 2 +
drivers/phy/Makefile | 1 +
drivers/phy/ulpi/Kconfig | 20 +++
drivers/phy/ulpi/Makefile | 4 +
drivers/phy/ulpi/tusb1210.c | 131 ++++++++++++++++++++
drivers/phy/ulpi/ulpi.c | 246 +++++++++++++++++++++++++++++++++++++
drivers/phy/ulpi/ulpi_phy.h | 31 +++++
drivers/usb/dwc3/Kconfig | 7 ++
drivers/usb/dwc3/Makefile | 4 +
drivers/usb/dwc3/core.c | 9 +-
drivers/usb/dwc3/core.h | 22 ++++
drivers/usb/dwc3/ulpi.c | 102 +++++++++++++++
include/linux/phy/ulpi/driver.h | 58 +++++++++
include/linux/phy/ulpi/interface.h | 23 ++++
include/linux/phy/ulpi/regs.h | 130 ++++++++++++++++++++
include/linux/usb/ulpi.h | 135 +-------------------
17 files changed, 799 insertions(+), 133 deletions(-)
create mode 100644 drivers/phy/ulpi/Kconfig
create mode 100644 drivers/phy/ulpi/Makefile
create mode 100644 drivers/phy/ulpi/tusb1210.c
create mode 100644 drivers/phy/ulpi/ulpi.c
create mode 100644 drivers/phy/ulpi/ulpi_phy.h
create mode 100644 drivers/usb/dwc3/ulpi.c
create mode 100644 include/linux/phy/ulpi/driver.h
create mode 100644 include/linux/phy/ulpi/interface.h
create mode 100644 include/linux/phy/ulpi/regs.h
--
2.1.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] phy: add bus for USB ULPI PHYs
2015-01-20 9:18 [PATCH 0/3] phy: ulpi bus Heikki Krogerus
@ 2015-01-20 9:18 ` Heikki Krogerus
2015-01-20 9:18 ` [PATCH 2/3] usb: dwc3: add ULPI interface support Heikki Krogerus
2015-01-20 9:18 ` [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210 Heikki Krogerus
2 siblings, 0 replies; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-20 9:18 UTC (permalink / raw)
To: Kishon Vijay Abraham I; +Cc: Baolu Lu, linux-usb, linux-kernel
UTMI+ Low Pin Interface (ULPI) is a commonly used PHY
interface for USB 2.0. It describe a standard set of
registers which the vendors can extend for their specific
needs.
ULPI registers are accessed from the controller. The purpose
of the bus is to provide nice way for the controller drivers
to share that access with the actual PHY drivers.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
MAINTAINERS | 7 ++
drivers/phy/Kconfig | 2 +
drivers/phy/Makefile | 1 +
drivers/phy/ulpi/Kconfig | 9 ++
drivers/phy/ulpi/Makefile | 2 +
drivers/phy/ulpi/ulpi.c | 246 +++++++++++++++++++++++++++++++++++++
drivers/phy/ulpi/ulpi_phy.h | 31 +++++
include/linux/phy/ulpi/driver.h | 58 +++++++++
include/linux/phy/ulpi/interface.h | 23 ++++
include/linux/phy/ulpi/regs.h | 130 ++++++++++++++++++++
include/linux/usb/ulpi.h | 135 +-------------------
11 files changed, 512 insertions(+), 132 deletions(-)
create mode 100644 drivers/phy/ulpi/Kconfig
create mode 100644 drivers/phy/ulpi/Makefile
create mode 100644 drivers/phy/ulpi/ulpi.c
create mode 100644 drivers/phy/ulpi/ulpi_phy.h
create mode 100644 include/linux/phy/ulpi/driver.h
create mode 100644 include/linux/phy/ulpi/interface.h
create mode 100644 include/linux/phy/ulpi/regs.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 4ef032a..fce81a4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4233,6 +4233,13 @@ S: Supported
F: drivers/phy/
F: include/linux/phy/
+ULPI PHY BUS
+M: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+L: linux-usb@vger.kernel.org
+S: Supported
+F: drivers/phy/ulpi/
+F: include/linux/phy/ulpi/
+
GENERIC UIO DRIVER FOR PCI DEVICES
M: "Michael S. Tsirkin" <mst@redhat.com>
L: kvm@vger.kernel.org
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index ccad880..9bfb556 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -277,4 +277,6 @@ config PHY_STIH41X_USB
Enable this to support the USB transceiver that is part of
STMicroelectronics STiH41x SoC series.
+source "drivers/phy/ulpi/Kconfig"
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index aa74f96..8914f3d 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o
obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o
+obj-$(CONFIG_ULPI_PHY) += ulpi/
diff --git a/drivers/phy/ulpi/Kconfig b/drivers/phy/ulpi/Kconfig
new file mode 100644
index 0000000..8007df2
--- /dev/null
+++ b/drivers/phy/ulpi/Kconfig
@@ -0,0 +1,9 @@
+
+config ULPI_PHY
+ bool "USB ULPI PHY interface support"
+ depends on USB || USB_GADGET
+ select GENERIC_PHY
+ help
+ Say yes if you have ULPI PHY attached to your USB controller.
+
+ If unsure, say N.
diff --git a/drivers/phy/ulpi/Makefile b/drivers/phy/ulpi/Makefile
new file mode 100644
index 0000000..59e61cb
--- /dev/null
+++ b/drivers/phy/ulpi/Makefile
@@ -0,0 +1,2 @@
+ulpiphy-y := ulpi.o
+obj-$(CONFIG_ULPI_PHY) += ulpiphy.o
diff --git a/drivers/phy/ulpi/ulpi.c b/drivers/phy/ulpi/ulpi.c
new file mode 100644
index 0000000..4a05dd5
--- /dev/null
+++ b/drivers/phy/ulpi/ulpi.c
@@ -0,0 +1,246 @@
+/**
+ * ulpi.c - USB ULPI PHY bus
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/phy/ulpi/interface.h>
+#include <linux/phy/ulpi/driver.h>
+#include <linux/phy/ulpi/regs.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+/* -------------------------------------------------------------------------- */
+
+int ulpi_read(struct ulpi *ulpi, u8 addr)
+{
+ return ulpi->ops->read(ulpi->ops, addr);
+}
+EXPORT_SYMBOL_GPL(ulpi_read);
+
+int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
+{
+ return ulpi->ops->write(ulpi->ops, addr, val);
+}
+EXPORT_SYMBOL_GPL(ulpi_write);
+
+/* -------------------------------------------------------------------------- */
+
+static int ulpi_match(struct device *dev, struct device_driver *driver)
+{
+ struct ulpi_driver *drv = to_ulpi_driver(driver);
+ struct ulpi *ulpi = to_ulpi_dev(dev);
+ const struct ulpi_device_id *id;
+
+ for (id = drv->id_table; id->vendor; id++)
+ if (id->vendor == ulpi->id.vendor &&
+ id->product == ulpi->id.product)
+ return 1;
+
+ return 0;
+}
+
+static int ulpi_probe(struct device *dev)
+{
+ struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
+
+ return drv->probe(to_ulpi_dev(dev));
+}
+
+static int ulpi_remove(struct device *dev)
+{
+ struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
+
+ if (drv->remove)
+ drv->remove(to_ulpi_dev(dev));
+
+ return 0;
+}
+
+struct bus_type ulpi_bus = {
+ .name = "ulpi",
+ .match = ulpi_match,
+ .probe = ulpi_probe,
+ .remove = ulpi_remove,
+};
+
+/**
+ * ulpi_register_driver - register a driver with the ULPI bus
+ * @drv: driver being registered
+ *
+ * Registers a driver with the ULPI bus.
+ */
+int ulpi_register_driver(struct ulpi_driver *drv)
+{
+ if (!drv->probe)
+ return -EINVAL;
+
+ drv->driver.bus = &ulpi_bus;
+
+ return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(ulpi_register_driver);
+
+/**
+ * ulpi_register_driver - unregister a driver with the ULPI bus
+ * @drv: driver to unregister
+ *
+ * Unregisters a driver with the ULPI bus.
+ */
+void ulpi_unregister_driver(struct ulpi_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
+
+/* -------------------------------------------------------------------------- */
+
+static void ulpi_dev_release(struct device *dev)
+{
+ kfree(to_ulpi_dev(dev));
+}
+
+static ssize_t
+vendor_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct ulpi *ulpi = to_ulpi_dev(dev);
+
+ return sprintf(buf, "%04x\n", ulpi->id.vendor);
+}
+static DEVICE_ATTR_RO(vendor);
+
+static ssize_t
+product_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct ulpi *ulpi = to_ulpi_dev(dev);
+
+ return sprintf(buf, "%04x\n", ulpi->id.product);
+}
+static DEVICE_ATTR_RO(product);
+
+static struct attribute *ulpi_dev_attrs[] = {
+ &dev_attr_vendor.attr,
+ &dev_attr_product.attr,
+ NULL
+};
+
+static struct attribute_group ulpi_dev_attr_group = {
+ .attrs = ulpi_dev_attrs,
+};
+
+static const struct attribute_group *ulpi_dev_attr_groups[] = {
+ &ulpi_dev_attr_group,
+ NULL
+};
+
+struct device_type ulpi_device_type = {
+ .name = "ulpi_device",
+ .groups = ulpi_dev_attr_groups,
+ .release = ulpi_dev_release,
+};
+
+/* -------------------------------------------------------------------------- */
+
+static int ulpi_register(struct device *dev, struct ulpi *ulpi)
+{
+ int ret;
+
+ /* Test the interface */
+ ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
+ if (ret < 0)
+ return ret;
+
+ ret = ulpi_read(ulpi, ULPI_SCRATCH);
+ if (ret < 0)
+ return ret;
+
+ if (ret != 0xaa)
+ return -ENODEV;
+
+ ulpi->id.vendor = ulpi_read(ulpi, ULPI_VENDOR_ID_LOW);
+ ulpi->id.vendor |= ulpi_read(ulpi, ULPI_VENDOR_ID_HIGH) << 8;
+
+ ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
+ ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
+
+ ulpi->dev.parent = dev;
+ ulpi->dev.bus = &ulpi_bus;
+ ulpi->dev.type = &ulpi_device_type;
+ dev_set_name(&ulpi->dev, "%s.ulpi", dev_name(dev));
+
+ ret = device_register(&ulpi->dev);
+ if (ret)
+ return ret;
+
+ dev_dbg(&ulpi->dev, "registered ULPI PHY: vendor %04x, product %04x\n",
+ ulpi->id.vendor, ulpi->id.product);
+
+ return 0;
+}
+
+/**
+ * ulpi_register_interface - instantiate new ULPI device
+ * @dev: USB controller's device interface
+ * @ops: ULPI register access
+ *
+ * Allocates and registers a ULPI device and an interface for it. Called from
+ * the USB controller that provides the ULPI interface.
+ */
+struct ulpi *ulpi_register_interface(struct device *dev, struct ulpi_ops *ops)
+{
+ struct ulpi *ulpi;
+ int ret;
+
+ ulpi = kzalloc(sizeof(*ulpi), GFP_KERNEL);
+ if (!ulpi)
+ return ERR_PTR(-ENOMEM);
+
+ ulpi->ops = ops;
+ ops->dev = dev;
+
+ ret = ulpi_register(dev, ulpi);
+ if (ret) {
+ kfree(ulpi);
+ return ERR_PTR(ret);
+ }
+
+ return ulpi;
+}
+EXPORT_SYMBOL_GPL(ulpi_register_interface);
+
+/**
+ * ulpi_unregister_interface - unregister ULPI interface
+ * @intrf: struct ulpi_interface
+ *
+ * Unregisters a ULPI device and it's interface that was created with
+ * ulpi_create_interface().
+ */
+void ulpi_unregister_interface(struct ulpi *ulpi)
+{
+ device_unregister(&ulpi->dev);
+}
+EXPORT_SYMBOL_GPL(ulpi_unregister_interface);
+
+/* -------------------------------------------------------------------------- */
+
+static int __init ulpi_init(void)
+{
+ return bus_register(&ulpi_bus);
+}
+module_init(ulpi_init);
+
+static void __exit ulpi_exit(void)
+{
+ bus_unregister(&ulpi_bus);
+}
+module_exit(ulpi_exit);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("USB ULPI PHY bus");
diff --git a/drivers/phy/ulpi/ulpi_phy.h b/drivers/phy/ulpi/ulpi_phy.h
new file mode 100644
index 0000000..ac49fb6
--- /dev/null
+++ b/drivers/phy/ulpi/ulpi_phy.h
@@ -0,0 +1,31 @@
+#include <linux/phy/phy.h>
+
+/**
+ * Helper that registers PHY for a ULPI device and adds a lookup for binding it
+ * and it's controller, which is always the parent.
+ */
+static inline struct phy
+*ulpi_phy_create(struct ulpi *ulpi, struct phy_ops *ops)
+{
+ struct phy *phy;
+ int ret;
+
+ phy = phy_create(&ulpi->dev, NULL, ops);
+ if (IS_ERR(phy))
+ return phy;
+
+ ret = phy_create_lookup(phy, "usb2-phy", dev_name(ulpi->dev.parent));
+ if (ret) {
+ phy_destroy(phy);
+ return ERR_PTR(ret);
+ }
+
+ return phy;
+}
+
+/* Remove a PHY that was created with ulpi_phy_create() and it's lookup. */
+static inline void ulpi_phy_destroy(struct ulpi *ulpi, struct phy *phy)
+{
+ phy_remove_lookup(phy, "usb2-phy", dev_name(ulpi->dev.parent));
+ phy_destroy(phy);
+}
diff --git a/include/linux/phy/ulpi/driver.h b/include/linux/phy/ulpi/driver.h
new file mode 100644
index 0000000..639a462
--- /dev/null
+++ b/include/linux/phy/ulpi/driver.h
@@ -0,0 +1,58 @@
+#ifndef __DRIVERS_PHY_ULPI_H
+#define __DRIVERS_PHY_ULPI_H
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+struct ulpi_ops;
+
+struct ulpi_device_id {
+ __u16 vendor;
+ __u16 product;
+};
+
+/**
+ * struct ulpi - describes ULPI PHY device
+ * @id: vendor and product ids for ULPI device
+ * @ops: I/O access
+ * @dev: device interface
+ * @priv: for drivers private use
+ */
+struct ulpi {
+ struct ulpi_device_id id;
+ struct ulpi_ops *ops;
+ struct device dev;
+ void *priv;
+};
+
+#define to_ulpi_dev(d) container_of(d, struct ulpi, dev)
+
+/**
+ * struct ulpi_driver - describes a ULPI PHY driver
+ * @id_table: array of device identifiers supported by this driver
+ * @probe: binds this driver to ULPI device
+ * @remove: unbinds this driver from ULPI device
+ * @driver: the name and owner members must be initialized by the drivers
+ */
+struct ulpi_driver {
+ struct ulpi_device_id *id_table;
+ int (*probe)(struct ulpi *ulpi);
+ void (*remove)(struct ulpi *ulpi);
+ struct device_driver driver;
+};
+
+#define to_ulpi_driver(d) container_of(d, struct ulpi_driver, driver)
+
+int ulpi_register_driver(struct ulpi_driver *drv);
+void ulpi_unregister_driver(struct ulpi_driver *drv);
+
+#define module_ulpi_driver(__ulpi_driver) \
+ module_driver(__ulpi_driver, ulpi_register_driver, \
+ ulpi_unregister_driver)
+
+int ulpi_read(struct ulpi *ulpi, u8 addr);
+int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val);
+
+extern struct device_type ulpi_device_type;
+
+#endif /* __DRIVERS_PHY_ULPI_H */
diff --git a/include/linux/phy/ulpi/interface.h b/include/linux/phy/ulpi/interface.h
new file mode 100644
index 0000000..51569fe
--- /dev/null
+++ b/include/linux/phy/ulpi/interface.h
@@ -0,0 +1,23 @@
+#ifndef __DRIVERS_PHY_ULPI_INTERFACE_H
+#define __DRIVERS_PHY_ULPI_INTERFACE_H
+
+#include <linux/types.h>
+
+struct ulpi;
+
+/**
+ * struct ulpi_ops - ULPI register access
+ * @dev: the interface provider
+ * @read: read opearation for ULPI register access
+ * @write: write opearation for ULPI register access
+ */
+struct ulpi_ops {
+ struct device *dev;
+ int (*read)(struct ulpi_ops *ops, u8 addr);
+ int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
+};
+
+struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
+void ulpi_unregister_interface(struct ulpi *);
+
+#endif /* __DRIVERS_PHY_ULPI_INTERFACE_H */
diff --git a/include/linux/phy/ulpi/regs.h b/include/linux/phy/ulpi/regs.h
new file mode 100644
index 0000000..8407df7
--- /dev/null
+++ b/include/linux/phy/ulpi/regs.h
@@ -0,0 +1,130 @@
+#ifndef __DRIVERS_PHY_ULPI_REGS_H
+#define __DRIVERS_PHY_ULPI_REGS_H
+
+/*
+ * Macros for Set and Clear
+ * See ULPI 1.1 specification to find the registers with Set and Clear offsets
+ */
+#define ULPI_SET(a) (a + 1)
+#define ULPI_CLR(a) (a + 2)
+
+/*
+ * Register Map
+ */
+#define ULPI_VENDOR_ID_LOW 0x00
+#define ULPI_VENDOR_ID_HIGH 0x01
+#define ULPI_PRODUCT_ID_LOW 0x02
+#define ULPI_PRODUCT_ID_HIGH 0x03
+#define ULPI_FUNC_CTRL 0x04
+#define ULPI_IFC_CTRL 0x07
+#define ULPI_OTG_CTRL 0x0a
+#define ULPI_USB_INT_EN_RISE 0x0d
+#define ULPI_USB_INT_EN_FALL 0x10
+#define ULPI_USB_INT_STS 0x13
+#define ULPI_USB_INT_LATCH 0x14
+#define ULPI_DEBUG 0x15
+#define ULPI_SCRATCH 0x16
+/* Optional Carkit Registers */
+#define ULPI_CARKIT_CTRL 0x19
+#define ULPI_CARKIT_INT_DELAY 0x1c
+#define ULPI_CARKIT_INT_EN 0x1d
+#define ULPI_CARKIT_INT_STS 0x20
+#define ULPI_CARKIT_INT_LATCH 0x21
+#define ULPI_CARKIT_PLS_CTRL 0x22
+/* Other Optional Registers */
+#define ULPI_TX_POS_WIDTH 0x25
+#define ULPI_TX_NEG_WIDTH 0x26
+#define ULPI_POLARITY_RECOVERY 0x27
+/* Access Extended Register Set */
+#define ULPI_ACCESS_EXTENDED 0x2f
+/* Vendor Specific */
+#define ULPI_VENDOR_SPECIFIC 0x30
+/* Extended Registers */
+#define ULPI_EXT_VENDOR_SPECIFIC 0x80
+
+/*
+ * Register Bits
+ */
+
+/* Function Control */
+#define ULPI_FUNC_CTRL_XCVRSEL BIT(0)
+#define ULPI_FUNC_CTRL_XCVRSEL_MASK 0x3
+#define ULPI_FUNC_CTRL_HIGH_SPEED 0x0
+#define ULPI_FUNC_CTRL_FULL_SPEED 0x1
+#define ULPI_FUNC_CTRL_LOW_SPEED 0x2
+#define ULPI_FUNC_CTRL_FS4LS 0x3
+#define ULPI_FUNC_CTRL_TERMSELECT BIT(2)
+#define ULPI_FUNC_CTRL_OPMODE BIT(3)
+#define ULPI_FUNC_CTRL_OPMODE_MASK (0x3 << 3)
+#define ULPI_FUNC_CTRL_OPMODE_NORMAL (0x0 << 3)
+#define ULPI_FUNC_CTRL_OPMODE_NONDRIVING (0x1 << 3)
+#define ULPI_FUNC_CTRL_OPMODE_DISABLE_NRZI (0x2 << 3)
+#define ULPI_FUNC_CTRL_OPMODE_NOSYNC_NOEOP (0x3 << 3)
+#define ULPI_FUNC_CTRL_RESET BIT(5)
+#define ULPI_FUNC_CTRL_SUSPENDM BIT(6)
+
+/* Interface Control */
+#define ULPI_IFC_CTRL_6_PIN_SERIAL_MODE BIT(0)
+#define ULPI_IFC_CTRL_3_PIN_SERIAL_MODE BIT(1)
+#define ULPI_IFC_CTRL_CARKITMODE BIT(2)
+#define ULPI_IFC_CTRL_CLOCKSUSPENDM BIT(3)
+#define ULPI_IFC_CTRL_AUTORESUME BIT(4)
+#define ULPI_IFC_CTRL_EXTERNAL_VBUS BIT(5)
+#define ULPI_IFC_CTRL_PASSTHRU BIT(6)
+#define ULPI_IFC_CTRL_PROTECT_IFC_DISABLE BIT(7)
+
+/* OTG Control */
+#define ULPI_OTG_CTRL_ID_PULLUP BIT(0)
+#define ULPI_OTG_CTRL_DP_PULLDOWN BIT(1)
+#define ULPI_OTG_CTRL_DM_PULLDOWN BIT(2)
+#define ULPI_OTG_CTRL_DISCHRGVBUS BIT(3)
+#define ULPI_OTG_CTRL_CHRGVBUS BIT(4)
+#define ULPI_OTG_CTRL_DRVVBUS BIT(5)
+#define ULPI_OTG_CTRL_DRVVBUS_EXT BIT(6)
+#define ULPI_OTG_CTRL_EXTVBUSIND BIT(7)
+
+/* USB Interrupt Enable Rising,
+ * USB Interrupt Enable Falling,
+ * USB Interrupt Status and
+ * USB Interrupt Latch
+ */
+#define ULPI_INT_HOST_DISCONNECT BIT(0)
+#define ULPI_INT_VBUS_VALID BIT(1)
+#define ULPI_INT_SESS_VALID BIT(2)
+#define ULPI_INT_SESS_END BIT(3)
+#define ULPI_INT_IDGRD BIT(4)
+
+/* Debug */
+#define ULPI_DEBUG_LINESTATE0 BIT(0)
+#define ULPI_DEBUG_LINESTATE1 BIT(1)
+
+/* Carkit Control */
+#define ULPI_CARKIT_CTRL_CARKITPWR BIT(0)
+#define ULPI_CARKIT_CTRL_IDGNDDRV BIT(1)
+#define ULPI_CARKIT_CTRL_TXDEN BIT(2)
+#define ULPI_CARKIT_CTRL_RXDEN BIT(3)
+#define ULPI_CARKIT_CTRL_SPKLEFTEN BIT(4)
+#define ULPI_CARKIT_CTRL_SPKRIGHTEN BIT(5)
+#define ULPI_CARKIT_CTRL_MICEN BIT(6)
+
+/* Carkit Interrupt Enable */
+#define ULPI_CARKIT_INT_EN_IDFLOAT_RISE BIT(0)
+#define ULPI_CARKIT_INT_EN_IDFLOAT_FALL BIT(1)
+#define ULPI_CARKIT_INT_EN_CARINTDET BIT(2)
+#define ULPI_CARKIT_INT_EN_DP_RISE BIT(3)
+#define ULPI_CARKIT_INT_EN_DP_FALL BIT(4)
+
+/* Carkit Interrupt Status and
+ * Carkit Interrupt Latch
+ */
+#define ULPI_CARKIT_INT_IDFLOAT BIT(0)
+#define ULPI_CARKIT_INT_CARINTDET BIT(1)
+#define ULPI_CARKIT_INT_DP BIT(2)
+
+/* Carkit Pulse Control*/
+#define ULPI_CARKIT_PLS_CTRL_TXPLSEN BIT(0)
+#define ULPI_CARKIT_PLS_CTRL_RXPLSEN BIT(1)
+#define ULPI_CARKIT_PLS_CTRL_SPKRLEFT_BIASEN BIT(2)
+#define ULPI_CARKIT_PLS_CTRL_SPKRRIGHT_BIASEN BIT(3)
+
+#endif /* __DRIVERS_PHY_ULPI_REGS_H */
diff --git a/include/linux/usb/ulpi.h b/include/linux/usb/ulpi.h
index 5c295c2..f5518e4 100644
--- a/include/linux/usb/ulpi.h
+++ b/include/linux/usb/ulpi.h
@@ -12,6 +12,9 @@
#define __LINUX_USB_ULPI_H
#include <linux/usb/otg.h>
+
+#include "../phy/ulpi/regs.h"
+
/*-------------------------------------------------------------------------*/
/*
@@ -49,138 +52,6 @@
/*-------------------------------------------------------------------------*/
-/*
- * Macros for Set and Clear
- * See ULPI 1.1 specification to find the registers with Set and Clear offsets
- */
-#define ULPI_SET(a) (a + 1)
-#define ULPI_CLR(a) (a + 2)
-
-/*-------------------------------------------------------------------------*/
-
-/*
- * Register Map
- */
-#define ULPI_VENDOR_ID_LOW 0x00
-#define ULPI_VENDOR_ID_HIGH 0x01
-#define ULPI_PRODUCT_ID_LOW 0x02
-#define ULPI_PRODUCT_ID_HIGH 0x03
-#define ULPI_FUNC_CTRL 0x04
-#define ULPI_IFC_CTRL 0x07
-#define ULPI_OTG_CTRL 0x0a
-#define ULPI_USB_INT_EN_RISE 0x0d
-#define ULPI_USB_INT_EN_FALL 0x10
-#define ULPI_USB_INT_STS 0x13
-#define ULPI_USB_INT_LATCH 0x14
-#define ULPI_DEBUG 0x15
-#define ULPI_SCRATCH 0x16
-/* Optional Carkit Registers */
-#define ULPI_CARCIT_CTRL 0x19
-#define ULPI_CARCIT_INT_DELAY 0x1c
-#define ULPI_CARCIT_INT_EN 0x1d
-#define ULPI_CARCIT_INT_STS 0x20
-#define ULPI_CARCIT_INT_LATCH 0x21
-#define ULPI_CARCIT_PLS_CTRL 0x22
-/* Other Optional Registers */
-#define ULPI_TX_POS_WIDTH 0x25
-#define ULPI_TX_NEG_WIDTH 0x26
-#define ULPI_POLARITY_RECOVERY 0x27
-/* Access Extended Register Set */
-#define ULPI_ACCESS_EXTENDED 0x2f
-/* Vendor Specific */
-#define ULPI_VENDOR_SPECIFIC 0x30
-/* Extended Registers */
-#define ULPI_EXT_VENDOR_SPECIFIC 0x80
-
-/*-------------------------------------------------------------------------*/
-
-/*
- * Register Bits
- */
-
-/* Function Control */
-#define ULPI_FUNC_CTRL_XCVRSEL (1 << 0)
-#define ULPI_FUNC_CTRL_XCVRSEL_MASK (3 << 0)
-#define ULPI_FUNC_CTRL_HIGH_SPEED (0 << 0)
-#define ULPI_FUNC_CTRL_FULL_SPEED (1 << 0)
-#define ULPI_FUNC_CTRL_LOW_SPEED (2 << 0)
-#define ULPI_FUNC_CTRL_FS4LS (3 << 0)
-#define ULPI_FUNC_CTRL_TERMSELECT (1 << 2)
-#define ULPI_FUNC_CTRL_OPMODE (1 << 3)
-#define ULPI_FUNC_CTRL_OPMODE_MASK (3 << 3)
-#define ULPI_FUNC_CTRL_OPMODE_NORMAL (0 << 3)
-#define ULPI_FUNC_CTRL_OPMODE_NONDRIVING (1 << 3)
-#define ULPI_FUNC_CTRL_OPMODE_DISABLE_NRZI (2 << 3)
-#define ULPI_FUNC_CTRL_OPMODE_NOSYNC_NOEOP (3 << 3)
-#define ULPI_FUNC_CTRL_RESET (1 << 5)
-#define ULPI_FUNC_CTRL_SUSPENDM (1 << 6)
-
-/* Interface Control */
-#define ULPI_IFC_CTRL_6_PIN_SERIAL_MODE (1 << 0)
-#define ULPI_IFC_CTRL_3_PIN_SERIAL_MODE (1 << 1)
-#define ULPI_IFC_CTRL_CARKITMODE (1 << 2)
-#define ULPI_IFC_CTRL_CLOCKSUSPENDM (1 << 3)
-#define ULPI_IFC_CTRL_AUTORESUME (1 << 4)
-#define ULPI_IFC_CTRL_EXTERNAL_VBUS (1 << 5)
-#define ULPI_IFC_CTRL_PASSTHRU (1 << 6)
-#define ULPI_IFC_CTRL_PROTECT_IFC_DISABLE (1 << 7)
-
-/* OTG Control */
-#define ULPI_OTG_CTRL_ID_PULLUP (1 << 0)
-#define ULPI_OTG_CTRL_DP_PULLDOWN (1 << 1)
-#define ULPI_OTG_CTRL_DM_PULLDOWN (1 << 2)
-#define ULPI_OTG_CTRL_DISCHRGVBUS (1 << 3)
-#define ULPI_OTG_CTRL_CHRGVBUS (1 << 4)
-#define ULPI_OTG_CTRL_DRVVBUS (1 << 5)
-#define ULPI_OTG_CTRL_DRVVBUS_EXT (1 << 6)
-#define ULPI_OTG_CTRL_EXTVBUSIND (1 << 7)
-
-/* USB Interrupt Enable Rising,
- * USB Interrupt Enable Falling,
- * USB Interrupt Status and
- * USB Interrupt Latch
- */
-#define ULPI_INT_HOST_DISCONNECT (1 << 0)
-#define ULPI_INT_VBUS_VALID (1 << 1)
-#define ULPI_INT_SESS_VALID (1 << 2)
-#define ULPI_INT_SESS_END (1 << 3)
-#define ULPI_INT_IDGRD (1 << 4)
-
-/* Debug */
-#define ULPI_DEBUG_LINESTATE0 (1 << 0)
-#define ULPI_DEBUG_LINESTATE1 (1 << 1)
-
-/* Carkit Control */
-#define ULPI_CARKIT_CTRL_CARKITPWR (1 << 0)
-#define ULPI_CARKIT_CTRL_IDGNDDRV (1 << 1)
-#define ULPI_CARKIT_CTRL_TXDEN (1 << 2)
-#define ULPI_CARKIT_CTRL_RXDEN (1 << 3)
-#define ULPI_CARKIT_CTRL_SPKLEFTEN (1 << 4)
-#define ULPI_CARKIT_CTRL_SPKRIGHTEN (1 << 5)
-#define ULPI_CARKIT_CTRL_MICEN (1 << 6)
-
-/* Carkit Interrupt Enable */
-#define ULPI_CARKIT_INT_EN_IDFLOAT_RISE (1 << 0)
-#define ULPI_CARKIT_INT_EN_IDFLOAT_FALL (1 << 1)
-#define ULPI_CARKIT_INT_EN_CARINTDET (1 << 2)
-#define ULPI_CARKIT_INT_EN_DP_RISE (1 << 3)
-#define ULPI_CARKIT_INT_EN_DP_FALL (1 << 4)
-
-/* Carkit Interrupt Status and
- * Carkit Interrupt Latch
- */
-#define ULPI_CARKIT_INT_IDFLOAT (1 << 0)
-#define ULPI_CARKIT_INT_CARINTDET (1 << 1)
-#define ULPI_CARKIT_INT_DP (1 << 2)
-
-/* Carkit Pulse Control*/
-#define ULPI_CARKIT_PLS_CTRL_TXPLSEN (1 << 0)
-#define ULPI_CARKIT_PLS_CTRL_RXPLSEN (1 << 1)
-#define ULPI_CARKIT_PLS_CTRL_SPKRLEFT_BIASEN (1 << 2)
-#define ULPI_CARKIT_PLS_CTRL_SPKRRIGHT_BIASEN (1 << 3)
-
-/*-------------------------------------------------------------------------*/
-
#if IS_ENABLED(CONFIG_USB_ULPI)
struct usb_phy *otg_ulpi_create(struct usb_phy_io_ops *ops,
unsigned int flags);
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] usb: dwc3: add ULPI interface support
2015-01-20 9:18 [PATCH 0/3] phy: ulpi bus Heikki Krogerus
2015-01-20 9:18 ` [PATCH 1/3] phy: add bus for USB ULPI PHYs Heikki Krogerus
@ 2015-01-20 9:18 ` Heikki Krogerus
2015-01-20 15:23 ` Felipe Balbi
2015-01-20 9:18 ` [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210 Heikki Krogerus
2 siblings, 1 reply; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-20 9:18 UTC (permalink / raw)
To: Kishon Vijay Abraham I; +Cc: Baolu Lu, linux-usb, linux-kernel, Felipe Balbi
Registers ULPI interface with the ULPI bus if HSPHY type is
ULPI.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Felipe Balbi <balbi@ti.com>
---
drivers/usb/dwc3/Kconfig | 7 ++++
drivers/usb/dwc3/Makefile | 4 ++
drivers/usb/dwc3/core.c | 9 +++-
drivers/usb/dwc3/core.h | 22 ++++++++++
drivers/usb/dwc3/ulpi.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/dwc3/ulpi.c
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 58b5b2c..6d0c5e6 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -11,6 +11,13 @@ config USB_DWC3
if USB_DWC3
+config USB_DWC3_ULPI
+ bool "Provide ULPI PHY Interface"
+ depends on ULPI_PHY=y || ULPI_PHY=USB_DWC3
+ help
+ Select this if you have ULPI type PHY attached to your DWC3
+ controller.
+
choice
bool "DWC3 Mode Selection"
default USB_DWC3_DUAL_ROLE if (USB && USB_GADGET)
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index bb34fbc..2fc44e0 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -16,6 +16,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
dwc3-y += gadget.o ep0.o
endif
+ifneq ($(CONFIG_USB_DWC3_ULPI),)
+ dwc3-y += ulpi.o
+endif
+
ifneq ($(CONFIG_DEBUG_FS),)
dwc3-y += debugfs.o
endif
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 25ddc39..5219bc7 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -876,12 +876,17 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->hird_threshold = hird_threshold
| (dwc->is_utmi_l1_suspend << 4);
+ platform_set_drvdata(pdev, dwc);
+
+ ret = dwc3_ulpi_init(dwc);
+ if (ret)
+ return ret;
+
ret = dwc3_core_get_phy(dwc);
if (ret)
return ret;
spin_lock_init(&dwc->lock);
- platform_set_drvdata(pdev, dwc);
if (!dev->dma_mask) {
dev->dma_mask = dev->parent->dma_mask;
@@ -965,6 +970,7 @@ err1:
err0:
dwc3_free_event_buffers(dwc);
+ dwc3_ulpi_exit(dwc);
return ret;
}
@@ -984,6 +990,7 @@ static int dwc3_remove(struct platform_device *pdev)
phy_power_off(dwc->usb3_generic_phy);
dwc3_core_exit(dwc);
+ dwc3_ulpi_exit(dwc);
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0842aa8..f6881a6 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -32,6 +32,7 @@
#include <linux/usb/otg.h>
#include <linux/phy/phy.h>
+#include <linux/phy/ulpi/interface.h>
#define DWC3_MSG_MAX 500
@@ -174,6 +175,14 @@
#define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31)
#define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6)
+/* Global USB2 PHY Vendor Control Register */
+#define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
+#define DWC3_GUSB2PHYACC_BUSY (1 << 23)
+#define DWC3_GUSB2PHYACC_WRITE (1 << 22)
+#define DWC3_GUSB2PHYACC_ADDR(n) (n << 16)
+#define DWC3_GUSB2PHYACC_EXTEND_ADDR(n) (n << 8)
+#define DWC3_GUSB2PHYACC_DATA(n) (n & 0xff)
+
/* Global USB3 PIPE Control Register */
#define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31)
#define DWC3_GUSB3PIPECTL_U2SSINP3OK (1 << 29)
@@ -590,6 +599,7 @@ struct dwc3_hwparams {
#define DWC3_NUM_INT(n) (((n) & (0x3f << 15)) >> 15)
/* HWPARAMS3 */
+#define DWC3_ULPI_HSPHY (1 << 3)
#define DWC3_NUM_IN_EPS_MASK (0x1f << 18)
#define DWC3_NUM_EPS_MASK (0x3f << 12)
#define DWC3_NUM_EPS(p) (((p)->hwparams3 & \
@@ -739,6 +749,8 @@ struct dwc3 {
struct phy *usb2_generic_phy;
struct phy *usb3_generic_phy;
+ struct ulpi *ulpi;
+
void __iomem *regs;
size_t regs_size;
@@ -1035,4 +1047,14 @@ static inline int dwc3_gadget_resume(struct dwc3 *dwc)
}
#endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
+#if IS_ENABLED(CONFIG_USB_DWC3_ULPI)
+int dwc3_ulpi_init(struct dwc3 *dwc);
+void dwc3_ulpi_exit(struct dwc3 *dwc);
+#else
+static inline int dwc3_ulpi_init(struct dwc3 *dwc)
+{ return 0; }
+static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
+{ }
+#endif
+
#endif /* __DRIVERS_USB_DWC3_CORE_H */
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
new file mode 100644
index 0000000..ee3ebbe
--- /dev/null
+++ b/drivers/usb/dwc3/ulpi.c
@@ -0,0 +1,102 @@
+/**
+ * ulpi.c - DesignWare USB3 Controller's ULPI PHY interface
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/phy/ulpi/regs.h>
+
+#include "core.h"
+#include "io.h"
+
+#define DWC3_ULPI_ADDR(a) \
+ ((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
+ DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
+ DWC3_GUSB2PHYACC_EXTEND_ADDR(a) : DWC3_GUSB2PHYACC_ADDR(a))
+
+static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
+{
+ unsigned count = 1000;
+ u32 reg;
+
+ while (count--) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
+ if (!(reg & DWC3_GUSB2PHYACC_BUSY))
+ return 0;
+ }
+
+ return -ETIMEDOUT;
+}
+
+static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
+{
+ struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+ u32 reg;
+ int ret;
+
+ reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
+
+ ret = dwc3_ulpi_busyloop(dwc);
+ if (ret)
+ return ret;
+
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
+
+ return DWC3_GUSB2PHYACC_DATA(reg);
+}
+
+static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
+{
+ struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+ u32 reg;
+ int ret;
+
+ reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
+ reg |= DWC3_GUSB2PHYACC_WRITE | val;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
+
+ ret = dwc3_ulpi_busyloop(dwc);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static struct ulpi_ops dwc3_ulpi = {
+ .read = dwc3_ulpi_read,
+ .write = dwc3_ulpi_write,
+};
+
+int dwc3_ulpi_init(struct dwc3 *dwc)
+{
+ int ret;
+
+ /* First check if there is ULPI PHY */
+ ret = dwc3_readl(dwc->regs, DWC3_GHWPARAMS3);
+ if (!(ret & DWC3_ULPI_HSPHY))
+ return 0;
+
+ /* Register the interface */
+ dwc->ulpi = ulpi_register_interface(dwc->dev, &dwc3_ulpi);
+ if (IS_ERR(dwc->ulpi)) {
+ dev_err(dwc->dev, "failed to register ULPI interface");
+ return PTR_ERR(dwc->ulpi);
+ }
+
+ return 0;
+}
+
+void dwc3_ulpi_exit(struct dwc3 *dwc)
+{
+ if (dwc->ulpi) {
+ ulpi_unregister_interface(dwc->ulpi);
+ dwc->ulpi = NULL;
+ }
+}
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-20 9:18 [PATCH 0/3] phy: ulpi bus Heikki Krogerus
2015-01-20 9:18 ` [PATCH 1/3] phy: add bus for USB ULPI PHYs Heikki Krogerus
2015-01-20 9:18 ` [PATCH 2/3] usb: dwc3: add ULPI interface support Heikki Krogerus
@ 2015-01-20 9:18 ` Heikki Krogerus
2015-01-20 15:45 ` Felipe Balbi
2 siblings, 1 reply; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-20 9:18 UTC (permalink / raw)
To: Kishon Vijay Abraham I; +Cc: Baolu Lu, linux-usb, linux-kernel
TUSB1210 ULPI PHY has vendor specific register for eye
diagram tuning. On some platforms the system firmware has
set optimized value to it. In order to not loose the
optimized value, the driver stores it during probe and
restores it every time the PHY is powered back on.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/phy/ulpi/Kconfig | 11 ++++
drivers/phy/ulpi/Makefile | 2 +
drivers/phy/ulpi/tusb1210.c | 131 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 144 insertions(+)
create mode 100644 drivers/phy/ulpi/tusb1210.c
diff --git a/drivers/phy/ulpi/Kconfig b/drivers/phy/ulpi/Kconfig
index 8007df2..7cd6f82 100644
--- a/drivers/phy/ulpi/Kconfig
+++ b/drivers/phy/ulpi/Kconfig
@@ -7,3 +7,14 @@ config ULPI_PHY
Say yes if you have ULPI PHY attached to your USB controller.
If unsure, say N.
+
+if ULPI_PHY
+
+config ULPI_TUSB1210
+ tristate "TI TUSB1210 USB PHY module"
+ depends on POWER_SUPPLY
+ select USB_PHY
+ help
+ Support for TI TUSB1210 USB ULPI PHY.
+
+endif
diff --git a/drivers/phy/ulpi/Makefile b/drivers/phy/ulpi/Makefile
index 59e61cb..7ee6679 100644
--- a/drivers/phy/ulpi/Makefile
+++ b/drivers/phy/ulpi/Makefile
@@ -1,2 +1,4 @@
ulpiphy-y := ulpi.o
obj-$(CONFIG_ULPI_PHY) += ulpiphy.o
+
+obj-$(CONFIG_ULPI_TUSB1210) += tusb1210.o
diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
new file mode 100644
index 0000000..ac77f98
--- /dev/null
+++ b/drivers/phy/ulpi/tusb1210.c
@@ -0,0 +1,131 @@
+/**
+ * tusb1210.c - TUSB1210 USB ULPI PHY driver
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/phy/ulpi/driver.h>
+#include <linux/phy/ulpi/regs.h>
+#include <linux/gpio/consumer.h>
+
+#include "ulpi_phy.h"
+
+struct tusb1210 {
+ struct ulpi *ulpi;
+ struct phy *phy;
+ struct gpio_desc *gpio_reset;
+ struct gpio_desc *gpio_cs;
+ u8 ctx[1];
+};
+
+static int tusb1210_power_on(struct phy *phy)
+{
+ struct tusb1210 *tusb = phy_get_drvdata(phy);
+
+ gpiod_set_value_cansleep(tusb->gpio_reset, 1);
+ gpiod_set_value_cansleep(tusb->gpio_cs, 1);
+
+ /* Restore eye optimisation value */
+ ulpi_write(tusb->ulpi, ULPI_EXT_VENDOR_SPECIFIC, tusb->ctx[0]);
+
+ return 0;
+}
+
+static int tusb1210_power_off(struct phy *phy)
+{
+ struct tusb1210 *tusb = phy_get_drvdata(phy);
+
+ gpiod_set_value_cansleep(tusb->gpio_reset, 0);
+ gpiod_set_value_cansleep(tusb->gpio_cs, 0);
+
+ return 0;
+}
+
+static struct phy_ops phy_ops = {
+ .power_on = tusb1210_power_on,
+ .power_off = tusb1210_power_off,
+ .init = tusb1210_power_on,
+ .exit = tusb1210_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int tusb1210_probe(struct ulpi *ulpi)
+{
+ struct gpio_desc *gpio;
+ struct tusb1210 *tusb;
+ int ret;
+
+ tusb = devm_kzalloc(&ulpi->dev, sizeof(*tusb), GFP_KERNEL);
+ if (!tusb)
+ return -ENOMEM;
+
+ gpio = devm_gpiod_get(&ulpi->dev, "reset");
+ if (!IS_ERR(gpio)) {
+ ret = gpiod_direction_output(gpio, 0);
+ if (ret)
+ return ret;
+ tusb->gpio_reset = gpio;
+ }
+
+ gpio = devm_gpiod_get(&ulpi->dev, "cs");
+ if (!IS_ERR(gpio)) {
+ ret = gpiod_direction_output(gpio, 0);
+ if (ret)
+ return ret;
+ tusb->gpio_cs = gpio;
+ }
+
+ /* Store initial eye diagram optimisation value */
+ ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
+ if (ret < 0)
+ return ret;
+
+ tusb->ctx[0] = ret;
+
+ tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
+ if (IS_ERR(tusb->phy))
+ return PTR_ERR(tusb->phy);
+
+ tusb->ulpi = ulpi;
+
+ phy_set_drvdata(tusb->phy, tusb);
+ dev_set_drvdata(&ulpi->dev, tusb);
+ return 0;
+}
+
+static void tusb1210_remove(struct ulpi *ulpi)
+{
+ struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
+
+ ulpi_phy_destroy(ulpi, tusb->phy);
+}
+
+#define TI_VENDOR_ID 0x0451
+
+static struct ulpi_device_id tusb1210_ulpi_id[] = {
+ { TI_VENDOR_ID, 0x1508, },
+ { },
+};
+MODULE_DEVICE_TABLE(ulpi, tusb1210_ulpi_id);
+
+static struct ulpi_driver tusb1210_driver = {
+ .id_table = tusb1210_ulpi_id,
+ .probe = tusb1210_probe,
+ .remove = tusb1210_remove,
+ .driver = {
+ .name = "tusb1210",
+ .owner = THIS_MODULE,
+ },
+};
+
+module_ulpi_driver(tusb1210_driver);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("TUSB1210 ULPI PHY driver");
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: add ULPI interface support
2015-01-20 9:18 ` [PATCH 2/3] usb: dwc3: add ULPI interface support Heikki Krogerus
@ 2015-01-20 15:23 ` Felipe Balbi
2015-01-21 8:58 ` Heikki Krogerus
0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2015-01-20 15:23 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel, Felipe Balbi
[-- Attachment #1: Type: text/plain, Size: 7983 bytes --]
Hi,
On Tue, Jan 20, 2015 at 11:18:21AM +0200, Heikki Krogerus wrote:
> Registers ULPI interface with the ULPI bus if HSPHY type is
> ULPI.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Felipe Balbi <balbi@ti.com>
you're doing quite a bit in a single patch...
> ---
> drivers/usb/dwc3/Kconfig | 7 ++++
> drivers/usb/dwc3/Makefile | 4 ++
> drivers/usb/dwc3/core.c | 9 +++-
> drivers/usb/dwc3/core.h | 22 ++++++++++
> drivers/usb/dwc3/ulpi.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 143 insertions(+), 1 deletion(-)
> create mode 100644 drivers/usb/dwc3/ulpi.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 58b5b2c..6d0c5e6 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -11,6 +11,13 @@ config USB_DWC3
>
> if USB_DWC3
>
> +config USB_DWC3_ULPI
> + bool "Provide ULPI PHY Interface"
> + depends on ULPI_PHY=y || ULPI_PHY=USB_DWC3
> + help
> + Select this if you have ULPI type PHY attached to your DWC3
> + controller.
> +
> choice
> bool "DWC3 Mode Selection"
> default USB_DWC3_DUAL_ROLE if (USB && USB_GADGET)
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index bb34fbc..2fc44e0 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -16,6 +16,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
> dwc3-y += gadget.o ep0.o
> endif
>
> +ifneq ($(CONFIG_USB_DWC3_ULPI),)
> + dwc3-y += ulpi.o
> +endif
> +
> ifneq ($(CONFIG_DEBUG_FS),)
> dwc3-y += debugfs.o
> endif
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25ddc39..5219bc7 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -876,12 +876,17 @@ static int dwc3_probe(struct platform_device *pdev)
> dwc->hird_threshold = hird_threshold
> | (dwc->is_utmi_l1_suspend << 4);
>
> + platform_set_drvdata(pdev, dwc);
> +
> + ret = dwc3_ulpi_init(dwc);
> + if (ret)
> + return ret;
> +
> ret = dwc3_core_get_phy(dwc);
> if (ret)
> return ret;
>
> spin_lock_init(&dwc->lock);
> - platform_set_drvdata(pdev, dwc);
why do you need to move this ? Looks like this should be a cleanup and
split into a single patch.
it also appears that you need another patch moving dwc3_cache_hwparams()
before all of these other calls, so you can use it from
dwc3_ulpi_init().
> @@ -965,6 +970,7 @@ err1:
>
> err0:
> dwc3_free_event_buffers(dwc);
> + dwc3_ulpi_exit(dwc);
>
> return ret;
> }
> @@ -984,6 +990,7 @@ static int dwc3_remove(struct platform_device *pdev)
> phy_power_off(dwc->usb3_generic_phy);
>
> dwc3_core_exit(dwc);
> + dwc3_ulpi_exit(dwc);
>
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 0842aa8..f6881a6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -32,6 +32,7 @@
> #include <linux/usb/otg.h>
>
> #include <linux/phy/phy.h>
> +#include <linux/phy/ulpi/interface.h>
>
> #define DWC3_MSG_MAX 500
>
> @@ -174,6 +175,14 @@
> #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31)
> #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6)
>
> +/* Global USB2 PHY Vendor Control Register */
> +#define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
> +#define DWC3_GUSB2PHYACC_BUSY (1 << 23)
> +#define DWC3_GUSB2PHYACC_WRITE (1 << 22)
> +#define DWC3_GUSB2PHYACC_ADDR(n) (n << 16)
> +#define DWC3_GUSB2PHYACC_EXTEND_ADDR(n) (n << 8)
> +#define DWC3_GUSB2PHYACC_DATA(n) (n & 0xff)
separate patch
> @@ -590,6 +599,7 @@ struct dwc3_hwparams {
> #define DWC3_NUM_INT(n) (((n) & (0x3f << 15)) >> 15)
>
> /* HWPARAMS3 */
> +#define DWC3_ULPI_HSPHY (1 << 3)
you also need a patch which defines this bit of HWPARAMS3. This is also
the wrong definition. Which core revision do you have ? I can see that
bit 3 is part of a 2 bit field called:
DWC_USB3_HSPHY_INTERFACE
moreover, there are systems which have both ULPI and UTMI enabled and
you can't really know which one the PHY is using.
This needs a bit more thought.
> #define DWC3_NUM_IN_EPS_MASK (0x1f << 18)
> #define DWC3_NUM_EPS_MASK (0x3f << 12)
> #define DWC3_NUM_EPS(p) (((p)->hwparams3 & \
> @@ -739,6 +749,8 @@ struct dwc3 {
> struct phy *usb2_generic_phy;
> struct phy *usb3_generic_phy;
>
> + struct ulpi *ulpi;
> +
> void __iomem *regs;
> size_t regs_size;
>
> @@ -1035,4 +1047,14 @@ static inline int dwc3_gadget_resume(struct dwc3 *dwc)
> }
> #endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
>
> +#if IS_ENABLED(CONFIG_USB_DWC3_ULPI)
> +int dwc3_ulpi_init(struct dwc3 *dwc);
> +void dwc3_ulpi_exit(struct dwc3 *dwc);
> +#else
> +static inline int dwc3_ulpi_init(struct dwc3 *dwc)
> +{ return 0; }
> +static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
> +{ }
> +#endif
> +
> #endif /* __DRIVERS_USB_DWC3_CORE_H */
> diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
> new file mode 100644
> index 0000000..ee3ebbe
> --- /dev/null
> +++ b/drivers/usb/dwc3/ulpi.c
> @@ -0,0 +1,102 @@
> +/**
> + * ulpi.c - DesignWare USB3 Controller's ULPI PHY interface
> + *
> + * Copyright (C) 2015 Intel Corporation
> + *
> + * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/phy/ulpi/regs.h>
> +
> +#include "core.h"
> +#include "io.h"
> +
> +#define DWC3_ULPI_ADDR(a) \
> + ((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
> + DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
> + DWC3_GUSB2PHYACC_EXTEND_ADDR(a) : DWC3_GUSB2PHYACC_ADDR(a))
> +
> +static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
> +{
> + unsigned count = 1000;
> + u32 reg;
> +
> + while (count--) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
> + if (!(reg & DWC3_GUSB2PHYACC_BUSY))
> + return 0;
this could use a cpu_relax();
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(ops->dev);
> + u32 reg;
> + int ret;
> +
> + reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
> +
> + ret = dwc3_ulpi_busyloop(dwc);
> + if (ret)
> + return ret;
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
> +
> + return DWC3_GUSB2PHYACC_DATA(reg);
> +}
> +
> +static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(ops->dev);
> + u32 reg;
> + int ret;
> +
> + reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
> + reg |= DWC3_GUSB2PHYACC_WRITE | val;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
> +
> + ret = dwc3_ulpi_busyloop(dwc);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static struct ulpi_ops dwc3_ulpi = {
> + .read = dwc3_ulpi_read,
> + .write = dwc3_ulpi_write,
> +};
> +
> +int dwc3_ulpi_init(struct dwc3 *dwc)
> +{
> + int ret;
> +
> + /* First check if there is ULPI PHY */
> + ret = dwc3_readl(dwc->regs, DWC3_GHWPARAMS3);
> + if (!(ret & DWC3_ULPI_HSPHY))
> + return 0;
should use the cached structure.
> + /* Register the interface */
> + dwc->ulpi = ulpi_register_interface(dwc->dev, &dwc3_ulpi);
> + if (IS_ERR(dwc->ulpi)) {
> + dev_err(dwc->dev, "failed to register ULPI interface");
> + return PTR_ERR(dwc->ulpi);
> + }
> +
> + return 0;
> +}
> +
> +void dwc3_ulpi_exit(struct dwc3 *dwc)
> +{
> + if (dwc->ulpi) {
> + ulpi_unregister_interface(dwc->ulpi);
> + dwc->ulpi = NULL;
> + }
> +}
> --
> 2.1.4
>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-20 9:18 ` [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210 Heikki Krogerus
@ 2015-01-20 15:45 ` Felipe Balbi
2015-01-21 9:17 ` Heikki Krogerus
0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2015-01-20 15:45 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5654 bytes --]
Hi,
On Tue, Jan 20, 2015 at 11:18:22AM +0200, Heikki Krogerus wrote:
> TUSB1210 ULPI PHY has vendor specific register for eye
> diagram tuning. On some platforms the system firmware has
> set optimized value to it. In order to not loose the
> optimized value, the driver stores it during probe and
> restores it every time the PHY is powered back on.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/phy/ulpi/Kconfig | 11 ++++
> drivers/phy/ulpi/Makefile | 2 +
> drivers/phy/ulpi/tusb1210.c | 131 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 144 insertions(+)
> create mode 100644 drivers/phy/ulpi/tusb1210.c
>
> diff --git a/drivers/phy/ulpi/Kconfig b/drivers/phy/ulpi/Kconfig
> index 8007df2..7cd6f82 100644
> --- a/drivers/phy/ulpi/Kconfig
> +++ b/drivers/phy/ulpi/Kconfig
> @@ -7,3 +7,14 @@ config ULPI_PHY
> Say yes if you have ULPI PHY attached to your USB controller.
>
> If unsure, say N.
> +
> +if ULPI_PHY
> +
> +config ULPI_TUSB1210
> + tristate "TI TUSB1210 USB PHY module"
> + depends on POWER_SUPPLY
> + select USB_PHY
> + help
> + Support for TI TUSB1210 USB ULPI PHY.
> +
> +endif
> diff --git a/drivers/phy/ulpi/Makefile b/drivers/phy/ulpi/Makefile
> index 59e61cb..7ee6679 100644
> --- a/drivers/phy/ulpi/Makefile
> +++ b/drivers/phy/ulpi/Makefile
> @@ -1,2 +1,4 @@
> ulpiphy-y := ulpi.o
> obj-$(CONFIG_ULPI_PHY) += ulpiphy.o
> +
> +obj-$(CONFIG_ULPI_TUSB1210) += tusb1210.o
> diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> new file mode 100644
> index 0000000..ac77f98
> --- /dev/null
> +++ b/drivers/phy/ulpi/tusb1210.c
do you really need this extra ulpi directory ?
I wonder if phy-tusb1210.c as a name would be enough.
> @@ -0,0 +1,131 @@
> +/**
> + * tusb1210.c - TUSB1210 USB ULPI PHY driver
> + *
> + * Copyright (C) 2015 Intel Corporation
> + *
> + * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/module.h>
> +#include <linux/phy/ulpi/driver.h>
> +#include <linux/phy/ulpi/regs.h>
> +#include <linux/gpio/consumer.h>
> +
> +#include "ulpi_phy.h"
> +
> +struct tusb1210 {
> + struct ulpi *ulpi;
> + struct phy *phy;
> + struct gpio_desc *gpio_reset;
> + struct gpio_desc *gpio_cs;
> + u8 ctx[1];
> +};
> +
> +static int tusb1210_power_on(struct phy *phy)
> +{
> + struct tusb1210 *tusb = phy_get_drvdata(phy);
> +
> + gpiod_set_value_cansleep(tusb->gpio_reset, 1);
> + gpiod_set_value_cansleep(tusb->gpio_cs, 1);
> +
> + /* Restore eye optimisation value */
> + ulpi_write(tusb->ulpi, ULPI_EXT_VENDOR_SPECIFIC, tusb->ctx[0]);
> +
> + return 0;
> +}
> +
> +static int tusb1210_power_off(struct phy *phy)
> +{
> + struct tusb1210 *tusb = phy_get_drvdata(phy);
> +
> + gpiod_set_value_cansleep(tusb->gpio_reset, 0);
> + gpiod_set_value_cansleep(tusb->gpio_cs, 0);
> +
> + return 0;
> +}
> +
> +static struct phy_ops phy_ops = {
> + .power_on = tusb1210_power_on,
> + .power_off = tusb1210_power_off,
> + .init = tusb1210_power_on,
> + .exit = tusb1210_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int tusb1210_probe(struct ulpi *ulpi)
> +{
> + struct gpio_desc *gpio;
> + struct tusb1210 *tusb;
> + int ret;
> +
> + tusb = devm_kzalloc(&ulpi->dev, sizeof(*tusb), GFP_KERNEL);
> + if (!tusb)
> + return -ENOMEM;
> +
> + gpio = devm_gpiod_get(&ulpi->dev, "reset");
> + if (!IS_ERR(gpio)) {
> + ret = gpiod_direction_output(gpio, 0);
> + if (ret)
> + return ret;
> + tusb->gpio_reset = gpio;
> + }
> +
> + gpio = devm_gpiod_get(&ulpi->dev, "cs");
> + if (!IS_ERR(gpio)) {
> + ret = gpiod_direction_output(gpio, 0);
> + if (ret)
> + return ret;
> + tusb->gpio_cs = gpio;
> + }
> +
> + /* Store initial eye diagram optimisation value */
> + ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
do they *all* use this register for eye diagram optimization or is this
something that Intel decided to do ?
(sorry, don't know much about tusb1210 other than it sucks like hell :-)
> + if (ret < 0)
> + return ret;
> +
> + tusb->ctx[0] = ret;
> +
> + tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> + if (IS_ERR(tusb->phy))
> + return PTR_ERR(tusb->phy);
> +
> + tusb->ulpi = ulpi;
> +
> + phy_set_drvdata(tusb->phy, tusb);
> + dev_set_drvdata(&ulpi->dev, tusb);
> + return 0;
> +}
> +
> +static void tusb1210_remove(struct ulpi *ulpi)
> +{
> + struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
completely unrelated to $subject, but we might want to have a
ulpi_{set,get}_drvdata() at some point.
In fact, we might decide to add an entire ULPI bus, eventually, though
I'm still considering if there's any benefit to that.
> +
> + ulpi_phy_destroy(ulpi, tusb->phy);
> +}
> +
> +#define TI_VENDOR_ID 0x0451
> +
> +static struct ulpi_device_id tusb1210_ulpi_id[] = {
> + { TI_VENDOR_ID, 0x1508, },
> + { },
> +};
> +MODULE_DEVICE_TABLE(ulpi, tusb1210_ulpi_id);
> +
> +static struct ulpi_driver tusb1210_driver = {
> + .id_table = tusb1210_ulpi_id,
> + .probe = tusb1210_probe,
> + .remove = tusb1210_remove,
> + .driver = {
> + .name = "tusb1210",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +module_ulpi_driver(tusb1210_driver);
> +
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
comment says GPL 2 only, this says GPL 2+
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: add ULPI interface support
2015-01-20 15:23 ` Felipe Balbi
@ 2015-01-21 8:58 ` Heikki Krogerus
0 siblings, 0 replies; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-21 8:58 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel
On Tue, Jan 20, 2015 at 09:23:37AM -0600, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jan 20, 2015 at 11:18:21AM +0200, Heikki Krogerus wrote:
> > Registers ULPI interface with the ULPI bus if HSPHY type is
> > ULPI.
> >
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Cc: Felipe Balbi <balbi@ti.com>
>
> you're doing quite a bit in a single patch...
>
> > ---
> > drivers/usb/dwc3/Kconfig | 7 ++++
> > drivers/usb/dwc3/Makefile | 4 ++
> > drivers/usb/dwc3/core.c | 9 +++-
> > drivers/usb/dwc3/core.h | 22 ++++++++++
> > drivers/usb/dwc3/ulpi.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 143 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/usb/dwc3/ulpi.c
> >
> > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> > index 58b5b2c..6d0c5e6 100644
> > --- a/drivers/usb/dwc3/Kconfig
> > +++ b/drivers/usb/dwc3/Kconfig
> > @@ -11,6 +11,13 @@ config USB_DWC3
> >
> > if USB_DWC3
> >
> > +config USB_DWC3_ULPI
> > + bool "Provide ULPI PHY Interface"
> > + depends on ULPI_PHY=y || ULPI_PHY=USB_DWC3
> > + help
> > + Select this if you have ULPI type PHY attached to your DWC3
> > + controller.
> > +
> > choice
> > bool "DWC3 Mode Selection"
> > default USB_DWC3_DUAL_ROLE if (USB && USB_GADGET)
> > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> > index bb34fbc..2fc44e0 100644
> > --- a/drivers/usb/dwc3/Makefile
> > +++ b/drivers/usb/dwc3/Makefile
> > @@ -16,6 +16,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
> > dwc3-y += gadget.o ep0.o
> > endif
> >
> > +ifneq ($(CONFIG_USB_DWC3_ULPI),)
> > + dwc3-y += ulpi.o
> > +endif
> > +
> > ifneq ($(CONFIG_DEBUG_FS),)
> > dwc3-y += debugfs.o
> > endif
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 25ddc39..5219bc7 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -876,12 +876,17 @@ static int dwc3_probe(struct platform_device *pdev)
> > dwc->hird_threshold = hird_threshold
> > | (dwc->is_utmi_l1_suspend << 4);
> >
> > + platform_set_drvdata(pdev, dwc);
> > +
> > + ret = dwc3_ulpi_init(dwc);
> > + if (ret)
> > + return ret;
> > +
> > ret = dwc3_core_get_phy(dwc);
> > if (ret)
> > return ret;
> >
> > spin_lock_init(&dwc->lock);
> > - platform_set_drvdata(pdev, dwc);
>
> why do you need to move this ? Looks like this should be a cleanup and
> split into a single patch.
OK.
> it also appears that you need another patch moving dwc3_cache_hwparams()
> before all of these other calls, so you can use it from
> dwc3_ulpi_init().
OK.
> > @@ -965,6 +970,7 @@ err1:
> >
> > err0:
> > dwc3_free_event_buffers(dwc);
> > + dwc3_ulpi_exit(dwc);
> >
> > return ret;
> > }
> > @@ -984,6 +990,7 @@ static int dwc3_remove(struct platform_device *pdev)
> > phy_power_off(dwc->usb3_generic_phy);
> >
> > dwc3_core_exit(dwc);
> > + dwc3_ulpi_exit(dwc);
> >
> > pm_runtime_put_sync(&pdev->dev);
> > pm_runtime_disable(&pdev->dev);
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > index 0842aa8..f6881a6 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -32,6 +32,7 @@
> > #include <linux/usb/otg.h>
> >
> > #include <linux/phy/phy.h>
> > +#include <linux/phy/ulpi/interface.h>
> >
> > #define DWC3_MSG_MAX 500
> >
> > @@ -174,6 +175,14 @@
> > #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31)
> > #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6)
> >
> > +/* Global USB2 PHY Vendor Control Register */
> > +#define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
> > +#define DWC3_GUSB2PHYACC_BUSY (1 << 23)
> > +#define DWC3_GUSB2PHYACC_WRITE (1 << 22)
> > +#define DWC3_GUSB2PHYACC_ADDR(n) (n << 16)
> > +#define DWC3_GUSB2PHYACC_EXTEND_ADDR(n) (n << 8)
> > +#define DWC3_GUSB2PHYACC_DATA(n) (n & 0xff)
>
> separate patch
OK.
> > @@ -590,6 +599,7 @@ struct dwc3_hwparams {
> > #define DWC3_NUM_INT(n) (((n) & (0x3f << 15)) >> 15)
> >
> > /* HWPARAMS3 */
> > +#define DWC3_ULPI_HSPHY (1 << 3)
>
> you also need a patch which defines this bit of HWPARAMS3. This is also
> the wrong definition. Which core revision do you have ? I can see that
> bit 3 is part of a 2 bit field called:
>
> DWC_USB3_HSPHY_INTERFACE
I have the same in my databook. I agree, it's not good like that.
> moreover, there are systems which have both ULPI and UTMI enabled and
> you can't really know which one the PHY is using.
>
> This needs a bit more thought.
Sure, I'll think of something better for this.
> > #define DWC3_NUM_IN_EPS_MASK (0x1f << 18)
> > #define DWC3_NUM_EPS_MASK (0x3f << 12)
> > #define DWC3_NUM_EPS(p) (((p)->hwparams3 & \
> > @@ -739,6 +749,8 @@ struct dwc3 {
> > struct phy *usb2_generic_phy;
> > struct phy *usb3_generic_phy;
> >
> > + struct ulpi *ulpi;
> > +
> > void __iomem *regs;
> > size_t regs_size;
> >
> > @@ -1035,4 +1047,14 @@ static inline int dwc3_gadget_resume(struct dwc3 *dwc)
> > }
> > #endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
> >
> > +#if IS_ENABLED(CONFIG_USB_DWC3_ULPI)
> > +int dwc3_ulpi_init(struct dwc3 *dwc);
> > +void dwc3_ulpi_exit(struct dwc3 *dwc);
> > +#else
> > +static inline int dwc3_ulpi_init(struct dwc3 *dwc)
> > +{ return 0; }
> > +static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
> > +{ }
> > +#endif
> > +
> > #endif /* __DRIVERS_USB_DWC3_CORE_H */
> > diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
> > new file mode 100644
> > index 0000000..ee3ebbe
> > --- /dev/null
> > +++ b/drivers/usb/dwc3/ulpi.c
> > @@ -0,0 +1,102 @@
> > +/**
> > + * ulpi.c - DesignWare USB3 Controller's ULPI PHY interface
> > + *
> > + * Copyright (C) 2015 Intel Corporation
> > + *
> > + * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/phy/ulpi/regs.h>
> > +
> > +#include "core.h"
> > +#include "io.h"
> > +
> > +#define DWC3_ULPI_ADDR(a) \
> > + ((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
> > + DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
> > + DWC3_GUSB2PHYACC_EXTEND_ADDR(a) : DWC3_GUSB2PHYACC_ADDR(a))
> > +
> > +static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
> > +{
> > + unsigned count = 1000;
> > + u32 reg;
> > +
> > + while (count--) {
> > + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
> > + if (!(reg & DWC3_GUSB2PHYACC_BUSY))
> > + return 0;
>
> this could use a cpu_relax();
Agreed.
> > + }
> > +
> > + return -ETIMEDOUT;
> > +}
> > +
> > +static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
> > +{
> > + struct dwc3 *dwc = dev_get_drvdata(ops->dev);
> > + u32 reg;
> > + int ret;
> > +
> > + reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
> > + dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
> > +
> > + ret = dwc3_ulpi_busyloop(dwc);
> > + if (ret)
> > + return ret;
> > +
> > + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
> > +
> > + return DWC3_GUSB2PHYACC_DATA(reg);
> > +}
> > +
> > +static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
> > +{
> > + struct dwc3 *dwc = dev_get_drvdata(ops->dev);
> > + u32 reg;
> > + int ret;
> > +
> > + reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
> > + reg |= DWC3_GUSB2PHYACC_WRITE | val;
> > + dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
> > +
> > + ret = dwc3_ulpi_busyloop(dwc);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static struct ulpi_ops dwc3_ulpi = {
> > + .read = dwc3_ulpi_read,
> > + .write = dwc3_ulpi_write,
> > +};
> > +
> > +int dwc3_ulpi_init(struct dwc3 *dwc)
> > +{
> > + int ret;
> > +
> > + /* First check if there is ULPI PHY */
> > + ret = dwc3_readl(dwc->regs, DWC3_GHWPARAMS3);
> > + if (!(ret & DWC3_ULPI_HSPHY))
> > + return 0;
>
> should use the cached structure.
Sure.
Thanks,
--
heikki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-20 15:45 ` Felipe Balbi
@ 2015-01-21 9:17 ` Heikki Krogerus
2015-01-21 11:39 ` Heikki Krogerus
2015-01-22 20:49 ` Felipe Balbi
0 siblings, 2 replies; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-21 9:17 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel
On Tue, Jan 20, 2015 at 09:45:39AM -0600, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jan 20, 2015 at 11:18:22AM +0200, Heikki Krogerus wrote:
> > TUSB1210 ULPI PHY has vendor specific register for eye
> > diagram tuning. On some platforms the system firmware has
> > set optimized value to it. In order to not loose the
> > optimized value, the driver stores it during probe and
> > restores it every time the PHY is powered back on.
> >
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> > drivers/phy/ulpi/Kconfig | 11 ++++
> > drivers/phy/ulpi/Makefile | 2 +
> > drivers/phy/ulpi/tusb1210.c | 131 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 144 insertions(+)
> > create mode 100644 drivers/phy/ulpi/tusb1210.c
> >
> > diff --git a/drivers/phy/ulpi/Kconfig b/drivers/phy/ulpi/Kconfig
> > index 8007df2..7cd6f82 100644
> > --- a/drivers/phy/ulpi/Kconfig
> > +++ b/drivers/phy/ulpi/Kconfig
> > @@ -7,3 +7,14 @@ config ULPI_PHY
> > Say yes if you have ULPI PHY attached to your USB controller.
> >
> > If unsure, say N.
> > +
> > +if ULPI_PHY
> > +
> > +config ULPI_TUSB1210
> > + tristate "TI TUSB1210 USB PHY module"
> > + depends on POWER_SUPPLY
> > + select USB_PHY
> > + help
> > + Support for TI TUSB1210 USB ULPI PHY.
> > +
> > +endif
> > diff --git a/drivers/phy/ulpi/Makefile b/drivers/phy/ulpi/Makefile
> > index 59e61cb..7ee6679 100644
> > --- a/drivers/phy/ulpi/Makefile
> > +++ b/drivers/phy/ulpi/Makefile
> > @@ -1,2 +1,4 @@
> > ulpiphy-y := ulpi.o
> > obj-$(CONFIG_ULPI_PHY) += ulpiphy.o
> > +
> > +obj-$(CONFIG_ULPI_TUSB1210) += tusb1210.o
> > diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> > new file mode 100644
> > index 0000000..ac77f98
> > --- /dev/null
> > +++ b/drivers/phy/ulpi/tusb1210.c
>
> do you really need this extra ulpi directory ?
>
> I wonder if phy-tusb1210.c as a name would be enough.
IMO grouping the ULPI PHY drivers and other ULPI bus code into
separate folder from the start is the right thing to do.
> > @@ -0,0 +1,131 @@
> > +/**
> > + * tusb1210.c - TUSB1210 USB ULPI PHY driver
> > + *
> > + * Copyright (C) 2015 Intel Corporation
> > + *
> > + * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +#include <linux/module.h>
> > +#include <linux/phy/ulpi/driver.h>
> > +#include <linux/phy/ulpi/regs.h>
> > +#include <linux/gpio/consumer.h>
> > +
> > +#include "ulpi_phy.h"
> > +
> > +struct tusb1210 {
> > + struct ulpi *ulpi;
> > + struct phy *phy;
> > + struct gpio_desc *gpio_reset;
> > + struct gpio_desc *gpio_cs;
> > + u8 ctx[1];
> > +};
> > +
> > +static int tusb1210_power_on(struct phy *phy)
> > +{
> > + struct tusb1210 *tusb = phy_get_drvdata(phy);
> > +
> > + gpiod_set_value_cansleep(tusb->gpio_reset, 1);
> > + gpiod_set_value_cansleep(tusb->gpio_cs, 1);
> > +
> > + /* Restore eye optimisation value */
> > + ulpi_write(tusb->ulpi, ULPI_EXT_VENDOR_SPECIFIC, tusb->ctx[0]);
> > +
> > + return 0;
> > +}
> > +
> > +static int tusb1210_power_off(struct phy *phy)
> > +{
> > + struct tusb1210 *tusb = phy_get_drvdata(phy);
> > +
> > + gpiod_set_value_cansleep(tusb->gpio_reset, 0);
> > + gpiod_set_value_cansleep(tusb->gpio_cs, 0);
> > +
> > + return 0;
> > +}
> > +
> > +static struct phy_ops phy_ops = {
> > + .power_on = tusb1210_power_on,
> > + .power_off = tusb1210_power_off,
> > + .init = tusb1210_power_on,
> > + .exit = tusb1210_power_off,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static int tusb1210_probe(struct ulpi *ulpi)
> > +{
> > + struct gpio_desc *gpio;
> > + struct tusb1210 *tusb;
> > + int ret;
> > +
> > + tusb = devm_kzalloc(&ulpi->dev, sizeof(*tusb), GFP_KERNEL);
> > + if (!tusb)
> > + return -ENOMEM;
> > +
> > + gpio = devm_gpiod_get(&ulpi->dev, "reset");
> > + if (!IS_ERR(gpio)) {
> > + ret = gpiod_direction_output(gpio, 0);
> > + if (ret)
> > + return ret;
> > + tusb->gpio_reset = gpio;
> > + }
> > +
> > + gpio = devm_gpiod_get(&ulpi->dev, "cs");
> > + if (!IS_ERR(gpio)) {
> > + ret = gpiod_direction_output(gpio, 0);
> > + if (ret)
> > + return ret;
> > + tusb->gpio_cs = gpio;
> > + }
> > +
> > + /* Store initial eye diagram optimisation value */
> > + ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
>
> do they *all* use this register for eye diagram optimization or is this
> something that Intel decided to do ?
>
> (sorry, don't know much about tusb1210 other than it sucks like hell :-)
All I know that somebody needs to save the value. The ones using this
PHY who don't need to save it can most likely live without the driver.
> > + if (ret < 0)
> > + return ret;
> > +
> > + tusb->ctx[0] = ret;
> > +
> > + tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> > + if (IS_ERR(tusb->phy))
> > + return PTR_ERR(tusb->phy);
> > +
> > + tusb->ulpi = ulpi;
> > +
> > + phy_set_drvdata(tusb->phy, tusb);
> > + dev_set_drvdata(&ulpi->dev, tusb);
> > + return 0;
> > +}
> > +
> > +static void tusb1210_remove(struct ulpi *ulpi)
> > +{
> > + struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
>
> completely unrelated to $subject, but we might want to have a
> ulpi_{set,get}_drvdata() at some point.
Makes sense.
> In fact, we might decide to add an entire ULPI bus, eventually, though
> I'm still considering if there's any benefit to that.
I don't think I understand this comment? ULPI bus is what I'm
introducing in this set (the first patch in it)?
> > +
> > + ulpi_phy_destroy(ulpi, tusb->phy);
> > +}
> > +
> > +#define TI_VENDOR_ID 0x0451
> > +
> > +static struct ulpi_device_id tusb1210_ulpi_id[] = {
> > + { TI_VENDOR_ID, 0x1508, },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(ulpi, tusb1210_ulpi_id);
> > +
> > +static struct ulpi_driver tusb1210_driver = {
> > + .id_table = tusb1210_ulpi_id,
> > + .probe = tusb1210_probe,
> > + .remove = tusb1210_remove,
> > + .driver = {
> > + .name = "tusb1210",
> > + .owner = THIS_MODULE,
> > + },
> > +};
> > +
> > +module_ulpi_driver(tusb1210_driver);
> > +
> > +MODULE_AUTHOR("Intel Corporation");
> > +MODULE_LICENSE("GPL");
>
> comment says GPL 2 only, this says GPL 2+
True. I'll fix it.
Thanks!
--
heikki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-21 9:17 ` Heikki Krogerus
@ 2015-01-21 11:39 ` Heikki Krogerus
2015-01-22 20:51 ` Felipe Balbi
2015-01-22 20:49 ` Felipe Balbi
1 sibling, 1 reply; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-21 11:39 UTC (permalink / raw)
To: Felipe Balbi, Alexander Shishkin
Cc: Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel
Hi,
On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> On Tue, Jan 20, 2015 at 09:45:39AM -0600, Felipe Balbi wrote:
> > > diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> > > new file mode 100644
> > > index 0000000..ac77f98
> > > --- /dev/null
> > > +++ b/drivers/phy/ulpi/tusb1210.c
> >
> > do you really need this extra ulpi directory ?
> >
> > I wonder if phy-tusb1210.c as a name would be enough.
>
> IMO grouping the ULPI PHY drivers and other ULPI bus code into
> separate folder from the start is the right thing to do.
A correction to this comment. I probable don't need this folder. Like
you said, phy-tusb1210.c should be enough..
<snip>
> > In fact, we might decide to add an entire ULPI bus, eventually, though
> > I'm still considering if there's any benefit to that.
>
> I don't think I understand this comment? ULPI bus is what I'm
> introducing in this set (the first patch in it)?
..I talked with Alex about this :). So I think the bus belongs under
drivers/usb/core/ instead of driver/phy/. It's not really tied to the
Generic PHY framework in any way, but ULPI is of course USB specific.
Cheers,
--
heikki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-21 9:17 ` Heikki Krogerus
2015-01-21 11:39 ` Heikki Krogerus
@ 2015-01-22 20:49 ` Felipe Balbi
2015-01-23 8:12 ` Heikki Krogerus
1 sibling, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2015-01-22 20:49 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Felipe Balbi, Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6408 bytes --]
Hi,
On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > TUSB1210 ULPI PHY has vendor specific register for eye
> > > diagram tuning. On some platforms the system firmware has
> > > set optimized value to it. In order to not loose the
> > > optimized value, the driver stores it during probe and
> > > restores it every time the PHY is powered back on.
> > >
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > > drivers/phy/ulpi/Kconfig | 11 ++++
> > > drivers/phy/ulpi/Makefile | 2 +
> > > drivers/phy/ulpi/tusb1210.c | 131 ++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 144 insertions(+)
> > > create mode 100644 drivers/phy/ulpi/tusb1210.c
> > >
> > > diff --git a/drivers/phy/ulpi/Kconfig b/drivers/phy/ulpi/Kconfig
> > > index 8007df2..7cd6f82 100644
> > > --- a/drivers/phy/ulpi/Kconfig
> > > +++ b/drivers/phy/ulpi/Kconfig
> > > @@ -7,3 +7,14 @@ config ULPI_PHY
> > > Say yes if you have ULPI PHY attached to your USB controller.
> > >
> > > If unsure, say N.
> > > +
> > > +if ULPI_PHY
> > > +
> > > +config ULPI_TUSB1210
> > > + tristate "TI TUSB1210 USB PHY module"
> > > + depends on POWER_SUPPLY
> > > + select USB_PHY
> > > + help
> > > + Support for TI TUSB1210 USB ULPI PHY.
> > > +
> > > +endif
> > > diff --git a/drivers/phy/ulpi/Makefile b/drivers/phy/ulpi/Makefile
> > > index 59e61cb..7ee6679 100644
> > > --- a/drivers/phy/ulpi/Makefile
> > > +++ b/drivers/phy/ulpi/Makefile
> > > @@ -1,2 +1,4 @@
> > > ulpiphy-y := ulpi.o
> > > obj-$(CONFIG_ULPI_PHY) += ulpiphy.o
> > > +
> > > +obj-$(CONFIG_ULPI_TUSB1210) += tusb1210.o
> > > diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> > > new file mode 100644
> > > index 0000000..ac77f98
> > > --- /dev/null
> > > +++ b/drivers/phy/ulpi/tusb1210.c
> >
> > do you really need this extra ulpi directory ?
> >
> > I wonder if phy-tusb1210.c as a name would be enough.
>
> IMO grouping the ULPI PHY drivers and other ULPI bus code into
> separate folder from the start is the right thing to do.
because... :-)
> > > @@ -0,0 +1,131 @@
> > > +/**
> > > + * tusb1210.c - TUSB1210 USB ULPI PHY driver
> > > + *
> > > + * Copyright (C) 2015 Intel Corporation
> > > + *
> > > + * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +#include <linux/module.h>
> > > +#include <linux/phy/ulpi/driver.h>
> > > +#include <linux/phy/ulpi/regs.h>
> > > +#include <linux/gpio/consumer.h>
> > > +
> > > +#include "ulpi_phy.h"
> > > +
> > > +struct tusb1210 {
> > > + struct ulpi *ulpi;
> > > + struct phy *phy;
> > > + struct gpio_desc *gpio_reset;
> > > + struct gpio_desc *gpio_cs;
> > > + u8 ctx[1];
> > > +};
> > > +
> > > +static int tusb1210_power_on(struct phy *phy)
> > > +{
> > > + struct tusb1210 *tusb = phy_get_drvdata(phy);
> > > +
> > > + gpiod_set_value_cansleep(tusb->gpio_reset, 1);
> > > + gpiod_set_value_cansleep(tusb->gpio_cs, 1);
> > > +
> > > + /* Restore eye optimisation value */
> > > + ulpi_write(tusb->ulpi, ULPI_EXT_VENDOR_SPECIFIC, tusb->ctx[0]);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tusb1210_power_off(struct phy *phy)
> > > +{
> > > + struct tusb1210 *tusb = phy_get_drvdata(phy);
> > > +
> > > + gpiod_set_value_cansleep(tusb->gpio_reset, 0);
> > > + gpiod_set_value_cansleep(tusb->gpio_cs, 0);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static struct phy_ops phy_ops = {
> > > + .power_on = tusb1210_power_on,
> > > + .power_off = tusb1210_power_off,
> > > + .init = tusb1210_power_on,
> > > + .exit = tusb1210_power_off,
> > > + .owner = THIS_MODULE,
> > > +};
> > > +
> > > +static int tusb1210_probe(struct ulpi *ulpi)
> > > +{
> > > + struct gpio_desc *gpio;
> > > + struct tusb1210 *tusb;
> > > + int ret;
> > > +
> > > + tusb = devm_kzalloc(&ulpi->dev, sizeof(*tusb), GFP_KERNEL);
> > > + if (!tusb)
> > > + return -ENOMEM;
> > > +
> > > + gpio = devm_gpiod_get(&ulpi->dev, "reset");
> > > + if (!IS_ERR(gpio)) {
> > > + ret = gpiod_direction_output(gpio, 0);
> > > + if (ret)
> > > + return ret;
> > > + tusb->gpio_reset = gpio;
> > > + }
> > > +
> > > + gpio = devm_gpiod_get(&ulpi->dev, "cs");
> > > + if (!IS_ERR(gpio)) {
> > > + ret = gpiod_direction_output(gpio, 0);
> > > + if (ret)
> > > + return ret;
> > > + tusb->gpio_cs = gpio;
> > > + }
> > > +
> > > + /* Store initial eye diagram optimisation value */
> > > + ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
> >
> > do they *all* use this register for eye diagram optimization or is this
> > something that Intel decided to do ?
> >
> > (sorry, don't know much about tusb1210 other than it sucks like hell :-)
>
> All I know that somebody needs to save the value. The ones using this
> PHY who don't need to save it can most likely live without the driver.
right, but what I mean is: is it mandatory that Eye diagram
configuration be stored in *this* register? Or is it more like a scratch
register which Intel just happens to be using for Eye diagram data ?
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + tusb->ctx[0] = ret;
> > > +
> > > + tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> > > + if (IS_ERR(tusb->phy))
> > > + return PTR_ERR(tusb->phy);
> > > +
> > > + tusb->ulpi = ulpi;
> > > +
> > > + phy_set_drvdata(tusb->phy, tusb);
> > > + dev_set_drvdata(&ulpi->dev, tusb);
> > > + return 0;
> > > +}
> > > +
> > > +static void tusb1210_remove(struct ulpi *ulpi)
> > > +{
> > > + struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
> >
> > completely unrelated to $subject, but we might want to have a
> > ulpi_{set,get}_drvdata() at some point.
>
> Makes sense.
>
> > In fact, we might decide to add an entire ULPI bus, eventually, though
> > I'm still considering if there's any benefit to that.
>
> I don't think I understand this comment? ULPI bus is what I'm
> introducing in this set (the first patch in it)?
I mean introducing a real struct bus ulpi_bus_type :-) With match,
probe, remove, etc.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-21 11:39 ` Heikki Krogerus
@ 2015-01-22 20:51 ` Felipe Balbi
2015-01-23 8:23 ` Heikki Krogerus
0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2015-01-22 20:51 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Felipe Balbi, Alexander Shishkin, Kishon Vijay Abraham I,
Baolu Lu, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]
On Wed, Jan 21, 2015 at 01:39:58PM +0200, Heikki Krogerus wrote:
> Hi,
>
> On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > On Tue, Jan 20, 2015 at 09:45:39AM -0600, Felipe Balbi wrote:
> > > > diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> > > > new file mode 100644
> > > > index 0000000..ac77f98
> > > > --- /dev/null
> > > > +++ b/drivers/phy/ulpi/tusb1210.c
> > >
> > > do you really need this extra ulpi directory ?
> > >
> > > I wonder if phy-tusb1210.c as a name would be enough.
> >
> > IMO grouping the ULPI PHY drivers and other ULPI bus code into
> > separate folder from the start is the right thing to do.
>
> A correction to this comment. I probable don't need this folder. Like
> you said, phy-tusb1210.c should be enough..
>
> <snip>
>
> > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > I'm still considering if there's any benefit to that.
> >
> > I don't think I understand this comment? ULPI bus is what I'm
> > introducing in this set (the first patch in it)?
>
> ..I talked with Alex about this :). So I think the bus belongs under
> drivers/usb/core/ instead of driver/phy/. It's not really tied to the
> Generic PHY framework in any way, but ULPI is of course USB specific.
right, maybe drivers/usb/ulpi or maybe drivers/ulpi, and have
phy-tusb1201 register under that ulpi_bus_type instead of
platform_bus_type, but still use drivers/phy to register itself a phy
provider ;-)
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-22 20:49 ` Felipe Balbi
@ 2015-01-23 8:12 ` Heikki Krogerus
2015-01-23 16:18 ` Felipe Balbi
0 siblings, 1 reply; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-23 8:12 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel
Hi,
On Thu, Jan 22, 2015 at 02:49:25PM -0600, Felipe Balbi wrote:
> On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > > + /* Store initial eye diagram optimisation value */
> > > > + ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
> > >
> > > do they *all* use this register for eye diagram optimization or is this
> > > something that Intel decided to do ?
> > >
> > > (sorry, don't know much about tusb1210 other than it sucks like hell :-)
> >
> > All I know that somebody needs to save the value. The ones using this
> > PHY who don't need to save it can most likely live without the driver.
>
> right, but what I mean is: is it mandatory that Eye diagram
> configuration be stored in *this* register? Or is it more like a scratch
> register which Intel just happens to be using for Eye diagram data ?
The eye diagram tuning is in that register. Here's the spec:
http://www.ti.com/lit/ds/symlink/tusb1210.pdf
I'll add definition for the register (which is colourfully named
"VENDOR_SPECIFIC2").
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + tusb->ctx[0] = ret;
> > > > +
> > > > + tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> > > > + if (IS_ERR(tusb->phy))
> > > > + return PTR_ERR(tusb->phy);
> > > > +
> > > > + tusb->ulpi = ulpi;
> > > > +
> > > > + phy_set_drvdata(tusb->phy, tusb);
> > > > + dev_set_drvdata(&ulpi->dev, tusb);
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static void tusb1210_remove(struct ulpi *ulpi)
> > > > +{
> > > > + struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
> > >
> > > completely unrelated to $subject, but we might want to have a
> > > ulpi_{set,get}_drvdata() at some point.
> >
> > Makes sense.
> >
> > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > I'm still considering if there's any benefit to that.
> >
> > I don't think I understand this comment? ULPI bus is what I'm
> > introducing in this set (the first patch in it)?
>
> I mean introducing a real struct bus ulpi_bus_type :-) With match,
> probe, remove, etc.
I'm already doing that. Please check the first patch in this set:
"phy: add bus for USB ULPI PHYs".
Cheers,
--
heikki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-22 20:51 ` Felipe Balbi
@ 2015-01-23 8:23 ` Heikki Krogerus
2015-01-23 16:16 ` Felipe Balbi
0 siblings, 1 reply; 15+ messages in thread
From: Heikki Krogerus @ 2015-01-23 8:23 UTC (permalink / raw)
To: Felipe Balbi
Cc: Alexander Shishkin, Kishon Vijay Abraham I, Baolu Lu, linux-usb,
linux-kernel
On Thu, Jan 22, 2015 at 02:51:24PM -0600, Felipe Balbi wrote:
> On Wed, Jan 21, 2015 at 01:39:58PM +0200, Heikki Krogerus wrote:
> > Hi,
> >
> > On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > On Tue, Jan 20, 2015 at 09:45:39AM -0600, Felipe Balbi wrote:
> > > > > diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> > > > > new file mode 100644
> > > > > index 0000000..ac77f98
> > > > > --- /dev/null
> > > > > +++ b/drivers/phy/ulpi/tusb1210.c
> > > >
> > > > do you really need this extra ulpi directory ?
> > > >
> > > > I wonder if phy-tusb1210.c as a name would be enough.
> > >
> > > IMO grouping the ULPI PHY drivers and other ULPI bus code into
> > > separate folder from the start is the right thing to do.
> >
> > A correction to this comment. I probable don't need this folder. Like
> > you said, phy-tusb1210.c should be enough..
> >
> > <snip>
> >
> > > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > > I'm still considering if there's any benefit to that.
> > >
> > > I don't think I understand this comment? ULPI bus is what I'm
> > > introducing in this set (the first patch in it)?
> >
> > ..I talked with Alex about this :). So I think the bus belongs under
> > drivers/usb/core/ instead of driver/phy/. It's not really tied to the
> > Generic PHY framework in any way, but ULPI is of course USB specific.
>
> right, maybe drivers/usb/ulpi or maybe drivers/ulpi, and have
> phy-tusb1201 register under that ulpi_bus_type instead of
> platform_bus_type, but still use drivers/phy to register itself a phy
> provider ;-)
So just for the record: This driver does not register under
platform_bus_type bus but instead already under ulpi_bus_type.
I'll prepare new version out of these today and try to figure out
proper place for the code (maybe drivers/usb/ulpi?).
Cheers,
--
heikki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-23 8:23 ` Heikki Krogerus
@ 2015-01-23 16:16 ` Felipe Balbi
0 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2015-01-23 16:16 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Felipe Balbi, Alexander Shishkin, Kishon Vijay Abraham I,
Baolu Lu, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2207 bytes --]
On Fri, Jan 23, 2015 at 10:23:48AM +0200, Heikki Krogerus wrote:
> On Thu, Jan 22, 2015 at 02:51:24PM -0600, Felipe Balbi wrote:
> > On Wed, Jan 21, 2015 at 01:39:58PM +0200, Heikki Krogerus wrote:
> > > Hi,
> > >
> > > On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > > On Tue, Jan 20, 2015 at 09:45:39AM -0600, Felipe Balbi wrote:
> > > > > > diff --git a/drivers/phy/ulpi/tusb1210.c b/drivers/phy/ulpi/tusb1210.c
> > > > > > new file mode 100644
> > > > > > index 0000000..ac77f98
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/phy/ulpi/tusb1210.c
> > > > >
> > > > > do you really need this extra ulpi directory ?
> > > > >
> > > > > I wonder if phy-tusb1210.c as a name would be enough.
> > > >
> > > > IMO grouping the ULPI PHY drivers and other ULPI bus code into
> > > > separate folder from the start is the right thing to do.
> > >
> > > A correction to this comment. I probable don't need this folder. Like
> > > you said, phy-tusb1210.c should be enough..
> > >
> > > <snip>
> > >
> > > > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > > > I'm still considering if there's any benefit to that.
> > > >
> > > > I don't think I understand this comment? ULPI bus is what I'm
> > > > introducing in this set (the first patch in it)?
> > >
> > > ..I talked with Alex about this :). So I think the bus belongs under
> > > drivers/usb/core/ instead of driver/phy/. It's not really tied to the
> > > Generic PHY framework in any way, but ULPI is of course USB specific.
> >
> > right, maybe drivers/usb/ulpi or maybe drivers/ulpi, and have
> > phy-tusb1201 register under that ulpi_bus_type instead of
> > platform_bus_type, but still use drivers/phy to register itself a phy
> > provider ;-)
>
> So just for the record: This driver does not register under
> platform_bus_type bus but instead already under ulpi_bus_type.
hah! :-)
> I'll prepare new version out of these today and try to figure out
> proper place for the code (maybe drivers/usb/ulpi?).
drivers/phy is fine, it is a phy driver anyway... I just should've read
it to see there was a ulpi_bus_type :-)
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210
2015-01-23 8:12 ` Heikki Krogerus
@ 2015-01-23 16:18 ` Felipe Balbi
0 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2015-01-23 16:18 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Felipe Balbi, Kishon Vijay Abraham I, Baolu Lu, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2472 bytes --]
On Fri, Jan 23, 2015 at 10:12:41AM +0200, Heikki Krogerus wrote:
> Hi,
>
> On Thu, Jan 22, 2015 at 02:49:25PM -0600, Felipe Balbi wrote:
> > On Wed, Jan 21, 2015 at 11:17:49AM +0200, Heikki Krogerus wrote:
> > > > > + /* Store initial eye diagram optimisation value */
> > > > > + ret = ulpi_read(ulpi, ULPI_EXT_VENDOR_SPECIFIC);
> > > >
> > > > do they *all* use this register for eye diagram optimization or is this
> > > > something that Intel decided to do ?
> > > >
> > > > (sorry, don't know much about tusb1210 other than it sucks like hell :-)
> > >
> > > All I know that somebody needs to save the value. The ones using this
> > > PHY who don't need to save it can most likely live without the driver.
> >
> > right, but what I mean is: is it mandatory that Eye diagram
> > configuration be stored in *this* register? Or is it more like a scratch
> > register which Intel just happens to be using for Eye diagram data ?
>
> The eye diagram tuning is in that register. Here's the spec:
> http://www.ti.com/lit/ds/symlink/tusb1210.pdf
>
> I'll add definition for the register (which is colourfully named
> "VENDOR_SPECIFIC2").
alright, thanks.
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > +
> > > > > + tusb->ctx[0] = ret;
> > > > > +
> > > > > + tusb->phy = ulpi_phy_create(ulpi, &phy_ops);
> > > > > + if (IS_ERR(tusb->phy))
> > > > > + return PTR_ERR(tusb->phy);
> > > > > +
> > > > > + tusb->ulpi = ulpi;
> > > > > +
> > > > > + phy_set_drvdata(tusb->phy, tusb);
> > > > > + dev_set_drvdata(&ulpi->dev, tusb);
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static void tusb1210_remove(struct ulpi *ulpi)
> > > > > +{
> > > > > + struct tusb1210 *tusb = dev_get_drvdata(&ulpi->dev);
> > > >
> > > > completely unrelated to $subject, but we might want to have a
> > > > ulpi_{set,get}_drvdata() at some point.
> > >
> > > Makes sense.
> > >
> > > > In fact, we might decide to add an entire ULPI bus, eventually, though
> > > > I'm still considering if there's any benefit to that.
> > >
> > > I don't think I understand this comment? ULPI bus is what I'm
> > > introducing in this set (the first patch in it)?
> >
> > I mean introducing a real struct bus ulpi_bus_type :-) With match,
> > probe, remove, etc.
>
> I'm already doing that. Please check the first patch in this set:
> "phy: add bus for USB ULPI PHYs".
yeah, sorry about that.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-01-23 16:18 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-20 9:18 [PATCH 0/3] phy: ulpi bus Heikki Krogerus
2015-01-20 9:18 ` [PATCH 1/3] phy: add bus for USB ULPI PHYs Heikki Krogerus
2015-01-20 9:18 ` [PATCH 2/3] usb: dwc3: add ULPI interface support Heikki Krogerus
2015-01-20 15:23 ` Felipe Balbi
2015-01-21 8:58 ` Heikki Krogerus
2015-01-20 9:18 ` [PATCH 3/3] phy: ulpi: add driver for TI TUSB1210 Heikki Krogerus
2015-01-20 15:45 ` Felipe Balbi
2015-01-21 9:17 ` Heikki Krogerus
2015-01-21 11:39 ` Heikki Krogerus
2015-01-22 20:51 ` Felipe Balbi
2015-01-23 8:23 ` Heikki Krogerus
2015-01-23 16:16 ` Felipe Balbi
2015-01-22 20:49 ` Felipe Balbi
2015-01-23 8:12 ` Heikki Krogerus
2015-01-23 16:18 ` 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.