From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932740AbbLCJl3 (ORCPT ); Thu, 3 Dec 2015 04:41:29 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:34352 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932655AbbLCJlY (ORCPT ); Thu, 3 Dec 2015 04:41:24 -0500 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfee68d-f79646d000001355-48-56600e3e5328 Content-transfer-encoding: 8BIT Message-id: <56600E3E.6080300@samsung.com> Date: Thu, 03 Dec 2015 18:41:18 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Heikki Krogerus , Greg Kroah-Hartman Cc: MyungJoo Ham , David Cohen , Lu Baolu , Mathias Nyman , Felipe Balbi , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux References: <1449134991-39095-1-git-send-email-heikki.krogerus@linux.intel.com> <1449134991-39095-2-git-send-email-heikki.krogerus@linux.intel.com> In-reply-to: <1449134991-39095-2-git-send-email-heikki.krogerus@linux.intel.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrBIsWRmVeSWpSXmKPExsWyRsSkQNeOLyHMYPY8Y4uD9+stNk/cymbx 8/J9dovmxevZLLpW72SxuLxrDpvFomWtzBavPzSxWNxuXMHmwOkx72Sgx/65a9g9+rasYvQ4 fmM7k8fnTXIBrFFcNimpOZllqUX6dglcGaunNzAXzDKt6G/uY2tg7NbuYuTkkBAwkdjVO4kN whaTuHBvPZgtJLCCUWLBc7suRg6wmpureLoYuYDCsxglLr0+wQhSwysgKPFj8j0WkBpmAXmJ I5eyQcLMAuoSk+YtYoaof8AocesnxHxeAS2Jlv/LmUFsFgFVid+PlrKA2GxA8f0vbrCBzBEV iJDoPlEJEhYRyJA4eOMeC8TMLiaJK42CILawgL3E/mU3WCDmT2WUeL39JitIglMgQOLek5Ws IAkJgUfsEocv7WaDWCYg8W3yIRaIZ2QlNh1ghvhXUuLgihssExjFZiF5ZxbCO7OQvLOAkXkV o2hqQXJBcVJ6kaFecWJucWleul5yfu4mRmAEnv73rHcH4+0D1ocYBTgYlXh4BTzjw4RYE8uK K3MPMZoCHTGRWUo0OR8Y53kl8YbGZkYWpiamxkbmlmZK4ryKUj+DhQTSE0tSs1NTC1KL4otK c1KLDzEycXBKNTAmryrtmW2f/K6BzW6d/eGtzt4vm5LPm9yrZgx92NmSdtdV8uqkXZmSUW+Z dm2zspuoaumx/MhUa+8dog9l56mblIasuWDGP++MgdasDbPPzNLbfNLs3R3uA823bpc7JJxu 1I0/nR/0zJqvicvrkOG+dAYjxSyP+bYZU81fOdxbK5k4+W2UwlYlluKMREMt5qLiRABIgBC7 uwIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrGIsWRmVeSWpSXmKPExsVy+t9jQV07voQwgxuXdSwO3q+32DxxK5vF z8v32S2aF69ns+havZPF4vKuOWwWi5a1Mlu8/tDEYnG7cQWbA6fHvJOBHvvnrmH36NuyitHj +I3tTB6fN8kFsEY1MNpkpCampBYppOYl56dk5qXbKnkHxzvHm5oZGOoaWlqYKynkJeam2iq5 +AToumXmAN2jpFCWmFMKFApILC5W0rfDNCE0xE3XAqYxQtc3JAiux8gADSSsYcxYPb2BuWCW aUV/cx9bA2O3dhcjB4eEgInEzVU8XYycQKaYxIV769m6GLk4hARmMUpcen2CESTBKyAo8WPy PRaQemYBeYkjl7JBwswC6hKT5i1ihqh/wChx6+ckNoh6LYmW/8uZQWwWAVWJ34+WsoDYbEDx /S9usIHMERWIkOg+UQkSFhHIkDh44x4LxMwuJokrjYIgtrCAvcT+ZTdYIOZPZZR4vf0mK0iC UyBA4t6TlawTGIGuRDhvFsJ5s5Cct4CReRWjRGpBckFxUnquYV5quV5xYm5xaV66XnJ+7iZG cJw/k9rBeHCX+yFGAQ5GJR5eAc/4MCHWxLLiytxDjBIczEoivN/WAoV4UxIrq1KL8uOLSnNS iw8xmgL9N5FZSjQ5H5iC8kriDY1NzIwsjcwNLYyMzZXEeWsvRYYJCaQnlqRmp6YWpBbB9DFx cEo1MPJq/lGTOj13xVv3Wfavzy8Ts/bXmPhiwhtOnX1yv+ebX/KOO9TYb37RQPzC4lMrv6db Ptbx+/CmdqJSd+8X69xuO+X1D9sPreWY/UP+9e3IpifdDJGzTX7MipN/xDnx6se5Nk/ktC44 KHxryte0jjljqZj3dYfU+vcCG8PuxNXM+e82d5vaTjclluKMREMt5qLiRAAvjqsfCQMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Heikki, I'm sorry for delay reply. On 2015년 12월 03일 18:29, Heikki Krogerus wrote: > Several Intel PCHs and SOCs have an internal mux that is > used to share one USB port between USB Device Controller and > xHCI. The mux is normally handled by System FW/BIOS, but not > always. For those platforms where the FW does not take care > of the mux, this driver is needed. > > Signed-off-by: Heikki Krogerus > --- > drivers/extcon/Kconfig | 5 ++ > drivers/extcon/Makefile | 1 + > drivers/extcon/extcon-intel-usb.c | 118 +++++++++++++++++++++++++++++++++++ > include/linux/extcon/intel_usb_mux.h | 31 +++++++++ > 4 files changed, 155 insertions(+) > create mode 100644 drivers/extcon/extcon-intel-usb.c > create mode 100644 include/linux/extcon/intel_usb_mux.h > > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig > index 0cebbf6..0a7ccb1 100644 > --- a/drivers/extcon/Kconfig > +++ b/drivers/extcon/Kconfig > @@ -118,3 +118,8 @@ config EXTCON_USB_GPIO > Used typically if GPIO is used for USB ID pin detection. > > endif > + > +config EXTCON_INTEL_USB > + bool > + depends on X86 && USB > + select EXTCON > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile > index ba787d0..e6e031a 100644 > --- a/drivers/extcon/Makefile > +++ b/drivers/extcon/Makefile > @@ -15,3 +15,4 @@ obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o > obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o > obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o > obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o > +obj-$(CONFIG_EXTCON_INTEL_USB) += extcon-intel-usb.o > diff --git a/drivers/extcon/extcon-intel-usb.c b/drivers/extcon/extcon-intel-usb.c > new file mode 100644 > index 0000000..3da6039 > --- /dev/null > +++ b/drivers/extcon/extcon-intel-usb.c > @@ -0,0 +1,118 @@ > +/** > + * extcon-intel-usb.c - Driver for Intel USB mux > + * > + * Copyright (C) 2015 Intel Corporation > + * Author: Heikki Krogerus > + * > + * 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 > + > +#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 intel_usb_mux { > + struct notifier_block nb; > + struct extcon_dev edev; > + void __iomem *regs; > + u32 cfg0_ctx; > +}; > + > +static const int intel_mux_cable[] = { > + EXTCON_USB_HOST, > + EXTCON_NONE, > +}; > + > +static int intel_usb_mux_notifier(struct notifier_block *nb, > + unsigned long old, void *ptr) > +{ > + struct intel_usb_mux *mux = container_of(nb, struct intel_usb_mux, nb); > + u32 val; > + > + if (mux->edev.state) > + val = CFG0_SW_IDPIN_EN | CFG0_SW_DRD_STATIC_HOST; > + else > + val = CFG0_SW_IDPIN_EN | CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID | > + CFG0_SW_DRD_STATIC_DEV; > + > + writel(val, mux->regs); > + return NOTIFY_OK; > +} > + > +struct intel_usb_mux *intel_usb_mux_register(struct device *dev, > + struct resource *r) > +{ > + struct intel_usb_mux *mux; > + int ret; > + > + mux = kzalloc(sizeof(*mux), GFP_KERNEL); > + if (!mux) > + return ERR_PTR(-ENOMEM); > + > + mux->regs = ioremap_nocache(r->start, resource_size(r)); > + if (!mux->regs) { > + kfree(mux); > + return ERR_PTR(-ENOMEM); > + } > + > + mux->cfg0_ctx = readl(mux->regs + INTEL_MUX_CFG0); > + > + mux->edev.dev.parent = dev; > + mux->edev.supported_cable = intel_mux_cable; > + > + ret = extcon_dev_register(&mux->edev); > + if (ret) > + goto err; > + > + mux->edev.name = "intel_usb_mux"; > + mux->edev.state = !!(readl(mux->regs + INTEL_MUX_CFG1) & CFG1_MODE); > + > + /* An external source needs to tell us what to do */ > + mux->nb.notifier_call = intel_usb_mux_notifier; > + ret = extcon_register_notifier(&mux->edev, EXTCON_USB_HOST, &mux->nb); > + if (ret) { > + dev_err(&mux->edev.dev, "failed to register notifier\n"); > + extcon_dev_unregister(&mux->edev); > + goto err; > + } > + return mux; > +err: > + iounmap(mux->regs); > + kfree(mux); > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(intel_usb_mux_register); > + > +void intel_usb_mux_unregister(struct intel_usb_mux *mux) > +{ > + if (!mux) > + return; > + > + if (WARN_ON(IS_ERR_OR_NULL(extcon_get_extcon_dev(mux->edev.name)))) > + return; > + > + extcon_unregister_notifier(&mux->edev, EXTCON_USB_HOST, &mux->nb); > + extcon_dev_unregister(&mux->edev); > + writel(mux->cfg0_ctx, mux->regs + INTEL_MUX_CFG0); > + iounmap(mux->regs); > + kfree(mux); > +} > +EXPORT_SYMBOL_GPL(intel_usb_mux_unregister); I do never want to add some specific funtcion for only this driver. I think is not appropriate way. - intel_usb_mux_unregister - intel_usb_mux_register The client driver using extcon driver should use the standard extcon API for code consistency. Also, I'll do the more detailed review for this patch. > diff --git a/include/linux/extcon/intel_usb_mux.h b/include/linux/extcon/intel_usb_mux.h > new file mode 100644 > index 0000000..f18ce52 > --- /dev/null > +++ b/include/linux/extcon/intel_usb_mux.h > @@ -0,0 +1,31 @@ > +/* > + * Driver for Intel USB mux > + * > + * Copyright (C) 2015 Intel Corporation > + * > + * 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 _INTEL_USB_MUX_H > +#define _INTEL_USB_MUX_H > + > +#include > + > +struct intel_usb_mux; > + > +#ifdef CONFIG_EXTCON_INTEL_USB > +struct intel_usb_mux *intel_usb_mux_register(struct device *dev, > + struct resource *r); > +void intel_usb_mux_unregister(struct intel_usb_mux *mux); > +#else > +static inline struct intel_usb_mux *intel_usb_mux_register(struct device *dev, > + struct resource *r) > +{ > + return ERR_PTR(-ENOTSUPP); > +} > +static inline void intel_usb_mux_unregister(struct intel_usb_mux *mux) { } ditto. > +#endif /* CONFIG_EXTCON_INTEL_USB */ > + > +#endif /* _INTEL_USB_MUX_H */ > Thanks, Chanwoo Choi