From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753539AbcCDBjE (ORCPT ); Thu, 3 Mar 2016 20:39:04 -0500 Received: from mga04.intel.com ([192.55.52.120]:23408 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750751AbcCDBjB (ORCPT ); Thu, 3 Mar 2016 20:39:01 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,533,1449561600"; d="scan'208";a="59521043" Subject: Re: [PATCH 5/7] usb: misc: add driver for Intel drcfg controlled port mux To: Greg Kroah-Hartman References: <1456987064-3365-1-git-send-email-baolu.lu@linux.intel.com> <1456987064-3365-6-git-send-email-baolu.lu@linux.intel.com> <20160303161408.GD3743@kroah.com> Cc: Felipe Balbi , Mathias Nyman , Lee Jones , Heikki Krogerus , MyungJoo Ham , Chanwoo Choi , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Wu Hao From: Lu Baolu Message-ID: <56D8E732.8080603@linux.intel.com> Date: Fri, 4 Mar 2016 09:38:58 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160303161408.GD3743@kroah.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> Signed-off-by: Lu Baolu >> Signed-off-by: Wu Hao >> Reviewed-by: Felipe Balbi >> --- >> 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 >> 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 >> + * Author: Lu Baolu >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> + >> +#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? > >> + >> + 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. Thanks, -Baolu