Re: [PATCH v2 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Wed, Jul 06, 2011 at 11:43:15PM +0800, Shawn Guo wrote: > On Tue, Jul 05, 2011 at 11:54:34AM -0600, Grant Likely wrote: > > On Tue, Jul 5, 2011 at 9:26 AM, Shawn Guo wrote: > > > The patch adds device tree probe support for sdhci-esdhc-imx driver. > > > > > > Signed-off-by: Shawn Guo > > > Cc: Wolfram Sang > > > Cc: Chris Ball > > > Cc: Grant Likely > > > --- > > > .../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40 > > > drivers/mmc/host/sdhci-esdhc-imx.c | 102 > > > +++- > > > 2 files changed, 137 insertions(+), 5 deletions(-) > > > create mode 100644 > > > Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > > b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > > new file mode 100644 > > > index 000..351d239 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > > @@ -0,0 +1,40 @@ > > > +* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX > > > + > > > +The Enhanced Secure Digital Host Controller on Freescale i.MX family > > > +provides an interface for MMC, SD, and SDIO types of memory cards. > > > + > > > +Required properties: > > > +- compatible : Should be "fsl,-esdhc" > > > +- reg : Should contain eSDHC registers location and length > > > +- interrupts : Should contain eSDHC interrupt > > > +- cd-type : String, card detection (CD) method. Supported values are: > > > + "none" : No CD > > > + "controller" : Uses eSDHC controller internal CD signal > > > + "gpio" : Uses GPIO pin for CD > > > + "permanent" : No CD because card is permanently wired to host > > > +- wp-type : String, write protection (WP) method. Supported values are: > > > + "none" : No WP > > > + "controller" : Uses eSDHC controller internal WP signal > > > + "gpio" : Uses GPIO pin for WP > > > +- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if > > > + properties cd-type and wp-type are "gpio". > > > > Again, be explicit in your gpios property names. Create a different > > property for each gpio: cd-gpios and wp-gpios. > > > > As for wp-type and cd-type, I think you can drop them. Default to > > internal controller CD and WP pins. Use gpio if cd-gpios or wp-gpios > > is present, and define specific properties for the no-wp, no-cd and > > fixed-card cases. (can you tell that I'm not a fan of the "*-type" > > binding for this driver?) :-) > > > I would let default be no CD/WP, and define properties for > controller internal CD/WP and wired case, if you do not see a > problem with it. Okay. g. -- 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 v2 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Tue, Jul 05, 2011 at 11:54:34AM -0600, Grant Likely wrote: > On Tue, Jul 5, 2011 at 9:26 AM, Shawn Guo wrote: > > The patch adds device tree probe support for sdhci-esdhc-imx driver. > > > > Signed-off-by: Shawn Guo > > Cc: Wolfram Sang > > Cc: Chris Ball > > Cc: Grant Likely > > --- > > .../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40 > > drivers/mmc/host/sdhci-esdhc-imx.c | 102 > > +++- > > 2 files changed, 137 insertions(+), 5 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > > > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > new file mode 100644 > > index 000..351d239 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt > > @@ -0,0 +1,40 @@ > > +* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX > > + > > +The Enhanced Secure Digital Host Controller on Freescale i.MX family > > +provides an interface for MMC, SD, and SDIO types of memory cards. > > + > > +Required properties: > > +- compatible : Should be "fsl,-esdhc" > > +- reg : Should contain eSDHC registers location and length > > +- interrupts : Should contain eSDHC interrupt > > +- cd-type : String, card detection (CD) method. Supported values are: > > + "none" : No CD > > + "controller" : Uses eSDHC controller internal CD signal > > + "gpio" : Uses GPIO pin for CD > > + "permanent" : No CD because card is permanently wired to host > > +- wp-type : String, write protection (WP) method. Supported values are: > > + "none" : No WP > > + "controller" : Uses eSDHC controller internal WP signal > > + "gpio" : Uses GPIO pin for WP > > +- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if > > + properties cd-type and wp-type are "gpio". > > Again, be explicit in your gpios property names. Create a different > property for each gpio: cd-gpios and wp-gpios. > > As for wp-type and cd-type, I think you can drop them. Default to > internal controller CD and WP pins. Use gpio if cd-gpios or wp-gpios > is present, and define specific properties for the no-wp, no-cd and > fixed-card cases. (can you tell that I'm not a fan of the "*-type" > binding for this driver?) :-) > I would let default be no CD/WP, and define properties for controller internal CD/WP and wired case, if you do not see a problem with it. -- Regards, Shawn -- 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 v2 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Tue, Jul 05, 2011 at 11:54:34AM -0600, Grant Likely wrote:
> On Tue, Jul 5, 2011 at 9:26 AM, Shawn Guo wrote:
> > The patch adds device tree probe support for sdhci-esdhc-imx driver.
> >
> > Signed-off-by: Shawn Guo
> > Cc: Wolfram Sang
> > Cc: Chris Ball
> > Cc: Grant Likely
> > ---
> > .../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40
> > drivers/mmc/host/sdhci-esdhc-imx.c | 102
> > +++-
> > 2 files changed, 137 insertions(+), 5 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> > b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> > new file mode 100644
> > index 000..351d239
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> > @@ -0,0 +1,40 @@
> > +* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
> > +
> > +The Enhanced Secure Digital Host Controller on Freescale i.MX family
> > +provides an interface for MMC, SD, and SDIO types of memory cards.
> > +
> > +Required properties:
> > +- compatible : Should be "fsl,-esdhc"
> > +- reg : Should contain eSDHC registers location and length
> > +- interrupts : Should contain eSDHC interrupt
> > +- cd-type : String, card detection (CD) method. Supported values are:
> > + "none" : No CD
> > + "controller" : Uses eSDHC controller internal CD signal
> > + "gpio" : Uses GPIO pin for CD
> > + "permanent" : No CD because card is permanently wired to host
> > +- wp-type : String, write protection (WP) method. Supported values are:
> > + "none" : No WP
> > + "controller" : Uses eSDHC controller internal WP signal
> > + "gpio" : Uses GPIO pin for WP
> > +- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if
> > + properties cd-type and wp-type are "gpio".
>
> Again, be explicit in your gpios property names. Create a different
> property for each gpio: cd-gpios and wp-gpios.
>
Sorry. I fixed the example below. But this one got missed.
> As for wp-type and cd-type, I think you can drop them. Default to
> internal controller CD and WP pins. Use gpio if cd-gpios or wp-gpios
> is present, and define specific properties for the no-wp, no-cd and
> fixed-card cases. (can you tell that I'm not a fan of the "*-type"
> binding for this driver?) :-)
>
Ok, I'm getting to know you.
> > +
> > +Examples:
> > +
> > +esdhc@70004000 {
> > + compatible = "fsl,imx51-esdhc";
> > + reg = <0x70004000 0x4000>;
> > + interrupts = <1>;
> > + fsl,cd-type = "controller";
> > + fsl,wp-type = "controller";
> > +};
> > +
> > +esdhc@70008000 {
> > + compatible = "fsl,imx51-esdhc";
> > + reg = <0x70008000 0x4000>;
> > + interrupts = <2>;
> > + fsl,cd-type = "gpio";
> > + fsl,wp-type = "gpio";
> > + cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
> > + wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */
> > +};
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 1edda29..593d6b9 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -20,6 +20,9 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> > #include
> > #include "sdhci-pltfm.h"
> > #include "sdhci-esdhc.h"
> > @@ -72,6 +75,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
> > }
> > };
> >
> > +static const struct of_device_id imx_esdhc_dt_ids[] = {
> > + { .compatible = "fsl,imx25-esdhc", .data =
> > &imx_esdhc_devtype[IMX25_ESDHC], },
> > + { .compatible = "fsl,imx35-esdhc", .data =
> > &imx_esdhc_devtype[IMX35_ESDHC], },
> > + { .compatible = "fsl,imx51-esdhc", .data =
> > &imx_esdhc_devtype[IMX51_ESDHC], },
> > + { .compatible = "fsl,imx53-esdhc", .data =
> > &imx_esdhc_devtype[IMX53_ESDHC], },
> > + { /* sentinel */ }
> > +};
> > +
> > static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
> > {
> > return data->devtype == IMX25_ESDHC;
> > @@ -305,24 +316,96 @@ static irqreturn_t cd_irq(int irq, void *data)
> > return IRQ_HANDLED;
> > };
> >
> > +#ifdef CONFIG_OF
> > +static const char *cd_types[] = {
> > + [ESDHC_CD_NONE] = "none",
> > + [ESDHC_CD_CONTROLLER] = "controller",
> > + [ESDHC_CD_GPIO] = "gpio",
> > + [ESDHC_CD_PERMANENT] = "permanent",
> > +};
> > +
> > +static const char *wp_types[] = {
> > + [ESDHC_WP_NONE] = "none",
> > + [ESDHC_WP_CONTROLLER] = "controller",
> > + [ESDHC_WP_GPIO] = "gpio",
> > +};
> > +
> > +static int __devinit sdhci_esdhc_imx_probe_dt(struct platform_device *pdev)
> > +{
> > + const struct of_device_id *of_id =
> > + of_match_device(imx_esdhc_dt_ids, &pdev->dev);
> > + struct device_node *np = pdev->dev.of_node;
> > + struct
Re: [PATCH v2 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Tue, Jul 5, 2011 at 9:26 AM, Shawn Guo wrote:
> The patch adds device tree probe support for sdhci-esdhc-imx driver.
>
> Signed-off-by: Shawn Guo
> Cc: Wolfram Sang
> Cc: Chris Ball
> Cc: Grant Likely
> ---
> .../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40
> drivers/mmc/host/sdhci-esdhc-imx.c | 102
> +++-
> 2 files changed, 137 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> new file mode 100644
> index 000..351d239
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
> @@ -0,0 +1,40 @@
> +* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
> +
> +The Enhanced Secure Digital Host Controller on Freescale i.MX family
> +provides an interface for MMC, SD, and SDIO types of memory cards.
> +
> +Required properties:
> +- compatible : Should be "fsl,-esdhc"
> +- reg : Should contain eSDHC registers location and length
> +- interrupts : Should contain eSDHC interrupt
> +- cd-type : String, card detection (CD) method. Supported values are:
> + "none" : No CD
> + "controller" : Uses eSDHC controller internal CD signal
> + "gpio" : Uses GPIO pin for CD
> + "permanent" : No CD because card is permanently wired to host
> +- wp-type : String, write protection (WP) method. Supported values are:
> + "none" : No WP
> + "controller" : Uses eSDHC controller internal WP signal
> + "gpio" : Uses GPIO pin for WP
> +- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if
> + properties cd-type and wp-type are "gpio".
Again, be explicit in your gpios property names. Create a different
property for each gpio: cd-gpios and wp-gpios.
As for wp-type and cd-type, I think you can drop them. Default to
internal controller CD and WP pins. Use gpio if cd-gpios or wp-gpios
is present, and define specific properties for the no-wp, no-cd and
fixed-card cases. (can you tell that I'm not a fan of the "*-type"
binding for this driver?) :-)
> +
> +Examples:
> +
> +esdhc@70004000 {
> + compatible = "fsl,imx51-esdhc";
> + reg = <0x70004000 0x4000>;
> + interrupts = <1>;
> + fsl,cd-type = "controller";
> + fsl,wp-type = "controller";
> +};
> +
> +esdhc@70008000 {
> + compatible = "fsl,imx51-esdhc";
> + reg = <0x70008000 0x4000>;
> + interrupts = <2>;
> + fsl,cd-type = "gpio";
> + fsl,wp-type = "gpio";
> + cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
> + wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */
> +};
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 1edda29..593d6b9 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -20,6 +20,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> #include
> #include "sdhci-pltfm.h"
> #include "sdhci-esdhc.h"
> @@ -72,6 +75,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
> }
> };
>
> +static const struct of_device_id imx_esdhc_dt_ids[] = {
> + { .compatible = "fsl,imx25-esdhc", .data =
> &imx_esdhc_devtype[IMX25_ESDHC], },
> + { .compatible = "fsl,imx35-esdhc", .data =
> &imx_esdhc_devtype[IMX35_ESDHC], },
> + { .compatible = "fsl,imx51-esdhc", .data =
> &imx_esdhc_devtype[IMX51_ESDHC], },
> + { .compatible = "fsl,imx53-esdhc", .data =
> &imx_esdhc_devtype[IMX53_ESDHC], },
> + { /* sentinel */ }
> +};
> +
> static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
> {
> return data->devtype == IMX25_ESDHC;
> @@ -305,24 +316,96 @@ static irqreturn_t cd_irq(int irq, void *data)
> return IRQ_HANDLED;
> };
>
> +#ifdef CONFIG_OF
> +static const char *cd_types[] = {
> + [ESDHC_CD_NONE] = "none",
> + [ESDHC_CD_CONTROLLER] = "controller",
> + [ESDHC_CD_GPIO] = "gpio",
> + [ESDHC_CD_PERMANENT] = "permanent",
> +};
> +
> +static const char *wp_types[] = {
> + [ESDHC_WP_NONE] = "none",
> + [ESDHC_WP_CONTROLLER] = "controller",
> + [ESDHC_WP_GPIO] = "gpio",
> +};
> +
> +static int __devinit sdhci_esdhc_imx_probe_dt(struct platform_device *pdev)
> +{
> + const struct of_device_id *of_id =
> + of_match_device(imx_esdhc_dt_ids, &pdev->dev);
> + struct device_node *np = pdev->dev.of_node;
> + struct esdhc_platform_data *boarddata;
> + int err, i;
> + const char *cd, *wp;
> +
> + if (!np)
> + return -ENODEV;
> +
> + boarddata = kzalloc(sizeof(*boarddata), GFP_KERNEL);
> + if (!boarddata)
> + return -ENOMEM;
> + pdev->dev.platform_data = boarddata;
This is illegal. As far as device drivers are concerned,
pdev->dev.platform_data is immutable
[PATCH v2 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
The patch adds device tree probe support for sdhci-esdhc-imx driver.
Signed-off-by: Shawn Guo
Cc: Wolfram Sang
Cc: Chris Ball
Cc: Grant Likely
---
.../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40
drivers/mmc/host/sdhci-esdhc-imx.c | 102 +++-
2 files changed, 137 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
new file mode 100644
index 000..351d239
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -0,0 +1,40 @@
+* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
+
+The Enhanced Secure Digital Host Controller on Freescale i.MX family
+provides an interface for MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible : Should be "fsl,-esdhc"
+- reg : Should contain eSDHC registers location and length
+- interrupts : Should contain eSDHC interrupt
+- cd-type : String, card detection (CD) method. Supported values are:
+"none" : No CD
+"controller" : Uses eSDHC controller internal CD signal
+"gpio" : Uses GPIO pin for CD
+"permanent" : No CD because card is permanently wired to host
+- wp-type : String, write protection (WP) method. Supported values are:
+"none" : No WP
+"controller" : Uses eSDHC controller internal WP signal
+"gpio" : Uses GPIO pin for WP
+- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if
+ properties cd-type and wp-type are "gpio".
+
+Examples:
+
+esdhc@70004000 {
+ compatible = "fsl,imx51-esdhc";
+ reg = <0x70004000 0x4000>;
+ interrupts = <1>;
+ fsl,cd-type = "controller";
+ fsl,wp-type = "controller";
+};
+
+esdhc@70008000 {
+ compatible = "fsl,imx51-esdhc";
+ reg = <0x70008000 0x4000>;
+ interrupts = <2>;
+ fsl,cd-type = "gpio";
+ fsl,wp-type = "gpio";
+ cd-gpios = <&gpio0 6 0>; /* GPIO1_6 */
+ wp-gpios = <&gpio0 5 0>; /* GPIO1_5 */
+};
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 1edda29..593d6b9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -20,6 +20,9 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include "sdhci-pltfm.h"
#include "sdhci-esdhc.h"
@@ -72,6 +75,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
}
};
+static const struct of_device_id imx_esdhc_dt_ids[] = {
+ { .compatible = "fsl,imx25-esdhc", .data =
&imx_esdhc_devtype[IMX25_ESDHC], },
+ { .compatible = "fsl,imx35-esdhc", .data =
&imx_esdhc_devtype[IMX35_ESDHC], },
+ { .compatible = "fsl,imx51-esdhc", .data =
&imx_esdhc_devtype[IMX51_ESDHC], },
+ { .compatible = "fsl,imx53-esdhc", .data =
&imx_esdhc_devtype[IMX53_ESDHC], },
+ { /* sentinel */ }
+};
+
static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
{
return data->devtype == IMX25_ESDHC;
@@ -305,24 +316,96 @@ static irqreturn_t cd_irq(int irq, void *data)
return IRQ_HANDLED;
};
+#ifdef CONFIG_OF
+static const char *cd_types[] = {
+ [ESDHC_CD_NONE] = "none",
+ [ESDHC_CD_CONTROLLER] = "controller",
+ [ESDHC_CD_GPIO] = "gpio",
+ [ESDHC_CD_PERMANENT]= "permanent",
+};
+
+static const char *wp_types[] = {
+ [ESDHC_WP_NONE] = "none",
+ [ESDHC_WP_CONTROLLER] = "controller",
+ [ESDHC_WP_GPIO] = "gpio",
+};
+
+static int __devinit sdhci_esdhc_imx_probe_dt(struct platform_device *pdev)
+{
+ const struct of_device_id *of_id =
+ of_match_device(imx_esdhc_dt_ids, &pdev->dev);
+ struct device_node *np = pdev->dev.of_node;
+ struct esdhc_platform_data *boarddata;
+ int err, i;
+ const char *cd, *wp;
+
+ if (!np)
+ return -ENODEV;
+
+ boarddata = kzalloc(sizeof(*boarddata), GFP_KERNEL);
+ if (!boarddata)
+ return -ENOMEM;
+ pdev->dev.platform_data = boarddata;
+
+ err = of_property_read_string(np, "fsl,cd-type", &cd);
+ if (err)
+ return err;
+ for (i = 0; i < ARRAY_SIZE(cd_types); i++)
+ if (!strcasecmp(cd, cd_types[i])) {
+ boarddata->cd_type = i;
+ break;
+ }
+
+ err = of_property_read_string(np, "fsl,wp-type", &wp);
+ if (err)
+ return err;
+ for (i = 0; i < ARRAY_SIZE(wp_types); i++)
+ if (!strcasecmp(wp, wp_types[i])) {
+ boarddata->wp_type = i;
+ break;
+ }
+
+ boarddata->cd_gpio = of_get_gpio(np, 0);
+ boarddata->wp_gpio = of_get_gpio(np, 1);
+
+ pdev->id_entry = of_id->data;
+
+ return 0;
+}
+#else
+static inline int sdhci_
