All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Daniel Palmer <daniel@0x0f.com>
Cc: linux-mtd@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mtd: spinand: add support for Foresee FS35ND01G-S1Y2
Date: Mon, 15 Feb 2021 12:16:53 +0100	[thread overview]
Message-ID: <20210215121653.4edd86c4@xps13> (raw)
In-Reply-To: <CAFr9PXkh+attaCc6C2UxB=qvXksWriWOaaoEndy4k6SGE0QOHQ@mail.gmail.com>

Hi Daniel,

Daniel Palmer <daniel@0x0f.com> wrote on Mon, 15 Feb 2021 19:53:13
+0900:

> Hi Miquel,
> 
> On Mon, 15 Feb 2021 at 19:24, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Can you please add a changelog here when you send a new version of a
> > patch?  
> 
> Sorry, I was going to add a cover letter but elsewhere got told that
> one isn't needed for a single patch..

A cover letter is useful when there are many patches, or when there is
some context that is important to remember.

But a changelog should always be added when you change something
between two versions. And the changelog can be located below the three
dashes ("---") without being part of the final commit message, it does
not need to be in a separate cover letter.

> Basically I changed FS35ND01G to FS35ND01G-S1Y2 as that's the proper
> part number for the chip I have and there seem to be a few variations
> of this.
> Aside from that I fixed up the hex numbers to be uppercase and added
> the oob layout callbacks.
> 
> > > +static int fs35nd01g_s1y2_ooblayout_free(struct mtd_info *mtd, int section,
> > > +                                 struct mtd_oob_region *region)
> > > +{
> > > +     if (section > 3)
> > > +             return -ERANGE;
> > > +
> > > +     /*
> > > +      * No ECC data is stored in the accessible OOB so the full 16 bytes
> > > +      * of each spare region is available to the user. Apparently also
> > > +      * covered by the internal ECC.  
> >
> > How is this even possible? ECC must be stored somewhere, maybe it is
> > not possible to retrieve it but I guess you cannot use the 32 bytes of
> > OOB for user data. Can you please verify that?  
> 
> This worried me too as I could not find the OOB layout anywhere.
> They simply list there being 4 512 byte main areas and then 4 16 byte
> spare areas. The only other note is that the first byte of spare0 is
> used for the bad block marker.
> 
> I contacted Longsys but they didn't get back to me.
> So what I did here was I started googling strings within the datasheet
> to find other chips that are probably the same IP inside and I found
> the FM25G01.
> It's datasheet shares a lot of the same text and the flash layout
> diagrams etc are the same.
> It has the same table for the flash layout. 4 512 byte areas and 4 16
> byte spare areas. It has the same note for the bad block marker and
> then one additional note:
> 
> "2. Spare area 800H to 83FH is all available for user.
>  ECC parity codes are programmed in
> additional space and not user accessible."
> 
> It would seem that the pages are actually bigger than 2K + 64 or there
> is some other place they keep the ECC.
> Or both datasheets are lying. Somewhere else in the datasheets it says
> that writes to the ECC area will be ignored but that doesn't make a
> lot of sense if the ECC area isn't user accessible in the first place.
> 
> I didn't think about it at the time but I can take a dump of the OOB
> area of my FS35ND01G-S1Y2 to confirm it's all 0xff except for any
> factory marked bad blocks.

I see. Can you please try the following:

nandwrite -o /dev/mtdx /dev/zero
nanddump -ol1 /dev/mtdx

If the entire area is effectively free to be used, you should see 0's
everywhere. Otherwise you should have ff's somewhere.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Daniel Palmer <daniel@0x0f.com>
Cc: linux-mtd@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mtd: spinand: add support for Foresee FS35ND01G-S1Y2
Date: Mon, 15 Feb 2021 12:16:53 +0100	[thread overview]
Message-ID: <20210215121653.4edd86c4@xps13> (raw)
In-Reply-To: <CAFr9PXkh+attaCc6C2UxB=qvXksWriWOaaoEndy4k6SGE0QOHQ@mail.gmail.com>

Hi Daniel,

Daniel Palmer <daniel@0x0f.com> wrote on Mon, 15 Feb 2021 19:53:13
+0900:

> Hi Miquel,
> 
> On Mon, 15 Feb 2021 at 19:24, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Can you please add a changelog here when you send a new version of a
> > patch?  
> 
> Sorry, I was going to add a cover letter but elsewhere got told that
> one isn't needed for a single patch..

A cover letter is useful when there are many patches, or when there is
some context that is important to remember.

But a changelog should always be added when you change something
between two versions. And the changelog can be located below the three
dashes ("---") without being part of the final commit message, it does
not need to be in a separate cover letter.

> Basically I changed FS35ND01G to FS35ND01G-S1Y2 as that's the proper
> part number for the chip I have and there seem to be a few variations
> of this.
> Aside from that I fixed up the hex numbers to be uppercase and added
> the oob layout callbacks.
> 
> > > +static int fs35nd01g_s1y2_ooblayout_free(struct mtd_info *mtd, int section,
> > > +                                 struct mtd_oob_region *region)
> > > +{
> > > +     if (section > 3)
> > > +             return -ERANGE;
> > > +
> > > +     /*
> > > +      * No ECC data is stored in the accessible OOB so the full 16 bytes
> > > +      * of each spare region is available to the user. Apparently also
> > > +      * covered by the internal ECC.  
> >
> > How is this even possible? ECC must be stored somewhere, maybe it is
> > not possible to retrieve it but I guess you cannot use the 32 bytes of
> > OOB for user data. Can you please verify that?  
> 
> This worried me too as I could not find the OOB layout anywhere.
> They simply list there being 4 512 byte main areas and then 4 16 byte
> spare areas. The only other note is that the first byte of spare0 is
> used for the bad block marker.
> 
> I contacted Longsys but they didn't get back to me.
> So what I did here was I started googling strings within the datasheet
> to find other chips that are probably the same IP inside and I found
> the FM25G01.
> It's datasheet shares a lot of the same text and the flash layout
> diagrams etc are the same.
> It has the same table for the flash layout. 4 512 byte areas and 4 16
> byte spare areas. It has the same note for the bad block marker and
> then one additional note:
> 
> "2. Spare area 800H to 83FH is all available for user.
>  ECC parity codes are programmed in
> additional space and not user accessible."
> 
> It would seem that the pages are actually bigger than 2K + 64 or there
> is some other place they keep the ECC.
> Or both datasheets are lying. Somewhere else in the datasheets it says
> that writes to the ECC area will be ignored but that doesn't make a
> lot of sense if the ECC area isn't user accessible in the first place.
> 
> I didn't think about it at the time but I can take a dump of the OOB
> area of my FS35ND01G-S1Y2 to confirm it's all 0xff except for any
> factory marked bad blocks.

I see. Can you please try the following:

nandwrite -o /dev/mtdx /dev/zero
nanddump -ol1 /dev/mtdx

If the entire area is effectively free to be used, you should see 0's
everywhere. Otherwise you should have ff's somewhere.

Thanks,
Miquèl

  reply	other threads:[~2021-02-15 11:17 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-13  9:57 [PATCH v2] mtd: spinand: add support for Foresee FS35ND01G-S1Y2 Daniel Palmer
2021-02-13  9:57 ` Daniel Palmer
2021-02-15 10:24 ` Miquel Raynal
2021-02-15 10:24   ` Miquel Raynal
2021-02-15 10:53   ` Daniel Palmer
2021-02-15 10:53     ` Daniel Palmer
2021-02-15 11:16     ` Miquel Raynal [this message]
2021-02-15 11:16       ` Miquel Raynal
2021-02-15 11:34       ` Daniel Palmer
2021-02-15 11:34         ` Daniel Palmer
2021-03-22 12:44       ` Daniel Palmer
2021-03-22 12:44         ` Daniel Palmer
2021-03-22 18:32         ` Miquel Raynal
2021-03-22 18:32           ` Miquel Raynal
2021-03-23  9:33           ` Daniel Palmer
2021-03-23  9:33             ` Daniel Palmer
2021-03-23 10:32             ` Miquel Raynal
2021-03-23 10:32               ` Miquel Raynal
2021-03-23 11:47               ` Daniel Palmer
2021-03-23 11:47                 ` Daniel Palmer
2021-03-23 14:06                 ` Miquel Raynal
2021-03-23 14:06                   ` Miquel Raynal
2021-03-23 14:14                   ` Daniel Palmer
2021-03-23 14:14                     ` Daniel Palmer
2021-03-26 14:09                   ` Daniel Palmer
2021-03-26 14:09                     ` Daniel Palmer
2021-04-07  8:02                     ` Miquel Raynal
2021-04-07  8:02                       ` Miquel Raynal
2021-04-07 12:01                       ` Daniel Palmer
2021-04-07 12:01                         ` Daniel Palmer
2021-04-08 15:49                         ` Miquel Raynal
2021-04-08 15:49                           ` Miquel Raynal

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=20210215121653.4edd86c4@xps13 \
    --to=miquel.raynal@bootlin.com \
    --cc=daniel@0x0f.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.