Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Marek Vasut
Dear Dong Aisheng,

 From: Dong Aisheng dong.aish...@linaro.org
 
 Signed-off-by: Dong Aisheng dong.aish...@linaro.org
 
 ---
 The patch is still using a private way for dma part binding
 since the common dma binding is still under discussion.
 http://www.spinics.net/lists/linux-omap/msg65528.html
 
 Will update to use common dma binding when it hits mainline.
 ---
  .../devicetree/bindings/mmc/fsl-mxs-mmc.txt|   23 ++
  drivers/mmc/host/mxs-mmc.c |   82
 +++- 2 files changed, 102 insertions(+), 3 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
 b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt new file mode
 100644
 index 000..adc1142
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
 @@ -0,0 +1,23 @@
 +* FREESCALE MXS MMC peripheral
 +
 +Required properties:
 +- compatible : Should be fsl,chip-mmc
 +- reg : Should contain registers location and length
 +- interrupts : Should contain interrupt.
 +  The format is irq_err irq_dma.
 +- dma_channel: Should contain the dma channel it uses
 +
 +Optional properties:
 +- wp-gpios : Specify GPIOs for write protection
 +- slot-4bit: Specify 4 bit mode support
 +- slot-8bit: Specify 8 bit and 4 bit mode support
 +
 +Examples:
 +mmc1: ssp@8001 {
 + compatible = fsl,imx28-mmc;
 + reg = 0x8001 2000;
 + /* irq_err irq_dma */
 + interrupts = 96 82;
 + dma_channel = 0;
 + slot-8bit;
 +};
 diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
 index 382c835..6cf2d17 100644
 --- a/drivers/mmc/host/mxs-mmc.c
 +++ b/drivers/mmc/host/mxs-mmc.c
 @@ -38,6 +38,10 @@
  #include linux/gpio.h
  #include linux/regulator/consumer.h
  #include linux/module.h
 +#include linux/of.h
 +#include linux/of_device.h
 +#include linux/of_gpio.h
 +#include linux/slab.h
 
  #include mach/mxs.h
  #include mach/common.h
 @@ -673,17 +677,79 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan,
 void *param) return true;
  }
 
 +#ifdef CONFIG_OF
 +static struct resource * __devinit mxs_mmc_get_of_dmares(
 + struct platform_device *pdev)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct resource *dmares;
 + int ret;
 +
 + if (!np)
 + return NULL;
 +
 + dmares = kzalloc(sizeof(*dmares), GFP_KERNEL);
 + dmares-flags = IORESOURCE_DMA;
 + ret = of_property_read_u32(np, dma_channel, dmares-start);
 + if (ret) {
 + dev_err(pdev-dev, unable to get dmares from dt\n);
 + return NULL;
 + }
 + dmares-end = dmares-start;
 +
 + return dmares;
 +}
 +
 +static int __devinit mxs_mmc_get_of_property(struct platform_device *pdev,
 + struct mxs_mmc_platform_data **ppdata)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct mxs_mmc_platform_data *pdata = *ppdata;
 +
 + if (!np)
 + return -ENODEV;
 +
 + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
 +
 + if (of_get_property(np, slot-8bit, NULL))
 + pdata-flags |= SLOTF_8_BIT_CAPABLE;
 +
 + if (of_get_property(np, slot-4bit, NULL))
 + pdata-flags |= SLOTF_4_BIT_CAPABLE;
 +
 + pdata-wp_gpio = of_get_named_gpio(np, wp-gpios, 0);
 +
 + dev_dbg(pdev-dev, wp-gpios %d flags %d\n, pdata-wp_gpio,
 + pdata-flags);
 +
 + return 0;
 +}
 +#else
 +static struct resource * __devinit mxs_mmc_get_of_dmares(
 + struct platform_device *pdev)
 +{
 + return NULL;
 +}
 +static inline int mxs_mmc_get_of_property(struct platform_device *pdev,
 + struct mxs_mmc_platform_data *pdata)
 +{
 + return -ENODEV;
 +}
 +#endif
 +
  static int mxs_mmc_probe(struct platform_device *pdev)
  {
   struct mxs_mmc_host *host;
   struct mmc_host *mmc;
   struct resource *iores, *dmares, *r;
 - struct mxs_mmc_platform_data *pdata;
 + struct mxs_mmc_platform_data *pdata = NULL;
   int ret = 0, irq_err, irq_dma;
   dma_cap_mask_t mask;
 
   iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 - dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 + dmares = mxs_mmc_get_of_dmares(pdev);
 + if (dmares == NULL)
 + dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
   irq_err = platform_get_irq(pdev, 0);
   irq_dma = platform_get_irq(pdev, 1);
   if (!iores || !dmares || irq_err  0 || irq_dma  0)
 @@ -740,7 +806,9 @@ static int mxs_mmc_probe(struct platform_device *pdev)
   mmc-caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
   MMC_CAP_SDIO_IRQ | MMC_CAP_NEEDS_POLL;
 
 - pdata = mmc_dev(host-mmc)-platform_data;
 + mxs_mmc_get_of_property(pdev, pdata);
 + if (pdata == NULL)
 + pdata = mmc_dev(host-mmc)-platform_data;
   if (pdata) {
   if (pdata-flags  SLOTF_8_BIT_CAPABLE)
   mmc-caps |= 

Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Dong Aisheng
On Wed, Mar 14, 2012 at 01:42:38AM +0800, Grant Likely wrote:
 On Tue, 13 Mar 2012 16:47:05 +0800, Dong Aisheng b29...@freescale.com wrote:
  From: Dong Aisheng dong.aish...@linaro.org
  
  Signed-off-by: Dong Aisheng dong.aish...@linaro.org
  
  ---
  The patch is still using a private way for dma part binding
  since the common dma binding is still under discussion.
  http://www.spinics.net/lists/linux-omap/msg65528.html
  
  Will update to use common dma binding when it hits mainline.
  ---
   .../devicetree/bindings/mmc/fsl-mxs-mmc.txt|   23 ++
   drivers/mmc/host/mxs-mmc.c |   82 
  +++-
   2 files changed, 102 insertions(+), 3 deletions(-)
  
  diff --git a/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt 
  b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
  new file mode 100644
  index 000..adc1142
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
  @@ -0,0 +1,23 @@
  +* FREESCALE MXS MMC peripheral
  +
  +Required properties:
  +- compatible : Should be fsl,chip-mmc
  +- reg : Should contain registers location and length
  +- interrupts : Should contain interrupt.
  +  The format is irq_err irq_dma.
  +- dma_channel: Should contain the dma channel it uses
 
 Don't use '_' in property names.
 
Yes, my mistake.

 The is a generic dma binding being drafted that uses a phandle to the dma
 controller and the ability to encode channel numbers.  You may want to take
 a look at it.
 
I will look at it.
Thanks for the reminder.

...
  +   dmares-flags = IORESOURCE_DMA;
  +   ret = of_property_read_u32(np, dma_channel, dmares-start);
  +   if (ret) {
  +   dev_err(pdev-dev, unable to get dmares from dt\n);
  +   return NULL;
  +   }
  +   dmares-end = dmares-start;
  +
  +   return dmares;
  +}
  +
  +static int __devinit mxs_mmc_get_of_property(struct platform_device *pdev,
  +   struct mxs_mmc_platform_data **ppdata)
  +{
  +   struct device_node *np = pdev-dev.of_node;
  +   struct mxs_mmc_platform_data *pdata = *ppdata;
  +
  +   if (!np)
  +   return -ENODEV;
  +
  +   pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
 
 Ditto
 
Got it.

 Fix up those comments and you can add my:
 
 Acked-by: Grant Likely grant.lik...@secretlab.ca
 
Thanks.

Regards
Dong Aisheng

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Dong Aisheng
On Wed, Mar 14, 2012 at 01:58:25PM +0800, Marek Vasut wrote:
 Dear Dong Aisheng,
 
  
  Signed-off-by: Dong Aisheng dong.aish...@linaro.org


  +static const struct of_device_id mxs_mmc_dt_ids[] = {
  +   { .compatible = fsl,imx23-mmc, .data = NULL, },
  +   { .compatible = fsl,imx28-mmc, .data = NULL, },
 
 Do you really need two distinct ones here?
 
Hmm, my original purpose is to put soc difference data in .data
to remove cpu_is_* function calls in the driver later.
Do you think if any issue?

Regards
Dong Aisheng

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Marek Vasut
Dear Dong Aisheng,

 On Wed, Mar 14, 2012 at 01:58:25PM +0800, Marek Vasut wrote:
  Dear Dong Aisheng,
  
   Signed-off-by: Dong Aisheng dong.aish...@linaro.org
 
 
 
   +static const struct of_device_id mxs_mmc_dt_ids[] = {
   + { .compatible = fsl,imx23-mmc, .data = NULL, },
   + { .compatible = fsl,imx28-mmc, .data = NULL, },
  
  Do you really need two distinct ones here?
 
 Hmm, my original purpose is to put soc difference data in .data
 to remove cpu_is_* function calls in the driver later.
 Do you think if any issue?
 

Well, what's the difference between the interfaces on mx233 and mx28? Is it 
something that can't be encoded otherwise? I think they're not so different.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread s.ha...@pengutronix.de
On Wed, Mar 14, 2012 at 08:09:22AM +0100, Marek Vasut wrote:
 Dear Dong Aisheng,
 
  On Wed, Mar 14, 2012 at 01:58:25PM +0800, Marek Vasut wrote:
   Dear Dong Aisheng,
   
Signed-off-by: Dong Aisheng dong.aish...@linaro.org
  
  
  
+static const struct of_device_id mxs_mmc_dt_ids[] = {
+   { .compatible = fsl,imx23-mmc, .data = NULL, },
+   { .compatible = fsl,imx28-mmc, .data = NULL, },
   
   Do you really need two distinct ones here?
  
  Hmm, my original purpose is to put soc difference data in .data
  to remove cpu_is_* function calls in the driver later.
  Do you think if any issue?
  
 
 Well, what's the difference between the interfaces on mx233 and mx28? Is it 
 something that can't be encoded otherwise? I think they're not so different.

They differ in the register layout. Matching two different compatible
strings is the right way here I think.

Sascha

-- 
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Dong Aisheng
On Wed, Mar 14, 2012 at 08:09:22AM +0100, Marek Vasut wrote:
 Dear Dong Aisheng,
 
  On Wed, Mar 14, 2012 at 01:58:25PM +0800, Marek Vasut wrote:
   Dear Dong Aisheng,
   
Signed-off-by: Dong Aisheng dong.aish...@linaro.org
  
  
  
+static const struct of_device_id mxs_mmc_dt_ids[] = {
+   { .compatible = fsl,imx23-mmc, .data = NULL, },
+   { .compatible = fsl,imx28-mmc, .data = NULL, },
   
   Do you really need two distinct ones here?
  
  Hmm, my original purpose is to put soc difference data in .data
  to remove cpu_is_* function calls in the driver later.
  Do you think if any issue?
  
 
 Well, what's the difference between the interfaces on mx233 and mx28? Is it 
 something that can't be encoded otherwise? I think they're not so different.
 
Not much difference except the one register offset and ip version.
See:
#define SSP_VERSION_LATEST  4
#define ssp_is_old()(host-version  SSP_VERSION_LATEST)
..
#define HW_SSP_VERSION (cpu_is_mx23() ? 0x110 : 0x130)
The ip version can be handled in driver, but for offset...
it depends on cpu_is_* macro.
Putting the HW_SSP_VERSION offset difference in .data can eliminate the need
of cpu_is_*.

Despite of that, since they're two devices,
i guess it's ok to put two compatible string there, right?
Or you thought just put one as below?
{ .compatible = fsl,mxs-mmc, .data = NULL, },

Regards
Dong Aisheng

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Jean-Christophe PLAGNIOL-VILLARD
On 16:47 Tue 13 Mar , Dong Aisheng wrote:
 From: Dong Aisheng dong.aish...@linaro.org
 
 Signed-off-by: Dong Aisheng dong.aish...@linaro.org
 
 ---
 The patch is still using a private way for dma part binding
 since the common dma binding is still under discussion.
 http://www.spinics.net/lists/linux-omap/msg65528.html
 
 Will update to use common dma binding when it hits mainline.
 ---
  .../devicetree/bindings/mmc/fsl-mxs-mmc.txt|   23 ++
  drivers/mmc/host/mxs-mmc.c |   82 
 +++-
  2 files changed, 102 insertions(+), 3 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt 
 b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
 new file mode 100644
 index 000..adc1142
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
 @@ -0,0 +1,23 @@
 +* FREESCALE MXS MMC peripheral
 +
 +Required properties:
 +- compatible : Should be fsl,chip-mmc
 +- reg : Should contain registers location and length
 +- interrupts : Should contain interrupt.
 +  The format is irq_err irq_dma.
 +- dma_channel: Should contain the dma channel it uses
 +
 +Optional properties:
 +- wp-gpios : Specify GPIOs for write protection
 +- slot-4bit: Specify 4 bit mode support
 +- slot-8bit: Specify 8 bit and 4 bit mode support
 +
 +Examples:
 +mmc1: ssp@8001 {
 + compatible = fsl,imx28-mmc;
 + reg = 0x8001 2000;
 + /* irq_err irq_dma */
 + interrupts = 96 82;
 + dma_channel = 0;
 + slot-8bit;
 +};
 diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
 index 382c835..6cf2d17 100644
 --- a/drivers/mmc/host/mxs-mmc.c
 +++ b/drivers/mmc/host/mxs-mmc.c
 @@ -38,6 +38,10 @@
  #include linux/gpio.h
  #include linux/regulator/consumer.h
  #include linux/module.h
 +#include linux/of.h
 +#include linux/of_device.h
 +#include linux/of_gpio.h
 +#include linux/slab.h
  
  #include mach/mxs.h
  #include mach/common.h
 @@ -673,17 +677,79 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan, 
 void *param)
   return true;
  }
  
 +#ifdef CONFIG_OF
 +static struct resource * __devinit mxs_mmc_get_of_dmares(
 + struct platform_device *pdev)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct resource *dmares;
 + int ret;
 +
 + if (!np)
 + return NULL;
 +
 + dmares = kzalloc(sizeof(*dmares), GFP_KERNEL);
 + dmares-flags = IORESOURCE_DMA;
 + ret = of_property_read_u32(np, dma_channel, dmares-start);
 + if (ret) {
 + dev_err(pdev-dev, unable to get dmares from dt\n);
 + return NULL;
 + }
 + dmares-end = dmares-start;
 +
 + return dmares;
 +}
 +
 +static int __devinit mxs_mmc_get_of_property(struct platform_device *pdev,
 + struct mxs_mmc_platform_data **ppdata)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct mxs_mmc_platform_data *pdata = *ppdata;
 +
 + if (!np)
 + return -ENODEV;
 +
 + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
 +
 + if (of_get_property(np, slot-8bit, NULL))
 + pdata-flags |= SLOTF_8_BIT_CAPABLE;
 +
 + if (of_get_property(np, slot-4bit, NULL))
 + pdata-flags |= SLOTF_4_BIT_CAPABLE;
it will conflit if both binding are set use a number instead

Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Dong Aisheng
On Wed, Mar 14, 2012 at 03:23:43PM +0800, Jean-Christophe PLAGNIOL-VILLARD 
wrote:
 On 16:47 Tue 13 Mar , Dong Aisheng wrote:
  From: Dong Aisheng dong.aish...@linaro.org
  
  Signed-off-by: Dong Aisheng dong.aish...@linaro.org
  
  ---
  The patch is still using a private way for dma part binding
  since the common dma binding is still under discussion.
  http://www.spinics.net/lists/linux-omap/msg65528.html
  
  Will update to use common dma binding when it hits mainline.
  ---
   .../devicetree/bindings/mmc/fsl-mxs-mmc.txt|   23 ++
   drivers/mmc/host/mxs-mmc.c |   82 
  +++-
   2 files changed, 102 insertions(+), 3 deletions(-)
  
  diff --git a/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt 
  b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
  new file mode 100644
  index 000..adc1142
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
  @@ -0,0 +1,23 @@
  +* FREESCALE MXS MMC peripheral
  +
  +Required properties:
  +- compatible : Should be fsl,chip-mmc
  +- reg : Should contain registers location and length
  +- interrupts : Should contain interrupt.
  +  The format is irq_err irq_dma.
  +- dma_channel: Should contain the dma channel it uses
  +
  +Optional properties:
  +- wp-gpios : Specify GPIOs for write protection
  +- slot-4bit: Specify 4 bit mode support
  +- slot-8bit: Specify 8 bit and 4 bit mode support
  +
  +Examples:
  +mmc1: ssp@8001 {
  +   compatible = fsl,imx28-mmc;
  +   reg = 0x8001 2000;
  +   /* irq_err irq_dma */
  +   interrupts = 96 82;
  +   dma_channel = 0;
  +   slot-8bit;
  +};
  diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
  index 382c835..6cf2d17 100644
  --- a/drivers/mmc/host/mxs-mmc.c
  +++ b/drivers/mmc/host/mxs-mmc.c
  @@ -38,6 +38,10 @@
   #include linux/gpio.h
   #include linux/regulator/consumer.h
   #include linux/module.h
  +#include linux/of.h
  +#include linux/of_device.h
  +#include linux/of_gpio.h
  +#include linux/slab.h
   
   #include mach/mxs.h
   #include mach/common.h
  @@ -673,17 +677,79 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan, 
  void *param)
  return true;
   }
   
  +#ifdef CONFIG_OF
  +static struct resource * __devinit mxs_mmc_get_of_dmares(
  +   struct platform_device *pdev)
  +{
  +   struct device_node *np = pdev-dev.of_node;
  +   struct resource *dmares;
  +   int ret;
  +
  +   if (!np)
  +   return NULL;
  +
  +   dmares = kzalloc(sizeof(*dmares), GFP_KERNEL);
  +   dmares-flags = IORESOURCE_DMA;
  +   ret = of_property_read_u32(np, dma_channel, dmares-start);
  +   if (ret) {
  +   dev_err(pdev-dev, unable to get dmares from dt\n);
  +   return NULL;
  +   }
  +   dmares-end = dmares-start;
  +
  +   return dmares;
  +}
  +
  +static int __devinit mxs_mmc_get_of_property(struct platform_device *pdev,
  +   struct mxs_mmc_platform_data **ppdata)
  +{
  +   struct device_node *np = pdev-dev.of_node;
  +   struct mxs_mmc_platform_data *pdata = *ppdata;
  +
  +   if (!np)
  +   return -ENODEV;
  +
  +   pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
  +
  +   if (of_get_property(np, slot-8bit, NULL))
  +   pdata-flags |= SLOTF_8_BIT_CAPABLE;
  +
  +   if (of_get_property(np, slot-4bit, NULL))
  +   pdata-flags |= SLOTF_4_BIT_CAPABLE;
 it will conflit if both binding are set use a number instead
 
Hmm, i did not see conflict, can you explain more?
The slot-8bit includes the support for 4bit.(see binding doc)
Even user define them two property in dt by mistake, it does not cause conflict.
See:
if (pdata) {
if (pdata-flags  SLOTF_8_BIT_CAPABLE)
mmc-caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
if (pdata-flags  SLOTF_4_BIT_CAPABLE)
mmc-caps |= MMC_CAP_4_BIT_DATA;
}

Regards
Dong Aisheng

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Jean-Christophe PLAGNIOL-VILLARD
On 16:09 Wed 14 Mar , Dong Aisheng wrote:
 On Wed, Mar 14, 2012 at 03:23:43PM +0800, Jean-Christophe PLAGNIOL-VILLARD 
 wrote:
  On 16:47 Tue 13 Mar , Dong Aisheng wrote:
   From: Dong Aisheng dong.aish...@linaro.org
   
   Signed-off-by: Dong Aisheng dong.aish...@linaro.org
   
   ---
   The patch is still using a private way for dma part binding
   since the common dma binding is still under discussion.
   http://www.spinics.net/lists/linux-omap/msg65528.html
   
   Will update to use common dma binding when it hits mainline.
   ---
.../devicetree/bindings/mmc/fsl-mxs-mmc.txt|   23 ++
drivers/mmc/host/mxs-mmc.c |   82 
   +++-
2 files changed, 102 insertions(+), 3 deletions(-)
   
   diff --git a/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt 
   b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
   new file mode 100644
   index 000..adc1142
   --- /dev/null
   +++ b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
   @@ -0,0 +1,23 @@
   +* FREESCALE MXS MMC peripheral
   +
   +Required properties:
   +- compatible : Should be fsl,chip-mmc
   +- reg : Should contain registers location and length
   +- interrupts : Should contain interrupt.
   +  The format is irq_err irq_dma.
   +- dma_channel: Should contain the dma channel it uses
   +
   +Optional properties:
   +- wp-gpios : Specify GPIOs for write protection
   +- slot-4bit: Specify 4 bit mode support
   +- slot-8bit: Specify 8 bit and 4 bit mode support
   +
   +Examples:
   +mmc1: ssp@8001 {
   + compatible = fsl,imx28-mmc;
   + reg = 0x8001 2000;
   + /* irq_err irq_dma */
   + interrupts = 96 82;
   + dma_channel = 0;
   + slot-8bit;
   +};
   diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
   index 382c835..6cf2d17 100644
   --- a/drivers/mmc/host/mxs-mmc.c
   +++ b/drivers/mmc/host/mxs-mmc.c
   @@ -38,6 +38,10 @@
#include linux/gpio.h
#include linux/regulator/consumer.h
#include linux/module.h
   +#include linux/of.h
   +#include linux/of_device.h
   +#include linux/of_gpio.h
   +#include linux/slab.h

#include mach/mxs.h
#include mach/common.h
   @@ -673,17 +677,79 @@ static bool mxs_mmc_dma_filter(struct dma_chan 
   *chan, void *param)
 return true;
}

   +#ifdef CONFIG_OF
   +static struct resource * __devinit mxs_mmc_get_of_dmares(
   + struct platform_device *pdev)
   +{
   + struct device_node *np = pdev-dev.of_node;
   + struct resource *dmares;
   + int ret;
   +
   + if (!np)
   + return NULL;
   +
   + dmares = kzalloc(sizeof(*dmares), GFP_KERNEL);
   + dmares-flags = IORESOURCE_DMA;
   + ret = of_property_read_u32(np, dma_channel, dmares-start);
   + if (ret) {
   + dev_err(pdev-dev, unable to get dmares from dt\n);
   + return NULL;
   + }
   + dmares-end = dmares-start;
   +
   + return dmares;
   +}
   +
   +static int __devinit mxs_mmc_get_of_property(struct platform_device 
   *pdev,
   + struct mxs_mmc_platform_data **ppdata)
   +{
   + struct device_node *np = pdev-dev.of_node;
   + struct mxs_mmc_platform_data *pdata = *ppdata;
   +
   + if (!np)
   + return -ENODEV;
   +
   + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
   +
   + if (of_get_property(np, slot-8bit, NULL))
   + pdata-flags |= SLOTF_8_BIT_CAPABLE;
   +
   + if (of_get_property(np, slot-4bit, NULL))
   + pdata-flags |= SLOTF_4_BIT_CAPABLE;
  it will conflit if both binding are set use a number instead
  
 Hmm, i did not see conflict, can you explain more?
 The slot-8bit includes the support for 4bit.(see binding doc)
 Even user define them two property in dt by mistake, it does not cause 
 conflict.
 See:
 if (pdata) {
   if (pdata-flags  SLOTF_8_BIT_CAPABLE)
   mmc-caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
   if (pdata-flags  SLOTF_4_BIT_CAPABLE)
   mmc-caps |= MMC_CAP_4_BIT_DATA;
 }

this is mmc specifc sohould have a generic binding

Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-14 Thread Marek Vasut
Dear Dong Aisheng,

 On Wed, Mar 14, 2012 at 08:09:22AM +0100, Marek Vasut wrote:
  Dear Dong Aisheng,
  
   On Wed, Mar 14, 2012 at 01:58:25PM +0800, Marek Vasut wrote:
Dear Dong Aisheng,

 Signed-off-by: Dong Aisheng dong.aish...@linaro.org
   
   
   
 +static const struct of_device_id mxs_mmc_dt_ids[] = {
 + { .compatible = fsl,imx23-mmc, .data = NULL, },
 + { .compatible = fsl,imx28-mmc, .data = NULL, },

Do you really need two distinct ones here?
   
   Hmm, my original purpose is to put soc difference data in .data
   to remove cpu_is_* function calls in the driver later.
   Do you think if any issue?
  
  Well, what's the difference between the interfaces on mx233 and mx28? Is
  it something that can't be encoded otherwise? I think they're not so
  different.
 
 Not much difference except the one register offset and ip version.
 See:
 #define SSP_VERSION_LATEST  4
 #define ssp_is_old()(host-version  SSP_VERSION_LATEST)
 ..
 #define HW_SSP_VERSION (cpu_is_mx23() ? 0x110 : 0x130)
 The ip version can be handled in driver, but for offset...
 it depends on cpu_is_* macro.
 Putting the HW_SSP_VERSION offset difference in .data can eliminate the
 need of cpu_is_*.
 
 Despite of that, since they're two devices,
 i guess it's ok to put two compatible string there, right?
 Or you thought just put one as below?
 { .compatible = fsl,mxs-mmc, .data = NULL, },
 
No, I understand now /wrt the register layout.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support

2012-03-13 Thread Grant Likely
On Tue, 13 Mar 2012 16:47:05 +0800, Dong Aisheng b29...@freescale.com wrote:
 From: Dong Aisheng dong.aish...@linaro.org
 
 Signed-off-by: Dong Aisheng dong.aish...@linaro.org
 
 ---
 The patch is still using a private way for dma part binding
 since the common dma binding is still under discussion.
 http://www.spinics.net/lists/linux-omap/msg65528.html
 
 Will update to use common dma binding when it hits mainline.
 ---
  .../devicetree/bindings/mmc/fsl-mxs-mmc.txt|   23 ++
  drivers/mmc/host/mxs-mmc.c |   82 
 +++-
  2 files changed, 102 insertions(+), 3 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt 
 b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
 new file mode 100644
 index 000..adc1142
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
 @@ -0,0 +1,23 @@
 +* FREESCALE MXS MMC peripheral
 +
 +Required properties:
 +- compatible : Should be fsl,chip-mmc
 +- reg : Should contain registers location and length
 +- interrupts : Should contain interrupt.
 +  The format is irq_err irq_dma.
 +- dma_channel: Should contain the dma channel it uses

Don't use '_' in property names.

The is a generic dma binding being drafted that uses a phandle to the dma
controller and the ability to encode channel numbers.  You may want to take
a look at it.

 +
 +Optional properties:
 +- wp-gpios : Specify GPIOs for write protection
 +- slot-4bit: Specify 4 bit mode support
 +- slot-8bit: Specify 8 bit and 4 bit mode support
 +
 +Examples:
 +mmc1: ssp@8001 {
 + compatible = fsl,imx28-mmc;
 + reg = 0x8001 2000;
 + /* irq_err irq_dma */
 + interrupts = 96 82;
 + dma_channel = 0;
 + slot-8bit;
 +};
 diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
 index 382c835..6cf2d17 100644
 --- a/drivers/mmc/host/mxs-mmc.c
 +++ b/drivers/mmc/host/mxs-mmc.c
 @@ -38,6 +38,10 @@
  #include linux/gpio.h
  #include linux/regulator/consumer.h
  #include linux/module.h
 +#include linux/of.h
 +#include linux/of_device.h
 +#include linux/of_gpio.h
 +#include linux/slab.h
  
  #include mach/mxs.h
  #include mach/common.h
 @@ -673,17 +677,79 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan, 
 void *param)
   return true;
  }
  
 +#ifdef CONFIG_OF
 +static struct resource * __devinit mxs_mmc_get_of_dmares(
 + struct platform_device *pdev)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct resource *dmares;
 + int ret;
 +
 + if (!np)
 + return NULL;
 +
 + dmares = kzalloc(sizeof(*dmares), GFP_KERNEL);

devm_kzalloc()

 + dmares-flags = IORESOURCE_DMA;
 + ret = of_property_read_u32(np, dma_channel, dmares-start);
 + if (ret) {
 + dev_err(pdev-dev, unable to get dmares from dt\n);
 + return NULL;
 + }
 + dmares-end = dmares-start;
 +
 + return dmares;
 +}
 +
 +static int __devinit mxs_mmc_get_of_property(struct platform_device *pdev,
 + struct mxs_mmc_platform_data **ppdata)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct mxs_mmc_platform_data *pdata = *ppdata;
 +
 + if (!np)
 + return -ENODEV;
 +
 + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);

Ditto

Fix up those comments and you can add my:

Acked-by: Grant Likely grant.lik...@secretlab.ca

 +
 + if (of_get_property(np, slot-8bit, NULL))
 + pdata-flags |= SLOTF_8_BIT_CAPABLE;
 +
 + if (of_get_property(np, slot-4bit, NULL))
 + pdata-flags |= SLOTF_4_BIT_CAPABLE;
 +
 + pdata-wp_gpio = of_get_named_gpio(np, wp-gpios, 0);
 +
 + dev_dbg(pdev-dev, wp-gpios %d flags %d\n, pdata-wp_gpio,
 + pdata-flags);
 +
 + return 0;
 +}
 +#else
 +static struct resource * __devinit mxs_mmc_get_of_dmares(
 + struct platform_device *pdev)
 +{
 + return NULL;
 +}
 +static inline int mxs_mmc_get_of_property(struct platform_device *pdev,
 + struct mxs_mmc_platform_data *pdata)
 +{
 + return -ENODEV;
 +}
 +#endif
 +
  static int mxs_mmc_probe(struct platform_device *pdev)
  {
   struct mxs_mmc_host *host;
   struct mmc_host *mmc;
   struct resource *iores, *dmares, *r;
 - struct mxs_mmc_platform_data *pdata;
 + struct mxs_mmc_platform_data *pdata = NULL;
   int ret = 0, irq_err, irq_dma;
   dma_cap_mask_t mask;
  
   iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 - dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 + dmares = mxs_mmc_get_of_dmares(pdev);
 + if (dmares == NULL)
 + dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
   irq_err = platform_get_irq(pdev, 0);
   irq_dma = platform_get_irq(pdev, 1);
   if (!iores || !dmares || irq_err  0 || irq_dma  0)
 @@ -740,7 +806,9 @@ static int mxs_mmc_probe(struct platform_device *pdev)
   mmc-caps =