From: Miquel Raynal <miquel.raynal@bootlin.com> To: Boris Brezillon <boris.brezillon@collabora.com> Cc: Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, <devicetree@vger.kernel.org>, Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, Tudor Ambarus <Tudor.Ambarus@microchip.com>, <linux-mtd@lists.infradead.org>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Michal Simek <monstr@monstr.eu>, Naga Sureshkumar Relli <nagasure@xilinx.com> Subject: Re: [PATCH v4 7/8] mtd: rawnand: arasan: Add new Arasan NAND controller Date: Sun, 10 May 2020 10:53:54 +0200 [thread overview] Message-ID: <20200510105354.2a6725a8@xps13> (raw) In-Reply-To: <20200510104145.7d53a58b@collabora.com> Hi Boris, Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 10 May 2020 10:41:45 +0200: > On Sun, 10 May 2020 10:35:47 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > Hi Boris, > > > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 10 May > > 2020 09:02:30 +0200: > > > > > On Fri, 8 May 2020 19:13:38 +0200 > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > +static int anfc_len_to_steps(struct nand_chip *chip, unsigned int len) > > > > +{ > > > > + unsigned int steps = 1, pktsize = len; > > > > + > > > > + while (pktsize > ANFC_MAX_PKT_SIZE) { > > > > + steps *= 2; > > > > + pktsize = DIV_ROUND_UP(len, steps); > > > > + } > > > > > > > > > Same here, you shouldn't have a round_up() but instead complain if > > > "len != pkt_size * steps" > > > > > > if (len % 4) > > > return -ENOTSUPP; > > > > This is not possible, we need unaligned accesses for NAND detection. > > Duh, this really calls for a comment saying how wrong this is and how > it should be fixed (discussions we had about data size constraints and > the 'can-issue-more' flag on data_in/out instructions). Agreed, I'll add a comment there :/ > > > > > > > > > if (len < ANFC_MAX_PKT_SIZE) > > > return len; > > > > > > for (steps = 2; steps < ANFC_MAX_STEPS; steps *= 2) { > > > pkt_size = len / steps; > > > if (pkt_size <= ANFC_MAX_PKT_SIZE) > > > break; > > > } > > > > > > if (pkt_size * steps != len) > > > return -ENOTSUPP; > > > > > > return pkt_size; > > > > The rest looks fine, I will change it and also add these checks in > > ->exec_op() check_nonly path. >
WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com> To: Boris Brezillon <boris.brezillon@collabora.com> Cc: Mark Rutland <mark.rutland@arm.com>, devicetree@vger.kernel.org, Michal Simek <monstr@monstr.eu>, Vignesh Raghavendra <vigneshr@ti.com>, Tudor Ambarus <Tudor.Ambarus@microchip.com>, Richard Weinberger <richard@nod.at>, Rob Herring <robh+dt@kernel.org>, linux-mtd@lists.infradead.org, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Naga Sureshkumar Relli <nagasure@xilinx.com> Subject: Re: [PATCH v4 7/8] mtd: rawnand: arasan: Add new Arasan NAND controller Date: Sun, 10 May 2020 10:53:54 +0200 [thread overview] Message-ID: <20200510105354.2a6725a8@xps13> (raw) In-Reply-To: <20200510104145.7d53a58b@collabora.com> Hi Boris, Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 10 May 2020 10:41:45 +0200: > On Sun, 10 May 2020 10:35:47 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > Hi Boris, > > > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 10 May > > 2020 09:02:30 +0200: > > > > > On Fri, 8 May 2020 19:13:38 +0200 > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > +static int anfc_len_to_steps(struct nand_chip *chip, unsigned int len) > > > > +{ > > > > + unsigned int steps = 1, pktsize = len; > > > > + > > > > + while (pktsize > ANFC_MAX_PKT_SIZE) { > > > > + steps *= 2; > > > > + pktsize = DIV_ROUND_UP(len, steps); > > > > + } > > > > > > > > > Same here, you shouldn't have a round_up() but instead complain if > > > "len != pkt_size * steps" > > > > > > if (len % 4) > > > return -ENOTSUPP; > > > > This is not possible, we need unaligned accesses for NAND detection. > > Duh, this really calls for a comment saying how wrong this is and how > it should be fixed (discussions we had about data size constraints and > the 'can-issue-more' flag on data_in/out instructions). Agreed, I'll add a comment there :/ > > > > > > > > > if (len < ANFC_MAX_PKT_SIZE) > > > return len; > > > > > > for (steps = 2; steps < ANFC_MAX_STEPS; steps *= 2) { > > > pkt_size = len / steps; > > > if (pkt_size <= ANFC_MAX_PKT_SIZE) > > > break; > > > } > > > > > > if (pkt_size * steps != len) > > > return -ENOTSUPP; > > > > > > return pkt_size; > > > > The rest looks fine, I will change it and also add these checks in > > ->exec_op() check_nonly path. > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2020-05-10 8:54 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-08 17:13 [PATCH v4 0/8] New Arasan NAND controller driver Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-08 17:13 ` [PATCH v4 1/8] lib/bch: Rework a little bit the exported function names Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-08 17:13 ` [PATCH v4 2/8] lib/bch: Allow easy bit swapping Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-08 17:13 ` [PATCH v4 3/8] mtd: rawnand: Ensure the number of bitflips is consistent Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-08 17:13 ` [PATCH v4 4/8] mtd: rawnand: Add nand_extract_bits() Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-08 17:13 ` [PATCH v4 5/8] MAINTAINERS: Add Arasan NAND controller and bindings Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-08 17:13 ` [PATCH v4 6/8] dt-bindings: mtd: Document ARASAN NAND bindings Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-11 14:10 ` Michal Simek 2020-05-11 14:10 ` Michal Simek 2020-05-18 18:12 ` Rob Herring 2020-05-18 18:12 ` Rob Herring 2020-05-08 17:13 ` [PATCH v4 7/8] mtd: rawnand: arasan: Add new Arasan NAND controller Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal 2020-05-10 6:51 ` Boris Brezillon 2020-05-10 6:51 ` Boris Brezillon 2020-05-10 6:52 ` Boris Brezillon 2020-05-10 6:52 ` Boris Brezillon 2020-05-10 8:33 ` Miquel Raynal 2020-05-10 8:33 ` Miquel Raynal 2020-05-10 7:02 ` Boris Brezillon 2020-05-10 7:02 ` Boris Brezillon 2020-05-10 8:35 ` Miquel Raynal 2020-05-10 8:35 ` Miquel Raynal 2020-05-10 8:41 ` Boris Brezillon 2020-05-10 8:41 ` Boris Brezillon 2020-05-10 8:53 ` Miquel Raynal [this message] 2020-05-10 8:53 ` Miquel Raynal 2020-05-11 16:14 ` Miquel Raynal 2020-05-11 16:14 ` Miquel Raynal 2020-05-10 7:03 ` Boris Brezillon 2020-05-10 7:03 ` Boris Brezillon 2020-05-11 15:07 ` Miquel Raynal 2020-05-11 15:07 ` Miquel Raynal 2020-05-11 15:32 ` Boris Brezillon 2020-05-11 15:32 ` Boris Brezillon 2020-05-11 15:46 ` Miquel Raynal 2020-05-11 15:46 ` Miquel Raynal 2020-05-11 15:50 ` Miquel Raynal 2020-05-11 15:50 ` Miquel Raynal 2020-05-11 15:59 ` Boris Brezillon 2020-05-11 15:59 ` Boris Brezillon 2020-05-08 17:13 ` [PATCH v4 8/8] mtd: rawnand: arasan: Support the hardware BCH ECC engine Miquel Raynal 2020-05-08 17:13 ` Miquel Raynal
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=20200510105354.2a6725a8@xps13 \ --to=miquel.raynal@bootlin.com \ --cc=Tudor.Ambarus@microchip.com \ --cc=boris.brezillon@collabora.com \ --cc=devicetree@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=mark.rutland@arm.com \ --cc=monstr@monstr.eu \ --cc=nagasure@xilinx.com \ --cc=richard@nod.at \ --cc=robh+dt@kernel.org \ --cc=thomas.petazzoni@bootlin.com \ --cc=vigneshr@ti.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.