From mboxrd@z Thu Jan 1 00:00:00 1970 From: Trent Piepho Subject: Re: [PATCH v2 1/4] spi: imx: GPIO based chip selects should not be required Date: Fri, 3 Nov 2017 19:18:56 +0000 Message-ID: <1509736735.5473.87.camel@impinj.com> References: <20171027010841.28624-1-tpiepho@impinj.com> <20171027010841.28624-2-tpiepho@impinj.com> <20171031111919.gocl7wwrhkwnxrya@sirena.co.uk> <1509469061.5473.16.camel@impinj.com> <20171102151439.6dpfud7a5vtc27dy@sirena.co.uk> <1509731639.5473.60.camel@impinj.com> <20171103183700.spnqbagr4q7fth4k@sirena.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org" , "fabio.estevam-3arQi8VN3Tc@public.gmane.org" To: "broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" Return-path: In-Reply-To: <20171103183700.spnqbagr4q7fth4k-7j8lgAiuQgnQXOPxS62xeg@public.gmane.org> Content-Language: en-US Content-ID: <0232022036B36A419D6C33C92BCE2CAC-+1mpgTUVCH2cE4WynfumptQqCkab/8FMAL8bYrjMMd8@public.gmane.org> Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: T24gRnJpLCAyMDE3LTExLTAzIGF0IDE4OjM3ICswMDAwLCBNYXJrIEJyb3duIHdyb3RlOg0KPiBP biBGcmksIE5vdiAwMywgMjAxNyBhdCAwNTo1Mzo1OVBNICswMDAwLCBUcmVudCBQaWVwaG8gd3Jv dGU6DQo+IA0KPiA+IEp1c3QgdG8gYmUgY2xlYXIsIG9uZSBkb2Vzbid0IG5lZWQgdG8gdXNlIEdQ SU9zIHdpdGggdGhlIGRyaXZlciBhcyBpcy4gDQo+ID4gQnV0IHRoZSBiaW5kaW5ncyB0byBkbyB0 aGF0IGFyZSBub24tc3RhbmRhcmQgYW5kIHRoZXNlIHBhdGNoZXMgbWFrZSB0aGUNCj4gPiBkcml2 ZXIgZm9sbG93IHRoZSBzdGFuZGFyZC4gKGFuZCBkb24ndCBicmVhayBhbnlvbmUpLg0KPiANCj4g SWYgdGhlcmUgYXJlIG5vbi1zdGFuZGFyZCBiaW5kaW5ncyB0aGVuIG1hcmsgdGhlbSBhcyBkZXBy ZWNhdGVkLiAgSQ0KPiBjYW4ndCBpbW1lZGlhdGVseSBmaW5kICphbnkqIGJpbmRpbmcgZG9jdW1l bnRhdGlvbiBmb3IgdGhpcyBjb250cm9sbGVyLg0KPiBUaGUgbGFzdCBjb21taXQgbG9va3MgbGlr ZSBpdCB3YXMgbW9yZSBhdHRlbXB0aW5nIHRvIHdvcmsgcm91bmQgYnJva2VuDQo+IGJvYXJkIGJp bmRpbmdzIGFuZCBkbyBzb21ldGhpbmcgc2Vuc2libGUgdGhhbiBhZGQgYSBuZXcgYmluZGluZywg YXQNCj4gbGVhc3QgdGhhdCdzIHdoYXQgSSByZW1lbWJlciBteSBzZW5zZSBvZiBpdCBiZWluZy4N Cg0KVGhlIG5vbi1zdGFuZGFyZCBwYXJ0IGlzIG5lZWRpbmcgdG8gYWRkIGNzLWdwaW9zID0gPDA+ IHRvIGdldCBhIG5hdGl2ZQ0KY2hpcCBzZWxlY3Qgd2hlbiB0aGF0IGlzIGRvY3VtZW50ZWQgYXMg YmVpbmcgb3B0aW9uYWwuICBJdCBkb2Vzbid0DQpmb2xsb3cgdGhlIHNwZWMuICBJdCBkb2Vzbid0 IG1hdGNoIG90aGVyIGRyaXZlcnMgKGFuZCBkdy1zcGkgaXMgZXF1YWxseQ0KYXMgYnJva2VuIGlu IHRoZSBzYW1lIG1hbm5lciwgcHJvYmFibHkgb3RoZXJzIHRvbykgdGhhdCBkbyBmb2xsb3cgdGhl DQpzcGVjLg0KDQo+IElmIHRoZSBoYXJkd2FyZSBpcyBhcyBicm9rZW4gYXMgdGhlc2UgY29udHJv bGxlcnMgYWx3YXlzIHdlcmUgaW4gdGhlDQo+IHBhc3QgYW5kIHRoZXJlIGFyZSB3b3JrYXJvdW5k cyB3aGljaCB3b3JrIGluIGFsbCBwcmFjdGljYWwgc2l0dWF0aW9ucw0KPiAoQUZBSUsgYWxsIHRo ZSByZWxldmFudCBTb0NzIHBlcm1pdCBHUElPIHVzYWdlIG9uIHRoZSBjaGlwIHNlbGVjdCBwaW5z KQ0KDQpDb21tZW50cyBpbiB0aGUgZHJpdmVyIGluZGljYXRlIHRoYXQgc29tZSBTb0NzIGRvIG5v dCBhbGxvdyBHUElPIHVzYWdlDQpvbiBhbGwgY2hpcCBzZWxlY3QgcGlucy4NCg0KPiBkb2N1bWVu dGluZyBzb21ldGhpbmcgYXMgc3VwcG9ydGVkIGlzIGp1c3QgZ29pbmcgdG8gbWFrZSBwZW9wbGUN Cj4gbWlzZXJhYmxlLiAgVGhlIHJlYXNvbiBJIGtub3cgYWJvdXQgdGhpcyBicmVha2FnZSBpcyB0 aGF0IEkgaGFkIHRvIGdvDQo+IHRocm91Z2ggdGhlIHByb2Nlc3Mgb2Ygd29ya2luZyBvdXQgdGhh dCB0aGUgbmF0aXZlIGNoaXAgc2VsZWN0IHN1cHBvcnQNCj4gZGlkbid0IHdvcmsgb24gYSBzeXN0 ZW0uDQoNCkkganVzdCBkb24ndCBzZWUgaG93IG5vdCBmb2xsb3dpbmcgdGhlIGRldmljZSB0cmVl IGJpbmRpbmcNCnNwZWNpZmljYXRpb24gZG9jdW1lbnRzIHRoZSBoYXJkd2FyZSBmbGF3LCBvciBo b3cgZm9sbG93aW5nIHRoZSBzcGVjDQpkb2N1bWVudHMgdGhhdCB0aGUgZmxhdyBkb2VzIG5vdCBl eGlzdC4NCg0KPiA+IElmIHRoZSBnb2FsIGlzIHRvIGRvY3VtZW50IHRoZSBoYXJkd2FyZSBxdWly a3MsIHRoZW4gc2hvdWxkbid0IHRoaXMgYmUNCj4gPiBkb25lIHRocm91Z2ggZG9jdW1lbnRhdGlv bj8gIEEgbm90ZSBvciBwb2ludGVyIGluIHRoZSBrY29uZmlnLA0KPiA+IHNvbWV0aGluZyBpbiB0 aGUgc291cmNlLCBhIGJldHRlciBkZXNjcmlwdGlvbiBpbiBEb2N1bWVudGlvbi8NCj4gPiBzb21l d2hlcmUsIGV0Yy4gIFRoYXQgd291bGQgaGF2ZSBhIGJldHRlciBjaGFuY2Ugb2YgYmVpbmcgc2Vl biBiZWZvcmUNCj4gPiBoYXJkd2FyZSBpcyBkZXNpZ25lZCwgYW5kIHdvdWxkIGV4cGxhaW4gdGhl IGlzc3VlIHRvbywgaW5zdGVhZCBvZiBqdXN0DQo+ID4gYXBwZWFyaW5nIGFzIGFub3RoZXIgcXVp cmsgaW4gZGV2aWNlIHRyZWUgYmluZGluZ3MuDQo+IA0KPiBZZXMsIGJldHRlciBkb2N1bWVudGF0 aW9uIHdvdWxkIGJlIGdyZWF0Lg0KDQpIb3cgYWJvdXQgSSBhZGQgc29tZXRoaW5nIHRvIERvY3Vt ZW50YXRpb24vc3BpIGFuZCBhZGQgYSBub3RlIGluDQpLY29uZmlnLCBidXQgbWFrZSB0aGUgZHJp dmVyIHN0YW5kYXJkIGNvbXBsaWFudCBpbiBpdHMgZGV2aWNlIHRyZWUNCmJpbmRpbmdzPw0KDQpU aGUgZ29hbCBoZXJlIGlzIHRoYXQgZXZlcnlvbmUgZG9lc24ndCBoYXZlIHRvIHN0aWNrIGEgc2Nv cGUgb24gdGhlDQpib2FyZCBhbmQgcmUtZGlzY292ZXIgdGhhdCB0aGUgbmF0aXZlIENTIGRvZXNu J3QgZG8gd2hhdCB0aGV5IHdhbnQsDQpyaWdodD8gQmVlbiB0aGVyZSwgZG9uZSB0aGF0LCBhbmQg Y2FuIGFwcHJlY2lhdGUgdGhlIHNlbnRpbWVudC4NCg0KSSBkb24ndCB0aGluayBub3QgZm9sbG93 aW5nIHRoZSBzdGFuZGFyZCBpcyBhbiBlZmZlY3RpdmUgd2F5IHRvIGRvDQp0aGF0LiAgRnJvbSBh IHNhbXBsZSBvZiB0aHJlZSBkZXZlbG9wZXJzLCB0d28gY2FtZSB1cCB3aXRoIGR0IGJpbmRpbmdz DQp0byB1c2UgbmF0aXZlIGNzIGFuZCBkZWNpZGVkIHRoZXkgYXBwYXJlbnRseSBtaXN1bmRlcnN0 b29kIHRoZSBzcGkgZHQNCnNwZWMgdG8gZXhwbGFpbiB3aHkgd2hhdCBzaG91bGQgaGF2ZSB3b3Jr ZWQgZGlkIG5vdC4gIFRoZSB0aGlyZCAobWUpDQpsb29rZWQgaW50byB0aGUgZHJpdmVyIHNvdXJj ZSB0byBmaW5kIHRoZSB3aHksIGFuZCBldmVuIHRoZW4gaXQgd2Fzbid0DQpjbGVhciB0aGUgYmVo YXZpb3Igd2FzIGludGVudGlvbmFsIG9yIGFuIG92ZXJzaWdodC4gIEkgdGhpbmsNCmRvY3VtZW50 aW5nIHRoZSBrbm93biBmbGF3cyB3b3VsZCBiZSBhIGJldHRlciBhbmQgbW9yZSBlZmZlY3RpdmUg d2F5IHRvDQpnZXQgdGhhdCBpbmZvcm1hdGlvbiBhY3Jvc3Mu -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: tpiepho@impinj.com (Trent Piepho) Date: Fri, 3 Nov 2017 19:18:56 +0000 Subject: [PATCH v2 1/4] spi: imx: GPIO based chip selects should not be required In-Reply-To: <20171103183700.spnqbagr4q7fth4k@sirena.co.uk> References: <20171027010841.28624-1-tpiepho@impinj.com> <20171027010841.28624-2-tpiepho@impinj.com> <20171031111919.gocl7wwrhkwnxrya@sirena.co.uk> <1509469061.5473.16.camel@impinj.com> <20171102151439.6dpfud7a5vtc27dy@sirena.co.uk> <1509731639.5473.60.camel@impinj.com> <20171103183700.spnqbagr4q7fth4k@sirena.co.uk> Message-ID: <1509736735.5473.87.camel@impinj.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 2017-11-03 at 18:37 +0000, Mark Brown wrote: > On Fri, Nov 03, 2017 at 05:53:59PM +0000, Trent Piepho wrote: > > > Just to be clear, one doesn't need to use GPIOs with the driver as is. > > But the bindings to do that are non-standard and these patches make the > > driver follow the standard. (and don't break anyone). > > If there are non-standard bindings then mark them as deprecated. I > can't immediately find *any* binding documentation for this controller. > The last commit looks like it was more attempting to work round broken > board bindings and do something sensible than add a new binding, at > least that's what I remember my sense of it being. The non-standard part is needing to add cs-gpios = <0> to get a native chip select when that is documented as being optional. It doesn't follow the spec. It doesn't match other drivers (and dw-spi is equally as broken in the same manner, probably others too) that do follow the spec. > If the hardware is as broken as these controllers always were in the > past and there are workarounds which work in all practical situations > (AFAIK all the relevant SoCs permit GPIO usage on the chip select pins) Comments in the driver indicate that some SoCs do not allow GPIO usage on all chip select pins. > documenting something as supported is just going to make people > miserable. The reason I know about this breakage is that I had to go > through the process of working out that the native chip select support > didn't work on a system. I just don't see how not following the device tree binding specification documents the hardware flaw, or how following the spec documents that the flaw does not exist. > > If the goal is to document the hardware quirks, then shouldn't this be > > done through documentation? A note or pointer in the kconfig, > > something in the source, a better description in Documention/ > > somewhere, etc. That would have a better chance of being seen before > > hardware is designed, and would explain the issue too, instead of just > > appearing as another quirk in device tree bindings. > > Yes, better documentation would be great. How about I add something to Documentation/spi and add a note in Kconfig, but make the driver standard compliant in its device tree bindings? The goal here is that everyone doesn't have to stick a scope on the board and re-discover that the native CS doesn't do what they want, right? Been there, done that, and can appreciate the sentiment. I don't think not following the standard is an effective way to do that. From a sample of three developers, two came up with dt bindings to use native cs and decided they apparently misunderstood the spi dt spec to explain why what should have worked did not. The third (me) looked into the driver source to find the why, and even then it wasn't clear the behavior was intentional or an oversight. I think documenting the known flaws would be a better and more effective way to get that information across.