Hi Boris,

Boris Brezillon [off-list ref] wrote on Thu, 21 Feb 2019
12:16:25 +0100:

On Thu, 21 Feb 2019 11:01:52 +0100
Miquel Raynal [off-list ref] wrote:

quoted

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 30f0fb02abe2..9e3b018d0b83 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h

@@ -82,8 +82,14 @@ struct nand_pos {
 	unsigned int page;
 };
 
+enum nand_page_io_req_type {
+	NAND_PAGE_READ = 0,
+	NAND_PAGE_WRITE,
+};
+
 /**
  * struct nand_page_io_req - NAND I/O request object
+ * @type: the type of page I/O: read or write
  * @pos: the position this I/O request is targeting
  * @dataoffs: the offset within the page
  * @datalen: number of data bytes to read from/write to this page

@@ -99,6 +105,7 @@ struct nand_pos {
  * specific commands/operations.
  */
 struct nand_page_io_req {
+	enum nand_page_io_req_type type;  

Can you add the reqtype enum and type field (+ patch the iterator
helpers) in a separate patch?

Sure.

quoted

 	struct nand_pos pos;
 	unsigned int dataoffs;
 	unsigned int datalen;

@@ -116,13 +123,35 @@ struct nand_page_io_req {
 };
 
 /**
- * struct nand_ecc_req - NAND ECC requirements
+ * struct nand_ecc_conf - NAND ECC configuration
+ * @strength: ECC strength
+ * @step_size: Number of bytes per step
+ * @total: Total number of bytes used for storing ECC codes, this is used by
+ *         generic OOB layouts
+ */
+struct nand_ecc_conf {  

Please do the s/nand_ecc_req/nand_ecc_conf/ in a separate patch.

Ok.

quoted

+	unsigned int strength;
+	unsigned int step_size;
+	unsigned int total;  

Do we really need to add this total field here? Looks like something
that should be kept private to the ECC engine implementation.

It was initially private, but I realized it was needed by generic parts
(including for instance the generic OOB layouts) so it could not be
made private.

I just moved the 'total' entry out of the struct nand_ecc_conf and
moved it in the struct nand_ecc_ctx. It is still public but not in the
very generic "conf" structure anymore.

quoted

+};
+
+/**
+ * struct nand_ecc_user_conf - User desired ECC configuration
+ * @mode: ECC mode
+ * @algo: ECC algorithm
  * @strength: ECC strength
  * @step_size: ECC step/block size
+ * @maximize: ECC parameters must be maximized depending on the device
+ *            capabilities
+ * @flags: User flags
  */
-struct nand_ecc_req {
+struct nand_ecc_user_conf {
+	int mode;  

We should definitely name that one differently ('provider' maybe).

Changed to provider.

quoted

+	unsigned int algo;
 	unsigned int strength;
 	unsigned int step_size;
+	unsigned int maximize;
+	unsigned int flags;  

maximize could be a flag.

It is now.

quoted

 };
 
 #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }

@@ -157,11 +186,76 @@ struct nand_ops {
 	bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos);
 };  

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel