From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [PATCH v2 5/5] i2c: davinci: use ICPFUNC to toggle I2C as gpio for bus recovery Date: Tue, 7 Jul 2015 17:53:59 +0300 Message-ID: <559BE807.2030802@ti.com> References: <1417010393-30598-1-git-send-email-grygorii.strashko@ti.com> <1417010393-30598-6-git-send-email-grygorii.strashko@ti.com> <20141126160407.GN4431@pengutronix.de> <5476085B.40009@ti.com> <1436276269.3344.94.camel@pengutronix.de> <20150707134852.GE9671@pengutronix.de> <20150707141312.GC3411@katana> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20150707141312.GC3411@katana> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Wolfram Sang , =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= Cc: devicetree@vger.kernel.org, =?UTF-8?B?SmFuIEzDvGJiZQ==?= , Kevin Hilman , Santosh Shilimkar , Sekhar Nori , linux-kernel@vger.kernel.org, Murali Karicheri , linux-i2c@vger.kernel.org, Mike Looijmans , linux-arm-kernel@lists.infradead.org List-Id: linux-i2c@vger.kernel.org T24gMDcvMDcvMjAxNSAwNToxMyBQTSwgV29sZnJhbSBTYW5nIHdyb3RlOgo+IE9uIFR1ZSwgSnVs IDA3LCAyMDE1IGF0IDAzOjQ4OjUyUE0gKzAyMDAsIFV3ZSBLbGVpbmUtS8O2bmlnIHdyb3RlOgoK Pj4gT24gVHVlLCBKdWwgMDcsIDIwMTUgYXQgMDM6Mzc6NDlQTSArMDIwMCwgSmFuIEzDvGJiZSB3 cm90ZToKPj4+IE9uIE1pLCAyMDE0LTExLTI2IGF0IDE5OjA1ICswMjAwLCBHcnlnb3JpaSBTdHJh c2hrbyB3cm90ZToKPj4+PiBPbiAxMS8yNi8yMDE0IDA2OjA0IFBNLCBVd2UgS2xlaW5lLUvDtm5p ZyB3cm90ZToKPj4+Pj4gT24gV2VkLCBOb3YgMjYsIDIwMTQgYXQgMDM6NTk6NTNQTSArMDIwMCwg R3J5Z29yaWkgU3RyYXNoa28gd3JvdGU6Cj4+Pj4+PiBIYXZpbmcgYSBib2FyZCB3aGVyZSB0aGUg STJDIGJ1cyBsb2NrcyB1cCBvY2Nhc2lvbmFsbHkgbWFkZSBpdCBjbGVhcgo+Pj4+Pj4gdGhhdCB0 aGUgYnVzIHJlY292ZXJ5IGluIHRoZSBpMmMtZGF2aW5jaSBkcml2ZXIgd2lsbCBvbmx5IHdvcmsg b24KPj4+Pj4+IHNvbWUgYm9hcmRzLCBiZWNhdXNlIG9uIHJlZ3VsYXIgYm9hcmRzLCB0aGlzIHdp bGwgb25seSB0b2dnbGUgR1BJTwo+Pj4+Pj4gbGluZXMgdGhhdCBhcmVuJ3QgbXV4ZWQgdG8gdGhl IGFjdHVhbCBwaW5zLgo+Pj4+Pj4KPj4+Pj4+IFRoZSBJMkMgY29udHJvbGxlciBvbiBTb0NzIGxp a2UgZGE4NTAgKGFuZCBkYTgzMCksIEtleXN0b25lIDIgaGFzIHRoZQo+Pj4+Pj4gYnVpbHQtaW4g Y2FwYWJpbGl0eSB0byBiaXQtYmFuZyBpdHMgbGluZXMgYnkgdXNpbmcgdGhlIElDUEZVTkMgcmVn aXN0ZXJzCj4+Pj4+PiBvZiB0aGUgaTJjIGNvbnRyb2xsZXIuCj4+Pj4+PiBJbXBsZW1lbnQgdGhl IHN1Z2dlc3RlZCBwcm9jZWR1cmUgYnkgdG9nZ2xpbmcgU0NMIGFuZCBjaGVja2luZyBTREEgdXNp bmcKPj4+Pj4+IHRoZSBJQ1BGVU5DIHJlZ2lzdGVycyBvZiB0aGUgSTJDIGNvbnRyb2xsZXIgd2hl biBwcmVzZW50LiBBbGxvdyBwbGF0Zm9ybXMKPj4+Pj4+IHRvIGluZGljYXRlIHRoZSBwcmVzZW5j ZSBvZiB0aGUgSUNQRlVOQyByZWdpc3RlcnMgd2l0aCBhIGhhc19wZnVuYyBwbGF0Zm9ybQo+Pj4+ Pj4gZGF0YSBmbGFnIGFuZCBhZGQgb3B0aW9uYWwgRFQgcHJvcGVydHkgInRpLGhhcy1wZnVuYyIg dG8gaW5kaWNhdGUKPj4+Pj4+IHRoZSBzYW1lIGluIERULgo+Pj4+PiBPbiB3aGF0IGRvZXMgaXQg ZGVwZW5kIGlmIHRoaXMgcGZ1bmMgc3R1ZmYgd29ya3Mgb3Igbm90PyBPbmx5IHRoZSBTb0MsCj4+ Pj4+IG9yIGFsc28gb24gc29tZSBib2FyZCBzcGVjaWZpYyBwcm9wZXJ0aWVzPwo+Pj4+Cj4+Pj4g U29DIC8gc2V0IG9mIFNvQ3MuIEFsc28sIHNpbWlsYXIgZmVhdHVyZSBpcyBzdXBwb3J0ZWQgYnkg T01BUCBhbmQgQU0zMzV4L0FNNDM3eCBTb0NzCj4+Pj4gdXNpbmcgSTJDX1NZU1RFU1QgcmVnaXN0 ZXIuCj4+Pj4KPj4+Pj4gR2l2ZW4gdGhlIGZvcm1lciB1c2luZyB0aGUKPj4+Pj4gY29tcGF0aWJs ZSBzdHJpbmcgdG8gZGV0ZWN0IGl0cyBhdmFpbGFiaWxpdHkgd291bGQgYmUgYmV0dGVyLiAoSW4g dGhpcwo+Pj4+PiBjYXNlIGFsc28gc29ycnksIGRpZG4ndCBjb25zaWRlciB0aGlzIGNhc2Ugd2hl biByZXF1ZXN0aW5nIHRoZSBwcm9wZXJ0eQo+Pj4+PiBpbiB0aGUgbGFzdCByb3VuZC4pCj4+Pgo+ Pj4gSSBvbmx5IHN0dW1ibGVkIGFjcm9zcyB0aGlzIGFmdGVyIGl0IHdhcyBtZXJnZWQsIHdpdGgg dGhlIGFkZGl0aW9uYWwKPj4gSSBhbHNvIHdvbmRlciBob3cgaXQgY2FtZSB0byB0aGUgUmV2aWV3 ZWQtYnkgdGFnIGZvciBtZS4gVGhlIGxhc3QgdGhpbmcKPj4gdGhhdCBJIHNhaWQgYWJvdXQgdGhl IHBhdGNoIHdhcyAiT24gd2hhdCBkb2VzIGl0IGRlcGVuZCBpZiB0aGlzIHBmdW5jCj4+IHN0dWZm IHdvcmtzIG9yIG5vdD8gT25seSB0aGUgU29DLCBvciBhbHNvIG9uIHNvbWUgYm9hcmQgc3BlY2lm aWMKPj4gcHJvcGVydGllcz8iIChzZWUgYWJvdmUpIGFuZCAidGhlIHBhdGNoIGxvb2tzIG9rIi4g SU1ITyB0aGlzIGhhcmRseQo+PiBqdXN0aWZpZXMgdG8gYWRkIHRoZSBSZXZpZXdlZC1ieSB0YWcg Zm9yIHRoZSBuZXh0IHJvdW5kLiA6LSgKPiAKPiBUaGF0IG5lZWRzIHRvIGJlIGRpc2N1c3NlZCB3 aXRoIEdyeWdvcmlpLiBJIGNhbid0IHZlcmlmeSB0aGUgY29ycmVjdG5lc3MKPiBvZiB0YWdzIGZv ciBldmVyeSBwYXRjaCwgYWx0aG91Z2ggSSBkbyB0cnkgdG8ga2VlcCBhbiBleWUgb24gaXQuLi4K PiAKClJlZ2FyZGluZyAidGhlIHBhdGNoIGxvb2tzIG9rIiAtIHNpbmNlcmVseSBzb3JyeSEKVGhp cyBpcyBub3QgdGhlIGZpcnN0IHRpbWUgSSd2ZSB0cmVhdGVkICJsb29rcyBnb29kLi4iIGFzIFJl dmlld2VkLWJ5IGFuZCBJIGdvdCBubyBjb21wbGFpbnRzIGJlZm9yZSA6KApXaWxsIHRha2UgaXQg aW50byBhY2NvdW50LgoKClJlZ2FyZGluZyB0ZWNobmljYWwgY29tbWVudHM6Ck9LLiBTZWVtcyBJ IG1pc3NlZCBzbXRoLiBvciB1bmRlcnN0b29kIHdyb25nbHkuClNvLCBJJ2xsIHNheSB3aGF0J3Mg cGVvcGxlIHVzdWFsbHkgc2F5aW5nIGhlcmUgLSBTb3JyeSBmb3IgdGhhdCA6KApCdXQsIHRvIGJl IGhvbmVzdCBJIGRvbid0IGZlZWwgZ3VpbHR5LCBiZWNhdXNlOgotIHY0IG9mIHRoZXNlIHBhdGNo ZXMgd2FzIG1lcmdlZCBmaW5hbGx5Ci0gdGhhdCB2NCBtaXNzZWQgPjIga2VybmVsIHJlbGVhc2Vz Ci0geW91IHdlcmUgYWRkZWQgaW4gIlRPOiIgZm9yIGFsbCB2ZXJzaW9ucyBvZiB0aGVzZSBwYXRj aGVzLgoKLS0gCnJlZ2FyZHMsCi1ncnlnb3JpaQoKX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5lbCBtYWlsaW5nIGxpc3QKbGludXgt YXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3Jn L21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: grygorii.strashko@ti.com (Grygorii Strashko) Date: Tue, 7 Jul 2015 17:53:59 +0300 Subject: [PATCH v2 5/5] i2c: davinci: use ICPFUNC to toggle I2C as gpio for bus recovery In-Reply-To: <20150707141312.GC3411@katana> References: <1417010393-30598-1-git-send-email-grygorii.strashko@ti.com> <1417010393-30598-6-git-send-email-grygorii.strashko@ti.com> <20141126160407.GN4431@pengutronix.de> <5476085B.40009@ti.com> <1436276269.3344.94.camel@pengutronix.de> <20150707134852.GE9671@pengutronix.de> <20150707141312.GC3411@katana> Message-ID: <559BE807.2030802@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 07/07/2015 05:13 PM, Wolfram Sang wrote: > On Tue, Jul 07, 2015 at 03:48:52PM +0200, Uwe Kleine-K?nig wrote: >> On Tue, Jul 07, 2015 at 03:37:49PM +0200, Jan L?bbe wrote: >>> On Mi, 2014-11-26 at 19:05 +0200, Grygorii Strashko wrote: >>>> On 11/26/2014 06:04 PM, Uwe Kleine-K?nig wrote: >>>>> On Wed, Nov 26, 2014 at 03:59:53PM +0200, Grygorii Strashko wrote: >>>>>> Having a board where the I2C bus locks up occasionally made it clear >>>>>> that the bus recovery in the i2c-davinci driver will only work on >>>>>> some boards, because on regular boards, this will only toggle GPIO >>>>>> lines that aren't muxed to the actual pins. >>>>>> >>>>>> The I2C controller on SoCs like da850 (and da830), Keystone 2 has the >>>>>> built-in capability to bit-bang its lines by using the ICPFUNC registers >>>>>> of the i2c controller. >>>>>> Implement the suggested procedure by toggling SCL and checking SDA using >>>>>> the ICPFUNC registers of the I2C controller when present. Allow platforms >>>>>> to indicate the presence of the ICPFUNC registers with a has_pfunc platform >>>>>> data flag and add optional DT property "ti,has-pfunc" to indicate >>>>>> the same in DT. >>>>> On what does it depend if this pfunc stuff works or not? Only the SoC, >>>>> or also on some board specific properties? >>>> >>>> SoC / set of SoCs. Also, similar feature is supported by OMAP and AM335x/AM437x SoCs >>>> using I2C_SYSTEST register. >>>> >>>>> Given the former using the >>>>> compatible string to detect its availability would be better. (In this >>>>> case also sorry, didn't consider this case when requesting the property >>>>> in the last round.) >>> >>> I only stumbled across this after it was merged, with the additional >> I also wonder how it came to the Reviewed-by tag for me. The last thing >> that I said about the patch was "On what does it depend if this pfunc >> stuff works or not? Only the SoC, or also on some board specific >> properties?" (see above) and "the patch looks ok". IMHO this hardly >> justifies to add the Reviewed-by tag for the next round. :-( > > That needs to be discussed with Grygorii. I can't verify the correctness > of tags for every patch, although I do try to keep an eye on it... > Regarding "the patch looks ok" - sincerely sorry! This is not the first time I've treated "looks good.." as Reviewed-by and I got no complaints before :( Will take it into account. Regarding technical comments: OK. Seems I missed smth. or understood wrongly. So, I'll say what's people usually saying here - Sorry for that :( But, to be honest I don't feel guilty, because: - v4 of these patches was merged finally - that v4 missed >2 kernel releases - you were added in "TO:" for all versions of these patches. -- regards, -grygorii From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757265AbbGGOzv (ORCPT ); Tue, 7 Jul 2015 10:55:51 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:36050 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757390AbbGGOyx (ORCPT ); Tue, 7 Jul 2015 10:54:53 -0400 Message-ID: <559BE807.2030802@ti.com> Date: Tue, 7 Jul 2015 17:53:59 +0300 From: Grygorii Strashko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Wolfram Sang , =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= CC: =?UTF-8?B?SmFuIEzDvGJiZQ==?= , Sekhar Nori , , , , Mike Looijmans , Kevin Hilman , Santosh Shilimkar , Murali Karicheri , Subject: Re: [PATCH v2 5/5] i2c: davinci: use ICPFUNC to toggle I2C as gpio for bus recovery References: <1417010393-30598-1-git-send-email-grygorii.strashko@ti.com> <1417010393-30598-6-git-send-email-grygorii.strashko@ti.com> <20141126160407.GN4431@pengutronix.de> <5476085B.40009@ti.com> <1436276269.3344.94.camel@pengutronix.de> <20150707134852.GE9671@pengutronix.de> <20150707141312.GC3411@katana> In-Reply-To: <20150707141312.GC3411@katana> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/07/2015 05:13 PM, Wolfram Sang wrote: > On Tue, Jul 07, 2015 at 03:48:52PM +0200, Uwe Kleine-König wrote: >> On Tue, Jul 07, 2015 at 03:37:49PM +0200, Jan Lübbe wrote: >>> On Mi, 2014-11-26 at 19:05 +0200, Grygorii Strashko wrote: >>>> On 11/26/2014 06:04 PM, Uwe Kleine-König wrote: >>>>> On Wed, Nov 26, 2014 at 03:59:53PM +0200, Grygorii Strashko wrote: >>>>>> Having a board where the I2C bus locks up occasionally made it clear >>>>>> that the bus recovery in the i2c-davinci driver will only work on >>>>>> some boards, because on regular boards, this will only toggle GPIO >>>>>> lines that aren't muxed to the actual pins. >>>>>> >>>>>> The I2C controller on SoCs like da850 (and da830), Keystone 2 has the >>>>>> built-in capability to bit-bang its lines by using the ICPFUNC registers >>>>>> of the i2c controller. >>>>>> Implement the suggested procedure by toggling SCL and checking SDA using >>>>>> the ICPFUNC registers of the I2C controller when present. Allow platforms >>>>>> to indicate the presence of the ICPFUNC registers with a has_pfunc platform >>>>>> data flag and add optional DT property "ti,has-pfunc" to indicate >>>>>> the same in DT. >>>>> On what does it depend if this pfunc stuff works or not? Only the SoC, >>>>> or also on some board specific properties? >>>> >>>> SoC / set of SoCs. Also, similar feature is supported by OMAP and AM335x/AM437x SoCs >>>> using I2C_SYSTEST register. >>>> >>>>> Given the former using the >>>>> compatible string to detect its availability would be better. (In this >>>>> case also sorry, didn't consider this case when requesting the property >>>>> in the last round.) >>> >>> I only stumbled across this after it was merged, with the additional >> I also wonder how it came to the Reviewed-by tag for me. The last thing >> that I said about the patch was "On what does it depend if this pfunc >> stuff works or not? Only the SoC, or also on some board specific >> properties?" (see above) and "the patch looks ok". IMHO this hardly >> justifies to add the Reviewed-by tag for the next round. :-( > > That needs to be discussed with Grygorii. I can't verify the correctness > of tags for every patch, although I do try to keep an eye on it... > Regarding "the patch looks ok" - sincerely sorry! This is not the first time I've treated "looks good.." as Reviewed-by and I got no complaints before :( Will take it into account. Regarding technical comments: OK. Seems I missed smth. or understood wrongly. So, I'll say what's people usually saying here - Sorry for that :( But, to be honest I don't feel guilty, because: - v4 of these patches was merged finally - that v4 missed >2 kernel releases - you were added in "TO:" for all versions of these patches. -- regards, -grygorii