linux-kernel - Re: [PATCH 1/3] mtd: rawnand: hynix: Add support for H27UCG8T2FTR-BC MLC NAND

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230102175932.6c11a0b3@xps-13>
Date:   Mon, 2 Jan 2023 17:59:32 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Samuel Holland <samuel@...lland.org>
Cc:     Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] mtd: rawnand: hynix: Add support for
 H27UCG8T2FTR-BC MLC NAND

Hi Samuel,

samuel@...lland.org wrote on Mon, 2 Jan 2023 09:50:21 -0600:

> On 1/2/23 04:06, Miquel Raynal wrote:
> > Hi Samuel,
> > 
> > samuel@...lland.org wrote on Fri, 30 Dec 2022 10:08:13 -0600:
> > 
> >> Hi Miquèl,
> >>
> >> On 12/30/22 06:45, Miquel Raynal wrote:
> >>> Hi Samuel,
> >>>
> >>> samuel@...lland.org wrote on Thu, 29 Dec 2022 13:09:03 -0600:
> >>>   
> >>>> H27UCG8T2FTR-BC is similar to the already-supported H27UCG8T2ETR-BC, but
> >>>> reports a different ID.  
> >>>
> >>> Can you provide a datasheet for this part? I am surprised by the page
> >>> size. In general anyway, it's best to provide a link when adding
> >>> support for a new component.  
> >>
> >> I was unable to find a datasheet for specifically H27UCG8T2FTR-BC. The
> >> best datasheet I could find is for H27UCG8T2ETR-BC[0][1]. However, there
> >> are layout parameters for H27UCG8T2FTR-BC in some versions of the vendor
> >> NAND driver[2][3][4]. The Hynix chip is packaged as Essencore
> >> I3T-8GQ8T2H5TARC, as referenced in that NAND ID table, which is the
> >> actual package on the board I have.
> >>
> >> Regards,
> >> Samuel
> >>
> >> [0]:
> >> https://z3d9b7u8.stackpathcdn.com/pdf-down/H/2/7/H27UCG8T2ETR-BC-Hynix.pdf
> > 
> > Pointing to [0] or [1] in the commit log would be nice at least, even
> > though we cannot get our hands on the real datasheet...
> 
> OK, I will do that for v2.
> 
> >> [1]: http://www.zsong.com.cn/userfiles/H27UC(D)G8T(U)2ETR-BC_Rev1.0_0826.pdf
> >> [2]:
> >> https://github.com/engSinteck/A133_Image/blob/main/longan/kernel/linux-4.9/modules/nand/sun8iw15p1/phy-nand/physic_v2/nand_id2.c#L1592
> >> [3]:
> >> https://github.com/launchfur/rg818-kernel/blob/master/modules/nand/sun8iw15p1/phy-nand/physic_v2/nand_id2.c#L1592
> >> [4]: Adding member names to that table entry:
> >>
> >> {.nand_id               = {0xad, 0xde, 0x14, 0xab, 0x42, 0x4a,
> >>                            0xff, 0xff},
> >>  .die_cnt_per_chip      = 1,
> >>  .sect_cnt_per_page     = 32,
> >>  .page_cnt_per_blk      = 256,
> >>  .blk_cnt_per_die       = 2112,
> >>  #define NAND_MULTI_PROGRAM (1 << 3)
> >>  #define NAND_RANDOM        (1 << 7)
> >>  #define NAND_READ_RETRY    (1 << 8)
> >>  #define NAND_LSB_PAGE_TYPE (0xff << 12)
> >>  .operation_opt         = 0x00002188,
> >>  .valid_blk_ratio       = 896,
> >>  .access_freq           = 40,
> >>  .ecc_mode              = 8,
> >>  .read_retry_type       = 0x050804,
> >>  .ddr_type              = 0,
> >>  .option_phyisc_op_no   = 14,
> >>  .ddr_info_no           = 0,
> >>  .id_number             = 0x010026,
> >>  .max_blk_erase_times   = 3000,
> >>  .driver_no             = 1,
> >>  .access_high_freq      = 40,
> >>  .random_cmd2_send_flag = 0,
> >>  .random_addr_num       = 0,
> >>  .nand_real_page_size   = 16384 + 1664},
> > 
> > This and what is displayed in the two datasheets pointed above looks
> > very much like out-of-band data to me, I don't get why we should treat
> > this part of the array differently? The OOB area is not only supposed to
> > be used for ECC bytes (even though that's how UBI make use of it), you
> > can store all the data you want there (but it's not necessarily
> > protected by the ECC engine, which, in general, matters a lot.
> > 
> > I don't see how this datasheet would be different than the others. They
> > don't detail the geometry, I would have expected them to do it if the
> > page size was anything different than the standard?
> 
> Maybe we are misunderstanding each other. The page size I have declared
> in the table is SZ_16K, which I believe is a standard value. The
> non-power-of-two chip size comes from the number of blocks in the chip;
> the ".blk_cnt_per_die = 2112" above corresponds to the "8448" in patch 3.
> 
> For H27UCG8T2ETR this is described in the datasheet on page 3 as "1,060
> blocks x 2 plane" and "(1,024 blocks + 36 block)/1plane". These extra
> blocks are separate from the OOB area inside each page.

Oh right, sorry I messed things up in my mind. So yes it's a real
situation. If we grep chip_shift and pagemask there are a number of
users (controller drivers) which might actually be impacted. We need to
be careful. Right now I am not sure we should we should play with the
core to support these extra blocks...

Thanks,
Miquèl