On 6 February 2017 at 14:39, Jan Glauber <[email protected]> wrote:
> This core driver will be used by a MIPS platform driver
> or by an ARM64 PCI driver. The core driver implements the
> mmc_host_ops and slot probe & remove functions.
> Callbacks are provided to allow platform specific interrupt
> enable and bus locking.
>
> The host controller supports:
> - up to 4 slots that can contain sd-cards or eMMC chips
> - 1, 4 and 8 bit bus width
> - SDR and DDR
> - transfers up to 52 Mhz (might be less when multiple slots are used)
> - DMA read/write
> - multi-block read/write (but not stream mode)
>
> Voltage is limited to 3.3v and shared for all slots.
What voltage? The I/O voltage or the voltage for the card?
VMMC or VMMCQ?
>
> A global lock for all MMC devices is required because the host
> controller is shared.
>
> Signed-off-by: Jan Glauber <[email protected]>
> Signed-off-by: David Daney <[email protected]>
> Signed-off-by: Steven J. Hill <[email protected]>
> ---
> drivers/mmc/host/cavium-mmc.c | 1029 +++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/host/cavium-mmc.h | 303 ++++++++++++
> 2 files changed, 1332 insertions(+)
> create mode 100644 drivers/mmc/host/cavium-mmc.c
> create mode 100644 drivers/mmc/host/cavium-mmc.h
>
> diff --git a/drivers/mmc/host/cavium-mmc.c b/drivers/mmc/host/cavium-mmc.c
> new file mode 100644
> index 0000000..40aee08
> --- /dev/null
> +++ b/drivers/mmc/host/cavium-mmc.c
[...]
> +
> +static bool bad_status(union mio_emm_rsp_sts *rsp_sts)
> +{
> + if (rsp_sts->s.rsp_bad_sts || rsp_sts->s.rsp_crc_err ||
> + rsp_sts->s.rsp_timeout || rsp_sts->s.blk_crc_err ||
> + rsp_sts->s.blk_timeout || rsp_sts->s.dbuf_err)
> + return true;
> +
> + return false;
> +}
> +
> +/* Try to clean up failed DMA. */
> +static void cleanup_dma(struct cvm_mmc_host *host,
> + union mio_emm_rsp_sts *rsp_sts)
> +{
> + union mio_emm_dma emm_dma;
> +
> + emm_dma.val = readq(host->base + MIO_EMM_DMA);
> + emm_dma.s.dma_val = 1;
> + emm_dma.s.dat_null = 1;
> + emm_dma.s.bus_id = rsp_sts->s.bus_id;
> + writeq(emm_dma.val, host->base + MIO_EMM_DMA);
> +}
> +
> +irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> +{
> + struct cvm_mmc_host *host = dev_id;
> + union mio_emm_rsp_sts rsp_sts;
> + union mio_emm_int emm_int;
> + struct mmc_request *req;
> + bool host_done;
> +
> + /* Clear interrupt bits (write 1 clears ). */
> + emm_int.val = readq(host->base + MIO_EMM_INT);
> + writeq(emm_int.val, host->base + MIO_EMM_INT);
> +
> + if (emm_int.s.switch_err)
> + check_switch_errors(host);
> +
> + req = host->current_req;
> + if (!req)
> + goto out;
> +
> + rsp_sts.val = readq(host->base + MIO_EMM_RSP_STS);
> + /*
> + * dma_val set means DMA is still in progress. Don't touch
> + * the request and wait for the interrupt indicating that
> + * the DMA is finished.
> + */
> + if (rsp_sts.s.dma_val && host->dma_active)
> + goto out;
> +
> + if (!host->dma_active && emm_int.s.buf_done && req->data) {
> + unsigned int type = (rsp_sts.val >> 7) & 3;
> +
> + if (type == 1)
> + do_read(host, req, rsp_sts.s.dbuf);
> + else if (type == 2)
> + do_write(req);
> + }
> +
> + host_done = emm_int.s.cmd_done || emm_int.s.dma_done ||
> + emm_int.s.cmd_err || emm_int.s.dma_err;
> +
> + if (!(host_done && req->done))
> + goto no_req_done;
> +
> + if (bad_status(&rsp_sts))
> + req->cmd->error = -EILSEQ;
I don't think you should treat all errors as -EILSEQ. Please assign a
proper error code, depending on the error.
> + else
> + req->cmd->error = 0;
> +
> + if (host->dma_active && req->data)
> + if (!finish_dma(host, req->data))
> + goto no_req_done;
> +
> + set_cmd_response(host, req, &rsp_sts);
> + if (emm_int.s.dma_err && rsp_sts.s.dma_pend)
> + cleanup_dma(host, &rsp_sts);
> +
> + host->current_req = NULL;
> + req->done(req);
> +
> +no_req_done:
> + if (host_done)
> + host->release_bus(host);
> +out:
> + return IRQ_RETVAL(emm_int.val != 0);
> +}
> +
> +/*
> + * Program DMA_CFG and if needed DMA_ADR.
> + * Returns 0 on error, DMA address otherwise.
> + */
> +static u64 prepare_dma_single(struct cvm_mmc_host *host, struct mmc_data *data)
> +{
> + union mio_emm_dma_cfg dma_cfg;
> + int count;
> + u64 addr;
> +
> + count = dma_map_sg(host->dev, data->sg, data->sg_len,
> + get_dma_dir(data));
> + if (!count)
> + return 0;
> +
> + dma_cfg.val = 0;
> + dma_cfg.s.en = 1;
> + dma_cfg.s.rw = (data->flags & MMC_DATA_WRITE) ? 1 : 0;
> +#ifdef __LITTLE_ENDIAN
> + dma_cfg.s.endian = 1;
> +#endif
> + dma_cfg.s.size = (sg_dma_len(&data->sg[0]) / 8) - 1;
> +
> + addr = sg_dma_address(&data->sg[0]);
> + dma_cfg.s.adr = addr;
> + writeq(dma_cfg.val, host->dma_base + MIO_EMM_DMA_CFG);
> +
> + pr_debug("[%s] sg_dma_len: %u total sg_elem: %d\n",
> + (dma_cfg.s.rw) ? "W" : "R", sg_dma_len(&data->sg[0]), count);
> + return addr;
> +}
> +
> +static u64 prepare_dma(struct cvm_mmc_host *host, struct mmc_data *data)
> +{
> + return prepare_dma_single(host, data);
> +}
> +
> +static void prepare_ext_dma(struct mmc_host *mmc, struct mmc_request *mrq,
> + union mio_emm_dma *emm_dma)
> +{
> + struct cvm_mmc_slot *slot = mmc_priv(mmc);
> +
> + /*
> + * Our MMC host hardware does not issue single commands,
> + * because that would require the driver and the MMC core
> + * to do work to determine the proper sequence of commands.
I don't get this. The allowed sequence of the commands is determined
by the SD/(e)MMC/SDIO spec and much of this knowledge is the
responsibility of the mmc core.
> + * Instead, our hardware is superior to most other MMC bus
No need to brag about your HW. Let's just describe how it works instead.
> + * hosts. The sequence of MMC commands required to execute
> + * a transfer are issued automatically by the bus hardware.
What does this really mean? Is this about HW support for better
dealing with data requests?
> + *
> + * - David Daney <[email protected]>
> + */
> + emm_dma->val = 0;
> + emm_dma->s.bus_id = slot->bus_id;
> + emm_dma->s.dma_val = 1;
> + emm_dma->s.sector = (mrq->data->blksz == 512) ? 1 : 0;
> + emm_dma->s.rw = (mrq->data->flags & MMC_DATA_WRITE) ? 1 : 0;
> + emm_dma->s.block_cnt = mrq->data->blocks;
> + emm_dma->s.card_addr = mrq->cmd->arg;
> + if (mmc_card_mmc(mmc->card) || (mmc_card_sd(mmc->card) &&
> + (mmc->card->scr.cmds & SD_SCR_CMD23_SUPPORT)))
> + emm_dma->s.multi = 1;
> +
> + pr_debug("[%s] blocks: %u multi: %d\n", (emm_dma->s.rw) ? "W" : "R",
> + mrq->data->blocks, emm_dma->s.multi);
> +}
> +
[...]
> +
> +static void cvm_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct cvm_mmc_slot *slot = mmc_priv(mmc);
> + struct cvm_mmc_host *host = slot->host;
> + int clk_period, power_class = 10, bus_width = 0;
> + union mio_emm_switch emm_switch;
> + u64 clock;
> +
> + host->acquire_bus(host);
> + cvm_mmc_switch_to(slot);
> +
> + /* Set the power state */
> + switch (ios->power_mode) {
> + case MMC_POWER_ON:
> + break;
> +
> + case MMC_POWER_OFF:
> + cvm_mmc_reset_bus(slot);
> +
> + if (host->global_pwr_gpiod)
> + gpiod_set_value_cansleep(host->global_pwr_gpiod, 0);
> + else
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
> + break;
> +
> + case MMC_POWER_UP:
> + if (host->global_pwr_gpiod)
> + gpiod_set_value_cansleep(host->global_pwr_gpiod, 1);
> + else
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, ios->vdd);
> + break;
> + }
> +
> + /* Set bus width */
> + switch (ios->bus_width) {
> + case MMC_BUS_WIDTH_8:
> + bus_width = 2;
> + break;
> + case MMC_BUS_WIDTH_4:
> + bus_width = 1;
> + break;
> + case MMC_BUS_WIDTH_1:
> + bus_width = 0;
> + break;
> + }
> +
> + slot->bus_width = bus_width;
> +
> + if (!ios->clock)
There are cases when the core change the clock rate to 0, and then it
expects the mmc host to gate the clock. It probably a good idea for
you to do that as well.
> + goto out;
> +
> + /* Change the clock frequency. */
> + clock = ios->clock;
> + if (clock > 52000000)
> + clock = 52000000;
> + slot->clock = clock;
> + clk_period = (host->sys_freq + clock - 1) / (2 * clock);
> +
> + emm_switch.val = 0;
> + emm_switch.s.hs_timing = (ios->timing == MMC_TIMING_MMC_HS);
> + emm_switch.s.bus_width = bus_width;
> + emm_switch.s.power_class = power_class;
> + emm_switch.s.clk_hi = clk_period;
> + emm_switch.s.clk_lo = clk_period;
> + emm_switch.s.bus_id = slot->bus_id;
> +
> + if (!switch_val_changed(slot, emm_switch.val))
> + goto out;
> +
> + set_wdog(slot, 0);
> + do_switch(host, emm_switch.val);
> + slot->cached_switch = emm_switch.val;
> +out:
> + host->release_bus(host);
> +}
[...]
> +
> +static int set_bus_width(struct device *dev, struct cvm_mmc_slot *slot, u32 id)
> +{
> + u32 bus_width;
> + int ret;
> +
> + /*
> + * The "cavium,bus-max-width" property is DEPRECATED and should
> + * not be used. We handle it here to support older firmware.
> + * Going forward, the standard "bus-width" property is used
> + * instead of the Cavium-specific property.
> + */
> + if (!(slot->mmc->caps & (MMC_CAP_8_BIT_DATA | MMC_CAP_4_BIT_DATA))) {
> + /* Try legacy "cavium,bus-max-width" property. */
> + ret = of_property_read_u32(dev->of_node, "cavium,bus-max-width",
> + &bus_width);
> + if (ret) {
> + /* No bus width specified, use default. */
> + bus_width = 8;
> + dev_info(dev, "Default width 8 used for slot %u\n", id);
> + }
> + } else {
> + /* Hosts capable of 8-bit transfers can also do 4 bits */
> + bus_width = (slot->mmc->caps & MMC_CAP_8_BIT_DATA) ? 8 : 4;
> + }
This looks a bit unnessarry complex.
I would instead suggest the following order of how to perform the OF
parsing. Bindings that get parsed later, overrides the earlier.
1. Parse deprecated bindings.
2. Parse cavium specific bindings.
3. Parse common mmc bindings.
4. Check some caps, to make sure those have valid values as to cover
cases when the OF parsing didn't find values.
The same comment applies for the other OF parsing functions below.
> +
> + switch (bus_width) {
> + case 8:
> + slot->bus_width = (MMC_BUS_WIDTH_8 - 1);
> + slot->mmc->caps = MMC_CAP_8_BIT_DATA | MMC_CAP_4_BIT_DATA;
> + break;
> + case 4:
> + slot->bus_width = (MMC_BUS_WIDTH_4 - 1);
> + slot->mmc->caps = MMC_CAP_4_BIT_DATA;
> + break;
> + case 1:
> + slot->bus_width = MMC_BUS_WIDTH_1;
> + break;
> + default:
> + dev_err(dev, "Invalid bus width for slot %u\n", id);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static void set_frequency(struct device *dev, struct mmc_host *mmc, u32 id)
> +{
> + int ret;
> +
> + /*
> + * The "spi-max-frequency" property is DEPRECATED and should
> + * not be used. We handle it here to support older firmware.
> + * Going forward, the standard "max-frequency" property is
> + * used instead of the Cavium-specific property.
> + */
> + if (mmc->f_max == 0) {
> + /* Try legacy "spi-max-frequency" property. */
> + ret = of_property_read_u32(dev->of_node, "spi-max-frequency",
> + &mmc->f_max);
> + if (ret) {
> + /* No frequency properties found, use default. */
> + mmc->f_max = 52000000;
> + dev_info(dev, "Default %u frequency used for slot %u\n",
> + mmc->f_max, id);
> + }
> + } else if (mmc->f_max > 52000000)
> + mmc->f_max = 52000000;
> +
> + /* Set minimum frequency */
> + mmc->f_min = 400000;
> +}
> +
> +static int set_voltage(struct device *dev, struct mmc_host *mmc,
> + struct cvm_mmc_host *host)
> +{
> + int ret;
> +
> + /*
> + * Legacy platform doesn't support regulator but enables power gpio
> + * directly during platform probe.
> + */
> + if (host->global_pwr_gpiod)
> + /* Get a sane OCR mask for other parts of the MMC subsytem. */
> + return mmc_of_parse_voltage(dev->of_node, &mmc->ocr_avail);
Does really the legacy platforms use the mmc voltage range DT bindings!?
I would rather see that you assign a default value to mmc->ocr_avail,
than using this binding.
> +
> + mmc->supply.vmmc = devm_regulator_get(dev, "vmmc");
> + if (IS_ERR(mmc->supply.vmmc)) {
> + ret = PTR_ERR(mmc->supply.vmmc);
> + } else {
> + ret = mmc_regulator_get_ocrmask(mmc->supply.vmmc);
> + if (ret > 0) {
> + mmc->ocr_avail = ret;
> + ret = 0;
> + }
> + }
This if-else-if is a bit messy.
Why not just return when you get an error instead. That should simply the code.
Maybe you can have look and try to clean up this in the hole file
where you think it would make an improvment.
> + return ret;
> +}
> +
> +int cvm_mmc_slot_probe(struct device *dev, struct cvm_mmc_host *host)
To reflect that OF is needed, perhaps rename function to
cvm_mmc_of_slot_probe().
> +{
> + struct device_node *node = dev->of_node;
> + u32 id, cmd_skew, dat_skew;
> + struct cvm_mmc_slot *slot;
> + struct mmc_host *mmc;
> + u64 clock_period;
> + int ret;
> +
> + ret = of_property_read_u32(node, "reg", &id);
> + if (ret) {
> + dev_err(dev, "Missing or invalid reg property on %s\n",
> + of_node_full_name(node));
> + return ret;
> + }
> +
> + if (id >= CAVIUM_MAX_MMC || host->slot[id]) {
> + dev_err(dev, "Invalid reg property on %s\n",
> + of_node_full_name(node));
> + return -EINVAL;
> + }
> +
> + mmc = mmc_alloc_host(sizeof(struct cvm_mmc_slot), dev);
> + if (!mmc)
> + return -ENOMEM;
> +
> + slot = mmc_priv(mmc);
> + slot->mmc = mmc;
> + slot->host = host;
> +
> + ret = mmc_of_parse(mmc);
> + if (ret)
> + goto error;
> +
> + ret = set_bus_width(dev, slot, id);
> + if (ret)
> + goto error;
> +
> + set_frequency(dev, mmc, id);
> +
> + /* Octeon-specific DT properties. */
> + ret = of_property_read_u32(node, "cavium,cmd-clk-skew", &cmd_skew);
> + if (ret)
> + cmd_skew = 0;
> + ret = of_property_read_u32(node, "cavium,dat-clk-skew", &dat_skew);
> + if (ret)
> + dat_skew = 0;
> +
> + ret = set_voltage(dev, mmc, host);
> + if (ret < 0)
> + goto error;
The functions set_bus_width(), set_freqeuncy(), set_voltage() all
performs OF parsing and there are some parsing also being done above.
I would suggest you bundle all OF parsing into one function, perhaps
name it "cvm_mmc_of_parse()" or similar. That should make the code a
lot cleaner.
> +
> + /* Set up host parameters */
> + mmc->ops = &cvm_mmc_ops;
> +
> + mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
> + MMC_CAP_ERASE | MMC_CAP_CMD23 | MMC_CAP_POWER_OFF_CARD;
> +
> + mmc->max_segs = 1;
> +
> + /* DMA size field can address up to 8 MB */
> + mmc->max_seg_size = 8 * 1024 * 1024;
> + mmc->max_req_size = mmc->max_seg_size;
> + /* External DMA is in 512 byte blocks */
> + mmc->max_blk_size = 512;
> + /* DMA block count field is 15 bits */
> + mmc->max_blk_count = 32767;
> +
> + slot->clock = mmc->f_min;
> + slot->sclock = host->sys_freq;
> +
> + /* Period in picoseconds. */
> + clock_period = 1000000000000ull / slot->sclock;
> + slot->cmd_cnt = (cmd_skew + clock_period / 2) / clock_period;
> + slot->dat_cnt = (dat_skew + clock_period / 2) / clock_period;
> +
> + slot->bus_id = id;
> + slot->cached_rca = 1;
> +
> + host->acquire_bus(host);
> + host->slot[id] = slot;
> + cvm_mmc_switch_to(slot);
> + cvm_mmc_init_lowlevel(slot);
> + host->release_bus(host);
> +
> + ret = mmc_add_host(mmc);
> + if (ret) {
> + dev_err(dev, "mmc_add_host() returned %d\n", ret);
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + slot->host->slot[id] = NULL;
> + mmc_free_host(slot->mmc);
> + return ret;
> +}
> +
> +int cvm_mmc_slot_remove(struct cvm_mmc_slot *slot)
> +{
> + mmc_remove_host(slot->mmc);
> + slot->host->slot[slot->bus_id] = NULL;
> + mmc_free_host(slot->mmc);
> + return 0;
> +}
> diff --git a/drivers/mmc/host/cavium-mmc.h b/drivers/mmc/host/cavium-mmc.h
> new file mode 100644
> index 0000000..27fb02b
> --- /dev/null
> +++ b/drivers/mmc/host/cavium-mmc.h
> @@ -0,0 +1,303 @@
> +/*
> + * Driver for MMC and SSD cards for Cavium OCTEON and ThunderX SOCs.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2012-2016 Cavium Inc.
> + */
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/mmc/host.h>
> +#include <linux/of.h>
> +#include <linux/scatterlist.h>
> +#include <linux/semaphore.h>
> +
> +#define CAVIUM_MAX_MMC 4
> +
> +struct cvm_mmc_host {
> + struct device *dev;
> + void __iomem *base;
> + void __iomem *dma_base;
> + u64 emm_cfg;
> + int last_slot;
> + struct clk *clk;
> + int sys_freq;
> +
> + struct mmc_request *current_req;
> + struct sg_mapping_iter smi;
> + bool dma_active;
> +
> + struct gpio_desc *global_pwr_gpiod;
> +
> + struct cvm_mmc_slot *slot[CAVIUM_MAX_MMC];
> +
> + void (*acquire_bus)(struct cvm_mmc_host *);
> + void (*release_bus)(struct cvm_mmc_host *);
> + void (*int_enable)(struct cvm_mmc_host *, u64);
> +};
> +
> +struct cvm_mmc_slot {
> + struct mmc_host *mmc; /* slot-level mmc_core object */
> + struct cvm_mmc_host *host; /* common hw for all slots */
> +
> + u64 clock;
> + unsigned int sclock;
> +
> + u64 cached_switch;
> + u64 cached_rca;
> +
> + unsigned int cmd_cnt; /* sample delay */
> + unsigned int dat_cnt; /* sample delay */
> +
> + int bus_width;
> + int bus_id;
> +};
> +
> +struct cvm_mmc_cr_type {
> + u8 ctype;
> + u8 rtype;
> +};
> +
> +struct cvm_mmc_cr_mods {
> + u8 ctype_xor;
> + u8 rtype_xor;
> +};
> +
> +/* Bitfield definitions */
> +
> +union mio_emm_cmd {
> + u64 val;
> + struct mio_emm_cmd_s {
> +#ifdef __BIG_ENDIAN_BITFIELD
Huh. Sorry, but this is a big nack from me.
This isn't the common method for how we deal with endian issues in the
kernel. Please remove all use of the union types here and below. The
follow common patterns for how we deal with endian issues.
> + u64 :2;
> + u64 bus_id:2;
> + u64 cmd_val:1;
> + u64 :3;
> + u64 dbuf:1;
> + u64 offset:6;
> + u64 :6;
> + u64 ctype_xor:2;
> + u64 rtype_xor:3;
> + u64 cmd_idx:6;
> + u64 arg:32;
> +#else
> + u64 arg:32;
> + u64 cmd_idx:6;
> + u64 rtype_xor:3;
> + u64 ctype_xor:2;
> + u64 :6;
> + u64 offset:6;
> + u64 dbuf:1;
> + u64 :3;
> + u64 cmd_val:1;
> + u64 bus_id:2;
> + u64 :2;
> +#endif
> + } s;
> +};
> +
> +union mio_emm_dma {
> + u64 val;
> + struct mio_emm_dma_s {
> +#ifdef __BIG_ENDIAN_BITFIELD
> + u64 :2;
> + u64 bus_id:2;
> + u64 dma_val:1;
> + u64 sector:1;
> + u64 dat_null:1;
> + u64 thres:6;
> + u64 rel_wr:1;
> + u64 rw:1;
> + u64 multi:1;
> + u64 block_cnt:16;
> + u64 card_addr:32;
> +#else
> + u64 card_addr:32;
> + u64 block_cnt:16;
> + u64 multi:1;
> + u64 rw:1;
> + u64 rel_wr:1;
> + u64 thres:6;
> + u64 dat_null:1;
> + u64 sector:1;
> + u64 dma_val:1;
> + u64 bus_id:2;
> + u64 :2;
> +#endif
> + } s;
> +};
> +
> +union mio_emm_dma_cfg {
> + u64 val;
> + struct mio_emm_dma_cfg_s {
> +#ifdef __BIG_ENDIAN_BITFIELD
> + u64 en:1;
> + u64 rw:1;
> + u64 clr:1;
> + u64 :1;
> + u64 swap32:1;
> + u64 swap16:1;
> + u64 swap8:1;
> + u64 endian:1;
> + u64 size:20;
> + u64 adr:36;
> +#else
> + u64 adr:36;
> + u64 size:20;
> + u64 endian:1;
> + u64 swap8:1;
> + u64 swap16:1;
> + u64 swap32:1;
> + u64 :1;
> + u64 clr:1;
> + u64 rw:1;
> + u64 en:1;
> +#endif
> + } s;
> +};
> +
> +union mio_emm_int {
> + u64 val;
> + struct mio_emm_int_s {
> +#ifdef __BIG_ENDIAN_BITFIELD
> + u64 :57;
> + u64 switch_err:1;
> + u64 switch_done:1;
> + u64 dma_err:1;
> + u64 cmd_err:1;
> + u64 dma_done:1;
> + u64 cmd_done:1;
> + u64 buf_done:1;
> +#else
> + u64 buf_done:1;
> + u64 cmd_done:1;
> + u64 dma_done:1;
> + u64 cmd_err:1;
> + u64 dma_err:1;
> + u64 switch_done:1;
> + u64 switch_err:1;
> + u64 :57;
> +#endif
> + } s;
> +};
> +
> +union mio_emm_rsp_sts {
> + u64 val;
> + struct mio_emm_rsp_sts_s {
> +#ifdef __BIG_ENDIAN_BITFIELD
> + u64 :2;
> + u64 bus_id:2;
> + u64 cmd_val:1;
> + u64 switch_val:1;
> + u64 dma_val:1;
> + u64 dma_pend:1;
> + u64 :27;
> + u64 dbuf_err:1;
> + u64 :4;
> + u64 dbuf:1;
> + u64 blk_timeout:1;
> + u64 blk_crc_err:1;
> + u64 rsp_busybit:1;
> + u64 stp_timeout:1;
> + u64 stp_crc_err:1;
> + u64 stp_bad_sts:1;
> + u64 stp_val:1;
> + u64 rsp_timeout:1;
> + u64 rsp_crc_err:1;
> + u64 rsp_bad_sts:1;
> + u64 rsp_val:1;
> + u64 rsp_type:3;
> + u64 cmd_type:2;
> + u64 cmd_idx:6;
> + u64 cmd_done:1;
> +#else
> + u64 cmd_done:1;
> + u64 cmd_idx:6;
> + u64 cmd_type:2;
> + u64 rsp_type:3;
> + u64 rsp_val:1;
> + u64 rsp_bad_sts:1;
> + u64 rsp_crc_err:1;
> + u64 rsp_timeout:1;
> + u64 stp_val:1;
> + u64 stp_bad_sts:1;
> + u64 stp_crc_err:1;
> + u64 stp_timeout:1;
> + u64 rsp_busybit:1;
> + u64 blk_crc_err:1;
> + u64 blk_timeout:1;
> + u64 dbuf:1;
> + u64 :4;
> + u64 dbuf_err:1;
> + u64 :27;
> + u64 dma_pend:1;
> + u64 dma_val:1;
> + u64 switch_val:1;
> + u64 cmd_val:1;
> + u64 bus_id:2;
> + u64 :2;
> +#endif
> + } s;
> +};
> +
> +union mio_emm_sample {
> + u64 val;
> + struct mio_emm_sample_s {
> +#ifdef __BIG_ENDIAN_BITFIELD
> + u64 :38;
> + u64 cmd_cnt:10;
> + u64 :6;
> + u64 dat_cnt:10;
> +#else
> + u64 dat_cnt:10;
> + u64 :6;
> + u64 cmd_cnt:10;
> + u64 :38;
> +#endif
> + } s;
> +};
> +
> +union mio_emm_switch {
> + u64 val;
> + struct mio_emm_switch_s {
> +#ifdef __BIG_ENDIAN_BITFIELD
> + u64 :2;
> + u64 bus_id:2;
> + u64 switch_exe:1;
> + u64 switch_err0:1;
> + u64 switch_err1:1;
> + u64 switch_err2:1;
> + u64 :7;
> + u64 hs_timing:1;
> + u64 :5;
> + u64 bus_width:3;
> + u64 :4;
> + u64 power_class:4;
> + u64 clk_hi:16;
> + u64 clk_lo:16;
> +#else
> + u64 clk_lo:16;
> + u64 clk_hi:16;
> + u64 power_class:4;
> + u64 :4;
> + u64 bus_width:3;
> + u64 :5;
> + u64 hs_timing:1;
> + u64 :7;
> + u64 switch_err2:1;
> + u64 switch_err1:1;
> + u64 switch_err0:1;
> + u64 switch_exe:1;
> + u64 bus_id:2;
> + u64 :2;
> +#endif
> + } s;
> +};
> +
> +/* Protoypes */
> +irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id);
> +int cvm_mmc_slot_probe(struct device *dev, struct cvm_mmc_host *host);
> +int cvm_mmc_slot_remove(struct cvm_mmc_slot *slot);
Why do you need this here? Are those intended as library functions for
the different cavium variant drivers?
> +extern const struct mmc_host_ops cvm_mmc_ops;
Why do you need this?
Kind regards
Uffe
On 03/03/2017 03:47 AM, Ulf Hansson wrote:
> On 6 February 2017 at 14:39, Jan Glauber <[email protected]> wrote:
[...]
>> +}
>> +
>> +static void prepare_ext_dma(struct mmc_host *mmc, struct mmc_request *mrq,
>> + union mio_emm_dma *emm_dma)
>> +{
>> + struct cvm_mmc_slot *slot = mmc_priv(mmc);
>> +
>> + /*
>> + * Our MMC host hardware does not issue single commands,
>> + * because that would require the driver and the MMC core
>> + * to do work to determine the proper sequence of commands.
>
> I don't get this. The allowed sequence of the commands is determined
> by the SD/(e)MMC/SDIO spec and much of this knowledge is the
> responsibility of the mmc core.
>
>> + * Instead, our hardware is superior to most other MMC bus
>
> No need to brag about your HW. Let's just describe how it works instead.
>
>> + * hosts. The sequence of MMC commands required to execute
>> + * a transfer are issued automatically by the bus hardware.
>
> What does this really mean? Is this about HW support for better
> dealing with data requests?
The entire comment should be removed. It is laced with sarcasm that is
very difficult for people not intimately familiar with the discussions
to detect and decipher.
The real story is that the Cavium MMC hardware was designed based on a
defective premise. Hardwired in to the core of the MMC bus interface
hardware is a multi-command synthesizer/sequencer that is hard coded to
work with a limited subset of MMC-only command sequences. This allows
you to read data from the first few MB of an MMC device with only a
handful of assembly language instructions contained in the on-die
mask-ROM of the SoC.
When it comes to actually using the MMC/SD bus interface from within the
Linux MMC framework, we have to apply a transform to the command
sequences supplied by the Linux MMC core so that the command sequence
synthesized by Cavium MMC hardware matches as closely as possible what
was actually requested. For many command sequences, the match between
what is synthesized by the hardware and what was requested is not exact.
>
>> + *
>> + * - David Daney <[email protected]>
>> + */
>> + emm_dma->val = 0;
>> + emm_dma->s.bus_id = slot->bus_id;
>> + emm_dma->s.dma_val = 1;
>> + emm_dma->s.sector = (mrq->data->blksz == 512) ? 1 : 0;
>> + emm_dma->s.rw = (mrq->data->flags & MMC_DATA_WRITE) ? 1 : 0;
>> + emm_dma->s.block_cnt = mrq->data->blocks;
>> + emm_dma->s.card_addr = mrq->cmd->arg;
>> + if (mmc_card_mmc(mmc->card) || (mmc_card_sd(mmc->card) &&
>> + (mmc->card->scr.cmds & SD_SCR_CMD23_SUPPORT)))
>> + emm_dma->s.multi = 1;
Here is a prefect example.
If a multi-block transfer was requested, the hardware command
synthesizer uses the "multi" bit to determine if it should generate a
command sequence for a single multi-block transfer, or multiple
single-block transfers.
Probably the MMC core would never request a multi-block transfer from a
device that doesn't support it, but we check here just-in-case.
>> +
>> + pr_debug("[%s] blocks: %u multi: %d\n", (emm_dma->s.rw) ? "W" : "R",
>> + mrq->data->blocks, emm_dma->s.multi);
>> +}
>> +
On Fri, Mar 03, 2017 at 12:47:14PM +0100, Ulf Hansson wrote:
> On 6 February 2017 at 14:39, Jan Glauber <[email protected]> wrote:
> > This core driver will be used by a MIPS platform driver
> > or by an ARM64 PCI driver. The core driver implements the
> > mmc_host_ops and slot probe & remove functions.
> > Callbacks are provided to allow platform specific interrupt
> > enable and bus locking.
> >
> > The host controller supports:
> > - up to 4 slots that can contain sd-cards or eMMC chips
> > - 1, 4 and 8 bit bus width
> > - SDR and DDR
> > - transfers up to 52 Mhz (might be less when multiple slots are used)
> > - DMA read/write
> > - multi-block read/write (but not stream mode)
> >
> > Voltage is limited to 3.3v and shared for all slots.
>
> What voltage? The I/O voltage or the voltage for the card?
>
> VMMC or VMMCQ?
>From my understanding both, VMMC and VMMCQ are fixed at 3.3v.
> >
> > A global lock for all MMC devices is required because the host
> > controller is shared.
> >
> > Signed-off-by: Jan Glauber <[email protected]>
> > Signed-off-by: David Daney <[email protected]>
> > Signed-off-by: Steven J. Hill <[email protected]>
> > ---
> > drivers/mmc/host/cavium-mmc.c | 1029 +++++++++++++++++++++++++++++++++++++++++
> > drivers/mmc/host/cavium-mmc.h | 303 ++++++++++++
> > 2 files changed, 1332 insertions(+)
> > create mode 100644 drivers/mmc/host/cavium-mmc.c
> > create mode 100644 drivers/mmc/host/cavium-mmc.h
> >
> > diff --git a/drivers/mmc/host/cavium-mmc.c b/drivers/mmc/host/cavium-mmc.c
> > new file mode 100644
> > index 0000000..40aee08
> > --- /dev/null
> > +++ b/drivers/mmc/host/cavium-mmc.c
>
> [...]
>
> > +
> > +static bool bad_status(union mio_emm_rsp_sts *rsp_sts)
> > +{
> > + if (rsp_sts->s.rsp_bad_sts || rsp_sts->s.rsp_crc_err ||
> > + rsp_sts->s.rsp_timeout || rsp_sts->s.blk_crc_err ||
> > + rsp_sts->s.blk_timeout || rsp_sts->s.dbuf_err)
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > +/* Try to clean up failed DMA. */
> > +static void cleanup_dma(struct cvm_mmc_host *host,
> > + union mio_emm_rsp_sts *rsp_sts)
> > +{
> > + union mio_emm_dma emm_dma;
> > +
> > + emm_dma.val = readq(host->base + MIO_EMM_DMA);
> > + emm_dma.s.dma_val = 1;
> > + emm_dma.s.dat_null = 1;
> > + emm_dma.s.bus_id = rsp_sts->s.bus_id;
> > + writeq(emm_dma.val, host->base + MIO_EMM_DMA);
> > +}
> > +
> > +irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
> > +{
> > + struct cvm_mmc_host *host = dev_id;
> > + union mio_emm_rsp_sts rsp_sts;
> > + union mio_emm_int emm_int;
> > + struct mmc_request *req;
> > + bool host_done;
> > +
> > + /* Clear interrupt bits (write 1 clears ). */
> > + emm_int.val = readq(host->base + MIO_EMM_INT);
> > + writeq(emm_int.val, host->base + MIO_EMM_INT);
> > +
> > + if (emm_int.s.switch_err)
> > + check_switch_errors(host);
> > +
> > + req = host->current_req;
> > + if (!req)
> > + goto out;
> > +
> > + rsp_sts.val = readq(host->base + MIO_EMM_RSP_STS);
> > + /*
> > + * dma_val set means DMA is still in progress. Don't touch
> > + * the request and wait for the interrupt indicating that
> > + * the DMA is finished.
> > + */
> > + if (rsp_sts.s.dma_val && host->dma_active)
> > + goto out;
> > +
> > + if (!host->dma_active && emm_int.s.buf_done && req->data) {
> > + unsigned int type = (rsp_sts.val >> 7) & 3;
> > +
> > + if (type == 1)
> > + do_read(host, req, rsp_sts.s.dbuf);
> > + else if (type == 2)
> > + do_write(req);
> > + }
> > +
> > + host_done = emm_int.s.cmd_done || emm_int.s.dma_done ||
> > + emm_int.s.cmd_err || emm_int.s.dma_err;
> > +
> > + if (!(host_done && req->done))
> > + goto no_req_done;
> > +
> > + if (bad_status(&rsp_sts))
> > + req->cmd->error = -EILSEQ;
>
> I don't think you should treat all errors as -EILSEQ. Please assign a
> proper error code, depending on the error.
Agreed, -ETIMEDOUT seems more appropriate for the timeouts. I'll go for
-EIO for the dbuf_err (buffer space missing). What should I use for the
CRC errors, -EILSEQ?
> > + else
> > + req->cmd->error = 0;
> > +
> > + if (host->dma_active && req->data)
> > + if (!finish_dma(host, req->data))
> > + goto no_req_done;
> > +
> > + set_cmd_response(host, req, &rsp_sts);
> > + if (emm_int.s.dma_err && rsp_sts.s.dma_pend)
> > + cleanup_dma(host, &rsp_sts);
> > +
> > + host->current_req = NULL;
> > + req->done(req);
> > +
> > +no_req_done:
> > + if (host_done)
> > + host->release_bus(host);
> > +out:
> > + return IRQ_RETVAL(emm_int.val != 0);
> > +}
> > +
> > +/*
> > + * Program DMA_CFG and if needed DMA_ADR.
> > + * Returns 0 on error, DMA address otherwise.
> > + */
> > +static u64 prepare_dma_single(struct cvm_mmc_host *host, struct mmc_data *data)
> > +{
> > + union mio_emm_dma_cfg dma_cfg;
> > + int count;
> > + u64 addr;
> > +
> > + count = dma_map_sg(host->dev, data->sg, data->sg_len,
> > + get_dma_dir(data));
> > + if (!count)
> > + return 0;
> > +
> > + dma_cfg.val = 0;
> > + dma_cfg.s.en = 1;
> > + dma_cfg.s.rw = (data->flags & MMC_DATA_WRITE) ? 1 : 0;
> > +#ifdef __LITTLE_ENDIAN
> > + dma_cfg.s.endian = 1;
> > +#endif
> > + dma_cfg.s.size = (sg_dma_len(&data->sg[0]) / 8) - 1;
> > +
> > + addr = sg_dma_address(&data->sg[0]);
> > + dma_cfg.s.adr = addr;
> > + writeq(dma_cfg.val, host->dma_base + MIO_EMM_DMA_CFG);
> > +
> > + pr_debug("[%s] sg_dma_len: %u total sg_elem: %d\n",
> > + (dma_cfg.s.rw) ? "W" : "R", sg_dma_len(&data->sg[0]), count);
> > + return addr;
> > +}
> > +
> > +static u64 prepare_dma(struct cvm_mmc_host *host, struct mmc_data *data)
> > +{
> > + return prepare_dma_single(host, data);
> > +}
> > +
> > +static void prepare_ext_dma(struct mmc_host *mmc, struct mmc_request *mrq,
> > + union mio_emm_dma *emm_dma)
> > +{
> > + struct cvm_mmc_slot *slot = mmc_priv(mmc);
> > +
> > + /*
> > + * Our MMC host hardware does not issue single commands,
> > + * because that would require the driver and the MMC core
> > + * to do work to determine the proper sequence of commands.
>
> I don't get this. The allowed sequence of the commands is determined
> by the SD/(e)MMC/SDIO spec and much of this knowledge is the
> responsibility of the mmc core.
>
> > + * Instead, our hardware is superior to most other MMC bus
>
> No need to brag about your HW. Let's just describe how it works instead.
I'll remove the comment.
> > + * hosts. The sequence of MMC commands required to execute
> > + * a transfer are issued automatically by the bus hardware.
>
> What does this really mean? Is this about HW support for better
> dealing with data requests?
Did David's reponse answer your questions?
> > + *
> > + * - David Daney <[email protected]>
> > + */
> > + emm_dma->val = 0;
> > + emm_dma->s.bus_id = slot->bus_id;
> > + emm_dma->s.dma_val = 1;
> > + emm_dma->s.sector = (mrq->data->blksz == 512) ? 1 : 0;
> > + emm_dma->s.rw = (mrq->data->flags & MMC_DATA_WRITE) ? 1 : 0;
> > + emm_dma->s.block_cnt = mrq->data->blocks;
> > + emm_dma->s.card_addr = mrq->cmd->arg;
> > + if (mmc_card_mmc(mmc->card) || (mmc_card_sd(mmc->card) &&
> > + (mmc->card->scr.cmds & SD_SCR_CMD23_SUPPORT)))
> > + emm_dma->s.multi = 1;
> > +
> > + pr_debug("[%s] blocks: %u multi: %d\n", (emm_dma->s.rw) ? "W" : "R",
> > + mrq->data->blocks, emm_dma->s.multi);
> > +}
> > +
>
> [...]
>
> > +
> > +static void cvm_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> > +{
> > + struct cvm_mmc_slot *slot = mmc_priv(mmc);
> > + struct cvm_mmc_host *host = slot->host;
> > + int clk_period, power_class = 10, bus_width = 0;
> > + union mio_emm_switch emm_switch;
> > + u64 clock;
> > +
> > + host->acquire_bus(host);
> > + cvm_mmc_switch_to(slot);
> > +
> > + /* Set the power state */
> > + switch (ios->power_mode) {
> > + case MMC_POWER_ON:
> > + break;
> > +
> > + case MMC_POWER_OFF:
> > + cvm_mmc_reset_bus(slot);
> > +
> > + if (host->global_pwr_gpiod)
> > + gpiod_set_value_cansleep(host->global_pwr_gpiod, 0);
> > + else
> > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
> > + break;
> > +
> > + case MMC_POWER_UP:
> > + if (host->global_pwr_gpiod)
> > + gpiod_set_value_cansleep(host->global_pwr_gpiod, 1);
> > + else
> > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, ios->vdd);
> > + break;
> > + }
> > +
> > + /* Set bus width */
> > + switch (ios->bus_width) {
> > + case MMC_BUS_WIDTH_8:
> > + bus_width = 2;
> > + break;
> > + case MMC_BUS_WIDTH_4:
> > + bus_width = 1;
> > + break;
> > + case MMC_BUS_WIDTH_1:
> > + bus_width = 0;
> > + break;
> > + }
> > +
> > + slot->bus_width = bus_width;
> > +
> > + if (!ios->clock)
>
> There are cases when the core change the clock rate to 0, and then it
> expects the mmc host to gate the clock. It probably a good idea for
> you to do that as well.
OK, seems to work.
> > + goto out;
> > +
> > + /* Change the clock frequency. */
> > + clock = ios->clock;
> > + if (clock > 52000000)
> > + clock = 52000000;
> > + slot->clock = clock;
> > + clk_period = (host->sys_freq + clock - 1) / (2 * clock);
> > +
> > + emm_switch.val = 0;
> > + emm_switch.s.hs_timing = (ios->timing == MMC_TIMING_MMC_HS);
> > + emm_switch.s.bus_width = bus_width;
> > + emm_switch.s.power_class = power_class;
> > + emm_switch.s.clk_hi = clk_period;
> > + emm_switch.s.clk_lo = clk_period;
> > + emm_switch.s.bus_id = slot->bus_id;
> > +
> > + if (!switch_val_changed(slot, emm_switch.val))
> > + goto out;
> > +
> > + set_wdog(slot, 0);
> > + do_switch(host, emm_switch.val);
> > + slot->cached_switch = emm_switch.val;
> > +out:
> > + host->release_bus(host);
> > +}
>
> [...]
>
> > +
> > +static int set_bus_width(struct device *dev, struct cvm_mmc_slot *slot, u32 id)
> > +{
> > + u32 bus_width;
> > + int ret;
> > +
> > + /*
> > + * The "cavium,bus-max-width" property is DEPRECATED and should
> > + * not be used. We handle it here to support older firmware.
> > + * Going forward, the standard "bus-width" property is used
> > + * instead of the Cavium-specific property.
> > + */
> > + if (!(slot->mmc->caps & (MMC_CAP_8_BIT_DATA | MMC_CAP_4_BIT_DATA))) {
> > + /* Try legacy "cavium,bus-max-width" property. */
> > + ret = of_property_read_u32(dev->of_node, "cavium,bus-max-width",
> > + &bus_width);
> > + if (ret) {
> > + /* No bus width specified, use default. */
> > + bus_width = 8;
> > + dev_info(dev, "Default width 8 used for slot %u\n", id);
> > + }
> > + } else {
> > + /* Hosts capable of 8-bit transfers can also do 4 bits */
> > + bus_width = (slot->mmc->caps & MMC_CAP_8_BIT_DATA) ? 8 : 4;
> > + }
>
> This looks a bit unnessarry complex.
>
> I would instead suggest the following order of how to perform the OF
> parsing. Bindings that get parsed later, overrides the earlier.
>
> 1. Parse deprecated bindings.
> 2. Parse cavium specific bindings.
> 3. Parse common mmc bindings.
> 4. Check some caps, to make sure those have valid values as to cover
> cases when the OF parsing didn't find values.
>
> The same comment applies for the other OF parsing functions below.
OK.
> > +
> > + switch (bus_width) {
> > + case 8:
> > + slot->bus_width = (MMC_BUS_WIDTH_8 - 1);
> > + slot->mmc->caps = MMC_CAP_8_BIT_DATA | MMC_CAP_4_BIT_DATA;
> > + break;
> > + case 4:
> > + slot->bus_width = (MMC_BUS_WIDTH_4 - 1);
> > + slot->mmc->caps = MMC_CAP_4_BIT_DATA;
> > + break;
> > + case 1:
> > + slot->bus_width = MMC_BUS_WIDTH_1;
> > + break;
> > + default:
> > + dev_err(dev, "Invalid bus width for slot %u\n", id);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static void set_frequency(struct device *dev, struct mmc_host *mmc, u32 id)
> > +{
> > + int ret;
> > +
> > + /*
> > + * The "spi-max-frequency" property is DEPRECATED and should
> > + * not be used. We handle it here to support older firmware.
> > + * Going forward, the standard "max-frequency" property is
> > + * used instead of the Cavium-specific property.
> > + */
> > + if (mmc->f_max == 0) {
> > + /* Try legacy "spi-max-frequency" property. */
> > + ret = of_property_read_u32(dev->of_node, "spi-max-frequency",
> > + &mmc->f_max);
> > + if (ret) {
> > + /* No frequency properties found, use default. */
> > + mmc->f_max = 52000000;
> > + dev_info(dev, "Default %u frequency used for slot %u\n",
> > + mmc->f_max, id);
> > + }
> > + } else if (mmc->f_max > 52000000)
> > + mmc->f_max = 52000000;
> > +
> > + /* Set minimum frequency */
> > + mmc->f_min = 400000;
> > +}
> > +
> > +static int set_voltage(struct device *dev, struct mmc_host *mmc,
> > + struct cvm_mmc_host *host)
> > +{
> > + int ret;
> > +
> > + /*
> > + * Legacy platform doesn't support regulator but enables power gpio
> > + * directly during platform probe.
> > + */
> > + if (host->global_pwr_gpiod)
> > + /* Get a sane OCR mask for other parts of the MMC subsytem. */
> > + return mmc_of_parse_voltage(dev->of_node, &mmc->ocr_avail);
>
> Does really the legacy platforms use the mmc voltage range DT bindings!?
The legacy DT's use (in the mmc slot nodes):
voltage-ranges = <3300 3300>;
> I would rather see that you assign a default value to mmc->ocr_avail,
> than using this binding.
The volatage seems to be identical for all legacy bindings I can find,
so is it better to not parse it and use the 3.3 as default?
> > +
> > + mmc->supply.vmmc = devm_regulator_get(dev, "vmmc");
> > + if (IS_ERR(mmc->supply.vmmc)) {
> > + ret = PTR_ERR(mmc->supply.vmmc);
> > + } else {
> > + ret = mmc_regulator_get_ocrmask(mmc->supply.vmmc);
> > + if (ret > 0) {
> > + mmc->ocr_avail = ret;
> > + ret = 0;
> > + }
> > + }
>
> This if-else-if is a bit messy.
>
> Why not just return when you get an error instead. That should simply the code.
OK, I'll simplify it.
> Maybe you can have look and try to clean up this in the hole file
> where you think it would make an improvment.
>
> > + return ret;
> > +}
> > +
> > +int cvm_mmc_slot_probe(struct device *dev, struct cvm_mmc_host *host)
>
> To reflect that OF is needed, perhaps rename function to
> cvm_mmc_of_slot_probe().
OK.
> > +{
> > + struct device_node *node = dev->of_node;
> > + u32 id, cmd_skew, dat_skew;
> > + struct cvm_mmc_slot *slot;
> > + struct mmc_host *mmc;
> > + u64 clock_period;
> > + int ret;
> > +
> > + ret = of_property_read_u32(node, "reg", &id);
> > + if (ret) {
> > + dev_err(dev, "Missing or invalid reg property on %s\n",
> > + of_node_full_name(node));
> > + return ret;
> > + }
> > +
> > + if (id >= CAVIUM_MAX_MMC || host->slot[id]) {
> > + dev_err(dev, "Invalid reg property on %s\n",
> > + of_node_full_name(node));
> > + return -EINVAL;
> > + }
> > +
> > + mmc = mmc_alloc_host(sizeof(struct cvm_mmc_slot), dev);
> > + if (!mmc)
> > + return -ENOMEM;
> > +
> > + slot = mmc_priv(mmc);
> > + slot->mmc = mmc;
> > + slot->host = host;
> > +
> > + ret = mmc_of_parse(mmc);
> > + if (ret)
> > + goto error;
> > +
> > + ret = set_bus_width(dev, slot, id);
> > + if (ret)
> > + goto error;
> > +
> > + set_frequency(dev, mmc, id);
> > +
> > + /* Octeon-specific DT properties. */
> > + ret = of_property_read_u32(node, "cavium,cmd-clk-skew", &cmd_skew);
> > + if (ret)
> > + cmd_skew = 0;
> > + ret = of_property_read_u32(node, "cavium,dat-clk-skew", &dat_skew);
> > + if (ret)
> > + dat_skew = 0;
> > +
> > + ret = set_voltage(dev, mmc, host);
> > + if (ret < 0)
> > + goto error;
>
> The functions set_bus_width(), set_freqeuncy(), set_voltage() all
> performs OF parsing and there are some parsing also being done above.
>
> I would suggest you bundle all OF parsing into one function, perhaps
> name it "cvm_mmc_of_parse()" or similar. That should make the code a
> lot cleaner.
OK.
> > +
> > + /* Set up host parameters */
> > + mmc->ops = &cvm_mmc_ops;
> > +
> > + mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
> > + MMC_CAP_ERASE | MMC_CAP_CMD23 | MMC_CAP_POWER_OFF_CARD;
> > +
> > + mmc->max_segs = 1;
> > +
> > + /* DMA size field can address up to 8 MB */
> > + mmc->max_seg_size = 8 * 1024 * 1024;
> > + mmc->max_req_size = mmc->max_seg_size;
> > + /* External DMA is in 512 byte blocks */
> > + mmc->max_blk_size = 512;
> > + /* DMA block count field is 15 bits */
> > + mmc->max_blk_count = 32767;
> > +
> > + slot->clock = mmc->f_min;
> > + slot->sclock = host->sys_freq;
> > +
> > + /* Period in picoseconds. */
> > + clock_period = 1000000000000ull / slot->sclock;
> > + slot->cmd_cnt = (cmd_skew + clock_period / 2) / clock_period;
> > + slot->dat_cnt = (dat_skew + clock_period / 2) / clock_period;
> > +
> > + slot->bus_id = id;
> > + slot->cached_rca = 1;
> > +
> > + host->acquire_bus(host);
> > + host->slot[id] = slot;
> > + cvm_mmc_switch_to(slot);
> > + cvm_mmc_init_lowlevel(slot);
> > + host->release_bus(host);
> > +
> > + ret = mmc_add_host(mmc);
> > + if (ret) {
> > + dev_err(dev, "mmc_add_host() returned %d\n", ret);
> > + goto error;
> > + }
> > +
> > + return 0;
> > +
> > +error:
> > + slot->host->slot[id] = NULL;
> > + mmc_free_host(slot->mmc);
> > + return ret;
> > +}
> > +
> > +int cvm_mmc_slot_remove(struct cvm_mmc_slot *slot)
> > +{
> > + mmc_remove_host(slot->mmc);
> > + slot->host->slot[slot->bus_id] = NULL;
> > + mmc_free_host(slot->mmc);
> > + return 0;
> > +}
> > diff --git a/drivers/mmc/host/cavium-mmc.h b/drivers/mmc/host/cavium-mmc.h
> > new file mode 100644
> > index 0000000..27fb02b
> > --- /dev/null
> > +++ b/drivers/mmc/host/cavium-mmc.h
> > @@ -0,0 +1,303 @@
> > +/*
> > + * Driver for MMC and SSD cards for Cavium OCTEON and ThunderX SOCs.
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License. See the file "COPYING" in the main directory of this archive
> > + * for more details.
> > + *
> > + * Copyright (C) 2012-2016 Cavium Inc.
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/mmc/host.h>
> > +#include <linux/of.h>
> > +#include <linux/scatterlist.h>
> > +#include <linux/semaphore.h>
> > +
> > +#define CAVIUM_MAX_MMC 4
> > +
> > +struct cvm_mmc_host {
> > + struct device *dev;
> > + void __iomem *base;
> > + void __iomem *dma_base;
> > + u64 emm_cfg;
> > + int last_slot;
> > + struct clk *clk;
> > + int sys_freq;
> > +
> > + struct mmc_request *current_req;
> > + struct sg_mapping_iter smi;
> > + bool dma_active;
> > +
> > + struct gpio_desc *global_pwr_gpiod;
> > +
> > + struct cvm_mmc_slot *slot[CAVIUM_MAX_MMC];
> > +
> > + void (*acquire_bus)(struct cvm_mmc_host *);
> > + void (*release_bus)(struct cvm_mmc_host *);
> > + void (*int_enable)(struct cvm_mmc_host *, u64);
> > +};
> > +
> > +struct cvm_mmc_slot {
> > + struct mmc_host *mmc; /* slot-level mmc_core object */
> > + struct cvm_mmc_host *host; /* common hw for all slots */
> > +
> > + u64 clock;
> > + unsigned int sclock;
> > +
> > + u64 cached_switch;
> > + u64 cached_rca;
> > +
> > + unsigned int cmd_cnt; /* sample delay */
> > + unsigned int dat_cnt; /* sample delay */
> > +
> > + int bus_width;
> > + int bus_id;
> > +};
> > +
> > +struct cvm_mmc_cr_type {
> > + u8 ctype;
> > + u8 rtype;
> > +};
> > +
> > +struct cvm_mmc_cr_mods {
> > + u8 ctype_xor;
> > + u8 rtype_xor;
> > +};
> > +
> > +/* Bitfield definitions */
> > +
> > +union mio_emm_cmd {
> > + u64 val;
> > + struct mio_emm_cmd_s {
> > +#ifdef __BIG_ENDIAN_BITFIELD
>
> Huh. Sorry, but this is a big nack from me.
>
> This isn't the common method for how we deal with endian issues in the
> kernel. Please remove all use of the union types here and below. The
> follow common patterns for how we deal with endian issues.
May I ask why you dislike the bitfields? Or maybe it is easier when I
explain why I decided to keep them:
- One drawback of bitfields is poor performance on some architectures.
That is not the case here, both MIPS64 and ARM64 have instructions
capable of using bitfields without performance impact.
- The used bitfield are all aligned to word size, usually the pattern in
the driver is to readq / writeq the whole word (therefore the union
val) and then set or read certain fields. That should avoid IMHO the
unspecified behaviour the C standard mentions.
- I prefer BIT_ULL and friends for single bits, but using macros for
more then one bit is (again IMHO) much less readable then using
bitfiels here. And all the endianess definitions are _only_ in the
header file.
Also, if I need to convert all of these I'll probably add some new bugs.
What we have currently works fine on both MIPS and ARM64.
> > + u64 :2;
> > + u64 bus_id:2;
> > + u64 cmd_val:1;
> > + u64 :3;
> > + u64 dbuf:1;
> > + u64 offset:6;
> > + u64 :6;
> > + u64 ctype_xor:2;
> > + u64 rtype_xor:3;
> > + u64 cmd_idx:6;
> > + u64 arg:32;
> > +#else
> > + u64 arg:32;
> > + u64 cmd_idx:6;
> > + u64 rtype_xor:3;
> > + u64 ctype_xor:2;
> > + u64 :6;
> > + u64 offset:6;
> > + u64 dbuf:1;
> > + u64 :3;
> > + u64 cmd_val:1;
> > + u64 bus_id:2;
> > + u64 :2;
> > +#endif
> > + } s;
> > +};
> > +
> > +union mio_emm_dma {
> > + u64 val;
> > + struct mio_emm_dma_s {
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > + u64 :2;
> > + u64 bus_id:2;
> > + u64 dma_val:1;
> > + u64 sector:1;
> > + u64 dat_null:1;
> > + u64 thres:6;
> > + u64 rel_wr:1;
> > + u64 rw:1;
> > + u64 multi:1;
> > + u64 block_cnt:16;
> > + u64 card_addr:32;
> > +#else
> > + u64 card_addr:32;
> > + u64 block_cnt:16;
> > + u64 multi:1;
> > + u64 rw:1;
> > + u64 rel_wr:1;
> > + u64 thres:6;
> > + u64 dat_null:1;
> > + u64 sector:1;
> > + u64 dma_val:1;
> > + u64 bus_id:2;
> > + u64 :2;
> > +#endif
> > + } s;
> > +};
> > +
> > +union mio_emm_dma_cfg {
> > + u64 val;
> > + struct mio_emm_dma_cfg_s {
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > + u64 en:1;
> > + u64 rw:1;
> > + u64 clr:1;
> > + u64 :1;
> > + u64 swap32:1;
> > + u64 swap16:1;
> > + u64 swap8:1;
> > + u64 endian:1;
> > + u64 size:20;
> > + u64 adr:36;
> > +#else
> > + u64 adr:36;
> > + u64 size:20;
> > + u64 endian:1;
> > + u64 swap8:1;
> > + u64 swap16:1;
> > + u64 swap32:1;
> > + u64 :1;
> > + u64 clr:1;
> > + u64 rw:1;
> > + u64 en:1;
> > +#endif
> > + } s;
> > +};
> > +
> > +union mio_emm_int {
> > + u64 val;
> > + struct mio_emm_int_s {
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > + u64 :57;
> > + u64 switch_err:1;
> > + u64 switch_done:1;
> > + u64 dma_err:1;
> > + u64 cmd_err:1;
> > + u64 dma_done:1;
> > + u64 cmd_done:1;
> > + u64 buf_done:1;
> > +#else
> > + u64 buf_done:1;
> > + u64 cmd_done:1;
> > + u64 dma_done:1;
> > + u64 cmd_err:1;
> > + u64 dma_err:1;
> > + u64 switch_done:1;
> > + u64 switch_err:1;
> > + u64 :57;
> > +#endif
> > + } s;
> > +};
> > +
> > +union mio_emm_rsp_sts {
> > + u64 val;
> > + struct mio_emm_rsp_sts_s {
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > + u64 :2;
> > + u64 bus_id:2;
> > + u64 cmd_val:1;
> > + u64 switch_val:1;
> > + u64 dma_val:1;
> > + u64 dma_pend:1;
> > + u64 :27;
> > + u64 dbuf_err:1;
> > + u64 :4;
> > + u64 dbuf:1;
> > + u64 blk_timeout:1;
> > + u64 blk_crc_err:1;
> > + u64 rsp_busybit:1;
> > + u64 stp_timeout:1;
> > + u64 stp_crc_err:1;
> > + u64 stp_bad_sts:1;
> > + u64 stp_val:1;
> > + u64 rsp_timeout:1;
> > + u64 rsp_crc_err:1;
> > + u64 rsp_bad_sts:1;
> > + u64 rsp_val:1;
> > + u64 rsp_type:3;
> > + u64 cmd_type:2;
> > + u64 cmd_idx:6;
> > + u64 cmd_done:1;
> > +#else
> > + u64 cmd_done:1;
> > + u64 cmd_idx:6;
> > + u64 cmd_type:2;
> > + u64 rsp_type:3;
> > + u64 rsp_val:1;
> > + u64 rsp_bad_sts:1;
> > + u64 rsp_crc_err:1;
> > + u64 rsp_timeout:1;
> > + u64 stp_val:1;
> > + u64 stp_bad_sts:1;
> > + u64 stp_crc_err:1;
> > + u64 stp_timeout:1;
> > + u64 rsp_busybit:1;
> > + u64 blk_crc_err:1;
> > + u64 blk_timeout:1;
> > + u64 dbuf:1;
> > + u64 :4;
> > + u64 dbuf_err:1;
> > + u64 :27;
> > + u64 dma_pend:1;
> > + u64 dma_val:1;
> > + u64 switch_val:1;
> > + u64 cmd_val:1;
> > + u64 bus_id:2;
> > + u64 :2;
> > +#endif
> > + } s;
> > +};
> > +
> > +union mio_emm_sample {
> > + u64 val;
> > + struct mio_emm_sample_s {
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > + u64 :38;
> > + u64 cmd_cnt:10;
> > + u64 :6;
> > + u64 dat_cnt:10;
> > +#else
> > + u64 dat_cnt:10;
> > + u64 :6;
> > + u64 cmd_cnt:10;
> > + u64 :38;
> > +#endif
> > + } s;
> > +};
> > +
> > +union mio_emm_switch {
> > + u64 val;
> > + struct mio_emm_switch_s {
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > + u64 :2;
> > + u64 bus_id:2;
> > + u64 switch_exe:1;
> > + u64 switch_err0:1;
> > + u64 switch_err1:1;
> > + u64 switch_err2:1;
> > + u64 :7;
> > + u64 hs_timing:1;
> > + u64 :5;
> > + u64 bus_width:3;
> > + u64 :4;
> > + u64 power_class:4;
> > + u64 clk_hi:16;
> > + u64 clk_lo:16;
> > +#else
> > + u64 clk_lo:16;
> > + u64 clk_hi:16;
> > + u64 power_class:4;
> > + u64 :4;
> > + u64 bus_width:3;
> > + u64 :5;
> > + u64 hs_timing:1;
> > + u64 :7;
> > + u64 switch_err2:1;
> > + u64 switch_err1:1;
> > + u64 switch_err0:1;
> > + u64 switch_exe:1;
> > + u64 bus_id:2;
> > + u64 :2;
> > +#endif
> > + } s;
> > +};
> > +
> > +/* Protoypes */
> > +irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id);
> > +int cvm_mmc_slot_probe(struct device *dev, struct cvm_mmc_host *host);
> > +int cvm_mmc_slot_remove(struct cvm_mmc_slot *slot);
>
> Why do you need this here? Are those intended as library functions for
> the different cavium variant drivers?
Yes, this is the minimum I need to share the cavium-mmc-core.
> > +extern const struct mmc_host_ops cvm_mmc_ops;
>
> Why do you need this?
Left-over from development, can be removed now.
> Kind regards
> Uffe
Thanks for the review!
Jan
[...]
>> > Voltage is limited to 3.3v and shared for all slots.
>>
>> What voltage? The I/O voltage or the voltage for the card?
>>
>> VMMC or VMMCQ?
>
> From my understanding both, VMMC and VMMCQ are fixed at 3.3v.
Okay, then make sure to explicitly state that here.
[...]
>> > + if (bad_status(&rsp_sts))
>> > + req->cmd->error = -EILSEQ;
>>
>> I don't think you should treat all errors as -EILSEQ. Please assign a
>> proper error code, depending on the error.
>
> Agreed, -ETIMEDOUT seems more appropriate for the timeouts. I'll go for
> -EIO for the dbuf_err (buffer space missing). What should I use for the
> CRC errors, -EILSEQ?
Yes, correct.
[...]
>> What does this really mean? Is this about HW support for better
>> dealing with data requests?
>
> Did David's reponse answer your questions?
Yes.
[...]
>> > + /*
>> > + * Legacy platform doesn't support regulator but enables power gpio
>> > + * directly during platform probe.
>> > + */
>> > + if (host->global_pwr_gpiod)
>> > + /* Get a sane OCR mask for other parts of the MMC subsytem. */
>> > + return mmc_of_parse_voltage(dev->of_node, &mmc->ocr_avail);
>>
>> Does really the legacy platforms use the mmc voltage range DT bindings!?
>
> The legacy DT's use (in the mmc slot nodes):
>
> voltage-ranges = <3300 3300>;
>
>> I would rather see that you assign a default value to mmc->ocr_avail,
>> than using this binding.
>
> The volatage seems to be identical for all legacy bindings I can find,
> so is it better to not parse it and use the 3.3 as default?
Yes, I think so.
[...]
>> > +union mio_emm_cmd {
>> > + u64 val;
>> > + struct mio_emm_cmd_s {
>> > +#ifdef __BIG_ENDIAN_BITFIELD
>>
>> Huh. Sorry, but this is a big nack from me.
>>
>> This isn't the common method for how we deal with endian issues in the
>> kernel. Please remove all use of the union types here and below. The
>> follow common patterns for how we deal with endian issues.
>
> May I ask why you dislike the bitfields? Or maybe it is easier when I
> explain why I decided to keep them:
My main concern is that is different compared to how we deal with
endian issues in the kernel.
I just don't like homebrewed hacks, but prefers sticking to defacto
standard methods.
>
> - One drawback of bitfields is poor performance on some architectures.
> That is not the case here, both MIPS64 and ARM64 have instructions
> capable of using bitfields without performance impact.
>
> - The used bitfield are all aligned to word size, usually the pattern in
> the driver is to readq / writeq the whole word (therefore the union
> val) and then set or read certain fields. That should avoid IMHO the
> unspecified behaviour the C standard mentions.
>
> - I prefer BIT_ULL and friends for single bits, but using macros for
> more then one bit is (again IMHO) much less readable then using
> bitfiels here. And all the endianess definitions are _only_ in the
> header file.
>
> Also, if I need to convert all of these I'll probably add some new bugs.
> What we have currently works fine on both MIPS and ARM64.
I understand that is will have an impact, however there are plenty of
good references in the kernel for how to do this.
[...]
Kind regards
Uffe
On Wed, Mar 08, 2017 at 10:45:19AM +0100, Ulf Hansson wrote:
[...]
> > May I ask why you dislike the bitfields? Or maybe it is easier when I
> > explain why I decided to keep them:
>
> My main concern is that is different compared to how we deal with
> endian issues in the kernel.
>
> I just don't like homebrewed hacks, but prefers sticking to defacto
> standard methods.
I don't see it as a homebrew hack, the BIG/LITTLE_ENDIAN_BITFIELD macros
are already used in the kernel. In my eyes it is a straight-forward and
obvious thing.
> >
> > - One drawback of bitfields is poor performance on some architectures.
> > That is not the case here, both MIPS64 and ARM64 have instructions
> > capable of using bitfields without performance impact.
> >
> > - The used bitfield are all aligned to word size, usually the pattern in
> > the driver is to readq / writeq the whole word (therefore the union
> > val) and then set or read certain fields. That should avoid IMHO the
> > unspecified behaviour the C standard mentions.
> >
> > - I prefer BIT_ULL and friends for single bits, but using macros for
> > more then one bit is (again IMHO) much less readable then using
> > bitfiels here. And all the endianess definitions are _only_ in the
> > header file.
> >
> > Also, if I need to convert all of these I'll probably add some new bugs.
> > What we have currently works fine on both MIPS and ARM64.
>
> I understand that is will have an impact, however there are plenty of
> good references in the kernel for how to do this.
As an experiment I've converted the bitfields to use FIELD_PREP|GET or
plain logic, see below patch (against unreleased v12, just to show the
difference).
While the header file looks cleaner I think the code is much harder to
read. Is there a better way to do this? Is it really worth it?
thanks,
Jan
> [...]
>
> Kind regards
> Uffe
---
diff --git a/drivers/mmc/host/cavium-mmc.c b/drivers/mmc/host/cavium-mmc.c
index b507a7a..b899720 100644
--- a/drivers/mmc/host/cavium-mmc.c
+++ b/drivers/mmc/host/cavium-mmc.c
@@ -13,6 +13,7 @@
* Steven J. Hill <[email protected]>
* Jan Glauber <[email protected]>
*/
+#include <linux/bitfield.h>
#include <linux/delay.h>
#include <linux/dma-direction.h>
#include <linux/dma-mapping.h>
@@ -151,14 +152,14 @@ static struct cvm_mmc_cr_mods cvm_mmc_get_cr_mods(struct mmc_command *cmd)
static void check_switch_errors(struct cvm_mmc_host *host)
{
- union mio_emm_switch emm_switch;
+ u64 emm_switch;
- emm_switch.val = readq(host->base + MIO_EMM_SWITCH(host));
- if (emm_switch.s.switch_err0)
+ emm_switch = readq(host->base + MIO_EMM_SWITCH(host));
+ if (emm_switch & MIO_EMM_SWITCH_ERR0)
dev_err(host->dev, "Switch power class error\n");
- if (emm_switch.s.switch_err1)
+ if (emm_switch & MIO_EMM_SWITCH_ERR1)
dev_err(host->dev, "Switch hs timing error\n");
- if (emm_switch.s.switch_err2)
+ if (emm_switch & MIO_EMM_SWITCH_ERR2)
dev_err(host->dev, "Switch bus width error\n");
}
@@ -168,28 +169,25 @@ static void check_switch_errors(struct cvm_mmc_host *host)
*/
static void do_switch(struct cvm_mmc_host *host, u64 val)
{
- union mio_emm_rsp_sts rsp_sts;
- union mio_emm_switch emm_switch;
+ u64 rsp_sts, emm_switch = val;
int retries = 100;
int bus_id;
- emm_switch.val = val;
-
/*
* Modes setting only taken from slot 0. Work around that hardware
* issue by first switching to slot 0.
*/
- bus_id = emm_switch.s.bus_id;
- emm_switch.s.bus_id = 0;
- writeq(emm_switch.val, host->base + MIO_EMM_SWITCH(host));
+ bus_id = FIELD_GET(MIO_EMM_SWITCH_BUS_ID, emm_switch);
+ emm_switch &= ~MIO_EMM_SWITCH_BUS_ID;
+ writeq(emm_switch, host->base + MIO_EMM_SWITCH(host));
- emm_switch.s.bus_id = bus_id;
- writeq(emm_switch.val, host->base + MIO_EMM_SWITCH(host));
+ emm_switch |= FIELD_PREP(MIO_EMM_SWITCH_BUS_ID, bus_id);
+ writeq(emm_switch, host->base + MIO_EMM_SWITCH(host));
/* wait for the switch to finish */
do {
- rsp_sts.val = readq(host->base + MIO_EMM_RSP_STS(host));
- if (!rsp_sts.s.switch_val)
+ rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
+ if (!(rsp_sts & MIO_EMM_RSP_STS_SWITCH_VAL))
break;
udelay(10);
} while (--retries);
@@ -222,20 +220,18 @@ static void set_wdog(struct cvm_mmc_slot *slot, unsigned int ns)
static void cvm_mmc_reset_bus(struct cvm_mmc_slot *slot)
{
struct cvm_mmc_host *host = slot->host;
- union mio_emm_switch emm_switch;
- u64 wdog = 0;
+ u64 emm_switch, wdog;
- emm_switch.val = readq(slot->host->base + MIO_EMM_SWITCH(host));
- wdog = readq(slot->host->base + MIO_EMM_WDOG(host));
+ emm_switch = readq(slot->host->base + MIO_EMM_SWITCH(host));
+ emm_switch &= ~(MIO_EMM_SWITCH_EXE | MIO_EMM_SWITCH_ERR0 |
+ MIO_EMM_SWITCH_ERR1 | MIO_EMM_SWITCH_ERR2 |
+ MIO_EMM_SWITCH_BUS_ID);
+ emm_switch |= FIELD_PREP(MIO_EMM_SWITCH_BUS_ID, slot->bus_id);
- emm_switch.s.switch_exe = 0;
- emm_switch.s.switch_err0 = 0;
- emm_switch.s.switch_err1 = 0;
- emm_switch.s.switch_err2 = 0;
- emm_switch.s.bus_id = slot->bus_id;
- do_switch(slot->host, emm_switch.val);
+ wdog = readq(slot->host->base + MIO_EMM_WDOG(host));
+ do_switch(slot->host, emm_switch);
- slot->cached_switch = emm_switch.val;
+ slot->cached_switch = emm_switch;
msleep(20);
@@ -247,8 +243,7 @@ static void cvm_mmc_switch_to(struct cvm_mmc_slot *slot)
{
struct cvm_mmc_host *host = slot->host;
struct cvm_mmc_slot *old_slot;
- union mio_emm_switch emm_switch;
- union mio_emm_sample emm_sample;
+ u64 emm_sample, emm_switch;
if (slot->bus_id == host->last_slot)
return;
@@ -260,14 +255,14 @@ static void cvm_mmc_switch_to(struct cvm_mmc_slot *slot)
}
writeq(slot->cached_rca, host->base + MIO_EMM_RCA(host));
- emm_switch.val = slot->cached_switch;
- emm_switch.s.bus_id = slot->bus_id;
- do_switch(host, emm_switch.val);
+ emm_switch = slot->cached_switch;
+ emm_switch &= ~MIO_EMM_SWITCH_BUS_ID;
+ emm_switch |= FIELD_PREP(MIO_EMM_SWITCH_BUS_ID, slot->bus_id);
+ do_switch(host, emm_switch);
- emm_sample.val = 0;
- emm_sample.s.cmd_cnt = slot->cmd_cnt;
- emm_sample.s.dat_cnt = slot->dat_cnt;
- writeq(emm_sample.val, host->base + MIO_EMM_SAMPLE(host));
+ emm_sample = FIELD_PREP(MIO_EMM_SAMPLE_CMD_CNT, slot->cmd_cnt) |
+ FIELD_PREP(MIO_EMM_SAMPLE_DAT_CNT, slot->dat_cnt);
+ writeq(emm_sample, host->base + MIO_EMM_SAMPLE(host));
host->last_slot = slot->bus_id;
}
@@ -315,16 +310,16 @@ static void do_write(struct mmc_request *req)
}
static void set_cmd_response(struct cvm_mmc_host *host, struct mmc_request *req,
- union mio_emm_rsp_sts *rsp_sts)
+ u64 rsp_sts)
{
u64 rsp_hi, rsp_lo;
- if (!rsp_sts->s.rsp_val)
+ if (!(rsp_sts & MIO_EMM_RSP_STS_RSP_VAL))
return;
rsp_lo = readq(host->base + MIO_EMM_RSP_LO(host));
- switch (rsp_sts->s.rsp_type) {
+ switch (FIELD_GET(MIO_EMM_RSP_STS_RSP_TYPE, rsp_sts)) {
case 1:
case 3:
req->cmd->resp[0] = (rsp_lo >> 8) & 0xffffffff;
@@ -356,13 +351,14 @@ static int finish_dma_single(struct cvm_mmc_host *host, struct mmc_data *data)
static int finish_dma_sg(struct cvm_mmc_host *host, struct mmc_data *data)
{
- union mio_emm_dma_fifo_cfg fifo_cfg;
+ u64 fifo_cfg;
+ int count;
/* Check if there are any pending requests left */
- fifo_cfg.val = readq(host->dma_base + MIO_EMM_DMA_FIFO_CFG(host));
- if (fifo_cfg.s.count)
- dev_err(host->dev, "%u requests still pending\n",
- fifo_cfg.s.count);
+ fifo_cfg = readq(host->dma_base + MIO_EMM_DMA_FIFO_CFG(host));
+ count = FIELD_GET(MIO_EMM_DMA_FIFO_CFG_COUNT, fifo_cfg);
+ if (count)
+ dev_err(host->dev, "%u requests still pending\n", count);
data->bytes_xfered = data->blocks * data->blksz;
data->error = 0;
@@ -381,38 +377,39 @@ static int finish_dma(struct cvm_mmc_host *host, struct mmc_data *data)
return finish_dma_single(host, data);
}
-static int check_status(union mio_emm_rsp_sts *rsp_sts)
+static int check_status(u64 rsp_sts)
{
- if (rsp_sts->s.rsp_bad_sts || rsp_sts->s.rsp_crc_err ||
- rsp_sts->s.blk_crc_err)
+ if (rsp_sts & MIO_EMM_RSP_STS_RSP_BAD_STS ||
+ rsp_sts & MIO_EMM_RSP_STS_RSP_CRC_ERR ||
+ rsp_sts & MIO_EMM_RSP_STS_BLK_CRC_ERR)
return -EILSEQ;
- if (rsp_sts->s.rsp_timeout || rsp_sts->s.blk_timeout)
+ if (rsp_sts & MIO_EMM_RSP_STS_RSP_TIMEOUT ||
+ rsp_sts & MIO_EMM_RSP_STS_BLK_TIMEOUT)
return -ETIMEDOUT;
- if (rsp_sts->s.dbuf_err)
+ if (rsp_sts & MIO_EMM_RSP_STS_DBUF_ERR)
return -EIO;
return 0;
}
/* Try to clean up failed DMA. */
-static void cleanup_dma(struct cvm_mmc_host *host,
- union mio_emm_rsp_sts *rsp_sts)
+static void cleanup_dma(struct cvm_mmc_host *host, u64 rsp_sts)
{
- union mio_emm_dma emm_dma;
-
- emm_dma.val = readq(host->base + MIO_EMM_DMA(host));
- emm_dma.s.dma_val = 1;
- emm_dma.s.dat_null = 1;
- emm_dma.s.bus_id = rsp_sts->s.bus_id;
- writeq(emm_dma.val, host->base + MIO_EMM_DMA(host));
+ u64 emm_dma;
+
+ emm_dma = readq(host->base + MIO_EMM_DMA(host));
+ emm_dma &= ~MIO_EMM_DMA_BUS_ID;
+ emm_dma |= FIELD_PREP(MIO_EMM_DMA_VAL, 1) |
+ FIELD_PREP(MIO_EMM_DMA_DAT_NULL, 1) |
+ FIELD_PREP(MIO_EMM_DMA_BUS_ID, FIELD_GET(MIO_EMM_RSP_STS_BUS_ID, rsp_sts));
+ writeq(emm_dma, host->base + MIO_EMM_DMA(host));
}
irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
{
struct cvm_mmc_host *host = dev_id;
- union mio_emm_rsp_sts rsp_sts;
- union mio_emm_int emm_int;
struct mmc_request *req;
unsigned long flags = 0;
+ u64 emm_int, rsp_sts;
bool host_done;
if (host->need_irq_handler_lock)
@@ -421,49 +418,53 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
__acquire(&host->irq_handler_lock);
/* Clear interrupt bits (write 1 clears ). */
- emm_int.val = readq(host->base + MIO_EMM_INT(host));
- writeq(emm_int.val, host->base + MIO_EMM_INT(host));
+ emm_int = readq(host->base + MIO_EMM_INT(host));
+ writeq(emm_int, host->base + MIO_EMM_INT(host));
- if (emm_int.s.switch_err)
+ if (emm_int & MIO_EMM_INT_SWITCH_ERR)
check_switch_errors(host);
req = host->current_req;
if (!req)
goto out;
- rsp_sts.val = readq(host->base + MIO_EMM_RSP_STS(host));
+ rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
/*
* dma_val set means DMA is still in progress. Don't touch
* the request and wait for the interrupt indicating that
* the DMA is finished.
*/
- if (rsp_sts.s.dma_val && host->dma_active)
+ if ((rsp_sts & MIO_EMM_RSP_STS_DMA_VAL) && host->dma_active)
goto out;
- if (!host->dma_active && emm_int.s.buf_done && req->data) {
- unsigned int type = (rsp_sts.val >> 7) & 3;
+ if (!host->dma_active && req->data &&
+ (emm_int & MIO_EMM_INT_BUF_DONE)) {
+ unsigned int type = (rsp_sts >> 7) & 3;
if (type == 1)
- do_read(host, req, rsp_sts.s.dbuf);
+ do_read(host, req, rsp_sts & MIO_EMM_RSP_STS_DBUF);
else if (type == 2)
do_write(req);
}
- host_done = emm_int.s.cmd_done || emm_int.s.dma_done ||
- emm_int.s.cmd_err || emm_int.s.dma_err;
+ host_done = emm_int & MIO_EMM_INT_CMD_DONE ||
+ emm_int & MIO_EMM_INT_DMA_DONE ||
+ emm_int & MIO_EMM_INT_CMD_ERR ||
+ emm_int & MIO_EMM_INT_DMA_ERR;
if (!(host_done && req->done))
goto no_req_done;
- req->cmd->error = check_status(&rsp_sts);
+ req->cmd->error = check_status(rsp_sts);
if (host->dma_active && req->data)
if (!finish_dma(host, req->data))
goto no_req_done;
- set_cmd_response(host, req, &rsp_sts);
- if (emm_int.s.dma_err && rsp_sts.s.dma_pend)
- cleanup_dma(host, &rsp_sts);
+ set_cmd_response(host, req, rsp_sts);
+ if ((emm_int & MIO_EMM_INT_DMA_ERR) &&
+ (rsp_sts & MIO_EMM_RSP_STS_DMA_PEND))
+ cleanup_dma(host, rsp_sts);
host->current_req = NULL;
req->done(req);
@@ -478,7 +479,7 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
spin_unlock_irqrestore(&host->irq_handler_lock, flags);
else
__release(&host->irq_handler_lock);
- return IRQ_RETVAL(emm_int.val != 0);
+ return IRQ_RETVAL(emm_int != 0);
}
/*
@@ -487,30 +488,30 @@ irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id)
*/
static u64 prepare_dma_single(struct cvm_mmc_host *host, struct mmc_data *data)
{
- union mio_emm_dma_cfg dma_cfg;
- int count;
- u64 addr;
+ u64 dma_cfg, addr;
+ int count, rw;
count = dma_map_sg(host->dev, data->sg, data->sg_len,
get_dma_dir(data));
if (!count)
return 0;
- dma_cfg.val = 0;
- dma_cfg.s.en = 1;
- dma_cfg.s.rw = (data->flags & MMC_DATA_WRITE) ? 1 : 0;
+ rw = (data->flags & MMC_DATA_WRITE) ? 1 : 0;
+ dma_cfg = FIELD_PREP(MIO_EMM_DMA_CFG_EN, 1) |
+ FIELD_PREP(MIO_EMM_DMA_CFG_RW, rw);
#ifdef __LITTLE_ENDIAN
- dma_cfg.s.endian = 1;
+ dma_cfg |= FIELD_PREP(MIO_EMM_DMA_CFG_ENDIAN, 1);
#endif
- dma_cfg.s.size = (sg_dma_len(&data->sg[0]) / 8) - 1;
+ dma_cfg |= FIELD_PREP(MIO_EMM_DMA_CFG_SIZE,
+ (sg_dma_len(&data->sg[0]) / 8) - 1);
addr = sg_dma_address(&data->sg[0]);
if (!host->big_dma_addr)
- dma_cfg.s.adr = addr;
- writeq(dma_cfg.val, host->dma_base + MIO_EMM_DMA_CFG(host));
+ dma_cfg |= FIELD_PREP(MIO_EMM_DMA_CFG_ADR, addr);
+ writeq(dma_cfg, host->dma_base + MIO_EMM_DMA_CFG(host));
pr_debug("[%s] sg_dma_len: %u total sg_elem: %d\n",
- (dma_cfg.s.rw) ? "W" : "R", sg_dma_len(&data->sg[0]), count);
+ (rw) ? "W" : "R", sg_dma_len(&data->sg[0]), count);
if (host->big_dma_addr)
writeq(addr, host->dma_base + MIO_EMM_DMA_ADR(host));
@@ -523,10 +524,9 @@ static u64 prepare_dma_single(struct cvm_mmc_host *host, struct mmc_data *data)
*/
static u64 prepare_dma_sg(struct cvm_mmc_host *host, struct mmc_data *data)
{
- union mio_emm_dma_fifo_cmd fifo_cmd;
struct scatterlist *sg;
- int count, i;
- u64 addr;
+ u64 fifo_cmd, addr;
+ int count, i, rw;
count = dma_map_sg(host->dev, data->sg, data->sg_len,
get_dma_dir(data));
@@ -550,26 +550,25 @@ static u64 prepare_dma_sg(struct cvm_mmc_host *host, struct mmc_data *data)
* register for the DMA addr, so no need to check
* host->big_dma_addr here.
*/
- fifo_cmd.val = 0;
- fifo_cmd.s.rw = (data->flags & MMC_DATA_WRITE) ? 1 : 0;
+ rw = (data->flags & MMC_DATA_WRITE) ? 1 : 0;
+ fifo_cmd = FIELD_PREP(MIO_EMM_DMA_FIFO_CMD_RW, rw);
/* enable interrupts on the last element */
- if (i + 1 == count)
- fifo_cmd.s.intdis = 0;
- else
- fifo_cmd.s.intdis = 1;
+ fifo_cmd |= FIELD_PREP(MIO_EMM_DMA_FIFO_CMD_INTDIS,
+ (i + 1 == count) ? 0 : 1);
#ifdef __LITTLE_ENDIAN
- fifo_cmd.s.endian = 1;
+ fifo_cmd |= FIELD_PREP(MIO_EMM_DMA_FIFO_CMD_ENDIAN, 1);
#endif
- fifo_cmd.s.size = sg_dma_len(sg) / 8 - 1;
+ fifo_cmd |= FIELD_PREP(MIO_EMM_DMA_FIFO_CMD_SIZE,
+ sg_dma_len(sg) / 8 - 1);
/*
* The write copies the address and the command to the FIFO
* and increments the FIFO's COUNT field.
*/
- writeq(fifo_cmd.val, host->dma_base + MIO_EMM_DMA_FIFO_CMD(host));
+ writeq(fifo_cmd, host->dma_base + MIO_EMM_DMA_FIFO_CMD(host));
pr_debug("[%s] sg_dma_len: %u sg_elem: %d/%d\n",
- (fifo_cmd.s.rw) ? "W" : "R", sg_dma_len(sg), i, count);
+ (rw) ? "W" : "R", sg_dma_len(sg), i, count);
}
/*
@@ -596,32 +595,28 @@ static u64 prepare_dma(struct cvm_mmc_host *host, struct mmc_data *data)
return prepare_dma_single(host, data);
}
-static void prepare_ext_dma(struct mmc_host *mmc, struct mmc_request *mrq,
- union mio_emm_dma *emm_dma)
+static u64 prepare_ext_dma(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct cvm_mmc_slot *slot = mmc_priv(mmc);
+ u64 emm_dma = 0;
+
+ emm_dma = FIELD_PREP(MIO_EMM_DMA_BUS_ID, slot->bus_id) |
+ FIELD_PREP(MIO_EMM_DMA_VAL, 1) |
+ FIELD_PREP(MIO_EMM_DMA_SECTOR,
+ (mrq->data->blksz == 512) ? 1 : 0) |
+ FIELD_PREP(MIO_EMM_DMA_RW,
+ (mrq->data->flags & MMC_DATA_WRITE) ? 1 : 0) |
+ FIELD_PREP(MIO_EMM_DMA_BLOCK_CNT, mrq->data->blocks) |
+ FIELD_PREP(MIO_EMM_DMA_CARD_ADDR, mrq->cmd->arg);
- emm_dma->val = 0;
- emm_dma->s.bus_id = slot->bus_id;
- emm_dma->s.dma_val = 1;
- emm_dma->s.sector = (mrq->data->blksz == 512) ? 1 : 0;
- emm_dma->s.rw = (mrq->data->flags & MMC_DATA_WRITE) ? 1 : 0;
- emm_dma->s.block_cnt = mrq->data->blocks;
- emm_dma->s.card_addr = mrq->cmd->arg;
if (mmc_card_mmc(mmc->card) || (mmc_card_sd(mmc->card) &&
(mmc->card->scr.cmds & SD_SCR_CMD23_SUPPORT)))
- emm_dma->s.multi = 1;
-
- pr_debug("[%s] blocks: %u multi: %d\n", (emm_dma->s.rw) ? "W" : "R",
- mrq->data->blocks, emm_dma->s.multi);
-}
+ emm_dma |= FIELD_PREP(MIO_EMM_DMA_MULTI, 1);
-static void prepare_emm_int(union mio_emm_int *emm_int)
-{
- emm_int->val = 0;
- emm_int->s.cmd_err = 1;
- emm_int->s.dma_done = 1;
- emm_int->s.dma_err = 1;
+ pr_debug("[%s] blocks: %u multi: %d\n",
+ (emm_dma & MIO_EMM_DMA_RW) ? "W" : "R",
+ mrq->data->blocks, (emm_dma & MIO_EMM_DMA_MULTI) ? 1 : 0);
+ return emm_dma;
}
static void cvm_mmc_dma_request(struct mmc_host *mmc,
@@ -629,10 +624,8 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc,
{
struct cvm_mmc_slot *slot = mmc_priv(mmc);
struct cvm_mmc_host *host = slot->host;
- union mio_emm_dma emm_dma;
- union mio_emm_int emm_int;
struct mmc_data *data;
- u64 addr;
+ u64 emm_dma, addr;
if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len ||
!mrq->stop || mrq->stop->opcode != MMC_STOP_TRANSMISSION) {
@@ -652,16 +645,16 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc,
WARN_ON(host->current_req);
host->current_req = mrq;
- prepare_ext_dma(mmc, mrq, &emm_dma);
+ emm_dma = prepare_ext_dma(mmc, mrq);
addr = prepare_dma(host, data);
if (!addr) {
dev_err(host->dev, "prepare_dma failed\n");
goto error;
}
- prepare_emm_int(&emm_int);
host->dma_active = true;
- host->int_enable(host, emm_int.val);
+ host->int_enable(host, MIO_EMM_INT_CMD_ERR | MIO_EMM_INT_DMA_DONE |
+ MIO_EMM_INT_DMA_ERR);
if (host->dmar_fixup)
host->dmar_fixup(host, mrq->cmd, data, addr);
@@ -675,7 +668,7 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc,
writeq(0x00b00000ull, host->base + MIO_EMM_STS_MASK(host));
else
writeq(0xe4390080ull, host->base + MIO_EMM_STS_MASK(host));
- writeq(emm_dma.val, host->base + MIO_EMM_DMA(host));
+ writeq(emm_dma, host->base + MIO_EMM_DMA(host));
return;
error:
@@ -733,10 +726,8 @@ static void cvm_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
struct cvm_mmc_slot *slot = mmc_priv(mmc);
struct cvm_mmc_host *host = slot->host;
struct mmc_command *cmd = mrq->cmd;
- union mio_emm_int emm_int;
- union mio_emm_cmd emm_cmd;
struct cvm_mmc_cr_mods mods;
- union mio_emm_rsp_sts rsp_sts;
+ u64 emm_cmd, rsp_sts;
int retries = 100;
/*
@@ -761,10 +752,6 @@ static void cvm_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
WARN_ON(host->current_req);
host->current_req = mrq;
- emm_int.val = 0;
- emm_int.s.cmd_done = 1;
- emm_int.s.cmd_err = 1;
-
if (cmd->data) {
if (cmd->data->flags & MMC_DATA_READ)
do_read_request(host, mrq);
@@ -777,31 +764,33 @@ static void cvm_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
set_wdog(slot, 0);
host->dma_active = false;
- host->int_enable(host, emm_int.val);
-
- emm_cmd.val = 0;
- emm_cmd.s.cmd_val = 1;
- emm_cmd.s.ctype_xor = mods.ctype_xor;
- emm_cmd.s.rtype_xor = mods.rtype_xor;
+ host->int_enable(host, MIO_EMM_INT_CMD_DONE | MIO_EMM_INT_CMD_ERR);
+
+ emm_cmd = FIELD_PREP(MIO_EMM_CMD_VAL, 1) |
+ FIELD_PREP(MIO_EMM_CMD_CTYPE_XOR, mods.ctype_xor) |
+ FIELD_PREP(MIO_EMM_CMD_RTYPE_XOR, mods.rtype_xor) |
+ FIELD_PREP(MIO_EMM_CMD_BUS_ID, slot->bus_id) |
+ FIELD_PREP(MIO_EMM_CMD_IDX, cmd->opcode) |
+ FIELD_PREP(MIO_EMM_CMD_ARG, cmd->arg);
if (mmc_cmd_type(cmd) == MMC_CMD_ADTC)
- emm_cmd.s.offset = 64 - ((cmd->data->blocks * cmd->data->blksz) / 8);
- emm_cmd.s.bus_id = slot->bus_id;
- emm_cmd.s.cmd_idx = cmd->opcode;
- emm_cmd.s.arg = cmd->arg;
+ emm_cmd |= FIELD_PREP(MIO_EMM_CMD_OFFSET,
+ 64 - ((cmd->data->blocks * cmd->data->blksz) / 8));
writeq(0, host->base + MIO_EMM_STS_MASK(host));
retry:
- rsp_sts.val = readq(host->base + MIO_EMM_RSP_STS(host));
- if (rsp_sts.s.dma_val || rsp_sts.s.cmd_val ||
- rsp_sts.s.switch_val || rsp_sts.s.dma_pend) {
+ rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
+ if (rsp_sts & MIO_EMM_RSP_STS_DMA_VAL ||
+ rsp_sts & MIO_EMM_RSP_STS_CMD_VAL ||
+ rsp_sts & MIO_EMM_RSP_STS_SWITCH_VAL ||
+ rsp_sts & MIO_EMM_RSP_STS_DMA_PEND) {
udelay(10);
if (--retries)
goto retry;
}
if (!retries)
- dev_err(host->dev, "Bad status: %Lx before command write\n", rsp_sts.val);
- writeq(emm_cmd.val, host->base + MIO_EMM_CMD(host));
+ dev_err(host->dev, "Bad status: %Lx before command write\n", rsp_sts);
+ writeq(emm_cmd, host->base + MIO_EMM_CMD(host));
}
static void cvm_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
@@ -809,8 +798,7 @@ static void cvm_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct cvm_mmc_slot *slot = mmc_priv(mmc);
struct cvm_mmc_host *host = slot->host;
int clk_period = 0, power_class = 10, bus_width = 0;
- union mio_emm_switch emm_switch;
- u64 clock;
+ u64 clock, emm_switch;
host->acquire_bus(host);
cvm_mmc_switch_to(slot);
@@ -865,20 +853,20 @@ static void cvm_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
if (clock)
clk_period = (host->sys_freq + clock - 1) / (2 * clock);
- emm_switch.val = 0;
- emm_switch.s.hs_timing = (ios->timing == MMC_TIMING_MMC_HS);
- emm_switch.s.bus_width = bus_width;
- emm_switch.s.power_class = power_class;
- emm_switch.s.clk_hi = clk_period;
- emm_switch.s.clk_lo = clk_period;
- emm_switch.s.bus_id = slot->bus_id;
+ emm_switch = FIELD_PREP(MIO_EMM_SWITCH_HS_TIMING,
+ (ios->timing == MMC_TIMING_MMC_HS)) |
+ FIELD_PREP(MIO_EMM_SWITCH_BUS_WIDTH, bus_width) |
+ FIELD_PREP(MIO_EMM_SWITCH_POWER_CLASS, power_class) |
+ FIELD_PREP(MIO_EMM_SWITCH_CLK_HI, clk_period) |
+ FIELD_PREP(MIO_EMM_SWITCH_CLK_LO, clk_period) |
+ FIELD_PREP(MIO_EMM_SWITCH_BUS_ID, slot->bus_id);
- if (!switch_val_changed(slot, emm_switch.val))
+ if (!switch_val_changed(slot, emm_switch))
goto out;
set_wdog(slot, 0);
- do_switch(host, emm_switch.val);
- slot->cached_switch = emm_switch.val;
+ do_switch(host, emm_switch);
+ slot->cached_switch = emm_switch;
out:
host->release_bus(host);
}
@@ -902,7 +890,7 @@ static void cvm_mmc_set_clock(struct cvm_mmc_slot *slot, unsigned int clock)
static int cvm_mmc_init_lowlevel(struct cvm_mmc_slot *slot)
{
struct cvm_mmc_host *host = slot->host;
- union mio_emm_switch emm_switch;
+ u64 emm_switch;
/* Enable this bus slot. */
host->emm_cfg |= (1ull << slot->bus_id);
@@ -911,16 +899,17 @@ static int cvm_mmc_init_lowlevel(struct cvm_mmc_slot *slot)
/* Program initial clock speed and power. */
cvm_mmc_set_clock(slot, slot->mmc->f_min);
- emm_switch.val = 0;
- emm_switch.s.power_class = 10;
- emm_switch.s.clk_hi = (host->sys_freq / slot->clock) / 2;
- emm_switch.s.clk_lo = (host->sys_freq / slot->clock) / 2;
+ emm_switch = FIELD_PREP(MIO_EMM_SWITCH_POWER_CLASS, 10);
+ emm_switch |= FIELD_PREP(MIO_EMM_SWITCH_CLK_HI,
+ (host->sys_freq / slot->clock) / 2);
+ emm_switch |= FIELD_PREP(MIO_EMM_SWITCH_CLK_LO,
+ (host->sys_freq / slot->clock) / 2);
/* Make the changes take effect on this bus slot. */
- emm_switch.s.bus_id = slot->bus_id;
- do_switch(host, emm_switch.val);
+ emm_switch |= FIELD_PREP(MIO_EMM_SWITCH_BUS_ID, slot->bus_id);
+ do_switch(host, emm_switch);
- slot->cached_switch = emm_switch.val;
+ slot->cached_switch = emm_switch;
/*
* Set watchdog timeout value and default reset value
diff --git a/drivers/mmc/host/cavium-mmc.h b/drivers/mmc/host/cavium-mmc.h
index 007f812..54aae61 100644
--- a/drivers/mmc/host/cavium-mmc.h
+++ b/drivers/mmc/host/cavium-mmc.h
@@ -7,6 +7,7 @@
*
* Copyright (C) 2012-2017 Cavium Inc.
*/
+#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/io.h>
#include <linux/mmc/host.h>
@@ -110,284 +111,94 @@ struct cvm_mmc_cr_mods {
/* Bitfield definitions */
-union mio_emm_dma_fifo_cfg {
- u64 val;
- struct mio_emm_dma_fifo_cfg_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :48;
- u64 clr:1;
- u64 :3;
- u64 int_lvl:4;
- u64 :3;
- u64 count:5;
-#else
- u64 count:5;
- u64 :3;
- u64 int_lvl:4;
- u64 :3;
- u64 clr:1;
- u64 :48;
-#endif
- } s;
-};
-
-union mio_emm_dma_fifo_cmd {
- u64 val;
- struct mio_emm_dma_fifo_cmd_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :1;
- u64 rw:1;
- u64 :1;
- u64 intdis:1;
- u64 swap32:1;
- u64 swap16:1;
- u64 swap8:1;
- u64 endian:1;
- u64 size:20;
- u64 :36;
-#else
- u64 :36;
- u64 size:20;
- u64 endian:1;
- u64 swap8:1;
- u64 swap16:1;
- u64 swap32:1;
- u64 intdis:1;
- u64 :1;
- u64 rw:1;
- u64 :1;
-#endif
- } s;
-};
-
-union mio_emm_cmd {
- u64 val;
- struct mio_emm_cmd_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :2;
- u64 bus_id:2;
- u64 cmd_val:1;
- u64 :3;
- u64 dbuf:1;
- u64 offset:6;
- u64 :6;
- u64 ctype_xor:2;
- u64 rtype_xor:3;
- u64 cmd_idx:6;
- u64 arg:32;
-#else
- u64 arg:32;
- u64 cmd_idx:6;
- u64 rtype_xor:3;
- u64 ctype_xor:2;
- u64 :6;
- u64 offset:6;
- u64 dbuf:1;
- u64 :3;
- u64 cmd_val:1;
- u64 bus_id:2;
- u64 :2;
-#endif
- } s;
-};
-
-union mio_emm_dma {
- u64 val;
- struct mio_emm_dma_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :2;
- u64 bus_id:2;
- u64 dma_val:1;
- u64 sector:1;
- u64 dat_null:1;
- u64 thres:6;
- u64 rel_wr:1;
- u64 rw:1;
- u64 multi:1;
- u64 block_cnt:16;
- u64 card_addr:32;
-#else
- u64 card_addr:32;
- u64 block_cnt:16;
- u64 multi:1;
- u64 rw:1;
- u64 rel_wr:1;
- u64 thres:6;
- u64 dat_null:1;
- u64 sector:1;
- u64 dma_val:1;
- u64 bus_id:2;
- u64 :2;
-#endif
- } s;
-};
-
-union mio_emm_dma_cfg {
- u64 val;
- struct mio_emm_dma_cfg_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 en:1;
- u64 rw:1;
- u64 clr:1;
- u64 :1;
- u64 swap32:1;
- u64 swap16:1;
- u64 swap8:1;
- u64 endian:1;
- u64 size:20;
- u64 adr:36;
-#else
- u64 adr:36;
- u64 size:20;
- u64 endian:1;
- u64 swap8:1;
- u64 swap16:1;
- u64 swap32:1;
- u64 :1;
- u64 clr:1;
- u64 rw:1;
- u64 en:1;
-#endif
- } s;
-};
-
-union mio_emm_int {
- u64 val;
- struct mio_emm_int_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :57;
- u64 switch_err:1;
- u64 switch_done:1;
- u64 dma_err:1;
- u64 cmd_err:1;
- u64 dma_done:1;
- u64 cmd_done:1;
- u64 buf_done:1;
-#else
- u64 buf_done:1;
- u64 cmd_done:1;
- u64 dma_done:1;
- u64 cmd_err:1;
- u64 dma_err:1;
- u64 switch_done:1;
- u64 switch_err:1;
- u64 :57;
-#endif
- } s;
-};
-
-union mio_emm_rsp_sts {
- u64 val;
- struct mio_emm_rsp_sts_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :2;
- u64 bus_id:2;
- u64 cmd_val:1;
- u64 switch_val:1;
- u64 dma_val:1;
- u64 dma_pend:1;
- u64 :27;
- u64 dbuf_err:1;
- u64 :4;
- u64 dbuf:1;
- u64 blk_timeout:1;
- u64 blk_crc_err:1;
- u64 rsp_busybit:1;
- u64 stp_timeout:1;
- u64 stp_crc_err:1;
- u64 stp_bad_sts:1;
- u64 stp_val:1;
- u64 rsp_timeout:1;
- u64 rsp_crc_err:1;
- u64 rsp_bad_sts:1;
- u64 rsp_val:1;
- u64 rsp_type:3;
- u64 cmd_type:2;
- u64 cmd_idx:6;
- u64 cmd_done:1;
-#else
- u64 cmd_done:1;
- u64 cmd_idx:6;
- u64 cmd_type:2;
- u64 rsp_type:3;
- u64 rsp_val:1;
- u64 rsp_bad_sts:1;
- u64 rsp_crc_err:1;
- u64 rsp_timeout:1;
- u64 stp_val:1;
- u64 stp_bad_sts:1;
- u64 stp_crc_err:1;
- u64 stp_timeout:1;
- u64 rsp_busybit:1;
- u64 blk_crc_err:1;
- u64 blk_timeout:1;
- u64 dbuf:1;
- u64 :4;
- u64 dbuf_err:1;
- u64 :27;
- u64 dma_pend:1;
- u64 dma_val:1;
- u64 switch_val:1;
- u64 cmd_val:1;
- u64 bus_id:2;
- u64 :2;
-#endif
- } s;
-};
-
-union mio_emm_sample {
- u64 val;
- struct mio_emm_sample_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :38;
- u64 cmd_cnt:10;
- u64 :6;
- u64 dat_cnt:10;
-#else
- u64 dat_cnt:10;
- u64 :6;
- u64 cmd_cnt:10;
- u64 :38;
-#endif
- } s;
-};
-
-union mio_emm_switch {
- u64 val;
- struct mio_emm_switch_s {
-#ifdef __BIG_ENDIAN_BITFIELD
- u64 :2;
- u64 bus_id:2;
- u64 switch_exe:1;
- u64 switch_err0:1;
- u64 switch_err1:1;
- u64 switch_err2:1;
- u64 :7;
- u64 hs_timing:1;
- u64 :5;
- u64 bus_width:3;
- u64 :4;
- u64 power_class:4;
- u64 clk_hi:16;
- u64 clk_lo:16;
-#else
- u64 clk_lo:16;
- u64 clk_hi:16;
- u64 power_class:4;
- u64 :4;
- u64 bus_width:3;
- u64 :5;
- u64 hs_timing:1;
- u64 :7;
- u64 switch_err2:1;
- u64 switch_err1:1;
- u64 switch_err0:1;
- u64 switch_exe:1;
- u64 bus_id:2;
- u64 :2;
-#endif
- } s;
-};
+#define MIO_EMM_DMA_FIFO_CFG_CLR BIT_ULL(16)
+#define MIO_EMM_DMA_FIFO_CFG_INT_LVL GENMASK_ULL(12, 8)
+#define MIO_EMM_DMA_FIFO_CFG_COUNT GENMASK_ULL(4, 0)
+
+#define MIO_EMM_DMA_FIFO_CMD_RW BIT_ULL(62)
+#define MIO_EMM_DMA_FIFO_CMD_INTDIS BIT_ULL(60)
+#define MIO_EMM_DMA_FIFO_CMD_SWAP32 BIT_ULL(59)
+#define MIO_EMM_DMA_FIFO_CMD_SWAP16 BIT_ULL(58)
+#define MIO_EMM_DMA_FIFO_CMD_SWAP8 BIT_ULL(57)
+#define MIO_EMM_DMA_FIFO_CMD_ENDIAN BIT_ULL(56)
+#define MIO_EMM_DMA_FIFO_CMD_SIZE GENMASK_ULL(55,36)
+
+#define MIO_EMM_CMD_SKIP_BUSY BIT_ULL(62)
+#define MIO_EMM_CMD_BUS_ID GENMASK_ULL(61, 60)
+#define MIO_EMM_CMD_VAL BIT_ULL(59)
+#define MIO_EMM_CMD_DBUF BIT_ULL(55)
+#define MIO_EMM_CMD_OFFSET GENMASK_ULL(54, 49)
+#define MIO_EMM_CMD_CTYPE_XOR GENMASK_ULL(42, 41)
+#define MIO_EMM_CMD_RTYPE_XOR GENMASK_ULL(40, 38)
+#define MIO_EMM_CMD_IDX GENMASK_ULL(37, 32)
+#define MIO_EMM_CMD_ARG GENMASK_ULL(31, 0)
+
+#define MIO_EMM_DMA_SKIP_BUSY BIT_ULL(62)
+#define MIO_EMM_DMA_BUS_ID GENMASK_ULL(61, 60)
+#define MIO_EMM_DMA_VAL BIT_ULL(59)
+#define MIO_EMM_DMA_SECTOR BIT_ULL(58)
+#define MIO_EMM_DMA_DAT_NULL BIT_ULL(57)
+#define MIO_EMM_DMA_THRES GENMASK_ULL(56, 51)
+#define MIO_EMM_DMA_REL_WR BIT_ULL(50)
+#define MIO_EMM_DMA_RW BIT_ULL(49)
+#define MIO_EMM_DMA_MULTI BIT_ULL(48)
+#define MIO_EMM_DMA_BLOCK_CNT GENMASK_ULL(47, 32)
+#define MIO_EMM_DMA_CARD_ADDR GENMASK_ULL(31, 0)
+
+#define MIO_EMM_DMA_CFG_EN BIT_ULL(63)
+#define MIO_EMM_DMA_CFG_RW BIT_ULL(62)
+#define MIO_EMM_DMA_CFG_CLR BIT_ULL(61)
+#define MIO_EMM_DMA_CFG_SWAP32 BIT_ULL(59)
+#define MIO_EMM_DMA_CFG_SWAP16 BIT_ULL(58)
+#define MIO_EMM_DMA_CFG_SWAP8 BIT_ULL(57)
+#define MIO_EMM_DMA_CFG_ENDIAN BIT_ULL(56)
+#define MIO_EMM_DMA_CFG_SIZE GENMASK_ULL(55, 36)
+#define MIO_EMM_DMA_CFG_ADR GENMASK_ULL(35, 0)
+
+#define MIO_EMM_INT_SWITCH_ERR BIT_ULL(6)
+#define MIO_EMM_INT_SWITCH_DONE BIT_ULL(5)
+#define MIO_EMM_INT_DMA_ERR BIT_ULL(4)
+#define MIO_EMM_INT_CMD_ERR BIT_ULL(3)
+#define MIO_EMM_INT_DMA_DONE BIT_ULL(2)
+#define MIO_EMM_INT_CMD_DONE BIT_ULL(1)
+#define MIO_EMM_INT_BUF_DONE BIT_ULL(0)
+
+#define MIO_EMM_RSP_STS_BUS_ID GENMASK_ULL(61, 60)
+#define MIO_EMM_RSP_STS_CMD_VAL BIT_ULL(59)
+#define MIO_EMM_RSP_STS_SWITCH_VAL BIT_ULL(58)
+#define MIO_EMM_RSP_STS_DMA_VAL BIT_ULL(57)
+#define MIO_EMM_RSP_STS_DMA_PEND BIT_ULL(56)
+#define MIO_EMM_RSP_STS_DBUF_ERR BIT_ULL(28)
+#define MIO_EMM_RSP_STS_DBUF BIT_ULL(23)
+#define MIO_EMM_RSP_STS_BLK_TIMEOUT BIT_ULL(22)
+#define MIO_EMM_RSP_STS_BLK_CRC_ERR BIT_ULL(21)
+#define MIO_EMM_RSP_STS_RSP_BUSYBIT BIT_ULL(20)
+#define MIO_EMM_RSP_STS_STP_TIMEOUT BIT_ULL(19)
+#define MIO_EMM_RSP_STS_STP_CRC_ERR BIT_ULL(18)
+#define MIO_EMM_RSP_STS_STP_BAD_STS BIT_ULL(17)
+#define MIO_EMM_RSP_STS_STP_VAL BIT_ULL(16)
+#define MIO_EMM_RSP_STS_RSP_TIMEOUT BIT_ULL(15)
+#define MIO_EMM_RSP_STS_RSP_CRC_ERR BIT_ULL(14)
+#define MIO_EMM_RSP_STS_RSP_BAD_STS BIT_ULL(13)
+#define MIO_EMM_RSP_STS_RSP_VAL BIT_ULL(12)
+#define MIO_EMM_RSP_STS_RSP_TYPE GENMASK_ULL(11, 9)
+#define MIO_EMM_RSP_STS_CMD_TYPE GENMASK_ULL(8, 7)
+#define MIO_EMM_RSP_STS_CMD_IDX GENMASK_ULL(6, 1)
+#define MIO_EMM_RSP_STS_CMD_DONE BIT_ULL(0)
+
+#define MIO_EMM_SAMPLE_CMD_CNT GENMASK_ULL(25, 16)
+#define MIO_EMM_SAMPLE_DAT_CNT GENMASK_ULL(9, 0)
+
+#define MIO_EMM_SWITCH_BUS_ID GENMASK_ULL(61, 60)
+#define MIO_EMM_SWITCH_EXE BIT_ULL(59)
+#define MIO_EMM_SWITCH_ERR0 BIT_ULL(58)
+#define MIO_EMM_SWITCH_ERR1 BIT_ULL(57)
+#define MIO_EMM_SWITCH_ERR2 BIT_ULL(56)
+#define MIO_EMM_SWITCH_HS_TIMING BIT_ULL(48)
+#define MIO_EMM_SWITCH_BUS_WIDTH GENMASK_ULL(42, 40)
+#define MIO_EMM_SWITCH_POWER_CLASS GENMASK_ULL(35, 32)
+#define MIO_EMM_SWITCH_CLK_HI GENMASK_ULL(31, 16)
+#define MIO_EMM_SWITCH_CLK_LO GENMASK_ULL(15, 0)
/* Protoypes */
irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id);
diff --git a/drivers/mmc/host/cavium-pci-thunderx.c b/drivers/mmc/host/cavium-pci-thunderx.c
index 8564612..7dc626a 100644
--- a/drivers/mmc/host/cavium-pci-thunderx.c
+++ b/drivers/mmc/host/cavium-pci-thunderx.c
@@ -155,7 +155,7 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
static void thunder_mmc_remove(struct pci_dev *pdev)
{
struct cvm_mmc_host *host = pci_get_drvdata(pdev);
- union mio_emm_dma_cfg dma_cfg;
+ u64 dma_cfg;
int i;
for (i = 0; i < CAVIUM_MAX_MMC; i++)
@@ -164,9 +164,9 @@ static void thunder_mmc_remove(struct pci_dev *pdev)
platform_device_del(slot_pdev[i]);
}
- dma_cfg.val = readq(host->dma_base + MIO_EMM_DMA_CFG(host));
- dma_cfg.s.en = 0;
- writeq(dma_cfg.val, host->dma_base + MIO_EMM_DMA_CFG(host));
+ dma_cfg = readq(host->dma_base + MIO_EMM_DMA_CFG(host));
+ dma_cfg &= ~MIO_EMM_DMA_CFG_EN;
+ writeq(dma_cfg, host->dma_base + MIO_EMM_DMA_CFG(host));
clk_disable_unprepare(host->clk);
}