[PATCH] rtc: rtc-max6900: SMBus support.

[PATCH] rtc: rtc-max6900: SMBus support.

[Paul Mundt]

The current rtc-max6900 driver only supports getting and setting the time
through bursting, which requires a controller capable of such. As
bursting is merely an optimization, we can also opt for the non-burst
fallback path that pokes at the date/time registers individually.

By opting for the fallback solution, it's possible to fully support the
RTC on controllers that only implement I2C_FUNC_SMBUS_BYTE_DATA instead
of requiring master transfers via I2C_FUNC_I2C.

This was tested with an I2C_FUNC_SMBUS_BYTE_DATA controller with a MAX6909
attached. Currently only a subset of the MAX6909 functionality is supported,
patches to extend the current driver to be forthcoming.

There are no functional changes for I2C_FUNC_I2C controllers.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 drivers/rtc/Kconfig       |    4 +-
 drivers/rtc/rtc-max6900.c |   83 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8167e9e..1f74b46 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -167,10 +167,10 @@ config RTC_DRV_DS1672
 	  will be called rtc-ds1672.
 
 config RTC_DRV_MAX6900
-	tristate "Maxim MAX6900"
+	tristate "Maxim MAX6900/MAX6909"
 	help
 	  If you say yes here you will get support for the
-	  Maxim MAX6900 I2C RTC chip.
+	  Maxim MAX6900 and MAX6909 I2C RTC chips.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-max6900.
diff --git a/drivers/rtc/rtc-max6900.c b/drivers/rtc/rtc-max6900.c
index a4f6665..5e0c289 100644
--- a/drivers/rtc/rtc-max6900.c
+++ b/drivers/rtc/rtc-max6900.c
@@ -1,8 +1,10 @@
 /*
- * rtc class driver for the Maxim MAX6900 chip
+ * rtc class driver for the Maxim MAX6900/MAX6909 chip
  *
  * Author: Dale Farnsworth <dale@farnsworth.org>
  *
+ * SMBus support by Paul Mundt.
+ *
  * based on previously existing rtc class drivers
  *
  * 2007 (c) MontaVista, Software, Inc.  This file is licensed under
@@ -17,7 +19,7 @@
 #include <linux/rtc.h>
 #include <linux/delay.h>
 
-#define DRV_VERSION "0.2"
+#define DRV_VERSION "0.3"
 
 /*
  * register indices
@@ -39,6 +41,12 @@
 #define MAX6900_REG_CT_WP		(1 << 7)	/* Write Protect */
 
 /*
+ * index to command wrappers
+ */
+#define MAX6900_REG_WRITE(reg)		(0x80 + ((reg) * sizeof(u16)))
+#define MAX6900_REG_READ(reg)		(0x81 + ((reg) * sizeof(u16)))
+
+/*
  * register read/write commands
  */
 #define MAX6900_REG_CONTROL_WRITE	0x8e
@@ -52,7 +60,9 @@
 
 static struct i2c_driver max6900_driver;
 
-static int max6900_i2c_read_regs(struct i2c_client *client, u8 *buf)
+static unsigned int smbus_mode;	/* disabled by default, prefer bursting */
+
+static int max6900_i2c_burst_read(struct i2c_client *client, u8 *buf)
 {
 	u8 reg_burst_read[1] = { MAX6900_REG_BURST_READ };
 	u8 reg_century_read[1] = { MAX6900_REG_CENTURY_READ };
@@ -92,7 +102,7 @@ static int max6900_i2c_read_regs(struct i2c_client *client, u8 *buf)
 	return 0;
 }
 
-static int max6900_i2c_write_regs(struct i2c_client *client, u8 const *buf)
+static int max6900_i2c_burst_write(struct i2c_client *client, u8 const *buf)
 {
 	u8 i2c_century_buf[1 + 1] = { MAX6900_REG_CENTURY_WRITE };
 	struct i2c_msg century_msgs[1] = {
@@ -141,6 +151,58 @@ static int max6900_i2c_write_regs(struct i2c_client *client, u8 const *buf)
 	return -EIO;
 }
 
+static int max6900_i2c_smbus_read(struct i2c_client *client, u8 *buf)
+{
+	int rc = 0, i;
+
+	for (i = 0; i < MAX6900_REG_LEN; i++) {
+		rc = i2c_smbus_read_byte_data(client, MAX6900_REG_READ(i));
+		if (rc < 0) {
+			dev_err(&client->dev, "%s: register read failed\n",
+				__func__);
+			return rc;
+		}
+		buf[i] = rc;
+	}
+
+	return 0;
+}
+
+static int max6900_i2c_smbus_write(struct i2c_client *client, u8 const *buf)
+{
+	int rc = 0, i;
+
+	for (i = 0; i < MAX6900_REG_LEN; i++)
+		rc |= i2c_smbus_write_byte_data(client,
+				MAX6900_REG_WRITE(i), buf[i]);
+
+	if (rc < 0) {
+		dev_err(&client->dev, "%s: register write failed\n",
+			__func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static inline int
+max6900_i2c_read_regs(struct i2c_client *client, u8 *buf)
+{
+	if (smbus_mode)
+		return max6900_i2c_smbus_read(client, buf);
+
+	return max6900_i2c_burst_read(client, buf);
+}
+
+static inline int
+max6900_i2c_write_regs(struct i2c_client *client, u8 const *buf)
+{
+	if (smbus_mode)
+		return max6900_i2c_smbus_write(client, buf);
+
+	return max6900_i2c_burst_write(client, buf);
+}
+
 static int max6900_i2c_read_time(struct i2c_client *client, struct rtc_time *tm)
 {
 	int rc;
@@ -232,8 +294,17 @@ max6900_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
 	struct rtc_device *rtc;
 
-	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
-		return -ENODEV;
+	/*
+	 * Prefer burst mode if there's a fully functional I2C adapter to
+	 * work with, otherwise scrape by with SMBus.
+	 */
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		if (i2c_check_functionality(client->adapter,
+				    I2C_FUNC_SMBUS_BYTE_DATA))
+			smbus_mode = 1;
+		else
+			return -ENODEV;
+	}
 
 	dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n");
 

Re: [PATCH] rtc: rtc-max6900: SMBus support.

[Dale Farnsworth]

On Thu, Jan 14, 2010 at 08:32:35PM +0900, Paul Mundt wrote:
> The current rtc-max6900 driver only supports getting and setting the time
> through bursting, which requires a controller capable of such. As
> bursting is merely an optimization, we can also opt for the non-burst
> fallback path that pokes at the date/time registers individually.
> 
> By opting for the fallback solution, it's possible to fully support the
> RTC on controllers that only implement I2C_FUNC_SMBUS_BYTE_DATA instead
> of requiring master transfers via I2C_FUNC_I2C.
> 
> This was tested with an I2C_FUNC_SMBUS_BYTE_DATA controller with a MAX6909
> attached. Currently only a subset of the MAX6909 functionality is supported,
> patches to extend the current driver to be forthcoming.
> 
> There are no functional changes for I2C_FUNC_I2C controllers.
> 
> Signed-off-by: Paul Mundt <lethal@linux-sh.org>

Acked-by: Dale Farnsworth <dale@farnsworth.org>

Re: [rtc-linux] [PATCH] rtc: rtc-max6900: SMBus support.

[Alessandro Zummo]

On Thu, 14 Jan 2010 20:32:35 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> The current rtc-max6900 driver only supports getting and setting the time
> through bursting, which requires a controller capable of such. As
> bursting is merely an optimization, we can also opt for the non-burst
> fallback path that pokes at the date/time registers individually.

 almost ok, review below.


> -static int max6900_i2c_read_regs(struct i2c_client *client, u8 *buf)
> +static unsigned int smbus_mode;	/* disabled by default, prefer bursting */

 that one should be per-device.

> +static int max6900_i2c_smbus_read(struct i2c_client *client, u8 *buf)
> +{
> +	int rc = 0, i;

 no need to init rc

> +static int max6900_i2c_smbus_write(struct i2c_client *client, u8 const *buf)
> +{
> +	int rc = 0, i;
> +
> +	for (i = 0; i < MAX6900_REG_LEN; i++)
> +		rc |= i2c_smbus_write_byte_data(client,
> +				MAX6900_REG_WRITE(i), buf[i]);

 please check rc at each write.

 you might want to check if you'll have troubles if you read/write
 when the minutes are changing.

 you might also want to investigate block SMbus messages. 

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it