Hi Juerg, hi Philip, On Thu, 1 May 2008 11:16:03 -0700, Juerg Haefliger wrote: > > Jean Delvare wrote: > > > Drop a lot of useless register defines, conversion macros, data structure > > > members and update code. All these register values were read from the > > > device but nothing is done out of them, so this is all dead code in > > > practice. > > > > The code to use all of those registers was posted in 2004: > > > > http://lists.lm-sensors.org/pipermail/lm-sensors/2004-August/008563.html > > > > But was apparently not picked up because there was an objection to the size > > of the patch? > > > > The patch was streamlined to try and match the "standard" for PWM control > > (which doesn't map to the LM85 registers very well) and reposted: > > > > http://lists.lm-sensors.org/pipermail/lm-sensors/2004-September/009059.html > > > > but without the additional functionality unfortunately. > > > > Wouldn't the better course of action be to add the accessors for these > > tuneables rather than remove the functionality from the driver? Of course, > > that means that I probably need to do the work, but I don't have the time to > > do this right now. > > > > So could I ask that we *not* remove this "dead" code, but leave it in for > > now so that when someone does get around to adding the accessor functions, > > they won't have to submit a patch to reverse this "dead" code elimination? > > The code Jean wants to remove hasn't been used for a very long time > and who is to guarantee that it doesn't take another couple years > until it is finally put to use? In the meantime the driver is bloated > and slowed down due to accessing unused registers. It's not too hard > to add that stuff back. I don't like the term 'until someone gets > around to adding the functions'. That sound like 'never' to me or > certainly not in the near future so I vote for ripping it out. +1 Nobody cared about these "extra features" for 3.5 years, I have no reason to believe that anyone will care in the next few months or years, if ever. This visible lack of interest is the reason why I decided to just get rid of the dead code (together with my limited time to work on the driver, and the lack of test hardware.) Note that I totally expected that someone would complain about me removing this code. It's always the same story, dead or broken code can stay in the kernel and burden the maintainers and users forever, nobody cares, but as soon as someone proposes to clean things up, everybody complains that it's "unfortunate". Anyway, as Juerg wrote, it's trivial to add the code back if/when you find the time to work on this, with the added bonus that you get more freedom in what exactly you want to implement and how. As an additional note, I am not convinced that we actually want to add all these fancy features to the driver. The first reason is that they don't look terribly useful to me. The second is that the lm85 driver is supposed to implement support for a relatively generic type of device. All the fancy features are model specific, so if you add them all you end up with a very large driver, which eats memory and is hard to maintain. If you really want to implement all these extra features then I think that what you want is a dedicated driver (as Juerg did for the DME1737 for example.) Thanks, -- Jean Delvare
