[PATCH 0/3] ARM: davinci: hawk: fix mmc gpio declaration

[PATCH 0/3] ARM: davinci: hawk: fix mmc gpio declaration

[Axel Haslam]

The hawk board does not have a write protect gpio, so remove
it form the platform data. Also the card detect gpio is declared
on the wrong pin. fix it.

After fixing the platform data bugs, change to use a gpio descriptor,
so we can take advantage of the drivers recent updates, and have card
detect irq instead of polling.

DEPENDENCIES:
MMC: davinci: fix card detect and write protect
https://lkml.org/lkml/2016/11/15/592

Axel Haslam (3):
  ARM: davinci: hawk: fix mmc card detect gpio
  ARM: davinci: hawk: remove mmc ro pin
  ARM: davinci: hawk: use gpio descriptor for card detect

 arch/arm/mach-davinci/board-omapl138-hawk.c | 44 ++++++++---------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

-- 
2.9.3

[PATCH 1/3] ARM: davinci: hawk: fix mmc card detect gpio

[Axel Haslam]

The card detect gpio on the hawk board is gpio4_0 and not gpio3_12

Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
 arch/arm/mach-davinci/board-omapl138-hawk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index a4e8726..c0c24ba9 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -25,7 +25,7 @@
 #include <mach/mux.h>
 
 #define HAWKBOARD_PHY_ID		"davinci_mdio-0:07"
-#define DA850_HAWK_MMCSD_CD_PIN		GPIO_TO_PIN(3, 12)
+#define DA850_HAWK_MMCSD_CD_PIN		GPIO_TO_PIN(4, 0)
 #define DA850_HAWK_MMCSD_WP_PIN		GPIO_TO_PIN(3, 13)
 
 #define DA850_USB1_VBUS_PIN		GPIO_TO_PIN(2, 4)
@@ -119,7 +119,7 @@ static struct edma_rsv_info *da850_edma_rsv[2] = {
 static const short hawk_mmcsd0_pins[] = {
 	DA850_MMCSD0_DAT_0, DA850_MMCSD0_DAT_1, DA850_MMCSD0_DAT_2,
 	DA850_MMCSD0_DAT_3, DA850_MMCSD0_CLK, DA850_MMCSD0_CMD,
-	DA850_GPIO3_12, DA850_GPIO3_13,
+	DA850_GPIO4_0, DA850_GPIO3_13,
 	-1
 };
 
-- 
2.9.3

Re: [PATCH 1/3] ARM: davinci: hawk: fix mmc card detect gpio

[Sekhar Nori]

On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
> The card detect gpio on the hawk board is gpio4_0 and not gpio3_12
> 
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>

The LCDK and HawkBoard are different boards. The HawkBoard schematic
from eLinux.org page is broken, but looking for it on the net, I found
one and the MMC/SD CD pin in that schematic is indeed connected to GPIO3_12.

So I believe the original code is correct.

Thanks,
Sekhar

Re: [PATCH 1/3] ARM: davinci: hawk: fix mmc card detect gpio

[Axel Haslam]

On Tue, Nov 22, 2016 at 10:53 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
>> The card detect gpio on the hawk board is gpio4_0 and not gpio3_12
>>
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>
> The LCDK and HawkBoard are different boards. The HawkBoard schematic
> from eLinux.org page is broken, but looking for it on the net, I found
> one and the MMC/SD CD pin in that schematic is indeed connected to GPIO3_12.
>
> So I believe the original code is correct.

mmm, ok, the lcdk is booted using the hawk board file,
so we should differentiate the two boards?
or should we just live with inconsistencies on the lcdk?

(im guessing the same will be ture for the usb)

Regards
Axel.
>
> Thanks,
> Sekhar
>

Re: [PATCH 1/3] ARM: davinci: hawk: fix mmc card detect gpio

[Sekhar Nori]

On Tuesday 22 November 2016 03:40 PM, Axel Haslam wrote:
> On Tue, Nov 22, 2016 at 10:53 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>> On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
>>> The card detect gpio on the hawk board is gpio4_0 and not gpio3_12
>>>
>>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>>
>> The LCDK and HawkBoard are different boards. The HawkBoard schematic
>> from eLinux.org page is broken, but looking for it on the net, I found
>> one and the MMC/SD CD pin in that schematic is indeed connected to GPIO3_12.
>>
>> So I believe the original code is correct.
> 
> mmm, ok, the lcdk is booted using the hawk board file,

I think this is just a coincidence. The LCDK came after the hawkboard as
its replacement, and I guess there was some effort to derive out of
hawkboard design. But they are not 100% software compatible, like you
are discovering now.

> so we should differentiate the two boards?

Yes.

> or should we just live with inconsistencies on the lcdk?

Not sure what you mean by "live with inconsistencies on the lcdk". The
two boards need to be treated as close cousins, but independent boards.

Thanks,
Sekhar

[PATCH 2/3] ARM: davinci: hawk: remove mmc ro pin

[Axel Haslam]

The hawk board does not have a write protect pin to read the card
write protect status.

So remove un-needed platform data declaration for this gpio.

Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
 arch/arm/mach-davinci/board-omapl138-hawk.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index c0c24ba9..ddc312d 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -26,7 +26,6 @@
 
 #define HAWKBOARD_PHY_ID		"davinci_mdio-0:07"
 #define DA850_HAWK_MMCSD_CD_PIN		GPIO_TO_PIN(4, 0)
-#define DA850_HAWK_MMCSD_WP_PIN		GPIO_TO_PIN(3, 13)
 
 #define DA850_USB1_VBUS_PIN		GPIO_TO_PIN(2, 4)
 #define DA850_USB1_OC_PIN		GPIO_TO_PIN(6, 13)
@@ -119,22 +118,16 @@ static struct edma_rsv_info *da850_edma_rsv[2] = {
 static const short hawk_mmcsd0_pins[] = {
 	DA850_MMCSD0_DAT_0, DA850_MMCSD0_DAT_1, DA850_MMCSD0_DAT_2,
 	DA850_MMCSD0_DAT_3, DA850_MMCSD0_CLK, DA850_MMCSD0_CMD,
-	DA850_GPIO4_0, DA850_GPIO3_13,
+	DA850_GPIO4_0,
 	-1
 };
 
-static int da850_hawk_mmc_get_ro(int index)
-{
-	return gpio_get_value(DA850_HAWK_MMCSD_WP_PIN);
-}
-
 static int da850_hawk_mmc_get_cd(int index)
 {
 	return !gpio_get_value(DA850_HAWK_MMCSD_CD_PIN);
 }
 
 static struct davinci_mmc_config da850_mmc_config = {
-	.get_ro		= da850_hawk_mmc_get_ro,
 	.get_cd		= da850_hawk_mmc_get_cd,
 	.wires		= 4,
 	.max_freq	= 50000000,
@@ -159,14 +152,6 @@ static __init void omapl138_hawk_mmc_init(void)
 		return;
 	}
 
-	ret = gpio_request_one(DA850_HAWK_MMCSD_WP_PIN,
-			GPIOF_DIR_IN, "MMC WP");
-	if (ret < 0) {
-		pr_warn("%s: can not open GPIO %d\n",
-			__func__, DA850_HAWK_MMCSD_WP_PIN);
-		goto mmc_setup_wp_fail;
-	}
-
 	ret = da8xx_register_mmcsd0(&da850_mmc_config);
 	if (ret) {
 		pr_warn("%s: MMC/SD0 registration failed: %d\n", __func__, ret);
@@ -176,8 +161,6 @@ static __init void omapl138_hawk_mmc_init(void)
 	return;
 
 mmc_setup_mmcsd_fail:
-	gpio_free(DA850_HAWK_MMCSD_WP_PIN);
-mmc_setup_wp_fail:
 	gpio_free(DA850_HAWK_MMCSD_CD_PIN);
 }
 
-- 
2.9.3

Re: [PATCH 2/3] ARM: davinci: hawk: remove mmc ro pin

[Sekhar Nori]

On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
> The hawk board does not have a write protect pin to read the card
> write protect status.
> 
> So remove un-needed platform data declaration for this gpio.
> 
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>

The schematic I found has the SD_WP pin connected to GPIO3_13.

Thanks,
Sekhar

[PATCH 3/3] ARM: davinci: hawk: use gpio descriptor for card detect

[Axel Haslam]

Currently the mmc driver is polling the gpio to know if the
card was removed.

By using a gpio descriptor instead of the platform callbacks, the
driver will be able to register the gpio with the mmc core with API's
designed for this purpose.

This has the advantage that an irq will be registered,
and polling is no longer needed. Also, platform callbacks can be removed.

Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
 arch/arm/mach-davinci/board-omapl138-hawk.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index ddc312d..8e4f2102 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -15,6 +15,7 @@
 #include <linux/gpio.h>
 #include <linux/platform_data/gpio-davinci.h>
 #include <linux/regulator/machine.h>
+#include <linux/gpio/machine.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -25,7 +26,6 @@
 #include <mach/mux.h>
 
 #define HAWKBOARD_PHY_ID		"davinci_mdio-0:07"
-#define DA850_HAWK_MMCSD_CD_PIN		GPIO_TO_PIN(4, 0)
 
 #define DA850_USB1_VBUS_PIN		GPIO_TO_PIN(2, 4)
 #define DA850_USB1_OC_PIN		GPIO_TO_PIN(6, 13)
@@ -122,13 +122,16 @@ static const short hawk_mmcsd0_pins[] = {
 	-1
 };
 
-static int da850_hawk_mmc_get_cd(int index)
-{
-	return !gpio_get_value(DA850_HAWK_MMCSD_CD_PIN);
-}
+static struct gpiod_lookup_table mmc_gpios_table = {
+	.dev_id = "da830-mmc.0",
+	.table = {
+		/* gpio4_0: chip 2 contains gpio range 64-95 */
+		GPIO_LOOKUP("davinci_gpio.2", 0, "cd",
+				GPIO_ACTIVE_LOW),
+	},
+};
 
 static struct davinci_mmc_config da850_mmc_config = {
-	.get_cd		= da850_hawk_mmc_get_cd,
 	.wires		= 4,
 	.max_freq	= 50000000,
 	.caps		= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED,
@@ -144,13 +147,7 @@ static __init void omapl138_hawk_mmc_init(void)
 		return;
 	}
 
-	ret = gpio_request_one(DA850_HAWK_MMCSD_CD_PIN,
-			GPIOF_DIR_IN, "MMC CD");
-	if (ret < 0) {
-		pr_warn("%s: can not open GPIO %d\n",
-			__func__, DA850_HAWK_MMCSD_CD_PIN);
-		return;
-	}
+	gpiod_add_lookup_table(&mmc_gpios_table);
 
 	ret = da8xx_register_mmcsd0(&da850_mmc_config);
 	if (ret) {
@@ -161,7 +158,7 @@ static __init void omapl138_hawk_mmc_init(void)
 	return;
 
 mmc_setup_mmcsd_fail:
-	gpio_free(DA850_HAWK_MMCSD_CD_PIN);
+	gpiod_remove_lookup_table(&mmc_gpios_table);
 }
 
 static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id);
-- 
2.9.3

Re: [PATCH 3/3] ARM: davinci: hawk: use gpio descriptor for card detect

[Sekhar Nori]

On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
> Currently the mmc driver is polling the gpio to know if the
> card was removed.
> 
> By using a gpio descriptor instead of the platform callbacks, the
> driver will be able to register the gpio with the mmc core with API's
> designed for this purpose.
> 
> This has the advantage that an irq will be registered,
> and polling is no longer needed. Also, platform callbacks can be removed.
> 
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>

This patch looks good, provided it is not based of 1/3 and 2/3. There
are other boards in mach-davinci using the card detect and wp callbacks.
And some like board-dm365-evm.c have those pins routed through a CPLD.
So I guess there is more work to be done before platform callbacks can
completely be removed from MMC/SD driver. But the closer we get, the
better it is :)

Thanks,
Sekhar

Re: [PATCH 3/3] ARM: davinci: hawk: use gpio descriptor for card detect

[Axel Haslam]

On Tue, Nov 22, 2016 at 11:26 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
>> Currently the mmc driver is polling the gpio to know if the
>> card was removed.
>>
>> By using a gpio descriptor instead of the platform callbacks, the
>> driver will be able to register the gpio with the mmc core with API's
>> designed for this purpose.
>>
>> This has the advantage that an irq will be registered,
>> and polling is no longer needed. Also, platform callbacks can be removed.
>>
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>
> This patch looks good, provided it is not based of 1/3 and 2/3. There
> are other boards in mach-davinci using the card detect and wp callbacks.
> And some like board-dm365-evm.c have those pins routed through a CPLD.
> So I guess there is more work to be done before platform callbacks can
> completely be removed from MMC/SD driver. But the closer we get, the
> better it is :)
>

Yes, i saw the platform using CPLD for the pins. that might be tricky to remove.
we can flag the mmc "broken card detect" which would force polling on the pin.
im not sure if it is feasible to move the cpld pin handling to a
driver.. (maybe a
gpio driver?)


> Thanks,
> Sekhar
>