Re: [PATCH 1/3] spi: spidev: fix the check for spidev in dt

[Michal Suchanek]

On 26 June 2016 at 03:13, Mark Brown <broonie@xxxxxxxxxx> wrote:
> On Thu, Jun 23, 2016 at 05:41:19PM -0000, Michal Suchanek wrote:
>
>> The check is supposed to warn about spidev specified directly in
>> devicetree as compatible. This just does not work. I have a devicetree
>> with no compatible whatsoever and hacked my kernel so I can manually
>> bind spidev. This still triggers.
>
> Well, a DT device won't instantiate without a compatible string...
> could you please explain exactly what makes you say this won't work?

That's because the whitelist concept for this check is completely broken.

Without any patches whatsoever I should be able to specify m25p80
binding in the DT, let the kernel create the device, unbind the
driver, and bind spidev.

Then I have the jedec,spi-nor compatible which is not on the whitelist.

>
>> Also I have no idea how this could have build with ! CONFIG_OF since the
>> id table which the code checks is not compiled then.
>
> of_match_device() compiles out when !OF.
>
>> +static const struct of_device_id spidev_check[] = {
>> +                     { .compatible = "spidev" },
>> +                     {}
>> +};
>
> The indentation here is completely non-standard.
>
>> -     if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)) {
>> +     if (spi->dev.of_node && of_match_device(spidev_check, &spi->dev)) {
>
> I think what you intend to say in the commit message is that you want to
> change from a whitelist to a blacklist since that is what the code says,
> but like I say we also need an explanation of the logic behind such a
> change.

It's because the check kernel log message says it's a blacklist and
it's incorrectly implemented as a whitelist.

The change is to correct that.

Thanks

Michal