Hi Miquel, > > > > > > +static void mxic_nand_select_chip(struct nand_chip *chip, int > > chipnr) > > > > > > > > > > _select_target() is preferred now > > > > > > > > Do you mean I implement mxic_nand_select_target() to control #CS ? > > > > > > > > If so, I need to call mxic_nand_select_target( ) to control #CS ON > > > > and then #CS OFF in _exec_op() due to nand_select_target()<in > > nand_base,c> > > > > is still calling chip->legacy.select_chip ? > > > > > > You must forget about the ->select_chip() callback. Now it should be > > > handled directly from the controller driver. Please have a look at the > > > commit pointed against the marvell_nand.c driver. > > > > I have no Marvell NFC datasheet and have one question. > > > > In marvell_nand.c, there is no xxx_deselect_target() or > > something like that doing #CS OFF. > > marvell_nfc_select_target() seems always to make one of chip or die > > #CS keep low. > > > > Is it right ? > > Yes, AFAIR there is no "de-assert" mechanism in this controller. > > > > > How to make all #CS keep high for NAND to enter > > low-power standby mode if driver don't use "legacy.select_chip()" ? > > See commit 02b4a52604a4 ("mtd: rawnand: Make ->select_chip() optional > when ->exec_op() is implemented") which states: > > "When [->select_chip() is] not implemented, the core is assuming > the CS line is automatically asserted/deasserted by the driver > ->exec_op() implementation." > > Of course, the above is right only when the controller driver supports > the ->exec_op() interface. Currently, it seems that we will get the incorrect data and error operation due to CS in error toggling if CS line is controlled in ->exec_op(). i.e,. 1) In nand_onfi_detect() to call nand_exec_op() twice by nand_read_param_page_op() and annd_read_data_op() 2) In nand_write_page_xxx to call nand_exec_op() many times by nand_prog_page_begin_op(), nand_write_data_op() and nand_prog_page_end_op(). Should we consider to add a CS line controller in struct nand_controller i.e,. struct nand_controller { struct mutex lock; const struct nand_controller_ops *ops; + void (*select_chip)(struct nand_chip *chip, int cs); }; to replace legacy.select_chip() ? To patch in nand_select_target() and nand_deselect_target() void nand_select_target(struct nand_chip *chip, unsigned int cs) { /* * cs should always lie between 0 and chip->numchips, when that's not * the case it's a bug and the caller should be fixed. */ if (WARN_ON(cs > chip->numchips)) return; chip->cur_cs = cs; + if (chip->controller->select_chip) + chip->controller->select_chip(chip, cs); + if (chip->legacy.select_chip) chip->legacy.select_chip(chip, cs); } void nand_deselect_target(struct nand_chip *chip) { + if (chip->controller->select_chip) + chip->controller->select_chip(chip, -1); + if (chip->legacy.select_chip) chip->legacy.select_chip(chip, -1); chip->cur_cs = -1; } > > So if you think it is not too time consuming and worth the trouble to > assert/deassert the CS at each operation, you may do it in your driver. > > > Thanks, > Miquèl 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. =====================================================================