From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH v2 04/10] spi: Extend the core to ease integration of SPI memory controllers Date: Wed, 18 Apr 2018 16:17:47 +0200 Message-ID: <20180418161747.7060ae3b@bbrezillon> References: <20180410224439.9260-1-boris.brezillon@bootlin.com> <20180410224439.9260-5-boris.brezillon@bootlin.com> <60f3d53a-9668-9e1c-9f29-45aaf4004630@ti.com> <20180412171005.5cfaba58@bbrezillon> <20180412215951.751bf889@bbrezillon> <991f6a90-2211-acb9-c384-1d49d5468b31@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: Yogesh Gaur , Kamal Dasu , Brian Norris , Richard Weinberger , Cyrille Pitchen , "linux-spi@vger.kernel.org" , Peter Pan , Marek Vasut , Frieder Schrempf , Mark Brown , "linux-mtd@lists.infradead.org" , Miquel Raynal , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Maxime Chevallier , David Woodhouse To: Vignesh R Return-path: In-Reply-To: <991f6a90-2211-acb9-c384-1d49d5468b31@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-mtd" Errors-To: linux-mtd-bounces+gldm-linux-mtd-36=gmane.org@lists.infradead.org List-Id: linux-spi.vger.kernel.org SGkgVmlnbmVzaCwKCk9uIFR1ZSwgMTcgQXByIDIwMTggMDk6NDI6MTggKzA1MzAKVmlnbmVzaCBS IDx2aWduZXNockB0aS5jb20+IHdyb3RlOgoKPiA+PiAgIAo+ID4+ID7CoMKgICAgCj4gPj4gPiA+ ICsgfSBhZGRyOwo+ID4+ID4gPiArCj4gPj4gPiA+ICsgc3RydWN0IHsKPiA+PiA+ID4gK8KgwqDC oMKgwqDCoMKgwqAgdTggbmJ5dGVzOwo+ID4+ID4gPiArwqDCoMKgwqDCoMKgwqDCoCB1OCBidXN3 aWR0aDsKPiA+PiA+ID4gKyB9IGR1bW15Owo+ID4+ID4gPiArCj4gPj4gPiA+ICsgc3RydWN0IHsK PiA+PiA+ID4gK8KgwqDCoMKgwqDCoMKgwqAgdTggYnVzd2lkdGg7Cj4gPj4gPiA+ICvCoMKgwqDC oMKgwqDCoMKgIGVudW0gc3BpX21lbV9kYXRhX2RpciBkaXI7Cj4gPj4gPiA+ICvCoMKgwqDCoMKg wqDCoMKgIHVuc2lnbmVkIGludCBuYnl0ZXM7Cj4gPj4gPiA+ICvCoMKgwqDCoMKgwqDCoMKgIC8q IGJ1Zi57aW4sb3V0fSBtdXN0IGJlIERNQS1hYmxlLiAqLwo+ID4+ID4gPiArwqDCoMKgwqDCoMKg wqDCoCB1bmlvbiB7Cj4gPj4gPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoCB2 b2lkICppbjsKPiA+PiA+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgIGNvbnN0 IHZvaWQgKm91dDsKPiA+PiA+ID4gK8KgwqDCoMKgwqDCoMKgwqAgfSBidWY7Cj4gPj4gPiA+ICsg fSBkYXRhOwo+ID4+ID4gPiArfTsKPiA+PiA+ID4gK8KgwqDCoCAgIAo+ID4+ID4gCj4gPj4gPiBT b21lIGZsYXNoIGRldmljZXMgc3VwcG9ydCBEdWFsL1F1YWQgRERSIChEb3VibGUgRGF0YSBSYXRl KSBtb2RlIGFuZCB0aGUKPiA+PiA+IFNQSSBjb250cm9sbGVyIGRyaXZlciB3b3VsZCBuZWVkIHRv IGtub3cgdGhpcyBpbmZvcm1hdGlvbi4gV2Ugd2lsbCBuZWVkCj4gPj4gPiB0byBhZGQgYSBmaWVs ZCBmb3IgdGhhdC7CoCAgIAo+ID4+IAo+ID4+IFdlbGwsIGxldCdzIHdhaXQgdW50aWwgd2UgYWN0 dWFsbHkgbmVlZCB0aGF0Lgo+ID4+ICAgCj4gPj4gPiAKPiA+PiA+IEN1cnJlbnRseSwgdGhlcmUg YXJlIGRyaXZlcnMgdW5kZXIgbXRkL3NwaS1ub3IvIHRoYXQgbmVlZCB0byBrbm93Cj4gPj4gPiBw YWdlL3NlY3Rvci90b3RhbCBzaXplIG9mIGZsYXNoIG1lbW9yeShpbmZvIGF2YWlsYWJsZSBpbgo+ ID4+ID4gLWBzdHJ1Y3Qgc3BpX25vcikuIFdlIHdvdWxkIG5lZWQgYSB3YXkgdG8gcHJvdmlkZSB0 aGlzIGluZm8gdG8gc3BpX21lbQo+ID4+ID4gZHJpdmVycywgaWYgd2UgZXZlciBwbGFuIHRvIG1v dmUgZHJpdmVycyB1bmRlciBtdGQvc3BpLW5vciB0byBzcGkvwqAgICAKPiA+PiAKPiA+PiBBZ2Fp biwgd2UnbGwgc2VlIHdoZW4gd2UnbGwgdHJ5IHRvIG1vdmUgdGhlbSwgYnV0IEkgaG9wZSBzdGls bCB3ZSB3b24ndAo+ID4+IG5lZWQgdGhhdC4gTG9va3MgbGlrZSB0aGUga2luZCBvZiBpbmZvcm1h dGlvbiBJJ2QgbGlrZSB0byBrZWVwIGF3YXkKPiA+PiBmcm9tIHNwaSBjb250cm9sbGVyIGRyaXZl cnMuICAKPiA+IAo+ID4gTGV0IG1lIGNsYXJpZnkgdGhpcyBwYXJ0LiBJIGFscmVhZHkgdGhvdWdo dCBhIGJpdCBhYm91dCB0aGlzIHByb2JsZW0sCj4gPiBhbmQgdGhhdCdzIHRoZSB2ZXJ5IHJlYXNv biB3ZSBoYXZlIGFuIGludGVybWVkaWF0ZSBsYXllciB3aXRoIGEgc3BpX21lbQo+ID4gc3RydWN0 IHBvaW50aW5nIHRvIHRoZSByZWFsIHNwaV9kZXZpY2Ugb2JqZWN0LiBUaGUgaWRlYSBpcyB0byBh ZGQgbmV3Cj4gPiBmaWVsZHMgdG8gc3BpX21lbSBvYmplY3QgaWYvd2hlbiB3ZSByZWFsbHkgaGF2 ZSB0by4gV2UnZCBhbHNvIGhhdmUgdG8KPiA+IGFkZCAtPmF0dGFjaC9kZXRhY2goKSBtZXRob2Rz IHRvIHNwaV9tZW1fb3BzIHNvIHRoYXQgU1BJIG1lbSBjb250cm9sbGVyCj4gPiBjYW4ga25vdyB3 aGVuIGEgbmV3IGRldmljZSBpcyBhYm91dCB0byBiZSBhY2Nlc3NlZCBieSBhIHNwaS1tZW0KPiA+ IGRyaXZlciwgY2FuIHBhcnNlIHRoZSBpbmZvcm1hdGlvbiBwcm92aWRlZCBpbiBzcGlfbWVtIGFu ZCBjb25maWd1cmUgdGhlCj4gPiBjb250cm9sbGVyIGFjY29yZGluZ2x5Lgo+ID4gCj4gPiBOb3cs IGV2ZW4gaWYgdGhhdCdzIHNvbWV0aGluZyBJIGNvbnNpZGVyZWQgd2hlbiBkZXNpZ25pbmcgdGhl IHNwaS1tZW0KPiA+IGludGVyZmFjZSwgSSdkIGxpa2UgdG8gc3RheSBhd2F5IGZyb20gdGhpcyBz b3J0IG9mIHNwZWNpYWxpemF0aW9uIGFzCj4gPiBsb25nIGFzIHBvc3NpYmxlLiBXaHk/IFNpbXBs eSBiZWNhdXNlIGRlYWxpbmcgd2l0aCBtZW1vcnkgc3BlY2lmaWNpdGllcwo+ID4gbGlrZSAiaXMg aXQgYSBOT1IsIGEgTkFORCBvciBhbiBTUkFNPyBTaG91bGQgSSBlcmFzZSBibG9ja3MgYmVmb3Jl Cj4gPiB3cml0aW5nIGRhdGE/IFdoYXQncyB0aGUgcGFnZSBzaXplLCBzZWN0b3Igc2l6ZSwgZXJh c2VibG9jayBzaXplPyAuLi4iCj4gPiBpcyBub3Qgc29tZXRoaW5nIHRoYXQgYmVsb25ncyBpbiB0 aGUgU1BJIGZyYW1ld29yay4gSU1ITywgaXQgc2hvdWxkCj4gPiBzdGF5IGluIFNQSSBtZW0gZHJp dmVycyAodGhlIFNQSSBOT1Igb3IgU1BJIE5BTkQgZnJhbWV3b3JrIGFyZSBzdWNoIFNQSQo+ID4g bWVtIGRyaXZlcnMpLgo+ID4gCj4gPiBUaGlzIGJlaW5nIHNhaWQsIEkgc2VlIGEgcmVhbCBuZWVk IGZvciBhZHZhbmNlZCBmZWF0dXJlcy4gT25lIGV4YW1wbGUgSQo+ID4gaGF2ZSBpbiBtaW5kIGlz IGEgImRpcmVjdC1tYXBwaW5nIEFQSSIsIHdoZXJlIGEgc3BpX21lbSB1c2VyIGNvdWxkIGFzawo+ ID4gZm9yIGEgc3BlY2lmaWMgcmVnaW9uIG9mIHRoZSBtZW1vcnkgdG8gYmUgZGlyZWN0bHkgbWFw cGVkIChpZiB0aGUKPiA+IGZlYXR1cmUgaXMgc3VwcG9ydGVkIGJ5IHRoZSBjb250cm9sbGVyIG9m IGNvdXJzZSkuIEFuZCB0aGF0J3Mgc29tZXRoaW5nCj4gPiBJIHRoaW5rIHdlIGNhbiBtYWtlIGdl bmVyaWMgZW5vdWdoIHRvIGNvbnNpZGVyIGFkZGluZyBpdCB0byB0aGUKPiA+IHNwaV9tZW1fb3Bz IGludGVyZmFjZS4gQWxsIHdlJ2xsIG5lZWQgaXMgYSB3YXkgdG8gc2F5ICJJIHdhbnQgdG8gbWFw Cj4gPiB0aGlzIHBvcnRpb24gb2YgdGhlIG1lbW9yeSBpbiBSLCBXIG9yIFJXIGFuZCB3aGVuIHlv dSBuZWVkIHRvCj4gPiByZWFkL3dyaXRlIHVzZSB0aGlzIHNwaV9tZW1fb3AgYW5kIHBhdGNoIHRo ZSBhZGRyZXNzIGJhc2VkIG9uIHRoZQo+ID4gYWN0dWFsIG1lbW9yeSBhZGRyZXNzIHRoYXQgaXMg YmVpbmcgYWNjZXNzZWQiLgo+ID4gIAo+IAo+IAo+IE1hbnkgb2YgdGhlIFNQSSBOT1IgY29udHJv bGxlcnMsIGVzcGVjaWFsbHkgdGhlIG9uZXMgdGhhdCBzdXBwb3J0IGRpcmVjdAo+IG1hcHBpbmcg YXJlIHNtYXJ0IGFuZCBuZWVkIG1vcmUgZmxhc2ggc3BlY2lmaWMgZGF0YS4gRm9yIGV4YW1wbGUs Cj4gY2FkZW5jZS1xdWFkc3BpIG5lZWRzIHRvIGtub3cgcGFnZXNpemUgYXMgdGhpcyBjb250cm9s bGVyIGF1dG9tYXRpY2FsbHkKPiBzZW5kcyB3cml0ZSBlbmFibGUgd2hlbiB3cml0ZXMgY3Jvc3Mg cGFnZSBib3VuZGFyeS4gSSBndWVzcywgc3VjaAo+IGNvbnRyb2xsZXJzIHBvc2UgYSBwcm9ibGVt IHRvIHNwaV9tZW1fb3BzIGluIHBhc3Npbmcgc3BpX25vciBpbnRlcm5hbAo+IGRhdGEgdG8gZHJp dmVycy4gT3Igc3VjaCBjb250cm9sbGVycyBtYXkgbmVlZCB0byBiZSBjb250aW51ZWQgdG8gYmUK PiBzdXBwb3J0ZWQgZGlyZWN0bHkgdW5kZXIgc3BpLW5vciBmcmFtZXdvcms/CgpJdCdzIG5vdCBk ZWNpZGVkIHlldC4gSSdtIG5vdCBjbG9zaW5nIHRoZSBkb29yIHRvIGFueSBvZiB0aG9zZSBhZHZh bmNlZApjb250cm9sbGVycyB5ZXQsIGp1c3QgbmVlZCB0byBoYXZlIGEgY2xvc2VyIGxvb2sgYmVm b3JlIHRha2luZyBhCmRlY2lzaW9uLiBUbyBtZSwgaXQgc291bmRzIGxpa2UgaW5mb3JtYXRpb24g dGhhdCBtaWdodCBiZSBuZWVkZWQgdG8KaGFuZGxlIGRpcmVjdCBtYXBwaW5nIGluIHdyaXRlIG1v ZGUgZm9yIFNQSSBOT1JzIGFuZCB3ZSdyZSBub3QgdGhlcmUKeWV0LgoKPiAKPiBJIGFtIG9rYXkg d2l0aCB0aGlzIHNlcmllcyBpbiBnZW5lcmFsLiBCdXQsIHdhcyB0cnlpbmcgdG8gdW5kZXJzdGFu ZAo+IHdoaWNoIGRyaXZlcnMgd2lsbCBmYWxsIHVuZGVyIHNwaV9tZW0gYW5kIHdoaWNoIHdpbGwg Y29udGludWUgdG8gcmVtYWluCj4gdW5kZXIgbXRkL3NwaS1ub3IKCklkZWFsbHksIGFsbCBvZiB0 aGVtLCBidXQgbWF5YmUgbm90IHdpdGggYWxsIHRoZSBhZHZhbmNlZCBmZWF0dXJlcwpzdXBwb3J0 ZWQgYXQgdGhlIGJlZ2lubmluZy4gVGhlIG9ubHkgdGhpbmdzIEknZCBsaWtlIHRvIGtlZXAgb3V0 c2lkZSBvZgpkcml2ZXJzL3NwaSBhcmUgY29udHJvbGxlcnMgdGhhdCBhcmUgb25seSBhYmxlIHRv IHNlbmQgY29tbWFuZHMgZm9yIGEKc3BlY2lmaWMgdHlwZSBvZiBtZW1vcnkgKGxpa2Ugb25seSBj b21tYW5kcyB0byBhZGRyZXNzIFNQSSBOT1JzKS4KCkknbSBmaW5lIGRpc2N1c3NpbmcgYWxsIG9m IHRoaXMsIHdpdGggcmVhbCB1c2UgY2FzZXMgdG8gZXhwbGFpbiB3aHkgd2UKbmVlZCB0byBleHBv c2UgdGhlIG1lbW9yeSB0eXBlLCBhbmQgdGhlIG1lbW9yeSBvcmdhbml6YXRpb24gaW5mb3JtYXRp b24KdG8gU1BJIGNvbnRyb2xsZXIgZHJpdmVycywgYnV0IEknZCBwcmVmZXIgdG8gZG8gdGhhdCBp biBhIHNlcGFyYXRlCnRocmVhZCBpZiB5b3UgZG9uJ3QgbWluZC4KClJlZ2FyZHMsCgpCb3JpcwoK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkxp bnV4IE1URCBkaXNjdXNzaW9uIG1haWxpbmcgbGlzdApodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LW10ZC8K From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.bootlin.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1f8nuM-0002X7-B3 for linux-mtd@lists.infradead.org; Wed, 18 Apr 2018 14:18:09 +0000 Date: Wed, 18 Apr 2018 16:17:47 +0200 From: Boris Brezillon To: Vignesh R Cc: Yogesh Gaur , Kamal Dasu , Richard Weinberger , Miquel Raynal , "linux-spi@vger.kernel.org" , Peter Pan , Marek Vasut , Frieder Schrempf , Mark Brown , "linux-mtd@lists.infradead.org" , Cyrille Pitchen , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Maxime Chevallier , Brian Norris , David Woodhouse Subject: Re: [PATCH v2 04/10] spi: Extend the core to ease integration of SPI memory controllers Message-ID: <20180418161747.7060ae3b@bbrezillon> In-Reply-To: <991f6a90-2211-acb9-c384-1d49d5468b31@ti.com> References: <20180410224439.9260-1-boris.brezillon@bootlin.com> <20180410224439.9260-5-boris.brezillon@bootlin.com> <60f3d53a-9668-9e1c-9f29-45aaf4004630@ti.com> <20180412171005.5cfaba58@bbrezillon> <20180412215951.751bf889@bbrezillon> <991f6a90-2211-acb9-c384-1d49d5468b31@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi Vignesh, On Tue, 17 Apr 2018 09:42:18 +0530 Vignesh R wrote: > >> =20 > >> >=C2=A0=C2=A0 =20 > >> > > + } addr; > >> > > + > >> > > + struct { > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 u8 nbytes; > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 u8 buswidth; > >> > > + } dummy; > >> > > + > >> > > + struct { > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 u8 buswidth; > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 enum spi_mem_dat= a_dir dir; > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 unsigned int nby= tes; > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* buf.{in,out} = must be DMA-able. */ > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 union { > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 void *in; > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 const void *out; > >> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } buf; > >> > > + } data; > >> > > +}; > >> > > +=C2=A0=C2=A0=C2=A0 =20 > >> >=20 > >> > Some flash devices support Dual/Quad DDR (Double Data Rate) mode and= the > >> > SPI controller driver would need to know this information. We will n= eed > >> > to add a field for that.=C2=A0 =20 > >>=20 > >> Well, let's wait until we actually need that. > >> =20 > >> >=20 > >> > Currently, there are drivers under mtd/spi-nor/ that need to know > >> > page/sector/total size of flash memory(info available in > >> > -`struct spi_nor). We would need a way to provide this info to spi_m= em > >> > drivers, if we ever plan to move drivers under mtd/spi-nor to spi/= =C2=A0 =20 > >>=20 > >> Again, we'll see when we'll try to move them, but I hope still we won't > >> need that. Looks like the kind of information I'd like to keep away > >> from spi controller drivers. =20 > >=20 > > Let me clarify this part. I already thought a bit about this problem, > > and that's the very reason we have an intermediate layer with a spi_mem > > struct pointing to the real spi_device object. The idea is to add new > > fields to spi_mem object if/when we really have to. We'd also have to > > add ->attach/detach() methods to spi_mem_ops so that SPI mem controller > > can know when a new device is about to be accessed by a spi-mem > > driver, can parse the information provided in spi_mem and configure the > > controller accordingly. > >=20 > > Now, even if that's something I considered when designing the spi-mem > > interface, I'd like to stay away from this sort of specialization as > > long as possible. Why? Simply because dealing with memory specificities > > like "is it a NOR, a NAND or an SRAM? Should I erase blocks before > > writing data? What's the page size, sector size, eraseblock size? ..." > > is not something that belongs in the SPI framework. IMHO, it should > > stay in SPI mem drivers (the SPI NOR or SPI NAND framework are such SPI > > mem drivers). > >=20 > > This being said, I see a real need for advanced features. One example I > > have in mind is a "direct-mapping API", where a spi_mem user could ask > > for a specific region of the memory to be directly mapped (if the > > feature is supported by the controller of course). And that's something > > I think we can make generic enough to consider adding it to the > > spi_mem_ops interface. All we'll need is a way to say "I want to map > > this portion of the memory in R, W or RW and when you need to > > read/write use this spi_mem_op and patch the address based on the > > actual memory address that is being accessed". > > =20 >=20 >=20 > Many of the SPI NOR controllers, especially the ones that support direct > mapping are smart and need more flash specific data. For example, > cadence-quadspi needs to know pagesize as this controller automatically > sends write enable when writes cross page boundary. I guess, such > controllers pose a problem to spi_mem_ops in passing spi_nor internal > data to drivers. Or such controllers may need to be continued to be > supported directly under spi-nor framework? It's not decided yet. I'm not closing the door to any of those advanced controllers yet, just need to have a closer look before taking a decision. To me, it sounds like information that might be needed to handle direct mapping in write mode for SPI NORs and we're not there yet. >=20 > I am okay with this series in general. But, was trying to understand > which drivers will fall under spi_mem and which will continue to remain > under mtd/spi-nor Ideally, all of them, but maybe not with all the advanced features supported at the beginning. The only things I'd like to keep outside of drivers/spi are controllers that are only able to send commands for a specific type of memory (like only commands to address SPI NORs). I'm fine discussing all of this, with real use cases to explain why we need to expose the memory type, and the memory organization information to SPI controller drivers, but I'd prefer to do that in a separate thread if you don't mind. Regards, Boris