2019-08-20 05:30:18

by Mason Yang

[permalink] [raw]
Subject: [PATCH] Add support for Macronix NAND randomizer

Macronix NANDs support randomizer operation for user data scrambled,
which can be enabled with a SET_FEATURE.

User data written to the NAND device without randomizer is still readable
after randomizer function enabled.
The penalty of randomizer are NOP = 1 instead of NOP = 4 and more time period
is needed in program operation and entering deep power-down mode.
i.e., tPROG 300us to 340us(randomizer enabled)

If subpage write not available with hardware ECC, for example,
NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
randomizer function is recommended for high-reliability.
Driver checks byte 167 of Vendor Blocks in ONFI parameter page table
to see if this high-reliability function is supported.

Signed-off-by: Mason Yang <[email protected]>
---
drivers/mtd/nand/raw/nand_macronix.c | 54 ++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 58511ae..b8b5bcb 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -11,6 +11,13 @@
#define MACRONIX_READ_RETRY_BIT BIT(0)
#define MACRONIX_NUM_READ_RETRY_MODES 6

+#define MACRONIX_RANDOMIZER_BIT BIT(1)
+#define ONFI_FEATURE_ADDR_MXIC_RANDOMIZER 0xB0
+#define MACRONIX_RANDOMIZER_ENPGM BIT(0)
+#define MACRONIX_RANDOMIZER_RANDEN BIT(1)
+#define MACRONIX_RANDOMIZER_RANDOPT BIT(2)
+#define MACRONIX_RANDOMIZER_MODE_EXIT 0
+
struct nand_onfi_vendor_macronix {
u8 reserved;
u8 reliability_func;
@@ -29,6 +36,42 @@ static int macronix_nand_setup_read_retry(struct nand_chip *chip, int mode)
return nand_set_features(chip, ONFI_FEATURE_ADDR_READ_RETRY, feature);
}

+static void macronix_nand_randomizer_check_enable(struct nand_chip *chip)
+{
+ u8 feature[ONFI_SUBFEATURE_PARAM_LEN];
+ int ret;
+
+ ret = nand_get_features(chip, ONFI_FEATURE_ADDR_MXIC_RANDOMIZER,
+ feature);
+ if (feature[0]) {
+ pr_info("Macronix NAND randomizer enabled:0x%x\n", feature[0]);
+ return;
+ }
+
+ feature[0] = MACRONIX_RANDOMIZER_ENPGM | MACRONIX_RANDOMIZER_RANDEN |
+ MACRONIX_RANDOMIZER_RANDOPT;
+ ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_RANDOMIZER,
+ feature);
+ if (ret)
+ goto err;
+
+ feature[0] = 0x0;
+ ret = nand_prog_page_op(chip, 0, 0, feature, 1);
+ if (ret)
+ goto err;
+
+ feature[0] = MACRONIX_RANDOMIZER_MODE_EXIT;
+ ret = nand_set_features(chip, ONFI_FEATURE_ADDR_MXIC_RANDOMIZER,
+ feature);
+ if (ret)
+ goto err;
+
+ pr_info("Macronix NAND randomizer enable ok\n");
+ return;
+err:
+ pr_err("Macronix NAND randomizer enable failed\n");
+}
+
static void macronix_nand_onfi_init(struct nand_chip *chip)
{
struct nand_parameters *p = &chip->parameters;
@@ -38,6 +81,17 @@ static void macronix_nand_onfi_init(struct nand_chip *chip)
return;

mxic = (struct nand_onfi_vendor_macronix *)p->onfi->vendor;
+ if (chip->options & NAND_NO_SUBPAGE_WRITE &&
+ mxic->reliability_func & MACRONIX_RANDOMIZER_BIT) {
+ if (p->supports_set_get_features) {
+ bitmap_set(p->set_feature_list,
+ ONFI_FEATURE_ADDR_MXIC_RANDOMIZER, 1);
+ bitmap_set(p->get_feature_list,
+ ONFI_FEATURE_ADDR_MXIC_RANDOMIZER, 1);
+ macronix_nand_randomizer_check_enable(chip);
+ }
+ }
+
if ((mxic->reliability_func & MACRONIX_READ_RETRY_BIT) == 0)
return;

--
1.9.1


2019-08-24 11:04:48

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer

Hi Mason,

Mason Yang <[email protected]> wrote on Tue, 20 Aug 2019 13:53:48
+0800:

> Macronix NANDs support randomizer operation for user data scrambled,
> which can be enabled with a SET_FEATURE.
>
> User data written to the NAND device without randomizer is still readable
> after randomizer function enabled.
> The penalty of randomizer are NOP = 1 instead of NOP = 4 and more time period

please don't use 'NOP' here, use 'subpage accesses' instead, otherwise
people might not understand what it means while it has a real impact.

> is needed in program operation and entering deep power-down mode.
> i.e., tPROG 300us to 340us(randomizer enabled)
>
> If subpage write not available with hardware ECC, for example,
> NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> randomizer function is recommended for high-reliability.
> Driver checks byte 167 of Vendor Blocks in ONFI parameter page table
> to see if this high-reliability function is supported.
>

You did not flagged this patch as a v2 and forgot about the changelog.
You did not listen to our comments in the last version neither. I was
open to a solution with a specific DT property for warned users but I
don't see it coming.


Thanks,
Miquèl

2019-08-26 02:54:19

by Mason Yang

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer


Hi Miquel,
>
> Mason Yang <[email protected]> wrote on Tue, 20 Aug 2019 13:53:48
> +0800:
>
> > Macronix NANDs support randomizer operation for user data scrambled,
> > which can be enabled with a SET_FEATURE.
> >
> > User data written to the NAND device without randomizer is still
readable
> > after randomizer function enabled.
> > The penalty of randomizer are NOP = 1 instead of NOP = 4 and more time
period
>
> please don't use 'NOP' here, use 'subpage accesses' instead, otherwise
> people might not understand what it means while it has a real impact.
>

okay, understood.
will fix it by next submitting.

> > is needed in program operation and entering deep power-down mode.
> > i.e., tPROG 300us to 340us(randomizer enabled)
> >
> > If subpage write not available with hardware ECC, for example,
> > NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> > randomizer function is recommended for high-reliability.
> > Driver checks byte 167 of Vendor Blocks in ONFI parameter page table
> > to see if this high-reliability function is supported.
> >
>
> You did not flagged this patch as a v2 and forgot about the changelog.

will fix, thank you.

> You did not listen to our comments in the last version neither. I was
> open to a solution with a specific DT property for warned users but I
> don't see it coming.

Sorry I missed the previous version of "read-retry and randomizer support"
patch.
Specific DT property is a good method to control it.

For more high-reliability concern, randomizer is recommended to enable by
default,
but sub-page write is not allowed when randomizer is enabled.

Since most of HW ECC did not support sub-page write and we think driver to
check
chip options flags is another simple and good way to enable randomizer.

>
>
> Thanks,
> Miqu?l

thanks for your time and comments.

best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

2019-08-26 09:26:01

by Mason Yang

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer


Hi Miquel,

>
> Re: [PATCH] Add support for Macronix NAND randomizer
>
> Hi Mason,
>
> [email protected] wrote on Mon, 26 Aug 2019 10:52:31 +0800:
>
> > Hi Miquel,
> > >
> > > Mason Yang <[email protected]> wrote on Tue, 20 Aug 2019
13:53:48
> > > +0800:
> > >
> > > > Macronix NANDs support randomizer operation for user data
scrambled,
> > > > which can be enabled with a SET_FEATURE.
> > > >
> > > > User data written to the NAND device without randomizer is still
> > readable
> > > > after randomizer function enabled.
> > > > The penalty of randomizer are NOP = 1 instead of NOP = 4 and more
time
> > period
> > >
> > > please don't use 'NOP' here, use 'subpage accesses' instead,
otherwise
> > > people might not understand what it means while it has a real
impact.
> > >
> >
> > okay, understood.
> > will fix it by next submitting.
> >
> > > > is needed in program operation and entering deep power-down mode.
> > > > i.e., tPROG 300us to 340us(randomizer enabled)
> > > >
> > > > If subpage write not available with hardware ECC, for example,
> > > > NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> > > > randomizer function is recommended for high-reliability.
> > > > Driver checks byte 167 of Vendor Blocks in ONFI parameter page
table
> > > > to see if this high-reliability function is supported.
> > > >
> > >
> > > You did not flagged this patch as a v2 and forgot about the
changelog.
> >
> > will fix, thank you.
> >
> > > You did not listen to our comments in the last version neither. I
was
> > > open to a solution with a specific DT property for warned users but
I
> > > don't see it coming.
> >
> > Sorry I missed the previous version of "read-retry and randomizer
support"
> > patch.
> > Specific DT property is a good method to control it.
> >
> > For more high-reliability concern, randomizer is recommended to enable
by
> > default,
> > but sub-page write is not allowed when randomizer is enabled.
> >
> > Since most of HW ECC did not support sub-page write and we think
driver to
> > check
> > chip options flags is another simple and good way to enable
randomizer.
>
> Sorry but this is wrong. Several controllers and NAND chips support
> subpages. And changing now the behavior with such chips would entirely
> break the concerned setups (see Boris answer about UBI complaining if
> the subpage size changes).

okay, I see.
thanks for your information.

I will patch it based on your comments in the last version.

>
> Thanks,
> Miqu?l

best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

2019-08-26 09:43:36

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer

Hi Mason,

[email protected] wrote on Mon, 26 Aug 2019 10:52:31 +0800:

> Hi Miquel,
> >
> > Mason Yang <[email protected]> wrote on Tue, 20 Aug 2019 13:53:48
> > +0800:
> >
> > > Macronix NANDs support randomizer operation for user data scrambled,
> > > which can be enabled with a SET_FEATURE.
> > >
> > > User data written to the NAND device without randomizer is still
> readable
> > > after randomizer function enabled.
> > > The penalty of randomizer are NOP = 1 instead of NOP = 4 and more time
> period
> >
> > please don't use 'NOP' here, use 'subpage accesses' instead, otherwise
> > people might not understand what it means while it has a real impact.
> >
>
> okay, understood.
> will fix it by next submitting.
>
> > > is needed in program operation and entering deep power-down mode.
> > > i.e., tPROG 300us to 340us(randomizer enabled)
> > >
> > > If subpage write not available with hardware ECC, for example,
> > > NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> > > randomizer function is recommended for high-reliability.
> > > Driver checks byte 167 of Vendor Blocks in ONFI parameter page table
> > > to see if this high-reliability function is supported.
> > >
> >
> > You did not flagged this patch as a v2 and forgot about the changelog.
>
> will fix, thank you.
>
> > You did not listen to our comments in the last version neither. I was
> > open to a solution with a specific DT property for warned users but I
> > don't see it coming.
>
> Sorry I missed the previous version of "read-retry and randomizer support"
> patch.
> Specific DT property is a good method to control it.
>
> For more high-reliability concern, randomizer is recommended to enable by
> default,
> but sub-page write is not allowed when randomizer is enabled.
>
> Since most of HW ECC did not support sub-page write and we think driver to
> check
> chip options flags is another simple and good way to enable randomizer.

Sorry but this is wrong. Several controllers and NAND chips support
subpages. And changing now the behavior with such chips would entirely
break the concerned setups (see Boris answer about UBI complaining if
the subpage size changes).

Thanks,
Miquèl

2019-08-29 09:10:53

by Mason Yang

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer


Hi Miquel,


> >
> > If subpage write not available with hardware ECC, for example,
> > NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> > randomizer function is recommended for high-reliability.
> > Driver checks byte 167 of Vendor Blocks in ONFI parameter page table
> > to see if this high-reliability function is supported.
> >
>
> You did not flagged this patch as a v2 and forgot about the changelog.
> You did not listen to our comments in the last version neither. I was
> open to a solution with a specific DT property for warned users but I
> don't see it coming.
>

Based on your comments by specific DT property for randomizer support.
to add a new property in children nodes:

i.e,.

nand: nand-controller@43c30000 {

nand@0 {
reg = <0>;
nand-reliability = "randomizer";
};
};


file of nand_macronix.c will patch to:

static void macronix_nand_onfi_init(struct nand_chip *chip)
{
struct nand_parameters *p = &chip->parameters;
struct device_node *dn = nand_get_flash_node(chip);
const char *pm;
int rand_enable = 0;

ret = of_property_read_string(dn, "nand-reliability", &pm);
if (!ret) {
if (!strcasecmp(pm, "randomizer"));
rand_enable = 1;
}

mxic = (struct nand_onfi_vendor_macronix *)p->onfi->vendor;
if (rand_enable &&
mxic->reliability_func & MACRONIX_RANDOMIZER_BIT) {
if (p->supports_set_get_features) {
bitmap_set(p->set_feature_list,
ONFI_FEATURE_ADDR_MXIC_RANDOMIZER, 1);
bitmap_set(p->get_feature_list,
ONFI_FEATURE_ADDR_MXIC_RANDOMIZER, 1);
/* set-up chip options with NAND_NO_SUBPAGE_WRITE
*/
chip->options |= NAND_NO_SUBPAGE_WRITE;
macronix_nand_randomizer_check_enable(chip);
}
}

}

something like this,

is it OK ?

thanks & best regards,
Mason


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

2019-08-30 09:53:51

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer

Hi Mason,

[email protected] wrote on Thu, 29 Aug 2019 17:07:51 +0800:

> Hi Miquel,
>
>
> > >
> > > If subpage write not available with hardware ECC, for example,
> > > NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> > > randomizer function is recommended for high-reliability.
> > > Driver checks byte 167 of Vendor Blocks in ONFI parameter page table
> > > to see if this high-reliability function is supported.
> > >
> >
> > You did not flagged this patch as a v2 and forgot about the changelog.
> > You did not listen to our comments in the last version neither. I was
> > open to a solution with a specific DT property for warned users but I
> > don't see it coming.
> >
>
> Based on your comments by specific DT property for randomizer support.
> to add a new property in children nodes:
>
> i.e,.
>
> nand: nand-controller@43c30000 {
>
> nand@0 {
> reg = <0>;
> nand-reliability = "randomizer";

mxic,enable-randomizer-otp;

Would be better (with the proper documentation in the bindings).


Thanks,
Miquèl

2019-09-02 07:04:23

by Mason Yang

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer


Hi Miquel,


> > > >
> > > > If subpage write not available with hardware ECC, for example,
> > > > NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> > > > randomizer function is recommended for high-reliability.
> > > > Driver checks byte 167 of Vendor Blocks in ONFI parameter page
table
> > > > to see if this high-reliability function is supported.
> > > >
> > >
> > > You did not flagged this patch as a v2 and forgot about the
changelog.
> > > You did not listen to our comments in the last version neither. I
was
> > > open to a solution with a specific DT property for warned users but
I
> > > don't see it coming.
> > >
> >
> > Based on your comments by specific DT property for randomizer support.
> > to add a new property in children nodes:
> >
> > i.e,.
> >
> > nand: nand-controller@43c30000 {
> >
> > nand@0 {
> > reg = <0>;
> > nand-reliability = "randomizer";
>
> mxic,enable-randomizer-otp;
>
> Would be better (with the proper documentation in the bindings).
>

okay, thanks for your opinions.

best regards,
Mason


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

2019-09-02 07:07:11

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer

On Mon, Sep 2, 2019 at 8:54 AM <[email protected]> wrote:
> > > nand@0 {
> > > reg = <0>;
> > > nand-reliability = "randomizer";
> >
> > mxic,enable-randomizer-otp;
> >
> > Would be better (with the proper documentation in the bindings).
> >
>
> okay, thanks for your opinions.

Please document also when/why one wants to enable the randomizer.

--
Thanks,
//richard

2019-09-02 07:40:50

by Mason Yang

[permalink] [raw]
Subject: Re: [PATCH] Add support for Macronix NAND randomizer


Hi Richard,

> Subject
>
> Re: [PATCH] Add support for Macronix NAND randomizer
>
> On Mon, Sep 2, 2019 at 8:54 AM <[email protected]> wrote:
> > > > nand@0 {
> > > > reg = <0>;
> > > > nand-reliability = "randomizer";
> > >
> > > mxic,enable-randomizer-otp;
> > >
> > > Would be better (with the proper documentation in the bindings).
> > >
> >
> > okay, thanks for your opinions.
>
> Please document also when/why one wants to enable the randomizer.

okay, sure.

>
> --
> Thanks,
> //richard

best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================