[PATCH v2 0/3] [PATCH 0/4] usb: typec: fixes for Cherry Trails

[PATCH v2 0/3] [PATCH 0/4] usb: typec: fixes for Cherry Trails

[Heikki Krogerus]

Hi,

This is second version of the remaining patches that fix various
problems I encountered while testing my USB Type-C Alternate Mode
patches with GPD Win board (Intel Cherry Trail based). In this version
I've addressed the problems pointed out by Hans and Guenter.

Link to the original version:
https://lkml.org/lkml/2018/4/30/350


Heikki Krogerus (3):
  usb: roles: intel_xhci: Always allow user control
  platform: x86: intel_cht_int33fe: Fix dependencies
  usb: typec: fusb302: Fix debugfs issue

 drivers/i2c/busses/Kconfig                    |  3 +--
 drivers/platform/x86/Kconfig                  |  4 ++--
 .../usb/roles/intel-xhci-usb-role-switch.c    | 21 +------------------
 drivers/usb/typec/fusb302/fusb302.c           |  1 +
 4 files changed, 5 insertions(+), 24 deletions(-)

-- 
2.17.0

[PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control

[Heikki Krogerus]

Trying to determine the USB port type with this mux is very
difficult. To simplify the situation, always allowing user
control, even if the port is USB Type-C port.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 .../usb/roles/intel-xhci-usb-role-switch.c    | 21 +------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 30b07ea3a3c6..3f14153d753f 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -39,20 +39,6 @@ struct intel_xhci_usb_data {
 	void __iomem *base;
 };
 
-struct intel_xhci_acpi_match {
-	const char *hid;
-	int hrv;
-};
-
-/*
- * ACPI IDs for PMICs which do not support separate data and power role
- * detection (USB ACA detection for micro USB OTG), we allow userspace to
- * change the role manually on these.
- */
-static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
-	{ "INT33F4",  3 }, /* X-Powers AXP288 PMIC */
-};
-
 static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
 {
 	struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
@@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev)
 static struct usb_role_switch_desc sw_desc = {
 	.set = intel_xhci_usb_set_role,
 	.get = intel_xhci_usb_get_role,
+	.allow_userspace_control = true,
 };
 
 static int intel_xhci_usb_probe(struct platform_device *pdev)
@@ -146,7 +133,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct intel_xhci_usb_data *data;
 	struct resource *res;
-	int i;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -159,11 +145,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
 	if (!data->base)
 		return -ENOMEM;
 
-	for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
-		if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
-				     allow_userspace_ctrl_ids[i].hrv))
-			sw_desc.allow_userspace_control = true;
-
 	platform_set_drvdata(pdev, data);
 
 	data->role_sw = usb_role_switch_register(dev, &sw_desc);
-- 
2.17.0

[v2,1/3] usb: roles: intel_xhci: Always allow user control

[Heikki Krogerus]

Trying to determine the USB port type with this mux is very
difficult. To simplify the situation, always allowing user
control, even if the port is USB Type-C port.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 .../usb/roles/intel-xhci-usb-role-switch.c    | 21 +------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 30b07ea3a3c6..3f14153d753f 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -39,20 +39,6 @@ struct intel_xhci_usb_data {
 	void __iomem *base;
 };
 
-struct intel_xhci_acpi_match {
-	const char *hid;
-	int hrv;
-};
-
-/*
- * ACPI IDs for PMICs which do not support separate data and power role
- * detection (USB ACA detection for micro USB OTG), we allow userspace to
- * change the role manually on these.
- */
-static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
-	{ "INT33F4",  3 }, /* X-Powers AXP288 PMIC */
-};
-
 static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
 {
 	struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
@@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev)
 static struct usb_role_switch_desc sw_desc = {
 	.set = intel_xhci_usb_set_role,
 	.get = intel_xhci_usb_get_role,
+	.allow_userspace_control = true,
 };
 
 static int intel_xhci_usb_probe(struct platform_device *pdev)
@@ -146,7 +133,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct intel_xhci_usb_data *data;
 	struct resource *res;
-	int i;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -159,11 +145,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
 	if (!data->base)
 		return -ENOMEM;
 
-	for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
-		if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
-				     allow_userspace_ctrl_ids[i].hrv))
-			sw_desc.allow_userspace_control = true;
-
 	platform_set_drvdata(pdev, data);
 
 	data->role_sw = usb_role_switch_register(dev, &sw_desc);

[PATCH v2 2/3] platform: x86: intel_cht_int33fe: Fix dependencies

[Heikki Krogerus]

The driver will not probe unless bq24190 is loaded, so
making it a dependency.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/Kconfig   | 3 +--
 drivers/platform/x86/Kconfig | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 99edffae27f6..4f8df2ec87b1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -202,8 +202,7 @@ config I2C_CHT_WC
 
 	  Note this controller is hooked up to a TI bq24292i charger-IC,
 	  combined with a FUSB302 Type-C port-controller as such it is advised
-	  to also select CONFIG_CHARGER_BQ24190=m and CONFIG_TYPEC_FUSB302=m
-	  (the fusb302 driver currently is in drivers/staging).
+	  to also select CONFIG_TYPEC_FUSB302=m.
 
 config I2C_NFORCE2
 	tristate "Nvidia nForce2, nForce3 and nForce4"
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 566644bb496a..f27cb186437d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -866,6 +866,7 @@ config ACPI_CMPC
 config INTEL_CHT_INT33FE
 	tristate "Intel Cherry Trail ACPI INT33FE Driver"
 	depends on X86 && ACPI && I2C && REGULATOR
+	depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
 	---help---
 	  This driver add support for the INT33FE ACPI device found on
 	  some Intel Cherry Trail devices.
@@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE
 	  i2c drivers for these chips can bind to the them.
 
 	  If you enable this driver it is advised to also select
-	  CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and
-	  CONFIG_BATTERY_MAX17042=m.
+	  CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m.
 
 config INTEL_INT0002_VGPIO
 	tristate "Intel ACPI INT0002 Virtual GPIO driver"
-- 
2.17.0

[v2,2/3] platform: x86: intel_cht_int33fe: Fix dependencies

[Heikki Krogerus]

The driver will not probe unless bq24190 is loaded, so
making it a dependency.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/Kconfig   | 3 +--
 drivers/platform/x86/Kconfig | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 99edffae27f6..4f8df2ec87b1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -202,8 +202,7 @@ config I2C_CHT_WC
 
 	  Note this controller is hooked up to a TI bq24292i charger-IC,
 	  combined with a FUSB302 Type-C port-controller as such it is advised
-	  to also select CONFIG_CHARGER_BQ24190=m and CONFIG_TYPEC_FUSB302=m
-	  (the fusb302 driver currently is in drivers/staging).
+	  to also select CONFIG_TYPEC_FUSB302=m.
 
 config I2C_NFORCE2
 	tristate "Nvidia nForce2, nForce3 and nForce4"
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 566644bb496a..f27cb186437d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -866,6 +866,7 @@ config ACPI_CMPC
 config INTEL_CHT_INT33FE
 	tristate "Intel Cherry Trail ACPI INT33FE Driver"
 	depends on X86 && ACPI && I2C && REGULATOR
+	depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
 	---help---
 	  This driver add support for the INT33FE ACPI device found on
 	  some Intel Cherry Trail devices.
@@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE
 	  i2c drivers for these chips can bind to the them.
 
 	  If you enable this driver it is advised to also select
-	  CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and
-	  CONFIG_BATTERY_MAX17042=m.
+	  CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m.
 
 config INTEL_INT0002_VGPIO
 	tristate "Intel ACPI INT0002 Virtual GPIO driver"

[PATCH v2 3/3] usb: typec: fusb302: Fix debugfs issue

[Heikki Krogerus]

Removing the "fusb302" debugfs directory when unloading
the driver. That allows the driver to be loaded more then
ones. The directory will not get actually removed until it
is empty, so only after the last instance has been removed.

This fixes an issue where the driver can't be re-loaded if
it has been unloaded as the "fusb302" debugfs directory
already exists.

Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/fusb302/fusb302.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c
index eba6bb890b17..9c1eba9ea004 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -234,6 +234,7 @@ static int fusb302_debugfs_init(struct fusb302_chip *chip)
 static void fusb302_debugfs_exit(struct fusb302_chip *chip)
 {
 	debugfs_remove(chip->dentry);
+	debugfs_remove(rootdir);
 }
 
 #else
-- 
2.17.0

[v2,3/3] usb: typec: fusb302: Fix debugfs issue

[Heikki Krogerus]

Removing the "fusb302" debugfs directory when unloading
the driver. That allows the driver to be loaded more then
ones. The directory will not get actually removed until it
is empty, so only after the last instance has been removed.

This fixes an issue where the driver can't be re-loaded if
it has been unloaded as the "fusb302" debugfs directory
already exists.

Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/fusb302/fusb302.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c
index eba6bb890b17..9c1eba9ea004 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -234,6 +234,7 @@ static int fusb302_debugfs_init(struct fusb302_chip *chip)
 static void fusb302_debugfs_exit(struct fusb302_chip *chip)
 {
 	debugfs_remove(chip->dentry);
+	debugfs_remove(rootdir);
 }
 
 #else

Re: [PATCH v2 3/3] usb: typec: fusb302: Fix debugfs issue

[Sergei Shtylyov]

Hello!

On 05/23/2018 05:37 PM, Heikki Krogerus wrote:

> Removing the "fusb302" debugfs directory when unloading
> the driver. That allows the driver to be loaded more then
> ones. The directory will not get actually removed until it

   s/ones/once/?

> is empty, so only after the last instance has been removed.
> 
> This fixes an issue where the driver can't be re-loaded if
> it has been unloaded as the "fusb302" debugfs directory
> already exists.
> 
> Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
[...]

MBR, Sergei

[v2,3/3] usb: typec: fusb302: Fix debugfs issue

[Sergei Shtylyov]

Hello!

On 05/23/2018 05:37 PM, Heikki Krogerus wrote:

> Removing the "fusb302" debugfs directory when unloading
> the driver. That allows the driver to be loaded more then
> ones. The directory will not get actually removed until it

   s/ones/once/?

> is empty, so only after the last instance has been removed.
> 
> This fixes an issue where the driver can't be re-loaded if
> it has been unloaded as the "fusb302" debugfs directory
> already exists.
> 
> Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
[...]

MBR, Sergei
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control

[Sergei Shtylyov]

On 05/23/2018 05:37 PM, Heikki Krogerus wrote:

> Trying to determine the USB port type with this mux is very
> difficult. To simplify the situation, always allowing user

   s/allowing/allow/? Else the statement doesn't parse for me. :-)

> control, even if the port is USB Type-C port.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
[...]

MBR, Sergei

[v2,1/3] usb: roles: intel_xhci: Always allow user control

[Sergei Shtylyov]

On 05/23/2018 05:37 PM, Heikki Krogerus wrote:

> Trying to determine the USB port type with this mux is very
> difficult. To simplify the situation, always allowing user

   s/allowing/allow/? Else the statement doesn't parse for me. :-)

> control, even if the port is USB Type-C port.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
[...]

MBR, Sergei
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control

[Hans de Goede]

Hi,

On 23-05-18 16:37, Heikki Krogerus wrote:
> Trying to determine the USB port type with this mux is very
> difficult. To simplify the situation, always allowing user
> control, even if the port is USB Type-C port.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>   .../usb/roles/intel-xhci-usb-role-switch.c    | 21 +------------------
>   1 file changed, 1 insertion(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index 30b07ea3a3c6..3f14153d753f 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -39,20 +39,6 @@ struct intel_xhci_usb_data {
>   	void __iomem *base;
>   };
>   
> -struct intel_xhci_acpi_match {
> -	const char *hid;
> -	int hrv;
> -};
> -
> -/*
> - * ACPI IDs for PMICs which do not support separate data and power role
> - * detection (USB ACA detection for micro USB OTG), we allow userspace to
> - * change the role manually on these.
> - */
> -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
> -	{ "INT33F4",  3 }, /* X-Powers AXP288 PMIC */
> -};
> -
>   static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
>   {
>   	struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> @@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev)
>   static struct usb_role_switch_desc sw_desc = {
>   	.set = intel_xhci_usb_set_role,
>   	.get = intel_xhci_usb_get_role,
> +	.allow_userspace_control = true,
>   };
>   
>   static int intel_xhci_usb_probe(struct platform_device *pdev)


I hate to be pedantic here, but the sw_desc can and should be made
const now, with that fixed the entire series is:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> @@ -146,7 +133,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
>   	struct device *dev = &pdev->dev;
>   	struct intel_xhci_usb_data *data;
>   	struct resource *res;
> -	int i;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
> @@ -159,11 +145,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
>   	if (!data->base)
>   		return -ENOMEM;
>   
> -	for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
> -		if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
> -				     allow_userspace_ctrl_ids[i].hrv))
> -			sw_desc.allow_userspace_control = true;
> -
>   	platform_set_drvdata(pdev, data);
>   
>   	data->role_sw = usb_role_switch_register(dev, &sw_desc);
> 

[v2,1/3] usb: roles: intel_xhci: Always allow user control

[Hans de Goede]

Hi,

On 23-05-18 16:37, Heikki Krogerus wrote:
> Trying to determine the USB port type with this mux is very
> difficult. To simplify the situation, always allowing user
> control, even if the port is USB Type-C port.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>   .../usb/roles/intel-xhci-usb-role-switch.c    | 21 +------------------
>   1 file changed, 1 insertion(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index 30b07ea3a3c6..3f14153d753f 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -39,20 +39,6 @@ struct intel_xhci_usb_data {
>   	void __iomem *base;
>   };
>   
> -struct intel_xhci_acpi_match {
> -	const char *hid;
> -	int hrv;
> -};
> -
> -/*
> - * ACPI IDs for PMICs which do not support separate data and power role
> - * detection (USB ACA detection for micro USB OTG), we allow userspace to
> - * change the role manually on these.
> - */
> -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
> -	{ "INT33F4",  3 }, /* X-Powers AXP288 PMIC */
> -};
> -
>   static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
>   {
>   	struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> @@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev)
>   static struct usb_role_switch_desc sw_desc = {
>   	.set = intel_xhci_usb_set_role,
>   	.get = intel_xhci_usb_get_role,
> +	.allow_userspace_control = true,
>   };
>   
>   static int intel_xhci_usb_probe(struct platform_device *pdev)


I hate to be pedantic here, but the sw_desc can and should be made
const now, with that fixed the entire series is:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> @@ -146,7 +133,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
>   	struct device *dev = &pdev->dev;
>   	struct intel_xhci_usb_data *data;
>   	struct resource *res;
> -	int i;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
> @@ -159,11 +145,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
>   	if (!data->base)
>   		return -ENOMEM;
>   
> -	for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
> -		if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
> -				     allow_userspace_ctrl_ids[i].hrv))
> -			sw_desc.allow_userspace_control = true;
> -
>   	platform_set_drvdata(pdev, data);
>   
>   	data->role_sw = usb_role_switch_register(dev, &sw_desc);
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control

[Heikki Krogerus]

On Wed, May 23, 2018 at 08:03:43PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 23-05-18 16:37, Heikki Krogerus wrote:
> > Trying to determine the USB port type with this mux is very
> > difficult. To simplify the situation, always allowing user
> > control, even if the port is USB Type-C port.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >   .../usb/roles/intel-xhci-usb-role-switch.c    | 21 +------------------
> >   1 file changed, 1 insertion(+), 20 deletions(-)
> > 
> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > index 30b07ea3a3c6..3f14153d753f 100644
> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > @@ -39,20 +39,6 @@ struct intel_xhci_usb_data {
> >   	void __iomem *base;
> >   };
> > -struct intel_xhci_acpi_match {
> > -	const char *hid;
> > -	int hrv;
> > -};
> > -
> > -/*
> > - * ACPI IDs for PMICs which do not support separate data and power role
> > - * detection (USB ACA detection for micro USB OTG), we allow userspace to
> > - * change the role manually on these.
> > - */
> > -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
> > -	{ "INT33F4",  3 }, /* X-Powers AXP288 PMIC */
> > -};
> > -
> >   static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> >   {
> >   	struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> > @@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev)
> >   static struct usb_role_switch_desc sw_desc = {
> >   	.set = intel_xhci_usb_set_role,
> >   	.get = intel_xhci_usb_get_role,
> > +	.allow_userspace_control = true,
> >   };
> >   static int intel_xhci_usb_probe(struct platform_device *pdev)
> 
> 
> I hate to be pedantic here, but the sw_desc can and should be made
> const now, with that fixed the entire series is:

Sure, I'll fix that.

> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Thanks Hans. I'll send the v3. I need to add one more fix to the
series.

I got a report where somebody hit the issue related to the runtime PM
we talked about a while back: If xHCI is suspended, we fail to program
the mux registers as they are part of xHCI MMIO, so I will add a patch
to this series where I enable runtime PM here to fix that problem.


Cheers,

-- 
heikki

[v2,1/3] usb: roles: intel_xhci: Always allow user control

[Heikki Krogerus]

On Wed, May 23, 2018 at 08:03:43PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 23-05-18 16:37, Heikki Krogerus wrote:
> > Trying to determine the USB port type with this mux is very
> > difficult. To simplify the situation, always allowing user
> > control, even if the port is USB Type-C port.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >   .../usb/roles/intel-xhci-usb-role-switch.c    | 21 +------------------
> >   1 file changed, 1 insertion(+), 20 deletions(-)
> > 
> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > index 30b07ea3a3c6..3f14153d753f 100644
> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > @@ -39,20 +39,6 @@ struct intel_xhci_usb_data {
> >   	void __iomem *base;
> >   };
> > -struct intel_xhci_acpi_match {
> > -	const char *hid;
> > -	int hrv;
> > -};
> > -
> > -/*
> > - * ACPI IDs for PMICs which do not support separate data and power role
> > - * detection (USB ACA detection for micro USB OTG), we allow userspace to
> > - * change the role manually on these.
> > - */
> > -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
> > -	{ "INT33F4",  3 }, /* X-Powers AXP288 PMIC */
> > -};
> > -
> >   static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> >   {
> >   	struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> > @@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev)
> >   static struct usb_role_switch_desc sw_desc = {
> >   	.set = intel_xhci_usb_set_role,
> >   	.get = intel_xhci_usb_get_role,
> > +	.allow_userspace_control = true,
> >   };
> >   static int intel_xhci_usb_probe(struct platform_device *pdev)
> 
> 
> I hate to be pedantic here, but the sw_desc can and should be made
> const now, with that fixed the entire series is:

Sure, I'll fix that.

> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Thanks Hans. I'll send the v3. I need to add one more fix to the
series.

I got a report where somebody hit the issue related to the runtime PM
we talked about a while back: If xHCI is suspended, we fail to program
the mux registers as they are part of xHCI MMIO, so I will add a patch
to this series where I enable runtime PM here to fix that problem.


Cheers,

Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control

[Heikki Krogerus]

On Wed, May 23, 2018 at 07:10:58PM +0300, Sergei Shtylyov wrote:
> On 05/23/2018 05:37 PM, Heikki Krogerus wrote:
> 
> > Trying to determine the USB port type with this mux is very
> > difficult. To simplify the situation, always allowing user
> 
>    s/allowing/allow/? Else the statement doesn't parse for me. :-)

Thanks, I'll fix that, and the other patch too.


Cheers,

-- 
heikki

[v2,1/3] usb: roles: intel_xhci: Always allow user control

[Heikki Krogerus]

On Wed, May 23, 2018 at 07:10:58PM +0300, Sergei Shtylyov wrote:
> On 05/23/2018 05:37 PM, Heikki Krogerus wrote:
> 
> > Trying to determine the USB port type with this mux is very
> > difficult. To simplify the situation, always allowing user
> 
>    s/allowing/allow/? Else the statement doesn't parse for me. :-)

Thanks, I'll fix that, and the other patch too.


Cheers,

Re: [PATCH v2 2/3] platform: x86: intel_cht_int33fe: Fix dependencies

[Andy Shevchenko]

On Wed, 2018-05-23 at 17:37 +0300, Heikki Krogerus wrote:
> The driver will not probe unless bq24190 is loaded, so
> making it a dependency.
> 

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

assuming it will go through some other tree.

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Darren Hart <dvhart@infradead.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/Kconfig   | 3 +--
>  drivers/platform/x86/Kconfig | 4 ++--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 99edffae27f6..4f8df2ec87b1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -202,8 +202,7 @@ config I2C_CHT_WC
>  
>  	  Note this controller is hooked up to a TI bq24292i charger-
> IC,
>  	  combined with a FUSB302 Type-C port-controller as such it
> is advised
> -	  to also select CONFIG_CHARGER_BQ24190=m and
> CONFIG_TYPEC_FUSB302=m
> -	  (the fusb302 driver currently is in drivers/staging).
> +	  to also select CONFIG_TYPEC_FUSB302=m.
>  
>  config I2C_NFORCE2
>  	tristate "Nvidia nForce2, nForce3 and nForce4"
> diff --git a/drivers/platform/x86/Kconfig
> b/drivers/platform/x86/Kconfig
> index 566644bb496a..f27cb186437d 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -866,6 +866,7 @@ config ACPI_CMPC
>  config INTEL_CHT_INT33FE
>  	tristate "Intel Cherry Trail ACPI INT33FE Driver"
>  	depends on X86 && ACPI && I2C && REGULATOR
> +	depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
>  	---help---
>  	  This driver add support for the INT33FE ACPI device found
> on
>  	  some Intel Cherry Trail devices.
> @@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE
>  	  i2c drivers for these chips can bind to the them.
>  
>  	  If you enable this driver it is advised to also select
> -	  CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and
> -	  CONFIG_BATTERY_MAX17042=m.
> +	  CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m.
>  
>  config INTEL_INT0002_VGPIO
>  	tristate "Intel ACPI INT0002 Virtual GPIO driver"

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[v2,2/3] platform: x86: intel_cht_int33fe: Fix dependencies

[Andy Shevchenko]

On Wed, 2018-05-23 at 17:37 +0300, Heikki Krogerus wrote:
> The driver will not probe unless bq24190 is loaded, so
> making it a dependency.
> 

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

assuming it will go through some other tree.

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Darren Hart <dvhart@infradead.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/Kconfig   | 3 +--
>  drivers/platform/x86/Kconfig | 4 ++--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 99edffae27f6..4f8df2ec87b1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -202,8 +202,7 @@ config I2C_CHT_WC
>  
>  	  Note this controller is hooked up to a TI bq24292i charger-
> IC,
>  	  combined with a FUSB302 Type-C port-controller as such it
> is advised
> -	  to also select CONFIG_CHARGER_BQ24190=m and
> CONFIG_TYPEC_FUSB302=m
> -	  (the fusb302 driver currently is in drivers/staging).
> +	  to also select CONFIG_TYPEC_FUSB302=m.
>  
>  config I2C_NFORCE2
>  	tristate "Nvidia nForce2, nForce3 and nForce4"
> diff --git a/drivers/platform/x86/Kconfig
> b/drivers/platform/x86/Kconfig
> index 566644bb496a..f27cb186437d 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -866,6 +866,7 @@ config ACPI_CMPC
>  config INTEL_CHT_INT33FE
>  	tristate "Intel Cherry Trail ACPI INT33FE Driver"
>  	depends on X86 && ACPI && I2C && REGULATOR
> +	depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
>  	---help---
>  	  This driver add support for the INT33FE ACPI device found
> on
>  	  some Intel Cherry Trail devices.
> @@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE
>  	  i2c drivers for these chips can bind to the them.
>  
>  	  If you enable this driver it is advised to also select
> -	  CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and
> -	  CONFIG_BATTERY_MAX17042=m.
> +	  CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m.
>  
>  config INTEL_INT0002_VGPIO
>  	tristate "Intel ACPI INT0002 Virtual GPIO driver"