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

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 6 Apr 2016 15:40:06 +0300
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Lu Baolu <baolu.lu@...ux.intel.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Felipe Balbi <balbi@...nel.org>,
	Mathias Nyman <mathias.nyman@...el.com>,
	Lee Jones <lee.jones@...aro.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	MyungJoo Ham <myungjoo.ham@...sung.com>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/7] usb: mux: add common code for Intel dual role port
 mux

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