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
