From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753467AbcGTLWg (ORCPT ); Wed, 20 Jul 2016 07:22:36 -0400 Received: from mga11.intel.com ([192.55.52.93]:20400 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752356AbcGTLW2 (ORCPT ); Wed, 20 Jul 2016 07:22:28 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,393,1464678000"; d="scan'208";a="737146783" Subject: Re: [PATCH v2 3/3] acpi spi: Initialize modalias from of_compatible To: Mark Brown References: <1d26e956f12fcd86ad915659adeb86e242b96ac7.1468409668.git.leonard.crestez@intel.com> <20160719102213.GZ30372@sirena.org.uk> Cc: linux-acpi@vger.kernel.org, "Rafael J. Wysocki" , Jarkko Nikula , Mika Westerberg , Len Brown , linux-i2c@vger.kernel.org, Wolfram Sang , linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org, Octavian Purdila From: Crestez Dan Leonard Message-ID: Date: Wed, 20 Jul 2016 14:21:55 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160719102213.GZ30372@sirena.org.uk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/19/2016 01:22 PM, Mark Brown wrote: > On Wed, Jul 13, 2016 at 02:53:42PM +0300, Crestez Dan Leonard wrote: >> When using devicetree spi_device.modalias is set to the compatible >> string with the vendor prefix removed. For SPI devices described via >> ACPI the i2c_board_info.type string is initialized by acpi_device_hid. >> When using ACPI and DT ids this string ends up something like "PRP0001". > > Please submit patches using subject lines reflecting the style for the > subsystem. This makes it easier for people to identify relevant > patches. Look at what existing commits in the area you're changing are > doing and make sure your subject lines visually resemble what they're > doing. So the prefix should be something like "spi: acpi: "? >> Change acpi_register_spi_device to use the of_compatible property if >> present. This makes it easier to instantiate spi drivers through ACPI >> with DT ids. > > This is basically fine but... > >> + if (adev->data.of_compatible) { >> + ret = acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)); >> + if (ret) { >> + spi_dev_put(spi); >> + return AE_NOT_FOUND; >> + } > > The only reason this could fail currently is that there wasn't a > compatible in the first place so why don't we just handle it like the no > compatible case? It's probably not realistic but it seems like there's > a small chance this could regress some platform if we do add more error > detection in acpi_of_modalias(). If acpi_of_modalias fails for some new reason wouldn't it be better to fail explicitly rather than ignore it? -- Regards, Leonard