Re: [PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

Re: [PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

[Dinh Nguyen]

On 10/07/2014 02:48 PM, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> 1. The of_node element must be initialized to enable discovery of node
> children. The discovery takes place in the of_register_spi_devices() function.
> 2. Grabbing the bus-num from the device tree.
> 3. Update the SOCFPGA configuration.

updating the socfpga_defconfig should be a separate patch.

Dinh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] SPI: Enable devicetree spi child nodes

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

This patch series is used to enable the spidev as a child of the
SPI device tree node. This patch was tested on the Altera
SOCFPGA platform.

Thor Thayer (2):
  spi: dw: Initialize of_node to discover device tree node children
  arm: dts: socfpga: Add SPI nodes & update copyright.

 Documentation/devicetree/bindings/spi/spi-dw.txt |    1 +
 arch/arm/boot/dts/socfpga.dtsi                   |   52 ++++++++++++++++++----
 arch/arm/configs/socfpga_defconfig               |    1 +
 drivers/spi/spi-dw-mmio.c                        |    5 ++-
 drivers/spi/spi-dw.c                             |    3 ++
 5 files changed, 53 insertions(+), 9 deletions(-)

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

1. The of_node element must be initialized to enable discovery of node
children. The discovery takes place in the of_register_spi_devices() function.
2. Grabbing the bus-num from the device tree.
3. Update the SOCFPGA configuration.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 arch/arm/configs/socfpga_defconfig |    1 +
 drivers/spi/spi-dw-mmio.c          |    5 ++++-
 drivers/spi/spi-dw.c               |    3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index d7a5855..54f3540 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -52,6 +52,7 @@ CONFIG_CAN_C_CAN_PLATFORM=y
 CONFIG_CAN_DEBUG_DEVICES=y
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_PROC_DEVICETREE=y
 CONFIG_BLK_DEV_RAM=y
 CONFIG_BLK_DEV_RAM_COUNT=2
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 21ce0e3..b8483057 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -82,7 +82,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	dws->num_cs = num_cs;
 
 	if (pdev->dev.of_node) {
-		int i;
+		int i, prop;
+
+		if (!of_property_read_u32(pdev->dev.of_node, "bus-num", &prop))
+			dws->bus_num = prop;
 
 		for (i = 0; i < dws->num_cs; i++) {
 			int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 0dd0623..d8be9f9 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -671,6 +671,9 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	master->cleanup = dw_spi_cleanup;
 	master->transfer_one_message = dw_spi_transfer_one_message;
 	master->max_speed_hz = dws->max_freq;
+#if defined(CONFIG_OF)
+	master->dev.of_node = dev->of_node;
+#endif
 
 	/* Basic HW init */
 	spi_hw_init(dws);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] arm: dts: socfpga: Add SPI nodes & update copyright.

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add 2 SPI nodes to SOCFPGA device tree. Update copyright.
Update spi-dw.txt with bus-num as an optional property.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/spi-dw.txt |    1 +
 arch/arm/boot/dts/socfpga.dtsi                   |   52 ++++++++++++++++++----
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-dw.txt b/Documentation/devicetree/bindings/spi/spi-dw.txt
index 7b63ed6..f1d54e6 100644
--- a/Documentation/devicetree/bindings/spi/spi-dw.txt
+++ b/Documentation/devicetree/bindings/spi/spi-dw.txt
@@ -11,6 +11,7 @@ Required properties:
 
 Optional properties:
 - cs-gpios: see spi-bus.txt
+- bus-num: see spi-bus.txt
 
 Example:
 
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 4d77ad6..42855bc 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -1,15 +1,14 @@
 /*
- *  Copyright (C) 2012 Altera <www.altera.com>
+ * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
  *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
@@ -628,6 +627,43 @@
 			clock-names = "biu", "ciu";
 		};
 
+		spi0: spi@fff00000 {
+			compatible = "snps,dw-apb-ssi";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0xfff00000 0x1000>;
+			interrupts = <0 154 4>;
+			num-cs = <4>;
+			bus-num = <0>;
+			clocks = <&per_base_clk>;
+
+			spidev@0 {
+				compatible = "spidev";
+				reg = <0>;	/* chip select */
+				spi-max-frequency = <100000000>;
+				enable-dma = <1>;
+			};
+		};
+
+		spi1: spi@fff01000 {
+			compatible = "snps,dw-apb-ssi";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0xfff01000 0x1000>;
+			interrupts = <0 156 4>;
+			num-cs = <4>;
+			bus-num = <1>;
+			clocks = <&per_base_clk>;
+			status = "disabled";
+
+			spidev@0 {
+				compatible = "spidev";
+				reg = <0>;
+				spi-max-frequency = <100000000>;
+				enable-dma = <1>;
+			};
+		};
+
 		/* Local timer */
 		timer@fffec600 {
 			compatible = "arm,cortex-a9-twd-timer";
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

[Thor Thayer]

On 10/07/2014 02:42 PM, Dinh Nguyen wrote:
> On 10/07/2014 02:48 PM, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
>> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
>>
>> 1. The of_node element must be initialized to enable discovery of node
>> children. The discovery takes place in the of_register_spi_devices() function.
>> 2. Grabbing the bus-num from the device tree.
>> 3. Update the SOCFPGA configuration.
> updating the socfpga_defconfig should be a separate patch.
>
> Dinh
OK, I will remove it. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] arm: dts: socfpga: Add SPI nodes & update copyright.

[Steffen Trumtrar]

Hi!

On Tue, Oct 07, 2014 at 02:48:17PM -0500, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> Add 2 SPI nodes to SOCFPGA device tree. Update copyright.
> Update spi-dw.txt with bus-num as an optional property.
> 
> Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/spi/spi-dw.txt |    1 +
>  arch/arm/boot/dts/socfpga.dtsi                   |   52 ++++++++++++++++++----
>  2 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-dw.txt b/Documentation/devicetree/bindings/spi/spi-dw.txt
> index 7b63ed6..f1d54e6 100644
> --- a/Documentation/devicetree/bindings/spi/spi-dw.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-dw.txt
> @@ -11,6 +11,7 @@ Required properties:
>  
>  Optional properties:
>  - cs-gpios: see spi-bus.txt
> +- bus-num: see spi-bus.txt
>  
>  Example:
>  
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index 4d77ad6..42855bc 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -1,15 +1,14 @@
>  /*
> - *  Copyright (C) 2012 Altera <www.altera.com>
> + * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> + * it under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
>   *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.

This shouldn't be in the same patch IMHO.

>   *
>   * You should have received a copy of the GNU General Public License
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> @@ -628,6 +627,43 @@
>  			clock-names = "biu", "ciu";
>  		};
>  
> +		spi0: spi@fff00000 {
> +			compatible = "snps,dw-apb-ssi";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0xfff00000 0x1000>;
> +			interrupts = <0 154 4>;
> +			num-cs = <4>;
> +			bus-num = <0>;
> +			clocks = <&per_base_clk>;
> +
> +			spidev@0 {
> +				compatible = "spidev";
> +				reg = <0>;	/* chip select */
> +				spi-max-frequency = <100000000>;
> +				enable-dma = <1>;
> +			};
> +		};
> +
> +		spi1: spi@fff01000 {
> +			compatible = "snps,dw-apb-ssi";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0xfff01000 0x1000>;
> +			interrupts = <0 156 4>;
> +			num-cs = <4>;
> +			bus-num = <1>;
> +			clocks = <&per_base_clk>;
> +			status = "disabled";
> +
> +			spidev@0 {
> +				compatible = "spidev";
> +				reg = <0>;
> +				spi-max-frequency = <100000000>;
> +				enable-dma = <1>;
> +			};
> +		};
> +

I don't think, this is the right place. Put it in the board dts.
Oh, and why is spi0 not disabled but spi1 is?

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

On Tue, Oct 07, 2014 at 02:48:16PM -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> 1. The of_node element must be initialized to enable discovery of node
> children. The discovery takes place in the of_register_spi_devices() function.
> 2. Grabbing the bus-num from the device tree.
> 3. Update the SOCFPGA configuration.

No, each of these should be in a separate patch - it's especially bad to
be updating the defconfig in the same patch as the driver, these changes
should be totally unrelated.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 2/2] arm: dts: socfpga: Add SPI nodes & update copyright.

[Thor Thayer]

On 10/07/2014 03:31 PM, Steffen Trumtrar wrote:
> Hi!
>
> On Tue, Oct 07, 2014 at 02:48:17PM -0500, tthayer@opensource.altera.com wrote:
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> Add 2 SPI nodes to SOCFPGA device tree. Update copyright.
>> Update spi-dw.txt with bus-num as an optional property.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>> ---
>>   Documentation/devicetree/bindings/spi/spi-dw.txt |    1 +
>>   arch/arm/boot/dts/socfpga.dtsi                   |   52 ++++++++++++++++++----
>>   2 files changed, 45 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-dw.txt b/Documentation/devicetree/bindings/spi/spi-dw.txt
>> index 7b63ed6..f1d54e6 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-dw.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-dw.txt
>> @@ -11,6 +11,7 @@ Required properties:
>>   
>>   Optional properties:
>>   - cs-gpios: see spi-bus.txt
>> +- bus-num: see spi-bus.txt
>>   
>>   Example:
>>   
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
>> index 4d77ad6..42855bc 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -1,15 +1,14 @@
>>   /*
>> - *  Copyright (C) 2012 Altera <www.altera.com>
>> + * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
>>    *
>>    * This program is free software; you can redistribute it and/or modify
>> - * it under the terms of the GNU General Public License as published by
>> - * the Free Software Foundation; either version 2 of the License, or
>> - * (at your option) any later version.
>> + * it under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>>    *
>> - * This program is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> - * GNU General Public License for more details.
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
> This shouldn't be in the same patch IMHO.
Hi Steffen,

Are you saying the socfpga.dtsi changes shouldn't be in the patch or the 
copyright update?

If the copyright update, it seemed like a good time since there were 
other changes.

>>    *
>>    * You should have received a copy of the GNU General Public License
>>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> @@ -628,6 +627,43 @@
>>   			clock-names = "biu", "ciu";
>>   		};
>>   
>> +		spi0: spi@fff00000 {
>> +			compatible = "snps,dw-apb-ssi";
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			reg = <0xfff00000 0x1000>;
>> +			interrupts = <0 154 4>;
>> +			num-cs = <4>;
>> +			bus-num = <0>;
>> +			clocks = <&per_base_clk>;
>> +
>> +			spidev@0 {
>> +				compatible = "spidev";
>> +				reg = <0>;	/* chip select */
>> +				spi-max-frequency = <100000000>;
>> +				enable-dma = <1>;
>> +			};
>> +		};
>> +
>> +		spi1: spi@fff01000 {
>> +			compatible = "snps,dw-apb-ssi";
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			reg = <0xfff01000 0x1000>;
>> +			interrupts = <0 156 4>;
>> +			num-cs = <4>;
>> +			bus-num = <1>;
>> +			clocks = <&per_base_clk>;
>> +			status = "disabled";
>> +
>> +			spidev@0 {
>> +				compatible = "spidev";
>> +				reg = <0>;
>> +				spi-max-frequency = <100000000>;
>> +				enable-dma = <1>;
>> +			};
>> +		};
>> +
> I don't think, this is the right place. Put it in the board dts.
> Oh, and why is spi0 not disabled but spi1 is?
OK. I can put it in the board dts. I thought the IP would go in the 
socfpga.dtsi file since it describes the chip.

I understand your point which is why the 2nd SPI is disabled - it is not 
pinned out on the Development Kit board.

Thanks,

Thor
>
> Regards,
> Steffen
>

Re: [PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

[Thor Thayer]

On 10/07/2014 03:40 PM, Mark Brown wrote:
> On Tue, Oct 07, 2014 at 02:48:16PM -0500, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
>> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
>>
>> 1. The of_node element must be initialized to enable discovery of node
>> children. The discovery takes place in the of_register_spi_devices() function.
>> 2. Grabbing the bus-num from the device tree.
>> 3. Update the SOCFPGA configuration.
> No, each of these should be in a separate patch - it's especially bad to
> be updating the defconfig in the same patch as the driver, these changes
> should be totally unrelated.
OK. I will split these and remove the defconfig.

Dinh pointed the defconfig issue out to me as well.

Thanks for reviewing,

Thor
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] arm: dts: socfpga: Add SPI nodes & update copyright.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On Tue, Oct 07, 2014 at 02:48:17PM -0500, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:

> Add 2 SPI nodes to SOCFPGA device tree. Update copyright.
> Update spi-dw.txt with bus-num as an optional property.

Again, this is just randomly mixing multiple changes.  Also why are we
adding a bus-num property - bus numbers are an implementation detail of
Linux and I can't see any reason to put them in the device tree.  If
there were one the binding would need to explain what they were.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 2/2] arm: dts: socfpga: Add SPI nodes & update copyright.

[Steffen Trumtrar]

Hi!

On Tue, Oct 07, 2014 at 03:55:54PM -0500, Thor Thayer wrote:
> 
> On 10/07/2014 03:31 PM, Steffen Trumtrar wrote:
> >Hi!
> >
> >On Tue, Oct 07, 2014 at 02:48:17PM -0500, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
> >>From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> >>
> >>Add 2 SPI nodes to SOCFPGA device tree. Update copyright.
> >>Update spi-dw.txt with bus-num as an optional property.
> >>
> >>Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> >>---
> >>  Documentation/devicetree/bindings/spi/spi-dw.txt |    1 +
> >>  arch/arm/boot/dts/socfpga.dtsi                   |   52 ++++++++++++++++++----
> >>  2 files changed, 45 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/spi/spi-dw.txt b/Documentation/devicetree/bindings/spi/spi-dw.txt
> >>index 7b63ed6..f1d54e6 100644
> >>--- a/Documentation/devicetree/bindings/spi/spi-dw.txt
> >>+++ b/Documentation/devicetree/bindings/spi/spi-dw.txt
> >>@@ -11,6 +11,7 @@ Required properties:
> >>  Optional properties:
> >>  - cs-gpios: see spi-bus.txt
> >>+- bus-num: see spi-bus.txt
> >>  Example:
> >>diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> >>index 4d77ad6..42855bc 100644
> >>--- a/arch/arm/boot/dts/socfpga.dtsi
> >>+++ b/arch/arm/boot/dts/socfpga.dtsi
> >>@@ -1,15 +1,14 @@
> >>  /*
> >>- *  Copyright (C) 2012 Altera <www.altera.com>
> >>+ * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
> >>   *
> >>   * This program is free software; you can redistribute it and/or modify
> >>- * it under the terms of the GNU General Public License as published by
> >>- * the Free Software Foundation; either version 2 of the License, or
> >>- * (at your option) any later version.
> >>+ * it under the terms and conditions of the GNU General Public License,
> >>+ * version 2, as published by the Free Software Foundation.
> >>   *
> >>- * This program is distributed in the hope that it will be useful,
> >>- * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>- * GNU General Public License for more details.
> >>+ * This program is distributed in the hope it will be useful, but WITHOUT
> >>+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >>+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> >>+ * more details.
> >This shouldn't be in the same patch IMHO.
> Hi Steffen,
> 
> Are you saying the socfpga.dtsi changes shouldn't be in the patch or
> the copyright update?
> 
> If the copyright update, it seemed like a good time since there were
> other changes.
> 

Going with what Mark Brown said: Split it up more.

> >>   *
> >>   * You should have received a copy of the GNU General Public License
> >>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >>@@ -628,6 +627,43 @@
> >>  			clock-names = "biu", "ciu";
> >>  		};
> >>+		spi0: spi@fff00000 {
> >>+			compatible = "snps,dw-apb-ssi";
> >>+			#address-cells = <1>;
> >>+			#size-cells = <0>;
> >>+			reg = <0xfff00000 0x1000>;
> >>+			interrupts = <0 154 4>;
> >>+			num-cs = <4>;
> >>+			bus-num = <0>;
> >>+			clocks = <&per_base_clk>;
> >>+
> >>+			spidev@0 {
> >>+				compatible = "spidev";
> >>+				reg = <0>;	/* chip select */
> >>+				spi-max-frequency = <100000000>;
> >>+				enable-dma = <1>;
> >>+			};
> >>+		};
> >>+
> >>+		spi1: spi@fff01000 {
> >>+			compatible = "snps,dw-apb-ssi";
> >>+			#address-cells = <1>;
> >>+			#size-cells = <0>;
> >>+			reg = <0xfff01000 0x1000>;
> >>+			interrupts = <0 156 4>;
> >>+			num-cs = <4>;
> >>+			bus-num = <1>;
> >>+			clocks = <&per_base_clk>;
				   ^^^^^^^^^^^^
Shouldn't this be spi_m_clk?

> >>+			status = "disabled";
> >>+
> >>+			spidev@0 {
> >>+				compatible = "spidev";
> >>+				reg = <0>;
> >>+				spi-max-frequency = <100000000>;
> >>+				enable-dma = <1>;
> >>+			};
> >>+		};
> >>+
> >I don't think, this is the right place. Put it in the board dts.
> >Oh, and why is spi0 not disabled but spi1 is?
> OK. I can put it in the board dts. I thought the IP would go in the
> socfpga.dtsi file since it describes the chip.
> 

Okay, I see that my comment is unclear. What I wanted to say is:
put the spidev into your board dts. Other boards might have something
else connected there.

> I understand your point which is why the 2nd SPI is disabled - it is
> not pinned out on the Development Kit board.
> 

Okay. Then disable both in the dtsi and enable the one you want to use
in your board file. The dtsi should only have stuff that is common to
ALL Socfpgas, so the IPs (if they are the same, which AFAIK they are).

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

[Andy Shevchenko]

On Tue, 2014-10-07 at 14:48 -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> 1. The of_node element must be initialized to enable discovery of node
> children. The discovery takes place in the of_register_spi_devices() function.
> 2. Grabbing the bus-num from the device tree.
> 3. Update the SOCFPGA configuration.

[]

> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -671,6 +671,9 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>  	master->cleanup = dw_spi_cleanup;
>  	master->transfer_one_message = dw_spi_transfer_one_message;
>  	master->max_speed_hz = dws->max_freq;
> +#if defined(CONFIG_OF)
> +	master->dev.of_node = dev->of_node;
> +#endif

I'm not sure you need this #ifdef surrounded an assignment.
If I'm wrong then you probably have to use if (IS_ENABLED(CONFIG_OF)).

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

Re: [PATCH 2/2] arm: dts: socfpga: Add SPI nodes & update copyright.

[Thor Thayer]

On 10/07/2014 06:12 PM, Mark Brown wrote:
> On Tue, Oct 07, 2014 at 02:48:17PM -0500,tthayer@opensource.altera.com  wrote:
>
>> Add 2 SPI nodes to SOCFPGA device tree. Update copyright.
>> Update spi-dw.txt with bus-num as an optional property.
> Again, this is just randomly mixing multiple changes.  Also why are we
> adding a bus-num property - bus numbers are an implementation detail of
> Linux and I can't see any reason to put them in the device tree.  If
> there were one the binding would need to explain what they were.
OK. I will leave these changes out of the patch.

Sorry about the noise. I saw the spi-summary referenced the bus-num 
spi<bus-num>.<chip-select> so I thought it could be added to the 
bindings document. The bus was defaulting to 32766 if the bus wasn't 
assigned from the device tree (spi32766.0).

Thanks,

Thor

Re: [PATCH 2/2] arm: dts: socfpga: Add SPI nodes & update copyright.

[Thor Thayer]

On 10/08/2014 03:01 AM, Steffen Trumtrar wrote:
> Hi!
>
> On Tue, Oct 07, 2014 at 03:55:54PM -0500, Thor Thayer wrote:
>> On 10/07/2014 03:31 PM, Steffen Trumtrar wrote:
>>> Hi!
>>>
>>> On Tue, Oct 07, 2014 at 02:48:17PM -0500, tthayer@opensource.altera.com wrote:
>>>> From: Thor Thayer <tthayer@opensource.altera.com>
>>>>
>>>> Add 2 SPI nodes to SOCFPGA device tree. Update copyright.
>>>> Update spi-dw.txt with bus-num as an optional property.
>>>>
>>>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/spi/spi-dw.txt |    1 +
>>>>   arch/arm/boot/dts/socfpga.dtsi                   |   52 ++++++++++++++++++----
>>>>   2 files changed, 45 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-dw.txt b/Documentation/devicetree/bindings/spi/spi-dw.txt
>>>> index 7b63ed6..f1d54e6 100644
>>>> --- a/Documentation/devicetree/bindings/spi/spi-dw.txt
>>>> +++ b/Documentation/devicetree/bindings/spi/spi-dw.txt
>>>> @@ -11,6 +11,7 @@ Required properties:
>>>>   Optional properties:
>>>>   - cs-gpios: see spi-bus.txt
>>>> +- bus-num: see spi-bus.txt
>>>>   Example:
>>>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
>>>> index 4d77ad6..42855bc 100644
>>>> --- a/arch/arm/boot/dts/socfpga.dtsi
>>>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>>>> @@ -1,15 +1,14 @@
>>>>   /*
>>>> - *  Copyright (C) 2012 Altera <www.altera.com>
>>>> + * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
>>>>    *
>>>>    * This program is free software; you can redistribute it and/or modify
>>>> - * it under the terms of the GNU General Public License as published by
>>>> - * the Free Software Foundation; either version 2 of the License, or
>>>> - * (at your option) any later version.
>>>> + * it under the terms and conditions of the GNU General Public License,
>>>> + * version 2, as published by the Free Software Foundation.
>>>>    *
>>>> - * This program is distributed in the hope that it will be useful,
>>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> - * GNU General Public License for more details.
>>>> + * This program is distributed in the hope it will be useful, but WITHOUT
>>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>>>> + * more details.
>>> This shouldn't be in the same patch IMHO.
>> Hi Steffen,
>>
>> Are you saying the socfpga.dtsi changes shouldn't be in the patch or
>> the copyright update?
>>
>> If the copyright update, it seemed like a good time since there were
>> other changes.
>>
> Going with what Mark Brown said: Split it up more.
Hi Steffen!

OK. I will do that.
>>>>    *
>>>>    * You should have received a copy of the GNU General Public License
>>>>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>> @@ -628,6 +627,43 @@
>>>>   			clock-names = "biu", "ciu";
>>>>   		};
>>>> +		spi0: spi@fff00000 {
>>>> +			compatible = "snps,dw-apb-ssi";
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <0>;
>>>> +			reg = <0xfff00000 0x1000>;
>>>> +			interrupts = <0 154 4>;
>>>> +			num-cs = <4>;
>>>> +			bus-num = <0>;
>>>> +			clocks = <&per_base_clk>;
>>>> +
>>>> +			spidev@0 {
>>>> +				compatible = "spidev";
>>>> +				reg = <0>;	/* chip select */
>>>> +				spi-max-frequency = <100000000>;
>>>> +				enable-dma = <1>;
>>>> +			};
>>>> +		};
>>>> +
>>>> +		spi1: spi@fff01000 {
>>>> +			compatible = "snps,dw-apb-ssi";
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <0>;
>>>> +			reg = <0xfff01000 0x1000>;
>>>> +			interrupts = <0 156 4>;
>>>> +			num-cs = <4>;
>>>> +			bus-num = <1>;
>>>> +			clocks = <&per_base_clk>;
> 				   ^^^^^^^^^^^^
> Shouldn't this be spi_m_clk?
Yes. Thank you!
>>>> +			status = "disabled";
>>>> +
>>>> +			spidev@0 {
>>>> +				compatible = "spidev";
>>>> +				reg = <0>;
>>>> +				spi-max-frequency = <100000000>;
>>>> +				enable-dma = <1>;
>>>> +			};
>>>> +		};
>>>> +
>>> I don't think, this is the right place. Put it in the board dts.
>>> Oh, and why is spi0 not disabled but spi1 is?
>> OK. I can put it in the board dts. I thought the IP would go in the
>> socfpga.dtsi file since it describes the chip.
>>
> Okay, I see that my comment is unclear. What I wanted to say is:
> put the spidev into your board dts. Other boards might have something
> else connected there.
>
>> I understand your point which is why the 2nd SPI is disabled - it is
>> not pinned out on the Development Kit board.
>>
> Okay. Then disable both in the dtsi and enable the one you want to use
> in your board file. The dtsi should only have stuff that is common to
> ALL Socfpgas, so the IPs (if they are the same, which AFAIK they are).
Understood & thanks for explaining. This would be better.

Thanks for reviewing!

Thor
> Regards,
> Steffen
>

Re: [PATCH 1/2] spi: dw: Initialize of_node to discover DT node children

[Thor Thayer]

On 10/08/2014 03:17 AM, Andy Shevchenko wrote:
> On Tue, 2014-10-07 at 14:48 -0500, tthayer@opensource.altera.com wrote:
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> 1. The of_node element must be initialized to enable discovery of node
>> children. The discovery takes place in the of_register_spi_devices() function.
>> 2. Grabbing the bus-num from the device tree.
>> 3. Update the SOCFPGA configuration.
> []
>
>> --- a/drivers/spi/spi-dw.c
>> +++ b/drivers/spi/spi-dw.c
>> @@ -671,6 +671,9 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>>   	master->cleanup = dw_spi_cleanup;
>>   	master->transfer_one_message = dw_spi_transfer_one_message;
>>   	master->max_speed_hz = dws->max_freq;
>> +#if defined(CONFIG_OF)
>> +	master->dev.of_node = dev->of_node;
>> +#endif
> I'm not sure you need this #ifdef surrounded an assignment.
> If I'm wrong then you probably have to use if (IS_ENABLED(CONFIG_OF)).
>
Hi Andy.

Good point. I will remove those.

Thank you for reviewing.