H27UCG8T2FTR-BC is similar to the already-supported H27UCG8T2ETR-BC, but
reports a different ID.
Signed-off-by: Samuel Holland <[email protected]>
---
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
Hi again,
[email protected] wrote on Fri, 30 Dec 2022 13:45:07 +0100:
> Hi Samuel,
>
> [email protected] 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
Hi Samuel,
[email protected] 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 <[email protected]>
> ---
>
> 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) },
Hi Miquèl,
On 12/30/22 06:45, Miquel Raynal wrote:
> Hi Samuel,
>
> [email protected] 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,
},
Hi Samuel,
[email protected] wrote on Fri, 30 Dec 2022 10:08:13 -0600:
> Hi Miquèl,
>
> On 12/30/22 06:45, Miquel Raynal wrote:
> > Hi Samuel,
> >
> > [email protected] 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
On 1/2/23 04:06, Miquel Raynal wrote:
> Hi Samuel,
>
> [email protected] wrote on Fri, 30 Dec 2022 10:08:13 -0600:
>
>> Hi Miquèl,
>>
>> On 12/30/22 06:45, Miquel Raynal wrote:
>>> Hi Samuel,
>>>
>>> [email protected] 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
Hi Samuel,
[email protected] wrote on Mon, 2 Jan 2023 09:50:21 -0600:
> On 1/2/23 04:06, Miquel Raynal wrote:
> > Hi Samuel,
> >
> > [email protected] wrote on Fri, 30 Dec 2022 10:08:13 -0600:
> >
> >> Hi Miquèl,
> >>
> >> On 12/30/22 06:45, Miquel Raynal wrote:
> >>> Hi Samuel,
> >>>
> >>> [email protected] 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