From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Subject: Re: [PATCH v5 1/3] mtd: nand: don't select chip in nand_chip's block_bad op Date: Thu, 7 Jan 2016 09:57:23 +0530 Message-ID: <568DE92B.7040402@codeaurora.org> References: <1439959746-25498-1-git-send-email-architt@codeaurora.org> <1451971501-18160-1-git-send-email-architt@codeaurora.org> <1451971501-18160-2-git-send-email-architt@codeaurora.org> <20160106170532.12e48ce9@bbrezillon> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:37685 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751272AbcAGE1e (ORCPT ); Wed, 6 Jan 2016 23:27:34 -0500 In-Reply-To: <20160106170532.12e48ce9@bbrezillon> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Boris Brezillon Cc: linux-arm-msm@vger.kernel.org, cernekee@gmail.com, sboyd@codeaurora.org, linux-mtd@lists.infradead.org, dehrenberg@google.com, andy.gross@linaro.org, computersforpeace@gmail.com On 01/06/2016 09:35 PM, Boris Brezillon wrote: > On Tue, 5 Jan 2016 10:54:59 +0530 > Archit Taneja wrote: > >> One of the arguments passed to struct nand_chip's block_bad op is >> 'getchip', which, if true, is supposed to get and select the nand device, >> and later unselect and release the device. >> >> This op is intended to be replaceable by drivers. The drivers shouldn't >> be responsible for selecting/unselecting chip. Like other ops, the chip >> should already be selected before the block_bad op is called. >> >> Remove the getchip argument from the block_bad op and move the chip >> selection to nand_block_checkbad (the only user of chip->block_bad). >> Modify nand_block_bad (the default function for the op) such that >> it doesn't select the chip. > > I would go even further and move the chip selection into > nand_block_isbad(), who is the only caller that requires this chip > selection. You're right. The only other place where nand_block_checkbad is used is nand_erase_nand, and that already selects the chip before. I'll update the patch. Thanks, Archit > >> >> Signed-off-by: Archit Taneja >> --- >> drivers/mtd/nand/nand_base.c | 41 +++++++++++++++++++++++------------------ >> include/linux/mtd/nand.h | 2 +- >> 2 files changed, 24 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index 928081b..42aa441 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -317,9 +317,9 @@ static void nand_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len) >> * >> * Check, if the block is bad. >> */ >> -static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip) >> +static int nand_block_bad(struct mtd_info *mtd, loff_t ofs) >> { >> - int page, chipnr, res = 0, i = 0; >> + int page, res = 0, i = 0; >> struct nand_chip *chip = mtd_to_nand(mtd); >> u16 bad; >> >> @@ -328,15 +328,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip) >> >> page = (int)(ofs >> chip->page_shift) & chip->pagemask; >> >> - if (getchip) { >> - chipnr = (int)(ofs >> chip->chip_shift); >> - >> - nand_get_device(mtd, FL_READING); >> - >> - /* Select the NAND device */ >> - chip->select_chip(mtd, chipnr); >> - } >> - >> do { >> if (chip->options & NAND_BUSWIDTH_16) { >> chip->cmdfunc(mtd, NAND_CMD_READOOB, >> @@ -361,11 +352,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip) >> i++; >> } while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)); >> >> - if (getchip) { >> - chip->select_chip(mtd, -1); >> - nand_release_device(mtd); >> - } >> - >> return res; >> } >> >> @@ -514,8 +500,27 @@ static int nand_block_checkbad(struct mtd_info *mtd, loff_t ofs, int getchip, >> { >> struct nand_chip *chip = mtd_to_nand(mtd); >> >> - if (!chip->bbt) >> - return chip->block_bad(mtd, ofs, getchip); >> + if (!chip->bbt) { >> + int ret; >> + >> + if (getchip) { >> + int chipnr = (int)(ofs >> chip->chip_shift); >> + >> + nand_get_device(mtd, FL_READING); >> + >> + /* Select the NAND device */ >> + chip->select_chip(mtd, chipnr); >> + } >> + >> + ret = chip->block_bad(mtd, ofs); >> + >> + if (getchip) { >> + chip->select_chip(mtd, -1); >> + nand_release_device(mtd); >> + } >> + >> + return ret; >> + } >> >> /* Return info from the table */ >> return nand_isbad_bbt(mtd, ofs, allowbbt); >> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h >> index 3e92be1..c15818e 100644 >> --- a/include/linux/mtd/nand.h >> +++ b/include/linux/mtd/nand.h >> @@ -650,7 +650,7 @@ struct nand_chip { >> void (*write_buf)(struct mtd_info *mtd, const uint8_t *buf, int len); >> void (*read_buf)(struct mtd_info *mtd, uint8_t *buf, int len); >> void (*select_chip)(struct mtd_info *mtd, int chip); >> - int (*block_bad)(struct mtd_info *mtd, loff_t ofs, int getchip); >> + int (*block_bad)(struct mtd_info *mtd, loff_t ofs); >> int (*block_markbad)(struct mtd_info *mtd, loff_t ofs); >> void (*cmd_ctrl)(struct mtd_info *mtd, int dat, unsigned int ctrl); >> int (*dev_ready)(struct mtd_info *mtd); > > > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation