From: Vignesh Raghavendra <vigneshr@ti.com> To: <Tudor.Ambarus@microchip.com>, <miquel.raynal@bootlin.com>, <richard@nod.at> Cc: marek.vasut@gmail.com, bbrezillon@kernel.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, tmaimon77@gmail.com Subject: Re: [PATCH v4 2/3] mtd: spi-nor: Move m25p80 code in spi-nor.c Date: Mon, 5 Aug 2019 18:01:42 +0530 Message-ID: <cdc6268d-16c7-82ad-53e0-9ec9352d0400@ti.com> (raw) In-Reply-To: <50066b1c-6c4c-4aa7-c03d-1d2876afa2c2@microchip.com> On 05/08/19 5:21 PM, Tudor.Ambarus@microchip.com wrote: > > > On 08/05/2019 02:10 PM, Vignesh Raghavendra wrote: >> External E-Mail >> >> >> >> On 05/08/19 3:55 PM, Tudor.Ambarus@microchip.com wrote: >>> >>> >>> On 08/01/2019 07:22 PM, Vignesh Raghavendra wrote: >>> >>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >>>> index e02376e1127b..866962c715b4 100644 >>>> --- a/drivers/mtd/spi-nor/spi-nor.c >>>> +++ b/drivers/mtd/spi-nor/spi-nor.c >>>> @@ -19,6 +19,7 @@ >>> > >>>> >>>> @@ -688,6 +1003,16 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) >>>> if (nor->erase) >>>> return nor->erase(nor, addr); >>>> >>>> + if (nor->spimem) { >>>> + struct spi_mem_op op = >>>> + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1), >>>> + SPI_MEM_OP_ADDR(nor->addr_width, addr, 1), >>>> + SPI_MEM_OP_NO_DUMMY, >>>> + SPI_MEM_OP_NO_DATA); >>>> + >>>> + return spi_mem_exec_op(nor->spimem, &op); >>>> + } >>>> + >>> >>> this should be done below in the function, after masking the address. >> >> Nope. It would have been true if addr been sent as part of op.data.buf.out. >> But since addr is being send as part of op.addr.val, spi-mem.c takes care of this for spi_transfer(s) >> > > Is this valid also for the controllers that implement the ctlr->mem_ops? Nope, address conversion logic is not required if ctlr->mem_ops is implemented. spi-mem drivers should be able to handle address field as is and there is no need to be converted to SPI bus order. Regards Vignesh > >>> >>> You add a double space after return in: >>>> + return nor->read_reg(nor, SPINOR_OP_RDSR2, sr2, 1); >>> >> >> Ah, Will fix >> >>> There are some checkpatch warnings that we can fix now. >>> >> >> I did see warnings like: >>> >>> WARNING: line over 80 characters >>> #1023: FILE: drivers/mtd/spi-nor/spi-nor.c:2554: >>> + SPI_MEM_OP_DATA_IN(SPI_NOR_MAX_ID_LEN, id, 1)); >> >> I think its okay to overshoot 80 char limit for better readability. > > ok > >> Let me know if you think otherwise> >>> ERROR: space required after that ',' (ctx:VxV) >>> #1270: FILE: drivers/mtd/spi-nor/spi-nor.c:4846: >>> + {"mx25l25635e"},{"mx66l51235l"}, >>> ^ >> >> This and similar warnings can be fixed, but will throw off alignment wrt previous lines. >> Therefore I let them be as is. > > ok > > Cheers,ta > -- Regards Vignesh ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply index Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-01 16:22 [PATCH v4 0/3] Merge m25p80 into spi-nor Vignesh Raghavendra 2019-08-01 16:22 ` [PATCH v4 1/3] mtd: spi-nor: always use bounce buffer for register read/writes Vignesh Raghavendra 2019-08-05 9:06 ` Tudor.Ambarus 2019-08-05 10:38 ` Vignesh Raghavendra 2019-08-05 11:24 ` Tudor.Ambarus 2019-08-01 16:22 ` [PATCH v4 2/3] mtd: spi-nor: Move m25p80 code in spi-nor.c Vignesh Raghavendra 2019-08-05 10:25 ` Tudor.Ambarus 2019-08-05 11:10 ` Vignesh Raghavendra 2019-08-05 11:51 ` Tudor.Ambarus 2019-08-05 12:31 ` Vignesh Raghavendra [this message] 2019-08-01 16:22 ` [PATCH v4 3/3] mtd: spi-nor: Rework hwcaps selection for the spi-mem case Vignesh Raghavendra 2019-08-05 17:45 ` Tudor.Ambarus
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=cdc6268d-16c7-82ad-53e0-9ec9352d0400@ti.com \ --to=vigneshr@ti.com \ --cc=Tudor.Ambarus@microchip.com \ --cc=bbrezillon@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=marek.vasut@gmail.com \ --cc=miquel.raynal@bootlin.com \ --cc=richard@nod.at \ --cc=tmaimon77@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-mtd Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \ linux-mtd@lists.infradead.org public-inbox-index linux-mtd Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd AGPL code for this site: git clone https://public-inbox.org/public-inbox.git