| Message ID | 1436288191-16773-2-git-send-email-heyi.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 7, 2015 at 11:47 AM, Laszlo Ersek <lersek@redhat.com> wrote: > comments below > > On 07/07/15 18:56, Heyi Guo wrote: >> Change default terminal type to be VT100, to make it consistent with >> default ConIn/ConOut device path. >> >> I can't say this is a bug, as we can pass the whole console device >> path to ConnectController, and TerminalDxe driver will pick up the >> terminal in the remaining device path. However, in rare circumstances, >> the console devices may be disconnected with the driver, and they will >> be ignored by ConPlatformDxe until we pass the device path explicitly >> just as BDS. >> >> Changing default terminal type to be the same with console device >> path could help serial terminal be reconnected with normal connect >> controller operation. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >> --- >> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >> index 0671469..0f104f3 100644 >> --- a/ArmVirtPkg/ArmVirtQemu.dsc >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >> @@ -140,6 +140,8 @@ >> # >> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >> + ## Terminal Type - VT100, consistent with ConOut/ConIn Device Path. >> + gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1 >> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 >> >> # >> > > (1) When you mention that this brings the default terminal type in > accordance with the PCDs visible just above it: those PCDs are only used > by the ARM BDS. When ArmVirtQemu.dsc is built with -D INTEL_BDS, those > PCDs are not used. > > The device path used for all of the consoles in case of -D INTEL_BDS is > hardcoded in the source. That device path happens to spceify VT100 too, > but the commit message doesn't reflect -D INTEL_BDS. > > (2) This patch should be rebased on Roy's recent patch set > > [PATCH V2 0/4] Add TtyTerm terminal type > http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16769 > > In particular, *if* we're doing this, then PcdDefaultTerminalType should > be kept in sync with the devpath that is in effect with > > -D TTY_TERMINAL -D INTEL_BDS > > (please refer to patch #4 in Roy's series). Meaning, the > PcdDefaultTerminalType setting will have to depend on TTY_TERMINAL too. > > So, ultimately, I think you need three separate cases: > > (a) !INTEL_BDS --> VT100TYPE (value 1) > (b) INTEL_BDS && !TTY_TERMINAL --> VT100TYPE (value 1) > (c) INTEL_BDS && TTY_TERMINAL --> TTYTERMTYPE (value 4) > > Again, this should be implemented on top of Roy's patch series. > > Thanks > Laszlo Hi Heyi, Also, there is currently no support for text->guid conversion for the tty terminal type, so you will need to use the "VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)" path element in place of "VenVt100()" Roy ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/
On Tue, Jul 7, 2015 at 1:28 PM, Laszlo Ersek <lersek@redhat.com> wrote: > On 07/07/15 22:16, Roy Franz wrote: >> On Tue, Jul 7, 2015 at 11:47 AM, Laszlo Ersek <lersek@redhat.com> wrote: >>> comments below >>> >>> On 07/07/15 18:56, Heyi Guo wrote: >>>> Change default terminal type to be VT100, to make it consistent with >>>> default ConIn/ConOut device path. >>>> >>>> I can't say this is a bug, as we can pass the whole console device >>>> path to ConnectController, and TerminalDxe driver will pick up the >>>> terminal in the remaining device path. However, in rare circumstances, >>>> the console devices may be disconnected with the driver, and they will >>>> be ignored by ConPlatformDxe until we pass the device path explicitly >>>> just as BDS. >>>> >>>> Changing default terminal type to be the same with console device >>>> path could help serial terminal be reconnected with normal connect >>>> controller operation. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>> --- >>>> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >>>> index 0671469..0f104f3 100644 >>>> --- a/ArmVirtPkg/ArmVirtQemu.dsc >>>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >>>> @@ -140,6 +140,8 @@ >>>> # >>>> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >>>> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >>>> + ## Terminal Type - VT100, consistent with ConOut/ConIn Device Path. >>>> + gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1 >>>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 >>>> >>>> # >>>> >>> >>> (1) When you mention that this brings the default terminal type in >>> accordance with the PCDs visible just above it: those PCDs are only used >>> by the ARM BDS. When ArmVirtQemu.dsc is built with -D INTEL_BDS, those >>> PCDs are not used. >>> >>> The device path used for all of the consoles in case of -D INTEL_BDS is >>> hardcoded in the source. That device path happens to spceify VT100 too, >>> but the commit message doesn't reflect -D INTEL_BDS. >>> >>> (2) This patch should be rebased on Roy's recent patch set >>> >>> [PATCH V2 0/4] Add TtyTerm terminal type >>> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16769 >>> >>> In particular, *if* we're doing this, then PcdDefaultTerminalType should >>> be kept in sync with the devpath that is in effect with >>> >>> -D TTY_TERMINAL -D INTEL_BDS >>> >>> (please refer to patch #4 in Roy's series). Meaning, the >>> PcdDefaultTerminalType setting will have to depend on TTY_TERMINAL too. >>> >>> So, ultimately, I think you need three separate cases: >>> >>> (a) !INTEL_BDS --> VT100TYPE (value 1) >>> (b) INTEL_BDS && !TTY_TERMINAL --> VT100TYPE (value 1) >>> (c) INTEL_BDS && TTY_TERMINAL --> TTYTERMTYPE (value 4) >>> >>> Again, this should be implemented on top of Roy's patch series. >>> >>> Thanks >>> Laszlo >> >> Hi Heyi, >> >> Also, there is currently no support for text->guid conversion for >> the tty terminal type, so you will need to use the >> "VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)" >> path element in place of "VenVt100()" > > Sorry, I don't understand; in which case is a VenMsg() textual > representation necessary? Well, the lines: gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" are still in the DSC file, and as I understand the change, the gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType will only be used if there _isn't_ a terminal type found in the device path. (For ARM BDS) > > All I had in mind was that > > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1 > > vs. > > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4 > > should be added several times, in accordance with the above conditions. > As far as I understand, the textual representation of any device path is > irrelevant here. > Yes, for the Intel BDS the text is irrelevant, and the change you describe is needed as well. It's for the ARM BDS that I think we still need the text device path change. > Thanks > Laszlo ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/
On Tue, Jul 7, 2015 at 1:57 PM, Laszlo Ersek <lersek@redhat.com> wrote: > On 07/07/15 22:37, Roy Franz wrote: >> On Tue, Jul 7, 2015 at 1:28 PM, Laszlo Ersek <lersek@redhat.com> wrote: >>> On 07/07/15 22:16, Roy Franz wrote: >>>> On Tue, Jul 7, 2015 at 11:47 AM, Laszlo Ersek <lersek@redhat.com> wrote: >>>>> comments below >>>>> >>>>> On 07/07/15 18:56, Heyi Guo wrote: >>>>>> Change default terminal type to be VT100, to make it consistent with >>>>>> default ConIn/ConOut device path. >>>>>> >>>>>> I can't say this is a bug, as we can pass the whole console device >>>>>> path to ConnectController, and TerminalDxe driver will pick up the >>>>>> terminal in the remaining device path. However, in rare circumstances, >>>>>> the console devices may be disconnected with the driver, and they will >>>>>> be ignored by ConPlatformDxe until we pass the device path explicitly >>>>>> just as BDS. >>>>>> >>>>>> Changing default terminal type to be the same with console device >>>>>> path could help serial terminal be reconnected with normal connect >>>>>> controller operation. >>>>>> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>>>> --- >>>>>> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ >>>>>> 1 file changed, 2 insertions(+) >>>>>> >>>>>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >>>>>> index 0671469..0f104f3 100644 >>>>>> --- a/ArmVirtPkg/ArmVirtQemu.dsc >>>>>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >>>>>> @@ -140,6 +140,8 @@ >>>>>> # >>>>>> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >>>>>> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >>>>>> + ## Terminal Type - VT100, consistent with ConOut/ConIn Device Path. >>>>>> + gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1 >>>>>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 >>>>>> >>>>>> # >>>>>> >>>>> >>>>> (1) When you mention that this brings the default terminal type in >>>>> accordance with the PCDs visible just above it: those PCDs are only used >>>>> by the ARM BDS. When ArmVirtQemu.dsc is built with -D INTEL_BDS, those >>>>> PCDs are not used. >>>>> >>>>> The device path used for all of the consoles in case of -D INTEL_BDS is >>>>> hardcoded in the source. That device path happens to spceify VT100 too, >>>>> but the commit message doesn't reflect -D INTEL_BDS. >>>>> >>>>> (2) This patch should be rebased on Roy's recent patch set >>>>> >>>>> [PATCH V2 0/4] Add TtyTerm terminal type >>>>> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16769 >>>>> >>>>> In particular, *if* we're doing this, then PcdDefaultTerminalType should >>>>> be kept in sync with the devpath that is in effect with >>>>> >>>>> -D TTY_TERMINAL -D INTEL_BDS >>>>> >>>>> (please refer to patch #4 in Roy's series). Meaning, the >>>>> PcdDefaultTerminalType setting will have to depend on TTY_TERMINAL too. >>>>> >>>>> So, ultimately, I think you need three separate cases: >>>>> >>>>> (a) !INTEL_BDS --> VT100TYPE (value 1) >>>>> (b) INTEL_BDS && !TTY_TERMINAL --> VT100TYPE (value 1) >>>>> (c) INTEL_BDS && TTY_TERMINAL --> TTYTERMTYPE (value 4) >>>>> >>>>> Again, this should be implemented on top of Roy's patch series. >>>>> >>>>> Thanks >>>>> Laszlo >>>> >>>> Hi Heyi, >>>> >>>> Also, there is currently no support for text->guid conversion for >>>> the tty terminal type, so you will need to use the >>>> "VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)" >>>> path element in place of "VenVt100()" >>> >>> Sorry, I don't understand; in which case is a VenMsg() textual >>> representation necessary? >> >> Well, the lines: >> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >> >> are still in the DSC file, and as I understand the change, the >> gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType will only be used if >> there _isn't_ a terminal type found in the device path. (For ARM BDS) >> >>> >>> All I had in mind was that >>> >>> gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1 >>> >>> vs. >>> >>> gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4 >>> >>> should be added several times, in accordance with the above conditions. >>> As far as I understand, the textual representation of any device path is >>> irrelevant here. >>> >> >> Yes, for the Intel BDS the text is irrelevant, and the change you >> describe is needed as well. It's for the ARM BDS that I think >> we still need the text device path change. > > What for? For the (!INTEL_BDS && TTY_TERMINAL) case? > > I didn't consider that case at all. I've been under the impression that > your TTY_TERMINAL series was meant for the INTEL_BDS case only. > > If you are targeting the ARM BDS too with that terminal, then your patch > set should be extended too. Yes, my patch should be extended to cover the ARM BDS case as well. Backspace is broken there too :) (Again, AIUI.) Currently TTY_TERMINAL only > affects gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer, which is used > solely by ArmVirtPkg's Intel Platform BDS Lib instance. In my > understanding, Heyi's patch tries to set PcdDefaultTerminalType only > within preexistent branches (it doesn't introduce new branches). > > Sorry, I'm a bit confused :) > > Thanks > Laszlo ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/
Hi Laszlo, Thanks for your comments. I'll send another version when Roy's patches are committed. On 07/08/2015 02:47 AM, Laszlo Ersek wrote: > comments below > > On 07/07/15 18:56, Heyi Guo wrote: >> Change default terminal type to be VT100, to make it consistent with >> default ConIn/ConOut device path. >> >> I can't say this is a bug, as we can pass the whole console device >> path to ConnectController, and TerminalDxe driver will pick up the >> terminal in the remaining device path. However, in rare circumstances, >> the console devices may be disconnected with the driver, and they will >> be ignored by ConPlatformDxe until we pass the device path explicitly >> just as BDS. >> >> Changing default terminal type to be the same with console device >> path could help serial terminal be reconnected with normal connect >> controller operation. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >> --- >> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >> index 0671469..0f104f3 100644 >> --- a/ArmVirtPkg/ArmVirtQemu.dsc >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >> @@ -140,6 +140,8 @@ >> # >> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" >> + ## Terminal Type - VT100, consistent with ConOut/ConIn Device Path. >> + gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1 >> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 >> >> # >> > (1) When you mention that this brings the default terminal type in > accordance with the PCDs visible just above it: those PCDs are only used > by the ARM BDS. When ArmVirtQemu.dsc is built with -D INTEL_BDS, those > PCDs are not used. > > The device path used for all of the consoles in case of -D INTEL_BDS is > hardcoded in the source. That device path happens to spceify VT100 too, > but the commit message doesn't reflect -D INTEL_BDS. > > (2) This patch should be rebased on Roy's recent patch set > > [PATCH V2 0/4] Add TtyTerm terminal type > http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16769 > > In particular, *if* we're doing this, then PcdDefaultTerminalType should > be kept in sync with the devpath that is in effect with > > -D TTY_TERMINAL -D INTEL_BDS > > (please refer to patch #4 in Roy's series). Meaning, the > PcdDefaultTerminalType setting will have to depend on TTY_TERMINAL too. > > So, ultimately, I think you need three separate cases: > > (a) !INTEL_BDS --> VT100TYPE (value 1) > (b) INTEL_BDS && !TTY_TERMINAL --> VT100TYPE (value 1) > (c) INTEL_BDS && TTY_TERMINAL --> TTYTERMTYPE (value 4) > > Again, this should be implemented on top of Roy's patch series. > > Thanks > Laszlo ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc index 0671469..0f104f3 100644 --- a/ArmVirtPkg/ArmVirtQemu.dsc +++ b/ArmVirtPkg/ArmVirtQemu.dsc @@ -140,6 +140,8 @@ # gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenVt100()" + ## Terminal Type - VT100, consistent with ConOut/ConIn Device Path. + gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1 gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 #
Change default terminal type to be VT100, to make it consistent with default ConIn/ConOut device path. I can't say this is a bug, as we can pass the whole console device path to ConnectController, and TerminalDxe driver will pick up the terminal in the remaining device path. However, in rare circumstances, the console devices may be disconnected with the driver, and they will be ignored by ConPlatformDxe until we pass the device path explicitly just as BDS. Changing default terminal type to be the same with console device path could help serial terminal be reconnected with normal connect controller operation. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Heyi Guo <heyi.guo@linaro.org> --- ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ 1 file changed, 2 insertions(+)