Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer driver

From: Hans de Goede <hdegoede@redhat.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Kuppuswamy Sathyanarayanan 
	<sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: Peter Rosin <peda@axentia.se>,
	"Krogerus, Heikki" <heikki.krogerus@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Sathyanarayanan Kuppuswamy Natarajan <sathyaosid@gmail.com>
Subject: Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer driver
Date: Wed, 31 May 2017 14:21:34 +0200	[thread overview]
Message-ID: <cd60e0fa-6140-423e-b713-f651de8e528d@redhat.com> (raw)
In-Reply-To: <CAHp75Ve=1X9X_QYDWcRmRC4gaCyHdVcwzKKxBFEEg3L8UUe7dQ@mail.gmail.com>

Hi,

On 30-05-17 20:50, Andy Shevchenko wrote:
> On Tue, May 30, 2017 at 9:21 PM, sathyanarayanan kuppuswamy
> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>>> On Tue, May 30, 2017 at 3:47 AM,
>>> <sathyanarayanan.kuppuswamy@linux.intel.com> 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,
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.

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