* [PATCH 01/11] mux: core: Add of_mux_control_get helper function
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Currently the mux_control_get implementation only deals with getting
mux controllers on DT platforms. This commit renames the current
implementation to of_mux_control_get to reflect this and makes
mux_control_get a wrapper around of_mux_control_get.
This is a preparation patch for adding support for getting muxes on
non DT platforms.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/mux/core.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index e75d5adc5949..6142493c327b 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -422,14 +422,8 @@ static struct mux_chip *of_get_mux_chip_by_node(struct device_node *np)
return dev ? to_mux_chip(dev) : NULL;
}
-/**
- * mux_control_get() - Get the mux-control for a device.
- * @dev: The device that needs a mux-control.
- * @mux_name: The name identifying the mux-control.
- *
- * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
- */
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+static struct mux_control *of_mux_control_get(struct device *dev,
+ const char *mux_name)
{
struct device_node *np = dev->of_node;
struct of_phandle_args args;
@@ -483,6 +477,22 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
return &mux_chip->mux[controller];
}
+
+/**
+ * mux_control_get() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+{
+ /* look up via DT first */
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_mux_control_get(dev, mux_name);
+
+ return ERR_PTR(-ENODEV);
+}
EXPORT_SYMBOL_GPL(mux_control_get);
/**
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 02/11] mux: core: Add support for getting a mux controller on a non DT platform
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede
On non DT platforms we cannot get the mux_chip by pnode. Other subsystems
(regulator, clock, pwm) have the same problem and solve this by allowing
platform / board-setup code to add entries to a lookup table and then use
this table to look things up.
This commit adds support for getting a mux controller on a non DT platform
following this pattern. It is based on a simplified version of the pwm
subsys lookup code, the dev_id and mux_name parts of a lookup table entry
are mandatory in the mux-core implementation.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/mux/core.c | 96 +++++++++++++++++++++++++++++++++++++++++++-
include/linux/mux/consumer.h | 11 +++++
2 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 6142493c327b..8864cc745506 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -24,6 +24,9 @@
#include <linux/of_platform.h>
#include <linux/slab.h>
+static DEFINE_MUTEX(mux_lookup_lock);
+static LIST_HEAD(mux_lookup_list);
+
/*
* The idle-as-is "state" is not an actual state that may be selected, it
* only implies that the state should not be changed. So, use that state
@@ -408,6 +411,23 @@ int mux_control_deselect(struct mux_control *mux)
}
EXPORT_SYMBOL_GPL(mux_control_deselect);
+static int parent_name_match(struct device *dev, const void *data)
+{
+ const char *parent_name = dev_name(dev->parent);
+ const char *name = data;
+
+ return strcmp(parent_name, name) == 0;
+}
+
+static struct mux_chip *mux_chip_get_by_name(const char *name)
+{
+ struct device *dev;
+
+ dev = class_find_device(&mux_class, NULL, name, parent_name_match);
+
+ return dev ? to_mux_chip(dev) : NULL;
+}
+
static int of_dev_node_match(struct device *dev, const void *data)
{
return dev->of_node == data;
@@ -479,6 +499,42 @@ static struct mux_control *of_mux_control_get(struct device *dev,
}
/**
+ * mux_add_table() - register PWM device consumers
+ * @table: array of consumers to register
+ * @num: number of consumers in table
+ */
+void mux_add_table(struct mux_lookup *table, size_t num)
+{
+ mutex_lock(&mux_lookup_lock);
+
+ while (num--) {
+ list_add_tail(&table->list, &mux_lookup_list);
+ table++;
+ }
+
+ mutex_unlock(&mux_lookup_lock);
+}
+EXPORT_SYMBOL_GPL(mux_add_table);
+
+/**
+ * mux_remove_table() - unregister PWM device consumers
+ * @table: array of consumers to unregister
+ * @num: number of consumers in table
+ */
+void mux_remove_table(struct mux_lookup *table, size_t num)
+{
+ mutex_lock(&mux_lookup_lock);
+
+ while (num--) {
+ list_del(&table->list);
+ table++;
+ }
+
+ mutex_unlock(&mux_lookup_lock);
+}
+EXPORT_SYMBOL_GPL(mux_remove_table);
+
+/**
* mux_control_get() - Get the mux-control for a device.
* @dev: The device that needs a mux-control.
* @mux_name: The name identifying the mux-control.
@@ -487,11 +543,49 @@ static struct mux_control *of_mux_control_get(struct device *dev,
*/
struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
{
+ struct mux_lookup *m, *chosen = NULL;
+ const char *dev_id = dev_name(dev);
+ struct mux_chip *mux_chip;
+
/* look up via DT first */
if (IS_ENABLED(CONFIG_OF) && dev->of_node)
return of_mux_control_get(dev, mux_name);
- return ERR_PTR(-ENODEV);
+ /*
+ * For non DT we look up the provider in the static table typically
+ * provided by board setup code.
+ *
+ * If a match is found, the provider mux chip is looked up by name
+ * and a mux-control is requested using the table provided index.
+ */
+ mutex_lock(&mux_lookup_lock);
+ list_for_each_entry(m, &mux_lookup_list, list) {
+ if (WARN_ON(!m->dev_id || !m->mux_name || !m->provider))
+ continue;
+
+ if (strcmp(m->dev_id, dev_id) == 0 &&
+ strcmp(m->mux_name, mux_name) == 0) {
+ chosen = m;
+ break;
+ }
+ }
+ mutex_unlock(&mux_lookup_lock);
+
+ if (!chosen)
+ return ERR_PTR(-ENODEV);
+
+ mux_chip = mux_chip_get_by_name(chosen->provider);
+ if (!mux_chip)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ if (chosen->index >= mux_chip->controllers) {
+ dev_err(dev, "Mux lookup table index out of bounds %u >= %u\n",
+ chosen->index, mux_chip->controllers);
+ put_device(&mux_chip->dev);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return &mux_chip->mux[chosen->index];
}
EXPORT_SYMBOL_GPL(mux_control_get);
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index ea96d4c82be7..912dd48a3a5d 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -18,6 +18,17 @@
struct device;
struct mux_control;
+struct mux_lookup {
+ struct list_head list;
+ const char *provider;
+ unsigned int index;
+ const char *dev_id;
+ const char *mux_name;
+};
+
+void mux_add_table(struct mux_lookup *table, size_t num);
+void mux_remove_table(struct mux_lookup *table, size_t num);
+
unsigned int mux_control_states(struct mux_control *mux);
int __must_check mux_control_select(struct mux_control *mux,
unsigned int state);
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 02/11] mux: core: Add support for getting a mux controller on a non DT platform
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
On non DT platforms we cannot get the mux_chip by pnode. Other subsystems
(regulator, clock, pwm) have the same problem and solve this by allowing
platform / board-setup code to add entries to a lookup table and then use
this table to look things up.
This commit adds support for getting a mux controller on a non DT platform
following this pattern. It is based on a simplified version of the pwm
subsys lookup code, the dev_id and mux_name parts of a lookup table entry
are mandatory in the mux-core implementation.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/mux/core.c | 96 +++++++++++++++++++++++++++++++++++++++++++-
include/linux/mux/consumer.h | 11 +++++
2 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 6142493c327b..8864cc745506 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -24,6 +24,9 @@
#include <linux/of_platform.h>
#include <linux/slab.h>
+static DEFINE_MUTEX(mux_lookup_lock);
+static LIST_HEAD(mux_lookup_list);
+
/*
* The idle-as-is "state" is not an actual state that may be selected, it
* only implies that the state should not be changed. So, use that state
@@ -408,6 +411,23 @@ int mux_control_deselect(struct mux_control *mux)
}
EXPORT_SYMBOL_GPL(mux_control_deselect);
+static int parent_name_match(struct device *dev, const void *data)
+{
+ const char *parent_name = dev_name(dev->parent);
+ const char *name = data;
+
+ return strcmp(parent_name, name) == 0;
+}
+
+static struct mux_chip *mux_chip_get_by_name(const char *name)
+{
+ struct device *dev;
+
+ dev = class_find_device(&mux_class, NULL, name, parent_name_match);
+
+ return dev ? to_mux_chip(dev) : NULL;
+}
+
static int of_dev_node_match(struct device *dev, const void *data)
{
return dev->of_node == data;
@@ -479,6 +499,42 @@ static struct mux_control *of_mux_control_get(struct device *dev,
}
/**
+ * mux_add_table() - register PWM device consumers
+ * @table: array of consumers to register
+ * @num: number of consumers in table
+ */
+void mux_add_table(struct mux_lookup *table, size_t num)
+{
+ mutex_lock(&mux_lookup_lock);
+
+ while (num--) {
+ list_add_tail(&table->list, &mux_lookup_list);
+ table++;
+ }
+
+ mutex_unlock(&mux_lookup_lock);
+}
+EXPORT_SYMBOL_GPL(mux_add_table);
+
+/**
+ * mux_remove_table() - unregister PWM device consumers
+ * @table: array of consumers to unregister
+ * @num: number of consumers in table
+ */
+void mux_remove_table(struct mux_lookup *table, size_t num)
+{
+ mutex_lock(&mux_lookup_lock);
+
+ while (num--) {
+ list_del(&table->list);
+ table++;
+ }
+
+ mutex_unlock(&mux_lookup_lock);
+}
+EXPORT_SYMBOL_GPL(mux_remove_table);
+
+/**
* mux_control_get() - Get the mux-control for a device.
* @dev: The device that needs a mux-control.
* @mux_name: The name identifying the mux-control.
@@ -487,11 +543,49 @@ static struct mux_control *of_mux_control_get(struct device *dev,
*/
struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
{
+ struct mux_lookup *m, *chosen = NULL;
+ const char *dev_id = dev_name(dev);
+ struct mux_chip *mux_chip;
+
/* look up via DT first */
if (IS_ENABLED(CONFIG_OF) && dev->of_node)
return of_mux_control_get(dev, mux_name);
- return ERR_PTR(-ENODEV);
+ /*
+ * For non DT we look up the provider in the static table typically
+ * provided by board setup code.
+ *
+ * If a match is found, the provider mux chip is looked up by name
+ * and a mux-control is requested using the table provided index.
+ */
+ mutex_lock(&mux_lookup_lock);
+ list_for_each_entry(m, &mux_lookup_list, list) {
+ if (WARN_ON(!m->dev_id || !m->mux_name || !m->provider))
+ continue;
+
+ if (strcmp(m->dev_id, dev_id) == 0 &&
+ strcmp(m->mux_name, mux_name) == 0) {
+ chosen = m;
+ break;
+ }
+ }
+ mutex_unlock(&mux_lookup_lock);
+
+ if (!chosen)
+ return ERR_PTR(-ENODEV);
+
+ mux_chip = mux_chip_get_by_name(chosen->provider);
+ if (!mux_chip)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ if (chosen->index >= mux_chip->controllers) {
+ dev_err(dev, "Mux lookup table index out of bounds %u >= %u\n",
+ chosen->index, mux_chip->controllers);
+ put_device(&mux_chip->dev);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return &mux_chip->mux[chosen->index];
}
EXPORT_SYMBOL_GPL(mux_control_get);
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index ea96d4c82be7..912dd48a3a5d 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -18,6 +18,17 @@
struct device;
struct mux_control;
+struct mux_lookup {
+ struct list_head list;
+ const char *provider;
+ unsigned int index;
+ const char *dev_id;
+ const char *mux_name;
+};
+
+void mux_add_table(struct mux_lookup *table, size_t num);
+void mux_remove_table(struct mux_lookup *table, size_t num);
+
unsigned int mux_control_states(struct mux_control *mux);
int __must_check mux_control_select(struct mux_control *mux,
unsigned int state);
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [PATCH 02/11] mux: core: Add support for getting a mux controller on a non DT platform
2017-09-01 21:48 ` Hans de Goede
(?)
@ 2017-09-02 19:13 ` sathya
2017-09-04 14:21 ` Hans de Goede
-1 siblings, 1 reply; 52+ messages in thread
From: sathya @ 2017-09-02 19:13 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Peter Rosin,
Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Hi,
On 09/01/2017 02:48 PM, Hans de Goede wrote:
> On non DT platforms we cannot get the mux_chip by pnode. Other subsystems
> (regulator, clock, pwm) have the same problem and solve this by allowing
> platform / board-setup code to add entries to a lookup table and then use
> this table to look things up.
>
> This commit adds support for getting a mux controller on a non DT platform
> following this pattern. It is based on a simplified version of the pwm
> subsys lookup code, the dev_id and mux_name parts of a lookup table entry
> are mandatory in the mux-core implementation.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/mux/core.c | 96 +++++++++++++++++++++++++++++++++++++++++++-
> include/linux/mux/consumer.h | 11 +++++
> 2 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 6142493c327b..8864cc745506 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -24,6 +24,9 @@
> #include <linux/of_platform.h>
> #include <linux/slab.h>
>
> +static DEFINE_MUTEX(mux_lookup_lock);
> +static LIST_HEAD(mux_lookup_list);
> +
> /*
> * The idle-as-is "state" is not an actual state that may be selected, it
> * only implies that the state should not be changed. So, use that state
> @@ -408,6 +411,23 @@ int mux_control_deselect(struct mux_control *mux)
> }
> EXPORT_SYMBOL_GPL(mux_control_deselect);
>
> +static int parent_name_match(struct device *dev, const void *data)
> +{
> + const char *parent_name = dev_name(dev->parent);
Device name usually contains id section (devname.id). Did you take this
into consideration ?
> + const char *name = data;
> +
> + return strcmp(parent_name, name) == 0;
> +}
> +
> +static struct mux_chip *mux_chip_get_by_name(const char *name)
> +{
> + struct device *dev;
> +
> + dev = class_find_device(&mux_class, NULL, name, parent_name_match);
> +
> + return dev ? to_mux_chip(dev) : NULL;
> +}
> +
> static int of_dev_node_match(struct device *dev, const void *data)
> {
> return dev->of_node == data;
> @@ -479,6 +499,42 @@ static struct mux_control *of_mux_control_get(struct device *dev,
> }
>
> /**
> + * mux_add_table() - register PWM device consumers
PWM -> MUX
> + * @table: array of consumers to register
> + * @num: number of consumers in table
> + */
> +void mux_add_table(struct mux_lookup *table, size_t num)
> +{
> + mutex_lock(&mux_lookup_lock);
> +
> + while (num--) {
> + list_add_tail(&table->list, &mux_lookup_list);
> + table++;
> + }
> +
> + mutex_unlock(&mux_lookup_lock);
> +}
> +EXPORT_SYMBOL_GPL(mux_add_table);
> +
> +/**
> + * mux_remove_table() - unregister PWM device consumers
> + * @table: array of consumers to unregister
> + * @num: number of consumers in table
> + */
> +void mux_remove_table(struct mux_lookup *table, size_t num)
> +{
> + mutex_lock(&mux_lookup_lock);
> +
> + while (num--) {
> + list_del(&table->list);
> + table++;
> + }
> +
> + mutex_unlock(&mux_lookup_lock);
> +}
> +EXPORT_SYMBOL_GPL(mux_remove_table);
> +
> +/**
> * mux_control_get() - Get the mux-control for a device.
> * @dev: The device that needs a mux-control.
> * @mux_name: The name identifying the mux-control.
> @@ -487,11 +543,49 @@ static struct mux_control *of_mux_control_get(struct device *dev,
> */
> struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> {
> + struct mux_lookup *m, *chosen = NULL;
> + const char *dev_id = dev_name(dev);
> + struct mux_chip *mux_chip;
> +
> /* look up via DT first */
> if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> return of_mux_control_get(dev, mux_name);
>
> - return ERR_PTR(-ENODEV);
> + /*
> + * For non DT we look up the provider in the static table typically
> + * provided by board setup code.
> + *
> + * If a match is found, the provider mux chip is looked up by name
> + * and a mux-control is requested using the table provided index.
> + */
> + mutex_lock(&mux_lookup_lock);
> + list_for_each_entry(m, &mux_lookup_list, list) {
> + if (WARN_ON(!m->dev_id || !m->mux_name || !m->provider))
> + continue;
> +
> + if (strcmp(m->dev_id, dev_id) == 0 &&
> + strcmp(m->mux_name, mux_name) == 0) {
> + chosen = m;
> + break;
> + }
> + }
> + mutex_unlock(&mux_lookup_lock);
> +
> + if (!chosen)
> + return ERR_PTR(-ENODEV);
> +
> + mux_chip = mux_chip_get_by_name(chosen->provider);
> + if (!mux_chip)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + if (chosen->index >= mux_chip->controllers) {
> + dev_err(dev, "Mux lookup table index out of bounds %u >= %u\n",
> + chosen->index, mux_chip->controllers);
> + put_device(&mux_chip->dev);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return &mux_chip->mux[chosen->index];
> }
> EXPORT_SYMBOL_GPL(mux_control_get);
>
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index ea96d4c82be7..912dd48a3a5d 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -18,6 +18,17 @@
> struct device;
> struct mux_control;
>
> +struct mux_lookup {
> + struct list_head list;
> + const char *provider;
> + unsigned int index;
> + const char *dev_id;
> + const char *mux_name;
> +};
> +
> +void mux_add_table(struct mux_lookup *table, size_t num);
> +void mux_remove_table(struct mux_lookup *table, size_t num);
> +
> unsigned int mux_control_states(struct mux_control *mux);
> int __must_check mux_control_select(struct mux_control *mux,
> unsigned int state);
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 02/11] mux: core: Add support for getting a mux controller on a non DT platform
2017-09-02 19:13 ` sathya
@ 2017-09-04 14:21 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-04 14:21 UTC (permalink / raw)
To: sathya, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Peter Rosin,
Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Hi,
On 02-09-17 21:13, sathya wrote:
> Hi,
>
>
> On 09/01/2017 02:48 PM, Hans de Goede wrote:
>> On non DT platforms we cannot get the mux_chip by pnode. Other subsystems
>> (regulator, clock, pwm) have the same problem and solve this by allowing
>> platform / board-setup code to add entries to a lookup table and then use
>> this table to look things up.
>>
>> This commit adds support for getting a mux controller on a non DT platform
>> following this pattern. It is based on a simplified version of the pwm
>> subsys lookup code, the dev_id and mux_name parts of a lookup table entry
>> are mandatory in the mux-core implementation.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/mux/core.c | 96 +++++++++++++++++++++++++++++++++++++++++++-
>> include/linux/mux/consumer.h | 11 +++++
>> 2 files changed, 106 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
>> index 6142493c327b..8864cc745506 100644
>> --- a/drivers/mux/core.c
>> +++ b/drivers/mux/core.c
>> @@ -24,6 +24,9 @@
>> #include <linux/of_platform.h>
>> #include <linux/slab.h>
>> +static DEFINE_MUTEX(mux_lookup_lock);
>> +static LIST_HEAD(mux_lookup_list);
>> +
>> /*
>> * The idle-as-is "state" is not an actual state that may be selected, it
>> * only implies that the state should not be changed. So, use that state
>> @@ -408,6 +411,23 @@ int mux_control_deselect(struct mux_control *mux)
>> }
>> EXPORT_SYMBOL_GPL(mux_control_deselect);
>> +static int parent_name_match(struct device *dev, const void *data)
>> +{
>> + const char *parent_name = dev_name(dev->parent);
> Device name usually contains id section (devname.id). Did you take this into consideration ?
>> + const char *name = data;
>> +
>> + return strcmp(parent_name, name) == 0;
>> +}
>> +
>> +static struct mux_chip *mux_chip_get_by_name(const char *name)
>> +{
>> + struct device *dev;
>> +
>> + dev = class_find_device(&mux_class, NULL, name, parent_name_match);
>> +
>> + return dev ? to_mux_chip(dev) : NULL;
>> +}
>> +
>> static int of_dev_node_match(struct device *dev, const void *data)
>> {
>> return dev->of_node == data;
>> @@ -479,6 +499,42 @@ static struct mux_control *of_mux_control_get(struct device *dev,
>> }
>> /**
>> + * mux_add_table() - register PWM device consumers
> PWM -> MUX
Thank you, good catch, fixed for v2.
Regards,
Hans
>> + * @table: array of consumers to register
>> + * @num: number of consumers in table
>> + */
>> +void mux_add_table(struct mux_lookup *table, size_t num)
>> +{
>> + mutex_lock(&mux_lookup_lock);
>> +
>> + while (num--) {
>> + list_add_tail(&table->list, &mux_lookup_list);
>> + table++;
>> + }
>> +
>> + mutex_unlock(&mux_lookup_lock);
>> +}
>> +EXPORT_SYMBOL_GPL(mux_add_table);
>> +
>> +/**
>> + * mux_remove_table() - unregister PWM device consumers
>> + * @table: array of consumers to unregister
>> + * @num: number of consumers in table
>> + */
>> +void mux_remove_table(struct mux_lookup *table, size_t num)
>> +{
>> + mutex_lock(&mux_lookup_lock);
>> +
>> + while (num--) {
>> + list_del(&table->list);
>> + table++;
>> + }
>> +
>> + mutex_unlock(&mux_lookup_lock);
>> +}
>> +EXPORT_SYMBOL_GPL(mux_remove_table);
>> +
>> +/**
>> * mux_control_get() - Get the mux-control for a device.
>> * @dev: The device that needs a mux-control.
>> * @mux_name: The name identifying the mux-control.
>> @@ -487,11 +543,49 @@ static struct mux_control *of_mux_control_get(struct device *dev,
>> */
>> struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
>> {
>> + struct mux_lookup *m, *chosen = NULL;
>> + const char *dev_id = dev_name(dev);
>> + struct mux_chip *mux_chip;
>> +
>> /* look up via DT first */
>> if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>> return of_mux_control_get(dev, mux_name);
>> - return ERR_PTR(-ENODEV);
>> + /*
>> + * For non DT we look up the provider in the static table typically
>> + * provided by board setup code.
>> + *
>> + * If a match is found, the provider mux chip is looked up by name
>> + * and a mux-control is requested using the table provided index.
>> + */
>> + mutex_lock(&mux_lookup_lock);
>> + list_for_each_entry(m, &mux_lookup_list, list) {
>> + if (WARN_ON(!m->dev_id || !m->mux_name || !m->provider))
>> + continue;
>> +
>> + if (strcmp(m->dev_id, dev_id) == 0 &&
>> + strcmp(m->mux_name, mux_name) == 0) {
>> + chosen = m;
>> + break;
>> + }
>> + }
>> + mutex_unlock(&mux_lookup_lock);
>> +
>> + if (!chosen)
>> + return ERR_PTR(-ENODEV);
>> +
>> + mux_chip = mux_chip_get_by_name(chosen->provider);
>> + if (!mux_chip)
>> + return ERR_PTR(-EPROBE_DEFER);
>> +
>> + if (chosen->index >= mux_chip->controllers) {
>> + dev_err(dev, "Mux lookup table index out of bounds %u >= %u\n",
>> + chosen->index, mux_chip->controllers);
>> + put_device(&mux_chip->dev);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + return &mux_chip->mux[chosen->index];
>> }
>> EXPORT_SYMBOL_GPL(mux_control_get);
>> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
>> index ea96d4c82be7..912dd48a3a5d 100644
>> --- a/include/linux/mux/consumer.h
>> +++ b/include/linux/mux/consumer.h
>> @@ -18,6 +18,17 @@
>> struct device;
>> struct mux_control;
>> +struct mux_lookup {
>> + struct list_head list;
>> + const char *provider;
>> + unsigned int index;
>> + const char *dev_id;
>> + const char *mux_name;
>> +};
>> +
>> +void mux_add_table(struct mux_lookup *table, size_t num);
>> +void mux_remove_table(struct mux_lookup *table, size_t num);
>> +
>> unsigned int mux_control_states(struct mux_control *mux);
>> int __must_check mux_control_select(struct mux_control *mux,
>> unsigned int state);
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 02/11] mux: core: Add support for getting a mux controller on a non DT platform
2017-09-01 21:48 ` Hans de Goede
(?)
(?)
@ 2017-09-04 11:19 ` Peter Rosin
[not found] ` <0c882d23-d008-3d61-27be-3fa22bf59521-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
-1 siblings, 1 reply; 52+ messages in thread
From: Peter Rosin @ 2017-09-04 11:19 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
Hi!
Some comments inline...
On 2017-09-01 23:48, Hans de Goede wrote:
> On non DT platforms we cannot get the mux_chip by pnode. Other subsystems
> (regulator, clock, pwm) have the same problem and solve this by allowing
> platform / board-setup code to add entries to a lookup table and then use
> this table to look things up.
>
> This commit adds support for getting a mux controller on a non DT platform
> following this pattern. It is based on a simplified version of the pwm
> subsys lookup code, the dev_id and mux_name parts of a lookup table entry
> are mandatory in the mux-core implementation.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/mux/core.c | 96 +++++++++++++++++++++++++++++++++++++++++++-
> include/linux/mux/consumer.h | 11 +++++
> 2 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 6142493c327b..8864cc745506 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -24,6 +24,9 @@
> #include <linux/of_platform.h>
> #include <linux/slab.h>
>
> +static DEFINE_MUTEX(mux_lookup_lock);
> +static LIST_HEAD(mux_lookup_list);
> +
> /*
> * The idle-as-is "state" is not an actual state that may be selected, it
> * only implies that the state should not be changed. So, use that state
> @@ -408,6 +411,23 @@ int mux_control_deselect(struct mux_control *mux)
> }
> EXPORT_SYMBOL_GPL(mux_control_deselect);
>
> +static int parent_name_match(struct device *dev, const void *data)
> +{
> + const char *parent_name = dev_name(dev->parent);
> + const char *name = data;
> +
> + return strcmp(parent_name, name) == 0;
> +}
> +
> +static struct mux_chip *mux_chip_get_by_name(const char *name)
> +{
> + struct device *dev;
> +
> + dev = class_find_device(&mux_class, NULL, name, parent_name_match);
> +
> + return dev ? to_mux_chip(dev) : NULL;
> +}
> +
> static int of_dev_node_match(struct device *dev, const void *data)
> {
> return dev->of_node == data;
> @@ -479,6 +499,42 @@ static struct mux_control *of_mux_control_get(struct device *dev,
> }
>
> /**
> + * mux_add_table() - register PWM device consumers
register mux controllers (because you are not registering consumers, right?
someone is registering controllers so that they can be found by consumers?)
> + * @table: array of consumers to register
> + * @num: number of consumers in table
controllers?
> + */
> +void mux_add_table(struct mux_lookup *table, size_t num)
> +{
> + mutex_lock(&mux_lookup_lock);
> +
> + while (num--) {
> + list_add_tail(&table->list, &mux_lookup_list);
> + table++;
> + }
I prefer
for (; num--; table++)
list_add_tail(&table->list, &mux_lookup_list);
> +
> + mutex_unlock(&mux_lookup_lock);
> +}
> +EXPORT_SYMBOL_GPL(mux_add_table);
> +
> +/**
> + * mux_remove_table() - unregister PWM device consumers
unregister mux controllers(?)
> + * @table: array of consumers to unregister
> + * @num: number of consumers in table
controllers?
> + */
> +void mux_remove_table(struct mux_lookup *table, size_t num)
> +{
> + mutex_lock(&mux_lookup_lock);
> +
> + while (num--) {
> + list_del(&table->list);
> + table++;
> + }
for() loop here as well.
> +
> + mutex_unlock(&mux_lookup_lock);
> +}
> +EXPORT_SYMBOL_GPL(mux_remove_table);
> +
> +/**
> * mux_control_get() - Get the mux-control for a device.
> * @dev: The device that needs a mux-control.
> * @mux_name: The name identifying the mux-control.
> @@ -487,11 +543,49 @@ static struct mux_control *of_mux_control_get(struct device *dev,
> */
> struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> {
> + struct mux_lookup *m, *chosen = NULL;
> + const char *dev_id = dev_name(dev);
> + struct mux_chip *mux_chip;
> +
> /* look up via DT first */
> if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> return of_mux_control_get(dev, mux_name);
>
> - return ERR_PTR(-ENODEV);
> + /*
> + * For non DT we look up the provider in the static table typically
> + * provided by board setup code.
> + *
> + * If a match is found, the provider mux chip is looked up by name
> + * and a mux-control is requested using the table provided index.
> + */
> + mutex_lock(&mux_lookup_lock);
> + list_for_each_entry(m, &mux_lookup_list, list) {
> + if (WARN_ON(!m->dev_id || !m->mux_name || !m->provider))
> + continue;
> +
> + if (strcmp(m->dev_id, dev_id) == 0 &&
> + strcmp(m->mux_name, mux_name) == 0) {
I want the below format (with ! instead of == 0 and the brace on the next line
when the condition has a line break):
if (!strcmp(m->dev_id, dev_id) &&
!strcmp(m->mux_name, mux_name))
{
> + chosen = m;
> + break;
> + }
> + }
> + mutex_unlock(&mux_lookup_lock);
> +
> + if (!chosen)
> + return ERR_PTR(-ENODEV);
> +
> + mux_chip = mux_chip_get_by_name(chosen->provider);
> + if (!mux_chip)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + if (chosen->index >= mux_chip->controllers) {
> + dev_err(dev, "Mux lookup table index out of bounds %u >= %u\n",
> + chosen->index, mux_chip->controllers);
> + put_device(&mux_chip->dev);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return &mux_chip->mux[chosen->index];
> }
> EXPORT_SYMBOL_GPL(mux_control_get);
>
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index ea96d4c82be7..912dd48a3a5d 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -18,6 +18,17 @@
> struct device;
> struct mux_control;
>
I want a kernel-doc comment here, describing the structure.
> +struct mux_lookup {
> + struct list_head list;
> + const char *provider;
> + unsigned int index;
> + const char *dev_id;
> + const char *mux_name;
> +};
> +
> +void mux_add_table(struct mux_lookup *table, size_t num);
> +void mux_remove_table(struct mux_lookup *table, size_t num);
> +
I'm not sure if consumer.h is the right place for this, but it can
be moved when I think of something better. Which I can't for the
moment...
> unsigned int mux_control_states(struct mux_control *mux);
> int __must_check mux_control_select(struct mux_control *mux,
> unsigned int state);
>
Cheers,
Peter
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede
Add MUX_USB_* state constant defines, which can be used by USB
device/host and Type-C polarity/role/altmode mux drivers and consumers
to ensure that they agree on the meaning of the mux_control_select()
state argument.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
include/linux/mux/consumer.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index 912dd48a3a5d..e3ec9b4db962 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -15,6 +15,22 @@
#include <linux/compiler.h>
+/*
+ * Mux state values for USB muxes, used for both USB device/host role muxes
+ * as well as for Type-C polarity/role/altmode muxes.
+ *
+ * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
+ * inverted-polarity (Type-C plugged in upside down) can happen with any
+ * other mux-state.
+ */
+#define MUX_USB_POLARITY_INV BIT(0) /* Polarity inverted bit */
+#define MUX_USB_NONE (1 << 1) /* Mux open / not connected */
+#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
+#define MUX_USB_HOST (3 << 1) /* USB host mode */
+#define MUX_USB_HOST_AND_DP_SRC (4 << 1) /* USB host + 2 lanes Display Port */
+#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
+#define MUX_USB_STATES (6 << 1)
+
struct device;
struct mux_control;
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Add MUX_USB_* state constant defines, which can be used by USB
device/host and Type-C polarity/role/altmode mux drivers and consumers
to ensure that they agree on the meaning of the mux_control_select()
state argument.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
include/linux/mux/consumer.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index 912dd48a3a5d..e3ec9b4db962 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -15,6 +15,22 @@
#include <linux/compiler.h>
+/*
+ * Mux state values for USB muxes, used for both USB device/host role muxes
+ * as well as for Type-C polarity/role/altmode muxes.
+ *
+ * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
+ * inverted-polarity (Type-C plugged in upside down) can happen with any
+ * other mux-state.
+ */
+#define MUX_USB_POLARITY_INV BIT(0) /* Polarity inverted bit */
+#define MUX_USB_NONE (1 << 1) /* Mux open / not connected */
+#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
+#define MUX_USB_HOST (3 << 1) /* USB host mode */
+#define MUX_USB_HOST_AND_DP_SRC (4 << 1) /* USB host + 2 lanes Display Port */
+#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
+#define MUX_USB_STATES (6 << 1)
+
struct device;
struct mux_control;
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines
2017-09-01 21:48 ` Hans de Goede
(?)
@ 2017-09-02 10:10 ` Andy Shevchenko
2017-09-02 11:59 ` Hans de Goede
-1 siblings, 1 reply; 52+ messages in thread
From: Andy Shevchenko @ 2017-09-02 10:10 UTC (permalink / raw)
To: Hans de Goede
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, USB
On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Add MUX_USB_* state constant defines, which can be used by USB
> device/host and Type-C polarity/role/altmode mux drivers and consumers
> to ensure that they agree on the meaning of the mux_control_select()
> state argument.
> +/*
> + * Mux state values for USB muxes, used for both USB device/host role muxes
> + * as well as for Type-C polarity/role/altmode muxes.
> + *
> + * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
> + * inverted-polarity (Type-C plugged in upside down) can happen with any
> + * other mux-state.
> + */
> +#define MUX_USB_POLARITY_INV BIT(0) /* Polarity inverted bit */
> +#define MUX_USB_NONE (1 << 1) /* Mux open / not connected */
> +#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
> +#define MUX_USB_HOST (3 << 1) /* USB host mode */
> +#define MUX_USB_HOST_AND_DP_SRC (4 << 1) /* USB host + 2 lanes Display Port */
> +#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
> +#define MUX_USB_STATES (6 << 1)
I would put OR'ed bits higher.
Like allocate 4 (8) bits for states and start special flagst from bit
8 and so on.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines
2017-09-02 10:10 ` Andy Shevchenko
@ 2017-09-02 11:59 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-02 11:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, USB
Hi,
On 02-09-17 12:10, Andy Shevchenko wrote:
> On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Add MUX_USB_* state constant defines, which can be used by USB
>> device/host and Type-C polarity/role/altmode mux drivers and consumers
>> to ensure that they agree on the meaning of the mux_control_select()
>> state argument.
>
>> +/*
>> + * Mux state values for USB muxes, used for both USB device/host role muxes
>> + * as well as for Type-C polarity/role/altmode muxes.
>> + *
>> + * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
>> + * inverted-polarity (Type-C plugged in upside down) can happen with any
>> + * other mux-state.
>> + */
>> +#define MUX_USB_POLARITY_INV BIT(0) /* Polarity inverted bit */
>> +#define MUX_USB_NONE (1 << 1) /* Mux open / not connected */
>> +#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
>> +#define MUX_USB_HOST (3 << 1) /* USB host mode */
>> +#define MUX_USB_HOST_AND_DP_SRC (4 << 1) /* USB host + 2 lanes Display Port */
>> +#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
>> +#define MUX_USB_STATES (6 << 1)
>
> I would put OR'ed bits higher.
>
> Like allocate 4 (8) bits for states and start special flags from bit
> 8 and so on.
That is not an option because the mux framework expects a mux to declare
how much states it has (which is where the MUX_USB_STATES define comes in)
and any attemp to set a state higher then max_states will return with
-EINVAL.
Regards,
Hans
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines
2017-09-01 21:48 ` Hans de Goede
@ 2017-09-02 14:59 ` Guenter Roeck
-1 siblings, 0 replies; 52+ messages in thread
From: Guenter Roeck @ 2017-09-02 14:59 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan
On 09/01/2017 02:48 PM, Hans de Goede wrote:
> Add MUX_USB_* state constant defines, which can be used by USB
> device/host and Type-C polarity/role/altmode mux drivers and consumers
> to ensure that they agree on the meaning of the mux_control_select()
> state argument.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> include/linux/mux/consumer.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index 912dd48a3a5d..e3ec9b4db962 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -15,6 +15,22 @@
>
> #include <linux/compiler.h>
>
> +/*
> + * Mux state values for USB muxes, used for both USB device/host role muxes
> + * as well as for Type-C polarity/role/altmode muxes.
> + *
> + * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
> + * inverted-polarity (Type-C plugged in upside down) can happen with any
> + * other mux-state.
> + */
> +#define MUX_USB_POLARITY_INV BIT(0) /* Polarity inverted bit */
> +#define MUX_USB_NONE (1 << 1) /* Mux open / not connected */
Why BIT(0) but (1 << 1) and so on ?
Guenter
> +#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
> +#define MUX_USB_HOST (3 << 1) /* USB host mode */
> +#define MUX_USB_HOST_AND_DP_SRC (4 << 1) /* USB host + 2 lanes Display Port */
> +#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
> +#define MUX_USB_STATES (6 << 1)
> +
> struct device;
> struct mux_control;
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines
@ 2017-09-02 14:59 ` Guenter Roeck
0 siblings, 0 replies; 52+ messages in thread
From: Guenter Roeck @ 2017-09-02 14:59 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
On 09/01/2017 02:48 PM, Hans de Goede wrote:
> Add MUX_USB_* state constant defines, which can be used by USB
> device/host and Type-C polarity/role/altmode mux drivers and consumers
> to ensure that they agree on the meaning of the mux_control_select()
> state argument.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> include/linux/mux/consumer.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index 912dd48a3a5d..e3ec9b4db962 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -15,6 +15,22 @@
>
> #include <linux/compiler.h>
>
> +/*
> + * Mux state values for USB muxes, used for both USB device/host role muxes
> + * as well as for Type-C polarity/role/altmode muxes.
> + *
> + * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
> + * inverted-polarity (Type-C plugged in upside down) can happen with any
> + * other mux-state.
> + */
> +#define MUX_USB_POLARITY_INV BIT(0) /* Polarity inverted bit */
> +#define MUX_USB_NONE (1 << 1) /* Mux open / not connected */
Why BIT(0) but (1 << 1) and so on ?
Guenter
> +#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
> +#define MUX_USB_HOST (3 << 1) /* USB host mode */
> +#define MUX_USB_HOST_AND_DP_SRC (4 << 1) /* USB host + 2 lanes Display Port */
> +#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
> +#define MUX_USB_STATES (6 << 1)
> +
> struct device;
> struct mux_control;
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines
2017-09-02 14:59 ` Guenter Roeck
(?)
@ 2017-09-02 15:59 ` Hans de Goede
[not found] ` <3e777501-aa94-70ae-3dc5-f64a15fd9704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
-1 siblings, 1 reply; 52+ messages in thread
From: Hans de Goede @ 2017-09-02 15:59 UTC (permalink / raw)
To: Guenter Roeck, MyungJoo Ham, Chanwoo Choi, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
Hi,
On 02-09-17 16:59, Guenter Roeck wrote:
> On 09/01/2017 02:48 PM, Hans de Goede wrote:
>> Add MUX_USB_* state constant defines, which can be used by USB
>> device/host and Type-C polarity/role/altmode mux drivers and consumers
>> to ensure that they agree on the meaning of the mux_control_select()
>> state argument.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> include/linux/mux/consumer.h | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
>> index 912dd48a3a5d..e3ec9b4db962 100644
>> --- a/include/linux/mux/consumer.h
>> +++ b/include/linux/mux/consumer.h
>> @@ -15,6 +15,22 @@
>> #include <linux/compiler.h>
>> +/*
>> + * Mux state values for USB muxes, used for both USB device/host role muxes
>> + * as well as for Type-C polarity/role/altmode muxes.
>> + *
>> + * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
>> + * inverted-polarity (Type-C plugged in upside down) can happen with any
>> + * other mux-state.
>> + */
>> +#define MUX_USB_POLARITY_INV BIT(0) /* Polarity inverted bit */
>> +#define MUX_USB_NONE (1 << 1) /* Mux open / not connected */
>
>
> Why BIT(0) but (1 << 1) and so on ?
Because the polarity can be or-ed together with any of the other options.
Each option can be selected normal and inverted polarity (connector
inserted upside down).
Regards,
Hans
>> +#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
>> +#define MUX_USB_HOST (3 << 1) /* USB host mode */
>> +#define MUX_USB_HOST_AND_DP_SRC (4 << 1) /* USB host + 2 lanes Display Port */
>> +#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
>> +#define MUX_USB_STATES (6 << 1)
>> +
>> struct device;
>> struct mux_control;
>>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede
The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Check xHCI controller PCI device-id instead of only checking for the
Intel Extended capability ID, as the Extended capability ID is used on
other model Intel xHCI controllers too
---
drivers/usb/host/Makefile | 2 +-
drivers/usb/host/xhci-intel-quirks.c | 85 ++++++++++++++++++++++++++++++++++++
drivers/usb/host/xhci-pci.c | 4 ++
drivers/usb/host/xhci.h | 2 +
4 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/host/xhci-intel-quirks.c
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691fffcc0..441edf82eb1c 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -63,7 +63,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
-obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
+obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-intel-quirks.o
obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o
obj-$(CONFIG_USB_XHCI_TEGRA) += xhci-tegra.o
diff --git a/drivers/usb/host/xhci-intel-quirks.c b/drivers/usb/host/xhci-intel-quirks.c
new file mode 100644
index 000000000000..819f5f9da9ee
--- /dev/null
+++ b/drivers/usb/host/xhci-intel-quirks.c
@@ -0,0 +1,85 @@
+/*
+ * Intel Vendor Defined XHCI extended capability handling
+ *
+ * Copyright (c) 2016) Hans de Goede <hdegoede@redhat.com>
+ *
+ * Loosely based on android x86 kernel code which is:
+ *
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Author: Wu, Hao
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;
+ */
+
+#include <linux/platform_device.h>
+#include "xhci.h"
+
+/* Extended capability IDs for Intel Vendor Defined */
+#define XHCI_EXT_CAPS_INTEL_HOST_CAP 192
+
+static void xhci_intel_unregister_pdev(void *arg)
+{
+ platform_device_unregister(arg);
+}
+
+int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci)
+{
+ struct usb_hcd *hcd = xhci_to_hcd(xhci);
+ struct device *dev = hcd->self.controller;
+ struct platform_device *pdev;
+ struct resource res = { 0, };
+ int ret, ext_offset;
+
+ ext_offset = xhci_find_next_ext_cap(&xhci->cap_regs->hc_capbase, 0,
+ XHCI_EXT_CAPS_INTEL_HOST_CAP);
+ if (!ext_offset) {
+ xhci_err(xhci, "couldn't find Intel ext caps\n");
+ return -ENODEV;
+ }
+
+ pdev = platform_device_alloc("intel_cht_usb_mux", PLATFORM_DEVID_NONE);
+ if (!pdev) {
+ xhci_err(xhci, "couldn't allocate intel_cht_usb_mux pdev\n");
+ return -ENOMEM;
+ }
+
+ res.start = hcd->rsrc_start + ext_offset;
+ res.end = res.start + 0x3ff;
+ res.name = "intel_cht_usb_mux";
+ res.flags = IORESOURCE_MEM;
+
+ ret = platform_device_add_resources(pdev, &res, 1);
+ if (ret) {
+ dev_err(dev, "couldn't add resources to intel_cht_usb_mux pdev\n");
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ pdev->dev.parent = dev;
+
+ ret = platform_device_add(pdev);
+ if (ret) {
+ dev_err(dev, "couldn't register intel_cht_usb_mux pdev\n");
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
+ if (ret) {
+ dev_err(dev, "couldn't add unregister action for intel_cht_usb_mux pdev\n");
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 8071c8fdd15e..b55c1e96abf0 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -188,6 +188,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
+ xhci->quirks |= XHCI_INTEL_CHT_USB_MUX;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
@@ -328,6 +329,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
xhci_pme_acpi_rtd3_enable(dev);
+ if (xhci->quirks & XHCI_INTEL_CHT_USB_MUX)
+ xhci_create_intel_cht_mux_pdev(xhci);
+
/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
pm_runtime_put_noidle(&dev->dev);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e3e935291ed6..f722ee31e50d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1821,6 +1821,7 @@ struct xhci_hcd {
#define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
#define XHCI_U2_DISABLE_WAKE (1 << 27)
#define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
+#define XHCI_INTEL_CHT_USB_MUX (1 << 29)
unsigned int num_active_eps;
unsigned int limit_active_eps;
@@ -2005,6 +2006,7 @@ void xhci_init_driver(struct hc_driver *drv,
const struct xhci_driver_overrides *over);
int xhci_disable_slot(struct xhci_hcd *xhci,
struct xhci_command *command, u32 slot_id);
+int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci);
int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup);
int xhci_resume(struct xhci_hcd *xhci, bool hibernated);
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Check xHCI controller PCI device-id instead of only checking for the
Intel Extended capability ID, as the Extended capability ID is used on
other model Intel xHCI controllers too
---
drivers/usb/host/Makefile | 2 +-
drivers/usb/host/xhci-intel-quirks.c | 85 ++++++++++++++++++++++++++++++++++++
drivers/usb/host/xhci-pci.c | 4 ++
drivers/usb/host/xhci.h | 2 +
4 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/host/xhci-intel-quirks.c
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691fffcc0..441edf82eb1c 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -63,7 +63,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
-obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
+obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-intel-quirks.o
obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o
obj-$(CONFIG_USB_XHCI_TEGRA) += xhci-tegra.o
diff --git a/drivers/usb/host/xhci-intel-quirks.c b/drivers/usb/host/xhci-intel-quirks.c
new file mode 100644
index 000000000000..819f5f9da9ee
--- /dev/null
+++ b/drivers/usb/host/xhci-intel-quirks.c
@@ -0,0 +1,85 @@
+/*
+ * Intel Vendor Defined XHCI extended capability handling
+ *
+ * Copyright (c) 2016) Hans de Goede <hdegoede@redhat.com>
+ *
+ * Loosely based on android x86 kernel code which is:
+ *
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Author: Wu, Hao
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;
+ */
+
+#include <linux/platform_device.h>
+#include "xhci.h"
+
+/* Extended capability IDs for Intel Vendor Defined */
+#define XHCI_EXT_CAPS_INTEL_HOST_CAP 192
+
+static void xhci_intel_unregister_pdev(void *arg)
+{
+ platform_device_unregister(arg);
+}
+
+int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci)
+{
+ struct usb_hcd *hcd = xhci_to_hcd(xhci);
+ struct device *dev = hcd->self.controller;
+ struct platform_device *pdev;
+ struct resource res = { 0, };
+ int ret, ext_offset;
+
+ ext_offset = xhci_find_next_ext_cap(&xhci->cap_regs->hc_capbase, 0,
+ XHCI_EXT_CAPS_INTEL_HOST_CAP);
+ if (!ext_offset) {
+ xhci_err(xhci, "couldn't find Intel ext caps\n");
+ return -ENODEV;
+ }
+
+ pdev = platform_device_alloc("intel_cht_usb_mux", PLATFORM_DEVID_NONE);
+ if (!pdev) {
+ xhci_err(xhci, "couldn't allocate intel_cht_usb_mux pdev\n");
+ return -ENOMEM;
+ }
+
+ res.start = hcd->rsrc_start + ext_offset;
+ res.end = res.start + 0x3ff;
+ res.name = "intel_cht_usb_mux";
+ res.flags = IORESOURCE_MEM;
+
+ ret = platform_device_add_resources(pdev, &res, 1);
+ if (ret) {
+ dev_err(dev, "couldn't add resources to intel_cht_usb_mux pdev\n");
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ pdev->dev.parent = dev;
+
+ ret = platform_device_add(pdev);
+ if (ret) {
+ dev_err(dev, "couldn't register intel_cht_usb_mux pdev\n");
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
+ if (ret) {
+ dev_err(dev, "couldn't add unregister action for intel_cht_usb_mux pdev\n");
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 8071c8fdd15e..b55c1e96abf0 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -188,6 +188,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
+ xhci->quirks |= XHCI_INTEL_CHT_USB_MUX;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
@@ -328,6 +329,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
xhci_pme_acpi_rtd3_enable(dev);
+ if (xhci->quirks & XHCI_INTEL_CHT_USB_MUX)
+ xhci_create_intel_cht_mux_pdev(xhci);
+
/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
pm_runtime_put_noidle(&dev->dev);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e3e935291ed6..f722ee31e50d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1821,6 +1821,7 @@ struct xhci_hcd {
#define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
#define XHCI_U2_DISABLE_WAKE (1 << 27)
#define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
+#define XHCI_INTEL_CHT_USB_MUX (1 << 29)
unsigned int num_active_eps;
unsigned int limit_active_eps;
@@ -2005,6 +2006,7 @@ void xhci_init_driver(struct hc_driver *drv,
const struct xhci_driver_overrides *over);
int xhci_disable_slot(struct xhci_hcd *xhci,
struct xhci_command *command, u32 slot_id);
+int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci);
int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup);
int xhci_resume(struct xhci_hcd *xhci, bool hibernated);
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [PATCH 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling
2017-09-01 21:48 ` Hans de Goede
(?)
@ 2017-09-04 7:31 ` Heikki Krogerus
2017-09-05 10:06 ` Hans de Goede
-1 siblings, 1 reply; 52+ messages in thread
From: Heikki Krogerus @ 2017-09-04 7:31 UTC (permalink / raw)
To: Hans de Goede
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Darren Hart,
Andy Shevchenko, Peter Rosin, Mathias Nyman, platform-driver-x86,
devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
Hi,
On Fri, Sep 01, 2017 at 11:48:38PM +0200, Hans de Goede wrote:
> The Intel cherrytrail xhci controller has an extended cap mmio-range
> which contains registers to control the muxing to the xhci (host mode)
> or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
>
> Having a mux driver included in the xhci code (or under drivers/usb/host)
> is not desirable. So this commit adds a simple handler for this extended
> capability, which creates a platform device with the caps mmio region as
> resource, this allows us to write a separate platform mux driver for the
> mux.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Check xHCI controller PCI device-id instead of only checking for the
> Intel Extended capability ID, as the Extended capability ID is used on
> other model Intel xHCI controllers too
> ---
> drivers/usb/host/Makefile | 2 +-
> drivers/usb/host/xhci-intel-quirks.c | 85 ++++++++++++++++++++++++++++++++++++
> drivers/usb/host/xhci-pci.c | 4 ++
> drivers/usb/host/xhci.h | 2 +
> 4 files changed, 92 insertions(+), 1 deletion(-)
> create mode 100644 drivers/usb/host/xhci-intel-quirks.c
>
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index cf2691fffcc0..441edf82eb1c 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -63,7 +63,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
> obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
> -obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
> +obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-intel-quirks.o
> obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
> obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o
> obj-$(CONFIG_USB_XHCI_TEGRA) += xhci-tegra.o
> diff --git a/drivers/usb/host/xhci-intel-quirks.c b/drivers/usb/host/xhci-intel-quirks.c
> new file mode 100644
> index 000000000000..819f5f9da9ee
> --- /dev/null
> +++ b/drivers/usb/host/xhci-intel-quirks.c
> @@ -0,0 +1,85 @@
> +/*
> + * Intel Vendor Defined XHCI extended capability handling
> + *
> + * Copyright (c) 2016) Hans de Goede <hdegoede@redhat.com>
> + *
> + * Loosely based on android x86 kernel code which is:
> + *
> + * Copyright (C) 2014 Intel Corp.
> + *
> + * Author: Wu, Hao
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program;
> + */
> +
> +#include <linux/platform_device.h>
> +#include "xhci.h"
> +
> +/* Extended capability IDs for Intel Vendor Defined */
> +#define XHCI_EXT_CAPS_INTEL_HOST_CAP 192
> +
> +static void xhci_intel_unregister_pdev(void *arg)
> +{
> + platform_device_unregister(arg);
> +}
> +
> +int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci)
> +{
> + struct usb_hcd *hcd = xhci_to_hcd(xhci);
> + struct device *dev = hcd->self.controller;
> + struct platform_device *pdev;
> + struct resource res = { 0, };
> + int ret, ext_offset;
> +
> + ext_offset = xhci_find_next_ext_cap(&xhci->cap_regs->hc_capbase, 0,
> + XHCI_EXT_CAPS_INTEL_HOST_CAP);
> + if (!ext_offset) {
> + xhci_err(xhci, "couldn't find Intel ext caps\n");
> + return -ENODEV;
> + }
> +
> + pdev = platform_device_alloc("intel_cht_usb_mux", PLATFORM_DEVID_NONE);
> + if (!pdev) {
> + xhci_err(xhci, "couldn't allocate intel_cht_usb_mux pdev\n");
> + return -ENOMEM;
> + }
> +
> + res.start = hcd->rsrc_start + ext_offset;
> + res.end = res.start + 0x3ff;
> + res.name = "intel_cht_usb_mux";
> + res.flags = IORESOURCE_MEM;
> +
> + ret = platform_device_add_resources(pdev, &res, 1);
> + if (ret) {
> + dev_err(dev, "couldn't add resources to intel_cht_usb_mux pdev\n");
> + platform_device_put(pdev);
> + return ret;
> + }
Previously the problem with this was that since platform bus reserves
the memory region, usb core fails to register the hcd, as it also
wants to reserve the same memory region. So xhci with this failed to
probe.
So has that been fixed? Does xhci really get registered with this?
> + pdev->dev.parent = dev;
> +
> + ret = platform_device_add(pdev);
> + if (ret) {
> + dev_err(dev, "couldn't register intel_cht_usb_mux pdev\n");
> + platform_device_put(pdev);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
> + if (ret) {
> + dev_err(dev, "couldn't add unregister action for intel_cht_usb_mux pdev\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 8071c8fdd15e..b55c1e96abf0 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -188,6 +188,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
> xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
> + xhci->quirks |= XHCI_INTEL_CHT_USB_MUX;
> }
> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
> @@ -328,6 +329,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
> xhci_pme_acpi_rtd3_enable(dev);
>
> + if (xhci->quirks & XHCI_INTEL_CHT_USB_MUX)
> + xhci_create_intel_cht_mux_pdev(xhci);
> +
> /* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
> pm_runtime_put_noidle(&dev->dev);
>
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index e3e935291ed6..f722ee31e50d 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1821,6 +1821,7 @@ struct xhci_hcd {
> #define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
> #define XHCI_U2_DISABLE_WAKE (1 << 27)
> #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
> +#define XHCI_INTEL_CHT_USB_MUX (1 << 29)
Is that really necessary? Why not just register the platform device
the moment we find match for the PCI device ID?
Br,
--
heikki
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling
2017-09-04 7:31 ` Heikki Krogerus
@ 2017-09-05 10:06 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-05 10:06 UTC (permalink / raw)
To: Heikki Krogerus
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Darren Hart,
Andy Shevchenko, Peter Rosin, Mathias Nyman, platform-driver-x86,
devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
Hi,
On 04-09-17 09:31, Heikki Krogerus wrote:
> Hi,
>
> On Fri, Sep 01, 2017 at 11:48:38PM +0200, Hans de Goede wrote:
>> The Intel cherrytrail xhci controller has an extended cap mmio-range
>> which contains registers to control the muxing to the xhci (host mode)
>> or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
>>
>> Having a mux driver included in the xhci code (or under drivers/usb/host)
>> is not desirable. So this commit adds a simple handler for this extended
>> capability, which creates a platform device with the caps mmio region as
>> resource, this allows us to write a separate platform mux driver for the
>> mux.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Check xHCI controller PCI device-id instead of only checking for the
>> Intel Extended capability ID, as the Extended capability ID is used on
>> other model Intel xHCI controllers too
>> ---
>> drivers/usb/host/Makefile | 2 +-
>> drivers/usb/host/xhci-intel-quirks.c | 85 ++++++++++++++++++++++++++++++++++++
>> drivers/usb/host/xhci-pci.c | 4 ++
>> drivers/usb/host/xhci.h | 2 +
>> 4 files changed, 92 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/usb/host/xhci-intel-quirks.c
>>
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index cf2691fffcc0..441edf82eb1c 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -63,7 +63,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
>> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
>> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
>> obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
>> -obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
>> +obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-intel-quirks.o
>> obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
>> obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o
>> obj-$(CONFIG_USB_XHCI_TEGRA) += xhci-tegra.o
>> diff --git a/drivers/usb/host/xhci-intel-quirks.c b/drivers/usb/host/xhci-intel-quirks.c
>> new file mode 100644
>> index 000000000000..819f5f9da9ee
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-intel-quirks.c
>> @@ -0,0 +1,85 @@
>> +/*
>> + * Intel Vendor Defined XHCI extended capability handling
>> + *
>> + * Copyright (c) 2016) Hans de Goede <hdegoede@redhat.com>
>> + *
>> + * Loosely based on android x86 kernel code which is:
>> + *
>> + * Copyright (C) 2014 Intel Corp.
>> + *
>> + * Author: Wu, Hao
>> + *
>> + * 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.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>> + * for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program;
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include "xhci.h"
>> +
>> +/* Extended capability IDs for Intel Vendor Defined */
>> +#define XHCI_EXT_CAPS_INTEL_HOST_CAP 192
>> +
>> +static void xhci_intel_unregister_pdev(void *arg)
>> +{
>> + platform_device_unregister(arg);
>> +}
>> +
>> +int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci)
>> +{
>> + struct usb_hcd *hcd = xhci_to_hcd(xhci);
>> + struct device *dev = hcd->self.controller;
>> + struct platform_device *pdev;
>> + struct resource res = { 0, };
>> + int ret, ext_offset;
>> +
>> + ext_offset = xhci_find_next_ext_cap(&xhci->cap_regs->hc_capbase, 0,
>> + XHCI_EXT_CAPS_INTEL_HOST_CAP);
>> + if (!ext_offset) {
>> + xhci_err(xhci, "couldn't find Intel ext caps\n");
>> + return -ENODEV;
>> + }
>> +
>> + pdev = platform_device_alloc("intel_cht_usb_mux", PLATFORM_DEVID_NONE);
>> + if (!pdev) {
>> + xhci_err(xhci, "couldn't allocate intel_cht_usb_mux pdev\n");
>> + return -ENOMEM;
>> + }
>> +
>> + res.start = hcd->rsrc_start + ext_offset;
>> + res.end = res.start + 0x3ff;
>> + res.name = "intel_cht_usb_mux";
>> + res.flags = IORESOURCE_MEM;
>> +
>> + ret = platform_device_add_resources(pdev, &res, 1);
>> + if (ret) {
>> + dev_err(dev, "couldn't add resources to intel_cht_usb_mux pdev\n");
>> + platform_device_put(pdev);
>> + return ret;
>> + }
>
> Previously the problem with this was that since platform bus reserves
> the memory region, usb core fails to register the hcd, as it also
> wants to reserve the same memory region. So xhci with this failed to
> probe.
>
> So has that been fixed? Does xhci really get registered with this?
This is not a problem if you set the xhci device as parent:
>> + pdev->dev.parent = dev;
And yes both the xhci and the intel_cht_usb_mux devices get registered
and work fine:
[root@localhost ~]# cat /proc/iomem
<snip>
80000000-dfffffff : PCI Bus 0000:00
<snip>
a1c00000-a1c0ffff : 0000:00:14.0
a1c00000-a1c0ffff : xhci-hcd
a1c08070-a1c0846f : intel_cht_usb_mux
[root@localhost ~]# lsusb -t
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 5000M
|__ Port 4: Dev 2, If 0, Class=Mass Storage, Driver=uas, 5000M
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/7p, 480M
|__ Port 2: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 1.5M
|__ Port 2: Dev 2, If 1, Class=Human Interface Device, Driver=usbhid, 1.5M
|__ Port 3: Dev 3, If 0, Class=Vendor Specific Class, Driver=btusb, 12M
|__ Port 3: Dev 3, If 1, Class=Vendor Specific Class, Driver=btusb, 12M
|__ Port 3: Dev 3, If 2, Class=Vendor Specific Class, Driver=btusb, 12M
|__ Port 3: Dev 3, If 3, Class=Application Specific Interface, Driver=, 12M
>> +
>> + ret = platform_device_add(pdev);
>> + if (ret) {
>> + dev_err(dev, "couldn't register intel_cht_usb_mux pdev\n");
>> + platform_device_put(pdev);
>> + return ret;
>> + }
>> +
>> + ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
>> + if (ret) {
>> + dev_err(dev, "couldn't add unregister action for intel_cht_usb_mux pdev\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 8071c8fdd15e..b55c1e96abf0 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -188,6 +188,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>> pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
>> xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
>> + xhci->quirks |= XHCI_INTEL_CHT_USB_MUX;
>> }
>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>> (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
>> @@ -328,6 +329,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>> if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
>> xhci_pme_acpi_rtd3_enable(dev);
>>
>> + if (xhci->quirks & XHCI_INTEL_CHT_USB_MUX)
>> + xhci_create_intel_cht_mux_pdev(xhci);
>> +
>> /* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
>> pm_runtime_put_noidle(&dev->dev);
>>
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index e3e935291ed6..f722ee31e50d 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -1821,6 +1821,7 @@ struct xhci_hcd {
>> #define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
>> #define XHCI_U2_DISABLE_WAKE (1 << 27)
>> #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
>> +#define XHCI_INTEL_CHT_USB_MUX (1 << 29)
>
> Is that really necessary? Why not just register the platform device
> the moment we find match for the PCI device ID?
That is possible, but not how the rest of the xhci-pci.c code
works in general, all PCI-id checking is done in xhci_pci_quirks()
and that function only sets quirks flags otherwise and then those
quirks are handled in various other places such as probe() so
without adding this flag one would get a PCI-id check outside
of xhci_pci_quirks() or code which does more then just setting
a quirk flag inside of xhci_pci_quirks(), either of which deviates
from the current code-patterns for the xhci code.
Regards,
Hans
>
>
> Br,
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
` (3 preceding siblings ...)
2017-09-01 21:48 ` Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
[not found] ` <20170901214845.7153-6-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-09-04 11:19 ` Peter Rosin
2017-09-01 21:48 ` Hans de Goede
` (6 subsequent siblings)
11 siblings, 2 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port
USB data lines between the xHCI host controller and the dwc3 gadget
controller. On some Cherrytrail systems this mux is controlled through
AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through
an _AIE ACPI method) so things just work.
But on other Cherrytrail systems we need to control the mux ourselves
this driver exports the mux through the mux subsys, so that other drivers
can control it if necessary.
This driver also updates the vbus-valid reporting to the dwc3 gadget
controller, as this uses the same registers as the mux. This is needed
for gadget/device mode to work properly (even on systems which control
the mux from their AML code).
Note this depends on the xhci driver registering a platform device
named "intel_cht_usb_mux", which has an IOMEM resource 0 which points
to the Intel Vendor Defined XHCI extended capabilities region.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Complete rewrite as a stand-alone platform-driver rather then as a phy
driver, since this is just a mux, not a phy
Changes in v3:
-Make this a mux subsys driver instead of listening to USB_HOST extcon
cable events and responding to those
---
drivers/mux/Kconfig | 11 ++
drivers/mux/Makefile | 2 +
drivers/mux/intel_cht_usb_mux.c | 269 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 282 insertions(+)
create mode 100644 drivers/mux/intel_cht_usb_mux.c
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index 19e4e904c9bf..17938918bf93 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -34,6 +34,17 @@ config MUX_GPIO
To compile the driver as a module, choose M here: the module will
be called mux-gpio.
+config MUX_CHT_USB_MUX
+ tristate "Intel Cherrytrail USB Multiplexer"
+ depends on ACPI && X86 && EXTCON
+ help
+ This driver adds support for the internal USB mux for muxing the OTG
+ USB data lines between the xHCI host controller and the dwc3 gadget
+ controller found on Intel Cherrytrail SoCs.
+
+ To compile the driver as a module, choose M here: the module will
+ be called mux-intel_cht_usb_mux.
+
config MUX_MMIO
tristate "MMIO register bitfield-controlled Multiplexer"
depends on (OF && MFD_SYSCON) || COMPILE_TEST
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index 0e1e59760e3f..a12e812c7966 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -6,8 +6,10 @@ mux-core-objs := core.o
mux-adg792a-objs := adg792a.o
mux-gpio-objs := gpio.o
mux-mmio-objs := mmio.o
+mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
obj-$(CONFIG_MULTIPLEXER) += mux-core.o
obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
+obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
diff --git a/drivers/mux/intel_cht_usb_mux.c b/drivers/mux/intel_cht_usb_mux.c
new file mode 100644
index 000000000000..7b1621a081d8
--- /dev/null
+++ b/drivers/mux/intel_cht_usb_mux.c
@@ -0,0 +1,269 @@
+/*
+ * Intel Cherrytrail USB OTG MUX driver
+ *
+ * Copyright (c) 2016 Hans de Goede <hdegoede@redhat.com>
+ *
+ * Loosely based on android x86 kernel code which is:
+ *
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Author: Wu, Hao
+ *
+ * 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, or (at your option)
+ * any later version.
+ */
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
+#include <linux/mux/driver.h>
+#include <linux/platform_device.h>
+#include <linux/workqueue.h>
+
+/* register definition */
+#define DUAL_ROLE_CFG0 0x68
+#define SW_VBUS_VALID (1 << 24)
+#define SW_IDPIN_EN (1 << 21)
+#define SW_IDPIN (1 << 20)
+
+#define DUAL_ROLE_CFG1 0x6c
+#define HOST_MODE (1 << 29)
+
+#define DUAL_ROLE_CFG1_POLL_TIMEOUT 1000
+
+#define DRV_NAME "intel_cht_usb_mux"
+
+struct intel_cht_usb_mux {
+ struct mutex cfg0_lock;
+ void __iomem *base;
+ struct extcon_dev *vbus_extcon;
+ struct notifier_block vbus_nb;
+ struct work_struct vbus_work;
+};
+
+struct intel_cht_extcon_info {
+ const char *hid;
+ int hrv;
+ const char *extcon;
+};
+
+struct intel_cht_extcon_info vbus_providers[] = {
+ { "INT33F4", -1, "axp288_extcon" },
+ { "INT34D3", 3, "cht_wcove_pwrsrc" },
+};
+
+static const unsigned int vbus_cable_ids[] = {
+ EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP,
+ EXTCON_CHG_USB_ACA, EXTCON_CHG_USB_FAST,
+};
+
+static void intel_cht_usb_mux_set_sw_mode(struct intel_cht_usb_mux *mux)
+{
+ u32 data;
+
+ data = readl(mux->base + DUAL_ROLE_CFG0);
+ if (!(data & SW_IDPIN_EN)) {
+ data |= SW_IDPIN_EN;
+ writel(data, mux->base + DUAL_ROLE_CFG0);
+ }
+}
+
+static int intel_cht_usb_mux_set_mux(struct mux_control *mux_ctrl, int state)
+{
+ struct intel_cht_usb_mux *mux = mux_chip_priv(mux_ctrl->chip);
+ unsigned long timeout;
+ bool host_mode;
+ u32 data;
+
+ mutex_lock(&mux->cfg0_lock);
+
+ intel_cht_usb_mux_set_sw_mode(mux);
+
+ /* Set idpin value as requested */
+ data = readl(mux->base + DUAL_ROLE_CFG0);
+ switch (state & ~MUX_USB_POLARITY_INV) {
+ case MUX_USB_NONE:
+ case MUX_USB_DEVICE:
+ data |= SW_IDPIN;
+ host_mode = false;
+ break;
+ default:
+ data &= ~SW_IDPIN;
+ host_mode = true;
+ }
+ writel(data, mux->base + DUAL_ROLE_CFG0);
+
+ mutex_unlock(&mux->cfg0_lock);
+
+ /* In most case it takes about 600ms to finish mode switching */
+ timeout = jiffies + msecs_to_jiffies(DUAL_ROLE_CFG1_POLL_TIMEOUT);
+
+ /* Polling on CFG1 register to confirm mode switch.*/
+ while (1) {
+ data = readl(mux->base + DUAL_ROLE_CFG1);
+ if (!!(data & HOST_MODE) == host_mode)
+ break;
+
+ /* Interval for polling is set to about 5 - 10 ms */
+ usleep_range(5000, 10000);
+
+ if (time_after(jiffies, timeout)) {
+ dev_warn(&mux_ctrl->chip->dev,
+ "Timeout waiting for mux to switch\n");
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static void intel_cht_usb_mux_set_vbus_valid(struct intel_cht_usb_mux *mux,
+ bool valid)
+{
+ u32 data;
+
+ mutex_lock(&mux->cfg0_lock);
+
+ intel_cht_usb_mux_set_sw_mode(mux);
+
+ data = readl(mux->base + DUAL_ROLE_CFG0);
+ if (valid)
+ data |= SW_VBUS_VALID;
+ else
+ data &= ~SW_VBUS_VALID;
+ writel(data, mux->base + DUAL_ROLE_CFG0);
+
+ mutex_unlock(&mux->cfg0_lock);
+}
+
+static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
+{
+ struct intel_cht_usb_mux *mux =
+ container_of(work, struct intel_cht_usb_mux, vbus_work);
+ bool vbus_present = false;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
+ if (extcon_get_state(mux->vbus_extcon, vbus_cable_ids[i]) > 0) {
+ vbus_present = true;
+ break;
+ }
+ }
+
+ intel_cht_usb_mux_set_vbus_valid(mux, vbus_present);
+}
+
+static int intel_cht_usb_mux_vbus_extcon_evt(struct notifier_block *nb,
+ unsigned long event, void *param)
+{
+ struct intel_cht_usb_mux *mux =
+ container_of(nb, struct intel_cht_usb_mux, vbus_nb);
+
+ schedule_work(&mux->vbus_work);
+
+ return NOTIFY_OK;
+}
+
+static const struct mux_control_ops intel_cht_usb_mux_ops = {
+ .set = intel_cht_usb_mux_set_mux,
+};
+
+static int intel_cht_usb_mux_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct intel_cht_usb_mux *mux;
+ struct mux_chip *mux_chip;
+ struct resource *res;
+ resource_size_t size;
+ int i, ret;
+
+ mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
+ if (IS_ERR(mux_chip))
+ return PTR_ERR(mux_chip);
+
+ mux_chip->ops = &intel_cht_usb_mux_ops;
+ mux_chip->mux[0].states = MUX_USB_STATES;
+ mux = mux_chip_priv(mux_chip);
+ mutex_init(&mux->cfg0_lock);
+
+ /*
+ * Besides controlling the mux we also need to control the vbus_valid
+ * flag for device/gadget mode to work properly. To do this we monitor
+ * the extcon interface exported by the PMIC drivers for the PMICs used
+ * with the Cherry Trail SoC.
+ *
+ * We try to get the extcon_dev before registering the mux as this
+ * may lead to us exiting with -EPROBE_DEFER.
+ */
+ for (i = 0 ; i < ARRAY_SIZE(vbus_providers); i++) {
+ if (!acpi_dev_present(vbus_providers[i].hid, NULL,
+ vbus_providers[i].hrv))
+ continue;
+
+ mux->vbus_extcon = extcon_get_extcon_dev(
+ vbus_providers[i].extcon);
+ if (mux->vbus_extcon == NULL)
+ return -EPROBE_DEFER;
+
+ dev_info(dev, "using extcon '%s' for vbus-valid\n",
+ vbus_providers[i].extcon);
+ break;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ size = (res->end + 1) - res->start;
+ mux->base = devm_ioremap_nocache(dev, res->start, size);
+ if (IS_ERR(mux->base)) {
+ ret = PTR_ERR(mux->base);
+ dev_err(dev, "can't iomap registers: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_mux_chip_register(dev, mux_chip);
+ if (ret < 0)
+ return ret;
+
+ if (mux->vbus_extcon) {
+ INIT_WORK(&mux->vbus_work, intel_cht_usb_mux_vbus_work);
+ mux->vbus_nb.notifier_call = intel_cht_usb_mux_vbus_extcon_evt;
+ ret = devm_extcon_register_notifier_all(dev, mux->vbus_extcon,
+ &mux->vbus_nb);
+ if (ret) {
+ dev_err(dev, "can't register vbus extcon notifier: %d\n",
+ ret);
+ return ret;
+ }
+
+ /* Sync initial mode */
+ schedule_work(&mux->vbus_work);
+ }
+
+ return 0;
+}
+
+static const struct platform_device_id intel_cht_usb_mux_table[] = {
+ { .name = DRV_NAME },
+ {},
+};
+MODULE_DEVICE_TABLE(platform, intel_cht_usb_mux_table);
+
+static struct platform_driver intel_cht_usb_mux_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ },
+ .id_table = intel_cht_usb_mux_table,
+ .probe = intel_cht_usb_mux_probe,
+};
+
+module_platform_driver(intel_cht_usb_mux_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Intel Cherrytrail USB mux driver");
+MODULE_LICENSE("GPL");
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread[parent not found: <20170901214845.7153-6-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
2017-09-01 21:48 ` [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver Hans de Goede
@ 2017-09-02 10:19 ` Andy Shevchenko
2017-09-04 11:19 ` Peter Rosin
1 sibling, 0 replies; 52+ messages in thread
From: Andy Shevchenko @ 2017-09-02 10:19 UTC (permalink / raw)
To: Hans de Goede
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Greg Kroah-Hartman, USB
On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port
> USB data lines between the xHCI host controller and the dwc3 gadget
> controller. On some Cherrytrail systems this mux is controlled through
> AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through
> an _AIE ACPI method) so things just work.
>
> But on other Cherrytrail systems we need to control the mux ourselves
> this driver exports the mux through the mux subsys, so that other drivers
> can control it if necessary.
>
> This driver also updates the vbus-valid reporting to the dwc3 gadget
> controller, as this uses the same registers as the mux. This is needed
> for gadget/device mode to work properly (even on systems which control
> the mux from their AML code).
>
> Note this depends on the xhci driver registering a platform device
> named "intel_cht_usb_mux", which has an IOMEM resource 0 which points
> to the Intel Vendor Defined XHCI extended capabilities region.
> +static void intel_cht_usb_mux_set_sw_mode(struct intel_cht_usb_mux *mux)
> +{
> + u32 data;
> +
> + data = readl(mux->base + DUAL_ROLE_CFG0);
> + if (!(data & SW_IDPIN_EN)) {
Do we need it? I think this kind of microoptimixzations not worth it
for most cases.
> + data |= SW_IDPIN_EN;
> + writel(data, mux->base + DUAL_ROLE_CFG0);
> + }
> +}
> + /* In most case it takes about 600ms to finish mode switching */
> + timeout = jiffies + msecs_to_jiffies(DUAL_ROLE_CFG1_POLL_TIMEOUT);
> +
> + /* Polling on CFG1 register to confirm mode switch.*/
> + while (1) {
do {} while (time_before()); ?
> + data = readl(mux->base + DUAL_ROLE_CFG1);
> + if (!!(data & HOST_MODE) == host_mode)
> + break;
> +
> + /* Interval for polling is set to about 5 - 10 ms */
> + usleep_range(5000, 10000);
> +
> + if (time_after(jiffies, timeout)) {
> + dev_warn(&mux_ctrl->chip->dev,
> + "Timeout waiting for mux to switch\n");
> + break;
> + }
> + }
> +static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
> +{
> + struct intel_cht_usb_mux *mux =
> + container_of(work, struct intel_cht_usb_mux, vbus_work);
> + bool vbus_present = false;
> + int i;
unsigned int i; ?
> +
> + for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
> + if (extcon_get_state(mux->vbus_extcon, vbus_cable_ids[i]) > 0) {
> + vbus_present = true;
> + break;
> + }
> + }
> +
> + intel_cht_usb_mux_set_vbus_valid(mux, vbus_present);
> +}
> +static int intel_cht_usb_mux_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct intel_cht_usb_mux *mux;
> + struct mux_chip *mux_chip;
> + struct resource *res;
> + resource_size_t size;
> + int i, ret;
Ditto.
> + for (i = 0 ; i < ARRAY_SIZE(vbus_providers); i++) {
> + if (!acpi_dev_present(vbus_providers[i].hid, NULL,
> + vbus_providers[i].hrv))
> + continue;
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
@ 2017-09-02 10:19 ` Andy Shevchenko
0 siblings, 0 replies; 52+ messages in thread
From: Andy Shevchenko @ 2017-09-02 10:19 UTC (permalink / raw)
To: Hans de Goede
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, USB
On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port
> USB data lines between the xHCI host controller and the dwc3 gadget
> controller. On some Cherrytrail systems this mux is controlled through
> AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through
> an _AIE ACPI method) so things just work.
>
> But on other Cherrytrail systems we need to control the mux ourselves
> this driver exports the mux through the mux subsys, so that other drivers
> can control it if necessary.
>
> This driver also updates the vbus-valid reporting to the dwc3 gadget
> controller, as this uses the same registers as the mux. This is needed
> for gadget/device mode to work properly (even on systems which control
> the mux from their AML code).
>
> Note this depends on the xhci driver registering a platform device
> named "intel_cht_usb_mux", which has an IOMEM resource 0 which points
> to the Intel Vendor Defined XHCI extended capabilities region.
> +static void intel_cht_usb_mux_set_sw_mode(struct intel_cht_usb_mux *mux)
> +{
> + u32 data;
> +
> + data = readl(mux->base + DUAL_ROLE_CFG0);
> + if (!(data & SW_IDPIN_EN)) {
Do we need it? I think this kind of microoptimixzations not worth it
for most cases.
> + data |= SW_IDPIN_EN;
> + writel(data, mux->base + DUAL_ROLE_CFG0);
> + }
> +}
> + /* In most case it takes about 600ms to finish mode switching */
> + timeout = jiffies + msecs_to_jiffies(DUAL_ROLE_CFG1_POLL_TIMEOUT);
> +
> + /* Polling on CFG1 register to confirm mode switch.*/
> + while (1) {
do {} while (time_before()); ?
> + data = readl(mux->base + DUAL_ROLE_CFG1);
> + if (!!(data & HOST_MODE) == host_mode)
> + break;
> +
> + /* Interval for polling is set to about 5 - 10 ms */
> + usleep_range(5000, 10000);
> +
> + if (time_after(jiffies, timeout)) {
> + dev_warn(&mux_ctrl->chip->dev,
> + "Timeout waiting for mux to switch\n");
> + break;
> + }
> + }
> +static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
> +{
> + struct intel_cht_usb_mux *mux =
> + container_of(work, struct intel_cht_usb_mux, vbus_work);
> + bool vbus_present = false;
> + int i;
unsigned int i; ?
> +
> + for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
> + if (extcon_get_state(mux->vbus_extcon, vbus_cable_ids[i]) > 0) {
> + vbus_present = true;
> + break;
> + }
> + }
> +
> + intel_cht_usb_mux_set_vbus_valid(mux, vbus_present);
> +}
> +static int intel_cht_usb_mux_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct intel_cht_usb_mux *mux;
> + struct mux_chip *mux_chip;
> + struct resource *res;
> + resource_size_t size;
> + int i, ret;
Ditto.
> + for (i = 0 ; i < ARRAY_SIZE(vbus_providers); i++) {
> + if (!acpi_dev_present(vbus_providers[i].hid, NULL,
> + vbus_providers[i].hrv))
> + continue;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
2017-09-02 10:19 ` Andy Shevchenko
(?)
@ 2017-09-02 10:37 ` Dan Carpenter
-1 siblings, 0 replies; 52+ messages in thread
From: Dan Carpenter @ 2017-09-02 10:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Hans de Goede, devel, Kuppuswamy Sathyanarayanan, Heikki Krogerus,
Mathias Nyman, Greg Kroah-Hartman, USB,
linux-kernel@vger.kernel.org, Platform Driver,
Sathyanarayanan Kuppuswamy Natarajan, Chanwoo Choi, MyungJoo Ham,
Guenter Roeck, Darren Hart, Peter Rosin, Andy Shevchenko
On Sat, Sep 02, 2017 at 01:19:05PM +0300, Andy Shevchenko wrote:
> > +static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
> > +{
> > + struct intel_cht_usb_mux *mux =
> > + container_of(work, struct intel_cht_usb_mux, vbus_work);
> > + bool vbus_present = false;
> > + int i;
>
> unsigned int i; ?
>
int i is fine. Static checkers which complain about stuff like that
should be uninstalled.
> > +
> > + for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
regards,
dan carpenter
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
2017-09-02 10:19 ` Andy Shevchenko
(?)
(?)
@ 2017-09-04 14:07 ` Hans de Goede
-1 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-04 14:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, USB
Hi,
Thank you for the reviews!
On 02-09-17 12:19, Andy Shevchenko wrote:
> On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port
>> USB data lines between the xHCI host controller and the dwc3 gadget
>> controller. On some Cherrytrail systems this mux is controlled through
>> AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through
>> an _AIE ACPI method) so things just work.
>>
>> But on other Cherrytrail systems we need to control the mux ourselves
>> this driver exports the mux through the mux subsys, so that other drivers
>> can control it if necessary.
>>
>> This driver also updates the vbus-valid reporting to the dwc3 gadget
>> controller, as this uses the same registers as the mux. This is needed
>> for gadget/device mode to work properly (even on systems which control
>> the mux from their AML code).
>>
>> Note this depends on the xhci driver registering a platform device
>> named "intel_cht_usb_mux", which has an IOMEM resource 0 which points
>> to the Intel Vendor Defined XHCI extended capabilities region.
>
>> +static void intel_cht_usb_mux_set_sw_mode(struct intel_cht_usb_mux *mux)
>> +{
>> + u32 data;
>> +
>> + data = readl(mux->base + DUAL_ROLE_CFG0);
>
>> + if (!(data & SW_IDPIN_EN)) {
>
> Do we need it? I think this kind of microoptimixzations not worth it
> for most cases.
Heh, Peter Rosin (the mux subsys maintainer) was actually asking if
we could even get rid of doing the read all the time. Lets discuss
this further in my reply to Peter's reply.
>
>> + data |= SW_IDPIN_EN;
>> + writel(data, mux->base + DUAL_ROLE_CFG0);
>> + }
>> +}
>
>> + /* In most case it takes about 600ms to finish mode switching */
>> + timeout = jiffies + msecs_to_jiffies(DUAL_ROLE_CFG1_POLL_TIMEOUT);
>> +
>> + /* Polling on CFG1 register to confirm mode switch.*/
>
>> + while (1) {
>
> do {} while (time_before()); ?
That means having to have a second conditional after the
loop to detect we timed-out and do the dev_warn, I would prefer to keep
this as is.
Regards,
Hans
>
>> + data = readl(mux->base + DUAL_ROLE_CFG1);
>> + if (!!(data & HOST_MODE) == host_mode)
>> + break;
>> +
>> + /* Interval for polling is set to about 5 - 10 ms */
>> + usleep_range(5000, 10000);
>> +
>> + if (time_after(jiffies, timeout)) {
>> + dev_warn(&mux_ctrl->chip->dev,
>> + "Timeout waiting for mux to switch\n");
>> + break;
>> + }
>> + }
>
>
>> +static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
>> +{
>> + struct intel_cht_usb_mux *mux =
>> + container_of(work, struct intel_cht_usb_mux, vbus_work);
>> + bool vbus_present = false;
>> + int i;
>
> unsigned int i; ?
>
>> +
>> + for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
>> + if (extcon_get_state(mux->vbus_extcon, vbus_cable_ids[i]) > 0) {
>> + vbus_present = true;
>> + break;
>> + }
>> + }
>> +
>> + intel_cht_usb_mux_set_vbus_valid(mux, vbus_present);
>> +}
>
>> +static int intel_cht_usb_mux_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct intel_cht_usb_mux *mux;
>> + struct mux_chip *mux_chip;
>> + struct resource *res;
>> + resource_size_t size;
>
>> + int i, ret;
>
> Ditto.
>
>> + for (i = 0 ; i < ARRAY_SIZE(vbus_providers); i++) {
>> + if (!acpi_dev_present(vbus_providers[i].hid, NULL,
>> + vbus_providers[i].hrv))
>> + continue;
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
2017-09-01 21:48 ` [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver Hans de Goede
[not found] ` <20170901214845.7153-6-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2017-09-04 11:19 ` Peter Rosin
2017-09-05 11:09 ` Hans de Goede
1 sibling, 1 reply; 52+ messages in thread
From: Peter Rosin @ 2017-09-04 11:19 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
Hi!
Some comments inline...
On 2017-09-01 23:48, Hans de Goede wrote:
> Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port
> USB data lines between the xHCI host controller and the dwc3 gadget
> controller. On some Cherrytrail systems this mux is controlled through
> AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through
> an _AIE ACPI method) so things just work.
>
> But on other Cherrytrail systems we need to control the mux ourselves
> this driver exports the mux through the mux subsys, so that other drivers
> can control it if necessary.
>
> This driver also updates the vbus-valid reporting to the dwc3 gadget
> controller, as this uses the same registers as the mux. This is needed
> for gadget/device mode to work properly (even on systems which control
> the mux from their AML code).
>
> Note this depends on the xhci driver registering a platform device
> named "intel_cht_usb_mux", which has an IOMEM resource 0 which points
> to the Intel Vendor Defined XHCI extended capabilities region.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Complete rewrite as a stand-alone platform-driver rather then as a phy
> driver, since this is just a mux, not a phy
>
> Changes in v3:
> -Make this a mux subsys driver instead of listening to USB_HOST extcon
> cable events and responding to those
> ---
> drivers/mux/Kconfig | 11 ++
> drivers/mux/Makefile | 2 +
> drivers/mux/intel_cht_usb_mux.c | 269 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 282 insertions(+)
> create mode 100644 drivers/mux/intel_cht_usb_mux.c
>
> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
> index 19e4e904c9bf..17938918bf93 100644
> --- a/drivers/mux/Kconfig
> +++ b/drivers/mux/Kconfig
> @@ -34,6 +34,17 @@ config MUX_GPIO
> To compile the driver as a module, choose M here: the module will
> be called mux-gpio.
>
> +config MUX_CHT_USB_MUX
> + tristate "Intel Cherrytrail USB Multiplexer"
> + depends on ACPI && X86 && EXTCON
> + help
> + This driver adds support for the internal USB mux for muxing the OTG
> + USB data lines between the xHCI host controller and the dwc3 gadget
> + controller found on Intel Cherrytrail SoCs.
> +
> + To compile the driver as a module, choose M here: the module will
> + be called mux-intel_cht_usb_mux.
> +
> config MUX_MMIO
> tristate "MMIO register bitfield-controlled Multiplexer"
> depends on (OF && MFD_SYSCON) || COMPILE_TEST
> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
> index 0e1e59760e3f..a12e812c7966 100644
> --- a/drivers/mux/Makefile
> +++ b/drivers/mux/Makefile
> @@ -6,8 +6,10 @@ mux-core-objs := core.o
> mux-adg792a-objs := adg792a.o
> mux-gpio-objs := gpio.o
> mux-mmio-objs := mmio.o
> +mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
I dislike underscores in file names (and other names), please use
dashes where possible. Also, please keep the list sorted.
>
> obj-$(CONFIG_MULTIPLEXER) += mux-core.o
> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
> +obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
Dito.
> diff --git a/drivers/mux/intel_cht_usb_mux.c b/drivers/mux/intel_cht_usb_mux.c
> new file mode 100644
> index 000000000000..7b1621a081d8
> --- /dev/null
> +++ b/drivers/mux/intel_cht_usb_mux.c
> @@ -0,0 +1,269 @@
> +/*
> + * Intel Cherrytrail USB OTG MUX driver
> + *
> + * Copyright (c) 2016 Hans de Goede <hdegoede@redhat.com>
> + *
> + * Loosely based on android x86 kernel code which is:
> + *
> + * Copyright (C) 2014 Intel Corp.
> + *
> + * Author: Wu, Hao
> + *
> + * 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, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/extcon.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
> +#include <linux/mux/driver.h>
> +#include <linux/platform_device.h>
> +#include <linux/workqueue.h>
> +
> +/* register definition */
> +#define DUAL_ROLE_CFG0 0x68
> +#define SW_VBUS_VALID (1 << 24)
> +#define SW_IDPIN_EN (1 << 21)
> +#define SW_IDPIN (1 << 20)
> +
> +#define DUAL_ROLE_CFG1 0x6c
> +#define HOST_MODE (1 << 29)
> +
> +#define DUAL_ROLE_CFG1_POLL_TIMEOUT 1000
> +
> +#define DRV_NAME "intel_cht_usb_mux"
Dashes, please, if possible. Or are there perhaps a lot of precedent
for other cherrytrail driver names? Because consistency across
the tree is a tad more important than my issues with underscores...
> +
> +struct intel_cht_usb_mux {
> + struct mutex cfg0_lock;
> + void __iomem *base;
> + struct extcon_dev *vbus_extcon;
> + struct notifier_block vbus_nb;
> + struct work_struct vbus_work;
> +};
> +
> +struct intel_cht_extcon_info {
> + const char *hid;
> + int hrv;
> + const char *extcon;
> +};
> +
> +struct intel_cht_extcon_info vbus_providers[] = {
> + { "INT33F4", -1, "axp288_extcon" },
> + { "INT34D3", 3, "cht_wcove_pwrsrc" },
> +};
Dito. And static const. What about:
static const struct intel_cht_extcon_info vbus_providers[] = {
{ .hid = "INT33F4", .hrv = -1, .extcon = "axp288_extcon", },
{ .hid = "INT34D3", .hrv = 3, .extcon = "cht_wcove_pwrsrc", },
};
> +
> +static const unsigned int vbus_cable_ids[] = {
> + EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP,
> + EXTCON_CHG_USB_ACA, EXTCON_CHG_USB_FAST,
> +};
> +
> +static void intel_cht_usb_mux_set_sw_mode(struct intel_cht_usb_mux *mux)
> +{
> + u32 data;
> +
> + data = readl(mux->base + DUAL_ROLE_CFG0);
> + if (!(data & SW_IDPIN_EN)) {
> + data |= SW_IDPIN_EN;
> + writel(data, mux->base + DUAL_ROLE_CFG0);
> + }
> +}
Is SW_IDPIN_EN a bit that unlocks the other bits in the register for
writing? Once? Perhaps it interacts with the mode switch confirmation
loop? Anyway, I'd like to see a comment about why this bit needs to be
set so many times.
> +
> +static int intel_cht_usb_mux_set_mux(struct mux_control *mux_ctrl, int state)
> +{
> + struct intel_cht_usb_mux *mux = mux_chip_priv(mux_ctrl->chip);
The "mux" variable name is used for the mux_control in other drivers, and
the private data is named something else. Please fix this all over the
driver for consistency across the subsys.
> + unsigned long timeout;
> + bool host_mode;
> + u32 data;
> +
> + mutex_lock(&mux->cfg0_lock);
> +
> + intel_cht_usb_mux_set_sw_mode(mux);
> +
> + /* Set idpin value as requested */
> + data = readl(mux->base + DUAL_ROLE_CFG0);
> + switch (state & ~MUX_USB_POLARITY_INV) {
> + case MUX_USB_NONE:
> + case MUX_USB_DEVICE:
> + data |= SW_IDPIN;
> + host_mode = false;
> + break;
> + default:
> + data &= ~SW_IDPIN;
> + host_mode = true;
> + }
> + writel(data, mux->base + DUAL_ROLE_CFG0);
> +
> + mutex_unlock(&mux->cfg0_lock);
> +
> + /* In most case it takes about 600ms to finish mode switching */
> + timeout = jiffies + msecs_to_jiffies(DUAL_ROLE_CFG1_POLL_TIMEOUT);
> +
> + /* Polling on CFG1 register to confirm mode switch.*/
> + while (1) {
> + data = readl(mux->base + DUAL_ROLE_CFG1);
> + if (!!(data & HOST_MODE) == host_mode)
> + break;
> +
> + /* Interval for polling is set to about 5 - 10 ms */
> + usleep_range(5000, 10000);
> +
> + if (time_after(jiffies, timeout)) {
> + dev_warn(&mux_ctrl->chip->dev,
> + "Timeout waiting for mux to switch\n");
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void intel_cht_usb_mux_set_vbus_valid(struct intel_cht_usb_mux *mux,
> + bool valid)
> +{
> + u32 data;
> +
> + mutex_lock(&mux->cfg0_lock);
> +
> + intel_cht_usb_mux_set_sw_mode(mux);
> +
> + data = readl(mux->base + DUAL_ROLE_CFG0);
> + if (valid)
> + data |= SW_VBUS_VALID;
> + else
> + data &= ~SW_VBUS_VALID;
> + writel(data, mux->base + DUAL_ROLE_CFG0);
> +
> + mutex_unlock(&mux->cfg0_lock);
> +}
> +
> +static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
> +{
> + struct intel_cht_usb_mux *mux =
> + container_of(work, struct intel_cht_usb_mux, vbus_work);
> + bool vbus_present = false;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
> + if (extcon_get_state(mux->vbus_extcon, vbus_cable_ids[i]) > 0) {
> + vbus_present = true;
> + break;
> + }
> + }
> +
> + intel_cht_usb_mux_set_vbus_valid(mux, vbus_present);
> +}
> +
> +static int intel_cht_usb_mux_vbus_extcon_evt(struct notifier_block *nb,
> + unsigned long event, void *param)
> +{
> + struct intel_cht_usb_mux *mux =
> + container_of(nb, struct intel_cht_usb_mux, vbus_nb);
> +
> + schedule_work(&mux->vbus_work);
> +
> + return NOTIFY_OK;
> +}
> +
> +static const struct mux_control_ops intel_cht_usb_mux_ops = {
> + .set = intel_cht_usb_mux_set_mux,
> +};
> +
> +static int intel_cht_usb_mux_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct intel_cht_usb_mux *mux;
> + struct mux_chip *mux_chip;
> + struct resource *res;
> + resource_size_t size;
> + int i, ret;
> +
> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
> + if (IS_ERR(mux_chip))
> + return PTR_ERR(mux_chip);
> +
> + mux_chip->ops = &intel_cht_usb_mux_ops;
> + mux_chip->mux[0].states = MUX_USB_STATES;
This is "a lie" I think, the mux only supports two states; device and host.
Looks good otherwise, if you also consider the remarks from Andy.
Cheers,
Peter
> + mux = mux_chip_priv(mux_chip);
> + mutex_init(&mux->cfg0_lock);
> +
> + /*
> + * Besides controlling the mux we also need to control the vbus_valid
> + * flag for device/gadget mode to work properly. To do this we monitor
> + * the extcon interface exported by the PMIC drivers for the PMICs used
> + * with the Cherry Trail SoC.
> + *
> + * We try to get the extcon_dev before registering the mux as this
> + * may lead to us exiting with -EPROBE_DEFER.
> + */
> + for (i = 0 ; i < ARRAY_SIZE(vbus_providers); i++) {
> + if (!acpi_dev_present(vbus_providers[i].hid, NULL,
> + vbus_providers[i].hrv))
> + continue;
> +
> + mux->vbus_extcon = extcon_get_extcon_dev(
> + vbus_providers[i].extcon);
> + if (mux->vbus_extcon == NULL)
> + return -EPROBE_DEFER;
> +
> + dev_info(dev, "using extcon '%s' for vbus-valid\n",
> + vbus_providers[i].extcon);
> + break;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + size = (res->end + 1) - res->start;
> + mux->base = devm_ioremap_nocache(dev, res->start, size);
> + if (IS_ERR(mux->base)) {
> + ret = PTR_ERR(mux->base);
> + dev_err(dev, "can't iomap registers: %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_mux_chip_register(dev, mux_chip);
> + if (ret < 0)
> + return ret;
> +
> + if (mux->vbus_extcon) {
> + INIT_WORK(&mux->vbus_work, intel_cht_usb_mux_vbus_work);
> + mux->vbus_nb.notifier_call = intel_cht_usb_mux_vbus_extcon_evt;
> + ret = devm_extcon_register_notifier_all(dev, mux->vbus_extcon,
> + &mux->vbus_nb);
> + if (ret) {
> + dev_err(dev, "can't register vbus extcon notifier: %d\n",
> + ret);
> + return ret;
> + }
> +
> + /* Sync initial mode */
> + schedule_work(&mux->vbus_work);
> + }
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id intel_cht_usb_mux_table[] = {
> + { .name = DRV_NAME },
> + {},
> +};
> +MODULE_DEVICE_TABLE(platform, intel_cht_usb_mux_table);
> +
> +static struct platform_driver intel_cht_usb_mux_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + },
> + .id_table = intel_cht_usb_mux_table,
> + .probe = intel_cht_usb_mux_probe,
> +};
> +
> +module_platform_driver(intel_cht_usb_mux_driver);
> +
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> +MODULE_DESCRIPTION("Intel Cherrytrail USB mux driver");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
2017-09-04 11:19 ` Peter Rosin
@ 2017-09-05 11:09 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-05 11:09 UTC (permalink / raw)
To: Peter Rosin, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan
Hi,
On 04-09-17 13:19, Peter Rosin wrote:
> Hi!
>
> Some comments inline...
>
> On 2017-09-01 23:48, Hans de Goede wrote:
>> Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port
>> USB data lines between the xHCI host controller and the dwc3 gadget
>> controller. On some Cherrytrail systems this mux is controlled through
>> AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through
>> an _AIE ACPI method) so things just work.
>>
>> But on other Cherrytrail systems we need to control the mux ourselves
>> this driver exports the mux through the mux subsys, so that other drivers
>> can control it if necessary.
>>
>> This driver also updates the vbus-valid reporting to the dwc3 gadget
>> controller, as this uses the same registers as the mux. This is needed
>> for gadget/device mode to work properly (even on systems which control
>> the mux from their AML code).
>>
>> Note this depends on the xhci driver registering a platform device
>> named "intel_cht_usb_mux", which has an IOMEM resource 0 which points
>> to the Intel Vendor Defined XHCI extended capabilities region.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Complete rewrite as a stand-alone platform-driver rather then as a phy
>> driver, since this is just a mux, not a phy
>>
>> Changes in v3:
>> -Make this a mux subsys driver instead of listening to USB_HOST extcon
>> cable events and responding to those
>> ---
>> drivers/mux/Kconfig | 11 ++
>> drivers/mux/Makefile | 2 +
>> drivers/mux/intel_cht_usb_mux.c | 269 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 282 insertions(+)
>> create mode 100644 drivers/mux/intel_cht_usb_mux.c
>>
>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
>> index 19e4e904c9bf..17938918bf93 100644
>> --- a/drivers/mux/Kconfig
>> +++ b/drivers/mux/Kconfig
>> @@ -34,6 +34,17 @@ config MUX_GPIO
>> To compile the driver as a module, choose M here: the module will
>> be called mux-gpio.
>>
>> +config MUX_CHT_USB_MUX
>> + tristate "Intel Cherrytrail USB Multiplexer"
>> + depends on ACPI && X86 && EXTCON
>> + help
>> + This driver adds support for the internal USB mux for muxing the OTG
>> + USB data lines between the xHCI host controller and the dwc3 gadget
>> + controller found on Intel Cherrytrail SoCs.
>> +
>> + To compile the driver as a module, choose M here: the module will
>> + be called mux-intel_cht_usb_mux.
>> +
>> config MUX_MMIO
>> tristate "MMIO register bitfield-controlled Multiplexer"
>> depends on (OF && MFD_SYSCON) || COMPILE_TEST
>> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
>> index 0e1e59760e3f..a12e812c7966 100644
>> --- a/drivers/mux/Makefile
>> +++ b/drivers/mux/Makefile
>> @@ -6,8 +6,10 @@ mux-core-objs := core.o
>> mux-adg792a-objs := adg792a.o
>> mux-gpio-objs := gpio.o
>> mux-mmio-objs := mmio.o
>> +mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
>
> I dislike underscores in file names (and other names), please use
> dashes where possible. Also, please keep the list sorted.
>
>>
>> obj-$(CONFIG_MULTIPLEXER) += mux-core.o
>> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
>> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
>> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
>> +obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
>
> Dito.
Ok, will fix both for v2.
>> diff --git a/drivers/mux/intel_cht_usb_mux.c b/drivers/mux/intel_cht_usb_mux.c
>> new file mode 100644
>> index 000000000000..7b1621a081d8
>> --- /dev/null
>> +++ b/drivers/mux/intel_cht_usb_mux.c
>> @@ -0,0 +1,269 @@
>> +/*
>> + * Intel Cherrytrail USB OTG MUX driver
>> + *
>> + * Copyright (c) 2016 Hans de Goede <hdegoede@redhat.com>
>> + *
>> + * Loosely based on android x86 kernel code which is:
>> + *
>> + * Copyright (C) 2014 Intel Corp.
>> + *
>> + * Author: Wu, Hao
>> + *
>> + * 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, or (at your option)
>> + * any later version.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/extcon.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
>> +#include <linux/mux/driver.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/workqueue.h>
>> +
>> +/* register definition */
>> +#define DUAL_ROLE_CFG0 0x68
>> +#define SW_VBUS_VALID (1 << 24)
>> +#define SW_IDPIN_EN (1 << 21)
>> +#define SW_IDPIN (1 << 20)
>> +
>> +#define DUAL_ROLE_CFG1 0x6c
>> +#define HOST_MODE (1 << 29)
>> +
>> +#define DUAL_ROLE_CFG1_POLL_TIMEOUT 1000
>> +
>> +#define DRV_NAME "intel_cht_usb_mux"
>
> Dashes, please, if possible. Or are there perhaps a lot of precedent
> for other cherrytrail driver names? Because consistency across
> the tree is a tad more important than my issues with underscores...
The Cherry Trail code uses _ for device-names everywhere, so lets
keep this one as is.
>
>> +
>> +struct intel_cht_usb_mux {
>> + struct mutex cfg0_lock;
>> + void __iomem *base;
>> + struct extcon_dev *vbus_extcon;
>> + struct notifier_block vbus_nb;
>> + struct work_struct vbus_work;
>> +};
>> +
>> +struct intel_cht_extcon_info {
>> + const char *hid;
>> + int hrv;
>> + const char *extcon;
>> +};
>> +
>> +struct intel_cht_extcon_info vbus_providers[] = {
>> + { "INT33F4", -1, "axp288_extcon" },
>> + { "INT34D3", 3, "cht_wcove_pwrsrc" },
>> +};
>
> Ditto.
I assume you mean the _ thingie with ditto? These are
names coded in existing drivers, so sorry this cannot
be (easily) changed.
> And static const. What about:
>
> static const struct intel_cht_extcon_info vbus_providers[] = {
> { .hid = "INT33F4", .hrv = -1, .extcon = "axp288_extcon", },
> { .hid = "INT34D3", .hrv = 3, .extcon = "cht_wcove_pwrsrc", },
> };
Sure, WFM.
>> +
>> +static const unsigned int vbus_cable_ids[] = {
>> + EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP,
>> + EXTCON_CHG_USB_ACA, EXTCON_CHG_USB_FAST,
>> +};
>> +
>> +static void intel_cht_usb_mux_set_sw_mode(struct intel_cht_usb_mux *mux)
>> +{
>> + u32 data;
>> +
>> + data = readl(mux->base + DUAL_ROLE_CFG0);
>> + if (!(data & SW_IDPIN_EN)) {
>> + data |= SW_IDPIN_EN;
>> + writel(data, mux->base + DUAL_ROLE_CFG0);
>> + }
>> +}
>
> Is SW_IDPIN_EN a bit that unlocks the other bits in the register for
> writing? Once? Perhaps it interacts with the mode switch confirmation
> loop? Anyway, I'd like to see a comment about why this bit needs to be
> set so many times.
It is not an unlock bit as much as it is an enable SW control bit,
I think Intel sourced the USB PHY and mux design from some 3th party,
because there is no dedicated hardware ID pin (instead a GPIO is used)
and we need to propagate the value of the GPIO connected to the ID
pin of the OTG connector to the SW_IDPIN register bit.
And for the PHY/mux to listen to the SW_IDPIN bit rather then to
the (Not Connected) physical ID pin we need to set SW_IDPIN_EN.
But thinking about this more, also given Andy's comments I'm
pretty sure I can safely just or this bit in while doing the
read-modify-write to set the SW_IDPIN or SW_VBUS_VALID bit
itself, so this can be removed.
>
>> +
>> +static int intel_cht_usb_mux_set_mux(struct mux_control *mux_ctrl, int state)
>> +{
>> + struct intel_cht_usb_mux *mux = mux_chip_priv(mux_ctrl->chip);
>
> The "mux" variable name is used for the mux_control in other drivers, and
> the private data is named something else. Please fix this all over the
> driver for consistency across the subsys.
Ok.
>
>> + unsigned long timeout;
>> + bool host_mode;
>> + u32 data;
>> +
>> + mutex_lock(&mux->cfg0_lock);
>> +
>> + intel_cht_usb_mux_set_sw_mode(mux);
>> +
>> + /* Set idpin value as requested */
>> + data = readl(mux->base + DUAL_ROLE_CFG0);
>> + switch (state & ~MUX_USB_POLARITY_INV) {
>> + case MUX_USB_NONE:
>> + case MUX_USB_DEVICE:
>> + data |= SW_IDPIN;
>> + host_mode = false;
>> + break;
>> + default:
>> + data &= ~SW_IDPIN;
>> + host_mode = true;
>> + }
>> + writel(data, mux->base + DUAL_ROLE_CFG0);
>> +
>> + mutex_unlock(&mux->cfg0_lock);
>> +
>> + /* In most case it takes about 600ms to finish mode switching */
>> + timeout = jiffies + msecs_to_jiffies(DUAL_ROLE_CFG1_POLL_TIMEOUT);
>> +
>> + /* Polling on CFG1 register to confirm mode switch.*/
>> + while (1) {
>> + data = readl(mux->base + DUAL_ROLE_CFG1);
>> + if (!!(data & HOST_MODE) == host_mode)
>> + break;
>> +
>> + /* Interval for polling is set to about 5 - 10 ms */
>> + usleep_range(5000, 10000);
>> +
>> + if (time_after(jiffies, timeout)) {
>> + dev_warn(&mux_ctrl->chip->dev,
>> + "Timeout waiting for mux to switch\n");
>> + break;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void intel_cht_usb_mux_set_vbus_valid(struct intel_cht_usb_mux *mux,
>> + bool valid)
>> +{
>> + u32 data;
>> +
>> + mutex_lock(&mux->cfg0_lock);
>> +
>> + intel_cht_usb_mux_set_sw_mode(mux);
>> +
>> + data = readl(mux->base + DUAL_ROLE_CFG0);
>> + if (valid)
>> + data |= SW_VBUS_VALID;
>> + else
>> + data &= ~SW_VBUS_VALID;
>> + writel(data, mux->base + DUAL_ROLE_CFG0);
>> +
>> + mutex_unlock(&mux->cfg0_lock);
>> +}
>> +
>> +static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
>> +{
>> + struct intel_cht_usb_mux *mux =
>> + container_of(work, struct intel_cht_usb_mux, vbus_work);
>> + bool vbus_present = false;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
>> + if (extcon_get_state(mux->vbus_extcon, vbus_cable_ids[i]) > 0) {
>> + vbus_present = true;
>> + break;
>> + }
>> + }
>> +
>> + intel_cht_usb_mux_set_vbus_valid(mux, vbus_present);
>> +}
>> +
>> +static int intel_cht_usb_mux_vbus_extcon_evt(struct notifier_block *nb,
>> + unsigned long event, void *param)
>> +{
>> + struct intel_cht_usb_mux *mux =
>> + container_of(nb, struct intel_cht_usb_mux, vbus_nb);
>> +
>> + schedule_work(&mux->vbus_work);
>> +
>> + return NOTIFY_OK;
>> +}
>> +
>> +static const struct mux_control_ops intel_cht_usb_mux_ops = {
>> + .set = intel_cht_usb_mux_set_mux,
>> +};
>> +
>> +static int intel_cht_usb_mux_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct intel_cht_usb_mux *mux;
>> + struct mux_chip *mux_chip;
>> + struct resource *res;
>> + resource_size_t size;
>> + int i, ret;
>> +
>> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
>> + if (IS_ERR(mux_chip))
>> + return PTR_ERR(mux_chip);
>> +
>> + mux_chip->ops = &intel_cht_usb_mux_ops;
>> + mux_chip->mux[0].states = MUX_USB_STATES;
>
> This is "a lie" I think, the mux only supports two states; device and host.
Right, this is the result me choosing to use a single set of defines
for both the USB-role and Type-C mode switches and then calling both
muxes with the same value. Will fix for v2.
Regards,
Hans
> Looks good otherwise, if you also consider the remarks from Andy.
>
> Cheers,
> Peter
>
>> + mux = mux_chip_priv(mux_chip);
>> + mutex_init(&mux->cfg0_lock);
>> +
>> + /*
>> + * Besides controlling the mux we also need to control the vbus_valid
>> + * flag for device/gadget mode to work properly. To do this we monitor
>> + * the extcon interface exported by the PMIC drivers for the PMICs used
>> + * with the Cherry Trail SoC.
>> + *
>> + * We try to get the extcon_dev before registering the mux as this
>> + * may lead to us exiting with -EPROBE_DEFER.
>> + */
>> + for (i = 0 ; i < ARRAY_SIZE(vbus_providers); i++) {
>> + if (!acpi_dev_present(vbus_providers[i].hid, NULL,
>> + vbus_providers[i].hrv))
>> + continue;
>> +
>> + mux->vbus_extcon = extcon_get_extcon_dev(
>> + vbus_providers[i].extcon);
>> + if (mux->vbus_extcon == NULL)
>> + return -EPROBE_DEFER;
>> +
>> + dev_info(dev, "using extcon '%s' for vbus-valid\n",
>> + vbus_providers[i].extcon);
>> + break;
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + size = (res->end + 1) - res->start;
>> + mux->base = devm_ioremap_nocache(dev, res->start, size);
>> + if (IS_ERR(mux->base)) {
>> + ret = PTR_ERR(mux->base);
>> + dev_err(dev, "can't iomap registers: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = devm_mux_chip_register(dev, mux_chip);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (mux->vbus_extcon) {
>> + INIT_WORK(&mux->vbus_work, intel_cht_usb_mux_vbus_work);
>> + mux->vbus_nb.notifier_call = intel_cht_usb_mux_vbus_extcon_evt;
>> + ret = devm_extcon_register_notifier_all(dev, mux->vbus_extcon,
>> + &mux->vbus_nb);
>> + if (ret) {
>> + dev_err(dev, "can't register vbus extcon notifier: %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + /* Sync initial mode */
>> + schedule_work(&mux->vbus_work);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct platform_device_id intel_cht_usb_mux_table[] = {
>> + { .name = DRV_NAME },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, intel_cht_usb_mux_table);
>> +
>> +static struct platform_driver intel_cht_usb_mux_driver = {
>> + .driver = {
>> + .name = DRV_NAME,
>> + },
>> + .id_table = intel_cht_usb_mux_table,
>> + .probe = intel_cht_usb_mux_probe,
>> +};
>> +
>> +module_platform_driver(intel_cht_usb_mux_driver);
>> +
>> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
>> +MODULE_DESCRIPTION("Intel Cherrytrail USB mux driver");
>> +MODULE_LICENSE("GPL");
>>
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 05/11] mux: Add Intel Cherrytrail USB mux driver
@ 2017-09-05 11:09 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-05 11:09 UTC (permalink / raw)
To: Peter Rosin, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
Hi,
On 04-09-17 13:19, Peter Rosin wrote:
> Hi!
>
> Some comments inline...
>
> On 2017-09-01 23:48, Hans de Goede wrote:
>> Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port
>> USB data lines between the xHCI host controller and the dwc3 gadget
>> controller. On some Cherrytrail systems this mux is controlled through
>> AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through
>> an _AIE ACPI method) so things just work.
>>
>> But on other Cherrytrail systems we need to control the mux ourselves
>> this driver exports the mux through the mux subsys, so that other drivers
>> can control it if necessary.
>>
>> This driver also updates the vbus-valid reporting to the dwc3 gadget
>> controller, as this uses the same registers as the mux. This is needed
>> for gadget/device mode to work properly (even on systems which control
>> the mux from their AML code).
>>
>> Note this depends on the xhci driver registering a platform device
>> named "intel_cht_usb_mux", which has an IOMEM resource 0 which points
>> to the Intel Vendor Defined XHCI extended capabilities region.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Complete rewrite as a stand-alone platform-driver rather then as a phy
>> driver, since this is just a mux, not a phy
>>
>> Changes in v3:
>> -Make this a mux subsys driver instead of listening to USB_HOST extcon
>> cable events and responding to those
>> ---
>> drivers/mux/Kconfig | 11 ++
>> drivers/mux/Makefile | 2 +
>> drivers/mux/intel_cht_usb_mux.c | 269 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 282 insertions(+)
>> create mode 100644 drivers/mux/intel_cht_usb_mux.c
>>
>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
>> index 19e4e904c9bf..17938918bf93 100644
>> --- a/drivers/mux/Kconfig
>> +++ b/drivers/mux/Kconfig
>> @@ -34,6 +34,17 @@ config MUX_GPIO
>> To compile the driver as a module, choose M here: the module will
>> be called mux-gpio.
>>
>> +config MUX_CHT_USB_MUX
>> + tristate "Intel Cherrytrail USB Multiplexer"
>> + depends on ACPI && X86 && EXTCON
>> + help
>> + This driver adds support for the internal USB mux for muxing the OTG
>> + USB data lines between the xHCI host controller and the dwc3 gadget
>> + controller found on Intel Cherrytrail SoCs.
>> +
>> + To compile the driver as a module, choose M here: the module will
>> + be called mux-intel_cht_usb_mux.
>> +
>> config MUX_MMIO
>> tristate "MMIO register bitfield-controlled Multiplexer"
>> depends on (OF && MFD_SYSCON) || COMPILE_TEST
>> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
>> index 0e1e59760e3f..a12e812c7966 100644
>> --- a/drivers/mux/Makefile
>> +++ b/drivers/mux/Makefile
>> @@ -6,8 +6,10 @@ mux-core-objs := core.o
>> mux-adg792a-objs := adg792a.o
>> mux-gpio-objs := gpio.o
>> mux-mmio-objs := mmio.o
>> +mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
>
> I dislike underscores in file names (and other names), please use
> dashes where possible. Also, please keep the list sorted.
>
>>
>> obj-$(CONFIG_MULTIPLEXER) += mux-core.o
>> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
>> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
>> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
>> +obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
>
> Dito.
Ok, will fix both for v2.
>> diff --git a/drivers/mux/intel_cht_usb_mux.c b/drivers/mux/intel_cht_usb_mux.c
>> new file mode 100644
>> index 000000000000..7b1621a081d8
>> --- /dev/null
>> +++ b/drivers/mux/intel_cht_usb_mux.c
>> @@ -0,0 +1,269 @@
>> +/*
>> + * Intel Cherrytrail USB OTG MUX driver
>> + *
>> + * Copyright (c) 2016 Hans de Goede <hdegoede@redhat.com>
>> + *
>> + * Loosely based on android x86 kernel code which is:
>> + *
>> + * Copyright (C) 2014 Intel Corp.
>> + *
>> + * Author: Wu, Hao
>> + *
>> + * 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, or (at your option)
>> + * any later version.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/extcon.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
>> +#include <linux/mux/driver.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/workqueue.h>
>> +
>> +/* register definition */
>> +#define DUAL_ROLE_CFG0 0x68
>> +#define SW_VBUS_VALID (1 << 24)
>> +#define SW_IDPIN_EN (1 << 21)
>> +#define SW_IDPIN (1 << 20)
>> +
>> +#define DUAL_ROLE_CFG1 0x6c
>> +#define HOST_MODE (1 << 29)
>> +
>> +#define DUAL_ROLE_CFG1_POLL_TIMEOUT 1000
>> +
>> +#define DRV_NAME "intel_cht_usb_mux"
>
> Dashes, please, if possible. Or are there perhaps a lot of precedent
> for other cherrytrail driver names? Because consistency across
> the tree is a tad more important than my issues with underscores...
The Cherry Trail code uses _ for device-names everywhere, so lets
keep this one as is.
>
>> +
>> +struct intel_cht_usb_mux {
>> + struct mutex cfg0_lock;
>> + void __iomem *base;
>> + struct extcon_dev *vbus_extcon;
>> + struct notifier_block vbus_nb;
>> + struct work_struct vbus_work;
>> +};
>> +
>> +struct intel_cht_extcon_info {
>> + const char *hid;
>> + int hrv;
>> + const char *extcon;
>> +};
>> +
>> +struct intel_cht_extcon_info vbus_providers[] = {
>> + { "INT33F4", -1, "axp288_extcon" },
>> + { "INT34D3", 3, "cht_wcove_pwrsrc" },
>> +};
>
> Ditto.
I assume you mean the _ thingie with ditto? These are
names coded in existing drivers, so sorry this cannot
be (easily) changed.
> And static const. What about:
>
> static const struct intel_cht_extcon_info vbus_providers[] = {
> { .hid = "INT33F4", .hrv = -1, .extcon = "axp288_extcon", },
> { .hid = "INT34D3", .hrv = 3, .extcon = "cht_wcove_pwrsrc", },
> };
Sure, WFM.
>> +
>> +static const unsigned int vbus_cable_ids[] = {
>> + EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP,
>> + EXTCON_CHG_USB_ACA, EXTCON_CHG_USB_FAST,
>> +};
>> +
>> +static void intel_cht_usb_mux_set_sw_mode(struct intel_cht_usb_mux *mux)
>> +{
>> + u32 data;
>> +
>> + data = readl(mux->base + DUAL_ROLE_CFG0);
>> + if (!(data & SW_IDPIN_EN)) {
>> + data |= SW_IDPIN_EN;
>> + writel(data, mux->base + DUAL_ROLE_CFG0);
>> + }
>> +}
>
> Is SW_IDPIN_EN a bit that unlocks the other bits in the register for
> writing? Once? Perhaps it interacts with the mode switch confirmation
> loop? Anyway, I'd like to see a comment about why this bit needs to be
> set so many times.
It is not an unlock bit as much as it is an enable SW control bit,
I think Intel sourced the USB PHY and mux design from some 3th party,
because there is no dedicated hardware ID pin (instead a GPIO is used)
and we need to propagate the value of the GPIO connected to the ID
pin of the OTG connector to the SW_IDPIN register bit.
And for the PHY/mux to listen to the SW_IDPIN bit rather then to
the (Not Connected) physical ID pin we need to set SW_IDPIN_EN.
But thinking about this more, also given Andy's comments I'm
pretty sure I can safely just or this bit in while doing the
read-modify-write to set the SW_IDPIN or SW_VBUS_VALID bit
itself, so this can be removed.
>
>> +
>> +static int intel_cht_usb_mux_set_mux(struct mux_control *mux_ctrl, int state)
>> +{
>> + struct intel_cht_usb_mux *mux = mux_chip_priv(mux_ctrl->chip);
>
> The "mux" variable name is used for the mux_control in other drivers, and
> the private data is named something else. Please fix this all over the
> driver for consistency across the subsys.
Ok.
>
>> + unsigned long timeout;
>> + bool host_mode;
>> + u32 data;
>> +
>> + mutex_lock(&mux->cfg0_lock);
>> +
>> + intel_cht_usb_mux_set_sw_mode(mux);
>> +
>> + /* Set idpin value as requested */
>> + data = readl(mux->base + DUAL_ROLE_CFG0);
>> + switch (state & ~MUX_USB_POLARITY_INV) {
>> + case MUX_USB_NONE:
>> + case MUX_USB_DEVICE:
>> + data |= SW_IDPIN;
>> + host_mode = false;
>> + break;
>> + default:
>> + data &= ~SW_IDPIN;
>> + host_mode = true;
>> + }
>> + writel(data, mux->base + DUAL_ROLE_CFG0);
>> +
>> + mutex_unlock(&mux->cfg0_lock);
>> +
>> + /* In most case it takes about 600ms to finish mode switching */
>> + timeout = jiffies + msecs_to_jiffies(DUAL_ROLE_CFG1_POLL_TIMEOUT);
>> +
>> + /* Polling on CFG1 register to confirm mode switch.*/
>> + while (1) {
>> + data = readl(mux->base + DUAL_ROLE_CFG1);
>> + if (!!(data & HOST_MODE) == host_mode)
>> + break;
>> +
>> + /* Interval for polling is set to about 5 - 10 ms */
>> + usleep_range(5000, 10000);
>> +
>> + if (time_after(jiffies, timeout)) {
>> + dev_warn(&mux_ctrl->chip->dev,
>> + "Timeout waiting for mux to switch\n");
>> + break;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void intel_cht_usb_mux_set_vbus_valid(struct intel_cht_usb_mux *mux,
>> + bool valid)
>> +{
>> + u32 data;
>> +
>> + mutex_lock(&mux->cfg0_lock);
>> +
>> + intel_cht_usb_mux_set_sw_mode(mux);
>> +
>> + data = readl(mux->base + DUAL_ROLE_CFG0);
>> + if (valid)
>> + data |= SW_VBUS_VALID;
>> + else
>> + data &= ~SW_VBUS_VALID;
>> + writel(data, mux->base + DUAL_ROLE_CFG0);
>> +
>> + mutex_unlock(&mux->cfg0_lock);
>> +}
>> +
>> +static void intel_cht_usb_mux_vbus_work(struct work_struct *work)
>> +{
>> + struct intel_cht_usb_mux *mux =
>> + container_of(work, struct intel_cht_usb_mux, vbus_work);
>> + bool vbus_present = false;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
>> + if (extcon_get_state(mux->vbus_extcon, vbus_cable_ids[i]) > 0) {
>> + vbus_present = true;
>> + break;
>> + }
>> + }
>> +
>> + intel_cht_usb_mux_set_vbus_valid(mux, vbus_present);
>> +}
>> +
>> +static int intel_cht_usb_mux_vbus_extcon_evt(struct notifier_block *nb,
>> + unsigned long event, void *param)
>> +{
>> + struct intel_cht_usb_mux *mux =
>> + container_of(nb, struct intel_cht_usb_mux, vbus_nb);
>> +
>> + schedule_work(&mux->vbus_work);
>> +
>> + return NOTIFY_OK;
>> +}
>> +
>> +static const struct mux_control_ops intel_cht_usb_mux_ops = {
>> + .set = intel_cht_usb_mux_set_mux,
>> +};
>> +
>> +static int intel_cht_usb_mux_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct intel_cht_usb_mux *mux;
>> + struct mux_chip *mux_chip;
>> + struct resource *res;
>> + resource_size_t size;
>> + int i, ret;
>> +
>> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
>> + if (IS_ERR(mux_chip))
>> + return PTR_ERR(mux_chip);
>> +
>> + mux_chip->ops = &intel_cht_usb_mux_ops;
>> + mux_chip->mux[0].states = MUX_USB_STATES;
>
> This is "a lie" I think, the mux only supports two states; device and host.
Right, this is the result me choosing to use a single set of defines
for both the USB-role and Type-C mode switches and then calling both
muxes with the same value. Will fix for v2.
Regards,
Hans
> Looks good otherwise, if you also consider the remarks from Andy.
>
> Cheers,
> Peter
>
>> + mux = mux_chip_priv(mux_chip);
>> + mutex_init(&mux->cfg0_lock);
>> +
>> + /*
>> + * Besides controlling the mux we also need to control the vbus_valid
>> + * flag for device/gadget mode to work properly. To do this we monitor
>> + * the extcon interface exported by the PMIC drivers for the PMICs used
>> + * with the Cherry Trail SoC.
>> + *
>> + * We try to get the extcon_dev before registering the mux as this
>> + * may lead to us exiting with -EPROBE_DEFER.
>> + */
>> + for (i = 0 ; i < ARRAY_SIZE(vbus_providers); i++) {
>> + if (!acpi_dev_present(vbus_providers[i].hid, NULL,
>> + vbus_providers[i].hrv))
>> + continue;
>> +
>> + mux->vbus_extcon = extcon_get_extcon_dev(
>> + vbus_providers[i].extcon);
>> + if (mux->vbus_extcon == NULL)
>> + return -EPROBE_DEFER;
>> +
>> + dev_info(dev, "using extcon '%s' for vbus-valid\n",
>> + vbus_providers[i].extcon);
>> + break;
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + size = (res->end + 1) - res->start;
>> + mux->base = devm_ioremap_nocache(dev, res->start, size);
>> + if (IS_ERR(mux->base)) {
>> + ret = PTR_ERR(mux->base);
>> + dev_err(dev, "can't iomap registers: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = devm_mux_chip_register(dev, mux_chip);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (mux->vbus_extcon) {
>> + INIT_WORK(&mux->vbus_work, intel_cht_usb_mux_vbus_work);
>> + mux->vbus_nb.notifier_call = intel_cht_usb_mux_vbus_extcon_evt;
>> + ret = devm_extcon_register_notifier_all(dev, mux->vbus_extcon,
>> + &mux->vbus_nb);
>> + if (ret) {
>> + dev_err(dev, "can't register vbus extcon notifier: %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + /* Sync initial mode */
>> + schedule_work(&mux->vbus_work);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct platform_device_id intel_cht_usb_mux_table[] = {
>> + { .name = DRV_NAME },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, intel_cht_usb_mux_table);
>> +
>> +static struct platform_driver intel_cht_usb_mux_driver = {
>> + .driver = {
>> + .name = DRV_NAME,
>> + },
>> + .id_table = intel_cht_usb_mux_table,
>> + .probe = intel_cht_usb_mux_probe,
>> +};
>> +
>> +module_platform_driver(intel_cht_usb_mux_driver);
>> +
>> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
>> +MODULE_DESCRIPTION("Intel Cherrytrail USB mux driver");
>> +MODULE_LICENSE("GPL");
>>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 06/11] mux: Add Pericom PI3USB30532 Type-C mux driver
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede
Add a driver for the Pericom PI3USB30532 Type-C cross switch /
mux chip found on some devices with a Type-C port.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/mux/Kconfig | 10 +++++
drivers/mux/Makefile | 2 +
drivers/mux/pi3usb30532.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 109 insertions(+)
create mode 100644 drivers/mux/pi3usb30532.c
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index 17938918bf93..19a3065c34e6 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -58,4 +58,14 @@ config MUX_MMIO
To compile the driver as a module, choose M here: the module will
be called mux-mmio.
+config MUX_PI3USB30532
+ tristate "Pericom PI3USB30532 Type-C cross switch driver"
+ depends on I2C
+ help
+ This driver adds support for the Pericom PI3USB30532 Type-C cross
+ switch / mux chip found on some devices with a Type-C port.
+
+ To compile the driver as a module, choose M here: the module will
+ be called mux-pi3usb30532.
+
endmenu
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index a12e812c7966..7563dbf04593 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -7,9 +7,11 @@ mux-adg792a-objs := adg792a.o
mux-gpio-objs := gpio.o
mux-mmio-objs := mmio.o
mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
+mux-pi3usb30532-objs := pi3usb30532.o
obj-$(CONFIG_MULTIPLEXER) += mux-core.o
obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
+obj-$(CONFIG_MUX_PI3USB30532) += mux-pi3usb30532.o
diff --git a/drivers/mux/pi3usb30532.c b/drivers/mux/pi3usb30532.c
new file mode 100644
index 000000000000..fa8abd851520
--- /dev/null
+++ b/drivers/mux/pi3usb30532.c
@@ -0,0 +1,97 @@
+/*
+ * Pericom PI3USB30532 Type-C cross switch / mux driver
+ *
+ * Copyright (c) 2017 Hans de Goede <hdegoede@redhat.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, or (at your option)
+ * any later version.
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
+#include <linux/mux/driver.h>
+
+#define PI3USB30532_CONF 0x00
+
+#define PI3USB30532_CONF_OPEN 0x00
+#define PI3USB30532_CONF_SWAP 0x01
+#define PI3USB30532_CONF_4LANE_DP 0x02
+#define PI3USB30532_CONF_USB3 0x04
+#define PI3USB30532_CONF_USB3_AND_2LANE_DP 0x06
+
+struct pi3usb30532_mux {
+ struct i2c_client *client;
+};
+
+static int pi3usb30532_mux_set_mux(struct mux_control *mux_ctrl, int state)
+{
+ struct pi3usb30532_mux *mux = mux_chip_priv(mux_ctrl->chip);
+ u8 conf = PI3USB30532_CONF_OPEN;
+
+ switch (state & ~MUX_USB_POLARITY_INV) {
+ case MUX_USB_NONE:
+ conf = PI3USB30532_CONF_OPEN;
+ break;
+ case MUX_USB_DEVICE:
+ case MUX_USB_HOST:
+ conf = PI3USB30532_CONF_USB3;
+ break;
+ case MUX_USB_HOST_AND_DP_SRC:
+ conf = PI3USB30532_CONF_USB3_AND_2LANE_DP;
+ break;
+ case MUX_USB_DP_SRC:
+ conf = PI3USB30532_CONF_4LANE_DP;
+ break;
+ }
+
+ if (state & MUX_USB_POLARITY_INV)
+ conf |= PI3USB30532_CONF_SWAP;
+
+ return i2c_smbus_write_byte_data(mux->client, PI3USB30532_CONF, conf);
+}
+
+static const struct mux_control_ops pi3usb30532_mux_ops = {
+ .set = pi3usb30532_mux_set_mux,
+};
+
+static int pi3usb30532_mux_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct pi3usb30532_mux *mux;
+ struct mux_chip *mux_chip;
+
+ mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
+ if (IS_ERR(mux_chip))
+ return PTR_ERR(mux_chip);
+
+ mux_chip->ops = &pi3usb30532_mux_ops;
+ mux_chip->mux[0].states = MUX_USB_STATES;
+ mux = mux_chip_priv(mux_chip);
+ mux->client = client;
+
+ return devm_mux_chip_register(dev, mux_chip);
+}
+
+static const struct i2c_device_id pi3usb30532_mux_table[] = {
+ { "pi3usb30532" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, pi3usb30532_mux_table);
+
+static struct i2c_driver pi3usb30532_mux_driver = {
+ .driver = {
+ .name = "pi3usb30532",
+ },
+ .probe_new = pi3usb30532_mux_probe,
+ .id_table = pi3usb30532_mux_table,
+};
+
+module_i2c_driver(pi3usb30532_mux_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Pericom PI3USB30532 Type-C mux driver");
+MODULE_LICENSE("GPL");
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 06/11] mux: Add Pericom PI3USB30532 Type-C mux driver
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Add a driver for the Pericom PI3USB30532 Type-C cross switch /
mux chip found on some devices with a Type-C port.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/mux/Kconfig | 10 +++++
drivers/mux/Makefile | 2 +
drivers/mux/pi3usb30532.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 109 insertions(+)
create mode 100644 drivers/mux/pi3usb30532.c
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index 17938918bf93..19a3065c34e6 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -58,4 +58,14 @@ config MUX_MMIO
To compile the driver as a module, choose M here: the module will
be called mux-mmio.
+config MUX_PI3USB30532
+ tristate "Pericom PI3USB30532 Type-C cross switch driver"
+ depends on I2C
+ help
+ This driver adds support for the Pericom PI3USB30532 Type-C cross
+ switch / mux chip found on some devices with a Type-C port.
+
+ To compile the driver as a module, choose M here: the module will
+ be called mux-pi3usb30532.
+
endmenu
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index a12e812c7966..7563dbf04593 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -7,9 +7,11 @@ mux-adg792a-objs := adg792a.o
mux-gpio-objs := gpio.o
mux-mmio-objs := mmio.o
mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
+mux-pi3usb30532-objs := pi3usb30532.o
obj-$(CONFIG_MULTIPLEXER) += mux-core.o
obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
+obj-$(CONFIG_MUX_PI3USB30532) += mux-pi3usb30532.o
diff --git a/drivers/mux/pi3usb30532.c b/drivers/mux/pi3usb30532.c
new file mode 100644
index 000000000000..fa8abd851520
--- /dev/null
+++ b/drivers/mux/pi3usb30532.c
@@ -0,0 +1,97 @@
+/*
+ * Pericom PI3USB30532 Type-C cross switch / mux driver
+ *
+ * Copyright (c) 2017 Hans de Goede <hdegoede@redhat.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, or (at your option)
+ * any later version.
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
+#include <linux/mux/driver.h>
+
+#define PI3USB30532_CONF 0x00
+
+#define PI3USB30532_CONF_OPEN 0x00
+#define PI3USB30532_CONF_SWAP 0x01
+#define PI3USB30532_CONF_4LANE_DP 0x02
+#define PI3USB30532_CONF_USB3 0x04
+#define PI3USB30532_CONF_USB3_AND_2LANE_DP 0x06
+
+struct pi3usb30532_mux {
+ struct i2c_client *client;
+};
+
+static int pi3usb30532_mux_set_mux(struct mux_control *mux_ctrl, int state)
+{
+ struct pi3usb30532_mux *mux = mux_chip_priv(mux_ctrl->chip);
+ u8 conf = PI3USB30532_CONF_OPEN;
+
+ switch (state & ~MUX_USB_POLARITY_INV) {
+ case MUX_USB_NONE:
+ conf = PI3USB30532_CONF_OPEN;
+ break;
+ case MUX_USB_DEVICE:
+ case MUX_USB_HOST:
+ conf = PI3USB30532_CONF_USB3;
+ break;
+ case MUX_USB_HOST_AND_DP_SRC:
+ conf = PI3USB30532_CONF_USB3_AND_2LANE_DP;
+ break;
+ case MUX_USB_DP_SRC:
+ conf = PI3USB30532_CONF_4LANE_DP;
+ break;
+ }
+
+ if (state & MUX_USB_POLARITY_INV)
+ conf |= PI3USB30532_CONF_SWAP;
+
+ return i2c_smbus_write_byte_data(mux->client, PI3USB30532_CONF, conf);
+}
+
+static const struct mux_control_ops pi3usb30532_mux_ops = {
+ .set = pi3usb30532_mux_set_mux,
+};
+
+static int pi3usb30532_mux_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct pi3usb30532_mux *mux;
+ struct mux_chip *mux_chip;
+
+ mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
+ if (IS_ERR(mux_chip))
+ return PTR_ERR(mux_chip);
+
+ mux_chip->ops = &pi3usb30532_mux_ops;
+ mux_chip->mux[0].states = MUX_USB_STATES;
+ mux = mux_chip_priv(mux_chip);
+ mux->client = client;
+
+ return devm_mux_chip_register(dev, mux_chip);
+}
+
+static const struct i2c_device_id pi3usb30532_mux_table[] = {
+ { "pi3usb30532" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, pi3usb30532_mux_table);
+
+static struct i2c_driver pi3usb30532_mux_driver = {
+ .driver = {
+ .name = "pi3usb30532",
+ },
+ .probe_new = pi3usb30532_mux_probe,
+ .id_table = pi3usb30532_mux_table,
+};
+
+module_i2c_driver(pi3usb30532_mux_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Pericom PI3USB30532 Type-C mux driver");
+MODULE_LICENSE("GPL");
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [PATCH 06/11] mux: Add Pericom PI3USB30532 Type-C mux driver
2017-09-01 21:48 ` Hans de Goede
(?)
@ 2017-09-04 11:19 ` Peter Rosin
2017-09-05 7:46 ` Peter Rosin
-1 siblings, 1 reply; 52+ messages in thread
From: Peter Rosin @ 2017-09-04 11:19 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
Hi!
One comment inline...
On 2017-09-01 23:48, Hans de Goede wrote:
> Add a driver for the Pericom PI3USB30532 Type-C cross switch /
> mux chip found on some devices with a Type-C port.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/mux/Kconfig | 10 +++++
> drivers/mux/Makefile | 2 +
> drivers/mux/pi3usb30532.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 109 insertions(+)
> create mode 100644 drivers/mux/pi3usb30532.c
>
> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
> index 17938918bf93..19a3065c34e6 100644
> --- a/drivers/mux/Kconfig
> +++ b/drivers/mux/Kconfig
> @@ -58,4 +58,14 @@ config MUX_MMIO
> To compile the driver as a module, choose M here: the module will
> be called mux-mmio.
>
> +config MUX_PI3USB30532
> + tristate "Pericom PI3USB30532 Type-C cross switch driver"
> + depends on I2C
> + help
> + This driver adds support for the Pericom PI3USB30532 Type-C cross
> + switch / mux chip found on some devices with a Type-C port.
> +
> + To compile the driver as a module, choose M here: the module will
> + be called mux-pi3usb30532.
> +
> endmenu
> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
> index a12e812c7966..7563dbf04593 100644
> --- a/drivers/mux/Makefile
> +++ b/drivers/mux/Makefile
> @@ -7,9 +7,11 @@ mux-adg792a-objs := adg792a.o
> mux-gpio-objs := gpio.o
> mux-mmio-objs := mmio.o
> mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
> +mux-pi3usb30532-objs := pi3usb30532.o
>
> obj-$(CONFIG_MULTIPLEXER) += mux-core.o
> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
> obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
> +obj-$(CONFIG_MUX_PI3USB30532) += mux-pi3usb30532.o
> diff --git a/drivers/mux/pi3usb30532.c b/drivers/mux/pi3usb30532.c
> new file mode 100644
> index 000000000000..fa8abd851520
> --- /dev/null
> +++ b/drivers/mux/pi3usb30532.c
> @@ -0,0 +1,97 @@
> +/*
> + * Pericom PI3USB30532 Type-C cross switch / mux driver
> + *
> + * Copyright (c) 2017 Hans de Goede <hdegoede@redhat.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, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
> +#include <linux/mux/driver.h>
> +
> +#define PI3USB30532_CONF 0x00
> +
> +#define PI3USB30532_CONF_OPEN 0x00
> +#define PI3USB30532_CONF_SWAP 0x01
> +#define PI3USB30532_CONF_4LANE_DP 0x02
> +#define PI3USB30532_CONF_USB3 0x04
> +#define PI3USB30532_CONF_USB3_AND_2LANE_DP 0x06
> +
> +struct pi3usb30532_mux {
> + struct i2c_client *client;
> +};
> +
> +static int pi3usb30532_mux_set_mux(struct mux_control *mux_ctrl, int state)
> +{
> + struct pi3usb30532_mux *mux = mux_chip_priv(mux_ctrl->chip);
The "mux" variable name is used for the mux_control in other drivers, and
I don't think the private data is needed. Like so:
static int pi3usb30532_mux_set_mux(struct mux_control *mux, int state)
{
struct i2c_client *i2c = to_i2c_client(mux->chip->dev.parent);
...
Cheers,
Peter
> + u8 conf = PI3USB30532_CONF_OPEN;
> +
> + switch (state & ~MUX_USB_POLARITY_INV) {
> + case MUX_USB_NONE:
> + conf = PI3USB30532_CONF_OPEN;
> + break;
> + case MUX_USB_DEVICE:
> + case MUX_USB_HOST:
> + conf = PI3USB30532_CONF_USB3;
> + break;
> + case MUX_USB_HOST_AND_DP_SRC:
> + conf = PI3USB30532_CONF_USB3_AND_2LANE_DP;
> + break;
> + case MUX_USB_DP_SRC:
> + conf = PI3USB30532_CONF_4LANE_DP;
> + break;
> + }
> +
> + if (state & MUX_USB_POLARITY_INV)
> + conf |= PI3USB30532_CONF_SWAP;
> +
> + return i2c_smbus_write_byte_data(mux->client, PI3USB30532_CONF, conf);
> +}
> +
> +static const struct mux_control_ops pi3usb30532_mux_ops = {
> + .set = pi3usb30532_mux_set_mux,
> +};
> +
> +static int pi3usb30532_mux_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct pi3usb30532_mux *mux;
> + struct mux_chip *mux_chip;
> +
> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
> + if (IS_ERR(mux_chip))
> + return PTR_ERR(mux_chip);
> +
> + mux_chip->ops = &pi3usb30532_mux_ops;
> + mux_chip->mux[0].states = MUX_USB_STATES;
> + mux = mux_chip_priv(mux_chip);
> + mux->client = client;
> +
> + return devm_mux_chip_register(dev, mux_chip);
> +}
> +
> +static const struct i2c_device_id pi3usb30532_mux_table[] = {
> + { "pi3usb30532" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, pi3usb30532_mux_table);
> +
> +static struct i2c_driver pi3usb30532_mux_driver = {
> + .driver = {
> + .name = "pi3usb30532",
> + },
> + .probe_new = pi3usb30532_mux_probe,
> + .id_table = pi3usb30532_mux_table,
> +};
> +
> +module_i2c_driver(pi3usb30532_mux_driver);
> +
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> +MODULE_DESCRIPTION("Pericom PI3USB30532 Type-C mux driver");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 06/11] mux: Add Pericom PI3USB30532 Type-C mux driver
2017-09-04 11:19 ` Peter Rosin
@ 2017-09-05 7:46 ` Peter Rosin
0 siblings, 0 replies; 52+ messages in thread
From: Peter Rosin @ 2017-09-05 7:46 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb
On 2017-09-04 13:19, Peter Rosin wrote:
> Hi!
>
> One comment inline...
Oh, and one more small nit, I think you should do
s/pi3usb30532_mux/pi3usb30532/g
to shorten the identifiers a bit. The _mux suffix (or infix) is kind of
selfevident from where the file lives anyway. pi3usb30532_mux_set_mux in
particular feels a bit much....
Cheers,
peda
> On 2017-09-01 23:48, Hans de Goede wrote:
>> Add a driver for the Pericom PI3USB30532 Type-C cross switch /
>> mux chip found on some devices with a Type-C port.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/mux/Kconfig | 10 +++++
>> drivers/mux/Makefile | 2 +
>> drivers/mux/pi3usb30532.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 109 insertions(+)
>> create mode 100644 drivers/mux/pi3usb30532.c
>>
>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
>> index 17938918bf93..19a3065c34e6 100644
>> --- a/drivers/mux/Kconfig
>> +++ b/drivers/mux/Kconfig
>> @@ -58,4 +58,14 @@ config MUX_MMIO
>> To compile the driver as a module, choose M here: the module will
>> be called mux-mmio.
>>
>> +config MUX_PI3USB30532
>> + tristate "Pericom PI3USB30532 Type-C cross switch driver"
>> + depends on I2C
>> + help
>> + This driver adds support for the Pericom PI3USB30532 Type-C cross
>> + switch / mux chip found on some devices with a Type-C port.
>> +
>> + To compile the driver as a module, choose M here: the module will
>> + be called mux-pi3usb30532.
>> +
>> endmenu
>> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
>> index a12e812c7966..7563dbf04593 100644
>> --- a/drivers/mux/Makefile
>> +++ b/drivers/mux/Makefile
>> @@ -7,9 +7,11 @@ mux-adg792a-objs := adg792a.o
>> mux-gpio-objs := gpio.o
>> mux-mmio-objs := mmio.o
>> mux-intel_cht_usb_mux-objs := intel_cht_usb_mux.o
>> +mux-pi3usb30532-objs := pi3usb30532.o
>>
>> obj-$(CONFIG_MULTIPLEXER) += mux-core.o
>> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
>> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
>> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
>> obj-$(CONFIG_MUX_CHT_USB_MUX) += mux-intel_cht_usb_mux.o
>> +obj-$(CONFIG_MUX_PI3USB30532) += mux-pi3usb30532.o
>> diff --git a/drivers/mux/pi3usb30532.c b/drivers/mux/pi3usb30532.c
>> new file mode 100644
>> index 000000000000..fa8abd851520
>> --- /dev/null
>> +++ b/drivers/mux/pi3usb30532.c
>> @@ -0,0 +1,97 @@
>> +/*
>> + * Pericom PI3USB30532 Type-C cross switch / mux driver
>> + *
>> + * Copyright (c) 2017 Hans de Goede <hdegoede@redhat.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, or (at your option)
>> + * any later version.
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mux/consumer.h> /* For the MUX_USB_* defines */
>> +#include <linux/mux/driver.h>
>> +
>> +#define PI3USB30532_CONF 0x00
>> +
>> +#define PI3USB30532_CONF_OPEN 0x00
>> +#define PI3USB30532_CONF_SWAP 0x01
>> +#define PI3USB30532_CONF_4LANE_DP 0x02
>> +#define PI3USB30532_CONF_USB3 0x04
>> +#define PI3USB30532_CONF_USB3_AND_2LANE_DP 0x06
>> +
>> +struct pi3usb30532_mux {
>> + struct i2c_client *client;
>> +};
>> +
>> +static int pi3usb30532_mux_set_mux(struct mux_control *mux_ctrl, int state)
>> +{
>> + struct pi3usb30532_mux *mux = mux_chip_priv(mux_ctrl->chip);
>
> The "mux" variable name is used for the mux_control in other drivers, and
> I don't think the private data is needed. Like so:
>
> static int pi3usb30532_mux_set_mux(struct mux_control *mux, int state)
> {
> struct i2c_client *i2c = to_i2c_client(mux->chip->dev.parent);
>
> ...
>
> Cheers,
> Peter
>
>> + u8 conf = PI3USB30532_CONF_OPEN;
>> +
>> + switch (state & ~MUX_USB_POLARITY_INV) {
>> + case MUX_USB_NONE:
>> + conf = PI3USB30532_CONF_OPEN;
>> + break;
>> + case MUX_USB_DEVICE:
>> + case MUX_USB_HOST:
>> + conf = PI3USB30532_CONF_USB3;
>> + break;
>> + case MUX_USB_HOST_AND_DP_SRC:
>> + conf = PI3USB30532_CONF_USB3_AND_2LANE_DP;
>> + break;
>> + case MUX_USB_DP_SRC:
>> + conf = PI3USB30532_CONF_4LANE_DP;
>> + break;
>> + }
>> +
>> + if (state & MUX_USB_POLARITY_INV)
>> + conf |= PI3USB30532_CONF_SWAP;
>> +
>> + return i2c_smbus_write_byte_data(mux->client, PI3USB30532_CONF, conf);
>> +}
>> +
>> +static const struct mux_control_ops pi3usb30532_mux_ops = {
>> + .set = pi3usb30532_mux_set_mux,
>> +};
>> +
>> +static int pi3usb30532_mux_probe(struct i2c_client *client)
>> +{
>> + struct device *dev = &client->dev;
>> + struct pi3usb30532_mux *mux;
>> + struct mux_chip *mux_chip;
>> +
>> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux));
>> + if (IS_ERR(mux_chip))
>> + return PTR_ERR(mux_chip);
>> +
>> + mux_chip->ops = &pi3usb30532_mux_ops;
>> + mux_chip->mux[0].states = MUX_USB_STATES;
>> + mux = mux_chip_priv(mux_chip);
>> + mux->client = client;
>> +
>> + return devm_mux_chip_register(dev, mux_chip);
>> +}
>> +
>> +static const struct i2c_device_id pi3usb30532_mux_table[] = {
>> + { "pi3usb30532" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, pi3usb30532_mux_table);
>> +
>> +static struct i2c_driver pi3usb30532_mux_driver = {
>> + .driver = {
>> + .name = "pi3usb30532",
>> + },
>> + .probe_new = pi3usb30532_mux_probe,
>> + .id_table = pi3usb30532_mux_table,
>> +};
>> +
>> +module_i2c_driver(pi3usb30532_mux_driver);
>> +
>> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
>> +MODULE_DESCRIPTION("Pericom PI3USB30532 Type-C mux driver");
>> +MODULE_LICENSE("GPL");
>>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 07/11] extcon: intel-int3496: Add support for controlling the USB-role mux
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede
Cherry Trail SoCs have a built-in USB-role mux for switching between
the host and device controllers, rather then using an external mux
controller by a GPIO.
There is a driver using the mux-subsys to control this mux, this
commit adds support to the intel-int3496 driver to get a mux_controller
handle for the mux and set the mux through the mux-subsys rather then
through a GPIO.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/extcon/Kconfig | 1 +
drivers/extcon/extcon-intel-int3496.c | 59 ++++++++++++++++++++++++++++++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index a7bca4207f44..7ab6acc9708a 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -45,6 +45,7 @@ config EXTCON_GPIO
config EXTCON_INTEL_INT3496
tristate "Intel INT3496 ACPI device extcon driver"
depends on GPIOLIB && ACPI && (X86 || COMPILE_TEST)
+ select MULTIPLEXER
help
Say Y here to enable extcon support for USB OTG ports controlled by
an Intel INT3496 ACPI device.
diff --git a/drivers/extcon/extcon-intel-int3496.c b/drivers/extcon/extcon-intel-int3496.c
index 1a45e745717d..cea9c9b1521c 100644
--- a/drivers/extcon/extcon-intel-int3496.c
+++ b/drivers/extcon/extcon-intel-int3496.c
@@ -23,8 +23,12 @@
#include <linux/gpio.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/mux/consumer.h>
#include <linux/platform_device.h>
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
#define INT3496_GPIO_USB_ID 0
#define INT3496_GPIO_VBUS_EN 1
#define INT3496_GPIO_USB_MUX 2
@@ -37,6 +41,8 @@ struct int3496_data {
struct gpio_desc *gpio_usb_id;
struct gpio_desc *gpio_vbus_en;
struct gpio_desc *gpio_usb_mux;
+ struct mux_control *usb_mux;
+ bool usb_mux_set;
int usb_id_irq;
};
@@ -56,11 +62,31 @@ static const struct acpi_gpio_mapping acpi_int3496_default_gpios[] = {
{ },
};
+static struct mux_lookup acpi_int3496_cht_mux_lookup[] = {
+ {
+ .provider = "intel_cht_usb_mux",
+ .dev_id = "INT3496:00",
+ .mux_name = "usb-role-mux",
+ },
+};
+
+#define ICPU(model) { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+static const struct x86_cpu_id cht_cpu_ids[] = {
+ ICPU(INTEL_FAM6_ATOM_AIRMONT), /* Braswell, Cherry Trail */
+ {}
+};
+
+static bool int3496_soc_has_mux(void)
+{
+ return x86_match_cpu(cht_cpu_ids);
+}
+
static void int3496_do_usb_id(struct work_struct *work)
{
struct int3496_data *data =
container_of(work, struct int3496_data, work.work);
- int id = gpiod_get_value_cansleep(data->gpio_usb_id);
+ int ret, id = gpiod_get_value_cansleep(data->gpio_usb_id);
/* id == 1: PERIPHERAL, id == 0: HOST */
dev_dbg(data->dev, "Connected %s cable\n", id ? "PERIPHERAL" : "HOST");
@@ -72,6 +98,22 @@ static void int3496_do_usb_id(struct work_struct *work)
if (!IS_ERR(data->gpio_usb_mux))
gpiod_direction_output(data->gpio_usb_mux, id);
+ if (data->usb_mux) {
+ /*
+ * The mux framework expects multiple competing users, we must
+ * release our previous setting before applying the new one.
+ */
+ if (data->usb_mux_set)
+ mux_control_deselect(data->usb_mux);
+
+ ret = mux_control_select(data->usb_mux,
+ id ? MUX_USB_DEVICE : MUX_USB_HOST);
+ if (ret)
+ dev_err(data->dev, "Error setting mux: %d\n", ret);
+
+ data->usb_mux_set = ret == 0;
+ }
+
if (!IS_ERR(data->gpio_vbus_en))
gpiod_direction_output(data->gpio_vbus_en, !id);
@@ -107,6 +149,21 @@ static int int3496_probe(struct platform_device *pdev)
data->dev = dev;
INIT_DELAYED_WORK(&data->work, int3496_do_usb_id);
+ if (int3496_soc_has_mux()) {
+ mux_add_table(acpi_int3496_cht_mux_lookup,
+ ARRAY_SIZE(acpi_int3496_cht_mux_lookup));
+ data->usb_mux = devm_mux_control_get(dev, "usb-role-mux");
+ /* Doing this here keeps our error handling clean. */
+ mux_remove_table(acpi_int3496_cht_mux_lookup,
+ ARRAY_SIZE(acpi_int3496_cht_mux_lookup));
+ if (IS_ERR(data->usb_mux)) {
+ ret = PTR_ERR(data->usb_mux);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "can't get mux: %d\n", ret);
+ return ret;
+ }
+ }
+
data->gpio_usb_id = devm_gpiod_get(dev, "id", GPIOD_IN);
if (IS_ERR(data->gpio_usb_id)) {
ret = PTR_ERR(data->gpio_usb_id);
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 07/11] extcon: intel-int3496: Add support for controlling the USB-role mux
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Cherry Trail SoCs have a built-in USB-role mux for switching between
the host and device controllers, rather then using an external mux
controller by a GPIO.
There is a driver using the mux-subsys to control this mux, this
commit adds support to the intel-int3496 driver to get a mux_controller
handle for the mux and set the mux through the mux-subsys rather then
through a GPIO.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/extcon/Kconfig | 1 +
drivers/extcon/extcon-intel-int3496.c | 59 ++++++++++++++++++++++++++++++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index a7bca4207f44..7ab6acc9708a 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -45,6 +45,7 @@ config EXTCON_GPIO
config EXTCON_INTEL_INT3496
tristate "Intel INT3496 ACPI device extcon driver"
depends on GPIOLIB && ACPI && (X86 || COMPILE_TEST)
+ select MULTIPLEXER
help
Say Y here to enable extcon support for USB OTG ports controlled by
an Intel INT3496 ACPI device.
diff --git a/drivers/extcon/extcon-intel-int3496.c b/drivers/extcon/extcon-intel-int3496.c
index 1a45e745717d..cea9c9b1521c 100644
--- a/drivers/extcon/extcon-intel-int3496.c
+++ b/drivers/extcon/extcon-intel-int3496.c
@@ -23,8 +23,12 @@
#include <linux/gpio.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/mux/consumer.h>
#include <linux/platform_device.h>
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
#define INT3496_GPIO_USB_ID 0
#define INT3496_GPIO_VBUS_EN 1
#define INT3496_GPIO_USB_MUX 2
@@ -37,6 +41,8 @@ struct int3496_data {
struct gpio_desc *gpio_usb_id;
struct gpio_desc *gpio_vbus_en;
struct gpio_desc *gpio_usb_mux;
+ struct mux_control *usb_mux;
+ bool usb_mux_set;
int usb_id_irq;
};
@@ -56,11 +62,31 @@ static const struct acpi_gpio_mapping acpi_int3496_default_gpios[] = {
{ },
};
+static struct mux_lookup acpi_int3496_cht_mux_lookup[] = {
+ {
+ .provider = "intel_cht_usb_mux",
+ .dev_id = "INT3496:00",
+ .mux_name = "usb-role-mux",
+ },
+};
+
+#define ICPU(model) { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+static const struct x86_cpu_id cht_cpu_ids[] = {
+ ICPU(INTEL_FAM6_ATOM_AIRMONT), /* Braswell, Cherry Trail */
+ {}
+};
+
+static bool int3496_soc_has_mux(void)
+{
+ return x86_match_cpu(cht_cpu_ids);
+}
+
static void int3496_do_usb_id(struct work_struct *work)
{
struct int3496_data *data =
container_of(work, struct int3496_data, work.work);
- int id = gpiod_get_value_cansleep(data->gpio_usb_id);
+ int ret, id = gpiod_get_value_cansleep(data->gpio_usb_id);
/* id == 1: PERIPHERAL, id == 0: HOST */
dev_dbg(data->dev, "Connected %s cable\n", id ? "PERIPHERAL" : "HOST");
@@ -72,6 +98,22 @@ static void int3496_do_usb_id(struct work_struct *work)
if (!IS_ERR(data->gpio_usb_mux))
gpiod_direction_output(data->gpio_usb_mux, id);
+ if (data->usb_mux) {
+ /*
+ * The mux framework expects multiple competing users, we must
+ * release our previous setting before applying the new one.
+ */
+ if (data->usb_mux_set)
+ mux_control_deselect(data->usb_mux);
+
+ ret = mux_control_select(data->usb_mux,
+ id ? MUX_USB_DEVICE : MUX_USB_HOST);
+ if (ret)
+ dev_err(data->dev, "Error setting mux: %d\n", ret);
+
+ data->usb_mux_set = ret == 0;
+ }
+
if (!IS_ERR(data->gpio_vbus_en))
gpiod_direction_output(data->gpio_vbus_en, !id);
@@ -107,6 +149,21 @@ static int int3496_probe(struct platform_device *pdev)
data->dev = dev;
INIT_DELAYED_WORK(&data->work, int3496_do_usb_id);
+ if (int3496_soc_has_mux()) {
+ mux_add_table(acpi_int3496_cht_mux_lookup,
+ ARRAY_SIZE(acpi_int3496_cht_mux_lookup));
+ data->usb_mux = devm_mux_control_get(dev, "usb-role-mux");
+ /* Doing this here keeps our error handling clean. */
+ mux_remove_table(acpi_int3496_cht_mux_lookup,
+ ARRAY_SIZE(acpi_int3496_cht_mux_lookup));
+ if (IS_ERR(data->usb_mux)) {
+ ret = PTR_ERR(data->usb_mux);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "can't get mux: %d\n", ret);
+ return ret;
+ }
+ }
+
data->gpio_usb_id = devm_gpiod_get(dev, "id", GPIOD_IN);
if (IS_ERR(data->gpio_usb_id)) {
ret = PTR_ERR(data->gpio_usb_id);
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [PATCH 07/11] extcon: intel-int3496: Add support for controlling the USB-role mux
2017-09-01 21:48 ` Hans de Goede
@ 2017-09-02 10:39 ` Andy Shevchenko
-1 siblings, 0 replies; 52+ messages in thread
From: Andy Shevchenko @ 2017-09-02 10:39 UTC (permalink / raw)
To: Hans de Goede
Cc: devel, Kuppuswamy Sathyanarayanan, Heikki Krogerus, Mathias Nyman,
Greg Kroah-Hartman, USB, linux-kernel@vger.kernel.org,
Platform Driver, Sathyanarayanan Kuppuswamy Natarajan,
Chanwoo Choi, MyungJoo Ham, Guenter Roeck, Darren Hart,
Peter Rosin, Andy Shevchenko
On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Cherry Trail SoCs have a built-in USB-role mux for switching between
> the host and device controllers, rather then using an external mux
> controller by a GPIO.
>
> There is a driver using the mux-subsys to control this mux, this
> commit adds support to the intel-int3496 driver to get a mux_controller
> handle for the mux and set the mux through the mux-subsys rather then
> through a GPIO.
> tristate "Intel INT3496 ACPI device extcon driver"
> depends on GPIOLIB && ACPI && (X86 || COMPILE_TEST)
> + select MULTIPLEXER
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
I think it is going to fail when !X86 && COMPILE_TEST.
> static void int3496_do_usb_id(struct work_struct *work)
> {
> struct int3496_data *data =
> container_of(work, struct int3496_data, work.work);
> - int id = gpiod_get_value_cansleep(data->gpio_usb_id);
> + int ret, id = gpiod_get_value_cansleep(data->gpio_usb_id);
Better to keep them on separate lines.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 07/11] extcon: intel-int3496: Add support for controlling the USB-role mux
@ 2017-09-02 10:39 ` Andy Shevchenko
0 siblings, 0 replies; 52+ messages in thread
From: Andy Shevchenko @ 2017-09-02 10:39 UTC (permalink / raw)
To: Hans de Goede
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, USB
On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Cherry Trail SoCs have a built-in USB-role mux for switching between
> the host and device controllers, rather then using an external mux
> controller by a GPIO.
>
> There is a driver using the mux-subsys to control this mux, this
> commit adds support to the intel-int3496 driver to get a mux_controller
> handle for the mux and set the mux through the mux-subsys rather then
> through a GPIO.
> tristate "Intel INT3496 ACPI device extcon driver"
> depends on GPIOLIB && ACPI && (X86 || COMPILE_TEST)
> + select MULTIPLEXER
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
I think it is going to fail when !X86 && COMPILE_TEST.
> static void int3496_do_usb_id(struct work_struct *work)
> {
> struct int3496_data *data =
> container_of(work, struct int3496_data, work.work);
> - int id = gpiod_get_value_cansleep(data->gpio_usb_id);
> + int ret, id = gpiod_get_value_cansleep(data->gpio_usb_id);
Better to keep them on separate lines.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 07/11] extcon: intel-int3496: Add support for controlling the USB-role mux
2017-09-02 10:39 ` Andy Shevchenko
(?)
@ 2017-09-04 14:11 ` Hans de Goede
-1 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-04 14:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, USB
Hi,
On 02-09-17 12:39, Andy Shevchenko wrote:
> On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Cherry Trail SoCs have a built-in USB-role mux for switching between
>> the host and device controllers, rather then using an external mux
>> controller by a GPIO.
>>
>> There is a driver using the mux-subsys to control this mux, this
>> commit adds support to the intel-int3496 driver to get a mux_controller
>> handle for the mux and set the mux through the mux-subsys rather then
>> through a GPIO.
>
>> tristate "Intel INT3496 ACPI device extcon driver"
>> depends on GPIOLIB && ACPI && (X86 || COMPILE_TEST)
>> + select MULTIPLEXER
>
>> +#include <asm/cpu_device_id.h>
>> +#include <asm/intel-family.h>
>
> I think it is going to fail when !X86 && COMPILE_TEST.
Good catch, I've dropped the || COMPILE_TEST for v2
>> static void int3496_do_usb_id(struct work_struct *work)
>> {
>> struct int3496_data *data =
>> container_of(work, struct int3496_data, work.work);
>> - int id = gpiod_get_value_cansleep(data->gpio_usb_id);
>> + int ret, id = gpiod_get_value_cansleep(data->gpio_usb_id);
>
> Better to keep them on separate lines.
Ok, fixed for v2.
Regards,
Hans
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 08/11] staging: typec: tcpm: Set mux to device mode when configured as such
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
` (6 preceding siblings ...)
2017-09-01 21:48 ` Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (3 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
Setting the mux to TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT when the
data-role is device is not correct. Plenty of devices support operating
as USB device through a (separate) USB device controller.
So this commit instead splits out TYPEC_MUX_USB into TYPEC_MUX_USB_HOST
and TYPEC_MUX_USB_DEVICE and makes tcpm_set_roles() set the mux
accordingly.
Likewise TCPC_MUX_DP gets renamed to TCPC_MUX_DP_SRC to make clear that
this is for configuring the Type-C port as a Display Port source, not a
sink.
Last this commit makes tcpm_reset_port() to set the mux to
TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT so that it does not and up
staying in host (and with this commit also device) mode after a detach.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/typec/tcpm.c | 7 ++++---
drivers/staging/typec/tcpm.h | 22 ++++++++++++++--------
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 8af62e74d54c..ffe7e26d4ed3 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -752,11 +752,11 @@ static int tcpm_set_roles(struct tcpm_port *port, bool attached,
int ret;
if (data == TYPEC_HOST)
- ret = tcpm_mux_set(port, TYPEC_MUX_USB,
+ ret = tcpm_mux_set(port, TYPEC_MUX_USB_HOST,
TCPC_USB_SWITCH_CONNECT);
else
- ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
- TCPC_USB_SWITCH_DISCONNECT);
+ ret = tcpm_mux_set(port, TYPEC_MUX_USB_DEVICE,
+ TCPC_USB_SWITCH_CONNECT);
if (ret < 0)
return ret;
@@ -2025,6 +2025,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
tcpm_init_vconn(port);
tcpm_set_current_limit(port, 0, 0);
tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
+ tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
tcpm_set_attached_state(port, false);
port->try_src_count = 0;
port->try_snk_count = 0;
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 7e9a6b7b5cd6..f662eed48c86 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -83,17 +83,23 @@ enum tcpc_usb_switch {
};
/* Mux state attributes */
-#define TCPC_MUX_USB_ENABLED BIT(0) /* USB enabled */
-#define TCPC_MUX_DP_ENABLED BIT(1) /* DP enabled */
-#define TCPC_MUX_POLARITY_INVERTED BIT(2) /* Polarity inverted */
+#define TCPC_MUX_USB_DEVICE_ENABLED BIT(0) /* USB device enabled */
+#define TCPC_MUX_USB_HOST_ENABLED BIT(1) /* USB host enabled */
+#define TCPC_MUX_DP_SRC_ENABLED BIT(2) /* DP enabled */
+#define TCPC_MUX_POLARITY_INVERTED BIT(3) /* Polarity inverted */
/* Mux modes, decoded to attributes */
enum tcpc_mux_mode {
- TYPEC_MUX_NONE = 0, /* Open switch */
- TYPEC_MUX_USB = TCPC_MUX_USB_ENABLED, /* USB only */
- TYPEC_MUX_DP = TCPC_MUX_DP_ENABLED, /* DP only */
- TYPEC_MUX_DOCK = TCPC_MUX_USB_ENABLED | /* Both USB and DP */
- TCPC_MUX_DP_ENABLED,
+ /* Open switch */
+ TYPEC_MUX_NONE = 0,
+ /* USB device only */
+ TYPEC_MUX_USB_DEVICE = TCPC_MUX_USB_DEVICE_ENABLED,
+ /* USB host only */
+ TYPEC_MUX_USB_HOST = TCPC_MUX_USB_HOST_ENABLED,
+ /* DP source only */
+ TYPEC_MUX_DP = TCPC_MUX_DP_SRC_ENABLED,
+ /* Both USB host and DP source */
+ TYPEC_MUX_DOCK = TCPC_MUX_USB_HOST_ENABLED | TCPC_MUX_DP_SRC_ENABLED,
};
struct tcpc_mux_dev {
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 09/11] staging: typec: Add Generic TCPC mux driver using the mux subsys
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede
So far the mux functionality of the tcpm code has not been hooked up
in any tcpc drivers. This commit adds a generic TCPC mux driver using
the mux subsys, which tcpc drivers can use to provide mux functionality
in cases where an external my is used.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/typec/Makefile | 2 +-
drivers/staging/typec/tcpc_gen_mux.c | 129 +++++++++++++++++++++++++++++++++++
drivers/staging/typec/tcpm.h | 2 +
3 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 drivers/staging/typec/tcpc_gen_mux.c
diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index 30a7e29cbc9e..623d78114ee3 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1,3 +1,3 @@
-obj-$(CONFIG_TYPEC_TCPM) += tcpm.o
+obj-$(CONFIG_TYPEC_TCPM) += tcpm.o tcpc_gen_mux.o
obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o
obj-y += fusb302/
diff --git a/drivers/staging/typec/tcpc_gen_mux.c b/drivers/staging/typec/tcpc_gen_mux.c
new file mode 100644
index 000000000000..dc725e17b106
--- /dev/null
+++ b/drivers/staging/typec/tcpc_gen_mux.c
@@ -0,0 +1,129 @@
+/*
+ * Generic TCPC mux driver using the mux subsys
+ *
+ * Copyright (c) 2017 Hans de Goede <hdegoede@redhat.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, or (at your option)
+ * any later version.
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h>
+#include "tcpm.h"
+
+struct tcpc_gen_mux_data {
+ struct tcpc_mux_dev mux;
+ struct device *dev;
+ struct mux_control *type_c_mode_mux; /* Type-C cross switch / mux */
+ struct mux_control *usb_role_mux; /* USB Device / Host mode mux */
+ bool muxes_set;
+};
+
+static int tcpc_gen_mux_set(struct tcpc_mux_dev *mux_dev,
+ enum tcpc_mux_mode mux_mode,
+ enum tcpc_usb_switch usb_config,
+ enum typec_cc_polarity polarity)
+{
+ struct tcpc_gen_mux_data *data =
+ container_of(mux_dev, struct tcpc_gen_mux_data, mux);
+ unsigned int state = MUX_USB_NONE;
+ int ret;
+
+ switch (mux_mode) {
+ case TYPEC_MUX_NONE:
+ state = MUX_USB_NONE;
+ break;
+ case TYPEC_MUX_USB_DEVICE:
+ state = MUX_USB_DEVICE;
+ break;
+ case TYPEC_MUX_USB_HOST:
+ state = MUX_USB_HOST;
+ break;
+ case TYPEC_MUX_DP:
+ state = MUX_USB_DP_SRC;
+ break;
+ case TYPEC_MUX_DOCK:
+ state = MUX_USB_HOST_AND_DP_SRC;
+ break;
+ }
+
+ if (polarity)
+ state |= MUX_USB_POLARITY_INV;
+
+ /*
+ * The mux framework expects multiple competing users, so we must
+ * release our previous setting before applying the new one.
+ */
+ if (data->muxes_set) {
+ mux_control_deselect(data->type_c_mode_mux);
+ if (data->usb_role_mux)
+ mux_control_deselect(data->usb_role_mux);
+ data->muxes_set = false;
+ }
+
+ ret = mux_control_select(data->type_c_mode_mux, state);
+ if (ret) {
+ dev_err(data->dev, "Error setting Type-C mode mux: %d\n", ret);
+ return ret;
+ }
+
+ if (!data->usb_role_mux)
+ goto out;
+
+ ret = mux_control_select(data->usb_role_mux, state);
+ if (ret) {
+ dev_err(data->dev, "Error setting USB role mux: %d\n", ret);
+ mux_control_deselect(data->type_c_mode_mux);
+ return ret;
+ }
+
+out:
+ data->muxes_set = true;
+ return 0;
+}
+
+struct tcpc_mux_dev *devm_tcpc_gen_mux_create(struct device *dev)
+{
+ struct tcpc_gen_mux_data *data;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return ERR_PTR(-ENOMEM);
+
+ data->type_c_mode_mux = devm_mux_control_get(dev, "type-c-mode-mux");
+ if (IS_ERR(data->type_c_mode_mux)) {
+ ret = PTR_ERR(data->type_c_mode_mux);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Error getting Type-C mux: %d\n", ret);
+ return ERR_PTR(-ret);
+ }
+
+ data->usb_role_mux = devm_mux_control_get(dev, "usb-role-mux");
+ if (IS_ERR(data->usb_role_mux)) {
+ ret = PTR_ERR(data->usb_role_mux);
+ /* The USB role mux is optional */
+ if (ret == -ENODEV) {
+ data->usb_role_mux = NULL;
+ } else {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Error getting USB role mux: %d\n",
+ ret);
+ return ERR_PTR(-ret);
+ }
+ }
+
+ data->dev = dev;
+ data->mux.set = tcpc_gen_mux_set;
+
+ return &data->mux;
+}
+EXPORT_SYMBOL_GPL(devm_tcpc_gen_mux_create);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Generic Type-C mux driver using the mux subsys");
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index f662eed48c86..36789005a2b0 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -164,4 +164,6 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
void tcpm_pd_hard_reset(struct tcpm_port *port);
void tcpm_tcpc_reset(struct tcpm_port *port);
+struct tcpc_mux_dev *devm_tcpc_gen_mux_create(struct device *dev);
+
#endif /* __LINUX_USB_TCPM_H */
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 09/11] staging: typec: Add Generic TCPC mux driver using the mux subsys
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
So far the mux functionality of the tcpm code has not been hooked up
in any tcpc drivers. This commit adds a generic TCPC mux driver using
the mux subsys, which tcpc drivers can use to provide mux functionality
in cases where an external my is used.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/typec/Makefile | 2 +-
drivers/staging/typec/tcpc_gen_mux.c | 129 +++++++++++++++++++++++++++++++++++
drivers/staging/typec/tcpm.h | 2 +
3 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 drivers/staging/typec/tcpc_gen_mux.c
diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index 30a7e29cbc9e..623d78114ee3 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1,3 +1,3 @@
-obj-$(CONFIG_TYPEC_TCPM) += tcpm.o
+obj-$(CONFIG_TYPEC_TCPM) += tcpm.o tcpc_gen_mux.o
obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o
obj-y += fusb302/
diff --git a/drivers/staging/typec/tcpc_gen_mux.c b/drivers/staging/typec/tcpc_gen_mux.c
new file mode 100644
index 000000000000..dc725e17b106
--- /dev/null
+++ b/drivers/staging/typec/tcpc_gen_mux.c
@@ -0,0 +1,129 @@
+/*
+ * Generic TCPC mux driver using the mux subsys
+ *
+ * Copyright (c) 2017 Hans de Goede <hdegoede@redhat.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, or (at your option)
+ * any later version.
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h>
+#include "tcpm.h"
+
+struct tcpc_gen_mux_data {
+ struct tcpc_mux_dev mux;
+ struct device *dev;
+ struct mux_control *type_c_mode_mux; /* Type-C cross switch / mux */
+ struct mux_control *usb_role_mux; /* USB Device / Host mode mux */
+ bool muxes_set;
+};
+
+static int tcpc_gen_mux_set(struct tcpc_mux_dev *mux_dev,
+ enum tcpc_mux_mode mux_mode,
+ enum tcpc_usb_switch usb_config,
+ enum typec_cc_polarity polarity)
+{
+ struct tcpc_gen_mux_data *data =
+ container_of(mux_dev, struct tcpc_gen_mux_data, mux);
+ unsigned int state = MUX_USB_NONE;
+ int ret;
+
+ switch (mux_mode) {
+ case TYPEC_MUX_NONE:
+ state = MUX_USB_NONE;
+ break;
+ case TYPEC_MUX_USB_DEVICE:
+ state = MUX_USB_DEVICE;
+ break;
+ case TYPEC_MUX_USB_HOST:
+ state = MUX_USB_HOST;
+ break;
+ case TYPEC_MUX_DP:
+ state = MUX_USB_DP_SRC;
+ break;
+ case TYPEC_MUX_DOCK:
+ state = MUX_USB_HOST_AND_DP_SRC;
+ break;
+ }
+
+ if (polarity)
+ state |= MUX_USB_POLARITY_INV;
+
+ /*
+ * The mux framework expects multiple competing users, so we must
+ * release our previous setting before applying the new one.
+ */
+ if (data->muxes_set) {
+ mux_control_deselect(data->type_c_mode_mux);
+ if (data->usb_role_mux)
+ mux_control_deselect(data->usb_role_mux);
+ data->muxes_set = false;
+ }
+
+ ret = mux_control_select(data->type_c_mode_mux, state);
+ if (ret) {
+ dev_err(data->dev, "Error setting Type-C mode mux: %d\n", ret);
+ return ret;
+ }
+
+ if (!data->usb_role_mux)
+ goto out;
+
+ ret = mux_control_select(data->usb_role_mux, state);
+ if (ret) {
+ dev_err(data->dev, "Error setting USB role mux: %d\n", ret);
+ mux_control_deselect(data->type_c_mode_mux);
+ return ret;
+ }
+
+out:
+ data->muxes_set = true;
+ return 0;
+}
+
+struct tcpc_mux_dev *devm_tcpc_gen_mux_create(struct device *dev)
+{
+ struct tcpc_gen_mux_data *data;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return ERR_PTR(-ENOMEM);
+
+ data->type_c_mode_mux = devm_mux_control_get(dev, "type-c-mode-mux");
+ if (IS_ERR(data->type_c_mode_mux)) {
+ ret = PTR_ERR(data->type_c_mode_mux);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Error getting Type-C mux: %d\n", ret);
+ return ERR_PTR(-ret);
+ }
+
+ data->usb_role_mux = devm_mux_control_get(dev, "usb-role-mux");
+ if (IS_ERR(data->usb_role_mux)) {
+ ret = PTR_ERR(data->usb_role_mux);
+ /* The USB role mux is optional */
+ if (ret == -ENODEV) {
+ data->usb_role_mux = NULL;
+ } else {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Error getting USB role mux: %d\n",
+ ret);
+ return ERR_PTR(-ret);
+ }
+ }
+
+ data->dev = dev;
+ data->mux.set = tcpc_gen_mux_set;
+
+ return &data->mux;
+}
+EXPORT_SYMBOL_GPL(devm_tcpc_gen_mux_create);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Generic Type-C mux driver using the mux subsys");
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index f662eed48c86..36789005a2b0 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -164,4 +164,6 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
void tcpm_pd_hard_reset(struct tcpm_port *port);
void tcpm_tcpc_reset(struct tcpm_port *port);
+struct tcpc_mux_dev *devm_tcpc_gen_mux_create(struct device *dev);
+
#endif /* __LINUX_USB_TCPM_H */
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 10/11] staging: typec: fusb302: Hook up mux support using tcpc_gen_mux support
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Mark Rutland, Kuppuswamy Sathyanarayanan, devicetree,
Greg Kroah-Hartman, linux-usb, linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede, Rob Herring
Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()
to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.
Also document the mux-names used by the generic tcpc_mux_dev code in
our devicetree bindings.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++
drivers/staging/typec/fusb302/fusb302.c | 11 ++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
index 472facfa5a71..63d639eadacd 100644
--- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
@@ -6,6 +6,9 @@ Required properties :
- interrupts : Interrupt specifier
Optional properties :
+- mux-controls : List of mux-ctrl-specifiers containing 1 or 2 muxes
+- mux-names : "type-c-mode-mux" when using 1 mux, or
+ "type-c-mode-mux", "usb-role-mux" when using 2 muxes
- fcs,max-sink-microvolt : Maximum voltage to negotiate when configured as sink
- fcs,max-sink-microamp : Maximum current to negotiate when configured as sink
- fcs,max-sink-microwatt : Maximum power to negotiate when configured as sink
diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index cf6355f59cd9..00d045d0246b 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1259,7 +1259,6 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
fusb302_tcpc_dev->set_roles = tcpm_set_roles;
fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
- fusb302_tcpc_dev->mux = NULL;
}
static const char * const cc_polarity_name[] = {
@@ -1817,6 +1816,16 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
+ chip->tcpc_dev.mux = devm_tcpc_gen_mux_create(dev);
+ if (IS_ERR(chip->tcpc_dev.mux)) {
+ ret = PTR_ERR(chip->tcpc_dev.mux);
+ /* Use of a mux is optional (for now?), ignore -ENODEV errors */
+ if (ret == -ENODEV)
+ chip->tcpc_dev.mux = NULL;
+ else
+ return ret;
+ }
+
cfg.drv_data = chip;
chip->psy = devm_power_supply_register(dev, &fusb302_psy_desc, &cfg);
if (IS_ERR(chip->psy)) {
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 10/11] staging: typec: fusb302: Hook up mux support using tcpc_gen_mux support
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb, Rob Herring,
Mark Rutland, devicetree
Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()
to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.
Also document the mux-names used by the generic tcpc_mux_dev code in
our devicetree bindings.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++
drivers/staging/typec/fusb302/fusb302.c | 11 ++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
index 472facfa5a71..63d639eadacd 100644
--- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
@@ -6,6 +6,9 @@ Required properties :
- interrupts : Interrupt specifier
Optional properties :
+- mux-controls : List of mux-ctrl-specifiers containing 1 or 2 muxes
+- mux-names : "type-c-mode-mux" when using 1 mux, or
+ "type-c-mode-mux", "usb-role-mux" when using 2 muxes
- fcs,max-sink-microvolt : Maximum voltage to negotiate when configured as sink
- fcs,max-sink-microamp : Maximum current to negotiate when configured as sink
- fcs,max-sink-microwatt : Maximum power to negotiate when configured as sink
diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index cf6355f59cd9..00d045d0246b 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1259,7 +1259,6 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
fusb302_tcpc_dev->set_roles = tcpm_set_roles;
fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
- fusb302_tcpc_dev->mux = NULL;
}
static const char * const cc_polarity_name[] = {
@@ -1817,6 +1816,16 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
+ chip->tcpc_dev.mux = devm_tcpc_gen_mux_create(dev);
+ if (IS_ERR(chip->tcpc_dev.mux)) {
+ ret = PTR_ERR(chip->tcpc_dev.mux);
+ /* Use of a mux is optional (for now?), ignore -ENODEV errors */
+ if (ret == -ENODEV)
+ chip->tcpc_dev.mux = NULL;
+ else
+ return ret;
+ }
+
cfg.drv_data = chip;
chip->psy = devm_power_supply_register(dev, &fusb302_psy_desc, &cfg);
if (IS_ERR(chip->psy)) {
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 11/11] platform/x86: intel_cht_int33fe: Add mux mappings for the Type-C port
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
@ 2017-09-01 21:48 ` Hans de Goede
2017-09-01 21:48 ` Hans de Goede
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: devel, Kuppuswamy Sathyanarayanan, Greg Kroah-Hartman, linux-usb,
linux-kernel, platform-driver-x86,
Sathyanarayanan Kuppuswamy Natarajan, Hans de Goede
We need to add mappings for the mux subsys to be able to find the
muxes for the fusb302 driver to be able to control the PI3USB30532
Type-C mux and the device/host mux integrated in the CHT SoC.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_cht_int33fe.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index c5554577681a..4256e05ee584 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -794,6 +794,7 @@ config ACPI_CMPC
config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
+ select MULTIPLEXER
---help---
This driver add support for the INT33FE ACPI device found on
some Intel Cherry Trail devices.
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 24a1662be81d..611b8af9cefd 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -24,6 +24,7 @@
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/mux/consumer.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -35,6 +36,19 @@ struct cht_int33fe_data {
struct i2c_client *pi3usb30532;
};
+static struct mux_lookup cht_int33fe_mux_lookup[] = {
+ {
+ .provider = "i2c-pi3usb30532",
+ .dev_id = "i2c-fusb302",
+ .mux_name = "type-c-mode-mux",
+ },
+ {
+ .provider = "intel_cht_usb_mux",
+ .dev_id = "i2c-fusb302",
+ .mux_name = "usb-role-mux",
+ },
+};
+
/*
* Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates
* the max17047 both through the INT33FE ACPI device (it is right there
@@ -170,6 +184,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
board_info.properties = fusb302_props;
board_info.irq = fusb302_irq;
+ mux_add_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
if (!data->fusb302)
goto out_unregister_max17047;
@@ -193,6 +210,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
+ mux_remove_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
}
@@ -205,6 +225,9 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
+ mux_remove_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
return 0;
}
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 11/11] platform/x86: intel_cht_int33fe: Add mux mappings for the Type-C port
@ 2017-09-01 21:48 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-01 21:48 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman
Cc: Hans de Goede, platform-driver-x86, devel,
Kuppuswamy Sathyanarayanan, Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel, Greg Kroah-Hartman, linux-usb
We need to add mappings for the mux subsys to be able to find the
muxes for the fusb302 driver to be able to control the PI3USB30532
Type-C mux and the device/host mux integrated in the CHT SoC.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_cht_int33fe.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index c5554577681a..4256e05ee584 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -794,6 +794,7 @@ config ACPI_CMPC
config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
+ select MULTIPLEXER
---help---
This driver add support for the INT33FE ACPI device found on
some Intel Cherry Trail devices.
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 24a1662be81d..611b8af9cefd 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -24,6 +24,7 @@
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/mux/consumer.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -35,6 +36,19 @@ struct cht_int33fe_data {
struct i2c_client *pi3usb30532;
};
+static struct mux_lookup cht_int33fe_mux_lookup[] = {
+ {
+ .provider = "i2c-pi3usb30532",
+ .dev_id = "i2c-fusb302",
+ .mux_name = "type-c-mode-mux",
+ },
+ {
+ .provider = "intel_cht_usb_mux",
+ .dev_id = "i2c-fusb302",
+ .mux_name = "usb-role-mux",
+ },
+};
+
/*
* Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates
* the max17047 both through the INT33FE ACPI device (it is right there
@@ -170,6 +184,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
board_info.properties = fusb302_props;
board_info.irq = fusb302_irq;
+ mux_add_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
if (!data->fusb302)
goto out_unregister_max17047;
@@ -193,6 +210,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
+ mux_remove_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
}
@@ -205,6 +225,9 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
+ mux_remove_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
return 0;
}
--
2.13.5
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [PATCH 11/11] platform/x86: intel_cht_int33fe: Add mux mappings for the Type-C port
2017-09-01 21:48 ` Hans de Goede
(?)
@ 2017-09-02 10:42 ` Andy Shevchenko
[not found] ` <CAHp75VdFAY6HaOh5-yBsgBoeJgpDrVUizX+zESq1Pr0-sQR6Pg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
-1 siblings, 1 reply; 52+ messages in thread
From: Andy Shevchenko @ 2017-09-02 10:42 UTC (permalink / raw)
To: Hans de Goede
Cc: MyungJoo Ham, Chanwoo Choi, Guenter Roeck, Heikki Krogerus,
Darren Hart, Andy Shevchenko, Peter Rosin, Mathias Nyman,
Platform Driver, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, USB
On Sat, Sep 2, 2017 at 12:48 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> We need to add mappings for the mux subsys to be able to find the
> muxes for the fusb302 driver to be able to control the PI3USB30532
> Type-C mux and the device/host mux integrated in the CHT SoC.
>
I suppose it will go via not PDx86 tree, so
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/platform/x86/Kconfig | 1 +
> drivers/platform/x86/intel_cht_int33fe.c | 23 +++++++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index c5554577681a..4256e05ee584 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -794,6 +794,7 @@ config ACPI_CMPC
> config INTEL_CHT_INT33FE
> tristate "Intel Cherry Trail ACPI INT33FE Driver"
> depends on X86 && ACPI && I2C && REGULATOR
> + select MULTIPLEXER
> ---help---
> This driver add support for the INT33FE ACPI device found on
> some Intel Cherry Trail devices.
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
> index 24a1662be81d..611b8af9cefd 100644
> --- a/drivers/platform/x86/intel_cht_int33fe.c
> +++ b/drivers/platform/x86/intel_cht_int33fe.c
> @@ -24,6 +24,7 @@
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> +#include <linux/mux/consumer.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
>
> @@ -35,6 +36,19 @@ struct cht_int33fe_data {
> struct i2c_client *pi3usb30532;
> };
>
> +static struct mux_lookup cht_int33fe_mux_lookup[] = {
> + {
> + .provider = "i2c-pi3usb30532",
> + .dev_id = "i2c-fusb302",
> + .mux_name = "type-c-mode-mux",
> + },
> + {
> + .provider = "intel_cht_usb_mux",
> + .dev_id = "i2c-fusb302",
> + .mux_name = "usb-role-mux",
> + },
> +};
> +
> /*
> * Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates
> * the max17047 both through the INT33FE ACPI device (it is right there
> @@ -170,6 +184,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
> board_info.properties = fusb302_props;
> board_info.irq = fusb302_irq;
>
> + mux_add_table(cht_int33fe_mux_lookup,
> + ARRAY_SIZE(cht_int33fe_mux_lookup));
> +
> data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
> if (!data->fusb302)
> goto out_unregister_max17047;
> @@ -193,6 +210,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
> if (data->max17047)
> i2c_unregister_device(data->max17047);
>
> + mux_remove_table(cht_int33fe_mux_lookup,
> + ARRAY_SIZE(cht_int33fe_mux_lookup));
> +
> return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
> }
>
> @@ -205,6 +225,9 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
> if (data->max17047)
> i2c_unregister_device(data->max17047);
>
> + mux_remove_table(cht_int33fe_mux_lookup,
> + ARRAY_SIZE(cht_int33fe_mux_lookup));
> +
> return 0;
> }
>
> --
> 2.13.5
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems
2017-09-01 21:48 [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede
` (10 preceding siblings ...)
2017-09-01 21:48 ` Hans de Goede
@ 2017-09-04 11:18 ` Peter Rosin
2017-09-05 10:54 ` Hans de Goede
11 siblings, 1 reply; 52+ messages in thread
From: Peter Rosin @ 2017-09-04 11:18 UTC (permalink / raw)
To: Hans de Goede, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb, Stephen Boyd
On 2017-09-01 23:48, Hans de Goede wrote:
> Hi All,
>
> This series consists of 4 parts:
>
> 1) Core mux changes to add support for getting mux-controllers on
> non DT platforms and to add some standardised state values for USB
>
> 2) Add Intel CHT USB mux and Pericom-PI3USB30532 Type-C mux drivers
>
> 3) Hookup the Intel CHT USB mux driver for CHT devices without Type-C
>
> 4) Hookup both the Intel CHT USB mux and Pericom-PI3USB30532 Type-C mux
> drivers to the fusb302 Type-C port controller found on some CHT x86 devs
>
> Please review, or in the case of the drivers/mux changes (which are a dep
> for some of the other patches) merge if the patches are ready.
Hi Hans,
I see commonalities with this and the below patch seriess from Stephen
Boyd [added to Cc].
https://lkml.org/lkml/2017/7/11/696 [PATCH 0/3] USB Mux support for Chipidea
https://lkml.org/lkml/2017/7/14/654 [PATCH v2 0/3] USB Mux support for Chipidea
Stephen had a patch that added mux_control_get_optional() that I think
could be of use for this series?
Anyway, there seems to be some interest in using the mux subsystem for
handling the USB host/device role. I think the role-switch interface
between the USB and mux subsystems should be done similarly, regardless
of what particular USB driver needs to switch role using whatever
particular mux driver. If at all possible.
The way I read it, the pi3usb30532 driver is the one adding the need to
involve the DP states and the inverted bit. Those things are not used by
anything else, and I think it clutters up things for everybody when the
weird needs of one driver "invades" the mux states needed to control the
USB role. So, I would like USB role switching to have 2 states, device
and host (0 and 1 is used by the above chipidea code). And then the USB
switch can of course be idle, which is best represented with one of
MUX_IDLE_DISCONNECT, MUX_IDLE_AS_IS or one of the modes depending on if
the USB switch can or can't disconnect (or other considerations). Now,
you can't explicitly set the idle state using mux_control_select(), it
can only be set implicitly using mux_control_deselect(), so that will
require some changes in the consumer logic.
Regarding the pi3usb30532 driver, I think the above is in fact also
better since the "swap bit" means something totally different when the
switch is "open" (if I read the datasheet correctly).
I.e. PI3USB30532_CONF_OPEN | PI3USB30532_CONF_SWAP is not sane, and that
would just go away completely if the driver implemented MUX_IDLE_DISCONNECT
as PI3USB30532_CONF_OPEN and removed the possibility to explicitly set the
"open" state with mux_control_select().
So, I think the generic USB role switch interface should be something like:
#define MUX_USB_DEVICE (0) /* USB device mode */
#define MUX_USB_HOST (1) /* USB host mode */
And then the pi3usb30532 driver can extend that:
#define MUX_PI3USB30352_HOST_AND_DP_SRC (2) /* USB host + 2 lanes Display Port */
#define MUX_PI3USB30352_DP_SRC (3) /* 4 lanes Display Port source */
#define MUX_PI3USB30352_POLARITY_INV BIT(2) /* Polarity inverted bit */
#define MUX_PI3USB30352_STATES (8)
Another idea is to expose the inverted bit as a separate mux controller,
but I suspect that you're not too keen on operating three muxes in the
tcpm driver... That will get simpler with mux_control_get_optional() though.
BTW, I don't think these USB defines belong in mux/consumer.h, please add
them in a new include/linux/mux/usb.h file or something. And I'd like some
MAINTAINER entry to cover the new mux drivers...
I'll respond to the individual patches with nits etc, but it generally looks
tidy, thank you for that!
Cheers,
Peter
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems
2017-09-04 11:18 ` [PATCH 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Peter Rosin
@ 2017-09-05 10:54 ` Hans de Goede
0 siblings, 0 replies; 52+ messages in thread
From: Hans de Goede @ 2017-09-05 10:54 UTC (permalink / raw)
To: Peter Rosin, MyungJoo Ham, Chanwoo Choi, Guenter Roeck,
Heikki Krogerus, Darren Hart, Andy Shevchenko, Mathias Nyman
Cc: platform-driver-x86, devel, Kuppuswamy Sathyanarayanan,
Sathyanarayanan Kuppuswamy Natarajan, linux-kernel,
Greg Kroah-Hartman, linux-usb, Stephen Boyd
Hi Peter,
On 04-09-17 13:18, Peter Rosin wrote:
> On 2017-09-01 23:48, Hans de Goede wrote:
>> Hi All,
>>
>> This series consists of 4 parts:
>>
>> 1) Core mux changes to add support for getting mux-controllers on
>> non DT platforms and to add some standardised state values for USB
>>
>> 2) Add Intel CHT USB mux and Pericom-PI3USB30532 Type-C mux drivers
>>
>> 3) Hookup the Intel CHT USB mux driver for CHT devices without Type-C
>>
>> 4) Hookup both the Intel CHT USB mux and Pericom-PI3USB30532 Type-C mux
>> drivers to the fusb302 Type-C port controller found on some CHT x86 devs
>>
>> Please review, or in the case of the drivers/mux changes (which are a dep
>> for some of the other patches) merge if the patches are ready.
>
> Hi Hans,
>
> I see commonalities with this and the below patch seriess from Stephen
> Boyd [added to Cc].
>
> https://lkml.org/lkml/2017/7/11/696 [PATCH 0/3] USB Mux support for Chipidea
> https://lkml.org/lkml/2017/7/14/654 [PATCH v2 0/3] USB Mux support for Chipidea
Interesting, it seems that Stephen and I are trying to use the mux-framework
for identical (device/host selection) purposes, except that in some cases
I also gave a Type-C cross-switch to deal with.
I noticed this discussion in the thread:
"""
> + usb-switch-states = <0>, <1>;
I don't see the need for usb-switch-states? Just assume states 0/1 and
if someone later needs some other states, make them add a property that
overrides the defaults. Just document that 0 is host and 1 is device.
"""
I think it makes sense to just agree on fixed "state" values for
certain use-cases, as done in my
"mux: consumer.h: Add MUX_USB_* state constant defines"
patch. This means that the "state" register in the hardware and the
state as passed to mux_control_select() may not map 1:1, but that can
easily be mapped in the driver and it allows inter-changeble (compatible)
mux drivers for some common mux use-cases such as an USB device/host
mux.
Edit: Ah I see you already suggest this below, good :)
> Stephen had a patch that added mux_control_get_optional() that I think
> could be of use for this series?
Ack, I think that would be useful. If you plan to merge Stephen's
patch for this, please give me a link to a git repo with that merged
then I will rebase my series on top.
> Anyway, there seems to be some interest in using the mux subsystem for
> handling the USB host/device role. I think the role-switch interface
> between the USB and mux subsystems should be done similarly, regardless
> of what particular USB driver needs to switch role using whatever
> particular mux driver. If at all possible. >
> The way I read it, the pi3usb30532 driver is the one adding the need to
> involve the DP states and the inverted bit.
Correct, the pi3usb30532 mux is a mux designed for Type-C ports, it
switches to 4 high-speed differential data-pairs the Type-C connector
has to one on: USB-controller(s), DisplayPort outputs on the GPU, or
a combination of both (2 data-pairs to each). It also takes into account
if the Type-C connector has been inserted normally or "upside-down",
which is where the inverted bits comes into play.
> Those things are not used by
> anything else, and I think it clutters up things for everybody when the
> weird needs of one driver "invades" the mux states needed to control the
> USB role. So, I would like USB role switching to have 2 states, device
> and host (0 and 1 is used by the above chipidea code). And then the USB
> switch can of course be idle, which is best represented with one of
> MUX_IDLE_DISCONNECT, MUX_IDLE_AS_IS or one of the modes depending on if
> the USB switch can or can't disconnect (or other considerations). Now,
> you can't explicitly set the idle state using mux_control_select(), it
> can only be set implicitly using mux_control_deselect(), so that will
> require some changes in the consumer logic.
>
> Regarding the pi3usb30532 driver, I think the above is in fact also
> better since the "swap bit" means something totally different when the
> switch is "open" (if I read the datasheet correctly >
> I.e. PI3USB30532_CONF_OPEN | PI3USB30532_CONF_SWAP is not sane,
The only "datasheet" I could find is "PI3USB30532-PI3USB31532_App_Type-C application Note_Rev.A.pdf"
and that says the swap bits does not do anything when the switch is "open"
but that does not matter for the rest of the discussion, I do agree
that mapping deselected to open makes sense.
> and that
> would just go away completely if the driver implemented MUX_IDLE_DISCONNECT
> as PI3USB30532_CONF_OPEN and removed the possibility to explicitly set the
> "open" state with mux_control_select().
>
> So, I think the generic USB role switch interface should be something like:
>
> #define MUX_USB_DEVICE (0) /* USB device mode */
> #define MUX_USB_HOST (1) /* USB host mode */
Those 2 work for me.
> And then the pi3usb30532 driver can extend that:
>
> #define MUX_PI3USB30352_HOST_AND_DP_SRC (2) /* USB host + 2 lanes Display Port */
> #define MUX_PI3USB30352_DP_SRC (3) /* 4 lanes Display Port source */
> #define MUX_PI3USB30352_POLARITY_INV BIT(2) /* Polarity inverted bit */
> #define MUX_PI3USB30352_STATES (8)
Erm, I would like to keep mux-driver specific knowledge out of the
tcpm code and there might be more Type-C mux drivers in the future,
how about:
#define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */
#define MUX_TYPEC_DEVICE (0 << 1) /* USB device mode */
#define MUX_TYPEC_HOST (1 << 1) /* USB host mode */
#define MUX_TYPEC_HOST_AND_DP_SRC (2 << 1) /* USB host + 2 lanes Display Port */
#define MUX_TYPEC_DP_SRC (3 << 1) /* 4 lanes Display Port source */
#define MUX_TYPEC_STATES (4 << 1)
?
> Another idea is to expose the inverted bit as a separate mux controller,
> but I suspect that you're not too keen on operating three muxes in the
> tcpm driver...
That won't be pretty I think, so thanks but no thanks :)
> BTW, I don't think these USB defines belong in mux/consumer.h, please add
> them in a new include/linux/mux/usb.h file or something. And I'd like some
> MAINTAINER entry to cover the new mux drivers...
Ok, I will fix both for the next version
> I'll respond to the individual patches with nits etc, but it generally looks
> tidy, thank you for that!
Thank you for all the feedback and reviews.
Regards,
Hans
^ permalink raw reply [flat|nested] 52+ messages in thread