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 6EC05C43334 for ; Tue, 14 Jun 2022 15:28:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236030AbiFNP2H (ORCPT ); Tue, 14 Jun 2022 11:28:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236270AbiFNP1q (ORCPT ); Tue, 14 Jun 2022 11:27:46 -0400 Received: from mail-vs1-xe34.google.com (mail-vs1-xe34.google.com [IPv6:2607:f8b0:4864:20::e34]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E0F9C27B04; Tue, 14 Jun 2022 08:27:43 -0700 (PDT) Received: by mail-vs1-xe34.google.com with SMTP id x187so9354259vsb.0; Tue, 14 Jun 2022 08:27:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:reply-to:from:date:message-id :subject:to:cc:content-transfer-encoding; bh=R6gek+7k8btaKHOjjow80I4o5y14I8XjWpXPB6g1/2Y=; b=B62h3QGdOCNz+CIyxl++5a3MAeWGpBpIqvE1LuIDxAiBT2KOwgQsKXMm+yXioIVHPB jjt0HWzkzU+KazGRGAzDxWR3PT8e2YZfMNNwZRjTISDAHYPOACZJXiWVcdlbj62O1iTH CRwo8tLU+5BPwTR0mx4PSFMKmNeaQFH128VbyZEXS80sLFMqOfLmOOzA8IxoLLnm8R1v HQXqYY7yI1kW48SwVZlyiRMCGRsWx44m6OPWTednk9+irfXrEil3L5JuvbC+KIUjgb7s Sv7mQHB3kSWGK1UJfXJjtiXsZawUtOKESqwxCQqrOQ56ByyQX/Ug9G+RZ1xaZgpJwVSc sYEg== 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:reply-to :from:date:message-id:subject:to:cc:content-transfer-encoding; bh=R6gek+7k8btaKHOjjow80I4o5y14I8XjWpXPB6g1/2Y=; b=EDhfSB7+NoOj7pKwu+KhQIx41dVAgqRQUZWIspAmJXXlxpVuS51qFrlkUYFWFJ7CHP 60RdPHIWtqQn6qf1AnaWu45PM2p4yD6lIK1siZhOQWU8klegAGAOMuz0tjzz5QJtXSHs ufcxtde/eXaorrfU16JkkqKTjM89Fw3wDQQi3tv8B25USD4JRb8dP/iqIqHIk3LwE2+S 7qQKPP+XbrA3yb3CRu/SZ6tYjoLqIhcOti5zRsvVXP59sY8cX25+lgo5IQzbWes5IvRD RoLtRVYVJWwmj1hSbLaBOKSmyIEDXt3bK8aeLgxpsQ4jXa25UMTTug5oUjDcXeuH/WxY VgNg== X-Gm-Message-State: AJIora8OxFStUU0P+VLaJpjmXpnPILPnFl8fyJMnccVj85gYMMvzXyxm ErVHzKK0N4YgdBYYY3ZQcNBBuYlgSXttrNgL1dw15x0q X-Google-Smtp-Source: AGRyM1sNY6Hgb1s6xJAYTUwIny72g5FhpKwdFq/0wKPM/o6VQY94iQFJ/lrdOaOTlJH1unL8KMyBHGsAJprHhcfb+js= X-Received: by 2002:a67:eecb:0:b0:34b:f863:ce1b with SMTP id o11-20020a67eecb000000b0034bf863ce1bmr2411704vsp.55.1655220463027; Tue, 14 Jun 2022 08:27:43 -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: Reply-To: andrea.merello@gmail.com From: Andrea Merello Date: Tue, 14 Jun 2022 17:27:32 +0200 Message-ID: Subject: Re: [v6 08/14] iio: imu: add Bosch Sensortec BNO055 core driver To: Andy Shevchenko Cc: Andrea Merello , 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 Il giorno mar 14 giu 2022 alle ore 17:11 Andy Shevchenko ha scritto: > > On Tue, Jun 14, 2022 at 2:15 PM Andrea Merello wr= ote: > > ... > > > >> >> + devm_add_action_or_reset(priv->dev, bno055_debugfs_remove= , priv->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 = indeed. > > >> > > >> Assuming that we don't want to make the whole driver fail in case de= bugfs stuff fails (see last part of the comment above debugfs_create_file()= implementation), and given that the devm_add_action_or_reset(), should ind= eed "reset" in case of failure (i.e. we should be in a clean situation any= way), 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. > > > > Hum, I would say this shouldn't ever happen: AFAICS devm_add_action_or_= reset() is a wrapper around devm_add_action() and it's purpose is exactly t= o add a check for failure; devm_add_action_or_reset() immediately invokes t= he action handler in case devm_add_action() fails. IOW in case of failure t= o add the devm stuff, the debugfs file is removed immediately and it should= n't cause any mess with next times probe()s; just the driver will go on wit= hout the debugfs file being here. > > > > I think this is the point of using devm_add_action_or_reset() instead o= f dev_add_action() indeed, or am I missing something? > > Reading that code again and I think you are right, so dev_warn() will > be sufficient to show that we fail. OTOH, what is the point of adding > a resource for the failed debugfs call? Ah, you are right here: I'll make the call to devm_add_action_or_reset() conditional to success of debugfs_create_file(). In case any of the two fails we can also warn the user. > -- > With Best Regards, > Andy Shevchenko