Re: [PATCH v2 2/2] platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface)

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

 



Hi,

On 2022-09-02 10:29, Hans de Goede wrote:
> Hi Guenter, Arvid,
<snip>
> 
> Actually that should be IS_ENABLED since you suggested that
> Arvid should use:
> 
> 	depends on HWMON || HWMON = n
> 
> In the Kconfig bit there is no need for IS_REACHABLE,
> note IS_REACHABLE will work too but I generally prefer
> to avoid it because cases which actually need it lead
> to weirdness where e.g. both HWMON and TOSHIBA_ACPI are
> enabled yet TOSHIBA_ACPI will still not have HWMON
> support.
> 
> Arvid, sorry about the "noise" here, let me try to
> explain.
> 
> First of all lets explain this bit of magic:
> 
> 	depends on HWMON || HWMON = n
> 
> What this says is that if HWMON is enabled it must
> be able to satisfy dependencies on it in toshiba_acpi.c
> (or it may also be fully disabled).
> 
> This magic is necessary to avoid a case where
> toshiba_acpi gets build into the kernel, but the
> hwmon code is a module. In that case linking errors
> (unresolved hwmon symbols) will happen when building
> the main vmlinuz kernel image.
> 
> So basically what this does is if HWMON is configured
> as a module, it limits the choices for TOSHIBA_ACPI
> to either n or m and disallows y.
> 
> I hope that so far I'm making sense...

Thanks, this clears up quite a bit of confusion.

> 
> So now to the #ifdef-ery. Since HWMON can be a module
> when enabled the #define's from Kconfig will either
> contain:
> 
> #define CONFIG_HWMON 1   // when builtin, HWMON=y
> 
> or:
> 
> #define CONFIG_HWMON_MODULE 1   // when a module, HWMON=m
> 
> So you would need to write:
> 
> #if defined CONFIG_HWMON || defined CONFIG_HWMON_MODULE
> 
> as a condition
> 
> #if IS_ENABLED(CONFIG_HWMON)
> 
> is a shorthand (macro) for this.
> 
> ###
> 
> Now back to:
> 
> #if IS_REACHABLE(CONFIG_HWMON)
> 
> This is a special macro for when your Kconfig bit would just be:
> 
> 	depends on HWMON
> 
> in that case TOSHIBA_ACPI might be set to builtin (y)
> while the HWMON core/class code is a module. As mentioned
> above that would lead to undefined hwmon symbols when
> using "#if IS_ENABLED(CONFIG_HWMON)" as test. IS_REACHABLE
> is special in that it will disable (evaluate to false)
> in the case where the code being build is builtin and
> the dependency is a module.
> 
> But that cannot happen here because your Kconfig bit is:
> 
> 	depends on HWMON || HWMON = n
> 
> So "#if IS_ENABLED(CONFIG_HWMON)" is sufficient.
> 
> TL;DR: please use "#if IS_ENABLED(CONFIG_HWMON)" to test
> if the hwmon code should be build.
> 
> Regards,
> 
> Hans

All of this will be useful to know in the future as well I imagine, so
thanks again.

<snip>

Best regards,
Arvid Norlander



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux