* [PATCH v3 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface
@ 2011-02-22 17:49 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-22 17:49 UTC (permalink / raw)
To: linux-i2c
Cc: grant.likely, devicetree-discuss, john.williams, linux-kernel,
John.Linn, Michal Simek
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2944 bytes --]
Add OF support to Xilinx i2c bus interface.
It is used on Microblaze systems.
Signed-off-by: Michal Simek <monstr@monstr.eu>
---
drivers/i2c/busses/i2c-xiic.c | 37 +++++++++++++++++++++++++++++--------
1 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index a9c419e..0b7647b 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -41,6 +41,11 @@
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_i2c.h>
+#include <linux/of_address.h>
+
#define DRIVER_NAME "xiic-i2c"
enum xilinx_i2c_state {
@@ -426,7 +431,7 @@ static void xiic_process(struct xiic_i2c *i2c)
xiic_wakeup(i2c, STATE_ERROR);
} else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) {
- /* Transmit register/FIFO is empty or ½ empty */
+ /* Transmit register/FIFO is empty or 1/2 empty */
clr = pend &
(XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK);
@@ -704,10 +709,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
if (irq < 0)
goto resource_missing;
- pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
- if (!pdata)
- return -EINVAL;
-
i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
if (!i2c)
return -ENOMEM;
@@ -730,6 +731,7 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
i2c->adap = xiic_adapter;
i2c_set_adapdata(&i2c->adap, i2c);
i2c->adap.dev.parent = &pdev->dev;
+ i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node);
xiic_reinit(i2c);
@@ -748,9 +750,16 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
goto add_adapter_failed;
}
- /* add in known devices to the bus */
- for (i = 0; i < pdata->num_devices; i++)
- i2c_new_device(&i2c->adap, pdata->devices + i);
+ dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n",
+ (unsigned int)i2c->base, irq);
+
+ pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
+ if (pdata) {
+ /* add in known devices to the bus */
+ for (i = 0; i < pdata->num_devices; i++)
+ i2c_new_device(&i2c->adap, pdata->devices + i);
+ }
+ of_i2c_register_devices(&i2c->adap);
return 0;
@@ -799,12 +808,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev)
/* work with hotplug and coldplug */
MODULE_ALIAS("platform:"DRIVER_NAME);
+#ifdef CONFIG_OF
+/* Match table for of_platform binding */
+static struct of_device_id __devinitdata xilinx_iic_of_match[] = {
+ { .compatible = "xlnx,xps-iic-2.00.a", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, xilinx_iic_of_match);
+#else
+# define xilinx_iic_of_match NULL
+#endif
+
static struct platform_driver xiic_i2c_driver = {
.probe = xiic_i2c_probe,
.remove = __devexit_p(xiic_i2c_remove),
.driver = {
.owner = THIS_MODULE,
.name = DRIVER_NAME,
+ .of_match_table = xilinx_iic_of_match,
},
};
--
1.5.5.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface
@ 2011-02-22 17:49 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-22 17:49 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
john.williams-g5w7nrANp4BDPfheJLI6IQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
John.Linn-gjFFaj9aHVfQT0dZR+AlfA, Michal Simek
Add OF support to Xilinx i2c bus interface.
It is used on Microblaze systems.
Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 37 +++++++++++++++++++++++++++++--------
1 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index a9c419e..0b7647b 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -41,6 +41,11 @@
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_i2c.h>
+#include <linux/of_address.h>
+
#define DRIVER_NAME "xiic-i2c"
enum xilinx_i2c_state {
@@ -426,7 +431,7 @@ static void xiic_process(struct xiic_i2c *i2c)
xiic_wakeup(i2c, STATE_ERROR);
} else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) {
- /* Transmit register/FIFO is empty or ½ empty */
+ /* Transmit register/FIFO is empty or 1/2 empty */
clr = pend &
(XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK);
@@ -704,10 +709,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
if (irq < 0)
goto resource_missing;
- pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
- if (!pdata)
- return -EINVAL;
-
i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
if (!i2c)
return -ENOMEM;
@@ -730,6 +731,7 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
i2c->adap = xiic_adapter;
i2c_set_adapdata(&i2c->adap, i2c);
i2c->adap.dev.parent = &pdev->dev;
+ i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node);
xiic_reinit(i2c);
@@ -748,9 +750,16 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
goto add_adapter_failed;
}
- /* add in known devices to the bus */
- for (i = 0; i < pdata->num_devices; i++)
- i2c_new_device(&i2c->adap, pdata->devices + i);
+ dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n",
+ (unsigned int)i2c->base, irq);
+
+ pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
+ if (pdata) {
+ /* add in known devices to the bus */
+ for (i = 0; i < pdata->num_devices; i++)
+ i2c_new_device(&i2c->adap, pdata->devices + i);
+ }
+ of_i2c_register_devices(&i2c->adap);
return 0;
@@ -799,12 +808,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev)
/* work with hotplug and coldplug */
MODULE_ALIAS("platform:"DRIVER_NAME);
+#ifdef CONFIG_OF
+/* Match table for of_platform binding */
+static struct of_device_id __devinitdata xilinx_iic_of_match[] = {
+ { .compatible = "xlnx,xps-iic-2.00.a", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, xilinx_iic_of_match);
+#else
+# define xilinx_iic_of_match NULL
+#endif
+
static struct platform_driver xiic_i2c_driver = {
.probe = xiic_i2c_probe,
.remove = __devexit_p(xiic_i2c_remove),
.driver = {
.owner = THIS_MODULE,
.name = DRIVER_NAME,
+ .of_match_table = xilinx_iic_of_match,
},
};
--
1.5.5.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-22 17:49 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-22 17:49 UTC (permalink / raw)
To: linux-i2c
Cc: grant.likely, devicetree-discuss, john.williams, linux-kernel,
John.Linn, Michal Simek
i2c driver is used for LE/BE that's why is useful to use
32bit accesses. Then it is not necessary to solve any
endian issues.
Signed-off-by: Michal Simek <monstr@monstr.eu>
---
drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
1 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 0b7647b..78b3b82 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -178,21 +178,6 @@ struct xiic_i2c {
static void xiic_start_xfer(struct xiic_i2c *i2c);
static void __xiic_start_xfer(struct xiic_i2c *i2c);
-static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
-{
- iowrite8(value, i2c->base + reg);
-}
-
-static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
-{
- return ioread8(i2c->base + reg);
-}
-
-static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
-{
- iowrite16(value, i2c->base + reg);
-}
-
static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
{
iowrite32(value, i2c->base + reg);
@@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
{
u8 sr;
- for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
+ for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
!(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
- sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
+ sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
+ }
}
static void xiic_reinit(struct xiic_i2c *i2c)
@@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
/* Set receive Fifo depth to maximum (zero based). */
- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
/* Reset Tx Fifo. */
- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
/* Enable IIC Device, remove Tx Fifo reset & disable general call. */
- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
/* make sure RX fifo is empty */
xiic_clear_rx_fifo(i2c);
@@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
/* Disable IIC Device. */
- cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
+ cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
+ cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
}
static void xiic_read_rx(struct xiic_i2c *i2c)
@@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
u8 bytes_in_fifo;
int i;
- bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
+ bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
", SR: 0x%x, CR: 0x%x\n",
__func__, bytes_in_fifo, xiic_rx_space(i2c),
- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
if (bytes_in_fifo > xiic_rx_space(i2c))
bytes_in_fifo = xiic_rx_space(i2c);
for (i = 0; i < bytes_in_fifo; i++)
i2c->rx_msg->buf[i2c->rx_pos++] =
- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
(xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
}
@@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
{
/* return the actual space left in the FIFO */
- return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
+ return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
}
static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
@@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
data |= XIIC_TX_DYN_STOP_MASK;
dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
} else
- xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
}
}
@@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
"pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
- __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
+ __func__, ier, isr, pend,
+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
i2c->tx_msg, i2c->nmsgs);
/* Do not processes a devices interrupts if the device has no
@@ -479,7 +467,7 @@ out:
static int xiic_bus_busy(struct xiic_i2c *i2c)
{
- u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
+ u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
}
@@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
rx_watermark = msg->len;
if (rx_watermark > IIC_RX_FIFO_DEPTH)
rx_watermark = IIC_RX_FIFO_DEPTH;
- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
if (!(msg->flags & I2C_M_NOSTART))
/* write the address */
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
(msg->addr << 1) | XIIC_READ_OPERATION |
XIIC_TX_DYN_START_MASK);
xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
if (i2c->nmsgs == 1)
/* very last, enable bus not busy as well */
@@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
"ISR: 0x%x, CR: 0x%x\n",
__func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
if (!(msg->flags & I2C_M_NOSTART)) {
/* write the address */
@@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
/* no data and last message -> add STOP */
data |= XIIC_TX_DYN_STOP_MASK;
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
}
xiic_fill_tx_fifo(i2c);
@@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
int err;
dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
err = xiic_busy(i2c);
if (err)
--
1.5.5.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-22 17:49 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-22 17:49 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
john.williams-g5w7nrANp4BDPfheJLI6IQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
John.Linn-gjFFaj9aHVfQT0dZR+AlfA, Michal Simek
i2c driver is used for LE/BE that's why is useful to use
32bit accesses. Then it is not necessary to solve any
endian issues.
Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
1 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 0b7647b..78b3b82 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -178,21 +178,6 @@ struct xiic_i2c {
static void xiic_start_xfer(struct xiic_i2c *i2c);
static void __xiic_start_xfer(struct xiic_i2c *i2c);
-static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
-{
- iowrite8(value, i2c->base + reg);
-}
-
-static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
-{
- return ioread8(i2c->base + reg);
-}
-
-static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
-{
- iowrite16(value, i2c->base + reg);
-}
-
static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
{
iowrite32(value, i2c->base + reg);
@@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
{
u8 sr;
- for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
+ for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
!(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
- sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
+ sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
+ }
}
static void xiic_reinit(struct xiic_i2c *i2c)
@@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
/* Set receive Fifo depth to maximum (zero based). */
- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
/* Reset Tx Fifo. */
- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
/* Enable IIC Device, remove Tx Fifo reset & disable general call. */
- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
/* make sure RX fifo is empty */
xiic_clear_rx_fifo(i2c);
@@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
/* Disable IIC Device. */
- cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
+ cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
+ cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
}
static void xiic_read_rx(struct xiic_i2c *i2c)
@@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
u8 bytes_in_fifo;
int i;
- bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
+ bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
", SR: 0x%x, CR: 0x%x\n",
__func__, bytes_in_fifo, xiic_rx_space(i2c),
- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
if (bytes_in_fifo > xiic_rx_space(i2c))
bytes_in_fifo = xiic_rx_space(i2c);
for (i = 0; i < bytes_in_fifo; i++)
i2c->rx_msg->buf[i2c->rx_pos++] =
- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
(xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
}
@@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
{
/* return the actual space left in the FIFO */
- return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
+ return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
}
static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
@@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
data |= XIIC_TX_DYN_STOP_MASK;
dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
} else
- xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
}
}
@@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
"pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
- __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
+ __func__, ier, isr, pend,
+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
i2c->tx_msg, i2c->nmsgs);
/* Do not processes a devices interrupts if the device has no
@@ -479,7 +467,7 @@ out:
static int xiic_bus_busy(struct xiic_i2c *i2c)
{
- u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
+ u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
}
@@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
rx_watermark = msg->len;
if (rx_watermark > IIC_RX_FIFO_DEPTH)
rx_watermark = IIC_RX_FIFO_DEPTH;
- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
if (!(msg->flags & I2C_M_NOSTART))
/* write the address */
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
(msg->addr << 1) | XIIC_READ_OPERATION |
XIIC_TX_DYN_START_MASK);
xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
if (i2c->nmsgs == 1)
/* very last, enable bus not busy as well */
@@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
"ISR: 0x%x, CR: 0x%x\n",
__func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
if (!(msg->flags & I2C_M_NOSTART)) {
/* write the address */
@@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
/* no data and last message -> add STOP */
data |= XIIC_TX_DYN_STOP_MASK;
- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
}
xiic_fill_tx_fifo(i2c);
@@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
int err;
dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
err = xiic_busy(i2c);
if (err)
--
1.5.5.6
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-22 18:08 ` Grant Likely
0 siblings, 0 replies; 17+ messages in thread
From: Grant Likely @ 2011-02-22 18:08 UTC (permalink / raw)
To: Michal Simek
Cc: linux-i2c, devicetree-discuss, john.williams, linux-kernel, John.Linn
On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr@monstr.eu> wrote:
> i2c driver is used for LE/BE that's why is useful to use
> 32bit accesses. Then it is not necessary to solve any
> endian issues.
Are you sure? I would expect the BE version needs to use
io{read,write}32be variants of the accessors. What platforms have you
tested on?
>
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> ---
> drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
> 1 files changed, 27 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index 0b7647b..78b3b82 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -178,21 +178,6 @@ struct xiic_i2c {
> static void xiic_start_xfer(struct xiic_i2c *i2c);
> static void __xiic_start_xfer(struct xiic_i2c *i2c);
>
> -static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
> -{
> - iowrite8(value, i2c->base + reg);
> -}
> -
> -static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
> -{
> - return ioread8(i2c->base + reg);
> -}
> -
> -static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
> -{
> - iowrite16(value, i2c->base + reg);
> -}
> -
> static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
> {
> iowrite32(value, i2c->base + reg);
> @@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
> static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
> {
> u8 sr;
> - for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> + for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
> !(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
> - sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> + sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
> + }
> }
>
> static void xiic_reinit(struct xiic_i2c *i2c)
> @@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>
> /* Set receive Fifo depth to maximum (zero based). */
> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
>
> /* Reset Tx Fifo. */
> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
>
> /* Enable IIC Device, remove Tx Fifo reset & disable general call. */
> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
>
> /* make sure RX fifo is empty */
> xiic_clear_rx_fifo(i2c);
> @@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>
> /* Disable IIC Device. */
> - cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> + cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
> + cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> }
>
> static void xiic_read_rx(struct xiic_i2c *i2c)
> @@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> u8 bytes_in_fifo;
> int i;
>
> - bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
> + bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
>
> dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
> ", SR: 0x%x, CR: 0x%x\n",
> __func__, bytes_in_fifo, xiic_rx_space(i2c),
> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>
> if (bytes_in_fifo > xiic_rx_space(i2c))
> bytes_in_fifo = xiic_rx_space(i2c);
>
> for (i = 0; i < bytes_in_fifo; i++)
> i2c->rx_msg->buf[i2c->rx_pos++] =
> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
>
> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
> (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
> IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
> }
> @@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
> {
> /* return the actual space left in the FIFO */
> - return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
> + return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
> }
>
> static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> @@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> data |= XIIC_TX_DYN_STOP_MASK;
> dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
>
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> } else
> - xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> }
> }
>
> @@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
>
> dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
> "pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
> - __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> + __func__, ier, isr, pend,
> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> i2c->tx_msg, i2c->nmsgs);
>
> /* Do not processes a devices interrupts if the device has no
> @@ -479,7 +467,7 @@ out:
>
> static int xiic_bus_busy(struct xiic_i2c *i2c)
> {
> - u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> + u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
>
> return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
> }
> @@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
> rx_watermark = msg->len;
> if (rx_watermark > IIC_RX_FIFO_DEPTH)
> rx_watermark = IIC_RX_FIFO_DEPTH;
> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
>
> if (!(msg->flags & I2C_M_NOSTART))
> /* write the address */
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> (msg->addr << 1) | XIIC_READ_OPERATION |
> XIIC_TX_DYN_START_MASK);
>
> xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
>
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
> if (i2c->nmsgs == 1)
> /* very last, enable bus not busy as well */
> @@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
> "ISR: 0x%x, CR: 0x%x\n",
> __func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>
> if (!(msg->flags & I2C_M_NOSTART)) {
> /* write the address */
> @@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> /* no data and last message -> add STOP */
> data |= XIIC_TX_DYN_STOP_MASK;
>
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> }
>
> xiic_fill_tx_fifo(i2c);
> @@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> int err;
>
> dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
>
> err = xiic_busy(i2c);
> if (err)
> --
> 1.5.5.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-22 18:08 ` Grant Likely
0 siblings, 0 replies; 17+ messages in thread
From: Grant Likely @ 2011-02-22 18:08 UTC (permalink / raw)
To: Michal Simek
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
john.williams-g5w7nrANp4BDPfheJLI6IQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
John.Linn-gjFFaj9aHVfQT0dZR+AlfA
On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote:
> i2c driver is used for LE/BE that's why is useful to use
> 32bit accesses. Then it is not necessary to solve any
> endian issues.
Are you sure? I would expect the BE version needs to use
io{read,write}32be variants of the accessors. What platforms have you
tested on?
>
> Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
> ---
> drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
> 1 files changed, 27 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index 0b7647b..78b3b82 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -178,21 +178,6 @@ struct xiic_i2c {
> static void xiic_start_xfer(struct xiic_i2c *i2c);
> static void __xiic_start_xfer(struct xiic_i2c *i2c);
>
> -static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
> -{
> - iowrite8(value, i2c->base + reg);
> -}
> -
> -static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
> -{
> - return ioread8(i2c->base + reg);
> -}
> -
> -static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
> -{
> - iowrite16(value, i2c->base + reg);
> -}
> -
> static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
> {
> iowrite32(value, i2c->base + reg);
> @@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
> static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
> {
> u8 sr;
> - for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> + for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
> !(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
> - sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> + sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
> + }
> }
>
> static void xiic_reinit(struct xiic_i2c *i2c)
> @@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>
> /* Set receive Fifo depth to maximum (zero based). */
> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
>
> /* Reset Tx Fifo. */
> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
>
> /* Enable IIC Device, remove Tx Fifo reset & disable general call. */
> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
>
> /* make sure RX fifo is empty */
> xiic_clear_rx_fifo(i2c);
> @@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>
> /* Disable IIC Device. */
> - cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> + cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
> + cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> }
>
> static void xiic_read_rx(struct xiic_i2c *i2c)
> @@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> u8 bytes_in_fifo;
> int i;
>
> - bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
> + bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
>
> dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
> ", SR: 0x%x, CR: 0x%x\n",
> __func__, bytes_in_fifo, xiic_rx_space(i2c),
> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>
> if (bytes_in_fifo > xiic_rx_space(i2c))
> bytes_in_fifo = xiic_rx_space(i2c);
>
> for (i = 0; i < bytes_in_fifo; i++)
> i2c->rx_msg->buf[i2c->rx_pos++] =
> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
>
> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
> (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
> IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
> }
> @@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
> {
> /* return the actual space left in the FIFO */
> - return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
> + return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
> }
>
> static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> @@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> data |= XIIC_TX_DYN_STOP_MASK;
> dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
>
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> } else
> - xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> }
> }
>
> @@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
>
> dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
> "pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
> - __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> + __func__, ier, isr, pend,
> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> i2c->tx_msg, i2c->nmsgs);
>
> /* Do not processes a devices interrupts if the device has no
> @@ -479,7 +467,7 @@ out:
>
> static int xiic_bus_busy(struct xiic_i2c *i2c)
> {
> - u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> + u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
>
> return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
> }
> @@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
> rx_watermark = msg->len;
> if (rx_watermark > IIC_RX_FIFO_DEPTH)
> rx_watermark = IIC_RX_FIFO_DEPTH;
> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
>
> if (!(msg->flags & I2C_M_NOSTART))
> /* write the address */
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> (msg->addr << 1) | XIIC_READ_OPERATION |
> XIIC_TX_DYN_START_MASK);
>
> xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
>
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
> if (i2c->nmsgs == 1)
> /* very last, enable bus not busy as well */
> @@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
> "ISR: 0x%x, CR: 0x%x\n",
> __func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>
> if (!(msg->flags & I2C_M_NOSTART)) {
> /* write the address */
> @@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> /* no data and last message -> add STOP */
> data |= XIIC_TX_DYN_STOP_MASK;
>
> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> }
>
> xiic_fill_tx_fifo(i2c);
> @@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> int err;
>
> dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
>
> err = xiic_busy(i2c);
> if (err)
> --
> 1.5.5.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface
@ 2011-02-22 18:11 ` Grant Likely
0 siblings, 0 replies; 17+ messages in thread
From: Grant Likely @ 2011-02-22 18:11 UTC (permalink / raw)
To: Michal Simek
Cc: linux-i2c, devicetree-discuss, john.williams, linux-kernel,
John.Linn, Ben Dooks
[cc'ing Ben Dooks, embedded i2c maintainer]
On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr@monstr.eu> wrote:
> Add OF support to Xilinx i2c bus interface.
> It is used on Microblaze systems.
>
> Signed-off-by: Michal Simek <monstr@monstr.eu>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Ben, since this one depends on a patch in my tree, do you want me to pick it up?
g.
> ---
> drivers/i2c/busses/i2c-xiic.c | 37 +++++++++++++++++++++++++++++--------
> 1 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index a9c419e..0b7647b 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -41,6 +41,11 @@
> #include <linux/io.h>
> #include <linux/slab.h>
>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_i2c.h>
> +#include <linux/of_address.h>
Nit: This should not be separate from the rest of the include block.
The blank line between slab.h and of.h can be dropped.
> +
> #define DRIVER_NAME "xiic-i2c"
>
> enum xilinx_i2c_state {
> @@ -426,7 +431,7 @@ static void xiic_process(struct xiic_i2c *i2c)
> xiic_wakeup(i2c, STATE_ERROR);
>
> } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) {
> - /* Transmit register/FIFO is empty or ½ empty */
> + /* Transmit register/FIFO is empty or 1/2 empty */
>
> clr = pend &
> (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK);
> @@ -704,10 +709,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
> if (irq < 0)
> goto resource_missing;
>
> - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
> - if (!pdata)
> - return -EINVAL;
> -
> i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
> if (!i2c)
> return -ENOMEM;
> @@ -730,6 +731,7 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
> i2c->adap = xiic_adapter;
> i2c_set_adapdata(&i2c->adap, i2c);
> i2c->adap.dev.parent = &pdev->dev;
> + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node);
>
> xiic_reinit(i2c);
>
> @@ -748,9 +750,16 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
> goto add_adapter_failed;
> }
>
> - /* add in known devices to the bus */
> - for (i = 0; i < pdata->num_devices; i++)
> - i2c_new_device(&i2c->adap, pdata->devices + i);
> + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n",
> + (unsigned int)i2c->base, irq);
> +
> + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
While you're touching this code anyway, the cast is unnecessary.
> + if (pdata) {
> + /* add in known devices to the bus */
> + for (i = 0; i < pdata->num_devices; i++)
> + i2c_new_device(&i2c->adap, pdata->devices + i);
> + }
> + of_i2c_register_devices(&i2c->adap);
>
> return 0;
>
> @@ -799,12 +808,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev)
> /* work with hotplug and coldplug */
> MODULE_ALIAS("platform:"DRIVER_NAME);
>
> +#ifdef CONFIG_OF
> +/* Match table for of_platform binding */
> +static struct of_device_id __devinitdata xilinx_iic_of_match[] = {
> + { .compatible = "xlnx,xps-iic-2.00.a", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match);
> +#else
> +# define xilinx_iic_of_match NULL
> +#endif
> +
> static struct platform_driver xiic_i2c_driver = {
> .probe = xiic_i2c_probe,
> .remove = __devexit_p(xiic_i2c_remove),
> .driver = {
> .owner = THIS_MODULE,
> .name = DRIVER_NAME,
> + .of_match_table = xilinx_iic_of_match,
> },
> };
>
> --
> 1.5.5.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface
@ 2011-02-22 18:11 ` Grant Likely
0 siblings, 0 replies; 17+ messages in thread
From: Grant Likely @ 2011-02-22 18:11 UTC (permalink / raw)
To: Michal Simek
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Ben Dooks,
john.williams-g5w7nrANp4BDPfheJLI6IQ
[cc'ing Ben Dooks, embedded i2c maintainer]
On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote:
> Add OF support to Xilinx i2c bus interface.
> It is used on Microblaze systems.
>
> Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Ben, since this one depends on a patch in my tree, do you want me to pick it up?
g.
> ---
> drivers/i2c/busses/i2c-xiic.c | 37 +++++++++++++++++++++++++++++--------
> 1 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index a9c419e..0b7647b 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -41,6 +41,11 @@
> #include <linux/io.h>
> #include <linux/slab.h>
>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_i2c.h>
> +#include <linux/of_address.h>
Nit: This should not be separate from the rest of the include block.
The blank line between slab.h and of.h can be dropped.
> +
> #define DRIVER_NAME "xiic-i2c"
>
> enum xilinx_i2c_state {
> @@ -426,7 +431,7 @@ static void xiic_process(struct xiic_i2c *i2c)
> xiic_wakeup(i2c, STATE_ERROR);
>
> } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) {
> - /* Transmit register/FIFO is empty or ½ empty */
> + /* Transmit register/FIFO is empty or 1/2 empty */
>
> clr = pend &
> (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK);
> @@ -704,10 +709,6 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
> if (irq < 0)
> goto resource_missing;
>
> - pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
> - if (!pdata)
> - return -EINVAL;
> -
> i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
> if (!i2c)
> return -ENOMEM;
> @@ -730,6 +731,7 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
> i2c->adap = xiic_adapter;
> i2c_set_adapdata(&i2c->adap, i2c);
> i2c->adap.dev.parent = &pdev->dev;
> + i2c->adap.dev.of_node = of_node_get(pdev->dev.of_node);
>
> xiic_reinit(i2c);
>
> @@ -748,9 +750,16 @@ static int __devinit xiic_i2c_probe(struct platform_device *pdev)
> goto add_adapter_failed;
> }
>
> - /* add in known devices to the bus */
> - for (i = 0; i < pdata->num_devices; i++)
> - i2c_new_device(&i2c->adap, pdata->devices + i);
> + dev_info(&pdev->dev, "mapped to 0x%08X, irq=%d\n",
> + (unsigned int)i2c->base, irq);
> +
> + pdata = (struct xiic_i2c_platform_data *) pdev->dev.platform_data;
While you're touching this code anyway, the cast is unnecessary.
> + if (pdata) {
> + /* add in known devices to the bus */
> + for (i = 0; i < pdata->num_devices; i++)
> + i2c_new_device(&i2c->adap, pdata->devices + i);
> + }
> + of_i2c_register_devices(&i2c->adap);
>
> return 0;
>
> @@ -799,12 +808,24 @@ static int __devexit xiic_i2c_remove(struct platform_device* pdev)
> /* work with hotplug and coldplug */
> MODULE_ALIAS("platform:"DRIVER_NAME);
>
> +#ifdef CONFIG_OF
> +/* Match table for of_platform binding */
> +static struct of_device_id __devinitdata xilinx_iic_of_match[] = {
> + { .compatible = "xlnx,xps-iic-2.00.a", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, xilinx_iic_of_match);
> +#else
> +# define xilinx_iic_of_match NULL
> +#endif
> +
> static struct platform_driver xiic_i2c_driver = {
> .probe = xiic_i2c_probe,
> .remove = __devexit_p(xiic_i2c_remove),
> .driver = {
> .owner = THIS_MODULE,
> .name = DRIVER_NAME,
> + .of_match_table = xilinx_iic_of_match,
> },
> };
>
> --
> 1.5.5.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 8:08 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-23 8:08 UTC (permalink / raw)
To: Grant Likely
Cc: linux-i2c, devicetree-discuss, john.williams, linux-kernel, John.Linn
Grant Likely wrote:
> On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr@monstr.eu> wrote:
>> i2c driver is used for LE/BE that's why is useful to use
>> 32bit accesses. Then it is not necessary to solve any
>> endian issues.
>
> Are you sure? I would expect the BE version needs to use
> io{read,write}32be variants of the accessors. What platforms have you
> tested on?
iowrite32 is the same with iowrite32be for Microblaze.
I have no problem to change it to iowrite32be if you like.
I have tested it on microblaze big and little endian platforms.
Thanks,
Michal
>
>> Signed-off-by: Michal Simek <monstr@monstr.eu>
>> ---
>> drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
>> 1 files changed, 27 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
>> index 0b7647b..78b3b82 100644
>> --- a/drivers/i2c/busses/i2c-xiic.c
>> +++ b/drivers/i2c/busses/i2c-xiic.c
>> @@ -178,21 +178,6 @@ struct xiic_i2c {
>> static void xiic_start_xfer(struct xiic_i2c *i2c);
>> static void __xiic_start_xfer(struct xiic_i2c *i2c);
>>
>> -static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
>> -{
>> - iowrite8(value, i2c->base + reg);
>> -}
>> -
>> -static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
>> -{
>> - return ioread8(i2c->base + reg);
>> -}
>> -
>> -static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
>> -{
>> - iowrite16(value, i2c->base + reg);
>> -}
>> -
>> static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
>> {
>> iowrite32(value, i2c->base + reg);
>> @@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
>> static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
>> {
>> u8 sr;
>> - for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
>> + for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
>> !(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
>> - sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
>> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
>> + sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
>> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
>> + }
>> }
>>
>> static void xiic_reinit(struct xiic_i2c *i2c)
>> @@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
>> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>>
>> /* Set receive Fifo depth to maximum (zero based). */
>> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
>> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
>>
>> /* Reset Tx Fifo. */
>> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
>>
>> /* Enable IIC Device, remove Tx Fifo reset & disable general call. */
>> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
>>
>> /* make sure RX fifo is empty */
>> xiic_clear_rx_fifo(i2c);
>> @@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
>> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>>
>> /* Disable IIC Device. */
>> - cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
>> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
>> + cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
>> + cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
>> }
>>
>> static void xiic_read_rx(struct xiic_i2c *i2c)
>> @@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
>> u8 bytes_in_fifo;
>> int i;
>>
>> - bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
>> + bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
>>
>> dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
>> ", SR: 0x%x, CR: 0x%x\n",
>> __func__, bytes_in_fifo, xiic_rx_space(i2c),
>> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
>> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
>> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
>> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>>
>> if (bytes_in_fifo > xiic_rx_space(i2c))
>> bytes_in_fifo = xiic_rx_space(i2c);
>>
>> for (i = 0; i < bytes_in_fifo; i++)
>> i2c->rx_msg->buf[i2c->rx_pos++] =
>> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
>> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
>>
>> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
>> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
>> (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
>> IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
>> }
>> @@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
>> static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
>> {
>> /* return the actual space left in the FIFO */
>> - return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
>> + return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
>> }
>>
>> static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
>> @@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
>> data |= XIIC_TX_DYN_STOP_MASK;
>> dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
>>
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
>> } else
>> - xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
>> }
>> }
>>
>> @@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
>>
>> dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
>> "pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
>> - __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
>> + __func__, ier, isr, pend,
>> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
>> i2c->tx_msg, i2c->nmsgs);
>>
>> /* Do not processes a devices interrupts if the device has no
>> @@ -479,7 +467,7 @@ out:
>>
>> static int xiic_bus_busy(struct xiic_i2c *i2c)
>> {
>> - u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
>> + u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
>>
>> return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
>> }
>> @@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
>> rx_watermark = msg->len;
>> if (rx_watermark > IIC_RX_FIFO_DEPTH)
>> rx_watermark = IIC_RX_FIFO_DEPTH;
>> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
>> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
>>
>> if (!(msg->flags & I2C_M_NOSTART))
>> /* write the address */
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
>> (msg->addr << 1) | XIIC_READ_OPERATION |
>> XIIC_TX_DYN_START_MASK);
>>
>> xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
>>
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
>> msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
>> if (i2c->nmsgs == 1)
>> /* very last, enable bus not busy as well */
>> @@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
>> dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
>> "ISR: 0x%x, CR: 0x%x\n",
>> __func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
>> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
>> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>>
>> if (!(msg->flags & I2C_M_NOSTART)) {
>> /* write the address */
>> @@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
>> /* no data and last message -> add STOP */
>> data |= XIIC_TX_DYN_STOP_MASK;
>>
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
>> }
>>
>> xiic_fill_tx_fifo(i2c);
>> @@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>> int err;
>>
>> dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
>> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
>> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
>>
>> err = xiic_busy(i2c);
>> if (err)
>> --
>> 1.5.5.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
>
>
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 8:08 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-23 8:08 UTC (permalink / raw)
To: Grant Likely
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
john.williams-g5w7nrANp4BDPfheJLI6IQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
John.Linn-gjFFaj9aHVfQT0dZR+AlfA
Grant Likely wrote:
> On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote:
>> i2c driver is used for LE/BE that's why is useful to use
>> 32bit accesses. Then it is not necessary to solve any
>> endian issues.
>
> Are you sure? I would expect the BE version needs to use
> io{read,write}32be variants of the accessors. What platforms have you
> tested on?
iowrite32 is the same with iowrite32be for Microblaze.
I have no problem to change it to iowrite32be if you like.
I have tested it on microblaze big and little endian platforms.
Thanks,
Michal
>
>> Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
>> ---
>> drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
>> 1 files changed, 27 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
>> index 0b7647b..78b3b82 100644
>> --- a/drivers/i2c/busses/i2c-xiic.c
>> +++ b/drivers/i2c/busses/i2c-xiic.c
>> @@ -178,21 +178,6 @@ struct xiic_i2c {
>> static void xiic_start_xfer(struct xiic_i2c *i2c);
>> static void __xiic_start_xfer(struct xiic_i2c *i2c);
>>
>> -static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
>> -{
>> - iowrite8(value, i2c->base + reg);
>> -}
>> -
>> -static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
>> -{
>> - return ioread8(i2c->base + reg);
>> -}
>> -
>> -static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
>> -{
>> - iowrite16(value, i2c->base + reg);
>> -}
>> -
>> static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
>> {
>> iowrite32(value, i2c->base + reg);
>> @@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
>> static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
>> {
>> u8 sr;
>> - for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
>> + for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
>> !(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
>> - sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
>> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
>> + sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
>> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
>> + }
>> }
>>
>> static void xiic_reinit(struct xiic_i2c *i2c)
>> @@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
>> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>>
>> /* Set receive Fifo depth to maximum (zero based). */
>> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
>> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
>>
>> /* Reset Tx Fifo. */
>> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
>>
>> /* Enable IIC Device, remove Tx Fifo reset & disable general call. */
>> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
>>
>> /* make sure RX fifo is empty */
>> xiic_clear_rx_fifo(i2c);
>> @@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
>> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
>>
>> /* Disable IIC Device. */
>> - cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
>> - xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
>> + cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
>> + xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
>> + cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
>> }
>>
>> static void xiic_read_rx(struct xiic_i2c *i2c)
>> @@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
>> u8 bytes_in_fifo;
>> int i;
>>
>> - bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
>> + bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
>>
>> dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
>> ", SR: 0x%x, CR: 0x%x\n",
>> __func__, bytes_in_fifo, xiic_rx_space(i2c),
>> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
>> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
>> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
>> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>>
>> if (bytes_in_fifo > xiic_rx_space(i2c))
>> bytes_in_fifo = xiic_rx_space(i2c);
>>
>> for (i = 0; i < bytes_in_fifo; i++)
>> i2c->rx_msg->buf[i2c->rx_pos++] =
>> - xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
>> + xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
>>
>> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
>> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
>> (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
>> IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
>> }
>> @@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
>> static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
>> {
>> /* return the actual space left in the FIFO */
>> - return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
>> + return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
>> }
>>
>> static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
>> @@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
>> data |= XIIC_TX_DYN_STOP_MASK;
>> dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
>>
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
>> } else
>> - xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
>> }
>> }
>>
>> @@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
>>
>> dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
>> "pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
>> - __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
>> + __func__, ier, isr, pend,
>> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
>> i2c->tx_msg, i2c->nmsgs);
>>
>> /* Do not processes a devices interrupts if the device has no
>> @@ -479,7 +467,7 @@ out:
>>
>> static int xiic_bus_busy(struct xiic_i2c *i2c)
>> {
>> - u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
>> + u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
>>
>> return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
>> }
>> @@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
>> rx_watermark = msg->len;
>> if (rx_watermark > IIC_RX_FIFO_DEPTH)
>> rx_watermark = IIC_RX_FIFO_DEPTH;
>> - xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
>> + xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
>>
>> if (!(msg->flags & I2C_M_NOSTART))
>> /* write the address */
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
>> (msg->addr << 1) | XIIC_READ_OPERATION |
>> XIIC_TX_DYN_START_MASK);
>>
>> xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
>>
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
>> msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
>> if (i2c->nmsgs == 1)
>> /* very last, enable bus not busy as well */
>> @@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
>> dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
>> "ISR: 0x%x, CR: 0x%x\n",
>> __func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
>> - xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
>> + xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
>>
>> if (!(msg->flags & I2C_M_NOSTART)) {
>> /* write the address */
>> @@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
>> /* no data and last message -> add STOP */
>> data |= XIIC_TX_DYN_STOP_MASK;
>>
>> - xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
>> + xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
>> }
>>
>> xiic_fill_tx_fifo(i2c);
>> @@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>> int err;
>>
>> dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
>> - xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
>> + xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
>>
>> err = xiic_busy(i2c);
>> if (err)
>> --
>> 1.5.5.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
>
>
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
2011-02-23 8:08 ` Michal Simek
(?)
@ 2011-02-23 8:36 ` Arnd Bergmann
2011-02-23 9:37 ` Michal Simek
-1 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2011-02-23 8:36 UTC (permalink / raw)
To: devicetree-discuss, monstr
Cc: Grant Likely, linux-i2c, john.williams, linux-kernel
On Wednesday 23 February 2011 09:08:47 Michal Simek wrote:
>
> Grant Likely wrote:
> > On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr@monstr.eu> wrote:
> >> i2c driver is used for LE/BE that's why is useful to use
> >> 32bit accesses. Then it is not necessary to solve any
> >> endian issues.
> >
> > Are you sure? I would expect the BE version needs to use
> > io{read,write}32be variants of the accessors. What platforms have you
> > tested on?
>
> iowrite32 is the same with iowrite32be for Microblaze.
> I have no problem to change it to iowrite32be if you like.
>
> I have tested it on microblaze big and little endian platforms.
I think what Grant was saying is that iowrite32 being the same as
iowrite32be is a bug, because iowrite32 is documented to be little-endian.
This is probably fine as long as you don't have any PCI devices,
but if you ever get PCI support, it won't work.
Also, it heavily confuses other developers such as Grant and me
if one architecture defines things to mean something completely
different from the other architectures.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 9:37 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-23 9:37 UTC (permalink / raw)
To: Arnd Bergmann
Cc: devicetree-discuss, Grant Likely, linux-i2c, john.williams, linux-kernel
Arnd Bergmann wrote:
> On Wednesday 23 February 2011 09:08:47 Michal Simek wrote:
>> Grant Likely wrote:
>>> On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>> i2c driver is used for LE/BE that's why is useful to use
>>>> 32bit accesses. Then it is not necessary to solve any
>>>> endian issues.
>>> Are you sure? I would expect the BE version needs to use
>>> io{read,write}32be variants of the accessors. What platforms have you
>>> tested on?
>> iowrite32 is the same with iowrite32be for Microblaze.
>> I have no problem to change it to iowrite32be if you like.
>>
>> I have tested it on microblaze big and little endian platforms.
>
> I think what Grant was saying is that iowrite32 being the same as
> iowrite32be is a bug, because iowrite32 is documented to be little-endian.
Can you pointed me to that documentation?
>
> This is probably fine as long as you don't have any PCI devices,
> but if you ever get PCI support, it won't work.
Microblaze have PCI support but I don't have the board for testing.
>
> Also, it heavily confuses other developers such as Grant and me
> if one architecture defines things to mean something completely
> different from the other architectures.
As I see will be good to review microblaze io.h and use generic one.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 9:37 ` Michal Simek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Simek @ 2011-02-23 9:37 UTC (permalink / raw)
To: Arnd Bergmann
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Grant Likely,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
john.williams-g5w7nrANp4BDPfheJLI6IQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Arnd Bergmann wrote:
> On Wednesday 23 February 2011 09:08:47 Michal Simek wrote:
>> Grant Likely wrote:
>>> On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote:
>>>> i2c driver is used for LE/BE that's why is useful to use
>>>> 32bit accesses. Then it is not necessary to solve any
>>>> endian issues.
>>> Are you sure? I would expect the BE version needs to use
>>> io{read,write}32be variants of the accessors. What platforms have you
>>> tested on?
>> iowrite32 is the same with iowrite32be for Microblaze.
>> I have no problem to change it to iowrite32be if you like.
>>
>> I have tested it on microblaze big and little endian platforms.
>
> I think what Grant was saying is that iowrite32 being the same as
> iowrite32be is a bug, because iowrite32 is documented to be little-endian.
Can you pointed me to that documentation?
>
> This is probably fine as long as you don't have any PCI devices,
> but if you ever get PCI support, it won't work.
Microblaze have PCI support but I don't have the board for testing.
>
> Also, it heavily confuses other developers such as Grant and me
> if one architecture defines things to mean something completely
> different from the other architectures.
As I see will be good to review microblaze io.h and use generic one.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 16:56 ` Grant Likely
0 siblings, 0 replies; 17+ messages in thread
From: Grant Likely @ 2011-02-23 16:56 UTC (permalink / raw)
To: Michal Simek
Cc: linux-i2c, devicetree-discuss, john.williams, linux-kernel, John.Linn
On Wed, Feb 23, 2011 at 09:08:47AM +0100, Michal Simek wrote:
> Grant Likely wrote:
> >On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr@monstr.eu> wrote:
> >>i2c driver is used for LE/BE that's why is useful to use
> >>32bit accesses. Then it is not necessary to solve any
> >>endian issues.
> >
> >Are you sure? I would expect the BE version needs to use
> >io{read,write}32be variants of the accessors. What platforms have you
> >tested on?
>
> iowrite32 is the same with iowrite32be for Microblaze.
> I have no problem to change it to iowrite32be if you like.
>
> I have tested it on microblaze big and little endian platforms.
But this driver was written for x86, and it should be usable on
powerpc too. You'll need to account for both possibilities.
g.
>
> Thanks,
> Michal
>
>
> >
> >>Signed-off-by: Michal Simek <monstr@monstr.eu>
> >>---
> >> drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
> >> 1 files changed, 27 insertions(+), 39 deletions(-)
> >>
> >>diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> >>index 0b7647b..78b3b82 100644
> >>--- a/drivers/i2c/busses/i2c-xiic.c
> >>+++ b/drivers/i2c/busses/i2c-xiic.c
> >>@@ -178,21 +178,6 @@ struct xiic_i2c {
> >> static void xiic_start_xfer(struct xiic_i2c *i2c);
> >> static void __xiic_start_xfer(struct xiic_i2c *i2c);
> >>
> >>-static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
> >>-{
> >>- iowrite8(value, i2c->base + reg);
> >>-}
> >>-
> >>-static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
> >>-{
> >>- return ioread8(i2c->base + reg);
> >>-}
> >>-
> >>-static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
> >>-{
> >>- iowrite16(value, i2c->base + reg);
> >>-}
> >>-
> >> static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
> >> {
> >> iowrite32(value, i2c->base + reg);
> >>@@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
> >> static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
> >> {
> >> u8 sr;
> >>- for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> >>+ for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
> >> !(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
> >>- sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
> >>- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> >>+ sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
> >>+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
> >>+ }
> >> }
> >>
> >> static void xiic_reinit(struct xiic_i2c *i2c)
> >>@@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
> >> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
> >>
> >> /* Set receive Fifo depth to maximum (zero based). */
> >>- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
> >>+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
> >>
> >> /* Reset Tx Fifo. */
> >>- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
> >>+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
> >>
> >> /* Enable IIC Device, remove Tx Fifo reset & disable general call. */
> >>- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
> >>+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
> >>
> >> /* make sure RX fifo is empty */
> >> xiic_clear_rx_fifo(i2c);
> >>@@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
> >> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
> >>
> >> /* Disable IIC Device. */
> >>- cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
> >>- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> >>+ cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
> >>+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
> >>+ cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> >> }
> >>
> >> static void xiic_read_rx(struct xiic_i2c *i2c)
> >>@@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> >> u8 bytes_in_fifo;
> >> int i;
> >>
> >>- bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
> >>+ bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
> >>
> >> dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
> >> ", SR: 0x%x, CR: 0x%x\n",
> >> __func__, bytes_in_fifo, xiic_rx_space(i2c),
> >>- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> >>- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> >>+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> >>+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
> >>
> >> if (bytes_in_fifo > xiic_rx_space(i2c))
> >> bytes_in_fifo = xiic_rx_space(i2c);
> >>
> >> for (i = 0; i < bytes_in_fifo; i++)
> >> i2c->rx_msg->buf[i2c->rx_pos++] =
> >>- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> >>+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
> >>
> >>- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
> >>+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
> >> (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
> >> IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
> >> }
> >>@@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> >> static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
> >> {
> >> /* return the actual space left in the FIFO */
> >>- return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
> >>+ return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
> >> }
> >>
> >> static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> >>@@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> >> data |= XIIC_TX_DYN_STOP_MASK;
> >> dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
> >>
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> >> } else
> >>- xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> >> }
> >> }
> >>
> >>@@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
> >>
> >> dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
> >> "pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
> >>- __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> >>+ __func__, ier, isr, pend,
> >>+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> >> i2c->tx_msg, i2c->nmsgs);
> >>
> >> /* Do not processes a devices interrupts if the device has no
> >>@@ -479,7 +467,7 @@ out:
> >>
> >> static int xiic_bus_busy(struct xiic_i2c *i2c)
> >> {
> >>- u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> >>+ u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
> >>
> >> return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
> >> }
> >>@@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
> >> rx_watermark = msg->len;
> >> if (rx_watermark > IIC_RX_FIFO_DEPTH)
> >> rx_watermark = IIC_RX_FIFO_DEPTH;
> >>- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
> >>+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
> >>
> >> if (!(msg->flags & I2C_M_NOSTART))
> >> /* write the address */
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> >> (msg->addr << 1) | XIIC_READ_OPERATION |
> >> XIIC_TX_DYN_START_MASK);
> >>
> >> xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
> >>
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> >> msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
> >> if (i2c->nmsgs == 1)
> >> /* very last, enable bus not busy as well */
> >>@@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> >> dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
> >> "ISR: 0x%x, CR: 0x%x\n",
> >> __func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
> >>- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> >>+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
> >>
> >> if (!(msg->flags & I2C_M_NOSTART)) {
> >> /* write the address */
> >>@@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> >> /* no data and last message -> add STOP */
> >> data |= XIIC_TX_DYN_STOP_MASK;
> >>
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> >> }
> >>
> >> xiic_fill_tx_fifo(i2c);
> >>@@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> >> int err;
> >>
> >> dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
> >>- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
> >>+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
> >>
> >> err = xiic_busy(i2c);
> >> if (err)
> >>--
> >>1.5.5.6
> >>
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>the body of a message to majordomo@vger.kernel.org
> >>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>Please read the FAQ at http://www.tux.org/lkml/
> >>
> >
> >
> >
>
>
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 16:56 ` Grant Likely
0 siblings, 0 replies; 17+ messages in thread
From: Grant Likely @ 2011-02-23 16:56 UTC (permalink / raw)
To: Michal Simek
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
john.williams-g5w7nrANp4BDPfheJLI6IQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Wed, Feb 23, 2011 at 09:08:47AM +0100, Michal Simek wrote:
> Grant Likely wrote:
> >On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote:
> >>i2c driver is used for LE/BE that's why is useful to use
> >>32bit accesses. Then it is not necessary to solve any
> >>endian issues.
> >
> >Are you sure? I would expect the BE version needs to use
> >io{read,write}32be variants of the accessors. What platforms have you
> >tested on?
>
> iowrite32 is the same with iowrite32be for Microblaze.
> I have no problem to change it to iowrite32be if you like.
>
> I have tested it on microblaze big and little endian platforms.
But this driver was written for x86, and it should be usable on
powerpc too. You'll need to account for both possibilities.
g.
>
> Thanks,
> Michal
>
>
> >
> >>Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
> >>---
> >> drivers/i2c/busses/i2c-xiic.c | 66 +++++++++++++++++------------------------
> >> 1 files changed, 27 insertions(+), 39 deletions(-)
> >>
> >>diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> >>index 0b7647b..78b3b82 100644
> >>--- a/drivers/i2c/busses/i2c-xiic.c
> >>+++ b/drivers/i2c/busses/i2c-xiic.c
> >>@@ -178,21 +178,6 @@ struct xiic_i2c {
> >> static void xiic_start_xfer(struct xiic_i2c *i2c);
> >> static void __xiic_start_xfer(struct xiic_i2c *i2c);
> >>
> >>-static inline void xiic_setreg8(struct xiic_i2c *i2c, int reg, u8 value)
> >>-{
> >>- iowrite8(value, i2c->base + reg);
> >>-}
> >>-
> >>-static inline u8 xiic_getreg8(struct xiic_i2c *i2c, int reg)
> >>-{
> >>- return ioread8(i2c->base + reg);
> >>-}
> >>-
> >>-static inline void xiic_setreg16(struct xiic_i2c *i2c, int reg, u16 value)
> >>-{
> >>- iowrite16(value, i2c->base + reg);
> >>-}
> >>-
> >> static inline void xiic_setreg32(struct xiic_i2c *i2c, int reg, int value)
> >> {
> >> iowrite32(value, i2c->base + reg);
> >>@@ -230,10 +215,11 @@ static inline void xiic_irq_clr_en(struct xiic_i2c *i2c, u32 mask)
> >> static void xiic_clear_rx_fifo(struct xiic_i2c *i2c)
> >> {
> >> u8 sr;
> >>- for (sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> >>+ for (sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
> >> !(sr & XIIC_SR_RX_FIFO_EMPTY_MASK);
> >>- sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET))
> >>- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> >>+ sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET)) {
> >>+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
> >>+ }
> >> }
> >>
> >> static void xiic_reinit(struct xiic_i2c *i2c)
> >>@@ -241,13 +227,13 @@ static void xiic_reinit(struct xiic_i2c *i2c)
> >> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
> >>
> >> /* Set receive Fifo depth to maximum (zero based). */
> >>- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
> >>+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, IIC_RX_FIFO_DEPTH - 1);
> >>
> >> /* Reset Tx Fifo. */
> >>- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
> >>+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
> >>
> >> /* Enable IIC Device, remove Tx Fifo reset & disable general call. */
> >>- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
> >>+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_ENABLE_DEVICE_MASK);
> >>
> >> /* make sure RX fifo is empty */
> >> xiic_clear_rx_fifo(i2c);
> >>@@ -265,8 +251,9 @@ static void xiic_deinit(struct xiic_i2c *i2c)
> >> xiic_setreg32(i2c, XIIC_RESETR_OFFSET, XIIC_RESET_MASK);
> >>
> >> /* Disable IIC Device. */
> >>- cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
> >>- xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> >>+ cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
> >>+ xiic_setreg32(i2c, XIIC_CR_REG_OFFSET,
> >>+ cr & ~XIIC_CR_ENABLE_DEVICE_MASK);
> >> }
> >>
> >> static void xiic_read_rx(struct xiic_i2c *i2c)
> >>@@ -274,22 +261,22 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> >> u8 bytes_in_fifo;
> >> int i;
> >>
> >>- bytes_in_fifo = xiic_getreg8(i2c, XIIC_RFO_REG_OFFSET) + 1;
> >>+ bytes_in_fifo = xiic_getreg32(i2c, XIIC_RFO_REG_OFFSET) + 1;
> >>
> >> dev_dbg(i2c->adap.dev.parent, "%s entry, bytes in fifo: %d, msg: %d"
> >> ", SR: 0x%x, CR: 0x%x\n",
> >> __func__, bytes_in_fifo, xiic_rx_space(i2c),
> >>- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> >>- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> >>+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> >>+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
> >>
> >> if (bytes_in_fifo > xiic_rx_space(i2c))
> >> bytes_in_fifo = xiic_rx_space(i2c);
> >>
> >> for (i = 0; i < bytes_in_fifo; i++)
> >> i2c->rx_msg->buf[i2c->rx_pos++] =
> >>- xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
> >>+ xiic_getreg32(i2c, XIIC_DRR_REG_OFFSET);
> >>
> >>- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET,
> >>+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET,
> >> (xiic_rx_space(i2c) > IIC_RX_FIFO_DEPTH) ?
> >> IIC_RX_FIFO_DEPTH - 1 : xiic_rx_space(i2c) - 1);
> >> }
> >>@@ -297,7 +284,7 @@ static void xiic_read_rx(struct xiic_i2c *i2c)
> >> static int xiic_tx_fifo_space(struct xiic_i2c *i2c)
> >> {
> >> /* return the actual space left in the FIFO */
> >>- return IIC_TX_FIFO_DEPTH - xiic_getreg8(i2c, XIIC_TFO_REG_OFFSET) - 1;
> >>+ return IIC_TX_FIFO_DEPTH - xiic_getreg32(i2c, XIIC_TFO_REG_OFFSET) - 1;
> >> }
> >>
> >> static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> >>@@ -317,9 +304,9 @@ static void xiic_fill_tx_fifo(struct xiic_i2c *i2c)
> >> data |= XIIC_TX_DYN_STOP_MASK;
> >> dev_dbg(i2c->adap.dev.parent, "%s TX STOP\n", __func__);
> >>
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> >> } else
> >>- xiic_setreg8(i2c, XIIC_DTR_REG_OFFSET, data);
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> >> }
> >> }
> >>
> >>@@ -348,7 +335,8 @@ static void xiic_process(struct xiic_i2c *i2c)
> >>
> >> dev_dbg(i2c->adap.dev.parent, "%s entry, IER: 0x%x, ISR: 0x%x, "
> >> "pend: 0x%x, SR: 0x%x, msg: %p, nmsgs: %d\n",
> >>- __func__, ier, isr, pend, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
> >>+ __func__, ier, isr, pend,
> >>+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET),
> >> i2c->tx_msg, i2c->nmsgs);
> >>
> >> /* Do not processes a devices interrupts if the device has no
> >>@@ -479,7 +467,7 @@ out:
> >>
> >> static int xiic_bus_busy(struct xiic_i2c *i2c)
> >> {
> >>- u8 sr = xiic_getreg8(i2c, XIIC_SR_REG_OFFSET);
> >>+ u8 sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
> >>
> >> return (sr & XIIC_SR_BUS_BUSY_MASK) ? -EBUSY : 0;
> >> }
> >>@@ -522,17 +510,17 @@ static void xiic_start_recv(struct xiic_i2c *i2c)
> >> rx_watermark = msg->len;
> >> if (rx_watermark > IIC_RX_FIFO_DEPTH)
> >> rx_watermark = IIC_RX_FIFO_DEPTH;
> >>- xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
> >>+ xiic_setreg32(i2c, XIIC_RFD_REG_OFFSET, rx_watermark - 1);
> >>
> >> if (!(msg->flags & I2C_M_NOSTART))
> >> /* write the address */
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> >> (msg->addr << 1) | XIIC_READ_OPERATION |
> >> XIIC_TX_DYN_START_MASK);
> >>
> >> xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK);
> >>
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET,
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET,
> >> msg->len | ((i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0));
> >> if (i2c->nmsgs == 1)
> >> /* very last, enable bus not busy as well */
> >>@@ -551,7 +539,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> >> dev_dbg(i2c->adap.dev.parent, "%s entry, msg: %p, len: %d, "
> >> "ISR: 0x%x, CR: 0x%x\n",
> >> __func__, msg, msg->len, xiic_getreg32(i2c, XIIC_IISR_OFFSET),
> >>- xiic_getreg8(i2c, XIIC_CR_REG_OFFSET));
> >>+ xiic_getreg32(i2c, XIIC_CR_REG_OFFSET));
> >>
> >> if (!(msg->flags & I2C_M_NOSTART)) {
> >> /* write the address */
> >>@@ -561,7 +549,7 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> >> /* no data and last message -> add STOP */
> >> data |= XIIC_TX_DYN_STOP_MASK;
> >>
> >>- xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, data);
> >>+ xiic_setreg32(i2c, XIIC_DTR_REG_OFFSET, data);
> >> }
> >>
> >> xiic_fill_tx_fifo(i2c);
> >>@@ -653,7 +641,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> >> int err;
> >>
> >> dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
> >>- xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
> >>+ xiic_getreg32(i2c, XIIC_SR_REG_OFFSET));
> >>
> >> err = xiic_busy(i2c);
> >> if (err)
> >>--
> >>1.5.5.6
> >>
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>Please read the FAQ at http://www.tux.org/lkml/
> >>
> >
> >
> >
>
>
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 17:49 ` Arnd Bergmann
0 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2011-02-23 17:49 UTC (permalink / raw)
To: monstr
Cc: devicetree-discuss, Grant Likely, linux-i2c, john.williams,
linux-kernel, Guan Xuetao
On Wednesday 23 February 2011 10:37:31 Michal Simek wrote:
> Arnd Bergmann wrote:
> > On Wednesday 23 February 2011 09:08:47 Michal Simek wrote:
> >> Grant Likely wrote:
> >>> On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr@monstr.eu> wrote:
> >>>> i2c driver is used for LE/BE that's why is useful to use
> >>>> 32bit accesses. Then it is not necessary to solve any
> >>>> endian issues.
> >>> Are you sure? I would expect the BE version needs to use
> >>> io{read,write}32be variants of the accessors. What platforms have you
> >>> tested on?
> >> iowrite32 is the same with iowrite32be for Microblaze.
> >> I have no problem to change it to iowrite32be if you like.
> >>
> >> I have tested it on microblaze big and little endian platforms.
> >
> > I think what Grant was saying is that iowrite32 being the same as
> > iowrite32be is a bug, because iowrite32 is documented to be little-endian.
>
> Can you pointed me to that documentation?
Documented was maybe a term that is bit too strong in this case ;-)
Documentation/memory-barriers.txt documents that iowrite32 is the same
as either writel or outl, depending on the source of the mapping.
Both include/asm-generic/io.h and include/asm-generic/iomap.h define
variants of iowrite32 and iowrite32be, but the definition in io.h is
actually broken.
> > This is probably fine as long as you don't have any PCI devices,
> > but if you ever get PCI support, it won't work.
>
> Microblaze have PCI support but I don't have the board for testing.
Ok, I see.
> > Also, it heavily confuses other developers such as Grant and me
> > if one architecture defines things to mean something completely
> > different from the other architectures.
>
> As I see will be good to review microblaze io.h and use generic one.
The generic io.h is lacking in a number of ways and should really
be improved. Guan Xuetao also fell into the same trap with the
unicore32 port.
The problem is that I copied asm/io.h from mn10300, which seemed
generic enough, but it really doesn't work correctly with PCI.
There is one patch for this that I did in the unicore32 tree,
I should probably go through it and fix everything else that sticks
out.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only
@ 2011-02-23 17:49 ` Arnd Bergmann
0 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2011-02-23 17:49 UTC (permalink / raw)
To: monstr-pSz03upnqPeHXe+LvDLADg
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Grant Likely,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
john.williams-g5w7nrANp4BDPfheJLI6IQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Guan Xuetao
On Wednesday 23 February 2011 10:37:31 Michal Simek wrote:
> Arnd Bergmann wrote:
> > On Wednesday 23 February 2011 09:08:47 Michal Simek wrote:
> >> Grant Likely wrote:
> >>> On Tue, Feb 22, 2011 at 10:49 AM, Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org> wrote:
> >>>> i2c driver is used for LE/BE that's why is useful to use
> >>>> 32bit accesses. Then it is not necessary to solve any
> >>>> endian issues.
> >>> Are you sure? I would expect the BE version needs to use
> >>> io{read,write}32be variants of the accessors. What platforms have you
> >>> tested on?
> >> iowrite32 is the same with iowrite32be for Microblaze.
> >> I have no problem to change it to iowrite32be if you like.
> >>
> >> I have tested it on microblaze big and little endian platforms.
> >
> > I think what Grant was saying is that iowrite32 being the same as
> > iowrite32be is a bug, because iowrite32 is documented to be little-endian.
>
> Can you pointed me to that documentation?
Documented was maybe a term that is bit too strong in this case ;-)
Documentation/memory-barriers.txt documents that iowrite32 is the same
as either writel or outl, depending on the source of the mapping.
Both include/asm-generic/io.h and include/asm-generic/iomap.h define
variants of iowrite32 and iowrite32be, but the definition in io.h is
actually broken.
> > This is probably fine as long as you don't have any PCI devices,
> > but if you ever get PCI support, it won't work.
>
> Microblaze have PCI support but I don't have the board for testing.
Ok, I see.
> > Also, it heavily confuses other developers such as Grant and me
> > if one architecture defines things to mean something completely
> > different from the other architectures.
>
> As I see will be good to review microblaze io.h and use generic one.
The generic io.h is lacking in a number of ways and should really
be improved. Guan Xuetao also fell into the same trap with the
unicore32 port.
The problem is that I copied asm/io.h from mn10300, which seemed
generic enough, but it really doesn't work correctly with PCI.
There is one patch for this that I did in the unicore32 tree,
I should probably go through it and fix everything else that sticks
out.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-02-23 17:50 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-22 17:49 [PATCH v3 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface Michal Simek
2011-02-22 17:49 ` Michal Simek
2011-02-22 17:49 ` [PATCH v3 2/2] i2c: xiic: Use 32bit accesses only Michal Simek
2011-02-22 17:49 ` Michal Simek
2011-02-22 18:08 ` Grant Likely
2011-02-22 18:08 ` Grant Likely
2011-02-23 8:08 ` Michal Simek
2011-02-23 8:08 ` Michal Simek
2011-02-23 8:36 ` Arnd Bergmann
2011-02-23 9:37 ` Michal Simek
2011-02-23 9:37 ` Michal Simek
2011-02-23 17:49 ` Arnd Bergmann
2011-02-23 17:49 ` Arnd Bergmann
2011-02-23 16:56 ` Grant Likely
2011-02-23 16:56 ` Grant Likely
2011-02-22 18:11 ` [PATCH v3 1/2] i2c: xiic: Add OF support for Xilinx i2c bus interface Grant Likely
2011-02-22 18:11 ` Grant Likely
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.