| Message ID | 1358819250-31625-10-git-send-email-otavio@ossystems.com.br |
|---|---|
| State | Not Applicable |
| Headers | show |
Dear Otavio Salvador, NAK. DMA works on MX28 and works in Linux on MX23, therefore the DMA is not broken on either. Thus there's no reason to introduce such "force_pio" stuff. Best regards, Marek Vasut
On Mon, Jan 21, 2013 at 11:50 PM, Marek Vasut <marex@denx.de> wrote: > Dear Otavio Salvador, > > NAK. DMA works on MX28 and works in Linux on MX23, therefore the DMA is not > broken on either. Thus there's no reason to introduce such "force_pio" stuff. Lack of time to work on it is one reason. I won't be able to work on it for some days and I prefer a working solution than skip it for another release. -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
Dear Otavio Salvador, > On Mon, Jan 21, 2013 at 11:50 PM, Marek Vasut <marex@denx.de> wrote: > > Dear Otavio Salvador, > > > > NAK. DMA works on MX28 and works in Linux on MX23, therefore the DMA is > > not broken on either. Thus there's no reason to introduce such > > "force_pio" stuff. > > Lack of time to work on it is one reason. I won't be able to work on > it for some days and I prefer a working solution than skip it for > another release. The solution you propose adds undocumented feature and is not working, this is clearly a workaround. Best regards, Marek Vasut
On 22/01/2013 02:56, Otavio Salvador wrote: > On Mon, Jan 21, 2013 at 11:50 PM, Marek Vasut <marex@denx.de> wrote: >> Dear Otavio Salvador, >> >> NAK. DMA works on MX28 and works in Linux on MX23, therefore the DMA is not >> broken on either. Thus there's no reason to introduce such "force_pio" stuff. > > Lack of time to work on it is one reason. I won't be able to work on > it for some days and I prefer a working solution than skip it for > another release. > > -- > Otavio Salvador O.S. Systems > E-mail: otavio@ossystems.com.br http://www.ossystems.com.br > Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br > Hi Otavio, hi Marek, let me say some words about this discussion. I agree that if there is no support for DMA for MX23, we could add support with PIO mode. However, there should be a very good reason, because as far as I understand, added support for DMA is valid for mxs platform. The solution introduced with this patchset lets also the door open for people that can use MX28 in PIO mode. I think (I have already seen in ML Marek's patchset for it) the way to fix DMA for mx23 and then add support for MMC is the rught way to do. Best regards, Stefano Babic
On Wed, Jan 23, 2013 at 7:04 AM, Stefano Babic <sbabic@denx.de> wrote: > On 22/01/2013 02:56, Otavio Salvador wrote: >> On Mon, Jan 21, 2013 at 11:50 PM, Marek Vasut <marex@denx.de> wrote: >>> Dear Otavio Salvador, >>> >>> NAK. DMA works on MX28 and works in Linux on MX23, therefore the DMA is not >>> broken on either. Thus there's no reason to introduce such "force_pio" stuff. >> >> Lack of time to work on it is one reason. I won't be able to work on >> it for some days and I prefer a working solution than skip it for >> another release. >> >> -- >> Otavio Salvador O.S. Systems >> E-mail: otavio@ossystems.com.br http://www.ossystems.com.br >> Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br >> > > Hi Otavio, hi Marek, > > let me say some words about this discussion. I agree that if there is no > support for DMA for MX23, we could add support with PIO mode. However, > there should be a very good reason, because as far as I understand, > added support for DMA is valid for mxs platform. The solution introduced > with this patchset lets also the door open for people that can use MX28 > in PIO mode. I think (I have already seen in ML Marek's patchset for it) > the way to fix DMA for mx23 and then add support for MMC is the rught > way to do. So you'd like to have this patch ported to Marek's patchset so the support for PIO mode is kept available? Instead of enabling it for MX23 we can have a config option which allows force PIO mode and disable DMA. This is what you want? -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
Dear Otavio Salvador, > On Wed, Jan 23, 2013 at 7:04 AM, Stefano Babic <sbabic@denx.de> wrote: > > On 22/01/2013 02:56, Otavio Salvador wrote: > >> On Mon, Jan 21, 2013 at 11:50 PM, Marek Vasut <marex@denx.de> wrote: > >>> Dear Otavio Salvador, > >>> > >>> NAK. DMA works on MX28 and works in Linux on MX23, therefore the DMA is > >>> not broken on either. Thus there's no reason to introduce such > >>> "force_pio" stuff. > >> > >> Lack of time to work on it is one reason. I won't be able to work on > >> it for some days and I prefer a working solution than skip it for > >> another release. > >> > >> -- > >> Otavio Salvador O.S. Systems > >> E-mail: otavio@ossystems.com.br http://www.ossystems.com.br > >> Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br > > > > Hi Otavio, hi Marek, > > > > let me say some words about this discussion. I agree that if there is no > > support for DMA for MX23, we could add support with PIO mode. However, > > there should be a very good reason, because as far as I understand, > > added support for DMA is valid for mxs platform. The solution introduced > > with this patchset lets also the door open for people that can use MX28 > > in PIO mode. I think (I have already seen in ML Marek's patchset for it) > > the way to fix DMA for mx23 and then add support for MMC is the rught > > way to do. > > So you'd like to have this patch ported to Marek's patchset so the > support for PIO mode is kept available? Instead of enabling it for > MX23 we can have a config option which allows force PIO mode and > disable DMA. This is what you want? Forcing PIO is nonsense, why would it be of any use? Best regards, Marek Vasut
On 23/01/2013 12:26, Marek Vasut wrote: >>> Hi Otavio, hi Marek, >>> >>> let me say some words about this discussion. I agree that if there is no >>> support for DMA for MX23, we could add support with PIO mode. However, >>> there should be a very good reason, because as far as I understand, >>> added support for DMA is valid for mxs platform. The solution introduced >>> with this patchset lets also the door open for people that can use MX28 >>> in PIO mode. I think (I have already seen in ML Marek's patchset for it) >>> the way to fix DMA for mx23 and then add support for MMC is the rught >>> way to do. >> >> So you'd like to have this patch ported to Marek's patchset so the >> support for PIO mode is kept available? Instead of enabling it for >> MX23 we can have a config option which allows force PIO mode and >> disable DMA. This is what you want? > > Forcing PIO is nonsense, why would it be of any use? No, I agree with Marek - if DMA is then working also for MX23, we do not need PIO at all. Best regards, Stefano Babic
Dear Stefano Babic, > On 23/01/2013 12:26, Marek Vasut wrote: > >>> Hi Otavio, hi Marek, > >>> > >>> let me say some words about this discussion. I agree that if there is > >>> no support for DMA for MX23, we could add support with PIO mode. > >>> However, there should be a very good reason, because as far as I > >>> understand, added support for DMA is valid for mxs platform. The > >>> solution introduced with this patchset lets also the door open for > >>> people that can use MX28 in PIO mode. I think (I have already seen in > >>> ML Marek's patchset for it) the way to fix DMA for mx23 and then add > >>> support for MMC is the rught way to do. > >> > >> So you'd like to have this patch ported to Marek's patchset so the > >> support for PIO mode is kept available? Instead of enabling it for > >> MX23 we can have a config option which allows force PIO mode and > >> disable DMA. This is what you want? > > > > Forcing PIO is nonsense, why would it be of any use? > > No, I agree with Marek - if DMA is then working also for MX23, we do not > need PIO at all. Stefano, it's now using mixed mode -- PIO for small transfers, DMA for large transfers. That is the best pick of both worlds ;-) Best regards, Marek Vasut
On Wed, Jan 23, 2013 at 9:48 AM, Marek Vasut <marex@denx.de> wrote: > Dear Stefano Babic, > >> On 23/01/2013 12:26, Marek Vasut wrote: >> >>> Hi Otavio, hi Marek, >> >>> >> >>> let me say some words about this discussion. I agree that if there is >> >>> no support for DMA for MX23, we could add support with PIO mode. >> >>> However, there should be a very good reason, because as far as I >> >>> understand, added support for DMA is valid for mxs platform. The >> >>> solution introduced with this patchset lets also the door open for >> >>> people that can use MX28 in PIO mode. I think (I have already seen in >> >>> ML Marek's patchset for it) the way to fix DMA for mx23 and then add >> >>> support for MMC is the rught way to do. >> >> >> >> So you'd like to have this patch ported to Marek's patchset so the >> >> support for PIO mode is kept available? Instead of enabling it for >> >> MX23 we can have a config option which allows force PIO mode and >> >> disable DMA. This is what you want? >> > >> > Forcing PIO is nonsense, why would it be of any use? >> >> No, I agree with Marek - if DMA is then working also for MX23, we do not >> need PIO at all. > > Stefano, it's now using mixed mode -- PIO for small transfers, DMA for large > transfers. That is the best pick of both worlds ;-) That's fine; I just wanted to check if I had understood it right. No problem; I will update the environments and send a v2 of my patchset basing on top of Marek's changes. -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c index 0a3f0c4..628d609 100644 --- a/drivers/mmc/mxsmmc.c +++ b/drivers/mmc/mxsmmc.c @@ -50,6 +50,7 @@ struct mxsmmc_priv { uint32_t buswidth; int (*mmc_is_wp)(int); struct mxs_dma_desc *desc; + int force_pio; }; #define MXSMMC_MAX_TIMEOUT 10000 @@ -184,7 +185,7 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) if (cmd->resp_type & MMC_RSP_136) /* It's a 136 bits response */ ctrl0 |= SSP_CTRL0_LONG_RESP; - if (data && (data->blocksize * data->blocks < MXSMMC_SMALL_TRANSFER)) + if (priv->force_pio || (data && (data->blocksize * data->blocks < MXSMMC_SMALL_TRANSFER))) writel(SSP_CTRL1_DMA_ENABLE, &ssp_regs->hw_ssp_ctrl1_clr); else writel(SSP_CTRL1_DMA_ENABLE, &ssp_regs->hw_ssp_ctrl1_set); @@ -287,7 +288,7 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) if (!data) return 0; - if (data->blocksize * data->blocks < MXSMMC_SMALL_TRANSFER) { + if (priv->force_pio || (data->blocksize * data->blocks < MXSMMC_SMALL_TRANSFER)) { ret = mxsmmc_send_cmd_pio(priv, data); if (ret) { printf("MMC%d: Data timeout with command %d " @@ -385,9 +386,11 @@ int mxsmmc_initialize(bd_t *bis, int id, int (*wp)(int)) #if defined(CONFIG_MX23) const unsigned int mxsmmc_max_id = 2; const unsigned int mxsmmc_clk_id = 0; + const unsigned int mxsmmc_force_pio = 1; #elif defined(CONFIG_MX28) const unsigned int mxsmmc_max_id = 4; const unsigned int mxsmmc_clk_id = id; + const unsigned int mxsmmc_force_pio = 0; #endif if (id >= mxsmmc_max_id) @@ -417,6 +420,7 @@ int mxsmmc_initialize(bd_t *bis, int id, int (*wp)(int)) priv->mmc_is_wp = wp; priv->id = id; priv->regs = mxs_ssp_regs_by_bus(id); + priv->force_pio = mxsmmc_force_pio; sprintf(mmc->name, "MXS MMC"); mmc->send_cmd = mxsmmc_send_cmd;
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> --- drivers/mmc/mxsmmc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)