arm: add missing writes{bwql}, reads{bwql}.
Subsystems (3)
PATCH v3 1/4 arm: add missing writes{bwql}, reads{bwql}.
ARM arch defines __raw_writes[bwql], __raw_reads[bwql] in io.h but not writes[bwql], reads[bwql] as required by some drivers. Some of the drivers are defining writes{bwlq} or reads{bwlq} as wrapper of their "__raw" version.
To avoid that lets add the wrapper in arch itself.
---
Changes in v3: None
Changes in v2: None
arch/arm/include/asm/io.h | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm/include/asm/io.h +7 -0
@@ -284,6 +284,13 @@ static inline void __raw_readsl(unsigned long addr, void *data, int longlen)#define insw_p (port,to,len) insw(port,to,len)#define insl_p (port,to,len) insl(port,to,len) +#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) +/* * ioremap and friends. *
Signature · 1 line
1.8.3.1
PATCH v3 2/4 drivers: musb-new: remove writes{bwlq} and reads{bwlq}.
Moved definition of writes{bwlq} and reads{bwlq} into arch. There is no need of having arch specific wrapper in driver.
---
Changes in v3: None
Changes in v2: None
drivers/usb/musb-new/linux-compat.h | 7 ------- 1 file changed, 7 deletions(-)
drivers/usb/musb-new/linux-compat.h +0 -7
@@ -13,13 +13,6 @@ printf(fmt, ##args); \ ret_warn; }) -#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) -#define device_init_wakeup (dev, a) do {} while (0)#define platform_data device_data
Signature · 1 line
1.8.3.1
PATCH v3 2/4 drivers: musb-new: remove writes{bwlq} and reads{bwlq}.
Quoted 2 lines · Purna Chandra Mandal
Moved definition of writes{bwlq} and reads{bwlq} into arch. There is no need of having arch specific wrapper in driver.
And so the patch does ... what exactly ? I cannot figure it out just by reading the commit message, sorry.
The patch itself is fine of course, but please fix the commit message.
Quoted 28 lines
Signed-off-by Purna Chandra Mandal <[email protected]>---
Changes in v3: None
Changes in v2: Nonedrivers/usb/musb-new/linux-compat.h | 7 ------- 1 file changed, 7 deletions(-)drivers/usb/musb-new/linux-compat.h +0
-7@@ -13,13 +13,6 @@ printf(fmt, ##args); \ ret_warn; }) -#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) -#define device_init_wakeup (dev, a) do {} while (0)#define platform_data device_data
Signature · 2 lines
Best regards, Marek Vasut
PATCH v3 2/4 drivers: musb-new: remove writes{bwlq} and reads{bwlq}.
Quoted 7 lines · Marek Vasut
Quoted 2 lines · Purna Chandra Mandal
Moved definition of writes{bwlq} and reads{bwlq} into arch. There is no need of having arch specific wrapper in driver.
And so the patch does ... what exactly ? I cannot figure it out just by reading the commit message, sorry.
The patch itself is fine of course, but please fix the commit message.
I hope new commit message will do.
"Definition of writes{bwlq}, reads{bwlq} are now added into arch specific asm/io.h.
So removing them from driver to fix re-definition error.
"
Quoted 29 lines
Quoted 28 lines
Signed-off-by Purna Chandra Mandal <[email protected]>---
Changes in v3: None
Changes in v2: Nonedrivers/usb/musb-new/linux-compat.h | 7 ------- 1 file changed, 7 deletions(-)drivers/usb/musb-new/linux-compat.h +0
-7@@ -13,13 +13,6 @@ printf(fmt, ##args); \ ret_warn; }) -#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) -#define device_init_wakeup (dev, a) do {} while (0)#define platform_data device_data
PATCH v3 2/4 drivers: musb-new: remove writes{bwlq} and reads{bwlq}.
Quoted 8 lines · Purna Chandra Mandal
Quoted 6 lines · Marek Vasut
Quoted 2 lines · Purna Chandra Mandal
Moved definition of writes{bwlq} and reads{bwlq} into arch. There is no need of having arch specific wrapper in driver.
And so the patch does ... what exactly ? I cannot figure it out just by reading the commit message, sorry.
Quoted 6 lines
Quoted 1 line
The patch itself is fine of course, but please fix the commit message.
I hope new commit message will do.
"Definition of writes{bwlq}, reads{bwlq} are now added into arch specificasm/io.h.
So removing them from driver to fix re-definition error.
"
Yes please :)
Signature · 2 lines
Best regards, Marek Vasut
PATCH v3 3/4 drivers: musb-new: Add USB DRC driver for Microchip PIC32 OTG controller.
In reply to Purna Chandra MandalThis driver adds support of PIC32 MUSB OTG controller as dual role device. It implements platform specific glue to reuse musb core.
---
Changes in v3: None
Changes in v2: None
drivers/usb/musb-new/Kconfig | 7 + drivers/usb/musb-new/Makefile | 1 + drivers/usb/musb-new/musb_core.c | 2 +- drivers/usb/musb-new/pic32.c | 294 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 drivers/usb/musb-new/pic32.c
drivers/usb/musb-new/Kconfig +7 -0
@@ -15,6 +15,13 @@ config USB_MUSB_GADGET if USB_MUSB_HOST || USB_MUSB_GADGET +config USB_MUSB_PIC32 + bool "Enable Microchip PIC32 DRC USB controller" + depends on DM_USB && MACH_PIC32 + help + Say y to enable PIC32 USB DRC controller support + if it is available on your Microchip PIC32 platform. + config USB_MUSB_SUNXI bool "Enable sunxi OTG / DRC USB controller" depends on ARCH_SUNXI
drivers/usb/musb-new/Makefile +1 -0
@@ -10,6 +10,7 @@ obj-$(CONFIG_USB_MUSB_HOST) += musb_host.o musb_core.o musb_uboot.o obj-$(CONFIG_USB_MUSB_DSPS) += musb_dsps.o obj-$(CONFIG_USB_MUSB_AM35X) += am35x.o obj-$(CONFIG_USB_MUSB_OMAP2PLUS) += omap2430.o +obj-$(CONFIG_USB_MUSB_PIC32) += pic32.o obj-$(CONFIG_USB_MUSB_SUNXI) += sunxi.o ccflags-y := $(call cc-option,-Wno-unused-variable) \
drivers/usb/musb-new/musb_core.c +1 -1
@@ -259,7 +259,7 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src) } } -#if !defined(CONFIG_USB_MUSB_AM35X ) +#if !defined(CONFIG_USB_MUSB_AM35X ) && !defined(CONFIG_USB_MUSB_PIC32 )/* * Unload an endpoint's FIFO */
drivers/usb/musb-new/pic32.c new file +294 -0
294 lines not on this page · raw message
Signature · 1 line
1.8.3.1
PATCH v3 3/4 drivers: musb-new: Add USB DRC driver for Microchip PIC32 OTG controller.
Quoted 5 lines · Purna Chandra Mandal
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 <[email protected]>Signed-off-by Purna Chandra Mandal <[email protected]>
[...]
Quoted 39 lines
drivers/usb/musb-new/pic32.c new file +33
-0@@ -0,0 +1,294 @@ +/* +* Microchip PIC32 MUSB "glue layer" +* +* Copyright (C) 2015, Microchip Technology Inc. +* Cristian Birsan <[email protected]> +* Purna Chandra Mandal <[email protected]> +* +* SPDX-License-Identifier: GPL-2.0+ +* +* Based on the dsps "glue layer" code. +*/ + +#include <common.h> +#include <linux/usb/musb.h> +#include "linux-compat.h" +#include "musb_core.h" +#include "musb_uboot.h" + +DECLARE_GLOBAL_DATA_PTR ; + +#define PIC32_TX_EP_MASK 0x0f/* EP0 + 7 Tx EPs */ +#define PIC32_RX_EP_MASK 0x0e/* 7 Rx EPs */ + +#define MUSB_SOFTRST 0x7f +#define MUSB_SOFTRST_NRST BIT(0) +#define MUSB_SOFTRST_NRSTX BIT(1) + +#define USBCRCON 0 +#define USBCRCON_USBWKUPEN BIT(0)/* Enable Wakeup Interrupt */ +#define USBCRCON_USBRIE BIT(1)/* Enable Remote resume Interrupt */ +#define USBCRCON_USBIE BIT(2)/* Enable USB General interrupt */ +#define USBCRCON_SENDMONEN BIT(3)/* Enable Session End VBUS monitoring */ +#define USBCRCON_BSVALMONEN BIT(4)/* Enable B-Device VBUS monitoring */
drivers/usb/musb-new/pic32.c line 43
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+#define USBCRCON_ASVALMONEN BIT(5)/* Enable A-Device VBUS monitoring */ +#define USBCRCON_VBUSMONEN BIT(6)/* Enable VBUS monitoring */ +#define USBCRCON_PHYIDEN BIT(7)/* PHY ID monitoring enable */ +#define USBCRCON_USBIDVAL BIT(8)/* USB ID value */ +#define USBCRCON_USBIDOVEN BIT(9)/* USB ID override enable */ +#define USBCRCON_USBWK BIT(24)/* USB Wakeup Status */ +#define USBCRCON_USBRF BIT(25)/* USB Resume Status */ +#define USBCRCON_USBIF BIT(26)/* USB General Interrupt Status */ + +static void __iomem *musb_glue;
What would happen once you make a chip with two MUSB controllers ?
drivers/usb/musb-new/pic32.c line 47
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+/* pic32_musb_disable - disable HDRC */ +static void pic32_musb_disable (struct musb *musb) +{
Is there no way to shut down the MUSB on the PIC32 ?
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+} + +/* pic32_musb_enable - enable HDRC */ +static int pic32_musb_enable (struct musb *musb) +{ +/* soft reset by NRSTx */ +musb_writeb (musb->mregs,MUSB_SOFTRST ,MUSB_SOFTRST_NRSTX ); +/* set mode */ +musb_platform_set_mode (musb, musb->board_mode); +
drivers/usb/musb-new/pic32.c line 67
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+return 0 ; +} + +static irqreturn_t pic32_interrupt (int irq,void *hci) +{ +struct musb *musb = hci; +irqreturn_t ret =IRQ_NONE ; +u32 epintr, usbintr; + +/* Get usb core interrupts */
You mean "get" or "ack" here ?
drivers/usb/musb-new/pic32.c line 72
Quoted 5 lines
drivers/usb/musb-new/pic32.c new file +5
-0+ musb->int_usb =musb_readb (musb->mregs,MUSB_INTRUSB ); +if (musb->int_usb) +musb_writeb (musb->mregs,MUSB_INTRUSB , musb->int_usb); + +/* Get endpoint interrupts */
DTTO
drivers/usb/musb-new/pic32.c line 75
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+ musb->int_rx =musb_readw (musb->mregs,MUSB_INTRRX ) &PIC32_RX_EP_MASK ; +if (musb->int_rx) +musb_writew (musb->mregs,MUSB_INTRRX , musb->int_rx);
Same here
Quoted 21 lines
drivers/usb/musb-new/pic32.c new file +21
-0+ musb->int_tx =musb_readw (musb->mregs,MUSB_INTRTX ) &PIC32_TX_EP_MASK ; +if (musb->int_tx) +musb_writew (musb->mregs,MUSB_INTRTX , musb->int_tx); + +/* Drop spurious RX and TX if device is disconnected */ +if (musb->int_usb &MUSB_INTR_DISCONNECT ) { + musb->int_tx =0 ; + musb->int_rx =0 ; + } + +if (musb->int_tx || musb->int_rx || musb->int_usb) + ret |=musb_interrupt (musb); + +return ret; +} + +static int pic32_musb_set_mode (struct musb *musb,u8 mode) +{ +struct device *dev = musb->controller; + +switch (mode) {
drivers/usb/musb-new/pic32.c line 107
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+case MUSB_HOST : +clrsetbits_le32 (musb_glue +USBCRCON , +USBCRCON_USBIDVAL ,USBCRCON_USBIDOVEN ); +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" );
So having the core in OTG mode is an error ? Why ?
Quoted 37 lines
drivers/usb/musb-new/pic32.c new file +37
-0+break ; +default : +dev_err (dev,"unsupported mode %d\n" , mode); +return -EINVAL ; + } + +return 0 ; +} + +static int pic32_musb_init (struct musb *musb) +{ +u32 ctrl, hwvers; +u8 power; + +/* Returns zero if not clocked */ + hwvers =musb_read_hwvers (musb->mregs); +if (!hwvers) +return -ENODEV ; + +/* Reset the musb */ + power =musb_readb (musb->mregs,MUSB_POWER ); + power = power |MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); +mdelay (100 ); + +/* Start the on-chip PHY and its PLL. */ + power = power & ~MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); + + musb->isr = pic32_interrupt; + + ctrl =USBCRCON_USBIF |USBCRCON_USBRF | +USBCRCON_USBWK |USBCRCON_USBIDOVEN | +USBCRCON_PHYIDEN |USBCRCON_USBIE | +USBCRCON_USBRIE |USBCRCON_USBWKUPEN | +USBCRCON_VBUSMONEN ; +writel (ctrl, musb_glue +USBCRCON );
drivers/usb/musb-new/pic32.c line 154
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+ +return 0 ; +} + +/* PIC32 supports only 32bit read operation */ +void musb_read_fifo (struct musb_hw_ep *hw_ep,u16 len,u8 *dst) +{ +void __iomem *fifo = hw_ep->fifo; +u32 val; +int i;
This could become:
bulk_len = len / 4;
bulk_rem = len % 4;
readsl(fifo, dst, bulk_len);
if (rem) {
dst += len & ~0x3;
tmp = readl(fifo);
copy the remaining bytes according to endianness
}
This is 10 LoC , not 50 ;-)
Quoted 24 lines
drivers/usb/musb-new/pic32.c new file +24
-0+/* Read for 32bit-aligned destination address */ +if (likely ((0x03 & (unsigned long )dst) ==0 ) && len >=4 ) { +readsl (fifo, dst, len /4 ); + dst += len & ~0x03 ; + len &=0x03 ; + } + +/* +* Now read the remaining 1 to 3 byte or complete length if +* unaligned address. +*/ +if (len >4 ) { +for (i =0 ; i < (len /4 ); i++) { + *(u32 *)dst =musb_readl (fifo,0 ); + dst +=4 ; + } + len &=0x03 ; + } + +if (len >0 ) { + val =musb_readl (fifo,0 ); +memcpy (dst, &val, len); + } +}
[...]
Signature · 2 lines
Best regards, Marek Vasut
PATCH v3 3/4 drivers: musb-new: Add USB DRC driver for Microchip PIC32 OTG controller.
Quoted 50 lines · Marek Vasut
Quoted 5 lines · Purna Chandra Mandal
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 <[email protected]>Signed-off-by Purna Chandra Mandal <[email protected]>[...]
Quoted 42 lines
drivers/usb/musb-new/pic32.c new file +36
-0@@ -0,0 +1,294 @@ +/* +* Microchip PIC32 MUSB "glue layer" +* +* Copyright (C) 2015, Microchip Technology Inc. +* Cristian Birsan <[email protected]> +* Purna Chandra Mandal <[email protected]> +* +* SPDX-License-Identifier: GPL-2.0+ +* +* Based on the dsps "glue layer" code. +*/ + +#include <common.h> +#include <linux/usb/musb.h> +#include "linux-compat.h" +#include "musb_core.h" +#include "musb_uboot.h" + +DECLARE_GLOBAL_DATA_PTR ; + +#define PIC32_TX_EP_MASK 0x0f/* EP0 + 7 Tx EPs */ +#define PIC32_RX_EP_MASK 0x0e/* 7 Rx EPs */ + +#define MUSB_SOFTRST 0x7f +#define MUSB_SOFTRST_NRST BIT(0) +#define MUSB_SOFTRST_NRSTX BIT(1) + +#define USBCRCON 0 +#define USBCRCON_USBWKUPEN BIT(0)/* Enable Wakeup Interrupt */ +#define USBCRCON_USBRIE BIT(1)/* Enable Remote resume Interrupt */ +#define USBCRCON_USBIE BIT(2)/* Enable USB General interrupt */ +#define USBCRCON_SENDMONEN BIT(3)/* Enable Session End VBUS monitoring */ +#define USBCRCON_BSVALMONEN BIT(4)/* Enable B-Device VBUS monitoring */ +#define USBCRCON_ASVALMONEN BIT(5)/* Enable A-Device VBUS monitoring */ +#define USBCRCON_VBUSMONEN BIT(6)/* Enable VBUS monitoring */ +#define USBCRCON_PHYIDEN BIT(7)/* PHY ID monitoring enable */
Quoted 8 lines
drivers/usb/musb-new/pic32.c line 43
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+#define USBCRCON_USBIDVAL BIT(8)/* USB ID value */ +#define USBCRCON_USBIDOVEN BIT(9)/* USB ID override enable */ +#define USBCRCON_USBWK BIT(24)/* USB Wakeup Status */ +#define USBCRCON_USBRF BIT(25)/* USB Resume Status */ +#define USBCRCON_USBIF BIT(26)/* USB General Interrupt Status */ + +static void __iomem *musb_glue;What would happen once you make a chip with two MUSB controllers ?
Currently PIC32 has only one MUSB controller and only one glue reg-space. Don't know how the reg-map will be in future when PIC32 will have multiple MUSB controllers. Assuming that glue address map will be separate for each controller we can add logic to support multiple MUSB controller.
IMO, better if we don't assume something of the future and bloat logic.
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 47
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+/* pic32_musb_disable - disable HDRC */ +static void pic32_musb_disable (struct musb *musb) +{Is there no way to shut down the MUSB on the PIC32 ?
There is no way to disable MUSB.
Quoted 13 lines
Quoted 13 lines
drivers/usb/musb-new/pic32.c new file +13
-0+} + +/* pic32_musb_enable - enable HDRC */ +static int pic32_musb_enable (struct musb *musb) +{ +/* soft reset by NRSTx */ +musb_writeb (musb->mregs,MUSB_SOFTRST ,MUSB_SOFTRST_NRSTX ); +/* set mode */ +musb_platform_set_mode (musb, musb->board_mode); + +return 0 ; +} +
Quoted 8 lines
drivers/usb/musb-new/pic32.c line 67
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+static irqreturn_t pic32_interrupt (int irq,void *hci) +{ +struct musb *musb = hci; +irqreturn_t ret =IRQ_NONE ; +u32 epintr, usbintr; + +/* Get usb core interrupts */ You mean "get" or "ack" here ?
I meant read-and-ack. Will update comment.
Quoted 6 lines
drivers/usb/musb-new/pic32.c line 72
Quoted 5 lines
drivers/usb/musb-new/pic32.c new file +5
-0+ musb->int_usb =musb_readb (musb->mregs,MUSB_INTRUSB ); +if (musb->int_usb) +musb_writeb (musb->mregs,MUSB_INTRUSB , musb->int_usb); + +/* Get endpoint interrupts */ DTTO
I meant read-and-ack.
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 75
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+ musb->int_rx =musb_readw (musb->mregs,MUSB_INTRRX ) &PIC32_RX_EP_MASK ; +if (musb->int_rx) +musb_writew (musb->mregs,MUSB_INTRRX , musb->int_rx);Same here
ack. Will update comment.
Quoted 24 lines
Quoted 24 lines
drivers/usb/musb-new/pic32.c new file +24
-0+ musb->int_tx =musb_readw (musb->mregs,MUSB_INTRTX ) &PIC32_TX_EP_MASK ; +if (musb->int_tx) +musb_writew (musb->mregs,MUSB_INTRTX , musb->int_tx); + +/* Drop spurious RX and TX if device is disconnected */ +if (musb->int_usb &MUSB_INTR_DISCONNECT ) { + musb->int_tx =0 ; + musb->int_rx =0 ; + } + +if (musb->int_tx || musb->int_rx || musb->int_usb) + ret |=musb_interrupt (musb); + +return ret; +} + +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 );
Quoted 8 lines
drivers/usb/musb-new/pic32.c line 107
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+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" );So having the core in OTG mode is an error ? Why ?
In PIC32 we haven't tested OTG. We use the controller as dual-role not as OTG. In future we might enable that.
Quoted 57 lines
Quoted 37 lines
drivers/usb/musb-new/pic32.c new file +37
-0+break ; +default : +dev_err (dev,"unsupported mode %d\n" , mode); +return -EINVAL ; + } + +return 0 ; +} + +static int pic32_musb_init (struct musb *musb) +{ +u32 ctrl, hwvers; +u8 power; + +/* Returns zero if not clocked */ + hwvers =musb_read_hwvers (musb->mregs); +if (!hwvers) +return -ENODEV ; + +/* Reset the musb */ + power =musb_readb (musb->mregs,MUSB_POWER ); + power = power |MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); +mdelay (100 ); + +/* Start the on-chip PHY and its PLL. */ + power = power & ~MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); + + musb->isr = pic32_interrupt; + + ctrl =USBCRCON_USBIF |USBCRCON_USBRF | +USBCRCON_USBWK |USBCRCON_USBIDOVEN | +USBCRCON_PHYIDEN |USBCRCON_USBIE | +USBCRCON_USBRIE |USBCRCON_USBWKUPEN | +USBCRCON_VBUSMONEN ; +writel (ctrl, musb_glue +USBCRCON );drivers/usb/musb-new/pic32.c line 154
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+ +return 0 ; +} + +/* PIC32 supports only 32bit read operation */ +void musb_read_fifo (struct musb_hw_ep *hw_ep,u16 len,u8 *dst) +{ +void __iomem *fifo = hw_ep->fifo; +u32 val; +int i;This could become:
bulk_len= len / 4;
bulk_rem= len % 4;
readsl(fifo, dst, bulk_len);
if (rem) {
dst += len & ~0x3;
tmp = readl(fifo);
copy the remaining bytes according to endianness
}
Quoted 1 line
This is 10 LoC , not 50 ;-)
May be comments in my code are not very clear :)
All the extra code is for handling (4-byte-word) alignment of destination buffer. Writing word to unaligned address will generate exception in MIPS which is not handled in U-Boot software.
Note MIPS can't handle unaligned access in h/w unless specific unaligned instructions are used.
Quoted 26 lines
Quoted 24 lines
drivers/usb/musb-new/pic32.c new file +24
-0+/* Read for 32bit-aligned destination address */ +if (likely ((0x03 & (unsigned long )dst) ==0 ) && len >=4 ) { +readsl (fifo, dst, len /4 ); + dst += len & ~0x03 ; + len &=0x03 ; + } + +/* +* Now read the remaining 1 to 3 byte or complete length if +* unaligned address. +*/ +if (len >4 ) { +for (i =0 ; i < (len /4 ); i++) { + *(u32 *)dst =musb_readl (fifo,0 ); + dst +=4 ; + } + len &=0x03 ; + } + +if (len >0 ) { + val =musb_readl (fifo,0 ); +memcpy (dst, &val, len); + } +}[...]
PATCH v3 3/4 drivers: musb-new: Add USB DRC driver for Microchip PIC32 OTG controller.
Quoted 61 lines · Purna Chandra Mandal
Quoted 50 lines · Marek Vasut
Quoted 5 lines · Purna Chandra Mandal
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 <[email protected]>Signed-off-by Purna Chandra Mandal <[email protected]>[...]
Quoted 42 lines
drivers/usb/musb-new/pic32.c new file +36
-0@@ -0,0 +1,294 @@ +/* +* Microchip PIC32 MUSB "glue layer" +* +* Copyright (C) 2015, Microchip Technology Inc. +* Cristian Birsan <[email protected]> +* Purna Chandra Mandal <[email protected]> +* +* SPDX-License-Identifier: GPL-2.0+ +* +* Based on the dsps "glue layer" code. +*/ + +#include <common.h> +#include <linux/usb/musb.h> +#include "linux-compat.h" +#include "musb_core.h" +#include "musb_uboot.h" + +DECLARE_GLOBAL_DATA_PTR ; + +#define PIC32_TX_EP_MASK 0x0f/* EP0 + 7 Tx EPs */ +#define PIC32_RX_EP_MASK 0x0e/* 7 Rx EPs */ + +#define MUSB_SOFTRST 0x7f +#define MUSB_SOFTRST_NRST BIT(0) +#define MUSB_SOFTRST_NRSTX BIT(1) + +#define USBCRCON 0 +#define USBCRCON_USBWKUPEN BIT(0)/* Enable Wakeup Interrupt */ +#define USBCRCON_USBRIE BIT(1)/* Enable Remote resume Interrupt */ +#define USBCRCON_USBIE BIT(2)/* Enable USB General interrupt */ +#define USBCRCON_SENDMONEN BIT(3)/* Enable Session End VBUS monitoring */ +#define USBCRCON_BSVALMONEN BIT(4)/* Enable B-Device VBUS monitoring */ +#define USBCRCON_ASVALMONEN BIT(5)/* Enable A-Device VBUS monitoring */ +#define USBCRCON_VBUSMONEN BIT(6)/* Enable VBUS monitoring */ +#define USBCRCON_PHYIDEN BIT(7)/* PHY ID monitoring enable */ Quoted 8 lines
drivers/usb/musb-new/pic32.c line 43
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+#define USBCRCON_USBIDVAL BIT(8)/* USB ID value */ +#define USBCRCON_USBIDOVEN BIT(9)/* USB ID override enable */ +#define USBCRCON_USBWK BIT(24)/* USB Wakeup Status */ +#define USBCRCON_USBRF BIT(25)/* USB Resume Status */ +#define USBCRCON_USBIF BIT(26)/* USB General Interrupt Status */ + +static void __iomem *musb_glue;What would happen once you make a chip with two MUSB controllers ?
Quoted 6 lines
Currently PIC32 has only one MUSB controller and only one glue reg-space. Don't know how the reg-map will be in future when PIC32 will have multiple MUSB controllers. Assuming that glue address map will be separate for each controller we can add logic to support multiple MUSB controller.
IMO, better if we don't assume something of the future and bloat logic.
If you switch this to driver model, you will need to weed out all the static global variables anyway. Better do it now.
Quoted 6 lines
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 47
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+/* pic32_musb_disable - disable HDRC */ +static void pic32_musb_disable (struct musb *musb) +{Is there no way to shut down the MUSB on the PIC32 ?
There is no way to disable MUSB.
Yet another broken chip design. Can't you put the controller into reset and gate the clock for it ?
Quoted 22 lines
Quoted 13 lines
Quoted 13 lines
drivers/usb/musb-new/pic32.c new file +13
-0+} + +/* pic32_musb_enable - enable HDRC */ +static int pic32_musb_enable (struct musb *musb) +{ +/* soft reset by NRSTx */ +musb_writeb (musb->mregs,MUSB_SOFTRST ,MUSB_SOFTRST_NRSTX ); +/* set mode */ +musb_platform_set_mode (musb, musb->board_mode); + +return 0 ; +} +Quoted 8 lines
drivers/usb/musb-new/pic32.c line 67
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+static irqreturn_t pic32_interrupt (int irq,void *hci) +{ +struct musb *musb = hci; +irqreturn_t ret =IRQ_NONE ; +u32 epintr, usbintr; + +/* Get usb core interrupts */ You mean "get" or "ack" here ?
Quoted 1 line
I meant read-and-ack. Will update comment.
Thanks
Quoted 49 lines
Quoted 6 lines
drivers/usb/musb-new/pic32.c line 72
Quoted 5 lines
drivers/usb/musb-new/pic32.c new file +5
-0+ musb->int_usb =musb_readb (musb->mregs,MUSB_INTRUSB ); +if (musb->int_usb) +musb_writeb (musb->mregs,MUSB_INTRUSB , musb->int_usb); + +/* Get endpoint interrupts */ DTTO
I meant read-and-ack.
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 75
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+ musb->int_rx =musb_readw (musb->mregs,MUSB_INTRRX ) &PIC32_RX_EP_MASK ; +if (musb->int_rx) +musb_writew (musb->mregs,MUSB_INTRRX , musb->int_rx);Same here
ack. Will update comment.
Quoted 24 lines
Quoted 24 lines
drivers/usb/musb-new/pic32.c new file +24
-0+ musb->int_tx =musb_readw (musb->mregs,MUSB_INTRTX ) &PIC32_TX_EP_MASK ; +if (musb->int_tx) +musb_writew (musb->mregs,MUSB_INTRTX , musb->int_tx); + +/* Drop spurious RX and TX if device is disconnected */ +if (musb->int_usb &MUSB_INTR_DISCONNECT ) { + musb->int_tx =0 ; + musb->int_rx =0 ; + } + +if (musb->int_tx || musb->int_rx || musb->int_usb) + ret |=musb_interrupt (musb); + +return ret; +} + +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 );Quoted 8 lines
drivers/usb/musb-new/pic32.c line 107
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+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" );So having the core in OTG mode is an error ? Why ?
Quoted 2 lines
In PIC32 we haven't tested OTG. We use the controller as dual-role not as OTG. In future we might enable that.
Then you might want to say "support for OTG is unimplemented" ?
Quoted 59 lines
Quoted 57 lines
Quoted 37 lines
drivers/usb/musb-new/pic32.c new file +37
-0+break ; +default : +dev_err (dev,"unsupported mode %d\n" , mode); +return -EINVAL ; + } + +return 0 ; +} + +static int pic32_musb_init (struct musb *musb) +{ +u32 ctrl, hwvers; +u8 power; + +/* Returns zero if not clocked */ + hwvers =musb_read_hwvers (musb->mregs); +if (!hwvers) +return -ENODEV ; + +/* Reset the musb */ + power =musb_readb (musb->mregs,MUSB_POWER ); + power = power |MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); +mdelay (100 ); + +/* Start the on-chip PHY and its PLL. */ + power = power & ~MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); + + musb->isr = pic32_interrupt; + + ctrl =USBCRCON_USBIF |USBCRCON_USBRF | +USBCRCON_USBWK |USBCRCON_USBIDOVEN | +USBCRCON_PHYIDEN |USBCRCON_USBIE | +USBCRCON_USBRIE |USBCRCON_USBWKUPEN | +USBCRCON_VBUSMONEN ; +writel (ctrl, musb_glue +USBCRCON );drivers/usb/musb-new/pic32.c line 154
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+ +return 0 ; +} + +/* PIC32 supports only 32bit read operation */ +void musb_read_fifo (struct musb_hw_ep *hw_ep,u16 len,u8 *dst) +{ +void __iomem *fifo = hw_ep->fifo; +u32 val; +int i;This could become:
bulk_len= len / 4;
bulk_rem= len % 4;
readsl(fifo, dst, bulk_len);
if (rem) {
dst += len & ~0x3;
tmp = readl(fifo);
copy the remaining bytes according to endianness
}Quoted 1 line
This is 10 LoC , not 50 ;-)
Quoted 7 lines
May be comments in my code are not very clear :)
All the extra code is for handling (4-byte-word) alignment of destination buffer. Writing word to unaligned address will generate exception in MIPS which is not handled in U-Boot software.
Note MIPS can't handle unaligned access in h/w unless specific unaligned instructions are used.
Ouch, did you ever see unaligned access here ?
Quoted 27 lines
Quoted 26 lines
Quoted 24 lines
drivers/usb/musb-new/pic32.c new file +24
-0+/* Read for 32bit-aligned destination address */ +if (likely ((0x03 & (unsigned long )dst) ==0 ) && len >=4 ) { +readsl (fifo, dst, len /4 ); + dst += len & ~0x03 ; + len &=0x03 ; + } + +/* +* Now read the remaining 1 to 3 byte or complete length if +* unaligned address. +*/ +if (len >4 ) { +for (i =0 ; i < (len /4 ); i++) { + *(u32 *)dst =musb_readl (fifo,0 ); + dst +=4 ; + } + len &=0x03 ; + } + +if (len >0 ) { + val =musb_readl (fifo,0 ); +memcpy (dst, &val, len); + } +}[...]
Signature · 2 lines
Best regards, Marek Vasut
PATCH v3 3/4 drivers: musb-new: Add USB DRC driver for Microchip PIC32 OTG controller.
Quoted 61 lines · Marek Vasut
Quoted 60 lines · Purna Chandra Mandal
Quoted 58 lines · Marek Vasut
Quoted 5 lines · Purna Chandra Mandal
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 <[email protected]>Signed-off-by Purna Chandra Mandal <[email protected]>[...]
Quoted 39 lines
drivers/usb/musb-new/pic32.c new file +33
-0@@ -0,0 +1,294 @@ +/* +* Microchip PIC32 MUSB "glue layer" +* +* Copyright (C) 2015, Microchip Technology Inc. +* Cristian Birsan <[email protected]> +* Purna Chandra Mandal <[email protected]> +* +* SPDX-License-Identifier: GPL-2.0+ +* +* Based on the dsps "glue layer" code. +*/ + +#include <common.h> +#include <linux/usb/musb.h> +#include "linux-compat.h" +#include "musb_core.h" +#include "musb_uboot.h" + +DECLARE_GLOBAL_DATA_PTR ; + +#define PIC32_TX_EP_MASK 0x0f/* EP0 + 7 Tx EPs */ +#define PIC32_RX_EP_MASK 0x0e/* 7 Rx EPs */ + +#define MUSB_SOFTRST 0x7f +#define MUSB_SOFTRST_NRST BIT(0) +#define MUSB_SOFTRST_NRSTX BIT(1) + +#define USBCRCON 0 +#define USBCRCON_USBWKUPEN BIT(0)/* Enable Wakeup Interrupt */ +#define USBCRCON_USBRIE BIT(1)/* Enable Remote resume Interrupt */ +#define USBCRCON_USBIE BIT(2)/* Enable USB General interrupt */ +#define USBCRCON_SENDMONEN BIT(3)/* Enable Session End VBUS monitoring */ +#define USBCRCON_BSVALMONEN BIT(4)/* Enable B-Device VBUS monitoring */ drivers/usb/musb-new/pic32.c line 43
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+#define USBCRCON_ASVALMONEN BIT(5)/* Enable A-Device VBUS monitoring */ +#define USBCRCON_VBUSMONEN BIT(6)/* Enable VBUS monitoring */ +#define USBCRCON_PHYIDEN BIT(7)/* PHY ID monitoring enable */ +#define USBCRCON_USBIDVAL BIT(8)/* USB ID value */ +#define USBCRCON_USBIDOVEN BIT(9)/* USB ID override enable */ +#define USBCRCON_USBWK BIT(24)/* USB Wakeup Status */ +#define USBCRCON_USBRF BIT(25)/* USB Resume Status */ +#define USBCRCON_USBIF BIT(26)/* USB General Interrupt Status */ + +static void __iomem *musb_glue;What would happen once you make a chip with two MUSB controllers ?
Quoted 8 lines
Quoted 6 lines
Currently PIC32 has only one MUSB controller and only one glue reg-space. Don't know how the reg-map will be in future when PIC32 will have multiple MUSB controllers. Assuming that glue address map will be separate for each controller we can add logic to support multiple MUSB controller.
IMO, better if we don't assume something of the future and bloat logic.
If you switch this to driver model, you will need to weed out all the static global variables anyway. Better do it now.
Thanks. Will do.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 47
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+/* pic32_musb_disable - disable HDRC */ +static void pic32_musb_disable (struct musb *musb) +{Is there no way to shut down the MUSB on the PIC32 ?
There is no way to disable MUSB.
Yet another broken chip design. Can't you put the controller into reset and gate the clock for it ?
USB clock can't be gated! USB controller clock is derived from peripheral bus clock(PBCLK5) which is shared with other modules and USB Phy clock derived from UPLL can;t be gated at all.
Quoted 70 lines
Quoted 21 lines
Quoted 13 lines
Quoted 13 lines
drivers/usb/musb-new/pic32.c new file +13
-0+} + +/* pic32_musb_enable - enable HDRC */ +static int pic32_musb_enable (struct musb *musb) +{ +/* soft reset by NRSTx */ +musb_writeb (musb->mregs,MUSB_SOFTRST ,MUSB_SOFTRST_NRSTX ); +/* set mode */ +musb_platform_set_mode (musb, musb->board_mode); + +return 0 ; +} +Quoted 8 lines
drivers/usb/musb-new/pic32.c line 67
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+static irqreturn_t pic32_interrupt (int irq,void *hci) +{ +struct musb *musb = hci; +irqreturn_t ret =IRQ_NONE ; +u32 epintr, usbintr; + +/* Get usb core interrupts */ You mean "get" or "ack" here ?
Quoted 1 line
I meant read-and-ack. Will update comment.
Thanks
Quoted 46 lines
Quoted 6 lines
drivers/usb/musb-new/pic32.c line 72
Quoted 5 lines
drivers/usb/musb-new/pic32.c new file +5
-0+ musb->int_usb =musb_readb (musb->mregs,MUSB_INTRUSB ); +if (musb->int_usb) +musb_writeb (musb->mregs,MUSB_INTRUSB , musb->int_usb); + +/* Get endpoint interrupts */ DTTO
I meant read-and-ack.
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 75
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+ musb->int_rx =musb_readw (musb->mregs,MUSB_INTRRX ) &PIC32_RX_EP_MASK ; +if (musb->int_rx) +musb_writew (musb->mregs,MUSB_INTRRX , musb->int_rx);Same here
ack. Will update comment.
Quoted 32 lines
Quoted 21 lines
drivers/usb/musb-new/pic32.c new file +21
-0+ musb->int_tx =musb_readw (musb->mregs,MUSB_INTRTX ) &PIC32_TX_EP_MASK ; +if (musb->int_tx) +musb_writew (musb->mregs,MUSB_INTRTX , musb->int_tx); + +/* Drop spurious RX and TX if device is disconnected */ +if (musb->int_usb &MUSB_INTR_DISCONNECT ) { + musb->int_tx =0 ; + musb->int_rx =0 ; + } + +if (musb->int_tx || musb->int_rx || musb->int_usb) + ret |=musb_interrupt (musb); + +return ret; +} + +static int pic32_musb_set_mode (struct musb *musb,u8 mode) +{ +struct device *dev = musb->controller; + +switch (mode) {drivers/usb/musb-new/pic32.c line 107
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+case MUSB_HOST : +clrsetbits_le32 (musb_glue +USBCRCON , +USBCRCON_USBIDVAL ,USBCRCON_USBIDOVEN ); +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" );So having the core in OTG mode is an error ? Why ?
Quoted 3 lines
Quoted 2 lines
In PIC32 we haven't tested OTG. We use the controller as dual-role not as OTG. In future we might enable that.
Then you might want to say "support for OTG is unimplemented" ?
Will update.
Quoted 58 lines
Quoted 58 lines
Quoted 58 lines
Quoted 37 lines
drivers/usb/musb-new/pic32.c new file +37
-0+break ; +default : +dev_err (dev,"unsupported mode %d\n" , mode); +return -EINVAL ; + } + +return 0 ; +} + +static int pic32_musb_init (struct musb *musb) +{ +u32 ctrl, hwvers; +u8 power; + +/* Returns zero if not clocked */ + hwvers =musb_read_hwvers (musb->mregs); +if (!hwvers) +return -ENODEV ; + +/* Reset the musb */ + power =musb_readb (musb->mregs,MUSB_POWER ); + power = power |MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); +mdelay (100 ); + +/* Start the on-chip PHY and its PLL. */ + power = power & ~MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); + + musb->isr = pic32_interrupt; + + ctrl =USBCRCON_USBIF |USBCRCON_USBRF | +USBCRCON_USBWK |USBCRCON_USBIDOVEN | +USBCRCON_PHYIDEN |USBCRCON_USBIE | +USBCRCON_USBRIE |USBCRCON_USBWKUPEN | +USBCRCON_VBUSMONEN ; +writel (ctrl, musb_glue +USBCRCON );drivers/usb/musb-new/pic32.c line 154
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+ +return 0 ; +} + +/* PIC32 supports only 32bit read operation */ +void musb_read_fifo (struct musb_hw_ep *hw_ep,u16 len,u8 *dst) +{ +void __iomem *fifo = hw_ep->fifo; +u32 val; +int i;This could become:
bulk_len= len / 4;
bulk_rem= len % 4;
readsl(fifo, dst, bulk_len);
if (rem) {
dst += len & ~0x3;
tmp = readl(fifo);
copy the remaining bytes according to endianness
}This is 10 LoC , not 50 ;-)
Quoted 8 lines
Quoted 7 lines
May be comments in my code are not very clear :)
All the extra code is for handling (4-byte-word) alignment of destination buffer. Writing word to unaligned address will generate exception in MIPS which is not handled in U-Boot software.
Note MIPS can't handle unaligned access in h/w unless specific unaligned instructions are used.Ouch, did you ever see unaligned access here ?
Not seen unaligned access here but not sure of!
Char array can be starting from any arbitrary address unless it is malloc()'d or special care is taken to make it aligned at word boundary.
I will take your logic as I'm not facing real problem.
Quoted 26 lines
Quoted 26 lines
Quoted 26 lines
Quoted 24 lines
drivers/usb/musb-new/pic32.c new file +24
-0+/* Read for 32bit-aligned destination address */ +if (likely ((0x03 & (unsigned long )dst) ==0 ) && len >=4 ) { +readsl (fifo, dst, len /4 ); + dst += len & ~0x03 ; + len &=0x03 ; + } + +/* +* Now read the remaining 1 to 3 byte or complete length if +* unaligned address. +*/ +if (len >4 ) { +for (i =0 ; i < (len /4 ); i++) { + *(u32 *)dst =musb_readl (fifo,0 ); + dst +=4 ; + } + len &=0x03 ; + } + +if (len >0 ) { + val =musb_readl (fifo,0 ); +memcpy (dst, &val, len); + } +}[...]
PATCH v3 3/4 drivers: musb-new: Add USB DRC driver for Microchip PIC32 OTG controller.
Quoted 68 lines · Purna Chandra Mandal
Quoted 66 lines · Marek Vasut
Quoted 65 lines · Purna Chandra Mandal
Quoted 50 lines · Marek Vasut
Quoted 5 lines · Purna Chandra Mandal
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 <[email protected]>Signed-off-by Purna Chandra Mandal <[email protected]>[...]
Quoted 42 lines
drivers/usb/musb-new/pic32.c new file +36
-0@@ -0,0 +1,294 @@ +/* +* Microchip PIC32 MUSB "glue layer" +* +* Copyright (C) 2015, Microchip Technology Inc. +* Cristian Birsan <[email protected]> +* Purna Chandra Mandal <[email protected]> +* +* SPDX-License-Identifier: GPL-2.0+ +* +* Based on the dsps "glue layer" code. +*/ + +#include <common.h> +#include <linux/usb/musb.h> +#include "linux-compat.h" +#include "musb_core.h" +#include "musb_uboot.h" + +DECLARE_GLOBAL_DATA_PTR ; + +#define PIC32_TX_EP_MASK 0x0f/* EP0 + 7 Tx EPs */ +#define PIC32_RX_EP_MASK 0x0e/* 7 Rx EPs */ + +#define MUSB_SOFTRST 0x7f +#define MUSB_SOFTRST_NRST BIT(0) +#define MUSB_SOFTRST_NRSTX BIT(1) + +#define USBCRCON 0 +#define USBCRCON_USBWKUPEN BIT(0)/* Enable Wakeup Interrupt */ +#define USBCRCON_USBRIE BIT(1)/* Enable Remote resume Interrupt */ +#define USBCRCON_USBIE BIT(2)/* Enable USB General interrupt */ +#define USBCRCON_SENDMONEN BIT(3)/* Enable Session End VBUS monitoring */ +#define USBCRCON_BSVALMONEN BIT(4)/* Enable B-Device VBUS monitoring */ +#define USBCRCON_ASVALMONEN BIT(5)/* Enable A-Device VBUS monitoring */ +#define USBCRCON_VBUSMONEN BIT(6)/* Enable VBUS monitoring */ +#define USBCRCON_PHYIDEN BIT(7)/* PHY ID monitoring enable */ Quoted 8 lines
drivers/usb/musb-new/pic32.c line 43
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+#define USBCRCON_USBIDVAL BIT(8)/* USB ID value */ +#define USBCRCON_USBIDOVEN BIT(9)/* USB ID override enable */ +#define USBCRCON_USBWK BIT(24)/* USB Wakeup Status */ +#define USBCRCON_USBRF BIT(25)/* USB Resume Status */ +#define USBCRCON_USBIF BIT(26)/* USB General Interrupt Status */ + +static void __iomem *musb_glue;What would happen once you make a chip with two MUSB controllers ?
Currently PIC32 has only one MUSB controller and only one glue reg-space. Don't know how the reg-map will be in future when PIC32 will have multiple MUSB controllers. Assuming that glue address map will be separate for each controller we can add logic to support multiple MUSB controller.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
IMO, better if we don't assume something of the future and bloat logic.
If you switch this to driver model, you will need to weed out all the static global variables anyway. Better do it now.
Thanks. Will do.
Thanks!
Quoted 11 lines
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 47
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+/* pic32_musb_disable - disable HDRC */ +static void pic32_musb_disable (struct musb *musb) +{Is there no way to shut down the MUSB on the PIC32 ?
There is no way to disable MUSB.
Yet another broken chip design. Can't you put the controller into reset and gate the clock for it ?
USB clock can't be gated! USB controller clock is derived from peripheral bus clock(PBCLK5) which is shared with other modules and USB Phy clock derived from UPLL can;t be gated at all.
I see, oh well, then it cannot be helped.
Quoted 70 lines
Quoted 70 lines
Quoted 21 lines
Quoted 13 lines
Quoted 13 lines
drivers/usb/musb-new/pic32.c new file +13
-0+} + +/* pic32_musb_enable - enable HDRC */ +static int pic32_musb_enable (struct musb *musb) +{ +/* soft reset by NRSTx */ +musb_writeb (musb->mregs,MUSB_SOFTRST ,MUSB_SOFTRST_NRSTX ); +/* set mode */ +musb_platform_set_mode (musb, musb->board_mode); + +return 0 ; +} +Quoted 8 lines
drivers/usb/musb-new/pic32.c line 67
Quoted 7 lines
drivers/usb/musb-new/pic32.c new file +7
-0+static irqreturn_t pic32_interrupt (int irq,void *hci) +{ +struct musb *musb = hci; +irqreturn_t ret =IRQ_NONE ; +u32 epintr, usbintr; + +/* Get usb core interrupts */ You mean "get" or "ack" here ?
Quoted 1 line
I meant read-and-ack. Will update comment.
Thanks
Quoted 46 lines
Quoted 6 lines
drivers/usb/musb-new/pic32.c line 72
Quoted 5 lines
drivers/usb/musb-new/pic32.c new file +5
-0+ musb->int_usb =musb_readb (musb->mregs,MUSB_INTRUSB ); +if (musb->int_usb) +musb_writeb (musb->mregs,MUSB_INTRUSB , musb->int_usb); + +/* Get endpoint interrupts */ DTTO
I meant read-and-ack.
Quoted 4 lines
drivers/usb/musb-new/pic32.c line 75
Quoted 3 lines
drivers/usb/musb-new/pic32.c new file +3
-0+ musb->int_rx =musb_readw (musb->mregs,MUSB_INTRRX ) &PIC32_RX_EP_MASK ; +if (musb->int_rx) +musb_writew (musb->mregs,MUSB_INTRRX , musb->int_rx);Same here
ack. Will update comment.
Quoted 32 lines
Quoted 21 lines
drivers/usb/musb-new/pic32.c new file +21
-0+ musb->int_tx =musb_readw (musb->mregs,MUSB_INTRTX ) &PIC32_TX_EP_MASK ; +if (musb->int_tx) +musb_writew (musb->mregs,MUSB_INTRTX , musb->int_tx); + +/* Drop spurious RX and TX if device is disconnected */ +if (musb->int_usb &MUSB_INTR_DISCONNECT ) { + musb->int_tx =0 ; + musb->int_rx =0 ; + } + +if (musb->int_tx || musb->int_rx || musb->int_usb) + ret |=musb_interrupt (musb); + +return ret; +} + +static int pic32_musb_set_mode (struct musb *musb,u8 mode) +{ +struct device *dev = musb->controller; + +switch (mode) {drivers/usb/musb-new/pic32.c line 107
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+case MUSB_HOST : +clrsetbits_le32 (musb_glue +USBCRCON , +USBCRCON_USBIDVAL ,USBCRCON_USBIDOVEN ); +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" );So having the core in OTG mode is an error ? Why ?
Quoted 5 lines
Quoted 3 lines
Quoted 2 lines
In PIC32 we haven't tested OTG. We use the controller as dual-role not as OTG. In future we might enable that.
Then you might want to say "support for OTG is unimplemented" ?
Will update.
Thanks
Quoted 65 lines
Quoted 65 lines
Quoted 65 lines
Quoted 57 lines
Quoted 37 lines
drivers/usb/musb-new/pic32.c new file +37
-0+break ; +default : +dev_err (dev,"unsupported mode %d\n" , mode); +return -EINVAL ; + } + +return 0 ; +} + +static int pic32_musb_init (struct musb *musb) +{ +u32 ctrl, hwvers; +u8 power; + +/* Returns zero if not clocked */ + hwvers =musb_read_hwvers (musb->mregs); +if (!hwvers) +return -ENODEV ; + +/* Reset the musb */ + power =musb_readb (musb->mregs,MUSB_POWER ); + power = power |MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); +mdelay (100 ); + +/* Start the on-chip PHY and its PLL. */ + power = power & ~MUSB_POWER_RESET ; +musb_writeb (musb->mregs,MUSB_POWER , power); + + musb->isr = pic32_interrupt; + + ctrl =USBCRCON_USBIF |USBCRCON_USBRF | +USBCRCON_USBWK |USBCRCON_USBIDOVEN | +USBCRCON_PHYIDEN |USBCRCON_USBIE | +USBCRCON_USBRIE |USBCRCON_USBWKUPEN | +USBCRCON_VBUSMONEN ; +writel (ctrl, musb_glue +USBCRCON );drivers/usb/musb-new/pic32.c line 154
Quoted 10 lines
drivers/usb/musb-new/pic32.c new file +10
-0+ +return 0 ; +} + +/* PIC32 supports only 32bit read operation */ +void musb_read_fifo (struct musb_hw_ep *hw_ep,u16 len,u8 *dst) +{ +void __iomem *fifo = hw_ep->fifo; +u32 val; +int i;This could become:
bulk_len= len / 4;
bulk_rem= len % 4;
readsl(fifo, dst, bulk_len);
if (rem) {
dst += len & ~0x3;
tmp = readl(fifo);
copy the remaining bytes according to endianness
}Quoted 1 line
This is 10 LoC , not 50 ;-)
May be comments in my code are not very clear :)
All the extra code is for handling (4-byte-word) alignment of destination buffer. Writing word to unaligned address will generate exception in MIPS which is not handled in U-Boot software.
Note MIPS can't handle unaligned access in h/w unless specific unaligned instructions are used.
Quoted 7 lines
Quoted 1 line
Ouch, did you ever see unaligned access here ?
Not seen unaligned access here but not sure of!
Char array can be starting from any arbitrary address unless it is malloc()'d or special care is taken to make it aligned at word boundary.I will take your logic as I'm not facing real problem.
The USB stack should be using only aligned buffers, so what you can try to do here is trap unaligned accesses and warn about them. Maybe factor out the unaligned access code and trigger it only if unaligned access really happens.
Quoted 27 lines
Quoted 26 lines
Quoted 26 lines
Quoted 26 lines
Quoted 24 lines
drivers/usb/musb-new/pic32.c new file +24
-0+/* Read for 32bit-aligned destination address */ +if (likely ((0x03 & (unsigned long )dst) ==0 ) && len >=4 ) { +readsl (fifo, dst, len /4 ); + dst += len & ~0x03 ; + len &=0x03 ; + } + +/* +* Now read the remaining 1 to 3 byte or complete length if +* unaligned address. +*/ +if (len >4 ) { +for (i =0 ; i < (len /4 ); i++) { + *(u32 *)dst =musb_readl (fifo,0 ); + dst +=4 ; + } + len &=0x03 ; + } + +if (len >0 ) { + val =musb_readl (fifo,0 ); +memcpy (dst, &val, len); + } +}[...]
Signature · 2 lines
Best regards, Marek Vasut
PATCH v3 4/4 board: pic32mzda: enable USB-host, USB-storage support.
In reply to Purna Chandra MandalEnable MUSB host and USB storage support for Microchip PIC32MZ[DA] Starter Kit.
---
Changes in v3:
- add arch specific reads{bwlq}, writes{bwlq} in respective arch io.h
- remove reads{bwlq}, writes{bwlq} in musb-new driver
Changes in v2:
- compilation fix in drivers/usb/musb-new/linux-compat.h seperated
- compilation fix in drivers/gadget/f_mass_storage.c seperated
arch/mips/dts/pic32mzda.dtsi | 10 ++++++++++ arch/mips/dts/pic32mzda_sk.dts | 4 ++++ configs/pic32mzdask_defconfig | 6 +++++- include/configs/pic32mzdask.h | 7 +++++++ 4 files changed, 26 insertions(+), 1 deletion(-)
arch/mips/dts/pic32mzda.dtsi +10 -0
@@ -171,4 +171,14 @@ #address-cells = <1>; #size-cells = <0>; }; + + usb: musb at 1f8e3000 { + compatible = "microchip,pic32mzda-usb"; + reg = <0x1f8e3000 0x1000>, + <0x1f884000 0x1000>; + reg-names = "mc", "control"; + interrupts = <132 IRQ_TYPE_EDGE_RISING>, + <133 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; };
arch/mips/dts/pic32mzda_sk.dts +4 -0
@@ -52,4 +52,8 @@ ethernet_phy: lan8740_phy at 0 { reg = <0>; }; +}; + +&usb { + status = "okay"; };
configs/pic32mzdask_defconfig +5 -1
@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="dask # " CONFIG_LOOPW=y CONFIG_CMD_MEMTEST=y CONFIG_CMD_MEMINFO=y +CONFIG_CMD_USB=y # CONFIG_CMD_FPGA is not set CONFIG_CMD_GPIO=y CONFIG_CMD_RARP=y
@@ -28,6 +29,9 @@ CONFIG_DM_ETH=y CONFIG_PIC32_ETH=y CONFIG_PINCTRL=y # CONFIG_PINCTRL_FULL is not set -CONFIG_SYS_VSNPRINTF=y +CONFIG_USB=y +CONFIG_DM_USB=y +CONFIG_USB_MUSB_HOST=y +CONFIG_USB_STORAGE=y CONFIG_USE_TINY_PRINTF=y CONFIG_CMD_DHRYSTONE=y
include/configs/pic32mzdask.h +7 -0
@@ -117,6 +117,12 @@#define CONFIG_GENERIC_MMC #define CONFIG_CMD_MMC +/*-------------------------------------------------- +* USB Configuration +*/ +#define CONFIG_USB_MUSB_PIO_ONLY +#define CONFIG_SYS_CACHELINE_SIZE 16 +/*----------------------------------------------------------------------- * File System Configuration */
@@ -167,6 +173,7 @@#define BOOT_TARGET_DEVICES (func) \ func(MMC, mmc, 0) \ + func(USB, usb, 0) \ func(DHCP, dhcp, na)#include <config_distro_bootcmd.h>
Signature · 1 line
1.8.3.1
PATCH v3 4/4 board: pic32mzda: enable USB-host, USB-storage support.
Am 15.03.2016 um 13:44 schrieb Purna Chandra Mandal:
Quoted 82 lines
Enable MUSB host and USB storage support for Microchip PIC32MZ[DA] Starter Kit.
Signed-off-by Purna Chandra Mandal <[email protected]>---
Changes in v3:
- add arch specific reads{bwlq}, writes{bwlq} in respective archio.h
- remove reads{bwlq}, writes{bwlq} in musb-new driverChanges in v2:
- compilation fix indrivers/usb/musb-new/linux-compat.hseperated
- compilation fix indrivers/gadget/f_mass_storage.cseperatedarch/mips/dts/pic32mzda.dtsi | 10 ++++++++++ arch/mips/dts/pic32mzda_sk.dts | 4 ++++ configs/pic32mzdask_defconfig | 6 +++++- include/configs/pic32mzdask.h | 7 +++++++ 4 files changed, 26 insertions(+), 1 deletion(-)arch/mips/dts/pic32mzda.dtsi +10
-0@@ -171,4 +171,14 @@ #address-cells = <1>; #size-cells = <0>; }; + + usb: musb at 1f8e3000 { + compatible = "microchip,pic32mzda-usb"; + reg = <0x1f8e3000 0x1000>, + <0x1f884000 0x1000>; + reg-names = "mc", "control"; + interrupts = <132 IRQ_TYPE_EDGE_RISING>, + <133 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; };arch/mips/dts/pic32mzda_sk.dts +4
-0@@ -52,4 +52,8 @@ ethernet_phy: lan8740_phy at 0 { reg = <0>; }; +}; + +&usb { + status = "okay"; };configs/pic32mzdask_defconfig +5
-1@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="dask # " CONFIG_LOOPW=y CONFIG_CMD_MEMTEST=y CONFIG_CMD_MEMINFO=y +CONFIG_CMD_USB=y # CONFIG_CMD_FPGA is not set CONFIG_CMD_GPIO=y CONFIG_CMD_RARP=y@@ -28,6 +29,9 @@ CONFIG_DM_ETH=y CONFIG_PIC32_ETH=y CONFIG_PINCTRL=y # CONFIG_PINCTRL_FULL is not set -CONFIG_SYS_VSNPRINTF=y +CONFIG_USB=y +CONFIG_DM_USB=y +CONFIG_USB_MUSB_HOST=y +CONFIG_USB_STORAGE=y CONFIG_USE_TINY_PRINTF=y CONFIG_CMD_DHRYSTONE=yinclude/configs/pic32mzdask.h +0
-0
include/configs/pic32mzdask.h line 124
Quoted 8 lines
include/configs/pic32mzdask.h +5
-0@@ -117,6 +117,12 @@#define CONFIG_GENERIC_MMC #define CONFIG_CMD_MMC +/*-------------------------------------------------- +* USB Configuration +*/ +#define CONFIG_USB_MUSB_PIO_ONLY +#define CONFIG_SYS_CACHELINE_SIZE 16
I see CONFIG_SYS_CACHELINE_SIZE is used in drivers/usb/ in various memalign() calls. Actually we have ARCH_DMA_MINALIGN for this case. At least for MIPS I want to get rid of CONFIG_SYS_CACHELINE_SIZE in the future becasue we have auto-detection for that.
If possible I'd like to see a patch which replaces CONFIG_SYS_CACHELINE_SIZE with ARCH_DMA_MINALIGN in drivers/user/. Marek what do you think?
Quoted 13 lines
include/configs/pic32mzdask.h +2
-0+/*----------------------------------------------------------------------- * File System Configuration */ @@ -167,6 +173,7 @@#define BOOT_TARGET_DEVICES (func) \ func(MMC, mmc, 0) \ + func(USB, usb, 0) \ func(DHCP, dhcp, na)#include <config_distro_bootcmd.h>
Signature · 1 line
- Daniel
PATCH v3 4/4 board: pic32mzda: enable USB-host, USB-storage support.
Quoted 96 lines · Daniel Schwierzeck
Am 15.03.2016 um 13:44 schrieb Purna Chandra Mandal:
Quoted 82 lines
Enable MUSB host and USB storage support for Microchip PIC32MZ[DA] Starter Kit.
Signed-off-by Purna Chandra Mandal <[email protected]>---
Changes in v3:
- add arch specific reads{bwlq}, writes{bwlq} in respective archio.h
- remove reads{bwlq}, writes{bwlq} in musb-new driverChanges in v2:
- compilation fix indrivers/usb/musb-new/linux-compat.hseperated
- compilation fix indrivers/gadget/f_mass_storage.cseperatedarch/mips/dts/pic32mzda.dtsi | 10 ++++++++++ arch/mips/dts/pic32mzda_sk.dts | 4 ++++ configs/pic32mzdask_defconfig | 6 +++++- include/configs/pic32mzdask.h | 7 +++++++ 4 files changed, 26 insertions(+), 1 deletion(-)arch/mips/dts/pic32mzda.dtsi +10
-0@@ -171,4 +171,14 @@ #address-cells = <1>; #size-cells = <0>; }; + + usb: musb at 1f8e3000 { + compatible = "microchip,pic32mzda-usb"; + reg = <0x1f8e3000 0x1000>, + <0x1f884000 0x1000>; + reg-names = "mc", "control"; + interrupts = <132 IRQ_TYPE_EDGE_RISING>, + <133 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; };arch/mips/dts/pic32mzda_sk.dts +4
-0@@ -52,4 +52,8 @@ ethernet_phy: lan8740_phy at 0 { reg = <0>; }; +}; + +&usb { + status = "okay"; };configs/pic32mzdask_defconfig +5
-1@@ -12,6 +12,7 @@ CONFIG_SYS_PROMPT="dask # " CONFIG_LOOPW=y CONFIG_CMD_MEMTEST=y CONFIG_CMD_MEMINFO=y +CONFIG_CMD_USB=y # CONFIG_CMD_FPGA is not set CONFIG_CMD_GPIO=y CONFIG_CMD_RARP=y@@ -28,6 +29,9 @@ CONFIG_DM_ETH=y CONFIG_PIC32_ETH=y CONFIG_PINCTRL=y # CONFIG_PINCTRL_FULL is not set -CONFIG_SYS_VSNPRINTF=y +CONFIG_USB=y +CONFIG_DM_USB=y +CONFIG_USB_MUSB_HOST=y +CONFIG_USB_STORAGE=y CONFIG_USE_TINY_PRINTF=y CONFIG_CMD_DHRYSTONE=yinclude/configs/pic32mzdask.h +0
-0include/configs/pic32mzdask.h line 124
Quoted 8 lines
include/configs/pic32mzdask.h +5
-0@@ -117,6 +117,12 @@#define CONFIG_GENERIC_MMC #define CONFIG_CMD_MMC +/*-------------------------------------------------- +* USB Configuration +*/ +#define CONFIG_USB_MUSB_PIO_ONLY +#define CONFIG_SYS_CACHELINE_SIZE 16I see
CONFIG_SYS_CACHELINE_SIZEis used indrivers/usb/in variousmemalign()calls. Actually we haveARCH_DMA_MINALIGNfor this case. At least for MIPS I want to get rid ofCONFIG_SYS_CACHELINE_SIZEin the future becasue we have auto-detection for that.
Quoted 1 line
If possible I'd like to see a patch which replaces
CONFIG_SYS_CACHELINE_SIZEwithARCH_DMA_MINALIGNindrivers/user/. Marek what do you think?
I think that makes sense.
Quoted 14 lines
Quoted 13 lines
include/configs/pic32mzdask.h +2
-0+/*----------------------------------------------------------------------- * File System Configuration */ @@ -167,6 +173,7 @@#define BOOT_TARGET_DEVICES (func) \ func(MMC, mmc, 0) \ + func(USB, usb, 0) \ func(DHCP, dhcp, na)#include <config_distro_bootcmd.h>
Signature · 2 lines
Best regards, Marek Vasut