From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Andrew F. Davis" Subject: Re: [PATCH v2 09/19] ASoC: tlv320aic31xx: Remove platform data Date: Wed, 6 Dec 2017 13:15:44 -0600 Message-ID: <611f3eed-b82a-c997-4bf9-e3a4f1b7e94e@ti.com> References: <20171129213300.20021-1-afd@ti.com> <20171129213300.20021-10-afd@ti.com> <20171201132606.vyw7wthmql5trkwq@sirena.org.uk> <4fee76ef-e518-69b6-3bcc-f65b0a31cacc@ti.com> <20171206124540.GC1827@finisterre> <10f1b19b-4a0a-e336-b757-933d46b5c8e9@ti.com> <20171206173059.qw4tt7nizmyo6xbl@sirena.org.uk> <20171206181534.7t23ljja6fgle4ar@sirena.org.uk> <86e13e26-e8d5-35ed-6bd0-d91d9323d5e6@ti.com> <20171206191112.ip47bylnvyfc2irt@sirena.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20171206191112.ip47bylnvyfc2irt@sirena.org.uk> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Mark Brown Cc: Mark Rutland , devicetree@vger.kernel.org, alsa-devel@alsa-project.org, Tony Lindgren , Liam Girdwood , linux-kernel@vger.kernel.org, Rob Herring , =?UTF-8?Q?Beno=c3=aet_Cousson?= List-Id: devicetree@vger.kernel.org On 12/06/2017 01:11 PM, Mark Brown wrote: > On Wed, Dec 06, 2017 at 12:40:45PM -0600, Andrew F. Davis wrote: > >> For some userspace feature sure, but this is kernel code, there is no >> guarantee for a sable API, in fact some would probably argue even >> further that there is a guarantee that stuff *will* change and this is a >> good thing as it kinda serves to punish for those you don't try to upstream. > >> So the helpfulness bar should be zero for changes that break out-of-tree >> stuff. > > There is no need to actively get in people's way or put up barriers to > people who do in future decide to upstream things, that doesn't help > anyone. > >> Even more so this patch isn't a zero gain, the cleaner, better looking, >> and easier to maintain code *is* the benefit in itself. Plus we gain the >> ability to set mic-gain voltage with ACPI, something you couldn't do >> before this patch. > > If this patch adds ACPI support then the patch description was clearly > not great (I don't think I read the patch itself since the description > just said that it was removing platform data without giving a reason, > that's the main review comment here). > I may not be clear that the ACPI part is new, but the message does say "and switch to using fwnode(DT/ACPI)" > If you want to use the device property stuff that's fine but there's no > need to remove platform data to do that, it's a smaller change. I find > it hard to see the platform data as a particularly big blight on the > code here, looking at the driver it's just going to remove the "pdata." > from a few variable accesses which isn't exactly transformational. > If keeping platform data is that important to you then I will split the patch into fwnode addition and pdata removal, you can just not take the pdata removal if you don't want it.