On 12/19/2018 11:50 PM, tristram...@microchip.com wrote: >>>>>> This header file makes no sense. Please move the functions into .c >>>>> >>>>> No, that would make code bigger & slower. >>>>> >>>>> It makes sense to me. But I'd add "inline" keyword to make the goal >>>>> explicit. >>>> >>>> 1) It makes no sense to have header files for things like this. The >>>> functions are only used within the single .c file. >>>> >>>> 2) You cannot inline them, as they are used as ops. >>> >>> Ok, sorry for the noise. >> >> If you were to use regmap, this whole boilerplate would go away ... > > Sometimes I am confused about this review process. > > The new code is same as previous code submitted. The old code was accepted, > but then there are objections to the new code.
Yes, continuous quality improvement is important. > So if I change the new code, should I go back to correct the old code? Maintaining old code and testing old code is indeed most welcome. > The ksz9477_i2c.c should be the same as ksz9477_i2c.c while ksz_i2c.h matches > ksz_spi.h. The idea is ksz_i2c.h can be used by another switch driver like > ksz####_i2c.c. The regmap hides all the complexity and avoids re-implementations of these kinds of accessors over and over again. -- Best regards, Marek Vasut
