[PATCH 0/3] mtd: rawnand: Add H27UCG8T2FTR-BC MLC NAND

[PATCH 0/3] mtd: rawnand: Add H27UCG8T2FTR-BC MLC NAND

[Samuel Holland]

This series adds support for the subject NAND chip, found in an
Allwinner A33 tablet, which appears to be a slightly newer revision of
the already-supported H27UCG8T2ETR-BC.

I initially tried declaring the chip with the number of pages from the
datasheet, but it appears the NAND core only supports power-of-two page
counts. I made a change which works for me, but I am marking it as RFC
because I am unsure of any other impacts of this change.


Samuel Holland (3):
  mtd: rawnand: hynix: Add support for H27UCG8T2FTR-BC MLC NAND
  mtd: rawnand: Support non-power-of-two chip sizes
  mtd: rawnand: hynix: Expose the full H27UCG8T2FTR-BC chip size

 drivers/mtd/nand/raw/nand_base.c  | 14 +++++---------
 drivers/mtd/nand/raw/nand_hynix.c |  4 ++++
 drivers/mtd/nand/raw/nand_ids.c   |  4 ++++
 3 files changed, 13 insertions(+), 9 deletions(-)

-- 
2.37.4

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

[Samuel Holland]

H27UCG8T2FTR-BC is similar to the already-supported H27UCG8T2ETR-BC, but
reports a different ID.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/mtd/nand/raw/nand_hynix.c | 4 ++++
 drivers/mtd/nand/raw/nand_ids.c   | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
index 0d4d4bbfdece..836f152612ab 100644
--- a/drivers/mtd/nand/raw/nand_hynix.c
+++ b/drivers/mtd/nand/raw/nand_hynix.c
@@ -721,6 +721,10 @@ static int hynix_nand_init(struct nand_chip *chip)
 		     sizeof("H27UCG8T2ETR-BC") - 1))
 		h27ucg8t2etrbc_init(chip);
 
+	if (!strncmp("H27UCG8T2FTR-BC", chip->parameters.model,
+		     sizeof("H27UCG8T2FTR-BC") - 1))
+		h27ucg8t2etrbc_init(chip);
+
 	ret = hynix_nand_rr_init(chip);
 	if (ret)
 		hynix_nand_cleanup(chip);
diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
index dacc5529b3df..167183ccb9e9 100644
--- a/drivers/mtd/nand/raw/nand_ids.c
+++ b/drivers/mtd/nand/raw/nand_ids.c
@@ -55,6 +55,10 @@ struct nand_flash_dev nand_flash_ids[] = {
 		{ .id = {0xad, 0xde, 0x14, 0xa7, 0x42, 0x4a} },
 		  SZ_16K, SZ_8K, SZ_4M, NAND_NEED_SCRAMBLING, 6, 1664,
 		  NAND_ECC_INFO(40, SZ_1K) },
+	{"H27UCG8T2FTR-BC 64G 3.3V 8-bit",
+		{ .id = {0xad, 0xde, 0x14, 0xab, 0x42, 0x4a} },
+		  SZ_16K, SZ_8K, SZ_4M, NAND_NEED_SCRAMBLING, 6, 1664,
+		  NAND_ECC_INFO(40, SZ_1K) },
 	{"TH58NVG2S3HBAI4 4G 3.3V 8-bit",
 		{ .id = {0x98, 0xdc, 0x91, 0x15, 0x76} },
 		  SZ_2K, SZ_512, SZ_128K, 0, 5, 128, NAND_ECC_INFO(8, SZ_512) },
-- 
2.37.4

[RFC PATCH 2/3] mtd: rawnand: Support non-power-of-two chip sizes

[Samuel Holland]

Some NAND chips have a number of pages that is not exactly a power of
two. Support this by calculating the shifts and masks for the next
larger power of two.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/mtd/nand/raw/nand_base.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c3cc66039925..f46bad7796ed 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5003,6 +5003,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	u8 *id_data = chip->id.data;
 	u8 maf_id, dev_id;
 	u64 targetsize;
+	u32 chip_page_shift;
 
 	/*
 	 * Let's start by initializing memorg fields that might be left
@@ -5148,18 +5149,13 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	chip->page_shift = ffs(mtd->writesize) - 1;
 	/* Convert chipsize to number of pages per chip -1 */
 	targetsize = nanddev_target_size(&chip->base);
-	chip->pagemask = (targetsize >> chip->page_shift) - 1;
+	chip_page_shift = order_base_2(targetsize >> chip->page_shift);
+	chip->pagemask = BIT(chip_page_shift) - 1;
 
 	chip->bbt_erase_shift = chip->phys_erase_shift =
 		ffs(mtd->erasesize) - 1;
-	if (targetsize & 0xffffffff)
-		chip->chip_shift = ffs((unsigned)targetsize) - 1;
-	else {
-		chip->chip_shift = ffs((unsigned)(targetsize >> 32));
-		chip->chip_shift += 32 - 1;
-	}
-
-	if (chip->chip_shift - chip->page_shift > 16)
+	chip->chip_shift = chip_page_shift + chip->page_shift;
+	if (chip_page_shift > 16)
 		chip->options |= NAND_ROW_ADDR_3;
 
 	chip->badblockbits = 8;
-- 
2.37.4

[RFC PATCH 3/3] mtd: rawnand: hynix: Expose the full H27UCG8T2FTR-BC chip size

[Samuel Holland]

This chip has slightly more than 8K pages. Now that non-power-of-two
chip sizes are supported, make the full chip available for use.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/mtd/nand/raw/nand_ids.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
index 167183ccb9e9..f20a65aa090f 100644
--- a/drivers/mtd/nand/raw/nand_ids.c
+++ b/drivers/mtd/nand/raw/nand_ids.c
@@ -57,7 +57,7 @@ struct nand_flash_dev nand_flash_ids[] = {
 		  NAND_ECC_INFO(40, SZ_1K) },
 	{"H27UCG8T2FTR-BC 64G 3.3V 8-bit",
 		{ .id = {0xad, 0xde, 0x14, 0xab, 0x42, 0x4a} },
-		  SZ_16K, SZ_8K, SZ_4M, NAND_NEED_SCRAMBLING, 6, 1664,
+		  SZ_16K, 8448, SZ_4M, NAND_NEED_SCRAMBLING, 6, 1664,
 		  NAND_ECC_INFO(40, SZ_1K) },
 	{"TH58NVG2S3HBAI4 4G 3.3V 8-bit",
 		{ .id = {0x98, 0xdc, 0x91, 0x15, 0x76} },
-- 
2.37.4

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

[Miquel Raynal]

Hi Samuel,

samuel@sholland.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.

Also, for your two series, no need to resend this time, but please use
git-format-patch and git-send-email to create your series, so that all
the patches are answers of the cover letter. It helps keeping all
patches and answers in the series packed together.

Thanks!
Miquèl

> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  drivers/mtd/nand/raw/nand_hynix.c | 4 ++++
>  drivers/mtd/nand/raw/nand_ids.c   | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
> index 0d4d4bbfdece..836f152612ab 100644
> --- a/drivers/mtd/nand/raw/nand_hynix.c
> +++ b/drivers/mtd/nand/raw/nand_hynix.c
> @@ -721,6 +721,10 @@ static int hynix_nand_init(struct nand_chip *chip)
>  		     sizeof("H27UCG8T2ETR-BC") - 1))
>  		h27ucg8t2etrbc_init(chip);
>  
> +	if (!strncmp("H27UCG8T2FTR-BC", chip->parameters.model,
> +		     sizeof("H27UCG8T2FTR-BC") - 1))
> +		h27ucg8t2etrbc_init(chip);
> +
>  	ret = hynix_nand_rr_init(chip);
>  	if (ret)
>  		hynix_nand_cleanup(chip);
> diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
> index dacc5529b3df..167183ccb9e9 100644
> --- a/drivers/mtd/nand/raw/nand_ids.c
> +++ b/drivers/mtd/nand/raw/nand_ids.c
> @@ -55,6 +55,10 @@ struct nand_flash_dev nand_flash_ids[] = {
>  		{ .id = {0xad, 0xde, 0x14, 0xa7, 0x42, 0x4a} },
>  		  SZ_16K, SZ_8K, SZ_4M, NAND_NEED_SCRAMBLING, 6, 1664,
>  		  NAND_ECC_INFO(40, SZ_1K) },
> +	{"H27UCG8T2FTR-BC 64G 3.3V 8-bit",
> +		{ .id = {0xad, 0xde, 0x14, 0xab, 0x42, 0x4a} },
> +		  SZ_16K, SZ_8K, SZ_4M, NAND_NEED_SCRAMBLING, 6, 1664,
> +		  NAND_ECC_INFO(40, SZ_1K) },
>  	{"TH58NVG2S3HBAI4 4G 3.3V 8-bit",
>  		{ .id = {0x98, 0xdc, 0x91, 0x15, 0x76} },
>  		  SZ_2K, SZ_512, SZ_128K, 0, 5, 128, NAND_ECC_INFO(8, SZ_512) },

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

[Miquel Raynal]

Hi again,

miquel.raynal@bootlin.com wrote on Fri, 30 Dec 2022 13:45:07 +0100:

> Hi Samuel,
> 
> samuel@sholland.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.
> 
> Also, for your two series, no need to resend this time, but please use
> git-format-patch and git-send-email to create your series, so that all
> the patches are answers of the cover letter. It helps keeping all
> patches and answers in the series packed together.

You can ignore this, I did momentarily change the display inadvertently
on my side, your series are perfectly fine.

Sorry for the noise.
Miquèl

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

[Samuel Holland]

Hi Miquèl,

On 12/30/22 06:45, Miquel Raynal wrote:
> Hi Samuel,
> 
> samuel@sholland.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
[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},

".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,
},

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

[Miquel Raynal]

Hi Samuel,

samuel@sholland.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@sholland.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...

> [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?

> ".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

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@sholland.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@sholland.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.

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

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

[Miquel Raynal]

Hi Samuel,

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

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