RE: [PATCH v6 01/11] x86/tsc: Add base clock properties in clocksource structure
From: D, Lakshmi Sowjanya
Date: Tue Apr 16 2024 - 06:10:42 EST
> -----Original Message-----
> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Sent: Thursday, April 11, 2024 2:51 AM
> To: D, Lakshmi Sowjanya <lakshmi.sowjanya.d@xxxxxxxxx>;
> jstultz@xxxxxxxxxx; giometti@xxxxxxxxxxxx; corbet@xxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Cc: x86@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; intel-
> wired-lan@xxxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; Dong, Eddie
> <eddie.dong@xxxxxxxxx>; Hall, Christopher S <christopher.s.hall@xxxxxxxxx>;
> Brandeburg, Jesse <jesse.brandeburg@xxxxxxxxx>; davem@xxxxxxxxxxxxx;
> alexandre.torgue@xxxxxxxxxxx; joabreu@xxxxxxxxxxxx;
> mcoquelin.stm32@xxxxxxxxx; perex@xxxxxxxx; linux-sound@xxxxxxxxxxxxxxx;
> Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>;
> peter.hilber@xxxxxxxxxxxxxxx; N, Pandith <pandith.n@xxxxxxxxx>; Mohan,
> Subramanian <subramanian.mohan@xxxxxxxxx>; T R, Thejesh Reddy
> <thejesh.reddy.t.r@xxxxxxxxx>; D, Lakshmi Sowjanya
> <lakshmi.sowjanya.d@xxxxxxxxx>
> Subject: Re: [PATCH v6 01/11] x86/tsc: Add base clock properties in clocksource
> structure
>
> On Wed, Apr 10 2024 at 17:18, lakshmi.sowjanya.d@xxxxxxxxx wrote:
> > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@xxxxxxxxx>
> >
> > Add base clock hardware abstraction in clocksource structure.
> >
> > Add clocksource ID for x86 ART(Always Running Timer). The newly added
> > clocksource ID and conversion parameters are used to convert time in a
> > clocksource domain to base clock and vice versa.
> >
> > Convert the base clock to the system clock using convert_base_to_cs()
> > in get_device_system_crosststamp().
>
> In https://lore.kernel.org/all/875xxhi1ty.ffs@tglx I asked you to provide a
> change log which explains the WHY and not the WHAT. The new change log still
> fails to explain WHY this change is needed and which problem it is trying to
> solve.
Rephrased the commit message to:
" Add base clock hardware abstraction in clocksource structure.
The core code has a mechanism to convert the ART base clock to
the corresponding tsc value.
Provide the generic functionality in convert_base_to_cs() to convert
base clock timestamps to system clocksource without requiring
architecture specific parameters.
Add the infrastructure in get_device_system_crosststamp()."
>
> I further asked you to do:
>
> 1) Add the clocksource_base struct and provide the infrastructure in
> get_device_system_crosststamp()
>
> 2) Make TSC/ART use it
>
> But this still does #1 and #2 in one go.
>
> If you don't understand my review comments, then please ask. If you disagree
> with them then please tell me and argue with me.
>
> Just ignoring them is not an option.
Sorry Thomas, that was a mis-understanding. We had split only realtime as separate patch.
We will split the first patch as suggested.
1. Timekeeping part(convert_base_to_cs() and infrastructure in get_device_system_crosststamp())
2. x86(TSC/ART values update into the structure)
Thanks,
Sowjanya
>
> Thanks,
>
> tglx