1 files changed, 78 insertions, 40 deletions

diff --git a/firmware/drivers/rtc/rtc_s35380a.c b/firmware/drivers/rtc/rtc_s35380a.c
index 05796e0462..0a889cf655 100644
--- a/firmware/drivers/rtc/rtc_s35380a.c
+++ b/firmware/drivers/rtc/rtc_s35380a.c
@@ -42,29 +42,25 @@
 #define CLOCK_CORR_REG  6
 #define FREE_REG        7
 
-/* STATUS_REG1 flags
- * bits order is reversed
- */
-#define STATUS_REG1_POC   0x01
-#define STATUS_REG1_BLD   0x02
-#define STATUS_REG1_INT2  0x04
-#define STATUS_REG1_INT1  0x08
-#define STATUS_REG1_SC1   0x10
-#define STATUS_REG1_SC0   0x20
-#define STATUS_REG1_H1224 0x40
-#define STATUS_REG1_RESET 0x80
-
-/* STATUS_REG2 flags
- * bits order is reversed
- */
-#define STATUS_REG2_TEST   0x01
-#define STATUS_REG2_INT2AE 0x02
-#define STATUS_REG2_INT2ME 0x04
-#define STATUS_REG2_INT2FE 0x08
-#define STATUS_REG2_32kE   0x10
-#define STATUS_REG2_INT1AE 0x20
-#define STATUS_REG2_INT1ME 0x40
-#define STATUS_REG2_INT1FE 0x80
+/* STATUS_REG1 flags */
+#define STATUS_REG1_POC   0x80
+#define STATUS_REG1_BLD   0x40
+#define STATUS_REG1_INT2  0x20
+#define STATUS_REG1_INT1  0x10
+#define STATUS_REG1_SC1   0x08
+#define STATUS_REG1_SC0   0x04
+#define STATUS_REG1_H1224 0x02
+#define STATUS_REG1_RESET 0x01
+
+/* STATUS_REG2 flags */
+#define STATUS_REG2_TEST   0x80
+#define STATUS_REG2_INT2AE 0x40
+#define STATUS_REG2_INT2ME 0x20
+#define STATUS_REG2_INT2FE 0x10
+#define STATUS_REG2_32kE   0x08
+#define STATUS_REG2_INT1AE 0x04
+#define STATUS_REG2_INT1ME 0x02
+#define STATUS_REG2_INT1FE 0x01
 
 /* REALTIME_DATA register bytes */
 #define TIME_YEAR     0
@@ -95,6 +91,9 @@
 
 static bool int_flag;
 
+/* s35380a chip has reversed bits order in byte
+ * This is little helper function to deal with
+ */
 static void reverse_bits(unsigned char* v, int size)
 {
     static const unsigned char flipnibble[] =
@@ -108,12 +107,40 @@ static void reverse_bits(unsigned char* v, int size)
     }
 }    
 
+/* Read 'size' bytes from RTC 'reg' and put data in 'buf'
+ * bits are reversed in data bytes afterwards so they appear in regular order
+ * return i2c transfer code
+ */
+static int rtc_read(unsigned char reg, unsigned char *buf, int size)
+{
+    int rc;
+    rc = i2c_read(I2C_IFACE_1, RTC_ADDR|(reg<<1), buf, size);
+    reverse_bits(buf, size);
+    return rc;
+}
+
+/* Write 'size' bytes to RTC 'reg' and put data in 'buf'
+ * bits are reversed in data bytes prior to sending them to RTC
+ * return i2c transfer code
+ */
+static int rtc_write(unsigned char reg, unsigned char *buf, int size)
+{
+    int rc;
+    reverse_bits(buf, size);
+    rc = i2c_write(I2C_IFACE_1, RTC_ADDR|(reg<<1), buf, size);
+    return rc;
+}
+
+/* Reset RTC by writing '1' to RESET bit in STATUS_REG1 */
 static inline void rtc_reset(void)
 {
     unsigned char reg = STATUS_REG1_RESET;
-    i2c_write(I2C_IFACE_1, RTC_ADDR|(STATUS_REG1<<1), &reg, 1);
+    rtc_write(STATUS_REG1, &reg, 1);
 }
 
+/* Initialize RTC (according to scheme outlined in datasheet).
+ * Configure chip to 24h time format.
+ */
 void rtc_init(void)
 {
     unsigned char reg;
@@ -122,7 +149,7 @@ void rtc_init(void)
     if ( initialized )
         return;
 
-    i2c_read(I2C_IFACE_1, RTC_ADDR|(STATUS_REG1<<1), &reg, 1);
+    rtc_read(STATUS_REG1, &reg, 1);
 
     /* cache INT1, INT2 flags as reading the register seem to clear
      * this bits (which is not described in datasheet)
@@ -133,7 +160,7 @@ void rtc_init(void)
     if ( (reg & STATUS_REG1_POC) || (reg & STATUS_REG1_BLD))
         rtc_reset();
 
-    i2c_read(I2C_IFACE_1, RTC_ADDR|(STATUS_REG2<<1), &reg, 1);
+    rtc_read(STATUS_REG2, &reg, 1);
 
     /* test TEST flag */
     if ( reg & STATUS_REG2_TEST )
@@ -141,19 +168,19 @@ void rtc_init(void)
 
     /* setup 24h time format */
     reg = STATUS_REG1_H1224;
-    i2c_write(I2C_IFACE_1, RTC_ADDR|(STATUS_REG1<<1), &reg, 1);
+    rtc_write(STATUS_REG1, &reg, 1);
 
     initialized = true;
 }
 
+/* Read realtime data register */
 int rtc_read_datetime(struct tm *tm)
 {
     unsigned char buf[TIME_REG_SIZE];
     unsigned int i;
     int ret;
 
-    ret = i2c_read(I2C_IFACE_1, RTC_ADDR|(REALTIME_DATA1<<1), buf, sizeof(buf));
-    reverse_bits(buf, sizeof(buf));
+    ret = rtc_read(REALTIME_DATA1, buf, sizeof(buf));
 
     buf[TIME_HOUR] &= 0x3f; /* mask out p.m. flag */
 
@@ -171,6 +198,7 @@ int rtc_read_datetime(struct tm *tm)
     return ret;
 }
 
+/* Write to realtime data register */
 int rtc_write_datetime(const struct tm *tm)
 {
     unsigned char buf[TIME_REG_SIZE];
@@ -188,13 +216,13 @@ int rtc_write_datetime(const struct tm *tm)
     for (i = 0; i < sizeof(buf); i++)
         buf[i] = DEC2BCD(buf[i]);
 
-    reverse_bits(buf, sizeof(buf));
-    ret = i2c_write(I2C_IFACE_1, RTC_ADDR|(REALTIME_DATA1<<1), buf, sizeof(buf));
+    ret = rtc_write(REALTIME_DATA1, buf, sizeof(buf));
 
     return ret;
 }
 
 #ifdef HAVE_RTC_ALARM
+/* Set alarm (INT1) data register */
 void rtc_set_alarm(int h, int m)
 {
     unsigned char buf[ALARM_REG_SIZE];
@@ -207,7 +235,7 @@ void rtc_set_alarm(int h, int m)
     buf[ALARM_HOUR]    = DEC2BCD(h) | A1HE;
     buf[ALARM_WEEKDAY] = 0;
 
-    /* AM/PM flag have to be set properly regardles of
+    /* AM/PM flag has to be set properly regardles of
      * time format used (H1224 flag in STATUS_REG1)
      * this is not described in datasheet for s35380a
      * but is somehow described in datasheet for s35390a
@@ -215,10 +243,10 @@ void rtc_set_alarm(int h, int m)
     if ( h >= 12 )
         buf[ALARM_HOUR] |= AMPM;
 
-    reverse_bits(buf, sizeof(buf));
-    i2c_write(I2C_IFACE_1, RTC_ADDR|(INT1_REG<<1), buf, sizeof(buf));
+    rtc_write(INT1_REG, buf, sizeof(buf));
 }
 
+/* Read alarm (INT1) data register */
 void rtc_get_alarm(int *h, int *m)
 {
     unsigned char buf[ALARM_REG_SIZE];
@@ -227,23 +255,32 @@ void rtc_get_alarm(int *h, int *m)
     rtc_enable_alarm(true);
 
     /* read the content of INT1 register */
-    i2c_read(I2C_IFACE_1, RTC_ADDR|(INT1_REG<<1), buf, sizeof(buf));
-    reverse_bits(buf, sizeof(buf));
+    rtc_read(INT1_REG, buf, sizeof(buf));
 
-    *h = BCD2DEC(buf[ALARM_HOUR] & 0x3f); /* mask out A1HE and PM/AM flag */
-    *m = BCD2DEC(buf[ALARM_MINUTE] & 0x7f); /* mask out A1mE */
+    *h = BCD2DEC(buf[ALARM_HOUR] & 0x3f);   /* mask out A1HE and PM/AM bits */
+    *m = BCD2DEC(buf[ALARM_MINUTE] & 0x7f); /* mask out A1mE bit */
 
+    /* Disable alarm - this is not strictly needed in rockbox
+     * as after rtc_get_alarm() rtc_set_alarm() or rtc_enable_alarm(false)
+     * are called. I just found this weird that simple reading register
+     * changes alarm settings.
+     */
     rtc_enable_alarm(false);
 }
 
+/* Check if we just triggered alarm.
+ * We check both INT1 and INT2. Rockbox uses only INT1 but
+ * OF in MPIO HD300 uses both
+ */
 bool rtc_check_alarm_flag(void)
 {
     unsigned char reg;
-    i2c_read(I2C_IFACE_1, RTC_ADDR|(STATUS_REG1<<1), &reg, 1);
+    rtc_read(STATUS_REG1, &reg, 1);
 
     return ((reg & STATUS_REG1_INT1) || (reg & STATUS_REG1_INT2));
 }
 
+/* Enable/disable alarm function */
 void rtc_enable_alarm(bool enable)
 {
     unsigned char reg = 0;
@@ -251,9 +288,10 @@ void rtc_enable_alarm(bool enable)
     if (enable)
         reg = STATUS_REG2_INT1AE;
 
-    i2c_write(I2C_IFACE_1, RTC_ADDR|(STATUS_REG2<<1), &reg, 1);
+    rtc_write(STATUS_REG2, &reg, 1);
 }
 
+/* Return true if wakeup is due to RTC alarm */
 bool rtc_check_alarm_started(bool release_alarm)
 {
     static bool run_before;