Messages in this thread | | | Subject | Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer driver | From | sathyanarayanan kuppuswamy <> | Date | Tue, 30 May 2017 10:47:48 -0700 |
| |
Hi Peter,
Thanks for your comments.
On 05/30/2017 06:40 AM, Peter Rosin wrote: > On 2017-05-30 02:47, sathyanarayanan.kuppuswamy@linux.intel.com wrote: >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> >> 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@linux.intel.com> > 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@intel.com> >> + * >> + * This driver is written based on extcon-intel-usb driver submitted by >> + * 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/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@intel.com>"); >> +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@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. >> + */ >> + >> +#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
| |