| Message ID | 1490262226-29092-4-git-send-email-peterpandong@micron.com |
|---|---|
| State | Superseded |
| Delegated to: | Boris Brezillon |
| Headers | show |
On 03/23/2017 10:43 AM, Peter Pan wrote: > This commit adds some helpers in nand.h ^^^^^^ patch > nand_size() > nand_check_address() > nand_check_oob_ops() > nand_oob_ops_across_page() > nand_check_erase_ops() > > Signed-off-by: Peter Pan <peterpandong@micron.com> > --- > include/linux/mtd/nand.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 54ded4c..0c52401 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -434,6 +434,68 @@ static inline int nand_neraseblocks(struct nand_device *nand) > } > > /** > + * nand_size - Get NAND size > + * @nand: NAND device > + * > + * Returns the total size exposed by @nand. > + */ > +static inline u64 nand_size(struct nand_device *nand) > +{ > + return nand->memorg.ndies * nand->memorg.diesize; > +} > + kerneldoc missing ... fix globally > +static inline int nand_check_address(struct nand_device *nand, loff_t addr) > +{ > + return addr < nand_size(nand) ? 0 : -EINVAL; > +} > + > +static inline int nand_check_oob_ops(struct nand_device *nand, loff_t start, > + struct mtd_oob_ops *ops) > +{ > + struct mtd_info *mtd = nand_to_mtd(nand); > + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? > + mtd->oobavail : mtd->oobsize; > + > + if ((!!ops->datbuf != !!ops->len) || > + (!!ops->oobbuf != !!ops->ooblen)) > + return -EINVAL; > + if (ops->ooboffs >= ooblen) > + return -EINVAL; > + if (ops->ooboffs + ops->ooblen > > + (nand_len_to_pages(nand, nand_size(nand)) - > + nand_offs_to_page(nand, start)) * ooblen) The indent here is wrong and misguiding . I think it might be better to introduce some variables to avoid 3-line long conditions too. > + return -EINVAL; > + > + return 0; > +} > + > +static inline bool nand_oob_ops_across_page(struct nand_device *nand, > + struct mtd_oob_ops *ops) > +{ > + struct mtd_info *mtd = nand_to_mtd(nand); > + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? > + mtd->oobavail : mtd->oobsize; The ooblen can probably be deduplicated by introducing some function. Dunno if it's worth it though. > + return (ops->ooboffs + ops->ooblen) > ooblen; > +} > + > +static inline int nand_check_erase_ops(struct nand_device *nand, > + struct erase_info *einfo) > +{ > + /* check address align on block boundary */ > + if (einfo->addr & (nand_eraseblock_size(nand) - 1)) > + return -EINVAL; > + /* check lendth align on block boundary */ > + if (einfo->len & (nand_eraseblock_size(nand) - 1)) > + return -EINVAL; > + /* Do not allow erase past end of device */ > + if ((einfo->addr + einfo->len) > nand_size(nand)) > + return -EINVAL; > + > + return 0; > +} > + > +/** > * nand_register - Register a NAND device > * @nand: NAND device > * >
On Thu, 23 Mar 2017 17:43:40 +0800 Peter Pan <peterpandong@micron.com> wrote: > This commit adds some helpers in nand.h > nand_size() > nand_check_address() > nand_check_oob_ops() > nand_oob_ops_across_page() > nand_check_erase_ops() > > Signed-off-by: Peter Pan <peterpandong@micron.com> > --- > include/linux/mtd/nand.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 54ded4c..0c52401 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -434,6 +434,68 @@ static inline int nand_neraseblocks(struct nand_device *nand) > } > > /** > + * nand_size - Get NAND size > + * @nand: NAND device > + * > + * Returns the total size exposed by @nand. > + */ > +static inline u64 nand_size(struct nand_device *nand) > +{ > + return nand->memorg.ndies * nand->memorg.diesize; > +} > + The nand_size() function should probably go in the commit introducing the interface-agnostic NAND layer (in my own series). Since you'll be the first one to use the generic NAND layer, I propose that you start maintaining my patch-set along with your SPI NAND patches. > +static inline int nand_check_address(struct nand_device *nand, loff_t addr) > +{ > + return addr < nand_size(nand) ? 0 : -EINVAL; > +} > + > +static inline int nand_check_oob_ops(struct nand_device *nand, loff_t start, > + struct mtd_oob_ops *ops) > +{ > + struct mtd_info *mtd = nand_to_mtd(nand); > + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? > + mtd->oobavail : mtd->oobsize; > + > + if ((!!ops->datbuf != !!ops->len) || > + (!!ops->oobbuf != !!ops->ooblen)) > + return -EINVAL; > + if (ops->ooboffs >= ooblen) > + return -EINVAL; > + if (ops->ooboffs + ops->ooblen > > + (nand_len_to_pages(nand, nand_size(nand)) - > + nand_offs_to_page(nand, start)) * ooblen) > + return -EINVAL; > + > + return 0; > +} > + > +static inline bool nand_oob_ops_across_page(struct nand_device *nand, > + struct mtd_oob_ops *ops) > +{ > + struct mtd_info *mtd = nand_to_mtd(nand); > + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? > + mtd->oobavail : mtd->oobsize; > + > + return (ops->ooboffs + ops->ooblen) > ooblen; > +} > + > +static inline int nand_check_erase_ops(struct nand_device *nand, > + struct erase_info *einfo) > +{ > + /* check address align on block boundary */ > + if (einfo->addr & (nand_eraseblock_size(nand) - 1)) > + return -EINVAL; > + /* check lendth align on block boundary */ > + if (einfo->len & (nand_eraseblock_size(nand) - 1)) > + return -EINVAL; > + /* Do not allow erase past end of device */ > + if ((einfo->addr + einfo->len) > nand_size(nand)) > + return -EINVAL; > + > + return 0; > +} Hm, looking at these functions and the tests already done in mtdcore.c I'm not sure this is the best location. AFAICS, all these tests can be done (or are already done) at the MTD level, and that's where they are the most useful. The only exception I see where it could be useful is when the BBT code directly calls nand functions instead of mtd ones, but this should not happen that much. > + > +/** > * nand_register - Register a NAND device > * @nand: NAND device > *
Hi Boris, On Thu, Mar 30, 2017 at 3:57 AM, Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > On Thu, 23 Mar 2017 17:43:40 +0800 > Peter Pan <peterpandong@micron.com> wrote: > >> This commit adds some helpers in nand.h >> nand_size() >> nand_check_address() >> nand_check_oob_ops() >> nand_oob_ops_across_page() >> nand_check_erase_ops() >> >> Signed-off-by: Peter Pan <peterpandong@micron.com> >> --- >> include/linux/mtd/nand.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 62 insertions(+) >> >> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h >> index 54ded4c..0c52401 100644 >> --- a/include/linux/mtd/nand.h >> +++ b/include/linux/mtd/nand.h >> @@ -434,6 +434,68 @@ static inline int nand_neraseblocks(struct nand_device *nand) >> } >> >> /** >> + * nand_size - Get NAND size >> + * @nand: NAND device >> + * >> + * Returns the total size exposed by @nand. >> + */ >> +static inline u64 nand_size(struct nand_device *nand) >> +{ >> + return nand->memorg.ndies * nand->memorg.diesize; >> +} >> + > > The nand_size() function should probably go in the commit introducing > the interface-agnostic NAND layer (in my own series). > > Since you'll be the first one to use the generic NAND layer, I propose > that you start maintaining my patch-set along with your SPI NAND > patches. You mean that I send your patch-set with SPI NAND patches? If so, you want me to do this in v5 or when SPI NAND patches is ready to merge? Thanks, Peter Pan > >> +static inline int nand_check_address(struct nand_device *nand, loff_t addr) >> +{ >> + return addr < nand_size(nand) ? 0 : -EINVAL; >> +} >> + >> +static inline int nand_check_oob_ops(struct nand_device *nand, loff_t start, >> + struct mtd_oob_ops *ops) >> +{ >> + struct mtd_info *mtd = nand_to_mtd(nand); >> + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? >> + mtd->oobavail : mtd->oobsize; >> + >> + if ((!!ops->datbuf != !!ops->len) || >> + (!!ops->oobbuf != !!ops->ooblen)) >> + return -EINVAL; >> + if (ops->ooboffs >= ooblen) >> + return -EINVAL; >> + if (ops->ooboffs + ops->ooblen > >> + (nand_len_to_pages(nand, nand_size(nand)) - >> + nand_offs_to_page(nand, start)) * ooblen) >> + return -EINVAL; >> + >> + return 0; >> +} >> + >> +static inline bool nand_oob_ops_across_page(struct nand_device *nand, >> + struct mtd_oob_ops *ops) >> +{ >> + struct mtd_info *mtd = nand_to_mtd(nand); >> + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? >> + mtd->oobavail : mtd->oobsize; >> + >> + return (ops->ooboffs + ops->ooblen) > ooblen; >> +} >> + >> +static inline int nand_check_erase_ops(struct nand_device *nand, >> + struct erase_info *einfo) >> +{ >> + /* check address align on block boundary */ >> + if (einfo->addr & (nand_eraseblock_size(nand) - 1)) >> + return -EINVAL; >> + /* check lendth align on block boundary */ >> + if (einfo->len & (nand_eraseblock_size(nand) - 1)) >> + return -EINVAL; >> + /* Do not allow erase past end of device */ >> + if ((einfo->addr + einfo->len) > nand_size(nand)) >> + return -EINVAL; >> + >> + return 0; >> +} > > Hm, looking at these functions and the tests already done in mtdcore.c > I'm not sure this is the best location. > AFAICS, all these tests can be done (or are already done) at the MTD > level, and that's where they are the most useful. The only exception I > see where it could be useful is when the BBT code directly calls nand > functions instead of mtd ones, but this should not happen that much. > >> + >> +/** >> * nand_register - Register a NAND device >> * @nand: NAND device >> * >
On Thu, 30 Mar 2017 16:04:44 +0800 Peter Pan <peterpansjtu@gmail.com> wrote: > Hi Boris, > > On Thu, Mar 30, 2017 at 3:57 AM, Boris Brezillon > <boris.brezillon@free-electrons.com> wrote: > > On Thu, 23 Mar 2017 17:43:40 +0800 > > Peter Pan <peterpandong@micron.com> wrote: > > > >> This commit adds some helpers in nand.h > >> nand_size() > >> nand_check_address() > >> nand_check_oob_ops() > >> nand_oob_ops_across_page() > >> nand_check_erase_ops() > >> > >> Signed-off-by: Peter Pan <peterpandong@micron.com> > >> --- > >> include/linux/mtd/nand.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 62 insertions(+) > >> > >> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > >> index 54ded4c..0c52401 100644 > >> --- a/include/linux/mtd/nand.h > >> +++ b/include/linux/mtd/nand.h > >> @@ -434,6 +434,68 @@ static inline int nand_neraseblocks(struct nand_device *nand) > >> } > >> > >> /** > >> + * nand_size - Get NAND size > >> + * @nand: NAND device > >> + * > >> + * Returns the total size exposed by @nand. > >> + */ > >> +static inline u64 nand_size(struct nand_device *nand) > >> +{ > >> + return nand->memorg.ndies * nand->memorg.diesize; > >> +} > >> + > > > > The nand_size() function should probably go in the commit introducing > > the interface-agnostic NAND layer (in my own series). > > > > Since you'll be the first one to use the generic NAND layer, I propose > > that you start maintaining my patch-set along with your SPI NAND > > patches. > > You mean that I send your patch-set with SPI NAND patches? If so, you want > me to do this in v5 or when SPI NAND patches is ready to merge? No need to send the "interface-agnostic NAND layer" patches in future versions of this series, let's wait until everyone is happy with the SPI NAND patches. But you can already create your own branch and maintain my patches (rebase on newer kernel versions or nand/next if needed, squash changes into existing commits if it makes sense, ...). It's also easier for people who want to test your patches to have a branch containing everything.
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 54ded4c..0c52401 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -434,6 +434,68 @@ static inline int nand_neraseblocks(struct nand_device *nand) } /** + * nand_size - Get NAND size + * @nand: NAND device + * + * Returns the total size exposed by @nand. + */ +static inline u64 nand_size(struct nand_device *nand) +{ + return nand->memorg.ndies * nand->memorg.diesize; +} + +static inline int nand_check_address(struct nand_device *nand, loff_t addr) +{ + return addr < nand_size(nand) ? 0 : -EINVAL; +} + +static inline int nand_check_oob_ops(struct nand_device *nand, loff_t start, + struct mtd_oob_ops *ops) +{ + struct mtd_info *mtd = nand_to_mtd(nand); + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? + mtd->oobavail : mtd->oobsize; + + if ((!!ops->datbuf != !!ops->len) || + (!!ops->oobbuf != !!ops->ooblen)) + return -EINVAL; + if (ops->ooboffs >= ooblen) + return -EINVAL; + if (ops->ooboffs + ops->ooblen > + (nand_len_to_pages(nand, nand_size(nand)) - + nand_offs_to_page(nand, start)) * ooblen) + return -EINVAL; + + return 0; +} + +static inline bool nand_oob_ops_across_page(struct nand_device *nand, + struct mtd_oob_ops *ops) +{ + struct mtd_info *mtd = nand_to_mtd(nand); + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ? + mtd->oobavail : mtd->oobsize; + + return (ops->ooboffs + ops->ooblen) > ooblen; +} + +static inline int nand_check_erase_ops(struct nand_device *nand, + struct erase_info *einfo) +{ + /* check address align on block boundary */ + if (einfo->addr & (nand_eraseblock_size(nand) - 1)) + return -EINVAL; + /* check lendth align on block boundary */ + if (einfo->len & (nand_eraseblock_size(nand) - 1)) + return -EINVAL; + /* Do not allow erase past end of device */ + if ((einfo->addr + einfo->len) > nand_size(nand)) + return -EINVAL; + + return 0; +} + +/** * nand_register - Register a NAND device * @nand: NAND device *
This commit adds some helpers in nand.h nand_size() nand_check_address() nand_check_oob_ops() nand_oob_ops_across_page() nand_check_erase_ops() Signed-off-by: Peter Pan <peterpandong@micron.com> --- include/linux/mtd/nand.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)