Re: [PATCH] drm/ssd130x: Set SPI .id_table to prevent an SPI core warning

[Javier Martinez Canillas <javierm@redhat.com>]

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

Hello Dmitry,

> On Tue, Dec 31, 2024 at 12:44:58PM +0100, Javier Martinez Canillas wrote:
>> The only reason for the ssd130x-spi driver to have an spi_device_id table
>> is that the SPI core always reports an "spi:" MODALIAS, even when the SPI
>> device has been registered via a Device Tree Blob.
>> 
>> Without spi_device_id table information in the module's metadata, module
>> autoloading would not work because there won't be an alias that matches
>> the MODALIAS reported by the SPI core.
>> 
>> This spi_device_id table is not needed for device matching though, since
>> the of_device_id table is always used in this case. For this reason, the
>> struct spi_driver .id_table field is currently not set in the SPI driver.
>> 
>> Because the spi_device_id table is always required for module autoloading,
>> the SPI core checks during driver registration that both an of_device_id
>> table and a spi_device_id table are present and that they contain the same
>> entries for all the SPI devices.
>> 
>> Not setting the .id_table field in the driver then confuses the core and
>> leads to the following warning when the ssd130x-spi driver is registered:
>> 
>>   [   41.091198] SPI driver ssd130x-spi has no spi_device_id for sinowealth,sh1106
>>   [   41.098614] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1305
>>   [   41.105862] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1306
>>   [   41.113062] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1307
>>   [   41.120247] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1309
>>   [   41.127449] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1322
>>   [   41.134627] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1325
>>   [   41.141784] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1327
>>   [   41.149021] SPI driver ssd130x-spi has no spi_device_id for solomon,ssd1331
>> 
>> To prevent the warning, set the .id_table even though it's not necessary.
>> 
>> Since the check is done even for built-in drivers, drop the condition to
>> only define the ID table when the driver is built as a module. Finally,
>> rename the variable to use the "_spi_id" convention used for ID tables.
>> 
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Fixes: 74373977d2ca ("drm/solomon: Add SSD130x OLED displays SPI support")
>

I was on the fence about adding a Fixes: tag due a) the issue being there
from the beginning as you pointed out and b) the warning being harmless.

But I'll add it to v2 or just before pushing it to drm-misc-next.

> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>

Thanks for your review!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat