2012-05-12 10:52:34

by Hiroshi Doyu

[permalink] [raw]
Subject: [RFC 1/1] driver core: Add dev_*_ratelimited() family

Hi,

An unclosed "if" statement in the MACRO seems a bit risky, but I don't
have any better/simple solution for this, ATM. Is there any alternative?


From: Hiroshi DOYU <[email protected]>

Add dev_*_ratelimited() family, dev_* version of pr_*_ratelimited().

Signed-off-by: Hiroshi DOYU <[email protected]>
---
include/linux/device.h | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8b9d03a..9d976df 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -23,6 +23,7 @@
#include <linux/mutex.h>
#include <linux/pm.h>
#include <linux/atomic.h>
+#include <linux/ratelimit.h>
#include <asm/device.h>

struct device;
@@ -937,6 +938,27 @@ int _dev_info(const struct device *dev, const char *fmt, ...)

#endif

+#define IF_DEV_RATELIMITED \
+ static DEFINE_RATELIMIT_STATE(_rs, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ if (__ratelimit(&_rs))
+
+#define dev_emerg_ratelimited(dev, fmt, ...) \
+ do { IF_DEV_RATELIMITED dev_printk(KERN_EMERG, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dev_alert_ratelimited(dev, fmt, ...) \
+ do { IF_DEV_RATELIMITED dev_printk(KERN_ALERT, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dev_crit_ratelimited(dev, fmt, ...) \
+ do { IF_DEV_RATELIMITED dev_printk(KERN_CRIT, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dev_err_ratelimited(dev, fmt, ...) \
+ do { IF_DEV_RATELIMITED dev_printk(KERN_ERR, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dev_warn_ratelimited(dev, fmt, ...) \
+ do { IF_DEV_RATELIMITED dev_printk(KERN_WARNING, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dev_notice_ratelimited(dev, fmt, ...) \
+ do { IF_DEV_RATELIMITED dev_printk(KERN_NOTICE, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dev_info_ratelimited(dev, fmt, ...) \
+ do { IF_DEV_RATELIMITED dev_printk(KERN_INFO, dev, fmt, ##__VA_ARGS__); } while (0)
+
/*
* Stupid hackaround for existing uses of non-printk uses dev_info
*
--
1.7.5.4


2012-05-12 12:07:59

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [RFC 1/1] driver core: Add dev_*_ratelimited() family

Hi!

On Sam, 2012-05-12 at 12:52 +0200, Hiroshi Doyu wrote:
[...]
> An unclosed "if" statement in the MACRO seems a bit risky, but I don't
> have any better/simple solution for this, ATM. Is there any alternative?

No, the "do { ... } while (0)" is actually the only one and widely used
- not only in the Linux kernel.

[....]

Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at

2012-05-12 15:31:53

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC 1/1] driver core: Add dev_*_ratelimited() family

On Sat, 2012-05-12 at 12:52 +0200, Hiroshi Doyu wrote:
> Hi,
>
> An unclosed "if" statement in the MACRO seems a bit risky, but I don't
> have any better/simple solution for this, ATM. Is there any alternative?

maybe something like:

#define dev_ratelimited_level(dev, level, fmt, ...)
do {
static DEFINE_RATELIMIT_STATE(_rs, \
DEFAULT_RATELIMIT_INTERVAL, \
DEFAULT_RATELIMIT_BURST); \
if (__ratelimit(&_rs)) \
dev_##level(fmt, ##__VA_ARGS__); \
} while (0)

#define dev_emerg_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, emerg, fmt, ##__VA_ARGS__)
#define dev_alert_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, alert, fmt, ##__VA_ARGS__)
#define dev_crit_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, crit, fmt, ##__VA_ARGS__)
#define dev_err_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, err, fmt, ##__VA_ARGS__)
#define dev_warn_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, warn, fmt, ##__VA_ARGS__)
#define dev_notice_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, notice, fmt, ##__VA_ARGS__)
#define dev_info_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, info, fmt, ##__VA_ARGS__)
#define dev_dbg_ratelimited(dev, fmt, ...) \
dev_ratelimited_level(dev, dbg, fmt, ##__VA_ARGS__)

2012-05-14 05:00:19

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [RFC 1/1] driver core: Add dev_*_ratelimited() family

Hi Joe,

Joe Perches <[email protected]> wrote @ Sat, 12 May 2012 17:31:35 +0200:

> On Sat, 2012-05-12 at 12:52 +0200, Hiroshi Doyu wrote:
> > Hi,
> >
> > An unclosed "if" statement in the MACRO seems a bit risky, but I don't
> > have any better/simple solution for this, ATM. Is there any alternative?
>
> maybe something like:
>
> #define dev_ratelimited_level(dev, level, fmt, ...)
> do {
> static DEFINE_RATELIMIT_STATE(_rs, \
> DEFAULT_RATELIMIT_INTERVAL, \
> DEFAULT_RATELIMIT_BURST); \
> if (__ratelimit(&_rs)) \
> dev_##level(fmt, ##__VA_ARGS__); \
> } while (0)
>
> #define dev_emerg_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, emerg, fmt, ##__VA_ARGS__)
> #define dev_alert_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, alert, fmt, ##__VA_ARGS__)
> #define dev_crit_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, crit, fmt, ##__VA_ARGS__)
> #define dev_err_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, err, fmt, ##__VA_ARGS__)
> #define dev_warn_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, warn, fmt, ##__VA_ARGS__)
> #define dev_notice_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, notice, fmt, ##__VA_ARGS__)
> #define dev_info_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, info, fmt, ##__VA_ARGS__)
> #define dev_dbg_ratelimited(dev, fmt, ...) \
> dev_ratelimited_level(dev, dbg, fmt, ##__VA_ARGS__)

"dev" isn't handled separately with __VA_ARGS__, and failed to build
as below:

Example:
dev_err_ratelimited(&pdev->dev, "%d\n", __LINE__);

After preprocessded:
do { ... if (___ratelimit(&_rs, __func__)) dev_err("%d\n", 18); } while (0);

2012-05-14 05:25:58

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC 1/1] driver core: Add dev_*_ratelimited() family

On Mon, 2012-05-14 at 07:00 +0200, Hiroshi Doyu wrote:
> Hi Joe,
>
> Joe Perches <[email protected]> wrote @ Sat, 12 May 2012 17:31:35 +0200:
>
> > On Sat, 2012-05-12 at 12:52 +0200, Hiroshi Doyu wrote:
> > > Hi,
> > >
> > > An unclosed "if" statement in the MACRO seems a bit risky, but I don't
> > > have any better/simple solution for this, ATM. Is there any alternative?
> >
> > maybe something like:
> >
> > #define dev_ratelimited_level(dev, level, fmt, ...)
> > do {
> > static DEFINE_RATELIMIT_STATE(_rs, \
> > DEFAULT_RATELIMIT_INTERVAL, \
> > DEFAULT_RATELIMIT_BURST); \
> > if (__ratelimit(&_rs)) \
> > dev_##level(fmt, ##__VA_ARGS__); \
> > } while (0)
> >
> > #define dev_emerg_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, emerg, fmt, ##__VA_ARGS__)
> > #define dev_alert_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, alert, fmt, ##__VA_ARGS__)
> > #define dev_crit_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, crit, fmt, ##__VA_ARGS__)
> > #define dev_err_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, err, fmt, ##__VA_ARGS__)
> > #define dev_warn_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, warn, fmt, ##__VA_ARGS__)
> > #define dev_notice_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, notice, fmt, ##__VA_ARGS__)
> > #define dev_info_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, info, fmt, ##__VA_ARGS__)
> > #define dev_dbg_ratelimited(dev, fmt, ...) \
> > dev_ratelimited_level(dev, dbg, fmt, ##__VA_ARGS__)
>
> "dev" isn't handled separately with __VA_ARGS__, and failed to build
> as below:
>
> Example:
> dev_err_ratelimited(&pdev->dev, "%d\n", __LINE__);
>
> After preprocessded:
> do { ... if (___ratelimit(&_rs, __func__)) dev_err("%d\n", 18); } while (0);
>

Sorry, I was just typing in the email client and
I missed the "dev" argument.

Add "dev" to the dev_##level statement like:

#define dev_ratelimited_level(dev, level, fmt, ...) \
do { \
static DEFINE_RATELIMIT_STATE(_rs, \
DEFAULT_RATELIMIT_INTERVAL, \
DEFAULT_RATELIMIT_BURST); \
if (__ratelimit(&_rs)) \
dev_##level(dev, fmt, ##__VA_ARGS__); \
} while (0)

2012-05-14 05:40:21

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [RFC 1/1] driver core: Add dev_*_ratelimited() family

Joe Perches <[email protected]> wrote @ Mon, 14 May 2012 07:25:55 +0200:

> On Mon, 2012-05-14 at 07:00 +0200, Hiroshi Doyu wrote:
> > Hi Joe,
> >
> > Joe Perches <[email protected]> wrote @ Sat, 12 May 2012 17:31:35 +0200:
> >
> > > On Sat, 2012-05-12 at 12:52 +0200, Hiroshi Doyu wrote:
> > > > Hi,
> > > >
> > > > An unclosed "if" statement in the MACRO seems a bit risky, but I don't
> > > > have any better/simple solution for this, ATM. Is there any alternative?
> > >
> > > maybe something like:
> > >
> > > #define dev_ratelimited_level(dev, level, fmt, ...)
> > > do {
> > > static DEFINE_RATELIMIT_STATE(_rs, \
> > > DEFAULT_RATELIMIT_INTERVAL, \
> > > DEFAULT_RATELIMIT_BURST); \
> > > if (__ratelimit(&_rs)) \
> > > dev_##level(fmt, ##__VA_ARGS__); \
> > > } while (0)
> > >
> > > #define dev_emerg_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, emerg, fmt, ##__VA_ARGS__)
> > > #define dev_alert_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, alert, fmt, ##__VA_ARGS__)
> > > #define dev_crit_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, crit, fmt, ##__VA_ARGS__)
> > > #define dev_err_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, err, fmt, ##__VA_ARGS__)
> > > #define dev_warn_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, warn, fmt, ##__VA_ARGS__)
> > > #define dev_notice_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, notice, fmt, ##__VA_ARGS__)
> > > #define dev_info_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, info, fmt, ##__VA_ARGS__)
> > > #define dev_dbg_ratelimited(dev, fmt, ...) \
> > > dev_ratelimited_level(dev, dbg, fmt, ##__VA_ARGS__)
> >
> > "dev" isn't handled separately with __VA_ARGS__, and failed to build
> > as below:
> >
> > Example:
> > dev_err_ratelimited(&pdev->dev, "%d\n", __LINE__);
> >
> > After preprocessded:
> > do { ... if (___ratelimit(&_rs, __func__)) dev_err("%d\n", 18); } while (0);
> >
>
> Sorry, I was just typing in the email client and
> I missed the "dev" argument.
>
> Add "dev" to the dev_##level statement like:
>
> #define dev_ratelimited_level(dev, level, fmt, ...) \
> do { \
> static DEFINE_RATELIMIT_STATE(_rs, \
> DEFAULT_RATELIMIT_INTERVAL, \
> DEFAULT_RATELIMIT_BURST); \
> if (__ratelimit(&_rs)) \
> dev_##level(dev, fmt, ##__VA_ARGS__); \
> } while (0)

Verified that the above works. Would you mind sending the complete version of this patch?

2012-05-14 06:05:42

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC 1/1] driver core: Add dev_*_ratelimited() family

On Mon, 2012-05-14 at 07:40 +0200, Hiroshi Doyu wrote:
> Joe Perches <[email protected]> wrote @ Mon, 14 May 2012 07:25:55 +0200:
> > On Mon, 2012-05-14 at 07:00 +0200, Hiroshi Doyu wrote:
> > > Joe Perches <[email protected]> wrote @ Sat, 12 May 2012 17:31:35 +0200:
> > > > On Sat, 2012-05-12 at 12:52 +0200, Hiroshi Doyu wrote:
> > > > > An unclosed "if" statement in the MACRO seems a bit risky, but I don't
> > > > > have any better/simple solution for this, ATM. Is there any alternative?
> > > >
> > > > maybe something like:
> > > >
> > > > #define dev_ratelimited_level(dev, level, fmt, ...)
> > > > do {
> > > > static DEFINE_RATELIMIT_STATE(_rs, \
> > > > DEFAULT_RATELIMIT_INTERVAL, \
> > > > DEFAULT_RATELIMIT_BURST); \
> > > > if (__ratelimit(&_rs)) \
> > > > dev_##level(fmt, ##__VA_ARGS__); \
> > > > } while (0)
> > > >
> > > > #define dev_emerg_ratelimited(dev, fmt, ...) \
> > > > dev_ratelimited_level(dev, emerg, fmt, ##__VA_ARGS__)
[...]
> > > > #define dev_dbg_ratelimited(dev, fmt, ...) \
> > > > dev_ratelimited_level(dev, dbg, fmt, ##__VA_ARGS__)
> > >
> > > "dev" isn't handled separately with __VA_ARGS__, and failed to build
> > > as below:
> > >
> > > Example:
> > > dev_err_ratelimited(&pdev->dev, "%d\n", __LINE__);
> > >
> > > After preprocessded:
> > > do { ... if (___ratelimit(&_rs, __func__)) dev_err("%d\n", 18); } while (0);
> > >
> >
> > Sorry, I was just typing in the email client and
> > I missed the "dev" argument.
> >
> > Add "dev" to the dev_##level statement like:
> >
> > #define dev_ratelimited_level(dev, level, fmt, ...) \
> > do { \
> > static DEFINE_RATELIMIT_STATE(_rs, \
> > DEFAULT_RATELIMIT_INTERVAL, \
> > DEFAULT_RATELIMIT_BURST); \
> > if (__ratelimit(&_rs)) \
> > dev_##level(dev, fmt, ##__VA_ARGS__); \
> > } while (0)
>
> Verified that the above works. Would you mind sending the complete version of this patch?

Hello Hiroshi.

It's your patch and your idea.
I think you should submit it.
You were just asking for alternatives or a bit
of guidance.

Maybe a better name for dev_ratelimited_level is
dev_level_ratelimited and the macro should be

#define dev_level_ratelimited(dev_level, dev, fmt, ...) \
do { \
static DEFINE_RATELIMIT_STATE(_rs, \
DEFAULT_RATELIMIT_INTERVAL, \
DEFAULT_RATELIMIT_BURST); \
if (__ratelimit(&_rs)) \
dev_level(dev, fmt, ##__VA_ARGS__); \
} while (0)

with uses like

#define dev_notice_ratelimited(dev, fmt, ...) \
dev_level_ratelimited(dev_notice, fmt, ##__VA_ARGS__)


Your choice though I think the last option above
may be better because it more closely follows the
style a dev_printk_ratelimited would use.

cheers, Joe

2012-05-14 07:45:18

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [RFC 1/1] driver core: Add dev_*_ratelimited() family

Joe Perches <[email protected]> wrote @ Mon, 14 May 2012 08:05:39 +0200:

> On Mon, 2012-05-14 at 07:40 +0200, Hiroshi Doyu wrote:
> > Joe Perches <[email protected]> wrote @ Mon, 14 May 2012 07:25:55 +0200:
> > > On Mon, 2012-05-14 at 07:00 +0200, Hiroshi Doyu wrote:
> > > > Joe Perches <[email protected]> wrote @ Sat, 12 May 2012 17:31:35 +0200:
> > > > > On Sat, 2012-05-12 at 12:52 +0200, Hiroshi Doyu wrote:
> > > > > > An unclosed "if" statement in the MACRO seems a bit risky, but I don't
> > > > > > have any better/simple solution for this, ATM. Is there any alternative?
> > > > >
> > > > > maybe something like:
> > > > >
> > > > > #define dev_ratelimited_level(dev, level, fmt, ...)
> > > > > do {
> > > > > static DEFINE_RATELIMIT_STATE(_rs, \
> > > > > DEFAULT_RATELIMIT_INTERVAL, \
> > > > > DEFAULT_RATELIMIT_BURST); \
> > > > > if (__ratelimit(&_rs)) \
> > > > > dev_##level(fmt, ##__VA_ARGS__); \
> > > > > } while (0)
> > > > >
> > > > > #define dev_emerg_ratelimited(dev, fmt, ...) \
> > > > > dev_ratelimited_level(dev, emerg, fmt, ##__VA_ARGS__)
> [...]
> > > > > #define dev_dbg_ratelimited(dev, fmt, ...) \
> > > > > dev_ratelimited_level(dev, dbg, fmt, ##__VA_ARGS__)
> > > >
> > > > "dev" isn't handled separately with __VA_ARGS__, and failed to build
> > > > as below:
> > > >
> > > > Example:
> > > > dev_err_ratelimited(&pdev->dev, "%d\n", __LINE__);
> > > >
> > > > After preprocessded:
> > > > do { ... if (___ratelimit(&_rs, __func__)) dev_err("%d\n", 18); } while (0);
> > > >
> > >
> > > Sorry, I was just typing in the email client and
> > > I missed the "dev" argument.
> > >
> > > Add "dev" to the dev_##level statement like:
> > >
> > > #define dev_ratelimited_level(dev, level, fmt, ...) \
> > > do { \
> > > static DEFINE_RATELIMIT_STATE(_rs, \
> > > DEFAULT_RATELIMIT_INTERVAL, \
> > > DEFAULT_RATELIMIT_BURST); \
> > > if (__ratelimit(&_rs)) \
> > > dev_##level(dev, fmt, ##__VA_ARGS__); \
> > > } while (0)
> >
> > Verified that the above works. Would you mind sending the complete version of this patch?
>
> Hello Hiroshi.
>
> It's your patch and your idea.
> I think you should submit it.
> You were just asking for alternatives or a bit
> of guidance.

Thanks.

> Maybe a better name for dev_ratelimited_level is
> dev_level_ratelimited and the macro should be
>
> #define dev_level_ratelimited(dev_level, dev, fmt, ...) \
> do { \
> static DEFINE_RATELIMIT_STATE(_rs, \
> DEFAULT_RATELIMIT_INTERVAL, \
> DEFAULT_RATELIMIT_BURST); \
> if (__ratelimit(&_rs)) \
> dev_level(dev, fmt, ##__VA_ARGS__); \
> } while (0)
>
> with uses like
>
> #define dev_notice_ratelimited(dev, fmt, ...) \
> dev_level_ratelimited(dev_notice, fmt, ##__VA_ARGS__)
>
>
> Your choice though I think the last option above
> may be better because it more closely follows the
> style a dev_printk_ratelimited would use.

Agree. The complete version of the above patch follows this email.

2012-05-14 07:48:13

by Hiroshi Doyu

[permalink] [raw]
Subject: [PATCH 1/2] driver core: Add dev_*_ratelimited() family

Add dev_*_ratelimited() family, dev_* version of pr_*_ratelimited().

Using Joe Perches's proposal/implementation.

Signed-off-by: Hiroshi DOYU <[email protected]>
Cc: Joe Perches <[email protected]>
---
include/linux/device.h | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8b9d03a..d3aafdb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -23,6 +23,7 @@
#include <linux/mutex.h>
#include <linux/pm.h>
#include <linux/atomic.h>
+#include <linux/ratelimit.h>
#include <asm/device.h>

struct device;
@@ -937,6 +938,32 @@ int _dev_info(const struct device *dev, const char *fmt, ...)

#endif

+#define dev_level_ratelimited(dev_level, dev, fmt, ...) \
+do { \
+ static DEFINE_RATELIMIT_STATE(_rs, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ if (__ratelimit(&_rs)) \
+ dev_level(dev, fmt, ##__VA_ARGS__); \
+} while (0)
+
+#define dev_emerg_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_emerg, dev, fmt, ##__VA_ARGS__)
+#define dev_alert_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_alert, dev, fmt, ##__VA_ARGS__)
+#define dev_crit_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_crit, dev, fmt, ##__VA_ARGS__)
+#define dev_err_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_err, dev, fmt, ##__VA_ARGS__)
+#define dev_warn_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_warn, dev, fmt, ##__VA_ARGS__)
+#define dev_notice_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
+#define dev_info_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
+#define dev_dbg_ratelimited(dev, fmt, ...) \
+ dev_level_ratelimited(dev_dbg, dev, fmt, ##__VA_ARGS__)
+
/*
* Stupid hackaround for existing uses of non-printk uses dev_info
*
--
1.7.5.4

2012-05-14 07:48:21

by Hiroshi Doyu

[permalink] [raw]
Subject: [PATCH 2/2] memory: tegra{20,30}-mc: Use dev_err_ratelimited()

Introduce a new dev_*_ratelimited() instead of pr_*_ratelimited() for
better info to print.

Signed-off-by: Hiroshi DOYU <[email protected]>
---
drivers/memory/tegra20-mc.c | 3 ++-
drivers/memory/tegra30-mc.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
index 21a735e..d525322 100644
--- a/drivers/memory/tegra20-mc.c
+++ b/drivers/memory/tegra20-mc.c
@@ -161,7 +161,8 @@ static void tegra20_mc_decode(struct tegra20_mc *mc, int n)

idx = n - MC_INT_ERR_SHIFT;
if ((idx < 0) || (idx >= ARRAY_SIZE(reg))) {
- pr_err_ratelimited("Unknown interrupt status %08lx\n", BIT(n));
+ dev_err_ratelimited(mc->dev, "Unknown interrupt status %08lx\n",
+ BIT(n));
return;
}

diff --git a/drivers/memory/tegra30-mc.c b/drivers/memory/tegra30-mc.c
index c391c4e..fad6bb5 100644
--- a/drivers/memory/tegra30-mc.c
+++ b/drivers/memory/tegra30-mc.c
@@ -220,7 +220,8 @@ static void tegra30_mc_decode(struct tegra30_mc *mc, int n)

idx = n - MC_INT_ERR_SHIFT;
if ((idx < 0) || (idx >= ARRAY_SIZE(mc_int_err)) || (idx == 1)) {
- pr_err_ratelimited("Unknown interrupt status %08lx\n", BIT(n));
+ dev_err_ratelimited(mc->dev, "Unknown interrupt status %08lx\n",
+ BIT(n));
return;
}

--
1.7.5.4

2012-05-14 10:07:20

by Hiroshi Doyu

[permalink] [raw]
Subject: [v2 1/1] memory: tegra{20,30}-mc: Use dev_err_ratelimited()

Introduce a new dev_*_ratelimited() instead of pr_*_ratelimited() for
better info to print.

Signed-off-by: Hiroshi DOYU <[email protected]>
---
drivers/memory/tegra20-mc.c | 5 +++--
drivers/memory/tegra30-mc.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
index 2a437e9..a38d9d3 100644
--- a/drivers/memory/tegra20-mc.c
+++ b/drivers/memory/tegra20-mc.c
@@ -161,7 +161,8 @@ static void tegra20_mc_decode(struct tegra20_mc *mc, int n)

idx = n - MC_INT_ERR_SHIFT;
if ((idx < 0) || (idx >= ARRAY_SIZE(reg))) {
- pr_err_ratelimited("Unknown interrupt status %08lx\n", BIT(n));
+ dev_err_ratelimited(mc->dev, "Unknown interrupt status %08lx\n",
+ BIT(n));
return;
}

@@ -172,7 +173,7 @@ static void tegra20_mc_decode(struct tegra20_mc *mc, int n)

addr = mc_readl(mc, reg[idx].offset + sizeof(u32));

- pr_err_ratelimited("%s (0x%08x): 0x%08x %s (%s %s)\n",
+ dev_err_ratelimited(mc->dev, "%s (0x%08x): 0x%08x %s (%s %s)\n",
reg[idx].message, req, addr, client,
(req & BIT(reg[idx].write_bit)) ? "write" : "read",
(reg[idx].offset == MC_SECURITY_VIOLATION_STATUS) ?
diff --git a/drivers/memory/tegra30-mc.c b/drivers/memory/tegra30-mc.c
index ec9fc78..cb639b1 100644
--- a/drivers/memory/tegra30-mc.c
+++ b/drivers/memory/tegra30-mc.c
@@ -220,7 +220,8 @@ static void tegra30_mc_decode(struct tegra30_mc *mc, int n)

idx = n - MC_INT_ERR_SHIFT;
if ((idx < 0) || (idx >= ARRAY_SIZE(mc_int_err)) || (idx == 1)) {
- pr_err_ratelimited("Unknown interrupt status %08lx\n", BIT(n));
+ dev_err_ratelimited(mc->dev, "Unknown interrupt status %08lx\n",
+ BIT(n));
return;
}

@@ -243,7 +244,7 @@ static void tegra30_mc_decode(struct tegra30_mc *mc, int n)

addr = readl(mc + MC_ERR_ADR);

- pr_err_ratelimited("%s (0x%08x): 0x%08x %s (%s %s %s %s)\n",
+ dev_err_ratelimited(mc->dev, "%s (0x%08x): 0x%08x %s (%s %s %s %s)\n",
mc_int_err[idx], err, addr, client,
(err & MC_ERR_SECURITY) ? "secure" : "non-secure",
(err & MC_ERR_RW) ? "write" : "read",
--
1.7.5.4

2012-05-14 18:33:08

by Stephen Warren

[permalink] [raw]
Subject: Re: [v2 1/1] memory: tegra{20,30}-mc: Use dev_err_ratelimited()

On 05/14/2012 04:07 AM, Hiroshi DOYU wrote:
> Introduce a new dev_*_ratelimited() instead of pr_*_ratelimited() for
> better info to print.
>
> Signed-off-by: Hiroshi DOYU <[email protected]>

Acked-by: Stephen Warren <[email protected]>

But note that this relies on the following patch or a variant of it:

http://patchwork.ozlabs.org/patch/158899/

which isn't anywhere yet.

(A variant of that patch was patch 1/2 the first time this patch was
posted, but v2 of this patch was posted separately without mentioning
the dependency)

2012-05-14 18:57:13

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [v2 1/1] memory: tegra{20,30}-mc: Use dev_err_ratelimited()

Hi Stepehn,

Stephen Warren <[email protected]> wrote @ Mon, 14 May 2012 20:33:02 +0200:

> On 05/14/2012 04:07 AM, Hiroshi DOYU wrote:
> > Introduce a new dev_*_ratelimited() instead of pr_*_ratelimited() for
> > better info to print.
> >
> > Signed-off-by: Hiroshi DOYU <[email protected]>
>
> Acked-by: Stephen Warren <[email protected]>
>
> But note that this relies on the following patch or a variant of it:
>
> http://patchwork.ozlabs.org/patch/158899/
>
> which isn't anywhere yet.
>
> (A variant of that patch was patch 1/2 the first time this patch was
> posted, but v2 of this patch was posted separately without mentioning
> the dependency)

Greg has put the dependee patch in:

http://git.kernel.org/?p=linux/kernel/git/gregkh/driver-core.git;a=commit;h=6ca045930338485a8cdef117e74372aa1678009d

Anyway, I should have described the above dependency in commit note in
the patch, sorry for confusion.

2012-05-14 19:09:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [v2 1/1] memory: tegra{20,30}-mc: Use dev_err_ratelimited()

On Mon, May 14, 2012 at 12:33:02PM -0600, Stephen Warren wrote:
> On 05/14/2012 04:07 AM, Hiroshi DOYU wrote:
> > Introduce a new dev_*_ratelimited() instead of pr_*_ratelimited() for
> > better info to print.
> >
> > Signed-off-by: Hiroshi DOYU <[email protected]>
>
> Acked-by: Stephen Warren <[email protected]>
>
> But note that this relies on the following patch or a variant of it:
>
> http://patchwork.ozlabs.org/patch/158899/
>
> which isn't anywhere yet.
>
> (A variant of that patch was patch 1/2 the first time this patch was
> posted, but v2 of this patch was posted separately without mentioning
> the dependency)

It was part of this thread, which I don't think you were copied on for
some reason. Anyway, I applied it earlier today, thanks.

greg k-h