Hi Boris, 2017-06-07 16:02 GMT+09:00 Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>: > On Wed, 7 Jun 2017 12:09:31 +0900 > Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: > >> >> + >> >> +static int denali_ecc_setup(struct mtd_info *mtd, struct nand_chip *chip, >> >> + struct denali_nand_info *denali) >> >> +{ >> >> + struct nand_ecc_caps caps; >> >> + int ret; >> >> + >> >> + caps.stepinfos = denali->stepinfo; >> >> + caps.nstepinfos = 1; >> >> + caps.calc_ecc_bytes = denali_calc_ecc_bytes; >> >> + caps.oob_reserve_bytes = denali->bbtskipbytes; >> > >> > If you get rid of this oob_reserve_bytes field, you can define caps as >> > a static const and even directly store ecc_caps in denali_nand_info. >> >> To make caps static const, denali_calc_ecc_bytes must be exported >> to be referenced from denali_dt/denali_pci. >> I am reluctant to do it. > > You already duplicate other information in denali_dt.c and > denali_pci.c, The ECC step-size and strength are tightly associated to each IP variant. I see duplication between denali_dt and denali_pci, but it is just because Intel and Altera happened to have the same parameters. On the other hand, denali_calc_ecc_bytes() is common to all variants because ECC algorithm is not customizable. > so what prevents you from duplicating this one-line > function? > > Also, denali core already exports 2 functions, They are entries for probe/remove. > I don't see the problem > in exporting the common nand_ecc_caps object. Why are you reluctant to > that? denali_calc_ecc_bytes() is independent of DT, PCI, or whatever. I see less reason to expose it. caps is only used on probing, so I used a local variable. I do not think it is a big problem. -- Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html