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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC15CC43334 for ; Tue, 14 Jun 2022 10:59:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234473AbiFNK7N (ORCPT ); Tue, 14 Jun 2022 06:59:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51468 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235667AbiFNK7B (ORCPT ); Tue, 14 Jun 2022 06:59:01 -0400 Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 36139286C1; Tue, 14 Jun 2022 03:59:00 -0700 (PDT) Received: by mail-ej1-x62e.google.com with SMTP id u12so16370712eja.8; Tue, 14 Jun 2022 03:59:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=pIl3ve2qWNEUoL+rbrKIn4jxMygDkC3EHjty1+HINlM=; b=kasr6ciKYU3aBbGLQ4cwq8w8ZCkS9QclEp3UGTJv9aWFhfeH63Jg5ksFBP6v/z7ODZ 8I8GLstOsKRPgLEdR6nqRTmmZ6+r6cxP14bnQM/fAjWdecUQD1jkIRF2hLq7+Qid7dn0 R5vl18NeVxyFIFQUxMz+KC6RcogcaOIOBx+vsCfV7wRcxpOAs23M0OoGXet2l9ttYeFv TVMxTLQRtnEU6DXjzxUgPffb8F2riiAGo++fA7Qgj5hz+VaXpEBcNpGfAgP+p5nSMwTI VIsWZog4iHy8NVOpkIYwgLV/3uD51HeAL//2tu2hHhBVA+Kup5+NxDZIsocLR+DN6w+J tWVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=pIl3ve2qWNEUoL+rbrKIn4jxMygDkC3EHjty1+HINlM=; b=60JKJwM/huQ/ECeL6ov3HJXIRqrlFK3JBtvLPxdYNVL2EYpU2cZocZZE2reL/YIXLg 9kUKBpZozSv2/BnwQ45lXgwS4E15SEKVEBUEDsEK2tjp1koZwziLI5pevH+rEqSS/Iz4 bzDhZna/pO3Zy6guJo38JcmwM3GQZshgH/FaVe9mE33mhHLm8IDoPWyQ4YnIy9KlwEG8 rGwRwGG8HFJ0PlUU/y7zCwwE+lLawtaJIn2W7ga4dtfftWkHrVglcDlT7yEVH1My7wfB TPeqj9+9C0i5wUQebMcqxX5y+nQAFaA3BqcleZrWNXfHJ0NzVcmsRiJWT9VyhCaJE4A+ p3eQ== X-Gm-Message-State: AOAM533X8VHEWEXFF4VW47o/j36427meWV1l2ALWYq6hEPaV60HTbbsR 1if/3694SxCg9xZLCPK7XgdXRJ7U+rcytMJvIE4= X-Google-Smtp-Source: ABdhPJyrQBhiI0HqIsQgPdH3NL5HHlDRcXK8aanoiPins9/RAWDYVugKpOEtTl8I4lVA6480iiOiM40AjuQf94G21GY= X-Received: by 2002:a17:906:149:b0:711:fca6:bc2f with SMTP id 9-20020a170906014900b00711fca6bc2fmr3774376ejh.497.1655204338631; Tue, 14 Jun 2022 03:58:58 -0700 (PDT) MIME-Version: 1.0 References: <20220613120534.36991-1-andrea.merello@iit.it> <20220613120534.36991-9-andrea.merello@iit.it> <164f663acbba481a8ea8f45f185aaf83@iit.it> In-Reply-To: <164f663acbba481a8ea8f45f185aaf83@iit.it> From: Andy Shevchenko Date: Tue, 14 Jun 2022 12:58:21 +0200 Message-ID: Subject: Re: [v6 08/14] iio: imu: add Bosch Sensortec BNO055 core driver To: Andrea Merello Cc: Jonathan Cameron , Mauro Carvalho Chehab , linux-iio , Linux Kernel Mailing List , devicetree , Lars-Peter Clausen , Rob Herring , Matt Ranostay , Alexandru Ardelean , jmondi Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org On Tue, Jun 14, 2022 at 11:11 AM Andrea Merello wro= te: ... > >> + /* G: 2, 4, 8, 16 */ > > > >Indentation of this comment is a bit off. > > > >> +static int bno055_acc_range_vals[] =3D {1962, 3924, 7848, 15696}; > > > >Perhaps split this to 4 lines and put the comment on top of the third li= ne? > > Not sure what you mean here, sorry. May you elaborate or provide an examp= le, please? static int ... [] =3D { /* Comment goes here */ value1, value2, ..., valueN, }; ... > >> +static void bno055_debugfs_init(struct iio_dev *iio_dev) > >> +{ > >> + struct bno055_priv *priv =3D iio_priv(iio_dev); > >> + > >> + priv->debugfs =3D debugfs_create_file("firmware_version", 0400= , > >> + iio_get_debugfs_dentry(iio= _dev), > >> + priv, &bno055_fw_version_o= ps); > > > >> + devm_add_action_or_reset(priv->dev, bno055_debugfs_remove, pri= v->debugfs); > > > >Shouldn't we report the potential error here? It's not directly > >related to debugfs, but something which is not directly related. > > The error eventually comes out from something that has nothing to do with= debugs per se (i.e. the devm stuff), but it will only affect debugfs indee= d. > > Assuming that we don't want to make the whole driver fail in case debugfs= stuff fails (see last part of the comment above debugfs_create_file() impl= ementation), and given that the devm_add_action_or_reset(), should indeed "= reset" in case of failure (i.e. we should be in a clean situation anyway), = I would say it should be OK not to propagate the error and let things go on= . As I said, it's not directly related to debugfs. Here is the resource leak possible or bad things happen if you probe the driver, that fails to add this call for removal, remove it, and try to insert again, in such case the debugfs will be stale. > However we can add a dev_warn() to report what happened. Not sure if it would suffice, I leave it to Jonathan. --=20 With Best Regards, Andy Shevchenko