RE: [PATCH v1 4/6] pps: generators: Add PPS Generator TIO Driver

[D, Lakshmi Sowjanya]

> -----Original Message-----
> From: Peter Hilber <peter.hilber@xxxxxxxxxxxxxxx>
> Sent: Friday, October 20, 2023 9:12 PM
> To: Thomas Gleixner <tglx@xxxxxxxxxxxxx>; D, Lakshmi Sowjanya
> <lakshmi.sowjanya.d@xxxxxxxxx>; jstultz@xxxxxxxxxx;
> giometti@xxxxxxxxxxxx; corbet@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: x86@xxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx;
> andriy.shevchenko@xxxxxxxxxxxxxxx; Dong, Eddie <eddie.dong@xxxxxxxxx>; Hall,
> Christopher S <christopher.s.hall@xxxxxxxxx>; N, Pandith
> <pandith.n@xxxxxxxxx>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@xxxxxxxxx>; T R, Thejesh Reddy
> <thejesh.reddy.t.r@xxxxxxxxx>
> Subject: Re: [PATCH v1 4/6] pps: generators: Add PPS Generator TIO Driver
> 
> On 17.10.23 18:27, Thomas Gleixner wrote:
> > On Tue, Oct 17 2023 at 10:54, lakshmi.sowjanya.d@xxxxxxxxx wrote:
> >> +	guard(spinlock_irqsave)(&tio->lock);
> >> +	if (enable && !tio->enabled) {
> >> +		if (!is_current_clocksource_art_related()) {
> >> +			dev_err(tio->dev, "PPS cannot be started as clock is not
> related to ART");
> >> +			return -EPERM;
> >> +		}
> >
> > Ah. Here is the usecase for this magic patch 3/6 hackery. Again, it's
> > the wrong abstraction. You want something like:
> >
> >     timekeeping_clocksource_has_base(CSID_X86_ART);
> >
> > or something like this, which can be handled completely in the core
> > code.
> >
> > All of this needs some serious rework. See the below disfunctional
> > mockup patch for illustration.
> >
> > There is also a patch series, which tried to replace the clocksource
> > pointer in system_counterval_t with a clocksource ID:
> >
> >
> > https://lore.kernel.org/all/20230818011256.211078-1-peter.hilber@opens
> > ynergy.com
> >
> > That went nowhere, but has some valid points. I took some of Peter's
> > (cc'ed) ideas into the mockup, but did it slightly different to make
> > all of this indirection mess go away.
> >
> > There are certainly bugs and thinkos in that mockup. If you find them,
> > you can keep and fix them :)
> >
> 
> Hi Sowjanya,
> 
> I am working on another iteration of the patch series cited by Thomas, which
> would implement part of the mockup. But the scope of the original patch series
> would remain, so no changes to ART conversion code, no introduction of the
> clocksource_base concept... The patch series should still be compatible with
> Thomas' proposal, though.
> 
> I was wondering if it would make sense to coordinate the posting of my patch
> series. I planned to post the series sometime in November along with other
> stuff, but I could potentially post it earlier if this could help to align.
> 
> In case of interest in aligning, I uploaded an unverified preview for the upcoming
> series to [1].
> 
> Best regards,
> 
> Peter
> 
> [1] https://github.com/OpenSynergy/linux.git clocksource-id-for-
> get_device_system_crosststamp-v2-preview

Hi Peter,

Thanks for the patches.
 
Along with the patch you shared, we have verified the mockup code shared by Thomas.

Please let us know when you are planning to post the patches, we can co-ordinate and post. 

Regards
Lakshmi Sowjanya D