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 X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 30F8FC5DF60 for ; Thu, 7 Nov 2019 06:51:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0CD6521882 for ; Thu, 7 Nov 2019 06:51:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727187AbfKGGvc (ORCPT ); Thu, 7 Nov 2019 01:51:32 -0500 Received: from metis.ext.pengutronix.de ([85.220.165.71]:47425 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725763AbfKGGvb (ORCPT ); Thu, 7 Nov 2019 01:51:31 -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.92) (envelope-from ) id 1iSbdX-0001kX-O4; Thu, 07 Nov 2019 07:51:19 +0100 Received: from ukl by pty.hi.pengutronix.de with local (Exim 4.89) (envelope-from ) id 1iSbdW-0007qU-3c; Thu, 07 Nov 2019 07:51:18 +0100 Date: Thu, 7 Nov 2019 07:51:18 +0100 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: =?iso-8859-1?Q?Cl=E9ment_P=E9ron?= Cc: Thierry Reding , Rob Herring , Mark Rutland , Maxime Ripard , Chen-Yu Tsai , Philipp Zabel , linux-pwm@vger.kernel.org, devicetree , linux-arm-kernel , linux-kernel , Jernej Skrabec Subject: Re: [PATCH v3 4/7] pwm: sun4i: Add support to output source clock directly Message-ID: <20191107065118.j4s5cghj4ark7sql@pengutronix.de> References: <20191105131456.32400-1-peron.clem@gmail.com> <20191105131456.32400-5-peron.clem@gmail.com> <20191105145659.ffezqntodsys4phn@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 Clément, On Wed, Nov 06, 2019 at 10:24:39PM +0100, Clément Péron wrote: > On Tue, 5 Nov 2019 at 15:57, Uwe Kleine-König > wrote: > > On Tue, Nov 05, 2019 at 02:14:53PM +0100, Clément Péron wrote: > > > + bypass = state->enabled && > > > + (state->period * clk_rate >= NSEC_PER_SEC) && > > > > This is too coarse. With state->period = 1000000 this is fulfilled > > (unless the multiplication overflows). > > Sorry, misunderstood the previous mail > > What about something like this ? > ((state->period - 1) * clk_rate <= NSEC_PER_SEC) && > ((state->period + 1) * clk_rate >= NSEC_PER_SEC) && > ((state->duty_cycle - 1) * 2 <= state->period) && > ((state->duty_cycle + 1) * 2 >= state->period); > > We are sure that the user is looking for a PWM around the OSC with a > 50% duty cycle ? This again is too strict. The general policy to fulfill a request is: 1) provide the longest possible period not bigger than requested 2) provide the longest possible duty cycle not bigger than requested 3) if possible complete the currently running period before switching and don't return to the user before the new setting is active. Document the behaviour prominently because the code (usually) doesn't allow to understand the hardware's features here. 4) A disabled PWM should output the inactive level And then there is a corner case: If the user requests .duty_cycle = 0, .enabled = 1 it is ok to provide .enabled = 0 iff otherwise 0% isn't possible. So the right check for bypass is: state->period * clk_rate >= NSEC_PER_SEC && state->period * clk_rate < whatevercanbereachedwithoutbypass && state->duty_cycle * clk_rate * 2 >= NSEC_PER_SEC Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |