Re: [PATCH v4 3/5] mmc: davinci_mmc: add DT support
On 3/5/2013 4:54 PM, Manjunathappa, Prakash wrote:
>>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>>> + if (!pdata) {
>>> + dev_err(&pdev->dev, "Failed to allocate memory for struct
>>> davinci_mmc_config\n");
>>> + goto nodata;
>>
>> devm_kzalloc() returns NULL on error so you will end up returning
>> success in this case.
>>
>
> Returns NULL on failure, caller is checking on NULL for failures.
>
> pdata = mmc_parse_pdata(pdev);
> if (pdata == NULL) {
> dev_err(&pdev->dev, "Can not get platform data\n");
> return -ENOENT;
> }
Ha, okay. I was somehow confused into thinking this is part of probe
routine.
Thanks,
Sekhar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v4 3/5] mmc: davinci_mmc: add DT support
Hi Sekhar,
On Wed, Feb 27, 2013 at 17:13:43, Nori, Sekhar wrote:
> On 2/15/2013 11:52 AM, Manjunathappa, Prakash wrote:
> > Adds device tree support for davinci_mmc. Also add binding documentation.
> > As of now in non-dma PIO mode and without GPIO card_detect/write_protect
> > option because of dependencies on EDMA and GPIO module DT support.
> >
> > Signed-off-by: Manjunathappa, Prakash
> > Reviewed-by: Mark Rutland
> > Cc: linux-...@vger.kernel.org
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: davinci-linux-open-sou...@linux.davincidsp.com
> > Cc: devicetree-discuss@lists.ozlabs.org
> > Cc: c...@laptop.org
> > Cc: Sekhar Nori
> > Cc: mpor...@ti.com
>
> > diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> > index 874dc60..6b08b9a 100644
> > --- a/drivers/mmc/host/davinci_mmc.c
> > +++ b/drivers/mmc/host/davinci_mmc.c
> > @@ -34,6 +34,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> > #include
> >
> > @@ -1156,6 +1158,7 @@ static void __init init_mmcsd_host(struct
> > mmc_davinci_host *host)
> >
> > mmc_davinci_reset_ctrl(host, 0);
> > }
> > +
>
> Stray line addition.
>
I will fix this.
> > static struct platform_device_id davinci_mmc_devtype[] = {
> > {
> > .name = "davinci-mmc-dm355",
> > @@ -1168,9 +1171,63 @@ static struct platform_device_id
> > davinci_mmc_devtype[] = {
> > };
> > MODULE_DEVICE_TABLE(platform, davinci_mmc_devtype);
> >
> > -static int __init davinci_mmcsd_probe(struct platform_device *pdev)
> > +static const struct of_device_id davinci_mmc_dt_ids[] = {
> > + {
> > + .compatible = "ti,davinci-mmc-dm355",
> > + .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_1],
> > + },
> > + {
> > + .compatible = "ti,davinci-mmc-da830",
> > + .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_2],
> > + },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, davinci_mmc_dt_ids);
> > +
> > +static struct davinci_mmc_config
> > + *mmc_parse_pdata(struct platform_device *pdev)
> > {
> > + struct device_node *np;
> > struct davinci_mmc_config *pdata = pdev->dev.platform_data;
> > + const struct of_device_id *match =
> > + of_match_device(of_match_ptr(davinci_mmc_dt_ids), &pdev->dev);
> > + u32 data;
> > +
> > + np = pdev->dev.of_node;
> > + if (!np)
> > + return pdata;
> > +
> > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> > + if (!pdata) {
> > + dev_err(&pdev->dev, "Failed to allocate memory for struct
> > davinci_mmc_config\n");
> > + goto nodata;
>
> devm_kzalloc() returns NULL on error so you will end up returning
> success in this case.
>
Returns NULL on failure, caller is checking on NULL for failures.
pdata = mmc_parse_pdata(pdev);
if (pdata == NULL) {
dev_err(&pdev->dev, "Can not get platform data\n");
return -ENOENT;
}
> > + }
> > +
> > + if (match)
> > + pdev->id_entry = match->data;
> > +
> > + of_property_read_u32(np, "max-frequency", &pdata->max_freq);
> > + if (!pdata->max_freq)
> > + dev_info(&pdev->dev, "'max-frequency' property not specified,
> > defaulting to 25MHz\n");
>
>
> of_property_read_u32() returns error on failure. pdata->max_freq is not
> guaranteed to be zero in case of error. See comments atop
> of_property_read_u32_array().
>
As pdata is from kzalloc, max_freq will remain NULL in case
of_property_read_u32_array fails.
It makes sense to check for return value, I agree to change this.
Thanks,
Prakash
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 3/5] mmc: davinci_mmc: add DT support
On 2/15/2013 11:52 AM, Manjunathappa, Prakash wrote:
> Adds device tree support for davinci_mmc. Also add binding documentation.
> As of now in non-dma PIO mode and without GPIO card_detect/write_protect
> option because of dependencies on EDMA and GPIO module DT support.
>
> Signed-off-by: Manjunathappa, Prakash
> Reviewed-by: Mark Rutland
> Cc: linux-...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: davinci-linux-open-sou...@linux.davincidsp.com
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: c...@laptop.org
> Cc: Sekhar Nori
> Cc: mpor...@ti.com
> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index 874dc60..6b08b9a 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -34,6 +34,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include
>
> @@ -1156,6 +1158,7 @@ static void __init init_mmcsd_host(struct
> mmc_davinci_host *host)
>
> mmc_davinci_reset_ctrl(host, 0);
> }
> +
Stray line addition.
> static struct platform_device_id davinci_mmc_devtype[] = {
> {
> .name = "davinci-mmc-dm355",
> @@ -1168,9 +1171,63 @@ static struct platform_device_id davinci_mmc_devtype[]
> = {
> };
> MODULE_DEVICE_TABLE(platform, davinci_mmc_devtype);
>
> -static int __init davinci_mmcsd_probe(struct platform_device *pdev)
> +static const struct of_device_id davinci_mmc_dt_ids[] = {
> + {
> + .compatible = "ti,davinci-mmc-dm355",
> + .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_1],
> + },
> + {
> + .compatible = "ti,davinci-mmc-da830",
> + .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_2],
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, davinci_mmc_dt_ids);
> +
> +static struct davinci_mmc_config
> + *mmc_parse_pdata(struct platform_device *pdev)
> {
> + struct device_node *np;
> struct davinci_mmc_config *pdata = pdev->dev.platform_data;
> + const struct of_device_id *match =
> + of_match_device(of_match_ptr(davinci_mmc_dt_ids), &pdev->dev);
> + u32 data;
> +
> + np = pdev->dev.of_node;
> + if (!np)
> + return pdata;
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + dev_err(&pdev->dev, "Failed to allocate memory for struct
> davinci_mmc_config\n");
> + goto nodata;
devm_kzalloc() returns NULL on error so you will end up returning
success in this case.
> + }
> +
> + if (match)
> + pdev->id_entry = match->data;
> +
> + of_property_read_u32(np, "max-frequency", &pdata->max_freq);
> + if (!pdata->max_freq)
> + dev_info(&pdev->dev, "'max-frequency' property not specified,
> defaulting to 25MHz\n");
of_property_read_u32() returns error on failure. pdata->max_freq is not
guaranteed to be zero in case of error. See comments atop
of_property_read_u32_array().
> +
> + of_property_read_u32(np, "bus-width", &data);
> + switch (data) {
> + case 1:
> + case 4:
> + case 8:
> + pdata->wires = data;
> + break;
> + default:
> + pdata->wires = 1;
> + dev_info(&pdev->dev, "Unsupported buswidth, defaulting to 1
> bit\n");
> + }
> +nodata:
> + return pdata;
> +}
> +
Thanks,
Sekhar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v4 3/5] mmc: davinci_mmc: add DT support
Adds device tree support for davinci_mmc. Also add binding documentation.
As of now in non-dma PIO mode and without GPIO card_detect/write_protect
option because of dependencies on EDMA and GPIO module DT support.
Signed-off-by: Manjunathappa, Prakash
Reviewed-by: Mark Rutland
Cc: linux-...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: devicetree-discuss@lists.ozlabs.org
Cc: c...@laptop.org
Cc: Sekhar Nori
Cc: mpor...@ti.com
---
Since v2:
Modified the default value for bus-width and removed the support for
highspeed card capabilties for now.
Since v1:
Modified DT parse function to take default values and accomodate controller
version in compatible field.
.../devicetree/bindings/mmc/davinci_mmc.txt| 25
drivers/mmc/host/davinci_mmc.c | 66 +++-
2 files changed, 89 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/davinci_mmc.txt
diff --git a/Documentation/devicetree/bindings/mmc/davinci_mmc.txt
b/Documentation/devicetree/bindings/mmc/davinci_mmc.txt
new file mode 100644
index 000..247be0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/davinci_mmc.txt
@@ -0,0 +1,25 @@
+* TI Highspeed MMC host controller for DaVinci
+
+The Highspeed MMC Host Controller on TI DaVinci family
+provides an interface for MMC, SD and SDIO types of memory cards.
+
+This file documents the properties used by the davinci_mmc driver.
+
+Required properties:
+- compatible:
+ Should be "ti,davinci-mmc-da830": for da830, da850, dm365
+ Should be "ti,davinci-mmc-dm355": for dm355, dm644x
+
+Optional properties:
+- bus-width: Number of data lines, can be <1>, <4>, or <8>, default <1>
+- max-frequency: Maximum operating clock frequency, default 25MHz.
+
+Example:
+ mmc0: mmc@1c4 {
+ compatible = "ti,davinci-mmc-da830",
+ reg = <0x4 0x1000>;
+ interrupts = <16>;
+ status = "okay";
+ bus-width = <4>;
+ max-frequency = <5000>;
+ };
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 874dc60..6b08b9a 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -34,6 +34,8 @@
#include
#include
#include
+#include
+#include
#include
@@ -1156,6 +1158,7 @@ static void __init init_mmcsd_host(struct
mmc_davinci_host *host)
mmc_davinci_reset_ctrl(host, 0);
}
+
static struct platform_device_id davinci_mmc_devtype[] = {
{
.name = "davinci-mmc-dm355",
@@ -1168,9 +1171,63 @@ static struct platform_device_id davinci_mmc_devtype[] =
{
};
MODULE_DEVICE_TABLE(platform, davinci_mmc_devtype);
-static int __init davinci_mmcsd_probe(struct platform_device *pdev)
+static const struct of_device_id davinci_mmc_dt_ids[] = {
+ {
+ .compatible = "ti,davinci-mmc-dm355",
+ .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_1],
+ },
+ {
+ .compatible = "ti,davinci-mmc-da830",
+ .data = &davinci_mmc_devtype[MMC_CTLR_VERSION_2],
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, davinci_mmc_dt_ids);
+
+static struct davinci_mmc_config
+ *mmc_parse_pdata(struct platform_device *pdev)
{
+ struct device_node *np;
struct davinci_mmc_config *pdata = pdev->dev.platform_data;
+ const struct of_device_id *match =
+ of_match_device(of_match_ptr(davinci_mmc_dt_ids), &pdev->dev);
+ u32 data;
+
+ np = pdev->dev.of_node;
+ if (!np)
+ return pdata;
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(&pdev->dev, "Failed to allocate memory for struct
davinci_mmc_config\n");
+ goto nodata;
+ }
+
+ if (match)
+ pdev->id_entry = match->data;
+
+ of_property_read_u32(np, "max-frequency", &pdata->max_freq);
+ if (!pdata->max_freq)
+ dev_info(&pdev->dev, "'max-frequency' property not specified,
defaulting to 25MHz\n");
+
+ of_property_read_u32(np, "bus-width", &data);
+ switch (data) {
+ case 1:
+ case 4:
+ case 8:
+ pdata->wires = data;
+ break;
+ default:
+ pdata->wires = 1;
+ dev_info(&pdev->dev, "Unsupported buswidth, defaulting to 1
bit\n");
+ }
+nodata:
+ return pdata;
+}
+
+static int __init davinci_mmcsd_probe(struct platform_device *pdev)
+{
+ struct davinci_mmc_config *pdata = NULL;
struct mmc_davinci_host *host = NULL;
struct mmc_host *mmc = NULL;
struct resource *r, *mem = NULL;
@@ -1178,7 +1235,11 @@ static int __init davinci_mmcsd_probe(struct
platform_device *pdev)
size_t mem_size;
const struct platform_device_id *id_entry;
- /* REVISIT: when we're fully co
