Re: [PATCH v2] hwmon: Add thermal sensor driver for Surface Aggregator Module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/7/24 9:50 PM, Guenter Roeck wrote:
On 8/7/24 12:25, Maximilian Luz wrote:
On 8/7/24 2:32 AM, Guenter Roeck wrote:
On 8/4/24 16:08, Maximilian Luz wrote:

[...]

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b60fe2e58ad6..70c6385f0ed6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2080,6 +2080,16 @@ config SENSORS_SURFACE_FAN
        Select M or Y here, if you want to be able to read the fan's speed.
+config SENSORS_SURFACE_TEMP
+    tristate "Microsoft Surface Thermal Sensor Driver"
+    depends on SURFACE_AGGREGATOR

As the kernel test robot points out, this dependency is wrong.
__ssam_device_driver_register() is only available
if SURFACE_AGGREGATOR_BUS is enabled.

Right, I should have spotted this before submission, sorry. This should
be

   depends on SURFACE_AGGREGATOR
   depends on SURFACE_AGGREGATOR_BUS


SURFACE_AGGREGATOR_BUS already depends on SURFACE_AGGREGATOR, so the extra
dependency is not needed.

Unfortunately, SURFACE_AGGREGATOR_BUS is a bool and SURFACE_AGGREGATOR
tri-state, and the inference of whether SURFACE_AGGREGATOR needs to be
built in or not breaks because of that. Meaning we could have something
like

    SENSORS_SURFACE_TEMP=y      (tri-state, module)
    SURFACE_AGGREGATOR_BUS=y    (bool, optional-code-flag)
    SURFACE_AGGREGATOR=m        (tri-state, module)

because SURFACE_AGGREGATOR_BUS is fine with either m or y. But in
reality, SENSORS_SURFACE_TEMP=y would require SURFACE_AGGREGATOR=y.

Maybe there's a better way to solve this? I guess it should be possible
to convert SURFACE_AGGREGATOR_BUS into a tri-state (even though it's
really just a flag to build a certain part of code into
SURFACE_AGGREGATOR).

I think at this point we could also consider removing that flag entirely
and just build the bus support in unconditionally... but that discussion
is outside of the scope here.

I'll fix that for v3 and likely re-spin this weekend. Anything else I
should address for that?


I didn't notice anything, but then I didn't try to build the code myself,
or try to run checkpatch. My tools run checkpatch --strict, as necessary
for hwmon submissions, so you might want to make sure that it passes.

Perfect, I already checked that. Thank you!

Best regards,
Max




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux