From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cbitw-0003fG-Cr for linux-mtd@lists.infradead.org; Thu, 09 Feb 2017 07:12:22 +0000 Date: Thu, 9 Feb 2017 08:11:55 +0100 From: Boris Brezillon To: Brian Norris Cc: =?UTF-8?B?Q8OpZHJpYw==?= Le Goater , linux-mtd@lists.infradead.org, David Woodhouse , Marek Vasut , Richard Weinberger , Cyrille Pitchen , devicetree@vger.kernel.org, Rob Herring , Mark Rutland Subject: Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property Message-ID: <20170209081155.48aba234@bbrezillon> In-Reply-To: <20170208223859.GK94627@google.com> References: <1485368255-12038-1-git-send-email-clg@kaod.org> <1485368255-12038-2-git-send-email-clg@kaod.org> <20170208223859.GK94627@google.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: , On Wed, 8 Feb 2017 14:38:59 -0800 Brian Norris wrote: > On Wed, Jan 25, 2017 at 07:17:34PM +0100, C=C3=A9dric Le Goater wrote: > > This can be used to easily identify a specific chip on a system with > > multiple chips. > >=20 > > Drivers wanting to support this new label property will benefit from > > it without a change. They might want to check in the future that > > mtd->name is NULL before assigning a default name to the mtd device. > > Other drivers will keep the current behavior, which is to override > > mtd->name with their own value. > >=20 > > Suggested-by: Boris Brezillon > > Signed-off-by: C=C3=A9dric Le Goater > > --- > >=20 > > Changes since v1: > >=20 > > - moved the use of the "label" property from mtd_set_dev_defaults() > > to mtd_set_of_node() to let drivers keep control on how mtd->name > > is set and allocated. > >=20 > > include/linux/mtd/mtd.h | 2 ++ > > 1 file changed, 2 insertions(+) > >=20 > > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > > index 13f8052b9ff9..f4fe15517295 100644 > > --- a/include/linux/mtd/mtd.h > > +++ b/include/linux/mtd/mtd.h > > @@ -24,6 +24,7 @@ > > #include > > #include > > #include > > +#include > > =20 > > #include > > =20 > > @@ -385,6 +386,7 @@ static inline void mtd_set_of_node(struct mtd_info = *mtd, > > struct device_node *np) > > { > > mtd->dev.of_node =3D np; > > + of_property_read_string(np, "label", &mtd->name); =20 >=20 > Seems like this could be done only if '!mtd->name'? >=20 > As it stands, you're still clobbering some names, e.g. ones from > physmap_of.c. Indeed. I thought all users of mtd_set_of_node()/nand_set_flash_node()/spi_nor_set_flash_node() were calling this function before initializing mtd->name, which is apparently not true. > Notably, this driver already supports a "linux,mtd-name" > (which your new property should probably supersede), but it seems like > you're breaking compatibility. Well, we're not really breaking things until someone starts defining a 'label' property under the flash node ;-). Moreover, if we want to deprecate 'linux,mtd-name' in favor of 'label', we should definitely override the 'linux,mtd-name' value with the 'label' one if someone defined both (which is bad ;-)). Anyway, I agree, we'd better check if !mtd->name before assigning the label name. Cedric, can you send a v3? From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH v2 1/2] mtd: name the mtd device with an optional label property Date: Thu, 9 Feb 2017 08:11:55 +0100 Message-ID: <20170209081155.48aba234@bbrezillon> References: <1485368255-12038-1-git-send-email-clg@kaod.org> <1485368255-12038-2-git-send-email-clg@kaod.org> <20170208223859.GK94627@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170208223859.GK94627@google.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 To: Brian Norris Cc: Mark Rutland , devicetree@vger.kernel.org, Richard Weinberger , Marek Vasut , Rob Herring , linux-mtd@lists.infradead.org, =?UTF-8?B?Q8OpZHJpYw==?= Le Goater , Cyrille Pitchen , David Woodhouse List-Id: devicetree@vger.kernel.org T24gV2VkLCA4IEZlYiAyMDE3IDE0OjM4OjU5IC0wODAwCkJyaWFuIE5vcnJpcyA8Y29tcHV0ZXJz Zm9ycGVhY2VAZ21haWwuY29tPiB3cm90ZToKCj4gT24gV2VkLCBKYW4gMjUsIDIwMTcgYXQgMDc6 MTc6MzRQTSArMDEwMCwgQ8OpZHJpYyBMZSBHb2F0ZXIgd3JvdGU6Cj4gPiBUaGlzIGNhbiBiZSB1 c2VkIHRvIGVhc2lseSBpZGVudGlmeSBhIHNwZWNpZmljIGNoaXAgb24gYSBzeXN0ZW0gd2l0aAo+ ID4gbXVsdGlwbGUgY2hpcHMuCj4gPiAKPiA+IERyaXZlcnMgd2FudGluZyB0byBzdXBwb3J0IHRo aXMgbmV3IGxhYmVsIHByb3BlcnR5IHdpbGwgYmVuZWZpdCBmcm9tCj4gPiBpdCB3aXRob3V0IGEg Y2hhbmdlLiBUaGV5IG1pZ2h0IHdhbnQgdG8gY2hlY2sgaW4gdGhlIGZ1dHVyZSB0aGF0Cj4gPiBt dGQtPm5hbWUgaXMgTlVMTCBiZWZvcmUgYXNzaWduaW5nIGEgZGVmYXVsdCBuYW1lIHRvIHRoZSBt dGQgZGV2aWNlLgo+ID4gT3RoZXIgZHJpdmVycyB3aWxsIGtlZXAgdGhlIGN1cnJlbnQgYmVoYXZp b3IsIHdoaWNoIGlzIHRvIG92ZXJyaWRlCj4gPiBtdGQtPm5hbWUgd2l0aCB0aGVpciBvd24gdmFs dWUuCj4gPiAKPiA+IFN1Z2dlc3RlZC1ieTogQm9yaXMgQnJlemlsbG9uIDxib3Jpcy5icmV6aWxs b25AZnJlZS1lbGVjdHJvbnMuY29tPgo+ID4gU2lnbmVkLW9mZi1ieTogQ8OpZHJpYyBMZSBHb2F0 ZXIgPGNsZ0BrYW9kLm9yZz4KPiA+IC0tLQo+ID4gCj4gPiAgQ2hhbmdlcyBzaW5jZSB2MToKPiA+ IAo+ID4gIC0gbW92ZWQgdGhlIHVzZSBvZiB0aGUgImxhYmVsIiBwcm9wZXJ0eSBmcm9tIG10ZF9z ZXRfZGV2X2RlZmF1bHRzKCkKPiA+ICAgIHRvIG10ZF9zZXRfb2Zfbm9kZSgpIHRvIGxldCBkcml2 ZXJzIGtlZXAgY29udHJvbCBvbiBob3cgbXRkLT5uYW1lCj4gPiAgICBpcyBzZXQgYW5kIGFsbG9j YXRlZC4KPiA+IAo+ID4gIGluY2x1ZGUvbGludXgvbXRkL210ZC5oIHwgMiArKwo+ID4gIDEgZmls ZSBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKykKPiA+IAo+ID4gZGlmZiAtLWdpdCBhL2luY2x1ZGUv bGludXgvbXRkL210ZC5oIGIvaW5jbHVkZS9saW51eC9tdGQvbXRkLmgKPiA+IGluZGV4IDEzZjgw NTJiOWZmOS4uZjRmZTE1NTE3Mjk1IDEwMDY0NAo+ID4gLS0tIGEvaW5jbHVkZS9saW51eC9tdGQv bXRkLmgKPiA+ICsrKyBiL2luY2x1ZGUvbGludXgvbXRkL210ZC5oCj4gPiBAQCAtMjQsNiArMjQs NyBAQAo+ID4gICNpbmNsdWRlIDxsaW51eC91aW8uaD4KPiA+ICAjaW5jbHVkZSA8bGludXgvbm90 aWZpZXIuaD4KPiA+ICAjaW5jbHVkZSA8bGludXgvZGV2aWNlLmg+Cj4gPiArI2luY2x1ZGUgPGxp bnV4L29mLmg+Cj4gPiAgCj4gPiAgI2luY2x1ZGUgPG10ZC9tdGQtYWJpLmg+Cj4gPiAgCj4gPiBA QCAtMzg1LDYgKzM4Niw3IEBAIHN0YXRpYyBpbmxpbmUgdm9pZCBtdGRfc2V0X29mX25vZGUoc3Ry dWN0IG10ZF9pbmZvICptdGQsCj4gPiAgCQkJCSAgIHN0cnVjdCBkZXZpY2Vfbm9kZSAqbnApCj4g PiAgewo+ID4gIAltdGQtPmRldi5vZl9ub2RlID0gbnA7Cj4gPiArCW9mX3Byb3BlcnR5X3JlYWRf c3RyaW5nKG5wLCAibGFiZWwiLCAmbXRkLT5uYW1lKTsgIAo+IAo+IFNlZW1zIGxpa2UgdGhpcyBj b3VsZCBiZSBkb25lIG9ubHkgaWYgJyFtdGQtPm5hbWUnPwo+IAo+IEFzIGl0IHN0YW5kcywgeW91 J3JlIHN0aWxsIGNsb2JiZXJpbmcgc29tZSBuYW1lcywgZS5nLiBvbmVzIGZyb20KPiBwaHlzbWFw X29mLmMuCgpJbmRlZWQuIEkgdGhvdWdodCBhbGwgdXNlcnMgb2YKbXRkX3NldF9vZl9ub2RlKCkv bmFuZF9zZXRfZmxhc2hfbm9kZSgpL3NwaV9ub3Jfc2V0X2ZsYXNoX25vZGUoKSB3ZXJlCmNhbGxp bmcgdGhpcyBmdW5jdGlvbiBiZWZvcmUgaW5pdGlhbGl6aW5nIG10ZC0+bmFtZSwgd2hpY2ggaXMK YXBwYXJlbnRseSBub3QgdHJ1ZS4KCj4gTm90YWJseSwgdGhpcyBkcml2ZXIgYWxyZWFkeSBzdXBw b3J0cyBhICJsaW51eCxtdGQtbmFtZSIKPiAod2hpY2ggeW91ciBuZXcgcHJvcGVydHkgc2hvdWxk IHByb2JhYmx5IHN1cGVyc2VkZSksIGJ1dCBpdCBzZWVtcyBsaWtlCj4geW91J3JlIGJyZWFraW5n IGNvbXBhdGliaWxpdHkuCgpXZWxsLCB3ZSdyZSBub3QgcmVhbGx5IGJyZWFraW5nIHRoaW5ncyB1 bnRpbCBzb21lb25lIHN0YXJ0cyBkZWZpbmluZyBhCidsYWJlbCcgcHJvcGVydHkgdW5kZXIgdGhl IGZsYXNoIG5vZGUgOy0pLiBNb3Jlb3ZlciwgaWYgd2Ugd2FudCB0bwpkZXByZWNhdGUgJ2xpbnV4 LG10ZC1uYW1lJyBpbiBmYXZvciBvZiAnbGFiZWwnLCB3ZSBzaG91bGQgZGVmaW5pdGVseQpvdmVy cmlkZSB0aGUgJ2xpbnV4LG10ZC1uYW1lJyB2YWx1ZSB3aXRoIHRoZSAnbGFiZWwnIG9uZSBpZiBz b21lb25lCmRlZmluZWQgYm90aCAod2hpY2ggaXMgYmFkIDstKSkuCgpBbnl3YXksIEkgYWdyZWUs IHdlJ2QgYmV0dGVyIGNoZWNrIGlmICFtdGQtPm5hbWUgYmVmb3JlIGFzc2lnbmluZyB0aGUKbGFi ZWwgbmFtZS4KQ2VkcmljLCBjYW4geW91IHNlbmQgYSB2Mz8KCgoKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkxpbnV4IE1URCBkaXNjdXNzaW9u IG1haWxpbmcgbGlzdApodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZv L2xpbnV4LW10ZC8K