From patchwork Sat Feb 24 11:29:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leif Middelschulte X-Patchwork-Id: 13570379 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 71533C48BF6 for ; Sat, 24 Feb 2024 11:29:23 +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:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=baBjVkF4ENBP2ZsA1SMInHBg5DWLqm6EHJb8Sdv4/Tk=; b=WjO4WF49hMW3Y0 4+K1YFujNm/WdEJ8SdN6OdTiHSidL789ubJJwH/2Sg/bFHsg7WZTwsK/AplT04AW9PWaBaKlyg0aN jg4ALsrtgjL9tSOQlEWDCBIncgF1oZQPqoa+5F6chIA2G0bmw0f5LokDrUOxrxr0Ztvh8dDMkLL8f H/0j3ZLiW/Vtb9DGBWlCLBeitk46NV6ddp5m3X52DwnLzHATxb0tZQMyuBrN7E6PZ157qe3Mf3KYH F3E6yuUY5/UfZVyyUMtg2tNXFq8leTbaFS4K005aeWM/My5FZxvLeaPcDejAe1aMgwVeQuDFAc5Pv ZRgjOxw6SN4tbLnM/2PA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rdqDR-0000000CjBq-4C1U; Sat, 24 Feb 2024 11:29:14 +0000 Received: from mail-ej1-x62e.google.com ([2a00:1450:4864:20::62e]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rdqDO-0000000CjAL-4AZZ for linux-arm-kernel@lists.infradead.org; Sat, 24 Feb 2024 11:29:12 +0000 Received: by mail-ej1-x62e.google.com with SMTP id a640c23a62f3a-a3122b70439so168549166b.3 for ; Sat, 24 Feb 2024 03:29:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708774149; x=1709378949; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=JYAho2X90qK7tOrS84lGLoReElvfwN9Vc1VtqG59PbA=; b=dD8wEUniWm3AaS0gHg8JcLYXWge8tkRZDTuchAZi0GTgrn74HifeqzxZ5n7y+uelgF CpXf9gGHucArcE6grOuZxYzVzHpQb8qdc3kV7thjnkykI7J05FViaFrroeOUyUpM5iom XYEPziLwpmKbmuwe0r5ZfXEi20VZfQ6WQ79BreQDOXcVwoSypoVO8EUyjaEMARNMmpoC TVAtINI/XIZxUUnaPnar6miknOIVgJpVYDCoSgAqDDqhAm4HQK20TVwD8FOn0s+CbJkT 9r4Z1++lWg2Akx0SUo/mAM4RmjktW6CGyr5Znrm7FT1Jvlwd6fD9lm+ggBMqVBXBrY4o Zj/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708774149; x=1709378949; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JYAho2X90qK7tOrS84lGLoReElvfwN9Vc1VtqG59PbA=; b=EV+eNiZtGHIwRl4sopa9m5ph8dPxDCmO6RLml/WeVFDkwfifmQaBao0/u4ZT4awb7Y GBuTGjC7hHly0pLCplzCTNigTQ+rBC0dZTAntRbqtx5Jd0makNdUVqGY2i4SQlfS395T tbeE9ARVp2gxv9u4gqImkbU6jiGZBpLs4AflZ86PuG7922FY+MZ5BY4ARhCGpptquOfY iN8FYNz0zAmS0n4cC6IZXH4PBDLs1IXHNpeZzhFYXuJKDvqIubtYribnx8F/LzlFVo+v YNoqREoiqFyYzg0bTmjs1LkmN1spFnJxhR9q/WgDnfb6V2XJOZPVNEtau4nEBgUUVM8B KbzA== X-Forwarded-Encrypted: i=1; AJvYcCXbrNnzMQa9dQrMl44iulqaW+DrdUKq7FFRiK59Wk5vzK0XRjVQU4sYvvZVJOgSddMU3Q8m4zO357h56IkUalfuKHFThuiqmpIkSm56VCjfZLdcmso= X-Gm-Message-State: AOJu0YxaIznlnpIATXcIrKHr4UcUSgMAoZCYL1vUUKZdKGaQdGSyxv0E 3gkRwDxg9wUsfRIykp280OSyO0LYItVY/bFCwK8pi7daIzw4scXp X-Google-Smtp-Source: AGHT+IFcZLVXuxBLLhhHFucSHV65YF5jlbWZN8OnRT43E3B5IERyuxH8H2URJWvmWc9ECMJuLJGqQg== X-Received: by 2002:a17:906:2b05:b0:a40:65ea:61 with SMTP id a5-20020a1709062b0500b00a4065ea0061mr1475681ejg.46.1708774148756; Sat, 24 Feb 2024 03:29:08 -0800 (PST) Received: from localhost.localdomain (p5dc68d61.dip0.t-ipconnect.de. [93.198.141.97]) by smtp.gmail.com with ESMTPSA id s1-20020a170906060100b00a3d9e6e9983sm507181ejb.174.2024.02.24.03.29.08 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Sat, 24 Feb 2024 03:29:08 -0800 (PST) From: Leif Middelschulte X-Google-Original-From: Leif Middelschulte To: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team Cc: Leif Middelschulte , linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 1/2] pwm: imx27: fix race condition .apply,.get_state Date: Sat, 24 Feb 2024 12:29:00 +0100 Message-Id: <20240224112902.55539-1-Leif.Middelschulte@gmail.com> X-Mailer: git-send-email 2.39.3 (Apple Git-145) In-Reply-To: <20230906154215.4ikrrbx4xgx2nmu5@pengutronix.de> References: <20230906154215.4ikrrbx4xgx2nmu5@pengutronix.de> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240224_032911_070645_CB3514FD X-CRM114-Status: GOOD ( 27.27 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Leif Middelschulte With CONFIG_PWM_DEBUG=y after writing a value to the PWMSAR register in .apply(), the register is read in .get_state(). Unless a period completed in the meantime, this read yields the previously used duty cycle configuration. As the PWM_DEBUG code applies the read out configuration for testing purposes this effectively undoes the intended effect by rewriting the previous hardware state. Note that this change merely implements a sensible heuristic. The i.MX has a 4 slot FIFO to configure the duty cycle. This FIFO cannot be read back in its entirety. The "write x then read back x from hw" semantics are therefore not easily applicable. With this change, the .get_state() function tries to wait for some stabilization in the FIFO (empty state). In this state it keeps applying the last value written to the sample register. Signed-off-by: Leif Middelschulte --- drivers/pwm/pwm-imx27.c | 55 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c index 7d9bc43f12b0..cb564460b79c 100644 --- a/drivers/pwm/pwm-imx27.c +++ b/drivers/pwm/pwm-imx27.c @@ -75,6 +75,7 @@ (x)) + 1) #define MX3_PWM_SWR_LOOP 5 +#define MX3_PWM_FIFOAV_EMPTY_LOOP 4 /* PWMPR register value of 0xffff has the same effect as 0xfffe */ #define MX3_PWMPR_MAX 0xfffe @@ -118,6 +119,31 @@ static void pwm_imx27_clk_disable_unprepare(struct pwm_imx27_chip *imx) clk_disable_unprepare(imx->clk_ipg); } +static int pwm_imx27_wait_fifo_empty(struct pwm_chip *chip, + struct pwm_device *pwm) +{ + struct pwm_imx27_chip *imx = to_pwm_imx27_chip(chip); + struct device *dev = chip->dev; + unsigned int period_ms = DIV_ROUND_UP_ULL(pwm->state.period, NSEC_PER_MSEC); + int tries = MX3_PWM_FIFOAV_EMPTY_LOOP; + int fifoav, previous_fifoav = INT_MAX; + u32 sr; + + while (tries--) { + sr = readl(imx->mmio_base + MX3_PWMSR); + fifoav = FIELD_GET(MX3_PWMSR_FIFOAV, sr); + if (fifoav == MX3_PWMSR_FIFOAV_EMPTY) + return 0; + /* if the FIFO value does not decrease, there is another problem */ + if (previous_fifoav == fifoav) + break; + previous_fifoav = fifoav; + msleep(period_ms); + } + + return -EAGAIN; +} + static int pwm_imx27_get_state(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state) { @@ -161,10 +187,35 @@ static int pwm_imx27_get_state(struct pwm_chip *chip, * PWMSAR can be read only if PWM is enabled. If the PWM is disabled, * use the cached value. */ - if (state->enabled) + if (state->enabled) { + /* + * From the i.MX PWM reference manual: + * "A read on the sample register yields the current FIFO value that + * is being used, or will be used, by the PWM for generation on the + * output signal. Therefore, a write and a subsequent read on the + * sample register may result in different values being obtained." + * Furthermore: + * "When a new value is written, the duty cycle changes after the + * current period is over." + * Note "changes" vs. "changes to the given value"! + * Finally: + * "The PWM will run at the last set duty-cycle setting if all the + * values of the FIFO has been utilized, until the FIFO is reloaded + * or the PWM is disabled." + * Try to be at least a bit more deterministic about which value is + * read by waiting until the FIFO is empty. In this state the last/most + * recently pushed sample (duty cycle) value is continuously applied. + * Beware that this approach is still racy, as a new value could have + * been supplied and a period expired between the call of the wait + * function and the subsequent readl. + */ + ret = pwm_imx27_wait_fifo_empty(chip, pwm); + if (ret) + return ret; val = readl(imx->mmio_base + MX3_PWMSAR); - else + } else { val = imx->duty_cycle; + } tmp = NSEC_PER_SEC * (u64)(val) * prescaler; state->duty_cycle = DIV_ROUND_UP_ULL(tmp, pwm_clk);