Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Wed, 3 Jan 2007 20:23:35 -0800 David Brownell <[EMAIL PROTECTED]> wrote: > > > > Works fine, thanks! Unfortunately the i2c-ixp3xx issue has not advanced in > > the meantime, so we still need the third method. > > Right. Thanks for confirming this! Alessandro? Given that we can not test on more boards, I'd keep the most compatible method. Acked-by: Alessandro Zummo <[EMAIL PROTECTED]> -- Best regards, Alessandro Zummo, Tower Technologies - Torino, Italy http://www.towertech.it - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Wed, 3 Jan 2007 20:23:35 -0800 David Brownell [EMAIL PROTECTED] wrote: Works fine, thanks! Unfortunately the i2c-ixp3xx issue has not advanced in the meantime, so we still need the third method. Right. Thanks for confirming this! Alessandro? Given that we can not test on more boards, I'd keep the most compatible method. Acked-by: Alessandro Zummo [EMAIL PROTECTED] -- Best regards, Alessandro Zummo, Tower Technologies - Torino, Italy http://www.towertech.it - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Wednesday 03 January 2007 2:51 pm, Voipio Riku wrote:
>
> > Yes, the patch you sent (switching to "method 3" to work around the
> > evident bug in the i2c-ixp3xx driver) works on the platform I was
> > using too (after unrelated tweaks).
>
> > Here's an updated patch, using "method 3". If it still behaves
> > for you, it'd seem ready to merge...
>
> Works fine, thanks! Unfortunately the i2c-ixp3xx issue has not advanced in
> the meantime, so we still need the third method.
Right. Thanks for confirming this! Alessandro?
Here's a version with corrected patch comments. (Against 2.6.20-rc3 now,
but I didn't change $SUBJECT).
- Dave
=== CUT HERE
Update the rtc-rs5c372 driver:
Bugfixes:
- Handle RTCs which are configured to use 12-hour mode.
- Never report bogus/un-initialized times.
- Displaying "raw trim" requires not masking it first!
- Fix the sysfs and procfs display of crystal and trim data.
Features:
- Handle other RTCs in this family, notably rv5c386/rv5c387.
- Declare the other registers.
- Provide alarm get/set functionality.
- Handle AIE and UIE; but no IRQ handling yet.
Cleanup:
- Shrink object by not including needless sysfs or procfs support
- We don't need no steenkin' forward declarations. (Except one.)
Until the I2C framework merges "new style" driver support, matching
the driver model better, using rv5c chips or alarm IRQs requires a
separate board-specific patch. (And an IRQ handler, handing off labor
through a work_struct...)
This uses the "method 3" register reads, but notes that it's done
to work around an evident i2c adapter driver bug.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Index: g26/drivers/rtc/rtc-rs5c372.c
===
--- g26.orig/drivers/rtc/rtc-rs5c372.c 2006-12-27 17:19:55.0 -0800
+++ g26/drivers/rtc/rtc-rs5c372.c 2007-01-02 18:44:35.0 -0800
@@ -1,5 +1,5 @@
/*
- * An I2C driver for the Ricoh RS5C372 RTC
+ * An I2C driver for Ricoh RS5C372 and RV5C38[67] RTCs
*
* Copyright (C) 2005 Pavel Mironchik <[EMAIL PROTECTED]>
* Copyright (C) 2006 Tower Technologies
@@ -13,7 +13,7 @@
#include
#include
-#define DRV_VERSION "0.3"
+#define DRV_VERSION "0.4"
/* Addresses to scan */
static unsigned short normal_i2c[] = { /* 0x32,*/ I2C_CLIENT_END };
@@ -21,6 +21,13 @@ static unsigned short normal_i2c[] = { /
/* Insmod parameters */
I2C_CLIENT_INSMOD;
+
+/*
+ * Ricoh has a family of I2C based RTCs, which differ only slightly from
+ * each other. Differences center on pinout (e.g. how many interrupts,
+ * output clock, etc) and how the control registers are used. The '372
+ * is significant only because that's the one this driver first supported.
+ */
#define RS5C372_REG_SECS 0
#define RS5C372_REG_MINS 1
#define RS5C372_REG_HOURS 2
@@ -29,59 +36,142 @@ I2C_CLIENT_INSMOD;
#define RS5C372_REG_MONTH 5
#define RS5C372_REG_YEAR 6
#define RS5C372_REG_TRIM 7
+# define RS5C372_TRIM_XSL 0x80
+# define RS5C372_TRIM_MASK0x7F
-#define RS5C372_TRIM_XSL 0x80
-#define RS5C372_TRIM_MASK 0x7F
-
-#define RS5C372_REG_BASE 0
-
-static int rs5c372_attach(struct i2c_adapter *adapter);
-static int rs5c372_detach(struct i2c_client *client);
-static int rs5c372_probe(struct i2c_adapter *adapter, int address, int kind);
+#define RS5C_REG_ALARM_A_MIN 8 /* or ALARM_W */
+#define RS5C_REG_ALARM_A_HOURS 9
+#define RS5C_REG_ALARM_A_WDAY 10
+
+#define RS5C_REG_ALARM_B_MIN 11 /* or ALARM_D */
+#define RS5C_REG_ALARM_B_HOURS 12
+#define RS5C_REG_ALARM_B_WDAY 13 /* (ALARM_B only) */
+
+#define RS5C_REG_CTRL1 14
+# define RS5C_CTRL1_AALE (1 << 7)/* or WALE */
+# define RS5C_CTRL1_BALE (1 << 6)/* or DALE */
+# define RV5C387_CTRL1_24 (1 << 5)
+# define RS5C372A_CTRL1_SL1 (1 << 5)
+# define RS5C_CTRL1_CT_MASK (7 << 0)
+# define RS5C_CTRL1_CT0 (0 << 0)/* no periodic irq */
+# define RS5C_CTRL1_CT4 (4 << 0)/* 1 Hz level irq */
+#define RS5C_REG_CTRL2 15
+# define RS5C372_CTRL2_24 (1 << 5)
+# define RS5C_CTRL2_XSTP (1 << 4)
+# define RS5C_CTRL2_CTFG (1 << 2)
+# define RS5C_CTRL2_AAFG (1 << 1)/* or WAFG */
+# define RS5C_CTRL2_BAFG (1 << 0)/* or DAFG */
+
+
+/* to read (style 1) or write registers starting at R */
+#define RS5C_ADDR(R) (((R) << 4) | 0)
+
+
+enum rtc_type {
+ rtc_undef = 0,
+ rtc_rs5c372a,
+ rtc_rs5c372b,
+ rtc_rv5c386,
+ rtc_rv5c387a,
+};
+/* REVISIT: this assumes that:
+ * - we're in the 21st century, so it's safe to ignore the century
+ *bit for rv5c38[67] (REG_MONTH bit 7);
+ * - we should use ALARM_A not ALARM_B (may be wrong on
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
Hi David,
> Yes, the patch you sent (switching to "method 3" to work around the
> evident bug in the i2c-ixp3xx driver) works on the platform I was
> using too (after unrelated tweaks).
> Here's an updated patch, using "method 3". If it still behaves
> for you, it'd seem ready to merge...
Works fine, thanks! Unfortunately the i2c-ixp3xx issue has not advanced in
the meantime, so we still need the third method.
> CUT HERE
> Update the rtc-rs5c372 driver:
>
> Bugfixes:
> - Handle RTCs which are configured to use 12-hour mode.
> - Never report bogus/un-initialized times.
> - Displaying "raw trim" requires not masking it first!
> - Fix the and sysfs procfs display of crystal and trim data.
>
> Features:
> - Handle other RTCs in this family, notably rv5c386/rv5c387.
> - Declare the other registers.
> - Provide alarm get/set functionality.
> - Handle AIE and UIE; but no IRQ handling yet.
> - Warn if the clock needs to be set.
>
> Cleanup:
> - Shrink object by not including needless sysfs or procfs support
> - We don't need no steenkin' forward declarations. (Except one.)
>
> Until the I2C framework merges "new style" driver support, matching
> the driver model better, using rv5c chips or alarm IRQs requires a
> separate board-specific patch. (And an IRQ handler, handing off labor
> through a work_struct...)
>
> This uses the "method 3" register reads, but notes that it's done
> to work around an evident i2c adapter driver bug, and the curious
> issue where the chip behavior disagrees with the chip docs.
>
> Signed-off-by: David Brownell <[EMAIL PROTECTED]>
>
> Index: g26/drivers/rtc/rtc-rs5c372.c
> ===
> --- g26.orig/drivers/rtc/rtc-rs5c372.c2006-12-27 17:19:55.0
> -0800
> +++ g26/drivers/rtc/rtc-rs5c372.c 2007-01-02 18:44:35.0 -0800
> @@ -1,5 +1,5 @@
> /*
> - * An I2C driver for the Ricoh RS5C372 RTC
> + * An I2C driver for Ricoh RS5C372 and RV5C38[67] RTCs
> *
> * Copyright (C) 2005 Pavel Mironchik <[EMAIL PROTECTED]>
> * Copyright (C) 2006 Tower Technologies
> @@ -13,7 +13,7 @@
> #include
> #include
>
> -#define DRV_VERSION "0.3"
> +#define DRV_VERSION "0.4"
>
> /* Addresses to scan */
> static unsigned short normal_i2c[] = { /* 0x32,*/ I2C_CLIENT_END };
> @@ -21,6 +21,13 @@ static unsigned short normal_i2c[] = { /
> /* Insmod parameters */
> I2C_CLIENT_INSMOD;
>
> +
> +/*
> + * Ricoh has a family of I2C based RTCs, which differ only slightly from
> + * each other. Differences center on pinout (e.g. how many interrupts,
> + * output clock, etc) and how the control registers are used. The '372
> + * is significant only because that's the one this driver first
> supported.
> + */
> #define RS5C372_REG_SECS 0
> #define RS5C372_REG_MINS 1
> #define RS5C372_REG_HOURS2
> @@ -29,59 +36,142 @@ I2C_CLIENT_INSMOD;
> #define RS5C372_REG_MONTH5
> #define RS5C372_REG_YEAR 6
> #define RS5C372_REG_TRIM 7
> +#define RS5C372_TRIM_XSL 0x80
> +#define RS5C372_TRIM_MASK0x7F
>
> -#define RS5C372_TRIM_XSL 0x80
> -#define RS5C372_TRIM_MASK0x7F
> -
> -#define RS5C372_REG_BASE 0
> -
> -static int rs5c372_attach(struct i2c_adapter *adapter);
> -static int rs5c372_detach(struct i2c_client *client);
> -static int rs5c372_probe(struct i2c_adapter *adapter, int address, int
> kind);
> +#define RS5C_REG_ALARM_A_MIN 8 /* or ALARM_W */
> +#define RS5C_REG_ALARM_A_HOURS 9
> +#define RS5C_REG_ALARM_A_WDAY10
> +
> +#define RS5C_REG_ALARM_B_MIN 11 /* or ALARM_D */
> +#define RS5C_REG_ALARM_B_HOURS 12
> +#define RS5C_REG_ALARM_B_WDAY13 /* (ALARM_B
> only) */
> +
> +#define RS5C_REG_CTRL1 14
> +#define RS5C_CTRL1_AALE (1 << 7)/* or WALE */
> +#define RS5C_CTRL1_BALE (1 << 6)/* or DALE */
> +#define RV5C387_CTRL1_24 (1 << 5)
> +#define RS5C372A_CTRL1_SL1 (1 << 5)
> +#define RS5C_CTRL1_CT_MASK (7 << 0)
> +#define RS5C_CTRL1_CT0 (0 << 0)/* no periodic irq */
> +#define RS5C_CTRL1_CT4 (4 << 0)/* 1 Hz level irq */
> +#define RS5C_REG_CTRL2 15
> +#define RS5C372_CTRL2_24 (1 << 5)
> +#define RS5C_CTRL2_XSTP (1 << 4)
> +#define RS5C_CTRL2_CTFG (1 << 2)
> +#define RS5C_CTRL2_AAFG (1 << 1)/* or WAFG */
> +#define RS5C_CTRL2_BAFG (1 << 0)/* or DAFG */
> +
> +
> +/* to read (style 1) or write registers starting at R */
> +#define RS5C_ADDR(R) (((R) << 4) | 0)
> +
> +
> +enum rtc_type {
> + rtc_undef = 0,
> + rtc_rs5c372a,
> + rtc_rs5c372b,
> + rtc_rv5c386,
> + rtc_rv5c387a,
> +};
>
> +/* REVISIT: this assumes that:
> + * - we're in the 21st century, so it's safe to ignore the century
> + *bit
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
Hi David,
Yes, the patch you sent (switching to method 3 to work around the
evident bug in the i2c-ixp3xx driver) works on the platform I was
using too (after unrelated tweaks).
Here's an updated patch, using method 3. If it still behaves
for you, it'd seem ready to merge...
Works fine, thanks! Unfortunately the i2c-ixp3xx issue has not advanced in
the meantime, so we still need the third method.
CUT HERE
Update the rtc-rs5c372 driver:
Bugfixes:
- Handle RTCs which are configured to use 12-hour mode.
- Never report bogus/un-initialized times.
- Displaying raw trim requires not masking it first!
- Fix the and sysfs procfs display of crystal and trim data.
Features:
- Handle other RTCs in this family, notably rv5c386/rv5c387.
- Declare the other registers.
- Provide alarm get/set functionality.
- Handle AIE and UIE; but no IRQ handling yet.
- Warn if the clock needs to be set.
Cleanup:
- Shrink object by not including needless sysfs or procfs support
- We don't need no steenkin' forward declarations. (Except one.)
Until the I2C framework merges new style driver support, matching
the driver model better, using rv5c chips or alarm IRQs requires a
separate board-specific patch. (And an IRQ handler, handing off labor
through a work_struct...)
This uses the method 3 register reads, but notes that it's done
to work around an evident i2c adapter driver bug, and the curious
issue where the chip behavior disagrees with the chip docs.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Index: g26/drivers/rtc/rtc-rs5c372.c
===
--- g26.orig/drivers/rtc/rtc-rs5c372.c2006-12-27 17:19:55.0
-0800
+++ g26/drivers/rtc/rtc-rs5c372.c 2007-01-02 18:44:35.0 -0800
@@ -1,5 +1,5 @@
/*
- * An I2C driver for the Ricoh RS5C372 RTC
+ * An I2C driver for Ricoh RS5C372 and RV5C38[67] RTCs
*
* Copyright (C) 2005 Pavel Mironchik [EMAIL PROTECTED]
* Copyright (C) 2006 Tower Technologies
@@ -13,7 +13,7 @@
#include linux/rtc.h
#include linux/bcd.h
-#define DRV_VERSION 0.3
+#define DRV_VERSION 0.4
/* Addresses to scan */
static unsigned short normal_i2c[] = { /* 0x32,*/ I2C_CLIENT_END };
@@ -21,6 +21,13 @@ static unsigned short normal_i2c[] = { /
/* Insmod parameters */
I2C_CLIENT_INSMOD;
+
+/*
+ * Ricoh has a family of I2C based RTCs, which differ only slightly from
+ * each other. Differences center on pinout (e.g. how many interrupts,
+ * output clock, etc) and how the control registers are used. The '372
+ * is significant only because that's the one this driver first
supported.
+ */
#define RS5C372_REG_SECS 0
#define RS5C372_REG_MINS 1
#define RS5C372_REG_HOURS2
@@ -29,59 +36,142 @@ I2C_CLIENT_INSMOD;
#define RS5C372_REG_MONTH5
#define RS5C372_REG_YEAR 6
#define RS5C372_REG_TRIM 7
+#define RS5C372_TRIM_XSL 0x80
+#define RS5C372_TRIM_MASK0x7F
-#define RS5C372_TRIM_XSL 0x80
-#define RS5C372_TRIM_MASK0x7F
-
-#define RS5C372_REG_BASE 0
-
-static int rs5c372_attach(struct i2c_adapter *adapter);
-static int rs5c372_detach(struct i2c_client *client);
-static int rs5c372_probe(struct i2c_adapter *adapter, int address, int
kind);
+#define RS5C_REG_ALARM_A_MIN 8 /* or ALARM_W */
+#define RS5C_REG_ALARM_A_HOURS 9
+#define RS5C_REG_ALARM_A_WDAY10
+
+#define RS5C_REG_ALARM_B_MIN 11 /* or ALARM_D */
+#define RS5C_REG_ALARM_B_HOURS 12
+#define RS5C_REG_ALARM_B_WDAY13 /* (ALARM_B
only) */
+
+#define RS5C_REG_CTRL1 14
+#define RS5C_CTRL1_AALE (1 7)/* or WALE */
+#define RS5C_CTRL1_BALE (1 6)/* or DALE */
+#define RV5C387_CTRL1_24 (1 5)
+#define RS5C372A_CTRL1_SL1 (1 5)
+#define RS5C_CTRL1_CT_MASK (7 0)
+#define RS5C_CTRL1_CT0 (0 0)/* no periodic irq */
+#define RS5C_CTRL1_CT4 (4 0)/* 1 Hz level irq */
+#define RS5C_REG_CTRL2 15
+#define RS5C372_CTRL2_24 (1 5)
+#define RS5C_CTRL2_XSTP (1 4)
+#define RS5C_CTRL2_CTFG (1 2)
+#define RS5C_CTRL2_AAFG (1 1)/* or WAFG */
+#define RS5C_CTRL2_BAFG (1 0)/* or DAFG */
+
+
+/* to read (style 1) or write registers starting at R */
+#define RS5C_ADDR(R) (((R) 4) | 0)
+
+
+enum rtc_type {
+ rtc_undef = 0,
+ rtc_rs5c372a,
+ rtc_rs5c372b,
+ rtc_rv5c386,
+ rtc_rv5c387a,
+};
+/* REVISIT: this assumes that:
+ * - we're in the 21st century, so it's safe to ignore the century
+ *bit for rv5c38[67] (REG_MONTH bit 7);
+ * - we should use ALARM_A not ALARM_B (may be wrong on some boards)
+ */
struct rs5c372 {
- u8 reg_addr;
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Wednesday 03 January 2007 2:51 pm, Voipio Riku wrote:
Yes, the patch you sent (switching to method 3 to work around the
evident bug in the i2c-ixp3xx driver) works on the platform I was
using too (after unrelated tweaks).
Here's an updated patch, using method 3. If it still behaves
for you, it'd seem ready to merge...
Works fine, thanks! Unfortunately the i2c-ixp3xx issue has not advanced in
the meantime, so we still need the third method.
Right. Thanks for confirming this! Alessandro?
Here's a version with corrected patch comments. (Against 2.6.20-rc3 now,
but I didn't change $SUBJECT).
- Dave
=== CUT HERE
Update the rtc-rs5c372 driver:
Bugfixes:
- Handle RTCs which are configured to use 12-hour mode.
- Never report bogus/un-initialized times.
- Displaying raw trim requires not masking it first!
- Fix the sysfs and procfs display of crystal and trim data.
Features:
- Handle other RTCs in this family, notably rv5c386/rv5c387.
- Declare the other registers.
- Provide alarm get/set functionality.
- Handle AIE and UIE; but no IRQ handling yet.
Cleanup:
- Shrink object by not including needless sysfs or procfs support
- We don't need no steenkin' forward declarations. (Except one.)
Until the I2C framework merges new style driver support, matching
the driver model better, using rv5c chips or alarm IRQs requires a
separate board-specific patch. (And an IRQ handler, handing off labor
through a work_struct...)
This uses the method 3 register reads, but notes that it's done
to work around an evident i2c adapter driver bug.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Index: g26/drivers/rtc/rtc-rs5c372.c
===
--- g26.orig/drivers/rtc/rtc-rs5c372.c 2006-12-27 17:19:55.0 -0800
+++ g26/drivers/rtc/rtc-rs5c372.c 2007-01-02 18:44:35.0 -0800
@@ -1,5 +1,5 @@
/*
- * An I2C driver for the Ricoh RS5C372 RTC
+ * An I2C driver for Ricoh RS5C372 and RV5C38[67] RTCs
*
* Copyright (C) 2005 Pavel Mironchik [EMAIL PROTECTED]
* Copyright (C) 2006 Tower Technologies
@@ -13,7 +13,7 @@
#include linux/rtc.h
#include linux/bcd.h
-#define DRV_VERSION 0.3
+#define DRV_VERSION 0.4
/* Addresses to scan */
static unsigned short normal_i2c[] = { /* 0x32,*/ I2C_CLIENT_END };
@@ -21,6 +21,13 @@ static unsigned short normal_i2c[] = { /
/* Insmod parameters */
I2C_CLIENT_INSMOD;
+
+/*
+ * Ricoh has a family of I2C based RTCs, which differ only slightly from
+ * each other. Differences center on pinout (e.g. how many interrupts,
+ * output clock, etc) and how the control registers are used. The '372
+ * is significant only because that's the one this driver first supported.
+ */
#define RS5C372_REG_SECS 0
#define RS5C372_REG_MINS 1
#define RS5C372_REG_HOURS 2
@@ -29,59 +36,142 @@ I2C_CLIENT_INSMOD;
#define RS5C372_REG_MONTH 5
#define RS5C372_REG_YEAR 6
#define RS5C372_REG_TRIM 7
+# define RS5C372_TRIM_XSL 0x80
+# define RS5C372_TRIM_MASK0x7F
-#define RS5C372_TRIM_XSL 0x80
-#define RS5C372_TRIM_MASK 0x7F
-
-#define RS5C372_REG_BASE 0
-
-static int rs5c372_attach(struct i2c_adapter *adapter);
-static int rs5c372_detach(struct i2c_client *client);
-static int rs5c372_probe(struct i2c_adapter *adapter, int address, int kind);
+#define RS5C_REG_ALARM_A_MIN 8 /* or ALARM_W */
+#define RS5C_REG_ALARM_A_HOURS 9
+#define RS5C_REG_ALARM_A_WDAY 10
+
+#define RS5C_REG_ALARM_B_MIN 11 /* or ALARM_D */
+#define RS5C_REG_ALARM_B_HOURS 12
+#define RS5C_REG_ALARM_B_WDAY 13 /* (ALARM_B only) */
+
+#define RS5C_REG_CTRL1 14
+# define RS5C_CTRL1_AALE (1 7)/* or WALE */
+# define RS5C_CTRL1_BALE (1 6)/* or DALE */
+# define RV5C387_CTRL1_24 (1 5)
+# define RS5C372A_CTRL1_SL1 (1 5)
+# define RS5C_CTRL1_CT_MASK (7 0)
+# define RS5C_CTRL1_CT0 (0 0)/* no periodic irq */
+# define RS5C_CTRL1_CT4 (4 0)/* 1 Hz level irq */
+#define RS5C_REG_CTRL2 15
+# define RS5C372_CTRL2_24 (1 5)
+# define RS5C_CTRL2_XSTP (1 4)
+# define RS5C_CTRL2_CTFG (1 2)
+# define RS5C_CTRL2_AAFG (1 1)/* or WAFG */
+# define RS5C_CTRL2_BAFG (1 0)/* or DAFG */
+
+
+/* to read (style 1) or write registers starting at R */
+#define RS5C_ADDR(R) (((R) 4) | 0)
+
+
+enum rtc_type {
+ rtc_undef = 0,
+ rtc_rs5c372a,
+ rtc_rs5c372b,
+ rtc_rv5c386,
+ rtc_rv5c387a,
+};
+/* REVISIT: this assumes that:
+ * - we're in the 21st century, so it's safe to ignore the century
+ *bit for rv5c38[67] (REG_MONTH bit 7);
+ * - we should use ALARM_A not ALARM_B (may be wrong on some boards)
+ */
struct rs5c372 {
-
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
Hi Voipio,
Yes, the patch you sent (switching to "method 3" to work around the
evident bug in the i2c-ixp3xx driver) works on the platform I was
using too (after unrelated tweaks).
Here's an updated patch, using "method 3". If it still behaves
for you, it'd seem ready to merge...
- Dave
CUT HERE
Update the rtc-rs5c372 driver:
Bugfixes:
- Handle RTCs which are configured to use 12-hour mode.
- Never report bogus/un-initialized times.
- Displaying "raw trim" requires not masking it first!
- Fix the and sysfs procfs display of crystal and trim data.
Features:
- Handle other RTCs in this family, notably rv5c386/rv5c387.
- Declare the other registers.
- Provide alarm get/set functionality.
- Handle AIE and UIE; but no IRQ handling yet.
- Warn if the clock needs to be set.
Cleanup:
- Shrink object by not including needless sysfs or procfs support
- We don't need no steenkin' forward declarations. (Except one.)
Until the I2C framework merges "new style" driver support, matching
the driver model better, using rv5c chips or alarm IRQs requires a
separate board-specific patch. (And an IRQ handler, handing off labor
through a work_struct...)
This uses the "method 3" register reads, but notes that it's done
to work around an evident i2c adapter driver bug, and the curious
issue where the chip behavior disagrees with the chip docs.
Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Index: g26/drivers/rtc/rtc-rs5c372.c
===
--- g26.orig/drivers/rtc/rtc-rs5c372.c 2006-12-27 17:19:55.0 -0800
+++ g26/drivers/rtc/rtc-rs5c372.c 2007-01-02 18:44:35.0 -0800
@@ -1,5 +1,5 @@
/*
- * An I2C driver for the Ricoh RS5C372 RTC
+ * An I2C driver for Ricoh RS5C372 and RV5C38[67] RTCs
*
* Copyright (C) 2005 Pavel Mironchik <[EMAIL PROTECTED]>
* Copyright (C) 2006 Tower Technologies
@@ -13,7 +13,7 @@
#include
#include
-#define DRV_VERSION "0.3"
+#define DRV_VERSION "0.4"
/* Addresses to scan */
static unsigned short normal_i2c[] = { /* 0x32,*/ I2C_CLIENT_END };
@@ -21,6 +21,13 @@ static unsigned short normal_i2c[] = { /
/* Insmod parameters */
I2C_CLIENT_INSMOD;
+
+/*
+ * Ricoh has a family of I2C based RTCs, which differ only slightly from
+ * each other. Differences center on pinout (e.g. how many interrupts,
+ * output clock, etc) and how the control registers are used. The '372
+ * is significant only because that's the one this driver first supported.
+ */
#define RS5C372_REG_SECS 0
#define RS5C372_REG_MINS 1
#define RS5C372_REG_HOURS 2
@@ -29,59 +36,142 @@ I2C_CLIENT_INSMOD;
#define RS5C372_REG_MONTH 5
#define RS5C372_REG_YEAR 6
#define RS5C372_REG_TRIM 7
+# define RS5C372_TRIM_XSL 0x80
+# define RS5C372_TRIM_MASK0x7F
-#define RS5C372_TRIM_XSL 0x80
-#define RS5C372_TRIM_MASK 0x7F
-
-#define RS5C372_REG_BASE 0
-
-static int rs5c372_attach(struct i2c_adapter *adapter);
-static int rs5c372_detach(struct i2c_client *client);
-static int rs5c372_probe(struct i2c_adapter *adapter, int address, int kind);
+#define RS5C_REG_ALARM_A_MIN 8 /* or ALARM_W */
+#define RS5C_REG_ALARM_A_HOURS 9
+#define RS5C_REG_ALARM_A_WDAY 10
+
+#define RS5C_REG_ALARM_B_MIN 11 /* or ALARM_D */
+#define RS5C_REG_ALARM_B_HOURS 12
+#define RS5C_REG_ALARM_B_WDAY 13 /* (ALARM_B only) */
+
+#define RS5C_REG_CTRL1 14
+# define RS5C_CTRL1_AALE (1 << 7)/* or WALE */
+# define RS5C_CTRL1_BALE (1 << 6)/* or DALE */
+# define RV5C387_CTRL1_24 (1 << 5)
+# define RS5C372A_CTRL1_SL1 (1 << 5)
+# define RS5C_CTRL1_CT_MASK (7 << 0)
+# define RS5C_CTRL1_CT0 (0 << 0)/* no periodic irq */
+# define RS5C_CTRL1_CT4 (4 << 0)/* 1 Hz level irq */
+#define RS5C_REG_CTRL2 15
+# define RS5C372_CTRL2_24 (1 << 5)
+# define RS5C_CTRL2_XSTP (1 << 4)
+# define RS5C_CTRL2_CTFG (1 << 2)
+# define RS5C_CTRL2_AAFG (1 << 1)/* or WAFG */
+# define RS5C_CTRL2_BAFG (1 << 0)/* or DAFG */
+
+
+/* to read (style 1) or write registers starting at R */
+#define RS5C_ADDR(R) (((R) << 4) | 0)
+
+
+enum rtc_type {
+ rtc_undef = 0,
+ rtc_rs5c372a,
+ rtc_rs5c372b,
+ rtc_rv5c386,
+ rtc_rv5c387a,
+};
+/* REVISIT: this assumes that:
+ * - we're in the 21st century, so it's safe to ignore the century
+ *bit for rv5c38[67] (REG_MONTH bit 7);
+ * - we should use ALARM_A not ALARM_B (may be wrong on some boards)
+ */
struct rs5c372 {
- u8 reg_addr;
- u8 regs[17];
- struct i2c_msg msg[1];
- struct i2c_client client;
- struct rtc_device *rtc;
-};
+ struct i2c_client *client;
+ struct
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
Hi Voipio,
Yes, the patch you sent (switching to method 3 to work around the
evident bug in the i2c-ixp3xx driver) works on the platform I was
using too (after unrelated tweaks).
Here's an updated patch, using method 3. If it still behaves
for you, it'd seem ready to merge...
- Dave
CUT HERE
Update the rtc-rs5c372 driver:
Bugfixes:
- Handle RTCs which are configured to use 12-hour mode.
- Never report bogus/un-initialized times.
- Displaying raw trim requires not masking it first!
- Fix the and sysfs procfs display of crystal and trim data.
Features:
- Handle other RTCs in this family, notably rv5c386/rv5c387.
- Declare the other registers.
- Provide alarm get/set functionality.
- Handle AIE and UIE; but no IRQ handling yet.
- Warn if the clock needs to be set.
Cleanup:
- Shrink object by not including needless sysfs or procfs support
- We don't need no steenkin' forward declarations. (Except one.)
Until the I2C framework merges new style driver support, matching
the driver model better, using rv5c chips or alarm IRQs requires a
separate board-specific patch. (And an IRQ handler, handing off labor
through a work_struct...)
This uses the method 3 register reads, but notes that it's done
to work around an evident i2c adapter driver bug, and the curious
issue where the chip behavior disagrees with the chip docs.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Index: g26/drivers/rtc/rtc-rs5c372.c
===
--- g26.orig/drivers/rtc/rtc-rs5c372.c 2006-12-27 17:19:55.0 -0800
+++ g26/drivers/rtc/rtc-rs5c372.c 2007-01-02 18:44:35.0 -0800
@@ -1,5 +1,5 @@
/*
- * An I2C driver for the Ricoh RS5C372 RTC
+ * An I2C driver for Ricoh RS5C372 and RV5C38[67] RTCs
*
* Copyright (C) 2005 Pavel Mironchik [EMAIL PROTECTED]
* Copyright (C) 2006 Tower Technologies
@@ -13,7 +13,7 @@
#include linux/rtc.h
#include linux/bcd.h
-#define DRV_VERSION 0.3
+#define DRV_VERSION 0.4
/* Addresses to scan */
static unsigned short normal_i2c[] = { /* 0x32,*/ I2C_CLIENT_END };
@@ -21,6 +21,13 @@ static unsigned short normal_i2c[] = { /
/* Insmod parameters */
I2C_CLIENT_INSMOD;
+
+/*
+ * Ricoh has a family of I2C based RTCs, which differ only slightly from
+ * each other. Differences center on pinout (e.g. how many interrupts,
+ * output clock, etc) and how the control registers are used. The '372
+ * is significant only because that's the one this driver first supported.
+ */
#define RS5C372_REG_SECS 0
#define RS5C372_REG_MINS 1
#define RS5C372_REG_HOURS 2
@@ -29,59 +36,142 @@ I2C_CLIENT_INSMOD;
#define RS5C372_REG_MONTH 5
#define RS5C372_REG_YEAR 6
#define RS5C372_REG_TRIM 7
+# define RS5C372_TRIM_XSL 0x80
+# define RS5C372_TRIM_MASK0x7F
-#define RS5C372_TRIM_XSL 0x80
-#define RS5C372_TRIM_MASK 0x7F
-
-#define RS5C372_REG_BASE 0
-
-static int rs5c372_attach(struct i2c_adapter *adapter);
-static int rs5c372_detach(struct i2c_client *client);
-static int rs5c372_probe(struct i2c_adapter *adapter, int address, int kind);
+#define RS5C_REG_ALARM_A_MIN 8 /* or ALARM_W */
+#define RS5C_REG_ALARM_A_HOURS 9
+#define RS5C_REG_ALARM_A_WDAY 10
+
+#define RS5C_REG_ALARM_B_MIN 11 /* or ALARM_D */
+#define RS5C_REG_ALARM_B_HOURS 12
+#define RS5C_REG_ALARM_B_WDAY 13 /* (ALARM_B only) */
+
+#define RS5C_REG_CTRL1 14
+# define RS5C_CTRL1_AALE (1 7)/* or WALE */
+# define RS5C_CTRL1_BALE (1 6)/* or DALE */
+# define RV5C387_CTRL1_24 (1 5)
+# define RS5C372A_CTRL1_SL1 (1 5)
+# define RS5C_CTRL1_CT_MASK (7 0)
+# define RS5C_CTRL1_CT0 (0 0)/* no periodic irq */
+# define RS5C_CTRL1_CT4 (4 0)/* 1 Hz level irq */
+#define RS5C_REG_CTRL2 15
+# define RS5C372_CTRL2_24 (1 5)
+# define RS5C_CTRL2_XSTP (1 4)
+# define RS5C_CTRL2_CTFG (1 2)
+# define RS5C_CTRL2_AAFG (1 1)/* or WAFG */
+# define RS5C_CTRL2_BAFG (1 0)/* or DAFG */
+
+
+/* to read (style 1) or write registers starting at R */
+#define RS5C_ADDR(R) (((R) 4) | 0)
+
+
+enum rtc_type {
+ rtc_undef = 0,
+ rtc_rs5c372a,
+ rtc_rs5c372b,
+ rtc_rv5c386,
+ rtc_rv5c387a,
+};
+/* REVISIT: this assumes that:
+ * - we're in the 21st century, so it's safe to ignore the century
+ *bit for rv5c38[67] (REG_MONTH bit 7);
+ * - we should use ALARM_A not ALARM_B (may be wrong on some boards)
+ */
struct rs5c372 {
- u8 reg_addr;
- u8 regs[17];
- struct i2c_msg msg[1];
- struct i2c_client client;
- struct rtc_device *rtc;
-};
+ struct i2c_client *client;
+ struct rtc_device *rtc;
+
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
Executive summary for the new in CC list: Is it possible that i2c-iop3xx driver in current mainline Linux is buggy regarding repeated start conditions? Dan Williams wrote: According to the latest specification update (http://www.intel.com/design/iio/specupdt/27351910.pdf) there are no known issues with the i2c. I looked through the thread and did not see what board you are using, can you send those details? We are using Thecus n2100, which has a IOP 80219. The vendor itself patched iq31244 board file, so presumably it's very similar. http://www.debonaras.org/wiki/Info/ThecusN2100Internals Any more specific information you want? I have not dealt with the i2c-iop3xx driver in the past. Have you tried contacting the last person to make functional changes to the driver? Well, now we have :) > Hi Riku, this is the first message I have received. This what I sent then: -snip- We have an Ricoh 5c372 RTC [1] which uses "repeated start" for internal register access. On a ixp-4xx device (Synology DS101), the mainline linux driver suposedly worked fine. On our device, a thecus n2100 with IOP 80219, the driver almost works. See page 31 on the datasheet[1] for example read transfer. After some debugging, it seems the RTC is ignoring the internal address setting request. On iop, we allways get the contents of beginning from internal register 0xF, which is the default internal address it sets after a stop condition. I'm not equipped with a scope, so I don't see what's happening on the wire.. I have a workaround for this specific case, but I don't like it, and this could be problem with other chips as well. -snip- Since then I learned the same rtc is in use with Kuro Boxes, which use a freescale ppc soc. There both mainline driver and my patched driver works fine. [1] http://www.ricoh.com/LSI/product_rtc/2wire/5c372/5c372a-e.pdf [2] http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;hb=HEAD;f=drivers/rtc/rtc-rs5c372.c - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Monday 11 December 2006 2:23 pm, Voipio Riku wrote: > from what I saw, the driver simply passes messages over to the i2c > controller. It even specifically mentiones that it supports repeated start > conditions, as needed for read method #1. Comparing to 80219 manual[1], I > did not spot anything obviously wrong. I skimmed i2c-ixp3xx too, but have never spent much time with I2C controller drivers or Intel's fancier XScales. > >> With your patch, the rtc acts like the chip would completely ignore the > >> "address" transfer, and starts reading from the last (default) register > >> anyway. Thus all the regs look shifted by one in the driver. > > > That's quite strange. The docs on the RTC are quite clear about what's > > supposed to happen with what I2C messages. And I'd expect them to be > > right ... especially since they behaved for me, and the original author > > of that code! That makes me suspect that your particular I2C controller > > driver must not be issuing the protocol requests it should be, at least > > on your hardware and revision. > > Well at least I'm happy that there is now someone more experienced working > on this driver. When I tried to get it working I could not find anyone > with another board to verify if the original and/or my patch works for > them.. Unfortunately our patches collided. The original code worked for me (other than bugs in the trim register). > > Plus, if I understand things correctly, using mode #3 would break when > > writing > > I should not. Writing isn't related to reading methods according the > datasheet[2]. It provides one addressing method for writing, and writing > works fine our Thecus/Allnet hardware. I see ... reading more closely "the internal address pointer is set to Fh when the stop condition is met", namely right after each transaction. It's not like other chips with such pointers that I've used (they never reset it). I don't mind if the "read all the registers" operation uses mode 3. I'll have to see if your updated version behaves (albeit without handling 12 hour time, the alarm, etc) for me. But I'm still concerned that switching to mode 3 seems to be just working around a bug in some other driver, and that _other_ bug should be fixed. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Monday 11 December 2006 3:33 pm, Dan Williams wrote: > > According to the latest specification update > (http://www.intel.com/design/iio/specupdt/27351910.pdf) there are no > known issues with the i2c. That's for the 80319 ... Riku said he was using 80219, that could imply some differences. And I distinctly recall Intel's XScale docs having a few problems whereby "live" silicon bugs didn't always stay in the "spec update" documents, so it's possible that older docs would be needed. At this point all we really _know_ is that requests made through 80219's i2c controller don't give the correct result (at least on Riku's 80219 board) and moreover seem to have a consistent failure mode ... whereas ones made with OMAP's controller (and presumably that of the original driver author, for a Synology DS-101 presumably with IXP420) do act ok. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Monday 11 December 2006 3:33 pm, Dan Williams wrote: According to the latest specification update (http://www.intel.com/design/iio/specupdt/27351910.pdf) there are no known issues with the i2c. That's for the 80319 ... Riku said he was using 80219, that could imply some differences. And I distinctly recall Intel's XScale docs having a few problems whereby live silicon bugs didn't always stay in the spec update documents, so it's possible that older docs would be needed. At this point all we really _know_ is that requests made through 80219's i2c controller don't give the correct result (at least on Riku's 80219 board) and moreover seem to have a consistent failure mode ... whereas ones made with OMAP's controller (and presumably that of the original driver author, for a Synology DS-101 presumably with IXP420) do act ok. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Monday 11 December 2006 2:23 pm, Voipio Riku wrote: from what I saw, the driver simply passes messages over to the i2c controller. It even specifically mentiones that it supports repeated start conditions, as needed for read method #1. Comparing to 80219 manual[1], I did not spot anything obviously wrong. I skimmed i2c-ixp3xx too, but have never spent much time with I2C controller drivers or Intel's fancier XScales. With your patch, the rtc acts like the chip would completely ignore the address transfer, and starts reading from the last (default) register anyway. Thus all the regs look shifted by one in the driver. That's quite strange. The docs on the RTC are quite clear about what's supposed to happen with what I2C messages. And I'd expect them to be right ... especially since they behaved for me, and the original author of that code! That makes me suspect that your particular I2C controller driver must not be issuing the protocol requests it should be, at least on your hardware and revision. Well at least I'm happy that there is now someone more experienced working on this driver. When I tried to get it working I could not find anyone with another board to verify if the original and/or my patch works for them.. Unfortunately our patches collided. The original code worked for me (other than bugs in the trim register). Plus, if I understand things correctly, using mode #3 would break when writing I should not. Writing isn't related to reading methods according the datasheet[2]. It provides one addressing method for writing, and writing works fine our Thecus/Allnet hardware. I see ... reading more closely the internal address pointer is set to Fh when the stop condition is met, namely right after each transaction. It's not like other chips with such pointers that I've used (they never reset it). I don't mind if the read all the registers operation uses mode 3. I'll have to see if your updated version behaves (albeit without handling 12 hour time, the alarm, etc) for me. But I'm still concerned that switching to mode 3 seems to be just working around a bug in some other driver, and that _other_ bug should be fixed. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
Executive summary for the new in CC list: Is it possible that i2c-iop3xx driver in current mainline Linux is buggy regarding repeated start conditions? Dan Williams wrote: According to the latest specification update (http://www.intel.com/design/iio/specupdt/27351910.pdf) there are no known issues with the i2c. I looked through the thread and did not see what board you are using, can you send those details? We are using Thecus n2100, which has a IOP 80219. The vendor itself patched iq31244 board file, so presumably it's very similar. http://www.debonaras.org/wiki/Info/ThecusN2100Internals Any more specific information you want? I have not dealt with the i2c-iop3xx driver in the past. Have you tried contacting the last person to make functional changes to the driver? Well, now we have :) Hi Riku, this is the first message I have received. This what I sent then: -snip- We have an Ricoh 5c372 RTC [1] which uses repeated start for internal register access. On a ixp-4xx device (Synology DS101), the mainline linux driver suposedly worked fine. On our device, a thecus n2100 with IOP 80219, the driver almost works. See page 31 on the datasheet[1] for example read transfer. After some debugging, it seems the RTC is ignoring the internal address setting request. On iop, we allways get the contents of beginning from internal register 0xF, which is the default internal address it sets after a stop condition. I'm not equipped with a scope, so I don't see what's happening on the wire.. I have a workaround for this specific case, but I don't like it, and this could be problem with other chips as well. -snip- Since then I learned the same rtc is in use with Kuro Boxes, which use a freescale ppc soc. There both mainline driver and my patched driver works fine. [1] http://www.ricoh.com/LSI/product_rtc/2wire/5c372/5c372a-e.pdf [2] http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;hb=HEAD;f=drivers/rtc/rtc-rs5c372.c - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On 12/11/06, Voipio Riku <[EMAIL PROTECTED]> wrote: > Have you asked around for anyone who may have insights about i2c-iop3xx > driver bugs? Maybe the driver maintainers, or arm-linux folk, or on > the i2c list. I was told to contact Dan Williams, I didn't get any response. Hi Riku, this is the first message I have received. According to the latest specification update (http://www.intel.com/design/iio/specupdt/27351910.pdf) there are no known issues with the i2c. I looked through the thread and did not see what board you are using, can you send those details? I have not dealt with the i2c-iop3xx driver in the past. Have you tried contacting the last person to make functional changes to the driver? http://kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=39288e1ac10b3b9a68a629be67d81a0b53512c4e Regards, Dan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
> On Sunday 10 December 2006 10:27 pm, Voipio Riku wrote: >> > Update the rtc-rs5c372 driver: >> > I suspect the >> > issue wasn't that "mode 1" didn't work on that board; the original >> > code to fetch the trim was broken. If "mode 1" really won't work, >> > that's almost certainly a bug in that board's I2C driver. >> It was not related to trim fetching. Yes, it very likely that the boards >> i2c controller (i2c-iop3xx) is has a bug, but I'm not competent enough >> to >> find out what it is actually sending out to the wire. > I'd expect that would be the controller _driver_ ... although it would > not surprise me to know there were also (unfixed) silicon bugs to cope > with, like version-specific differences. One hopes errata are published > for the chip you're using, and that they don't lie. from what I saw, the driver simply passes messages over to the i2c controller. It even specifically mentiones that it supports repeated start conditions, as needed for read method #1. Comparing to 80219 manual[1], I did not spot anything obviously wrong. > Have you asked around for anyone who may have insights about i2c-iop3xx > driver bugs? Maybe the driver maintainers, or arm-linux folk, or on > the i2c list. I was told to contact Dan Williams, I didn't get any response. >> With your patch, the rtc acts like the chip would completely ignore the >> "address" transfer, and starts reading from the last (default) register >> anyway. Thus all the regs look shifted by one in the driver. > That's quite strange. The docs on the RTC are quite clear about what's > supposed to happen with what I2C messages. And I'd expect them to be > right ... especially since they behaved for me, and the original author > of that code! That makes me suspect that your particular I2C controller > driver must not be issuing the protocol requests it should be, at least > on your hardware and revision. Well at least I'm happy that there is now someone more experienced working on this driver. When I tried to get it working I could not find anyone with another board to verify if the original and/or my patch works for them.. >> > + /* this implements the first (most portable) reading method >> > + * specified in the datasheet. >> > */ >> Why is this method considered more portable? Howabout making the read >> method a module parameter? > Of the three methods, #2 depends on messages that not all I2C masters > are necessarily going to be able to issue, and #3 assumes that there's > no other I2C master accessing that chip. Agreed, I wouldn't consider method #2 either. > Plus, if I understand things correctly, using mode #3 would break when > writing I should not. Writing isn't related to reading methods according the datasheet[2]. It provides one addressing method for writing, and writing works fine our Thecus/Allnet hardware. [1] http://www.intel.com/design/iio/manuals/274017.htm [2] http://www.ricoh.com/LSI/product_rtc/2wire/5c372/5c372a-e.pdf - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Sunday 10 December 2006 10:27 pm, Voipio Riku wrote: > > Update the rtc-rs5c372 driver: > > I suspect the > > issue wasn't that "mode 1" didn't work on that board; the original > > code to fetch the trim was broken. If "mode 1" really won't work, > > that's almost certainly a bug in that board's I2C driver. > > It was not related to trim fetching. Yes, it very likely that the boards > i2c controller (i2c-iop3xx) is has a bug, but I'm not competent enough to > find out what it is actually sending out to the wire. I'd expect that would be the controller _driver_ ... although it would not surprise me to know there were also (unfixed) silicon bugs to cope with, like version-specific differences. One hopes errata are published for the chip you're using, and that they don't lie. Have you asked around for anyone who may have insights about i2c-iop3xx driver bugs? Maybe the driver maintainers, or arm-linux folk, or on the i2c list. I did notice the changelog for that driver included changes (e.g. July 1 this year) for chip-specific differences ... there could easily be issues like that still lingering. > With your patch, the rtc acts like the chip would completely ignore the > "address" transfer, and starts reading from the last (default) register > anyway. Thus all the regs look shifted by one in the driver. That's quite strange. The docs on the RTC are quite clear about what's supposed to happen with what I2C messages. And I'd expect them to be right ... especially since they behaved for me, and the original author of that code! That makes me suspect that your particular I2C controller driver must not be issuing the protocol requests it should be, at least on your hardware and revision. > > + /* this implements the first (most portable) reading method > > +* specified in the datasheet. > > */ > > Why is this method considered more portable? Howabout making the read > method a module parameter? Of the three methods, #2 depends on messages that not all I2C masters are necessarily going to be able to issue, and #3 assumes that there's no other I2C master accessing that chip. Plus, if I understand things correctly, using mode #3 would break when writing e.g. just REG_CTRL1 to enable alarms or force an uninitialized chip into 24 hr mode ... or when writing the alarm. Because, just as with the multi-master case, the implicit register pointer would no longer stay at "15" all the time. So I don't see how having an option to choose the mode would be a good solution. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Sunday 10 December 2006 10:27 pm, Voipio Riku wrote: Update the rtc-rs5c372 driver: I suspect the issue wasn't that mode 1 didn't work on that board; the original code to fetch the trim was broken. If mode 1 really won't work, that's almost certainly a bug in that board's I2C driver. It was not related to trim fetching. Yes, it very likely that the boards i2c controller (i2c-iop3xx) is has a bug, but I'm not competent enough to find out what it is actually sending out to the wire. I'd expect that would be the controller _driver_ ... although it would not surprise me to know there were also (unfixed) silicon bugs to cope with, like version-specific differences. One hopes errata are published for the chip you're using, and that they don't lie. Have you asked around for anyone who may have insights about i2c-iop3xx driver bugs? Maybe the driver maintainers, or arm-linux folk, or on the i2c list. I did notice the changelog for that driver included changes (e.g. July 1 this year) for chip-specific differences ... there could easily be issues like that still lingering. With your patch, the rtc acts like the chip would completely ignore the address transfer, and starts reading from the last (default) register anyway. Thus all the regs look shifted by one in the driver. That's quite strange. The docs on the RTC are quite clear about what's supposed to happen with what I2C messages. And I'd expect them to be right ... especially since they behaved for me, and the original author of that code! That makes me suspect that your particular I2C controller driver must not be issuing the protocol requests it should be, at least on your hardware and revision. + /* this implements the first (most portable) reading method +* specified in the datasheet. */ Why is this method considered more portable? Howabout making the read method a module parameter? Of the three methods, #2 depends on messages that not all I2C masters are necessarily going to be able to issue, and #3 assumes that there's no other I2C master accessing that chip. Plus, if I understand things correctly, using mode #3 would break when writing e.g. just REG_CTRL1 to enable alarms or force an uninitialized chip into 24 hr mode ... or when writing the alarm. Because, just as with the multi-master case, the implicit register pointer would no longer stay at 15 all the time. So I don't see how having an option to choose the mode would be a good solution. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On Sunday 10 December 2006 10:27 pm, Voipio Riku wrote: Update the rtc-rs5c372 driver: I suspect the issue wasn't that mode 1 didn't work on that board; the original code to fetch the trim was broken. If mode 1 really won't work, that's almost certainly a bug in that board's I2C driver. It was not related to trim fetching. Yes, it very likely that the boards i2c controller (i2c-iop3xx) is has a bug, but I'm not competent enough to find out what it is actually sending out to the wire. I'd expect that would be the controller _driver_ ... although it would not surprise me to know there were also (unfixed) silicon bugs to cope with, like version-specific differences. One hopes errata are published for the chip you're using, and that they don't lie. from what I saw, the driver simply passes messages over to the i2c controller. It even specifically mentiones that it supports repeated start conditions, as needed for read method #1. Comparing to 80219 manual[1], I did not spot anything obviously wrong. Have you asked around for anyone who may have insights about i2c-iop3xx driver bugs? Maybe the driver maintainers, or arm-linux folk, or on the i2c list. I was told to contact Dan Williams, I didn't get any response. With your patch, the rtc acts like the chip would completely ignore the address transfer, and starts reading from the last (default) register anyway. Thus all the regs look shifted by one in the driver. That's quite strange. The docs on the RTC are quite clear about what's supposed to happen with what I2C messages. And I'd expect them to be right ... especially since they behaved for me, and the original author of that code! That makes me suspect that your particular I2C controller driver must not be issuing the protocol requests it should be, at least on your hardware and revision. Well at least I'm happy that there is now someone more experienced working on this driver. When I tried to get it working I could not find anyone with another board to verify if the original and/or my patch works for them.. + /* this implements the first (most portable) reading method + * specified in the datasheet. */ Why is this method considered more portable? Howabout making the read method a module parameter? Of the three methods, #2 depends on messages that not all I2C masters are necessarily going to be able to issue, and #3 assumes that there's no other I2C master accessing that chip. Agreed, I wouldn't consider method #2 either. Plus, if I understand things correctly, using mode #3 would break when writing I should not. Writing isn't related to reading methods according the datasheet[2]. It provides one addressing method for writing, and writing works fine our Thecus/Allnet hardware. [1] http://www.intel.com/design/iio/manuals/274017.htm [2] http://www.ricoh.com/LSI/product_rtc/2wire/5c372/5c372a-e.pdf - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
On 12/11/06, Voipio Riku [EMAIL PROTECTED] wrote: snip Have you asked around for anyone who may have insights about i2c-iop3xx driver bugs? Maybe the driver maintainers, or arm-linux folk, or on the i2c list. I was told to contact Dan Williams, I didn't get any response. Hi Riku, this is the first message I have received. According to the latest specification update (http://www.intel.com/design/iio/specupdt/27351910.pdf) there are no known issues with the i2c. I looked through the thread and did not see what board you are using, can you send those details? I have not dealt with the i2c-iop3xx driver in the past. Have you tried contacting the last person to make functional changes to the driver? http://kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=39288e1ac10b3b9a68a629be67d81a0b53512c4e Regards, Dan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
> Update the rtc-rs5c372 driver: > I suspect the > issue wasn't that "mode 1" didn't work on that board; the original > code to fetch the trim was broken. If "mode 1" really won't work, > that's almost certainly a bug in that board's I2C driver. It was not related to trim fetching. Yes, it very likely that the boards i2c controller (i2c-iop3xx) is has a bug, but I'm not competent enough to find out what it is actually sending out to the wire. With your patch, the rtc acts like the chip would completely ignore the "address" transfer, and starts reading from the last (default) register anyway. Thus all the regs look shifted by one in the driver. With current git driver results look like: # cat /proc/driver/rtc ; sleep 1 ; echo one sec gone; cat /proc/driver/rtc rtc_time: 06:16:55 rtc_date: 2006-12-11 24hr: yes 32.768 KHz trim: 1 one sec gone rtc_time: 06:16:56 rtc_date: 2006-12-11 24hr: yes 32.768 KHz trim: 1 And same after your patch: rtc_time: 16:23:28 rtc_date: 2012-11-01 alrm_time : **:01:00 alrm_date : -**-** alrm_wakeup : no alrm_pending: no 24hr: yes crystal : 32.768 KHz trim: 10 one sec gone rtc_time: 16:24:28 rtc_date: 2012-11-01 alrm_time : **:01:00 alrm_date : -**-** alrm_wakeup : no alrm_pending: no 24hr: yes crystal : 32.768 KHz trim: 10 > + /* this implements the first (most portable) reading method > + * specified in the datasheet. >*/ Why is this method considered more portable? Howabout making the read method a module parameter? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.19-git] rts-rs5c372 updates: more chips, alarm, 12hr mode, etc
Update the rtc-rs5c372 driver: I suspect the issue wasn't that mode 1 didn't work on that board; the original code to fetch the trim was broken. If mode 1 really won't work, that's almost certainly a bug in that board's I2C driver. It was not related to trim fetching. Yes, it very likely that the boards i2c controller (i2c-iop3xx) is has a bug, but I'm not competent enough to find out what it is actually sending out to the wire. With your patch, the rtc acts like the chip would completely ignore the address transfer, and starts reading from the last (default) register anyway. Thus all the regs look shifted by one in the driver. With current git driver results look like: # cat /proc/driver/rtc ; sleep 1 ; echo one sec gone; cat /proc/driver/rtc rtc_time: 06:16:55 rtc_date: 2006-12-11 24hr: yes 32.768 KHz trim: 1 one sec gone rtc_time: 06:16:56 rtc_date: 2006-12-11 24hr: yes 32.768 KHz trim: 1 And same after your patch: rtc_time: 16:23:28 rtc_date: 2012-11-01 alrm_time : **:01:00 alrm_date : -**-** alrm_wakeup : no alrm_pending: no 24hr: yes crystal : 32.768 KHz trim: 10 one sec gone rtc_time: 16:24:28 rtc_date: 2012-11-01 alrm_time : **:01:00 alrm_date : -**-** alrm_wakeup : no alrm_pending: no 24hr: yes crystal : 32.768 KHz trim: 10 + /* this implements the first (most portable) reading method + * specified in the datasheet. */ Why is this method considered more portable? Howabout making the read method a module parameter? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
