From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wolverine02.qualcomm.com ([199.106.114.251]:21383 "EHLO wolverine02.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752100Ab0KRHya (ORCPT ); Thu, 18 Nov 2010 02:54:30 -0500 Date: Thu, 18 Nov 2010 13:24:25 +0530 From: Pavan Kondeti Subject: Re: [PATCH 1/4] USB: Add MSM OTG Controller driver Message-ID: <20101118075425.GB11700@codeaurora.org> References: <1289301591-26228-1-git-send-email-pkondeti@codeaurora.org> <1289301591-26228-2-git-send-email-pkondeti@codeaurora.org> <20101116214346.GA18387@kroah.com> <20101117102352.GA29845@codeaurora.org> <20101117161637.GE10992@kroah.com> <20101118050016.GA11700@codeaurora.org> <20101118052016.GA13001@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101118052016.GA13001@kroah.com> Sender: linux-arm-msm-owner@vger.kernel.org List-ID: To: Greg KH Cc: linux-usb@vger.kernel.org, linux-arm-msm@vger.kernel.org On Wed, Nov 17, 2010 at 09:20:16PM -0800, Greg KH wrote: > On Thu, Nov 18, 2010 at 10:30:16AM +0530, Pavan Kondeti wrote: > > Hi Greg, > > > > On Wed, Nov 17, 2010 at 08:16:37AM -0800, Greg KH wrote: > > > On Wed, Nov 17, 2010 at 03:53:52PM +0530, Pavan Kondeti wrote: > > > > Hi Greg, > > > > > > > > On Tue, Nov 16, 2010 at 01:43:46PM -0800, Greg KH wrote: > > > > > On Tue, Nov 09, 2010 at 04:49:48PM +0530, Pavankumar Kondeti wrote: > > > > > > This driver implements PHY initialization, clock management, memory mapping > > > > > > register address space, ULPI IO ops and simple OTG state machine to kick > > > > > > host/peripheral based on Id/VBUS line status. VBUS/Id lines are tied to a > > > > > > reference voltage on some boards. Hence provide a sysfs interface to > > > > > > select host/peripheral mode. > > > > > > > > > > As you are creating a new user/kernel abi, it MUST be documented in the > > > > > Documentation/ABI/ directory. I can't take this patch set until that > > > > > happens. > > > > > > > > > Thanks for letting me know this. I will add the documentation for the sysfs file. > > > > > > Also note that if you are adding a new ABI like this one, it needs to > > > work the same for the other existing OTG drivers as well. So please > > > also work to fix them to do the same thing, or change your code to work > > > like the existing drivers do (hint, do the latter one...) > > > > > I am not sure if other OTG driver require this sysfs file. > > That's the point, why does this driver require something that no other > driver does? > > > USB mode i.e Host/Peripheral is changed based on Id/VBUS status. But > > on some of the MSM boards, Id/VBUS is connected to reference voltage > > and need an additional sysfs file for user to change the operation > > mode. > > Are you sure this should be user selectable? Who is going to do that > selection and how is it going to happen "automatically" like OTG is > supposed to handle? > The board I am using is a reference board where Id is grounded always. But we use it in peripheral mode for ADB (Android debugging Bridge). So user is going to write into the sysfs file ("host"/"peripheral"/"none"). But this is mainly for debugging/testing purpose. > > Where should a driver specific sysfs file should go in Doc/ABI/ ? > > Where all others are, there are lots of examples in that directory, > including a README, right? Did you read that and it not explain things > sufficiently? If so, please let me know what can be expanded on in that > file to make it easier for others in the future. > I have read the README file and it only talks about different levels of stability. I have not looked at examples. After looking at examples I understand that testing/sysfs-driver-usb-msm_otg is the correct file. > > My main complaint here is that you are creating a brand new > kernel/userspace ABI, and you bury it in a driver patch without giving > really any warning or description of it at all. This is something that > we need to make sure we get correct as you (yes you) will be maintaining > it for the next 12+ years. This is not something to do lightly at all, > as I'm sure you can imagine. > Okay. I understand now. Thanks for the explanation. I should not have used a sysfs file, which creates a new kernel/userspace ABI. I will use debugfs as it is mainly intended for debugging purpose. The only reason I have gone for sysfs is that to have this option available without CONFIG_DEBUGFS. > -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.