Hi,
On 15/11/2023 13:36, Heiko Schocher wrote:
> Hello Roger,
>
> On 15.11.23 12:09, Roger Quadros wrote:
>>
>>
>> On 15/11/2023 07:40, Heiko Schocher wrote:
>>> commit 04fcd2587321 ("mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based
>>> correction")
>>>
>>> broke AM335x based boards booting from NAND with ECC BCH8 code.
>>>
>>> Use omap_enable_hwecc() instead of omap_enable_hwecc_bch()
>>> in SPL restores correct SPL nand_read_page functionality.
>>>
>>> Tested on draco thuban board.
>>>
>>> Signed-off-by: Heiko Schocher <h...@denx.de>
>>>
>>> ---
>>> fix NAND boot for BCH8 based TI AM335x boards
>>>
>>> Fix is based on series from Enrico:
>>>
>>> https://lists.denx.de/pipermail/u-boot/2023-November/536793.html
>>>
>>> and fixes NAND boot for the draco thuban board. But this patch
>>> apply also without the patches from above patchseries, see
>>> azure build:
>>>
>>> https://dev.azure.com/hs0298/hs/_build/results?buildId=111&view=results
>>>
>>> which is clean.
>>>
>>> Above commit seems to change only U-Boot code and did not
>>> adapt am335x_spl_bch.c, which breaks nand_read_page in SPL
>>> code for AM335x based boards. So use in SPL "old" hw setup
>>> and reading page from NAND in SPL works again.
>>>
>>>
>>> drivers/mtd/nand/raw/omap_gpmc.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/raw/omap_gpmc.c
>>> b/drivers/mtd/nand/raw/omap_gpmc.c
>>> index 1a5ed0de31..c9b66dadbd 100644
>>> --- a/drivers/mtd/nand/raw/omap_gpmc.c
>>> +++ b/drivers/mtd/nand/raw/omap_gpmc.c
>>> @@ -983,7 +983,11 @@ static int omap_select_ecc_scheme(struct nand_chip
>>> *nand,
>>> nand->ecc.strength = 8;
>>> nand->ecc.size = SECTOR_BYTES;
>>> nand->ecc.bytes = 14;
>>> +#if defined(CONFIG_SPL_BUILD)
>>> + nand->ecc.hwctl = omap_enable_hwecc;
>>
>> I don't think this is correct. omap_enable_hwecc does not setup
>> the BCH settings and should be used only for 1-bit ECC i.e.
>> OMAP_ECC_HAM1_CODE_*
>>
>> I'm sure it will break the boards not using
>> CONFIG_SPL_NAND_AM33XX_BCH which is
>> pretty much most TI boards that are not AM335x.
>>
>> Now, I need to identify why AM335x NAND is broken.
>
> Yes, I played with setting up stuff in drivers/mtd/nand/raw/am335x_spl_bch.c
> but failed ...
>
> If I can help you testing, feel free to ask!
OK. So I did some tests and we are ending up in a different ECC config on
AM335x.
The ecc.steps is not set at all by the driver (it is at 0) and so so nsectors
gets
set to -1 and so 7 (0b111) which is wrong.
static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd,
int mode)
{
...
case OMAP_ECC_BCH8_CODE_HW:
bch_type = 1;
nsectors = chip->ecc.steps;
...
/* BCH configuration */
val = ((1 << 16) | /* enable BCH */
(bch_type << 12) | /* BCH4/BCH8/BCH16 */
(wr_mode << 8) | /* wrap mode */
(dev_width << 7) | /* bus width */
--> (((nsectors - 1) & 0x7) << 4) | /* number of sectors */
(info->cs << 1) | /* ECC CS */
(0x1));
...
}
setting chip->ecc.steps needs to be fixed in original commit
c3754e9cc23a ("omap_gpmc: BCH8 support (ELM based)")
But this will still not fix the issue for AM335x as am335x_spl_bch.c
expects 1 sector at a time ECC calculation.
I also realized that the error correction logic (ELM) is not in page mode
and so is not suitable for multi-sector error correction.
This means we have to revert the commit
04fcd2587321 ("mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction")
and go back to 1 sector BCH ECC generation and correction.
I will send a patch fix in a day or two after doing some tests at my end.
--
cheers,
-roger
