| Message ID | 1572256527-5074-2-git-send-email-masonccyang@mxic.com.tw |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: rawnand: Add support Macronix Block Protection & deep power down mode | expand |
Hi Mason, Mason Yang <masonccyang@mxic.com.tw> wrote on Mon, 28 Oct 2019 17:55:24 +0800: > Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock > operation while the device supports Block Protection function. > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> > --- > drivers/mtd/nand/raw/nand_base.c | 32 ++++++++++++++++++++++++++++++-- > include/linux/mtd/rawnand.h | 3 +++ > 2 files changed, 33 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 5c2c30a..5e318ff 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -4356,6 +4356,34 @@ static void nand_shutdown(struct mtd_info *mtd) > nand_suspend(mtd); > } > > +/** > + * nand_lock - [MTD Interface] Lock the NAND flash > + * @mtd: MTD device structure > + */ > +static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (!chip->_lock) > + return -ENOTSUPP; > + > + return chip->_lock(chip, ofs, len); > +} > + > +/** > + * nand_unlock - [MTD Interface] Unlock the NAND flash > + * @mtd: MTD device structure > + */ > +static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (!chip->_unlock) > + return -ENOTSUPP; > + > + return chip->_unlock(chip, ofs, len); > +} > + > /* Set default functions */ > static void nand_set_defaults(struct nand_chip *chip) > { > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip) > mtd->_read_oob = nand_read_oob; > mtd->_write_oob = nand_write_oob; > mtd->_sync = nand_sync; > - mtd->_lock = NULL; > - mtd->_unlock = NULL; > + mtd->_lock = nand_lock; > + mtd->_unlock = nand_unlock; > mtd->_suspend = nand_suspend; > mtd->_resume = nand_resume; > mtd->_reboot = nand_shutdown; > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index 4ab9bcc..2430ecd 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -1136,6 +1136,9 @@ struct nand_chip { > const struct nand_manufacturer *desc; > void *priv; > } manufacturer; > + > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); Kernel documentation is missing here. Also please fix kbuildtest robot warnings. With all this done, please add my: Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks, Miquèl
On Mon, 28 Oct 2019 17:55:24 +0800 Mason Yang <masonccyang@mxic.com.tw> wrote: > /* Set default functions */ > static void nand_set_defaults(struct nand_chip *chip) > { > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip) > mtd->_read_oob = nand_read_oob; > mtd->_write_oob = nand_write_oob; > mtd->_sync = nand_sync; > - mtd->_lock = NULL; > - mtd->_unlock = NULL; > + mtd->_lock = nand_lock; > + mtd->_unlock = nand_unlock; > mtd->_suspend = nand_suspend; > mtd->_resume = nand_resume; > mtd->_reboot = nand_shutdown; > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index 4ab9bcc..2430ecd 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -1136,6 +1136,9 @@ struct nand_chip { > const struct nand_manufacturer *desc; > void *priv; > } manufacturer; > + > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); Please drop this _ prefix. > };
Hi Miquel, > > /* Set default functions */ > > static void nand_set_defaults(struct nand_chip *chip) > > { > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip) > > mtd->_read_oob = nand_read_oob; > > mtd->_write_oob = nand_write_oob; > > mtd->_sync = nand_sync; > > - mtd->_lock = NULL; > > - mtd->_unlock = NULL; > > + mtd->_lock = nand_lock; > > + mtd->_unlock = nand_unlock; > > mtd->_suspend = nand_suspend; > > mtd->_resume = nand_resume; > > mtd->_reboot = nand_shutdown; > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > index 4ab9bcc..2430ecd 100644 > > --- a/include/linux/mtd/rawnand.h > > +++ b/include/linux/mtd/rawnand.h > > @@ -1136,6 +1136,9 @@ struct nand_chip { > > const struct nand_manufacturer *desc; > > void *priv; > > } manufacturer; > > + > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > Kernel documentation is missing here. > > Also please fix kbuildtest robot warnings. okay, will fix both ! > > With all this done, please add my: > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> > > Thanks, > Miquèl 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, > > > /* Set default functions */ > > static void nand_set_defaults(struct nand_chip *chip) > > { > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip) > > mtd->_read_oob = nand_read_oob; > > mtd->_write_oob = nand_write_oob; > > mtd->_sync = nand_sync; > > - mtd->_lock = NULL; > > - mtd->_unlock = NULL; > > + mtd->_lock = nand_lock; > > + mtd->_unlock = nand_unlock; > > mtd->_suspend = nand_suspend; > > mtd->_resume = nand_resume; > > mtd->_reboot = nand_shutdown; > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > index 4ab9bcc..2430ecd 100644 > > --- a/include/linux/mtd/rawnand.h > > +++ b/include/linux/mtd/rawnand.h > > @@ -1136,6 +1136,9 @@ struct nand_chip { > > const struct nand_manufacturer *desc; > > void *priv; > > } manufacturer; > > + > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > Please drop this _ prefix. Drop _ prefix of _lock will get compile error due to there is already defined "struct mutex lock" in struct nand_chip. What about keep this _ prefix or patch it to blocklock/blockunlock, i.e., int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len); int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); 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 Mason, masonccyang@mxic.com.tw wrote on Mon, 17 Feb 2020 16:14:23 +0800: > Hi Boris, > > > > > > /* Set default functions */ > > > static void nand_set_defaults(struct nand_chip *chip) > > > { > > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip > *chip) > > > mtd->_read_oob = nand_read_oob; > > > mtd->_write_oob = nand_write_oob; > > > mtd->_sync = nand_sync; > > > - mtd->_lock = NULL; > > > - mtd->_unlock = NULL; > > > + mtd->_lock = nand_lock; > > > + mtd->_unlock = nand_unlock; > > > mtd->_suspend = nand_suspend; > > > mtd->_resume = nand_resume; > > > mtd->_reboot = nand_shutdown; > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > > index 4ab9bcc..2430ecd 100644 > > > --- a/include/linux/mtd/rawnand.h > > > +++ b/include/linux/mtd/rawnand.h > > > @@ -1136,6 +1136,9 @@ struct nand_chip { > > > const struct nand_manufacturer *desc; > > > void *priv; > > > } manufacturer; > > > + > > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > > > Please drop this _ prefix. > > Drop _ prefix of _lock will get compile error due to there is already > defined "struct mutex lock" in struct nand_chip. Right! > > What about keep this _ prefix or patch it to blocklock/blockunlock, > i.e., > int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); What about lock_area() unlock_area() ? Seems more accurate to me, tell me if I'm wrong. > > > thanks for your time & comments. > Mason Thanks, Miquèl
On Mon, 17 Feb 2020 10:01:24 +0100 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Hi Mason, > > masonccyang@mxic.com.tw wrote on Mon, 17 Feb 2020 16:14:23 +0800: > > > Hi Boris, > > > > > > > > > /* Set default functions */ > > > > static void nand_set_defaults(struct nand_chip *chip) > > > > { > > > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip > > *chip) > > > > mtd->_read_oob = nand_read_oob; > > > > mtd->_write_oob = nand_write_oob; > > > > mtd->_sync = nand_sync; > > > > - mtd->_lock = NULL; > > > > - mtd->_unlock = NULL; > > > > + mtd->_lock = nand_lock; > > > > + mtd->_unlock = nand_unlock; > > > > mtd->_suspend = nand_suspend; > > > > mtd->_resume = nand_resume; > > > > mtd->_reboot = nand_shutdown; > > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > > > index 4ab9bcc..2430ecd 100644 > > > > --- a/include/linux/mtd/rawnand.h > > > > +++ b/include/linux/mtd/rawnand.h > > > > @@ -1136,6 +1136,9 @@ struct nand_chip { > > > > const struct nand_manufacturer *desc; > > > > void *priv; > > > > } manufacturer; > > > > + > > > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > > > > > Please drop this _ prefix. > > > > Drop _ prefix of _lock will get compile error due to there is already > > defined "struct mutex lock" in struct nand_chip. > > Right! Or maybe move all hooks to a sub-struct (struct nand_chip_ops ops). I had planned to do that in my nand_chip_legacy refactor but never did, so maybe now is a good time. > > > > > What about keep this _ prefix or patch it to blocklock/blockunlock, > > i.e., > > int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > What about lock_area() unlock_area() ? Seems more accurate to me, tell > me if I'm wrong. Yep, definitely better.
Hi Miquel, > > > > /* Set default functions */ > > > > static void nand_set_defaults(struct nand_chip *chip) > > > > { > > > > @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip > > *chip) > > > > mtd->_read_oob = nand_read_oob; > > > > mtd->_write_oob = nand_write_oob; > > > > mtd->_sync = nand_sync; > > > > - mtd->_lock = NULL; > > > > - mtd->_unlock = NULL; > > > > + mtd->_lock = nand_lock; > > > > + mtd->_unlock = nand_unlock; > > > > mtd->_suspend = nand_suspend; > > > > mtd->_resume = nand_resume; > > > > mtd->_reboot = nand_shutdown; > > > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > > > index 4ab9bcc..2430ecd 100644 > > > > --- a/include/linux/mtd/rawnand.h > > > > +++ b/include/linux/mtd/rawnand.h > > > > @@ -1136,6 +1136,9 @@ struct nand_chip { > > > > const struct nand_manufacturer *desc; > > > > void *priv; > > > > } manufacturer; > > > > + > > > > + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > > > + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > > > > > Please drop this _ prefix. > > > > Drop _ prefix of _lock will get compile error due to there is already > > defined "struct mutex lock" in struct nand_chip. > > Right! > > > > > What about keep this _ prefix or patch it to blocklock/blockunlock, > > i.e., > > int (*blocklock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > int (*blockunlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); > > What about lock_area() unlock_area() ? Seems more accurate to me, tell > me if I'm wrong. yup, you are right! Using lock/unlock_area is better, will patch it. thanks for your 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. =====================================================================
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 5c2c30a..5e318ff 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -4356,6 +4356,34 @@ static void nand_shutdown(struct mtd_info *mtd) nand_suspend(mtd); } +/** + * nand_lock - [MTD Interface] Lock the NAND flash + * @mtd: MTD device structure + */ +static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + + if (!chip->_lock) + return -ENOTSUPP; + + return chip->_lock(chip, ofs, len); +} + +/** + * nand_unlock - [MTD Interface] Unlock the NAND flash + * @mtd: MTD device structure + */ +static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + + if (!chip->_unlock) + return -ENOTSUPP; + + return chip->_unlock(chip, ofs, len); +} + /* Set default functions */ static void nand_set_defaults(struct nand_chip *chip) { @@ -5782,8 +5810,8 @@ static int nand_scan_tail(struct nand_chip *chip) mtd->_read_oob = nand_read_oob; mtd->_write_oob = nand_write_oob; mtd->_sync = nand_sync; - mtd->_lock = NULL; - mtd->_unlock = NULL; + mtd->_lock = nand_lock; + mtd->_unlock = nand_unlock; mtd->_suspend = nand_suspend; mtd->_resume = nand_resume; mtd->_reboot = nand_shutdown; diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 4ab9bcc..2430ecd 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1136,6 +1136,9 @@ struct nand_chip { const struct nand_manufacturer *desc; void *priv; } manufacturer; + + int (*_lock)(struct nand_chip *chip, loff_t ofs, uint64_t len); + int (*_unlock)(struct nand_chip *chip, loff_t ofs, uint64_t len); }; extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
Add nand_lock() & nand_unlock() for manufacturer specific lock & unlock operation while the device supports Block Protection function. Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> --- drivers/mtd/nand/raw/nand_base.c | 32 ++++++++++++++++++++++++++++++-- include/linux/mtd/rawnand.h | 3 +++ 2 files changed, 33 insertions(+), 2 deletions(-)