diff mbox series

[edk2,edk2-platforms,v1,01/16] Hisilicon/D0x: Remove SerdesLib

Message ID 20190201133436.10500-2-ming.huang@linaro.org
State New
Headers show
Series Fix issues and improve D0x | expand

Commit Message

Ming Huang Feb. 1, 2019, 1:34 p.m. UTC 
SerdesLib is useless for SmbiosMiscDxe and D06, so remove it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>

---
 Platform/Hisilicon/D06/D06.dsc                                   | 2 --
 Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
 2 files changed, 3 deletions(-)

-- 
2.9.5

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Feb. 11, 2019, 3:05 p.m. UTC | #1 
On Fri, Feb 01, 2019 at 09:34:21PM +0800, Ming Huang wrote:
> SerdesLib is useless for SmbiosMiscDxe and D06, so remove it.


Should it not then also delete #include <Library/SerdesLib.h> from
Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
?

Meanwhile,
Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
and
Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
both include this header, but
Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
and 
Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
do not declare the dependency.

Can you investigate and submit an updated patch addressing all of the
unnecessary references?

Best Regards,

Leif

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ming Huang <ming.huang@linaro.org>

> ---

>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --

>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -

>  2 files changed, 3 deletions(-)

> 

> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc

> index 396bd03c9d24..cbbd99e4a659 100644

> --- a/Platform/Hisilicon/D06/D06.dsc

> +++ b/Platform/Hisilicon/D06/D06.dsc

> @@ -64,8 +64,6 @@ [LibraryClasses.common]

>  

>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf

>  

> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf

> -

>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf

>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf

>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf

> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

> index 61cead7779b9..8e5c56fa41fd 100644

> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

> @@ -77,7 +77,6 @@ [LibraryClasses]

>  

>    IpmiCmdLib

>  

> -  SerdesLib

>  

>  [Protocols]

>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED

> -- 

> 2.9.5

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ming Huang Feb. 13, 2019, 6:36 a.m. UTC | #2 
On 2/11/2019 11:05 PM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:21PM +0800, Ming Huang wrote:

>> SerdesLib is useless for SmbiosMiscDxe and D06, so remove it.

> 

> Should it not then also delete #include <Library/SerdesLib.h> from

> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,

> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and

> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c

> ?

> 

> Meanwhile,

> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c

> and

> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c

> both include this header, but

> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf

> and 

> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf

> do not declare the dependency.


OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
SerdesLib.h, other .c files can not remove the header file.

> 

> Can you investigate and submit an updated patch addressing all of the

> unnecessary references?


This may takes a lot of time, as Hi1620(D06) is our important project,
maybe we should focus on D06.

Thanks

> 

> Best Regards,

> 

> Leif

> 

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Ming Huang <ming.huang@linaro.org>

>> ---

>>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --

>>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -

>>  2 files changed, 3 deletions(-)

>>

>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc

>> index 396bd03c9d24..cbbd99e4a659 100644

>> --- a/Platform/Hisilicon/D06/D06.dsc

>> +++ b/Platform/Hisilicon/D06/D06.dsc

>> @@ -64,8 +64,6 @@ [LibraryClasses.common]

>>  

>>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf

>>  

>> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf

>> -

>>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf

>>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf

>>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf

>> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>> index 61cead7779b9..8e5c56fa41fd 100644

>> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>> @@ -77,7 +77,6 @@ [LibraryClasses]

>>  

>>    IpmiCmdLib

>>  

>> -  SerdesLib

>>  

>>  [Protocols]

>>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED

>> -- 

>> 2.9.5

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Feb. 13, 2019, 9:42 a.m. UTC | #3 
On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:
> > Should it not then also delete #include <Library/SerdesLib.h> from

> > Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,

> > Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and

> > Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c

> > ?

> > 

> > Meanwhile,

> > Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c

> > and

> > Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c

> > both include this header, but

> > Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf

> > and 

> > Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf

> > do not declare the dependency.

> 

> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in

> SerdesLib.h, other .c files can not remove the header file.


If they are using definitions from the library header, but not the
library itself, there is something suspicious about the code
structuring.

But in the meantime, if they are referencing library header files,
they need to list those libraryclasses in their .inf.

> > Can you investigate and submit an updated patch addressing all of the

> > unnecessary references?

> 

> This may takes a lot of time, as Hi1620(D06) is our important project,

> maybe we should focus on D06.


Feel free to submit deletions for all and any platforms you are
unwilling to maintain.

Best Regards,

Leif


> Thanks

> 

> > 

> > Best Regards,

> > 

> > Leif

> > 

> >> Contributed-under: TianoCore Contribution Agreement 1.1

> >> Signed-off-by: Ming Huang <ming.huang@linaro.org>

> >> ---

> >>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --

> >>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -

> >>  2 files changed, 3 deletions(-)

> >>

> >> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc

> >> index 396bd03c9d24..cbbd99e4a659 100644

> >> --- a/Platform/Hisilicon/D06/D06.dsc

> >> +++ b/Platform/Hisilicon/D06/D06.dsc

> >> @@ -64,8 +64,6 @@ [LibraryClasses.common]

> >>  

> >>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf

> >>  

> >> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf

> >> -

> >>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf

> >>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf

> >>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf

> >> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

> >> index 61cead7779b9..8e5c56fa41fd 100644

> >> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

> >> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

> >> @@ -77,7 +77,6 @@ [LibraryClasses]

> >>  

> >>    IpmiCmdLib

> >>  

> >> -  SerdesLib

> >>  

> >>  [Protocols]

> >>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED

> >> -- 

> >> 2.9.5

> >>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ming Huang Feb. 13, 2019, 11:39 a.m. UTC | #4 
On 2/13/2019 5:42 PM, Leif Lindholm wrote:
> On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:

>>> Should it not then also delete #include <Library/SerdesLib.h> from

>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,

>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and

>>> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c

>>> ?

>>>

>>> Meanwhile,

>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c

>>> and

>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c

>>> both include this header, but

>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf

>>> and 

>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf

>>> do not declare the dependency.

>>

>> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in

>> SerdesLib.h, other .c files can not remove the header file.

> 

> If they are using definitions from the library header, but not the

> library itself, there is something suspicious about the code

> structuring.


Yes, there are maybe some unreasonable design in code structuring
for history reason.

> 

> But in the meantime, if they are referencing library header files,

> they need to list those libraryclasses in their .inf.


Do you mean add SerdesLib back to SmbiosMiscDxe.inf? If yes, maybe this
patch should be droped.

Thanks

> 

>>> Can you investigate and submit an updated patch addressing all of the

>>> unnecessary references?

>>

>> This may takes a lot of time, as Hi1620(D06) is our important project,

>> maybe we should focus on D06.

> 

> Feel free to submit deletions for all and any platforms you are

> unwilling to maintain.

> 

> Best Regards,

> 

> Leif

> 

> 

>> Thanks

>>

>>>

>>> Best Regards,

>>>

>>> Leif

>>>

>>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>>> Signed-off-by: Ming Huang <ming.huang@linaro.org>

>>>> ---

>>>>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --

>>>>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -

>>>>  2 files changed, 3 deletions(-)

>>>>

>>>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc

>>>> index 396bd03c9d24..cbbd99e4a659 100644

>>>> --- a/Platform/Hisilicon/D06/D06.dsc

>>>> +++ b/Platform/Hisilicon/D06/D06.dsc

>>>> @@ -64,8 +64,6 @@ [LibraryClasses.common]

>>>>  

>>>>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf

>>>>  

>>>> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf

>>>> -

>>>>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf

>>>>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf

>>>>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf

>>>> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>>>> index 61cead7779b9..8e5c56fa41fd 100644

>>>> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>>>> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>>>> @@ -77,7 +77,6 @@ [LibraryClasses]

>>>>  

>>>>    IpmiCmdLib

>>>>  

>>>> -  SerdesLib

>>>>  

>>>>  [Protocols]

>>>>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED

>>>> -- 

>>>> 2.9.5

>>>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ming Huang Feb. 14, 2019, 10:51 a.m. UTC | #5 
On 2/13/2019 5:42 PM, Leif Lindholm wrote:
> On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:

>>> Should it not then also delete #include <Library/SerdesLib.h> from

>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,

>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and

>>> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c

>>> ?

>>>

>>> Meanwhile,

>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c

>>> and

>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c

>>> both include this header, but

>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf

>>> and 

>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf

>>> do not declare the dependency.

>>

>> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in

>> SerdesLib.h, other .c files can not remove the header file.

> 

> If they are using definitions from the library header, but not the

> library itself, there is something suspicious about the code

> structuring.

> 

> But in the meantime, if they are referencing library header files,

> they need to list those libraryclasses in their .inf.

> 

>>> Can you investigate and submit an updated patch addressing all of the

>>> unnecessary references?

>>

>> This may takes a lot of time, as Hi1620(D06) is our important project,

>> maybe we should focus on D06.

> 

> Feel free to submit deletions for all and any platforms you are

> unwilling to maintain.


Maybe there are no enough time to investigate this. Is ok to do this for 19.06?
When should I send the v2 out?

Thanks

> 

> Best Regards,

> 

> Leif

> 

> 

>> Thanks

>>

>>>

>>> Best Regards,

>>>

>>> Leif

>>>

>>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>>> Signed-off-by: Ming Huang <ming.huang@linaro.org>

>>>> ---

>>>>  Platform/Hisilicon/D06/D06.dsc                                   | 2 --

>>>>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -

>>>>  2 files changed, 3 deletions(-)

>>>>

>>>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc

>>>> index 396bd03c9d24..cbbd99e4a659 100644

>>>> --- a/Platform/Hisilicon/D06/D06.dsc

>>>> +++ b/Platform/Hisilicon/D06/D06.dsc

>>>> @@ -64,8 +64,6 @@ [LibraryClasses.common]

>>>>  

>>>>    CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf

>>>>  

>>>> -  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf

>>>> -

>>>>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf

>>>>    RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf

>>>>    OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf

>>>> diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>>>> index 61cead7779b9..8e5c56fa41fd 100644

>>>> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>>>> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf

>>>> @@ -77,7 +77,6 @@ [LibraryClasses]

>>>>  

>>>>    IpmiCmdLib

>>>>  

>>>> -  SerdesLib

>>>>  

>>>>  [Protocols]

>>>>    gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED

>>>> -- 

>>>> 2.9.5

>>>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
index 396bd03c9d24..cbbd99e4a659 100644
--- a/Platform/Hisilicon/D06/D06.dsc
+++ b/Platform/Hisilicon/D06/D06.dsc
@@ -64,8 +64,6 @@  [LibraryClasses.common]
 
   CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
 
-  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
-
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
   RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
   OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
diff --git a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
index 61cead7779b9..8e5c56fa41fd 100644
--- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
+++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
@@ -77,7 +77,6 @@  [LibraryClasses]
 
   IpmiCmdLib
 
-  SerdesLib
 
 [Protocols]
   gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED