From: Hans de Goede <hdegoede@redhat.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Darren Hart <dvhart@infradead.org>,
Andy Shevchenko <andy@infradead.org>,
MyungJoo Ham <myungjoo.ham@samsung.com>,
Chanwoo Choi <cw00.choi@samsung.com>,
Mathias Nyman <mathias.nyman@intel.com>,
Heikki Krogerus <heikki.krogerus@linux.intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Platform Driver <platform-driver-x86@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
USB <linux-usb@vger.kernel.org>
Subject: Re: [PATCH 12/12] extcon: axp288: Set USB role where necessary
Date: Sun, 25 Feb 2018 14:49:59 +0100 [thread overview]
Message-ID: <5bb7a9d5-c4d2-f48b-f17e-c64cc3b2bb6d@redhat.com> (raw)
In-Reply-To: <CAHp75VcNpnx1vknr43sgDPEi8hu9yYRKv2JzOtg+ruuWoik3pg@mail.gmail.com>
Hi,
On 16-02-18 14:19, Andy Shevchenko wrote:
> On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdegoede@redhat.com> 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@redhat.com>
>
>> 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
next prev parent reply other threads:[~2018-02-25 13:49 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-16 10:47 [PATCH 00/12] USB Type-C device-connection, mux and switch support Hans de Goede
2018-02-16 10:47 ` [PATCH 01/12] drivers: base: Unified device connection lookup Hans de Goede
2018-02-16 11:00 ` Andy Shevchenko
2018-02-16 11:21 ` Hans de Goede
2018-02-16 11:28 ` Heikki Krogerus
2018-02-16 10:47 ` [PATCH 02/12] usb: typec: Start using ERR_PTR Hans de Goede
2018-02-16 11:07 ` Andy Shevchenko
2018-02-16 11:21 ` Hans de Goede
2018-02-16 10:47 ` [PATCH 03/12] usb: typec: API for controlling USB Type-C Multiplexers Hans de Goede
2018-02-16 14:00 ` Andy Shevchenko
2018-02-25 12:13 ` Hans de Goede
2018-02-16 10:47 ` [PATCH 04/12] usb: common: Small class for USB role switches Hans de Goede
2018-02-16 14:07 ` Andy Shevchenko
2018-02-16 14:22 ` Heikki Krogerus
2018-02-19 9:32 ` Hans de Goede
2018-02-25 12:16 ` Hans de Goede
2018-02-16 18:33 ` Randy Dunlap
2018-02-25 12:16 ` Hans de Goede
2018-02-16 10:47 ` [PATCH 05/12] usb: typec: tcpm: Set USB role switch to device mode when configured as such Hans de Goede
2018-02-16 13:19 ` Heikki Krogerus
2018-02-16 10:47 ` [PATCH 06/12] usb: typec: tcpm: Use new Type-C switch/mux and usb-role-switch functions Hans de Goede
2018-02-16 13:20 ` Heikki Krogerus
2018-02-16 10:47 ` [PATCH 07/12] xhci: Add option to get next extended capability in list by passing id = 0 Hans de Goede
2018-02-16 13:21 ` Heikki Krogerus
2018-02-16 10:47 ` [PATCH 08/12] xhci: Add Intel extended cap / otg phy mux handling Hans de Goede
2018-02-16 13:32 ` Heikki Krogerus
2018-02-16 13:53 ` Andy Shevchenko
2018-02-25 13:01 ` Hans de Goede
2018-02-16 10:47 ` [PATCH 09/12] usb: roles: Add Intel XHCI USB role switch driver Hans de Goede
2018-02-16 13:38 ` Heikki Krogerus
2018-02-25 12:11 ` Hans de Goede
2018-02-16 13:47 ` Andy Shevchenko
2018-02-25 13:29 ` Hans de Goede
2018-02-16 10:47 ` [PATCH 10/12] usb: typec: driver for Pericom PI3USB30532 Type-C cross switch Hans de Goede
2018-02-16 13:33 ` Andy Shevchenko
2018-02-16 13:43 ` Heikki Krogerus
2018-02-16 10:47 ` [PATCH 11/12] platform/x86: intel_cht_int33fe: Add device connections for the Type-C port Hans de Goede
2018-02-16 13:10 ` Andy Shevchenko
2018-02-16 13:43 ` Heikki Krogerus
2018-02-16 10:47 ` [PATCH 12/12] extcon: axp288: Set USB role where necessary Hans de Goede
2018-02-16 13:19 ` Andy Shevchenko
2018-02-25 13:49 ` Hans de Goede [this message]
2018-02-16 13:45 ` Heikki Krogerus
2018-02-16 13:36 ` [PATCH 00/12] USB Type-C device-connection, mux and switch support Andy Shevchenko
-- strict thread matches above, loose matches on Subject: below --
2018-02-16 10:43 [PATCH 0/12] " Hans de Goede
2018-02-16 10:43 ` [PATCH 12/12] extcon: axp288: Set USB role where necessary Hans de Goede
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5bb7a9d5-c4d2-f48b-f17e-c64cc3b2bb6d@redhat.com \
--to=hdegoede@redhat.com \
--cc=andy.shevchenko@gmail.com \
--cc=andy@infradead.org \
--cc=cw00.choi@samsung.com \
--cc=dvhart@infradead.org \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=myungjoo.ham@samsung.com \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).