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=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS 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 AED43C433E6 for ; Sun, 3 Jan 2021 17:05:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8080320936 for ; Sun, 3 Jan 2021 17:05:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727727AbhACREz (ORCPT ); Sun, 3 Jan 2021 12:04:55 -0500 Received: from mail.pqgruber.com ([52.59.78.55]:57830 "EHLO mail.pqgruber.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726952AbhACREy (ORCPT ); Sun, 3 Jan 2021 12:04:54 -0500 Received: from workstation.tuxnet (213-47-165-233.cable.dynamic.surfer.at [213.47.165.233]) by mail.pqgruber.com (Postfix) with ESMTPSA id 1EE9DC68B88; Sun, 3 Jan 2021 18:04:12 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1609693452; bh=5dqq/DsJwfVHsgRwxYYIC0zb6W8E9Tab6yx3XUhiPgk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZrhMgvCZoiIvHxr4i0+l4IoL7AL5J4mPkOi9NXMdTEW92fVjRmNmhO8DMvVOmeNAa DE4GKX1Mzy8vPzwj3OKG6dHGCiKpsUdlkJGEhfzlf0iVrPcMuJsy4Wd06LMYUcQ1g6 HFFLguxQModKdMFhZXKgGPDRSo/EXQBObus0il/k= Date: Sun, 3 Jan 2021 18:04:10 +0100 From: Clemens Gruber To: Thierry Reding , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Sven Van Asbroeck Cc: Linux Kernel Mailing List , linux-pwm@vger.kernel.org Subject: Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout Message-ID: References: <20201216125320.5277-1-clemens.gruber@pqgruber.com> <20201216125320.5277-2-clemens.gruber@pqgruber.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi everyone, happy new year, hope you are all well! On Thu, Dec 17, 2020 at 12:52:42PM -0500, Sven Van Asbroeck wrote: > On Thu, Dec 17, 2020 at 12:43 PM Clemens Gruber > wrote: > > > > > > Conclusion: .get_state() will always return "pwm disabled", so why do we > > > bother reading out the h/w? > > > > If there are no plans for the PWM core to call .get_state more often in > > the future, we could just read out the period and return 0 duty and > > disabled. > > I'm not sure why we should even read out the period? > When a channel is disabled, the period is not externally visible, > therefore it's meaningless ? > > As far as I can tell, we can use this for .get_state(): > memset(&pwm->state, 0, sizeof(pwm_state)); > > > > > Thierry, Uwe, what's your take on this? I will continue working on this series in the upcoming weeks. Feedback on the .get_state issue would be greatly appreciated. To summarize: Is it OK for a driver to expect the chip->ops->get_state function to be only called from the place in pwm core it is currently called from? (Namely in pwm_device_request after chip->ops->request) If yes, we could always return a 0 duty cycle and disabled state, because this is the state we left it in after .probe (and .free). However, if in the future, the pwm core adds additional calls to chip->ops->get_state in other places, this could lead to problems. -- Another point is the period: Sven suggested we do not read out the period at all, as the PWM is disabled anyway (see above). Is this acceptable? And, if we never return anything but 0 in .get_state, should it be implemented at all? > > > > > Of course, if we choose to leave the pwm enabled after .free(), then > > > .get_state() can even be left out! Do we want that? Genuine question, I do > > > not know the answer. > > > > I do not think we should leave it enabled after free. It is less > > complicated if we know that unrequested channels are not in use. > > > > Good point, I agree with you. Thanks and best regards, Clemens