* [RFC PATCH 01/10] mtd: nand: omap: Use a single hardware controller instance
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count Roger Quadros
` (9 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: jg1.han, nsekhar, linux-kernel, linux-mtd, pekon, ezequiel.garcia,
javier, linux-omap, dwmw2, Roger Quadros
There is only one NAND controller (ECC generator) that needs to be
shared among multiple devices. So point nand_chip->hwcontrol to
a single omap_hw_controller instance.
This way the NAND base driver can take care of serializing access
to this single controller (via nand_chip->controller->lock)
when multiple NAND devices are present.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/mtd/nand/omap2.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f0ed92e..5b8739c 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -149,8 +149,9 @@ static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
/* oob info generated runtime depending on ecc algorithm and layout selected */
static struct nand_ecclayout omap_oobinfo;
+static struct nand_hw_control omap_hw_controller;
+
struct omap_nand_info {
- struct nand_hw_control controller;
struct omap_nand_platform_data *pdata;
struct mtd_info mtd;
struct nand_chip nand;
@@ -1649,9 +1650,6 @@ static int omap_nand_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
- spin_lock_init(&info->controller.lock);
- init_waitqueue_head(&info->controller.wq);
-
info->pdev = pdev;
info->gpmc_cs = pdata->cs;
info->reg = pdata->reg;
@@ -1672,7 +1670,13 @@ static int omap_nand_probe(struct platform_device *pdev)
info->phys_base = res->start;
- nand_chip->controller = &info->controller;
+ /*
+ * There is only one NAND controller (ECC generator) that needs to be
+ * shared among multiple devices. The NAND base driver takes care of
+ * serializing access to this single controller when multiple NAND
+ * devices are present.
+ */
+ nand_chip->hwcontrol = omap_hw_controller;
nand_chip->IO_ADDR_W = nand_chip->IO_ADDR_R;
nand_chip->cmd_ctrl = omap_hwcontrol;
--
1.8.3.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 01/10] mtd: nand: omap: Use a single hardware controller instance Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-11 7:43 ` Gupta, Pekon
2014-07-09 12:37 ` [RFC PATCH 03/10] OMAP: GPMC: Introduce APIs to access NAND control registers Roger Quadros
` (8 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: javier, pekon, ezequiel.garcia, dwmw2, jg1.han, nsekhar,
linux-mtd, linux-omap, linux-kernel, Roger Quadros
Instead of hardcoding use the pre-calculated chip->ecc.steps for
configuring number of sectors to process with the BCH algorithm.
This also avoids unnecessary access to the ECC_CONFIG register in
omap_calculate_ecc_bch().
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/mtd/nand/omap2.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 5b8739c..6f3d7cd 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1066,10 +1066,10 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
unsigned int ecc_size1, ecc_size0;
/* GPMC configurations for calculating ECC */
+ nsectors = chip->ecc.steps;
switch (ecc_opt) {
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
bch_type = 0;
- nsectors = 1;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_6;
ecc_size0 = BCH_ECC_SIZE0;
@@ -1082,7 +1082,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
break;
case OMAP_ECC_BCH4_CODE_HW:
bch_type = 0;
- nsectors = chip->ecc.steps;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_1;
ecc_size0 = BCH4R_ECC_SIZE0;
@@ -1095,7 +1094,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
break;
case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
bch_type = 1;
- nsectors = 1;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_6;
ecc_size0 = BCH_ECC_SIZE0;
@@ -1108,7 +1106,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
break;
case OMAP_ECC_BCH8_CODE_HW:
bch_type = 1;
- nsectors = chip->ecc.steps;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_1;
ecc_size0 = BCH8R_ECC_SIZE0;
@@ -1121,7 +1118,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
break;
case OMAP_ECC_BCH16_CODE_HW:
bch_type = 0x2;
- nsectors = chip->ecc.steps;
if (mode == NAND_ECC_READ) {
wr_mode = 0x01;
ecc_size0 = 52; /* ECC bits in nibbles per sector */
@@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
{
struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
mtd);
+ struct nand_chip *chip = mtd->priv;
int eccbytes = info->nand.ecc.bytes;
struct gpmc_nand_regs *gpmc_regs = &info->reg;
u8 *ecc_code;
@@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
u32 val;
int i, j;
- nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
+ nsectors = chip->ecc.steps;
for (i = 0; i < nsectors; i++) {
ecc_code = ecc_calc;
switch (info->ecc_opt) {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* RE: [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count
2014-07-09 12:37 ` [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count Roger Quadros
@ 2014-07-11 7:43 ` Gupta, Pekon
2014-07-11 10:35 ` Roger Quadros
0 siblings, 1 reply; 23+ messages in thread
From: Gupta, Pekon @ 2014-07-11 7:43 UTC (permalink / raw)
To: Quadros, Roger, tony@atomide.com, computersforpeace@gmail.com
Cc: javier@dowhile0.org, ezequiel.garcia@free-electrons.com,
dwmw2@infradead.org, jg1.han@samsung.com, Nori, Sekhar,
linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org,
linux-kernel@vger.kernel.org
>From: Quadros, Roger
>
>Instead of hardcoding use the pre-calculated chip->ecc.steps for
>configuring number of sectors to process with the BCH algorithm.
>
>This also avoids unnecessary access to the ECC_CONFIG register in
>omap_calculate_ecc_bch().
>
>Signed-off-by: Roger Quadros <rogerq@ti.com>
>---
> drivers/mtd/nand/omap2.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>index 5b8739c..6f3d7cd 100644
>--- a/drivers/mtd/nand/omap2.c
>+++ b/drivers/mtd/nand/omap2.c
>@@ -1066,10 +1066,10 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> unsigned int ecc_size1, ecc_size0;
>
> /* GPMC configurations for calculating ECC */
>+ nsectors = chip->ecc.steps;
> switch (ecc_opt) {
> case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> bch_type = 0;
>- nsectors = 1;
> if (mode == NAND_ECC_READ) {
> wr_mode = BCH_WRAPMODE_6;
> ecc_size0 = BCH_ECC_SIZE0;
>@@ -1082,7 +1082,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> break;
> case OMAP_ECC_BCH4_CODE_HW:
> bch_type = 0;
>- nsectors = chip->ecc.steps;
> if (mode == NAND_ECC_READ) {
> wr_mode = BCH_WRAPMODE_1;
> ecc_size0 = BCH4R_ECC_SIZE0;
>@@ -1095,7 +1094,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> break;
> case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> bch_type = 1;
>- nsectors = 1;
> if (mode == NAND_ECC_READ) {
> wr_mode = BCH_WRAPMODE_6;
> ecc_size0 = BCH_ECC_SIZE0;
>@@ -1108,7 +1106,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> break;
> case OMAP_ECC_BCH8_CODE_HW:
> bch_type = 1;
>- nsectors = chip->ecc.steps;
> if (mode == NAND_ECC_READ) {
> wr_mode = BCH_WRAPMODE_1;
> ecc_size0 = BCH8R_ECC_SIZE0;
>@@ -1121,7 +1118,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> break;
> case OMAP_ECC_BCH16_CODE_HW:
> bch_type = 0x2;
>- nsectors = chip->ecc.steps;
> if (mode == NAND_ECC_READ) {
> wr_mode = 0x01;
> ecc_size0 = 52; /* ECC bits in nibbles per sector */
>@@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
> {
> struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
> mtd);
>+ struct nand_chip *chip = mtd->priv;
> int eccbytes = info->nand.ecc.bytes;
> struct gpmc_nand_regs *gpmc_regs = &info->reg;
> u8 *ecc_code;
>@@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
> u32 val;
> int i, j;
>
>- nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>+ nsectors = chip->ecc.steps;
Sorry NAK.. I'm sure you are breaking something here :-)
NAND driver supports multiple ECC schemes like;
OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
OMAP_ECC_CODE_BCH4_HW
OMAP_ECC_CODE_BCH8_HW
OMAP_ECC_CODE_BCH8_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
OMAP_ECC_CODE_BCH16_HW
IIRC ..
- software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)
- hardware based ecc-schemes OMAP_ECC_CODE_BCHx_HW, perform
Reads/Write in per-page granularity (or something like this).
Therefore you have custom implementation of
chip->ecc.read_page = omap_read_page_bch()
Also if you change the configurations here, it will break the compatibility with
u-boot, so images flashed via u-boot will stop to boot in kernel and vice-versa.
I suggest, please refrain from these tweaks for now..
All these optimizations have been tested on multiple scenario so please test
all ecc-schemes before doing anything, otherwise you will end-up in
a bad loop of breaking and fixing NAND driver :-).
with regards, pekon
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count
2014-07-11 7:43 ` Gupta, Pekon
@ 2014-07-11 10:35 ` Roger Quadros
2014-07-11 11:27 ` Gupta, Pekon
0 siblings, 1 reply; 23+ messages in thread
From: Roger Quadros @ 2014-07-11 10:35 UTC (permalink / raw)
To: Gupta, Pekon, tony@atomide.com, computersforpeace@gmail.com
Cc: javier@dowhile0.org, ezequiel.garcia@free-electrons.com,
dwmw2@infradead.org, jg1.han@samsung.com, Nori, Sekhar,
linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org,
linux-kernel@vger.kernel.org
On 07/11/2014 10:43 AM, Gupta, Pekon wrote:
>> From: Quadros, Roger
>>
>> Instead of hardcoding use the pre-calculated chip->ecc.steps for
>> configuring number of sectors to process with the BCH algorithm.
>>
>> This also avoids unnecessary access to the ECC_CONFIG register in
>> omap_calculate_ecc_bch().
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> drivers/mtd/nand/omap2.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index 5b8739c..6f3d7cd 100644
>> --- a/drivers/mtd/nand/omap2.c
>> +++ b/drivers/mtd/nand/omap2.c
>> @@ -1066,10 +1066,10 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> unsigned int ecc_size1, ecc_size0;
>>
>> /* GPMC configurations for calculating ECC */
>> + nsectors = chip->ecc.steps;
>> switch (ecc_opt) {
>> case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
>> bch_type = 0;
>> - nsectors = 1;
>> if (mode == NAND_ECC_READ) {
>> wr_mode = BCH_WRAPMODE_6;
>> ecc_size0 = BCH_ECC_SIZE0;
>> @@ -1082,7 +1082,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> break;
>> case OMAP_ECC_BCH4_CODE_HW:
>> bch_type = 0;
>> - nsectors = chip->ecc.steps;
>> if (mode == NAND_ECC_READ) {
>> wr_mode = BCH_WRAPMODE_1;
>> ecc_size0 = BCH4R_ECC_SIZE0;
>> @@ -1095,7 +1094,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> break;
>> case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
>> bch_type = 1;
>> - nsectors = 1;
>> if (mode == NAND_ECC_READ) {
>> wr_mode = BCH_WRAPMODE_6;
>> ecc_size0 = BCH_ECC_SIZE0;
>> @@ -1108,7 +1106,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> break;
>> case OMAP_ECC_BCH8_CODE_HW:
>> bch_type = 1;
>> - nsectors = chip->ecc.steps;
>> if (mode == NAND_ECC_READ) {
>> wr_mode = BCH_WRAPMODE_1;
>> ecc_size0 = BCH8R_ECC_SIZE0;
>> @@ -1121,7 +1118,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> break;
>> case OMAP_ECC_BCH16_CODE_HW:
>> bch_type = 0x2;
>> - nsectors = chip->ecc.steps;
>> if (mode == NAND_ECC_READ) {
>> wr_mode = 0x01;
>> ecc_size0 = 52; /* ECC bits in nibbles per sector */
>> @@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>> {
>> struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>> mtd);
>> + struct nand_chip *chip = mtd->priv;
>> int eccbytes = info->nand.ecc.bytes;
>> struct gpmc_nand_regs *gpmc_regs = &info->reg;
>> u8 *ecc_code;
>> @@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>> u32 val;
>> int i, j;
>>
>> - nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>> + nsectors = chip->ecc.steps;
>
> Sorry NAK.. I'm sure you are breaking something here :-)
>
> NAND driver supports multiple ECC schemes like;
> OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
> OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
> OMAP_ECC_CODE_BCH4_HW
> OMAP_ECC_CODE_BCH8_HW
> OMAP_ECC_CODE_BCH8_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
> OMAP_ECC_CODE_BCH16_HW
>
> IIRC ..
> - software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
> Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)
OK. I still don't have a full understanding about the ECC schemes so to ensure we
don't break anything I can just leave nsectors as it is and probably just store a
copy of it in omap_nand_info to avoid reading it back from gpmc_ecc_config.
I still have a few questions to clarify my understanding.
The only difference between OMAP_ECC_CODE_BCHx_HW_DETECTION_SW and
OMAP_ECC_CODE_BCHx_HW is that in the former the _correction_ is done by software
and in the latter the _correction_ is done by hardware (i.e. ELM module).
In both cases the _detection_ is done by the same hardware IP via ecc.calculate(),
i.e. omap_calculate_ecc_bch().
so why should nsector be different for both in the _detection_ stage?
An I right that ecc_steps is nothing but number of sub-blocks ECC calculation and correction
needs to be done for larger pages. This is a function of ECC hw capability (chip->ecc.size)
and NAND flash capability (mtd->writesize). i.e. ecc_steps = mtd->writesize / chip->ecc.size
We hardcode chip->ecc.size to 512 for all the ECC schemes in omap_nand_probe() so
calculate and correct will always be called for 512 byte sized blocks. So when does
the usecase for nsector > 1 come in?
cheers,
-roger
^ permalink raw reply [flat|nested] 23+ messages in thread* RE: [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count
2014-07-11 10:35 ` Roger Quadros
@ 2014-07-11 11:27 ` Gupta, Pekon
2014-07-11 11:51 ` Roger Quadros
0 siblings, 1 reply; 23+ messages in thread
From: Gupta, Pekon @ 2014-07-11 11:27 UTC (permalink / raw)
To: Quadros, Roger, tony@atomide.com, computersforpeace@gmail.com
Cc: javier@dowhile0.org, ezequiel.garcia@free-electrons.com,
dwmw2@infradead.org, jg1.han@samsung.com, Nori, Sekhar,
linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org,
linux-kernel@vger.kernel.org
>From: Quadros, Roger
>>On 07/11/2014 10:43 AM, Gupta, Pekon wrote:
>>> From: Quadros, Roger
[...]
>>> @@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>*mtd,
>>> {
>>> struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>>> mtd);
>>> + struct nand_chip *chip = mtd->priv;
>>> int eccbytes = info->nand.ecc.bytes;
>>> struct gpmc_nand_regs *gpmc_regs = &info->reg;
>>> u8 *ecc_code;
>>> @@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>*mtd,
>>> u32 val;
>>> int i, j;
>>>
>>> - nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>>> + nsectors = chip->ecc.steps;
>>
>> Sorry NAK.. I'm sure you are breaking something here :-)
>>
>> NAND driver supports multiple ECC schemes like;
>> OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
>> OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
>> OMAP_ECC_CODE_BCH4_HW
>> OMAP_ECC_CODE_BCH8_HW
>> OMAP_ECC_CODE_BCH8_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
>> OMAP_ECC_CODE_BCH16_HW
>>
>> IIRC ..
>> - software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>> Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)
>
>OK. I still don't have a full understanding about the ECC schemes so to ensure we
>don't break anything I can just leave nsectors as it is and probably just store a
>copy of it in omap_nand_info to avoid reading it back from gpmc_ecc_config.
>
>I still have a few questions to clarify my understanding.
>
>The only difference between OMAP_ECC_CODE_BCHx_HW_DETECTION_SW and
>OMAP_ECC_CODE_BCHx_HW is that in the former the _correction_ is done by software
>and in the latter the _correction_ is done by hardware (i.e. ELM module).
>In both cases the _detection_ is done by the same hardware IP via ecc.calculate(),
>i.e. omap_calculate_ecc_bch().
>
>so why should nsector be different for both in the _detection_ stage?
>
Again IIRC, That is because of ELM driver.
ELM hardware engine has 8 channels with each of them having 512Bytes capacity.
Now, there can be two approaches:
(1) SECTOR_MODE: Process only one sector of 512 bytes at a time, and iterate
chip->ecc.steps times.
(2) PAGE_MODE: Process complete page at a time, enabling multiple channels
simultaneously. This improves some throughput, especially for large-page
NAND devices and MLC NAND where bit-flips are common.
As ELM uses (2)nd approach, so the GPMC also has to calculate and store
ECC for complete page at a time. That is why trace NAND driver you will find
- OMAP_ECC_CODE_BCHx_HW_DETECTION_SW use generic implementation
chip->ecc.read_page= nand_read_page_hwecc() defined in nand_base.c
whereas,
- OMAP_ECC_CODE_BCHx_HW use custom implementation
chip->ecc.read_page= omap_read_page_bch() defined in omap.c
>An I right that ecc_steps is nothing but number of sub-blocks ECC calculation and correction
>needs to be done for larger pages. This is a function of ECC hw capability (chip->ecc.size)
>and NAND flash capability (mtd->writesize). i.e. ecc_steps = mtd->writesize / chip->ecc.size
>
>We hardcode chip->ecc.size to 512 for all the ECC schemes in omap_nand_probe() so
>calculate and correct will always be called for 512 byte sized blocks. So when does
>the usecase for nsector > 1 come in?
>
Ok.. I'll try to explain above details again in probably simplified version
- OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
uses lib/bch.c (via nand_bch.c) to correct ECC. And is generic implementation
so here each sector (data chunk of ecc_size) is corrected independently.
So nsector = 1;
- OMAP_ECC_CODE_BCHx_HW
Uses ELM to correct ECC. But the way ELM driver is written today. It corrects
the whole page in single go. Not individual sectors (ecc_size chunks).
So, its doable to make it same like OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
But then you _may_ have performance penalty for new technology NAND and MLC
NAND on which bit-flips are very common.
So to keep ELM driver as it is (performance tweaked), we use different
configurations in GPMC to read complete page in single go. This is where
nsector = chip->ecc.steps;
Hope that clarifies the implementation..
Good to document this detail somewhere for OMAP drivers both (u-boot and kernel).
Any thoughts ?
with regards, pekon
>cheers,
>-roger
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count
2014-07-11 11:27 ` Gupta, Pekon
@ 2014-07-11 11:51 ` Roger Quadros
0 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-11 11:51 UTC (permalink / raw)
To: Gupta, Pekon, tony@atomide.com, computersforpeace@gmail.com
Cc: javier@dowhile0.org, ezequiel.garcia@free-electrons.com,
dwmw2@infradead.org, jg1.han@samsung.com, Nori, Sekhar,
linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org,
linux-kernel@vger.kernel.org
On 07/11/2014 02:27 PM, Gupta, Pekon wrote:
>> From: Quadros, Roger
>>> On 07/11/2014 10:43 AM, Gupta, Pekon wrote:
>>>> From: Quadros, Roger
> [...]
>
>>>> @@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>> *mtd,
>>>> {
>>>> struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>>>> mtd);
>>>> + struct nand_chip *chip = mtd->priv;
>>>> int eccbytes = info->nand.ecc.bytes;
>>>> struct gpmc_nand_regs *gpmc_regs = &info->reg;
>>>> u8 *ecc_code;
>>>> @@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>> *mtd,
>>>> u32 val;
>>>> int i, j;
>>>>
>>>> - nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>>>> + nsectors = chip->ecc.steps;
>>>
>>> Sorry NAK.. I'm sure you are breaking something here :-)
>>>
>>> NAND driver supports multiple ECC schemes like;
>>> OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
>>> OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
>>> OMAP_ECC_CODE_BCH4_HW
>>> OMAP_ECC_CODE_BCH8_HW
>>> OMAP_ECC_CODE_BCH8_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
>>> OMAP_ECC_CODE_BCH16_HW
>>>
>>> IIRC ..
>>> - software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>>> Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)
>>
>> OK. I still don't have a full understanding about the ECC schemes so to ensure we
>> don't break anything I can just leave nsectors as it is and probably just store a
>> copy of it in omap_nand_info to avoid reading it back from gpmc_ecc_config.
>>
>> I still have a few questions to clarify my understanding.
>>
>> The only difference between OMAP_ECC_CODE_BCHx_HW_DETECTION_SW and
>> OMAP_ECC_CODE_BCHx_HW is that in the former the _correction_ is done by software
>> and in the latter the _correction_ is done by hardware (i.e. ELM module).
>> In both cases the _detection_ is done by the same hardware IP via ecc.calculate(),
>> i.e. omap_calculate_ecc_bch().
>>
>> so why should nsector be different for both in the _detection_ stage?
>>
> Again IIRC, That is because of ELM driver.
> ELM hardware engine has 8 channels with each of them having 512Bytes capacity.
> Now, there can be two approaches:
> (1) SECTOR_MODE: Process only one sector of 512 bytes at a time, and iterate
> chip->ecc.steps times.
> (2) PAGE_MODE: Process complete page at a time, enabling multiple channels
> simultaneously. This improves some throughput, especially for large-page
> NAND devices and MLC NAND where bit-flips are common.
>
> As ELM uses (2)nd approach, so the GPMC also has to calculate and store
> ECC for complete page at a time. That is why trace NAND driver you will find
> - OMAP_ECC_CODE_BCHx_HW_DETECTION_SW use generic implementation
> chip->ecc.read_page= nand_read_page_hwecc() defined in nand_base.c
> whereas,
> - OMAP_ECC_CODE_BCHx_HW use custom implementation
> chip->ecc.read_page= omap_read_page_bch() defined in omap.c
>
>
>> An I right that ecc_steps is nothing but number of sub-blocks ECC calculation and correction
>> needs to be done for larger pages. This is a function of ECC hw capability (chip->ecc.size)
>> and NAND flash capability (mtd->writesize). i.e. ecc_steps = mtd->writesize / chip->ecc.size
>>
>> We hardcode chip->ecc.size to 512 for all the ECC schemes in omap_nand_probe() so
>> calculate and correct will always be called for 512 byte sized blocks. So when does
>> the usecase for nsector > 1 come in?
>>
> Ok.. I'll try to explain above details again in probably simplified version
> - OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
> uses lib/bch.c (via nand_bch.c) to correct ECC. And is generic implementation
> so here each sector (data chunk of ecc_size) is corrected independently.
> So nsector = 1;
>
> - OMAP_ECC_CODE_BCHx_HW
> Uses ELM to correct ECC. But the way ELM driver is written today. It corrects
> the whole page in single go. Not individual sectors (ecc_size chunks).
Then shouldn't chip->ecc.size be equal page size and chip->ecc.steps be equal to 1 for upto 4KB pages?
For larger pages it can be a multiple of 4KB page size. i.e. 2 for 8KB, 4 for 16KB and so on.
So nsectors is not necessarily equal to ecc.steps but equal to how many 512 byte blocks are there in
one step. i.e. [min(4096, page_size) / 512]. And it must be local to omap NAND driver.
> So, its doable to make it same like OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
> But then you _may_ have performance penalty for new technology NAND and MLC
> NAND on which bit-flips are very common.
> So to keep ELM driver as it is (performance tweaked), we use different
> configurations in GPMC to read complete page in single go. This is where
> nsector = chip->ecc.steps;
>
> Hope that clarifies the implementation..
>
> Good to document this detail somewhere for OMAP drivers both (u-boot and kernel).
> Any thoughts ?
Sure. we have the processors wiki. That should be a good place.
cheers,
-roger
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH 03/10] OMAP: GPMC: Introduce APIs to access NAND control registers
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 01/10] mtd: nand: omap: Use a single hardware controller instance Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 04/10] mtd: nand: omap: Use GPMC APIs for NAND control Roger Quadros
` (7 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: jg1.han, nsekhar, linux-kernel, linux-mtd, pekon, ezequiel.garcia,
javier, linux-omap, dwmw2, Roger Quadros
Introduce omap_gpmc_read_reg() and omap_gpmc_write_reg() so that the
NAND driver can access the required registers (only those specified in
enum omap_gpmc_reg).
The NAND driver must use these APIs instead of directly accesing the
NAND control registers as they belong to the GPMC controller's register space.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
arch/arm/mach-omap2/gpmc.c | 88 +++++++++++++++++++++++++++++++++++++++++-
include/linux/omap-gpmc-nand.h | 40 +++++++++++++++++++
2 files changed, 127 insertions(+), 1 deletion(-)
create mode 100644 include/linux/omap-gpmc-nand.h
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 2c0c281..92bbada 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -31,7 +31,7 @@
#include <linux/of_device.h>
#include <linux/mtd/nand.h>
#include <linux/pm_runtime.h>
-
+#include <linux/omap-gpmc-nand.h>
#include <linux/platform_data/mtd-nand-omap2.h>
#include <asm/mach-types.h>
@@ -167,10 +167,32 @@ static resource_size_t phys_base, mem_size;
static unsigned gpmc_capability;
static void __iomem *gpmc_base;
+struct gpmc_nand_reg {
+ void __iomem *command;
+ void __iomem *address;
+ void __iomem *data;
+};
+
+static struct gpmc_nand_reg gpmc_nand_reg_map[GPMC_CS_NUM];
+
static struct clk *gpmc_l3_clk;
static irqreturn_t gpmc_handle_irq(int irq, void *dev);
+static void gpmc_fill_nand_reg_map(void)
+{
+ int i;
+
+ for (i = 0; i < gpmc_cs_num; i++) {
+ gpmc_nand_reg_map[i].command = gpmc_base + GPMC_CS0_OFFSET +
+ GPMC_CS_NAND_COMMAND + GPMC_CS_SIZE * i;
+ gpmc_nand_reg_map[i].address = gpmc_base + GPMC_CS0_OFFSET +
+ GPMC_CS_NAND_ADDRESS + GPMC_CS_SIZE * i;
+ gpmc_nand_reg_map[i].data = gpmc_base + GPMC_CS0_OFFSET +
+ GPMC_CS_NAND_DATA + GPMC_CS_SIZE * i;
+ }
+}
+
static void gpmc_write_reg(int idx, u32 val)
{
writel_relaxed(val, gpmc_base + idx);
@@ -1720,6 +1742,7 @@ static int gpmc_probe(struct platform_device *pdev)
return rc;
}
+ gpmc_fill_nand_reg_map();
return 0;
}
@@ -1885,3 +1908,66 @@ void omap3_gpmc_restore_context(void)
}
}
}
+
+/**
+ * omap_gpmc_read_reg - Read the specified GPMC register
+ * @cs: chip select number
+ * @reg: GPMC register id
+ *
+ * Reads the specified register from the appropriate chip select region
+ * and returns the read value.
+ */
+u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg)
+{
+ if (!gpmc_dev)
+ return 0;
+
+ if (cs >= gpmc_cs_num)
+ return 0;
+
+ switch (reg) {
+ case OMAP_GPMC_STATUS:
+ return gpmc_read_reg(GPMC_STATUS);
+ case OMAP_GPMC_NAND_COMMAND:
+ case OMAP_GPMC_NAND_ADDRESS:
+ return 0; /* command & address regs can't be read */
+ case OMAP_GPMC_NAND_DATA:
+ return readb_relaxed(gpmc_nand_reg_map[cs].data);
+ default:
+ return 0;
+ }
+}
+
+/**
+ * omap_gpmc_write_reg - Write into the specified GPMC register
+ * @cs: chip select number
+ * @reg: GPMC register id
+ * @val: value to write
+ *
+ * Writes the value into the specified register in the appropriate
+ * chip select region.
+ */
+void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val)
+{
+ if (!gpmc_dev)
+ return;
+
+ if (cs >= gpmc_cs_num)
+ return;
+
+ switch (reg) {
+ case OMAP_GPMC_STATUS:
+ gpmc_write_reg(GPMC_STATUS, val);
+ case OMAP_GPMC_NAND_COMMAND:
+ writeb_relaxed(val, gpmc_nand_reg_map[cs].command);
+ break;
+ case OMAP_GPMC_NAND_ADDRESS:
+ writeb_relaxed(val, gpmc_nand_reg_map[cs].address);
+ break;
+ case OMAP_GPMC_NAND_DATA:
+ writeb_relaxed(val, gpmc_nand_reg_map[cs].data);
+ break;
+ default:
+ break;
+ }
+}
diff --git a/include/linux/omap-gpmc-nand.h b/include/linux/omap-gpmc-nand.h
new file mode 100644
index 0000000..dcb2abe
--- /dev/null
+++ b/include/linux/omap-gpmc-nand.h
@@ -0,0 +1,40 @@
+/*
+ * OMAP NAND to GPMC custom API interface
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc. - http://www.ti.com
+ * Roger Quadros <rogerq@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#ifndef _OMAP_GPMC_NAND_H_
+#define _OMAP_GPMC_NAND_H_
+
+/**
+ * Registers that need to be accessed by the OMAP NAND driver
+ */
+
+enum omap_gpmc_reg {
+ OMAP_GPMC_STATUS,
+ OMAP_GPMC_NAND_COMMAND,
+ OMAP_GPMC_NAND_ADDRESS,
+ OMAP_GPMC_NAND_DATA,
+};
+
+#ifdef CONFIG_ARCH_OMAP2PLUS
+u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
+void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
+#else
+static inline u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg)
+{
+ return 0;
+}
+
+static inline void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val)
+{
+}
+#endif
+
+#endif /* _GPMC_OMAP_H_ */
--
1.8.3.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH 04/10] mtd: nand: omap: Use GPMC APIs for NAND control
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (2 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 03/10] OMAP: GPMC: Introduce APIs to access NAND control registers Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 05/10] OMAP: GPMC: Introduce APIs for accessing Prefetch/Write-post engine Roger Quadros
` (6 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: javier, pekon, ezequiel.garcia, dwmw2, jg1.han, nsekhar,
linux-mtd, linux-omap, linux-kernel, Roger Quadros
Use the omap_gpmc_read_reg() and omap_gpmc_write_reg() APIs to
access the GPMC_STATUS, NAND_COMMAND, NAND_ADDRESS and NAND_DATA
registers from the GPMC register space.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/mtd/nand/omap2.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 6f3d7cd..f50e71d 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -24,7 +24,7 @@
#include <linux/slab.h>
#include <linux/of.h>
#include <linux/of_device.h>
-
+#include <linux/omap-gpmc-nand.h>
#include <linux/mtd/nand_bch.h>
#include <linux/platform_data/elm.h>
@@ -251,13 +251,16 @@ static void omap_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
if (cmd != NAND_CMD_NONE) {
if (ctrl & NAND_CLE)
- writeb(cmd, info->reg.gpmc_nand_command);
+ omap_gpmc_write_reg(info->gpmc_cs,
+ OMAP_GPMC_NAND_COMMAND, cmd);
else if (ctrl & NAND_ALE)
- writeb(cmd, info->reg.gpmc_nand_address);
+ omap_gpmc_write_reg(info->gpmc_cs,
+ OMAP_GPMC_NAND_ADDRESS, cmd);
else /* NAND_NCE */
- writeb(cmd, info->reg.gpmc_nand_data);
+ omap_gpmc_write_reg(info->gpmc_cs,
+ OMAP_GPMC_NAND_DATA, cmd);
}
}
@@ -291,7 +294,7 @@ static void omap_write_buf8(struct mtd_info *mtd, const u_char *buf, int len)
iowrite8(*p++, info->nand.IO_ADDR_W);
/* wait until buffer is available for write */
do {
- status = readl(info->reg.gpmc_status) &
+ status = omap_gpmc_read_reg(0, OMAP_GPMC_STATUS) &
STATUS_BUFF_EMPTY;
} while (!status);
}
@@ -329,7 +332,7 @@ static void omap_write_buf16(struct mtd_info *mtd, const u_char * buf, int len)
iowrite16(*p++, info->nand.IO_ADDR_W);
/* wait until buffer is available for write */
do {
- status = readl(info->reg.gpmc_status) &
+ status = omap_gpmc_read_reg(0, OMAP_GPMC_STATUS) &
STATUS_BUFF_EMPTY;
} while (!status);
}
@@ -1011,15 +1014,16 @@ static int omap_wait(struct mtd_info *mtd, struct nand_chip *chip)
else
timeo += msecs_to_jiffies(20);
- writeb(NAND_CMD_STATUS & 0xFF, info->reg.gpmc_nand_command);
+ omap_gpmc_write_reg(info->gpmc_cs, OMAP_GPMC_NAND_COMMAND,
+ NAND_CMD_STATUS);
while (time_before(jiffies, timeo)) {
- status = readb(info->reg.gpmc_nand_data);
+ status = omap_gpmc_read_reg(info->gpmc_cs, OMAP_GPMC_NAND_DATA);
if (status & NAND_STATUS_READY)
break;
cond_resched();
}
- status = readb(info->reg.gpmc_nand_data);
+ status = omap_gpmc_read_reg(info->gpmc_cs, OMAP_GPMC_NAND_DATA);
return status;
}
@@ -1030,10 +1034,8 @@ static int omap_wait(struct mtd_info *mtd, struct nand_chip *chip)
static int omap_dev_ready(struct mtd_info *mtd)
{
unsigned int val = 0;
- struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
- mtd);
- val = readl(info->reg.gpmc_status);
+ val = omap_gpmc_read_reg(0, OMAP_GPMC_STATUS);
if ((val & 0x100) == 0x100) {
return 1;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH 05/10] OMAP: GPMC: Introduce APIs for accessing Prefetch/Write-post engine
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (3 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 04/10] mtd: nand: omap: Use GPMC APIs for NAND control Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 06/10] mtd: nand: omap: Use GPMC APIs for accessing Prefetch engine Roger Quadros
` (5 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: jg1.han, nsekhar, linux-kernel, linux-mtd, pekon, ezequiel.garcia,
javier, linux-omap, dwmw2, Roger Quadros
Even though the Prefetch engine is meant for exclusive use by
the OMAP NAND controller, itst registers belong to the GPMC controller's
register space.
Introduce 4 APIs to access the Prefetch/Write-post engine.
int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
u32 count, int is_write);
int omap_gpmc_prefetch_stop(int cs);
u32 omap_gpmc_get_prefetch_count(void);
u32 omap_gpmc_get_prefetch_fifo_count(void);
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
arch/arm/mach-omap2/gpmc.c | 134 +++++++++++++++++++++++++++++++++++++++++
include/linux/omap-gpmc-nand.h | 30 +++++++++
2 files changed, 164 insertions(+)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 92bbada..43e2a9d 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -33,6 +33,7 @@
#include <linux/pm_runtime.h>
#include <linux/omap-gpmc-nand.h>
#include <linux/platform_data/mtd-nand-omap2.h>
+#include <linux/bitops.h>
#include <asm/mach-types.h>
@@ -114,6 +115,20 @@
#define GPMC_NR_WAITPINS 4
+/* GPMC Prefetch/Write-post Engine */
+#define GPMC_PREFETCH_CONFIG1_ENABLE_PREFETCH BIT(7)
+#define GPMC_PREFETCH_CONFIG1_DMAMODE BIT(2)
+#define GPMC_PREFETCH_CONFIG1_ACCESSMODE BIT(0)
+#define GPMC_PREFETCH_CONFIG1_CS_MASK GENMASK(26, 24)
+#define GPMC_PREFETCH_CONFIG1_CS_SHIFT 24
+#define GPMC_PREFETCH_CONFIG1_FIFOTH_MASK GENMASK(14, 8)
+#define GPMC_PREFETCH_CONFIG1_FIFOTH_SHIFT 8
+
+#define GPMC_PREFETCH_CONTROL_START BIT(0)
+
+#define GPMC_PREFETCH_STATUS_COUNT(val) (val & 0x00003fff)
+#define GPMC_PREFETCH_STATUS_FIFO_CNT(val) ((val >> 24) & 0x7F)
+
/* XXX: Only NAND irq has been considered,currently these are the only ones used
*/
#define GPMC_NR_IRQ 2
@@ -1971,3 +1986,122 @@ void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val)
break;
}
}
+
+/**
+ * omap_gpmc_prefetch_start - configures and starts the prefetch engine
+ *
+ * @cs: cs (chip select) number
+ * @fifo_th: fifo threshold to be used for read/ write
+ * @dma: dma mode enable (1) or disable (0)
+ * @count: number of bytes to be transferred
+ * @is_write: prefetch read(0) or write post(1) mode
+ *
+ * As there is a single prefetch engine that must be shared between
+ * chip selects containing NAND flashes, the function returns -EBUSY if
+ * the engine is already in use. Returns 0 on success.
+ */
+int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
+ u32 count, int is_write)
+{
+ u32 val;
+
+ if (!gpmc_dev)
+ return -ENODEV;
+
+ if (cs >= gpmc_cs_num)
+ return -EINVAL;
+
+ if (fifo_th > GPMC_PREFETCH_FIFOTHRESHOLD_MAX)
+ return -EINVAL;
+
+ /* Check if engine is already in use */
+ if (gpmc_read_reg(GPMC_PREFETCH_CONTROL))
+ return -EBUSY;
+
+ /* Set the amount of bytes to be prefetched */
+ gpmc_write_reg(GPMC_PREFETCH_CONFIG2, count);
+
+ /* Set dma/mpu mode, the prefetch read / post write and
+ * enable the engine. Set which cs is has requested for.
+ */
+ val = ((cs << GPMC_PREFETCH_CONFIG1_CS_SHIFT) |
+ GPMC_PREFETCH_CONFIG1_ENABLE_PREFETCH |
+ (GPMC_PREFETCH_CONFIG1_ACCESSMODE & is_write));
+
+ val |= (fifo_th << GPMC_PREFETCH_CONFIG1_FIFOTH_SHIFT) &
+ GPMC_PREFETCH_CONFIG1_FIFOTH_MASK;
+
+ if (dma)
+ val |= GPMC_PREFETCH_CONFIG1_DMAMODE;
+
+ gpmc_write_reg(GPMC_PREFETCH_CONFIG1, val);
+
+ /* Start the prefetch engine */
+ gpmc_write_reg(GPMC_PREFETCH_CONTROL, GPMC_PREFETCH_CONTROL_START);
+
+ return 0;
+}
+
+/**
+ * omap_gpmc_prefetch_stop - stops and disables the prefetch engine
+ *
+ * @cs: Chip select number
+ */
+int omap_gpmc_prefetch_stop(int cs)
+{
+ u32 config1;
+
+ if (!gpmc_dev)
+ return -ENODEV;
+
+ if (cs >= gpmc_cs_num)
+ return -EINVAL;
+
+ /* check if the same module/cs is trying to reset */
+ config1 = gpmc_read_reg(GPMC_PREFETCH_CONFIG1);
+ config1 &= GPMC_PREFETCH_CONFIG1_CS_MASK;
+
+ if ((config1 >> GPMC_PREFETCH_CONFIG1_CS_SHIFT) != cs)
+ return -EINVAL;
+
+ /* Stop the engine */
+ gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0);
+
+ /* Reset/disable the engine */
+ gpmc_write_reg(GPMC_PREFETCH_CONFIG1, 0);
+
+ return 0;
+}
+
+/**
+ * omap_gpmc_get_prefetch_count - Returns the number of bytes remaining to be
+ * read or written by the prefetch/write-post engine
+ */
+u32 omap_gpmc_get_prefetch_count(void)
+{
+ u32 count;
+
+ if (!gpmc_dev)
+ return 0;
+
+ count = gpmc_read_reg(GPMC_PREFETCH_STATUS);
+ count = GPMC_PREFETCH_STATUS_COUNT(count);
+ return count;
+}
+
+/**
+ * omap_gpmc_get_prefetch_fifo_count - Returns the number of bytes available
+ * to be read from the FIFO or free bytes available to be written to the FIFO
+ * by the CPU
+ */
+u32 omap_gpmc_get_prefetch_fifo_count(void)
+{
+ u32 count;
+
+ if (!gpmc_dev)
+ return 0;
+
+ count = gpmc_read_reg(GPMC_PREFETCH_STATUS);
+ count = GPMC_PREFETCH_STATUS_FIFO_CNT(count);
+ return count;
+}
diff --git a/include/linux/omap-gpmc-nand.h b/include/linux/omap-gpmc-nand.h
index dcb2abe..c445d89 100644
--- a/include/linux/omap-gpmc-nand.h
+++ b/include/linux/omap-gpmc-nand.h
@@ -26,6 +26,12 @@ enum omap_gpmc_reg {
#ifdef CONFIG_ARCH_OMAP2PLUS
u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
+
+int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
+ u32 count, int is_write);
+int omap_gpmc_prefetch_stop(int cs);
+u32 omap_gpmc_get_prefetch_count(void);
+u32 omap_gpmc_get_prefetch_fifo_count(void);
#else
static inline u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg)
{
@@ -35,6 +41,30 @@ static inline u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg)
static inline void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val)
{
}
+
+static inline int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
+ u32 count, int is_write)
+{
+ return -ENOSYS;
+}
+
+static inline int omap_gpmc_prefetch_stop(int cs)
+{
+ return -ENOSYS;
+}
+
+static inline u32 omap_gpmc_get_prefetch_count(void)
+{
+ return 0;
+}
+
+static inline u32 omap_gpmc_get_prefetch_fifo_count(void)
+{
+ return 0;
+}
#endif
+/* Prefetch/Write-post Engine */
+#define GPMC_PREFETCH_FIFOTHRESHOLD_MAX 0x40
+
#endif /* _GPMC_OMAP_H_ */
--
1.8.3.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH 06/10] mtd: nand: omap: Use GPMC APIs for accessing Prefetch engine
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (4 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 05/10] OMAP: GPMC: Introduce APIs for accessing Prefetch/Write-post engine Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 07/10] OMAP: GPMC: Introduce APIs for Configuring ECC Engine Roger Quadros
` (4 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: javier, pekon, ezequiel.garcia, dwmw2, jg1.han, nsekhar,
linux-mtd, linux-omap, linux-kernel, Roger Quadros
Don't access the Prefetch engine registers directly as they belong
to the GPMC controller's register space. Use the relevant GPMC
APIs instead.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/mtd/nand/omap2.c | 155 ++++++++++++++---------------------------------
1 file changed, 45 insertions(+), 110 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f50e71d..420ef0b 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -100,20 +100,13 @@
#define P4e_s(a) (TF(a & NAND_Ecc_P4e) << 0)
#define P4o_s(a) (TF(a & NAND_Ecc_P4o) << 1)
-#define PREFETCH_CONFIG1_CS_SHIFT 24
#define ECC_CONFIG_CS_SHIFT 1
#define CS_MASK 0x7
-#define ENABLE_PREFETCH (0x1 << 7)
-#define DMA_MPU_MODE_SHIFT 2
#define ECCSIZE0_SHIFT 12
#define ECCSIZE1_SHIFT 22
#define ECC1RESULTSIZE 0x1
#define ECCCLEAR 0x100
#define ECC1 0x1
-#define PREFETCH_FIFOTHRESHOLD_MAX 0x40
-#define PREFETCH_FIFOTHRESHOLD(val) ((val) << 8)
-#define PREFETCH_STATUS_COUNT(val) (val & 0x00003fff)
-#define PREFETCH_STATUS_FIFO_CNT(val) ((val >> 24) & 0x7F)
#define STATUS_BUFF_EMPTY 0x00000001
#define OMAP24XX_DMA_GPMC 4
@@ -177,63 +170,6 @@ struct omap_nand_info {
};
/**
- * omap_prefetch_enable - configures and starts prefetch transfer
- * @cs: cs (chip select) number
- * @fifo_th: fifo threshold to be used for read/ write
- * @dma_mode: dma mode enable (1) or disable (0)
- * @u32_count: number of bytes to be transferred
- * @is_write: prefetch read(0) or write post(1) mode
- */
-static int omap_prefetch_enable(int cs, int fifo_th, int dma_mode,
- unsigned int u32_count, int is_write, struct omap_nand_info *info)
-{
- u32 val;
-
- if (fifo_th > PREFETCH_FIFOTHRESHOLD_MAX)
- return -1;
-
- if (readl(info->reg.gpmc_prefetch_control))
- return -EBUSY;
-
- /* Set the amount of bytes to be prefetched */
- writel(u32_count, info->reg.gpmc_prefetch_config2);
-
- /* Set dma/mpu mode, the prefetch read / post write and
- * enable the engine. Set which cs is has requested for.
- */
- val = ((cs << PREFETCH_CONFIG1_CS_SHIFT) |
- PREFETCH_FIFOTHRESHOLD(fifo_th) | ENABLE_PREFETCH |
- (dma_mode << DMA_MPU_MODE_SHIFT) | (0x1 & is_write));
- writel(val, info->reg.gpmc_prefetch_config1);
-
- /* Start the prefetch engine */
- writel(0x1, info->reg.gpmc_prefetch_control);
-
- return 0;
-}
-
-/**
- * omap_prefetch_reset - disables and stops the prefetch engine
- */
-static int omap_prefetch_reset(int cs, struct omap_nand_info *info)
-{
- u32 config1;
-
- /* check if the same module/cs is trying to reset */
- config1 = readl(info->reg.gpmc_prefetch_config1);
- if (((config1 >> PREFETCH_CONFIG1_CS_SHIFT) & CS_MASK) != cs)
- return -EINVAL;
-
- /* Stop the PFPW engine */
- writel(0x0, info->reg.gpmc_prefetch_control);
-
- /* Reset/disable the PFPW engine */
- writel(0x0, info->reg.gpmc_prefetch_config1);
-
- return 0;
-}
-
-/**
* omap_hwcontrol - hardware specific access to control-lines
* @mtd: MTD device structure
* @cmd: command to device
@@ -362,26 +298,26 @@ static void omap_read_buf_pref(struct mtd_info *mtd, u_char *buf, int len)
len -= len % 4;
}
- /* configure and start prefetch transfer */
- ret = omap_prefetch_enable(info->gpmc_cs,
- PREFETCH_FIFOTHRESHOLD_MAX, 0x0, len, 0x0, info);
+ /* configure and start prefetch engine */
+ ret = omap_gpmc_prefetch_start(info->gpmc_cs,
+ GPMC_PREFETCH_FIFOTHRESHOLD_MAX,
+ false, len, false);
if (ret) {
- /* PFPW engine is busy, use cpu copy method */
+ /* prefetch engine is busy, use cpu copy method */
if (info->nand.options & NAND_BUSWIDTH_16)
omap_read_buf16(mtd, (u_char *)p, len);
else
omap_read_buf8(mtd, (u_char *)p, len);
} else {
do {
- r_count = readl(info->reg.gpmc_prefetch_status);
- r_count = PREFETCH_STATUS_FIFO_CNT(r_count);
+ r_count = omap_gpmc_get_prefetch_fifo_count();
r_count = r_count >> 2;
ioread32_rep(info->nand.IO_ADDR_R, p, r_count);
p += r_count;
len -= r_count << 2;
} while (len);
- /* disable and stop the PFPW engine */
- omap_prefetch_reset(info->gpmc_cs, info);
+ /* disable and stop the prefetch engine */
+ omap_gpmc_prefetch_stop(info->gpmc_cs);
}
}
@@ -409,35 +345,35 @@ static void omap_write_buf_pref(struct mtd_info *mtd,
len--;
}
- /* configure and start prefetch transfer */
- ret = omap_prefetch_enable(info->gpmc_cs,
- PREFETCH_FIFOTHRESHOLD_MAX, 0x0, len, 0x1, info);
+ /* configure and start posted write engine */
+ ret = omap_gpmc_prefetch_start(info->gpmc_cs,
+ GPMC_PREFETCH_FIFOTHRESHOLD_MAX,
+ false, len, true);
if (ret) {
- /* PFPW engine is busy, use cpu copy method */
+ /* posted write engine is busy, use cpu copy method */
if (info->nand.options & NAND_BUSWIDTH_16)
omap_write_buf16(mtd, (u_char *)p, len);
else
omap_write_buf8(mtd, (u_char *)p, len);
} else {
while (len) {
- w_count = readl(info->reg.gpmc_prefetch_status);
- w_count = PREFETCH_STATUS_FIFO_CNT(w_count);
+ w_count = omap_gpmc_get_prefetch_fifo_count();
w_count = w_count >> 1;
for (i = 0; (i < w_count) && len; i++, len -= 2)
iowrite16(*p++, info->nand.IO_ADDR_W);
}
- /* wait for data to flushed-out before reset the prefetch */
+
+ /* wait for data to flushed-out before resetting the engine */
tim = 0;
limit = (loops_per_jiffy *
msecs_to_jiffies(OMAP_NAND_TIMEOUT_MS));
do {
cpu_relax();
- val = readl(info->reg.gpmc_prefetch_status);
- val = PREFETCH_STATUS_COUNT(val);
+ val = omap_gpmc_get_prefetch_count();
} while (val && (tim++ < limit));
- /* disable and stop the PFPW engine */
- omap_prefetch_reset(info->gpmc_cs, info);
+ /* disable and stop the posted write engine */
+ omap_gpmc_prefetch_stop(info->gpmc_cs);
}
}
@@ -458,7 +394,7 @@ static void omap_nand_dma_callback(void *data)
* @is_write: flag for read/write operation
*/
static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
- unsigned int len, int is_write)
+ unsigned int len, bool is_write)
{
struct omap_nand_info *info = container_of(mtd,
struct omap_nand_info, mtd);
@@ -501,9 +437,10 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
tx->callback_param = &info->comp;
dmaengine_submit(tx);
- /* configure and start prefetch transfer */
- ret = omap_prefetch_enable(info->gpmc_cs,
- PREFETCH_FIFOTHRESHOLD_MAX, 0x1, len, is_write, info);
+ /* configure and start prefetch engine */
+ ret = omap_gpmc_prefetch_start(info->gpmc_cs,
+ GPMC_PREFETCH_FIFOTHRESHOLD_MAX,
+ true, len, is_write);
if (ret)
/* PFPW engine is busy, use cpu copy method */
goto out_copy_unmap;
@@ -518,12 +455,11 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
do {
cpu_relax();
- val = readl(info->reg.gpmc_prefetch_status);
- val = PREFETCH_STATUS_COUNT(val);
+ val = omap_gpmc_get_prefetch_count();
} while (val && (tim++ < limit));
- /* disable and stop the PFPW engine */
- omap_prefetch_reset(info->gpmc_cs, info);
+ /* disable and stop the prefetch engine */
+ omap_gpmc_prefetch_stop(info->gpmc_cs);
dma_unmap_sg(info->dma->device->dev, &sg, 1, dir);
return 0;
@@ -552,7 +488,7 @@ static void omap_read_buf_dma_pref(struct mtd_info *mtd, u_char *buf, int len)
omap_read_buf_pref(mtd, buf, len);
else
/* start transfer in DMA mode */
- omap_nand_dma_transfer(mtd, buf, len, 0x0);
+ omap_nand_dma_transfer(mtd, buf, len, false);
}
/**
@@ -568,7 +504,7 @@ static void omap_write_buf_dma_pref(struct mtd_info *mtd,
omap_write_buf_pref(mtd, buf, len);
else
/* start transfer in DMA mode */
- omap_nand_dma_transfer(mtd, (u_char *) buf, len, 0x1);
+ omap_nand_dma_transfer(mtd, (u_char *)buf, len, true);
}
/*
@@ -581,8 +517,7 @@ static irqreturn_t omap_nand_irq(int this_irq, void *dev)
struct omap_nand_info *info = (struct omap_nand_info *) dev;
u32 bytes;
- bytes = readl(info->reg.gpmc_prefetch_status);
- bytes = PREFETCH_STATUS_FIFO_CNT(bytes);
+ bytes = omap_gpmc_get_prefetch_fifo_count();
bytes = bytes & 0xFFFC; /* io in multiple of 4 bytes */
if (info->iomode == OMAP_NAND_IO_WRITE) { /* checks for write io */
if (this_irq == info->gpmc_irq_count)
@@ -638,11 +573,12 @@ static void omap_read_buf_irq_pref(struct mtd_info *mtd, u_char *buf, int len)
info->buf = buf;
init_completion(&info->comp);
- /* configure and start prefetch transfer */
- ret = omap_prefetch_enable(info->gpmc_cs,
- PREFETCH_FIFOTHRESHOLD_MAX/2, 0x0, len, 0x0, info);
+ /* configure and start prefetch engine */
+ ret = omap_gpmc_prefetch_start(info->gpmc_cs,
+ GPMC_PREFETCH_FIFOTHRESHOLD_MAX/2,
+ false, len, false);
if (ret)
- /* PFPW engine is busy, use cpu copy method */
+ /* prefetch engine is busy, use cpu copy method */
goto out_copy;
info->buf_len = len;
@@ -653,8 +589,8 @@ static void omap_read_buf_irq_pref(struct mtd_info *mtd, u_char *buf, int len)
/* waiting for read to complete */
wait_for_completion(&info->comp);
- /* disable and stop the PFPW engine */
- omap_prefetch_reset(info->gpmc_cs, info);
+ /* disable and stop the prefetch engine */
+ omap_gpmc_prefetch_stop(info->gpmc_cs);
return;
out_copy:
@@ -688,11 +624,11 @@ static void omap_write_buf_irq_pref(struct mtd_info *mtd,
info->buf = (u_char *) buf;
init_completion(&info->comp);
- /* configure and start prefetch transfer : size=24 */
- ret = omap_prefetch_enable(info->gpmc_cs,
- (PREFETCH_FIFOTHRESHOLD_MAX * 3) / 8, 0x0, len, 0x1, info);
+ /* configure and start posted write engine : fifo size = 24 */
+ ret = omap_gpmc_prefetch_start(info->gpmc_cs, 24,
+ false, len, true);
if (ret)
- /* PFPW engine is busy, use cpu copy method */
+ /* posted write engine is busy, use cpu copy method */
goto out_copy;
info->buf_len = len;
@@ -703,17 +639,16 @@ static void omap_write_buf_irq_pref(struct mtd_info *mtd,
/* waiting for write to complete */
wait_for_completion(&info->comp);
- /* wait for data to flushed-out before reset the prefetch */
+ /* wait for data to flushed-out before reset the engine */
tim = 0;
limit = (loops_per_jiffy * msecs_to_jiffies(OMAP_NAND_TIMEOUT_MS));
do {
- val = readl(info->reg.gpmc_prefetch_status);
- val = PREFETCH_STATUS_COUNT(val);
+ val = omap_gpmc_get_prefetch_count();
cpu_relax();
} while (val && (tim++ < limit));
- /* disable and stop the PFPW engine */
- omap_prefetch_reset(info->gpmc_cs, info);
+ /* disable and stop the posted write engine */
+ omap_gpmc_prefetch_stop(info->gpmc_cs);
return;
out_copy:
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH 07/10] OMAP: GPMC: Introduce APIs for Configuring ECC Engine
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (5 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 06/10] mtd: nand: omap: Use GPMC APIs for accessing Prefetch engine Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-11 7:54 ` Gupta, Pekon
2014-07-09 12:37 ` [RFC PATCH 08/10] OMAP: GPMC: Introduce APIs to get ECC/BCH results Roger Quadros
` (3 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: javier, pekon, ezequiel.garcia, dwmw2, jg1.han, nsekhar,
linux-mtd, linux-omap, linux-kernel, Roger Quadros
Even though the ECC/BCH engine is meant for exclusive use by
the OMAP NAND controller, the ECC/BCH registers belong
to the GPMC controller's register space
Add omap_gpmc_ecc_configure_enable() and omap_gpmc_ecc_disable()
to manage the ECC engine. OMAP NAND driver must use these APIs
instead of directly accessing the ECC Engine registers.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
arch/arm/mach-omap2/gpmc.c | 109 ++++++++++++++++++++++++++++++++++++-----
include/linux/omap-gpmc-nand.h | 25 ++++++++++
2 files changed, 121 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 43e2a9d..8befd16 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -73,19 +73,6 @@
#define GPMC_ECC_BCH_RESULT_5 0x304 /* not available on OMAP2 */
#define GPMC_ECC_BCH_RESULT_6 0x308 /* not available on OMAP2 */
-/* GPMC ECC control settings */
-#define GPMC_ECC_CTRL_ECCCLEAR 0x100
-#define GPMC_ECC_CTRL_ECCDISABLE 0x000
-#define GPMC_ECC_CTRL_ECCREG1 0x001
-#define GPMC_ECC_CTRL_ECCREG2 0x002
-#define GPMC_ECC_CTRL_ECCREG3 0x003
-#define GPMC_ECC_CTRL_ECCREG4 0x004
-#define GPMC_ECC_CTRL_ECCREG5 0x005
-#define GPMC_ECC_CTRL_ECCREG6 0x006
-#define GPMC_ECC_CTRL_ECCREG7 0x007
-#define GPMC_ECC_CTRL_ECCREG8 0x008
-#define GPMC_ECC_CTRL_ECCREG9 0x009
-
#define GPMC_CONFIG2_CSEXTRADELAY BIT(7)
#define GPMC_CONFIG3_ADVEXTRADELAY BIT(7)
#define GPMC_CONFIG4_OEEXTRADELAY BIT(7)
@@ -129,6 +116,28 @@
#define GPMC_PREFETCH_STATUS_COUNT(val) (val & 0x00003fff)
#define GPMC_PREFETCH_STATUS_FIFO_CNT(val) ((val >> 24) & 0x7F)
+/* GPMC ECC config */
+#define GPMC_ECC_CONFIG_ECCENABLE BIT(0)
+#define GPMC_ECC_CONFIG_ECCCS_MASK GENMASK(3, 1)
+#define GPMC_ECC_CONFIG_ECCCS_SHIFT 1
+#define GPMC_ECC_CONFIG_ECCNSECTOR_MASK GENMASK(6, 4)
+#define GPMC_ECC_CONFIG_ECCNSECTOR_SHIFT 4
+#define GPMC_ECC_CONFIG_ECC16B BIT(7)
+#define GPMC_ECC_CONFIG_ECCWRAPMODE_MASK GENMASK(11, 8)
+#define GPMC_ECC_CONFIG_ECCWRAPMODE_SHIFT 8
+#define GPMC_ECC_CONFIG_ECCBCHTSEL_MASK GENMASK(13, 12)
+#define GPMC_ECC_CONFIG_ECCBCHTSEL_SHIFT 12
+#define GPMC_ECC_CONFIG_ECCALGBCH BIT(16)
+
+/* GPMC ECC control */
+#define GPMC_ECC_CONTROL_ECCCLEAR BIT(8)
+#define GPMC_ECC_CONTROL_ECCPOINTER_MASK GENMASK(3, 0)
+#define GPMC_ECC_CONTROL_ECCPOINTER_SHIFT 0
+
+/* GPMC ECC size */
+#define GPMC_ECC_SIZE_ECCSIZE0_SHIFT 12
+#define GPMC_ECC_SIZE_ECCSIZE1_SHIFT 22
+
/* XXX: Only NAND irq has been considered,currently these are the only ones used
*/
#define GPMC_NR_IRQ 2
@@ -2105,3 +2114,77 @@ u32 omap_gpmc_get_prefetch_fifo_count(void)
count = GPMC_PREFETCH_STATUS_FIFO_CNT(count);
return count;
}
+
+/**
+ * omap_gpmc_ecc_disable - Disables the ECC engine
+ * but doesn't clear the ECC result registers.
+ */
+void omap_gpmc_ecc_disable(void)
+{
+ u32 val;
+
+ /* Disable ECC engine if running */
+ val = gpmc_read_reg(GPMC_ECC_CONFIG);
+ if (val & GPMC_ECC_CONFIG_ECCENABLE) {
+ val &= ~GPMC_ECC_CONFIG_ECCENABLE;
+ gpmc_write_reg(GPMC_ECC_CONFIG, val);
+ }
+}
+
+/**
+ * omap_gpmc_ecc_configure_enable - Configures and enables the ECC-BCH engine
+ *
+ * @cs: chip select number
+ * @ecc16: true for 16-bit ECC column, false for 8-bit ECC columna
+ * @ecc_size0: This value is written to ECCSIZE0 of GPMC_ECC_SIZE_CONFIG reg.
+ * @ecc_size1: This value is written to ECCSIZE1 of GPMC_ECC_SIZE_CONFIG reg.
+ * @use_bch: true for BCH algorithm, false for 1-bit Hamming code algorithm
+ * @bch_type: enum omap_gpmc_bch_type, 4-bit, 8-bit or 16-bit BCH error
+ * correction cabability. Ignored if use_bch is false.
+ * @bch_sectors: No. of sectors to process with BCH. Ignored if use_bch false.
+ * @bch_wrap_mode: pre defined spare area defination for BCH calculation.
+ * Ignored if use_bch is false
+ */
+void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
+ u8 ecc_size1, bool use_bch,
+ enum omap_gpmc_bch_type bch_type,
+ u8 bch_sectors, u8 bch_wrap_mode)
+{
+ u32 val, valx;
+
+ /* Disable ECC engine if running */
+ omap_gpmc_ecc_disable();
+
+ /* Clear all ECC/BCH result registers */
+ gpmc_write_reg(GPMC_ECC_CONTROL, GPMC_ECC_CONTROL_ECCCLEAR);
+
+ /* ECC size */
+ val = (ecc_size1 << GPMC_ECC_SIZE_ECCSIZE1_SHIFT) |
+ (ecc_size0 << GPMC_ECC_SIZE_ECCSIZE0_SHIFT);
+ gpmc_write_reg(GPMC_ECC_SIZE_CONFIG, val);
+
+ /* ECC config */
+ val = (cs << GPMC_ECC_CONFIG_ECCCS_SHIFT) & GPMC_ECC_CONFIG_ECCCS_MASK;
+
+ if (ecc16)
+ val |= GPMC_ECC_CONFIG_ECC16B;
+
+ if (use_bch) {
+ val |= GPMC_ECC_CONFIG_ECCALGBCH;
+ val |= (bch_type << GPMC_ECC_CONFIG_ECCBCHTSEL_SHIFT) &
+ GPMC_ECC_CONFIG_ECCBCHTSEL_MASK;
+ val |= (bch_sectors << GPMC_ECC_CONFIG_ECCNSECTOR_SHIFT) &
+ GPMC_ECC_CONFIG_ECCNSECTOR_MASK;
+ val |= (bch_wrap_mode << GPMC_ECC_CONFIG_ECCWRAPMODE_SHIFT) &
+ GPMC_ECC_CONFIG_ECCWRAPMODE_MASK;
+ } else {
+ /* Reset ECC result pointer to 1 */
+ valx = (1 << GPMC_ECC_CONTROL_ECCPOINTER_SHIFT) &
+ GPMC_ECC_CONTROL_ECCPOINTER_MASK;
+ gpmc_write_reg(GPMC_ECC_CONTROL, valx);
+ }
+
+ /* Enable ECC engine */
+ val |= GPMC_ECC_CONFIG_ECCENABLE;
+ gpmc_write_reg(GPMC_ECC_CONFIG, val);
+}
diff --git a/include/linux/omap-gpmc-nand.h b/include/linux/omap-gpmc-nand.h
index c445d89..f08cd05 100644
--- a/include/linux/omap-gpmc-nand.h
+++ b/include/linux/omap-gpmc-nand.h
@@ -23,6 +23,12 @@ enum omap_gpmc_reg {
OMAP_GPMC_NAND_DATA,
};
+enum omap_gpmc_bch_type {
+ OMAP_GPMC_BCH4,
+ OMAP_GPMC_BCH8,
+ OMAP_GPMC_BCH16,
+};
+
#ifdef CONFIG_ARCH_OMAP2PLUS
u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
@@ -32,6 +38,11 @@ int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
int omap_gpmc_prefetch_stop(int cs);
u32 omap_gpmc_get_prefetch_count(void);
u32 omap_gpmc_get_prefetch_fifo_count(void);
+void omap_gpmc_ecc_disable(void);
+void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
+ u8 ecc_size1, bool use_bch,
+ enum omap_gpmc_bch_type bch_type,
+ u8 bch_sectors, u8 bch_wrap_mode);
#else
static inline u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg)
{
@@ -62,6 +73,20 @@ static inline u32 omap_gpmc_get_prefetch_fifo_count(void)
{
return 0;
}
+
+static inline void omap_gpmc_ecc_disable(void)
+{
+}
+
+static inline void omap_gpmc_ecc_configure_enable(int cs, bool ecc16,
+ u8 ecc_size0, u8 ecc_size1,
+ bool use_bch,
+ enum omap_gpmc_bch_type bch_type,
+ u8 bch_sectors,
+ u8 bch_wrap_mode)
+{
+}
+
#endif
/* Prefetch/Write-post Engine */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* RE: [RFC PATCH 07/10] OMAP: GPMC: Introduce APIs for Configuring ECC Engine
2014-07-09 12:37 ` [RFC PATCH 07/10] OMAP: GPMC: Introduce APIs for Configuring ECC Engine Roger Quadros
@ 2014-07-11 7:54 ` Gupta, Pekon
0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Pekon @ 2014-07-11 7:54 UTC (permalink / raw)
To: Quadros, Roger, tony@atomide.com, computersforpeace@gmail.com
Cc: javier@dowhile0.org, ezequiel.garcia@free-electrons.com,
dwmw2@infradead.org, jg1.han@samsung.com, Nori, Sekhar,
linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Roger,
>From: Quadros, Roger
>
>Even though the ECC/BCH engine is meant for exclusive use by
>the OMAP NAND controller, the ECC/BCH registers belong
>to the GPMC controller's register space
>
>Add omap_gpmc_ecc_configure_enable() and omap_gpmc_ecc_disable()
>to manage the ECC engine. OMAP NAND driver must use these APIs
>instead of directly accessing the ECC Engine registers.
>
>Signed-off-by: Roger Quadros <rogerq@ti.com>
>---
As suggested in
http://lists.infradead.org/pipermail/linux-mtd/2014-July/054595.html
Please try to move chip->ecc.calculate() implementations like
- omap_calculate_ecc_bch()
- omap_calculate_ecc()
>From NAND driver To GPMC driver, as that should be easy, And solve
most of your work (no need to export GPMC registers).
- You may rename them appropriately and change the argument list
if needed.
- And NAND driver can just have some wrapper functions to match
the MTD interface arguments.
with regards, pekon
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH 08/10] OMAP: GPMC: Introduce APIs to get ECC/BCH results
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (6 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 07/10] OMAP: GPMC: Introduce APIs for Configuring ECC Engine Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-09 12:37 ` [RFC PATCH 09/10] mtd: nand: omap: Use GPMC APIs for accessing ECC/BCH engine Roger Quadros
` (2 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: javier, pekon, ezequiel.garcia, dwmw2, jg1.han, nsekhar,
linux-mtd, linux-omap, linux-kernel, Roger Quadros
Even though the ECC/BCH engine is meant for exclusive use by
the OMAP NAND controller, the ECC/BCH result registers belong
to the GPMC controller's register space.
Introduce 2 APIs to access the ECC/BCH results.
void omap_gpmc_ecc_get_result(int length, u32 *result);
void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result);
The first one is to get the Hamming code ECC result registers
and the second one is to get the BCH ECC result registers.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
arch/arm/mach-omap2/gpmc.c | 97 +++++++++++++++++++++++++++++++++++++++---
include/linux/omap-gpmc-nand.h | 11 +++++
2 files changed, 101 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 8befd16..9222244 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -65,6 +65,7 @@
#define GPMC_ECC_CONTROL 0x1f8
#define GPMC_ECC_SIZE_CONFIG 0x1fc
#define GPMC_ECC1_RESULT 0x200
+#define GPMC_ECC9_RESULT 0x220
#define GPMC_ECC_BCH_RESULT_0 0x240 /* not available on OMAP2 */
#define GPMC_ECC_BCH_RESULT_1 0x244 /* not available on OMAP2 */
#define GPMC_ECC_BCH_RESULT_2 0x248 /* not available on OMAP2 */
@@ -83,6 +84,7 @@
#define GPMC_CS0_OFFSET 0x60
#define GPMC_CS_SIZE 0x30
#define GPMC_BCH_SIZE 0x10
+#define GPMC_BCH_NUM 7 /* Max no. of BCH registers 0-6 */
#define GPMC_MEM_END 0x3FFFFFFF
@@ -96,9 +98,10 @@
#define GPMC_REVISION_MAJOR(l) ((l >> 4) & 0xf)
#define GPMC_REVISION_MINOR(l) (l & 0xf)
-#define GPMC_HAS_WR_ACCESS 0x1
-#define GPMC_HAS_WR_DATA_MUX_BUS 0x2
-#define GPMC_HAS_MUX_AAD 0x4
+#define GPMC_HAS_WR_ACCESS BIT(0)
+#define GPMC_HAS_WR_DATA_MUX_BUS BIT(1)
+#define GPMC_HAS_MUX_AAD BIT(2)
+#define GPMC_HAS_BCH BIT(3)
#define GPMC_NR_WAITPINS 4
@@ -185,6 +188,7 @@ static DEFINE_SPINLOCK(gpmc_mem_lock);
static unsigned int gpmc_cs_map = ((1 << GPMC_CS_NUM) - 1);
static unsigned int gpmc_cs_num = GPMC_CS_NUM;
static unsigned int gpmc_nr_waitpins;
+static unsigned int gpmc_bch_num = GPMC_BCH_NUM;
static struct device *gpmc_dev;
static int gpmc_irq;
static resource_size_t phys_base, mem_size;
@@ -198,6 +202,7 @@ struct gpmc_nand_reg {
};
static struct gpmc_nand_reg gpmc_nand_reg_map[GPMC_CS_NUM];
+void __iomem *gpmc_bch_reg_map[GPMC_BCH_NUM][GPMC_BCH_NUM_REMAINDER];
static struct clk *gpmc_l3_clk;
@@ -205,7 +210,7 @@ static irqreturn_t gpmc_handle_irq(int irq, void *dev);
static void gpmc_fill_nand_reg_map(void)
{
- int i;
+ int i, j;
for (i = 0; i < gpmc_cs_num; i++) {
gpmc_nand_reg_map[i].command = gpmc_base + GPMC_CS0_OFFSET +
@@ -215,6 +220,28 @@ static void gpmc_fill_nand_reg_map(void)
gpmc_nand_reg_map[i].data = gpmc_base + GPMC_CS0_OFFSET +
GPMC_CS_NAND_DATA + GPMC_CS_SIZE * i;
}
+
+ if (!(gpmc_capability & GPMC_HAS_BCH))
+ return;
+
+
+ for (i = 0; i < 4; i++) {
+ for (j = 0; j < 8; j++) {
+ gpmc_bch_reg_map[i][j] = gpmc_base +
+ GPMC_ECC_BCH_RESULT_0 +
+ i * 4 + GPMC_BCH_SIZE * j;
+ }
+ }
+
+ /* 2nd for loop for BCH4 onwards due to non-consecutive address */
+ for (i = 4; i < gpmc_bch_num; i++) {
+ for (j = 0; j < 8; j++) {
+ gpmc_bch_reg_map[i][j] = gpmc_base +
+ GPMC_ECC_BCH_RESULT_4 +
+ (i - 4) * 4 +
+ GPMC_BCH_SIZE * j;
+ }
+ }
}
static void gpmc_write_reg(int idx, u32 val)
@@ -1738,10 +1765,17 @@ static int gpmc_probe(struct platform_device *pdev)
* - OMAP3xxx = 5.0
* - OMAP44xx/54xx/AM335x = 6.0
*/
- if (GPMC_REVISION_MAJOR(l) > 0x4)
- gpmc_capability = GPMC_HAS_WR_ACCESS | GPMC_HAS_WR_DATA_MUX_BUS;
- if (GPMC_REVISION_MAJOR(l) > 0x5)
+ if (GPMC_REVISION_MAJOR(l) >= 5) {
+ gpmc_capability = GPMC_HAS_WR_ACCESS |
+ GPMC_HAS_WR_DATA_MUX_BUS | GPMC_HAS_BCH;
+ gpmc_bch_num = 4;
+ }
+
+ if (GPMC_REVISION_MAJOR(l) >= 6) {
gpmc_capability |= GPMC_HAS_MUX_AAD;
+ gpmc_bch_num = GPMC_BCH_NUM;
+ }
+
dev_info(gpmc_dev, "GPMC revision %d.%d\n", GPMC_REVISION_MAJOR(l),
GPMC_REVISION_MINOR(l));
@@ -2188,3 +2222,52 @@ void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
val |= GPMC_ECC_CONFIG_ECCENABLE;
gpmc_write_reg(GPMC_ECC_CONFIG, val);
}
+
+/**
+ * omap_gpmc_ecc_get_result - reads out the Hamming code ECC result registers
+ *
+ * @length: Number of 32-bit registers to read
+ * @result: pointer to 32-bit buffer where results should be copied into
+ */
+void omap_gpmc_ecc_get_result(int length, u32 *result)
+{
+ u32 reg_addr;
+ int i;
+
+ if (!gpmc_dev)
+ return;
+
+ reg_addr = GPMC_ECC1_RESULT;
+ for (i = 0; i < length; i++) {
+ *result++ = gpmc_read_reg(reg_addr);
+ reg_addr += 4;
+ /* Don't read past ECC_RESULT region */
+ if (reg_addr > GPMC_ECC9_RESULT)
+ break;
+ }
+}
+
+/**
+ * omap_gpmc_ecc_get_bch_result - reads out the BCH result registers
+ *
+ * @length: Number of 32-bit registers to read
+ * @sector: Which sector's results to read (0 to 7)
+ * @result: pointer to 32-bit buffer where results should be copied into
+ */
+void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result)
+{
+ int i;
+
+ if (!gpmc_dev)
+ return;
+
+ if (sector > GPMC_BCH_NUM_REMAINDER)
+ return;
+
+ /* Don't read past BCH_RESULT region */
+ if (length > gpmc_bch_num)
+ length = gpmc_bch_num;
+
+ for (i = 0; i < length; i++)
+ *result++ = readl_relaxed(gpmc_bch_reg_map[i][sector]);
+}
diff --git a/include/linux/omap-gpmc-nand.h b/include/linux/omap-gpmc-nand.h
index f08cd05..d0ef165 100644
--- a/include/linux/omap-gpmc-nand.h
+++ b/include/linux/omap-gpmc-nand.h
@@ -43,6 +43,8 @@ void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
u8 ecc_size1, bool use_bch,
enum omap_gpmc_bch_type bch_type,
u8 bch_sectors, u8 bch_wrap_mode);
+void omap_gpmc_ecc_get_result(int length, u32 *result);
+void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result);
#else
static inline u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg)
{
@@ -87,6 +89,15 @@ static inline void omap_gpmc_ecc_configure_enable(int cs, bool ecc16,
{
}
+static inline void omap_gpmc_ecc_get_result(int length, u32 *result)
+{
+}
+
+static inline void omap_gpmc_ecc_get_bch_result(int length, u8 sector,
+ u32 *result)
+{
+}
+
#endif
/* Prefetch/Write-post Engine */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH 09/10] mtd: nand: omap: Use GPMC APIs for accessing ECC/BCH engine
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (7 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 08/10] OMAP: GPMC: Introduce APIs to get ECC/BCH results Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-11 7:56 ` Gupta, Pekon
2014-07-09 12:37 ` [RFC PATCH 10/10] OMAP: GPMC: NAND: Don't pass NAND/ECC/BCH register adresses to NAND driver Roger Quadros
2014-07-11 6:52 ` [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Tony Lindgren
10 siblings, 1 reply; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: javier, pekon, ezequiel.garcia, dwmw2, jg1.han, nsekhar,
linux-mtd, linux-omap, linux-kernel, Roger Quadros
Don't access the ECC/BCH engine registers directly as they belong
to the GPMC controller's register space. Use the relevant
GPMC APIs instead.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/mtd/nand/omap2.c | 191 +++++++++++++++++++----------------------------
1 file changed, 76 insertions(+), 115 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 420ef0b..6b0f953 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -865,16 +865,10 @@ static int omap_correct_data(struct mtd_info *mtd, u_char *dat,
static int omap_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
u_char *ecc_code)
{
- struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
- mtd);
u32 val;
- val = readl(info->reg.gpmc_ecc_config);
- if (((val >> ECC_CONFIG_CS_SHIFT) & ~CS_MASK) != info->gpmc_cs)
- return -EINVAL;
-
/* read ecc result */
- val = readl(info->reg.gpmc_ecc1_result);
+ omap_gpmc_ecc_get_result(1, &val);
*ecc_code++ = val; /* P128e, ..., P1e */
*ecc_code++ = val >> 16; /* P128o, ..., P1o */
/* P2048o, P1024o, P512o, P256o, P2048e, P1024e, P512e, P256e */
@@ -894,34 +888,22 @@ static void omap_enable_hwecc(struct mtd_info *mtd, int mode)
mtd);
struct nand_chip *chip = mtd->priv;
unsigned int dev_width = (chip->options & NAND_BUSWIDTH_16) ? 1 : 0;
- u32 val;
-
- /* clear ecc and enable bits */
- val = ECCCLEAR | ECC1;
- writel(val, info->reg.gpmc_ecc_control);
+ u32 ecc_size0;
- /* program ecc and result sizes */
- val = ((((info->nand.ecc.size >> 1) - 1) << ECCSIZE1_SHIFT) |
- ECC1RESULTSIZE);
- writel(val, info->reg.gpmc_ecc_size_config);
+ ecc_size0 = (info->nand.ecc.size >> 1) - 1;
switch (mode) {
case NAND_ECC_READ:
case NAND_ECC_WRITE:
- writel(ECCCLEAR | ECC1, info->reg.gpmc_ecc_control);
+ omap_gpmc_ecc_configure_enable(info->gpmc_cs, dev_width,
+ ecc_size0, 0, false,
+ 0, 0, 0);
break;
case NAND_ECC_READSYN:
- writel(ECCCLEAR, info->reg.gpmc_ecc_control);
- break;
- default:
- dev_info(&info->pdev->dev,
- "error: unrecognized Mode[%d]!\n", mode);
+ /* Disable the engine, but don't clear ECC results */
+ omap_gpmc_ecc_disable();
break;
}
-
- /* (ECC 16 or 8 bit col) | ( CS ) | ECC Enable */
- val = (dev_width << 7) | (info->gpmc_cs << 1) | (0x1);
- writel(val, info->reg.gpmc_ecc_config);
}
/**
@@ -993,20 +975,20 @@ static int omap_dev_ready(struct mtd_info *mtd)
*/
static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
{
- unsigned int bch_type;
+ enum omap_gpmc_bch_type bch_type;
unsigned int dev_width, nsectors;
struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
mtd);
enum omap_ecc ecc_opt = info->ecc_opt;
struct nand_chip *chip = mtd->priv;
- u32 val, wr_mode;
+ u32 wr_mode;
unsigned int ecc_size1, ecc_size0;
/* GPMC configurations for calculating ECC */
nsectors = chip->ecc.steps;
switch (ecc_opt) {
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
- bch_type = 0;
+ bch_type = OMAP_GPMC_BCH4;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_6;
ecc_size0 = BCH_ECC_SIZE0;
@@ -1018,7 +1000,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
}
break;
case OMAP_ECC_BCH4_CODE_HW:
- bch_type = 0;
+ bch_type = OMAP_GPMC_BCH4;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_1;
ecc_size0 = BCH4R_ECC_SIZE0;
@@ -1030,7 +1012,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
}
break;
case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
- bch_type = 1;
+ bch_type = OMAP_GPMC_BCH8;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_6;
ecc_size0 = BCH_ECC_SIZE0;
@@ -1042,7 +1024,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
}
break;
case OMAP_ECC_BCH8_CODE_HW:
- bch_type = 1;
+ bch_type = OMAP_GPMC_BCH8;
if (mode == NAND_ECC_READ) {
wr_mode = BCH_WRAPMODE_1;
ecc_size0 = BCH8R_ECC_SIZE0;
@@ -1054,7 +1036,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
}
break;
case OMAP_ECC_BCH16_CODE_HW:
- bch_type = 0x2;
+ bch_type = OMAP_GPMC_BCH16;
if (mode == NAND_ECC_READ) {
wr_mode = 0x01;
ecc_size0 = 52; /* ECC bits in nibbles per sector */
@@ -1069,27 +1051,11 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
return;
}
- writel(ECC1, info->reg.gpmc_ecc_control);
-
- /* Configure ecc size for BCH */
- val = (ecc_size1 << ECCSIZE1_SHIFT) | (ecc_size0 << ECCSIZE0_SHIFT);
- writel(val, info->reg.gpmc_ecc_size_config);
-
dev_width = (chip->options & NAND_BUSWIDTH_16) ? 1 : 0;
- /* BCH configuration */
- val = ((1 << 16) | /* enable BCH */
- (bch_type << 12) | /* BCH4/BCH8/BCH16 */
- (wr_mode << 8) | /* wrap mode */
- (dev_width << 7) | /* bus width */
- (((nsectors-1) & 0x7) << 4) | /* number of sectors */
- (info->gpmc_cs << 1) | /* ECC CS */
- (0x1)); /* enable ECC */
-
- writel(val, info->reg.gpmc_ecc_config);
-
- /* Clear ecc and enable bits */
- writel(ECCCLEAR | ECC1, info->reg.gpmc_ecc_control);
+ omap_gpmc_ecc_configure_enable(info->gpmc_cs, dev_width,
+ ecc_size0, ecc_size1, true,
+ bch_type, nsectors - 1, wr_mode);
}
static u8 bch4_polynomial[] = {0x28, 0x13, 0xcc, 0x39, 0x96, 0xac, 0x7f};
@@ -1111,11 +1077,10 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
mtd);
struct nand_chip *chip = mtd->priv;
int eccbytes = info->nand.ecc.bytes;
- struct gpmc_nand_regs *gpmc_regs = &info->reg;
u8 *ecc_code;
- unsigned long nsectors, bch_val1, bch_val2, bch_val3, bch_val4;
- u32 val;
+ unsigned long nsectors;
int i, j;
+ u32 bch_val[7];
nsectors = chip->ecc.steps;
for (i = 0; i < nsectors; i++) {
@@ -1123,71 +1088,67 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
switch (info->ecc_opt) {
case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
case OMAP_ECC_BCH8_CODE_HW:
- bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
- bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
- bch_val3 = readl(gpmc_regs->gpmc_bch_result2[i]);
- bch_val4 = readl(gpmc_regs->gpmc_bch_result3[i]);
- *ecc_code++ = (bch_val4 & 0xFF);
- *ecc_code++ = ((bch_val3 >> 24) & 0xFF);
- *ecc_code++ = ((bch_val3 >> 16) & 0xFF);
- *ecc_code++ = ((bch_val3 >> 8) & 0xFF);
- *ecc_code++ = (bch_val3 & 0xFF);
- *ecc_code++ = ((bch_val2 >> 24) & 0xFF);
- *ecc_code++ = ((bch_val2 >> 16) & 0xFF);
- *ecc_code++ = ((bch_val2 >> 8) & 0xFF);
- *ecc_code++ = (bch_val2 & 0xFF);
- *ecc_code++ = ((bch_val1 >> 24) & 0xFF);
- *ecc_code++ = ((bch_val1 >> 16) & 0xFF);
- *ecc_code++ = ((bch_val1 >> 8) & 0xFF);
- *ecc_code++ = (bch_val1 & 0xFF);
+ omap_gpmc_ecc_get_bch_result(4, i, bch_val);
+ *ecc_code++ = (bch_val[3] & 0xFF);
+ *ecc_code++ = ((bch_val[2] >> 24) & 0xFF);
+ *ecc_code++ = ((bch_val[2] >> 16) & 0xFF);
+ *ecc_code++ = ((bch_val[2] >> 8) & 0xFF);
+ *ecc_code++ = (bch_val[2] & 0xFF);
+ *ecc_code++ = ((bch_val[1] >> 24) & 0xFF);
+ *ecc_code++ = ((bch_val[1] >> 16) & 0xFF);
+ *ecc_code++ = ((bch_val[1] >> 8) & 0xFF);
+ *ecc_code++ = (bch_val[1] & 0xFF);
+ *ecc_code++ = ((bch_val[0] >> 24) & 0xFF);
+ *ecc_code++ = ((bch_val[0] >> 16) & 0xFF);
+ *ecc_code++ = ((bch_val[0] >> 8) & 0xFF);
+ *ecc_code++ = (bch_val[0] & 0xFF);
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
case OMAP_ECC_BCH4_CODE_HW:
- bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
- bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
- *ecc_code++ = ((bch_val2 >> 12) & 0xFF);
- *ecc_code++ = ((bch_val2 >> 4) & 0xFF);
- *ecc_code++ = ((bch_val2 & 0xF) << 4) |
- ((bch_val1 >> 28) & 0xF);
- *ecc_code++ = ((bch_val1 >> 20) & 0xFF);
- *ecc_code++ = ((bch_val1 >> 12) & 0xFF);
- *ecc_code++ = ((bch_val1 >> 4) & 0xFF);
- *ecc_code++ = ((bch_val1 & 0xF) << 4);
+ omap_gpmc_ecc_get_bch_result(2, i, bch_val);
+ *ecc_code++ = ((bch_val[1] >> 12) & 0xFF);
+ *ecc_code++ = ((bch_val[1] >> 4) & 0xFF);
+ *ecc_code++ = ((bch_val[1] & 0xF) << 4) |
+ ((bch_val[0] >> 28) & 0xF);
+ *ecc_code++ = ((bch_val[0] >> 20) & 0xFF);
+ *ecc_code++ = ((bch_val[0] >> 12) & 0xFF);
+ *ecc_code++ = ((bch_val[0] >> 4) & 0xFF);
+ *ecc_code++ = ((bch_val[0] & 0xF) << 4);
break;
case OMAP_ECC_BCH16_CODE_HW:
- val = readl(gpmc_regs->gpmc_bch_result6[i]);
- ecc_code[0] = ((val >> 8) & 0xFF);
- ecc_code[1] = ((val >> 0) & 0xFF);
- val = readl(gpmc_regs->gpmc_bch_result5[i]);
- ecc_code[2] = ((val >> 24) & 0xFF);
- ecc_code[3] = ((val >> 16) & 0xFF);
- ecc_code[4] = ((val >> 8) & 0xFF);
- ecc_code[5] = ((val >> 0) & 0xFF);
- val = readl(gpmc_regs->gpmc_bch_result4[i]);
- ecc_code[6] = ((val >> 24) & 0xFF);
- ecc_code[7] = ((val >> 16) & 0xFF);
- ecc_code[8] = ((val >> 8) & 0xFF);
- ecc_code[9] = ((val >> 0) & 0xFF);
- val = readl(gpmc_regs->gpmc_bch_result3[i]);
- ecc_code[10] = ((val >> 24) & 0xFF);
- ecc_code[11] = ((val >> 16) & 0xFF);
- ecc_code[12] = ((val >> 8) & 0xFF);
- ecc_code[13] = ((val >> 0) & 0xFF);
- val = readl(gpmc_regs->gpmc_bch_result2[i]);
- ecc_code[14] = ((val >> 24) & 0xFF);
- ecc_code[15] = ((val >> 16) & 0xFF);
- ecc_code[16] = ((val >> 8) & 0xFF);
- ecc_code[17] = ((val >> 0) & 0xFF);
- val = readl(gpmc_regs->gpmc_bch_result1[i]);
- ecc_code[18] = ((val >> 24) & 0xFF);
- ecc_code[19] = ((val >> 16) & 0xFF);
- ecc_code[20] = ((val >> 8) & 0xFF);
- ecc_code[21] = ((val >> 0) & 0xFF);
- val = readl(gpmc_regs->gpmc_bch_result0[i]);
- ecc_code[22] = ((val >> 24) & 0xFF);
- ecc_code[23] = ((val >> 16) & 0xFF);
- ecc_code[24] = ((val >> 8) & 0xFF);
- ecc_code[25] = ((val >> 0) & 0xFF);
+ omap_gpmc_ecc_get_bch_result(7, i, bch_val);
+ ecc_code[0] = ((bch_val[6] >> 8) & 0xFF);
+ ecc_code[1] = ((bch_val[6] >> 0) & 0xFF);
+
+ ecc_code[2] = ((bch_val[5] >> 24) & 0xFF);
+ ecc_code[3] = ((bch_val[5] >> 16) & 0xFF);
+ ecc_code[4] = ((bch_val[5] >> 8) & 0xFF);
+ ecc_code[5] = ((bch_val[5] >> 0) & 0xFF);
+
+ ecc_code[6] = ((bch_val[4] >> 24) & 0xFF);
+ ecc_code[7] = ((bch_val[4] >> 16) & 0xFF);
+ ecc_code[8] = ((bch_val[4] >> 8) & 0xFF);
+ ecc_code[9] = ((bch_val[4] >> 0) & 0xFF);
+
+ ecc_code[10] = ((bch_val[3] >> 24) & 0xFF);
+ ecc_code[11] = ((bch_val[3] >> 16) & 0xFF);
+ ecc_code[12] = ((bch_val[3] >> 8) & 0xFF);
+ ecc_code[13] = ((bch_val[3] >> 0) & 0xFF);
+
+ ecc_code[14] = ((bch_val[2] >> 24) & 0xFF);
+ ecc_code[15] = ((bch_val[2] >> 16) & 0xFF);
+ ecc_code[16] = ((bch_val[2] >> 8) & 0xFF);
+ ecc_code[17] = ((bch_val[2] >> 0) & 0xFF);
+
+ ecc_code[18] = ((bch_val[1] >> 24) & 0xFF);
+ ecc_code[19] = ((bch_val[1] >> 16) & 0xFF);
+ ecc_code[20] = ((bch_val[1] >> 8) & 0xFF);
+ ecc_code[21] = ((bch_val[1] >> 0) & 0xFF);
+
+ ecc_code[22] = ((bch_val[0] >> 24) & 0xFF);
+ ecc_code[23] = ((bch_val[0] >> 16) & 0xFF);
+ ecc_code[24] = ((bch_val[0] >> 8) & 0xFF);
+ ecc_code[25] = ((bch_val[0] >> 0) & 0xFF);
break;
default:
return -EINVAL;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* RE: [RFC PATCH 09/10] mtd: nand: omap: Use GPMC APIs for accessing ECC/BCH engine
2014-07-09 12:37 ` [RFC PATCH 09/10] mtd: nand: omap: Use GPMC APIs for accessing ECC/BCH engine Roger Quadros
@ 2014-07-11 7:56 ` Gupta, Pekon
0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Pekon @ 2014-07-11 7:56 UTC (permalink / raw)
To: Quadros, Roger, tony@atomide.com, computersforpeace@gmail.com
Cc: javier@dowhile0.org, ezequiel.garcia@free-electrons.com,
dwmw2@infradead.org, jg1.han@samsung.com, Nori, Sekhar,
linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org,
linux-kernel@vger.kernel.org
Roger,
>From: Quadros, Roger
>Don't access the ECC/BCH engine registers directly as they belong
>to the GPMC controller's register space. Use the relevant
>GPMC APIs instead.
>
>Signed-off-by: Roger Quadros <rogerq@ti.com>
>---
> drivers/mtd/nand/omap2.c | 191 +++++++++++++++++++----------------------------
> 1 file changed, 76 insertions(+), 115 deletions(-)
>
>diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>index 420ef0b..6b0f953 100644
>--- a/drivers/mtd/nand/omap2.c
>+++ b/drivers/mtd/nand/omap2.c
>@@ -1123,71 +1088,67 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>*mtd,
> switch (info->ecc_opt) {
> case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> case OMAP_ECC_BCH8_CODE_HW:
>- bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
>- bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
>- bch_val3 = readl(gpmc_regs->gpmc_bch_result2[i]);
>- bch_val4 = readl(gpmc_regs->gpmc_bch_result3[i]);
>- *ecc_code++ = (bch_val4 & 0xFF);
>- *ecc_code++ = ((bch_val3 >> 24) & 0xFF);
>- *ecc_code++ = ((bch_val3 >> 16) & 0xFF);
>- *ecc_code++ = ((bch_val3 >> 8) & 0xFF);
>- *ecc_code++ = (bch_val3 & 0xFF);
>- *ecc_code++ = ((bch_val2 >> 24) & 0xFF);
>- *ecc_code++ = ((bch_val2 >> 16) & 0xFF);
>- *ecc_code++ = ((bch_val2 >> 8) & 0xFF);
>- *ecc_code++ = (bch_val2 & 0xFF);
>- *ecc_code++ = ((bch_val1 >> 24) & 0xFF);
>- *ecc_code++ = ((bch_val1 >> 16) & 0xFF);
>- *ecc_code++ = ((bch_val1 >> 8) & 0xFF);
>- *ecc_code++ = (bch_val1 & 0xFF);
>+ omap_gpmc_ecc_get_bch_result(4, i, bch_val);
>+ *ecc_code++ = (bch_val[3] & 0xFF);
>+ *ecc_code++ = ((bch_val[2] >> 24) & 0xFF);
>+ *ecc_code++ = ((bch_val[2] >> 16) & 0xFF);
>+ *ecc_code++ = ((bch_val[2] >> 8) & 0xFF);
>+ *ecc_code++ = (bch_val[2] & 0xFF);
>+ *ecc_code++ = ((bch_val[1] >> 24) & 0xFF);
>+ *ecc_code++ = ((bch_val[1] >> 16) & 0xFF);
>+ *ecc_code++ = ((bch_val[1] >> 8) & 0xFF);
>+ *ecc_code++ = (bch_val[1] & 0xFF);
>+ *ecc_code++ = ((bch_val[0] >> 24) & 0xFF);
>+ *ecc_code++ = ((bch_val[0] >> 16) & 0xFF);
>+ *ecc_code++ = ((bch_val[0] >> 8) & 0xFF);
>+ *ecc_code++ = (bch_val[0] & 0xFF);
> break;
> case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> case OMAP_ECC_BCH4_CODE_HW:
>- bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
>- bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
>- *ecc_code++ = ((bch_val2 >> 12) & 0xFF);
>- *ecc_code++ = ((bch_val2 >> 4) & 0xFF);
>- *ecc_code++ = ((bch_val2 & 0xF) << 4) |
>- ((bch_val1 >> 28) & 0xF);
>- *ecc_code++ = ((bch_val1 >> 20) & 0xFF);
>- *ecc_code++ = ((bch_val1 >> 12) & 0xFF);
>- *ecc_code++ = ((bch_val1 >> 4) & 0xFF);
>- *ecc_code++ = ((bch_val1 & 0xF) << 4);
>+ omap_gpmc_ecc_get_bch_result(2, i, bch_val);
>+ *ecc_code++ = ((bch_val[1] >> 12) & 0xFF);
>+ *ecc_code++ = ((bch_val[1] >> 4) & 0xFF);
>+ *ecc_code++ = ((bch_val[1] & 0xF) << 4) |
>+ ((bch_val[0] >> 28) & 0xF);
>+ *ecc_code++ = ((bch_val[0] >> 20) & 0xFF);
>+ *ecc_code++ = ((bch_val[0] >> 12) & 0xFF);
>+ *ecc_code++ = ((bch_val[0] >> 4) & 0xFF);
>+ *ecc_code++ = ((bch_val[0] & 0xF) << 4);
> break;
> case OMAP_ECC_BCH16_CODE_HW:
>- val = readl(gpmc_regs->gpmc_bch_result6[i]);
>- ecc_code[0] = ((val >> 8) & 0xFF);
>- ecc_code[1] = ((val >> 0) & 0xFF);
>- val = readl(gpmc_regs->gpmc_bch_result5[i]);
>- ecc_code[2] = ((val >> 24) & 0xFF);
>- ecc_code[3] = ((val >> 16) & 0xFF);
>- ecc_code[4] = ((val >> 8) & 0xFF);
>- ecc_code[5] = ((val >> 0) & 0xFF);
>- val = readl(gpmc_regs->gpmc_bch_result4[i]);
>- ecc_code[6] = ((val >> 24) & 0xFF);
>- ecc_code[7] = ((val >> 16) & 0xFF);
>- ecc_code[8] = ((val >> 8) & 0xFF);
>- ecc_code[9] = ((val >> 0) & 0xFF);
>- val = readl(gpmc_regs->gpmc_bch_result3[i]);
>- ecc_code[10] = ((val >> 24) & 0xFF);
>- ecc_code[11] = ((val >> 16) & 0xFF);
>- ecc_code[12] = ((val >> 8) & 0xFF);
>- ecc_code[13] = ((val >> 0) & 0xFF);
>- val = readl(gpmc_regs->gpmc_bch_result2[i]);
>- ecc_code[14] = ((val >> 24) & 0xFF);
>- ecc_code[15] = ((val >> 16) & 0xFF);
>- ecc_code[16] = ((val >> 8) & 0xFF);
>- ecc_code[17] = ((val >> 0) & 0xFF);
>- val = readl(gpmc_regs->gpmc_bch_result1[i]);
>- ecc_code[18] = ((val >> 24) & 0xFF);
>- ecc_code[19] = ((val >> 16) & 0xFF);
>- ecc_code[20] = ((val >> 8) & 0xFF);
>- ecc_code[21] = ((val >> 0) & 0xFF);
>- val = readl(gpmc_regs->gpmc_bch_result0[i]);
>- ecc_code[22] = ((val >> 24) & 0xFF);
>- ecc_code[23] = ((val >> 16) & 0xFF);
>- ecc_code[24] = ((val >> 8) & 0xFF);
>- ecc_code[25] = ((val >> 0) & 0xFF);
>+ omap_gpmc_ecc_get_bch_result(7, i, bch_val);
>+ ecc_code[0] = ((bch_val[6] >> 8) & 0xFF);
>+ ecc_code[1] = ((bch_val[6] >> 0) & 0xFF);
>+
>+ ecc_code[2] = ((bch_val[5] >> 24) & 0xFF);
>+ ecc_code[3] = ((bch_val[5] >> 16) & 0xFF);
>+ ecc_code[4] = ((bch_val[5] >> 8) & 0xFF);
>+ ecc_code[5] = ((bch_val[5] >> 0) & 0xFF);
>+
>+ ecc_code[6] = ((bch_val[4] >> 24) & 0xFF);
>+ ecc_code[7] = ((bch_val[4] >> 16) & 0xFF);
>+ ecc_code[8] = ((bch_val[4] >> 8) & 0xFF);
>+ ecc_code[9] = ((bch_val[4] >> 0) & 0xFF);
>+
>+ ecc_code[10] = ((bch_val[3] >> 24) & 0xFF);
>+ ecc_code[11] = ((bch_val[3] >> 16) & 0xFF);
>+ ecc_code[12] = ((bch_val[3] >> 8) & 0xFF);
>+ ecc_code[13] = ((bch_val[3] >> 0) & 0xFF);
>+
>+ ecc_code[14] = ((bch_val[2] >> 24) & 0xFF);
>+ ecc_code[15] = ((bch_val[2] >> 16) & 0xFF);
>+ ecc_code[16] = ((bch_val[2] >> 8) & 0xFF);
>+ ecc_code[17] = ((bch_val[2] >> 0) & 0xFF);
>+
>+ ecc_code[18] = ((bch_val[1] >> 24) & 0xFF);
>+ ecc_code[19] = ((bch_val[1] >> 16) & 0xFF);
>+ ecc_code[20] = ((bch_val[1] >> 8) & 0xFF);
>+ ecc_code[21] = ((bch_val[1] >> 0) & 0xFF);
>+
>+ ecc_code[22] = ((bch_val[0] >> 24) & 0xFF);
>+ ecc_code[23] = ((bch_val[0] >> 16) & 0xFF);
>+ ecc_code[24] = ((bch_val[0] >> 8) & 0xFF);
>+ ecc_code[25] = ((bch_val[0] >> 0) & 0xFF);
> break;
> default:
> return -EINVAL;
>--
>1.8.3.2
Again same feedback.
You won't need all these changes, if you move this function
completely into GPMC driver, leaving only the wrapper here
which make the GPMC function compatible to chip->ecc.calculate.
with regards, pekon
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH 10/10] OMAP: GPMC: NAND: Don't pass NAND/ECC/BCH register adresses to NAND driver
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (8 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 09/10] mtd: nand: omap: Use GPMC APIs for accessing ECC/BCH engine Roger Quadros
@ 2014-07-09 12:37 ` Roger Quadros
2014-07-11 6:52 ` [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Tony Lindgren
10 siblings, 0 replies; 23+ messages in thread
From: Roger Quadros @ 2014-07-09 12:37 UTC (permalink / raw)
To: tony, computersforpeace
Cc: javier, pekon, ezequiel.garcia, dwmw2, jg1.han, nsekhar,
linux-mtd, linux-omap, linux-kernel, Roger Quadros
The NAND driver now relies completely on the GPMC APIs to access
the NAND control lines and ECC/BCH engine. We no longer need
gpmc_update_nand_reg() and struct gpmc_nand_regs, so get rid of them.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
arch/arm/mach-omap2/gpmc-nand.c | 2 --
arch/arm/mach-omap2/gpmc.c | 38 ----------------------------
arch/arm/mach-omap2/gpmc.h | 1 -
drivers/mtd/nand/omap2.c | 2 --
include/linux/platform_data/mtd-nand-omap2.h | 5 +++-
5 files changed, 4 insertions(+), 44 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index 17cd393..565bd15 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -138,8 +138,6 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
if (err < 0)
goto out_free_cs;
- gpmc_update_nand_reg(&gpmc_nand_data->reg, gpmc_nand_data->cs);
-
if (!gpmc_hwecc_bch_capable(gpmc_nand_data->ecc_opt)) {
dev_err(dev, "Unsupported NAND ECC scheme selected\n");
return -EINVAL;
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 9222244..fbd651e 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -724,44 +724,6 @@ int gpmc_configure(int cmd, int wval)
}
EXPORT_SYMBOL(gpmc_configure);
-void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs)
-{
- int i;
-
- reg->gpmc_status = gpmc_base + GPMC_STATUS;
- reg->gpmc_nand_command = gpmc_base + GPMC_CS0_OFFSET +
- GPMC_CS_NAND_COMMAND + GPMC_CS_SIZE * cs;
- reg->gpmc_nand_address = gpmc_base + GPMC_CS0_OFFSET +
- GPMC_CS_NAND_ADDRESS + GPMC_CS_SIZE * cs;
- reg->gpmc_nand_data = gpmc_base + GPMC_CS0_OFFSET +
- GPMC_CS_NAND_DATA + GPMC_CS_SIZE * cs;
- reg->gpmc_prefetch_config1 = gpmc_base + GPMC_PREFETCH_CONFIG1;
- reg->gpmc_prefetch_config2 = gpmc_base + GPMC_PREFETCH_CONFIG2;
- reg->gpmc_prefetch_control = gpmc_base + GPMC_PREFETCH_CONTROL;
- reg->gpmc_prefetch_status = gpmc_base + GPMC_PREFETCH_STATUS;
- reg->gpmc_ecc_config = gpmc_base + GPMC_ECC_CONFIG;
- reg->gpmc_ecc_control = gpmc_base + GPMC_ECC_CONTROL;
- reg->gpmc_ecc_size_config = gpmc_base + GPMC_ECC_SIZE_CONFIG;
- reg->gpmc_ecc1_result = gpmc_base + GPMC_ECC1_RESULT;
-
- for (i = 0; i < GPMC_BCH_NUM_REMAINDER; i++) {
- reg->gpmc_bch_result0[i] = gpmc_base + GPMC_ECC_BCH_RESULT_0 +
- GPMC_BCH_SIZE * i;
- reg->gpmc_bch_result1[i] = gpmc_base + GPMC_ECC_BCH_RESULT_1 +
- GPMC_BCH_SIZE * i;
- reg->gpmc_bch_result2[i] = gpmc_base + GPMC_ECC_BCH_RESULT_2 +
- GPMC_BCH_SIZE * i;
- reg->gpmc_bch_result3[i] = gpmc_base + GPMC_ECC_BCH_RESULT_3 +
- GPMC_BCH_SIZE * i;
- reg->gpmc_bch_result4[i] = gpmc_base + GPMC_ECC_BCH_RESULT_4 +
- i * GPMC_BCH_SIZE;
- reg->gpmc_bch_result5[i] = gpmc_base + GPMC_ECC_BCH_RESULT_5 +
- i * GPMC_BCH_SIZE;
- reg->gpmc_bch_result6[i] = gpmc_base + GPMC_ECC_BCH_RESULT_6 +
- i * GPMC_BCH_SIZE;
- }
-}
-
int gpmc_get_client_irq(unsigned irq_config)
{
int i;
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 707f6d5..a91fab7 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -211,7 +211,6 @@ extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
struct gpmc_settings *gpmc_s,
struct gpmc_device_timings *dev_t);
-extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
extern int gpmc_get_client_irq(unsigned irq_config);
extern unsigned int gpmc_ticks_to_ns(unsigned int ticks);
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 6b0f953..e894e65 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -163,7 +163,6 @@ struct omap_nand_info {
} iomode;
u_char *buf;
int buf_len;
- struct gpmc_nand_regs reg;
/* fields specific for BCHx_HW ECC scheme */
struct device *elm_dev;
struct device_node *of_node;
@@ -1547,7 +1546,6 @@ static int omap_nand_probe(struct platform_device *pdev)
info->pdev = pdev;
info->gpmc_cs = pdata->cs;
- info->reg = pdata->reg;
info->of_node = pdata->of_node;
info->ecc_opt = pdata->ecc_opt;
mtd = &info->mtd;
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 660c029..44198be 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -35,6 +35,7 @@ enum omap_ecc {
OMAP_ECC_BCH16_CODE_HW,
};
+/* Deprecated - Don't use */
struct gpmc_nand_regs {
void __iomem *gpmc_status;
void __iomem *gpmc_nand_command;
@@ -65,10 +66,12 @@ struct omap_nand_platform_data {
enum nand_io xfer_type;
int devsize;
enum omap_ecc ecc_opt;
- struct gpmc_nand_regs reg;
/* for passing the partitions */
struct device_node *of_node;
struct device_node *elm_of_node;
+
+
+ struct gpmc_nand_regs reg; /* Deprecated. Don't use */
};
#endif
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND
2014-07-09 12:37 [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Roger Quadros
` (9 preceding siblings ...)
2014-07-09 12:37 ` [RFC PATCH 10/10] OMAP: GPMC: NAND: Don't pass NAND/ECC/BCH register adresses to NAND driver Roger Quadros
@ 2014-07-11 6:52 ` Tony Lindgren
2014-07-11 7:27 ` Gupta, Pekon
10 siblings, 1 reply; 23+ messages in thread
From: Tony Lindgren @ 2014-07-11 6:52 UTC (permalink / raw)
To: Roger Quadros
Cc: computersforpeace, javier, pekon, ezequiel.garcia, dwmw2, jg1.han,
nsekhar, linux-mtd, linux-omap, linux-kernel
* Roger Quadros <rogerq@ti.com> [140709 05:39]:
> Hi,
>
> The following hardware modules/registers are meant for NAND controller driver
> usage:
> - NAND I/O control (NAND address, data, command registers)
> - Prefetch/Write-post engine
> - ECC/BCH engine
>
> However, these registers sit in the GPMC controller's register space and there
> need to be some sane way to access them from the OMAP NAND controller driver.
>
> Till now the GPMC driver was populating a data structure (struct gpmc_nand_regs)
> with the register addresses and passing it to the OMAP NAND driver via platform
> data. This mechanism cannot be used for true Device tree support as custom
> platform data passing mechanism doesn't seem to work. Moreover, direct
> access to these registers must be limited to the GPMC driver. This calls for
> a few custom OMAP GPMC specific APIs that the OMAP NAND driver can use
> to access these GPMC space registers.
>
> This series attempts to add the following new APIs and gets rid of
> 'struct gpmc_nand_regs' and 'gpmc_update_nand_regs()'.
>
> -For NAND I/O control registers
> u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
> void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
>
> -For Prefetch engine
> int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
> u32 count, int is_write);
> int omap_gpmc_prefetch_stop(int cs);
> u32 omap_gpmc_get_prefetch_count(void);
> u32 omap_gpmc_get_prefetch_fifo_count(void);
>
> -For ECC/BCH engine
> void omap_gpmc_ecc_disable(void);
> void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
> u8 ecc_size1, bool use_bch,
> enum omap_gpmc_bch_type bch_type,
> u8 bch_sectors, u8 bch_wrap_mode);
> void omap_gpmc_ecc_get_result(int length, u32 *result);
> void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result);
These seem fine to me. At least I don't have any better ideas to
expose these GPMC registers to the NAND driver(s).
> These APIs don't implement any logic to serialize access to the
> NAND/Prefetch/ECC registers. It is upto the NAND controller driver
> to ensure that. As these modules can only handle one NAND controller context
> at a time, we set the nand_chip->hwcontrol to point to a single
> controller instance even if there are multiple NAND chips on different
> Chip select spaces. The NAND base driver then takes care of serializing
> access to the NAND controller (and ECC) through nandchip->hwcontrol->lock.
>
> NOTE: Patches are still untested and only meant for review.
Regards,
Tony
^ permalink raw reply [flat|nested] 23+ messages in thread* RE: [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND
2014-07-11 6:52 ` [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND Tony Lindgren
@ 2014-07-11 7:27 ` Gupta, Pekon
2014-07-11 8:28 ` Roger Quadros
0 siblings, 1 reply; 23+ messages in thread
From: Gupta, Pekon @ 2014-07-11 7:27 UTC (permalink / raw)
To: Tony Lindgren, Quadros, Roger
Cc: computersforpeace@gmail.com, javier@dowhile0.org,
ezequiel.garcia@free-electrons.com, dwmw2@infradead.org,
jg1.han@samsung.com, Nori, Sekhar, linux-mtd@lists.infradead.org,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Roger,
>From: Tony Lindgren [mailto:tony@atomide.com]
>>* Roger Quadros <rogerq@ti.com> [140709 05:39]:
>> Hi,
>>
>> The following hardware modules/registers are meant for NAND controller driver
>> usage:
>> - NAND I/O control (NAND address, data, command registers)
>> - Prefetch/Write-post engine
>> - ECC/BCH engine
>>
>> However, these registers sit in the GPMC controller's register space and there
>> need to be some sane way to access them from the OMAP NAND controller driver.
>>
>> Till now the GPMC driver was populating a data structure (struct gpmc_nand_regs)
>> with the register addresses and passing it to the OMAP NAND driver via platform
>> data. This mechanism cannot be used for true Device tree support as custom
>> platform data passing mechanism doesn't seem to work. Moreover, direct
>> access to these registers must be limited to the GPMC driver. This calls for
>> a few custom OMAP GPMC specific APIs that the OMAP NAND driver can use
>> to access these GPMC space registers.
>>
>> This series attempts to add the following new APIs and gets rid of
>> 'struct gpmc_nand_regs' and 'gpmc_update_nand_regs()'.
>>
>> -For NAND I/O control registers
>> u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
>> void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
>>
>> -For Prefetch engine
>> int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
>> u32 count, int is_write);
>> int omap_gpmc_prefetch_stop(int cs);
>> u32 omap_gpmc_get_prefetch_count(void);
>> u32 omap_gpmc_get_prefetch_fifo_count(void);
>>
>> -For ECC/BCH engine
>> void omap_gpmc_ecc_disable(void);
>> void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
>> u8 ecc_size1, bool use_bch,
>> enum omap_gpmc_bch_type bch_type,
>> u8 bch_sectors, u8 bch_wrap_mode);
I think this one has too big argument list.
And also this interface will become inconsistent when you will expand the
NAND driver to support devices with larger page-size(like 8K NAND devices)
Why can't we just use
omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg);
as already defined above?
This is one-time configuration per read/write cycle so using
'omap_gpmc_write_reg()' shouldn't be much of issue. And this will
automatically plugin to current chip->ecc.hwctl() calls.
>> void omap_gpmc_ecc_get_result(int length, u32 *result);
Can you please rename it to "omap_gpmc_ecc_get_hamming_result()"
Just to keep it consistent with "omap_gpmc_ecc_get_bch_result()"
>> void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result);
>
This one looks good, but you should also take in int 'ecc-scheme'.
Actually you can just move omap_calculate_ecc_bch(...) out of NAND
driver into GPMC driver and rename it, because support of ECC
scheme is property of hardware controller not NAND driver.
What ecc-schemes GPMC controller supports should be inside GPMC driver,
and NAND driver should just attach them to appropriate interfaces. Right ?
Same way just think of moving chip->ecc.hwctl() callbacks implementations
out of NAND driver into GPMC driver. Then you would _not_ need to
export any GPMC registers into NAND driver.
with regards, pekon
>These seem fine to me. At least I don't have any better ideas to
>expose these GPMC registers to the NAND driver(s).
>
>> These APIs don't implement any logic to serialize access to the
>> NAND/Prefetch/ECC registers. It is upto the NAND controller driver
>> to ensure that. As these modules can only handle one NAND controller context
>> at a time, we set the nand_chip->hwcontrol to point to a single
>> controller instance even if there are multiple NAND chips on different
>> Chip select spaces. The NAND base driver then takes care of serializing
>> access to the NAND controller (and ECC) through nandchip->hwcontrol->lock.
>>
>> NOTE: Patches are still untested and only meant for review.
>
>Regards,
>
>Tony
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND
2014-07-11 7:27 ` Gupta, Pekon
@ 2014-07-11 8:28 ` Roger Quadros
2014-07-11 9:42 ` Gupta, Pekon
0 siblings, 1 reply; 23+ messages in thread
From: Roger Quadros @ 2014-07-11 8:28 UTC (permalink / raw)
To: Gupta, Pekon, Tony Lindgren
Cc: computersforpeace@gmail.com, javier@dowhile0.org,
ezequiel.garcia@free-electrons.com, dwmw2@infradead.org,
jg1.han@samsung.com, Nori, Sekhar, linux-mtd@lists.infradead.org,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Pekon,
On 07/11/2014 10:27 AM, Gupta, Pekon wrote:
> Hi Roger,
>
>> From: Tony Lindgren [mailto:tony@atomide.com]
>>> * Roger Quadros <rogerq@ti.com> [140709 05:39]:
>>> Hi,
>>>
>>> The following hardware modules/registers are meant for NAND controller driver
>>> usage:
>>> - NAND I/O control (NAND address, data, command registers)
>>> - Prefetch/Write-post engine
>>> - ECC/BCH engine
>>>
>>> However, these registers sit in the GPMC controller's register space and there
>>> need to be some sane way to access them from the OMAP NAND controller driver.
>>>
>>> Till now the GPMC driver was populating a data structure (struct gpmc_nand_regs)
>>> with the register addresses and passing it to the OMAP NAND driver via platform
>>> data. This mechanism cannot be used for true Device tree support as custom
>>> platform data passing mechanism doesn't seem to work. Moreover, direct
>>> access to these registers must be limited to the GPMC driver. This calls for
>>> a few custom OMAP GPMC specific APIs that the OMAP NAND driver can use
>>> to access these GPMC space registers.
>>>
>>> This series attempts to add the following new APIs and gets rid of
>>> 'struct gpmc_nand_regs' and 'gpmc_update_nand_regs()'.
>>>
>>> -For NAND I/O control registers
>>> u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
>>> void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
>>>
>>> -For Prefetch engine
>>> int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
>>> u32 count, int is_write);
>>> int omap_gpmc_prefetch_stop(int cs);
>>> u32 omap_gpmc_get_prefetch_count(void);
>>> u32 omap_gpmc_get_prefetch_fifo_count(void);
>>>
>>> -For ECC/BCH engine
>>> void omap_gpmc_ecc_disable(void);
>>> void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
>>> u8 ecc_size1, bool use_bch,
>>> enum omap_gpmc_bch_type bch_type,
>>> u8 bch_sectors, u8 bch_wrap_mode);
>
> I think this one has too big argument list.
> And also this interface will become inconsistent when you will expand the
> NAND driver to support devices with larger page-size(like 8K NAND devices)
> Why can't we just use
> omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg);
> as already defined above?
omap_gpmc_write_reg will not be optimal for reading larger result blocks
and does not create enough abstraction between the clearly separate blocks
i.e. prefetch engine and ecc engine.
> This is one-time configuration per read/write cycle so using
> 'omap_gpmc_write_reg()' shouldn't be much of issue. And this will
> automatically plugin to current chip->ecc.hwctl() calls.
>
hwctl() doesn't take care of ECC. Those are done by
chip->ecc.correct() and chip->ecc.calculate().
>
>
>>> void omap_gpmc_ecc_get_result(int length, u32 *result);
> Can you please rename it to "omap_gpmc_ecc_get_hamming_result()"
> Just to keep it consistent with "omap_gpmc_ecc_get_bch_result()"
>
OK. Sounds reasonable.
>>> void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result);
>>
> This one looks good, but you should also take in int 'ecc-scheme'.
Could you please explain why?
>
> Actually you can just move omap_calculate_ecc_bch(...) out of NAND
> driver into GPMC driver and rename it, because support of ECC
> scheme is property of hardware controller not NAND driver.
> What ecc-schemes GPMC controller supports should be inside GPMC driver,
> and NAND driver should just attach them to appropriate interfaces. Right ?
>
> Same way just think of moving chip->ecc.hwctl() callbacks implementations
> out of NAND driver into GPMC driver. Then you would _not_ need to
> export any GPMC registers into NAND driver.
I don't agree with moving anything nand_chip related (i.e. ecc.hwctl(), ecc.calculate()
or ecc.correct()) to GMPC driver because they are supposed to be implemented by
the NAND driver. ECC is a functionality of NAND controller not of GPMC controller.
It is just a IP design mistake that they smudged the ECC registers so badly
in the GPMC register space.
Could you please explain your view point as to why you want me to move these ecc.*()
to GPMC driver other than being extensively tested?
We can extensively test the new changes before merging any code to mainline as well.
cheers,
-roger
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND
2014-07-11 8:28 ` Roger Quadros
@ 2014-07-11 9:42 ` Gupta, Pekon
2014-07-11 10:23 ` Roger Quadros
0 siblings, 1 reply; 23+ messages in thread
From: Gupta, Pekon @ 2014-07-11 9:42 UTC (permalink / raw)
To: Quadros, Roger, Tony Lindgren
Cc: computersforpeace@gmail.com, javier@dowhile0.org,
ezequiel.garcia@free-electrons.com, dwmw2@infradead.org,
jg1.han@samsung.com, Nori, Sekhar, linux-mtd@lists.infradead.org,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org
>From: Quadros, Roger
>>On 07/11/2014 10:27 AM, Gupta, Pekon wrote:
>>> From: Tony Lindgren [mailto:tony@atomide.com]
>>>> * Roger Quadros <rogerq@ti.com> [140709 05:39]:
>>>> Hi,
>>>>
>>>> The following hardware modules/registers are meant for NAND controller driver
>>>> usage:
>>>> - NAND I/O control (NAND address, data, command registers)
>>>> - Prefetch/Write-post engine
>>>> - ECC/BCH engine
>>>>
>>>> However, these registers sit in the GPMC controller's register space and there
>>>> need to be some sane way to access them from the OMAP NAND controller driver.
>>>>
>>>> Till now the GPMC driver was populating a data structure (struct gpmc_nand_regs)
>>>> with the register addresses and passing it to the OMAP NAND driver via platform
>>>> data. This mechanism cannot be used for true Device tree support as custom
>>>> platform data passing mechanism doesn't seem to work. Moreover, direct
>>>> access to these registers must be limited to the GPMC driver. This calls for
>>>> a few custom OMAP GPMC specific APIs that the OMAP NAND driver can use
>>>> to access these GPMC space registers.
>>>>
>>>> This series attempts to add the following new APIs and gets rid of
>>>> 'struct gpmc_nand_regs' and 'gpmc_update_nand_regs()'.
>>>>
>>>> -For NAND I/O control registers
>>>> u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
>>>> void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
>>>>
>>>> -For Prefetch engine
>>>> int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
>>>> u32 count, int is_write);
>>>> int omap_gpmc_prefetch_stop(int cs);
>>>> u32 omap_gpmc_get_prefetch_count(void);
>>>> u32 omap_gpmc_get_prefetch_fifo_count(void);
>>>>
>>>> -For ECC/BCH engine
>>>> void omap_gpmc_ecc_disable(void);
>>>> void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
>>>> u8 ecc_size1, bool use_bch,
>>>> enum omap_gpmc_bch_type bch_type,
>>>> u8 bch_sectors, u8 bch_wrap_mode);
>>
>> I think this one has too big argument list.
>> And also this interface will become inconsistent when you will expand the
>> NAND driver to support devices with larger page-size(like 8K NAND devices)
>> Why can't we just use
>> omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg);
>> as already defined above?
>
>omap_gpmc_write_reg will not be optimal for reading larger result blocks
>and does not create enough abstraction between the clearly separate blocks
>i.e. prefetch engine and ecc engine.
>
>> This is one-time configuration per read/write cycle so using
>> 'omap_gpmc_write_reg()' shouldn't be much of issue. And this will
>> automatically plugin to current chip->ecc.hwctl() calls.
>>
>
>hwctl() doesn't take care of ECC. Those are done by
>chip->ecc.correct() and chip->ecc.calculate().
>
Incorrect assumption :-).
chip-ecc.hwctl() is used to configure ecc_size0 and ecc_size1, which in-turn
depend on ecc-scheme selected. Also, ecc_wrap_mode differs from
ecc-scheme for software and hardware implementations.
>>
>>
>>>> void omap_gpmc_ecc_get_result(int length, u32 *result);
>> Can you please rename it to "omap_gpmc_ecc_get_hamming_result()"
>> Just to keep it consistent with "omap_gpmc_ecc_get_bch_result()"
>>
>
>OK. Sounds reasonable.
>
>>>> void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result);
>>>
>> This one looks good, but you should also take in int 'ecc-scheme'.
>
>Could you please explain why?
>
>>
>> Actually you can just move omap_calculate_ecc_bch(...) out of NAND
>> driver into GPMC driver and rename it, because support of ECC
>> scheme is property of hardware controller not NAND driver.
>> What ecc-schemes GPMC controller supports should be inside GPMC driver,
>> and NAND driver should just attach them to appropriate interfaces. Right ?
>>
>> Same way just think of moving chip->ecc.hwctl() callbacks implementations
>> out of NAND driver into GPMC driver. Then you would _not_ need to
>> export any GPMC registers into NAND driver.
>
>I don't agree with moving anything nand_chip related (i.e. ecc.hwctl(), ecc.calculate()
>or ecc.correct()) to GMPC driver because they are supposed to be implemented by
>the NAND driver. ECC is a functionality of NAND controller not of GPMC controller.
>It is just a IP design mistake that they smudged the ECC registers so badly
>in the GPMC register space.
>
Not really, I don't think that's a problem. That's TI's way of looking at
controller architecture. If you read discussion of GPMI (freescale's
NAND controller) and NAND controllers from other vendors, there are
other complexities. Anyways that's not the discussion here, so converging back :-) ..
And, yes, you should _not_ touch chip->ecc.correct() or any other such
Implementations because those are NAND framework specific.
But below code isn't related to NAND framework, all NAND needs is how
you calcaluate the ECC, whether you do by
- reading GPMC registers or
- by using software bch library, or
- mixed of both.
Therefore, I suggest you to move following code from NAND driver to
GPMC driver, nothing other than. And this exported function does not
need any NAND framework information, except number_of_sectors.
@@omap_calculate_ecc_bch(...)
switch (info->ecc_opt) {
case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
case OMAP_ECC_BCH8_CODE_HW:
bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
bch_val3 = readl(gpmc_regs->gpmc_bch_result2[i]);
bch_val4 = readl(gpmc_regs->gpmc_bch_result3[i]);
*ecc_code++ = (bch_val4 & 0xFF);
*ecc_code++ = ((bch_val3 >> 24) & 0xFF);
*ecc_code++ = ((bch_val3 >> 16) & 0xFF);
*ecc_code++ = ((bch_val3 >> 8) & 0xFF);
*ecc_code++ = (bch_val3 & 0xFF);
*ecc_code++ = ((bch_val2 >> 24) & 0xFF);
*ecc_code++ = ((bch_val2 >> 16) & 0xFF);
*ecc_code++ = ((bch_val2 >> 8) & 0xFF);
*ecc_code++ = (bch_val2 & 0xFF);
*ecc_code++ = ((bch_val1 >> 24) & 0xFF);
*ecc_code++ = ((bch_val1 >> 16) & 0xFF);
*ecc_code++ = ((bch_val1 >> 8) & 0xFF);
*ecc_code++ = (bch_val1 & 0xFF);
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
case OMAP_ECC_BCH4_CODE_HW:
bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
*ecc_code++ = ((bch_val2 >> 12) & 0xFF);
*ecc_code++ = ((bch_val2 >> 4) & 0xFF);
*ecc_code++ = ((bch_val2 & 0xF) << 4) |
((bch_val1 >> 28) & 0xF);
*ecc_code++ = ((bch_val1 >> 20) & 0xFF);
*ecc_code++ = ((bch_val1 >> 12) & 0xFF);
*ecc_code++ = ((bch_val1 >> 4) & 0xFF);
*ecc_code++ = ((bch_val1 & 0xF) << 4);
break;
case OMAP_ECC_BCH16_CODE_HW:
val = readl(gpmc_regs->gpmc_bch_result6[i]);
ecc_code[0] = ((val >> 8) & 0xFF);
ecc_code[1] = ((val >> 0) & 0xFF);
val = readl(gpmc_regs->gpmc_bch_result5[i]);
ecc_code[2] = ((val >> 24) & 0xFF);
ecc_code[3] = ((val >> 16) & 0xFF);
ecc_code[4] = ((val >> 8) & 0xFF);
ecc_code[5] = ((val >> 0) & 0xFF);
val = readl(gpmc_regs->gpmc_bch_result4[i]);
ecc_code[6] = ((val >> 24) & 0xFF);
ecc_code[7] = ((val >> 16) & 0xFF);
ecc_code[8] = ((val >> 8) & 0xFF);
ecc_code[9] = ((val >> 0) & 0xFF);
val = readl(gpmc_regs->gpmc_bch_result3[i]);
ecc_code[10] = ((val >> 24) & 0xFF);
ecc_code[11] = ((val >> 16) & 0xFF);
ecc_code[12] = ((val >> 8) & 0xFF);
ecc_code[13] = ((val >> 0) & 0xFF);
val = readl(gpmc_regs->gpmc_bch_result2[i]);
ecc_code[14] = ((val >> 24) & 0xFF);
ecc_code[15] = ((val >> 16) & 0xFF);
ecc_code[16] = ((val >> 8) & 0xFF);
ecc_code[17] = ((val >> 0) & 0xFF);
val = readl(gpmc_regs->gpmc_bch_result1[i]);
ecc_code[18] = ((val >> 24) & 0xFF);
ecc_code[19] = ((val >> 16) & 0xFF);
ecc_code[20] = ((val >> 8) & 0xFF);
ecc_code[21] = ((val >> 0) & 0xFF);
val = readl(gpmc_regs->gpmc_bch_result0[i]);
ecc_code[22] = ((val >> 24) & 0xFF);
ecc_code[23] = ((val >> 16) & 0xFF);
ecc_code[24] = ((val >> 8) & 0xFF);
ecc_code[25] = ((val >> 0) & 0xFF);
break;
default:
return -EINVAL;
}
>Could you please explain your view point as to why you want me to move these ecc.*()
>to GPMC driver other than being extensively tested?
>
I'm not asking to move ecc.*() to GPMC .. Not at all.. As just explained above.
>We can extensively test the new changes before merging any code to mainline as well.
>
OMAP NAND driver is almost stable and feature complete in kernel-3.16.
I'm in support for cleaning interface between GPMC and NAND, But
keeping other changes like NAND driver optimizations out of current patch
series for now at-least.
with regards, pekon
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND
2014-07-11 9:42 ` Gupta, Pekon
@ 2014-07-11 10:23 ` Roger Quadros
2014-07-29 10:39 ` Tony Lindgren
0 siblings, 1 reply; 23+ messages in thread
From: Roger Quadros @ 2014-07-11 10:23 UTC (permalink / raw)
To: Gupta, Pekon, Tony Lindgren
Cc: computersforpeace@gmail.com, javier@dowhile0.org,
ezequiel.garcia@free-electrons.com, dwmw2@infradead.org,
jg1.han@samsung.com, Nori, Sekhar, linux-mtd@lists.infradead.org,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org
On 07/11/2014 12:42 PM, Gupta, Pekon wrote:
>> From: Quadros, Roger
>>> On 07/11/2014 10:27 AM, Gupta, Pekon wrote:
>>>> From: Tony Lindgren [mailto:tony@atomide.com]
>>>>> * Roger Quadros <rogerq@ti.com> [140709 05:39]:
>>>>> Hi,
>>>>>
>>>>> The following hardware modules/registers are meant for NAND controller driver
>>>>> usage:
>>>>> - NAND I/O control (NAND address, data, command registers)
>>>>> - Prefetch/Write-post engine
>>>>> - ECC/BCH engine
>>>>>
>>>>> However, these registers sit in the GPMC controller's register space and there
>>>>> need to be some sane way to access them from the OMAP NAND controller driver.
>>>>>
>>>>> Till now the GPMC driver was populating a data structure (struct gpmc_nand_regs)
>>>>> with the register addresses and passing it to the OMAP NAND driver via platform
>>>>> data. This mechanism cannot be used for true Device tree support as custom
>>>>> platform data passing mechanism doesn't seem to work. Moreover, direct
>>>>> access to these registers must be limited to the GPMC driver. This calls for
>>>>> a few custom OMAP GPMC specific APIs that the OMAP NAND driver can use
>>>>> to access these GPMC space registers.
>>>>>
>>>>> This series attempts to add the following new APIs and gets rid of
>>>>> 'struct gpmc_nand_regs' and 'gpmc_update_nand_regs()'.
>>>>>
>>>>> -For NAND I/O control registers
>>>>> u32 omap_gpmc_read_reg(int cs, enum omap_gpmc_reg reg);
>>>>> void omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg, u32 val);
>>>>>
>>>>> -For Prefetch engine
>>>>> int omap_gpmc_prefetch_start(int cs, int fifo_th, bool dma,
>>>>> u32 count, int is_write);
>>>>> int omap_gpmc_prefetch_stop(int cs);
>>>>> u32 omap_gpmc_get_prefetch_count(void);
>>>>> u32 omap_gpmc_get_prefetch_fifo_count(void);
>>>>>
>>>>> -For ECC/BCH engine
>>>>> void omap_gpmc_ecc_disable(void);
>>>>> void omap_gpmc_ecc_configure_enable(int cs, bool ecc16, u8 ecc_size0,
>>>>> u8 ecc_size1, bool use_bch,
>>>>> enum omap_gpmc_bch_type bch_type,
>>>>> u8 bch_sectors, u8 bch_wrap_mode);
>>>
>>> I think this one has too big argument list.
>>> And also this interface will become inconsistent when you will expand the
>>> NAND driver to support devices with larger page-size(like 8K NAND devices)
>>> Why can't we just use
>>> omap_gpmc_write_reg(int cs, enum omap_gpmc_reg reg);
>>> as already defined above?
>>
>> omap_gpmc_write_reg will not be optimal for reading larger result blocks
>> and does not create enough abstraction between the clearly separate blocks
>> i.e. prefetch engine and ecc engine.
>>
>>> This is one-time configuration per read/write cycle so using
>>> 'omap_gpmc_write_reg()' shouldn't be much of issue. And this will
>>> automatically plugin to current chip->ecc.hwctl() calls.
>>>
>>
>> hwctl() doesn't take care of ECC. Those are done by
>> chip->ecc.correct() and chip->ecc.calculate().
>>
> Incorrect assumption :-).
> chip-ecc.hwctl() is used to configure ecc_size0 and ecc_size1, which in-turn
> depend on ecc-scheme selected. Also, ecc_wrap_mode differs from
> ecc-scheme for software and hardware implementations.
>
Right. so hwctl(), calculate() and correct() go hand in hand and must be implemented
by the NAND controller driver.
>
>>>
>>>
>>>>> void omap_gpmc_ecc_get_result(int length, u32 *result);
>>> Can you please rename it to "omap_gpmc_ecc_get_hamming_result()"
>>> Just to keep it consistent with "omap_gpmc_ecc_get_bch_result()"
>>>
>>
>> OK. Sounds reasonable.
>>
>>>>> void omap_gpmc_ecc_get_bch_result(int length, u8 sector, u32 *result);
>>>>
>>> This one looks good, but you should also take in int 'ecc-scheme'.
>>
>> Could you please explain why?
>>
>>>
>>> Actually you can just move omap_calculate_ecc_bch(...) out of NAND
>>> driver into GPMC driver and rename it, because support of ECC
>>> scheme is property of hardware controller not NAND driver.
>>> What ecc-schemes GPMC controller supports should be inside GPMC driver,
>>> and NAND driver should just attach them to appropriate interfaces. Right ?
>>>
>>> Same way just think of moving chip->ecc.hwctl() callbacks implementations
>>> out of NAND driver into GPMC driver. Then you would _not_ need to
>>> export any GPMC registers into NAND driver.
>>
>> I don't agree with moving anything nand_chip related (i.e. ecc.hwctl(), ecc.calculate()
>> or ecc.correct()) to GMPC driver because they are supposed to be implemented by
>> the NAND driver. ECC is a functionality of NAND controller not of GPMC controller.
>> It is just a IP design mistake that they smudged the ECC registers so badly
>> in the GPMC register space.
>>
> Not really, I don't think that's a problem. That's TI's way of looking at
Moreover, that is TI's legacy stuff coming right from OMAP2/3 days.
> controller architecture. If you read discussion of GPMI (freescale's
> NAND controller) and NAND controllers from other vendors, there are
> other complexities. Anyways that's not the discussion here, so converging back :-) ..
>
> And, yes, you should _not_ touch chip->ecc.correct() or any other such
> Implementations because those are NAND framework specific.
> But below code isn't related to NAND framework, all NAND needs is how
> you calcaluate the ECC, whether you do by
> - reading GPMC registers or
> - by using software bch library, or
> - mixed of both.
> Therefore, I suggest you to move following code from NAND driver to
> GPMC driver, nothing other than. And this exported function does not
> need any NAND framework information, except number_of_sectors.
Well I you have still not pointed out what is the benefit of that approach or what are the problems with the approach I am suggesting.
I am suggesting that the API just return the ECC/BCH result registers as it is.
You are suggesting that the API do the calculations and return the NAND ready ECC buffer.
To do the calculations it needs to know about the ECC scheme which is NAND controller specific and this is unnecessary in the GPMC driver.
Tony, what is your opinion?
>
> @@omap_calculate_ecc_bch(...)
> switch (info->ecc_opt) {
> case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> case OMAP_ECC_BCH8_CODE_HW:
>
>
>> Could you please explain your view point as to why you want me to move these ecc.*()
>> to GPMC driver other than being extensively tested?
>>
> I'm not asking to move ecc.*() to GPMC .. Not at all.. As just explained above.
>
>> We can extensively test the new changes before merging any code to mainline as well.
>>
> OMAP NAND driver is almost stable and feature complete in kernel-3.16.
> I'm in support for cleaning interface between GPMC and NAND, But
> keeping other changes like NAND driver optimizations out of current patch
> series for now at-least.
I too support not breaking existing functionality. I didn't write this series with optimization in mind. Just establishing the APIs necessary.
cheers,
-roger
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH 00/10] OMAP: GPMC: NAND: Introduce GPMC APIs for OMAP NAND
2014-07-11 10:23 ` Roger Quadros
@ 2014-07-29 10:39 ` Tony Lindgren
0 siblings, 0 replies; 23+ messages in thread
From: Tony Lindgren @ 2014-07-29 10:39 UTC (permalink / raw)
To: Roger Quadros
Cc: Gupta, Pekon, computersforpeace@gmail.com, javier@dowhile0.org,
ezequiel.garcia@free-electrons.com, dwmw2@infradead.org,
jg1.han@samsung.com, Nori, Sekhar, linux-mtd@lists.infradead.org,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org
* Roger Quadros <rogerq@ti.com> [140711 03:25]:
> On 07/11/2014 12:42 PM, Gupta, Pekon wrote:
> >
> > Therefore, I suggest you to move following code from NAND driver to
> > GPMC driver, nothing other than. And this exported function does not
> > need any NAND framework information, except number_of_sectors.
>
> Well I you have still not pointed out what is the benefit of that approach or what are the problems with the approach I am suggesting.
>
> I am suggesting that the API just return the ECC/BCH result registers as it is.
> You are suggesting that the API do the calculations and return the NAND ready ECC buffer.
>
> To do the calculations it needs to know about the ECC scheme which is NAND controller specific and this is unnecessary in the GPMC driver.
>
> Tony, what is your opinion?
As long as the GPMC registers are exposed to drivers in a controlled
way and we have only one driver using the ECC registers, I don't care.
Regards,
Tony
^ permalink raw reply [flat|nested] 23+ messages in thread