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@sang-engineering.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
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.
+
config SENSORS_TC74
tristate "Microchip TC74"
depends on I2C
diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5cb5e52c0a38..19d203283a21 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -313,7 +313,7 @@ static bool spd5118_vendor_valid(u8 bank, u8 id)
}
/* Return 0 if detection is successful, -ENODEV otherwise */
-static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
+static int __maybe_unused spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
{
struct i2c_adapter *adapter = client->adapter;
int regval;
@@ -647,7 +647,9 @@ static struct i2c_driver spd5118_driver = {
},
.probe = spd5118_probe,
.id_table = spd5118_id,
+#ifdef CONFIG_SENSORS_SPD5118_DETECT
.detect = spd5118_detect,
+#endif
.address_list = normal_i2c,
};
--
2.39.2> +#ifdef CONFIG_SENSORS_SPD5118_DETECT > .detect = spd5118_detect, > +#endif > .address_list = normal_i2c, > }; So, I think we have too many Kconfig symbols already. What about using !DMI here?
On 6/4/24 00:44, Wolfram Sang wrote: >> +#ifdef CONFIG_SENSORS_SPD5118_DETECT >> .detect = spd5118_detect, >> +#endif >> .address_list = normal_i2c, >> }; > > So, I think we have too many Kconfig symbols already. What about using > !DMI here? > That would be nice, but i2c_register_spd() is only called from i2c-i801.c and, with the recent patch, from i2c-piix4.c. DMI is also supported on arm, arm64, loongarch, and mips. i2c_register_spd() will not be called on those systems or, more generally, if the adapter used to connect to the DIMMs is not one of those two. On top of that, it may be desirable to disable it for a system with DMI support if that system boots from devicetree. I don't know if such a system exists, but I would not want to bet that it doesn't. Thanks, Guenter
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@sang-engineering.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> 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.
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.
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")
> +
> config SENSORS_TC74
> tristate "Microchip TC74"
> depends on I2C
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5cb5e52c0a38..19d203283a21 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -313,7 +313,7 @@ static bool spd5118_vendor_valid(u8 bank, u8 id)
> }
>
> /* Return 0 if detection is successful, -ENODEV otherwise */
> -static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
> +static int __maybe_unused spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
> {
> struct i2c_adapter *adapter = client->adapter;
> int regval;
> @@ -647,7 +647,9 @@ static struct i2c_driver spd5118_driver = {
> },
> .probe = spd5118_probe,
> .id_table = spd5118_id,
> +#ifdef CONFIG_SENSORS_SPD5118_DETECT
> .detect = spd5118_detect,
> +#endif
> .address_list = normal_i2c,
.address_list is also only needed with CONFIG_SENSORS_SPD5118_DETECT.
If you use
.detect = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? spd5118_detect : NULL,
.address_list = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
then the need for __maybe_unused goes away and type checking is a tiny
bit better.
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@sang-engineering.com> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> 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
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@sang-engineering.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Sent as v4a to avoid resending the entire series.
v4a:
Do not auto-select SENSORS_SPD5118_DETECT if DMI is disabled
Modify help text of SENSORS_SPD5118_DETECT
Default SENSORS_SPD5118_DETECT to y if (!DMI || !X86)
v4: New patch
drivers/hwmon/Kconfig | 19 +++++++++++++++++++
drivers/hwmon/spd5118.c | 4 +++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7a84e7637b51..d5eced417fc3 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2193,6 +2193,25 @@ 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 (!DMI || !X86)
+ 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 X86 systems with DMI support
+ this will typically be done from DMI DDR detection code in the
+ I2C SMBus subsystem. Devicetree based systems will instantiate
+ attached devices if the DIMMs are listed in the devicetree file.
+
+ Disabling the detect function will speed up boot time and reduce
+ the risk of mis-detecting SPD5118 compliant devices. However, it
+ may result in missed DIMMs under some circumstances.
+
+ If unsure, say Y.
+
config SENSORS_TC74
tristate "Microchip TC74"
depends on I2C
diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5cb5e52c0a38..19d203283a21 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -313,7 +313,7 @@ static bool spd5118_vendor_valid(u8 bank, u8 id)
}
/* Return 0 if detection is successful, -ENODEV otherwise */
-static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
+static int __maybe_unused spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
{
struct i2c_adapter *adapter = client->adapter;
int regval;
@@ -647,7 +647,9 @@ static struct i2c_driver spd5118_driver = {
},
.probe = spd5118_probe,
.id_table = spd5118_id,
+#ifdef CONFIG_SENSORS_SPD5118_DETECT
.detect = spd5118_detect,
+#endif
.address_list = normal_i2c,
};
--
2.39.2Am 05.06.24 um 04:19 schrieb Guenter Roeck:
> 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.
Tested-by: Armin Wolf <W_Armin@gmx.de>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Sent as v4a to avoid resending the entire series.
>
> v4a:
> Do not auto-select SENSORS_SPD5118_DETECT if DMI is disabled
> Modify help text of SENSORS_SPD5118_DETECT
> Default SENSORS_SPD5118_DETECT to y if (!DMI || !X86)
>
> v4: New patch
>
> drivers/hwmon/Kconfig | 19 +++++++++++++++++++
> drivers/hwmon/spd5118.c | 4 +++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7a84e7637b51..d5eced417fc3 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2193,6 +2193,25 @@ 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 (!DMI || !X86)
> + 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 X86 systems with DMI support
> + this will typically be done from DMI DDR detection code in the
> + I2C SMBus subsystem. Devicetree based systems will instantiate
> + attached devices if the DIMMs are listed in the devicetree file.
> +
> + Disabling the detect function will speed up boot time and reduce
> + the risk of mis-detecting SPD5118 compliant devices. However, it
> + may result in missed DIMMs under some circumstances.
> +
> + If unsure, say Y.
> +
> config SENSORS_TC74
> tristate "Microchip TC74"
> depends on I2C
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5cb5e52c0a38..19d203283a21 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -313,7 +313,7 @@ static bool spd5118_vendor_valid(u8 bank, u8 id)
> }
>
> /* Return 0 if detection is successful, -ENODEV otherwise */
> -static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
> +static int __maybe_unused spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
> {
> struct i2c_adapter *adapter = client->adapter;
> int regval;
> @@ -647,7 +647,9 @@ static struct i2c_driver spd5118_driver = {
> },
> .probe = spd5118_probe,
> .id_table = spd5118_id,
> +#ifdef CONFIG_SENSORS_SPD5118_DETECT
> .detect = spd5118_detect,
> +#endif
> .address_list = normal_i2c,
> };
>
On 2024-06-04 19:19:07+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@sang-engineering.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Sent as v4a to avoid resending the entire series.
>
> v4a:
> Do not auto-select SENSORS_SPD5118_DETECT if DMI is disabled
> Modify help text of SENSORS_SPD5118_DETECT
> Default SENSORS_SPD5118_DETECT to y if (!DMI || !X86)
>
> v4: New patch
>
> drivers/hwmon/Kconfig | 19 +++++++++++++++++++
> drivers/hwmon/spd5118.c | 4 +++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7a84e7637b51..d5eced417fc3 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2193,6 +2193,25 @@ 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 (!DMI || !X86)
> + 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 X86 systems with DMI support
> + this will typically be done from DMI DDR detection code in the
> + I2C SMBus subsystem. Devicetree based systems will instantiate
> + attached devices if the DIMMs are listed in the devicetree file.
> +
> + Disabling the detect function will speed up boot time and reduce
> + the risk of mis-detecting SPD5118 compliant devices. However, it
> + may result in missed DIMMs under some circumstances.
> +
> + If unsure, say Y.
> +
> config SENSORS_TC74
> tristate "Microchip TC74"
> depends on I2C
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5cb5e52c0a38..19d203283a21 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -313,7 +313,7 @@ static bool spd5118_vendor_valid(u8 bank, u8 id)
> }
>
> /* Return 0 if detection is successful, -ENODEV otherwise */
> -static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
> +static int __maybe_unused spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
> {
> struct i2c_adapter *adapter = client->adapter;
> int regval;
> @@ -647,7 +647,9 @@ static struct i2c_driver spd5118_driver = {
> },
> .probe = spd5118_probe,
> .id_table = spd5118_id,
> +#ifdef CONFIG_SENSORS_SPD5118_DETECT
> .detect = spd5118_detect,
> +#endif
> .address_list = normal_i2c,
> };
For the if-deffery I proposed something during the last review,
I'm not sure if you saw it. If you did, please ignore this comment:
.address_list is also only needed with CONFIG_SENSORS_SPD5118_DETECT.
If you use
.detect = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? spd5118_detect : NULL,
.address_list = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
then the need for __maybe_unused goes away and type checking is a tiny
bit better.
On 6/5/24 02:22, Thomas Weißschuh wrote:
> On 2024-06-04 19:19:07+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@sang-engineering.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> Sent as v4a to avoid resending the entire series.
>>
>> v4a:
>> Do not auto-select SENSORS_SPD5118_DETECT if DMI is disabled
>> Modify help text of SENSORS_SPD5118_DETECT
>> Default SENSORS_SPD5118_DETECT to y if (!DMI || !X86)
>>
>> v4: New patch
>>
>> drivers/hwmon/Kconfig | 19 +++++++++++++++++++
>> drivers/hwmon/spd5118.c | 4 +++-
>> 2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 7a84e7637b51..d5eced417fc3 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -2193,6 +2193,25 @@ 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 (!DMI || !X86)
>> + 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 X86 systems with DMI support
>> + this will typically be done from DMI DDR detection code in the
>> + I2C SMBus subsystem. Devicetree based systems will instantiate
>> + attached devices if the DIMMs are listed in the devicetree file.
>> +
>> + Disabling the detect function will speed up boot time and reduce
>> + the risk of mis-detecting SPD5118 compliant devices. However, it
>> + may result in missed DIMMs under some circumstances.
>> +
>> + If unsure, say Y.
>> +
>> config SENSORS_TC74
>> tristate "Microchip TC74"
>> depends on I2C
>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>> index 5cb5e52c0a38..19d203283a21 100644
>> --- a/drivers/hwmon/spd5118.c
>> +++ b/drivers/hwmon/spd5118.c
>> @@ -313,7 +313,7 @@ static bool spd5118_vendor_valid(u8 bank, u8 id)
>> }
>>
>> /* Return 0 if detection is successful, -ENODEV otherwise */
>> -static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
>> +static int __maybe_unused spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
>> {
>> struct i2c_adapter *adapter = client->adapter;
>> int regval;
>> @@ -647,7 +647,9 @@ static struct i2c_driver spd5118_driver = {
>> },
>> .probe = spd5118_probe,
>> .id_table = spd5118_id,
>> +#ifdef CONFIG_SENSORS_SPD5118_DETECT
>> .detect = spd5118_detect,
>> +#endif
>> .address_list = normal_i2c,
>> };
>
> For the if-deffery I proposed something during the last review,
> I'm not sure if you saw it. If you did, please ignore this comment:
>
I missed that, sorry (and I hope I didn't miss anything else).
>
> .address_list is also only needed with CONFIG_SENSORS_SPD5118_DETECT.
>
> If you use
>
> .detect = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? spd5118_detect : NULL,
> .address_list = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
>
> then the need for __maybe_unused goes away and type checking is a tiny
> bit better.
It does let me drop the #ifdef, so I agree it is a bit better.
I made that change, but I'll wait for a couple of days before sending
the updated version to give others time for additional feedback.
Thanks,
Guenter
© 2016 - 2026 Red Hat, Inc.