| 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: <56D8EBCC.8090505@linux.intel.com>
Date: Fri, 4 Mar 2016 09:58:36 +0800
From: Lu Baolu <baolu.lu@...ux.intel.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Felipe Balbi <balbi@...nel.org>,
Mathias Nyman <mathias.nyman@...el.com>,
Lee Jones <lee.jones@...aro.org>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
MyungJoo Ham <myungjoo.ham@...sung.com>,
Chanwoo Choi <cw00.choi@...sung.com>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
Wu Hao <hao.wu@...el.com>
Subject: Re: [PATCH 5/7] usb: misc: add driver for Intel drcfg controlled port
mux
On 03/04/2016 09:44 AM, Greg Kroah-Hartman wrote:
> On Fri, Mar 04, 2016 at 09:38:58AM +0800, Lu Baolu wrote:
>>
>> On 03/04/2016 12:14 AM, Greg Kroah-Hartman wrote:
>>> On Thu, Mar 03, 2016 at 02:37:42PM +0800, Lu Baolu wrote:
>>>> Several Intel PCHs and SOCs have an internal mux that is used to
>>>> share one USB port between device controller and host controller.
>>>> The mux is handled through the Dual Role Configuration Register.
>>>>
>>>> Signed-off-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
>>>> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
>>>> Signed-off-by: Wu Hao <hao.wu@...el.com>
>>>> Reviewed-by: Felipe Balbi <balbi@...nel.org>
>>>> ---
>>>> MAINTAINERS | 1 +
>>>> drivers/usb/misc/Kconfig | 7 ++
>>>> drivers/usb/misc/Makefile | 1 +
>>>> drivers/usb/misc/intel-mux-drcfg.c | 174 +++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 183 insertions(+)
>>>> create mode 100644 drivers/usb/misc/intel-mux-drcfg.c
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 0f321e4..20eb873 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -11396,6 +11396,7 @@ S: Supported
>>>> F: drivers/usb/misc/mux.c
>>>> F: include/linux/usb/mux.h
>>>> F: drivers/usb/misc/intel-mux-gpio.c
>>>> +F: drivers/usb/misc/intel-mux-drcfg.c
>>>>
>>>> USB PRINTER DRIVER (usblp)
>>>> M: Pete Zaitcev <zaitcev@...hat.com>
>>>> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
>>>> index 33e6386..befd910 100644
>>>> --- a/drivers/usb/misc/Kconfig
>>>> +++ b/drivers/usb/misc/Kconfig
>>>> @@ -281,3 +281,10 @@ config INTEL_MUX_GPIO
>>>> help
>>>> Say Y here to enable support for Intel dual role port mux
>>>> controlled by GPIOs.
>>>> +
>>>> +config INTEL_MUX_DRCFG
>>>> + tristate "Intel dual role port mux controlled by register"
>>>> + select USB_MUX
>>>> + help
>>>> + Say Y here to enable support for Intel dual role port mux
>>>> + controlled by the Dual Role Configuration Registers.
>>>> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
>>>> index da4fb4e..c4d19a0 100644
>>>> --- a/drivers/usb/misc/Makefile
>>>> +++ b/drivers/usb/misc/Makefile
>>>> @@ -32,3 +32,4 @@ obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o
>>>>
>>>> obj-$(CONFIG_USB_MUX) += mux.o
>>>> obj-$(CONFIG_INTEL_MUX_GPIO) += intel-mux-gpio.o
>>>> +obj-$(CONFIG_INTEL_MUX_DRCFG) += intel-mux-drcfg.o
>>>> diff --git a/drivers/usb/misc/intel-mux-drcfg.c b/drivers/usb/misc/intel-mux-drcfg.c
>>>> new file mode 100644
>>>> index 0000000..29081c5
>>>> --- /dev/null
>>>> +++ b/drivers/usb/misc/intel-mux-drcfg.c
>>>> @@ -0,0 +1,174 @@
>>>> +/**
>>>> + * intel-mux-drcfg.c - Driver for Intel USB mux via register
>>>> + *
>>>> + * Copyright (C) 2016 Intel Corporation
>>>> + * Author: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
>>>> + * Author: Lu Baolu <baolu.lu@...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/slab.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/property.h>
>>>> +#include <linux/usb/mux.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> +#define INTEL_MUX_CFG0 0x00
>>>> +#define INTEL_MUX_CFG1 0x04
>>>> +#define CFG0_SW_IDPIN BIT(20)
>>>> +#define CFG0_SW_IDPIN_EN BIT(21)
>>>> +#define CFG0_SW_VBUS_VALID BIT(24)
>>>> +#define CFG1_SW_MODE BIT(29)
>>>> +#define CFG1_POLL_TIMEOUT 1000
>>>> +
>>>> +struct intel_usb_mux {
>>>> + struct usb_mux_dev umdev;
>>>> + void __iomem *regs;
>>>> + u32 cfg0_ctx;
>>>> +};
>>>> +
>>>> +static inline int intel_mux_drcfg_switch(struct usb_mux_dev *umdev, bool host)
>>>> +{
>>>> + struct intel_usb_mux *mux;
>>>> + unsigned long timeout;
>>>> + u32 data;
>>>> +
>>>> + mux = container_of(umdev, struct intel_usb_mux, umdev);
>>>> +
>>>> + /* Check and set mux to SW controlled mode */
>>>> + data = readl(mux->regs + INTEL_MUX_CFG0);
>>>> + if (!(data & CFG0_SW_IDPIN_EN)) {
>>>> + data |= CFG0_SW_IDPIN_EN;
>>>> + writel(data, mux->regs + INTEL_MUX_CFG0);
>>>> + }
>>>> +
>>>> + /*
>>>> + * Configure CFG0 to switch the mux and VBUS_VALID bit is
>>>> + * required for device mode.
>>>> + */
>>>> + data = readl(mux->regs + INTEL_MUX_CFG0);
>>>> + if (host)
>>>> + data &= ~(CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
>>>> + else
>>>> + data |= (CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
>>>> + writel(data, mux->regs + INTEL_MUX_CFG0);
>>>> +
>>>> + /*
>>>> + * Polling CFG1 for safety, most case it takes about 600ms
>>>> + * to finish mode switching, set TIMEOUT long enough.
>>>> + */
>>>> + timeout = jiffies + msecs_to_jiffies(CFG1_POLL_TIMEOUT);
>>>> +
>>>> + /* Polling on CFG1 register to confirm mode switch. */
>>>> + while (!time_after(jiffies, timeout)) {
>>>> + data = readl(mux->regs + INTEL_MUX_CFG1);
>>>> + if (!(host ^ (data & CFG1_SW_MODE)))
>>>> + return 0;
>>>> + /* interval for polling is set to about 5ms */
>>>> + usleep_range(5000, 5100);
>>>> + }
>>>> +
>>>> + return -ETIMEDOUT;
>>>> +}
>>>> +
>>>> +static int intel_mux_drcfg_cable_set(struct usb_mux_dev *umdev)
>>>> +{
>>>> + dev_dbg(umdev->dev, "drcfg mux switch to HOST\n");
>>>> +
>>>> + return intel_mux_drcfg_switch(umdev, true);
>>>> +}
>>>> +
>>>> +static int intel_mux_drcfg_cable_unset(struct usb_mux_dev *umdev)
>>>> +{
>>>> + dev_dbg(umdev->dev, "drcfg mux switch to DEVICE\n");
>>>> +
>>>> + return intel_mux_drcfg_switch(umdev, false);
>>>> +}
>>>> +
>>>> +static int intel_mux_drcfg_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct intel_usb_mux *mux;
>>>> + struct usb_mux_dev *umdev;
>>>> + struct device *dev = &pdev->dev;
>>>> + u64 start, size;
>>>> + int ret;
>>>> +
>>>> + mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
>>>> + if (!mux)
>>>> + return -ENOMEM;
>>>> +
>>>> + ret = device_property_read_u64(dev, "reg-start", &start);
>>>> + ret |= device_property_read_u64(dev, "reg-size", &size);
>>>> + if (ret)
>>>> + return -ENODEV;
>>> And you leaked memory :(
>> Did you mean device_property_read_*() allocated memory
>> and I need to release it before return error?
>>
>>>> +
>>>> + mux->regs = devm_ioremap_nocache(dev, start, size);
>>>> + if (!mux->regs)
>>>> + return -ENOMEM;
>>> Memory gone :(
>> devm interface will manage the resources by itself, or not?
> Ugh, you are right, nevermind, these both are fine, sorry for the noise.
>
>>>> + mux->cfg0_ctx = readl(mux->regs + INTEL_MUX_CFG0);
>>>> +
>>>> + umdev = &mux->umdev;
>>>> + umdev->dev = dev;
>>>> + umdev->cable_name = "USB-HOST";
>>>> + umdev->cable_set_cb = intel_mux_drcfg_cable_set;
>>>> + umdev->cable_unset_cb = intel_mux_drcfg_cable_unset;
>>>> +
>>>> + ret = usb_mux_register(umdev);
>>>> + if (ret)
>>>> + writel(mux->cfg0_ctx, mux->regs + INTEL_MUX_CFG0);
>>> So if an error happens you just keep going?
>> No. If error happens, I will restore the register and return error.
> That seems a bit "odd", and will change when you fix up the api...
Sure.
>
> thanks,
>
> greg k-h
>
Thank you for your time.
Best Regards,
baolu
Powered by blists - more mailing lists