| Message ID | 20190718115537.2096227-1-pn@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,1/2] i.MX6: nand: extend nandbcb command for imx6UL(L) | expand |
Hi Parthiban, Thanks a lot for working on this. Please see comments below. > Firmware Configuration Block(FCB) for imx6ul(l) needs to be > BCH encoded. This patch depends on [1]. > > [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=113810 > > Signed-off-by: Parthiban Nallathambi <pn@denx.de> > --- > arch/arm/mach-imx/Kconfig | 1 + > arch/arm/mach-imx/cmd_nandbcb.c | 72 ++++++++++++++++++++++++++++++++- > 2 files changed, 71 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig > index aeb5493488..175bed601e 100644 > --- a/arch/arm/mach-imx/Kconfig > +++ b/arch/arm/mach-imx/Kconfig > @@ -74,6 +74,7 @@ config CMD_HDMIDETECT > config CMD_NANDBCB > bool "i.MX6 NAND Boot Control Block(BCB) command" > depends on NAND && CMD_MTDPARTS > + select BCH if MX6UL || MX6ULL > default y if ARCH_MX6 && NAND_MXS > help > Unlike normal 'nand write/erase' commands, this command update > diff --git a/arch/arm/mach-imx/cmd_nandbcb.c b/arch/arm/mach-imx/cmd_nandbcb.c > index 065b814b2e..34176ee6e4 100644 > --- a/arch/arm/mach-imx/cmd_nandbcb.c > +++ b/arch/arm/mach-imx/cmd_nandbcb.c > @@ -14,8 +14,10 @@ > > #include <asm/io.h> > #include <jffs2/jffs2.h> > +#include <linux/bch.h> > #include <linux/mtd/mtd.h> > > +#include <asm/arch/sys_proto.h> > #include <asm/mach-imx/imx-nandbcb.h> > #include <asm/mach-imx/imximage.cfg> > #include <mxs_nand.h> > @@ -25,6 +27,66 @@ > #define BF_VAL(v, bf) (((v) & bf##_MASK) >> bf##_OFFSET) > #define GETBIT(v, n) (((v) >> (n)) & 0x1) > > +static uint8_t reverse_bit(uint8_t b) > +{ > + b = (b & 0xf0) >> 4 | (b & 0x0f) << 4; > + b = (b & 0xcc) >> 2 | (b & 0x33) << 2; > + b = (b & 0xaa) >> 1 | (b & 0x55) << 1; > + > + return b; > +} > + > +static void encode_bch_ecc(void *buf, struct fcb_block *fcb, int eccbits) > +{ > + int i, j, m = 13; > + int blocksize = 128; > + int numblocks = 8; > + int ecc_buf_size = (m * eccbits + 7) / 8; > + struct bch_control *bch = init_bch(m, eccbits, 0); > + u8 *ecc_buf = kzalloc(ecc_buf_size, GFP_KERNEL); > + u8 *tmp_buf = kzalloc(blocksize * numblocks, GFP_KERNEL); > + u8 *psrc, *pdst; > + > + /* > + * The blocks here are bit aligned. If eccbits is a multiple of 8, > + * we just can copy bytes. Otherwiese we must move the blocks to > + * the next free bit position. > + */ > + WARN_ON(eccbits % 8); > + > + memcpy(tmp_buf, fcb, sizeof(*fcb)); > + > + for (i = 0; i < numblocks; i++) { > + memset(ecc_buf, 0, ecc_buf_size); > + psrc = tmp_buf + i * blocksize; > + pdst = buf + i * (blocksize + ecc_buf_size); > + > + /* copy data byte aligned to destination buf */ > + memcpy(pdst, psrc, blocksize); > + > + /* > + * imx-kobs use a modified encode_bch which reverse the > + * bit order of the data before calculating bch. > + * Do this in the buffer and use the bch lib here. > + */ > + for (j = 0; j < blocksize; j++) > + psrc[j] = reverse_bit(psrc[j]); > + > + encode_bch(bch, psrc, blocksize, ecc_buf); > + > + /* reverse ecc bit */ > + for (j = 0; j < ecc_buf_size; j++) > + ecc_buf[j] = reverse_bit(ecc_buf[j]); > + > + /* Here eccbuf is byte aligned and we can just copy it */ > + memcpy(pdst + blocksize, ecc_buf, ecc_buf_size); > + } > + > + free(ecc_buf); > + free(tmp_buf); > + free_bch(bch); I have used kfree() instead of free() in entire nand bcb code So, I think for consistency reason it should be kfree here. Could you please resend this patch with above mentioned changes. Other than this, > > static u8 calculate_parity_13_8(u8 d) > { > u8 p = 0; > @@ -231,8 +293,14 @@ static int nandbcb_update(struct mtd_info *mtd, loff_t off, size_t size, > goto dbbt_data_page_err; > } > > - memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); > - encode_hamming_13_8(fcb_raw_page + 12, fcb_raw_page + 12 + 512, 512); > + if (is_mx6ul() || is_mx6ull()) { > + /* 40 bit BCH, for i.MX6UL(L) */ > + encode_bch_ecc(fcb_raw_page + 32, fcb, 40); > + } else { > + memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); > + encode_hamming_13_8(fcb_raw_page + 12, > + fcb_raw_page + 12 + 512, 512); > + } > /* > * Set the first and second byte of OOB data to 0xFF, not 0x00. These > * bytes are used as the Manufacturers Bad Block Marker (MBBM). Since Acked-by: Shyam Saini <shyam.saini@amarulasolutions.com>
Hi Shyam, On 8/13/19 3:40 PM, Shyam Saini wrote: > Hi Parthiban, > > Thanks a lot for working on this. Still enabling SECURE_BOOT fails to boot. Am yet to figure out this. Do you have secure boot working from NAND? > > Please see comments below. > >> Firmware Configuration Block(FCB) for imx6ul(l) needs to be >> BCH encoded. This patch depends on [1]. >> >> [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=113810 >> >> Signed-off-by: Parthiban Nallathambi <pn@denx.de> >> --- >> arch/arm/mach-imx/Kconfig | 1 + >> arch/arm/mach-imx/cmd_nandbcb.c | 72 ++++++++++++++++++++++++++++++++- >> 2 files changed, 71 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig >> index aeb5493488..175bed601e 100644 >> --- a/arch/arm/mach-imx/Kconfig >> +++ b/arch/arm/mach-imx/Kconfig >> @@ -74,6 +74,7 @@ config CMD_HDMIDETECT >> config CMD_NANDBCB >> bool "i.MX6 NAND Boot Control Block(BCB) command" >> depends on NAND && CMD_MTDPARTS >> + select BCH if MX6UL || MX6ULL >> default y if ARCH_MX6 && NAND_MXS >> help >> Unlike normal 'nand write/erase' commands, this command update >> diff --git a/arch/arm/mach-imx/cmd_nandbcb.c b/arch/arm/mach-imx/cmd_nandbcb.c >> index 065b814b2e..34176ee6e4 100644 >> --- a/arch/arm/mach-imx/cmd_nandbcb.c >> +++ b/arch/arm/mach-imx/cmd_nandbcb.c >> @@ -14,8 +14,10 @@ >> >> #include <asm/io.h> >> #include <jffs2/jffs2.h> >> +#include <linux/bch.h> >> #include <linux/mtd/mtd.h> >> >> +#include <asm/arch/sys_proto.h> >> #include <asm/mach-imx/imx-nandbcb.h> >> #include <asm/mach-imx/imximage.cfg> >> #include <mxs_nand.h> >> @@ -25,6 +27,66 @@ >> #define BF_VAL(v, bf) (((v) & bf##_MASK) >> bf##_OFFSET) >> #define GETBIT(v, n) (((v) >> (n)) & 0x1) >> >> +static uint8_t reverse_bit(uint8_t b) >> +{ >> + b = (b & 0xf0) >> 4 | (b & 0x0f) << 4; >> + b = (b & 0xcc) >> 2 | (b & 0x33) << 2; >> + b = (b & 0xaa) >> 1 | (b & 0x55) << 1; >> + >> + return b; >> +} >> + >> +static void encode_bch_ecc(void *buf, struct fcb_block *fcb, int eccbits) >> +{ >> + int i, j, m = 13; >> + int blocksize = 128; >> + int numblocks = 8; >> + int ecc_buf_size = (m * eccbits + 7) / 8; >> + struct bch_control *bch = init_bch(m, eccbits, 0); >> + u8 *ecc_buf = kzalloc(ecc_buf_size, GFP_KERNEL); >> + u8 *tmp_buf = kzalloc(blocksize * numblocks, GFP_KERNEL); >> + u8 *psrc, *pdst; >> + >> + /* >> + * The blocks here are bit aligned. If eccbits is a multiple of 8, >> + * we just can copy bytes. Otherwiese we must move the blocks to >> + * the next free bit position. >> + */ >> + WARN_ON(eccbits % 8); >> + >> + memcpy(tmp_buf, fcb, sizeof(*fcb)); >> + >> + for (i = 0; i < numblocks; i++) { >> + memset(ecc_buf, 0, ecc_buf_size); >> + psrc = tmp_buf + i * blocksize; >> + pdst = buf + i * (blocksize + ecc_buf_size); >> + >> + /* copy data byte aligned to destination buf */ >> + memcpy(pdst, psrc, blocksize); >> + >> + /* >> + * imx-kobs use a modified encode_bch which reverse the >> + * bit order of the data before calculating bch. >> + * Do this in the buffer and use the bch lib here. >> + */ >> + for (j = 0; j < blocksize; j++) >> + psrc[j] = reverse_bit(psrc[j]); >> + >> + encode_bch(bch, psrc, blocksize, ecc_buf); >> + >> + /* reverse ecc bit */ >> + for (j = 0; j < ecc_buf_size; j++) >> + ecc_buf[j] = reverse_bit(ecc_buf[j]); >> + >> + /* Here eccbuf is byte aligned and we can just copy it */ >> + memcpy(pdst + blocksize, ecc_buf, ecc_buf_size); >> + } >> + >> + free(ecc_buf); >> + free(tmp_buf); >> + free_bch(bch); > > I have used kfree() instead of free() in entire nand bcb code > So, I think for consistency reason it should be kfree here. > > Could you please resend this patch with above mentioned changes. > Other than this, Thanks, will change it to kfree in v2. > >> >> static u8 calculate_parity_13_8(u8 d) >> { >> u8 p = 0; >> @@ -231,8 +293,14 @@ static int nandbcb_update(struct mtd_info *mtd, loff_t off, size_t size, >> goto dbbt_data_page_err; >> } >> >> - memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); >> - encode_hamming_13_8(fcb_raw_page + 12, fcb_raw_page + 12 + 512, 512); >> + if (is_mx6ul() || is_mx6ull()) { >> + /* 40 bit BCH, for i.MX6UL(L) */ >> + encode_bch_ecc(fcb_raw_page + 32, fcb, 40); >> + } else { >> + memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); >> + encode_hamming_13_8(fcb_raw_page + 12, >> + fcb_raw_page + 12 + 512, 512); >> + } >> /* >> * Set the first and second byte of OOB data to 0xFF, not 0x00. These >> * bytes are used as the Manufacturers Bad Block Marker (MBBM). Since > > Acked-by: Shyam Saini <shyam.saini@amarulasolutions.com> >
> Hi Shyam, > > On 8/13/19 3:40 PM, Shyam Saini wrote: > > Hi Parthiban, > > > > Thanks a lot for working on this. > > Still enabling SECURE_BOOT fails to boot. Am yet to figure out this. > Do you have secure boot working from NAND? I haven't tried secure boot with nand and our imx6ul board has some other nand boot issues so couldn't test it soon. > > > > > Please see comments below. > > > >> Firmware Configuration Block(FCB) for imx6ul(l) needs to be > >> BCH encoded. This patch depends on [1]. > >> > >> [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=113810 > >> > >> Signed-off-by: Parthiban Nallathambi <pn@denx.de> > >> --- > >> arch/arm/mach-imx/Kconfig | 1 + > >> arch/arm/mach-imx/cmd_nandbcb.c | 72 ++++++++++++++++++++++++++++++++- > >> 2 files changed, 71 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig > >> index aeb5493488..175bed601e 100644 > >> --- a/arch/arm/mach-imx/Kconfig > >> +++ b/arch/arm/mach-imx/Kconfig > >> @@ -74,6 +74,7 @@ config CMD_HDMIDETECT > >> config CMD_NANDBCB > >> bool "i.MX6 NAND Boot Control Block(BCB) command" > >> depends on NAND && CMD_MTDPARTS > >> + select BCH if MX6UL || MX6ULL > >> default y if ARCH_MX6 && NAND_MXS > >> help > >> Unlike normal 'nand write/erase' commands, this command update > >> diff --git a/arch/arm/mach-imx/cmd_nandbcb.c b/arch/arm/mach-imx/cmd_nandbcb.c > >> index 065b814b2e..34176ee6e4 100644 > >> --- a/arch/arm/mach-imx/cmd_nandbcb.c > >> +++ b/arch/arm/mach-imx/cmd_nandbcb.c > >> @@ -14,8 +14,10 @@ > >> > >> #include <asm/io.h> > >> #include <jffs2/jffs2.h> > >> +#include <linux/bch.h> > >> #include <linux/mtd/mtd.h> > >> > >> +#include <asm/arch/sys_proto.h> > >> #include <asm/mach-imx/imx-nandbcb.h> > >> #include <asm/mach-imx/imximage.cfg> > >> #include <mxs_nand.h> > >> @@ -25,6 +27,66 @@ > >> #define BF_VAL(v, bf) (((v) & bf##_MASK) >> bf##_OFFSET) > >> #define GETBIT(v, n) (((v) >> (n)) & 0x1) > >> > >> +static uint8_t reverse_bit(uint8_t b) > >> +{ > >> + b = (b & 0xf0) >> 4 | (b & 0x0f) << 4; > >> + b = (b & 0xcc) >> 2 | (b & 0x33) << 2; > >> + b = (b & 0xaa) >> 1 | (b & 0x55) << 1; > >> + > >> + return b; > >> +} > >> + > >> +static void encode_bch_ecc(void *buf, struct fcb_block *fcb, int eccbits) > >> +{ > >> + int i, j, m = 13; > >> + int blocksize = 128; > >> + int numblocks = 8; > >> + int ecc_buf_size = (m * eccbits + 7) / 8; > >> + struct bch_control *bch = init_bch(m, eccbits, 0); > >> + u8 *ecc_buf = kzalloc(ecc_buf_size, GFP_KERNEL); > >> + u8 *tmp_buf = kzalloc(blocksize * numblocks, GFP_KERNEL); > >> + u8 *psrc, *pdst; > >> + > >> + /* > >> + * The blocks here are bit aligned. If eccbits is a multiple of 8, > >> + * we just can copy bytes. Otherwiese we must move the blocks to > >> + * the next free bit position. > >> + */ > >> + WARN_ON(eccbits % 8); > >> + > >> + memcpy(tmp_buf, fcb, sizeof(*fcb)); > >> + > >> + for (i = 0; i < numblocks; i++) { > >> + memset(ecc_buf, 0, ecc_buf_size); > >> + psrc = tmp_buf + i * blocksize; > >> + pdst = buf + i * (blocksize + ecc_buf_size); > >> + > >> + /* copy data byte aligned to destination buf */ > >> + memcpy(pdst, psrc, blocksize); > >> + > >> + /* > >> + * imx-kobs use a modified encode_bch which reverse the > >> + * bit order of the data before calculating bch. > >> + * Do this in the buffer and use the bch lib here. > >> + */ > >> + for (j = 0; j < blocksize; j++) > >> + psrc[j] = reverse_bit(psrc[j]); > >> + > >> + encode_bch(bch, psrc, blocksize, ecc_buf); > >> + > >> + /* reverse ecc bit */ > >> + for (j = 0; j < ecc_buf_size; j++) > >> + ecc_buf[j] = reverse_bit(ecc_buf[j]); > >> + > >> + /* Here eccbuf is byte aligned and we can just copy it */ > >> + memcpy(pdst + blocksize, ecc_buf, ecc_buf_size); > >> + } > >> + > >> + free(ecc_buf); > >> + free(tmp_buf); > >> + free_bch(bch); > > > > I have used kfree() instead of free() in entire nand bcb code > > So, I think for consistency reason it should be kfree here. > > > > Could you please resend this patch with above mentioned changes. > > Other than this, > > Thanks, will change it to kfree in v2. > > > > >> > >> static u8 calculate_parity_13_8(u8 d) > >> { > >> u8 p = 0; > >> @@ -231,8 +293,14 @@ static int nandbcb_update(struct mtd_info *mtd, loff_t off, size_t size, > >> goto dbbt_data_page_err; > >> } > >> > >> - memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); > >> - encode_hamming_13_8(fcb_raw_page + 12, fcb_raw_page + 12 + 512, 512); > >> + if (is_mx6ul() || is_mx6ull()) { > >> + /* 40 bit BCH, for i.MX6UL(L) */ > >> + encode_bch_ecc(fcb_raw_page + 32, fcb, 40); > >> + } else { > >> + memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); > >> + encode_hamming_13_8(fcb_raw_page + 12, > >> + fcb_raw_page + 12 + 512, 512); > >> + } > >> /* > >> * Set the first and second byte of OOB data to 0xFF, not 0x00. These > >> * bytes are used as the Manufacturers Bad Block Marker (MBBM). Since > > > > Acked-by: Shyam Saini <shyam.saini@amarulasolutions.com> > > > > -- > Thanks, > Parthiban N > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-22 Fax: (+49)-8142-66989-80 Email: pn@denx.de
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index aeb5493488..175bed601e 100644 --- a/arch/arm/mach-imx/Kconfig +++ b/arch/arm/mach-imx/Kconfig @@ -74,6 +74,7 @@ config CMD_HDMIDETECT config CMD_NANDBCB bool "i.MX6 NAND Boot Control Block(BCB) command" depends on NAND && CMD_MTDPARTS + select BCH if MX6UL || MX6ULL default y if ARCH_MX6 && NAND_MXS help Unlike normal 'nand write/erase' commands, this command update diff --git a/arch/arm/mach-imx/cmd_nandbcb.c b/arch/arm/mach-imx/cmd_nandbcb.c index 065b814b2e..34176ee6e4 100644 --- a/arch/arm/mach-imx/cmd_nandbcb.c +++ b/arch/arm/mach-imx/cmd_nandbcb.c @@ -14,8 +14,10 @@ #include <asm/io.h> #include <jffs2/jffs2.h> +#include <linux/bch.h> #include <linux/mtd/mtd.h> +#include <asm/arch/sys_proto.h> #include <asm/mach-imx/imx-nandbcb.h> #include <asm/mach-imx/imximage.cfg> #include <mxs_nand.h> @@ -25,6 +27,66 @@ #define BF_VAL(v, bf) (((v) & bf##_MASK) >> bf##_OFFSET) #define GETBIT(v, n) (((v) >> (n)) & 0x1) +static uint8_t reverse_bit(uint8_t b) +{ + b = (b & 0xf0) >> 4 | (b & 0x0f) << 4; + b = (b & 0xcc) >> 2 | (b & 0x33) << 2; + b = (b & 0xaa) >> 1 | (b & 0x55) << 1; + + return b; +} + +static void encode_bch_ecc(void *buf, struct fcb_block *fcb, int eccbits) +{ + int i, j, m = 13; + int blocksize = 128; + int numblocks = 8; + int ecc_buf_size = (m * eccbits + 7) / 8; + struct bch_control *bch = init_bch(m, eccbits, 0); + u8 *ecc_buf = kzalloc(ecc_buf_size, GFP_KERNEL); + u8 *tmp_buf = kzalloc(blocksize * numblocks, GFP_KERNEL); + u8 *psrc, *pdst; + + /* + * The blocks here are bit aligned. If eccbits is a multiple of 8, + * we just can copy bytes. Otherwiese we must move the blocks to + * the next free bit position. + */ + WARN_ON(eccbits % 8); + + memcpy(tmp_buf, fcb, sizeof(*fcb)); + + for (i = 0; i < numblocks; i++) { + memset(ecc_buf, 0, ecc_buf_size); + psrc = tmp_buf + i * blocksize; + pdst = buf + i * (blocksize + ecc_buf_size); + + /* copy data byte aligned to destination buf */ + memcpy(pdst, psrc, blocksize); + + /* + * imx-kobs use a modified encode_bch which reverse the + * bit order of the data before calculating bch. + * Do this in the buffer and use the bch lib here. + */ + for (j = 0; j < blocksize; j++) + psrc[j] = reverse_bit(psrc[j]); + + encode_bch(bch, psrc, blocksize, ecc_buf); + + /* reverse ecc bit */ + for (j = 0; j < ecc_buf_size; j++) + ecc_buf[j] = reverse_bit(ecc_buf[j]); + + /* Here eccbuf is byte aligned and we can just copy it */ + memcpy(pdst + blocksize, ecc_buf, ecc_buf_size); + } + + free(ecc_buf); + free(tmp_buf); + free_bch(bch); +} + static u8 calculate_parity_13_8(u8 d) { u8 p = 0; @@ -231,8 +293,14 @@ static int nandbcb_update(struct mtd_info *mtd, loff_t off, size_t size, goto dbbt_data_page_err; } - memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); - encode_hamming_13_8(fcb_raw_page + 12, fcb_raw_page + 12 + 512, 512); + if (is_mx6ul() || is_mx6ull()) { + /* 40 bit BCH, for i.MX6UL(L) */ + encode_bch_ecc(fcb_raw_page + 32, fcb, 40); + } else { + memcpy(fcb_raw_page + 12, fcb, sizeof(struct fcb_block)); + encode_hamming_13_8(fcb_raw_page + 12, + fcb_raw_page + 12 + 512, 512); + } /* * Set the first and second byte of OOB data to 0xFF, not 0x00. These * bytes are used as the Manufacturers Bad Block Marker (MBBM). Since
Firmware Configuration Block(FCB) for imx6ul(l) needs to be BCH encoded. This patch depends on [1]. [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=113810 Signed-off-by: Parthiban Nallathambi <pn@denx.de> --- arch/arm/mach-imx/Kconfig | 1 + arch/arm/mach-imx/cmd_nandbcb.c | 72 ++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 2 deletions(-)