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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B2029CDB47E for ; Fri, 13 Oct 2023 21:22:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=gEqwdOteDiVVvAbFvcR4XGZ/s/Mj8cuUZ/+Zg2hXF3w=; b=ncvqgKJjuMmLS+Jx1Z0wMMpxFI FjO47oTYJT5wgvdLvr4gEtr5m8HZXpG/EzTz1/6Ax5YtzKe6JZvESx0w4A0FocbyLYhnqeh44Fy5s nSpHrhVTRoYvMPwUVIaGJLBYb0bBLNYagl6LSQFvT1Bsb/fi/JZh2mAvKDYZOnBeOFH5aFk0XLuRn Ake0QDveByU5t3iVxzh3Nbn1Hal0lthRe8ugnXvwDdVREAZ2lXGPaimBTFWrVFBvkunsBp8DQkIJ1 OqGjT1f07/BVbBfF212iak5737hbe8aCgAq3aqkLBMVdoqIW1wGoX1R9HiZZdijBip6m6C6pilCOi YynDpMqg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qrPbt-004ISo-1K; Fri, 13 Oct 2023 21:22:17 +0000 Received: from mail-vs1-xe2a.google.com ([2607:f8b0:4864:20::e2a]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qrPbp-004IRo-0N for linux-arm-kernel@lists.infradead.org; Fri, 13 Oct 2023 21:22:15 +0000 Received: by mail-vs1-xe2a.google.com with SMTP id ada2fe7eead31-4527d7f7305so1086259137.1 for ; Fri, 13 Oct 2023 14:22:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1697232124; x=1697836924; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=R+mGCb1GSkk1Ef5TQGY3Gg3lLuwymx7Pt5o2k9FQ6c8=; b=R4BUCTb+XBDdo2p5D9qaQjwKpXepgRVO4BAphmTp+wGrRzZSHe41T22ptZiw3cLyN4 yBN2u12Txj32lzpu9UF6Z45NwSCFwPPDfIL1LjnzdsLGVmKNuOJRY6Tpr3zyFZCQcYOW 9rQuYVygzg7qBLkKIkBa2uhNSTNDErw5UHgPcZNNpcerGuXPfYD0PxVVOhkrcQWHpylx kzZ3xsX9ps4zFK/kDp0dLMeAQyoBoEb+IPL8E33Rh71k1Qh9tixjzBb6D4xO0+Gi6vnR CDLL2jR8vNoKjBF9++t0Q9eedC+fjWeNVNHqWJu9ERgYjPU5yjWrTMLQ/EU6lMMBHMpA tPFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697232124; x=1697836924; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=R+mGCb1GSkk1Ef5TQGY3Gg3lLuwymx7Pt5o2k9FQ6c8=; b=YW2BTgrebHBjSDKkY46gWOKZaIuA328Yu54oEeUOMEnUrO51mlQhXOCCEToV/B3l40 AZLuddw3ZQ3RtiWV2y/soevkXShjUxx+U4s/eySDzWymovXlCgXV1zIX6OlAy9O1oGoN 9qdYM1dHV0pjVpfrzh2qhOgT1KLimSYhs+15mVQiieTSsVNWGSHch3sYeOz711UIEzrF 3IbF4cLS6SsYu08ARZrmrs9T/tXJgtCHWXyT8pczZvpber55hlgAQc0Cgf9zOTYC+PbL /Kf9NcBUsRlcHMY1nXeo6hH+Oce1y6zXOweyR8/L7Di7odfkR0OnrouCBitAH/bFm5bs JIsg== X-Gm-Message-State: AOJu0Yw1qDVBr6WSoitcn2yT0CMbO+W1EpiHwKHo9y3Yfwrlwm/oOqEr bSolgwIH1qlkrYp7VbHI0RNzzA== X-Google-Smtp-Source: AGHT+IEDG3uuEFHlb6qFT5yY62dBqu4rI5BjhHaq7CBLG1/YuI1ErPh3D3atVeOV1iBu350IVW/QGg== X-Received: by 2002:a05:6102:a52:b0:457:adcf:2f9e with SMTP id i18-20020a0561020a5200b00457adcf2f9emr7988232vss.24.1697232124483; Fri, 13 Oct 2023 14:22:04 -0700 (PDT) Received: from fedora (072-189-067-006.res.spectrum.com. [72.189.67.6]) by smtp.gmail.com with ESMTPSA id n25-20020a67e459000000b0044d4e63aa03sm518904vsm.25.2023.10.13.14.22.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Oct 2023 14:22:03 -0700 (PDT) Date: Fri, 13 Oct 2023 17:22:01 -0400 From: William Breathitt Gray To: Fabrice Gasnier Cc: lee@kernel.org, alexandre.torgue@foss.st.com, linux-iio@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 4/6] counter: stm32-timer-cnt: introduce clock signal Message-ID: References: <20230922143920.3144249-1-fabrice.gasnier@foss.st.com> <20230922143920.3144249-5-fabrice.gasnier@foss.st.com> MIME-Version: 1.0 In-Reply-To: <20230922143920.3144249-5-fabrice.gasnier@foss.st.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231013_142213_153925_B3F2C33F X-CRM114-Status: GOOD ( 33.52 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: multipart/mixed; boundary="===============1475698049026851818==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============1475698049026851818== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="nThdsgLAWTvUMCvU" Content-Disposition: inline --nThdsgLAWTvUMCvU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 22, 2023 at 04:39:18PM +0200, Fabrice Gasnier wrote: > Introduce the internal clock signal, used to count when in simple rising > function. Define signal ids, to improve readability. Also add the > "frequency" attribute for the clock signal, and "prescaler" for the > counter. Hi Fabrice, Split the addition of "frequency" and "prescaler" extensions each to their own respective patches so we can keep the clock signal introduction code separate (useful in case we need to git bisect an issue in the future). >=20 > Whit this patch, signal action reports consistent state when "increase" Looks like a typo there for the first word. > function is used, and the counting frequency: > $ echo increase > function > $ grep -H "" signal*_action > signal0_action:rising edge > signal1_action:none > signal2_action:none > $ echo 1 > enable > $ cat count > 25425 > $ cat count > 44439 > $ cat ../signal0/frequency > 208877930 Since you're fixing this description anyway, indent the shell example by four spaces to make it stand-out and look nice. >=20 > Signed-off-by: Fabrice Gasnier > --- > drivers/counter/stm32-timer-cnt.c | 84 ++++++++++++++++++++++++++++--- > 1 file changed, 76 insertions(+), 8 deletions(-) >=20 > diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-ti= mer-cnt.c > index 668e9d1061d3..11c66876b213 100644 > --- a/drivers/counter/stm32-timer-cnt.c > +++ b/drivers/counter/stm32-timer-cnt.c > @@ -21,6 +21,10 @@ > #define TIM_CCER_MASK (TIM_CCER_CC1P | TIM_CCER_CC1NP | \ > TIM_CCER_CC2P | TIM_CCER_CC2NP) > =20 > +#define STM32_CLOCK_SIG 0 > +#define STM32_CH1_SIG 1 > +#define STM32_CH2_SIG 2 > + > struct stm32_timer_regs { > u32 cr1; > u32 cnt; > @@ -216,11 +220,44 @@ static int stm32_count_enable_write(struct counter_= device *counter, > return 0; > } > =20 > +static int stm32_count_prescaler_read(struct counter_device *counter, > + struct counter_count *count, u64 *prescaler) > +{ > + struct stm32_timer_cnt *const priv =3D counter_priv(counter); > + u32 psc; > + > + regmap_read(priv->regmap, TIM_PSC, &psc); > + > + *prescaler =3D psc + 1; > + > + return 0; > +} > + > +static int stm32_count_prescaler_write(struct counter_device *counter, > + struct counter_count *count, u64 prescaler) > +{ > + struct stm32_timer_cnt *const priv =3D counter_priv(counter); > + u32 psc; > + > + if (!prescaler || prescaler > MAX_TIM_PSC + 1) > + return -ERANGE; > + > + psc =3D prescaler - 1; > + > + return regmap_write(priv->regmap, TIM_PSC, psc); > +} > + > static struct counter_comp stm32_count_ext[] =3D { > COUNTER_COMP_DIRECTION(stm32_count_direction_read), > COUNTER_COMP_ENABLE(stm32_count_enable_read, stm32_count_enable_write), > COUNTER_COMP_CEILING(stm32_count_ceiling_read, > stm32_count_ceiling_write), > + COUNTER_COMP_COUNT_U64("prescaler", stm32_count_prescaler_read, > + stm32_count_prescaler_write), > +}; > + > +static const enum counter_synapse_action stm32_clock_synapse_actions[] = =3D { > + COUNTER_SYNAPSE_ACTION_RISING_EDGE, > }; > =20 > static const enum counter_synapse_action stm32_synapse_actions[] =3D { > @@ -243,25 +280,31 @@ static int stm32_action_read(struct counter_device = *counter, > switch (function) { > case COUNTER_FUNCTION_INCREASE: > /* counts on internal clock when CEN=3D1 */ > - *action =3D COUNTER_SYNAPSE_ACTION_NONE; > + if (synapse->signal->id =3D=3D STM32_CLOCK_SIG) > + *action =3D COUNTER_SYNAPSE_ACTION_RISING_EDGE; > + else > + *action =3D COUNTER_SYNAPSE_ACTION_NONE; > return 0; > case COUNTER_FUNCTION_QUADRATURE_X2_A: > /* counts up/down on TI1FP1 edge depending on TI2FP2 level */ > - if (synapse->signal->id =3D=3D count->synapses[0].signal->id) > + if (synapse->signal->id =3D=3D STM32_CH1_SIG) > *action =3D COUNTER_SYNAPSE_ACTION_BOTH_EDGES; > else > *action =3D COUNTER_SYNAPSE_ACTION_NONE; > return 0; > case COUNTER_FUNCTION_QUADRATURE_X2_B: > /* counts up/down on TI2FP2 edge depending on TI1FP1 level */ > - if (synapse->signal->id =3D=3D count->synapses[1].signal->id) > + if (synapse->signal->id =3D=3D STM32_CH2_SIG) > *action =3D COUNTER_SYNAPSE_ACTION_BOTH_EDGES; > else > *action =3D COUNTER_SYNAPSE_ACTION_NONE; > return 0; > case COUNTER_FUNCTION_QUADRATURE_X4: > /* counts up/down on both TI1FP1 and TI2FP2 edges */ > - *action =3D COUNTER_SYNAPSE_ACTION_BOTH_EDGES; > + if (synapse->signal->id =3D=3D STM32_CH1_SIG || synapse->signal->id = =3D=3D STM32_CH2_SIG) > + *action =3D COUNTER_SYNAPSE_ACTION_BOTH_EDGES; > + else > + *action =3D COUNTER_SYNAPSE_ACTION_NONE; > return 0; > default: > return -EINVAL; > @@ -276,27 +319,52 @@ static const struct counter_ops stm32_timer_cnt_ops= =3D { > .action_read =3D stm32_action_read, > }; > =20 > +static int stm32_count_clk_get_freq(struct counter_device *counter, > + struct counter_signal *signal, u64 *freq) > +{ > + struct stm32_timer_cnt *const priv =3D counter_priv(counter); > + > + *freq =3D clk_get_rate(priv->clk); > + > + return 0; > +} > + > +static struct counter_comp stm32_count_clock_ext[] =3D { > + COUNTER_COMP_SIGNAL_U64("frequency", stm32_count_clk_get_freq, NULL), > +}; > + > static struct counter_signal stm32_signals[] =3D { > { > - .id =3D 0, > + .id =3D STM32_CLOCK_SIG, This will break userspace programs that expect signal0 to represent the "Channel 1" Signal. Instead, add the clock Signal to the end of the stm32_signals array so that the existing Signals are not reordered. Although the clock signal may be represented by an id of 0 on the device, the Counter API Signal id is a more abstract concept so it does not necessarily need to match the device's numbering scheme. Side note: you can keep the "id" member value the same if you want. The Counter subsystem uses the array position to index the Signals; the "id" value is ignored by the subsystem in that regard, and is rather provided for the driver's internal use so it can differentiate between the Signals. > + .name =3D "Clock Signal", > + .ext =3D stm32_count_clock_ext, > + .num_ext =3D ARRAY_SIZE(stm32_count_clock_ext), > + }, > + { > + .id =3D STM32_CH1_SIG, > .name =3D "Channel 1" > }, > { > - .id =3D 1, > + .id =3D STM32_CH2_SIG, > .name =3D "Channel 2" > } > }; > =20 > static struct counter_synapse stm32_count_synapses[] =3D { > + { > + .actions_list =3D stm32_clock_synapse_actions, > + .num_actions =3D ARRAY_SIZE(stm32_clock_synapse_actions), > + .signal =3D &stm32_signals[STM32_CLOCK_SIG] > + }, Same reordering issue here as the previous comment. William Breathitt Gray > { > .actions_list =3D stm32_synapse_actions, > .num_actions =3D ARRAY_SIZE(stm32_synapse_actions), > - .signal =3D &stm32_signals[0] > + .signal =3D &stm32_signals[STM32_CH1_SIG] > }, > { > .actions_list =3D stm32_synapse_actions, > .num_actions =3D ARRAY_SIZE(stm32_synapse_actions), > - .signal =3D &stm32_signals[1] > + .signal =3D &stm32_signals[STM32_CH2_SIG] > } > }; > =20 > --=20 > 2.25.1 >=20 --nThdsgLAWTvUMCvU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQSNN83d4NIlKPjon7a1SFbKvhIjKwUCZSm0+QAKCRC1SFbKvhIj K14mAQCsrkk0dopWfu2DIM32HZjaXkMVreIXmMbKQUCNm/CGHAD8DgYtTUX1wt0d hXp3+ANc8PzIAH3BiKBDg7a/K4n97Ao= =BVjf -----END PGP SIGNATURE----- --nThdsgLAWTvUMCvU-- --===============1475698049026851818== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============1475698049026851818==--