* [RFC][PATCH][MTD][OneNAND] Fix OneNAND byte access
@ 2008-05-16 4:45 Kyungmin Park
2008-05-16 7:25 ` Adrian Hunter
0 siblings, 1 reply; 3+ messages in thread
From: Kyungmin Park @ 2008-05-16 4:45 UTC (permalink / raw)
To: dedekind, ext-adrian.hunter; +Cc: linux-mtd
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 <kyungmin.park@samsung.com>
---
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 5d7965f..49194b6 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -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--;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC][PATCH][MTD][OneNAND] Fix OneNAND byte access
2008-05-16 4:45 [RFC][PATCH][MTD][OneNAND] Fix OneNAND byte access Kyungmin Park
@ 2008-05-16 7:25 ` Adrian Hunter
2008-05-16 7:52 ` Kyungmin Park
0 siblings, 1 reply; 3+ messages in thread
From: Adrian Hunter @ 2008-05-16 7:25 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mtd
Kyungmin Park wrote:
> 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?
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.
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
> index 5d7965f..49194b6 100644
> --- a/drivers/mtd/onenand/onenand_base.c
> +++ b/drivers/mtd/onenand/onenand_base.c
> @@ -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--;
>
>
I presume you will remove the printks.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC][PATCH][MTD][OneNAND] Fix OneNAND byte access
2008-05-16 7:25 ` Adrian Hunter
@ 2008-05-16 7:52 ` Kyungmin Park
0 siblings, 0 replies; 3+ messages in thread
From: Kyungmin Park @ 2008-05-16 7:52 UTC (permalink / raw)
To: Adrian Hunter; +Cc: Kyungmin Park, linux-mtd
>> 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.
>
I'm not sure it makes a problem but potentially it access wrongly.
Well I will check it at omap2 driver
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
>> index 5d7965f..49194b6 100644
>> --- a/drivers/mtd/onenand/onenand_base.c
>> +++ b/drivers/mtd/onenand/onenand_base.c
>> @@ -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--;
>>
>>
>
> I presume you will remove the printks.
>
Sure, it remains only for checking for you. Must be removed at patch.
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-05-16 7:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-16 4:45 [RFC][PATCH][MTD][OneNAND] Fix OneNAND byte access Kyungmin Park
2008-05-16 7:25 ` Adrian Hunter
2008-05-16 7:52 ` Kyungmin Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox