From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-AM5-obe.outbound.protection.outlook.com (mail-eopbgr30065.outbound.protection.outlook.com [40.107.3.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5C38521D046B2 for ; Mon, 18 Sep 2017 03:21:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=fxE0JvkJqwYrmHOXGZvE8GKMdb8BrOXgB9wBSdNmd1k=; b=T39OuDGdVoVDEHMixHiXWXXqZ64Yq2sDyEOLmTDfkaEvrjyqAEkgi8lQFW7HWcFx/Ij9fI9H5nIRLCOLhroIBVfpJzqElqIJpOqbF/t6inmwC/4N5GSb5TNjm92rPLhkRDk/0xC3fdmXL5w/2QvEHKW+EC0zWGNmCUzPlCfSiKA= Received: from AM0PR0402MB3940.eurprd04.prod.outlook.com (52.133.40.140) by AM0PR0402MB3619.eurprd04.prod.outlook.com (52.133.46.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.56.11; Mon, 18 Sep 2017 10:24:18 +0000 Received: from AM0PR0402MB3940.eurprd04.prod.outlook.com ([fe80::d0d3:8e74:b9b0:d4af]) by AM0PR0402MB3940.eurprd04.prod.outlook.com ([fe80::d0d3:8e74:b9b0:d4af%13]) with mapi id 15.20.0056.016; Mon, 18 Sep 2017 10:24:17 +0000 From: Pankaj Bansal To: "Zeng, Star" , Laszlo Ersek , "Heyi Guo" , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" , Varun Sethi Thread-Topic: [edk2] [PATCH] Fix not able to change serial attributes Thread-Index: AQHTLiQ02NebkL7qvEa80dXGaHK07qK2t+OAgAE9gwCAAn4/gIAAAFyA Date: Mon, 18 Sep 2017 10:24:17 +0000 Message-ID: References: <0C09AFA07DD0434D9E2A0C6AEB0483103B969DE8@shsmsx102.ccr.corp.intel.com> <135e15b4-9db2-04ac-2ae9-244bb8711945@redhat.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B970AB3@shsmsx102.ccr.corp.intel.com> In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B970AB3@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=pankaj.bansal@nxp.com; x-originating-ip: [192.88.169.43] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM0PR0402MB3619; 6:F0honEbXNg6X3X80oCfQUwtSvcJR9a5Ioy3+OLo2zsMq80FZoyZuJk+P/pI7M+qASKolb+N0C3aGQ9PKZ6rS4KkyqjeGNH+p0KoGuQ0wIZwHeDJwBxuv1dbnvTmU83BYngRJZ1zGhmYlUuU6r7LFi8wz/efYAK33+/CKIekMxWHl16v8SgiQ/xkRCC3ctkNVjtOGWeiY3P7t1TWZ//enteWoXhLzg6x1S3vOEmt5P8u4w36iETWBlunCw+pEZYd3g7ZXER0xtNBDSiB84QzweeiESQh5w7meeItWbK0zFipqkNbUyBiJO2lQMDAS/fO4LXxBGC+OBSAKeeV21NzvNg==; 5:ncK8oYVT9KptYGQDf7+pYeMGYwUUUToUMqktwvHEgtI9LeyOw8jDfa4X1oKYlzFE+htNF8KJu7gk48PjLny/LZBnMSm1ZhL04xt8Q/bnHQYm6snDiFiWIYQtPfZWcHD2BufoMQklCHuYeP+Yt0x1NA==; 24:UMBrQtsb58pmM3566HbG7+/6HEMWoIa4Gkr+sKsMR+HjUpt5xgQWJ95r3sOEzTJwjKLaoGTprqiI/WnBULQEAn4rjR/QVun29pN+8csYwGo=; 7:xx3OeAXnplb35gcf0pUcd+wcAajBqs820icSM+abpvBul9Cqh1zfC4+zWcjPmDS9teT8+5OBpnsm1dXMrLBULYj6cdKSC2y5XQEnhBhQaLOv9qDp3/g8f5+UNtnXxU36UQ0ThjZRiEA35CUrhQ/VZG4bFEXhvisMJn/YGPxy3atw+LriJ0reah3D0Cl4RLZBamS61itL0Xk6MRlRw4GkuD8GT+HamW16c//j7Wsiu44= x-ms-exchange-antispam-srfa-diagnostics: SSOS;SSOR; x-forefront-antispam-report: SFV:SKI; SCL:-1; SFV:NSPM; SFS:(10009020)(6009001)(346002)(376002)(39860400002)(51914003)(199003)(24454002)(377454003)(189002)(13464003)(81156014)(2950100002)(8936002)(8676002)(81166006)(561924002)(6246003)(4326008)(93886005)(7736002)(54356999)(76176999)(53546010)(66066001)(101416001)(6506006)(33656002)(6436002)(106356001)(50986999)(105586002)(305945005)(229853002)(14454004)(316002)(5250100002)(97736004)(7696004)(86362001)(74316002)(54906002)(966005)(68736007)(6306002)(55016002)(6116002)(102836003)(3846002)(3660700001)(2501003)(9686003)(189998001)(99286003)(3280700002)(2900100001)(2906002)(478600001)(5660300001)(53936002)(25786009)(44824005); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR0402MB3619; H:AM0PR0402MB3940.eurprd04.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; x-ms-office365-filtering-correlation-id: abeccb45-88c2-4bca-81a5-08d4fe7f6b18 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(48565401081)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:AM0PR0402MB3619; x-ms-traffictypediagnostic: AM0PR0402MB3619: x-exchange-antispam-report-test: UriScan:(185117386973197)(162533806227266)(228905959029699); x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3002001)(100000703101)(100105400095)(6055026)(6041248)(20161123564025)(20161123558100)(20161123562025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:AM0PR0402MB3619; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:AM0PR0402MB3619; x-forefront-prvs: 04347F8039 received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Sep 2017 10:24:17.6527 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR0402MB3619 Subject: Re: [PATCH] Fix not able to change serial attributes X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Sep 2017 10:21:17 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Laszlo, Zeng Thanks for patch review. @Zeng: The sermode command calls SerialSetAttributes, which sets H/W attrib= utes of Serial device. After that the SerialIo protocol is reinstalled, which causes MdeModulePkg/= Universal/Console/TerminalDxe and MdeModulePkg/Universal/Console/ConPlatformDxe drivers' bindings to stop= and then start. This in turn calls SerialReset, which undoes changes of SerialSetAttributes= . Thanks & Regards, Pankaj Bansal -----Original Message----- From: Zeng, Star [mailto:star.zeng@intel.com]=20 Sent: Monday, September 18, 2017 3:52 PM To: Laszlo Ersek ; Heyi Guo ; Panka= j Bansal ; edk2-devel@lists.01.org Cc: Ni, Ruiyu ; Zeng, Star Subject: RE: [edk2] [PATCH] Fix not able to change serial attributes Thanks for your good comments. :) Since there is no clear description for the behavior of Reset() :(, I prone= to align the behavior with MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c= and IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c, that means I ag= ree the fix. Laszlo and Gary, if you can help do some simple regression test with the pa= tch, that will be better. Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo Ersek Sent: Sunday, September 17, 2017 4:18 AM To: Zeng, Star ; Pankaj Bansal = ; edk2-devel@lists.01.org Cc: Ni, Ruiyu Subject: Re: [edk2] [PATCH] Fix not able to change serial attributes On 09/16/17 03:21, Zeng, Star wrote: > Pankaj, >=20 > Thanks for the contribution. > Could you help update the title to be like MdeModulePkg SerialDxe: XXXXXX= XXXX? >=20 > In fact, I agree the fix as it matches the code at MdeModulePkg/Bus/Pci/P= ciSioSerialDxe/SerialIo.c and IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/= Serial.c. > But I am curious about how you reproduce the issue by sermode command as = I see sermode command only calls SerialIo->SetAttributes() but not SerialIo= ->Reset(). >=20 >=20 > Ray, Laszlo, > Do you have any comment? Thanks for the CC. The UEFI spec is very superficial on EFI_SERIAL_IO_PROTOCOL.Reset(). It onl= y says, "The Reset() function resets the hardware of a serial device." It d= oesn't define what state the device should return to after resetting the ha= rdware. Under the generic description of EFI_SERIAL_IO_PROTOCOL, we have "The default attributes for all UART-style serial device interfaces are: 115,200 baud, a 1 byte receive FIFO, a 1,000,000 microsecond timeout per ch= aracter, no parity, 8 data bits, and 1 stop bit." Now, the PCDs that are currently used in the code may differ from the above= standard-mandated values, but that's the platform's responsibility. The po= int is that the UEFI spec seems to require that the device be returned to a= predefined state, and the PCDs make that possible. I don't think that the = argument in the commit message, "Serial Reset command should set the attrib= utes which have been changed by user after calling SerialSetAttributes.", i= s consistent with the UEFI spec's intent. However, I could easily be wrong about this, given especially that the othe= r two SerialIo implementations already follow the suggested practice. I guess I'll have to stay neutral on this patch. Hopefully it won't regress= anything. Thanks, Laszlo > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Pankaj Bansal > Sent: Friday, September 15, 2017 9:14 PM > To: edk2-devel@lists.01.org > Cc: Pankaj Bansal > Subject: [edk2] [PATCH] Fix not able to change serial attributes >=20 > Issue : When try to change serial attributes using sermode command, the d= efault values are set Cause : The SerialReset command resets the attributes= ' values to default Fix : Serial Reset command should set the attributes wh= ich have been changed by user after calling SerialSetAttributes. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Pankaj Bansal > --- > MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66 > ++++++++--------------------- > 1 file changed, 18 insertions(+), 48 deletions(-) >=20 > diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c > b/MdeModulePkg/Universal/SerialDxe/SerialIo.c > index 43d33db..dc7e13a 100644 > --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c > +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c > @@ -220,7 +220,6 @@ SerialReset ( > ) > { > EFI_STATUS Status; > - EFI_TPL Tpl; > =20 > Status =3D SerialPortInitialize (); > if (EFI_ERROR (Status)) { > @@ -228,49 +227,17 @@ SerialReset ( > } > =20 > // > - // Set the Serial I/O mode and update the device path > - // > - > - Tpl =3D gBS->RaiseTPL (TPL_NOTIFY); > - > - // > - // Set the Serial I/O mode > - // > - This->Mode->ReceiveFifoDepth =3D PcdGet16 (PcdUartDefaultReceiveFifoD= epth); > - This->Mode->Timeout =3D 1000 * 1000; > - This->Mode->BaudRate =3D PcdGet64 (PcdUartDefaultBaudRate); > - This->Mode->DataBits =3D (UINT32) PcdGet8 (PcdUartDefaultData= Bits); > - This->Mode->Parity =3D (UINT32) PcdGet8 (PcdUartDefaultPari= ty); > - This->Mode->StopBits =3D (UINT32) PcdGet8 (PcdUartDefaultStop= Bits); > - > - // > - // Check if the device path has actually changed > - // > - if (mSerialDevicePath.Uart.BaudRate =3D=3D This->Mode->BaudRate && > - mSerialDevicePath.Uart.DataBits =3D=3D (UINT8) This->Mode->DataBit= s && > - mSerialDevicePath.Uart.Parity =3D=3D (UINT8) This->Mode->Parity = && > - mSerialDevicePath.Uart.StopBits =3D=3D (UINT8) This->Mode->StopBit= s > - ) { > - gBS->RestoreTPL (Tpl); > - return EFI_SUCCESS; > - } > - > - // > - // Update the device path > + // Go set the current attributes > // > - mSerialDevicePath.Uart.BaudRate =3D This->Mode->BaudRate; > - mSerialDevicePath.Uart.DataBits =3D (UINT8) This->Mode->DataBits; > - mSerialDevicePath.Uart.Parity =3D (UINT8) This->Mode->Parity; > - mSerialDevicePath.Uart.StopBits =3D (UINT8) This->Mode->StopBits; > - > - Status =3D gBS->ReinstallProtocolInterface ( > - mSerialHandle, > - &gEfiDevicePathProtocolGuid, > - &mSerialDevicePath, > - &mSerialDevicePath > - ); > - > - gBS->RestoreTPL (Tpl); > + Status =3D This->SetAttributes ( > + This, > + This->Mode->BaudRate, > + This->Mode->ReceiveFifoDepth, > + This->Mode->Timeout, > + (EFI_PARITY_TYPE) This->Mode->Parity, > + (UINT8) This->Mode->DataBits, > + (EFI_STOP_BITS_TYPE) This->Mode->StopBits > + ); > =20 > return Status; > } > @@ -513,11 +480,6 @@ SerialDxeInitialize ( { > EFI_STATUS Status; > =20 > - Status =3D SerialPortInitialize (); > - if (EFI_ERROR (Status)) { > - return Status; > - } > - > mSerialIoMode.BaudRate =3D PcdGet64 (PcdUartDefaultBaudRate); > mSerialIoMode.DataBits =3D (UINT32) PcdGet8 (PcdUartDefaultDataBits); > mSerialIoMode.Parity =3D (UINT32) PcdGet8 (PcdUartDefaultParity); > @@ -529,6 +491,14 @@ SerialDxeInitialize ( > mSerialDevicePath.Uart.StopBits =3D PcdGet8 (PcdUartDefaultStopBits); > =20 > // > + // Issue a reset to initialize the COM port // Status =3D=20 > + mSerialIoTemplate.Reset (&mSerialIoTemplate); if (EFI_ERROR > + (Status)) { > + return Status; > + } > + > + // > // Make a new handle with Serial IO protocol and its device path on it= . > // > Status =3D gBS->InstallMultipleProtocolInterfaces ( > -- > 2.7.4 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >=20 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel