Re: [PATCH 1/4] hwmon, fam15h_power: Change email address, MAINTAINERS entry
On Mon, Oct 29, 2012 at 06:50:47PM +0100, Andreas Herrmann wrote: Signed-off-by: Andreas Herrmann herrmann.der.u...@googlemail.com Applied. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hwmon: da9055: Fix chan_mux[DA9055_ADC_ADCIN3] setting
On Mon, Oct 29, 2012 at 04:34:38PM +0800, Axel Lin wrote:
Set chan_mux[DA9055_ADC_ADCIN3] = DA9055_ADC_MUX_ADCIN3.
Signed-off-by: Axel Lin axel@ingics.com
---
Hi,
This looks like a typo, but I don't have a hardware to test it.
Axel
drivers/hwmon/da9055-hwmon.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/da9055-hwmon.c b/drivers/hwmon/da9055-hwmon.c
index 0b41f2c..3a3377c 100644
--- a/drivers/hwmon/da9055-hwmon.c
+++ b/drivers/hwmon/da9055-hwmon.c
@@ -54,7 +54,7 @@ static const u8 chan_mux[DA9055_ADC_TJUNC + 1] = {
[DA9055_ADC_VSYS] = DA9055_ADC_MUX_VSYS,
[DA9055_ADC_ADCIN1] = DA9055_ADC_MUX_ADCIN1,
[DA9055_ADC_ADCIN2] = DA9055_ADC_MUX_ADCIN2,
- [DA9055_ADC_ADCIN3] = DA9055_ADC_MUX_ADCIN1,
+ [DA9055_ADC_ADCIN3] = DA9055_ADC_MUX_ADCIN3,
[DA9055_ADC_TJUNC] = DA9055_ADC_MUX_T_SENSE,
};
Very much loks like a typo.
David, Ashish, any comments ? I would like to get your Ack before applying it.
Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hwmon: da9055: Fix chan_mux[DA9055_ADC_ADCIN3] setting
On Mon, Oct 29, 2012 at 04:34:38PM +0800, Axel Lin wrote:
Set chan_mux[DA9055_ADC_ADCIN3] = DA9055_ADC_MUX_ADCIN3.
Signed-off-by: Axel Lin axel@ingics.com
---
Hi,
This looks like a typo, but I don't have a hardware to test it.
Axel
drivers/hwmon/da9055-hwmon.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/da9055-hwmon.c b/drivers/hwmon/da9055-hwmon.c
index 0b41f2c..3a3377c 100644
--- a/drivers/hwmon/da9055-hwmon.c
+++ b/drivers/hwmon/da9055-hwmon.c
@@ -54,7 +54,7 @@ static const u8 chan_mux[DA9055_ADC_TJUNC + 1] = {
[DA9055_ADC_VSYS] = DA9055_ADC_MUX_VSYS,
[DA9055_ADC_ADCIN1] = DA9055_ADC_MUX_ADCIN1,
[DA9055_ADC_ADCIN2] = DA9055_ADC_MUX_ADCIN2,
- [DA9055_ADC_ADCIN3] = DA9055_ADC_MUX_ADCIN1,
+ [DA9055_ADC_ADCIN3] = DA9055_ADC_MUX_ADCIN3,
[DA9055_ADC_TJUNC] = DA9055_ADC_MUX_T_SENSE,
};
Applied.
Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] hwmon patches for 3.7-rc4
Hi Linus, Please pull hwmon patches for Linux 3.7-rc4 from signed tag: git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-for-linus Thanks, Guenter -- The following changes since commit 8f0d8163b50e01f398b14bcd4dc039ac5ab18d64: Linux 3.7-rc3 (2012-10-28 12:24:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git tags/hwmon-for-linus for you to fetch changes up to eaa7cc60f7dff5e74ef387ace8228235fab8241b: hwmon: Only include of_match_table with CONFIG_OF_GPIO (2012-11-01 17:31:20 -0700) An e-mail address update, and fix a compile error on SPARC Andreas Herrmann (1): hwmon, fam15h_power: Change email address, MAINTAINERS entry Jamie Lentin (1): hwmon: Only include of_match_table with CONFIG_OF_GPIO Documentation/hwmon/fam15h_power |2 +- MAINTAINERS |2 +- drivers/hwmon/fam15h_power.c |4 ++-- drivers/hwmon/gpio-fan.c |2 ++ 4 files changed, 6 insertions(+), 4 deletions(-) signature.asc Description: Digital signature
Re: [PATCH resend] spi core: Provide means to instantiate devices through sysfs
On Sat, Sep 22, 2012 at 12:11:33PM -0400, Mark Brown wrote: On Mon, Sep 17, 2012 at 04:51:20PM -0700, Guenter Roeck wrote: The I2C core provides a means to instantiate devices from userspace using sysfs attributes. Provide the same mechanism for SPI devices. So, unlike I2C this is only going to work for a subset of controllers - anything that relies on GPIOs for chip select will need more data to add the chip selects for example. This means I'm having a hard time getting enthused about the idea, it seems like it might be a support burden. Yes, obviously that model can not be used if SPI chip select is handled outside the SPI master driver. Anyway, no problem. That the code is useful for me doesn't mean it has to be useful for others. I'll make it available as branch on my linux tree on github. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] hwmon: (ads7828) add support for ADS7830
On Wed, Oct 03, 2012 at 10:03:08AM -0400, Vivien Didelot wrote:
Hi Guenter,
On Tue, 2012-10-02 at 22:07 -0700, Guenter Roeck wrote:
On Tue, Oct 02, 2012 at 11:33:27PM -0400, Vivien Didelot wrote:
From: Guillaume Roguez guillaume.rog...@savoirfairelinux.com
The ADS7830 device is almost the same as the ADS7828,
except that it does 8-bit sampling, instead of 12-bit.
This patch extends the ads7828 driver to support this chip.
Signed-off-by: Guillaume Roguez guillaume.rog...@savoirfairelinux.com
Signed-off-by: Vivien Didelot vivien.dide...@savoirfairelinux.com
Guillaume,
Vivien,
[ ... ]
@@ -147,6 +152,7 @@ static int ads7828_detect(struct i2c_client *client,
{
struct i2c_adapter *adapter = client-adapter;
u8 default_cmd_byte = ADS7828_CMD_SD_SE | ADS7828_CMD_PD3;
+ bool is_8bit = false;
int ch;
/* Check we have a valid client */
@@ -158,7 +164,9 @@ static int ads7828_detect(struct i2c_client *client,
* dedicated register so attempt to sanity check using knowledge of
* the chip
* - Read from the 8 channel addresses
- * - Check the top 4 bits of each result are not set (12 data bits)
+ * - Check the top 4 bits of each result:
+ * - They should not be set in case of 12-bit samples
+ * - The two bytes should be equal in case of 8-bit samples
*/
for (ch = 0; ch ADS7828_NCH; ch++) {
u8 cmd = ads7828_cmd_byte(default_cmd_byte, ch);
@@ -168,13 +176,20 @@ static int ads7828_detect(struct i2c_client *client,
return -ENODEV;
if (in_data 0xF000) {
- pr_debug(%s : Doesn't look like an ads7828 device\n,
- __func__);
- return -ENODEV;
+ if ((in_data 8) == (in_data 0xFF)) {
+ /* Seems to be an ADS7830 (8-bit sample) */
+ is_8bit = true;
+ } else {
+ dev_dbg(client-dev, doesn't look like an
ADS7828 compatible device\n);
+ return -ENODEV;
+ }
}
}
I have been thinking about this. The detection function is already quite
weak,
and this makes it even weaker. Reason is that you conly check for ADS7830
if the
check for ADS7828 failed, and you repeat the pattern for each channel.
Unfortunately, that means that you don't check for the ADS7830 condition if
the
value returned for a channel happens to be a valid ADS7828 value, even if
it is
not valid for ADS7830 (and even if you already know that the chip is not a
ADS7828).
Example:
ch=0: 0x1818-- You know it is not ADS7828
ch=1: 0x0818-- You know it is not ADS7830, but you don't check for
it
I don't know an optimal solution right now, but maybe something like
maybe_7828 = true;
maybe_7830 = true;
for (ch = 0; ch ADS7828_NCH (maybe_7828 || maybe_7830); ch++) {
...
if (in_data 0xF000)
maybe_7828 = false;
if ((in_data 8) != (in_data 0xFF))
maybe_7830 = false;
}
if (!maybe_7828 !maybe_7830)
return -ENODEV;
if (maybe_7828)
strlcpy(info-type, ads7828, I2C_NAME_SIZE);
else
strlcpy(info-type, ads7830, I2C_NAME_SIZE);
Frankly I would prefer to get rid of the _detect function entirely, I just
don't
know if that would negatively affect some users. To give you an example for
a
bad result: The function will wrongly detect an ADS7830 as ADS7828 if all
ADC
channels report a value between 0x00 and 0x0f.
We totally agree with you here. There is no clean way to detect (i.e. to
be sure) that this *is* an ADS7828 compatible device.
How do you use the chip ? Do you need the detect function in your
application ?
In our application, this device is statically declared in the platform
support code, so we don't need to detect it.
I propose to re-send a v5 with the s/u16 in_data/int in_data/ fix and
the ads7828_detect() removal in the first cleanup patch, then the
ADS7830 support. Does it sound good for you?
Yes.
Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: make CONFIG_EXPERIMENTAL invisible and default
On Tue, Oct 02, 2012 at 07:50:42PM -, Kees Cook wrote: This config item has not carried much meaning for a while now and is almost always enabled by default. As agreed during the Linux kernel summit, it should be removed. As a first step, remove it from being listed, and default it to on. Once it has been removed from all subsystem Kconfigs, it will be dropped entirely. CC: Greg KH gre...@linuxfoundation.org CC: Eric W. Biederman ebied...@xmission.com CC: Serge Hallyn serge.hal...@canonical.com CC: Paul E. McKenney paul...@linux.vnet.ibm.com CC: Andrew Morton a...@linux-foundation.org CC: Frederic Weisbecker fweis...@gmail.com Signed-off-by: Kees Cook keesc...@chromium.org Acked-by: Serge Hallyn serge.hal...@canonical.com --- This is the first of a series of 202 patches removing EXPERIMENTAL from all the Kconfigs in the tree. Should I send them all to lkml (with all the associated CCs), or do people want to cherry-pick changes from my tree? I don't want to needlessly flood the list. http://git.kernel.org/?p=linux/kernel/git/kees/linux.git;a=shortlog;h=refs/heads/experimental I cherry-picked the hwmon/pmbus patch (commit 41c5b6bb). Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: make CONFIG_EXPERIMENTAL invisible and default
On Wed, Oct 03, 2012 at 04:33:21PM -0700, Kees Cook wrote: On Wed, Oct 3, 2012 at 4:29 PM, Guenter Roeck li...@roeck-us.net wrote: On Tue, Oct 02, 2012 at 07:50:42PM -, Kees Cook wrote: This config item has not carried much meaning for a while now and is almost always enabled by default. As agreed during the Linux kernel summit, it should be removed. As a first step, remove it from being listed, and default it to on. Once it has been removed from all subsystem Kconfigs, it will be dropped entirely. CC: Greg KH gre...@linuxfoundation.org CC: Eric W. Biederman ebied...@xmission.com CC: Serge Hallyn serge.hal...@canonical.com CC: Paul E. McKenney paul...@linux.vnet.ibm.com CC: Andrew Morton a...@linux-foundation.org CC: Frederic Weisbecker fweis...@gmail.com Signed-off-by: Kees Cook keesc...@chromium.org Acked-by: Serge Hallyn serge.hal...@canonical.com --- This is the first of a series of 202 patches removing EXPERIMENTAL from all the Kconfigs in the tree. Should I send them all to lkml (with all the associated CCs), or do people want to cherry-pick changes from my tree? I don't want to needlessly flood the list. http://git.kernel.org/?p=linux/kernel/git/kees/linux.git;a=shortlog;h=refs/heads/experimental I cherry-picked the hwmon/pmbus patch (commit 41c5b6bb). Great, thanks! I also picked the Documentation/hwmon patch (fbe3d657620). Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] hwmon: (ads7828) driver cleanup
On Wed, Oct 03, 2012 at 04:54:07PM -0400, Vivien Didelot wrote: As there is no reliable way to identify the chip, it is preferable to remove the detect callback, to avoid misdetection. Module parameters are not worth it here, so let's get rid of them and add an ads7828_platform_data structure instead. Clean the code by removing unused macros, fixing coding style issues, avoiding function prototypes and using convenient macros such as module_i2c_driver(). Signed-off-by: Vivien Didelot vivien.dide...@savoirfairelinux.com Applied to -next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/2] hwmon: (ads7828) add support for ADS7830
On Wed, Oct 03, 2012 at 04:54:08PM -0400, Vivien Didelot wrote: From: Guillaume Roguez guillaume.rog...@savoirfairelinux.com The ADS7830 device is almost the same as the ADS7828, except that it does 8-bit sampling, instead of 12-bit. This patch extends the ads7828 driver to support this chip. Signed-off-by: Guillaume Roguez guillaume.rog...@savoirfairelinux.com Signed-off-by: Vivien Didelot vivien.dide...@savoirfairelinux.com Applied to -next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drm: nouveau: Fix build warning seen is HWMON is undefined
Fix:
nouveau_pm.c: In function ‘nouveau_hwmon_init’:
nouveau_pm.c:703:24: warning: unused variable ‘therm’ [-Wunused-variable]
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
This was seen in the latest upstream kernel.
drivers/gpu/drm/nouveau/nouveau_pm.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_pm.c
b/drivers/gpu/drm/nouveau/nouveau_pm.c
index 0bf64c9..90dc63e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_pm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_pm.c
@@ -699,10 +699,10 @@ static int
nouveau_hwmon_init(struct drm_device *dev)
{
struct nouveau_pm *pm = nouveau_pm(dev);
- struct nouveau_drm *drm = nouveau_drm(dev);
- struct nouveau_therm *therm = nouveau_therm(drm-device);
#if defined(CONFIG_HWMON) || (defined(MODULE) defined(CONFIG_HWMON_MODULE))
+ struct nouveau_drm *drm = nouveau_drm(dev);
+ struct nouveau_therm *therm = nouveau_therm(drm-device);
struct device *hwmon_dev;
int ret = 0;
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] hwmon fixes for 3.7-rc2
Hi Linus, Please pull hwmon fixes for Linux 3.7-rc2 from signed tag: git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-for-linus Thanks, Guenter -- The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37: Linux 3.7-rc1 (2012-10-14 14:41:04 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git tags/hwmon-for-linus for you to fetch changes up to 1102dcab849313bd5a340b299b5cf61b518fbc0f: hwmon: (coretemp) Add support for Atom CE4110/4150/4170 (2012-10-14 15:21:33 -0700) Drop some leftover dependencies on CONFIG_EXPERIMENTAL, and add support for Intel Atom CE4110/4150/4170. Guenter Roeck (1): hwmon: (coretemp) Add support for Atom CE4110/4150/4170 Kees Cook (2): hwmon: (pmbus) remove CONFIG_EXPERIMENTAL Documentation/hwmon: remove CONFIG_EXPERIMENTAL Documentation/hwmon/coretemp |1 + Documentation/hwmon/submitting-patches |3 +-- drivers/hwmon/coretemp.c |7 +-- drivers/hwmon/pmbus/Kconfig|2 +- 4 files changed, 8 insertions(+), 5 deletions(-) signature.asc Description: Digital signature
Re: [RFC] Energy/power monitoring within the kernel
On Tue, Oct 23, 2012 at 06:30:49PM +0100, Pawel Moll wrote: Greetings All, More and more of people are getting interested in the subject of power (energy) consumption monitoring. We have some external tools like battery simulators, energy probes etc., but some targets can measure their power usage on their own. Traditionally such data should be exposed to the user via hwmon sysfs interface, and that's exactly what I did for my platform - I have a /sys/class/hwmon/hwmon*/device/energy*_input and this was good enough to draw pretty graphs in userspace. Everyone was happy... Only driver supporting energy output so far is ibmaem, and the reported energy is supposed to be cumulative, as in energy = power * time. Do you mean power, possibly ? Now I am getting new requests to do more with this data. In particular I'm asked how to add such information to ftrace/perf output. The second most frequent request is about providing it to a energy aware cpufreq governor. Anything energy related would have to be along the line of do something after a certain amount of work has been performed, which at least at the surface does not make much sense to me, unless you mean something along the line of a process scheduler which schedules a process not based on time slices but based on energy consumed, ie if you want to define a time slice not in milli-seconds but in Joule. If so, I would argue that a similar behavior could be achieved by varying the duration of time slices with the current CPU speed, or simply by using cycle count instead of time as time slice parameter. Not that I am sure if such an approach would really be of interest for anyone. Or do you really mean power, not energy, such as in reduce CPU speed if its power consumption is above X Watt ? I've came up with three (non-mutually exclusive) options. I will appreciate any other ideas and comments (including it makes not sense whatsoever ones, with justification). Of course I am more than willing to spend time on prototyping anything that seems reasonable and propose patches. === Option 1: Trace event === This seems to be the cheapest option. Simply defining a trace event that can be generated by a hwmon (or any other) driver makes the interesting data immediately available to any ftrace/perf user. Of course it doesn't really help with the cpufreq case, but seems to be a good place to start with. The question is how to define it... I've came up with two prototypes: = Generic hwmon trace event = This one allows any driver to generate a trace event whenever any hwmon attribute (measured value) gets updated. The rate at which the updates happen can be controlled by already existing update_interval attribute. 8--- TRACE_EVENT(hwmon_attr_update, TP_PROTO(struct device *dev, struct attribute *attr, long long input), TP_ARGS(dev, attr, input), TP_STRUCT__entry( __string( dev,dev_name(dev)) __string( attr, attr-name) __field(long long, input) ), TP_fast_assign( __assign_str(dev, dev_name(dev)); __assign_str(attr, attr-name); __entry-input = input; ), TP_printk(%s %s %lld, __get_str(dev), __get_str(attr), __entry-input) ); 8--- It generates such ftrace message: ...212.673126: hwmon_attr_update: hwmon4 temp1_input 34361 One issue with this is that some external knowledge is required to relate a number to a processor core. Or maybe it's not an issue at all because it should be left for the user(space)? = CPU power/energy/temperature trace event = This one is designed to emphasize the relation between the measured value (whether it is energy, temperature or any other physical phenomena, really) and CPUs, so it is quite specific (too specific?) 8--- TRACE_EVENT(cpus_environment, TP_PROTO(const struct cpumask *cpus, long long value, char unit), TP_ARGS(cpus, value, unit), TP_STRUCT__entry( __array(unsigned char, cpus, sizeof(struct cpumask)) __field(long long, value) __field(char, unit) ), TP_fast_assign( memcpy(__entry-cpus, cpus, sizeof(struct cpumask)); __entry-value = value; __entry-unit = unit; ), TP_printk(cpus %s %lld[%c], __print_cpumask((struct cpumask *)__entry-cpus), __entry-value, __entry-unit) ); 8--- And the equivalent ftrace message is: ...127.063107: cpus_environment: cpus 0,1,2,3 34361[C] It's a cpumask, not just single cpu id, because the sensor may measure the value per set of CPUs, eg. a temperature of
Re: [PATCH] hwmon: ina2xx: use module_i2c_driver to simplify the code
On Mon, Oct 08, 2012 at 08:40:35PM +0800, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn Use the module_i2c_driver() macro to make the code smaller and a bit simpler. dpatch engine is used to auto generate this patch. (https://github.com/weiyj/dpatch) Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn Applied to -next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hwmon: da9052: Use da9052_reg_update for rmw operations
On Thu, Oct 11, 2012 at 10:03:51PM +0800, Axel Lin wrote: Signed-off-by: Axel Lin axel@ingics.com --- drivers/hwmon/da9052-hwmon.c | 27 +++ 1 file changed, 7 insertions(+), 20 deletions(-) Applied to -next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hwmon: (ina2xx) Use module_i2c_driver macro
On Thu, Oct 11, 2012 at 11:56:26PM +0800, Axel Lin wrote: Signed-off-by: Axel Lin axel@ingics.com --- drivers/hwmon/ina2xx.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) Hi Axel, A similar patch from Wei Yongjun is already queued in my -next branch. It is not in linux-next yet since I am supposed to hold off until the commit window closes. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Energy/power monitoring within the kernel
On Wed, Oct 24, 2012 at 05:37:27PM +0100, Pawel Moll wrote: On Tue, 2012-10-23 at 23:02 +0100, Guenter Roeck wrote: Traditionally such data should be exposed to the user via hwmon sysfs interface, and that's exactly what I did for my platform - I have a /sys/class/hwmon/hwmon*/device/energy*_input and this was good enough to draw pretty graphs in userspace. Everyone was happy... Only driver supporting energy output so far is ibmaem, and the reported energy is supposed to be cumulative, as in energy = power * time. Do you mean power, possibly ? So the vexpress would be the second one, than :-) as the energy monitor actually on the latest tiles reports 64-bit value of microJoules consumed (or produced) since the power-up. Some of the older boards were able to report instant power, but this metrics is less useful in our case. Now I am getting new requests to do more with this data. In particular I'm asked how to add such information to ftrace/perf output. The second most frequent request is about providing it to a energy aware cpufreq governor. Anything energy related would have to be along the line of do something after a certain amount of work has been performed, which at least at the surface does not make much sense to me, unless you mean something along the line of a process scheduler which schedules a process not based on time slices but based on energy consumed, ie if you want to define a time slice not in milli-seconds but in Joule. Actually there is some research being done in this direction, but it's way too early to draw any conclusions... If so, I would argue that a similar behavior could be achieved by varying the duration of time slices with the current CPU speed, or simply by using cycle count instead of time as time slice parameter. Not that I am sure if such an approach would really be of interest for anyone. Or do you really mean power, not energy, such as in reduce CPU speed if its power consumption is above X Watt ? Uh. To be completely honest I must answer: I'm not sure how the energy aware cpufreq governor is supposed to work. I have been simply asked to provide the data in some standard way, if possible. I am not sure how this would be expected to work. hwmon is, by its very nature, a passive subsystem: It doesn't do anything unless data is explicitly requested from it. It does not update an attribute unless that attribute is read. That does not seem to fit well with the idea of tracing - which assumes that some activity is happening, ultimately, all by itself, presumably periodically. The idea to have a user space application read hwmon data only for it to trigger trace events does not seem to be very compelling to me. What I had in mind was similar to what adt7470 driver does. The driver would automatically access the device every now and then to update it's internal state and generate the trace event on the way. This auto-refresh feature is particularly appealing for me, as on some of my platforms can take up to 500 microseconds to actually get the data. So doing this in background (and providing users with the last known value in the meantime) seems attractive. A bad example doesn't mean it should be used elsewhere. adt7470 needs up to two seconds for a temperature measurement cycle, and it can not perform automatic cycles all by itself. In this context, executing temperature measurement cycles in the background makes a lot of sense, especially since one does not want to wait for two seconds when reading a sysfs attribute. But that only means that the chip is most likely not a good choice when selecting a temperature sensor, not that the code necessary to get it working should be used as an example for other drivers. Guenter An exception is if a monitoring device suppports interrupts, and if its driver actually implements those interrupts. This is, however, not the case for most of the current drivers (if any), mostly because interrupt support for hardware monitoring devices is very platform dependent and thus difficult to implement. Interestingly enough the newest version of our platform control micro (doing the energy monitoring as well) can generate and interrupt when a transaction is finished, so I was planning to periodically update the all sort of values. And again, generating a trace event on this opportunity would be trivial. Of course a particular driver could register its own perf PMU on its own. It's certainly an option, just very suboptimal in my opinion. Or maybe not? Maybe the task is so specialized that it makes sense? We had a couple of attempts to provide an in-kernel API. Unfortunately, the result was, at least so far, more complexity on the driver side. So the difficulty is really to define an API which is really simple, and does not just complicate driver development
Re: [lm-sensors] [PATCH 1/2] hwmon: (applesmc) Allow negative temperature values
On Mon, Jul 16, 2012 at 09:18:10AM +0200, Henrik Rydberg wrote: There are many userland reports of sensors with unreasonably small and large temperatures. There seem to be several reasons for this: Firstly, the major sensor type (sp78) is actually a signed number. This explains why some sensors show very small or large values - they are in fact all small, but of different sign. Secondly, the other sensor type (1-hex) is not properly understood; it may be that it is not a temperature after all. Thirdly, some sensors are differential in nature, showing changes over time rather than absolute numbers. This explains why those values are small and of varying sign. This patch interprets the sp78 type as signed short, but keeps the original scaling. For other types, -EINVAL is returned, since the nature of those sensors is unknown. Signed-off-by: Henrik Rydberg rydb...@euromail.se --- Applied to -next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH 2/2] hwmon: (applesmc) Ignore some temperature registers
On Mon, Jul 16, 2012 at 09:18:11AM +0200, Henrik Rydberg wrote: Not all sensors in the T range are useful temperatures. This patch creates a subset of sensors to be exported to userland, excluding the unknown types. Signed-off-by: Henrik Rydberg rydb...@euromail.se Applied to -next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] watchdog: introduce new watchdog AUTOSTART option
On Mon, Apr 15, 2013 at 12:43:31AM +, Kim, Milo wrote: Hi Guenter I really don't like that idea. It defeats a significant part of the purpose for having a watchdog, which is to prevent user-space hangups. To make this a driver option is even more odd - it forces every user of this driver to use it in-kernel only, and makes /dev/watchdog quite useless. I mean, really, if you have such a watchdog, what is the point of using the watchdog infrastructure in the first place ? Just make it a kernel thread or timer-activated platform code which pings your watchdog once in a while. No need to get the watchdog infrastructure involved in the first place. Am I missing something ? I wanted to enable the watchdog timer without the watchdog application for making sure the system alive. However, I think I misunderstood the purpose of the watchdog driver. The watchdog is for detecting user-space hangups rather than kernel stall. Is it correct? If yes, this patch is totally wrong. Correct. After all, if the kernel stalls, user space will stall as well, so by covering user space it covers both. Covering kernel alone doesn't help much, since most of the stalls (at least in my experience) happen in user space. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
On Mon, Apr 15, 2013 at 03:00:58PM +, Opensource [Anthony Olech] wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 12 April 2013 14:32 To: Opensource [Anthony Olech] Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML; David Dajun Chen Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote: This is the REGULATOR component driver of the Dialog DA9058 PMIC. This driver is just one component of the whole DA9058 PMIC driver. It depends on the CORE component driver of the DA9058 MFD. There are 6 warnings from scripts/checkpatch.pl, but since it seems to be complaining about variable names such as min_uV are in CamelCase, when it is obvious that they are not in CamelCase I have ignored them. ??? min_uV _is_ CamelCase ??? Ok, maybe it is camelcasE, but you are splitting hairs here. it is not me splitting hairs, it is scripts/checkpatch.pl Maybe you did not understand what I meant. Per your logic, MicroVolt is CamelCase uVolt is ??? uV is not CamelCase By abbreviating CamelCase to camelCase to cC you make it, in your opinion, acceptable. If you want to declare CamelCase variables, just do it, but don't claim that they are not really CamelCase. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
On Mon, Apr 15, 2013 at 05:29:13PM +, Opensource [Anthony Olech] wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 15 April 2013 17:36 To: Opensource [Anthony Olech] Cc: LKML Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver On Mon, Apr 15, 2013 at 03:00:58PM +, Opensource [Anthony Olech] wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 12 April 2013 14:32 To: Opensource [Anthony Olech] Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML; David Dajun Chen Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote: This is the REGULATOR component driver of the Dialog DA9058 PMIC. This driver is just one component of the whole DA9058 PMIC driver. It depends on the CORE component driver of the DA9058 MFD. There are 6 warnings from scripts/checkpatch.pl, but since it seems to be complaining about variable names such as min_uV are in CamelCase, when it is obvious that they are not in CamelCase I have ignored them. ??? min_uV _is_ CamelCase ??? Ok, maybe it is camelcasE, but you are splitting hairs here. it is not me splitting hairs, it is scripts/checkpatch.pl Maybe you did not understand what I meant. Per your logic, MicroVolt is CamelCase uVolt is ??? uV is not CamelCase By abbreviating CamelCase to camelCase to cC you make it, in your opinion, acceptable. If you want to declare CamelCase variables, just do it, but don't claim that they are not really CamelCase. Guenter I always thought that camel case meant changing from lower case to upper case the first letter of each word and then joining the capitalized words together, so by that definition uV or mW are not camel case because v and w are not words! Either way it seems that the algorithm in scripts/checkpatch.pl is wrong! and that was my point. Guess we'll have to agree to disagree here, as I happen to think that checkpatch is perfectly right. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Mon, Apr 15, 2013 at 04:55:04PM -0400, Don Zickus wrote: On Fri, Apr 12, 2013 at 02:30:24PM -0700, Guenter Roeck wrote: On Fri, Apr 12, 2013 at 05:16:27PM -0400, Don Zickus wrote: On Wed, Apr 10, 2013 at 08:10:41AM -0700, Guenter Roeck wrote: have no idea how to even find out if multiple watchdogs are open on the system. Is there a list I could walk? And with regard to 'watchdog is /* the dev_t structure to store the dynamically allocated watchdog devices */ static dev_t watchdog_devt; One way to look up the allocated watchdogs might be to loop through all kobj instances for the major device using kobj_lookup. Don't know if there is a better way. Hmm, I got around to poking at this today and I am not sure kobj_lookup will work. Besides being surrounded with another mutex, I don't have access to the character device domain to pass to kobj_lookup. Perhaps I am not reading the code right, but I can't find a good way forward. The only other hack I can think of, is to embed a list object in the watchdog structure and list_add each new register'd watchdog. Then it would be trivial to walk the watchdog list. After looking into it again, I agree. Maybe you can give it a try. At least other options look even more complicated (eg creating a watchdog class ?). I looked at the watchdog class in watchdog_core.c. Even implemented class_for_each_device. But got stuck trying to figure out how to go from a struct device *dev to a struct watchdog_device. Then I realized in the bowels of class_for_each_device were spinlocks. :-( So I implemented an RCU list. It isn't the prettiest solution but it seems to work. I posted a V2 and realized I forgot to cc you on it. I apologize for that. :-( I hope you can find it, otherwise I can post a pointer to it. Where did you submit it ? I am subscribed to the watchdog mailing list, but I didn't see it. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
On Tue, Apr 16, 2013 at 09:17:27AM +, Opensource [Anthony Olech] wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 15 April 2013 18:46 To: Opensource [Anthony Olech] Cc: LKML Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver On Mon, Apr 15, 2013 at 05:29:13PM +, Opensource [Anthony Olech] wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 15 April 2013 17:36 To: Opensource [Anthony Olech] Cc: LKML Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver On Mon, Apr 15, 2013 at 03:00:58PM +, Opensource [Anthony Olech] wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 12 April 2013 14:32 To: Opensource [Anthony Olech] Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML; David Dajun Chen Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote: This is the REGULATOR component driver of the Dialog DA9058 PMIC. This driver is just one component of the whole DA9058 PMIC driver. It depends on the CORE component driver of the DA9058 MFD. There are 6 warnings from scripts/checkpatch.pl, but since it seems to be complaining about variable names such as min_uV are in CamelCase, when it is obvious that they are not in CamelCase I have ignored them. ??? min_uV _is_ CamelCase ??? Ok, maybe it is camelcasE, but you are splitting hairs here. it is not me splitting hairs, it is scripts/checkpatch.pl Maybe you did not understand what I meant. Per your logic, MicroVolt is CamelCase uVolt is ??? uV is not CamelCase By abbreviating CamelCase to camelCase to cC you make it, in your opinion, acceptable. If you want to declare CamelCase variables, just do it, but don't claim that they are not really CamelCase. Guenter I always thought that camel case meant changing from lower case to upper case the first letter of each word and then joining the capitalized words together, so by that definition uV or mW are not camel case because v and w are not words! The definition of CamelCase From Wikipedia, the free encyclopedia is: CamelCase (camel case) is a term which refers to the practice of writing compound words where the first letter of an identifier is lowercase and the first letter of each subsequent concatenated word is capitalized. Maybe the rule should read don't mix lowercase and uppercase letters in variable names and defines to prevent variable names such as cAmelcAse or cameLcasE, which would be permitted with your logic :). Either way it seems that the algorithm in scripts/checkpatch.pl is wrong! and that was my point. Guess we'll have to agree to disagree here, as I happen to think that checkpatch is perfectly right. Guenter Hi Guenter, I am quite happy to accept the algorithm in scripts/checkpatch.pl as the arbiter for correctly formed linux kernel variable names. On that basis old_mV, new_uA etc are incorrectly formed variable names. Could you possibly suggest legal alternatives to mA, uV, kW ?? I just changed it to lowercase in the ntc_thermistor driver. What you use is really your call as long as it does not mix uppercase and lowercase letters. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver
On Tue, Apr 16, 2013 at 11:36:33AM +0200, Wolfram Sang wrote: Doug, [ ... ] callenge response? ... diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c new file mode 100644 index 000..bda020a --- /dev/null +++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c @@ -0,0 +1,252 @@ +/* + * GPIO-based I2C Arbitration callenge response? s/callenge/challenge/g -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver
On Tue, Apr 16, 2013 at 09:29:00AM -0700, Doug Anderson wrote: The i2c-arb-gpio-challenge driver implements an I2C arbitration scheme where masters need to claim the bus with a GPIO before they can start a transcation. This should generally only be used when standard I2C I am having fun with spelling today :) s/transcation/transaction/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] watchdog: Add hook for kicking in kdump path
On Wed, Apr 17, 2013 at 05:19:56PM -0400, Don Zickus wrote:
A common problem with kdump is that during the boot up of the
second kernel, the hardware watchdog times out and reboots the
machine before a vmcore can be captured.
Instead of tellling customers to disable their hardware watchdog
timers, I hacked up a hook to put in the kdump path that provides
one last kick before jumping into the second kernel.
The assumption is the watchdog timeout is at least 10-30 seconds
long, enough to get the second kernel to userspace to kick the watchdog
again, if needed.
Of course kdump is usually executed on a panic path, so grabbing the
watchdog mutexes to communicate with the hardware won't work. For now,
I use trylock, otherwise fail.
I have tested this with a machine using iTCO_wdt and the 'watchdog' app.
The extra kicked happened as expected.
v2: based on feedback, implemented a linked list of watchdog references.
added trylock in watchdog_ping and used that function for kicking.
renamed export function to be more generic.
v3: small cleanups, remove mutex_safe variable from EXPORT_SYMBOL
Signed-off-by: Don Zickus dzic...@redhat.com
I know, I am nitpicking. Just a couple of small issues.
---
drivers/watchdog/watchdog_dev.c | 74 +++---
include/linux/watchdog.h|9 +
kernel/kexec.c |6 +++
3 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 08b48bb..52cb465 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -49,6 +49,16 @@ static dev_t watchdog_devt;
/* the watchdog device behind /dev/watchdog */
static struct watchdog_device *old_wdd;
+/* link list of all watchdog devices */
+struct watchdog_list {
+ spinlock_t lock;
+ struct list_head head;
+};
+static struct watchdog_list wdlist = {
+ .lock = __SPIN_LOCK_UNLOCKED(wdlist.lock),
+ .head = LIST_HEAD_INIT(wdlist.head),
+};
+
/*
* watchdog_ping: ping the watchdog.
* @wddev: the watchdog device to ping
@@ -59,11 +69,18 @@ static struct watchdog_device *old_wdd;
* We only ping when the watchdog device is running.
*/
-static int watchdog_ping(struct watchdog_device *wddev)
+static int watchdog_ping(struct watchdog_device *wddev, bool mutex_safe)
{
int err = 0;
- mutex_lock(wddev-lock);
+ if (mutex_safe) {
+ mutex_lock(wddev-lock);
+ } else {
+ if (!mutex_trylock(wddev-lock)) {
+ pr_warn(watchdog%d: Unable to lock mutex\n,
wddev-id);
+ return -EAGAIN;
+ }
+ }
if (test_bit(WDOG_UNREGISTERED, wddev-status)) {
err = -ENODEV;
@@ -83,6 +100,38 @@ out_ping:
return err;
}
+/**
+ * watchdog_kick_all: kick all the watchdogs
+ *
+ * There are times when the kernel needs to kick all the
+ * watchdogs at once without the use of references. For
+ * example in the kdump path, when the kernel is about
+ * to jump into the second kernel.
+ *
+ * The 'false' variable is for contextes that can not
+ * sleep, therefore try to kick the watchdog with trylock
+ * instead.
The variable no longer exists.
+ *
+ * Walk the link list locklessly using RCU to handle various
+ * contexts this could be called in. Should support irq and
+ * NMI contexts correctly.
+ */
+
+void watchdog_kick_all(void)
+{
+ struct watchdog_device *wddev;
+
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(wddev, wdlist.head, list)
+ watchdog_ping(wddev, false);
+
+ rcu_read_unlock();
+
+ return;
Unnecessary return statement.
+}
+EXPORT_SYMBOL_GPL(watchdog_kick_all);
+
/*
* watchdog_start: wrapper to start the watchdog.
* @wddev: the watchdog device to start
@@ -314,7 +363,7 @@ static ssize_t watchdog_write(struct file *file, const
char __user *data,
}
/* someone wrote to us, so we send the watchdog a keepalive ping */
- watchdog_ping(wdd);
+ watchdog_ping(wdd, true);
return len;
}
@@ -370,7 +419,7 @@ static long watchdog_ioctl(struct file *file, unsigned
int cmd,
case WDIOC_KEEPALIVE:
if (!(wdd-info-options WDIOF_KEEPALIVEPING))
return -EOPNOTSUPP;
- watchdog_ping(wdd);
+ watchdog_ping(wdd, true);
return 0;
case WDIOC_SETTIMEOUT:
if (get_user(val, p))
@@ -381,7 +430,7 @@ static long watchdog_ioctl(struct file *file, unsigned
int cmd,
/* If the watchdog is active then we send a keepalive ping
* to make sure that the watchdog keep's running (and if
* possible that it takes the new timeout) */
- watchdog_ping(wdd);
+ watchdog_ping(wdd, true);
Re: [PATCH v3] watchdog: Add hook for kicking in kdump path
On Wed, Apr 17, 2013 at 02:49:59PM -0700, Eric W. Biederman wrote: Don Zickus dzic...@redhat.com writes: A common problem with kdump is that during the boot up of the second kernel, the hardware watchdog times out and reboots the machine before a vmcore can be captured. Instead of tellling customers to disable their hardware watchdog timers, I hacked up a hook to put in the kdump path that provides one last kick before jumping into the second kernel. The assumption is the watchdog timeout is at least 10-30 seconds long, enough to get the second kernel to userspace to kick the watchdog again, if needed. Why not double the watchdog timeout? and/or pet the watchdog a little more frequently. This is the least icky hook I have seen proposed to be put on the kexec on panic path, but I still suspect this may reduce the ability to take a crash dump. What happens if it was the watchdog timer that panic'd for example. That should be addressed by the use of mutex_trylock(), which should fail in that case. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V5 6/7] drivers/hwmon: DA9058 HWMON driver
On Wed, Apr 17, 2013 at 05:33:36PM +0100, Anthony Olech wrote: This patch is relative to linux next-20130417 This is the HWMON component driver of the Dialog DA9058 PMIC. This driver is just one component of the whole DA9058 PMIC driver. It depends on the CORE and ADC component drivers of the DA9058 MFD. Changes relative to V3 of this patch: - rebased to latest tagged linux-next - previously relative to mainline Documentation/hwmon/da9058 - added final NL drivers/hwmon/Kconfig - changed dependancy from I2C to MFD drivers/hwmon/Makefile - put in alphabetical order drivers/hwmon/da9058-hwmon.c - aligned subsequent lines of function declarations - used single function for all slow labels - used recommended ..._label as function name - error conditions are returned as negative integers - chaned to suggested return value casting - removed all constant sysfile atributes except the labels - corrected parameter to adc read function to unsigned - used suggest name 'input' instead of 'value' - changed first temp attribute to temp1 - fixed expression error to boolean and from bitwise and - removed redundant return statement - removed race condition by initializing before create sysfs - corected alignments on broken long lines Signed-off-by: Anthony Olech anthony.olech.opensou...@diasemi.com Signed-off-by: David Dajun Chen david.c...@diasemi.com --- Documentation/hwmon/da9058 | 39 + drivers/hwmon/Kconfig| 10 ++ drivers/hwmon/Makefile |3 +- drivers/hwmon/da9058-hwmon.c | 349 ++ 4 files changed, 400 insertions(+), 1 deletion(-) create mode 100644 Documentation/hwmon/da9058 create mode 100644 drivers/hwmon/da9058-hwmon.c diff --git a/Documentation/hwmon/da9058 b/Documentation/hwmon/da9058 new file mode 100644 index 000..eaedfe7 --- /dev/null +++ b/Documentation/hwmon/da9058 @@ -0,0 +1,39 @@ +Kernel driver da9058-hwmon +== + +Supported chips: + * Dialog Semiconductor DA9058 PMIC +Prefix: 'da9058' +Datasheet: + http://www.dialog-semiconductor.com/products/power-management/da9058 + +Authors: Opensource [Anthony Olech] anthony.olech.opensou...@diasemi.com + +Description +--- + +The DA9058 PMIC contains a 5 channel ADC which can be used to monitor a +range of system operating parameters, including the battery voltage and +temperature. The ADC measures voltage, but two of the ADC channels can +be configured to supply a current, so that if an NTC termister is connected +then the voltage reading can be converted to a temperature. Currently the +driver provides reporting of all the input values but does not provide any +alarms. + +Voltage Monitoring +-- + +Voltages are sampled in either 'automatic' or 'manual' mode, which is an +initialization parameter set in the platform data by the machine driver. +In manual mode the ADC conversion is 12 bit and in automatic mode it is +10 bit. However all the raw readings are reported as 12 bit numbers. + +Physical Limits +--- + +vbat 2500 - 4500 milliVolts +tbat 0- 2500 milliVolts +adc0- 2500 milliVolts +vfpin 0- 4095 milliVolts +tjunc there is a correction factor programmed during manufacturing + diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index da93094..8014af2 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -324,6 +324,16 @@ config SENSORS_ATXP1 This driver can also be built as a module. If so, the module will be called atxp1. +config SENSORS_DA9058 + tristate Dialog Semiconductor DA9058 ADC + depends on MFD_DA9058 DA9058_ADC + help + If you say yes here you get support for the hardware monitoring + functionality of the Dialog Semiconductor DA9058 PMIC. + + This driver can also be built as a module. If so, the module + will be called da9058-hwmon. + config SENSORS_DS620 tristate Dallas Semiconductor DS620 depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index c51b0dc..5b7705a 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -46,7 +46,8 @@ obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o -obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o +obj-$(CONFIG_SENSORS_DA9055) += da9055-hwmon.o +obj-$(CONFIG_SENSORS_DA9058) += da9058-hwmon.o obj-$(CONFIG_SENSORS_DME1737)+= dme1737.o obj-$(CONFIG_SENSORS_DS620) += ds620.o obj-$(CONFIG_SENSORS_DS1621) += ds1621.o diff --git a/drivers/hwmon/da9058-hwmon.c b/drivers/hwmon/da9058-hwmon.c new file mode 100644 index 000..b273f58 --- /dev/null +++ b/drivers/hwmon/da9058-hwmon.c @@ -0,0 +1,349 @@ +/* + * Copyright (C) 2012 Dialog Semiconductor Ltd. + *
Re: [PATCH 1/4] mfd: Kontron PLD mfd driver
On Sat, Apr 13, 2013 at 10:38:07PM +0200, Thomas Gleixner wrote: [ ... ] + return kempld_read8(pld, index) | kempld_read8(pld, index+1) 8; index + 1) Please Wondering why does checkpatch not report those ? I just reviewed another driver with the same kind of problem, and checkpatch is just as silent. There seems to be a whole class of expressions where it does not complain about missing spaces. Joe, any idea ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mfd: Kontron PLD mfd driver
On Wed, Apr 17, 2013 at 09:40:53PM -0700, Joe Perches wrote: On Wed, 2013-04-17 at 21:19 -0700, Guenter Roeck wrote: On Sat, Apr 13, 2013 at 10:38:07PM +0200, Thomas Gleixner wrote: + return kempld_read8(pld, index) | kempld_read8(pld, index+1) 8; index + 1) Please Wondering why does checkpatch not report those ? because checkpatch doesn't care about spacing around arithmetic as long as it's consistent. Documentation/CodingStyle doesn't say anything about it either. Hi Joe, Use one space around (on each side of) most binary and ternary operators doesn't apply, then ? When does it apply ? I always thought it would apply to cases such as the above. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CodingStyle
On Thu, Apr 18, 2013 at 10:48:06AM +, Opensource [Anthony Olech] wrote:
-Original Message-
From: Guenter Roeck [mailto:li...@roeck-us.net]
Sent: 18 April 2013 05:14
To: Opensource [Anthony Olech]
Cc: Jean Delvare; Mark Brown; Randy Dunlap; lm-sens...@lm-sensors.org;
LKML; David Dajun Chen
Subject: Re: [NEW DRIVER V5 6/7] drivers/hwmon: DA9058 HWMON driver
[...]
+ if (cell == NULL) {
+ ret = -ENODEV;
+ goto exit;
Just return -ENODEV is good enough here. See CodingStyle, chapter 7.
[...]
+exit:
+ return ret;
+}
[...]
Hi Guenter,
I have read CodingStyle, chapter 7 and it seems to encourage a single exit
point from functions.
Yes, unless you can return directly. There is even an example.
It actually says ... and some common work such as cleanup has to be done.
During development there was some logging done at the (single) exit point but
that has been stripped out for submission.
Whilst I can duplicate the logging and do an immediate 'return' at all those
points, I am hesitant to do so when chapter 7 of the CodingStyle appears to
say to use a single exit point. In addition I had thought that a single exit
point from functions was a good thing.
Not if you add a goto to a return statement. That defeats the purpose
of easy readability.
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] watchdog: Add hook for kicking in kdump path
On Thu, Apr 18, 2013 at 09:00:09AM -0400, Don Zickus wrote: On Wed, Apr 17, 2013 at 02:49:59PM -0700, Eric W. Biederman wrote: Don Zickus dzic...@redhat.com writes: A common problem with kdump is that during the boot up of the second kernel, the hardware watchdog times out and reboots the machine before a vmcore can be captured. Instead of tellling customers to disable their hardware watchdog timers, I hacked up a hook to put in the kdump path that provides one last kick before jumping into the second kernel. The assumption is the watchdog timeout is at least 10-30 seconds long, enough to get the second kernel to userspace to kick the watchdog again, if needed. Why not double the watchdog timeout? and/or pet the watchdog a little more frequently. I am not sure if the watchdog timeouts can be doubled. I think Guenter was saying some have a max of a couple seconds?? Petting a little more frequently might be an option. Guenter can that be done with a softdog option? Most watchdog driver permit at least a minute. Some are more limited. Worst I have seen is the BookE watchdog timer (non-Freescale version) which has a maximum of three seconds. But that is broken anyway. Most hardware watchdogs implement a softdog on top of the hardware watchdog if the hardware needs to be pinged faster than every 60 seconds. So, yes, for the most common case you should actually be able to live with a, say, 30-60 second timeout which is pinged at least every 5-10 seconds. I thought that somehow did not work in your case. Maybe a misunderstanding ? If you have a customer with a specific problem on a specific watchdog which has a too-short maximum interval, maybe another solution sould be to look into that specific watchdog driver and see if it can be fixed. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] watchdog: Add hook for kicking in kdump path
On Thu, Apr 18, 2013 at 09:52:57AM -0400, Don Zickus wrote: On Thu, Apr 18, 2013 at 06:49:04AM -0700, Guenter Roeck wrote: On Thu, Apr 18, 2013 at 09:00:09AM -0400, Don Zickus wrote: On Wed, Apr 17, 2013 at 02:49:59PM -0700, Eric W. Biederman wrote: Don Zickus dzic...@redhat.com writes: A common problem with kdump is that during the boot up of the second kernel, the hardware watchdog times out and reboots the machine before a vmcore can be captured. Instead of tellling customers to disable their hardware watchdog timers, I hacked up a hook to put in the kdump path that provides one last kick before jumping into the second kernel. The assumption is the watchdog timeout is at least 10-30 seconds long, enough to get the second kernel to userspace to kick the watchdog again, if needed. Why not double the watchdog timeout? and/or pet the watchdog a little more frequently. I am not sure if the watchdog timeouts can be doubled. I think Guenter was saying some have a max of a couple seconds?? Petting a little more frequently might be an option. Guenter can that be done with a softdog option? Most watchdog driver permit at least a minute. Some are more limited. Worst I have seen is the BookE watchdog timer (non-Freescale version) which has a maximum of three seconds. But that is broken anyway. Most hardware watchdogs implement a softdog on top of the hardware watchdog if the hardware needs to be pinged faster than every 60 seconds. So, yes, for the most common case you should actually be able to live with a, say, 30-60 second timeout which is pinged at least every 5-10 seconds. I thought that somehow did not work in your case. Maybe a misunderstanding ? No, that will probably work. It is my misunderstanding. Is there a common way to check the timeout length and the ping frequency? Usually it is configured in /etc/watchdog.conf if the watchdog package is installed. The standard ping interval is interval, the timeout is watchdog-timeout. See man watchdog.conf for details. Minimum and maximum values for a given watchdog driver are not exported to user space, so you would have to look into the driver sources to find out what they are. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mfd: Kontron PLD mfd driver
On Thu, Apr 18, 2013 at 09:42:17AM -0700, Joe Perches wrote: On Thu, 2013-04-18 at 06:35 -0700, Guenter Roeck wrote: On Wed, Apr 17, 2013 at 09:40:53PM -0700, Joe Perches wrote: On Wed, 2013-04-17 at 21:19 -0700, Guenter Roeck wrote: On Sat, Apr 13, 2013 at 10:38:07PM +0200, Thomas Gleixner wrote: + return kempld_read8(pld, index) | kempld_read8(pld, index+1) 8; index + 1) Please Wondering why does checkpatch not report those ? because checkpatch doesn't care about spacing around arithmetic as long as it's consistent. Documentation/CodingStyle doesn't say anything about it either. Hi Joe, Use one space around (on each side of) most binary and ternary operators doesn't apply, then ? When does it apply ? I always thought it would apply to cases such as the above. There's a _lot_ of code that doesn't follow the single space around binary operators style, it's contentious, and was determined when Andy did the original checkpatch implementation to not be a valuable addition or worth the complaint pain. Looks like it is contentious either way. Thanks a lot for the clarification. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V6 6/7] drivers/hwmon: DA9058 HWMON driver
On Fri, Apr 19, 2013 at 08:25:02PM +0200, Lars-Peter Clausen wrote:
Same comment as before, I'd like to see this using the generic IIO to HWMON
bridge instead of recreating it.
... and I agree. Seems we are getting more and more of those, and at some point
it makes really sense to find a generic solution.
Anthony, can you please look into that ?
Thanks,
Guenter
On 04/19/2013 06:56 PM, Anthony Olech wrote:
This patch is relative to next-20130419 of linux-next
This is the HWMON component driver of the Dialog DA9058 PMIC.
This driver is just one component of the whole DA9058 PMIC driver.
It depends on the CORE and ADC component drivers of the DA9058 MFD.
Please note that this driver does use regmap via the CORE and ADC
component drivers of the DA9058 MFD.
Changes relative to V5 of this patch:
- rebased to next-20130419 in
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
- removed redundant #include linux/mfd/da9058/version.h
- corrected dates on copyright statements
Documentation/hwmon/da9058
- removed trailing blank line to prevent 'git apply' warning
drivers/hwmon/da9058-hwmon.c
- put spaces aount the '*' multiply operator
- use the word 'extract' rather than 'recover' in a comment
- use da9058_labels[] in show_label instead of switch case
- use multiple exit points in functions when no common code
is to be executed.
- aligned continuation lines to preceeding '(' or indent + 2 tabs
- removed redundant mutex hwmon_lock
- merged 6 duplicate lines from 2 branches of if statement
Signed-off-by: Anthony Olech anthony.olech.opensou...@diasemi.com
Signed-off-by: David Dajun Chen david.c...@diasemi.com
---
Documentation/hwmon/da9058 | 38 +
drivers/hwmon/Kconfig| 10 ++
drivers/hwmon/Makefile |3 +-
drivers/hwmon/da9058-hwmon.c | 330
++
4 files changed, 380 insertions(+), 1 deletion(-)
create mode 100644 Documentation/hwmon/da9058
create mode 100644 drivers/hwmon/da9058-hwmon.c
diff --git a/Documentation/hwmon/da9058 b/Documentation/hwmon/da9058
new file mode 100644
index 000..841148f
--- /dev/null
+++ b/Documentation/hwmon/da9058
@@ -0,0 +1,38 @@
+Kernel driver da9058-hwmon
+==
+
+Supported chips:
+ * Dialog Semiconductor DA9058 PMIC
+Prefix: 'da9058'
+Datasheet:
+ http://www.dialog-semiconductor.com/products/power-management/da9058
+
+Authors: Opensource [Anthony Olech] anthony.olech.opensou...@diasemi.com
+
+Description
+---
+
+The DA9058 PMIC contains a 5 channel ADC which can be used to monitor a
+range of system operating parameters, including the battery voltage and
+temperature. The ADC measures voltage, but two of the ADC channels can
+be configured to supply a current, so that if an NTC termister is connected
+then the voltage reading can be converted to a temperature. Currently the
+driver provides reporting of all the input values but does not provide any
+alarms.
+
+Voltage Monitoring
+--
+
+Voltages are sampled in either 'automatic' or 'manual' mode, which is an
+initialization parameter set in the platform data by the machine driver.
+In manual mode the ADC conversion is 12 bit and in automatic mode it is
+10 bit. However all the raw readings are reported as 12 bit numbers.
+
+Physical Limits
+---
+
+vbat 2500 - 4500 milliVolts
+tbat 0- 2500 milliVolts
+adc0- 2500 milliVolts
+vfpin 0- 4095 milliVolts
+tjunc there is a correction factor programmed during manufacturing
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 79bc431..cb074c4 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -337,6 +337,16 @@ config SENSORS_ATXP1
This driver can also be built as a module. If so, the module
will be called atxp1.
+config SENSORS_DA9058
+ tristate Dialog Semiconductor DA9058 ADC
+ depends on MFD_DA9058 DA9058_ADC
+ help
+ If you say yes here you get support for the hardware monitoring
+ functionality of the Dialog Semiconductor DA9058 PMIC.
+
+ This driver can also be built as a module. If so, the module
+ will be called da9058-hwmon.
+
config SENSORS_DS620
tristate Dallas Semiconductor DS620
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d17d3e6..6001549 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -47,7 +47,8 @@ obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
obj-$(CONFIG_SENSORS_ATXP1)+= atxp1.o
obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
-obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
+obj-$(CONFIG_SENSORS_DA9055) += da9055-hwmon.o
+obj-$(CONFIG_SENSORS_DA9058) +=
Re: [NEW DRIVER V6 6/7] drivers/hwmon: DA9058 HWMON driver
On Mon, Apr 22, 2013 at 01:14:26AM +, Opensource [Anthony Olech] wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 20 April 2013 18:35 To: Lars-Peter Clausen Cc: Opensource [Anthony Olech]; Jean Delvare; Mark Brown; Randy Dunlap; lm-sens...@lm-sensors.org; LKML; David Dajun Chen Subject: Re: [NEW DRIVER V6 6/7] drivers/hwmon: DA9058 HWMON driver On Fri, Apr 19, 2013 at 08:25:02PM +0200, Lars-Peter Clausen wrote: Same comment as before, I'd like to see this using the generic IIO to HWMON bridge instead of recreating it. ... and I agree. Seems we are getting more and more of those, and at some point it makes really sense to find a generic solution. Anthony, can you please look into that ? Thanks, Guenter OK -- generic IIO to HWMON bridge I will investigate - but if you know any prototypes or any useful starting point could you let me know as I don't want to re-invent the wheel. The idea is that you would implement an IIO ADC driver (drivers/iio/adc has lots of examples). The IIO to HWMON bridge already exists (drivers/hwmon/iio_hwmon.c in linux-next or drivers/staging/iio/iio_hwmon.c in mainline). Setting it up would then then a matter of platform data or device tree data. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] watchdog: fix w83627hf_wdt clear timeout expired
On Wed, Apr 03, 2013 at 09:39:44PM -0700, Tony Chung wrote:
Observed that the Watchdog Timer Status bit can be set when the driver is
loaded. Reset it during initialization. The time-out value must be set to 0
explicitly in this case to prevent an immediate reset.
Depending on the motherboard and with watchdog disabled in the BIOS, this
reset problem can easily be reproduced by the following steps:
1. reboot the system
2. wait for 5+ minutes
3. don't start any watchdog server.
4. just run modprobe w83627hf_wdt
If the system load the driver after 5 minutes, it rebooted immediately
because of timer expired. For example, fsck could take more than 5 minutes
to run,
then the computer reboot as soon as the driver was loaded.
[ the above aplies to your system, so I would not add that to the commit log,
but that it up to Wim to decide ]
Signed-off-by: Tony Chung tonychun...@gmail.com
Acked-by: Guenter Roeck li...@roeck-us.net
---
drivers/watchdog/w83627hf_wdt.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 8fd..7eaa226 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -168,14 +168,16 @@ static void w83627hf_init(void)
pr_info(Watchdog already running. Resetting timeout to %d
sec\n,
wdt_get_timeout_secs(timeout));
outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */
+ } else {
+ outb_p(0, WDT_EFDR);/* disable to prevent reboot */
}
w83627hf_setup_crf5();
outb_p(0xF7, WDT_EFER); /* Select CRF7 */
t = inb_p(WDT_EFDR); /* read CRF7 */
- t = ~0xC0; /* disable keyboard mouse turning off
- watchdog */
+ t = ~0xD0; /* clear timeout occurred and disable keyboard
+ mouse turning off watchdog */
outb_p(t, WDT_EFDR);/* Write back to CRF7 */
w83627hf_unselect_wd_register();
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-watchdog in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Blackfin: bf537: rename CONFIG_AD7314
On Thu, Apr 04, 2013 at 12:17:03PM +0200, Paul Bolle wrote:
[Forwarded to a recent address of Guenter, as the ericsson address
bounces.]
Paul Bolle
On Thu, 2013-04-04 at 12:08 +0200, Paul Bolle wrote:
In v3.2 the Analog Devices AD7314 temperature sensor driver was removed
as an IIO driver and added as a HWMON driver. But it was apparently
overlooked to rename two references to CONFIG_AD7314 to
CONFIG_SENSORS_AD7314. Do so now. Use the IS_ENABLED() macro, while
we're at it.
Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) Entirely untested.
1) See commits 4f3a659581cabf1be441d6467b523be914615496 (hwmon: AD7314
driver (ported from IIO)) and 48a2c3799b7141c271a771d3249142a104faeefc
(staging:iio:adc:ad7314 removal. Supported via hwmon.) for the two
patches involved.
arch/blackfin/mach-bf537/boards/stamp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/blackfin/mach-bf537/boards/stamp.c
b/arch/blackfin/mach-bf537/boards/stamp.c
index 23bb55d..b40b849 100644
--- a/arch/blackfin/mach-bf537/boards/stamp.c
+++ b/arch/blackfin/mach-bf537/boards/stamp.c
@@ -681,7 +681,7 @@ static struct bfin5xx_spi_chip ad2s1210_spi_chip_info =
{
};
#endif
-#if defined(CONFIG_AD7314) || defined(CONFIG_AD7314_MODULE)
+#if IS_ENABLED(CONFIG_SENSORS_AD7314)
static struct bfin5xx_spi_chip ad7314_spi_chip_info = {
.enable_dma = 0,
};
@@ -1039,7 +1039,7 @@ static struct spi_board_info bfin_spi_board_info[]
__initdata = {
},
#endif
-#if defined(CONFIG_AD7314) || defined(CONFIG_AD7314_MODULE)
+#if IS_ENABLED(CONFIG_SENSORS_AD7314)
{
.modalias = ad7314,
.max_speed_hz = 100,
Acked-by: Guenter Roeck li...@roeck-us.net
Really wonder ... there should be a better solution than all those ifdefs.
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Blackfin: bf537: rename CONFIG_ADT75
On Thu, Apr 04, 2013 at 01:40:03PM +0200, Jean Delvare wrote:
On Thu, 04 Apr 2013 12:31:06 +0200, Paul Bolle wrote:
In v3.2 the Analog Devices ADT75 temperature sensor driver was removed
as an IIO driver and support for it was added to the LM75 HWMON driver.
But it was apparently overlooked to rename one reference to CONFIG_ADT75
to CONFIG_SENSORS_LM75. Do so now. Use the IS_ENABLED() macro, while
we're at it.
Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) Also untested.
1) See commits e96f9d89e6213c7630a3323cd0c754e7f2619564 (hwmon: (lm75)
Add support for Analog Devices ADT75) and
cdea0bec8d37f2943ae500512b0c178bc76de6e3 (iio: adc: remove ADT75 driver
- hwmon/lm75 will take over ADT75 support) for the two patches involved
in this operation.
arch/blackfin/mach-bf537/boards/stamp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
This breakage is telling us something about the weird approach taken in
this file, methinks.
Yes, they have persistent problems with renamed or moved drivers. I somewhat
understood the need for SPI chips, but even there it seemed to me there should
be a better solution.
diff --git a/arch/blackfin/mach-bf537/boards/stamp.c
b/arch/blackfin/mach-bf537/boards/stamp.c
index b40b849..9735345 100644
--- a/arch/blackfin/mach-bf537/boards/stamp.c
+++ b/arch/blackfin/mach-bf537/boards/stamp.c
@@ -2221,7 +2221,7 @@ static struct i2c_board_info __initdata
bfin_i2c_board_info[] = {
},
#endif
-#if defined(CONFIG_ADT75) || defined(CONFIG_ADT75_MODULE)
+#if IS_ENABLED(CONFIG_SENSORS_LM75)
{
I2C_BOARD_INFO(adt75, 0x9),
Unrelated to this patch, but this is very odd, as the ADT75 can't have
slave address 0x09. This address is almost exclusively used by battery
chargers AFAIK.
Either not really an ADT75, or it should have been 0x49.
Couple of lines down, CONFIG_ADT7410 has the same problem.
Guenter
.irq = IRQ_PG5,
The patch itself looks good.
Reviewed-by: Jean Delvare kh...@linux-fr.org
--
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Blackfin: bf537: rename CONFIG_ADT75
On Thu, Apr 04, 2013 at 02:59:43PM +0200, Paul Bolle wrote: On Thu, 2013-04-04 at 05:07 -0700, Guenter Roeck wrote: On Thu, Apr 04, 2013 at 01:40:03PM +0200, Jean Delvare wrote: This breakage is telling us something about the weird approach taken in this file, methinks. Yes, they have persistent problems with renamed or moved drivers. I somewhat understood the need for SPI chips, but even there it seemed to me there should be a better solution. The output of the (local) scripts I use mention about a dozen more (possible) problems in both Blackfin's stamp.c files. I'm roughly working my way back in git history, slowly, so those problems are probably older. Since I know from previous cleaning up efforts that the Blackfin people know they have non-working code (eg, code not actually buildable since it has no working Kconfig support) I do not give Blackfin much priority. But this patch (and the other two related to the v3.2 release) were obvious oversights that I could as well fix right away. Makes me shiver ... Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 3.8.3 and 3.9git occasional watchdog oops
On Fri, Apr 05, 2013 at 12:23:30AM +0200, Arkadiusz Miskiewicz wrote: On Thursday 14 of March 2013, Arkadiusz Miśkiewicz wrote: Hi. Just hit watchdog related oops in 3.8.3 kernel. Unfortunately photos only. http://ixion.pld-linux.org/~arekm/watchdog-oops-3.8.3/IMG_8942.JPG http://ixion.pld-linux.org/~arekm/watchdog-oops-3.8.3/IMG_8941.JPG 3.9git from today isn't any better unfortunately: http://ixion.pld-linux.org/~arekm/watchdog-oops-3.9git.jpg oops started after I enabled systemd watchdog functionality. Cannot reproduce easily. watchdog here (thinkpad t400) is: iTCO_wdt: Found a ICH9M-E TCO device (Version=2, TCOBASE=0x1060) Wonder if there is a race condition in the watchdog driver: The watchdog device is opened before watchdog_register_device returns. I suspect systemd waits for a udev event, or by some other means detects that /dev/watchdog was created, and opens it immediately. I just have no idea where exactly the race condition, if there is one, is hiding. Or maybe I am completely off track. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Blackfin: bf537: use CONFIG_SND_SOC_AD193X
On Fri, Apr 05, 2013 at 11:43:37AM +0200, Jean Delvare wrote: On Fri, 05 Apr 2013 11:20:46 +0200, Paul Bolle wrote: 1) Added Jean and Guenter because they seem to take in interest in Blackfin's stamp files. Doh, no, I only express my disgust and I'd rather stay away from them as much as I can ;) Same here :) Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] watchdog: core: don't try to stop device if not running
On Fri, Apr 05, 2013 at 06:09:44PM +0200, Hector Palacios wrote:
A watchdog device may be stopped from userspace using WDIOC_SETOPTIONS
ioctl and flag WDIOS_DISABLECARD. If the device is closed after this
operation, watchdog_release() is called and status bits checked for
stopping it. Besides, if the device has not been unregistered a critical
message watchdog did not stop! is printed, although the ioctl may have
successfully stopped it already.
Without the patch a user application sample code like this will successfully
stop the watchdog, but the kernel will output the message
watchdog did not stop!:
wd_fd = open(/dev/watchdog, O_RDWR);
flags = WDIOS_DISABLECARD;
ioctl(wd_fd, WDIOC_SETOPTIONS, flags);
close(wd_fd);
Signed-off-by: Hector Palacios hector.palac...@digi.com
How about the following patch instead ?
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 08b48bb..9775e8d 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -469,7 +469,9 @@ static int watchdog_release(struct inode *inode, struct
file *file)
* or if WDIOF_MAGICCLOSE is not set. If nowayout was set then
* watchdog_stop will fail.
*/
- if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
+ if (!test_bit(WDOG_ACTIVE, wdd-status))
+ err = 0;
+ else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
!(wdd-info-options WDIOF_MAGICCLOSE))
err = watchdog_stop(wdd);
Much less invasive and the result is the same.
Thanks,
Guenter
---
drivers/watchdog/watchdog_dev.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index ef8edec..adadbdd 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -463,24 +463,28 @@ out:
static int watchdog_release(struct inode *inode, struct file *file)
{
struct watchdog_device *wdd = file-private_data;
- int err = -EBUSY;
- /*
- * We only stop the watchdog if we received the magic character
- * or if WDIOF_MAGICCLOSE is not set. If nowayout was set then
- * watchdog_stop will fail.
- */
- if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
- !(wdd-info-options WDIOF_MAGICCLOSE))
- err = watchdog_stop(wdd);
-
- /* If the watchdog was not stopped, send a keepalive ping */
- if (err 0) {
- mutex_lock(wdd-lock);
- if (!test_bit(WDOG_UNREGISTERED, wdd-status))
- dev_crit(wdd-dev, watchdog did not stop!\n);
- mutex_unlock(wdd-lock);
- watchdog_ping(wdd);
+ if (test_bit(WDOG_ACTIVE, wdd-status)) {
+ int err = -EBUSY;
+
+ /*
+ * We only stop the watchdog if we received the magic character
+ * or if WDIOF_MAGICCLOSE is not set. If nowayout was set then
+ * watchdog_stop will fail.
+ */
+ if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
+ !(wdd-info-options WDIOF_MAGICCLOSE)) {
+ err = watchdog_stop(wdd);
+ }
+
+ /* If the watchdog was not stopped, send a keepalive ping */
+ if (err 0) {
+ mutex_lock(wdd-lock);
+ if (!test_bit(WDOG_UNREGISTERED, wdd-status))
+ dev_crit(wdd-dev, watchdog did not stop!\n);
+ mutex_unlock(wdd-lock);
+ watchdog_ping(wdd);
+ }
}
/* Allow the owner module to be unloaded again */
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-watchdog in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 3.8.3 and 3.9git occasional watchdog oops
On Thu, Apr 04, 2013 at 06:59:59PM -0700, Guenter Roeck wrote: On Fri, Apr 05, 2013 at 12:23:30AM +0200, Arkadiusz Miskiewicz wrote: On Thursday 14 of March 2013, Arkadiusz Miśkiewicz wrote: Hi. Just hit watchdog related oops in 3.8.3 kernel. Unfortunately photos only. http://ixion.pld-linux.org/~arekm/watchdog-oops-3.8.3/IMG_8942.JPG http://ixion.pld-linux.org/~arekm/watchdog-oops-3.8.3/IMG_8941.JPG 3.9git from today isn't any better unfortunately: http://ixion.pld-linux.org/~arekm/watchdog-oops-3.9git.jpg oops started after I enabled systemd watchdog functionality. Cannot reproduce easily. watchdog here (thinkpad t400) is: iTCO_wdt: Found a ICH9M-E TCO device (Version=2, TCOBASE=0x1060) Wonder if there is a race condition in the watchdog driver: The watchdog device is opened before watchdog_register_device returns. I suspect systemd waits for a udev event, or by some other means detects that /dev/watchdog was created, and opens it immediately. I just have no idea where exactly the race condition, if there is one, is hiding. Or maybe I am completely off track. I _think_ I understand the sequence of events. - The driver is the first watchdog driver to register. - watchdog_dev_register() gets called and creates the watchdog misc device by calling misc_register(). At that time, the matching character device (/dev/watchdog0) does not yet exist, and old_wdd is not set either. - Userspace gets an event and opens /dev/watchdog - watchdog_open() is called and sets sets wdd = old_wdd, which is still NULL, and tries to dereference it. Bang. If this is the problem, a simple fix would be to set old_wdd before calling misc_register(). Can you test a patch ? Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] watchdog: Fix race condition in registration code
A race condition exists when registering the first watchdog device.
Sequence of events:
- watchdog_register_device calls watchdog_dev_register
- watchdog_dev_register creates the watchdog misc device by calling
misc_register.
At that time, the matching character device (/dev/watchdog0) does not yet
exist, and old_wdd is not set either.
- Userspace gets an event and opens /dev/watchdog
- watchdog_open is called and sets sets wdd = old_wdd, which is still NULL,
and tries to dereference it. This causes the kernel to panic.
Seen with systemd trying to open /dev/watchdog immediately after
it was created.
Reported-by: Arkadiusz Miskiewicz a.miskiew...@gmail.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
Arkadiusz,
would be great if you can test this in your system.
drivers/watchdog/watchdog_dev.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 08b48bb..faf4e18 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -523,6 +523,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
int err, devno;
if (watchdog-id == 0) {
+ old_wdd = watchdog;
watchdog_miscdev.parent = watchdog-parent;
err = misc_register(watchdog_miscdev);
if (err != 0) {
@@ -531,9 +532,9 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
if (err == -EBUSY)
pr_err(%s: a legacy watchdog module is
probably present.\n,
watchdog-info-identity);
+ old_wdd = NULL;
return err;
}
- old_wdd = watchdog;
}
/* Fill in the data structures */
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver
On Fri, Apr 05, 2013 at 02:03:52PM -0600, Stephen Warren wrote: On 04/05/2013 01:37 PM, Simon Glass wrote: HI Wolfram, On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang w...@the-dreams.de wrote: Doug, Separately from a discussion of the technical merits, I'd say that this patch is needed because the Embedded Controller (EC) on the ARM Chromebook shipped expecting to communicate with this scheme. While Uhrm, with all respect, we already shipped it is not a convincing argument regarding inclusion. Benefit for the kernel is. I'm not quite sure why that isn't a convincing argument. The hardware has shipped. I don't know whether the EC microcode can be updated in the field; it seems risky to do so even if it's possible. So, it either gets supported or not; the HW/ucode isn't going to change I suspect. Hence, it seems that the decision would be: a) Disallow the implementation of the arbitration scheme in the kernel, and hence don't support this board in the kernel. (or at least some very core features of this board) b) Allow the implementation of the arbitration scheme in the kernel, and hence make possible support this board in the kernel. From that perspective, the benefit for the kernel question comes down to: do we see a benefit for the kernel to support this board? I can't see why that wouldn't be a benefit. The only disadvantage would be having to carrying code to support that board. That same argument can be made for any board, and I think typically doesn't cause any issue. The code for this I2C mux seems pretty self-contained, so even if it was absolutely terrible (which I don't think it is), it still wouldn't cause any wide-spread issues, I think. Very interesting discussion, especially the argument that we already shipped would not be a convincing argument. I had senior kernel maintainers tell me and the company I work for that we should submit _all_ our platform specific kernel code and drivers for inclusion into the upstream kernel. This exchange suggests that it is a shipping product does not count for such submissions, and that Benefit for the kernel would be the deciding factor instead. Which of course is a very vague statement - if code supporting Chromebookis is of no benefit for the kernel, support for my company's products for sure is much less so. Which kind of leaves me in a bind. On one side I promote that we should submit all our kernel code, on the other side there is a very compelling case to be made that it won't be accepted anyway. That doesn't make my life easier - essentially since it supports those who say that we should not submit anything in the first place. And believe me, there are many of those. Just to give some examples: - I2C multiplexers. We have a bunch of those. Looking at this exchange, it doesn't look good to get that code included. - Custom multi-function FPGAs and CPLDs, amongst others implementing I2C controllers, I2C muxes, GPIO access, Flash access, and other functions. Same as above. - Devicetree support for UIO devices (mostly forwarding ASICs), including gpio bindings, interrupt bindings, and clock bindings. Looking at older exchanges, that doesn't look good either. And please dont expect me to implement hacks around a clean solution because any devicetree binding for UIO drivers does not describe hardware but its use. Now, I can understand that there may be technical or architectural issues preventing one driver or another from being accepted. For example, I can understand if a driver for an USB-I2C adapder isn't accepted because the adapter reports itself to the USB subsystem as serial driver. But Benefit for the kernel is vague enough to reject anything for no real reason other than someone not liking it (or the submitter, or the company the submitter works for, or the hardware architecture). It would be nice have to get some well defined guidelines for acceptable contributions. Benefit for the kernel sure isn't one. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver
On Sat, Apr 06, 2013 at 10:11:32PM +0200, Wolfram Sang wrote: Hi, Very interesting discussion, especially the argument that we already shipped would not be a convincing argument. I had senior kernel maintainers tell me and the company I work for that we should submit _all_ our platform specific kernel code and drivers for inclusion into the upstream kernel. Yes, great. Really! Yes, though, thinking about it, it was submit and didn't say anything about potential for acceptance. This exchange suggests that it is a shipping product does not count for such submissions, and that Benefit for the kernel would be the deciding factor instead. Which of course is a very vague statement - if code supporting Chromebookis is of no benefit for the kernel, support for my company's products for sure is much less so. First, let me state that I did not intend to say that the arbitrator muxer here has NO benefit for the kernel. I was aware there might be arguments for that and I wanted some more discussion to make that clearer to me. Simon's mail was very helpful in that regard and I will comment on that somewhen later. Still, I do have problems with we already shipped it. If the driver is bad or the underlying concept is fragile, I want the freedom to reject a patch, product shipped or not. I will be supportive to find a proper solution, promised. If all fails, there is still staging/ or the possibility of out-of-tree patches. I think there is a difference between a bad driver or underlying hardware. To me, shipped applies to hardware or firmware which can not be upgraded, not to the software running on it. Which kind of leaves me in a bind. On one side I promote that we should submit all our kernel code, on the other side there is a very compelling case to be made that it won't be accepted anyway. That doesn't make my life easier - Concentrate on argumenting why the driver is useful and it will be fine. we already shipped this feels a bit like blackmailing to me. And since most drivers do have well thought reasons for their existance, I'd primarily like to hear about those. essentially since it supports those who say that we should not submit anything in the first place. And believe me, there are many of those. Just to give some examples: - I2C multiplexers. We have a bunch of those. Looking at this exchange, it doesn't look good to get that code included. Multiplexers should be easy going, it is the arbitration which is discussed here. I am open to consider some generic arbitration schemes. What I am reluctant to do is to allow every board to have its own arbitration scheme. This would be board support hosted in the I2C directory. Meh. board support hosted in the I2C directory. But that is exactly what I am talking about, isn't it ? I have board specific multiplexers and a board specific I2C controller, and that is just talking about the I2C code. It would be nice have to get some well defined guidelines for acceptable contributions. Benefit for the kernel sure isn't one. I don't think it is possible to write down concrete guidelines for this. According to rule 4a) of the guidelines you have to accept my patch? That would be a mess, I think. Looking at it from a maintainer perspective, I agree. Where it gets murky is really the hardware part. The (in my opinion) philosophical arguments around not permitting device-tree based instantiation of uio devices is one example. Another practical example I had to deal with in my previous company is VGA memory space. Some hw geniuses decided to re-use the VGA memory space in an embedded x86 device for an EEPROM. Guess what - the x86 kernel writes into that space no matter what. A patch to address that problem was rejected because you should not re-use VGA memory space. As if I had a choice. A better example might be Kontron board support. They implement gpio, I2C mux, and a watchdog in a PLD. They too have an access arbitration scheme where one has to acquire a hardware mutex before accessing the pld, if I understand correctly because some microcontroller might need to access it as well. Leaving the actual code aside, would you reject that too if you don't like the arbitration scheme, or because you don't want to have board support in the i2c directory ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v5] clk: add si5351 i2c common clock driver
On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver supports
DT kernels only and VXCO feature of si5351b is not implemented. DT
bindings selectively allow to overwrite stored Si5351 configuration
which is very helpful for clock generators with empty eeprom
configuration. Corresponding device tree binding documentation is
also added.
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Tested-by: Daniel Mack zon...@gmail.com
[ ... ]
+static inline void _si5351_msynth_set_pll_master(
+ struct si5351_driver_data *drvdata, unsigned char num, int is_master)
+{
+ unsigned long flags;
+
+ if (num 8 ||
+ (drvdata-variant == SI5351_VARIANT_A3 num 3))
+ return;
+
+ flags = __clk_get_flags(drvdata-msynth[num].hw.clk);
+ if (is_master)
+ flags |= CLK_SET_RATE_PARENT;
+ else
+ flags = ~CLK_SET_RATE_PARENT;
+ __clk_set_flags(drvdata-msynth[num].hw.clk, flags);
+}
+
Unless I am missing something, neither __clk_get_flags() nor the new
__clk_set_flags is exported.
Did you try to build and load the driver as module ?
Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [v5] clk: add si5351 i2c common clock driver
On Mon, Apr 08, 2013 at 01:49:24AM +0200, Sebastian Hesselbarth wrote:
On 04/08/2013 12:50 AM, Guenter Roeck wrote:
On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver supports
DT kernels only and VXCO feature of si5351b is not implemented. DT
bindings selectively allow to overwrite stored Si5351 configuration
which is very helpful for clock generators with empty eeprom
configuration. Corresponding device tree binding documentation is
also added.
Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
Tested-by: Daniel Mackzon...@gmail.com
[ ... ]
+static inline void _si5351_msynth_set_pll_master(
+ struct si5351_driver_data *drvdata, unsigned char num, int is_master)
+{
+ unsigned long flags;
+
+ if (num 8 ||
+ (drvdata-variant == SI5351_VARIANT_A3 num 3))
+ return;
+
+ flags = __clk_get_flags(drvdata-msynth[num].hw.clk);
+ if (is_master)
+ flags |= CLK_SET_RATE_PARENT;
+ else
+ flags= ~CLK_SET_RATE_PARENT;
+ __clk_set_flags(drvdata-msynth[num].hw.clk, flags);
+}
+
Unless I am missing something, neither __clk_get_flags() nor the new
__clk_set_flags is exported.
Did you try to build and load the driver as module ?
Well, good catch. I didn't try to build v5 as a module, but I guess it
will fail. But I consider this as something that has to be addressed in
clk framework itself, not in this patch. There will be other
clk-providers built as module in the future for sure.
Sure, but you provided the patch to make __clk_set_flags global. To avoid
build failures, I would suggest to either submit a patch to export the
missing functions, or to remove the ability to build the driver as module.
On a side note, do you happen to know anyone working on drivers for Si5319 or
Si5368 ?
Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] watchdog: core: don't try to stop device if not running
On Mon, Apr 08, 2013 at 09:48:57AM +0200, Hector Palacios wrote:
On 04/05/2013 08:34 PM, Guenter Roeck wrote:
On Fri, Apr 05, 2013 at 06:09:44PM +0200, Hector Palacios wrote:
A watchdog device may be stopped from userspace using WDIOC_SETOPTIONS
ioctl and flag WDIOS_DISABLECARD. If the device is closed after this
operation, watchdog_release() is called and status bits checked for
stopping it. Besides, if the device has not been unregistered a critical
message watchdog did not stop! is printed, although the ioctl may have
successfully stopped it already.
Without the patch a user application sample code like this will successfully
stop the watchdog, but the kernel will output the message
watchdog did not stop!:
wd_fd = open(/dev/watchdog, O_RDWR);
flags = WDIOS_DISABLECARD;
ioctl(wd_fd, WDIOC_SETOPTIONS, flags);
close(wd_fd);
Signed-off-by: Hector Palacios hector.palac...@digi.com
How about the following patch instead ?
diff --git a/drivers/watchdog/watchdog_dev.c
b/drivers/watchdog/watchdog_dev.c
index 08b48bb..9775e8d 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -469,7 +469,9 @@ static int watchdog_release(struct inode *inode, struct
file *file)
* or if WDIOF_MAGICCLOSE is not set. If nowayout was set then
* watchdog_stop will fail.
*/
-if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
+if (!test_bit(WDOG_ACTIVE, wdd-status))
+err = 0;
+else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
!(wdd-info-options WDIOF_MAGICCLOSE))
err = watchdog_stop(wdd);
Much less invasive and the result is the same.
I like the simplicity but it is kind of inverted logic to initially
define err = -EBUSY only to turn it to zero later, so I'm rebuilding
your approach like this:
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index ef8edec..a4163cd 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -463,16 +463,19 @@ out:
static int watchdog_release(struct inode *inode, struct file *file)
{
struct watchdog_device *wdd = file-private_data;
- int err = -EBUSY;
+ int err = 0;
/*
* We only stop the watchdog if we received the magic character
* or if WDIOF_MAGICCLOSE is not set. If nowayout was set then
* watchdog_stop will fail.
*/
- if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
- !(wdd-info-options WDIOF_MAGICCLOSE))
+ if (test_bit(WDOG_ACTIVE, wdd-status))
+ err = -EBUSY;
+ else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
+ !(wdd-info-options WDIOF_MAGICCLOSE)) {
err = watchdog_stop(wdd);
+ }
Ok, but the added { } are unnecessary and violate coding style rules.
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] watchdog: core: don't try to stop device if not running
On Mon, Apr 08, 2013 at 11:10:58AM +0200, Hector Palacios wrote:
A watchdog device may be stopped from userspace using WDIOC_SETOPTIONS
ioctl and flag WDIOS_DISABLECARD. If the device is closed after this
operation, watchdog_release() is called and status bits checked for
stopping it. Besides, if the device has not been unregistered a critical
message watchdog did not stop! is printed, although the ioctl may have
successfully stopped it already.
Without the patch a user application sample code like this will successfully
stop the watchdog, but the kernel will output the message
watchdog did not stop!:
wd_fd = open(/dev/watchdog, O_RDWR);
flags = WDIOS_DISABLECARD;
ioctl(wd_fd, WDIOC_SETOPTIONS, flags);
close(wd_fd);
Signed-off-by: Hector Palacios hector.palac...@digi.com
---
Changes from v1:
- Make it less intrusive with 'if/else if' clauses
- Change the logic of 'err' variable for better readability of code
- Remove one-liner if brackets that broke coding sytle
drivers/watchdog/watchdog_dev.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index ef8edec..8e0f1b8 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -463,15 +463,17 @@ out:
static int watchdog_release(struct inode *inode, struct file *file)
{
struct watchdog_device *wdd = file-private_data;
- int err = -EBUSY;
+ int err = 0;
/*
* We only stop the watchdog if we received the magic character
* or if WDIOF_MAGICCLOSE is not set. If nowayout was set then
* watchdog_stop will fail.
*/
- if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
- !(wdd-info-options WDIOF_MAGICCLOSE))
+ if (test_bit(WDOG_ACTIVE, wdd-status))
+ err = -EBUSY;
+ else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) ||
+ !(wdd-info-options WDIOF_MAGICCLOSE))
err = watchdog_stop(wdd);
Looking at it again, it is now broken and fails with EBUSY if WDOG_ACTIVE is
set, and it tries to stop the watchdog if it is already stopped.
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [v5] clk: add si5351 i2c common clock driver
On Mon, Apr 08, 2013 at 08:11:38AM +0200, Sebastian Hesselbarth wrote:
On 04/08/2013 02:17 AM, Guenter Roeck wrote:
On Mon, Apr 08, 2013 at 01:49:24AM +0200, Sebastian Hesselbarth wrote:
On 04/08/2013 12:50 AM, Guenter Roeck wrote:
On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver supports
DT kernels only and VXCO feature of si5351b is not implemented. DT
bindings selectively allow to overwrite stored Si5351 configuration
which is very helpful for clock generators with empty eeprom
configuration. Corresponding device tree binding documentation is
also added.
Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
Tested-by: Daniel Mackzon...@gmail.com
[ ... ]
+static inline void _si5351_msynth_set_pll_master(
+ struct si5351_driver_data *drvdata, unsigned char num, int is_master)
+{
+ unsigned long flags;
+
+ if (num 8 ||
+ (drvdata-variant == SI5351_VARIANT_A3 num 3))
+ return;
+
+ flags = __clk_get_flags(drvdata-msynth[num].hw.clk);
+ if (is_master)
+ flags |= CLK_SET_RATE_PARENT;
+ else
+ flags= ~CLK_SET_RATE_PARENT;
+ __clk_set_flags(drvdata-msynth[num].hw.clk, flags);
+}
+
Unless I am missing something, neither __clk_get_flags() nor the new
__clk_set_flags is exported.
Did you try to build and load the driver as module ?
Well, good catch. I didn't try to build v5 as a module, but I guess it
will fail. But I consider this as something that has to be addressed in
clk framework itself, not in this patch. There will be other
clk-providers built as module in the future for sure.
Sure, but you provided the patch to make __clk_set_flags global. To avoid
build failures, I would suggest to either submit a patch to export the
missing functions, or to remove the ability to build the driver as module.
Actually, I knew that __clk_set_flags patch will not be accepted
before posting it ;)
Ah, but part of that is to get you to think about it again, and to defend it if
it is really needed. After all, it can be abused applies to pretty much every
API.
Key question is if you _really_ need run-time flag modifications, or if you can
live with initialization-time settings. If you really need it, you'll have to
explain the reasons.
On a side note, do you happen to know anyone working on drivers for Si5319 or
Si5368 ?
No.
Too bad ... I may have to write that code myself then.
Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Mon, Apr 08, 2013 at 08:48:58AM -0400, Don Zickus wrote:
On Mon, Apr 08, 2013 at 01:46:58PM +0800, Dave Young wrote:
On 04/06/2013 04:16 AM, Don Zickus wrote:
A common problem with kdump is that during the boot up of the
second kernel, the hardware watchdog times out and reboots the
machine before a vmcore can be captured.
Instead of tellling customers to disable their hardware watchdog
timers, I hacked up a hook to put in the kdump path that provides
one last kick before jumping into the second kernel.
The assumption is the watchdog timeout is at least 10-30 seconds
long, enough to get the second kernel to userspace to kick the watchdog
again, if needed.
For kdump kernel some devices need to reset, this might increase the
boot time, it's not so reliable for the 10-30s for us to kicking the
watchdog.
Could we have another option to disable/stop the watchdog while panic
happens? Ie. add a kernel cmdline panic_stop_wd=0|1 for 1st kernel, if
it's set to 1, then just stop the watchdog or we can kick the watchdog
like what you do in this patch. Of course stopping watchdog should be
lockless as well..
Hmm, I can look into that. But I am not sure all watchdogs have the
ability to stop once started. I was also worried about the case where
Correct.
kdump hangs for some reason. Having the watchdog there to 'reboot' would
be a nice safety net.
Absolutely agree. After all, the reason for the kdump is most likely that
something went really wrong, meaning there is some likelyhood for the hang
to occur. Turning off the watchdog in this condition does not seem to be
a good idea.
Perhaps adjusting the watchdog 'timeout' to something like 3 minutes would
be easier?
Not all watchdogs support such large timeouts, unfortunately. Maybe it would
make sense to implement infrastructure support for a softdog on top of the
hardware watchdog. Several drivers implement that outside the infrastructure
already.
Guenter
I'll wait on feedback from the watchdog community to help point me in a
good direction.
Cheers,
Don
Of course kdump is usually executed on a panic path, so grabbing the
watchdog mutexes to communicate with the hardware won't work. For now,
I do everything locklessly.
I can attempt a 'mutex_trylock' but not sure what to do in the failure
case? spin up to a second?
This patch is more of a proof of concept right now. Hopefully feedback
will help solve this problem better.
I have tested this with a machine using iTCO_wdt and the 'watchdog' app.
The extra kicked happened as expected.
Signed-off-by: Don Zickus dzic...@redhat.com
---
drivers/watchdog/watchdog_dev.c | 35
+++
include/linux/watchdog.h|7 +++
kernel/kexec.c |6 ++
3 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c
b/drivers/watchdog/watchdog_dev.c
index 08b48bb..6393572 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -84,6 +84,41 @@ out_ping:
}
/*
+ * watchdog_kick_for_kdump: kick the watchdog in the kdump path
+ *
+ * The kdump path needs time to reboot the kernel back into
+ * userspace. This window is big enough the hardware watchdog
+ * may come in and reboot the box. This hook gives the watchdog
+ * one final kick to get kdump over the hump.
+ *
+ * We don't know what devices are open, so just use the legacy
+ * interface for now. We have to do this locklessly as we can
+ * not wait.
+ */
+
+void watchdog_kick_for_kdump(void)
+{
+ struct watchdog_device *wddev = old_wdd;
+
+ /* FIXME - Perhaps use a mutex_trylock? */
+
+ if (!wddev)
+ return;
+
+ if (test_bit(WDOG_UNREGISTERED, wddev-status))
+ return;
+
+ if (!watchdog_active(wddev))
+ return;
+
+ if (wddev-ops-ping)
+ wddev-ops-ping(wddev); /* ping the watchdog */
+ else
+ wddev-ops-start(wddev); /* restart watchdog */
+}
+EXPORT_SYMBOL_GPL(watchdog_kick_for_kdump);
+
+/*
* watchdog_start: wrapper to start the watchdog.
* @wddev: the watchdog device to start
*
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 2a3038e..5dff975 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -142,4 +142,11 @@ extern int watchdog_init_timeout(struct
watchdog_device *wdd,
extern int watchdog_register_device(struct watchdog_device *);
extern void watchdog_unregister_device(struct watchdog_device *);
+#ifdef CONFIG_WATCHDOG_CORE
+/* drivers/watchdog/watchdog_dev.c */
+extern void watchdog_kick_for_kdump(void);
+#else
+static inline void watchdog_kick_for_kdump(void)
Re: [PATCH v3] watchdog: core: don't try to stop device if not running
On Mon, Apr 08, 2013 at 05:06:32PM +0200, Hector Palacios wrote: A watchdog device may be stopped from userspace using WDIOC_SETOPTIONS ioctl and flag WDIOS_DISABLECARD. If the device is closed after this operation, watchdog_release() is called and status bits checked for stopping it. Besides, if the device has not been unregistered a critical message watchdog did not stop! is printed, although the ioctl may have successfully stopped it already. Without the patch a user application sample code like this will successfully stop the watchdog, but the kernel will output the message watchdog did not stop!: wd_fd = open(/dev/watchdog, O_RDWR); flags = WDIOS_DISABLECARD; ioctl(wd_fd, WDIOC_SETOPTIONS, flags); close(wd_fd); Signed-off-by: Hector Palacios hector.palac...@digi.com Reviewed-by: Guenter Roeck li...@roeck-us.net --- Changes from v1: - Make it less intrusive with 'if/else if' clauses. - Change the logic of 'err' variable for better readability of code. - Remove one-liner if brackets that broke coding sytle. Changes from v2: - Revert Change the logic of 'err'... from v1 because otherwise the 'else if' can't be reached if the watchdog is running. drivers/watchdog/watchdog_dev.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index ef8edec..6117c48 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -470,8 +470,10 @@ static int watchdog_release(struct inode *inode, struct file *file) * or if WDIOF_MAGICCLOSE is not set. If nowayout was set then * watchdog_stop will fail. */ - if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) || - !(wdd-info-options WDIOF_MAGICCLOSE)) + if (!test_bit(WDOG_ACTIVE, wdd-status)) + err = 0; + else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, wdd-status) || + !(wdd-info-options WDIOF_MAGICCLOSE)) err = watchdog_stop(wdd); /* If the watchdog was not stopped, send a keepalive ping */ -- 1.8.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6] clk: add si5351 i2c common clock driver
On Mon, Apr 08, 2013 at 06:46:57PM +0200, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver does not
support VXCO feature of si5351b. Passing platform_data or DT bindings
selectively allow to overwrite stored Si5351 configuration which is
very helpful for clock generators with empty eeprom configuration.
Corresponding device tree binding documentation is also added.
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
[ ... ]
+
+/*
+ * Si5351 i2c probe and DT
+ */
+#ifdef CONFIG_OF
+static const struct of_device_id si5351_dt_ids[] = {
+ { .compatible = silabs,si5351a, .data = (void *)SI5351_VARIANT_A, },
+ { .compatible = silabs,si5351a-msop,
+ .data = (void *)SI5351_VARIANT_A3, },
+ { .compatible = silabs,si5351b, .data = (void *)SI5351_VARIANT_B, },
+ { .compatible = silabs,si5351c, .data = (void *)SI5351_VARIANT_C, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, si5351_dt_ids);
+
+static int si5351_dt_parse(struct i2c_client *client)
+{
+ struct device_node *child, *np = client-dev.of_node;
+ struct si5351_platform_data *pdata;
+ const struct of_device_id *match;
+ struct property *prop;
+ const __be32 *p;
+ int num = 0;
+ u32 val;
+
+ if (np == NULL)
+ return 0;
+
+ match = of_match_node(si5351_dt_ids, np);
+ if (match == NULL)
+ return -EINVAL;
+
+ pdata = devm_kzalloc(client-dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ pdata-variant = (enum si5351_variant)match-data;
+ pdata-clk_xtal = of_clk_get(np, 0);
+ if (!IS_ERR(pdata-clk_xtal))
+ clk_put(pdata-clk_xtal);
+ pdata-clk_clkin = of_clk_get(np, 1);
+ if (!IS_ERR(pdata-clk_clkin))
+ clk_put(pdata-clk_clkin);
+
+ /*
+ * property silabs,pll-source : num src, [..]
+ * allow to selectively set pll source
+ */
+ of_property_for_each_u32(np, silabs,pll-source, prop, p, num) {
+ if (num = 2) {
+ dev_err(client-dev,
+ invalid pll %d on pll-source prop\n, num);
+ break;
You report dev_err here, but you don't return an error to the caller.
Is this on purpose ? If it is not a fatal error, maybe it should be dev_info ?
+ }
+
+ p = of_prop_next_u32(prop, p, val);
+ if (!p)
+ break;
+
+ switch (val) {
+ case 0:
+ pdata-pll_src[num] = SI5351_PLL_SRC_XTAL;
+ break;
+ case 1:
+ pdata-pll_src[num] = SI5351_PLL_SRC_CLKIN;
+ break;
+ default:
+ dev_warn(client-dev,
+ invalid parent %d for pll %d\n, val, num);
+ continue;
Same here, and a couple of times below. Given the context, I think it would
be better if the error cases would return an error. After all, the condition
suggests that the device tree data is wrong, meaning one has to assume it
won't work anyway and should be fixed in the device tree and not be ignored
by the driver.
+ }
+ }
+
+ /* per clkout properties */
+ num = of_get_child_count(np);
+
+ if (num == 0)
+ return 0;
+
This doesn't set client-dev.platform_data yet returns no error, meaning the
calling code will either use provided platform data or fail after all if it is
NULL. That seems to be inconsistent, given that a dt entry was already detected.
The user might end up wondering why the provided device tree data is not used,
not realizing that it is incomplete.
If children are not mandatory, you could just drop the code above and go through
for_each_child_of_node() anyway; it won't do anything and set
client-dev.platform_data at the end. If children are mandatory, it might make
sense to return an eror here (if there is dt information, it should be complete
and consistent).
+ for_each_child_of_node(np, child) {
+ if (of_property_read_u32(child, reg, num)) {
+ dev_err(client-dev, missing reg property of %s\n,
+ child-name);
+ continue;
+ }
+
What if num returns 100 ? Or 1000 ?
+ if (!of_property_read_u32(child, silabs,multisynth-source,
+ val)) {
+ switch (val) {
+ case 0:
+ pdata-clkout[num].multisynth_src =
+ SI5351_MULTISYNTH_SRC_VCO0;
+ break;
+ case 1:
+ pdata-clkout[num].multisynth_src =
+
Re: mfd: Core driver for Winbond chips
On Tue, Apr 09, 2013 at 11:37:01AM +0200, Samuel Ortiz wrote: Hi Guenter, On Sat, Mar 23, 2013 at 10:49:14AM -0700, Guenter Roeck wrote: MFD core driver for various variants of Winbond/Nuvoton SuperIO chips. Signed-off-by: Guenter Roeck li...@roeck-us.net --- drivers/mfd/Kconfig | 22 +++ drivers/mfd/Makefile |1 + drivers/mfd/w83627hf-core.c | 324 ++ include/linux/mfd/w83627hf.h | 131 + 4 files changed, 478 insertions(+) create mode 100644 drivers/mfd/w83627hf-core.c create mode 100644 include/linux/mfd/w83627hf.h This driver looks overall good to me. Is this your final version, or should I expect any follow up patch ? Hi Samuel, I was waiting for feedback from Wim, who submitted a similar driver, about his thoughts. Key question is how to reserve access to the shared resource - either through an exported function in the mfd driver requesting a mutex, or through request_muxed_region(). I am going back and forth myself on which one is better. Maybe it does not really matter, but using a function has the slight advantage that it auto-loads and locks the mfd module while one of its client modules is loaded. If we use request_muxed_region, that is not the case and the client module must use another means to request and lock the mfd module. Maybe you have an opinion ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Tue, Apr 09, 2013 at 10:44:31AM -0400, Don Zickus wrote: On Mon, Apr 08, 2013 at 08:15:09AM -0700, Guenter Roeck wrote: On Mon, Apr 08, 2013 at 08:48:58AM -0400, Don Zickus wrote: On Mon, Apr 08, 2013 at 01:46:58PM +0800, Dave Young wrote: On 04/06/2013 04:16 AM, Don Zickus wrote: A common problem with kdump is that during the boot up of the second kernel, the hardware watchdog times out and reboots the machine before a vmcore can be captured. Instead of tellling customers to disable their hardware watchdog timers, I hacked up a hook to put in the kdump path that provides one last kick before jumping into the second kernel. The assumption is the watchdog timeout is at least 10-30 seconds long, enough to get the second kernel to userspace to kick the watchdog again, if needed. For kdump kernel some devices need to reset, this might increase the boot time, it's not so reliable for the 10-30s for us to kicking the watchdog. Could we have another option to disable/stop the watchdog while panic happens? Ie. add a kernel cmdline panic_stop_wd=0|1 for 1st kernel, if it's set to 1, then just stop the watchdog or we can kick the watchdog like what you do in this patch. Of course stopping watchdog should be lockless as well.. Hmm, I can look into that. But I am not sure all watchdogs have the ability to stop once started. I was also worried about the case where Correct. kdump hangs for some reason. Having the watchdog there to 'reboot' would be a nice safety net. Absolutely agree. After all, the reason for the kdump is most likely that something went really wrong, meaning there is some likelyhood for the hang to occur. Turning off the watchdog in this condition does not seem to be a good idea. Perhaps adjusting the watchdog 'timeout' to something like 3 minutes would be easier? Not all watchdogs support such large timeouts, unfortunately. Maybe it would make sense to implement infrastructure support for a softdog on top of the hardware watchdog. Several drivers implement that outside the infrastructure already. Hi Guenter, I am not familar with a softdog. Can you give me an example of how it works? Just look for the use of mod_timer in the watchdog directory. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Tue, Apr 09, 2013 at 11:14:23AM -0400, Don Zickus wrote: On Tue, Apr 09, 2013 at 07:52:28AM -0700, Guenter Roeck wrote: On Tue, Apr 09, 2013 at 10:44:31AM -0400, Don Zickus wrote: On Mon, Apr 08, 2013 at 08:15:09AM -0700, Guenter Roeck wrote: On Mon, Apr 08, 2013 at 08:48:58AM -0400, Don Zickus wrote: On Mon, Apr 08, 2013 at 01:46:58PM +0800, Dave Young wrote: On 04/06/2013 04:16 AM, Don Zickus wrote: A common problem with kdump is that during the boot up of the second kernel, the hardware watchdog times out and reboots the machine before a vmcore can be captured. Instead of tellling customers to disable their hardware watchdog timers, I hacked up a hook to put in the kdump path that provides one last kick before jumping into the second kernel. The assumption is the watchdog timeout is at least 10-30 seconds long, enough to get the second kernel to userspace to kick the watchdog again, if needed. For kdump kernel some devices need to reset, this might increase the boot time, it's not so reliable for the 10-30s for us to kicking the watchdog. Could we have another option to disable/stop the watchdog while panic happens? Ie. add a kernel cmdline panic_stop_wd=0|1 for 1st kernel, if it's set to 1, then just stop the watchdog or we can kick the watchdog like what you do in this patch. Of course stopping watchdog should be lockless as well.. Hmm, I can look into that. But I am not sure all watchdogs have the ability to stop once started. I was also worried about the case where Correct. kdump hangs for some reason. Having the watchdog there to 'reboot' would be a nice safety net. Absolutely agree. After all, the reason for the kdump is most likely that something went really wrong, meaning there is some likelyhood for the hang to occur. Turning off the watchdog in this condition does not seem to be a good idea. Perhaps adjusting the watchdog 'timeout' to something like 3 minutes would be easier? Not all watchdogs support such large timeouts, unfortunately. Maybe it would make sense to implement infrastructure support for a softdog on top of the hardware watchdog. Several drivers implement that outside the infrastructure already. Hi Guenter, I am not familar with a softdog. Can you give me an example of how it works? Just look for the use of mod_timer in the watchdog directory. So looking at the mod_timer logic in various drivers, it seems regardless if the /dev/watchdog device is opened or not, if it is running, it will automagically kick the watchdog. yes This seems that we can avoid pulling in userspace pieces for this. Just load the driver and the hardware starts getting kicked. Only if it is already running. Also, you don't want to rely on it, because you lose protection against user space issues. A second use is if the hw watchdog needs to be pinged more often than user space can provide. Some of the HW watchdogs need a ping in one-second intervals or even faster. Is that true? And if so, do all drivers detect if the hardware is already running during their init? Or is it based on the first device open? It is usually done in the probe function. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mfd: Core driver for Winbond chips
On Tue, Apr 09, 2013 at 01:45:44PM +0200, Wim Van Sebroeck wrote: Hi Guenter, I was waiting for feedback from Wim, who submitted a similar driver, about his thoughts. Key question is how to reserve access to the shared resource - either through an exported function in the mfd driver requesting a mutex, or through request_muxed_region(). I am going back and forth myself on which one is better. Maybe it does not really matter, but using a function has the slight advantage that it auto-loads and locks the mfd module while one of its client modules is loaded. If we use request_muxed_region, that is not the case and the client module must use another means to request and lock the mfd module. Maybe you have an opinion ? This is indeed the main issue that has to be solved. Both options will work. I like the auto-load and lock, but I need to look at the request_muxed_region code again first before I can see what the possible drawbacks are :-). One drawback of using request_muxed_region is that it needs a return value from superio_enter. Also, it needs some code in the client driver init function to ensure that the mfd driver gets loaded, and possibly a call to __module_get() in the client driver probe function to keep the mfd driver loaded. winbond_superio_enter() would not need a return value and could use devm_request_region. We could also consider allocating the hwmon memory space in the mfd driver and pass it as resource to the client drivers, which would remove a few more lines of code from those. Overall I am slightly in favor of using an exported function. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] gpio: Kontron PLD gpio driver
On Tue, Apr 09, 2013 at 10:46:15AM +0200, Linus Walleij wrote: On Mon, Apr 8, 2013 at 7:15 PM, Kevin Strasser kevin.stras...@linux.intel.com wrote: From: Michael Brunner michael.brun...@kontron.com Add gpio support for the on-board PLD found on some Kontron embedded modules. Signed-off-by: Michael Brunner michael.brun...@kontron.com Signed-off-by: Kevin Strasser kevin.stras...@linux.intel.com This looks very generic, setting and clearing bits in bytesized registers. Can you please attempt to use generic GPIO for this? Linus, I looked into it, but for my part I seem to be missing how the generic GPIO code permits locking access to the hardware (PLD) and setting the PLD's page register. In other words, I don't immediately see how to call kempld_get_mutex_set_index() from the generic GPIO code. The other drivers using generic GPIO code don't seem to have that requirement. Thanks, Guenter drivers/gpio/gpio-generic.c linux/basic_mmio_gpio.h See for example: gpio-ep93xx.c, gpio-sodaville.c ... Since you don't even have IRQ support in this it will be even simpler. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-watchdog in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mfd: Core driver for Winbond chips
On Tue, Apr 09, 2013 at 07:31:15PM +0200, Wim Van Sebroeck wrote: Hi Guenter, I was waiting for feedback from Wim, who submitted a similar driver, about his thoughts. Key question is how to reserve access to the shared resource - either through an exported function in the mfd driver requesting a mutex, or through request_muxed_region(). I am going back and forth myself on which one is better. Maybe it does not really matter, but using a function has the slight advantage that it auto-loads and locks the mfd module while one of its client modules is loaded. If we use request_muxed_region, that is not the case and the client module must use another means to request and lock the mfd module. Maybe you have an opinion ? This is indeed the main issue that has to be solved. Both options will work. I like the auto-load and lock, but I need to look at the request_muxed_region code again first before I can see what the possible drawbacks are :-). One drawback of using request_muxed_region is that it needs a return value from superio_enter. Also, it needs some code in the client driver init function to ensure that the mfd driver gets loaded, and possibly a call to __module_get() in the client driver probe function to keep the mfd driver loaded. winbond_superio_enter() would not need a return value and could use devm_request_region. We could also consider allocating the hwmon memory space in the mfd driver and pass it as resource to the client drivers, which would remove a few more lines of code from those. Overall I am slightly in favor of using an exported function. I looked at commit 8b6d043b7ee2d1b819dc833d677ea2aead71a0c0 (which implements request_muxed_region). You indeed need some extra code for loading the lowl-level mfd driver. So I am also in favour of the exported function. So which way should we go ? Take your driver as a starting point or mine ? One thing I'll want to add is support for both superio regions, as I have a use case for it. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Wed, Apr 10, 2013 at 09:40:39AM -0400, Don Zickus wrote: On Tue, Apr 09, 2013 at 09:07:58AM -0700, Guenter Roeck wrote: Just look for the use of mod_timer in the watchdog directory. So looking at the mod_timer logic in various drivers, it seems regardless if the /dev/watchdog device is opened or not, if it is running, it will automagically kick the watchdog. yes This seems that we can avoid pulling in userspace pieces for this. Just load the driver and the hardware starts getting kicked. Only if it is already running. Also, you don't want to rely on it, because you lose protection against user space issues. IOW if something goes wrong with a runaway userspace app, the kernel blindly continues to kick the watchdog, which masks the problem, right? That would be wrong if any of the drivers does that. The kernel should stop kicking after the software timeout expires. For example, if the HW needs to be kicked every second, and the high level timeout is set to one minute, the driver should keep kicking the hardware watchdog for one minute and then stop doing it if /dev/watchdog was opened and userspace is silent. A second use is if the hw watchdog needs to be pinged more often than user space can provide. Some of the HW watchdogs need a ping in one-second intervals or even faster. Is that true? And if so, do all drivers detect if the hardware is already running during their init? Or is it based on the first device open? It is usually done in the probe function. Ok. Thanks for the understanding of how the softdog stuff works. However, we still have the problem that if the machine panics and we want to jump into the kdump kernel, we need to 'kick' the watchdog one more time. This provides us a sane sync point for determining how long we have to load the watchdog driver in the second kernel before the hardware reboots us. Otherwise the reboots are pretty random and nothing is guaranteed. Hence the need for some sort of patch resembling the one I posted. S, any thoughts about that patch and what changes I should make? :-) The FIXME is a problem, and I think the name and scope would have to be more generic (watchdog_kick ?). Also, it doesn't solve the problem of having multiple open watchdogs (my system has three, for example), and it doesn't check if the watchdog is running. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Wed, Apr 10, 2013 at 10:20:55AM -0400, Don Zickus wrote: On Wed, Apr 10, 2013 at 06:51:23AM -0700, Guenter Roeck wrote: On Wed, Apr 10, 2013 at 09:40:39AM -0400, Don Zickus wrote: On Tue, Apr 09, 2013 at 09:07:58AM -0700, Guenter Roeck wrote: Just look for the use of mod_timer in the watchdog directory. So looking at the mod_timer logic in various drivers, it seems regardless if the /dev/watchdog device is opened or not, if it is running, it will automagically kick the watchdog. yes This seems that we can avoid pulling in userspace pieces for this. Just load the driver and the hardware starts getting kicked. Only if it is already running. Also, you don't want to rely on it, because you lose protection against user space issues. IOW if something goes wrong with a runaway userspace app, the kernel blindly continues to kick the watchdog, which masks the problem, right? That would be wrong if any of the drivers does that. The kernel should stop kicking after the software timeout expires. For example, if the HW needs to be kicked every second, and the high level timeout is set to one minute, the driver should keep kicking the hardware watchdog for one minute and then stop doing it if /dev/watchdog was opened and userspace is silent. Ah ok. A second use is if the hw watchdog needs to be pinged more often than user space can provide. Some of the HW watchdogs need a ping in one-second intervals or even faster. Is that true? And if so, do all drivers detect if the hardware is already running during their init? Or is it based on the first device open? It is usually done in the probe function. Ok. Thanks for the understanding of how the softdog stuff works. However, we still have the problem that if the machine panics and we want to jump into the kdump kernel, we need to 'kick' the watchdog one more time. This provides us a sane sync point for determining how long we have to load the watchdog driver in the second kernel before the hardware reboots us. Otherwise the reboots are pretty random and nothing is guaranteed. Hence the need for some sort of patch resembling the one I posted. S, any thoughts about that patch and what changes I should make? :-) The FIXME is a problem, and I think the name and scope would have to be more generic (watchdog_kick ?). Also, it doesn't solve the problem of having multiple open watchdogs (my system has three, for example), and it doesn't check if the watchdog is running. Ok. I didn't know the watchdog subsystem well enough, so I just took stabs in the dark about how things should work. I appreciate the feedback. I could make the name more generic. I wasn't sure if the watchdog community would frown on that. The FIXME is a problem, I am not sure how I don't know what others think, but I would prefer a more generic name. That its current use is for kdump is besides the point; there could be other valid uses. to handle the 'fail' scenario (can't get the mutex with trylock). And I I would just return without doing anything. After all, if the mutex is held, it is possible if not likely that the code crashed _in_ the watchdog code, so it might be better to just let it crash and burn in that case. have no idea how to even find out if multiple watchdogs are open on the system. Is there a list I could walk? And with regard to 'watchdog is /* the dev_t structure to store the dynamically allocated watchdog devices */ static dev_t watchdog_devt; One way to look up the allocated watchdogs might be to loop through all kobj instances for the major device using kobj_lookup. Don't know if there is a better way. running', I thought 'watchdog_active' would do that. But again, I could be misreading the code. You are right. Missed that part, sorry. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Wed, Apr 10, 2013 at 12:17:39PM -0400, Don Zickus wrote: On Wed, Apr 10, 2013 at 08:10:41AM -0700, Guenter Roeck wrote: On Wed, Apr 10, 2013 at 10:20:55AM -0400, Don Zickus wrote: On Wed, Apr 10, 2013 at 06:51:23AM -0700, Guenter Roeck wrote: On Wed, Apr 10, 2013 at 09:40:39AM -0400, Don Zickus wrote: On Tue, Apr 09, 2013 at 09:07:58AM -0700, Guenter Roeck wrote: Just look for the use of mod_timer in the watchdog directory. So looking at the mod_timer logic in various drivers, it seems regardless if the /dev/watchdog device is opened or not, if it is running, it will automagically kick the watchdog. yes This seems that we can avoid pulling in userspace pieces for this. Just load the driver and the hardware starts getting kicked. Only if it is already running. Also, you don't want to rely on it, because you lose protection against user space issues. IOW if something goes wrong with a runaway userspace app, the kernel blindly continues to kick the watchdog, which masks the problem, right? That would be wrong if any of the drivers does that. The kernel should stop kicking after the software timeout expires. For example, if the HW needs to be kicked every second, and the high level timeout is set to one minute, the driver should keep kicking the hardware watchdog for one minute and then stop doing it if /dev/watchdog was opened and userspace is silent. Ah ok. A second use is if the hw watchdog needs to be pinged more often than user space can provide. Some of the HW watchdogs need a ping in one-second intervals or even faster. Is that true? And if so, do all drivers detect if the hardware is already running during their init? Or is it based on the first device open? It is usually done in the probe function. Ok. Thanks for the understanding of how the softdog stuff works. However, we still have the problem that if the machine panics and we want to jump into the kdump kernel, we need to 'kick' the watchdog one more time. This provides us a sane sync point for determining how long we have to load the watchdog driver in the second kernel before the hardware reboots us. Otherwise the reboots are pretty random and nothing is guaranteed. Hence the need for some sort of patch resembling the one I posted. S, any thoughts about that patch and what changes I should make? :-) The FIXME is a problem, and I think the name and scope would have to be more generic (watchdog_kick ?). Also, it doesn't solve the problem of having multiple open watchdogs (my system has three, for example), and it doesn't check if the watchdog is running. Ok. I didn't know the watchdog subsystem well enough, so I just took stabs in the dark about how things should work. I appreciate the feedback. I could make the name more generic. I wasn't sure if the watchdog community would frown on that. The FIXME is a problem, I am not sure how I don't know what others think, but I would prefer a more generic name. That its current use is for kdump is besides the point; there could be other valid uses. Ok. to handle the 'fail' scenario (can't get the mutex with trylock). And I I would just return without doing anything. After all, if the mutex is held, it is possible if not likely that the code crashed _in_ the watchdog code, so it might be better to just let it crash and burn in that case. Hmm. Interesting point. So you are implying most watchdog mutex locks surround quickly executed code that are not called often. And if we do catch the mutex being held it is either because the watchdog code has failed (bad) or just bad timing :-). Hopefully all locks are for short periods of time ;). Since the lock is per device, it would either mean that the code has crashed there, or that some other entity is busy dealing with the device, presumably trying to ping it. Either case, I think the best (safest) way to deal with the situation would be to ignore it. For now I can just do the trylock and return on failure, printing a big fat warning for post-debug purposes. If it becomes a bigger problem, we can refine the approach later I guess. Ok. have no idea how to even find out if multiple watchdogs are open on the system. Is there a list I could walk? And with regard to 'watchdog is /* the dev_t structure to store the dynamically allocated watchdog devices */ static dev_t watchdog_devt; One way to look up the allocated watchdogs might be to loop through all kobj instances for the major device using kobj_lookup. Don't know
Re: [PATCH 4/4] watchdog: Kontron PLD watchdog timer
On Mon, Apr 08, 2013 at 10:15:21AM -0700, Kevin Strasser wrote: From: Michael Brunner michael.brun...@kontron.com Add watchdog timer support for the on-board PLD found on some Kontron embedded modules. Signed-off-by: Michael Brunner michael.brun...@kontron.com Signed-off-by: Kevin Strasser kevin.stras...@linux.intel.com Personally I would prefer two separate patches for the two drivers, and to have the drivers converted to the watchdog infrastructure. Wim's call, of course. Thanks, Guenter --- drivers/watchdog/Kconfig | 20 + drivers/watchdog/Makefile |2 + drivers/watchdog/kempld_now1_wdt.c | 602 +++ drivers/watchdog/kempld_wdt.c | 796 drivers/watchdog/kempld_wdt.h | 75 5 files changed, 1495 insertions(+) create mode 100644 drivers/watchdog/kempld_now1_wdt.c create mode 100644 drivers/watchdog/kempld_wdt.c create mode 100644 drivers/watchdog/kempld_wdt.h diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 9fcc70c..9ac71ca 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -687,6 +687,26 @@ config HP_WATCHDOG To compile this driver as a module, choose M here: the module will be called hpwdt. +config KEMPLD_WDT + tristate Kontron COM watchdog + depends on MFD_KEMPLD + help + Support for the PLD watchdog on some Kontron ETX and COMexpress + (ETXexpress) modules + + This driver can also be built as a module. If so, the module will be + called kempld_wdt. + +config KEMPLD_NOW1_WDT + tristate Kontron COMe-mSP1 (nanoETXexpress-SP) watchdog + depends on MFD_KEMPLD + help + Support for the PLD watchdog on the Kontron COMe-mSP1 + (nanoETXexpress-SP) module. + + This driver can also be built as a module. If so, the module will + be called kempld_now1_wdt. + config HPWDT_NMI_DECODING bool NMI decoding support for the HP ProLiant iLO2+ Hardware Watchdog Timer depends on HP_WATCHDOG diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index a300b94..a029930 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -90,6 +90,8 @@ endif obj-$(CONFIG_IT8712F_WDT) += it8712f_wdt.o obj-$(CONFIG_IT87_WDT) += it87_wdt.o obj-$(CONFIG_HP_WATCHDOG) += hpwdt.o +obj-$(CONFIG_KEMPLD_WDT) += kempld_wdt.o +obj-$(COnFIG_KEMPLD_NOW1_WDT) += kempld_now1_wdt.o obj-$(CONFIG_SC1200_WDT) += sc1200wdt.o obj-$(CONFIG_SCx200_WDT) += scx200_wdt.o obj-$(CONFIG_PC87413_WDT) += pc87413_wdt.o diff --git a/drivers/watchdog/kempld_now1_wdt.c b/drivers/watchdog/kempld_now1_wdt.c new file mode 100644 index 000..19b7272 --- /dev/null +++ b/drivers/watchdog/kempld_now1_wdt.c @@ -0,0 +1,602 @@ +/* + * kempld_now1_wdt.c - Kontron PLD watchdog driver for COMe-mSP1 + * + * Copyright (c) 2011-2012 Kontron Europe GmbH + * Author: Michael Brunner michael.brun...@kontron.com + * + * Note: The watchdog of the COMe-mSP1 (nanoETXexpress-SP) has one stage and + *only supports predefined watchdog timeout values. + * + *The supported timeouts are: + * 1 s, 5 s, 10 s, 30 s, 1 min, 5 min, 10 min, 15 min + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License 2 as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; see the file COPYING. If not, write to + * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include linux/module.h +#include linux/moduleparam.h +#include linux/types.h +#include linux/miscdevice.h +#include linux/watchdog.h +#include linux/fs.h +#include linux/ioport.h +#include linux/init.h +#include linux/uaccess.h +#include linux/slab.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/mfd/kempld.h + +#include kempld_wdt.h + +#define WATCHDOG_TIMEOUT 30 +static int timeout = WATCHDOG_TIMEOUT; +module_param(timeout, int, 0); +MODULE_PARM_DESC(timeout, + Watchdog timeout in seconds. (1, 5, 10, 30, 60, 300, 600, + 900, default= __MODULE_STRING(WATCHDOG_TIMEOUT) )); + +static bool nowayout = WATCHDOG_NOWAYOUT; +module_param(nowayout, bool, 0); +MODULE_PARM_DESC(nowayout, + Watchdog cannot be stopped once started (default= + __MODULE_STRING(WATCHDOG_NOWAYOUT) )); + +/* nanoETXexpress-SP watchdog register definitions */ +#define KEMPLD_WDT_NOW1
Re: [PATCH 2/4] i2c: Kontron PLD i2c bus driver
On Mon, Apr 08, 2013 at 10:15:19AM -0700, Kevin Strasser wrote:
From: Michael Brunner michael.brun...@kontron.com
Add i2c support for the on-board PLD found on some Kontron embedded
modules.
Signed-off-by: Michael Brunner michael.brun...@kontron.com
Signed-off-by: Kevin Strasser kevin.stras...@linux.intel.com
Overall well written, though I have a couple of nitpicks.
I would prefer two separate drivers, one for the mux and one for the i2c bus.
If that is possible, it would help getting rid of the #ifdef in the code, which
is frowned upon in the kernel.
I dislike unnecessary ( ). Maintainer's call, though.
Couple of places have missing spaces around operators (checkpatch doesn't catch
all those).
As far as I know, devm_ functions are supposed to print an error message on
failure, so it should be unnecessary to print another one if that happens (this
might need some confirmation).
Thanks,
Guenter
---
drivers/i2c/busses/Kconfig | 20 ++
drivers/i2c/busses/Makefile |1 +
drivers/i2c/busses/i2c-kempld.c | 679
+++
drivers/i2c/busses/i2c-kempld.h | 86 +
4 files changed, 786 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-kempld.c
create mode 100644 drivers/i2c/busses/i2c-kempld.h
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index adfee98..7aecd61 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -494,6 +494,26 @@ config I2C_IOP3XX
This driver can also be built as a module. If so, the module
will be called i2c-iop3xx.
+config I2C_KEMPLD
+ tristate Kontron COM I2C
+ depends on MFD_KEMPLD
+ help
+ This enables support for the I2C bus interface on some Kontron ETX
+ and COMexpress (ETXexpress) modules.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-kempld.
+
+config I2C_KEMPLD_MUX
+ bool Enable MUXed I2C ports (EXPERIMENTAL)
+ depends on I2C_KEMPLD I2C_MUX
+ default n
+ help
+ This enables support for additional I2C ports available on some
+ modules. Usually those ports are for board internal usage and
+ not routed outside the module.
+ Do not use this option unless you know what you are doing!
+
config I2C_MPC
tristate MPC107/824x/85xx/512x/52xx/83xx/86xx
depends on PPC
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 8f4fc23..411b8ce 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
obj-$(CONFIG_I2C_IMX)+= i2c-imx.o
obj-$(CONFIG_I2C_INTEL_MID) += i2c-intel-mid.o
obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
+obj-$(CONFIG_I2C_KEMPLD)+= i2c-kempld.o
obj-$(CONFIG_I2C_MPC)+= i2c-mpc.o
obj-$(CONFIG_I2C_MV64XXX)+= i2c-mv64xxx.o
obj-$(CONFIG_I2C_MXS)+= i2c-mxs.o
diff --git a/drivers/i2c/busses/i2c-kempld.c b/drivers/i2c/busses/i2c-kempld.c
new file mode 100644
index 000..c6b44e7
--- /dev/null
+++ b/drivers/i2c/busses/i2c-kempld.c
@@ -0,0 +1,679 @@
+/*
+ * i2c-kempld.c: I2C bus driver for Kontron COM modules
+ *
+ * Copyright (c) 2010-2013 Kontron Europe GmbH
+ * Author: Michael Brunner michael.brun...@kontron.com
+ *
+ * The driver is based on the i2c-ocores driver by Peter Korsgaard.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; see the file COPYING. If not, write to
+ * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/init.h
+#include linux/errno.h
+#include linux/platform_device.h
+#include linux/i2c.h
+#ifdef CONFIG_I2C_KEMPLD_MUX
+#include linux/i2c-mux.h
+#endif
+#include linux/delay.h
+#include linux/slab.h
+#include linux/mfd/kempld.h
+#include linux/interrupt.h
+#include linux/time.h
+
+#include i2c-kempld.h
+
+static int scl_frequency;
+static int i2c_bus = -1;
+static int i2c_mx_bus = -1;
+static bool force_polling;
+static int i2c_gpio_mux = -1;
+
+#ifdef CONFIG_I2C_KEMPLD_MUX
+static int kempld_i2cmux_select(struct i2c_adapter *adap, void *data, u32
chan)
+{
+ struct kempld_i2c_data *i2c = data;
+ struct kempld_device_data *pld = i2c-pld;
+ int ret = 0;
+
+ if ((i2c-state == STATE_DONE)
+ || (i2c-state ==
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Wed, Apr 10, 2013 at 12:49:14PM -0400, David Teigland wrote: On Wed, Apr 10, 2013 at 09:40:39AM -0400, Don Zickus wrote: However, we still have the problem that if the machine panics and we want to jump into the kdump kernel, we need to 'kick' the watchdog one more time. This provides us a sane sync point for determining how long we have to load the watchdog driver in the second kernel before the hardware reboots us. Otherwise the reboots are pretty random and nothing is guaranteed. Some time ago I submitted this patch http://www.spinics.net/lists/linux-watchdog/msg01477.html to get rid of the one extraneous ping that was causing me trouble. I'd still like to see merged, but haven't had time to follow up. The use case makes sense to me, so it gets my Ack. Did Wim ever comment on it ? Thanks, Guenter I have a use case where I need to guarantee that the watchdog will *not* be pinged unless my userland daemon does the ping. If my daemon is killed, the close() generates a ping that I don't intend. This kdump ping looks like it would be another instance that I'd need to suppress. Perhaps by renaming my flag WDOG_NO_EXTRA_PING and checking it both in release and in kick_for_kdump? (My daemon associates watchdog pings with shared storage heartbeats. Based on the heartbeats, hosts in a cluster can calculate when an unresponsive host last pinged its watchdog, and can be fairly certain that the dead host has been reset by its watchdog 60 seconds later. This is used as an alternative to i/o fencing where we're protecting data on shared storage from corruption after host failures. If there are uncontrolled watchdog pings, then hosts don't know when a dead host might have last pinged its watchdog, since it is no longer based on the last timestamp it wrote to shared storage.) Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt
On Mon, Apr 08, 2013 at 01:19:23PM -0700, John Stultz wrote: On 04/08/2013 05:47 AM, Prarit Bhargava wrote: 2nd submit: I did quite a bit of testing with this patch and I don't see any ill effects. I've tested across several large and small x86 systems, and a ppc system for good measure. P. 8--- ` The settimeofday01 test in the LTP testsuite effectively does gettimeofday(current time); settimeofday(Jan 1, 1970 + 100 seconds); settimeofday(current time); This test causes a stack trace to be displayed on the console during the setting of timeofday to Jan 1, 1970 + 100 seconds: [ 131.066751] [ cut here ] [ 131.096448] WARNING: at kernel/time/clockevents.c:209 clockevents_program_event+0x135/0x140() [ 131.104935] Hardware name: Dinar [ 131.108150] Modules linked in: sg nfsv3 nfs_acl nfsv4 auth_rpcgss nfs dns_resolver fscache lockd sunrpc nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6table_mangle ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat iptable_mangle ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables kvm_amd kvm sp5100_tco bnx2 i2c_piix4 crc32c_intel k10temp fam15h_power ghash_clmulni_intel amd64_edac_mod pcspkr serio_raw edac_mce_amd edac_core microcode xfs libcrc32c sr_mod sd_mod cdrom ata_generic crc_t10dif pata_acpi radeon i2c_algo_bit drm_kms_helper ttm drm ahci pata_atiixp libahci libata usb_storage i2c_core dm_mirror dm_region_hash dm_log dm_mod [ 131.176784] Pid: 0, comm: swapper/28 Not tainted 3.8.0+ #6 [ 131.182248] Call Trace: [ 131.184684] IRQ [810612af] warn_slowpath_common+0x7f/0xc0 [ 131.191312] [8106130a] warn_slowpath_null+0x1a/0x20 [ 131.197131] [810b9fd5] clockevents_program_event+0x135/0x140 [ 131.203721] [810bb584] tick_program_event+0x24/0x30 [ 131.209534] [81089ab1] hrtimer_interrupt+0x131/0x230 [ 131.215437] [814b9600] ? cpufreq_p4_target+0x130/0x130 [ 131.221509] [81619119] smp_apic_timer_interrupt+0x69/0x99 [ 131.227839] [8161805d] apic_timer_interrupt+0x6d/0x80 [ 131.233816] EOI [81099745] ? sched_clock_cpu+0xc5/0x120 [ 131.240267] [814b9ff0] ? cpuidle_wrap_enter+0x50/0xa0 [ 131.246252] [814b9fe9] ? cpuidle_wrap_enter+0x49/0xa0 [ 131.252238] [814ba050] cpuidle_enter_tk+0x10/0x20 [ 131.257877] [814b9c89] cpuidle_idle_call+0xa9/0x260 [ 131.263692] [8101c42f] cpu_idle+0xaf/0x120 [ 131.268727] [815f8971] start_secondary+0x255/0x257 [ 131.274449] ---[ end trace 1151a50552231615 ]--- When we change the system time to a low value like this, the value of timekeeper-offs_real will be a negative value. It seems that the WARN occurs because an hrtimer has been started in the time between the releasing of the timekeeper lock and the IPI call (via a call to on_each_cpu) in clock_was_set() in the do_settimeofday() code. The end result is that a REALTIME_CLOCK timer has been added with softexpires = expires = KTIME_MAX. The hrtimer_interrupt() fires/is called and the loop at kernel/hrtimer.c:1289 is executed. In this loop the code subtracts the clock base's offset (which was set to timekeeper-offs_real in do_settimeofday()) from the current hrtimer_cpu_base-expiry value (which was KTIME_MAX): KTIME_MAX - (a negative value) = overflow A simple check for an overflow can resolve this problem. Using KTIME_MAX instead of the overflow value will result in the hrtimer function being run, and the reprogramming of the timer after that. Signed-off-by: Prarit Bhargava pra...@redhat.com Cc: Thomas Gleixner t...@linutronix.de Cc: John Stultz john.stu...@linaro.org Prarit: Should this be tagged for -stable? Please. I am hitting this problem with 3.8.6 on powerpc. The patch fixes it for me. Tested-by: Guenter Roeck li...@roeck-us.net Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
This is the REGULATOR component driver of the Dialog DA9058 PMIC.
This driver is just one component of the whole DA9058 PMIC driver.
It depends on the CORE component driver of the DA9058 MFD.
There are 6 warnings from scripts/checkpatch.pl, but since it seems to
be complaining about variable names such as min_uV are in CamelCase,
when it is obvious that they are not in CamelCase I have ignored them.
??? min_uV _is_ CamelCase ???
Ok, maybe it is camelcasE, but you are splitting hairs here.
Guenter
Signed-off-by: Anthony Olech anthony.olech.opensou...@diasemi.com
Signed-off-by: David Dajun Chen david.c...@diasemi.com
---
drivers/regulator/Kconfig| 11 ++
drivers/regulator/Makefile |1 +
drivers/regulator/da9058-regulator.c | 228
++
3 files changed, 240 insertions(+)
create mode 100644 drivers/regulator/da9058-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index a5d97ea..3b7b098 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -64,6 +64,17 @@ config REGULATOR_USERSPACE_CONSUMER
If unsure, say no.
+config REGULATOR_DA9058
+ tristate Support regulators on Dialog Semiconductor DA9058 PMIC
+ depends on MFD_DA9058
+ help
+ Say y here to support the BUCKs and LDOs regulators found on
+ Dialog Semiconductor DA9058 PMIC.
+
+ This driver can also be built as a module. If so, the module
+ will be called da9058-regulator.
+
+
config REGULATOR_GPIO
tristate GPIO regulator support
depends on GENERIC_GPIO
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6e82503..f8e1784 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
obj-$(CONFIG_REGULATOR_FAN53555) += fan53555.o
obj-$(CONFIG_REGULATOR_GPIO) += gpio-regulator.o
obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
+obj-$(CONFIG_REGULATOR_DA9058) += da9058-regulator.o
obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
obj-$(CONFIG_REGULATOR_LP3972) += lp3972.o
obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o
diff --git a/drivers/regulator/da9058-regulator.c
b/drivers/regulator/da9058-regulator.c
new file mode 100644
index 000..a033364
--- /dev/null
+++ b/drivers/regulator/da9058-regulator.c
@@ -0,0 +1,228 @@
+/*
+ * Copyright (C) 2012 Dialog Semiconductor Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include linux/module.h
+#include linux/err.h
+#include linux/regulator/machine.h
+#include linux/regulator/driver.h
+#include linux/regmap.h
+#include linux/mfd/core.h
+
+#include linux/mfd/da9058/version.h
+#include linux/mfd/da9058/registers.h
+#include linux/mfd/da9058/core.h
+#include linux/mfd/da9058/regulator.h
+
+struct da9058_regulator {
+ struct da9058 *da9058;
+ int ramp_register;
+ int ramp_enable_mask;
+ struct platform_device *pdev;
+ struct regulator_dev *reg_dev;
+ struct regulator_desc desc;
+ struct regulator_init_data init;
+};
+
+static int da9058_buck_ramp_voltage(struct regulator_dev *rdev,
+ unsigned int old_selector,
+ unsigned int new_selector)
+{
+ struct da9058_regulator *regulator = rdev_get_drvdata(rdev);
+ struct da9058 *da9058 = regulator-da9058;
+ int ret;
+
+ if (regulator-ramp_register == 0)
+ return -EINVAL;
+
+ if (regulator-ramp_enable_mask == 0)
+ return -EINVAL;
+
+ ret = da9058_set_bits(da9058, regulator-ramp_register,
+ regulator-ramp_enable_mask);
+
+ if (ret)
+ return ret;
+
+ return 2200; /* micro Seconds needed to ramp to new voltage*/
+}
+
+static struct regulator_ops da9058_buck_regulator_ops = {
+ .map_voltage = regulator_map_voltage_linear,
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_time_sel = da9058_buck_ramp_voltage,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+static struct regulator_ops da9058_ldo_regulator_ops = {
+ .map_voltage = regulator_map_voltage_linear,
+ .list_voltage = regulator_list_voltage_linear,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .enable =
Re: [PATCH 2/2] watchdog: fix cleanup device code on registration
On Fri, Apr 12, 2013 at 06:51:48AM +, Kim, Milo wrote:
Duplicate lines of code are moved to cleanup_dev section.
And it returns 0 explicitly in case of no error.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/watchdog/watchdog_dev.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 42bfc9a..37c2dcc 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -613,11 +613,7 @@ int watchdog_dev_register(struct watchdog_device
*watchdog)
if (err) {
pr_err(watchdog%d unable to add device %d:%d\n,
watchdog-id, MAJOR(watchdog_devt), watchdog-id);
- if (watchdog-id == 0) {
- misc_deregister(watchdog_miscdev);
- old_wdd = NULL;
- }
- return err;
+ goto cleanup_dev;
}
/* Activate the watchdog automatically by the driver itself */
@@ -625,13 +621,17 @@ int watchdog_dev_register(struct watchdog_device
*watchdog)
err = watchdog_auto_start(watchdog);
if (err) {
cdev_del(watchdog-cdev);
If you are doing this, you should also move cdev_del into the cleanup path:
- if (watchdog-id == 0) {
- misc_deregister(watchdog_miscdev);
- old_wdd = NULL;
- }
+ goto cleanup_dev;
}
}
+ return 0;
+
cleanup_cdev:
cdev_del(watchdog-cdev);
+cleanup_dev:
+ if (watchdog-id == 0) {
+ misc_deregister(watchdog_miscdev);
+ old_wdd = NULL;
+ }
return err;
}
--
1.7.9.5
Best Regards,
Milo
--
To unsubscribe from this list: send the line unsubscribe linux-watchdog in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V4 6/7] DA9058 HWMON driver
On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote: This is the HWMON component driver of the Dialog DA9058 PMIC. This driver is just one component of the whole DA9058 PMIC driver. It depends on the CORE and ADC component drivers of the DA9058 MFD. Signed-off-by: Anthony Olech anthony.olech.opensou...@diasemi.com Signed-off-by: David Dajun Chen david.c...@diasemi.com --- No changelog ? That makes it unnecessarily difficult to review the driver, especially since a cursory glance suggests that you introduced new problems since V3. What is the point of calling devm_kfree() ? Why bother with devm_kzalloc() if you ignore its benefits anyway ? Anyway, I don't have time right now to re-review the entire thing as I'll have to pull my comments from the previous version, compare and figure out what you changed and what you didn't change. So this will have to wait a bit. Guenter Documentation/hwmon/da9058 | 38 + drivers/hwmon/Kconfig| 10 ++ drivers/hwmon/Makefile |3 +- drivers/hwmon/da9058-hwmon.c | 341 ++ 4 files changed, 391 insertions(+), 1 deletion(-) create mode 100644 Documentation/hwmon/da9058 create mode 100644 drivers/hwmon/da9058-hwmon.c diff --git a/Documentation/hwmon/da9058 b/Documentation/hwmon/da9058 new file mode 100644 index 000..841148f --- /dev/null +++ b/Documentation/hwmon/da9058 @@ -0,0 +1,38 @@ +Kernel driver da9058-hwmon +== + +Supported chips: + * Dialog Semiconductor DA9058 PMIC +Prefix: 'da9058' +Datasheet: + http://www.dialog-semiconductor.com/products/power-management/da9058 + +Authors: Opensource [Anthony Olech] anthony.olech.opensou...@diasemi.com + +Description +--- + +The DA9058 PMIC contains a 5 channel ADC which can be used to monitor a +range of system operating parameters, including the battery voltage and +temperature. The ADC measures voltage, but two of the ADC channels can +be configured to supply a current, so that if an NTC termister is connected +then the voltage reading can be converted to a temperature. Currently the +driver provides reporting of all the input values but does not provide any +alarms. + +Voltage Monitoring +-- + +Voltages are sampled in either 'automatic' or 'manual' mode, which is an +initialization parameter set in the platform data by the machine driver. +In manual mode the ADC conversion is 12 bit and in automatic mode it is +10 bit. However all the raw readings are reported as 12 bit numbers. + +Physical Limits +--- + +vbat 2500 - 4500 milliVolts +tbat 0- 2500 milliVolts +adc0- 2500 milliVolts +vfpin 0- 4095 milliVolts +tjunc there is a correction factor programmed during manufacturing diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 89ac1cb..9a3f226 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -304,6 +304,16 @@ config SENSORS_ATXP1 This driver can also be built as a module. If so, the module will be called atxp1. +config SENSORS_DA9058 + tristate Dialog Semiconductor DA9058 ADC + depends on MFD_DA9058 DA9058_ADC + help + If you say yes here you get support for the hardware monitoring + functionality of the Dialog Semiconductor DA9058 PMIC. + + This driver can also be built as a module. If so, the module + will be called da9058-hwmon. + config SENSORS_DS620 tristate Dallas Semiconductor DS620 depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 8d6d97e..c3b103b 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -44,7 +44,8 @@ obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o -obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o +obj-$(CONFIG_SENSORS_DA9055) += da9055-hwmon.o +obj-$(CONFIG_SENSORS_DA9058) += da9058-hwmon.o obj-$(CONFIG_SENSORS_DME1737)+= dme1737.o obj-$(CONFIG_SENSORS_DS620) += ds620.o obj-$(CONFIG_SENSORS_DS1621) += ds1621.o diff --git a/drivers/hwmon/da9058-hwmon.c b/drivers/hwmon/da9058-hwmon.c new file mode 100644 index 000..3859b16 --- /dev/null +++ b/drivers/hwmon/da9058-hwmon.c @@ -0,0 +1,341 @@ +/* + * Copyright (C) 2012 Dialog Semiconductor Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + */ + +#include linux/module.h +#include linux/err.h +#include linux/hwmon.h +#include linux/hwmon-sysfs.h +#include linux/regmap.h +#include linux/mfd/core.h + +#include linux/mfd/da9058/version.h +#include
Re: [PATCH 1/2] watchdog: introduce new watchdog AUTOSTART option
On Fri, Apr 12, 2013 at 06:51:47AM +, Kim, Milo wrote:
Conventionally, watchdog timer driver is activated by an user space request.
This patch enables an automatic start/ping operation of a watchdog without an
application.
(a) Work flow
If a watchdog timer driver is configured with WDIOF_AUTOSTART option,
then it starts and ping automatically on watchdog device registration.
A delayed workqueue invokes a ping operation of the watchdog periodically.
In the mean time, if an application tries to open the watchdog device,
it returns as -EBUSY because the watchdog is already running.
When the watchdog is unregistered, timed workqueue is canceled.
(b) Structure changes
ping_period : Period time for watchdog ping. Unit is millisecond.
work : Workqueue for automatic ping operation.
WDOG_AUTOSTARTED : Added status bit. Set when ping is done.
Cleared when period time is expired.
WDIOF_AUTOSTART : New option flag.
(c) Documentation
It describes how it works and how to use this feature in a watchdog driver.
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
I really don't like that idea. It defeats a significant part of the purpose
for having a watchdog, which is to prevent user-space hangups.
To make this a driver option is even more odd - it forces every user of this
driver to use it in-kernel only, and makes /dev/watchdog quite useless.
I mean, really, if you have such a watchdog, what is the point of using the
watchdog infrastructure in the first place ? Just make it a kernel thread or
timer-activated platform code which pings your watchdog once in a while. No
need to get the watchdog infrastructure involved in the first place.
Am I missing something ?
Thanks,
Guenter
---
Documentation/watchdog/watchdog-without-app.txt | 59
drivers/watchdog/watchdog_dev.c | 86
++-
include/linux/watchdog.h|6 ++
include/uapi/linux/watchdog.h |1 +
4 files changed, 151 insertions(+), 1 deletion(-)
create mode 100644 Documentation/watchdog/watchdog-without-app.txt
diff --git a/Documentation/watchdog/watchdog-without-app.txt
b/Documentation/watchdog/watchdog-without-app.txt
new file mode 100644
index 000..596a30a
--- /dev/null
+++ b/Documentation/watchdog/watchdog-without-app.txt
@@ -0,0 +1,59 @@
+The Linux WatchDog Timer Driver without an Application
+==
+
+Milo Kim milo@ti.com
+
+Introduction
+
+Linux WatchDog Timer (WDT) driver is activated by an user space request.
+If a WDT needs to be enabled alone, without an application, you can configure
+the WDT driver with 'AUTOSTART' option.
+
+How it works
+
+* Conventional watchdog system
+
+ (WDT driver) (WDT Application)
+ register-- WDT Core
+
+ .start -- WDT Dev /dev/watchdogN -- open
+ .ping .ioctl - keepalive
+ .stop .close
+ .
+ activated by an
application
+
+* Watchdog autostart feature
+
+ (WDT driver)
+ register-- WDT Core
+
+ .start -- WDT Dev /dev/watchdogN
+ .ping.
+.
+.
+start and ping periodically without an application
+
+How to use
+--
+1) Add 'WDIOF_AUTOSTART' option in your watchdog_info.
+
+static const struct watchdog_info foo_wdt_info = {
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_AUTOSTART,
+ .identity = Auto Watchdog Sample,
+};
+
+2) Define the value of 'ping_period' in your watchdog_device structure.
+ Unit is millisecond.
+
+static int foo_wdt_probe(struct platform_device *pdev)
+{
+ struct watchdog_device *foo_wdt;
+
+ foo_wdt-info = foo_wdt_info;
+ foo_wdt-ops = foo_wdt_ops;
+ foo_wdt-ping_period = 1000;/* kick a watchdog per 1 sec */
+
+ return watchdog_register_device(foo_wdt);
+}
+
+Then the foo watchdog is started and ping periodically without an
application.
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 08b48bb..42bfc9a 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -44,6 +44,8 @@
#include watchdog_core.h
+#define MAX_PING_PERIOD (60 * 60 * 1000)/* 1 hour */
+
/* the dev_t structure to store the dynamically allocated watchdog devices */
static dev_t watchdog_devt;
/* the watchdog device behind /dev/watchdog */
@@ -115,6 +117,70 @@ out_start:
return err;
}
+static
Re: [RFC PATCH] watchdog: Add hook for kicking in kdump path
On Fri, Apr 12, 2013 at 05:16:27PM -0400, Don Zickus wrote: On Wed, Apr 10, 2013 at 08:10:41AM -0700, Guenter Roeck wrote: have no idea how to even find out if multiple watchdogs are open on the system. Is there a list I could walk? And with regard to 'watchdog is /* the dev_t structure to store the dynamically allocated watchdog devices */ static dev_t watchdog_devt; One way to look up the allocated watchdogs might be to loop through all kobj instances for the major device using kobj_lookup. Don't know if there is a better way. Hmm, I got around to poking at this today and I am not sure kobj_lookup will work. Besides being surrounded with another mutex, I don't have access to the character device domain to pass to kobj_lookup. Perhaps I am not reading the code right, but I can't find a good way forward. The only other hack I can think of, is to embed a list object in the watchdog structure and list_add each new register'd watchdog. Then it would be trivial to walk the watchdog list. After looking into it again, I agree. Maybe you can give it a try. At least other options look even more complicated (eg creating a watchdog class ?). Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] documentation: hwmon: Fix typo in documentation/hwmon
On Sat, Apr 13, 2013 at 01:22:11AM +0900, Masanari Iida wrote: Correct spelling typo in Documentation/hwmon Signed-off-by: Masanari Iida standby2...@gmail.com --- Documentation/hwmon/adt7410 | 2 +- Documentation/hwmon/sht15 | 2 +- Documentation/hwmon/zl6100 | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) Partially applied to -next. adt7410 was already taken care of. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [NEW DRIVER V4 6/7] DA9058 HWMON driver
On Fri, Apr 12, 2013 at 05:53:48PM +0200, Lars-Peter Clausen wrote: On 04/12/2013 03:05 PM, Anthony Olech wrote: This is the HWMON component driver of the Dialog DA9058 PMIC. This driver is just one component of the whole DA9058 PMIC driver. It depends on the CORE and ADC component drivers of the DA9058 MFD. Signed-off-by: Anthony Olech anthony.olech.opensou...@diasemi.com Signed-off-by: David Dajun Chen david.c...@diasemi.com Hi, can't you use the generic IIO to HWMON bridge driver? And if not it's probably better to extent the bridge driver than writing this custom driver. I like that idea. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gcc4: Disable __compiletime_object_size for GCC 4.6+
__builtin_object_size is known to be broken on gcc 4.6+. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48880 for details. This causes unnecssary build warnings and errors such as In function 'copy_from_user', inlined from 'sb16_copy_from_user' at sound/oss/sb_audio.c:878:22: arch/x86/include/asm/uaccess_32.h:211:26: error: call to 'copy_from_user_overflow' declared with attribute error: copy_from_user() buffer size is not provably correct make[3]: [sound/oss/sb_audio.o] Error 1 (ignored) Disable it where broken. Signed-off-by: Guenter Roeck li...@roeck-us.net --- include/linux/compiler-gcc4.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h index 68b162d..842de22 100644 --- a/include/linux/compiler-gcc4.h +++ b/include/linux/compiler-gcc4.h @@ -13,7 +13,7 @@ #define __must_check __attribute__((warn_unused_result)) #define __compiler_offsetof(a,b) __builtin_offsetof(a,b) -#if GCC_VERSION = 40100 +#if GCC_VERSION = 40100 GCC_VERSION 40600 # define __compiletime_object_size(obj) __builtin_object_size(obj, 0) #endif -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL REQUEST] watchdog - v3.9-rc6 Fixes
On Sun, Apr 14, 2013 at 09:17:03AM +0200, Wim Van Sebroeck wrote: Hi Linus, Please pull from 'master' branch of git://www.linux-watchdog.org/linux-watchdog.git It will fix compile errors for teh at91rm9200_wdt driver. This will update the following files: Kconfig |2 +- 1 files changed, 1 insertion(+), 1 deletion(-) with these Changes: commit 09549cd01726a7ff8b102a93e46b059531583ab6 Author: Nicolas Ferre nicolas.fe...@atmel.com Date: Wed Apr 10 14:36:22 2013 +0200 Hi Wim, What is your take on watchdog: Fix race condition in registration code [1] ? Thanks, Guenter [1] http://www.spinics.net/lists/linux-watchdog/msg02291.html, https://patchwork.kernel.org/patch/2400801/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: gpio-ucb1400
On Fri, Mar 29, 2013 at 08:46:39PM +0100, Jean Delvare wrote: Hi all, In September 2009, a driver for the GPIO function of the UCB1400 chip was added to the kernel tree. The probe function of this driver requires ucbdata to be set. The only place where this happens is in function ucb1400_gpio_set_data(). This function was never call, and still isn't. So this is dead code for 3.5 years as far as the upstream kernel is concerned. To make things worse, this driver can't be built as a module, for no good reason that I can see. Marek, can you explain what was the point of submitting this driver that nobody can use? I would like either this driver to be fixed so that it can be used (and that would IMHO start with dropping the ugly ucb1400_gpio_set_data hook and global variable ucbdata), or this driver to be dropped from the kernel tree. If the driver is kept, it should be adjusted so that it can be built as a module. If I overlooked something, please let me know. Interestingly, the author made an attempt to fix that with [1]. It looks like the rest of that series was merged, but this patch wasn't, though I don't find any information about the reason. Guenter [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/028656.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: gpio-ucb1400
On Sat, Mar 30, 2013 at 08:20:44PM +0100, Marek Vasut wrote: Dear Guenter Roeck, On Fri, Mar 29, 2013 at 08:46:39PM +0100, Jean Delvare wrote: Hi all, In September 2009, a driver for the GPIO function of the UCB1400 chip was added to the kernel tree. The probe function of this driver requires ucbdata to be set. The only place where this happens is in function ucb1400_gpio_set_data(). This function was never call, and still isn't. So this is dead code for 3.5 years as far as the upstream kernel is concerned. To make things worse, this driver can't be built as a module, for no good reason that I can see. Marek, can you explain what was the point of submitting this driver that nobody can use? I would like either this driver to be fixed so that it can be used (and that would IMHO start with dropping the ugly ucb1400_gpio_set_data hook and global variable ucbdata), or this driver to be dropped from the kernel tree. If the driver is kept, it should be adjusted so that it can be built as a module. If I overlooked something, please let me know. Interestingly, the author made an attempt to fix that with [1]. It looks like the rest of that series was merged, but this patch wasn't, though I don't find any information about the reason. It's been a while. Guenter, thanks for finding that link, but I suspect the patch is heavily obsolete by now. Oh, it most definitely is, starting with the gpio driver name. Just wonder why it was never applied, and why no one seems to have noticed or cared. Jean is absolutely right - it should get fixed, or the driver should be dropped if no one is using it anyway. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 5/5] hwmon: add ST-Ericsson ABX500 hwmon driver
On Sat, Mar 30, 2013 at 04:18:18PM -0700, Anton Vorontsov wrote: On Wed, Mar 27, 2013 at 12:48:07PM -0700, Guenter Roeck wrote: On Wed, Mar 27, 2013 at 03:13:32PM +0800, Hongbo Zhang wrote: Each of ST-Ericsson X500 chip set series consists of both ABX500 and DBX500 chips. This is ABX500 hwmon driver, where the abx500.c is a common layer for all ABX500s, and the ab8500.c is specific for AB8500 chip. Under this designed structure, other chip specific files can be added simply using the same common layer abx500.c. Signed-off-by: Hongbo Zhang hongbo.zh...@linaro.org Reviewed-by: Guenter Roeck li...@roeck-us.net Thanks a lot for your reviews, Guenter! Guenter, Jean, As the driver depends on the battery driver changes, do you mind if I take it via battery-2.6.git tree? Ok with me. I actually assumed it would be merged through another tree because of its dependencies. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 2/2] watchdog: fix w83627hf_wdt reboot due to timeout expired
On Sat, Mar 30, 2013 at 06:59:08PM -0700, Tony Chung wrote:
Observed that the w83627hf watchdog timer start counting during reboot.
If the system load the driver after 5 minutes, it rebooted immediately
because of timer expired.
For example, fsck took more than 5 minutes to run, then reboot will occurred.
Signed-off-by: Tony Chung tonychun...@gmail.com
---
drivers/watchdog/w83627hf_wdt.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 8fd..47eb233 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -174,6 +174,11 @@ static void w83627hf_init(void)
outb_p(0xF7, WDT_EFER); /* Select CRF7 */
t = inb_p(WDT_EFDR); /* read CRF7 */
+ if (t 0x10) {
+ pr_info(Watchdog Timer timeout occurred!);
+ t = ~0x10; /* clear the event */
+ pr_info(Event cleared\n);
+ }
t = ~0xC0; /* disable keyboard mouse turning off
watchdog */
I would suggest to simply clear it silently. I don't think an extra message
provides any real value.
t = ~0xE0; /* clear watchdog, disable keyboard mouse
turning off watchdog */
Guenter
outb_p(t, WDT_EFDR);/* Write back to CRF7 */
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-watchdog in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 2/2] watchdog: fix w83627hf_wdt reboot due to timeout expired
On Sun, Mar 31, 2013 at 08:45:54AM -0700, Tony Chung wrote: How about this? - t = ~0xC0; /* disable keyboard mouse turning off - watchdog */ + t = ~0xE0; /* clear timeout occurred and disable keyboard + mouse turning off watchdog */ Ys, only make it 0xD0. Looks like I was fighting with binaries last night and lost. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] misc/vmw_vmci: VMWARE_VMCI depends on NET
Fix: ERROR: memcpy_toiovec [drivers/misc/vmw_vmci/vmw_vmci.ko] undefined! ERROR: memcpy_fromiovec [drivers/misc/vmw_vmci/vmw_vmci.ko] undefined! Both functions are defined in the core networking code. Signed-off-by: Guenter Roeck li...@roeck-us.net --- drivers/misc/vmw_vmci/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/vmw_vmci/Kconfig b/drivers/misc/vmw_vmci/Kconfig index 39c2eca..ea98f7e 100644 --- a/drivers/misc/vmw_vmci/Kconfig +++ b/drivers/misc/vmw_vmci/Kconfig @@ -4,7 +4,7 @@ config VMWARE_VMCI tristate VMware VMCI Driver - depends on X86 PCI + depends on X86 PCI NET help This is VMware's Virtual Machine Communication Interface. It enables high-speed communication between host and guest in a virtual -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drm/tilcdc: Disable building as module
Attempts to compile tilcdc as module results in: drivers/gpu/drm/tilcdc/tilcdc_slave.o:(.data+0x54): multiple definition of `__mod_of_device_table' drivers/gpu/drm/tilcdc/tilcdc_tfp410.o:(.data+0x54): first defined here drivers/gpu/drm/tilcdc/tilcdc_panel.o:(.data+0x54): multiple definition of `__mod_of_device_table' drivers/gpu/drm/tilcdc/tilcdc_tfp410.o:(.data+0x54): first defined here drivers/gpu/drm/tilcdc/tilcdc_drv.o:(.data+0x184): multiple definition of `__mod_of_device_table' drivers/gpu/drm/tilcdc/tilcdc_tfp410.o:(.data+0x54): first defined here make[5]: [drivers/gpu/drm/tilcdc/tilcdc.o] Error 1 (ignored) It appears it was never even tried to build it as module, or the problem would have been seen. So running this driver as module was never really tested. Disable it. If really needed, the code wil require some structural changes and in-depth testing, which can and should be done separately. Cc: Rob Clark robdcl...@gmail.com Signed-off-by: Guenter Roeck li...@roeck-us.net --- drivers/gpu/drm/tilcdc/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/Kconfig b/drivers/gpu/drm/tilcdc/Kconfig index d24d040..6ddb729 100644 --- a/drivers/gpu/drm/tilcdc/Kconfig +++ b/drivers/gpu/drm/tilcdc/Kconfig @@ -1,5 +1,5 @@ config DRM_TILCDC - tristate DRM Support for TI LCDC Display Controller + bool DRM Support for TI LCDC Display Controller depends on DRM OF ARM select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
is configured. While most callers check for the define, not all do, and those
who do require #ifdef around the code. For those who don't, the missing check
can result in errors such as
arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
Provide dummy function declarations if OF_NET is not configured. This is safe
because all callers do check the return values. If desired, at least some of
the #ifdefs in the code can subsequently be removed.
Cc: David Daney david.da...@cavium.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
include/linux/of_net.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index f474641..61bf53b 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,16 @@
#include linux/of.h
extern const int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
+#else
+static inline const int of_get_phy_mode(struct device_node *np)
+{
+ return -ENODEV;
+}
+
+static inline const void *of_get_mac_address(struct device_node *np)
+{
+ return NULL;
+}
#endif
#endif /* __LINUX_OF_NET_H */
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
On Mon, Apr 01, 2013 at 01:44:24PM -0500, Rob Herring wrote:
On 04/01/2013 01:19 PM, Guenter Roeck wrote:
of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
is configured. While most callers check for the define, not all do, and
those
who do require #ifdef around the code. For those who don't, the missing
check
can result in errors such as
How about removing the ifdef from those callers?
That would be the next step, after/if this one is accepted.
If not, it doesn't make sense to waste my time.
Thanks,
Guenter
Rob
arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
Provide dummy function declarations if OF_NET is not configured. This is
safe
because all callers do check the return values. If desired, at least some of
the #ifdefs in the code can subsequently be removed.
Cc: David Daney david.da...@cavium.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
include/linux/of_net.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index f474641..61bf53b 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,16 @@
#include linux/of.h
extern const int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
+#else
+static inline const int of_get_phy_mode(struct device_node *np)
+{
+ return -ENODEV;
+}
+
+static inline const void *of_get_mac_address(struct device_node *np)
+{
+ return NULL;
+}
#endif
#endif /* __LINUX_OF_NET_H */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] misc/vmw_vmci: VMWARE_VMCI depends on NET
On Mon, Apr 01, 2013 at 12:29:39PM -0700, Greg Kroah-Hartman wrote: On Sun, Mar 31, 2013 at 09:43:59PM -0700, Guenter Roeck wrote: Fix: ERROR: memcpy_toiovec [drivers/misc/vmw_vmci/vmw_vmci.ko] undefined! ERROR: memcpy_fromiovec [drivers/misc/vmw_vmci/vmw_vmci.ko] undefined! Both functions are defined in the core networking code. This is already in linux-next, thanks. Uuh ... and I submitted it. Sorry for the noise. Any chance to apply this to 3.9-rc ? It causes a bunch of unnecessary nightly build errors for me. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] misc/vmw_vmci: VMWARE_VMCI depends on NET
On Mon, Apr 01, 2013 at 01:11:52PM -0700, Greg Kroah-Hartman wrote: On Mon, Apr 01, 2013 at 01:02:35PM -0700, Guenter Roeck wrote: On Mon, Apr 01, 2013 at 12:29:39PM -0700, Greg Kroah-Hartman wrote: On Sun, Mar 31, 2013 at 09:43:59PM -0700, Guenter Roeck wrote: Fix: ERROR: memcpy_toiovec [drivers/misc/vmw_vmci/vmw_vmci.ko] undefined! ERROR: memcpy_fromiovec [drivers/misc/vmw_vmci/vmw_vmci.ko] undefined! Both functions are defined in the core networking code. This is already in linux-next, thanks. Uuh ... and I submitted it. Sorry for the noise. Any chance to apply this to 3.9-rc ? It causes a bunch of unnecessary nightly build errors for me. As it's a configuration that no real user would ever hit, it's not really 3.9 material, sorry. Fair enough. On the other side, value of make randconfig has been reduced significantly compared to earlier times, when patches like this tended to be accepted into release candidates. Until a few releases ago, make randconfig usually passed at least for main targets by the time a kernel was relased. With this no longer the case, fewer and fewer people will look into nightly or per-rc build results and provide patches. This in turn will likely reduce reliability, as real problems are more and more hidden among all the unreal build errors. In addition to that, more and more people will end up with non-buildable configurations and have to spend time trying to figure out why exactly a build failed. But maybe this is all my imagination, so feel free to just ignore my ranting ;). Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
On Sun, Mar 31, 2013 at 10:59:32PM -0700, Tony Chung wrote:
Observed that the w83627hf watchdog timer start counting during reboot.
If the system load the driver after 5 minutes, it rebooted immediately
because of timer expired.
For example, fsck took more than 5 minutes to run, then the computer reboot
as soon as the driver was loaded.
Thinking about it, I wonder if that really solves your problem.
If the watchdog driver is built into the kernel, and if the watchdog is already
running, you still may have the problem that it may time out again and trigger
(again) before the watchdog application can be loaded. Ultimately, in such a
situation, the watchdog application, or at least a primitive one, should really
be started from initramfs and possibly be replaced later on by the real
watchdog
application. The watchdog package includes a small binary for that purpose,
though I have no idea if is is loaded in initramfs or only later.
Ultimately, that also applies to the original problem you tried to solve with
the longer timeout. It doesn't seem correct to have to set a long timeout to
address excessive boot times; in a way that defeats the purpose of a watchdog.
A cleaner solution for that problem would be to make sure that a small watchdog
application is started early in the boot process.
Signed-off-by: Tony Chung tonychun...@gmail.com
---
drivers/watchdog/w83627hf_wdt.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 8fd..7eaa226 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -168,14 +168,16 @@ static void w83627hf_init(void)
pr_info(Watchdog already running. Resetting timeout to %d
sec\n,
wdt_get_timeout_secs(timeout));
outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */
+ } else {
+ outb_p(0, WDT_EFDR);/* disable to prevent reboot */
}
Did I miss that earlier, or did yo add it in this version ?
Reading the counter register just returned 0, so what are the benefits
of writing 0 into it ?
Thanks,
Guenter
w83627hf_setup_crf5();
outb_p(0xF7, WDT_EFER); /* Select CRF7 */
t = inb_p(WDT_EFDR); /* read CRF7 */
- t = ~0xC0; /* disable keyboard mouse turning off
- watchdog */
+ t = ~0xD0; /* clear timeout occurred and disable keyboard
+ mouse turning off watchdog */
outb_p(t, WDT_EFDR);/* Write back to CRF7 */
w83627hf_unselect_wd_register();
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-watchdog in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: UIO device tree bindings.
On Mon, Apr 01, 2013 at 05:40:08PM +0200, Pavel Machek wrote: On Mon 2013-04-01 16:23:36, Pavel Machek wrote: Hi! I'd like to get uio device tree bindings to work -- with recent FPGA parts it will be important. Latest version I see is https://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/073087.html ... Is there anything newer? I red the discussion, and main problem seems to be the tell kernel to drive this device tree device, right? Problem seems to be the notion that the proposed devicetree entry would not describe the hardware, but its use. Not really sure I understand the problem, as I would see the hardware description to be A hardware device which is compatible to and managed by the generic-uio driver. I would argue that this _is_ a hardware description (if not, what is ?), but I am not the one to make the call. So... here's the port to recent kernels. Not for mainline. Signed-off-by: Pavel Machek pa...@denx.de Turns out this is exactly what I need, and your post saves me the time I would have spent writing essentially the same code and submitting it, only to have it rejected. Thanks a lot for digging this up! Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO device tree bindings.
On Tue, Apr 02, 2013 at 01:51:51PM +0200, Pavel Machek wrote: On Mon 2013-04-01 19:42:12, Guenter Roeck wrote: On Mon, Apr 01, 2013 at 05:40:08PM +0200, Pavel Machek wrote: On Mon 2013-04-01 16:23:36, Pavel Machek wrote: Hi! I'd like to get uio device tree bindings to work -- with recent FPGA parts it will be important. Latest version I see is https://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/073087.html ... Is there anything newer? I red the discussion, and main problem seems to be the tell kernel to drive this device tree device, right? Problem seems to be the notion that the proposed devicetree entry would not describe the hardware, but its use. Not really sure I understand the problem, as I would see the hardware description to be A hardware device which is compatible to and managed by the generic-uio driver. I would argue that this _is_ a hardware description (if not, what is ?), but I am not the one to make the call. Well... one could argue that having generic-uio in board's device tree _is_ wrong, but having driver that binds to generic-uio is not. Hmm? You mean like ata-generic ? Or maybe we can do some magic with module parameter. That should be enough for expected use. I don't think that would make a difference. I mean, just take ns16550 as another example. No one has problems declaring some block of hardware addresses to be compatible with ns16550, even though it can be anything including a memory block on one of the FPGAs or ASICs we are talking about here, it can be anything but a NS16550, and many of the actual compatible strings are not defined anythere either. So there is no problem with ata-generic and ns16550, and no one cares if fsl,mpc8349emitx-pata or xlnx,xps-uart16550-2.00.b is defined or not, but generic-uio together with ptx,c64fpga001 is unacceptable. I think it has more to do with the uio driver not being an actual driver, but the kernel part of a user-space driver, though that is just a wild guess. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: UIO device tree bindings.
On Tue, Apr 02, 2013 at 03:46:45PM +0200, Pavel Machek wrote:
Hi!
Or maybe we can do some magic with module parameter. That should be
enough for expected use.
I don't think that would make a difference. I mean, just take ns16550 as
another
example. No one has problems declaring some block of hardware addresses
to be
compatible with ns16550, even though it can be anything including a
memory
block on one of the FPGAs or ASICs we are talking about here, it can be
anything
but a NS16550, and many of the actual compatible strings are not defined
anythere either.
So there is no problem with ata-generic and ns16550, and no one cares
if
fsl,mpc8349emitx-pata or xlnx,xps-uart16550-2.00.b is defined or not,
but
generic-uio together with ptx,c64fpga001 is unacceptable.
I think it has more to do with the uio driver not being an actual driver,
but
the kernel part of a user-space driver, though that is just a wild guess.
Well, it turned out that adding module parameter was not that much
work... and yes I believe it is a tiny bit cleaner.
Here's updated patch, if you can please test this one.
[You can just pass uio_of_genirq.of_id=generic-uio to get previous
behaviour. But don't tell anyone :-)]
Actually, looking at it some more, perhaps even solution is possible?
(This will need uio_pdrv_genirq.of_id=generic-uio .)
Possibly for your case, but not for mine, as I'll have several instances of the
driver, and the hardware is not always present. Also, I am not sure if the match
function runs before the module paratemers are initialized or afterwards, but
that
would be a matter of testing.
Anyway, after sleeping over it, I think I understand the concern better.
ns16550 describes some HW compatible to a specific chip with well defined
functionality, and generic-uio doesn't. Maybe it would have to be something
like generic-uio-hw instead.
Unfortunately, I don't have easy way to test this :-(. So feedback is
essential.
I'll try to get it to work, but it will take a bit.
Thanks,
Guenter
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index 4a50ead..64b6dac 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -271,9 +273,13 @@ static const struct dev_pm_ops
uio_pdrv_genirq_dev_pm_ops = {
#ifdef CONFIG_OF
static const struct of_device_id uio_of_genirq_match[] = {
- { /* empty for now */ },
+ { /* This is filled with module_parm */ },
+ { /* Sentinel */ },
};
MODULE_DEVICE_TABLE(of, uio_of_genirq_match);
+
+module_param_string(of_id, uio_of_genirq_match[0].compatible, 128, 0);
+MODULE_PARM_DESC(of_id, Openfirmware id of the device to be handled by
uio);
#else
# define uio_of_genirq_match NULL
#endif
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/5] Provide empty functions if OF_NET is not configured
Provide empty functions for of_get_phy_mode() and of_get_mac_address() if OF_NET is not configured. Modify affected drivers to rely on the now available functions. Guenter Roeck (5): of_net.h: Provide dummy functions if OF_NET is not configured net/cadence/at91_ether: Simplify OF dependencies net/cadence/macb: Simplify OF dependencies net/freescale/fec: Simplify OF dependencies net/nxp/lpc_eth: Drop ifdef CONFIG_OF_NET drivers/net/ethernet/cadence/at91_ether.c | 44 - drivers/net/ethernet/cadence/macb.c | 43 drivers/net/ethernet/freescale/fec.c | 19 + drivers/net/ethernet/nxp/lpc_eth.c|2 -- include/linux/of_net.h| 10 +++ 5 files changed, 23 insertions(+), 95 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/5] net/cadence/at91_ether: Simplify OF dependencies
With of_get_mac_address() and of_get_phy_mode() now defined as dummy
functions if OF_NET is not configured, it is no longer necessary to
provide OF dependent functions as front-end. Also, the two functions
depend on OF_NET, not on OF, so the conditional code was not correct
anyway.
Drop the front-end functions and call of_get_mac_address() and
of_get_phy_mode() directly instead.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/cadence/at91_ether.c | 44 -
1 file changed, 6 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/cadence/at91_ether.c
b/drivers/net/ethernet/cadence/at91_ether.c
index 3becdb2..50565cb 100644
--- a/drivers/net/ethernet/cadence/at91_ether.c
+++ b/drivers/net/ethernet/cadence/at91_ether.c
@@ -303,42 +303,7 @@ static const struct of_device_id at91ether_dt_ids[] = {
{ .compatible = cdns,emac },
{ /* sentinel */ }
};
-
MODULE_DEVICE_TABLE(of, at91ether_dt_ids);
-
-static int at91ether_get_phy_mode_dt(struct platform_device *pdev)
-{
- struct device_node *np = pdev-dev.of_node;
-
- if (np)
- return of_get_phy_mode(np);
-
- return -ENODEV;
-}
-
-static int at91ether_get_hwaddr_dt(struct macb *bp)
-{
- struct device_node *np = bp-pdev-dev.of_node;
-
- if (np) {
- const char *mac = of_get_mac_address(np);
- if (mac) {
- memcpy(bp-dev-dev_addr, mac, ETH_ALEN);
- return 0;
- }
- }
-
- return -ENODEV;
-}
-#else
-static int at91ether_get_phy_mode_dt(struct platform_device *pdev)
-{
- return -ENODEV;
-}
-static int at91ether_get_hwaddr_dt(struct macb *bp)
-{
- return -ENODEV;
-}
#endif
/* Detect MAC PHY and perform ethernet interface initialization */
@@ -352,6 +317,7 @@ static int __init at91ether_probe(struct platform_device
*pdev)
struct macb *lp;
int res;
u32 reg;
+ const char *mac;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs)
@@ -403,11 +369,13 @@ static int __init at91ether_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, pdev-dev);
- res = at91ether_get_hwaddr_dt(lp);
- if (res 0)
+ mac = of_get_mac_address(pdev-dev.of_node);
+ if (mac)
+ memcpy(lp-dev-dev_addr, mac, ETH_ALEN);
+ else
macb_get_hwaddr(lp);
- res = at91ether_get_phy_mode_dt(pdev);
+ res = of_get_phy_mode(pdev-dev.of_node);
if (res 0) {
if (board_data board_data-is_rmii)
lp-phy_interface = PHY_INTERFACE_MODE_RMII;
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] net/freescale/fec: Simplify OF dependencies
Since of_get_mac_address() is now defined even if CONFIG_OF_NET
is not configured, the ifdef around the code calling it is no longer
necessary and can be removed.
Similar, since of_get_phy_mode() is now defined as dummy function
if OF_NET is not configured, it is no longer necessary to provide
an OF dependent function as front-end. Also, the function depends
on OF_NET, not on OF, so the conditional code was not correct anyway.
Drop the front-end function and call of_get_phy_mode() directly.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/freescale/fec.c | 19 +--
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c
b/drivers/net/ethernet/freescale/fec.c
index 911d025..d374a94 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -868,7 +868,6 @@ static void fec_get_mac(struct net_device *ndev)
*/
iap = macaddr;
-#ifdef CONFIG_OF
/*
* 2) from device tree data
*/
@@ -880,7 +879,6 @@ static void fec_get_mac(struct net_device *ndev)
iap = (unsigned char *) mac;
}
}
-#endif
/*
* 3) from flash or fuse (via platform data)
@@ -1666,16 +1664,6 @@ static int fec_enet_init(struct net_device *ndev)
}
#ifdef CONFIG_OF
-static int fec_get_phy_mode_dt(struct platform_device *pdev)
-{
- struct device_node *np = pdev-dev.of_node;
-
- if (np)
- return of_get_phy_mode(np);
-
- return -ENODEV;
-}
-
static void fec_reset_phy(struct platform_device *pdev)
{
int err, phy_reset;
@@ -1704,11 +1692,6 @@ static void fec_reset_phy(struct platform_device *pdev)
gpio_set_value(phy_reset, 1);
}
#else /* CONFIG_OF */
-static int fec_get_phy_mode_dt(struct platform_device *pdev)
-{
- return -ENODEV;
-}
-
static void fec_reset_phy(struct platform_device *pdev)
{
/*
@@ -1773,7 +1756,7 @@ fec_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ndev);
- ret = fec_get_phy_mode_dt(pdev);
+ ret = of_get_phy_mode(pdev-dev.of_node);
if (ret 0) {
pdata = pdev-dev.platform_data;
if (pdata)
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] net/nxp/lpc_eth: Drop ifdef CONFIG_OF_NET
Since of_get_mac_address() is now declared even if CONFIG_OF_NET
is not configured, the ifdef is no longer necessary and can be
removed.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/nxp/lpc_eth.c |2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
b/drivers/net/ethernet/nxp/lpc_eth.c
index efa29b7..99276d7 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -1434,13 +1434,11 @@ static int lpc_eth_drv_probe(struct platform_device
*pdev)
/* Get MAC address from current HW setting (POR state is all zeros) */
__lpc_get_mac(pldat, ndev-dev_addr);
-#ifdef CONFIG_OF_NET
if (!is_valid_ether_addr(ndev-dev_addr)) {
const char *macaddr = of_get_mac_address(pdev-dev.of_node);
if (macaddr)
memcpy(ndev-dev_addr, macaddr, ETH_ALEN);
}
-#endif
if (!is_valid_ether_addr(ndev-dev_addr))
eth_hw_addr_random(ndev);
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] net/cadence/macb: Simplify OF dependencies
With of_get_mac_address() and of_get_phy_mode() now defined as dummy
functions if OF_NET is not configured, it is no longer necessary to
provide OF dependent functions as front-end. Also, the two functions
depend on OF_NET, not on OF, so the conditional code was not correct
anyway.
Drop the front-end functions and call of_get_mac_address() and
of_get_phy_mode() directly instead.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/cadence/macb.c | 43 +--
1 file changed, 6 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c
b/drivers/net/ethernet/cadence/macb.c
index 7903943..b2a9d20 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1472,41 +1472,7 @@ static const struct of_device_id macb_dt_ids[] = {
{ .compatible = cdns,gem },
{ /* sentinel */ }
};
-
MODULE_DEVICE_TABLE(of, macb_dt_ids);
-
-static int macb_get_phy_mode_dt(struct platform_device *pdev)
-{
- struct device_node *np = pdev-dev.of_node;
-
- if (np)
- return of_get_phy_mode(np);
-
- return -ENODEV;
-}
-
-static int macb_get_hwaddr_dt(struct macb *bp)
-{
- struct device_node *np = bp-pdev-dev.of_node;
- if (np) {
- const char *mac = of_get_mac_address(np);
- if (mac) {
- memcpy(bp-dev-dev_addr, mac, ETH_ALEN);
- return 0;
- }
- }
-
- return -ENODEV;
-}
-#else
-static int macb_get_phy_mode_dt(struct platform_device *pdev)
-{
- return -ENODEV;
-}
-static int macb_get_hwaddr_dt(struct macb *bp)
-{
- return -ENODEV;
-}
#endif
static int __init macb_probe(struct platform_device *pdev)
@@ -1519,6 +1485,7 @@ static int __init macb_probe(struct platform_device *pdev)
u32 config;
int err = -ENXIO;
struct pinctrl *pinctrl;
+ const char *mac;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
@@ -1592,11 +1559,13 @@ static int __init macb_probe(struct platform_device
*pdev)
config |= macb_dbw(bp);
macb_writel(bp, NCFGR, config);
- err = macb_get_hwaddr_dt(bp);
- if (err 0)
+ mac = of_get_mac_address(pdev-dev.of_node);
+ if (mac)
+ memcpy(bp-dev-dev_addr, mac, ETH_ALEN);
+ else
macb_get_hwaddr(bp);
- err = macb_get_phy_mode_dt(pdev);
+ err = of_get_phy_mode(pdev-dev.of_node);
if (err 0) {
pdata = pdev-dev.platform_data;
if (pdata pdata-is_rmii)
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] of_net.h: Provide empty functions if OF_NET is not configured
of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
is configured. While most callers check for the define, not all do, and those
who do require #ifdef around the code. For those who don't, the missing check
can result in errors such as
arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
Provide empty functions if OF_NET is not configured.
This is safe because all callers do check the return values.
Cc: David Daney david.da...@cavium.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
Acked-by: Rob Herring rob.herr...@calxeda.com
---
include/linux/of_net.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index f474641..61bf53b 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,16 @@
#include linux/of.h
extern const int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
+#else
+static inline const int of_get_phy_mode(struct device_node *np)
+{
+ return -ENODEV;
+}
+
+static inline const void *of_get_mac_address(struct device_node *np)
+{
+ return NULL;
+}
#endif
#endif /* __LINUX_OF_NET_H */
--
1.7.9.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
On Mon, Apr 01, 2013 at 09:59:00PM -0700, Tony Chung wrote: Thanks Guenter! I agree with you. My first reaction was also about a small watchdog server that will start in early boot process. There are pros and cons. For example, there are many types of watchdog devices such as ipmi_watchdog which can accept more than 255 seconds for timeout. So you really need udev to pick the correct watchdog driver. It could be very complex. Our requirement don't need watchdog protection during the boot process until application is fully up but a driver should not assume anything. Ok, but then the BIOS should not enable the watchdog. Anyway, an unexpected reboot is definitely a bug that need to be fixed. It is really easily reproducible. Depending on your hardware Agreed. and BIOS settings, just reboot the boot, wait for 5 minutes and then run insmod w83627hf_wdt.ko. The box just reboot by itself. The watchdog sever is not even started. Doesn't happen for me, as the watchdog is initially not enabled in my system, and bit 4 is never set. And when it gets set, the system immediately reboots. This line is actually the original fix that is running over a year: outb_p(0, WDT_EFDR);/* disable to prevent reboot */ Unfortunately this turns off the watchdog if it was running and has triggered. When I tried to cleanup it up, I thought I don't need it but it turned out it was still needed. When I changed it from 0xC0 to 0xD0, it still reboot. So it looks like the watchdog triggered, for some reason did not cause a reset, but resetting the trigger flag does. Ultimately, the new code turns the watchdog off if it was running, has already triggered, but did not cause a reset. The ultimate effect is that the system will hang if it gets stuck before the watchdog application is started later on. Maybe that is not your application, but others wil defintely want that protection. So I don't think the solution is correct. We should find out why the watchdog trigger did not cause a reset. Also, I think it would be better in this situation (if watchdog has triggered) to restart it with the default timeout. What is the exact chip type in your system ? I want to have a look into the datasheet; maybe I can find out how it can trigger without causing a reset. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
