Hi Jules,
Some small things inline.
On Wed, Aug 16, 2023 at 11:39:45AM +0200, Jules Maselbas wrote:
> +#include "sdhci.h"
> +
> +#define CARD_STATUS_MASK (0x1e00)
> +#define CARD_STATUS_TRAN (4 << 9)
> +static int dwcmshc_mci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
> + struct mci_data *data);
> +
> +struct dwcmshc_host {
> + struct mci_host mci;
> + struct sdhci sdhci;
> + unsigned int in_abort_sequence;
> +};
> +
> +#define priv_from_mci_host(h) container_of(h, struct dwcmshc_host, mci)
Writing this as static inline function immediately makes the type of 'h'
clear.
> +
> +static void mci_setup_cmd(struct mci_cmd *p, unsigned int cmd, unsigned int
> arg,
> + unsigned int response)
> +{
> + p->cmdidx = cmd;
> + p->cmdarg = arg;
> + p->resp_type = response;
> +}
> +
> +static int do_abort_sequence(struct mci_host *mci, struct mci_cmd
> *current_cmd)
> +{
> + int ret = 0;
> + struct dwcmshc_host *host = priv_from_mci_host(mci);
> + struct mci_cmd cmd;
> + u64 start;
> +
> + host->in_abort_sequence = 1;
> +
> + mci_setup_cmd(&cmd, MMC_CMD_STOP_TRANSMISSION, 0, MMC_RSP_R1b);
> + ret = dwcmshc_mci_send_cmd(mci, &cmd, NULL);
> + if (ret) {
> + dev_err(host->mci.hw_dev, "Abort failed at first cmd12!\n");
> + goto out;
> + }
> + mci_setup_cmd(&cmd, MMC_CMD_SEND_STATUS, mci->mci->rca << 16,
> + MMC_RSP_R1);
> + ret = dwcmshc_mci_send_cmd(mci, &cmd, NULL);
> + if (ret) {
> + dev_err(host->mci.hw_dev, "Abort failed at first cmd13!\n");
> + goto out;
> + }
> +
> + if ((cmd.response[0] & CARD_STATUS_MASK) == CARD_STATUS_TRAN)
> + goto out; /* All is OK! */
> +
> + mci_setup_cmd(&cmd, MMC_CMD_STOP_TRANSMISSION, 0, MMC_RSP_R1b);
> + ret = dwcmshc_mci_send_cmd(mci, &cmd, NULL);
> + if (ret) {
> + dev_err(host->mci.hw_dev, "Abort failed at second cmd12!\n");
> + goto out;
> + }
> +
> + mci_setup_cmd(&cmd, MMC_CMD_SEND_STATUS, mci->mci->rca << 16,
> + MMC_RSP_R1);
> + ret = dwcmshc_mci_send_cmd(mci, &cmd, NULL);
> + if (ret) {
> + dev_err(host->mci.hw_dev, "Abort failed at second cmd13!\n");
> + goto out;
> + }
> +
> + if ((cmd.response[0] & CARD_STATUS_MASK) == CARD_STATUS_TRAN) {
> + goto out; /* All is OK! */
> + } else {
> + dev_err(host->mci.hw_dev,
> + "Abort sequence failed to put card in TRAN state!\n");
> + ret = 1;
This should be an error code.
> + goto out;
> + }
> +
> +out:
> + /* Perform SW reset if in abort sequence */
> + sdhci_write8(&host->sdhci, SDHCI_SOFTWARE_RESET,
> + SDHCI_RESET_DATA | SDHCI_RESET_CMD);
> + start = get_time_ns();
> + while (sdhci_read8(&host->sdhci, SDHCI_SOFTWARE_RESET) != 0) {
> + if (is_timeout(start, 50 * MSECOND)) {
> + dev_err(host->mci.hw_dev,
> + "SDHCI data reset timeout\n");
> + break;
> + }
> + }
> + host->in_abort_sequence = 0;
> + return ret;
> +}
> +
> +static int dwcmshc_mci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
> + struct mci_data *data)
> +{
> + struct dwcmshc_host *host = priv_from_mci_host(mci);
> + dma_addr_t dma = SDHCI_NO_DMA;
> + u32 mask, command, xfer;
> + int ret;
> +
> + if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION
> + && host->in_abort_sequence == 0)
> + return do_abort_sequence(mci, cmd);
You could factor the rest of this function out to some dwcmshc_mci_do_send_cmd()
and call it from here and do_abort_sequence().
> +
> + /* Do not wait for CMD_INHIBIT_DAT on stop commands */
> + mask = SDHCI_CMD_INHIBIT_CMD;
> + if (cmd->cmdidx != MMC_CMD_STOP_TRANSMISSION)
> + mask |= SDHCI_CMD_INHIBIT_DATA;
> +
> + /* Wait for bus idle */
> + ret = wait_on_timeout(50 * MSECOND,
> + !(sdhci_read16(&host->sdhci, SDHCI_PRESENT_STATE)
> & mask));
> + if (ret) {
> + dev_err(host->mci.hw_dev, "SDHCI timeout while waiting for
> idle\n");
> + return -ETIMEDOUT;
> + }
> +
> + if (data)
> + dev_dbg(host->mci.hw_dev, "cmd %d arg %d bcnt %d bsize %d\n",
> + cmd->cmdidx, cmd->cmdarg, data->blocks,
> data->blocksize);
> + else
> + dev_dbg(host->mci.hw_dev, "cmd %d arg %d\n", cmd->cmdidx,
> cmd->cmdarg);
> +
> + sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
> +
> + sdhci_setup_data_dma(&host->sdhci, data, &dma);
> +
> + sdhci_write8(&host->sdhci, SDHCI_TIMEOUT_CONTROL, 0xe);
> +
> + sdhci_set_cmd_xfer_mode(&host->sdhci, cmd, data,
> + dma == SDHCI_NO_DMA ? false : true,
> + &command, &xfer);
> +
> + sdhci_write16(&host->sdhci, SDHCI_TRANSFER_MODE, xfer);
> +
> + sdhci_write32(&host->sdhci, SDHCI_ARGUMENT, cmd->cmdarg);
> + sdhci_write16(&host->sdhci, SDHCI_COMMAND, command);
> +
> + ret = sdhci_wait_for_done(&host->sdhci, SDHCI_INT_CMD_COMPLETE);
> + if (ret)
> + goto error;
> +
> + sdhci_read_response(&host->sdhci, cmd);
> +
> + ret = sdhci_transfer_data(&host->sdhci, data, dma);
> +error:
> + if (ret) {
> + sdhci_reset(&host->sdhci, SDHCI_RESET_CMD);
> + sdhci_reset(&host->sdhci, SDHCI_RESET_DATA);
> + }
> +
> + sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
> + return ret;
> +}
> +
> +static void dwcmshc_mci_set_ios(struct mci_host *mci, struct mci_ios *ios)
> +{
> + struct dwcmshc_host *host = priv_from_mci_host(mci);
> + u16 val;
> +
> + debug("%s: clock = %u, bus-width = %d, timing = %02x\n", __func__,
> + ios->clock, ios->bus_width, ios->timing);
dev_dbg()
> +static int dwcmshc_mci_init(struct mci_host *mci, struct device *dev)
> +{
> + struct dwcmshc_host *host = priv_from_mci_host(mci);
> + u16 ctrl2;
> +
> + /* reset mshci controller */
> + sdhci_write8(&host->sdhci, SDHCI_SOFTWARE_RESET, SDHCI_RESET_ALL);
> +
> + /* wait for reset completion */
> + if (wait_on_timeout(100 * MSECOND,
> + (sdhci_read8(&host->sdhci, SDHCI_SOFTWARE_RESET)
> + & SDHCI_RESET_ALL) == 0)) {
> + dev_err(dev, "SDHCI reset timeout\n");
> + return -ETIMEDOUT;
> + }
> +
> + sdhci_write16(&host->sdhci, SDHCI_INT_ERROR_ENABLE, ~0);
> + sdhci_write16(&host->sdhci, SDHCI_INT_ENABLE, ~0);
> + sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
> + sdhci_write32(&host->sdhci, SDHCI_SIGNAL_ENABLE, ~0);
> +
> + sdhci_enable_v4_mode(&host->sdhci);
> +
> + dev_dbg(host->mci.hw_dev, "host version4: %s\n",
> + ctrl2 & SDHCI_CTRL_V4_MODE ? "enabled" : "disabled");
> +
> + return 0;
> +}
> +
[...]
> +static int dwcmshc_probe(struct device *dev)
> +{
> + struct dwcmshc_host *host;
> + struct resource *iores;
> + struct mci_host *mci;
> + struct clk *clk;
> + int ret;
> +
> + host = xzalloc(sizeof(*host));
> + mci = &host->mci;
> +
> + iores = dev_request_mem_resource(dev, 0);
> + if (IS_ERR(iores)) {
> + ret = PTR_ERR(iores);
> + goto err_mem_req;
> + }
> +
> + clk = clk_get(dev, NULL);
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
> + goto err_clk_get;
> + }
> + clk_enable(clk);
> +
> + host->sdhci.base = IOMEM(iores->start);
> + host->sdhci.mci = mci;
> + host->sdhci.max_clk = clk_get_rate(clk);
> +
> + mci->hw_dev = dev;
> + mci->init = dwcmshc_mci_init;
> + mci->set_ios = dwcmshc_mci_set_ios;
> + mci->send_cmd = dwcmshc_mci_send_cmd;
> + mci->card_present = dwcmshc_mci_card_present;
> +
> + mci_of_parse(&host->mci);
> +
> + /* Enable host_version4 */
> + sdhci_enable_v4_mode(&host->sdhci);
This is also called from dwcmshc_mci_init(). Do we need both?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
