| Message ID | 1461653646-22235-3-git-send-email-van.freenix@gmail.com |
|---|---|
| State | Accepted |
| Commit | 1483151e84161449c3f652a751a04e06b0723bff |
| Delegated to: | Jaehoon Chung |
| Headers | show |
Hi Peng, On Tue, Apr 26, 2016 at 3:54 AM, Peng Fan <van.freenix@gmail.com> wrote: > Introudce wp_enable. If want to check WPSPL, then in board code, > need to set wp_enable to 1. > > Take i.MX6UL for example, to some boards, they do not use WP singal, > so they does not configure USDHC1_WP_SELECT_INPUT, and its default > value is 0(GPIO1_IO02). However GPIO1_IO02 is muxed for i2c usage and > SION bit set. So USDHC controller can always get wp signal and WPSPL > shows write protect and blocks driver continuing. This is not what > we want to see, so add wp_enable, and if set to 0, just omit the > WPSPL checking and this does not effect normal working of usdhc > controller. > > To DT case, add wp_gpio, if there is wp-gpios provided in dts, > wp_enable is set to 1; if no, set to 0. > > Signed-off-by: Peng Fan <van.freenix@gmail.com> > Cc: Pantelis Antoniou <panto@antoniou-consulting.com> > Cc: York Sun <york.sun@nxp.com> > Cc: Stefano Babic <sbabic@denx.de> Just saw this issue on a mx6ul pico board: after adding I2C support then the eMMC could not longer be written: => saveenv Saving Environment to MMC... Writing to MMC(0)... The SD card is locked. Can not write to a locked card. mmc write failed failed Your patch allows me to write to the eMMC succesfully: Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
Hi Peng, On Tue, Jun 14, 2016 at 8:01 PM, Fabio Estevam <festevam@gmail.com> wrote: > Just saw this issue on a mx6ul pico board: after adding I2C support > then the eMMC could not longer be written: > > => saveenv > Saving Environment to MMC... > Writing to MMC(0)... > The SD card is locked. Can not write to a locked card. > > mmc write failed > failed > > Your patch allows me to write to the eMMC succesfully: > > Tested-by: Fabio Estevam <fabio.estevam@nxp.com> Looks like this is an issue with the MX6UL IOMUXC, not the esdhc driver itself, so maybe we should not do this change for all other SoCs. If I manually do: => mw.l 20E066C 2 Then 'saveenv' works fine. Shouldn't we fix this in the MX6UL IOMUXC code instead?
Hi Fabio, On Tue, Jun 14, 2016 at 08:23:27PM -0300, Fabio Estevam wrote: >Hi Peng, > >On Tue, Jun 14, 2016 at 8:01 PM, Fabio Estevam <festevam@gmail.com> wrote: > >> Just saw this issue on a mx6ul pico board: after adding I2C support >> then the eMMC could not longer be written: >> >> => saveenv >> Saving Environment to MMC... >> Writing to MMC(0)... >> The SD card is locked. Can not write to a locked card. >> >> mmc write failed >> failed >> >> Your patch allows me to write to the eMMC succesfully: >> >> Tested-by: Fabio Estevam <fabio.estevam@nxp.com> Thanks. This patch set was posted some time ago. > >Looks like this is an issue with the MX6UL IOMUXC, not the esdhc >driver itself, so maybe we should not do this change for all other >SoCs. > >If I manually do: >=> mw.l 20E066C 2 >Then 'saveenv' works fine. > >Shouldn't we fix this in the MX6UL IOMUXC code instead? No. We can not avoid such issue for now. You changed register 20e066c'value to 2 2 means "CSI_DATA04_ALT8 — Selecting Pad: CSI_DATA04 for Mode: ALT8" Look at "Figure 31-3. Daisy chain illustration" of i.MX6UL RM, if changed to 2, that means you let CSI_DATA04 pad goes into usdhc1_wp. select input can not be closed or disabled, which will cause issues like what you met. Even worse, some one may need to redesign their board to avoid pin conflict. Regards, Peng.
Hi Peng, On Tue, Jun 14, 2016 at 10:17 PM, Peng Fan <van.freenix@gmail.com> wrote: > No. We can not avoid such issue for now. You changed register 20e066c'value to 2 > 2 means "CSI_DATA04_ALT8 — Selecting Pad: CSI_DATA04 for Mode: ALT8" > > Look at "Figure 31-3. Daisy chain illustration" of i.MX6UL RM, if changed to 2, > that means you let CSI_DATA04 pad goes into usdhc1_wp. Yes, this was just a quick hack. I am not proposing this workaround :-) > select input can not be closed or disabled, which will cause issues like what > you met. Even worse, some one may need to redesign their board to avoid pin > conflict. Yes, I understood the problem and don't have any alternative suggestion at the moment. Pantelis/Stefano/Tom/York, Any feedback on this series, please?
On Tue, Jun 14, 2016 at 10:17 PM, Peng Fan <van.freenix@gmail.com> wrote: >>> Your patch allows me to write to the eMMC succesfully: >>> >>> Tested-by: Fabio Estevam <fabio.estevam@nxp.com> > > Thanks. This patch set was posted some time ago. Care to resend this series, please? Thanks
Hi Fabio, On Tue, Jun 14, 2016 at 10:46:15PM -0300, Fabio Estevam wrote: >On Tue, Jun 14, 2016 at 10:17 PM, Peng Fan <van.freenix@gmail.com> wrote: > >>>> Your patch allows me to write to the eMMC succesfully: >>>> >>>> Tested-by: Fabio Estevam <fabio.estevam@nxp.com> >> >> Thanks. This patch set was posted some time ago. > >Care to resend this series, please? Ok. I will resend this series with your tested-by tag. Thanks, Peng. > >Thanks
On 06/14/2016 06:35 PM, Fabio Estevam wrote: > Hi Peng, > > On Tue, Jun 14, 2016 at 10:17 PM, Peng Fan <van.freenix@gmail.com> wrote: > >> No. We can not avoid such issue for now. You changed register 20e066c'value to 2 >> 2 means "CSI_DATA04_ALT8 — Selecting Pad: CSI_DATA04 for Mode: ALT8" >> >> Look at "Figure 31-3. Daisy chain illustration" of i.MX6UL RM, if changed to 2, >> that means you let CSI_DATA04 pad goes into usdhc1_wp. > > Yes, this was just a quick hack. I am not proposing this workaround :-) > >> select input can not be closed or disabled, which will cause issues like what >> you met. Even worse, some one may need to redesign their board to avoid pin >> conflict. > > Yes, I understood the problem and don't have any alternative > suggestion at the moment. > > Pantelis/Stefano/Tom/York, > > Any feedback on this series, please? > It looks good to me, but I didn't test it. York
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index b69c766..4dd1765 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -92,7 +92,9 @@ struct fsl_esdhc { * Following is used when Driver Model is enabled for MMC * @dev: pointer for the device * @non_removable: 0: removable; 1: non-removable + * @wp_enable: 1: enable checking wp; 0: no check * @cd_gpio: gpio for card detection + * @wp_gpio: gpio for write protection */ struct fsl_esdhc_priv { struct fsl_esdhc *esdhc_regs; @@ -102,7 +104,9 @@ struct fsl_esdhc_priv { struct mmc *mmc; struct udevice *dev; int non_removable; + int wp_enable; struct gpio_desc cd_gpio; + struct gpio_desc wp_gpio; }; /* Return the XFERTYP flags for a given command and data packet */ @@ -246,9 +250,12 @@ static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data) #endif if (wml_value > WML_WR_WML_MAX) wml_value = WML_WR_WML_MAX_VAL; - if ((esdhc_read32(®s->prsstat) & PRSSTAT_WPSPL) == 0) { - printf("\nThe SD card is locked. Can not write to a locked card.\n\n"); - return TIMEOUT; + if (priv->wp_enable) { + if ((esdhc_read32(®s->prsstat) & + PRSSTAT_WPSPL) == 0) { + printf("\nThe SD card is locked. Can not write to a locked card.\n\n"); + return TIMEOUT; + } } esdhc_clrsetbits32(®s->wml, WML_WR_WML_MASK, @@ -747,6 +754,7 @@ static int fsl_esdhc_cfg_to_priv(struct fsl_esdhc_cfg *cfg, priv->esdhc_regs = (struct fsl_esdhc *)(unsigned long)(cfg->esdhc_base); priv->bus_width = cfg->max_bus_width; priv->sdhc_clk = cfg->sdhc_clk; + priv->wp_enable = cfg->wp_enable; return 0; }; @@ -989,6 +997,13 @@ static int fsl_esdhc_probe(struct udevice *dev) &priv->cd_gpio, GPIOD_IS_IN); } + priv->wp_enable = 1; + + ret = gpio_request_by_name_nodev(fdt, node, "wp-gpios", 0, + &priv->wp_gpio, GPIOD_IS_IN); + if (ret) + priv->wp_enable = 0; + /* * TODO: * Because lack of clk driver, if SDHC clk is not enabled, diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index 78c67c8..c6f4666 100644 --- a/include/fsl_esdhc.h +++ b/include/fsl_esdhc.h @@ -177,6 +177,7 @@ struct fsl_esdhc_cfg { phys_addr_t esdhc_base; u32 sdhc_clk; u8 max_bus_width; + u8 wp_enable; struct mmc_config cfg; };
Introudce wp_enable. If want to check WPSPL, then in board code, need to set wp_enable to 1. Take i.MX6UL for example, to some boards, they do not use WP singal, so they does not configure USDHC1_WP_SELECT_INPUT, and its default value is 0(GPIO1_IO02). However GPIO1_IO02 is muxed for i2c usage and SION bit set. So USDHC controller can always get wp signal and WPSPL shows write protect and blocks driver continuing. This is not what we want to see, so add wp_enable, and if set to 0, just omit the WPSPL checking and this does not effect normal working of usdhc controller. To DT case, add wp_gpio, if there is wp-gpios provided in dts, wp_enable is set to 1; if no, set to 0. Signed-off-by: Peng Fan <van.freenix@gmail.com> Cc: Pantelis Antoniou <panto@antoniou-consulting.com> Cc: York Sun <york.sun@nxp.com> Cc: Stefano Babic <sbabic@denx.de> --- drivers/mmc/fsl_esdhc.c | 21 ++++++++++++++++++--- include/fsl_esdhc.h | 1 + 2 files changed, 19 insertions(+), 3 deletions(-)