Re: [edk2] [PATCH v1 1/2] EmbeddedPkg: add DwUsb protocol

2018-10-21 Thread Haojian Zhuang
On Thu, 4 Oct 2018 at 22:49, Leif Lindholm  wrote:
>
> On Mon, Aug 20, 2018 at 06:31:24PM +0800, Haojian Zhuang wrote:
> > The protocol defines the callbacks that could be implemented in
> > platform driver. DwUsb device driver needs these callbacks to
> > implement USB device functionality.
> >
> > Cc: Leif Lindholm 
> > Cc: Ard Biesheuvel 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Haojian Zhuang 
> > ---
> >  EmbeddedPkg/Include/Protocol/DwUsb.h | 81 
> >  1 file changed, 81 insertions(+)
> >
> > diff --git a/EmbeddedPkg/Include/Protocol/DwUsb.h 
> > b/EmbeddedPkg/Include/Protocol/DwUsb.h
> > new file mode 100644
> > index ..b9fb776f9258
> > --- /dev/null
> > +++ b/EmbeddedPkg/Include/Protocol/DwUsb.h
> > @@ -0,0 +1,81 @@
> > +/** @file
> > +
> > +  Copyright (c) 2018, Linaro. All rights reserved.
> > +
> > +  This program and the accompanying materials are licensed and made 
> > available
> > +  under the terms and conditions of the BSD License which accompanies this
> > +  distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> > IMPLIED.
> > +
> > +**/
> > +
> > +#ifndef __DW_USB_H__
> > +#define __DW_USB_H__
>
> Can you expand all instances of public interfaces to DESIGNWARE?
>

OK

> > +
> > +//
> > +// Protocol GUID
> > +//
> > +#define DW_USB_PROTOCOL_GUID { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 
> > 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }}
> > +
> > +//
> > +// Protocol interface structure
> > +//
> > +typedef struct _DW_USB_PROTOCOL  DW_USB_PROTOCOL;
> > +
> > +#define USB_HOST_MODE 0
> > +#define USB_DEVICE_MODE   1
> > +#define USB_CABLE_NOT_ATTACHED2
>
> And add a DESIGNWARE_ prefix to these?
>
OK

> > +
> > +#define LANG_LENGTH   8
> > +#define MANU_FACTURER_STRING_LENGTH   32
>
> MANUFACTURER.
>
OK

> Why the hardcoded string lengths?
> Are they mandated somewhere?
>
> > +#define PRODUCT_STRING_LENGTH 32
> > +#define SERIAL_STRING_LENGTH  17
> > +
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *DW_USB_GET_LANG) (
> > +  OUT CHAR16   *Lang,
> > +  OUT UINT8*Length
> > +  );
> > +
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *DW_USB_GET_MANU_FACTURER) (
> > +  OUT CHAR16   *ManuFacturer,
>
> ManuFacturer ->
> Manufacturer.
>
OK

> > +  OUT UINT8*Length
> > +  );
> > +
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *DW_USB_GET_PRODUCT) (
> > +  OUT CHAR16   *Product,
> > +  OUT UINT8*Length
> > +  );
> > +
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *DW_USB_GET_SERIAL_NO) (
>
> SERIAL_NUMBER
>
> > +  OUT CHAR16   *SerialNo,
>
> SerialNumber.
>
OK

> > +  OUT UINT8*Length
> > +  );
> > +
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *DW_USB_PHY_INIT) (
> > +  IN UINT8 Mode
> > +  );
> > +
> > +struct _DW_USB_PROTOCOL {
> > +  DW_USB_GET_LANG  GetLang;
> > +  DW_USB_GET_MANU_FACTURER GetManuFacturer;
>
> GetManuFacturer ->
> GetManufacturer.
>
OK

> > +  DW_USB_GET_PRODUCT   GetProduct;
> > +  DW_USB_GET_SERIAL_NO GetSerialNo;
> > +  DW_USB_PHY_INIT  PhyInit;
> > +};
> > +
> > +extern EFI_GUID gDwUsbProtocolGuid;
>
> gDesignWareUsbProtocolGuid.
>
OK

Best Regards
Haojian
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v1 1/2] EmbeddedPkg: add DwUsb protocol

2018-10-04 Thread Leif Lindholm
On Mon, Aug 20, 2018 at 06:31:24PM +0800, Haojian Zhuang wrote:
> The protocol defines the callbacks that could be implemented in
> platform driver. DwUsb device driver needs these callbacks to
> implement USB device functionality.
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  EmbeddedPkg/Include/Protocol/DwUsb.h | 81 
>  1 file changed, 81 insertions(+)
> 
> diff --git a/EmbeddedPkg/Include/Protocol/DwUsb.h 
> b/EmbeddedPkg/Include/Protocol/DwUsb.h
> new file mode 100644
> index ..b9fb776f9258
> --- /dev/null
> +++ b/EmbeddedPkg/Include/Protocol/DwUsb.h
> @@ -0,0 +1,81 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro. All rights reserved.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#ifndef __DW_USB_H__
> +#define __DW_USB_H__

Can you expand all instances of public interfaces to DESIGNWARE?

> +
> +//
> +// Protocol GUID
> +//
> +#define DW_USB_PROTOCOL_GUID { 0x109fa264, 0x7811, 0x4862, { 0xa9, 0x73, 
> 0x4a, 0xb2, 0xef, 0x2e, 0xe2, 0xff }}
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _DW_USB_PROTOCOL  DW_USB_PROTOCOL;
> +
> +#define USB_HOST_MODE 0
> +#define USB_DEVICE_MODE   1
> +#define USB_CABLE_NOT_ATTACHED2

And add a DESIGNWARE_ prefix to these?

> +
> +#define LANG_LENGTH   8
> +#define MANU_FACTURER_STRING_LENGTH   32

MANUFACTURER.

Why the hardcoded string lengths?
Are they mandated somewhere?

> +#define PRODUCT_STRING_LENGTH 32
> +#define SERIAL_STRING_LENGTH  17
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_LANG) (
> +  OUT CHAR16   *Lang,
> +  OUT UINT8*Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_MANU_FACTURER) (
> +  OUT CHAR16   *ManuFacturer,

ManuFacturer ->
Manufacturer.

> +  OUT UINT8*Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_PRODUCT) (
> +  OUT CHAR16   *Product,
> +  OUT UINT8*Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_GET_SERIAL_NO) (

SERIAL_NUMBER

> +  OUT CHAR16   *SerialNo,

SerialNumber.

> +  OUT UINT8*Length
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *DW_USB_PHY_INIT) (
> +  IN UINT8 Mode
> +  );
> +
> +struct _DW_USB_PROTOCOL {
> +  DW_USB_GET_LANG  GetLang;
> +  DW_USB_GET_MANU_FACTURER GetManuFacturer;

GetManuFacturer ->
GetManufacturer.

> +  DW_USB_GET_PRODUCT   GetProduct;
> +  DW_USB_GET_SERIAL_NO GetSerialNo;
> +  DW_USB_PHY_INIT  PhyInit;
> +};
> +
> +extern EFI_GUID gDwUsbProtocolGuid;

gDesignWareUsbProtocolGuid.

/
Leif

> +
> +#endif /* __DW_USB_H__ */
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel