From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Cyrus-Session-Id: sloti22d1t05-4000417-1517255893-2-5519689965017158347 X-Sieve: CMU Sieve 3.0 X-Spam-known-sender: no X-Spam-score: 0.0 X-Spam-hits: BAYES_00 -1.9, HEADER_FROM_DIFFERENT_DOMAINS 0.001, RCVD_IN_DNSWL_HI -5, T_RP_MATCHES_RCVD -0.01, LANGUAGES en, BAYES_USED global, SA_VERSION 3.4.0 X-Spam-source: IP='209.132.180.67', Host='vger.kernel.org', Country='US', FromHeader='net', MailFrom='org' X-Spam-charsets: to='utf-8', cc='utf-8', plain='utf-8' X-Resolved-to: greg@kroah.com X-Delivered-to: greg@kroah.com X-Mail-from: linux-usb-owner@vger.kernel.org ARC-Seal: i=1; a=rsa-sha256; cv=none; d=messagingengine.com; s=arctest; t=1517255893; b=Lr1hPF6Pe07reM7bsidT33/QZEa+9vjbAq+XXWmysHLbiLG wcDsIzzor4a9h4FIVUPrEAGX0YAP6GegKeIOSLeAWujDKJcRncOXUMH554R7ByUA qnkLOB6qpbkScRZFD0bU02gtv7H55u42S0Z+ww/1+LZ6CHkJOaBOHq2PqTVf1+yX R7Gh/YZdCpJXUBAxbIM/DE8nx3Wkv8rY9XUlvVObDhEiTzMqfwzT0Hy8vGJ4RuYS rhADqnRWXITX5Q1FdfHkG6H44Nfv8oHHlKzjgFkgWc/+55jfBaMLwskhlRrBZyD/ eVj71mMyT90diZU7E3243YHeahmtrPhL3/PqtTA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=date:from:to:cc:subject:message-id :references:mime-version:content-type:content-transfer-encoding :in-reply-to:sender:list-id; s=arctest; t=1517255893; bh=xfxXqg3 ize82h54wniPdJH6KucknGeirbNFDNW8GSjA=; b=pXzHt4iWrvcFUguG/TPYU9d XA/a+4wNrljE6ofu/Vg1eNioJh2Xp0Qa5fODs9NaTFcbQf0GhUCDijYiUD3KbRJT SHgFiITZxCLzhhWNhCSzOhgOhgMk9Mn7O9ixPntq4j6lUi3sT/mxXML6+D/jqV3v K47q6DvfYCa7aBVnPa3hFcwkbeyKMpMmDSGAL4vosW0gxpG5NZPSMK+iQT56PxAT Z5RNFqsb1r4XKcfrQfVhFKvh102iECbPnLFeAlDTyIkwHl8TcF2R3utA6GfjyhCt 6/QvakbCGj/reHznXou/Oi1Bd0m4UUydPduRSQWFYYla9frkZoY0Zyvz/73VGPg= = ARC-Authentication-Results: i=1; mx4.messagingengine.com; arc=none (no signatures found); dkim=fail (message has been altered; 2048-bit rsa key sha256) header.d=gmail.com header.i=@gmail.com header.b=bhi1UC7l x-bits=2048 x-keytype=rsa x-algorithm=sha256 x-selector=20161025; dmarc=none (p=none,has-list-id=yes,d=none) header.from=roeck-us.net; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=linux-usb-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-google-dkim=fail (message has been altered; 2048-bit rsa key) header.d=1e100.net header.i=@1e100.net header.b=FdSp3Apb; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=roeck-us.net header.result=pass header_is_org_domain=yes Authentication-Results: mx4.messagingengine.com; arc=none (no signatures found); dkim=fail (message has been altered; 2048-bit rsa key sha256) header.d=gmail.com header.i=@gmail.com header.b=bhi1UC7l x-bits=2048 x-keytype=rsa x-algorithm=sha256 x-selector=20161025; dmarc=none (p=none,has-list-id=yes,d=none) header.from=roeck-us.net; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=linux-usb-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-google-dkim=fail (message has been altered; 2048-bit rsa key) header.d=1e100.net header.i=@1e100.net header.b=FdSp3Apb; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=roeck-us.net header.result=pass header_is_org_domain=yes Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751966AbeA2T55 (ORCPT ); Mon, 29 Jan 2018 14:57:57 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:34842 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751604AbeA2T5z (ORCPT ); Mon, 29 Jan 2018 14:57:55 -0500 X-Google-Smtp-Source: AH8x224KYyPrAg6uyL65hqYhKZ4Wgjmx8OD8wKPzqqCV+JhhT2xO0reb+/D9t8qg9xvmL1uriUttOg== Date: Mon, 29 Jan 2018 11:57:52 -0800 From: Guenter Roeck To: =?utf-8?B?c2h1ZmFuX2xlZSjmnY7mm7jluIYp?= Cc: Heikki Krogerus , 'Jun Li' , ShuFanLee , =?utf-8?B?Y3lfaHVhbmco6buD5ZWf5Y6fKQ==?= , "linux-kernel@vger.kernel.org" , "linux-usb@vger.kernel.org" Subject: Re: [PATCH] USB TYPEC: RT1711H Type-C Chip Driver Message-ID: <20180129195752.GA24352@roeck-us.net> References: <1515567552-7692-1-git-send-email-leechu729@gmail.com> <20180119082218.GA22976@kuha.fi.intel.com> <25ced79e8ea84908bf6110a613ed81a2@ex1.rt.l> <20180119092413.GB22976@kuha.fi.intel.com> <20180119160235.GA21066@roeck-us.net> <20180122185034.GA26058@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-usb-owner@vger.kernel.org X-Mailing-List: linux-usb@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Mon, Jan 29, 2018 at 07:19:06AM +0000, shufan_lee(李書帆) wrote: > Hi Guenter, > > We try to use the TCPCI driver on RT1711H and here are some questions. > > Q1. Is current TCPCI driver written according to TypeC Port Controller Interface Specification Revision 1.0 & Version 1.2? Revision 1.0. Note that I did not find revision 1.2, only Revision 1.0 and Revision 2.0 version 1.0. > Q2. Because 0x80~0xFF are vendor defined registers. Some of them are needed to be initialized in tcpci_init for RT1711H (or other chips also). > In the future TCPCI driver, will an initial interface that is called in tcpci_init be released for different vendors to implement. My suggestion would be to provide an API for vendor specific code. > Or, we should directly copy tcpci.c to tcpci_rt1711h.c and add the different parts? That would defeat the purpose. It would be better to implement vendor specific code in tcpci_rt1711h.c and call it from tcpci.c Possible example: struct tcpci_vendor_data { int (*init)(...); int (*irq_handler)(...); ... } static irqreturn_t tcpci_irq(...) { struct tcpci *tcpci = dev_id; ... if (tcpci->vendor_data->irq_handler) { ret = (*tcpci->vendor_data->irq_handler)(...); ... } ... } tcpci_init() { struct tcpci_vendor_data *vendor_data = &tcpci_rt1711h_data; // eg from devicetree compatible property ... if (vendor_data->init) { ret = (*vendor_data->init)(...); if (ret) return ret; } ... } > Q3. If there are IRQs defined in vendor defined registers, will an interface that is called in tcpci_irq be released for different vendors to implement. > So that they can handle their own IRQs first? If there are vendor specific interrupts, I would assume that vendor specific code will have to be called. Either the generic interrupt handler can call vendor specific code, or the vendor specific code handles the interrupt and calls the generic handler. I don't know at this point which one is better. > If the suggestion of Q2 is to copy tcpci.c to tcpci_rt1711h.c, then Q3 will not be a problem. > Q4. According to TCPCI Specification Revision 1.0, we should set DRP = 1 and role to Rp/Rp or Rd/Rd and set LOOK4CONNECTION command to start toggle. > So we modify the tcpci_start_drp_toggling in TCPCI driver as following. Here we write Rd/Rd and DRP = 0 simultaneously so that Rd/Rd takes effect. > Then we write DRP = 1 and set LOOK4CONNECTION command to start toggling. > SGTM. > static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc, > enum typec_cc_status cc) > { > +int ret = 0; > struct tcpci *tcpci = tcpc_to_tcpci(tcpc); > -unsigned int reg = TCPC_ROLE_CTRL_DRP; > +u32 reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) | > +(TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT); > > switch (cc) { > default: > @@ -125,8 +672,19 @@ static int tcpci_start_drp_toggling(stru > TCPC_ROLE_CTRL_RP_VAL_SHIFT); > break; > } > - > -return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > +ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > +if (ret < 0) > +return ret; > +mdelay(1); That is bad; you don't want to hold up teh system for that much. Try to use usleep_range(). > +reg |= TCPC_ROLE_CTRL_DRP; > +ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > +if (ret < 0) > +return ret; > +ret = regmap_write(tcpci->regmap, TCPC_COMMAND, > +TCPC_CMD_LOOK4CONNECTION); > +if (ret < 0) > +return ret; > +return 0; > } > > Q5. The tcpci_set_vbus in TCPCI driver uses command to control Sink/Source VBUS. > If our chip does not support power path, i.e. Sink & Source are controlled by other charger IC. Our chip will do nothing while setting these commands. > In the future TCPCI driver, will a framework be applied to notify these events. i.g. power_supply or notifier. > I would think so. Note that the driver is, at this point, fair game to change to make it work with your chip. The only condition is that a standard chip should still work, ie you should not make any changes which would cause the driver to _only_ work with your chip. Everything else is fair game. Thanks, Guenter