Hi Ira, On Mon, 5 Apr 2010 11:41:14 -0700, Ira W. Snyder wrote: > On Sat, Apr 03, 2010 at 08:08:01PM +0200, Jean Delvare wrote: > > Hi Ira, > > > > On Fri, 2 Apr 2010 11:09:38 -0700, Ira W. Snyder wrote: > > > But it *does* make sense to change this one: to let you use all 3 GPIO > > > pins as voltage sensors. Especially if your board is already very dense, > > > and you don't have enough space to fit another component. > > > > I think you're off the point. It's not a matter of fitting another > > component, it's a matter of picking the right component to start with. > > If you had 15 voltage lines you wanted to monitor, you should have > > chosen a chip which can monitor 15 voltage lines right away, rather > > than one that can monitor "only 13 and maybe 2 more if I add funky code > > to the driver and run user-space applications in some specific way". > > > > I guess my hardware engineer would disagree with you ;) > > The fact is, our board is packed pretty dense, there isn't much more > space for extra components. As I already wrote before, the point isn't extra components, but choosing the right component in the first place. > Also, I have ~150 of them all wired this > way, so no matter what, I have to support them. If that means I have to > forward port this patch forever, then that's what I'll do. If the hardware is already built, then indeed I can understand that you are ready to whatever ugly quirks are needed to get them to work. > > > Right, in my application, I attempt to read the sensors every 500ms. > > > Hence the 3s update time for the chip. I'm aware that not everyone reads > > > their sensors this fast. > > > > > > Would you accept a patch that zeroed out stale values (older than a few > > > seconds)? I'm aware that it is suboptimal, but I think that zeroes would > > > be better than hours-old values. I could add a big warning to the code > > > and in the Documentation directory. > > > > I don't quite agree. You're trading one evil for another. Instead of > > getting an old reading, you'll get a zero which is statistically more > > wrong. The only benefit is that it is more obviously incorrect, but it > > is also totally unfriendly for monitoring applications. Think of people > > running sensord, for example. 2 out of 3 reading cycles will return 0 > > and the user will end up with totally unusable graphs. > > > > What about returning -EINVAL for stale values? I think the sensors tool > and libsensors library will do the right thing. You'd rather test. I know that libsensors and sensors are notoriously bad at dealing with some error codes (for example the ones returned by the thinkpad driver.) So don't assume it will work, test and be ready to have to fix libsensors and/or sensors. > At least if the user is > reading "fast enough" they'll get good values each time. The meaning of > "fast enough" could be 5-10 seconds. Which is pretty arbitrary, but would work in practice, I agree. > > > As the only known user of this driver, I'm trying to play nice and > > > follow the "get it upstream before you ship it" mantra. > > > > I fail to see how it matters here. > > > > > Remember the recent fiasco with the Google Android folks? > > > > No, I have no idea what you are talking about, sorry. > > > > > The fact is, I've got a board with this chip, and it is wired in a way > > > you don't like, but is within the capabilities of the chip. I don't want > > > to forward port these changes forever. Can we please try and decide on > > > something that will make this chip work the way I have it hooked up? > > > From my point of view, you're just telling me "I don't like it, go away" > > > while I'm trying to make a chip work. > > > > You got my point totally right as I see. I understand you don't like > > it, I wouldn't like it if I were you. > > > > Don't get me wrong, you're a nice guy, that's nothing personal. Simply, > > none of your workaround proposals so far pleased me, and I can't think > > of anything better myself. I don't want to have in mainline a driver > > which behaves strangely until you read the documentation. Especially if > > this would mean making life difficult for users who did wire their own > > chip in a smart way (that is, at most one GPIO used for analog voltage > > monitoring.) > > I apologize. I was getting frustrated on Friday in the last few emails. > > If you don't like the -EINVAL idea, then I guess I'll just forward port > this forever. What I want is: * A patch fixing the current behavior of the driver, because it is broken for everybody. The driver should only expose 13 voltage channels. This should be fairly easy to fix, and the fix should go upstream ASAP. * That the 13 voltage channels setup stays the default. If you want to add a custom mode for your weird hardware setup, it needs to be explicitly enabled either by the platform (through device platform data) or by the user (through a module parameter or a sysfs attribute.) No way it can become the default behavior. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
