mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
From: Arseniy Krasnov <avkrasnov@sberdevices.ru>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Liang Yang <liang.yang@amlogic.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Yixun Lan <yixun.lan@amlogic.com>,
	Jianxin Pan <jianxin.pan@amlogic.com>, <oxffffaa@gmail.com>,
	<kernel@sberdevices.ru>, <linux-mtd@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 2/5] mtd: rawnand: meson: move OOB to non-protected ECC area
Date: Tue, 30 May 2023 11:28:33 +0300	[thread overview]
Message-ID: <1d1cfd95-d0ee-e6de-d3d9-acf6defde661@sberdevices.ru> (raw)
In-Reply-To: <20230530102143.6b2a199a@xps-13>



On 30.05.2023 11:21, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@sberdevices.ru wrote on Tue, 30 May 2023 11:09:10 +0300:
> 
>> Hi Miquel,
>>
>> On 30.05.2023 10:44, Miquel Raynal wrote:
>>> Hi Arseniy,
>>>   
>>>>>>>> -static void meson_nfc_get_user_byte(struct nand_chip *nand, u8 *oob_buf)
>>>>>>>> -{
>>>>>>>> -	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>>>>>>>> -	__le64 *info;
>>>>>>>> -	int i, count;
>>>>>>>> +	int i;
>>>>>>>>  
>>>>>>>> -	for (i = 0, count = 0; i < nand->ecc.steps; i++, count += 2) {
>>>>>>>> +	for (i = 0; i < nand->ecc.steps; i++) {
>>>>>>>>  		info = &meson_chip->info_buf[i];
>>>>>>>> -		oob_buf[count] = *info;
>>>>>>>> -		oob_buf[count + 1] = *info >> 8;
>>>>>>>> +		/* Always ignore user bytes programming. */      
>>>>>>>
>>>>>>> Why?      
>>>>>>
>>>>>> I think comment message is wrong a little bit. Here "user bytes" are
>>>>>> user bytes protected by ECC (e.g. location of these bytes differs from new
>>>>>> OOB layout introduced by this patch). During page write this hardware
>>>>>> always writes these bytes along with data. But, new OOB layout always ignores
>>>>>> these 4 bytes, so set them to 0xFF always.    
>>>>>
>>>>> When performing page reads/writes, you need to take the data as it's
>>>>> been provided. You may move the data around in the buffer provided to
>>>>> the controller, so that it get the ECC data at the right location, and
>>>>> you need of course to reorganize the data when reading as well, so that
>>>>> the user sees XkiB of data + YB of OOB. That's all you need to do in
>>>>> these helpers.
>>>>>     
>>>>
>>>> I think there is some misunderstanding about these "user bytes" above: there are 4
>>>> bytes which this NAND controller always writes to page in ECC mode - it was free OOB
>>>> bytes covered by ECC. Controller grabs values from DMA buffer (second DMA buffer which
>>>> doesn't contains page data) and writes it along with data and ECC codes. Idea of this
>>>> change is to always suppress this write by setting them to 0xFF (may be there is some
>>>> command option to not write it, but I don't have doc), because all of them (4 bytes)
>>>> become unavailable to reader/writer.  
>>>
>>> At the NAND controller level, I would rather avoid doing things like
>>> that.
>>>
>>> I believe you can just update the ooblayout so that protected OOB bytes
>>> are not exposed to the user as free bytes. Then your buffers should
>>> already contain 0xffffff at the problematic location.  
>>
>> So Your idea is to continue fill DMA buffer (for these 4 bytes) from provided OOB buffer,
>> relying on that as these bytes are unused, they will be 0xFF in OOB buffer so we get the same result?
> 
> Yes.
> 
> The problem you face is due to jffs2 using free OOB bytes to store some
> data. If this data is in the protected area -> BOOM.
> 
> If another application wants to use all the bytes and writes them all
> in the same PROGRAM operation it's fine.
> 
> Jffs2 accesses the free area through the OOB layouts only, so just
> tweaking the OOB layouts should work.

I see, I'll try this.

Thanks,
Arseniy

> 
> Thanks,
> Miquèl

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2023-05-30  8:33 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-15  9:44 [PATCH v4 0/5] refactoring and fix for Meson NAND Arseniy Krasnov
2023-05-15  9:44 ` [PATCH v4 1/5] mtd: rawnand: meson: fix command sequence for read/write Arseniy Krasnov
2023-05-22 15:05   ` Miquel Raynal
2023-05-23  9:12     ` Arseniy Krasnov
2023-05-24  9:05       ` Arseniy Krasnov
2023-05-26 17:22         ` Miquel Raynal
2023-05-30 11:19           ` Arseniy Krasnov
2023-05-30 13:05             ` Miquel Raynal
2023-05-30 13:35               ` Arseniy Krasnov
2023-05-30 13:58                 ` Miquel Raynal
2023-05-15  9:44 ` [PATCH v4 2/5] mtd: rawnand: meson: move OOB to non-protected ECC area Arseniy Krasnov
2023-05-22 15:33   ` Miquel Raynal
2023-05-23 17:17     ` Arseniy Krasnov
2023-05-26 17:03       ` Miquel Raynal
2023-05-29 19:43         ` Arseniy Krasnov
2023-05-30  7:44           ` Miquel Raynal
2023-05-30  8:09             ` Arseniy Krasnov
2023-05-30  8:21               ` Miquel Raynal
2023-05-30  8:28                 ` Arseniy Krasnov [this message]
2023-05-15  9:44 ` [PATCH v4 3/5] mtd: rawnand: meson: always read whole OOB bytes Arseniy Krasnov
2023-05-22 15:38   ` Miquel Raynal
2023-05-23 17:27     ` Arseniy Krasnov
2023-05-26 17:09       ` Miquel Raynal
2023-05-29 19:46         ` Arseniy Krasnov
2023-05-15  9:44 ` [PATCH v4 4/5] mtd: rawnand: meson: check buffer length Arseniy Krasnov
2023-05-22 15:43   ` Miquel Raynal
2023-05-15  9:44 ` [PATCH v4 5/5] mtd: rawnand: meson: remove unneeded bitwise OR with zeroes Arseniy Krasnov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1d1cfd95-d0ee-e6de-d3d9-acf6defde661@sberdevices.ru \
    --to=avkrasnov@sberdevices.ru \
    --cc=jbrunet@baylibre.com \
    --cc=jianxin.pan@amlogic.com \
    --cc=kernel@sberdevices.ru \
    --cc=khilman@baylibre.com \
    --cc=liang.yang@amlogic.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=neil.armstrong@linaro.org \
    --cc=oxffffaa@gmail.com \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    --cc=yixun.lan@amlogic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®