Skip to contentSkip to conversation

Fix OneNAND byte access

RFC PATCH In review Kyungmin Park 3 participants 3 messages May 16, 2008 Memory technology devices (MTD)

lore
Share & tools
lore

Kyungmin Park

author
#0 lore raw

RFC PATCH 1/1 Fix OneNAND byte access

Some upper layer try to read unaligned offset access So it adjusts the buffer, offset, and count variables

/ # mount -t ubifs ubi0 /mnt
UBIFS: recovery needed
onenand_read_bufferram[528] 0 11 <- Unaligned count
onenand_read_bufferram[514] 91 12 <- Unaligned offset
onenand_read_bufferram[528] 92 11
onenand_read_bufferram[528] 74 17
onenand_read_bufferram[514] 103 17 <- Unaligned offset
onenand_read_bufferram[514] 11 17
onenand_read_bufferram[514] 137 17
onenand_read_bufferram[528] 120 17
onenand_read_bufferram[528] 40 17
onenand_read_bufferram[514] 57 17
onenand_read_bufferram[528] 632 61
onenand_read_bufferram[528] 360 61
onenand_read_bufferram[528] 80 61

It's only optimization at driver level

I think it's the best that it handles at UBIFS itself e.g., Now it passed down from name handling If the size of name is odd how about pad it even?

Signed-off-by Kyungmin Park <[email protected]>

---

drivers/mtd/onenand/onenand_base.c +14 -0
index 5d7965f..49194b6 100644
@@ -509,9 +509,23 @@ static int onenand_read_bufferram(struct mtd_info *mtd, int area,
 
 	bufferram += onenand_bufferram_offset(mtd, area);
 
+	if (ONENAND_CHECK_BYTE_ACCESS(offset)) {
+		unsigned short word;
+		printk("%s[%d] %d %zd\n", __func__, __LINE__, offset, count);
+
+		/* Align with word(16-bit) size */
+		/* Read word and save byte */
+		word = this->read_word(bufferram + offset - 1);
+		buffer[0] = (word & 0xff00) >> 8;
+		buffer++;
+		offset++;
+		count--;
+	}
+
 	if (ONENAND_CHECK_BYTE_ACCESS(count)) {
 		unsigned short word;
 
+		printk("%s[%d] %d %zd\n", __func__, __LINE__, offset, count);
 		/* Align with word(16-bit) size */
 		count--;
#1 lore raw
Quoted 19 lines · Kyungmin Park

Some upper layer try to read unaligned offset access So it adjusts the buffer, offset, and count variables

/ # mount -t ubifs ubi0 /mnt
UBIFS: recovery needed
onenand_read_bufferram[528] 0 11 <- Unaligned count
onenand_read_bufferram[514] 91 12 <- Unaligned offset
onenand_read_bufferram[528] 92 11
onenand_read_bufferram[528] 74 17
onenand_read_bufferram[514] 103 17 <- Unaligned offset
onenand_read_bufferram[514] 11 17
onenand_read_bufferram[514] 137 17
onenand_read_bufferram[528] 120 17
onenand_read_bufferram[528] 40 17
onenand_read_bufferram[514] 57 17
onenand_read_bufferram[528] 632 61
onenand_read_bufferram[528] 360 61
onenand_read_bufferram[528] 80 61

Quoted 5 lines

It's only optimization at driver level

I think it's the best that it handles at UBIFS itself e.g., Now it passed down from name handling If the size of name is odd how about pad it even?

It would be a lot of work to change UBIFS.

It is really a driver problem. For example OMAP2 does not have the problem, so it should be fixed at the driver level.

Quoted 22 lines
Signed-off-by Kyungmin Park <[email protected]>

---

drivers/mtd/onenand/onenand_base.c +10 -0
index 5d7965f..49194b6 100644
@@ -509,9 +509,23 @@ static int onenand_read_bufferram(struct mtd_info *mtd, int area,
 
 	bufferram += onenand_bufferram_offset(mtd, area);
 
+	if (ONENAND_CHECK_BYTE_ACCESS(offset)) {
+		unsigned short word;
+		printk("%s[%d] %d %zd\n", __func__, __LINE__, offset, count);
+
+		/* Align with word(16-bit) size */
+		/* Read word and save byte */
+		word = this->read_word(bufferram + offset - 1);
+		buffer[0] = (word & 0xff00) >> 8;
+		buffer++;
+		offset++;

drivers/mtd/onenand/onenand_base.c static int onenand_read_bufferram(struct mtd_info *mtd, int area, · line 531

Quoted 10 lines
drivers/mtd/onenand/onenand_base.c +4 -0
+		count--;
+	}
+
 	if (ONENAND_CHECK_BYTE_ACCESS(count)) {
 		unsigned short word;
 
+		printk("%s[%d] %d %zd\n", __func__, __LINE__, offset, count);
 		/* Align with word(16-bit) size */
 		count--;

I presume you will remove the printks.

#2 lore raw
Quoted 6 lines
Quoted 5 lines

It's only optimization at driver level

I think it's the best that it handles at UBIFS itself e.g., Now it passed down from name handling If the size of name is odd how about pad it even?

Quoted 6 lines

It would be a lot of work to change UBIFS.

It is really a driver problem. For example OMAP2 does not have the problem, so it should be fixed at the driver level.

I'm not sure it makes a problem but potentially it access wrongly. Well I will check it at omap2 driver

Quoted 27 lines
Quoted 27 lines
Signed-off-by Kyungmin Park <[email protected]>

---

drivers/mtd/onenand/onenand_base.c +13 -0
index 5d7965f..49194b6 100644
@@ -509,9 +509,23 @@ static int onenand_read_bufferram(struct mtd_info *mtd, int area,
 
      bufferram += onenand_bufferram_offset(mtd, area);
 
+     if (ONENAND_CHECK_BYTE_ACCESS(offset)) {
+             unsigned short word;
+             printk("%s[%d] %d %zd\n", __func__, __LINE__, offset, count);
+
+             /* Align with word(16-bit) size */
+             /* Read word and save byte */
+             word = this->read_word(bufferram + offset - 1);
+             buffer[0] = (word & 0xff00) >> 8;
+             buffer++;
+             offset++;
+             count--;
+     }
+
      if (ONENAND_CHECK_BYTE_ACCESS(count)) {
              unsigned short word;
Quoted 8 lines

drivers/mtd/onenand/onenand_base.c static int onenand_read_bufferram(struct mtd_info *mtd, int area, · line 531

Quoted 5 lines
drivers/mtd/onenand/onenand_base.c +1 -0
+             printk("%s[%d] %d %zd\n", __func__, __LINE__, offset, count);
              /* Align with word(16-bit) size */
              count--;

I presume you will remove the printks.

Sure, it remains only for checking for you. Must be removed at patch.

Thank you,
Kyungmin Park