[PATCH 1/5] drivers: w1: omap_hdq: cleanup and bug fixes.

Sourav Poddar
2014-04-16T12:32:54Z

The patch adds the following to the omap hdq driver.
1. HDQ Device reset call in probe.
2. Enabling '1 wire mode' and checking for presence pulse bit.
3. Proper disabling and enabling of interrupts during read path.
4. Add re-initialization code during SKIP ROM command execution.
5. Miscellaneous cleanup(formatting, return error checks).

Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
drivers/w1/masters/omap_hdq.c | 85 ++++++++++++++++++++++++++++++++---------
1 file changed, 68 insertions(+), 17 deletions(-)

diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c
index 9900e8e..0a7bf7f 100644
--- a/drivers/w1/masters/omap_hdq.c
+++ b/drivers/w1/masters/omap_hdq.c
@@ -27,21 +27,22 @@
#define OMAP_HDQ_TX_DATA 0x04
#define OMAP_HDQ_RX_DATA 0x08
#define OMAP_HDQ_CTRL_STATUS 0x0c
-#define OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK (1<<6)
-#define OMAP_HDQ_CTRL_STATUS_CLOCKENABLE (1<<5)
-#define OMAP_HDQ_CTRL_STATUS_GO (1<<4)
-#define OMAP_HDQ_CTRL_STATUS_INITIALIZATION (1<<2)
-#define OMAP_HDQ_CTRL_STATUS_DIR (1<<1)
-#define OMAP_HDQ_CTRL_STATUS_MODE (1<<0)
+#define OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK (1 << 6)
+#define OMAP_HDQ_CTRL_STATUS_CLOCKENABLE (1 << 5)
+#define OMAP_HDQ_CTRL_STATUS_PRESENCE (1 << 3)
+#define OMAP_HDQ_CTRL_STATUS_GO (1 << 4)
+#define OMAP_HDQ_CTRL_STATUS_INITIALIZATION (1 << 2)
+#define OMAP_HDQ_CTRL_STATUS_DIR (1 << 1)
+#define OMAP_HDQ_CTRL_STATUS_MODE (1 << 0)
#define OMAP_HDQ_INT_STATUS 0x10
-#define OMAP_HDQ_INT_STATUS_TXCOMPLETE (1<<2)
-#define OMAP_HDQ_INT_STATUS_RXCOMPLETE (1<<1)
-#define OMAP_HDQ_INT_STATUS_TIMEOUT (1<<0)
+#define OMAP_HDQ_INT_STATUS_TXCOMPLETE (1 << 2)
+#define OMAP_HDQ_INT_STATUS_RXCOMPLETE (1 << 1)
+#define OMAP_HDQ_INT_STATUS_TIMEOUT (1 << 0)
#define OMAP_HDQ_SYSCONFIG 0x14
-#define OMAP_HDQ_SYSCONFIG_SOFTRESET (1<<1)
-#define OMAP_HDQ_SYSCONFIG_AUTOIDLE (1<<0)
+#define OMAP_HDQ_SYSCONFIG_SOFTRESET (1 << 1)
+#define OMAP_HDQ_SYSCONFIG_AUTOIDLE (1 << 0)
#define OMAP_HDQ_SYSSTATUS 0x18
-#define OMAP_HDQ_SYSSTATUS_RESETDONE (1<<0)
+#define OMAP_HDQ_SYSSTATUS_RESETDONE (1 << 0)

#define OMAP_HDQ_FLAG_CLEAR 0
#define OMAP_HDQ_FLAG_SET 1
@@ -115,6 +116,15 @@ static inline u8 hdq_reg_merge(struct hdq_data *hdq_data, u32 offset,
return new_val;
}

+static void hdq_disable_interrupt(struct hdq_data *hdq_data, u32 offset,
+ u8 mask)
+{
+ u32 ie;
+
+ ie = readl(hdq_data->hdq_base + offset);
+ writel(ie & mask, hdq_data->hdq_base + offset);
+}
+
/*
* Wait for one or more bits in flag change.
* HDQ_FLAG_SET: wait until any bit in the flag is set.
@@ -263,8 +273,7 @@ static int _omap_hdq_reset(struct hdq_data *hdq_data)
* interrupt.
*/
hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS,
- OMAP_HDQ_CTRL_STATUS_CLOCKENABLE |
- OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK);
+ OMAP_HDQ_CTRL_STATUS_CLOCKENABLE);

/* wait for reset to complete */
ret = hdq_wait_for_flag(hdq_data, OMAP_HDQ_SYSSTATUS,
@@ -275,7 +284,8 @@ static int _omap_hdq_reset(struct hdq_data *hdq_data)
else {
hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS,
OMAP_HDQ_CTRL_STATUS_CLOCKENABLE |
- OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK);
+ OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK |
+ OMAP_HDQ_CTRL_STATUS_MODE);
hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG,
OMAP_HDQ_SYSCONFIG_AUTOIDLE);
}
@@ -327,6 +337,18 @@ static int omap_hdq_break(struct hdq_data *hdq_data)
ret = -ETIMEDOUT;
goto out;
}
+
+ /*
+ * check for the presence detect bit to get
+ * set to show that the slave is responding
+ */
+ if (hdq_reg_in(hdq_data, OMAP_HDQ_CTRL_STATUS) &
+ OMAP_HDQ_CTRL_STATUS_PRESENCE) {
+ dev_dbg(hdq_data->dev, "Presence bit not set\n");
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
/*
* wait for both INIT and GO bits rerurn to zero.
* zero wait time expected for interrupt mode.
@@ -361,6 +383,8 @@ static int hdq_read_byte(struct hdq_data *hdq_data, u8 *val)
goto out;
}

+ hdq_data->hdq_irqstatus = 0;
+
if (!(hdq_data->hdq_irqstatus & OMAP_HDQ_INT_STATUS_RXCOMPLETE)) {
hdq_reg_merge(hdq_data, OMAP_HDQ_CTRL_STATUS,
OMAP_HDQ_CTRL_STATUS_DIR | OMAP_HDQ_CTRL_STATUS_GO,
@@ -426,7 +450,8 @@ static int omap_hdq_get(struct hdq_data *hdq_data)
/* select HDQ mode & enable clocks */
hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS,
OMAP_HDQ_CTRL_STATUS_CLOCKENABLE |
- OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK);
+ OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK |
+ OMAP_HDQ_CTRL_STATUS_MODE);
hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG,
OMAP_HDQ_SYSCONFIG_AUTOIDLE);
hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
@@ -471,6 +496,10 @@ static u8 omap_w1_read_byte(void *_hdq)
u8 val = 0;
int ret;

+ /* First write to initialize the transfer */
+ if (hdq_data->init_trans == 0)
+ omap_hdq_get(hdq_data);
+
ret = hdq_read_byte(hdq_data, &val);
if (ret) {
ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
@@ -484,6 +513,10 @@ static u8 omap_w1_read_byte(void *_hdq)
return -1;
}

+ hdq_disable_interrupt(hdq_data, OMAP_HDQ_CTRL_STATUS,
+ ~OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK);
+ hdq_data->hdq_usecount = 0;
+
/* Write followed by a read, release the module */
if (hdq_data->init_trans) {
ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
@@ -510,6 +543,14 @@ static void omap_w1_write_byte(void *_hdq, u8 byte)
if (hdq_data->init_trans == 0)
omap_hdq_get(hdq_data);

+ /*
+ * We need to reset the slave before
+ * issuing the SKIP ROM command, else
+ * the slave will not work.
+ */
+ if (byte == W1_SKIP_ROM)
+ omap_hdq_break(hdq_data);
+
ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
if (ret < 0) {
dev_dbg(hdq_data->dev, "Could not acquire mutex\n");
@@ -563,7 +604,17 @@ static int omap_hdq_probe(struct platform_device *pdev)
mutex_init(&hdq_data->hdq_mutex);

pm_runtime_enable(&pdev->dev);
- pm_runtime_get_sync(&pdev->dev);
+ ret = pm_runtime_get_sync(&pdev->dev);
+ if (ret < 0) {
+ dev_dbg(&pdev->dev, "pm_runtime_get_sync failed\n");
+ goto err_w1;
+ }
+
+ ret = _omap_hdq_reset(hdq_data);
+ if (ret) {
+ dev_dbg(&pdev->dev, "reset failed\n");
+ return -EINVAL;
+ }

rev = hdq_reg_in(hdq_data, OMAP_HDQ_REVISION);
dev_info(&pdev->dev, "OMAP HDQ Hardware Rev %c.%c. Driver in %s mode\n",
--
1.7.9.5