Hi Arseniy, > >>>> Yes, this code looks strange. 'nsectors' is used to calculate space in OOB > >>>> that could be used by ECC engine (this value will be passed as 'oobavail' > >>>> to 'nand_ecc_choose_conf()'). Idea of 512 is to consider "worst" case > >>>> for ECC, e.g. minimal number of bytes for ECC engine (and at the same time > >>>> maximum number of free bytes). For Meson, if ECC step size is 512, then we > >>>> have 4 x 2 free bytes in OOB (if step size if 1024 then we have 2 x 2 free > >>>> bytes in OOB). > >>>> > >>>> I think this code could be reworked in the following way: > >>>> > >>>> if ECC step size is already known here (from DTS), calculate 'nsectors' using > >>>> given value (div by 512 for example). Otherwise calculate 'nsectors' in the > >>>> current manner: > >>> > >>> It will always be known when these function are run. There is no > >>> guessing here. > >> > >> Hm I checked, that but if step size is not set in DTS, here it will be 0, > >> then it will be selected in 'nand_ecc_choose_conf()' according provided 'ecc_caps' > >> and 'oobavail'... > >> > >> Anyway, I'll do the following thing: > >> > >> int nsectors; > >> > >> if (nand->ecc.size) > >> nsectors = mtd->writesize / nand->ecc.size; <--- this is for 512 ECC > > > > You should set nand->ecc.size in ->attach_chip() instead. > > Sorry, didn't get it... if ECC step size is set in DTS, then here, in chip attach > callback it will be already known (DT part was processed in 'rawnand_dt_init()'). > If ECC step size is unknown (e.g. 0 here), 'nand_ecc_choose_conf()' will set it > according provided ecc caps. What do You mean for "You should set ..." ? The current approach is wrong, it decides the number of ECC chunks (called nsectors in the driver) and then asks the core to decide the number of ECC chunks to use. Just provide mtd->oobsize - 2 as last parameter and then rely on the core's logic to find the right ECC step-size/strength? There is no point in requesting a particular step size without a specific strength, or? So I believe you should provide both in the DTS if you want particular parameters to be applied, otherwise you can let the core decide what is best. Thanks, Miquèl
