Skip to contentSkip to conversation

Provides bug fixes for rtc-mcp795.c

PATCH Superseded Emil Bartczak 2 participants 13 messages Dec 5, 2016 Real time clock (RTC)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Emil Bartczak

author
#0 lore raw

cover Provides bug fixes for rtc-mcp795.c

This patchset provides 3 bug fixes (patch 1, 2 and 3) and one improvement (patch 4) in the file driver/rtc/rtc-mcp795.c. Please review the changes and consider to apply them to the main kernel tree.

Emil Bartczak (4):
rtc: mcp795: fix invalid month setting.
rtc: mcp795: fix time range difference between linux and RTC chip
rtc: mcp795: fix month write resetting date to 1.
rtc: mcp795: use bcd2bin/bin2bcd.

 drivers/rtc/rtc-mcp795.c | 125 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 94 insertions(+), 31 deletions(-)
Signature · 11 lines
2.7.4

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

PATCH 1/4 rtc: mcp795: fix invalid month setting.

The 10 month register was always set to value 0 in the RTC hardware. Due to the bug month November or December became February.

---
 drivers/rtc/rtc-mcp795.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/rtc/rtc-mcp795.c +7 -5
index 4021fd0..266328b 100644
@@ -29,7 +29,7 @@
 #define MCP795_EEWREN	0x06
 #define MCP795_SRREAD	0x05
 #define MCP795_SRWRITE	0x01
-#define MCP795_READ		0x13
+#define MCP795_READ	0x13
 #define MCP795_WRITE	0x12
 #define MCP795_UNLOCK	0x14
 #define MCP795_IDWRITE	0x32

@@ -39,6 +39,7 @@
 
 #define MCP795_ST_BIT	0x80
 #define MCP795_24_BIT	0x40
+#define MCP795_LP_BIT	0x20
 
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

@@ -108,7 +109,8 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
 	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
 	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
-	data[5] = (data[5] & 0x10) | (tim->tm_mon / 10) | (tim->tm_mon % 10);
+	data[5] = (data[5] & MCP795_LP_BIT) |
+			((tim->tm_mon / 10) << 4) | (tim->tm_mon % 10);
 
 	if (tim->tm_year > 100)
 		tim->tm_year -= 100;

@@ -137,11 +139,11 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	if (ret)
 		return ret;
 
-	tim->tm_sec		= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
-	tim->tm_min		= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
+	tim->tm_sec	= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
+	tim->tm_min	= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
 	tim->tm_hour	= ((data[2] & 0x30) >> 4) * 10 + (data[2] & 0x0f);
 	tim->tm_mday	= ((data[4] & 0x30) >> 4) * 10 + (data[4] & 0x0f);
-	tim->tm_mon		= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);
+	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);
 	tim->tm_year	= ((data[6] & 0xf0) >> 4) * 10 + (data[6] & 0x0f) + 100; /* Assume we are in 20xx */
 
 	dev_dbg(dev, "Read from mcp795: %04d-%02d-%02d %02d:%02d:%02d\n",
Signature · 11 lines
2.7.4

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

Re: [PATCH 1/4] rtc: mcp795: fix invalid month setting.

Hi,

Quoted 2 lines · Emil Bartczak

The 10 month register was always set to value 0 in the RTC hardware. Due to the bug month November or December became February.

All your patches are missing your SoB, see Developer's Certificate of Origin 1.1 in Documentation/SubmittingPatches

Quoted 9 lines
---
 drivers/rtc/rtc-mcp795.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/rtc/rtc-mcp795.c +0 -0
index 4021fd0..266328b 100644

drivers/rtc/rtc-mcp795.c line 32

Quoted 5 lines
drivers/rtc/rtc-mcp795.c +1 -1
@@ -29,7 +29,7 @@
 #define MCP795_EEWREN	0x06
 #define MCP795_SRREAD	0x05
 #define MCP795_SRWRITE	0x01
-#define MCP795_READ		0x13
+#define MCP795_READ	0x13

Unrelated change

Quoted 12 lines
drivers/rtc/rtc-mcp795.c +1 -0
 #define MCP795_WRITE	0x12
 #define MCP795_UNLOCK	0x14
 #define MCP795_IDWRITE	0x32

@@ -39,6 +39,7 @@
 
 #define MCP795_ST_BIT	0x80
 #define MCP795_24_BIT	0x40
+#define MCP795_LP_BIT	0x20
 
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

drivers/rtc/rtc-mcp795.c static int mcp795_set_time(struct device *dev, struct rtc_time *tim) · line 112

Quoted 5 lines
drivers/rtc/rtc-mcp795.c +1 -1
@@ -108,7 +109,8 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
 	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
 	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
-	data[5] = (data[5] & 0x10) | (tim->tm_mon / 10) | (tim->tm_mon % 10);
+	data[5] = (data[5] & MCP795_LP_BIT) |

You changed 0x10 in MCP795_LP_BIT which you defined as 0x20, is that right?

This is also an unrelated change.

Quoted 6 lines
drivers/rtc/rtc-mcp795.c +1 -0
+			((tim->tm_mon / 10) << 4) | (tim->tm_mon % 10);
 
 	if (tim->tm_year > 100)
 		tim->tm_year -= 100;

@@ -137,11 +139,11 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	if (ret)

drivers/rtc/rtc-mcp795.c static int mcp795_read_time(struct device *dev, struct rtc_time *tim) · line 146

Quoted 10 lines
drivers/rtc/rtc-mcp795.c +3 -3
 		return ret;
 
-	tim->tm_sec		= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
-	tim->tm_min		= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
+	tim->tm_sec	= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
+	tim->tm_min	= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
 	tim->tm_hour	= ((data[2] & 0x30) >> 4) * 10 + (data[2] & 0x0f);
 	tim->tm_mday	= ((data[4] & 0x30) >> 4) * 10 + (data[4] & 0x0f);
-	tim->tm_mon		= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);
+	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);

All those whitespace changes are actually confusing. Please do them in a separate patch or in your last patch.

Quoted 6 lines
drivers/rtc/rtc-mcp795.c +0 -0
 	tim->tm_year	= ((data[6] & 0xf0) >> 4) * 10 + (data[6] & 0x0f) + 100; /* Assume we are in 20xx */
 
 	dev_dbg(dev, "Read from mcp795: %04d-%02d-%02d %02d:%02d:%02d\n",
Signature · 1 line
2.7.4
Signature · 13 lines
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH 1/4] rtc: mcp795: fix invalid month setting.

Hi,

Quoted 8 lines · Alexandre Belloni

Hi,

Quoted 2 lines · Emil Bartczak

The 10 month register was always set to value 0 in the RTC hardware. Due to the bug month November or December became February.

All your patches are missing your SoB, see Developer's Certificate of Origin 1.1 in Documentation/SubmittingPatches

Ok, I will fix it in the next patchset. Sorry for that simple mistakes but I'm newbie in sending patches.

Quoted 5 lines
Quoted 5 lines
---
 drivers/rtc/rtc-mcp795.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
Quoted 12 lines

drivers/rtc/rtc-mcp795.c line 32

Quoted 10 lines
drivers/rtc/rtc-mcp795.c +1 -1
index 4021fd0..266328b 100644
@@ -29,7 +29,7 @@
 #define MCP795_EEWREN	0x06
 #define MCP795_SRREAD	0x05
 #define MCP795_SRWRITE	0x01
-#define MCP795_READ		0x13
+#define MCP795_READ	0x13

Unrelated change

Ok, I will remove it from patch.

Quoted 10 lines
Quoted 10 lines
drivers/rtc/rtc-mcp795.c +1 -0
 #define MCP795_WRITE	0x12
 #define MCP795_UNLOCK	0x14
 #define MCP795_IDWRITE	0x32

@@ -39,6 +39,7 @@
 
 #define MCP795_ST_BIT	0x80
 #define MCP795_24_BIT	0x40
+#define MCP795_LP_BIT	0x20
Quoted 11 lines

drivers/rtc/rtc-mcp795.c static int mcp795_set_time(struct device *dev, struct rtc_time *tim) · line 112

Quoted 8 lines
drivers/rtc/rtc-mcp795.c +1 -1
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

@@ -108,7 +109,8 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
 	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
 	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
-	data[5] = (data[5] & 0x10) | (tim->tm_mon / 10) | (tim->tm_mon % 10);
+	data[5] = (data[5] & MCP795_LP_BIT) |

You changed 0x10 in MCP795_LP_BIT which you defined as 0x20, is that right?

Yes, it should be 0x20 (checked in datasheet).

Quoted 4 lines

This is also an unrelated change.

drivers/rtc/rtc-mcp795.c static int mcp795_set_time(struct device *dev, struct rtc_time *tim) · line 113

Quoted 1 line
drivers/rtc/rtc-mcp795.c +1 -0
+			((tim->tm_mon / 10) << 4) | (tim->tm_mon % 10);

What do you mean exactly?
That above line of code was moved to the new line? Or that I added shift left operation (tim->tm_mon / 10) << 4)?
Changing 0x10 to 0x20 and adding shift right operation fixes the problem.

Quoted 7 lines
Quoted 7 lines
drivers/rtc/rtc-mcp795.c +0 -0
 	if (tim->tm_year > 100)
 		tim->tm_year -= 100;

@@ -137,11 +139,11 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	if (ret)
 		return ret;
Quoted 11 lines

drivers/rtc/rtc-mcp795.c static int mcp795_read_time(struct device *dev, struct rtc_time *tim) · line 146

Quoted 8 lines
drivers/rtc/rtc-mcp795.c +3 -3
-	tim->tm_sec		= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
-	tim->tm_min		= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
+	tim->tm_sec	= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
+	tim->tm_min	= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
 	tim->tm_hour	= ((data[2] & 0x30) >> 4) * 10 + (data[2] & 0x0f);
 	tim->tm_mday	= ((data[4] & 0x30) >> 4) * 10 + (data[4] & 0x0f);
-	tim->tm_mon		= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);
+	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);

All those whitespace changes are actually confusing. Please do them in a separate patch or in your last patch.

Ok, I will have a separate patch for them.

Quoted 12 lines
Quoted 7 lines
drivers/rtc/rtc-mcp795.c +0 -0
 	tim->tm_year	= ((data[6] & 0xf0) >> 4) * 10 + (data[6] & 0x0f) + 100; /* Assume we are in 20xx */
 
 	dev_dbg(dev, "Read from mcp795: %04d-%02d-%02d %02d:%02d:%02d\n",
Signature · 1 line
2.7.4
Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Cheers,
Emil

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

Re: [PATCH 1/4] rtc: mcp795: fix invalid month setting.

Quoted 24 lines · Emil Bartczak
Quoted 10 lines
Quoted 10 lines

#define MCP795_WRITE 0x12
#define MCP795_UNLOCK 0x14
#define MCP795_IDWRITE 0x32

drivers/rtc/rtc-mcp795.c +1 -0
@@ -39,6 +39,7 @@
 
 #define MCP795_ST_BIT	0x80
 #define MCP795_24_BIT	0x40
+#define MCP795_LP_BIT	0x20
Quoted 11 lines

drivers/rtc/rtc-mcp795.c static int mcp795_set_time(struct device *dev, struct rtc_time *tim) · line 112

Quoted 8 lines
drivers/rtc/rtc-mcp795.c +1 -1
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

@@ -108,7 +109,8 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
 	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
 	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
-	data[5] = (data[5] & 0x10) | (tim->tm_mon / 10) | (tim->tm_mon % 10);
+	data[5] = (data[5] & MCP795_LP_BIT) |

You changed 0x10 in MCP795_LP_BIT which you defined as 0x20, is that right?

Yes, it should be 0x20 (checked in datasheet).

Quoted 1 line
Quoted 8 lines
Quoted 3 lines

This is also an unrelated change.

drivers/rtc/rtc-mcp795.c static int mcp795_set_time(struct device *dev, struct rtc_time *tim) · line 113

Quoted 1 line
drivers/rtc/rtc-mcp795.c +1 -0
+			((tim->tm_mon / 10) << 4) | (tim->tm_mon % 10);

What do you mean exactly?
That above line of code was moved to the new line? Or that I added shift left operation (tim->tm_mon / 10) << 4)?
Changing 0x10 to 0x20 and adding shift right operation fixes the problem.

I meant that I feel like changing 0x10 to 0x20 is a separate bugfix from adding the shift. At least mention that in the commit message.

Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
drivers/rtc/rtc-mcp795.c +0 -0
 	if (tim->tm_year > 100)
 		tim->tm_year -= 100;

@@ -137,11 +139,11 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	if (ret)
 		return ret;
Quoted 11 lines

drivers/rtc/rtc-mcp795.c static int mcp795_read_time(struct device *dev, struct rtc_time *tim) · line 146

Quoted 8 lines
drivers/rtc/rtc-mcp795.c +3 -3
-	tim->tm_sec		= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
-	tim->tm_min		= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
+	tim->tm_sec	= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
+	tim->tm_min	= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
 	tim->tm_hour	= ((data[2] & 0x30) >> 4) * 10 + (data[2] & 0x0f);
 	tim->tm_mday	= ((data[4] & 0x30) >> 4) * 10 + (data[4] & 0x0f);
-	tim->tm_mon		= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);
+	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);

All those whitespace changes are actually confusing. Please do them in a separate patch or in your last patch.

Quoted 1 line

Ok, I will have a separate patch for them.

Maybe switching to bcd2bin/bin2bcd first is better as it touches all those lines anyway and also solves the shift in mcp795_rtcc_read()

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

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

Re: [PATCH 1/4] rtc: mcp795: fix invalid month setting.

Quoted 34 lines · Alexandre Belloni
Quoted 24 lines · Emil Bartczak
Quoted 10 lines
Quoted 10 lines

#define MCP795_WRITE 0x12
#define MCP795_UNLOCK 0x14
#define MCP795_IDWRITE 0x32

drivers/rtc/rtc-mcp795.c +1 -0
@@ -39,6 +39,7 @@
 
 #define MCP795_ST_BIT	0x80
 #define MCP795_24_BIT	0x40
+#define MCP795_LP_BIT	0x20
Quoted 11 lines

drivers/rtc/rtc-mcp795.c static int mcp795_set_time(struct device *dev, struct rtc_time *tim) · line 112

Quoted 8 lines
drivers/rtc/rtc-mcp795.c +1 -1
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

@@ -108,7 +109,8 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
 	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
 	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
-	data[5] = (data[5] & 0x10) | (tim->tm_mon / 10) | (tim->tm_mon % 10);
+	data[5] = (data[5] & MCP795_LP_BIT) |

You changed 0x10 in MCP795_LP_BIT which you defined as 0x20, is that right?

Yes, it should be 0x20 (checked in datasheet).

Quoted 1 line
Quoted 8 lines
Quoted 3 lines

This is also an unrelated change.

drivers/rtc/rtc-mcp795.c static int mcp795_set_time(struct device *dev, struct rtc_time *tim) · line 113

Quoted 1 line
drivers/rtc/rtc-mcp795.c +1 -0
+			((tim->tm_mon / 10) << 4) | (tim->tm_mon % 10);

What do you mean exactly?
That above line of code was moved to the new line? Or that I added shift left operation (tim->tm_mon / 10) << 4)?
Changing 0x10 to 0x20 and adding shift right operation fixes the problem.

Quoted 2 lines

I meant that I feel like changing 0x10 to 0x20 is a separate bugfix from adding the shift. At least mention that in the commit message.

Ok, I will improve commit message.

Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
Quoted 6 lines
drivers/rtc/rtc-mcp795.c +0 -0
 	if (tim->tm_year > 100)
 		tim->tm_year -= 100;

@@ -137,11 +139,11 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	if (ret)

drivers/rtc/rtc-mcp795.c static int mcp795_read_time(struct device *dev, struct rtc_time *tim) · line 146

Quoted 10 lines
drivers/rtc/rtc-mcp795.c +3 -3
 		return ret;
 
-	tim->tm_sec		= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
-	tim->tm_min		= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
+	tim->tm_sec	= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
+	tim->tm_min	= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
 	tim->tm_hour	= ((data[2] & 0x30) >> 4) * 10 + (data[2] & 0x0f);
 	tim->tm_mday	= ((data[4] & 0x30) >> 4) * 10 + (data[4] & 0x0f);
-	tim->tm_mon		= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);
+	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);

All those whitespace changes are actually confusing. Please do them in a separate patch or in your last patch.

Quoted 4 lines
Quoted 1 line

Ok, I will have a separate patch for them.

Maybe switching to bcd2bin/bin2bcd first is better as it touches all those lines anyway and also solves the shift in mcp795_rtcc_read()

Yes, this is a good idea. I will prepare a new patchset where first patch will provide switching to bcd2bin/bin2bcd.

Quoted 6 lines
Signature · 4 lines
-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Emil,

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

PATCH 2/4 rtc: mcp795: fix time range difference between linux and RTC chip

In reply to Emil Bartczak

In linux rtc_time struct, tm_mon range is 0~11, while in RTC HW REG, month range is 1~12. This patch adjusts difference of them.

---
 drivers/rtc/rtc-mcp795.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/rtc/rtc-mcp795.c +4 -2
index 266328b..d15072c 100644
@@ -96,6 +96,7 @@ static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state)
 
 static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 {
+	int month;
 	int ret;
 	u8 data[7];
 

@@ -109,8 +110,9 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
 	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
 	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
+	month = tim->tm_mon + 1;
 	data[5] = (data[5] & MCP795_LP_BIT) |
-			((tim->tm_mon / 10) << 4) | (tim->tm_mon % 10);
+			((month / 10) << 4) | (month % 10);
 
 	if (tim->tm_year > 100)
 		tim->tm_year -= 100;

@@ -143,7 +145,7 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	tim->tm_min	= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
 	tim->tm_hour	= ((data[2] & 0x30) >> 4) * 10 + (data[2] & 0x0f);
 	tim->tm_mday	= ((data[4] & 0x30) >> 4) * 10 + (data[4] & 0x0f);
-	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f);
+	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f) - 1;
 	tim->tm_year	= ((data[6] & 0xf0) >> 4) * 10 + (data[6] & 0x0f) + 100; /* Assume we are in 20xx */
 
 	dev_dbg(dev, "Read from mcp795: %04d-%02d-%02d %02d:%02d:%02d\n",
Signature · 11 lines
2.7.4

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

PATCH 3/4 rtc: mcp795: fix month write resetting date to 1.

In reply to Emil Bartczak

According to Microchip errata some combinations of date and month values may result in the date being reset to 1, even if the date is also written with the month (for example 31-07 or 31-08). As a workaround avoid writing date and month values within the same Write command. Instead, terminate the Write command after loading the date and begin a new command to write the month. In addition, disable the oscillator before loading the new values. This is done by ensuring both the ST and EXTOSC bits are cleared and waiting for the OSCON bit to clear.

---
 drivers/rtc/rtc-mcp795.c | 103 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 82 insertions(+), 21 deletions(-)
drivers/rtc/rtc-mcp795.c +82 -21
index d15072c..c9ad46c 100644
@@ -21,25 +21,34 @@
 #include <linux/spi/spi.h>
 #include <linux/rtc.h>
 #include <linux/of.h>
+#include <linux/delay.h>
 
 /* MCP795 Instructions, see datasheet table 3-1 */
-#define MCP795_EEREAD	0x03
-#define MCP795_EEWRITE	0x02
-#define MCP795_EEWRDI	0x04
-#define MCP795_EEWREN	0x06
-#define MCP795_SRREAD	0x05
-#define MCP795_SRWRITE	0x01
-#define MCP795_READ	0x13
-#define MCP795_WRITE	0x12
-#define MCP795_UNLOCK	0x14
-#define MCP795_IDWRITE	0x32
-#define MCP795_IDREAD	0x33
-#define MCP795_CLRWDT	0x44
-#define MCP795_CLRRAM	0x54
-
-#define MCP795_ST_BIT	0x80
-#define MCP795_24_BIT	0x40
-#define MCP795_LP_BIT	0x20
+#define MCP795_EEREAD		0x03
+#define MCP795_EEWRITE		0x02
+#define MCP795_EEWRDI		0x04
+#define MCP795_EEWREN		0x06
+#define MCP795_SRREAD		0x05
+#define MCP795_SRWRITE		0x01
+#define MCP795_READ		0x13
+#define MCP795_WRITE		0x12
+#define MCP795_UNLOCK		0x14
+#define MCP795_IDWRITE		0x32
+#define MCP795_IDREAD		0x33
+#define MCP795_CLRWDT		0x44
+#define MCP795_CLRRAM		0x54
+
+/* MCP795 RTCC registers, see datasheet table 4-1 */
+#define MCP795_REG_SECONDS	0x01
+#define MCP795_REG_DAY		0x04
+#define MCP795_REG_MONTH	0x06
+#define MCP795_REG_CONTROL	0x08
+
+#define MCP795_ST_BIT		0x80
+#define MCP795_24_BIT		0x40
+#define MCP795_LP_BIT		0x20
+#define MCP795_EXTOSC_BIT	0x08
+#define MCP795_OSCON_BIT	0x20
 
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

@@ -94,14 +103,51 @@ static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state)
 	return ret;
 }
 
+static int mcp795_stop_oscillator(struct device *dev)
+{
+	int retries = 5;
+	int ret;
+	u8 data;
+
+	ret = mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS, MCP795_ST_BIT, 0);
+	if (ret)
+		return ret;
+	ret = mcp795_rtcc_set_bits(dev, MCP795_REG_CONTROL, MCP795_EXTOSC_BIT, 0);
+	if (ret)
+		return ret;
+	do {
+		usleep_range(700, 800);
+		ret = mcp795_rtcc_read(dev, MCP795_REG_DAY,
+					&data, sizeof(data));
+		if (ret)
+			break;
+		if (!(data & MCP795_OSCON_BIT))
+			break;
+
+	} while (--retries);
+
+	return !retries ? -EIO : ret;
+}
+
+static int mcp795_start_oscillator(struct device *dev)
+{
+	return mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS,
+					MCP795_ST_BIT, MCP795_ST_BIT);
+}
+
 static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 {
 	int month;
 	int ret;
 	u8 data[7];
 
+	/* Stop RTC while updating the registers */
+	ret = mcp795_stop_oscillator(dev);
+	if (ret)
+		return ret;
+
 	/* Read first, so we can leave config bits untouched */
-	ret = mcp795_rtcc_read(dev, 0x01, data, sizeof(data));
+	ret = mcp795_rtcc_read(dev, MCP795_REG_SECONDS, data, sizeof(data));
 
 	if (ret)
 		return ret;

@@ -110,6 +156,16 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
 	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
 	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
+
+	/* Always write the date and month using a separate Write command.
+	 * This is a workaround for a know silicon issue that some combinations
+	 * of date and month values may result in the date being reset to 1.
+	 */
+	ret = mcp795_rtcc_write(dev, MCP795_REG_SECONDS, data, 5);
+
+	if (ret)
+		return ret;
+
 	month = tim->tm_mon + 1;
 	data[5] = (data[5] & MCP795_LP_BIT) |
 			((month / 10) << 4) | (month % 10);

@@ -119,8 +175,13 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 
 	data[6] = ((tim->tm_year / 10) << 4) | (tim->tm_year % 10);
 
-	ret = mcp795_rtcc_write(dev, 0x01, data, sizeof(data));
+	ret = mcp795_rtcc_write(dev, MCP795_REG_MONTH, &data[5], 2);
+
+	if (ret)
+		return ret;
 
+	/* Start back RTC */
+	ret = mcp795_start_oscillator(dev);
 	if (ret)
 		return ret;
 

@@ -136,7 +197,7 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	int ret;
 	u8 data[7];
 
-	ret = mcp795_rtcc_read(dev, 0x01, data, sizeof(data));
+	ret = mcp795_rtcc_read(dev, MCP795_REG_SECONDS, data, sizeof(data));
 
 	if (ret)
 		return ret;

@@ -174,7 +235,7 @@ static int mcp795_probe(struct spi_device *spi)
 	}
 
 	/* Start the oscillator */
-	mcp795_rtcc_set_bits(&spi->dev, 0x01, MCP795_ST_BIT, MCP795_ST_BIT);
+	mcp795_start_oscillator(&spi->dev);
 	/* Clear the 12 hour mode flag*/
 	mcp795_rtcc_set_bits(&spi->dev, 0x03, MCP795_24_BIT, 0);
Signature · 11 lines
2.7.4

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

Re: [PATCH 3/4] rtc: mcp795: fix month write resetting date to 1.

Quoted 45 lines · Emil Bartczak

According to Microchip errata some combinations of date and month values may result in the date being reset to 1, even if the date is also written with the month (for example 31-07 or 31-08). As a workaround avoid writing date and month values within the same Write command. Instead, terminate the Write command after loading the date and begin a new command to write the month. In addition, disable the oscillator before loading the new values. This is done by ensuring both the ST and EXTOSC bits are cleared and waiting for the OSCON bit to clear.

---
 drivers/rtc/rtc-mcp795.c | 103 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 82 insertions(+), 21 deletions(-)
drivers/rtc/rtc-mcp795.c +5 -17
index d15072c..c9ad46c 100644
@@ -21,25 +21,34 @@
 #include <linux/spi/spi.h>
 #include <linux/rtc.h>
 #include <linux/of.h>
+#include <linux/delay.h>
 
 /* MCP795 Instructions, see datasheet table 3-1 */
-#define MCP795_EEREAD	0x03
-#define MCP795_EEWRITE	0x02
-#define MCP795_EEWRDI	0x04
-#define MCP795_EEWREN	0x06
-#define MCP795_SRREAD	0x05
-#define MCP795_SRWRITE	0x01
-#define MCP795_READ	0x13
-#define MCP795_WRITE	0x12
-#define MCP795_UNLOCK	0x14
-#define MCP795_IDWRITE	0x32
-#define MCP795_IDREAD	0x33
-#define MCP795_CLRWDT	0x44
-#define MCP795_CLRRAM	0x54
-
-#define MCP795_ST_BIT	0x80
-#define MCP795_24_BIT	0x40
-#define MCP795_LP_BIT	0x20
+#define MCP795_EEREAD		0x03
+#define MCP795_EEWRITE		0x02
+#define MCP795_EEWRDI		0x04
+#define MCP795_EEWREN		0x06

drivers/rtc/rtc-mcp795.c line 40

Quoted 10 lines
drivers/rtc/rtc-mcp795.c +10 -0
+#define MCP795_SRREAD		0x05
+#define MCP795_SRWRITE		0x01
+#define MCP795_READ		0x13
+#define MCP795_WRITE		0x12
+#define MCP795_UNLOCK		0x14
+#define MCP795_IDWRITE		0x32
+#define MCP795_IDREAD		0x33
+#define MCP795_CLRWDT		0x44
+#define MCP795_CLRRAM		0x54
+

Please don't reindent, they are a separate block anyway.

drivers/rtc/rtc-mcp795.c line 51

Quoted 11 lines
drivers/rtc/rtc-mcp795.c +11 -0
+/* MCP795 RTCC registers, see datasheet table 4-1 */
+#define MCP795_REG_SECONDS	0x01
+#define MCP795_REG_DAY		0x04
+#define MCP795_REG_MONTH	0x06
+#define MCP795_REG_CONTROL	0x08
+
+#define MCP795_ST_BIT		0x80
+#define MCP795_24_BIT		0x40
+#define MCP795_LP_BIT		0x20
+#define MCP795_EXTOSC_BIT	0x08
+#define MCP795_OSCON_BIT	0x20

Please use BIT() and that is valid for the first patch too).

Quoted 27 lines
drivers/rtc/rtc-mcp795.c +20 -0
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

@@ -94,14 +103,51 @@ static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state)
 	return ret;
 }
 
+static int mcp795_stop_oscillator(struct device *dev)
+{
+	int retries = 5;
+	int ret;
+	u8 data;
+
+	ret = mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS, MCP795_ST_BIT, 0);
+	if (ret)
+		return ret;
+	ret = mcp795_rtcc_set_bits(dev, MCP795_REG_CONTROL, MCP795_EXTOSC_BIT, 0);
+	if (ret)
+		return ret;
+	do {
+		usleep_range(700, 800);
+		ret = mcp795_rtcc_read(dev, MCP795_REG_DAY,
+					&data, sizeof(data));
+		if (ret)
+			break;
+		if (!(data & MCP795_OSCON_BIT))
+			break;

drivers/rtc/rtc-mcp795.c static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state) · line 135

Quoted 10 lines
drivers/rtc/rtc-mcp795.c +10 -0
+
+	} while (--retries);
+
+	return !retries ? -EIO : ret;
+}
+
+static int mcp795_start_oscillator(struct device *dev)
+{
+	return mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS,
+					MCP795_ST_BIT, MCP795_ST_BIT);

You probably want to restore EXTOSC to its previous value here.

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

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

Re: [PATCH 3/4] rtc: mcp795: fix month write resetting date to 1.

Hi,

Quoted 50 lines · Alexandre Belloni
Quoted 49 lines · Emil Bartczak

According to Microchip errata some combinations of date and month values may result in the date being reset to 1, even if the date is also written with the month (for example 31-07 or 31-08). As a workaround avoid writing date and month values within the same Write command. Instead, terminate the Write command after loading the date and begin a new command to write the month. In addition, disable the oscillator before loading the new values. This is done by ensuring both the ST and EXTOSC bits are cleared and waiting for the OSCON bit to clear.

---
 drivers/rtc/rtc-mcp795.c | 103 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 82 insertions(+), 21 deletions(-)
drivers/rtc/rtc-mcp795.c +9 -17
index d15072c..c9ad46c 100644
@@ -21,25 +21,34 @@
 #include <linux/spi/spi.h>
 #include <linux/rtc.h>
 #include <linux/of.h>
+#include <linux/delay.h>
 
 /* MCP795 Instructions, see datasheet table 3-1 */
-#define MCP795_EEREAD	0x03
-#define MCP795_EEWRITE	0x02
-#define MCP795_EEWRDI	0x04
-#define MCP795_EEWREN	0x06
-#define MCP795_SRREAD	0x05
-#define MCP795_SRWRITE	0x01
-#define MCP795_READ	0x13
-#define MCP795_WRITE	0x12
-#define MCP795_UNLOCK	0x14
-#define MCP795_IDWRITE	0x32
-#define MCP795_IDREAD	0x33
-#define MCP795_CLRWDT	0x44
-#define MCP795_CLRRAM	0x54
-
-#define MCP795_ST_BIT	0x80
-#define MCP795_24_BIT	0x40
-#define MCP795_LP_BIT	0x20
+#define MCP795_EEREAD		0x03
+#define MCP795_EEWRITE		0x02
+#define MCP795_EEWRDI		0x04
+#define MCP795_EEWREN		0x06
+#define MCP795_SRREAD		0x05
+#define MCP795_SRWRITE		0x01
+#define MCP795_READ		0x13
+#define MCP795_WRITE		0x12
Quoted 8 lines

drivers/rtc/rtc-mcp795.c line 40

Quoted 6 lines
drivers/rtc/rtc-mcp795.c +6 -0
+#define MCP795_UNLOCK		0x14
+#define MCP795_IDWRITE		0x32
+#define MCP795_IDREAD		0x33
+#define MCP795_CLRWDT		0x44
+#define MCP795_CLRRAM		0x54
+

Please don't reindent, they are a separate block anyway.

Ok,

Quoted 14 lines

drivers/rtc/rtc-mcp795.c line 51

Quoted 12 lines
drivers/rtc/rtc-mcp795.c +11 -0
+/* MCP795 RTCC registers, see datasheet table 4-1 */
+#define MCP795_REG_SECONDS	0x01
+#define MCP795_REG_DAY		0x04
+#define MCP795_REG_MONTH	0x06
+#define MCP795_REG_CONTROL	0x08
+
+#define MCP795_ST_BIT		0x80
+#define MCP795_24_BIT		0x40
+#define MCP795_LP_BIT		0x20
+#define MCP795_EXTOSC_BIT	0x08
+#define MCP795_OSCON_BIT	0x20

Please use BIT() and that is valid for the first patch too).

Ok, I will fix it in the next patchset.

Quoted 32 lines
Quoted 32 lines
drivers/rtc/rtc-mcp795.c +24 -0
 static int mcp795_rtcc_read(struct device *dev, u8 addr, u8 *buf, u8 count)
 {

@@ -94,14 +103,51 @@ static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state)
 	return ret;
 }
 
+static int mcp795_stop_oscillator(struct device *dev)
+{
+	int retries = 5;
+	int ret;
+	u8 data;
+
+	ret = mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS, MCP795_ST_BIT, 0);
+	if (ret)
+		return ret;
+	ret = mcp795_rtcc_set_bits(dev, MCP795_REG_CONTROL, MCP795_EXTOSC_BIT, 0);
+	if (ret)
+		return ret;
+	do {
+		usleep_range(700, 800);
+		ret = mcp795_rtcc_read(dev, MCP795_REG_DAY,
+					&data, sizeof(data));
+		if (ret)
+			break;
+		if (!(data & MCP795_OSCON_BIT))
+			break;
+
+	} while (--retries);
+
+	return !retries ? -EIO : ret;
Quoted 8 lines

drivers/rtc/rtc-mcp795.c static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state) · line 135

Quoted 6 lines
drivers/rtc/rtc-mcp795.c +6 -0
+}
+
+static int mcp795_start_oscillator(struct device *dev)
+{
+	return mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS,
+					MCP795_ST_BIT, MCP795_ST_BIT);

You probably want to restore EXTOSC to its previous value here.

I came to the conclusion that it is better to remove clearing of bit EXTOSC in function mcp795_stop_oscillator. Because regarding datasheet, after power up the RTC chip (or reset) that bit has value 0. What do you think?

Quoted 6 lines
Signature · 4 lines
-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Emil,

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

Re: [PATCH 3/4] rtc: mcp795: fix month write resetting date to 1.

Quoted 10 lines · Emil Bartczak
Quoted 6 lines

drivers/rtc/rtc-mcp795.c static int mcp795_rtcc_set_bits(struct device *dev, u8 addr, u8 mask, u8 state) · line 135

Quoted 4 lines
drivers/rtc/rtc-mcp795.c +4 -0
+static int mcp795_start_oscillator(struct device *dev)
+{
+	return mcp795_rtcc_set_bits(dev, MCP795_REG_SECONDS,
+					MCP795_ST_BIT, MCP795_ST_BIT);

You probably want to restore EXTOSC to its previous value here.

I came to the conclusion that it is better to remove clearing of bit EXTOSC in function mcp795_stop_oscillator. Because regarding datasheet, after power up the RTC chip (or reset) that bit has value 0. What do you think?

That is one way but maybe some people are setting that bit in the bootloader. It doesn't matter much. If you are not using the feature, let it that way. If anybody needs it at some point they can still send a patch :)

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

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

PATCH 4/4 rtc: mcp795: use bcd2bin/bin2bcd.

In reply to Emil Bartczak

Change rtc-mcp795.c to use the bcd2bin/bin2bcd functions.

---
 drivers/rtc/rtc-mcp795.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)
drivers/rtc/rtc-mcp795.c +13 -15
index c9ad46c..1d823f9 100644
@@ -22,6 +22,7 @@
 #include <linux/rtc.h>
 #include <linux/of.h>
 #include <linux/delay.h>
+#include <linux/bcd.h>
 
 /* MCP795 Instructions, see datasheet table 3-1 */
 #define MCP795_EEREAD		0x03

@@ -137,7 +138,6 @@ static int mcp795_start_oscillator(struct device *dev)
 
 static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 {
-	int month;
 	int ret;
 	u8 data[7];
 

@@ -152,10 +152,10 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	if (ret)
 		return ret;
 
-	data[0] = (data[0] & 0x80) | ((tim->tm_sec / 10) << 4) | (tim->tm_sec % 10);
-	data[1] = (data[1] & 0x80) | ((tim->tm_min / 10) << 4) | (tim->tm_min % 10);
-	data[2] = ((tim->tm_hour / 10) << 4) | (tim->tm_hour % 10);
-	data[4] = ((tim->tm_mday / 10) << 4) | ((tim->tm_mday) % 10);
+	data[0] = (data[0] & 0x80) | bin2bcd(tim->tm_sec);
+	data[1] = (data[1] & 0x80) | bin2bcd(tim->tm_min);
+	data[2] = bin2bcd(tim->tm_hour);
+	data[4] = bin2bcd(tim->tm_mday);
 
 	/* Always write the date and month using a separate Write command.
 	 * This is a workaround for a know silicon issue that some combinations

@@ -166,14 +166,12 @@ static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 	if (ret)
 		return ret;
 
-	month = tim->tm_mon + 1;
-	data[5] = (data[5] & MCP795_LP_BIT) |
-			((month / 10) << 4) | (month % 10);
+	data[5] = (data[5] & MCP795_LP_BIT) | bin2bcd(tim->tm_mon + 1);
 
 	if (tim->tm_year > 100)
 		tim->tm_year -= 100;
 
-	data[6] = ((tim->tm_year / 10) << 4) | (tim->tm_year % 10);
+	data[6] = bin2bcd(tim->tm_year);
 
 	ret = mcp795_rtcc_write(dev, MCP795_REG_MONTH, &data[5], 2);
 

@@ -202,12 +200,12 @@ static int mcp795_read_time(struct device *dev, struct rtc_time *tim)
 	if (ret)
 		return ret;
 
-	tim->tm_sec	= ((data[0] & 0x70) >> 4) * 10 + (data[0] & 0x0f);
-	tim->tm_min	= ((data[1] & 0x70) >> 4) * 10 + (data[1] & 0x0f);
-	tim->tm_hour	= ((data[2] & 0x30) >> 4) * 10 + (data[2] & 0x0f);
-	tim->tm_mday	= ((data[4] & 0x30) >> 4) * 10 + (data[4] & 0x0f);
-	tim->tm_mon	= ((data[5] & 0x10) >> 4) * 10 + (data[5] & 0x0f) - 1;
-	tim->tm_year	= ((data[6] & 0xf0) >> 4) * 10 + (data[6] & 0x0f) + 100; /* Assume we are in 20xx */
+	tim->tm_sec	= bcd2bin(data[0] & 0x7F);
+	tim->tm_min	= bcd2bin(data[1] & 0x7F);
+	tim->tm_hour	= bcd2bin(data[2] & 0x3F);
+	tim->tm_mday	= bcd2bin(data[4] & 0x3F);
+	tim->tm_mon	= bcd2bin(data[5] & 0x1F) - 1;
+	tim->tm_year	= bcd2bin(data[6]) + 100; /* Assume we are in 20xx */
 
 	dev_dbg(dev, "Read from mcp795: %04d-%02d-%02d %02d:%02d:%02d\n",
 				tim->tm_year + 1900, tim->tm_mon, tim->tm_mday,
Signature · 11 lines
2.7.4

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

Re: [PATCH 4/4] rtc: mcp795: use bcd2bin/bin2bcd.

Quoted 18 lines · Emil Bartczak

Change rtc-mcp795.c to use the bcd2bin/bin2bcd functions.

---
 drivers/rtc/rtc-mcp795.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)
drivers/rtc/rtc-mcp795.c +1 -0
index c9ad46c..1d823f9 100644
@@ -22,6 +22,7 @@
 #include <linux/rtc.h>
 #include <linux/of.h>
 #include <linux/delay.h>
+#include <linux/bcd.h>
 
 /* MCP795 Instructions, see datasheet table 3-1 */
 #define MCP795_EEREAD		0x03

drivers/rtc/rtc-mcp795.c static int mcp795_start_oscillator(struct device *dev) · line 140

Quoted 4 lines
drivers/rtc/rtc-mcp795.c +0 -1
@@ -137,7 +138,6 @@ static int mcp795_start_oscillator(struct device *dev)
 
 static int mcp795_set_time(struct device *dev, struct rtc_time *tim)
 {
-	int month;

You just introduced this variable to fix a bug. Maybe it was not necessary?

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

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