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 1gH8Tx-00062H-UP for linux-mtd@lists.infradead.org; Mon, 29 Oct 2018 14:25:32 +0000 Date: Mon, 29 Oct 2018 15:25:08 +0100 From: Miquel Raynal To: Boris Brezillon Cc: Richard Weinberger , linux-mtd@lists.infradead.org, David Woodhouse , Brian Norris , Marek Vasut , Han Xu , Masahiro Yamada , Tudor Ambarus , Harvey Hunt , Xiaolei Li , Maxim Levitsky , Marc Gonzalez , Stefan Agner Subject: Re: [PATCH 06/15] mtd: rawnand: Add nand_[de]select_target() helpers Message-ID: <20181029152508.253216c9@xps13> In-Reply-To: <20181029151634.76c86cf8@bbrezillon> References: <20181023185011.3356-1-boris.brezillon@bootlin.com> <20181023185011.3356-7-boris.brezillon@bootlin.com> <20181029143647.2efe60cb@xps13> <20181029143924.7ed5dc4f@xps13> <20181029145727.773c8118@bbrezillon> <20181029150641.7ef0f92e@xps13> <20181029151634.76c86cf8@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Boris Brezillon wrote on Mon, 29 Oct 2018 15:16:34 +0100: > On Mon, 29 Oct 2018 15:06:41 +0100 > Miquel Raynal wrote: > > > Boris Brezillon wrote on Mon, 29 Oct 2018 > > 14:57:27 +0100: > > > > > On Mon, 29 Oct 2018 14:39:24 +0100 > > > Miquel Raynal wrote: > > > > > > > Miquel Raynal wrote on Mon, 29 Oct 2018 > > > > 14:36:47 +0100: > > > > > > > > > Hi Boris, > > > > > > > > > > Boris Brezillon wrote on Tue, 23 Oct 2018 > > > > > 20:50:02 +0200: > > > > > > > > > > > Add a wrapper to prevent drivers and core code from directly calling > > > > > > the ->select_chip hook which we are about to deprecate. > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > --- > > > > > > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 23 +++-- > > > > > > drivers/mtd/nand/raw/jz4740_nand.c | 4 +- > > > > > > drivers/mtd/nand/raw/nand_base.c | 114 ++++++++++++++------- > > > > > > drivers/mtd/nand/raw/r852.c | 4 +- > > > > > > include/linux/mtd/rawnand.h | 4 + > > > > > > 5 files changed, 98 insertions(+), 51 deletions(-) > > > > > > > > > > > > > > > > So far I am glad to see all these changes. > > > > > > > > > > About the ->select_chip() removal, I wonder if it would not be better > > > > > to also change the local variables "chipnr" or "chip_number" (or > > > > > even "i") that suggest that this ID selects a chip, while it > > > > > actually selects a die in a chip (and it is possible to have multiple > > > > > die on a chip, so multiple CS for one single NAND chip). > > > > > > I agree. > > > > > > > > > > > > > Do you think it is worth the change ? If yes, would it fit in this patch > > > > > or is it better to do this change elsewhere? > > > > > > > > > > > > > This request actually applies to the following patches as well. Maybe we > > > > could even find a uniform way to name it, "die_nr" or something like > > > > this? > > > > > > Target is the name used in the ONFI spec, hence the function names. > > > Note that die is not accurate since you might have several dies exposed > > > through a single CS line (that's called LUNs). > > > > I'm completely fine with the rename of the functions being now > > _select_target(). > > > > What about renaming local variables -when relevant- as lun_nr? lun? > > lun_idx? > > Nope, a LUN is a logically selection-able die, while a target is a > physically selection-able one. We should rename the vars/args target or > cs, not lun. Ok then, let's try 'target' to avoid the confusion with cs -> chip select -> NAND chip select > > > > > Do you think it should be done in place in these patches or as a > > separate change? > > I think it should be done separately. Ok.