Re: [PATCH 12/12] extcon: axp288: Set USB role where necessary

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 16-02-18 14:19, Andy Shevchenko wrote:
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdegoede@xxxxxxxxxx> 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

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>

  config EXTCON_AXP288
         tristate "X-Power AXP288 EXTCON support"
-       depends on MFD_AXP20X && USB_PHY
+       depends on MFD_AXP20X && USB_SUPPORT
+       select USB_ROLE_SWITCH

Is it supposed to work outside of x86 world?..

No.

+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>

...if yes, this should go under CONFIG_X86 along with accompanying parts.

...if no, put corresponding dependency to Kconfig.

Ack, added the dependency for v2 of the patch-set.

+       if (info->role_sw) {
+               ret = devm_add_action_or_reset(dev, axp288_put_role_sw, info);
+               if (ret)
+                       return ret;
+
+               if (acpi_dev_present("INT3496", NULL, -1)) {
+                       info->id_extcon = extcon_get_extcon_dev("INT3496:00");

Please use instance found by acpi_dev_present(). Okay, actually new
helper is here:
acpi_dev_get_first_match_name().

Good call, I've switched to acpi_dev_get_first_match_name() for v2.



+                       if (!info->id_extcon)
+                               return -EPROBE_DEFER;
+
+                       dev_info(dev, "controlling USB role\n");
+               } else {
+                       dev_info(dev, "controlling USB role based on vbus presence\n");
+               }
+       }
+


Andy, Thank you for all the reviews!

Regards,

Hans



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux