* [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4
@ 2018-02-17 21:07 Tony Lindgren
2018-02-18 0:31 ` Sebastian Reichel
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tony Lindgren @ 2018-02-17 21:07 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: linux-kernel, linux-usb, linux-omap, devicetree, Mark Rutland,
Marcel Partap, Michael Scott, Rob Herring, Sebastian Reichel
Let's add support for the GPIO controlled USB PHY on the MDM6600 modem.
It is used on some Motorola Mapphone series of phones and tablets such
as Droid 4.
The MDM6600 is hardwired to the first OHCI port in the Droid 4 case, and
is controlled by several GPIOs. The USB PHY is integrated into the MDM6600
device it seems. We know this as we get L3 errors from omap-usb-host if
trying to use the PHY before MDM6600 is configured.
The GPIOs controlling MDM6600 are used to power MDM660 on and off, to
configure the USB start-up mode (normal mode versus USB flashing), and
they also tell the state of the MDM6600 device.
The two start-up mode GPIOs are dual-purposed and used for out of band
(OOB) wake-up for USB and TS 27.010 serial mux. But we need to configure
the USB start-up mode first to get MDM6600 booted in the right mode to
be usable in the first place.
For now, this driver just gives up the two start-up mode GPIOs after the
modem has been configured to boot in normal mode. One of them we may
want to configure for USB OOB wake in this driver later on, but that's a
separate series of patches and needs more work.
Note that the Motorola Mapphone Linux kernel tree has a "radio-ctrl"
driver for modems. But it really does not control the radio at all, it
just controls the modem power and start-up mode for USB. So I came to
the conclusion that we're better off having this done in the USB PHY
driver. For adding support for USB flashing mode, we can later on add
a kernel module option for flash_mode=1 or something similar.
Also note that currently there is no PM runtime support for the OHCI
on omap variant SoCs. So for low(er) power idle states, currenty both
ohci-platform and phy-mapphone-mdm6600 must be unloaded or unbound.
For reference here is what I measured for total power consumption on
an idle Droid 4 with and without USB related MDM6600 modules:
idle lcd off phy-mapphone-mdm6600 ohci-platform
153mW 284mW 344mW
So it seems that MDM6600 is currently not yet idling even with it's
radio turned off, but that's something that is beyond the control of
this USB PHY driver.
Cc: devicetree@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Michael Scott <michael.scott@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
.../bindings/phy/phy-mapphone-mdm6600.txt | 30 ++
drivers/phy/motorola/Kconfig | 9 +
drivers/phy/motorola/Makefile | 1 +
drivers/phy/motorola/phy-mapphone-mdm6600.c | 490 +++++++++++++++++++++
4 files changed, 530 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
create mode 100644 drivers/phy/motorola/phy-mapphone-mdm6600.c
diff --git a/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
@@ -0,0 +1,30 @@
+Device tree binding documentation for Motorola Mapphone MDM6600 USB PHY
+
+Required properties:
+- compatible Must be "motorola,mapphone-mdm6600"
+- enable-gpios GPIO to enable the USB PHY
+- power-gpios GPIO to power on the device
+- reset-gpios GPIO to reset the device
+- mode-gpios Two GPIOs to configure MDM6600 USB start-up mode for
+ normal mode versus USB flashing mode
+- status-gpios Three GPIOs to read the power state of the MDM6600
+- cmd-gpios Three GPIOs to control the power state of the MDM6600
+
+Example:
+
+fsusb1_phy: fsusb1_phy {
+ compatible = "motorola,mapphone-mdm6600";
+ enable-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>; /* gpio_95 */
+ power-gpios = <&gpio2 22 GPIO_ACTIVE_HIGH>; /* gpio_54 */
+ reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; /* gpio_49 */
+ mode-gpios = <&gpio5 20 GPIO_ACTIVE_HIGH>, /* gpio_148 */
+ <&gpio5 21 GPIO_ACTIVE_HIGH>; /* gpio_149 */
+ status-gpios = <&gpio2 23 GPIO_ACTIVE_HIGH>, /* gpio_55 */
+ <&gpio2 21 GPIO_ACTIVE_HIGH>, /* gpio_53 */
+ <&gpio2 20 GPIO_ACTIVE_HIGH>; /* gpio_52 */
+ cmd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>, /* gpio_103 */
+ <&gpio4 8 GPIO_ACTIVE_HIGH>, /* gpio_104 */
+ <&gpio5 14 GPIO_ACTIVE_HIGH>; /* gpio_142 */
+ #phy-cells = <0>;
+};
+
diff --git a/drivers/phy/motorola/Kconfig b/drivers/phy/motorola/Kconfig
--- a/drivers/phy/motorola/Kconfig
+++ b/drivers/phy/motorola/Kconfig
@@ -10,3 +10,12 @@ config PHY_CPCAP_USB
help
Enable this for USB to work on Motorola phones and tablets
such as Droid 4.
+
+config PHY_MAPPHONE_MDM6600
+ tristate "Motorola Mapphone MDM6600 modem USB PHY driver"
+ depends on USB_SUPPORT
+ select GENERIC_PHY
+ select USB_PHY
+ help
+ Enable this for MDM6600 USB modem to work on Motorola phones
+ and tablets such as Droid 4.
diff --git a/drivers/phy/motorola/Makefile b/drivers/phy/motorola/Makefile
--- a/drivers/phy/motorola/Makefile
+++ b/drivers/phy/motorola/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_PHY_CPCAP_USB) += phy-cpcap-usb.o
+obj-$(CONFIG_PHY_MAPPHONE_MDM6600) += phy-mapphone-mdm6600.o
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
new file mode 100644
--- /dev/null
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Motorola Mapphone MDM6600 modem GPIO controlled USB PHY driver
+ * Copyright (C) 2018 Tony Lindgren <tony@atomide.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/usb/phy_companion.h>
+
+#define PHY_MDM6600_STARTUP_DELAY_MS 3000 /* A least 2.2s usually */
+
+/*
+ * MDM6600 status codes. These are copied from Motorola Mapphone Linux
+ * kernel tree. The BB naming here refers to "BaseBand" for modem.
+ */
+enum phy_mdm6600_status {
+ BP_STATUS_PANIC, /* Seems to be really off state */
+ BP_STATUS_PANIC_BUSY_WAIT,
+ BP_STATUS_QC_DLOAD,
+ BP_STATUS_RAM_DOWNLOADER, /* MDM6600 USB flashing mode */
+ BP_STATUS_PHONE_CODE_AWAKE, /* MDM6600 normal USB mode */
+ BP_STATUS_PHONE_CODE_ASLEEP,
+ BP_STATUS_SHUTDOWN_ACK,
+ BP_STATUS_UNDEFINED,
+};
+
+static const char * const
+phy_mdm6600_status_name[] = {
+ "off", "busy", "qc_dl", "ram_dl", "awake",
+ "asleep", "shutdown", "undefined",
+};
+
+/*
+ * MDM6600 command codes. These are copied from Motorola Mapphone Linux
+ * kernel tree. The AP naming here refers to "Application Processor".
+ */
+enum phy_mdm6600_cmd {
+ AP_STATUS_BP_PANIC_ACK,
+ AP_STATUS_DATA_ONLY_BYPASS, /* Reroute USB to CPCAP PHY */
+ AP_STATUS_FULL_BYPASS, /* Reroute USB to CPCAP PHY */
+ AP_STATUS_NO_BYPASS, /* Request normal start-up mode */
+ AP_STATUS_BP_SHUTDOWN_REQ, /* Request device power off */
+ AP_STATUS_BP_UNKNOWN_5,
+ AP_STATUS_BP_UNKNOWN_6,
+ AP_STATUS_UNDEFINED,
+};
+
+enum phy_mdm6600_lines {
+ PHY_MDM6600_ENABLE, /* USB PHY enable */
+ PHY_MDM6600_POWER, /* Device power */
+ PHY_MDM6600_RESET, /* Device reset */
+ PHY_MDM6600_MODE0, /* USB boot mode flashing vs normal */
+ PHY_MDM6600_MODE1, /* USB boot mode flashing vs normal */
+ PHY_MDM6600_STATUS0, /* Device state */
+ PHY_MDM6600_STATUS1, /* Device state */
+ PHY_MDM6600_STATUS2, /* Device state */
+ PHY_MDM6600_CMD0, /* Device command */
+ PHY_MDM6600_CMD1, /* Device command */
+ PHY_MDM6600_CMD2, /* Device command */
+ PHY_MDM6600_NR_LINES,
+};
+
+struct phy_mdm6600 {
+ struct device *dev;
+ struct usb_phy phy;
+ struct phy *generic_phy;
+ struct phy_provider *phy_provider;
+ struct gpio_desc *gpio[PHY_MDM6600_NR_LINES];
+ struct delayed_work bootup_work;
+ struct delayed_work status_work;
+ struct completion ack;
+ bool enabled;
+ int status;
+};
+
+static int phy_mdm6600_init(struct phy *x)
+{
+ struct phy_mdm6600 *ddata = phy_get_drvdata(x);
+ struct gpio_desc *enable_gpio = ddata->gpio[PHY_MDM6600_ENABLE];
+
+ if (!ddata->enabled)
+ return -EPROBE_DEFER;
+
+ gpiod_set_value_cansleep(enable_gpio, 0);
+
+ return 0;
+}
+
+static int phy_mdm6600_power_on(struct phy *x)
+{
+ struct phy_mdm6600 *ddata = phy_get_drvdata(x);
+ struct gpio_desc *enable_gpio = ddata->gpio[PHY_MDM6600_ENABLE];
+
+ if (!ddata->enabled)
+ return -ENODEV;
+
+ gpiod_set_value_cansleep(enable_gpio, 1);
+
+ return 0;
+}
+
+static int phy_mdm6600_power_off(struct phy *x)
+{
+ struct phy_mdm6600 *ddata = phy_get_drvdata(x);
+ struct gpio_desc *enable_gpio = ddata->gpio[PHY_MDM6600_ENABLE];
+
+ if (!ddata->enabled)
+ return -ENODEV;
+
+ gpiod_set_value_cansleep(enable_gpio, 0);
+
+ return 0;
+}
+
+static const struct phy_ops gpio_usb_ops = {
+ .init = phy_mdm6600_init,
+ .power_on = phy_mdm6600_power_on,
+ .power_off = phy_mdm6600_power_off,
+ .owner = THIS_MODULE,
+};
+
+struct phy_mdm6600_map {
+ const char *name;
+ int nr_gpios;
+ int direction;
+};
+
+static const struct phy_mdm6600_map
+phy_mdm6600_line_map[PHY_MDM6600_NR_LINES] = {
+ { "enable", 1, GPIOD_OUT_LOW, }, /* low = disabled */
+ { "power", 1, GPIOD_OUT_LOW, }, /* low = off */
+ { "reset", 1, GPIOD_OUT_HIGH, }, /* high = reset */
+ { "mode", 2, GPIOD_OUT_LOW, },
+ { "status", 3, GPIOD_IN, },
+ { "cmd", 3, GPIOD_OUT_LOW, }, /* low = no command */
+};
+
+/**
+ * phy_mdm6600_cmd() - send a command request to mdm6600
+ * @ddata: device driver data
+ *
+ * Configures the three command request GPIOs to the specified value.
+ */
+static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
+{
+ int i;
+
+ val &= 0x7;
+
+ for (i = PHY_MDM6600_CMD0;
+ i <= PHY_MDM6600_CMD2; i++) {
+ struct gpio_desc *gpio = ddata->gpio[i];
+ int shift = (2 - (i - PHY_MDM6600_CMD0));
+
+ if (IS_ERR(gpio))
+ return;
+
+ gpiod_set_value_cansleep(gpio, (val & BIT(shift)) >> shift);
+ }
+}
+
+/**
+ * phy_mdm6600_status() - read mdm6600 status lines
+ * @ddata: device driver data
+ */
+static void phy_mdm6600_status(struct work_struct *work)
+{
+ struct phy_mdm6600 *ddata;
+ struct device *dev;
+ int i, val;
+
+ ddata = container_of(work, struct phy_mdm6600, status_work.work);
+ dev = ddata->dev;
+
+ for (i = PHY_MDM6600_STATUS0;
+ i <= PHY_MDM6600_STATUS2; i++) {
+ struct gpio_desc *gpio = ddata->gpio[i];
+ int shift = (2 - (i - PHY_MDM6600_STATUS0));
+
+ if (IS_ERR(ddata->gpio[i]))
+ continue;
+ val = gpiod_get_value_cansleep(gpio);
+ ddata->status &= ~(BIT(shift));
+ ddata->status |= (val << shift);
+ }
+ dev_info(dev, "modem status: %i %s\n",
+ ddata->status,
+ phy_mdm6600_status_name[ddata->status & 7]);
+ complete(&ddata->ack);
+}
+
+static irqreturn_t phy_mdm6600_irq_thread(int irq, void *data)
+{
+ struct phy_mdm6600 *ddata = data;
+
+ schedule_delayed_work(&ddata->status_work, msecs_to_jiffies(10));
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * phy_mdm6600_init_irq() - initialize mdm6600 status IRQ lines
+ * @ddata: device driver data
+ */
+static void phy_mdm6600_init_irq(struct phy_mdm6600 *ddata)
+{
+ struct device *dev = ddata->dev;
+ int i, error, irq;
+
+ for (i = PHY_MDM6600_STATUS0;
+ i <= PHY_MDM6600_STATUS2; i++) {
+ if (IS_ERR(ddata->gpio[i]))
+ continue;
+
+ irq = gpiod_to_irq(ddata->gpio[i]);
+ if (irq <= 0)
+ continue;
+
+ error = devm_request_threaded_irq(dev, irq, NULL,
+ phy_mdm6600_irq_thread,
+ IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT,
+ "mdm6600",
+ ddata);
+ if (error)
+ dev_warn(dev, "no modem status irq%i: %i\n",
+ irq, error);
+ }
+}
+
+/**
+ * phy_mdm6600_init_lines() - initialize mdm6600 GPIO lines
+ * @ddata: device driver data
+ */
+static int phy_mdm6600_init_lines(struct phy_mdm6600 *ddata)
+{
+ struct device *dev = ddata->dev;
+ int i, j, nr_gpio = 0;
+
+ for (i = 0; i < ARRAY_SIZE(phy_mdm6600_line_map); i++) {
+ const struct phy_mdm6600_map *map =
+ &phy_mdm6600_line_map[i];
+
+ for (j = 0; j < map->nr_gpios; j++) {
+ struct gpio_desc **gpio = &ddata->gpio[nr_gpio];
+
+ *gpio = devm_gpiod_get_index(dev,
+ map->name, j,
+ map->direction);
+ if (IS_ERR(*gpio)) {
+ dev_info(dev,
+ "gpio %s error %li, already taken?\n",
+ map->name, PTR_ERR(*gpio));
+ return PTR_ERR(*gpio);
+ }
+ nr_gpio++;
+ }
+ }
+
+ return 0;
+}
+
+/**
+ * phy_mdm6600_device_power_on() - power on mdm6600 device
+ * @ddata: device driver data
+ *
+ * To get the integrated USB phy in MDM6600 takes some hoops. We must ensure
+ * the shared USB bootmode GPIOs are configured, then request modem start-up,
+ * reset and power-up.. And then we need to give up the shared USB bootmode
+ * GPIOs as they are also used for Out of Band (OOB) wake for the USB and
+ * TS 27.010 serial mux.
+ */
+static int phy_mdm6600_device_power_on(struct phy_mdm6600 *ddata)
+{
+ struct gpio_desc *mode_gpio0 = ddata->gpio[PHY_MDM6600_MODE0];
+ struct gpio_desc *mode_gpio1 = ddata->gpio[PHY_MDM6600_MODE1];
+ struct gpio_desc *reset_gpio = ddata->gpio[PHY_MDM6600_RESET];
+ struct gpio_desc *power_gpio = ddata->gpio[PHY_MDM6600_POWER];
+ int error = 0;
+
+ /*
+ * Shared GPIOs must be low for normal USB mode. After booting,
+ * we don't need them. These can be also used to configure USB
+ * flashing mode later on based on a module parameter.
+ */
+ gpiod_set_value_cansleep(mode_gpio0, 0);
+ gpiod_set_value_cansleep(mode_gpio1, 0);
+
+ /* Request start-up mode */
+ phy_mdm6600_cmd(ddata, AP_STATUS_NO_BYPASS);
+
+ /* Request a reset first */
+ gpiod_set_value_cansleep(reset_gpio, 0);
+ msleep(100);
+
+ /* Toggle power GPIO to request mdm6600 to start */
+ gpiod_set_value_cansleep(power_gpio, 1);
+ msleep(100);
+ gpiod_set_value_cansleep(power_gpio, 0);
+
+ /*
+ * Looks like the USB PHY is at least 2.2 seconds.
+ * If we try to use it before that, we will get L3 errors
+ * from omap-usb-host trying to access the PHY. See also
+ * phy_mdm6600_init() for -EPROBE_DEFER.
+ */
+ msleep(PHY_MDM6600_STARTUP_DELAY_MS);
+ ddata->enabled = true;
+
+ /* Booting up the rest of MDM6600 will take total about 8 seconds */
+ dev_info(ddata->dev, "Waiting for power up request to complete..\n");
+ if (wait_for_completion_timeout(&ddata->ack,
+ msecs_to_jiffies(8000))) {
+ dev_info(ddata->dev, "Powered up OK\n");
+ } else {
+ ddata->enabled = false;
+ error = -ETIMEDOUT;
+ dev_err(ddata->dev, "Timed out powering up\n");
+ }
+
+ /* Give up shared GPIOs now, they will be used for OOB wake */
+ devm_gpiod_put(ddata->dev, mode_gpio0);
+ ddata->gpio[PHY_MDM6600_MODE0] = ERR_PTR(-ENODEV);
+ devm_gpiod_put(ddata->dev, mode_gpio1);
+ ddata->gpio[PHY_MDM6600_MODE0] = ERR_PTR(-ENODEV);
+
+ return error;
+}
+
+/**
+ * phy_mdm6600_device_power_off() - power off mdm6600 device
+ * @ddata: device driver data
+ */
+static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
+{
+ struct gpio_desc *reset_gpio =
+ ddata->gpio[PHY_MDM6600_RESET];
+
+ ddata->enabled = false;
+ phy_mdm6600_cmd(ddata, AP_STATUS_BP_SHUTDOWN_REQ);
+ msleep(100);
+
+ if (reset_gpio >= 0)
+ gpiod_set_value_cansleep(reset_gpio, 1);
+
+ dev_info(ddata->dev, "Waiting for power down request to complete.. ");
+ if (wait_for_completion_timeout(&ddata->ack,
+ msecs_to_jiffies(5000))) {
+ dev_info(ddata->dev, "Powered down OK\n");
+ } else {
+ dev_err(ddata->dev, "Timed out powering down\n");
+ }
+}
+
+static void phy_mdm6600_deferred_power_on(struct work_struct *work)
+{
+ struct phy_mdm6600 *ddata;
+ int error;
+
+ ddata = container_of(work, struct phy_mdm6600, bootup_work.work);
+
+ error = phy_mdm6600_device_power_on(ddata);
+ if (error)
+ dev_err(ddata->dev, "Device not functional\n");
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id phy_mdm6600_id_table[] = {
+ { .compatible = "motorola,mapphone-mdm6600", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, phy_mdm6600_id_table);
+#endif
+
+static int phy_mdm6600_probe(struct platform_device *pdev)
+{
+ struct phy_mdm6600 *ddata;
+ struct usb_otg *otg;
+ const struct of_device_id *of_id;
+ int error;
+
+ of_id = of_match_device(of_match_ptr(phy_mdm6600_id_table),
+ &pdev->dev);
+ if (!of_id)
+ return -EINVAL;
+
+ ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ INIT_DELAYED_WORK(&ddata->bootup_work,
+ phy_mdm6600_deferred_power_on);
+ INIT_DELAYED_WORK(&ddata->status_work, phy_mdm6600_status);
+ init_completion(&ddata->ack);
+
+ otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
+ if (!otg)
+ return -ENOMEM;
+
+ ddata->dev = &pdev->dev;
+ ddata->phy.dev = ddata->dev;
+ ddata->phy.label = "phy_mdm6600";
+ ddata->phy.otg = otg;
+ ddata->phy.type = USB_PHY_TYPE_USB2;
+ otg->usb_phy = &ddata->phy;
+
+ platform_set_drvdata(pdev, ddata);
+
+ error = phy_mdm6600_init_lines(ddata);
+ if (error)
+ return error;
+
+ phy_mdm6600_init_irq(ddata);
+
+ ddata->generic_phy = devm_phy_create(ddata->dev, NULL, &gpio_usb_ops);
+ if (IS_ERR(ddata->generic_phy)) {
+ error = PTR_ERR(ddata->generic_phy);
+ goto cleanup;
+ }
+
+ phy_set_drvdata(ddata->generic_phy, ddata);
+
+ ddata->phy_provider =
+ devm_of_phy_provider_register(ddata->dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(ddata->phy_provider)) {
+ error = PTR_ERR(ddata->phy_provider);
+ goto cleanup;
+ }
+
+ schedule_delayed_work(&ddata->bootup_work, 0);
+
+ /*
+ * See phy_mdm6600_device_power_on(). We should be able
+ * to remove this eventually when ohci-platform can deal
+ * with -EPROBE_DEFER.
+ */
+ msleep(PHY_MDM6600_STARTUP_DELAY_MS + 500);
+
+ usb_add_phy_dev(&ddata->phy);
+
+ return 0;
+
+cleanup:
+ phy_mdm6600_device_power_off(ddata);
+ return error;
+}
+
+static int phy_mdm6600_remove(struct platform_device *pdev)
+{
+ struct phy_mdm6600 *ddata = platform_get_drvdata(pdev);
+ struct gpio_desc *reset_gpio = ddata->gpio[PHY_MDM6600_RESET];
+
+ if (!IS_ERR(reset_gpio))
+ gpiod_set_value_cansleep(reset_gpio, 1);
+
+ phy_mdm6600_device_power_off(ddata);
+
+ cancel_delayed_work_sync(&ddata->bootup_work);
+ cancel_delayed_work_sync(&ddata->status_work);
+
+ return 0;
+}
+
+static struct platform_driver phy_mdm6600_driver = {
+ .probe = phy_mdm6600_probe,
+ .remove = phy_mdm6600_remove,
+ .driver = {
+ .name = "phy-mapphone-mdm6600",
+ .of_match_table = of_match_ptr(phy_mdm6600_id_table),
+ },
+};
+
+module_platform_driver(phy_mdm6600_driver);
+
+MODULE_ALIAS("platform:gpio_usb");
+MODULE_AUTHOR("Tony Lindgren <tony@atomide.com>");
+MODULE_DESCRIPTION("generic gpio usb phy driver");
+MODULE_LICENSE("GPL v2");
--
2.16.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4
2018-02-17 21:07 [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4 Tony Lindgren
@ 2018-02-18 0:31 ` Sebastian Reichel
2018-02-18 17:50 ` Tony Lindgren
2018-02-19 7:45 ` kbuild test robot
2018-02-19 20:40 ` Rob Herring
2 siblings, 1 reply; 7+ messages in thread
From: Sebastian Reichel @ 2018-02-18 0:31 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kishon Vijay Abraham I, linux-kernel, linux-usb, linux-omap,
devicetree, Mark Rutland, Marcel Partap, Michael Scott,
Rob Herring
[-- Attachment #1: Type: text/plain, Size: 21430 bytes --]
Hi Tony,
On Sat, Feb 17, 2018 at 01:07:23PM -0800, Tony Lindgren wrote:
> Let's add support for the GPIO controlled USB PHY on the MDM6600 modem.
> It is used on some Motorola Mapphone series of phones and tablets such
> as Droid 4.
>
> The MDM6600 is hardwired to the first OHCI port in the Droid 4 case, and
> is controlled by several GPIOs. The USB PHY is integrated into the MDM6600
> device it seems. We know this as we get L3 errors from omap-usb-host if
> trying to use the PHY before MDM6600 is configured.
>
> The GPIOs controlling MDM6600 are used to power MDM660 on and off, to
> configure the USB start-up mode (normal mode versus USB flashing), and
> they also tell the state of the MDM6600 device.
>
> The two start-up mode GPIOs are dual-purposed and used for out of band
> (OOB) wake-up for USB and TS 27.010 serial mux. But we need to configure
> the USB start-up mode first to get MDM6600 booted in the right mode to
> be usable in the first place.
>
> For now, this driver just gives up the two start-up mode GPIOs after the
> modem has been configured to boot in normal mode. One of them we may
> want to configure for USB OOB wake in this driver later on, but that's a
> separate series of patches and needs more work.
>
> Note that the Motorola Mapphone Linux kernel tree has a "radio-ctrl"
> driver for modems. But it really does not control the radio at all, it
> just controls the modem power and start-up mode for USB. So I came to
> the conclusion that we're better off having this done in the USB PHY
> driver. For adding support for USB flashing mode, we can later on add
> a kernel module option for flash_mode=1 or something similar.
>
> Also note that currently there is no PM runtime support for the OHCI
> on omap variant SoCs. So for low(er) power idle states, currenty both
> ohci-platform and phy-mapphone-mdm6600 must be unloaded or unbound.
>
> For reference here is what I measured for total power consumption on
> an idle Droid 4 with and without USB related MDM6600 modules:
>
> idle lcd off phy-mapphone-mdm6600 ohci-platform
> 153mW 284mW 344mW
So more than twice the idle power... We really want to get runtime
PM working :/
> So it seems that MDM6600 is currently not yet idling even with it's
> radio turned off, but that's something that is beyond the control of
> this USB PHY driver.
>
> Cc: devicetree@vger.kernel.org
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Michael Scott <michael.scott@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> .../bindings/phy/phy-mapphone-mdm6600.txt | 30 ++
> drivers/phy/motorola/Kconfig | 9 +
> drivers/phy/motorola/Makefile | 1 +
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 490 +++++++++++++++++++++
> 4 files changed, 530 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> create mode 100644 drivers/phy/motorola/phy-mapphone-mdm6600.c
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> @@ -0,0 +1,30 @@
> +Device tree binding documentation for Motorola Mapphone MDM6600 USB PHY
> +
> +Required properties:
> +- compatible Must be "motorola,mapphone-mdm6600"
> +- enable-gpios GPIO to enable the USB PHY
> +- power-gpios GPIO to power on the device
> +- reset-gpios GPIO to reset the device
> +- mode-gpios Two GPIOs to configure MDM6600 USB start-up mode for
> + normal mode versus USB flashing mode
> +- status-gpios Three GPIOs to read the power state of the MDM6600
> +- cmd-gpios Three GPIOs to control the power state of the MDM6600
> +
> +Example:
> +
> +fsusb1_phy: fsusb1_phy {
> + compatible = "motorola,mapphone-mdm6600";
> + enable-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>; /* gpio_95 */
> + power-gpios = <&gpio2 22 GPIO_ACTIVE_HIGH>; /* gpio_54 */
> + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; /* gpio_49 */
> + mode-gpios = <&gpio5 20 GPIO_ACTIVE_HIGH>, /* gpio_148 */
> + <&gpio5 21 GPIO_ACTIVE_HIGH>; /* gpio_149 */
> + status-gpios = <&gpio2 23 GPIO_ACTIVE_HIGH>, /* gpio_55 */
> + <&gpio2 21 GPIO_ACTIVE_HIGH>, /* gpio_53 */
> + <&gpio2 20 GPIO_ACTIVE_HIGH>; /* gpio_52 */
> + cmd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>, /* gpio_103 */
> + <&gpio4 8 GPIO_ACTIVE_HIGH>, /* gpio_104 */
> + <&gpio5 14 GPIO_ACTIVE_HIGH>; /* gpio_142 */
> + #phy-cells = <0>;
> +};
> +
> diff --git a/drivers/phy/motorola/Kconfig b/drivers/phy/motorola/Kconfig
> --- a/drivers/phy/motorola/Kconfig
> +++ b/drivers/phy/motorola/Kconfig
> @@ -10,3 +10,12 @@ config PHY_CPCAP_USB
> help
> Enable this for USB to work on Motorola phones and tablets
> such as Droid 4.
> +
> +config PHY_MAPPHONE_MDM6600
> + tristate "Motorola Mapphone MDM6600 modem USB PHY driver"
> + depends on USB_SUPPORT
> + select GENERIC_PHY
> + select USB_PHY
> + help
> + Enable this for MDM6600 USB modem to work on Motorola phones
> + and tablets such as Droid 4.
> diff --git a/drivers/phy/motorola/Makefile b/drivers/phy/motorola/Makefile
> --- a/drivers/phy/motorola/Makefile
> +++ b/drivers/phy/motorola/Makefile
> @@ -3,3 +3,4 @@
> #
>
> obj-$(CONFIG_PHY_CPCAP_USB) += phy-cpcap-usb.o
> +obj-$(CONFIG_PHY_MAPPHONE_MDM6600) += phy-mapphone-mdm6600.o
> diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> @@ -0,0 +1,490 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Motorola Mapphone MDM6600 modem GPIO controlled USB PHY driver
> + * Copyright (C) 2018 Tony Lindgren <tony@atomide.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/usb/phy_companion.h>
> +
> +#define PHY_MDM6600_STARTUP_DELAY_MS 3000 /* A least 2.2s usually */
> +
> +/*
> + * MDM6600 status codes. These are copied from Motorola Mapphone Linux
> + * kernel tree. The BB naming here refers to "BaseBand" for modem.
> + */
Actually your status codes are BP_ (baseband processor) prefixed.
> +enum phy_mdm6600_status {
> + BP_STATUS_PANIC, /* Seems to be really off state */
> + BP_STATUS_PANIC_BUSY_WAIT,
> + BP_STATUS_QC_DLOAD,
> + BP_STATUS_RAM_DOWNLOADER, /* MDM6600 USB flashing mode */
> + BP_STATUS_PHONE_CODE_AWAKE, /* MDM6600 normal USB mode */
> + BP_STATUS_PHONE_CODE_ASLEEP,
> + BP_STATUS_SHUTDOWN_ACK,
> + BP_STATUS_UNDEFINED,
> +};
> +
> +static const char * const
> +phy_mdm6600_status_name[] = {
> + "off", "busy", "qc_dl", "ram_dl", "awake",
> + "asleep", "shutdown", "undefined",
> +};
> +
> +/*
> + * MDM6600 command codes. These are copied from Motorola Mapphone Linux
> + * kernel tree. The AP naming here refers to "Application Processor".
> + */
> +enum phy_mdm6600_cmd {
> + AP_STATUS_BP_PANIC_ACK,
> + AP_STATUS_DATA_ONLY_BYPASS, /* Reroute USB to CPCAP PHY */
> + AP_STATUS_FULL_BYPASS, /* Reroute USB to CPCAP PHY */
> + AP_STATUS_NO_BYPASS, /* Request normal start-up mode */
> + AP_STATUS_BP_SHUTDOWN_REQ, /* Request device power off */
> + AP_STATUS_BP_UNKNOWN_5,
> + AP_STATUS_BP_UNKNOWN_6,
> + AP_STATUS_UNDEFINED,
> +};
> +
> +enum phy_mdm6600_lines {
> + PHY_MDM6600_ENABLE, /* USB PHY enable */
> + PHY_MDM6600_POWER, /* Device power */
> + PHY_MDM6600_RESET, /* Device reset */
> + PHY_MDM6600_MODE0, /* USB boot mode flashing vs normal */
> + PHY_MDM6600_MODE1, /* USB boot mode flashing vs normal */
> + PHY_MDM6600_STATUS0, /* Device state */
> + PHY_MDM6600_STATUS1, /* Device state */
> + PHY_MDM6600_STATUS2, /* Device state */
> + PHY_MDM6600_CMD0, /* Device command */
> + PHY_MDM6600_CMD1, /* Device command */
> + PHY_MDM6600_CMD2, /* Device command */
> + PHY_MDM6600_NR_LINES,
> +};
> +
> +struct phy_mdm6600 {
> + struct device *dev;
> + struct usb_phy phy;
> + struct phy *generic_phy;
> + struct phy_provider *phy_provider;
> + struct gpio_desc *gpio[PHY_MDM6600_NR_LINES];
> + struct delayed_work bootup_work;
> + struct delayed_work status_work;
> + struct completion ack;
> + bool enabled;
> + int status;
> +};
> +
> +static int phy_mdm6600_init(struct phy *x)
> +{
> + struct phy_mdm6600 *ddata = phy_get_drvdata(x);
> + struct gpio_desc *enable_gpio = ddata->gpio[PHY_MDM6600_ENABLE];
> +
> + if (!ddata->enabled)
> + return -EPROBE_DEFER;
> +
> + gpiod_set_value_cansleep(enable_gpio, 0);
> +
> + return 0;
> +}
> +
> +static int phy_mdm6600_power_on(struct phy *x)
> +{
> + struct phy_mdm6600 *ddata = phy_get_drvdata(x);
> + struct gpio_desc *enable_gpio = ddata->gpio[PHY_MDM6600_ENABLE];
> +
> + if (!ddata->enabled)
> + return -ENODEV;
> +
> + gpiod_set_value_cansleep(enable_gpio, 1);
> +
> + return 0;
> +}
> +
> +static int phy_mdm6600_power_off(struct phy *x)
> +{
> + struct phy_mdm6600 *ddata = phy_get_drvdata(x);
> + struct gpio_desc *enable_gpio = ddata->gpio[PHY_MDM6600_ENABLE];
> +
> + if (!ddata->enabled)
> + return -ENODEV;
> +
> + gpiod_set_value_cansleep(enable_gpio, 0);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops gpio_usb_ops = {
> + .init = phy_mdm6600_init,
> + .power_on = phy_mdm6600_power_on,
> + .power_off = phy_mdm6600_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +struct phy_mdm6600_map {
> + const char *name;
> + int nr_gpios;
> + int direction;
> +};
> +
> +static const struct phy_mdm6600_map
> +phy_mdm6600_line_map[PHY_MDM6600_NR_LINES] = {
> + { "enable", 1, GPIOD_OUT_LOW, }, /* low = disabled */
> + { "power", 1, GPIOD_OUT_LOW, }, /* low = off */
> + { "reset", 1, GPIOD_OUT_HIGH, }, /* high = reset */
> + { "mode", 2, GPIOD_OUT_LOW, },
> + { "status", 3, GPIOD_IN, },
> + { "cmd", 3, GPIOD_OUT_LOW, }, /* low = no command */
> +};
> +
> +/**
> + * phy_mdm6600_cmd() - send a command request to mdm6600
> + * @ddata: device driver data
> + *
> + * Configures the three command request GPIOs to the specified value.
> + */
> +static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
> +{
> + int i;
> +
> + val &= 0x7;
> +
> + for (i = PHY_MDM6600_CMD0;
> + i <= PHY_MDM6600_CMD2; i++) {
> + struct gpio_desc *gpio = ddata->gpio[i];
> + int shift = (2 - (i - PHY_MDM6600_CMD0));
> +
> + if (IS_ERR(gpio))
> + return;
> +
> + gpiod_set_value_cansleep(gpio, (val & BIT(shift)) >> shift);
> + }
> +}
> +
> +/**
> + * phy_mdm6600_status() - read mdm6600 status lines
> + * @ddata: device driver data
> + */
> +static void phy_mdm6600_status(struct work_struct *work)
> +{
> + struct phy_mdm6600 *ddata;
> + struct device *dev;
> + int i, val;
> +
> + ddata = container_of(work, struct phy_mdm6600, status_work.work);
> + dev = ddata->dev;
> +
> + for (i = PHY_MDM6600_STATUS0;
> + i <= PHY_MDM6600_STATUS2; i++) {
> + struct gpio_desc *gpio = ddata->gpio[i];
> + int shift = (2 - (i - PHY_MDM6600_STATUS0));
> +
> + if (IS_ERR(ddata->gpio[i]))
> + continue;
> + val = gpiod_get_value_cansleep(gpio);
> + ddata->status &= ~(BIT(shift));
> + ddata->status |= (val << shift);
> + }
> + dev_info(dev, "modem status: %i %s\n",
> + ddata->status,
> + phy_mdm6600_status_name[ddata->status & 7]);
> + complete(&ddata->ack);
> +}
> +
> +static irqreturn_t phy_mdm6600_irq_thread(int irq, void *data)
> +{
> + struct phy_mdm6600 *ddata = data;
> +
> + schedule_delayed_work(&ddata->status_work, msecs_to_jiffies(10));
> +
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * phy_mdm6600_init_irq() - initialize mdm6600 status IRQ lines
> + * @ddata: device driver data
> + */
> +static void phy_mdm6600_init_irq(struct phy_mdm6600 *ddata)
> +{
> + struct device *dev = ddata->dev;
> + int i, error, irq;
> +
> + for (i = PHY_MDM6600_STATUS0;
> + i <= PHY_MDM6600_STATUS2; i++) {
> + if (IS_ERR(ddata->gpio[i]))
> + continue;
This can be dropped, since the driver errors out of probe
when there are gpio errors.
> + irq = gpiod_to_irq(ddata->gpio[i]);
> + if (irq <= 0)
> + continue;
> +
> + error = devm_request_threaded_irq(dev, irq, NULL,
> + phy_mdm6600_irq_thread,
> + IRQF_TRIGGER_RISING |
> + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT,
> + "mdm6600",
> + ddata);
> + if (error)
> + dev_warn(dev, "no modem status irq%i: %i\n",
> + irq, error);
> + }
> +}
> +
> +/**
> + * phy_mdm6600_init_lines() - initialize mdm6600 GPIO lines
> + * @ddata: device driver data
> + */
> +static int phy_mdm6600_init_lines(struct phy_mdm6600 *ddata)
> +{
> + struct device *dev = ddata->dev;
> + int i, j, nr_gpio = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(phy_mdm6600_line_map); i++) {
> + const struct phy_mdm6600_map *map =
> + &phy_mdm6600_line_map[i];
> +
> + for (j = 0; j < map->nr_gpios; j++) {
> + struct gpio_desc **gpio = &ddata->gpio[nr_gpio];
> +
> + *gpio = devm_gpiod_get_index(dev,
> + map->name, j,
> + map->direction);
> + if (IS_ERR(*gpio)) {
> + dev_info(dev,
> + "gpio %s error %li, already taken?\n",
> + map->name, PTR_ERR(*gpio));
> + return PTR_ERR(*gpio);
> + }
> + nr_gpio++;
> + }
I think the code should use the gpiod_get_array() API.
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * phy_mdm6600_device_power_on() - power on mdm6600 device
> + * @ddata: device driver data
> + *
> + * To get the integrated USB phy in MDM6600 takes some hoops. We must ensure
> + * the shared USB bootmode GPIOs are configured, then request modem start-up,
> + * reset and power-up.. And then we need to give up the shared USB bootmode
> + * GPIOs as they are also used for Out of Band (OOB) wake for the USB and
> + * TS 27.010 serial mux.
> + */
> +static int phy_mdm6600_device_power_on(struct phy_mdm6600 *ddata)
> +{
> + struct gpio_desc *mode_gpio0 = ddata->gpio[PHY_MDM6600_MODE0];
> + struct gpio_desc *mode_gpio1 = ddata->gpio[PHY_MDM6600_MODE1];
> + struct gpio_desc *reset_gpio = ddata->gpio[PHY_MDM6600_RESET];
> + struct gpio_desc *power_gpio = ddata->gpio[PHY_MDM6600_POWER];
> + int error = 0;
> +
> + /*
> + * Shared GPIOs must be low for normal USB mode. After booting,
> + * we don't need them. These can be also used to configure USB
> + * flashing mode later on based on a module parameter.
> + */
> + gpiod_set_value_cansleep(mode_gpio0, 0);
> + gpiod_set_value_cansleep(mode_gpio1, 0);
> +
> + /* Request start-up mode */
> + phy_mdm6600_cmd(ddata, AP_STATUS_NO_BYPASS);
> +
> + /* Request a reset first */
> + gpiod_set_value_cansleep(reset_gpio, 0);
> + msleep(100);
> +
> + /* Toggle power GPIO to request mdm6600 to start */
> + gpiod_set_value_cansleep(power_gpio, 1);
> + msleep(100);
> + gpiod_set_value_cansleep(power_gpio, 0);
> +
> + /*
> + * Looks like the USB PHY is at least 2.2 seconds.
> + * If we try to use it before that, we will get L3 errors
> + * from omap-usb-host trying to access the PHY. See also
> + * phy_mdm6600_init() for -EPROBE_DEFER.
> + */
> + msleep(PHY_MDM6600_STARTUP_DELAY_MS);
> + ddata->enabled = true;
> +
> + /* Booting up the rest of MDM6600 will take total about 8 seconds */
> + dev_info(ddata->dev, "Waiting for power up request to complete..\n");
> + if (wait_for_completion_timeout(&ddata->ack,
> + msecs_to_jiffies(8000))) {
> + dev_info(ddata->dev, "Powered up OK\n");
> + } else {
> + ddata->enabled = false;
> + error = -ETIMEDOUT;
> + dev_err(ddata->dev, "Timed out powering up\n");
> + }
> +
> + /* Give up shared GPIOs now, they will be used for OOB wake */
> + devm_gpiod_put(ddata->dev, mode_gpio0);
> + ddata->gpio[PHY_MDM6600_MODE0] = ERR_PTR(-ENODEV);
> + devm_gpiod_put(ddata->dev, mode_gpio1);
> + ddata->gpio[PHY_MDM6600_MODE0] = ERR_PTR(-ENODEV);
You want PHY_MDM6600_MODE1 instead. Also I would just use NULL.
NULL is used by gpiod_get_optional and is handled by the gpiod
functions, so you don't need to check for gpio errors everywhere.
> + return error;
> +}
> +
> +/**
> + * phy_mdm6600_device_power_off() - power off mdm6600 device
> + * @ddata: device driver data
> + */
> +static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
> +{
> + struct gpio_desc *reset_gpio =
> + ddata->gpio[PHY_MDM6600_RESET];
> +
> + ddata->enabled = false;
> + phy_mdm6600_cmd(ddata, AP_STATUS_BP_SHUTDOWN_REQ);
> + msleep(100);
> +
> + if (reset_gpio >= 0)
> + gpiod_set_value_cansleep(reset_gpio, 1);
> +
> + dev_info(ddata->dev, "Waiting for power down request to complete.. ");
> + if (wait_for_completion_timeout(&ddata->ack,
> + msecs_to_jiffies(5000))) {
> + dev_info(ddata->dev, "Powered down OK\n");
> + } else {
> + dev_err(ddata->dev, "Timed out powering down\n");
> + }
> +}
> +
> +static void phy_mdm6600_deferred_power_on(struct work_struct *work)
> +{
> + struct phy_mdm6600 *ddata;
> + int error;
> +
> + ddata = container_of(work, struct phy_mdm6600, bootup_work.work);
> +
> + error = phy_mdm6600_device_power_on(ddata);
> + if (error)
> + dev_err(ddata->dev, "Device not functional\n");
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id phy_mdm6600_id_table[] = {
> + { .compatible = "motorola,mapphone-mdm6600", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, phy_mdm6600_id_table);
> +#endif
I suggest to just depend on CONFIG_OF in Kconfig and drop the ifdef
and of_match_ptr() parts. It's very unlikely, that this will be
used without DT and would need quite some rework anyways.
> +static int phy_mdm6600_probe(struct platform_device *pdev)
> +{
> + struct phy_mdm6600 *ddata;
> + struct usb_otg *otg;
> + const struct of_device_id *of_id;
> + int error;
> +
> + of_id = of_match_device(of_match_ptr(phy_mdm6600_id_table),
> + &pdev->dev);
> + if (!of_id)
> + return -EINVAL;
I suggest to drop the of_match_device(). The driver will error
out anyways when it can't get the gpios.
> + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + INIT_DELAYED_WORK(&ddata->bootup_work,
> + phy_mdm6600_deferred_power_on);
> + INIT_DELAYED_WORK(&ddata->status_work, phy_mdm6600_status);
> + init_completion(&ddata->ack);
> +
> + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> + if (!otg)
> + return -ENOMEM;
> +
> + ddata->dev = &pdev->dev;
> + ddata->phy.dev = ddata->dev;
> + ddata->phy.label = "phy_mdm6600";
> + ddata->phy.otg = otg;
> + ddata->phy.type = USB_PHY_TYPE_USB2;
> + otg->usb_phy = &ddata->phy;
> +
> + platform_set_drvdata(pdev, ddata);
> +
> + error = phy_mdm6600_init_lines(ddata);
> + if (error)
> + return error;
> +
> + phy_mdm6600_init_irq(ddata);
> +
> + ddata->generic_phy = devm_phy_create(ddata->dev, NULL, &gpio_usb_ops);
> + if (IS_ERR(ddata->generic_phy)) {
> + error = PTR_ERR(ddata->generic_phy);
> + goto cleanup;
> + }
> +
> + phy_set_drvdata(ddata->generic_phy, ddata);
> +
> + ddata->phy_provider =
> + devm_of_phy_provider_register(ddata->dev,
> + of_phy_simple_xlate);
> + if (IS_ERR(ddata->phy_provider)) {
> + error = PTR_ERR(ddata->phy_provider);
> + goto cleanup;
> + }
> +
> + schedule_delayed_work(&ddata->bootup_work, 0);
> +
> + /*
> + * See phy_mdm6600_device_power_on(). We should be able
> + * to remove this eventually when ohci-platform can deal
> + * with -EPROBE_DEFER.
> + */
> + msleep(PHY_MDM6600_STARTUP_DELAY_MS + 500);
> +
> + usb_add_phy_dev(&ddata->phy);
> +
> + return 0;
> +
> +cleanup:
> + phy_mdm6600_device_power_off(ddata);
> + return error;
> +}
> +
> +static int phy_mdm6600_remove(struct platform_device *pdev)
> +{
> + struct phy_mdm6600 *ddata = platform_get_drvdata(pdev);
> + struct gpio_desc *reset_gpio = ddata->gpio[PHY_MDM6600_RESET];
> +
> + if (!IS_ERR(reset_gpio))
> + gpiod_set_value_cansleep(reset_gpio, 1);
> +
> + phy_mdm6600_device_power_off(ddata);
> +
> + cancel_delayed_work_sync(&ddata->bootup_work);
> + cancel_delayed_work_sync(&ddata->status_work);
> +
> + return 0;
> +}
> +
> +static struct platform_driver phy_mdm6600_driver = {
> + .probe = phy_mdm6600_probe,
> + .remove = phy_mdm6600_remove,
> + .driver = {
> + .name = "phy-mapphone-mdm6600",
> + .of_match_table = of_match_ptr(phy_mdm6600_id_table),
> + },
> +};
> +
> +module_platform_driver(phy_mdm6600_driver);
> +
> +MODULE_ALIAS("platform:gpio_usb");
> +MODULE_AUTHOR("Tony Lindgren <tony@atomide.com>");
> +MODULE_DESCRIPTION("generic gpio usb phy driver");
> +MODULE_LICENSE("GPL v2");
Generally I'm a bit worried about handling the mode gpios in two
different drivers. It looks like it might become a dependency hell.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4
2018-02-18 0:31 ` Sebastian Reichel
@ 2018-02-18 17:50 ` Tony Lindgren
0 siblings, 0 replies; 7+ messages in thread
From: Tony Lindgren @ 2018-02-18 17:50 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Kishon Vijay Abraham I, linux-kernel, linux-usb, linux-omap,
devicetree, Mark Rutland, Marcel Partap, Michael Scott,
Rob Herring
* Sebastian Reichel <sre@kernel.org> [180218 00:32]:
> On Sat, Feb 17, 2018 at 01:07:23PM -0800, Tony Lindgren wrote:
> > For reference here is what I measured for total power consumption on
> > an idle Droid 4 with and without USB related MDM6600 modules:
> >
> > idle lcd off phy-mapphone-mdm6600 ohci-platform
> > 153mW 284mW 344mW
>
> So more than twice the idle power... We really want to get runtime
> PM working :/
Yes and we want' modem to idle too :)
> > +/*
> > + * MDM6600 status codes. These are copied from Motorola Mapphone Linux
> > + * kernel tree. The BB naming here refers to "BaseBand" for modem.
> > + */
>
> Actually your status codes are BP_ (baseband processor) prefixed.
I'll just get rid of the naming and stick to MDM6600 prefixies.
No need to keep the confusing BP/AP prefixes.
> > +static void phy_mdm6600_init_irq(struct phy_mdm6600 *ddata)
> > +{
> > + struct device *dev = ddata->dev;
> > + int i, error, irq;
> > +
> > + for (i = PHY_MDM6600_STATUS0;
> > + i <= PHY_MDM6600_STATUS2; i++) {
> > + if (IS_ERR(ddata->gpio[i]))
> > + continue;
>
> This can be dropped, since the driver errors out of probe
> when there are gpio errors.
True will drop.
> > +static int phy_mdm6600_init_lines(struct phy_mdm6600 *ddata)
> > +{
> > + struct device *dev = ddata->dev;
> > + int i, j, nr_gpio = 0;
> > +
> > + for (i = 0; i < ARRAY_SIZE(phy_mdm6600_line_map); i++) {
> > + const struct phy_mdm6600_map *map =
> > + &phy_mdm6600_line_map[i];
> > +
> > + for (j = 0; j < map->nr_gpios; j++) {
> > + struct gpio_desc **gpio = &ddata->gpio[nr_gpio];
> > +
> > + *gpio = devm_gpiod_get_index(dev,
> > + map->name, j,
> > + map->direction);
> > + if (IS_ERR(*gpio)) {
> > + dev_info(dev,
> > + "gpio %s error %li, already taken?\n",
> > + map->name, PTR_ERR(*gpio));
> > + return PTR_ERR(*gpio);
> > + }
> > + nr_gpio++;
> > + }
>
> I think the code should use the gpiod_get_array() API.
OK thanks will take a look.
> > + /* Give up shared GPIOs now, they will be used for OOB wake */
> > + devm_gpiod_put(ddata->dev, mode_gpio0);
> > + ddata->gpio[PHY_MDM6600_MODE0] = ERR_PTR(-ENODEV);
> > + devm_gpiod_put(ddata->dev, mode_gpio1);
> > + ddata->gpio[PHY_MDM6600_MODE0] = ERR_PTR(-ENODEV);
>
> You want PHY_MDM6600_MODE1 instead. Also I would just use NULL.
> NULL is used by gpiod_get_optional and is handled by the gpiod
> functions, so you don't need to check for gpio errors everywhere.
Oops yup let's just drop this until we know what to do in the
further patches.
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id phy_mdm6600_id_table[] = {
> > + { .compatible = "motorola,mapphone-mdm6600", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, phy_mdm6600_id_table);
> > +#endif
>
> I suggest to just depend on CONFIG_OF in Kconfig and drop the ifdef
> and of_match_ptr() parts. It's very unlikely, that this will be
> used without DT and would need quite some rework anyways.
OK
> > +static int phy_mdm6600_probe(struct platform_device *pdev)
> > +{
> > + struct phy_mdm6600 *ddata;
> > + struct usb_otg *otg;
> > + const struct of_device_id *of_id;
> > + int error;
> > +
> > + of_id = of_match_device(of_match_ptr(phy_mdm6600_id_table),
> > + &pdev->dev);
> > + if (!of_id)
> > + return -EINVAL;
>
> I suggest to drop the of_match_device(). The driver will error
> out anyways when it can't get the gpios.
OK
> Generally I'm a bit worried about handling the mode gpios in two
> different drivers. It looks like it might become a dependency hell.
Yeah you're right. That will require the modules to be loaded
in the right order. It's probably best that we handle the mdm6600
to SoC wake-up in this driver. And then maybe export a function for
toggling the SoC to mdm660 wake to make sure the dependencies are
clear for the modules.
Regards,
Tony
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4
2018-02-17 21:07 [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4 Tony Lindgren
2018-02-18 0:31 ` Sebastian Reichel
@ 2018-02-19 7:45 ` kbuild test robot
2018-03-01 3:47 ` Tony Lindgren
2018-02-19 20:40 ` Rob Herring
2 siblings, 1 reply; 7+ messages in thread
From: kbuild test robot @ 2018-02-19 7:45 UTC (permalink / raw)
To: Tony Lindgren
Cc: kbuild-all, Kishon Vijay Abraham I, linux-kernel, linux-usb,
linux-omap, devicetree, Mark Rutland, Marcel Partap,
Michael Scott, Rob Herring, Sebastian Reichel
Hi Tony,
I love your patch! Perhaps something to improve:
[auto build test WARNING on phy/next]
[also build test WARNING on v4.16-rc2 next-20180216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tony-Lindgren/phy-mapphone-mdm6600-Add-USB-PHY-driver-for-MDM6600-on-Droid-4/20180219-131127
base: https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/phy/motorola/phy-mapphone-mdm6600.c:354:24: sparse: incompatible types for operation (>=)
drivers/phy/motorola/phy-mapphone-mdm6600.c:354:24: left side has type struct gpio_desc
drivers/phy/motorola/phy-mapphone-mdm6600.c:354:24: right side has type int
>> drivers/phy/motorola/phy-mapphone-mdm6600.c:354:24: sparse: incorrect type in conditional
drivers/phy/motorola/phy-mapphone-mdm6600.c:354:24: got bad type
vim +354 drivers/phy/motorola/phy-mapphone-mdm6600.c
340
341 /**
342 * phy_mdm6600_device_power_off() - power off mdm6600 device
343 * @ddata: device driver data
344 */
345 static void phy_mdm6600_device_power_off(struct phy_mdm6600 *ddata)
346 {
347 struct gpio_desc *reset_gpio =
348 ddata->gpio[PHY_MDM6600_RESET];
349
350 ddata->enabled = false;
351 phy_mdm6600_cmd(ddata, AP_STATUS_BP_SHUTDOWN_REQ);
352 msleep(100);
353
> 354 if (reset_gpio >= 0)
355 gpiod_set_value_cansleep(reset_gpio, 1);
356
357 dev_info(ddata->dev, "Waiting for power down request to complete.. ");
358 if (wait_for_completion_timeout(&ddata->ack,
359 msecs_to_jiffies(5000))) {
360 dev_info(ddata->dev, "Powered down OK\n");
361 } else {
362 dev_err(ddata->dev, "Timed out powering down\n");
363 }
364 }
365
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4
2018-02-17 21:07 [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4 Tony Lindgren
2018-02-18 0:31 ` Sebastian Reichel
2018-02-19 7:45 ` kbuild test robot
@ 2018-02-19 20:40 ` Rob Herring
2018-03-01 3:46 ` Tony Lindgren
2 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2018-02-19 20:40 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kishon Vijay Abraham I, linux-kernel, linux-usb, linux-omap,
devicetree, Mark Rutland, Marcel Partap, Michael Scott,
Sebastian Reichel
On Sat, Feb 17, 2018 at 01:07:23PM -0800, Tony Lindgren wrote:
> Let's add support for the GPIO controlled USB PHY on the MDM6600 modem.
> It is used on some Motorola Mapphone series of phones and tablets such
> as Droid 4.
>
> The MDM6600 is hardwired to the first OHCI port in the Droid 4 case, and
> is controlled by several GPIOs. The USB PHY is integrated into the MDM6600
> device it seems. We know this as we get L3 errors from omap-usb-host if
> trying to use the PHY before MDM6600 is configured.
>
> The GPIOs controlling MDM6600 are used to power MDM660 on and off, to
MDM660 a typo?
> configure the USB start-up mode (normal mode versus USB flashing), and
> they also tell the state of the MDM6600 device.
>
> The two start-up mode GPIOs are dual-purposed and used for out of band
> (OOB) wake-up for USB and TS 27.010 serial mux. But we need to configure
> the USB start-up mode first to get MDM6600 booted in the right mode to
> be usable in the first place.
>
> For now, this driver just gives up the two start-up mode GPIOs after the
> modem has been configured to boot in normal mode. One of them we may
> want to configure for USB OOB wake in this driver later on, but that's a
> separate series of patches and needs more work.
>
> Note that the Motorola Mapphone Linux kernel tree has a "radio-ctrl"
> driver for modems. But it really does not control the radio at all, it
> just controls the modem power and start-up mode for USB. So I came to
> the conclusion that we're better off having this done in the USB PHY
> driver. For adding support for USB flashing mode, we can later on add
> a kernel module option for flash_mode=1 or something similar.
>
> Also note that currently there is no PM runtime support for the OHCI
> on omap variant SoCs. So for low(er) power idle states, currenty both
> ohci-platform and phy-mapphone-mdm6600 must be unloaded or unbound.
>
> For reference here is what I measured for total power consumption on
> an idle Droid 4 with and without USB related MDM6600 modules:
>
> idle lcd off phy-mapphone-mdm6600 ohci-platform
> 153mW 284mW 344mW
>
> So it seems that MDM6600 is currently not yet idling even with it's
> radio turned off, but that's something that is beyond the control of
> this USB PHY driver.
>
> Cc: devicetree@vger.kernel.org
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Michael Scott <michael.scott@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> .../bindings/phy/phy-mapphone-mdm6600.txt | 30 ++
> drivers/phy/motorola/Kconfig | 9 +
> drivers/phy/motorola/Makefile | 1 +
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 490 +++++++++++++++++++++
> 4 files changed, 530 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> create mode 100644 drivers/phy/motorola/phy-mapphone-mdm6600.c
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> @@ -0,0 +1,30 @@
> +Device tree binding documentation for Motorola Mapphone MDM6600 USB PHY
> +
> +Required properties:
> +- compatible Must be "motorola,mapphone-mdm6600"
> +- enable-gpios GPIO to enable the USB PHY
> +- power-gpios GPIO to power on the device
> +- reset-gpios GPIO to reset the device
The are pretty standard, but...
> +- mode-gpios Two GPIOs to configure MDM6600 USB start-up mode for
> + normal mode versus USB flashing mode
> +- status-gpios Three GPIOs to read the power state of the MDM6600
> +- cmd-gpios Three GPIOs to control the power state of the MDM6600
These 3 should have vendor a prefix.
> +
> +Example:
> +
> +fsusb1_phy: fsusb1_phy {
usb-phy {
> + compatible = "motorola,mapphone-mdm6600";
> + enable-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>; /* gpio_95 */
> + power-gpios = <&gpio2 22 GPIO_ACTIVE_HIGH>; /* gpio_54 */
> + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; /* gpio_49 */
> + mode-gpios = <&gpio5 20 GPIO_ACTIVE_HIGH>, /* gpio_148 */
> + <&gpio5 21 GPIO_ACTIVE_HIGH>; /* gpio_149 */
> + status-gpios = <&gpio2 23 GPIO_ACTIVE_HIGH>, /* gpio_55 */
> + <&gpio2 21 GPIO_ACTIVE_HIGH>, /* gpio_53 */
> + <&gpio2 20 GPIO_ACTIVE_HIGH>; /* gpio_52 */
> + cmd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>, /* gpio_103 */
> + <&gpio4 8 GPIO_ACTIVE_HIGH>, /* gpio_104 */
> + <&gpio5 14 GPIO_ACTIVE_HIGH>; /* gpio_142 */
> + #phy-cells = <0>;
> +};
> +
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4
2018-02-19 20:40 ` Rob Herring
@ 2018-03-01 3:46 ` Tony Lindgren
0 siblings, 0 replies; 7+ messages in thread
From: Tony Lindgren @ 2018-03-01 3:46 UTC (permalink / raw)
To: Rob Herring
Cc: Kishon Vijay Abraham I, linux-kernel, linux-usb, linux-omap,
devicetree, Mark Rutland, Marcel Partap, Michael Scott,
Sebastian Reichel
* Rob Herring <robh@kernel.org> [180219 20:41]:
> On Sat, Feb 17, 2018 at 01:07:23PM -0800, Tony Lindgren wrote:
> > The GPIOs controlling MDM6600 are used to power MDM660 on and off, to
>
> MDM660 a typo?
Thanks fixing.
> > +Required properties:
> > +- compatible Must be "motorola,mapphone-mdm6600"
> > +- enable-gpios GPIO to enable the USB PHY
> > +- power-gpios GPIO to power on the device
> > +- reset-gpios GPIO to reset the device
>
> The are pretty standard, but...
>
> > +- mode-gpios Two GPIOs to configure MDM6600 USB start-up mode for
> > + normal mode versus USB flashing mode
> > +- status-gpios Three GPIOs to read the power state of the MDM6600
> > +- cmd-gpios Three GPIOs to control the power state of the MDM6600
>
> These 3 should have vendor a prefix.
OK
> > +
> > +Example:
> > +
> > +fsusb1_phy: fsusb1_phy {
>
> usb-phy {
Thanks will send out an updated version shortly.
Regards,
Tony
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4
2018-02-19 7:45 ` kbuild test robot
@ 2018-03-01 3:47 ` Tony Lindgren
0 siblings, 0 replies; 7+ messages in thread
From: Tony Lindgren @ 2018-03-01 3:47 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Kishon Vijay Abraham I, linux-kernel, linux-usb,
linux-omap, devicetree, Mark Rutland, Marcel Partap,
Michael Scott, Rob Herring, Sebastian Reichel
* kbuild test robot <lkp@intel.com> [180219 07:47]:
> > 354 if (reset_gpio >= 0)
> 355 gpiod_set_value_cansleep(reset_gpio, 1);
Thanks this test can be just dropped. Will send out an updated
version shortly.
Regards,
Tony
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-01 3:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-17 21:07 [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4 Tony Lindgren
2018-02-18 0:31 ` Sebastian Reichel
2018-02-18 17:50 ` Tony Lindgren
2018-02-19 7:45 ` kbuild test robot
2018-03-01 3:47 ` Tony Lindgren
2018-02-19 20:40 ` Rob Herring
2018-03-01 3:46 ` Tony Lindgren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).