[PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Sachin Kamat]

Added compatibility string for Exynos4412 SoC.

Cc: Thomas Abraham <thomas.abraham@linaro.org>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/mmc/host/dw_mmc-exynos.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 72fd0f2..f15b4d3 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -209,6 +209,8 @@ static const struct dw_mci_drv_data exynos5250_drv_data = {
 };
 
 static const struct of_device_id dw_mci_exynos_match[] = {
+	{ .compatible = "samsung,exynos4412-dw-mshc",
+			.data = &exynos5250_drv_data, },
 	{ .compatible = "samsung,exynos5250-dw-mshc",
 			.data = &exynos5250_drv_data, },
 	{},
-- 
1.7.4.1

[PATCH 2/2] mmc: dw_mmc: exynos: Add pinctrl support

[Sachin Kamat]

Added support for pin configuration using pinctrl subsystem.

Cc: Thomas Abraham <thomas.abraham@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/mmc/host/dw_mmc-exynos.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index f15b4d3..31f5f47 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -11,11 +11,13 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/mmc/host.h>
 #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"
@@ -36,6 +38,8 @@
 #define EXYNOS4210_FIXED_CIU_CLK_DIV	2
 #define EXYNOS4412_FIXED_CIU_CLK_DIV	4
 
+static struct pinctrl	*pctrl;
+
 /* Variations in Exynos specific dw-mshc controller */
 enum dw_mci_exynos_type {
 	DW_MCI_TYPE_EXYNOS4210,
@@ -160,6 +164,10 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
 	if (!slot_np)
 		return -EINVAL;
 
+	pctrl = devm_pinctrl_get_select_default(host->dev);
+	if (!IS_ERR(pctrl))
+		return 0;
+
 	/* cmd + clock + bus-width pins */
 	for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
 		gpio = of_get_gpio(slot_np, idx);
-- 
1.7.4.1

Re: [PATCH 2/2] mmc: dw_mmc: exynos: Add pinctrl support

[Linus Walleij]

On Thu, Feb 7, 2013 at 12:57 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:

> Added support for pin configuration using pinctrl subsystem.
>
> Cc: Thomas Abraham <thomas.abraham@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

NACK.

In the v3.9 merge window we will merge this patch:
http://marc.info/?l=linux-kernel&m=135887740715083&w=2

That should do the trick. Just define your tables properly.

Yours,
Linus Walleij

Re: [PATCH 2/2] mmc: dw_mmc: exynos: Add pinctrl support

[Sachin Kamat]

On 7 February 2013 21:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Feb 7, 2013 at 12:57 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>
>> Added support for pin configuration using pinctrl subsystem.
>>
>
> NACK.
>
> In the v3.9 merge window we will merge this patch:
> http://marc.info/?l=linux-kernel&m=135887740715083&w=2
>
> That should do the trick. Just define your tables properly.
>

Just a small clarification needed in this regard.
On systems that do not use pinctrl and need to configure the gpios, we
used to check the return value of devm_pinctrl_get_select_default()
for error and then parse the dt file for gpios and configure them if
needed. Now your latest patch "drivers/pinctrl: grab default handles
from device core" suggests not to use boilerplate code as it will
handle pinctrl configuration based on dt entries for it. So in the
case like dw_mmc (in my patch above), please suggest best way to avoid
parsing gpios from dt file in case where we have pinctrl defined.

The following 3 lines (with +) wouldn't be required according to the
patch and hence 'for' loop would get executed (which I want to avoid
as the system supports pinctrl in this case).

+       pctrl = devm_pinctrl_get_select_default(host->dev);
+       if (!IS_ERR(pctrl))
+               return 0;
+
        /* cmd + clock + bus-width pins */
        for (idx = 0; idx < NUM_PINS(bus_width); idx++) {
                gpio = of_get_gpio(slot_np, idx);

-- 
With warm regards,
Sachin

Re: [PATCH 2/2] mmc: dw_mmc: exynos: Add pinctrl support

[Linus Walleij]

On Mon, Feb 25, 2013 at 5:39 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:

> Just a small clarification needed in this regard.
> On systems that do not use pinctrl and need to configure the gpios, we
> used to check the return value of devm_pinctrl_get_select_default()
> for error and then parse the dt file for gpios and configure them if
> needed.

The idea of pinctrl_get_select_default() is (as you could guess
from it's name) to get a pin ctrl handle and select the default state.

The return code from that function tells you whether that was
successful or not.

It does *NOT* tell you to go and look up GPIO values if it wasn't!

Then the function would be named
bool pinctrl_is_used_on_this_system().

Please review the recent conversation I had with Tomasz and
Heiko on this, especially this mail:
http://marc.info/?l=linux-kernel&m=136166647121345&w=2

Yours,
Linus Walleij

Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Sachin Kamat]

On 07/02/2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Added compatibility string for Exynos4412 SoC.
>
> Cc: Thomas Abraham <thomas.abraham@linaro.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Any comments on this patch?

> ---
>  drivers/mmc/host/dw_mmc-exynos.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c
> b/drivers/mmc/host/dw_mmc-exynos.c
> index 72fd0f2..f15b4d3 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -209,6 +209,8 @@ static const struct dw_mci_drv_data exynos5250_drv_data
> = {
>  };
>
>  static const struct of_device_id dw_mci_exynos_match[] = {
> +	{ .compatible = "samsung,exynos4412-dw-mshc",
> +			.data = &exynos5250_drv_data, },
>  	{ .compatible = "samsung,exynos5250-dw-mshc",
>  			.data = &exynos5250_drv_data, },
>  	{},
> --
> 1.7.4.1
>
>


-- 
With warm regards,
Sachin

Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Alim Akhtar]

Hi Sachin,

On Thu, Feb 14, 2013 at 8:55 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> On 07/02/2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> Added compatibility string for Exynos4412 SoC.
>>
>> Cc: Thomas Abraham <thomas.abraham@linaro.org>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>
> Any comments on this patch?
>
>> ---
>>  drivers/mmc/host/dw_mmc-exynos.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c
>> b/drivers/mmc/host/dw_mmc-exynos.c
>> index 72fd0f2..f15b4d3 100644
>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> @@ -209,6 +209,8 @@ static const struct dw_mci_drv_data exynos5250_drv_data
>> = {
>>  };
>>
>>  static const struct of_device_id dw_mci_exynos_match[] = {
>> +     { .compatible = "samsung,exynos4412-dw-mshc",
>> +                     .data = &exynos5250_drv_data, },
>>       { .compatible = "samsung,exynos5250-dw-mshc",
>>                       .data = &exynos5250_drv_data, },
>>       {},

Looks like dw_mci_drv_data is a generic enough to be used for all
exynos SoCs including exynos4210. My thought is, if 4412 and 5250 is
using the same drv_data(please check 4210 once), let rename them as
exynos. In that case we still have different compatible name and not
so __confusing__ .data field.
I am ok if maintainers suggested something else/better here.
>>
>> 1.7.4.1
>>
>>
>
>
> --
> With warm regards,
> Sachin
> --
> 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
-- 
Regards,
Alim

Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Jaehoon Chung]

On 02/15/2013 02:09 PM, Alim Akhtar wrote:
> Hi Sachin,
> 
> On Thu, Feb 14, 2013 at 8:55 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> On 07/02/2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>> Added compatibility string for Exynos4412 SoC.
>>>
>>> Cc: Thomas Abraham <thomas.abraham@linaro.org>
>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>
>> Any comments on this patch?
>>
>>> ---
>>>  drivers/mmc/host/dw_mmc-exynos.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c
>>> b/drivers/mmc/host/dw_mmc-exynos.c
>>> index 72fd0f2..f15b4d3 100644
>>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>>> @@ -209,6 +209,8 @@ static const struct dw_mci_drv_data exynos5250_drv_data
>>> = {
>>>  };
>>>
>>>  static const struct of_device_id dw_mci_exynos_match[] = {
>>> +     { .compatible = "samsung,exynos4412-dw-mshc",
>>> +                     .data = &exynos5250_drv_data, },
>>>       { .compatible = "samsung,exynos5250-dw-mshc",
>>>                       .data = &exynos5250_drv_data, },
>>>       {},
> 
> Looks like dw_mci_drv_data is a generic enough to be used for all
> exynos SoCs including exynos4210. My thought is, if 4412 and 5250 is
> using the same drv_data(please check 4210 once), let rename them as
> exynos. In that case we still have different compatible name and not
> so __confusing__ .data field.
I agreed Alim's opinion. Actually, i mentioned this point at the Dongin's patch.
If using same drv_data, i would prefer to use "&exynos_drv_data" instead of &exynos5250_drv_data.

This patch could be separated into two patches. like this "compatibility for exynos4412 SOC" and "rename the drv_data"

Best Regards,
Jaehoon Chung
> I am ok if maintainers suggested something else/better here.
>>>
>>> 1.7.4.1
>>>
>>>
>>
>>
>> --
>> With warm regards,
>> Sachin
>> --
>> 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

Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Sachin Kamat]

On 15/02/2013, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 02/15/2013 02:09 PM, Alim Akhtar wrote:
>> Hi Sachin,
>>
>> On Thu, Feb 14, 2013 at 8:55 PM, Sachin Kamat <sachin.kamat@linaro.org>
>> wrote:
>>> On 07/02/2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>>> Added compatibility string for Exynos4412 SoC.
>>>>
>>>> Cc: Thomas Abraham <thomas.abraham@linaro.org>
>>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>>
>>> Any comments on this patch?
>>>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc-exynos.c |    2 ++
>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c
>>>> b/drivers/mmc/host/dw_mmc-exynos.c
>>>> index 72fd0f2..f15b4d3 100644
>>>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>>>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>>>> @@ -209,6 +209,8 @@ static const struct dw_mci_drv_data
>>>> exynos5250_drv_data
>>>> = {
>>>>  };
>>>>
>>>>  static const struct of_device_id dw_mci_exynos_match[] = {
>>>> +     { .compatible = "samsung,exynos4412-dw-mshc",
>>>> +                     .data = &exynos5250_drv_data, },
>>>>       { .compatible = "samsung,exynos5250-dw-mshc",
>>>>                       .data = &exynos5250_drv_data, },
>>>>       {},
>>
>> Looks like dw_mci_drv_data is a generic enough to be used for all
>> exynos SoCs including exynos4210. My thought is, if 4412 and 5250 is
>> using the same drv_data(please check 4210 once), let rename them as
>> exynos. In that case we still have different compatible name and not
>> so __confusing__ .data field.
> I agreed Alim's opinion. Actually, i mentioned this point at the Dongin's
> patch.
> If using same drv_data, i would prefer to use "&exynos_drv_data" instead of
> &exynos5250_drv_data.
>
> This patch could be separated into two patches. like this "compatibility for
> exynos4412 SOC" and "rename the drv_data"
>

If the driver data is same for all exynos socs, then it makes sense to
name it just exynos. I will update and resend accordingly.

> Best Regards,
> Jaehoon Chung
>> I am ok if maintainers suggested something else/better here.
>>>>
>>>> 1.7.4.1
>>>>
>>>>
>>>
>>>
>>> --
>>> With warm regards,
>>> Sachin
>>> --
>>> 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
>
>


-- 
With warm regards,
Sachin

Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Sachin Kamat]

>>> On 07/02/2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>>> Added compatibility string for Exynos4412 SoC.
>>>>
> If using same drv_data, i would prefer to use "&exynos_drv_data" instead of &exynos5250_drv_data.
>
> This patch could be separated into two patches. like this "compatibility for exynos4412 SOC" and "rename the drv_data"

Came across this patch from Dongjin Kim [1] who has already attempetd
the above suggestion. Hence dropping my patch.

Dongjin,
Are you still working on this patch [1] as this has not yet been
accepted or merged?

[1] http://lkml.indiana.edu/hypermail/linux/kernel/1301.3/00450.html

-- 
With warm regards,
Sachin

Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Dongjin Kim]

Hello Sachin,

I have one more patch which use '&exynos_drv_data' for its drv_data,
but not submitted yet.
I will send it soon anyway, anything else do you like to point out on
the patch? :)

Regards,
Dongjin.

On Mon, Feb 18, 2013 at 5:46 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>>> On 07/02/2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>>>> Added compatibility string for Exynos4412 SoC.
>>>>>
>> If using same drv_data, i would prefer to use "&exynos_drv_data" instead of &exynos5250_drv_data.
>>
>> This patch could be separated into two patches. like this "compatibility for exynos4412 SOC" and "rename the drv_data"
>
> Came across this patch from Dongjin Kim [1] who has already attempetd
> the above suggestion. Hence dropping my patch.
>
> Dongjin,
> Are you still working on this patch [1] as this has not yet been
> accepted or merged?
>
> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1301.3/00450.html
>
> --
> With warm regards,
> Sachin

Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC

[Sachin Kamat]

Hi Dongjin,


On 18 February 2013 14:34, Dongjin Kim <tobetter@gmail.com> wrote:
> Hello Sachin,
>
> I have one more patch which use '&exynos_drv_data' for its drv_data,
> but not submitted yet.
> I will send it soon anyway, anything else do you like to point out on
> the patch? :)

Please verify if the capabilities of Exynos 4412 and 5 are same? Other
comments are already provided by Jaehoon and Alim.

-- 
With warm regards,
Sachin