Re: [PATCH v3 3/7] usb: mux: add common code for Intel dual role port mux

[Sergei Shtylyov]

Hello.

On 4/6/2016 9:44 AM, Lu Baolu wrote:

> > > > > +struct intel_mux_dev {
> > > > > +	struct device	*dev;
> > > > > +	char		*extcon_name;
> > > > > +	char		*cable_name;
> > > > > +	int		(*cable_set_cb)(struct intel_mux_dev *mux);
> > > > > +	int		(*cable_unset_cb)(struct intel_mux_dev *mux);
> > > > > +};
> > > > This is a device, why not make it one?  Don't just hold a reference.
> > > > And do you really even hold that reference?
> > > It's not a device. It's just an encapsulation for parameters passed into
> > > intel_usb_mux_register().
> > But you called it a device, so you can understand my confusion.
> >
> > And why not make it a device?  Why isn't this one?  Hint, I really think
> > it should be...
> >
> > > > And your api is horrid, think about what you want the "core" to do here,
> > > > it should be the one creating the device and returning it to the caller,
> > > > not forcing the caller to somehow create it first and then pass it in.
> > > This isn't a layer or core. It doesn't create any new devices. It's actually
> > > some shared code which can be used by all Intel dual role port drivers.
> > It should be a device, as you are treating it like one :)
> >
> > > I put it in a separated file because 1) this can avoid duplication; 2) this code
> > > could be used for any architectures as long as a USB port is shared by
> > > two components and it needs an OS response when event triggers.
> > It's a bit hard for other arches to be using something called "intel_"
> > :(
>
> Are there any other implementations which need an external mux
> to swap the usb roles?

   There are, of course, like Renesas R-Car gen2 SoCs. I've implemented the 
USB port multiplexing as a part of the PHY driver (see 
drivers/phy/phy-rcar-gen2.c).

MBR, Sergei