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

[Hans de Goede]

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@xxxxxxxxxxxxxxx> wrote:
> > > > > On Tue, May 30, 2017 at 3:47 AM,
> > > > > <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> 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