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 9504FCDB47E for ; Fri, 13 Oct 2023 22:49:25 +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=K5XHsgebwO01GQSZV6eCcP1VaiX5fiy44IjujqK3W84=; b=DGuE8oi0fWkJGOPvBdkzbRNMxv H00LqUx28cspA5aUpLu2CPFAu0aJ0w3nv8UZRhMM7M4F8ACFMeTyXBTtCrtbP1I51ImAeqYZrsVo1 61aWNliGZDcv0LXsQKhuy/LbEmcZZ77JuXJCLA0JkRyBOOSBfI+ic732ci8P5lHQ/EJw3mNx8wthP 8g/6LGbHSudwVB/W9x2yDOdb8KhV8Vc4H7YSYzOCBxdFypeIgk24bf4VLI545OrJckm1D8j/loOvj luxGIW0lQW9wRutgFj3inQ5ilTiLC+/Mi7FRNFNtpv5Flt3SHLyLTERXfWSl1Hl7JYY37gyeUoGs9 89GiOkjw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qrQxi-004PNP-0h; Fri, 13 Oct 2023 22:48:54 +0000 Received: from mail-ua1-x930.google.com ([2607:f8b0:4864:20::930]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qrQxb-004PDm-1o for linux-arm-kernel@lists.infradead.org; Fri, 13 Oct 2023 22:48:52 +0000 Received: by mail-ua1-x930.google.com with SMTP id a1e0cc1a2514c-7b07548b084so937576241.1 for ; Fri, 13 Oct 2023 15:48:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1697237322; x=1697842122; 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=M7+2V0YlS8ZDKtKvIbNHqNSalMBU1GrhT1IpSjeCQTw=; b=Pt4is7+fsqeCCDB90Uvt9oe0TxelA8Zw8UrzGxsC/FAUPRGADPh34Lm0rV/nIrqHW0 RBOB0fBq2nSegfUt8TAB67SjoCD7ujXP0SKnexQ7Mt4w6Sy6Rgh6XouRVj/L4u2n1gwJ Ye5O9RHDYuOMsx8kcQwC1owEe7+F6Ed6glkJkldQkExlP3EM8EpPzeAvvuXkcFrsIJzh lawOPRii1vAK3CRx6QPWifm20zukeuoX79I7v7CVMlAYN9431kC9av+3D7h/rhK/Pqvp vQyCk5comBL2l7w4Fy73zLjxK1ogqXIkbM+zX+weXZ9x+fQ4bUJfpFIUY6blQ4G/Pmks bQQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697237322; x=1697842122; 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=M7+2V0YlS8ZDKtKvIbNHqNSalMBU1GrhT1IpSjeCQTw=; b=WqA2oHnwz1gk7SHnxDfOOtYI4wgtCTkz98+o/9nzqv6aWwH7q2/RjlpHsgpjZsiHPw XwjejQ8oorSBcnWnkoAhwYfAPxQ2I4cQiIybs5JKxgZnrkVkTh2rsVFT2udsWxo+PlSB Jjg04E6MAxQ4KAVotEerMauLbBVfYgeWmXZxHmoN6NJwBygihwvH5uyaSw3TojjzfDew wO83A0CLk3abpHD3Z67FVt+EAmqgYQKozbcnyoweZnHukx6m1HfyidfZNyRrLqosZHCY Sf2uITLEXYT4yvQcTNXyyCpJ5zEsqtoS/eNLMpDHrb2NMbD/EY7U1B+MUd5cUdoeJuZg 3vOw== X-Gm-Message-State: AOJu0YyHQbwmxp5ihwy+Sd8yZAeV3eYhMoG08a29HUlqLzTKFMdkX9n0 y9zdWiJPEt5XkLBwnGSP8DFXhg== X-Google-Smtp-Source: AGHT+IF7jx2I05oes/GMbZo7J3cDYSck7gV+oLZiWc5TQCd+sjI62q0d0Zzk234dBluUdpibqgZoIA== X-Received: by 2002:a67:f4d3:0:b0:452:66a7:1ac with SMTP id s19-20020a67f4d3000000b0045266a701acmr27391344vsn.6.1697237322128; Fri, 13 Oct 2023 15:48:42 -0700 (PDT) Received: from fedora (072-189-067-006.res.spectrum.com. [72.189.67.6]) by smtp.gmail.com with ESMTPSA id bi6-20020a05610234e600b0045255981807sm551389vsb.0.2023.10.13.15.48.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Oct 2023 15:48:41 -0700 (PDT) Date: Fri, 13 Oct 2023 18:48:39 -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 5/6] counter: stm32-timer-cnt: populate capture channels and check encoder Message-ID: References: <20230922143920.3144249-1-fabrice.gasnier@foss.st.com> <20230922143920.3144249-6-fabrice.gasnier@foss.st.com> MIME-Version: 1.0 In-Reply-To: <20230922143920.3144249-6-fabrice.gasnier@foss.st.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231013_154847_631590_ABE1CB74 X-CRM114-Status: GOOD ( 28.58 ) 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="===============9174165656411925917==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============9174165656411925917== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="DOR58DHOIf3n8XuL" Content-Disposition: inline --DOR58DHOIf3n8XuL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 22, 2023 at 04:39:19PM +0200, Fabrice Gasnier wrote: > This is a precursor patch to support capture channels on all possible > channels and stm32 timer types. Original driver was intended to be used > only as quadrature encoder and simple counter on internal clock. >=20 > So, add ch3 and ch4 definition. Also add a check on encoder capability, > so the driver may be probed for timer instances without encoder feature. > This way, all timers may be used as simple counter on internal clock, > starting from here. Hi Fabrice, Let's split the encoder capability probing code, detect number of channels code, and channel introduction code to their own patches in order to simplify things. > Encoder capability is retrieved by using the timer index (originally in > stm32-timer-trigger driver and dt-bindings). The need to keep backward > compatibility with existing device tree lead to parse aside trigger node. > Add diversity as STM32 timers with capture feature may have either 4, 2, > 1 or no cc (capture/compare) channels. >=20 > Signed-off-by: Fabrice Gasnier I think this patch is more complicated than it needs to be. > @@ -400,13 +558,47 @@ static int stm32_timer_cnt_probe(struct platform_de= vice *pdev) > priv->clk =3D ddata->clk; > priv->max_arr =3D ddata->max_arr; > =20 > + ret =3D stm32_timer_cnt_probe_encoder(pdev, priv); > + if (ret) > + return ret; > + > + stm32_timer_cnt_detect_channels(pdev, priv); > + > counter->name =3D dev_name(dev); > counter->parent =3D dev; > counter->ops =3D &stm32_timer_cnt_ops; > - counter->counts =3D &stm32_counts; > counter->num_counts =3D 1; > - counter->signals =3D stm32_signals; > - counter->num_signals =3D ARRAY_SIZE(stm32_signals); Keep this the same. > + > + /* > + * Handle diversity for stm32 timers features. For now encoder is found= with > + * advanced timers or gp timers with 4 channels. Timers with less chann= els > + * doesn't support encoder. > + */ > + switch (priv->nchannels) { > + case 4: > + if (priv->has_encoder) > + counter->counts =3D &stm32_counts_enc_4ch; > + else > + counter->counts =3D &stm32_counts_4ch; > + counter->signals =3D stm32_signals; > + counter->num_signals =3D ARRAY_SIZE(stm32_signals); > + break; > + case 2: > + counter->counts =3D &stm32_counts_2ch; > + counter->signals =3D stm32_signals; > + counter->num_signals =3D 3; /* clock, ch1 and ch2 */ > + break; > + case 1: > + counter->counts =3D &stm32_counts_1ch; > + counter->signals =3D stm32_signals; > + counter->num_signals =3D 2; /* clock, ch1 */ > + break; > + default: > + counter->counts =3D &stm32_counts; > + counter->signals =3D stm32_signals; > + counter->num_signals =3D 1; /* clock */ > + break; > + } Rather than adjusting the number of counts and signals, keep the configuration static and use a single stm32_counts array. The reason is that in the Counter subsystem paradigm Signals do not necessary correlate to specific hardware signals but are rather an abstract representation of the device behavior at a high level. In other words, a Synapse with an action mode set to COUNTER_SYNAPSE_ACTION_NONE can be viewed as representing a Signal that does not affect the Count (i.e. in this case equivalent to an unconnected line). What you'll need to do instead is check priv->nchannels during stm32_action_read and stm32_count_function_read calls in order to return the correct synapse action and count function for the particular channels configuration you have. In stm32_count_function_write you would return an -EINVAL (maybe -EOPNOTSUPP would be better?) when the channels configuration does not support a particular count function. William Breathitt Gray --DOR58DHOIf3n8XuL Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQSNN83d4NIlKPjon7a1SFbKvhIjKwUCZSnJRwAKCRC1SFbKvhIj K0moAQD7wGvZVP3oXlqW7ObHpexVDKnM0MGwBchWQQSXSCVj9QEApHtanCqbJLLZ mWbFf52y1xmscsdAdL7XhWiNLieTCQM= =2nuH -----END PGP SIGNATURE----- --DOR58DHOIf3n8XuL-- --===============9174165656411925917== 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 --===============9174165656411925917==--