2023-06-15 17:40:37

by Johan Jonker

[permalink] [raw]
Subject: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option

On Rockchip SoCs the first boot stages are written on NAND
with help of manufacturer software that uses a different format
then the MTD framework. Skip the automatic BBT scan with the
NAND_SKIP_BBTSCAN option so that the original content is unchanged
during the driver probe.
The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with
the nand_erase_nand() function and the flash_erase command.
With these options the user has the "freedom of choice" by neutral
access mode to read and write in whatever format is needed.

Signed-off-by: Johan Jonker <[email protected]>
---

Changes V3:
Change prefixes

Changed V2:
reword

---

I'm aware that the maintainer finds it "awful",
but it's absolute necessary to:
1: read/write boot blocks in user space without touching original content
2: format a NAND for MTD either with built in or external driver module

So we keep it include in this serie.
---
drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
index 5a0468034..fcda4c760 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -188,6 +188,10 @@ struct rk_nfc {
unsigned long assigned_cs;
};

+static int skipbbt;
+module_param(skipbbt, int, 0644);
+MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format.");
+
static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip)
{
return container_of(chip, struct rk_nfc_nand_chip, chip);
@@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc,

nand_set_controller_data(chip, nfc);

+ if (skipbbt)
+ chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK;
+
chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE;
chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;

--
2.30.2



2023-07-04 15:23:06

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option

Hi Johan,

[email protected] wrote on Thu, 15 Jun 2023 19:34:26 +0200:

> On Rockchip SoCs the first boot stages are written on NAND
> with help of manufacturer software that uses a different format
> then the MTD framework. Skip the automatic BBT scan with the
> NAND_SKIP_BBTSCAN option so that the original content is unchanged
> during the driver probe.
> The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with
> the nand_erase_nand() function and the flash_erase command.
> With these options the user has the "freedom of choice" by neutral
> access mode to read and write in whatever format is needed.

Can the boot_rom_mode thing help?

For writing this part you can disable the bad block protection using
debugfs and then write an externally processed file in raw mode I guess.

For the boot I think I proposed a DT property. I don't remember how far
the discussion went.

>
> Signed-off-by: Johan Jonker <[email protected]>
> ---
>
> Changes V3:
> Change prefixes
>
> Changed V2:
> reword
>
> ---
>
> I'm aware that the maintainer finds it "awful",
> but it's absolute necessary to:
> 1: read/write boot blocks in user space without touching original content
> 2: format a NAND for MTD either with built in or external driver module
>
> So we keep it include in this serie.
> ---
> drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
> index 5a0468034..fcda4c760 100644
> --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
> +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
> @@ -188,6 +188,10 @@ struct rk_nfc {
> unsigned long assigned_cs;
> };
>
> +static int skipbbt;
> +module_param(skipbbt, int, 0644);
> +MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format.");
> +
> static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip)
> {
> return container_of(chip, struct rk_nfc_nand_chip, chip);
> @@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc,
>
> nand_set_controller_data(chip, nfc);
>
> + if (skipbbt)
> + chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK;
> +
> chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE;
> chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
>
> --
> 2.30.2
>


Thanks,
Miquèl

2023-07-07 15:46:26

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option

Hi Miquel,

Some comments/questions below,

On 7/4/23 17:13, Miquel Raynal wrote:
> Hi Johan,
>
> [email protected] wrote on Thu, 15 Jun 2023 19:34:26 +0200:
>
>> On Rockchip SoCs the first boot stages are written on NAND

>> with help of manufacturer software that uses a different format
>> then the MTD framework. Skip the automatic BBT scan with the

Not only about the boot blocks.
The rest of the Nand is formatted as well by closed source.
It formats the location at the new BBT pages as well.

>> NAND_SKIP_BBTSCAN option so that the original content is unchanged
>> during the driver probe.
>> The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with
>> the nand_erase_nand() function and the flash_erase command.
>> With these options the user has the "freedom of choice" by neutral
>> access mode to read and write in whatever format is needed.
>

> Can the boot_rom_mode thing help?

This boot_rom_mode thing is for switching ECC mode in boot block pages and is not related to BBT pages.

>
> For writing this part you can disable the bad block protection using
> debugfs and then write an externally processed file in raw mode I guess.

The use of debugfs only might make sense when the driver can pass the probe function without errors.
It does however not save guard existing data when it creates and writes to a new BBT page location.
When the data at the new BBT pages previously is written with a different ECC or data/OOB format the hardware driver probe fails.

Your suggestion does not work for the Rockchip case.
Please advise what to do with this patch.

===

[ 2148.026403] calling rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] @ 2062
[ 2148.039156] rockchip-nfc 10500000.nand-controller: timing 11e1
[ 2148.056891] nand: device found, Manufacturer ID: 0xad, Chip ID: 0xde
[ 2148.067700] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit
[ 2148.076717] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, OOB size: 640
[ 2148.089022] rockchip-nfc 10500000.nand-controller: hynix_nand_init
[ 2148.099317] rockchip-nfc 10500000.nand-controller: h27ucg8t2atrbc_choose_interface_config
[ 2148.111779] rockchip-nfc 10500000.nand-controller: timing 11e1
[ 2148.129836] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575
[ 2148.149364] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error!
[ 2148.160742] rockchip-nfc 10500000.nand-controller: rd hw page: 1048319
[ 2148.180164] rockchip-nfc 10500000.nand-controller: read page: ffeff ecc error!
[ 2148.191413] rockchip-nfc 10500000.nand-controller: rd hw page: 1048063

[..]

Check for existing BBT pages.

[ 2148.339836] rockchip-nfc 10500000.nand-controller: read page: ffdff ecc error!
[ 2148.350864] rockchip-nfc 10500000.nand-controller: rd hw page: 1047807
[ 2148.369835] rockchip-nfc 10500000.nand-controller: read page: ffcff ecc error!
[ 2148.380597] Bad block table not found for chip 0
[ 2148.388479] Scanning device for bad blocks
[ 2148.395591] rockchip-nfc 10500000.nand-controller: rd hw page: 255
[ 2148.414087] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0

[..]

Check all pages to create a new BBT.

[ 2258.693279] rockchip-nfc 10500000.nand-controller: rd hw page: 1030143
[ 2258.710970] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0
[ 2258.720804] rockchip-nfc 10500000.nand-controller: rd hw page: 1030399
[ 2258.738394] rockchip-nfc 10500000.nand-controller: read page: fb8ff ecc error!
[ 2258.748229] Bad eraseblock 4024 at 0x0001f7000000

[..]

All BBT pages are marked bad, so it thinks there's no space left

[ 2261.403708] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575
[ 2261.422750] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error!
[ 2261.433634] Bad eraseblock 4095 at 0x0001ffe00000

[ 2261.441723] No space left to write bad block table

This is not done.
A Nand disk should not be altered if it's formatted in a different format unless an explicit command is given.

[ 2261.449860] nand_bbt: error while writing bad block table -28
[ 2261.467156] rockchip-nfc 10500000.nand-controller: failed to init NAND chips
[ 2261.477917] rockchip-nfc: probe of 10500000.nand-controller failed with error -28
[ 2261.497011] probe of 10500000.nand-controller returned 28 after 113456649 usecs
[ 2261.508273] initcall rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] returned 0 after 113468040 usecs

Probe failed, but there nothing wrong with hardware.
MTDx partitions are not created.
User space commands that rely strings like "/dev/mtd0" don't work.

===

Application
Kernel
Drivers
Hardware

Why do we fail a hardware driver probe when the level above doesn't deal with all real world situations.
Shouldn't that be more separated in MTD levels.

===

>
> For the boot I think I proposed a DT property. I don't remember how far
> the discussion went.

Is there a web link of that discussion?

How do I link 'compatible' to '/dev/mtd0' for example.
In a '/proc/mtd' entry?
See /drivers/mtd/spi-nor/debugfs.c


In order to write boot blocks in a pattern it needs to know the chip ID as used in nand_flash_ids[].
How can we export this ID to userspace?
Also in a '/proc/mtd' entry?

===

partitions {
compatible = "fixed-partitions";
#address-cells = <2>;
#size-cells = <2>;

partition@0 {
reg = <0x0 0x0 0x0 0x02000000>;
compatible = "rockchip,boot-blk";
label = "boot-blk";
};

partition@2000000 {
reg = <0x0 0x02000000 0x1 0xfa000000>;
label = "rootfs";
};

partition@1fc000000 {
reg = <0x1 0xfc000000 0x0 0x04000000>;
compatible = "mtd,bbt"
label = "bbt";
};
};

How many erase blocks is MTD using for BBT pages? 4 or 8 or ?

BBT pages are not clearly defined in the DT.
They are somehow marked bad in the overlapping partition.
How are suppose to erase BBT pages in a automated way?
Is there a need for a new BBT compatible?

What is your idea about compatibles suggestion:
rockchip,boot-blk
mtd,bbt

===

Johan

>
>>
>> Signed-off-by: Johan Jonker <[email protected]>
>> ---
>>
>> Changes V3:
>> Change prefixes
>>
>> Changed V2:
>> reword
>>
>> ---
>>
>> I'm aware that the maintainer finds it "awful",
>> but it's absolute necessary to:
>> 1: read/write boot blocks in user space without touching original content
>> 2: format a NAND for MTD either with built in or external driver module
>>
>> So we keep it include in this serie.
>> ---
>> drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
>> index 5a0468034..fcda4c760 100644
>> --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
>> +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
>> @@ -188,6 +188,10 @@ struct rk_nfc {
>> unsigned long assigned_cs;
>> };
>>
>> +static int skipbbt;
>> +module_param(skipbbt, int, 0644);
>> +MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format.");
>> +
>> static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip)
>> {
>> return container_of(chip, struct rk_nfc_nand_chip, chip);
>> @@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc,
>>
>> nand_set_controller_data(chip, nfc);
>>
>> + if (skipbbt)
>> + chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK;
>> +
>> chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE;
>> chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
>>
>> --
>> 2.30.2
>>
>
>
> Thanks,
> Miquèl

2023-07-12 14:04:13

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option

Hi Johan,

Richard, a question for you below :-)

[email protected] wrote on Fri, 7 Jul 2023 17:27:56 +0200:

> Hi Miquel,
>
> Some comments/questions below,

There are a lot, I've selected the ones which appear the most relevant
to me right now.

> On 7/4/23 17:13, Miquel Raynal wrote:
> > Hi Johan,
> >
> > [email protected] wrote on Thu, 15 Jun 2023 19:34:26 +0200:
> >
> >> On Rockchip SoCs the first boot stages are written on NAND
>
> >> with help of manufacturer software that uses a different format
> >> then the MTD framework. Skip the automatic BBT scan with the
>
> Not only about the boot blocks.
> The rest of the Nand is formatted as well by closed source.
> It formats the location at the new BBT pages as well.
>
> >> NAND_SKIP_BBTSCAN option so that the original content is unchanged
> >> during the driver probe.
> >> The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with
> >> the nand_erase_nand() function and the flash_erase command.
> >> With these options the user has the "freedom of choice" by neutral
> >> access mode to read and write in whatever format is needed.
> >
>
> > Can the boot_rom_mode thing help?
>
> This boot_rom_mode thing is for switching ECC mode in boot block pages and is not related to BBT pages.
>
> >
> > For writing this part you can disable the bad block protection using
> > debugfs and then write an externally processed file in raw mode I guess.
>
> The use of debugfs only might make sense when the driver can pass the probe function without errors.
> It does however not save guard existing data when it creates and writes to a new BBT page location.
> When the data at the new BBT pages previously is written with a different ECC or data/OOB format the hardware driver probe fails.
>
> Your suggestion does not work for the Rockchip case.
> Please advise what to do with this patch.
>
> ===
>
> [ 2148.026403] calling rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] @ 2062
> [ 2148.039156] rockchip-nfc 10500000.nand-controller: timing 11e1
> [ 2148.056891] nand: device found, Manufacturer ID: 0xad, Chip ID: 0xde
> [ 2148.067700] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit
> [ 2148.076717] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, OOB size: 640
> [ 2148.089022] rockchip-nfc 10500000.nand-controller: hynix_nand_init
> [ 2148.099317] rockchip-nfc 10500000.nand-controller: h27ucg8t2atrbc_choose_interface_config
> [ 2148.111779] rockchip-nfc 10500000.nand-controller: timing 11e1
> [ 2148.129836] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575
> [ 2148.149364] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error!
> [ 2148.160742] rockchip-nfc 10500000.nand-controller: rd hw page: 1048319
> [ 2148.180164] rockchip-nfc 10500000.nand-controller: read page: ffeff ecc error!
> [ 2148.191413] rockchip-nfc 10500000.nand-controller: rd hw page: 1048063
>
> [..]
>
> Check for existing BBT pages.
>
> [ 2148.339836] rockchip-nfc 10500000.nand-controller: read page: ffdff ecc error!
> [ 2148.350864] rockchip-nfc 10500000.nand-controller: rd hw page: 1047807
> [ 2148.369835] rockchip-nfc 10500000.nand-controller: read page: ffcff ecc error!
> [ 2148.380597] Bad block table not found for chip 0
> [ 2148.388479] Scanning device for bad blocks
> [ 2148.395591] rockchip-nfc 10500000.nand-controller: rd hw page: 255
> [ 2148.414087] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0
>
> [..]
>
> Check all pages to create a new BBT.
>
> [ 2258.693279] rockchip-nfc 10500000.nand-controller: rd hw page: 1030143
> [ 2258.710970] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0
> [ 2258.720804] rockchip-nfc 10500000.nand-controller: rd hw page: 1030399
> [ 2258.738394] rockchip-nfc 10500000.nand-controller: read page: fb8ff ecc error!
> [ 2258.748229] Bad eraseblock 4024 at 0x0001f7000000
>
> [..]
>
> All BBT pages are marked bad, so it thinks there's no space left
>
> [ 2261.403708] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575
> [ 2261.422750] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error!
> [ 2261.433634] Bad eraseblock 4095 at 0x0001ffe00000
>
> [ 2261.441723] No space left to write bad block table
>
> This is not done.
> A Nand disk should not be altered if it's formatted in a different format unless an explicit command is given.
>
> [ 2261.449860] nand_bbt: error while writing bad block table -28
> [ 2261.467156] rockchip-nfc 10500000.nand-controller: failed to init NAND chips
> [ 2261.477917] rockchip-nfc: probe of 10500000.nand-controller failed with error -28
> [ 2261.497011] probe of 10500000.nand-controller returned 28 after 113456649 usecs
> [ 2261.508273] initcall rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] returned 0 after 113468040 usecs
>
> Probe failed, but there nothing wrong with hardware.
> MTDx partitions are not created.
> User space commands that rely strings like "/dev/mtd0" don't work.
>
> ===
>
> Application
> Kernel
> Drivers
> Hardware
>
> Why do we fail a hardware driver probe when the level above doesn't deal with all real world situations.
> Shouldn't that be more separated in MTD levels.
>
> ===
>
> >
> > For the boot I think I proposed a DT property. I don't remember how far
> > the discussion went.
>
> Is there a web link of that discussion?

https://lore.kernel.org/linux-mtd/20230609104426.3901df54@xps-13/

Maybe the term "DT property" did not appear but that's what I had in
mind :-) I don't know if it must be a chip or a partition property.

Richard, here is where I would like your feedback, I am kind of opposed
to a "skip_bbt" module parameter. It's not a strong opinion, if you
think it's fine I'm open.

I would rather prefer a DT property which says "do not use the
standard pattern".

Johan, how are bad blocks supposed to be tracked if the bad block
tables are written in a way which does not allow Linux to read it?

> How do I link 'compatible' to '/dev/mtd0' for example.
> In a '/proc/mtd' entry?
> See /drivers/mtd/spi-nor/debugfs.c
>
>
> In order to write boot blocks in a pattern it needs to know the chip ID as used in nand_flash_ids[].
> How can we export this ID to userspace?

There was an attempt a few days back, it's not upstream, I can't find
it back ATM. I'll send it once I find it back.

> Also in a '/proc/mtd' entry?
>
> ===
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <2>;
> #size-cells = <2>;
>
> partition@0 {
> reg = <0x0 0x0 0x0 0x02000000>;
> compatible = "rockchip,boot-blk";
> label = "boot-blk";
> };
>
> partition@2000000 {
> reg = <0x0 0x02000000 0x1 0xfa000000>;
> label = "rootfs";
> };
>
> partition@1fc000000 {
> reg = <0x1 0xfc000000 0x0 0x04000000>;
> compatible = "mtd,bbt"
> label = "bbt";

This does not work, it would take me hours to explain why, it's all
about:
- stability
- how mtd has been implemented

> };
> };
>
> How many erase blocks is MTD using for BBT pages? 4 or 8 or ?

As many blocks are needed to cover the size of the device, times 2.

> BBT pages are not clearly defined in the DT.

No, because this is software, not hardware. MTD is a Linux thing, which
gives you access to an mtd device through a number of operations. It
handles bad blocks.

> They are somehow marked bad in the overlapping partition.

Not sure what "overlapping partition" means.

> How are suppose to erase BBT pages in a automated way?
> Is there a need for a new BBT compatible?

No, Linux has run with this standard pattern for 20 years, downstream
kernels sometimes make silly design decisions, we do not want to
support them.

> What is your idea about compatibles suggestion:
> rockchip,boot-blk
> mtd,bbt
>
> ===
>
> Johan
>
> >
> >>
> >> Signed-off-by: Johan Jonker <[email protected]>
> >> ---
> >>
> >> Changes V3:
> >> Change prefixes
> >>
> >> Changed V2:
> >> reword
> >>
> >> ---
> >>
> >> I'm aware that the maintainer finds it "awful",
> >> but it's absolute necessary to:
> >> 1: read/write boot blocks in user space without touching original content
> >> 2: format a NAND for MTD either with built in or external driver module
> >>
> >> So we keep it include in this serie.
> >> ---
> >> drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
> >> index 5a0468034..fcda4c760 100644
> >> --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
> >> +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
> >> @@ -188,6 +188,10 @@ struct rk_nfc {
> >> unsigned long assigned_cs;
> >> };
> >>
> >> +static int skipbbt;
> >> +module_param(skipbbt, int, 0644);
> >> +MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format.");
> >> +
> >> static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip)
> >> {
> >> return container_of(chip, struct rk_nfc_nand_chip, chip);
> >> @@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc,
> >>
> >> nand_set_controller_data(chip, nfc);
> >>
> >> + if (skipbbt)
> >> + chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK;
> >> +
> >> chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE;
> >> chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
> >>
> >> --
> >> 2.30.2
> >>
> >
> >
> > Thanks,
> > Miquèl


Thanks,
Miquèl

2023-07-13 15:29:51

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option

----- Ursprüngliche Mail -----
>
>> > For the boot I think I proposed a DT property. I don't remember how far
>> > the discussion went.
>>
>> Is there a web link of that discussion?
>
> https://lore.kernel.org/linux-mtd/20230609104426.3901df54@xps-13/
>
> Maybe the term "DT property" did not appear but that's what I had in
> mind :-) I don't know if it must be a chip or a partition property.
>
> Richard, here is where I would like your feedback, I am kind of opposed
> to a "skip_bbt" module parameter. It's not a strong opinion, if you
> think it's fine I'm open.
>
> I would rather prefer a DT property which says "do not use the
> standard pattern".

Yes, please no new module parameters. A module parameter should only affect
the behavior of the driver itself (e.g. debug mode) but not be device specific.
We made this mistake in the past too often. :-(
A single driver can serve more devices. Therefore IMHO controlling this via DT
makes perfectly sense.

Thanks,
//richard