| Message ID | 1568793387-25199-3-git-send-email-masonccyang@mxic.com.tw |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | [RFC,1/3] mtd: rawnand: Add support manufacturer postponed initialization | expand |
Hi Mason, Mason Yang <masonccyang@mxic.com.tw> wrote on Wed, 18 Sep 2019 15:56:26 +0800: > Macronix AD series support using power down command to > enter a minimum power consumption state. > > MTD default _suspend/_resume function replacement by > manufacturer postponed initialization. > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> > --- > drivers/mtd/nand/raw/nand_macronix.c | 78 +++++++++++++++++++++++++++++++++++- > 1 file changed, 77 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c > index 991c636..99a7b2e 100644 > --- a/drivers/mtd/nand/raw/nand_macronix.c > +++ b/drivers/mtd/nand/raw/nand_macronix.c > @@ -15,6 +15,8 @@ > #define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38 > #define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0 > > +#define NAND_CMD_POWER_DOWN 0xB9 > + > struct nand_onfi_vendor_macronix { > u8 reserved; > u8 reliability_func; > @@ -78,6 +80,12 @@ static void macronix_nand_onfi_init(struct nand_chip *chip) > "MX30UF4G28AC", > }; > > +static const char * const nand_power_down[] = { > + "MX30LF1G28AD", > + "MX30LF2G28AD", > + "MX30LF4G28AD", > +}; > + > static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip) > { > unsigned int i; > @@ -144,8 +152,64 @@ static int mxic_nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) > return ret; > } > > +int nand_power_down_op(struct nand_chip *chip) > +{ > + int ret; > + > + if (nand_has_exec_op(chip)) { > + struct nand_op_instr instrs[] = { > + NAND_OP_CMD(NAND_CMD_POWER_DOWN, 0), > + }; > + > + struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs); > + > + ret = nand_exec_op(chip, &op); > + if (ret) > + return ret; > + > + } else { > + chip->legacy.cmdfunc(chip, NAND_CMD_POWER_DOWN, -1, -1); > + } > + > + return 0; > +} > + > +static int mxic_nand_suspend(struct mtd_info *mtd) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + mutex_lock(&chip->lock); > + > + nand_select_target(chip, 0); > + nand_power_down_op(chip); > + nand_deselect_target(chip); > + > + chip->suspend = 1; > + mutex_unlock(&chip->lock); > + > + return 0; > +} > + > +static void mxic_nand_resume(struct mtd_info *mtd) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + mutex_lock(&chip->lock); > + // toggle #CS pin to resume NAND device C++ style comments are forbidden in code. > + nand_select_target(chip, 0); On several NAND controllers there is no way to act on the CS line without actually writing bytes to the NAND chip. So basically this is very likely to not work. > + ndelay(20); Is this delay known somewhere? Is this purely experimental? > + nand_deselect_target(chip); > + > + if (chip->suspend) > + chip->suspended = 0; > + else > + pr_err("%s call for a chip which is not in suspended state\n", > + __func__); > + mutex_unlock(&chip->lock); > +} > + > /* > - * Macronix AC series support using SET/GET_FEATURES to change > + * Macronix AC and AD series support using SET/GET_FEATURES to change > * Block Protection and Unprotection. > * > * MTD call-back function replacement by manufacturer postponed > @@ -163,6 +227,18 @@ static void macronix_nand_post_init(struct nand_chip *chip) > } > } > > + for (i = 0; i < ARRAY_SIZE(nand_power_down); i++) { > + if (!strcmp(nand_power_down[i], chip->parameters.model)) { > + blockprotected = 1; > + break; > + } > + } > + > + if (i < ARRAY_SIZE(nand_power_down)) { > + mtd->_suspend = mxic_nand_suspend; > + mtd->_resume = mxic_nand_resume; > + } > + > if (blockprotected && chip->parameters.supports_set_get_features) { > bitmap_set(chip->parameters.set_feature_list, > ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1); Thanks, Miquèl
Hi Miquel, > > +int nand_power_down_op(struct nand_chip *chip) > > +{ > > + int ret; > > + > > + if (nand_has_exec_op(chip)) { > > + struct nand_op_instr instrs[] = { > > + NAND_OP_CMD(NAND_CMD_POWER_DOWN, 0), > > + }; > > + > > + struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs); > > + > > + ret = nand_exec_op(chip, &op); > > + if (ret) > > + return ret; > > + > > + } else { > > + chip->legacy.cmdfunc(chip, NAND_CMD_POWER_DOWN, -1, -1); > > + } > > + > > + return 0; > > +} > > + > > +static int mxic_nand_suspend(struct mtd_info *mtd) > > +{ > > + struct nand_chip *chip = mtd_to_nand(mtd); > > + > > + mutex_lock(&chip->lock); > > + > > + nand_select_target(chip, 0); > > + nand_power_down_op(chip); > > + nand_deselect_target(chip); > > + > > + chip->suspend = 1; > > + mutex_unlock(&chip->lock); > > + > > + return 0; > > +} > > + > > +static void mxic_nand_resume(struct mtd_info *mtd) > > +{ > > + struct nand_chip *chip = mtd_to_nand(mtd); > > + > > + mutex_lock(&chip->lock); > > + // toggle #CS pin to resume NAND device > > C++ style comments are forbidden in code. okay, got it. thanks. > > > + nand_select_target(chip, 0); > > On several NAND controllers there is no way to act on the CS line > without actually writing bytes to the NAND chip. So basically this > is very likely to not work. any other way to make it work ? GPIO ? or just have some comments description here. i.e,. /* The NAND chip will exit the deep power down mode with #CS toggling, * please refer to datasheet for the timing requirement of tCRDP and tRDP. */ > > > + ndelay(20); > > Is this delay known somewhere? Is this purely experimental? it's timing requirement tCRDP 20 ns(min) to release device from deep power-down mode. You may download datasheet at https://www.macronix.com/zh-tw/products/NAND-Flash/SLC-NAND-Flash/Pages/spec.aspx?p=MX30LF4G28AD&m=SLC%20NAND&n=PM2579 > > > + nand_deselect_target(chip); > > + > > + if (chip->suspend) > > + chip->suspended = 0; > > + else > > + pr_err("%s call for a chip which is not in suspended state\n", > > + __func__); > > + mutex_unlock(&chip->lock); > > +} 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. =====================================================================
On Tue, 8 Oct 2019 10:06:50 +0800 masonccyang@mxic.com.tw wrote: > > > + nand_select_target(chip, 0); > > > > On several NAND controllers there is no way to act on the CS line > > without actually writing bytes to the NAND chip. So basically this > > is very likely to not work. > > any other way to make it work ? GPIO ? > or just have some comments description here. > i.e,. > > /* The NAND chip will exit the deep power down mode with #CS toggling, > * please refer to datasheet for the timing requirement of tCRDP and tRDP. > */ > Good luck with that. As Miquel said, on most NAND controllers select_target() is a dummy operation that just assigns nand_chip->target to the specified value but doesn't assert the CS line. You could send a dummy command here, like a READ_ID, but I guess you need CS to be asserted for at least 20ns before asserting any other signals (CLE/ALE) which might be an issue. > > > > > + ndelay(20); > > > > Is this delay known somewhere? Is this purely experimental? > > it's timing requirement tCRDP 20 ns(min) to release device > from deep power-down mode. > You may download datasheet at > https://www.macronix.com/zh-tw/products/NAND-Flash/SLC-NAND-Flash/Pages/spec.aspx?p=MX30LF4G28AD&m=SLC%20NAND&n=PM2579 Just looked at the datasheet, and there's actually more than tCRDP: - you have to make sure you entered power-down state for at least tDPDD before you try to wake up the device - the device goes back to stand-by state tRDP after the CS pin has been deasserted. I guess we can use ndelay() for those, since they happen before/after the CS pin is asserted/de-asserted. Be careful with ndelay() though, it's not guaranteed to wait the the time you pass, it can return before (maybe we should add a helper to deal with that). Another solution would be to describe CS assertion/de-assertion in the instruction flow, but that requires patching all exec_op() drivers. For the tCRDP timing, I think we should use a nand_operation, this way we can check if the controller is able to deal with dummy CS-assertion before entering deep-power mode. In order to do that you'll have to add a NAND_OP_DUMMY_INSTR (or NAND_OP_DELAY_INSTR), and then have something like: struct nand_op_instr instrs[] = { NAND_OP_DUMMY(tCRDP), };
Hi Boris, > > > > + nand_select_target(chip, 0); > > > > > > On several NAND controllers there is no way to act on the CS line > > > without actually writing bytes to the NAND chip. So basically this > > > is very likely to not work. > > > > any other way to make it work ? GPIO ? > > or just have some comments description here. > > i.e,. > > > > /* The NAND chip will exit the deep power down mode with #CS toggling, > > * please refer to datasheet for the timing requirement of tCRDP and tRDP. > > */ > > > > Good luck with that. As Miquel said, on most NAND controllers > select_target() is a dummy operation that just assigns nand_chip->target > to the specified value but doesn't assert the CS line. You could send a > dummy command here, like a READ_ID, but I guess you need CS to be > asserted for at least 20ns before asserting any other signals (CLE/ALE) > which might be an issue. okay, got it. But if possible, I think adding CS line control in nand_select_target() is a simple and generic way for MTD and all raw NAND controllers. > > > > > > > > + ndelay(20); > > > > > > Is this delay known somewhere? Is this purely experimental? > > > > it's timing requirement tCRDP 20 ns(min) to release device > > from deep power-down mode. > > You may download datasheet at > > https://www.macronix.com/zh-tw/products/NAND-Flash/SLC-NAND-Flash/Pages/ > spec.aspx?p=MX30LF4G28AD&m=SLC%20NAND&n=PM2579 > > Just looked at the datasheet, and there's actually more than tCRDP: > > - you have to make sure you entered power-down state for at least tDPDD > before you try to wake up the device > - the device goes back to stand-by state tRDP after the CS pin has been > deasserted. > > I guess we can use ndelay() for those, since they happen before/after > the CS pin is asserted/de-asserted. Be careful with ndelay() though, > it's not guaranteed to wait the the time you pass, it can return > before (maybe we should add a helper to deal with that). > Another solution would be to describe CS assertion/de-assertion in > the instruction flow, but that requires patching all exec_op() drivers. > > For the tCRDP timing, I think we should use a nand_operation, this way > we can check if the controller is able to deal with dummy CS-assertion > before entering deep-power mode. > In order to do that you'll have to add a NAND_OP_DUMMY_INSTR (or > NAND_OP_DELAY_INSTR), and then have something like: > > struct nand_op_instr instrs[] = { > NAND_OP_DUMMY(tCRDP), > }; got it. 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 Tue, 15 Oct 2019 10:33:29 +0800: > Hi Boris, > > > > > > > + nand_select_target(chip, 0); > > > > > > > > On several NAND controllers there is no way to act on the CS line > > > > without actually writing bytes to the NAND chip. So basically this > > > > is very likely to not work. > > > > > > any other way to make it work ? GPIO ? > > > or just have some comments description here. > > > i.e,. > > > > > > /* The NAND chip will exit the deep power down mode with #CS toggling, > > > > * please refer to datasheet for the timing requirement of tCRDP and > tRDP. > > > */ > > > > > > > Good luck with that. As Miquel said, on most NAND controllers > > select_target() is a dummy operation that just assigns nand_chip->target > > to the specified value but doesn't assert the CS line. You could send a > > dummy command here, like a READ_ID, but I guess you need CS to be > > asserted for at least 20ns before asserting any other signals (CLE/ALE) > > which might be an issue. > > okay, got it. > But if possible, I think adding CS line control in nand_select_target() > is a simple and generic way for MTD and all raw NAND controllers. The problem is not that we do not want to; the problem is that controllers are not capable of doing it reliably if no byte is sent over the NAND bus. Thanks, Miquèl
Hi Miquel, > > > > > > > > + nand_select_target(chip, 0); > > > > > > > > > > On several NAND controllers there is no way to act on the CS line > > > > > without actually writing bytes to the NAND chip. So basically this > > > > > is very likely to not work. > > > > > > > > any other way to make it work ? GPIO ? > > > > or just have some comments description here. > > > > i.e,. > > > > > > > > /* The NAND chip will exit the deep power down mode with #CS toggling, > > > > > > * please refer to datasheet for the timing requirement of tCRDP and > > tRDP. > > > > */ > > > > > > > > > > Good luck with that. As Miquel said, on most NAND controllers > > > select_target() is a dummy operation that just assigns nand_chip->target > > > to the specified value but doesn't assert the CS line. You could send a > > > dummy command here, like a READ_ID, but I guess you need CS to be > > > asserted for at least 20ns before asserting any other signals (CLE/ALE) > > > which might be an issue. > > > > okay, got it. > > But if possible, I think adding CS line control in nand_select_target() > > is a simple and generic way for MTD and all raw NAND controllers. > > The problem is not that we do not want to; the problem is that > controllers are not capable of doing it reliably if no byte is sent > over the NAND bus. okay, it's kind of pity even though our raw NAND controller is capable of doing it with no byte is sent over the NAND bus. As you mentioned that other controllers are not capable of doing it reliably if no byte is sent over the NAND bus. if so, does it work by adding a NAND_OP_DUMMY_INSTR ? (as Boris's comments) thanks for your time & comments. 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 Boris, > > Subject > > Re: [PATCH RFC 3/3] mtd: rawnand: Add support Macronix power down mode > > On Tue, 8 Oct 2019 10:06:50 +0800 > masonccyang@mxic.com.tw wrote: > > > > > + nand_select_target(chip, 0); > > > > > > On several NAND controllers there is no way to act on the CS line > > > without actually writing bytes to the NAND chip. So basically this > > > is very likely to not work. > > > > any other way to make it work ? GPIO ? > > or just have some comments description here. > > i.e,. > > > > /* The NAND chip will exit the deep power down mode with #CS toggling, > > * please refer to datasheet for the timing requirement of tCRDP and tRDP. > > */ > > > > Good luck with that. As Miquel said, on most NAND controllers > select_target() is a dummy operation that just assigns nand_chip->target > to the specified value but doesn't assert the CS line. You could send a > dummy command here, like a READ_ID, but I guess you need CS to be > asserted for at least 20ns before asserting any other signals (CLE/ALE) > which might be an issue. > I have validated & checked w/ designer that NANDs device just need CS# line toggling and don't care of CLE/WE#/RE#. That is raw NAND controller sending a command nand_status_op() or nand_power_down_op() will exit the deep power down too. The first command(just CS# line toggling) to NAND device in deep power down mode will not be recognized. We may publish the application notes or update the datasheet later. 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. =====================================================================
diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c index 991c636..99a7b2e 100644 --- a/drivers/mtd/nand/raw/nand_macronix.c +++ b/drivers/mtd/nand/raw/nand_macronix.c @@ -15,6 +15,8 @@ #define MXIC_BLOCK_PROTECTION_ALL_LOCK 0x38 #define MXIC_BLOCK_PROTECTION_ALL_UNLOCK 0x0 +#define NAND_CMD_POWER_DOWN 0xB9 + struct nand_onfi_vendor_macronix { u8 reserved; u8 reliability_func; @@ -78,6 +80,12 @@ static void macronix_nand_onfi_init(struct nand_chip *chip) "MX30UF4G28AC", }; +static const char * const nand_power_down[] = { + "MX30LF1G28AD", + "MX30LF2G28AD", + "MX30LF4G28AD", +}; + static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip) { unsigned int i; @@ -144,8 +152,64 @@ static int mxic_nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) return ret; } +int nand_power_down_op(struct nand_chip *chip) +{ + int ret; + + if (nand_has_exec_op(chip)) { + struct nand_op_instr instrs[] = { + NAND_OP_CMD(NAND_CMD_POWER_DOWN, 0), + }; + + struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs); + + ret = nand_exec_op(chip, &op); + if (ret) + return ret; + + } else { + chip->legacy.cmdfunc(chip, NAND_CMD_POWER_DOWN, -1, -1); + } + + return 0; +} + +static int mxic_nand_suspend(struct mtd_info *mtd) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + + mutex_lock(&chip->lock); + + nand_select_target(chip, 0); + nand_power_down_op(chip); + nand_deselect_target(chip); + + chip->suspend = 1; + mutex_unlock(&chip->lock); + + return 0; +} + +static void mxic_nand_resume(struct mtd_info *mtd) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + + mutex_lock(&chip->lock); + // toggle #CS pin to resume NAND device + nand_select_target(chip, 0); + ndelay(20); + nand_deselect_target(chip); + + if (chip->suspend) + chip->suspended = 0; + else + pr_err("%s call for a chip which is not in suspended state\n", + __func__); + mutex_unlock(&chip->lock); +} + /* - * Macronix AC series support using SET/GET_FEATURES to change + * Macronix AC and AD series support using SET/GET_FEATURES to change * Block Protection and Unprotection. * * MTD call-back function replacement by manufacturer postponed @@ -163,6 +227,18 @@ static void macronix_nand_post_init(struct nand_chip *chip) } } + for (i = 0; i < ARRAY_SIZE(nand_power_down); i++) { + if (!strcmp(nand_power_down[i], chip->parameters.model)) { + blockprotected = 1; + break; + } + } + + if (i < ARRAY_SIZE(nand_power_down)) { + mtd->_suspend = mxic_nand_suspend; + mtd->_resume = mxic_nand_resume; + } + if (blockprotected && chip->parameters.supports_set_get_features) { bitmap_set(chip->parameters.set_feature_list, ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
Macronix AD series support using power down command to enter a minimum power consumption state. MTD default _suspend/_resume function replacement by manufacturer postponed initialization. Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> --- drivers/mtd/nand/raw/nand_macronix.c | 78 +++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-)