From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Tue, 9 Feb 2016 14:18:58 +0100
Subject: [U-Boot] [PATCH v1 1/2] drivers: musb-new: Add USB DRC driver
	for Microchip PIC32 OTG controller.
In-Reply-To: <1455019370-12197-1-git-send-email-purna.mandal@microchip.com>
References: <1455019370-12197-1-git-send-email-purna.mandal@microchip.com>
Message-ID: <201602091418.58398.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.

>  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?

>  #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.

> +		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.