Re: API breakages and their implications. Was: [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support


Leif Lindholm
 

On Tue, Mar 31, 2020 at 01:53:21 +0000, Ni, Ray wrote:
Leif,
Please understand that the concern of this change is all the platforms that uses
this serial port lib must be changed otherwise build breaks.
Yes. This is the nature of collaborative development.
This is something we on the ARM side have lived with since we got
involved with EDK2, being the less-deployed user of the codebase, and
that is fine.

TianoCore specifically produced the UDK to make life easier for those
who are unable to track upstream, and we have followed that up with
stable tags. I would highly recommend that any team that feels that
this change would be too much effort to move to edk2-stable202002. Of
course, they would then need to manually cherry-pick any improvements
and security fixes from master. That is their choice.

Nevertheless, breaking existing platforms is a side effect, not a
goal. So if an alternative solution can be found (which is not a hack
that is going to affect the codebase negatively over time and simply
need to be fixed properly at a later date), then clearly that is
preferable.

"This breaks many platforms" is a good argument for seeing if a
solution can be found that does not break (as) many platforms. It is
not an argument for duplicating drivers when the change needed for
those platforms is trivial.

These days, Linux tends to deal with API breakages (and other things)
using a semantic patch tool called Coccinelle[1]. It would not be
unreasonable, and indeed it would be helpful to us on the non-x86 side
if something similar was adopted (and semantic patches mandated) for
API breakages in EDK2.

[1] http://coccinelle.lip6.fr/sp.php

Regards,

Leif

Ard,
Using Guided HOB sounds a good idea to me: )
The benefits of using HOB is:
Length field in the HOB header can be used for extension if more parameters are needed.
DXE can have the HOB access as well.

EFI_SEC_HOB_DATA_PPI can be used to return the new Guided HOB from SEC phase if needed.

Thanks,
Ray


-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Sent: Monday, March 30, 2020 3:45 PM
To: Leif Lindholm <leif@nuviainc.com>
Cc: Jiang, Guomin <guomin.jiang@intel.com>; devel@edk2.groups.io; pankaj.bansal@nxp.com; Ni, Ray <ray.ni@intel.com>;
Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Dong,
Guo <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>; Meenakshi Aggarwal
<meenakshi.aggarwal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>
Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support

On Mon, 30 Mar 2020 at 09:35, Leif Lindholm <leif@nuviainc.com> wrote:

Hi Jiang,

It is not a question of effort of copying a driver, it is a question
that copying drivers is something that should be avoided wherever
practically possible. I did not think this topic was still under
debate.

If the existing 16550 SerialPortLib is overspecialised to the point
where it only works on a subset of 16550 implementations, then it
should change. There are going to be more non-PC systems turning up
with 16550 UARTs - should they each copy/modify their drivers?

If there are better ways of solving that problem, please suggest.
But more duplicated drivers is not the answer.
Could we use a GUIDed HOB? If it exists, we use its contents, and if
it doesn't, we use the default set by the FixedPCD.

Join devel@edk2.groups.io to automatically receive all group messages.