Received: by 2002:a25:1985:0:0:0:0:0 with SMTP id 127csp3225133ybz; Mon, 27 Apr 2020 12:12:22 -0700 (PDT) X-Google-Smtp-Source: APiQypLbsz6TtIczFMXKGfhDCykEOD8ChRLk7gNkuiyMiEm+xBZ2wI2X2CdPFdPUziY5Bx9xWU0f X-Received: by 2002:a17:906:edc2:: with SMTP id sb2mr19736669ejb.129.1588014742025; Mon, 27 Apr 2020 12:12:22 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1588014742; cv=none; d=google.com; s=arc-20160816; b=boFPDpFEv09aK0taRr/X2u3O3wfWVjr3XghU6OJN7b7AkUW9fj1mmDHM7Rn6zIrOj0 BU4HGY7myPzAMxJfvQ72ITlNzUMgD9yyFDwIwJjRiE+zewEeVmrTMAuRjI3HZjzkXldd lqMCaq2in8rIqb0bYdY5guF2eYGzb5TXFI8+uMDWox0ju9dXXJCO8NqJAWtqa+Z9p+dM sihlfgOgUQ78GxZcDKMhETJ7D/Qh5nX8IJ/H7OJeQPFV7l4o1bwInYuMghPcmTDwFcMt WUexewmssxtpRP/7+6sb+XmiUq60GS1omfdWJDOy2TNpTxF3wOsG8VZLY5I0erV0pn3G i5PQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :date; bh=YoxiBhTYbClBpymU/u41JY6CMSGyS27m1vq7QeMlN64=; b=ib8YguzgHkOKCsOgsF/ccq7AzWT1MAzh/LJcdFzz2IR9fCcqJSqpMqdHN1pCW7qIyC XE4UPNpG2bEsKvGpJWSrRvb/LB9cdd05gyW5IOuaH9Ry7lla9qg9hCNDwUnDElfConfR rdXA7ibYgwk+49n/zgva3d71btROloMHdW+p0pF3ET0VW1PcikPIWR5AK4QKKOWwW+UK aceuHWIWda2MnMtPeSQdiJB+WAGvXyf3DOlN95NF/D9gTy9PRfVVDnnJjwsNyrkzglOX QFac3b4YuTtLzq2puaxXUvovYj8bW0TZNvi15MeunJirhhvCTGsUnA6fmaBGQBirGCmN J8Vg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=collabora.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id c3si273215eja.251.2020.04.27.12.11.58; Mon, 27 Apr 2020 12:12:22 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=collabora.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726633AbgD0TKb (ORCPT + 99 others); Mon, 27 Apr 2020 15:10:31 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:40642 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726364AbgD0TKb (ORCPT ); Mon, 27 Apr 2020 15:10:31 -0400 Received: from localhost (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id BF9392A05B6; Mon, 27 Apr 2020 20:10:28 +0100 (BST) Date: Mon, 27 Apr 2020 21:10:24 +0200 From: Boris Brezillon To: Miquel Raynal Cc: Ricardo Ribalda Delgado , Richard Weinberger , Vignesh Raghavendra , linux-mtd@lists.infradead.org, LKML , Tudor Ambarus Subject: Re: [PATCH v2] mtd: Fix mtd not the same name not registered if nvmem Message-ID: <20200427211024.174f5830@collabora.com> In-Reply-To: <20200427164922.5829717f@xps13> References: <20200401100240.445447-1-ribalda@kernel.org> <20200402065953.9974-1-ribalda@kernel.org> <20200427162222.1c2b2c85@xps13> <20200427163711.07614619@collabora.com> <20200427164922.5829717f@xps13> Organization: Collabora X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 27 Apr 2020 16:49:22 +0200 Miquel Raynal wrote: > Hi Boris, > > Boris Brezillon wrote on Mon, 27 Apr > 2020 16:37:11 +0200: > > > On Mon, 27 Apr 2020 16:22:22 +0200 > > Miquel Raynal wrote: > > > > > Hi Ricardo, > > > > > > Ricardo Ribalda Delgado wrote on Tue, 14 Apr 2020 > > > 15:47:23 +0200: > > > > > > > Ping? > > > > > > > > On Thu, Apr 2, 2020 at 8:59 AM Ricardo Ribalda Delgado > > > > wrote: > > > > > > > > > > When the nvmem framework is enabled, a nvmem device is created per mtd > > > > > device/partition. > > > > > > > > > > It is not uncommon that a device can have multiple mtd devices with > > > > > partitions that have the same name. Eg, when there DT overlay is allowed > > > > > and the same device with mtd is attached twice. > > > > > > > > > > Under that circumstances, the mtd fails to register due to a name > > > > > duplication on the nvmem framework. > > > > > > > > > > With this patch we add a _1, _2, _X to the subsequent names if there is > > > > > a collition, and throw a warning, instead of not starting the mtd > > > > > device. > > > > > > > > > > [ 8.948991] sysfs: cannot create duplicate filename '/bus/nvmem/devices/Production Data' > > > > > [ 8.948992] CPU: 7 PID: 246 Comm: systemd-udevd Not tainted 5.5.0-qtec-standard #13 > > > > > [ 8.948993] Hardware name: AMD Dibbler/Dibbler, BIOS 05.22.04.0019 10/26/2019 > > > > > [ 8.948994] Call Trace: > > > > > [ 8.948996] dump_stack+0x50/0x70 > > > > > [ 8.948998] sysfs_warn_dup.cold+0x17/0x2d > > > > > [ 8.949000] sysfs_do_create_link_sd.isra.0+0xc2/0xd0 > > > > > [ 8.949002] bus_add_device+0x74/0x140 > > > > > [ 8.949004] device_add+0x34b/0x850 > > > > > [ 8.949006] nvmem_register.part.0+0x1bf/0x640 > > > > > ... > > > > > [ 8.948926] mtd mtd8: Failed to register NVMEM device > > > > > > > > > > Signed-off-by: Ricardo Ribalda Delgado > > > > > > Thanks for proposing this change. Indeed we are aware of the problem > > > and the best solution that we could come up with was to create an > > > additional "unique_name" field to the mtd_info structure. This new > > > field would have the form: > > > > > > [] > > > > > > The separator might be '~' (but I am completely open on that), and that > > > would give for instance: > > > > > > my-controller~my-device~my-part~mysub-part > > > > I'd prefer something slightly more standard for the separator, like '/', > > which is what we usually use when we want to represent a path in a tree. > > I do agree on the general approach though. > > I am not sure / is a valid separator here we would use this > name to create a sysfs entry. Would it work? Hm, you're probably right, I didn't check how the name was used by nvmem. I also see that partition names can contain spaces, which is not that great. So, if we want to use mtd->unique_name we should probably sanitize it before passing it to nvmem. All this makes me reconsider your initial proposal: use 'mtdX' as the nvmem name. It's unique and it's simple enough to not require this extra sanitization pass, on the other hand, guessing the nvmem partition based on such an opaque name is not simple, not to mention that the mtd device name can change depending on the probe order. I also wonder if creating nvmem devs unconditionally for all mtd devices is a good idea. Sounds like we should only do that if there's an explicit request to have one partition exposed as an nvmem. Note that, no matter what we decide about nvmem, I think having unique names at the MTD level is a good thing.