DS1374 Watchdog fixes

PATCH Applied Moritz Fischer 4 participants 14 messages Apr 24, 2017 – May 4, 2017

Subsystems (2)

Real time clock (RTC) Watchdog device drivers

lore

Share & tools

lore

Moritz Fischer

author

2017-04-24 22:05 UTC #0 lore raw

cover DS1374 Watchdog fixes

Hi all,

this series fixes two issues that I ran into when trying to use the watchdog part of the DS1374 on of our products.

This series is basically a precursor to some further cleanup work, that turns the DS1374 into a MFD device with an RTC and WDT in separate drivers [1] each integrated in either the watchdog and the rtc frameworks.

I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling the watchdog behavior and currently I'm investigating how to make that work via DT.

Watchdog maintainers, do you have an idea on how to do that in a non breaking fashion?

Thanks,
Moritz

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mdf/linux.git/log/?h=wip/mfd-ds1374-rfc

Moritz Fischer (2):
rtc: ds1374: wdt: Fix issue with timeout scaling from secs to wdt
ticks
rtc: ds1374: wdt: Fix stop/start ioctl always returning -EINVAL

 drivers/rtc/rtc-ds1374.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Signature · 1 line

2.7.4

Moritz Fischer

author

2017-04-24 22:05 UTC #1 lore raw

PATCH 1/2 rtc: ds1374: wdt: Fix issue with timeout scaling from secs to wdt ticks

Fix commit 920f91e50c5b ("drivers/rtc/rtc-ds1374.c: add watchdog support")

The issue is that the internal counter that triggers the watchdog reset is actually running at 4096 Hz instead of 1Hz, therefore the value given by userland (in sec) needs to be multiplied by 4096 to get the correct behavior.

Signed-off-by Moritz Fischer <[email protected]>

---
 drivers/rtc/rtc-ds1374.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/rtc/rtc-ds1374.c +6 -1

index 4cd115e..2a8b5b3 100644
@@ -525,6 +525,10 @@ static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
 		if (get_user(new_margin, (int __user *)arg))
 			return -EFAULT;
 
+		/* the hardware's tick rate is 4096 Hz, so
+		 * the counter value needs to be scaled accordingly
+		 */
+		new_margin <<= 12;
 		if (new_margin < 1 || new_margin > 16777216)
 			return -EINVAL;

@@ -533,7 +537,8 @@ static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
 		ds1374_wdt_ping();
 		/* fallthrough */
 	case WDIOC_GETTIMEOUT:
-		return put_user(wdt_margin, (int __user *)arg);
+		/* when returning ... inverse is true */
+		return put_user((wdt_margin >> 12), (int __user *)arg);
 	case WDIOC_SETOPTIONS:
 		if (copy_from_user(&options, (int __user *)arg, sizeof(int)))
 			return -EFAULT;

Signature · 11 lines

2.7.4

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Alexandre Belloni

Applied

2017-05-04 12:47 UTC #2 lore raw

Re: [PATCH 1/2] rtc: ds1374: wdt: Fix issue with timeout scaling from secs to wdt ticks

Quoted 7 lines · Moritz Fischer

Fix commit 920f91e50c5b ("drivers/rtc/rtc-ds1374.c: add watchdog support")
The issue is that the internal counter that triggers the watchdog reset is actually running at 4096 Hz instead of 1Hz, therefore the value given by userland (in sec) needs to be multiplied by 4096 to get the correct behavior.

Quoted 5 lines

Signed-off-by Moritz Fischer <[email protected]>
---
 drivers/rtc/rtc-ds1374.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Applied, thanks.

Signature · 13 lines

Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Moritz Fischer

author

2017-04-24 22:05 UTC #3 lore raw

PATCH 2/2 rtc: ds1374: wdt: Fix stop/start ioctl always returning -EINVAL

In reply to Moritz Fischer

Fix commit 920f91e50c5b ("drivers/rtc/rtc-ds1374.c: add watchdog support")

The WDIOC_SETOPTIONS case in the watchdog ioctl would alwayss falls through to the -EINVAL case. This is wrong since thew watchdog does actually get stopped or started correctly.

Signed-off-by Moritz Fischer <[email protected]>

---
 drivers/rtc/rtc-ds1374.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/rtc/rtc-ds1374.c +2 -1

index 2a8b5b3..38a2e9e 100644
@@ -546,14 +546,15 @@ static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
 		if (options & WDIOS_DISABLECARD) {
 			pr_info("disable watchdog\n");
 			ds1374_wdt_disable();
+			return 0;
 		}
 
 		if (options & WDIOS_ENABLECARD) {
 			pr_info("enable watchdog\n");
 			ds1374_wdt_settimeout(wdt_margin);
 			ds1374_wdt_ping();
+			return 0;
 		}
-
 		return -EINVAL;
 	}
 	return -ENOTTY;

Signature · 1 line

2.7.4

Alexandre Belloni

Applied

2017-05-04 12:47 UTC #4 lore raw

Re: [PATCH 2/2] rtc: ds1374: wdt: Fix stop/start ioctl always returning -EINVAL

Quoted 11 lines · Moritz Fischer

Fix commit 920f91e50c5b ("drivers/rtc/rtc-ds1374.c: add watchdog support")
The WDIOC_SETOPTIONS case in the watchdog ioctl would alwayss falls through to the -EINVAL case. This is wrong since thew watchdog does actually get stopped or started correctly.
Signed-off-by Moritz Fischer <[email protected]>
---
 drivers/rtc/rtc-ds1374.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.

Signature · 13 lines

Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Guenter Roeck

maintainer

2017-04-25 05:03 UTC #5 lore raw

In reply to Moritz Fischer

Quoted 10 lines · Moritz Fischer

Hi all,
this series fixes two issues that I ran into when trying to use the watchdog part of the DS1374 on of our products.
This series is basically a precursor to some further cleanup work, that turns the DS1374 into a MFD device with an RTC and WDT in separate drivers [1] each integrated in either the watchdog and the rtc frameworks.

Quoted 7 lines

I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling the watchdog behavior and currently I'm investigating how to make that work via DT.
Watchdog maintainers, do you have an idea on how to do that in a non breaking fashion?

Depends on what you mean with "non breaking". Just using the normal mfd mechanisms, ie define an mfd cell for each client driver, should work. Do you see any problems with that ? Either case, that doesn't seem to be a watchdog driver problem, or am I missing something ?

Quoted 10 lines

Thanks,
Moritz
[1] https://git.kernel.org/pub/scm/linux/kernel/git/mdf/linux.git/log/?h=wip/mfd-ds1374-rfc
Moritz Fischer (2):
rtc: ds1374: wdt: Fix issue with timeout scaling from secs to wdt
ticks
rtc: ds1374: wdt: Fix stop/start ioctl always returning -EINVAL

I don't really see the point of doing that if you plan to move the watchdog part of the driver into the watchdog directory. We for sure won't accept a watchdog driver that does not use the watchdog infrastructure.

Regarding
+ /* WHY? */
+ ds1374->wdd.timeout = t;

Assuming you mean why the driver has to set the timeout value - not every watchdog hardware supports timeouts in multiples of 1 second. The driver is expected to set the value to the real timeout, not to the timeout asked for by the infrastructure.

Guenter

Signature · 7 lines

You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Moritz Fischer

2017-04-25 14:55 UTC #6 lore raw

Hi Guenter,

Quoted 6 lines · Guenter Roeck

Quoted 4 lines · Moritz Fischer
I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling the watchdog behavior and currently I'm investigating how to make that work via DT.

Quoted 8 lines

Quoted 3 lines
Watchdog maintainers, do you have an idea on how to do that in a non breaking fashion?
Depends on what you mean with "non breaking". Just using the normal mfd mechanisms, ie define an mfd cell for each client driver, should work. Do you see any problems with that ? Either case, that doesn't seem to be a watchdog driver problem, or am I missing something ?

Well so currently watchdog behavior is selected (out of the two options alarm, or watchdog) by enabling the configuration option mentioned above. If I change this over to use a dt-based approach like dallas,ds1374-mode = <2>; to select the behavior in the mfd for example, won't that break people that relied on the old behavior? If everyone involved is ok with that, I'm happy to just add it to the binding.

Quoted 3 lines

I don't really see the point of doing that if you plan to move the watchdog part of the driver into the watchdog directory. We for sure won't accept a watchdog driver that does not use the watchdog infrastructure.

The idea was to fix what's broken currently (this patchset) and then refactor. But if you prefer I can do all in one go instead.

Quoted 9 lines

Regarding
(quoted diff) +2 -0
+       /* WHY? */
+       ds1374->wdd.timeout = t;
Assuming you mean why the driver has to set the timeout value - not every watchdog hardware supports timeouts in multiples of 1 second. The driver is expected to set the value to the real timeout, not to the timeout asked for by the infrastructure.

Yeah that branch is work in progress and needs cleanup. Leftover from testing, before I had understood why. Branch needs cleanup. It also doesn't really use regmap_update_bits etc.

Thanks for the explanation,

Moritz

Guenter Roeck

maintainer

2017-04-25 16:17 UTC #7 lore raw

Quoted 18 lines · Moritz Fischer

Hi Guenter,
Quoted 6 lines · Guenter Roeck
Quoted 4 lines · Moritz Fischer
I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling the watchdog behavior and currently I'm investigating how to make that work via DT.
Quoted 8 lines
Quoted 3 lines
Watchdog maintainers, do you have an idea on how to do that in a non breaking fashion?
Depends on what you mean with "non breaking". Just using the normal mfd mechanisms, ie define an mfd cell for each client driver, should work. Do you see any problems with that ? Either case, that doesn't seem to be a watchdog driver problem, or am I missing something ?

Quoted 7 lines

Well so currently watchdog behavior is selected (out of the two options alarm, or watchdog) by enabling the configuration option mentioned above. If I change this over to use a dt-based approach like dallas,ds1374-mode = <2>; to select the behavior in the mfd for example, won't that break people that relied on the old behavior? If everyone involved is ok with that, I'm happy to just add it to the binding.

Sorry, I must be missing something. Looking into the driver code, my understanding is that CONFIG_RTC_DRV_DS1374_WDT enables the watchdog in addition to rtc functionality, not one or the other. Sure you would need a different configuration option if you were to move the watchdog code into drivers/watchdog, but other than that I don't really understand the problem. What is the issue with, for example,

config DS1374_WDT
bool "Dallas/Maxim DS1374 watchdog timer" depends on MFD_DS1374
help
If you say Y here you will get support for the watchdog timer in the Dallas Semiconductor DS1374 real-time clock chips.

in drivers/watchdog/Kconfig, and the mfd driver instantiating it like any other mfd client driver ?

Either case, limiting support to DT based systems seems to be the wrong approach. There might be Intel platforms using this chip.

Quoted 7 lines

Quoted 3 lines
I don't really see the point of doing that if you plan to move the watchdog part of the driver into the watchdog directory. We for sure won't accept a watchdog driver that does not use the watchdog infrastructure.
The idea was to fix what's broken currently (this patchset) and then refactor. But if you prefer I can do all in one go instead.

It just seemed a waste to me to change/fix a function which is going to be removed in a subsequent patch (I seem to recall that there was a fix to the ioctl function).

If/when you move the driver to drivers/watchdog, please make sure that it doesn't use any instantiation related static variables (ie other than module parameters).

Quoted 5 lines

Quoted 5 lines
Regarding
(quoted diff) +2 -0
+       /* WHY? */
+       ds1374->wdd.timeout = t;

Quoted 9 lines

Quoted 4 lines
Assuming you mean why the driver has to set the timeout value - not every watchdog hardware supports timeouts in multiples of 1 second. The driver is expected to set the value to the real timeout, not to the timeout asked for by the infrastructure.
Yeah that branch is work in progress and needs cleanup. Leftover from testing, before I had understood why. Branch needs cleanup. It also doesn't really use regmap_update_bits etc.

Well, you did enhance the code to use regmap, which by itself is a significant improvement ...

Thanks,
Guenter

Signature · 8 lines

You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Alexandre Belloni

2017-04-25 16:32 UTC #8 lore raw

Quoted 27 lines · Guenter Roeck

Quoted 18 lines · Moritz Fischer
Hi Guenter,
Quoted 6 lines · Guenter Roeck
Quoted 4 lines · Moritz Fischer
I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling the watchdog behavior and currently I'm investigating how to make that work via DT.
Quoted 8 lines
Quoted 3 lines
Watchdog maintainers, do you have an idea on how to do that in a non breaking fashion?
Depends on what you mean with "non breaking". Just using the normal mfd mechanisms, ie define an mfd cell for each client driver, should work. Do you see any problems with that ? Either case, that doesn't seem to be a watchdog driver problem, or am I missing something ?
Quoted 7 lines
Well so currently watchdog behavior is selected (out of the two options alarm, or watchdog) by enabling the configuration option mentioned above. If I change this over to use a dt-based approach like dallas,ds1374-mode = <2>; to select the behavior in the mfd for example, won't that break people that relied on the old behavior? If everyone involved is ok with that, I'm happy to just add it to the binding.

Quoted 7 lines

Sorry, I must be missing something. Looking into the driver code, my understanding is that CONFIG_RTC_DRV_DS1374_WDT enables the watchdog in addition to rtc functionality, not one or the other. Sure you would need a different configuration option if you were to move the watchdog code into drivers/watchdog, but other than that I don't really understand the problem. What is the issue with, for example,

The watchdog functionality and the rtc alarm are mutually exclusive.

Quoted 8 lines

Quoted 3 lines
The idea was to fix what's broken currently (this patchset) and then refactor. But if you prefer I can do all in one go instead.
It just seemed a waste to me to change/fix a function which is going to be removed in a subsequent patch (I seem to recall that there was a fix to the ioctl function).

I'd say that it depends on whether you want to backport the fixes to the stable kernels. Backporting the full rework is probably riskier.

Signature · 3 lines

Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Guenter Roeck

maintainer

2017-04-25 16:58 UTC #9 lore raw

Quoted 36 lines · Alexandre Belloni

Quoted 27 lines · Guenter Roeck
Quoted 18 lines · Moritz Fischer
Hi Guenter,
Quoted 6 lines · Guenter Roeck
Quoted 4 lines · Moritz Fischer
I'm very unhappy with the CONFIG_DRV_RTC_DS1374_WDT way of enabling the watchdog behavior and currently I'm investigating how to make that work via DT.
Quoted 8 lines
Quoted 3 lines
Watchdog maintainers, do you have an idea on how to do that in a non breaking fashion?
Depends on what you mean with "non breaking". Just using the normal mfd mechanisms, ie define an mfd cell for each client driver, should work. Do you see any problems with that ? Either case, that doesn't seem to be a watchdog driver problem, or am I missing something ?
Quoted 7 lines
Well so currently watchdog behavior is selected (out of the two options alarm, or watchdog) by enabling the configuration option mentioned above. If I change this over to use a dt-based approach like dallas,ds1374-mode = <2>; to select the behavior in the mfd for example, won't that break people that relied on the old behavior? If everyone involved is ok with that, I'm happy to just add it to the binding.
Quoted 7 lines
Sorry, I must be missing something. Looking into the driver code, my understanding is that CONFIG_RTC_DRV_DS1374_WDT enables the watchdog in addition to rtc functionality, not one or the other. Sure you would need a different configuration option if you were to move the watchdog code into drivers/watchdog, but other than that I don't really understand the problem. What is the issue with, for example,

Quoted 2 lines

The watchdog functionality and the rtc alarm are mutually exclusive.

Ah, I missed the "n" in various #ifndef statements.

I can't really comment on how to solve that; I simply don't know. Also, even with a dt property, it still would be necessary to have a non-DT means to configure one or the other. Making whatever solution backward compatible also seems tricky; I don't have a solution for that problem either.

Quoted 4 lines

Quoted 4 lines
Quoted 3 lines
The idea was to fix what's broken currently (this patchset) and then refactor. But if you prefer I can do all in one go instead.

Quoted 8 lines

Quoted 4 lines
It just seemed a waste to me to change/fix a function which is going to be removed in a subsequent patch (I seem to recall that there was a fix to the ioctl function).
I'd say that it depends on whether you want to backport the fixes to the stable kernels. Backporting the full rework is probably riskier.

Good point.

Guenter

Signature · 8 lines

You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Moritz Fischer

author

2017-04-25 19:58 UTC #10 lore raw

Quoted 7 lines · Guenter Roeck

Ah, I missed the "n" in various #ifndef statements.
I can't really comment on how to solve that; I simply don't know. Also, even with a dt property, it still would be necessary to have a non-DT means to configure one or the other. Making whatever solution backward compatible also seems tricky; I don't have a solution for that problem either.

How does one do these things in a non-dt context? Platform data? I'd let the MFD select the 'mode'. Maybe being backwards compatible isn't possible in any case. Is there a rule somewhere that we guarantee you'll never have to change your CONFIG_FOO options?

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
The idea was to fix what's broken currently (this patchset) and then refactor. But if you prefer I can do all in one go instead.

Quoted 7 lines

Quoted 7 lines
Quoted 4 lines
It just seemed a waste to me to change/fix a function which is going to be removed in a subsequent patch (I seem to recall that there was a fix to the ioctl function).
I'd say that it depends on whether you want to backport the fixes to the stable kernels. Backporting the full rework is probably riskier.

I suck at communicating these days. But yeah. That was basically my concern when I split it up into 'Fixes' and 'Rework'.

Mostly since the rework might take a couple of rounds of review, while the fix can unbrick stuff (might still need review of course)

Cheers,

Moritz

Guenter Roeck

maintainer

2017-04-25 20:22 UTC #11 lore raw

Quoted 11 lines · Moritz Fischer

Quoted 7 lines · Guenter Roeck
Ah, I missed the "n" in various #ifndef statements.
I can't really comment on how to solve that; I simply don't know. Also, even with a dt property, it still would be necessary to have a non-DT means to configure one or the other. Making whatever solution backward compatible also seems tricky; I don't have a solution for that problem either.
How does one do these things in a non-dt context? Platform data? I'd let

Platform data is out of favor. You'd probably want to use device properties (see drivers/base/property.c). Question though is if this is considered configuration, hardware description, or both. Presumably the watchdog only makes sense if the reset signal is wired, and the alarm only makes sense if the interrupt is wired, but what if both are wired ?

Quoted 4 lines

the MFD select the 'mode'. Maybe being backwards compatible isn't possible in any case. Is there a rule somewhere that we guarantee you'll never have to change your CONFIG_FOO options?

That would be nice, but no, there is no such rule. You can probably argue that no published kernel configuration enables the watchdog flag, so there is nothing to be concerned about.

Guenter

Quoted 22 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
The idea was to fix what's broken currently (this patchset) and then refactor. But if you prefer I can do all in one go instead.
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
It just seemed a waste to me to change/fix a function which is going to be removed in a subsequent patch (I seem to recall that there was a fix to the ioctl function).
I'd say that it depends on whether you want to backport the fixes to the stable kernels. Backporting the full rework is probably riskier.
I suck at communicating these days. But yeah. That was basically my concern when I split it up into 'Fixes' and 'Rework'.
Mostly since the rework might take a couple of rounds of review, while the fix can unbrick stuff (might still need review of course)
Cheers,
Moritz

Signature · 8 lines

You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Moritz Fischer

author

2017-04-25 20:34 UTC #12 lore raw

Quoted 11 lines · Guenter Roeck

Quoted 10 lines · Moritz Fischer
Quoted 7 lines · Guenter Roeck
Ah, I missed the "n" in various #ifndef statements.
I can't really comment on how to solve that; I simply don't know. Also, even with a dt property, it still would be necessary to have a non-DT means to configure one or the other. Making whatever solution backward compatible also seems tricky; I don't have a solution for that problem either.

Quoted 7 lines

Quoted 1 line
How does one do these things in a non-dt context? Platform data? I'd let
Platform data is out of favor. You'd probably want to use device properties (see drivers/base/property.c). Question though is if this is considered configuration, hardware description, or both. Presumably the watchdog only makes sense if the reset signal is wired, and the alarm only makes sense if the interrupt is wired, but what if both are wired ?

To make things worse you can even remap the reset output to the INT pin (which my platform does).

I'll look at device properties. Thanks for the pointer.

Quoted 9 lines

Quoted 5 lines
the MFD select the 'mode'. Maybe being backwards compatible isn't possible in any case. Is there a rule somewhere that we guarantee you'll never have to change your CONFIG_FOO options?
That would be nice, but no, there is no such rule. You can probably argue that no published kernel configuration enables the watchdog flag, so there is nothing to be concerned about.

Alright, cool. Thanks

Moritz

PS: Haven't forgotten about the cros-ec-hwmon patch that I sent out ...

Guenter Roeck

maintainer

2017-04-25 21:05 UTC #13 lore raw

Quoted 14 lines · Moritz Fischer

Quoted 13 lines · Guenter Roeck
Quoted 11 lines · Moritz Fischer
Quoted 7 lines · Guenter Roeck
Ah, I missed the "n" in various #ifndef statements.
I can't really comment on how to solve that; I simply don't know. Also, even with a dt property, it still would be necessary to have a non-DT means to configure one or the other. Making whatever solution backward compatible also seems tricky; I don't have a solution for that problem either.
How does one do these things in a non-dt context? Platform data? I'd let

Quoted 9 lines

Quoted 5 lines
Platform data is out of favor. You'd probably want to use device properties (see drivers/base/property.c). Question though is if this is considered configuration, hardware description, or both. Presumably the watchdog only makes sense if the reset signal is wired, and the alarm only makes sense if the interrupt is wired, but what if both are wired ?
To make things worse you can even remap the reset output to the INT pin (which my platform does).

So that is what the weird 250ms "interrupt signal" is for. I had wondered what that is supposed to be used for.

Quoted 12 lines

I'll look at device properties. Thanks for the pointer.
Quoted 9 lines
Quoted 5 lines
the MFD select the 'mode'. Maybe being backwards compatible isn't possible in any case. Is there a rule somewhere that we guarantee you'll never have to change your CONFIG_FOO options?
That would be nice, but no, there is no such rule. You can probably argue that no published kernel configuration enables the watchdog flag, so there is nothing to be concerned about.

Quoted 5 lines

Alright, cool. Thanks
Moritz
PS: Haven't forgotten about the cros-ec-hwmon patch that I sent out ...

No, still trying to get internal feedback. I'll have to ask again.

Guenter

Signature · 8 lines

You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.