On Mon, 11 Jul 2011 23:49:10 +0200, Pavel Herrmann wrote: > On Monday 11 of July 2011 14:03:13 Guenter Roeck wrote: > > On Mon, 2011-07-11 at 16:36 -0400, Pavel Herrmann wrote: > > > the structure is dynamically allocated, but the pointer used to hold it > > > is a static global var. > > > > This is true only if CONFIG_SHARPSL_PM is defined, and it assumes that > > the driver is instantiated exactly once. That is pretty badly broken > > (the commit introducing it even admits that), and should be fixed. This > > does not happen CONFIG_SHARPSL_PM is not defined. If CONFIG_SHARPSL_PM > > _is_ defined in your environment, and you do have multiple instances of > > the driver (ie if you have multiple MAX1111 chips in your system), a > > severe problem is that max1111_read_channel() does not identify the > > driver instance. That can not be fixed with a mutex. > > if you don't have CONFIG_SHARPSL_PM then there is nothing calling > max1111_read, and thus any of the discussed doesn't matter This assumption of yours is incorrect. Even with CONFIG_SHARPSL_PM disabled, the max1111 driver creates sysfs attributes which, when read, call max1111_read(). What CONFIG_SHARPSL_PM adds is the in-kernel access. > AFAIK max1111 is only used in sharpsl devices (according to kernel drivers > anyways), and only one a piece. > this patch is meant to fix a crash, not make the driver code pretty just in > case someone else decides to use it. this patch also doesn't present any more > challenges for solving the multiple devices issue and would be necessary > either way, as drvdata is not thread-safe anyways (or I am badly mistaken) You are right, drvdata is not thread-safe, and this is the most obvious reason why your patch is needed. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
