From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 74BC1C433E0 for ; Tue, 19 Jan 2021 09:22:45 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 20A3320867 for ; Tue, 19 Jan 2021 09:22:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 20A3320867 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Moxvt8j4CVxh1xupxaktZOzebe7WS6+Gjm5XnO1rXkA=; b=DsEvJUcODMLd+3+0lsDGEDV9A CQ2N9whK63z3jzLlKirFo4EaZyP5jFM1BYhmJNm8SpElLvOITy39Cry0tGHTGsNEUBeD6C4rmNgQW a1DlWLVxn+zkHZXgOUmO9rsYK53QUEVLEhZayT/PSl6TE1A+a8WMYE7dyZazCLNRx/993Zb7uCRds Pw6d9LAfJ9QSnefT7AdFCKH+Oy50Gq4y4IbsmiTSn0Fr/aK20ngBYVXJF1QuPPm9rxDJBm2nlvIZF v3CPsJvK58fhuFZ1C04PR237oemGg6c6z30JT1/dLdP15QyHYadQY4omFq3lp7/vp237MjHZVXOV0 deGMHVcBA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l1nCP-0007pm-5O; Tue, 19 Jan 2021 09:21:17 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l1nCM-0007oh-NI for linux-arm-kernel@lists.infradead.org; Tue, 19 Jan 2021 09:21:15 +0000 Received: from dude.hi.pengutronix.de ([2001:67c:670:100:1d::7]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l1nBZ-0004WN-1x; Tue, 19 Jan 2021 10:20:25 +0100 Received: from ore by dude.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1l1nBW-0001Dn-E1; Tue, 19 Jan 2021 10:20:22 +0100 Date: Tue, 19 Jan 2021 10:20:22 +0100 From: Oleksij Rempel To: William Breathitt Gray Subject: Re: [PATCH v6 3/5] counter: Add character device interface Message-ID: <20210119092022.GA14502@pengutronix.de> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 10:05:44 up 35 days, 17:13, 74 users, load average: 0.14, 0.75, 1.44 User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::7 X-SA-Exim-Mail-From: ore@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210119_042114_851849_D41768C4 X-CRM114-Status: GOOD ( 23.11 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arm-kernel@lists.infradead.org, kamel.bouhara@bootlin.com, gwendal@chromium.org, a.fatoum@pengutronix.de, david@lechnology.com, linux-iio@vger.kernel.org, patrick.havelange@essensium.com, linux-stm32@st-md-mailman.stormreply.com, alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org, Dan Carpenter , mcoquelin.stm32@gmail.com, David Jander , fabrice.gasnier@st.com, syednwaris@gmail.com, kernel@pengutronix.de, jic23@kernel.org, alexandre.torgue@st.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, Nov 22, 2020 at 03:29:54PM -0500, William Breathitt Gray wrote: > This patch introduces a character device interface for the Counter > subsystem. Device data is exposed through standard character device read > operations. Device data is gathered when a Counter event is pushed by > the respective Counter device driver. Configuration is handled via ioctl > operations on the respective Counter character device node. > > Cc: David Lechner > Cc: Gwendal Grignou > Cc: Dan Carpenter > Signed-off-by: William Breathitt Gray > --- Hello William, the series looks quite interesting, we have some thoughts... see below: [...] > +/** > + * counter_push_event - queue event for userspace reading > + * @counter: pointer to Counter structure > + * @event: triggered event > + * @channel: event channel > + * > + * Note: If no one is watching for the respective event, it is silently > + * discarded. > + * > + * RETURNS: > + * 0 on success, negative error number on failure. > + */ > +int counter_push_event(struct counter_device *const counter, const u8 event, > + const u8 channel) > +{ > + struct counter_event ev = {0}; > + unsigned int copied = 0; > + unsigned long flags; > + struct counter_event_node *event_node; > + struct counter_comp_node *comp_node; > + int err = 0; > + > + ev.timestamp = ktime_get_ns(); > + ev.watch.event = event; > + ev.watch.channel = channel; > + > + raw_spin_lock_irqsave(&counter->events_lock, flags); > + > + /* Search for event in the list */ > + list_for_each_entry(event_node, &counter->events_list, l) > + if (event_node->event == event && > + event_node->channel == channel) > + break; > + > + /* If event is not in the list */ > + if (&event_node->l == &counter->events_list) > + goto exit_early; > + > + /* Read and queue relevant comp for userspace */ > + list_for_each_entry(comp_node, &event_node->comp_list, l) { > + err = counter_get_data(counter, comp_node, &ev.value); > + if (err) > + goto exit_early; > + > + ev.watch.component = comp_node->component; > + > + copied += kfifo_put(&counter->events, ev); We want to calculate the frequency of some IRQ pulses in user space and counter values with time stamps really fits well here. As the pulses are from a physical system (rotating wheel), they will only change at a certain rate. We want to have the possibility to read from the counter device less often, we intentionally want to skip (meaning miss) events. When reading we're interested in the newest events. The kfifo implements a "tail" drop FIFO, which means new values are added at the end, and if the FIFO is full, they are dropped. We need a "head" drop FIFO which discards the oldest events, keeping only the recent ones. As far as we know, kfifo doesn't offer a head drop mode, but I think this can be added. [...] > struct counter_device { > const char *name; > @@ -270,12 +270,20 @@ struct counter_device { > > int id; > struct device dev; > + struct cdev chrdev; > + raw_spinlock_t events_lock; > + struct list_head events_list; > + struct list_head next_events_list; > + DECLARE_KFIFO(events, struct counter_event, 64); Do you plan to make the size of the FIFO configurable? regards, Oleksij & Marc -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel