From mboxrd@z Thu Jan  1 00:00:00 1970
From: Purna Chandra Mandal <purna.mandal@microchip.com>
Date: Tue, 9 Feb 2016 18:56:37 +0530
Subject: [U-Boot] [PATCH v1 1/2] drivers: musb-new: Add USB DRC driver
 for Microchip PIC32 OTG controller.
In-Reply-To: <201602091418.58398.marex@denx.de>
References: <1455019370-12197-1-git-send-email-purna.mandal@microchip.com>
 <201602091418.58398.marex@denx.de>
Message-ID: <56B9E90D.7060509@microchip.com>
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 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.

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

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

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