[PATCH 2/2] Creating helper func for block alignment verfication

[PATCH 2/2] Creating helper func for block alignment verfication

[Vimal Singh]

>From cde21be80002b49fb8baeb341e8ae5a1a2394cb6 Mon Sep 17 00:00:00 2001
From: Vimal Singh <vimalsingh@ti.com>
Date: Wed, 13 Jan 2010 18:11:47 +0530
Subject: [PATCH] Creating helper func for block alignment verfication

These checks are fairly common in 'nand_erase_nand', 'nand_lock'
and 'nand_unlock' fucntions.

Signed-off-by: Vimal Singh <vimalsingh@ti.com>
---
 drivers/mtd/nand/nand_base.c |   96 ++++++++++++++---------------------------
 1 files changed, 33 insertions(+), 63 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 4e27426..0bd3092 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -108,6 +108,36 @@ static int nand_do_write_oob(struct
  */
 DEFINE_LED_TRIGGER(nand_led_trigger);

+static int block_alignment_verification(struct mtd_info *mtd, loff_t
ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd->priv;
+
+	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
+			__func__, (unsigned long long)ofs, len);
+
+	/* Start address must align on block boundary */
+	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
+		ret = -EINVAL;
+	}
+
+	/* Length must align on block boundary */
+	if (len & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	/* Do not allow past end of device */
+	if (ofs + len > mtd->size) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * nand_release_device - [GENERIC] release chip
  * @mtd:	MTD device structure
@@ -894,28 +924,8 @@ int nand_unlock(struct mtd_info
 	int chipnr;
 	struct nand_chip *chip = mtd->priv;

-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}

 	/* Align to last block address if size addresses end of the device */
 	if (ofs + len == mtd->size)
@@ -968,31 +978,8 @@ int nand_lock(struct mtd_info
 	int chipnr, status, page;
 	struct nand_chip *chip = mtd->priv;

-	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
-			__func__, (unsigned long long)ofs, len);
-
-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}

 	nand_get_device(chip, mtd, FL_LOCKING);

@@ -2495,25 +2482,8 @@ int nand_erase_nand(struct
 				__func__, (unsigned long long)instr->addr,
 				(unsigned long long)instr->len);

-	/* Start address must align on block boundary */
-	if (instr->addr & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
+	if (block_alignment_verification(mtd, instr->addr, instr->len))
 		return -EINVAL;
-	}
-
-	/* Length must align on block boundary */
-	if (instr->len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		return -EINVAL;
-	}
-
-	/* Do not allow erase past end of device */
-	if ((instr->len + instr->addr) > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Erase past end of device\n",
-					__func__);
-		return -EINVAL;
-	}

 	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;

-- 
1.5.5

Re: [PATCH 2/2] Creating helper func for block alignment verfication

[Vimal Singh]

On Wed, Jan 13, 2010 at 6:23 PM, Vimal Singh <vimal.newwork@gmail.com> wrote:
> From cde21be80002b49fb8baeb341e8ae5a1a2394cb6 Mon Sep 17 00:00:00 2001
> From: Vimal Singh <vimalsingh@ti.com>
> Date: Wed, 13 Jan 2010 18:11:47 +0530
> Subject: [PATCH] Creating helper func for block alignment verfication
>
> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
> and 'nand_unlock' fucntions.
>

I just ran checkpatch.pl and this patch had an error.
Corrected patch is below.

-vimal

>From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
From: Vimal Singh <vimalsingh@ti.com>
Date: Wed, 13 Jan 2010 18:11:47 +0530
Subject: [PATCH] Creating helper func for block alignment verfication

These checks are fairly common in 'nand_erase_nand', 'nand_lock'
and 'nand_unlock' functions.

Signed-off-by: Vimal Singh <vimalsingh@ti.com>
---
 drivers/mtd/nand/nand_base.c |   97 +++++++++++++++---------------------------
 1 files changed, 34 insertions(+), 63 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 4e27426..c80cec5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -108,6 +108,37 @@ static int nand_do_write_oob(struct
  */
 DEFINE_LED_TRIGGER(nand_led_trigger);

+static int block_alignment_verification(struct mtd_info *mtd,
+					loff_t ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd->priv;
+
+	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
+			__func__, (unsigned long long)ofs, len);
+
+	/* Start address must align on block boundary */
+	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
+		ret = -EINVAL;
+	}
+
+	/* Length must align on block boundary */
+	if (len & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	/* Do not allow past end of device */
+	if (ofs + len > mtd->size) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * nand_release_device - [GENERIC] release chip
  * @mtd:	MTD device structure
@@ -894,28 +925,8 @@ int nand_unlock(struct mtd_info
 	int chipnr;
 	struct nand_chip *chip = mtd->priv;

-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}

 	/* Align to last block address if size addresses end of the device */
 	if (ofs + len == mtd->size)
@@ -968,31 +979,8 @@ int nand_lock(struct mtd_info
 	int chipnr, status, page;
 	struct nand_chip *chip = mtd->priv;

-	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
-			__func__, (unsigned long long)ofs, len);
-
-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}

 	nand_get_device(chip, mtd, FL_LOCKING);

@@ -2495,25 +2483,8 @@ int nand_erase_nand(struct
 				__func__, (unsigned long long)instr->addr,
 				(unsigned long long)instr->len);

-	/* Start address must align on block boundary */
-	if (instr->addr & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
+	if (block_alignment_verification(mtd, instr->addr, instr->len))
 		return -EINVAL;
-	}
-
-	/* Length must align on block boundary */
-	if (instr->len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		return -EINVAL;
-	}
-
-	/* Do not allow erase past end of device */
-	if ((instr->len + instr->addr) > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Erase past end of device\n",
-					__func__);
-		return -EINVAL;
-	}

 	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;

-- 
1.5.5

Re: [PATCH 2/2] Creating helper func for block alignment verfication

[Vimal Singh]

Please drop this patch. This patch breaks compilation. My bad...
I will be posting new version of this patch soon.

I apology for creating noise.

-vimal

On Wed, Jan 13, 2010 at 6:36 PM, Vimal Singh <vimal.newwork@gmail.com> wrote:
> On Wed, Jan 13, 2010 at 6:23 PM, Vimal Singh <vimal.newwork@gmail.com> wrote:
>> From cde21be80002b49fb8baeb341e8ae5a1a2394cb6 Mon Sep 17 00:00:00 2001
>> From: Vimal Singh <vimalsingh@ti.com>
>> Date: Wed, 13 Jan 2010 18:11:47 +0530
>> Subject: [PATCH] Creating helper func for block alignment verfication
>>
>> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
>> and 'nand_unlock' fucntions.
>>
>
> I just ran checkpatch.pl and this patch had an error.
> Corrected patch is below.
>
> -vimal
>
> From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
> From: Vimal Singh <vimalsingh@ti.com>
> Date: Wed, 13 Jan 2010 18:11:47 +0530
> Subject: [PATCH] Creating helper func for block alignment verfication
>
> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
> and 'nand_unlock' functions.
>
> Signed-off-by: Vimal Singh <vimalsingh@ti.com>
> ---
>  drivers/mtd/nand/nand_base.c |   97 +++++++++++++++---------------------------
>  1 files changed, 34 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 4e27426..c80cec5 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -108,6 +108,37 @@ static int nand_do_write_oob(struct
>  */
>  DEFINE_LED_TRIGGER(nand_led_trigger);
>
> +static int block_alignment_verification(struct mtd_info *mtd,
> +                                       loff_t ofs, uint64_t len)
> +{
> +       struct nand_chip *chip = mtd->priv;
> +
> +       DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
> +                       __func__, (unsigned long long)ofs, len);
> +
> +       /* Start address must align on block boundary */
> +       if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
> +               DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
> +               ret = -EINVAL;
> +       }
> +
> +       /* Length must align on block boundary */
> +       if (len & ((1 << chip->phys_erase_shift) - 1)) {
> +               DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
> +                                       __func__);
> +               ret = -EINVAL;
> +       }
> +
> +       /* Do not allow past end of device */
> +       if (ofs + len > mtd->size) {
> +               DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
> +                                       __func__);
> +               ret = -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>  /**
>  * nand_release_device - [GENERIC] release chip
>  * @mtd:       MTD device structure
> @@ -894,28 +925,8 @@ int nand_unlock(struct mtd_info
>        int chipnr;
>        struct nand_chip *chip = mtd->priv;
>
> -       /* Start address must align on block boundary */
> -       if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
> -               ret = -EINVAL;
> -               goto out;
> -       }
> -
> -       /* Length must align on block boundary */
> -       if (len & ((1 << chip->phys_erase_shift) - 1)) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
> -                                       __func__);
> -               ret = -EINVAL;
> +       if (block_alignment_verification(mtd, ofs, len))
>                goto out;
> -       }
> -
> -       /* Do not allow past end of device */
> -       if (ofs + len > mtd->size) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
> -                                       __func__);
> -               ret = -EINVAL;
> -               goto out;
> -       }
>
>        /* Align to last block address if size addresses end of the device */
>        if (ofs + len == mtd->size)
> @@ -968,31 +979,8 @@ int nand_lock(struct mtd_info
>        int chipnr, status, page;
>        struct nand_chip *chip = mtd->priv;
>
> -       DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
> -                       __func__, (unsigned long long)ofs, len);
> -
> -       /* Start address must align on block boundary */
> -       if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
> -               ret = -EINVAL;
> -               goto out;
> -       }
> -
> -       /* Length must align on block boundary */
> -       if (len & ((1 << chip->phys_erase_shift) - 1)) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
> -                                       __func__);
> -               ret = -EINVAL;
> +       if (block_alignment_verification(mtd, ofs, len))
>                goto out;
> -       }
> -
> -       /* Do not allow past end of device */
> -       if (ofs + len > mtd->size) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
> -                                       __func__);
> -               ret = -EINVAL;
> -               goto out;
> -       }
>
>        nand_get_device(chip, mtd, FL_LOCKING);
>
> @@ -2495,25 +2483,8 @@ int nand_erase_nand(struct
>                                __func__, (unsigned long long)instr->addr,
>                                (unsigned long long)instr->len);
>
> -       /* Start address must align on block boundary */
> -       if (instr->addr & ((1 << chip->phys_erase_shift) - 1)) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
> +       if (block_alignment_verification(mtd, instr->addr, instr->len))
>                return -EINVAL;
> -       }
> -
> -       /* Length must align on block boundary */
> -       if (instr->len & ((1 << chip->phys_erase_shift) - 1)) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
> -                                       __func__);
> -               return -EINVAL;
> -       }
> -
> -       /* Do not allow erase past end of device */
> -       if ((instr->len + instr->addr) > mtd->size) {
> -               DEBUG(MTD_DEBUG_LEVEL0, "%s: Erase past end of device\n",
> -                                       __func__);
> -               return -EINVAL;
> -       }
>
>        instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>
> --
> 1.5.5
>



-- 
Regards,
Vimal Singh

[PATCH v2 2/2] Creating helper func for block alignment verfication

[Vimal Singh]

>From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
From: Vimal Singh <vimalsingh@ti.com>
Date: Wed, 13 Jan 2010 18:11:47 +0530
Subject: [PATCH] Creating helper func for block alignment verfication

These checks are fairly common in 'nand_erase_nand', 'nand_lock'
and 'nand_unlock' functions.

Signed-off-by: Vimal Singh <vimalsingh@ti.com>
---
 drivers/mtd/nand/nand_base.c |   97 +++++++++++++++---------------------------
 1 files changed, 34 insertions(+), 63 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 4e27426..c80cec5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -108,6 +108,37 @@ static int nand_do_write_oob(struct
  */
 DEFINE_LED_TRIGGER(nand_led_trigger);

+static int block_alignment_verification(struct mtd_info *mtd,
+					loff_t ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd->priv;
+
+	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
+			__func__, (unsigned long long)ofs, len);
+
+	/* Start address must align on block boundary */
+	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
+		ret = -EINVAL;
+	}
+
+	/* Length must align on block boundary */
+	if (len & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	/* Do not allow past end of device */
+	if (ofs + len > mtd->size) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * nand_release_device - [GENERIC] release chip
  * @mtd:	MTD device structure
@@ -894,28 +925,8 @@ int nand_unlock(struct mtd_info
 	int chipnr;
 	struct nand_chip *chip = mtd->priv;

-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}

 	/* Align to last block address if size addresses end of the device */
 	if (ofs + len == mtd->size)
@@ -968,31 +979,8 @@ int nand_lock(struct mtd_info
 	int chipnr, status, page;
 	struct nand_chip *chip = mtd->priv;

-	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
-			__func__, (unsigned long long)ofs, len);
-
-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}

 	nand_get_device(chip, mtd, FL_LOCKING);

@@ -2495,25 +2483,8 @@ int nand_erase_nand(struct
 				__func__, (unsigned long long)instr->addr,
 				(unsigned long long)instr->len);

-	/* Start address must align on block boundary */
-	if (instr->addr & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
+	if (block_alignment_verification(mtd, instr->addr, instr->len))
 		return -EINVAL;
-	}
-
-	/* Length must align on block boundary */
-	if (instr->len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		return -EINVAL;
-	}
-
-	/* Do not allow erase past end of device */
-	if ((instr->len + instr->addr) > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Erase past end of device\n",
-					__func__);
-		return -EINVAL;
-	}

 	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;

-- 
1.5.5

Re: [PATCH v2 2/2] Creating helper func for block alignment verfication

[Vimal Singh]

On Wed, Jan 13, 2010 at 6:59 PM, Vimal Singh <vimal.newwork@gmail.com> wrote:
> From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
> From: Vimal Singh <vimalsingh@ti.com>
> Date: Wed, 13 Jan 2010 18:11:47 +0530
> Subject: [PATCH] Creating helper func for block alignment verfication
>
> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
> and 'nand_unlock' functions.
>


There is something seriously wrong with me... I posted old patch again. :(
I am sorry for this. Below is the correct patch. No more confusion this time.

-vimal

>From 7516e73f22b8822552f28d50bb104d888c5385b3 Mon Sep 17 00:00:00 2001
From: Vimal Singh <vimalsingh@ti.com>
Date: Wed, 13 Jan 2010 18:11:47 +0530
Subject: [PATCH] Creating helper func for block alignment verfication

These checks are fairly common in 'nand_erase_nand', 'nand_lock'
and 'nand_unlock' fucntions.

Signed-off-by: Vimal Singh <vimalsingh@ti.com>
---
 drivers/mtd/nand/nand_base.c |   98 +++++++++++++++---------------------------
 1 files changed, 35 insertions(+), 63 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 4e27426..63ee49e 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -108,6 +108,38 @@ static int nand_do_write_oob(struct
  */
 DEFINE_LED_TRIGGER(nand_led_trigger);

+static int block_alignment_verification(struct mtd_info *mtd,
+					loff_t ofs, uint64_t len)
+{
+	struct nand_chip *chip = mtd->priv;
+	int ret = 0;
+
+	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
+			__func__, (unsigned long long)ofs, len);
+
+	/* Start address must align on block boundary */
+	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
+		ret = -EINVAL;
+	}
+
+	/* Length must align on block boundary */
+	if (len & ((1 << chip->phys_erase_shift) - 1)) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	/* Do not allow past end of device */
+	if (ofs + len > mtd->size) {
+		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
+					__func__);
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
 /**
  * nand_release_device - [GENERIC] release chip
  * @mtd:	MTD device structure
@@ -894,28 +926,8 @@ int nand_unlock(struct mtd_info
 	int chipnr;
 	struct nand_chip *chip = mtd->priv;

-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}

 	/* Align to last block address if size addresses end of the device */
 	if (ofs + len == mtd->size)
@@ -968,31 +980,8 @@ int nand_lock(struct mtd_info
 	int chipnr, status, page;
 	struct nand_chip *chip = mtd->priv;

-	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
-			__func__, (unsigned long long)ofs, len);
-
-	/* Start address must align on block boundary */
-	if (ofs & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Length must align on block boundary */
-	if (len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* Do not allow past end of device */
-	if (ofs + len > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Past end of device\n",
-					__func__);
-		ret = -EINVAL;
+	if (block_alignment_verification(mtd, ofs, len))
 		goto out;
-	}

 	nand_get_device(chip, mtd, FL_LOCKING);

@@ -2495,25 +2484,8 @@ int nand_erase_nand(struct
 				__func__, (unsigned long long)instr->addr,
 				(unsigned long long)instr->len);

-	/* Start address must align on block boundary */
-	if (instr->addr & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Unaligned address\n", __func__);
-		return -EINVAL;
-	}
-
-	/* Length must align on block boundary */
-	if (instr->len & ((1 << chip->phys_erase_shift) - 1)) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Length not block aligned\n",
-					__func__);
+	if (block_alignment_verification(mtd, instr->addr, instr->len))
 		return -EINVAL;
-	}
-
-	/* Do not allow erase past end of device */
-	if ((instr->len + instr->addr) > mtd->size) {
-		DEBUG(MTD_DEBUG_LEVEL0, "%s: Erase past end of device\n",
-					__func__);
-		return -EINVAL;
-	}

 	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;

-- 
1.5.5

Re: [PATCH v2 2/2] Creating helper func for block alignment verfication

[Artem Bityutskiy]

Hi,

On Wed, 2010-01-13 at 18:59 +0530, Vimal Singh wrote:
> From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
> From: Vimal Singh <vimalsingh@ti.com>
> Date: Wed, 13 Jan 2010 18:11:47 +0530
> Subject: [PATCH] Creating helper func for block alignment verfication
> 
> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
> and 'nand_unlock' functions.
> 
> Signed-off-by: Vimal Singh <vimalsingh@ti.com>
> ---
>  drivers/mtd/nand/nand_base.c |   97 +++++++++++++++---------------------------
>  1 files changed, 34 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 4e27426..c80cec5 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -108,6 +108,37 @@ static int nand_do_write_oob(struct
>   */
>  DEFINE_LED_TRIGGER(nand_led_trigger);
> 
> +static int block_alignment_verification(struct mtd_info *mtd,
> +					loff_t ofs, uint64_t len)
> +{

This function checks not only alignment, so the name is bad. I suggest
check_offs_len() - it at least does not lie about what it does :-)

> +	struct nand_chip *chip = mtd->priv;
> +
> +	DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
> +			__func__, (unsigned long long)ofs, len);

No, you should keep the DEBUG part in the caller. Because of __func__.

Also please, introduce the helper in the _first_ patch, and then use it
in your functions in the second patch. This is more logical.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH v2 2/2] Creating helper func for block alignment verfication

[Vimal Singh]

On Thu, Jan 28, 2010 at 8:16 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> Hi,
>
> On Wed, 2010-01-13 at 18:59 +0530, Vimal Singh wrote:
>> From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
>> From: Vimal Singh <vimalsingh@ti.com>
>> Date: Wed, 13 Jan 2010 18:11:47 +0530
>> Subject: [PATCH] Creating helper func for block alignment verfication
>>
>> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
>> and 'nand_unlock' functions.
>>
>> Signed-off-by: Vimal Singh <vimalsingh@ti.com>
>> ---
>>  drivers/mtd/nand/nand_base.c |   97 +++++++++++++++---------------------------
>>  1 files changed, 34 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 4e27426..c80cec5 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -108,6 +108,37 @@ static int nand_do_write_oob(struct
>>   */
>>  DEFINE_LED_TRIGGER(nand_led_trigger);
>>
>> +static int block_alignment_verification(struct mtd_info *mtd,
>> +                                     loff_t ofs, uint64_t len)
>> +{
>
> This function checks not only alignment, so the name is bad. I suggest
> check_offs_len() - it at least does not lie about what it does :-)

OK, no problem.

>
>> +     struct nand_chip *chip = mtd->priv;
>> +
>> +     DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
>> +                     __func__, (unsigned long long)ofs, len);
>
> No, you should keep the DEBUG part in the caller. Because of __func__.

Agree.

>
> Also please, introduce the helper in the _first_ patch, and then use it
> in your functions in the second patch. This is more logical.

Before 1st patch this helper will be called by just one function
"nand_erase_nand". And then in that creating helper function does not
makes sense to me.
To me doing this in 2nd patch looks more logical.

Either way we will achieve same goal only number of lines in patches will defer.
So, if you still insist I can make it 1st patch.

-- 
Regards,
Vimal Singh

Re: [PATCH v2 2/2] Creating helper func for block alignment verfication

[Artem Bityutskiy]

On Fri, 2010-01-29 at 09:49 +0530, Vimal Singh wrote:
> On Thu, Jan 28, 2010 at 8:16 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > Hi,
> >
> > On Wed, 2010-01-13 at 18:59 +0530, Vimal Singh wrote:
> >> From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
> >> From: Vimal Singh <vimalsingh@ti.com>
> >> Date: Wed, 13 Jan 2010 18:11:47 +0530
> >> Subject: [PATCH] Creating helper func for block alignment verfication
> >>
> >> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
> >> and 'nand_unlock' functions.
> >>
> >> Signed-off-by: Vimal Singh <vimalsingh@ti.com>
> >> ---
> >>  drivers/mtd/nand/nand_base.c |   97 +++++++++++++++---------------------------
> >>  1 files changed, 34 insertions(+), 63 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> >> index 4e27426..c80cec5 100644
> >> --- a/drivers/mtd/nand/nand_base.c
> >> +++ b/drivers/mtd/nand/nand_base.c
> >> @@ -108,6 +108,37 @@ static int nand_do_write_oob(struct
> >>   */
> >>  DEFINE_LED_TRIGGER(nand_led_trigger);
> >>
> >> +static int block_alignment_verification(struct mtd_info *mtd,
> >> +                                     loff_t ofs, uint64_t len)
> >> +{
> >
> > This function checks not only alignment, so the name is bad. I suggest
> > check_offs_len() - it at least does not lie about what it does :-)
> 
> OK, no problem.
> 
> >
> >> +     struct nand_chip *chip = mtd->priv;
> >> +
> >> +     DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
> >> +                     __func__, (unsigned long long)ofs, len);
> >
> > No, you should keep the DEBUG part in the caller. Because of __func__.
> 
> Agree.
> 
> >
> > Also please, introduce the helper in the _first_ patch, and then use it
> > in your functions in the second patch. This is more logical.
> 
> Before 1st patch this helper will be called by just one function
> "nand_erase_nand". And then in that creating helper function does not
> makes sense to me.

It does. It will be a preparation to the next patch.

> To me doing this in 2nd patch looks more logical.
> 
> Either way we will achieve same goal only number of lines in patches will defer.
> So, if you still insist I can make it 1st patch.

OK, it is not a big deal.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH v2 2/2] Creating helper func for block alignment verfication

[Vimal Singh]

On Fri, Jan 29, 2010 at 10:15 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Fri, 2010-01-29 at 09:49 +0530, Vimal Singh wrote:
>> On Thu, Jan 28, 2010 at 8:16 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, 2010-01-13 at 18:59 +0530, Vimal Singh wrote:
>> >> From 310f7faa8f319bd9384512f7d5a7f13dcfbeebc8 Mon Sep 17 00:00:00 2001
>> >> From: Vimal Singh <vimalsingh@ti.com>
>> >> Date: Wed, 13 Jan 2010 18:11:47 +0530
>> >> Subject: [PATCH] Creating helper func for block alignment verfication
>> >>
>> >> These checks are fairly common in 'nand_erase_nand', 'nand_lock'
>> >> and 'nand_unlock' functions.
>> >>
>> >> Signed-off-by: Vimal Singh <vimalsingh@ti.com>
>> >> ---
>> >>  drivers/mtd/nand/nand_base.c |   97 +++++++++++++++---------------------------
>> >>  1 files changed, 34 insertions(+), 63 deletions(-)
>> >>
>> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> >> index 4e27426..c80cec5 100644
>> >> --- a/drivers/mtd/nand/nand_base.c
>> >> +++ b/drivers/mtd/nand/nand_base.c
>> >> @@ -108,6 +108,37 @@ static int nand_do_write_oob(struct
>> >>   */
>> >>  DEFINE_LED_TRIGGER(nand_led_trigger);
>> >>
>> >> +static int block_alignment_verification(struct mtd_info *mtd,
>> >> +                                     loff_t ofs, uint64_t len)
>> >> +{
>> >
>> > This function checks not only alignment, so the name is bad. I suggest
>> > check_offs_len() - it at least does not lie about what it does :-)
>>
>> OK, no problem.
>>
>> >
>> >> +     struct nand_chip *chip = mtd->priv;
>> >> +
>> >> +     DEBUG(MTD_DEBUG_LEVEL3, "%s: start = 0x%012llx, len = %llu\n",
>> >> +                     __func__, (unsigned long long)ofs, len);
>> >
>> > No, you should keep the DEBUG part in the caller. Because of __func__.
>>
>> Agree.
>>
>> >
>> > Also please, introduce the helper in the _first_ patch, and then use it
>> > in your functions in the second patch. This is more logical.
>>
>> Before 1st patch this helper will be called by just one function
>> "nand_erase_nand". And then in that creating helper function does not
>> makes sense to me.
>
> It does. It will be a preparation to the next patch.
>
>> To me doing this in 2nd patch looks more logical.
>>
>> Either way we will achieve same goal only number of lines in patches will defer.
>> So, if you still insist I can make it 1st patch.
>
> OK, it is not a big deal.

Well in that case I'll send this patch set by changing the patch order
sometime today.

-- 
Regards,
Vimal Singh