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

[Samuel Holland]

On 1/2/23 04:06, Miquel Raynal wrote:
> Hi Samuel,
> 
> samuel@xxxxxxxxxxxx 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@xxxxxxxxxxxx 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.

Regards,
Samuel

>> ".option_phyisc_op_no = 14" references this entry:
>> {
>>     /* 14 */
>>     .multi_plane_read_cmd = {0x60, 0x30},
>>     .multi_plane_write_cmd = {0x11, 0x81},
>>     .multi_plane_copy_read_cmd = {0x60, 0x60, 0x35},
>>     .multi_plane_copy_write_cmd = {0x85, 0x11, 0x81},
>>     .multi_plane_status_cmd = 0x78,
>>     .inter_bnk0_status_cmd = 0x78,
>>     .inter_bnk1_status_cmd = 0x78,
>>     .bad_block_flag_position = 0x00,
>>     .multi_plane_block_offset = 1024,
>> },
>>
> 
> Thanks,
> Miquèl