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,URIBL_BLOCKED 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 45459C433E6 for ; Fri, 29 Jan 2021 16:34:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1ED8464D9D for ; Fri, 29 Jan 2021 16:34:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231682AbhA2QeU (ORCPT ); Fri, 29 Jan 2021 11:34:20 -0500 Received: from mail.pqgruber.com ([52.59.78.55]:55744 "EHLO mail.pqgruber.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231452AbhA2Qcb (ORCPT ); Fri, 29 Jan 2021 11:32:31 -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 B31B0C6B26F; Fri, 29 Jan 2021 17:31:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1611937908; bh=DyVjiqkvwtlqI4o3EcQtV2h3pSbBneM1WDytITBk7RI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QvmqkrRHoO8OTWpUz6Mk8rhaDh2GYebv8JjCP4MzmIpHbNvim7sdHMXdvdxdk1WV6 wh8DkbdtjnGeJljhSaUYGVmhqJNmnOmqZQycOFhkEvCq+/twNMF7ojFC9pl5EukbN9 gOerXCtBQV58ahmyweGooFaqHbEaj7t/MRYb2SFw= Date: Fri, 29 Jan 2021 17:31:47 +0100 From: Clemens Gruber To: Sven Van Asbroeck Cc: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Thierry Reding , 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> <20210111203532.m3yvq6e5bcpjs7mc@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sven, On Fri, Jan 29, 2021 at 08:42:13AM -0500, Sven Van Asbroeck wrote: > On Mon, Jan 11, 2021 at 3:35 PM Uwe Kleine-König > wrote: > > > > My position here is: A consumer should disable a PWM before calling > > pwm_put. The driver should however not enforce this and so should not > > modify the hardware state in .free(). > > > > Also .probe should not change the PWM configuration. > > I agree that this is the most user-friendly behaviour. > > The problem however with the pca9685 is that it has many degrees of > freedom: there are many possible register values which produce the same > physical chip outputs. > > This could lead to a situation where, if .probe() does not reset the register > values, subsequent writes may lead to different outputs than expected. > > One possible solution is to write .get_state() so that it always reads the > correct state, even if "unconventional" register settings are present, i.e. > those written by an outside entity, e.g. a bootloader. Then write that > state back using driver conventions. > > This may be trickier than it sounds - after all we've learnt that the pca9685 > looks simple on the surface, but is actually quite challenging to get right. > > Clemens, Uwe, what do you think? Ok, so you suggest we extend our get_state logic to deal with cases like the following: If neither full OFF nor full ON is set && ON == OFF, we should probably set the full OFF bit to disable the PWM and log a warning message? (e.g. "invalid register setting detected: pwm disabled" ?) If the ON registers are set and the nxp,staggered-outputs property is not, I'd calculate (off - on) & 4095, set the OFF register to that value and clear the ON register. And then call our get_state in .probe, followed by a write of the resulting / fixed-up state? This would definitely solve the problem of invalid/unconventional values set by the bootloader and avoid inconsistencies. Sounds good to me! If Thierry and Uwe have no objections, I can send out a new round of patches in the upcoming weeks. My current goal is to get the changes into 5.13. Thanks, Clemens