[U-Boot] [PATCH v2 3/4] usb: gadget: bcm_udc_otg files

[Steve Rae]

Hi, Lukasz

On 15-07-07 06:33 AM, Lukasz Majewski wrote:
> Hi Steve,
>
>> From: "JD (Jiandong) Zheng" <jdzheng at broadcom.com>
>>
>> Add the required files for the Broadcom UDC OTG interface.
>>
>> Signed-off-by: Steve Rae <srae at broadcom.com>
>> ---
>>
>> Changes in v2: None
>>
>>   arch/arm/include/asm/arch-bcm281xx/sysmap.h |  7 ++++
>>   drivers/usb/gadget/bcm_udc_otg.h            | 17 ++++++++++
>>   drivers/usb/gadget/bcm_udc_otg_phy.c        | 50
>> +++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+)
>>   create mode 100644 drivers/usb/gadget/bcm_udc_otg.h
>>   create mode 100644 drivers/usb/gadget/bcm_udc_otg_phy.c
>>
>> diff --git a/arch/arm/include/asm/arch-bcm281xx/sysmap.h
>> b/arch/arm/include/asm/arch-bcm281xx/sysmap.h index 93ebf34..dbcc88c
>> 100644 --- a/arch/arm/include/asm/arch-bcm281xx/sysmap.h
>> +++ b/arch/arm/include/asm/arch-bcm281xx/sysmap.h
>> @@ -27,4 +27,11 @@
>>   #define SECWD2_BASE_ADDR	0x35002f40
>>   #define TIMER_BASE_ADDR		0x3e00d000
>>
>> +#define HSOTG_DCTL_OFFSET
>> 0x00000804 +#define
>> HSOTG_DCTL_SFTDISCON_MASK				0x00000002 +
>> +#define HSOTG_CTRL_PHY_P1CTL_OFFSET
>> 0x00000008 +#define
>> HSOTG_CTRL_PHY_P1CTL_SOFT_RESET_MASK
>> 0x00000002 +#define
>> HSOTG_CTRL_PHY_P1CTL_NON_DRIVING_MASK		0x00000001 +
>> #endif diff --git a/drivers/usb/gadget/bcm_udc_otg.h
>> b/drivers/usb/gadget/bcm_udc_otg.h new file mode 100644
>> index 0000000..81a1fc0
>> --- /dev/null
>> +++ b/drivers/usb/gadget/bcm_udc_otg.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright 2015 Broadcom Corporation.
>
> --> Please also add the name of the code author.
this is not standard practice at Broadcom....
and it seems that at least five (5) people have contributed to the code 
in this commit....
What would you suggest? The most recent author, or ???

>
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#ifndef __BCM_UDC_OTG_H
>> +#define __BCM_UDC_OTG_H
>> +
>> +#include <linux/types.h>
>> +
>> +#define wfld_set(addr, fld_val, fld_mask) \
>> +		(writel(((readl(addr) & ~(fld_mask)) | (fld_val)),
>> (addr))) +#define wfld_clear(addr, fld_mask) \
>> +		(writel((readl(addr) & ~(fld_mask)), (addr)))
>
> Maybe we could replace those preprocessor macros with static inline
> functions?
Hmmm - since writel() and readl() are also macros, it makes it difficult 
to make these into functions....
I guess we could use "uintprt_t" and "uint32_t"
Would you agree?
>
>> +
>> +#endif
>> diff --git a/drivers/usb/gadget/bcm_udc_otg_phy.c
>> b/drivers/usb/gadget/bcm_udc_otg_phy.c new file mode 100644
>> index 0000000..1aa9f91
>> --- /dev/null
>> +++ b/drivers/usb/gadget/bcm_udc_otg_phy.c
>> @@ -0,0 +1,50 @@
>> +/*
>> + * Copyright 2015 Broadcom Corporation.
>
> ---> The same as above.
>
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#include <config.h>
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/sysmap.h>
>> +
>> +#include <usb/s3c_udc.h>
>> +#include "bcm_udc_otg.h"
>> +
>> +void otg_phy_init(struct s3c_udc *dev)
>> +{
>> +	/* set Phy to driving mode */
>> +	wfld_clear(HSOTG_CTRL_BASE_ADDR +
>> HSOTG_CTRL_PHY_P1CTL_OFFSET,
>> +		   HSOTG_CTRL_PHY_P1CTL_NON_DRIVING_MASK);
>> +
>> +	udelay(100);
>> +
>> +	/* clear Soft Disconnect */
>> +	wfld_clear(HSOTG_BASE_ADDR + HSOTG_DCTL_OFFSET,
>> +		   HSOTG_DCTL_SFTDISCON_MASK);
>> +
>> +	/* invoke Reset (active low) */
>> +	wfld_clear(HSOTG_CTRL_BASE_ADDR +
>> HSOTG_CTRL_PHY_P1CTL_OFFSET,
>> +		   HSOTG_CTRL_PHY_P1CTL_SOFT_RESET_MASK);
>> +
>> +	udelay(10000);
> 	^^^^^^^^^^^^^^ -- would it be possible to add some comment
> 	regarding for what we are waiting here?
OK

>> +
>> +	/* release Reset */
>> +	wfld_set(HSOTG_CTRL_BASE_ADDR + HSOTG_CTRL_PHY_P1CTL_OFFSET,
>> +		 HSOTG_CTRL_PHY_P1CTL_SOFT_RESET_MASK,
>> +		 HSOTG_CTRL_PHY_P1CTL_SOFT_RESET_MASK);
>> +}
>> +
>> +void otg_phy_off(struct s3c_udc *dev)
>> +{
>> +	/* Soft Disconnect */
>> +	wfld_set(HSOTG_BASE_ADDR + HSOTG_DCTL_OFFSET,
>> +		 HSOTG_DCTL_SFTDISCON_MASK,
>> +		 HSOTG_DCTL_SFTDISCON_MASK);
>> +
>> +	/* set Phy to non-driving (reset) mode */
>> +	wfld_set(HSOTG_CTRL_BASE_ADDR + HSOTG_CTRL_PHY_P1CTL_OFFSET,
>> +		 HSOTG_CTRL_PHY_P1CTL_NON_DRIVING_MASK,
>> +		 HSOTG_CTRL_PHY_P1CTL_NON_DRIVING_MASK);
>> +}
>
>
>
Thanks, Steve