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=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=ham 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 6D6E3CA9EB0 for ; Sun, 3 Nov 2019 11:25:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 41D612084D for ; Sun, 3 Nov 2019 11:25:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1572780347; bh=iVW9cBPnp0fjG+zTXydLaRUI+YAsffRYMHdiQGWeTNo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=ftGSJSWbNVjEf/oGmcxxo7qSoBQActVO+MKaSnVPEIcDtet/eCbJGCoFXIggWlV1K wvJ1nKpirn+LubwrYE97ltc03y1qY28ICra12YD8Q/XMv4gZpXjlKPHxPGKV0YXkI6 bd6kKwCd88JZ298JRMtNV4QiwBZqFvQPB98NZHgA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727590AbfKCLZq (ORCPT ); Sun, 3 Nov 2019 06:25:46 -0500 Received: from mail.kernel.org ([198.145.29.99]:40356 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727554AbfKCLZq (ORCPT ); Sun, 3 Nov 2019 06:25:46 -0500 Received: from archlinux (cpc149474-cmbg20-2-0-cust94.5-4.cable.virginm.net [82.4.196.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 2AB602080F; Sun, 3 Nov 2019 11:25:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1572780345; bh=iVW9cBPnp0fjG+zTXydLaRUI+YAsffRYMHdiQGWeTNo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=KnLJ+ocoKBFLnYGZIbVgwmWNxa5NTEVReT7rXqEDHZFdSgW8ESY92vPuukCGO5mN5 y9vqz0hcZ3nU0N3F5FZomd4QzsIqsOyxrHhXvE+x5X+YxUZ9W2fXjI1LxKiU3qhAkW xI7w+lUfCUUpXO6DQQ9rXH3gMMs4gyKQZjNPR5QU= Date: Sun, 3 Nov 2019 11:25:40 +0000 From: Jonathan Cameron To: zhong jiang Cc: "Ardelean, Alexandru" , "Popa, Stefan Serban" , "Hennerich, Michael" , "linux-kernel@vger.kernel.org" , "linux-iio@vger.kernel.org" Subject: Re: [PATCH 1/2] iio: imu: adis16460: use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs fops Message-ID: <20191103112540.5fdfccad@archlinux> In-Reply-To: <5DB958DA.7080305@huawei.com> References: <1572423581-59762-1-git-send-email-zhongjiang@huawei.com> <1572423581-59762-2-git-send-email-zhongjiang@huawei.com> <5DB958DA.7080305@huawei.com> X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-iio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org On Wed, 30 Oct 2019 17:33:14 +0800 zhong jiang wrote: > On 2019/10/30 17:13, Ardelean, Alexandru wrote: > > On Wed, 2019-10-30 at 16:19 +0800, zhong jiang wrote: > >> [External] > >> > >> It is more clear to use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs file > >> operation rather than DEFINE_SIMPLE_ATTRIBUTE. > > Not sure if "more clear" is the word. > Should be more clearly. :-) > > But it is more correct to use DEFINE_DEBUGFS_ATTRIBUTE(), since they are > > debugfs attrs. > > > > In any case, this is no big deal. > > So: > > > > Reviewed-by: Alexandru Ardelean > > > >> Signed-off-by: zhong jiang I started looking into why this attributes were introduced. There are potential issues, so Alex can you confirm you've tested this series. Whilst it looks right, it seems some other patches making this change have had to switch over to the unsafe registration functions. https://patchwork.kernel.org/patch/11051725/ https://lkml.org/lkml/2019/10/30/144 The reference counting is subtly different between the two versions. Seems you are getting some push back on similar patches. Perhaps a v2 with reference to the other threads if those get resolved to say it is sensible to make this change. Thanks, Jonathan > >> --- > >> drivers/iio/imu/adis16460.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c > >> index 6aed9e8..2e7a582 100644 > >> --- a/drivers/iio/imu/adis16460.c > >> +++ b/drivers/iio/imu/adis16460.c > >> @@ -87,7 +87,7 @@ static int adis16460_show_serial_number(void *arg, u64 > >> *val) > >> > >> return 0; > >> } > >> -DEFINE_SIMPLE_ATTRIBUTE(adis16460_serial_number_fops, > >> +DEFINE_DEBUGFS_ATTRIBUTE(adis16460_serial_number_fops, > >> adis16460_show_serial_number, NULL, "0x%.4llx\n"); > >> > >> static int adis16460_show_product_id(void *arg, u64 *val) > >> @@ -105,7 +105,7 @@ static int adis16460_show_product_id(void *arg, u64 > >> *val) > >> > >> return 0; > >> } > >> -DEFINE_SIMPLE_ATTRIBUTE(adis16460_product_id_fops, > >> +DEFINE_DEBUGFS_ATTRIBUTE(adis16460_product_id_fops, > >> adis16460_show_product_id, NULL, "%llu\n"); > >> > >> static int adis16460_show_flash_count(void *arg, u64 *val) > >> @@ -123,7 +123,7 @@ static int adis16460_show_flash_count(void *arg, u64 > >> *val) > >> > >> return 0; > >> } > >> -DEFINE_SIMPLE_ATTRIBUTE(adis16460_flash_count_fops, > >> +DEFINE_DEBUGFS_ATTRIBUTE(adis16460_flash_count_fops, > >> adis16460_show_flash_count, NULL, "%lld\n"); > >> > >> static int adis16460_debugfs_init(struct iio_dev *indio_dev) > >