Received-SPF: pass (google.com: domain of linux-kernel+bounces-146612-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: "D, Lakshmi Sowjanya" <lakshmi.sowjanya.d@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>, "jstultz@google.com"
	<jstultz@google.com>, "giometti@enneenne.com" <giometti@enneenne.com>,
	"corbet@lwn.net" <corbet@lwn.net>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
CC: "x86@kernel.org" <x86@kernel.org>, "netdev@vger.kernel.org"
	<netdev@vger.kernel.org>, "linux-doc@vger.kernel.org"
	<linux-doc@vger.kernel.org>, "intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>, "andriy.shevchenko@linux.intel.com"
	<andriy.shevchenko@linux.intel.com>, "Dong, Eddie" <eddie.dong@intel.com>,
	"Hall, Christopher S" <christopher.s.hall@intel.com>, "Brandeburg, Jesse"
	<jesse.brandeburg@intel.com>, "davem@davemloft.net" <davem@davemloft.net>,
	"alexandre.torgue@foss.st.com" <alexandre.torgue@foss.st.com>,
	"joabreu@synopsys.com" <joabreu@synopsys.com>, "mcoquelin.stm32@gmail.com"
	<mcoquelin.stm32@gmail.com>, "perex@perex.cz" <perex@perex.cz>,
	"linux-sound@vger.kernel.org" <linux-sound@vger.kernel.org>, "Nguyen, Anthony
 L" <anthony.l.nguyen@intel.com>, "peter.hilber@opensynergy.com"
	<peter.hilber@opensynergy.com>, "N, Pandith" <pandith.n@intel.com>, "Mohan,
 Subramanian" <subramanian.mohan@intel.com>, "T R, Thejesh Reddy"
	<thejesh.reddy.t.r@intel.com>
Subject: RE: [PATCH v6 09/11] pps: generators: Add PPS Generator TIO Driver
Thread-Topic: [PATCH v6 09/11] pps: generators: Add PPS Generator TIO Driver
Thread-Index: AQHaiz0vASRBRbSJ8kCIgRN774y8wLFiFmmAgAihucA=
Date: Tue, 16 Apr 2024 10:19:44 +0000
Message-ID: <CY8PR11MB7364070B11D5D9E4711613F0C4082@CY8PR11MB7364.namprd11.prod.outlook.com>
References: <20240410114828.25581-1-lakshmi.sowjanya.d@intel.com>
 <20240410114828.25581-10-lakshmi.sowjanya.d@intel.com> <874jc83l9g.ffs@tglx>
In-Reply-To: <874jc83l9g.ffs@tglx>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?qvrCfjbqUOpRQ+famMB8/lG+OgQf8Zk3q/Da7PxtnPSGUIeS+Uh3ag63NQqc?=
 =?us-ascii?Q?Z990GHyRVhSNWRNxeIzGgbna8/1sPcv4IDkusvyvwA06ZJ0PrUnmTH00J59c?=
 =?us-ascii?Q?oEZLZ8zDSo5CJUHSCtnnXrb2nE0mnjesGjkEQBfT3zbDygK1qhHEZQKElWZN?=
 =?us-ascii?Q?JvwmfnOVbDrUe+MsQLkzcHJ8M5HweDNalNApy5jWJeZDBJgbJymXR68icuGZ?=
 =?us-ascii?Q?NM3LkQFI+RZHnHtiBARo9CW791F31IdbTtqKXkg439PKatK9WIkR/hgJ9PUo?=
 =?us-ascii?Q?NqW/jpoHRk57pSGe4mFUA4WVwiJa7l8AS+XbjAXUXpw/Buie2JG0v9DEi6kY?=
 =?us-ascii?Q?udG+5urO0yHtbd3KCcl5nt1a9ZKG/MhCOzvDOzBVkELqQCltfHBvr01qFlSX?=
 =?us-ascii?Q?9dIa5nIfpUVe2uJ9l7gsQvkFMR8pvjz/9j+SnU3NwZuFE0Z/9BbN7QhaHBAW?=
 =?us-ascii?Q?8veZf9hLPCq1QMUOBIRxTNoerwGpDeCKAVXsp5+Nh7IwVLwXz3MwlZQyz+ZY?=
 =?us-ascii?Q?8GoC6D8wTWrEp9peQQiGq44Mf1efwc+hf55N8dOkgBkVv7hxBBY892bBpsOD?=
 =?us-ascii?Q?4n43PcycP9RIKV0d1RpoWyqUHfugb9ke7/ZxjnCg8blAOoRoQExguCt2/ui6?=
 =?us-ascii?Q?mnBYVMUxNAKrSHxUSNQo2MFQJVD8zZjXZHWF6YgbI57kbZ1pPj9rsGGmrWQU?=
 =?us-ascii?Q?1WXloD0sPYKHGumt/BzhrjPNOStY6ryyb9uC3Mdr0RHVCy4VswkH45dF4xQN?=
 =?us-ascii?Q?iXr1G/Ai8VZdgfR3LBZJIMyTt9n0bMEJ0MRmvUBQstCMNFOiWLv52rR8NskL?=
 =?us-ascii?Q?iafbrZoSZA8r7ShiD4FOdNhmsspquf3GUywX52X7+JjFz/SvB/NkNWKamlAJ?=
 =?us-ascii?Q?GaENPRN6shKH7+mbhZ26EPS1nK2pMCht6FwWBJVDcxM7h0++dalEl4/tcM3m?=
 =?us-ascii?Q?uyMWlzDjOlDLaLWVtC/TN5n0UN5jkkca0pWEmgFKk9CGbWaVk1NiKO/FoiwX?=
 =?us-ascii?Q?zD9xGiSZiRZGfYvrM5kcPMVg4O1tmgAWf6LdpA8tvoDozh8Bd9ZEhilTJF0d?=
 =?us-ascii?Q?PasisG5LrCQ+SnATr+D2Bqx7j3mhJgFkkT4ViUDsoFdiilvh9LMdjnRC1Vfh?=
 =?us-ascii?Q?7JafI3W+HPsxg3diJdOx928CbV9j3CgDPFfjkSbGG8URvi1GaeNwFOp1/Mut?=
 =?us-ascii?Q?68OGi1kJILFyUS5fMrR/MmxswZaBN57QMurgTiWqp9GX84c7h8LbAotkMJvv?=
 =?us-ascii?Q?n4fowCXULO4VsPm2YjWm+7RQlhya7WhFbVizB94bZKowN5OK7l1Fi/66OzLH?=
 =?us-ascii?Q?6blvDAIpDHlf98mfwMhjLEMCNCA4j3AWbE/Klveq7irM87VDVxIpLx9uXqVb?=
 =?us-ascii?Q?tLFQDLHj8aAvGHw7eES+7YxcGKND3qU4Ttdjzx5DV9egAjBjz/y12bp17ZHv?=
 =?us-ascii?Q?EF6PJE4UuVomlToeGx0fiXYX3hJXh42BSXEZKA8Tly8OPOyqdSOpdv/hSD+u?=
 =?us-ascii?Q?u9F8trhwSWfsYkwLSAI6jx5SMQ1996Gawnzl65woxvtY+pJZpjfSzMFXdfdo?=
 =?us-ascii?Q?I1rC4VsYQlVSeT5lBKm4fDc+azJwGOsalb7XTAVg+igWjjVDfm3+goLptjBg?=
 =?us-ascii?Q?Qw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CY8PR11MB7364.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c9efa102-a957-4c44-0214-08dc5dfebcc3
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Apr 2024 10:19:44.9280
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: J5REHUYtmJMdoF0AcWK0gxgwYsQ4EXPHBjnm40YKiiFSUgJbggcUnB6hosUcVeDxlHrHAmsnAIipAwLsBTFnqKJv8pBTZ5e5iy7Twst0xXU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7315
X-OriginatorOrg: intel.com


> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Thursday, April 11, 2024 3:59 AM
> To: D, Lakshmi Sowjanya <lakshmi.sowjanya.d@intel.com>;
> jstultz@google.com; giometti@enneenne.com; corbet@lwn.net; linux-
> kernel@vger.kernel.org
> Cc: x86@kernel.org; netdev@vger.kernel.org; linux-doc@vger.kernel.org; in=
tel-
> wired-lan@lists.osuosl.org; andriy.shevchenko@linux.intel.com; Dong, Eddi=
e
> <eddie.dong@intel.com>; Hall, Christopher S <christopher.s.hall@intel.com=
>;
> Brandeburg, Jesse <jesse.brandeburg@intel.com>; davem@davemloft.net;
> alexandre.torgue@foss.st.com; joabreu@synopsys.com;
> mcoquelin.stm32@gmail.com; perex@perex.cz; linux-sound@vger.kernel.org;
> Nguyen, Anthony L <anthony.l.nguyen@intel.com>;
> peter.hilber@opensynergy.com; N, Pandith <pandith.n@intel.com>; Mohan,
> Subramanian <subramanian.mohan@intel.com>; T R, Thejesh Reddy
> <thejesh.reddy.t.r@intel.com>; D, Lakshmi Sowjanya
> <lakshmi.sowjanya.d@intel.com>
> Subject: Re: [PATCH v6 09/11] pps: generators: Add PPS Generator TIO Driv=
er
>=20
> On Wed, Apr 10 2024 at 17:18, lakshmi.sowjanya.d@intel.com wrote:
> > +static bool pps_generate_next_pulse(struct pps_tio *tio, ktime_t
> > +expires) {
> > +	u64 art;
> > +
> > +	if (!ktime_real_to_base_clock(expires, CSID_X86_ART, &art)) {
> > +		pps_tio_disable(tio);
> > +		return false;
> > +	}
> > +
> > +	pps_compv_write(tio, art - ART_HW_DELAY_CYCLES);
> > +	return true;
> > +}
> > +
> > +static enum hrtimer_restart hrtimer_callback(struct hrtimer *timer) {
> > +	struct pps_tio *tio =3D container_of(timer, struct pps_tio, timer);
> > +	ktime_t expires, now;
> > +	u32 event_count;
> > +
> > +	guard(spinlock)(&tio->lock);
> > +
> > +	/* Check if any event is missed. If an event is missed, TIO will be
> disabled*/
> > +	event_count =3D pps_tio_read(tio, TIOEC);
> > +	if (!tio->prev_count && tio->prev_count =3D=3D event_count)
> > +		goto err;
> > +	tio->prev_count =3D event_count;
> > +	expires =3D hrtimer_get_expires(timer);
> > +	now =3D ktime_get_real();
> > +
> > +	if (now - expires < SAFE_TIME_NS) {
> > +		if (!pps_generate_next_pulse(tio, expires + SAFE_TIME_NS))
> > +			goto err;
> > +	}
>=20
> If the hrtimer callback is invoked late so that now - expires is >=3D SAF=
E_TIME_NS
> then this just forwards the timer and tries again.

Yes we will introduce a return HRTIMER_NORESTART if the time is expired.

>=20
> This lacks any form of explanation why this is correct as obviously there=
 will be a
> missing pulse, no?

We had added an event count check to detect the missed pulse(i.e if we had =
programmed an expired time).=20
Timed I/O hardware has an event count register to log the number of pulses =
generated.

>=20
> > +	hrtimer_forward(timer, now, NSEC_PER_SEC / 2);
> > +	return HRTIMER_RESTART;
> > +err:
> > +	dev_err(tio->dev, "Event missed, Disabling Timed I/O");
> > +	pps_tio_disable(tio);
>=20
> Why does this disable it again? The failure path in
> pps_generate_next_pulse() does so already, no?

will remove disabling twice in the next version of patchset.

>=20
> > +	return HRTIMER_NORESTART;
> > +}
> > +
>=20
> Thanks,
>=20
>         tglx