[PATCH 1/3] rtc: initialize rtc name early

[PATCH 1/3] rtc: initialize rtc name early

[Aaro Koskinen]

In some error cases RTC name is used before it is initialized:

	rtc-rs5c372 0-0032: clock needs to be set
	rtc-rs5c372 0-0032: rs5c372b found, 24hr, driver version 0.6
	rtc (null): read_time: fail to read
	rtc-rs5c372 0-0032: rtc core: registered rtc-rs5c372 as rtc0

Fix by initializing the name early.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/rtc/class.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 472a5ad..014ecbc 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -225,15 +225,15 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
 	rtc->pie_timer.function = rtc_pie_update_irq;
 	rtc->pie_enabled = 0;
 
+	strlcpy(rtc->name, name, RTC_DEVICE_NAME_SIZE);
+	dev_set_name(&rtc->dev, "rtc%d", id);
+
 	/* Check to see if there is an ALARM already set in hw */
 	err = __rtc_read_alarm(rtc, &alrm);
 
 	if (!err && !rtc_valid_tm(&alrm.time))
 		rtc_initialize_alarm(rtc, &alrm);
 
-	strlcpy(rtc->name, name, RTC_DEVICE_NAME_SIZE);
-	dev_set_name(&rtc->dev, "rtc%d", id);
-
 	rtc_dev_prepare(rtc);
 
 	err = device_register(&rtc->dev);
-- 
2.2.0

[PATCH 2/3] rtc: __rtc_read_time: reduce log level

[Aaro Koskinen]

__rtc_read_time logs should be debug logs instead of error logs.

For example, when the RTC clock is not set, it's not really useful
to print a kernel error log every time someone tries to read the clock:

	~ # hwclock -r
	[  604.508263] rtc rtc0: read_time: fail to read
	hwclock: RTC_RD_TIME: Invalid argument

If there's a real error, it's likely that lower level or higher level
code will tell it anyway. Make these logs debug logs, and also print
the error code for the read failure.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/rtc/interface.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 37215cf..c786818 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -31,13 +31,14 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
 		memset(tm, 0, sizeof(struct rtc_time));
 		err = rtc->ops->read_time(rtc->dev.parent, tm);
 		if (err < 0) {
-			dev_err(&rtc->dev, "read_time: fail to read\n");
+			dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
+				err);
 			return err;
 		}
 
 		err = rtc_valid_tm(tm);
 		if (err < 0)
-			dev_err(&rtc->dev, "read_time: rtc_time isn't valid\n");
+			dev_dbg(&rtc->dev, "read_time: rtc_time isn't valid\n");
 	}
 	return err;
 }
-- 
2.2.0

Re: [PATCH 2/3] rtc: __rtc_read_time: reduce log level

[Alexandre Belloni]

On 28/03/2015 at 23:09:35 +0200, Aaro Koskinen wrote :
> __rtc_read_time logs should be debug logs instead of error logs.
> 
> For example, when the RTC clock is not set, it's not really useful
> to print a kernel error log every time someone tries to read the clock:
> 
> 	~ # hwclock -r
> 	[  604.508263] rtc rtc0: read_time: fail to read
> 	hwclock: RTC_RD_TIME: Invalid argument
> 
> If there's a real error, it's likely that lower level or higher level
> code will tell it anyway. Make these logs debug logs, and also print
> the error code for the read failure.
> 

That actually may be the only error message printed for some failures.
Some RTCs don't print anything in case of error in their .read_time()
and there are in-kernel users of rtc_read_time that simply bail out
without printing anything or have a trace that is already at the debug
level.

I would agree that this would need a better harmonization and I guess we
can do that for now. I'll try to fix the in-kernel cases.

> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> ---
>  drivers/rtc/interface.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index 37215cf..c786818 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -31,13 +31,14 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
>  		memset(tm, 0, sizeof(struct rtc_time));
>  		err = rtc->ops->read_time(rtc->dev.parent, tm);
>  		if (err < 0) {
> -			dev_err(&rtc->dev, "read_time: fail to read\n");
> +			dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
> +				err);
>  			return err;
>  		}
>  
>  		err = rtc_valid_tm(tm);
>  		if (err < 0)
> -			dev_err(&rtc->dev, "read_time: rtc_time isn't valid\n");
> +			dev_dbg(&rtc->dev, "read_time: rtc_time isn't valid\n");
>  	}
>  	return err;
>  }
> -- 
> 2.2.0
> 

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

Re: [PATCH 2/3] rtc: __rtc_read_time: reduce log level

[Joe Perches]

On Wed, 2015-04-01 at 05:21 +0200, Alexandre Belloni wrote:
> On 28/03/2015 at 23:09:35 +0200, Aaro Koskinen wrote :
> > __rtc_read_time logs should be debug logs instead of error logs.
> > 
> > For example, when the RTC clock is not set, it's not really useful
> > to print a kernel error log every time someone tries to read the clock:
> > 
> > 	~ # hwclock -r
> > 	[  604.508263] rtc rtc0: read_time: fail to read
> > 	hwclock: RTC_RD_TIME: Invalid argument
> > 
> > If there's a real error, it's likely that lower level or higher level
> > code will tell it anyway. Make these logs debug logs, and also print
> > the error code for the read failure.
> > 
> 
> That actually may be the only error message printed for some failures.
> Some RTCs don't print anything in case of error in their .read_time()
> and there are in-kernel users of rtc_read_time that simply bail out
> without printing anything or have a trace that is already at the debug
> level.
> 
> I would agree that this would need a better harmonization and I guess we
> can do that for now. I'll try to fix the in-kernel cases.

Maybe these should use dev_err_once().

> > diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
[]
> > @@ -31,13 +31,14 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
> >  		memset(tm, 0, sizeof(struct rtc_time));
> >  		err = rtc->ops->read_time(rtc->dev.parent, tm);
> >  		if (err < 0) {
> > -			dev_err(&rtc->dev, "read_time: fail to read\n");
> > +			dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
> > +				err);
> >  			return err;
> >  		}
> >  
> >  		err = rtc_valid_tm(tm);
> >  		if (err < 0)
> > -			dev_err(&rtc->dev, "read_time: rtc_time isn't valid\n");
> > +			dev_dbg(&rtc->dev, "read_time: rtc_time isn't valid\n");
> >  	}
> >  	return err;
> >  }

Re: [PATCH 2/3] rtc: __rtc_read_time: reduce log level

[Alexandre Belloni]

On 31/03/2015 at 20:25:19 -0700, Joe Perches wrote :
> On Wed, 2015-04-01 at 05:21 +0200, Alexandre Belloni wrote:
> > On 28/03/2015 at 23:09:35 +0200, Aaro Koskinen wrote :
> > > __rtc_read_time logs should be debug logs instead of error logs.
> > > 
> > > For example, when the RTC clock is not set, it's not really useful
> > > to print a kernel error log every time someone tries to read the clock:
> > > 
> > > 	~ # hwclock -r
> > > 	[  604.508263] rtc rtc0: read_time: fail to read
> > > 	hwclock: RTC_RD_TIME: Invalid argument
> > > 
> > > If there's a real error, it's likely that lower level or higher level
> > > code will tell it anyway. Make these logs debug logs, and also print
> > > the error code for the read failure.
> > > 
> > 
> > That actually may be the only error message printed for some failures.
> > Some RTCs don't print anything in case of error in their .read_time()
> > and there are in-kernel users of rtc_read_time that simply bail out
> > without printing anything or have a trace that is already at the debug
> > level.
> > 
> > I would agree that this would need a better harmonization and I guess we
> > can do that for now. I'll try to fix the in-kernel cases.
> 
> Maybe these should use dev_err_once().
> 

As the issue may be sporadic, I would say that the reasoning behind the
patch is correct. Userspace is already aware that something went wrong
when reading and those debug messages are not adding any information.


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

[PATCH 3/3] rtc: hctosys: use function name in the error log

[Aaro Koskinen]

Use function name in the error log instead of __FILE__.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/rtc/hctosys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
index 6c719f2..7748a61 100644
--- a/drivers/rtc/hctosys.c
+++ b/drivers/rtc/hctosys.c
@@ -33,7 +33,7 @@ static int __init rtc_hctosys(void)
 
 	if (rtc == NULL) {
 		pr_err("%s: unable to open rtc device (%s)\n",
-			__FILE__, CONFIG_RTC_HCTOSYS_DEVICE);
+			__func__, CONFIG_RTC_HCTOSYS_DEVICE);
 		goto err_open;
 	}
 
-- 
2.2.0

Re: [PATCH 3/3] rtc: hctosys: use function name in the error log

[Alexandre Belloni]

On 28/03/2015 at 23:09:36 +0200, Aaro Koskinen wrote :
> Use function name in the error log instead of __FILE__.
> 
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> ---
>  drivers/rtc/hctosys.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
> index 6c719f2..7748a61 100644
> --- a/drivers/rtc/hctosys.c
> +++ b/drivers/rtc/hctosys.c
> @@ -33,7 +33,7 @@ static int __init rtc_hctosys(void)
>  
>  	if (rtc == NULL) {
>  		pr_err("%s: unable to open rtc device (%s)\n",
> -			__FILE__, CONFIG_RTC_HCTOSYS_DEVICE);
> +			__func__, CONFIG_RTC_HCTOSYS_DEVICE);
>  		goto err_open;
>  	}
>  
> -- 
> 2.2.0
> 

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

Re: [PATCH 3/3] rtc: hctosys: use function name in the error log

[Joe Perches]

On Wed, 2015-04-01 at 05:01 +0200, Alexandre Belloni wrote:
> On 28/03/2015 at 23:09:36 +0200, Aaro Koskinen wrote :
> > Use function name in the error log instead of __FILE__.
> > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

Hello.

> > diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
[]
>  @@ -33,7 +33,7 @@ static int __init rtc_hctosys(void)
> >  
> >  	if (rtc == NULL) {
> >  		pr_err("%s: unable to open rtc device (%s)\n",
> > -			__FILE__, CONFIG_RTC_HCTOSYS_DEVICE);
> > +			__func__, CONFIG_RTC_HCTOSYS_DEVICE);
> >  		goto err_open;
> >  	}

Neither __func__ or __FILE__ is really useful here.
The message is already specific enough without it.

If anything, it'd probably be better to add

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

to the various files in drivers/rtc that don't
already have it and remove most all of these
logging __func__ uses.

Something like:

 drivers/rtc/hctosys.c          | 6 ++++--
 drivers/rtc/rtc-cmos.c         | 6 ++++--
 drivers/rtc/rtc-ds1374.c       | 8 +++++---
 drivers/rtc/rtc-ds1685.c       | 4 +++-
 drivers/rtc/rtc-ds3232.c       | 6 ++++--
 drivers/rtc/rtc-efi-platform.c | 3 +++
 drivers/rtc/rtc-m41t80.c       | 6 ++++--
 drivers/rtc/rtc-max77686.c     | 6 ++++--
 drivers/rtc/rtc-max8997.c      | 8 +++++---
 drivers/rtc/rtc-msm6242.c      | 4 +++-
 drivers/rtc/rtc-opal.c         | 3 ++-
 drivers/rtc/rtc-s5m.c          | 4 +++-
 drivers/rtc/rtc-twl.c          | 9 +++++----
 13 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
index fb4251d..e1cfa06 100644
--- a/drivers/rtc/hctosys.c
+++ b/drivers/rtc/hctosys.c
@@ -9,6 +9,8 @@
  * published by the Free Software Foundation.
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/rtc.h>
 
 /* IMPORTANT: the RTC only stores whole seconds. It is arbitrary
@@ -32,8 +34,8 @@ static int __init rtc_hctosys(void)
 	struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
 
 	if (rtc == NULL) {
-		pr_info("%s: unable to open rtc device (%s)\n",
-			__FILE__, CONFIG_RTC_HCTOSYS_DEVICE);
+		pr_info("unable to open rtc device (%s)\n",
+			CONFIG_RTC_HCTOSYS_DEVICE);
 		goto err_open;
 	}
 
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 87647f4..a82556a 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -28,6 +28,9 @@
  * interrupts disabled, holding the global rtc_lock, to exclude those
  * other drivers and utilities on correctly configured systems.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -385,8 +388,7 @@ static bool alarm_disable_quirk;
 static int __init set_alarm_disable_quirk(const struct dmi_system_id *id)
 {
 	alarm_disable_quirk = true;
-	pr_info("rtc-cmos: BIOS has alarm-disable quirk. ");
-	pr_info("RTC alarms disabled\n");
+	pr_info("BIOS has alarm-disable quirk - RTC alarms disabled\n");
 	return 0;
 }
 
diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
index 8605fde..167783f 100644
--- a/drivers/rtc/rtc-ds1374.c
+++ b/drivers/rtc/rtc-ds1374.c
@@ -18,6 +18,8 @@
  * "Sending and receiving", using SMBus level communication is preferred.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
@@ -406,7 +408,7 @@ static int ds1374_wdt_settimeout(unsigned int timeout)
 	/* Set new watchdog time */
 	ret = ds1374_write_rtc(save_client, timeout, DS1374_REG_WDALM0, 3);
 	if (ret) {
-		pr_info("rtc-ds1374 - couldn't set new watchdog time\n");
+		pr_info("couldn't set new watchdog time\n");
 		goto out;
 	}
 
@@ -539,12 +541,12 @@ static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		if (options & WDIOS_DISABLECARD) {
-			pr_info("rtc-ds1374: disable watchdog\n");
+			pr_info("disable watchdog\n");
 			ds1374_wdt_disable();
 		}
 
 		if (options & WDIOS_ENABLECARD) {
-			pr_info("rtc-ds1374: enable watchdog\n");
+			pr_info("enable watchdog\n");
 			ds1374_wdt_settimeout(wdt_margin);
 			ds1374_wdt_ping();
 		}
diff --git a/drivers/rtc/rtc-ds1685.c b/drivers/rtc/rtc-ds1685.c
index 7020209..818a363 100644
--- a/drivers/rtc/rtc-ds1685.c
+++ b/drivers/rtc/rtc-ds1685.c
@@ -16,6 +16,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/bcd.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -2182,7 +2184,7 @@ ds1685_rtc_poweroff(struct platform_device *pdev)
 
 	/* Check for valid RTC data, else, spin forever. */
 	if (unlikely(!pdev)) {
-		pr_emerg("rtc-ds1685: platform device data not available, spinning forever ...\n");
+		pr_emerg("platform device data not available, spinning forever ...\n");
 		unreachable();
 	} else {
 		/* Get the rtc data. */
diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
index adaf06c..7e48e53 100644
--- a/drivers/rtc/rtc-ds3232.c
+++ b/drivers/rtc/rtc-ds3232.c
@@ -15,6 +15,8 @@
  * "Sending and receiving", using SMBus level communication is preferred.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
@@ -373,8 +375,8 @@ static void ds3232_work(struct work_struct *work)
 	if (stat & DS3232_REG_SR_A1F) {
 		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
 		if (control < 0) {
-			pr_warn("Read DS3232 Control Register error."
-				"Disable IRQ%d.\n", client->irq);
+			pr_warn("Read Control Register error - Disable IRQ%d\n",
+				client->irq);
 		} else {
 			/* disable alarm1 interrupt */
 			control &= ~(DS3232_REG_CR_A1IE);
diff --git a/drivers/rtc/rtc-efi-platform.c b/drivers/rtc/rtc-efi-platform.c
index b40fbe3..1a7f1d1 100644
--- a/drivers/rtc/rtc-efi-platform.c
+++ b/drivers/rtc/rtc-efi-platform.c
@@ -8,6 +8,9 @@
  * Copyright (C) 1999-2000 VA Linux Systems
  * Copyright (C) 1999-2000 Walt Drummond <drummond@valinux.com>
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 7ff7427..a82937e 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -13,6 +13,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/bcd.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -513,12 +515,12 @@ static int wdt_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		if (rv & WDIOS_DISABLECARD) {
-			pr_info("rtc-m41t80: disable watchdog\n");
+			pr_info("disable watchdog\n");
 			wdt_disable();
 		}
 
 		if (rv & WDIOS_ENABLECARD) {
-			pr_info("rtc-m41t80: enable watchdog\n");
+			pr_info("enable watchdog\n");
 			wdt_ping();
 		}
 
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 9d71328..7632a87 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -12,6 +12,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/slab.h>
 #include <linux/rtc.h>
 #include <linux/delay.h>
@@ -103,8 +105,8 @@ static int max77686_rtc_tm_to_data(struct rtc_time *tm, u8 *data)
 	data[RTC_YEAR] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
 
 	if (tm->tm_year < 100) {
-		pr_warn("%s: MAX77686 RTC cannot handle the year %d."
-			"Assume it's 2000.\n", __func__, 1900 + tm->tm_year);
+		pr_warn("RTC cannot handle the year %d.  Assume it's 2000.\n",
+			1900 + tm->tm_year);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c
index 67fbe55..9e02bcd 100644
--- a/drivers/rtc/rtc-max8997.c
+++ b/drivers/rtc/rtc-max8997.c
@@ -12,6 +12,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/slab.h>
 #include <linux/rtc.h>
 #include <linux/delay.h>
@@ -107,8 +109,8 @@ static int max8997_rtc_tm_to_data(struct rtc_time *tm, u8 *data)
 	data[RTC_YEAR] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
 
 	if (tm->tm_year < 100) {
-		pr_warn("%s: MAX8997 RTC cannot handle the year %d."
-			"Assume it's 2000.\n", __func__, 1900 + tm->tm_year);
+		pr_warn("RTC cannot handle the year %d.  Assume it's 2000.\n",
+			1900 + tm->tm_year);
 		return -EINVAL;
 	}
 	return 0;
@@ -424,7 +426,7 @@ static void max8997_rtc_enable_smpl(struct max8997_rtc_info *info, bool enable)
 
 	val = 0;
 	max8997_read_reg(info->rtc, MAX8997_RTC_WTSR_SMPL, &val);
-	pr_info("%s: WTSR_SMPL(0x%02x)\n", __func__, val);
+	pr_info("WTSR_SMPL(0x%02x)\n", val);
 }
 
 static int max8997_rtc_init_reg(struct max8997_rtc_info *info)
diff --git a/drivers/rtc/rtc-msm6242.c b/drivers/rtc/rtc-msm6242.c
index 9bf877b..c1c5c4e 100644
--- a/drivers/rtc/rtc-msm6242.c
+++ b/drivers/rtc/rtc-msm6242.c
@@ -7,6 +7,8 @@
  *  Copyright (C) 1993 Hamish Macdonald
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -111,7 +113,7 @@ static void msm6242_lock(struct msm6242_priv *priv)
 	}
 
 	if (!cnt)
-		pr_warn("msm6242: timed out waiting for RTC (0x%x)\n",
+		pr_warn("timed out waiting for RTC (0x%x)\n",
 			msm6242_read(priv, MSM6242_CD));
 }
 
diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
index 95f6521..7061dca 100644
--- a/drivers/rtc/rtc-opal.c
+++ b/drivers/rtc/rtc-opal.c
@@ -16,8 +16,9 @@
  * along with this program.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #define DRVNAME		"rtc-opal"
-#define pr_fmt(fmt)	DRVNAME ": " fmt
 
 #include <linux/module.h>
 #include <linux/err.h>
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index 1f15b67..635c472 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -15,6 +15,8 @@
  *  GNU General Public License for more details.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/bcd.h>
@@ -146,7 +148,7 @@ static int s5m8767_tm_to_data(struct rtc_time *tm, u8 *data)
 	data[RTC_YEAR1] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
 
 	if (tm->tm_year < 100) {
-		pr_err("s5m8767 RTC cannot handle the year %d.\n",
+		pr_err("RTC cannot handle the year %d\n",
 		       1900 + tm->tm_year);
 		return -EINVAL;
 	} else {
diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
index 5baea3f..2dc787d 100644
--- a/drivers/rtc/rtc-twl.c
+++ b/drivers/rtc/rtc-twl.c
@@ -18,6 +18,8 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/init.h>
@@ -145,8 +147,7 @@ static int twl_rtc_read_u8(u8 *data, u8 reg)
 
 	ret = twl_i2c_read_u8(TWL_MODULE_RTC, data, (rtc_reg_map[reg]));
 	if (ret < 0)
-		pr_err("twl_rtc: Could not read TWL"
-		       "register %X - error %d\n", reg, ret);
+		pr_err("Could not read TWL register %X - error %d\n", reg, ret);
 	return ret;
 }
 
@@ -159,8 +160,8 @@ static int twl_rtc_write_u8(u8 data, u8 reg)
 
 	ret = twl_i2c_write_u8(TWL_MODULE_RTC, data, (rtc_reg_map[reg]));
 	if (ret < 0)
-		pr_err("twl_rtc: Could not write TWL"
-		       "register %X - error %d\n", reg, ret);
+		pr_err("Could not write TWL register %X - error %d\n",
+		       reg, ret);
 	return ret;
 }
 

Re: [rtc-linux] Re: [PATCH 3/3] rtc: hctosys: use function name in the error log

[Alessandro Zummo]

On Tue, 31 Mar 2015 20:18:28 -0700
Joe Perches <joe@perches.com> wrote:

> Neither __func__ or __FILE__ is really useful here.
> The message is already specific enough without it.
> 
> If anything, it'd probably be better to add
> 
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

 Seems reasonable given that pr_fmt is widely
 used elsewhere.

 I don't like too much the need to have one define for
 each file, but it's the way it works :-/

-- 

 Best regards,

 Alessandro Zummo - CEO,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Re: [rtc-linux] Re: [PATCH 3/3] rtc: hctosys: use function name in the error log

[Joe Perches]

On Wed, 2015-04-01 at 09:05 +0200, Alessandro Zummo wrote:
> On Tue, 31 Mar 2015 20:18:28 -0700
> Joe Perches <joe@perches.com> wrote:
> 
> > Neither __func__ or __FILE__ is really useful here.
> > The message is already specific enough without it.
> > 
> > If anything, it'd probably be better to add
> > 
> > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
>  Seems reasonable given that pr_fmt is widely
>  used elsewhere.
> 
>  I don't like too much the need to have one define for
>  each file, but it's the way it works :-/

True.  Right now.

I've a desire to make a change one day so that's
the default instead of blank.

http://comments.gmane.org/gmane.linux.kernel/1052288

There are some remaining treewide complications
before that can be done effectively though.

A few files will need "#define pr_fmt(fmt) fmt"
so no prefix is used.

Re: [PATCH 3/3] rtc: hctosys: use function name in the error log

[Alexandre Belloni]

Hi,

On 31/03/2015 at 20:18:28 -0700, Joe Perches wrote :
> Neither __func__ or __FILE__ is really useful here.
> The message is already specific enough without it.
> 
> If anything, it'd probably be better to add
> 
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> to the various files in drivers/rtc that don't
> already have it and remove most all of these
> logging __func__ uses.
> 

Indeed, that seems better. Can you submit that patch?

> Something like:
> 
>  drivers/rtc/hctosys.c          | 6 ++++--
>  drivers/rtc/rtc-cmos.c         | 6 ++++--
>  drivers/rtc/rtc-ds1374.c       | 8 +++++---
>  drivers/rtc/rtc-ds1685.c       | 4 +++-
>  drivers/rtc/rtc-ds3232.c       | 6 ++++--
>  drivers/rtc/rtc-efi-platform.c | 3 +++
>  drivers/rtc/rtc-m41t80.c       | 6 ++++--
>  drivers/rtc/rtc-max77686.c     | 6 ++++--
>  drivers/rtc/rtc-max8997.c      | 8 +++++---
>  drivers/rtc/rtc-msm6242.c      | 4 +++-
>  drivers/rtc/rtc-opal.c         | 3 ++-
>  drivers/rtc/rtc-s5m.c          | 4 +++-
>  drivers/rtc/rtc-twl.c          | 9 +++++----
>  13 files changed, 49 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
> index fb4251d..e1cfa06 100644
> --- a/drivers/rtc/hctosys.c
> +++ b/drivers/rtc/hctosys.c
> @@ -9,6 +9,8 @@
>   * published by the Free Software Foundation.
>  */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/rtc.h>
>  
>  /* IMPORTANT: the RTC only stores whole seconds. It is arbitrary
> @@ -32,8 +34,8 @@ static int __init rtc_hctosys(void)
>  	struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
>  
>  	if (rtc == NULL) {
> -		pr_info("%s: unable to open rtc device (%s)\n",
> -			__FILE__, CONFIG_RTC_HCTOSYS_DEVICE);
> +		pr_info("unable to open rtc device (%s)\n",
> +			CONFIG_RTC_HCTOSYS_DEVICE);
>  		goto err_open;
>  	}
>  
> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index 87647f4..a82556a 100644
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -28,6 +28,9 @@
>   * interrupts disabled, holding the global rtc_lock, to exclude those
>   * other drivers and utilities on correctly configured systems.
>   */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
> @@ -385,8 +388,7 @@ static bool alarm_disable_quirk;
>  static int __init set_alarm_disable_quirk(const struct dmi_system_id *id)
>  {
>  	alarm_disable_quirk = true;
> -	pr_info("rtc-cmos: BIOS has alarm-disable quirk. ");
> -	pr_info("RTC alarms disabled\n");
> +	pr_info("BIOS has alarm-disable quirk - RTC alarms disabled\n");
>  	return 0;
>  }
>  
> diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
> index 8605fde..167783f 100644
> --- a/drivers/rtc/rtc-ds1374.c
> +++ b/drivers/rtc/rtc-ds1374.c
> @@ -18,6 +18,8 @@
>   * "Sending and receiving", using SMBus level communication is preferred.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
> @@ -406,7 +408,7 @@ static int ds1374_wdt_settimeout(unsigned int timeout)
>  	/* Set new watchdog time */
>  	ret = ds1374_write_rtc(save_client, timeout, DS1374_REG_WDALM0, 3);
>  	if (ret) {
> -		pr_info("rtc-ds1374 - couldn't set new watchdog time\n");
> +		pr_info("couldn't set new watchdog time\n");
>  		goto out;
>  	}
>  
> @@ -539,12 +541,12 @@ static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
>  			return -EFAULT;
>  
>  		if (options & WDIOS_DISABLECARD) {
> -			pr_info("rtc-ds1374: disable watchdog\n");
> +			pr_info("disable watchdog\n");
>  			ds1374_wdt_disable();
>  		}
>  
>  		if (options & WDIOS_ENABLECARD) {
> -			pr_info("rtc-ds1374: enable watchdog\n");
> +			pr_info("enable watchdog\n");
>  			ds1374_wdt_settimeout(wdt_margin);
>  			ds1374_wdt_ping();
>  		}
> diff --git a/drivers/rtc/rtc-ds1685.c b/drivers/rtc/rtc-ds1685.c
> index 7020209..818a363 100644
> --- a/drivers/rtc/rtc-ds1685.c
> +++ b/drivers/rtc/rtc-ds1685.c
> @@ -16,6 +16,8 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/bcd.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
> @@ -2182,7 +2184,7 @@ ds1685_rtc_poweroff(struct platform_device *pdev)
>  
>  	/* Check for valid RTC data, else, spin forever. */
>  	if (unlikely(!pdev)) {
> -		pr_emerg("rtc-ds1685: platform device data not available, spinning forever ...\n");
> +		pr_emerg("platform device data not available, spinning forever ...\n");
>  		unreachable();
>  	} else {
>  		/* Get the rtc data. */
> diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
> index adaf06c..7e48e53 100644
> --- a/drivers/rtc/rtc-ds3232.c
> +++ b/drivers/rtc/rtc-ds3232.c
> @@ -15,6 +15,8 @@
>   * "Sending and receiving", using SMBus level communication is preferred.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
> @@ -373,8 +375,8 @@ static void ds3232_work(struct work_struct *work)
>  	if (stat & DS3232_REG_SR_A1F) {
>  		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
>  		if (control < 0) {
> -			pr_warn("Read DS3232 Control Register error."
> -				"Disable IRQ%d.\n", client->irq);
> +			pr_warn("Read Control Register error - Disable IRQ%d\n",
> +				client->irq);
>  		} else {
>  			/* disable alarm1 interrupt */
>  			control &= ~(DS3232_REG_CR_A1IE);
> diff --git a/drivers/rtc/rtc-efi-platform.c b/drivers/rtc/rtc-efi-platform.c
> index b40fbe3..1a7f1d1 100644
> --- a/drivers/rtc/rtc-efi-platform.c
> +++ b/drivers/rtc/rtc-efi-platform.c
> @@ -8,6 +8,9 @@
>   * Copyright (C) 1999-2000 VA Linux Systems
>   * Copyright (C) 1999-2000 Walt Drummond <drummond@valinux.com>
>   */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
> index 7ff7427..a82937e 100644
> --- a/drivers/rtc/rtc-m41t80.c
> +++ b/drivers/rtc/rtc-m41t80.c
> @@ -13,6 +13,8 @@
>   *
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/bcd.h>
>  #include <linux/i2c.h>
>  #include <linux/init.h>
> @@ -513,12 +515,12 @@ static int wdt_ioctl(struct file *file, unsigned int cmd,
>  			return -EFAULT;
>  
>  		if (rv & WDIOS_DISABLECARD) {
> -			pr_info("rtc-m41t80: disable watchdog\n");
> +			pr_info("disable watchdog\n");
>  			wdt_disable();
>  		}
>  
>  		if (rv & WDIOS_ENABLECARD) {
> -			pr_info("rtc-m41t80: enable watchdog\n");
> +			pr_info("enable watchdog\n");
>  			wdt_ping();
>  		}
>  
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 9d71328..7632a87 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -12,6 +12,8 @@
>   *
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/slab.h>
>  #include <linux/rtc.h>
>  #include <linux/delay.h>
> @@ -103,8 +105,8 @@ static int max77686_rtc_tm_to_data(struct rtc_time *tm, u8 *data)
>  	data[RTC_YEAR] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
>  
>  	if (tm->tm_year < 100) {
> -		pr_warn("%s: MAX77686 RTC cannot handle the year %d."
> -			"Assume it's 2000.\n", __func__, 1900 + tm->tm_year);
> +		pr_warn("RTC cannot handle the year %d.  Assume it's 2000.\n",
> +			1900 + tm->tm_year);
>  		return -EINVAL;
>  	}
>  	return 0;
> diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c
> index 67fbe55..9e02bcd 100644
> --- a/drivers/rtc/rtc-max8997.c
> +++ b/drivers/rtc/rtc-max8997.c
> @@ -12,6 +12,8 @@
>   *
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/slab.h>
>  #include <linux/rtc.h>
>  #include <linux/delay.h>
> @@ -107,8 +109,8 @@ static int max8997_rtc_tm_to_data(struct rtc_time *tm, u8 *data)
>  	data[RTC_YEAR] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
>  
>  	if (tm->tm_year < 100) {
> -		pr_warn("%s: MAX8997 RTC cannot handle the year %d."
> -			"Assume it's 2000.\n", __func__, 1900 + tm->tm_year);
> +		pr_warn("RTC cannot handle the year %d.  Assume it's 2000.\n",
> +			1900 + tm->tm_year);
>  		return -EINVAL;
>  	}
>  	return 0;
> @@ -424,7 +426,7 @@ static void max8997_rtc_enable_smpl(struct max8997_rtc_info *info, bool enable)
>  
>  	val = 0;
>  	max8997_read_reg(info->rtc, MAX8997_RTC_WTSR_SMPL, &val);
> -	pr_info("%s: WTSR_SMPL(0x%02x)\n", __func__, val);
> +	pr_info("WTSR_SMPL(0x%02x)\n", val);
>  }
>  
>  static int max8997_rtc_init_reg(struct max8997_rtc_info *info)
> diff --git a/drivers/rtc/rtc-msm6242.c b/drivers/rtc/rtc-msm6242.c
> index 9bf877b..c1c5c4e 100644
> --- a/drivers/rtc/rtc-msm6242.c
> +++ b/drivers/rtc/rtc-msm6242.c
> @@ -7,6 +7,8 @@
>   *  Copyright (C) 1993 Hamish Macdonald
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> @@ -111,7 +113,7 @@ static void msm6242_lock(struct msm6242_priv *priv)
>  	}
>  
>  	if (!cnt)
> -		pr_warn("msm6242: timed out waiting for RTC (0x%x)\n",
> +		pr_warn("timed out waiting for RTC (0x%x)\n",
>  			msm6242_read(priv, MSM6242_CD));
>  }
>  
> diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
> index 95f6521..7061dca 100644
> --- a/drivers/rtc/rtc-opal.c
> +++ b/drivers/rtc/rtc-opal.c
> @@ -16,8 +16,9 @@
>   * along with this program.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #define DRVNAME		"rtc-opal"
> -#define pr_fmt(fmt)	DRVNAME ": " fmt
>  
>  #include <linux/module.h>
>  #include <linux/err.h>
> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
> index 1f15b67..635c472 100644
> --- a/drivers/rtc/rtc-s5m.c
> +++ b/drivers/rtc/rtc-s5m.c
> @@ -15,6 +15,8 @@
>   *  GNU General Public License for more details.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/bcd.h>
> @@ -146,7 +148,7 @@ static int s5m8767_tm_to_data(struct rtc_time *tm, u8 *data)
>  	data[RTC_YEAR1] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
>  
>  	if (tm->tm_year < 100) {
> -		pr_err("s5m8767 RTC cannot handle the year %d.\n",
> +		pr_err("RTC cannot handle the year %d\n",
>  		       1900 + tm->tm_year);
>  		return -EINVAL;
>  	} else {
> diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
> index 5baea3f..2dc787d 100644
> --- a/drivers/rtc/rtc-twl.c
> +++ b/drivers/rtc/rtc-twl.c
> @@ -18,6 +18,8 @@
>   * 2 of the License, or (at your option) any later version.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kernel.h>
>  #include <linux/errno.h>
>  #include <linux/init.h>
> @@ -145,8 +147,7 @@ static int twl_rtc_read_u8(u8 *data, u8 reg)
>  
>  	ret = twl_i2c_read_u8(TWL_MODULE_RTC, data, (rtc_reg_map[reg]));
>  	if (ret < 0)
> -		pr_err("twl_rtc: Could not read TWL"
> -		       "register %X - error %d\n", reg, ret);
> +		pr_err("Could not read TWL register %X - error %d\n", reg, ret);
>  	return ret;
>  }
>  
> @@ -159,8 +160,8 @@ static int twl_rtc_write_u8(u8 data, u8 reg)
>  
>  	ret = twl_i2c_write_u8(TWL_MODULE_RTC, data, (rtc_reg_map[reg]));
>  	if (ret < 0)
> -		pr_err("twl_rtc: Could not write TWL"
> -		       "register %X - error %d\n", reg, ret);
> +		pr_err("Could not write TWL register %X - error %d\n",
> +		       reg, ret);
>  	return ret;
>  }
>  
> 
> 

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

[PATCH] rtc: Use more standard kernel logging styles

[Joe Perches]

Neaten the logging a bit by adding #define pr_fmt

Miscellanea:

o Remove __FILE__/__func__ uses
o Coalesce formats adding missing spaces
o Align arguments
o (rtc-cmos) Integrated 2 consecutive messages
 
Signed-off-by: Joe Perches <joe@perches.com>
---

compiled x86, rtc-opal uncompiled, untested

 drivers/rtc/hctosys.c          | 6 ++++--
 drivers/rtc/rtc-cmos.c         | 6 ++++--
 drivers/rtc/rtc-ds1374.c       | 8 +++++---
 drivers/rtc/rtc-ds1685.c       | 4 +++-
 drivers/rtc/rtc-ds3232.c       | 6 ++++--
 drivers/rtc/rtc-efi-platform.c | 3 +++
 drivers/rtc/rtc-m41t80.c       | 6 ++++--
 drivers/rtc/rtc-max77686.c     | 6 ++++--
 drivers/rtc/rtc-max8997.c      | 8 +++++---
 drivers/rtc/rtc-msm6242.c      | 4 +++-
 drivers/rtc/rtc-opal.c         | 3 ++-
 drivers/rtc/rtc-s5m.c          | 4 +++-
 drivers/rtc/rtc-twl.c          | 9 +++++----
 13 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
index fb4251d..e1cfa06 100644
--- a/drivers/rtc/hctosys.c
+++ b/drivers/rtc/hctosys.c
@@ -9,6 +9,8 @@
  * published by the Free Software Foundation.
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/rtc.h>
 
 /* IMPORTANT: the RTC only stores whole seconds. It is arbitrary
@@ -32,8 +34,8 @@ static int __init rtc_hctosys(void)
 	struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
 
 	if (rtc == NULL) {
-		pr_info("%s: unable to open rtc device (%s)\n",
-			__FILE__, CONFIG_RTC_HCTOSYS_DEVICE);
+		pr_info("unable to open rtc device (%s)\n",
+			CONFIG_RTC_HCTOSYS_DEVICE);
 		goto err_open;
 	}
 
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 87647f4..a82556a 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -28,6 +28,9 @@
  * interrupts disabled, holding the global rtc_lock, to exclude those
  * other drivers and utilities on correctly configured systems.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -385,8 +388,7 @@ static bool alarm_disable_quirk;
 static int __init set_alarm_disable_quirk(const struct dmi_system_id *id)
 {
 	alarm_disable_quirk = true;
-	pr_info("rtc-cmos: BIOS has alarm-disable quirk. ");
-	pr_info("RTC alarms disabled\n");
+	pr_info("BIOS has alarm-disable quirk - RTC alarms disabled\n");
 	return 0;
 }
 
diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
index 8605fde..167783f 100644
--- a/drivers/rtc/rtc-ds1374.c
+++ b/drivers/rtc/rtc-ds1374.c
@@ -18,6 +18,8 @@
  * "Sending and receiving", using SMBus level communication is preferred.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
@@ -406,7 +408,7 @@ static int ds1374_wdt_settimeout(unsigned int timeout)
 	/* Set new watchdog time */
 	ret = ds1374_write_rtc(save_client, timeout, DS1374_REG_WDALM0, 3);
 	if (ret) {
-		pr_info("rtc-ds1374 - couldn't set new watchdog time\n");
+		pr_info("couldn't set new watchdog time\n");
 		goto out;
 	}
 
@@ -539,12 +541,12 @@ static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		if (options & WDIOS_DISABLECARD) {
-			pr_info("rtc-ds1374: disable watchdog\n");
+			pr_info("disable watchdog\n");
 			ds1374_wdt_disable();
 		}
 
 		if (options & WDIOS_ENABLECARD) {
-			pr_info("rtc-ds1374: enable watchdog\n");
+			pr_info("enable watchdog\n");
 			ds1374_wdt_settimeout(wdt_margin);
 			ds1374_wdt_ping();
 		}
diff --git a/drivers/rtc/rtc-ds1685.c b/drivers/rtc/rtc-ds1685.c
index 7020209..818a363 100644
--- a/drivers/rtc/rtc-ds1685.c
+++ b/drivers/rtc/rtc-ds1685.c
@@ -16,6 +16,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/bcd.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -2182,7 +2184,7 @@ ds1685_rtc_poweroff(struct platform_device *pdev)
 
 	/* Check for valid RTC data, else, spin forever. */
 	if (unlikely(!pdev)) {
-		pr_emerg("rtc-ds1685: platform device data not available, spinning forever ...\n");
+		pr_emerg("platform device data not available, spinning forever ...\n");
 		unreachable();
 	} else {
 		/* Get the rtc data. */
diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
index adaf06c..7e48e53 100644
--- a/drivers/rtc/rtc-ds3232.c
+++ b/drivers/rtc/rtc-ds3232.c
@@ -15,6 +15,8 @@
  * "Sending and receiving", using SMBus level communication is preferred.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
@@ -373,8 +375,8 @@ static void ds3232_work(struct work_struct *work)
 	if (stat & DS3232_REG_SR_A1F) {
 		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
 		if (control < 0) {
-			pr_warn("Read DS3232 Control Register error."
-				"Disable IRQ%d.\n", client->irq);
+			pr_warn("Read Control Register error - Disable IRQ%d\n",
+				client->irq);
 		} else {
 			/* disable alarm1 interrupt */
 			control &= ~(DS3232_REG_CR_A1IE);
diff --git a/drivers/rtc/rtc-efi-platform.c b/drivers/rtc/rtc-efi-platform.c
index b40fbe3..1a7f1d1 100644
--- a/drivers/rtc/rtc-efi-platform.c
+++ b/drivers/rtc/rtc-efi-platform.c
@@ -8,6 +8,9 @@
  * Copyright (C) 1999-2000 VA Linux Systems
  * Copyright (C) 1999-2000 Walt Drummond <drummond@valinux.com>
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 7ff7427..a82937e 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -13,6 +13,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/bcd.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -513,12 +515,12 @@ static int wdt_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		if (rv & WDIOS_DISABLECARD) {
-			pr_info("rtc-m41t80: disable watchdog\n");
+			pr_info("disable watchdog\n");
 			wdt_disable();
 		}
 
 		if (rv & WDIOS_ENABLECARD) {
-			pr_info("rtc-m41t80: enable watchdog\n");
+			pr_info("enable watchdog\n");
 			wdt_ping();
 		}
 
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 9d71328..7632a87 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -12,6 +12,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/slab.h>
 #include <linux/rtc.h>
 #include <linux/delay.h>
@@ -103,8 +105,8 @@ static int max77686_rtc_tm_to_data(struct rtc_time *tm, u8 *data)
 	data[RTC_YEAR] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
 
 	if (tm->tm_year < 100) {
-		pr_warn("%s: MAX77686 RTC cannot handle the year %d."
-			"Assume it's 2000.\n", __func__, 1900 + tm->tm_year);
+		pr_warn("RTC cannot handle the year %d.  Assume it's 2000.\n",
+			1900 + tm->tm_year);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c
index 67fbe55..9e02bcd 100644
--- a/drivers/rtc/rtc-max8997.c
+++ b/drivers/rtc/rtc-max8997.c
@@ -12,6 +12,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/slab.h>
 #include <linux/rtc.h>
 #include <linux/delay.h>
@@ -107,8 +109,8 @@ static int max8997_rtc_tm_to_data(struct rtc_time *tm, u8 *data)
 	data[RTC_YEAR] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
 
 	if (tm->tm_year < 100) {
-		pr_warn("%s: MAX8997 RTC cannot handle the year %d."
-			"Assume it's 2000.\n", __func__, 1900 + tm->tm_year);
+		pr_warn("RTC cannot handle the year %d.  Assume it's 2000.\n",
+			1900 + tm->tm_year);
 		return -EINVAL;
 	}
 	return 0;
@@ -424,7 +426,7 @@ static void max8997_rtc_enable_smpl(struct max8997_rtc_info *info, bool enable)
 
 	val = 0;
 	max8997_read_reg(info->rtc, MAX8997_RTC_WTSR_SMPL, &val);
-	pr_info("%s: WTSR_SMPL(0x%02x)\n", __func__, val);
+	pr_info("WTSR_SMPL(0x%02x)\n", val);
 }
 
 static int max8997_rtc_init_reg(struct max8997_rtc_info *info)
diff --git a/drivers/rtc/rtc-msm6242.c b/drivers/rtc/rtc-msm6242.c
index 9bf877b..c1c5c4e 100644
--- a/drivers/rtc/rtc-msm6242.c
+++ b/drivers/rtc/rtc-msm6242.c
@@ -7,6 +7,8 @@
  *  Copyright (C) 1993 Hamish Macdonald
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -111,7 +113,7 @@ static void msm6242_lock(struct msm6242_priv *priv)
 	}
 
 	if (!cnt)
-		pr_warn("msm6242: timed out waiting for RTC (0x%x)\n",
+		pr_warn("timed out waiting for RTC (0x%x)\n",
 			msm6242_read(priv, MSM6242_CD));
 }
 
diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
index 95f6521..7061dca 100644
--- a/drivers/rtc/rtc-opal.c
+++ b/drivers/rtc/rtc-opal.c
@@ -16,8 +16,9 @@
  * along with this program.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #define DRVNAME		"rtc-opal"
-#define pr_fmt(fmt)	DRVNAME ": " fmt
 
 #include <linux/module.h>
 #include <linux/err.h>
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index 1f15b67..635c472 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -15,6 +15,8 @@
  *  GNU General Public License for more details.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/bcd.h>
@@ -146,7 +148,7 @@ static int s5m8767_tm_to_data(struct rtc_time *tm, u8 *data)
 	data[RTC_YEAR1] = tm->tm_year > 100 ? (tm->tm_year - 100) : 0;
 
 	if (tm->tm_year < 100) {
-		pr_err("s5m8767 RTC cannot handle the year %d.\n",
+		pr_err("RTC cannot handle the year %d\n",
 		       1900 + tm->tm_year);
 		return -EINVAL;
 	} else {
diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
index 5baea3f..2dc787d 100644
--- a/drivers/rtc/rtc-twl.c
+++ b/drivers/rtc/rtc-twl.c
@@ -18,6 +18,8 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/init.h>
@@ -145,8 +147,7 @@ static int twl_rtc_read_u8(u8 *data, u8 reg)
 
 	ret = twl_i2c_read_u8(TWL_MODULE_RTC, data, (rtc_reg_map[reg]));
 	if (ret < 0)
-		pr_err("twl_rtc: Could not read TWL"
-		       "register %X - error %d\n", reg, ret);
+		pr_err("Could not read TWL register %X - error %d\n", reg, ret);
 	return ret;
 }
 
@@ -159,8 +160,8 @@ static int twl_rtc_write_u8(u8 data, u8 reg)
 
 	ret = twl_i2c_write_u8(TWL_MODULE_RTC, data, (rtc_reg_map[reg]));
 	if (ret < 0)
-		pr_err("twl_rtc: Could not write TWL"
-		       "register %X - error %d\n", reg, ret);
+		pr_err("Could not write TWL register %X - error %d\n",
+		       reg, ret);
 	return ret;
 }
 

Re: [rtc-linux] [PATCH] rtc: Use more standard kernel logging styles

[Krzysztof Kozlowski]

2015-04-01 18:12 GMT+02:00 Joe Perches <joe@perches.com>:
> Neaten the logging a bit by adding #define pr_fmt
>
> Miscellanea:
>
> o Remove __FILE__/__func__ uses
> o Coalesce formats adding missing spaces
> o Align arguments
> o (rtc-cmos) Integrated 2 consecutive messages
>
> Signed-off-by: Joe Perches <joe@perches.com>

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

[Tested on Arndale Octa (rtc-s5m) and Trats2 (rtc-max77686)]
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH] rtc: Use more standard kernel logging styles

[Alexandre Belloni]

On 01/04/2015 at 09:12:51 -0700, Joe Perches wrote :
> Neaten the logging a bit by adding #define pr_fmt
> 
> Miscellanea:
> 
> o Remove __FILE__/__func__ uses
> o Coalesce formats adding missing spaces
> o Align arguments
> o (rtc-cmos) Integrated 2 consecutive messages
>  
> Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

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

Re: [PATCH 1/3] rtc: initialize rtc name early

[Alexandre Belloni]

On 28/03/2015 at 23:09:34 +0200, Aaro Koskinen wrote :
> In some error cases RTC name is used before it is initialized:
> 
> 	rtc-rs5c372 0-0032: clock needs to be set
> 	rtc-rs5c372 0-0032: rs5c372b found, 24hr, driver version 0.6
> 	rtc (null): read_time: fail to read
> 	rtc-rs5c372 0-0032: rtc core: registered rtc-rs5c372 as rtc0
> 
> Fix by initializing the name early.
> 
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> ---
>  drivers/rtc/class.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index 472a5ad..014ecbc 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -225,15 +225,15 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
>  	rtc->pie_timer.function = rtc_pie_update_irq;
>  	rtc->pie_enabled = 0;
>  
> +	strlcpy(rtc->name, name, RTC_DEVICE_NAME_SIZE);
> +	dev_set_name(&rtc->dev, "rtc%d", id);
> +
>  	/* Check to see if there is an ALARM already set in hw */
>  	err = __rtc_read_alarm(rtc, &alrm);
>  
>  	if (!err && !rtc_valid_tm(&alrm.time))
>  		rtc_initialize_alarm(rtc, &alrm);
>  
> -	strlcpy(rtc->name, name, RTC_DEVICE_NAME_SIZE);
> -	dev_set_name(&rtc->dev, "rtc%d", id);
> -
>  	rtc_dev_prepare(rtc);
>  
>  	err = device_register(&rtc->dev);
> -- 
> 2.2.0
> 

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