Messages in this thread | | | Date | Mon, 15 Jan 2018 18:08:12 +0900 | From | Chanwoo Choi <> | Subject | Re: [PATCH 2/2] extcon: axp288: Only reschedule charger-detection at boot when a SDP is detected |
| |
On 2018년 01월 15일 17:36, Hans de Goede wrote: > Hi, > > On 15-01-18 06:22, Chanwoo Choi wrote: >> On 2018년 01월 15일 00:10, Hans de Goede wrote: >>> The only misdetection which can happen at boot due to data-lines mux issues >>> is detecting a non SDP as SDP, so we only need to retry if we detect a SDP >>> on our first detection. >>> >>> Note Vbus misdetection is not a problem, as soon as the drivers controlling >>> the Vbus path set it correctly we will get an interrupt which reschedules >>> the charger-detection. >>> >>> Also update the comment about the re-detection to reflect this. >>> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- >>> drivers/extcon/extcon-axp288.c | 14 +++++++------- >>> 1 file changed, 7 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c >>> index 63b99d5becd7..17e6808af0d1 100644 >>> --- a/drivers/extcon/extcon-axp288.c >>> +++ b/drivers/extcon/extcon-axp288.c >>> @@ -161,16 +161,16 @@ static void axp288_chrg_detect_complete(struct axp288_extcon_info *info, >>> /* >>> * We depend on other drivers to do things like mux the data lines, >>> * enable/disable vbus based on the id-pin, etc. Sometimes the BIOS has >>> - * not set these things up correctly resulting in the initial charger >>> - * cable type detection giving a wrong result and we end up not charging >>> - * or charging at only 0.5A. >>> + * not set these things up correctly resulting in a wrong result for the >>> + * initial charger type detection and we end up charging at only 0.5A. >>> * >>> - * So we schedule a second cable type detection after 2 seconds to >>> - * give the other drivers time to load and do their thing. >>> + * If our first detect detects an SDP charger-type, we try again after >>> + * 2 seconds to give the other drivers time to load and do their thing. >>> */ >>> if (!info->first_detect_done) { >>> - queue_delayed_work(system_wq, &info->det_work, >>> - msecs_to_jiffies(2000)); >>> + if (info->previous_cable == EXTCON_CHG_USB_SDP) >>> + queue_delayed_work(system_wq, &info->det_work, >>> + msecs_to_jiffies(2000)); >>> info->first_detect_done = true; >>> } >>> >> >> I understand why you add the second delayed_work because of dependency >> of other consumer driver. But, this patch is not proper method. It looks >> like the workaround. >> >> We need to consider the fundamental solution such as using OF graph >> or sending the pending notification when consumer driver is probed. > > I agree that having some sort of proper probe ordering here would be > better. But on these ACPI systems that is going to be quite tricky todo, > since we've no control over the firmware there. > > Note that you've already merged the workaround, this patch merely changes > the workaround to avoid it in cases where it is not necessary, so I would > really like to see this get merged.
I merged your patch because I knew this issue related to dependency.
But, I don't want to merge this patch until developing the fundamental method. All extcon provider driver have the same issue. I'll try to resolve this issue thro extcon framework.
-- Best Regards, Chanwoo Choi Samsung Electronics
| |