mmc: sdhci-esdhci-imx: fix cd/wp regression
Subsystems (5)
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
Commit 8d86e4f mmc: sdhci-esdhc-imx: Call mmc_of_parse() seriously break the sd card detect/write protect function of most IMX platforms since a lot of GPIO flags of cd-gpios/wp-gpios are wrongly set in devicetree.
It breaks the following things:
1) cd-gpio function enable
Most IMX boards GPIO card detect function becomes unwork and using card polling mode instead by mistake.
Openning CONFIG_MMC_DEBUG will easily see that.
It is caused by the default quirk SDHCI_QUIRK_BROKEN_CARD_DETECTION is not cleared for dt platform by commit 8d86e4f, then MMC_CAP_NEEDS_POLL is wrongly set.
Patch 1~2 is to fix it.
2) cd-gpio/wp-gpio polarity
Before commit 8d86e4f, we do not use GPIO flags passed from devicetree. But after it, GPIO flags specfied in dt becomes valid and will affect the normal cd/wp function.
This is fixed by another dts fix patch series:
[PATCH 0/5] dts: imx: fix sd card gpio polarity specified in device tree
http://www.spinics.net/lists/arm-kernel/msg426156.html
Since it needs to change a lot board dts files, so i cook the patch based on Shawn/for-next branch to make sure all board dts file are changed in the same time.
The dts fix patches series should be picked before this series since the driver fixes depend on the correct dts(correct GPIO polarity) to work properly.
3) max-frequency not work in sdhcimmc_of_parse will parse the max-frequency from devicetree, however, its value will be overwritten by common sdhci driver finnally,so it does not actually work.
Patch 3 is to fix it.
4) patch 4~6 clear unneeded parsing code after switch to mmc_of_parse().
The series is based on ulf/next tree.
I also CCed this patch series to all board maintainer related in this change, hope they can help test it.
I only tested some freescale IMX boards.
BTW, There's indeed a problem here that dts fix patch series needs to be picked before this series, but it can't go through Ulf tree since some board dts changed does not in Ulf's tree.
And this patch series also can not go through Shawn's tree, since mmc part change depends on some patches in Ulf tree. Two series are cross dependant.
Hope you guy can find a solution for it.
Those two patch series is an important fix, hopefully can catch up the final 4.1 merge window.
Dong Aisheng (6):
mmc: sdhci-esdhc-imx: fix cd regression for dt platform
mmc: sdhci-esdhc-imx: move all non dt probe code into one function
mmc: sdhci: make max-frequency property in device tree work
mmc: sdhci-esdhc-imx: remove duplicated dts parsing
mmc: sdhci-esdhc-imx: clear f_max in boarddata
dts: mmc: fsl-imx-esdhc: remove fsl,cd-controller support
.../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 2 - drivers/mmc/host/sdhci-esdhc-imx.c | 210 ++++++++++----------- drivers/mmc/host/sdhci.c | 9 +- include/linux/platform_data/mmc-esdhc-imx.h | 1 - 4 files changed, 111 insertions(+), 111 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/6 mmc: sdhci-esdhc-imx: fix cd regression for dt platform
Current card detect probe process is that when driver finds a valid ESDHC_CD_GPIO, it will clear the quirk SDHCI_QUIRK_BROKEN_CARD_DETECTION which is set by default for all esdhc/usdhc controllers. Then host driver will know there's a valid card detect function.
Commit 8d86e4fcccf6 ("mmc: sdhci-esdhc-imx: Call mmc_of_parse()") breaks GPIO CD function for dt platform that it will return directly when find ESDHC_CD_GPIO for dt platform which result in the later wrongly to keep SDHCI_QUIRK_BROKEN_CARD_DETECTION for all dt platforms. Then MMC_CAP_NEEDS_POLL will be used instead even there's a valid GPIO card detect.
This patch adds back this function and follows the original approach to clear the quirk if find an valid CD GPIO for dt platforms.
--- drivers/mmc/host/sdhci-esdhc-imx.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/mmc/host/sdhci-esdhc-imx.c +9 -1
@@ -881,6 +881,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev, struct esdhc_platform_data *boarddata) {struct device_node *np = pdev->dev.of_node; +int ret;if (!np)return -ENODEV ;
@@ -917,7 +918,14 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,mmc_of_parse_voltage (np, &host->ocr_mask);/* call to generic mmc_of_parse to support additional capabilities */ -return mmc_of_parse (host->mmc); + ret =mmc_of_parse (host->mmc); +if (ret) +return ret; + +if (!IS_ERR_VALUE (mmc_gpio_get_cd (host->mmc))) + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; + +return 0 ; }#else static inline int
Signature · 1 line
1.9.1
PATCH 2/6 mmc: sdhci-esdhc-imx: move all non dt probe code into one function
In reply to Dong AishengThis is an incremental fix of commite62bd351b("mmc: sdhci-esdhc-imx: Do not break platform data boards").
After commit 8d86e4fcccf6 ("mmc: sdhci-esdhc-imx: Call mmc_of_parse()"), we do not need to run the check of boarddata->wp_type/cd_type/max_bus_width again for dt platform since those are already handled by mmc_of_parse().
Current code only exclude the checking of wp_type for dt platform which does not make sense.
This patch moves all non dt probe code into one function. Besides, since we only support SD3.0/eMMC HS200 for dt platform, the support_vsel checking and ultra high speed pinctrl state are also merged into sdhci_esdhc_imx_probe_dt.
Then we have two separately probe function for dt and non dt type. This can make the driver probe more clearly.
--- drivers/mmc/host/sdhci-esdhc-imx.c | 179 +++++++++++++++++++------------------ 1 file changed, 94 insertions(+), 85 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +94 -85
@@ -878,14 +878,12 @@ static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {static int sdhci_esdhc_imx_probe_dt (struct platform_device *pdev,struct sdhci_host *host, -struct esdhc_platform_data *boarddata) +struct pltfm_imx_data *imx_data) {struct device_node *np = pdev->dev.of_node; +struct esdhc_platform_data *boarddata = &imx_data->boarddata;int ret; -if (!np) -return -ENODEV ; -if (of_get_property (np,"non-removable" ,NULL )) boarddata->cd_type =ESDHC_CD_PERMANENT ;
@@ -917,6 +915,26 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,mmc_of_parse_voltage (np, &host->ocr_mask); +/* sdr50 and sdr104 needs work on 1.8v signal voltage */ +if ((boarddata->support_vsel) &&esdhc_is_usdhc (imx_data) && + !IS_ERR (imx_data->pins_default)) { + imx_data->pins_100mhz =pinctrl_lookup_state (imx_data->pinctrl, +ESDHC_PINCTRL_STATE_100MHZ ); + imx_data->pins_200mhz =pinctrl_lookup_state (imx_data->pinctrl, +ESDHC_PINCTRL_STATE_200MHZ ); +if (IS_ERR (imx_data->pins_100mhz) || +IS_ERR (imx_data->pins_200mhz)) { +dev_warn (mmc_dev (host->mmc), +"could not get ultra high speed state, work on normal mode\n" ); +/* +* fall back to not support uhs by specify no 1.8v quirk +*/ + host->quirks2 |=SDHCI_QUIRK2_NO_1_8_V ; + } + }else { + host->quirks2 |=SDHCI_QUIRK2_NO_1_8_V ; + } +/* call to generic mmc_of_parse to support additional capabilities */ ret =mmc_of_parse (host->mmc);if (ret)
@@ -931,22 +949,85 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,static inline int sdhci_esdhc_imx_probe_dt (struct platform_device *pdev,struct sdhci_host *host, -struct esdhc_platform_data *boarddata) +struct pltfm_imx_data *imx_data) {return -ENODEV ; }#endif +static int sdhci_esdhc_imx_probe_nondt (struct platform_device *pdev, +struct sdhci_host *host, +struct pltfm_imx_data *imx_data) +{ +struct esdhc_platform_data *boarddata = &imx_data->boarddata; +int err; + +if (!host->mmc->parent->platform_data) { +dev_err (mmc_dev (host->mmc),"no board data!\n" ); +return -EINVAL ; + } + + imx_data->boarddata = *((struct esdhc_platform_data *) + host->mmc->parent->platform_data); +/* write_protect */ +if (boarddata->wp_type ==ESDHC_WP_GPIO ) { + err =mmc_gpio_request_ro (host->mmc, boarddata->wp_gpio); +if (err) { +dev_err (mmc_dev (host->mmc), +"failed to request write-protect gpio!\n" ); +return err; + } + host->mmc->caps2 |=MMC_CAP2_RO_ACTIVE_HIGH ; + } + +/* card_detect */ +switch (boarddata->cd_type) { +case ESDHC_CD_GPIO : + err =mmc_gpio_request_cd (host->mmc, boarddata->cd_gpio,0 ); +if (err) { +dev_err (mmc_dev (host->mmc), +"failed to request card-detect gpio!\n" ); +return err; + } +/* fall through */ + +case ESDHC_CD_CONTROLLER : +/* we have a working card_detect back */ + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +break ; + +case ESDHC_CD_PERMANENT : + host->mmc->caps |=MMC_CAP_NONREMOVABLE ; +break ; + +case ESDHC_CD_NONE : +break ; + } + +switch (boarddata->max_bus_width) { +case 8 : + host->mmc->caps |=MMC_CAP_8_BIT_DATA |MMC_CAP_4_BIT_DATA ; +break ; +case 4 : + host->mmc->caps |=MMC_CAP_4_BIT_DATA ; +break ; +case 1 : +default : + host->quirks |=SDHCI_QUIRK_FORCE_1_BIT_DATA ; +break ; + } + +return 0 ; +} +static int sdhci_esdhc_imx_probe (struct platform_device *pdev) {const struct of_device_id *of_id =of_match_device (imx_esdhc_dt_ids, &pdev->dev);struct sdhci_pltfm_host *pltfm_host;struct sdhci_host *host; -struct esdhc_platform_data *boarddata;int err;struct pltfm_imx_data *imx_data; -bool dt = true; host =sdhci_pltfm_init (pdev, &sdhci_esdhc_imx_pdata,0 );if (IS_ERR (host))
@@ -1038,84 +1119,12 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)if (imx_data->socdata->flags &ESDHC_FLAG_ERR004536 ) host->quirks |=SDHCI_QUIRK_BROKEN_ADMA ; - boarddata = &imx_data->boarddata; -if (sdhci_esdhc_imx_probe_dt (pdev, host, boarddata) <0 ) { -if (!host->mmc->parent->platform_data) { -dev_err (mmc_dev (host->mmc),"no board data!\n" ); - err = -EINVAL ; - goto disable_clk; - } - imx_data->boarddata = *((struct esdhc_platform_data *) - host->mmc->parent->platform_data); - dt = false; - } -/* write_protect */ -if (boarddata->wp_type ==ESDHC_WP_GPIO && !dt) { - err =mmc_gpio_request_ro (host->mmc, boarddata->wp_gpio); -if (err) { -dev_err (mmc_dev (host->mmc), -"failed to request write-protect gpio!\n" ); - goto disable_clk; - } - host->mmc->caps2 |=MMC_CAP2_RO_ACTIVE_HIGH ; - } - -/* card_detect */ -switch (boarddata->cd_type) { -case ESDHC_CD_GPIO : -if (dt) -break ; - err =mmc_gpio_request_cd (host->mmc, boarddata->cd_gpio,0 ); -if (err) { -dev_err (mmc_dev (host->mmc), -"failed to request card-detect gpio!\n" ); - goto disable_clk; - } -/* fall through */ - -case ESDHC_CD_CONTROLLER : -/* we have a working card_detect back */ - host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; -break ; - -case ESDHC_CD_PERMANENT : - host->mmc->caps |=MMC_CAP_NONREMOVABLE ; -break ; - -case ESDHC_CD_NONE : -break ; - } - -switch (boarddata->max_bus_width) { -case 8 : - host->mmc->caps |=MMC_CAP_8_BIT_DATA |MMC_CAP_4_BIT_DATA ; -break ; -case 4 : - host->mmc->caps |=MMC_CAP_4_BIT_DATA ; -break ; -case 1 : -default : - host->quirks |=SDHCI_QUIRK_FORCE_1_BIT_DATA ; -break ; - } - -/* sdr50 and sdr104 needs work on 1.8v signal voltage */ -if ((boarddata->support_vsel) &&esdhc_is_usdhc (imx_data) && - !IS_ERR (imx_data->pins_default)) { - imx_data->pins_100mhz =pinctrl_lookup_state (imx_data->pinctrl, -ESDHC_PINCTRL_STATE_100MHZ ); - imx_data->pins_200mhz =pinctrl_lookup_state (imx_data->pinctrl, -ESDHC_PINCTRL_STATE_200MHZ ); -if (IS_ERR (imx_data->pins_100mhz) || -IS_ERR (imx_data->pins_200mhz)) { -dev_warn (mmc_dev (host->mmc), -"could not get ultra high speed state, work on normal mode\n" ); -/* fall back to not support uhs by specify no 1.8v quirk */ - host->quirks2 |=SDHCI_QUIRK2_NO_1_8_V ; - } - }else { - host->quirks2 |=SDHCI_QUIRK2_NO_1_8_V ; - } +if (of_id) + err =sdhci_esdhc_imx_probe_dt (pdev, host, imx_data); +else + err =sdhci_esdhc_imx_probe_nondt (pdev, host, imx_data); +if (err) + goto disable_clk; err =sdhci_add_host (host);if (err)
Signature · 1 line
1.9.1
PATCH 3/6 mmc: sdhci: make max-frequency property in device tree work
In reply to Dong AishengDevice tree provides option to specify the max freqency with property "max-frequency" in dts and common parse function mmc_of_parse() will parse it and use this value to set host->f_max to tell the MMC core the maxinum frequency the host works.
However, current sdhci driver will finally overwrite this value with host->max_clk regardless of the max-frequency property.
This patch makes sure not overwrite the max-frequency set from device tree and do basic sanity check.
--- drivers/mmc/host/sdhci.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci.c +7 -2
@@ -2866,6 +2866,7 @@ int sdhci_add_host(struct sdhci_host *host)u32 max_current_caps;unsigned int ocr_avail;unsigned int override_timeout_clk; +u32 max_clk;int ret;WARN_ON (host ==NULL );
@@ -3047,18 +3048,22 @@ int sdhci_add_host(struct sdhci_host *host)* Set host parameters. */ mmc->ops = &sdhci_ops; - mmc->f_max = host->max_clk; + max_clk = host->max_clk; +if (host->ops->get_min_clock) mmc->f_min = host->ops->get_min_clock (host);else if (host->version >=SDHCI_SPEC_300 ) {if (host->clk_mul) { mmc->f_min = (host->max_clk * host->clk_mul) /1024 ; - mmc->f_max = host->max_clk * host->clk_mul; + max_clk = host->max_clk * host->clk_mul; }else mmc->f_min = host->max_clk /SDHCI_MAX_DIV_SPEC_300 ; }else mmc->f_min = host->max_clk /SDHCI_MAX_DIV_SPEC_200 ; +if (!mmc->f_max || (mmc->f_max && (mmc->f_max > max_clk))) + mmc->f_max = max_clk; +if (!(host->quirks &SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK )) { host->timeout_clk = (caps[0 ] &SDHCI_TIMEOUT_CLK_MASK ) >>SDHCI_TIMEOUT_CLK_SHIFT ;
Signature · 1 line
1.9.1
PATCH 4/6 mmc: sdhci-esdhc-imx: remove duplicated dts parsing
In reply to Dong AishengAfter commit 8d86e4fcccf6 ("mmc: sdhci-esdhc-imx: Call mmc_of_parse()"), we do not need those duplicated parsing anymore.
Note: fsl,cd-controller is also deleted due to the driver does not support controller card detection anymore after switch to runtime pm. And there's no user of it right now in device tree.
wp-gpios is kept because we're still support fsl,wp-controller, so we need a way to check if it's gpio wp or controller wp.
--- drivers/mmc/host/sdhci-esdhc-imx.c | 14 -------------- 1 file changed, 14 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +0 -14
@@ -884,27 +884,13 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,struct esdhc_platform_data *boarddata = &imx_data->boarddata;int ret; -if (of_get_property (np,"non-removable" ,NULL )) - boarddata->cd_type =ESDHC_CD_PERMANENT ; - -if (of_get_property (np,"fsl,cd-controller" ,NULL )) - boarddata->cd_type =ESDHC_CD_CONTROLLER ; -if (of_get_property (np,"fsl,wp-controller" ,NULL )) boarddata->wp_type =ESDHC_WP_CONTROLLER ; - boarddata->cd_gpio =of_get_named_gpio (np,"cd-gpios" ,0 ); -if (gpio_is_valid (boarddata->cd_gpio)) - boarddata->cd_type =ESDHC_CD_GPIO ; - boarddata->wp_gpio =of_get_named_gpio (np,"wp-gpios" ,0 );if (gpio_is_valid (boarddata->wp_gpio)) boarddata->wp_type =ESDHC_WP_GPIO ; -of_property_read_u32 (np,"bus-width" , &boarddata->max_bus_width); - -of_property_read_u32 (np,"max-frequency" , &boarddata->f_max); -if (of_find_property (np,"no-1-8-v" ,NULL )) boarddata->support_vsel = false;else
Signature · 1 line
1.9.1
PATCH 4/6 mmc: sdhci-esdhc-imx: remove duplicated dts parsing
Quoted 9 lines · Dong Aisheng
After commit
8d86e4fcccf6("mmc: sdhci-esdhc-imx: Callmmc_of_parse()"), we do not need those duplicated parsing anymore.Note: fsl,cd-controller is also deleted due to the driver does not support controller card detection anymore after switch to runtime pm. And there's no user of it right now in device tree.
wp-gpios is kept because we're still support fsl,wp-controller, so we need a way to check if it's gpio wp or controller wp.
I do not remember the reason why controller based CD stops working after we switch to runtime PM. But if CD stops working for some reason, shouldn't controller based WP stop working for the same reason?
Shawn
Quoted 2 lines
Signed-off-by Dong Aisheng <[email protected]>
PATCH 4/6 mmc: sdhci-esdhc-imx: remove duplicated dts parsing
Quoted 8 lines · Shawn Guo
Quoted 7 lines · Dong Aisheng
After commit
8d86e4fcccf6("mmc: sdhci-esdhc-imx: Callmmc_of_parse()"), we do not need those duplicated parsing anymore.Note: fsl,cd-controller is also deleted due to the driver does not support controller card detection anymore after switch to runtime pm. And there's no user of it right now in device tree.
Quoted 7 lines
Quoted 2 lines
wp-gpios is kept because we're still support fsl,wp-controller, so we need a way to check if it's gpio wp or controller wp.
I do not remember the reason why controller based CD stops working after we switch to runtime PM. But if CD stops working for some reason, shouldn't controller based WP stop working for the same reason?
The main reason may be CD/WP function needs controller clock on. But after enable runtime pm, the clock will be disabled.
See below commit:
commit dacf49223fc680e6d5b5ca4ea43dcd197c1814c5
Author: Sascha Hauer <[email protected]> Date: Fri May 23 14:33:04 2014 +0200
ARM: dts: imx51-babbage: Fix esdhc setup
Since commit 89d7e5c13122 (mmc: sdhci-esdhc-imx: add runtime pm support), controller based card detection / write protection is not supported anymore by esdhc driver. Let's use GPIO for CD/WP on esdhc1 instead.
While at it, fix cd gpio polarity for esdhc2. This is wrong and currently only works because the imx esdhc driver ignores the polarity.
WP is bit different since sdhci_get_ro will call runtime_pm_get to enable clocks. So i guess WP may still work.
I did not test, but i did see there's still a lot users of fsl,wp_controller in device tree which is supposed to work.
There's no fsl,cd-controller users anymore.
Regards
Dong Aisheng
Quoted 4 lines
Shawn
Quoted 2 lines
Signed-off-by Dong Aisheng <[email protected]>
PATCH 5/6 mmc: sdhci-esdhc-imx: clear f_max in boarddata
In reply to Dong AishengAfter commit 8d86e4fcccf6 ("mmc: sdhci-esdhc-imx: Call mmc_of_parse()"), it's not used anymore.
--- drivers/mmc/host/sdhci-esdhc-imx.c | 7 +------ include/linux/platform_data/mmc-esdhc-imx.h | 1 - 2 files changed, 1 insertion(+), 7 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +1 -6
@@ -581,13 +581,8 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)static unsigned int esdhc_pltfm_get_max_clock (struct sdhci_host *host) {struct sdhci_pltfm_host *pltfm_host =sdhci_priv (host); -struct pltfm_imx_data *imx_data = pltfm_host->priv; -struct esdhc_platform_data *boarddata = &imx_data->boarddata; -if (boarddata->f_max && (boarddata->f_max < pltfm_host->clock)) -return boarddata->f_max; -else -return pltfm_host->clock; +return pltfm_host->clock; }static unsigned int esdhc_pltfm_get_min_clock (struct sdhci_host *host)
include/linux/platform_data/mmc-esdhc-imx.h +0 -1
@@ -43,7 +43,6 @@ struct esdhc_platform_data {enum wp_types wp_type;enum cd_types cd_type;int max_bus_width; -unsigned int f_max;bool support_vsel;unsigned int delay_line; };
Signature · 1 line
1.9.1
PATCH 6/6 dts: mmc: fsl-imx-esdhc: remove fsl, cd-controller support
In reply to Dong AishengIt's not supported by driver anymore after using runtime pm and there's no user of it, so delete it now.
--- Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt | 2 -- 1 file changed, 2 deletions(-)
Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt +0 -2
@@ -17,7 +17,6 @@ Required properties: "fsl,imx6sx-usdhc" Optional properties: -- fsl,cd-controller : Indicate to use controller internal card detection - fsl,wp-controller : Indicate to use controller internal write protection - fsl,delay-line : Specify the number of delay cells for override mode. This is used to set the clock delay for DLL(Delay Line) on override mode
@@ -35,7 +34,6 @@ esdhc at 70004000 { compatible = "fsl,imx51-esdhc"; reg = <0x70004000 0x4000>; interrupts = <1>; - fsl,cd-controller; fsl,wp-controller; };
Signature · 1 line
1.9.1
Quoted 5 lines · Dong Aisheng <aisheng.dong <at> freescale.com> writes:
Commit 8d86e4f mmc: sdhci-esdhc-imx: Call
mmc_of_parse()seriously break the sd card detect/write protect function of most IMX platforms since a lot of GPIO flags of cd-gpios/wp-gpios are wrongly set in devicetree.
...
Hi,
I have tested this patches on a nitrogen board and two custom boards with imx6dl. They seem to be working OK. I had continuous mmc interrupts before they were applied.
Will they be included in 4.2 or is it to late for that?
Best regards,
Johan
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
In reply to Dong AishengQuoted 6 lines · Dong Aisheng
Commit 8d86e4f mmc: sdhci-esdhc-imx: Call
mmc_of_parse()seriously break the sd card detect/write protect function of most IMX platforms since a lot of GPIO flags of cd-gpios/wp-gpios are wrongly set in devicetree.It breaks the following things:
My opinion would be that we revert the commit 8d86e4f for v4.2, as it's so broken.
Quoted 18 lines
1) cd-gpio function enable
Most IMX boards GPIO card detect function becomes unwork and using card polling mode instead by mistake.
OpenningCONFIG_MMC_DEBUGwill easily see that.
It is caused by the default quirkSDHCI_QUIRK_BROKEN_CARD_DETECTIONis not cleared for dt platform by commit 8d86e4f, thenMMC_CAP_NEEDS_POLLis wrongly set.
Patch 1~2 is to fix it.2) cd-gpio/wp-gpio polarity
Before commit 8d86e4f, we do not use GPIO flags passed from devicetree. But after it, GPIO flags specfied in dt becomes valid and will affect the normal cd/wp function.This is fixed by another dts fix patch series:
[PATCH 0/5] dts: imx: fix sd card gpio polarity specified in device tree
http://www.spinics.net/lists/arm-kernel/msg426156.html
Quoted 7 lines
Since it needs to change a lot board dts files, so i cook the patch based on Shawn/for-next branch to make sure all board dts file are changed in the same time.
The dts fix patches series should be picked before this series since the driver fixes depend on the correct dts(correct GPIO polarity) to work properly.
So this is a device tree ABI breakage. All the existing DTBs already installed on devices will be broken with the new kernel. You can argue that the device trees are buggy and we're fixing bugs. But if a bugfix breaks things, we should be conservative. Here is my suggestion:
1. Revert commit 8d86e4f for v4.2
2. I queue up your dts fixing series for v4.2
3. Keep things as they are for a few releases and then consider to move to mmc_of_parse() with the hope that those buggy DTBs have been upgraded during that period.
Shawn
Quoted 49 lines
3) max-frequency not work in sdhci
mmc_of_parsewill parse the max-frequency from devicetree, however, its value will be overwritten by common sdhci driver finnally,so it does not actually work.
Patch 3 is to fix it.4) patch 4~6 clear unneeded parsing code after switch to
mmc_of_parse().The series is based on ulf/next tree.
I also CCed this patch series to all board maintainer related in this change, hope they can help test it.
I only tested some freescale IMX boards.BTW, There's indeed a problem here that dts fix patch series needs to be picked before this series, but it can't go through Ulf tree since some board dts changed does not in Ulf's tree.
And this patch series also can not go through Shawn's tree, since mmc part change depends on some patches in Ulf tree. Two series are cross dependant.
Hope you guy can find a solution for it.Those two patch series is an important fix, hopefully can catch up the final 4.1 merge window.
Dong Aisheng (6):
mmc: sdhci-esdhc-imx: fix cd regression for dt platform
mmc: sdhci-esdhc-imx: move all non dt probe code into one function
mmc: sdhci: make max-frequency property in device tree work
mmc: sdhci-esdhc-imx: remove duplicated dts parsing
mmc: sdhci-esdhc-imx: clearf_maxin boarddata
dts: mmc: fsl-imx-esdhc: remove fsl,cd-controller support.../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 2 - drivers/mmc/host/sdhci-esdhc-imx.c | 210 ++++++++++----------- drivers/mmc/host/sdhci.c | 9 +- include/linux/platform_data/mmc-esdhc-imx.h | 1 - 4 files changed, 111 insertions(+), 111 deletions(-)Signature · 7 lines
1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
Quoted 11 lines · Shawn Guo
Quoted 6 lines · Dong Aisheng
Commit 8d86e4f mmc: sdhci-esdhc-imx: Call
mmc_of_parse()seriously break the sd card detect/write protect function of most IMX platforms since a lot of GPIO flags of cd-gpios/wp-gpios are wrongly set in devicetree.It breaks the following things:
My opinion would be that we revert the commit 8d86e4f for v4.2, as it's so broken.
It may be a bit difficult since there're a few successive patches already in Ulf's tree based on 8d86e4f.
Most are also fixes for 8d86e4f by Fabio.
Quoted 22 lines
Quoted 22 lines
1) cd-gpio function enable
Most IMX boards GPIO card detect function becomes unwork and using card polling mode instead by mistake.
OpenningCONFIG_MMC_DEBUGwill easily see that.
It is caused by the default quirkSDHCI_QUIRK_BROKEN_CARD_DETECTIONis not cleared for dt platform by commit 8d86e4f, thenMMC_CAP_NEEDS_POLLis wrongly set.
Patch 1~2 is to fix it.2) cd-gpio/wp-gpio polarity
Before commit 8d86e4f, we do not use GPIO flags passed from devicetree. But after it, GPIO flags specfied in dt becomes valid and will affect the normal cd/wp function.This is fixed by another dts fix patch series:
[PATCH 0/5] dts: imx: fix sd card gpio polarity specified in device tree
http://www.spinics.net/lists/arm-kernel/msg426156.htmlSince it needs to change a lot board dts files, so i cook the patch based on Shawn/for-next branch to make sure all board dts file are changed in the same time.
Quoted 9 lines
Quoted 3 lines
The dts fix patches series should be picked before this series since the driver fixes depend on the correct dts(correct GPIO polarity) to work properly.
So this is a device tree ABI breakage. All the existing DTBs already installed on devices will be broken with the new kernel. You can argue that the device trees are buggy and we're fixing bugs. But if a bugfix breaks things, we should be conservative. Here is my suggestion:
I agree that it's a pain, but we may have to suffer. Your below suggestion can't eventually avoid this pain. The old dts will finally not work again if upgrade to new kernel.
As currently dts and kernel tree are still not separated, it may be not a serious issue and usually user uses new kernel and dts together.
Quoted 6 lines
1. Revert commit 8d86e4f for v4.2
2. I queue up your dts fixing series for v4.2
3. Keep things as they are for a few releases and then consider to move tommc_of_parse()with the hope that those buggy DTBs have been upgraded during that period.
I agree with you.
One difference is that i'd like to fix it ASAP without reverting 8d86e4f due to more patches depends on it is already there as i mentioned above.. Revert it may need to revert a lot others.
The pain is that v4.1 is left broken.
How about you queue dts fixing first for v4.2 rc1, then we can fix the driver in v4.2 rc2?
Regards
Dong Aisheng
Quoted 51 lines
Shawn
Quoted 49 lines
3) max-frequency not work in sdhci
mmc_of_parsewill parse the max-frequency from devicetree, however, its value will be overwritten by common sdhci driver finnally,so it does not actually work.
Patch 3 is to fix it.4) patch 4~6 clear unneeded parsing code after switch to
mmc_of_parse().The series is based on ulf/next tree.
I also CCed this patch series to all board maintainer related in this change, hope they can help test it.
I only tested some freescale IMX boards.BTW, There's indeed a problem here that dts fix patch series needs to be picked before this series, but it can't go through Ulf tree since some board dts changed does not in Ulf's tree.
And this patch series also can not go through Shawn's tree, since mmc part change depends on some patches in Ulf tree. Two series are cross dependant.
Hope you guy can find a solution for it.Those two patch series is an important fix, hopefully can catch up the final 4.1 merge window.
Dong Aisheng (6):
mmc: sdhci-esdhc-imx: fix cd regression for dt platform
mmc: sdhci-esdhc-imx: move all non dt probe code into one function
mmc: sdhci: make max-frequency property in device tree work
mmc: sdhci-esdhc-imx: remove duplicated dts parsing
mmc: sdhci-esdhc-imx: clearf_maxin boarddata
dts: mmc: fsl-imx-esdhc: remove fsl,cd-controller support.../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 2 - drivers/mmc/host/sdhci-esdhc-imx.c | 210 ++++++++++----------- drivers/mmc/host/sdhci.c | 9 +- include/linux/platform_data/mmc-esdhc-imx.h | 1 - 4 files changed, 111 insertions(+), 111 deletions(-)Signature · 7 lines
1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
Quoted 6 lines · Dong Aisheng
I agree with you.
One difference is that i'd like to fix it ASAP without reverting 8d86e4f due to more patches depends on it is already there as i mentioned above.. Revert it may need to revert a lot others.The pain is that v4.1 is left broken.
Oh, crap! I thought it's been there with only v4.2-rc1, and did not know v4.1 is already broken. In that case, reverting 8d86e4f isn't the best option. I suggest you rebase the dts series on top of v4.2-rc1, and send it via mmc tree.
Shawn
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
Am 09.07.2015 um 15:27 schrieb Shawn Guo:
Quoted 4 lines
Oh, crap! I thought it's been there with only v4.2-rc1, and did not know v4.1 is already broken. In that case, reverting 8d86e4f isn't the best option. I suggest you rebase the dts series on top of v4.2-rc1, and send it via mmc tree.
Yeah, I already stumbled across that. Fortunately, I dont need it at all, so I just removed it from dts:
(quoted diff) +0 -0
b/arch/arm/boot/dts/imx53-tqma53.dtsi
index 2dea6dc..3c545e3 100644
arch/arm/boot/dts/imx53-tqma53.dtsi +0 -1
@@ -41,7 +41,6 @@ pinctrl-0 = <&pinctrl_esdhc2>, <&pinctrl_esdhc2_cdwp>; vmmc-supply = <®_3p3v>; - wp-gpios = <&gpio1 2 0>; cd-gpios = <&gpio1 4 0>; status = "disabled"; };
--mtx
https://www.facebook.com/MELAG.Medizintechnik
[http://www.melag.de/fbbanner.png]<https://www.facebook.com/MELAG.Medizintechnik>
MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B
Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur f?r einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschlie?lich f?r denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, d?rfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrt?mlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte l?schen Sie in diesem Fall diese Nachricht und alle Anh?nge, ohne eine Kopie zu behalten. Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy.
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
In reply to Shawn GuoQuoted 6 lines · Shawn Guo
Quoted 5 lines · Dong Aisheng
I agree with you.
One difference is that i'd like to fix it ASAP without reverting 8d86e4f due to more patches depends on it is already there as i mentioned above.. Revert it may need to revert a lot others.
Quoted 7 lines
Quoted 1 line
The pain is that v4.1 is left broken.
Oh, crap! I thought it's been there with only v4.2-rc1, and did not know v4.1 is already broken. In that case, reverting 8d86e4f isn't the best option. I suggest you rebase the dts series on top of v4.2-rc1, and send it via mmc tree.
Yes, will go that way.
Then you may need help take care of new dts going through your tree to make sure no broken code in again.
Thanks
Regards
Dong Aisheng
Quoted 1 line
Shawn
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
Quoted 15 lines · Dong Aisheng
Quoted 6 lines · Shawn Guo
Quoted 5 lines · Dong Aisheng
I agree with you.
One difference is that i'd like to fix it ASAP without reverting 8d86e4f due to more patches depends on it is already there as i mentioned above.. Revert it may need to revert a lot others.Quoted 7 lines
Quoted 1 line
The pain is that v4.1 is left broken.
Oh, crap! I thought it's been there with only v4.2-rc1, and did not know v4.1 is already broken. In that case, reverting 8d86e4f isn't the best option. I suggest you rebase the dts series on top of v4.2-rc1, and send it via mmc tree.
Quoted 4 lines
Yes, will go that way.
Then you may need help take care of new dts going through your tree to make sure no broken code in again.
Dong,
I use a microSD card (mmc0) with the root filesystem on a tbs2910 board. I cannot see any difference between linux-4.0 and linux-4.1 (vanilla kernel with corresponding dtb), there is absolutely no problem with card detect.
Can you please explain what exactly is broken in linux-4.1?
Thanks,
Soeren
Quoted 6 lines
Thanks
Regards
Dong AishengQuoted 1 line
Shawn
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
Quoted 23 lines · Soeren Moch
Quoted 15 lines · Dong Aisheng
Quoted 6 lines · Shawn Guo
Quoted 5 lines · Dong Aisheng
I agree with you.
One difference is that i'd like to fix it ASAP without reverting 8d86e4f due to more patches depends on it is already there as i mentioned above.. Revert it may need to revert a lot others.Quoted 7 lines
Quoted 1 line
The pain is that v4.1 is left broken.
Oh, crap! I thought it's been there with only v4.2-rc1, and did not know v4.1 is already broken. In that case, reverting 8d86e4f isn't the best option. I suggest you rebase the dts series on top of v4.2-rc1, and send it via mmc tree.
Quoted 4 lines
Yes, will go that way.
Then you may need help take care of new dts going through your tree to make sure no broken code in again.
Quoted 9 lines
Dong,
I use a microSD card (mmc0) with the root filesystem on a tbs2910 board. I cannot see any difference between linux-4.0 and linux-4.1 (vanilla kernel with corresponding dtb), there is absolutely no problem with card detect.
Can you please explain what exactly is broken in linux-4.1?
The CD function is broken.
If you enable CONFIG_MMC_DEBUG, you will see the MMC core keep polling the card because MMC_CAP_NEEDS_POLL is wrongly set.
You can refer to cover-letter for more detailed explaination.
If you're using a non-removable card(having non-removable property in dts), you may not have this issue.
Regards
Dong Aisheng
Quoted 9 lines
Thanks,
SoerenQuoted 6 lines
Thanks
Regards
Dong AishengQuoted 1 line
Shawn
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
In reply to Dong AishengQuoted 15 lines · Dong Aisheng
Quoted 6 lines · Shawn Guo
Quoted 5 lines · Dong Aisheng
I agree with you.
One difference is that i'd like to fix it ASAP without reverting 8d86e4f due to more patches depends on it is already there as i mentioned above.. Revert it may need to revert a lot others.Quoted 7 lines
Quoted 1 line
The pain is that v4.1 is left broken.
Oh, crap! I thought it's been there with only v4.2-rc1, and did not know v4.1 is already broken. In that case, reverting 8d86e4f isn't the best option. I suggest you rebase the dts series on top of v4.2-rc1, and send it via mmc tree.
Quoted 4 lines
Yes, will go that way.
Then you may need help take care of new dts going through your tree to make sure no broken code in again.
Dong, Shawn,
I am happy to help out, I assume you want this to go as fixes for 4.2rc[n].
Correct me if I am wrong, you want me to apply the DTS patches prior the MMC patches. I tried applying the DTS patches on 4.2rc3 - it still works nicely so no re-base is needed.
So, then I need an ack from Shawn for the DTS series...
Kind regards
Uffe
cover mmc: sdhci-esdhci-imx: fix cd/wp regression
Quoted 23 lines · Ulf Hansson
Quoted 15 lines · Dong Aisheng
Quoted 6 lines · Shawn Guo
Quoted 5 lines · Dong Aisheng
I agree with you.
One difference is that i'd like to fix it ASAP without reverting 8d86e4f due to more patches depends on it is already there as i mentioned above.. Revert it may need to revert a lot others.Quoted 7 lines
Quoted 1 line
The pain is that v4.1 is left broken.
Oh, crap! I thought it's been there with only v4.2-rc1, and did not know v4.1 is already broken. In that case, reverting 8d86e4f isn't the best option. I suggest you rebase the dts series on top of v4.2-rc1, and send it via mmc tree.
Quoted 4 lines
Yes, will go that way.
Then you may need help take care of new dts going through your tree to make sure no broken code in again.
Dong, Shawn,
Quoted 9 lines
I am happy to help out, I assume you want this to go as fixes for 4.2rc[n].
Correct me if I am wrong, you want me to apply the DTS patches prior the MMC patches. I tried applying the DTS patches on 4.2rc3 - it still works nicely so no re-base is needed.
So, then I need an ack from Shawn for the DTS series...
Hi Ulf,
I have merged the two seris into one (dts changes is before controller changes) and rebase based on your ulf/next branch.
The updated patch series is just sent out for review.
Shawn,
Please help review and ACK for Ulf to apply.
Regards
Dong Aisheng
Quoted 2 lines
Kind regards
Uffe