[PATCH v1 1/6] rockchip: block: add Rockchip IDB block device
Johan Jonker
jbx6244 at gmail.com
Wed Jul 6 23:09:11 CEST 2022
Hi Xavier,
Thank you for your comments.
This driver in it's current form does not seek to comply with the MTD frame work.
Goal is to write boot blocks to NAND the Rockchip way in a memory limited environment (usbplug) and to get rid of all the closed source tools !!
In this particular case with MTD nand_flash_ids of no use and no bch settings it does makes sense to load a whole frame work for only page_op's.
On 7/5/22 16:17, Xavier Drudis Ferran wrote:
>
> Thanks for your work.
>
> El Tue, Jul 05, 2022 at 03:04:15PM +0200, Johan Jonker deia:
>> From: Johan Jonker <jbx6244 at gmail.com>
>>
>> The Rockchip SoCs with a NAND as boot device need
>> a special Rockchip IDB block device to transfer the data
>> from the rockusb gadget to the NAND driver.
>>
>
> Sorry for the fast browsing, lack of experience and possibly wrong and
> noisy comment (I'm no U-Boot expert), but if you have patience for my
> curiosity... This isn't a review, I haven't read it all, just some
> small parts.
>
>> diff --git a/arch/arm/mach-rockchip/rockchip_idb.c b/arch/arm/mach-rockchip/rockchip_idb.c
>> new file mode 100644
>> index 00000000..6243131d
>> --- /dev/null
>> +++ b/arch/arm/mach-rockchip/rockchip_idb.c
> [...]
>> +struct NandParaInfo {
>> + u8 id_bytes;
>> + u8 nand_id[6];
>> + u8 vendor;
>> + u8 die_per_chip;
>> + u8 sec_per_page;
>> + u16 page_per_blk;
>> + u8 cell;
>> + u8 plane_per_die;
>> + u16 blk_per_plane;
>> + u16 operation_opt;
>> + u8 lsb_mode;
>> + u8 read_retry_mode;
>> + u8 ecc_bits;
>> + u8 access_freq;
>> + u8 opt_mode;
>> + u8 die_gap;
>> + u8 bad_block_mode;
>> + u8 multi_plane_mode;
>> + u8 slc_mode;
>> + u8 reserved[5];
>> +};
>> +
>
> Is part of this info already represented in
> nand_flash_dev in include/linux/mtd/rawnand.h ?
See answer below.
>
> And is it worth merging somehow ?
> Or should
> this be synced to something external and the
> .h file I mentioned be synced to linux, so merging
> would be more trouble than it is worth ?
This is a structure used by the manufacturer tools, ko modules and usbplugs.
For now this is only used here. If more users appear a .h file might be better.
>
>> +
>> +u16 random_seed[] = {
Although Linux rockchip-nand-controller.c and U-boot rockchip_nfc.c don't have it,
on MK808 with "H27UCG8T2ATR-BC" it has a NAND_NEED_SCRAMBLING flag.
In order to read anything written by a Rockchip tools after/beyond the boot blocks.
Not sure how to enable/disable this? With a DT property? PLease advice!
rockchip,enable-randomizer ???
For writing boot blocks only it's not needed for now.
Please advice on whether to keep or remove the randomizer? Thanks!
{"H27UCG8T2ATR-BC 64G 3.3V 8-bit",
{ .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
SZ_8K, SZ_8K, SZ_2M, NAND_NEED_SCRAMBLING, 6, 640,
NAND_ECC_INFO(40, SZ_1K)},
>> + 0x576a, 0x05e8, 0x629d, 0x45a3,
>> + 0x649c, 0x4bf0, 0x2342, 0x272e,
>> + 0x7358, 0x4ff3, 0x73ec, 0x5f70,
> [...]
>> + 0x3b2e, 0x7ec7, 0x4fd2, 0x5d28,
>> + 0x751f, 0x3ef8, 0x39b1, 0x4e49,
>> + 0x746b, 0x6ef6, 0x44be, 0x6db7,
>> +};
>
> Where does this come from ? Is it copyrightable ? If so, is it
> licensed ? fair use ? Does it need to be synced every so often with
> some external source ?
random_seed is a constant value for Rockchip only. See:
https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/rk_nand/rk_ftl_arm_v7.S#L25550
GPL-2.0+
>
>> +
>> +struct NandParaInfo NandFlashParaTbl[] = {
https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/rk_nand/rk_ftl_arm_v7.S#L25936
GPL-2.0+
>> + {6, {0x2c, 0x64, 0x44, 0x4b, 0xa9, 0x00}, 4, 1, 16, 256, 2, 2, 2048, 0x01df, 3, 17, 40, 32, 1, 0, 1, 0, 0, {0, 0, 0, 0, 0}},
>> + {6, {0x2c, 0x44, 0x44, 0x4b, 0xa9, 0x00}, 4, 1, 16, 256, 2, 2, 1064, 0x01df, 3, 17, 40, 32, 1, 0, 1, 0, 0, {0, 0, 0, 0, 0}},
>> + {6, {0x2c, 0x68, 0x04, 0x4a, 0xa9, 0x00}, 4, 1, 8, 256, 2, 2, 2048, 0x011f, 1, 0, 24, 32, 1, 0, 1, 0, 0, {0, 0, 0, 0, 0}},
> [...]
>> + {6, {0x98, 0xde, 0x94, 0x82, 0x76, 0x56}, 1, 1, 16, 256, 2, 2, 2062, 0x05d1, 1, 33, 40, 32, 2, 1, 1, 0, 0, {0, 0, 0, 0, 0}},
> [...]
>> + {6, {0xec, 0xd5, 0x94, 0x76, 0x54, 0x43}, 0, 1, 16, 128, 2, 2, 1038, 0x0119, 2, 0, 24, 36, 3, 1, 3, 0, 0, {0, 0, 0, 0, 0}},
>> + {6, {0xec, 0xd7, 0x14, 0x76, 0x54, 0xc2}, 0, 1, 16, 128, 2, 2, 2076, 0x0491, 2, 0, 24, 40, 3, 1, 3, 0, 0, {0, 0, 0, 0, 0}},
>> + {6, {0xec, 0xde, 0x94, 0xc3, 0xa4, 0xca}, 0, 1, 32, 792, 2, 1, 688, 0x04c1, 11, 50, 40, 32, 3, 1, 1, 0, 1, {0, 0, 0, 0, 0}},
>> +};
>> +
>
> Is this info partially duplicated in drivers/mtd/nand/raw/nand_ids.c ?
> Should it be merged and this added there somehow ? It seems to have
> more data, but I don't know if some items are deductible from others.
Reasons not to use nand_ids.c:
1:
>From the NandFlashParaTbl array only "4" NAND nand_flash_ids types are recognized.
That not enough given the support list from the manufacturer.
2:
struct nand_flash_dev nand_flash_ids[] contains insufficient data to decide what page pattern lsb_mode [0 to 12] should be used.
See test program below, there no relation between "pages per erase block", "sec_per_page" and lsb_mode.
3:
We can re-use/combine the source with a usbplug or RK NAND FTL driver.
4:
The MTD frame work does not have a mtd-abi.h ioctl to set different bch settings.
This driver must be "generic" and expect all possible options when scanning.
========================
int main(void)
{
struct tagNandParaInfo *gpNandParaInfo;
struct nand_flash_dev *type;
uint8_t id[8];
int i, j;
int size = sizeof(NandFlashParaTbl)/sizeof(struct tagNandParaInfo);
int size2 = sizeof(struct tagNandParaInfo);
printf("NandFlashParaTbl size %d\n", size);
printf("tagNandParaInfo size %d\n", size2);
for (i = 0; i < size; i++) {
struct tagNandParaInfo *p = (struct tagNandParaInfo*)&NandFlashParaTbl[i];
gpNandParaInfo = p;
id[0] = p->nand_id[0];
id[1] = p->nand_id[1];
id[2] = p->nand_id[2];
id[3] = p->nand_id[3];
id[4] = p->nand_id[4];
id[5] = p->nand_id[5];
id[6] = 0;
id[7] = 0;
for (type = nand_flash_ids; type->name; type++)
if (type->id_len) {
if (!strncmp((char *)type->id, (char *)id, type->id_len))
break;
} else {
if (type->dev_id == id[1])
break;
}
if (!type->name || !type->pagesize) {
//printf("\n----\n");
} else {
printf("\nNAND: %s\n", type->name);
printf("FLASH ID:%02x %02x %02x %02x %02x %02x\n",p->nand_id[0],p->nand_id[1],p->nand_id[2],p->nand_id[3],p->nand_id[4],p->nand_id[5]);
}
}
printf("\n----\n");
char lsb_mode[] = {0,1,2,3,4,5,6,9};
for (j = 0; j < sizeof(lsb_mode); j++) {
printf("lsb_mode: %d:", lsb_mode[j]);
for (i = 0; i < size; i++) {
struct tagNandParaInfo *p = (struct tagNandParaInfo*)&NandFlashParaTbl[i];
if (lsb_mode[j] == p->lsb_mode) {
printf("%d ", p->page_per_blk);
}
}
printf("\n");
}
return 0;
}
=================================
NAND: H27UBG8T2BTR-BC 32G 3.3V 8-bit
FLASH ID:ad d7 94 da 74 c3
NAND: H27UCG8T2ATR-BC 64G 3.3V 8-bit
FLASH ID:ad de 94 da 74 c4
NAND: H27QCG8T2E5R‐BCF 64G 3.3V 8-bit
FLASH ID:ad de 14 a7 42 4a
NAND: SDTNRGAMA 64G 3.3V 8-bit
FLASH ID:45 de 94 93 76 50
--page_per_blk--
lsb_mode: 0:128 64 64
lsb_mode: 1:256 256 256 256 256 128 256 256 256 256 256 256 256 256 256 256 256 256 256 256 256 256 256 256 256 256
lsb_mode: 2:256 256 256 256 256 256 256 256 128 128 256 256 128 256 256 256 256 256 256 256 256 256 256 256 256 256 256 256 128 128 128 128 128 128
lsb_mode: 3:256 256 512 256 512 512 256 256
lsb_mode: 4:512 512 512 512 512
lsb_mode: 5:512 512 512 512 512 512
lsb_mode: 6:
lsb_mode: 9:388
--sec_per_page--
lsb_mode: 0:16 8 8
lsb_mode: 1:8 16 16 8 8 8 8 8 16 16 16 16 16 32 16 16 16 16 8 8 8 8 16 16 16 16
lsb_mode: 2:32 16 32 32 32 32 32 32 16 16 32 32 16 32 32 32 32 32 32 32 32 32 32 32 32 32 32 32 16 16 16 16 16 16
lsb_mode: 3:16 16 32 16 32 32 16 16
lsb_mode: 4:32 24 32 32 32
lsb_mode: 5:32 32 32 32 32 32
lsb_mode: 6:
lsb_mode: 9:32
===================================
The manufacturer tree gives us a little clue about the lsb_mode.
Is there an other way to obtain this then from NandFlashParaTbl?
Could someone from Rockchip or a maintainer give/disclose the relation between the lsb_mode and the page write pattern? Thanks!
https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/rkflash/flash.h#L74
GPL-2.0
/* 0 1 2 3 4 5 6 7 8 9 slc */
#define LSB_0 0
/* 0 1 2 3 6 7 A B E F hynix, micron 74A */
#define LSB_1 1
/* 0 1 3 5 7 9 B D toshiba samsung sandisk */
#define LSB_2 2
/* 0 1 2 3 4 5 8 9 C D 10 11 micron 84A */
#define LSB_3 3
/* 0 1 2 3 4 5 7 8 A B E F micron L95B */
#define LSB_4 4
/* 0 1 2 3 4 5 8 9 14 15 20 21 26 27 micron B74A TLC */
#define LSB_6 6
/* 0 3 6 9 C F 12 15 18 15 1B 1E 21 24 K9ABGD8U0C TLC */
#define LSB_7 7
> Could some constants be used to make it easier to read ?
For now this is most compact readable form that passes through ./scripts/checkpatch.pl --strict
Any macro would make this line even more longer.
Suggestions welcome of course.
Johan
More information about the U-Boot
mailing list