Received: by 2002:ac0:aed5:0:0:0:0:0 with SMTP id t21csp5853909imb; Fri, 8 Mar 2019 03:58:54 -0800 (PST) X-Google-Smtp-Source: APXvYqyOVMkivIzfVW4L7/pXdVKwnzj+OukBfxDrUchHPvwJVPP2cyHFkfCH76np+ZazTN6NgSYN X-Received: by 2002:aa7:928d:: with SMTP id j13mr18315988pfa.112.1552046334105; Fri, 08 Mar 2019 03:58:54 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1552046334; cv=none; d=google.com; s=arc-20160816; b=ojT7ufF+NW+wG53We5uixrleroNxycZxPxMLyGBTBYVdfLOmUWUGNh/GLnxMo7iRGp D4UZyh/tN4JVAaXu0n7HiHsLboEn+vYE7aC6nk1BkqabuNcIuaIaJDPwvaXjyJC7sQRg 6usbmMq1Kc0Wwjr5aUo313i7OU3o1qkKMmZGfVYBl7tyaFmwdz+xUNZ5fyGOObOYo4Cj XVbt1mJ3hHEiJhnggj6U1/Q7F9E6xDYD9CYzSy/MugZBMN4LpwEXlOCfUkXb5LIaicnz cmxYwhQ2TnbNYlDc9S46IaLB8DWqnlpzGuGZQWlpowkx+8clIElgDCW2fr++eaztWIfu xASQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date; bh=Xi/zjYdQD3pnwzCk2T8Q4ZFjNBa4Lv636+SXwidJcv4=; b=BWWZVJwtdgChCE+TTI/VXWdSCG+ufPxdb9HGnOCs2JONrR6k9gEssd8soqtKmT8VWu RYUU7pWtHapVmfqHqMtMt0vhm87qNyEplmYuUsZryW07z+JTx4PNj/zxy9HYl6x8T6Id jznS7UiIY/r7S5YX762bTA8VwqWsMNHouIVZuw/Qorcc5FgFSEzXxElNCMaboyP45MDj 7M+ZRP8JILPoWvcmioj2GNUeEtBAw0og008yDBO+OVSuNoiarHJcpnwwsVFTmZzjaJth tUaCWJZTBo/J5DekavWTDeLqzQQwofsltSGdbfO045xhmJuXmTHauWuouoVg8Zus0o9w AKNw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q19si6888800pff.177.2019.03.08.03.58.38; Fri, 08 Mar 2019 03:58:54 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726530AbfCHL5f (ORCPT + 99 others); Fri, 8 Mar 2019 06:57:35 -0500 Received: from metis.ext.pengutronix.de ([85.220.165.71]:49841 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726042AbfCHL5e (ORCPT ); Fri, 8 Mar 2019 06:57:34 -0500 Received: from pty.hi.pengutronix.de ([2001:67c:670:100:1d::c5]) by metis.ext.pengutronix.de with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1h2E80-0003WO-6a; Fri, 08 Mar 2019 12:57:28 +0100 Received: from ukl by pty.hi.pengutronix.de with local (Exim 4.89) (envelope-from ) id 1h2E7y-0006fT-Mh; Fri, 08 Mar 2019 12:57:26 +0100 Date: Fri, 8 Mar 2019 12:57:26 +0100 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Yash Shah Cc: Palmer Dabbelt , linux-pwm@vger.kernel.org, linux-riscv@lists.infradead.org, Thierry Reding , robh+dt@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Sachin Ghadi , Paul Walmsley , kernel@pengutronix.de Subject: Re: [PATCH v8 2/2] pwm: sifive: Add a driver for SiFive SoC PWM Message-ID: <20190308115726.smgooacsr37fmxwg@pengutronix.de> References: <1551437599-29509-1-git-send-email-yash.shah@sifive.com> <1551437599-29509-3-git-send-email-yash.shah@sifive.com> <20190307152745.kaiv6q4ygf2apmuv@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c5 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Fri, Mar 08, 2019 at 04:59:36PM +0530, Yash Shah wrote: > On Thu, Mar 7, 2019 at 8:57 PM Uwe Kleine-K?nig > wrote: > > > +static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *dev, > > > + struct pwm_state *state) > > > +{ > > > + struct pwm_sifive_ddata *pwm = pwm_sifive_chip_to_ddata(chip); > > > + unsigned int duty_cycle, x; > > > + u32 frac; > > > + struct pwm_state cur_state; > > > + bool enabled; > > > + int ret = 0; > > > + unsigned long num; > > > + > > > + if (state->polarity != PWM_POLARITY_INVERSED) > > > + return -EINVAL; > > > + > > > + mutex_lock(&pwm->lock); > > > + pwm_get_state(dev, &cur_state); > > > + enabled = cur_state.enabled; > > > + > > > + if (state->period != cur_state.period) { > > > > Did you test this with more than one consumer? For sure the following > > should work: > > > > pwm1 = pwm_get(.. the first ..); > > pwm_apply_state(pwm1, { .enabled = true, .period = 10000000, .... }); > > > > pwm2 = pwm_get(.. the second ..); > > pwm_apply_state(pwm2, { .enabled = true, .period = 10000000, .... }); > > > > but for the second pwm_apply_state() run state->period is likely not > > exactly 10000000. > > Yes, I have tested multiple consumers using sysfs interface. It is working. Can you provide details about your testing here? What is the parent clk rate? Which settings did you test? Can you confirm my claim that the above sequence would fail or point out my error in reasoning? > > > + x = 1U << PWM_SIFIVE_CMPWIDTH; > > > + num = (u64)duty_cycle * x + x / 2; > > > + frac = div_u64(num, state->period); > > > > I don't understand the "+ x / 2" part. Should this better be > > "+ state->period / 2"? Something like > > This eqn is as per your comments against v5 of this patch series. > frac = (duty_cycle * (1 << PWM_SIFIVE_CMPWIDTH) + (1 << > PWM_SIFIVE_CMPWIDTH) / 2) / period; OK, then not only the code is wrong, but also my suggestion was. :-) > > #define div_u64_round(a, b) ({typeof(b) __b = b; div_u64(a + __b / 2, __b)}) > > > > would make this less error prone. This still stands. It makes it easier to get the code right and makes it easier to understand. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |