All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>
Cc: <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: [PATCH 2/8] mtd: rawnand: Let callers use the bare Hamming helpers
Date: Wed, 29 Sep 2021 00:15:01 +0200	[thread overview]
Message-ID: <20210928221507.199198-3-miquel.raynal@bootlin.com> (raw)
In-Reply-To: <20210928221507.199198-1-miquel.raynal@bootlin.com>

Before the introduction of the ECC framework infrastructure, many
drivers used the ->calculate/correct() Hamming helpers directly. The
point of this framework was to avoid this kind of hackish calls and use a
proper and generic API but it is true that in certain cases, drivers
still need to use these helpers in order to do ECC computations on
behalf of their limited hardware.

Right after the introduction of the ECC engine core introduction, it was
spotted that it was not possible to use the shiny rawnand software ECC
helpers so easily because an ECC engine object should have been
allocated and initialized first. While this works well in most cases,
for these drivers just leveraging the power of a single helper in
conjunction with some pretty old and limited hardware, it did not fit.

The idea back then was to declare intermediate helpers which would make
use of the exported software ECC engine bare functions while keeping the
rawnand layer compatibility. As there was already functions with the
rawnand_sw_hamming_ prefix it was decided to declare new local helpers
for this purpose in each driver needing one.

Besides being far from optimal, this design choice was blamed by Linus
when he pulled the "fixes" pull request [1] so that is why now it is
time to clean this mess up.

Enhancing the implementation of the rawnand_ecc_sw_* helpers to support
both cases, when the ECC object is instantiated and when it is not is a
quite elegant way to solve this situation. This way, we can still use
the existing and exported rawnand helpers while avoiding the need for
each driver to declare its own helper.

Following this change, most of the fixes sent in [2] can now be safely
reverted. Only the fsmc fix will need to be kept because there is
actually something specific to the driver to do in its ->correct()
helper.

[1] https://lore.kernel.org/lkml/CAHk-=wh_ZHF685Fni8V9is17mj=pFisUaZ_0=gq6nbK+ZcyQmg@mail.gmail.com/
[2] https://lore.kernel.org/linux-mtd/20210413161840.345208-1-miquel.raynal@bootlin.com/

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc-sw-hamming.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/ecc-sw-hamming.c b/drivers/mtd/nand/ecc-sw-hamming.c
index a7655b668f32..254db2e7f8bb 100644
--- a/drivers/mtd/nand/ecc-sw-hamming.c
+++ b/drivers/mtd/nand/ecc-sw-hamming.c
@@ -364,9 +364,9 @@ int nand_ecc_sw_hamming_calculate(struct nand_device *nand,
 {
 	struct nand_ecc_sw_hamming_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned int step_size = nand->ecc.ctx.conf.step_size;
+	bool sm_order = engine_conf ? engine_conf->sm_order : false;
 
-	return ecc_sw_hamming_calculate(buf, step_size, code,
-					engine_conf->sm_order);
+	return ecc_sw_hamming_calculate(buf, step_size, code, sm_order);
 }
 EXPORT_SYMBOL(nand_ecc_sw_hamming_calculate);
 
@@ -457,9 +457,10 @@ int nand_ecc_sw_hamming_correct(struct nand_device *nand, unsigned char *buf,
 {
 	struct nand_ecc_sw_hamming_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned int step_size = nand->ecc.ctx.conf.step_size;
+	bool sm_order = engine_conf ? engine_conf->sm_order : false;
 
 	return ecc_sw_hamming_correct(buf, read_ecc, calc_ecc, step_size,
-				      engine_conf->sm_order);
+				      sm_order);
 }
 EXPORT_SYMBOL(nand_ecc_sw_hamming_correct);
 
-- 
2.27.0


WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>
Cc: <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: [PATCH 2/8] mtd: rawnand: Let callers use the bare Hamming helpers
Date: Wed, 29 Sep 2021 00:15:01 +0200	[thread overview]
Message-ID: <20210928221507.199198-3-miquel.raynal@bootlin.com> (raw)
In-Reply-To: <20210928221507.199198-1-miquel.raynal@bootlin.com>

Before the introduction of the ECC framework infrastructure, many
drivers used the ->calculate/correct() Hamming helpers directly. The
point of this framework was to avoid this kind of hackish calls and use a
proper and generic API but it is true that in certain cases, drivers
still need to use these helpers in order to do ECC computations on
behalf of their limited hardware.

Right after the introduction of the ECC engine core introduction, it was
spotted that it was not possible to use the shiny rawnand software ECC
helpers so easily because an ECC engine object should have been
allocated and initialized first. While this works well in most cases,
for these drivers just leveraging the power of a single helper in
conjunction with some pretty old and limited hardware, it did not fit.

The idea back then was to declare intermediate helpers which would make
use of the exported software ECC engine bare functions while keeping the
rawnand layer compatibility. As there was already functions with the
rawnand_sw_hamming_ prefix it was decided to declare new local helpers
for this purpose in each driver needing one.

Besides being far from optimal, this design choice was blamed by Linus
when he pulled the "fixes" pull request [1] so that is why now it is
time to clean this mess up.

Enhancing the implementation of the rawnand_ecc_sw_* helpers to support
both cases, when the ECC object is instantiated and when it is not is a
quite elegant way to solve this situation. This way, we can still use
the existing and exported rawnand helpers while avoiding the need for
each driver to declare its own helper.

Following this change, most of the fixes sent in [2] can now be safely
reverted. Only the fsmc fix will need to be kept because there is
actually something specific to the driver to do in its ->correct()
helper.

[1] https://lore.kernel.org/lkml/CAHk-=wh_ZHF685Fni8V9is17mj=pFisUaZ_0=gq6nbK+ZcyQmg@mail.gmail.com/
[2] https://lore.kernel.org/linux-mtd/20210413161840.345208-1-miquel.raynal@bootlin.com/

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc-sw-hamming.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/ecc-sw-hamming.c b/drivers/mtd/nand/ecc-sw-hamming.c
index a7655b668f32..254db2e7f8bb 100644
--- a/drivers/mtd/nand/ecc-sw-hamming.c
+++ b/drivers/mtd/nand/ecc-sw-hamming.c
@@ -364,9 +364,9 @@ int nand_ecc_sw_hamming_calculate(struct nand_device *nand,
 {
 	struct nand_ecc_sw_hamming_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned int step_size = nand->ecc.ctx.conf.step_size;
+	bool sm_order = engine_conf ? engine_conf->sm_order : false;
 
-	return ecc_sw_hamming_calculate(buf, step_size, code,
-					engine_conf->sm_order);
+	return ecc_sw_hamming_calculate(buf, step_size, code, sm_order);
 }
 EXPORT_SYMBOL(nand_ecc_sw_hamming_calculate);
 
@@ -457,9 +457,10 @@ int nand_ecc_sw_hamming_correct(struct nand_device *nand, unsigned char *buf,
 {
 	struct nand_ecc_sw_hamming_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned int step_size = nand->ecc.ctx.conf.step_size;
+	bool sm_order = engine_conf ? engine_conf->sm_order : false;
 
 	return ecc_sw_hamming_correct(buf, read_ecc, calc_ecc, step_size,
-				      engine_conf->sm_order);
+				      sm_order);
 }
 EXPORT_SYMBOL(nand_ecc_sw_hamming_correct);
 
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>
Cc: <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: [PATCH 2/8] mtd: rawnand: Let callers use the bare Hamming helpers
Date: Wed, 29 Sep 2021 00:15:01 +0200	[thread overview]
Message-ID: <20210928221507.199198-3-miquel.raynal@bootlin.com> (raw)
In-Reply-To: <20210928221507.199198-1-miquel.raynal@bootlin.com>

Before the introduction of the ECC framework infrastructure, many
drivers used the ->calculate/correct() Hamming helpers directly. The
point of this framework was to avoid this kind of hackish calls and use a
proper and generic API but it is true that in certain cases, drivers
still need to use these helpers in order to do ECC computations on
behalf of their limited hardware.

Right after the introduction of the ECC engine core introduction, it was
spotted that it was not possible to use the shiny rawnand software ECC
helpers so easily because an ECC engine object should have been
allocated and initialized first. While this works well in most cases,
for these drivers just leveraging the power of a single helper in
conjunction with some pretty old and limited hardware, it did not fit.

The idea back then was to declare intermediate helpers which would make
use of the exported software ECC engine bare functions while keeping the
rawnand layer compatibility. As there was already functions with the
rawnand_sw_hamming_ prefix it was decided to declare new local helpers
for this purpose in each driver needing one.

Besides being far from optimal, this design choice was blamed by Linus
when he pulled the "fixes" pull request [1] so that is why now it is
time to clean this mess up.

Enhancing the implementation of the rawnand_ecc_sw_* helpers to support
both cases, when the ECC object is instantiated and when it is not is a
quite elegant way to solve this situation. This way, we can still use
the existing and exported rawnand helpers while avoiding the need for
each driver to declare its own helper.

Following this change, most of the fixes sent in [2] can now be safely
reverted. Only the fsmc fix will need to be kept because there is
actually something specific to the driver to do in its ->correct()
helper.

[1] https://lore.kernel.org/lkml/CAHk-=wh_ZHF685Fni8V9is17mj=pFisUaZ_0=gq6nbK+ZcyQmg@mail.gmail.com/
[2] https://lore.kernel.org/linux-mtd/20210413161840.345208-1-miquel.raynal@bootlin.com/

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc-sw-hamming.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/ecc-sw-hamming.c b/drivers/mtd/nand/ecc-sw-hamming.c
index a7655b668f32..254db2e7f8bb 100644
--- a/drivers/mtd/nand/ecc-sw-hamming.c
+++ b/drivers/mtd/nand/ecc-sw-hamming.c
@@ -364,9 +364,9 @@ int nand_ecc_sw_hamming_calculate(struct nand_device *nand,
 {
 	struct nand_ecc_sw_hamming_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned int step_size = nand->ecc.ctx.conf.step_size;
+	bool sm_order = engine_conf ? engine_conf->sm_order : false;
 
-	return ecc_sw_hamming_calculate(buf, step_size, code,
-					engine_conf->sm_order);
+	return ecc_sw_hamming_calculate(buf, step_size, code, sm_order);
 }
 EXPORT_SYMBOL(nand_ecc_sw_hamming_calculate);
 
@@ -457,9 +457,10 @@ int nand_ecc_sw_hamming_correct(struct nand_device *nand, unsigned char *buf,
 {
 	struct nand_ecc_sw_hamming_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned int step_size = nand->ecc.ctx.conf.step_size;
+	bool sm_order = engine_conf ? engine_conf->sm_order : false;
 
 	return ecc_sw_hamming_correct(buf, read_ecc, calc_ecc, step_size,
-				      engine_conf->sm_order);
+				      sm_order);
 }
 EXPORT_SYMBOL(nand_ecc_sw_hamming_correct);
 
-- 
2.27.0


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

  parent reply	other threads:[~2021-09-28 22:15 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28 22:14 [PATCH 0/8] Cleanup series about Hamming helpers Miquel Raynal
2021-09-28 22:14 ` Miquel Raynal
2021-09-28 22:14 ` Miquel Raynal
2021-09-28 22:15 ` [PATCH 1/8] mtd: rawnand: fsmc: Fix use of SM ORDER Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:33   ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-09-28 22:15 ` Miquel Raynal [this message]
2021-09-28 22:15   ` [PATCH 2/8] mtd: rawnand: Let callers use the bare Hamming helpers Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:33   ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-09-28 22:15 ` [PATCH 3/8] Revert "mtd: rawnand: txx9ndfmc: Fix external use of SW Hamming ECC helper" Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:33   ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-09-28 22:15 ` [PATCH 4/8] Revert "mtd: rawnand: tmio: " Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:33   ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-10-15 10:33     ` Miquel Raynal
2021-09-28 22:15 ` [PATCH 5/8] Revert "mtd: rawnand: sharpsl: " Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:32   ` Miquel Raynal
2021-10-15 10:32     ` Miquel Raynal
2021-10-15 10:32     ` Miquel Raynal
2021-09-28 22:15 ` [PATCH 6/8] Revert "mtd: rawnand: ndfc: " Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:32   ` Miquel Raynal
2021-10-15 10:32     ` Miquel Raynal
2021-10-15 10:32     ` Miquel Raynal
2021-09-28 22:15 ` [PATCH 7/8] Revert "mtd: rawnand: lpc32xx_slc: " Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:32   ` Miquel Raynal
2021-10-15 10:32     ` Miquel Raynal
2021-10-15 10:32     ` Miquel Raynal
2021-09-28 22:15 ` [PATCH 8/8] Revert "mtd: rawnand: cs553x: " Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-09-28 22:15   ` Miquel Raynal
2021-10-15 10:32   ` Miquel Raynal
2021-10-15 10:32     ` Miquel Raynal
2021-10-15 10:32     ` 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=20210928221507.199198-3-miquel.raynal@bootlin.com \
    --to=miquel.raynal@bootlin.com \
    --cc=Tudor.Ambarus@microchip.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    --cc=vz@mleia.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
Be 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.