Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: BaseSerialPortLib16550: Update Mmio32 to UNI

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Loh,
> Tien Hock
> Sent: Tuesday, May 07, 2019 7:08 PM
> To: devel@edk2.groups.io; thloh85@gmail.com
> Cc: tien.hock.loh; Loh, Tien Hock; Wang, Jian J; Wu, Hao A; Zhu, Yonghong
> Subject: [edk2-devel] [PATCH 1/1] MdeModulePkg: BaseSerialPortLib16550:
> Update Mmio32 to UNI
> 

Hello Tien Hock,

> From: "tien.hock.loh" <thloh@ppglcf0027.png.intel.com>

Is the above information automatically generated when formatting the patch?
It seems a bit weird to me.

Also, could you help to add the Bugzilla link reference information in the
commit message? Like:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1771

> 
> Some busses doesn't allow 8 bit MMIO read/write, this adds support for
> 32 bits read/write. This patch adds the UNI information on the new
> Pcd introduced - PcdSerialRegisterAccessWidth
> 
> Signed-off-by: "Tien Hock, Loh" <tien.hock.loh@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: "Zhu, YongHong" <yonghong.zhu@intel.com>
> 
> Signed-off-by: tien.hock.loh <thloh@ppglcf0027.png.intel.com>

There are 2 'Signed-off' tags here, could you help to cleanup to drop the
unneeded one?

> ---
>  MdeModulePkg/MdeModulePkg.uni | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.uni
> b/MdeModulePkg/MdeModulePkg.uni
> index cf2aefa7ad..654dfeadb0 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -160,6 +160,12 @@
>                                                                                    "TRUE  - 16550 serial port
> registers are in MMIO space.<BR>\n"
>                                                                                    "FALSE - 16550 serial port
> registers are in I/O space.<BR>"
> 
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialRegisterAccessWidth_PRO
> MPT  #language en-US "Serial port registers access width"
> +
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialRegisterAccessWidth_HELP
> #language en-US "Sets the 16550 serial port registers access width in MMIO
> space. Default is 8 bits access.<BR><BR>\n"

IMO, you can directly use the text:
'Indicates the access width for 16550 serial port registers.'
in MdeModulePkg.dec here for
STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialRegisterAccessWidth_HELP.

> +                                                                                  "8  - 16550 serial port
> MMIO register access are in 8 bits mode.<BR>\n"
> +                                                                                  "32 - 16550 serial port
> MMIO registers acess are in 32 bits mode..<BR>"

acess -> access
Please also help to remove the extra '.' for the above line.

Best Regards,
Hao Wu

> +
>  #string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialUseHardwareFlowControl_P
> ROMPT  #language en-US "Enable serial port hardware flow control"
> 
>  #string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialUseHardwareFlowControl_
> HELP  #language en-US "Indicates if the 16550 serial port hardware flow
> control will be enabled. Default is FALSE.<BR><BR>\n"
> --
> 2.13.0
> 
> 
>