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.kroge...@linux.intel.com>
> >> Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
> >> Signed-off-by: Wu Hao <hao...@intel.com>
> >> Reviewed-by: Felipe Balbi <ba...@kernel.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 <zait...@redhat.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.kroge...@linux.intel.com>
> >> + * Author: Lu Baolu <baolu...@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/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...

thanks,

greg k-h

Reply via email to