On Wed, 30 Nov 2016 15:20:10 +0900 Masahiro Yamada <yamada.masah...@socionext.com> wrote:
> Hi Boris, > > > 2016-11-28 1:09 GMT+09:00 Boris Brezillon > <boris.brezil...@free-electrons.com>: > &max_bitflips); > > > > Okay, so you currently have two ways of handling ECC errors. What if a > > new revision introduces yet another way to do it? > > > > How about making denali_caps a structure where you have one (or several) > > function pointers to implement operations differently depending on the > > IP revision? > > > > struct denali_caps { > > u32 feature_flags; /* If needed. */ > > bool (*handle_ecc)(...); > > ... > > }; > > > > I think a problem is the difference of function arguments: > > static bool denali_hw_ecc_fixup(struct denali_nand_info *denali, > unsigned int *max_bitflips) > > vs > > static bool denali_sw_ecc_fixup(struct denali_nand_info *denali, u8 *buf, > u32 irq_status, unsigned int *max_bitflips) > > > I do not want to pass redundant arguments, > which are used for one, but not used for the other. > We do that all the time when defining generic interfaces. > > We do not need to think about the situation that may not happen. > If happens, we can refactor the code any time. > Well, as I said in my other reply, I still think it's better to plan for this now, rather than having to change a lot things when we appear to need this. But that's only my POV, and I don't care enough to fight.