diff mbox

[1/3] ARM: dts: Add SPI flash node for Peach boards

Message ID 1416246184-29071-2-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Nov. 17, 2014, 5:43 p.m. UTC
From: Simon Glass <sjg@chromium.org>

Peach Pit and Pi machines have a SPI flash memory that is used to store
firmware data and different system parameters.

Add a spidev node so the flash chip can be accessed by user-space tools.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
[javier.martinez: Extend commit message]
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/exynos5420-peach-pit.dts | 26 ++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 26 ++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

Comments

Doug Anderson Nov. 18, 2014, 5:50 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Nov. 19, 2014, 10:07 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Anderson Nov. 19, 2014, 5:19 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Nov. 19, 2014, 5:29 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Nov. 19, 2014, 5:47 p.m. UTC | #5
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.
Javier Martinez Canillas Nov. 19, 2014, 5:59 p.m. UTC | #6
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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>;