* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-09 5:59 ` Thomas Abraham
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Abraham @ 2013-04-09 5:59 UTC (permalink / raw)
To: linux-mmc
Cc: cjb, linux-samsung-soc, kgene.kim, girish.shivananjappa,
jh80.chung, tgih.jun, linux-arm-kernel, t.figa, heiko,
linus.walleij, dianders, patches
With device core now able to setup the default pin configuration,
the pin configuration code based on the deprecated Samsung specific
gpio bindings is removed.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes since v3:
- Rebased to the latest mmc-next branch, resolving the compilation
error with this patch due to changes in commit f2f942ce
"mmc: dw_mmc: Check return value of regulator_enable".
Thanks to Doug for pointing out this issue with the v3 patch.
drivers/mmc/host/dw_mmc-exynos.c | 38 --------------------------------------
drivers/mmc/host/dw_mmc.c | 14 +++-----------
drivers/mmc/host/dw_mmc.h | 3 ---
3 files changed, 3 insertions(+), 52 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index c7f0976..f013e7e 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -152,43 +152,6 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
return 0;
}
-static int dw_mci_exynos_setup_bus(struct dw_mci *host,
- struct device_node *slot_np, u8 bus_width)
-{
- 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;
- }
- }
-
- if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
- return 0;
-
- 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"))
- dev_err(host->dev, "gpio [%d] request failed\n", gpio);
- } else {
- dev_info(host->dev, "cd gpio not available");
- }
-
- return 0;
-}
-
/* Common capabilities of Exynos4/Exynos5 SoC */
static unsigned long exynos_dwmmc_caps[4] = {
MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR |
@@ -205,7 +168,6 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.prepare_command = dw_mci_exynos_prepare_command,
.set_ios = dw_mci_exynos_set_ios,
.parse_dt = dw_mci_exynos_parse_dt,
- .setup_bus = dw_mci_exynos_setup_bus,
};
static const struct of_device_id dw_mci_exynos_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 45d9216..5dcef43 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1952,14 +1952,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
else
bus_width = 1;
- if (drv_data && drv_data->setup_bus) {
- struct device_node *slot_np;
- slot_np = dw_mci_of_find_slot_node(host->dev, slot->id);
- ret = drv_data->setup_bus(host, slot_np, bus_width);
- if (ret)
- goto err_setup_bus;
- }
-
switch (bus_width) {
case 8:
mmc->caps |= MMC_CAP_8_BIT_DATA;
@@ -2002,7 +1994,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
if (ret) {
dev_err(host->dev,
"failed to enable regulator: %d\n", ret);
- goto err_setup_bus;
+ return ret;
}
}
@@ -2015,7 +2007,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
ret = mmc_add_host(mmc);
if (ret)
- goto err_setup_bus;
+ goto err_add_host;
#if defined(CONFIG_DEBUG_FS)
dw_mci_init_debugfs(slot);
@@ -2032,7 +2024,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
return 0;
-err_setup_bus:
+err_add_host:
mmc_free_host(mmc);
return -EINVAL;
}
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 53b8fd9..0b74189 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -190,7 +190,6 @@ extern int dw_mci_resume(struct dw_mci *host);
* @prepare_command: handle CMD register extensions.
* @set_ios: handle bus specific extensions.
* @parse_dt: parse implementation specific device tree properties.
- * @setup_bus: initialize io-interface
*
* Provide controller implementation specific extensions. The usage of this
* data structure is fully optional and usage of each member in this structure
@@ -203,7 +202,5 @@ struct dw_mci_drv_data {
void (*prepare_command)(struct dw_mci *host, u32 *cmdr);
void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
- int (*setup_bus)(struct dw_mci *host,
- struct device_node *slot_np, u8 bus_width);
};
#endif /* _DW_MMC_H_ */
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-09 5:59 ` Thomas Abraham
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Abraham @ 2013-04-09 5:59 UTC (permalink / raw)
To: linux-arm-kernel
With device core now able to setup the default pin configuration,
the pin configuration code based on the deprecated Samsung specific
gpio bindings is removed.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes since v3:
- Rebased to the latest mmc-next branch, resolving the compilation
error with this patch due to changes in commit f2f942ce
"mmc: dw_mmc: Check return value of regulator_enable".
Thanks to Doug for pointing out this issue with the v3 patch.
drivers/mmc/host/dw_mmc-exynos.c | 38 --------------------------------------
drivers/mmc/host/dw_mmc.c | 14 +++-----------
drivers/mmc/host/dw_mmc.h | 3 ---
3 files changed, 3 insertions(+), 52 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index c7f0976..f013e7e 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -152,43 +152,6 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
return 0;
}
-static int dw_mci_exynos_setup_bus(struct dw_mci *host,
- struct device_node *slot_np, u8 bus_width)
-{
- 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;
- }
- }
-
- if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
- return 0;
-
- 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"))
- dev_err(host->dev, "gpio [%d] request failed\n", gpio);
- } else {
- dev_info(host->dev, "cd gpio not available");
- }
-
- return 0;
-}
-
/* Common capabilities of Exynos4/Exynos5 SoC */
static unsigned long exynos_dwmmc_caps[4] = {
MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR |
@@ -205,7 +168,6 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.prepare_command = dw_mci_exynos_prepare_command,
.set_ios = dw_mci_exynos_set_ios,
.parse_dt = dw_mci_exynos_parse_dt,
- .setup_bus = dw_mci_exynos_setup_bus,
};
static const struct of_device_id dw_mci_exynos_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 45d9216..5dcef43 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1952,14 +1952,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
else
bus_width = 1;
- if (drv_data && drv_data->setup_bus) {
- struct device_node *slot_np;
- slot_np = dw_mci_of_find_slot_node(host->dev, slot->id);
- ret = drv_data->setup_bus(host, slot_np, bus_width);
- if (ret)
- goto err_setup_bus;
- }
-
switch (bus_width) {
case 8:
mmc->caps |= MMC_CAP_8_BIT_DATA;
@@ -2002,7 +1994,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
if (ret) {
dev_err(host->dev,
"failed to enable regulator: %d\n", ret);
- goto err_setup_bus;
+ return ret;
}
}
@@ -2015,7 +2007,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
ret = mmc_add_host(mmc);
if (ret)
- goto err_setup_bus;
+ goto err_add_host;
#if defined(CONFIG_DEBUG_FS)
dw_mci_init_debugfs(slot);
@@ -2032,7 +2024,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
return 0;
-err_setup_bus:
+err_add_host:
mmc_free_host(mmc);
return -EINVAL;
}
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 53b8fd9..0b74189 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -190,7 +190,6 @@ extern int dw_mci_resume(struct dw_mci *host);
* @prepare_command: handle CMD register extensions.
* @set_ios: handle bus specific extensions.
* @parse_dt: parse implementation specific device tree properties.
- * @setup_bus: initialize io-interface
*
* Provide controller implementation specific extensions. The usage of this
* data structure is fully optional and usage of each member in this structure
@@ -203,7 +202,5 @@ struct dw_mci_drv_data {
void (*prepare_command)(struct dw_mci *host, u32 *cmdr);
void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
- int (*setup_bus)(struct dw_mci *host,
- struct device_node *slot_np, u8 bus_width);
};
#endif /* _DW_MMC_H_ */
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
2013-04-09 5:59 ` Thomas Abraham
@ 2013-04-09 23:30 ` Doug Anderson
-1 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2013-04-09 23:30 UTC (permalink / raw)
To: Thomas Abraham
Cc: linux-mmc, Chris Ball, linux-samsung-soc, Kukjin Kim,
Girish Shivananjappa, Jaehoon Chung, Seungwon Jeon,
linux-arm-kernel, Tomasz Figa, Heiko Stübner, Linus Walleij,
patches
Thomas,
On Mon, Apr 8, 2013 at 10:59 PM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> @@ -2002,7 +1994,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> if (ret) {
> dev_err(host->dev,
> "failed to enable regulator: %d\n", ret);
> - goto err_setup_bus;
> + return ret;
It seems like you'd need a "mmc_free_host(mmc);" in this case don't
you? AKA: this should be a goto and not a return.
-Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-09 23:30 ` Doug Anderson
0 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2013-04-09 23:30 UTC (permalink / raw)
To: linux-arm-kernel
Thomas,
On Mon, Apr 8, 2013 at 10:59 PM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> @@ -2002,7 +1994,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> if (ret) {
> dev_err(host->dev,
> "failed to enable regulator: %d\n", ret);
> - goto err_setup_bus;
> + return ret;
It seems like you'd need a "mmc_free_host(mmc);" in this case don't
you? AKA: this should be a goto and not a return.
-Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
2013-04-09 23:30 ` Doug Anderson
@ 2013-04-10 12:48 ` Thomas Abraham
-1 siblings, 0 replies; 16+ messages in thread
From: Thomas Abraham @ 2013-04-10 12:48 UTC (permalink / raw)
To: Doug Anderson
Cc: linux-mmc, Chris Ball, linux-samsung-soc, Kukjin Kim,
Girish Shivananjappa, Jaehoon Chung, Seungwon Jeon,
linux-arm-kernel, Tomasz Figa, Heiko Stübner, Linus Walleij,
Patch Tracking
On 10 April 2013 05:00, Doug Anderson <dianders@chromium.org> wrote:
> Thomas,
>
> On Mon, Apr 8, 2013 at 10:59 PM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
>> @@ -2002,7 +1994,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>> if (ret) {
>> dev_err(host->dev,
>> "failed to enable regulator: %d\n", ret);
>> - goto err_setup_bus;
>> + return ret;
>
> It seems like you'd need a "mmc_free_host(mmc);" in this case don't
> you? AKA: this should be a goto and not a return.
Hi Doug,
The call to regulator_enable() is prior to the call to mmc_add_host().
Hence, call to mmc_fre_host is not required in this case. So the above
change should be right.
Thanks,
Thomas.
>
> -Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-10 12:48 ` Thomas Abraham
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Abraham @ 2013-04-10 12:48 UTC (permalink / raw)
To: linux-arm-kernel
On 10 April 2013 05:00, Doug Anderson <dianders@chromium.org> wrote:
> Thomas,
>
> On Mon, Apr 8, 2013 at 10:59 PM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
>> @@ -2002,7 +1994,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>> if (ret) {
>> dev_err(host->dev,
>> "failed to enable regulator: %d\n", ret);
>> - goto err_setup_bus;
>> + return ret;
>
> It seems like you'd need a "mmc_free_host(mmc);" in this case don't
> you? AKA: this should be a goto and not a return.
Hi Doug,
The call to regulator_enable() is prior to the call to mmc_add_host().
Hence, call to mmc_fre_host is not required in this case. So the above
change should be right.
Thanks,
Thomas.
>
> -Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
2013-04-10 12:48 ` Thomas Abraham
@ 2013-04-10 13:56 ` Doug Anderson
-1 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2013-04-10 13:56 UTC (permalink / raw)
To: Thomas Abraham
Cc: linux-mmc, Chris Ball, linux-samsung-soc, Kukjin Kim,
Girish Shivananjappa, Jaehoon Chung, Seungwon Jeon,
linux-arm-kernel, Tomasz Figa, Heiko Stübner, Linus Walleij,
Patch Tracking
Thomas,
On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> The call to regulator_enable() is prior to the call to mmc_add_host().
> Hence, call to mmc_fre_host is not required in this case. So the above
> change should be right.
Are you sure that mmc_free_host() is the opposite of mmc_add_host()
and not mmc_alloc_host()?
I'll double-check myself in a few hours when I'm in front of a computer.
-Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-10 13:56 ` Doug Anderson
0 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2013-04-10 13:56 UTC (permalink / raw)
To: linux-arm-kernel
Thomas,
On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> The call to regulator_enable() is prior to the call to mmc_add_host().
> Hence, call to mmc_fre_host is not required in this case. So the above
> change should be right.
Are you sure that mmc_free_host() is the opposite of mmc_add_host()
and not mmc_alloc_host()?
I'll double-check myself in a few hours when I'm in front of a computer.
-Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
2013-04-10 13:56 ` Doug Anderson
@ 2013-04-10 14:55 ` Doug Anderson
-1 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2013-04-10 14:55 UTC (permalink / raw)
To: Thomas Abraham
Cc: linux-mmc, Chris Ball, linux-samsung-soc, Kukjin Kim,
Girish Shivananjappa, Jaehoon Chung, Seungwon Jeon,
linux-arm-kernel, Tomasz Figa, Heiko Stübner, Linus Walleij,
Patch Tracking
Thomas,
On Wed, Apr 10, 2013 at 6:56 AM, Doug Anderson <dianders@chromium.org> wrote:
> On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
>> The call to regulator_enable() is prior to the call to mmc_add_host().
>> Hence, call to mmc_fre_host is not required in this case. So the above
>> change should be right.
>
> Are you sure that mmc_free_host() is the opposite of mmc_add_host()
> and not mmc_alloc_host()?
>
> I'll double-check myself in a few hours when I'm in front of a computer.
OK, I double checked and I'm still convinced that the free is needed
because we've already called mmc_alloc_host(). Current code always
makes sure to free when it returns an error and that seems right to
me.
-Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-10 14:55 ` Doug Anderson
0 siblings, 0 replies; 16+ messages in thread
From: Doug Anderson @ 2013-04-10 14:55 UTC (permalink / raw)
To: linux-arm-kernel
Thomas,
On Wed, Apr 10, 2013 at 6:56 AM, Doug Anderson <dianders@chromium.org> wrote:
> On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
>> The call to regulator_enable() is prior to the call to mmc_add_host().
>> Hence, call to mmc_fre_host is not required in this case. So the above
>> change should be right.
>
> Are you sure that mmc_free_host() is the opposite of mmc_add_host()
> and not mmc_alloc_host()?
>
> I'll double-check myself in a few hours when I'm in front of a computer.
OK, I double checked and I'm still convinced that the free is needed
because we've already called mmc_alloc_host(). Current code always
makes sure to free when it returns an error and that seems right to
me.
-Doug
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
2013-04-10 14:55 ` Doug Anderson
@ 2013-04-11 3:13 ` Seungwon Jeon
-1 siblings, 0 replies; 16+ messages in thread
From: Seungwon Jeon @ 2013-04-11 3:13 UTC (permalink / raw)
To: 'Doug Anderson', 'Thomas Abraham'
Cc: linux-mmc, 'Chris Ball', 'linux-samsung-soc',
'Kukjin Kim', 'Girish Shivananjappa',
'Jaehoon Chung', linux-arm-kernel, 'Tomasz Figa',
'Heiko Stübner', 'Linus Walleij',
'Patch Tracking'
On Wednesday, April 10, 2013, Doug Anderson wrote:
> Thomas,
>
> On Wed, Apr 10, 2013 at 6:56 AM, Doug Anderson <dianders@chromium.org> wrote:
> > On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
> > <thomas.abraham@linaro.org> wrote:
> >> The call to regulator_enable() is prior to the call to mmc_add_host().
> >> Hence, call to mmc_fre_host is not required in this case. So the above
> >> change should be right.
> >
> > Are you sure that mmc_free_host() is the opposite of mmc_add_host()
> > and not mmc_alloc_host()?
> >
> > I'll double-check myself in a few hours when I'm in front of a computer.
>
> OK, I double checked and I'm still convinced that the free is needed
> because we've already called mmc_alloc_host(). Current code always
> makes sure to free when it returns an error and that seems right to
> me.
mmc_alloc_host corresponds to mmc_free_host. And mmc_add_host matches
with mmc_remove_host. Let's focus on mmc_alloc_host.
mmc_alloc_host is called prior to regulator_enable.
So, if regulator_enable is failed, call of mmc_free_host makes sense.
Thanks,
Seungwon Jeon
>
> -Doug
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-11 3:13 ` Seungwon Jeon
0 siblings, 0 replies; 16+ messages in thread
From: Seungwon Jeon @ 2013-04-11 3:13 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday, April 10, 2013, Doug Anderson wrote:
> Thomas,
>
> On Wed, Apr 10, 2013 at 6:56 AM, Doug Anderson <dianders@chromium.org> wrote:
> > On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
> > <thomas.abraham@linaro.org> wrote:
> >> The call to regulator_enable() is prior to the call to mmc_add_host().
> >> Hence, call to mmc_fre_host is not required in this case. So the above
> >> change should be right.
> >
> > Are you sure that mmc_free_host() is the opposite of mmc_add_host()
> > and not mmc_alloc_host()?
> >
> > I'll double-check myself in a few hours when I'm in front of a computer.
>
> OK, I double checked and I'm still convinced that the free is needed
> because we've already called mmc_alloc_host(). Current code always
> makes sure to free when it returns an error and that seems right to
> me.
mmc_alloc_host corresponds to mmc_free_host. And mmc_add_host matches
with mmc_remove_host. Let's focus on mmc_alloc_host.
mmc_alloc_host is called prior to regulator_enable.
So, if regulator_enable is failed, call of mmc_free_host makes sense.
Thanks,
Seungwon Jeon
>
> -Doug
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
2013-04-11 3:13 ` Seungwon Jeon
@ 2013-04-11 12:06 ` Thomas Abraham
-1 siblings, 0 replies; 16+ messages in thread
From: Thomas Abraham @ 2013-04-11 12:06 UTC (permalink / raw)
To: Seungwon Jeon
Cc: Doug Anderson, linux-mmc, Chris Ball, linux-samsung-soc,
Kukjin Kim, Girish Shivananjappa, Jaehoon Chung, arm-linux,
Tomasz Figa, Heiko Stübner, Linus Walleij, Patch Tracking
On 11 April 2013 08:43, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> On Wednesday, April 10, 2013, Doug Anderson wrote:
>> Thomas,
>>
>> On Wed, Apr 10, 2013 at 6:56 AM, Doug Anderson <dianders@chromium.org> wrote:
>> > On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
>> > <thomas.abraham@linaro.org> wrote:
>> >> The call to regulator_enable() is prior to the call to mmc_add_host().
>> >> Hence, call to mmc_fre_host is not required in this case. So the above
>> >> change should be right.
>> >
>> > Are you sure that mmc_free_host() is the opposite of mmc_add_host()
>> > and not mmc_alloc_host()?
>> >
>> > I'll double-check myself in a few hours when I'm in front of a computer.
>>
>> OK, I double checked and I'm still convinced that the free is needed
>> because we've already called mmc_alloc_host(). Current code always
>> makes sure to free when it returns an error and that seems right to
>> me.
> mmc_alloc_host corresponds to mmc_free_host. And mmc_add_host matches
> with mmc_remove_host. Let's focus on mmc_alloc_host.
> mmc_alloc_host is called prior to regulator_enable.
> So, if regulator_enable is failed, call of mmc_free_host makes sense.
>
Hi Doug, Seungwon,
Ok, I got it wrong again. Thanks for letting me know. I will fix this
and send the v5 of this patch.
Thanks,
Thomas.
> Thanks,
> Seungwon Jeon
>>
>> -Doug
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-11 12:06 ` Thomas Abraham
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Abraham @ 2013-04-11 12:06 UTC (permalink / raw)
To: linux-arm-kernel
On 11 April 2013 08:43, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> On Wednesday, April 10, 2013, Doug Anderson wrote:
>> Thomas,
>>
>> On Wed, Apr 10, 2013 at 6:56 AM, Doug Anderson <dianders@chromium.org> wrote:
>> > On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
>> > <thomas.abraham@linaro.org> wrote:
>> >> The call to regulator_enable() is prior to the call to mmc_add_host().
>> >> Hence, call to mmc_fre_host is not required in this case. So the above
>> >> change should be right.
>> >
>> > Are you sure that mmc_free_host() is the opposite of mmc_add_host()
>> > and not mmc_alloc_host()?
>> >
>> > I'll double-check myself in a few hours when I'm in front of a computer.
>>
>> OK, I double checked and I'm still convinced that the free is needed
>> because we've already called mmc_alloc_host(). Current code always
>> makes sure to free when it returns an error and that seems right to
>> me.
> mmc_alloc_host corresponds to mmc_free_host. And mmc_add_host matches
> with mmc_remove_host. Let's focus on mmc_alloc_host.
> mmc_alloc_host is called prior to regulator_enable.
> So, if regulator_enable is failed, call of mmc_free_host makes sense.
>
Hi Doug, Seungwon,
Ok, I got it wrong again. Thanks for letting me know. I will fix this
and send the v5 of this patch.
Thanks,
Thomas.
> Thanks,
> Seungwon Jeon
>>
>> -Doug
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
2013-04-10 13:56 ` Doug Anderson
@ 2013-04-19 10:11 ` Russell King - ARM Linux
-1 siblings, 0 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2013-04-19 10:11 UTC (permalink / raw)
To: Doug Anderson
Cc: Thomas Abraham, linux-samsung-soc, Heiko Stübner,
Patch Tracking, Seungwon Jeon, Tomasz Figa, linux-mmc,
Jaehoon Chung, Kukjin Kim, Girish Shivananjappa, Chris Ball,
Linus Walleij, linux-arm-kernel
On Wed, Apr 10, 2013 at 06:56:48AM -0700, Doug Anderson wrote:
> Thomas,
>
> On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
> > The call to regulator_enable() is prior to the call to mmc_add_host().
> > Hence, call to mmc_fre_host is not required in this case. So the above
> > change should be right.
>
> Are you sure that mmc_free_host() is the opposite of mmc_add_host()
> and not mmc_alloc_host()?
mmc_free_host() undoes mmc_alloc_host(). mmc_remove_host() undoes
mmc_add_host().
alloc
add
remove
free
is pretty standard terminology, standard ordering. If add fails, then
free is the right thing to call to clean up after the alloc.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration
@ 2013-04-19 10:11 ` Russell King - ARM Linux
0 siblings, 0 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2013-04-19 10:11 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 10, 2013 at 06:56:48AM -0700, Doug Anderson wrote:
> Thomas,
>
> On Wed, Apr 10, 2013 at 5:48 AM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
> > The call to regulator_enable() is prior to the call to mmc_add_host().
> > Hence, call to mmc_fre_host is not required in this case. So the above
> > change should be right.
>
> Are you sure that mmc_free_host() is the opposite of mmc_add_host()
> and not mmc_alloc_host()?
mmc_free_host() undoes mmc_alloc_host(). mmc_remove_host() undoes
mmc_add_host().
alloc
add
remove
free
is pretty standard terminology, standard ordering. If add fails, then
free is the right thing to call to clean up after the alloc.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-04-19 10:11 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-09 5:59 [PATCH v4] mmc: dw_mmc: let device core setup the default pin configuration Thomas Abraham
2013-04-09 5:59 ` Thomas Abraham
2013-04-09 23:30 ` Doug Anderson
2013-04-09 23:30 ` Doug Anderson
2013-04-10 12:48 ` Thomas Abraham
2013-04-10 12:48 ` Thomas Abraham
2013-04-10 13:56 ` Doug Anderson
2013-04-10 13:56 ` Doug Anderson
2013-04-10 14:55 ` Doug Anderson
2013-04-10 14:55 ` Doug Anderson
2013-04-11 3:13 ` Seungwon Jeon
2013-04-11 3:13 ` Seungwon Jeon
2013-04-11 12:06 ` Thomas Abraham
2013-04-11 12:06 ` Thomas Abraham
2013-04-19 10:11 ` Russell King - ARM Linux
2013-04-19 10:11 ` Russell King - ARM Linux
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.