[PATCH 0/3] Fix fixed regulators support
Hello, I've noticed that the voltage check in regulator_is_supported_voltage() function was inverted, what resulted in strange side effects. One of such side effects appeared in sdhci driver, which has some build-in support for special case of fixed regulators. Now, after fixing regulator_is_supported_voltage(), the sdhci driver stopped working with fixed regulators. The provided patch series fixes regulator_is_supported_voltage() function and updates sdhci driver to correctly operate with fixed voltage regulators. The second patch unifies handling of fixed regulators and regulators with disabled voltage change due to their constraints. This restores support for such regulators in sdhci driver (such case is present on Samsung GONI board). If possible, I would recomment to push those patches to v3.7-rc series. Best regards Marek Szyprowski Samsung Poland RD Center Patch summary: Marek Szyprowski (3): regulator: fix voltage check in regulator_is_supported_voltage() regulator: threat regulators with constant volatage as fixed mmc: sdhci: apply voltage range check only for non-fixed regulators drivers/mmc/host/sdhci.c |2 +- drivers/regulator/core.c |7 +-- 2 files changed, 6 insertions(+), 3 deletions(-) -- 1.7.9.5 -- 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 3/3] mmc: sdhci: apply voltage range check only for non-fixed regulators
Fixed regulators cannot change their voltage, so disable all voltage range
checking for them, otherwise the driver fails to operate with fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
--
1.7.9.5
--
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 1/3] regulator: fix voltage check in regulator_is_supported_voltage()
regulator_is_supported_voltage() should return true only if the voltage
of fixed/constant regulator is between min_uV and max_uV.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/regulator/core.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1a35251..042c1ff 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1974,7 +1974,7 @@ int regulator_is_supported_voltage(struct regulator
*regulator,
if (!(rdev-constraints-valid_ops_mask REGULATOR_CHANGE_VOLTAGE)) {
ret = regulator_get_voltage(regulator);
if (ret = 0)
- return (min_uV = ret ret = max_uV);
+ return (min_uV = ret ret = max_uV);
else
return ret;
}
--
1.7.9.5
--
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 2/3] regulator: threat regulators with constant volatage as fixed
Some drivers has additional logic for fixed regulators. Let regulator core
to threat regulators which cannot change their voltage due to applied
constraints as fixed.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/regulator/core.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 042c1ff..271182e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1872,7 +1872,10 @@ int regulator_count_voltages(struct regulator *regulator)
{
struct regulator_dev*rdev = regulator-rdev;
- return rdev-desc-n_voltages ? : -EINVAL;
+ if (rdev-constraints-valid_ops_mask REGULATOR_CHANGE_VOLTAGE)
+ return rdev-desc-n_voltages ? : -EINVAL;
+ else
+ return 1;
}
EXPORT_SYMBOL_GPL(regulator_count_voltages);
--
1.7.9.5
--
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/3] regulator: threat regulators with constant volatage as fixed
On Tue, Nov 13, 2012 at 09:48:52AM +0100, Marek Szyprowski wrote: Some drivers has additional logic for fixed regulators. Let regulator core to threat regulators which cannot change their voltage due to applied YM treat. + if (rdev-constraints-valid_ops_mask REGULATOR_CHANGE_VOLTAGE) + return rdev-desc-n_voltages ? : -EINVAL; Please don't perpetuate the use of ? : as it's not a triumph of legibility (even worse than the regular ternery operator). I realise that the original code did this but there's no need to carry on doing the same thing. signature.asc Description: Digital signature
Re: [PATCH 1/3] regulator: fix voltage check in regulator_is_supported_voltage()
On Tue, Nov 13, 2012 at 09:48:51AM +0100, Marek Szyprowski wrote: regulator_is_supported_voltage() should return true only if the voltage of fixed/constant regulator is between min_uV and max_uV. Applied, thanks. signature.asc Description: Digital signature
[PATCH 2/2 V2] Powerpc/eSDHC: Add limit to data and erase timeout value calculation
Some cards apply too larg timeout value for host to response,
So limit the maximum data transfer timeout value and maximum erase
timeout value to aviod timeout issue.
Signed-off-by: Haijun Zhang haijun.zh...@freescale.com
Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com
CC: Anton Vorontsov cbouatmai...@gmail.com
---
changes for v2:
- Add limit to data and erase timeout value calculation
- split the V1 patch into two V2 patchs.
drivers/mmc/core/core.c | 27 ---
1 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c241fc1..c90b791 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -721,6 +721,10 @@ void mmc_set_data_timeout(struct mmc_data *data, const
struct mmc_card *card)
data-timeout_ns = 1; /* 100ms */
}
}
+
+ if (card-host-max_discard_to
+ card-host-max_discard_to div_u64(data-timeout_ns, 100))
+ data-timeout_ns = (u64)card-host-max_discard_to * 100;
}
EXPORT_SYMBOL(mmc_set_data_timeout);
@@ -1880,7 +1884,7 @@ static unsigned int mmc_do_calc_max_discard(struct
mmc_card *card,
return 0;
if (qty == 1)
- return 1;
+ return 1 card-erase_shift;
/* Convert qty to sectors */
if (card-erase_shift)
@@ -1898,16 +1902,17 @@ unsigned int mmc_calc_max_discard(struct mmc_card *card)
struct mmc_host *host = card-host;
unsigned int max_discard, max_trim;
- if (!host-max_discard_to)
- return UINT_MAX;
-
- /*
-* Without erase_group_def set, MMC erase timeout depends on clock
-* frequence which can change. In that case, the best choice is
-* just the preferred erase size.
-*/
- if (mmc_card_mmc(card) !(card-ext_csd.erase_group_def 1))
- return card-pref_erase;
+ if (!host-max_discard_to) {
+ if (mmc_card_sd(card))
+ return UINT_MAX;
+ /*
+* Without erase_group_def set, MMC erase timeout depends on
+* clock frequence which can change. In that case, the best
+* choice is just the preferred erase size.
+*/
+ if (mmc_card_mmc(card) !(card-ext_csd.erase_group_def 1))
+ return card-pref_erase;
+ }
max_discard = mmc_do_calc_max_discard(card, MMC_ERASE_ARG);
if (mmc_can_trim(card)) {
--
1.7.0.4
--
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 1/2 V2] Powerpc/eSDHC: Calculate the applicable mmc erase timeout value
As large area erase needs long time usually a few minutes,
which the host can't wait will bring about timeout error.
So we need to split the large area to small sections which
only need short erase time to avoid timeout error.
Signed-off-by: Haijun Zhang haijun.zh...@freescale.com
Signed-off-by: Jerry Huang chang-ming.hu...@freescale.com
CC: Anton Vorontsov cbouatmai...@gmail.com
---
changes for v2:
- Recompute the timeout value and max_discard_to for mmc erase
- split the V1 patch into two V2 patchs.
drivers/mmc/host/sdhci-esdhc.h|1 -
drivers/mmc/host/sdhci-of-esdhc.c | 13 +
2 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index d25f9ab..bb6d664 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -21,7 +21,6 @@
#define ESDHC_DEFAULT_QUIRKS (SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
SDHCI_QUIRK_NO_BUSY_IRQ | \
SDHCI_QUIRK_NONSTANDARD_CLOCK | \
- SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
SDHCI_QUIRK_PIO_NEEDS_DELAY | \
SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
b/drivers/mmc/host/sdhci-of-esdhc.c
index 63d219f..a09ea67 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -154,6 +154,18 @@ static void esdhc_of_set_clock(struct sdhci_host *host,
unsigned int clock)
/* Set the clock */
esdhc_set_clock(host, clock);
}
+/*
+ * As host dosn't supply us the method to calculate the timeout value,
+ * we assigned one for high speed SDHC card. So we can use this to calculate
+ * the max discard timeout value to limit the max discard sectors to avoid the
+ * timeout issue during large area erase.
+ */
+static unsigned int esdhc_of_get_timeout_clock(struct sdhci_host *host)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+ return pltfm_host-clock / 1000 / 32;
+}
#ifdef CONFIG_PM
static u32 esdhc_proctl;
@@ -190,6 +202,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
.enable_dma = esdhc_of_enable_dma,
.get_max_clock = esdhc_of_get_max_clock,
.get_min_clock = esdhc_of_get_min_clock,
+ .get_timeout_clock = esdhc_of_get_timeout_clock,
.platform_init = esdhc_of_platform_init,
#ifdef CONFIG_PM
.platform_suspend = esdhc_of_suspend,
--
1.7.0.4
--
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 v2] regulator: treat regulators with constant volatage as fixed
Some drivers has additional logic for fixed regulators. Let regulator core
to treat regulators which cannot change their voltage due to applied
constraints as fixed.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/regulator/core.c |6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 042c1ff..78b34b7 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1872,7 +1872,11 @@ int regulator_count_voltages(struct regulator *regulator)
{
struct regulator_dev*rdev = regulator-rdev;
- return rdev-desc-n_voltages ? : -EINVAL;
+ if (rdev-constraints-valid_ops_mask REGULATOR_CHANGE_VOLTAGE)
+ return rdev-desc-n_voltages ? rdev-desc-n_voltages :
+ -EINVAL;
+ else
+ return 1;
}
EXPORT_SYMBOL_GPL(regulator_count_voltages);
--
1.7.9.5
--
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 v2] regulator: treat regulators with constant volatage as fixed
On Tue, Nov 13, 2012 at 10:35:34AM +0100, Marek Szyprowski wrote: + if (rdev-constraints-valid_ops_mask REGULATOR_CHANGE_VOLTAGE) + return rdev-desc-n_voltages ? rdev-desc-n_voltages : +-EINVAL; The idea here was to avoid the ternery operator completely. signature.asc Description: Digital signature
[PATCH v3] regulator: treat regulators with constant volatage as fixed
Some drivers has additional logic for fixed regulators. Let regulator core
to treat regulators which cannot change their voltage due to applied
constraints as fixed.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/regulator/core.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 042c1ff..d07c240 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1872,7 +1872,14 @@ int regulator_count_voltages(struct regulator *regulator)
{
struct regulator_dev*rdev = regulator-rdev;
- return rdev-desc-n_voltages ? : -EINVAL;
+ if (rdev-constraints-valid_ops_mask REGULATOR_CHANGE_VOLTAGE) {
+ if (rdev-desc-n_voltages)
+ return rdev-desc-n_voltages;
+ else
+ return -EINVAL;
+ } else {
+ return 1;
+ }
}
EXPORT_SYMBOL_GPL(regulator_count_voltages);
--
1.7.9.5
--
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 0/2] mmc: host: sdhci-s3c: Add support for pinctrl interface
This series intends to add support for pin configuration using pin control interface. First patch cleans up GPIO requesting and freeing in the driver to simplify adding pin control support. Second patch adds pin control support to the driver. Tomasz Figa (2): mmc: host: sdhci-s3c: Use devm_gpio_request to request GPIOs mmc: host: sdhci-s3c: Add support for pinctrl .../devicetree/bindings/mmc/samsung-sdhci.txt | 20 ++--- drivers/mmc/host/sdhci-s3c.c | 50 -- 2 files changed, 31 insertions(+), 39 deletions(-) -- 1.8.0 -- 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 1/2] mmc: host: sdhci-s3c: Use devm_gpio_request to request GPIOs
The set of GPIO pins used by sdhci-s3c driver varies between
configurations, such as card detect method, pinctrl availability, etc.
This overly complicates the code requesting and freeing GPIO pins, which
must check which pins are used, when freeing them.
This patch modifies the sdhci-s3c driver to use devm_gpio_request to
free requested pins automatically after unbinding the driver.
Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/mmc/host/sdhci-s3c.c | 38 --
1 file changed, 8 insertions(+), 30 deletions(-)
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 2903949..039ed99 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -406,7 +406,7 @@ static void sdhci_s3c_setup_card_detect_gpio(struct
sdhci_s3c *sc)
struct s3c_sdhci_platdata *pdata = sc-pdata;
struct device *dev = sc-pdev-dev;
- if (gpio_request(pdata-ext_cd_gpio, SDHCI EXT CD) == 0) {
+ if (devm_gpio_request(dev, pdata-ext_cd_gpio, SDHCI EXT CD) == 0) {
sc-ext_cd_gpio = pdata-ext_cd_gpio;
sc-ext_cd_irq = gpio_to_irq(pdata-ext_cd_gpio);
if (sc-ext_cd_irq
@@ -487,7 +487,7 @@ static int __devinit sdhci_s3c_parse_dt(struct device *dev,
if (of_get_property(node, cd-inverted, NULL))
pdata-ext_cd_gpio_invert = 1;
} else if (pdata-cd_type == S3C_SDHCI_CD_INTERNAL) {
- ret = gpio_request(gpio, sdhci-cd);
+ ret = devm_gpio_request(dev, gpio, sdhci-cd);
if (ret) {
dev_err(dev, card detect gpio request failed\n);
return -EINVAL;
@@ -501,28 +501,20 @@ static int __devinit sdhci_s3c_parse_dt(struct device
*dev,
gpio = of_get_gpio(node, cnt);
if (!gpio_is_valid(gpio)) {
dev_err(dev, invalid gpio[%d]\n, cnt);
- goto err_free_dt_cd_gpio;
+ return -EINVAL;
}
ourhost-gpios[cnt] = gpio;
}
for (cnt = 0; cnt NUM_GPIOS(pdata-max_width); cnt++) {
- ret = gpio_request(ourhost-gpios[cnt], sdhci-gpio);
+ ret = devm_gpio_request(dev, ourhost-gpios[cnt], sdhci-gpio);
if (ret) {
dev_err(dev, gpio[%d] request failed\n, cnt);
- goto err_free_dt_gpios;
+ return -EINVAL;
}
}
return 0;
-
- err_free_dt_gpios:
- while (--cnt = 0)
- gpio_free(ourhost-gpios[cnt]);
- err_free_dt_cd_gpio:
- if (pdata-cd_type == S3C_SDHCI_CD_INTERNAL)
- gpio_free(ourhost-ext_cd_gpio);
- return -EINVAL;
}
#else
static int __devinit sdhci_s3c_parse_dt(struct device *dev,
@@ -585,7 +577,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device
*pdev)
if (pdev-dev.of_node) {
ret = sdhci_s3c_parse_dt(pdev-dev, host, pdata);
if (ret)
- goto err_pdata;
+ goto err_pdata_io_clk;
} else {
memcpy(pdata, pdev-dev.platform_data, sizeof(*pdata));
sc-ext_cd_gpio = -1; /* invalid gpio number */
@@ -603,7 +595,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device
*pdev)
if (IS_ERR(sc-clk_io)) {
dev_err(dev, failed to get io clock\n);
ret = PTR_ERR(sc-clk_io);
- goto err_io_clk;
+ goto err_pdata_io_clk;
}
/* enable the local io clock and keep it running for the moment. */
@@ -765,13 +757,7 @@ static int __devinit sdhci_s3c_probe(struct
platform_device *pdev)
clk_disable(sc-clk_io);
clk_put(sc-clk_io);
- err_io_clk:
- for (ptr = 0; ptr NUM_GPIOS(sc-pdata-max_width); ptr++)
- gpio_free(sc-gpios[ptr]);
- if (pdata-cd_type == S3C_SDHCI_CD_INTERNAL)
- gpio_free(sc-ext_cd_gpio);
-
- err_pdata:
+ err_pdata_io_clk:
sdhci_free_host(host);
return ret;
@@ -790,9 +776,6 @@ static int __devexit sdhci_s3c_remove(struct
platform_device *pdev)
if (sc-ext_cd_irq)
free_irq(sc-ext_cd_irq, sc);
- if (gpio_is_valid(sc-ext_cd_gpio))
- gpio_free(sc-ext_cd_gpio);
-
#ifdef CONFIG_PM_RUNTIME
clk_enable(sc-clk_io);
#endif
@@ -812,11 +795,6 @@ static int __devexit sdhci_s3c_remove(struct
platform_device *pdev)
clk_disable(sc-clk_io);
clk_put(sc-clk_io);
- if (pdev-dev.of_node) {
- for (ptr = 0; ptr NUM_GPIOS(sc-pdata-max_width); ptr++)
- gpio_free(sc-gpios[ptr]);
- }
-
sdhci_free_host(host);
platform_set_drvdata(pdev, NULL);
--
1.8.0
--
To unsubscribe from this list: send the line
[PATCH 2/2] mmc: host: sdhci-s3c: Add support for pinctrl
This patch adds support for pin configuration using pinctrl subsystem
to the sdhci-s3c driver.
Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
.../devicetree/bindings/mmc/samsung-sdhci.txt| 20 +---
drivers/mmc/host/sdhci-s3c.c | 12 ++--
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
index 630a7d7..97e9e31 100644
--- a/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
@@ -12,10 +12,6 @@ is used. The Samsung's SDHCI controller bindings extends
this as listed below.
[A] The property samsung,cd-pinmux-gpio can be used as stated in the
Optional Board Specific Properties section below.
-[B] If core card-detect bindings and samsung,cd-pinmux-gpio property
-is not specified, it is assumed that there is no card detection
-mechanism used.
-
Required SoC Specific Properties:
- compatible: should be one of the following
- samsung,s3c6410-sdhci: For controllers compatible with s3c6410 sdhci
@@ -24,14 +20,18 @@ Required SoC Specific Properties:
controller.
Required Board Specific Properties:
-- gpios: Should specify the gpios used for clock, command and data lines. The
- gpio specifier format depends on the gpio controller.
+- Samsung GPIO variant (will be completely replaced by pinctrl):
+ - gpios: Should specify the gpios used for clock, command and data lines. The
+gpio specifier format depends on the gpio controller.
+- Pinctrl variant (preferred if available):
+ - pinctrl-0: Should specify pin control groups used for this controller.
+ - pinctrl-names: Should contain only one value - default.
Optional Board Specific Properties:
- samsung,cd-pinmux-gpio: Specifies the card detect line that is routed
through a pinmux to the card-detect pin of the card slot. This property
should be used only if none of the mmc core card-detect properties are
- used.
+ used. Only for Samsung GPIO variant.
Example:
sdhci@1253 {
@@ -40,12 +40,18 @@ Example:
interrupts = 0 75 0;
bus-width = 4;
cd-gpios = gpk2 2 2 3 3;
+
+ /* Samsung GPIO variant */
gpios = gpk2 0 2 0 3, /* clock line */
gpk2 1 2 0 3, /* command line */
gpk2 3 2 3 3, /* data line 0 */
gpk2 4 2 3 3, /* data line 1 */
gpk2 5 2 3 3, /* data line 2 */
gpk2 6 2 3 3; /* data line 3 */
+
+ /* Pinctrl variant */
+ pinctrl-0 = sd0_clk sd0_cmd sd0_bus4;
+ pinctrl-names = default;
};
Note: This example shows both SoC specific and board specific properties
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 039ed99..f6731b0 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -24,6 +24,7 @@
#include linux/of_gpio.h
#include linux/pm.h
#include linux/pm_runtime.h
+#include linux/pinctrl/consumer.h
#include linux/mmc/host.h
@@ -57,6 +58,7 @@ struct sdhci_s3c {
int ext_cd_irq;
int ext_cd_gpio;
int *gpios;
+ struct pinctrl *pctrl;
struct clk *clk_io;
struct clk *clk_bus[MAX_BUS_CLK];
@@ -477,8 +479,9 @@ static int __devinit sdhci_s3c_parse_dt(struct device *dev,
return -EINVAL;
}
- dev_info(dev, assuming no card detect line available\n);
- pdata-cd_type = S3C_SDHCI_CD_NONE;
+ /* assuming internal card detect that will be configured by pinctrl */
+ pdata-cd_type = S3C_SDHCI_CD_INTERNAL;
+ goto setup_bus;
found_cd:
if (pdata-cd_type == S3C_SDHCI_CD_GPIO) {
@@ -496,6 +499,9 @@ static int __devinit sdhci_s3c_parse_dt(struct device *dev,
}
setup_bus:
+ if (!IS_ERR(ourhost-pctrl))
+ return 0;
+
/* get the gpios for command, clock and data lines */
for (cnt = 0; cnt NUM_GPIOS(pdata-max_width); cnt++) {
gpio = of_get_gpio(node, cnt);
@@ -574,6 +580,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device
*pdev)
goto err_pdata;
}
+ sc-pctrl = devm_pinctrl_get_select_default(pdev-dev);
+
if (pdev-dev.of_node) {
ret = sdhci_s3c_parse_dt(pdev-dev, host, pdata);
if (ret)
--
1.8.0
--
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] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi Marek,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage range
checking for them, otherwise the driver fails to operate with fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Perhaps it's a good idea to mention that the regulator API is changing
(being fixed) at the same time, and that's why this patch is necessary.
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
What about other occasions where is_supported_voltage() is used, like
this one -- is it necessary here too?
else if (regulator_is_supported_voltage(host-vqmmc, 180, 180))
regulator_enable(host-vqmmc);
Thanks,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
--
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 v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
--
1.7.9.5
--
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 v3] mmc: fix async request mechanism for sequential read scenarios
Hi Konstantin,
I'm looking into this patch.
Idea looks good as a whole.
On Tuesday, November 13, 2012, Konstantin Dorfman kdorf...@codeaurora.org
wrote:
When current request is running on the bus and if next request fetched
by mmcqd is NULL, mmc context (mmcqd thread) gets blocked until the
current request completes. This means if new request comes in while
the mmcqd thread is blocked, this new request can not be prepared in
parallel to current ongoing request. This may result in latency to
start new request.
This change allows to wake up the MMC thread (which
is waiting for the current running request to complete). Now once the
MMC thread is woken up, new request can be fetched and prepared in
parallel to current running request which means this new request can
be started immediately after the current running request completes.
With this change read throughput is improved by 16%.
Signed-off-by: Konstantin Dorfman kdorf...@codeaurora.org
---
v3:
- new MMC_QUEUE_NEW_REQUEST flag to mark new request case
- lock added to update is_new_req flag
- condition for sending new req notification changed
- Moved start waiting for new req notification after
fetching NULL
v2:
- Removed synchronization flags
- removed lock from done()
- flags names changed
v1:
- Initial submit
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 172a768..34d8bd9 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -112,17 +112,6 @@ struct mmc_blk_data {
static DEFINE_MUTEX(open_lock);
-enum mmc_blk_status {
- MMC_BLK_SUCCESS = 0,
- MMC_BLK_PARTIAL,
- MMC_BLK_CMD_ERR,
- MMC_BLK_RETRY,
- MMC_BLK_ABORT,
- MMC_BLK_DATA_ERR,
- MMC_BLK_ECC_ERR,
- MMC_BLK_NOMEDIUM,
-};
-
module_param(perdev_minors, int, 0444);
MODULE_PARM_DESC(perdev_minors, Minors numbers to allocate per device);
@@ -1225,6 +1214,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req
*mqrq,
mqrq-mmc_active.mrq = brq-mrq;
mqrq-mmc_active.err_check = mmc_blk_err_check;
+ mqrq-mmc_active.mrq-context_info = mq-context_info;
mmc_queue_bounce_pre(mqrq);
}
@@ -1284,9 +1274,12 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
areq = mq-mqrq_cur-mmc_active;
} else
areq = NULL;
- areq = mmc_start_req(card-host, areq, (int *) status);
- if (!areq)
+ areq = mmc_start_req(card-host, areq, (int *)status);
+ if (!areq) {
+ if (status == MMC_BLK_NEW_REQUEST)
+ mq-flags |= MMC_QUEUE_NEW_REQUEST;
return 0;
+ }
mq_rq = container_of(areq, struct mmc_queue_req, mmc_active);
brq = mq_rq-brq;
@@ -1295,6 +1288,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
mmc_queue_bounce_post(mq_rq);
switch (status) {
+ case MMC_BLK_NEW_REQUEST:
+ BUG(); /* should never get here */
case MMC_BLK_SUCCESS:
case MMC_BLK_PARTIAL:
/*
@@ -1367,7 +1362,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
* prepare it again and resend.
*/
mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq);
- mmc_start_req(card-host, mq_rq-mmc_active, NULL);
+ mmc_start_req(card-host, mq_rq-mmc_active,
+ (int *)status);
Here, we'll try to send previous request which is not completed successfully.
And then at the top of do~while, mmc_start_req will be called for current
request and get the status.
Above change looks like unnecessary. Is there any reason?
Thanks,
Seungwon Jeon
}
} while (ret);
@@ -1407,6 +1403,8 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,
struct request *req)
goto out;
}
+ mq-flags = ~MMC_QUEUE_NEW_REQUEST;
+
if (req req-cmd_flags REQ_DISCARD) {
/* complete ongoing async transfer before issuing discard */
if (card-host-areq)
@@ -1426,9 +1424,10 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,
struct request *req)
}
out:
- if (!req)
+ if (!req !(mq-flags MMC_QUEUE_NEW_REQUEST))
/* release host only when there are no more requests */
mmc_release_host(card-host);
+
return ret;
}
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index fadf52e..ef575ac 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -22,7 +22,6 @@
#define MMC_QUEUE_BOUNCESZ 65536
-#define MMC_QUEUE_SUSPENDED (1 0)
/*
*
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi Marek,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
--
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 v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hello,
On 11/13/2012 2:45 PM, Chris Ball wrote:
Hi Marek,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
Best regards
--
Marek Szyprowski
Samsung Poland RD Center
--
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 v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
Thanks,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
--
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: drivers/mmc/card/block.c infinite loop in mmc_blk_err_check waiting on R1_READY_FOR_DATA
Hi Trey, On Tue, Nov 13 2012, Trey Ramsay wrote: Thanks Chris. Any idea how long the timeout should be? It could cause problems if we timeout to early. This what I have so far with a 10 minute timeout. The code is based off of v3.7-rc3 Ten minutes sounds excessive, which is actually fine for our purpose; I don't think we need to make the value tunable. Thanks! Please can you: * resend in plain text instead of HTML, with no line-wrapping corrupting the patch * include a commit message and Signed-off-by (see SubmittingPatches in the Documentation/ directory) * change the pr_err()s to include the MMC hostname, and give an English description of the error instead of the line number -- most users don't have their kernel source available to find line numbers in. Something like Card stuck in programming state! sounds good. After that, I'll merge the patch for testing. Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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 v1] mmc: fix async request mechanism for sequential read scenarios
On Tue, Oct 30, 2012 at 1:19 PM, Konstantin Dorfman
kdorf...@codeaurora.org wrote:
Hello,
On 10/29/2012 11:40 PM, Per Forlin wrote:
Hi,
I would like to move the focus of my concerns from root cause analysis
to the actual patch,
My first reflection is that the patch is relatively complex and some
of the code looks duplicated.
Would it be possible to simplify it and re-use the current execution flow.
Is the following flow feasible?
in mmc_start_req():
--
if (rqc == NULL not_resend)
wait_for_both_mmc_and_arrival_of_new_req
else
wait_only_for_mmc
if (arrival_of_new_req) {
set flag to indicate fetch-new_req
return NULL;
}
-
in queue.c:
if (fetch-new_req)
don't overwrite previous req.
BR
Per
You are talking about using both waiting mechanisms, old (wait on
completion) and new - wait_event_interruptible()? But how done()
callback, called on host controller irq context, will differentiate
which one is relevant for the request?
I think it is better to use single wait point for mmc thread.
I have sketch a patch to better explain my point. It's not tested it
barely compiles :)
The patch tries to introduce your feature and still keep the same code
path. And it exposes an API that could be used by SDIO as well.
The intention of my sketch patch is only to explain what I tried to
visualize in the pseudo code previously in this thread.
The out come of your final patch should be documented here I think:
Documentation/mmc/mmc-async-req.txt
Here follows my patch sketch:
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index e360a97..08036a1 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -66,6 +66,8 @@ static int mmc_queue_thread(void *d)
spin_unlock_irq(q-queue_lock);
if (req || mq-mqrq_prev-req) {
+ if (!req)
+ mmc_prefetch_start(mq-mqrq_prev-mmc_active,
true);
set_current_state(TASK_RUNNING);
mq-issue_fn(mq, req);
} else {
@@ -79,11 +81,13 @@ static int mmc_queue_thread(void *d)
}
/* Current request becomes previous request and vice versa. */
- mq-mqrq_prev-brq.mrq.data = NULL;
- mq-mqrq_prev-req = NULL;
- tmp = mq-mqrq_prev;
- mq-mqrq_prev = mq-mqrq_cur;
- mq-mqrq_cur = tmp;
+ if (!mmc_prefetch_pending(mq-mqrq_prev-mmc_active)) {
+ mq-mqrq_prev-brq.mrq.data = NULL;
+ mq-mqrq_prev-req = NULL;
+ tmp = mq-mqrq_prev;
+ mq-mqrq_prev = mq-mqrq_cur;
+ mq-mqrq_cur = tmp;
+ }
} while (1);
up(mq-thread_sem);
@@ -109,10 +113,44 @@ static void mmc_request_fn(struct request_queue *q)
return;
}
+ if (mq-prefetch_enable) {
+ spin_lock(mq-prefetch_lock);
+ if (mq-prefetch_completion)
+ complete(mq-prefetch_completion);
+ mq-prefetch_pending = true;
+ spin_unlock(mq-prefetch_lock);
+ }
+
if (!mq-mqrq_cur-req !mq-mqrq_prev-req)
wake_up_process(mq-thread);
}
+static void mmc_req_init(struct mmc_async_req *areq, struct completion *compl)
+{
+ struct mmc_queue *mq =
+ container_of(areq-prefetch, struct mmc_queue, prefetch);
+
+ spin_lock(mq-prefetch_lock);
+ mq-prefetch_completion = compl;
+ if (mq-prefetch_pending)
+ complete(mq-prefetch_completion);
+ spin_unlock(mq-prefetch_lock);
+}
+
+static void mmc_req_start(struct mmc_async_req *areq, bool enable)
+{
+ struct mmc_queue *mq =
+ container_of(areq-prefetch, struct mmc_queue, prefetch);
+ mq-prefetch_enable = enable;
+}
+
+static bool mmc_req_pending(struct mmc_async_req *areq)
+{
+ struct mmc_queue *mq =
+ container_of(areq-prefetch, struct mmc_queue, prefetch);
+ return mq-prefetch_pending;
+}
+
static struct scatterlist *mmc_alloc_sg(int sg_len, int *err)
{
struct scatterlist *sg;
@@ -166,6 +204,12 @@ int mmc_init_queue(struct mmc_queue *mq, struct
mmc_card *card,
int ret;
struct mmc_queue_req *mqrq_cur = mq-mqrq[0];
struct mmc_queue_req *mqrq_prev = mq-mqrq[1];
+ spin_lock_init(mq-prefetch_lock);
+ mq-prefetch.wait_req_init = mmc_req_init;
+ mq-prefetch.wait_req_start = mmc_req_start;
+ mq-prefetch.wait_req_pending = mmc_req_pending;
+ mqrq_cur-mmc_active.prefetch = mq-prefetch;
+ mqrq_prev-mmc_active.prefetch = mq-prefetch;
if (mmc_dev(host)-dma_mask *mmc_dev(host)-dma_mask)
limit = *mmc_dev(host)-dma_mask;
diff --git a/drivers/mmc/card/queue.h
Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
Hi Marek,
Is the regulator dedicated ? or is it shared ? Is it used for eMMC ?
If it cannot be turned off -- then just don't list it in the regulators list
for vmmc.
If it CAN be turned off then need to get back to you.
Philip
On Nov 13, 2012, at 2:14 PM, Chris Ball c...@laptop.org wrote:
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
Thanks,
- Chris.
--
Chris Ball c...@laptop.org http://printf.net/
One Laptop Per Child
--
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] mmc: sdhci: apply voltage range check only for non-fixed regulators
On Tue, Nov 13, 2012 at 07:45:07AM -0500, Chris Ball wrote: On Tue, Nov 13 2012, Marek Szyprowski wrote: Fixed regulators cannot change their voltage, so disable all voltage range checking for them, otherwise the driver fails to operate with fixed regulators. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Perhaps it's a good idea to mention that the regulator API is changing (being fixed) at the same time, and that's why this patch is necessary. This should be totally unrelated to any externally visible change in the regulator API, if anything I would expect fixes in the API to *improve* the ability of clients to work with fixed voltage regulators. What's missing here is some explanation as to what the problem is and how it's being fixed. As I understand it this should really be a workaround for hardware which runs cards at out of spec voltages; we have a fixed voltage regulator which claims to not support any of the voltages that are in spec (due to this being what the hardware does) so if we try to use the voltage management stuff in the MMC API it gets upset. As a workaround if we can't change the voltage we just ignore the voltage aspects of the API. signature.asc Description: Digital signature
Re: [PATCH v6 05/14] mmc: sdhci: fix null return check of regulator_get
On Tue, Nov 13, 2012 at 11:00 AM, Kevin Liu keyuan@gmail.com wrote:
Updated Haojian and Philip's mail address.
2012/11/12 Kevin Liu keyuan@gmail.com:
2012/11/5 Chris Ball c...@laptop.org:
Hi,
On Wed, Oct 17 2012, Kevin Liu wrote:
From: Kevin Liu kl...@marvell.com
regulator_get() returns NULL when CONFIG_REGULATOR not defined,
which should not print out the warning.
Reviewed-by: Philip Rakity prak...@marvell.com
Signed-off-by: Bin Wang b...@marvell.com
Signed-off-by: Kevin Liu kl...@marvell.com
---
drivers/mmc/host/sdhci.c | 18 --
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8e6a6f0..0104ae9 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2845,9 +2845,12 @@ int sdhci_add_host(struct sdhci_host *host)
/* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
host-vqmmc = regulator_get(mmc_dev(mmc), vqmmc);
- if (IS_ERR(host-vqmmc)) {
- pr_info(%s: no vqmmc regulator found\n, mmc_hostname(mmc));
- host-vqmmc = NULL;
+ if (IS_ERR_OR_NULL(host-vqmmc)) {
+ if (PTR_ERR(host-vqmmc) 0) {
+ pr_info(%s: no vqmmc regulator found\n,
+ mmc_hostname(mmc));
+ host-vqmmc = NULL;
+ }
}
else if (regulator_is_supported_voltage(host-vqmmc, 180,
180))
regulator_enable(host-vqmmc);
@@ -2903,9 +2906,12 @@ int sdhci_add_host(struct sdhci_host *host)
ocr_avail = 0;
host-vmmc = regulator_get(mmc_dev(mmc), vmmc);
- if (IS_ERR(host-vmmc)) {
- pr_info(%s: no vmmc regulator found\n, mmc_hostname(mmc));
- host-vmmc = NULL;
+ if (IS_ERR_OR_NULL(host-vmmc)) {
+ if (PTR_ERR(host-vmmc) 0) {
+ pr_info(%s: no vmmc regulator found\n,
+ mmc_hostname(mmc));
+ host-vmmc = NULL;
+ }
} else
regulator_enable(host-vmmc);
Pushed to mmc-next for 3.7 with this commit message:
Chris,
Thanks for the push.
But how about the other 13 patches in the same patchset ([PATCH v6
00/14] mmc: sdhci: mmc: sdhci: fixes and enhancements)?
Most of them also aim to fix bugs.
Kevin
Hi Chris,
How do you think about other patches in this patch set?
Regards
Haojian
--
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 v3] regulator: treat regulators with constant volatage as fixed
On Tue, Nov 13, 2012 at 10:49:37AM +0100, Marek Szyprowski wrote:
+ if (rdev-constraints-valid_ops_mask REGULATOR_CHANGE_VOLTAGE) {
+ if (rdev-desc-n_voltages)
+ return rdev-desc-n_voltages;
+ else
+ return -EINVAL;
+ } else {
+ return 1;
+ }
Hrm, now I can read the logic I'm not convinced this is a good idea.
This will report that we have an available voltage for devices which
don't know their voltage (things like battery supplies often do this as
the voltage is unregulated) and it will mean that we are doing something
different for the case where there's only one voltage (reporting the
restricted count instead of the physically supported count).
I think we want a regulator_can_change_voltage() or possibly a count
function (though I can't see any use cases except this) which answers
the question directly instead of layering on top of this function.
signature.asc
Description: Digital signature
Re: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
Hi Maya, On Sun, Nov 04 2012, me...@codeaurora.org wrote: Packed commands is a mandatory eMMC4.5 feature and is supported by all the card vendors. We're still only talking about using packed writes, though, right? It wa proven to be beneficial for eMMC4.5 cards and harmless for non eMMC4.5 cards. My understanding is that write packing causes a regression in read performance that can be tuned/fixed by your num_wr_reqs_to_start_packing tunable (and read packing causes a read regression with current eMMC 4.5 cards). Is that wrong? I don't see a point to hold it back while it can be enabled or disabled by a flag and most of the code it adds is guarded in specific functions and is not active when packed commands is disabled. Earlier in the thread I wrote: * I still don't have a good set of representative benchmarks showing what kind of performance changes come with this patchset. It seems like we've had a small amount of testing on one controller/eMMC part combo from Seungwon, and an entirely different test from Maya, and the results aren't documented fully anywhere to the level of describing what the hardware was, what the test was, and what the results were before and after the patchset. I still feel this way. I'm worried that we might be merging code that works well on your controller/card but causes large regressions for everyone else. I don't want to handle this by making a tunable that everyone has to tune for their system, because I don't think anyone will tune it. I don't think that shipping a capability that will probably lead to performance regressions if you turn it on is a good idea. I'm in a better position to help now, though -- I have some motherboards with Marvell SoCs and a socketed eMMC slot, and I have eMMC 4.5 parts from Sandisk and Toshiba. So I can try to help work out how generalizable your results are across other controllers and cards. So far I've only tried the Sandisk part, but it didn't show any write improvement with write packing. I've verified that the switch command to turn on packed_event_en happens and succeeds, and that the caps are set correctly, so I'm not sure what's wrong yet. With iozone I get: KB reclen write rewrite Unpacked writes:102408192 17250 16794 Packed writes: 102408192 16930 17353 I'll try the Toshiba part next, and I'll start using lmdd as well as iozone. Any ideas on why I might not be seeing improvements with Sandisk? I'm not opposed to merging packed write support in principle, I just want to be convinced that we're not causing regressions for most users who turn it on. (And more than that, I want to see that it leads to improvements that make it worth adding the code complexity for.) Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
From: linux-mmc-ow...@vger.kernel.org
[mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Chris Ball
Sent: Tuesday, November 13, 2012 10:14 PM
To: Marek Szyprowski
Cc: linux-ker...@vger.kernel.org; linux-mmc@vger.kernel.org; Kyungmin Park;
Mark Brown; Liam Girdwood; Philip Rakity
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only for
non-fixed regulators
Hi,
On Tue, Nov 13 2012, Marek Szyprowski wrote:
On Tue, Nov 13 2012, Marek Szyprowski wrote:
Fixed regulators cannot change their voltage, so disable all voltage
range checking for them, otherwise the driver fails to operate with
fixed regulators. Up to now it worked only by luck, because
regulator_is_supported_voltage() function returned incorrect values.
Commit regulator: fix voltage check in regulator_is_supported_voltage()
fixed that function and now additional check is needed for fixed
regulators.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/mmc/host/sdhci.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c7851c0..6f6534e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
regulator_enable(host-vmmc);
#ifdef CONFIG_REGULATOR
- if (host-vmmc) {
+ if (host-vmmc regulator_count_voltages(host-vmmc) 1) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
Thanks for the longer explanation. I'm still missing something, though;
what's wrong with running the check as it was with the new regulator code?
(I haven't tried it yet.)
#ifdef CONFIG_REGULATOR
if (host-vmmc) {
ret = regulator_is_supported_voltage(host-vmmc, 330,
330);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_330)))
caps[0] = ~SDHCI_CAN_VDD_330;
ret = regulator_is_supported_voltage(host-vmmc, 300,
300);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_300)))
caps[0] = ~SDHCI_CAN_VDD_300;
ret = regulator_is_supported_voltage(host-vmmc, 180,
180);
if ((ret = 0) || (!(caps[0] SDHCI_CAN_VDD_180)))
caps[0] = ~SDHCI_CAN_VDD_180;
}
#endif /* CONFIG_REGULATOR */
The point is to remove unsupported voltages, so if someone sets up a
fixed regulator at 330, all of the other caps are disabled. Why
wouldn't that work without this change, and how are we supposed to
remove those caps on a fixed regulator after your patchset?
Thanks, sorry if I'm missing something obvious,
On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).
I see. Sounds like a separate bug -- Philip (or anyone else), any
idea how we should be treating eMMCs with a fixed voltage here?
I think we should check the voltage range rather than the voltage
point accoring to the spec.
Otherwise some valid voltage like 2.8v will be discarded by mistake.
My below old patch aim to fix this issue.
How do you think?
-Original Message-
From: Kevin Liu [mailto:keyuan@gmail.com]
Sent: Friday, September 28, 2012 3:56 PM
To: linux-mmc@vger.kernel.org; c...@laptop.org; pie...@ossman.eu;
ulf.hans...@linaro.org; Zhangfei Gao
Cc: Haojian Zhuang; Chao Xie; Philip Rakity; Kevin Liu; Jialing Fu
Subject: [PATCH v5 03/13] mmc: sdhci: use regulator min/max voltage
range according to spec
From: Kevin Liu kl...@marvell.com
For regulator vmmc/vmmcq, use voltage range as below
3.3v/3.0v: (2.7v, 3.6v)
1.8v: (1.7v, 1.95v)
Original code use the specific value which may fail in regulator
driver if it does NOT support the specific voltage.
Signed-off-by: Jialing Fu j...@marvell.com
Signed-off-by: Kevin Liu kl...@marvell.com
---
drivers/mmc/host/sdhci.c | 16 +++-
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3aef580..36afd47 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1628,7 +1628,7 @@ static int
sdhci_do_3_3v_signal_voltage_switch(struct sdhci_host *host,
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
if (host-vqmmc) {
- ret = regulator_set_voltage(host-vqmmc, 330, 330);
+ ret = regulator_set_voltage(host-vqmmc, 270, 360);
Re: FW: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators
On Wed, Nov 14, 2012 at 03:11:37PM +0800, Kevin Liu wrote: - ret = regulator_set_voltage(host-vqmmc, 330, 330); + ret = regulator_set_voltage(host-vqmmc, 270, 360); Should this be regulator_set_voltage_tol()? Otherwise it'd be good to explain where the numbers come from. + ret = regulator_is_supported_voltage(host-vmmc, 170, + 195); We should really add a regulator_is_supported_voltage_tol... let me just do that. signature.asc Description: Digital signature
