| Message ID | 20221128024407.224393-7-doug@schmorgal.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci-pxav2: Add support for PXA168 | expand |
On 28/11/22 04:44, Doug Brown wrote: > The PXA168 has a documented silicon bug that causes SDIO card IRQs to be > missed. Implement the first half of the suggested workaround, which > involves resetting the data port logic and issuing a dummy CMD0 to > restart the clock. > > Signed-off-by: Doug Brown <doug@schmorgal.com> > --- > drivers/mmc/host/sdhci-pxav2.c | 36 ++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c > index 4996d72c6d23..0b9b2e4b2153 100644 > --- a/drivers/mmc/host/sdhci-pxav2.c > +++ b/drivers/mmc/host/sdhci-pxav2.c > @@ -20,6 +20,8 @@ > #include <linux/slab.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/mmc/sdio.h> > +#include <linux/mmc/mmc.h> > > #include "sdhci.h" > #include "sdhci-pltfm.h" > @@ -43,6 +45,7 @@ > > struct sdhci_pxav2_host { > struct clk *clk_core; > + void (*orig_post_req)(struct mmc_host *mmc, struct mmc_request *mrq, int err); > }; > > static void pxav2_reset(struct sdhci_host *host, u8 mask) > @@ -96,6 +99,37 @@ static inline u16 pxav1_readw(struct sdhci_host *host, int reg) > return readw(host->ioaddr + reg); > } > > +static void pxav1_post_req(struct mmc_host *mmc, struct mmc_request *mrq, int err) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pxav2_host *pxav2_host; > + struct mmc_command dummy_cmd = {}; > + u16 tmp; > + > + /* If this is an SDIO command, perform errata workaround for silicon bug. */ > + if (!err && mrq->cmd && !mrq->cmd->error && > + (mrq->cmd->opcode == SD_IO_RW_DIRECT || > + mrq->cmd->opcode == SD_IO_RW_EXTENDED)) { > + /* Reset data port */ > + tmp = readw(host->ioaddr + SDHCI_TIMEOUT_CONTROL); > + tmp |= 0x400; > + writew(tmp, host->ioaddr + SDHCI_TIMEOUT_CONTROL); > + > + /* Clock is now stopped, so restart it by sending a dummy CMD0. */ > + pxav2_host = sdhci_pltfm_priv(sdhci_priv(host)); > + > + dummy_cmd.opcode = MMC_GO_IDLE_STATE; > + dummy_cmd.arg = 0; > + dummy_cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_NONE | MMC_CMD_BC; > + > + mmc_wait_for_cmd(host->mmc, &dummy_cmd, 0); This is not what post_req() is for. Instead could you use SDHCI host op ->request_done()? Also, do you really need to wait for the dummy CMD0 - perhaps just write SDHCI_ARGUMENT, SDHCI_TRANSFER_MODE, and SDHCI_COMMAND ? > + } > + > + /* Pass onto SDHCI host driver now */ > + if (pxav2_host->orig_post_req) > + pxav2_host->orig_post_req(mmc, mrq, err); > +} > + > static void pxav2_mmc_set_bus_width(struct sdhci_host *host, int width) > { > u8 ctrl; > @@ -252,6 +286,8 @@ static int sdhci_pxav2_probe(struct platform_device *pdev) > if (match && of_device_is_compatible(dev->of_node, "mrvl,pxav1-mmc")) { > host->quirks |= SDHCI_QUIRK_NO_BUSY_IRQ | SDHCI_QUIRK_32BIT_DMA_SIZE; > host->ops = &pxav1_sdhci_ops; > + pxav2_host->orig_post_req = host->mmc_host_ops.post_req; > + host->mmc_host_ops.post_req = pxav1_post_req; > } else { > host->ops = &pxav2_sdhci_ops; > }
Hi Adrian, On 11/28/2022 11:29 PM, Adrian Hunter wrote: > On 28/11/22 04:44, Doug Brown wrote: >> + >> + /* Clock is now stopped, so restart it by sending a dummy CMD0. */ >> + pxav2_host = sdhci_pltfm_priv(sdhci_priv(host)); >> + >> + dummy_cmd.opcode = MMC_GO_IDLE_STATE; >> + dummy_cmd.arg = 0; >> + dummy_cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_NONE | MMC_CMD_BC; >> + >> + mmc_wait_for_cmd(host->mmc, &dummy_cmd, 0); > > This is not what post_req() is for. Instead could you use SDHCI > host op ->request_done()? Also, do you really need to wait for > the dummy CMD0 - perhaps just write SDHCI_ARGUMENT, > SDHCI_TRANSFER_MODE, and SDHCI_COMMAND ? Thanks for the feedback! That makes perfect sense. I do need to know when the dummy CMD0 finishes so I can restore the pinctrl to default in the next patch, but I think I can handle that with the irq() host op, which I need to do anyway so that sdhci_cmd_irq() doesn't get confused when the CMD0 finishes. I'll give that a shot in the next version of the series and will also address your other feedback. Doug
diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c index 4996d72c6d23..0b9b2e4b2153 100644 --- a/drivers/mmc/host/sdhci-pxav2.c +++ b/drivers/mmc/host/sdhci-pxav2.c @@ -20,6 +20,8 @@ #include <linux/slab.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/mmc/sdio.h> +#include <linux/mmc/mmc.h> #include "sdhci.h" #include "sdhci-pltfm.h" @@ -43,6 +45,7 @@ struct sdhci_pxav2_host { struct clk *clk_core; + void (*orig_post_req)(struct mmc_host *mmc, struct mmc_request *mrq, int err); }; static void pxav2_reset(struct sdhci_host *host, u8 mask) @@ -96,6 +99,37 @@ static inline u16 pxav1_readw(struct sdhci_host *host, int reg) return readw(host->ioaddr + reg); } +static void pxav1_post_req(struct mmc_host *mmc, struct mmc_request *mrq, int err) +{ + struct sdhci_host *host = mmc_priv(mmc); + struct sdhci_pxav2_host *pxav2_host; + struct mmc_command dummy_cmd = {}; + u16 tmp; + + /* If this is an SDIO command, perform errata workaround for silicon bug. */ + if (!err && mrq->cmd && !mrq->cmd->error && + (mrq->cmd->opcode == SD_IO_RW_DIRECT || + mrq->cmd->opcode == SD_IO_RW_EXTENDED)) { + /* Reset data port */ + tmp = readw(host->ioaddr + SDHCI_TIMEOUT_CONTROL); + tmp |= 0x400; + writew(tmp, host->ioaddr + SDHCI_TIMEOUT_CONTROL); + + /* Clock is now stopped, so restart it by sending a dummy CMD0. */ + pxav2_host = sdhci_pltfm_priv(sdhci_priv(host)); + + dummy_cmd.opcode = MMC_GO_IDLE_STATE; + dummy_cmd.arg = 0; + dummy_cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_NONE | MMC_CMD_BC; + + mmc_wait_for_cmd(host->mmc, &dummy_cmd, 0); + } + + /* Pass onto SDHCI host driver now */ + if (pxav2_host->orig_post_req) + pxav2_host->orig_post_req(mmc, mrq, err); +} + static void pxav2_mmc_set_bus_width(struct sdhci_host *host, int width) { u8 ctrl; @@ -252,6 +286,8 @@ static int sdhci_pxav2_probe(struct platform_device *pdev) if (match && of_device_is_compatible(dev->of_node, "mrvl,pxav1-mmc")) { host->quirks |= SDHCI_QUIRK_NO_BUSY_IRQ | SDHCI_QUIRK_32BIT_DMA_SIZE; host->ops = &pxav1_sdhci_ops; + pxav2_host->orig_post_req = host->mmc_host_ops.post_req; + host->mmc_host_ops.post_req = pxav1_post_req; } else { host->ops = &pxav2_sdhci_ops; }
The PXA168 has a documented silicon bug that causes SDIO card IRQs to be missed. Implement the first half of the suggested workaround, which involves resetting the data port logic and issuing a dummy CMD0 to restart the clock. Signed-off-by: Doug Brown <doug@schmorgal.com> --- drivers/mmc/host/sdhci-pxav2.c | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)