Hi Andrey, On 2005-11-16, Andrey Volkov wrote: > > I wrestled with the ST website for the m41t85 datasheet but lost so I > > I can only guess from the patch. The drivers do look very similar. > > It looks like the m41t85 is basically a m41t00 with an alarm (watchdog > > timer never used AFAICT). > > http://www.st.com/stonline/products/literature/ds/7531/m41st85w.pdf > (I agree ST's (and Maxim's too) site is for strength of mind men :) ). We're sliding off-topic, but I find Maxim's website quite good. > > Also there are some differences in register > > offsets and [maybe] some minor differences within the registers but > > nothing that serious. > > Mainly you're right, but, as I wrote before, due to _many_ little > differences I get #if/#else.. noise (half file, if be more precisely, > was under #if/#else) in unified file. But, if this noise > will be acceptable, then yes, I agree to merge this drivers, as minimum, > for better administration. This simply means you did it the wrong way. The use of #if/#else/#endif suggests that you made the distinction between chips at compilation time, and the generated driver binary ended up supporting only one of the two chips. This doesn't make any sense. Same is true for your output clock frequency, making it a kernel configuration option means that the decision happens at compilation time, which is very restrictive. Think of what will happen when a Linux distribution has to build a kernel for their next release. How are they going to build a kernel suitable for all systems? With your approach, they just won't be able to. They'd have to build one kernel with m41t00 support, and one with m41t85 support. Even worse, they would need one separate build for each variant of the m41t85 driver, and there are over a dozen of these (one without clock output, and one more for each allowed clock frequency). That's obviously insane. This is why you want to move the decision at the run time level rather than the compilation time level whenever possible. The clock output option can't be a configuration option. I previously suggested a sysfs file, because this gives the greatest flexibility. If you don't like the idea for whatever reason, you may go for a module parameter instead. Same reasonment holds for the m41t00 vs. m41t85 choice. You can't decide at compilation time. If we go for a common driver, it has to support both devices at the same time. Mark suggested to use platform-specific data. I'm not familiar with this, but it sounds reasonable. I don't know for sure at this point whether having a single driver is the right choice, I'll let you and Mark check it out and decide. But the right way to determine this is definitely not through the use of #if/#endif preprocessing stuff. Thanks, -- Jean Delvare
