> > > > > > Might be worth taking the opportunity to swap out these magic numbers > > > > > > now. > > > > > > > > > > There's nothing magic about them, they tell the driver how many fans > > > > > each device supports. If you don't pass them as driver_data you'll have > > > > > to derive them from the device name in the probe function. > > > > > > > > They're magic in that they're not easily identifiable. In the few > > > > moments that I looked at the patch I assumed they were device > > > > IDs. They should be clearly defined. > > > > > > They could have been device IDs, some drivers do that, and that would > > > have been equally fine. driver_data can be anything. Best thing to do > > > is to document it right above the device id array if you really find it > > > confusing (I don't.) I don't know what else exactly you had in mind, > > > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't > > > strike me as the best coding practice. > > > > On the contrary. Perhaps the nomenclature can be worked on a little, > > but if I saw the aforementioned defines I would have known instantly > > what was being defined without searching for co-located comments. Thus > > elevating the requirement for me to even mention it. Even when we > > use the .data element for very simple information such as device IDs > > we do so with a #define. > > Right, you have a point here. > > I suppose it was deemed unneeded for a ~750 lines driver nobody really > cared about. But if the driver is becoming more complex and popular > then indeed it makes sense to clean it up a little. Starting with > reordering functions to kill forward declarations ^^ Another worthwhile endeavour. :) -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
