Re: [PATCH v1 1/6] rockchip: block: add Rockchip IDB block device

[Xavier Drudis Ferran <xdrudis@tinet.cat>]

Thanks for your work. 

El Tue, Jul 05, 2022 at 03:04:15PM +0200, Johan Jonker deia:
> From: Johan Jonker <jbx6244@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 ?

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 ? 

> +
> +u16 random_seed[] = {
> +	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 ?

> +
> +struct NandParaInfo NandFlashParaTbl[] = {
> +	{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.
Could some constants be used to make it easier to read ?