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

[Peter Rosin]

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?

2. What are these "all the consumers of this mux" that you mention, and how
   do they find the correct mux control to interact with?

> ---
>  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?

> +	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.

> 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.

> +
> +#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

> +
> +	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 */
>