From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: USB: serial: ftdi_sio: implement GPIO support for FT230X From: Johan Hovold Message-Id: <20180905081939.GU28861@localhost> Date: Wed, 5 Sep 2018 10:19:39 +0200 To: Karoly Pados Cc: Johan Hovold , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Loic Poulain , andy.shevchenko@gmail.com, ajaykuee@gmail.com, daniel.thompson@linaro.org List-ID: T24gVHVlLCBTZXAgMDQsIDIwMTggYXQgMDU6NTM6MTRQTSArMDAwMCwgS2Fyb2x5IFBhZG9zIHdy b3RlOgoKPiA+IEFjdHVhbGx5LCBhZnRlciB0aGlua2luZyBhYm91dCB0aGlzIHNvbWUgbW9yZSwg aXQgbWF5IGJlIGJldHRlciB0byBqdXN0Cj4gPiBjb25maWd1cmUgYWxsIHBpbnMgZHVyaW5nIHBy b2JlLiBUaGUgZGV2aWNlIGlzIG1hbmFnZWQgYnkgdGhlIGtlcm5lbCwKPiA+IGFuZCB3ZSBjYW4n dCByZWFsbHkgY29uc2lkZXIgd2hhdCB1c2VyIHNwYWNlIG1heSBoYXZlIGRvbmUgYmVmb3JlLCBh dAo+ID4gbGVhc3QgaWYgd2UgY2FuJ3QgcXVlcnkgdGhlIGRldmljZS4gQmV0dGVyIHRvIGJlIGlu IGEgY29uc2lzdGVudCBzdGF0ZQo+ID4gd2hpbGUgdGhlIGRyaXZlciBpcyBib3VuZC4KPiAKPiBU aGUgQ0JVUyBwaW5zIGRvIG5vdCBhZmZlY3QgdGhlIFVBUlQgY29tbXVuaWNhdGlvbiBpbiBhbnkg d2F5LCBzbyBpZiB3ZSdyZQo+IG5vdCB1c2luZyB0aGUgR1BJTywgSSBkb24ndCBzZWUgYW55IHJl YXNvbiB3aHkgd2Ugc2hvdWxkIGRlc3Ryb3kgdGhlIENCVVMKPiBzdGF0ZSBqdXN0IGZvciB0aGUg c2FrZSBvZiBrbm93aW5nIHdoYXQgdmFsdWUgdGhleSBoYXZlLCBldmVuIHRob3VnaCB3ZSBkb24n dAo+IHNldCBvciB1c2Ugb3IgbmVlZCB0aGVtIGFuZCB0aGVyZSBpcyBhbHNvIG5vIGludGVyYWN0 aW9uLiBCdXQgaWYgeW91IHdpc2ggc28sCj4gSSdsbCBzZXQgdGhlbSBkdXJpbmcgcHJvYmUsIEkg anVzdCB0aGluayB3ZSBhcmUgZGlzYWJsaW5nIGEgcG9zc2libGUgdXNlLWNhc2UKPiB3aXRoIG5v IGFkZGVkIGdhaW4uCgpZZWFoLCBJIHVuZGVyc3RhbmQgdGhhdCBwb2ludCwgYnV0IHN0aWxsIG5v dCBzb2xkIG9uIHRoZSBpZGVhIG9mIGhhdmluZwpwb3RlbnRpYWxseSBhbGwgZm91ciBwaW5zIGNo YW5nZSBzdGF0ZSB3aGVuIHlvdSByZXF1ZXN0IG9uZSBvZiB0aGVtLgoKR29pbmcgYmFjayBhbmQg Zm9ydGgsIHNlYW1sZXNzbHksIGJldHdlZW4gaGF2aW5nIHRoZSBrZXJuZWwgb3IgdXNlcgpzcGFj ZSBtYW5hZ2UgYSBkZXZpY2UgZ2VuZXJhbGx5IGp1c3QgaXNuJ3QgYSBzdXBwb3J0ZWQgdXNlIGNh c2UuCgpCdXQgdGhlcmUgaXMgYW5vdGhlciAoYXNwZWN0IG9mIHlvdXIpIGFyZ3VtZW50IGZvciB5 b3VyIGFwcHJvYWNoLCBhbmQKdGhhdCB3b3VsZCBiZSB0aGF0IHBlb3BsZSBtYXkgYmUgcmVseWlu ZyBvbiB0aGlzIGJlaGF2aW91ciBhbHJlYWR5LiBUaGF0CmlzLCBkdWUgdG8gbGFjayBvZiBDQlVT IHN1cHBvcnQgaW4gdGhlIGtlcm5lbCBkcml2ZXIsIHlvdSBoYXZlIGxpYmZ0ZGkKc2V0dXAgdGhv c2UgcGlucyBhbmQgdGhlbiBiaW5kIHRoZSBrZXJuZWwgZHJpdmVyLiBXb3VsZCB0aGF0IGV2ZW4g d29yawp0b2RheSAoaS5lLiBpdCdzIG5vdGhpbmcgdGhhdCBnZXRzIHJlc2V0IHdoZW4gYmluZGlu ZyB0aGUgZHJpdmVyKT8gSWYKc28sIG1lIG1heSBoYXZlIHRvIGNvbnRpbnVlIHN1cHBvcnRpbmcg aXQuCgpXaGF0IGlzIHRoZSBkZWZhdWx0IHN0YXRlIG9mIHlvdXIgZGV2aWNlcyBhZnRlciByZXNl dCBieSB0aGUgd2F5PwpBbGwgaW5wdXRzPwoKT2ssIHlvdSBtYXkgaGF2ZSBjb252aW5jZWQgbWUu IDopCgo+ID4+ICsgLyogZGV2aWNlJ3MgZGlyZWN0aW9uIHBvbGFyaXR5IGlzIGRpZmZlcmVudCBm cm9tIGtlcm5lbCdzICovCj4gPiAKPiA+IFdoeSBzbz8gWW91IGNvdWxkIGp1c3QgcmVwbGFjZSBn cGlvX2lucHV0IHdpdGggZ3Bpb19vdXRwdXQuIEkgdGhpbmsgdGhhdAo+ID4gbWF5IGJlIHByZWZl cnJlZC4KPiAKPiBUaGF0IGRvZXNuJ3QgY2hhbmdlIHRoZSBmYWN0IHRoYXQgZm9yIHRoZSBrZXJu ZWwgKGZvciBleGFtcGxlIGluCj4gZnRkaV9ncGlvX2RpcmVjdGlvbl9pbnB1dCBvciBmdGRpX2dw aW9fZGlyZWN0aW9uX291dHB1dCkgJzEnIGlzIHN0aWxsIGlucHV0LgoKQWN0dWFsbHkgdGhvc2Ug dHdvIGZ1bmN0aW9ucyBkb24ndCB0YWtlIGFueSBkaXJlY3Rpb24gYXJndW1lbnQsIGJ1dApnZXRf ZGlyZWN0aW9uKCkgZG9lcyBpbmRlZWQgcmV0dXJuIDEgZm9yIGlucHV0LiBCdXQgeW91IGNhbiBm aW5kCmV4YW1wbGVzIG9mIHRoZSBvcHBvc2l0ZSB0b28gKGUuZy4gRkxBR19JU19PVVQgaW4gZ3Bp b19kZXNjKS4KCj4gWWVzIEkga25vdyB3ZSBjYW4gZG8gdGhlIGludmVyc2lvbiBpbiB0aG9zZSBm dW5jdGlvbnMgImZvciBmcmVlIiBieSBzZXR0aW5nCj4gaW5zdGVhZCBvZiBjbGVhcmluZyBhbmQg dmljZS12ZXJzYSwgSSBqdXN0IHRob3VnaHQgaXQgaXMgYmV0dGVyIGlmIEkgY2hvb3NlCj4gdG8g c3RheSB3aXRoIHRoZSBrZXJuZWwgY29udmVudGlvbiBhbmQgdHVybiBpdCBkZXZpY2Utc3BlY2lm aWMgYXQgdGhlIGRlZXBlc3QKPiBsZXZlbCBwb3NzaWJsZS4gTm90IGFyZ3VpbmcsIGp1c3QgZXhw bGFpbmluZyB3aGF0IG15IG1vdGl2YXRpb24gd2FzLiBJJ2xsCj4gY2hhbmdlIGl0IGFzIHlvdSBy ZXF1ZXN0ZWQuCgpUaGFua3MsIEkgdGhpbmsgaW52ZXJ0aW5nIHRoZSBkaXJlY3Rpb24gbWFzayB3 aWxsIGFsbG93IGZvciBzb21lCmVhc2llci10by1mb2xsb3cgY29kZSBpbiB0aGlzIGNhc2UuCgo+ ID4gRmFjdG9yIHRoaXMgb3V0IHRvIGFuIGVlcHJvbSBoZWxwZXIgdGhhdCBjYW4gYmUgcmV1c2Vk IGJ5IG90aGVyIGNoaXAKPiA+IHR5cGVzLgo+IAo+IFllcCwgSSdsbCBjb25zdWx0IHRoZSBvdGhl ciBncGlvIHBhdGNoIHJlZ2FyZGluZyB0aGlzIGFzIHlvdSBzdWdnZXN0ZWQKPiBpbiB0aGUgb3Ro ZXIncyByZXZpZXcuIEJ5IHRoZSB3YXksIHNvcnJ5IGZvciB0aGUgcGFyYWxsZWwgc3VibWlzc2lv biwKPiBJIHdhc24ndCBhd2FyZSBvZiBQb3VsYWluJ3MgcGF0Y2ggaW4gdGhpcyBzYW1lIG1vbnRo LgoKTm8gd29ycmllcywganVzdCBhbiB1bmZvcnR1bmF0ZSBjb2luY2lkZW5jZS4gQW5kIHRoZSBt b3JlIGV5ZXMgb24gdGhpcywKdGhlIGJldHRlci4KCj4gPj4gKyBwcml2LT5nYy5uZ3BpbyA9IDQ7 Cj4gPiAKPiA+IFNob3VsZG4ndCB0aGlzIGJlIGhhbmRsZWQgdGhlIG90aGVyIHdheSByb3VuZD8g SUlSQyB0aGVyZSBhcmUgdHdvIEZUWAo+ID4gZGV2aWNlIHR5cGVzIHdpdGggZm91ciBwaW5zLCBh bmQgb25lIHR5cGUgd2hlcmUgb25seSBvbmUgcGluIGlzCj4gPiBhY2Nlc3NpYmxlLgo+IAo+IFRo ZXJlIGFyZSA0IGRldmljZXMgd2l0aCAxIEdQSU8sIDEgZGV2aWNlIHdpdGggMiBHUElPcywgMiBk ZXZpY2VzIHdpdGggNCBHUElPcywKPiBhbmQgMSBkZXZpY2Ugd2l0aCA2IEdQSU9zLiBTb3VyY2U6 IGh0dHA6Ly93d3cuZnRkaWNoaXAuY29tL0ZULVguaHRtICgybmQgdGFibGUpCj4gRG8geW91IHN0 aWxsIHdhbnQgbWUgdHVybiB0aGlzIG92ZXI/CgpZZXAsIGFzIHlvdSBub3RpY2VkIGluIHlvdXIg Zm9sbG93IHVwLCBzb21lIG9mIHRob3NlIGRldmljZXMgeW91IHJlZmVyCnRvIGFib3ZlIGFyZSBu b3QgVVNCLVVBUlQgYnJpZGdlcy4KCj4gPj4gKyNlbHNlCj4gPj4gKwo+ID4+ICtzdGF0aWMgaW50 IGZ0ZGlfc2lvX2dwaW9faW5pdChzdHJ1Y3QgdXNiX3NlcmlhbCAqc2VyaWFsKQo+ID4gCj4gPiBB cyB0aGUgdGVzdCByb2JvdCByZXBvcnRlZCwgdGhlc2Ugc2hvdWxkIHRha2UgYSBwb3J0IGFzIHRo ZWlyIGFyZ3VtZW50Lgo+IAo+IFllcywgSSBhbHJlYWR5IHNlbnQgaW4gYSB2MiBmb3IgdGhhdC4g U28gbXkgcGF0Y2ggaW5jb3Jwb3JhdGluZyB5b3VyIGZlZWRiYWNrCj4gd2lsbCBiZSB2My4KCkdy ZWF0LCB0aGFua3MuCgpKb2hhbgo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.4 required=3.0 tests=DKIM_SIGNED, MAILING_LIST_MULTI,SPF_PASS,T_DKIM_INVALID,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4082CC433F5 for ; Wed, 5 Sep 2018 08:19:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D5BDB206BA for ; Wed, 5 Sep 2018 08:19:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="gkIiLMFP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D5BDB206BA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727713AbeIEMsg (ORCPT ); Wed, 5 Sep 2018 08:48:36 -0400 Received: from mail-lj1-f194.google.com ([209.85.208.194]:40585 "EHLO mail-lj1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726342AbeIEMsg (ORCPT ); Wed, 5 Sep 2018 08:48:36 -0400 Received: by mail-lj1-f194.google.com with SMTP id j19-v6so5458153ljc.7; Wed, 05 Sep 2018 01:19:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=GLOoQ8DK8xtx6gwKI/UCn2K/qifXex2M6r0t2iBHwC0=; b=gkIiLMFP1K8aXxG3xPU4yiJM9HGUfGnpdAf5Ksqyn7UQWfoob4EbPY0qM0P0VQtlEO z7iTYXyqoMACEAPeV6ZhkAvkTQfGmjU2y6m/4IfaSOpci73ajeAaCOADVb2Q3MX5un2+ KbvH2gi86YFt+CsMmmegdoX8d1CS4pFCRVeTOBAFOvMzUiEfyWuR2Y2n7FzJukwvijDi 4AR/ue7+ghprSYr5/8ngx778RSSEGmTeXb56IdHuZllE4Z7bXdaFI72NYYQNOUR+TXIm Rqj8ch9KCVxWO1vV0cfGC22KHJMDlz7C4XJpWSNtjoiWWgdRdiTjHINixkkrAwBMK/N1 MHfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=GLOoQ8DK8xtx6gwKI/UCn2K/qifXex2M6r0t2iBHwC0=; b=lxOsawr81IXZRhp8LRr2GxK+eMk5Buj5ICLf6txiExMkRkiiEZFhsSt/+OBVXbqCcw 0gaTuBP9vUsMBsrWnMhIFACws8fT86K/OOTplCYRC34jhA9wikdGnNBPHhZXilUHD/Zx l83Dm/jQbdHHjomDiajGn6Gkpgh97NfgOez0EFPkQuWG4e81+wzdJ5fxN1tLHV3PX8Zv yQ6zszvi+KOuG062LLJKPrOSJ1cWqxytbzp5MWRYQ32P4p0ClyUS+9zbkTXyzok/wlkZ XoMsrNoh4jlV2lPlh5jh7QIIH7wnLtAWJyeJ8++Tymrm4sM4vTVccRjMTtUMlwr+kt40 Phww== X-Gm-Message-State: APzg51CMzCAhOQqLZlU1QiTNuM8BDE9F7wzwMiHtUY91YT6Ofn/Wocjf Q+f1De38qM+7vs+ASK7n7GU= X-Google-Smtp-Source: ANB0VdbFQeTrO7cQeHoc64yv5z38Ot82fWFjlsglBKGmPL79TELH3v9/mHa8RQAPiDkZvw8KhmMDTA== X-Received: by 2002:a2e:1301:: with SMTP id 1-v6mr18529364ljt.56.1536135570610; Wed, 05 Sep 2018 01:19:30 -0700 (PDT) Received: from xi.terra (c-74bee655.07-184-6d6c6d4.bbcust.telenor.se. [85.230.190.116]) by smtp.gmail.com with ESMTPSA id w18-v6sm185612ljd.73.2018.09.05.01.19.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Sep 2018 01:19:29 -0700 (PDT) Received: from johan by xi.terra with local (Exim 4.91) (envelope-from ) id 1fxT2J-0003j6-T4; Wed, 05 Sep 2018 10:19:39 +0200 Date: Wed, 5 Sep 2018 10:19:39 +0200 From: Johan Hovold To: Karoly Pados Cc: Johan Hovold , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Loic Poulain , andy.shevchenko@gmail.com, ajaykuee@gmail.com, daniel.thompson@linaro.org Subject: Re: [PATCH] USB: serial: ftdi_sio: implement GPIO support for FT230X Message-ID: <20180905081939.GU28861@localhost> References: <20180904124924.GA7278@localhost> <20180825204744.2307-1-pados@pados.hu> <2d91567c930ae5770cc55f92c37a9c6d@pados.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2d91567c930ae5770cc55f92c37a9c6d@pados.hu> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 04, 2018 at 05:53:14PM +0000, Karoly Pados wrote: > > Actually, after thinking about this some more, it may be better to just > > configure all pins during probe. The device is managed by the kernel, > > and we can't really consider what user space may have done before, at > > least if we can't query the device. Better to be in a consistent state > > while the driver is bound. > > The CBUS pins do not affect the UART communication in any way, so if we're > not using the GPIO, I don't see any reason why we should destroy the CBUS > state just for the sake of knowing what value they have, even though we don't > set or use or need them and there is also no interaction. But if you wish so, > I'll set them during probe, I just think we are disabling a possible use-case > with no added gain. Yeah, I understand that point, but still not sold on the idea of having potentially all four pins change state when you request one of them. Going back and forth, seamlessly, between having the kernel or user space manage a device generally just isn't a supported use case. But there is another (aspect of your) argument for your approach, and that would be that people may be relying on this behaviour already. That is, due to lack of CBUS support in the kernel driver, you have libftdi setup those pins and then bind the kernel driver. Would that even work today (i.e. it's nothing that gets reset when binding the driver)? If so, me may have to continue supporting it. What is the default state of your devices after reset by the way? All inputs? Ok, you may have convinced me. :) > >> + /* device's direction polarity is different from kernel's */ > > > > Why so? You could just replace gpio_input with gpio_output. I think that > > may be preferred. > > That doesn't change the fact that for the kernel (for example in > ftdi_gpio_direction_input or ftdi_gpio_direction_output) '1' is still input. Actually those two functions don't take any direction argument, but get_direction() does indeed return 1 for input. But you can find examples of the opposite too (e.g. FLAG_IS_OUT in gpio_desc). > Yes I know we can do the inversion in those functions "for free" by setting > instead of clearing and vice-versa, I just thought it is better if I choose > to stay with the kernel convention and turn it device-specific at the deepest > level possible. Not arguing, just explaining what my motivation was. I'll > change it as you requested. Thanks, I think inverting the direction mask will allow for some easier-to-follow code in this case. > > Factor this out to an eeprom helper that can be reused by other chip > > types. > > Yep, I'll consult the other gpio patch regarding this as you suggested > in the other's review. By the way, sorry for the parallel submission, > I wasn't aware of Poulain's patch in this same month. No worries, just an unfortunate coincidence. And the more eyes on this, the better. > >> + priv->gc.ngpio = 4; > > > > Shouldn't this be handled the other way round? IIRC there are two FTX > > device types with four pins, and one type where only one pin is > > accessible. > > There are 4 devices with 1 GPIO, 1 device with 2 GPIOs, 2 devices with 4 GPIOs, > and 1 device with 6 GPIOs. Source: http://www.ftdichip.com/FT-X.htm (2nd table) > Do you still want me turn this over? Yep, as you noticed in your follow up, some of those devices you refer to above are not USB-UART bridges. > >> +#else > >> + > >> +static int ftdi_sio_gpio_init(struct usb_serial *serial) > > > > As the test robot reported, these should take a port as their argument. > > Yes, I already sent in a v2 for that. So my patch incorporating your feedback > will be v3. Great, thanks. Johan