Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support
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
> > >
> > >
> > >
> > > > > +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
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
> > >
> > > Signed-off-by: Dong Aisheng
> > >
> > > ---
> > > 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,-mmc"
> > > +- reg : Should contain registers location and length
> > > +- interrupts : Should contain interrupt.
> > > + The format is .
> > > +- 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>;
> > > + /* */
> > > + 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
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > >
> > > #include
> > > #include
> > > @@ -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
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
> >
> > Signed-off-by: Dong Aisheng
> >
> > ---
> > 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,-mmc"
> > +- reg : Should contain registers location and length
> > +- interrupts : Should contain interrupt.
> > + The format is .
> > +- 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>;
> > + /* */
> > + 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
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> > +#include
> >
> > #include
> > #include
> > @@ -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
On 16:47 Tue 13 Mar , Dong Aisheng wrote:
> From: Dong Aisheng
>
> Signed-off-by: Dong Aisheng
>
> ---
> 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,-mmc"
> +- reg : Should contain registers location and length
> +- interrupts : Should contain interrupt.
> + The format is .
> +- 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>;
> + /* */
> + 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
> #include
> #include
> +#include
> +#include
> +#include
> +#include
>
> #include
> #include
> @@ -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
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
> >
> >
> >
> > > > +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
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
> >
> >
> >
> > > > +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
Dear Dong Aisheng,
> On Wed, Mar 14, 2012 at 01:58:25PM +0800, Marek Vasut wrote:
> > Dear Dong Aisheng,
> >
> > > Signed-off-by: Dong Aisheng
>
>
>
> > > +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
On Wed, Mar 14, 2012 at 01:58:25PM +0800, Marek Vasut wrote:
> Dear Dong Aisheng,
>
> >
> > Signed-off-by: Dong Aisheng
> > +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
On Wed, Mar 14, 2012 at 01:42:38AM +0800, Grant Likely wrote:
> On Tue, 13 Mar 2012 16:47:05 +0800, Dong Aisheng wrote:
> > From: Dong Aisheng
> >
> > Signed-off-by: Dong Aisheng
> >
> > ---
> > 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,-mmc"
> > +- reg : Should contain registers location and length
> > +- interrupts : Should contain interrupt.
> > + The format is .
> > +- 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
>
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
Dear Dong Aisheng,
> From: Dong Aisheng
>
> Signed-off-by: Dong Aisheng
>
> ---
> 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,-mmc"
> +- reg : Should contain registers location and length
> +- interrupts : Should contain interrupt.
> + The format is .
> +- 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>;
> + /* */
> + 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
> #include
> #include
> +#include
> +#include
> +#include
> +#include
>
> #include
> #include
> @@ -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 |= MMC_CAP_4_BIT_DATA
Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support
On Tue, 13 Mar 2012 16:47:05 +0800, Dong Aisheng wrote:
> From: Dong Aisheng
>
> Signed-off-by: Dong Aisheng
>
> ---
> 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,-mmc"
> +- reg : Should contain registers location and length
> +- interrupts : Should contain interrupt.
> + The format is .
> +- 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>;
> + /* */
> + 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
> #include
> #include
> +#include
> +#include
> +#include
> +#include
>
> #include
> #include
> @@ -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
> +
> + 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_SDI
