On Mon, Feb 21, 2011 at 04:33:49AM -0500, R, Durgadoss wrote: > Hi Guenter, > > Sorry for the delayed response due to the weekend. > > > I tried to install your patch, but must have done something wrong - it crashes > > for > > me with a null pointer access. System is running 2.6.35, so it might not be the > > driver's fault but something I missed while backporting it. > > > > Yes. This patch will apply cleanly on any kernel version above 2.6.36-stable. > I prefer creating these patches on 2.6.38-rc4. > I should have specified it while sending the patch..Sorry that I missed it. > > I applied the patch as-is to the latest kernel as of two hours ago and loaded it. This resulted in a null pointer dereference crash - exactly the same as I got when trying with 2.6.35. CPU is i3-540. > > > +static DEFINE_MUTEX(update_lock); > > > > > Not a good idea. You don't want any global variables. > > Ok. Shall move it back into the structure. > > > > +/* Attributes per physical CPU */ > > > +static SENSOR_DEVICE_ATTR(name, S_IRUGO, show_name, NULL, DRV_NAME); > > > +static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, show_tjmax, NULL, 0); > > > +/* Coretemp attributes */ > > > +static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, show_name, NULL, COREID); > > > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0); > > > +static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, show_ttarget, NULL, 0); > > > +static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_crit_alarm, NULL, > > 0); > > > +/* Packagetemp attributes */ > > > +static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, show_name, NULL, > > PHYS_PROCID); > > > +static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp, NULL, 1); > > > +static SENSOR_DEVICE_ATTR(temp2_max, S_IRUGO, show_ttarget, NULL, 1); > > > +static SENSOR_DEVICE_ATTR(temp2_crit_alarm, S_IRUGO, show_crit_alarm, NULL, > > 1); > > > > > Still not the right approach here. I could not test the driver, but since you > > only have two sets of sensors it can not do what it is supposed to do if there > > are multiple cores. Again, we would expect a single instance of the driver > > to handle all cores plus the package sensor. Looks like you merged core temp > > with pkg temp, but you still create an instance of the driver per core. > > And I am not sure if the package temp now shows up on each core. > > I think we are missing something here. > > In 2.6.35 version of the coretemp, there is no #ifdef CONFIG_SMP > in the probe function. This creates a hwmon device for every core > Of the CPU. (In fact I verified this in one board that was running > 2.6.35 kernel. Though the board had only One physical CPU, there were > two instances of the hwmon device, Created by Coretemp. And both of them > showed exactly same temperature always.) > The same board running a 2.6.37 showed only one hwmon device. > Then I figured out that this is due to the SMP check in the probe > function. This check eliminates the creation of redundant coretemp attributes. > (These were created for all cores in a Physical CPU in older kernels) > Hence, in newer kernels, there will be one set of attributes(temp1*) for all cores > in a Physical CPU created by Coretemp and other set (temp2*) of attributes for PKG temp. > > Kindly correct me if I am wrong. > Yes, you are. With a 4-core CPU, the assumption is that there would be either four or five tempX_input attributes, one per core plus optionally another one for the package. Your code only has temp1_input and temp2_input, thus it can not do what we expected to see from the merged driver. This has nothing to do with CONFIG_SMP. There is supposed to be one temperature reading per physical core. On 2.6.38-r55+, output with i3-540 is coretemp-isa-0000 Adapter: ISA adapter Core 0: +16.0°C (high = +89.0°C, crit = +105.0°C) coretemp-isa-0002 Adapter: ISA adapter Core 2: +23.0°C (high = +89.0°C, crit = +105.0°C) Again, expected output would be along the line of coretemp-isa-0000 Adapter: ISA adapter Core 0: +16.0°C (high = +89.0°C, crit = +105.0°C) Core 2: +23.0°C (high = +89.0°C, crit = +105.0°C) ie there should be only one instance of the driver instead of two for the two cores. You don't see Core 1 and Core 3 because those are virtual (hyperthreading) cores, not physical cores. This is the effect you are talking about above (ie the change affects physical vs. virtual cores, not physical CPUs). Guenter _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
