| Message ID | 1416246184-29071-2-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Javier, On Mon, Nov 17, 2014 at 9:43 AM, Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote: > +&spi_1 { > + status = "okay"; > + samsung,spi-src-clk = <0>; > + num-cs = <1>; > + cs-gpios = <&gpa2 5 0>; > + > + spidev@0 { > + compatible = "spidev"; This is common practice in the Chrome OS tree, but we've gotten pushback from upstream questioning about whether "spidev" is really a physical device. See: http://www.spinics.net/lists/linux-samsung-soc/msg29563.html I don't really have an answer for something better to do here but I figured I'd at least bring up the point. -Doug
Hello Doug, Thanks for your feedback. On 11/18/2014 06:50 PM, Doug Anderson wrote: > This is common practice in the Chrome OS tree, but we've gotten > pushback from upstream questioning about whether "spidev" is really a > physical device. See: > > http://www.spinics.net/lists/linux-samsung-soc/msg29563.html > I see, I thought that it was a common practice in the mainline kernel too since I saw that many board DTS currently have a spidev node: $ git grep 'compatible = "spidev"' arch/arm/boot/dts/ | wc -l 19 > > I don't really have an answer for something better to do here but I > figured I'd at least bring up the point. > I wonder how the spidev user-space interface is supposed to be used when booting with Device Trees. Best regards, Javier
Javier, On Wed, Nov 19, 2014 at 2:07 AM, Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote: > Hello Doug, > > Thanks for your feedback. > > On 11/18/2014 06:50 PM, Doug Anderson wrote: >> This is common practice in the Chrome OS tree, but we've gotten >> pushback from upstream questioning about whether "spidev" is really a >> physical device. See: >> >> http://www.spinics.net/lists/linux-samsung-soc/msg29563.html >> > > I see, I thought that it was a common practice in the mainline kernel > too since I saw that many board DTS currently have a spidev node: > > $ git grep 'compatible = "spidev"' arch/arm/boot/dts/ | wc -l > 19 > >> >> I don't really have an answer for something better to do here but I >> figured I'd at least bring up the point. >> > > I wonder how the spidev user-space interface is supposed to be used > when booting with Device Trees. OK. Please don't take my comments as a NAK on this patch. I should have done the same grep myself before sending--sorry. I just remembered the old conversation and looked for that instead. If the convention is to use "spidev" like this then I guess we're OK. I do wish it was a little more like "i2c" myself where you could get a direct access interface no matter what driver was bound underneath (and also if no drivers were bound underneath). ...but I could just be naive. ;) -Doug
Hello Doug, On 11/19/2014 06:19 PM, Doug Anderson wrote: >> >> I wonder how the spidev user-space interface is supposed to be used >> when booting with Device Trees. > > OK. Please don't take my comments as a NAK on this patch. I should > have done the same grep myself before sending--sorry. I just > remembered the old conversation and looked for that instead. > Ok, let's see what others say. At the very least documentation about the spidev DT binding should be added to Documentation/devicetree/bindings/ > If the convention is to use "spidev" like this then I guess we're OK. > I do wish it was a little more like "i2c" myself where you could get a > direct access interface no matter what driver was bound underneath > (and also if no drivers were bound underneath). ...but I could just > be naive. ;) > +1 > -Doug > Best regards, Javier
On Wed, Nov 19, 2014 at 09:19:13AM -0800, Doug Anderson wrote: Please stop CCing my work address for upstream things. > On Wed, Nov 19, 2014 at 2:07 AM, Javier Martinez Canillas > > I see, I thought that it was a common practice in the mainline kernel > > too since I saw that many board DTS currently have a spidev node: > > $ git grep 'compatible = "spidev"' arch/arm/boot/dts/ | wc -l > > 19 These are bugs. The device tree should describe the hardware, spidev is a Linux implementation detail. Provide a compatible string for the device that is there just as you would for any other device. > OK. Please don't take my comments as a NAK on this patch. I should > have done the same grep myself before sending--sorry. I just > remembered the old conversation and looked for that instead. Please take this as one.
Hello Mark, On 11/19/2014 06:47 PM, Mark Brown wrote: > These are bugs. The device tree should describe the hardware, spidev is > a Linux implementation detail. Provide a compatible string for the > device that is there just as you would for any other device. > Thanks a lot for your explanation. >> OK. Please don't take my comments as a NAK on this patch. I should >> have done the same grep myself before sending--sorry. I just >> remembered the old conversation and looked for that instead. > > Please take this as one. > Ok Best regards, Javier
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts index 9a050e1..e7c2a9f 100644 --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts @@ -703,6 +703,13 @@ samsung,pin-drv = <2>; }; + spi_flash_cs: spi-flash-cs { + samsung,pins = "gpa2-5"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + usb300_vbus_en: usb300-vbus-en { samsung,pins = "gph0-0"; samsung,pin-function = <1>; @@ -732,6 +739,25 @@ clock-names = "rtc", "rtc_src"; }; +&spi_1 { + status = "okay"; + samsung,spi-src-clk = <0>; + num-cs = <1>; + cs-gpios = <&gpa2 5 0>; + + spidev@0 { + compatible = "spidev"; + reg = <0>; + spi-max-frequency = <50000000>; + pinctrl-names = "default"; + pinctrl-0 = <&spi_flash_cs>; + + controller-data { + samsung,spi-feedback-delay = <2>; + }; + }; +}; + &spi_2 { status = "okay"; num-cs = <1>; diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index e8fdda8..7a0a2a6 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -691,6 +691,13 @@ samsung,pin-drv = <2>; }; + spi_flash_cs: spi-flash-cs { + samsung,pins = "gpa2-5"; + samsung,pin-function = <1>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + usb300_vbus_en: usb300-vbus-en { samsung,pins = "gph0-0"; samsung,pin-function = <1>; @@ -720,6 +727,25 @@ clock-names = "rtc", "rtc_src"; }; +&spi_1 { + status = "okay"; + samsung,spi-src-clk = <0>; + num-cs = <1>; + cs-gpios = <&gpa2 5 0>; + + spidev@0 { + compatible = "spidev"; + reg = <0>; + spi-max-frequency = <50000000>; + pinctrl-names = "default"; + pinctrl-0 = <&spi_flash_cs>; + + controller-data { + samsung,spi-feedback-delay = <2>; + }; + }; +}; + &spi_2 { status = "okay"; num-cs = <1>;