[U-Boot] [PATCH v5 5/8] x86: slimbootloader: Set TSC information for timer driver

[Park, Aiden]

Hi Andy,

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko at gmail.com]
> Sent: Monday, July 22, 2019 8:42 AM
> To: Park, Aiden <aiden.park at intel.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Simon Glass
> <sjg at chromium.org>; Bin Meng <bmeng.cn at gmail.com>
> Subject: Re: [PATCH v5 5/8] x86: slimbootloader: Set TSC information for timer
> driver
> 
> On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden <aiden.park at intel.com> wrote:
> >
> > Slim Bootloader provides TSC clock information in its performance info
> > hob. For now, TSC clock information is only used for timer driver from
> > the performance info hob.
> > - Get TSC frequency from performance info hob
> > - Set tsc_base and clock_rate for timer driver
> 
> So why do we need this at all? We have a common TSC driver that works for all
> x86.
> 
This series also uses TSC driver, but giving these values will bypass TSC calibration in the driver.
Slim Bootloader already has CPU specific TSC value and provides the TSC value
in its performance data HOB. Therefore, U-Boot TSC driver does not have to re-calibrate TSC.

> > +#define LOADER_PERFORMANCE_INFO_GUID \
> > +       { \
> > +       0x868204be, 0x23d0, 0x4ff9, \
> > +       { 0xac, 0x34, 0xb9, 0x95, 0xac, 0x04, 0xb1, 0xb9 } \
> > +       }
> 
> Use EFI_GUID(), please.
> 
Let me use EFI_GUID.

> > +struct performance_info {
> 
> Same, you better to make less generic names for data structs.
>
Let me use particular one. Thanks.

> > +} __packed;
> 
> Same question. If it's aligned naturally by 32-bit boundaries, Why do you need
> __packed?
>
Let me remote __packed. Thanks.
 
> --
> With Best Regards,
> Andy Shevchenko

Best Regards,
Aiden