Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add new JedecJep106Lib to fetch JEDEC JEP106 manufacturer

["Michael D Kinney" <michael.d.kinney@intel.com>]

Yes.  That would be good to add to as well.

Mike

> -----Original Message-----
> From: Rebecca Cran <rebecca@bsdio.com>
> Sent: Friday, April 7, 2023 4:11 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>
> Cc: Rebecca Cran <rebecca@quicinc.com>
> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add new JedecJep106Lib to fetch JEDEC JEP106 manufacturer
> 
> Sorry, not the struct - the array:
> 
> 
> STATIC CONST JEDEC_MANUFACTURERS *Jep106Manufacturers[] = {
>    Jep106ManufacturersBank1,
>    Jep106ManufacturersBank2,
>    Jep106ManufacturersBank3,
>    Jep106ManufacturersBank4,
>    Jep106ManufacturersBank5,
>    Jep106ManufacturersBank6,
>    Jep106ManufacturersBank7,
>    Jep106ManufacturersBank8,
>    Jep106ManufacturersBank9,
>    Jep106ManufacturersBank10,
>    Jep106ManufacturersBank11,
>    Jep106ManufacturersBank12,
>    Jep106ManufacturersBank13,
>    Jep106ManufacturersBank14
> };
> 
> On 4/7/23 5:06 PM, Michael D Kinney wrote:
> > Which struct?  That macro only applies to global variables, not structure declarations.
> >
> > Mike
> >
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca Cran
> >> Sent: Friday, April 7, 2023 3:59 PM
> >> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@intel.com>; Gao, Liming
> >> <gaoliming@byosoft.com.cn>
> >> Cc: Rebecca Cran <rebecca@quicinc.com>
> >> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add new JedecJep106Lib to fetch JEDEC JEP106 manufacturer
> >>
> >> I guess GLOBAL_REMOVE_IF_UNREFERENCED should be added to the struct too?
> >>
> >>
> >> On 4/7/23 4:42 PM, Rebecca Cran wrote:
> >>> On 4/7/23 2:25 PM, Kinney, Michael D wrote:
> >>>> Comments below.
> >>>>
> >>>> Hopefully this lib would only be used by modules that get compressed.
> >>> I guess so, but that's for the user to decide.
> >>>> Might add GLOBAL_REMOVE_IF_UNREFERENCED to the arrays of strings to
> >>>> help the optimizer remove the data that is not referenced.
> >>> Good idea - I'll add that.
> >>>
> >>>>> +CONST CHAR8 *
> >>>>> +EFIAPI
> >>>>> +Jep106GetManufacturerName (
> >>>>> +  IN UINT8  Code,
> >>>>> +  IN UINT8  ContinuationBytes
> >>>>> +  )
> >>>>> +{
> >>>>> +  UINTN                      Index;
> >>>>> +  CONST JEDEC_MANUFACTURERS  *ManufacturersBank;
> >>>>> +
> >>>>> +  Index = 0;
> >>>>> +
> >>>>> +  if (ContinuationBytes >= JEP106_MANUFACTURERS_NUM_BANKS) {
> >>>>> +    ASSERT (0);
> >>>> Do you really want an ASSERT() from this?  If this is data from a DIMM
> >>>> I doubt we want platform to ASSERT().  Perhaps just return NULL?
> >>> I'll remove it. I added it for validation when I was writing the SPD
> >>> parsing library.
> >>>>> +UINTN
> >>>>> +EFIAPI
> >>>>> +Jep106GetLongestManufacturerName (
> >>>>> +  VOID
> >>>>> +  )
> >>>> Why is this API needed?  Wouldn’t you really just need the
> >>>> longest of the ones present in the current boot to build the
> >>>> SMBIOS record?.  Not the longest of all in the banks?
> >>> I added it because it's useful to know the maximum possible size of
> >>> the strings that could be in the SMBIOS Type 17 table before doing the
> >>> SPD parsing.
> >>>
> >>> I'll remove it for now, and if it's really needed I can add it back in.
> >>>
> >>>
> >>
> >>
> >>
> >
> >
> > 
> >
> >