| Message ID | 20170704193133.87092-9-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Jaehoon Chung |
| Headers | show |
On Tue, Jul 04, 2017 at 01:31:25PM -0600, Simon Glass wrote: > At present the driver-private data is obtained in various functions by > various means. With driver model this is provided automatically. Without > driver model it comes from a C array declared at the top of the file. > > Adjust internal functions so that they are passed the private data as > a parameter, allowing the caller to obtain it using either means. > > Signed-off-by: Simon Glass <sjg@chromium.org> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Thanks! Maxime
Hi Simon, On Wed, Jul 5, 2017 at 3:31 AM, Simon Glass <sjg@chromium.org> wrote: > At present the driver-private data is obtained in various functions by > various means. With driver model this is provided automatically. Without > driver model it comes from a C array declared at the top of the file. > > Adjust internal functions so that they are passed the private data as > a parameter, allowing the caller to obtain it using either means. > > Signed-off-by: Simon Glass <sjg@chromium.org> eMMC is currently broken for sunxi on my Orangepi Plus 2E. I've narrowed it down to this patch. It seems the driver or device is now referencing the wrong controller. On versions before this patch, for MMC1 (or eMMC): => mmc dev 1 switch to partitions #0, OK mmc1(part 0) is current device => mmc info Device: SUNXI SD/MMC Manufacturer ID: 15 OEM: 100 Name: AWPD3 Tran Speed: 52000000 Rd Block Len: 512 MMC version 5.0 High Capacity: Yes Capacity: 14.6 GiB Bus Width: 8-bit Erase Group Size: 512 KiB HC WP Group Size: 8 MiB User Capacity: 14.6 GiB WRREL Boot Capacity: 4 MiB ENH RPMB Capacity: 4 MiB ENH On later versions I get: => mmc dev 1 switch to partitions #0, OK mmc1 is current device => mmc info Device: SUNXI SD/MMC Manufacturer ID: 27 OEM: 5048 Name: SD08G Tran Speed: 50000000 Rd Block Len: 512 SD version 3.0 High Capacity: Yes Capacity: 7.4 GiB Bus Width: 1-bit Erase Group Size: 512 Bytes For reference, mmc0 looks like: => mmc dev 0 switch to partitions #0, OK mmc0 is current device => mmc info Device: SUNXI SD/MMC Manufacturer ID: 27 OEM: 5048 Name: SD08G Tran Speed: 50000000 Rd Block Len: 512 SD version 3.0 High Capacity: Yes Capacity: 7.4 GiB Bus Width: 4-bit Erase Group Size: 512 Bytes So it seems somewhere down the line, the driver is getting passed the wrong set of priv data. Regards ChenYu
Hi Chen-Yu, On 8 August 2017 at 21:27, Chen-Yu Tsai <wens@csie.org> wrote: > Hi Simon, > > On Wed, Jul 5, 2017 at 3:31 AM, Simon Glass <sjg@chromium.org> wrote: >> At present the driver-private data is obtained in various functions by >> various means. With driver model this is provided automatically. Without >> driver model it comes from a C array declared at the top of the file. >> >> Adjust internal functions so that they are passed the private data as >> a parameter, allowing the caller to obtain it using either means. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> > > eMMC is currently broken for sunxi on my Orangepi Plus 2E. > I've narrowed it down to this patch. > > It seems the driver or device is now referencing the wrong > controller. On versions before this patch, for MMC1 (or eMMC): > > => mmc dev 1 > switch to partitions #0, OK > mmc1(part 0) is current device > => mmc info > Device: SUNXI SD/MMC > Manufacturer ID: 15 > OEM: 100 > Name: AWPD3 > Tran Speed: 52000000 > Rd Block Len: 512 > MMC version 5.0 > High Capacity: Yes > Capacity: 14.6 GiB > Bus Width: 8-bit > Erase Group Size: 512 KiB > HC WP Group Size: 8 MiB > User Capacity: 14.6 GiB WRREL > Boot Capacity: 4 MiB ENH > RPMB Capacity: 4 MiB ENH > > > On later versions I get: > > => mmc dev 1 > switch to partitions #0, OK > mmc1 is current device > => mmc info > Device: SUNXI SD/MMC > Manufacturer ID: 27 > OEM: 5048 > Name: SD08G > Tran Speed: 50000000 > Rd Block Len: 512 > SD version 3.0 > High Capacity: Yes > Capacity: 7.4 GiB > Bus Width: 1-bit > Erase Group Size: 512 Bytes > > > For reference, mmc0 looks like: > > => mmc dev 0 > switch to partitions #0, OK > mmc0 is current device > => mmc info > Device: SUNXI SD/MMC > Manufacturer ID: 27 > OEM: 5048 > Name: SD08G > Tran Speed: 50000000 > Rd Block Len: 512 > SD version 3.0 > High Capacity: Yes > Capacity: 7.4 GiB > Bus Width: 4-bit > Erase Group Size: 512 Bytes > > > So it seems somewhere down the line, the driver is getting > passed the wrong set of priv data. Are you sure it was this patch? The ordering may have changed because there was a strange hack in the code before. There was some discussion about it but unfortunately I cannot find the thread right now. Can you take a look? Regards, Simon
On Tue, Aug 15, 2017 at 5:35 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Chen-Yu, > > On 8 August 2017 at 21:27, Chen-Yu Tsai <wens@csie.org> wrote: >> Hi Simon, >> >> On Wed, Jul 5, 2017 at 3:31 AM, Simon Glass <sjg@chromium.org> wrote: >>> At present the driver-private data is obtained in various functions by >>> various means. With driver model this is provided automatically. Without >>> driver model it comes from a C array declared at the top of the file. >>> >>> Adjust internal functions so that they are passed the private data as >>> a parameter, allowing the caller to obtain it using either means. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >> >> eMMC is currently broken for sunxi on my Orangepi Plus 2E. >> I've narrowed it down to this patch. >> >> It seems the driver or device is now referencing the wrong >> controller. On versions before this patch, for MMC1 (or eMMC): >> >> => mmc dev 1 >> switch to partitions #0, OK >> mmc1(part 0) is current device >> => mmc info >> Device: SUNXI SD/MMC >> Manufacturer ID: 15 >> OEM: 100 >> Name: AWPD3 >> Tran Speed: 52000000 >> Rd Block Len: 512 >> MMC version 5.0 >> High Capacity: Yes >> Capacity: 14.6 GiB >> Bus Width: 8-bit >> Erase Group Size: 512 KiB >> HC WP Group Size: 8 MiB >> User Capacity: 14.6 GiB WRREL >> Boot Capacity: 4 MiB ENH >> RPMB Capacity: 4 MiB ENH >> >> >> On later versions I get: >> >> => mmc dev 1 >> switch to partitions #0, OK >> mmc1 is current device >> => mmc info >> Device: SUNXI SD/MMC >> Manufacturer ID: 27 >> OEM: 5048 >> Name: SD08G >> Tran Speed: 50000000 >> Rd Block Len: 512 >> SD version 3.0 >> High Capacity: Yes >> Capacity: 7.4 GiB >> Bus Width: 1-bit >> Erase Group Size: 512 Bytes >> >> >> For reference, mmc0 looks like: >> >> => mmc dev 0 >> switch to partitions #0, OK >> mmc0 is current device >> => mmc info >> Device: SUNXI SD/MMC >> Manufacturer ID: 27 >> OEM: 5048 >> Name: SD08G >> Tran Speed: 50000000 >> Rd Block Len: 512 >> SD version 3.0 >> High Capacity: Yes >> Capacity: 7.4 GiB >> Bus Width: 4-bit >> Erase Group Size: 512 Bytes >> >> >> So it seems somewhere down the line, the driver is getting >> passed the wrong set of priv data. > > Are you sure it was this patch? > > The ordering may have changed because there was a strange hack in the > code before. There was some discussion about it but unfortunately I > cannot find the thread right now. Can you take a look? Indeed it was this patch. Maxime's latest patch [1] fixes it. And looks like the hacks are on their way out as well. ChenYu [1] https://lists.denx.de/pipermail/u-boot/2017-August/303443.html
diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c index 9276a29d76..bd41fbb752 100644 --- a/drivers/mmc/sunxi_mmc.c +++ b/drivers/mmc/sunxi_mmc.c @@ -176,9 +176,8 @@ static int mmc_clk_io_on(int sdc_no) return mmc_set_mod_clk(priv, 24000000); } -static int mmc_update_clk(struct mmc *mmc) +static int mmc_update_clk(struct sunxi_mmc_priv *priv) { - struct sunxi_mmc_priv *priv = mmc->priv; unsigned int cmd; unsigned timeout_msecs = 2000; @@ -198,15 +197,14 @@ static int mmc_update_clk(struct mmc *mmc) return 0; } -static int mmc_config_clock(struct mmc *mmc) +static int mmc_config_clock(struct sunxi_mmc_priv *priv, struct mmc *mmc) { - struct sunxi_mmc_priv *priv = mmc->priv; unsigned rval = readl(&priv->reg->clkcr); /* Disable Clock */ rval &= ~SUNXI_MMC_CLK_ENABLE; writel(rval, &priv->reg->clkcr); - if (mmc_update_clk(mmc)) + if (mmc_update_clk(priv)) return -1; /* Set mod_clk to new rate */ @@ -220,21 +218,20 @@ static int mmc_config_clock(struct mmc *mmc) /* Re-enable Clock */ rval |= SUNXI_MMC_CLK_ENABLE; writel(rval, &priv->reg->clkcr); - if (mmc_update_clk(mmc)) + if (mmc_update_clk(priv)) return -1; return 0; } -static int sunxi_mmc_set_ios(struct mmc *mmc) +static int sunxi_mmc_set_ios_common(struct sunxi_mmc_priv *priv, + struct mmc *mmc) { - struct sunxi_mmc_priv *priv = mmc->priv; - debug("set ios: bus_width: %x, clock: %d\n", mmc->bus_width, mmc->clock); /* Change clock first */ - if (mmc->clock && mmc_config_clock(mmc) != 0) { + if (mmc->clock && mmc_config_clock(priv, mmc) != 0) { priv->fatal_err = 1; return -EINVAL; } @@ -261,9 +258,9 @@ static int sunxi_mmc_core_init(struct mmc *mmc) return 0; } -static int mmc_trans_data_by_cpu(struct mmc *mmc, struct mmc_data *data) +static int mmc_trans_data_by_cpu(struct sunxi_mmc_priv *priv, struct mmc *mmc, + struct mmc_data *data) { - struct sunxi_mmc_priv *priv = mmc->priv; const int reading = !!(data->flags & MMC_DATA_READ); const uint32_t status_bit = reading ? SUNXI_MMC_STATUS_FIFO_EMPTY : SUNXI_MMC_STATUS_FIFO_FULL; @@ -293,10 +290,9 @@ static int mmc_trans_data_by_cpu(struct mmc *mmc, struct mmc_data *data) return 0; } -static int mmc_rint_wait(struct mmc *mmc, unsigned int timeout_msecs, - unsigned int done_bit, const char *what) +static int mmc_rint_wait(struct sunxi_mmc_priv *priv, struct mmc *mmc, + uint timeout_msecs, uint done_bit, const char *what) { - struct sunxi_mmc_priv *priv = mmc->priv; unsigned int status; do { @@ -313,10 +309,10 @@ static int mmc_rint_wait(struct mmc *mmc, unsigned int timeout_msecs, return 0; } -static int sunxi_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, - struct mmc_data *data) +static int sunxi_mmc_send_cmd_common(struct sunxi_mmc_priv *priv, + struct mmc *mmc, struct mmc_cmd *cmd, + struct mmc_data *data) { - struct sunxi_mmc_priv *priv = mmc->priv; unsigned int cmdval = SUNXI_MMC_CMD_START; unsigned int timeout_msecs; int error = 0; @@ -372,7 +368,7 @@ static int sunxi_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, bytecnt = data->blocksize * data->blocks; debug("trans data %d bytes\n", bytecnt); writel(cmdval | cmd->cmdidx, &priv->reg->cmd); - ret = mmc_trans_data_by_cpu(mmc, data); + ret = mmc_trans_data_by_cpu(priv, mmc, data); if (ret) { error = readl(&priv->reg->rint) & SUNXI_MMC_RINT_INTERRUPT_ERROR_BIT; @@ -381,14 +377,15 @@ static int sunxi_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, } } - error = mmc_rint_wait(mmc, 1000, SUNXI_MMC_RINT_COMMAND_DONE, "cmd"); + error = mmc_rint_wait(priv, mmc, 1000, SUNXI_MMC_RINT_COMMAND_DONE, + "cmd"); if (error) goto out; if (data) { timeout_msecs = 120; debug("cacl timeout %x msec\n", timeout_msecs); - error = mmc_rint_wait(mmc, timeout_msecs, + error = mmc_rint_wait(priv, mmc, timeout_msecs, data->blocks > 1 ? SUNXI_MMC_RINT_AUTO_COMMAND_DONE : SUNXI_MMC_RINT_DATA_OVER, @@ -425,7 +422,7 @@ static int sunxi_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, out: if (error < 0) { writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl); - mmc_update_clk(mmc); + mmc_update_clk(priv); } writel(0xffffffff, &priv->reg->rint); writel(readl(&priv->reg->gctrl) | SUNXI_MMC_GCTRL_FIFO_RESET, @@ -434,7 +431,22 @@ out: return error; } -static int sunxi_mmc_getcd(struct mmc *mmc) +static int sunxi_mmc_set_ios_legacy(struct mmc *mmc) +{ + struct sunxi_mmc_priv *priv = mmc->priv; + + return sunxi_mmc_set_ios_common(priv, mmc); +} + +static int sunxi_mmc_send_cmd_legacy(struct mmc *mmc, struct mmc_cmd *cmd, + struct mmc_data *data) +{ + struct sunxi_mmc_priv *priv = mmc->priv; + + return sunxi_mmc_send_cmd_common(priv, mmc, cmd, data); +} + +static int sunxi_mmc_getcd_legacy(struct mmc *mmc) { struct sunxi_mmc_priv *priv = mmc->priv; int cd_pin; @@ -447,17 +459,18 @@ static int sunxi_mmc_getcd(struct mmc *mmc) } static const struct mmc_ops sunxi_mmc_ops = { - .send_cmd = sunxi_mmc_send_cmd, - .set_ios = sunxi_mmc_set_ios, + .send_cmd = sunxi_mmc_send_cmd_legacy, + .set_ios = sunxi_mmc_set_ios_legacy, .init = sunxi_mmc_core_init, - .getcd = sunxi_mmc_getcd, + .getcd = sunxi_mmc_getcd_legacy, }; struct mmc *sunxi_mmc_init(int sdc_no) { - struct mmc_config *cfg = &mmc_host[sdc_no].cfg; + struct sunxi_mmc_priv *priv = &mmc_host[sdc_no]; + struct mmc_config *cfg = &priv->cfg; - memset(&mmc_host[sdc_no], 0, sizeof(struct sunxi_mmc_priv)); + memset(priv, '\0', sizeof(struct sunxi_mmc_priv)); cfg->name = "SUNXI SD/MMC"; cfg->ops = &sunxi_mmc_ops; @@ -479,5 +492,5 @@ struct mmc *sunxi_mmc_init(int sdc_no) mmc_clk_io_on(sdc_no); - return mmc_create(cfg, &mmc_host[sdc_no]); + return mmc_create(cfg, mmc_host); }
At present the driver-private data is obtained in various functions by various means. With driver model this is provided automatically. Without driver model it comes from a C array declared at the top of the file. Adjust internal functions so that they are passed the private data as a parameter, allowing the caller to obtain it using either means. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: None drivers/mmc/sunxi_mmc.c | 71 +++++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 29 deletions(-)