[RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Anton Vorontsov]

mmc_spi already tested to work. When it will hit mainline
the only change that would be needed is replacing "spidev"
by "mmc_spi", and adding trivial platform data to mmc_spi
driver.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc832x_rdb.dts     |   33 ++++++++++++++++++++-
 arch/powerpc/platforms/83xx/mpc832x_rdb.c |   46 +++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
index 7c4beff..5dcbdd3 100644
--- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
@@ -183,6 +183,21 @@
 					1  c  1  0  1  0 	/* TX_EN */
 					1  d  2  0  1  0>;      /* CRS */
 			};
+			spi1pio:spi_pin@01 {
+				pio-map = <
+			/* port  pin  dir  open_drain  assignment  has_irq    */
+					3  0  3  0  1  0	/* SPI1 MOSI, I/O */
+					3  1  3  0  1  0	/* SPI1 MISO, I/O */
+					3  2  3  0  1  0	/* SPI1 CLK,  I/O */
+					3  3  2  0  1  0>;	/* SPI1 SEL,  I   */
+			};
+			mmc1pio:mmc_pin@01 {
+				pio-map = <
+			/* port  pin  dir  open_drain  assignment  has_irq    */
+					3  d  1  0  0  0	/* !SD_CS     */
+					3  e  2  0  0  0	/* SD_INSERT  */
+					3  f  2  0  0  0>;	/* SD_PROTECT */
+			};
 		};
 	};
 
@@ -207,20 +222,36 @@
 
 		spi@4c0 {
 			device_type = "spi";
+			device-id = <1>;
 			compatible = "fsl_spi";
 			reg = <4c0 40>;
 			interrupts = <2>;
 			interrupt-parent = <&qeic>;
-			mode = "cpu";
+			mode = "qe";
+			sysclk = <5f5e100>; /* 100000000 Hz */
+			max-chipselect = <1>;
+			pio-handle = <&spi1pio>;
+
+			mmc@01 {
+				device_type = "mmc";
+				compatible = "mmc-spi";
+				device-id = <1>;
+				max-speed-hz = <bebc20>; /* 12500000 Hz */
+				chip-select = <0>;
+				pio-handle = <&mmc1pio>;
+			};
 		};
 
 		spi@500 {
 			device_type = "spi";
+			device-id = <2>;
 			compatible = "fsl_spi";
 			reg = <500 40>;
 			interrupts = <1>;
 			interrupt-parent = <&qeic>;
 			mode = "cpu";
+			sysclk = <5f5e100>; /* 100000000 Hz */
+			max_chipselect = <1>;
 		};
 
 		ucc@3000 {
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index b2b28a4..c5463c7 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/pci.h>
+#include <linux/spi/spi.h>
 
 #include <asm/of_platform.h>
 #include <asm/time.h>
@@ -24,6 +25,7 @@
 #include <asm/qe_ic.h>
 
 #include "mpc83xx.h"
+#include "../../sysdev/fsl_soc.h"
 
 #undef DEBUG
 #ifdef DEBUG
@@ -32,6 +34,43 @@
 #define DBG(fmt...)
 #endif
 
+extern int par_io_data_set(u8 port, u8 pin, u8 val);
+
+static void mpc83xx_spi_activate_cs(u8 cs, u8 polarity)
+{
+	pr_debug("%s %d %d\n", __func__, cs, polarity);
+	par_io_data_set(3, 13, polarity);
+}
+
+static void mpc83xx_spi_deactivate_cs(u8 cs, u8 polarity)
+{
+	pr_debug("%s %d %d\n", __func__, cs, polarity);
+	par_io_data_set(3, 13, !polarity);
+}
+
+static int __init mpc83xx_spi_devices_init(void)
+{
+	struct spi_board_info spi_bi = {
+		.bus_num = 1,
+		/*
+		 * XXX: This is spidev (spi in userspace) stub, should
+		 * be replaced by "mmc_spi" when mmc_spi will hit mainline.
+		 */
+		.modalias = "spidev",
+	};
+	struct device_node *np = NULL;
+
+	np = of_find_compatible_node(np, "mmc", "mmc-spi");
+	if (!np)
+		return 0;
+
+	spi_bi.max_speed_hz = *((u32 *)of_get_property(np, "max-speed-hz", NULL));
+	spi_bi.chip_select = *((u32 *)of_get_property(np, "chip-select", NULL));
+
+	return spi_register_board_info(&spi_bi, 1);
+}
+device_initcall(mpc83xx_spi_devices_init);
+
 /* ************************************************************************
  *
  * Setup the architecture
@@ -62,8 +101,15 @@ static void __init mpc832x_rdb_setup_arch(void)
 
 		for (np = NULL; (np = of_find_node_by_name(np, "ucc")) != NULL;)
 			par_io_of_config(np);
+		for (np = NULL; (np = of_find_node_by_name(np, "spi")) != NULL;)
+			par_io_of_config(np);
+		for (np = NULL; (np = of_find_node_by_name(np, "mmc")) != NULL;)
+			par_io_of_config(np);
 	}
 #endif				/* CONFIG_QUICC_ENGINE */
+
+	fsl_spi_activate_cs = mpc83xx_spi_activate_cs;
+	fsl_spi_deactivate_cs = mpc83xx_spi_deactivate_cs;
 }
 
 static struct of_device_id mpc832x_ids[] = {
-- 
1.5.0.6

RE: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Joakim Tjernlund]

 

> -----Original Message-----
> From: 
> linuxppc-dev-bounces+joakim.tjernlund=transmode.se@ozlabs.org 
> [mailto:linuxppc-dev-bounces+joakim.tjernlund=transmode.se@ozl
> abs.org] On Behalf Of Anton Vorontsov
> Sent: den 26 juli 2007 15:58
> To: linuxppc-dev@ozlabs.org
> Subject: [RFC][PATCH] MPC832x_RDB: update dts to use spi, 
> register mmc_spi stub
> 
> mmc_spi already tested to work. When it will hit mainline
> the only change that would be needed is replacing "spidev"
> by "mmc_spi", and adding trivial platform data to mmc_spi
> driver.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  arch/powerpc/boot/dts/mpc832x_rdb.dts     |   33 
> ++++++++++++++++++++-
>  arch/powerpc/platforms/83xx/mpc832x_rdb.c |   46 
> +++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts 
> b/arch/powerpc/boot/dts/mpc832x_rdb.dts
> index 7c4beff..5dcbdd3 100644
> --- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
> +++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
> @@ -183,6 +183,21 @@
>  					1  c  1  0  1  0 	
> /* TX_EN */
>  					1  d  2  0  1  0>;      
> /* CRS */
>  			};
> +			spi1pio:spi_pin@01 {
> +				pio-map = <
> +			/* port  pin  dir  open_drain  
> assignment  has_irq    */
> +					3  0  3  0  1  0	
> /* SPI1 MOSI, I/O */
> +					3  1  3  0  1  0	
> /* SPI1 MISO, I/O */
> +					3  2  3  0  1  0	
> /* SPI1 CLK,  I/O */
> +					3  3  2  0  1  0>;	
> /* SPI1 SEL,  I   */
> +			};
> +			mmc1pio:mmc_pin@01 {
> +				pio-map = <
> +			/* port  pin  dir  open_drain  
> assignment  has_irq    */
> +					3  d  1  0  0  0	
> /* !SD_CS     */
> +					3  e  2  0  0  0	
> /* SD_INSERT  */
> +					3  f  2  0  0  0>;	
> /* SD_PROTECT */
> +			};
>  		};
>  	};
>  
> @@ -207,20 +222,36 @@
>  
>  		spi@4c0 {
>  			device_type = "spi";
> +			device-id = <1>;
>  			compatible = "fsl_spi";
>  			reg = <4c0 40>;
>  			interrupts = <2>;
>  			interrupt-parent = <&qeic>;
> -			mode = "cpu";
> +			mode = "qe";

The driver is still using "cpu" mode. It just happens that the cpu mode for QE
enabled CPUs isn't 100% compatible with the defacto cpu mode used by other CPU:s.
So mode should probably be "cpu_qe" or, even better, if the drver could
autodetect that it is running on a QE enabled CPU and adjust accordinly.
Maybe the Freescale guys have some ideas how to do that.

 Jocke

PS.
   Will be on vacation for a week as of tmw, won't have access
   during this time.

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Anton Vorontsov]

On Thu, Jul 26, 2007 at 05:36:06PM +0200, Joakim Tjernlund wrote:
[...]
> >  		spi@4c0 {
> >  			device_type = "spi";
> > +			device-id = <1>;
> >  			compatible = "fsl_spi";
> >  			reg = <4c0 40>;
> >  			interrupts = <2>;
> >  			interrupt-parent = <&qeic>;
> > -			mode = "cpu";
> > +			mode = "qe";
> 
> The driver is still using "cpu" mode. It just happens that the cpu mode for QE
> enabled CPUs isn't 100% compatible with the defacto cpu mode used by other CPU:s.

Yup, indeed.

> So mode should probably be "cpu_qe" or, even better, if the drver could

That's easy to change, thanks.

> autodetect that it is running on a QE enabled CPU and adjust accordinly.
> Maybe the Freescale guys have some ideas how to do that.

Ok. Well, for now could we use machine_is() for kind of autodetect?
Or it's better leave it as is, and just rename mode to cpu_qe?

>  Jocke
> 
> PS.
>    Will be on vacation for a week as of tmw, won't have access
>    during this time.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

RE: [RFC][PATCH] MPC832x_RDB: update dts to use spi, registermmc_spi stub

[Joakim Tjernlund]

 

> -----Original Message-----
> From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com] 
> Sent: den 26 juli 2007 17:48
> To: Joakim Tjernlund
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, 
> registermmc_spi stub
> 
> On Thu, Jul 26, 2007 at 05:36:06PM +0200, Joakim Tjernlund wrote:
> [...]
> > >  		spi@4c0 {
> > >  			device_type = "spi";
> > > +			device-id = <1>;
> > >  			compatible = "fsl_spi";
> > >  			reg = <4c0 40>;
> > >  			interrupts = <2>;
> > >  			interrupt-parent = <&qeic>;
> > > -			mode = "cpu";
> > > +			mode = "qe";
> > 
> > The driver is still using "cpu" mode. It just happens that 
> the cpu mode for QE
> > enabled CPUs isn't 100% compatible with the defacto cpu 
> mode used by other CPU:s.
> 
> Yup, indeed.
> 
> > So mode should probably be "cpu_qe" or, even better, if the 
> drver could
> 
> That's easy to change, thanks.
> 
> > autodetect that it is running on a QE enabled CPU and 
> adjust accordinly.
> > Maybe the Freescale guys have some ideas how to do that.
> 
> Ok. Well, for now could we use machine_is() for kind of autodetect?
> Or it's better leave it as is, and just rename mode to cpu_qe?

Unless the DTS guys thinks otherwise, just rename to "cpu_qe".
"qe" would then be reserved for true QE mode.

 Jocke

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, registermmc_spi stub

[Kumar Gala]

On Jul 26, 2007, at 2:40 PM, Joakim Tjernlund wrote:

>
>
>> -----Original Message-----
>> From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]
>> Sent: den 26 juli 2007 17:48
>> To: Joakim Tjernlund
>> Cc: linuxppc-dev@ozlabs.org
>> Subject: Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi,
>> registermmc_spi stub
>>
>> On Thu, Jul 26, 2007 at 05:36:06PM +0200, Joakim Tjernlund wrote:
>> [...]
>>>>  		spi@4c0 {
>>>>  			device_type = "spi";
>>>> +			device-id = <1>;
>>>>  			compatible = "fsl_spi";
>>>>  			reg = <4c0 40>;
>>>>  			interrupts = <2>;
>>>>  			interrupt-parent = <&qeic>;
>>>> -			mode = "cpu";
>>>> +			mode = "qe";
>>>
>>> The driver is still using "cpu" mode. It just happens that
>> the cpu mode for QE
>>> enabled CPUs isn't 100% compatible with the defacto cpu
>> mode used by other CPU:s.
>>
>> Yup, indeed.
>>
>>> So mode should probably be "cpu_qe" or, even better, if the
>> drver could
>>
>> That's easy to change, thanks.
>>
>>> autodetect that it is running on a QE enabled CPU and
>> adjust accordinly.
>>> Maybe the Freescale guys have some ideas how to do that.
>>
>> Ok. Well, for now could we use machine_is() for kind of autodetect?
>> Or it's better leave it as is, and just rename mode to cpu_qe?
>
> Unless the DTS guys thinks otherwise, just rename to "cpu_qe".
> "qe" would then be reserved for true QE mode.

Agreed, we need to update booting-without-of.txt.

- k

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Kumar Gala]

On Jul 26, 2007, at 8:57 AM, Anton Vorontsov wrote:

> mmc_spi already tested to work. When it will hit mainline
> the only change that would be needed is replacing "spidev"
> by "mmc_spi", and adding trivial platform data to mmc_spi
> driver.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  arch/powerpc/boot/dts/mpc832x_rdb.dts     |   33 ++++++++++++++++++ 
> ++-
>  arch/powerpc/platforms/83xx/mpc832x_rdb.c |   46 ++++++++++++++++++ 
> +++++++++++
>  2 files changed, 78 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/ 
> boot/dts/mpc832x_rdb.dts
> index 7c4beff..5dcbdd3 100644
> --- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
> +++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
> @@ -183,6 +183,21 @@
>  					1  c  1  0  1  0 	/* TX_EN */
>  					1  d  2  0  1  0>;      /* CRS */
>  			};
> +			spi1pio:spi_pin@01 {
> +				pio-map = <
> +			/* port  pin  dir  open_drain  assignment  has_irq    */
> +					3  0  3  0  1  0	/* SPI1 MOSI, I/O */
> +					3  1  3  0  1  0	/* SPI1 MISO, I/O */
> +					3  2  3  0  1  0	/* SPI1 CLK,  I/O */
> +					3  3  2  0  1  0>;	/* SPI1 SEL,  I   */
> +			};
> +			mmc1pio:mmc_pin@01 {
> +				pio-map = <
> +			/* port  pin  dir  open_drain  assignment  has_irq    */
> +					3  d  1  0  0  0	/* !SD_CS     */
> +					3  e  2  0  0  0	/* SD_INSERT  */
> +					3  f  2  0  0  0>;	/* SD_PROTECT */
> +			};
>  		};
>  	};
>
> @@ -207,20 +222,36 @@
>
>  		spi@4c0 {
>  			device_type = "spi";
> +			device-id = <1>;

Can we just use the reg value for bus_num in the kernel.

>  			compatible = "fsl_spi";
>  			reg = <4c0 40>;
>  			interrupts = <2>;
>  			interrupt-parent = <&qeic>;
> -			mode = "cpu";
> +			mode = "qe";
> +			sysclk = <5f5e100>; /* 100000000 Hz */

we don't need this in the spi node, its just the system clock  
frequency which we can get other ways.

> +			max-chipselect = <1>;

I'm not sure how I feel about this in here, I'm thinking it should go.

> +			pio-handle = <&spi1pio>;
> +
> +			mmc@01 {
> +				device_type = "mmc";
> +				compatible = "mmc-spi";
> +				device-id = <1>;
> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
> +				chip-select = <0>;
> +				pio-handle = <&mmc1pio>;
> +			};

we should do this in board code and not the device tree.

>  		};
>
>  		spi@500 {
>  			device_type = "spi";
> +			device-id = <2>;
>  			compatible = "fsl_spi";
>  			reg = <500 40>;
>  			interrupts = <1>;
>  			interrupt-parent = <&qeic>;
>  			mode = "cpu";
> +			sysclk = <5f5e100>; /* 100000000 Hz */
> +			max_chipselect = <1>;
>  		};
>
>  		ucc@3000 {
> diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/ 
> powerpc/platforms/83xx/mpc832x_rdb.c
> index b2b28a4..c5463c7 100644
> --- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> +++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> @@ -15,6 +15,7 @@
>   */
>
>  #include <linux/pci.h>
> +#include <linux/spi/spi.h>
>
>  #include <asm/of_platform.h>
>  #include <asm/time.h>
> @@ -24,6 +25,7 @@
>  #include <asm/qe_ic.h>
>
>  #include "mpc83xx.h"
> +#include "../../sysdev/fsl_soc.h"
>
>  #undef DEBUG
>  #ifdef DEBUG
> @@ -32,6 +34,43 @@
>  #define DBG(fmt...)
>  #endif
>
> +extern int par_io_data_set(u8 port, u8 pin, u8 val);
> +
> +static void mpc83xx_spi_activate_cs(u8 cs, u8 polarity)
> +{
> +	pr_debug("%s %d %d\n", __func__, cs, polarity);
> +	par_io_data_set(3, 13, polarity);
> +}
> +
> +static void mpc83xx_spi_deactivate_cs(u8 cs, u8 polarity)
> +{
> +	pr_debug("%s %d %d\n", __func__, cs, polarity);
> +	par_io_data_set(3, 13, !polarity);
> +}
> +
> +static int __init mpc83xx_spi_devices_init(void)
> +{
> +	struct spi_board_info spi_bi = {
> +		.bus_num = 1,
> +		/*
> +		 * XXX: This is spidev (spi in userspace) stub, should
> +		 * be replaced by "mmc_spi" when mmc_spi will hit mainline.
> +		 */
> +		.modalias = "spidev",
> +	};
> +	struct device_node *np = NULL;
> +
> +	np = of_find_compatible_node(np, "mmc", "mmc-spi");
> +	if (!np)
> +		return 0;
> +
> +	spi_bi.max_speed_hz = *((u32 *)of_get_property(np, "max-speed- 
> hz", NULL));
> +	spi_bi.chip_select = *((u32 *)of_get_property(np, "chip-select",  
> NULL));
> +
> +	return spi_register_board_info(&spi_bi, 1);
> +}
> +device_initcall(mpc83xx_spi_devices_init);
> +
>  /*  
> ********************************************************************** 
> **
>   *
>   * Setup the architecture
> @@ -62,8 +101,15 @@ static void __init mpc832x_rdb_setup_arch(void)
>
>  		for (np = NULL; (np = of_find_node_by_name(np, "ucc")) != NULL;)
>  			par_io_of_config(np);
> +		for (np = NULL; (np = of_find_node_by_name(np, "spi")) != NULL;)
> +			par_io_of_config(np);
> +		for (np = NULL; (np = of_find_node_by_name(np, "mmc")) != NULL;)
> +			par_io_of_config(np);
>  	}
>  #endif				/* CONFIG_QUICC_ENGINE */
> +
> +	fsl_spi_activate_cs = mpc83xx_spi_activate_cs;
> +	fsl_spi_deactivate_cs = mpc83xx_spi_deactivate_cs;
>  }
>
>  static struct of_device_id mpc832x_ids[] = {
> -- 
> 1.5.0.6
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Anton Vorontsov]

On Fri, Jul 27, 2007 at 03:14:06AM -0500, Kumar Gala wrote:
>
> On Jul 26, 2007, at 8:57 AM, Anton Vorontsov wrote:
[...]
>>
>> @@ -207,20 +222,36 @@
>>
>>  		spi@4c0 {
>>  			device_type = "spi";
>> +			device-id = <1>;
>
> Can we just use the reg value for bus_num in the kernel.

Sure, technically nothing prevents this. But, QE specs names
SPIs by these ids. Plus, from the kernel side spi name will be
not pretty, it will be spi1216.1. Reg value making little sense
to the userspace (or kernel-side SPI layer). Still want get
rid of device-id?

>>  			compatible = "fsl_spi";
>>  			reg = <4c0 40>;
>>  			interrupts = <2>;
>>  			interrupt-parent = <&qeic>;
>> -			mode = "cpu";
>> +			mode = "qe";
>> +			sysclk = <5f5e100>; /* 100000000 Hz */
>
> we don't need this in the spi node, its just the system clock frequency 
> which we can get other ways.

Will try, I guess "bus-frequency" property of soc8323 is what I need.

>> +			max-chipselect = <1>;
>
> I'm not sure how I feel about this in here, I'm thinking it should go.

It's board-specific, i.e. how much chips connected to this SPI bus.
SPI layer needs this. Otherwise I have to pass it from board file,
but isn't it fits nicely in the DT?

>> +			pio-handle = <&spi1pio>;
>> +
>> +			mmc@01 {
>> +				device_type = "mmc";
>> +				compatible = "mmc-spi";
>> +				device-id = <1>;
>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
>> +				chip-select = <0>;
>> +				pio-handle = <&mmc1pio>;
>> +			};
>
> we should do this in board code and not the device tree.

Well, I've done this initially. But Vitaly hinted that this could
be done in the DT instead, which made sense to me - mmc is the child
device of SPI bus. Why do you think it shouldn't be in the DT? I'm
not arguing, just want understand this.

Thanks!

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Kumar Gala]

On Jul 27, 2007, at 6:45 AM, Anton Vorontsov wrote:

> On Fri, Jul 27, 2007 at 03:14:06AM -0500, Kumar Gala wrote:
>>
>> On Jul 26, 2007, at 8:57 AM, Anton Vorontsov wrote:
> [...]
>>>
>>> @@ -207,20 +222,36 @@
>>>
>>>  		spi@4c0 {
>>>  			device_type = "spi";
>>> +			device-id = <1>;
>>
>> Can we just use the reg value for bus_num in the kernel.
>
> Sure, technically nothing prevents this. But, QE specs names
> SPIs by these ids. Plus, from the kernel side spi name will be
> not pretty, it will be spi1216.1. Reg value making little sense
> to the userspace (or kernel-side SPI layer). Still want get
> rid of device-id?
>
>>>  			compatible = "fsl_spi";
>>>  			reg = <4c0 40>;
>>>  			interrupts = <2>;
>>>  			interrupt-parent = <&qeic>;
>>> -			mode = "cpu";
>>> +			mode = "qe";
>>> +			sysclk = <5f5e100>; /* 100000000 Hz */
>>
>> we don't need this in the spi node, its just the system clock  
>> frequency
>> which we can get other ways.
>
> Will try, I guess "bus-frequency" property of soc8323 is what I need.
>
>>> +			max-chipselect = <1>;
>>
>> I'm not sure how I feel about this in here, I'm thinking it should  
>> go.
>
> It's board-specific, i.e. how much chips connected to this SPI bus.
> SPI layer needs this. Otherwise I have to pass it from board file,
> but isn't it fits nicely in the DT?
>
>>> +			pio-handle = <&spi1pio>;
>>> +
>>> +			mmc@01 {
>>> +				device_type = "mmc";
>>> +				compatible = "mmc-spi";
>>> +				device-id = <1>;
>>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
>>> +				chip-select = <0>;
>>> +				pio-handle = <&mmc1pio>;
>>> +			};
>>
>> we should do this in board code and not the device tree.
>
> Well, I've done this initially. But Vitaly hinted that this could
> be done in the DT instead, which made sense to me - mmc is the child
> device of SPI bus. Why do you think it shouldn't be in the DT? I'm
> not arguing, just want understand this.

I understand. Look at the dts/device tree from a non-Linux  
perspective.  From that point of view its not always clear how one  
would interpret some of the fields or if they are necessary.  I get  
very concerned when we start trying to describe any devices connect  
to a bus when the bus doesn't have any specific for things that are  
just software convention for a given OS.

The "id" concept and number is from Linux and another OS may have  
come up w/some other way of describe this.

- k

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Scott Wood]

On Fri, Jul 27, 2007 at 03:45:59PM +0400, Anton Vorontsov wrote:
> Sure, technically nothing prevents this. But, QE specs names
> SPIs by these ids. Plus, from the kernel side spi name will be
> not pretty, it will be spi1216.1. Reg value making little sense
> to the userspace (or kernel-side SPI layer). Still want get
> rid of device-id?

Naming devices for human consumption is a more general problem than
device-id addresses; I'd like to see a standard "label" property added to
the device tree spec that software could use to present a human-friendly
label that corresponds to markings on the case, position on the board,
etc.

-Scott

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, registermmc_spi stub

[Segher Boessenkool]

> Unless the DTS guys thinks otherwise, just rename to "cpu_qe".
> "qe" would then be reserved for true QE mode.

If I understand it correctly, this is a property of how the
driver works, not how the hardware works / is connected; so
there should be _no_ such property in the device tree.

If I understand that wrong :-), just keep mode = "cpu", but
add an extra (empty) property representing that you need some
quirky mode special thing.

Either way, don't use underscores.


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

> Naming devices for human consumption is a more general problem than
> device-id addresses; I'd like to see a standard "label" property added 
> to
> the device tree spec that software could use to present a 
> human-friendly
> label that corresponds to markings on the case, position on the board,
> etc.

For some buses, there is a "slot-names" property; some (non-core)
bindings seem to define a "location" property.

For "random" human-readable labelling, i.e. not corresponding to
physical markings on the hardware, I recommend you look for a
matching entry in /aliases.  It won't ever be _exactly_ what you
want though, the Linux device namespace is separate from the
device tree.


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

>>>  		spi@4c0 {
>>>  			device_type = "spi";
>>> +			device-id = <1>;
>>
>> Can we just use the reg value for bus_num in the kernel.
>
> Sure, technically nothing prevents this. But, QE specs names
> SPIs by these ids.

As a minimum the property name should start with "fsl," then.

> Plus, from the kernel side spi name will be
> not pretty, it will be spi1216.1.

What, the kernel cannot implement a counter itself?

>>> +			max-chipselect = <1>;
>>
>> I'm not sure how I feel about this in here, I'm thinking it should go.
>
> It's board-specific, i.e. how much chips connected to this SPI bus.

It is misnamed then.  It should be automatically derived from
the child nodes, though.

>>> +			mmc@01 {

@01 should be @1.  Except that it is wrong, since there is
no "reg" property.

>>> +				device_type = "mmc";

No device_type please.

>>> +				compatible = "mmc-spi";

Needs to be more specific.

>>> +				device-id = <1>;

Get rid of this.

>>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */

Just max-speed.

>>> +				chip-select = <0>;

This should be named "reg".  And the parent needs #address-cells
and #size-cells properties.

>>> +				pio-handle = <&mmc1pio>;

What is this for?

>> we should do this in board code and not the device tree.
>
> Well, I've done this initially. But Vitaly hinted that this could
> be done in the DT instead, which made sense to me - mmc is the child
> device of SPI bus. Why do you think it shouldn't be in the DT? I'm
> not arguing, just want understand this.

The hardware should be described in the device tree.  This isn't
the same as simply copying all your Linux code into it ;-)


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

> +			spi1pio:spi_pin@01 {

There should be whitespace after the label.  @01 should be
spelled @1.  Except there is no "reg" property.  What is this
stuff, anyway?

> +				pio-map = <
> +			/* port  pin  dir  open_drain  assignment  has_irq    */
> +					3  0  3  0  1  0	/* SPI1 MOSI, I/O */
> +					3  1  3  0  1  0	/* SPI1 MISO, I/O */
> +					3  2  3  0  1  0	/* SPI1 CLK,  I/O */
> +					3  3  2  0  1  0>;	/* SPI1 SEL,  I   */
> +			};


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Anton Vorontsov]

On Wed, Aug 01, 2007 at 12:10:28AM +0200, Segher Boessenkool wrote:
>> +			spi1pio:spi_pin@01 {
>
> There should be whitespace after the label.  @01 should be
> spelled @1.  Except there is no "reg" property.

Hm. I've just tried to keep original style in this particular dts.
Want to ack patch below?

What is prefered style of <&label> vs. < &label > usage, btw?

arch/powerpc/boot/dts$ grep "<&" -r . | wc -l
327
arch/powerpc/boot/dts$ grep "< &" -r . | wc -l
92

I can only guess - the first?

> What is this
> stuff, anyway?

Which one? pio-map for spi? This is GPIO pins configuration, to use
dedicated functions (SPI) for these pins, otherwise SPI will not work.

>> +				pio-map = <
>> +			/* port  pin  dir  open_drain  assignment  has_irq    */
>> +					3  0  3  0  1  0	/* SPI1 MOSI, I/O */
>> +					3  1  3  0  1  0	/* SPI1 MISO, I/O */
>> +					3  2  3  0  1  0	/* SPI1 CLK,  I/O */
>> +					3  3  2  0  1  0>;	/* SPI1 SEL,  I   */
>> +			};


p.s. mpc8272ads.dts is broken wrt spaces/tabs, very.

- - - -
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Subject: [POWERPC] boot/dts/*: cleanup labels usage

There should be whitespace after labels.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc8272ads.dts  |    6 +++---
 arch/powerpc/boot/dts/mpc832x_rdb.dts |   12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8272ads.dts b/arch/powerpc/boot/dts/mpc8272ads.dts
index 1934b80..97c5d4e 100644
--- a/arch/powerpc/boot/dts/mpc8272ads.dts
+++ b/arch/powerpc/boot/dts/mpc8272ads.dts
@@ -66,14 +66,14 @@
                        reg = <0 0>;
                        #address-cells = <1>;
                        #size-cells = <0>;
-			phy0:ethernet-phy@0 {
+			phy0: ethernet-phy@0 {
 				interrupt-parent = <&Cpm_pic>;
                                interrupts = <17 4>;
                                reg = <0>;
                                bitbang = [ 12 12 13 02 02 01 ];
                                device_type = "ethernet-phy";
                        };
-			phy1:ethernet-phy@1 {
+			phy1: ethernet-phy@1 {
 				interrupt-parent = <&Cpm_pic>;
                                interrupts = <17 4>;
                                bitbang = [ 12 12 13 02 02 01 ];
@@ -153,7 +153,7 @@
                        };
 
                };
-			cpm_pic:interrupt-controller@10c00 {
+			cpm_pic: interrupt-controller@10c00 {
                        #address-cells = <0>;
                        #interrupt-cells = <2>;
                        interrupt-controller;
diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
index 7c4beff..e9c332f 100644
--- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
@@ -127,7 +127,7 @@
 			device_type = "pci";
 		};
 
-		pic:pic@700 {
+		pic: pic@700 {
 			interrupt-controller;
 			#address-cells = <0>;
 			#interrupt-cells = <2>;
@@ -141,7 +141,7 @@
 			device_type = "par_io";
 			num-ports = <7>;
 
-			ucc2pio:ucc_pin@02 {
+			ucc2pio: ucc_pin@02 {
 				pio-map = <
 			/* port  pin  dir  open_drain  assignment  has_irq */
 					3  4  3  0  2  0 	/* MDIO */
@@ -163,7 +163,7 @@
 					0 1e  1  0  1  0 	/* TX_EN */
 					0 1f  2  0  1  0>;      /* CRS */
 			};
-			ucc3pio:ucc_pin@03 {
+			ucc3pio: ucc_pin@03 {
 				pio-map = <
 			/* port  pin  dir  open_drain  assignment  has_irq */
 					0  d  2  0  1  0 	/* RX_CLK (CLK9) */
@@ -272,13 +272,13 @@
 			device_type = "mdio";
 			compatible = "ucc_geth_phy";
 
-			phy00:ethernet-phy@00 {
+			phy00: ethernet-phy@00 {
 				interrupt-parent = <&pic>;
 				interrupts = <0>;
 				reg = <0>;
 				device_type = "ethernet-phy";
 			};
-			phy04:ethernet-phy@04 {
+			phy04: ethernet-phy@04 {
 				interrupt-parent = <&pic>;
 				interrupts = <0>;
 				reg = <4>;
@@ -286,7 +286,7 @@
 			};
 		};
 
-		qeic:qeic@80 {
+		qeic: qeic@80 {
 			interrupt-controller;
 			device_type = "qeic";
 			#address-cells = <0>;
-- 
1.5.0.6

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Anton Vorontsov]

On Wed, Aug 01, 2007 at 12:06:46AM +0200, Segher Boessenkool wrote:
>>>>  		spi@4c0 {
>>>>  			device_type = "spi";
>>>> +			device-id = <1>;
>>>
>>> Can we just use the reg value for bus_num in the kernel.
>>
>> Sure, technically nothing prevents this. But, QE specs names
>> SPIs by these ids.
>
> As a minimum the property name should start with "fsl," then.

fsl,device-id = <1>;, correct?

>> Plus, from the kernel side spi name will be
>> not pretty, it will be spi1216.1.
>
> What, the kernel cannot implement a counter itself?

Just counter is especially meaningless and confusing. It will
work in that particular case, though. But then SPI bus number will
depend on definition order in the dts file. This isn't how SPI
bus numbers should be assigned. SPI bus numbers taken from specs,
this is how people know which SPI is which.

>>>> +			max-chipselect = <1>;
>>>
>>> I'm not sure how I feel about this in here, I'm thinking it should go.
>>
>> It's board-specific, i.e. how much chips connected to this SPI bus.
>
> It is misnamed then.  It should be automatically derived from
> the child nodes, though.
>
>>>> +			mmc@01 {
>
> @01 should be @1.  Except that it is wrong, since there is
> no "reg" property.
>
>>>> +				device_type = "mmc";
>
> No device_type please.
>
>>>> +				compatible = "mmc-spi";
>
> Needs to be more specific.

Um.. for example? I can't imagine anything specific for this. ;-)

>>>> +				device-id = <1>;
>
> Get rid of this.
>
>>>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
>
> Just max-speed.
>
>>>> +				chip-select = <0>;
>
> This should be named "reg".  And the parent needs #address-cells
> and #size-cells properties.
>
>>>> +				pio-handle = <&mmc1pio>;
>
> What is this for?

To set up output function of GPIO pin for MMC chip select.

And well, I've just looked into par_io_of_config(), and I've found
that pio-handle is mandatory (obviously), and thus let's back to:

>>> we should do this in board code and not the device tree.
>>
>> Well, I've done this initially. But Vitaly hinted that this could
>> be done in the DT instead, which made sense to me - mmc is the child
>> device of SPI bus. Why do you think it shouldn't be in the DT? I'm
>> not arguing, just want understand this.
>
> The hardware should be described in the device tree.  This isn't
> the same as simply copying all your Linux code into it ;-)

Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
and chip select pin. To set up this pin, I need mmc node, which means
that I can't completely move mmc definitions to the board file, as
suggested by Kumar Gala.

Advices?

> Segher

Thanks!

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Scott Wood]

Segher Boessenkool wrote:
> For some buses, there is a "slot-names" property; some (non-core)
> bindings seem to define a "location" property.
> 
> For "random" human-readable labelling, i.e. not corresponding to
> physical markings on the hardware, I recommend you look for a
> matching entry in /aliases.

Aliases could work, but are awkward to use for the purposes I'm thinking 
of (giving the OS a name to present to the user in association with a 
device).  They're more suited to interactive OF use where the device 
tree is being directly referenced by the user.

Plus, you're then restricted to valid property names for the alias, 
whereas with a label property you could use any string, including spaces 
and such.

>  It won't ever be _exactly_ what you
> want though, the Linux device namespace is separate from the
> device tree.

That's Linux's choice.  Nothing stops it from showing device tree labels 
  to the user in various situations -- what got me thinking about this 
was that apparently ALSA lets the driver pass an arbitrary string to 
identify the device, and it seemed that such a device-tree-derived label 
would be the most useful to the user.  To use aliases for that, it'd 
have to get the full path to the audio node, compare it to each alias, 
and hope it finds one and only one, and that that alias was intended to 
be a user label and not something else.

-Scott

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Kim Phillips]

On Wed, 1 Aug 2007 00:06:46 +0200
Segher Boessenkool <segher@kernel.crashing.org> wrote:

> >>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
> 
> Just max-speed.

Segher, how is this different from:

http://ozlabs.org/pipermail/linuxppc-dev/2007-April/034557.html

?

Kim

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

>>> +			spi1pio:spi_pin@01 {
>>
>> There should be whitespace after the label.  @01 should be
>> spelled @1.  Except there is no "reg" property.
>
> Hm. I've just tried to keep original style in this particular dts.
> Want to ack patch below?

Not unless you get rid of the extra zeroes, too :-)

> What is prefered style of <&label> vs. < &label > usage, btw?
>
> arch/powerpc/boot/dts$ grep "<&" -r . | wc -l
> 327
> arch/powerpc/boot/dts$ grep "< &" -r . | wc -l
> 92
>
> I can only guess - the first?

I think it looks neater, yes.  I wouldn't worry about it
too much though.

>> What is this
>> stuff, anyway?
>
> Which one? pio-map for spi? This is GPIO pins configuration, to use
> dedicated functions (SPI) for these pins, otherwise SPI will not work.

The weird pseudo-nodes, yes.  This really should be handled in
platform code, not in the device tree; if there is a need to
describe what GPIOs are used for what, that should be handled
differently.

> p.s. mpc8272ads.dts is broken wrt spaces/tabs, very.

Care to do a cleanup patch?  Good for your karma ;-)


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

>>>>>  		spi@4c0 {
>>>>>  			device_type = "spi";
>>>>> +			device-id = <1>;
>>>>
>>>> Can we just use the reg value for bus_num in the kernel.
>>>
>>> Sure, technically nothing prevents this. But, QE specs names
>>> SPIs by these ids.
>>
>> As a minimum the property name should start with "fsl," then.
>
> fsl,device-id = <1>;, correct?

Fine with me.  Someone more familiar with the FSL SoCs might
have a different opinion about polluting their namespace though.

>>> Plus, from the kernel side spi name will be
>>> not pretty, it will be spi1216.1.
>>
>> What, the kernel cannot implement a counter itself?
>
> Just counter is especially meaningless and confusing. It will
> work in that particular case, though. But then SPI bus number will
> depend on definition order in the dts file. This isn't how SPI
> bus numbers should be assigned. SPI bus numbers taken from specs,
> this is how people know which SPI is which.

Right, so the kernel platform code should number the SPI busses
based on their position in the device tree, etc.; that doesn't
mean you should put a Linux-specific "device name" property in
there.

>>>>> +				compatible = "mmc-spi";
>>
>> Needs to be more specific.
>
> Um.. for example? I can't imagine anything specific for this. ;-)

It should include a vendor name, a device name, and/or a board
name.  Something that uniquely defines the hardware programming
model for the device.

>>>>> +				pio-handle = <&mmc1pio>;
>>
>> What is this for?
>
> To set up output function of GPIO pin for MMC chip select.
>
> And well, I've just looked into par_io_of_config(), and I've found
> that pio-handle is mandatory (obviously), and thus let's back to:
>
>>>> we should do this in board code and not the device tree.
>>>
>>> Well, I've done this initially. But Vitaly hinted that this could
>>> be done in the DT instead, which made sense to me - mmc is the child
>>> device of SPI bus. Why do you think it shouldn't be in the DT? I'm
>>> not arguing, just want understand this.
>>
>> The hardware should be described in the device tree.  This isn't
>> the same as simply copying all your Linux code into it ;-)
>
> Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
> and chip select pin. To set up this pin, I need mmc node, which means
> that I can't completely move mmc definitions to the board file, as
> suggested by Kumar Gala.
>
> Advices?

You need to declare in the SPI node which GPIOs it uses for
what.  You shouldn't have the actual values to put into the
GPIO registers in the device tree; the kernel driver can
figure it out.

Hope this helps,


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

>> For some buses, there is a "slot-names" property; some (non-core)
>> bindings seem to define a "location" property.
>> For "random" human-readable labelling, i.e. not corresponding to
>> physical markings on the hardware, I recommend you look for a
>> matching entry in /aliases.
>
> Aliases could work, but are awkward to use for the purposes I'm 
> thinking of (giving the OS a name to present to the user in 
> association with a device).

Sure; it's just one thing a platform driver could use.

> Plus, you're then restricted to valid property names for the alias, 
> whereas with a label property you could use any string, including 
> spaces and such.

Spaces in a device name are a bad idea anyway ;-)

>>  It won't ever be _exactly_ what you
>> want though, the Linux device namespace is separate from the
>> device tree.
>
> That's Linux's choice.  Nothing stops it from showing device tree 
> labels  to the user in various situations -- what got me thinking 
> about this was that apparently ALSA lets the driver pass an arbitrary 
> string to identify the device, and it seemed that such a 
> device-tree-derived label would be the most useful to the user.

Yeah.  The platform code has the final responsibility for those
names, it can use whatever mechanism is appropriate for that
platform.

> To use aliases for that, it'd have to get the full path to the audio 
> node, compare it to each alias, and hope it finds one and only one,

This would of course be split off into a nice prom.c utility
function, so it isn't much code, just a function call.

> and that that alias was intended to be a user label and not something 
> else.

Hey at least they're all printable :-)


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

>>>>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
>>
>> Just max-speed.
>
> Segher, how is this different from:
>
> http://ozlabs.org/pipermail/linuxppc-dev/2007-April/034557.html

Not sure what you mean.  I'm just saying that "speed-hz" is a
terrible name, I'm not saying that "max-speed" is perfect at all.


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Scott Wood]

Segher Boessenkool wrote:
>>p.s. mpc8272ads.dts is broken wrt spaces/tabs, very.
> 
> 
> Care to do a cleanup patch?  Good for your karma ;-)

I've got one in my 82xx patchset (a respin of which should be coming soon).

There's a lot more broken in there than just spaces/tabs, BTW. :-)

-Scott

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Kim Phillips]

On Mon, 6 Aug 2007 20:25:13 +0200
Segher Boessenkool <segher@kernel.crashing.org> wrote:

> >>>>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
> >>
> >> Just max-speed.
> >
> > Segher, how is this different from:
> >
> > http://ozlabs.org/pipermail/linuxppc-dev/2007-April/034557.html
> 
> Not sure what you mean.  I'm just saying that "speed-hz" is a
> terrible name, I'm not saying that "max-speed" is perfect at all.

yet you suggest a /more/ generic name, contrary to your prior comments.

My interpretation of your recent comments is that 'max-speed' is now a
valid property name for devices such as ucc_geth.  Do I have that right?

Kim

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

>>>>>>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */
>>>>
>>>> Just max-speed.
>>>
>>> Segher, how is this different from:
>>>
>>> http://ozlabs.org/pipermail/linuxppc-dev/2007-April/034557.html
>>
>> Not sure what you mean.  I'm just saying that "speed-hz" is a
>> terrible name, I'm not saying that "max-speed" is perfect at all.
>
> yet you suggest a /more/ generic name, contrary to your prior comments.

Uh, you mean "hz" doesn't mean "Hertz"?  What a great name,
then</sarcasm>.

> My interpretation of your recent comments is that 'max-speed' is now a
> valid property name for devices such as ucc_geth.  Do I have that 
> right?

It is (and always was) a _valid_ name.  Whether it is a _good_
name depends on the context; if there is only one speed it can
be (reasonably) referring to, it is okay; if not (or even if so),
you're better off being a bit more verbose in your property names.
No need to go over the top though, it's all a tradeoff.


Segher

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Anton Vorontsov]

On Mon, Aug 06, 2007 at 08:18:47PM +0200, Segher Boessenkool wrote:
[...]
>>> The hardware should be described in the device tree.  This isn't
>>> the same as simply copying all your Linux code into it ;-)
>>
>> Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
>> and chip select pin. To set up this pin, I need mmc node, which means
>> that I can't completely move mmc definitions to the board file, as
>> suggested by Kumar Gala.
>>
>> Advices?
>
> You need to declare in the SPI node which GPIOs it uses for
> what.  You shouldn't have the actual values to put into the
> GPIO registers in the device tree; the kernel driver can
> figure it out.

Well, how SPI differs from UCC in that regard? This is how mpc832x_mds.dts,
mpc832x_rdb.dts, mpc836x_mds.dts, ... doing things already for UCC pins.
And then what pio-map exists for?.. In my understanding pio-map tries to
describe hardware (GPIO) wiring, exactly how SPI (and UCC) nodes trying
to use it.

Heh.. anyway, it's really hard to find proper logic around device tree,
do's and don'ts, so I'll just follow your suggestions in hope that I'll
get it as time goes by. ;-)

> Hope this helps,

Thanks!

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

[Segher Boessenkool]

>>>> The hardware should be described in the device tree.  This isn't
>>>> the same as simply copying all your Linux code into it ;-)
>>>
>>> Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
>>> and chip select pin. To set up this pin, I need mmc node, which means
>>> that I can't completely move mmc definitions to the board file, as
>>> suggested by Kumar Gala.
>>>
>>> Advices?
>>
>> You need to declare in the SPI node which GPIOs it uses for
>> what.  You shouldn't have the actual values to put into the
>> GPIO registers in the device tree; the kernel driver can
>> figure it out.
>
> Well, how SPI differs from UCC in that regard?

Not at all, I think.  You just copied a bad example that
shouldn't exist like that at all :-(

> This is how mpc832x_mds.dts,
> mpc832x_rdb.dts, mpc836x_mds.dts, ... doing things already for UCC 
> pins.
> And then what pio-map exists for?.. In my understanding pio-map tries 
> to
> describe hardware (GPIO) wiring, exactly how SPI (and UCC) nodes trying
> to use it.

Yeah.  It however contains lots more information that really
should be implicit in the driver using it (like, GPIO output
lines that are marked "no interrupt" -- what a surprise, duh).

As far as I can see the devices that need some GPIOs should
just say which GPIOs (on what GPIO controller) they use, and
that's about it, the kernel device drivers can handle the
rest (since they need to know lots more implicit information
_anyway_, there is no point in describing more details in the
device tree, esp. since many of those details describe only
how a device is used, not what it _is_).

Anyway, not your fault, I'd prefer not to see the madness
spread though :-)

> Heh.. anyway, it's really hard to find proper logic around device tree,
> do's and don'ts, so I'll just follow your suggestions in hope that I'll
> get it as time goes by. ;-)

:-)


Segher