Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\))
Subject: Re: [PATCH] mtd: rawnand: brcmnand: force raw OOB writes
From:   =?utf-8?Q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= <noltari@gmail.com>
In-Reply-To: <CAC=U0a3xa9k76mxsiVKDyXsuvboZAyMkXT-S3-6oVZKjnWkeHA@mail.gmail.com>
Date:   Sat, 13 Jun 2020 11:01:56 +0200
Cc:     Brian Norris <computersforpeace@gmail.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        "R, Vignesh" <vigneshr@ti.com>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        MTD Maling List <linux-mtd@lists.infradead.org>,
        bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linaro-mm-sig@lists.linaro.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <05C962F2-7D3B-4103-91DD-8D31C439D521@gmail.com>
References: <20200605170720.2478262-1-noltari@gmail.com>
 <CAC=U0a3xa9k76mxsiVKDyXsuvboZAyMkXT-S3-6oVZKjnWkeHA@mail.gmail.com>
To:     Kamal Dasu <kdasu.kdev@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Kamal,

> El 12 jun 2020, a las 20:47, Kamal Dasu <kdasu.kdev@gmail.com> =
escribi=C3=B3:
>=20
> On Fri, Jun 5, 2020 at 1:07 PM =C3=81lvaro Fern=C3=A1ndez Rojas =
<noltari@gmail.com> wrote:
>>=20
>> MTD_OPS_AUTO_OOB is writting OOB with ECC enabled, which changes all =
ECC bytes
>> from an erased page to 0x00 when JFFS2 cleanmarkers are added with =
mtd-utils.
>>         | BBI |   JFFS2   |   ECC   |   JFFS2   | Spare  |
>> 00000800  ff ff 19 85 20 03 00 00  00 00 00 00 08 ff ff ff
>>=20
>> However, if OOB is written with ECC disabled, the JFFS2 cleanmarkers =
are
>> correctly written without changing the ECC bytes:
>>         | BBI |   JFFS2   |   ECC   |   JFFS2   | Spare  |
>> 00000800  ff ff 19 85 20 03 ff ff  ff 00 00 00 08 ff ff ff
>=20
> Both brcmand_write_oob_raw() and brcmnand_write_oob() use
> brcmnand_write() that uses PROGRAM_PAGE cmd, means also programs data
> areas and ECC when enabled  is always calculated on DATA+OOB.  since

Are you sure about that? I think that HW ECC is only calculated for =
DATA, not for OOB.
The fact that we=E2=80=99re not writing any DATA seems to be the problem =
that is changing all ECC bytes to 0x00.

> in both cases we only want to modify OOB, data is always assumed to be
> 0xffs (means erased state). So as far as this api always is used on
> the erased page it should be good. Are the sub-pages/oob areas read by
> jffs2  also read without ecc enabled?. Just want to be sure that it
> does not break any other utilities like nandwrite.

No, sub-pages/oob areas read by JFFS2 with ECC enabled.
I don=E2=80=99t think this breaks anything at all, since I tested =
nandwrite with OOB enabled and everything was written perfectly.

BTW, I tried another approach that forced MTD_OPS_AUTO_OOB to be written =
as raw OOB, but it was rejected:
=
http://patchwork.ozlabs.org/project/linux-mtd/patch/20200504094253.2741109=
-1-noltari@gmail.com/
https://lkml.org/lkml/2020/5/4/215

>=20
>>=20
>> Signed-off-by: =C3=81lvaro Fern=C3=A1ndez Rojas <noltari@gmail.com>
>> ---
>> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 9 +--------
>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>=20
>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c =
b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> index 8f9ffb46a09f..566281770841 100644
>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> @@ -2279,13 +2279,6 @@ static int brcmnand_write_page_raw(struct =
nand_chip *chip, const uint8_t *buf,
>>        return nand_prog_page_end_op(chip);
>> }
>>=20
>> -static int brcmnand_write_oob(struct nand_chip *chip, int page)
>> -{
>> -       return brcmnand_write(nand_to_mtd(chip), chip,
>> -                             (u64)page << chip->page_shift, NULL,
>> -                             chip->oob_poi);
>> -}
>> -
>> static int brcmnand_write_oob_raw(struct nand_chip *chip, int page)
>> {
>>        struct mtd_info *mtd =3D nand_to_mtd(chip);
>> @@ -2642,7 +2635,7 @@ static int brcmnand_init_cs(struct =
brcmnand_host *host, struct device_node *dn)
>>        chip->ecc.write_oob_raw =3D brcmnand_write_oob_raw;
>>        chip->ecc.read_oob_raw =3D brcmnand_read_oob_raw;
>>        chip->ecc.read_oob =3D brcmnand_read_oob;
>> -       chip->ecc.write_oob =3D brcmnand_write_oob;
>> +       chip->ecc.write_oob =3D brcmnand_write_oob_raw;
>>=20
>>        chip->controller =3D &ctrl->controller;
>>=20
>> --
>> 2.26.2
>>=20

Best regards,
=C3=81lvaro.=