From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751294AbdEaXhK (ORCPT ); Wed, 31 May 2017 19:37:10 -0400 Received: from mga04.intel.com ([192.55.52.120]:29057 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751015AbdEaXhJ (ORCPT ); Wed, 31 May 2017 19:37:09 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,276,1493708400"; d="scan'208";a="863320806" Reply-To: sathyanarayanan.kuppuswamy@linux.intel.com Subject: Re: [PATCH v1 1/1] mux: mux-intel-usb: Add Intel USB Multiplexer driver References: <84e3fdaf979c070a5c39c7635910fee30c3a8b06.1496104447.git.sathyanarayanan.kuppuswamy@linux.intel.com> <1919b140-e61e-7b7d-18f3-60f4fe3a248d@axentia.se> <6d63d001-5b41-9d18-0b57-119109b66a90@axentia.se> To: Peter Rosin , heikki.krogerus@linux.intel.com Cc: linux-kernel@vger.kernel.org, sathyaosid@gmail.com From: sathyanarayanan kuppuswamy Organization: Intel Message-ID: <6d466118-3448-7a3f-df06-49f9a43a1295@linux.intel.com> Date: Wed, 31 May 2017 16:33:46 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <6d63d001-5b41-9d18-0b57-119109b66a90@axentia.se> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 05/30/2017 11:29 PM, Peter Rosin wrote: > On 2017-05-30 19:47, sathyanarayanan kuppuswamy wrote: >> Hi Peter, >> >> Thanks for your comments. >> >> On 05/30/2017 06:40 AM, Peter Rosin wrote: >>> On 2017-05-30 02:47, sathyanarayanan.kuppuswamy@linux.intel.com wrote: >>>> From: Kuppuswamy Sathyanarayanan >>>> >>>> In some Intel SOCs, a single USB port is shared between USB device and >>>> host controller and an internal mux is used to control the selection of >>>> port by host/device controllers. This driver adds support for the USB >>>> internal mux, and all consumers of this mux can use interfaces provided >>>> by mux subsytem to control the state of the internal mux. >>>> >>>> Signed-off-by: Kuppuswamy Sathyanarayanan >>> Hi! >>> >>> A few things make me curious... >>> >>> 1. How are platform devices w/o any match table probed? Do you have to >>> manually load them, or what? >> Yes, for now, I am manually creating this device from dwc3 driver to >> test it. But in future I am planning to get ACPI ID for this device to >> probe it from BIOS. >>> 2. What are these "all the consumers of this mux" that you mention, >> Currently the only consumer for this mux device is, Broxton USB PHY >> driver. Its not yet upstreamed. I hoping to get this driver merged first >> before submitting the other driver for review. > Is any other consumer in the charts at all? Can this existing consumer > ever make use of some other mux? If the answer to both those questions > are 'no', then I do not see much point in involving the mux subsystem at > all. The Broxton USB PHY driver could just as well write to the register > all by itself, no? > >>> and how >>> do they find the correct mux control to interact with? >> Your current mux_control_get() API has tight dependency on device tree >> node and hence we can't use it for this use case. > Yes, that was all I needed, so far. Trying to keep things simple... > >> So I am planning to add a new API to get the mux-control based on >> mux-device name. API interface looks some thing like below. I haven't >> finalized the patch yet. I will send it you for review in next few days. >> Let me know if you agree with this idea. >> >> struct mux_chip *devm_mux_chip_get_by_index(struct device *dev, >> const char *parent_name, unsigned int index) >> >> struct mux_control *devm_mux_control_get_by_index(struct device *dev, >> struct mux_chip *mux_chip, unsigned int index) > I don't like exposing struct mux_chip to clients. > > My lose plan was to try to dig into the device name if the of_node is > not present, and thus overload the mux_control_get() api. I think this would make the implementation bit complex. Why not maintain uniformity ? Like searching for mux control based on device name or using unique chip name + index. This would work for both DT and non DT cases. > Not sure about > how to handle non-DT muxes with more than one mux-control though, maybe > add an index argument that is ignored when it's not needed? But you > don't really need the index either, so that can wait... > > Cheers, > peda > -- Sathyanarayanan Kuppuswamy Linux kernel developer