From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756198AbbE2Pxg (ORCPT ); Fri, 29 May 2015 11:53:36 -0400 Received: from cantor2.suse.de ([195.135.220.15]:40976 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbbE2Px2 (ORCPT ); Fri, 29 May 2015 11:53:28 -0400 Date: Fri, 29 May 2015 17:53:26 +0200 Message-ID: From: Takashi Iwai To: Quentin Lambert Cc: Johannes Berg , Jaroslav Kysela , linuxppc-dev@lists.ozlabs.org, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH] ALSA: aoa: convert bus code to use dev_groups In-Reply-To: <55688A72.7000306@gmail.com> References: <1432817307-28380-1-git-send-email-lambert.quentin@gmail.com> <55671F56.6080402@gmail.com> <55672980.4010904@gmail.com> <55688A72.7000306@gmail.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Fri, 29 May 2015 17:49:06 +0200, Quentin Lambert wrote: > > > > On 28/05/2015 17:01, Takashi Iwai wrote: > >>> Also, it'd be better to move ATTRIBUTE_GROUPS(soundbus_dev) into > >>> soundbus/sysfs.c, and make it this global instead of > >>> soundbus_dev_attrs[]. > >> Ok, I need to find a nice way to do that because ATTRIBUTE_GROUPS > >> declares the > >> structure as static. > > > > If it results in an ungly code, it's fine with the original code, > > too. But, maybe with a comment indicating that xxx_dev_attrs[] is > > defined in xxx.c. > > > > > Since sound/aoa/soundbus/sysfs is small, a solution would be > to merge sound/aoa/soundbus/sysfs.c and sound/aoa/soundus/core.c. > Moreover all 172 other usages of the ATTRIBUTE_GROUPS macro > define the struct attribute *xxx_attrs[] in the same file > they assign the .dev_groups field. > > I'm not sure about this change as it seems way more important than > adding a comment line as you suggested. Not "important" but more "radical", I'd say. > Should I send a patch merging these two files? I don't think it's worth. This is a fairly old hardware, thus the code isn't so actively used/maintained. Unless it looks too ugly, we shouldn't touch too many things just for refactoring. So, go for the way to have a minimum change. thanks, Takashi From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Date: Fri, 29 May 2015 15:53:26 +0000 Subject: Re: [PATCH] ALSA: aoa: convert bus code to use dev_groups Message-Id: List-Id: References: <1432817307-28380-1-git-send-email-lambert.quentin@gmail.com> <55671F56.6080402@gmail.com> <55672980.4010904@gmail.com> <55688A72.7000306@gmail.com> In-Reply-To: <55688A72.7000306@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Quentin Lambert Cc: Johannes Berg , Jaroslav Kysela , linuxppc-dev@lists.ozlabs.org, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org At Fri, 29 May 2015 17:49:06 +0200, Quentin Lambert wrote: > > > > On 28/05/2015 17:01, Takashi Iwai wrote: > >>> Also, it'd be better to move ATTRIBUTE_GROUPS(soundbus_dev) into > >>> soundbus/sysfs.c, and make it this global instead of > >>> soundbus_dev_attrs[]. > >> Ok, I need to find a nice way to do that because ATTRIBUTE_GROUPS > >> declares the > >> structure as static. > > > > If it results in an ungly code, it's fine with the original code, > > too. But, maybe with a comment indicating that xxx_dev_attrs[] is > > defined in xxx.c. > > > > > Since sound/aoa/soundbus/sysfs is small, a solution would be > to merge sound/aoa/soundbus/sysfs.c and sound/aoa/soundus/core.c. > Moreover all 172 other usages of the ATTRIBUTE_GROUPS macro > define the struct attribute *xxx_attrs[] in the same file > they assign the .dev_groups field. > > I'm not sure about this change as it seems way more important than > adding a comment line as you suggested. Not "important" but more "radical", I'd say. > Should I send a patch merging these two files? I don't think it's worth. This is a fairly old hardware, thus the code isn't so actively used/maintained. Unless it looks too ugly, we shouldn't touch too many things just for refactoring. So, go for the way to have a minimum change. thanks, Takashi