2010-06-21 15:53:57

by Hartley Sweeten

[permalink] [raw]
Subject: [PATCH v2] Staging: dt3155: Cleanup memory mapped i/o access

The macros ReadMReg and WriteMReg are really just private versions of
the kernel's readl and writel functions. Use the kernel's functions
instead. And since ioremap returns a (void __iomem *) not a (u8 *),
change all the uses of dt3155_lbase to reflect this.

While here, make dt3155_lbase static since it is only used in the
dt3155_drv.c file. Also, remove the global variable dt3155_bbase
since it is not used anywhere in the code.

Where is makes sense, create a local 'mmio' variable instead of using
dt3155_lbase[minor] to make the code more readable.

This change also affects the {Read|Write}I2C functions so they are
also modified as needed.

Signed-off-by: H Hartley Sweeten <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Scott Smedley <[email protected]>

---

V2 - rebased to next-20100621

diff --git a/drivers/staging/dt3155/dt3155_drv.c b/drivers/staging/dt3155/dt3155_drv.c
index dcd3849..88c9e59 100644
--- a/drivers/staging/dt3155/dt3155_drv.c
+++ b/drivers/staging/dt3155/dt3155_drv.c
@@ -64,8 +64,8 @@ extern void printques(int);
#include <linux/poll.h>
#include <linux/sched.h>
#include <linux/smp_lock.h>
+#include <linux/io.h>

-#include <asm/io.h>
#include <asm/uaccess.h>

#include "dt3155.h"
@@ -112,14 +112,12 @@ int dt3155_major = 0;
struct dt3155_status dt3155_status[MAXBOARDS];

/* kernel logical address of the board */
-u8 *dt3155_lbase[MAXBOARDS] = { NULL
+static void __iomem *dt3155_lbase[MAXBOARDS] = { NULL
#if MAXBOARDS == 2
, NULL
#endif
};
-/* DT3155 registers */
-u8 *dt3155_bbase = NULL; /* kernel logical address of the *
- * buffer region */
+
u32 dt3155_dev_open[MAXBOARDS] = {0
#if MAXBOARDS == 2
, 0
@@ -139,11 +137,11 @@ static void quick_stop (int minor)
{
// TODO: scott was here
#if 1
- ReadMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ int_csr_r.reg = readl(dt3155_lbase[minor] + INT_CSR);
/* disable interrupts */
int_csr_r.fld.FLD_END_EVE_EN = 0;
int_csr_r.fld.FLD_END_ODD_EN = 0;
- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, dt3155_lbase[minor] + INT_CSR);

dt3155_status[minor].state &= ~(DT3155_STATE_STOP|0xff);
/* mark the system stopped: */
@@ -171,6 +169,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
int index;
unsigned long flags;
u32 buffer_addr;
+ void __iomem *mmio;

/* find out who issued the interrupt */
for (index = 0; index < ndevices; index++) {
@@ -187,8 +186,10 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
return;
}

+ mmio = dt3155_lbase[minor];
+
/* Check for corruption and set a flag if so */
- ReadMReg((dt3155_lbase[minor] + CSR1), csr1_r.reg);
+ csr1_r.reg = readl(mmio + CSR1);

if ((csr1_r.fld.FLD_CRPT_EVE) || (csr1_r.fld.FLD_CRPT_ODD))
{
@@ -200,7 +201,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
return;
}

- ReadMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ int_csr_r.reg = readl(mmio + INT_CSR);

/* Handle the even field ... */
if (int_csr_r.fld.FLD_END_EVE)
@@ -211,7 +212,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
dt3155_fbuffer[minor]->frame_count++;
}

- ReadI2C(dt3155_lbase[minor], EVEN_CSR, &i2c_even_csr.reg);
+ ReadI2C(mmio, EVEN_CSR, &i2c_even_csr.reg);

/* Clear the interrupt? */
int_csr_r.fld.FLD_END_EVE = 1;
@@ -231,7 +232,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
}
}

- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, mmio + INT_CSR);

/* Set up next DMA if we are doing FIELDS */
if ((dt3155_status[minor].state & DT3155_STATE_MODE) ==
@@ -249,7 +250,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)

/* Set up the DMA address for the next field */
local_irq_restore(flags);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_START), buffer_addr);
+ writel(buffer_addr, mmio + ODD_DMA_START);
}

/* Check for errors. */
@@ -257,7 +258,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
if (i2c_even_csr.fld.ERROR_EVE)
dt3155_errno = DT_ERR_OVERRUN;

- WriteI2C(dt3155_lbase[minor], EVEN_CSR, i2c_even_csr.reg);
+ WriteI2C(mmio, EVEN_CSR, i2c_even_csr.reg);

/* Note that we actually saw an even field meaning */
/* that subsequent odd field complete the frame */
@@ -274,7 +275,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
/* ... now handle the odd field */
if (int_csr_r.fld.FLD_END_ODD)
{
- ReadI2C(dt3155_lbase[minor], ODD_CSR, &i2c_odd_csr.reg);
+ ReadI2C(mmio, ODD_CSR, &i2c_odd_csr.reg);

/* Clear the interrupt? */
int_csr_r.fld.FLD_END_ODD = 1;
@@ -310,7 +311,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
}
}

- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, mmio + INT_CSR);

/* if the odd field has been acquired, then */
/* change the next dma location for both fields */
@@ -387,14 +388,14 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
if ((dt3155_status[minor].state & DT3155_STATE_MODE) ==
DT3155_STATE_FLD)
{
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START), buffer_addr);
+ writel(buffer_addr, mmio + EVEN_DMA_START);
}
else
{
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START), buffer_addr);
+ writel(buffer_addr, mmio + EVEN_DMA_START);

- WriteMReg((dt3155_lbase[minor] + ODD_DMA_START), buffer_addr
- + dt3155_status[minor].config.cols);
+ writel(buffer_addr + dt3155_status[minor].config.cols,
+ mmio + ODD_DMA_START);
}

/* Do error checking */
@@ -402,7 +403,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
if (i2c_odd_csr.fld.ERROR_ODD)
dt3155_errno = DT_ERR_OVERRUN;

- WriteI2C(dt3155_lbase[minor], ODD_CSR, i2c_odd_csr.reg);
+ WriteI2C(mmio, ODD_CSR, i2c_odd_csr.reg);

return;
}
@@ -419,6 +420,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
static void dt3155_init_isr(int minor)
{
const u32 stride = dt3155_status[minor].config.cols;
+ void __iomem *mmio = dt3155_lbase[minor];

switch (dt3155_status[minor].state & DT3155_STATE_MODE)
{
@@ -429,12 +431,9 @@ static void dt3155_init_isr(int minor)
even_dma_stride_r = 0;
odd_dma_stride_r = 0;

- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START),
- even_dma_start_r);
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_STRIDE),
- even_dma_stride_r);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_STRIDE),
- odd_dma_stride_r);
+ writel(even_dma_start_r, mmio + EVEN_DMA_START);
+ writel(even_dma_stride_r, mmio + EVEN_DMA_STRIDE);
+ writel(odd_dma_stride_r, mmio + ODD_DMA_STRIDE);
break;
}

@@ -447,14 +446,10 @@ static void dt3155_init_isr(int minor)
even_dma_stride_r = stride;
odd_dma_stride_r = stride;

- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START),
- even_dma_start_r);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_START),
- odd_dma_start_r);
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_STRIDE),
- even_dma_stride_r);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_STRIDE),
- odd_dma_stride_r);
+ writel(even_dma_start_r, mmio + EVEN_DMA_START);
+ writel(odd_dma_start_r, mmio + ODD_DMA_START);
+ writel(even_dma_stride_r, mmio + EVEN_DMA_STRIDE);
+ writel(odd_dma_stride_r, mmio + ODD_DMA_STRIDE);
break;
}
}
@@ -462,9 +457,9 @@ static void dt3155_init_isr(int minor)
/* 50/60 Hz should be set before this point but let's make sure it is */
/* right anyway */

- ReadI2C(dt3155_lbase[minor], CSR2, &i2c_csr2.reg);
+ ReadI2C(mmio, CSR2, &i2c_csr2.reg);
i2c_csr2.fld.HZ50 = FORMAT50HZ;
- WriteI2C(dt3155_lbase[minor], CSR2, i2c_csr2.reg);
+ WriteI2C(mmio, CSR2, i2c_csr2.reg);

/* enable busmaster chip, clear flags */

@@ -484,7 +479,7 @@ static void dt3155_init_isr(int minor)
csr1_r.fld.FLD_CRPT_EVE = 1; /* writing a 1 clears flags */
csr1_r.fld.FLD_CRPT_ODD = 1;

- WriteMReg((dt3155_lbase[minor] + CSR1),csr1_r.reg);
+ writel(csr1_r.reg, mmio + CSR1);

/* Enable interrupts at the end of each field */

@@ -493,14 +488,14 @@ static void dt3155_init_isr(int minor)
int_csr_r.fld.FLD_END_ODD_EN = 1;
int_csr_r.fld.FLD_START_EN = 0;

- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, mmio + INT_CSR);

/* start internal BUSY bits */

- ReadI2C(dt3155_lbase[minor], CSR2, &i2c_csr2.reg);
+ ReadI2C(mmio, CSR2, &i2c_csr2.reg);
i2c_csr2.fld.BUSY_ODD = 1;
i2c_csr2.fld.BUSY_EVE = 1;
- WriteI2C(dt3155_lbase[minor], CSR2, i2c_csr2.reg);
+ WriteI2C(mmio, CSR2, i2c_csr2.reg);

/* Now its up to the interrupt routine!! */

@@ -709,7 +704,7 @@ static int dt3155_open(struct inode* inode, struct file* filep)

/* Disable ALL interrupts */
int_csr_r.reg = 0;
- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, dt3155_lbase[minor] + INT_CSR);

init_waitqueue_head(&(dt3155_read_wait_queue[minor]));

@@ -911,7 +906,7 @@ static int find_PCI (void)

/* Remap the base address to a logical address through which we
* can access it. */
- dt3155_lbase[pci_index - 1] = ioremap(base,PCI_PAGE_SIZE);
+ dt3155_lbase[pci_index - 1] = ioremap(base, PCI_PAGE_SIZE);
dt3155_status[pci_index - 1].reg_addr = base;
DT_3155_DEBUG_MSG("DT3155: New logical address is %p \n",
dt3155_lbase[pci_index-1]);
@@ -1036,7 +1031,7 @@ int init_module(void)
int_csr_r.reg = 0;
for( index = 0; index < ndevices; index++)
{
- WriteMReg((dt3155_lbase[index] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, dt3155_lbase[index] + INT_CSR);
if(dt3155_status[index].device_installed)
{
/*
diff --git a/drivers/staging/dt3155/dt3155_drv.h b/drivers/staging/dt3155/dt3155_drv.h
index 95e68c3..c447c61 100644
--- a/drivers/staging/dt3155/dt3155_drv.h
+++ b/drivers/staging/dt3155/dt3155_drv.h
@@ -24,12 +24,6 @@ MA 02111-1307 USA
#ifndef DT3155_DRV_INC
#define DT3155_DRV_INC

-/* kernel logical address of the frame grabbers */
-extern u8 *dt3155_lbase[MAXBOARDS];
-
-/* kernel logical address of ram buffer */
-extern u8 *dt3155_bbase;
-
#ifdef __KERNEL__
#include <linux/wait.h>

diff --git a/drivers/staging/dt3155/dt3155_io.c b/drivers/staging/dt3155/dt3155_io.c
index 7792e71..485cc5e 100644
--- a/drivers/staging/dt3155/dt3155_io.c
+++ b/drivers/staging/dt3155/dt3155_io.c
@@ -21,6 +21,8 @@
*/

#include <linux/delay.h>
+#include <linux/io.h>
+
#include "dt3155.h"
#include "dt3155_io.h"
#include "dt3155_drv.h"
@@ -75,13 +77,13 @@ u8 i2c_pm_lut_data;
*
* This function handles read/write timing and r/w timeout error
*/
-static int wait_ibsyclr(u8 *lpReg)
+static int wait_ibsyclr(void __iomem *mmio)
{
/* wait 100 microseconds */
udelay(100L);
/* __delay(loops_per_sec/10000); */

- ReadMReg(lpReg + IIC_CSR2, iic_csr2_r.reg);
+ iic_csr2_r.reg = readl(mmio + IIC_CSR2);
if (iic_csr2_r.fld.NEW_CYCLE) {
/* if NEW_CYCLE didn't clear */
/* TIMEOUT ERROR */
@@ -101,11 +103,11 @@ static int wait_ibsyclr(u8 *lpReg)
* 2nd parameter is reg. index;
* 3rd is value to be written
*/
-int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal)
+int WriteI2C(void __iomem *mmio, u_short wIregIndex, u8 byVal)
{
/* read 32 bit IIC_CSR2 register data into union */

- ReadMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ iic_csr2_r.reg = readl(mmio + IIC_CSR2);

/* for write operation */
iic_csr2_r.fld.DIR_RD = 0;
@@ -117,10 +119,10 @@ int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal)
iic_csr2_r.fld.NEW_CYCLE = 1;

/* xfer union data into 32 bit IIC_CSR2 register */
- WriteMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ writel(iic_csr2_r.reg, mmio + IIC_CSR2);

/* wait for IIC cycle to finish */
- return wait_ibsyclr(lpReg);
+ return wait_ibsyclr(mmio);
}

/*
@@ -132,12 +134,12 @@ int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal)
* 2nd parameter is reg. index;
* 3rd is adrs of value to be read
*/
-int ReadI2C(u8 *lpReg, u_short wIregIndex, u8 *byVal)
+int ReadI2C(void __iomem *mmio, u_short wIregIndex, u8 *byVal)
{
int writestat; /* status for return */

/* read 32 bit IIC_CSR2 register data into union */
- ReadMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ iic_csr2_r.reg = readl(mmio + IIC_CSR2);

/* for read operation */
iic_csr2_r.fld.DIR_RD = 1;
@@ -149,14 +151,14 @@ int ReadI2C(u8 *lpReg, u_short wIregIndex, u8 *byVal)
iic_csr2_r.fld.NEW_CYCLE = 1;

/* xfer union's data into 32 bit IIC_CSR2 register */
- WriteMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ writel(iic_csr2_r.reg, mmio + IIC_CSR2);

/* wait for IIC cycle to finish */
- writestat = wait_ibsyclr(lpReg);
+ writestat = wait_ibsyclr(mmio);

/* Next 2 commands read 32 bit IIC_CSR1 register's data into union */
/* first read data is in IIC_CSR1 */
- ReadMReg((lpReg + IIC_CSR1), iic_csr1_r.reg);
+ iic_csr1_r.reg = readl(mmio + IIC_CSR1);

/* now get data u8 out of register */
*byVal = (u8) iic_csr1_r.fld.RD_DATA;
diff --git a/drivers/staging/dt3155/dt3155_io.h b/drivers/staging/dt3155/dt3155_io.h
index d1a2510..a9aa754 100644
--- a/drivers/staging/dt3155/dt3155_io.h
+++ b/drivers/staging/dt3155/dt3155_io.h
@@ -34,11 +34,6 @@ MA 02111-1307 USA
#ifndef DT3155_IO_INC
#define DT3155_IO_INC

-/* macros to access registers */
-
-#define WriteMReg(Address, Data) (*((u32 *)(Address)) = Data)
-#define ReadMReg(Address, Data) (Data = *((u32 *)(Address)))
-
/***************** 32 bit register globals **************/

/* offsets for 32-bit memory mapped registers */
@@ -352,7 +347,7 @@ extern u8 i2c_pm_lut_data;

/* access 8-bit IIC registers */

-extern int ReadI2C(u8 *lpReg, u_short wIregIndex, u8 *byVal);
-extern int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal);
+extern int ReadI2C(void __iomem *mmio, u_short wIregIndex, u8 *byVal);
+extern int WriteI2C(void __iomem *mmio, u_short wIregIndex, u8 byVal);

#endif


2010-06-22 22:41:58

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2] Staging: dt3155: Cleanup memory mapped i/o access

On Mon, Jun 21, 2010 at 10:51:54AM -0500, H Hartley Sweeten wrote:
> The macros ReadMReg and WriteMReg are really just private versions of
> the kernel's readl and writel functions. Use the kernel's functions
> instead. And since ioremap returns a (void __iomem *) not a (u8 *),
> change all the uses of dt3155_lbase to reflect this.
>
> While here, make dt3155_lbase static since it is only used in the
> dt3155_drv.c file. Also, remove the global variable dt3155_bbase
> since it is not used anywhere in the code.
>
> Where is makes sense, create a local 'mmio' variable instead of using
> dt3155_lbase[minor] to make the code more readable.
>
> This change also affects the {Read|Write}I2C functions so they are
> also modified as needed.
>
> Signed-off-by: H Hartley Sweeten <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Scott Smedley <[email protected]>
>
> ---
>
> V2 - rebased to next-20100621

You are still doing something odd, this one doesn't apply either. Is
your email client messing something up?

strange.

greg k-h

2010-06-22 22:45:41

by Hartley Sweeten

[permalink] [raw]
Subject: RE: [PATCH v2] Staging: dt3155: Cleanup memory mapped i/o access

On Tuesday, June 22, 2010 3:39 PM, Greg KH wrote:
> You are still doing something odd, this one doesn't apply either. Is
> your email client messing something up?
>
> strange.

Hmm. I thought I had that worked out since I haven't had any problems lately.

Can you please try this one?

---

The macros ReadMReg and WriteMReg are really just private versions of
the kernel's readl and writel functions. Use the kernel's functions
instead. And since ioremap returns a (void __iomem *) not a (u8 *),
change all the uses of dt3155_lbase to reflect this.

While here, make dt3155_lbase static since it is only used in the
dt3155_drv.c file. Also, remove the global variable dt3155_bbase
since it is not used anywhere in the code.

Where is makes sense, create a local 'mmio' variable instead of using
dt3155_lbase[minor] to make the code more readable.

This change also affects the {Read|Write}I2C functions so they are
also modified as needed.

Signed-off-by: H Hartley Sweeten <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Scott Smedley <[email protected]>

---

V2 - rebased to next-20100621

diff --git a/drivers/staging/dt3155/dt3155_drv.c b/drivers/staging/dt3155/dt3155_drv.c
index dcd3849..88c9e59 100644
--- a/drivers/staging/dt3155/dt3155_drv.c
+++ b/drivers/staging/dt3155/dt3155_drv.c
@@ -64,8 +64,8 @@ extern void printques(int);
#include <linux/poll.h>
#include <linux/sched.h>
#include <linux/smp_lock.h>
+#include <linux/io.h>

-#include <asm/io.h>
#include <asm/uaccess.h>

#include "dt3155.h"
@@ -112,14 +112,12 @@ int dt3155_major = 0;
struct dt3155_status dt3155_status[MAXBOARDS];

/* kernel logical address of the board */
-u8 *dt3155_lbase[MAXBOARDS] = { NULL
+static void __iomem *dt3155_lbase[MAXBOARDS] = { NULL
#if MAXBOARDS == 2
, NULL
#endif
};
-/* DT3155 registers */
-u8 *dt3155_bbase = NULL; /* kernel logical address of the *
- * buffer region */
+
u32 dt3155_dev_open[MAXBOARDS] = {0
#if MAXBOARDS == 2
, 0
@@ -139,11 +137,11 @@ static void quick_stop (int minor)
{
// TODO: scott was here
#if 1
- ReadMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ int_csr_r.reg = readl(dt3155_lbase[minor] + INT_CSR);
/* disable interrupts */
int_csr_r.fld.FLD_END_EVE_EN = 0;
int_csr_r.fld.FLD_END_ODD_EN = 0;
- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, dt3155_lbase[minor] + INT_CSR);

dt3155_status[minor].state &= ~(DT3155_STATE_STOP|0xff);
/* mark the system stopped: */
@@ -171,6 +169,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
int index;
unsigned long flags;
u32 buffer_addr;
+ void __iomem *mmio;

/* find out who issued the interrupt */
for (index = 0; index < ndevices; index++) {
@@ -187,8 +186,10 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
return;
}

+ mmio = dt3155_lbase[minor];
+
/* Check for corruption and set a flag if so */
- ReadMReg((dt3155_lbase[minor] + CSR1), csr1_r.reg);
+ csr1_r.reg = readl(mmio + CSR1);

if ((csr1_r.fld.FLD_CRPT_EVE) || (csr1_r.fld.FLD_CRPT_ODD))
{
@@ -200,7 +201,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
return;
}

- ReadMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ int_csr_r.reg = readl(mmio + INT_CSR);

/* Handle the even field ... */
if (int_csr_r.fld.FLD_END_EVE)
@@ -211,7 +212,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
dt3155_fbuffer[minor]->frame_count++;
}

- ReadI2C(dt3155_lbase[minor], EVEN_CSR, &i2c_even_csr.reg);
+ ReadI2C(mmio, EVEN_CSR, &i2c_even_csr.reg);

/* Clear the interrupt? */
int_csr_r.fld.FLD_END_EVE = 1;
@@ -231,7 +232,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
}
}

- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, mmio + INT_CSR);

/* Set up next DMA if we are doing FIELDS */
if ((dt3155_status[minor].state & DT3155_STATE_MODE) ==
@@ -249,7 +250,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)

/* Set up the DMA address for the next field */
local_irq_restore(flags);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_START), buffer_addr);
+ writel(buffer_addr, mmio + ODD_DMA_START);
}

/* Check for errors. */
@@ -257,7 +258,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
if (i2c_even_csr.fld.ERROR_EVE)
dt3155_errno = DT_ERR_OVERRUN;

- WriteI2C(dt3155_lbase[minor], EVEN_CSR, i2c_even_csr.reg);
+ WriteI2C(mmio, EVEN_CSR, i2c_even_csr.reg);

/* Note that we actually saw an even field meaning */
/* that subsequent odd field complete the frame */
@@ -274,7 +275,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
/* ... now handle the odd field */
if (int_csr_r.fld.FLD_END_ODD)
{
- ReadI2C(dt3155_lbase[minor], ODD_CSR, &i2c_odd_csr.reg);
+ ReadI2C(mmio, ODD_CSR, &i2c_odd_csr.reg);

/* Clear the interrupt? */
int_csr_r.fld.FLD_END_ODD = 1;
@@ -310,7 +311,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
}
}

- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, mmio + INT_CSR);

/* if the odd field has been acquired, then */
/* change the next dma location for both fields */
@@ -387,14 +388,14 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
if ((dt3155_status[minor].state & DT3155_STATE_MODE) ==
DT3155_STATE_FLD)
{
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START), buffer_addr);
+ writel(buffer_addr, mmio + EVEN_DMA_START);
}
else
{
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START), buffer_addr);
+ writel(buffer_addr, mmio + EVEN_DMA_START);

- WriteMReg((dt3155_lbase[minor] + ODD_DMA_START), buffer_addr
- + dt3155_status[minor].config.cols);
+ writel(buffer_addr + dt3155_status[minor].config.cols,
+ mmio + ODD_DMA_START);
}

/* Do error checking */
@@ -402,7 +403,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
if (i2c_odd_csr.fld.ERROR_ODD)
dt3155_errno = DT_ERR_OVERRUN;

- WriteI2C(dt3155_lbase[minor], ODD_CSR, i2c_odd_csr.reg);
+ WriteI2C(mmio, ODD_CSR, i2c_odd_csr.reg);

return;
}
@@ -419,6 +420,7 @@ static void dt3155_isr(int irq, void *dev_id, struct pt_regs *regs)
static void dt3155_init_isr(int minor)
{
const u32 stride = dt3155_status[minor].config.cols;
+ void __iomem *mmio = dt3155_lbase[minor];

switch (dt3155_status[minor].state & DT3155_STATE_MODE)
{
@@ -429,12 +431,9 @@ static void dt3155_init_isr(int minor)
even_dma_stride_r = 0;
odd_dma_stride_r = 0;

- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START),
- even_dma_start_r);
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_STRIDE),
- even_dma_stride_r);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_STRIDE),
- odd_dma_stride_r);
+ writel(even_dma_start_r, mmio + EVEN_DMA_START);
+ writel(even_dma_stride_r, mmio + EVEN_DMA_STRIDE);
+ writel(odd_dma_stride_r, mmio + ODD_DMA_STRIDE);
break;
}

@@ -447,14 +446,10 @@ static void dt3155_init_isr(int minor)
even_dma_stride_r = stride;
odd_dma_stride_r = stride;

- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_START),
- even_dma_start_r);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_START),
- odd_dma_start_r);
- WriteMReg((dt3155_lbase[minor] + EVEN_DMA_STRIDE),
- even_dma_stride_r);
- WriteMReg((dt3155_lbase[minor] + ODD_DMA_STRIDE),
- odd_dma_stride_r);
+ writel(even_dma_start_r, mmio + EVEN_DMA_START);
+ writel(odd_dma_start_r, mmio + ODD_DMA_START);
+ writel(even_dma_stride_r, mmio + EVEN_DMA_STRIDE);
+ writel(odd_dma_stride_r, mmio + ODD_DMA_STRIDE);
break;
}
}
@@ -462,9 +457,9 @@ static void dt3155_init_isr(int minor)
/* 50/60 Hz should be set before this point but let's make sure it is */
/* right anyway */

- ReadI2C(dt3155_lbase[minor], CSR2, &i2c_csr2.reg);
+ ReadI2C(mmio, CSR2, &i2c_csr2.reg);
i2c_csr2.fld.HZ50 = FORMAT50HZ;
- WriteI2C(dt3155_lbase[minor], CSR2, i2c_csr2.reg);
+ WriteI2C(mmio, CSR2, i2c_csr2.reg);

/* enable busmaster chip, clear flags */

@@ -484,7 +479,7 @@ static void dt3155_init_isr(int minor)
csr1_r.fld.FLD_CRPT_EVE = 1; /* writing a 1 clears flags */
csr1_r.fld.FLD_CRPT_ODD = 1;

- WriteMReg((dt3155_lbase[minor] + CSR1),csr1_r.reg);
+ writel(csr1_r.reg, mmio + CSR1);

/* Enable interrupts at the end of each field */

@@ -493,14 +488,14 @@ static void dt3155_init_isr(int minor)
int_csr_r.fld.FLD_END_ODD_EN = 1;
int_csr_r.fld.FLD_START_EN = 0;

- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, mmio + INT_CSR);

/* start internal BUSY bits */

- ReadI2C(dt3155_lbase[minor], CSR2, &i2c_csr2.reg);
+ ReadI2C(mmio, CSR2, &i2c_csr2.reg);
i2c_csr2.fld.BUSY_ODD = 1;
i2c_csr2.fld.BUSY_EVE = 1;
- WriteI2C(dt3155_lbase[minor], CSR2, i2c_csr2.reg);
+ WriteI2C(mmio, CSR2, i2c_csr2.reg);

/* Now its up to the interrupt routine!! */

@@ -709,7 +704,7 @@ static int dt3155_open(struct inode* inode, struct file* filep)

/* Disable ALL interrupts */
int_csr_r.reg = 0;
- WriteMReg((dt3155_lbase[minor] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, dt3155_lbase[minor] + INT_CSR);

init_waitqueue_head(&(dt3155_read_wait_queue[minor]));

@@ -911,7 +906,7 @@ static int find_PCI (void)

/* Remap the base address to a logical address through which we
* can access it. */
- dt3155_lbase[pci_index - 1] = ioremap(base,PCI_PAGE_SIZE);
+ dt3155_lbase[pci_index - 1] = ioremap(base, PCI_PAGE_SIZE);
dt3155_status[pci_index - 1].reg_addr = base;
DT_3155_DEBUG_MSG("DT3155: New logical address is %p \n",
dt3155_lbase[pci_index-1]);
@@ -1036,7 +1031,7 @@ int init_module(void)
int_csr_r.reg = 0;
for( index = 0; index < ndevices; index++)
{
- WriteMReg((dt3155_lbase[index] + INT_CSR), int_csr_r.reg);
+ writel(int_csr_r.reg, dt3155_lbase[index] + INT_CSR);
if(dt3155_status[index].device_installed)
{
/*
diff --git a/drivers/staging/dt3155/dt3155_drv.h b/drivers/staging/dt3155/dt3155_drv.h
index 95e68c3..c447c61 100644
--- a/drivers/staging/dt3155/dt3155_drv.h
+++ b/drivers/staging/dt3155/dt3155_drv.h
@@ -24,12 +24,6 @@ MA 02111-1307 USA
#ifndef DT3155_DRV_INC
#define DT3155_DRV_INC

-/* kernel logical address of the frame grabbers */
-extern u8 *dt3155_lbase[MAXBOARDS];
-
-/* kernel logical address of ram buffer */
-extern u8 *dt3155_bbase;
-
#ifdef __KERNEL__
#include <linux/wait.h>

diff --git a/drivers/staging/dt3155/dt3155_io.c b/drivers/staging/dt3155/dt3155_io.c
index 7792e71..485cc5e 100644
--- a/drivers/staging/dt3155/dt3155_io.c
+++ b/drivers/staging/dt3155/dt3155_io.c
@@ -21,6 +21,8 @@
*/

#include <linux/delay.h>
+#include <linux/io.h>
+
#include "dt3155.h"
#include "dt3155_io.h"
#include "dt3155_drv.h"
@@ -75,13 +77,13 @@ u8 i2c_pm_lut_data;
*
* This function handles read/write timing and r/w timeout error
*/
-static int wait_ibsyclr(u8 *lpReg)
+static int wait_ibsyclr(void __iomem *mmio)
{
/* wait 100 microseconds */
udelay(100L);
/* __delay(loops_per_sec/10000); */

- ReadMReg(lpReg + IIC_CSR2, iic_csr2_r.reg);
+ iic_csr2_r.reg = readl(mmio + IIC_CSR2);
if (iic_csr2_r.fld.NEW_CYCLE) {
/* if NEW_CYCLE didn't clear */
/* TIMEOUT ERROR */
@@ -101,11 +103,11 @@ static int wait_ibsyclr(u8 *lpReg)
* 2nd parameter is reg. index;
* 3rd is value to be written
*/
-int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal)
+int WriteI2C(void __iomem *mmio, u_short wIregIndex, u8 byVal)
{
/* read 32 bit IIC_CSR2 register data into union */

- ReadMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ iic_csr2_r.reg = readl(mmio + IIC_CSR2);

/* for write operation */
iic_csr2_r.fld.DIR_RD = 0;
@@ -117,10 +119,10 @@ int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal)
iic_csr2_r.fld.NEW_CYCLE = 1;

/* xfer union data into 32 bit IIC_CSR2 register */
- WriteMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ writel(iic_csr2_r.reg, mmio + IIC_CSR2);

/* wait for IIC cycle to finish */
- return wait_ibsyclr(lpReg);
+ return wait_ibsyclr(mmio);
}

/*
@@ -132,12 +134,12 @@ int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal)
* 2nd parameter is reg. index;
* 3rd is adrs of value to be read
*/
-int ReadI2C(u8 *lpReg, u_short wIregIndex, u8 *byVal)
+int ReadI2C(void __iomem *mmio, u_short wIregIndex, u8 *byVal)
{
int writestat; /* status for return */

/* read 32 bit IIC_CSR2 register data into union */
- ReadMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ iic_csr2_r.reg = readl(mmio + IIC_CSR2);

/* for read operation */
iic_csr2_r.fld.DIR_RD = 1;
@@ -149,14 +151,14 @@ int ReadI2C(u8 *lpReg, u_short wIregIndex, u8 *byVal)
iic_csr2_r.fld.NEW_CYCLE = 1;

/* xfer union's data into 32 bit IIC_CSR2 register */
- WriteMReg((lpReg + IIC_CSR2), iic_csr2_r.reg);
+ writel(iic_csr2_r.reg, mmio + IIC_CSR2);

/* wait for IIC cycle to finish */
- writestat = wait_ibsyclr(lpReg);
+ writestat = wait_ibsyclr(mmio);

/* Next 2 commands read 32 bit IIC_CSR1 register's data into union */
/* first read data is in IIC_CSR1 */
- ReadMReg((lpReg + IIC_CSR1), iic_csr1_r.reg);
+ iic_csr1_r.reg = readl(mmio + IIC_CSR1);

/* now get data u8 out of register */
*byVal = (u8) iic_csr1_r.fld.RD_DATA;
diff --git a/drivers/staging/dt3155/dt3155_io.h b/drivers/staging/dt3155/dt3155_io.h
index d1a2510..a9aa754 100644
--- a/drivers/staging/dt3155/dt3155_io.h
+++ b/drivers/staging/dt3155/dt3155_io.h
@@ -34,11 +34,6 @@ MA 02111-1307 USA
#ifndef DT3155_IO_INC
#define DT3155_IO_INC

-/* macros to access registers */
-
-#define WriteMReg(Address, Data) (*((u32 *)(Address)) = Data)
-#define ReadMReg(Address, Data) (Data = *((u32 *)(Address)))
-
/***************** 32 bit register globals **************/

/* offsets for 32-bit memory mapped registers */
@@ -352,7 +347,7 @@ extern u8 i2c_pm_lut_data;

/* access 8-bit IIC registers */

-extern int ReadI2C(u8 *lpReg, u_short wIregIndex, u8 *byVal);
-extern int WriteI2C(u8 *lpReg, u_short wIregIndex, u8 byVal);
+extern int ReadI2C(void __iomem *mmio, u_short wIregIndex, u8 *byVal);
+extern int WriteI2C(void __iomem *mmio, u_short wIregIndex, u8 byVal);

#endif

2010-06-22 23:05:10

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2] Staging: dt3155: Cleanup memory mapped i/o access

On Tue, Jun 22, 2010 at 05:45:32PM -0500, H Hartley Sweeten wrote:
> On Tuesday, June 22, 2010 3:39 PM, Greg KH wrote:
> > You are still doing something odd, this one doesn't apply either. Is
> > your email client messing something up?
> >
> > strange.
>
> Hmm. I thought I had that worked out since I haven't had any problems lately.
>
> Can you please try this one?

Nope, I get the following errors:


>tching file drivers/staging/dt3155/dt3155_drv.c
Hunk #2 FAILED at 112.
Hunk #7 succeeded at 214 with fuzz 2.
Hunk #8 succeeded at 234 with fuzz 2.
Hunk #9 FAILED at 252.
Hunk #10 succeeded at 260 with fuzz 2.
Hunk #12 succeeded at 313 with fuzz 2.
Hunk #13 FAILED at 390.
Hunk #14 succeeded at 405 with fuzz 2.
Hunk #16 FAILED at 433.
Hunk #17 FAILED at 451.
Hunk #22 succeeded at 915 with fuzz 1.
Hunk #23 succeeded at 1040 with fuzz 2.
5 out of 23 hunks FAILED -- saving rejects to file drivers/staging/dt3155/dt3155_drv.c.rej
patching file drivers/staging/dt3155/dt3155_drv.h
patching file drivers/staging/dt3155/dt3155_io.c
Hunk #2 FAILED at 77.
Hunk #3 FAILED at 103.
Hunk #4 FAILED at 119.
Hunk #5 FAILED at 134.
Hunk #6 FAILED at 151.
5 out of 6 hunks FAILED -- saving rejects to file drivers/staging/dt3155/dt3155_io.c.rej
patching file drivers/staging/dt3155/dt3155_io.h
Reversed (or previously applied) patch detected! Assume -R? [n]

not good...

thanks,

greg k-h

2010-06-22 23:36:17

by Hartley Sweeten

[permalink] [raw]
Subject: RE: [PATCH v2] Staging: dt3155: Cleanup memory mapped i/o access

On Tuesday, June 22, 2010 4:04 PM, Greg KH wrote:
> On Tue, Jun 22, 2010 at 05:45:32PM -0500, H Hartley Sweeten wrote:
>> On Tuesday, June 22, 2010 3:39 PM, Greg KH wrote:
>>> You are still doing something odd, this one doesn't apply either. Is
>>> your email client messing something up?
>>>
>>> strange.
>>
>> Hmm. I thought I had that worked out since I haven't had any problems lately.
>>
>> Can you please try this one?
>
> Nope, I get the following errors:
>
>
>>tching file drivers/staging/dt3155/dt3155_drv.c
> Hunk #2 FAILED at 112.
> Hunk #7 succeeded at 214 with fuzz 2.
> Hunk #8 succeeded at 234 with fuzz 2.
> Hunk #9 FAILED at 252.
> Hunk #10 succeeded at 260 with fuzz 2.
> Hunk #12 succeeded at 313 with fuzz 2.
> Hunk #13 FAILED at 390.
> Hunk #14 succeeded at 405 with fuzz 2.
> Hunk #16 FAILED at 433.
> Hunk #17 FAILED at 451.
> Hunk #22 succeeded at 915 with fuzz 1.
> Hunk #23 succeeded at 1040 with fuzz 2.
> 5 out of 23 hunks FAILED -- saving rejects to file drivers/staging/dt3155/dt3155_drv.c.rej
> patching file drivers/staging/dt3155/dt3155_drv.h
> patching file drivers/staging/dt3155/dt3155_io.c
> Hunk #2 FAILED at 77.
> Hunk #3 FAILED at 103.
> Hunk #4 FAILED at 119.
> Hunk #5 FAILED at 134.
> Hunk #6 FAILED at 151.
> 5 out of 6 hunks FAILED -- saving rejects to file drivers/staging/dt3155/dt3155_io.c.rej
> patching file drivers/staging/dt3155/dt3155_io.h
> Reversed (or previously applied) patch detected! Assume -R? [n]
>
> not good...

Now I'm really confused.

~/src/git/linux-next $ patch --dry-run -p1 -i ../patches/dt3155_mmio_v2.patch
patching file drivers/staging/dt3155/dt3155_drv.c
patching file drivers/staging/dt3155/dt3155_drv.h
patching file drivers/staging/dt3155/dt3155_io.c
patching file drivers/staging/dt3155/dt3155_io.h

~/src/git/staging-next-2.6 $ patch --dry-run -p1 -i ../patches/dt3155_mmio_v2.patch
patching file drivers/staging/dt3155/dt3155_drv.c
Hunk #2 succeeded at 115 (offset 3 lines).
Hunk #3 succeeded at 140 (offset 3 lines).
Hunk #4 succeeded at 172 (offset 3 lines).
Hunk #5 succeeded at 189 (offset 3 lines).
Hunk #6 succeeded at 204 (offset 3 lines).
Hunk #7 succeeded at 215 (offset 3 lines).
Hunk #8 succeeded at 235 (offset 3 lines).
Hunk #9 succeeded at 253 (offset 3 lines).
Hunk #10 succeeded at 261 (offset 3 lines).
Hunk #11 succeeded at 278 (offset 3 lines).
Hunk #12 succeeded at 314 (offset 3 lines).
Hunk #13 succeeded at 391 (offset 3 lines).
Hunk #14 succeeded at 406 (offset 3 lines).
Hunk #15 succeeded at 423 (offset 3 lines).
Hunk #16 succeeded at 434 (offset 3 lines).
Hunk #17 succeeded at 449 (offset 3 lines).
Hunk #18 succeeded at 460 (offset 3 lines).
Hunk #19 succeeded at 482 (offset 3 lines).
Hunk #20 succeeded at 491 (offset 3 lines).
Hunk #21 succeeded at 706 (offset 2 lines).
Hunk #22 succeeded at 908 (offset 2 lines).
Hunk #23 succeeded at 1033 (offset 2 lines).
patching file drivers/staging/dt3155/dt3155_drv.h
patching file drivers/staging/dt3155/dt3155_io.c
patching file drivers/staging/dt3155/dt3155_io.h

Locally the patch applies fine against linux-next (next-20100622). And it applies
against your staging-next tree with some fuzz due to the following commits not in
that tree.

commit a46f9087e634224b3d0a6560e223425816846dff
Author: H Hartley Sweeten <[email protected]>
Date: Tue Jun 8 10:36:57 2010 -0500

Staging: dt3155: remove DT_3155_* errno defines

commit 0f3ff30b9384ffa1b435f263234531582080b100
Author: H Hartley Sweeten <[email protected]>
Date: Mon Jun 7 13:25:37 2010 -0500

Staging: dt3155: fix different address spaces noise in dt3155_drv.c

But, the patch in the mail does not apply. I get the same errors you have above.

Comparing the two files it appears my client does screw with the line endings.
This has been working fine so I'm not sure what changed. I will resend the
patch with a different client to see if that fixes the problem.

Thanks,
Hartley-