Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer driver

From: sathyanarayanan kuppuswamy
Date: Tue May 30 2017 - 13:51:14 EST

Hi Peter,

Thanks for your comments.

On 05/30/2017 06:40 AM, Peter Rosin wrote:
On 2017-05-30 02:47, sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx wrote:
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>

In some Intel SOCs, a single USB port is shared between USB device and
host controller and an internal mux is used to control the selection of
port by host/device controllers. This driver adds support for the USB
internal mux, and all consumers of this mux can use interfaces provided
by mux subsytem to control the state of the internal mux.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
Hi!

A few things make me curious...

1. How are platform devices w/o any match table probed? Do you have to
manually load them, or what?
Yes, for now, I am manually creating this device from dwc3 driver to test it. But in future I am planning to get ACPI ID for this device to probe it from BIOS.

2. What are these "all the consumers of this mux" that you mention,
Currently the only consumer for this mux device is, Broxton USB PHY driver. Its not yet upstreamed. I hoping to get this driver merged first before submitting the other driver for review.
and how
do they find the correct mux control to interact with?
Your current mux_control_get() API has tight dependency on device tree node and hence we can't use it for this use case.

So I am planning to add a new API to get the mux-control based on mux-device name. API interface looks some thing like below. I haven't finalized the patch yet. I will send it you for review in next few days. Let me know if you agree with this idea.

struct mux_chip *devm_mux_chip_get_by_index(struct device *dev,
const char *parent_name, unsigned int index)

struct mux_control *devm_mux_control_get_by_index(struct device *dev,
struct mux_chip *mux_chip, unsigned int index)

---
drivers/mux/Kconfig | 13 ++++
drivers/mux/Makefile | 1 +
drivers/mux/mux-intel-usb.c | 132 ++++++++++++++++++++++++++++++++++++++
include/linux/mux/mux-intel-usb.h | 22 +++++++
4 files changed, 168 insertions(+)
create mode 100644 drivers/mux/mux-intel-usb.c
create mode 100644 include/linux/mux/mux-intel-usb.h

diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index e8f1df7..0e3af09 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -56,4 +56,17 @@ config MUX_MMIO
To compile the driver as a module, choose M here: the module will
be called mux-mmio.
+config MUX_INTEL_USB
+ tristate "Intel USB Mux"
+ depends on USB
Why depend on USB?
This device register mapping comes from DWC3 USB host controller. So I thought there is no point in enabling it, if USB is disabled.

+ help
+ In some Intel SOCs, a single USB port is shared between USB
+ device and host controller and an internal mux is used to
+ control the selection of port by host/device controllers. This
+ driver adds support for the USB internal mux, and all consumers
+ of this mux can use interfaces provided by mux subsytem to control
+ the state of the internal mux.
+
+ To compile the driver as a module, choose M here.
+
endif
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index 6bac5b0..9154616 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_MULTIPLEXER) += mux-core.o
obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
+obj-$(CONFIG_MUX_INTEL_USB) += mux-intel-usb.o
Alphabetical order, please.
I will fix it in next version.

diff --git a/drivers/mux/mux-intel-usb.c b/drivers/mux/mux-intel-usb.c
new file mode 100644
index 0000000..587e9bb
--- /dev/null
+++ b/drivers/mux/mux-intel-usb.c
@@ -0,0 +1,132 @@
+/*
+ * Intel USB Multiplexer Driver
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Author: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxx>
+ *
+ * This driver is written based on extcon-intel-usb driver submitted by
+ * Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
+ *
+ * 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/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/mux/driver.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/mux/mux-intel-usb.h>
+#include <linux/io.h>
Alphabetical order, please.
ok. I will fix it in next version.

+
+#define INTEL_MUX_CFG0 0x00
+#define INTEL_MUX_CFG1 0x04
+
+#define CFG0_SW_DRD_MODE_MASK 0x3
+#define CFG0_SW_DRD_DYN 0
+#define CFG0_SW_DRD_STATIC_HOST 1
+#define CFG0_SW_DRD_STATIC_DEV 2
+#define CFG0_SW_SYNC_SS_AND_HS BIT(2)
+#define CFG0_SW_SWITCH_EN BIT(16)
+#define CFG0_SW_IDPIN BIT(20)
+#define CFG0_SW_IDPIN_EN BIT(21)
+#define CFG0_SW_VBUS_VALID BIT(24)
+
+#define CFG1_MODE BIT(29)
+
+struct mux_intel_usb {
+ struct device *dev;
+ void __iomem *regs;
+};
+
+static int mux_intel_usb_set(struct mux_control *mux_ctrl, int state)
+{
+ struct mux_intel_usb *mux = mux_chip_priv(mux_ctrl->chip);
+ u32 val;
+
+ dev_info(mux->dev, "Intel USB mux set called");
Isn't this very spammy?

Cheers,
peda
yes. Added it for debug/testing purpose. Will remove it.

+
+ switch (state) {
+ case INTEL_USB_MUX_STATE_HOST:
+ val = CFG0_SW_IDPIN_EN | CFG0_SW_DRD_STATIC_HOST;
+ break;
+ case INTEL_USB_MUX_STATE_DEVICE:
+ val = CFG0_SW_IDPIN_EN | CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID |
+ CFG0_SW_DRD_STATIC_DEV;
+ break;
+ default:
+ return 0;
+ };
+
+ writel(val, mux->regs);
+
+ return 0;
+}
+
+static const struct mux_control_ops mux_intel_usb_ops = {
+ .set = mux_intel_usb_set,
+};
+
+static int mux_intel_usb_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mux_chip *mux_chip;
+ struct mux_intel_usb *mux;
+ struct resource *res;
+ int ret;
+
+ mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
+ if (IS_ERR(mux_chip))
+ return PTR_ERR(mux_chip);
+
+ mux = mux_chip_priv(mux_chip);
+ mux->dev = dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "Failed to get mem resource\n");
+ return -ENXIO;
+ }
+
+ mux->regs = devm_ioremap_nocache(dev, res->start, resource_size(res));
+ if (!mux->regs) {
+ dev_err(mux->dev, "mux regs io remap failed\n");
+ return -ENOMEM;
+ }
+
+ mux_chip->ops = &mux_intel_usb_ops;
+ mux_chip->mux->states = INTEL_USB_MUX_STATE_MAX;
+ mux_chip->mux->idle_state = MUX_IDLE_AS_IS;
+
+ ret = devm_mux_chip_register(dev, mux_chip);
+ if (ret < 0)
+ return ret;
+
+ dev_info(dev, "%u-way mux-controller registered\n",
+ mux_chip->mux->states);
+
+ return 0;
+}
+
+static const struct platform_device_id mux_intel_usb_id[] = {
+ { .name = "intel-usb-mux", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, mux_intel_usb_id);
+
+static struct platform_driver mux_intel_usb_driver = {
+ .driver = {
+ .name = "intel-usb-mux",
+ },
+ .probe = mux_intel_usb_probe,
+ .id_table = mux_intel_usb_id,
+};
+module_platform_driver(mux_intel_usb_driver);
+
+MODULE_DESCRIPTION("Intel USB multiplexer driver");
+MODULE_AUTHOR("Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxx>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mux/mux-intel-usb.h b/include/linux/mux/mux-intel-usb.h
new file mode 100644
index 0000000..e77516c
--- /dev/null
+++ b/include/linux/mux/mux-intel-usb.h
@@ -0,0 +1,22 @@
+/*
+ * Intel USB multiplexer header file
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Author: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxx>
+ *
+ * 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.
+ */
+
+#ifndef _LINUX_MUX_MUX_INTEL_USB_H
+#define _LINUX_MUX_MUX_INTEL_USB_H
+
+#include <linux/mux/consumer.h>
+
+#define INTEL_USB_MUX_STATE_HOST 0
+#define INTEL_USB_MUX_STATE_DEVICE 1
+#define INTEL_USB_MUX_STATE_MAX 2
+
+#endif /* _LINUX_MUX_MUX_INTEL_USB_H */



--
Sathyanarayanan Kuppuswamy
Linux kernel developer