From mboxrd@z Thu Jan 1 00:00:00 1970 From: xiaolei li Subject: Re: [PATCH v4 3/4] mtd: nand: mediatek: add support for different MTK NAND FLASH Controller IP Date: Wed, 31 May 2017 14:52:41 +0800 Message-ID: <1496213561.492.12.camel@mhfsdcap03> References: <1496201877-34373-1-git-send-email-xiaolei.li@mediatek.com> <1496201877-34373-4-git-send-email-xiaolei.li@mediatek.com> <20170531081241.2137fe38@bbrezillon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170531081241.2137fe38@bbrezillon> 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 To: Boris Brezillon Cc: srv_heupstream@mediatek.com, yt.shen@mediatek.com, robh+dt@kernel.org, linux-mtd@lists.infradead.org, matthias.bgg@gmail.com, linux-mediatek@lists.infradead.org, computersforpeace@gmail.com, dwmw2@infradead.org, rogercc.lin@mediatek.com List-Id: linux-mediatek@lists.infradead.org SGkgQm9yaXMsCgpPbiBXZWQsIDIwMTctMDUtMzEgYXQgMDg6MTIgKzAyMDAsIEJvcmlzIEJyZXpp bGxvbiB3cm90ZToKPiBMZSBXZWQsIDMxIE1heSAyMDE3IDExOjM3OjU2ICswODAwLAo+IFhpYW9s ZWkgTGkgPHhpYW9sZWkubGlAbWVkaWF0ZWsuY29tPiBhIMOpY3JpdCA6Cj4gCj4gPiAgCj4gPiAt c3RhdGljIHZvaWQgbXRrX25mY19zZXRfc3BhcmVfcGVyX3NlY3Rvcih1MzIgKnNwcywgc3RydWN0 IG10ZF9pbmZvICptdGQpCj4gPiArc3RhdGljIGludCBtdGtfbmZjX3NldF9zcGFyZV9wZXJfc2Vj dG9yKHUzMiAqc3BzLCBzdHJ1Y3QgbXRkX2luZm8gKm10ZCkKPiAKPiBXaHkgZG8geW91IGNoYW5n ZSB0aGUgcHJvdG90eXBlIGhlcmU/IFlvdSBzZWVtIHRvIGFsd2F5cyByZXR1cm4gMAo+IGFueXdh eS4KPiAKU29ycnksIGl0IHNob3VsZCBiZSB2b2lkLgoKQnV0IGFzIHlvdXIgc3VnZ2VzdGlvbiBi ZWxvdywgSSB3aWxsIGtlZXAgdXNpbmcgaW50IGhlcmUgdG8gcmV0dXJuIGVycm9yCmlmIHRoZXJl IGlzIG5vIGVudHJ5IHRoYXQgaXMgbGVzcyB0aGFuICpzcHMuCgo+ID4gIHsKPiA+ICAJc3RydWN0 IG5hbmRfY2hpcCAqbmFuZCA9IG10ZF90b19uYW5kKG10ZCk7Cj4gPiAtCXUzMiBzcGFyZVtdID0g ezE2LCAyNiwgMjcsIDI4LCAzMiwgMzYsIDQwLCA0NCwKPiA+IC0JCQk0OCwgNDksIDUwLCA1MSwg NTIsIDYyLCA2MywgNjR9Owo+ID4gLQl1MzIgZWNjc3RlcHMsIGk7Cj4gPiArCXN0cnVjdCBtdGtf bmZjICpuZmMgPSBuYW5kX2dldF9jb250cm9sbGVyX2RhdGEobmFuZCk7Cj4gPiArCWNvbnN0IHU4 ICpzcGFyZSA9IG5mYy0+Y2Fwcy0+c3BhcmVfc2l6ZTsKPiA+ICsJdTMyIGVjY3N0ZXBzLCBpLCBq ID0gMDsKPiAKPiBDYW4gd2UgcmVuYW1lICdqJyBpbnRvICdjbG9zZXN0X3NwYXJlJz8KPiAKb2su Cgo+ID4gIAo+ID4gIAllY2NzdGVwcyA9IG10ZC0+d3JpdGVzaXplIC8gbmFuZC0+ZWNjLnNpemU7 Cj4gPiAgCSpzcHMgPSBtdGQtPm9vYnNpemUgLyBlY2NzdGVwczsKPiA+IEBAIC0xMTQ0LDI4ICsx MTAyLDI4IEBAIHN0YXRpYyB2b2lkIG10a19uZmNfc2V0X3NwYXJlX3Blcl9zZWN0b3IodTMyICpz cHMsIHN0cnVjdCBtdGRfaW5mbyAqbXRkKQo+ID4gIAlpZiAobmFuZC0+ZWNjLnNpemUgPT0gMTAy NCkKPiA+ICAJCSpzcHMgPj49IDE7Cj4gPiAgCj4gPiAtCWZvciAoaSA9IDA7IGkgPCBBUlJBWV9T SVpFKHNwYXJlKTsgaSsrKSB7Cj4gPiAtCQlpZiAoKnNwcyA8PSBzcGFyZVtpXSkgewo+ID4gLQkJ CWlmICghaSkKPiA+IC0JCQkJKnNwcyA9IHNwYXJlW2ldOwo+ID4gLQkJCWVsc2UgaWYgKCpzcHMg IT0gc3BhcmVbaV0pCj4gPiAtCQkJCSpzcHMgPSBzcGFyZVtpIC0gMV07Cj4gPiAtCQkJYnJlYWs7 Cj4gPiArCWZvciAoaSA9IDA7IGkgPCBuZmMtPmNhcHMtPm51bV9zcGFyZV9zaXplOyBpKyspIHsK PiA+ICsJCWlmICgoKnNwcyA+PSBzcGFyZVtpXSkgJiYgKHNwYXJlW2ldID49IHNwYXJlW2pdKSkg ewo+IAo+IFBhcmVudGhlc2lzIGFyb3VuZCB0aGUgJ2EgPj0gYicgdGVzdHMgYXJlIHVubmVlZGVk Ogo+IAo+IAkJaWYgKCpzcHMgPj0gc3BhcmVbaV0gJiYgc3BhcmVbaV0gPj0gc3BhcmVbal0pIHsK PiAKb2suCgo+ID4gKwkJCWogPSBpOwo+ID4gKwkJCWlmICgqc3BzID09IHNwYXJlW2ldKQo+ID4g KwkJCQlicmVhazsKPiA+ICAJCX0KPiA+ICAJfQo+ID4gIAo+ID4gLQlpZiAoaSA+PSBBUlJBWV9T SVpFKHNwYXJlKSkKPiA+IC0JCSpzcHMgPSBzcGFyZVtBUlJBWV9TSVpFKHNwYXJlKSAtIDFdOwo+ IAo+IE1heWJlIHlvdSBjb3VsZCByZXR1cm4gYW4gZXJyb3IgaWYgeW91IGRpZG4ndCBmaW5kIGFu eSBlbnRyeSB0aGF0IGlzCj4gbGVzcyB0aGF0ICpzcHMgaW4gdGhlIHRhYmxlLCBidXQgSSdtIG5v dCBzdXJlIHRoaXMgY2FuIHJlYWxseSBoYXBwZW4sCj4gYW5kIHRoZSBtaW5pbXVtIHNwYXJlIHNp emUgc2VlbXMgdG8gYmUgdGhlIHNhbWUgZm9yIGFsbCBJUHMsIHRoaXMgaXMKPiBzb21ldGhpbmcg eW91IGNhbiBjaGVjayBiZWZvcmUgaXRlcmF0aW5nIG92ZXIgdGhlIGFycmF5Ogo+IAo+IAlpZiAo KnNwcyA8IE1US19ORkNfTUlOX1NQQVJFKQo+IAkJcmV0dXJuIC1FSU5WQUw7Cj4gCk9LLiBJdCBz ZWVtcyBiZXR0ZXIgdG8gY2hlY2sgd2hldGhlciB0aGVyZSBpcyBubyBlbnRyeSB0aGF0IGlzIGxl c3MgdGhhbgoqc3BzLgpXaWxsIGFkZCBNVEtfTkZDX01JTl9TUEFSRSBhbmQgY2hlY2sgaXQuCgo+ ID4gKwkqc3BzID0gc3BhcmVbal07Cj4gPiAgCj4gPiAgCWlmIChuYW5kLT5lY2Muc2l6ZSA9PSAx MDI0KQo+ID4gIAkJKnNwcyA8PD0gMTsKPiA+ICsKPiA+ICsJcmV0dXJuIDA7Cj4gPiAgfQoKVGhh bmtzLgpYaWFvbGVpCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fCkxpbnV4IE1URCBkaXNjdXNzaW9uIG1haWxpbmcgbGlzdApodHRwOi8vbGlz dHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LW10ZC8K From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <1496213561.492.12.camel@mhfsdcap03> Subject: Re: [PATCH v4 3/4] mtd: nand: mediatek: add support for different MTK NAND FLASH Controller IP From: xiaolei li To: Boris Brezillon CC: , , , , , , , , Date: Wed, 31 May 2017 14:52:41 +0800 In-Reply-To: <20170531081241.2137fe38@bbrezillon> References: <1496201877-34373-1-git-send-email-xiaolei.li@mediatek.com> <1496201877-34373-4-git-send-email-xiaolei.li@mediatek.com> <20170531081241.2137fe38@bbrezillon> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit MIME-Version: 1.0 List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi Boris, On Wed, 2017-05-31 at 08:12 +0200, Boris Brezillon wrote: > Le Wed, 31 May 2017 11:37:56 +0800, > Xiaolei Li a écrit : > > > > > -static void mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd) > > +static int mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd) > > Why do you change the prototype here? You seem to always return 0 > anyway. > Sorry, it should be void. But as your suggestion below, I will keep using int here to return error if there is no entry that is less than *sps. > > { > > struct nand_chip *nand = mtd_to_nand(mtd); > > - u32 spare[] = {16, 26, 27, 28, 32, 36, 40, 44, > > - 48, 49, 50, 51, 52, 62, 63, 64}; > > - u32 eccsteps, i; > > + struct mtk_nfc *nfc = nand_get_controller_data(nand); > > + const u8 *spare = nfc->caps->spare_size; > > + u32 eccsteps, i, j = 0; > > Can we rename 'j' into 'closest_spare'? > ok. > > > > eccsteps = mtd->writesize / nand->ecc.size; > > *sps = mtd->oobsize / eccsteps; > > @@ -1144,28 +1102,28 @@ static void mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd) > > if (nand->ecc.size == 1024) > > *sps >>= 1; > > > > - for (i = 0; i < ARRAY_SIZE(spare); i++) { > > - if (*sps <= spare[i]) { > > - if (!i) > > - *sps = spare[i]; > > - else if (*sps != spare[i]) > > - *sps = spare[i - 1]; > > - break; > > + for (i = 0; i < nfc->caps->num_spare_size; i++) { > > + if ((*sps >= spare[i]) && (spare[i] >= spare[j])) { > > Parenthesis around the 'a >= b' tests are unneeded: > > if (*sps >= spare[i] && spare[i] >= spare[j]) { > ok. > > + j = i; > > + if (*sps == spare[i]) > > + break; > > } > > } > > > > - if (i >= ARRAY_SIZE(spare)) > > - *sps = spare[ARRAY_SIZE(spare) - 1]; > > Maybe you could return an error if you didn't find any entry that is > less that *sps in the table, but I'm not sure this can really happen, > and the minimum spare size seems to be the same for all IPs, this is > something you can check before iterating over the array: > > if (*sps < MTK_NFC_MIN_SPARE) > return -EINVAL; > OK. It seems better to check whether there is no entry that is less than *sps. Will add MTK_NFC_MIN_SPARE and check it. > > + *sps = spare[j]; > > > > if (nand->ecc.size == 1024) > > *sps <<= 1; > > + > > + return 0; > > } Thanks. Xiaolei