| 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 |
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
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. =====================================================================
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
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. =====================================================================
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. =====================================================================
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
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. =====================================================================
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.
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 --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;
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(+)