On 7/19/23 00:18, Henning Schild wrote:
Am Tue, 18 Jul 2023 08:10:09 -0700
schrieb Guenter Roeck <linux@xxxxxxxxxxxx>:
On 7/18/23 07:42, Henning Schild wrote:
Am Tue, 18 Jul 2023 17:20:48 +0300
schrieb Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>:
On Tue, Jul 18, 2023 at 12:52:11PM +0200, Henning Schild wrote:
If a user did choose to enable Siemens Simatic platform support
they likely want that driver to be enabled without having to flip
more config switches. So we make the watchdog driver config switch
default to the platform driver switches value.
A nit-pick below.
...
config SIEMENS_SIMATIC_IPC_WDT
tristate "Siemens Simatic IPC Watchdog"
depends on SIEMENS_SIMATIC_IPC
+ default SIEMENS_SIMATIC_IPC
It's more natural to group tristate and default, vs. depends and
select.
Will be ignored unless maintainer insists.
Maintainer wants to know why "default SIEMENS_SIMATIC_IPC" is needed
or warranted instead of the much simpler and easier to understand
"default y".
I thought a "default y" or "default m" was maybe not the best idea for
a platform that is not super common. That is why i did not dare to even
think about defaulting any of the Simatic stuff to not-no.
But it seems that this would be ok after all. And i would be very happy
to do so because it means less work on distro configs.
SIEMENS_SIMATIC_IPC_WDT will drive a platform device which gets
registered by SIEMENS_SIMATIC_IPC and nothing else. That is why
"default SIEMENS_SIMATIC_IPC" was chosen.
It depends on SIEMENS_SIMATIC_IPC. "default y" would make it y if
SIEMENS_SIMATIC_IPC=y, and m if SIEMENS_SIMATIC_IPC=m. If
SIEMENS_SIMATIC_IPC=n, it won't even be offered as option, and
default={m,y} will be ignored.
But if i may i would change that to "default m", not "y" because there
is an out of tree driver package which if installed on top, should be
able to override the in-tree drivers.
So i will go ahead and make that one "default m"
Why make it m as default even if SIEMENS_SIMATIC_IPC=y for whatever
reason ? Presumably anyone selecting SIEMENS_SIMATIC_IPC=y would
also want SIEMENS_SIMATIC_IPC_WDT=y, which is what you had before.
Sorry, I don't understand your logic.
Guenter
