Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1282841 > unrolled thread
| Started by | Heikki Krogerus <heikki.krogerus@linux.intel.com> |
|---|---|
| First post | 2015-12-03 10:40 +0100 |
| Last post | 2015-12-03 20:10 +0100 |
| Articles | 11 — 3 participants |
Back to article view | Back to linux.kernel
[PATCHv2 0/2] extcon: driver for Intel USB MUX Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2015-12-03 10:40 +0100
[PATCHv2 1/2] extcon: add driver for Intel USB mux Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2015-12-03 10:40 +0100
Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux Chanwoo Choi <cw00.choi@samsung.com> - 2015-12-03 10:50 +0100
Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2015-12-04 10:00 +0100
Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux Chanwoo Choi <cw00.choi@samsung.com> - 2015-12-07 02:30 +0100
Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2015-12-07 14:00 +0100
Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux Chanwoo Choi <cw00.choi@samsung.com> - 2015-12-08 02:20 +0100
Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2015-12-08 13:30 +0100
Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-03 20:20 +0100
[PATCHv2 2/2] usb: pci-quirks: register USB mux found on Cherrytrail SOC Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2015-12-03 10:40 +0100
Re: [PATCHv2 2/2] usb: pci-quirks: register USB mux found on Cherrytrail SOC Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-03 20:10 +0100
| From | Heikki Krogerus <heikki.krogerus@linux.intel.com> |
|---|---|
| Date | 2015-12-03 10:40 +0100 |
| Subject | [PATCHv2 0/2] extcon: driver for Intel USB MUX |
| Message-ID | <qBywN-38I-9@gated-at.bofh.it> |
Changes since v1: - Using xhci_find_next_ext_cap() as suggested by Baolu - Protection agains unbalanced uregistering as suggested by David Cohen Hi, This is a driver for an internal mux which is available on most modern Intel platforms that shares an USB port between USB Device Controller and xHCI. Normally BIOS or ACPI take care of it, but on some platforms that is not possible, and the OS has to control it. When the mux needs to be handled by OS, there is always an external component that detects connection changes in the port behind the mux, for example PMIC. The driver for that component needs to notify this driver. The mux itself has no means to detect connection changes on the port. User selectable kconfig option is deliberately left out. The driver for the mux needs to be always selected by the drivers for the component that can notify the mux driver about connection changes. The only platforms the need the OS to be in control of the mux so far are Cherrytrail based, and on Cherrytrail SOC the mux control registers are mapped to xHCI MMIO. The second patch will register an instance of the mux from pci_quirks.c if the mux is detected and if the driver has been loaded. The mux control registers are defined in Cherrytrail datasheets [1] page 2230-2234. I think these should go via extcon tree, but it's up to you guys of course. [1] http://www.intel.es/content/www/es/es/processors/atom/atom-z8000-datasheet-vol-2.html Heikki Krogerus (2): extcon: add driver for Intel USB mux usb: pci-quirks: register USB mux found on Cherrytrail SOC drivers/extcon/Kconfig | 5 ++ drivers/extcon/Makefile | 1 + drivers/extcon/extcon-intel-usb.c | 118 +++++++++++++++++++++++++++++++++++ drivers/usb/host/pci-quirks.c | 26 +++++++- include/linux/extcon/intel_usb_mux.h | 31 +++++++++ 5 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 drivers/extcon/extcon-intel-usb.c create mode 100644 include/linux/extcon/intel_usb_mux.h -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Heikki Krogerus <heikki.krogerus@linux.intel.com> |
|---|---|
| Date | 2015-12-03 10:40 +0100 |
| Subject | [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qBywN-38I-7@gated-at.bofh.it> |
| In reply to | #1282841 |
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 <heikki.krogerus@linux.intel.com>
---
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 <heikki.krogerus@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/extcon.h>
+
+#include <linux/extcon/intel_usb_mux.h>
+
+#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);
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 <linux/errno.h>
+
+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) { }
+#endif /* CONFIG_EXTCON_INTEL_USB */
+
+#endif /* _INTEL_USB_MUX_H */
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2015-12-03 10:50 +0100 |
| Subject | Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qByGu-3cm-23@gated-at.bofh.it> |
| In reply to | #1282842 |
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 <heikki.krogerus@linux.intel.com>
> ---
> 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 <heikki.krogerus@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/extcon.h>
> +
> +#include <linux/extcon/intel_usb_mux.h>
> +
> +#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 <linux/errno.h>
> +
> +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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Heikki Krogerus <heikki.krogerus@linux.intel.com> |
|---|---|
| Date | 2015-12-04 10:00 +0100 |
| Subject | Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qBUnF-nU-15@gated-at.bofh.it> |
| In reply to | #1282861 |
Hi, > 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. The internal mux we are controlling here is physically separate device. Ideally we could populate child device for it, but since that is not possible because of the resource conflict, we use the library approach, which is really not that uncommon. I don't think I agree with your point even at general level. The control required to handle this mux, even though simple, is enough to deserve to be separated from xHCI code. xHCI should not need to care about anything else expect does it have the mux, i.e. does it need to register it or not. It should not need to care about how it needs to be controlled or even what it is. We may decide to create something else out of it instead of an extcon device later. But in any case, the mux is available on all new Intel platforms, but it needs to be controlled by OS only in few "special" cases. We can not force xHCI (or pci-quirks.c to be more precise) to be aware of these "special" cases. The only way to make it work like that would bet by using ifdefs, and we really really don't want that. And please also note that, though for now we only expect the mux control registers to be part of xHCI MMIO, that is not always the case. The control registers are part of the device controller MMIO on some platforms. We do not want to duplicate the whole control of the mux if/when we need the OS to be in control of it on a platform that has those control registers mapped somewhere else then xHCI MMIO, So I would say that we have pretty good justification for separating the mux control, which means unfortunately custom API in this case. But if you would prefer that we put the files somewhere else then drivers/extcon/ and include/linux/extcon/ I'm fine with that. If you like, we can put it to drivers/usb/host/ as that is where pci-quirks.c is. That way I think we can also put the header to include/usb/. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2015-12-07 02:30 +0100 |
| Subject | Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qCSMO-6cJ-21@gated-at.bofh.it> |
| In reply to | #1283657 |
Hi, On 2015년 12월 04일 17:51, Heikki Krogerus wrote: > Hi, > >> 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. > > The internal mux we are controlling here is physically separate > device. Ideally we could populate child device for it, but since that > is not possible because of the resource conflict, we use the library > approach, which is really not that uncommon. New added functions for only specific device driver is not library. The all device drivers which is included in some framework should connect to the other device driver through only framework API as following: -------------------- ---------------- | EXTCON framework |<-------->| USB framework | -------------------- ----------------- | | | | extcon-intel-usb.c pci-quirks.c But, in this case, added funticon is just direct call function without any standard API. The below case is never appropriate implementation. -------------------- ---------------- | EXTCON framework | | USB framework | -------------------- ----------------- | | | | extcon-intel-usb.c <-------- pci-quirks.c > > I don't think I agree with your point even at general level. The > control required to handle this mux, even though simple, is enough to > deserve to be separated from xHCI code. xHCI should not need to care > about anything else expect does it have the mux, i.e. does it need to > register it or not. It should not need to care about how it needs to > be controlled or even what it is. We may decide to create something > else out of it instead of an extcon device later. > > But in any case, the mux is available on all new Intel platforms, but > it needs to be controlled by OS only in few "special" cases. We can > not force xHCI (or pci-quirks.c to be more precise) to be aware of > these "special" cases. The only way to make it work like that would > bet by using ifdefs, and we really really don't want that. > > And please also note that, though for now we only expect the mux > control registers to be part of xHCI MMIO, that is not always the > case. The control registers are part of the device controller MMIO on > some platforms. We do not want to duplicate the whole control of the > mux if/when we need the OS to be in control of it on a platform that > has those control registers mapped somewhere else then xHCI MMIO, > > So I would say that we have pretty good justification for separating > the mux control, which means unfortunately custom API in this case. > > But if you would prefer that we put the files somewhere else then > drivers/extcon/ and include/linux/extcon/ I'm fine with that. If you > like, we can put it to drivers/usb/host/ as that is where > pci-quirks.c is. That way I think we can also put the header to > include/usb/. There are the two type of extcon drivers as following: - provider extcon driver which use the devm_extcon_dev_register() and extcon_set_cable_state(). - client extcon driver which use the extcon_register_notifier() and extcon_set_cable_state() usually. The drivers/extcon directory only includes the provider extcon driver. You make the extcon-intel-usb.c as provider extcon driver. At the same time, this driver is used for client extcon driver by using the extcon_register_notifier(). If you want to recevie the notification from extcon_register_notifier(), the extcon device should update the state of external connector throught extcon_set_cable_state(). But, this driver don' use the extcon_set_cable_state(). I think that you should divide it according to role. Again, the usage case of extcon have to consist of both provider extcon driver and client extcon driver. If there is no provider extcon driver, client extcon driver don't receive the any notification of external connector from provider extcon driver. Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Heikki Krogerus <heikki.krogerus@linux.intel.com> |
|---|---|
| Date | 2015-12-07 14:00 +0100 |
| Subject | Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qD3yz-4AE-25@gated-at.bofh.it> |
| In reply to | #1284955 |
Hi, On Mon, Dec 07, 2015 at 10:24:22AM +0900, Chanwoo Choi wrote: > Hi, > > On 2015년 12월 04일 17:51, Heikki Krogerus wrote: > > Hi, > > > >> 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. > > > > The internal mux we are controlling here is physically separate > > device. Ideally we could populate child device for it, but since that > > is not possible because of the resource conflict, we use the library > > approach, which is really not that uncommon. > > New added functions for only specific device driver is not library. > > The all device drivers which is included in some framework should > connect to the other device driver through only framework API as following: > -------------------- ---------------- > | EXTCON framework |<-------->| USB framework | > -------------------- ----------------- > | | > | | > extcon-intel-usb.c pci-quirks.c > > But, in this case, added funticon is just direct call function > without any standard API. The below case is never appropriate implementation. > > -------------------- ---------------- > | EXTCON framework | | USB framework | > -------------------- ----------------- > | | > | | > extcon-intel-usb.c <-------- pci-quirks.c Man.. Cal it what you want, but like I said, exposing driver specific API is not ideal, but it is acceptable in special cases like this where we simply are not able to populate child device. If nothing else, then at least the fact that the code for the mux would otherwise need to be duplicated, is enough to justify it. > > I don't think I agree with your point even at general level. The > > control required to handle this mux, even though simple, is enough to > > deserve to be separated from xHCI code. xHCI should not need to care > > about anything else expect does it have the mux, i.e. does it need to > > register it or not. It should not need to care about how it needs to > > be controlled or even what it is. We may decide to create something > > else out of it instead of an extcon device later. > > > > But in any case, the mux is available on all new Intel platforms, but > > it needs to be controlled by OS only in few "special" cases. We can > > not force xHCI (or pci-quirks.c to be more precise) to be aware of > > these "special" cases. The only way to make it work like that would > > bet by using ifdefs, and we really really don't want that. > > > > And please also note that, though for now we only expect the mux > > control registers to be part of xHCI MMIO, that is not always the > > case. The control registers are part of the device controller MMIO on > > some platforms. We do not want to duplicate the whole control of the > > mux if/when we need the OS to be in control of it on a platform that > > has those control registers mapped somewhere else then xHCI MMIO, > > > > So I would say that we have pretty good justification for separating > > the mux control, which means unfortunately custom API in this case. > > > > But if you would prefer that we put the files somewhere else then > > drivers/extcon/ and include/linux/extcon/ I'm fine with that. If you > > like, we can put it to drivers/usb/host/ as that is where > > pci-quirks.c is. That way I think we can also put the header to > > include/usb/. > > There are the two type of extcon drivers as following: > - provider extcon driver which use the devm_extcon_dev_register() and extcon_set_cable_state(). > - client extcon driver which use the extcon_register_notifier() and extcon_set_cable_state() usually. > The drivers/extcon directory only includes the provider extcon driver. > > You make the extcon-intel-usb.c as provider extcon driver. > At the same time, this driver is used for client extcon driver > by using the extcon_register_notifier(). If you want to recevie > the notification from extcon_register_notifier(), the extcon device > should update the state of external connector throught extcon_set_cable_state(). > But, this driver don' use the extcon_set_cable_state(). > > I think that you should divide it according to role. > > Again, the usage case of extcon have to consist of both provider extcon driver > and client extcon driver. If there is no provider extcon driver, > client extcon driver don't receive the any notification of external connector > from provider extcon driver. What you are saying is that it is OK for both "client" and "provider" to change the state, but only client is allowed to react to a state change? You got to admit that the roles are pretty obscure here... In any case, I'm using the framework the way it allows itself to be used. If you want your framework to be used in a particular way, then you need to protect it from being used otherwise. Now anything with a handle to the extcon_dev can use it in every way possible. You are not documenting how you want the framework to be used. The only document for extcon in Documentation/extcon/ is about porting stuff from some Android specific "switch class" to extcon. Nowhere do you define what is a "client" and what is a "provider" and what are they meant for. If you want to limit what a "client" can do, you need to separate the API for it. Use the gpio API as an example. Check how the consumers and drivers are separated into their own headers in include/linux/gpio/. Keep the include/extcon.h header as legacy and deprecate it. And if you do that, the framework should really be improved with a few basic things regarding the "clients". At least start using some kind of ref counting with them. Now nothing really prevents a "provider" from being removed even if it has users (clients), or does it? This basically also shows how obscure the line between a "client" and "provider" is at the moment. Right now with what we have I see nothing wrong with my approach. Cheers, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2015-12-08 02:20 +0100 |
| Subject | Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qDf6G-40S-9@gated-at.bofh.it> |
| In reply to | #1285279 |
Hi, On 2015년 12월 07일 21:52, Heikki Krogerus wrote: > Hi, > > On Mon, Dec 07, 2015 at 10:24:22AM +0900, Chanwoo Choi wrote: >> Hi, >> >> On 2015년 12월 04일 17:51, Heikki Krogerus wrote: >>> Hi, >>> >>>> 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. >>> >>> The internal mux we are controlling here is physically separate >>> device. Ideally we could populate child device for it, but since that >>> is not possible because of the resource conflict, we use the library >>> approach, which is really not that uncommon. >> >> New added functions for only specific device driver is not library. >> >> The all device drivers which is included in some framework should >> connect to the other device driver through only framework API as following: >> -------------------- ---------------- >> | EXTCON framework |<-------->| USB framework | >> -------------------- ----------------- >> | | >> | | >> extcon-intel-usb.c pci-quirks.c >> >> But, in this case, added funticon is just direct call function >> without any standard API. The below case is never appropriate implementation. >> >> -------------------- ---------------- >> | EXTCON framework | | USB framework | >> -------------------- ----------------- >> | | >> | | >> extcon-intel-usb.c <-------- pci-quirks.c > > Man.. Cal it what you want, but like I said, exposing driver specific > API is not ideal, but it is acceptable in special cases like this > where we simply are not able to populate child device. If nothing > else, then at least the fact that the code for the mux would otherwise > need to be duplicated, is enough to justify it. > >>> I don't think I agree with your point even at general level. The >>> control required to handle this mux, even though simple, is enough to >>> deserve to be separated from xHCI code. xHCI should not need to care >>> about anything else expect does it have the mux, i.e. does it need to >>> register it or not. It should not need to care about how it needs to >>> be controlled or even what it is. We may decide to create something >>> else out of it instead of an extcon device later. >>> >>> But in any case, the mux is available on all new Intel platforms, but >>> it needs to be controlled by OS only in few "special" cases. We can >>> not force xHCI (or pci-quirks.c to be more precise) to be aware of >>> these "special" cases. The only way to make it work like that would >>> bet by using ifdefs, and we really really don't want that. >>> >>> And please also note that, though for now we only expect the mux >>> control registers to be part of xHCI MMIO, that is not always the >>> case. The control registers are part of the device controller MMIO on >>> some platforms. We do not want to duplicate the whole control of the >>> mux if/when we need the OS to be in control of it on a platform that >>> has those control registers mapped somewhere else then xHCI MMIO, >>> >>> So I would say that we have pretty good justification for separating >>> the mux control, which means unfortunately custom API in this case. >>> >>> But if you would prefer that we put the files somewhere else then >>> drivers/extcon/ and include/linux/extcon/ I'm fine with that. If you >>> like, we can put it to drivers/usb/host/ as that is where >>> pci-quirks.c is. That way I think we can also put the header to >>> include/usb/. >> >> There are the two type of extcon drivers as following: >> - provider extcon driver which use the devm_extcon_dev_register() and extcon_set_cable_state(). >> - client extcon driver which use the extcon_register_notifier() and extcon_set_cable_state() usually. >> The drivers/extcon directory only includes the provider extcon driver. >> >> You make the extcon-intel-usb.c as provider extcon driver. >> At the same time, this driver is used for client extcon driver >> by using the extcon_register_notifier(). If you want to recevie >> the notification from extcon_register_notifier(), the extcon device >> should update the state of external connector throught extcon_set_cable_state(). >> But, this driver don' use the extcon_set_cable_state(). >> >> I think that you should divide it according to role. >> >> Again, the usage case of extcon have to consist of both provider extcon driver >> and client extcon driver. If there is no provider extcon driver, >> client extcon driver don't receive the any notification of external connector >> from provider extcon driver. > > What you are saying is that it is OK for both "client" and "provider" > to change the state, but only client is allowed to react to a state > change? You got to admit that the roles are pretty obscure here... Yes, only client driver is able to take some behavior after receiving the notification from extcon provider driver. > > In any case, I'm using the framework the way it allows itself to be > used. If you want your framework to be used in a particular way, then > you need to protect it from being used otherwise. Now anything with a > handle to the extcon_dev can use it in every way possible. You are not > documenting how you want the framework to be used. The only document > for extcon in Documentation/extcon/ is about porting stuff from some > Android specific "switch class" to extcon. Nowhere do you define what > is a "client" and what is a "provider" and what are they meant for. I agree. The extcon framework uses the 'extcon_dev' structure for both provider and client driver. As you mentioned, it is the problem. So, I'll resolve this issue to protect change the state of external connector from client driver. Unitl now, I'm focusing on expand the usage case of extcon framework and improve the extcon core feature. As you comment, It is necessary task for extcon framework. The improvement of extcon framework is going on. > > If you want to limit what a "client" can do, you need to separate the > API for it. Use the gpio API as an example. Check how the consumers > and drivers are separated into their own headers in > include/linux/gpio/. Keep the include/extcon.h header as legacy and > deprecate it. > > And if you do that, the framework should really be improved with a few > basic things regarding the "clients". At least start using some kind > of ref counting with them. Now nothing really prevents a "provider" > from being removed even if it has users (clients), or does it? This > basically also shows how obscure the line between a "client" and > "provider" is at the moment. I agree. There are not enough document for extcon framework. After updating the extcon framework above, I'll make the appropriate document for guide. > > Right now with what we have I see nothing wrong with my approach. I can't agree to add specific function for only one device driver. As I commented, it is not appropriate way. Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Heikki Krogerus <heikki.krogerus@linux.intel.com> |
|---|---|
| Date | 2015-12-08 13:30 +0100 |
| Subject | Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qDpz3-2iu-1@gated-at.bofh.it> |
| In reply to | #1286138 |
Hi, On Tue, Dec 08, 2015 at 10:17:47AM +0900, Chanwoo Choi wrote: > I can't agree to add specific function for only one device driver. > As I commented, it is not appropriate way. OK, I'll prepare something else for this. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2015-12-03 20:20 +0100 |
| Subject | Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux |
| Message-ID | <qBHA5-Ns-3@gated-at.bofh.it> |
| In reply to | #1282842 |
On 12/03/2015 12:29 PM, 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 <heikki.krogerus@linux.intel.com>
[...]
> 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 @@
[...]
> +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);
I don't see where are you calling extcon_set_cable_state() fot the
"USB-HOST" cable...
This doesn't seem a legitimate extcon driver to me... :-/
> + 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);
So in reality this is an extcon client, not a provider? BTW, this API
isn't recommended...
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Heikki Krogerus <heikki.krogerus@linux.intel.com> |
|---|---|
| Date | 2015-12-03 10:40 +0100 |
| Subject | [PATCHv2 2/2] usb: pci-quirks: register USB mux found on Cherrytrail SOC |
| Message-ID | <qBywO-38I-21@gated-at.bofh.it> |
| In reply to | #1282841 |
Intel Braswell/Cherrytrail has an internal mux that shares
one USB port between USB Device Controller and xHCI. The
same mux is found on several SOCs from Intel, but only on
a few Cherrytrail based platforms the OS is expected to
configure it. Normally BIOS takes care of it.
The driver for the mux is an "extcon" driver. With this we
only register the mux if it's detected.
Suggested-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/host/pci-quirks.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 26cb8c8..ee875e1 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,6 +16,7 @@
#include <linux/export.h>
#include <linux/acpi.h>
#include <linux/dmi.h>
+#include <linux/extcon/intel_usb_mux.h>
#include "pci-quirks.h"
#include "xhci-ext-caps.h"
@@ -1022,9 +1023,32 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
hc_init:
- if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
usb_enable_intel_xhci_ports(pdev);
+ /*
+ * Initialize the internal mux that shares a port between USB
+ * Device Controller and xHCI on platforms that have it.
+ */
+#define XHCI_INTEL_VENDOR_CAPS 192
+#define XHCI_INTEL_USB_MUX_OFFSET 0x80d8
+ if (xhci_find_next_ext_cap(base, 0, XHCI_INTEL_VENDOR_CAPS)) {
+ struct intel_usb_mux *mux;
+ struct resource r;
+
+ r.start = pci_resource_start(pdev, 0) +
+ XHCI_INTEL_USB_MUX_OFFSET;
+ r.end = r.start + 8;
+ r.flags = IORESOURCE_MEM;
+
+ mux = intel_usb_mux_register(&pdev->dev, &r);
+ if (IS_ERR(mux) && PTR_ERR(mux) == -ENOTSUPP)
+ dev_dbg(&pdev->dev, "USB mux not supported\n");
+ else if (IS_ERR(mux))
+ dev_err(&pdev->dev, "failed to register mux\n");
+ }
+ }
+
op_reg_base = base + XHCI_HC_LENGTH(readl(base));
/* Wait for the host controller to be ready before writing any
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2015-12-03 20:10 +0100 |
| Subject | Re: [PATCHv2 2/2] usb: pci-quirks: register USB mux found on Cherrytrail SOC |
| Message-ID | <qBHqq-Kh-19@gated-at.bofh.it> |
| In reply to | #1282848 |
Hello.
On 12/03/2015 12:29 PM, Heikki Krogerus wrote:
> Intel Braswell/Cherrytrail has an internal mux that shares
> one USB port between USB Device Controller and xHCI. The
> same mux is found on several SOCs from Intel, but only on
> a few Cherrytrail based platforms the OS is expected to
> configure it. Normally BIOS takes care of it.
>
> The driver for the mux is an "extcon" driver. With this we
> only register the mux if it's detected.
Hm, I had somewhat identical case on the Renesas SoC: the 2 channel mux
was mapped to the USB device PHY register space, so I chose to implement a PHY
driver, not extcon...
I don't quite understand how mux maps to the extcon core -- doesn't it
provide support only the input signals?
> Suggested-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/host/pci-quirks.c | 26 +++++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 26cb8c8..ee875e1 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
[...]
> @@ -1022,9 +1023,32 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
> writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
>
> hc_init:
> - if (pdev->vendor == PCI_VENDOR_ID_INTEL)
> + if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
> usb_enable_intel_xhci_ports(pdev);
>
> + /*
> + * Initialize the internal mux that shares a port between USB
> + * Device Controller and xHCI on platforms that have it.
> + */
> +#define XHCI_INTEL_VENDOR_CAPS 192
> +#define XHCI_INTEL_USB_MUX_OFFSET 0x80d8
> + if (xhci_find_next_ext_cap(base, 0, XHCI_INTEL_VENDOR_CAPS)) {
> + struct intel_usb_mux *mux;
> + struct resource r;
> +
> + r.start = pci_resource_start(pdev, 0) +
> + XHCI_INTEL_USB_MUX_OFFSET;
> + r.end = r.start + 8;
> + r.flags = IORESOURCE_MEM;
> +
> + mux = intel_usb_mux_register(&pdev->dev, &r);
> + if (IS_ERR(mux) && PTR_ERR(mux) == -ENOTSUPP)
I think you can drop IS_ERR() check here...
> + dev_dbg(&pdev->dev, "USB mux not supported\n");
> + else if (IS_ERR(mux))
> + dev_err(&pdev->dev, "failed to register mux\n");
> + }
> + }
> +
> op_reg_base = base + XHCI_HC_LENGTH(readl(base));
>
> /* Wait for the host controller to be ready before writing any
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web