Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
On Wed, Jun 19, 2013 at 7:21 AM, Ulf Hansson ulf.hans...@linaro.org wrote: It seems like a bad idea that an insertion of an SD card should trigger the display to be light up. That is indirectly in principle what you suggest should happen from user space once a new SD card is found. Right? Most likely what will happen is the system will mount the sdcard, and if necessary start the media scanner so that the user can see their media on the sd card when they turn the screen on. But that is mostly irrelevant, the point is that the event needs to be passed to userspace to allow it to make the decision in a timely fashion. I have been working with Android for several years, we never used this kind of setup. Instead we wait for the user to press the display on button. At that time the confirmation will be received. Not saying that this is the only way of doing it, but it seems to be an accepted solution for all our customers. This patch is ported from the Android common tree, so you've probably been using it. I agree to that this patch should have negligible impact though - if we get things right. I will try to review the code in more detail soon. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
On Tue, Jun 18, 2013 at 6:17 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 17 June 2013 20:33, Colin Cross ccr...@android.com wrote: This is a generic requirement for using a kernel with autosleep enabled. Autosleep will enter suspend whenever there is no wakeup source/wakelock held. Consider the following sequence: Kernel is suspended Card is inserted, triggering a wakeup interrupt, which is an implicit wakeup source until it is handled I don't think a card insert/remove irq need to be configured as a wakeup interrupt. As you say, it will force a resume to detect the card, but for what reason? Instead, I think it it better to leave the card detection to be handled at the next resume, thus not resuming the system when not needed. That decision is going to be different on each device. On Android devices it has been configured as a wakeup interrupt. This patch is necessary on devices that want to handle the event as a wakeup event, and has negligible impact on those that do not. Kernel starts resuming, resumes the mmc driver The mmc driver enables its interrupt, which is immediately handled and queues an event to be handled by userspace At this point the wakeup interrupt is handled and gone, and no wakeup sources are being held, so the kernel can choose to go back to suspend, so userspace can't handle the insertion event until the kernel wakes up for another reason. Is this a problem? From my point of view it should be perfectly acceptable to let userspace handle the event at the next resume/wakeup instead. Don't you think so? Depends what userspace is doing. If userspace would like to provide the user some feedback on the event, then it has to be a wakeup interrupt, and it has to hold a wakelock until it has passed the event to userspace. In general, an event that is triggered by a wakeup interrupt that is being passed from the kernel to userspace needs to have a wakeup source held while the event is queued. That's sounds reasonable. Would it then make sense to hold a generic wakeup source in the suspend/resume core, once a wakeup interrupt is fetched? No, the suspend/resume core can only hold a wakeup source until it has handed the event off to the driver, at which point it is the driver's responsibility to hold a wakeup source. The suspend/resume core cannot tell if the event was handled by the driver or will be passed to userspace. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
On Mon, Jun 17, 2013 at 7:22 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 14 June 2013 22:52, Colin Cross ccr...@android.com wrote: On Fri, Jun 14, 2013 at 11:42 AM, Zoran Markovic zoran.marko...@linaro.org wrote: I am not sure I understand why this patch is needed. When a new card is inserted/removed and the upper levels gets notification about the new card, triggering the mounting/un-mounting of the file system, why should it be the lowest layer (mmc) that prevents the platform from enter suspend/sleep? Why do we need to prevent it at all? Note that notifier handling in mmc_pm_notify, was if I remember correctly, not completely developed when the original version of this patch was being discussed. mmc_pm_notify prevents cards from being inserted/removed in the middle of suspend-resume sequence, is that not enough? I will try to speak on behalf of the original implementers in a hope they would provide the original motivation for the patch. My understanding is that any preemption in the procedure could be an opportunity to suspend, as there may be a suspend request racing with this code. This is why the calls to __pm_stay_awake() and queue_delayed_work() are so tightly coupled. It would be up to the delayed work procedure (mmc_rescan()) to decide whether or not it is safe to suspend. If there are no changes in the MMC state or all changes can be handled by mmc_rescan(), it is safe to call __pm_relax(). Otherwise, userland may take over processing of this event, and this is why the awake state needs to be extended by 1/2 second. The __pm_stay_awake() is required to prevent autosleep during the time between the card detect interrupt and when the userspace process that gets the notification runs. The 1/2 second delay is used because it is easier than trying to detect when the userspace process has received the notification, at which time it should hold its own wakelock and the mmc subsystem can call __pm_relax(). Hi Colin, I don't have the in-depth knowledge about how the userspace deamons handles the event notifications, so please bare with me while I am trying to understand more here. First of all, are we trying to solve an issue here or just improving some specific situation, that is not clear to me. I might have misunderstood this patch, but it seems like your concern is that you believe the event notification can get lost - if userspace are about to trigger a suspend while a card is being inserted/removed. If that is the case, could you elaborate on what level the notification can get lost? Kind regards Ulf Hansson This is a generic requirement for using a kernel with autosleep enabled. Autosleep will enter suspend whenever there is no wakeup source/wakelock held. Consider the following sequence: Kernel is suspended Card is inserted, triggering a wakeup interrupt, which is an implicit wakeup source until it is handled Kernel starts resuming, resumes the mmc driver The mmc driver enables its interrupt, which is immediately handled and queues an event to be handled by userspace At this point the wakeup interrupt is handled and gone, and no wakeup sources are being held, so the kernel can choose to go back to suspend, so userspace can't handle the insertion event until the kernel wakes up for another reason. In general, an event that is triggered by a wakeup interrupt that is being passed from the kernel to userspace needs to have a wakeup source held while the event is queued. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
On Fri, Jun 14, 2013 at 11:42 AM, Zoran Markovic zoran.marko...@linaro.org wrote: I am not sure I understand why this patch is needed. When a new card is inserted/removed and the upper levels gets notification about the new card, triggering the mounting/un-mounting of the file system, why should it be the lowest layer (mmc) that prevents the platform from enter suspend/sleep? Why do we need to prevent it at all? Note that notifier handling in mmc_pm_notify, was if I remember correctly, not completely developed when the original version of this patch was being discussed. mmc_pm_notify prevents cards from being inserted/removed in the middle of suspend-resume sequence, is that not enough? I will try to speak on behalf of the original implementers in a hope they would provide the original motivation for the patch. My understanding is that any preemption in the procedure could be an opportunity to suspend, as there may be a suspend request racing with this code. This is why the calls to __pm_stay_awake() and queue_delayed_work() are so tightly coupled. It would be up to the delayed work procedure (mmc_rescan()) to decide whether or not it is safe to suspend. If there are no changes in the MMC state or all changes can be handled by mmc_rescan(), it is safe to call __pm_relax(). Otherwise, userland may take over processing of this event, and this is why the awake state needs to be extended by 1/2 second. The __pm_stay_awake() is required to prevent autosleep during the time between the card detect interrupt and when the userspace process that gets the notification runs. The 1/2 second delay is used because it is easier than trying to detect when the userspace process has received the notification, at which time it should hold its own wakelock and the mmc subsystem can call __pm_relax(). -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: subtract boot sectors from disk size for eMMC 4.3+ devices
On Thu, Apr 28, 2011 at 3:33 AM, Linus Walleij linus.wall...@linaro.org wrote: 2011/3/5 Linus Walleij linus.wall...@stericsson.com: This patch is a squash of patches from Gary King and Ulf Hansson done in Android trees, hopefully fixing the issue properly. The csd sector count reported by eMMC 4.3+ cards includes the boot partition size; subtract this from the size reported to the disk since the boot partition is inaccessible. I'm trying to get somewhere with this. AFAICT reading the spec it does not say anything about the boot sectors being subtracted from the pool of available sectors, i.e. either you set these sectors aside permanently or you do not support the feature. So the behaviour fixed in this patch should be a per-card quirk, not generic. Then this: + /* size is in 256K chunks, i.e. 512 sectors each */ + boot_sectors = ext_csd[EXT_CSD_BOOT_SIZE_MULTI] * 512; This is not what the spec says. It states that the size is in 128KB chunks, so the correct code shoul be: boot_sectors = ext_csd[EXT_CSD_BOOT_SIZE_MULTI] * 256; Any comments on this? I guess it's been working, the patch prent in the Android kernels simply remove twice as many sectors as required from the card sector pool :-/ This patch was dropped from the Android tree, it is clearly not correct for all eMMC devices. We have worked around the problem on the affected by letting the bootloader tell us where it put the GPT near the end of the device, in which case the actual size of the device becomes irrelevant. I'm not convinced that any eMMC chip has this problem, it may have just been confusion on the part of nVidia caused by their bootloader's logical block addressing system, where the boot sectors are addressed as 0, and the regular area starts at sizeof(boot sector). The off-by-two error may have been caused by the affected eMMC having two boot sectors. I would suggest forgetting about this patch unless someone can prove that there is a device that fails to write at the end. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] mmc_block: Allow more than 8 partitions per card
On Tue, Apr 26, 2011 at 6:22 AM, Arnd Bergmann a...@arndb.de wrote: On Saturday 23 April 2011, John Stultz wrote: From: Colin Cross ccr...@android.com Set the GENHD_FL_EXT_DEVT flag, which will allocate minor numbers in major 259 for partitions past disk-minors. Also remove the use of disk_devt to determine devidx from md-disk. md-disk-first_minor is always initialized from devidx and can always be used to recover it. CC: Chris Ball c...@laptop.org CC: Arnd Bergmann a...@arndb.de CC: Dima Zavin d...@android.com Signed-off-by: Colin Cross ccr...@android.com Signed-off-by: John Stultz john.stu...@linaro.org The new code looks reasonable, but wouldn't changing this be incompatible with existing root file systems that contain static device nodes? I don't think so. Without this change, /dev/mmcblk0p1 will be (179, 1), /dev/mmcblk0p7 will be (179, 7), and /dev/mmcblk0p8 will be dropped. After this change, /dev/mmcblk0p1-7 will be the same, but /dev/mmcblk0p8 will be (259, random number). A root file system with static inodes will still have access to partitions 1-7, and will still not have access to dynamically-assigned partition 8. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc_block: Allow more than 8 partitions per card
Set the GENHD_FL_EXT_DEVT flag, which will allocate minor numbers
in major 259 for partitions past disk-minors.
Also remove the use of disk_devt to determine devidx from md-disk.
md-disk-first_minor is always initialized from devidx and can
always be used to recover it.
Signed-off-by: Colin Cross ccr...@android.com
---
drivers/mmc/card/block.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index d545f79..07d8eb0 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -87,11 +87,7 @@ static void mmc_blk_put(struct mmc_blk_data *md)
mutex_lock(open_lock);
md-usage--;
if (md-usage == 0) {
- int devmaj = MAJOR(disk_devt(md-disk));
- int devidx = MINOR(disk_devt(md-disk)) MMC_SHIFT;
-
- if (!devmaj)
- devidx = md-disk-first_minor MMC_SHIFT;
+ int devidx = md-disk-first_minor MMC_SHIFT;
blk_cleanup_queue(md-queue.queue);
@@ -607,6 +603,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card
*card)
md-disk-private_data = md;
md-disk-queue = md-queue.queue;
md-disk-driverfs_dev = card-dev;
+ md-disk-flags = GENHD_FL_EXT_DEVT;
/*
* As discussed on lkml, GENHD_FL_REMOVABLE should:
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] SDHCI: 8-bit data transfer width support
These patches are not coming from the tegra/for-next branch, they are
coming from Grant's devicetree-next branch. Grant, why are these
patches in your tree, and why is tegra/for-next in your tree? It's
going to cause conflicts when we rebase our for-next branch. Please
remove tegra and this sdhci patch from your tree.
As for the mmc patch, it seems to have been over-squashed to contain
the changes to the common code and the tegra sdhci driver. Now that
tegra is in for-next, we'll be cleaning up and sending subsystem
drivers for review. Expect a patch within a week or two.
On Mon, Jun 28, 2010 at 11:39 AM, Andrew Morton
a...@linux-foundation.org wrote:
On Sat, 12 Jun 2010 14:45:02 +0900
Kyungmin Park kyungmin.p...@samsung.com wrote:
From: Kyungmin Park kyungmin.p...@samsung.com
Some host constroller such as s5pc110 has WIDE8 support feature.
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/mmc/host/sdhci.c | 5 +
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 142419c..6cf018a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1159,6 +1159,11 @@ static void sdhci_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+ if (ios-bus_width == MMC_BUS_WIDTH_8)
+ ctrl |= SDHCI_CTRL_8BITBUS;
+ else
+ ctrl = ~SDHCI_CTRL_8BITBUS;
+
if (ios-bus_width == MMC_BUS_WIDTH_4)
ctrl |= SDHCI_CTRL_4BITBUS;
else
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..eb5efe0 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -72,6 +72,7 @@
#define SDHCI_CTRL_ADMA1 0x08
#define SDHCI_CTRL_ADMA32 0x10
#define SDHCI_CTRL_ADMA64 0x18
+#define SDHCI_CTRL_8BITBUS 0x20
#define SDHCI_POWER_CONTROL 0x29
#define SDHCI_POWER_ON 0x01
This change (or something similar) also seems to have been lumped into
the unchangelogged, unreviewed mmc: sdhci: Initial Tegra sdhci driver
patch. So again, I'll drop your patch and if mmc: sdhci: Initial
Tegra sdhci driver doesn't get merged, your patch will be lost.
I wonder what else it does. Here it is:
commit feed6702dc4bb130869171cbd8167637ea13c33c
Author: Colin Cross ccr...@android.com
AuthorDate: Wed Mar 10 20:42:35 2010 -0800
Commit: Grant Likely grant.lik...@secretlab.ca
CommitDate: Fri Jun 25 09:47:58 2010 -0600
mmc: sdhci: Initial Tegra sdhci driver
Signed-off-by: Colin Cross ccr...@android.com
[Olof: Fixed up merge conflicts]
Signed-off-by: Olof Johansson o...@lixom.net
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
diff --git a/arch/arm/mach-tegra/include/mach/sdhci.h
b/arch/arm/mach-tegra/include/mach/sdhci.h
new file mode 100644
index 000..34e2686
--- /dev/null
+++ b/arch/arm/mach-tegra/include/mach/sdhci.h
@@ -0,0 +1,33 @@
+/*
+ * include/asm-arm/arch-tegra/sdhci.h
+ *
+ * Copyright (C) 2009 Palm, Inc.
+ * Author: Yvonne Yip y...@palm.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __ASM_ARM_ARCH_TEGRA_SDHCI_H
+#define __ASM_ARM_ARCH_TEGRA_SDHCI_H
+
+#include linux/mmc/host.h
+
+struct tegra_sdhci_platform_data {
+ const char *clk_id;
+ int force_hs;
+ int cd_gpio;
+ int wp_gpio;
+ int power_gpio;
+
+ void (*board_probe)(int id, struct mmc_host *);
+ void (*board_remove)(int id, struct mmc_host *);
+};
+
+#endif
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index b9dee28..85c473e 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -229,11 +229,13 @@ static int sdio_enable_hs(struct mmc_card *card)
int ret;
u8 speed;
- if (!(card-host-caps MMC_CAP_SD_HIGHSPEED))
- return 0;
+ if (!(card-host-caps MMC_CAP_FORCE_HS)) {
+ if (!(card-host-caps MMC_CAP_SD_HIGHSPEED))
+ return 0;
- if (!card-cccr.high_speed)
- return 0;
+ if (!card-cccr.high_speed)
+ return 0;
+ }
ret = mmc_io_rw_direct(card, 0, 0, SDIO_CCCR_SPEED, 0, speed);
if (ret)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index f06d06e..357c294 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -382,6 +382,12 @@ config MMC_TMIO
