diff mbox series

Add support for Macronix NAND randomizer

Message ID 1566280428-4159-1-git-send-email-masonccyang@mxic.com.tw
State Changes Requested
Delegated to: Miquel Raynal
Headers show
Series Add support for Macronix NAND randomizer | expand

Commit Message

Mason Yang Aug. 20, 2019, 5:53 a.m. UTC
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 <masonccyang@mxic.com.tw>
---
 drivers/mtd/nand/raw/nand_macronix.c | 54 ++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

Comments

Miquel Raynal Aug. 24, 2019, 11:03 a.m. UTC | #1
Hi Mason,

Mason Yang <masonccyang@mxic.com.tw> 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
Mason Yang Aug. 26, 2019, 2:52 a.m. UTC | #2
Hi Miquel,
> 
> Mason Yang <masonccyang@mxic.com.tw> 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.

=====================================================================
Miquel Raynal Aug. 26, 2019, 7:23 a.m. UTC | #3
Hi Mason,

masonccyang@mxic.com.tw wrote on Mon, 26 Aug 2019 10:52:31 +0800:

> Hi Miquel,
> > 
> > Mason Yang <masonccyang@mxic.com.tw> 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
Mason Yang Aug. 26, 2019, 9:24 a.m. UTC | #4
Hi Miquel,

> 
> Re: [PATCH] Add support for Macronix NAND randomizer
> 
> Hi Mason,
> 
> masonccyang@mxic.com.tw wrote on Mon, 26 Aug 2019 10:52:31 +0800:
> 
> > Hi Miquel,
> > > 
> > > Mason Yang <masonccyang@mxic.com.tw> 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.

=====================================================================
Mason Yang Aug. 29, 2019, 9:07 a.m. UTC | #5
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.

=====================================================================
Miquel Raynal Aug. 30, 2019, 9:51 a.m. UTC | #6
Hi Mason,

masonccyang@mxic.com.tw 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
Mason Yang Sept. 2, 2019, 6:53 a.m. UTC | #7
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.

=====================================================================
Richard Weinberger Sept. 2, 2019, 7:05 a.m. UTC | #8
On Mon, Sep 2, 2019 at 8:54 AM <masonccyang@mxic.com.tw> 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.
Mason Yang Sept. 2, 2019, 7:39 a.m. UTC | #9
Hi Richard,
 
> Subject
> 
> Re: [PATCH] Add support for Macronix NAND randomizer
> 
> On Mon, Sep 2, 2019 at 8:54 AM <masonccyang@mxic.com.tw> 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.

=====================================================================
diff mbox series

Patch

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;