linux-kernel - Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 27 Apr 2015 18:25:26 +0200
From:	Martin Sperl <kernel@...tin.sperl.org>
To:	Mark Brown <broonie@...nel.org>
Cc:	Hans de Goede <hdegoede@...hat.com>,
	Michal Suchanek <hramrach@...il.com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	linux-sunxi <linux-sunxi@...glegroups.com>,
	Jonathan Corbet <corbet@....net>,
	linux-spi <linux-spi@...r.kernel.org>,
	linux-doc <linux-doc@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.


> On 27.04.2015, at 17:27, Mark Brown <broonie@...nel.org> wrote:
> 
> 
> OK, so that is just a default overlay which is abusing the fact that we
> will bind to spidev without a DT compatible and when the binding is
> undocumented (which also applies to other devices and buses sadly).
> 
> Unfortunately nobody ever mentioned this upstream and the feedback
> upstream that listing spidev in a DT is a bad idea has been ignored.

Maybe it should also have been documented as such in
Documentation/spi/spidev or in Documentation/devicetree/bindings/spi/

> The whole reason we're doing this in the first place is that we got sick
> of telling people that using spidev in DTs like this was a bad idea.


The only reference I found in my history of the spi-list is this email:
> On 08.10.2014, at 22:05, Mark Brown <broonie@...nel.org> wrote:
> 
> On Wed, Oct 08, 2014 at 02:27:08PM -0500, tthayer@...nsource.altera.com wrote:
> 
>> +			spidev@0 {
>> +				compatible = "spidev";
>> +				reg = <0>;	/* chip select */
>> +				spi-max-frequency = <100000000>;
>> +			};
> 
> No, if you're putting spidev into the DT that's broken - describe the
> hardware, not the software you're using to control it.


And google found this patch from a little earlier:
http://comments.gmane.org/gmane.linux.ports.arm.kernel/231050

So finding this piece of information on the “use-policy” is quite hard
- google finds lots of links where it is described as working that way.

> It does sound like the people maintianing the u-boot fork for the Pi
> need to talk to both u-boot upstream (nothing here is specific to the
> Pi that I can see) and the kernel community a bit more.  I'd be a bit
> worried that they may be relying on other things that just happen to
> work without being intentional (and are therefore more vulnerable to
> issues) and it's a bit depressing to see things like this stuck in a
> fork where only a limited community can make use of them.
Actually this functionality is not in u-boot, but in the firmware
boot-loader itself, which can load the kernel (and the devicetree)
without u-boot, but which can also load u-boot as an additional
intermediate boot-stage.

>> The only thing that could possibly be better would be that
>> the user would define the "real" name of the device in the
>> device tree and spidev would bind to it if there is no driver
>> available (but that would require this "fallback" binding by
>> spidev in case of no driver).
> 
> Yes, that is exactly the solution I'd suggest - change the UI to provide
> a DT compatible to be used for the new device.  That would also have the
> benefit of meaning that users who have connected some device that does
> have a driver that works with a simple binding wouldn't need to write an
> overlay which seems like it should be useful.

Well then why did you just make the system complain loudly and bringing
problems to people instead of solving it in a usable manner that does not
require people to maintain an out of tree patch to work around that warning?

We still have the one-line warning about using the depreciated 
spi_master.transfer interface, but it is not such loud warning as this one.

I guess the time spent discussing this could have been better spent
implementing that solution instead.

All we need is a volunteer to get that implemented.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/