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

["Loh, Tien Hock" <tien.hock.loh@intel.com>]

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