Added Guenter Roeck to Cc, since he merged the hwmon patch. On 12/12/2012 09:59 AM, Jonathan Cameron wrote: > On 11/12/12 20:10, Lars-Peter Clausen wrote: >> On 12/11/2012 08:57 PM, Hartmut Knaack wrote: >>> Jonathan Cameron schrieb: >>>> On 7/16/2012 9:34 AM, Sascha Hauer wrote: >>>>> On Sun, Jul 15, 2012 at 11:11:52PM +0200, Lars-Peter Clausen wrote: >>>>>> On 07/15/2012 10:40 PM, Hartmut Knaack wrote: >>>>>>> This patchset makes the ADT7410 usable. Main reason was the following >>>>>>> error when trying to register one of these devices: >>>>>>> [ 180.945561] BUG: unable to handle kernel NULL pointer dereference >>>>>>> at (null) >>>>>>> [ 180.945592] IP: [<f84b1e80>] >>>>>>> iio_device_register_eventset+0x380/0x3a0 [industrialio] >>>>>>> This happens, since in industrialio-events.c >>>>>>> __iio_add_event_config_attrs (which does a INIT_LIST_HEAD) is not >>>>>>> called, if the channels attribute of the device is not set. As a >>>>>>> result, list_for_each_entry causes those NULL pointer dereference >>>>>>> problems. So, before trying to access those list elements, we check >>>>>>> for their existence (also in unregister functions). >>>>>>> Now the adt7410.c ended in some complex changes - my apologies for this: >>>>>>> >>>>>>> * check for exact values provided through sysfs, otherwise output >>>>>>> a verbose error/usage message for: sample mode, resolution, event mode >>>>>>> * adt7410_show_id: I don't see a point in masking out some LSBs, >>>>>>> if they get shifted to oblivion, anyway -> no more need for >>>>>>> ADT7410_MANUFACTORY_ID_MASK >>>>>>> * adt7410_convert_temperature: in 13 bit mode, the driver assumed >>>>>>> the data to be stored in bits 0-12, but according to the data sheet, >>>>>>> it is stored in bits 3-15. Temperature readings were always around >>>>>>> 200(°C), while real temperature was something around 20(°C). Simple >>>>>>> fix: mask out bits 0-2 in 13 bit mode and use the 16 bit routines >>>>>>> from that point on. >>>>>>> * adt7410_set_t_bound: handle float values provided through sysfs >>>>>>> properly. Also, simplify treatment of 13 bit values by masking out >>>>>>> bits 0-2. >>>>>>> * adt7410_probe, adt7410_remove: fix another possible NULL >>>>>>> pointer dereference issue, in case no platform data is provided. Sync >>>>>>> chip->config with the config register >>>>>> Hi, >>>>>> >>>>>> Sascha Hauer posted a couple of patches a while ago which seem to fix a >>>>>> similar set of issues. >>>>>> >>>>>> See: http://www.spinics.net/lists/linux-iio/msg05947.html and >>>>>> http://www.spinics.net/lists/linux-iio/msg05955.html >>>>> Indeed, most if not all of the mentioned issues should be fixed already >>>>> with these patches. >>>>> >>>> Just to reiterate what I said to Sascha at the time... This driver does >>>> not belong in IIO. It is very much a hardware monitoring part and so >>>> wants to move to hwmon. We aren't going to take it out of staging into >>>> drivers/iio and I'm rather unwilling to take any 'new' features into the >>>> staging version. Someone with hardware who is interesting needs to bite >>>> the bullet and convert / rewrite this driver as a hwmon device. >>>> >>> FYI, I created a basic hwmon driver for ADT7410, which now made it into >>> linux 3.7. >>> Take care >> >> Ah, great, now we have two drivers which bind to the same device... >> > Yup, can one of you take a look at the two drivers and see where they > differ in functionality. We obviously don't want this messy situation > to go on for long! Then again, we don't want to end up with missing > functionality in one of them either... The IIO driver supports more. It has support for the similar adt7310 and it also has support for reporting over- or undertemperature, something which can't really be implemented that nicely in the hwmon framework. We did think about moving this driver to hwmon, but since it does not really support threshold events and also is more meant for PC-style hardware monitoring and the adt7410 has primarily other applications I did decide against it. Another reason was that we do have the IIO-to-hwmon bridge which would allow to instantiate a hwmon userspace interface for the driver if necessary. Hartmut you knew of the existence of the IIO driver it would have really been nice if you had included us on Cc when you submitted the hwmon driver that would have allowed us to avoid this messy situation. One solution could be to just sent a patch to revert the hwmon driver patch, but I guess that would be kind of counterproductive. Hartmut any suggestions from your side how to solve this? Thanks - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
