From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751216AbdEaXQX (ORCPT ); Wed, 31 May 2017 19:16:23 -0400 Received: from mga05.intel.com ([192.55.52.43]:26796 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751106AbdEaXQW (ORCPT ); Wed, 31 May 2017 19:16:22 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,276,1493708400"; d="scan'208";a="107714202" Reply-To: sathyanarayanan.kuppuswamy@linux.intel.com Subject: Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer driver References: <84e3fdaf979c070a5c39c7635910fee30c3a8b06.1496104447.git.sathyanarayanan.kuppuswamy@linux.intel.com> <0e2a3df8-e3b4-28d3-eea9-5ec6a684bf53@linux.intel.com> To: Hans de Goede , Andy Shevchenko Cc: Peter Rosin , "Krogerus, Heikki" , "linux-kernel@vger.kernel.org" , Sathyanarayanan Kuppuswamy Natarajan From: sathyanarayanan kuppuswamy Organization: Intel Message-ID: <402e68ce-0fcd-bfe5-0db5-b02526937495@linux.intel.com> Date: Wed, 31 May 2017 16:12:58 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Hans, On 05/31/2017 05:21 AM, Hans de Goede wrote: > Hi, > > On 30-05-17 20:50, Andy Shevchenko wrote: >> On Tue, May 30, 2017 at 9:21 PM, sathyanarayanan kuppuswamy >> wrote: >>>> On Tue, May 30, 2017 at 3:47 AM, >>>> wrote: >> >>>>> + tristate "Intel USB Mux" >>>> >>>> It's indeed Intel's IP? >>> >>> Register map to control this MUX comes from Intel vendor defined XHCI >>> extended cap region of SOC. >>>> >>>> I would rather believe that it is some 3rd >>>> party known IP block with platform specific soldering. >>> >>> I don't think its platform specific support. I believe its a SOC >>> specific >>> thing( mainly for CHT and APL SoCs). >> >> Okay, the best people to give a feedback here are Heikki and Hans. > > Interesting, I have been working on this myself too (and posted > a version of my driver for the mux block a while back), see: > > https://www.spinics.net/lists/linux-usb/msg151032.html > https://www.spinics.net/lists/linux-usb/msg151033.html > https://www.spinics.net/lists/linux-usb/msg151034.html > > I was actually planning on posting a new version of this set > soon. I still need to to modify the first patch to trigger > bu PCI-ids to avoid registering non existing mux platform > device on non CHT / APL Intel platforms. > > I've since changed the second patch into a platform driver, > see: > > https://github.com/jwrdegoede/linux-sunxi/commit/e51057609102c35dd35b4a5965139aff81fbefb8 > > > Note that this patch has 2 differences compared to > sathyanarayanan's patch: > > 1) It ties directly into the extcon framework and responds > to extcon events instead of using the mux framework, What about devices with no ID events ? At least the APL device that I am using does not have external IRQ for VBUS and ID. > actually this is the first time I hear about a mux framework > at all. Is there a git tree with the patches for this somewhere ? > > 2) It controls the ID and VBUS bits separately, this is > important because on some Cherry Trail devices the ID bit > is automatically set be ACPI code (through a gpio irq > event handler), but the ACPI code does not update the > VBUS bit causing the otg port on these devices to not > work in device mode. Even if you listen to ID and VBUS events separately, I think the end result is selecting either SW host or SW device mode right ? Then why not abstract MUX functionality outside the your driver and control the MUX from your driver appropriately ? or do we get any advantage is modifying just VBUS_VALID or SW_IDPIN bits separately ? > > 2. also means that this no longer really is a mux driver ... > I've run a whole lot of tests with these patches on > various Cherry Trail devices using either ACPI code > to set the ID pin or the already merged > drivers/extcon/extcon-intel-int3496.c > driver for the tablets where the ACPI code defines > a INT3496 ACPI device instead of handling the id > pin / bit itself. > > And this driver works well in both scenarios. Recently > I've become aware of one potential problem, which is > integrating this with devices which have a USB-C > connector. I've one CHT device with a USB-C connector > and a FUSB302 USB controller, in that case we need > the mux driver to react to data / changes from the > FUSB302 driver. One way to do this would be for the > FUSB302 driver to also register an extcon driver. > > But there still is a lot of work TBD wrt getting USB-C > to work on these devices (where it is not all handled > in firmware like it is on regular x86 laptops). So it > might be best to just move forward with my version of > this driver for now, which at least makes the micro-usb > connector found on many of these devices work properly > in both host and device mode all the time. > > Regards, > > Hans > > -- Sathyanarayanan Kuppuswamy Linux kernel developer