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 3A680C433DB for ; Sat, 27 Mar 2021 15:55:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F40B8619C7 for ; Sat, 27 Mar 2021 15:55:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230213AbhC0PzP (ORCPT ); Sat, 27 Mar 2021 11:55:15 -0400 Received: from mail.pqgruber.com ([52.59.78.55]:52716 "EHLO mail.pqgruber.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229582AbhC0Pyw (ORCPT ); Sat, 27 Mar 2021 11:54:52 -0400 Received: from workstation.tuxnet (213-47-165-233.cable.dynamic.surfer.at [213.47.165.233]) by mail.pqgruber.com (Postfix) with ESMTPSA id D5828CB478B; Sat, 27 Mar 2021 16:54:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pqgruber.com; s=mail; t=1616860489; bh=U1/5mO9/pvifbFBvwKUxzTpOhHdjmuuw1LjJ30iSfIA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CKcIdTgNnf59N9VMHw0m7We3VIpxLqea3+nr91l+J/DMGef0MRIFoUdzJT31hwbrx /s1xEfLrMT2FbJkaf89jiRpOC/uAjkQ0s6Zk3MG2aQrGZM1BmklZ12xKu0FWb/0/nY p/N0ykruPyO5NIuviR2ESTT2TbmuG5qhP82I+KAY= Date: Sat, 27 Mar 2021 16:54:47 +0100 From: Clemens Gruber To: Thierry Reding Cc: Sven Van Asbroeck , linux-pwm@vger.kernel.org, Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Lee Jones , Linux Kernel Mailing List , Mika Westerberg , David Jander Subject: Re: [PATCH v5 1/7] pwm: pca9685: Switch to atomic API Message-ID: References: <20201215212228.185517-1-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 Thierry, On Mon, Mar 22, 2021 at 08:58:35AM +0100, Thierry Reding wrote: > On Thu, Dec 17, 2020 at 12:10:10PM -0500, Sven Van Asbroeck wrote: > > On Thu, Dec 17, 2020 at 11:48 AM Clemens Gruber > > wrote: > > > > > > I can initialize the values to 0 of course and check the file for other > > > places with missing initializations. > > > > > > Or would it be better to check the return codes of regmap_read/write in > > > such cases? I'm not sure. > > > > I think that checking the regmap_read/write return values is overkill > > in this driver. These functions can't realistically fail, except if the i2c > > bus is bad, i.e. h/w failure or intermittency. And that's an externality > > which I believe we can ignore. > > I think there are (rare) occasions where it's fine to not check for > errors, i.e. if you definitively know that calls can't fail. However, > given that this uses regmap and you don't really know what's backing > this, I think it's always better to err on the side of caution and > properly check the return values. > > The fact that this can be externally caused is actually a reason why > we shouldn't be ignoring any errors. If there's a chip that's hogging > the I2C bus or if you've even just mistyped the I2C client's address > in DT, it's better if the PWM driver tells you with an error message > than if it is silently ignoring the errors and keeps you guessing at > why the PWM isn't behaving the way it should. > > Granted, the error code isn't always going to pinpoint exactly what's > going wrong, but for serious errors often the I2C bus driver will let > you know with an extra error message. However, it's much easier to go > looking for that error message if the PWM driver lets you know that > something went wrong. > > Please just add full checking of regmap operations. OK, I will create a separate patch adding these checks in the next series. This will lead to > 20 additional dev_err statements, let me know if I should instead just return the error code and not add dev_err's for every failed regmap operation. Clemens