On Tuesday, February 09, 2016 at 02:26:37 PM, Purna Chandra Mandal wrote:
On 02/09/2016 06:48 PM, Marek Vasut wrote:
On Tuesday, February 09, 2016 at 01:02:49 PM, Purna Chandra Mandal wrote:
From: Cristian Birsan cristian.birsan@microchip.com
This driver adds support of PIC32 MUSB OTG controller as dual role device. It implements platform specific glue to reuse musb core.
Signed-off-by: Cristian Birsan cristian.birsan@microchip.com Signed-off-by: Purna Chandra Mandal purna.mandal@microchip.com
drivers/usb/gadget/f_mass_storage.c | 2 + drivers/usb/musb-new/Kconfig | 7 + drivers/usb/musb-new/Makefile | 1 + drivers/usb/musb-new/linux-compat.h | 2 + drivers/usb/musb-new/musb_core.c | 2 +- drivers/usb/musb-new/pic32.c | 294
++++++++++++++++++++++++++++++++++++ 6 files changed, 307 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/musb-new/pic32.c
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c index 1ecb92a..8ca02f2 100644 --- a/drivers/usb/gadget/f_mass_storage.c +++ b/drivers/usb/gadget/f_mass_storage.c @@ -283,6 +283,7 @@ static const char fsg_string_interface[] = "Mass Storage"; struct kref {int x; };
struct completion {int x; };
+#if !defined(CONFIG_MIPS)
Why is this change needed, endianness issues ?
Also, you should put this into separate patch.
This is to fix compilation error with MIPS. MIPS already implements these functions in "arch/mips/include/asm/bitops.h" (recently added). I'll add this in separate patch.
Ouch, I don't like seeing a driver become a platform abstraction library :( Unless you explicitly want to learn how far this rabbit hole goes, I won't push you to fix this. But if you really enjoy pain, patches are welcome to implement some sort of generic __weak set_bit and clear_bit() code.
inline void set_bit(int nr, volatile void *addr) {
int mask;
@@ -302,6 +303,7 @@ inline void clear_bit(int nr, volatile void *addr)
mask = 1 << (nr & 0x1f); *a &= ~mask;
}
+#endif
struct fsg_dev; struct fsg_common;
[...]
diff --git a/drivers/usb/musb-new/linux-compat.h b/drivers/usb/musb-new/linux-compat.h index 46f83d9..9ac48c1 100644 --- a/drivers/usb/musb-new/linux-compat.h +++ b/drivers/usb/musb-new/linux-compat.h @@ -13,12 +13,14 @@
printf(fmt, ##args); \ret_warn; })
+#if !defined(CONFIG_MIPS)
#define writesl(a, d, s) __raw_writesl((unsigned long)a, d, s) #define readsl(a, d, s) __raw_readsl((unsigned long)a, d, s) #define writesw(a, d, s) __raw_writesw((unsigned long)a, d, s) #define readsw(a, d, s) __raw_readsw((unsigned long)a, d, s) #define writesb(a, d, s) __raw_writesb((unsigned long)a, d, s) #define readsb(a, d, s) __raw_readsb((unsigned long)a, d, s)
+#endif
Why is this needed?
To fix compilation error with MIPS. MIPS already implements these macros in "arch/mips/include/asm/io.h".
OK, but these macros are also implemented on ARM systems and it's not a problem there.We should just nuke this whole section in a separate patch and see if some platform catches fire and fix that one instead.
#define device_init_wakeup(dev, a) do {} while (0)
[...]
+static int pic32_musb_set_mode(struct musb *musb, u8 mode) +{
- struct device *dev = musb->controller;
- switch (mode) {
- case MUSB_HOST:
clrsetbits_le32(musb_glue + USBCRCON,USBCRCON_USBIDVAL, USBCRCON_USBIDOVEN);Is pic32 mipsel ? Or is the core doing endian swapping ?
I would expect _be32() stuff on mips.
In PIC32 MIPS is little-endian.
Wow, neat :) Thanks for clarifying!
break;- case MUSB_PERIPHERAL:
setbits_le32(musb_glue + USBCRCON,USBCRCON_USBIDVAL | USBCRCON_USBIDOVEN);break;- case MUSB_OTG:
dev_err(dev, "MUSB OTG mode enabled\n");break;- default:
dev_err(dev, "unsupported mode %d\n", mode);return -EINVAL;- }
- return 0;
+}
[...]
Looks good otherwise.