Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1431285

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

From Michal Suchanek <hramrach@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/3] spi: spidev: fix the check for spidev in dt
Date 2016-06-26 04:20 +0200
Message-ID <rO7PX-5hD-1@gated-at.bofh.it> (permalink)
References <rNAhj-Ks-9@gated-at.bofh.it> <rNAhj-Ks-13@gated-at.bofh.it> <rO6TU-4Bp-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 26 June 2016 at 03:13, Mark Brown <broonie@kernel.org> 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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 1/3] spi: spidev: fix the check for spidev in dt Michal Suchanek <hramrach@gmail.com> - 2016-06-24 16:30 +0200
  Re: [PATCH 1/3] spi: spidev: fix the check for spidev in dt Mark Brown <broonie@kernel.org> - 2016-06-26 03:20 +0200
    Re: [PATCH 1/3] spi: spidev: fix the check for spidev in dt Michal Suchanek <hramrach@gmail.com> - 2016-06-26 04:20 +0200
      Re: [PATCH 1/3] spi: spidev: fix the check for spidev in dt Mark Brown <broonie@kernel.org> - 2016-06-26 13:10 +0200
  Re: [PATCH 1/3] spi: spidev: fix the check for spidev in dt Mark Brown <broonie@kernel.org> - 2016-06-26 03:20 +0200
    Re: [PATCH 1/3] spi: spidev: fix the check for spidev in dt Michal Suchanek <hramrach@gmail.com> - 2016-06-26 04:00 +0200

csiph-web