Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

[Tadeusz Gozdek]

It solves the problems with clock in the PXA255/270 MMC/SD hardware.
This solution was testes with our devices and works good.
Kernel 2.6.24.4 (but should work with >=2.6.19)

Regards
Tadeusz Gozdek
 
diff -NuarBb /x/work/navi_os/mmc/host/pxamci.c mmc/host/pxamci.c
--- /x/work/navi_os/mmc/host/pxamci.c	2008-03-24 19:49:18.000000000 +0100
+++ mmc/host/pxamci.c	2008-04-25 12:14:00.000000000 +0200
@@ -26,6 +26,7 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
 
 #include <asm/dma.h>
 #include <asm/io.h>
@@ -67,6 +68,8 @@
 	unsigned int		dma_dir;
 };
 
+static int local_cmd = -1;
+
 static void pxamci_stop_clock(struct pxamci_host *host)
 {
 	if (readl(host->base + MMC_STAT) & STAT_CLK_EN) {
@@ -84,6 +87,7 @@
 
 		if (v & STAT_CLK_EN)
 			dev_err(mmc_dev(host->mmc), "unable to stop clock\n");
+		local_cmd = -1;
 	}
 }
 
@@ -232,6 +235,14 @@
 		v = w2;
 	}
 
+	local_cmd = cmd->opcode;
+	if ((local_cmd == MMC_GO_IDLE_STATE) ||
+	    (local_cmd == MMC_GO_INACTIVE_STATE) ||
+	    (local_cmd == MMC_SEND_STATUS) ||
+	    (local_cmd == MMC_STOP_TRANSMISSION) ||
+	    (local_cmd == MMC_SET_BLOCKLEN)
+	    ) pxamci_stop_clock(host);
+
 	if (stat & STAT_TIME_OUT_RESPONSE) {
 		cmd->error = -ETIMEDOUT;
 	} else if (stat & STAT_RES_CRC_ERR && cmd->flags & MMC_RSP_CRC) {
@@ -290,7 +301,7 @@
 
 	host->data = NULL;
 	if (host->mrq->stop) {
-		pxamci_stop_clock(host);
+		//pxamci_stop_clock(host);
 		pxamci_start_cmd(host, host->mrq->stop, host->cmdat);
 	} else {
 		pxamci_finish_request(host, host->mrq);

Re: Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

[Pierre Ossman]

On Mon, 28 Apr 2008 09:32:49 +0200
Tadeusz Gozdek <linux_fan@o2.pl> wrote:

> It solves the problems with clock in the PXA255/270 MMC/SD hardware.
> This solution was testes with our devices and works good.
> Kernel 2.6.24.4 (but should work with >=2.6.19)
> 

What issue is this supposed to solve?

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: Re: Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

[Tadeusz Gozdek]

Hi
As you can check, after finishing all operations clock is leaved as switched on (its switched off and on again during sending next command e.g. GO_INACTIVE_STATE). The same is after wake-up the system, even if the clock was switched off. 
Switched on clock has sometimes an influence on the rest of the hardware e.g. the GPS modules and on battery life in a battery powered systems.

Tadeusz

> On Mon, 28 Apr 2008 09:32:49 +0200
> Tadeusz Gozdek <linux_fan@o2.pl> wrote:
> 
> > It solves the problems with clock in the PXA255/270 MMC/SD hardware.
> > This solution was testes with our devices and works good.
> > Kernel 2.6.24.4 (but should work with >=2.6.19)
> > 
> 
> What issue is this supposed to solve?
> 
> Rgds
> 
> -- 
>      -- Pierre Ossman
> 
>   Linux kernel, MMC maintainer        http://www.kernel.org
>   PulseAudio, core developer          http://pulseaudio.org
>   rdesktop, core developer          http://www.rdesktop.org
---- Wiadomość Oryginalna ----
Od: Pierre Ossman <drzeus-list@drzeus.cx>
Do: Tadeusz Gozdek <linux_fan@o2.pl>
Kopia do: linux-kernel@vger.kernel.org, Marcel Holtmann <marcel@holtmann.org>
Data: 28 kwietnia 2008 15:28
Temat: Re: Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

> On Mon, 28 Apr 2008 09:32:49 +0200
> Tadeusz Gozdek <linux_fan@o2.pl> wrote:
> 
> > It solves the problems with clock in the PXA255/270 MMC/SD hardware.
> > This solution was testes with our devices and works good.
> > Kernel 2.6.24.4 (but should work with >=2.6.19)
> > 
> 
> What issue is this supposed to solve?
> 
> Rgds
> 
> -- 
>      -- Pierre Ossman
> 
>   Linux kernel, MMC maintainer        http://www.kernel.org
>   PulseAudio, core developer          http://pulseaudio.org
>   rdesktop, core developer          http://www.rdesktop.org

Re: Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

[Pierre Ossman]

On Tue, 29 Apr 2008 08:49:17 +0200
Tadeusz Gozdek <linux_fan@o2.pl> wrote:

> Hi
> As you can check, after finishing all operations clock is leaved as switched on (its switched off and on again during sending next command e.g. GO_INACTIVE_STATE). The same is after wake-up the system, even if the clock was switched off. 
> Switched on clock has sometimes an influence on the rest of the hardware e.g. the GPS modules and on battery life in a battery powered systems.
> 

That is a policy with possible side-effects (i.e. needs to be
configurable), and host independent so it should be in the core. So I'm
afraid I have to NAK your patch.

As a side-note, the PXA driver shouldn't be fiddling with the clock the
way it does right now in the first place, but I have no maintainer to
yell at. Patches removing that behaviour are very welcome.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: Re: Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

[Tadeusz Gozdek]

OK probably I didn't understand your policy. I ever thought that the host controller should know, when the clock should be off.
The PXA controller is different and it was the simples (but effective) way to solve this difference.

So, I hope it will be usable for people which has problem with "always on" clock, until you solve it in different way (if you going to do this).

Thanks 
Tadeusz

> That is a policy with possible side-effects (i.e. needs to be
> configurable), and host independent so it should be in the core. So I'm
> afraid I have to NAK your patch.
> 
> As a side-note, the PXA driver shouldn't be fiddling with the clock the
> way it does right now in the first place, but I have no maintainer to
> yell at. Patches removing that behaviour are very welcome.

Re: Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

[Pierre Ossman]

On Tue, 29 Apr 2008 12:30:44 +0200
Tadeusz Gozdek <linux_fan@o2.pl> wrote:

> OK probably I didn't understand your policy. I ever thought that the host controller should know, when the clock should be off.

No, it's not because it's not my policy, but because the kernel tries
to avoid handling "policy". :)

It is common to talk about software that implements "policy" or
"mechanism". "mechanism" is simple the act of performing a given, well
defined task (e.g. turn on LED A). "policy" on the other hand is
selecting an action when there are multiple, equally valid actions.
Usually this involves the user, directly or indirectly.

The kernel tries to stay clear of "policy" as it is a complex problem
that is better solved per system, in user space. So what I meant when I
said that this is policy, I was saying that this is a "policy" kind of
decision that needs to be configurable from user space.

> The PXA controller is different and it was the simples (but effective) way to solve this difference.

Different in what regard? Or are you referring to the already present
clock fudging in the driver?

> 
> So, I hope it will be usable for people which has problem with "always on" clock, until you solve it in different way (if you going to do this).
> 

This is a project that has been popping up now and then, but the big
thing that is lacking is someone who is able to actually measure if
there is a power gain from this. I don't want to add a lot of code that
isn't doing any good, so I want hard numbers.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: Re: Fwd: [PATCH] [MMC] fix clock problem in PXA255/270

[Tadeusz Gozdek]

> Different in what regard? Or are you referring to the already present
> clock fudging in the driver?
I mean already present clock... but it looks only I have problem with this using Linux. 
I've checked other OS with the same board and it switch off the SD clock after complete the data transfer and switching the SD to idle mode.

> This is a project that has been popping up now and then, but the big
> thing that is lacking is someone who is able to actually measure if
> there is a power gain from this. I don't want to add a lot of code that
> isn't doing any good, so I want hard numbers.
OK OK. I understand. :)
I check some devices with the SD and Linux inside and all of them had modified driver which is able to switch off the clock :)
Thus the "persistent clock" problem exist, but nobody say about it :)

Thank you for all explanations.