[PATCH] powerpc/usb: use ioremap instead of base plus offset to access regs

Below are codes for accessing usb sysif_regs in driver:

	usb_sys_regs = (struct usb_sys_interface *)
		((u32)dr_regs + USB_DR_SYS_OFFSET);

these codes work in 32-bit, but in 64-bit, accessing members of 'usb_sys_regs'
will cause call trace like below:

Unable to handle kernel paging request for data at address 0x8817a500
Faulting instruction address: 0x800000000011bae8
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2 P5020 DS
Modules linked in: fsl_usb2_udc(+)
NIP: 800000000011bae8 LR: 800000000011efb8 CTR: c0000000000ef500
REGS: c0000000f3e2f350 TRAP: 0300   Not tainted  (3.0.6-00001-g0700776)
MSR: 0000000080029000 <EE,ME,CE>  CR: 24000222  XER: 00000000
DEAR: 000000008817a500, ESR: 0000000000000000
TASK = c0000000fb3b8840[3466] 'insmod' THREAD: c0000000f3e2c000 CPU: 1
GPR00: 0000000000000002 c0000000f3e2f5d0 80000000001286b8 c0000000fb2e7c00
GPR04: 00000000000000d0 0000000000000000 c0000000f8b026c0 0000000000000041
GPR08: 0000000000000000 000000008817a400 c000000007653d28 0000000000000000
GPR12: 800000000011f3a8 c00000000ffff400 0000000000000000 0000000000000000
GPR16: 0000000000000000 000000007ff9eee4 0000000000000000 0000000000000000
GPR20: 0000000000000000 00000000100e2645 0000000000000001 0000000000000000
GPR24: c0000000f8b72c10 80000000001208e8 c0000000fb2e7c00 0000000000000000
GPR28: 0000000000000000 c0000000fb2e7c00 8000000000128198 80000000001208e8
NIP [800000000011bae8] .dr_controller_setup+0x318/0x360 [fsl_usb2_udc]
LR [800000000011efb8] .fsl_udc_probe+0x3b4/0x630 [fsl_usb2_udc]
Call Trace:
[c0000000f3e2f5d0] [c0000000f3e2f660] 0xc0000000f3e2f660 (unreliable)
[c0000000f3e2f660] [800000000011efb8] .fsl_udc_probe+0x3b4/0x630 [fsl_usb2_udc]
[c0000000f3e2f730] [c0000000002f2960] .platform_drv_probe+0x30/0x50
[c0000000f3e2f7a0] [c0000000002f0df0] .driver_probe_device+0xe0/0x230
[c0000000f3e2f840] [c0000000002f104c] .__driver_attach+0x10c/0x110
[c0000000f3e2f8d0] [c0000000002ef83c] .bus_for_each_dev+0x7c/0xd0
[c0000000f3e2f970] [c0000000002f08c8] .driver_attach+0x28/0x40
[c0000000f3e2f9f0] [c0000000002f02c0] .bus_add_driver+0xd0/0x310
[c0000000f3e2faa0] [c0000000002f15dc] .driver_register+0x9c/0x1a0
[c0000000f3e2fb40] [c0000000002f2dc0] .platform_driver_register+0x60/0x80
[c0000000f3e2fbc0] [c0000000002f2e14] .platform_driver_probe+0x34/0xf0
[c0000000f3e2fc50] [800000000011f26c] .udc_init+0x38/0x894 [fsl_usb2_udc]
[c0000000f3e2fcd0] [c000000000000eb0] .do_one_initcall+0x60/0x1e0
[c0000000f3e2fd90] [c00000000008a7b0] .SyS_init_module+0xd0/0x220
[c0000000f3e2fe30] [c0000000000005a0] syscall_exit+0x0/0x2c
Instruction dump:
eba1ffe8 ebc1fff0 ebe1fff8 7c0803a6 4e800020 6529c000 793c0020 4bfffdac
65291000 793c0020 e93f0010 7c0004ac
 0c000000 4c00012c 60000204
 ---[ end trace d152129396f60c53 ]---

Signed-off-by: Shaohui Xie <redacted>
---
 drivers/usb/gadget/fsl_udc_core.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index c81fbad..c656d2c 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c

@@ -263,9 +263,11 @@ static int dr_controller_setup(struct fsl_udc *udc)
 		/* fall through */
 	case FSL_USB2_PHY_UTMI:
 #ifdef CONFIG_FSL_SOC_BOOKE
+	if (udc->pdata->have_sysif_regs) {
 		setbits32(&usb_sys_regs->control, USB_CTRL_UTMI_PHY_EN |
 			       USB_CTRL_USB_EN);
 		udelay(10*1000);  /* delay for PHY clk to ready */
+	}
 #endif
 		portctrl |= PORTSCX_PTS_UTMI;
 		break;

@@ -986,7 +988,7 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 					queue);
 
 			/* Point the QH to the first TD of next request */
-			fsl_writel((u32) next_req->head, &qh->curr_dtd_ptr);
+			fsl_writel(*(u32 *) next_req->head, &qh->curr_dtd_ptr);
 		}
 
 		/* The request hasn't been processed, patch up the TD chain */

@@ -2497,8 +2499,8 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 
 #ifndef CONFIG_ARCH_MXC
 	if (pdata->have_sysif_regs)
-		usb_sys_regs = (struct usb_sys_interface *)
-				((u32)dr_regs + USB_DR_SYS_OFFSET);
+		usb_sys_regs = ioremap(res->start + USB_DR_SYS_OFFSET,
+				sizeof(struct usb_sys_interface)/sizeof(int));
 #endif
 
 	/* Initialize USB clocks */

-- 
1.6.4

=20

 #ifndef CONFIG_ARCH_MXC
 	if (pdata->have_sysif_regs)
-		usb_sys_regs =3D (struct usb_sys_interface *)
-				((u32)dr_regs + USB_DR_SYS_OFFSET);
+		usb_sys_regs =3D ioremap(res->start + USB_DR_SYS_OFFSET,
+				sizeof(struct

usb_sys_interface)/sizeof(int));

 #endif

That ioremap() doesn't look right.
Isn't the 'size' in bytes??
(Although it will only matter if it crosses a page boundary.)
Mind you, I'd have though the original ioremap() should
have covered the entire structure??

	David

-----Original Message-----
From: David Laight [mailto:David.Laight@ACULAB.COM]
Sent: Wednesday, November 02, 2011 5:07 PM
To: Xie Shaohui-B21989; linuxppc-dev@lists.ozlabs.org
Cc: linux-usb@vger.kernel.org
Subject: RE: [PATCH] powerpc/usb: use ioremap instead of base plus offset
to access regs

quoted

 #ifndef CONFIG_ARCH_MXC
 	if (pdata->have_sysif_regs)
-		usb_sys_regs =3D (struct usb_sys_interface *)
-				((u32)dr_regs + USB_DR_SYS_OFFSET);
+		usb_sys_regs =3D ioremap(res->start + USB_DR_SYS_OFFSET,
+				sizeof(struct

usb_sys_interface)/sizeof(int));

quoted

 #endif

That ioremap() doesn't look right.
Isn't the 'size' in bytes??

[Xie Shaohui] Yes, should not use sizeof(int).

(Although it will only matter if it crosses a page boundary.) Mind you,
I'd have though the original ioremap() should have covered the entire
structure??

[Xie Shaohui] The original ioremap() did cover the entire structure, but th=
e sysif_regs are not defined in dr_regs,
So regs of sysif_regs cannot be accessed as a member of a structure, curren=
t driver handle this by type casting, this did work in 32-bit, but in 64-bi=
t, this is not safe. So I use ioremap() to cover it again.


Best Regards,=20
Shaohui Xie

On Nov 2, 2011, at 4:39 AM, Xie Shaohui-B21989 wrote:

quoted

-----Original Message-----
From: David Laight [mailto:David.Laight@ACULAB.COM]
Sent: Wednesday, November 02, 2011 5:07 PM
To: Xie Shaohui-B21989; linuxppc-dev@lists.ozlabs.org
Cc: linux-usb@vger.kernel.org
Subject: RE: [PATCH] powerpc/usb: use ioremap instead of base plus =

offset

quoted

to access regs
=20
=20

quoted

#ifndef CONFIG_ARCH_MXC
	if (pdata->have_sysif_regs)
-		usb_sys_regs =3D (struct usb_sys_interface *)
-				((u32)dr_regs + USB_DR_SYS_OFFSET);
+		usb_sys_regs =3D ioremap(res->start + USB_DR_SYS_OFFSET,
+				sizeof(struct

usb_sys_interface)/sizeof(int));

quoted

#endif

=20
That ioremap() doesn't look right.
Isn't the 'size' in bytes??

[Xie Shaohui] Yes, should not use sizeof(int).
=20

quoted

(Although it will only matter if it crosses a page boundary.) Mind =

you,

quoted

I'd have though the original ioremap() should have covered the entire
structure??

[Xie Shaohui] The original ioremap() did cover the entire structure, =

but the sysif_regs are not defined in dr_regs,

So regs of sysif_regs cannot be accessed as a member of a structure, =

current driver handle this by type casting, this did work in 32-bit, but =
in 64-bit, this is not safe. So I use ioremap() to cover it again.

What's status on a new version of this patch.

Also, make sure to CC: Greg (gregkh@suse.de) on usb patches

- k=