* [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl
@ 2012-12-21 17:11 Dongjin Kim
2012-12-31 2:59 ` Dongjin Kim
2012-12-31 5:57 ` Thomas Abraham
0 siblings, 2 replies; 5+ messages in thread
From: Dongjin Kim @ 2012-12-21 17:11 UTC (permalink / raw)
Cc: Dongjin Kim, Chris Ball, Arnd Bergmann, Thomas Abraham,
Will Newton, linux-mmc, linux-kernel
This patch adds support for pin configuration using pinctrl subsystem to
dw_mmc-exynos driver. The property 'wp-gpios' can be specified for write
protect but 'samsung,cd-pinmux-gpio' and gpios used for clock, command and
data lines will be ignored.
-. 'pinctrl-0' should specify pin control groups (clock, comand and data
lines) used for this controller.
-. 'pinctrl-names' should contain only one value, 'default'.
Signed-off-by: Dongjin Kim <tobetter@gmail.com>
---
drivers/mmc/host/dw_mmc-exynos.c | 44 ++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..d1c9963 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -16,6 +16,7 @@
#include <linux/mmc/dw_mmc.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
#include "dw_mmc.h"
#include "dw_mmc-pltfm.h"
@@ -49,6 +50,7 @@ struct dw_mci_exynos_priv_data {
u8 ciu_div;
u32 sdr_timing;
u32 ddr_timing;
+ struct pinctrl *pctrl;
};
static struct dw_mci_exynos_compatible {
@@ -84,6 +86,10 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host)
priv->ctrl_type = exynos_compat[idx].ctrl_type;
}
+ priv->pctrl = devm_pinctrl_get_select_default(host->dev);
+ if (IS_ERR(priv->pctrl))
+ dev_dbg(host->dev, "no pinctrl node\n");
+
host->priv = priv;
return 0;
}
@@ -149,32 +155,19 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
return ret;
priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
+
return 0;
}
static int dw_mci_exynos_setup_bus(struct dw_mci *host,
struct device_node *slot_np, u8 bus_width)
{
+ struct dw_mci_exynos_priv_data *priv = host->priv;
int idx, gpio, ret;
if (!slot_np)
return -EINVAL;
- /* cmd + clock + bus-width pins */
- for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
- gpio = of_get_gpio(slot_np, idx);
- if (!gpio_is_valid(gpio)) {
- dev_err(host->dev, "invalid gpio: %d\n", gpio);
- return -EINVAL;
- }
-
- ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
- if (ret) {
- dev_err(host->dev, "gpio [%d] request failed\n", gpio);
- return -EBUSY;
- }
- }
-
gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
if (gpio_is_valid(gpio)) {
if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
@@ -185,9 +178,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
}
- if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
+ if (!IS_ERR(priv->pctrl))
return 0;
+ if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
+ goto setup_bus;
+
gpio = of_get_named_gpio(slot_np, "samsung,cd-pinmux-gpio", 0);
if (gpio_is_valid(gpio)) {
if (devm_gpio_request(host->dev, gpio, "dw-mci-cd"))
@@ -196,6 +192,22 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
dev_info(host->dev, "cd gpio not available");
}
+ setup_bus:
+ /* cmd + clock + bus-width pins */
+ for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
+ gpio = of_get_gpio(slot_np, idx);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(host->dev, "invalid gpio: %d\n", gpio);
+ return -EINVAL;
+ }
+
+ ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
+ if (ret) {
+ dev_err(host->dev, "gpio [%d] request failed\n", gpio);
+ return -EBUSY;
+ }
+ }
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl
2012-12-21 17:11 [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl Dongjin Kim
@ 2012-12-31 2:59 ` Dongjin Kim
2012-12-31 5:57 ` Thomas Abraham
1 sibling, 0 replies; 5+ messages in thread
From: Dongjin Kim @ 2012-12-31 2:59 UTC (permalink / raw)
To: Dongjin Kim
Cc: Chris Ball, Arnd Bergmann, Thomas Abraham, Will Newton,
linux-mmc, linux-kernel
Hi,
Does anyone have advice on this patch?
On Sat, Dec 22, 2012 at 2:11 AM, Dongjin Kim <tobetter@gmail.com> wrote:
> This patch adds support for pin configuration using pinctrl subsystem to
> dw_mmc-exynos driver. The property 'wp-gpios' can be specified for write
> protect but 'samsung,cd-pinmux-gpio' and gpios used for clock, command and
> data lines will be ignored.
>
> -. 'pinctrl-0' should specify pin control groups (clock, comand and data
> lines) used for this controller.
> -. 'pinctrl-names' should contain only one value, 'default'.
>
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
> drivers/mmc/host/dw_mmc-exynos.c | 44
> ++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c
> b/drivers/mmc/host/dw_mmc-exynos.c
> index 4d50da6..d1c9963 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -16,6 +16,7 @@
> #include <linux/mmc/dw_mmc.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> +#include <linux/pinctrl/consumer.h>
>
> #include "dw_mmc.h"
> #include "dw_mmc-pltfm.h"
> @@ -49,6 +50,7 @@ struct dw_mci_exynos_priv_data {
> u8 ciu_div;
> u32 sdr_timing;
> u32 ddr_timing;
> + struct pinctrl *pctrl;
> };
>
> static struct dw_mci_exynos_compatible {
> @@ -84,6 +86,10 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host)
> priv->ctrl_type = exynos_compat[idx].ctrl_type;
> }
>
> + priv->pctrl = devm_pinctrl_get_select_default(host->dev);
> + if (IS_ERR(priv->pctrl))
> + dev_dbg(host->dev, "no pinctrl node\n");
> +
> host->priv = priv;
> return 0;
> }
> @@ -149,32 +155,19 @@ static int dw_mci_exynos_parse_dt(struct dw_mci
> *host)
> return ret;
>
> priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
> +
> return 0;
> }
>
> static int dw_mci_exynos_setup_bus(struct dw_mci *host,
> struct device_node *slot_np, u8 bus_width)
> {
> + struct dw_mci_exynos_priv_data *priv = host->priv;
> int idx, gpio, ret;
>
> if (!slot_np)
> return -EINVAL;
>
> - /* cmd + clock + bus-width pins */
> - for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
> - gpio = of_get_gpio(slot_np, idx);
> - if (!gpio_is_valid(gpio)) {
> - dev_err(host->dev, "invalid gpio: %d\n", gpio);
> - return -EINVAL;
> - }
> -
> - ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
> - if (ret) {
> - dev_err(host->dev, "gpio [%d] request failed\n",
> gpio);
> - return -EBUSY;
> - }
> - }
> -
> gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
> if (gpio_is_valid(gpio)) {
> if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
> @@ -185,9 +178,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci
> *host,
> host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
> }
>
> - if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
> + if (!IS_ERR(priv->pctrl))
> return 0;
>
> + if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
> + goto setup_bus;
> +
> gpio = of_get_named_gpio(slot_np, "samsung,cd-pinmux-gpio", 0);
> if (gpio_is_valid(gpio)) {
> if (devm_gpio_request(host->dev, gpio, "dw-mci-cd"))
> @@ -196,6 +192,22 @@ static int dw_mci_exynos_setup_bus(struct dw_mci
> *host,
> dev_info(host->dev, "cd gpio not available");
> }
>
> + setup_bus:
> + /* cmd + clock + bus-width pins */
> + for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
> + gpio = of_get_gpio(slot_np, idx);
> + if (!gpio_is_valid(gpio)) {
> + dev_err(host->dev, "invalid gpio: %d\n", gpio);
> + return -EINVAL;
> + }
> +
> + ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
> + if (ret) {
> + dev_err(host->dev, "gpio [%d] request failed\n",
> gpio);
> + return -EBUSY;
> + }
> + }
> +
> return 0;
> }
>
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl
2012-12-21 17:11 [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl Dongjin Kim
2012-12-31 2:59 ` Dongjin Kim
@ 2012-12-31 5:57 ` Thomas Abraham
2013-01-03 18:33 ` Dongjin Kim
1 sibling, 1 reply; 5+ messages in thread
From: Thomas Abraham @ 2012-12-31 5:57 UTC (permalink / raw)
To: Dongjin Kim
Cc: Chris Ball, Arnd Bergmann, Will Newton, linux-mmc, linux-kernel
On 21 December 2012 09:11, Dongjin Kim <tobetter@gmail.com> wrote:
> This patch adds support for pin configuration using pinctrl subsystem to
> dw_mmc-exynos driver. The property 'wp-gpios' can be specified for write
> protect but 'samsung,cd-pinmux-gpio' and gpios used for clock, command and
> data lines will be ignored.
>
> -. 'pinctrl-0' should specify pin control groups (clock, comand and data
> lines) used for this controller.
> -. 'pinctrl-names' should contain only one value, 'default'.
>
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
> drivers/mmc/host/dw_mmc-exynos.c | 44 ++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index 4d50da6..d1c9963 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -16,6 +16,7 @@
> #include <linux/mmc/dw_mmc.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> +#include <linux/pinctrl/consumer.h>
>
> #include "dw_mmc.h"
> #include "dw_mmc-pltfm.h"
> @@ -49,6 +50,7 @@ struct dw_mci_exynos_priv_data {
> u8 ciu_div;
> u32 sdr_timing;
> u32 ddr_timing;
> + struct pinctrl *pctrl;
> };
>
> static struct dw_mci_exynos_compatible {
> @@ -84,6 +86,10 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host)
> priv->ctrl_type = exynos_compat[idx].ctrl_type;
> }
>
> + priv->pctrl = devm_pinctrl_get_select_default(host->dev);
> + if (IS_ERR(priv->pctrl))
> + dev_dbg(host->dev, "no pinctrl node\n");
> +
This could have been handled in dw_mci_exynos_setup_bus function. And
we also need to check if this patch gets merged.
https://patchwork.kernel.org/patch/1870231/. If it gets merged, this
change can be avoided.
> host->priv = priv;
> return 0;
> }
> @@ -149,32 +155,19 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
> return ret;
>
> priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
> +
> return 0;
> }
>
> static int dw_mci_exynos_setup_bus(struct dw_mci *host,
> struct device_node *slot_np, u8 bus_width)
> {
> + struct dw_mci_exynos_priv_data *priv = host->priv;
> int idx, gpio, ret;
>
> if (!slot_np)
> return -EINVAL;
>
> - /* cmd + clock + bus-width pins */
> - for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
> - gpio = of_get_gpio(slot_np, idx);
> - if (!gpio_is_valid(gpio)) {
> - dev_err(host->dev, "invalid gpio: %d\n", gpio);
> - return -EINVAL;
> - }
> -
> - ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
> - if (ret) {
> - dev_err(host->dev, "gpio [%d] request failed\n", gpio);
> - return -EBUSY;
> - }
> - }
> -
> gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
> if (gpio_is_valid(gpio)) {
> if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
> @@ -185,9 +178,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
> host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
> }
>
> - if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
> + if (!IS_ERR(priv->pctrl))
> return 0;
>
> + if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
> + goto setup_bus;
> +
Why do the entire bus setup if card detection is broken?
> gpio = of_get_named_gpio(slot_np, "samsung,cd-pinmux-gpio", 0);
> if (gpio_is_valid(gpio)) {
> if (devm_gpio_request(host->dev, gpio, "dw-mci-cd"))
> @@ -196,6 +192,22 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
> dev_info(host->dev, "cd gpio not available");
> }
>
> + setup_bus:
> + /* cmd + clock + bus-width pins */
> + for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
> + gpio = of_get_gpio(slot_np, idx);
> + if (!gpio_is_valid(gpio)) {
> + dev_err(host->dev, "invalid gpio: %d\n", gpio);
> + return -EINVAL;
> + }
> +
> + ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
> + if (ret) {
> + dev_err(host->dev, "gpio [%d] request failed\n", gpio);
> + return -EBUSY;
> + }
> + }
This change should not have been there. If the mmc bus setup is being
done using pinctrl framework, this change can be avoided.
Thanks,
Thomas.
> +
> return 0;
> }
>
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl
2012-12-31 5:57 ` Thomas Abraham
@ 2013-01-03 18:33 ` Dongjin Kim
2013-01-23 6:08 ` Dongjin Kim
0 siblings, 1 reply; 5+ messages in thread
From: Dongjin Kim @ 2013-01-03 18:33 UTC (permalink / raw)
To: Thomas Abraham
Cc: Chris Ball, Arnd Bergmann, Will Newton, linux-mmc, linux-kernel
Hi Thomas,
Thank you for your reviewing, and
https://patchwork.kernel.org/patch/1870231 works. So this change is
needless.
I had tested with below changes on my hardware.
[1] https://patchwork.kernel.org/patch/1904431
[2] https://patchwork.kernel.org/patch/1920661
Best regards,
Dongjin.
On Mon, Dec 31, 2012 at 2:57 PM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> On 21 December 2012 09:11, Dongjin Kim <tobetter@gmail.com> wrote:
>> This patch adds support for pin configuration using pinctrl subsystem to
>> dw_mmc-exynos driver. The property 'wp-gpios' can be specified for write
>> protect but 'samsung,cd-pinmux-gpio' and gpios used for clock, command and
>> data lines will be ignored.
>>
>> -. 'pinctrl-0' should specify pin control groups (clock, comand and data
>> lines) used for this controller.
>> -. 'pinctrl-names' should contain only one value, 'default'.
>>
>> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
>> ---
>> drivers/mmc/host/dw_mmc-exynos.c | 44 ++++++++++++++++++++++++--------------
>> 1 file changed, 28 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
>> index 4d50da6..d1c9963 100644
>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> @@ -16,6 +16,7 @@
>> #include <linux/mmc/dw_mmc.h>
>> #include <linux/of.h>
>> #include <linux/of_gpio.h>
>> +#include <linux/pinctrl/consumer.h>
>>
>> #include "dw_mmc.h"
>> #include "dw_mmc-pltfm.h"
>> @@ -49,6 +50,7 @@ struct dw_mci_exynos_priv_data {
>> u8 ciu_div;
>> u32 sdr_timing;
>> u32 ddr_timing;
>> + struct pinctrl *pctrl;
>> };
>>
>> static struct dw_mci_exynos_compatible {
>> @@ -84,6 +86,10 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host)
>> priv->ctrl_type = exynos_compat[idx].ctrl_type;
>> }
>>
>> + priv->pctrl = devm_pinctrl_get_select_default(host->dev);
>> + if (IS_ERR(priv->pctrl))
>> + dev_dbg(host->dev, "no pinctrl node\n");
>> +
>
> This could have been handled in dw_mci_exynos_setup_bus function. And
> we also need to check if this patch gets merged.
> https://patchwork.kernel.org/patch/1870231/. If it gets merged, this
> change can be avoided.
>
>> host->priv = priv;
>> return 0;
>> }
>> @@ -149,32 +155,19 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
>> return ret;
>>
>> priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
>> +
>> return 0;
>> }
>>
>> static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>> struct device_node *slot_np, u8 bus_width)
>> {
>> + struct dw_mci_exynos_priv_data *priv = host->priv;
>> int idx, gpio, ret;
>>
>> if (!slot_np)
>> return -EINVAL;
>>
>> - /* cmd + clock + bus-width pins */
>> - for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
>> - gpio = of_get_gpio(slot_np, idx);
>> - if (!gpio_is_valid(gpio)) {
>> - dev_err(host->dev, "invalid gpio: %d\n", gpio);
>> - return -EINVAL;
>> - }
>> -
>> - ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
>> - if (ret) {
>> - dev_err(host->dev, "gpio [%d] request failed\n", gpio);
>> - return -EBUSY;
>> - }
>> - }
>> -
>> gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
>> if (gpio_is_valid(gpio)) {
>> if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
>> @@ -185,9 +178,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>> host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
>> }
>>
>> - if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
>> + if (!IS_ERR(priv->pctrl))
>> return 0;
>>
>> + if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
>> + goto setup_bus;
>> +
>
> Why do the entire bus setup if card detection is broken?
>
>> gpio = of_get_named_gpio(slot_np, "samsung,cd-pinmux-gpio", 0);
>> if (gpio_is_valid(gpio)) {
>> if (devm_gpio_request(host->dev, gpio, "dw-mci-cd"))
>> @@ -196,6 +192,22 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>> dev_info(host->dev, "cd gpio not available");
>> }
>>
>> + setup_bus:
>> + /* cmd + clock + bus-width pins */
>> + for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
>> + gpio = of_get_gpio(slot_np, idx);
>> + if (!gpio_is_valid(gpio)) {
>> + dev_err(host->dev, "invalid gpio: %d\n", gpio);
>> + return -EINVAL;
>> + }
>> +
>> + ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
>> + if (ret) {
>> + dev_err(host->dev, "gpio [%d] request failed\n", gpio);
>> + return -EBUSY;
>> + }
>> + }
>
> This change should not have been there. If the mmc bus setup is being
> done using pinctrl framework, this change can be avoided.
>
> Thanks,
> Thomas.
>
>> +
>> return 0;
>> }
>>
>> --
>> 1.7.9.5
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl
2013-01-03 18:33 ` Dongjin Kim
@ 2013-01-23 6:08 ` Dongjin Kim
0 siblings, 0 replies; 5+ messages in thread
From: Dongjin Kim @ 2013-01-23 6:08 UTC (permalink / raw)
To: Thomas Abraham
Cc: Chris Ball, Arnd Bergmann, Will Newton, linux-mmc, linux-kernel
Hi Thomas,
I have a question regarding the bus setup when I use the patch
https://patchwork.kernel.org/patch/1870231. It configures buses as
defined in "pinctrl-0 = ...".
But in the function dw_mci_exynos_setup_bus(), it tries again to
configure gpios and probing is failed because there is no "gpios =
<...>" property. So I guess bus setup has to be ignored when the gpios
are configured with pinctrl.
What's your advice?
Thank you in advance.
Dongjin.
On Fri, Jan 4, 2013 at 3:33 AM, Dongjin Kim <tobetter@gmail.com> wrote:
> Hi Thomas,
>
> Thank you for your reviewing, and
> https://patchwork.kernel.org/patch/1870231 works. So this change is
> needless.
>
> I had tested with below changes on my hardware.
>
> [1] https://patchwork.kernel.org/patch/1904431
> [2] https://patchwork.kernel.org/patch/1920661
>
> Best regards,
> Dongjin.
>
> On Mon, Dec 31, 2012 at 2:57 PM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
>> On 21 December 2012 09:11, Dongjin Kim <tobetter@gmail.com> wrote:
>>> This patch adds support for pin configuration using pinctrl subsystem to
>>> dw_mmc-exynos driver. The property 'wp-gpios' can be specified for write
>>> protect but 'samsung,cd-pinmux-gpio' and gpios used for clock, command and
>>> data lines will be ignored.
>>>
>>> -. 'pinctrl-0' should specify pin control groups (clock, comand and data
>>> lines) used for this controller.
>>> -. 'pinctrl-names' should contain only one value, 'default'.
>>>
>>> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
>>> ---
>>> drivers/mmc/host/dw_mmc-exynos.c | 44 ++++++++++++++++++++++++--------------
>>> 1 file changed, 28 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
>>> index 4d50da6..d1c9963 100644
>>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>>> @@ -16,6 +16,7 @@
>>> #include <linux/mmc/dw_mmc.h>
>>> #include <linux/of.h>
>>> #include <linux/of_gpio.h>
>>> +#include <linux/pinctrl/consumer.h>
>>>
>>> #include "dw_mmc.h"
>>> #include "dw_mmc-pltfm.h"
>>> @@ -49,6 +50,7 @@ struct dw_mci_exynos_priv_data {
>>> u8 ciu_div;
>>> u32 sdr_timing;
>>> u32 ddr_timing;
>>> + struct pinctrl *pctrl;
>>> };
>>>
>>> static struct dw_mci_exynos_compatible {
>>> @@ -84,6 +86,10 @@ static int dw_mci_exynos_priv_init(struct dw_mci *host)
>>> priv->ctrl_type = exynos_compat[idx].ctrl_type;
>>> }
>>>
>>> + priv->pctrl = devm_pinctrl_get_select_default(host->dev);
>>> + if (IS_ERR(priv->pctrl))
>>> + dev_dbg(host->dev, "no pinctrl node\n");
>>> +
>>
>> This could have been handled in dw_mci_exynos_setup_bus function. And
>> we also need to check if this patch gets merged.
>> https://patchwork.kernel.org/patch/1870231/. If it gets merged, this
>> change can be avoided.
>>
>>> host->priv = priv;
>>> return 0;
>>> }
>>> @@ -149,32 +155,19 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
>>> return ret;
>>>
>>> priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
>>> +
>>> return 0;
>>> }
>>>
>>> static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>>> struct device_node *slot_np, u8 bus_width)
>>> {
>>> + struct dw_mci_exynos_priv_data *priv = host->priv;
>>> int idx, gpio, ret;
>>>
>>> if (!slot_np)
>>> return -EINVAL;
>>>
>>> - /* cmd + clock + bus-width pins */
>>> - for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
>>> - gpio = of_get_gpio(slot_np, idx);
>>> - if (!gpio_is_valid(gpio)) {
>>> - dev_err(host->dev, "invalid gpio: %d\n", gpio);
>>> - return -EINVAL;
>>> - }
>>> -
>>> - ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
>>> - if (ret) {
>>> - dev_err(host->dev, "gpio [%d] request failed\n", gpio);
>>> - return -EBUSY;
>>> - }
>>> - }
>>> -
>>> gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
>>> if (gpio_is_valid(gpio)) {
>>> if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
>>> @@ -185,9 +178,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>>> host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
>>> }
>>>
>>> - if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
>>> + if (!IS_ERR(priv->pctrl))
>>> return 0;
>>>
>>> + if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
>>> + goto setup_bus;
>>> +
>>
>> Why do the entire bus setup if card detection is broken?
>>
>>> gpio = of_get_named_gpio(slot_np, "samsung,cd-pinmux-gpio", 0);
>>> if (gpio_is_valid(gpio)) {
>>> if (devm_gpio_request(host->dev, gpio, "dw-mci-cd"))
>>> @@ -196,6 +192,22 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>>> dev_info(host->dev, "cd gpio not available");
>>> }
>>>
>>> + setup_bus:
>>> + /* cmd + clock + bus-width pins */
>>> + for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
>>> + gpio = of_get_gpio(slot_np, idx);
>>> + if (!gpio_is_valid(gpio)) {
>>> + dev_err(host->dev, "invalid gpio: %d\n", gpio);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = devm_gpio_request(host->dev, gpio, "dw-mci-bus");
>>> + if (ret) {
>>> + dev_err(host->dev, "gpio [%d] request failed\n", gpio);
>>> + return -EBUSY;
>>> + }
>>> + }
>>
>> This change should not have been there. If the mmc bus setup is being
>> done using pinctrl framework, this change can be avoided.
>>
>> Thanks,
>> Thomas.
>>
>>> +
>>> return 0;
>>> }
>>>
>>> --
>>> 1.7.9.5
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-01-23 6:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-21 17:11 [PATCH] mmc: host: dw_mmc-exynos: Add support for pinctrl Dongjin Kim
2012-12-31 2:59 ` Dongjin Kim
2012-12-31 5:57 ` Thomas Abraham
2013-01-03 18:33 ` Dongjin Kim
2013-01-23 6:08 ` Dongjin Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).