On Fri, 2020-02-14 at 11:47 +0000, Mark Brown wrote: > On Wed, Feb 12, 2020 at 06:56:37AM +0000, Vaittinen, Matti wrote: > > On Tue, 2020-02-11 at 19:06 +0000, Mark Brown wrote: > > > Note also that we already have quite extensive helpers for this > > > sort > > > of > > > stuff in the regulator API which I sense may have been involved > > > in > > > this > > > implementation > > You sense well xD > > If you're factoring stuff out of an existing implementation it'd be > good > to explicitly do that - this both shows where things came from and > also > means that you can show that the existing user works with the new > code > which is good. True. But I didn't refactor the regulator code - I stole the idea and wrote this thing in BD70528 power-supply driver. I didn't think of creating generic helper until Linus W mentioned we should create one. So now when I did the BD99954 driver and noticed I needed same helper in power-supply area again I decided to get the implementation I did in BD70528 driver and make it available for all. That was a low-hanging fruit for me as I authored the BD70528 and know the linear-range code was easy to pull out of the BD70528. Changing the regulator system was not as easy for me - although it is doable. > > But another option - which I thought only now - would be to see if > > current regulator implementation could be re-named to more generic > > and > > placed under some more generic component (I thought of regmap but > > as > > you pointed out this is equally usefull for devices connected to > > memory > > mapped buses - so maybe under lib - if static inline functions in a > > header are not a good option). I just have a feeling that the > > linear- > > ranges is currently kind of embedded in the code which is internal > > to > > regulator framework so it is probably not easily extracted from > > regulator code? > > It is a bit but I think that's solvable with some refactoring in > place > (eg, pushing things into a smaller struct embedded in the main > regulator > one and then moving them out). I might look at it myself if nobody > else > gets to it first... I need something like this in order to convert BD99954 current and voltage values to register values. I will happily use what-ever you do pull together - but if you don't feel like doing it now I might look at the regulator part while I am working with BD99954 anyways. Please just let me know if you want me to see if I can pull the range stuff out of regulator area. > > So if we do not start pulling the range code out of regulator > > framework > > (for now at least) - and if we do not place this under regmap - > > then I > > can drop you out of the recipient list for this charger driver in > > order > > to not pollute your inbox ;) How do you feel Mark, do you want to > > be > > following this series? > > Well, if there's a refactoring out of the regulator code going on > I'll > need to look at that anyway. My first idea was not to change the regulators now - hence I asked if I should drop you. But I definitely need your support if we decide to refactor the regulator code in this series and create these common helpers out of it. Br, Matti Vaittinen
