[PATCH] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel

[PATCH] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel

[Guennadi Liakhovetski]

When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
also card eject events will have to be detected by the platform.
This patch prepares mackerel to this switch.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 arch/arm/mach-shmobile/board-mackerel.c |   68 +++++++++++++++++++++++++++----
 1 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 1b30195..05f5fe7 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -45,6 +45,7 @@
 #include <linux/tca6416_keypad.h>
 #include <linux/usb/r8a66597.h>
 #include <linux/usb/renesas_usbhs.h>
+#include <linux/workqueue.h>
 
 #include <video/sh_mobile_hdmi.h>
 #include <video/sh_mobile_lcdc.h>
@@ -990,7 +991,7 @@ static struct platform_device fsi_ak4643_device = {
 
 /*
  * The card detect pin of the top SD/MMC slot (CN7) is active low and is
- * connected to GPIO A22 of SH7372 (GPIO_PORT41).
+ * connected to GPIO A22 of SH7372 (GPIO_PORT41 / IRQ8).
  */
 static int slot_cn7_get_cd(struct platform_device *pdev)
 {
@@ -998,12 +999,49 @@ static int slot_cn7_get_cd(struct platform_device *pdev)
 }
 
 /* SDHI0 */
-static irqreturn_t mackerel_sdhi0_gpio_cd(int irq, void *arg)
+struct sdhi_card_detect {
+	struct delayed_work work;
+	struct sh_mobile_sdhi_info *info;
+	int gpio_irq;
+	int gpio_port;
+	int gpio_cd;
+};
+
+static void sdhi_cd_work(struct work_struct *work)
+{
+	struct sdhi_card_detect *cd = container_of(work, struct sdhi_card_detect, work.work);
+	int ret;
+
+	if (cd->gpio_cd >= 0)
+		gpio_free(cd->gpio_cd);
+	ret = gpio_request(cd->gpio_port, NULL);
+	if (!ret) {
+		gpio_direction_input(cd->gpio_port);
+		ret = gpio_get_value(cd->gpio_port);
+		gpio_free(cd->gpio_port);
+		if (ret)
+			/* No card */
+			irq_set_irq_type(cd->gpio_irq, IRQ_TYPE_EDGE_FALLING);
+		else
+			/* Card in the slot */
+			irq_set_irq_type(cd->gpio_irq, IRQ_TYPE_EDGE_RISING);
+	}
+	if (cd->gpio_cd >= 0)
+		gpio_request(cd->gpio_cd, NULL);
+}
+
+static irqreturn_t mackerel_sdhi_gpio_cd(int irq, void *arg)
 {
-	struct device *dev = arg;
-	struct sh_mobile_sdhi_info *info = dev->platform_data;
+	struct sdhi_card_detect *cd = arg;
+	struct sh_mobile_sdhi_info *info = cd->info;
 	struct tmio_mmc_data *pdata = info->pdata;
 
+	if (irq != cd->gpio_irq)
+		return IRQ_NONE;
+
+	irq_set_irq_type(irq, IRQ_TYPE_NONE);
+
+	schedule_delayed_work(&cd->work, msecs_to_jiffies(200));
 	tmio_mmc_cd_wakeup(pdata);
 
 	return IRQ_HANDLED;
@@ -1015,6 +1053,14 @@ static struct sh_mobile_sdhi_info sdhi0_info = {
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 };
 
+static struct sdhi_card_detect sdhi0_cd = {
+	.work		= __DELAYED_WORK_INITIALIZER(sdhi0_cd.work, sdhi_cd_work),
+	.info		= &sdhi0_info,
+	.gpio_irq	= evt2irq(0x3340),
+	.gpio_port	= GPIO_PORT172,
+	.gpio_cd	= GPIO_FN_SDHICD0,
+};
+
 static struct resource sdhi0_resources[] = {
 	[0] = {
 		.name	= "SDHI0",
@@ -1092,7 +1138,7 @@ static struct platform_device sdhi1_device = {
 
 /*
  * The card detect pin of the top SD/MMC slot (CN23) is active low and is
- * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162).
+ * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162 / IRQ0).
  */
 static int slot_cn23_get_cd(struct platform_device *pdev)
 {
@@ -1500,12 +1546,18 @@ static void __init mackerel_init(void)
 	gpio_request(GPIO_FN_SDHID0_1, NULL);
 	gpio_request(GPIO_FN_SDHID0_0, NULL);
 
-	ret = request_irq(evt2irq(0x3340), mackerel_sdhi0_gpio_cd,
-			  IRQF_TRIGGER_FALLING, "sdhi0 cd", &sdhi0_device.dev);
+	/*
+	 * If the driver probes with a card plugged in, the native SDHICD0 IRQ
+	 * will trigger, when the runtime PM brings the interface up, and the
+	 * card will be detected. This interrupt is needed if there is no card
+	 * during probing and runtime PM turns the interface power off.
+	 */
+	ret = request_irq(sdhi0_cd.gpio_irq, mackerel_sdhi_gpio_cd,
+			  IRQF_TRIGGER_FALLING, "sdhi0 cd", &sdhi0_cd);
 	if (!ret)
 		sdhi0_info.tmio_flags |= TMIO_MMC_HAS_COLD_CD;
 	else
-		pr_err("Cannot get IRQ #%d: %d\n", evt2irq(0x3340), ret);
+		pr_err("Cannot get IRQ #%d: %d\n", sdhi0_cd.gpio_irq, ret);
 
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 	/* enable SDHI1 */
-- 
1.7.2.5

Re: [PATCH] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel

[Magnus Damm]

On Thu, Jul 14, 2011 at 8:25 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
> also card eject events will have to be detected by the platform.
> This patch prepares mackerel to this switch.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  arch/arm/mach-shmobile/board-mackerel.c |   68 +++++++++++++++++++++++++++----
>  1 files changed, 60 insertions(+), 8 deletions(-)

Thanks for your work on this.

What is the reason why we can't use GPIO for all cases of insert/eject
event detection? Switching between GPIO and function mode for the pin
seems overly complex IMO - that unless there is a real win behind it.
Please explain.

Thank you!

/ magnus

Re: [PATCH] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel

[Guennadi Liakhovetski]

Hi Magnus

On Thu, 14 Jul 2011, Magnus Damm wrote:

> On Thu, Jul 14, 2011 at 8:25 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
> > also card eject events will have to be detected by the platform.
> > This patch prepares mackerel to this switch.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  arch/arm/mach-shmobile/board-mackerel.c |   68 +++++++++++++++++++++++++++----
> >  1 files changed, 60 insertions(+), 8 deletions(-)
> 
> Thanks for your work on this.
> 
> What is the reason why we can't use GPIO for all cases of insert/eject
> event detection? Switching between GPIO and function mode for the pin
> seems overly complex IMO - that unless there is a real win behind it.
> Please explain.

Because the way, board-supported (cold) card-detection is currently 
implemented on mackerel is, that on a card slot event a pm_runtime_get() 
is issued, which then calls the .runtime_resume() hook in the tmio driver. 
However, if the runtime PM is inactive, if the hook doesn't get called, 
e.g., bacause the power domain, the respective SDHI controller is in, is 
held on by other devices, the event would be missed. That's why the normal 
pin state is the SDHI card-detection function, which then works performs 
the detection in such cases.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 0/2] use both-edge GPIO interrupt for SDHI0 on mackerel

[Guennadi Liakhovetski]

As correctly pointed out by Magnus, IRQ-capable GPIOs on sh7372 can 
produce interrupts on both edges. These patches extend the intc driver and 
use this for SDHI0 hotplug detection on mackerel.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 1/2] sh-mobile: enable both edges GPIO interrupts on sh7372

[Guennadi Liakhovetski]

From: Magnus Damm <damm@opensource.se>

IRQ-capable GPIOs on sh7372 can be configured to produce interrupts on
both edges.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <damm@opensource.se>
---
 drivers/sh/intc/chip.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/sh/intc/chip.c b/drivers/sh/intc/chip.c
index f33e2dd..33b2ed4 100644
--- a/drivers/sh/intc/chip.c
+++ b/drivers/sh/intc/chip.c
@@ -186,6 +186,9 @@ static unsigned char intc_irq_sense_table[IRQ_TYPE_SENSE_MASK + 1] = {
     !defined(CONFIG_CPU_SUBTYPE_SH7709)
 	[IRQ_TYPE_LEVEL_HIGH] = VALID(3),
 #endif
+#if defined(CONFIG_ARCH_SH7372)
+	[IRQ_TYPE_EDGE_BOTH] = VALID(4),
+#endif
 };
 
 static int intc_set_type(struct irq_data *data, unsigned int type)
-- 
1.7.2.5

Re: [PATCH 1/2] sh-mobile: enable both edges GPIO interrupts on sh7372

[Magnus Damm]

On Fri, Jul 15, 2011 at 7:58 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> From: Magnus Damm <damm@opensource.se>
>
> IRQ-capable GPIOs on sh7372 can be configured to produce interrupts on
> both edges.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <damm@opensource.se>
> ---
>  drivers/sh/intc/chip.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Magnus Damm <damm@opensource.se>

Re: [PATCH 1/2] sh-mobile: enable both edges GPIO interrupts on sh7372

[Paul Mundt]

On Fri, Jul 15, 2011 at 08:14:23PM +0900, Magnus Damm wrote:
> On Fri, Jul 15, 2011 at 7:58 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > From: Magnus Damm <damm@opensource.se>
> >
> > IRQ-capable GPIOs on sh7372 can be configured to produce interrupts on
> > both edges.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <damm@opensource.se>
> > ---
> > ?drivers/sh/intc/chip.c | ? ?3 +++
> > ?1 files changed, 3 insertions(+), 0 deletions(-)
> 
> Acked-by: Magnus Damm <damm@opensource.se>

Applied, thanks.

[PATCH 2/2 v2] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel

[Guennadi Liakhovetski]

When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
also card eject events will have to be detected by the platform.
This patch prepares mackerel to this switch.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v2: reconfiguring the pin for card insertion and ejection is no longer 
needed.

 arch/arm/mach-shmobile/board-mackerel.c |   48 +++++++++++++++++++++++++-----
 1 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 1b30195..9addaae 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -45,6 +45,7 @@
 #include <linux/tca6416_keypad.h>
 #include <linux/usb/r8a66597.h>
 #include <linux/usb/renesas_usbhs.h>
+#include <linux/workqueue.h>
 
 #include <video/sh_mobile_hdmi.h>
 #include <video/sh_mobile_lcdc.h>
@@ -990,7 +991,7 @@ static struct platform_device fsi_ak4643_device = {
 
 /*
  * The card detect pin of the top SD/MMC slot (CN7) is active low and is
- * connected to GPIO A22 of SH7372 (GPIO_PORT41).
+ * connected to GPIO A22 of SH7372 (GPIO_PORT41 / IRQ8).
  */
 static int slot_cn7_get_cd(struct platform_device *pdev)
 {
@@ -998,12 +999,30 @@ static int slot_cn7_get_cd(struct platform_device *pdev)
 }
 
 /* SDHI0 */
-static irqreturn_t mackerel_sdhi0_gpio_cd(int irq, void *arg)
+struct sdhi_card_detect {
+	struct delayed_work work;
+	struct sh_mobile_sdhi_info *info;
+	int gpio_irq;
+};
+
+static void sdhi_cd_work(struct work_struct *work)
+{
+	struct sdhi_card_detect *cd = container_of(work, struct sdhi_card_detect, work.work);
+	irq_set_irq_type(cd->gpio_irq, IRQ_TYPE_EDGE_BOTH);
+}
+
+static irqreturn_t mackerel_sdhi_gpio_cd(int irq, void *arg)
 {
-	struct device *dev = arg;
-	struct sh_mobile_sdhi_info *info = dev->platform_data;
+	struct sdhi_card_detect *cd = arg;
+	struct sh_mobile_sdhi_info *info = cd->info;
 	struct tmio_mmc_data *pdata = info->pdata;
 
+	if (irq != cd->gpio_irq)
+		return IRQ_NONE;
+
+	irq_set_irq_type(irq, IRQ_TYPE_NONE);
+
+	schedule_delayed_work(&cd->work, msecs_to_jiffies(200));
 	tmio_mmc_cd_wakeup(pdata);
 
 	return IRQ_HANDLED;
@@ -1015,6 +1034,12 @@ static struct sh_mobile_sdhi_info sdhi0_info = {
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 };
 
+static struct sdhi_card_detect sdhi0_cd = {
+	.work		= __DELAYED_WORK_INITIALIZER(sdhi0_cd.work, sdhi_cd_work),
+	.info		= &sdhi0_info,
+	.gpio_irq	= evt2irq(0x3340),
+};
+
 static struct resource sdhi0_resources[] = {
 	[0] = {
 		.name	= "SDHI0",
@@ -1092,7 +1117,7 @@ static struct platform_device sdhi1_device = {
 
 /*
  * The card detect pin of the top SD/MMC slot (CN23) is active low and is
- * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162).
+ * connected to GPIO SCIFB_SCK of SH7372 (GPIO_PORT162 / IRQ0).
  */
 static int slot_cn23_get_cd(struct platform_device *pdev)
 {
@@ -1500,12 +1525,19 @@ static void __init mackerel_init(void)
 	gpio_request(GPIO_FN_SDHID0_1, NULL);
 	gpio_request(GPIO_FN_SDHID0_0, NULL);
 
-	ret = request_irq(evt2irq(0x3340), mackerel_sdhi0_gpio_cd,
-			  IRQF_TRIGGER_FALLING, "sdhi0 cd", &sdhi0_device.dev);
+	/*
+	 * If the driver probes with a card plugged in, the native SDHICD0 IRQ
+	 * will trigger, when the runtime PM brings the interface up, and the
+	 * card will be detected. This interrupt is needed if there is no card
+	 * during probing and runtime PM turns the interface power off.
+	 */
+	ret = request_irq(sdhi0_cd.gpio_irq, mackerel_sdhi_gpio_cd,
+			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			  "sdhi0 cd", &sdhi0_cd);
 	if (!ret)
 		sdhi0_info.tmio_flags |= TMIO_MMC_HAS_COLD_CD;
 	else
-		pr_err("Cannot get IRQ #%d: %d\n", evt2irq(0x3340), ret);
+		pr_err("Cannot get IRQ #%d: %d\n", sdhi0_cd.gpio_irq, ret);
 
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 	/* enable SDHI1 */
-- 
1.7.2.5

Re: [PATCH 2/2 v2] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel

[Magnus Damm]

On Fri, Jul 15, 2011 at 7:59 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
> also card eject events will have to be detected by the platform.
> This patch prepares mackerel to this switch.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> v2: reconfiguring the pin for card insertion and ejection is no longer
> needed.

Hi Guennadi,

Thanks for your work on this. I may be reading the code wrong, or
perhaps my stack of patches is not the same as yours, but I can't see
any code dealing with hotplug for SDHI2? It looks to me that your
patch is updating the comment about PORT162 and IRQ0 but does nothing
about it on the software side. I sort of assume that we want to have
hotplug support via IRQ on all SDHI channels if possible. But maybe
that's not possible?

Cheers,

/ magnus

Re: [PATCH 2/2 v2] ARM: mach-shmobile: use GPIO interrupt also for card eject on SDHI0 on mackerel

[Guennadi Liakhovetski]

On Fri, 15 Jul 2011, Magnus Damm wrote:

> On Fri, Jul 15, 2011 at 7:59 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > When we switch to transaction-based runtime PM on SDHI / TMIO MMC,
> > also card eject events will have to be detected by the platform.
> > This patch prepares mackerel to this switch.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >
> > v2: reconfiguring the pin for card insertion and ejection is no longer
> > needed.
> 
> Hi Guennadi,
> 
> Thanks for your work on this. I may be reading the code wrong, or
> perhaps my stack of patches is not the same as yours, but I can't see
> any code dealing with hotplug for SDHI2? It looks to me that your
> patch is updating the comment about PORT162 and IRQ0 but does nothing
> about it on the software side. I sort of assume that we want to have
> hotplug support via IRQ on all SDHI channels if possible. But maybe
> that's not possible?

Maybe;-) I've spent quite a bit of time trying to make SDHI2 work 
according to the same scheme, but so far I failed. With a kernel, patched 
to produce GPIO interrupts from SDHI2 card insertion, as soon as I insert 
a card the board locks up hard without a single character on any of the 
consoles... So, I'm afraid, we have to leave SDHI2 alone for now.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/