On Thu, 2022-08-04 at 07:57 -0600, Simon Glass wrote:
> Hi Weijie,
>
> On Wed, 3 Aug 2022 at 21:37, Weijie Gao <weijie....@mediatek.com>
> wrote:
> >
> > The predefined NAND headers take too much spaces in the
> > mtk_image.c.
> > Moving them into a new file can significantly improve the
> > readability of
> > both mtk_image.c and the new mtk_nand_headers.c.
> >
> > This is a preparation for adding more NAND headers.
> >
> > Signed-off-by: Weijie Gao <weijie....@mediatek.com>
> > ---
> > tools/Makefile | 1 +
> > tools/mtk_image.c | 304 ++++++-----------------------------
> > ----
> > tools/mtk_image.h | 25 ----
> > tools/mtk_nand_headers.c | 286
> > ++++++++++++++++++++++++++++++++++++
> > tools/mtk_nand_headers.h | 52 +++++++
> > 5 files changed, 379 insertions(+), 289 deletions(-)
> > create mode 100644 tools/mtk_nand_headers.c
> > create mode 100644 tools/mtk_nand_headers.h
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> [..]
>
> > new file mode 100644
> > index 0000000000..691db85005
> > --- /dev/null
> > +++ b/tools/mtk_nand_headers.h
> > @@ -0,0 +1,52 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * MediaTek BootROM NAND header definitions
> > + *
> > + * Copyright (C) 2022 MediaTek Inc.
> > + * Author: Weijie Gao <weijie....@mediatek.com>
> > + */
> > +
> > +#ifndef _MTK_NAND_HEADERS_H
> > +#define _MTK_NAND_HEADERS_H
> > +
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +
> > +struct nand_header_info {
> > + uint32_t page_size;
> > + uint32_t spare_size;
> > + uint32_t gfh_offset;
> > +};
> > +
> > +/* AP BROM Header for NAND */
>
> Where is this documented?
This wasn't documented. The bootrom doesn't know the configuration
of the nand device. It has to read the first page to retrive necessary
information from this header.
>
> > +union nand_boot_header {
> > + struct {
> > + char name[12];
> > + char version[4];
> > + char id[8];
> > + uint16_t ioif;
> > + uint16_t pagesize;
> > + uint16_t addrcycles;
> > + uint16_t oobsize;
> > + uint16_t pages_of_block;
> > + uint16_t numblocks;
> > + uint16_t writesize_shift;
> > + uint16_t erasesize_shift;
> > + uint8_t dummy[60];
> > + uint8_t ecc_parity[28];
> > + };
> > +
> > + uint8_t data[0x80];
> > +};
> > +
> > +#define NAND_BOOT_NAME "BOOTLOADER!"
> > +#define NAND_BOOT_VERSION "V006"
> > +#define NAND_BOOT_ID "NFIINFO"
> > +
> > +const union nand_boot_header *mtk_nand_header_find(const char
> > *name);
> > +uint32_t mtk_nand_header_size(const union nand_boot_header
> > *hdr_nand);
> > +int mtk_nand_header_info(const void *ptr, struct nand_header_info
> > *info);
> > +bool is_mtk_nand_header(const void *ptr);
> > +uint32_t mtk_nand_header_put(const union nand_boot_header
> > *hdr_nand, void
> > *ptr);
>
> Please comment each of these.
OK.
>
> > +
> > +#endif /* _MTK_NAND_HEADERS_H */
> > --
> > 2.17.1
> >
>
> Regards,
> SImon
