[PATCH 1/1] MdeModulePkg: BaseSerialPortLib16550: Update Mmio32 to UNI

[PATCH 1/1] MdeModulePkg: BaseSerialPortLib16550: Update Mmio32 to UNI

[Loh, Tien Hock]

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

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>
---
 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_PROMPT  #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"
+                                                                                  "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>"
+
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialUseHardwareFlowControl_PROMPT  #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

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

[Wu, Hao A]

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

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

[Loh, Tien Hock]

> -----Original Message-----
> From: Wu, Hao A
> Sent: Thursday, May 9, 2019 2:37 PM
> To: devel@edk2.groups.io; Loh, Tien Hock <tien.hock.loh@intel.com>;
> thloh85@gmail.com
> Cc: tien.hock.loh <thloh@ppglcf0027.png.intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: RE: [edk2-devel] [PATCH 1/1] MdeModulePkg:
> BaseSerialPortLib16550: Update Mmio32 to UNI
> 
> > -----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.
Yeah, must be a broken config. I'll fix this.

> 
> 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
OK noted. Will generate a patch v2. 
> 
> >
> > 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?
Yeah, I must've broken my config. I'll fix and send a new patch. 

> 
> > ---
> >  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_PR
> O
> > MPT  #language en-US "Serial port registers access width"
> > +
> > +#string
> >
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialRegisterAccessWidth_HE
> LP
> > #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_HE
> LP.
> 
> > +
> > + "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_PcdSerialUseHardwareFlowContr
> ol_P
> > ROMPT  #language en-US "Enable serial port hardware flow control"
> >
> >  #string
> >
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialUseHardwareFlowContr
> ol_
> > 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
> >
> >
> >