From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932966AbcDFMkO (ORCPT ); Wed, 6 Apr 2016 08:40:14 -0400 Received: from mail-lf0-f41.google.com ([209.85.215.41]:34075 "EHLO mail-lf0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754840AbcDFMkL (ORCPT ); Wed, 6 Apr 2016 08:40:11 -0400 Subject: Re: [PATCH v3 3/7] usb: mux: add common code for Intel dual role port mux To: Lu Baolu , Greg Kroah-Hartman References: <1457423628-3183-1-git-send-email-baolu.lu@linux.intel.com> <1457423628-3183-4-git-send-email-baolu.lu@linux.intel.com> <20160311000643.GD2586@kroah.com> <56E60F42.8030701@linux.intel.com> <20160314032714.GA4665@kroah.com> <5704B067.1010001@linux.intel.com> Cc: Felipe Balbi , Mathias Nyman , Lee Jones , Heikki Krogerus , MyungJoo Ham , Chanwoo Choi , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org From: Sergei Shtylyov Message-ID: <570503A6.4070001@cogentembedded.com> Date: Wed, 6 Apr 2016 15:40:06 +0300 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <5704B067.1010001@linux.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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