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 625E4C4321E for ; Thu, 1 Dec 2022 11:12:45 +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-Transfer-Encoding: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-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=utb7nAMiabjA406iDrmbzdRnku18xRU75wvfnG+gsRA=; b=YrVRAPWBxSIR5A qD0XiKmZ9DxE+MH6Oie+O6RxAC2sWqRuFv3//V5dS2OK5aC0+nnDk1JvUDc9r5YXipWFivrfWJRBg 22Vq47jXfY9k3wZJTjEhglT4Qfvmy952D1G9OQQM9OZVONYBGWAr+BLQRvMpuLpLDBFTCOQUQejHM his4SVEiMQpcfb2PPcWOmYfcgiT1TollXrjX2zgDhdHlz183fxM3lgMkwPVu/S43atZH7fwKAKcFR CFk3qtXh2XyqhIc3tObGTEhOLayKgqVo/zABC4qIajSkAqJ7HWwZ5KJ8d8QLNPTMLWbGEXF2QLBPB 4rsJWxDRQLDwPtmzJz6A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0hUc-006yfS-9q; Thu, 01 Dec 2022 11:12:38 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0hUS-006yYM-Bw; Thu, 01 Dec 2022 11:12:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1669893148; x=1701429148; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=oqwp1prmygFipaS7+KydzauQtsm23Yws4T/2+sBogyI=; b=riUR8tErQKZFZIyCxW8RmecrzsRsQUlVi1qxefFltVZYsFJliNRovVAQ MNvuyhl5LcQ9AX5w7ZoSxSxuxYVXGZn1eLp3ezNMpJGdhcaP85GGuqSOS i/hYBw/4AHv9ixTclMTVwoajd10EjQxA1zd86Bw5GHKiwDPEYfYDP4ssI Ozk0t3tr4V7lz4XJwcIrOygoDGz/XFbFyX8tS0a4UkjxmjediSEF8tWnQ VOIhEf3/URMek9i4IBaWfQWq1eP/E+FHZW1SvQeKuifyifL4V6eqUO/0q NULMKVK8t3FNTFfV6m+TGdqXZYtfkHdkLoM7ovNAITULzl9zp3PRp3g+A A==; X-IronPort-AV: E=Sophos;i="5.96,209,1665471600"; d="scan'208";a="202161679" Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa1.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 01 Dec 2022 04:12:19 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12; Thu, 1 Dec 2022 04:12:19 -0700 Received: from wendy (10.10.115.15) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12 via Frontend Transport; Thu, 1 Dec 2022 04:12:10 -0700 Date: Thu, 1 Dec 2022 11:11:51 +0000 From: Conor Dooley To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= CC: Thierry Reding , Linus Walleij , Bartosz Golaszewski , "Douglas Anderson" , Pavel Machek , Claudiu Beznea , Nicolas Ferre , Alexandre Belloni , Ray Jui , Scott Branden , "Broadcom internal kernel review list" , Benson Leung , Guenter Roeck , Shawn Guo , Sascha Hauer , "Pengutronix Kernel Team" , Fabio Estevam , "NXP Linux Team" , Kevin Hilman , "Jerome Brunet" , Martin Blumenstingl , Matthias Brugger , Florian Fainelli , "Heiko Stuebner" , Palmer Dabbelt , "Paul Walmsley" , Michael Walle , "Orson Zhai" , Baolin Wang , Chunyan Zhang , Fabrice Gasnier , Maxime Coquelin , Alexandre Torgue , Chen-Yu Tsai , Samuel Holland , Hammer Hsieh , Nobuhiro Iwamatsu , Sean Anderson , Michal Simek , Bjorn Andersson , Stephen Boyd , Matthias Kaehlcke , Satya Priya , , , , , , , , , , , , , , Steven Rostedt , Masami Hiramatsu , Marijn Suijten Subject: Re: [PATCH v2 00/11] pwm: Allow .get_state to fail Message-ID: References: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221201_031228_456121_2E5A5586 X-CRM114-Status: GOOD ( 22.26 ) X-BeenThere: linux-riscv@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: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org Hey Uwe! On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-K=F6nig wrote: > Hello, > = > I forgot about this series and was remembered when I talked to Conor > Dooley about how .get_state() should behave in an error case. In the context of "my" driver, get_state() the proposal was to fail with -ETIMEDOUT rather than block a caller, potentially, for seconds or report a potentially "random" state. Specifically, values writen to the registers that control the PWM duty cycle are not visible to the cpu until the changes have propagated to the waveform at the start of a new period. The timeout would occur if the bit that signifies that the "shadow registers" contain a value which has not yet propagated. This bit is per PWM "controller" and not per PWM channel. Returning from apply() without waiting, possibly for seconds, for the writes to become visible could cause get_state() to see anything between the new and old states, inclusive! If anyone cares at all, the discussion is here: https://lore.kernel.org/linux-pwm/20221110093512.333881-1-conor.dooley@micr= ochip.com/T/#m800eeabad29067940a5684e54106fd0bb7261944 > In v1 Thierry had the concern: > = > | That raises the question about what to do in these cases. If we return > | an error, that could potentially throw off consumers. So perhaps the > | closest would be to return a disabled PWM? > | Or perhaps it'd be up to the > | consumer to provide some fallback configuration for invalidly configured > | or unconfigured PWMs. > = > .get_state() is only called in pwm_device_request on a pwm_state that a > consumer might see. Before my series a consumer might have seen a > partial modified pwm_state (because .get_state() might have modified > .period, then stumbled and returned silently). The last patch ensures > that this partial modification isn't given out to the consumer. Instead > they now see the same as if .get_state wasn't implemented at all. Getting the same thing as if get_state() did not exist seems preferable to me in this context than "lying" and pretending that a PWM is disabled or potentially inconsistent reports from get_state() that I mentioned above. TL;DR, I quite like the ability to return an error and not mislead the caller. Thanks for sending a v2 of this so quickly :) Conor. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv 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 911A4C43217 for ; Thu, 1 Dec 2022 11:12:48 +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-Transfer-Encoding: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-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ObVLHa04eej3Z2IJT8OIVLva+mqIW6Wz5H/8vUJCrTk=; b=LwHeW8DDgtYoXk N9L5TCmT0FdsHzrY93FQ6OJulzNyW5CYyyx+PRE3oI4sqxff3pNWnOS5wWkrZIdIktUMC2YQtDn8Y 1l9O6arZD1JfxfUnIimnMIGPL1KjJdfNcbGuzWbqCQg5cyMiHe2+BmTSkxHc/BCCYGhyNIWSppqKl gu5NH5GueKHPN/jxgT4pCdlKkkJoWE5NhweCIX76Z8D+cXT7FVBDCTM7w4O2+97TSps7QPv/GCmFt yacX8XJU+GjlAKRBVUXlYUIPI5/tvZSWsdjGuaapJGvGuqyEHZDMTtbyZ0UZKhftE5cY9kxknZQ7z sYJPUdBEjiNMjMYErSPQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0hUb-006yfG-HH; Thu, 01 Dec 2022 11:12:37 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0hUS-006yYM-Bw; Thu, 01 Dec 2022 11:12:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1669893148; x=1701429148; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=oqwp1prmygFipaS7+KydzauQtsm23Yws4T/2+sBogyI=; b=riUR8tErQKZFZIyCxW8RmecrzsRsQUlVi1qxefFltVZYsFJliNRovVAQ MNvuyhl5LcQ9AX5w7ZoSxSxuxYVXGZn1eLp3ezNMpJGdhcaP85GGuqSOS i/hYBw/4AHv9ixTclMTVwoajd10EjQxA1zd86Bw5GHKiwDPEYfYDP4ssI Ozk0t3tr4V7lz4XJwcIrOygoDGz/XFbFyX8tS0a4UkjxmjediSEF8tWnQ VOIhEf3/URMek9i4IBaWfQWq1eP/E+FHZW1SvQeKuifyifL4V6eqUO/0q NULMKVK8t3FNTFfV6m+TGdqXZYtfkHdkLoM7ovNAITULzl9zp3PRp3g+A A==; X-IronPort-AV: E=Sophos;i="5.96,209,1665471600"; d="scan'208";a="202161679" Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa1.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 01 Dec 2022 04:12:19 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12; Thu, 1 Dec 2022 04:12:19 -0700 Received: from wendy (10.10.115.15) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12 via Frontend Transport; Thu, 1 Dec 2022 04:12:10 -0700 Date: Thu, 1 Dec 2022 11:11:51 +0000 From: Conor Dooley To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= CC: Thierry Reding , Linus Walleij , Bartosz Golaszewski , "Douglas Anderson" , Pavel Machek , Claudiu Beznea , Nicolas Ferre , Alexandre Belloni , Ray Jui , Scott Branden , "Broadcom internal kernel review list" , Benson Leung , Guenter Roeck , Shawn Guo , Sascha Hauer , "Pengutronix Kernel Team" , Fabio Estevam , "NXP Linux Team" , Kevin Hilman , "Jerome Brunet" , Martin Blumenstingl , Matthias Brugger , Florian Fainelli , "Heiko Stuebner" , Palmer Dabbelt , "Paul Walmsley" , Michael Walle , "Orson Zhai" , Baolin Wang , Chunyan Zhang , Fabrice Gasnier , Maxime Coquelin , Alexandre Torgue , Chen-Yu Tsai , Samuel Holland , Hammer Hsieh , Nobuhiro Iwamatsu , Sean Anderson , Michal Simek , Bjorn Andersson , Stephen Boyd , Matthias Kaehlcke , Satya Priya , , , , , , , , , , , , , , Steven Rostedt , Masami Hiramatsu , Marijn Suijten Subject: Re: [PATCH v2 00/11] pwm: Allow .get_state to fail Message-ID: References: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221201_031228_456121_2E5A5586 X-CRM114-Status: GOOD ( 22.26 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org Hey Uwe! On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-K=F6nig wrote: > Hello, > = > I forgot about this series and was remembered when I talked to Conor > Dooley about how .get_state() should behave in an error case. In the context of "my" driver, get_state() the proposal was to fail with -ETIMEDOUT rather than block a caller, potentially, for seconds or report a potentially "random" state. Specifically, values writen to the registers that control the PWM duty cycle are not visible to the cpu until the changes have propagated to the waveform at the start of a new period. The timeout would occur if the bit that signifies that the "shadow registers" contain a value which has not yet propagated. This bit is per PWM "controller" and not per PWM channel. Returning from apply() without waiting, possibly for seconds, for the writes to become visible could cause get_state() to see anything between the new and old states, inclusive! If anyone cares at all, the discussion is here: https://lore.kernel.org/linux-pwm/20221110093512.333881-1-conor.dooley@micr= ochip.com/T/#m800eeabad29067940a5684e54106fd0bb7261944 > In v1 Thierry had the concern: > = > | That raises the question about what to do in these cases. If we return > | an error, that could potentially throw off consumers. So perhaps the > | closest would be to return a disabled PWM? > | Or perhaps it'd be up to the > | consumer to provide some fallback configuration for invalidly configured > | or unconfigured PWMs. > = > .get_state() is only called in pwm_device_request on a pwm_state that a > consumer might see. Before my series a consumer might have seen a > partial modified pwm_state (because .get_state() might have modified > .period, then stumbled and returned silently). The last patch ensures > that this partial modification isn't given out to the consumer. Instead > they now see the same as if .get_state wasn't implemented at all. Getting the same thing as if get_state() did not exist seems preferable to me in this context than "lying" and pretending that a PWM is disabled or potentially inconsistent reports from get_state() that I mentioned above. TL;DR, I quite like the ability to return an error and not mislead the caller. Thanks for sending a v2 of this so quickly :) Conor. _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from esa.microchip.iphmx.com (esa.microchip.iphmx.com [68.232.153.233]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 429C21116; Thu, 1 Dec 2022 11:13:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1669893209; x=1701429209; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=oqwp1prmygFipaS7+KydzauQtsm23Yws4T/2+sBogyI=; b=Z/3h6Q9k4V3XsrkV+Qy0vu4OobfONq3lCoCg24OVBEBRSy6GwKRerOpZ ZTbojb1/37XOFwAwv56sMJg8rAesYRIX/NPK4fpAl8LFI852dW6I9jjaG jEHASMqE9eaqUaGTOfIcNnlThTFuyffEbe3/zH+zewg39FNB2bj9Mwov2 GUQHm8Lg96uUHthOuT2LMpQ1wosGwzkpVTLYh9XfOuNbwYlddBVHgEXH/ 9m1/dsOM40nLCwWMPkQiOGDXd9eaRbSptCuMWop+nP+fIvK7FFnyJSOqS ve/G6ExNqKSGvsSRpvA6XlsHFduswL1QojDwGL4KfBtR/+JuLcS7rzguk g==; X-IronPort-AV: E=Sophos;i="5.96,209,1665471600"; d="scan'208";a="202161679" Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa1.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 01 Dec 2022 04:12:19 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12; Thu, 1 Dec 2022 04:12:19 -0700 Received: from wendy (10.10.115.15) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12 via Frontend Transport; Thu, 1 Dec 2022 04:12:10 -0700 Date: Thu, 1 Dec 2022 11:11:51 +0000 From: Conor Dooley To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= CC: Thierry Reding , Linus Walleij , Bartosz Golaszewski , "Douglas Anderson" , Pavel Machek , Claudiu Beznea , Nicolas Ferre , Alexandre Belloni , Ray Jui , Scott Branden , "Broadcom internal kernel review list" , Benson Leung , Guenter Roeck , Shawn Guo , Sascha Hauer , "Pengutronix Kernel Team" , Fabio Estevam , "NXP Linux Team" , Kevin Hilman , "Jerome Brunet" , Martin Blumenstingl , Matthias Brugger , Florian Fainelli , "Heiko Stuebner" , Palmer Dabbelt , "Paul Walmsley" , Michael Walle , "Orson Zhai" , Baolin Wang , Chunyan Zhang , Fabrice Gasnier , Maxime Coquelin , Alexandre Torgue , Chen-Yu Tsai , Samuel Holland , Hammer Hsieh , Nobuhiro Iwamatsu , Sean Anderson , Michal Simek , Bjorn Andersson , Stephen Boyd , Matthias Kaehlcke , Satya Priya , , , , , , , , , , , , , , Steven Rostedt , Masami Hiramatsu , Marijn Suijten Subject: Re: [PATCH v2 00/11] pwm: Allow .get_state to fail Message-ID: References: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> Hey Uwe! On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-König wrote: > Hello, > > I forgot about this series and was remembered when I talked to Conor > Dooley about how .get_state() should behave in an error case. In the context of "my" driver, get_state() the proposal was to fail with -ETIMEDOUT rather than block a caller, potentially, for seconds or report a potentially "random" state. Specifically, values writen to the registers that control the PWM duty cycle are not visible to the cpu until the changes have propagated to the waveform at the start of a new period. The timeout would occur if the bit that signifies that the "shadow registers" contain a value which has not yet propagated. This bit is per PWM "controller" and not per PWM channel. Returning from apply() without waiting, possibly for seconds, for the writes to become visible could cause get_state() to see anything between the new and old states, inclusive! If anyone cares at all, the discussion is here: https://lore.kernel.org/linux-pwm/20221110093512.333881-1-conor.dooley@microchip.com/T/#m800eeabad29067940a5684e54106fd0bb7261944 > In v1 Thierry had the concern: > > | That raises the question about what to do in these cases. If we return > | an error, that could potentially throw off consumers. So perhaps the > | closest would be to return a disabled PWM? > | Or perhaps it'd be up to the > | consumer to provide some fallback configuration for invalidly configured > | or unconfigured PWMs. > > .get_state() is only called in pwm_device_request on a pwm_state that a > consumer might see. Before my series a consumer might have seen a > partial modified pwm_state (because .get_state() might have modified > .period, then stumbled and returned silently). The last patch ensures > that this partial modification isn't given out to the consumer. Instead > they now see the same as if .get_state wasn't implemented at all. Getting the same thing as if get_state() did not exist seems preferable to me in this context than "lying" and pretending that a PWM is disabled or potentially inconsistent reports from get_state() that I mentioned above. TL;DR, I quite like the ability to return an error and not mislead the caller. Thanks for sending a v2 of this so quickly :) Conor. 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 5C20DC43217 for ; Thu, 1 Dec 2022 11:12:46 +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-Transfer-Encoding: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-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=X7TnJfehA6E/Zt6r65wF2KMoWETNA6NlhKEML4MWfs0=; b=Mz8DCXmQlQX4f/ 7njD21sJMrATAzZQC4Df8ffxB77/Aqv7usA3IU2pcmQreD2ogEvzP1+KbjlcYLhVfE5eRcfV6Yaa4 d2a3awSW2CcPJ4O1+GznH0X6qyDuPUCnxmD0d9cw1T1ngazMvwtdmpiXwiE+ae11qkMvryIp3ynpl q1EHn6TTluec1w9mcluoiZOmAAo2Ug240JTzmwuJi06FPbxzN0UMutA7aylhz22hSDp5u+6qDHeXJ O2WOPNZwpcbkSd4AvdoURp66Jk+WQva1l1K4sCpaxc+ptYhc9SaeBByCnOcQTkYrVqXzCR/NSMIkx TVspQdOD71CRwMEvZPAw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0hUX-006ydd-14; Thu, 01 Dec 2022 11:12:33 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0hUS-006yYM-Bw; Thu, 01 Dec 2022 11:12:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1669893148; x=1701429148; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=oqwp1prmygFipaS7+KydzauQtsm23Yws4T/2+sBogyI=; b=riUR8tErQKZFZIyCxW8RmecrzsRsQUlVi1qxefFltVZYsFJliNRovVAQ MNvuyhl5LcQ9AX5w7ZoSxSxuxYVXGZn1eLp3ezNMpJGdhcaP85GGuqSOS i/hYBw/4AHv9ixTclMTVwoajd10EjQxA1zd86Bw5GHKiwDPEYfYDP4ssI Ozk0t3tr4V7lz4XJwcIrOygoDGz/XFbFyX8tS0a4UkjxmjediSEF8tWnQ VOIhEf3/URMek9i4IBaWfQWq1eP/E+FHZW1SvQeKuifyifL4V6eqUO/0q NULMKVK8t3FNTFfV6m+TGdqXZYtfkHdkLoM7ovNAITULzl9zp3PRp3g+A A==; X-IronPort-AV: E=Sophos;i="5.96,209,1665471600"; d="scan'208";a="202161679" Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa1.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 01 Dec 2022 04:12:19 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12; Thu, 1 Dec 2022 04:12:19 -0700 Received: from wendy (10.10.115.15) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12 via Frontend Transport; Thu, 1 Dec 2022 04:12:10 -0700 Date: Thu, 1 Dec 2022 11:11:51 +0000 From: Conor Dooley To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= CC: Thierry Reding , Linus Walleij , Bartosz Golaszewski , "Douglas Anderson" , Pavel Machek , Claudiu Beznea , Nicolas Ferre , Alexandre Belloni , Ray Jui , Scott Branden , "Broadcom internal kernel review list" , Benson Leung , Guenter Roeck , Shawn Guo , Sascha Hauer , "Pengutronix Kernel Team" , Fabio Estevam , "NXP Linux Team" , Kevin Hilman , "Jerome Brunet" , Martin Blumenstingl , Matthias Brugger , Florian Fainelli , "Heiko Stuebner" , Palmer Dabbelt , "Paul Walmsley" , Michael Walle , "Orson Zhai" , Baolin Wang , Chunyan Zhang , Fabrice Gasnier , Maxime Coquelin , Alexandre Torgue , Chen-Yu Tsai , Samuel Holland , Hammer Hsieh , Nobuhiro Iwamatsu , Sean Anderson , Michal Simek , Bjorn Andersson , Stephen Boyd , Matthias Kaehlcke , Satya Priya , , , , , , , , , , , , , , Steven Rostedt , Masami Hiramatsu , Marijn Suijten Subject: Re: [PATCH v2 00/11] pwm: Allow .get_state to fail Message-ID: References: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221201_031228_456121_2E5A5586 X-CRM114-Status: GOOD ( 22.26 ) X-BeenThere: linux-amlogic@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: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org Hey Uwe! On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-K=F6nig wrote: > Hello, > = > I forgot about this series and was remembered when I talked to Conor > Dooley about how .get_state() should behave in an error case. In the context of "my" driver, get_state() the proposal was to fail with -ETIMEDOUT rather than block a caller, potentially, for seconds or report a potentially "random" state. Specifically, values writen to the registers that control the PWM duty cycle are not visible to the cpu until the changes have propagated to the waveform at the start of a new period. The timeout would occur if the bit that signifies that the "shadow registers" contain a value which has not yet propagated. This bit is per PWM "controller" and not per PWM channel. Returning from apply() without waiting, possibly for seconds, for the writes to become visible could cause get_state() to see anything between the new and old states, inclusive! If anyone cares at all, the discussion is here: https://lore.kernel.org/linux-pwm/20221110093512.333881-1-conor.dooley@micr= ochip.com/T/#m800eeabad29067940a5684e54106fd0bb7261944 > In v1 Thierry had the concern: > = > | That raises the question about what to do in these cases. If we return > | an error, that could potentially throw off consumers. So perhaps the > | closest would be to return a disabled PWM? > | Or perhaps it'd be up to the > | consumer to provide some fallback configuration for invalidly configured > | or unconfigured PWMs. > = > .get_state() is only called in pwm_device_request on a pwm_state that a > consumer might see. Before my series a consumer might have seen a > partial modified pwm_state (because .get_state() might have modified > .period, then stumbled and returned silently). The last patch ensures > that this partial modification isn't given out to the consumer. Instead > they now see the same as if .get_state wasn't implemented at all. Getting the same thing as if get_state() did not exist seems preferable to me in this context than "lying" and pretending that a PWM is disabled or potentially inconsistent reports from get_state() that I mentioned above. TL;DR, I quite like the ability to return an error and not mislead the caller. Thanks for sending a v2 of this so quickly :) Conor. _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 97F44C4321E for ; Thu, 1 Dec 2022 11:19:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EF2A610E5B9; Thu, 1 Dec 2022 11:19:31 +0000 (UTC) X-Greylist: delayed 426 seconds by postgrey-1.36 at gabe; Thu, 01 Dec 2022 11:19:28 UTC Received: from esa.microchip.iphmx.com (esa.microchip.iphmx.com [68.232.153.233]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6F25010E5B9 for ; Thu, 1 Dec 2022 11:19:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1669893568; x=1701429568; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=oqwp1prmygFipaS7+KydzauQtsm23Yws4T/2+sBogyI=; b=sLBYdtnWZJQic8fXO4xR/Im76T36jErpd1NAJ60vn8UO4xzDj/T4XXAa +oe+nBTeyY1kId54v7zKGAhI/kvAN2u+pVSWGFQxMm9sb/gR8YHVEnmy9 G+SyGTI4tFniGb56iV565Fkj2e31gyr/Nq0hM1AZNbuClUNTu7xaat7YT BhYl0gzy+KeTS6RmNjcMePZ3Bl1hH6RtzuXh99b7GCklWgwRdYKoXqKEs MN+OwN6t/aQfSY/03HVz2JNyUlzZUNH9CFYQSdd7oKg1jAGY+tUBZV35r YpQZBzvlQwpLWX3yzYFPZ5XglQEdianKRU9QsE2FXgEyVspB9Lnco0Eug A==; X-IronPort-AV: E=Sophos;i="5.96,209,1665471600"; d="scan'208";a="202161679" Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa1.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 01 Dec 2022 04:12:19 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12; Thu, 1 Dec 2022 04:12:19 -0700 Received: from wendy (10.10.115.15) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.12 via Frontend Transport; Thu, 1 Dec 2022 04:12:10 -0700 Date: Thu, 1 Dec 2022 11:11:51 +0000 From: Conor Dooley To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Subject: Re: [PATCH v2 00/11] pwm: Allow .get_state to fail Message-ID: References: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221130152148.2769768-1-u.kleine-koenig@pengutronix.de> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Alexandre Belloni , Alexandre Torgue , dri-devel@lists.freedesktop.org, Nicolas Ferre , Thierry Reding , Satya Priya , Pavel Machek , Guenter Roeck , Marijn Suijten , Nobuhiro Iwamatsu , linux-riscv@lists.infradead.org, linux-leds@vger.kernel.org, Jerome Brunet , chrome-platform@lists.linux.dev, Florian Fainelli , Samuel Holland , Sean Anderson , Kevin Hilman , Bartosz Golaszewski , Michal Simek , linux-stm32@st-md-mailman.stormreply.com, Hammer Hsieh , linux-rockchip@lists.infradead.org, Chen-Yu Tsai , Matthias Kaehlcke , Broadcom internal kernel review list , NXP Linux Team , Orson Zhai , linux-sunxi@lists.linux.dev, linux-pwm@vger.kernel.org, Maxime Coquelin , Martin Blumenstingl , Ray Jui , Sascha Hauer , Steven Rostedt , Stephen Boyd , linux-gpio@vger.kernel.org, Fabrice Gasnier , linux-mediatek@lists.infradead.org, linux-rpi-kernel@lists.infradead.org, Baolin Wang , Paul Walmsley , Matthias Brugger , linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, Scott Branden , Bjorn Andersson , Douglas Anderson , Michael Walle , Palmer Dabbelt , Masami Hiramatsu , Pengutronix Kernel Team , Chunyan Zhang , Shawn Guo , Claudiu Beznea Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hey Uwe! On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-König wrote: > Hello, > > I forgot about this series and was remembered when I talked to Conor > Dooley about how .get_state() should behave in an error case. In the context of "my" driver, get_state() the proposal was to fail with -ETIMEDOUT rather than block a caller, potentially, for seconds or report a potentially "random" state. Specifically, values writen to the registers that control the PWM duty cycle are not visible to the cpu until the changes have propagated to the waveform at the start of a new period. The timeout would occur if the bit that signifies that the "shadow registers" contain a value which has not yet propagated. This bit is per PWM "controller" and not per PWM channel. Returning from apply() without waiting, possibly for seconds, for the writes to become visible could cause get_state() to see anything between the new and old states, inclusive! If anyone cares at all, the discussion is here: https://lore.kernel.org/linux-pwm/20221110093512.333881-1-conor.dooley@microchip.com/T/#m800eeabad29067940a5684e54106fd0bb7261944 > In v1 Thierry had the concern: > > | That raises the question about what to do in these cases. If we return > | an error, that could potentially throw off consumers. So perhaps the > | closest would be to return a disabled PWM? > | Or perhaps it'd be up to the > | consumer to provide some fallback configuration for invalidly configured > | or unconfigured PWMs. > > .get_state() is only called in pwm_device_request on a pwm_state that a > consumer might see. Before my series a consumer might have seen a > partial modified pwm_state (because .get_state() might have modified > .period, then stumbled and returned silently). The last patch ensures > that this partial modification isn't given out to the consumer. Instead > they now see the same as if .get_state wasn't implemented at all. Getting the same thing as if get_state() did not exist seems preferable to me in this context than "lying" and pretending that a PWM is disabled or potentially inconsistent reports from get_state() that I mentioned above. TL;DR, I quite like the ability to return an error and not mislead the caller. Thanks for sending a v2 of this so quickly :) Conor.