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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <372ee565-7add-c81e-46cd-5696e03bfe6e@redhat.com>
Date:   Thu, 1 Jun 2017 16:54:35 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     sathyanarayanan.kuppuswamy@...ux.intel.com,
        Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Peter Rosin <peda@...ntia.se>,
        "Krogerus, Heikki" <heikki.krogerus@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Sathyanarayanan Kuppuswamy Natarajan <sathyaosid@...il.com>
Subject: Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer
 driver

HI,

On 01-06-17 01:12, sathyanarayanan kuppuswamy wrote:
> 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
>>> <sathyanarayanan.kuppuswamy@...ux.intel.com> wrote:
>>>>> On Tue, May 30, 2017 at 3:47 AM,
>>>>> <sathyanarayanan.kuppuswamy@...ux.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,
> 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 ?

No, the ID bit causes the mux to get thrown either way, the
VBUS bit causes the device controller to see a connect or
not.

> 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 ?

They MUST be controlled separately because on some systems
the SW_IDPIN bit is controller by ACPI AML code, where as
the VBUS_VALID needs to be set by the kernel.

We could model the VBUS_VALID bit as a separate mux_controller,
that would be a bit weird though ...

Regards,

Hans