Re: [PATCH v5 12/12] extcon: axp288: Set USB role where necessary
Hi,
On 02-03-18 01:39, Chanwoo Choi wrote:
Hi,
Basically, I have no objection. But I'll reply the my ack tag
after finishing the review of 'devcon and usb_role_switch' from USB maintainer.
And I have a question.
Before this patch, extcon-axp288 is used to detect charger connector
and extcon-intel-int3496 is used to detect the USB_HOST connector
on one h/w device?
Yes the ACPI tables of some devices with an AXP288 PMIC have an INT3496
ACPI device which gives access to the id-pin on the micro-AB connector
which the extcon-intel-int3496 exports as a USB_HOST connector, on these
devices we fully control the USB role.
On other devices the switching of the USB data lines is controlled by AML
code which switches the data lines, but never sets the VBus valid bit
in the USB PHY control registers, so it is effectively switching between
between the host and none roles, since the device role only works when
the VBus valid bit is set.
This patch addresses both types of devices / ACPI tables and makes
host *and* device mode work. This patch also makes switching between them
by plugging in a different cable after boot work.
Regards,
Hans
Best Regards,
Chanwoo Choi
Samsung Electronics
On 2018년 03월 01일 00:07, Hans de Goede wrote:
The AXP288 BC1.2 charger detection / extcon code may seem like a strange
place to add code to control the USB role-switch on devices with an AXP288,
but there are 2 reasons to do this inside the axp288 extcon code:
1) On many devices the USB role is controlled by ACPI AML code, but the AML
code only switches between the host and none roles, because of Windows
not really using device mode. To make device mode work we need to toggle
between the none/device roles based on Vbus presence, and the axp288
extcon gets interrupts on Vbus insertion / removal.
2) In order for our BC1.2 charger detection to work properly the role
mux must be properly set to device mode before we do the detection.
Also note the Kconfig help-text / obsolete depends on USB_PHY which are
remnants from older never upstreamed code also controlling the mux from
the axp288 extcon code.
This commit also adds code to get notifications from the INT3496 extcon
device, which is used on some devices to notify the kernel about id-pin
changes instead of them being handled through AML code.
This fixes:
-Device mode not working on most CHT devices with an AXP288
-Host mode not working on devices with an INT3496 ACPI device
-Charger-type misdetection (always SDP) on devices with an INT3496 when the
USB role (always) gets initialized as host
Reviewed-by: Heikki Krogerus
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
Changes in v4:
-Add Andy's Reviewed-by
Changes in v2:
-Add depends on X86 to Kconfig (the AXP288 PMIC is only used on X86)
-Use new acpi_dev_get_first_match_name() helper to get the INT3496 device-name
-Add Heikki's Reviewed-by
---
drivers/extcon/Kconfig | 3 +-
drivers/extcon/extcon-axp288.c | 177 +++--
2 files changed, 171 insertions(+), 9 deletions(-)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index a7bca4207f44..de15bf55895b 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -30,7 +30,8 @@ config EXTCON_ARIZONA
config EXTCON_AXP288
tristate "X-Power AXP288 EXTCON support"
- depends on MFD_AXP20X && USB_PHY
+ depends on MFD_AXP20X && USB_SUPPORT && X86
+ select USB_ROLE_SWITCH
help
Say Y here to enable support for USB peripheral detection
and USB MUX switching by X-Power AXP288 PMIC.
diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index 3ec4c715e240..51e77c7a32c2 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -1,6 +1,7 @@
/*
* extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
*
+ * Copyright (c) 2017-2018 Hans de Goede
* Copyright (C) 2015 Intel Corporation
* Author: Ramakrishna Pallala
*
@@ -14,6 +15,8 @@
* GNU General Public License for more details.
*/
+#include
+#include
#include
#include
#include
@@ -25,6 +28,11 @@
#include
#include
#include
+#include
+#include
+
+#include
+#include
/* Power source status register */
#define PS_STAT_VBUS_TRIGGER BIT(0)
@@ -97,9 +105,19 @@ struct axp288_extcon_info {
struct device *dev;
struct regmap *regmap;
struct regmap_irq_chip_data *regmap_irqc;
+ struct usb_role_switch *role_sw;
+ struct work_struct role_work;
int irq[EXTCON_IRQ_END];
struct extcon_dev *edev;
+ struct extcon_dev *id_extcon;
+ struct notifier_block id_nb;
unsigned int previous_cable;
+ bool vbus_attach;
+};
+
+static const struct x86_cpu_id cherry_trail_cpu_ids[] = {
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT, X86_FEATURE_ANY },
+ {}
};
/* Power up/down reason s
Re: [PATCH v5 12/12] extcon: axp288: Set USB role where necessary
Hi,
Basically, I have no objection. But I'll reply the my ack tag
after finishing the review of 'devcon and usb_role_switch' from USB maintainer.
And I have a question.
Before this patch, extcon-axp288 is used to detect charger connector
and extcon-intel-int3496 is used to detect the USB_HOST connector
on one h/w device?
Best Regards,
Chanwoo Choi
Samsung Electronics
On 2018년 03월 01일 00:07, Hans de Goede wrote:
> The AXP288 BC1.2 charger detection / extcon code may seem like a strange
> place to add code to control the USB role-switch on devices with an AXP288,
> but there are 2 reasons to do this inside the axp288 extcon code:
>
> 1) On many devices the USB role is controlled by ACPI AML code, but the AML
>code only switches between the host and none roles, because of Windows
>not really using device mode. To make device mode work we need to toggle
>between the none/device roles based on Vbus presence, and the axp288
>extcon gets interrupts on Vbus insertion / removal.
>
> 2) In order for our BC1.2 charger detection to work properly the role
>mux must be properly set to device mode before we do the detection.
>
> Also note the Kconfig help-text / obsolete depends on USB_PHY which are
> remnants from older never upstreamed code also controlling the mux from
> the axp288 extcon code.
>
> This commit also adds code to get notifications from the INT3496 extcon
> device, which is used on some devices to notify the kernel about id-pin
> changes instead of them being handled through AML code.
>
> This fixes:
> -Device mode not working on most CHT devices with an AXP288
> -Host mode not working on devices with an INT3496 ACPI device
> -Charger-type misdetection (always SDP) on devices with an INT3496 when the
> USB role (always) gets initialized as host
>
> Reviewed-by: Heikki Krogerus
> Reviewed-by: Andy Shevchenko
> Signed-off-by: Hans de Goede
> ---
> Changes in v4:
> -Add Andy's Reviewed-by
>
> Changes in v2:
> -Add depends on X86 to Kconfig (the AXP288 PMIC is only used on X86)
> -Use new acpi_dev_get_first_match_name() helper to get the INT3496 device-name
> -Add Heikki's Reviewed-by
> ---
> drivers/extcon/Kconfig | 3 +-
> drivers/extcon/extcon-axp288.c | 177
> +++--
> 2 files changed, 171 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index a7bca4207f44..de15bf55895b 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -30,7 +30,8 @@ config EXTCON_ARIZONA
>
> config EXTCON_AXP288
> tristate "X-Power AXP288 EXTCON support"
> - depends on MFD_AXP20X && USB_PHY
> + depends on MFD_AXP20X && USB_SUPPORT && X86
> + select USB_ROLE_SWITCH
> help
> Say Y here to enable support for USB peripheral detection
> and USB MUX switching by X-Power AXP288 PMIC.
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index 3ec4c715e240..51e77c7a32c2 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -1,6 +1,7 @@
> /*
> * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
> *
> + * Copyright (c) 2017-2018 Hans de Goede
> * Copyright (C) 2015 Intel Corporation
> * Author: Ramakrishna Pallala
> *
> @@ -14,6 +15,8 @@
> * GNU General Public License for more details.
> */
>
> +#include
> +#include
> #include
> #include
> #include
> @@ -25,6 +28,11 @@
> #include
> #include
> #include
> +#include
> +#include
> +
> +#include
> +#include
>
> /* Power source status register */
> #define PS_STAT_VBUS_TRIGGER BIT(0)
> @@ -97,9 +105,19 @@ struct axp288_extcon_info {
> struct device *dev;
> struct regmap *regmap;
> struct regmap_irq_chip_data *regmap_irqc;
> + struct usb_role_switch *role_sw;
> + struct work_struct role_work;
> int irq[EXTCON_IRQ_END];
> struct extcon_dev *edev;
> + struct extcon_dev *id_extcon;
> + struct notifier_block id_nb;
> unsigned int previous_cable;
> + bool vbus_attach;
> +};
> +
> +static const struct x86_cpu_id cherry_trail_cpu_ids[] = {
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT, X86_FEATURE_ANY },
> + {}
> };
>
> /* Power up/down reason string array */
> @@ -137,20 +155,74 @@ static void axp288_extcon_log_rsi(struct
> axp288_extcon_info *info)
> regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
> }
>
> -static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
> +/*
> + * The below code to control the USB role-switch on devices with an AXP288
> + * may seem out of place, but there are 2 reasons why this is the best place
> + * to control the USB role-switch on such devices:
> + * 1) On many devices the USB role is controlled by AML code, but the AML
> code
> + *only switches between the host and none roles, because of Windows not
> + *really using device mode
