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

[Lukasz Majewski]

Hi Steve,

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

I will not push you to add names if it is not mandatory in your company.

I just wanted to stress here that putting ones name is some kind of
reward when working with community.
 

> 
> >
> >> + *
> >> + * 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 think that it would be doable to embed writel() and readl() in a
static inline function.

> I guess we could use "uintprt_t" and "uint32_t"
> Would you agree?

We can give it a try.

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



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group