Re: [PATCH v4 6/6] hwmon: (spd5118) Add configuration option for auto-detection

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

 



On 6/3/24 21:37, Thomas Weißschuh wrote:
On 2024-06-03 21:02:37+0000, Guenter Roeck wrote:
With SPD5118 chip detection for the most part handled by the i2c-smbus
core using DMI information, the spd5118 driver no longer needs to
auto-detect spd5118 compliant chips.

Auto-detection by the driver is still needed on systems with no DMI support
or on systems with more than eight DIMMs and can not be removed entirely.
However, it affects boot time and introduces the risk of mis-identifying
chips. Add configuration option to be able to disable it on systems where
chip detection is handled outside the driver.

Cc: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
v4: New patch

  drivers/hwmon/Kconfig   | 18 ++++++++++++++++++
  drivers/hwmon/spd5118.c |  4 +++-
  2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7a84e7637b51..0bb1bdee3e43 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2185,6 +2185,7 @@ config SENSORS_SPD5118
  	tristate "SPD5118 Compliant Temperature Sensors"
  	depends on I2C
  	select REGMAP_I2C
+	select SENSORS_SPD5118_DETECT if !DMI
  	help
  	  If you say yes here you get support for SPD5118 (JEDEC JESD300)
  	  compliant temperature sensors. Such sensors are found on DDR5 memory
@@ -2193,6 +2194,23 @@ config SENSORS_SPD5118
  	  This driver can also be built as a module. If so, the module
  	  will be called spd5118.
+config SENSORS_SPD5118_DETECT
+	bool "Enable detect function"
+	depends on SENSORS_SPD5118
+	default y
+	help
+	  If enabled, the driver auto-detects if a chip in the SPD address
+	  range is compliant to the SPD51888 standard and auto-instantiates
+	  if that is the case. If disabled, SPD5118 compliant devices have
+	  to be instantiated by other means. On systems with DMI support
+	  this will typically be done from DMI DDR detection code in the
+	  I2C SMBus subsystem.
+	  Disabling the detect function will speed up boot time and reduce
+	  the risk of mis-detecting SPD5118 compliant devices. In general
+	  it should only be enabled if necessary.
+
+	  If unsure, say Y.

The combination of

"In general it should only be enabled if necessary."

and

"default y" / "If unsure, say Y."

looks weird.


Yes, I know. I just couldn't find a better wording.


Also right now it is not possible to disable detection on non-DMI
configurations. But when using OF, custom kernel code or userspace
instantiation then neither DMI nor CONFIG_DETECT are necessary.


That is a good point.

The following would support those usecases, too:

config SENSORS_SPD5118_DETECT
	bool "Enable detect function"
	depends on SENSORS_SPD5118
	default !DMI

(And no "select SENSORS_SPD5118_DETECT if !DMI")


I don't think "default !DMI" would be a good idea because DMI
is supported by multiple architectures, but only two X86 specific
controllers call i2c_register_spd().

"default !DMI || !X86" might work, though. I think I'll use that.

I'll rephrase the text and drop the "select".

Thanks,
Guenter





[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