From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753197AbbGVVow (ORCPT ); Wed, 22 Jul 2015 17:44:52 -0400 Received: from mail-pa0-f49.google.com ([209.85.220.49]:35028 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752461AbbGVVou (ORCPT ); Wed, 22 Jul 2015 17:44:50 -0400 Date: Wed, 22 Jul 2015 14:44:46 -0700 From: Dmitry Torokhov To: Alexandre Belloni Cc: Krzysztof Kozlowski , rtc-linux@googlegroups.com, Alessandro Zummo , linux-kernel@vger.kernel.org Subject: Re: [rtc-linux] [PATCH 4/4] RTC: switch to using is_visible() to control sysfs attributes Message-ID: <20150722214446.GE14875@dtor-ws> References: <1437433372-15425-1-git-send-email-dmitry.torokhov@gmail.com> <1437433372-15425-4-git-send-email-dmitry.torokhov@gmail.com> <20150722205735.GP2853@piout.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150722205735.GP2853@piout.net> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 22, 2015 at 10:57:35PM +0200, Alexandre Belloni wrote: > (Krzysztof, be careful, Dmitry was not in copy of your maili, you should > probably check your mailer config) > > On 21/07/2015 at 10:21:11 +0900, Krzysztof Kozlowski wrote : > > 2015-07-21 8:02 GMT+09:00 Dmitry Torokhov : > > > static ssize_t > > > -rtc_sysfs_set_wakealarm(struct device *dev, struct device_attribute *attr, > > > +wakealarm_store(struct device *dev, struct device_attribute *attr, > > > const char *buf, size_t n) > > > { > > > ssize_t retval; > > > @@ -221,45 +209,58 @@ rtc_sysfs_set_wakealarm(struct device *dev, struct device_attribute *attr, > > > retval = rtc_set_alarm(rtc, &alm); > > > return (retval < 0) ? retval : n; > > > } > > > -static DEVICE_ATTR(wakealarm, S_IRUGO | S_IWUSR, > > > - rtc_sysfs_show_wakealarm, rtc_sysfs_set_wakealarm); > > > +static DEVICE_ATTR_RW(wakealarm); > > > > This and renaming of show/store functions look unrelated > > > > I don't really mind that one but I would also prefer if it could be > separated. OK, I will. > > > > > > > +static struct attribute *rtc_attrs[] = { > > > + &dev_attr_name.attr, > > > + &dev_attr_date.attr, > > > + &dev_attr_time.attr, > > > + &dev_attr_since_epoch.attr, > > > + &dev_attr_max_user_freq.attr, > > > + &dev_attr_hctosys.attr, > > > + &dev_attr_wakealarm.attr, > > > + NULL, > > > +}; > > > > > > -/* The reason to trigger an alarm with no process watching it (via sysfs) > > > +/* > > > + * The reason to trigger an alarm with no process watching it (via sysfs) > > > * is its side effect: waking from a system state like suspend-to-RAM or > > > * suspend-to-disk. So: no attribute unless that side effect is possible. > > > * (Userspace may disable that mechanism later.) > > > */ > > > -static inline int rtc_does_wakealarm(struct rtc_device *rtc) > > > +static bool rtc_does_wakealarm(struct rtc_device *rtc) > > > { > > > if (!device_can_wakeup(rtc->dev.parent)) > > > - return 0; > > > + return false; > > > + > > > return rtc->ops->set_alarm != NULL; > > > } > > > > This looks unrelated too and confuses me. Could you split such cleanup > > from main goal of the patch? > > > > That one is bothering me too. Will separate this too. Thanks. -- Dmitry