| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <fab385b1-025d-eca2-7662-aa4a41a89093@linux.intel.com>
Date: Tue, 30 May 2017 10:47:48 -0700
From: sathyanarayanan kuppuswamy
<sathyanarayanan.kuppuswamy@...ux.intel.com>
To: Peter Rosin <peda@...ntia.se>, heikki.krogerus@...ux.intel.com
Cc: linux-kernel@...r.kernel.org, sathyaosid@...il.com
Subject: Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer
driver
Hi Peter,
Thanks for your comments.
On 05/30/2017 06:40 AM, Peter Rosin wrote:
> On 2017-05-30 02:47, sathyanarayanan.kuppuswamy@...ux.intel.com wrote:
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.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@...ux.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@...el.com>
>> + *
>> + * This driver is written based on extcon-intel-usb driver submitted by
>> + * Heikki Krogerus <heikki.krogerus@...ux.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@...el.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@...el.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
Powered by blists - more mailing lists