From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW Date: Wed, 20 Dec 2017 10:32:11 +0100 Message-ID: <20171220093211.GA16177@kroah.com> References: <20171220083403.GA27231@bitmer.com> <1513761884.1234.83.camel@perches.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1513761884.1234.83.camel@perches.com> Sender: linux-pm-owner@vger.kernel.org To: Joe Perches Cc: Jarkko Nikula , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Cezary Jackiewicz , Darren Hart , Andy Shevchenko , Sebastian Ott , Peter Oberparleiter , James Smart , Dick Kennedy , Zhang Rui , Eduardo Valentin , Mathias Nyman , Felipe Balbi , "Luis R. Rodriguez" , Peter Ujfalusi , Martin Schwidefsky , Heiko List-Id: alsa-devel@alsa-project.org On Wed, Dec 20, 2017 at 01:24:44AM -0800, Joe Perches wrote: > On Wed, 2017-12-20 at 10:34 +0200, Jarkko Nikula wrote: > > On Tue, Dec 19, 2017 at 10:15:07AM -0800, Joe Perches wrote: > > > Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible. > [] > > > diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c > [] > > > @@ -854,7 +854,7 @@ static ssize_t dma_op_mode_store(struct device *dev, > > > return size; > > > } > > > > > > -static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store); > > > +static DEVICE_ATTR_RW(dma_op_mode); > > > > > > > While I can ack this part here if it helps generic cleanup effort I > > don't understart would it improve code readability in general? Mode 644 > > is clear and don't need any grepping but for DEVICE_ATTR_RW() I had to go > > through all of these files in order to see what does it mean: Yeah, 644 is "clear", but _RW() is even more clear. Ideally I want to get rid of all of the "non-standard" users that set random modes of sysfs files, as we get it wrong too many times. Using the "defaults" is much better. > Are you suggesting that device.h (as that is where > DEVICE_ATTR and the other DEVICE_ATTR_ variants > are #defined) should have better comments for the > _ variants? > > > DEVICE_ATTR_RW: include/linux/device.h > > __ATTR_RW: include/linux/sysfs.h > > S_IWUSR: include/uapi/linux/stat.h > > S_IRUGO: include/linux/stat.h > > btw: patch 1/4 of the series does remove the uses of > S_ from these macros in sysfs.h and converts > them to simple octal instead. Why you didn't send that patch to the sysfs maintainer is a bit odd... :) I should be taking this whole series through my tree. Joe, thanks for doing this in an automated way, I've been wanting to see this done for a long time. thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Date: Wed, 20 Dec 2017 09:32:11 +0000 Subject: Re: [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW Message-Id: <20171220093211.GA16177@kroah.com> List-Id: References: <20171220083403.GA27231@bitmer.com> <1513761884.1234.83.camel@perches.com> In-Reply-To: <1513761884.1234.83.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Joe Perches Cc: Jarkko Nikula , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Cezary Jackiewicz , Darren Hart , Andy Shevchenko , Sebastian Ott , Peter Oberparleiter , James Smart , Dick Kennedy , Zhang Rui , Eduardo Valentin , Mathias Nyman , Felipe Balbi , "Luis R. Rodriguez" , Peter Ujfalusi , Martin Schwidefsky , Heiko On Wed, Dec 20, 2017 at 01:24:44AM -0800, Joe Perches wrote: > On Wed, 2017-12-20 at 10:34 +0200, Jarkko Nikula wrote: > > On Tue, Dec 19, 2017 at 10:15:07AM -0800, Joe Perches wrote: > > > Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible. > [] > > > diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c > [] > > > @@ -854,7 +854,7 @@ static ssize_t dma_op_mode_store(struct device *dev, > > > return size; > > > } > > > > > > -static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store); > > > +static DEVICE_ATTR_RW(dma_op_mode); > > > > > > > While I can ack this part here if it helps generic cleanup effort I > > don't understart would it improve code readability in general? Mode 644 > > is clear and don't need any grepping but for DEVICE_ATTR_RW() I had to go > > through all of these files in order to see what does it mean: Yeah, 644 is "clear", but _RW() is even more clear. Ideally I want to get rid of all of the "non-standard" users that set random modes of sysfs files, as we get it wrong too many times. Using the "defaults" is much better. > Are you suggesting that device.h (as that is where > DEVICE_ATTR and the other DEVICE_ATTR_ variants > are #defined) should have better comments for the > _ variants? > > > DEVICE_ATTR_RW: include/linux/device.h > > __ATTR_RW: include/linux/sysfs.h > > S_IWUSR: include/uapi/linux/stat.h > > S_IRUGO: include/linux/stat.h > > btw: patch 1/4 of the series does remove the uses of > S_ from these macros in sysfs.h and converts > them to simple octal instead. Why you didn't send that patch to the sysfs maintainer is a bit odd... :) I should be taking this whole series through my tree. Joe, thanks for doing this in an automated way, I've been wanting to see this done for a long time. thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [-next,2/4] treewide: Use DEVICE_ATTR_RW From: Greg Kroah-Hartman Message-Id: <20171220093211.GA16177@kroah.com> Date: Wed, 20 Dec 2017 10:32:11 +0100 To: Joe Perches Cc: Jarkko Nikula , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Cezary Jackiewicz , Darren Hart , Andy Shevchenko , Sebastian Ott , Peter Oberparleiter , James Smart , Dick Kennedy , Zhang Rui , Eduardo Valentin , Mathias Nyman , Felipe Balbi , "Luis R. Rodriguez" , Peter Ujfalusi , Martin Schwidefsky , Heiko Carstens , David Airlie , "James E.J. Bottomley" , "Martin K. Petersen" , Jiri Slaby , Bartlomiej Zolnierkiewicz , Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, platform-driver-x86@vger.kernel.org, linux-scsi@vger.kernel.org, linux-pm@vger.kernel.org, linux-serial@vger.kernel.org, linux-usb@vger.kernel.org, linux-fbdev@vger.kernel.org, alsa-devel@alsa-project.org, linux-omap@vger.kernel.org List-ID: T24gV2VkLCBEZWMgMjAsIDIwMTcgYXQgMDE6MjQ6NDRBTSAtMDgwMCwgSm9lIFBlcmNoZXMgd3Jv dGU6Cj4gT24gV2VkLCAyMDE3LTEyLTIwIGF0IDEwOjM0ICswMjAwLCBKYXJra28gTmlrdWxhIHdy b3RlOgo+ID4gT24gVHVlLCBEZWMgMTksIDIwMTcgYXQgMTA6MTU6MDdBTSAtMDgwMCwgSm9lIFBl cmNoZXMgd3JvdGU6Cj4gPiA+IENvbnZlcnQgREVWSUNFX0FUVFIgdXNlcyB0byBERVZJQ0VfQVRU Ul9SVyB3aGVyZSBwb3NzaWJsZS4KPiBbXSAKPiA+ID4gZGlmZiAtLWdpdCBhL3NvdW5kL3NvYy9v bWFwL21jYnNwLmMgYi9zb3VuZC9zb2Mvb21hcC9tY2JzcC5jCj4gW10KPiA+ID4gQEAgLTg1NCw3 ICs4NTQsNyBAQCBzdGF0aWMgc3NpemVfdCBkbWFfb3BfbW9kZV9zdG9yZShzdHJ1Y3QgZGV2aWNl ICpkZXYsCj4gPiA+ICAJcmV0dXJuIHNpemU7Cj4gPiA+ICB9Cj4gPiA+ICAKPiA+ID4gLXN0YXRp YyBERVZJQ0VfQVRUUihkbWFfb3BfbW9kZSwgMDY0NCwgZG1hX29wX21vZGVfc2hvdywgZG1hX29w X21vZGVfc3RvcmUpOwo+ID4gPiArc3RhdGljIERFVklDRV9BVFRSX1JXKGRtYV9vcF9tb2RlKTsK PiA+ID4gIAo+ID4gCj4gPiBXaGlsZSBJIGNhbiBhY2sgdGhpcyBwYXJ0IGhlcmUgaWYgaXQgaGVs cHMgZ2VuZXJpYyBjbGVhbnVwIGVmZm9ydCBJCj4gPiBkb24ndCB1bmRlcnN0YXJ0IHdvdWxkIGl0 IGltcHJvdmUgY29kZSByZWFkYWJpbGl0eSBpbiBnZW5lcmFsPyBNb2RlIDY0NAo+ID4gaXMgY2xl YXIgYW5kIGRvbid0IG5lZWQgYW55IGdyZXBwaW5nIGJ1dCBmb3IgREVWSUNFX0FUVFJfUlcoKSBJ IGhhZCB0byBnbwo+ID4gdGhyb3VnaCBhbGwgb2YgdGhlc2UgZmlsZXMgaW4gb3JkZXIgdG8gc2Vl IHdoYXQgZG9lcyBpdCBtZWFuOgoKWWVhaCwgNjQ0IGlzICJjbGVhciIsIGJ1dCBfUlcoKSBpcyBl dmVuIG1vcmUgY2xlYXIuICBJZGVhbGx5IEkgd2FudCB0bwpnZXQgcmlkIG9mIGFsbCBvZiB0aGUg Im5vbi1zdGFuZGFyZCIgdXNlcnMgdGhhdCBzZXQgcmFuZG9tIG1vZGVzIG9mCnN5c2ZzIGZpbGVz LCBhcyB3ZSBnZXQgaXQgd3JvbmcgdG9vIG1hbnkgdGltZXMuICBVc2luZyB0aGUgImRlZmF1bHRz IiBpcwptdWNoIGJldHRlci4KCj4gQXJlIHlvdSBzdWdnZXN0aW5nIHRoYXQgZGV2aWNlLmggKGFz IHRoYXQgaXMgd2hlcmUKPiBERVZJQ0VfQVRUUiBhbmQgdGhlIG90aGVyIERFVklDRV9BVFRSXzxG T08+IHZhcmlhbnRzCj4gYXJlICNkZWZpbmVkKSBzaG91bGQgaGF2ZSBiZXR0ZXIgY29tbWVudHMg Zm9yIHRoZQo+IF88Rk9PPiB2YXJpYW50cz8KPiAKPiA+IERFVklDRV9BVFRSX1JXOiBpbmNsdWRl L2xpbnV4L2RldmljZS5oCj4gPiBfX0FUVFJfUlc6IGluY2x1ZGUvbGludXgvc3lzZnMuaAo+ID4g U19JV1VTUjogaW5jbHVkZS91YXBpL2xpbnV4L3N0YXQuaAo+ID4gU19JUlVHTzogaW5jbHVkZS9s aW51eC9zdGF0LmgKPiAKPiBidHc6IHBhdGNoIDEvNCBvZiB0aGUgc2VyaWVzIGRvZXMgcmVtb3Zl IHRoZSB1c2VzIG9mCj4gU188UEVSTVM+IGZyb20gdGhlc2UgbWFjcm9zIGluIHN5c2ZzLmggYW5k IGNvbnZlcnRzCj4gdGhlbSB0byBzaW1wbGUgb2N0YWwgaW5zdGVhZC4KCldoeSB5b3UgZGlkbid0 IHNlbmQgdGhhdCBwYXRjaCB0byB0aGUgc3lzZnMgbWFpbnRhaW5lciBpcyBhIGJpdCBvZGQuLi4g IDopCgpJIHNob3VsZCBiZSB0YWtpbmcgdGhpcyB3aG9sZSBzZXJpZXMgdGhyb3VnaCBteSB0cmVl LiAgSm9lLCB0aGFua3MgZm9yCmRvaW5nIHRoaXMgaW4gYW4gYXV0b21hdGVkIHdheSwgSSd2ZSBi ZWVuIHdhbnRpbmcgdG8gc2VlIHRoaXMgZG9uZSBmb3IgYQpsb25nIHRpbWUuCgp0aGFua3MsCgpn cmVnIGstaAotLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUg InVuc3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4KdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9y ZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIu a2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754575AbdLTJcP (ORCPT ); Wed, 20 Dec 2017 04:32:15 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:34930 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753269AbdLTJcL (ORCPT ); Wed, 20 Dec 2017 04:32:11 -0500 Date: Wed, 20 Dec 2017 10:32:11 +0100 From: Greg Kroah-Hartman To: Joe Perches Cc: Jarkko Nikula , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Cezary Jackiewicz , Darren Hart , Andy Shevchenko , Sebastian Ott , Peter Oberparleiter , James Smart , Dick Kennedy , Zhang Rui , Eduardo Valentin , Mathias Nyman , Felipe Balbi , "Luis R. Rodriguez" , Peter Ujfalusi , Martin Schwidefsky , Heiko Carstens , David Airlie , "James E.J. Bottomley" , "Martin K. Petersen" , Jiri Slaby , Bartlomiej Zolnierkiewicz , Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, platform-driver-x86@vger.kernel.org, linux-scsi@vger.kernel.org, linux-pm@vger.kernel.org, linux-serial@vger.kernel.org, linux-usb@vger.kernel.org, linux-fbdev@vger.kernel.org, alsa-devel@alsa-project.org, linux-omap@vger.kernel.org Subject: Re: [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW Message-ID: <20171220093211.GA16177@kroah.com> References: <20171220083403.GA27231@bitmer.com> <1513761884.1234.83.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1513761884.1234.83.camel@perches.com> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 20, 2017 at 01:24:44AM -0800, Joe Perches wrote: > On Wed, 2017-12-20 at 10:34 +0200, Jarkko Nikula wrote: > > On Tue, Dec 19, 2017 at 10:15:07AM -0800, Joe Perches wrote: > > > Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible. > [] > > > diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c > [] > > > @@ -854,7 +854,7 @@ static ssize_t dma_op_mode_store(struct device *dev, > > > return size; > > > } > > > > > > -static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store); > > > +static DEVICE_ATTR_RW(dma_op_mode); > > > > > > > While I can ack this part here if it helps generic cleanup effort I > > don't understart would it improve code readability in general? Mode 644 > > is clear and don't need any grepping but for DEVICE_ATTR_RW() I had to go > > through all of these files in order to see what does it mean: Yeah, 644 is "clear", but _RW() is even more clear. Ideally I want to get rid of all of the "non-standard" users that set random modes of sysfs files, as we get it wrong too many times. Using the "defaults" is much better. > Are you suggesting that device.h (as that is where > DEVICE_ATTR and the other DEVICE_ATTR_ variants > are #defined) should have better comments for the > _ variants? > > > DEVICE_ATTR_RW: include/linux/device.h > > __ATTR_RW: include/linux/sysfs.h > > S_IWUSR: include/uapi/linux/stat.h > > S_IRUGO: include/linux/stat.h > > btw: patch 1/4 of the series does remove the uses of > S_ from these macros in sysfs.h and converts > them to simple octal instead. Why you didn't send that patch to the sysfs maintainer is a bit odd... :) I should be taking this whole series through my tree. Joe, thanks for doing this in an automated way, I've been wanting to see this done for a long time. thanks, greg k-h