[PATCH 01/17] of: add dma-mask binding

[PATCH 01/17] of: add dma-mask binding

[Wenyou Yang]

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

This will allow each device to specify its dma-mask for this we use the
coherent_dma_mask as pointer. By default the dma-mask will be set to
DMA_BIT_MASK(32).
The microblaze architecture hook is drop

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org
Cc: rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
---
 drivers/of/platform.c |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index e44f8c2..11c765c 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -121,6 +121,21 @@ void of_device_make_bus_id(struct device *dev)
 	dev_set_name(dev, "%s.%d", node->name, magic - 1);
 }
 
+static void of_get_dma_mask(struct device *dev, struct device_node *np)
+{
+	const __be32 *prop;
+	int len;
+
+	prop = of_get_property(np, "dma-mask", &len);
+
+	dev->dma_mask = &dev->coherent_dma_mask;
+
+	if (!prop)
+		dev->coherent_dma_mask = DMA_BIT_MASK(32);
+	else
+		dev->coherent_dma_mask = of_read_number(prop, len / 4);
+}
+
 /**
  * of_device_alloc - Allocate and initialize an of_device
  * @np: device node to assign to device
@@ -161,10 +176,8 @@ struct platform_device *of_device_alloc(struct device_node *np,
 		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
 	}
 
+	of_get_dma_mask(&dev->dev, np);
 	dev->dev.of_node = of_node_get(np);
-#if defined(CONFIG_MICROBLAZE)
-	dev->dev.dma_mask = &dev->archdata.dma_mask;
-#endif
 	dev->dev.parent = parent;
 
 	if (bus_id)
@@ -201,10 +214,6 @@ struct platform_device *of_platform_device_create_pdata(
 	if (!dev)
 		return NULL;
 
-#if defined(CONFIG_MICROBLAZE)
-	dev->archdata.dma_mask = 0xffffffffUL;
-#endif
-	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
 
-- 
1.7.9.5

[PATCH 02/17] of_spi: add generic binding support to specify cs gpio

[Wenyou Yang]

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

This will allow to use gpio for chip select with no modification in the
driver binding

When use the cs-gpios, the gpio number will be passed via the cs_gpio field
and the number of chip select will automatically increased.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org
Cc: rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org
Cc: rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org
Cc: richard.genoud-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
---
 Documentation/devicetree/bindings/spi/spi-bus.txt |    6 +++
 drivers/spi/spi.c                                 |   55 +++++++++++++++++++--
 include/linux/spi/spi.h                           |    3 ++
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index e782add..c253379 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -12,6 +12,7 @@ The SPI master node requires the following properties:
 - #size-cells     - should be zero.
 - compatible      - name of SPI bus controller following generic names
     		recommended practice.
+- cs-gpios	  - (optional) gpios chip select.
 No other properties are required in the SPI bus node.  It is assumed
 that a driver for an SPI bus device will understand that it is an SPI bus.
 However, the binding does not attempt to define the specific method for
@@ -21,6 +22,8 @@ assumption that board specific platform code will be used to manage
 chip selects.  Individual drivers can define additional properties to
 support describing the chip select layout.
 
+If cs-gpios is used the number of chip select will automatically increased.
+
 SPI slave nodes must be children of the SPI master node and can
 contain the following properties.
 - reg             - (required) chip select address of device.
@@ -34,6 +37,9 @@ contain the following properties.
 - spi-cs-high     - (optional) Empty property indicating device requires
     		chip select active high
 
+If a gpio chipselect is used for the SPI slave the gpio number will be passed
+via the controller_data
+
 SPI example for an MPC5200 SPI bus:
 	spi@f00 {
 		#address-cells = <1>;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 84c2861..74e6577 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -30,6 +30,7 @@
 #include <linux/slab.h>
 #include <linux/mod_devicetable.h>
 #include <linux/spi/spi.h>
+#include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 #include <linux/export.h>
 #include <linux/sched.h>
@@ -327,6 +328,7 @@ struct spi_device *spi_alloc_device(struct spi_master *master)
 	spi->dev.parent = &master->dev;
 	spi->dev.bus = &spi_bus_type;
 	spi->dev.release = spidev_release;
+	spi->cs_gpio = -EINVAL;
 	device_initialize(&spi->dev);
 	return spi;
 }
@@ -344,15 +346,16 @@ EXPORT_SYMBOL_GPL(spi_alloc_device);
 int spi_add_device(struct spi_device *spi)
 {
 	static DEFINE_MUTEX(spi_add_lock);
-	struct device *dev = spi->master->dev.parent;
+	struct spi_master *master = spi->master;
+	struct device *dev = master->dev.parent;
 	struct device *d;
 	int status;
 
 	/* Chipselects are numbered 0..max; validate. */
-	if (spi->chip_select >= spi->master->num_chipselect) {
+	if (spi->chip_select >= master->num_chipselect) {
 		dev_err(dev, "cs%d >= max %d\n",
 			spi->chip_select,
-			spi->master->num_chipselect);
+			master->num_chipselect);
 		return -EINVAL;
 	}
 
@@ -376,6 +379,9 @@ int spi_add_device(struct spi_device *spi)
 		goto done;
 	}
 
+	if (master->cs_gpios)
+		spi->cs_gpio = master->cs_gpios[spi->chip_select];
+
 	/* Drivers may modify this initial i/o setup, but will
 	 * normally rely on the device being setup.  Devices
 	 * using SPI_CS_HIGH can't coexist well otherwise...
@@ -946,6 +952,45 @@ struct spi_master *spi_alloc_master(struct device *dev, unsigned size)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_master);
 
+#ifdef CONFIG_OF
+static int of_spi_register_master(struct spi_master *master)
+{
+	int nb, i;
+	int *cs;
+	struct device_node *np = master->dev.of_node;
+
+	if (!np)
+		return 0;
+
+	nb = of_gpio_named_count(np, "cs-gpios");
+
+	if (nb < 1)
+		return 0;
+
+	cs = devm_kzalloc(&master->dev,
+			  sizeof(int) * (master->num_chipselect + nb),
+			  GFP_KERNEL);
+	master->cs_gpios = cs;
+
+	if (!master->cs_gpios)
+		return -ENOMEM;
+
+	memset(cs, -EINVAL, master->num_chipselect);
+	cs += master->num_chipselect;
+	master->num_chipselect += nb;
+
+	for (i = 0; i < nb; i++)
+		cs[i] = of_get_named_gpio(np, "cs-gpios", i);
+
+	return 0;
+}
+#else
+static int of_spi_register_master(struct spi_master *master)
+{
+	return 0;
+}
+#endif
+
 /**
  * spi_register_master - register SPI master controller
  * @master: initialized master, originally from spi_alloc_master()
@@ -977,6 +1022,10 @@ int spi_register_master(struct spi_master *master)
 	if (!dev)
 		return -ENODEV;
 
+	status = of_spi_register_master(master);
+	if (status)
+		return status;
+
 	/* even if it's just one always-selected device, there must
 	 * be at least one chipselect
 	 */
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index fa702ae..f629189 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -90,6 +90,7 @@ struct spi_device {
 	void			*controller_state;
 	void			*controller_data;
 	char			modalias[SPI_NAME_SIZE];
+	int			cs_gpio;	/* chip select gpio */
 
 	/*
 	 * likely need more hooks for more protocol options affecting how
@@ -362,6 +363,8 @@ struct spi_master {
 	int (*transfer_one_message)(struct spi_master *master,
 				    struct spi_message *mesg);
 	int (*unprepare_transfer_hardware)(struct spi_master *master);
+	/* gpio chip select */
+	int			*cs_gpios;
 };
 
 static inline void *spi_master_get_devdata(struct spi_master *master)
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar

Re: [PATCH 02/17] of_spi: add generic binding support to specify cs gpio

[Grant Likely]

On Fri, 22 Mar 2013 17:16:54 +0800, Wenyou Yang <wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> wrote:
> From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> 
> This will allow to use gpio for chip select with no modification in the
> driver binding
> 
> When use the cs-gpios, the gpio number will be passed via the cs_gpio field
> and the number of chip select will automatically increased.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

This patch has been in mainline since Dec 13

g.

RE: [PATCH 02/17] of_spi: add generic binding support to specify cs gpio

[Yang, Wenyou]

Hi, Grant,

> -----Original Message-----
> From: Grant Likely [mailto:glikely@secretlab.ca] On Behalf Of Grant Likely
> Sent: 2013年4月15日 20:56
> To: Yang, Wenyou; Gao, Andy
> Cc: Jean-Christophe PLAGNIOL-VILLARD; devicetree-discuss@lists.ozlabs.org;
> spi-devel-general@lists.sourceforge.net; rob.herring@calxeda.com;
> rob@landley.net; richard.genoud@gmail.com
> Subject: Re: [PATCH 02/17] of_spi: add generic binding support to specify cs
> gpio
> 
> On Fri, 22 Mar 2013 17:16:54 +0800, Wenyou Yang <wenyou.yang@atmel.com>
> wrote:
> > From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >
> > This will allow to use gpio for chip select with no modification in the
> > driver binding
> >
> > When use the cs-gpios, the gpio number will be passed via the cs_gpio field
> > and the number of chip select will automatically increased.
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> This patch has been in mainline since Dec 13
Yes, it has been applied.
This patch is one of the 1st version of Atmel spi patch series. Since it is applied, it is dropped in the next version.
The latest version is v8.
https://patchwork.kernel.org/patch/2384681/
thanks.
> 
> g.

Best Regards,
Wenyou Yang

------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

[PATCH 01/17] of: add dma-mask binding

[Wenyou Yang]

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

This will allow each device to specify its dma-mask for this we use the
coherent_dma_mask as pointer. By default the dma-mask will be set to
DMA_BIT_MASK(32).
The microblaze architecture hook is drop

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: grant.likely@secretlab.ca
Cc: rob.herring@calxeda.com
Cc: devicetree-discuss@lists.ozlabs.org
---
 drivers/of/platform.c |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b80891b..31ed405 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -130,6 +130,21 @@ void of_device_make_bus_id(struct device *dev)
 	dev_set_name(dev, "%s.%d", node->name, magic - 1);
 }
 
+static void of_get_dma_mask(struct device *dev, struct device_node *np)
+{
+	const __be32 *prop;
+	int len;
+
+	prop = of_get_property(np, "dma-mask", &len);
+
+	dev->dma_mask = &dev->coherent_dma_mask;
+
+	if (!prop)
+		dev->coherent_dma_mask = DMA_BIT_MASK(32);
+	else
+		dev->coherent_dma_mask = of_read_number(prop, len / 4);
+}
+
 /**
  * of_device_alloc - Allocate and initialize an of_device
  * @np: device node to assign to device
@@ -171,10 +186,8 @@ struct platform_device *of_device_alloc(struct device_node *np,
 		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
 	}
 
+	of_get_dma_mask(&dev->dev, np);
 	dev->dev.of_node = of_node_get(np);
-#if defined(CONFIG_MICROBLAZE)
-	dev->dev.dma_mask = &dev->archdata.dma_mask;
-#endif
 	dev->dev.parent = parent;
 
 	if (bus_id)
@@ -211,10 +224,6 @@ struct platform_device *of_platform_device_create_pdata(
 	if (!dev)
 		return NULL;
 
-#if defined(CONFIG_MICROBLAZE)
-	dev->archdata.dma_mask = 0xffffffffUL;
-#endif
-	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
 
-- 
1.7.9.5

Re: [PATCH 01/17] of: add dma-mask binding

[Rob Herring]

On 11/12/2012 02:52 AM, Wenyou Yang wrote:
> From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> 
> This will allow each device to specify its dma-mask for this we use the
> coherent_dma_mask as pointer. By default the dma-mask will be set to
> DMA_BIT_MASK(32).

Do you really have a use case this is not DMA_BIT_MASK(32)?

> The microblaze architecture hook is drop
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org
> Cc: rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> ---
>  drivers/of/platform.c |   23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index b80891b..31ed405 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -130,6 +130,21 @@ void of_device_make_bus_id(struct device *dev)
>  	dev_set_name(dev, "%s.%d", node->name, magic - 1);
>  }
>  
> +static void of_get_dma_mask(struct device *dev, struct device_node *np)
> +{
> +	const __be32 *prop;
> +	int len;
> +
> +	prop = of_get_property(np, "dma-mask", &len);

dma-ranges may work for this purpose.

> +
> +	dev->dma_mask = &dev->coherent_dma_mask;

I don't really know, but I suspect this is wrong.

> +
> +	if (!prop)
> +		dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +	else
> +		dev->coherent_dma_mask = of_read_number(prop, len / 4);
> +}
> +
>  /**
>   * of_device_alloc - Allocate and initialize an of_device
>   * @np: device node to assign to device
> @@ -171,10 +186,8 @@ struct platform_device *of_device_alloc(struct device_node *np,
>  		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
>  	}
>  
> +	of_get_dma_mask(&dev->dev, np);
>  	dev->dev.of_node = of_node_get(np);
> -#if defined(CONFIG_MICROBLAZE)
> -	dev->dev.dma_mask = &dev->archdata.dma_mask;
> -#endif
>  	dev->dev.parent = parent;
>  
>  	if (bus_id)
> @@ -211,10 +224,6 @@ struct platform_device *of_platform_device_create_pdata(
>  	if (!dev)
>  		return NULL;
>  
> -#if defined(CONFIG_MICROBLAZE)
> -	dev->archdata.dma_mask = 0xffffffffUL;
> -#endif
> -	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>  	dev->dev.bus = &platform_bus_type;
>  	dev->dev.platform_data = platform_data;
>  
> 

Re: [PATCH 01/17] of: add dma-mask binding

[Jean-Christophe PLAGNIOL-VILLARD]

On 21:55 Tue 13 Nov     , Rob Herring wrote:
> On 11/12/2012 02:52 AM, Wenyou Yang wrote:
> > From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> > 
> > This will allow each device to specify its dma-mask for this we use the
> > coherent_dma_mask as pointer. By default the dma-mask will be set to
> > DMA_BIT_MASK(32).
> 
> Do you really have a use case this is not DMA_BIT_MASK(32)?
yes as exmample on 64bit platfrom it will be 64 on x86 it's also 24, on power
pc 40
> 
> > The microblaze architecture hook is drop
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> > Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org
> > Cc: rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org
> > Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > ---
> >  drivers/of/platform.c |   23 ++++++++++++++++-------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index b80891b..31ed405 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -130,6 +130,21 @@ void of_device_make_bus_id(struct device *dev)
> >  	dev_set_name(dev, "%s.%d", node->name, magic - 1);
> >  }
> >  
> > +static void of_get_dma_mask(struct device *dev, struct device_node *np)
> > +{
> > +	const __be32 *prop;
> > +	int len;
> > +
> > +	prop = of_get_property(np, "dma-mask", &len);
> 
> dma-ranges may work for this purpose.
> 
> > +
> > +	dev->dma_mask = &dev->coherent_dma_mask;
> 
> I don't really know, but I suspect this is wrong.
no this is correct was suggest by Russell
we already do so on other ARM or SH as example

the dma expect a pointer for dma_mask but the value is the same as coherent
> 
> > +
> > +	if (!prop)
> > +		dev->coherent_dma_mask = DMA_BIT_MASK(32);
> > +	else
> > +		dev->coherent_dma_mask = of_read_number(prop, len / 4);
> > +}
> > +
> >  /**
> >   * of_device_alloc - Allocate and initialize an of_device
> >   * @np: device node to assign to device
> > @@ -171,10 +186,8 @@ struct platform_device *of_device_alloc(struct device_node *np,
> >  		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
> >  	}
> >  
> > +	of_get_dma_mask(&dev->dev, np);
> >  	dev->dev.of_node = of_node_get(np);
> > -#if defined(CONFIG_MICROBLAZE)
> > -	dev->dev.dma_mask = &dev->archdata.dma_mask;
> > -#endif
> >  	dev->dev.parent = parent;
> >  
> >  	if (bus_id)
> > @@ -211,10 +224,6 @@ struct platform_device *of_platform_device_create_pdata(
> >  	if (!dev)
> >  		return NULL;
> >  
> > -#if defined(CONFIG_MICROBLAZE)
> > -	dev->archdata.dma_mask = 0xffffffffUL;
> > -#endif
> > -	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> >  	dev->dev.bus = &platform_bus_type;
> >  	dev->dev.platform_data = platform_data;
> >  
> > 

Re: [PATCH 01/17] of: add dma-mask binding

[Rob Herring]

On 11/14/2012 12:00 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 21:55 Tue 13 Nov     , Rob Herring wrote:
>> On 11/12/2012 02:52 AM, Wenyou Yang wrote:
>>> From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>>
>>> This will allow each device to specify its dma-mask for this we use the
>>> coherent_dma_mask as pointer. By default the dma-mask will be set to
>>> DMA_BIT_MASK(32).
>>
>> Do you really have a use case this is not DMA_BIT_MASK(32)?
> yes as exmample on 64bit platfrom it will be 64 on x86 it's also 24, on power
> pc 40
>>
>>> The microblaze architecture hook is drop
>>>
>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>> Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org
>>> Cc: rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org
>>> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>>> ---
>>>  drivers/of/platform.c |   23 ++++++++++++++++-------
>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>>> index b80891b..31ed405 100644
>>> --- a/drivers/of/platform.c
>>> +++ b/drivers/of/platform.c
>>> @@ -130,6 +130,21 @@ void of_device_make_bus_id(struct device *dev)
>>>  	dev_set_name(dev, "%s.%d", node->name, magic - 1);
>>>  }
>>>  
>>> +static void of_get_dma_mask(struct device *dev, struct device_node *np)
>>> +{
>>> +	const __be32 *prop;
>>> +	int len;
>>> +
>>> +	prop = of_get_property(np, "dma-mask", &len);
>>
>> dma-ranges may work for this purpose.
>>
>>> +
>>> +	dev->dma_mask = &dev->coherent_dma_mask;
>>
>> I don't really know, but I suspect this is wrong.
> no this is correct was suggest by Russell
> we already do so on other ARM or SH as example
> 
> the dma expect a pointer for dma_mask but the value is the same as coherent

Okay. Then perhaps this part should be a separate patch as that is
useful on its own for 32-bit machines with no DMA address restrictions
(most modern ARM h/w).

My comment on using dma-ranges still stands though. The form is
<parent-address child-address size>. Normally, parent and child would be
the same. I think the mask would be "parent address + size - 1". The
simple case is <0 0 0> for all of 32-bit memory. I'm using 0 for full
32-bit size here as #size-cells is typically 1 and I can't imagine that
0 size would be useful.

Rob

>>
>>> +
>>> +	if (!prop)
>>> +		dev->coherent_dma_mask = DMA_BIT_MASK(32);
>>> +	else
>>> +		dev->coherent_dma_mask = of_read_number(prop, len / 4);
>>> +}
>>> +
>>>  /**
>>>   * of_device_alloc - Allocate and initialize an of_device
>>>   * @np: device node to assign to device
>>> @@ -171,10 +186,8 @@ struct platform_device *of_device_alloc(struct device_node *np,
>>>  		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
>>>  	}
>>>  
>>> +	of_get_dma_mask(&dev->dev, np);
>>>  	dev->dev.of_node = of_node_get(np);
>>> -#if defined(CONFIG_MICROBLAZE)
>>> -	dev->dev.dma_mask = &dev->archdata.dma_mask;
>>> -#endif
>>>  	dev->dev.parent = parent;
>>>  
>>>  	if (bus_id)
>>> @@ -211,10 +224,6 @@ struct platform_device *of_platform_device_create_pdata(
>>>  	if (!dev)
>>>  		return NULL;
>>>  
>>> -#if defined(CONFIG_MICROBLAZE)
>>> -	dev->archdata.dma_mask = 0xffffffffUL;
>>> -#endif
>>> -	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>>>  	dev->dev.bus = &platform_bus_type;
>>>  	dev->dev.platform_data = platform_data;
>>>  
>>>

Re: [PATCH 01/17] of: add dma-mask binding

[Russell King - ARM Linux]

On Wed, Nov 14, 2012 at 02:56:14PM -0600, Rob Herring wrote:
> On 11/14/2012 12:00 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 21:55 Tue 13 Nov     , Rob Herring wrote:
> >> On 11/12/2012 02:52 AM, Wenyou Yang wrote:
> >>> +
> >>> +	dev->dma_mask = &dev->coherent_dma_mask;
> >>
> >> I don't really know, but I suspect this is wrong.
> > no this is correct was suggest by Russell
> > we already do so on other ARM or SH as example
> > 
> > the dma expect a pointer for dma_mask but the value is the same as coherent
> 
> Okay. Then perhaps this part should be a separate patch as that is
> useful on its own for 32-bit machines with no DMA address restrictions
> (most modern ARM h/w).

I'm not sure that I did make the exact suggestion being alluded to above
(I think I may have made the suggestion that dev->dma_mask should be
pointed at a dma_mask, and it might be a good idea that it should be
part of struct device.)

I've always shy'd away from making it the same thing as the coherent
DMA mask, because there are drivers around which modify the value
pointed to by dev->dma_mask via standard DMA API calls.  (Eg, when they
know that the device is only N-bit capable.)

With that set to the same as dev->coherent_dma_mask, it ends up
modifying the coherent DMA mask too which may or may not be entirely
a good thing; I suspect ultimately that depends on the driver.

My thoughts on this though is that the whole thing is a mess.  I'm
not sure why we ended up with a separate coherent_dma_mask from the
main dma_mask, and why we ended up with dma_mask being a pointer to
something allocated elsewhere... it all seems like it's unnecessarily
complicated and was designed to cause confusion...