lkml.org 
[lkml]   [2015]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k
    Date
    On Saturday 28 March 2015 15:23:15 Guenter Roeck wrote:
    > > + ---help---
    > > + This hwmon driver adds support for reporting temperature
    > > of different + sensors and controls the fans on Dell
    > > laptops via System Management + Mode provided by Dell
    > > BIOS.
    > > +
    > > + When option I8K is also enabled this driver provides
    > > legacy /proc/i8k + userspace interface for i8kutils
    > > package.
    > > +
    >
    > Please add this in alphabetic order.
    >

    ok

    > > if ACPI
    > >
    > > comment "ACPI drivers"
    > >
    > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
    > > index 1c3e458..9eec614 100644
    > > --- a/drivers/hwmon/Makefile
    > > +++ b/drivers/hwmon/Makefile
    > > @@ -155,7 +155,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) +=
    > > w83l785ts.o
    > >
    > > obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
    > > obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
    > > obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
    > >
    > > -obj-$(CONFIG_I8K) += dell-smm-hwmon.o
    > > +obj-$(CONFIG_SENSORS_DELL_SMM) += dell-smm-hwmon.o
    >
    > Same here.
    >

    ok

    > > - proc_i8k = proc_create("i8k", 0, NULL, &i8k_fops);
    > > - if (!proc_i8k)
    > > + if (!proc_create("i8k", 0, NULL, &i8k_fops))
    > >
    > > return -ENOENT;
    >
    > I would prefer not to fail here but report a warning.
    > This is no longer a fatal condition.
    >

    ok, no problem

    >
    > Can you move all the conditional functions and global
    > variables together under a single #ifdef ? That should
    > include functions to create the proc entries, and shim
    > functions for the same if I8K is not configured.
    >

    ok, I will move procfs code to one location.

    > Also, the #ifdef would not cover the case where I8K is
    > configured as module (there is no reason to force it to
    > bool). You should use "#if IS_ENABLED(CONFIG_I8K)" instead.
    >

    I think that setting CONFIG_I8K to tristate (Y/M/N) does not make
    sense... CONFIG_I8K just control if /proc/i8k will be compiled
    into dell-smm-hwmon or not.

    --
    Pali Rohár
    pali.rohar@gmail.com
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2015-03-28 23:41    [W:4.356 / U:0.172 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site