On Thursday 13 May 2021 20:41:06 Armin Wolf wrote: > On 13.05.21 18:53 Guenter Roeck wrote: > > On 5/13/21 9:48 AM, Pali Rohár wrote: > > > On Thursday 13 May 2021 17:45:46 W_Armin@xxxxxx wrote: > > > > From: Armin Wolf <W_Armin@xxxxxx> > > > > > > > > When support for up to 10 temp sensors and for disabling automatic BIOS > > > > fan control was added, noone updated the index values used for > > > > disallowing fan support and fan type calls. > > > > Fix those values. > > > > > > Do you mean this change, right? > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1bb46a20e73b0bb3364cff3839c9f716ed327770 > > > > > > > > > Yes, it looks like that it should have been part of that change. > > > > > > Therefore I suggest to add Fixes tag: > > > > > > Fixes: 1bb46a20e73b ("hwmon: (dell-smm) Support up to 10 temp sensors") > > > > > > Otherwise looks good! > > > > > > Reviewed-by: Pali Rohár <pali@xxxxxxxxxx> > > > > > > For future development I would suggest to rewrite/drop these magic > > > numbers as same problem can be easily repeated in future. > > > > > > > The best solution would be to rewrite the driver to use > > hwmon_device_register_with_info(), but that should be done > > by someone with access to the hardware. > > > > Guenter > Im currently doing exactly that, since i have an old dell notebook. But > that might take some time. Perfect! Thank you for looking at this. I think that this driver converged into state when it is used by more non-developer users on LTS -stable kernels as developers which use play with -rc kernels. So issues or breakages are detected sometimes after longer time. Also I think that driver is basically feature-complete so I do not have reason for investing time in its development. Currently I have only one Dell laptop and I'm not testing on it kernel patches as it is used for different daily work and also uses 4.19 LTS distribution kernel. But when you send a patch which is larger and needs more testing, I have no problem to try compile it as module for current distribution kernel and exchange .ko module at runtime. And test if functionality works as expected. > > > > > > Signed-off-by: Armin Wolf <W_Armin@xxxxxx> > > > > --- > > > > Tested on a Dell Latitude C600. > > > > --- > > > > drivers/hwmon/dell-smm-hwmon.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/hwmon/dell-smm-hwmon.c > > > > b/drivers/hwmon/dell-smm-hwmon.c > > > > index 2970892bed82..f2221ca0aa7b 100644 > > > > --- a/drivers/hwmon/dell-smm-hwmon.c > > > > +++ b/drivers/hwmon/dell-smm-hwmon.c > > > > @@ -838,10 +838,10 @@ static struct attribute *i8k_attrs[] = { > > > > static umode_t i8k_is_visible(struct kobject *kobj, struct > > > > attribute *attr, > > > > int index) > > > > { > > > > - if (disallow_fan_support && index >= 8) > > > > + if (disallow_fan_support && index >= 20) > > > > return 0; > > > > if (disallow_fan_type_call && > > > > - (index == 9 || index == 12 || index == 15)) > > > > + (index == 21 || index == 25 || index == 28)) > > > > return 0; > > > > if (index >= 0 && index <= 1 && > > > > !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP1)) > > > > -- > > > > 2.20.1 > > > > > > >
